Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c
@ 2020-02-25 11:15 Jani Nikula
  2020-02-25 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/dram: use intel_uncore_*() functions for register access Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jani Nikula @ 2020-02-25 11:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The DRAM related routines are pretty isolated from the rest of the
i915_drv.c, split it out to a separate file. Put the eDRAM stuff in the
same bag, and rename the visible functions to have intel_dram_
prefix. Do some benign whitespace fixes and dev_priv -> i915 conversions
while at it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/Makefile     |   1 +
 drivers/gpu/drm/i915/i915_drv.c   | 493 +-----------------------------
 drivers/gpu/drm/i915/intel_dram.c | 487 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dram.h |  14 +
 4 files changed, 505 insertions(+), 490 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_dram.c
 create mode 100644 drivers/gpu/drm/i915/intel_dram.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index bc28c31c4f78..7e473135e617 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -47,6 +47,7 @@ i915-y += i915_drv.o \
 	  i915_sysfs.o \
 	  i915_utils.o \
 	  intel_device_info.o \
+	  intel_dram.o \
 	  intel_memory_region.o \
 	  intel_pch.o \
 	  intel_pm.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dba5fe1391e8..7f0e0ba918e9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -80,6 +80,7 @@
 #include "i915_sysfs.h"
 #include "i915_trace.h"
 #include "i915_vgpu.h"
+#include "intel_dram.h"
 #include "intel_memory_region.h"
 #include "intel_pm.h"
 #include "vlv_suspend.h"
@@ -558,494 +559,6 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 	intel_gvt_sanitize_options(dev_priv);
 }
 
-#define DRAM_TYPE_STR(type) [INTEL_DRAM_ ## type] = #type
-
-static const char *intel_dram_type_str(enum intel_dram_type type)
-{
-	static const char * const str[] = {
-		DRAM_TYPE_STR(UNKNOWN),
-		DRAM_TYPE_STR(DDR3),
-		DRAM_TYPE_STR(DDR4),
-		DRAM_TYPE_STR(LPDDR3),
-		DRAM_TYPE_STR(LPDDR4),
-	};
-
-	if (type >= ARRAY_SIZE(str))
-		type = INTEL_DRAM_UNKNOWN;
-
-	return str[type];
-}
-
-#undef DRAM_TYPE_STR
-
-static int intel_dimm_num_devices(const struct dram_dimm_info *dimm)
-{
-	return dimm->ranks * 64 / (dimm->width ?: 1);
-}
-
-/* Returns total GB for the whole DIMM */
-static int skl_get_dimm_size(u16 val)
-{
-	return val & SKL_DRAM_SIZE_MASK;
-}
-
-static int skl_get_dimm_width(u16 val)
-{
-	if (skl_get_dimm_size(val) == 0)
-		return 0;
-
-	switch (val & SKL_DRAM_WIDTH_MASK) {
-	case SKL_DRAM_WIDTH_X8:
-	case SKL_DRAM_WIDTH_X16:
-	case SKL_DRAM_WIDTH_X32:
-		val = (val & SKL_DRAM_WIDTH_MASK) >> SKL_DRAM_WIDTH_SHIFT;
-		return 8 << val;
-	default:
-		MISSING_CASE(val);
-		return 0;
-	}
-}
-
-static int skl_get_dimm_ranks(u16 val)
-{
-	if (skl_get_dimm_size(val) == 0)
-		return 0;
-
-	val = (val & SKL_DRAM_RANK_MASK) >> SKL_DRAM_RANK_SHIFT;
-
-	return val + 1;
-}
-
-/* Returns total GB for the whole DIMM */
-static int cnl_get_dimm_size(u16 val)
-{
-	return (val & CNL_DRAM_SIZE_MASK) / 2;
-}
-
-static int cnl_get_dimm_width(u16 val)
-{
-	if (cnl_get_dimm_size(val) == 0)
-		return 0;
-
-	switch (val & CNL_DRAM_WIDTH_MASK) {
-	case CNL_DRAM_WIDTH_X8:
-	case CNL_DRAM_WIDTH_X16:
-	case CNL_DRAM_WIDTH_X32:
-		val = (val & CNL_DRAM_WIDTH_MASK) >> CNL_DRAM_WIDTH_SHIFT;
-		return 8 << val;
-	default:
-		MISSING_CASE(val);
-		return 0;
-	}
-}
-
-static int cnl_get_dimm_ranks(u16 val)
-{
-	if (cnl_get_dimm_size(val) == 0)
-		return 0;
-
-	val = (val & CNL_DRAM_RANK_MASK) >> CNL_DRAM_RANK_SHIFT;
-
-	return val + 1;
-}
-
-static bool
-skl_is_16gb_dimm(const struct dram_dimm_info *dimm)
-{
-	/* Convert total GB to Gb per DRAM device */
-	return 8 * dimm->size / (intel_dimm_num_devices(dimm) ?: 1) == 16;
-}
-
-static void
-skl_dram_get_dimm_info(struct drm_i915_private *dev_priv,
-		       struct dram_dimm_info *dimm,
-		       int channel, char dimm_name, u16 val)
-{
-	if (INTEL_GEN(dev_priv) >= 10) {
-		dimm->size = cnl_get_dimm_size(val);
-		dimm->width = cnl_get_dimm_width(val);
-		dimm->ranks = cnl_get_dimm_ranks(val);
-	} else {
-		dimm->size = skl_get_dimm_size(val);
-		dimm->width = skl_get_dimm_width(val);
-		dimm->ranks = skl_get_dimm_ranks(val);
-	}
-
-	drm_dbg_kms(&dev_priv->drm,
-		    "CH%u DIMM %c size: %u GB, width: X%u, ranks: %u, 16Gb DIMMs: %s\n",
-		    channel, dimm_name, dimm->size, dimm->width, dimm->ranks,
-		    yesno(skl_is_16gb_dimm(dimm)));
-}
-
-static int
-skl_dram_get_channel_info(struct drm_i915_private *dev_priv,
-			  struct dram_channel_info *ch,
-			  int channel, u32 val)
-{
-	skl_dram_get_dimm_info(dev_priv, &ch->dimm_l,
-			       channel, 'L', val & 0xffff);
-	skl_dram_get_dimm_info(dev_priv, &ch->dimm_s,
-			       channel, 'S', val >> 16);
-
-	if (ch->dimm_l.size == 0 && ch->dimm_s.size == 0) {
-		drm_dbg_kms(&dev_priv->drm, "CH%u not populated\n", channel);
-		return -EINVAL;
-	}
-
-	if (ch->dimm_l.ranks == 2 || ch->dimm_s.ranks == 2)
-		ch->ranks = 2;
-	else if (ch->dimm_l.ranks == 1 && ch->dimm_s.ranks == 1)
-		ch->ranks = 2;
-	else
-		ch->ranks = 1;
-
-	ch->is_16gb_dimm =
-		skl_is_16gb_dimm(&ch->dimm_l) ||
-		skl_is_16gb_dimm(&ch->dimm_s);
-
-	drm_dbg_kms(&dev_priv->drm, "CH%u ranks: %u, 16Gb DIMMs: %s\n",
-		    channel, ch->ranks, yesno(ch->is_16gb_dimm));
-
-	return 0;
-}
-
-static bool
-intel_is_dram_symmetric(const struct dram_channel_info *ch0,
-			const struct dram_channel_info *ch1)
-{
-	return !memcmp(ch0, ch1, sizeof(*ch0)) &&
-		(ch0->dimm_s.size == 0 ||
-		 !memcmp(&ch0->dimm_l, &ch0->dimm_s, sizeof(ch0->dimm_l)));
-}
-
-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(dev_priv, &ch0, 0, 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(dev_priv, &ch1, 1, val);
-	if (ret == 0)
-		dram_info->num_channels++;
-
-	if (dram_info->num_channels == 0) {
-		drm_info(&dev_priv->drm,
-			 "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.ranks == 1 || ch1.ranks == 1)
-		dram_info->ranks = 1;
-	else
-		dram_info->ranks = max(ch0.ranks, ch1.ranks);
-
-	if (dram_info->ranks == 0) {
-		drm_info(&dev_priv->drm,
-			 "couldn't get memory rank information\n");
-		return -EINVAL;
-	}
-
-	dram_info->is_16gb_dimm = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
-
-	dram_info->symmetric_memory = intel_is_dram_symmetric(&ch0, &ch1);
-
-	drm_dbg_kms(&dev_priv->drm, "Memory configuration is symmetric? %s\n",
-		    yesno(dram_info->symmetric_memory));
-	return 0;
-}
-
-static enum intel_dram_type
-skl_get_dram_type(struct drm_i915_private *dev_priv)
-{
-	u32 val;
-
-	val = I915_READ(SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN);
-
-	switch (val & SKL_DRAM_DDR_TYPE_MASK) {
-	case SKL_DRAM_DDR_TYPE_DDR3:
-		return INTEL_DRAM_DDR3;
-	case SKL_DRAM_DDR_TYPE_DDR4:
-		return INTEL_DRAM_DDR4;
-	case SKL_DRAM_DDR_TYPE_LPDDR3:
-		return INTEL_DRAM_LPDDR3;
-	case SKL_DRAM_DDR_TYPE_LPDDR4:
-		return INTEL_DRAM_LPDDR4;
-	default:
-		MISSING_CASE(val);
-		return INTEL_DRAM_UNKNOWN;
-	}
-}
-
-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;
-
-	dram_info->type = skl_get_dram_type(dev_priv);
-	drm_dbg_kms(&dev_priv->drm, "DRAM type: %s\n",
-		    intel_dram_type_str(dram_info->type));
-
-	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(&dev_priv->drm,
-			 "Couldn't get system memory bandwidth\n");
-		return -EINVAL;
-	}
-
-	dram_info->valid = true;
-	return 0;
-}
-
-/* Returns Gb per DRAM device */
-static int bxt_get_dimm_size(u32 val)
-{
-	switch (val & BXT_DRAM_SIZE_MASK) {
-	case BXT_DRAM_SIZE_4GBIT:
-		return 4;
-	case BXT_DRAM_SIZE_6GBIT:
-		return 6;
-	case BXT_DRAM_SIZE_8GBIT:
-		return 8;
-	case BXT_DRAM_SIZE_12GBIT:
-		return 12;
-	case BXT_DRAM_SIZE_16GBIT:
-		return 16;
-	default:
-		MISSING_CASE(val);
-		return 0;
-	}
-}
-
-static int bxt_get_dimm_width(u32 val)
-{
-	if (!bxt_get_dimm_size(val))
-		return 0;
-
-	val = (val & BXT_DRAM_WIDTH_MASK) >> BXT_DRAM_WIDTH_SHIFT;
-
-	return 8 << val;
-}
-
-static int bxt_get_dimm_ranks(u32 val)
-{
-	if (!bxt_get_dimm_size(val))
-		return 0;
-
-	switch (val & BXT_DRAM_RANK_MASK) {
-	case BXT_DRAM_RANK_SINGLE:
-		return 1;
-	case BXT_DRAM_RANK_DUAL:
-		return 2;
-	default:
-		MISSING_CASE(val);
-		return 0;
-	}
-}
-
-static enum intel_dram_type bxt_get_dimm_type(u32 val)
-{
-	if (!bxt_get_dimm_size(val))
-		return INTEL_DRAM_UNKNOWN;
-
-	switch (val & BXT_DRAM_TYPE_MASK) {
-	case BXT_DRAM_TYPE_DDR3:
-		return INTEL_DRAM_DDR3;
-	case BXT_DRAM_TYPE_LPDDR3:
-		return INTEL_DRAM_LPDDR3;
-	case BXT_DRAM_TYPE_DDR4:
-		return INTEL_DRAM_DDR4;
-	case BXT_DRAM_TYPE_LPDDR4:
-		return INTEL_DRAM_LPDDR4;
-	default:
-		MISSING_CASE(val);
-		return INTEL_DRAM_UNKNOWN;
-	}
-}
-
-static void bxt_get_dimm_info(struct dram_dimm_info *dimm,
-			      u32 val)
-{
-	dimm->width = bxt_get_dimm_width(val);
-	dimm->ranks = bxt_get_dimm_ranks(val);
-
-	/*
-	 * Size in register is Gb per DRAM device. Convert to total
-	 * GB to match the way we report this for non-LP platforms.
-	 */
-	dimm->size = bxt_get_dimm_size(val) * intel_dimm_num_devices(dimm) / 8;
-}
-
-static int
-bxt_get_dram_info(struct drm_i915_private *dev_priv)
-{
-	struct dram_info *dram_info = &dev_priv->dram_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_MASK;
-	num_active_channels = hweight32(dram_channels);
-
-	/* Each active bit represents 4-byte channel */
-	dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
-
-	if (dram_info->bandwidth_kbps == 0) {
-		drm_info(&dev_priv->drm,
-			 "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++) {
-		struct dram_dimm_info dimm;
-		enum intel_dram_type type;
-
-		val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
-		if (val == 0xFFFFFFFF)
-			continue;
-
-		dram_info->num_channels++;
-
-		bxt_get_dimm_info(&dimm, val);
-		type = bxt_get_dimm_type(val);
-
-		drm_WARN_ON(&dev_priv->drm, type != INTEL_DRAM_UNKNOWN &&
-			    dram_info->type != INTEL_DRAM_UNKNOWN &&
-			    dram_info->type != type);
-
-		drm_dbg_kms(&dev_priv->drm,
-			    "CH%u DIMM size: %u GB, width: X%u, ranks: %u, type: %s\n",
-			    i - BXT_D_CR_DRP0_DUNIT_START,
-			    dimm.size, dimm.width, dimm.ranks,
-			    intel_dram_type_str(type));
-
-		/*
-		 * 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 (dram_info->ranks == 0)
-			dram_info->ranks = dimm.ranks;
-		else if (dimm.ranks == 1)
-			dram_info->ranks = 1;
-
-		if (type != INTEL_DRAM_UNKNOWN)
-			dram_info->type = type;
-	}
-
-	if (dram_info->type == INTEL_DRAM_UNKNOWN ||
-	    dram_info->ranks == 0) {
-		drm_info(&dev_priv->drm, "couldn't get memory information\n");
-		return -EINVAL;
-	}
-
-	dram_info->valid = true;
-	return 0;
-}
-
-static void
-intel_get_dram_info(struct drm_i915_private *dev_priv)
-{
-	struct dram_info *dram_info = &dev_priv->dram_info;
-	int ret;
-
-	/*
-	 * Assume 16Gb DIMMs are present until proven otherwise.
-	 * This is only used for the level 0 watermark latency
-	 * w/a which does not apply to bxt/glk.
-	 */
-	dram_info->is_16gb_dimm = !IS_GEN9_LP(dev_priv);
-
-	if (INTEL_GEN(dev_priv) < 9 || !HAS_DISPLAY(dev_priv))
-		return;
-
-	if (IS_GEN9_LP(dev_priv))
-		ret = bxt_get_dram_info(dev_priv);
-	else
-		ret = skl_get_dram_info(dev_priv);
-	if (ret)
-		return;
-
-	drm_dbg_kms(&dev_priv->drm, "DRAM bandwidth: %u kBps, channels: %u\n",
-		    dram_info->bandwidth_kbps,
-		    dram_info->num_channels);
-
-	drm_dbg_kms(&dev_priv->drm, "DRAM ranks: %u, 16Gb DIMMs: %s\n",
-		    dram_info->ranks, yesno(dram_info->is_16gb_dimm));
-}
-
-static u32 gen9_edram_size_mb(struct drm_i915_private *dev_priv, u32 cap)
-{
-	static const u8 ways[8] = { 4, 8, 12, 16, 16, 16, 16, 16 };
-	static const u8 sets[4] = { 1, 1, 2, 2 };
-
-	return EDRAM_NUM_BANKS(cap) *
-		ways[EDRAM_WAYS_IDX(cap)] *
-		sets[EDRAM_SETS_IDX(cap)];
-}
-
-static void edram_detect(struct drm_i915_private *dev_priv)
-{
-	u32 edram_cap = 0;
-
-	if (!(IS_HASWELL(dev_priv) ||
-	      IS_BROADWELL(dev_priv) ||
-	      INTEL_GEN(dev_priv) >= 9))
-		return;
-
-	edram_cap = __raw_uncore_read32(&dev_priv->uncore, HSW_EDRAM_CAP);
-
-	/* NB: We can't write IDICR yet because we don't have gt funcs set up */
-
-	if (!(edram_cap & EDRAM_ENABLED))
-		return;
-
-	/*
-	 * The needed capability bits for size calculation are not there with
-	 * pre gen9 so return 128MB always.
-	 */
-	if (INTEL_GEN(dev_priv) < 9)
-		dev_priv->edram_size_mb = 128;
-	else
-		dev_priv->edram_size_mb =
-			gen9_edram_size_mb(dev_priv, edram_cap);
-
-	dev_info(dev_priv->drm.dev,
-		 "Found %uMB of eDRAM\n", dev_priv->edram_size_mb);
-}
-
 /**
  * i915_driver_hw_probe - setup state requiring device access
  * @dev_priv: device private
@@ -1089,7 +602,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
 	intel_sanitize_options(dev_priv);
 
 	/* needs to be done before ggtt probe */
-	edram_detect(dev_priv);
+	intel_dram_edram_detect(dev_priv);
 
 	i915_perf_init(dev_priv);
 
@@ -1191,7 +704,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
 	 * Fill the dram structure to get the system raw bandwidth and
 	 * dram info. This will be used for memory latency calculation.
 	 */
-	intel_get_dram_info(dev_priv);
+	intel_dram_detect(dev_priv);
 
 	intel_bw_init_hw(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
new file mode 100644
index 000000000000..cabe7b5de4e0
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -0,0 +1,487 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include "i915_drv.h"
+#include "intel_dram.h"
+
+#define DRAM_TYPE_STR(type) [INTEL_DRAM_ ## type] = #type
+
+static const char *intel_dram_type_str(enum intel_dram_type type)
+{
+	static const char * const str[] = {
+		DRAM_TYPE_STR(UNKNOWN),
+		DRAM_TYPE_STR(DDR3),
+		DRAM_TYPE_STR(DDR4),
+		DRAM_TYPE_STR(LPDDR3),
+		DRAM_TYPE_STR(LPDDR4),
+	};
+
+	if (type >= ARRAY_SIZE(str))
+		type = INTEL_DRAM_UNKNOWN;
+
+	return str[type];
+}
+
+#undef DRAM_TYPE_STR
+
+static int intel_dimm_num_devices(const struct dram_dimm_info *dimm)
+{
+	return dimm->ranks * 64 / (dimm->width ?: 1);
+}
+
+/* Returns total GB for the whole DIMM */
+static int skl_get_dimm_size(u16 val)
+{
+	return val & SKL_DRAM_SIZE_MASK;
+}
+
+static int skl_get_dimm_width(u16 val)
+{
+	if (skl_get_dimm_size(val) == 0)
+		return 0;
+
+	switch (val & SKL_DRAM_WIDTH_MASK) {
+	case SKL_DRAM_WIDTH_X8:
+	case SKL_DRAM_WIDTH_X16:
+	case SKL_DRAM_WIDTH_X32:
+		val = (val & SKL_DRAM_WIDTH_MASK) >> SKL_DRAM_WIDTH_SHIFT;
+		return 8 << val;
+	default:
+		MISSING_CASE(val);
+		return 0;
+	}
+}
+
+static int skl_get_dimm_ranks(u16 val)
+{
+	if (skl_get_dimm_size(val) == 0)
+		return 0;
+
+	val = (val & SKL_DRAM_RANK_MASK) >> SKL_DRAM_RANK_SHIFT;
+
+	return val + 1;
+}
+
+/* Returns total GB for the whole DIMM */
+static int cnl_get_dimm_size(u16 val)
+{
+	return (val & CNL_DRAM_SIZE_MASK) / 2;
+}
+
+static int cnl_get_dimm_width(u16 val)
+{
+	if (cnl_get_dimm_size(val) == 0)
+		return 0;
+
+	switch (val & CNL_DRAM_WIDTH_MASK) {
+	case CNL_DRAM_WIDTH_X8:
+	case CNL_DRAM_WIDTH_X16:
+	case CNL_DRAM_WIDTH_X32:
+		val = (val & CNL_DRAM_WIDTH_MASK) >> CNL_DRAM_WIDTH_SHIFT;
+		return 8 << val;
+	default:
+		MISSING_CASE(val);
+		return 0;
+	}
+}
+
+static int cnl_get_dimm_ranks(u16 val)
+{
+	if (cnl_get_dimm_size(val) == 0)
+		return 0;
+
+	val = (val & CNL_DRAM_RANK_MASK) >> CNL_DRAM_RANK_SHIFT;
+
+	return val + 1;
+}
+
+static bool
+skl_is_16gb_dimm(const struct dram_dimm_info *dimm)
+{
+	/* Convert total GB to Gb per DRAM device */
+	return 8 * dimm->size / (intel_dimm_num_devices(dimm) ?: 1) == 16;
+}
+
+static void
+skl_dram_get_dimm_info(struct drm_i915_private *i915,
+		       struct dram_dimm_info *dimm,
+		       int channel, char dimm_name, u16 val)
+{
+	if (INTEL_GEN(i915) >= 10) {
+		dimm->size = cnl_get_dimm_size(val);
+		dimm->width = cnl_get_dimm_width(val);
+		dimm->ranks = cnl_get_dimm_ranks(val);
+	} else {
+		dimm->size = skl_get_dimm_size(val);
+		dimm->width = skl_get_dimm_width(val);
+		dimm->ranks = skl_get_dimm_ranks(val);
+	}
+
+	drm_dbg_kms(&i915->drm,
+		    "CH%u DIMM %c size: %u GB, width: X%u, ranks: %u, 16Gb DIMMs: %s\n",
+		    channel, dimm_name, dimm->size, dimm->width, dimm->ranks,
+		    yesno(skl_is_16gb_dimm(dimm)));
+}
+
+static int
+skl_dram_get_channel_info(struct drm_i915_private *i915,
+			  struct dram_channel_info *ch,
+			  int channel, u32 val)
+{
+	skl_dram_get_dimm_info(i915, &ch->dimm_l,
+			       channel, 'L', val & 0xffff);
+	skl_dram_get_dimm_info(i915, &ch->dimm_s,
+			       channel, 'S', val >> 16);
+
+	if (ch->dimm_l.size == 0 && ch->dimm_s.size == 0) {
+		drm_dbg_kms(&i915->drm, "CH%u not populated\n", channel);
+		return -EINVAL;
+	}
+
+	if (ch->dimm_l.ranks == 2 || ch->dimm_s.ranks == 2)
+		ch->ranks = 2;
+	else if (ch->dimm_l.ranks == 1 && ch->dimm_s.ranks == 1)
+		ch->ranks = 2;
+	else
+		ch->ranks = 1;
+
+	ch->is_16gb_dimm = skl_is_16gb_dimm(&ch->dimm_l) ||
+		skl_is_16gb_dimm(&ch->dimm_s);
+
+	drm_dbg_kms(&i915->drm, "CH%u ranks: %u, 16Gb DIMMs: %s\n",
+		    channel, ch->ranks, yesno(ch->is_16gb_dimm));
+
+	return 0;
+}
+
+static bool
+intel_is_dram_symmetric(const struct dram_channel_info *ch0,
+			const struct dram_channel_info *ch1)
+{
+	return !memcmp(ch0, ch1, sizeof(*ch0)) &&
+		(ch0->dimm_s.size == 0 ||
+		 !memcmp(&ch0->dimm_l, &ch0->dimm_s, sizeof(ch0->dimm_l)));
+}
+
+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(dev_priv, &ch0, 0, 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(dev_priv, &ch1, 1, val);
+	if (ret == 0)
+		dram_info->num_channels++;
+
+	if (dram_info->num_channels == 0) {
+		drm_info(&dev_priv->drm, "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.ranks == 1 || ch1.ranks == 1)
+		dram_info->ranks = 1;
+	else
+		dram_info->ranks = max(ch0.ranks, ch1.ranks);
+
+	if (dram_info->ranks == 0) {
+		drm_info(&dev_priv->drm,
+			 "couldn't get memory rank information\n");
+		return -EINVAL;
+	}
+
+	dram_info->is_16gb_dimm = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
+
+	dram_info->symmetric_memory = intel_is_dram_symmetric(&ch0, &ch1);
+
+	drm_dbg_kms(&dev_priv->drm, "Memory configuration is symmetric? %s\n",
+		    yesno(dram_info->symmetric_memory));
+
+	return 0;
+}
+
+static enum intel_dram_type
+skl_get_dram_type(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	val = I915_READ(SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN);
+
+	switch (val & SKL_DRAM_DDR_TYPE_MASK) {
+	case SKL_DRAM_DDR_TYPE_DDR3:
+		return INTEL_DRAM_DDR3;
+	case SKL_DRAM_DDR_TYPE_DDR4:
+		return INTEL_DRAM_DDR4;
+	case SKL_DRAM_DDR_TYPE_LPDDR3:
+		return INTEL_DRAM_LPDDR3;
+	case SKL_DRAM_DDR_TYPE_LPDDR4:
+		return INTEL_DRAM_LPDDR4;
+	default:
+		MISSING_CASE(val);
+		return INTEL_DRAM_UNKNOWN;
+	}
+}
+
+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;
+
+	dram_info->type = skl_get_dram_type(dev_priv);
+	drm_dbg_kms(&dev_priv->drm, "DRAM type: %s\n",
+		    intel_dram_type_str(dram_info->type));
+
+	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(&dev_priv->drm,
+			 "Couldn't get system memory bandwidth\n");
+		return -EINVAL;
+	}
+
+	dram_info->valid = true;
+	return 0;
+}
+
+/* Returns Gb per DRAM device */
+static int bxt_get_dimm_size(u32 val)
+{
+	switch (val & BXT_DRAM_SIZE_MASK) {
+	case BXT_DRAM_SIZE_4GBIT:
+		return 4;
+	case BXT_DRAM_SIZE_6GBIT:
+		return 6;
+	case BXT_DRAM_SIZE_8GBIT:
+		return 8;
+	case BXT_DRAM_SIZE_12GBIT:
+		return 12;
+	case BXT_DRAM_SIZE_16GBIT:
+		return 16;
+	default:
+		MISSING_CASE(val);
+		return 0;
+	}
+}
+
+static int bxt_get_dimm_width(u32 val)
+{
+	if (!bxt_get_dimm_size(val))
+		return 0;
+
+	val = (val & BXT_DRAM_WIDTH_MASK) >> BXT_DRAM_WIDTH_SHIFT;
+
+	return 8 << val;
+}
+
+static int bxt_get_dimm_ranks(u32 val)
+{
+	if (!bxt_get_dimm_size(val))
+		return 0;
+
+	switch (val & BXT_DRAM_RANK_MASK) {
+	case BXT_DRAM_RANK_SINGLE:
+		return 1;
+	case BXT_DRAM_RANK_DUAL:
+		return 2;
+	default:
+		MISSING_CASE(val);
+		return 0;
+	}
+}
+
+static enum intel_dram_type bxt_get_dimm_type(u32 val)
+{
+	if (!bxt_get_dimm_size(val))
+		return INTEL_DRAM_UNKNOWN;
+
+	switch (val & BXT_DRAM_TYPE_MASK) {
+	case BXT_DRAM_TYPE_DDR3:
+		return INTEL_DRAM_DDR3;
+	case BXT_DRAM_TYPE_LPDDR3:
+		return INTEL_DRAM_LPDDR3;
+	case BXT_DRAM_TYPE_DDR4:
+		return INTEL_DRAM_DDR4;
+	case BXT_DRAM_TYPE_LPDDR4:
+		return INTEL_DRAM_LPDDR4;
+	default:
+		MISSING_CASE(val);
+		return INTEL_DRAM_UNKNOWN;
+	}
+}
+
+static void bxt_get_dimm_info(struct dram_dimm_info *dimm, u32 val)
+{
+	dimm->width = bxt_get_dimm_width(val);
+	dimm->ranks = bxt_get_dimm_ranks(val);
+
+	/*
+	 * Size in register is Gb per DRAM device. Convert to total
+	 * GB to match the way we report this for non-LP platforms.
+	 */
+	dimm->size = bxt_get_dimm_size(val) * intel_dimm_num_devices(dimm) / 8;
+}
+
+static int bxt_get_dram_info(struct drm_i915_private *dev_priv)
+{
+	struct dram_info *dram_info = &dev_priv->dram_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_MASK;
+	num_active_channels = hweight32(dram_channels);
+
+	/* Each active bit represents 4-byte channel */
+	dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
+
+	if (dram_info->bandwidth_kbps == 0) {
+		drm_info(&dev_priv->drm,
+			 "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++) {
+		struct dram_dimm_info dimm;
+		enum intel_dram_type type;
+
+		val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
+		if (val == 0xFFFFFFFF)
+			continue;
+
+		dram_info->num_channels++;
+
+		bxt_get_dimm_info(&dimm, val);
+		type = bxt_get_dimm_type(val);
+
+		drm_WARN_ON(&dev_priv->drm, type != INTEL_DRAM_UNKNOWN &&
+			    dram_info->type != INTEL_DRAM_UNKNOWN &&
+			    dram_info->type != type);
+
+		drm_dbg_kms(&dev_priv->drm,
+			    "CH%u DIMM size: %u GB, width: X%u, ranks: %u, type: %s\n",
+			    i - BXT_D_CR_DRP0_DUNIT_START,
+			    dimm.size, dimm.width, dimm.ranks,
+			    intel_dram_type_str(type));
+
+		/*
+		 * 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 (dram_info->ranks == 0)
+			dram_info->ranks = dimm.ranks;
+		else if (dimm.ranks == 1)
+			dram_info->ranks = 1;
+
+		if (type != INTEL_DRAM_UNKNOWN)
+			dram_info->type = type;
+	}
+
+	if (dram_info->type == INTEL_DRAM_UNKNOWN || dram_info->ranks == 0) {
+		drm_info(&dev_priv->drm, "couldn't get memory information\n");
+		return -EINVAL;
+	}
+
+	dram_info->valid = true;
+
+	return 0;
+}
+
+void intel_dram_detect(struct drm_i915_private *i915)
+{
+	struct dram_info *dram_info = &i915->dram_info;
+	int ret;
+
+	/*
+	 * Assume 16Gb DIMMs are present until proven otherwise.
+	 * This is only used for the level 0 watermark latency
+	 * w/a which does not apply to bxt/glk.
+	 */
+	dram_info->is_16gb_dimm = !IS_GEN9_LP(i915);
+
+	if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
+		return;
+
+	if (IS_GEN9_LP(i915))
+		ret = bxt_get_dram_info(i915);
+	else
+		ret = skl_get_dram_info(i915);
+	if (ret)
+		return;
+
+	drm_dbg_kms(&i915->drm, "DRAM bandwidth: %u kBps, channels: %u\n",
+		    dram_info->bandwidth_kbps, dram_info->num_channels);
+
+	drm_dbg_kms(&i915->drm, "DRAM ranks: %u, 16Gb DIMMs: %s\n",
+		    dram_info->ranks, yesno(dram_info->is_16gb_dimm));
+}
+
+static u32 gen9_edram_size_mb(struct drm_i915_private *i915, u32 cap)
+{
+	static const u8 ways[8] = { 4, 8, 12, 16, 16, 16, 16, 16 };
+	static const u8 sets[4] = { 1, 1, 2, 2 };
+
+	return EDRAM_NUM_BANKS(cap) *
+		ways[EDRAM_WAYS_IDX(cap)] *
+		sets[EDRAM_SETS_IDX(cap)];
+}
+
+void intel_dram_edram_detect(struct drm_i915_private *i915)
+{
+	u32 edram_cap = 0;
+
+	if (!(IS_HASWELL(i915) || IS_BROADWELL(i915) || INTEL_GEN(i915) >= 9))
+		return;
+
+	edram_cap = __raw_uncore_read32(&i915->uncore, HSW_EDRAM_CAP);
+
+	/* NB: We can't write IDICR yet because we don't have gt funcs set up */
+
+	if (!(edram_cap & EDRAM_ENABLED))
+		return;
+
+	/*
+	 * The needed capability bits for size calculation are not there with
+	 * pre gen9 so return 128MB always.
+	 */
+	if (INTEL_GEN(i915) < 9)
+		i915->edram_size_mb = 128;
+	else
+		i915->edram_size_mb = gen9_edram_size_mb(i915, edram_cap);
+
+	dev_info(i915->drm.dev,
+		 "Found %uMB of eDRAM\n", i915->edram_size_mb);
+}
diff --git a/drivers/gpu/drm/i915/intel_dram.h b/drivers/gpu/drm/i915/intel_dram.h
new file mode 100644
index 000000000000..4ba13c13162c
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dram.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef __INTEL_DRAM_H__
+#define __INTEL_DRAM_H__
+
+struct drm_i915_private;
+
+void intel_dram_edram_detect(struct drm_i915_private *i915);
+void intel_dram_detect(struct drm_i915_private *i915);
+
+#endif /* __INTEL_DRAM_H__ */
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Intel-gfx] [PATCH 2/3] drm/i915/dram: use intel_uncore_*() functions for register access
  2020-02-25 11:15 [Intel-gfx] [PATCH 1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Jani Nikula
@ 2020-02-25 11:15 ` Jani Nikula
  2020-02-25 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() " Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2020-02-25 11:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The implicit "dev_priv" local variable use has been a long-standing pain
point in the register access macros I915_READ(), I915_WRITE(),
POSTING_READ(), I915_READ_FW(), and I915_WRITE_FW().

Replace them with the corresponding uncore register accessors
intel_uncore_read(), intel_uncore_write(), intel_uncore_posting_read(),
intel_uncore_read_fw(), and intel_uncore_write_fw().

Rename dev_priv to i915 while at it.

No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dram.c | 57 ++++++++++++++++---------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index cabe7b5de4e0..9bb9dd724d3f 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -166,25 +166,27 @@ intel_is_dram_symmetric(const struct dram_channel_info *ch0,
 }
 
 static int
-skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
+skl_dram_get_channels_info(struct drm_i915_private *i915)
 {
-	struct dram_info *dram_info = &dev_priv->dram_info;
+	struct dram_info *dram_info = &i915->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(dev_priv, &ch0, 0, val);
+	val = intel_uncore_read(&i915->uncore,
+				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+	ret = skl_dram_get_channel_info(i915, &ch0, 0, 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(dev_priv, &ch1, 1, val);
+	val = intel_uncore_read(&i915->uncore,
+				SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+	ret = skl_dram_get_channel_info(i915, &ch1, 1, val);
 	if (ret == 0)
 		dram_info->num_channels++;
 
 	if (dram_info->num_channels == 0) {
-		drm_info(&dev_priv->drm, "Number of memory channels is zero\n");
+		drm_info(&i915->drm, "Number of memory channels is zero\n");
 		return -EINVAL;
 	}
 
@@ -199,8 +201,7 @@ skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
 		dram_info->ranks = max(ch0.ranks, ch1.ranks);
 
 	if (dram_info->ranks == 0) {
-		drm_info(&dev_priv->drm,
-			 "couldn't get memory rank information\n");
+		drm_info(&i915->drm, "couldn't get memory rank information\n");
 		return -EINVAL;
 	}
 
@@ -208,18 +209,19 @@ skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
 
 	dram_info->symmetric_memory = intel_is_dram_symmetric(&ch0, &ch1);
 
-	drm_dbg_kms(&dev_priv->drm, "Memory configuration is symmetric? %s\n",
+	drm_dbg_kms(&i915->drm, "Memory configuration is symmetric? %s\n",
 		    yesno(dram_info->symmetric_memory));
 
 	return 0;
 }
 
 static enum intel_dram_type
-skl_get_dram_type(struct drm_i915_private *dev_priv)
+skl_get_dram_type(struct drm_i915_private *i915)
 {
 	u32 val;
 
-	val = I915_READ(SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN);
+	val = intel_uncore_read(&i915->uncore,
+				SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN);
 
 	switch (val & SKL_DRAM_DDR_TYPE_MASK) {
 	case SKL_DRAM_DDR_TYPE_DDR3:
@@ -237,21 +239,22 @@ skl_get_dram_type(struct drm_i915_private *dev_priv)
 }
 
 static int
-skl_get_dram_info(struct drm_i915_private *dev_priv)
+skl_get_dram_info(struct drm_i915_private *i915)
 {
-	struct dram_info *dram_info = &dev_priv->dram_info;
+	struct dram_info *dram_info = &i915->dram_info;
 	u32 mem_freq_khz, val;
 	int ret;
 
-	dram_info->type = skl_get_dram_type(dev_priv);
-	drm_dbg_kms(&dev_priv->drm, "DRAM type: %s\n",
+	dram_info->type = skl_get_dram_type(i915);
+	drm_dbg_kms(&i915->drm, "DRAM type: %s\n",
 		    intel_dram_type_str(dram_info->type));
 
-	ret = skl_dram_get_channels_info(dev_priv);
+	ret = skl_dram_get_channels_info(i915);
 	if (ret)
 		return ret;
 
-	val = I915_READ(SKL_MC_BIOS_DATA_0_0_0_MCHBAR_PCU);
+	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);
 
@@ -259,7 +262,7 @@ skl_get_dram_info(struct drm_i915_private *dev_priv)
 		mem_freq_khz * 8;
 
 	if (dram_info->bandwidth_kbps == 0) {
-		drm_info(&dev_priv->drm,
+		drm_info(&i915->drm,
 			 "Couldn't get system memory bandwidth\n");
 		return -EINVAL;
 	}
@@ -346,15 +349,15 @@ static void bxt_get_dimm_info(struct dram_dimm_info *dimm, u32 val)
 	dimm->size = bxt_get_dimm_size(val) * intel_dimm_num_devices(dimm) / 8;
 }
 
-static int bxt_get_dram_info(struct drm_i915_private *dev_priv)
+static int bxt_get_dram_info(struct drm_i915_private *i915)
 {
-	struct dram_info *dram_info = &dev_priv->dram_info;
+	struct dram_info *dram_info = &i915->dram_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);
+	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);
 
@@ -365,7 +368,7 @@ static int bxt_get_dram_info(struct drm_i915_private *dev_priv)
 	dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
 
 	if (dram_info->bandwidth_kbps == 0) {
-		drm_info(&dev_priv->drm,
+		drm_info(&i915->drm,
 			 "Couldn't get system memory bandwidth\n");
 		return -EINVAL;
 	}
@@ -377,7 +380,7 @@ static int bxt_get_dram_info(struct drm_i915_private *dev_priv)
 		struct dram_dimm_info dimm;
 		enum intel_dram_type type;
 
-		val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
+		val = intel_uncore_read(&i915->uncore, BXT_D_CR_DRP0_DUNIT(i));
 		if (val == 0xFFFFFFFF)
 			continue;
 
@@ -386,11 +389,11 @@ static int bxt_get_dram_info(struct drm_i915_private *dev_priv)
 		bxt_get_dimm_info(&dimm, val);
 		type = bxt_get_dimm_type(val);
 
-		drm_WARN_ON(&dev_priv->drm, type != INTEL_DRAM_UNKNOWN &&
+		drm_WARN_ON(&i915->drm, type != INTEL_DRAM_UNKNOWN &&
 			    dram_info->type != INTEL_DRAM_UNKNOWN &&
 			    dram_info->type != type);
 
-		drm_dbg_kms(&dev_priv->drm,
+		drm_dbg_kms(&i915->drm,
 			    "CH%u DIMM size: %u GB, width: X%u, ranks: %u, type: %s\n",
 			    i - BXT_D_CR_DRP0_DUNIT_START,
 			    dimm.size, dimm.width, dimm.ranks,
@@ -411,7 +414,7 @@ static int bxt_get_dram_info(struct drm_i915_private *dev_priv)
 	}
 
 	if (dram_info->type == INTEL_DRAM_UNKNOWN || dram_info->ranks == 0) {
-		drm_info(&dev_priv->drm, "couldn't get memory information\n");
+		drm_info(&i915->drm, "couldn't get memory information\n");
 		return -EINVAL;
 	}
 
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() for register access
  2020-02-25 11:15 [Intel-gfx] [PATCH 1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Jani Nikula
  2020-02-25 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/dram: use intel_uncore_*() functions for register access Jani Nikula
@ 2020-02-25 11:15 ` Jani Nikula
  2020-02-25 11:46   ` Chris Wilson
  2020-02-25 13:17 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Patchwork
  2020-02-25 13:41 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  3 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2020-02-25 11:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The implicit "dev_priv" local variable use has been a long-standing pain
point in the register access macros I915_READ(), I915_WRITE(),
POSTING_READ(), I915_READ_FW(), and I915_WRITE_FW().

Replace the sole remaining I915_WRITE() in i915_drv.c with
intel_uncore_write(), although it might be better to keep the entire
file void of direct register access.

No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7f0e0ba918e9..57e2fc911dac 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -758,7 +758,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	 * when running inside a VM.
 	 */
 	if (intel_vgpu_active(dev_priv))
-		I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
+		intel_uncore_write(&dev_priv->uncore, vgtif_reg(display_ready),
+				   VGT_DRV_DISPLAY_READY);
 
 	/* Reveal our presence to userspace */
 	if (drm_dev_register(dev, 0) == 0) {
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() for register access
  2020-02-25 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() " Jani Nikula
@ 2020-02-25 11:46   ` Chris Wilson
  2020-02-27  7:20     ` Jani Nikula
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2020-02-25 11:46 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: jani.nikula

Quoting Jani Nikula (2020-02-25 11:15:09)
> The implicit "dev_priv" local variable use has been a long-standing pain
> point in the register access macros I915_READ(), I915_WRITE(),
> POSTING_READ(), I915_READ_FW(), and I915_WRITE_FW().
> 
> Replace the sole remaining I915_WRITE() in i915_drv.c with
> intel_uncore_write(), although it might be better to keep the entire
> file void of direct register access.
> 
> No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 7f0e0ba918e9..57e2fc911dac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -758,7 +758,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>          * when running inside a VM.
>          */
>         if (intel_vgpu_active(dev_priv))
> -               I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
> +               intel_uncore_write(&dev_priv->uncore, vgtif_reg(display_ready),
> +                                  VGT_DRV_DISPLAY_READY);

Bonus patch for intel_vgpu_register() ?

Series is
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c
  2020-02-25 11:15 [Intel-gfx] [PATCH 1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Jani Nikula
  2020-02-25 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/dram: use intel_uncore_*() functions for register access Jani Nikula
  2020-02-25 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() " Jani Nikula
@ 2020-02-25 13:17 ` Patchwork
  2020-02-25 13:41 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-02-25 13:17 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c
URL   : https://patchwork.freedesktop.org/series/73894/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a633c5989ab0 drm/i915: split out intel_dram.[ch] from i915_drv.c
-:552: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#552: 
new file mode 100644

-:565: ERROR:BRACKET_SPACE: space prohibited before open square bracket '['
#565: FILE: drivers/gpu/drm/i915/intel_dram.c:9:
+#define DRAM_TYPE_STR(type) [INTEL_DRAM_ ## type] = #type

total: 1 errors, 1 warnings, 0 checks, 1025 lines checked
744ac95da66a drm/i915/dram: use intel_uncore_*() functions for register access
9c8d12fe133b drm/i915/drv: use intel_uncore_write() for register access

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c
  2020-02-25 11:15 [Intel-gfx] [PATCH 1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Jani Nikula
                   ` (2 preceding siblings ...)
  2020-02-25 13:17 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Patchwork
@ 2020-02-25 13:41 ` Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-02-25 13:41 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c
URL   : https://patchwork.freedesktop.org/series/73894/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8004 -> Patchwork_16700
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-tgl-y:           [PASS][1] -> [FAIL][2] ([CI#94])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-tgl-y/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@kms_addfb_basic@addfb25-x-tiled-mismatch:
    - fi-tgl-y:           [PASS][3] -> [DMESG-WARN][4] ([CI#94] / [i915#402]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-tgl-y/igt@kms_addfb_basic@addfb25-x-tiled-mismatch.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-tgl-y/igt@kms_addfb_basic@addfb25-x-tiled-mismatch.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-icl-y:           [INCOMPLETE][5] ([i915#140]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-icl-y/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-icl-y/igt@i915_selftest@live_execlists.html

  * igt@kms_addfb_basic@addfb25-bad-modifier:
    - fi-tgl-y:           [DMESG-WARN][7] ([CI#94] / [i915#402]) -> [PASS][8] +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-tgl-y/igt@kms_addfb_basic@addfb25-bad-modifier.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-tgl-y/igt@kms_addfb_basic@addfb25-bad-modifier.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-cml-u2:          [FAIL][9] ([i915#217] / [i915#976]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][11] ([fdo#111096] / [i915#323]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Warnings ####

  * igt@i915_selftest@live_gt_lrc:
    - fi-tgl-y:           [DMESG-FAIL][13] ([CI#94] / [i915#1233]) -> [INCOMPLETE][14] ([CI#94] / [i915#1233])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8004/fi-tgl-y/igt@i915_selftest@live_gt_lrc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16700/fi-tgl-y/igt@i915_selftest@live_gt_lrc.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [i915#1233]: https://gitlab.freedesktop.org/drm/intel/issues/1233
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#976]: https://gitlab.freedesktop.org/drm/intel/issues/976


Participating hosts (43 -> 41)
------------------------------

  Additional (5): fi-ehl-1 fi-ilk-650 fi-snb-2520m fi-elk-e7500 fi-blb-e6850 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-byt-clapper fi-bsw-nick fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8004 -> Patchwork_16700

  CI-20190529: 20190529
  CI_DRM_8004: 1a2e0cce5af4a9ad9694995610ed64578ccc430f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5464: 8cf2f8684992052ab89de1cf328c418224c0c2a7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16700: 9c8d12fe133b783cc1744a638a8a8af762698080 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9c8d12fe133b drm/i915/drv: use intel_uncore_write() for register access
744ac95da66a drm/i915/dram: use intel_uncore_*() functions for register access
a633c5989ab0 drm/i915: split out intel_dram.[ch] from i915_drv.c

== Logs ==

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() for register access
  2020-02-25 11:46   ` Chris Wilson
@ 2020-02-27  7:20     ` Jani Nikula
  0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2020-02-27  7:20 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Tue, 25 Feb 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2020-02-25 11:15:09)
>> The implicit "dev_priv" local variable use has been a long-standing pain
>> point in the register access macros I915_READ(), I915_WRITE(),
>> POSTING_READ(), I915_READ_FW(), and I915_WRITE_FW().
>> 
>> Replace the sole remaining I915_WRITE() in i915_drv.c with
>> intel_uncore_write(), although it might be better to keep the entire
>> file void of direct register access.
>> 
>> No functional changes.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 7f0e0ba918e9..57e2fc911dac 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -758,7 +758,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>>          * when running inside a VM.
>>          */
>>         if (intel_vgpu_active(dev_priv))
>> -               I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
>> +               intel_uncore_write(&dev_priv->uncore, vgtif_reg(display_ready),
>> +                                  VGT_DRV_DISPLAY_READY);
>
> Bonus patch for intel_vgpu_register() ?

Sure!

> Series is
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks, pushed to dinq.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-02-27  7:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-25 11:15 [Intel-gfx] [PATCH 1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Jani Nikula
2020-02-25 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/dram: use intel_uncore_*() functions for register access Jani Nikula
2020-02-25 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/drv: use intel_uncore_write() " Jani Nikula
2020-02-25 11:46   ` Chris Wilson
2020-02-27  7:20     ` Jani Nikula
2020-02-25 13:17 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: split out intel_dram.[ch] from i915_drv.c Patchwork
2020-02-25 13:41 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox