[PATCH v8 4/6] media: rockchip: rkisp1: reduce number of histogram grid elements in uapi

[Helen Koike]

On 1/21/21 2:12 PM, Heiko Stübner wrote:
> Hi Helen,
> 
> Am Donnerstag, 21. Januar 2021, 18:04:49 CET schrieb Helen Koike:
>> Hi Dafna,
>>
>> On 1/21/21 11:44 AM, Dafna Hirschfeld wrote:
>>> From: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
>>>
>>> The uapi right now specifies an array size of 28 but the actual number
>>> of elements is only 25 with the last 3 being unused.
>>>
>>> Reduce the array size to the correct number of elements and change
>>> the params code to iterate the array 25 times.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
>>> ---
>>>  drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 3 ++-
>>>  include/uapi/linux/rkisp1-config.h                     | 3 +--
>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
>>> index 6af4d551ffb5..021939466b24 100644
>>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
>>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
>>> @@ -589,7 +589,6 @@ static void rkisp1_hst_config(struct rkisp1_params *params,
>>>  		RKISP1_CIF_ISP_HIST_WEIGHT_22TO03,
>>>  		RKISP1_CIF_ISP_HIST_WEIGHT_13TO43,
>>>  		RKISP1_CIF_ISP_HIST_WEIGHT_04TO34,
>>> -		RKISP1_CIF_ISP_HIST_WEIGHT_44,
>>>  	};
>>>  	const u8 *weight;
>>>  	unsigned int i;
>>> @@ -622,6 +621,8 @@ static void rkisp1_hst_config(struct rkisp1_params *params,
>>>  							    weight[2],
>>>  							    weight[3]),
>>>  				 hist_weight_regs[i]);
>>> +
>>> +	rkisp1_write(params->rkisp1, weight[0] & 0x1F, RKISP1_CIF_ISP_HIST_WEIGHT_44);
>>
>> I just noticed that the default value to arg->hist_weight was:
>>
>> 	memset(hst.hist_weight, 0x01, sizeof(hst.hist_weight));
>>
>> Shouldn't the value be RKISP1_CIF_ISP_HIST_WEIGHT_SET(weight[0], 0x01, 0x01, 0x01) ?
>> Or am I missing something?
> 
> The last register only has that one value from weight[24];
> 
> I.e. before we were writing the 4 values into that last register as well
> from indices 24, 25, 26, 27 ... hence the 3 unused values in the weight array.
> 
> But that was merely a crutch to make that fit into the write loop.

right, lgtm, thank you and Dafna for working on this.

For the whole series:

Acked-by: Helen Koike <helen.koike at collabora.com>

Regards,
Helen

> 
> 
> Heiko
> 
> 
>>>  }
>>>  
>>>  static void
>>> diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
>>> index 24f05d6d258f..35aa82d5f6dd 100644
>>> --- a/include/uapi/linux/rkisp1-config.h
>>> +++ b/include/uapi/linux/rkisp1-config.h
>>> @@ -102,8 +102,7 @@
>>>  /*
>>>   * Histogram calculation
>>>   */
>>> -/* Last 3 values unused. */
>>> -#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE 28
>>> +#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE 25
>>>  
>>>  /*
>>>   * Defect Pixel Cluster Correction
>>>
>>
> 
> 
> 
>