| Message ID | 20240627-multistream-v2-7-6ae96c54c1c3@ti.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: cadence,ti: CSI2RX Multistream Support | expand |
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 > >
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 --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);