All,
________________________
BRs,
Bingbu Cao
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Sent: Wednesday, October 6, 2021 4:54 AM
> To: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxxxxxxxx>
> Cc: linux-media@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx; Cao,
> Bingbu <bingbu.cao@xxxxxxxxx>; tfiga@xxxxxxxxxx; Qiu, Tian Shu
> <tian.shu.qiu@xxxxxxxxx>
> Subject: Re: [PATCH v2] media: staging: ipu3-imgu: add the AWB memory
> layout
>
> Hi Jean-Michel,
>
> Thank you for the patch.
>
> On Tue, Oct 05, 2021 at 10:20:19PM +0200, Jean-Michel Hautbois wrote:
> > While parsing the RAW AWB metadata, the AWB layout was missing to
> > fully understand which byte corresponds to which feature. Make the
> > field names and usage explicit, as it is used by the userspace
> applications.
> >
> > Signed-off-by: Jean-Michel Hautbois
> > <jeanmichel.hautbois@xxxxxxxxxxxxxxxx>
> > ---
> > .../media/ipu3/include/uapi/intel-ipu3.h | 32 ++++++++++++++++---
> > 1 file changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > index 585f55981c86..ad116a912e44 100644
> > --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > @@ -61,17 +61,39 @@ struct ipu3_uapi_grid_config {
> > __u16 y_end;
> > } __packed;
> >
> > +/**
> > + * struct ipu3_uapi_awb_set_item - Memory layout for each cell in AWB
> > + *
> > + * @Gr_avg: Green average for red lines in the cell.
> > + * @R_avg: Red average in the cell.
> > + * @B_avg: Blue average in the cell.
> > + * @Gb_avg: Green average for blue lines in the cell.
> > + * @sat_ratio: Percentage of pixels over a given threshold set in
>
> s/over a given threshold set in/over the thresholds specified in/
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> I would still like a review from someone knowledgeable with the hardware
> and firmware, as we're partly guessing here.
Reviewed-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
I have no concern on this patch, I think it has minor impact on user-space
implementation from this change. Tomasz, do you have any comment on this?
>
> > + * ipu3_uapi_awb_config_s, coded from 0 to 255.
> > + * @padding0: Unused byte for padding.
> > + * @padding1: Unused byte for padding.
> > + * @padding2: Unused byte for padding.
> > + */
> > +struct ipu3_uapi_awb_set_item {
> > + __u8 Gr_avg;
> > + __u8 R_avg;
> > + __u8 B_avg;
> > + __u8 Gb_avg;
> > + __u8 sat_ratio;
> > + __u8 padding0;
> > + __u8 padding1;
> > + __u8 padding2;
> > +} __attribute__((packed));
> > +
> > /*
> > * The grid based data is divided into "slices" called set, each slice
> of setX
> > * refers to ipu3_uapi_grid_config width * height_per_slice.
> > */
> > #define IPU3_UAPI_AWB_MAX_SETS 60
> > /* Based on grid size 80 * 60 and cell size 16 x 16 */
> > -#define IPU3_UAPI_AWB_SET_SIZE 1280
> > -#define IPU3_UAPI_AWB_MD_ITEM_SIZE 8
> > +#define IPU3_UAPI_AWB_SET_SIZE 160
> > #define IPU3_UAPI_AWB_SPARE_FOR_BUBBLES \
> > - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> > - IPU3_UAPI_AWB_MD_ITEM_SIZE)
> > + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES)
> > #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
> > (IPU3_UAPI_AWB_MAX_SETS * \
> > (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) @@
> > -83,7 +105,7 @@ struct ipu3_uapi_grid_config {
> > * the average values for each color channel.
> > */
> > struct ipu3_uapi_awb_raw_buffer {
> > - __u8 meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
> > + struct ipu3_uapi_awb_set_item
> > +meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
> > __attribute__((aligned(32)));
> > } __packed;
> >
>
> --
> Regards,
>
> Laurent Pinchart
