Hi Paul, > Am 23.09.2021 um 10:49 schrieb Paul Cercueil <paul@xxxxxxxxxxxxxxx>: > > Hi Nikolaus, > > Le jeu., sept. 23 2021 at 07:52:08 +0200, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> a écrit : >> 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. > >>> + 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. > >> 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? > >> 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
