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

[Loic Poulain]

On Mon, Apr 27, 2026 at 10:25 PM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> 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 ?

I believe some of these structures could be reused across different
platforms and bus implementations. In particular, a given e.g. wb_gain
parameter struct could potentially be shared between OPE, ICP
(firmware‑based), and the inline engine (IFE), with minimal
driver-side adaptation (limited to shifting and masking) or even zero
adaptation. Bryan has sent another RFC [1] defining parameter
structures and layouts that (as far as I understand) mirror the HFI
data layout, so I plan to sync with him to identify opportunities to
leverage common types or structures.

[1] https://lore.kernel.org/all/20260426000418.1158716-1-bryan.odonoghue@xxxxxxxxxx/