[v3,3/3] media: imx: lift CSI and PRP ENC/VF width alignment restriction

Message ID 20190111111053.12551-3-p.zabel@pengutronix.de (mailing list archive)
State Superseded, archived
Delegated to: Hans Verkuil
Headers

Commit Message

Philipp Zabel Jan. 11, 2019, 11:10 a.m. UTC
  The CSI, PRP ENC, and PRP VF subdevices shouldn't have to care about
IDMAC line start address alignment. With compose rectangle support in
the capture driver, they don't have to anymore.
If the direct CSI -> IC path is enabled, the CSI output width must
still be aligned to 8 pixels (IC burst length).

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v2:
 - drop csi_src_pad_enabled(), check active_output_pad instead
 - initialize active_output_pad and set it to CSI_SRC_PAD_IDMAC
   while source pad links are disabled
---
 drivers/staging/media/imx/imx-ic-prpencvf.c |  2 +-
 drivers/staging/media/imx/imx-media-csi.c   | 11 +++++++++--
 drivers/staging/media/imx/imx-media-utils.c | 15 ++++++++++++---
 3 files changed, 22 insertions(+), 6 deletions(-)
  

Comments

Steve Longerbeam Jan. 11, 2019, 6:13 p.m. UTC | #1
Reviewed-by: Steve Longerbeam <slongerbeam@gmail.com>

On 1/11/19 3:10 AM, Philipp Zabel wrote:
> The CSI, PRP ENC, and PRP VF subdevices shouldn't have to care about
> IDMAC line start address alignment. With compose rectangle support in
> the capture driver, they don't have to anymore.
> If the direct CSI -> IC path is enabled, the CSI output width must
> still be aligned to 8 pixels (IC burst length).
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v2:
>   - drop csi_src_pad_enabled(), check active_output_pad instead
>   - initialize active_output_pad and set it to CSI_SRC_PAD_IDMAC
>     while source pad links are disabled
> ---
>   drivers/staging/media/imx/imx-ic-prpencvf.c |  2 +-
>   drivers/staging/media/imx/imx-media-csi.c   | 11 +++++++++--
>   drivers/staging/media/imx/imx-media-utils.c | 15 ++++++++++++---
>   3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index fe5a77baa592..7bb754cb703e 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -48,7 +48,7 @@
>   
>   #define MAX_W_SRC  1024
>   #define MAX_H_SRC  1024
> -#define W_ALIGN_SRC   4 /* multiple of 16 pixels */
> +#define W_ALIGN_SRC   1 /* multiple of 2 pixels */
>   #define H_ALIGN_SRC   1 /* multiple of 2 lines */
>   
>   #define S_ALIGN       1 /* multiple of 2 */
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index c4523afe7b48..390beb61aa9b 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -41,7 +41,7 @@
>   #define MIN_H       144
>   #define MAX_W      4096
>   #define MAX_H      4096
> -#define W_ALIGN    4 /* multiple of 16 pixels */
> +#define W_ALIGN    1 /* multiple of 2 pixels */
>   #define H_ALIGN    1 /* multiple of 2 lines */
>   #define S_ALIGN    1 /* multiple of 2 */
>   
> @@ -1000,6 +1000,8 @@ static int csi_link_setup(struct media_entity *entity,
>   		v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
>   		v4l2_ctrl_handler_init(&priv->ctrl_hdlr, 0);
>   		priv->sink = NULL;
> +		/* do not apply IC burst alignment in csi_try_crop */
> +		priv->active_output_pad = CSI_SRC_PAD_IDMAC;
>   		goto out;
>   	}
>   
> @@ -1141,7 +1143,10 @@ static void csi_try_crop(struct csi_priv *priv,
>   		crop->left = infmt->width - crop->width;
>   	/* adjust crop left/width to h/w alignment restrictions */
>   	crop->left &= ~0x3;
> -	crop->width &= ~0x7;
> +	if (priv->active_output_pad == CSI_SRC_PAD_DIRECT)
> +		crop->width &= ~0x7; /* multiple of 8 pixels (IC burst) */
> +	else
> +		crop->width &= ~0x1; /* multiple of 2 pixels */
>   
>   	/*
>   	 * FIXME: not sure why yet, but on interlaced bt.656,
> @@ -1863,6 +1868,8 @@ static int imx_csi_probe(struct platform_device *pdev)
>   	priv->csi_id = pdata->csi;
>   	priv->smfc_id = (priv->csi_id == 0) ? 0 : 2;
>   
> +	priv->active_output_pad = CSI_SRC_PAD_IDMAC;
> +
>   	timer_setup(&priv->eof_timeout_timer, csi_idmac_eof_timeout, 0);
>   	spin_lock_init(&priv->irqlock);
>   
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 0eaa353d5cb3..5f110d90a4ef 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -580,6 +580,7 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>   				  struct v4l2_mbus_framefmt *mbus,
>   				  const struct imx_media_pixfmt *cc)
>   {
> +	u32 width;
>   	u32 stride;
>   
>   	if (!cc) {
> @@ -602,9 +603,16 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>   		cc = imx_media_find_mbus_format(code, CS_SEL_YUV, false);
>   	}
>   
> -	stride = cc->planar ? mbus->width : (mbus->width * cc->bpp) >> 3;
> +	/* Round up width for minimum burst size */
> +	width = round_up(mbus->width, 8);
>   
> -	pix->width = mbus->width;
> +	/* Round up stride for IDMAC line start address alignment */
> +	if (cc->planar)
> +		stride = round_up(width, 16);
> +	else
> +		stride = round_up((width * cc->bpp) >> 3, 8);
> +
> +	pix->width = width;
>   	pix->height = mbus->height;
>   	pix->pixelformat = cc->fourcc;
>   	pix->colorspace = mbus->colorspace;
> @@ -613,7 +621,8 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>   	pix->quantization = mbus->quantization;
>   	pix->field = mbus->field;
>   	pix->bytesperline = stride;
> -	pix->sizeimage = (pix->width * pix->height * cc->bpp) >> 3;
> +	pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
> +			 stride * pix->height;
>   
>   	return 0;
>   }
  

