1. Patchew
  2. linux
  3. [PATCH 00/16] drm/msm: A8xx Support - Batch 2
  4. View patch

[PATCH 02/16] drm/msm/a6xx: Switch to preemption safe AO counter

Akhil P Oommen posted 16 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH 02/16] drm/msm/a6xx: Switch to preemption safe AO counter
Posted by Akhil P Oommen 1 week, 5 days ago 
CP_ALWAYS_ON_COUNTER is not save-restored during preemption, so it won't
provide accurate data about the 'submit' when preemption is enabled.
Switch to CP_ALWAYS_ON_CONTEXT which is preemption safe.

Fixes: e7ae83da4a28 ("drm/msm/a6xx: Implement preemption for a7xx targets")
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 14d5b5e266f7..93bf2c40bfb9 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -345,7 +345,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
 	 * GPU registers so we need to add 0x1a800 to the register value on A630
 	 * to get the right value from PM4.
 	 */
-	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
+	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
 		rbmemptr_stats(ring, index, alwayson_start));
 
 	/* Invalidate CCU depth and color */
@@ -386,7 +386,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
 
 	get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP(0),
 		rbmemptr_stats(ring, index, cpcycles_end));
-	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
+	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
 		rbmemptr_stats(ring, index, alwayson_end));
 
 	/* Write the fence to the scratch register */
@@ -478,10 +478,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
 
 	if (adreno_is_a8xx(adreno_gpu)) {
 		rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
-		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
+		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT;
 	} else {
 		rbbm_perfctr_cp0 = REG_A7XX_RBBM_PERFCTR_CP(0);
-		cp_always_on_counter = REG_A6XX_CP_ALWAYS_ON_COUNTER;
+		cp_always_on_counter = REG_A6XX_CP_ALWAYS_ON_CONTEXT;
 	}
 
 	get_stats_counter(ring, rbbm_perfctr_cp0, rbmemptr_stats(ring, index, cpcycles_start));

