MIPS: JZ4780 and CI20 HDMI

so that specialization drivers like ingenic-dw-hdmi can enable polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
  include/drm/bridge/dw_hdmi.h              | 1 +
  2 files changed, 10 insertions(+)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +9 -0

index 4befc104d2200..43e375da131e8 100644
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
  	return 0;
  }
 
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
  			      const struct dw_hdmi_plat_data *plat_data)
  {

include/drm/bridge/dw_hdmi.h +0 -0

index 2a1f85f9a8a3f..963960794b40e 100644

Am 03.03.2022 um 17:23 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 29 lines · H. Nikolaus Schaller

so that specialization drivers like ingenic-dw-hdmi can enable polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
 include/drm/bridge/dw_hdmi.h              | 1 +
 2 files changed, 10 insertions(+)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +8 -0

index 4befc104d2200..43e375da131e8 100644
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
 	return 0;
 }
 +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {

include/drm/bridge/dw_hdmi.h +0 -0

index 2a1f85f9a8a3f..963960794b40e 100644

include/drm/bridge/dw_hdmi.h enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi, · line 200

Quoted 6 lines

include/drm/bridge/dw_hdmi.h +1 -0

@@ -196,5 +196,6 @@ enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
 void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 			    bool force, bool disabled, bool rxsense);
 void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
   #endif /* __IMX_HDMI_H__ */

As I understand, this is because the IRQ line of the dw-hdmi IP isn't connected right ? and you use the display-connector ddc gpio instead ?

In this case I think the Ingenic DRM core should call drm_kms_helper_poll_init(drm) instead.

Am 03.03.2022 um 17:30 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Neil,

Quoted 34 lines

Am 03.03.2022 um 17:23 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 29 lines · H. Nikolaus Schaller

so that specialization drivers like ingenic-dw-hdmi can enable polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
include/drm/bridge/dw_hdmi.h | 1 +

2 files changed, 10 insertions(+)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +9 -0

index 4befc104d2200..43e375da131e8 100644
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
 	return 0;
 }
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {

include/drm/bridge/dw_hdmi.h +0 -0

index 2a1f85f9a8a3f..963960794b40e 100644

Quoted 8 lines

include/drm/bridge/dw_hdmi.h enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi, · line 200

Quoted 6 lines

include/drm/bridge/dw_hdmi.h +1 -0

@@ -196,5 +196,6 @@ enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
 void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 			    bool force, bool disabled, bool rxsense);
 void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
  #endif /* __IMX_HDMI_H__ */

As I understand, this is because the IRQ line of the dw-hdmi IP isn't connected right ? and you use the display-connector ddc gpio instead ?

Yes. The IRQ line is not connected on all boards as far as I can see.

Quoted 2 lines

In this case I think the Ingenic DRM core should call drm_kms_helper_poll_init(drm) instead.

Ah, that is good. it seems to do "dw_hdmi_enable_poll()" in a more generic way.

Hi Neil,

Quoted 59 lines

Am 03.03.2022 um 17:30 schrieb H. Nikolaus Schaller
<[email protected]>:

Hi Neil,

Quoted 47 lines

Am 03.03.2022 um 17:23 schrieb Neil Armstrong
<[email protected]>:

Hi,

Quoted 38 lines · H. Nikolaus Schaller

so that specialization drivers like ingenic-dw-hdmi can enable
polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
 include/drm/bridge/dw_hdmi.h              | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 4befc104d2200..43e375da131e8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi
*hdmi)

(quoted diff) +0 -0

 	return 0;
 }
 +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
 +{
 +	if (hdmi->bridge.dev)
 +		hdmi->bridge.dev->mode_config.poll_enabled = enable;
 +	else
 +		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
 +}
 +EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
 +
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {
 diff --git a/include/drm/bridge/dw_hdmi.h 

b/include/drm/bridge/dw_hdmi.h
index 2a1f85f9a8a3f..963960794b40e 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -196,5 +196,6 @@ enum drm_connector_status
dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
bool force, bool disabled, bool rxsense);
void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);

Quoted 2 lines

 +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
   #endif /* __IMX_HDMI_H__ */

Quoted 2 lines

As I understand, this is because the IRQ line of the dw-hdmi IP
isn't connected right ? and you use the display-connector ddc gpio instead ?

Yes. The IRQ line is not connected on all boards as far as I can
see.

Quoted 1 line

Quoted 5 lines

Quoted 2 lines

In this case I think the Ingenic DRM core should call
drm_kms_helper_poll_init(drm) instead.

Ah, that is good. it seems to do "dw_hdmi_enable_poll()" in a more
generic way.

Well, I looked through source code and it is defined as

void drm_kms_helper_poll_init(struct drm_device *dev)

But there is no direct pointer to some drm_device available. Neither in dw-hdmi nor ingenic-dw-hdmi.

What should the parameter to drm_kms_helper_poll_init(drm) be?

From comparing code to be able to set mode_config.poll_enabled = enable it should be

	&hdmi->bridge.dev

but the struct dw_hdmi *hdmi is an opaque type for the ingenic-dw-hdmi driver. So it can't access the hdmi-bridge directly.

What we can do is to make dw_hdmi_enable_poll() call drm_kms_helper_poll_init() or drm_kms_helper_poll_fini().

BR and thanks,
Nikolaus

Am 03.03.2022 um 17:51 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

Quoted 43 lines · H. Nikolaus Schaller

Hi Neil,

Quoted 42 lines

Am 03.03.2022 um 17:30 schrieb H. Nikolaus Schaller <[email protected]>:
Hi Neil,

Quoted 31 lines

Am 03.03.2022 um 17:23 schrieb Neil Armstrong <[email protected]>:
Hi,

Quoted 28 lines · H. Nikolaus Schaller

so that specialization drivers like ingenic-dw-hdmi can enable polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
include/drm/bridge/dw_hdmi.h | 1 +

2 files changed, 10 insertions(+)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +9 -0

index 4befc104d2200..43e375da131e8 100644
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
 	return 0;
 }
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {

include/drm/bridge/dw_hdmi.h +0 -1

index 2a1f85f9a8a3f..963960794b40e 100644
--- a/include/drm/bridge/dw_hdmi.h

Quoted 8 lines

include/drm/bridge/dw_hdmi.h enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi, · line 200

Quoted 7 lines

+++ b/include/drm/bridge/dw_hdmi.h

include/drm/bridge/dw_hdmi.h +1 -0

@@ -196,5 +196,6 @@ enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
 void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 			    bool force, bool disabled, bool rxsense);
 void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
  #endif /* __IMX_HDMI_H__ */

As I understand, this is because the IRQ line of the dw-hdmi IP isn't connected right ? and you use the display-connector ddc gpio instead ?

Yes. The IRQ line is not connected on all boards as far as I can see.

Quoted 6 lines

Quoted 2 lines

Quoted 1 line

In this case I think the Ingenic DRM core should call drm_kms_helper_poll_init(drm) instead.

Ah, that is good. it seems to do "dw_hdmi_enable_poll()" in a more generic way.

Well, I looked through source code and it is defined as
void drm_kms_helper_poll_init(struct drm_device *dev)
But there is no direct pointer to some drm_device available. Neither in dw-hdmi nor ingenic-dw-hdmi.

Well he said "the Ingenic DRM core" aka ingenic-drm-drv.c. You do have access to the main drm_device in the ingenic_drm_bind() function, so you can add it there (with a cleanup function calling drm_kms_helper_poll_fini() registered with drmm_add_action_or_reset()).

Quoted 2 lines

Well he said "the Ingenic DRM core" aka ingenic-drm-drv.c. You do
have access to the main drm_device in the ingenic_drm_bind() function, so you can add it there (with a cleanup function calling drm_kms_helper_poll_fini() registered with drmm_add_action_or_reset()).

Well, do you really want to mix HPD detection between connector, Synopsys bridge and Ingenic DRM core? These are independent... Or should be accessed only through the bridge chain pointers.

IMHO we should keep separate functions separate.

And maybe this should also be conditional? Maybe not depend on compatible = jz4780 but compatible = ci20?

Looks to me to be a quick fix in the wrong place.

Let's fix the CSC issue first.

BR,
Nikolaus

Am 03.03.2022 um 18:20 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

[snip]

Quoted 4 lines

Quoted 1 line

Well he said "the Ingenic DRM core" aka ingenic-drm-drv.c. You do have access to the main drm_device in the ingenic_drm_bind() function, so you can add it there (with a cleanup function calling drm_kms_helper_poll_fini() registered with drmm_add_action_or_reset()).

Well, do you really want to mix HPD detection between connector, Synopsys bridge and Ingenic DRM core? These are independent... Or should be accessed only through the bridge chain pointers.
IMHO we should keep separate functions separate.

The drm_kms_helper_poll_init() just says "this DRM device may have connectors that need to be polled" so it very well fits inside the main driver, IMHO.

Hi Paul, Neil,

Quoted 4 lines

Am 03.03.2022 um 18:20 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

Quoted 8 lines

[snip]

Quoted 4 lines

Quoted 1 line

Well he said "the Ingenic DRM core" aka ingenic-drm-drv.c. You do have access to the main drm_device in the ingenic_drm_bind() function, so you can add it there (with a cleanup function calling drm_kms_helper_poll_fini() registered with drmm_add_action_or_reset()).

Well, do you really want to mix HPD detection between connector, Synopsys bridge and Ingenic DRM core? These are independent... Or should be accessed only through the bridge chain pointers.
IMHO we should keep separate functions separate.

The drm_kms_helper_poll_init() just says "this DRM device may have connectors that need to be polled" so it very well fits inside the main driver, IMHO.

As far as I understand, it has the side-effect to always set dev->mode_config.poll_enabled and schedule_delayed_work() for all devices.
I am not sure if this is intended for arbitrary ingenic-drm devices. But you know better than me.

Hm. But wait, I think I now finally remember why I have proposed it the way it is! It is always better to go back to requirements and find the least invasive solution.

- HPD IRQ works and calls dw_hdmi_irq() [as can be shown by adding printk()]
- it is just that the udevd is only notified if poll_enabled = true (but no polling takes place!).

An earlier version (v4) to fix this proposed to add an explicit call to drm_kms_helper_hotplug_event() in dw_hdmi_irq() but that was rejected a while ago because drm_helper_hpd_irq_event() will already call it:

	https://www.spinics.net/lists/dri-devel/msg316846.html

Since this did not take into account that dev->mode_config.poll_enabled must be set true, I then proposed the enable_poll() mechanism just to set this bit for the ingenic-dw-hdmi specialization.

So a HPD event is delivered to the dw-hdmi driver as dw_hdmi_irq() and that calls drm_helper_hpd_irq_event() but not drm_kms_helper_hotplug_event() and user-space is not getting aware.

It is all a hack because we mix the dw-hdmi driver which originally did register its own connector with an explicit connector...

In summary I now thing that the v4 patch is the simplest and least invasive solution.

We neither have to introduce a dw_hdmi_enable_poll() function or call drm_kms_helper_poll_init() anywhere.

It is just a single line to add to dw-hdmi. And neither touches ingenic-dw-hdmi nor ingenic-drm-drv.

So let's go back to v4 version (just modify commit message to better describe why we have to call drm_kms_helper_hotplug_event() explicitly) and forget about alternatives.

BR,
Nikolaus

Hi,

Quoted 48 lines · H. Nikolaus Schaller

Hi Paul, Neil,

Quoted 4 lines

Am 03.03.2022 um 18:20 schrieb Paul Cercueil <[email protected]>:

Hi Nikolaus,

Quoted 8 lines

[snip]

Quoted 4 lines

Quoted 1 line

Well he said "the Ingenic DRM core" aka ingenic-drm-drv.c. You do have access to the main drm_device in the ingenic_drm_bind() function, so you can add it there (with a cleanup function calling drm_kms_helper_poll_fini() registered with drmm_add_action_or_reset()).

Well, do you really want to mix HPD detection between connector, Synopsys bridge and Ingenic DRM core? These are independent... Or should be accessed only through the bridge chain pointers.
IMHO we should keep separate functions separate.

The drm_kms_helper_poll_init() just says "this DRM device may have connectors that need to be polled" so it very well fits inside the main driver, IMHO.

As far as I understand, it has the side-effect to always set dev->mode_config.poll_enabled and schedule_delayed_work() for all devices.
I am not sure if this is intended for arbitrary ingenic-drm devices. But you know better than me.

Hm. But wait, I think I now finally remember why I have proposed it the way it is! It is always better to go back to requirements and find the least invasive solution.

- HPD IRQ works and calls dw_hdmi_irq() [as can be shown by adding printk()]
- it is just that the udevd is only notified if poll_enabled = true (but no polling takes place!).

An earlier version (v4) to fix this proposed to add an explicit call to drm_kms_helper_hotplug_event() in dw_hdmi_irq() but that was rejected a while ago because drm_helper_hpd_irq_event() will already call it:

	https://www.spinics.net/lists/dri-devel/msg316846.html

Since this did not take into account that dev->mode_config.poll_enabled must be set true, I then proposed the enable_poll() mechanism just to set this bit for the ingenic-dw-hdmi specialization.

So a HPD event is delivered to the dw-hdmi driver as dw_hdmi_irq() and that calls drm_helper_hpd_irq_event() but not drm_kms_helper_hotplug_event() and user-space is not getting aware.

It is all a hack because we mix the dw-hdmi driver which originally did register its own connector with an explicit connector...

In summary I now thing that the v4 patch is the simplest and least invasive solution.

We neither have to introduce a dw_hdmi_enable_poll() function or call drm_kms_helper_poll_init() anywhere.

It is just a single line to add to dw-hdmi. And neither touches ingenic-dw-hdmi nor ingenic-drm-drv.

