| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <175097004739.8144.1542268744363236258@freya>
Date: Thu, 26 Jun 2025 13:34:07 -0700
From: Jai Luthra <jai.luthra@...asonboard.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, Hans Verkuil <hverkuil@...all.nl>, Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, Maxime Ripard <mripard@...nel.org>, Devarsh Thakkar <devarsht@...com>, Rishikesh Donadkar <r-donadkar@...com>, Vaishnav Achath <vaishnav.a@...com>, Changhuang Liang <changhuang.liang@...rfivetech.com>, linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/6] media: ti: j721e-csi2rx: Support multiple pixels per clock
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@...asonboard.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
Powered by blists - more mailing lists