[12/14] staging: vc04_services: bcm2835_isp: Allow multiple users

Message ID 20221121214722.22563-13-umang.jain@ideasonboard.com
State New
Headers
Series staging: vc04_services: bcm2835-isp support |

Commit Message

Umang Jain Nov. 21, 2022, 9:47 p.m. UTC
  From: Naushir Patuck <naush@raspberrypi.com>

Add a second (identical) set of device nodes to allow concurrent use of
the bcm2835-isp hardware by another user. This change effectively
creates a second state structure (struct bcm2835_isp_dev) to maintain
independent state for the second user. Node and media entity names are
appened with the instance index appropriately.

Further users can be added by changing the BCM2835_ISP_NUM_INSTANCES
define.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 .../bcm2835-isp/bcm2835-v4l2-isp.c            | 77 +++++++++++++++----
 1 file changed, 61 insertions(+), 16 deletions(-)
  

Comments

Laurent Pinchart Nov. 21, 2022, 11:29 p.m. UTC | #1
Hi Umang and Naush,

Thank you for the patch.

On Tue, Nov 22, 2022 at 03:17:20AM +0530, Umang Jain wrote:
> From: Naushir Patuck <naush@raspberrypi.com>
> 
> Add a second (identical) set of device nodes to allow concurrent use of
> the bcm2835-isp hardware by another user. This change effectively
> creates a second state structure (struct bcm2835_isp_dev) to maintain
> independent state for the second user. Node and media entity names are
> appened with the instance index appropriately.
> 
> Further users can be added by changing the BCM2835_ISP_NUM_INSTANCES
> define.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Let's squash this with 09/14.

> ---
>  .../bcm2835-isp/bcm2835-v4l2-isp.c            | 77 +++++++++++++++----
>  1 file changed, 61 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> index cb7cdba76682..0dbcb25595e7 100644
> --- a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> +++ b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> @@ -28,13 +28,19 @@
>  
>  MODULE_IMPORT_NS(DMA_BUF);
>  
> +/*
> + * We want to instantiate 2 independent instances allowing 2 simultaneous users
> + * of the ISP hardware.
> + */
> +#define BCM2835_ISP_NUM_INSTANCES 2
> +
>  static unsigned int debug;
>  module_param(debug, uint, 0644);
>  MODULE_PARM_DESC(debug, "activates debug info");
>  
> -static unsigned int video_nr = 13;
> -module_param(video_nr, uint, 0644);
> -MODULE_PARM_DESC(video_nr, "base video device number");
> +static unsigned int video_nr[BCM2835_ISP_NUM_INSTANCES] = { 13, 20 };
> +module_param_array(video_nr, uint, NULL, 0644);
> +MODULE_PARM_DESC(video_nr, "base video device numbers");
>  
>  #define BCM2835_ISP_NAME "bcm2835-isp"
>  #define BCM2835_ISP_ENTITY_NAME_LEN 32
> @@ -1286,6 +1292,7 @@ static int bcm2835_isp_get_supported_fmts(struct bcm2835_isp_node *node)
>   * or output nodes.
>   */
>  static int register_node(struct bcm2835_isp_dev *dev,
> +			 unsigned int instance,
>  			 struct bcm2835_isp_node *node,
>  			 int index)
>  {
> @@ -1447,7 +1454,7 @@ static int register_node(struct bcm2835_isp_dev *dev,
>  	snprintf(vfd->name, sizeof(node->vfd.name), "%s-%s%d", BCM2835_ISP_NAME,
>  		 node->name, node->id);
>  
> -	ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr + index);
> +	ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr[instance]);
>  	if (ret) {
>  		v4l2_err(&dev->v4l2_dev,
>  			 "Failed to register video %s[%d] device node\n",
> @@ -1668,9 +1675,8 @@ static int media_controller_register(struct bcm2835_isp_dev *dev)
>  	return ret;
>  }
>  
> -static int bcm2835_isp_remove(struct platform_device *pdev)
> +static void bcm2835_isp_remove_instance(struct bcm2835_isp_dev *dev)
>  {
> -	struct bcm2835_isp_dev *dev = platform_get_drvdata(pdev);
>  	unsigned int i;
>  
>  	media_controller_unregister(dev);
> @@ -1685,11 +1691,11 @@ static int bcm2835_isp_remove(struct platform_device *pdev)
>  					      dev->component);
>  
>  	vchiq_mmal_finalise(dev->mmal_instance);
> -
> -	return 0;
>  }
>  
> -static int bcm2835_isp_probe(struct platform_device *pdev)
> +static int bcm2835_isp_probe_instance(struct platform_device *pdev,
> +				      struct bcm2835_isp_dev **dev_int,
> +				      unsigned int instance)
>  {
>  	struct bcm2835_isp_dev *dev;
>  	unsigned int i;
> @@ -1699,6 +1705,7 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
>  	if (!dev)
>  		return -ENOMEM;
>  
> +	*dev_int = dev;
>  	dev->dev = &pdev->dev;
>  
>  	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> @@ -1716,7 +1723,7 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
>  	if (ret) {
>  		v4l2_err(&dev->v4l2_dev,
>  			 "%s: failed to create ril.isp component\n", __func__);
> -		goto error;
> +		return ret;
>  	}
>  
>  	if (dev->component->inputs < BCM2835_ISP_NUM_OUTPUTS ||
> @@ -1728,7 +1735,7 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
>  			  BCM2835_ISP_NUM_OUTPUTS,
>  			  dev->component->outputs,
>  			  BCM2835_ISP_NUM_CAPTURES + BCM2835_ISP_NUM_METADATA);
> -		goto error;
> +		return -EINVAL;
>  	}
>  
>  	atomic_set(&dev->num_streaming, 0);
> @@ -1736,17 +1743,55 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
>  	for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
>  		struct bcm2835_isp_node *node = &dev->node[i];
>  
> -		ret = register_node(dev, node, i);
> +		ret = register_node(dev, instance, node, i);
>  		if (ret)
> -			goto error;
> +			return ret;
>  	}
>  
>  	ret = media_controller_register(dev);
>  	if (ret)
> -		goto error;
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int bcm2835_isp_remove(struct platform_device *pdev)
> +{
> +	struct bcm2835_isp_dev **bcm2835_isp_instances;
> +	unsigned int i;
> +
> +	bcm2835_isp_instances = platform_get_drvdata(pdev);
> +	for (i = 0; i < BCM2835_ISP_NUM_INSTANCES; i++) {
> +		if (bcm2835_isp_instances[i])
> +			bcm2835_isp_remove_instance(bcm2835_isp_instances[i]);
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcm2835_isp_probe(struct platform_device *pdev)
> +{
> +	struct bcm2835_isp_dev **bcm2835_isp_instances;
> +	unsigned int i;
> +	int ret;
> +
> +	bcm2835_isp_instances = devm_kzalloc(&pdev->dev,
> +					     sizeof(bcm2835_isp_instances) *
> +						      BCM2835_ISP_NUM_INSTANCES,
> +					     GFP_KERNEL);
> +	if (!bcm2835_isp_instances)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, bcm2835_isp_instances);
> +
> +	for (i = 0; i < BCM2835_ISP_NUM_INSTANCES; i++) {
> +		ret = bcm2835_isp_probe_instance(pdev,
> +						 &bcm2835_isp_instances[i], i);
> +		if (ret)
> +			goto error;
> +	}
>  
> -	platform_set_drvdata(pdev, dev);
> -	v4l2_info(&dev->v4l2_dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
> +	dev_info(&pdev->dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
>  	return 0;
>  
>  error:
  
Laurent Pinchart Nov. 21, 2022, 11:35 p.m. UTC | #2
On Tue, Nov 22, 2022 at 01:29:48AM +0200, Laurent Pinchart wrote:
> Hi Umang and Naush,
> 
> Thank you for the patch.
> 
> On Tue, Nov 22, 2022 at 03:17:20AM +0530, Umang Jain wrote:
> > From: Naushir Patuck <naush@raspberrypi.com>
> > 
> > Add a second (identical) set of device nodes to allow concurrent use of
> > the bcm2835-isp hardware by another user. This change effectively
> > creates a second state structure (struct bcm2835_isp_dev) to maintain
> > independent state for the second user. Node and media entity names are
> > appened with the instance index appropriately.
> > 
> > Further users can be added by changing the BCM2835_ISP_NUM_INSTANCES
> > define.
> > 
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> 
> Let's squash this with 09/14.

Also, the documentation in 13/14 should be updated.

> > ---
> >  .../bcm2835-isp/bcm2835-v4l2-isp.c            | 77 +++++++++++++++----
> >  1 file changed, 61 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> > index cb7cdba76682..0dbcb25595e7 100644
> > --- a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> > +++ b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
> > @@ -28,13 +28,19 @@
> >  
> >  MODULE_IMPORT_NS(DMA_BUF);
> >  
> > +/*
> > + * We want to instantiate 2 independent instances allowing 2 simultaneous users
> > + * of the ISP hardware.
> > + */
> > +#define BCM2835_ISP_NUM_INSTANCES 2
> > +
> >  static unsigned int debug;
> >  module_param(debug, uint, 0644);
> >  MODULE_PARM_DESC(debug, "activates debug info");
> >  
> > -static unsigned int video_nr = 13;
> > -module_param(video_nr, uint, 0644);
> > -MODULE_PARM_DESC(video_nr, "base video device number");
> > +static unsigned int video_nr[BCM2835_ISP_NUM_INSTANCES] = { 13, 20 };
> > +module_param_array(video_nr, uint, NULL, 0644);
> > +MODULE_PARM_DESC(video_nr, "base video device numbers");
> >  
> >  #define BCM2835_ISP_NAME "bcm2835-isp"
> >  #define BCM2835_ISP_ENTITY_NAME_LEN 32
> > @@ -1286,6 +1292,7 @@ static int bcm2835_isp_get_supported_fmts(struct bcm2835_isp_node *node)
> >   * or output nodes.
> >   */
> >  static int register_node(struct bcm2835_isp_dev *dev,
> > +			 unsigned int instance,
> >  			 struct bcm2835_isp_node *node,
> >  			 int index)
> >  {
> > @@ -1447,7 +1454,7 @@ static int register_node(struct bcm2835_isp_dev *dev,
> >  	snprintf(vfd->name, sizeof(node->vfd.name), "%s-%s%d", BCM2835_ISP_NAME,
> >  		 node->name, node->id);
> >  
> > -	ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr + index);
> > +	ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr[instance]);
> >  	if (ret) {
> >  		v4l2_err(&dev->v4l2_dev,
> >  			 "Failed to register video %s[%d] device node\n",
> > @@ -1668,9 +1675,8 @@ static int media_controller_register(struct bcm2835_isp_dev *dev)
> >  	return ret;
> >  }
> >  
> > -static int bcm2835_isp_remove(struct platform_device *pdev)
> > +static void bcm2835_isp_remove_instance(struct bcm2835_isp_dev *dev)
> >  {
> > -	struct bcm2835_isp_dev *dev = platform_get_drvdata(pdev);
> >  	unsigned int i;
> >  
> >  	media_controller_unregister(dev);
> > @@ -1685,11 +1691,11 @@ static int bcm2835_isp_remove(struct platform_device *pdev)
> >  					      dev->component);
> >  
> >  	vchiq_mmal_finalise(dev->mmal_instance);
> > -
> > -	return 0;
> >  }
> >  
> > -static int bcm2835_isp_probe(struct platform_device *pdev)
> > +static int bcm2835_isp_probe_instance(struct platform_device *pdev,
> > +				      struct bcm2835_isp_dev **dev_int,
> > +				      unsigned int instance)
> >  {
> >  	struct bcm2835_isp_dev *dev;
> >  	unsigned int i;
> > @@ -1699,6 +1705,7 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
> >  	if (!dev)
> >  		return -ENOMEM;
> >  
> > +	*dev_int = dev;
> >  	dev->dev = &pdev->dev;
> >  
> >  	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > @@ -1716,7 +1723,7 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
> >  	if (ret) {
> >  		v4l2_err(&dev->v4l2_dev,
> >  			 "%s: failed to create ril.isp component\n", __func__);
> > -		goto error;
> > +		return ret;
> >  	}
> >  
> >  	if (dev->component->inputs < BCM2835_ISP_NUM_OUTPUTS ||
> > @@ -1728,7 +1735,7 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
> >  			  BCM2835_ISP_NUM_OUTPUTS,
> >  			  dev->component->outputs,
> >  			  BCM2835_ISP_NUM_CAPTURES + BCM2835_ISP_NUM_METADATA);
> > -		goto error;
> > +		return -EINVAL;
> >  	}
> >  
> >  	atomic_set(&dev->num_streaming, 0);
> > @@ -1736,17 +1743,55 @@ static int bcm2835_isp_probe(struct platform_device *pdev)
> >  	for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
> >  		struct bcm2835_isp_node *node = &dev->node[i];
> >  
> > -		ret = register_node(dev, node, i);
> > +		ret = register_node(dev, instance, node, i);
> >  		if (ret)
> > -			goto error;
> > +			return ret;
> >  	}
> >  
> >  	ret = media_controller_register(dev);
> >  	if (ret)
> > -		goto error;
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int bcm2835_isp_remove(struct platform_device *pdev)
> > +{
> > +	struct bcm2835_isp_dev **bcm2835_isp_instances;
> > +	unsigned int i;
> > +
> > +	bcm2835_isp_instances = platform_get_drvdata(pdev);
> > +	for (i = 0; i < BCM2835_ISP_NUM_INSTANCES; i++) {
> > +		if (bcm2835_isp_instances[i])
> > +			bcm2835_isp_remove_instance(bcm2835_isp_instances[i]);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int bcm2835_isp_probe(struct platform_device *pdev)
> > +{
> > +	struct bcm2835_isp_dev **bcm2835_isp_instances;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	bcm2835_isp_instances = devm_kzalloc(&pdev->dev,
> > +					     sizeof(bcm2835_isp_instances) *
> > +						      BCM2835_ISP_NUM_INSTANCES,
> > +					     GFP_KERNEL);
> > +	if (!bcm2835_isp_instances)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, bcm2835_isp_instances);
> > +
> > +	for (i = 0; i < BCM2835_ISP_NUM_INSTANCES; i++) {
> > +		ret = bcm2835_isp_probe_instance(pdev,
> > +						 &bcm2835_isp_instances[i], i);
> > +		if (ret)
> > +			goto error;
> > +	}
> >  
> > -	platform_set_drvdata(pdev, dev);
> > -	v4l2_info(&dev->v4l2_dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
> > +	dev_info(&pdev->dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
> >  	return 0;
> >  
> >  error:
  

Patch

diff --git a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
index cb7cdba76682..0dbcb25595e7 100644
--- a/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
+++ b/drivers/staging/vc04_services/bcm2835-isp/bcm2835-v4l2-isp.c
@@ -28,13 +28,19 @@ 
 
 MODULE_IMPORT_NS(DMA_BUF);
 
+/*
+ * We want to instantiate 2 independent instances allowing 2 simultaneous users
+ * of the ISP hardware.
+ */
+#define BCM2835_ISP_NUM_INSTANCES 2
+
 static unsigned int debug;
 module_param(debug, uint, 0644);
 MODULE_PARM_DESC(debug, "activates debug info");
 
-static unsigned int video_nr = 13;
-module_param(video_nr, uint, 0644);
-MODULE_PARM_DESC(video_nr, "base video device number");
+static unsigned int video_nr[BCM2835_ISP_NUM_INSTANCES] = { 13, 20 };
+module_param_array(video_nr, uint, NULL, 0644);
+MODULE_PARM_DESC(video_nr, "base video device numbers");
 
 #define BCM2835_ISP_NAME "bcm2835-isp"
 #define BCM2835_ISP_ENTITY_NAME_LEN 32
@@ -1286,6 +1292,7 @@  static int bcm2835_isp_get_supported_fmts(struct bcm2835_isp_node *node)
  * or output nodes.
  */
 static int register_node(struct bcm2835_isp_dev *dev,
+			 unsigned int instance,
 			 struct bcm2835_isp_node *node,
 			 int index)
 {
@@ -1447,7 +1454,7 @@  static int register_node(struct bcm2835_isp_dev *dev,
 	snprintf(vfd->name, sizeof(node->vfd.name), "%s-%s%d", BCM2835_ISP_NAME,
 		 node->name, node->id);
 
-	ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr + index);
+	ret = video_register_device(vfd, VFL_TYPE_VIDEO, video_nr[instance]);
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev,
 			 "Failed to register video %s[%d] device node\n",
@@ -1668,9 +1675,8 @@  static int media_controller_register(struct bcm2835_isp_dev *dev)
 	return ret;
 }
 
-static int bcm2835_isp_remove(struct platform_device *pdev)
+static void bcm2835_isp_remove_instance(struct bcm2835_isp_dev *dev)
 {
-	struct bcm2835_isp_dev *dev = platform_get_drvdata(pdev);
 	unsigned int i;
 
 	media_controller_unregister(dev);
@@ -1685,11 +1691,11 @@  static int bcm2835_isp_remove(struct platform_device *pdev)
 					      dev->component);
 
 	vchiq_mmal_finalise(dev->mmal_instance);
-
-	return 0;
 }
 
-static int bcm2835_isp_probe(struct platform_device *pdev)
+static int bcm2835_isp_probe_instance(struct platform_device *pdev,
+				      struct bcm2835_isp_dev **dev_int,
+				      unsigned int instance)
 {
 	struct bcm2835_isp_dev *dev;
 	unsigned int i;
@@ -1699,6 +1705,7 @@  static int bcm2835_isp_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
+	*dev_int = dev;
 	dev->dev = &pdev->dev;
 
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
@@ -1716,7 +1723,7 @@  static int bcm2835_isp_probe(struct platform_device *pdev)
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev,
 			 "%s: failed to create ril.isp component\n", __func__);
-		goto error;
+		return ret;
 	}
 
 	if (dev->component->inputs < BCM2835_ISP_NUM_OUTPUTS ||
@@ -1728,7 +1735,7 @@  static int bcm2835_isp_probe(struct platform_device *pdev)
 			  BCM2835_ISP_NUM_OUTPUTS,
 			  dev->component->outputs,
 			  BCM2835_ISP_NUM_CAPTURES + BCM2835_ISP_NUM_METADATA);
-		goto error;
+		return -EINVAL;
 	}
 
 	atomic_set(&dev->num_streaming, 0);
@@ -1736,17 +1743,55 @@  static int bcm2835_isp_probe(struct platform_device *pdev)
 	for (i = 0; i < BCM2835_ISP_NUM_NODES; i++) {
 		struct bcm2835_isp_node *node = &dev->node[i];
 
-		ret = register_node(dev, node, i);
+		ret = register_node(dev, instance, node, i);
 		if (ret)
-			goto error;
+			return ret;
 	}
 
 	ret = media_controller_register(dev);
 	if (ret)
-		goto error;
+		return ret;
+
+	return 0;
+}
+
+static int bcm2835_isp_remove(struct platform_device *pdev)
+{
+	struct bcm2835_isp_dev **bcm2835_isp_instances;
+	unsigned int i;
+
+	bcm2835_isp_instances = platform_get_drvdata(pdev);
+	for (i = 0; i < BCM2835_ISP_NUM_INSTANCES; i++) {
+		if (bcm2835_isp_instances[i])
+			bcm2835_isp_remove_instance(bcm2835_isp_instances[i]);
+	}
+
+	return 0;
+}
+
+static int bcm2835_isp_probe(struct platform_device *pdev)
+{
+	struct bcm2835_isp_dev **bcm2835_isp_instances;
+	unsigned int i;
+	int ret;
+
+	bcm2835_isp_instances = devm_kzalloc(&pdev->dev,
+					     sizeof(bcm2835_isp_instances) *
+						      BCM2835_ISP_NUM_INSTANCES,
+					     GFP_KERNEL);
+	if (!bcm2835_isp_instances)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, bcm2835_isp_instances);
+
+	for (i = 0; i < BCM2835_ISP_NUM_INSTANCES; i++) {
+		ret = bcm2835_isp_probe_instance(pdev,
+						 &bcm2835_isp_instances[i], i);
+		if (ret)
+			goto error;
+	}
 
-	platform_set_drvdata(pdev, dev);
-	v4l2_info(&dev->v4l2_dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
+	dev_info(&pdev->dev, "Loaded V4L2 %s\n", BCM2835_ISP_NAME);
 	return 0;
 
 error: