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

[Tomi Valkeinen]

On 24/11/16 12:38, Jyri Sarha wrote:

>> 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.

Yes, runtime get is needed to access the HW, but the question here was
"has runtime_get been active ever since setting the registers previously".

If you do

runtime_get();
setup_things();
runtime_put();

runtime_get();
setup_more_things();
runtime_put();

the code is broken as the context is possibly lost between those two
blocks. Unless runtime suspend/resume callbacks do a full context save
and restore.

>>> 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.

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.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161124/64cb4620/attachment.sig>