Re: [PATCH] drm/vc4: Enable the DSI module and link before other enables.

[Eric Anholt]

Andrzej Hajda <a.hajda@xxxxxxxxxxx> writes:

> On 04.06.2018 21:44, Eric Anholt wrote:
>> We want the DSI PHY to be enabled and the DSI module clocked before a
>> panel or bridge's prepare() function, since most DSI panel drivers
>> with a prepare() hook are doing DCS transactions inside of them.
>>
>> Signed-off-by: Eric Anholt <eric@xxxxxxxxxx>
>> Cc: Kevin Quigley <kevin@xxxxxxxxxxxxxx>
>> Cc: James Hughes <james.hughes@xxxxxxxxxxxxxxx>
>> Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
>> ---
>>
>> I'm not sure it makes sense to enable CRTCs before encoders on vc4 at
>> all.  Why start scanning pixels before the encoder is ready to hear
>> about VSTART?  However, this patch will hopefully unblock people
>> trying to attach other panels to rpi
>
> But this patch is about enabling encoder before panel/bridge prepare. Or
> am I missing something.
> Anyway I would be more precise here, MIPI-DSI bus is used for two things:
> - control bus - accessing DSI device (configuration/detection,...),
> - video data bus - sending video stream.
>
> I suspect in prepare/pre_enable phase of DSI panel/bridge only control
> bus should be functional, video stream transfer does not make sense.
> So the best solution (I guess) would be to split DSI-encoder enable
> sequence into control bus enable and video transfer enable parts and
> call the former before 1st transfer request from device and the latter
> in encoder enable callback. Of course there will be a problem then with
> disabling sequence, ie stopping video stream should be performed in
> encoder's disable, but when we should disable control bus? If we do it
> in encoder's disable callback we could not perform transfers in
> post_disable cb of the bridge - in most cases (maybe all currently
> present in kernel) it will work, but it does not look ideal.
> All this recalls me discussion that hardwiring bridge callbacks into drm
> core is problematic and maybe it would be better to call downstream
> callbacks explicitly from upstream element - the callback order should
> depend on the local bus protocol, and should not be fixed in drm core.

It does seem like for DSI encoders they generally would need to be able
to control when the call down to the bridge happens.  However, from my
experience with panels, drivers are bad about calling both prepare and
enable, if their particular panel only cares about one of them.  So, how
about:

- encoders can call drm_bridge_disable_midlayer_calls() (any naming
  suggestions?) to flag this bridge as not wanting the calls from the
  atomic helpers.

- atomic helpers WARN if bridge midlayer_calls flag was set and
  drm_bridge_pre_enable/enable/disable/post_disable failed to be called
  by the encoder

- drm_bridge_detach() clears disable_midlayer_calls flag for the next
  encoder

Attachment: signature.asc
Description: PGP signature