[PATCH 0/5] drm/meson: Add support for HDMI2.0 YUV420 4k60

[PATCH 0/5] drm/meson: Add support for HDMI2.0 YUV420 4k60

[Neil Armstrong]

The Synopsys DW-HDMI CSC does not support downsampling to YUV420, so
the encoder must downsamle before, feeding the controller with a YUV420
pixel stream.

The encoder must declare the new bus format enc encoding the bridge, in
order to take it in account.

To solve this, a new format_set() bridge op is added, permitting setting
a new input bus format and encoding to the bridge chain.

This solves YUV420 setup, but also solved setting 10bit, 12bit or 16bit
input bus format in order to support HDMI >8bit depths.

The DW-HDMI controller is updated to dynamically select a coherent output
bus format depending on the input bus format and on the internal CSC
supported modes.

The DW-HDMI is also updated to support the connector display_info bus_formats
entry to permit forcing a specific output bus format to force, for example,
an YUV444 output format instead of the default RGB output bus format.

Only the meson DRM dw_hdmi glue allows ycbcr420 modes, so no breakage
is expected here.

The remaining patches adds support for 4:2:0 output and clock setup for
the meson DW-HDMI glue, and how YUV444 output can be forced.

Changes since rfc:
* Fixed small logic error in drm_bridge_format_set()
* rebased on v5.2-rc1

Neil Armstrong (5):
  drm/bridge: dw-hdmi: allow ycbcr420 modes for >= 0x200a
  drm/bridge: add encoder support to specify bridge input format
  drm/bridge: dw-hdmi: Add support for dynamic output format setup
  drm/meson: Add YUV420 output support
  drm/meson: Output in YUV444 if sink supports it

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 127 ++++++++++++++++++++--
 drivers/gpu/drm/drm_bridge.c              |  35 ++++++
 drivers/gpu/drm/meson/meson_dw_hdmi.c     | 111 ++++++++++++++++---
 drivers/gpu/drm/meson/meson_vclk.c        |  93 ++++++++++++----
 drivers/gpu/drm/meson/meson_vclk.h        |   7 +-
 drivers/gpu/drm/meson/meson_venc.c        |   6 +-
 drivers/gpu/drm/meson/meson_venc.h        |  11 ++
 drivers/gpu/drm/meson/meson_venc_cvbs.c   |   3 +-
 include/drm/bridge/dw_hdmi.h              |   1 +
 include/drm/drm_bridge.h                  |  19 ++++
 10 files changed, 358 insertions(+), 55 deletions(-)

-- 
2.21.0


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

[PATCH 1/5] drm/bridge: dw-hdmi: allow ycbcr420 modes for >= 0x200a

[Neil Armstrong]

Now the DW-HDMI Controller supports the HDMI2.0 modes, enable support
for these modes in the connector if the platform supports them.
We limit these modes to DW-HDMI IP version >= 0x200a which
are designed to support HDMI2.0 display modes.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++++++
 include/drm/bridge/dw_hdmi.h              | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a2..b50c49caf7ae 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2629,6 +2629,12 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	if (hdmi->phy.ops->setup_hpd)
 		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
 
+	if (hdmi->version >= 0x200a)
+		hdmi->connector.ycbcr_420_allowed =
+			hdmi->plat_data->ycbcr_420_allowed;
+	else
+		hdmi->connector.ycbcr_420_allowed = false;
+
 	memset(&pdevinfo, 0, sizeof(pdevinfo));
 	pdevinfo.parent = dev;
 	pdevinfo.id = PLATFORM_DEVID_AUTO;
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 66e70770cce5..0f0e82638fbe 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -130,6 +130,7 @@ struct dw_hdmi_plat_data {
 					   const struct drm_display_mode *mode);
 	unsigned long input_bus_format;
 	unsigned long input_bus_encoding;
+	bool ycbcr_420_allowed;
 
 	/* Vendor PHY support */
 	const struct dw_hdmi_phy_ops *phy_ops;
-- 
2.21.0


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

Re: [PATCH 1/5] drm/bridge: dw-hdmi: allow ycbcr420 modes for >= 0x200a

[Andrzej Hajda]

On 20.05.2019 15:37, Neil Armstrong wrote:
> Now the DW-HDMI Controller supports the HDMI2.0 modes, enable support
> for these modes in the connector if the platform supports them.
> We limit these modes to DW-HDMI IP version >= 0x200a which
> are designed to support HDMI2.0 display modes.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++++++
>  include/drm/bridge/dw_hdmi.h              | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ab7968c8f6a2..b50c49caf7ae 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2629,6 +2629,12 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  	if (hdmi->phy.ops->setup_hpd)
>  		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
>  
> +	if (hdmi->version >= 0x200a)
> +		hdmi->connector.ycbcr_420_allowed =
> +			hdmi->plat_data->ycbcr_420_allowed;
> +	else
> +		hdmi->connector.ycbcr_420_allowed = false;
> +


I suspect else clause can be dropped.

Beside this:

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej


>  	memset(&pdevinfo, 0, sizeof(pdevinfo));
>  	pdevinfo.parent = dev;
>  	pdevinfo.id = PLATFORM_DEVID_AUTO;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 66e70770cce5..0f0e82638fbe 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -130,6 +130,7 @@ struct dw_hdmi_plat_data {
>  					   const struct drm_display_mode *mode);
>  	unsigned long input_bus_format;
>  	unsigned long input_bus_encoding;
> +	bool ycbcr_420_allowed;
>  
>  	/* Vendor PHY support */
>  	const struct dw_hdmi_phy_ops *phy_ops;



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

Re: [PATCH 1/5] drm/bridge: dw-hdmi: allow ycbcr420 modes for >= 0x200a

[Neil Armstrong]

On 22/05/2019 08:07, Andrzej Hajda wrote:
> On 20.05.2019 15:37, Neil Armstrong wrote:
>> Now the DW-HDMI Controller supports the HDMI2.0 modes, enable support
>> for these modes in the connector if the platform supports them.
>> We limit these modes to DW-HDMI IP version >= 0x200a which
>> are designed to support HDMI2.0 display modes.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++++++
>>  include/drm/bridge/dw_hdmi.h              | 1 +
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index ab7968c8f6a2..b50c49caf7ae 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2629,6 +2629,12 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  	if (hdmi->phy.ops->setup_hpd)
>>  		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
>>  
>> +	if (hdmi->version >= 0x200a)
>> +		hdmi->connector.ycbcr_420_allowed =
>> +			hdmi->plat_data->ycbcr_420_allowed;
>> +	else
>> +		hdmi->connector.ycbcr_420_allowed = false;
>> +
> 
> 
> I suspect else clause can be dropped.

You are right, thanks for the review.

Do you have comments on the patches 2, 3 & 4 of serie ?

Thanks,
Neil

> 
> Beside this:
> 
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> 
>  --
> Regards
> Andrzej
> 
> 
>>  	memset(&pdevinfo, 0, sizeof(pdevinfo));
>>  	pdevinfo.parent = dev;
>>  	pdevinfo.id = PLATFORM_DEVID_AUTO;
>> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
>> index 66e70770cce5..0f0e82638fbe 100644
>> --- a/include/drm/bridge/dw_hdmi.h
>> +++ b/include/drm/bridge/dw_hdmi.h
>> @@ -130,6 +130,7 @@ struct dw_hdmi_plat_data {
>>  					   const struct drm_display_mode *mode);
>>  	unsigned long input_bus_format;
>>  	unsigned long input_bus_encoding;
>> +	bool ycbcr_420_allowed;
>>  
>>  	/* Vendor PHY support */
>>  	const struct dw_hdmi_phy_ops *phy_ops;
> 
> 


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

[PATCH 2/5] drm/bridge: add encoder support to specify bridge input format

[Neil Armstrong]

This patch adds a new format_set() callback to the bridge ops permitting
the encoder to specify the new input format and encoding.

This allows supporting the very specific HDMI2.0 YUV420 output mode
when the bridge cannot convert from RGB or YUV444 to YUV420.

In this case, the encode must downsample before the bridge and must
specify the bridge the new input bus format differs.

This will also help supporting the YUV420 mode where the bridge cannot
downsample, and also support 10bit, 12bit and 16bit output modes
when the bridge cannot convert between different bit depths.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/drm_bridge.c | 35 +++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     | 19 +++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 138b2711d389..33be74a977f7 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -307,6 +307,41 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
 }
 EXPORT_SYMBOL(drm_bridge_mode_set);
 
+/**
+ * drm_bridge_format_set - setup with proposed input format and encoding for
+ *			   all bridges in the encoder chain
+ * @bridge: bridge control structure
+ * @input_bus_format: proposed input bus format for the bridge
+ * @input_encoding: proposed input encoding for this bridge
+ *
+ * Calls &drm_bridge_funcs.format_set op for all the bridges in the
+ * encoder chain, starting from the first bridge to the last.
+ *
+ * Note: the bridge passed should be the one closest to the encoder
+ *
+ * RETURNS:
+ * true on success, false if one of the bridge cannot handle the format
+ */
+bool drm_bridge_format_set(struct drm_bridge *bridge,
+			   const u32 input_bus_format,
+			   const u32 input_encoding)
+{
+	bool ret = true;
+
+	if (!bridge)
+		return true;
+
+	if (bridge->funcs->format_set)
+		ret = bridge->funcs->format_set(bridge, input_bus_format,
+						input_encoding);
+	if (!ret)
+		return ret;
+
+	return drm_bridge_format_set(bridge->next, input_bus_format,
+				     input_encoding);
+}
+EXPORT_SYMBOL(drm_bridge_format_set);
+
 /**
  * drm_bridge_pre_enable - prepares for enabling all
  *			   bridges in the encoder chain
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d4428913a4e1..7a79e61b7825 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -198,6 +198,22 @@ struct drm_bridge_funcs {
 	void (*mode_set)(struct drm_bridge *bridge,
 			 const struct drm_display_mode *mode,
 			 const struct drm_display_mode *adjusted_mode);
+
+	/**
+	 * @format_set:
+	 *
+	 * This callback should configure the bridge for the given input bus
+	 * format and encoding. It is called after the @format_set callback
+	 * for the preceding element in the display pipeline has been called
+	 * already. If the bridge is the first element then this would be
+	 * &drm_encoder_helper_funcs.format_set. The display pipe (i.e.
+	 * clocks and timing signals) is off when this function is called.
+	 *
+	 * @returns: true in success, false is a bridge refuses the format
+	 */
+	bool (*format_set)(struct drm_bridge *bridge,
+			   const u32 input_bus_format,
+			   const u32 input_encoding);
 	/**
 	 * @pre_enable:
 	 *
@@ -311,6 +327,9 @@ void drm_bridge_post_disable(struct drm_bridge *bridge);
 void drm_bridge_mode_set(struct drm_bridge *bridge,
 			 const struct drm_display_mode *mode,
 			 const struct drm_display_mode *adjusted_mode);
+bool drm_bridge_format_set(struct drm_bridge *bridge,
+			   const u32 input_bus_format,
+			   const u32 input_encoding);
 void drm_bridge_pre_enable(struct drm_bridge *bridge);
 void drm_bridge_enable(struct drm_bridge *bridge);
 
-- 
2.21.0


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

Re: [PATCH 2/5] drm/bridge: add encoder support to specify bridge input format

[Jernej Škrabec]

Hi!

Thanks for working on this!

Dne ponedeljek, 20. maj 2019 ob 15:37:50 CEST je Neil Armstrong napisal(a):
> This patch adds a new format_set() callback to the bridge ops permitting
> the encoder to specify the new input format and encoding.
> 
> This allows supporting the very specific HDMI2.0 YUV420 output mode
> when the bridge cannot convert from RGB or YUV444 to YUV420.
> 
> In this case, the encode must downsample before the bridge and must
> specify the bridge the new input bus format differs.
> 
> This will also help supporting the YUV420 mode where the bridge cannot
> downsample, and also support 10bit, 12bit and 16bit output modes
> when the bridge cannot convert between different bit depths.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---

Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej



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

Re: [PATCH 2/5] drm/bridge: add encoder support to specify bridge input format

[Laurent Pinchart]

Hi Neil,

Thank you for the patch.

On Mon, May 20, 2019 at 03:37:50PM +0200, Neil Armstrong wrote:
> This patch adds a new format_set() callback to the bridge ops permitting
> the encoder to specify the new input format and encoding.
> 
> This allows supporting the very specific HDMI2.0 YUV420 output mode
> when the bridge cannot convert from RGB or YUV444 to YUV420.
> 
> In this case, the encode must downsample before the bridge and must
> specify the bridge the new input bus format differs.
> 
> This will also help supporting the YUV420 mode where the bridge cannot
> downsample, and also support 10bit, 12bit and 16bit output modes
> when the bridge cannot convert between different bit depths.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/drm_bridge.c | 35 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_bridge.h     | 19 +++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 138b2711d389..33be74a977f7 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -307,6 +307,41 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
>  }
>  EXPORT_SYMBOL(drm_bridge_mode_set);
>  
> +/**
> + * drm_bridge_format_set - setup with proposed input format and encoding for
> + *			   all bridges in the encoder chain
> + * @bridge: bridge control structure
> + * @input_bus_format: proposed input bus format for the bridge
> + * @input_encoding: proposed input encoding for this bridge
> + *
> + * Calls &drm_bridge_funcs.format_set op for all the bridges in the
> + * encoder chain, starting from the first bridge to the last.
> + *
> + * Note: the bridge passed should be the one closest to the encoder
> + *
> + * RETURNS:
> + * true on success, false if one of the bridge cannot handle the format

I would return an int to propagate the failure reason upstream. It will
reach the commit tail handler in any case, so will be dropped there, but
could help debugging issues if we print it in the right place.

> + */
> +bool drm_bridge_format_set(struct drm_bridge *bridge,
> +			   const u32 input_bus_format,
> +			   const u32 input_encoding)

You don't need a const here.

> +{
> +	bool ret = true;
> +
> +	if (!bridge)
> +		return true;
> +
> +	if (bridge->funcs->format_set)
> +		ret = bridge->funcs->format_set(bridge, input_bus_format,
> +						input_encoding);
> +	if (!ret)
> +		return ret;
> +
> +	return drm_bridge_format_set(bridge->next, input_bus_format,
> +				     input_encoding);

I don't think this will scale. It's not that uncommon for bridges to
change the format (most likely converting from YUV to RGB or the other
way around, or reducing the number of bits per sample) and the encoding.
We thus can't propagate it from bridge to bridge and expect that to
work.

At the very least, the bridge should report its output bus format and
encoding, to be applied to the next bridge, but this won't allow
checking if the configuration can be applied ahead of time, resulting in
possible failures of a commit tail handler. I wonder if this wouldn't be
a good time to introduce bridge states...

> +}
> +EXPORT_SYMBOL(drm_bridge_format_set);
> +
>  /**
>   * drm_bridge_pre_enable - prepares for enabling all
>   *			   bridges in the encoder chain
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index d4428913a4e1..7a79e61b7825 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -198,6 +198,22 @@ struct drm_bridge_funcs {
>  	void (*mode_set)(struct drm_bridge *bridge,
>  			 const struct drm_display_mode *mode,
>  			 const struct drm_display_mode *adjusted_mode);
> +
> +	/**
> +	 * @format_set:
> +	 *
> +	 * This callback should configure the bridge for the given input bus
> +	 * format and encoding. It is called after the @format_set callback
> +	 * for the preceding element in the display pipeline has been called
> +	 * already. If the bridge is the first element then this would be
> +	 * &drm_encoder_helper_funcs.format_set. The display pipe (i.e.
> +	 * clocks and timing signals) is off when this function is called.
> +	 *
> +	 * @returns: true in success, false is a bridge refuses the format
> +	 */
> +	bool (*format_set)(struct drm_bridge *bridge,
> +			   const u32 input_bus_format,
> +			   const u32 input_encoding);
>  	/**
>  	 * @pre_enable:
>  	 *
> @@ -311,6 +327,9 @@ void drm_bridge_post_disable(struct drm_bridge *bridge);
>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>  			 const struct drm_display_mode *mode,
>  			 const struct drm_display_mode *adjusted_mode);
> +bool drm_bridge_format_set(struct drm_bridge *bridge,
> +			   const u32 input_bus_format,
> +			   const u32 input_encoding);
>  void drm_bridge_pre_enable(struct drm_bridge *bridge);
>  void drm_bridge_enable(struct drm_bridge *bridge);
>  

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 2/5] drm/bridge: add encoder support to specify bridge input format

[Neil Armstrong]

On 07/06/2019 15:38, Laurent Pinchart wrote:
> Hi Neil,
> 
> Thank you for the patch.
> 
> On Mon, May 20, 2019 at 03:37:50PM +0200, Neil Armstrong wrote:
>> This patch adds a new format_set() callback to the bridge ops permitting
>> the encoder to specify the new input format and encoding.
>>
>> This allows supporting the very specific HDMI2.0 YUV420 output mode
>> when the bridge cannot convert from RGB or YUV444 to YUV420.
>>
>> In this case, the encode must downsample before the bridge and must
>> specify the bridge the new input bus format differs.
>>
>> This will also help supporting the YUV420 mode where the bridge cannot
>> downsample, and also support 10bit, 12bit and 16bit output modes
>> when the bridge cannot convert between different bit depths.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/gpu/drm/drm_bridge.c | 35 +++++++++++++++++++++++++++++++++++
>>  include/drm/drm_bridge.h     | 19 +++++++++++++++++++
>>  2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 138b2711d389..33be74a977f7 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -307,6 +307,41 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
>>  }
>>  EXPORT_SYMBOL(drm_bridge_mode_set);
>>  
>> +/**
>> + * drm_bridge_format_set - setup with proposed input format and encoding for
>> + *			   all bridges in the encoder chain
>> + * @bridge: bridge control structure
>> + * @input_bus_format: proposed input bus format for the bridge
>> + * @input_encoding: proposed input encoding for this bridge
>> + *
>> + * Calls &drm_bridge_funcs.format_set op for all the bridges in the
>> + * encoder chain, starting from the first bridge to the last.
>> + *
>> + * Note: the bridge passed should be the one closest to the encoder
>> + *
>> + * RETURNS:
>> + * true on success, false if one of the bridge cannot handle the format
> 
> I would return an int to propagate the failure reason upstream. It will
> reach the commit tail handler in any case, so will be dropped there, but
> could help debugging issues if we print it in the right place.

Indeed.

> 
>> + */
>> +bool drm_bridge_format_set(struct drm_bridge *bridge,
>> +			   const u32 input_bus_format,
>> +			   const u32 input_encoding)
> 
> You don't need a const here.

