[libcamera-devel] [PATCH 09/11] ipa: ipu3: agc: Document AGC mean-based algorithm

[Jean-Michel Hautbois]

Hi Kieran,

On 22/10/2021 15:15, Laurent Pinchart wrote:
> On Fri, Oct 22, 2021 at 11:23:56AM +0100, Kieran Bingham wrote:
>> Hi,
>>
>> Seems I'm replying to quite an old mail, so I'm not sure if this is
>> already done/handled/relevant, but in case it is...
>>
>> Quoting Laurent Pinchart (2021-09-14 17:18:47)
>>> Hi Kieran,
>>>
>>> On Tue, Sep 14, 2021 at 12:20:14PM +0100, Kieran Bingham wrote:
>>>> On 14/09/2021 06:21, Laurent Pinchart wrote:
>>>>>>   {
>>>>>> +  /*
>>>>>> +   * Get the applied grid from the statistics buffer. When the kernel
>>>>>> +   * receives a grid from the parameters buffer, it will check and align
>>>>>> +   * all the values. For instance, it will automatically fill the x_end
>>>>>> +   * value based on x_start, grid width and log2 width.
>>>>>> +   * \todo Use the grid calculated in configure as there is a bug in IPU3
>>>>>> +   * causing the width (maybe height) to be bit-shifted.
>>>>>
>>>>> Don't we do so already ?
>>>>
>>>> Is this relating to the line directly below which references the grid
>>>> from the stats->stats_4a_config.awb_config.grid?
>>>>
>>>> Can we simply/directly reference our grid from our IPAContext?
>>>>
>>>> And if so - it might be better to do that immediately, rather than add
>>>> the \todo.
>>>>
>>>> And if we do that we should mention that it is referenced from our grid,
>>>> rather than the one provided in the statistics due to a kernel bug..
>>>
>>> I meant that we use grid.width, with the following comment:
>>>
>>>   * We observed a bit shift which makes the value 160 to
>>>   * be 32 in the stats grid. Use the one from configure.
>>>
>>> Isn't this what the \todo is about ? Or did I misundertand either the
>>> code of the \todo ?
>>
>> My statement was that the todo being added sounds so simple - it would
>> be better to add a patch to *do* the thing, rather than add a todo.
>>
>> If you believe the action of the todo is already done, then that's even
>> better.
>>
>> Either way, I don't think this should be added as a todo - and it should
>> be ... done ;-), and perhaps it already is. If it isn't then can we do
>> it please? It sounds like a very short patch if it isn't done already.
> 
> I understand it as being done already, but I could be wrong.
> 

I think it has been done... I don't have that patch in the "Document 
IPAIPU3" v3 series ;-)