* [PATCH v2] media: staging: ipu3-imgu: add the AWB memory layout
@ 2021-10-05 20:20 Jean-Michel Hautbois
2021-10-05 20:53 ` Laurent Pinchart
0 siblings, 1 reply; 4+ messages in thread
From: Jean-Michel Hautbois @ 2021-10-05 20:20 UTC (permalink / raw)
To: linux-media
Cc: sakari.ailus, bingbu.cao, laurent.pinchart, tfiga, tian.shu.qiu,
Jean-Michel Hautbois
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>
---
.../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
+ * 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;
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] media: staging: ipu3-imgu: add the AWB memory layout
2021-10-05 20:20 [PATCH v2] media: staging: ipu3-imgu: add the AWB memory layout Jean-Michel Hautbois
@ 2021-10-05 20:53 ` Laurent Pinchart
2021-10-11 3:30 ` Cao, Bingbu
0 siblings, 1 reply; 4+ messages in thread
From: Laurent Pinchart @ 2021-10-05 20:53 UTC (permalink / raw)
To: Jean-Michel Hautbois
Cc: linux-media, sakari.ailus, bingbu.cao, tfiga, tian.shu.qiu
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@ideasonboard.com>
> ---
> .../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@ideasonboard.com>
I would still like a review from someone knowledgeable with the hardware
and firmware, as we're partly guessing here.
> + * 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] media: staging: ipu3-imgu: add the AWB memory layout
2021-10-05 20:53 ` Laurent Pinchart
@ 2021-10-11 3:30 ` Cao, Bingbu
2021-10-14 3:42 ` Tomasz Figa
0 siblings, 1 reply; 4+ messages in thread
From: Cao, Bingbu @ 2021-10-11 3:30 UTC (permalink / raw)
To: Laurent Pinchart, Jean-Michel Hautbois
Cc: linux-media, sakari.ailus, tfiga, Qiu, Tian Shu
All,
________________________
BRs,
Bingbu Cao
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Wednesday, October 6, 2021 4:54 AM
> To: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Cc: linux-media@vger.kernel.org; sakari.ailus@linux.intel.com; Cao,
> Bingbu <bingbu.cao@intel.com>; tfiga@google.com; Qiu, Tian Shu
> <tian.shu.qiu@intel.com>
> 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@ideasonboard.com>
> > ---
> > .../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@ideasonboard.com>
>
> 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@intel.com>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] media: staging: ipu3-imgu: add the AWB memory layout
2021-10-11 3:30 ` Cao, Bingbu
@ 2021-10-14 3:42 ` Tomasz Figa
0 siblings, 0 replies; 4+ messages in thread
From: Tomasz Figa @ 2021-10-14 3:42 UTC (permalink / raw)
To: Cao, Bingbu
Cc: Laurent Pinchart, Jean-Michel Hautbois, linux-media,
sakari.ailus, Qiu, Tian Shu
On Mon, Oct 11, 2021 at 12:30 PM Cao, Bingbu <bingbu.cao@intel.com> wrote:
>
> All,
>
> ________________________
> BRs,
> Bingbu Cao
>
> > -----Original Message-----
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: Wednesday, October 6, 2021 4:54 AM
> > To: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > Cc: linux-media@vger.kernel.org; sakari.ailus@linux.intel.com; Cao,
> > Bingbu <bingbu.cao@intel.com>; tfiga@google.com; Qiu, Tian Shu
> > <tian.shu.qiu@intel.com>
> > 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@ideasonboard.com>
> > > ---
> > > .../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@ideasonboard.com>
> >
> > 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@intel.com>
>
> 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?
>
No, looks fine to me as well. Thanks!
(We'll have to update our userspace when we pick this, but it should
be trivial.)
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Best regards,
Tomasz
>
> >
> > > + * 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-14 3:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 20:20 [PATCH v2] media: staging: ipu3-imgu: add the AWB memory layout Jean-Michel Hautbois
2021-10-05 20:53 ` Laurent Pinchart
2021-10-11 3:30 ` Cao, Bingbu
2021-10-14 3:42 ` Tomasz Figa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.