On Thu, 20 Mar 2025 11:17:33 +0000
Karunika Choo <karunika.c...@arm.com> wrote:

> This patch adds 64-bit register accessors to simplify register access in
> Panthor. It also adds 32-bit and 64-bit variants for read_poll_timeout.
> 
> Signed-off-by: Karunika Choo <karunika.c...@arm.com>
> ---
>  drivers/gpu/drm/panthor/panthor_regs.h | 55 ++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_regs.h 
> b/drivers/gpu/drm/panthor/panthor_regs.h
> index 42dc3fedb0d4..7ec4a1d04e20 100644
> --- a/drivers/gpu/drm/panthor/panthor_regs.h
> +++ b/drivers/gpu/drm/panthor/panthor_regs.h
> @@ -238,4 +238,59 @@
>  #define gpu_read(dev, reg) \
>       readl((dev)->iomem + (reg))
>  
> +#define gpu_read_relaxed(dev, reg) readl_relaxed((dev)->iomem + (reg))
> +
> +#define gpu_write64(dev, reg, data)                            \
> +     do {                                                   \
> +             u64 __val = (u64)(data);                       \
> +             gpu_write(dev, reg, lower_32_bits(__val));     \
> +             gpu_write(dev, reg + 4, upper_32_bits(__val)); \
> +     } while (0)

We're not doing funky name concatenation in these macros, so I'd rather
have them defined as static inline funcs in panthor_device.h. We
probably want to move the gpu_read/write definitions there as well if
we do that.

> +
> +#define gpu_read64(dev, reg) \
> +     (gpu_read(dev, reg) | ((u64)gpu_read(dev, reg + 4) << 32))
> +
> +#define gpu_read64_relaxed(dev, reg)  \
> +     (gpu_read_relaxed(dev, reg) | \
> +      ((u64)gpu_read_relaxed(dev, reg + 4) << 32))
> +
> +#define gpu_read64_sync(dev, reg)                     \
> +     ({                                            \
> +             u32 lo, hi1, hi2;                     \
> +             do {                                  \
> +                     hi1 = gpu_read(dev, reg + 4); \
> +                     lo = gpu_read(dev, reg);      \
> +                     hi2 = gpu_read(dev, reg + 4); \
> +             } while (hi1 != hi2);                 \
> +             lo | ((u64)hi2 << 32);                \
> +     })

I would name that one gpu_read64_counter and make it a static inline
function. Note that we already have panthor_gpu_read_64bit_counter()
which does the same thing, so maybe move it there and rename it along
the way.

> +
> +#define gpu_read_poll_timeout(dev, reg, val, cond, delay_us, timeout_us)    \
> +     read_poll_timeout(gpu_read, val, cond, delay_us, timeout_us, false, \
> +                       dev, reg)
> +
> +#define gpu_read_poll_timeout_atomic(dev, reg, val, cond, delay_us,         \
> +                                  timeout_us)                            \
> +     read_poll_timeout_atomic(gpu_read, val, cond, delay_us, timeout_us, \
> +                              false, dev, reg)
> +
> +#define gpu_read64_poll_timeout(dev, reg, val, cond, delay_us, timeout_us)   
>  \
> +     read_poll_timeout(gpu_read64, val, cond, delay_us, timeout_us, false, \
> +                       dev, reg)
> +
> +#define gpu_read64_poll_timeout_atomic(dev, reg, val, cond, delay_us,        
>  \
> +                                    timeout_us)                            \
> +     read_poll_timeout_atomic(gpu_read64, val, cond, delay_us, timeout_us, \
> +                              false, dev, reg)
> +
> +#define gpu_read_relaxed_poll_timeout_atomic(dev, reg, val, cond, delay_us, \
> +                                          timeout_us)                    \
> +     read_poll_timeout_atomic(gpu_read_relaxed, val, cond, delay_us,     \
> +                              timeout_us, false, dev, reg)
> +
> +#define gpu_read64_relaxed_poll_timeout(dev, reg, val, cond, delay_us,       
>   \
> +                                     timeout_us)                            \
> +     read_poll_timeout(gpu_read64_relaxed, val, cond, delay_us, timeout_us, \
> +                       false, dev, reg)
> +
>  #endif

Reply via email to