On 14/08/2024 12:21, Mary Guillemard wrote:
> Extend the uAPI with a new job requirement flag for cycle
> counters. This requirement is used by userland to indicate that a job
> requires cycle counters or system timestamp to be propagated. (for use
> with write value timestamp jobs)
>
> We cannot enable cycle counters unconditionally as this would result in
> an increase of GPU power consumption. As a result, they should be left
> off unless required by the application.
>
> If a job requires cycle counters or system timestamps propagation, we
> must enable cycle counting before issuing a job and disable it right
> after the job completes.
>
> Since this extends the uAPI and because userland needs a way to advertise
> features like VK_KHR_shader_clock conditionally, we bumps the driver
> minor version.
>
> v2:
> - Rework commit message
> - Squash uAPI changes and implementation in this commit
> - Simplify changes based on Steven Price comments
>
> Signed-off-by: Mary Guillemard <mary.guillem...@collabora.com>
Reviewed-by: Steven Price <steven.pr...@arm.com>
Thanks,
Steve
> ---
> drivers/gpu/drm/panfrost/panfrost_drv.c | 8 +++++--
> drivers/gpu/drm/panfrost/panfrost_job.c | 28 +++++++++++++++----------
> include/uapi/drm/panfrost_drm.h | 1 +
> 3 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 83696d06d697..07a09f32c32e 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -25,6 +25,8 @@
> #include "panfrost_gpu.h"
> #include "panfrost_perfcnt.h"
>
> +#define JOB_REQUIREMENTS (PANFROST_JD_REQ_FS | PANFROST_JD_REQ_CYCLE_COUNT)
> +
> static bool unstable_ioctls;
> module_param_unsafe(unstable_ioctls, bool, 0600);
>
> @@ -280,7 +282,7 @@ static int panfrost_ioctl_submit(struct drm_device *dev,
> void *data,
> if (!args->jc)
> return -EINVAL;
>
> - if (args->requirements && args->requirements != PANFROST_JD_REQ_FS)
> + if (args->requirements & ~JOB_REQUIREMENTS)
> return -EINVAL;
>
> if (args->out_sync > 0) {
> @@ -619,6 +621,8 @@ static const struct file_operations
> panfrost_drm_driver_fops = {
> * - 1.0 - initial interface
> * - 1.1 - adds HEAP and NOEXEC flags for CREATE_BO
> * - 1.2 - adds AFBC_FEATURES query
> + * - 1.3 - adds JD_REQ_CYCLE_COUNT job requirement for SUBMIT
> + * - adds SYSTEM_TIMESTAMP and SYSTEM_TIMESTAMP_FREQUENCY queries
> */
> static const struct drm_driver panfrost_drm_driver = {
> .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ,
> @@ -632,7 +636,7 @@ static const struct drm_driver panfrost_drm_driver = {
> .desc = "panfrost DRM",
> .date = "20180908",
> .major = 1,
> - .minor = 2,
> + .minor = 3,
>
> .gem_create_object = panfrost_gem_create_object,
> .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table,
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
> b/drivers/gpu/drm/panfrost/panfrost_job.c
> index df49d37d0e7e..e5e62ee356ef 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -159,16 +159,17 @@ panfrost_dequeue_job(struct panfrost_device *pfdev, int
> slot)
> struct panfrost_job *job = pfdev->jobs[slot][0];
>
> WARN_ON(!job);
> - if (job->is_profiled) {
> - if (job->engine_usage) {
> - job->engine_usage->elapsed_ns[slot] +=
> - ktime_to_ns(ktime_sub(ktime_get(),
> job->start_time));
> - job->engine_usage->cycles[slot] +=
> - panfrost_cycle_counter_read(pfdev) -
> job->start_cycles;
> - }
> - panfrost_cycle_counter_put(job->pfdev);
> +
> + if (job->is_profiled && job->engine_usage) {
> + job->engine_usage->elapsed_ns[slot] +=
> + ktime_to_ns(ktime_sub(ktime_get(), job->start_time));
> + job->engine_usage->cycles[slot] +=
> + panfrost_cycle_counter_read(pfdev) - job->start_cycles;
> }
>
> + if (job->requirements & PANFROST_JD_REQ_CYCLE_COUNT || job->is_profiled)
> + panfrost_cycle_counter_put(pfdev);
> +
> pfdev->jobs[slot][0] = pfdev->jobs[slot][1];
> pfdev->jobs[slot][1] = NULL;
>
> @@ -243,9 +244,13 @@ static void panfrost_job_hw_submit(struct panfrost_job
> *job, int js)
> subslot = panfrost_enqueue_job(pfdev, js, job);
> /* Don't queue the job if a reset is in progress */
> if (!atomic_read(&pfdev->reset.pending)) {
> - if (pfdev->profile_mode) {
> + job->is_profiled = pfdev->profile_mode;
> +
> + if (job->requirements & PANFROST_JD_REQ_CYCLE_COUNT ||
> + job->is_profiled)
> panfrost_cycle_counter_get(pfdev);
> - job->is_profiled = true;
> +
> + if (job->is_profiled) {
> job->start_time = ktime_get();
> job->start_cycles = panfrost_cycle_counter_read(pfdev);
> }
> @@ -693,7 +698,8 @@ panfrost_reset(struct panfrost_device *pfdev,
> spin_lock(&pfdev->js->job_lock);
> for (i = 0; i < NUM_JOB_SLOTS; i++) {
> for (j = 0; j < ARRAY_SIZE(pfdev->jobs[0]) &&
> pfdev->jobs[i][j]; j++) {
> - if (pfdev->jobs[i][j]->is_profiled)
> + if (pfdev->jobs[i][j]->requirements &
> PANFROST_JD_REQ_CYCLE_COUNT ||
> + pfdev->jobs[i][j]->is_profiled)
>
> panfrost_cycle_counter_put(pfdev->jobs[i][j]->pfdev);
> pm_runtime_put_noidle(pfdev->dev);
> panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
> diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> index 52b050e2b660..568724be6628 100644
> --- a/include/uapi/drm/panfrost_drm.h
> +++ b/include/uapi/drm/panfrost_drm.h
> @@ -40,6 +40,7 @@ extern "C" {
> #define DRM_IOCTL_PANFROST_PERFCNT_DUMP
> DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct
> drm_panfrost_perfcnt_dump)
>
> #define PANFROST_JD_REQ_FS (1 << 0)
> +#define PANFROST_JD_REQ_CYCLE_COUNT (1 << 1)
> /**
> * struct drm_panfrost_submit - ioctl argument for submitting commands to
> the 3D
> * engine.
