Re: [PATCH v2 7/8] drm/bridge/synopsys: dsi: add dual-dsi support

Andrzej Hajda
2018-07-09T16:02:51Z

On 09.07.2018 15:45, Heiko Stuebner wrote:
> Am Dienstag, 3. Juli 2018, 19:07:02 CEST schrieb Andrzej Hajda:
>> On 18.06.2018 12:28, Heiko Stuebner wrote:
>>> From: Nickey Yang <nickey.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>>
>>> Allow to also drive a slave dw-mipi-dsi controller in a dual-dsi
>>> setup. This will require additional implementation-specific
>>> code to look up the slave instance and do specific setup.
>>> Also will probably need code in the specific crtcs as dual-dsi
>>> does not equal two separate dsi outputs.
>>>
>>> To activate, the implementation-specific code should set the slave
>>> using dw_mipi_dsi_set_slave() before calling __dw_mipi_dsi_bind().
>>>
>>> v2:
>>> - expect real interface number of lanes
>>> - keep links to both master and slave
>> I did not see the whole driver/pipeline, but it seems the point of this
>> patch is to perform the same work on the slave as on the master in case
>> of dual mode. I think DSI should not be a place for it,
>> DSI masters usually are stupid devices from display stack PoV, they just
>> convert video streams, in dual mode also. In this case the panel and/or
>> crtc adds complications so they should be responsible for handling it.
>> Panel should:
>> - register its both mipi interfaces with proper mode_flags (maybe some
>> dual-mode indication flags should be added if necessary),
>> - register drm_panel for both interfaces (it requires change in
>> drm_panel api), and provide video mode timings.
>> - in case it needs perform transfers perform it to master/slave/both
>> interfaces according to its needs,
>>
>> I am not sure about DRM pipeline, it should model, maybe it could be
>> done this way:
>> CRTC -->ENCODER0(dsi master) --> CONNECTOR0 (panel interface 0)
>> |---> ENCODER1(dsi slave) --> CONNECTOR1 (panel interface 1)
>>
>> But I am not sure if it is not reserved only for mirroring.
>>
>> For me more tempting solution is to create meta-encoder-connector let's
>> call it dual-encoder (maybe it could be even generic), which is visible
>> to userspace as single pipeline and encapsulates both dsi bridges/panel
>> inputs. So its every callback will be translated usually to sequence of
>> callbacks to 1st and 2nd dsi, or in case of get_modes it should return
>> mode which represent sum of modes in both panels.
>> Maybe it looks more complicated, but it can be more universal - you can
>> use it with different bridges/panels even two single-panels if necessary.
>>
>> Of course I do not see the whole picture, or I can be just wrong, or
>> just freaking purist :). If there are arguments against my vision please
>> provide them.
>> I am also not strongly against your solution, I just want to show
>> alternatives, which could be better/more generic.
> I cannot really claim to know the correct way to go forward, as these
> drm-internals are still very much unknown land for me, but current
> thought points I had on this:
>
> - strawman argument ;-) : this is the same way tegra handles dual-dsi
> in its ganged mode ... also with regards to panel handling
> With Thierry helming both panels and the tegra dual-dsi change

Yes, and I remember our disagreement on the subject :)

>
> - while the panel may expose two DSI data interfaces, there is still
> only one power-sequence and also only one init-command sequence.

As in case of almost all devices exposing two, or more interfaces.

> So I don't think you can really handle a dual-dsi panels as two fully
> separate panels, creating instead the need for both panel-instances
> to work together

The question is if they can work separately? Ie is it possible to use
only one interface, and have picture only on left/top side, even/odd
lines? Or to have connected interfaces to two independent display
controllers (of course via some dsi bridges)? Btw I have not seen
bindings/driver for the panel, what is the panel name?
The answer for above questions would be helpful.
If the only way of work is with both dsi interfaces, exposing one
drm_panel maybe could be OK.

The point is if the panel has two interfaces it should be his
responsibility for handling it, not the poor DSI host, which is just for
converting native video signal (RGB I guess) to DSI. The other
responsible for the mess is CRTC which actually should negotiate with
panel how the data is split between both channels (left/right,
top/bottom, odd/even, ....). DSI-host here serves only as data
transport, in fact it does not need to know if it works in dual mode or
not (from HW point of view) - dual mode is just something which only
CRTC and panel should be really aware.

See what will happen to your solution if in next version of hardware vendor:
1. replace dual-panel with two panels glued together.
2. replace DSI bridges with different ones.
3. replace only one DSI briges - ie two different bridges working in
dual mode.

>
> - Right now we have two data-points on dual-something voodoo
> (tegra+rockchip)
> So for me it sounds hard to design something generic that survives
> the first non-dsi use-case.

As I said I do not insist on the solution, but I want to show you it can
be dead-end. On the other side maybe dual dsi solutions will die
eventually soon, so there is no point in polishing it :)

Regards
Andrzej

>
> The devicetree binding (see cover-letter) should be pretty stable, as
> a panel like this will need to be bound to one of the two dsi controllers
> so nothing hinders code changes later on if the need for a more generic
> solution comes up.
>
>
> Heiko
>
>
>
>
>