Quoted 2 lines

So let's go back to v4 version (just modify commit message to better describe why we have to call drm_kms_helper_hotplug_event() explicitly) and forget about alternatives.

Please don't and add drm_kms_helper_poll_init() from the ingenic-drm-drv.c like every other DRM driver.

Adding drm_kms_helper_hotplug_event() in dw-hdmi will impact other drivers using dw-hdmi but correctly calling drm_kms_helper_poll_init().

Am 04.03.2022 um 17:47 schrieb Paul Cercueil <[email protected]>:

From what I understood in Nikolaus' last message, HDMI hotplug is actually correctly detected, so there's no need for polling. What is missing is the call to drm_kms_helper_hotplug_event *somewhere*, so that the information is correctly relayed to userspace.

I think this issue can be fixed by calling drm_bridge_connector_enable_hpd() on the connector in ingenic-drm-drv.c.

Hi Paul, Neil,

Quoted 4 lines

Am 04.03.2022 um 17:47 schrieb Paul Cercueil <[email protected]>:

From what I understood in Nikolaus' last message, HDMI hotplug is
actually correctly detected, so there's no need for polling. What is missing is the call to drm_kms_helper_hotplug_event *somewhere*, so that the information is correctly relayed to userspace.

Exactly.

As Maxime pointed out it should already be called by drm_helper_hpd_irq_event() in dw_hdmi_irq() but isn't because mode_config.poll_enabled isn't enabled.

So we can either
a) enable mode_config.poll_enabled so that it is called by drm_helper_hpd_irq_event() or

b) make drm_kms_helper_hotplug_event() being called explicitly in dw_hdmi_irq(). We could guard that by mode_config.poll_enabled to avoid
drm_kms_helper_hotplug_event()
being called twice (but I think the "changed" mechanism will take
care of).

Quoted 2 lines

I think this issue can be fixed by calling
drm_bridge_connector_enable_hpd() on the connector in ingenic-drm-drv.c.

I don't see yet how this would solve it, but it may work.

Anyways, this all is a missing feature (sometimes called "bug") of the *dw-hdmi driver* and IMHO neither of the connector nor the ingenic-drm-drv.

So I think it should not be solved outside dw-hdmi.

BR and thanks,
Nikolaus

Am 04.03.2022 um 19:04 schrieb Paul Cercueil <[email protected]>:

Quoted 11 lines · H. Nikolaus Schaller

Hi Paul, Neil,

Quoted 2 lines

Am 04.03.2022 um 17:47 schrieb Paul Cercueil <[email protected]>:
From what I understood in Nikolaus' last message, HDMI hotplug is actually correctly detected, so there's no need for polling. What is missing is the call to drm_kms_helper_hotplug_event *somewhere*, so that the information is correctly relayed to userspace.

Exactly.
As Maxime pointed out it should already be called by drm_helper_hpd_irq_event() in dw_hdmi_irq() but isn't because mode_config.poll_enabled isn't enabled.
So we can either
a) enable mode_config.poll_enabled so that it is called by drm_helper_hpd_irq_event() or
b) make drm_kms_helper_hotplug_event() being called explicitly in dw_hdmi_irq().
We could guard that by mode_config.poll_enabled to avoid drm_kms_helper_hotplug_event() being called twice (but I think the "changed" mechanism will take care of).

Quoted 2 lines

Quoted 1 line

I think this issue can be fixed by calling drm_bridge_connector_enable_hpd() on the connector in ingenic-drm-drv.c.

I don't see yet how this would solve it, but it may work.

dw_hdmi_irq() calls drm_bridge_hpd_notify(), which would call bridge->hpd_cb() if it was non-NULL.

Calling drm_bridge_connector_enable_hpd() will set the bridge->hpd_cb() callback to point to drm_bridge_connector_hpd_cb(), which itself will call drm_kms_helper_hotplug_event(). Therefore, all that is missing is one call to drm_bridge_connector_enable_hpd().

Quoted 2 lines

Anyways, this all is a missing feature (sometimes called "bug") of the *dw-hdmi driver* and IMHO neither of the connector nor the ingenic-drm-drv.

Quoted 1 line

So I think it should not be solved outside dw-hdmi.

Hi Paul,

Quoted 24 lines

Am 04.03.2022 um 19:04 schrieb Paul Cercueil <[email protected]>:

Quoted 18 lines · H. Nikolaus Schaller

Hi Paul, Neil,

Quoted 4 lines

Am 04.03.2022 um 17:47 schrieb Paul Cercueil
<[email protected]>:
From what I understood in Nikolaus' last message, HDMI hotplug is
actually correctly detected, so there's no need for polling. What is missing is the call to drm_kms_helper_hotplug_event *somewhere*, so that the information is correctly relayed to userspace.

Exactly.
As Maxime pointed out it should already be called by
drm_helper_hpd_irq_event() in dw_hdmi_irq() but isn't
because mode_config.poll_enabled isn't enabled.
So we can either
a) enable mode_config.poll_enabled so that it is called by
drm_helper_hpd_irq_event() or
b) make drm_kms_helper_hotplug_event() being called explicitly in
dw_hdmi_irq().
We could guard that by mode_config.poll_enabled to avoid
drm_kms_helper_hotplug_event()
being called twice (but I think the "changed" mechanism will
take care of).

Quoted 6 lines

Quoted 3 lines

Quoted 2 lines

I think this issue can be fixed by calling
drm_bridge_connector_enable_hpd() on the connector in ingenic-drm-drv.c.

I don't see yet how this would solve it, but it may work.

dw_hdmi_irq() calls drm_bridge_hpd_notify(), which would call
bridge->hpd_cb() if it was non-NULL.

Ok, this is a case c).

I vaguely remember having tried to analyse what bridge->hpd_cb is but stopped since it is NULL...

Quoted 3 lines

Calling drm_bridge_connector_enable_hpd() will set the
bridge->hpd_cb() callback to point to drm_bridge_connector_hpd_cb(), which itself will call drm_kms_helper_hotplug_event(). Therefore, all that is missing is one call to drm_bridge_connector_enable_hpd().

Ah, ok, I see.

Quoted 3 lines

Quoted 3 lines

Anyways, this all is a missing feature (sometimes called "bug") of
the *dw-hdmi driver* and IMHO
neither of the connector nor the ingenic-drm-drv.

Well, a little more analysis shows that drm_bridge_connector_enable_hpd is called in the *-drv.c for some other plaforms:

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/imx/dcss/dcss-dev.c#L292
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/imx/dcss/dcss-kms.c#L145
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/omapdrm/omap_drv.c#L393
https://elixir.bootlin.com/linux/v5.17-rc6/source/drivers/gpu/drm/msm/hdmi/hdmi.c#L317

Quoted 1 line

Quoted 1 line

So I think it should not be solved outside dw-hdmi.

Hm. Can we call drm_bridge_connector_enable_hpd() from inside dw-hdmi?

