[PATCH v6 6/6] drm/v3d: Introduce Runtime Power Management

[Stefan Wahren]

Hi Maíra,

just 2 nits.

Am 18.02.26 um 21:45 schrieb Maíra Canal:
> Commit 90a64adb0876 ("drm/v3d: Get rid of pm code") removed the last
> bits of power management code that V3D had, which were actually never
> hooked. Therefore, currently, the GPU clock is enabled during probe
> and only disabled when removing the driver.
>
> Implement proper power management using the kernel's Runtime PM
> framework.
>
> Signed-off-by: Maíra Canal <mcanal at igalia.com>
> ---
>   drivers/gpu/drm/v3d/Makefile      |  3 +-
>   drivers/gpu/drm/v3d/v3d_debugfs.c | 23 ++++++++++-
>   drivers/gpu/drm/v3d/v3d_drv.c     | 84 ++++++++++++++++++---------------------
>   drivers/gpu/drm/v3d/v3d_drv.h     | 17 ++++++++
>   drivers/gpu/drm/v3d/v3d_mmu.c     | 10 ++++-
>   drivers/gpu/drm/v3d/v3d_perfmon.c | 18 +++++++--
>   drivers/gpu/drm/v3d/v3d_power.c   | 83 ++++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/v3d/v3d_submit.c  | 19 +++++++--
>   8 files changed, 198 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/Makefile b/drivers/gpu/drm/v3d/Makefile
> index b7d673f1153bef16db3800e50b2bfaf36bf8871b..9cb1fd4e4091dbb56e6a73e2b8a51fa0d242698b 100644
> --- a/drivers/gpu/drm/v3d/Makefile
> +++ b/drivers/gpu/drm/v3d/Makefile
> @@ -13,7 +13,8 @@ v3d-y := \
>   	v3d_trace_points.o \
>   	v3d_sched.o \
>   	v3d_sysfs.o \
> -	v3d_submit.o
> +	v3d_submit.o \
> +	v3d_power.o
Please keep the alphabetical order.
>   
>   v3d-$(CONFIG_DEBUG_FS) += v3d_debugfs.o
>   
...
>   
> diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/v3d_power.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f3d30ef5de4ea6ecdbd21c438b8af54bdf4d0dba
> --- /dev/null
> +++ b/drivers/gpu/drm/v3d/v3d_power.c
> @@ -0,0 +1,83 @@
> +// 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);
Shouldn't we handle the error case?
> +
> +	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;
> +
> +	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 794c3571662de7eb566bf4c0561571d7618dd234..fea7573505cd0d2b84d21efda88ed4da657d18d4 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_dbg(&v3d->drm, "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;
>   }
>