[PATCH v2 0/4] drm/msm/adreno: keep the GPU timestamp monotonic across power collapse

[PATCH v2 0/4] drm/msm/adreno: keep the GPU timestamp monotonic across power collapse

[Dmitry Baryshkov]

Before a750 the always-on counter behind MSM_PARAM_TIMESTAMP, which the
GPU also writes for its own timestamps, restarts whenever the GPU powers
up. This is the case on the GMU-less a5xx and a6xx parts, and also on
the GMU parts before a750. GL_TIMESTAMP, Vulkan calibrated timestamps
and clGetDeviceAndHostTimer() go backwards after every runtime suspend.

The counter can't be written back, so keep an offset that makes it
continuous, advancing it by the CLOCK_MONOTONIC time the counter was
stopped. Publish the offset in a new page that every context's command
streams can read, so that userspace can add it to the timestamps the
GPU writes, and report the counter with the offset already added as the
new MSM_PARAM_TIMESTAMP_MONOTONIC. The page isn't mapped to the CPU, so
it needs no sequence count. MSM_PARAM_TIMESTAMP stays raw. Where the
counter doesn't restart the offset stays zero, so userspace takes the
same path everywhere.

Tested on a512, a530, a540, a610, a630, a643, a650, a660, a702 and
a730: the corrected counter stays monotonic and tracks CLOCK_MONOTONIC
across runtime suspend, and the GPU can read the page but not write it.
With the Mesa changes GL and Vulkan timestamps stay monotonic, and the
OpenCL CTS device_timer test, which failed before, passes.

For the corresponding Mesa changes see [1].

[1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/45806

---
Changes in v2:
- Left MSM_PARAM_TIMESTAMP raw and let userspace add the offset, so
  that it keeps matching the timestamps the GPU writes (Connor, Rob).
- Added a GPU-readable page for kernel state, MSM_PARAM_VDSO_IOVA (Rob).
- Added MSM_PARAM_TIMESTAMP_MONOTONIC for the CPU side.
- Covered the a6xx GMU parts before a750, whose counter restarts too.
- Dropped a4xx, whose get_timestamp counts GPU cycles, not 19.2 MHz ticks.
- Split the change into four patches.
- Link to v1: https://patch.msgid.link/20260828-b4-adreno-timestamp-v1-1-273525da144c@oss.qualcomm.com

---
Dmitry Baryshkov (4):
      drm/msm: share a page of kernel state with the GPU
      drm/msm/adreno: add a timestamp offset for counters that restart
      drm/msm/a5xx: keep the timestamp offset
      drm/msm/a6xx: keep the timestamp offset before a750

 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 13 ++++++-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 21 ++++++++++-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 66 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 +++++
 drivers/gpu/drm/msm/msm_gpu.c           | 15 ++++++++
 drivers/gpu/drm/msm/msm_gpu.h           |  5 +++
 include/uapi/drm/msm_drm.h              | 25 +++++++++++++
 7 files changed, 153 insertions(+), 2 deletions(-)
---
base-commit: d024361c631b3f020a26c07d683e5103404b0fbf
change-id: 20260828-b4-adreno-timestamp-f4c846391b97

Best regards,
--  
With best wishes
Dmitry

[PATCH v2 1/4] drm/msm: share a page of kernel state with the GPU

[Dmitry Baryshkov]

Some state the kernel keeps has to be applied to values the GPU produces
by itself, the first case being the always-on counter that restarts
across a GPU power collapse. A GET_PARAM can't carry it, as command
streams need the value at the time they execute.

Allocate one page in the global GPU address space, read-only to the GPU
and not privileged, so the command streams of every context can read
it, and report its address. Start it with a flags word so that fields
appended later can be announced.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c |  3 +++
 drivers/gpu/drm/msm/msm_gpu.c           | 15 +++++++++++++++
 drivers/gpu/drm/msm/msm_gpu.h           |  5 +++++
 include/uapi/drm/msm_drm.h              |  9 +++++++++
 4 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index dadfe4c69438..0adf56fb9563 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -404,6 +404,9 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_context *ctx,
 			return 0;
 		}
 		return -EINVAL;
