[PATCH v2 0/7] drm/exynos: add pm runtime support

[Inki Dae]

2015-11-03 22:24 GMT+09:00 Andrzej Hajda <a.hajda at samsung.com>:
> Hi Inki,
>
> On 11/03/2015 11:47 AM, Inki Dae wrote:
>> This patch series adds pm runtime support for Exynos drm.
>>
>> Originally, this patch was posted by Gustavo but there was no any
>> answer about some comments. So I rebased this patch series on top of
>> exynos-drm-next, removed unnecessary patches and modified wrong macro.
>
> I have sent comment to original patchset[1], but for some strange reasons
> it went only to mailing lists.
> My concerns were as follows:
> - exynos_drm has already pm_runtime support via exynos_drm_drv pm ops,
>   why should we add per component support?

For this, I think I already mentioned enough,
        http://www.spinics.net/lists/linux-samsung-soc/msg47695.html

In brief, exynos_drm_drv doesn't control each power domain of each kms
device. It means that we cannot power off fully without pm runtime
interface of each kms device - just they can only control their clock not
power domain. So question. How we could control power domain of each kms
device without runtime pm interface?

> - component suspend sequence is non deterministic, but in case of
>   video pipelines, specification often requires fixed order,

Can your show me an example? I think component suspend and resume are
invoked by only drm dpms interface, and the dpms interface has a fixed
order - when dpms goes to off, encoder first, and when dpms goes to on,
crtc first.

My only concern is that runtime pm interface of each kms driver is called
within pm sleep context of Exynos drm driver. So I will look into this no
problem.

> - the patchset adds implicit dependency on PM_SLEEP.
>
>
> Current solution should work correctly and it was OK last time I have
tested it.
> I am not sure about atomic requirements, are there special ones?

Not related to atomic feature.

>
> There are other issues with current solution, rather easy to solve:
> - it assumes that exynos-drm device will be suspended first - it should
be true,
>  as it is created at the end and suspend order is reverse to creation
order, but
>  I am not sure if we can rely on it - some solution is to add pm
callbacks to
>  all components, and from those callbacks call one centralized pm routine,

You mean pm callbacks are pm sleep interface? And you mean let's add sleep
pm interface to each kms driver?  If so, I'm not agree with you because
sleep pm should be controlled by only drm top like single driver does.

> - suspend/resume callbacks theoretically can be called during component
>   master initialization/deinitailization it could be racy,

I'm not sure what you mean but component master
initialization/deinitailization mean bind and unbind? If so, bind/unbind
interfaces of all components will never call pm relevant interfaces but the
the pm relevant interfaces are called by only dpms interface.

> - exynos_drm_sys_suspend/resume calls exynos_drm_suspend/resume
>   for historical reasons, these function can be merged together.

Also can you show me more detail?

Thanks,
Inki Dae

>
> [1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/48395
>
> Regards
> Andrzej
>
>>
>> Changelog v2:
>> - Remove patch 5 and 6.
>>   . commit callback are already removed so isn't required anymore.
>> - Remove patch 8 which makes dp clock enabled directly from FIMD.
>>   . Really not mendatory for FIMD uses DP, and it could be different
>>     according to Board.
>> - Modified CONFIG_PM_SLEEP to CONFIG_PM.
>>   . In case of runtime pm, CONFIG_PM macro should be used instead of
>>     CONFIG_PM_SLEEP.
>>
>> Gustavo Padovan (7):
>>   drm/exynos: do not start enabling DP at bind() phase
>>   drm/exynos: add pm_runtime to DP
>>   drm/exynos: add pm_runtime to HDMI
>>   drm/exynos: add pm_runtime to Mixer
>>   drm/exynos: add pm_runtime to FIMD
>>   drm/exynos: add pm_runtime to DECON 5433
>>   drm/exynos: add pm_runtime to DECON 7
>>
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  54 ++++++---
>>  drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 125 +++++++++----------
>>  drivers/gpu/drm/exynos/exynos_dp_core.c       | 165
+++++++++++++++++++-------
>>  drivers/gpu/drm/exynos/exynos_dp_core.h       |   1 +
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  91 ++++++--------
>>  drivers/gpu/drm/exynos/exynos_hdmi.c          |  56 ++++++---
>>  drivers/gpu/drm/exynos/exynos_mixer.c         | 125 ++++++++++---------
>>  7 files changed, 352 insertions(+), 265 deletions(-)
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151104/355db834/attachment-0001.html>