* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-08-31 18:51 [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout Jean-Michel Hautbois
@ 2021-08-31 21:33 ` Laurent Pinchart
2021-09-08 8:17 ` Laurent Pinchart
2021-09-01 1:00 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2021-08-31 21:33 UTC (permalink / raw)
To: Jean-Michel Hautbois; +Cc: linux-media, sakari.ailus, bingbu.cao, Tian Shu Qiu
Hi Jean-Michel,
Just replying to CC Tian Shu as well.
On Tue, Aug 31, 2021 at 08:51:40PM +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@ideasonboard.com>
> ---
> This structure layout is defined in CrOs:
> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
>
> There are a few things not really understood right now:
> - Is sat_ratio a full scale ratio (I can't get more than some values out
> of it, is it a ratio of 25%, 50%, 75%, 100% ?)
> - What are the real minimum and maximum values for the grid size ? From
> CrOs it appears to be [16, 80] for width and [16, 60] for height while
> in this file it seems to be [16, 160] for width and not really defined
> for height AFAICT ?
> - Same for the block_width_log2 and block_height_log2 which are [3, 7]
> in this file and [3, 6] in the awb_public.h header ?
>
> .../media/ipu3/include/uapi/intel-ipu3.h | 38 ++++++++++++++-----
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> index fa3d6ee5adf2..83191aff2ddd 100644
> --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> @@ -61,20 +61,40 @@ struct ipu3_uapi_grid_config {
> __u16 y_end;
> } __packed;
>
> +/**
> + * struct ipu3_uapi_awb_raw_buffer - 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: Saturation ratio in the cell.
> + * @padding0: Unused byte for padding.
> + * @padding1: Unused byte for padding.
> + * @padding2: Unused byte for padding.
> + */
> +struct ipu3_uapi_awb_raw_buffer {
> + unsigned char Gr_avg;
> + unsigned char R_avg;
> + unsigned char B_avg;
> + unsigned char Gb_avg;
> + unsigned char sat_ratio;
> + unsigned char padding0;
> + unsigned char padding1;
> + unsigned char padding2;
> +} __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_SPARE_FOR_BUBBLES \
> - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> - IPU3_UAPI_AWB_MD_ITEM_SIZE)
> +#define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
> +/* Based on max grid height + Spare for bubbles */
> +#define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
> + (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))
> + AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
>
> /**
> * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
> @@ -83,7 +103,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_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
> __attribute__((aligned(32)));
> } __packed;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-08-31 21:33 ` Laurent Pinchart
@ 2021-09-08 8:17 ` Laurent Pinchart
2021-09-09 2:19 ` Bingbu Cao
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2021-09-08 8:17 UTC (permalink / raw)
To: bingbu.cao, Tian Shu Qiu, sakari.ailus
Cc: linux-media, Jean-Michel Hautbois, Tomasz Figa
Hello,
(CC'ing Tomasz)
Gentle ping.
On Wed, Sep 01, 2021 at 12:34:00AM +0300, Laurent Pinchart wrote:
> On Tue, Aug 31, 2021 at 08:51:40PM +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@ideasonboard.com>
> > ---
> > This structure layout is defined in CrOs:
> > https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
> >
> > There are a few things not really understood right now:
> > - Is sat_ratio a full scale ratio (I can't get more than some values out
> > of it, is it a ratio of 25%, 50%, 75%, 100% ?)
> > - What are the real minimum and maximum values for the grid size ? From
> > CrOs it appears to be [16, 80] for width and [16, 60] for height while
> > in this file it seems to be [16, 160] for width and not really defined
> > for height AFAICT ?
> > - Same for the block_width_log2 and block_height_log2 which are [3, 7]
> > in this file and [3, 6] in the awb_public.h header ?
> >
> > .../media/ipu3/include/uapi/intel-ipu3.h | 38 ++++++++++++++-----
> > 1 file changed, 29 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > index fa3d6ee5adf2..83191aff2ddd 100644
> > --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> > @@ -61,20 +61,40 @@ struct ipu3_uapi_grid_config {
> > __u16 y_end;
> > } __packed;
> >
> > +/**
> > + * struct ipu3_uapi_awb_raw_buffer - 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: Saturation ratio in the cell.
> > + * @padding0: Unused byte for padding.
> > + * @padding1: Unused byte for padding.
> > + * @padding2: Unused byte for padding.
> > + */
> > +struct ipu3_uapi_awb_raw_buffer {
> > + unsigned char Gr_avg;
> > + unsigned char R_avg;
> > + unsigned char B_avg;
> > + unsigned char Gb_avg;
> > + unsigned char sat_ratio;
> > + unsigned char padding0;
> > + unsigned char padding1;
> > + unsigned char padding2;
> > +} __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_SPARE_FOR_BUBBLES \
> > - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> > - IPU3_UAPI_AWB_MD_ITEM_SIZE)
> > +#define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
> > +/* Based on max grid height + Spare for bubbles */
> > +#define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
> > + (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))
> > + AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
> >
> > /**
> > * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
> > @@ -83,7 +103,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_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
> > __attribute__((aligned(32)));
> > } __packed;
> >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-09-08 8:17 ` Laurent Pinchart
@ 2021-09-09 2:19 ` Bingbu Cao
2021-09-09 5:52 ` Jean-Michel Hautbois
0 siblings, 1 reply; 10+ messages in thread
From: Bingbu Cao @ 2021-09-09 2:19 UTC (permalink / raw)
To: Laurent Pinchart, bingbu.cao, Tian Shu Qiu, sakari.ailus
Cc: linux-media, Jean-Michel Hautbois, Tomasz Figa
Jean-Michel,
Thanks for your patch.
On 9/8/21 4:17 PM, Laurent Pinchart wrote:
> Hello,
>
> (CC'ing Tomasz)
>
> Gentle ping.
>
> On Wed, Sep 01, 2021 at 12:34:00AM +0300, Laurent Pinchart wrote:
>> On Tue, Aug 31, 2021 at 08:51:40PM +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@ideasonboard.com>
>>> ---
>>> This structure layout is defined in CrOs:
>>> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
>>>
>>> There are a few things not really understood right now:
>>> - Is sat_ratio a full scale ratio (I can't get more than some values out
>>> of it, is it a ratio of 25%, 50%, 75%, 100% ?)
>>> - What are the real minimum and maximum values for the grid size ? From
>>> CrOs it appears to be [16, 80] for width and [16, 60] for height while
>>> in this file it seems to be [16, 160] for width and not really defined
>>> for height AFAICT ?
>>> - Same for the block_width_log2 and block_height_log2 which are [3, 7]
>>> in this file and [3, 6] in the awb_public.h header ?
>>>
>>> .../media/ipu3/include/uapi/intel-ipu3.h | 38 ++++++++++++++-----
>>> 1 file changed, 29 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>>> index fa3d6ee5adf2..83191aff2ddd 100644
>>> --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>>> +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>>> @@ -61,20 +61,40 @@ struct ipu3_uapi_grid_config {
>>> __u16 y_end;
>>> } __packed;
>>>
>>> +/**
>>> + * struct ipu3_uapi_awb_raw_buffer - 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: Saturation ratio in the cell.
>>> + * @padding0: Unused byte for padding.
>>> + * @padding1: Unused byte for padding.
>>> + * @padding2: Unused byte for padding.
>>> + */
>>> +struct ipu3_uapi_awb_raw_buffer {
>>> + unsigned char Gr_avg;
>>> + unsigned char R_avg;
>>> + unsigned char B_avg;
>>> + unsigned char Gb_avg;
>>> + unsigned char sat_ratio;
>>> + unsigned char padding0;
>>> + unsigned char padding1;
>>> + unsigned char padding2;
It is fine for me to define and exposure the awb memory layout in uAPI.
nit: use __u8 here?
>>> +} __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_SPARE_FOR_BUBBLES \
>>> - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
>>> - IPU3_UAPI_AWB_MD_ITEM_SIZE)
>>> +#define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
>>> +/* Based on max grid height + Spare for bubbles */
>>> +#define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
>>> + (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))
>>> + AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
It's better to update the name of 'IPU3_UAPI_AWB_MAX_BUFFER_SIZE' to align current
definition.
>>>
>>> /**
>>> * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
>>> @@ -83,7 +103,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_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
>>> __attribute__((aligned(32)));
>>> } __packed;
>>>
>
--
Best regards,
Bingbu Cao
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-09-09 2:19 ` Bingbu Cao
@ 2021-09-09 5:52 ` Jean-Michel Hautbois
2021-09-21 11:33 ` Laurent Pinchart
0 siblings, 1 reply; 10+ messages in thread
From: Jean-Michel Hautbois @ 2021-09-09 5:52 UTC (permalink / raw)
To: Bingbu Cao, Laurent Pinchart, bingbu.cao, Tian Shu Qiu, sakari.ailus
Cc: linux-media, Tomasz Figa
Hi Bingbu, thanks for your answer !
On 09/09/2021 04:19, Bingbu Cao wrote:
> Jean-Michel,
>
> Thanks for your patch.
>
> On 9/8/21 4:17 PM, Laurent Pinchart wrote:
>> Hello,
>>
>> (CC'ing Tomasz)
>>
>> Gentle ping.
>>
>> On Wed, Sep 01, 2021 at 12:34:00AM +0300, Laurent Pinchart wrote:
>>> On Tue, Aug 31, 2021 at 08:51:40PM +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@ideasonboard.com>
>>>> ---
>>>> This structure layout is defined in CrOs:
>>>> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
>>>>
>>>> There are a few things not really understood right now:
>>>> - Is sat_ratio a full scale ratio (I can't get more than some values out
>>>> of it, is it a ratio of 25%, 50%, 75%, 100% ?)
>>>> - What are the real minimum and maximum values for the grid size ? From
>>>> CrOs it appears to be [16, 80] for width and [16, 60] for height while
>>>> in this file it seems to be [16, 160] for width and not really defined
>>>> for height AFAICT ?
>>>> - Same for the block_width_log2 and block_height_log2 which are [3, 7]
>>>> in this file and [3, 6] in the awb_public.h header ?
Do you have any clue for those questions please :-) ?
>>>>
>>>> .../media/ipu3/include/uapi/intel-ipu3.h | 38 ++++++++++++++-----
>>>> 1 file changed, 29 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>>>> index fa3d6ee5adf2..83191aff2ddd 100644
>>>> --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>>>> +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>>>> @@ -61,20 +61,40 @@ struct ipu3_uapi_grid_config {
>>>> __u16 y_end;
>>>> } __packed;
>>>>
>>>> +/**
>>>> + * struct ipu3_uapi_awb_raw_buffer - 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: Saturation ratio in the cell.
>>>> + * @padding0: Unused byte for padding.
>>>> + * @padding1: Unused byte for padding.
>>>> + * @padding2: Unused byte for padding.
>>>> + */
>>>> +struct ipu3_uapi_awb_raw_buffer {
>>>> + unsigned char Gr_avg;
>>>> + unsigned char R_avg;
>>>> + unsigned char B_avg;
>>>> + unsigned char Gb_avg;
>>>> + unsigned char sat_ratio;
>>>> + unsigned char padding0;
>>>> + unsigned char padding1;
>>>> + unsigned char padding2;
>
> It is fine for me to define and exposure the awb memory layout in uAPI.
>
> nit: use __u8 here?
Sure !
>
>>>> +} __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_SPARE_FOR_BUBBLES \
>>>> - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
>>>> - IPU3_UAPI_AWB_MD_ITEM_SIZE)
>>>> +#define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
>>>> +/* Based on max grid height + Spare for bubbles */
>>>> +#define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
>>>> + (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))
>>>> + AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
>
> It's better to update the name of 'IPU3_UAPI_AWB_MAX_BUFFER_SIZE' to align current
> definition.
>
>>>>
>>>> /**
>>>> * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
>>>> @@ -83,7 +103,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_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
>>>> __attribute__((aligned(32)));
>>>> } __packed;
>>>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-09-09 5:52 ` Jean-Michel Hautbois
@ 2021-09-21 11:33 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2021-09-21 11:33 UTC (permalink / raw)
To: Bingbu Cao, bingbu.cao
Cc: Jean-Michel Hautbois, Tian Shu Qiu, sakari.ailus, linux-media,
Tomasz Figa
Hello,
On Thu, Sep 09, 2021 at 07:52:46AM +0200, Jean-Michel Hautbois wrote:
> On 09/09/2021 04:19, Bingbu Cao wrote:
> > On 9/8/21 4:17 PM, Laurent Pinchart wrote:
> >> On Wed, Sep 01, 2021 at 12:34:00AM +0300, Laurent Pinchart wrote:
> >>> On Tue, Aug 31, 2021 at 08:51:40PM +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@ideasonboard.com>
> >>>> ---
> >>>> This structure layout is defined in CrOs:
> >>>> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
> >>>>
> >>>> There are a few things not really understood right now:
> >>>> - Is sat_ratio a full scale ratio (I can't get more than some values out
> >>>> of it, is it a ratio of 25%, 50%, 75%, 100% ?)
> >>>> - What are the real minimum and maximum values for the grid size ? From
> >>>> CrOs it appears to be [16, 80] for width and [16, 60] for height while
> >>>> in this file it seems to be [16, 160] for width and not really defined
> >>>> for height AFAICT ?
> >>>> - Same for the block_width_log2 and block_height_log2 which are [3, 7]
> >>>> in this file and [3, 6] in the awb_public.h header ?
>
> Do you have any clue for those questions please :-) ?
This is becoming a blocker, it would be really nice if we could have
answers to these questions. The grid size constraints are the most
immediate priority, but understanding the sat_ratio value will be next
veyr shortly.
> >>>>
> >>>> .../media/ipu3/include/uapi/intel-ipu3.h | 38 ++++++++++++++-----
> >>>> 1 file changed, 29 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> >>>> index fa3d6ee5adf2..83191aff2ddd 100644
> >>>> --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> >>>> +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> >>>> @@ -61,20 +61,40 @@ struct ipu3_uapi_grid_config {
> >>>> __u16 y_end;
> >>>> } __packed;
> >>>>
> >>>> +/**
> >>>> + * struct ipu3_uapi_awb_raw_buffer - 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: Saturation ratio in the cell.
> >>>> + * @padding0: Unused byte for padding.
> >>>> + * @padding1: Unused byte for padding.
> >>>> + * @padding2: Unused byte for padding.
> >>>> + */
> >>>> +struct ipu3_uapi_awb_raw_buffer {
> >>>> + unsigned char Gr_avg;
> >>>> + unsigned char R_avg;
> >>>> + unsigned char B_avg;
> >>>> + unsigned char Gb_avg;
> >>>> + unsigned char sat_ratio;
> >>>> + unsigned char padding0;
> >>>> + unsigned char padding1;
> >>>> + unsigned char padding2;
> >
> > It is fine for me to define and exposure the awb memory layout in uAPI.
> >
> > nit: use __u8 here?
>
> Sure !
>
> >>>> +} __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_SPARE_FOR_BUBBLES \
> >>>> - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> >>>> - IPU3_UAPI_AWB_MD_ITEM_SIZE)
> >>>> +#define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
> >>>> +/* Based on max grid height + Spare for bubbles */
> >>>> +#define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
> >>>> + (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))
> >>>> + AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
> >
> > It's better to update the name of 'IPU3_UAPI_AWB_MAX_BUFFER_SIZE' to align current
> > definition.
> >
> >>>>
> >>>> /**
> >>>> * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
> >>>> @@ -83,7 +103,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_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
> >>>> __attribute__((aligned(32)));
> >>>> } __packed;
> >>>>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-08-31 18:51 [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout Jean-Michel Hautbois
@ 2021-09-01 1:00 ` kernel test robot
2021-09-01 1:00 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-09-01 1:00 UTC (permalink / raw)
To: Jean-Michel Hautbois; +Cc: llvm, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6390 bytes --]
Hi Jean-Michel,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on staging/staging-testing]
url: https://github.com/0day-ci/linux/commits/Jean-Michel-Hautbois/media-staging-ipu3-imgu-add-the-AWB-memory-layout/20210901-025225
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 4adb389e08c95fdf91995271932c59250ff0d561
config: x86_64-randconfig-a013-20210831 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e9de209cdb5156e33f665630622ea9f4769937f3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jean-Michel-Hautbois/media-staging-ipu3-imgu-add-the-AWB-memory-layout/20210901-025225
git checkout e9de209cdb5156e33f665630622ea9f4769937f3
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/staging/media/ipu3/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/staging/media/ipu3/ipu3-dmamap.c:12:
In file included from drivers/staging/media/ipu3/ipu3.h:14:
In file included from drivers/staging/media/ipu3/ipu3-css.h:10:
In file included from drivers/staging/media/ipu3/ipu3-abi.h:7:
>> drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:105:8: error: redefinition of 'ipu3_uapi_awb_raw_buffer'
struct ipu3_uapi_awb_raw_buffer {
^
drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:76:8: note: previous definition is here
struct ipu3_uapi_awb_raw_buffer {
^
1 error generated.
--
In file included from drivers/staging/media/ipu3/ipu3-css-params.c:6:
In file included from drivers/staging/media/ipu3/ipu3-css.h:10:
In file included from drivers/staging/media/ipu3/ipu3-abi.h:7:
>> drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:105:8: error: redefinition of 'ipu3_uapi_awb_raw_buffer'
struct ipu3_uapi_awb_raw_buffer {
^
drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:76:8: note: previous definition is here
struct ipu3_uapi_awb_raw_buffer {
^
drivers/staging/media/ipu3/ipu3-css-params.c:774:8: warning: variable 'pin_scale' set but not used [-Wunused-but-set-variable]
int pin_scale = 0;
^
1 warning and 1 error generated.
vim +/ipu3_uapi_awb_raw_buffer +105 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 86
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 87 /*
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 88 * The grid based data is divided into "slices" called set, each slice of setX
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 89 * refers to ipu3_uapi_grid_config width * height_per_slice.
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 90 */
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 91 #define IPU3_UAPI_AWB_MAX_SETS 60
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 92 #define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 93 /* Based on max grid height + Spare for bubbles */
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 94 #define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 95 (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES)
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 96 #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 97 AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 98
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 99 /**
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 100 * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 101 *
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 102 * @meta_data: buffer to hold auto white balance meta data which is
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 103 * the average values for each color channel.
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 104 */
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 @105 struct ipu3_uapi_awb_raw_buffer {
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 106 struct ipu3_uapi_awb_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 107 __attribute__((aligned(32)));
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 108 } __packed;
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 109
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37750 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
@ 2021-09-01 1:00 ` kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-09-01 1:00 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6474 bytes --]
Hi Jean-Michel,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on staging/staging-testing]
url: https://github.com/0day-ci/linux/commits/Jean-Michel-Hautbois/media-staging-ipu3-imgu-add-the-AWB-memory-layout/20210901-025225
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 4adb389e08c95fdf91995271932c59250ff0d561
config: x86_64-randconfig-a013-20210831 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e9de209cdb5156e33f665630622ea9f4769937f3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jean-Michel-Hautbois/media-staging-ipu3-imgu-add-the-AWB-memory-layout/20210901-025225
git checkout e9de209cdb5156e33f665630622ea9f4769937f3
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/staging/media/ipu3/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/staging/media/ipu3/ipu3-dmamap.c:12:
In file included from drivers/staging/media/ipu3/ipu3.h:14:
In file included from drivers/staging/media/ipu3/ipu3-css.h:10:
In file included from drivers/staging/media/ipu3/ipu3-abi.h:7:
>> drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:105:8: error: redefinition of 'ipu3_uapi_awb_raw_buffer'
struct ipu3_uapi_awb_raw_buffer {
^
drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:76:8: note: previous definition is here
struct ipu3_uapi_awb_raw_buffer {
^
1 error generated.
--
In file included from drivers/staging/media/ipu3/ipu3-css-params.c:6:
In file included from drivers/staging/media/ipu3/ipu3-css.h:10:
In file included from drivers/staging/media/ipu3/ipu3-abi.h:7:
>> drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:105:8: error: redefinition of 'ipu3_uapi_awb_raw_buffer'
struct ipu3_uapi_awb_raw_buffer {
^
drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:76:8: note: previous definition is here
struct ipu3_uapi_awb_raw_buffer {
^
drivers/staging/media/ipu3/ipu3-css-params.c:774:8: warning: variable 'pin_scale' set but not used [-Wunused-but-set-variable]
int pin_scale = 0;
^
1 warning and 1 error generated.
vim +/ipu3_uapi_awb_raw_buffer +105 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 86
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 87 /*
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 88 * The grid based data is divided into "slices" called set, each slice of setX
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 89 * refers to ipu3_uapi_grid_config width * height_per_slice.
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 90 */
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 91 #define IPU3_UAPI_AWB_MAX_SETS 60
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 92 #define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 93 /* Based on max grid height + Spare for bubbles */
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 94 #define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 95 (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES)
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 96 #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 97 AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 98
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 99 /**
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 100 * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 101 *
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 102 * @meta_data: buffer to hold auto white balance meta data which is
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 103 * the average values for each color channel.
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 104 */
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 @105 struct ipu3_uapi_awb_raw_buffer {
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 106 struct ipu3_uapi_awb_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 107 __attribute__((aligned(32)));
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 108 } __packed;
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 109
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37750 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-08-31 18:51 [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout Jean-Michel Hautbois
2021-08-31 21:33 ` Laurent Pinchart
2021-09-01 1:00 ` kernel test robot
@ 2021-09-01 2:35 ` kernel test robot
2021-09-21 11:29 ` Laurent Pinchart
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-09-01 2:35 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5713 bytes --]
Hi Jean-Michel,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on staging/staging-testing]
url: https://github.com/0day-ci/linux/commits/Jean-Michel-Hautbois/media-staging-ipu3-imgu-add-the-AWB-memory-layout/20210901-025225
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 4adb389e08c95fdf91995271932c59250ff0d561
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/e9de209cdb5156e33f665630622ea9f4769937f3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jean-Michel-Hautbois/media-staging-ipu3-imgu-add-the-AWB-memory-layout/20210901-025225
git checkout e9de209cdb5156e33f665630622ea9f4769937f3
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/staging/media/ipu3/ipu3-abi.h:7,
from drivers/staging/media/ipu3/ipu3-css.h:10,
from drivers/staging/media/ipu3/ipu3.h:14,
from drivers/staging/media/ipu3/ipu3-dmamap.c:12:
>> drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:105:8: error: redefinition of 'struct ipu3_uapi_awb_raw_buffer'
105 | struct ipu3_uapi_awb_raw_buffer {
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:76:8: note: originally defined here
76 | struct ipu3_uapi_awb_raw_buffer {
| ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/media/ipu3/include/uapi/intel-ipu3.h:106:34: error: array type has incomplete element type 'struct ipu3_uapi_awb_raw_buffer'
106 | struct ipu3_uapi_awb_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
| ^~~~~~~~~
vim +105 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 86
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 87 /*
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 88 * The grid based data is divided into "slices" called set, each slice of setX
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 89 * refers to ipu3_uapi_grid_config width * height_per_slice.
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 90 */
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 91 #define IPU3_UAPI_AWB_MAX_SETS 60
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 92 #define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 93 /* Based on max grid height + Spare for bubbles */
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 94 #define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 95 (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES)
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 96 #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 97 AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 98
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 99 /**
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 100 * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 101 *
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 102 * @meta_data: buffer to hold auto white balance meta data which is
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 103 * the average values for each color channel.
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 104 */
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 @105 struct ipu3_uapi_awb_raw_buffer {
e9de209cdb5156 drivers/staging/media/ipu3/include/uapi/intel-ipu3.h Jean-Michel Hautbois 2021-08-31 @106 struct ipu3_uapi_awb_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 107 __attribute__((aligned(32)));
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 108 } __packed;
41158dabfd913c drivers/staging/media/ipu3/include/intel-ipu3.h Yong Zhi 2018-12-06 109
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65653 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout
2021-08-31 18:51 [RFC PATCH] media: staging: ipu3-imgu: add the AWB memory layout Jean-Michel Hautbois
` (2 preceding siblings ...)
2021-09-01 2:35 ` kernel test robot
@ 2021-09-21 11:29 ` Laurent Pinchart
3 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2021-09-21 11:29 UTC (permalink / raw)
To: Jean-Michel Hautbois; +Cc: linux-media, sakari.ailus, bingbu.cao
Hi Jean-Michel,
Another comment.
On Tue, Aug 31, 2021 at 08:51:40PM +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@ideasonboard.com>
> ---
> This structure layout is defined in CrOs:
> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
>
> There are a few things not really understood right now:
> - Is sat_ratio a full scale ratio (I can't get more than some values out
> of it, is it a ratio of 25%, 50%, 75%, 100% ?)
> - What are the real minimum and maximum values for the grid size ? From
> CrOs it appears to be [16, 80] for width and [16, 60] for height while
> in this file it seems to be [16, 160] for width and not really defined
> for height AFAICT ?
> - Same for the block_width_log2 and block_height_log2 which are [3, 7]
> in this file and [3, 6] in the awb_public.h header ?
>
> .../media/ipu3/include/uapi/intel-ipu3.h | 38 ++++++++++++++-----
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> index fa3d6ee5adf2..83191aff2ddd 100644
> --- a/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> +++ b/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
> @@ -61,20 +61,40 @@ struct ipu3_uapi_grid_config {
> __u16 y_end;
> } __packed;
>
> +/**
> + * struct ipu3_uapi_awb_raw_buffer - 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: Saturation ratio in the cell.
> + * @padding0: Unused byte for padding.
> + * @padding1: Unused byte for padding.
> + * @padding2: Unused byte for padding.
> + */
> +struct ipu3_uapi_awb_raw_buffer {
> + unsigned char Gr_avg;
> + unsigned char R_avg;
> + unsigned char B_avg;
> + unsigned char Gb_avg;
> + unsigned char sat_ratio;
> + unsigned char padding0;
> + unsigned char padding1;
> + unsigned char padding2;
> +} __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_SPARE_FOR_BUBBLES \
> - (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> - IPU3_UAPI_AWB_MD_ITEM_SIZE)
> +#define AWB_PUBLIC_NUM_OF_ITEMS_IN_SET 160
> +/* Based on max grid height + Spare for bubbles */
> +#define AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER IPU3_UAPI_AWB_MAX_SETS + \
> + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES)
You're missing parentheses, this won't work as expected. Look at the
usage below. The size is likely computed incorrectly.
Have you tested this ?
> #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
> - (IPU3_UAPI_AWB_MAX_SETS * \
> - (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES))
> + AWB_PUBLIC_NUM_OF_SETS_IN_BUFFER * AWB_PUBLIC_NUM_OF_ITEMS_IN_SET
>
> /**
> * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
> @@ -83,7 +103,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_raw_buffer meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]
> __attribute__((aligned(32)));
> } __packed;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread