Re: [Intel-gfx] [PATCH v1 1/2] drm/i915/reg: fix QGV points register access offsets

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Wed, Mar 22, 2023 at 03:01:37AM +0200, Vinod Govindapillai wrote:
> Wrong offsets are calculated to read QGV point registers. Fix it
> to read from the correct registers.
> 
> Bspec: 64602
> 
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d22ffd7a32dc..ae8ba090c0f4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7724,12 +7724,12 @@ enum skl_power_gate {
>  #define   MTL_N_OF_POPULATED_CH_MASK		REG_GENMASK(7, 4)
>  #define   MTL_DDR_TYPE_MASK			REG_GENMASK(3, 0)
>  
> -#define MTL_MEM_SS_INFO_QGV_POINT_LOW(point)	 _MMIO(0x45710 + (point) * 2)
> +#define MTL_MEM_SS_INFO_QGV_POINT_LOW(point)	 _MMIO(0x45710 + (point) * 2 * 0x4)

Omg, how did we screw up so badly here? Basically were reading bogus values. _Excellent_ finding Vinod.

The only thing is that 2 * 0x4 looks a bit complicated to read. 
Wonder if we could just organize it that way:

#define MTL_MEM_SS_INFO_QGV_POINT_OFFSET       0x45710

#define MTL_MEM_SS_INFO_QGV_POINT_LOW(point)   _MMIO(MTL_MEM_SS_INFO_QGV_POINT_OFFSET + (point) * 2 * 0x4)
#define MTL_MEM_SS_INFO_QGV_POINT_HIGH(point)  _MMIO(MTL_MEM_SS_INFO_QGV_POINT_OFFSET + ((point) * 2 + 1) * 0x4)

Or you may just leave 0x45710, point is that this way we don't need that 0x45714 which isn't
mentioned anywhere, so we don't have some additional magic numbers to change potentially, if we need to
do that once again. Probably also having more readable code, could save us from such issues like
you found..

Anyway up to you - you may just leave it as is, I don't insist.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>


>  #define   MTL_TRCD_MASK			REG_GENMASK(31, 24)
>  #define   MTL_TRP_MASK			REG_GENMASK(23, 16)
>  #define   MTL_DCLK_MASK			REG_GENMASK(15, 0)
>  
> -#define MTL_MEM_SS_INFO_QGV_POINT_HIGH(point)	 _MMIO(0x45714 + (point) * 2)
> +#define MTL_MEM_SS_INFO_QGV_POINT_HIGH(point)	 _MMIO(0x45714 + (point) * 2 * 0x4)
>  #define   MTL_TRAS_MASK			REG_GENMASK(16, 8)
>  #define   MTL_TRDPRE_MASK		REG_GENMASK(7, 0)
>  
> -- 
> 2.34.1
>