Hi Laurent,
Thank you for the review.
On Thu, Oct 3, 2024 at 3:25 PM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> Just one minor comment below.
>
Missed Reviewed-by?
> On Tue, Oct 01, 2024 at 03:09:12PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Refactor the handling of supported formats in the RZ/G2L CRU driver by
> > moving the `rzg2l_cru_ip_format` struct to the common header to allow
> > reuse across multiple files and adding pixelformat and bpp members to it.
> > This change centralizes format handling, making it easier to manage and
> > extend.
> >
> > - Moved the `rzg2l_cru_ip_format` struct to `rzg2l-cru.h` for better
> > accessibility.
> > - Added format, datatype and bpp members to `rzg2l_cru_ip_format` struct
> > - Dropped rzg2l_cru_formats
> > - Introduced helper functions `rzg2l_cru_ip_code_to_fmt()`,
> > `rzg2l_cru_ip_format_to_fmt()`, and
> > `rzg2l_cru_ip_index_to_fmt()` to streamline format lookups.
> > - Refactored the `rzg2l_cru_csi2_setup` and format alignment functions
> > to utilize the new helpers.
> >
> > Suggested-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > v2->v3
> > - Updated subject line and commit message
> > - Implemented rzg2l_cru_ip_format_to_fmt() and rzg2l_cru_ip_index_to_fmt()
> > - Dropped checking fmt in rzg2l_cru_initialize_image_conv()
> >
> > v1->v2
> > - New patch
> > ---
> > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 20 +++++-
> > .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 35 ++++++++--
> > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 67 ++++++-------------
> > 3 files changed, 68 insertions(+), 54 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..39296a59b3da 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > @@ -62,6 +62,20 @@ struct rzg2l_cru_ip {
> > struct v4l2_subdev *remote;
> > };
> >
> > +/**
> > + * struct rzg2l_cru_ip_format - CRU IP format
> > + * @code: Media bus code
> > + * @format: 4CC format identifier (V4L2_PIX_FMT_*)
> > + * @datatype: MIPI CSI2 data type
> > + * @bpp: bytes per pixel
> > + */
> > +struct rzg2l_cru_ip_format {
> > + u32 code;
> > + u32 format;
> > + u32 datatype;
> > + u8 bpp;
> > +};
> > +
> > /**
> > * struct rzg2l_cru_dev - Renesas CRU device structure
> > * @dev: (OF) device
> > @@ -144,10 +158,12 @@ int rzg2l_cru_video_register(struct rzg2l_cru_dev *cru);
> > void rzg2l_cru_video_unregister(struct rzg2l_cru_dev *cru);
> > irqreturn_t rzg2l_cru_irq(int irq, void *data);
> >
> > -const struct v4l2_format_info *rzg2l_cru_format_from_pixel(u32 format);
> > -
> > 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);
> > +const struct rzg2l_cru_ip_format *rzg2l_cru_ip_format_to_fmt(u32 format);
> > +const struct rzg2l_cru_ip_format *rzg2l_cru_ip_index_to_fmt(u32 index);
> > +
> > #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..12aac9d6cb4b 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
> > @@ -6,17 +6,21 @@
> > */
> >
> > #include <linux/delay.h>
> > -#include "rzg2l-cru.h"
> >
> > -struct rzg2l_cru_ip_format {
> > - u32 code;
> > -};
> > +#include <media/mipi-csi2.h>
> > +
> > +#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,
> > + .format = V4L2_PIX_FMT_UYVY,
> > + .datatype = MIPI_CSI2_DT_YUV422_8B,
> > + .bpp = 2,
> > + },
> > };
> >
> > -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;
> >
> > @@ -27,6 +31,25 @@ static const struct rzg2l_cru_ip_format *rzg2l_cru_ip_code_to_fmt(unsigned int c
> > return NULL;
> > }
> >
> > +const struct rzg2l_cru_ip_format *rzg2l_cru_ip_format_to_fmt(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];
>
> 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];
> }
>
> Sakari can probably handle this when applying the series to his tree.
>
As there will be v4 anyway for fixing the link_validate(), I'll
address it for v4.
Cheers,
Prabhakar
