Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state

[Maxime Ripard]

+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

Attachment: signature.asc
Description: PGP signature