LKML Archive on
help / color / mirror / Atom feed
From: Andrzej Hajda <>
To: Eric Anholt <>,
Cc: Kevin Quigley <>,
	Boris Brezillon <>,,
	James Hughes <>,
	Archit Taneja <>
Subject: Re: [PATCH] drm/vc4: Enable the DSI module and link before other enables.
Date: Wed, 6 Jun 2018 10:28:02 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 05.06.2018 20:49, Eric Anholt wrote:
> Andrzej Hajda <> 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 <>
>>> Cc: Kevin Quigley <>
>>> Cc: James Hughes <>
>>> Cc: Boris Brezillon <>
>>> ---
>>> 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

I like the idea, I guess the flag should be placed in "struct
drm_bridge". Since I plan to propose flag to avoid device links in
panels/bridges maybe it would be good to set flags directly, or by
helper similar to irq_set_status_flags, instead of creating separate
helper for each flag.
I am not sure about warns from atomic helpers, maybe it would be enough
to track and verify state transitions of bridges similarly to the ones
proposed (and abandoned?) by Sean [1].

And little off-topic: all these duplication of
ideas/code/functionalities added to/from panels from/to bridges and this
crazy code:
    sink = drm_of_find_panel_or_bridge...
    if (sink is panel)
    else if (sink is bridge)
laying in multiple encoders/bridges, provokes me to raise again a
question, wouldn't be better to merge drm_bridge and drm_panel into one
drm_sink object.



  reply	other threads:[~2018-06-06  8:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
2018-06-04 19:44 ` Eric Anholt
2018-06-05  8:25   ` Andrzej Hajda
2018-06-05 18:49     ` Eric Anholt
2018-06-06  8:28       ` Andrzej Hajda [this message]
2018-06-07 17:22     ` Kevin Quigley
2018-06-21 23:19       ` Eric Anholt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] drm/vc4: Enable the DSI module and link before other enables.' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).