[PATCH 0/3] Decode memdev info and bandwidth and implemnt latency WA

[PATCH 0/3] Decode memdev info and bandwidth and implemnt latency WA

[Mahesh Kumar]

This series adds support to calculate system memdev parameters and calculate
total system memory bandwidth. This parameters and BW will be used to enable
WM level-0 latency workaround and display memory bandwidth related WA for gen9.

Mahesh Kumar (3):
  drm/i915/bxt: Decode memory bandwidth and parameters
  drm/i915/skl+: Decode memory bandwidth and parameters
  drm/i915: Implement 16GB dimm wa for latency level-0

 drivers/gpu/drm/i915/i915_drv.c | 261 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  13 ++
 drivers/gpu/drm/i915/i915_reg.h |  51 ++++++++
 drivers/gpu/drm/i915/intel_pm.c |  13 ++
 4 files changed, 338 insertions(+)

-- 
2.16.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915/bxt: Decode memory bandwidth and parameters

[Mahesh Kumar]

This patch adds support to decode system memory bandwidth and other
parameters for broxton platform, which will be used for arbitrated
display memory bandwidth calculation in GEN9 based platforms and
WM latency level-0 Work-around calculation on GEN9+ platforms.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 116 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  11 ++++
 drivers/gpu/drm/i915/i915_reg.h |  30 +++++++++++
 3 files changed, 157 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0db3c83cce29..7cf6d3ade878 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1063,6 +1063,116 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 	intel_gvt_sanitize_options(dev_priv);
 }
 
+static int
+bxt_get_memdev_info(struct drm_i915_private *dev_priv)
+{
+	struct memdev_info *memdev_info = &dev_priv->memdev_info;
+	u32 dram_channels;
+	u32 mem_freq_khz, val;
+	u8 num_active_channels;
+	int i;
+
+	val = I915_READ(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_SHIFT) &
+					BXT_DRAM_CHANNEL_ACTIVE_MASK;
+	num_active_channels = hweight32(dram_channels);
+
+	/* Each active bit represents 4-byte channel */
+	memdev_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
+
+	if (memdev_info->bandwidth_kbps == 0) {
+		DRM_INFO("Couldn't get system memory bandwidth\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Now read each DUNIT8/9/10/11 to check the rank of each dimms.
+	 */
+	for (i = BXT_D_CR_DRP0_DUNIT_START; i <= BXT_D_CR_DRP0_DUNIT_END; i++) {
+		u8 rank, size, width;
+		enum memdev_rank ch_rank;
+
+		val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
+		if (val == 0xFFFFFFFF)
+			continue;
+
+		memdev_info->num_channels++;
+		rank = val & BXT_DRAM_RANK_MASK;
+		width = (val >> BXT_DRAM_WIDTH_SHIFT) & BXT_DRAM_WIDTH_MASK;
+		size = (val >> BXT_DRAM_SIZE_SHIFT) & BXT_DRAM_SIZE_MASK;
+		if (rank == BXT_DRAM_RANK_SINGLE)
+			ch_rank = I915_DRAM_RANK_SINGLE;
+		else if (rank == BXT_DRAM_RANK_DUAL)
+			ch_rank = I915_DRAM_RANK_DUAL;
+		else
+			ch_rank = I915_DRAM_RANK_INVALID;
+
+		if (size == BXT_DRAM_SIZE_4GB)
+			size = 4;
+		else if (size == BXT_DRAM_SIZE_6GB)
+			size = 6;
+		else if (size == BXT_DRAM_SIZE_8GB)
+			size = 8;
+		else if (size == BXT_DRAM_SIZE_12GB)
+			size = 12;
+		else if (size == BXT_DRAM_SIZE_16GB)
+			size = 16;
+		else
+			size = 0;
+
+		width = (1 << width) * 8;
+		DRM_DEBUG_KMS("Memdev size:%dGB width:X%d rank:%s\n", size,
+			      width, rank == BXT_DRAM_RANK_SINGLE ? "single" :
+			      rank == BXT_DRAM_RANK_DUAL ? "dual" : "unknown");
+
+		/*
+		 * 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 (memdev_info->rank == I915_DRAM_RANK_INVALID)
+			memdev_info->rank = ch_rank;
+		else if (ch_rank == I915_DRAM_RANK_SINGLE)
+			memdev_info->rank = I915_DRAM_RANK_SINGLE;
+	}
+
+	if (memdev_info->rank == I915_DRAM_RANK_INVALID) {
+		DRM_INFO("couldn't get memory rank information\n");
+		return -EINVAL;
+	}
+
+	memdev_info->valid = true;
+	return 0;
+}
+
+static void
+intel_get_memdev_info(struct drm_i915_private *dev_priv)
+{
+	struct memdev_info *memdev_info = &dev_priv->memdev_info;
+	int ret;
+
+	memdev_info->valid = false;
+	memdev_info->rank = I915_DRAM_RANK_INVALID;
+	memdev_info->bandwidth_kbps = 0;
+	memdev_info->num_channels = 0;
+
+	if (!IS_BROXTON(dev_priv))
+		return;
+
+	ret = bxt_get_memdev_info(dev_priv);
+	if (ret)
+		return;
+
+	DRM_DEBUG_KMS("DRAM bandwidth:%d KBps, total-channels: %u\n",
+		      memdev_info->bandwidth_kbps, memdev_info->num_channels);
+	DRM_DEBUG_KMS("DRAM rank: %s rank\n",
+		      (memdev_info->rank == I915_DRAM_RANK_DUAL) ?
+		      "dual" : "single");
+}
+
 /**
  * i915_driver_init_hw - setup state requiring device access
  * @dev_priv: device private
@@ -1180,6 +1290,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		goto err_msi;
 
 	intel_opregion_setup(dev_priv);
+	/*
+	 * Fill the memdev structure to get the system raw bandwidth and
+	 * memdev info. This will be used for memory latency calculation.
+	 */
+	intel_get_memdev_info(dev_priv);
+
 
 	return 0;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 09ab12458244..845447d3806a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1912,6 +1912,17 @@ struct drm_i915_private {
 		bool distrust_bios_wm;
 	} wm;
 
+	struct memdev_info {
+		bool valid;
+		u8 num_channels;
+		enum memdev_rank {
+			I915_DRAM_RANK_INVALID = 0,
+			I915_DRAM_RANK_SINGLE,
+			I915_DRAM_RANK_DUAL
+		} rank;
+		u32 bandwidth_kbps;
+	} memdev_info;
+
 	struct i915_runtime_pm runtime_pm;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0424e45f88db..81705b952889 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9421,6 +9421,36 @@ 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
+#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
+#define  BXT_D_CR_DRP0_DUNIT_END		11
+#define BXT_D_CR_DRP0_DUNIT(x)	_MMIO(MCHBAR_MIRROR_BASE_SNB + \
+				      _PICK_EVEN((x) - 8, BXT_D_CR_DRP0_DUNIT8,\
+						 BXT_D_CR_DRP0_DUNIT9))
+#define  BXT_DRAM_RANK_MASK			0x3
+#define  BXT_DRAM_RANK_SINGLE			0x1
+#define  BXT_DRAM_RANK_DUAL			0x3
+#define  BXT_DRAM_WIDTH_MASK			0x3
+#define  BXT_DRAM_WIDTH_SHIFT			4
+#define  BXT_DRAM_WIDTH_X8			0x0
+#define  BXT_DRAM_WIDTH_X16			0x1
+#define  BXT_DRAM_WIDTH_X32			0x2
+#define  BXT_DRAM_WIDTH_X64			0x3
+#define  BXT_DRAM_SIZE_MASK			0x7
+#define  BXT_DRAM_SIZE_SHIFT			6
+#define  BXT_DRAM_SIZE_4GB			0x0
+#define  BXT_DRAM_SIZE_6GB			0x1
+#define  BXT_DRAM_SIZE_8GB			0x2
+#define  BXT_DRAM_SIZE_12GB			0x3
+#define  BXT_DRAM_SIZE_16GB			0x4
+
 /* 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/skl+: Decode memory bandwidth and parameters

[Mahesh Kumar]

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+.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 122 ++++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h |  21 +++++++
 2 files changed, 139 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7cf6d3ade878..b93194cbd820 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1063,6 +1063,109 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 	intel_gvt_sanitize_options(dev_priv);
 }
 
+static enum memdev_rank
+skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
+{
+	u8 l_rank, s_rank;
+	u8 l_size, s_size;
+	u8 l_width, s_width;
+	enum memdev_rank rank;
+
+	if (!val)
+		return I915_DRAM_RANK_INVALID;
+
+	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 I915_DRAM_RANK_INVALID;
+
+	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;
+
+	return rank;
+}
+
+static int
+skl_get_mem_dimm_info(struct drm_i915_private *dev_priv)
+{
+	struct memdev_info *memdev_info = &dev_priv->memdev_info;
+	enum memdev_rank ch0_rank, ch1_rank;
+	u32 val;
+
+	val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+	ch0_rank = skl_memdev_get_channel_rank(memdev_info, val);
+	if (ch0_rank != I915_DRAM_RANK_INVALID)
+		memdev_info->num_channels++;
+
+	val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+	ch1_rank = skl_memdev_get_channel_rank(memdev_info, val);
+	if (ch1_rank != I915_DRAM_RANK_INVALID)
+		memdev_info->num_channels++;
+
+	if (memdev_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)
+		memdev_info->rank = I915_DRAM_RANK_SINGLE;
+	else
+		memdev_info->rank = max(ch0_rank, ch1_rank);
+
+	if (memdev_info->rank == I915_DRAM_RANK_INVALID) {
+		DRM_INFO("couldn't get memory rank information\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int
+skl_get_memdev_info(struct drm_i915_private *dev_priv)
+{
+	struct memdev_info *memdev_info = &dev_priv->memdev_info;
+	u32 mem_freq_khz, val;
+	int ret;
+
+	ret = skl_get_mem_dimm_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);
+
+	memdev_info->bandwidth_kbps = memdev_info->num_channels *
+							mem_freq_khz * 8;
+
+	if (memdev_info->bandwidth_kbps == 0) {
+		DRM_INFO("Couldn't get system memory bandwidth\n");
+		return -EINVAL;
+	}
+
+	memdev_info->valid = true;
+	return 0;
+}
+
 static int
 bxt_get_memdev_info(struct drm_i915_private *dev_priv)
 {
@@ -1152,6 +1255,7 @@ static void
 intel_get_memdev_info(struct drm_i915_private *dev_priv)
 {
 	struct memdev_info *memdev_info = &dev_priv->memdev_info;
+	char bandwidth_str[32];
 	int ret;
 
 	memdev_info->valid = false;
@@ -1159,15 +1263,25 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
 	memdev_info->bandwidth_kbps = 0;
 	memdev_info->num_channels = 0;
 
-	if (!IS_BROXTON(dev_priv))
+	if (INTEL_GEN(dev_priv) < 9)
 		return;
 
-	ret = bxt_get_memdev_info(dev_priv);
+	/* Need to calculate bandwidth only for Gen9 */
+	if (IS_BROXTON(dev_priv))
+		ret = bxt_get_memdev_info(dev_priv);
+	else if (INTEL_GEN(dev_priv) == 9)
+		ret = skl_get_memdev_info(dev_priv);
+	else
+		ret = skl_get_mem_dimm_info(dev_priv);
 	if (ret)
 		return;
 
-	DRM_DEBUG_KMS("DRAM bandwidth:%d KBps, total-channels: %u\n",
-		      memdev_info->bandwidth_kbps, memdev_info->num_channels);
+	if (memdev_info->bandwidth_kbps)
+		sprintf(bandwidth_str, "%d KBps", memdev_info->bandwidth_kbps);
+	else
+		sprintf(bandwidth_str, "unknown");
+	DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
+		      bandwidth_str, memdev_info->num_channels);
 	DRM_DEBUG_KMS("DRAM rank: %s rank\n",
 		      (memdev_info->rank == I915_DRAM_RANK_DUAL) ?
 		      "dual" : "single");
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 81705b952889..c232d59f5886 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9451,6 +9451,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
+
 /* 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Implement 16GB dimm wa for latency level-0

[Mahesh Kumar]

Memory with 16GB dimms require an increase of 1us in level-0 latency.
This patch implements the same.
Bspec: 4381

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 35 +++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b93194cbd820..c6d30653d70c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1063,6 +1063,29 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 	intel_gvt_sanitize_options(dev_priv);
 }
 
+static void
+intel_memdev_is_16gb_dimm(struct memdev_info *memdev_info,
+			  u8 rank, u8 size, u8 width)
+{
+	bool found_16gb_dimm = false;
+
+	if (size == 16 && width == SKL_DRAM_WIDTH_X8 &&
+	    rank == SKL_DRAM_RANK_SINGLE)
+		found_16gb_dimm = true;
+	else if (size == 32 && width == SKL_DRAM_WIDTH_X8 &&
+		 rank == SKL_DRAM_RANK_DUAL)
+		found_16gb_dimm = true;
+	else if (size == 8 && width == SKL_DRAM_WIDTH_X16 &&
+		 rank == SKL_DRAM_RANK_SINGLE)
+		found_16gb_dimm = true;
+	else if (size == 16 && width == SKL_DRAM_WIDTH_X16 &&
+		 rank == SKL_DRAM_RANK_DUAL)
+		found_16gb_dimm = true;
+
+	if (!memdev_info->is_16gb_dimm)
+		memdev_info->is_16gb_dimm = found_16gb_dimm;
+}
+
 static enum memdev_rank
 skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
 {
@@ -1084,6 +1107,8 @@ skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
 	if (l_size == 0 && s_size == 0)
 		return I915_DRAM_RANK_INVALID;
 
+	memdev_info->valid_dimm = true;
+
 	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");
@@ -1096,6 +1121,9 @@ skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
 	else
 		rank = I915_DRAM_RANK_SINGLE;
 
+	intel_memdev_is_16gb_dimm(memdev_info, l_rank, l_size, l_width);
+	intel_memdev_is_16gb_dimm(memdev_info, s_rank, s_size, s_width);
+
 	return rank;
 }
 
@@ -1247,6 +1275,7 @@ bxt_get_memdev_info(struct drm_i915_private *dev_priv)
 		return -EINVAL;
 	}
 
+	memdev_info->valid_dimm = true;
 	memdev_info->valid = true;
 	return 0;
 }
@@ -1259,6 +1288,8 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
 	int ret;
 
 	memdev_info->valid = false;
+	memdev_info->valid_dimm = false;
+	memdev_info->is_16gb_dimm = false;
 	memdev_info->rank = I915_DRAM_RANK_INVALID;
 	memdev_info->bandwidth_kbps = 0;
 	memdev_info->num_channels = 0;
@@ -1282,9 +1313,9 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
 		sprintf(bandwidth_str, "unknown");
 	DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
 		      bandwidth_str, memdev_info->num_channels);
-	DRM_DEBUG_KMS("DRAM rank: %s rank\n",
+	DRM_DEBUG_KMS("DRAM rank: %s rank 16GB-dimm:%s\n",
 		      (memdev_info->rank == I915_DRAM_RANK_DUAL) ?
-		      "dual" : "single");
+		      "dual" : "single", yesno(memdev_info->is_16gb_dimm));
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 845447d3806a..244adf8a30f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1914,6 +1914,8 @@ struct drm_i915_private {
 
 	struct memdev_info {
 		bool valid;
+		bool valid_dimm;
+		bool is_16gb_dimm;
 		u8 num_channels;
 		enum memdev_rank {
 			I915_DRAM_RANK_INVALID = 0,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 53aaaa3e6886..f20f2f9118df 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2874,6 +2874,19 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 			}
 		}
 
+		/*
+		 * WA Level-0 adjustment for 16GB DIMMs: SKL+
+		 * If we could not get dimm info enable this WA to prevent from
+		 * any underrun. If not able to get Dimm info assume 16GB dimm
+		 * to avoid any underrun.
+		 */
+		if (dev_priv->memdev_info.valid_dimm) {
+			if (dev_priv->memdev_info.is_16gb_dimm)
+				wm[0] += 1;
+		} else {
+			wm[0] += 1;
+		}
+
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		uint64_t sskpd = I915_READ64(MCH_SSKPD);
 
-- 
2.16.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for Decode memdev info and bandwidth and implemnt latency WA

[Patchwork]

== Series Details ==

Series: Decode memdev info and bandwidth and implemnt latency WA
URL   : https://patchwork.freedesktop.org/series/46481/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/bxt: Decode memory bandwidth and parameters
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3656:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3667:16: warning: expression using sizeof(void)

Commit: drm/i915/skl+: Decode memory bandwidth and parameters
+drivers/gpu/drm/i915/i915_drv.c:1138:37: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_drv.c:1138:37: warning: expression using sizeof(void)

Commit: drm/i915: Implement 16GB dimm wa for latency level-0
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3667:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3669:16: warning: expression using sizeof(void)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Decode memdev info and bandwidth and implemnt latency WA

[Patchwork]

== Series Details ==

Series: Decode memdev info and bandwidth and implemnt latency WA
URL   : https://patchwork.freedesktop.org/series/46481/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4485 -> Patchwork_9647 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46481/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9647 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-skl-6700hq:      PASS -> FAIL (fdo#106083)

    
  fdo#106083 https://bugs.freedesktop.org/show_bug.cgi?id=106083


== Participating hosts (46 -> 42) ==

  Additional (1): fi-ivb-3520m 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4485 -> Patchwork_9647

  CI_DRM_4485: d3d471400bf907f8a6e51c8a475202a61bcdf2de @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4554: a742ebd9b4908c7eaca8a3d54f86b3d14583b5b5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9647: 3ffafccbd1c7a79a90151d4ab9bbad4dac1a9469 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3ffafccbd1c7 drm/i915: Implement 16GB dimm wa for latency level-0
e870cca4b75a drm/i915/skl+: Decode memory bandwidth and parameters
a5ffeddc5262 drm/i915/bxt: Decode memory bandwidth and parameters

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9647/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] Decode memdev info and bandwidth and implemnt latency WA

[Ville Syrjälä]

On Fri, Jul 13, 2018 at 07:41:21PM +0530, Mahesh Kumar wrote:
> This series adds support to calculate system memdev parameters and calculate

What's "memdev"?

> total system memory bandwidth. This parameters and BW will be used to enable
> WM level-0 latency workaround and display memory bandwidth related WA for gen9.
> 
> Mahesh Kumar (3):
>   drm/i915/bxt: Decode memory bandwidth and parameters
>   drm/i915/skl+: Decode memory bandwidth and parameters
>   drm/i915: Implement 16GB dimm wa for latency level-0
> 
>  drivers/gpu/drm/i915/i915_drv.c | 261 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  13 ++
>  drivers/gpu/drm/i915/i915_reg.h |  51 ++++++++
>  drivers/gpu/drm/i915/intel_pm.c |  13 ++
>  4 files changed, 338 insertions(+)
> 
> -- 
> 2.16.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Implement 16GB dimm wa for latency level-0

[Ville Syrjälä]

On Fri, Jul 13, 2018 at 07:41:24PM +0530, Mahesh Kumar wrote:
> Memory with 16GB dimms require an increase of 1us in level-0 latency.
> This patch implements the same.
> Bspec: 4381
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 35 +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
>  3 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b93194cbd820..c6d30653d70c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1063,6 +1063,29 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>  	intel_gvt_sanitize_options(dev_priv);
>  }
>  
> +static void
> +intel_memdev_is_16gb_dimm(struct memdev_info *memdev_info,
> +			  u8 rank, u8 size, u8 width)
> +{
> +	bool found_16gb_dimm = false;
> +
> +	if (size == 16 && width == SKL_DRAM_WIDTH_X8 &&
> +	    rank == SKL_DRAM_RANK_SINGLE)
> +		found_16gb_dimm = true;
> +	else if (size == 32 && width == SKL_DRAM_WIDTH_X8 &&
> +		 rank == SKL_DRAM_RANK_DUAL)
> +		found_16gb_dimm = true;
> +	else if (size == 8 && width == SKL_DRAM_WIDTH_X16 &&
> +		 rank == SKL_DRAM_RANK_SINGLE)
> +		found_16gb_dimm = true;
> +	else if (size == 16 && width == SKL_DRAM_WIDTH_X16 &&
> +		 rank == SKL_DRAM_RANK_DUAL)
> +		found_16gb_dimm = true;
> +
> +	if (!memdev_info->is_16gb_dimm)
> +		memdev_info->is_16gb_dimm = found_16gb_dimm;
> +}

Pure functions are generally more fun. Would also make the function name
match the implementation.

> +
>  static enum memdev_rank
>  skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
>  {
> @@ -1084,6 +1107,8 @@ skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
>  	if (l_size == 0 && s_size == 0)
>  		return I915_DRAM_RANK_INVALID;
>  
> +	memdev_info->valid_dimm = true;
> +
>  	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");
> @@ -1096,6 +1121,9 @@ skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
>  	else
>  		rank = I915_DRAM_RANK_SINGLE;
>  
> +	intel_memdev_is_16gb_dimm(memdev_info, l_rank, l_size, l_width);
> +	intel_memdev_is_16gb_dimm(memdev_info, s_rank, s_size, s_width);
> +
>  	return rank;
>  }
>  
> @@ -1247,6 +1275,7 @@ bxt_get_memdev_info(struct drm_i915_private *dev_priv)
>  		return -EINVAL;
>  	}
>  
> +	memdev_info->valid_dimm = true;
>  	memdev_info->valid = true;
>  	return 0;
>  }
> @@ -1259,6 +1288,8 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
>  	int ret;
>  
>  	memdev_info->valid = false;
> +	memdev_info->valid_dimm = false;
> +	memdev_info->is_16gb_dimm = false;
>  	memdev_info->rank = I915_DRAM_RANK_INVALID;
>  	memdev_info->bandwidth_kbps = 0;
>  	memdev_info->num_channels = 0;
> @@ -1282,9 +1313,9 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
>  		sprintf(bandwidth_str, "unknown");
>  	DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
>  		      bandwidth_str, memdev_info->num_channels);
> -	DRM_DEBUG_KMS("DRAM rank: %s rank\n",
> +	DRM_DEBUG_KMS("DRAM rank: %s rank 16GB-dimm:%s\n",
>  		      (memdev_info->rank == I915_DRAM_RANK_DUAL) ?
> -		      "dual" : "single");
> +		      "dual" : "single", yesno(memdev_info->is_16gb_dimm));
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 845447d3806a..244adf8a30f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1914,6 +1914,8 @@ struct drm_i915_private {
>  
>  	struct memdev_info {
>  		bool valid;
> +		bool valid_dimm;
> +		bool is_16gb_dimm;
>  		u8 num_channels;
>  		enum memdev_rank {
>  			I915_DRAM_RANK_INVALID = 0,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 53aaaa3e6886..f20f2f9118df 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2874,6 +2874,19 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
>  			}
>  		}
>  
> +		/*
> +		 * WA Level-0 adjustment for 16GB DIMMs: SKL+
> +		 * If we could not get dimm info enable this WA to prevent from
> +		 * any underrun. If not able to get Dimm info assume 16GB dimm
> +		 * to avoid any underrun.
> +		 */
> +		if (dev_priv->memdev_info.valid_dimm) {
> +			if (dev_priv->memdev_info.is_16gb_dimm)

So we only need these two bools in the end? Is there any point in storing
all the intermediate stuff under dev_priv?

> +				wm[0] += 1;
> +		} else {
> +			wm[0] += 1;
> +		}
> +
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		uint64_t sskpd = I915_READ64(MCH_SSKPD);
>  
> -- 
> 2.16.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Implement 16GB dimm wa for latency level-0

[Kumar, Mahesh]

Hi,

Thanks for review.


On 7/13/2018 8:27 PM, Ville Syrjälä wrote:
> On Fri, Jul 13, 2018 at 07:41:24PM +0530, Mahesh Kumar wrote:
>> Memory with 16GB dimms require an increase of 1us in level-0 latency.
>> This patch implements the same.
>> Bspec: 4381
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c | 35 +++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/i915_drv.h |  2 ++
>>   drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
>>   3 files changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index b93194cbd820..c6d30653d70c 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1063,6 +1063,29 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>>   	intel_gvt_sanitize_options(dev_priv);
>>   }
>>   
>> +static void
>> +intel_memdev_is_16gb_dimm(struct memdev_info *memdev_info,
>> +			  u8 rank, u8 size, u8 width)
>> +{
>> +	bool found_16gb_dimm = false;
>> +
>> +	if (size == 16 && width == SKL_DRAM_WIDTH_X8 &&
>> +	    rank == SKL_DRAM_RANK_SINGLE)
>> +		found_16gb_dimm = true;
>> +	else if (size == 32 && width == SKL_DRAM_WIDTH_X8 &&
>> +		 rank == SKL_DRAM_RANK_DUAL)
>> +		found_16gb_dimm = true;
>> +	else if (size == 8 && width == SKL_DRAM_WIDTH_X16 &&
>> +		 rank == SKL_DRAM_RANK_SINGLE)
>> +		found_16gb_dimm = true;
>> +	else if (size == 16 && width == SKL_DRAM_WIDTH_X16 &&
>> +		 rank == SKL_DRAM_RANK_DUAL)
>> +		found_16gb_dimm = true;
>> +
>> +	if (!memdev_info->is_16gb_dimm)
>> +		memdev_info->is_16gb_dimm = found_16gb_dimm;
>> +}
> Pure functions are generally more fun. Would also make the function name
> match the implementation.
Are you suggesting to change the name to only "is_16gb_dimm" ?
>> +
>>   static enum memdev_rank
>>   skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
>>   {
>> @@ -1084,6 +1107,8 @@ skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
>>   	if (l_size == 0 && s_size == 0)
>>   		return I915_DRAM_RANK_INVALID;
>>   
>> +	memdev_info->valid_dimm = true;
>> +
>>   	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");
>> @@ -1096,6 +1121,9 @@ skl_memdev_get_channel_rank(struct memdev_info *memdev_info, u32 val)
>>   	else
>>   		rank = I915_DRAM_RANK_SINGLE;
>>   
>> +	intel_memdev_is_16gb_dimm(memdev_info, l_rank, l_size, l_width);
>> +	intel_memdev_is_16gb_dimm(memdev_info, s_rank, s_size, s_width);
>> +
>>   	return rank;
>>   }
>>   
>> @@ -1247,6 +1275,7 @@ bxt_get_memdev_info(struct drm_i915_private *dev_priv)
>>   		return -EINVAL;
>>   	}
>>   
>> +	memdev_info->valid_dimm = true;
>>   	memdev_info->valid = true;
>>   	return 0;
>>   }
>> @@ -1259,6 +1288,8 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
>>   	int ret;
>>   
>>   	memdev_info->valid = false;
>> +	memdev_info->valid_dimm = false;
>> +	memdev_info->is_16gb_dimm = false;
>>   	memdev_info->rank = I915_DRAM_RANK_INVALID;
>>   	memdev_info->bandwidth_kbps = 0;
>>   	memdev_info->num_channels = 0;
>> @@ -1282,9 +1313,9 @@ intel_get_memdev_info(struct drm_i915_private *dev_priv)
>>   		sprintf(bandwidth_str, "unknown");
>>   	DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
>>   		      bandwidth_str, memdev_info->num_channels);
>> -	DRM_DEBUG_KMS("DRAM rank: %s rank\n",
>> +	DRM_DEBUG_KMS("DRAM rank: %s rank 16GB-dimm:%s\n",
>>   		      (memdev_info->rank == I915_DRAM_RANK_DUAL) ?
>> -		      "dual" : "single");
>> +		      "dual" : "single", yesno(memdev_info->is_16gb_dimm));
>>   }
>>   
>>   /**
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 845447d3806a..244adf8a30f1 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1914,6 +1914,8 @@ struct drm_i915_private {
>>   
>>   	struct memdev_info {
>>   		bool valid;
>> +		bool valid_dimm;
>> +		bool is_16gb_dimm;
>>   		u8 num_channels;
>>   		enum memdev_rank {
>>   			I915_DRAM_RANK_INVALID = 0,
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 53aaaa3e6886..f20f2f9118df 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2874,6 +2874,19 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
>>   			}
>>   		}
>>   
>> +		/*
>> +		 * WA Level-0 adjustment for 16GB DIMMs: SKL+
>> +		 * If we could not get dimm info enable this WA to prevent from
>> +		 * any underrun. If not able to get Dimm info assume 16GB dimm
>> +		 * to avoid any underrun.
>> +		 */
>> +		if (dev_priv->memdev_info.valid_dimm) {
>> +			if (dev_priv->memdev_info.is_16gb_dimm)
> So we only need these two bools in the end? Is there any point in storing
> all the intermediate stuff under dev_priv?
Gen-9 Watermark calculations require other parameters "system bandwidth, 
memory rank and number of channels" for memory-bandwidth workaround  
calculation. I'm working on patches to enable workaround only if it's 
required or may be required. (calculate only during modeset to avoid 
taking modeset lock during each flip)
Currently we enabling those workaround unconditionally because of which 
we are facing multiple issues i.e. bug similar to following
https://bugs.freedesktop.org/show_bug.cgi?id=107113

-Mahesh
>> +				wm[0] += 1;
>> +		} else {
>> +			wm[0] += 1;
>> +		}
>> +
>>   	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>>   		uint64_t sskpd = I915_READ64(MCH_SSKPD);
>>   
>> -- 
>> 2.16.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] Decode memdev info and bandwidth and implemnt latency WA

