All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: linux-media@vger.kernel.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.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-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] media: imx: imx7-media-csi: Init default format with __imx7_csi_video_try_fmt()
Date: Tue, 18 Apr 2023 14:20:11 +0200	[thread overview]
Message-ID: <2253651.irdbgypaU6@steina-w> (raw)
In-Reply-To: <20230418100417.20428-1-laurent.pinchart@ideasonboard.com>

Hi Laurent,

thanks for the nice cleanup.

Am Dienstag, 18. April 2023, 12:04:17 CEST schrieb Laurent Pinchart:
> Use the __imx7_csi_video_try_fmt() helper function to initialize the
> default format at probe time. This improves consistency by using the
> same code path for both default initialization and validation at
> runtime, and allows dropping the now unused imx7_csi_find_pixel_format()
> function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Hi Alexander,
> 
> This is an additional cleanup that applies on top of "[PATCH v2 0/3] Fix
> imx7-media-csi format settings". I've only compile-tested it as I'm
> currently lacking access to test hardware. Would you be able to test the
> patch ? If so, could you please include it in the v2 of your series ?

I can't detect any difference in 'media-ctl -p' right after boot, so I
assume the initialization is identical. LGTM, I'll include in v3 of my
series.

Thanks,
Alexander

> ---
>  drivers/media/platform/nxp/imx7-media-csi.c | 55 +++------------------
>  1 file changed, 6 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c
> b/drivers/media/platform/nxp/imx7-media-csi.c index
> 5240670476b2..e52d617eea59 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -1014,39 +1014,6 @@ static int imx7_csi_enum_mbus_formats(u32 *code, u32
> index) return -EINVAL;
>  }
> 
> -static int imx7_csi_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> -					const struct 
v4l2_mbus_framefmt *mbus,
> -					const struct imx7_csi_pixfmt 
*cc)
> -{
> -	u32 width;
> -	u32 stride;
> -
> -	if (!cc) {
> -		cc = imx7_csi_find_mbus_format(mbus->code);
> -		if (!cc)
> -			return -EINVAL;
> -	}
> -
> -	/* Round up width for minimum burst size */
> -	width = round_up(mbus->width, 8);
> -
> -	/* Round up stride for IDMAC line start address alignment */
> -	stride = round_up((width * cc->bpp) >> 3, 8);
> -
> -	pix->width = width;
> -	pix->height = mbus->height;
> -	pix->pixelformat = cc->fourcc;
> -	pix->colorspace = mbus->colorspace;
> -	pix->xfer_func = mbus->xfer_func;
> -	pix->ycbcr_enc = mbus->ycbcr_enc;
> -	pix->quantization = mbus->quantization;
> -	pix->field = mbus->field;
> -	pix->bytesperline = stride;
> -	pix->sizeimage = stride * pix->height;
> -
> -	return 0;
> -}
> -
>  /*
> ---------------------------------------------------------------------------
> -- * Video Capture Device - IOCTLs
>   */
> @@ -1618,22 +1585,14 @@ static struct imx7_csi_vb2_buffer
> *imx7_csi_video_next_buf(struct imx7_csi *csi) return buf;
>  }
> 
> -static int imx7_csi_video_init_format(struct imx7_csi *csi)
> +static void imx7_csi_video_init_format(struct imx7_csi *csi)
>  {
> -	struct v4l2_mbus_framefmt format = { };
> +	struct v4l2_pix_format *pixfmt = &csi->vdev_fmt;
> 
> -	format.code = IMX7_CSI_DEF_MBUS_CODE;
> -	format.width = IMX7_CSI_DEF_PIX_WIDTH;
> -	format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> -	format.field = V4L2_FIELD_NONE;
> +	pixfmt->width = IMX7_CSI_DEF_PIX_WIDTH;
> +	pixfmt->height = IMX7_CSI_DEF_PIX_HEIGHT;
> 
> -	imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> -	csi->vdev_compose.width = format.width;
> -	csi->vdev_compose.height = format.height;
> -
> -	csi->vdev_cc = imx7_csi_find_pixel_format(csi-
>vdev_fmt.pixelformat);
> -
> -	return 0;
> +	csi->vdev_cc = __imx7_csi_video_try_fmt(pixfmt, &csi->vdev_compose);
>  }
> 
>  static int imx7_csi_video_register(struct imx7_csi *csi)
> @@ -1646,9 +1605,7 @@ static int imx7_csi_video_register(struct imx7_csi
> *csi) vdev->v4l2_dev = v4l2_dev;
> 
>  	/* Initialize the default format and compose rectangle. */
> -	ret = imx7_csi_video_init_format(csi);
> -	if (ret < 0)
> -		return ret;
> +	imx7_csi_video_init_format(csi);
> 
>  	/* Register the video device. */
>  	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



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

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: linux-media@vger.kernel.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.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-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] media: imx: imx7-media-csi: Init default format with __imx7_csi_video_try_fmt()
Date: Tue, 18 Apr 2023 14:20:11 +0200	[thread overview]
Message-ID: <2253651.irdbgypaU6@steina-w> (raw)
In-Reply-To: <20230418100417.20428-1-laurent.pinchart@ideasonboard.com>

