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

[Kieran Bingham]

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.

--
Kieran