diff mbox series

[v2,07/13] media: ti: j721e-csi2rx: get number of contexts from device tree

Message ID 20240627-multistream-v2-7-6ae96c54c1c3@ti.com
State New
Headers show
Series media: cadence,ti: CSI2RX Multistream Support | expand

Commit Message

Jai Luthra June 27, 2024, 1:10 p.m. UTC
From: Pratyush Yadav <p.yadav@ti.com>

Different platforms that use this driver might have different number of
DMA channels allocated for CSI. So only as many DMA contexts can be used
as the number of DMA channels available. Get the number of channels
provided via device tree and only configure that many contexts, and
hence only that many pads.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
Co-developed-by: Jai Luthra <j-luthra@ti.com>
Signed-off-by: Jai Luthra <j-luthra@ti.com>
---
 .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  | 44 ++++++++++++++++------
 1 file changed, 32 insertions(+), 12 deletions(-)

Comments

Jacopo Mondi July 12, 2024, 2:43 p.m. UTC | #1
On Thu, Jun 27, 2024 at 06:40:02PM GMT, Jai Luthra wrote:
> From: Pratyush Yadav <p.yadav@ti.com>
>
> Different platforms that use this driver might have different number of
> DMA channels allocated for CSI. So only as many DMA contexts can be used
> as the number of DMA channels available. Get the number of channels
> provided via device tree and only configure that many contexts, and
> hence only that many pads.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> Co-developed-by: Jai Luthra <j-luthra@ti.com>
> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> ---
>  .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  | 44 ++++++++++++++++------
>  1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> index 13d7426ab4ba..b4b4bb69c88a 100644
> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> @@ -40,7 +40,7 @@
>  #define SHIM_PSI_CFG0_DST_TAG		GENMASK(31, 16)
>
>  #define PSIL_WORD_SIZE_BYTES		16
> -#define TI_CSI2RX_NUM_CTX		1
> +#define TI_CSI2RX_MAX_CTX		32
>
>  /*
>   * There are no hard limits on the width or height. The DMA engine can handle
> @@ -53,8 +53,8 @@
>
>  #define TI_CSI2RX_PAD_SINK		0
>  #define TI_CSI2RX_PAD_FIRST_SOURCE	1
> -#define TI_CSI2RX_NUM_SOURCE_PADS	1
> -#define TI_CSI2RX_NUM_PADS		(1 + TI_CSI2RX_NUM_SOURCE_PADS)
> +#define TI_CSI2RX_MAX_SOURCE_PADS	TI_CSI2RX_MAX_CTX
> +#define TI_CSI2RX_MAX_PADS		(1 + TI_CSI2RX_MAX_SOURCE_PADS)
>
>  #define DRAIN_TIMEOUT_MS		50
>  #define DRAIN_BUFFER_SIZE		SZ_32K
> @@ -112,14 +112,15 @@ struct ti_csi2rx_dev {
>  	void __iomem			*shim;
>  	struct mutex			mutex; /* To serialize ioctls. */
>  	unsigned int			enable_count;
> +	unsigned int			num_ctx;
>  	struct v4l2_device		v4l2_dev;
>  	struct media_device		mdev;
>  	struct media_pipeline		pipe;
> -	struct media_pad		pads[TI_CSI2RX_NUM_PADS];
> +	struct media_pad		pads[TI_CSI2RX_MAX_PADS];
>  	struct v4l2_async_notifier	notifier;
>  	struct v4l2_subdev		*source;
>  	struct v4l2_subdev		subdev;
> -	struct ti_csi2rx_ctx		ctx[TI_CSI2RX_NUM_CTX];
> +	struct ti_csi2rx_ctx		ctx[TI_CSI2RX_MAX_CTX];
>  	/* Buffer to drain stale data from PSI-L endpoint */
>  	struct {
>  		void			*vaddr;
> @@ -449,7 +450,7 @@ static int csi_async_notifier_complete(struct v4l2_async_notifier *notifier)
>  		return ret;
>
>  	/* Create and link video nodes for all DMA contexts */
> -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
> +	for (i = 0; i < csi->num_ctx; i++) {
>  		struct ti_csi2rx_ctx *ctx = &csi->ctx[i];
>  		struct video_device *vdev = &ctx->vdev;
>
> @@ -1231,10 +1232,12 @@ static int ti_csi2rx_v4l2_init(struct ti_csi2rx_dev *csi)
>
>  	csi->pads[TI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
>
> -	for (i = TI_CSI2RX_PAD_FIRST_SOURCE; i < TI_CSI2RX_NUM_PADS; i++)
> +	for (i = TI_CSI2RX_PAD_FIRST_SOURCE;
> +	     i < TI_CSI2RX_PAD_FIRST_SOURCE + csi->num_ctx; i++)
>  		csi->pads[i].flags = MEDIA_PAD_FL_SOURCE;
>
> -	ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(csi->pads),
> +	ret = media_entity_pads_init(&sd->entity,
> +				     TI_CSI2RX_PAD_FIRST_SOURCE + csi->num_ctx,
>  				     csi->pads);
>  	if (ret)
>  		goto unregister_media;
> @@ -1320,8 +1323,9 @@ static int ti_csi2rx_init_ctx(struct ti_csi2rx_ctx *ctx)
>
>  static int ti_csi2rx_probe(struct platform_device *pdev)
>  {
> +	struct device_node *np = pdev->dev.of_node;
>  	struct ti_csi2rx_dev *csi;
> -	int ret, i;
> +	int ret, i, count;
>
>  	csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);
>  	if (!csi)
> @@ -1343,13 +1347,29 @@ static int ti_csi2rx_probe(struct platform_device *pdev)
>  	if (!csi->drain.vaddr)
>  		return -ENOMEM;
>
> +	/* Only use as many contexts as the number of DMA channels allocated. */
> +	count = of_property_count_strings(np, "dma-names");

Is the property marked as required in the DTS bindings ?

If that's the case, I thought the assumption was that dts are
'correct'.

> +	if (count < 0) {
> +		dev_err(csi->dev, "Failed to get DMA channel count: %d\n",
> +			count);
> +		return count;
> +	}
> +
> +	csi->num_ctx = count;
> +	if (csi->num_ctx > TI_CSI2RX_MAX_CTX) {
> +		dev_warn(csi->dev,
> +			 "%u DMA channels passed. Maximum is %u. Ignoring the rest.\n",
> +			 csi->num_ctx, TI_CSI2RX_MAX_CTX);
> +		csi->num_ctx = TI_CSI2RX_MAX_CTX;
> +	}
> +
>  	mutex_init(&csi->mutex);
>
>  	ret = ti_csi2rx_v4l2_init(csi);
>  	if (ret)
>  		goto err_v4l2;
>
> -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
> +	for (i = 0; i < csi->num_ctx; i++) {
>  		csi->ctx[i].idx = i;
>  		csi->ctx[i].csi = csi;
>  		ret = ti_csi2rx_init_ctx(&csi->ctx[i]);
> @@ -1388,14 +1408,14 @@ static void ti_csi2rx_remove(struct platform_device *pdev)
>  	struct ti_csi2rx_dev *csi = platform_get_drvdata(pdev);
>  	int i;
>
> -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
> +	for (i = 0; i < csi->num_ctx; i++) {
>  		if (vb2_is_busy(&csi->ctx[i].vidq))
>  			dev_err(csi->dev,
>  				"Failed to remove as queue busy for ctx %u\n",
>  				i);
>  	}
>
> -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++)
> +	for (i = 0; i < csi->num_ctx; i++)
>  		ti_csi2rx_cleanup_ctx(&csi->ctx[i]);
>
>  	ti_csi2rx_cleanup_notifier(csi);
>
> --
> 2.43.0
>
>
Jai Luthra July 16, 2024, 9:33 a.m. UTC | #2
Hi Jacopo,

Thanks for the review.

On Jul 12, 2024 at 16:43:38 +0200, Jacopo Mondi wrote:
> On Thu, Jun 27, 2024 at 06:40:02PM GMT, Jai Luthra wrote:
> > From: Pratyush Yadav <p.yadav@ti.com>
> >
> > Different platforms that use this driver might have different number of
> > DMA channels allocated for CSI. So only as many DMA contexts can be used
> > as the number of DMA channels available. Get the number of channels
> > provided via device tree and only configure that many contexts, and
> > hence only that many pads.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > Co-developed-by: Jai Luthra <j-luthra@ti.com>
> > Signed-off-by: Jai Luthra <j-luthra@ti.com>
> > ---
> >  .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  | 44 ++++++++++++++++------
> >  1 file changed, 32 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> > index 13d7426ab4ba..b4b4bb69c88a 100644
> > --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> > +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> > @@ -40,7 +40,7 @@
> >  #define SHIM_PSI_CFG0_DST_TAG		GENMASK(31, 16)
> >
> >  #define PSIL_WORD_SIZE_BYTES		16
> > -#define TI_CSI2RX_NUM_CTX		1
> > +#define TI_CSI2RX_MAX_CTX		32
> >
> >  /*
> >   * There are no hard limits on the width or height. The DMA engine can handle
> > @@ -53,8 +53,8 @@
> >
> >  #define TI_CSI2RX_PAD_SINK		0
> >  #define TI_CSI2RX_PAD_FIRST_SOURCE	1
> > -#define TI_CSI2RX_NUM_SOURCE_PADS	1
> > -#define TI_CSI2RX_NUM_PADS		(1 + TI_CSI2RX_NUM_SOURCE_PADS)
> > +#define TI_CSI2RX_MAX_SOURCE_PADS	TI_CSI2RX_MAX_CTX
> > +#define TI_CSI2RX_MAX_PADS		(1 + TI_CSI2RX_MAX_SOURCE_PADS)
> >
> >  #define DRAIN_TIMEOUT_MS		50
> >  #define DRAIN_BUFFER_SIZE		SZ_32K
> > @@ -112,14 +112,15 @@ struct ti_csi2rx_dev {
> >  	void __iomem			*shim;
> >  	struct mutex			mutex; /* To serialize ioctls. */
> >  	unsigned int			enable_count;
> > +	unsigned int			num_ctx;
> >  	struct v4l2_device		v4l2_dev;
> >  	struct media_device		mdev;
> >  	struct media_pipeline		pipe;
> > -	struct media_pad		pads[TI_CSI2RX_NUM_PADS];
> > +	struct media_pad		pads[TI_CSI2RX_MAX_PADS];
> >  	struct v4l2_async_notifier	notifier;
> >  	struct v4l2_subdev		*source;
> >  	struct v4l2_subdev		subdev;
> > -	struct ti_csi2rx_ctx		ctx[TI_CSI2RX_NUM_CTX];
> > +	struct ti_csi2rx_ctx		ctx[TI_CSI2RX_MAX_CTX];
> >  	/* Buffer to drain stale data from PSI-L endpoint */
> >  	struct {
> >  		void			*vaddr;
> > @@ -449,7 +450,7 @@ static int csi_async_notifier_complete(struct v4l2_async_notifier *notifier)
> >  		return ret;
> >
> >  	/* Create and link video nodes for all DMA contexts */
> > -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
> > +	for (i = 0; i < csi->num_ctx; i++) {
> >  		struct ti_csi2rx_ctx *ctx = &csi->ctx[i];
> >  		struct video_device *vdev = &ctx->vdev;
> >
> > @@ -1231,10 +1232,12 @@ static int ti_csi2rx_v4l2_init(struct ti_csi2rx_dev *csi)
> >
> >  	csi->pads[TI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> >
> > -	for (i = TI_CSI2RX_PAD_FIRST_SOURCE; i < TI_CSI2RX_NUM_PADS; i++)
> > +	for (i = TI_CSI2RX_PAD_FIRST_SOURCE;
> > +	     i < TI_CSI2RX_PAD_FIRST_SOURCE + csi->num_ctx; i++)
> >  		csi->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> >
> > -	ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(csi->pads),
> > +	ret = media_entity_pads_init(&sd->entity,
> > +				     TI_CSI2RX_PAD_FIRST_SOURCE + csi->num_ctx,
> >  				     csi->pads);
> >  	if (ret)
> >  		goto unregister_media;
> > @@ -1320,8 +1323,9 @@ static int ti_csi2rx_init_ctx(struct ti_csi2rx_ctx *ctx)
> >
> >  static int ti_csi2rx_probe(struct platform_device *pdev)
> >  {
> > +	struct device_node *np = pdev->dev.of_node;
> >  	struct ti_csi2rx_dev *csi;
> > -	int ret, i;
> > +	int ret, i, count;
> >
> >  	csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);
> >  	if (!csi)
> > @@ -1343,13 +1347,29 @@ static int ti_csi2rx_probe(struct platform_device *pdev)
> >  	if (!csi->drain.vaddr)
> >  		return -ENOMEM;
> >
> > +	/* Only use as many contexts as the number of DMA channels allocated. */
> > +	count = of_property_count_strings(np, "dma-names");
> 
> Is the property marked as required in the DTS bindings ?
> 
> If that's the case, I thought the assumption was that dts are
> 'correct'.
> 

I see. Yes it is a required property, so correct dts should have at 
least one channel. In that case I don't see a problem dropping these 
checks.

> > +	if (count < 0) {
> > +		dev_err(csi->dev, "Failed to get DMA channel count: %d\n",
> > +			count);
> > +		return count;
> > +	}
> > +
> > +	csi->num_ctx = count;
> > +	if (csi->num_ctx > TI_CSI2RX_MAX_CTX) {
> > +		dev_warn(csi->dev,
> > +			 "%u DMA channels passed. Maximum is %u. Ignoring the rest.\n",
> > +			 csi->num_ctx, TI_CSI2RX_MAX_CTX);
> > +		csi->num_ctx = TI_CSI2RX_MAX_CTX;
> > +	}
> > +
> >  	mutex_init(&csi->mutex);
> >
> >  	ret = ti_csi2rx_v4l2_init(csi);
> >  	if (ret)
> >  		goto err_v4l2;
> >
> > -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
> > +	for (i = 0; i < csi->num_ctx; i++) {
> >  		csi->ctx[i].idx = i;
> >  		csi->ctx[i].csi = csi;
> >  		ret = ti_csi2rx_init_ctx(&csi->ctx[i]);
> > @@ -1388,14 +1408,14 @@ static void ti_csi2rx_remove(struct platform_device *pdev)
> >  	struct ti_csi2rx_dev *csi = platform_get_drvdata(pdev);
> >  	int i;
> >
> > -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
> > +	for (i = 0; i < csi->num_ctx; i++) {
> >  		if (vb2_is_busy(&csi->ctx[i].vidq))
> >  			dev_err(csi->dev,
> >  				"Failed to remove as queue busy for ctx %u\n",
> >  				i);
> >  	}
> >
> > -	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++)
> > +	for (i = 0; i < csi->num_ctx; i++)
> >  		ti_csi2rx_cleanup_ctx(&csi->ctx[i]);
> >
> >  	ti_csi2rx_cleanup_notifier(csi);
> >
> > --
> > 2.43.0
> >
> >
diff mbox series

Patch

diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
index 13d7426ab4ba..b4b4bb69c88a 100644
--- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
+++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
@@ -40,7 +40,7 @@ 
 #define SHIM_PSI_CFG0_DST_TAG		GENMASK(31, 16)
 
 #define PSIL_WORD_SIZE_BYTES		16
-#define TI_CSI2RX_NUM_CTX		1
+#define TI_CSI2RX_MAX_CTX		32
 
 /*
  * There are no hard limits on the width or height. The DMA engine can handle
@@ -53,8 +53,8 @@ 
 
 #define TI_CSI2RX_PAD_SINK		0
 #define TI_CSI2RX_PAD_FIRST_SOURCE	1
-#define TI_CSI2RX_NUM_SOURCE_PADS	1
-#define TI_CSI2RX_NUM_PADS		(1 + TI_CSI2RX_NUM_SOURCE_PADS)
+#define TI_CSI2RX_MAX_SOURCE_PADS	TI_CSI2RX_MAX_CTX
+#define TI_CSI2RX_MAX_PADS		(1 + TI_CSI2RX_MAX_SOURCE_PADS)
 
 #define DRAIN_TIMEOUT_MS		50
 #define DRAIN_BUFFER_SIZE		SZ_32K
@@ -112,14 +112,15 @@  struct ti_csi2rx_dev {
 	void __iomem			*shim;
 	struct mutex			mutex; /* To serialize ioctls. */
 	unsigned int			enable_count;
+	unsigned int			num_ctx;
 	struct v4l2_device		v4l2_dev;
 	struct media_device		mdev;
 	struct media_pipeline		pipe;
-	struct media_pad		pads[TI_CSI2RX_NUM_PADS];
+	struct media_pad		pads[TI_CSI2RX_MAX_PADS];
 	struct v4l2_async_notifier	notifier;
 	struct v4l2_subdev		*source;
 	struct v4l2_subdev		subdev;
-	struct ti_csi2rx_ctx		ctx[TI_CSI2RX_NUM_CTX];
+	struct ti_csi2rx_ctx		ctx[TI_CSI2RX_MAX_CTX];
 	/* Buffer to drain stale data from PSI-L endpoint */
 	struct {
 		void			*vaddr;
@@ -449,7 +450,7 @@  static int csi_async_notifier_complete(struct v4l2_async_notifier *notifier)
 		return ret;
 
 	/* Create and link video nodes for all DMA contexts */
-	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
+	for (i = 0; i < csi->num_ctx; i++) {
 		struct ti_csi2rx_ctx *ctx = &csi->ctx[i];
 		struct video_device *vdev = &ctx->vdev;
 
@@ -1231,10 +1232,12 @@  static int ti_csi2rx_v4l2_init(struct ti_csi2rx_dev *csi)
 
 	csi->pads[TI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
 
-	for (i = TI_CSI2RX_PAD_FIRST_SOURCE; i < TI_CSI2RX_NUM_PADS; i++)
+	for (i = TI_CSI2RX_PAD_FIRST_SOURCE;
+	     i < TI_CSI2RX_PAD_FIRST_SOURCE + csi->num_ctx; i++)
 		csi->pads[i].flags = MEDIA_PAD_FL_SOURCE;
 
-	ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(csi->pads),
+	ret = media_entity_pads_init(&sd->entity,
+				     TI_CSI2RX_PAD_FIRST_SOURCE + csi->num_ctx,
 				     csi->pads);
 	if (ret)
 		goto unregister_media;
@@ -1320,8 +1323,9 @@  static int ti_csi2rx_init_ctx(struct ti_csi2rx_ctx *ctx)
 
 static int ti_csi2rx_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct ti_csi2rx_dev *csi;
-	int ret, i;
+	int ret, i, count;
 
 	csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);
 	if (!csi)
@@ -1343,13 +1347,29 @@  static int ti_csi2rx_probe(struct platform_device *pdev)
 	if (!csi->drain.vaddr)
 		return -ENOMEM;
 
+	/* Only use as many contexts as the number of DMA channels allocated. */
+	count = of_property_count_strings(np, "dma-names");
+	if (count < 0) {
+		dev_err(csi->dev, "Failed to get DMA channel count: %d\n",
+			count);
+		return count;
+	}
+
+	csi->num_ctx = count;
+	if (csi->num_ctx > TI_CSI2RX_MAX_CTX) {
+		dev_warn(csi->dev,
+			 "%u DMA channels passed. Maximum is %u. Ignoring the rest.\n",
+			 csi->num_ctx, TI_CSI2RX_MAX_CTX);
+		csi->num_ctx = TI_CSI2RX_MAX_CTX;
+	}
+
 	mutex_init(&csi->mutex);
 
 	ret = ti_csi2rx_v4l2_init(csi);
 	if (ret)
 		goto err_v4l2;
 
-	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
+	for (i = 0; i < csi->num_ctx; i++) {
 		csi->ctx[i].idx = i;
 		csi->ctx[i].csi = csi;
 		ret = ti_csi2rx_init_ctx(&csi->ctx[i]);
@@ -1388,14 +1408,14 @@  static void ti_csi2rx_remove(struct platform_device *pdev)
 	struct ti_csi2rx_dev *csi = platform_get_drvdata(pdev);
 	int i;
 
-	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++) {
+	for (i = 0; i < csi->num_ctx; i++) {
 		if (vb2_is_busy(&csi->ctx[i].vidq))
 			dev_err(csi->dev,
 				"Failed to remove as queue busy for ctx %u\n",
 				i);
 	}
 
-	for (i = 0; i < TI_CSI2RX_NUM_CTX; i++)
+	for (i = 0; i < csi->num_ctx; i++)
 		ti_csi2rx_cleanup_ctx(&csi->ctx[i]);
 
 	ti_csi2rx_cleanup_notifier(csi);