[PATCH v3 4/4] drm/v3d: Introduce Runtime Power Management
Maíra Canal
mcanal at igalia.com
Mon Jan 26 06:37:23 PST 2026
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.
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