1. Patchew
  2. linux
  3. [PATCH v2 0/6] media: ti, cdns: Multiple pixel support and misc fixes
  4. View patch

[PATCH v2 6/6] media: ti: j721e-csi2rx: Support multiple pixels per clock

Jai Luthra posted 6 patches 8 months, 1 week ago
There is a newer version of this series
[PATCH v2 6/6] media: ti: j721e-csi2rx: Support multiple pixels per clock
Posted by Jai Luthra 8 months, 1 week ago 
Add support for negotiating the highest possible pixel mode (from
single, dual, quad) with the Cadence CSI2RX bridge. This is required to
drain the Cadence stream FIFOs without overflowing when the source is
operating at a high link-frequency [1].

Also, update the Kconfig as this introduces a hard build-time dependency
on the Cadence CSI2RX driver, even for a COMPILE_TEST.

[1] Section 12.6.1.4.8.14 CSI_RX_IF Programming Restrictions of AM62 TRM

Link: https://www.ti.com/lit/pdf/spruj16
Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
---
 drivers/media/platform/ti/Kconfig                  |  3 +-
 .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  | 38 ++++++++++++++++++++--
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/ti/Kconfig b/drivers/media/platform/ti/Kconfig
index bab998c4179aca3b07372782b9be7de340cb8d45..3bc4aa35887e6edc9fa8749d9956a67714c59001 100644
--- a/drivers/media/platform/ti/Kconfig
+++ b/drivers/media/platform/ti/Kconfig
@@ -67,7 +67,8 @@ config VIDEO_TI_J721E_CSI2RX
 	tristate "TI J721E CSI2RX wrapper layer driver"
 	depends on VIDEO_DEV && VIDEO_V4L2_SUBDEV_API
 	depends on MEDIA_SUPPORT && MEDIA_CONTROLLER
-	depends on (PHY_CADENCE_DPHY_RX && VIDEO_CADENCE_CSI2RX) || COMPILE_TEST
+	depends on VIDEO_CADENCE_CSI2RX
+	depends on PHY_CADENCE_DPHY_RX || COMPILE_TEST
 	depends on ARCH_K3 || COMPILE_TEST
 	select VIDEOBUF2_DMA_CONTIG
 	select V4L2_FWNODE
diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
index ad51d033b6725426550578bdac1bae8443458f13..425324c3d6802cfda79d116d3967b61a2e7a015a 100644
--- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
+++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
@@ -21,6 +21,8 @@
 #include <media/v4l2-mc.h>
 #include <media/videobuf2-dma-contig.h>
 
+#include "../../cadence/cdns-csi2rx.h"
+
 #define TI_CSI2RX_MODULE_NAME		"j721e-csi2rx"
 
 #define SHIM_CNTL			0x10
@@ -29,6 +31,7 @@
 #define SHIM_DMACNTX			0x20
 #define SHIM_DMACNTX_EN			BIT(31)
 #define SHIM_DMACNTX_YUV422		GENMASK(27, 26)
+#define SHIM_DMACNTX_DUAL_PCK_CFG	BIT(24)
 #define SHIM_DMACNTX_SIZE		GENMASK(21, 20)
 #define SHIM_DMACNTX_FMT		GENMASK(5, 0)
 #define SHIM_DMACNTX_YUV422_MODE_11	3
@@ -40,6 +43,7 @@
 #define SHIM_PSI_CFG0_SRC_TAG		GENMASK(15, 0)
 #define SHIM_PSI_CFG0_DST_TAG		GENMASK(31, 16)
 
