Re: [PATCH v2 09/13] media: ti: j721e-csi2rx: add support for processing virtual channels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/07/2024 18:01, Jacopo Mondi wrote:
Hi Jai

On Thu, Jun 27, 2024 at 06:40:04PM GMT, Jai Luthra wrote:
Use get_frame_desc() to get the frame desc from the connected source,
and use the provided virtual channel instead of hardcoded one.

get_frame_desc() works per stream, but as we don't support multiple
streams yet, we will just always use stream 0. If the source doesn't
support get_frame_desc(), fall back to the previous method of always
capturing virtual channel 0.

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

diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
index b4b4bb69c88a..c0916ca1a6f8 100644
--- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
+++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
@@ -29,6 +29,7 @@
  #define SHIM_DMACNTX_EN			BIT(31)
  #define SHIM_DMACNTX_YUV422		GENMASK(27, 26)
  #define SHIM_DMACNTX_SIZE		GENMASK(21, 20)
+#define SHIM_DMACNTX_VC			GENMASK(9, 6)
  #define SHIM_DMACNTX_FMT		GENMASK(5, 0)
  #define SHIM_DMACNTX_YUV422_MODE_11	3
  #define SHIM_DMACNTX_SIZE_8		0
@@ -105,6 +106,8 @@ struct ti_csi2rx_ctx {
  	struct media_pad		pad;
  	u32				sequence;
  	u32				idx;
+	u32				vc;
+	u32				stream;
  };

  struct ti_csi2rx_dev {
@@ -571,6 +574,7 @@ static void ti_csi2rx_setup_shim(struct ti_csi2rx_ctx *ctx)
  	}

  	reg |= FIELD_PREP(SHIM_DMACNTX_SIZE, fmt->size);
+	reg |= FIELD_PREP(SHIM_DMACNTX_VC, ctx->vc);

  	writel(reg, csi->shim + SHIM_DMACNTX(ctx->idx));

@@ -844,6 +848,33 @@ static void ti_csi2rx_buffer_queue(struct vb2_buffer *vb)
  	}
  }

+static int ti_csi2rx_get_vc(struct ti_csi2rx_ctx *ctx)
+{
+	struct ti_csi2rx_dev *csi = ctx->csi;
+	struct v4l2_mbus_frame_desc fd;
+	struct media_pad *pad;
+	int ret, i;
+
+	pad = media_entity_remote_pad_unique(&csi->subdev.entity, MEDIA_PAD_FL_SOURCE);
+	if (!pad)
+		return -ENODEV;
+
+	ret = v4l2_subdev_call(csi->source, pad, get_frame_desc, pad->index,
+			       &fd);
+	if (ret)
+		return ret;

Would it be better to fail at bound() time if the remote subdev
doesn't support get_frame_desc ? you can use

         if (!v4l2_subdev_has_op(subdev, pad, get_frame_desc)) {

+
+	if (fd.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
+		return -EINVAL;
+
+	for (i = 0; i < fd.num_entries; i++) {

         for (unsigned int i

should num_entries be validated ?

+		if (ctx->stream == fd.entry[i].stream)
+			return fd.entry[i].bus.csi2.vc;
+	}
+
+	return -ENODEV;
+}
+
  static int ti_csi2rx_start_streaming(struct vb2_queue *vq, unsigned int count)
  {
  	struct ti_csi2rx_ctx *ctx = vb2_get_drv_priv(vq);
@@ -864,6 +895,14 @@ static int ti_csi2rx_start_streaming(struct vb2_queue *vq, unsigned int count)
  	if (ret)
  		goto err;

+	ret = ti_csi2rx_get_vc(ctx);
+	if (ret == -ENOIOCTLCMD)
+		ctx->vc = 0;

Ah, so you fallback to 0 in case the subdev doesn't support
get_frame_desc. I'm not sure what would be better here maybe wait for
other's opinions as well.

Personally I would fail earlier to make sure subdev drivers are forced
to impement get_frame_desc

As this driver is already in upstream, with single-stream support, I think requiring frame_desc could break currently working setups.

I do a similar thing as above in CAL and RPi CFE drivers.

 Tomi





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux