Re: [PATCH] media: rzg2l-cru: Switch to v4l2_subdev_get_frame_desc()

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Prabhakar

On Mon, Mar 30, 2026 at 04:06:39PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Replace direct v4l2_subdev_call() invocations of the get_frame_desc pad
> operation with the new v4l2_subdev_get_frame_desc() helper in both
> rzg2l-csi2 and rzg2l-video.
>
> Drop the -ENOIOCTLCMD handling and frame descriptor type validation as
> these are redundant after switching to v4l2_subdev_get_frame_desc().
> Set fd.type to V4L2_MBUS_FRAME_DESC_TYPE_CSI2 before calling the helper
> as required by its API contract.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> Note, this patch depends on the patch [0] posted by Sakari which adds
> the v4l2_subdev_get_frame_desc() helper.
>
> [0] https://lore.kernel.org/all/20260329195625.2840728-1-sakari.ailus@linux.intel.com/
> ---
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c    |  5 +++--
>  .../platform/renesas/rzg2l-cru/rzg2l-video.c   | 18 +++++++-----------
>  2 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> index 6dc4b53607b4..1fdd423f6e6c 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c

There is one thing I'm missing here.

The media topology I see on V2H is:

"CRU output" -> "cru-ip" -> "cru" -> sensor

With the 'output' being the video device.

The handling of the get_frame_desc call chain is implemented in the
video device rzg2l-video.c and in the "cru" subdevice (rzg2l-csi2.c)
while the "ip" subdev doesn't implement get_frame_desc at all

static const struct v4l2_subdev_pad_ops rzg2l_cru_ip_pad_ops = {
	.enum_mbus_code = rzg2l_cru_ip_enum_mbus_code,
	.enum_frame_size = rzg2l_cru_ip_enum_frame_size,
	.get_fmt = v4l2_subdev_get_fmt,
	.set_fmt = rzg2l_cru_ip_set_format,
};

Have I missed how the call chain is propagated from the video device
to the sensor through the 'ip' block maybe ?

> @@ -737,8 +737,9 @@ static int rzg2l_csi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  			csi2->remote_source->name, remote_pad);
>  		return PTR_ERR(remote_pad);
>  	}
> -	return v4l2_subdev_call(csi2->remote_source, pad, get_frame_desc,
> -				remote_pad->index, fd);
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	return v4l2_subdev_get_frame_desc(csi2->remote_source,
> +					  remote_pad->index, fd);
>  }
>
>  static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 162e2ace6931..e701a591ce8a 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -406,24 +406,20 @@ void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru)
>
>  static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)

>  {
> -	struct v4l2_mbus_frame_desc fd = { };
> +	struct v4l2_mbus_frame_desc fd = {
> +		.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2,
> +	};
>  	struct media_pad *remote_pad;
>  	int ret;
>
>  	remote_pad = media_pad_remote_pad_unique(&cru->ip.pads[RZG2L_CRU_IP_SINK]);
> -	ret = v4l2_subdev_call(cru->ip.remote, pad, get_frame_desc, remote_pad->index, &fd);
> -	if (ret < 0 && ret != -ENOIOCTLCMD) {
> +
> +	ret = v4l2_subdev_get_frame_desc(cru->ip.remote,
> +					 remote_pad->index, &fd);
> +	if (ret < 0) {
>  		dev_err(cru->dev, "get_frame_desc failed on IP remote subdev\n");
>  		return ret;
>  	}
> -	/* If remote subdev does not implement .get_frame_desc default to VC0. */
> -	if (ret == -ENOIOCTLCMD)
> -		return 0;
> -
> -	if (fd.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> -		dev_err(cru->dev, "get_frame_desc returned invalid bus type %d\n", fd.type);
> -		return -EINVAL;
> -	}
>
>  	if (!fd.num_entries) {
>  		dev_err(cru->dev, "get_frame_desc returned zero entries\n");
> --
> 2.53.0
>
>