+#define TI_CSI2RX_MAX_PIX_PER_CLK	4
 #define PSIL_WORD_SIZE_BYTES		16
 /*
  * There are no hard limits on the width or height. The DMA engine can handle
@@ -110,6 +114,7 @@ struct ti_csi2rx_dev {
 	struct v4l2_format		v_fmt;
 	struct ti_csi2rx_dma		dma;
 	u32				sequence;
+	u8				pix_per_clk;
 };
 
 static const struct ti_csi2rx_fmt ti_csi2rx_formats[] = {
@@ -485,6 +490,26 @@ static int ti_csi2rx_notifier_register(struct ti_csi2rx_dev *csi)
 	return 0;
 }
 
+/* Request maximum possible pixels per clock from the bridge */
+static void ti_csi2rx_request_max_ppc(struct ti_csi2rx_dev *csi)
+{
+	struct media_pad *pad;
+	int ret;
+	u8 ppc = TI_CSI2RX_MAX_PIX_PER_CLK;
+
+	pad = media_entity_remote_source_pad_unique(&csi->vdev.entity);
+	if (!pad)
+		return;
+
+	ret = cdns_csi2rx_negotiate_ppc(csi->source, pad->index, &ppc);
+	if (ret) {
+		dev_warn(csi->dev, "NUM_PIXELS negotiation failed: %d\n", ret);
+		csi->pix_per_clk = 1;
+	} else {
+		csi->pix_per_clk = ppc;
+	}
+}
+
 static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
 {
 	const struct ti_csi2rx_fmt *fmt;
@@ -496,6 +521,9 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
 	reg = SHIM_CNTL_PIX_RST;
 	writel(reg, csi->shim + SHIM_CNTL);
 
+	/* Negotiate pixel count from the source */
+	ti_csi2rx_request_max_ppc(csi);
+
 	reg = SHIM_DMACNTX_EN;
 	reg |= FIELD_PREP(SHIM_DMACNTX_FMT, fmt->csi_dt);
 
@@ -524,14 +552,18 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
 	case V4L2_PIX_FMT_YVYU:
 		reg |= FIELD_PREP(SHIM_DMACNTX_YUV422,
 				  SHIM_DMACNTX_YUV422_MODE_11);
+		/* Multiple pixels are handled differently for packed YUV */
+		if (csi->pix_per_clk == 2)
+			reg |= SHIM_DMACNTX_DUAL_PCK_CFG;
+		reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
 		break;
 	default:
-		/* Ignore if not YUV 4:2:2 */
+		/* By default we change the shift size for multiple pixels */
+		reg |= FIELD_PREP(SHIM_DMACNTX_SIZE,
+				  fmt->size + (csi->pix_per_clk >> 1));
 		break;
 	}
 
-	reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
-
 	writel(reg, csi->shim + SHIM_DMACNTX);
 
 	reg = FIELD_PREP(SHIM_PSI_CFG0_SRC_TAG, 0) |

-- 
2.49.0
Re: [PATCH v2 6/6] media: ti: j721e-csi2rx: Support multiple pixels per clock
Posted by Sakari Ailus 5 months, 3 weeks ago 
Hi Jai,

On Thu, Apr 10, 2025 at 12:19:04PM +0530, Jai Luthra wrote:
> Add support for negotiating the highest possible pixel mode (from
> single, dual, quad) with the Cadence CSI2RX bridge. This is required to
> drain the Cadence stream FIFOs without overflowing when the source is
> operating at a high link-frequency [1].
> 
> Also, update the Kconfig as this introduces a hard build-time dependency
> on the Cadence CSI2RX driver, even for a COMPILE_TEST.
> 
> [1] Section 12.6.1.4.8.14 CSI_RX_IF Programming Restrictions of AM62 TRM
> 
> Link: https://www.ti.com/lit/pdf/spruj16
> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>

This creates a dependency between the two drivers.

Can you confirm that the TI device only exists in conjunction with the
cadence HW block?