Noted

> 
>> +{
>> +	bool ret = true;
>> +
>> +	if (!bridge)
>> +		return true;
>> +
>> +	if (bridge->funcs->format_set)
>> +		ret = bridge->funcs->format_set(bridge, input_bus_format,
>> +						input_encoding);
>> +	if (!ret)
>> +		return ret;
>> +
>> +	return drm_bridge_format_set(bridge->next, input_bus_format,
>> +				     input_encoding);
> 
> I don't think this will scale. It's not that uncommon for bridges to
> change the format (most likely converting from YUV to RGB or the other
> way around, or reducing the number of bits per sample) and the encoding.
> We thus can't propagate it from bridge to bridge and expect that to
> work.
> 
> At the very least, the bridge should report its output bus format and
> encoding, to be applied to the next bridge, but this won't allow
> checking if the configuration can be applied ahead of time, resulting in
> possible failures of a commit tail handler. I wonder if this wouldn't be
> a good time to introduce bridge states...

Ok for chaining the format_set, I thought about it when writing it.

And what if a added a second call drm_bridge_format_check() to check
if a future format_set chaining would work ? then we could either do the
format_set, try another or fallback to a known default format setup.

Would that be sufficient ?

Bridge states would be interesting, but it's really out of the scope of
our current needs in term of bridge changes.

IMHO we can start with format_check/format_set and switch to bridges states
when we have a sufficient use case needing a finer handling of states.

Neil

> 
>> +}
>> +EXPORT_SYMBOL(drm_bridge_format_set);
>> +
>>  /**
>>   * drm_bridge_pre_enable - prepares for enabling all
>>   *			   bridges in the encoder chain
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index d4428913a4e1..7a79e61b7825 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -198,6 +198,22 @@ struct drm_bridge_funcs {
>>  	void (*mode_set)(struct drm_bridge *bridge,
>>  			 const struct drm_display_mode *mode,
>>  			 const struct drm_display_mode *adjusted_mode);
>> +
>> +	/**
>> +	 * @format_set:
>> +	 *
>> +	 * This callback should configure the bridge for the given input bus
>> +	 * format and encoding. It is called after the @format_set callback
>> +	 * for the preceding element in the display pipeline has been called
>> +	 * already. If the bridge is the first element then this would be
>> +	 * &drm_encoder_helper_funcs.format_set. The display pipe (i.e.
>> +	 * clocks and timing signals) is off when this function is called.
>> +	 *
>> +	 * @returns: true in success, false is a bridge refuses the format
>> +	 */
>> +	bool (*format_set)(struct drm_bridge *bridge,
>> +			   const u32 input_bus_format,
>> +			   const u32 input_encoding);
>>  	/**
>>  	 * @pre_enable:
>>  	 *
>> @@ -311,6 +327,9 @@ void drm_bridge_post_disable(struct drm_bridge *bridge);
>>  void drm_bridge_mode_set(struct drm_bridge *bridge,
>>  			 const struct drm_display_mode *mode,
>>  			 const struct drm_display_mode *adjusted_mode);
>> +bool drm_bridge_format_set(struct drm_bridge *bridge,
>> +			   const u32 input_bus_format,
>> +			   const u32 input_encoding);
>>  void drm_bridge_pre_enable(struct drm_bridge *bridge);
>>  void drm_bridge_enable(struct drm_bridge *bridge);
>>  
> 


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

[PATCH 3/5] drm/bridge: dw-hdmi: Add support for dynamic output format setup

[Neil Armstrong]

In order to support the HDMI2.0 YUV420, YUV422 and the 10bit, 12bit and
16bits outpu use cases, add support for the recently introduced bridge
callback format_set().

This callback will setup the new input format and encoding from encoder,
then these information will be used instead of the default ones
in the dw_hdmi_setup() function.

To determine the output bus format, has been added :
- support for the connector display_info bus_formats, where a fixed
  output bus format can be enforced by the encoder
- support for synami output bus format depending on the input format,
  especially the YUV420 input bus format, enforcing YUV420 as output
  with the correct bit depth

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 121 ++++++++++++++++++++--
 1 file changed, 112 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index b50c49caf7ae..284ce59be8f8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -103,6 +103,8 @@ struct hdmi_vmode {
 };
 
 struct hdmi_data_info {
+	unsigned int bridge_in_bus_format;
+	unsigned int bridge_in_encoding;
 	unsigned int enc_in_bus_format;
 	unsigned int enc_out_bus_format;
 	unsigned int enc_in_encoding;
@@ -1838,8 +1840,51 @@ static void hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
 		    HDMI_IH_MUTE_FC_STAT2);
 }
 
+/*
+ * The DW-HDMI CSC can only interpolate and decimate from 4:2:2 to 4:4:4/RGB
+ * and from 4:4:4/RGB to 4:2:2.
+ * Default to RGB output except if 4:2:0 as input, which CSC cannot convert.
+ */
+static unsigned long dw_hdmi_determine_output_bus_format(struct dw_hdmi *hdmi)
+{
+	unsigned int depth = hdmi_bus_fmt_color_depth(
+					hdmi->hdmi_data.enc_in_bus_format);
+	bool is_420 = hdmi_bus_fmt_is_yuv420(hdmi->hdmi_data.enc_in_bus_format);
+	unsigned long fmt = MEDIA_BUS_FMT_RGB888_1X24;
+
+	switch (depth) {
+	case 8:
+		if (is_420)
+			fmt = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
+		else
+			fmt = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	case 10:
+		if (is_420)
+			fmt = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
+		else
+			fmt = MEDIA_BUS_FMT_RGB101010_1X30;
+		break;
+	case 12:
+		if (is_420)
+			fmt = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
+		else
+			fmt = MEDIA_BUS_FMT_RGB121212_1X36;
+		break;
+	case 16:
+		if (is_420)
+			fmt = MEDIA_BUS_FMT_UYYVYY16_0_5X48;
+		else
+			fmt = MEDIA_BUS_FMT_RGB161616_1X48;
+		break;
+	}
+
+	return fmt;
+}
+
 static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 {
+	struct drm_display_info *display = &hdmi->connector.display_info;
 	int ret;
 
 	hdmi_disable_overflow_interrupts(hdmi);
@@ -1853,9 +1898,9 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	}
 
 	if ((hdmi->vic == 6) || (hdmi->vic == 7) ||
-	    (hdmi->vic == 21) || (hdmi->vic == 22) ||
-	    (hdmi->vic == 2) || (hdmi->vic == 3) ||
-	    (hdmi->vic == 17) || (hdmi->vic == 18))
+		 (hdmi->vic == 21) || (hdmi->vic == 22) ||
+		 (hdmi->vic == 2) || (hdmi->vic == 3) ||
+		 (hdmi->vic == 17) || (hdmi->vic == 18))
 		hdmi->hdmi_data.enc_out_encoding = V4L2_YCBCR_ENC_601;
 	else
 		hdmi->hdmi_data.enc_out_encoding = V4L2_YCBCR_ENC_709;
@@ -1863,22 +1908,29 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0;
 	hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0;
 
-	/* TOFIX: Get input format from plat data or fallback to RGB888 */
-	if (hdmi->plat_data->input_bus_format)
+	if (hdmi->hdmi_data.bridge_in_bus_format)
+		hdmi->hdmi_data.enc_in_bus_format =
+			hdmi->hdmi_data.bridge_in_bus_format;
+	else if (hdmi->plat_data->input_bus_format)
 		hdmi->hdmi_data.enc_in_bus_format =
 			hdmi->plat_data->input_bus_format;
 	else
 		hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 
