[PATCH v3 02/11] drm/tilcdc: implement palette loading for rev1

[Daniel Vetter]

On Thu, Nov 24, 2016 at 10:32:59PM +0200, Jyri Sarha wrote:
> On 11/24/16 14:56, Tomi Valkeinen wrote:
> > On 24/11/16 14:03, Jyri Sarha wrote:
> > 
> >>> The suspend/resume you're talking there is the system suspend/resume.
> >>> That's quite different than the runtime suspend/resume, and they should
> >>> do very different things.
> >>>
> >>> The current system suspend/resume in tilcdc looks fine.
> >>>
> >>
> >> I don't undestand the same mechanism could not be used for runtime
> >> resume. Why should it matter if we configure the previous drm atomic
> >> state after system suspend or simple LCDC power down suspend?
> > 
> > They are quite different things.
> > 
> > For example, you have display up and running. The user requests for
> > system suspend. System suspend callback is called in tilcdc. That
> > callback should turn off the displays etc.
> > 
> > Runtime PM suspend should not do anything like that, because it's called
> > when the driver says the IP is not used, which means the driver has
> > already turned off the displays etc.
> > 
> > So, for example, when the system suspend happens, tilcdc's system
> > suspend callback disables the displays by calling some functions. These
> > functions stop the HW, maybe do other cleanups, and then do
> > pm_runtime_put(). This then causes runtime PM suspend callback to be called.
> > 
> > And, of course, runtime PM suspend is called anytime the driver is not
> > using the HW, not only when system suspend happens.
> > 
> > So, system suspend is a high level thing, comes at any point of time
> > from the userspace. Runtime PM is a driver internal thing, comes from
> > the driver.
> > 
> 
> I am aware of the difference, but in theory I thought it should work,
> because the situation is pretty much the same. We need restore the state
> that was in LCDC when it was shut down in disable and restore the state
> right before we are turning it back on, all the runtime_get() and puts
> would serialize nicely.
> 
> But now after giving it a bit more thought, the drm locking prevents
> this from working. Who ever is enabling the display, is already holding
> some modeset locks and prevents drm_atomic_helper_resume() from taking
> the drm_modeset_lock_all().
> 
> Actually following your suggestion appears to be really straight
> forward. Simply get rid of mode_set_nofb() callback and call the same
> function in enable just before turning the raster on.

Yup, that's the right way to do this, and the kernel-doc for mode_set_nofb
even explains it ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch