Re: [PATCH v2 12/14] media: uapi: Add CAMSS ISP configuration definition

[Laurent Pinchart]

On Mon, Apr 27, 2026 at 10:08:59PM +0200, Loic Poulain wrote:
> On Mon, Apr 27, 2026 at 2:56 PM Konrad Dybcio wrote:
> > On 4/27/26 2:43 PM, Loic Poulain wrote:
> > > Add the uapi header camss-config.h defining the ISP parameter
> > > structures used by the CAMSS Offline Processing Engine (OPE) driver.
> > > This includes structures for white balance, chroma enhancement and
> > > color correction configuration.
> > >
> > > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> > > ---
> >
> > [...]
> >
> >
> > > +/**
> > > + * struct camss_params_wb_gain - White Balance gains
> > > + *
> > > + * @header:   generic block header; @header.type = CAMSS_PARAMS_WB_GAIN
> > > + * @g_gain:   green channel gain (15uQ10)
> > > + * @b_gain:   blue channel gain (15uQ10)
> > > + * @r_gain:   red channel gain (15uQ10)
> > > + */
> > > +struct camss_params_wb_gain {
> > > +     struct v4l2_isp_params_block_header header;
> > > +     __u16 g_gain;
> > > +     __u16 b_gain;
> > > +     __u16 r_gain;
> > > +     __u16 _pad;
> > > +} __attribute__((aligned(8)));
> >
> > Should this be __le for all of the related types?
> 
> At the moment, this is purely a UAPI, the values are not dumped
> directly to hardware as-is. Instead, each field is translated into one
> or more register writes, with the appropriate math, masking and
> shifting applied. Adding explicit endianness in the definition would
> therefore require special handling on both user and kernel side
> (to_le16, from_le16).
> 
> On the other side, there are scenarios, such as platforms that rely on
> ICP (firmware-driven processing), where we may want to forward these
> structures directly within an HFI packet to the ICP MCU. In that
> context, explicitly defining the endianness could make some sense...

Would those be different structures, or do you envision that someone
could develop an ICP firmware that understands these structures ?

-- 
Regards,

Laurent Pinchart