[Kumar, Mahesh]

On 7/13/2018 8:21 PM, Ville Syrjälä wrote:
> On Fri, Jul 13, 2018 at 07:41:21PM +0530, Mahesh Kumar wrote:
>> This series adds support to calculate system memdev parameters and calculate
> What's "memdev"?
memory device.
-Mahesh
>
>> total system memory bandwidth. This parameters and BW will be used to enable
>> WM level-0 latency workaround and display memory bandwidth related WA for gen9.
>>
>> Mahesh Kumar (3):
>>    drm/i915/bxt: Decode memory bandwidth and parameters
>>    drm/i915/skl+: Decode memory bandwidth and parameters
>>    drm/i915: Implement 16GB dimm wa for latency level-0
>>
>>   drivers/gpu/drm/i915/i915_drv.c | 261 ++++++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_drv.h |  13 ++
>>   drivers/gpu/drm/i915/i915_reg.h |  51 ++++++++
>>   drivers/gpu/drm/i915/intel_pm.c |  13 ++
>>   4 files changed, 338 insertions(+)
>>
>> -- 
>> 2.16.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] Decode memdev info and bandwidth and implemnt latency WA

[Ville Syrjälä]

On Sat, Jul 14, 2018 at 07:42:17PM +0530, Kumar, Mahesh wrote:
> 
> 
> On 7/13/2018 8:21 PM, Ville Syrjälä wrote:
> > On Fri, Jul 13, 2018 at 07:41:21PM +0530, Mahesh Kumar wrote:
> >> This series adds support to calculate system memdev parameters and calculate
> > What's "memdev"?
> memory device.

