| Message ID | 20241011173052.1088341-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| Headers | show |
| Series | media: platform: rzg2l-cru: CSI-2 and CRU enhancements | expand |
Hi Prabhakar, Thank you for the patch. On Fri, Oct 11, 2024 at 06:30:41PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Move the `rzg2l_cru_ip_format` struct to `rzg2l-cru.h` for better > accessibility and add a `datatype` member to it, allowing the > configuration of the ICnMC register based on the MIPI CSI2 data type. > > Also, move the `rzg2l_cru_ip_code_to_fmt()` function to `rzg2l-cru.h` > to streamline format lookup and make it more accessible across the > driver. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > .../media/platform/renesas/rzg2l-cru/rzg2l-cru.h | 12 ++++++++++++ > drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c | 13 +++++++------ > .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 13 +++++++------ > 3 files changed, 26 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > index 4fe24bdde5b2..9b1ba015df3b 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h > @@ -62,6 +62,16 @@ struct rzg2l_cru_ip { > struct v4l2_subdev *remote; > }; > > +/** > + * struct rzg2l_cru_ip_format - CRU IP format > + * @code: Media bus code > + * @datatype: MIPI CSI2 data type > + */ > +struct rzg2l_cru_ip_format { > + u32 code; > + u32 datatype; > +}; > + > /** > * struct rzg2l_cru_dev - Renesas CRU device structure > * @dev: (OF) device > @@ -150,4 +160,6 @@ int rzg2l_cru_ip_subdev_register(struct rzg2l_cru_dev *cru); > void rzg2l_cru_ip_subdev_unregister(struct rzg2l_cru_dev *cru); > 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); > + > #endif > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > index 7b006a0bfaae..8f9683bf31fa 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c > @@ -6,17 +6,18 @@ > */ > > #include <linux/delay.h> > -#include "rzg2l-cru.h" > +#include <media/mipi-csi2.h> > > -struct rzg2l_cru_ip_format { > - u32 code; > -}; > +#include "rzg2l-cru.h" > > static const struct rzg2l_cru_ip_format rzg2l_cru_ip_formats[] = { > - { .code = MEDIA_BUS_FMT_UYVY8_1X16, }, > + { > + .code = MEDIA_BUS_FMT_UYVY8_1X16, > + .datatype = MIPI_CSI2_DT_YUV422_8B, > + }, > }; > > -static const struct rzg2l_cru_ip_format *rzg2l_cru_ip_code_to_fmt(unsigned int code) > +const struct rzg2l_cru_ip_format *rzg2l_cru_ip_code_to_fmt(unsigned int code) > { > unsigned int i; > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > index 401ef7be58ec..37fea2bed00f 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > @@ -301,18 +301,17 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru) > } > > static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, bool *input_is_yuv, > - struct v4l2_mbus_framefmt *ip_sd_fmt, u8 csi_vc) > + const struct rzg2l_cru_ip_format *ip_fmt, > + u8 csi_vc) > { > - u32 icnmc; > + u32 icnmc = ICnMC_INF(ip_fmt->datatype); > > - switch (ip_sd_fmt->code) { > + switch (ip_fmt->code) { > case MEDIA_BUS_FMT_UYVY8_1X16: > - icnmc = ICnMC_INF(MIPI_CSI2_DT_YUV422_8B); > *input_is_yuv = true; > break; > default: > *input_is_yuv = false; > - icnmc = ICnMC_INF(MIPI_CSI2_DT_USER_DEFINED(0)); > break; > } > > @@ -328,11 +327,13 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > struct v4l2_mbus_framefmt *ip_sd_fmt, > u8 csi_vc) > { > + const struct rzg2l_cru_ip_format *cru_ip_fmt; > bool output_is_yuv = false; > bool input_is_yuv = false; > u32 icndmr; > > - rzg2l_cru_csi2_setup(cru, &input_is_yuv, ip_sd_fmt, csi_vc); > + cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code); > + rzg2l_cru_csi2_setup(cru, &input_is_yuv, cru_ip_fmt, csi_vc); > > /* Output format */ > switch (cru->format.pixelformat) { >
Hi Laurent, Thank you for the review. On Tue, Oct 15, 2024 at 11:23 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Prabhakar, > > Thank you for the patch. > > On Fri, Oct 11, 2024 at 06:30:43PM +0100, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Refactor the handling of supported formats in the RZ/G2L CRU driver by > > moving the `rzg2l_cru_ip_format` struct to the common header for reuse > > across multiple files and adding `pixelformat` and `bpp` members to it. > > The structure is already in a common header. You can write > > Refactor the handling of supported formats in the RZ/G2L CRU driver by > adding `pixelformat` and `bpp` members to the `rzg2l_cru_ip_format` > structure. > I will update the commit message as above. Cheers, Prabhakar
Hi Laurent, Thank you for the review. On Tue, Oct 15, 2024 at 11:33 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Prabhakar, > > Thank you for the patch. > > On Fri, Oct 11, 2024 at 06:30:45PM +0100, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Make use of v4l2_format_info() helpers to determine the input and > > output formats. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 22 ++++++------------- > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > index 8932fab7c656..0cc69a7440bf 100644 > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > @@ -300,21 +300,12 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru) > > rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr); > > } > > > > -static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, bool *input_is_yuv, > > +static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, > > const struct rzg2l_cru_ip_format *ip_fmt, > > u8 csi_vc) > > { > > u32 icnmc = ICnMC_INF(ip_fmt->datatype); > > > > - switch (ip_fmt->code) { > > - case MEDIA_BUS_FMT_UYVY8_1X16: > > - *input_is_yuv = true; > > - break; > > - default: > > - *input_is_yuv = false; > > - break; > > - } > > - > > icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK); > > > > /* Set virtual channel CSI2 */ > > @@ -327,19 +318,17 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > > struct v4l2_mbus_framefmt *ip_sd_fmt, > > u8 csi_vc) > > { > > + const struct v4l2_format_info *src_finfo, *dst_finfo; > > const struct rzg2l_cru_ip_format *cru_ip_fmt; > > - bool output_is_yuv = false; > > - bool input_is_yuv = false; > > u32 icndmr; > > > > cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code); > > - rzg2l_cru_csi2_setup(cru, &input_is_yuv, cru_ip_fmt, csi_vc); > > + rzg2l_cru_csi2_setup(cru, cru_ip_fmt, csi_vc); > > > > /* Output format */ > > switch (cru->format.pixelformat) { > > case V4L2_PIX_FMT_UYVY: > > icndmr = ICnDMR_YCMODE_UYVY; > > - output_is_yuv = true; > > break; > > default: > > dev_err(cru->dev, "Invalid pixelformat (0x%x)\n", > > @@ -347,8 +336,11 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > > return -EINVAL; > > } > > > > + src_finfo = v4l2_format_info(cru_ip_fmt->format); > > + dst_finfo = v4l2_format_info(cru->format.pixelformat); > > It would be a bit more efficient to add a yuv boolean field to the > rzg2l_cru_ip_format structure, as you already have looked up cru_ip_fmt > for the IP subdev format. > I will consider this change, when adding support for the RZ/V2H SoC, hope that's OK for you. Cheers, Prabhakar > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > + > > /* If input and output use same colorspace, do bypass mode */ > > - if (output_is_yuv == input_is_yuv) > > + if (v4l2_is_format_yuv(src_finfo) == v4l2_is_format_yuv(dst_finfo)) > > rzg2l_cru_write(cru, ICnMC, > > rzg2l_cru_read(cru, ICnMC) | ICnMC_CSCTHR); > > else > > -- > Regards, > > Laurent Pinchart
On Fri, Oct 18, 2024 at 12:45:36PM +0100, Lad, Prabhakar wrote: > On Tue, Oct 15, 2024 at 11:33 AM Laurent Pinchart wrote: > > On Fri, Oct 11, 2024 at 06:30:45PM +0100, Prabhakar wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Make use of v4l2_format_info() helpers to determine the input and > > > output formats. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 22 ++++++------------- > > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > index 8932fab7c656..0cc69a7440bf 100644 > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > @@ -300,21 +300,12 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru) > > > rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr); > > > } > > > > > > -static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, bool *input_is_yuv, > > > +static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, > > > const struct rzg2l_cru_ip_format *ip_fmt, > > > u8 csi_vc) > > > { > > > u32 icnmc = ICnMC_INF(ip_fmt->datatype); > > > > > > - switch (ip_fmt->code) { > > > - case MEDIA_BUS_FMT_UYVY8_1X16: > > > - *input_is_yuv = true; > > > - break; > > > - default: > > > - *input_is_yuv = false; > > > - break; > > > - } > > > - > > > icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK); > > > > > > /* Set virtual channel CSI2 */ > > > @@ -327,19 +318,17 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > > > struct v4l2_mbus_framefmt *ip_sd_fmt, > > > u8 csi_vc) > > > { > > > + const struct v4l2_format_info *src_finfo, *dst_finfo; > > > const struct rzg2l_cru_ip_format *cru_ip_fmt; > > > - bool output_is_yuv = false; > > > - bool input_is_yuv = false; > > > u32 icndmr; > > > > > > cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code); > > > - rzg2l_cru_csi2_setup(cru, &input_is_yuv, cru_ip_fmt, csi_vc); > > > + rzg2l_cru_csi2_setup(cru, cru_ip_fmt, csi_vc); > > > > > > /* Output format */ > > > switch (cru->format.pixelformat) { > > > case V4L2_PIX_FMT_UYVY: > > > icndmr = ICnDMR_YCMODE_UYVY; > > > - output_is_yuv = true; > > > break; > > > default: > > > dev_err(cru->dev, "Invalid pixelformat (0x%x)\n", > > > @@ -347,8 +336,11 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > > > return -EINVAL; > > > } > > > > > > + src_finfo = v4l2_format_info(cru_ip_fmt->format); > > > + dst_finfo = v4l2_format_info(cru->format.pixelformat); > > > > It would be a bit more efficient to add a yuv boolean field to the > > rzg2l_cru_ip_format structure, as you already have looked up cru_ip_fmt > > for the IP subdev format. > > I will consider this change, when adding support for the RZ/V2H SoC, > hope that's OK for you. It's not a blocker, it can be done on top indeed. If you end up submitting a v6 you could add a patch on top already, but otherwise later is fine too. > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > + > > > /* If input and output use same colorspace, do bypass mode */ > > > - if (output_is_yuv == input_is_yuv) > > > + if (v4l2_is_format_yuv(src_finfo) == v4l2_is_format_yuv(dst_finfo)) > > > rzg2l_cru_write(cru, ICnMC, > > > rzg2l_cru_read(cru, ICnMC) | ICnMC_CSCTHR); > > > else
Hi Laurent, On Fri, Oct 18, 2024 at 1:26 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Fri, Oct 18, 2024 at 12:45:36PM +0100, Lad, Prabhakar wrote: > > On Tue, Oct 15, 2024 at 11:33 AM Laurent Pinchart wrote: > > > On Fri, Oct 11, 2024 at 06:30:45PM +0100, Prabhakar wrote: > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > > > Make use of v4l2_format_info() helpers to determine the input and > > > > output formats. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > --- > > > > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 22 ++++++------------- > > > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > > index 8932fab7c656..0cc69a7440bf 100644 > > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > > > > @@ -300,21 +300,12 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru) > > > > rzg2l_cru_write(cru, AMnAXIATTR, amnaxiattr); > > > > } > > > > > > > > -static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, bool *input_is_yuv, > > > > +static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru, > > > > const struct rzg2l_cru_ip_format *ip_fmt, > > > > u8 csi_vc) > > > > { > > > > u32 icnmc = ICnMC_INF(ip_fmt->datatype); > > > > > > > > - switch (ip_fmt->code) { > > > > - case MEDIA_BUS_FMT_UYVY8_1X16: > > > > - *input_is_yuv = true; > > > > - break; > > > > - default: > > > > - *input_is_yuv = false; > > > > - break; > > > > - } > > > > - > > > > icnmc |= (rzg2l_cru_read(cru, ICnMC) & ~ICnMC_INF_MASK); > > > > > > > > /* Set virtual channel CSI2 */ > > > > @@ -327,19 +318,17 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > > > > struct v4l2_mbus_framefmt *ip_sd_fmt, > > > > u8 csi_vc) > > > > { > > > > + const struct v4l2_format_info *src_finfo, *dst_finfo; > > > > const struct rzg2l_cru_ip_format *cru_ip_fmt; > > > > - bool output_is_yuv = false; > > > > - bool input_is_yuv = false; > > > > u32 icndmr; > > > > > > > > cru_ip_fmt = rzg2l_cru_ip_code_to_fmt(ip_sd_fmt->code); > > > > - rzg2l_cru_csi2_setup(cru, &input_is_yuv, cru_ip_fmt, csi_vc); > > > > + rzg2l_cru_csi2_setup(cru, cru_ip_fmt, csi_vc); > > > > > > > > /* Output format */ > > > > switch (cru->format.pixelformat) { > > > > case V4L2_PIX_FMT_UYVY: > > > > icndmr = ICnDMR_YCMODE_UYVY; > > > > - output_is_yuv = true; > > > > break; > > > > default: > > > > dev_err(cru->dev, "Invalid pixelformat (0x%x)\n", > > > > @@ -347,8 +336,11 @@ static int rzg2l_cru_initialize_image_conv(struct rzg2l_cru_dev *cru, > > > > return -EINVAL; > > > > } > > > > > > > > + src_finfo = v4l2_format_info(cru_ip_fmt->format); > > > > + dst_finfo = v4l2_format_info(cru->format.pixelformat); > > > > > > It would be a bit more efficient to add a yuv boolean field to the > > > rzg2l_cru_ip_format structure, as you already have looked up cru_ip_fmt > > > for the IP subdev format. > > > > I will consider this change, when adding support for the RZ/V2H SoC, > > hope that's OK for you. > > It's not a blocker, it can be done on top indeed. If you end up > submitting a v6 you could add a patch on top already, but otherwise > later is fine too. > I was intending to send a v6 anyway with the updated commit messages, so I will add a new patch on top. Cheers, Prabhakar
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Hi All, This patch series aims to add the below: - Retrieve virtual channel from remote subdev - Support to capture 8bit Bayer formats. v4->v5 - Updated .link_validate function to drop local variable, invalid format check and input/output code fmt check - Split up patch 10/17 from v4 into multiple patches (10/11/12/13/14/15)/22 v3->v4 - Added {} in rzg2l_cru_ip_format_to_fmt() for the for loop (in patch 10/17) - Added checks for formats in .link_validate callback (in patch 13/17) - Got rid of icndmr local variable in rzg2l_cru_initialize_image_conv() (in patch 15/17) - Moved macro ICnDMR_YCMODE_UYVY to rzg2l-cru-regs.h - Included RB tags from Laurent for patches 15 and 17 v2->v3 - Added MUST_CONNECT flag for source pads - Used ARRAY_SIZE() instead of NR_OF_RZG2L_CSI2_PAD - Implemented rzg2l_cru_ip_format_to_fmt() and rzg2l_cru_ip_index_to_fmt() - Dropped checking fmt in rzg2l_cru_initialize_image_conv() - Dropped fse->index checks - Implemented link_validate for video node - Re-used rzg2l_cru_ip_format_to_fmt() to fetch icndmr details - Moved register definitions to separate header file - Updated subject lines and commit messages - Collected RB tag v1->v2 - Fixed retrieving VC from subdev - Fixed review comments pointed by Laurent * Refactored supported CRU formats * Added MUST_CONNECT flag wherever required * Dropped `channel` member from `struct v1: Link: https://lore.kernel.org/all/20240906173947.282402-1-prabhakar.mahadev-lad.rj@bp.renesas.com/ Cheers, Prabhakar Lad Prabhakar (22): media: rzg2l-cru: Use RZG2L_CRU_IP_SINK/SOURCE enum entries media: rzg2l-cru: Mark sink and source pad with MUST_CONNECT flag media: rzg2l-cru: csi2: Mark sink and source pad with MUST_CONNECT flag media: rzg2l-cru: csi2: Use ARRAY_SIZE() in media_entity_pads_init() media: rzg2l-cru: csi2: Implement .get_frame_desc() media: rzg2l-cru: Retrieve virtual channel information media: rzg2l-cru: Remove `channel` member from `struct rzg2l_cru_csi` media: rzg2l-cru: Use MIPI CSI-2 data types for ICnMC_INF definitions media: rzg2l-cru: Remove unused fields from rzg2l_cru_ip_format struct media: rzg2l-cru: Remove unnecessary WARN_ON check in format func media: rzg2l-cru: Simplify configuring input format for image processing media: rzg2l-cru: Inline calculating image size media: rzg2l-cru: Simplify handling of supported formats media: rzg2l-cru: Inline calculating bytesperline media: rzg2l-cru: Make use of v4l2_format_info() helpers media: rzg2l-cru: Use `rzg2l_cru_ip_formats` array in enum_frame_size media: rzg2l-cru: csi2: Remove unused field from rzg2l_csi2_format media: rzg2l-cru: video: Implement .link_validate() callback media: rzg2l-cru: csi2: Use rzg2l_csi2_formats array in enum_frame_size media: rzg2l-cru: Refactor ICnDMR register configuration media: rzg2l-cru: Add support to capture 8bit raw sRGB media: rzg2l-cru: Move register definitions to a separate file .../platform/renesas/rzg2l-cru/rzg2l-core.c | 3 +- .../renesas/rzg2l-cru/rzg2l-cru-regs.h | 80 +++++ .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 26 +- .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 39 ++- .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 80 ++++- .../platform/renesas/rzg2l-cru/rzg2l-video.c | 291 +++++++----------- 6 files changed, 312 insertions(+), 207 deletions(-) create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru-regs.h