[PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types

[PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types

[Steve Longerbeam]

The function ipu_csi_init_interface() was inverting the F-bit for
NTSC case, in the CCIR_CODE_1/2 registers. The result being that
for NTSC bottom-top field order, the CSI would swap fields and
capture in top-bottom order.

Instead, base field swap on the field order of the input to the CSI,
and the field order of the requested output. If the input/output
fields are sequential but different, swap fields, otherwise do
not swap. This requires passing both the input and output mbus
frame formats to ipu_csi_init_interface().

Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
possibly interlaced BT.1120 in the future).

When detecting input video standard from the input frame width/height,
make sure to double height if input field type is alternate, since
in that case input height only includes lines for one field.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/gpu/ipu-v3/ipu-csi.c              | 132 +++++++++++++++-------
 drivers/staging/media/imx/imx-media-csi.c |  13 +--
 include/video/imx-ipu-v3.h                |   3 +-
 3 files changed, 97 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index 954eefe144e2..759fcd724ff9 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
 	return 0;
 }
 
+/* translate alternate field mode based on given standard */
+static inline enum v4l2_field
+ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
+{
+	return (field != V4L2_FIELD_ALTERNATE) ? field :
+		((std & V4L2_STD_525_60) ?
+		 V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
+}
+
 /*
  * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
  */
@@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
 	return 0;
 }
 
+static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
+					   struct v4l2_mbus_framefmt *infmt,
+					   struct v4l2_mbus_framefmt *outfmt,
+					   v4l2_std_id std)
+{
+	enum v4l2_field infield, outfield;
+	bool swap_fields;
+
+	/* get translated field type of input and output */
+	infield = ipu_csi_translate_field(infmt->field, std);
+	outfield = ipu_csi_translate_field(outfmt->field, std);
+
+	/*
+	 * Write the H-V-F codes the CSI will match against the
+	 * incoming data for start/end of active and blanking
+	 * field intervals. If input and output field types are
+	 * sequential but not the same (one is SEQ_BT and the other
+	 * is SEQ_TB), swap the F-bit so that the CSI will capture
+	 * field 1 lines before field 0 lines.
+	 */
+	swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
+		       V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
+		       infield != outfield);
+
+	if (!swap_fields) {
+		/*
+		 * Field0BlankEnd  = 110, Field0BlankStart  = 010
+		 * Field0ActiveEnd = 100, Field0ActiveStart = 000
+		 * Field1BlankEnd  = 111, Field1BlankStart  = 011
+		 * Field1ActiveEnd = 101, Field1ActiveStart = 001
+		 */
+		ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+			      CSI_CCIR_CODE_1);
+		ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+	} else {
+		dev_dbg(csi->ipu->dev, "capture field swap\n");
+
+		/* same as above but with F-bit inverted */
+		ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
+			      CSI_CCIR_CODE_1);
+		ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
+	}
+
+	ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+
+	return 0;
+}
+
+
 int ipu_csi_init_interface(struct ipu_csi *csi,
 			   struct v4l2_mbus_config *mbus_cfg,
-			   struct v4l2_mbus_framefmt *mbus_fmt)
+			   struct v4l2_mbus_framefmt *infmt,
+			   struct v4l2_mbus_framefmt *outfmt)
 {
 	struct ipu_csi_bus_config cfg;
 	unsigned long flags;
 	u32 width, height, data = 0;
+	v4l2_std_id std;
 	int ret;
 
-	ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
+	ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
 	if (ret < 0)
 		return ret;
 
 	/* set default sensor frame width and height */
-	width = mbus_fmt->width;
-	height = mbus_fmt->height;
+	width = infmt->width;
+	height = infmt->height;
+	if (infmt->field = V4L2_FIELD_ALTERNATE)
+		height *= 2;
 
 	/* Set the CSI_SENS_CONF register remaining fields */
 	data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
@@ -416,42 +478,33 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
 		ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
 		break;
 	case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
-		if (mbus_fmt->width = 720 && mbus_fmt->height = 576) {
-			/*
-			 * PAL case
-			 *
-			 * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
-			 * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
-			 * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
-			 * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
-			 */
-			height = 625; /* framelines for PAL */
-
-			ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
-					  CSI_CCIR_CODE_1);
-			ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
-			ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
-		} else if (mbus_fmt->width = 720 && mbus_fmt->height = 480) {
-			/*
-			 * NTSC case
-			 *
-			 * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
-			 * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
-			 * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
-			 * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
-			 */
-			height = 525; /* framelines for NTSC */
-
-			ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
-					  CSI_CCIR_CODE_1);
-			ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
-			ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
-		} else {
+		std = V4L2_STD_UNKNOWN;
+		if (width = 720) {
+			switch (height) {
+			case 480:
+				std = V4L2_STD_NTSC;
+				break;
+			case 576:
+				std = V4L2_STD_PAL;
+				break;
+			default:
+				break;
+			}
+		}
+
+		if (std = V4L2_STD_UNKNOWN) {
 			dev_err(csi->ipu->dev,
-				"Unsupported CCIR656 interlaced video mode\n");
-			spin_unlock_irqrestore(&csi->lock, flags);
-			return -EINVAL;
+				"Unsupported interlaced video mode\n");
+			ret = -EINVAL;
+			goto out_unlock;
 		}
