[PATCH v17 0/6] MIPS: JZ4780 and CI20 HDMI

[PATCH v17 0/6] MIPS: JZ4780 and CI20 HDMI

[H. Nikolaus Schaller]

PATCH V17 2022-04-06 18:26:08:
* removed our local attempts to enable HPD because
  "drm/bridge_connector: enable HPD by default if supported"
  became available and solves the issue globally (by nikita.yoush@cogentembedded.com)
* add new mechanism to disable Color Space Conversion (by narmstrong@baylibre.com)
* rebased to v5.18-rc1 based drm-misc/drm-misc-next 

PATCH V16 2022-02-26 18:13:02:
* fixed and renamed dw-hdmi bus negotiation patch (by narmstrong@baylibre.com)
* reordered and merged HPD fix (suggested by paul@crapouillou.net)
* fixed MODULE_ALIAS for dw-hdmi-ingenic (reported by paul@crapouillou.net)
* dropped some already merged commits from the series

PATCH V15 2022-02-12 16:50:54:
* remove already (elsewhere) merged commits (suggested by paul@crapouillou.net)
* clarify commit message for (now) 1/7 ((suggested by paul@crapouillou.net))

PATCH V14 2022-02-12 15:19:25:
* make compatible to c03d0b52ff71d5 ("drm/connector: Fix typo in output format")
* move "dw-hdmi/ingenic-dw-hdmi: repair interworking with hdmi-connector" before
  drm/ingenic: Add dw-hdmi driver specialization for jz4780 (by paul@crapouillou.net)
* split introduction of dw_hdmi_enable_poll() into separate patch
* explicitly mark plane f0 as not working in jz4780 (suggested by paul@crapouillou.net)
* drop 1/9 since it is now in drm-misc/drm-misc-next

PATCH V13 2022-02-02 17:31:22:
* 7/9: remove call to gpiod_set_value() because of GPIOD_OUT_HIGH (by paul@crapouillou.net)
* 4/9: replace ".." by "." (by paul@crapouillou.net)
* 3/9: remove old hdmi-5v-power in the example (by paul@crapouillou.net)
* 2/9: disable handling of plane f0 only for jz4780 (by paul@crapouillou.net)

PATCH V12 2022-01-31 13:26:54:
This version reworks how hdmi ddc power is controlled by connector and not
by ddc/hdmi bridge driver.

Also some patches of the previous version of this series have been removed
since they are already applied to mips-next/linux/next/v5.17-rc1.

Fixes and changes:

- repair interworking of dw-hdmi with connector-hdmi (by hns@goldelico.com)
- fix JZ_REG_LCD_OSDC setup for jz4780 (by hns@goldelico.com and paul@crapouillou.net)
- adjustments for ci20.dts to use connector gpio for +5v (suggested by several)
- to add control of "ddc-en-gpios" to hdmi-connector driver (by hns@goldelico.com)
- regulator code removed because we now use the "ddc-en-gpios" of the connector
  driver (suggested by paul@crapouillou.net)
- bindings: addition of "ddc-i2c-bus" and "hdmi-5v-supply" removed (suggested by robh+dt@kernel.org)
- rebase on v5.17-rc2

PATCH V11 2021-12-02 19:39:52:
- patch 4/8: change devm_regulator_get_optional to devm_regulator_get and
             remove NULL check (requested by broonie@kernel.org)
- patch 3/8: make hdmi-5v-supply required (requested by broonie@kernel.org)

PATCH V10 2021-11-30 22:26:41:
- patch 3/8: fix $id and $ref paths (found by robh@kernel.org)

PATCH V9 2021-11-24 22:29:14:
- patch 6/8: remove optional <0> for assigned-clocks and unintentionally included "unwedge" setup (found by paul@crapouillou.net)
- patch 4/8: some cosmetics
             make regulator enable/disable only if not NULL (found by paul@crapouillou.net)
             simplify/fix error handling and driver cleanup on remove (proposed by paul@crapouillou.net)
- patch 3/8: fix #include path in example (found by paul@crapouillou.net)
             fix missing "i" in unevaluatedProperties (found by robh@kernel.org)
             fix 4 spaces indentation for required: property (found by robh@kernel.org)

PATCH V8 2021-11-23 19:14:00:
- fix a bad editing result from patch 2/8 (found by paul@crapouillou.net)

PATCH V7 2021-11-23 18:46:23:
- changed gpio polarity of hdmi_power to 0 (suggested by paul@crapouillou.net)
- fixed LCD1 irq number (bug found by paul@crapouillou.net)
- removed "- 4" for calculating max_register (suggested by paul@crapouillou.net)
- use unevaluatedPropertes instead of additionalProperties (suggested by robh@kernel.org)
- moved and renamed ingenic,jz4780-hdmi.yaml (suggested by robh@kernel.org)
- adjusted assigned-clocks changes to upstream which added some for SSI (by hns@goldelico.com)
- rebased and tested with v5.16-rc2 + patch set drm/ingenic by paul@crapouillou.net (by hns@goldelico.com)

PATCH V6 2021-11-10 20:43:33:
- changed CONFIG_DRM_INGENIC_DW_HDMI to "m" (by hns@goldelico.com)
- made ingenic-dw-hdmi an independent platform driver which can be compiled as module
  and removed error patch fixes for IPU (suggested by paul@crapouillou.net)
- moved assigned-clocks from jz4780.dtsi to ci20.dts (suggested by paul@crapouillou.net)
- fixed reg property in jz4780.dtsi to cover all registers incl. gamma and vee (by hns@goldelico.com)
- added a base patch to calculate regmap size from DTS reg property (requested by paul@crapouillou.net)
- restored resetting all bits except one in LCDOSDC (requested by paul@crapouillou.net)
- clarified setting of cpos (suggested by paul@crapouillou.net)
- moved bindings definition for ddc-i2c-bus (suggested by paul@crapouillou.net)
- simplified mask definitions for JZ_LCD_DESSIZE (requested by paul@crapouillou.net)
- removed setting alpha premultiplication (suggested by paul@crapouillou.net)
- removed some comments (suggested by paul@crapouillou.net)

PATCH V5 2021-10-05 14:28:44:
- dropped mode_fixup and timings support in dw-hdmi as it is no longer needed in this V5 (by hns@goldelico.com)
- dropped "drm/ingenic: add some jz4780 specific features" (stimulated by paul@crapouillou.net)
- fixed typo in commit subject: "synopsis" -> "synopsys" (by hns@goldelico.com)
- swapped clocks in jz4780.dtsi to match synopsys,dw-hdmi.yaml (by hns@goldelico.com)
- improved, simplified, fixed, dtbschecked ingenic-jz4780-hdmi.yaml and made dependent of bridge/synopsys,dw-hdmi.yaml (based on suggestions by maxime@cerno.tech)
- fixed binding vs. driver&DTS use of hdmi-5v regulator (suggested by maxime@cerno.tech)
- dropped "drm/bridge: synopsis: Fix to properly handle HPD" - was a no longer needed workaround for a previous version
  (suggested by maxime@cerno.tech)

PATCH V4 2021-09-27 18:44:38:
- fix setting output_port = 1 (issue found by paul@crapouillou.net)
- ci20.dts: convert to use hdmi-connector (by hns@goldelico.com)
- add a hdmi-regulator to control +5V power (by hns@goldelico.com)
- added a fix to dw-hdmi to call drm_kms_helper_hotplug_event on plugin event detection (by hns@goldelico.com)
- always allocate extended descriptor but initialize only for jz4780 (by hns@goldelico.com)
- updated to work on top of "[PATCH v3 0/6] drm/ingenic: Various improvements v3" (by paul@crapouillou.net)
- rebased to v5.13-rc3

PATCH V3 2021-08-08 07:10:50:
This series adds HDMI support for JZ4780 and CI20 board (and fixes one IPU related issue in registration error path)
- [patch 1/8] switched from mode_fixup to atomic_check (suggested by robert.foss@linaro.org)
  - the call to the dw-hdmi specialization is still called mode_fixup
- [patch 3/8] diverse fixes for ingenic-drm-drv (suggested by paul@crapouillou.net)
  - factor out some non-HDMI features of the jz4780 into a separate patch
  - multiple fixes around max height
  - do not change regmap config but a copy on stack
  - define some constants
  - factor out fixing of drm_init error path for IPU into separate patch
  - use FIELD_PREP()
- [patch 8/8] conversion to component framework dropped (suggested by Laurent.pinchart@ideasonboard.com and paul@crapouillou.net)

PATCH V2 2021-08-05 16:08:05:
- code and commit messages revisited for checkpatch warnings
- rebased on v5.14-rc4
- include (failed, hence RFC 8/8) attempt to convert to component framework
  (was suggested by Paul Cercueil <paul@crapouillou.net> a while ago)

This series adds HDMI support for JZ4780 and CI20 board



H. Nikolaus Schaller (2):
  drm/bridge: dw-hdmi: add YCBCR formats only if CSC is available
  drm/bridge: display-connector: add ddc-en gpio support

Neil Armstrong (2):
  drm/bridge: dw-hdmi: filter out YUV output formats when DVI
  drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

Paul Boddie (1):
  drm/ingenic: Add dw-hdmi driver specialization for jz4780

Paul Cercueil (1):
  drm/ingenic: Implement proper .atomic_get_input_bus_fmts

 drivers/gpu/drm/bridge/display-connector.c |  15 +++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  | 119 ++++++++++++++-------
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.h  |   1 +
 drivers/gpu/drm/ingenic/Kconfig            |   9 ++
 drivers/gpu/drm/ingenic/Makefile           |   1 +
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c  |  28 ++++-
 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c  | 104 ++++++++++++++++++
 include/drm/bridge/dw_hdmi.h               |   1 +
 8 files changed, 238 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c

-- 
2.33.0

[PATCH v17 1/6] drm/ingenic: Implement proper .atomic_get_input_bus_fmts

[H. Nikolaus Schaller]

From: Paul Cercueil <paul@crapouillou.net>

The .atomic_get_input_bus_fmts() callback of our top bridge should
return the possible input formats for a given output format. If the
requested output format is not supported, then NULL should be returned,
otherwise the bus format negociation will end with a bus format that the
encoder does not support.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 28 ++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index a4f5a323f4906..8eb0ad501a7b9 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -833,6 +833,32 @@ static int ingenic_drm_bridge_atomic_check(struct drm_bridge *bridge,
 	}
 }
 
