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