+
+		ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
+		if (ret)
+			goto out_unlock;
+
+		/* framelines for NTSC / PAL */
+		height = (std & V4L2_STD_525_60) ? 525 : 625;
 		break;
 	case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
 	case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
@@ -476,9 +529,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
 	dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
 		ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
 
+out_unlock:
 	spin_unlock_irqrestore(&csi->lock, flags);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
 
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 4acdd7ae612b..ad66f007d395 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -666,7 +666,6 @@ static int csi_setup(struct csi_priv *priv)
 	struct v4l2_mbus_framefmt *infmt, *outfmt;
 	const struct imx_media_pixfmt *incc;
 	struct v4l2_mbus_config mbus_cfg;
-	struct v4l2_mbus_framefmt if_fmt;
 	struct v4l2_rect crop;
 
 	infmt = &priv->format_mbus[CSI_SINK_PAD];
@@ -679,22 +678,14 @@ static int csi_setup(struct csi_priv *priv)
 		priv->upstream_ep.bus.parallel.flags :
 		priv->upstream_ep.bus.mipi_csi2.flags;
 
-	/*
-	 * we need to pass input frame to CSI interface, but
-	 * with translated field type from output format
-	 */
-	if_fmt = *infmt;
-	if_fmt.field = outfmt->field;
 	crop = priv->crop;
 
 	/*
 	 * if cycles is set, we need to handle this over multiple cycles as
 	 * generic/bayer data
 	 */
-	if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
-		if_fmt.width *= incc->cycles;
+	if (is_parallel_bus(&priv->upstream_ep) && incc->cycles)
 		crop.width *= incc->cycles;
-	}
 
 	ipu_csi_set_window(priv->csi, &crop);
 
@@ -702,7 +693,7 @@ static int csi_setup(struct csi_priv *priv)
 			     priv->crop.width = 2 * priv->compose.width,
 			     priv->crop.height = 2 * priv->compose.height);
 
-	ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
+	ipu_csi_init_interface(priv->csi, &mbus_cfg, infmt, outfmt);
 
 	ipu_csi_set_dest(priv->csi, priv->dest);
 
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index abbad94e14a1..f44a35192313 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
 struct ipu_csi;
 int ipu_csi_init_interface(struct ipu_csi *csi,
 			   struct v4l2_mbus_config *mbus_cfg,
-			   struct v4l2_mbus_framefmt *mbus_fmt);
+			   struct v4l2_mbus_framefmt *infmt,
+			   struct v4l2_mbus_framefmt *outfmt);
 bool ipu_csi_is_interlaced(struct ipu_csi *csi);
 void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
 void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
-- 
2.17.1

[PATCH v4 03/11] gpu: ipu-v3: Add planar support to interlaced scan

[Steve Longerbeam]

To support interlaced scan with planar formats, cpmem SLUV must
be programmed with the correct chroma line stride. For full and
partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
chroma line stride must _not_ be doubled, since a single chroma line
is shared by two luma lines.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/gpu/ipu-v3/ipu-cpmem.c              | 26 +++++++++++++++++++--
 drivers/staging/media/imx/imx-ic-prpencvf.c |  3 ++-
 drivers/staging/media/imx/imx-media-csi.c   |  3 ++-
 include/video/imx-ipu-v3.h                  |  3 ++-
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a9d2501500a1..d41df8034c5b 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off)
 }
 EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
 
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+			       u32 pixelformat)
 {
-	u32 ilo, sly;
+	u32 ilo, sly, sluv;
 
 	if (stride < 0) {
 		stride = -stride;
@@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
 
 	sly = (stride * 2) - 1;
 
+	switch (pixelformat) {
+	case V4L2_PIX_FMT_YUV420:
+	case V4L2_PIX_FMT_YVU420:
+		sluv = stride / 2 - 1;
+		break;
+	case V4L2_PIX_FMT_NV12:
+		sluv = stride - 1;
+		break;
+	case V4L2_PIX_FMT_YUV422P:
+		sluv = stride - 1;
+		break;
+	case V4L2_PIX_FMT_NV16:
+		sluv = stride * 2 - 1;
+		break;
+	default:
+		sluv = 0;
+		break;
+	}
+
 	ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
 	ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
 	ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
+	if (sluv)
+		ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
 };
 EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
 
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..af7224846bd5 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -412,7 +412,8 @@ static int prp_setup_channel(struct prp_priv *priv,
 	if (image.pix.field = V4L2_FIELD_NONE &&
 	    V4L2_FIELD_HAS_BOTH(infmt->field) &&
 	    channel = priv->out_ch)
-		ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
+		ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+					  image.pix.pixelformat);
 
 	ret = ipu_ic_task_idma_init(priv->ic, channel,
 				    image.pix.width, image.pix.height,
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index ad66f007d395..5e3aa4f3a1dd 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -512,7 +512,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 	if (image.pix.field = V4L2_FIELD_NONE &&
 	    V4L2_FIELD_HAS_BOTH(infmt->field))
 		ipu_cpmem_interlaced_scan(priv->idmac_ch,
-					  image.pix.bytesperline);
+					  image.pix.bytesperline,
+					  image.pix.pixelformat);
 
 	ipu_idmac_set_double_buffer(priv->idmac_ch, true);
 
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index f44a35192313..e888c66b9d9d 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -255,7 +255,8 @@ void ipu_cpmem_set_stride(struct ipuv3_channel *ch, int stride);
 void ipu_cpmem_set_high_priority(struct ipuv3_channel *ch);
 void ipu_cpmem_set_buffer(struct ipuv3_channel *ch, int bufnum, dma_addr_t buf);
 void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride);
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+			       u32 pixelformat);
 void ipu_cpmem_set_axi_id(struct ipuv3_channel *ch, u32 id);
 int ipu_cpmem_get_burstsize(struct ipuv3_channel *ch);
 void ipu_cpmem_set_burstsize(struct ipuv3_channel *ch, int burstsize);
-- 
2.17.1

