LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] i.MX6: Support 16-bit BT.1120 video input
@ 2021-10-06  6:13 Krzysztof Hałasa
  2021-10-06  8:45 ` Dan Carpenter
  2021-10-06  9:36 ` Philipp Zabel
  0 siblings, 2 replies; 5+ messages in thread
From: Krzysztof Hałasa @ 2021-10-06  6:13 UTC (permalink / raw)
  To: Philipp Zabel, Steve Longerbeam
  Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-kernel, linux-media, linux-staging,
	linux-arm-kernel

Confirmed to work with ADV7610 HDMI receiver.

Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>

diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index 658c173bebdf..2893b68f1f7a 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -261,10 +261,24 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
 		cfg->data_width = IPU_CSI_DATA_WIDTH_8;
 		break;
 	case MEDIA_BUS_FMT_UYVY8_1X16:
+		if (mbus_type == V4L2_MBUS_BT656) {
+			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_YUV422_UYVY;
+			cfg->data_width = IPU_CSI_DATA_WIDTH_8;
+		} else {
+			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
+			cfg->data_width = IPU_CSI_DATA_WIDTH_16;
+		}
+		cfg->mipi_dt = MIPI_DT_YUV422;
+		break;
 	case MEDIA_BUS_FMT_YUYV8_1X16:
-		cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
+		if (mbus_type == V4L2_MBUS_BT656) {
+			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_YUV422_YUYV;
+			cfg->data_width = IPU_CSI_DATA_WIDTH_8;
+		} else {
+			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
+			cfg->data_width = IPU_CSI_DATA_WIDTH_16;
+		}
 		cfg->mipi_dt = MIPI_DT_YUV422;
-		cfg->data_width = IPU_CSI_DATA_WIDTH_16;
 		break;
 	case MEDIA_BUS_FMT_SBGGR8_1X8:
 	case MEDIA_BUS_FMT_SGBRG8_1X8:
@@ -352,7 +366,7 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
 			    const struct v4l2_mbus_config *mbus_cfg,
 			    const struct v4l2_mbus_framefmt *mbus_fmt)
 {
-	int ret;
+	int ret, is_bt1120;
 
 	memset(csicfg, 0, sizeof(*csicfg));
 
@@ -373,11 +387,18 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
 		break;
 	case V4L2_MBUS_BT656:
 		csicfg->ext_vsync = 0;
+		/* UYVY10_1X20 etc. should be supported as well */
+		is_bt1120 = mbus_fmt->code == MEDIA_BUS_FMT_UYVY8_1X16 ||
+			mbus_fmt->code == MEDIA_BUS_FMT_YUYV8_1X16;
 		if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field) ||
 		    mbus_fmt->field == V4L2_FIELD_ALTERNATE)
-			csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
+			csicfg->clk_mode = is_bt1120 ?
+				IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR :
+				IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
 		else
-			csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
+			csicfg->clk_mode = is_bt1120 ?
+				IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR :
+				IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
 		break;
 	case V4L2_MBUS_CSI2_DPHY:
 		/*
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 45f9d797b9da..ba93512f8c71 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -139,6 +139,8 @@ static inline bool is_parallel_16bit_bus(struct v4l2_fwnode_endpoint *ep)
  * Check for conditions that require the IPU to handle the
  * data internally as generic data, aka passthrough mode:
  * - raw bayer media bus formats, or
+ * - BT.656 and BT.1120 (8/10-bit YUV422) data can always be processed
+ *   on-the-fly (converted to YUV420)
  * - the CSI is receiving from a 16-bit parallel bus, or
  * - the CSI is receiving from an 8-bit parallel bus and the incoming
  *   media bus format is other than UYVY8_2X8/YUYV8_2X8.
@@ -147,6 +149,9 @@ static inline bool requires_passthrough(struct v4l2_fwnode_endpoint *ep,
 					struct v4l2_mbus_framefmt *infmt,
 					const struct imx_media_pixfmt *incc)
 {
+	if (ep->bus_type == V4L2_MBUS_BT656) // including BT.1120
+		return 0;
+
 	return incc->bayer || is_parallel_16bit_bus(ep) ||
 		(is_parallel_bus(ep) &&
 		 infmt->code != MEDIA_BUS_FMT_UYVY8_2X8 &&

-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX6: Support 16-bit BT.1120 video input
  2021-10-06  6:13 [PATCH] i.MX6: Support 16-bit BT.1120 video input Krzysztof Hałasa
@ 2021-10-06  8:45 ` Dan Carpenter
  2021-10-06  9:36 ` Philipp Zabel
  1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-10-06  8:45 UTC (permalink / raw)
  To: Krzysztof Hałasa
  Cc: Philipp Zabel, Steve Longerbeam, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-kernel, linux-media, linux-staging, linux-arm-kernel

On Wed, Oct 06, 2021 at 08:13:48AM +0200, Krzysztof Hałasa wrote:
> @@ -373,11 +387,18 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>  		break;
>  	case V4L2_MBUS_BT656:
>  		csicfg->ext_vsync = 0;
> +		/* UYVY10_1X20 etc. should be supported as well */
> +		is_bt1120 = mbus_fmt->code == MEDIA_BUS_FMT_UYVY8_1X16 ||
> +			mbus_fmt->code == MEDIA_BUS_FMT_YUYV8_1X16;

