Il 13/06/23 01:32, Douglas Anderson ha scritto:
In order to read the EDID from an eDP panel, you not only need to power on the bridge chip itself but also the panel. In the ps8640 driver, this was made to work by having the bridge chip manually power the panel on by calling pre_enable() on everything connectorward on the bridge chain. This worked OK, but... ...when trying to do the same thing on ti-sn65dsi86, feedback was that this wasn't a great idea. As a result, we designed the "DP AUX" bus. With the design we ended up with the panel driver itself was in charge of reading the EDID. The panel driver could power itself on and the bridge chip was able to power itself on because it implemented the DP AUX bus. Despite the fact that we came up with a new scheme, implemented in on ti-sn65dsi86, and even implemented it on parade-ps8640, we still kept the old code around. This was because the new scheme required a DT change. Previously the panel was a simple "platform_device" and in DT at the top level. With the new design the panel needs to be listed in DT under the DP controller node. The old code allowed us to properly fetch EDIDs with ps8640 with the old DTs. Unfortunately, the old code stopped working as of commit 102e80d1fa2c ("drm/bridge: ps8640: Use atomic variants of drm_bridge_funcs"). There are cases at bootup where connector->state->state is NULL and the kernel crashed at: * drm_atomic_bridge_chain_pre_enable * drm_atomic_get_old_bridge_state * drm_atomic_get_old_private_obj_state A bit of digging was done to see if there was an easy fix but there was nothing obvious. Instead, the only device using ps8640 the "old" way had its DT updated so that the panel was no longer a simple "platform_deice". See commit c2d94f72140a ("arm64: dts: mediatek: mt8173-elm: Move display to ps8640 auxiliary bus") and commit 113b5cc06f44 ("arm64: dts: mediatek: mt8173-elm: remove panel model number in DT"). Let's delete the old, crashing code so nobody gets tempted to copy it or figure out how it works (since it doesn't). NOTE: from a device tree "purist" point of view, we're supposed to keep old device trees working and this patch is technically "against policy". Reasons I'm still proposing it anyway: 1. Officially, old mt8173-elm device trees worked via the "little white lie" approach. The DT would list an arbitrary/representative panel that would be used for power sequencing. The mode information in the panel driver would then be ignored / overridden by the EDID reading code in ps8640. I don't feel too terrible breaking DTs that contained the wrong "compatible" string to begin with. NOTE that any old device trees that _didn't_ lie about their compatible will still work because the mode information will come from the hardcoded panels in panel-edp. 2. The only users of the old code were Chromebooks and Chromebooks don't bake their DTs into the BIOS (they are bundled with the kernel). Thus we don't need to worry about breaking someone using an old DT with a new kernel. 3. The old code was crashing anyway. If someone wants to fix the old code instead of deleting it then they have my blessing, but without a proper fix the old code isn't useful. I'll list this as "Fixing" the code that made the old code start failing. There's not lots of reason to bring this back any further than that.
Hoping to see removal of non-aux EDID reading code from all drivers that can support aux-bus is exactly why I moved Elm to the proper... aux-bus.. so... Yes! Let's go!
Fixes: 102e80d1fa2c ("drm/bridge: ps8640: Use atomic variants of drm_bridge_funcs")
...but this Fixes tag will cause this commit to be backported to kernel versions before my commit moving Elm to aux-bus, and break display on those. I would suggest to either find a different Fixes tag, or don't add any, since technically this is a deprecation commit. We could imply that the old technique is deprecated since kernel version X.Y and get away with it. Otherwise, if you want it backported *anyway*, the safest way would be to Cc it to stable and explicitly say which versions should it be backported to. I really want to give my R-b tag to this one. Cheers! Angelo