+static u32 *
+ingenic_drm_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+					     struct drm_bridge_state *bridge_state,
+					     struct drm_crtc_state *crtc_state,
+					     struct drm_connector_state *conn_state,
+					     u32 output_fmt,
+					     unsigned int *num_input_fmts)
+{
+	switch (output_fmt) {
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB666_1X18:
+	case MEDIA_BUS_FMT_RGB565_1X16:
+	case MEDIA_BUS_FMT_RGB888_3X8:
+	case MEDIA_BUS_FMT_RGB888_3X8_DELTA:
+		break;
+	default:
+		*num_input_fmts = 0;
+		return NULL;
+	}
+
+	return drm_atomic_helper_bridge_propagate_bus_fmt(bridge, bridge_state,
+							  crtc_state, conn_state,
+							  output_fmt,
+							  num_input_fmts);
+}
+
 static irqreturn_t ingenic_drm_irq_handler(int irq, void *arg)
 {
 	struct ingenic_drm *priv = drm_device_get_priv(arg);
@@ -984,7 +1010,7 @@ static const struct drm_bridge_funcs ingenic_drm_bridge_funcs = {
 	.atomic_reset		= drm_atomic_helper_bridge_reset,
 	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
-	.atomic_get_input_bus_fmts = drm_atomic_helper_bridge_propagate_bus_fmt,
+	.atomic_get_input_bus_fmts = ingenic_drm_bridge_atomic_get_input_bus_fmts,
 };
 
 static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
-- 
2.33.0

[PATCH v17 2/6] drm/ingenic: Add dw-hdmi driver specialization for jz4780

[H. Nikolaus Schaller]

From: Paul Boddie <paul@boddie.org.uk>

A specialisation of the generic Synopsys HDMI driver is employed for
JZ4780 HDMI support. This requires a new driver, plus device tree and
configuration modifications.

Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code.

Note that there is no hpd-gpio installed on the CI20 board HDMI
connector. Hence there is no hpd detection by the connector driver
and we have to enable polling in the dw-hdmi core driver.

For that we need to set .poll_enabled but that struct component
can only be accessed by core code. Hence we use the public
setter function drm_kms_helper_hotplug_event() introduced before.

Also note that we disable Color Space Conversion since it is not
working on jz4780.

Signed-off-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/ingenic/Kconfig           |   9 ++
 drivers/gpu/drm/ingenic/Makefile          |   1 +
 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 104 ++++++++++++++++++++++
 3 files changed, 114 insertions(+)
 create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c

diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
index 001f59fb06d56..090830bcbde7f 100644
--- a/drivers/gpu/drm/ingenic/Kconfig
+++ b/drivers/gpu/drm/ingenic/Kconfig
@@ -24,4 +24,13 @@ config DRM_INGENIC_IPU
 
 	  The Image Processing Unit (IPU) will appear as a second primary plane.
 
+config DRM_INGENIC_DW_HDMI
+	tristate "Ingenic specific support for Synopsys DW HDMI"
+	depends on MACH_JZ4780
+	select DRM_DW_HDMI
+	help
+	  Choose this option to enable Synopsys DesignWare HDMI based driver.
+	  If you want to enable HDMI on Ingenic JZ4780 based SoC, you should
+	  select this option.
+
 endif
diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile
index d313326bdddbb..f10cc1c5a5f22 100644
--- a/drivers/gpu/drm/ingenic/Makefile
+++ b/drivers/gpu/drm/ingenic/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o
 ingenic-drm-y = ingenic-drm-drv.o
 ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o
+obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o
diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
new file mode 100644
index 0000000000000..10c56183613b0
--- /dev/null
+++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
+ * Copyright (C) 2019, 2020 Paul Boddie <paul@boddie.org.uk>
+ *
+ * Derived from dw_hdmi-imx.c with i.MX portions removed.
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_print.h>
+
+static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = {
+	{ 45250000,  { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, 0x0000 } } },
+	{ 92500000,  { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 0x0005 } } },
+	{ 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 0x000a } } },
+	{ 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 0x000f } } },
+	{ ~0UL,      { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, 0x0000 } } }
+};
+
+static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = {
+	/*pixelclk     bpp8    bpp10   bpp12 */
+	{ 54000000,  { 0x091c, 0x091c, 0x06dc } },
+	{ 58400000,  { 0x091c, 0x06dc, 0x06dc } },
+	{ 72000000,  { 0x06dc, 0x06dc, 0x091c } },
+	{ 74250000,  { 0x06dc, 0x0b5c, 0x091c } },
+	{ 118800000, { 0x091c, 0x091c, 0x06dc } },
+	{ 216000000, { 0x06dc, 0x0b5c, 0x091c } },
+	{ ~0UL,      { 0x0000, 0x0000, 0x0000 } },
+};
+
+/*
+ * Resistance term 133Ohm Cfg
+ * PREEMP config 0.00
+ * TX/CK level 10
+ */
+static const struct dw_hdmi_phy_config ingenic_phy_config[] = {
+	/*pixelclk   symbol   term   vlev */
+	{ 216000000, 0x800d, 0x0005, 0x01ad},
+	{ ~0UL,      0x0000, 0x0000, 0x0000}
+};
+
+static enum drm_mode_status
+ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
+			   const struct drm_display_info *info,
+			   const struct drm_display_mode *mode)
+{
+	if (mode->clock < 13500)
+		return MODE_CLOCK_LOW;
+	/* FIXME: Hardware is capable of 270MHz, but setup data is missing. */
+	if (mode->clock > 216000)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = {
+	.mpll_cfg   = ingenic_mpll_cfg,
+	.cur_ctr    = ingenic_cur_ctr,
+	.phy_config = ingenic_phy_config,
+	.mode_valid = ingenic_dw_hdmi_mode_valid,
+	.output_port	= 1,
+	.disable_csc	= 0,	/* DRM_COLOR_FORMAT_YCBCR do not work */
+};
+
+static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = {
+	{ .compatible = "ingenic,jz4780-dw-hdmi" },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids);
+
+static void ingenic_dw_hdmi_cleanup(void *data)
+{
+	struct dw_hdmi *hdmi = (struct dw_hdmi *)data;
+
+	dw_hdmi_remove(hdmi);
+}
+
+static int ingenic_dw_hdmi_probe(struct platform_device *pdev)
+{
+	struct dw_hdmi *hdmi;
+
+	hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi);
+}
+
+static struct platform_driver ingenic_dw_hdmi_driver = {
+	.probe  = ingenic_dw_hdmi_probe,
+	.driver = {
+		.name = "dw-hdmi-ingenic",
+		.of_match_table = ingenic_dw_hdmi_dt_ids,
+	},
+};
+module_platform_driver(ingenic_dw_hdmi_driver);
+
+MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:dw-hdmi-ingenic");
-- 
2.33.0

[PATCH v17 3/6] drm/bridge: dw-hdmi: filter out YUV output formats when DVI

[H. Nikolaus Schaller]

From: Neil Armstrong <narmstrong@baylibre.com>

When the display is not an HDMI sink, only the RGB output format is
valid. Thus stop returning YUV output formats when sink is not HDMI.

