[Freedreno] [PATCH 09/16] drm/msm: gpu Add new gpu register read/write functions

[Rob Clark]

On Fri, Nov 4, 2016 at 6:44 PM, Jordan Crouse <jcrouse at codeaurora.org> 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 f39e082..f2f9c91 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -515,16 +515,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);

probably other places are ok to use gpu_read64, but I kinda think the
timestamp counters should stay with the loop while(tmp!=hi)..

I guess if we were reading perfctrs in enough places, a special
read_perfctr type fxn would be in order.

BR,
-R

>         return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 161cd2f..bec3735 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -154,6 +154,45 @@ static inline u32 gpu_read(struct msm_gpu *gpu, u32 reg)
>         return msm_readl(gpu->mmio + (reg << 2));
>  }
>
> +static inline void gpu_rmw(struct msm_gpu *gpu, u32 reg, u32 mask, u32 or)
> +{
> +       uint32_t val = gpu_read(gpu, reg);
> +
> +       val &= ~mask;
> +       gpu_write(gpu, reg, val | or);
> +}
> +
> +static inline u64 gpu_read64(struct msm_gpu *gpu, u32 lo, u32 hi)
> +{
> +       u64 val;
> +
> +       /*
> +        * Why not a readq here? Two reasons: 1) many of the LO registers are
> +        * not quad word aligned and 2) the GPU hardware designers have a bit
> +        * of a history of putting registers where they fit, especially in
> +        * spins. The longer a GPU family goes the higher the chance that
> +        * we'll get burned.  We could do a series of validity checks if we
> +        * wanted to, but really is a readq() that much better? Nah.
> +        */
> +
> +       /*
> +        * For some lo/hi registers (like perfcounters), the hi value is latched
> +        * when the lo is read, so make sure to read the lo first to trigger
> +        * that
> +        */
> +       val = (u64) msm_readl(gpu->mmio + (lo << 2));
> +       val |= ((u64) msm_readl(gpu->mmio + (hi << 2)) << 32);
> +
> +       return val;
> +}
> +
> +static inline void gpu_write64(struct msm_gpu *gpu, u32 lo, u32 hi, u64 val)
> +{
> +       /* Why not a writeq here? Read the screed above */
> +       msm_writel(lower_32_bits(val), gpu->mmio + (lo << 2));
> +       msm_writel(upper_32_bits(val), gpu->mmio + (hi << 2));
> +}
> +
>  int msm_gpu_pm_suspend(struct msm_gpu *gpu);
>  int msm_gpu_pm_resume(struct msm_gpu *gpu);
>
> --
> 1.9.1
>
> _______________________________________________
> Freedreno mailing list
> Freedreno at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno