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 <[email protected]>
>>>> ---
>>>> 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
