Hi Jagan,
On 25.11.2021 07:33, Jagan Teki wrote:
Hi Andrej and all,
Andrzej :)
I'm trying to convert existing exynos dsi driver to bridge and make them accessible for i.MX8MM platform.
I've a few questions on the existing exynos dsi driver and which is indeed incompatible to proceed to make the bridge conversion.
There were few problems the design of exynos_drm solved:
- avoid late exynos_drm creation - when exynos_drm deferred it's probe (waiting for panel, which was waiting for regulator, which was waiting for ...) , after some point of booting userspace assumed there is no graphic console at all and we ended with black screen - I do not know if it is still valid reason,
- avoid crashes due to panel unbind/rebind - it is still valid, if I remember correctly.
How it was solved? Exynos_drm does not wait for non-componetized panels (bridge support was added later) it is created as soon as all components are probed. It uses DSI attach/detach notifications and hotplug event to signal the panel appeared or will be removed, then the panel is dynamically added/removed from the pipeline - it is safe because pipeline is in disconnected state and componentized encoder has full control of panel ops.
Adding support for bridges complicated things, but the principle was similar.
With bridges decision was taken to not use bridge chain because:
- it has fixed and from DSI PoV broken call order - the issue many DSI developers are facing,
- it passes control of the bridge to DRM core - encoder loses the control.
To be clear, I am not a maintainer of exynos so I can only advice :)
As I understand the idea in your patchset is to convert exynos_dsi to standard bridge, and loose above 'benefits'?
If so then lets go to the details.
- Hotplug event
Commit from 295e7954c0d3fdbe0550d13e3cf4dd4604d42c68 which is waiting for drm to hotplug the downstream devices like panel or bridge to probe.
Any idea how it works?
As described earlier exynos_drm is already working but since there is no panel DSI pipeline is in disconnected state.
In exynos_dsi_host_attach exynos_dsi knows the panel has been registered so it finds it, attaches it to the pipeline and gather panel properties.
It is done under mutex as DSI attach is asynchronous call from DRM PoV.
Then hotplug event is generated to inform DRM about the change.
what if we move drm_bridge_attach in bind callback so-that binding will start once all the devices get attached.
Yes, but you should also postpone bind till the bridge appears (ie, move component_add to the end of dsi_host_attach cb).
- Host register in bind
Usual host registration is done in the probe, but the driver registers host in bind once the in_bridge is attached. any idea why?
Host registration is in bind to be sure exynos_drm is already created.
Putting DSI host registration in probe would create multiple concurrent scenarios in dsi_host_attach/detach callbacks:
1. exynos_drm not yet created - we should postpone panel attach.
2. exynos_drm created - we can attach now.
Since DSI and DRM frameworks are asynchronous, it is possible that creation/removal of exynos_drm can happen during dsi_host_attach/detach cb. So it would require additional synchronization.
So the above is the simplest solution.
Regarding in_bridge I am not sure how it exactly works I was not involved in it's 'design'.
What if we find the DSI as an output port in MIC and start attaching from there?
- CRTC handling in DSI
Commit from c038f53842cf840889473d219ace7f9121694e8d is trying to send the DSI flags information to CRTC with a function call. any specific reason for this? Any proper way of doing this move out from DSI?
Some panel info should be somehow passed to display controller (CRTC), at the time of writing it was the simplest one - as ExynosDSI was the part of Exynos Display Subsystem.
i80 stands for command mode, you can look at other command mode drivers (I hope there are some) how it can be done in more generic way.
- Mutex calls while assigning device attributes.
Assignment of lanes, format, mode_flags are done in mutex context, I think we can even do it in normal context isn't it? or any specific reason for doing this?
DSI host callbacks are asynchronous with DRM. With careful approach you could probably omit them.
- Clock rates.
pll_clk_rate, burst_clk_rate, burst_clk_rate are these clock rates retrieved from DT. which is not a proper way to support multi platform. I think pll-clock and burst-clock are computed based on panel pixel or bridge clocks. any specific computation for these to handle dynamically on code?
I also though about it, there was even some discussion on dri-devel about it. Calculated clocks seems for me more generic as well, bu on other side often platform specs provides only fixed ratings often encoded in initialization sequence of devices. I guess we should support both approaches. Of course it does not mean there is no place for improvement.
Regards
Andrzej
All this information is essential for me to move this further as I don't have direct hardware and I'm trying to take some help from Marek Szyprowski.
Please take some time, and help me.
Thanks, Jagan.