On 11/24/16 12:25, Tomi Valkeinen wrote: > On 24/11/16 12:03, Jyri Sarha wrote: >> On 11/24/16 11:43, Tomi Valkeinen wrote: >>> What is the difference? If mode changes, you need to disable and enable >>> the crtc, right? What other cases there are to enable the display? After >>> blank? Then the display has been off, and I presume palette has to be >>> loaded. >> >> At the moment the palette or register values do not appear to vanish >> ever. But that is probably due to PM not doing much to optimize the LCDC >> power consumption. > > If runtime PM for the device goes to suspend, you have to presume the IP > has lost all context. That may not always happen, but that's what the > driver has to presume, unless there's a way for the driver to verify > whether the context has been lost or not. > There is couple of registers I can verify that from (reset value != value always used by the driver). >> Anyway, if simple enable is enough to turn on the display - all video >> timings, frame buffer dma addresses etc. are already in the registers - >> then I think it is safe to assume that the palette is still in there too. > > As long as the driver makes sure the device doesn't go to suspend (by > having called pm_runtime_get). Runtime get has always been called when modeset_nofb() is called. > >> Then it is a different issue, that I should probably put the same >> functionality into PM runtime_suspend() and runtime_resume() callbacks, >> that is currently in suspend() and resume() callbacks, to be ready if PM >> ever does anything more for LCDC that it does today. I could of course >> add a test if the registers are still intact before doing a restore. > > You can do things in resume callback, but I think quite often it's > simplest to just do those things when enabling the display. The device > never goes to suspend if the display is enabled. And if you disable the > display, the device should go to suspend, so usually enable is called > after the device has been in suspend. > Well, the two places are pretty much the same thing in tilcdc, because enable calls pm_runtime_get(). Also with atomic the suspend/resume implementation is really straight forward. Just get the current atomic state with drm_atomic_helper_suspend() and commit it back in with drm_atomic_helper_resume(), if needed. I don't think I should implement myself something that is so well provided by pm and drm infrastructure. I will implement that as soon as I get this current mess with LCDC rev 1 and bridge support sorted out. > So, I haven't looked at the tilcdc code in detail in this regard, but my > guess is that runtime PM resume could be used to set hardcoded things to > registers, stuff that you always know how they are set. Everything else > can be just programmed at enable. > > Looking at the registers to find out if they're intact is fine, but it's > just an optimization. > Yes, of course. BR, Jyri
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
