On Wed, 2021-10-13 at 12:32 +0300, Ville Syrjälä wrote:
> On Tue, Oct 12, 2021 at 06:00:46PM -0700, José Roberto de Souza wrote:
> > This memory frequency calculated is only used to check if it is zero,
> > what is not useful as it will never actually be zero.
> >
> > Also the calculation is wrong, we should be checking other bit to
> > select the appropriate frequency multiplier while this code is stuck
> > with a fixed multiplier.
>
> I don't think the alternate ref clock was ever used.
> At least I don't recall ever seeing it.
>
> The real problem with this is that IIRC this is just the last
> requested frequency. So on a system with SAGV this will
> change dynamically.
>
> >
> > So here dropping it as whole.
>
> We have a second copy of this in gen6_update_ring_freq(). Rather
> than removing one and leaving another potentially broken one behind we
> should probably just consolidate on a single implementation.
gen6_update_ring_freq() is related to GPU frequency not memory, don't look
related at all to me.
>
> >
> > v2:
> > - Also remove memory frequency calculation for gen9 LP platforms
> >
> > Cc: Yakui Zhao <yakui.z...@intel.com>
> > Cc: Matt Roper <matthew.d.ro...@intel.com>
> > Fixes: f8112cb9574b ("drm/i915/gen11+: Only load DRAM information from
> > pcode")
> > Signed-off-by: José Roberto de Souza <jose.so...@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 8 --------
> > drivers/gpu/drm/i915/intel_dram.c | 30 ++----------------------------
> > 2 files changed, 2 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index a897f4abea0c3..8825f7ac477b6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -11109,12 +11109,6 @@ enum skl_power_gate {
> > #define DC_STATE_DEBUG_MASK_CORES (1 << 0)
> > #define DC_STATE_DEBUG_MASK_MEMORY_UP (1 << 1)
> >
> > -#define BXT_P_CR_MC_BIOS_REQ_0_0_0 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x7114)
> > -#define BXT_REQ_DATA_MASK 0x3F
> > -#define BXT_DRAM_CHANNEL_ACTIVE_SHIFT 12
> > -#define BXT_DRAM_CHANNEL_ACTIVE_MASK (0xF << 12)
> > -#define BXT_MEMORY_FREQ_MULTIPLIER_HZ 133333333
> > -
> > #define BXT_D_CR_DRP0_DUNIT8 0x1000
> > #define BXT_D_CR_DRP0_DUNIT9 0x1200
> > #define BXT_D_CR_DRP0_DUNIT_START 8
> > @@ -11145,9 +11139,7 @@ enum skl_power_gate {
> > #define BXT_DRAM_TYPE_LPDDR4 (0x2 << 22)
> > #define BXT_DRAM_TYPE_DDR4 (0x4 << 22)
> >
> > -#define SKL_MEMORY_FREQ_MULTIPLIER_HZ 266666666
> > #define SKL_MC_BIOS_DATA_0_0_0_MCHBAR_PCU _MMIO(MCHBAR_MIRROR_BASE_SNB +
> > 0x5E04)
> > -#define SKL_REQ_DATA_MASK (0xF << 0)
> > #define DG1_GEAR_TYPE REG_BIT(16)
> >
> > #define SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN
> > _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5000)
> > diff --git a/drivers/gpu/drm/i915/intel_dram.c
> > b/drivers/gpu/drm/i915/intel_dram.c
> > index 30a0cab5eff46..0adadfd9528aa 100644
> > --- a/drivers/gpu/drm/i915/intel_dram.c
> > +++ b/drivers/gpu/drm/i915/intel_dram.c
> > @@ -244,7 +244,6 @@ static int
> > skl_get_dram_info(struct drm_i915_private *i915)
> > {
> > struct dram_info *dram_info = &i915->dram_info;
> > - u32 mem_freq_khz, val;
> > int ret;
> >
> > dram_info->type = skl_get_dram_type(i915);
> > @@ -255,17 +254,6 @@ skl_get_dram_info(struct drm_i915_private *i915)
> > if (ret)
> > return ret;
> >
> > - val = intel_uncore_read(&i915->uncore,
> > - SKL_MC_BIOS_DATA_0_0_0_MCHBAR_PCU);
> > - mem_freq_khz = DIV_ROUND_UP((val & SKL_REQ_DATA_MASK) *
> > - SKL_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
> > -
> > - if (dram_info->num_channels * mem_freq_khz == 0) {
> > - drm_info(&i915->drm,
> > - "Couldn't get system memory bandwidth\n");
> > - return -EINVAL;
> > - }
> > -
> > return 0;
> > }
> >
> > @@ -350,24 +338,10 @@ static void bxt_get_dimm_info(struct dram_dimm_info
> > *dimm, u32 val)
> > static int bxt_get_dram_info(struct drm_i915_private *i915)
> > {
> > struct dram_info *dram_info = &i915->dram_info;
> > - u32 dram_channels;
> > - u32 mem_freq_khz, val;
> > - u8 num_active_channels, valid_ranks = 0;
> > + u32 val;
> > + u8 valid_ranks = 0;
> > int i;
> >
> > - val = intel_uncore_read(&i915->uncore, BXT_P_CR_MC_BIOS_REQ_0_0_0);
> > - mem_freq_khz = DIV_ROUND_UP((val & BXT_REQ_DATA_MASK) *
> > - BXT_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
> > -
> > - dram_channels = val & BXT_DRAM_CHANNEL_ACTIVE_MASK;
> > - num_active_channels = hweight32(dram_channels);
> > -
> > - if (mem_freq_khz * num_active_channels == 0) {
> > - drm_info(&i915->drm,
> > - "Couldn't get system memory bandwidth\n");
> > - return -EINVAL;
> > - }
> > -
> > /*
> > * Now read each DUNIT8/9/10/11 to check the rank of each dimms.
> > */
> > --
> > 2.33.0
>