Still not sure what that means. Makes me think of something akin to chardev
or blockdev. I guess I'd just call it mem_info or something like that.

> -Mahesh
> >
> >> total system memory bandwidth. This parameters and BW will be used to enable
> >> WM level-0 latency workaround and display memory bandwidth related WA for gen9.
> >>
> >> Mahesh Kumar (3):
> >>    drm/i915/bxt: Decode memory bandwidth and parameters
> >>    drm/i915/skl+: Decode memory bandwidth and parameters
> >>    drm/i915: Implement 16GB dimm wa for latency level-0
> >>
> >>   drivers/gpu/drm/i915/i915_drv.c | 261 ++++++++++++++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/i915/i915_drv.h |  13 ++
> >>   drivers/gpu/drm/i915/i915_reg.h |  51 ++++++++
> >>   drivers/gpu/drm/i915/intel_pm.c |  13 ++
> >>   4 files changed, 338 insertions(+)
> >>
> >> -- 
> >> 2.16.2
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Implement 16GB dimm wa for latency level-0

[Ville Syrjälä]

On Sat, Jul 14, 2018 at 07:40:43PM +0530, Kumar, Mahesh wrote:
> Hi,
> 
> Thanks for review.
> 
> 
> On 7/13/2018 8:27 PM, Ville Syrjälä wrote:
> > On Fri, Jul 13, 2018 at 07:41:24PM +0530, Mahesh Kumar wrote:
> >> Memory with 16GB dimms require an increase of 1us in level-0 latency.
> >> This patch implements the same.
> >> Bspec: 4381
> >>
> >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.c | 35 +++++++++++++++++++++++++++++++++--
> >>   drivers/gpu/drm/i915/i915_drv.h |  2 ++
> >>   drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
> >>   3 files changed, 48 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index b93194cbd820..c6d30653d70c 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -1063,6 +1063,29 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
> >>   	intel_gvt_sanitize_options(dev_priv);
> >>   }
> >>   
> >> +static void
> >> +intel_memdev_is_16gb_dimm(struct memdev_info *memdev_info,
> >> +			  u8 rank, u8 size, u8 width)
> >> +{
> >> +	bool found_16gb_dimm = false;
> >> +
> >> +	if (size == 16 && width == SKL_DRAM_WIDTH_X8 &&
> >> +	    rank == SKL_DRAM_RANK_SINGLE)
> >> +		found_16gb_dimm = true;
> >> +	else if (size == 32 && width == SKL_DRAM_WIDTH_X8 &&
> >> +		 rank == SKL_DRAM_RANK_DUAL)
> >> +		found_16gb_dimm = true;
> >> +	else if (size == 8 && width == SKL_DRAM_WIDTH_X16 &&
> >> +		 rank == SKL_DRAM_RANK_SINGLE)
> >> +		found_16gb_dimm = true;
> >> +	else if (size == 16 && width == SKL_DRAM_WIDTH_X16 &&
> >> +		 rank == SKL_DRAM_RANK_DUAL)
> >> +		found_16gb_dimm = true;
> >> +
> >> +	if (!memdev_info->is_16gb_dimm)
> >> +		memdev_info->is_16gb_dimm = found_16gb_dimm;
> >> +}
> > Pure functions are generally more fun. Would also make the function name
> > match the implementation.
> Are you suggesting to change the name to only "is_16gb_dimm" ?

I'm suggesting making it pure.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] Decode memdev info and bandwidth and implemnt latency WA

[Kumar, Mahesh]

Hi,


On 7/18/2018 5:33 PM, Ville Syrjälä wrote:
> On Sat, Jul 14, 2018 at 07:42:17PM +0530, Kumar, Mahesh wrote:
>>
>> On 7/13/2018 8:21 PM, Ville Syrjälä wrote:
>>> On Fri, Jul 13, 2018 at 07:41:21PM +0530, Mahesh Kumar wrote:
>>>> This series adds support to calculate system memdev parameters and calculate
>>> What's "memdev"?
>> memory device.
> Still not sure what that means. Makes me think of something akin to chardev
> or blockdev. I guess I'd just call it mem_info or something like that.
It's in reality dram specification information, what about if we call it 
"dram_info"?
If ok, will update all the occurrence to s/memdev_info/dram_info

-Mahesh
>
>> -Mahesh
>>>> total system memory bandwidth. This parameters and BW will be used to enable
>>>> WM level-0 latency workaround and display memory bandwidth related WA for gen9.
>>>>
>>>> Mahesh Kumar (3):
>>>>     drm/i915/bxt: Decode memory bandwidth and parameters
>>>>     drm/i915/skl+: Decode memory bandwidth and parameters
>>>>     drm/i915: Implement 16GB dimm wa for latency level-0
>>>>
>>>>    drivers/gpu/drm/i915/i915_drv.c | 261 ++++++++++++++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/i915/i915_drv.h |  13 ++
>>>>    drivers/gpu/drm/i915/i915_reg.h |  51 ++++++++
>>>>    drivers/gpu/drm/i915/intel_pm.c |  13 ++
>>>>    4 files changed, 338 insertions(+)
>>>>
>>>> -- 
>>>> 2.16.2
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Implement 16GB dimm wa for latency level-0