Fixes: 6c3c719936da ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a563460f8d209..f50af40e10340 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2538,6 +2538,7 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 	struct drm_connector *conn = conn_state->connector;
 	struct drm_display_info *info = &conn->display_info;
 	struct drm_display_mode *mode = &crtc_state->mode;
+	struct dw_hdmi *hdmi = bridge->driver_private;
 	u8 max_bpc = conn_state->max_requested_bpc;
 	bool is_hdmi2_sink = info->hdmi.scdc.supported ||
 			     (info->color_formats & DRM_COLOR_FORMAT_YCBCR420);
@@ -2564,7 +2565,7 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 	 * If the current mode enforces 4:2:0, force the output but format
 	 * to 4:2:0 and do not add the YUV422/444/RGB formats
 	 */
-	if (conn->ycbcr_420_allowed &&
+	if (hdmi->sink_is_hdmi && conn->ycbcr_420_allowed &&
 	    (drm_mode_is_420_only(info, mode) ||
 	     (is_hdmi2_sink && drm_mode_is_420_also(info, mode)))) {
 
@@ -2595,36 +2596,36 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 	 */
 
 	if (max_bpc >= 16 && info->bpc == 16) {
-		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+		if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
 			output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
 
 		output_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
 	}
 
 	if (max_bpc >= 12 && info->bpc >= 12) {
-		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+		if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
 			output_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
 
-		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+		if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
 			output_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
 
 		output_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
 	}
 
 	if (max_bpc >= 10 && info->bpc >= 10) {
-		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+		if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
 			output_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
 
-		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+		if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
 			output_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
 
 		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
 	}
 
-	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
 		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
 
-	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
 		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
 
 	/* Default 8bit RGB fallback */
-- 
2.33.0

[PATCH v17 4/6] drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

[H. Nikolaus Schaller]

From: Neil Armstrong <narmstrong@baylibre.com>

The dw-hdmi integrates an optional Color Space Conversion feature used
to handle color-space conversions.

On some platforms, the CSC isn't built-in or non-functional.

This adds the necessary code to disable the CSC functionality
and limit the bus format negotiation to force using the same
input bus format as the output bus format.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 100 +++++++++++++++-------
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |   1 +
 include/drm/bridge/dw_hdmi.h              |   1 +
 3 files changed, 71 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f50af40e10340..b5a665c5e406e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -158,6 +158,8 @@ struct dw_hdmi {
 	struct hdmi_data_info hdmi_data;
 	const struct dw_hdmi_plat_data *plat_data;
 
+	bool csc_available;		/* indicates if the CSC engine is usable */
+
 	int vic;
 
 	u8 edid[HDMI_EDID_LEN];
@@ -1009,9 +1011,10 @@ static int is_color_space_interpolation(struct dw_hdmi *hdmi)
 
 static bool is_csc_needed(struct dw_hdmi *hdmi)
 {
-	return is_color_space_conversion(hdmi) ||
-	       is_color_space_decimation(hdmi) ||
-	       is_color_space_interpolation(hdmi);
+	return hdmi->csc_available &&
+	       (is_color_space_conversion(hdmi) ||
+		is_color_space_decimation(hdmi) ||
+		is_color_space_interpolation(hdmi));
 }
 
 static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
@@ -1064,6 +1067,9 @@ static void hdmi_video_csc(struct dw_hdmi *hdmi)
 	int interpolation = HDMI_CSC_CFG_INTMODE_DISABLE;
 	int decimation = 0;
 
+	if (!hdmi->csc_available)
+		return;
+
 	/* YCC422 interpolation to 444 mode */
 	if (is_color_space_interpolation(hdmi))
 		interpolation = HDMI_CSC_CFG_INTMODE_CHROMA_INT_FORMULA1;
@@ -2665,6 +2671,7 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
 					u32 output_fmt,
 					unsigned int *num_input_fmts)
 {
+	struct dw_hdmi *hdmi = bridge->driver_private;
 	u32 *input_fmts;
 	unsigned int i = 0;
 
@@ -2683,62 +2690,81 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
 	/* 8bit */
 	case MEDIA_BUS_FMT_RGB888_1X24:
 		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
-		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+			input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+		}
 		break;
 	case MEDIA_BUS_FMT_YUV8_1X24:
 		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
-		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		}
 		break;
 	case MEDIA_BUS_FMT_UYVY8_1X16:
 		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		}
 		break;
 
 	/* 10bit */
 	case MEDIA_BUS_FMT_RGB101010_1X30:
 		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
-		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
+			input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
+		}
 		break;
 	case MEDIA_BUS_FMT_YUV10_1X30:
 		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
-		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
+			input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+		}
 		break;
 	case MEDIA_BUS_FMT_UYVY10_1X20:
 		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
+			input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
+		}
 		break;
 
 	/* 12bit */
 	case MEDIA_BUS_FMT_RGB121212_1X36:
 		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
-		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
+			input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
+		}
 		break;
 	case MEDIA_BUS_FMT_YUV12_1X36:
 		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
-		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
+			input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+		}
 		break;
 	case MEDIA_BUS_FMT_UYVY12_1X24:
 		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+		if (hdmi->csc_available) {
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
+			input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
+		}
 		break;
 
 	/* 16bit */
 	case MEDIA_BUS_FMT_RGB161616_1X48:
 		input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
+		if (hdmi->csc_available)
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
 		break;
 	case MEDIA_BUS_FMT_YUV16_1X48:
-		input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
-		input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
+		if (hdmi->csc_available)
+			input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
 		break;
 
 	/*YUV 4:2:0 */
@@ -2767,15 +2793,24 @@ static int dw_hdmi_bridge_atomic_check(struct drm_bridge *bridge,
 {
 	struct dw_hdmi *hdmi = bridge->driver_private;
 
-	hdmi->hdmi_data.enc_out_bus_format =
-			bridge_state->output_bus_cfg.format;
+	if (!hdmi->csc_available &&
+	    bridge_state->output_bus_cfg.format != bridge_state->input_bus_cfg.format) {
+		dev_warn(hdmi->dev, "different input format 0x%04x & output format 0x%04x while CSC isn't usable, fallback to safe format\n",
+			 bridge_state->input_bus_cfg.format,
+			 bridge_state->output_bus_cfg.format);
+		hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_FIXED;
+		hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_FIXED;
+	} else {
+		hdmi->hdmi_data.enc_out_bus_format =
+				bridge_state->output_bus_cfg.format;
 
-	hdmi->hdmi_data.enc_in_bus_format =
-			bridge_state->input_bus_cfg.format;
+		hdmi->hdmi_data.enc_in_bus_format =
+				bridge_state->input_bus_cfg.format;
 
-	dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
-		bridge_state->input_bus_cfg.format,
-		bridge_state->output_bus_cfg.format);
+		dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
+			bridge_state->input_bus_cfg.format,
+			bridge_state->output_bus_cfg.format);
+	}
 
 	return 0;
 }
@@ -3481,6 +3516,9 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->cec = platform_device_register_full(&pdevinfo);
 	}
 
+	/* Get CSC useability from config0 register and permit override for platforms */
+	hdmi->csc_available = !plat_data->disable_csc || (config0 & HDMI_CONFIG0_CSC);
+
 	drm_bridge_add(&hdmi->bridge);
 
 	return hdmi;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
index 1999db05bc3b2..279722e4d1898 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
@@ -541,6 +541,7 @@ enum {
 
 /* CONFIG0_ID field values */
 	HDMI_CONFIG0_I2S = 0x10,
+	HDMI_CONFIG0_CSC = 0x04,
 	HDMI_CONFIG0_CEC = 0x02,
 
 /* CONFIG1_ID field values */
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 2a1f85f9a8a3f..b2f689cbe864c 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -157,6 +157,7 @@ struct dw_hdmi_plat_data {
 			     unsigned long mpixelclock);
 
 	unsigned int disable_cec : 1;
+	unsigned int disable_csc : 1;
 };
 
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
-- 
2.33.0

[PATCH v17 5/6] drm/bridge: dw-hdmi: add YCBCR formats only if CSC is available

[H. Nikolaus Schaller]

otherwise they will produce a black HDMI screen.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index b5a665c5e406e..ec40dd8865065 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2628,10 +2628,12 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
 	}
 
-	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+	if (hdmi->csc_available && hdmi->sink_is_hdmi &&
+	    (info->color_formats & DRM_COLOR_FORMAT_YCBCR422))
 		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
 
-	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+	if (hdmi->csc_available && hdmi->sink_is_hdmi &&
+	    (info->color_formats & DRM_COLOR_FORMAT_YCBCR444))
 		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
 
 	/* Default 8bit RGB fallback */
-- 
2.33.0

[PATCH v17 6/6] drm/bridge: display-connector: add ddc-en gpio support

[H. Nikolaus Schaller]

"hdmi-connector.yaml" bindings defines an optional property
"ddc-en-gpios" for a single gpio to enable DDC operation.

Usually this controls +5V power on the HDMI connector.
This +5V may also be needed for HPD.

This was not reflected in code but is needed to make the CI20
board work.

Now, the driver activates the ddc gpio after probe and
deactivates after remove so it is "almost on".

But only if this driver is loaded (and not e.g. blacklisted
as module).

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/display-connector.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index d24f5b90feabf..e4d52a7e31b71 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -24,6 +24,7 @@ struct display_connector {
 	int			hpd_irq;
 
 	struct regulator	*dp_pwr;
+	struct gpio_desc	*ddc_en;
 };
 
 static inline struct display_connector *
@@ -345,6 +346,17 @@ static int display_connector_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* enable DDC */
+	if (type == DRM_MODE_CONNECTOR_HDMIA) {
+		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
+						       GPIOD_OUT_HIGH);
+
+		if (IS_ERR(conn->ddc_en)) {
+			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
+			return PTR_ERR(conn->ddc_en);
+		}
+	}
+
 	conn->bridge.funcs = &display_connector_bridge_funcs;
 	conn->bridge.of_node = pdev->dev.of_node;
 
@@ -373,6 +385,9 @@ static int display_connector_remove(struct platform_device *pdev)
 {
 	struct display_connector *conn = platform_get_drvdata(pdev);
 
+	if (conn->ddc_en)
+		gpiod_set_value(conn->ddc_en, 0);
+
 	if (conn->dp_pwr)
 		regulator_disable(conn->dp_pwr);
 
-- 
2.33.0

Re: [PATCH v17 6/6] drm/bridge: display-connector: add ddc-en gpio support

[Laurent Pinchart]

Hi Nikolaus,

Thank you for the patch.

On Wed, Apr 06, 2022 at 06:26:08PM +0200, H. Nikolaus Schaller wrote:
> "hdmi-connector.yaml" bindings defines an optional property
> "ddc-en-gpios" for a single gpio to enable DDC operation.
> 
> Usually this controls +5V power on the HDMI connector.
> This +5V may also be needed for HPD.
> 
> This was not reflected in code but is needed to make the CI20
> board work.
> 
> Now, the driver activates the ddc gpio after probe and
> deactivates after remove so it is "almost on".
> 
> But only if this driver is loaded (and not e.g. blacklisted
> as module).
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/bridge/display-connector.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> index d24f5b90feabf..e4d52a7e31b71 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
> @@ -24,6 +24,7 @@ struct display_connector {
>  	int			hpd_irq;
>  
>  	struct regulator	*dp_pwr;
> +	struct gpio_desc	*ddc_en;
>  };
>  
>  static inline struct display_connector *
> @@ -345,6 +346,17 @@ static int display_connector_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/* enable DDC */
> +	if (type == DRM_MODE_CONNECTOR_HDMIA) {
> +		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
> +						       GPIOD_OUT_HIGH);
> +
> +		if (IS_ERR(conn->ddc_en)) {
> +			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
> +			return PTR_ERR(conn->ddc_en);
> +		}
> +	}
> +
>  	conn->bridge.funcs = &display_connector_bridge_funcs;
>  	conn->bridge.of_node = pdev->dev.of_node;
>  
> @@ -373,6 +385,9 @@ static int display_connector_remove(struct platform_device *pdev)
>  {
>  	struct display_connector *conn = platform_get_drvdata(pdev);
>  
> +	if (conn->ddc_en)
> +		gpiod_set_value(conn->ddc_en, 0);
> +
>  	if (conn->dp_pwr)
>  		regulator_disable(conn->dp_pwr);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v17 4/6] drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