Could you align better for readability, otherwise it's harder to spot
the two characters which are swapped between those to macros.  (Also
it's basically normal for both sides of a || to be aligned...)

		is_bt1120 = (mbus_fmt->code == MEDIA_BUS_FMT_UYVY8_1X16 ||
			     mbus_fmt->code == MEDIA_BUS_FMT_YUYV8_1X16);

>  		if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field) ||
>  		    mbus_fmt->field == V4L2_FIELD_ALTERNATE)
> -			csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
> +			csicfg->clk_mode = is_bt1120 ?
> +				IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR :
> +				IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
>  		else
> -			csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
> +			csicfg->clk_mode = is_bt1120 ?
> +				IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR :
> +				IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
>  		break;
>  	case V4L2_MBUS_CSI2_DPHY:
>  		/*

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX6: Support 16-bit BT.1120 video input
  2021-10-06  6:13 [PATCH] i.MX6: Support 16-bit BT.1120 video input Krzysztof Hałasa
  2021-10-06  8:45 ` Dan Carpenter
@ 2021-10-06  9:36 ` Philipp Zabel
  2021-10-06 10:52   ` Krzysztof Hałasa
  1 sibling, 1 reply; 5+ messages in thread
From: Philipp Zabel @ 2021-10-06  9:36 UTC (permalink / raw)
  To: Krzysztof Hałasa, Steve Longerbeam
  Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-kernel, linux-media, linux-staging,
	linux-arm-kernel

Hi Krzysztof,

On Wed, 2021-10-06 at 08:13 +0200, Krzysztof Hałasa wrote:
> Confirmed to work with ADV7610 HDMI receiver.
> 
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index 658c173bebdf..2893b68f1f7a 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -261,10 +261,24 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
>  		cfg->data_width = IPU_CSI_DATA_WIDTH_8;
>  		break;
>  	case MEDIA_BUS_FMT_UYVY8_1X16:
> +		if (mbus_type == V4L2_MBUS_BT656) {
> +			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_YUV422_UYVY;
> +			cfg->data_width = IPU_CSI_DATA_WIDTH_8;
> +		} else {
> +			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
> +			cfg->data_width = IPU_CSI_DATA_WIDTH_16;
> +		}
> +		cfg->mipi_dt = MIPI_DT_YUV422;
> +		break;
>  	case MEDIA_BUS_FMT_YUYV8_1X16:
> -		cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
> +		if (mbus_type == V4L2_MBUS_BT656) {
> +			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_YUV422_YUYV;
> +			cfg->data_width = IPU_CSI_DATA_WIDTH_8;
> +		} else {
> +			cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER;
> +			cfg->data_width = IPU_CSI_DATA_WIDTH_16;
> +		}
>  		cfg->mipi_dt = MIPI_DT_YUV422;
> -		cfg->data_width = IPU_CSI_DATA_WIDTH_16;
>  		break;
>  	case MEDIA_BUS_FMT_SBGGR8_1X8:
>  	case MEDIA_BUS_FMT_SGBRG8_1X8:
> @@ -352,7 +366,7 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>  			    const struct v4l2_mbus_config *mbus_cfg,
>  			    const struct v4l2_mbus_framefmt *mbus_fmt)
>  {
> -	int ret;
> +	int ret, is_bt1120;
>  
>  	memset(csicfg, 0, sizeof(*csicfg));
>  
> @@ -373,11 +387,18 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>  		break;
>  	case V4L2_MBUS_BT656:
>  		csicfg->ext_vsync = 0;
> +		/* UYVY10_1X20 etc. should be supported as well */
> +		is_bt1120 = mbus_fmt->code == MEDIA_BUS_FMT_UYVY8_1X16 ||
> +			mbus_fmt->code == MEDIA_BUS_FMT_YUYV8_1X16;
>  		if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field) ||
>  		    mbus_fmt->field == V4L2_FIELD_ALTERNATE)
> -			csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
> +			csicfg->clk_mode = is_bt1120 ?
> +				IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR :
> +				IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
>  		else
> -			csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
> +			csicfg->clk_mode = is_bt1120 ?
> +				IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR :
> +				IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
>  		break;
>  	case V4L2_MBUS_CSI2_DPHY:
>  		/*
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 45f9d797b9da..ba93512f8c71 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -139,6 +139,8 @@ static inline bool is_parallel_16bit_bus(struct v4l2_fwnode_endpoint *ep)
>   * Check for conditions that require the IPU to handle the
>   * data internally as generic data, aka passthrough mode:
>   * - raw bayer media bus formats, or
> + * - BT.656 and BT.1120 (8/10-bit YUV422) data can always be processed
> + *   on-the-fly (converted to YUV420)

This comment seems misleading. The CSI converts to YUV 4:4:4 internally.

>   * - the CSI is receiving from a 16-bit parallel bus, or
>   * - the CSI is receiving from an 8-bit parallel bus and the incoming
>   *   media bus format is other than UYVY8_2X8/YUYV8_2X8.
> @@ -147,6 +149,9 @@ static inline bool requires_passthrough(struct v4l2_fwnode_endpoint *ep,
>  					struct v4l2_mbus_framefmt *infmt,
>  					const struct imx_media_pixfmt *incc)
>  {
> +	if (ep->bus_type == V4L2_MBUS_BT656) // including BT.1120
> +		return 0;
> +
>  	return incc->bayer || is_parallel_16bit_bus(ep) ||
>  		(is_parallel_bus(ep) &&
>  		 infmt->code != MEDIA_BUS_FMT_UYVY8_2X8 &&

regards
Philipp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX6: Support 16-bit BT.1120 video input
  2021-10-06  9:36 ` Philipp Zabel
@ 2021-10-06 10:52   ` Krzysztof Hałasa
  2021-10-08 11:53     ` Philipp Zabel
  0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Hałasa @ 2021-10-06 10:52 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Steve Longerbeam, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-kernel, linux-media, linux-staging,
	linux-arm-kernel

Hi Philipp,

Philipp Zabel <p.zabel@pengutronix.de> writes:

>> + * - BT.656 and BT.1120 (8/10-bit YUV422) data can always be processed
>> + *   on-the-fly (converted to YUV420)
>
> This comment seems misleading. The CSI converts to YUV 4:4:4 internally.

Well... this is surprising. You mean "on the internal bus", don't you?

Please correct me if the following is wrong:

I always though that the "on-the-fly processing", in case of YUV422,
means in practice I can get YUV420 out of the IPU, without a need to do
e.g. NEON conversion. I know I can get the original YUV422 as well,
using the "generic data" mode, but it's incompatible with the CODA H.264
encoder.

Ok, the DQRM (37.4.3.2.1) states that for parallel YUV the output from
CSI is always YUV444.



Then 37.4.3.9 says that the only YUV422 way is to use 16-bit "generic
data". This doesn't seem to be very true, however I'm not exactly sure
about the "on-the-fly" thing.
The fact is the patch works.
Also, the CSIx_SENS_DATA_FORMAT field in IPUx_CSIx_SENS_CONF register
shows YUV422 YUYV and UYVY input data formats, clearly separate from
"Bayer of Generic data".

DQIEC, 4.12.10.1, isn't very clear either:
8) YCbCr 20-bit (10-bit Y + 10-bit U/V) is supported with BT.1120 only
7) YCbCr 16-bit is supported under the same conditions as 8)
6) YCbCr 16-bit (= YUV422) is also supported as "generic-data"
   (no on-the-fly processing). This seems to imply 8) and 7) are
   supported WITH o-t-f-p (and obviously I have tested it, 16-bit only).

I think I will just remove the comment :-)
-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX6: Support 16-bit BT.1120 video input
  2021-10-06 10:52   ` Krzysztof Hałasa
@ 2021-10-08 11:53     ` Philipp Zabel
  0 siblings, 0 replies; 5+ messages in thread
From: Philipp Zabel @ 2021-10-08 11:53 UTC (permalink / raw)
  To: Krzysztof Hałasa
  Cc: Steve Longerbeam, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-kernel, linux-media, linux-staging,
	linux-arm-kernel

Hi Krzysztof,

On Wed, 2021-10-06 at 12:52 +0200, Krzysztof Hałasa wrote:
> Hi Philipp,
> 
> Philipp Zabel <p.zabel@pengutronix.de> writes:
> 
> > > + * - BT.656 and BT.1120 (8/10-bit YUV422) data can always be processed
> > > + *   on-the-fly (converted to YUV420)
> > 
> > This comment seems misleading. The CSI converts to YUV 4:4:4 internally.
> 
> Well... this is surprising. You mean "on the internal bus", don't you?

Yes, anything apart from the bayer/generic data modes, internally
everything is converted into 32-bit YUVA/RGBA pixels (according to the
Reference Manual, 37.4.2.3 FCW & FCR). That is represented by the
MEDIA_BUS_FMT_AYUV8_1X32 media bus format at the CSI source pads.

> Please correct me if the following is wrong:
> 
> I always though that the "on-the-fly processing", in case of YUV422,
> means in practice I can get YUV420 out of the IPU, without a need to do
> e.g. NEON conversion.

That is done in the IDMAC, which can write any supported YUV format from
the internal YUV pixels (if not in bayer/generic data mode).

> I know I can get the original YUV422 as well,
> using the "generic data" mode, but it's incompatible with the CODA H.264
> encoder.

You should also be able to store the YUV formatted pixels as NV12, NV16,
YUYV, etc.

> Ok, the DQRM (37.4.3.2.1) states that for parallel YUV the output from
> CSI is always YUV444.

Ack.

> Then 37.4.3.9 says that the only YUV422 way is to use 16-bit "generic
> data". This doesn't seem to be very true, however I'm not exactly sure
> about the "on-the-fly" thing.

I think that statement is limited to the parallel 16-bit interface in
hsync/vsync mode, whereas in bt.656 / bt.1120 mode the interface
operates as if the two components were clocked in as two separate 8-bit
(or 10-bit) values.

> The fact is the patch works.
> Also, the CSIx_SENS_DATA_FORMAT field in IPUx_CSIx_SENS_CONF register
> shows YUV422 YUYV and UYVY input data formats, clearly separate from
> "Bayer of Generic data".
> 
> DQIEC, 4.12.10.1, isn't very clear either:
> 8) YCbCr 20-bit (10-bit Y + 10-bit U/V) is supported with BT.1120 only
> 7) YCbCr 16-bit is supported under the same conditions as 8)
> 6) YCbCr 16-bit (= YUV422) is also supported as "generic-data"
>    (no on-the-fly processing). This seems to imply 8) and 7) are
>    supported WITH o-t-f-p (and obviously I have tested it, 16-bit only).
>
> I think I will just remove the comment :-)

That sounds good to me.

regards
Philipp

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-10-08 11:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06  6:13 [PATCH] i.MX6: Support 16-bit BT.1120 video input Krzysztof Hałasa
2021-10-06  8:45 ` Dan Carpenter
2021-10-06  9:36 ` Philipp Zabel
2021-10-06 10:52   ` Krzysztof Hałasa
2021-10-08 11:53     ` Philipp Zabel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).