Re: [PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types

[Philipp Zabel]

Hi Steve,

On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote:
[...]
>  int ipu_csi_init_interface(struct ipu_csi *csi,
>  			   struct v4l2_mbus_config *mbus_cfg,
> -			   struct v4l2_mbus_framefmt *mbus_fmt)
> +			   struct v4l2_mbus_framefmt *infmt,
> +			   struct v4l2_mbus_framefmt *outfmt)
>  {
[...] 
> +		std = V4L2_STD_UNKNOWN;
> +		if (width = 720) {
> +			switch (height) {
> +			case 480:
> +				std = V4L2_STD_NTSC;
> +				break;
> +			case 576:
> +				std = V4L2_STD_PAL;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +
> +		if (std = V4L2_STD_UNKNOWN) {
>  			dev_err(csi->ipu->dev,
[...]
> +				"Unsupported interlaced video mode\n");
> +			ret = -EINVAL;
> +			goto out_unlock;
>  		}
[...]
> +
> +		/* framelines for NTSC / PAL */
> +		height = (std & V4L2_STD_525_60) ? 525 : 625;

I think this is a bit convoluted. Instead of initializing std, then
possibly changing it, and then comparing to the inital value, and then
checking it again to determine the new height, why not just:

		if (width = 720 && height = 480) {
			std = V4L2_STD_NTSC;
			height = 525;
		} else if (width = 720 && height = 576) {
			std = V4L2_STD_PAL;
			height = 625;
		} else {
			dev_err(csi->ipu->dev,
				"Unsupported interlaced video mode\n");
			ret = -EINVAL;
			goto out_unlock;
		}

?

>  		break;
>  	case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
>  	case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +529,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>  	dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
>  		ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>  
> +out_unlock:
>  	spin_unlock_irqrestore(&csi->lock, flags);
>  
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>  
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4acdd7ae612b..ad66f007d395 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -666,7 +666,6 @@ static int csi_setup(struct csi_priv *priv)
>  	struct v4l2_mbus_framefmt *infmt, *outfmt;
>  	const struct imx_media_pixfmt *incc;
>  	struct v4l2_mbus_config mbus_cfg;
> -	struct v4l2_mbus_framefmt if_fmt;
>  	struct v4l2_rect crop;
>  
>  	infmt = &priv->format_mbus[CSI_SINK_PAD];
> @@ -679,22 +678,14 @@ static int csi_setup(struct csi_priv *priv)
>  		priv->upstream_ep.bus.parallel.flags :
>  		priv->upstream_ep.bus.mipi_csi2.flags;
>  
> -	/*
> -	 * we need to pass input frame to CSI interface, but
> -	 * with translated field type from output format
> -	 */
> -	if_fmt = *infmt;
> -	if_fmt.field = outfmt->field;
>  	crop = priv->crop;
>  
>  	/*
>  	 * if cycles is set, we need to handle this over multiple cycles as
>  	 * generic/bayer data
>  	 */
> -	if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> -		if_fmt.width *= incc->cycles;

If the input format width passed to ipu_csi_init_interface is not
multiplied by the number of cycles per pixel anymore, width in the
CSI_SENS_FRM_SIZE register will be set to the unmultiplied value from
infmt.
This breaks 779680e2e793 ("media: imx: add support for RGB565_2X8 on
parallel bus").

> +	if (is_parallel_bus(&priv->upstream_ep) && incc->cycles)
>  		crop.width *= incc->cycles;
> -	}
>  
>  	ipu_csi_set_window(priv->csi, &crop);
>  
> @@ -702,7 +693,7 @@ static int csi_setup(struct csi_priv *priv)
>  			     priv->crop.width = 2 * priv->compose.width,
>  			     priv->crop.height = 2 * priv->compose.height);
>  
> -	ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> +	ipu_csi_init_interface(priv->csi, &mbus_cfg, infmt, outfmt);

regards
Philipp

Re: [PATCH v4 03/11] gpu: ipu-v3: Add planar support to interlaced scan

[Philipp Zabel]

On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote:
> To support interlaced scan with planar formats, cpmem SLUV must
> be programmed with the correct chroma line stride. For full and
> partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
> be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
> chroma line stride must _not_ be doubled, since a single chroma line
> is shared by two luma lines.
> 
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/gpu/ipu-v3/ipu-cpmem.c              | 26 +++++++++++++++++++--
>  drivers/staging/media/imx/imx-ic-prpencvf.c |  3 ++-
>  drivers/staging/media/imx/imx-media-csi.c   |  3 ++-
>  include/video/imx-ipu-v3.h                  |  3 ++-
>  4 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
> index a9d2501500a1..d41df8034c5b 100644
> --- a/drivers/gpu/ipu-v3/ipu-cpmem.c
> +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
> @@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off)
>  }
>  EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
>  
> -void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
> +void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
> +			       u32 pixelformat)
>  {
> -	u32 ilo, sly;
> +	u32 ilo, sly, sluv;
>  
>  	if (stride < 0) {
>  		stride = -stride;
> @@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
>  
>  	sly = (stride * 2) - 1;
>  
> +	switch (pixelformat) {
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YVU420:
> +		sluv = stride / 2 - 1;
> +		break;
> +	case V4L2_PIX_FMT_NV12:
> +		sluv = stride - 1;
> +		break;
> +	case V4L2_PIX_FMT_YUV422P:
> +		sluv = stride - 1;
> +		break;
> +	case V4L2_PIX_FMT_NV16:
> +		sluv = stride * 2 - 1;
> +		break;
> +	default:
> +		sluv = 0;
> +		break;
> +	}
> +
>  	ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
>  	ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
>  	ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
> +	if (sluv)
> +		ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
>  };
>  EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
[...]

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

and

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

to be merged with the rest of the series via the media tree. I'll take
care not to introduce nontrivial conflicts in imx-drm.

regards
Philipp

Re: [PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types

[Steve Longerbeam]

Hi Philipp,


On 10/05/2018 02:44 AM, Philipp Zabel wrote:
> Hi Steve,
>
> On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote:
>
>
>> +
>> +		/* framelines for NTSC / PAL */
>> +		height = (std & V4L2_STD_525_60) ? 525 : 625;
> I think this is a bit convoluted. Instead of initializing std, then
> possibly changing it, and then comparing to the inital value, and then
> checking it again to determine the new height, why not just:
>
> 		if (width = 720 && height = 480) {
> 			std = V4L2_STD_NTSC;
> 			height = 525;
> 		} else if (width = 720 && height = 576) {
> 			std = V4L2_STD_PAL;
> 			height = 625;
> 		} else {
> 			dev_err(csi->ipu->dev,
> 				"Unsupported interlaced video mode\n");
> 			ret = -EINVAL;
> 			goto out_unlock;
> 		}
>
> ?

Yes that was a bit convoluted, fixed.

>
>>   
>>   	/*
>>   	 * if cycles is set, we need to handle this over multiple cycles as
>>   	 * generic/bayer data
>>   	 */
>> -	if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
>> -		if_fmt.width *= incc->cycles;
> If the input format width passed to ipu_csi_init_interface is not
> multiplied by the number of cycles per pixel anymore, width in the
> CSI_SENS_FRM_SIZE register will be set to the unmultiplied value from
> infmt.
> This breaks 779680e2e793 ("media: imx: add support for RGB565_2X8 on
> parallel bus").

Oops, that was a mistake, thanks for catching, fixed.

Steve

Re: [PATCH v4 03/11] gpu: ipu-v3: Add planar support to interlaced scan

[Steve Longerbeam]

On 10/05/2018 02:48 AM, Philipp Zabel wrote:
> On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote:
>> To support interlaced scan with planar formats, cpmem SLUV must
>> be programmed with the correct chroma line stride. For full and
>> partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
>> be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
>> chroma line stride must _not_ be doubled, since a single chroma line
>> is shared by two luma lines.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>>   drivers/gpu/ipu-v3/ipu-cpmem.c              | 26 +++++++++++++++++++--
>>   drivers/staging/media/imx/imx-ic-prpencvf.c |  3 ++-
>>   drivers/staging/media/imx/imx-media-csi.c   |  3 ++-
>>   include/video/imx-ipu-v3.h                  |  3 ++-
>>   4 files changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
>> index a9d2501500a1..d41df8034c5b 100644
>> --- a/drivers/gpu/ipu-v3/ipu-cpmem.c
>> +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
>> @@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off)
>>   }
>>   EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
>>   
>> -void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
>> +void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
>> +			       u32 pixelformat)
>>   {
>> -	u32 ilo, sly;
>> +	u32 ilo, sly, sluv;
>>   
>>   	if (stride < 0) {
>>   		stride = -stride;
>> @@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
>>   
>>   	sly = (stride * 2) - 1;
>>   
>> +	switch (pixelformat) {
>> +	case V4L2_PIX_FMT_YUV420:
>> +	case V4L2_PIX_FMT_YVU420:
>> +		sluv = stride / 2 - 1;
>> +		break;
>> +	case V4L2_PIX_FMT_NV12:
>> +		sluv = stride - 1;
>> +		break;
>> +	case V4L2_PIX_FMT_YUV422P:
>> +		sluv = stride - 1;
>> +		break;
>> +	case V4L2_PIX_FMT_NV16:
>> +		sluv = stride * 2 - 1;
>> +		break;
>> +	default:
>> +		sluv = 0;
>> +		break;
>> +	}
>> +
>>   	ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
>>   	ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
>>   	ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
>> +	if (sluv)
>> +		ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
>>   };
>>   EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
> [...]
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> and
>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> to be merged with the rest of the series via the media tree. I'll take
> care not to introduce nontrivial conflicts in imx-drm.

Ok thanks.

Hans, for v5 I will just include the two IPU patches as before. As Philipp
stated, he is OK with merging them to the media tree (after his ack of
course), along with the rest of the patches in this series.

Steve