+	case MSM_PARAM_VDSO_IOVA:
+		*value = gpu->vdso_iova;
+		return 0;
 	case MSM_PARAM_PRIORITIES:
 		*value = gpu->nr_rings * NR_SCHED_PRIORITIES;
 		return 0;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 0c2c35636251..747cd90e329a 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -989,6 +989,19 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 	msm_gem_object_set_name(gpu->memptrs_bo, "memptrs");
 
+	gpu->vdso = msm_gem_kernel_new(drm, PAGE_SIZE,
+		MSM_BO_WC | MSM_BO_GPU_READONLY, gpu->vm, &gpu->vdso_bo,
+		&gpu->vdso_iova);
+
+	if (IS_ERR(gpu->vdso)) {
+		ret = PTR_ERR(gpu->vdso);
+		gpu->vdso = NULL;
+		DRM_DEV_ERROR(drm->dev, "could not allocate vdso: %d\n", ret);
+		goto fail;
+	}
+
+	msm_gem_object_set_name(gpu->vdso_bo, "vdso");
+
 	if (nr_rings > ARRAY_SIZE(gpu->rb)) {
 		DRM_DEV_INFO_ONCE(drm->dev, "Only creating %zu ringbuffers\n",
 			ARRAY_SIZE(gpu->rb));
@@ -1033,6 +1046,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		gpu->rb[i] = NULL;
 	}
 
+	msm_gem_kernel_put(gpu->vdso_bo, gpu->vm);
 	msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);
 
 	platform_set_drvdata(pdev, NULL);
@@ -1050,6 +1064,7 @@ void msm_gpu_cleanup(struct msm_gpu *gpu)
 		gpu->rb[i] = NULL;
 	}
 
