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.
1. 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).
2. 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?
3. 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.
4. 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.
5. 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.