[Kumar, Mahesh]

Hi,


On 7/18/2018 5:34 PM, Ville Syrjälä wrote:
> On Sat, Jul 14, 2018 at 07:40:43PM +0530, Kumar, Mahesh wrote:
>> Hi,
>>
>> Thanks for review.
>>
>>
>> On 7/13/2018 8:27 PM, Ville Syrjälä wrote:
>>> On Fri, Jul 13, 2018 at 07:41:24PM +0530, Mahesh Kumar wrote:
>>>> Memory with 16GB dimms require an increase of 1us in level-0 latency.
>>>> This patch implements the same.
>>>> Bspec: 4381
>>>>
>>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_drv.c | 35 +++++++++++++++++++++++++++++++++--
>>>>    drivers/gpu/drm/i915/i915_drv.h |  2 ++
>>>>    drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
>>>>    3 files changed, 48 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>>>> index b93194cbd820..c6d30653d70c 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>> @@ -1063,6 +1063,29 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>>>>    	intel_gvt_sanitize_options(dev_priv);
>>>>    }
>>>>    
>>>> +static void
>>>> +intel_memdev_is_16gb_dimm(struct memdev_info *memdev_info,
>>>> +			  u8 rank, u8 size, u8 width)
>>>> +{
>>>> +	bool found_16gb_dimm = false;
>>>> +
>>>> +	if (size == 16 && width == SKL_DRAM_WIDTH_X8 &&
>>>> +	    rank == SKL_DRAM_RANK_SINGLE)
>>>> +		found_16gb_dimm = true;
>>>> +	else if (size == 32 && width == SKL_DRAM_WIDTH_X8 &&
>>>> +		 rank == SKL_DRAM_RANK_DUAL)
>>>> +		found_16gb_dimm = true;
>>>> +	else if (size == 8 && width == SKL_DRAM_WIDTH_X16 &&
>>>> +		 rank == SKL_DRAM_RANK_SINGLE)
>>>> +		found_16gb_dimm = true;
>>>> +	else if (size == 16 && width == SKL_DRAM_WIDTH_X16 &&
>>>> +		 rank == SKL_DRAM_RANK_DUAL)
>>>> +		found_16gb_dimm = true;
>>>> +
>>>> +	if (!memdev_info->is_16gb_dimm)
>>>> +		memdev_info->is_16gb_dimm = found_16gb_dimm;
>>>> +}
>>> Pure functions are generally more fun. Would also make the function name
>>> match the implementation.
>> Are you suggesting to change the name to only "is_16gb_dimm" ?
> I'm suggesting making it pure.
ok sure, got it.

-Mahesh
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx