LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Eric Anholt <eric@anholt.net>
To: Andrzej Hajda <a.hajda@samsung.com>, dri-devel@lists.freedesktop.org
Cc: Kevin Quigley <kevin@kquigley.co.uk>,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	linux-kernel@vger.kernel.org,
	James Hughes <james.hughes@raspberrypi.org>,
	Archit Taneja <architt@codeaurora.org>
Subject: Re: [PATCH] drm/vc4: Enable the DSI module and link before other enables.
Date: Tue, 05 Jun 2018 11:49:53 -0700	[thread overview]
Message-ID: <87lgbtt6em.fsf@anholt.net> (raw)
In-Reply-To: <20180605082532eucas1p21328ec5bea7d588c81480829f862bdfd~1NhnMydDJ0562405624eucas1p2l@eucas1p2.samsung.com>

[-- Attachment #1: Type: text/plain, Size: 2804 bytes --]

Andrzej Hajda <a.hajda@samsung.com> 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@anholt.net>
>> Cc: Kevin Quigley <kevin@kquigley.co.uk>
>> Cc: James Hughes <james.hughes@raspberrypi.org>
>> Cc: Boris Brezillon <boris.brezillon@bootlin.com>
>> ---
>>
>> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2018-06-05 18:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20180604194448epcas1p29627759e2d8c0b1388c6700a7da77a45@epcas1p2.samsung.com>
2018-06-04 19:44 ` Eric Anholt
2018-06-05  8:25   ` Andrzej Hajda
2018-06-05 18:49     ` Eric Anholt [this message]
2018-06-06  8:28       ` Andrzej Hajda
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=87lgbtt6em.fsf@anholt.net \
    --to=eric@anholt.net \
    --cc=a.hajda@samsung.com \
    --cc=architt@codeaurora.org \
    --cc=boris.brezillon@bootlin.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=james.hughes@raspberrypi.org \
    --cc=kevin@kquigley.co.uk \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH] drm/vc4: Enable the DSI module and link before other enables.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

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