| 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: <aFzif59lHvNz-p-0@kekkonen.localdomain>
Date: Thu, 26 Jun 2025 06:02:39 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Jai Luthra <jai.luthra@...asonboard.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 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?
> ---
> 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
Powered by blists - more mailing lists