Hi Paul,

Am 23.09.2021 um 10:49 schrieb Paul Cercueil [off-list ref]:

Hi Nikolaus,

Le jeu., sept. 23 2021 at 07:52:08 +0200, H. Nikolaus Schaller [off-list ref] a écrit :

quoted

Hi Paul,
thanks for another update.
We have been delayed to rework the CI20 HDMI code on top of your series
but it basically works in some situations. There is for example a problem
if the EDID reports DRM_COLOR_FORMAT_YCRCB422 but it appears to be outside
of your series.

I think the SoC can output YCbCr as well, but I never tried to use it.

Maybe there is code missing or something else. We have not yet deeply researched.
Except that when ignoring DRM_COLOR_FORMAT_YCRCB422 capability it uses RGB
and works.

quoted

quoted

+		ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
+					DRM_BRIDGE_ATTACH_NO_CONNECTOR);

DRM_BRIDGE_ATTACH_NO_CONNECTOR makes it fundamentally incompatible
with synopsys/dw_hdmi.c
That driver checks for DRM_BRIDGE_ATTACH_NO_CONNECTOR being NOT present,
since it wants to register its own connector through dw_hdmi_connector_create().
It does it for a reason: the dw-hdmi is a multi-function driver which does
HDMI and DDC/EDID stuff in a single driver (because I/O registers and power
management seem to be shared).

The IT66121 driver does all of that too, and does not need DRM_BRIDGE_ATTACH_NO_CONNECTOR. The drm_bridge_funcs struct has callbacks to handle cable detection and DDC stuff.

quoted

Since I do not see who could split this into a separate bridge and a connector driver
and test it on multiple SoC platforms (there are at least 3 or 4), I think modifying
the fundamentals of the dw-hdmi architecture just to get CI20 HDMI working is not
our turf.

You could have a field in the dw-hdmi pdata structure, that would instruct the driver whether or not it should use the new API. Ugly, I know, and would probably duplicate a lot of code, but that would allow other drivers to be updated at a later date.

Yes, would be very ugly.

But generally who has the knowledge (and time) to do this work?
And has a working platform to test (jz4780 isn't a good development environment)?

The driver seems to have a turbulent history starting 2013 in staging/imx and
apparently it was generalized since then... Is Laurent currently dw-hdmi maintainer?

quoted

Therefore the code here should be able to detect if drm_bridge_attach() already
creates and attaches a connector and then skip the code below.

Not that easy, unfortunately. On one side we have dw-hdmi which checks that DRM_BRIDGE_ATTACH_NO_CONNECTOR is not set, and on the other side we have other drivers like the IT66121 which will fail if this flag is not set.

Ok, I see. You have to handle contradicting cases here.

Would it be possible to run it with DRM_BRIDGE_ATTACH_NO_CONNECTOR first
and retry if it fails without?

But IMHO the return value (in error case) is not well defined. So there
must be a test if a connector has been created (I do not know how this
would work).

Another suggestion: can you check if there is a downstream connector defined in
device tree (dw-hdmi does not need such a definition)?
If not we call it with 0 and if there is one we call it with
DRM_BRIDGE_ATTACH_NO_CONNECTOR and create one?

Just some ideas how to solve without touching hdmi drivers.

BR and thanks,
Nikolaus