[v3] media: rcar: Streams support

[PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

Posted by Tomi Valkeinen 6 months, 2 weeks ago

Add v4l2_subdev_pad_ops.get_frame_desc() implementation.

We also implement a fallback for the case where the upstream subdevice
does not implement .get_frame_desc. It assumes a single stream with VC =
0 and DT based on the configured stream mbus format.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index 20bd44274bd2..65c7f3040696 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
+					 unsigned int pad,
+					 struct v4l2_mbus_frame_desc *fd)
+{
+	const struct rcar_csi2_format *format;
+	struct v4l2_subdev_state *state;
+	struct v4l2_mbus_framefmt *fmt;
+	int ret = 0;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+
+	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
+	if (!fmt) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	format = rcsi2_code_to_fmt(fmt->code);
+	if (!format) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	fd->num_entries = 1;
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+	fd->entry[0].stream = 0;
+	fd->entry[0].pixelcode = fmt->code;
+	fd->entry[0].bus.csi2.vc = 0;
+	fd->entry[0].bus.csi2.dt = format->datatype;
+
+out:
+	v4l2_subdev_unlock_state(state);
+
+	return ret;
+}
+
+static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				struct v4l2_mbus_frame_desc *fd)
+{
+	struct rcar_csi2 *priv = sd_to_csi2(sd);
+	int ret;
+
+	if (WARN_ON(!priv->info->use_isp))
+		return -ENOTTY;
+
+	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
+		return -EINVAL;
+
+	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
+	if (ret == -ENOIOCTLCMD)
+		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
+	return ret;
+}
+
 static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
 	.enable_streams = rcsi2_enable_streams,
 	.disable_streams = rcsi2_disable_streams,
 
 	.set_fmt = rcsi2_set_pad_format,
 	.get_fmt = v4l2_subdev_get_fmt,
+
+	.get_frame_desc = rcsi2_get_frame_desc,
 };
 
 static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {

-- 
2.43.0

Re: [PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

Posted by Niklas Söderlund 6 months, 1 week ago

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:39 +0300, Tomi Valkeinen wrote:
> Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
> 
> We also implement a fallback for the case where the upstream subdevice
> does not implement .get_frame_desc. It assumes a single stream with VC =
> 0 and DT based on the configured stream mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 20bd44274bd2..65c7f3040696 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
> +					 unsigned int pad,
> +					 struct v4l2_mbus_frame_desc *fd)
> +{
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_subdev_state *state;
> +	struct v4l2_mbus_framefmt *fmt;
> +	int ret = 0;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +	if (!fmt) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	format = rcsi2_code_to_fmt(fmt->code);
> +	if (!format) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	fd->num_entries = 1;
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	fd->entry[0].stream = 0;
> +	fd->entry[0].pixelcode = fmt->code;
> +	fd->entry[0].bus.csi2.vc = 0;
> +	fd->entry[0].bus.csi2.dt = format->datatype;
> +
> +out:
> +	v4l2_subdev_unlock_state(state);
> +
> +	return ret;
> +}
> +
> +static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	int ret;
> +
> +	if (WARN_ON(!priv->info->use_isp))
> +		return -ENOTTY;

This looks odd, why can't we support the frame descriptor and streams on 
Gen3 that do not use the ISP Channel Selector?

> +
> +	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
> +	if (ret == -ENOIOCTLCMD)
> +		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
> +	return ret;
> +}
> +
>  static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>  	.enable_streams = rcsi2_enable_streams,
>  	.disable_streams = rcsi2_disable_streams,
>  
>  	.set_fmt = rcsi2_set_pad_format,
>  	.get_fmt = v4l2_subdev_get_fmt,
> +
> +	.get_frame_desc = rcsi2_get_frame_desc,
>  };
>  
>  static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

Posted by Tomi Valkeinen 9 hours ago

Hi,

On 06/06/2025 15:14, Niklas Söderlund wrote:
> Hi Tomi,
> 
> Thanks for your work.
> 
> On 2025-05-30 16:50:39 +0300, Tomi Valkeinen wrote:
>> Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
>>
>> We also implement a fallback for the case where the upstream subdevice
>> does not implement .get_frame_desc. It assumes a single stream with VC =
>> 0 and DT based on the configured stream mbus format.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>  drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
>> index 20bd44274bd2..65c7f3040696 100644
>> --- a/drivers/media/platform/renesas/rcar-csi2.c
>> +++ b/drivers/media/platform/renesas/rcar-csi2.c
>> @@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>>  	return 0;
>>  }
>>  
>> +static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
>> +					 unsigned int pad,
>> +					 struct v4l2_mbus_frame_desc *fd)
>> +{
>> +	const struct rcar_csi2_format *format;
>> +	struct v4l2_subdev_state *state;
>> +	struct v4l2_mbus_framefmt *fmt;
>> +	int ret = 0;
>> +
>> +	state = v4l2_subdev_lock_and_get_active_state(sd);
>> +
>> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
>> +	if (!fmt) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	format = rcsi2_code_to_fmt(fmt->code);
>> +	if (!format) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	fd->num_entries = 1;
>> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
>> +	fd->entry[0].stream = 0;
>> +	fd->entry[0].pixelcode = fmt->code;
>> +	fd->entry[0].bus.csi2.vc = 0;
>> +	fd->entry[0].bus.csi2.dt = format->datatype;
>> +
>> +out:
>> +	v4l2_subdev_unlock_state(state);
>> +
>> +	return ret;
>> +}
>> +
>> +static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>> +				struct v4l2_mbus_frame_desc *fd)
>> +{
>> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
>> +	int ret;
>> +
>> +	if (WARN_ON(!priv->info->use_isp))
>> +		return -ENOTTY;
> 
> This looks odd, why can't we support the frame descriptor and streams on 
> Gen3 that do not use the ISP Channel Selector?

Yes, this is not right. There was another similar case. I think they are
remnants of a gen4-only series.

 Tomi

> 
>> +
>> +	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
>> +		return -EINVAL;
>> +
>> +	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
>> +	if (ret == -ENOIOCTLCMD)
>> +		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
>> +	return ret;
>> +}
>> +
>>  static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>>  	.enable_streams = rcsi2_enable_streams,
>>  	.disable_streams = rcsi2_disable_streams,
>>  
>>  	.set_fmt = rcsi2_set_pad_format,
>>  	.get_fmt = v4l2_subdev_get_fmt,
>> +
>> +	.get_frame_desc = rcsi2_get_frame_desc,
>>  };
>>  
>>  static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
>>
>> -- 
>> 2.43.0
>>
>

Re: [PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

Posted by Tomi Valkeinen 8 hours ago

Hi,

On 16/12/2025 15:18, Tomi Valkeinen wrote:
> Hi,
> 
> On 06/06/2025 15:14, Niklas Söderlund wrote:
>> Hi Tomi,
>>
>> Thanks for your work.
>>
>> On 2025-05-30 16:50:39 +0300, Tomi Valkeinen wrote:
>>> Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
>>>
>>> We also implement a fallback for the case where the upstream subdevice
>>> does not implement .get_frame_desc. It assumes a single stream with VC =
>>> 0 and DT based on the configured stream mbus format.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> ---
>>>  drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
>>>  1 file changed, 56 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
>>> index 20bd44274bd2..65c7f3040696 100644
>>> --- a/drivers/media/platform/renesas/rcar-csi2.c
>>> +++ b/drivers/media/platform/renesas/rcar-csi2.c
>>> @@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>>>  	return 0;
>>>  }
>>>  
>>> +static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
>>> +					 unsigned int pad,
>>> +					 struct v4l2_mbus_frame_desc *fd)
>>> +{
>>> +	const struct rcar_csi2_format *format;
>>> +	struct v4l2_subdev_state *state;
>>> +	struct v4l2_mbus_framefmt *fmt;
>>> +	int ret = 0;
>>> +
>>> +	state = v4l2_subdev_lock_and_get_active_state(sd);
>>> +
>>> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
>>> +	if (!fmt) {
>>> +		ret = -EINVAL;
>>> +		goto out;
>>> +	}
>>> +
>>> +	format = rcsi2_code_to_fmt(fmt->code);
>>> +	if (!format) {
>>> +		ret = -EINVAL;
>>> +		goto out;
>>> +	}
>>> +
>>> +	fd->num_entries = 1;
>>> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
>>> +	fd->entry[0].stream = 0;
>>> +	fd->entry[0].pixelcode = fmt->code;
>>> +	fd->entry[0].bus.csi2.vc = 0;
>>> +	fd->entry[0].bus.csi2.dt = format->datatype;
>>> +
>>> +out:
>>> +	v4l2_subdev_unlock_state(state);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>>> +				struct v4l2_mbus_frame_desc *fd)
>>> +{
>>> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
>>> +	int ret;
>>> +
>>> +	if (WARN_ON(!priv->info->use_isp))
>>> +		return -ENOTTY;
>>
>> This looks odd, why can't we support the frame descriptor and streams on 
>> Gen3 that do not use the ISP Channel Selector?
> 
> Yes, this is not right. There was another similar case. I think they are
> remnants of a gen4-only series.

Ah, never mind, now I see. On Gen3 the VINs are connected to the CSI-2.
There's no one that would call get_frame_desc on csi2.

Also, we use v4l2_subdev_get_frame_desc_passthrough() which requires a
1-sink-1-source subdevice, i.e. CSI-2 on Gen4, but not on Gen3.

 Tomi

>>
>>> +
>>> +	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
>>> +		return -EINVAL;
>>> +
>>> +	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
>>> +	if (ret == -ENOIOCTLCMD)
>>> +		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
>>> +	return ret;
>>> +}
>>> +
>>>  static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>>>  	.enable_streams = rcsi2_enable_streams,
>>>  	.disable_streams = rcsi2_disable_streams,
>>>  
>>>  	.set_fmt = rcsi2_set_pad_format,
>>>  	.get_fmt = v4l2_subdev_get_fmt,
>>> +
>>> +	.get_frame_desc = rcsi2_get_frame_desc,
>>>  };
>>>  
>>>  static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
>>>
>>> -- 
>>> 2.43.0
>>>
>>
>

Re: [PATCH v3 10/15] media: rcar-csi2: Add .get_frame_desc op

Posted by Laurent Pinchart 6 months, 2 weeks ago

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:39PM +0300, Tomi Valkeinen wrote:
> Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
> 
> We also implement a fallback for the case where the upstream subdevice
> does not implement .get_frame_desc. It assumes a single stream with VC =
> 0 and DT based on the configured stream mbus format.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 56 ++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 20bd44274bd2..65c7f3040696 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1676,12 +1676,68 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
> +					 unsigned int pad,
> +					 struct v4l2_mbus_frame_desc *fd)
> +{
> +	const struct rcar_csi2_format *format;
> +	struct v4l2_subdev_state *state;
> +	struct v4l2_mbus_framefmt *fmt;
> +	int ret = 0;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);

This reminds me we should pass the state to the .get_frame_desc()
operation.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK, 0);
> +	if (!fmt) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	format = rcsi2_code_to_fmt(fmt->code);
> +	if (!format) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	fd->num_entries = 1;
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	fd->entry[0].stream = 0;
> +	fd->entry[0].pixelcode = fmt->code;
> +	fd->entry[0].bus.csi2.vc = 0;
> +	fd->entry[0].bus.csi2.dt = format->datatype;
> +
> +out:
> +	v4l2_subdev_unlock_state(state);
> +
> +	return ret;
> +}
> +
> +static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	int ret;
> +
> +	if (WARN_ON(!priv->info->use_isp))
> +		return -ENOTTY;
> +
> +	if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
> +		return -EINVAL;
> +
> +	ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
> +	if (ret == -ENOIOCTLCMD)
> +		ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
> +	return ret;
> +}
> +
>  static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
>  	.enable_streams = rcsi2_enable_streams,
>  	.disable_streams = rcsi2_disable_streams,
>  
>  	.set_fmt = rcsi2_set_pad_format,
>  	.get_fmt = v4l2_subdev_get_fmt,
> +
> +	.get_frame_desc = rcsi2_get_frame_desc,
>  };
>  
>  static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {

-- 
Regards,

Laurent Pinchart