> ---
>  drivers/media/platform/ti/Kconfig                  |  3 +-
>  .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  | 38 ++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/ti/Kconfig b/drivers/media/platform/ti/Kconfig
> index bab998c4179aca3b07372782b9be7de340cb8d45..3bc4aa35887e6edc9fa8749d9956a67714c59001 100644
> --- a/drivers/media/platform/ti/Kconfig
> +++ b/drivers/media/platform/ti/Kconfig
> @@ -67,7 +67,8 @@ config VIDEO_TI_J721E_CSI2RX
>  	tristate "TI J721E CSI2RX wrapper layer driver"
>  	depends on VIDEO_DEV && VIDEO_V4L2_SUBDEV_API
>  	depends on MEDIA_SUPPORT && MEDIA_CONTROLLER
> -	depends on (PHY_CADENCE_DPHY_RX && VIDEO_CADENCE_CSI2RX) || COMPILE_TEST
> +	depends on VIDEO_CADENCE_CSI2RX
> +	depends on PHY_CADENCE_DPHY_RX || COMPILE_TEST
>  	depends on ARCH_K3 || COMPILE_TEST
>  	select VIDEOBUF2_DMA_CONTIG
>  	select V4L2_FWNODE
> diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> index ad51d033b6725426550578bdac1bae8443458f13..425324c3d6802cfda79d116d3967b61a2e7a015a 100644
> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> @@ -21,6 +21,8 @@
>  #include <media/v4l2-mc.h>
>  #include <media/videobuf2-dma-contig.h>
>  
> +#include "../../cadence/cdns-csi2rx.h"
> +
>  #define TI_CSI2RX_MODULE_NAME		"j721e-csi2rx"
>  
>  #define SHIM_CNTL			0x10
> @@ -29,6 +31,7 @@
>  #define SHIM_DMACNTX			0x20
>  #define SHIM_DMACNTX_EN			BIT(31)
>  #define SHIM_DMACNTX_YUV422		GENMASK(27, 26)
> +#define SHIM_DMACNTX_DUAL_PCK_CFG	BIT(24)
>  #define SHIM_DMACNTX_SIZE		GENMASK(21, 20)
>  #define SHIM_DMACNTX_FMT		GENMASK(5, 0)
>  #define SHIM_DMACNTX_YUV422_MODE_11	3
> @@ -40,6 +43,7 @@
>  #define SHIM_PSI_CFG0_SRC_TAG		GENMASK(15, 0)
>  #define SHIM_PSI_CFG0_DST_TAG		GENMASK(31, 16)
>  
> +#define TI_CSI2RX_MAX_PIX_PER_CLK	4
>  #define PSIL_WORD_SIZE_BYTES		16
>  /*
>   * There are no hard limits on the width or height. The DMA engine can handle
> @@ -110,6 +114,7 @@ struct ti_csi2rx_dev {
>  	struct v4l2_format		v_fmt;
>  	struct ti_csi2rx_dma		dma;
>  	u32				sequence;
> +	u8				pix_per_clk;
>  };
>  
>  static const struct ti_csi2rx_fmt ti_csi2rx_formats[] = {
> @@ -485,6 +490,26 @@ static int ti_csi2rx_notifier_register(struct ti_csi2rx_dev *csi)
>  	return 0;
>  }
>  
> +/* Request maximum possible pixels per clock from the bridge */
> +static void ti_csi2rx_request_max_ppc(struct ti_csi2rx_dev *csi)
> +{
> +	struct media_pad *pad;
> +	int ret;
> +	u8 ppc = TI_CSI2RX_MAX_PIX_PER_CLK;

Could you make this look like a reverse Christmas tree?

> +
> +	pad = media_entity_remote_source_pad_unique(&csi->vdev.entity);
> +	if (!pad)
> +		return;
> +
> +	ret = cdns_csi2rx_negotiate_ppc(csi->source, pad->index, &ppc);
> +	if (ret) {
> +		dev_warn(csi->dev, "NUM_PIXELS negotiation failed: %d\n", ret);
> +		csi->pix_per_clk = 1;
> +	} else {
> +		csi->pix_per_clk = ppc;
> +	}
> +}
> +
>  static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
>  {
>  	const struct ti_csi2rx_fmt *fmt;
> @@ -496,6 +521,9 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
>  	reg = SHIM_CNTL_PIX_RST;
>  	writel(reg, csi->shim + SHIM_CNTL);
>  
> +	/* Negotiate pixel count from the source */
> +	ti_csi2rx_request_max_ppc(csi);
> +
>  	reg = SHIM_DMACNTX_EN;
>  	reg |= FIELD_PREP(SHIM_DMACNTX_FMT, fmt->csi_dt);
>  
> @@ -524,14 +552,18 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
>  	case V4L2_PIX_FMT_YVYU:
>  		reg |= FIELD_PREP(SHIM_DMACNTX_YUV422,
>  				  SHIM_DMACNTX_YUV422_MODE_11);
> +		/* Multiple pixels are handled differently for packed YUV */
> +		if (csi->pix_per_clk == 2)
> +			reg |= SHIM_DMACNTX_DUAL_PCK_CFG;
> +		reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
>  		break;
>  	default:
> -		/* Ignore if not YUV 4:2:2 */
> +		/* By default we change the shift size for multiple pixels */
> +		reg |= FIELD_PREP(SHIM_DMACNTX_SIZE,
> +				  fmt->size + (csi->pix_per_clk >> 1));
>  		break;
>  	}
>  
> -	reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
> -
>  	writel(reg, csi->shim + SHIM_DMACNTX);
>  
>  	reg = FIELD_PREP(SHIM_PSI_CFG0_SRC_TAG, 0) |
> 