[Neil Armstrong]

Hi,

On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
> From: Neil Armstrong <narmstrong@baylibre.com>
> 
> The dw-hdmi integrates an optional Color Space Conversion feature used
> to handle color-space conversions.
> 
> On some platforms, the CSC isn't built-in or non-functional.
> 
> This adds the necessary code to disable the CSC functionality
> and limit the bus format negotiation to force using the same
> input bus format as the output bus format.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 100 +++++++++++++++-------
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |   1 +
>   include/drm/bridge/dw_hdmi.h              |   1 +
>   3 files changed, 71 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index f50af40e10340..b5a665c5e406e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -158,6 +158,8 @@ struct dw_hdmi {
>   	struct hdmi_data_info hdmi_data;
>   	const struct dw_hdmi_plat_data *plat_data;
>   
> +	bool csc_available;		/* indicates if the CSC engine is usable */
> +
>   	int vic;
>   
>   	u8 edid[HDMI_EDID_LEN];
> @@ -1009,9 +1011,10 @@ static int is_color_space_interpolation(struct dw_hdmi *hdmi)
>   
>   static bool is_csc_needed(struct dw_hdmi *hdmi)
>   {
> -	return is_color_space_conversion(hdmi) ||
> -	       is_color_space_decimation(hdmi) ||
> -	       is_color_space_interpolation(hdmi);
> +	return hdmi->csc_available &&
> +	       (is_color_space_conversion(hdmi) ||
> +		is_color_space_decimation(hdmi) ||
> +		is_color_space_interpolation(hdmi));
>   }
>   
>   static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
> @@ -1064,6 +1067,9 @@ static void hdmi_video_csc(struct dw_hdmi *hdmi)
>   	int interpolation = HDMI_CSC_CFG_INTMODE_DISABLE;
>   	int decimation = 0;
>   
> +	if (!hdmi->csc_available)
> +		return;
> +
>   	/* YCC422 interpolation to 444 mode */
>   	if (is_color_space_interpolation(hdmi))
>   		interpolation = HDMI_CSC_CFG_INTMODE_CHROMA_INT_FORMULA1;
> @@ -2665,6 +2671,7 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>   					u32 output_fmt,
>   					unsigned int *num_input_fmts)
>   {
> +	struct dw_hdmi *hdmi = bridge->driver_private;
>   	u32 *input_fmts;
>   	unsigned int i = 0;
>   
> @@ -2683,62 +2690,81 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>   	/* 8bit */
>   	case MEDIA_BUS_FMT_RGB888_1X24:
>   		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> +		}
>   		break;
>   	case MEDIA_BUS_FMT_YUV8_1X24:
>   		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		}
>   		break;
>   	case MEDIA_BUS_FMT_UYVY8_1X16:
>   		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		}
>   		break;
>   
>   	/* 10bit */
>   	case MEDIA_BUS_FMT_RGB101010_1X30:
>   		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> +		}
>   		break;
>   	case MEDIA_BUS_FMT_YUV10_1X30:
>   		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> +		}
>   		break;
>   	case MEDIA_BUS_FMT_UYVY10_1X20:
>   		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
> +		}
>   		break;
>   
>   	/* 12bit */
>   	case MEDIA_BUS_FMT_RGB121212_1X36:
>   		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> +		}
>   		break;
>   	case MEDIA_BUS_FMT_YUV12_1X36:
>   		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> +		}
>   		break;
>   	case MEDIA_BUS_FMT_UYVY12_1X24:
>   		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> +		if (hdmi->csc_available) {
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
> +		}
>   		break;
>   
>   	/* 16bit */
>   	case MEDIA_BUS_FMT_RGB161616_1X48:
>   		input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> +		if (hdmi->csc_available)
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>   		break;
>   	case MEDIA_BUS_FMT_YUV16_1X48:
> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
> +		if (hdmi->csc_available)
> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>   		break;
>   
>   	/*YUV 4:2:0 */
> @@ -2767,15 +2793,24 @@ static int dw_hdmi_bridge_atomic_check(struct drm_bridge *bridge,
>   {
>   	struct dw_hdmi *hdmi = bridge->driver_private;
>   
> -	hdmi->hdmi_data.enc_out_bus_format =
> -			bridge_state->output_bus_cfg.format;
> +	if (!hdmi->csc_available &&
> +	    bridge_state->output_bus_cfg.format != bridge_state->input_bus_cfg.format) {
> +		dev_warn(hdmi->dev, "different input format 0x%04x & output format 0x%04x while CSC isn't usable, fallback to safe format\n",
> +			 bridge_state->input_bus_cfg.format,
> +			 bridge_state->output_bus_cfg.format);
> +		hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_FIXED;
> +		hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_FIXED;
> +	} else {
> +		hdmi->hdmi_data.enc_out_bus_format =
> +				bridge_state->output_bus_cfg.format;
>   
> -	hdmi->hdmi_data.enc_in_bus_format =
> -			bridge_state->input_bus_cfg.format;
> +		hdmi->hdmi_data.enc_in_bus_format =
> +				bridge_state->input_bus_cfg.format;
>   
> -	dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
> -		bridge_state->input_bus_cfg.format,
> -		bridge_state->output_bus_cfg.format);
> +		dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
> +			bridge_state->input_bus_cfg.format,
> +			bridge_state->output_bus_cfg.format);
> +	}
>   
>   	return 0;
>   }
> @@ -3481,6 +3516,9 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>   		hdmi->cec = platform_device_register_full(&pdevinfo);
>   	}
>   
> +	/* Get CSC useability from config0 register and permit override for platforms */
> +	hdmi->csc_available = !plat_data->disable_csc || (config0 & HDMI_CONFIG0_CSC);
> +
>   	drm_bridge_add(&hdmi->bridge);
>   
>   	return hdmi;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> index 1999db05bc3b2..279722e4d1898 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -541,6 +541,7 @@ enum {
>   
>   /* CONFIG0_ID field values */
>   	HDMI_CONFIG0_I2S = 0x10,
> +	HDMI_CONFIG0_CSC = 0x04,
>   	HDMI_CONFIG0_CEC = 0x02,
>   
>   /* CONFIG1_ID field values */
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 2a1f85f9a8a3f..b2f689cbe864c 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -157,6 +157,7 @@ struct dw_hdmi_plat_data {
>   			     unsigned long mpixelclock);
>   
>   	unsigned int disable_cec : 1;
> +	unsigned int disable_csc : 1;
>   };
>   
>   struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,

Is this really still needed now you filter correctly the possible
modes in patch 1 ?

Neil

Re: [PATCH v17 5/6] drm/bridge: dw-hdmi: add YCBCR formats only if CSC is available

[Neil Armstrong]

On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
> otherwise they will produce a black HDMI screen.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index b5a665c5e406e..ec40dd8865065 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2628,10 +2628,12 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>   		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>   	}
>   
> -	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
> +	if (hdmi->csc_available && hdmi->sink_is_hdmi &&
> +	    (info->color_formats & DRM_COLOR_FORMAT_YCBCR422))
>   		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>   
> -	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
> +	if (hdmi->csc_available && hdmi->sink_is_hdmi &&
> +	    (info->color_formats & DRM_COLOR_FORMAT_YCBCR444))
>   		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>   
>   	/* Default 8bit RGB fallback */

This patch shouldn't be needed anymore with proper filtering in patch 1.

Neil

Re: [PATCH v17 4/6] drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

[H. Nikolaus Schaller]

Hi Neil,

> Am 07.04.2022 um 10:28 schrieb Neil Armstrong <narmstrong@baylibre.com>:
> 
> Hi,
> 
> On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
>> From: Neil Armstrong <narmstrong@baylibre.com>
>> The dw-hdmi integrates an optional Color Space Conversion feature used
>> to handle color-space conversions.
>> On some platforms, the CSC isn't built-in or non-functional.
>> This adds the necessary code to disable the CSC functionality
>> and limit the bus format negotiation to force using the same
>> input bus format as the output bus format.
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 100 +++++++++++++++-------
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |   1 +
>>  include/drm/bridge/dw_hdmi.h              |   1 +
>>  3 files changed, 71 insertions(+), 31 deletions(-)
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index f50af40e10340..b5a665c5e406e 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -158,6 +158,8 @@ struct dw_hdmi {
>>  	struct hdmi_data_info hdmi_data;
>>  	const struct dw_hdmi_plat_data *plat_data;
>>  +	bool csc_available;		/* indicates if the CSC engine is usable */
>> +
>>  	int vic;
>>    	u8 edid[HDMI_EDID_LEN];
>> @@ -1009,9 +1011,10 @@ static int is_color_space_interpolation(struct dw_hdmi *hdmi)
>>    static bool is_csc_needed(struct dw_hdmi *hdmi)
>>  {
>> -	return is_color_space_conversion(hdmi) ||
>> -	       is_color_space_decimation(hdmi) ||
>> -	       is_color_space_interpolation(hdmi);
>> +	return hdmi->csc_available &&
>> +	       (is_color_space_conversion(hdmi) ||
>> +		is_color_space_decimation(hdmi) ||
>> +		is_color_space_interpolation(hdmi));
>>  }
>>    static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
>> @@ -1064,6 +1067,9 @@ static void hdmi_video_csc(struct dw_hdmi *hdmi)
>>  	int interpolation = HDMI_CSC_CFG_INTMODE_DISABLE;
>>  	int decimation = 0;
>>  +	if (!hdmi->csc_available)
>> +		return;
>> +
>>  	/* YCC422 interpolation to 444 mode */
>>  	if (is_color_space_interpolation(hdmi))
>>  		interpolation = HDMI_CSC_CFG_INTMODE_CHROMA_INT_FORMULA1;
>> @@ -2665,6 +2671,7 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>>  					u32 output_fmt,
>>  					unsigned int *num_input_fmts)
>>  {
>> +	struct dw_hdmi *hdmi = bridge->driver_private;
>>  	u32 *input_fmts;
>>  	unsigned int i = 0;
>>  @@ -2683,62 +2690,81 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>>  	/* 8bit */
>>  	case MEDIA_BUS_FMT_RGB888_1X24:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>> +		}
>>  		break;
>>  	case MEDIA_BUS_FMT_YUV8_1X24:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>> +		}
>>  		break;
>>  	case MEDIA_BUS_FMT_UYVY8_1X16:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>> +		}
>>  		break;
>>    	/* 10bit */
>>  	case MEDIA_BUS_FMT_RGB101010_1X30:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
>> +		}
>>  		break;
>>  	case MEDIA_BUS_FMT_YUV10_1X30:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>> +		}
>>  		break;
>>  	case MEDIA_BUS_FMT_UYVY10_1X20:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>> +		}
>>  		break;
>>    	/* 12bit */
>>  	case MEDIA_BUS_FMT_RGB121212_1X36:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
>> +		}
>>  		break;
>>  	case MEDIA_BUS_FMT_YUV12_1X36:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
>> +		}
>>  		break;
>>  	case MEDIA_BUS_FMT_UYVY12_1X24:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
>> +		if (hdmi->csc_available) {
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
>> +			input_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
>> +		}
>>  		break;
>>    	/* 16bit */
>>  	case MEDIA_BUS_FMT_RGB161616_1X48:
>>  		input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>> +		if (hdmi->csc_available)
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>>  		break;
>>  	case MEDIA_BUS_FMT_YUV16_1X48:
>> -		input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>> -		input_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
>> +		if (hdmi->csc_available)
>> +			input_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>>  		break;
>>    	/*YUV 4:2:0 */
>> @@ -2767,15 +2793,24 @@ static int dw_hdmi_bridge_atomic_check(struct drm_bridge *bridge,
>>  {
>>  	struct dw_hdmi *hdmi = bridge->driver_private;
>>  -	hdmi->hdmi_data.enc_out_bus_format =
>> -			bridge_state->output_bus_cfg.format;
>> +	if (!hdmi->csc_available &&
>> +	    bridge_state->output_bus_cfg.format != bridge_state->input_bus_cfg.format) {
>> +		dev_warn(hdmi->dev, "different input format 0x%04x & output format 0x%04x while CSC isn't usable, fallback to safe format\n",
>> +			 bridge_state->input_bus_cfg.format,
>> +			 bridge_state->output_bus_cfg.format);
>> +		hdmi->hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_FIXED;
>> +		hdmi->hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_FIXED;
>> +	} else {
>> +		hdmi->hdmi_data.enc_out_bus_format =
>> +				bridge_state->output_bus_cfg.format;
>>  -	hdmi->hdmi_data.enc_in_bus_format =
>> -			bridge_state->input_bus_cfg.format;
>> +		hdmi->hdmi_data.enc_in_bus_format =
>> +				bridge_state->input_bus_cfg.format;
>>  -	dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
>> -		bridge_state->input_bus_cfg.format,
>> -		bridge_state->output_bus_cfg.format);
>> +		dev_dbg(hdmi->dev, "input format 0x%04x, output format 0x%04x\n",
>> +			bridge_state->input_bus_cfg.format,
>> +			bridge_state->output_bus_cfg.format);
>> +	}
>>    	return 0;
>>  }
>> @@ -3481,6 +3516,9 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>>  		hdmi->cec = platform_device_register_full(&pdevinfo);
>>  	}
>>  +	/* Get CSC useability from config0 register and permit override for platforms */
>> +	hdmi->csc_available = !plat_data->disable_csc || (config0 & HDMI_CONFIG0_CSC);
>> +
>>  	drm_bridge_add(&hdmi->bridge);
>>    	return hdmi;
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>> index 1999db05bc3b2..279722e4d1898 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>> @@ -541,6 +541,7 @@ enum {
>>    /* CONFIG0_ID field values */
>>  	HDMI_CONFIG0_I2S = 0x10,
>> +	HDMI_CONFIG0_CSC = 0x04,
>>  	HDMI_CONFIG0_CEC = 0x02,
>>    /* CONFIG1_ID field values */
>> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
>> index 2a1f85f9a8a3f..b2f689cbe864c 100644
>> --- a/include/drm/bridge/dw_hdmi.h
>> +++ b/include/drm/bridge/dw_hdmi.h
>> @@ -157,6 +157,7 @@ struct dw_hdmi_plat_data {
>>  			     unsigned long mpixelclock);
>>    	unsigned int disable_cec : 1;
>> +	unsigned int disable_csc : 1;
>>  };
>>    struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> 
> Is this really still needed now you filter correctly the possible
> modes in patch 1 ?