Or would this be the solution if merged? (I currently can't try code).

https://lore.kernel.org/lkml/[email protected]/T/

Am 04.03.2022 um 19:33 schrieb Paul Cercueil <[email protected]>:

Quoted 29 lines · H. Nikolaus Schaller

Hi Paul,

Quoted 13 lines

Am 04.03.2022 um 19:04 schrieb Paul Cercueil <[email protected]>:

Quoted 11 lines · H. Nikolaus Schaller

Hi Paul, Neil,

Quoted 2 lines

Am 04.03.2022 um 17:47 schrieb Paul Cercueil <[email protected]>:
From what I understood in Nikolaus' last message, HDMI hotplug is actually correctly detected, so there's no need for polling. What is missing is the call to drm_kms_helper_hotplug_event *somewhere*, so that the information is correctly relayed to userspace.

Exactly.
As Maxime pointed out it should already be called by drm_helper_hpd_irq_event() in dw_hdmi_irq() but isn't because mode_config.poll_enabled isn't enabled.
So we can either
a) enable mode_config.poll_enabled so that it is called by drm_helper_hpd_irq_event() or
b) make drm_kms_helper_hotplug_event() being called explicitly in dw_hdmi_irq().
We could guard that by mode_config.poll_enabled to avoid drm_kms_helper_hotplug_event() being called twice (but I think the "changed" mechanism will take care of).

Quoted 3 lines

Quoted 2 lines

Quoted 1 line

I think this issue can be fixed by calling drm_bridge_connector_enable_hpd() on the connector in ingenic-drm-drv.c.

I don't see yet how this would solve it, but it may work.

dw_hdmi_irq() calls drm_bridge_hpd_notify(), which would call bridge->hpd_cb() if it was non-NULL.

Ok, this is a case c).
I vaguely remember having tried to analyse what bridge->hpd_cb is but stopped since it is NULL...

Quoted 1 line

Calling drm_bridge_connector_enable_hpd() will set the bridge->hpd_cb() callback to point to drm_bridge_connector_hpd_cb(), which itself will call drm_kms_helper_hotplug_event(). Therefore, all that is missing is one call to drm_bridge_connector_enable_hpd().

Ah, ok, I see.

Quoted 2 lines

Quoted 2 lines

Anyways, this all is a missing feature (sometimes called "bug") of the *dw-hdmi driver* and IMHO neither of the connector nor the ingenic-drm-drv.

Well, a little more analysis shows that drm_bridge_connector_enable_hpd is called in the *-drv.c for some other plaforms:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/imx/dcss/dcss-dev.c#L292
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/imx/dcss/dcss-kms.c#L145
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/omapdrm/omap_drv.c#L393
https://elixir.bootlin.com/linux/v5.17-rc6/source/drivers/gpu/drm/msm/hdmi/hdmi.c#L317

Quoted 4 lines

Quoted 1 line

Quoted 1 line

So I think it should not be solved outside dw-hdmi.

Hm. Can we call drm_bridge_connector_enable_hpd() from inside dw-hdmi? Or would this be the solution if merged? (I currently can't try code).
https://lore.kernel.org/lkml/[email protected]/T/

Looks correct to me. It has been reviewed by two people so I believe it will be merged very soon.

Am 04.03.2022 um 19:41 schrieb H. Nikolaus Schaller <[email protected]>:

Quoted 39 lines

Am 04.03.2022 um 19:33 schrieb Paul Cercueil <[email protected]>:

Quoted 33 lines · H. Nikolaus Schaller

Hi Paul,

Quoted 13 lines

Am 04.03.2022 um 19:04 schrieb Paul Cercueil <[email protected]>:

Quoted 11 lines · H. Nikolaus Schaller

Hi Paul, Neil,

Quoted 2 lines

Am 04.03.2022 um 17:47 schrieb Paul Cercueil <[email protected]>:
From what I understood in Nikolaus' last message, HDMI hotplug is actually correctly detected, so there's no need for polling. What is missing is the call to drm_kms_helper_hotplug_event *somewhere*, so that the information is correctly relayed to userspace.

Exactly.
As Maxime pointed out it should already be called by drm_helper_hpd_irq_event() in dw_hdmi_irq() but isn't because mode_config.poll_enabled isn't enabled.
So we can either
a) enable mode_config.poll_enabled so that it is called by drm_helper_hpd_irq_event() or
b) make drm_kms_helper_hotplug_event() being called explicitly in dw_hdmi_irq().
We could guard that by mode_config.poll_enabled to avoid drm_kms_helper_hotplug_event() being called twice (but I think the "changed" mechanism will take care of).

Quoted 3 lines

Quoted 2 lines

Quoted 1 line

I think this issue can be fixed by calling drm_bridge_connector_enable_hpd() on the connector in ingenic-drm-drv.c.

I don't see yet how this would solve it, but it may work.

dw_hdmi_irq() calls drm_bridge_hpd_notify(), which would call bridge->hpd_cb() if it was non-NULL.

Ok, this is a case c).
I vaguely remember having tried to analyse what bridge->hpd_cb is but stopped since it is NULL...

Quoted 1 line

Calling drm_bridge_connector_enable_hpd() will set the bridge->hpd_cb() callback to point to drm_bridge_connector_hpd_cb(), which itself will call drm_kms_helper_hotplug_event(). Therefore, all that is missing is one call to drm_bridge_connector_enable_hpd().

Ah, ok, I see.

Quoted 2 lines

Quoted 2 lines

Anyways, this all is a missing feature (sometimes called "bug") of the *dw-hdmi driver* and IMHO neither of the connector nor the ingenic-drm-drv.

Well, a little more analysis shows that drm_bridge_connector_enable_hpd is called in the *-drv.c for some other plaforms:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/imx/dcss/dcss-dev.c#L292
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/imx/dcss/dcss-kms.c#L145
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/omapdrm/omap_drv.c#L393
https://elixir.bootlin.com/linux/v5.17-rc6/source/drivers/gpu/drm/msm/hdmi/hdmi.c#L317

Quoted 1 line

Quoted 1 line

So I think it should not be solved outside dw-hdmi.

Hm. Can we call drm_bridge_connector_enable_hpd() from inside dw-hdmi? Or would this be the solution if merged? (I currently can't try code).
https://lore.kernel.org/lkml/[email protected]/T/

Quoted 1 line

Looks correct to me. It has been reviewed by two people so I believe it will be merged very soon.

Great. I will try asap. If it works we can drop all our private ideas...

And focus on the last missing piece for jz4780 HDMI: the output format negotiation (which still is not working properly - but I have to analyse why).

Hi,

Quoted 33 lines · H. Nikolaus Schaller

so that specialization drivers like ingenic-dw-hdmi can enable polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
  include/drm/bridge/dw_hdmi.h              | 1 +
  2 files changed, 10 insertions(+)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +8 -0

index 4befc104d2200..43e375da131e8 100644
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi 
 *hdmi)
  	return 0;
  }
  +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
  struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
  			      const struct dw_hdmi_plat_data *plat_data)
  {

include/drm/bridge/dw_hdmi.h +0 -0

index 2a1f85f9a8a3f..963960794b40e 100644
@@ -196,5 +196,6 @@ enum drm_connector_status 

include/drm/bridge/dw_hdmi.h enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,

Quoted 6 lines

include/drm/bridge/dw_hdmi.h +1 -0

 dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
  void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
  			    bool force, bool disabled, bool rxsense);
  void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
    #endif /* __IMX_HDMI_H__ */