-- 
2.51.0
Re: [PATCH 02/16] drm/msm/a6xx: Switch to preemption safe AO counter
Posted by Konrad Dybcio 1 week, 5 days ago 
On 3/23/26 9:12 PM, Akhil P Oommen wrote:
> CP_ALWAYS_ON_COUNTER is not save-restored during preemption, so it won't
> provide accurate data about the 'submit' when preemption is enabled.
> Switch to CP_ALWAYS_ON_CONTEXT which is preemption safe.
> 
> Fixes: e7ae83da4a28 ("drm/msm/a6xx: Implement preemption for a7xx targets")
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 14d5b5e266f7..93bf2c40bfb9 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -345,7 +345,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>  	 * GPU registers so we need to add 0x1a800 to the register value on A630
>  	 * to get the right value from PM4.
>  	 */
> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>  		rbmemptr_stats(ring, index, alwayson_start));
>  
>  	/* Invalidate CCU depth and color */
> @@ -386,7 +386,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>  
>  	get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP(0),
>  		rbmemptr_stats(ring, index, cpcycles_end));
> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>  		rbmemptr_stats(ring, index, alwayson_end));
>  
>  	/* Write the fence to the scratch register */
> @@ -478,10 +478,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>  
>  	if (adreno_is_a8xx(adreno_gpu)) {
>  		rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
> -		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
> +		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT;

I'm a little worried about mixing the names here - KGSL uses both of
these registers (A6XX_KERNEL_PROFILE vs A6XX_KERNEL_PROFILE_CONTEXT)
to track different fields of the struct adreno_drawobj_profile_entry

Konrad
Re: [PATCH 02/16] drm/msm/a6xx: Switch to preemption safe AO counter
Posted by Akhil P Oommen 1 week, 3 days ago 
On 3/24/2026 3:21 PM, Konrad Dybcio wrote:
> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>> CP_ALWAYS_ON_COUNTER is not save-restored during preemption, so it won't
>> provide accurate data about the 'submit' when preemption is enabled.
>> Switch to CP_ALWAYS_ON_CONTEXT which is preemption safe.
>>
>> Fixes: e7ae83da4a28 ("drm/msm/a6xx: Implement preemption for a7xx targets")
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> index 14d5b5e266f7..93bf2c40bfb9 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> @@ -345,7 +345,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>  	 * GPU registers so we need to add 0x1a800 to the register value on A630
>>  	 * to get the right value from PM4.
>>  	 */
>> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
>> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>>  		rbmemptr_stats(ring, index, alwayson_start));
>>  
>>  	/* Invalidate CCU depth and color */
>> @@ -386,7 +386,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>  
>>  	get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP(0),
>>  		rbmemptr_stats(ring, index, cpcycles_end));
>> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
>> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>>  		rbmemptr_stats(ring, index, alwayson_end));
>>  
>>  	/* Write the fence to the scratch register */
>> @@ -478,10 +478,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>  
>>  	if (adreno_is_a8xx(adreno_gpu)) {
>>  		rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
>> -		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
>> +		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT;
> 
> I'm a little worried about mixing the names here - KGSL uses both of
> these registers (A6XX_KERNEL_PROFILE vs A6XX_KERNEL_PROFILE_CONTEXT)
> to track different fields of the struct adreno_drawobj_profile_entry

But this naming aligns with the HW reg spec. So I prefer to use the same.

-Akhil

> 
> Konrad
Re: [PATCH 02/16] drm/msm/a6xx: Switch to preemption safe AO counter
Posted by Konrad Dybcio 1 week, 3 days ago 
On 3/25/26 10:46 PM, Akhil P Oommen wrote:
> On 3/24/2026 3:21 PM, Konrad Dybcio wrote:
>> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>>> CP_ALWAYS_ON_COUNTER is not save-restored during preemption, so it won't
>>> provide accurate data about the 'submit' when preemption is enabled.
>>> Switch to CP_ALWAYS_ON_CONTEXT which is preemption safe.
>>>
>>> Fixes: e7ae83da4a28 ("drm/msm/a6xx: Implement preemption for a7xx targets")
>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>> ---
>>>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> index 14d5b5e266f7..93bf2c40bfb9 100644
>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> @@ -345,7 +345,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>>  	 * GPU registers so we need to add 0x1a800 to the register value on A630
>>>  	 * to get the right value from PM4.
>>>  	 */
>>> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
>>> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>>>  		rbmemptr_stats(ring, index, alwayson_start));
>>>  
>>>  	/* Invalidate CCU depth and color */
>>> @@ -386,7 +386,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>>  
>>>  	get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP(0),
>>>  		rbmemptr_stats(ring, index, cpcycles_end));
>>> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
>>> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>>>  		rbmemptr_stats(ring, index, alwayson_end));
>>>  
>>>  	/* Write the fence to the scratch register */
>>> @@ -478,10 +478,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>>  
>>>  	if (adreno_is_a8xx(adreno_gpu)) {
>>>  		rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
>>> -		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
>>> +		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT;
>>
>> I'm a little worried about mixing the names here - KGSL uses both of
>> these registers (A6XX_KERNEL_PROFILE vs A6XX_KERNEL_PROFILE_CONTEXT)
>> to track different fields of the struct adreno_drawobj_profile_entry
> 
> But this naming aligns with the HW reg spec. So I prefer to use the same.

To make it clear, my confusion comes from:

cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT
             ^^^^^^^    vs                   ^^^^^^^

i.e. I'm not saying this is wrong, but rather that the local variable
could be renamed as well, to match

Konrad
Re: [PATCH 02/16] drm/msm/a6xx: Switch to preemption safe AO counter
Posted by Akhil P Oommen 1 week, 3 days ago 
On 3/26/2026 2:34 PM, Konrad Dybcio wrote:
> On 3/25/26 10:46 PM, Akhil P Oommen wrote:
>> On 3/24/2026 3:21 PM, Konrad Dybcio wrote:
>>> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>>>> CP_ALWAYS_ON_COUNTER is not save-restored during preemption, so it won't
>>>> provide accurate data about the 'submit' when preemption is enabled.
>>>> Switch to CP_ALWAYS_ON_CONTEXT which is preemption safe.
>>>>
>>>> Fixes: e7ae83da4a28 ("drm/msm/a6xx: Implement preemption for a7xx targets")
>>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>>> ---
>>>>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>>> index 14d5b5e266f7..93bf2c40bfb9 100644
>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>>> @@ -345,7 +345,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>>>  	 * GPU registers so we need to add 0x1a800 to the register value on A630
>>>>  	 * to get the right value from PM4.
>>>>  	 */
>>>> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
>>>> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>>>>  		rbmemptr_stats(ring, index, alwayson_start));
>>>>  
>>>>  	/* Invalidate CCU depth and color */
>>>> @@ -386,7 +386,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>>>  
>>>>  	get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP(0),
>>>>  		rbmemptr_stats(ring, index, cpcycles_end));
>>>> -	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_COUNTER,
>>>> +	get_stats_counter(ring, REG_A6XX_CP_ALWAYS_ON_CONTEXT,
>>>>  		rbmemptr_stats(ring, index, alwayson_end));
>>>>  
>>>>  	/* Write the fence to the scratch register */
>>>> @@ -478,10 +478,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>>>>  
>>>>  	if (adreno_is_a8xx(adreno_gpu)) {
>>>>  		rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
>>>> -		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
>>>> +		cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT;
>>>
>>> I'm a little worried about mixing the names here - KGSL uses both of
>>> these registers (A6XX_KERNEL_PROFILE vs A6XX_KERNEL_PROFILE_CONTEXT)
>>> to track different fields of the struct adreno_drawobj_profile_entry
>>
>> But this naming aligns with the HW reg spec. So I prefer to use the same.
> 
> To make it clear, my confusion comes from:
> 
> cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_CONTEXT
>              ^^^^^^^    vs                   ^^^^^^^
> 
> i.e. I'm not saying this is wrong, but rather that the local variable
> could be renamed as well, to match
> 

Aah! okay. Ack. :)

-Akhil.

> Konrad

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.