* [PATCH 0/2] drm/msm: Fixes/updates for perfetto profiling @ 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 ` [PATCH 2/2] drm/msm: Add param for userspace to query suspend count Rob Clark 0 siblings, 2 replies; 5+ 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] 5+ messages in thread
* [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps 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-04-02 14:50 ` [Freedreno] " Jordan Crouse 2021-03-25 1:23 ` [PATCH 2/2] drm/msm: Add param for userspace to query suspend count Rob Clark 1 sibling, 1 reply; 5+ 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] 5+ messages in thread
* Re: [Freedreno] [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps 2021-03-25 1:23 ` [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps Rob Clark @ 2021-04-02 14:50 ` Jordan Crouse 0 siblings, 0 replies; 5+ 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] 5+ messages in thread
* [PATCH 2/2] drm/msm: Add param for userspace to query suspend count 2021-03-25 1:23 [PATCH 0/2] drm/msm: Fixes/updates for perfetto profiling 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 1 sibling, 1 reply; 5+ 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] 5+ messages in thread
* Re: [Freedreno] [PATCH 2/2] drm/msm: Add param for userspace to query suspend count 2021-03-25 1:23 ` [PATCH 2/2] drm/msm: Add param for userspace to query suspend count Rob Clark @ 2021-04-02 14:50 ` Jordan Crouse 0 siblings, 0 replies; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2021-04-02 14:50 UTC | newest] Thread overview: 5+ 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 ` [PATCH 1/2] drm/msm: Fix a5xx/a6xx timestamps Rob Clark 2021-04-02 14:50 ` [Freedreno] " Jordan Crouse 2021-03-25 1:23 ` [PATCH 2/2] drm/msm: Add param for userspace to query suspend count Rob Clark 2021-04-02 14:50 ` [Freedreno] " Jordan Crouse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).