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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
