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

Heiko Stuebner
2018-07-09T13:45:52Z

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@rock-chips.com>
> >
> > 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

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

- 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.

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