On 11/23/2016 11:46 AM, Rob Clark wrote:
> On Wed, Nov 23, 2016 at 2:34 PM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
>>> ---
>>> 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
>
>
Ah ok. There's already a comment in the newly introduced function to
that effect but no mention in the commit text hence the confusion.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