+	msm_gem_kernel_put(gpu->vdso_bo, gpu->vm);
 	msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);
 
 	if (!IS_ERR_OR_NULL(gpu->vm)) {
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index d27d54bdb7a7..f78f3bc62977 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -252,6 +252,11 @@ struct msm_gpu {
 
 	struct drm_gem_object *memptrs_bo;
 
+	/* page of kernel state shared read-only with the GPU: */
+	struct drm_gem_object *vdso_bo;
+	struct drm_msm_vdso *vdso;
+	uint64_t vdso_iova;
+
 	struct msm_gpu_devfreq devfreq;
 
 	uint32_t suspend_count;
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index 3d363f12f66a..f8bf48fdc339 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -118,6 +118,7 @@ struct drm_msm_timespec {
  */
 #define MSM_PARAM_EN_VM_BIND 0x16  /* WO, once */
 #define MSM_PARAM_AQE	     0x17  /* RO */
+#define MSM_PARAM_VDSO_IOVA  0x18  /* RO: GPU address of struct drm_msm_vdso */
 
 /* For backwards compat.  The original support for preemption was based on
  * a single ring per priority level so # of priority levels equals the #
@@ -127,6 +128,14 @@ struct drm_msm_timespec {
  */
 #define MSM_PARAM_NR_RINGS   MSM_PARAM_PRIORITIES
 
+/* A page of kernel state that the GPU can read, at MSM_PARAM_VDSO_IOVA, for
+ * command streams to use.  New fields are only ever appended, and the kernel
+ * sets a bit in @flags for each one that needs announcing.
+ */
+struct drm_msm_vdso {
+	__u64 flags;
+};
+
 struct drm_msm_param {
 	__u32 pipe;           /* in, MSM_PIPE_x */
 	__u32 param;          /* in, MSM_PARAM_x */

-- 
2.47.3

[PATCH v2 2/4] drm/msm/adreno: add a timestamp offset for counters that restart

[Dmitry Baryshkov]

Before a750, the always-on counter behind MSM_PARAM_TIMESTAMP, which the
GPU also writes for its own timestamps, restarts whenever the GPU powers
up, so timestamps go backwards across a runtime suspend. The counter
can't be written back, and adding an offset to MSM_PARAM_TIMESTAMP alone
breaks its match with the timestamps the GPU writes.

Keep an offset that makes the counter continuous, advancing it by the
CLOCK_MONOTONIC time the counter was stopped, and publish it in the
vdso page so that shaders can add it to the timestamps the GPU writes.
Report the counter with the offset added as the new
MSM_PARAM_TIMESTAMP_MONOTONIC, read under gpu->lock so that recovery
can't move the offset in between. MSM_PARAM_TIMESTAMP stays raw for the
existing userspace. On GPUs where the counter doesn't restart the offset
stays zero, so userspace can take the same path everywhere.

Nothing calls the save and restore yet, so the offset stays zero.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 63 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++++
 include/uapi/drm/msm_drm.h              | 16 +++++++++
 3 files changed, 89 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 0adf56fb9563..f13e54e75dc1 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -226,6 +226,50 @@ adreno_iommu_create_vm(struct msm_gpu *gpu,
 	return vm;
 }
 
+/* The always-on counters tick at 19.2 MHz */
+static u64 adreno_ns_to_ticks(u64 ns)
+{
+	return mul_u64_u32_div(ns, 12, 625);
+}
+
+/* For a GPU whose always-on counter restarts when it powers up */
+void adreno_timestamp_init(struct msm_gpu *gpu)
+{
+	to_adreno_gpu(gpu)->timestamp_restarts = true;
+}
+
+/* Call with the GPU powered, before anything that may restart the counter */
+void adreno_timestamp_save(struct msm_gpu *gpu)
+{
+	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+
+	if (!adreno_gpu->timestamp_restarts || adreno_gpu->timestamp_stopped)
+		return;
+
+	adreno_gpu->timestamp_saved = adreno_gpu->funcs->get_timestamp(gpu);
+	adreno_gpu->timestamp_saved_ns = ktime_get_ns();
+	adreno_gpu->timestamp_stopped = true;
+}
+
+/* Call with the GPU powered, before it runs anything */
+void adreno_timestamp_restore(struct msm_gpu *gpu)
+{
+	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+	u64 now, gap;
+
+	if (!adreno_gpu->timestamp_stopped)
+		return;
+
+	now = adreno_gpu->funcs->get_timestamp(gpu);
+	gap = adreno_ns_to_ticks(ktime_get_ns() - adreno_gpu->timestamp_saved_ns);
+	adreno_gpu->timestamp_offset += adreno_gpu->timestamp_saved + gap - now;
+
+	gpu->vdso->timestamp_offset = adreno_gpu->timestamp_offset;
+	wmb();
+
+	adreno_gpu->timestamp_stopped = false;
+}
+
 u64 adreno_private_vm_size(struct msm_gpu *gpu)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -404,6 +448,25 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_context *ctx,
 			return 0;
 		}
 		return -EINVAL;
+	case MSM_PARAM_TIMESTAMP_MONOTONIC:
+		if (adreno_gpu->funcs->get_timestamp) {
+			int ret;
+
+			/* recovery moves the offset under gpu->lock */
+			ret = mutex_lock_interruptible(&gpu->lock);
+			if (ret)
+				return ret;
+
+			pm_runtime_get_sync(&gpu->pdev->dev);
+			*value = adreno_gpu->timestamp_offset +
+				 adreno_gpu->funcs->get_timestamp(gpu);
+			pm_runtime_put_autosuspend(&gpu->pdev->dev);
+
+			mutex_unlock(&gpu->lock);
+
+			return 0;
+		}
+		return -EINVAL;
 	case MSM_PARAM_VDSO_IOVA:
 		*value = gpu->vdso_iova;
 		return 0;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 4b2156825bdf..51ef57b536ed 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -209,6 +209,13 @@ struct adreno_gpu {
 	uint16_t speedbin;
 	const struct adreno_gpu_funcs *funcs;
 
+	/* offset that keeps a restarting always-on counter continuous: */
+	bool timestamp_restarts;
+	u64 timestamp_offset;
+	u64 timestamp_saved;
+	u64 timestamp_saved_ns;
+	bool timestamp_stopped;
+
 	struct completion fault_coredump_done;
 
 	/* interesting register offsets to dump: */
@@ -641,6 +648,9 @@ static inline int adreno_is_a850_family(struct adreno_gpu *gpu)
 /* Put vm_start above 32b to catch issues with not setting xyz_BASE_HI */
 #define ADRENO_VM_START 0x100000000ULL
 u64 adreno_private_vm_size(struct msm_gpu *gpu);
+void adreno_timestamp_init(struct msm_gpu *gpu);
+void adreno_timestamp_save(struct msm_gpu *gpu);
+void adreno_timestamp_restore(struct msm_gpu *gpu);
 int adreno_get_param(struct msm_gpu *gpu, struct msm_context *ctx,
 		     uint32_t param, uint64_t *value, uint32_t *len);
 int adreno_set_param(struct msm_gpu *gpu, struct msm_context *ctx,
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index f8bf48fdc339..2fd185f0ffcc 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -120,6 +120,16 @@ struct drm_msm_timespec {
 #define MSM_PARAM_AQE	     0x17  /* RO */
 #define MSM_PARAM_VDSO_IOVA  0x18  /* RO: GPU address of struct drm_msm_vdso */
 
+/* On GPUs before A750 the always-on counter behind MSM_PARAM_TIMESTAMP
+ * restarts whenever the GPU powers down. The GPU writes the same value as a
+ * timestamp.  MSM_PARAM_TIMESTAMP_MONOTONIC returns the counter with
+ * drm_msm_vdso::timestamp_offset added, resulting in a monotonic timestamp
+ * that most uAPIs require. Shaders can add the same to the timestamps
+ * the GPU writes when required, ending up with the stable counter values.
+ */
+
+#define MSM_PARAM_TIMESTAMP_MONOTONIC 0x19  /* RO */
+
 /* For backwards compat.  The original support for preemption was based on
  * a single ring per priority level so # of priority levels equals the #
  * of rings.  With drm/scheduler providing additional levels of priority,
@@ -134,6 +144,12 @@ struct drm_msm_timespec {
  */
 struct drm_msm_vdso {
 	__u64 flags;
+	/*
+	 * Value added to produce MSM_PARAM_TIMESTAMP_MONOTONIC.
+	 * The offset doesn't change while the GPU executes a submit, and stays
+	 * zero where the counter doesn't restart.
+	 */
+	__u64 timestamp_offset;
 };
 
 struct drm_msm_param {

-- 
2.47.3

[PATCH v2 3/4] drm/msm/a5xx: keep the timestamp offset

[Dmitry Baryshkov]

The always-on counter of a5xx is in the GPU power domain and restarts on
every runtime resume and on recovery.

Save the counter before the GPU powers down or is reset, and restore the
offset once it is back up, before it runs anything.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 16c7335c4b1f..fbb92cadf94f 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1025,6 +1025,8 @@ static void a5xx_recover(struct msm_gpu *gpu)
 	if (hang_debug)
 		a5xx_dump(gpu);
 
+	adreno_timestamp_save(gpu);
+
 	gpu_write(gpu, REG_A5XX_RBBM_SW_RESET_CMD, 1);
 	gpu_read(gpu, REG_A5XX_RBBM_SW_RESET_CMD);
 	gpu_write(gpu, REG_A5XX_RBBM_SW_RESET_CMD, 0);
@@ -1374,6 +1376,7 @@ static int a5xx_pm_resume(struct msm_gpu *gpu)
 		a5xx_set_hwcg(gpu, true);
 		/* Turn on sp_input_clk at HM level */
 		gpu_rmw(gpu, REG_A5XX_RBBM_CLOCK_CNTL, 0xff, 0);
+		adreno_timestamp_restore(gpu);
 		return 0;
 	}
 
@@ -1399,6 +1402,8 @@ static int a5xx_pm_resume(struct msm_gpu *gpu)
 	if (ret)
 		DRM_ERROR("%s: timeout waiting for SP GDSC enable\n",
 			gpu->name);
+	else
+		adreno_timestamp_restore(gpu);
 
 	return ret;
 }
@@ -1415,6 +1420,8 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
 	    adreno_is_a508(adreno_gpu) || adreno_is_a510(adreno_gpu))
 		mask = 0x7;
 
+	adreno_timestamp_save(gpu);
+
 	/* Clear the VBIF pipe before shutting down */
 	gpu_write(gpu, REG_A5XX_VBIF_XIN_HALT_CTRL0, mask);
 	spin_until((gpu_read(gpu, REG_A5XX_VBIF_XIN_HALT_CTRL1) &
@@ -1432,8 +1439,10 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
 	}
 
 	ret = msm_gpu_pm_suspend(gpu);
-	if (ret)
+	if (ret) {
+		adreno_timestamp_restore(gpu);
 		return ret;
+	}
 
 	if (a5xx_gpu->has_whereami)
 		for (i = 0; i < gpu->nr_rings; i++)
@@ -1824,6 +1833,8 @@ static struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
 		return ERR_PTR(ret);
 	}
 
+	adreno_timestamp_init(gpu);
+
 	/* Set the speedbin value that is passed to userspace */
 	if (adreno_read_speedbin(&pdev->dev, &speedbin) || !speedbin)
 		speedbin = 0xffff;

-- 
2.47.3

[PATCH v2 4/4] drm/msm/a6xx: keep the timestamp offset before a750

[Dmitry Baryshkov]

Before a750 the always-on counter does not survive a power collapse
either: on the GMU-less parts it is in the GPU power domain, and on a650
the GMU counter behind MSM_PARAM_TIMESTAMP restarts with every runtime
suspend, reading 65229 and 68216 ticks two seconds apart. From a750 on
the counter is loaded from one in the CX domain that keeps running, and
userspace already relies on that.

Save and restore the counter around power collapse and recovery on the
earlier parts, as on a5xx.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 92e8c8abb86d..7cbeeccca000 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1575,6 +1575,8 @@ static void a6xx_recover(struct msm_gpu *gpu)
 
 	adreno_dump_info(gpu);
 
+	adreno_timestamp_save(gpu);
+
 	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
 		/* Sometimes crashstate capture is skipped, so SQE should be halted here again */
 		gpu_write(gpu, REG_A6XX_CP_SQE_CNTL, 3);
@@ -1638,6 +1640,9 @@ static void a6xx_recover(struct msm_gpu *gpu)
 	gpu->active_submits = active_submits;
 	mutex_unlock(&gpu->active_lock);
 
+	/* the GPU may not have suspended, so resume did not restore */
+	adreno_timestamp_restore(gpu);
+
 	msm_gpu_hw_init(gpu);
 	a6xx_gpu->hung = false;
 }
@@ -2151,6 +2156,8 @@ static int a6xx_gmu_pm_resume(struct msm_gpu *gpu)
 	if (ret)
 		return ret;
 
+	adreno_timestamp_restore(gpu);
+
 	msm_devfreq_resume(gpu);
 
 	if (adreno_is_a8xx(adreno_gpu))
@@ -2212,6 +2219,8 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
 	msm_devfreq_resume(gpu);
 	a6xx_llc_activate(a6xx_gpu);
 
+	adreno_timestamp_restore(gpu);
+
 	return 0;
 
 	/* If anything goes south, tear the GPU down piece by piece.. */
@@ -2234,6 +2243,8 @@ static int a6xx_gmu_pm_suspend(struct msm_gpu *gpu)
 
 	trace_msm_gpu_suspend(0);
 
+	adreno_timestamp_save(gpu);
+
 	a6xx_llc_deactivate(a6xx_gpu);
 
 	msm_devfreq_suspend(gpu);
@@ -2241,8 +2252,10 @@ static int a6xx_gmu_pm_suspend(struct msm_gpu *gpu)
 	mutex_lock(&a6xx_gpu->gmu.lock);
 	ret = a6xx_gmu_stop(a6xx_gpu);
 	mutex_unlock(&a6xx_gpu->gmu.lock);
-	if (ret)
+	if (ret) {
+		adreno_timestamp_restore(gpu);
 		return ret;
+	}
 
 	if (a6xx_gpu->shadow_bo)
 		for (i = 0; i < gpu->nr_rings; i++)
@@ -2262,6 +2275,8 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
 
 	trace_msm_gpu_suspend(0);
 
+	adreno_timestamp_save(gpu);
+
 	a6xx_llc_deactivate(a6xx_gpu);
 
 	msm_devfreq_suspend(gpu);
@@ -2757,6 +2772,10 @@ static struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 		return ERR_PTR(ret);
 	}
 
+	/* From a750 on, the always-on counter survives a power collapse */
+	if (!adreno_is_a750_family(adreno_gpu) && !adreno_is_a8xx(adreno_gpu))
+		adreno_timestamp_init(gpu);
+
 	/* Set the speedbin value that is passed to userspace */
 	if (a6xx_read_speedbin(&pdev->dev, a6xx_gpu, info, &speedbin) || !speedbin)
 		speedbin = 0xffff;

-- 
2.47.3

Re: [PATCH v2 1/4] drm/msm: share a page of kernel state with the GPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm: double msm_gem_kernel_put() on error path in msm_gpu_init()
--

commit 2d367cff8e8b826f570b2fdbbb2520e0cf351371
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/msm: share a page of kernel state with the GPU

This patch allocates a global read-only GPU page (vdso) to share kernel
state with GPU command streams and exposes its IOVA via
MSM_PARAM_VDSO_IOVA.

> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 0c2c356362518..747cd90e329a2 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -989,6 +989,19 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>  
>  	msm_gem_object_set_name(gpu->memptrs_bo, "memptrs");
>  
> +	gpu->vdso = msm_gem_kernel_new(drm, PAGE_SIZE,
> +		MSM_BO_WC | MSM_BO_GPU_READONLY, gpu->vm, &gpu->vdso_bo,
> +		&gpu->vdso_iova);
> +
> +	if (IS_ERR(gpu->vdso)) {
> +		ret = PTR_ERR(gpu->vdso);
> +		gpu->vdso = NULL;
> +		DRM_DEV_ERROR(drm->dev, "could not allocate vdso: %d\n", ret);
> +		goto fail;
> +	}
> +
> +	msm_gem_object_set_name(gpu->vdso_bo, "vdso");
> +
>  	if (nr_rings > ARRAY_SIZE(gpu->rb)) {
>  		DRM_DEV_INFO_ONCE(drm->dev, "Only creating %zu ringbuffers\n",
>  			ARRAY_SIZE(gpu->rb));
> @@ -1033,6 +1046,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>  		gpu->rb[i] = NULL;
>  	}
>  
> +	msm_gem_kernel_put(gpu->vdso_bo, gpu->vm);
>  	msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);
>  
>  	platform_set_drvdata(pdev, NULL);
> @@ -1050,6 +1064,7 @@ void msm_gpu_cleanup(struct msm_gpu *gpu)
>  		gpu->rb[i] = NULL;
>  	}
>  
> +	msm_gem_kernel_put(gpu->vdso_bo, gpu->vm);
>  	msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);

[Severity: High]
Can this cause a double put and use-after-free on gpu->vdso_bo and
gpu->memptrs_bo if msm_gpu_init() fails after allocating them?

In msm_gpu_init(), if allocating gpu->vdso fails (after gpu->memptrs_bo was
allocated) or if a later step such as msm_ringbuffer_new() or
msm_perfcntr_init() fails, the fail path calls msm_gem_kernel_put() on
gpu->vdso_bo and gpu->memptrs_bo without clearing those pointers to NULL
(unlike gpu->rb[i], which is cleared to NULL in the loop right above).

When msm_gpu_init() returns an error to adreno_gpu_init(), the callers
(a2xx_gpu_init() through a6xx_gpu_init()) clean up by invoking their
destroy callback, for example:

a6xx_gpu_init()
  adreno_gpu_init() -> msm_gpu_init() (fails, puts vdso_bo and memptrs_bo)
  a6xx_destroy()
    adreno_gpu_cleanup()
      msm_gpu_cleanup()
        msm_gem_kernel_put(gpu->vdso_bo, gpu->vm);
        msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);

Because gpu->vdso_bo and gpu->memptrs_bo are still non-NULL,
msm_gpu_cleanup() passes the already-freed GEM objects to
msm_gem_kernel_put() a second time. Should the fail path in msm_gpu_init()
clear gpu->vdso_bo and gpu->memptrs_bo to NULL after calling
msm_gem_kernel_put(), or leave their release to msm_gpu_cleanup()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031-b4-adreno-timestamp-v2-0-0836856b228f@oss.qualcomm.com?part=1