Hi Laurent,

thanks for the nice cleanup.

Am Dienstag, 18. April 2023, 12:04:17 CEST schrieb Laurent Pinchart:
> Use the __imx7_csi_video_try_fmt() helper function to initialize the
> default format at probe time. This improves consistency by using the
> same code path for both default initialization and validation at
> runtime, and allows dropping the now unused imx7_csi_find_pixel_format()
> function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Hi Alexander,
> 
> This is an additional cleanup that applies on top of "[PATCH v2 0/3] Fix
> imx7-media-csi format settings". I've only compile-tested it as I'm
> currently lacking access to test hardware. Would you be able to test the
> patch ? If so, could you please include it in the v2 of your series ?

I can't detect any difference in 'media-ctl -p' right after boot, so I
assume the initialization is identical. LGTM, I'll include in v3 of my
series.

Thanks,
Alexander

> ---
>  drivers/media/platform/nxp/imx7-media-csi.c | 55 +++------------------
>  1 file changed, 6 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c
> b/drivers/media/platform/nxp/imx7-media-csi.c index
> 5240670476b2..e52d617eea59 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -1014,39 +1014,6 @@ static int imx7_csi_enum_mbus_formats(u32 *code, u32
> index) return -EINVAL;
>  }
> 
> -static int imx7_csi_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> -					const struct 
v4l2_mbus_framefmt *mbus,
> -					const struct imx7_csi_pixfmt 
*cc)
> -{
> -	u32 width;
> -	u32 stride;
> -
> -	if (!cc) {
> -		cc = imx7_csi_find_mbus_format(mbus->code);
> -		if (!cc)
> -			return -EINVAL;
> -	}
> -
> -	/* Round up width for minimum burst size */
> -	width = round_up(mbus->width, 8);
> -
> -	/* Round up stride for IDMAC line start address alignment */
> -	stride = round_up((width * cc->bpp) >> 3, 8);
> -
> -	pix->width = width;
> -	pix->height = mbus->height;
> -	pix->pixelformat = cc->fourcc;
> -	pix->colorspace = mbus->colorspace;
> -	pix->xfer_func = mbus->xfer_func;
> -	pix->ycbcr_enc = mbus->ycbcr_enc;
> -	pix->quantization = mbus->quantization;
> -	pix->field = mbus->field;
> -	pix->bytesperline = stride;
> -	pix->sizeimage = stride * pix->height;
> -
> -	return 0;
> -}
> -
>  /*
> ---------------------------------------------------------------------------
> -- * Video Capture Device - IOCTLs
>   */
> @@ -1618,22 +1585,14 @@ static struct imx7_csi_vb2_buffer
> *imx7_csi_video_next_buf(struct imx7_csi *csi) return buf;
>  }
> 
> -static int imx7_csi_video_init_format(struct imx7_csi *csi)
> +static void imx7_csi_video_init_format(struct imx7_csi *csi)
>  {
> -	struct v4l2_mbus_framefmt format = { };
> +	struct v4l2_pix_format *pixfmt = &csi->vdev_fmt;
> 
> -	format.code = IMX7_CSI_DEF_MBUS_CODE;
> -	format.width = IMX7_CSI_DEF_PIX_WIDTH;
> -	format.height = IMX7_CSI_DEF_PIX_HEIGHT;
> -	format.field = V4L2_FIELD_NONE;
> +	pixfmt->width = IMX7_CSI_DEF_PIX_WIDTH;
> +	pixfmt->height = IMX7_CSI_DEF_PIX_HEIGHT;
> 
> -	imx7_csi_mbus_fmt_to_pix_fmt(&csi->vdev_fmt, &format, NULL);
> -	csi->vdev_compose.width = format.width;
> -	csi->vdev_compose.height = format.height;
> -
> -	csi->vdev_cc = imx7_csi_find_pixel_format(csi-
>vdev_fmt.pixelformat);
> -
> -	return 0;
> +	csi->vdev_cc = __imx7_csi_video_try_fmt(pixfmt, &csi->vdev_compose);
>  }
> 
>  static int imx7_csi_video_register(struct imx7_csi *csi)
> @@ -1646,9 +1605,7 @@ static int imx7_csi_video_register(struct imx7_csi
> *csi) vdev->v4l2_dev = v4l2_dev;
> 
>  	/* Initialize the default format and compose rectangle. */
> -	ret = imx7_csi_video_init_format(csi);
> -	if (ret < 0)
> -		return ret;
> +	imx7_csi_video_init_format(csi);
> 
>  	/* Register the video device. */
>  	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



  reply	other threads:[~2023-04-18 12:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-18  7:14 [PATCH v2 0/3] Fix imx7-media-csi format settings Alexander Stein
2023-04-18  7:14 ` Alexander Stein
2023-04-18  7:14 ` [PATCH v2 1/3] media: imx: imx7-media-csi: Get rid of superfluous call to imx7_csi_mbus_fmt_to_pix_fmt Alexander Stein
2023-04-18  7:14   ` Alexander Stein
2023-04-18  9:27   ` Laurent Pinchart
2023-04-18  9:27     ` Laurent Pinchart
2023-04-18 10:00     ` Alexander Stein
2023-04-18 10:00       ` Alexander Stein
2023-04-18 10:05       ` Laurent Pinchart
2023-04-18 10:05         ` Laurent Pinchart
2023-04-18 10:08         ` Alexander Stein
2023-04-18 10:08           ` Alexander Stein
2023-04-18 10:18           ` Laurent Pinchart
2023-04-18 10:18             ` Laurent Pinchart
2023-04-18  7:14 ` [PATCH v2 2/3] media: imx: imx7-media-csi: Remove interlave fields Alexander Stein
2023-04-18  7:14   ` Alexander Stein
2023-04-18  9:34   ` Laurent Pinchart
2023-04-18  9:34     ` Laurent Pinchart
2023-04-18  7:14 ` [PATCH v2 3/3] media: imx: imx7-media-csi: Lift width constraints for 8bpp formats Alexander Stein
2023-04-18  7:14   ` Alexander Stein
2023-04-18  9:37   ` Laurent Pinchart
2023-04-18  9:37     ` Laurent Pinchart
2023-04-18 10:04 ` [PATCH] media: imx: imx7-media-csi: Init default format with __imx7_csi_video_try_fmt() Laurent Pinchart
2023-04-18 10:04   ` Laurent Pinchart
2023-04-18 12:20   ` Alexander Stein [this message]
2023-04-18 12:20     ` Alexander Stein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2253651.irdbgypaU6@steina-w \
    --to=alexander.stein@ew.tq-group.com \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rmfrfs@gmail.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.