I had not tried to remove them because they were needed in [PATCH v16]
but indeed they are no longer needed. Something (which I personally
don't understand) may have blocked it so far, but it is not worth
further analyses.

So we can shrink the series and no need to touch drm/bridge: dw-hdmi:
any more!

I'll now post a new v18.

BR and thanks for review,
Nikolaus

Re: [PATCH v17 5/6] drm/bridge: dw-hdmi: add YCBCR formats only if CSC is available

[H. Nikolaus Schaller]

> Am 07.04.2022 um 10:29 schrieb Neil Armstrong <narmstrong@baylibre.com>:
> 
> On 06/04/2022 18:26, H. Nikolaus Schaller wrote:
>> otherwise they will produce a black HDMI screen.
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index b5a665c5e406e..ec40dd8865065 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2628,10 +2628,12 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>>  		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>>  	}
>>  -	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>> +	if (hdmi->csc_available && hdmi->sink_is_hdmi &&
>> +	    (info->color_formats & DRM_COLOR_FORMAT_YCBCR422))
>>  		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>>  -	if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>> +	if (hdmi->csc_available && hdmi->sink_is_hdmi &&
>> +	    (info->color_formats & DRM_COLOR_FORMAT_YCBCR444))
>>  		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>>    	/* Default 8bit RGB fallback */
> 
> This patch shouldn't be needed anymore with proper filtering in patch 1.

It can indeed be removed as well.

BR and thanks,
Nikolaus

Re: [PATCH v17 6/6] drm/bridge: display-connector: add ddc-en gpio support

[H. Nikolaus Schaller]

Hi Laurent,

> Am 07.04.2022 um 09:30 schrieb Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> 
> Hi Nikolaus,
> 
> Thank you for the patch.
> 
> On Wed, Apr 06, 2022 at 06:26:08PM +0200, H. Nikolaus Schaller wrote:
>> "hdmi-connector.yaml" bindings defines an optional property
>> "ddc-en-gpios" for a single gpio to enable DDC operation.
>> 
>> Usually this controls +5V power on the HDMI connector.
>> This +5V may also be needed for HPD.
>> 
>> This was not reflected in code but is needed to make the CI20
>> board work.
>> 
>> Now, the driver activates the ddc gpio after probe and
>> deactivates after remove so it is "almost on".
>> 
>> But only if this driver is loaded (and not e.g. blacklisted
>> as module).
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Added to v18 (which is needed to remove some now redundant patches
and fix a detail in the ingenic-drm driver).

BR and thanks for review,
Nikolaus