[PATCH v3 4/4] drm/v3d: Introduce Runtime Power Management
Melissa Wen
mwen at igalia.com
Mon Jan 26 06:45:47 PST 2026
On 26/01/2026 11:37, Maíra Canal wrote:
> Hi Melissa,
>
> On 26/01/26 10:16, Melissa Wen wrote:
>>
>>
>> On 16/01/2026 17:19, Maíra Canal wrote:
>
> [...]
>
>>> + ret = pm_runtime_resume_and_get(dev);
>>> + if (ret)
>>> + goto gem_destroy;
>>> +
>>> + /* If PM is disabled, we need to call v3d_power_resume()
>>> manually. */
>>> + if (!IS_ENABLED(CONFIG_PM)) {
>>> + ret = v3d_power_resume(dev);
>>> + if (ret)
>>> + goto gem_destroy;
>>> + }
>>
>> Curious, I expected that previous pm_runtime attempts to enable and
>> resume would trigger a -ENOSYS if no CONFIG_PM.
>
> Looking at the code, I see the following call chain:
>
> pm_runtime_resume_and_get() -> pm_runtime_get_active(dev, 0) ->
> __pm_runtime_resume(dev, RPM_GET_PUT)
>
> With CONFIG_PM=n, the stub __pm_runtime_resume() returns 1, so the
> function succeeds unconditionally.
Yes, I noticed that too, and also in the _enable() call.
I found it curious because, on the other hand, I see -ENOSYS in suspend
paths.
Moreover, this is also triggered by some calls that we don't check the
return error.
But I don't have enough background to understand why some calls return
ENOSYS and others not.
>
> Please let me know if my understanding is mistaken.
>
> Thanks a lot for your review! I'll address all your comments in the next
> version.
>
> Best regards,
> - Maíra
>
>>
>>> mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
>>> mask = DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug,
>>> V3D_MMU_PA_WIDTH));
>>> ret = dma_set_mask_and_coherent(dev, mask);
>>> if (ret)
>>> - goto clk_disable;
>>> + goto runtime_pm_put;
>>> dma_set_max_seg_size(&pdev->dev, UINT_MAX);
>>> @@ -433,25 +421,27 @@ static int v3d_platform_drm_probe(struct
>>> platform_device *pdev)
>>> v3d->rev = V3D_GET_FIELD(ident3, V3D_HUB_IDENT3_IPREV);
>>> v3d_gem_init(drm);
>>> - v3d_irq_enable(v3d);
>>> +
>>> + pm_runtime_set_autosuspend_delay(dev, 50);
>>> + pm_runtime_use_autosuspend(dev);
>>> ret = drm_dev_register(drm, 0);
>>> if (ret)
>>> - goto irq_disable;
>>> + goto runtime_pm_put;
>>> ret = v3d_sysfs_init(dev);
>>> if (ret)
>>> goto drm_unregister;
>>> + pm_runtime_mark_last_busy(dev);
>>> + pm_runtime_put_autosuspend(dev);
>>> +
>>> return 0;
>>> drm_unregister:
>>> drm_dev_unregister(drm);
>>> -irq_disable:
>>> - v3d_irq_disable(v3d);
>>> -clk_disable:
>>> - v3d_power_off_sms(v3d);
>>> - clk_disable_unprepare(v3d->clk);
>>> +runtime_pm_put:
>>> + pm_runtime_put_sync_suspend(dev);
>>> gem_destroy:
>>> v3d_gem_destroy(drm);
>>> dma_free:
>>> @@ -469,21 +459,27 @@ static void v3d_platform_drm_remove(struct
>>> platform_device *pdev)
>>> drm_dev_unregister(drm);
>>> - v3d_power_off_sms(v3d);
>>> + pm_runtime_suspend(dev);
>>> - clk_disable_unprepare(v3d->clk);
>>> + /* If PM is disabled, we need to call v3d_power_suspend()
>>> manually. */
>>> + if (!IS_ENABLED(CONFIG_PM))
>>> + v3d_power_suspend(dev);
>>> v3d_gem_destroy(drm);
>>> dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
>>> }
>>> +static DEFINE_RUNTIME_DEV_PM_OPS(v3d_pm_ops, v3d_power_suspend,
>>> + v3d_power_resume, NULL);
>>> +
>>> static struct platform_driver v3d_platform_driver = {
>>> .probe = v3d_platform_drm_probe,
>>> .remove = v3d_platform_drm_remove,
>>> .driver = {
>>> .name = "v3d",
>>> .of_match_table = v3d_of_match,
>>> + .pm = pm_ptr(&v3d_pm_ops),
>>> },
>>> };
>>> diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/
>>> v3d_drv.h
>>> index
>>> 738a09351c306db33078db1e053cd133d55d2138..32835b83caf0309a9e316d6882f63685f58bb6e3
>>> 100644
>>> --- a/drivers/gpu/drm/v3d/v3d_drv.h
>>> +++ b/drivers/gpu/drm/v3d/v3d_drv.h
>>> @@ -3,6 +3,7 @@
>>> #include <linux/delay.h>
>>> #include <linux/mutex.h>
>>> +#include <linux/pm_runtime.h>
>>> #include <linux/spinlock_types.h>
>>> #include <linux/workqueue.h>
>>> @@ -134,6 +135,8 @@ struct v3d_dev {
>>> void __iomem *gca_regs;
>>> void __iomem *sms_regs;
>>> struct clk *clk;
>>> + unsigned long max_clk_rate;
>>> +
>>> struct reset_control *reset;
>>> /* Virtual and DMA addresses of the single shared page table. */
>>> @@ -324,6 +327,8 @@ struct v3d_job {
>>> /* Callback for the freeing of the job on refcount going to 0. */
>>> void (*free)(struct kref *ref);
>>> +
>>> + bool has_pm_ref;
>>> };
>>> struct v3d_bin_job {
>>> @@ -597,6 +602,21 @@ int v3d_mmu_set_page_table(struct v3d_dev *v3d);
>>> void v3d_mmu_insert_ptes(struct v3d_bo *bo);
>>> void v3d_mmu_remove_ptes(struct v3d_bo *bo);
>>> +/* v3d_power.c */
>>> +int v3d_power_suspend(struct device *dev);
>>> +int v3d_power_resume(struct device *dev);
>>> +
>>> +static __always_inline int v3d_pm_runtime_get(struct v3d_dev *v3d)
>>> +{
>>> + return pm_runtime_resume_and_get(v3d->drm.dev);
>>> +}
>>> +
>>> +static __always_inline int v3d_pm_runtime_put(struct v3d_dev *v3d)
>>> +{
>>> + pm_runtime_mark_last_busy(v3d->drm.dev);
>>> + return pm_runtime_put_autosuspend(v3d->drm.dev);
>>> +}
>>> +
>>> /* v3d_sched.c */
>>> void v3d_timestamp_query_info_free(struct v3d_timestamp_query_info
>>> *query_info,
>>> unsigned int count);
>>> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/
>>> v3d_gem.c
>>> index
>>> 1f532030c3883257810877c75da38636bf25f58e..70e488180c4684db3415201f19586099914afb15
>>> 100644
>>> --- a/drivers/gpu/drm/v3d/v3d_gem.c
>>> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
>>> @@ -125,10 +125,16 @@ void
>>> v3d_reset(struct v3d_dev *v3d)
>>> {
>>> struct drm_device *dev = &v3d->drm;
>>> + int ret;
>>> +
>>> + ret = v3d_pm_runtime_get(v3d);
>>> + if (ret)
>>> + return;
>>> DRM_DEV_ERROR(dev->dev, "Resetting GPU for hang.\n");
>>> DRM_DEV_ERROR(dev->dev, "V3D_ERR_STAT: 0x%08x\n",
>>> V3D_CORE_READ(0, V3D_ERR_STAT));
>>> +
>>> trace_v3d_reset_begin(dev);
>>> /* XXX: only needed for safe powerdown, not reset. */
>>> @@ -147,6 +153,8 @@ v3d_reset(struct v3d_dev *v3d)
>>> v3d_perfmon_stop(v3d, v3d->active_perfmon, false);
>>> trace_v3d_reset_end(dev);
>>> +
>>> + v3d_pm_runtime_put(v3d);
>>> }
>>> static void
>>> @@ -344,7 +352,6 @@ v3d_gem_init(struct drm_device *dev)
>>> struct v3d_dev *v3d = to_v3d_dev(dev);
>>> v3d_init_hw_state(v3d);
>>> - v3d_mmu_set_page_table(v3d);
>>
>> Do we still need v3d_gem_init() only wrapping v3d_init_hw_state() ?
>>
>>> }
>>> void
>>> diff --git a/drivers/gpu/drm/v3d/v3d_mmu.c b/drivers/gpu/drm/v3d/
>>> v3d_mmu.c
>>> index
>>> a25d25a8ae617bf68e133e1793cd0bb930bb07f6..1699819756aadfc40f7d41ff19847d42ddf10dce
>>> 100644
>>> --- a/drivers/gpu/drm/v3d/v3d_mmu.c
>>> +++ b/drivers/gpu/drm/v3d/v3d_mmu.c
>>> @@ -37,7 +37,13 @@ static bool v3d_mmu_is_aligned(u32 page, u32
>>> page_address, size_t alignment)
>>> int v3d_mmu_flush_all(struct v3d_dev *v3d)
>>> {
>>> - int ret;
>>> + int ret = 0;
>>> +
>>> + pm_runtime_get_noresume(v3d->drm.dev);
>>> +
>>> + /* Flush the PTs only if we're already awake */
>>> + if (!pm_runtime_active(v3d->drm.dev))
>>> + goto pm_put;
>>> V3D_WRITE(V3D_MMUC_CONTROL, V3D_MMUC_CONTROL_FLUSH |
>>> V3D_MMUC_CONTROL_ENABLE);
>>> @@ -46,7 +52,7 @@ int v3d_mmu_flush_all(struct v3d_dev *v3d)
>>> V3D_MMUC_CONTROL_FLUSHING), 100);
>>> if (ret) {
>>> dev_err(v3d->drm.dev, "MMUC flush wait idle failed\n");
>>> - return ret;
>>> + goto pm_put;
>>> }
>>> V3D_WRITE(V3D_MMU_CTL, V3D_READ(V3D_MMU_CTL) |
>>> @@ -57,6 +63,8 @@ int v3d_mmu_flush_all(struct v3d_dev *v3d)
>>> if (ret)
>>> dev_err(v3d->drm.dev, "MMU TLB clear wait idle failed\n");
>>> +pm_put:
>>> + pm_runtime_put_autosuspend(v3d->drm.dev);
>>> return ret;
>>> }
>>> diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/
>>> v3d_power.c
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..285f56acf544bbfd3d9848253e788a138aacf2af
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/v3d/v3d_power.c
>>> @@ -0,0 +1,96 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/* Copyright (C) 2026 Raspberry Pi */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/reset.h>
>>> +
>>> +#include <drm/drm_print.h>
>>> +
>>> +#include "v3d_drv.h"
>>> +#include "v3d_regs.h"
>>> +
>>> +static void
>>> +v3d_resume_sms(struct v3d_dev *v3d)
>>> +{
>>> + if (v3d->ver < V3D_GEN_71)
>>> + return;
>>> +
>>> + V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
>>> +
>>> + if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
>>> + V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
>>> + drm_err(&v3d->drm, "Failed to power up SMS\n");
>>> + }
>>> +
>>> + v3d_reset_sms(v3d);
>>> +}
>>> +
>>> +static void
>>> +v3d_suspend_sms(struct v3d_dev *v3d)
>>> +{
>>> + if (v3d->ver < V3D_GEN_71)
>>> + return;
>>> +
>>> + V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
>>> +
>>> + if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
>>> + V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE),
>>> 100)) {
>>> + drm_err(&v3d->drm, "Failed to power off SMS\n");
>>> + }
>>> +}
>>> +
>>> +int v3d_power_suspend(struct device *dev)
>>> +{
>>> + struct drm_device *drm = dev_get_drvdata(dev);
>>> + struct v3d_dev *v3d = to_v3d_dev(drm);
>>> +
>>> + v3d_irq_disable(v3d);
>>> + v3d_suspend_sms(v3d);
>>> +
>>> + if (v3d->reset)
>>> + reset_control_assert(v3d->reset);
>>> +
>>> + /* Some firmware versions might not actually power off the clock
>>> + * when we set the clock state to off. Therefore, set the clock
>>> + * rate to 0 to ensure it is running in the minimum rate.
>>> + */
>>> + if (v3d->clk)
>>> + clk_set_rate(v3d->clk, 0);
>>> +
>>> + clk_disable_unprepare(v3d->clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int v3d_power_resume(struct device *dev)
>>> +{
>>> + struct drm_device *drm = dev_get_drvdata(dev);
>>> + struct v3d_dev *v3d = to_v3d_dev(drm);
>>> + int ret;
>>> +
>>> + ret = clk_prepare_enable(v3d->clk);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Set the clock to the maximum rate and let the firmware decide
>>> + * if we can actually keep it.
>>> + */
>>> + if (v3d->clk)
>>> + clk_set_rate(v3d->clk, v3d->max_clk_rate);
>>> +
>>> + if (v3d->reset) {
>>> + ret = reset_control_deassert(v3d->reset);
>>> + if (ret)
>>> + goto clk_disable;
>>> + }
>>> +
>>> + v3d_resume_sms(v3d);
>>> + v3d_mmu_set_page_table(v3d);
>>> + v3d_irq_enable(v3d);
>>> +
>>> + return 0;
>>> +
>>> +clk_disable:
>>> + clk_disable_unprepare(v3d->clk);
>>> + return ret;
>>> +}
>>> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/
>>> v3d_submit.c
>>> index
>>> 7de5a95ee7ca92d480af1f2996c12f2cefa56f34..7487aff499f4587b2887a886c366d735952cee95
>>> 100644
>>> --- a/drivers/gpu/drm/v3d/v3d_submit.c
>>> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
>>> @@ -103,6 +103,9 @@ v3d_job_free(struct kref *ref)
>>> if (job->perfmon)
>>> v3d_perfmon_put(job->perfmon);
>>> + if (job->has_pm_ref)
>>> + v3d_pm_runtime_put(job->v3d);
>>> +
>>> kfree(job);
>>> }
>>> @@ -184,13 +187,13 @@ v3d_job_init(struct v3d_dev *v3d, struct
>>> drm_file *file_priv,
>>> if (copy_from_user(&in, handle++, sizeof(in))) {
>>> ret = -EFAULT;
>>> DRM_DEBUG("Failed to copy wait dep handle.\n");
>>> - goto fail_deps;
>>> + goto fail_job_init;
>>> }
>>> ret = drm_sched_job_add_syncobj_dependency(&job-
>>> >base, file_priv, in.handle, 0);
>>> // TODO: Investigate why this was filtered out for
>>> the IOCTL.
>>> if (ret && ret != -ENOENT)
>>> - goto fail_deps;
>>> + goto fail_job_init;
>>> }
>>> }
>>> } else {
>>> @@ -198,14 +201,22 @@ v3d_job_init(struct v3d_dev *v3d, struct
>>> drm_file *file_priv,
>>> // TODO: Investigate why this was filtered out for the IOCTL.
>>> if (ret && ret != -ENOENT)
>>> - goto fail_deps;
>>> + goto fail_job_init;
>>> + }
>>> +
>>> + /* CPU jobs don't require hardware resources */
>>> + if (queue != V3D_CPU) {
>>> + ret = v3d_pm_runtime_get(v3d);
>>> + if (ret)
>>> + goto fail_job_init;
>>> + job->has_pm_ref = true;
>>> }
>>> kref_init(&job->refcount);
>>> return 0;
>>> -fail_deps:
>>> +fail_job_init:
>>> drm_sched_job_cleanup(&job->base);
>>> return ret;
>>> }
>>>
>>
>
More information about the linux-arm-kernel
mailing list