-	/* TOFIX: Get input encoding from plat data or fallback to none */
-	if (hdmi->plat_data->input_bus_encoding)
+	if (hdmi->hdmi_data.bridge_in_encoding)
+		hdmi->hdmi_data.enc_in_encoding =
+			hdmi->hdmi_data.bridge_in_encoding;
+	else if (hdmi->plat_data->input_bus_encoding)
 		hdmi->hdmi_data.enc_in_encoding =
 			hdmi->plat_data->input_bus_encoding;
 	else
 		hdmi->hdmi_data.enc_in_encoding = V4L2_YCBCR_ENC_DEFAULT;
 
-	/* TOFIX: Default to RGB888 output format */
-	hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	if (display->num_bus_formats)
+		hdmi->hdmi_data.enc_out_bus_format = display->bus_formats[0];
+	else
+		hdmi->hdmi_data.enc_out_bus_format =
+				dw_hdmi_determine_output_bus_format(hdmi);
 
 	hdmi->hdmi_data.pix_repet_factor = 0;
 	hdmi->hdmi_data.hdcp_enable = 0;
@@ -2150,6 +2202,56 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
 	return mode_status;
 }
 
+static bool dw_hdmi_drm_bridge_format_set(struct drm_bridge *bridge,
+					 const u32 input_bus_format,
+					 const u32 input_encoding)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	/* Filter supported input bus formats */
+	switch (input_bus_format) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB101010_1X30:
+	case MEDIA_BUS_FMT_RGB121212_1X36:
+	case MEDIA_BUS_FMT_RGB161616_1X48:
+	case MEDIA_BUS_FMT_YUV8_1X24:
+	case MEDIA_BUS_FMT_YUV10_1X30:
+	case MEDIA_BUS_FMT_YUV12_1X36:
+	case MEDIA_BUS_FMT_YUV16_1X48:
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+	case MEDIA_BUS_FMT_UYVY10_1X20:
+	case MEDIA_BUS_FMT_UYVY12_1X24:
+	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+	case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
+	case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
+		break;
+	default:
+		dev_dbg(hdmi->dev, "Unsupported Input bus format %x\n",
+			input_bus_format);
+		return false;
+	}
+
+	/* Filter supported input bus encoding */
+	switch (input_encoding) {
+	case V4L2_YCBCR_ENC_DEFAULT:
+	case V4L2_YCBCR_ENC_601:
+	case V4L2_YCBCR_ENC_709:
+	case V4L2_YCBCR_ENC_XV601:
+	case V4L2_YCBCR_ENC_XV709:
+		break;
+	default:
+		dev_dbg(hdmi->dev, "Unsupported Input encoding %x\n",
+			input_bus_format);
+		return false;
+	}
+
+	hdmi->hdmi_data.bridge_in_bus_format = input_bus_format;
+	hdmi->hdmi_data.bridge_in_encoding = input_encoding;
+
+	return true;
+}
+
 static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
 				    const struct drm_display_mode *orig_mode,
 				    const struct drm_display_mode *mode)
@@ -2192,6 +2294,7 @@ static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
 	.disable = dw_hdmi_bridge_disable,
 	.mode_set = dw_hdmi_bridge_mode_set,
 	.mode_valid = dw_hdmi_bridge_mode_valid,
+	.format_set = dw_hdmi_drm_bridge_format_set,
 };
 
 static irqreturn_t dw_hdmi_i2c_irq(struct dw_hdmi *hdmi)
-- 
2.21.0


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

Re: [PATCH 3/5] drm/bridge: dw-hdmi: Add support for dynamic output format setup

[Jernej Škrabec]

Hi!

Dne ponedeljek, 20. maj 2019 ob 15:37:51 CEST je Neil Armstrong napisal(a):
> In order to support the HDMI2.0 YUV420, YUV422 and the 10bit, 12bit and
> 16bits outpu use cases, add support for the recently introduced bridge
> callback format_set().
> 
> This callback will setup the new input format and encoding from encoder,
> then these information will be used instead of the default ones
> in the dw_hdmi_setup() function.
> 
> To determine the output bus format, has been added :
> - support for the connector display_info bus_formats, where a fixed
>   output bus format can be enforced by the encoder
> - support for synami output bus format depending on the input format,
>   especially the YUV420 input bus format, enforcing YUV420 as output
>   with the correct bit depth
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 121 ++++++++++++++++++++--
>  1 file changed, 112 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> b50c49caf7ae..284ce59be8f8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -103,6 +103,8 @@ struct hdmi_vmode {
>  };
> 
>  struct hdmi_data_info {
> +	unsigned int bridge_in_bus_format;
> +	unsigned int bridge_in_encoding;
>  	unsigned int enc_in_bus_format;
>  	unsigned int enc_out_bus_format;
>  	unsigned int enc_in_encoding;
> @@ -1838,8 +1840,51 @@ static void hdmi_disable_overflow_interrupts(struct
> dw_hdmi *hdmi) HDMI_IH_MUTE_FC_STAT2);
>  }
> 
> +/*
> + * The DW-HDMI CSC can only interpolate and decimate from 4:2:2 to
> 4:4:4/RGB + * and from 4:4:4/RGB to 4:2:2.
> + * Default to RGB output except if 4:2:0 as input, which CSC cannot
> convert. + */
> +static unsigned long dw_hdmi_determine_output_bus_format(struct dw_hdmi
> *hdmi) +{
> +	unsigned int depth = hdmi_bus_fmt_color_depth(
> +					hdmi-
>hdmi_data.enc_in_bus_format);
> +	bool is_420 = hdmi_bus_fmt_is_yuv420(hdmi-
>hdmi_data.enc_in_bus_format);
> +	unsigned long fmt = MEDIA_BUS_FMT_RGB888_1X24;
> +
> +	switch (depth) {
> +	case 8:
> +		if (is_420)
> +			fmt = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
> +		else
> +			fmt = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	case 10:
> +		if (is_420)
> +			fmt = MEDIA_BUS_FMT_UYYVYY10_0_5X30;
> +		else
> +			fmt = MEDIA_BUS_FMT_RGB101010_1X30;
> +		break;
> +	case 12:
> +		if (is_420)
> +			fmt = MEDIA_BUS_FMT_UYYVYY12_0_5X36;
> +		else
> +			fmt = MEDIA_BUS_FMT_RGB121212_1X36;
> +		break;
> +	case 16:
> +		if (is_420)
> +			fmt = MEDIA_BUS_FMT_UYYVYY16_0_5X48;
> +		else
> +			fmt = MEDIA_BUS_FMT_RGB161616_1X48;
> +		break;
> +	}
> +
> +	return fmt;
> +}
> +
>  static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode
> *mode) {
> +	struct drm_display_info *display = &hdmi->connector.display_info;
>  	int ret;
> 
>  	hdmi_disable_overflow_interrupts(hdmi);
> @@ -1853,9 +1898,9 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
> drm_display_mode *mode) }
> 
>  	if ((hdmi->vic == 6) || (hdmi->vic == 7) ||
> -	    (hdmi->vic == 21) || (hdmi->vic == 22) ||
> -	    (hdmi->vic == 2) || (hdmi->vic == 3) ||
> -	    (hdmi->vic == 17) || (hdmi->vic == 18))
> +		 (hdmi->vic == 21) || (hdmi->vic == 22) ||
> +		 (hdmi->vic == 2) || (hdmi->vic == 3) ||
> +		 (hdmi->vic == 17) || (hdmi->vic == 18))
>  		hdmi->hdmi_data.enc_out_encoding = V4L2_YCBCR_ENC_601;
>  	else
>  		hdmi->hdmi_data.enc_out_encoding = V4L2_YCBCR_ENC_709;
> @@ -1863,22 +1908,29 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode)
> hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0;
>  	hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0;
> 
> -	/* TOFIX: Get input format from plat data or fallback to RGB888 */
> -	if (hdmi->plat_data->input_bus_format)
> +	if (hdmi->hdmi_data.bridge_in_bus_format)
> +		hdmi->hdmi_data.enc_in_bus_format =
> +			hdmi->hdmi_data.bridge_in_bus_format;
> +	else if (hdmi->plat_data->input_bus_format)
>  		hdmi->hdmi_data.enc_in_bus_format =
>  			hdmi->plat_data->input_bus_format;
>  	else
>  		hdmi->hdmi_data.enc_in_bus_format = 
MEDIA_BUS_FMT_RGB888_1X24;
> 
> -	/* TOFIX: Get input encoding from plat data or fallback to none */
> -	if (hdmi->plat_data->input_bus_encoding)
> +	if (hdmi->hdmi_data.bridge_in_encoding)
> +		hdmi->hdmi_data.enc_in_encoding =
> +			hdmi->hdmi_data.bridge_in_encoding;
> +	else if (hdmi->plat_data->input_bus_encoding)
>  		hdmi->hdmi_data.enc_in_encoding =
>  			hdmi->plat_data->input_bus_encoding;
>  	else
>  		hdmi->hdmi_data.enc_in_encoding = 
V4L2_YCBCR_ENC_DEFAULT;
> 
> -	/* TOFIX: Default to RGB888 output format */
> -	hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	if (display->num_bus_formats)
> +		hdmi->hdmi_data.enc_out_bus_format = display-
>bus_formats[0];
> +	else
> +		hdmi->hdmi_data.enc_out_bus_format =
> +				
dw_hdmi_determine_output_bus_format(hdmi);
> 
>  	hdmi->hdmi_data.pix_repet_factor = 0;
>  	hdmi->hdmi_data.hdcp_enable = 0;
> @@ -2150,6 +2202,56 @@ dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>  	return mode_status;
>  }
> 
> +static bool dw_hdmi_drm_bridge_format_set(struct drm_bridge *bridge,
> +					 const u32 
input_bus_format,
> +					 const u32 
input_encoding)
> +{
> +	struct dw_hdmi *hdmi = bridge->driver_private;
> +
> +	/* Filter supported input bus formats */
> +	switch (input_bus_format) {
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_RGB101010_1X30:
> +	case MEDIA_BUS_FMT_RGB121212_1X36:
> +	case MEDIA_BUS_FMT_RGB161616_1X48:
> +	case MEDIA_BUS_FMT_YUV8_1X24:
> +	case MEDIA_BUS_FMT_YUV10_1X30:
> +	case MEDIA_BUS_FMT_YUV12_1X36:
> +	case MEDIA_BUS_FMT_YUV16_1X48:
> +	case MEDIA_BUS_FMT_UYVY8_1X16:
> +	case MEDIA_BUS_FMT_UYVY10_1X20:
> +	case MEDIA_BUS_FMT_UYVY12_1X24:
> +	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
> +	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
> +	case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
> +	case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
> +		break;
> +	default:
> +		dev_dbg(hdmi->dev, "Unsupported Input bus format %x\n",
> +			input_bus_format);
> +		return false;
> +	}
> +
> +	/* Filter supported input bus encoding */
> +	switch (input_encoding) {
> +	case V4L2_YCBCR_ENC_DEFAULT:
> +	case V4L2_YCBCR_ENC_601:
> +	case V4L2_YCBCR_ENC_709:
> +	case V4L2_YCBCR_ENC_XV601:
> +	case V4L2_YCBCR_ENC_XV709:
> +		break;
> +	default:
> +		dev_dbg(hdmi->dev, "Unsupported Input encoding %x\n",
> +			input_bus_format);
> +		return false;
> +	}
> +
> +	hdmi->hdmi_data.bridge_in_bus_format = input_bus_format;
> +	hdmi->hdmi_data.bridge_in_encoding = input_encoding;
> +
> +	return true;
> +}
> +
>  static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
>  				    const struct drm_display_mode 
*orig_mode,
>  				    const struct drm_display_mode 
*mode)
> @@ -2192,6 +2294,7 @@ static const struct drm_bridge_funcs
> dw_hdmi_bridge_funcs = { .disable = dw_hdmi_bridge_disable,
>  	.mode_set = dw_hdmi_bridge_mode_set,
>  	.mode_valid = dw_hdmi_bridge_mode_valid,
> +	.format_set = dw_hdmi_drm_bridge_format_set,
>  };
> 
>  static irqreturn_t dw_hdmi_i2c_irq(struct dw_hdmi *hdmi)

Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>

