W dniu 14.09.2021 o 16:35, Maxime Ripard pisze:
Hi,
On Mon, Sep 13, 2021 at 08:29:37AM +0200, Andrzej Hajda wrote:
W dniu 10.09.2021 o 12:11, Maxime Ripard pisze:
Interactions between bridges, panels, MIPI-DSI host and the component framework are not trivial and can lead to probing issues when implementing a display driver. Let's document the various cases we need too consider, and the solution to support all the cases.
Signed-off-by: Maxime Ripard maxime@cerno.tech
Documentation/gpu/drm-kms-helpers.rst | 6 +++ drivers/gpu/drm/drm_bridge.c | 57 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+)
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 10f8df7aecc0..ec2f65b31930 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -157,6 +157,12 @@ Display Driver Integration .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :doc: display driver integration
+Special Care with MIPI-DSI bridges +----------------------------------
+.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
- :doc: special care dsi
Bridge Operations
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index baff74ea4a33..7cc2d2f94ae3 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -96,6 +96,63 @@ * documentation of bridge operations for more details). */
+/**
- DOC: special care dsi
- The interaction between the bridges and other frameworks involved in
- the probing of the upstream driver and the bridge driver can be
- challenging. Indeed, there's multiple cases that needs to be
- considered:
- The upstream driver doesn't use the component framework and isn't a
- MIPI-DSI host. In this case, the bridge driver will probe at some
- point and the upstream driver should try to probe again by returning
- EPROBE_DEFER as long as the bridge driver hasn't probed.
- The upstream driver doesn't use the component framework, but is a
- MIPI-DSI host. The bridge device uses the MIPI-DCS commands to be
- controlled. In this case, the bridge device is a child of the
- display device and when it will probe it's assured that the display
- device (and MIPI-DSI host) is present. The upstream driver will be
- assured that the bridge driver is connected between the
- &mipi_dsi_host_ops.attach and &mipi_dsi_host_ops.detach operations.
- Therefore, it must run mipi_dsi_host_register() in its probe
- function, and then run drm_bridge_attach() in its
- &mipi_dsi_host_ops.attach hook.
- The upstream driver uses the component framework and is a MIPI-DSI
- host. The bridge device uses the MIPI-DCS commands to be
- controlled. This is the same situation than above, and can run
- mipi_dsi_host_register() in either its probe or bind hooks.
- The upstream driver uses the component framework and is a MIPI-DSI
- host. The bridge device uses a separate bus (such as I2C) to be
- controlled. In this case, there's no correlation between the probe
- of the bridge and upstream drivers, so care must be taken to avoid
- an endless EPROBE_DEFER loop, with each driver waiting for the
- other to probe.
- The ideal pattern to cover the last item (and all the others in the
- MIPI-DSI host driver case) is to split the operations like this:
- The MIPI-DSI host driver must run mipi_dsi_host_register() in its
- probe hook. It will make sure that the MIPI-DSI host sticks around,
- and that the driver's bind can be called.
- In its probe hook, the bridge driver must try to find its MIPI-DSI
- host, register as a MIPI-DSI device and attach the MIPI-DSI device
- to its host. The bridge driver is now functional.
- In its &struct mipi_dsi_host_ops.attach hook, the MIPI-DSI host can
- now add its component. Its bind hook will now be called and since
- the bridge driver is attached and registered, we can now look for
- and attach it.
- At this point, we're now certain that both the upstream driver and
- the bridge driver are functional and we can't have a deadlock-like
- situation when probing.
- */
- static DEFINE_MUTEX(bridge_lock); static LIST_HEAD(bridge_list);
Nice work with documenting this initialization dance. It clearly shows that bridge API lacks better mechanism - usage of mipi dsi callbacks to get notifications about bridge appearance is ugly.
Yeah, there's so many moving parts it's definitely not great.
It remains me my resource tracking patches which I have posted long time ago [1] - they would solve the issue in much more elegant way, described here [2]. Apparently I was not stubborn enough in promoting this solution.
Wow, that sounds like a massive change indeed :/
Anyway:
Reviewed-by: Andrzej Hajda a.hajda@samsung.com
I assume you'll want me to hold off that patch before someone reviews the rest?
The last exynos patch should be dropped, kirin patch should be tested/reviewed/acked by kirin maintaner. I am not sure about bridge patches, which ones have been tested by you, and which one have other users.
If yes it would be good to test them as well - changes in initialization flow can beat sometimes :)
I think patches 1-4 can be merged earlier, if you like, as they are on the list for long time.
Regards
Andrzej
Thanks! Maxime