Message-ID: <2z3yvvtd6ttsd7qw43sl5svtfijxevdr6omxxmws64k6l5qv55@5nnh2b32h2ep>
Date:   Tue, 7 Nov 2023 11:57:49 +0100
From:   Maxime Ripard <mripard@...nel.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Dave Stevenson <dave.stevenson@...pberrypi.com>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Douglas Anderson <dianders@...omium.org>,
        Rob Clark <robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Sean Paul <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Jessica Zhang <quic_jesszhan@...cinc.com>,
        Marek Vasut <marex@...x.de>, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.org
Subject: Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over
 the DSI link power state

+GKH

On Thu, Oct 26, 2023 at 11:41:34AM +0300, Dmitry Baryshkov wrote:
> > > > Also, we would still need to update every single panel driver, which is
> > > > going to create a lot of boilerplate that people might get wrong.
> > >
> > > Yes, quite unfortunately. Another approach that I have in mind is to add two
> > > callbacks to mipi_dsi_device. This way the DSI host will call into the
> > > device to initialise it once the link has been powered up and just before
> > > tearing it down. We solve a lot of problems this way, no boilerplate and the
> > > panel / bridge are in control of the initialisation procedure. WDYT?
> > >
> > > > I have the feeling that we should lay out the problem without talking
> > > > about any existing code base first. So, what does the MIPI-DSI spec
> > > > requires and what does panels and bridges expect?
> > >
> > > There is not that much in the DSI spec (or maybe I do not understand the
> > > question). The spec is more about the power states and the commands. Our
> > > problem is that this doesn't fully match kernel expectations.
> >
> > You're explicitly asking for comments on that series. How can we provide
> > any comment if you're dead-set on a particular implementation and not
> > explain what the problem you are trying to solve is?
> 
> Ah, excuse me. I thought that I explained that in the cover letter.
> 
> DSI device lifetime has three different stages:
> 1. before the DSI link being powered up and clocking,
> 2. when the DSI link is in LP state (for the purpose of this question,
> this is the time between the DSI link being powered up and the video
> stream start)
> 3. when the DSI link is in HS state (while streaming the video).
> 
> Different DSI bridges have different requirements with respect to the
> code being executed at stages 1 and 2. For example several DSI-to-eDP
> bridges (ps8640, tc358767 require for the link to be quiet during
> reset time.
> The DSI-controlled bridges and DSI panels need to send some commands
> in stage 2, before starting up video
> 
> In the DRM subsystem stage 3 naturally maps to the
> drm_bridge_funcs::enable, stage 1 also naturally maps to the
> drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in
> the DRM call chain.
> Earlier we attempted to solve that using the pre_enable_prev_first,
> which remapped pre-enable callback execution order. However it has led
> us to the two issues. First, at the DSI host driver we do not know
> whether the panel / bridge were updated to use pre_enable_prev_first
> or not. Second, if the bridge has to perform steps during both stages
> 1 and 2, it can not do that.
> 
> I'm trying to find a way to express the difference between stages 1
> and 2 in the generic code, so that we do not to worry about particular
> DSI host and DSI bridge / panel peculiarities when implementing the
> DSI host and/or DSI panel driver.
> 
> Last, but not least, we currently document that it is fine to call DSI
> transfer functions at any point during the driver's life time (at
> least that was the interpretation that we have agreed in the
> DSI-related threads). It has its own drawbacks for the DSI host
> drivers. The hosts have to deal with the DSI commands being sent at
> the different times, when the host is fully powered down, when it is
> running in the LP mode and when it is fully running and streaming
> video. By defining DSI lifetime more precisely, we can limit the
> period when the DSI commands can be legitimately sent, simplifying DSI
> host drives.

Thanks for writing this :)

> > Thinking more about it, I'm even more skeptical about the general
> > approach that this should be implemented at the bridge level (or in
> > KMS).
> >
> > It looks to me that this is very much a bus problem. USB device drivers
> > also require the bus to be powered and generally available to send data
> > to their device, and you don't fix that up in the HID or storage
> > drivers, you make the bus behave that way.
> >
> > What prevents us from fixing it at the bus level?
> 
> Yes, this can also be possible. Do you mean adding code / callbacks to
> struct mipi_dsi_device ?

Yes, even more so with your summary above, I really think this should be
dealt with at the bus layer.

To put it in a different way, if we had an (imaginary, probably)
MIPI-DSI device that didn't fit into KMS but in any other framework, we
would still have all the constraints you list above. It really is a bus
matter, not something that bridges need to express.

I had a look at the other buses we have in the kernel and it looks like
HSI might have the same requirements?

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists