Re: [PATCH v5 01/14] media: ti: j721e-csi2rx: Remove word size alignment on frame width

On 05/09/25 16:37, Jai Luthra wrote:

Hi Rishikesh,

Hi Jai,

Thank you for the comments

Quoting Rishikesh Donadkar (2025-08-25 19:55:09)

quoted

j721e-csi2rx driver has a limitation of frame width being a multiple
word size. However, there is no such limitation imposed by the
hardware [1].

Is there no limitation for the step size, or also not limitation for the
minimum size of transfer?

I did not see any mention of restrictions on the step size and minimum 
size of transfer in the TRM and the CSI functional specifications.

quoted

Remove this limitation from the driver.

Link: https://www.ti.com/lit/pdf/spruj16
Signed-off-by: Rishikesh Donadkar <redacted>
---
  .../platform/ti/j721e-csi2rx/j721e-csi2rx.c     | 17 +++--------------
  1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
index 3992f8b754b7..b3a27f4c3210 100644
--- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
+++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c

@@ -260,9 +260,6 @@ static void ti_csi2rx_fill_fmt(const struct ti_csi2rx_fmt *csi_fmt,
                              MAX_WIDTH_BYTES * 8 / csi_fmt->bpp);

Here the pix->width is restricted to be at minimum pixels_in_word.
So TRY_FMT/S_FMT with a width = 1 will be clamped by the driver.

quoted

         pix->height = clamp_t(unsigned int, pix->height, 1, MAX_HEIGHT_LINES);
  
-       /* Width should be a multiple of transfer word-size */
-       pix->width = rounddown(pix->width, pixels_in_word);
-
         v4l2_fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
         pix->pixelformat = csi_fmt->fourcc;
         pix->bytesperline = pix->width * (csi_fmt->bpp / 8);

@@ -360,23 +357,15 @@ static int ti_csi2rx_enum_framesizes(struct file *file, void *fh,
                                      struct v4l2_frmsizeenum *fsize)
  {
         const struct ti_csi2rx_fmt *fmt;
-       unsigned int pixels_in_word;
  
         fmt = find_format_by_fourcc(fsize->pixel_format);
         if (!fmt || fsize->index != 0)
                 return -EINVAL;
  
-       /*
-        * Number of pixels in one PSI-L word. The transfer happens in multiples
-        * of PSI-L word sizes.
-        */
-       pixels_in_word = PSIL_WORD_SIZE_BYTES * 8 / fmt->bpp;
-
         fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
-       fsize->stepwise.min_width = pixels_in_word;
-       fsize->stepwise.max_width = rounddown(MAX_WIDTH_BYTES * 8 / fmt->bpp,
-                                             pixels_in_word);
-       fsize->stepwise.step_width = pixels_in_word;
+       fsize->stepwise.min_width = 1;

But here, in ENUM_FRAMESIZES we allow width to go as low as 1.

Can you make sure both of them match whatever is correct (and possible by
the hardware)?

Sure

quoted

+       fsize->stepwise.max_width = MAX_WIDTH_BYTES * 8 / fmt->bpp;
+       fsize->stepwise.step_width = 1;
         fsize->stepwise.min_height = 1;
         fsize->stepwise.max_height = MAX_HEIGHT_LINES;
         fsize->stepwise.step_height = 1;
-- 
2.34.1

Thanks,
     Jai

Regards,

Rishikesh