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 <[email protected]>
> ---
>  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 <[email protected]>


>  #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
> 

Reply via email to