lkml.org 
[lkml]   [2022]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 12/26] drm: etnaviv: Remove #ifdef guards for PM related functions
Hi Lucas,

Le lun. 7 nov. 2022 à 19:07:32 +0100, Lucas Stach
<l.stach@pengutronix.de> a écrit :
> Am Montag, dem 07.11.2022 um 17:52 +0000 schrieb Paul Cercueil:
>> Use the RUNTIME_PM_OPS() and pm_ptr() macros to handle the
>> .runtime_suspend/.runtime_resume callbacks.
>>
>> These macros allow the suspend and resume functions to be
>> automatically
>> dropped by the compiler when CONFIG_PM is disabled, without having
>> to use #ifdef guards.
>>
>> This has the advantage of always compiling these functions in,
>> independently of any Kconfig option. Thanks to that, bugs and other
>> regressions are subsequently easier to catch.
>>
>> Some #ifdef CONFIG_PM guards were protecting simple statements, and
>> were
>> also converted to "if (IS_ENABLED(CONFIG_PM))".
>>
> Reasoning and the change itself look good.

That's an ack? :)

>> Note that this driver should probably use the
>> DEFINE_RUNTIME_DEV_PM_OPS() macro instead, which will provide
>> .suspend/.resume callbacks, pointing to pm_runtime_force_suspend()
>> and
>> pm_runtime_force_resume() respectively; unless those callbacks
>> really
>> aren't needed.
>
> This however isn't true, specifically this driver can _not_ use
> pm_runtime_force_suspend, as the GPU can't be forced into suspend by
> calling the rpm callback. A real suspend implementation would first
> need to make sure the GPU finished working on the current queued jobs,
> only then it would be able to power it down via the rpm suspend
> callback.

Understood. I'll remove this paragraph if I have to V2.

Cheers,
-Paul

>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Russell King <linux+etnaviv@armlinux.org.uk>
>> Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
>> Cc: etnaviv@lists.freedesktop.org
>> ---
>> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 30
>> +++++++++++----------------
>> 1 file changed, 12 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> index 37018bc55810..e9a5444ec1c7 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> @@ -1605,7 +1605,6 @@ static int etnaviv_gpu_hw_suspend(struct
>> etnaviv_gpu *gpu)
>> return etnaviv_gpu_clk_disable(gpu);
>> }
>>
>> -#ifdef CONFIG_PM
>> static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu)
>> {
>> int ret;
>> @@ -1621,7 +1620,6 @@ static int etnaviv_gpu_hw_resume(struct
>> etnaviv_gpu *gpu)
>>
>> return 0;
>> }
>> -#endif
>>
>> static int
>> etnaviv_gpu_cooling_get_max_state(struct thermal_cooling_device
>> *cdev,
>> @@ -1689,11 +1687,10 @@ static int etnaviv_gpu_bind(struct device
>> *dev, struct device *master,
>> if (ret)
>> goto out_workqueue;
>>
>> -#ifdef CONFIG_PM
>> - ret = pm_runtime_get_sync(gpu->dev);
>> -#else
>> - ret = etnaviv_gpu_clk_enable(gpu);
>> -#endif
>> + if (IS_ENABLED(CONFIG_PM))
>> + ret = pm_runtime_get_sync(gpu->dev);
>> + else
>> + ret = etnaviv_gpu_clk_enable(gpu);
>> if (ret < 0)
>> goto out_sched;
>>
>> @@ -1737,12 +1734,12 @@ static void etnaviv_gpu_unbind(struct
>> device *dev, struct device *master,
>>
>> etnaviv_sched_fini(gpu);
>>
>> -#ifdef CONFIG_PM
>> - pm_runtime_get_sync(gpu->dev);
>> - pm_runtime_put_sync_suspend(gpu->dev);
>> -#else
>> - etnaviv_gpu_hw_suspend(gpu);
>> -#endif
>> + if (IS_ENABLED(CONFIG_PM)) {
>> + pm_runtime_get_sync(gpu->dev);
>> + pm_runtime_put_sync_suspend(gpu->dev);
>> + } else {
>> + etnaviv_gpu_hw_suspend(gpu);
>> + }
>>
>> if (gpu->mmu_context)
>> etnaviv_iommu_context_put(gpu->mmu_context);
>> @@ -1856,7 +1853,6 @@ static int etnaviv_gpu_platform_remove(struct
>> platform_device *pdev)
>> return 0;
>> }
>>
>> -#ifdef CONFIG_PM
>> static int etnaviv_gpu_rpm_suspend(struct device *dev)
>> {
>> struct etnaviv_gpu *gpu = dev_get_drvdata(dev);
>> @@ -1899,18 +1895,16 @@ static int etnaviv_gpu_rpm_resume(struct
>> device *dev)
>>
>> return 0;
>> }
>> -#endif
>>
>> static const struct dev_pm_ops etnaviv_gpu_pm_ops = {
>> - SET_RUNTIME_PM_OPS(etnaviv_gpu_rpm_suspend,
>> etnaviv_gpu_rpm_resume,
>> - NULL)
>> + RUNTIME_PM_OPS(etnaviv_gpu_rpm_suspend, etnaviv_gpu_rpm_resume,
>> NULL)
>> };
>>
>> struct platform_driver etnaviv_gpu_driver = {
>> .driver = {
>> .name = "etnaviv-gpu",
>> .owner = THIS_MODULE,
>> - .pm = &etnaviv_gpu_pm_ops,
>> + .pm = pm_ptr(&etnaviv_gpu_pm_ops),
>> .of_match_table = etnaviv_gpu_match,
>> },
>> .probe = etnaviv_gpu_platform_probe,
>
>


\
 
 \ /
  Last update: 2022-11-07 22:04    [W:2.985 / U:0.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site