As I understand, this is because the IRQ line of the dw-hdmi IP isn't connected right ? and you use the display-connector ddc gpio instead ?

In this case I think the Ingenic DRM core should call drm_kms_helper_poll_init(drm) instead.

Am 03.03.2022 um 17:46 schrieb Paul Cercueil <[email protected]>:

Hi Neil,

Quoted 38 lines · Neil Armstrong

Hi,

Quoted 30 lines · H. Nikolaus Schaller

so that specialization drivers like ingenic-dw-hdmi can enable polling.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
 include/drm/bridge/dw_hdmi.h              | 1 +
 2 files changed, 10 insertions(+)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +8 -0

index 4befc104d2200..43e375da131e8 100644
@@ -3217,6 +3217,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
 	return 0;
 }
 +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {

include/drm/bridge/dw_hdmi.h +0 -0

index 2a1f85f9a8a3f..963960794b40e 100644

include/drm/bridge/dw_hdmi.h enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi, · line 200

Quoted 5 lines

include/drm/bridge/dw_hdmi.h +1 -0

@@ -196,5 +196,6 @@ enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
 void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 			    bool force, bool disabled, bool rxsense);
 void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
   #endif /* __IMX_HDMI_H__ */

As I understand, this is because the IRQ line of the dw-hdmi IP isn't connected right ? and you use the display-connector ddc gpio instead ?

According to the CI20 schematic, it is wired properly. So that's strange.

Quoted 2 lines

In this case I think the Ingenic DRM core should call drm_kms_helper_poll_init(drm) instead.

Yes, the ingenic-drm driver does not poll for connectors because until now it never has been needed.

Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")

introduced a new mechanism to negotiate bus formats between hdmi connectors and bridges which is to be used e.g. for the jz4780 based CI20 board.

In this case dw-hdmi sets up a list of formats in
dw_hdmi_bridge_atomic_get_output_bus_fmts().

This includes e.g. MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the CI20 but only produces a black screen.

Analysis revealed an omission in

Commit 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")

to check for 8 bit with when adding UYVY8 or YUV8 formats.

This fix is based on the observation that max_bpc = 0 when running this function while info->bpc = 8.

Adding the proposed patch makes the jz4780/CI20 panel work again with default MEDIA_BUS_FMT_RGB888_1X24 mode.

Fixes 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")

Fixes 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +0 -1

index 43e375da131e8..c08e2cc96584c 100644
@@ -2621,11 +2621,13 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
  		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
  	}
 
-	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +0 -0

  	/* Default 8bit RGB fallback */
  	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;

Am 01.03.2022 um 10:18 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 4 lines · H. Nikolaus Schaller

Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks") introduced a new mechanism to negotiate bus formats between hdmi connectors and bridges which is to be used e.g. for the jz4780 based CI20 board. In this case dw-hdmi sets up a list of formats in

Quoted 8 lines

dw_hdmi_bridge_atomic_get_output_bus_fmts().
This includes e.g. MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the CI20 but only produces a black screen.
Analysis revealed an omission in
Commit 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation") to check for 8 bit with when adding UYVY8 or YUV8 formats.
This fix is based on the observation that max_bpc = 0 when running this function while info->bpc = 8.

In fact if bpc = 0, it should be considered as 8, so the issue is elsewhere.

Quoted 15 lines

Adding the proposed patch makes the jz4780/CI20 panel work again with default MEDIA_BUS_FMT_RGB888_1X24 mode.

Fixes 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")

Fixes 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +0 -0

index 43e375da131e8..c08e2cc96584c 100644
@@ -2621,11 +2621,13 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
 	}

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, · line 2630

Quoted 10 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +6 -2

 -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
-		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+	if (max_bpc >= 8 && info->bpc >= 8) {
+		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+			output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
 -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
-		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+			output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+	}

It should not select YUV here if it's not possible, so something is wrong.

Can you check if https://lore.kernel.org/r/[email protected] fixes this issue instead ?

Hi Neil,

Quoted 48 lines

Am 01.03.2022 um 10:18 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 4 lines · H. Nikolaus Schaller

Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks") introduced a new mechanism to negotiate bus formats between hdmi connectors and bridges which is to be used e.g. for the jz4780 based CI20 board. In this case dw-hdmi sets up a list of formats in

Quoted 8 lines

dw_hdmi_bridge_atomic_get_output_bus_fmts().
This includes e.g. MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the CI20 but only produces a black screen.
Analysis revealed an omission in
Commit 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation") to check for 8 bit with when adding UYVY8 or YUV8 formats.
This fix is based on the observation that max_bpc = 0 when running this function while info->bpc = 8.

In fact if bpc = 0, it should be considered as 8, so the issue is elsewhere.

Quoted 15 lines

Adding the proposed patch makes the jz4780/CI20 panel work again with default MEDIA_BUS_FMT_RGB888_1X24 mode.

Fixes 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")

Fixes 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +0 -0

index 43e375da131e8..c08e2cc96584c 100644
@@ -2621,11 +2621,13 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
  		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
  	}

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, · line 2630

Quoted 10 lines

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +6 -2

  -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
-		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
+	if (max_bpc >= 8 && info->bpc >= 8) {
+		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
+			output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
  -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
-		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
+			output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
+	}

It should not select YUV here if it's not possible, so something is wrong.

Quoted 1 line

Can you check if https://lore.kernel.org/r/[email protected] fixes this issue instead ?

Well, I had to manually fix it to be appliable to drm-misc/drm-misc-next and specifically:

c03d0b52ff71 ("drm/connector: Fix typo in output format")

My resulting patch is attached.

Unfortunately it did not work.

I added a printk for hdmi->sink_is_hdmi. This returns 1. Which IMHO is to be expected since I am using a HDMI connector and panel... So your patch will still add the UYVY formats.

Either the synposys module inside the jz4780 or the panel does not understand them.

Here is the EDID. Unfortunately it does not pretty print the extended descriptors for UYVY etc. so that I don't know the exact capabilities of the panel. And what I am not sure is if the jz4780 SoC can convert to UYVY or how it can.

root@letux:~# parse-edid </sys/devices/platform/13050000.lcdc0/drm/card0/card0-HDMI-A-1/edid
Checksum Correct

Section "Monitor"

         Identifier "LEN L1950wD"
         ModelName "LEN L1950wD"
         VendorName "LEN"
         # Monitor Manufactured week 34 of 2011
         # EDID version 1.3
         # Digital Display
         DisplaySize 410 260
         Gamma 2.20
         Option "DPMS" "true"
         Horizsync 30-81
         VertRefresh 50-76
         # Maximum pixel clock is 140MHz
         #Not giving standard mode: 1152x864, 75Hz
         #Not giving standard mode: 1280x720, 60Hz
         #Not giving standard mode: 1280x1024, 60Hz
         #Not giving standard mode: 1280x1024, 60Hz
         #Not giving standard mode: 1280x1024, 60Hz
         #Not giving standard mode: 1440x900, 60Hz
         #Not giving standard mode: 1440x900, 75Hz
         #Not giving standard mode: 1920x1080, 60Hz

         #Extension block found. Parsing...
         Modeline        "Mode 15" -hsync -vsync
         Modeline        "Mode 0" -hsync +vsync
         Modeline        "Mode 1" 27.027 1440 1478 1602 1716 480 484 487 525 -hsync -vsync interlace
         Modeline        "Mode 2" 27.027 1440 1478 1602 1716 480 484 487 525 -hsync -vsync interlace
         Modeline        "Mode 3" 27.027 720 736 798 858 480 489 495 525 -hsync -vsync
         Modeline        "Mode 4" 27.027 720 736 798 858 480 489 495 525 -hsync -vsync
         Modeline        "Mode 5" 27.000 1440 1464 1590 1728 576 578 581 625 -hsync -vsync interlace
         Modeline        "Mode 6" 27.000 1440 1464 1590 1728 576 578 581 625 -hsync -vsync interlace
         Modeline        "Mode 7" 27.000 720 732 796 864 576 581 586 625 -hsync -vsync
         Modeline        "Mode 8" 27.000 720 732 796 864 576 581 586 625 -hsync -vsync
         Modeline        "Mode 9" 74.250 1280 1720 1760 1980 720 725 730 750 +hsync +vsync
         Modeline        "Mode 10" 74.250 1280 1390 1420 1650 720 725 730 750 +hsync +vsync
         Modeline        "Mode 11" 74.250 1920 2448 2492 2640 1080 1082 1089 1125 +hsync +vsync interlace
         Modeline        "Mode 12" 74.250 1920 2008 2052 2200 1080 1082 1087 1125 +hsync +vsync interlace
         Modeline        "Mode 13" 148.500 1920 2448 2492 2640 1080 1084 1089 1125 +hsync +vsync
         Modeline        "Mode 14" 148.500 1920 2008 2052 2200 1080 1084 1089 1125 +hsync +vsync
         Modeline        "Mode 16" +hsync +vsync interlace
         Modeline        "Mode 17" +hsync +vsync interlace
         Modeline        "Mode 18" +hsync +vsync
         Option "PreferredMode" "Mode 15"
