From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "José Roberto de Souza" <jose.souza@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
Yakui Zhao <yakui.zhao@intel.com>,
Matt Roper <matthew.d.roper@intel.com>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Remove memory frequency calculation
Date: Wed, 13 Oct 2021 12:32:43 +0300 [thread overview]
Message-ID: <YWanuyKbmIDWOGZ3@intel.com> (raw)
In-Reply-To: <20211013010046.91858-1-jose.souza@intel.com>
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.
>
> v2:
> - Also remove memory frequency calculation for gen9 LP platforms
>
> Cc: Yakui Zhao <yakui.zhao@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Fixes: f8112cb9574b ("drm/i915/gen11+: Only load DRAM information from pcode")
> Signed-off-by: José Roberto de Souza <jose.souza@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
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2021-10-13 9:32 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-13 1:00 [Intel-gfx] [PATCH v2] drm/i915: Remove memory frequency calculation José Roberto de Souza
2021-10-13 0:55 ` Souza, Jose
2021-10-13 1:20 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Remove memory frequency calculation (rev2) Patchwork
2021-10-13 1:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-10-13 2:54 ` [Intel-gfx] [PATCH v2] drm/i915: Remove memory frequency calculation Matt Roper
2021-10-13 8:20 ` Zhao, Yakui
2021-10-13 9:22 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Remove memory frequency calculation (rev2) Patchwork
2021-10-13 9:32 ` Ville Syrjälä [this message]
2021-10-13 19:17 ` [Intel-gfx] [PATCH v2] drm/i915: Remove memory frequency calculation Souza, Jose
2021-10-13 19:31 ` Ville Syrjälä
2021-10-13 19:38 ` Souza, Jose
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YWanuyKbmIDWOGZ3@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jose.souza@intel.com \
--cc=matthew.d.roper@intel.com \
--cc=yakui.zhao@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.