-- 
Regards,

Sakari Ailus
Re: [PATCH v2 6/6] media: ti: j721e-csi2rx: Support multiple pixels per clock
Posted by Jai Luthra 5 months, 3 weeks ago 
Hi Sakari,

Quoting Sakari Ailus (2025-06-25 23:02:39)
> Hi Jai,
> 
> On Thu, Apr 10, 2025 at 12:19:04PM +0530, Jai Luthra wrote:
> > Add support for negotiating the highest possible pixel mode (from
> > single, dual, quad) with the Cadence CSI2RX bridge. This is required to
> > drain the Cadence stream FIFOs without overflowing when the source is
> > operating at a high link-frequency [1].
> > 
> > Also, update the Kconfig as this introduces a hard build-time dependency
> > on the Cadence CSI2RX driver, even for a COMPILE_TEST.
> > 
> > [1] Section 12.6.1.4.8.14 CSI_RX_IF Programming Restrictions of AM62 TRM
> > 
> > Link: https://www.ti.com/lit/pdf/spruj16
> > Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> 
> This creates a dependency between the two drivers.
> 
> Can you confirm that the TI device only exists in conjunction with the
> cadence HW block?
> 

Yes, the main job of TI device is to unwrap the Cadence-specific pixel
stream to something the TI K3 DMA engine can understand. So these are (and
will be) always paired together in all K3 SoCs that support CSI2RX.