Best regards,
Jernej




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

[PATCH 4/5] drm/meson: Add YUV420 output support

[Neil Armstrong]

This patch adds support for the YUV420 output from the Amlogic Meson SoCs
Video Processing Unit to the HDMI Controller.

The YUV420 is obtained by generating a YUV444 pixel stream like
the classic HDMI display modes, but then the Video Encoder output
can be configured to down-sample the YUV444 pixel stream to a YUV420
stream.
In addition if pixel stream down-sampling, the Y Cb Cr components must
also be mapped differently to align with the HDMI2.0 specifications.

This mode needs a different clock generation scheme since the TMDS PHY
clock must match the 10x ration with the YUV420 pixel clock, but
the video encoder must run at 2x the pixel clock.

This patch adds the TMDS PHY clock value in all the video clock setup
in order to better support these specific uses cases and switch
to the Common Clock framework for clocks handling in the future.

When 420 is needed, it calls drm_bridge_format_set() for notify the
bridge the input format has changed to YUV420.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c   | 100 +++++++++++++++++++-----
 drivers/gpu/drm/meson/meson_vclk.c      |  93 ++++++++++++++++------
 drivers/gpu/drm/meson/meson_vclk.h      |   7 +-
 drivers/gpu/drm/meson/meson_venc.c      |   6 +-
 drivers/gpu/drm/meson/meson_venc.h      |  11 +++
 drivers/gpu/drm/meson/meson_venc_cvbs.c |   3 +-
 6 files changed, 174 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 779da21143b9..5d67e2beba58 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -159,6 +159,7 @@ struct meson_dw_hdmi {
 	struct regulator *hdmi_supply;
 	u32 irq_stat;
 	struct dw_hdmi *hdmi;
+	unsigned long input_bus_format;
 };
 #define encoder_to_meson_dw_hdmi(x) \
 	container_of(x, struct meson_dw_hdmi, encoder)
@@ -308,6 +309,10 @@ static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
 	struct meson_drm *priv = dw_hdmi->priv;
 	unsigned int pixel_clock = mode->clock;
 
+	/* For 420, pixel clock is half unlike venc clock */
+	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+		pixel_clock /= 2;
+
 	if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
 	    dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
 		if (pixel_clock >= 371250) {
@@ -383,25 +388,36 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
 {
 	struct meson_drm *priv = dw_hdmi->priv;
 	int vic = drm_match_cea_mode(mode);
+	unsigned int phy_freq;
 	unsigned int vclk_freq;
 	unsigned int venc_freq;
 	unsigned int hdmi_freq;
 
 	vclk_freq = mode->clock;
 
+	/* For 420, pixel clock is half unlike venc clock */
+	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+		vclk_freq /= 2;
+
+	/* TMDS clock is pixel_clock * 10 */
+	phy_freq = vclk_freq * 10;
+
 	if (!vic) {
-		meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, vclk_freq,
-				 vclk_freq, vclk_freq, false);
+		meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq,
+				 vclk_freq, vclk_freq, vclk_freq, false);
 		return;
 	}
 
+	/* 480i/576i needs global pixel doubling */
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		vclk_freq *= 2;
 
 	venc_freq = vclk_freq;
 	hdmi_freq = vclk_freq;
 
-	if (meson_venc_hdmi_venc_repeat(vic))
+	/* VENC double pixels for 1080i, 720p and YUV420 modes */
+	if (meson_venc_hdmi_venc_repeat(vic) ||
+	    dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
 		venc_freq *= 2;
 
 	vclk_freq = max(venc_freq, hdmi_freq);
@@ -409,11 +425,11 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		venc_freq /= 2;
 
-	DRM_DEBUG_DRIVER("vclk:%d venc=%d hdmi=%d enci=%d\n",
-		vclk_freq, venc_freq, hdmi_freq,
+	DRM_DEBUG_DRIVER("vclk:%d phy=%d venc=%d hdmi=%d enci=%d\n",
+		phy_freq, vclk_freq, venc_freq, hdmi_freq,
 		priv->venc.hdmi_use_enci);
 
-	meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, vclk_freq,
+	meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq,
 			 venc_freq, hdmi_freq, priv->venc.hdmi_use_enci);
 }
 
@@ -446,8 +462,9 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
 	/* Enable normal output to PHY */
 	dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
 
-	/* TMDS pattern setup (TOFIX Handle the YUV420 case) */
-	if (mode->clock > 340000) {
+	/* TMDS pattern setup */
+	if (mode->clock > 340000 &&
+	    dw_hdmi->input_bus_format == MEDIA_BUS_FMT_YUV8_1X24) {
 		dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
 				  0);
 		dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23,
@@ -622,6 +639,8 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 		   const struct drm_display_mode *mode)
 {
 	struct meson_drm *priv = connector->dev->dev_private;
+	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
+	unsigned int phy_freq;
 	unsigned int vclk_freq;
 	unsigned int venc_freq;
 	unsigned int hdmi_freq;
@@ -630,9 +649,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 
 	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
 
-	/* If sink max TMDS clock, we reject the mode */
+	/* If sink does not support 540MHz, reject the non-420 HDMI2 modes */
 	if (connector->display_info.max_tmds_clock &&
-	    mode->clock > connector->display_info.max_tmds_clock)
+	    mode->clock > connector->display_info.max_tmds_clock &&
+	    !drm_mode_is_420_only(&connector->display_info, mode) &&
+	    !drm_mode_is_420_also(&connector->display_info, mode))
 		return MODE_BAD;
 
 	/* Check against non-VIC supported modes */
