[PATCH 3/3] drm/msm: gpu Add new gpu register read/write functions

[Rob Clark]

On Wed, Nov 23, 2016 at 2:34 PM, Stephen Boyd <sboyd at codeaurora.org> wrote:
> On 11/22/2016 07:47 AM, Jordan Crouse wrote:
>> Add some new functions to manipulate GPU registers.  gpu_read64 and
>> gpu_write64 can read/write a 64 bit value to two 32 bit registers.
>> For 4XX and older these are normally perfcounter registers, but
>> future targets will use 64 bit addressing so there will be many
>> more spots where a 64 bit read and write are needed.
>>
>> gpu_rmw() does a read/modify/write on a 32 bit register given a mask
>> and bits to OR in.
>>
>> Signed-off-by: Jordan Crouse <jcrouse at codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 12 ++---------
>>  drivers/gpu/drm/msm/msm_gpu.h         | 39 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
>> index ba16507..b82210c 100644
>> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
>> @@ -513,16 +513,8 @@ static int a4xx_pm_suspend(struct msm_gpu *gpu) {
>>
>>  static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>>  {
>> -     uint32_t hi, lo, tmp;
>> -
>> -     tmp = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_HI);
>> -     do {
>> -             hi = tmp;
>> -             lo = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO);
>> -             tmp = gpu_read(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_HI);
>> -     } while (tmp != hi);
>> -
>> -     *value = (((uint64_t)hi) << 32) | lo;
>> +     *value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO,
>> +             REG_A4XX_RBBM_PERFCTR_CP_0_HI);
>
> Did we stop caring about the case where the high bits changed while the
> low bits were read? gpu_read64 (pretty poor name by the way considering
> how many GPUs there are supported in the kernel) doesn't look to check
> for or handle that case.

fancy hw is fancy ;-)

seems like for the perf ctrs reading _LO latches _HI so the loop was
overkill (and only worked in the first place because I happened to
read _LO first)

I was planning to smash in a comment to that effect when I merged this patch

BR,
-R


> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel