Hi Prabhakar, Thank you for the patch. On Tue, Sep 10, 2024 at 06:53:56PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Refactor the ICnDMR register configuration in > `rzg2l_cru_initialize_image_conv()` by adding a new member `icndmr` in the > `rzg2l_cru_ip_format` structure. This change introduces a new function > `rzg2l_cru_ip_pix_fmt_to_icndmr()` to map the pixel format to its > corresponding ICnDMR value. Skip this new function, use the function thar returns a rzg2l_cru_ip_format pointer, and access the icndmr field from there. rzg2l_cru_initialize_image_conv() already gets the format info pointer, so the code will be simpler and more efficient. > > Suggested-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > --- > drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h | 5 +++++ > drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 12 ++++++++++++ > .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 10 ++++------ > 3 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > index 24097df14881..3da9e8e7025a 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > @@ -31,6 +31,8 @@ > #define RZG2L_CRU_MIN_INPUT_HEIGHT 240 > #define RZG2L_CRU_MAX_INPUT_HEIGHT 4095 > > +#define ICnDMR_YCMODE_UYVY (1 << 4) Not a candidate for this patch, but I would recommend moving all the register definitions to this file, or to a rzg2l-cru-regs.h file. > + > enum rzg2l_csi2_pads { > RZG2L_CRU_IP_SINK = 0, > RZG2L_CRU_IP_SOURCE, > @@ -68,12 +70,14 @@ struct rzg2l_cru_ip { > * @format: 4CC format identifier (V4L2_PIX_FMT_*) > * @datatype: MIPI CSI2 data type > * @bpp: bytes per pixel > + * @icndmr: ICnDMR register value > */ > struct rzg2l_cru_ip_format { > u32 code; > u32 format; > u32 datatype; > u8 bpp; > + u32 icndmr; > }; > > /** > @@ -165,5 +169,6 @@ struct v4l2_mbus_framefmt *rzg2l_cru_ip_get_src_fmt(struct rzg2l_cru_dev *cru); > const struct rzg2l_cru_ip_format *rzg2l_cru_ip_code_to_fmt(unsigned int code); > u8 rzg2l_cru_ip_pix_fmt_to_bpp(u32 format); > int rzg2l_cru_ip_index_to_pix_fmt(u32 index); > +int rzg2l_cru_ip_pix_fmt_to_icndmr(u32 format); > > #endif > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > index c7bc82bf3f14..9b0563198b50 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > @@ -17,6 +17,7 @@ static const struct rzg2l_cru_ip_format rzg2l_cru_ip_formats[] = { > .format = V4L2_PIX_FMT_UYVY, > .datatype = MIPI_CSI2_DT_YUV422_8B, > .bpp = 2, > + .icndmr = ICnDMR_YCMODE_UYVY, > }, > }; > > @@ -50,6 +51,17 @@ int rzg2l_cru_ip_index_to_pix_fmt(u32 index) > return rzg2l_cru_ip_formats[index].format; > } > > +int rzg2l_cru_ip_pix_fmt_to_icndmr(u32 format) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(rzg2l_cru_ip_formats); i++) > + if (rzg2l_cru_ip_formats[i].format == format) > + return rzg2l_cru_ip_formats[i].icndmr; > + > + return -EINVAL; > +} > + > struct v4l2_mbus_framefmt *rzg2l_cru_ip_get_src_fmt(struct rzg2l_cru_dev *cru) > { > struct v4l2_subdev_state *state; > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > index c32608c557a3..1f25dcff2515 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > @@ -88,7 +88,6 @@ > > /* CRU Data Output Mode Register */ > #define ICnDMR 0x26c > -#define ICnDMR_YCMODE_UYVY (1 << 4) > > #define RZG2L_TIMEOUT_MS 100 > #define RZG2L_RETRIES 10 > @@ -316,6 +315,7 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > const struct v4l2_format_info *src_finfo, *dst_finfo; > const struct rzg2l_cru_ip_format *cru_ip_fmt; > u32 icndmr; > + int ret; > > cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code); > if (!cru_ip_fmt) > @@ -327,15 +327,13 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > dst_finfo = v4l2_format_info(cru->format.pixelformat); > > /* Output format */ > - switch (cru->format.pixelformat) { > - case V4L2_PIX_FMT_UYVY: > - icndmr = ICnDMR_YCMODE_UYVY; > - break; > - default: > + ret = rzg2l_cru_ip_pix_fmt_to_icndmr(cru->format.pixelformat); > + if (ret < 0) { > dev_err(cru->dev, "Invalid pixelformat (0x%x)\n", > cru->format.pixelformat); > return -EINVAL; > } > + icndmr = ret; > > /* If input and output use same colorspace, do bypass mode */ > if (v4l2_is_format_yuv(src_finfo) && v4l2_is_format_yuv(dst_finfo)) -- Regards, Laurent Pinchart