Re: [PATCH v3 1/4] media: imx: imx7-media-csi: Get rid of superfluous call to imx7_csi_mbus_fmt_to_pix_fmt

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Rui Miguel Silva <rmfrfs@gmail.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-media@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 1/4] media: imx: imx7-media-csi: Get rid of superfluous call to imx7_csi_mbus_fmt_to_pix_fmt
Date: Tue, 18 Apr 2023 18:35:18 +0300	[thread overview]
Message-ID: <20230418153518.GG30837@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230418122041.1318862-2-alexander.stein@ew.tq-group.com>

Hi Alexander,

Thank you for the patch.

On Tue, Apr 18, 2023 at 02:20:38PM +0200, Alexander Stein wrote:
> There is no need to convert input pixformat to mbus_framefmt and back
> again. Instead apply pixformat width constrains directly.
> Assign compose values before adjusting pixformat height/width.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Changes in v3:
> * Move compose assignment before width adjustments
> * Add comments regarding width multiples
> * Remove unneeded stride rounding
> 
>  drivers/media/platform/nxp/imx7-media-csi.c | 22 ++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> index b701e823436a8..b149374b07ee1 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -1145,9 +1145,13 @@ static const struct imx7_csi_pixfmt *
>  __imx7_csi_video_try_fmt(struct v4l2_pix_format *pixfmt,
>  			 struct v4l2_rect *compose)
>  {
> -	struct v4l2_mbus_framefmt fmt_src;
>  	const struct imx7_csi_pixfmt *cc;
>  
> +	if (compose) {
> +		compose->width = pixfmt->width;
> +		compose->height = pixfmt->height;
> +	}
> +
>  	/*
>  	 * Find the pixel format, default to the first supported format if not
>  	 * found.
> @@ -1172,13 +1176,17 @@ __imx7_csi_video_try_fmt(struct v4l2_pix_format *pixfmt,
>  		}
>  	}
>  
> -	v4l2_fill_mbus_format(&fmt_src, pixfmt, 0);
> -	imx7_csi_mbus_fmt_to_pix_fmt(pixfmt, &fmt_src, cc);
> +	/*
> +	 * Round up width for minimum burst size.
> +	 *
> +	 * TODO: Implement configurable stride support, and check what the real
> +	 * hardware alignment constraint on the width is.
> +	 */
> +	v4l_bound_align_image(&pixfmt->width, 1, 0xffff, 8,
> +			      &pixfmt->height, 1, 0xffff, 1, 0);
>  
> -	if (compose) {
> -		compose->width = fmt_src.width;
> -		compose->height = fmt_src.height;
> -	}
> +	pixfmt->bytesperline = pixfmt->width * cc->bpp / 8;
> +	pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
>  
>  	return cc;
>  }

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel