All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/msm: Fixes/updates for perfetto profiling
@ 2021-03-25  1:23 ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2021-03-25  1:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, Akhil P Oommen, Bjorn Andersson, Dave Airlie,
	Emil Velikov, Eric Anholt,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Gustavo A. R. Silva,
	Jonathan Marek, Jordan Crouse, Konrad Dybcio,
	Kristian H. Kristensen, open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list, Marijn Suijten, Sai Prakash Ranjan, Sam Ravnborg,
	Sharat Masetty

From: Rob Clark <robdclark@chromium.org>

A couple kernel side things I realized I needed in the process of
implementing performance-counter and render-stage support for perfetto,
the first patch fixes the MSM_PARAM_TIMESTAMP query which was just
wrong on a5xx/a6xx (ALWAYS_COUNT vs ALWAYS_ON).  The second adds a
way for userspace to determine whether the device has suspended since
the last sampling period (which means counters have lost their state
and configuration).

I am a bit tempted to add a way that a CAP_SYS_ADMIN user could ask
the GPU to not suspend (until the drm_file is closed), but so far
I've managed to avoid needing this.

Rob Clark (2):
  drm/msm: Fix a5xx/a6xx timestamps
  drm/msm: Add param for userspace to query suspend count

 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 4 ++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 4 ++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
 drivers/gpu/drm/msm/msm_drv.c           | 1 +
 drivers/gpu/drm/msm/msm_gpu.c           | 2 ++
 drivers/gpu/drm/msm/msm_gpu.h           | 2 ++
 include/uapi/drm/msm_drm.h              | 1 +
 7 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 0/2] drm/msm: Fixes/updates for perfetto profiling
@ 2021-03-25  1:23 ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2021-03-25  1:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, Sai Prakash Ranjan, Gustavo A. R. Silva,
	Jonathan Marek, open list:DRM DRIVER FOR MSM ADRENO GPU,
	Sharat Masetty, Konrad Dybcio, Akhil P Oommen, Bjorn Andersson,
	Sam Ravnborg, Jordan Crouse, Kristian H. Kristensen,
	Marijn Suijten, Dave Airlie,
	open list:DRM DRIVER FOR MSM ADRENO GPU, open list, Emil Velikov

From: Rob Clark <robdclark@chromium.org>

A couple kernel side things I realized I needed in the process of
implementing performance-counter and render-stage support for perfetto,
the first patch fixes the MSM_PARAM_TIMESTAMP query which was just
wrong on a5xx/a6xx (ALWAYS_COUNT vs ALWAYS_ON).  The second adds a
way for userspace to determine whether the device has suspended since
the last sampling period (which means counters have lost their state
and configuration).

I am a bit tempted to add a way that a CAP_SYS_ADMIN user could ask
the GPU to not suspend (until the drm_file is closed), but so far
I've managed to avoid needing this.

Rob Clark (2):
  drm/msm: Fix a5xx/a6xx timestamps
  drm/msm: Add param for userspace to query suspend count

 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 4 ++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 4 ++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
 drivers/gpu/drm/msm/msm_drv.c           | 1 +
 drivers/gpu/drm/msm/msm_gpu.c           | 2 ++
 drivers/gpu/drm/msm/msm_gpu.h           | 2 ++
 include/uapi/drm/msm_drm.h              | 1 +
 7 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps
  2021-03-25  1:23 ` Rob Clark
@ 2021-03-25  1:23   ` Rob Clark
  -1 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2021-03-25  1:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, Rob Clark, Sean Paul, David Airlie, Daniel Vetter,
	Jordan Crouse, Kristian H. Kristensen, Bjorn Andersson,
	Sam Ravnborg, Emil Velikov, Gustavo A. R. Silva, Marijn Suijten,
	Jonathan Marek, Eric Anholt, Sai Prakash Ranjan, Sharat Masetty,
	open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, open list

From: Rob Clark <robdclark@chromium.org>

They were reading a counter that was configured to ALWAYS_COUNT (ie.
cycles that the GPU is doing something) rather than ALWAYS_ON.  This
isn't the thing that userspace is looking for.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index a5af223eaf50..bb82fcd9df81 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1241,8 +1241,8 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
 
 static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 {
-	*value = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_CP_0_LO,
-		REG_A5XX_RBBM_PERFCTR_CP_0_HI);
+	*value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO,
+		REG_A5XX_RBBM_ALWAYSON_COUNTER_HI);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 130661898546..59718c304488 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1173,8 +1173,8 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 	/* Force the GPU power on so we can read this register */
 	a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
 
-	*value = gpu_read64(gpu, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
-		REG_A6XX_RBBM_PERFCTR_CP_0_HI);
+	*value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER_LO,
+		REG_A6XX_CP_ALWAYS_ON_COUNTER_HI);
 
 	a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
 	return 0;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps
@ 2021-03-25  1:23   ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2021-03-25  1:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, open list:DRM DRIVER FOR MSM ADRENO GPU,
	Sai Prakash Ranjan, Gustavo A. R. Silva, Jonathan Marek,
	David Airlie, Sam Ravnborg, Sharat Masetty, Jordan Crouse,
	Bjorn Andersson, Kristian H. Kristensen,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Marijn Suijten,
	Sean Paul, open list, Emil Velikov

From: Rob Clark <robdclark@chromium.org>

They were reading a counter that was configured to ALWAYS_COUNT (ie.
cycles that the GPU is doing something) rather than ALWAYS_ON.  This
isn't the thing that userspace is looking for.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index a5af223eaf50..bb82fcd9df81 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1241,8 +1241,8 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
 
 static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 {
-	*value = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_CP_0_LO,
-		REG_A5XX_RBBM_PERFCTR_CP_0_HI);
+	*value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO,
+		REG_A5XX_RBBM_ALWAYSON_COUNTER_HI);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 130661898546..59718c304488 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1173,8 +1173,8 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 	/* Force the GPU power on so we can read this register */
 	a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
 
-	*value = gpu_read64(gpu, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
-		REG_A6XX_RBBM_PERFCTR_CP_0_HI);
+	*value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER_LO,
+		REG_A6XX_CP_ALWAYS_ON_COUNTER_HI);
 
 	a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
 	return 0;
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/msm: Add param for userspace to query suspend count
  2021-03-25  1:23 ` Rob Clark
@ 2021-03-25  1:23   ` Rob Clark
  -1 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2021-03-25  1:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, Rob Clark, Sean Paul, David Airlie, Daniel Vetter,
	Jordan Crouse, Jonathan Marek, Dave Airlie, Bjorn Andersson,
	Sharat Masetty, Konrad Dybcio, Akhil P Oommen,
	open list:DRM DRIVER FOR MSM ADRENO GPU,
	open list:DRM DRIVER FOR MSM ADRENO GPU, open list

From: Rob Clark <robdclark@chromium.org>

Performance counts, and ALWAYS_ON counters used for capturing GPU
timestamps, lose their state across suspend/resume cycles.  Userspace
tooling for performance monitoring needs to be aware of this.  For
example, after a suspend userspace needs to recalibrate it's offset
between CPU and GPU time.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
 drivers/gpu/drm/msm/msm_drv.c           | 1 +
 drivers/gpu/drm/msm/msm_gpu.c           | 2 ++
 drivers/gpu/drm/msm/msm_gpu.h           | 2 ++
 include/uapi/drm/msm_drm.h              | 1 +
 5 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index f09175698827..e473b7c9ff7f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -280,6 +280,9 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
 	case MSM_PARAM_FAULTS:
 		*value = gpu->global_faults;
 		return 0;
+	case MSM_PARAM_SUSPENDS:
+		*value = gpu->suspend_count;
+		return 0;
 	default:
 		DBG("%s: invalid param: %u", gpu->name, param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b29e439eb299..4f9fa0189a07 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -39,6 +39,7 @@
  *           GEM object's debug name
  * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
  * - 1.6.0 - Syncobj support
+ * - 1.7.0 - Add MSM_PARAM_SUSPENDS to access suspend count
  */
 #define MSM_VERSION_MAJOR	1
 #define MSM_VERSION_MINOR	6
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 7bdb01f202f4..ab888d83b887 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -256,6 +256,8 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
 	if (ret)
 		return ret;
 
+	gpu->suspend_count++;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index d7cd02cd2109..18baf935e143 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -152,6 +152,8 @@ struct msm_gpu {
 		ktime_t time;
 	} devfreq;
 
+	uint32_t suspend_count;
+
 	struct msm_gpu_state *crashstate;
 	/* True if the hardware supports expanded apriv (a650 and newer) */
 	bool hw_apriv;
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index a6c1f3eb2623..5596d7c37f9e 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -76,6 +76,7 @@ struct drm_msm_timespec {
 #define MSM_PARAM_NR_RINGS   0x07
 #define MSM_PARAM_PP_PGTABLE 0x08  /* => 1 for per-process pagetables, else 0 */
 #define MSM_PARAM_FAULTS     0x09
+#define MSM_PARAM_SUSPENDS   0x0a
 
 struct drm_msm_param {
 	__u32 pipe;           /* in, MSM_PIPE_x */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/msm: Add param for userspace to query suspend count
@ 2021-03-25  1:23   ` Rob Clark
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Clark @ 2021-03-25  1:23 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Clark, open list:DRM DRIVER FOR MSM ADRENO GPU,
	Jonathan Marek, David Airlie,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Sharat Masetty,
	Konrad Dybcio, Jordan Crouse, Bjorn Andersson, Akhil P Oommen,
	Dave Airlie, Sean Paul, open list

From: Rob Clark <robdclark@chromium.org>

Performance counts, and ALWAYS_ON counters used for capturing GPU
timestamps, lose their state across suspend/resume cycles.  Userspace
tooling for performance monitoring needs to be aware of this.  For
example, after a suspend userspace needs to recalibrate it's offset
between CPU and GPU time.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
 drivers/gpu/drm/msm/msm_drv.c           | 1 +
 drivers/gpu/drm/msm/msm_gpu.c           | 2 ++
 drivers/gpu/drm/msm/msm_gpu.h           | 2 ++
 include/uapi/drm/msm_drm.h              | 1 +
 5 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index f09175698827..e473b7c9ff7f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -280,6 +280,9 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
 	case MSM_PARAM_FAULTS:
 		*value = gpu->global_faults;
 		return 0;
+	case MSM_PARAM_SUSPENDS:
+		*value = gpu->suspend_count;
+		return 0;
 	default:
 		DBG("%s: invalid param: %u", gpu->name, param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b29e439eb299..4f9fa0189a07 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -39,6 +39,7 @@
  *           GEM object's debug name
  * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
  * - 1.6.0 - Syncobj support
+ * - 1.7.0 - Add MSM_PARAM_SUSPENDS to access suspend count
  */
 #define MSM_VERSION_MAJOR	1
 #define MSM_VERSION_MINOR	6
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 7bdb01f202f4..ab888d83b887 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -256,6 +256,8 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
 	if (ret)
 		return ret;
 
+	gpu->suspend_count++;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index d7cd02cd2109..18baf935e143 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -152,6 +152,8 @@ struct msm_gpu {
 		ktime_t time;
 	} devfreq;
 
+	uint32_t suspend_count;
+
 	struct msm_gpu_state *crashstate;
 	/* True if the hardware supports expanded apriv (a650 and newer) */
 	bool hw_apriv;
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index a6c1f3eb2623..5596d7c37f9e 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -76,6 +76,7 @@ struct drm_msm_timespec {
 #define MSM_PARAM_NR_RINGS   0x07
 #define MSM_PARAM_PP_PGTABLE 0x08  /* => 1 for per-process pagetables, else 0 */
 #define MSM_PARAM_FAULTS     0x09
+#define MSM_PARAM_SUSPENDS   0x0a
 
 struct drm_msm_param {
 	__u32 pipe;           /* in, MSM_PIPE_x */
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [Freedreno] [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps
  2021-03-25  1:23   ` Rob Clark
@ 2021-04-02 14:50     ` Jordan Crouse
  -1 siblings, 0 replies; 10+ messages in thread
From: Jordan Crouse @ 2021-04-02 14:50 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Rob Clark, open list:DRM DRIVER FOR MSM ADRENO GPU,
	Sai Prakash Ranjan, Gustavo A. R. Silva, Jonathan Marek,
	David Airlie, Sam Ravnborg, Sharat Masetty, Jordan Crouse,
	Bjorn Andersson, Eric Anholt, Kristian H. Kristensen,
	Daniel Vetter, open list:DRM DRIVER FOR MSM ADRENO GPU,
	Marijn Suijten, Sean Paul, open list, Emil Velikov

On Wed, Mar 24, 2021 at 06:23:52PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> They were reading a counter that was configured to ALWAYS_COUNT (ie.
> cycles that the GPU is doing something) rather than ALWAYS_ON.  This
> isn't the thing that userspace is looking for.

Acked-by: Jordan Crouse <jordan@cosmicpenguin.net>

> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index a5af223eaf50..bb82fcd9df81 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1241,8 +1241,8 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
>  
>  static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>  {
> -	*value = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_CP_0_LO,
> -		REG_A5XX_RBBM_PERFCTR_CP_0_HI);
> +	*value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO,
> +		REG_A5XX_RBBM_ALWAYSON_COUNTER_HI);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 130661898546..59718c304488 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1173,8 +1173,8 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>  	/* Force the GPU power on so we can read this register */
>  	a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
>  
> -	*value = gpu_read64(gpu, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
> -		REG_A6XX_RBBM_PERFCTR_CP_0_HI);
> +	*value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER_LO,
> +		REG_A6XX_CP_ALWAYS_ON_COUNTER_HI);
>  
>  	a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
>  	return 0;
> -- 
> 2.29.2
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Freedreno] [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps
@ 2021-04-02 14:50     ` Jordan Crouse
  0 siblings, 0 replies; 10+ messages in thread
From: Jordan Crouse @ 2021-04-02 14:50 UTC (permalink / raw)
  To: Rob Clark
  Cc: Rob Clark, Sai Prakash Ranjan, Jonathan Marek, David Airlie,
	Sam Ravnborg, Sharat Masetty, Gustavo A. R. Silva, dri-devel,
	Bjorn Andersson, Jordan Crouse, Kristian H. Kristensen,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Marijn Suijten,
	Sean Paul, open list:DRM DRIVER FOR MSM ADRENO GPU, open list,
	Emil Velikov

On Wed, Mar 24, 2021 at 06:23:52PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> They were reading a counter that was configured to ALWAYS_COUNT (ie.
> cycles that the GPU is doing something) rather than ALWAYS_ON.  This
> isn't the thing that userspace is looking for.

Acked-by: Jordan Crouse <jordan@cosmicpenguin.net>

> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index a5af223eaf50..bb82fcd9df81 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1241,8 +1241,8 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
>  
>  static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>  {
> -	*value = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_CP_0_LO,
> -		REG_A5XX_RBBM_PERFCTR_CP_0_HI);
> +	*value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO,
> +		REG_A5XX_RBBM_ALWAYSON_COUNTER_HI);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 130661898546..59718c304488 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1173,8 +1173,8 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>  	/* Force the GPU power on so we can read this register */
>  	a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
>  
> -	*value = gpu_read64(gpu, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
> -		REG_A6XX_RBBM_PERFCTR_CP_0_HI);
> +	*value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER_LO,
> +		REG_A6XX_CP_ALWAYS_ON_COUNTER_HI);
>  
>  	a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
>  	return 0;
> -- 
> 2.29.2
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Freedreno] [PATCH 2/2] drm/msm: Add param for userspace to query suspend count
  2021-03-25  1:23   ` Rob Clark
@ 2021-04-02 14:50     ` Jordan Crouse
  -1 siblings, 0 replies; 10+ messages in thread
From: Jordan Crouse @ 2021-04-02 14:50 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Rob Clark, open list:DRM DRIVER FOR MSM ADRENO GPU,
	Jonathan Marek, David Airlie,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Sharat Masetty,
	Konrad Dybcio, Jordan Crouse, Bjorn Andersson, Akhil P Oommen,
	Daniel Vetter, Dave Airlie, Sean Paul, open list

On Wed, Mar 24, 2021 at 06:23:53PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Performance counts, and ALWAYS_ON counters used for capturing GPU
> timestamps, lose their state across suspend/resume cycles.  Userspace
> tooling for performance monitoring needs to be aware of this.  For
> example, after a suspend userspace needs to recalibrate it's offset
> between CPU and GPU time.
> 

Acked-by: Jordan Crouse <jordan@cosmicpenguin.net>

> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
>  drivers/gpu/drm/msm/msm_drv.c           | 1 +
>  drivers/gpu/drm/msm/msm_gpu.c           | 2 ++
>  drivers/gpu/drm/msm/msm_gpu.h           | 2 ++
>  include/uapi/drm/msm_drm.h              | 1 +
>  5 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index f09175698827..e473b7c9ff7f 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -280,6 +280,9 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
>  	case MSM_PARAM_FAULTS:
>  		*value = gpu->global_faults;
>  		return 0;
> +	case MSM_PARAM_SUSPENDS:
> +		*value = gpu->suspend_count;
> +		return 0;
>  	default:
>  		DBG("%s: invalid param: %u", gpu->name, param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index b29e439eb299..4f9fa0189a07 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -39,6 +39,7 @@
>   *           GEM object's debug name
>   * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
>   * - 1.6.0 - Syncobj support
> + * - 1.7.0 - Add MSM_PARAM_SUSPENDS to access suspend count
>   */
>  #define MSM_VERSION_MAJOR	1
>  #define MSM_VERSION_MINOR	6
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 7bdb01f202f4..ab888d83b887 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -256,6 +256,8 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
>  	if (ret)
>  		return ret;
>  
> +	gpu->suspend_count++;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index d7cd02cd2109..18baf935e143 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -152,6 +152,8 @@ struct msm_gpu {
>  		ktime_t time;
>  	} devfreq;
>  
> +	uint32_t suspend_count;
> +
>  	struct msm_gpu_state *crashstate;
>  	/* True if the hardware supports expanded apriv (a650 and newer) */
>  	bool hw_apriv;
> diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
> index a6c1f3eb2623..5596d7c37f9e 100644
> --- a/include/uapi/drm/msm_drm.h
> +++ b/include/uapi/drm/msm_drm.h
> @@ -76,6 +76,7 @@ struct drm_msm_timespec {
>  #define MSM_PARAM_NR_RINGS   0x07
>  #define MSM_PARAM_PP_PGTABLE 0x08  /* => 1 for per-process pagetables, else 0 */
>  #define MSM_PARAM_FAULTS     0x09
> +#define MSM_PARAM_SUSPENDS   0x0a
>  
>  struct drm_msm_param {
>  	__u32 pipe;           /* in, MSM_PIPE_x */
> -- 
> 2.29.2
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Freedreno] [PATCH 2/2] drm/msm: Add param for userspace to query suspend count
@ 2021-04-02 14:50     ` Jordan Crouse
  0 siblings, 0 replies; 10+ messages in thread
From: Jordan Crouse @ 2021-04-02 14:50 UTC (permalink / raw)
  To: Rob Clark
  Cc: Rob Clark, Jonathan Marek, David Airlie,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Sharat Masetty,
	Konrad Dybcio, Akhil P Oommen, dri-devel, Bjorn Andersson,
	Sean Paul, Jordan Crouse, Dave Airlie,
	open list:DRM DRIVER FOR MSM ADRENO GPU, open list

On Wed, Mar 24, 2021 at 06:23:53PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Performance counts, and ALWAYS_ON counters used for capturing GPU
> timestamps, lose their state across suspend/resume cycles.  Userspace
> tooling for performance monitoring needs to be aware of this.  For
> example, after a suspend userspace needs to recalibrate it's offset
> between CPU and GPU time.
> 

Acked-by: Jordan Crouse <jordan@cosmicpenguin.net>

> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
>  drivers/gpu/drm/msm/msm_drv.c           | 1 +
>  drivers/gpu/drm/msm/msm_gpu.c           | 2 ++
>  drivers/gpu/drm/msm/msm_gpu.h           | 2 ++
>  include/uapi/drm/msm_drm.h              | 1 +
>  5 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index f09175698827..e473b7c9ff7f 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -280,6 +280,9 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
>  	case MSM_PARAM_FAULTS:
>  		*value = gpu->global_faults;
>  		return 0;
> +	case MSM_PARAM_SUSPENDS:
> +		*value = gpu->suspend_count;
> +		return 0;
>  	default:
>  		DBG("%s: invalid param: %u", gpu->name, param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index b29e439eb299..4f9fa0189a07 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -39,6 +39,7 @@
>   *           GEM object's debug name
>   * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
>   * - 1.6.0 - Syncobj support
> + * - 1.7.0 - Add MSM_PARAM_SUSPENDS to access suspend count
>   */
>  #define MSM_VERSION_MAJOR	1
>  #define MSM_VERSION_MINOR	6
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 7bdb01f202f4..ab888d83b887 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -256,6 +256,8 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
>  	if (ret)
>  		return ret;
>  
> +	gpu->suspend_count++;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index d7cd02cd2109..18baf935e143 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -152,6 +152,8 @@ struct msm_gpu {
>  		ktime_t time;
>  	} devfreq;
>  
> +	uint32_t suspend_count;
> +
>  	struct msm_gpu_state *crashstate;
>  	/* True if the hardware supports expanded apriv (a650 and newer) */
>  	bool hw_apriv;
> diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
> index a6c1f3eb2623..5596d7c37f9e 100644
> --- a/include/uapi/drm/msm_drm.h
> +++ b/include/uapi/drm/msm_drm.h
> @@ -76,6 +76,7 @@ struct drm_msm_timespec {
>  #define MSM_PARAM_NR_RINGS   0x07
>  #define MSM_PARAM_PP_PGTABLE 0x08  /* => 1 for per-process pagetables, else 0 */
>  #define MSM_PARAM_FAULTS     0x09
> +#define MSM_PARAM_SUSPENDS   0x0a
>  
>  struct drm_msm_param {
>  	__u32 pipe;           /* in, MSM_PIPE_x */
> -- 
> 2.29.2
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-02 14:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25  1:23 [PATCH 0/2] drm/msm: Fixes/updates for perfetto profiling Rob Clark
2021-03-25  1:23 ` Rob Clark
2021-03-25  1:23 ` [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps Rob Clark
2021-03-25  1:23   ` Rob Clark
2021-04-02 14:50   ` [Freedreno] " Jordan Crouse
2021-04-02 14:50     ` Jordan Crouse
2021-03-25  1:23 ` [PATCH 2/2] drm/msm: Add param for userspace to query suspend count Rob Clark
2021-03-25  1:23   ` Rob Clark
2021-04-02 14:50   ` [Freedreno] " Jordan Crouse
2021-04-02 14:50     ` Jordan Crouse

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.