| Message ID | 20250224-v4h-iif-v2-5-0305e3c1fe2d@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: renesas: vsp1: Add support for IIF | expand |
Hi Jacopo, Thank you for the patch. On Mon, Feb 24, 2025 at 09:19:45PM +0100, Jacopo Mondi wrote: > With the forthcoming support for VSPX the r/wpf unit will be used > to perform memory access on the behalf of the ISP units. > > Prepare to support reading from external memory images in RAW Bayer > format and ISP configuration parameters by expanding the list > of supported media bus codes. > > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com> > --- > drivers/media/platform/renesas/vsp1/vsp1_rwpf.c | 97 +++++++++++++++++++++++-- > 1 file changed, 92 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c > index 93b0ed5fd0da0c6a182dbbfe1e54eb8cfd66c493..aef7b3d53a2171cda028a272f587641b4a8f85dc 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c > @@ -10,18 +10,102 @@ > #include <media/v4l2-subdev.h> > > #include "vsp1.h" > +#include "vsp1_pipe.h" I don't think this is needed. > #include "vsp1_rwpf.h" > #include "vsp1_video.h" > > #define RWPF_MIN_WIDTH 1 > #define RWPF_MIN_HEIGHT 1 > > +struct vsp1_rwpf_codes { > + const u32 *codes; > + unsigned int num_codes; > +}; > + > static const u32 rwpf_mbus_codes[] = { > MEDIA_BUS_FMT_ARGB8888_1X32, > MEDIA_BUS_FMT_AHSV8888_1X32, > MEDIA_BUS_FMT_AYUV8_1X32, > }; > > +static const struct vsp1_rwpf_codes rwpf_codes = { > + .codes = rwpf_mbus_codes, > + .num_codes = ARRAY_SIZE(rwpf_mbus_codes), > +}; > + > +static const u32 vspx_rpf0_mbus_codes[] = { > + MEDIA_BUS_FMT_SBGGR8_1X8, > + MEDIA_BUS_FMT_SGBRG8_1X8, > + MEDIA_BUS_FMT_SGRBG8_1X8, > + MEDIA_BUS_FMT_SRGGB8_1X8, > + MEDIA_BUS_FMT_SBGGR10_1X10, > + MEDIA_BUS_FMT_SGBRG10_1X10, > + MEDIA_BUS_FMT_SGRBG10_1X10, > + MEDIA_BUS_FMT_SRGGB10_1X10, > + MEDIA_BUS_FMT_SBGGR12_1X12, > + MEDIA_BUS_FMT_SGBRG12_1X12, > + MEDIA_BUS_FMT_SGRBG12_1X12, > + MEDIA_BUS_FMT_SRGGB12_1X12, > + MEDIA_BUS_FMT_SBGGR16_1X16, > + MEDIA_BUS_FMT_SGBRG16_1X16, > + MEDIA_BUS_FMT_SGRBG16_1X16, > + MEDIA_BUS_FMT_SRGGB16_1X16, Same comment as in 1/6, MEDIA_BUS_FMT_Y8_1X8, MEDIA_BUS_FMT_Y10_1X10, MEDIA_BUS_FMT_Y12_1X12, MEDIA_BUS_FMT_Y16_1X16, > + MEDIA_BUS_FMT_METADATA_FIXED > +}; > + > +static const struct vsp1_rwpf_codes vspx_rpf0_codes = { > + .codes = vspx_rpf0_mbus_codes, > + .num_codes = ARRAY_SIZE(vspx_rpf0_mbus_codes), > +}; > + > +static const u32 vspx_rpf1_mbus_codes[] = { > + MEDIA_BUS_FMT_SBGGR8_1X8, > + MEDIA_BUS_FMT_SGBRG8_1X8, > + MEDIA_BUS_FMT_SGRBG8_1X8, > + MEDIA_BUS_FMT_SRGGB8_1X8, > + MEDIA_BUS_FMT_SBGGR10_1X10, > + MEDIA_BUS_FMT_SGBRG10_1X10, > + MEDIA_BUS_FMT_SGRBG10_1X10, > + MEDIA_BUS_FMT_SRGGB10_1X10, > + MEDIA_BUS_FMT_SBGGR12_1X12, > + MEDIA_BUS_FMT_SGBRG12_1X12, > + MEDIA_BUS_FMT_SGRBG12_1X12, > + MEDIA_BUS_FMT_SRGGB12_1X12, > + MEDIA_BUS_FMT_SBGGR16_1X16, > + MEDIA_BUS_FMT_SGBRG16_1X16, > + MEDIA_BUS_FMT_SGRBG16_1X16, > + MEDIA_BUS_FMT_SRGGB16_1X16, Here too. > +}; > + > +static const struct vsp1_rwpf_codes vspx_rpf1_codes = { > + .codes = vspx_rpf1_mbus_codes, > + .num_codes = ARRAY_SIZE(vspx_rpf1_mbus_codes), > +}; > + > +static const struct vsp1_rwpf_codes *vsp1_rwpf_codes(struct v4l2_subdev *sd) > +{ > + struct vsp1_rwpf *rwpf = to_rwpf(sd); > + struct vsp1_entity *ent = &rwpf->entity; > + > + /* Only VSPX supports reading Bayer formats. */ > + if (!vsp1_feature(ent->vsp1, VSP1_HAS_IIF)) > + return &rwpf_codes; > + > + if (ent->type == VSP1_ENTITY_RPF) { > + switch (ent->index) { > + case 0: > + /* VSPX RPF0 supports ISP config data too. */ > + return &vspx_rpf0_codes; > + case 1: > + return &vspx_rpf1_codes; > + default: This should never happen. See below for a proposal on how to handle it. > + return &rwpf_codes; > + } > + } > + > + return &rwpf_codes; You could lower indentation with if (ent->type != VSP1_ENTITY_RPF) return &rwpf_codes; /* Only VSPX supports reading Bayer formats. */ if (!vsp1_feature(ent->vsp1, VSP1_HAS_IIF)) return &rwpf_codes; switch (ent->index) { case 0: /* VSPX RPF0 supports ISP config data too. */ return &vspx_rpf0_codes; case 1: return &vspx_rpf1_codes; default: return &rwpf_codes; } > +} Would it make sense to call this function at init time, and store the const struct vsp1_rwpf_codes pointer in the vsp1_rwpf structure ? You could rename vsp1_rwpf_init_ctrls() to vsp1_rwpf_init() (or add a vsp1_rwpf_init() function that calls vsp1_rwpf_init_ctrls()) and handle the format initialization there. If the RPF index is > 1, vsp1_rwpf_init() should return an error (callers should be fixed to handle the error, that's a patch that can be applied on its own already). > + > /* ----------------------------------------------------------------------------- > * V4L2 Subdevice Operations > */ > @@ -30,10 +114,12 @@ static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev, > struct v4l2_subdev_state *sd_state, > struct v4l2_subdev_mbus_code_enum *code) > { > - if (code->index >= ARRAY_SIZE(rwpf_mbus_codes)) > + const struct vsp1_rwpf_codes *codes = vsp1_rwpf_codes(subdev); > + > + if (code->index >= codes->num_codes) > return -EINVAL; > > - code->code = rwpf_mbus_codes[code->index]; > + code->code = codes->codes[code->index]; > > return 0; > } > @@ -54,6 +140,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, > struct v4l2_subdev_state *sd_state, > struct v4l2_subdev_format *fmt) > { > + const struct vsp1_rwpf_codes *codes = vsp1_rwpf_codes(subdev); > struct vsp1_rwpf *rwpf = to_rwpf(subdev); > struct v4l2_subdev_state *state; > struct v4l2_mbus_framefmt *format; > @@ -69,11 +156,11 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, > } > > /* Default to YUV if the requested format is not supported. */ > - for (i = 0; i < ARRAY_SIZE(rwpf_mbus_codes); ++i) { > - if (fmt->format.code == rwpf_mbus_codes[i]) > + for (i = 0; i < codes->num_codes; ++i) { > + if (fmt->format.code == codes->codes[i]) > break; > } > - if (i == ARRAY_SIZE(rwpf_mbus_codes)) > + if (i == codes->num_codes) > fmt->format.code = MEDIA_BUS_FMT_AYUV8_1X32; This isn't a valid format for the VSP-X. I would pick codes->codes[0] as a default. > > format = v4l2_subdev_state_get_format(state, fmt->pad); >
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c index 93b0ed5fd0da0c6a182dbbfe1e54eb8cfd66c493..aef7b3d53a2171cda028a272f587641b4a8f85dc 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c @@ -10,18 +10,102 @@ #include <media/v4l2-subdev.h> #include "vsp1.h" +#include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_video.h" #define RWPF_MIN_WIDTH 1 #define RWPF_MIN_HEIGHT 1 +struct vsp1_rwpf_codes { + const u32 *codes; + unsigned int num_codes; +}; + static const u32 rwpf_mbus_codes[] = { MEDIA_BUS_FMT_ARGB8888_1X32, MEDIA_BUS_FMT_AHSV8888_1X32, MEDIA_BUS_FMT_AYUV8_1X32, }; +static const struct vsp1_rwpf_codes rwpf_codes = { + .codes = rwpf_mbus_codes, + .num_codes = ARRAY_SIZE(rwpf_mbus_codes), +}; + +static const u32 vspx_rpf0_mbus_codes[] = { + MEDIA_BUS_FMT_SBGGR8_1X8, + MEDIA_BUS_FMT_SGBRG8_1X8, + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SRGGB8_1X8, + MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10, + MEDIA_BUS_FMT_SBGGR12_1X12, + MEDIA_BUS_FMT_SGBRG12_1X12, + MEDIA_BUS_FMT_SGRBG12_1X12, + MEDIA_BUS_FMT_SRGGB12_1X12, + MEDIA_BUS_FMT_SBGGR16_1X16, + MEDIA_BUS_FMT_SGBRG16_1X16, + MEDIA_BUS_FMT_SGRBG16_1X16, + MEDIA_BUS_FMT_SRGGB16_1X16, + MEDIA_BUS_FMT_METADATA_FIXED +}; + +static const struct vsp1_rwpf_codes vspx_rpf0_codes = { + .codes = vspx_rpf0_mbus_codes, + .num_codes = ARRAY_SIZE(vspx_rpf0_mbus_codes), +}; + +static const u32 vspx_rpf1_mbus_codes[] = { + MEDIA_BUS_FMT_SBGGR8_1X8, + MEDIA_BUS_FMT_SGBRG8_1X8, + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SRGGB8_1X8, + MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10, + MEDIA_BUS_FMT_SBGGR12_1X12, + MEDIA_BUS_FMT_SGBRG12_1X12, + MEDIA_BUS_FMT_SGRBG12_1X12, + MEDIA_BUS_FMT_SRGGB12_1X12, + MEDIA_BUS_FMT_SBGGR16_1X16, + MEDIA_BUS_FMT_SGBRG16_1X16, + MEDIA_BUS_FMT_SGRBG16_1X16, + MEDIA_BUS_FMT_SRGGB16_1X16, +}; + +static const struct vsp1_rwpf_codes vspx_rpf1_codes = { + .codes = vspx_rpf1_mbus_codes, + .num_codes = ARRAY_SIZE(vspx_rpf1_mbus_codes), +}; + +static const struct vsp1_rwpf_codes *vsp1_rwpf_codes(struct v4l2_subdev *sd) +{ + struct vsp1_rwpf *rwpf = to_rwpf(sd); + struct vsp1_entity *ent = &rwpf->entity; + + /* Only VSPX supports reading Bayer formats. */ + if (!vsp1_feature(ent->vsp1, VSP1_HAS_IIF)) + return &rwpf_codes; + + if (ent->type == VSP1_ENTITY_RPF) { + switch (ent->index) { + case 0: + /* VSPX RPF0 supports ISP config data too. */ + return &vspx_rpf0_codes; + case 1: + return &vspx_rpf1_codes; + default: + return &rwpf_codes; + } + } + + return &rwpf_codes; +} + /* ----------------------------------------------------------------------------- * V4L2 Subdevice Operations */ @@ -30,10 +114,12 @@ static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_mbus_code_enum *code) { - if (code->index >= ARRAY_SIZE(rwpf_mbus_codes)) + const struct vsp1_rwpf_codes *codes = vsp1_rwpf_codes(subdev); + + if (code->index >= codes->num_codes) return -EINVAL; - code->code = rwpf_mbus_codes[code->index]; + code->code = codes->codes[code->index]; return 0; } @@ -54,6 +140,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_format *fmt) { + const struct vsp1_rwpf_codes *codes = vsp1_rwpf_codes(subdev); struct vsp1_rwpf *rwpf = to_rwpf(subdev); struct v4l2_subdev_state *state; struct v4l2_mbus_framefmt *format; @@ -69,11 +156,11 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev, } /* Default to YUV if the requested format is not supported. */ - for (i = 0; i < ARRAY_SIZE(rwpf_mbus_codes); ++i) { - if (fmt->format.code == rwpf_mbus_codes[i]) + for (i = 0; i < codes->num_codes; ++i) { + if (fmt->format.code == codes->codes[i]) break; } - if (i == ARRAY_SIZE(rwpf_mbus_codes)) + if (i == codes->num_codes) fmt->format.code = MEDIA_BUS_FMT_AYUV8_1X32; format = v4l2_subdev_state_get_format(state, fmt->pad);
With the forthcoming support for VSPX the r/wpf unit will be used to perform memory access on the behalf of the ISP units. Prepare to support reading from external memory images in RAW Bayer format and ISP configuration parameters by expanding the list of supported media bus codes. Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com> --- drivers/media/platform/renesas/vsp1/vsp1_rwpf.c | 97 +++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 5 deletions(-)