> > ---
> >  drivers/media/platform/ti/Kconfig                  |  3 +-
> >  .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  | 38 ++++++++++++++++++++--
> >  2 files changed, 37 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/platform/ti/Kconfig b/drivers/media/platform/ti/Kconfig
> > index bab998c4179aca3b07372782b9be7de340cb8d45..3bc4aa35887e6edc9fa8749d9956a67714c59001 100644
> > --- a/drivers/media/platform/ti/Kconfig
> > +++ b/drivers/media/platform/ti/Kconfig
> > @@ -67,7 +67,8 @@ config VIDEO_TI_J721E_CSI2RX
> >       tristate "TI J721E CSI2RX wrapper layer driver"
> >       depends on VIDEO_DEV && VIDEO_V4L2_SUBDEV_API
> >       depends on MEDIA_SUPPORT && MEDIA_CONTROLLER
> > -     depends on (PHY_CADENCE_DPHY_RX && VIDEO_CADENCE_CSI2RX) || COMPILE_TEST
> > +     depends on VIDEO_CADENCE_CSI2RX
> > +     depends on PHY_CADENCE_DPHY_RX || COMPILE_TEST
> >       depends on ARCH_K3 || COMPILE_TEST
> >       select VIDEOBUF2_DMA_CONTIG
> >       select V4L2_FWNODE
> > diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> > index ad51d033b6725426550578bdac1bae8443458f13..425324c3d6802cfda79d116d3967b61a2e7a015a 100644
> > --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> > +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> > @@ -21,6 +21,8 @@
> >  #include <media/v4l2-mc.h>
> >  #include <media/videobuf2-dma-contig.h>
> >  
> > +#include "../../cadence/cdns-csi2rx.h"
> > +
> >  #define TI_CSI2RX_MODULE_NAME                "j721e-csi2rx"
> >  
> >  #define SHIM_CNTL                    0x10
> > @@ -29,6 +31,7 @@
> >  #define SHIM_DMACNTX                 0x20
> >  #define SHIM_DMACNTX_EN                      BIT(31)
> >  #define SHIM_DMACNTX_YUV422          GENMASK(27, 26)
> > +#define SHIM_DMACNTX_DUAL_PCK_CFG    BIT(24)
> >  #define SHIM_DMACNTX_SIZE            GENMASK(21, 20)
> >  #define SHIM_DMACNTX_FMT             GENMASK(5, 0)
> >  #define SHIM_DMACNTX_YUV422_MODE_11  3
> > @@ -40,6 +43,7 @@
> >  #define SHIM_PSI_CFG0_SRC_TAG                GENMASK(15, 0)
> >  #define SHIM_PSI_CFG0_DST_TAG                GENMASK(31, 16)
> >  
> > +#define TI_CSI2RX_MAX_PIX_PER_CLK    4
> >  #define PSIL_WORD_SIZE_BYTES         16
> >  /*
> >   * There are no hard limits on the width or height. The DMA engine can handle
> > @@ -110,6 +114,7 @@ struct ti_csi2rx_dev {
> >       struct v4l2_format              v_fmt;
> >       struct ti_csi2rx_dma            dma;
> >       u32                             sequence;
> > +     u8                              pix_per_clk;
> >  };
> >  
> >  static const struct ti_csi2rx_fmt ti_csi2rx_formats[] = {
> > @@ -485,6 +490,26 @@ static int ti_csi2rx_notifier_register(struct ti_csi2rx_dev *csi)
> >       return 0;
> >  }
> >  
> > +/* Request maximum possible pixels per clock from the bridge */
> > +static void ti_csi2rx_request_max_ppc(struct ti_csi2rx_dev *csi)
> > +{
> > +     struct media_pad *pad;
> > +     int ret;
> > +     u8 ppc = TI_CSI2RX_MAX_PIX_PER_CLK;
> 
> Could you make this look like a reverse Christmas tree?
> 

Will do.

> > +
> > +     pad = media_entity_remote_source_pad_unique(&csi->vdev.entity);
> > +     if (!pad)
> > +             return;
> > +
> > +     ret = cdns_csi2rx_negotiate_ppc(csi->source, pad->index, &ppc);
> > +     if (ret) {
> > +             dev_warn(csi->dev, "NUM_PIXELS negotiation failed: %d\n", ret);
> > +             csi->pix_per_clk = 1;
> > +     } else {
> > +             csi->pix_per_clk = ppc;
> > +     }
> > +}
> > +
> >  static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
> >  {
> >       const struct ti_csi2rx_fmt *fmt;
> > @@ -496,6 +521,9 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
> >       reg = SHIM_CNTL_PIX_RST;
> >       writel(reg, csi->shim + SHIM_CNTL);
> >  
> > +     /* Negotiate pixel count from the source */
> > +     ti_csi2rx_request_max_ppc(csi);
> > +
> >       reg = SHIM_DMACNTX_EN;
> >       reg |= FIELD_PREP(SHIM_DMACNTX_FMT, fmt->csi_dt);
> >  
> > @@ -524,14 +552,18 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_dev *csi)
> >       case V4L2_PIX_FMT_YVYU:
> >               reg |= FIELD_PREP(SHIM_DMACNTX_YUV422,
> >                                 SHIM_DMACNTX_YUV422_MODE_11);
> > +             /* Multiple pixels are handled differently for packed YUV */
> > +             if (csi->pix_per_clk == 2)
> > +                     reg |= SHIM_DMACNTX_DUAL_PCK_CFG;
> > +             reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
> >               break;
> >       default:
> > -             /* Ignore if not YUV 4:2:2 */
> > +             /* By default we change the shift size for multiple pixels */
> > +             reg |= FIELD_PREP(SHIM_DMACNTX_SIZE,
> > +                               fmt->size + (csi->pix_per_clk >> 1));
> >               break;
> >       }
> >  
> > -     reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
> > -
> >       writel(reg, csi->shim + SHIM_DMACNTX);
> >  
> >       reg = FIELD_PREP(SHIM_PSI_CFG0_SRC_TAG, 0) |
> > 
> 
> -- 
> Regards,
> 
> Sakari Ailus

Thanks,
Jai

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.