Hi Alexander,
Thank you for the patch.
On Tue, Apr 18, 2023 at 09:14:37AM +0200, Alexander Stein wrote:
> There is no need to convert input pixformat to mbus_framefmt and back
> again. Instead apply pixformat width contrains directly.
>
> Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/nxp/imx7-media-csi.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> index b701e823436a8..bd649fd9166fd 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -1145,8 +1145,8 @@ 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;
> + u32 stride;
>
> /*
> * Find the pixel format, default to the first supported format if not
> @@ -1172,12 +1172,16 @@ __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);
Could you please keep the comment from imx7_csi_mbus_fmt_to_pix_fmt()
here, to indicate where the alignment comes from ?
/* Round up width for minimum burst size */
We should likely revisit this in the future, I don't think the alignment
is actually needed. This could be recorded already:
/*
* 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);
> +
> + stride = round_up((pixfmt->width * cc->bpp) / 8, 8);
You can drop the round_up(), as pixfmt->width is now a multiple of 8, so
pixfmt->bytesperline = pixfmt->width * cc->bpp / 8;
> + pixfmt->bytesperline = stride;
> + pixfmt->sizeimage = stride * pixfmt->height;
>
> if (compose) {
> - compose->width = fmt_src.width;
> - compose->height = fmt_src.height;
> + compose->width = pixfmt->width;
This is a change of behaviour, compose->width used to be set to the
unaligned width.
> + compose->height = pixfmt->height;
> }
>
> return cc;
--
Regards,
Laurent Pinchart
