Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly

From: Doug Anderson <dianders@chromium.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Sankeerth Billakanti <quic_sbillaka@quicinc.com>,
	Philip Chen <philipchen@chromium.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>,
	LKML <linux-kernel@vger.kernel.org>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Robert Foss <robert.foss@linaro.org>,
	Stephen Boyd <swboyd@chromium.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Hsin-Yi Wang <hsinyi@chromium.org>
Subject: Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
Date: Tue, 3 May 2022 16:23:56 -0700	[thread overview]
Message-ID: <CAD=FV=WSXUB_yQWtCVQuHvnAWvPJDaiCTyc5o5cR3fH78nJ3hA@mail.gmail.com> (raw)
In-Reply-To: <CAD=FV=Ur3afHhsXe7a3baWEnD=MFKFeKRbhFU+bt3P67G0MVzQ@mail.gmail.com>

Hi,

On Mon, Apr 18, 2022 at 4:10 PM Doug Anderson <dianders@chromium.org> wrote:
>
> > > 5. In general I've been asserting that it should be up to the panel to
> > > power things on and drive all AUX transactions. ...but clearly my
> > > model isn't reality. We certainly do AUX transactions from the DP
> > > driver because the DP driver needs to know things about the connected
> > > device, like the number of lanes it has, the version of eDP it
> > > supports, and the available bit rates to name a few. Those things all
> > > work today by relying on the fact that pre-enable powers the panel on.
> > > It's pretty easy to say that reading the EDID (and I guess AUX
> > > backlight) is the odd one out. So right now I guess my model is:
> > >
> > > 5a) If the panel code wants to access the AUX bus it can do so by
> > > powering itself on and then just doing an AUX transaction and assuming
> > > that the provider of the AUX bus can power itself on as needed.
> > >
> > > 5b) If the DP code wants to access the AUX bus it should make sure
> > > that the next bridge's pre_enable() has been called. It can then
> > > assume that the device is powered on until the next bridge's
> > > post_disable() has been called.
> > >
> > > So I guess tl;dr: I'm not really a huge fan of the DP driver powering
> > > the panel on by doing a pm_runtime_get() on it. I'd prefer to keep
> > > with the interface that we have to pre_enable() the panel to turn it
> > > on.
> >
> > Again, thank for the explanation. Your concerns make more sense now.
> > As much as I hate writing docs, maybe we should put at least basic notes
> > (regarding panel requirements) somewhere to the DP/DP AUX documentation?
>
> Sure. I actually don't mind writing docs, but my problem is trying to
> figure out where the heck to put them where someone would actually
> notice them. I could throw a blurb in the kernel doc for `struct
> drm_dp_aux` I guess? How about a deal: if you can tell me where to put
> the above facts (essentially 5a and 5b) then I'm happy to post a patch
> adding them.
>
> Huh, actually, maybe the right place is in the "transfer" function of
> that structure which, as of commit bacbab58f09d ("drm: Mention the
> power state requirement on side-channel operations"), actually has a
> blurb. ...but I don't think the blurb there is totally complete. What
> if I changed it to this:
>
>  * Also note that this callback can be called no matter the
>  * state @dev is in and also no matter what state the panel is
>  * in. It's expected:
>  * - If the @dev providing the AUX bus is currently unpowered then
>  *   it will power itself up for the transfer.
>  * - If the panel is not in a state where it can respond (it's not
>  *   powered or it's in a low power state) then this function will
>  *   fail. Note that if a panel driver is initiating a DP AUX transfer
>  *   it may power itself up however it wants. All other code should
>  *   use the pre_enable() bridge chain (which eventually calls the
>  *   panel prepare function) to power the panel.

I didn't ignore this documentation request. Please take a look here
and see what you think:

https://lore.kernel.org/r/20220503162033.1.Ia8651894026707e4fa61267da944ff739610d180@changeid

-Doug