@@ -648,6 +669,15 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 
 	vclk_freq = mode->clock;
 
+	/* For 420, pixel clock is half unlike venc clock */
+	if (drm_mode_is_420_only(&connector->display_info, mode) ||
+	    (!is_hdmi2_sink &&
+	     drm_mode_is_420_also(&connector->display_info, mode)))
+		vclk_freq /= 2;
+
+	/* TMDS clock is pixel_clock * 10 */
+	phy_freq = vclk_freq * 10;
+
 	/* 480i/576i needs global pixel doubling */
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		vclk_freq *= 2;
@@ -655,8 +685,11 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 	venc_freq = vclk_freq;
 	hdmi_freq = vclk_freq;
 
-	/* VENC double pixels for 1080i and 720p modes */
-	if (meson_venc_hdmi_venc_repeat(vic))
+	/* VENC double pixels for 1080i, 720p and YUV420 modes */
+	if (meson_venc_hdmi_venc_repeat(vic) ||
+	    drm_mode_is_420_only(&connector->display_info, mode) ||
+	    (!is_hdmi2_sink &&
+	     drm_mode_is_420_also(&connector->display_info, mode)))
 		venc_freq *= 2;
 
 	vclk_freq = max(venc_freq, hdmi_freq);
@@ -664,10 +697,10 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		venc_freq /= 2;
 
-	dev_dbg(connector->dev->dev, "%s: vclk:%d venc=%d hdmi=%d\n", __func__,
-		vclk_freq, venc_freq, hdmi_freq);
+	dev_dbg(connector->dev->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n",
+		__func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);
 
-	return meson_vclk_vic_supported_freq(vclk_freq);
+	return meson_vclk_vic_supported_freq(phy_freq, vclk_freq);
 }
 
 /* Encoder */
@@ -685,6 +718,24 @@ static int meson_venc_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 					struct drm_crtc_state *crtc_state,
 					struct drm_connector_state *conn_state)
 {
+	struct meson_dw_hdmi *dw_hdmi = encoder_to_meson_dw_hdmi(encoder);
+	struct drm_display_info *info = &conn_state->connector->display_info;
+	struct drm_display_mode *mode = &crtc_state->mode;
+	bool is_hdmi2_sink =
+		conn_state->connector->display_info.hdmi.scdc.supported;
+
+	if (drm_mode_is_420_only(info, mode) ||
+	    (!is_hdmi2_sink && drm_mode_is_420_also(info, mode)))
+		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
+	else
+		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
+
+	/* Specify the encoder output format to the bridge */
+	if (!drm_bridge_format_set(encoder->bridge,
+				   dw_hdmi->input_bus_format,
+				   V4L2_YCBCR_ENC_709))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -722,17 +773,29 @@ static void meson_venc_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	struct meson_dw_hdmi *dw_hdmi = encoder_to_meson_dw_hdmi(encoder);
 	struct meson_drm *priv = dw_hdmi->priv;
 	int vic = drm_match_cea_mode(mode);
+	unsigned int ycrcb_map = MESON_VENC_MAP_CB_Y_CR;
+	bool yuv420_mode = false;
 
 	DRM_DEBUG_DRIVER("\"%s\" vic %d\n", mode->name, vic);
 
+	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24) {
+		ycrcb_map = MESON_VENC_MAP_CR_Y_CB;
+		yuv420_mode = true;
+	}
+
 	/* VENC + VENC-DVI Mode setup */
-	meson_venc_hdmi_mode_set(priv, vic, mode);
+	meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, yuv420_mode, mode);
 
 	/* VCLK Set clock */
 	dw_hdmi_set_vclk(dw_hdmi, mode);
 
-	/* Setup YUV444 to HDMI-TX, no 10bit diphering */
-	writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
+	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+		/* Setup YUV420 to HDMI-TX, no 10bit diphering */
+		writel_relaxed(2 | (2 << 2),
+			       priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
+	else
+		/* Setup YUV444 to HDMI-TX, no 10bit diphering */
+		writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
 }
 
 static const struct drm_encoder_helper_funcs
@@ -977,6 +1040,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	dw_plat_data->phy_data = meson_dw_hdmi;
 	dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
 	dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
+	dw_plat_data->ycbcr_420_allowed = true;
 
 	platform_set_drvdata(pdev, meson_dw_hdmi);
 
diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c
index b39034745444..44250eff8a3f 100644
--- a/drivers/gpu/drm/meson/meson_vclk.c
+++ b/drivers/gpu/drm/meson/meson_vclk.c
@@ -364,12 +364,17 @@ enum {
 /* 2970 /1 /1 /1 /5 /2  => /1 /1 */
 	MESON_VCLK_HDMI_297000,
 /* 5940 /1 /1 /2 /5 /1  => /1 /1 */
-	MESON_VCLK_HDMI_594000
+	MESON_VCLK_HDMI_594000,
+/* 2970 /1 /1 /1 /5 /1  => /1 /2 */
+	MESON_VCLK_HDMI_594000_YUV420,
 };
 
 struct meson_vclk_params {
+	unsigned int pll_freq;
+	unsigned int phy_freq;
+	unsigned int vclk_freq;
+	unsigned int venc_freq;
 	unsigned int pixel_freq;
-	unsigned int pll_base_freq;
 	unsigned int pll_od1;
 	unsigned int pll_od2;
 	unsigned int pll_od3;
@@ -377,8 +382,11 @@ struct meson_vclk_params {
 	unsigned int vclk_div;
 } params[] = {
 	[MESON_VCLK_HDMI_ENCI_54000] = {
+		.pll_freq = 4320000,
+		.phy_freq = 270000,
+		.vclk_freq = 54000,
+		.venc_freq = 54000,
 		.pixel_freq = 54000,
-		.pll_base_freq = 4320000,
 		.pll_od1 = 4,
 		.pll_od2 = 4,
 		.pll_od3 = 1,
@@ -386,8 +394,11 @@ struct meson_vclk_params {
 		.vclk_div = 1,
 	},
 	[MESON_VCLK_HDMI_DDR_54000] = {
-		.pixel_freq = 54000,
-		.pll_base_freq = 4320000,
+		.pll_freq = 4320000,
+		.phy_freq = 270000,
+		.vclk_freq = 54000,
+		.venc_freq = 54000,
+		.pixel_freq = 27000,
 		.pll_od1 = 4,
 		.pll_od2 = 4,
 		.pll_od3 = 1,
@@ -395,8 +406,11 @@ struct meson_vclk_params {
 		.vclk_div = 1,
 	},
 	[MESON_VCLK_HDMI_DDR_148500] = {
-		.pixel_freq = 148500,
-		.pll_base_freq = 2970000,
+		.pll_freq = 2970000,
+		.phy_freq = 742500,
+		.vclk_freq = 148500,
+		.venc_freq = 148500,
+		.pixel_freq = 74250,
 		.pll_od1 = 4,
 		.pll_od2 = 1,
 		.pll_od3 = 1,
@@ -404,8 +418,11 @@ struct meson_vclk_params {
 		.vclk_div = 1,
 	},
 	[MESON_VCLK_HDMI_74250] = {
+		.pll_freq = 2970000,
+		.phy_freq = 742500,
+		.vclk_freq = 74250,
+		.venc_freq = 74250,
 		.pixel_freq = 74250,
-		.pll_base_freq = 2970000,
 		.pll_od1 = 2,
 		.pll_od2 = 2,
 		.pll_od3 = 2,
@@ -413,8 +430,11 @@ struct meson_vclk_params {
 		.vclk_div = 1,
 	},
 	[MESON_VCLK_HDMI_148500] = {
+		.pll_freq = 2970000,
+		.phy_freq = 1485000,
+		.vclk_freq = 148500,
+		.venc_freq = 148500,
 		.pixel_freq = 148500,
-		.pll_base_freq = 2970000,
 		.pll_od1 = 1,
 		.pll_od2 = 2,
 		.pll_od3 = 2,
@@ -422,8 +442,11 @@ struct meson_vclk_params {
 		.vclk_div = 1,
 	},
 	[MESON_VCLK_HDMI_297000] = {
+		.pll_freq = 5940000,
+		.phy_freq = 2970000,
+		.venc_freq = 297000,
+		.vclk_freq = 297000,
 		.pixel_freq = 297000,
-		.pll_base_freq = 5940000,
 		.pll_od1 = 2,
 		.pll_od2 = 1,
 		.pll_od3 = 1,
@@ -431,14 +454,29 @@ struct meson_vclk_params {
 		.vclk_div = 2,
 	},
 	[MESON_VCLK_HDMI_594000] = {
+		.pll_freq = 5940000,
+		.phy_freq = 5940000,
+		.venc_freq = 594000,
+		.vclk_freq = 594000,
 		.pixel_freq = 594000,
-		.pll_base_freq = 5940000,
 		.pll_od1 = 1,
 		.pll_od2 = 1,
 		.pll_od3 = 2,
 		.vid_pll_div = VID_PLL_DIV_5,
 		.vclk_div = 1,
 	},
+	[MESON_VCLK_HDMI_594000_YUV420] = {
+		.pll_freq = 5940000,
+		.phy_freq = 2970000,
+		.venc_freq = 594000,
+		.vclk_freq = 594000,
+		.pixel_freq = 297000,
+		.pll_od1 = 2,
+		.pll_od2 = 1,
+		.pll_od3 = 1,
+		.vid_pll_div = VID_PLL_DIV_5,
+		.vclk_div = 1,
+	},
 	{ /* sentinel */ },
 };
 
@@ -696,6 +734,7 @@ static void meson_hdmi_pll_generic_set(struct meson_drm *priv,
 	unsigned int od, m, frac, od1, od2, od3;
 
 	if (meson_hdmi_pll_find_params(priv, pll_freq, &m, &frac, &od)) {
+		/* OD2 goes to the PHY, and needs to be *10, so keep OD3=1 */
 		od3 = 1;
 		if (od < 4) {
 			od1 = 2;
@@ -718,21 +757,28 @@ static void meson_hdmi_pll_generic_set(struct meson_drm *priv,
 }
 
 enum drm_mode_status
-meson_vclk_vic_supported_freq(unsigned int freq)
+meson_vclk_vic_supported_freq(unsigned int phy_freq,
+			      unsigned int vclk_freq)
 {
 	int i;
 
-	DRM_DEBUG_DRIVER("freq = %d\n", freq);
+	DRM_DEBUG_DRIVER("phy_freq = %d vclk_freq = %d\n",
+			 phy_freq, vclk_freq);
 
 	for (i = 0 ; params[i].pixel_freq ; ++i) {
 		DRM_DEBUG_DRIVER("i = %d pixel_freq = %d alt = %d\n",
 				 i, params[i].pixel_freq,
 				 FREQ_1000_1001(params[i].pixel_freq));
+		DRM_DEBUG_DRIVER("i = %d phy_freq = %d alt = %d\n",
+				 i, params[i].phy_freq,
+				 FREQ_1000_1001(params[i].phy_freq/10)*10);
 		/* Match strict frequency */
-		if (freq == params[i].pixel_freq)
+		if (phy_freq == params[i].phy_freq &&
+		    vclk_freq == params[i].vclk_freq)
 			return MODE_OK;
 		/* Match 1000/1001 variant */
-		if (freq == FREQ_1000_1001(params[i].pixel_freq))
+		if (phy_freq == (FREQ_1000_1001(params[i].phy_freq/10)*10) &&
+		    vclk_freq == FREQ_1000_1001(params[i].vclk_freq))
 			return MODE_OK;
 	}
 
@@ -960,8 +1006,9 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
 }
 
 void meson_vclk_setup(struct meson_drm *priv, unsigned int target,
-		      unsigned int vclk_freq, unsigned int venc_freq,
-		      unsigned int dac_freq, bool hdmi_use_enci)
+		      unsigned int phy_freq, unsigned int vclk_freq,
+		      unsigned int venc_freq, unsigned int dac_freq,
+		      bool hdmi_use_enci)
 {
 	bool vic_alternate_clock = false;
 	unsigned int freq;
@@ -980,7 +1027,7 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target,
 		 * - venc_div = 1
 		 * - encp encoder
 		 */
-		meson_vclk_set(priv, vclk_freq * 10, 0, 0, 0,
+		meson_vclk_set(priv, phy_freq, 0, 0, 0,
 			       VID_PLL_DIV_5, 2, 1, 1, false, false);
 		return;
 	}
@@ -1002,9 +1049,11 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target,
 	}
 
 	for (freq = 0 ; params[freq].pixel_freq ; ++freq) {
-		if (vclk_freq == params[freq].pixel_freq ||
-		    vclk_freq == FREQ_1000_1001(params[freq].pixel_freq)) {
-			if (vclk_freq != params[freq].pixel_freq)
+		if ((phy_freq == params[freq].phy_freq ||
+		     phy_freq == FREQ_1000_1001(params[freq].phy_freq/10)*10) &&
+		    (vclk_freq == params[freq].vclk_freq ||
+		     vclk_freq == FREQ_1000_1001(params[freq].vclk_freq))) {
+			if (vclk_freq != params[freq].vclk_freq)
 				vic_alternate_clock = true;
 			else
 				vic_alternate_clock = false;
@@ -1033,7 +1082,7 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned int target,
 		return;
 	}
 
-	meson_vclk_set(priv, params[freq].pll_base_freq,
+	meson_vclk_set(priv, params[freq].pll_freq,
 		       params[freq].pll_od1, params[freq].pll_od2,
 		       params[freq].pll_od3, params[freq].vid_pll_div,
 		       params[freq].vclk_div, hdmi_tx_div, venc_div,
diff --git a/drivers/gpu/drm/meson/meson_vclk.h b/drivers/gpu/drm/meson/meson_vclk.h
index 4bd8752da02a..c4d19ddfcd79 100644
--- a/drivers/gpu/drm/meson/meson_vclk.h
+++ b/drivers/gpu/drm/meson/meson_vclk.h
@@ -33,10 +33,11 @@ enum {
 enum drm_mode_status
 meson_vclk_dmt_supported_freq(struct meson_drm *priv, unsigned int freq);
 enum drm_mode_status
-meson_vclk_vic_supported_freq(unsigned int freq);
+meson_vclk_vic_supported_freq(unsigned int phy_freq, unsigned int vclk_freq);
 
 void meson_vclk_setup(struct meson_drm *priv, unsigned int target,
-		      unsigned int vclk_freq, unsigned int venc_freq,
-		      unsigned int dac_freq, bool hdmi_use_enci);
+		      unsigned int phy_freq, unsigned int vclk_freq,
+		      unsigned int venc_freq, unsigned int dac_freq,
+		      bool hdmi_use_enci);
 
 #endif /* __MESON_VCLK_H */
diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
index 6faca7313339..5460c4439612 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -958,6 +958,8 @@ bool meson_venc_hdmi_venc_repeat(int vic)
 EXPORT_SYMBOL_GPL(meson_venc_hdmi_venc_repeat);
 
 void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
+			      unsigned int ycrcb_map,
+			      bool yuv420_mode,
 			      struct drm_display_mode *mode)
 {
 	union meson_hdmi_venc_mode *vmode = NULL;
@@ -1508,8 +1510,8 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
 	writel_relaxed((use_enci ? 1 : 2) |
 		       (mode->flags & DRM_MODE_FLAG_PHSYNC ? 1 << 2 : 0) |
 		       (mode->flags & DRM_MODE_FLAG_PVSYNC ? 1 << 3 : 0) |
-		       4 << 5 |
-		       (venc_repeat ? 1 << 8 : 0) |
+		       (ycrcb_map << 5) |
+		       (venc_repeat || yuv420_mode ? 1 << 8 : 0) |
 		       (hdmi_repeat ? 1 << 12 : 0),
 		       priv->io_base + _REG(VPU_HDMI_SETTING));
 
diff --git a/drivers/gpu/drm/meson/meson_venc.h b/drivers/gpu/drm/meson/meson_venc.h
index 97eaebbfa0c4..5580bf38e381 100644
--- a/drivers/gpu/drm/meson/meson_venc.h
+++ b/drivers/gpu/drm/meson/meson_venc.h
@@ -33,6 +33,15 @@ enum {
 	MESON_VENC_MODE_HDMI,
 };
 
+enum {
+	MESON_VENC_MAP_CR_Y_CB = 0,
+	MESON_VENC_MAP_Y_CB_CR,
+	MESON_VENC_MAP_Y_CR_CB,
+	MESON_VENC_MAP_CB_CR_Y,
+	MESON_VENC_MAP_CB_Y_CR,
+	MESON_VENC_MAP_CR_CB_Y,
+};
+
 struct meson_cvbs_enci_mode {
 	unsigned int mode_tag;
 	unsigned int hso_begin; /* HSO begin position */
@@ -70,6 +79,8 @@ extern struct meson_cvbs_enci_mode meson_cvbs_enci_ntsc;
 void meson_venci_cvbs_mode_set(struct meson_drm *priv,
 			       struct meson_cvbs_enci_mode *mode);
 void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
+			      unsigned int ycrcb_map,
+			      bool yuv420_mode,
 			      struct drm_display_mode *mode);
 unsigned int meson_venci_get_field(struct meson_drm *priv);
 
diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
index 2c5341c881c4..c1bbc601cf13 100644
--- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
@@ -218,7 +218,8 @@ static void meson_venc_cvbs_encoder_mode_set(struct drm_encoder *encoder,
 			/* Setup 27MHz vclk2 for ENCI and VDAC */
 			meson_vclk_setup(priv, MESON_VCLK_TARGET_CVBS,
 					 MESON_VCLK_CVBS, MESON_VCLK_CVBS,
-					 MESON_VCLK_CVBS, true);
+					 MESON_VCLK_CVBS, MESON_VCLK_CVBS,
+					 true);
 			break;
 		}
 	}
-- 
2.21.0


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

Re: [PATCH 4/5] drm/meson: Add YUV420 output support

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> This patch adds support for the YUV420 output from the Amlogic Meson SoCs
> Video Processing Unit to the HDMI Controller.
>
> The YUV420 is obtained by generating a YUV444 pixel stream like
> the classic HDMI display modes, but then the Video Encoder output
> can be configured to down-sample the YUV444 pixel stream to a YUV420
> stream.
> In addition if pixel stream down-sampling, the Y Cb Cr components must
> also be mapped differently to align with the HDMI2.0 specifications.
>
> This mode needs a different clock generation scheme since the TMDS PHY
> clock must match the 10x ration with the YUV420 pixel clock, but

s/ration/ratio/

> the video encoder must run at 2x the pixel clock.
>
> This patch adds the TMDS PHY clock value in all the video clock setup
> in order to better support these specific uses cases and switch
> to the Common Clock framework for clocks handling in the future.
>
> When 420 is needed, it calls drm_bridge_format_set() for notify the
> bridge the input format has changed to YUV420.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

with a couple very minor nits to cleanup below...

> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c   | 100 +++++++++++++++++++-----
>  drivers/gpu/drm/meson/meson_vclk.c      |  93 ++++++++++++++++------
>  drivers/gpu/drm/meson/meson_vclk.h      |   7 +-
>  drivers/gpu/drm/meson/meson_venc.c      |   6 +-
>  drivers/gpu/drm/meson/meson_venc.h      |  11 +++
>  drivers/gpu/drm/meson/meson_venc_cvbs.c |   3 +-
>  6 files changed, 174 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 779da21143b9..5d67e2beba58 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -159,6 +159,7 @@ struct meson_dw_hdmi {
>  	struct regulator *hdmi_supply;
>  	u32 irq_stat;
>  	struct dw_hdmi *hdmi;
> +	unsigned long input_bus_format;
>  };
>  #define encoder_to_meson_dw_hdmi(x) \
>  	container_of(x, struct meson_dw_hdmi, encoder)
> @@ -308,6 +309,10 @@ static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
>  	struct meson_drm *priv = dw_hdmi->priv;
>  	unsigned int pixel_clock = mode->clock;
>  
> +	/* For 420, pixel clock is half unlike venc clock */
> +	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> +		pixel_clock /= 2;
> +
>  	if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
>  	    dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
>  		if (pixel_clock >= 371250) {
> @@ -383,25 +388,36 @@ static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
>  {
>  	struct meson_drm *priv = dw_hdmi->priv;
>  	int vic = drm_match_cea_mode(mode);
> +	unsigned int phy_freq;
>  	unsigned int vclk_freq;
>  	unsigned int venc_freq;
>  	unsigned int hdmi_freq;
>  
>  	vclk_freq = mode->clock;
>  
> +	/* For 420, pixel clock is half unlike venc clock */

minor grammar nit: put a comma after "half" (here and several other
places in the patch)

[...]

> @@ -722,17 +773,29 @@ static void meson_venc_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>  	struct meson_dw_hdmi *dw_hdmi = encoder_to_meson_dw_hdmi(encoder);
>  	struct meson_drm *priv = dw_hdmi->priv;
>  	int vic = drm_match_cea_mode(mode);
> +	unsigned int ycrcb_map = MESON_VENC_MAP_CB_Y_CR;
> +	bool yuv420_mode = false;
>  
>  	DRM_DEBUG_DRIVER("\"%s\" vic %d\n", mode->name, vic);
>  
> +	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24) {
> +		ycrcb_map = MESON_VENC_MAP_CR_Y_CB;
> +		yuv420_mode = true;
> +	}
> +
>  	/* VENC + VENC-DVI Mode setup */
> -	meson_venc_hdmi_mode_set(priv, vic, mode);
> +	meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, yuv420_mode, mode);
>  
>  	/* VCLK Set clock */
>  	dw_hdmi_set_vclk(dw_hdmi, mode);
>  
> -	/* Setup YUV444 to HDMI-TX, no 10bit diphering */
> -	writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
> +	if (dw_hdmi->input_bus_format == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> +		/* Setup YUV420 to HDMI-TX, no 10bit diphering */
> +		writel_relaxed(2 | (2 << 2),

nit: #define'd bitfields would be better, IIUC, I see these named as
"use average" and "Convert to 420" in the datasheet.

[...]

> @@ -1508,8 +1510,8 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
>  	writel_relaxed((use_enci ? 1 : 2) |
>  		       (mode->flags & DRM_MODE_FLAG_PHSYNC ? 1 << 2 : 0) |
>  		       (mode->flags & DRM_MODE_FLAG_PVSYNC ? 1 << 3 : 0) |
> -		       4 << 5 |
> -		       (venc_repeat ? 1 << 8 : 0) |
> +		       (ycrcb_map << 5) |
> +		       (venc_repeat || yuv420_mode ? 1 << 8 : 0) |

Lots of magic constants here, but I guess there's no new ones, so it's
just a(nother) reminder for a future readability cleanup.

>  		       (hdmi_repeat ? 1 << 12 : 0),
>  		       priv->io_base + _REG(VPU_HDMI_SETTING));
>  

Kevin

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

[PATCH 5/5] drm/meson: Output in YUV444 if sink supports it

[Neil Armstrong]

With the YUV420 handling, we can dynamically setup the HDMI output
pixel format depending on the mode and connector info.
So now, we can output in YUV444, which is the native video pipeline
format, directly to the HDMI Sink if it's supported without
necessarily involving the HDMI Controller CSC.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5d67e2beba58..8bf9db7f39a4 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -723,12 +723,23 @@ static int meson_venc_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
 	struct drm_display_mode *mode = &crtc_state->mode;
 	bool is_hdmi2_sink =
 		conn_state->connector->display_info.hdmi.scdc.supported;
+	bool specify_out_format = false;
+	u32 out_format;
 
 	if (drm_mode_is_420_only(info, mode) ||
 	    (!is_hdmi2_sink && drm_mode_is_420_also(info, mode)))
 		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
-	else
+	else {
 		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
+		if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444) {
+			out_format = MEDIA_BUS_FMT_YUV8_1X24;
+			specify_out_format = true;
+		}
+	}
+
+	/* Set a connector bus format if required */
+	drm_display_info_set_bus_formats(info, &out_format,
+					 (specify_out_format ? 1 : 0));
 
 	/* Specify the encoder output format to the bridge */
 	if (!drm_bridge_format_set(encoder->bridge,
-- 
2.21.0


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

Re: [PATCH 5/5] drm/meson: Output in YUV444 if sink supports it

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> With the YUV420 handling, we can dynamically setup the HDMI output
> pixel format depending on the mode and connector info.
> So now, we can output in YUV444, which is the native video pipeline
> format, directly to the HDMI Sink if it's supported without
> necessarily involving the HDMI Controller CSC.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 5d67e2beba58..8bf9db7f39a4 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -723,12 +723,23 @@ static int meson_venc_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
>  	struct drm_display_mode *mode = &crtc_state->mode;
>  	bool is_hdmi2_sink =
>  		conn_state->connector->display_info.hdmi.scdc.supported;
> +	bool specify_out_format = false;
> +	u32 out_format;
>  
>  	if (drm_mode_is_420_only(info, mode) ||
>  	    (!is_hdmi2_sink && drm_mode_is_420_also(info, mode)))
>  		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
> -	else
> +	else {

nit: if the else has {} you should add to the 'if' (even if the if side
is a single statement): c.f. end of this section of CodingStyle:
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces

>  		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
> +		if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444) {
> +			out_format = MEDIA_BUS_FMT_YUV8_1X24;
> +			specify_out_format = true;
> +		}
> +	}
> +
> +	/* Set a connector bus format if required */
> +	drm_display_info_set_bus_formats(info, &out_format,
> +					 (specify_out_format ? 1 : 0));
>  

Otherwise,

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

Kevin


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