[12/14] staging: vc04_services: bcm2835_isp: Allow multiple users
Commit Message
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
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:
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:
@@ -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: