Hi,
On 8/17/2018 4:05 AM, Rodrigo Vivi wrote:
On Thu, Jul 26, 2018 at 07:44:07PM +0530, Mahesh Kumar wrote:
This patch adds support to decode system memory bandwidth and other
parameters for skylake and Gen9+ platforms, which will be used for
arbitrated display memory bandwidth calculation in GEN9 based
platforms and WM latency level-0 Work-around calculation on GEN9+.
Changes Since V1:
- s/memdev_info/dram_info
- create a struct to hold channel info
Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 131 ++++++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_drv.h | 7 +++
drivers/gpu/drm/i915/i915_reg.h | 21 +++++++
3 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 16629601c9f4..ddf6bf9b500a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1070,6 +1070,118 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
intel_gvt_sanitize_options(dev_priv);
}
+static int
+skl_dram_get_channel_info(struct dram_channel_info *ch, u32 val)
+{
+ u8 l_rank, s_rank;
+ u8 l_size, s_size;
+ u8 l_width, s_width;
+ enum dram_rank rank;
+
+ if (!val)
+ return -1;
-SOMEERRNO?
ok sure.
+
+ l_size = (val >> SKL_DRAM_SIZE_L_SHIFT) & SKL_DRAM_SIZE_MASK;
+ s_size = (val >> SKL_DRAM_SIZE_S_SHIFT) & SKL_DRAM_SIZE_MASK;
+ l_width = (val >> SKL_DRAM_WIDTH_L_SHIFT) & SKL_DRAM_WIDTH_MASK;
+ s_width = (val >> SKL_DRAM_WIDTH_S_SHIFT) & SKL_DRAM_WIDTH_MASK;
+ l_rank = (val >> SKL_DRAM_RANK_L_SHIFT) & SKL_DRAM_RANK_MASK;
+ s_rank = (val >> SKL_DRAM_RANK_S_SHIFT) & SKL_DRAM_RANK_MASK;
+
+ if (l_size == 0 && s_size == 0)
+ return -1;
ditto
+
+ DRM_DEBUG_KMS("(size:width:rank) L(%dGB:X%d:%s) S(%dGB:X%d:%s)\n",
+ l_size, (1 << l_width) * 8, l_rank ? "dual" : "single",
+ s_size, (1 << s_width) * 8, s_rank ? "dual" : "single");
+
+ if (l_rank == SKL_DRAM_RANK_DUAL || s_rank == SKL_DRAM_RANK_DUAL)
+ rank = I915_DRAM_RANK_DUAL;
+ else if ((l_size && l_rank == SKL_DRAM_RANK_SINGLE) &&
+ (s_size && s_rank == SKL_DRAM_RANK_SINGLE))
+ rank = I915_DRAM_RANK_DUAL;
+ else
+ rank = I915_DRAM_RANK_SINGLE;
+
+ ch->l_info.size = l_size;
+ ch->s_info.size = s_size;
+ ch->l_info.width = l_width;
+ ch->s_info.width = s_width;
+ ch->l_info.rank = l_rank;
+ ch->s_info.rank = s_rank;
+ ch->rank = rank;
could we do this directly without intermediates? not clear if we change
in the middle after printing...
This information is needed later while checking is memories are
symmetric, that's why we need to keep this as well.
+
+ return 0;
+}
+
+static int
+skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = &dev_priv->dram_info;
+ struct dram_channel_info ch0, ch1;
+ u32 val;
+ int ret;
+
+ val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(&ch0, val);
+ if (ret == 0)
+ dram_info->num_channels++;
+
+ val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(&ch1, val);
+ if (ret == 0)
+ dram_info->num_channels++;
+
+ if (dram_info->num_channels == 0) {
+ DRM_INFO("Number of memory channels is zero\n");
+ return -EINVAL;
+ }
+
+ /*
+ * If any of the channel is single rank channel, worst case output
+ * will be same as if single rank memory, so consider single rank
+ * memory.
+ */
+ if (ch0.rank == I915_DRAM_RANK_SINGLE ||
+ ch1.rank == I915_DRAM_RANK_SINGLE)
+ dram_info->rank = I915_DRAM_RANK_SINGLE;
+ else
+ dram_info->rank = max(ch0.rank, ch1.rank);
+
+ if (dram_info->rank == I915_DRAM_RANK_INVALID) {
+ DRM_INFO("couldn't get memory rank information\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int
+skl_get_dram_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = &dev_priv->dram_info;
+ u32 mem_freq_khz, val;
+ int ret;
+
+ ret = skl_dram_get_channels_info(dev_priv);
+ if (ret)
+ return ret;
+
+ val = I915_READ(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);
+
+ dram_info->bandwidth_kbps = dram_info->num_channels *
+ mem_freq_khz * 8;
+
+ if (dram_info->bandwidth_kbps == 0) {
+ DRM_INFO("Couldn't get system memory bandwidth\n");
+ return -EINVAL;
+ }
+
+ dram_info->valid = true;
+ return 0;
+}
+
static int
bxt_get_dram_info(struct drm_i915_private *dev_priv)
{
@@ -1159,6 +1271,7 @@ static void
intel_get_dram_info(struct drm_i915_private *dev_priv)
{
struct dram_info *dram_info = &dev_priv->dram_info;
+ char bandwidth_str[32];
int ret;
dram_info->valid = false;
@@ -1166,15 +1279,25 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
dram_info->bandwidth_kbps = 0;
dram_info->num_channels = 0;
- if (!IS_BROXTON(dev_priv))
+ if (INTEL_GEN(dev_priv) < 9)
return;
- ret = bxt_get_dram_info(dev_priv);
+ /* Need to calculate bandwidth only for Gen9 */
+ if (IS_BROXTON(dev_priv))
+ ret = bxt_get_dram_info(dev_priv);
+ else if (INTEL_GEN(dev_priv) == 9)
+ ret = skl_get_dram_info(dev_priv);
+ else
+ ret = skl_dram_get_channels_info(dev_priv);
I din't get this... why newer platforms only need this partial path?
also this highlights that the names of the functions are confusing...
In gen9 platform we need complete DRAM info (channel configuration +
bandwidth supported by DRAM) which will be used by bandwidth related WA,
and only applicable for GEN9 platforms. Bspec:4380 (Gen9 Watermark
Calculations: Workaround)
But Channel info is applicable for all the platforms, This will be used
to check if system has 16GB dimm or if memory channels are not symmetric.
BSpec:4381 (Retrieve Memory Latency Data)
I agree names seems little confusing I used following convention
dram_info -> channels_info -> channel_info
if (ret)
return;
- DRM_DEBUG_KMS("DRAM bandwidth:%d KBps, total-channels: %u\n",
- dram_info->bandwidth_kbps, dram_info->num_channels);
+ if (dram_info->bandwidth_kbps)
+ sprintf(bandwidth_str, "%d KBps", dram_info->bandwidth_kbps);
+ else
+ sprintf(bandwidth_str, "unknown");
+ DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
+ bandwidth_str, dram_info->num_channels);
DRM_DEBUG_KMS("DRAM rank: %s rank\n",
(dram_info->rank == I915_DRAM_RANK_DUAL) ?
"dual" : "single");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 46f942fa7d60..2d12fc152b49 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2128,6 +2128,13 @@ struct drm_i915_private {
*/
};
+struct dram_channel_info {
+ struct info {
+ u8 size, width, rank;
+ } l_info, s_info;
+ enum dram_rank rank;
why do we need duplicated rand information?
first one represents rank of each dimm left/right dimm.
Second one stores the rank of channel.
-Mahesh
+};
+
static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
{
return container_of(dev, struct drm_i915_private, drm);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 66900d027570..e4d61167fb64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9664,6 +9664,27 @@ enum skl_power_gate {
#define BXT_DRAM_SIZE_12GB 0x3
#define BXT_DRAM_SIZE_16GB 0x4
+#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 SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x500C)
+#define SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5010)
+#define SKL_DRAM_SIZE_MASK 0x3F
+#define SKL_DRAM_SIZE_L_SHIFT 0
+#define SKL_DRAM_SIZE_S_SHIFT 16
+#define SKL_DRAM_WIDTH_MASK 0x3
+#define SKL_DRAM_WIDTH_L_SHIFT 8
+#define SKL_DRAM_WIDTH_S_SHIFT 24
+#define SKL_DRAM_WIDTH_X8 0x0
+#define SKL_DRAM_WIDTH_X16 0x1
+#define SKL_DRAM_WIDTH_X32 0x2
+#define SKL_DRAM_RANK_MASK 0x1
+#define SKL_DRAM_RANK_L_SHIFT 10
+#define SKL_DRAM_RANK_S_SHIFT 26
+#define SKL_DRAM_RANK_SINGLE 0x0
+#define SKL_DRAM_RANK_DUAL 0x1
again, spec pointers please.
+
/* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
* since on HSW we can't write to it using I915_WRITE. */
#define D_COMP_HSW _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
--
2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
