[v3] media: rcar: Streams support

[PATCH v3 09/15] media: rcar-isp: Switch to Streams API

Posted by Tomi Valkeinen 6 months, 2 weeks ago

Switch to Streams API with a single hardcoded route. This breaks any
existing userspace which depended on the custom rcar streams
implementation, but a single camera use case should continue to work.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 62 ++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 2337c5d44c40..a04cbf96b809 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -124,6 +124,17 @@ static const struct rcar_isp_format rcar_isp_formats[] = {
 	},
 };
 
+static const struct v4l2_mbus_framefmt risp_default_fmt = {
+	.width = 1920,
+	.height = 1080,
+	.code = MEDIA_BUS_FMT_RGB888_1X24,
+	.colorspace = V4L2_COLORSPACE_SRGB,
+	.field = V4L2_FIELD_NONE,
+	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
+	.quantization = V4L2_QUANTIZATION_DEFAULT,
+	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
+};
+
 static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
 {
 	unsigned int i;
@@ -222,7 +233,7 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
 	u32 sel_csi = 0;
 	int ret;
 
-	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK);
+	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
 	if (!fmt)
 		return -EINVAL;
 
@@ -336,7 +347,7 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_state *state,
 			       struct v4l2_subdev_format *format)
 {
-	struct v4l2_mbus_framefmt *framefmt;
+	struct v4l2_mbus_framefmt *fmt;
 
 	if (format->pad > RCAR_ISP_SINK)
 		return v4l2_subdev_get_fmt(sd, state, format);
@@ -344,10 +355,20 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
 	if (!risp_code_to_fmt(format->format.code))
 		format->format.code = rcar_isp_formats[0].code;
 
-	for (unsigned int i = 0; i < RCAR_ISP_NUM_PADS; i++) {
-		framefmt = v4l2_subdev_state_get_format(state, i);
-		*framefmt = format->format;
-	}
+	/* Set sink format */
+	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
+
+	/* Propagate to source format */
+	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+							   format->stream);
+	if (!fmt)
+		return -EINVAL;
+
+	*fmt = format->format;
 
 	return 0;
 }
@@ -364,6 +385,32 @@ static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {
 	.pad	= &risp_pad_ops,
 };
 
+static int risp_init_state(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_state *state)
+{
+	static struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = RCAR_ISP_SINK,
+			.sink_stream = 0,
+			.source_pad = RCAR_ISP_PORT0,
+			.source_stream = 0,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+
+	static const struct v4l2_subdev_krouting routing = {
+		.num_routes = ARRAY_SIZE(routes),
+		.routes = routes,
+	};
+
+	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
+						&risp_default_fmt);
+}
+
+static const struct v4l2_subdev_internal_ops risp_internal_ops = {
+	.init_state = risp_init_state,
+};
+
 /* -----------------------------------------------------------------------------
  * Async handling and registration of subdevices and links
  */
@@ -521,11 +568,12 @@ static int risp_probe(struct platform_device *pdev)
 
 	isp->subdev.owner = THIS_MODULE;
 	isp->subdev.dev = &pdev->dev;
+	isp->subdev.internal_ops = &risp_internal_ops;
 	v4l2_subdev_init(&isp->subdev, &rcar_isp_subdev_ops);
 	v4l2_set_subdevdata(&isp->subdev, &pdev->dev);
 	snprintf(isp->subdev.name, sizeof(isp->subdev.name), "%s %s",
 		 KBUILD_MODNAME, dev_name(&pdev->dev));
-	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
 
 	isp->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
 	isp->subdev.entity.ops = &risp_entity_ops;

-- 
2.43.0

Re: [PATCH v3 09/15] media: rcar-isp: Switch to Streams API

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

Hi Tomi,

Thanks for your work.

On 2025-05-30 16:50:38 +0300, Tomi Valkeinen wrote:
> Switch to Streams API with a single hardcoded route. This breaks any
> existing userspace which depended on the custom rcar streams
> implementation, but a single camera use case should continue to work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 62 ++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 2337c5d44c40..a04cbf96b809 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -124,6 +124,17 @@ static const struct rcar_isp_format rcar_isp_formats[] = {
>  	},
>  };
>  
> +static const struct v4l2_mbus_framefmt risp_default_fmt = {
> +	.width = 1920,
> +	.height = 1080,
> +	.code = MEDIA_BUS_FMT_RGB888_1X24,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +	.field = V4L2_FIELD_NONE,
> +	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> +	.quantization = V4L2_QUANTIZATION_DEFAULT,
> +	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
> +};
> +
>  static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
>  {
>  	unsigned int i;
> @@ -222,7 +233,7 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	u32 sel_csi = 0;
>  	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK);
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
>  	if (!fmt)
>  		return -EINVAL;
>  
> @@ -336,7 +347,7 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  			       struct v4l2_subdev_state *state,
>  			       struct v4l2_subdev_format *format)
>  {
> -	struct v4l2_mbus_framefmt *framefmt;
> +	struct v4l2_mbus_framefmt *fmt;
>  
>  	if (format->pad > RCAR_ISP_SINK)
>  		return v4l2_subdev_get_fmt(sd, state, format);
> @@ -344,10 +355,20 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  	if (!risp_code_to_fmt(format->format.code))
>  		format->format.code = rcar_isp_formats[0].code;
>  
> -	for (unsigned int i = 0; i < RCAR_ISP_NUM_PADS; i++) {
> -		framefmt = v4l2_subdev_state_get_format(state, i);
> -		*framefmt = format->format;
> -	}
> +	/* Set sink format */
> +	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
> +
> +	/* Propagate to source format */
> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
>  
>  	return 0;
>  }
> @@ -364,6 +385,32 @@ static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {
>  	.pad	= &risp_pad_ops,
>  };
>  
> +static int risp_init_state(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *state)
> +{
> +	static struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = RCAR_ISP_SINK,
> +			.sink_stream = 0,
> +			.source_pad = RCAR_ISP_PORT0,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +
> +	static const struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
> +
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
> +						&risp_default_fmt);
> +}
> +
> +static const struct v4l2_subdev_internal_ops risp_internal_ops = {
> +	.init_state = risp_init_state,
> +};
> +
>  /* -----------------------------------------------------------------------------
>   * Async handling and registration of subdevices and links
>   */
> @@ -521,11 +568,12 @@ static int risp_probe(struct platform_device *pdev)
>  
>  	isp->subdev.owner = THIS_MODULE;
>  	isp->subdev.dev = &pdev->dev;
> +	isp->subdev.internal_ops = &risp_internal_ops;
>  	v4l2_subdev_init(&isp->subdev, &rcar_isp_subdev_ops);
>  	v4l2_set_subdevdata(&isp->subdev, &pdev->dev);
>  	snprintf(isp->subdev.name, sizeof(isp->subdev.name), "%s %s",
>  		 KBUILD_MODNAME, dev_name(&pdev->dev));
> -	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
>  
>  	isp->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
>  	isp->subdev.entity.ops = &risp_entity_ops;
> 
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v3 09/15] media: rcar-isp: Switch to Streams API

Posted by Laurent Pinchart 6 months, 2 weeks ago

Hi Tomi,

Thank you for the patch.

On Fri, May 30, 2025 at 04:50:38PM +0300, Tomi Valkeinen wrote:
> Switch to Streams API with a single hardcoded route. This breaks any
> existing userspace which depended on the custom rcar streams
> implementation, but a single camera use case should continue to work.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 62 ++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 2337c5d44c40..a04cbf96b809 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -124,6 +124,17 @@ static const struct rcar_isp_format rcar_isp_formats[] = {
>  	},
>  };
>  
> +static const struct v4l2_mbus_framefmt risp_default_fmt = {
> +	.width = 1920,
> +	.height = 1080,
> +	.code = MEDIA_BUS_FMT_RGB888_1X24,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +	.field = V4L2_FIELD_NONE,
> +	.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT,
> +	.quantization = V4L2_QUANTIZATION_DEFAULT,
> +	.xfer_func = V4L2_XFER_FUNC_DEFAULT,
> +};
> +
>  static const struct rcar_isp_format *risp_code_to_fmt(unsigned int code)
>  {
>  	unsigned int i;
> @@ -222,7 +233,7 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	u32 sel_csi = 0;
>  	int ret;
>  
> -	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK);
> +	fmt = v4l2_subdev_state_get_format(state, RCAR_ISP_SINK, 0);
>  	if (!fmt)
>  		return -EINVAL;
>  
> @@ -336,7 +347,7 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  			       struct v4l2_subdev_state *state,
>  			       struct v4l2_subdev_format *format)
>  {
> -	struct v4l2_mbus_framefmt *framefmt;
> +	struct v4l2_mbus_framefmt *fmt;
>  
>  	if (format->pad > RCAR_ISP_SINK)
>  		return v4l2_subdev_get_fmt(sd, state, format);
> @@ -344,10 +355,20 @@ static int risp_set_pad_format(struct v4l2_subdev *sd,
>  	if (!risp_code_to_fmt(format->format.code))
>  		format->format.code = rcar_isp_formats[0].code;
>  
> -	for (unsigned int i = 0; i < RCAR_ISP_NUM_PADS; i++) {
> -		framefmt = v4l2_subdev_state_get_format(state, i);
> -		*framefmt = format->format;
> -	}
> +	/* Set sink format */

s/format/format./

> +	fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
> +
> +	/* Propagate to source format */

	/* Propagate the format to the source pad. */

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

> +	fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> +							   format->stream);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	*fmt = format->format;
>  
>  	return 0;
>  }
> @@ -364,6 +385,32 @@ static const struct v4l2_subdev_ops rcar_isp_subdev_ops = {
>  	.pad	= &risp_pad_ops,
>  };
>  
> +static int risp_init_state(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *state)
> +{
> +	static struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = RCAR_ISP_SINK,
> +			.sink_stream = 0,
> +			.source_pad = RCAR_ISP_PORT0,
> +			.source_stream = 0,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +
> +	static const struct v4l2_subdev_krouting routing = {
> +		.num_routes = ARRAY_SIZE(routes),
> +		.routes = routes,
> +	};
> +
> +	return v4l2_subdev_set_routing_with_fmt(sd, state, &routing,
> +						&risp_default_fmt);
> +}
> +
> +static const struct v4l2_subdev_internal_ops risp_internal_ops = {
> +	.init_state = risp_init_state,
> +};
> +
>  /* -----------------------------------------------------------------------------
>   * Async handling and registration of subdevices and links
>   */
> @@ -521,11 +568,12 @@ static int risp_probe(struct platform_device *pdev)
>  
>  	isp->subdev.owner = THIS_MODULE;
>  	isp->subdev.dev = &pdev->dev;
> +	isp->subdev.internal_ops = &risp_internal_ops;
>  	v4l2_subdev_init(&isp->subdev, &rcar_isp_subdev_ops);
>  	v4l2_set_subdevdata(&isp->subdev, &pdev->dev);
>  	snprintf(isp->subdev.name, sizeof(isp->subdev.name), "%s %s",
>  		 KBUILD_MODNAME, dev_name(&pdev->dev));
> -	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	isp->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
>  
>  	isp->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
>  	isp->subdev.entity.ops = &risp_entity_ops;

-- 
Regards,

Laurent Pinchart