EndSection
root@letux:~# xxd /sys/devices/platform/13050000.lcdc0/drm/card0/card0-HDMI-A-1/
00000000: 00ff ffff ffff ff00 30ae 8610 0101 0101  ........0.......
00000010: 2215 0103 8029 1a78 eee5 b5a3 5549 9927  "....).x....UI.'
00000020: 1350 54af ef00 714f 81c0 8180 8180 8180  .PT...qO........
00000030: 9500 950f d1c0 2413 0020 4158 1620 050d  ......$.. AX. ..
00000040: 2300 ffff 0000 001c 0000 00fc 004c 454e  #............LEN
00000050: 204c 3139 3530 7744 0a20 0000 00fd 0032   L1950wD. .....2
00000060: 4c1e 510e 000a 2020 2020 2020 0000 00ff  L.Q...      ....
00000070: 0042 3334 3332 3834 350a 2020 2020 0101  .B3432845.    ..
00000080: 0203 2171 4e06 0702 0315 9611 1213 0414  ..!qN...........
00000090: 051f 9023 0907 0783 0100 0065 030c 0010  ...#.......e....
000000a0: 008c 0ad0 9020 4031 200c 4055 00b9 8821  ..... @1 .@U...!
000000b0: 0000 1801 1d80 1871 1c16 2058 2c25 00b9  .......q.. X,%..
000000c0: 8821 0000 9e01 1d80 d072 1c16 2010 2c25  .!.......r.. .,%
000000d0: 80b9 8821 0000 9e01 1d00 bc52 d01e 20b8  ...!.......R.. .
000000e0: 2855 40b9 8821 0000 1e02 3a80 d072 382d  (U@..!....:..r8-
000000f0: 4010 2c45 80b9 8821 0000 1e00 0000 00d0  @.,E...!........
root@letux:~# root@letux:~# dmesg|grep dw.hdmi
[    9.622138] dw-hdmi-ingenic 10180000.hdmi: Detected HDMI TX controller v1.31a with HDCP (DWC HDMI 3D TX PHY)
[    9.727840] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare HDMI I2C bus driver
[   10.103864] dw_hdmi_bridge_atomic_get_output_bus_fmts: hdmi->sink_is_hdmi=1

So please let me know which parameters I should try to printk()...

BR and thanks,
Nikolaus

------

From c84a3c4a500684e57b1243fe5386696c48fa1e1b Mon Sep 17 00:00:00 2001
From: Neil Armstrong <[email protected]>
Date: Wed, 19 Jan 2022 13:36:56 +0100
Subject: [PATCH] drm/bridge: dw-hdmi: filter out YUV output formats when DVI

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 <[email protected]>

---
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++--------
  1 file changed, 9 insertions(+), 8 deletions(-)

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +9 -8

index 43e375da131e8..0ec0cbe448e05 100644
@@ -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 */

Am 02.03.2022 um 11:25 schrieb Neil Armstrong <[email protected]>:

Quoted 3 lines

I added a printk for hdmi->sink_is_hdmi. This returns 1. Which IMHO is to be expected since I am using a HDMI connector and panel... So your patch will still add the UYVY formats. Either the synposys module inside the jz4780 or the panel does not understand them.

By selecting the UYVY formats, the driver will enable the colorspace converters in the dw-hdmi IP, I don't see why it doesn't work here...

There is a bit called Support Color Space Converter in config0_id:
bit | Name | R/W | Desc
2 | csc | R | Indicates if Color Space Conversion block is present

Could you dump all the config0 bits:

=======================><=============================

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +1 -0

index 54d8fdad395f..547731482da8 100644
@@ -3431,6 +3431,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
       pdevinfo.id = PLATFORM_DEVID_AUTO;
 
       config0 = hdmi_readb(hdmi, HDMI_CONFIG0_ID);
+       dev_info(dev, "config0: %x\n", config0);
       config3 = hdmi_readb(hdmi, HDMI_CONFIG3_ID);
 
       if (iores && config3 & HDMI_CONFIG3_AHBAUDDMA) {

=======================><=============================

If this bit is missing, this would explain the black screen.

Hi Neil,

Quoted 30 lines

Am 02.03.2022 um 11:25 schrieb Neil Armstrong <[email protected]>:

Quoted 3 lines

I added a printk for hdmi->sink_is_hdmi. This returns 1. Which IMHO is to be expected since I am using a HDMI connector and panel... So your patch will still add the UYVY formats. Either the synposys module inside the jz4780 or the panel does not understand them.

By selecting the UYVY formats, the driver will enable the colorspace converters in the dw-hdmi IP, I don't see why it doesn't work here...

There is a bit called Support Color Space Converter in config0_id:
bit | Name | R/W | Desc
2 | csc | R | Indicates if Color Space Conversion block is present

Could you dump all the config0 bits:

=======================><=============================

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +1 -0

index 54d8fdad395f..547731482da8 100644
@@ -3431,6 +3431,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
        pdevinfo.id = PLATFORM_DEVID_AUTO;
 
        config0 = hdmi_readb(hdmi, HDMI_CONFIG0_ID);
+       dev_info(dev, "config0: %x\n", config0);
        config3 = hdmi_readb(hdmi, HDMI_CONFIG3_ID);
 
        if (iores && config3 & HDMI_CONFIG3_AHBAUDDMA) {

=======================><=============================

Quoted 1 line

If this bit is missing, this would explain the black screen.

[    9.291011] dw-hdmi-ingenic 10180000.hdmi: config0: bf

Hm. Or is the color-space conversion of the sw-hdmi module inside the jz4780 broken or not configured properly?

(cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)

BR and thanks,
Nikolaus

Am 02.03.2022 um 15:34 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 1 line

(cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)

I don't understand what's wrong, can you try to make the logic select MEDIA_BUS_FMT_YUV8_1X24 instead of DRM_COLOR_FORMAT_YCBCR422 ?

If your CSC is broken, we'll need to disable it on your platform.

Hi Neil,

Quoted 7 lines

Am 02.03.2022 um 15:34 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 1 line

(cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)

I don't understand what's wrong, can you try to make the logic select MEDIA_BUS_FMT_YUV8_1X24 instead of DRM_COLOR_FORMAT_YCBCR422 ?

I have forced hdmi->sink_is_hdmi = false and replaced

/* Default 8bit RGB fallback */

(quoted diff) +1 -1

-	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+	output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;

And then screen remains black. MEDIA_BUS_FMT_RGB888_1X24 works. (MEDIA_BUS_FMT_VUY8_1X24 doesn't work either).

So this indicates that YUV conversion is not working properly. Maybe missing some special setup.

What I have to test if it works on a different monitor. Not that this specific panel (a 7 inch waveshare touch with HDMIinput) is buggy and reports YUV capabilities but does not handle them...

On the other hand this panel works on RasPi and OMAP5 (where I admit I do not know in which mode).

Quoted 2 lines

If your CSC is broken, we'll need to disable it on your platform.

Indeed.

So it seems as if we need a mechanism to overwrite dw_hdmi_bridge_atomic_get_output_bus_fmts() in our ingenic-dw-hdmi platform specialization [1] to always return MEDIA_BUS_FMT_RGB888_1X24.

Or alternatively set sink_is_hdmi = false there (unfortunately there is no direct access to struct dw_hdmi in a specialization drivers).

Is this already possible or how can it be done?

BR and thanks,
Nikolaus

[1]: https://lore.kernel.org/all/24a27226a22adf5f5573f013e5d7d89b0ec73664.1645895582.git.hns@goldelico.com/

Am 03.03.2022 um 09:35 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 8 lines · H. Nikolaus Schaller

Hi Neil,

Quoted 7 lines

Am 02.03.2022 um 15:34 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 1 line

(cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)

I don't understand what's wrong, can you try to make the logic select MEDIA_BUS_FMT_YUV8_1X24 instead of DRM_COLOR_FORMAT_YCBCR422 ?

Quoted 9 lines

I have forced hdmi->sink_is_hdmi = false and replaced

(quoted diff) +1 -1

 	/* Default 8bit RGB fallback */
-	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+	output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;

And then screen remains black. MEDIA_BUS_FMT_RGB888_1X24 works. (MEDIA_BUS_FMT_VUY8_1X24 doesn't work either).
So this indicates that YUV conversion is not working properly. Maybe missing some special setup.
What I have to test if it works on a different monitor.

Quoted 5 lines

Not that this specific panel
(a 7 inch waveshare touch with HDMIinput) is buggy and reports YUV capabilities but does not handle them...
On the other hand this panel works on RasPi and OMAP5 (where I admit I do not know in which mode).

Pretty sure they don't support YUV HDMI output.

If you can try on a certified HDMI devices like a TV, it would here figuring out where comes the issue.

Quoted 8 lines

Quoted 2 lines

If your CSC is broken, we'll need to disable it on your platform.

Indeed.
So it seems as if we need a mechanism to overwrite dw_hdmi_bridge_atomic_get_output_bus_fmts() in our ingenic-dw-hdmi platform specialization [1] to always return MEDIA_BUS_FMT_RGB888_1X24. Or alternatively set sink_is_hdmi = false there (unfortunately there is no direct access to struct dw_hdmi in a specialization drivers).
Is this already possible or how can it be done?

It's not handled yet, but we may add the logic to handle the lack of CSC config bit and add a glue config bit to override this like we already did for CEC.

I wrote an initial support to disable CSC (only compile-tested), could you try on your platform with setting disable_csc = 1 in your dw-hdmi glue code ?

Hi Neil,

Quoted 13 lines

Am 03.03.2022 um 09:35 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 8 lines · H. Nikolaus Schaller

Hi Neil,

Quoted 7 lines

Am 02.03.2022 um 15:34 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 1 line

(cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)

I don't understand what's wrong, can you try to make the logic select MEDIA_BUS_FMT_YUV8_1X24 instead of DRM_COLOR_FORMAT_YCBCR422 ?

Quoted 9 lines

Quoted 9 lines

I have forced hdmi->sink_is_hdmi = false and replaced
/* Default 8bit RGB fallback */

(quoted diff) +1 -1

-	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+	output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;

And then screen remains black. MEDIA_BUS_FMT_RGB888_1X24 works. (MEDIA_BUS_FMT_VUY8_1X24 doesn't work either).
So this indicates that YUV conversion is not working properly. Maybe missing some special setup.
What I have to test if it works on a different monitor.

Same effect on a Xiaomi monitor (user manual just telling HDMI1,4 compatible), an older Acer video projector and a Sharp TV set.

The Xiaomi monitor does not say "No signal" but shows a black screen. The others do not even report any HDMI signals. All work well with MEDIA_BUS_FMT_RGB888_1X24.

This means the transcoding to YUV does not work properly on the jz4780 SoC setup.

So it looks as if we have to disable it (at least unless someone finds a fix).

Quoted 9 lines

Quoted 5 lines

Not that this specific panel
(a 7 inch waveshare touch with HDMIinput) is buggy and reports YUV capabilities but does not handle them...
On the other hand this panel works on RasPi and OMAP5 (where I admit I do not know in which mode).

Pretty sure they don't support YUV HDMI output.

If you can try on a certified HDMI devices like a TV, it would here figuring out where comes the issue.

I am not sure if the Sharp TV is fully certified but would assume...

Quoted 12 lines

Quoted 8 lines

Quoted 2 lines

If your CSC is broken, we'll need to disable it on your platform.

Indeed.
So it seems as if we need a mechanism to overwrite dw_hdmi_bridge_atomic_get_output_bus_fmts() in our ingenic-dw-hdmi platform specialization [1] to always return MEDIA_BUS_FMT_RGB888_1X24. Or alternatively set sink_is_hdmi = false there (unfortunately there is no direct access to struct dw_hdmi in a specialization drivers).
Is this already possible or how can it be done?

It's not handled yet, but we may add the logic to handle the lack of CSC config bit and add a glue config bit to override this like we already did for CEC.

Quoted 1 line

I wrote an initial support to disable CSC (only compile-tested), could you try on your platform with setting disable_csc = 1 in your dw-hdmi glue code ?

This works!

So how can we get that merged? IMHO your proposal should be before we add ingenic-dw-hdmi. If you have a version with proper commit message I can add it to the beginning of my seried and include it in a v17. Or if you get yours merged to drm-misc/drm-misc-next I can build on top.

BR and thanks,
Nikolaus

Am 03.03.2022 um 12:42 schrieb Neil Armstrong <[email protected]>:

Quoted 4 lines

So how can we get that merged? IMHO your proposal should be before we add ingenic-dw-hdmi. If you have a version with proper commit message I can add it to the beginning of my seried and include it in a v17. Or if you get yours merged to drm-misc/drm-misc-next I can build on top.

You can add it in your v17 patchset with my authorship and my Signed-off-by tag + yours.

As commit message something like :
====================
drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

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.
====================

Am 03.03.2022 um 12:45 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Neil,

Quoted 16 lines

Am 03.03.2022 um 12:42 schrieb Neil Armstrong <[email protected]>:

Quoted 4 lines

So how can we get that merged? IMHO your proposal should be before we add ingenic-dw-hdmi. If you have a version with proper commit message I can add it to the beginning of my seried and include it in a v17. Or if you get yours merged to drm-misc/drm-misc-next I can build on top.

You can add it in your v17 patchset with my authorship and my Signed-off-by tag + yours.

As commit message something like :
====================
drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

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

Quoted 6 lines

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.
====================

Fine! Will do.

Hi Neil,

Quoted 20 lines

Am 03.03.2022 um 12:45 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Neil,

Quoted 16 lines

Am 03.03.2022 um 12:42 schrieb Neil Armstrong <[email protected]>:

Quoted 4 lines

So how can we get that merged? IMHO your proposal should be before we add ingenic-dw-hdmi. If you have a version with proper commit message I can add it to the beginning of my seried and include it in a v17. Or if you get yours merged to drm-misc/drm-misc-next I can build on top.

You can add it in your v17 patchset with my authorship and my Signed-off-by tag + yours.

As commit message something like :
====================
drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

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

Quoted 8 lines

Quoted 6 lines

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.
====================

Fine! Will do.

I was a little too early.

While preparing the patches I found that I still had the hack to force sink_is_hdmi = false in my test branch. Sorry for that.

Removing this made the panel go black again, even with your latest proposal.

So I looked deeper into your patch and it seems to influence the input formats only in dw_hdmi_bridge_atomic_get_input_bus_fmts()?

While the problem I see is with output formats and we had worked on modifying dw_hdmi_bridge_atomic_get_output_bus_fmts().

BR and thanks,
Nikolaus

Quoted 59 lines · H. Nikolaus Schaller

Hi Neil,

Quoted 13 lines

Am 03.03.2022 um 09:35 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 8 lines · H. Nikolaus Schaller

Hi Neil,

Quoted 7 lines

Am 02.03.2022 um 15:34 schrieb Neil Armstrong <[email protected]>:

Hi,

Quoted 1 line

(cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)

I don't understand what's wrong, can you try to make the logic select MEDIA_BUS_FMT_YUV8_1X24 instead of DRM_COLOR_FORMAT_YCBCR422 ?

Quoted 9 lines

Quoted 9 lines

I have forced hdmi->sink_is_hdmi = false and replaced
/* Default 8bit RGB fallback */

(quoted diff) +1 -1

-	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+	output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;

And then screen remains black. MEDIA_BUS_FMT_RGB888_1X24 works. (MEDIA_BUS_FMT_VUY8_1X24 doesn't work either).
So this indicates that YUV conversion is not working properly. Maybe missing some special setup.
What I have to test if it works on a different monitor.

Same effect on a Xiaomi monitor (user manual just telling HDMI1,4 compatible), an older Acer video projector and a Sharp TV set.

The Xiaomi monitor does not say "No signal" but shows a black screen. The others do not even report any HDMI signals. All work well with MEDIA_BUS_FMT_RGB888_1X24.

This means the transcoding to YUV does not work properly on the jz4780 SoC setup.

So it looks as if we have to disable it (at least unless someone finds a fix).

Quoted 9 lines

Quoted 5 lines

Not that this specific panel
(a 7 inch waveshare touch with HDMIinput) is buggy and reports YUV capabilities but does not handle them...
On the other hand this panel works on RasPi and OMAP5 (where I admit I do not know in which mode).

Pretty sure they don't support YUV HDMI output.

If you can try on a certified HDMI devices like a TV, it would here figuring out where comes the issue.

I am not sure if the Sharp TV is fully certified but would assume...

Quoted 12 lines

Quoted 8 lines

Quoted 2 lines

If your CSC is broken, we'll need to disable it on your platform.

Indeed.
So it seems as if we need a mechanism to overwrite dw_hdmi_bridge_atomic_get_output_bus_fmts() in our ingenic-dw-hdmi platform specialization [1] to always return MEDIA_BUS_FMT_RGB888_1X24. Or alternatively set sink_is_hdmi = false there (unfortunately there is no direct access to struct dw_hdmi in a specialization drivers).
Is this already possible or how can it be done?

It's not handled yet, but we may add the logic to handle the lack of CSC config bit and add a glue config bit to override this like we already did for CEC.

Quoted 8 lines

Quoted 1 line

I wrote an initial support to disable CSC (only compile-tested), could you try on your platform with setting disable_csc = 1 in your dw-hdmi glue code ?

This works!

So how can we get that merged? IMHO your proposal should be before we add ingenic-dw-hdmi. If you have a version with proper commit message I can add it to the beginning of my seried and include it in a v17. Or if you get yours merged to drm-misc/drm-misc-next I can build on top.

You can add it in your v17 patchset with my authorship and my Signed-off-by tag + yours.

As commit message something like :
====================
drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

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.
====================

Thanks,
Neil

Quoted 4 lines

BR and thanks,
Nikolaus