RE: [PATCH v4 07/11] media: imx8-isi: Select MSB-aligned RAW input from pixel link on i.MX95

[G.N. Zhou (OSS)]

Hi Frank,

Thanks for your review.


NXP Confidential
> -----Original Message-----
> From: Frank Li (OSS) <frank.li@xxxxxxxxxxx>
> Sent: Tuesday, September 29, 2026 10:32 PM
> To: G.N. Zhou (OSS) <guoniu.zhou@xxxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>; Frank Li
> <frank.li@xxxxxxx>; Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>; Sascha
> Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; Rob
> Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor
> Dooley <conor+dt@xxxxxxxxxx>; Loic Poulain
> <loic.poulain@xxxxxxxxxxxxxxxx>; linux-media@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 07/11] media: imx8-isi: Select MSB-aligned RAW input
> from pixel link on i.MX95
>
> On Tue, Sep 29, 2026 at 04:52:28PM +0800, Guoniu Zhou wrote:
> > The ISI receives RAW data from the Pixel Link, whose alignment is
> > determined by the SoC integration. Before i.MX93, the Pixel Link
> > provides LSB-aligned RAW data to the ISI. Starting with i.MX93, both
> > LSB and MSB alignment are supported and can be selected via
> > CHNL_CTRL_RAW_IN_MSB_ALIGN, with the default keeping LSB alignment
> for
> > backward compatibility. From i.MX95 onwards, the CSI formatter
> > replaces the gasket and only provides MSB-aligned RAW data.
> >
> > Set CHNL_CTRL_RAW_IN_MSB_ALIGN on i.MX95 so the ISI selects RAW
> > samples from the high bits of the Pixel Link bus.
> >
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@xxxxxxxxxxx>
> > ---
> > Changes in v4:
> > - New patch
> > ---
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c   | 20
> ++++++++++++++++++++
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h |  1 +
> >  2 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
> > index cc52a90c2477..b3f32b9796d5 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
> > @@ -303,6 +303,22 @@ static void
> mxc_isi_channel_set_panic_threshold(struct mxc_isi_pipe *pipe)
> >     mxc_isi_write(pipe, CHNL_OUT_BUF_CTRL, val);  }
> >
> > +/*
> > + * The ISI receives RAW data from the Pixel Link, whose alignment is
> > + * determined by the SoC integration. It originally selected RAW
> > +samples
> > + * from the low bits of the Pixel Link bus. Starting with i.MX93,
> > + * CHNL_CTRL_RAW_IN_MSB_ALIGN was introduced to allow the ISI to
> > +select
> > + * RAW samples from either the high or low bits, as both LSB and MSB
> > + * alignment can be selected in the gasket.
> > + *
> > + * On i.MX95, the CSI formatter provides MSB-aligned RAW data, so the
> > +ISI
> > + * must select RAW samples from the high bits of the Pixel Link bus.
> > + */
> > +static bool isi_raw_in_msb(const struct mxc_isi_plat_data *pdata) {
> > +   return pdata->model == MXC_ISI_IMX95;
>
> Can you add bool in plat_data to avoid check model? In case future other chip
> also use msb and complex check logic here.

I used the model field here to be consistent with the approach suggested by Laurent
during the v2 review.

In v2, I used a Boolean(bool raw_out_lsb) in plat_data to determine whether the ISI RAW
output should be LSB-aligned, and Laurent suggested using the model field instead. Based
on that feedback, I followed the same approach here for the RAW input alignment.

Please see the discussion here for more details:
https://lore.kernel.org/all/20260915000520.GB2804748@xxxxxxxxxxxxxxxxxxxxxxxxxx/

Best Regards
G.N Zhou

>
> Frank
>
> > +}
> > +
> >  static void mxc_isi_channel_set_control(struct mxc_isi_pipe *pipe,
> >                                     enum mxc_isi_input_id input,
> >                                     unsigned int vc,
> > @@ -357,6 +373,10 @@ static void mxc_isi_channel_set_control(struct
> mxc_isi_pipe *pipe,
> >                     val |= CHNL_CTRL_VC_ID_1(vc >> 2);
> >     }
> >
> > +   /* Select MSB-aligned RAW data from the Pixel Link when needed. */
> > +   if (isi_raw_in_msb(pipe->isi->pdata))
> > +           val |= CHNL_CTRL_RAW_IN_MSB_ALIGN;
> > +
> >     mxc_isi_write(pipe, CHNL_CTRL, val);
> >
> >     mutex_unlock(&pipe->lock);
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
> > b/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
> > index 5f66b7c9cd1c..35438abf3172 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
> > @@ -20,6 +20,7 @@
> >  #define CHNL_CTRL_CHAIN_BUF_NO_CHAIN                               0
> >  #define CHNL_CTRL_CHAIN_BUF_2_CHAIN                                1
> >  #define CHNL_CTRL_SW_RST                                   BIT(24)
> > +#define CHNL_CTRL_RAW_IN_MSB_ALIGN
>       BIT(22) /* i.MX{91,93,95} */
> >  /*
> >   * CHNL_CTRL_BLANK_PXL: i.MX8{QM,QXP} only
> >   * CHNL_CTRL_VC_ID_1, CHNL_CTRL_VC_ID_1_MASK: i.MX95 only
> >
> > --
> > 2.34.1
> >
> >