Hi Michael, Thank you for the patch. On Thu, Jan 05, 2023 at 02:47:24PM +0100, Michael Tretter wrote: > The driver must configure the data path through the Pixel Pipeline. > > Currently, the driver is using a fixed setup, but once there are > different pipeline configurations, it is helpful to have a dedicated > function for determining the register value for the data path. > > Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- > drivers/media/platform/nxp/imx-pxp.c | 62 +++++++++++++++++++--------- > 1 file changed, 42 insertions(+), 20 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c > index 05abe40558b0..a957fee88829 100644 > --- a/drivers/media/platform/nxp/imx-pxp.c > +++ b/drivers/media/platform/nxp/imx-pxp.c > @@ -726,6 +726,47 @@ static void pxp_setup_csc(struct pxp_ctx *ctx) > } > } > > +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx) > +{ > + u32 ctrl0; > + > + ctrl0 = 0; > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0); > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0); > + > + return ctrl0; > +} > + > +static void pxp_set_data_path(struct pxp_ctx *ctx) > +{ > + struct pxp_dev *dev = ctx->dev; > + u32 ctrl0; > + u32 ctrl1; > + > + ctrl0 = pxp_data_path_ctrl0(ctx); > + > + ctrl1 = 0; > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1); > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1); > + > + writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0); > + writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1); > +} > + > static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb, > struct vb2_v4l2_buffer *out_vb) > { > @@ -912,26 +953,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb, > /* bypass LUT */ > writel(BM_PXP_LUT_CTRL_BYPASS, dev->mmio + HW_PXP_LUT_CTRL); > > - writel(BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1)| > - BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1)| > - BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0)| > - BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0), > - dev->mmio + HW_PXP_DATA_PATH_CTRL0); > - writel(BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1) | > - BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1), > - dev->mmio + HW_PXP_DATA_PATH_CTRL1); > + pxp_set_data_path(ctx); > > writel(0xffff, dev->mmio + HW_PXP_IRQ_MASK); > -- Regards, Laurent Pinchart