Patch

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index fe5a77baa592..7bb754cb703e 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -48,7 +48,7 @@ 
 
 #define MAX_W_SRC  1024
 #define MAX_H_SRC  1024
-#define W_ALIGN_SRC   4 /* multiple of 16 pixels */
+#define W_ALIGN_SRC   1 /* multiple of 2 pixels */
 #define H_ALIGN_SRC   1 /* multiple of 2 lines */
 
 #define S_ALIGN       1 /* multiple of 2 */
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index c4523afe7b48..390beb61aa9b 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -41,7 +41,7 @@ 
 #define MIN_H       144
 #define MAX_W      4096
 #define MAX_H      4096
-#define W_ALIGN    4 /* multiple of 16 pixels */
+#define W_ALIGN    1 /* multiple of 2 pixels */
 #define H_ALIGN    1 /* multiple of 2 lines */
 #define S_ALIGN    1 /* multiple of 2 */
 
@@ -1000,6 +1000,8 @@  static int csi_link_setup(struct media_entity *entity,
 		v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
 		v4l2_ctrl_handler_init(&priv->ctrl_hdlr, 0);
 		priv->sink = NULL;
+		/* do not apply IC burst alignment in csi_try_crop */
+		priv->active_output_pad = CSI_SRC_PAD_IDMAC;
 		goto out;
 	}
 
@@ -1141,7 +1143,10 @@  static void csi_try_crop(struct csi_priv *priv,
 		crop->left = infmt->width - crop->width;
 	/* adjust crop left/width to h/w alignment restrictions */
 	crop->left &= ~0x3;
-	crop->width &= ~0x7;
+	if (priv->active_output_pad == CSI_SRC_PAD_DIRECT)
+		crop->width &= ~0x7; /* multiple of 8 pixels (IC burst) */
+	else
+		crop->width &= ~0x1; /* multiple of 2 pixels */
 
 	/*
 	 * FIXME: not sure why yet, but on interlaced bt.656,
@@ -1863,6 +1868,8 @@  static int imx_csi_probe(struct platform_device *pdev)
 	priv->csi_id = pdata->csi;
 	priv->smfc_id = (priv->csi_id == 0) ? 0 : 2;
 
+	priv->active_output_pad = CSI_SRC_PAD_IDMAC;
+
 	timer_setup(&priv->eof_timeout_timer, csi_idmac_eof_timeout, 0);
 	spin_lock_init(&priv->irqlock);
 
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 0eaa353d5cb3..5f110d90a4ef 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -580,6 +580,7 @@  int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 				  struct v4l2_mbus_framefmt *mbus,
 				  const struct imx_media_pixfmt *cc)
 {
+	u32 width;
 	u32 stride;
 
 	if (!cc) {
@@ -602,9 +603,16 @@  int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 		cc = imx_media_find_mbus_format(code, CS_SEL_YUV, false);
 	}
 
-	stride = cc->planar ? mbus->width : (mbus->width * cc->bpp) >> 3;
+	/* Round up width for minimum burst size */
+	width = round_up(mbus->width, 8);
 
-	pix->width = mbus->width;
+	/* Round up stride for IDMAC line start address alignment */
+	if (cc->planar)
+		stride = round_up(width, 16);
+	else
+		stride = round_up((width * cc->bpp) >> 3, 8);
+
+	pix->width = width;
 	pix->height = mbus->height;
 	pix->pixelformat = cc->fourcc;
 	pix->colorspace = mbus->colorspace;
@@ -613,7 +621,8 @@  int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 	pix->quantization = mbus->quantization;
 	pix->field = mbus->field;
 	pix->bytesperline = stride;
-	pix->sizeimage = (pix->width * pix->height * cc->bpp) >> 3;
+	pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
+			 stride * pix->height;
 
 	return 0;
 }