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

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

Hi,

On 11/09/2025 13:28, Rishikesh Donadkar wrote:
> 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].
> 
> Remove this limitation from the driver.
> 
> Link: https://www.ti.com/lit/pdf/spruj16
> Reviewed-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> Signed-off-by: Rishikesh Donadkar <r-donadkar@ti.com>
> ---
>  .../platform/ti/j721e-csi2rx/j721e-csi2rx.c   | 23 ++++---------------
>  1 file changed, 4 insertions(+), 19 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..6a981b5f5d51 100644
> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> @@ -250,19 +250,12 @@ static void ti_csi2rx_fill_fmt(const struct ti_csi2rx_fmt *csi_fmt,
>  			       struct v4l2_format *v4l2_fmt)
>  {
>  	struct v4l2_pix_format *pix = &v4l2_fmt->fmt.pix;
> -	unsigned int pixels_in_word;
> -
> -	pixels_in_word = PSIL_WORD_SIZE_BYTES * 8 / csi_fmt->bpp;

The define for PSIL_WORD_SIZE_BYTES can also be removed, if I'm not
mistaken.

I assume this has been tested, but I'm still a bit curious. Usually
these kind of lines of code don't just appear out of nowhere. You linked
to AM62Ax docs. None of the other SoCs have any limitations here either?

PSIL_WORD_SIZE_BYTES hints to a system DMA level limit, not CSI-2 RX
limit, so I wonder if some earlier SoCs had such limitations in the DMA?

 Tomi

>  
>  	/* Clamp width and height to sensible maximums (16K x 16K) */
>  	pix->width = clamp_t(unsigned int, pix->width,
> -			     pixels_in_word,
> -			     MAX_WIDTH_BYTES * 8 / csi_fmt->bpp);
> +			     1, MAX_WIDTH_BYTES * 8 / csi_fmt->bpp);
>  	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 +353,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;
> +	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;