Hi Jagan,
W dniu 09.08.2021 o 10:00, Jagan Teki pisze:
Hi Andrzej,
On Wed, Aug 4, 2021 at 7:48 PM a.hajda a.hajda@samsung.com wrote:
Hi Maxime,
I have been busy with other tasks, and I did not follow the list last time, so sorry for my late response.
On 28.07.2021 15:32, Maxime Ripard wrote:
Hi,
We've encountered an issue with the RaspberryPi DSI panel that prevented the whole display driver from probing.
The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi: Only register our component once a DSI device is attached"), but the basic idea is that since the panel is probed through i2c, there's no synchronization between its probe and the registration of the MIPI-DSI host it's attached to.
We initially moved the component framework registration to the MIPI-DSI Host attach hook to make sure we register our component only when we have a DSI device attached to our MIPI-DSI host, and then use lookup our DSI device in our bind hook.
However, all the DSI bridges controlled through i2c are only registering their associated DSI device in their bridge attach hook, meaning with our change
I guess this is incorrect. I have promoted several times the pattern that device driver shouldn't expose its interfaces (for example component_add, drm_panel_add, drm_bridge_add) until it gathers all required dependencies. In this particular case bridges should defer probe until DSI bus becomes available. I guess this way the patch you reverts would work.
I advised few times this pattern in case of DSI hosts, apparently I didn't notice the similar issue can appear in case of bridges. Or there is something I have missed???
Look like Maxime is correct. I2C based DSI bridge will get probe during bridge_attach which usually called from bridge driver bridge_attach call. Non-I2C bridges and DSI panels will get probe during host.attach. We do get similar situation for dw-mipi-dsi bridges, where icn6211 bridge is not I2C-based bridge and it gets probed in host_attach and sn65dsi83 is I2C based bridge and it gets probed in bridge_attach.
Here is the simple call trace we have observed with dw-mipi-dsi bridge when all possible DSI device are trying to probe.
DSI panels and bridges will invoke the host attach from probe in order to find the panel_or_bridge.
chipone_probe() dw_mipi_dsi_host_attach().start dw_mipi_dsi_panel_or_bridge() ...found the panel_or_bridge...
ltdc_encoder_init().start dw_mipi_dsi_bridge_attach().start dw_mipi_dsi_host_attach().start chipone_attach(). start
chipone_attach(). done dw_mipi_dsi_host_attach().done dw_mipi_dsi_bridge_attach(). doneltdc_encoder_init().done
I2C based DSI bridge will invoke the drm_bridge_attach from bridge attach in order to find the panel_or_bridge.
ltdc_encoder_init().start dw_mipi_dsi_bridge_attach().start dw_mipi_dsi_panel_or_bridge() ...found the panel_or_bridge... dw_mipi_dsi_host_attach().start sn65dsi83_attach(). start
sn65dsi83_attach(). done dw_mipi_dsi_host_attach().done dw_mipi_dsi_bridge_attach(). doneltdc_encoder_init().done
It is correct that the I2C-based bridges will attach the host via mipi_dsi_attach in driver bridge API where as it done in probe for Non-I2C bridges and DSI panels.
The call order depends on the registration time of DSI host. In case of dw-mipi-dsi it is called from .component_bind callback (dsi_bind-> dsi_host_init -> mipi_dsi_host_register). And this is "the original sin" :)
dw-mipi-dsi calls component_add without prior acquiring its dependency - drm_bridge and before DSI host registration. In such situation bridge author should follow this pattern and perform similar initialization: first drm_bridge_add, then mipi_dsi_attach.
And now authors of bridges are in dead end in case they want their bridge/panel drivers cooperate with dw-mipi-dsi host (with pattern look for sink - bridge/panel, then register DSI bus) and with other DSI hosts (most of then use pattern - register DSI bus then look for the sink - panel or bridge).
Quick look at the DSI hosts suggests the pattern get-sink-then-register-bus are used only by kirin/dw_drm_dsi.c and msm/dsi.
All other DSI hosts uses apparently register-bus-then-get-sink pattern - as I said it was not profound analysis - just few greps of some keywords.
Anyway there are already eleven(?) bridge drivers using this pattern. I wonder if fixing it would be difficult, or if it expose other issues??? The patches should be quite straightforward - move of_find_mipi_dsi_host_by_node and mipi_dsi_device_register_full to probe time.
Finally I think that if we will not fix these bridge drivers we will encounter another set of issues with new platforms connecting "DSI host drivers assuming this pattern" and "i2c/dsi device drivers assuming pattern already present in the bridges".
Agreed, I'm trying to understand the several ways to fix this. Right now I'm trying this on sun6i_mipi_dsi and exynos_drm_dsi. Will let you know for any update and suggestions on the same.
Quick look at sun6i suggests it uses register-bus-then-get-sink pattern (incompatible with kirin), only issue is that currently it support only panel sink.
Exynos uses also register-bus-then-get-sink pattern, but it slightly different as it supports dynamic attach/detach of sinks.
Regards
Andrzej
Thanks, Jagan.