drm/i915: Shrink ilk-bdw wm storage by using u16

PATCH New Ville Syrjala 3 participants 11 messages Dec 12, 2018 – Dec 13, 2018

Subsystems (3)

DRM drivers Intel i915 Intel GVT-g drivers (Intel GPU Virtualization)

lore

Share & tools

lore

Ville Syrjala

author

2018-12-12 19:38 UTC #0 lore raw

PATCH 1/6 drm/i915: Shrink ilk-bdw wm storage by using u16

From: Ville Syrjälä <[email protected]>

The maximum watermark value we can ever have on ilk-bdw is 11 bits. Thus we can safely store all of these values in u16.

Also toss in a few s/uint16_t/u16/ etc. while at it.

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h  |  16 ++---
 drivers/gpu/drm/i915/intel_drv.h |   2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 101 +++++++++++++++----------------
 3 files changed, 58 insertions(+), 61 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +8 -8

index e70707e79386..9264fd1b8662 100644
@@ -1031,17 +1031,17 @@ enum intel_ddb_partitioning {
 
 struct intel_wm_level {
 	bool enable;
-	uint32_t pri_val;
-	uint32_t spr_val;
-	uint32_t cur_val;
-	uint32_t fbc_val;
+	u16 pri_val;
+	u16 spr_val;
+	u16 cur_val;
+	u16 fbc_val;
 };
 
 struct ilk_wm_values {
-	uint32_t wm_pipe[3];
-	uint32_t wm_lp[3];
-	uint32_t wm_lp_spr[3];
-	uint32_t wm_linetime[3];
+	u32 wm_pipe[3];
+	u32 wm_lp[3];
+	u32 wm_lp_spr[3];
+	u32 wm_linetime[3];
 	bool enable_fbc_wm;
 	enum intel_ddb_partitioning partitioning;
 };

drivers/gpu/drm/i915/intel_drv.h +1 -1

index d08f08f607dd..09abc1035335 100644
@@ -630,7 +630,7 @@ struct intel_crtc_scaler_state {
 
 struct intel_pipe_wm {
 	struct intel_wm_level wm[5];
-	uint32_t linetime;
+	u32 linetime;
 	bool fbc_wm_enabled;
 	bool pipe_enabled;
 	bool sprites_enabled;

drivers/gpu/drm/i915/intel_pm.c +49 ~~-52~~

212 lines not on this page · raw message

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjala

author

2018-12-12 19:38 UTC #1 lore raw

PATCH 2/6 drm/i915: Rename ilk watermark structs/enums

From: Ville Syrjälä <[email protected]>

Rename all the watermark related structs/enums specific to ilk-bdw to have an ilk_ prefix rather than an intel_ prefix. Should make it less confusing for everyone when it's clear where these things get used.

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h  | 12 ++--
 drivers/gpu/drm/i915/intel_drv.h | 10 ++--
 drivers/gpu/drm/i915/intel_pm.c  | 96 ++++++++++++++++----------------
 3 files changed, 58 insertions(+), 60 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +6 -6

index 9264fd1b8662..95231f5f813d 100644
@@ -1024,12 +1024,12 @@ struct intel_vbt_data {
 	struct sdvo_device_mapping sdvo_mappings[2];
 };
 
-enum intel_ddb_partitioning {
-	INTEL_DDB_PART_1_2,
-	INTEL_DDB_PART_5_6, /* IVB+ */
+enum ilk_ddb_partitioning {
+	ILK_DDB_PART_1_2,
+	ILK_DDB_PART_5_6, /* IVB+ */
 };
 
-struct intel_wm_level {
+struct ilk_wm_level {
 	bool enable;
 	u16 pri_val;
 	u16 spr_val;

@@ -1043,7 +1043,7 @@ struct ilk_wm_values {
 	u32 wm_lp_spr[3];
 	u32 wm_linetime[3];
 	bool enable_fbc_wm;
-	enum intel_ddb_partitioning partitioning;
+	enum ilk_ddb_partitioning partitioning;
 };
 
 struct g4x_pipe_wm {

@@ -1195,7 +1195,7 @@ struct i915_virtual_gpu {
 };
 
 /* used in computing the new watermarks state */
-struct intel_wm_config {
+struct ilk_wm_config {
 	unsigned int num_pipes_active;
 	bool sprites_enabled;
 	bool sprites_scaled;

drivers/gpu/drm/i915/intel_drv.h +5 -5

index 09abc1035335..cc4d8d8382b6 100644
@@ -628,8 +628,8 @@ struct intel_crtc_scaler_state {
 /* Flag to get scanline using frame time stamps */
 #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
 
-struct intel_pipe_wm {
-	struct intel_wm_level wm[5];
+struct ilk_pipe_wm {
+	struct ilk_wm_level wm[5];
 	u32 linetime;
 	bool fbc_wm_enabled;
 	bool pipe_enabled;

@@ -693,13 +693,13 @@ struct intel_crtc_wm_state {
 			 * switching away from and the new
 			 * configuration we're switching to.
 			 */
-			struct intel_pipe_wm intermediate;
+			struct ilk_pipe_wm intermediate;
 
 			/*
 			 * Optimal watermarks, programmed post-vblank
 			 * when this state is committed.
 			 */
-			struct intel_pipe_wm optimal;
+			struct ilk_pipe_wm optimal;
 		} ilk;
 
 		struct {

@@ -982,7 +982,7 @@ struct intel_crtc {
 	struct {
 		/* watermarks currently being used  */
 		union {
-			struct intel_pipe_wm ilk;
+			struct ilk_pipe_wm ilk;
 			struct vlv_wm_state vlv;
 			struct g4x_wm_state g4x;
 		} active;

drivers/gpu/drm/i915/intel_pm.c +47 ~~-49~~

303 lines not on this page · raw message

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjala

author

2018-12-12 19:38 UTC #2 lore raw

PATCH 3/6 drm/i915: Stash away the original SSKPD latency values

In reply to Ville Syrjala

From: Ville Syrjälä <[email protected]>

On ILK-IVB we must write the latency value read from SSKPD into the latency field in the WM_LP registers. While bspec was never clear on how the punit (or whatever) interprets these values empirical evidence has shown that these are treated as a cookie rather than as a literal latency value. That is, if we write a value that we didn't get from SSKPD (just off by one is sufficient) the system no longer appears to enter the corresponding power saving state.

This was made much more obvious on HSW/BDW since there we longer write the latency value into the WM_LP registers, and rather we write the desired watermark level number (well, 2x the level number).

Since we allow the user to adjust the latency values via debugfs, and since we have some quirks where we adjust the values automagically, we must stash away the originals read from SSKPD for later use in the WM_LP registers.

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
 drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++----------
 2 files changed, 32 insertions(+), 11 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +6 -0

index 95231f5f813d..c0204802d9cd 100644
@@ -1778,6 +1778,12 @@ struct drm_i915_private {
 		 * (which we don't fully trust).
 		 */
 		bool distrust_bios_wm;
+
+		/*
+		 * The values we must write to the LP watermark
+		 * registers' latency field on ILK-BDW.
+		 */
+		u16 ilk_wm_lp_latency[5];
 	} wm;
 
 	struct dram_info {

drivers/gpu/drm/i915/intel_pm.c +26 ~~-11~~

index 6328f0f8aa88..d5ad84be769d 100644
@@ -3038,10 +3038,35 @@ static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
 	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
 }
 
+/* The value we need to program into the WM_LPx latency field */
+static u16 ilk_wm_lp_latency(struct drm_i915_private *dev_priv, int level)
+{
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		return 2 * level;
+	else
+		return dev_priv->wm.pri_latency[level];
+}
+
+static void ilk_setup_wm_lp_latency(struct drm_i915_private *dev_priv)
+{
+	int level, max_level = ilk_wm_max_level(dev_priv);
+
+	for (level = 1; level <= max_level; level++)
+		dev_priv->wm.ilk_wm_lp_latency[level] =
+			ilk_wm_lp_latency(dev_priv, level);
+}
+
 static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 {
 	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
 
+	/*
+	 * On ILK-IVB the values written to the LP watermark register
+	 * latency field must match SSKPD 100%. So do this before any
+	 * adjustments are made to the latency values we got from SSKPD.
+	 */
+	ilk_setup_wm_lp_latency(dev_priv);
+
 	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
 	       sizeof(dev_priv->wm.pri_latency));
 	memcpy(dev_priv->wm.cur_latency, dev_priv->wm.pri_latency,

@@ -3326,16 +3351,6 @@ static int ilk_wm_lp_to_level(int wm_lp, const struct ilk_pipe_wm *pipe_wm)
 	return wm_lp + (wm_lp >= 2 && pipe_wm->wm[4].enable);
 }
 
-/* The value we need to program into the WM_LPx latency field */
-static unsigned int ilk_wm_lp_latency(struct drm_i915_private *dev_priv,
-				      int level)
-{
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		return 2 * level;
-	else
-		return dev_priv->wm.pri_latency[level];
-}
-
 static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 				   const struct ilk_pipe_wm *merged,
 				   enum ilk_ddb_partitioning partitioning,

@@ -3360,7 +3375,7 @@ static void ilk_compute_wm_results(struct drm_i915_private *dev_priv,
 		 * disabled. Doing otherwise could cause underruns.
 		 */
 		results->wm_lp[wm_lp - 1] =
-			(ilk_wm_lp_latency(dev_priv, level) << WM1_LP_LATENCY_SHIFT) |
+			(dev_priv->wm.ilk_wm_lp_latency[level] << WM1_LP_LATENCY_SHIFT) |
 			(r->pri_val << WM1_LP_SR_SHIFT) |
 			r->cur_val;

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjala

author

2018-12-12 19:38 UTC #3 lore raw

PATCH 4/6 drm/i915: Remove gen6_check_mch_setup()

In reply to Ville Syrjala

From: Ville Syrjälä <[email protected]>

snb_wm_latency_quirk() already boosts up the latency values so the extra warning about the SSKPD value being insufficient is now redundant. Drop it.

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h |  2 --
 drivers/gpu/drm/i915/intel_pm.c | 14 --------------
 2 files changed, 16 deletions(-)

drivers/gpu/drm/i915/i915_reg.h +0 -2

index 0a7d60509ca7..9dce09ed47f7 100644
@@ -3538,8 +3538,6 @@ enum i915_power_well_id {
 
 /* snb MCH registers for priority tuning */
 #define MCH_SSKPD			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
-#define   MCH_SSKPD_WM0_MASK		0x3f
-#define   MCH_SSKPD_WM0_VAL		0xc
 
 #define MCH_SECP_NRG_STTS		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x592c)

drivers/gpu/drm/i915/intel_pm.c +0 ~~-14~~

index d5ad84be769d..1a9ad431efbd 100644
@@ -8612,16 +8612,6 @@ static void cpt_init_clock_gating(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void gen6_check_mch_setup(struct drm_i915_private *dev_priv)
-{
-	uint32_t tmp;
-
-	tmp = I915_READ(MCH_SSKPD);
-	if ((tmp & MCH_SSKPD_WM0_MASK) != MCH_SSKPD_WM0_VAL)
-		DRM_DEBUG_KMS("Wrong MCH_SSKPD value: 0x%08x This can cause underruns.\n",
-			      tmp);
-}
-
 static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
 {
 	uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;

@@ -8712,8 +8702,6 @@ static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
 	g4x_disable_trickle_feed(dev_priv);
 
 	cpt_init_clock_gating(dev_priv);
-
-	gen6_check_mch_setup(dev_priv);
 }
 
 static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)

@@ -9081,8 +9069,6 @@ static void ivb_init_clock_gating(struct drm_i915_private *dev_priv)
 
 	if (!HAS_PCH_NOP(dev_priv))
 		cpt_init_clock_gating(dev_priv);
-
-	gen6_check_mch_setup(dev_priv);
 }
 
 static void vlv_init_clock_gating(struct drm_i915_private *dev_priv)

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjala

author

2018-12-12 19:38 UTC #4 lore raw

PATCH 5/6 drm/i915: Clean up SSKPD defines

In reply to Ville Syrjala

From: Ville Syrjälä <[email protected]>

Give names to the HSW/BDW SSKPD mask/shift values, give and _SNB suffix to the SNB/IVB mask/shift values, and drop the bogus non-mirrored SSKPD register define.

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h | 26 +++++++++++++++++---------
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++----------
 2 files changed, 27 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/i915_reg.h +17 -9

index 9dce09ed47f7..ea9a664980a6 100644
@@ -3536,8 +3536,24 @@ enum i915_power_well_id {
 #define   MAD_DIMM_A_SIZE_SHIFT		0
 #define   MAD_DIMM_A_SIZE_MASK		(0xff << MAD_DIMM_A_SIZE_SHIFT)
 
-/* snb MCH registers for priority tuning */
 #define MCH_SSKPD			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
+#define SSKPD_WM0_SHIFT_SNB		0
+#define SSKPD_WM1_SHIFT_SNB		8
+#define SSKPD_WM2_SHIFT_SNB		16
+#define SSKPD_WM3_SHIFT_SNB		24
+#define SSKPD_WM_MASK_SNB		0x3f
+#define SSKPD_NEW_WM0_SHIFT_HSW		56
+#define SSKPD_NEW_WM0_MASK_HSW		0xff
+#define SSKPD_OLD_WM0_SHIFT_HSW		0
+#define SSKPD_OLD_WM0_MASK_HSW		0xf
+#define SSKPD_WM1_SHIFT_HSW		4
+#define SSKPD_WM1_MASK_HSW		0xff
+#define SSKPD_WM2_SHIFT_HSW		12
+#define SSKPD_WM2_MASK_HSW		0xff
+#define SSKPD_WM3_SHIFT_HSW		20
+#define SSKPD_WM3_MASK_HSW		0x1ff
+#define SSKPD_WM4_SHIFT_HSW		32
+#define SSKPD_WM4_MASK_HSW		0x1ff
 
 #define MCH_SECP_NRG_STTS		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x592c)

@@ -6016,14 +6032,6 @@ enum {
 #define  ILK_SRLT_MASK		0x3f
 
 
-/* the address where we get all kinds of latency value */
-#define SSKPD			_MMIO(0x5d10)
-#define SSKPD_WM_MASK		0x3f
-#define SSKPD_WM0_SHIFT		0
-#define SSKPD_WM1_SHIFT		8
-#define SSKPD_WM2_SHIFT		16
-#define SSKPD_WM3_SHIFT		24
-
 /*
  * The two pipe frame counter registers are not synchronized, so
  * reading a stable value is somewhat tricky. The following code

drivers/gpu/drm/i915/intel_pm.c +10 ~~-10~~

index 1a9ad431efbd..6ebde7bbac4e 100644
@@ -2889,20 +2889,20 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		uint64_t sskpd = I915_READ64(MCH_SSKPD);
 
-		wm[0] = (sskpd >> 56) & 0xFF;
+		wm[0] = (sskpd >> SSKPD_NEW_WM0_SHIFT_HSW) & SSKPD_NEW_WM0_MASK_HSW;
 		if (wm[0] == 0)
-			wm[0] = sskpd & 0xF;
-		wm[1] = (sskpd >> 4) & 0xFF;
-		wm[2] = (sskpd >> 12) & 0xFF;
-		wm[3] = (sskpd >> 20) & 0x1FF;
-		wm[4] = (sskpd >> 32) & 0x1FF;
+			wm[0] = (sskpd >> SSKPD_OLD_WM0_SHIFT_HSW) & SSKPD_OLD_WM0_MASK_HSW;
+		wm[1] = (sskpd >> SSKPD_WM1_SHIFT_HSW) & SSKPD_WM1_MASK_HSW;
+		wm[2] = (sskpd >> SSKPD_WM2_SHIFT_HSW) & SSKPD_WM2_MASK_HSW;
+		wm[3] = (sskpd >> SSKPD_WM3_SHIFT_HSW) & SSKPD_WM3_MASK_HSW;
+		wm[4] = (sskpd >> SSKPD_WM4_SHIFT_HSW) & SSKPD_WM4_MASK_HSW;
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		uint32_t sskpd = I915_READ(MCH_SSKPD);
 
-		wm[0] = (sskpd >> SSKPD_WM0_SHIFT) & SSKPD_WM_MASK;
-		wm[1] = (sskpd >> SSKPD_WM1_SHIFT) & SSKPD_WM_MASK;
-		wm[2] = (sskpd >> SSKPD_WM2_SHIFT) & SSKPD_WM_MASK;
-		wm[3] = (sskpd >> SSKPD_WM3_SHIFT) & SSKPD_WM_MASK;
+		wm[0] = (sskpd >> SSKPD_WM0_SHIFT_SNB) & SSKPD_WM_MASK_SNB;
+		wm[1] = (sskpd >> SSKPD_WM1_SHIFT_SNB) & SSKPD_WM_MASK_SNB;
+		wm[2] = (sskpd >> SSKPD_WM2_SHIFT_SNB) & SSKPD_WM_MASK_SNB;
+		wm[3] = (sskpd >> SSKPD_WM3_SHIFT_SNB) & SSKPD_WM_MASK_SNB;
 	} else if (INTEL_GEN(dev_priv) >= 5) {
 		uint32_t mltr = I915_READ(MLTR_ILK);

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjala

author

2018-12-12 19:38 UTC #5 lore raw

PATCH 6/6 drm/i915: Use _MMIO_PIPE3() for ilk+ WM0_PIPE registers

In reply to Ville Syrjala

From: Ville Syrjälä <[email protected]>

Remove the hand rolled array of WM0_PIPE register offsets and use the standard _MMIO_PIPE3() instead.

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h |  9 +++++----
 drivers/gpu/drm/i915/intel_pm.c | 13 ++++---------
 2 files changed, 9 insertions(+), 13 deletions(-)

drivers/gpu/drm/i915/i915_reg.h +5 -4

index ea9a664980a6..246e5e77e7c5 100644
@@ -5992,15 +5992,16 @@ enum {
 	_MMIO(_PLANE(plane, _PLANE_WM_TRANS_1(pipe), _PLANE_WM_TRANS_2(pipe)))
 
 /* define the Watermark register on Ironlake */
-#define WM0_PIPEA_ILK		_MMIO(0x45100)
+#define _WM0_PIPEA_ILK		0x45100
+#define _WM0_PIPEB_ILK		0x45104
+#define _WM0_PIPEC_IVB		0x45200
+#define WM0_PIPE_ILK(pipe)	_MMIO_PIPE3((pipe), _WM0_PIPEA_ILK, \
+					    _WM0_PIPEB_ILK, _WM0_PIPEC_IVB)
 #define  WM0_PIPE_PLANE_MASK	(0xffff << 16)
 #define  WM0_PIPE_PLANE_SHIFT	16
 #define  WM0_PIPE_SPRITE_MASK	(0xff << 8)
 #define  WM0_PIPE_SPRITE_SHIFT	8
 #define  WM0_PIPE_CURSOR_MASK	(0xff)
-
-#define WM0_PIPEB_ILK		_MMIO(0x45104)
-#define WM0_PIPEC_IVB		_MMIO(0x45200)
 #define WM1_LP_ILK		_MMIO(0x45108)
 #define  WM1_LP_SR_EN		(1 << 31)
 #define  WM1_LP_LATENCY_SHIFT	24

drivers/gpu/drm/i915/intel_pm.c +4 -9

index 6ebde7bbac4e..46f8c8728847 100644
@@ -3555,11 +3555,11 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 	_ilk_disable_lp_wm(dev_priv, dirty);
 
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
-		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
-		I915_WRITE(WM0_PIPEB_ILK, results->wm_pipe[1]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_C))
-		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
 		I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results->wm_linetime[0]);

@@ -5647,13 +5647,8 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
 	struct ilk_pipe_wm *active = &cstate->wm.ilk.optimal;
 	enum pipe pipe = crtc->pipe;
-	static const i915_reg_t wm0_pipe_reg[] = {
-		[PIPE_A] = WM0_PIPEA_ILK,
-		[PIPE_B] = WM0_PIPEB_ILK,
-		[PIPE_C] = WM0_PIPEC_IVB,
-	};
 
-	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
+	hw->wm_pipe[pipe] = I915_READ(WM0_PIPE_ILK(pipe));
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjala

author

2018-12-12 21:17 UTC #6 lore raw

PATCH v2 6/6 drm/i915: Use _MMIO_PIPE3() for ilk+ WM0_PIPE registers

From: Ville Syrjälä <[email protected]>

Remove the hand rolled array of WM0_PIPE register offsets and use the standard _MMIO_PIPE3() instead.

v2: Take care of gvt too

Signed-off-by Ville Syrjälä <[email protected]>

---
 drivers/gpu/drm/i915/gvt/handlers.c |  6 +++---
 drivers/gpu/drm/i915/i915_reg.h     |  9 +++++----
 drivers/gpu/drm/i915/intel_pm.c     | 13 ++++---------
 3 files changed, 12 insertions(+), 16 deletions(-)

drivers/gpu/drm/i915/gvt/handlers.c +3 -3

index b5475c91e2ef..2edab387221d 100644
@@ -2120,9 +2120,9 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
 	MMIO_D(PF_VSCALE(PIPE_C), D_ALL);
 	MMIO_D(PF_HSCALE(PIPE_C), D_ALL);
 
-	MMIO_D(WM0_PIPEA_ILK, D_ALL);
-	MMIO_D(WM0_PIPEB_ILK, D_ALL);
-	MMIO_D(WM0_PIPEC_IVB, D_ALL);
+	MMIO_D(WM0_PIPE_ILK(PIPE_A), D_ALL);
+	MMIO_D(WM0_PIPE_ILK(PIPE_B), D_ALL);
+	MMIO_D(WM0_PIPE_ILK(PIPE_C), D_ALL);
 	MMIO_D(WM1_LP_ILK, D_ALL);
 	MMIO_D(WM2_LP_ILK, D_ALL);
 	MMIO_D(WM3_LP_ILK, D_ALL);

drivers/gpu/drm/i915/i915_reg.h +5 -4

index ea9a664980a6..246e5e77e7c5 100644
@@ -5992,15 +5992,16 @@ enum {
 	_MMIO(_PLANE(plane, _PLANE_WM_TRANS_1(pipe), _PLANE_WM_TRANS_2(pipe)))
 
 /* define the Watermark register on Ironlake */
-#define WM0_PIPEA_ILK		_MMIO(0x45100)
+#define _WM0_PIPEA_ILK		0x45100
+#define _WM0_PIPEB_ILK		0x45104
+#define _WM0_PIPEC_IVB		0x45200
+#define WM0_PIPE_ILK(pipe)	_MMIO_PIPE3((pipe), _WM0_PIPEA_ILK, \
+					    _WM0_PIPEB_ILK, _WM0_PIPEC_IVB)
 #define  WM0_PIPE_PLANE_MASK	(0xffff << 16)
 #define  WM0_PIPE_PLANE_SHIFT	16
 #define  WM0_PIPE_SPRITE_MASK	(0xff << 8)
 #define  WM0_PIPE_SPRITE_SHIFT	8
 #define  WM0_PIPE_CURSOR_MASK	(0xff)
-
-#define WM0_PIPEB_ILK		_MMIO(0x45104)
-#define WM0_PIPEC_IVB		_MMIO(0x45200)
 #define WM1_LP_ILK		_MMIO(0x45108)
 #define  WM1_LP_SR_EN		(1 << 31)
 #define  WM1_LP_LATENCY_SHIFT	24

drivers/gpu/drm/i915/intel_pm.c +4 -9

index 6ebde7bbac4e..46f8c8728847 100644
@@ -3555,11 +3555,11 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 	_ilk_disable_lp_wm(dev_priv, dirty);
 
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
-		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
-		I915_WRITE(WM0_PIPEB_ILK, results->wm_pipe[1]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_C))
-		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
 		I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results->wm_linetime[0]);

@@ -5647,13 +5647,8 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
 	struct ilk_pipe_wm *active = &cstate->wm.ilk.optimal;
 	enum pipe pipe = crtc->pipe;
-	static const i915_reg_t wm0_pipe_reg[] = {
-		[PIPE_A] = WM0_PIPEA_ILK,
-		[PIPE_B] = WM0_PIPEB_ILK,
-		[PIPE_C] = WM0_PIPEC_IVB,
-	};
 
-	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
+	hw->wm_pipe[pipe] = I915_READ(WM0_PIPE_ILK(pipe));
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Lucas De Marchi

Reviewed-by on v2 6/6

2018-12-13 01:54 UTC #7 lore raw

Re: [PATCH v2 6/6] drm/i915: Use _MMIO_PIPE3() for ilk+ WM0_PIPE registers

Quoted 8 lines · Ville Syrjala

From: Ville Syrjälä <[email protected]>
Remove the hand rolled array of WM0_PIPE register offsets and use the standard _MMIO_PIPE3() instead.
v2: Take care of gvt too
Signed-off-by Ville Syrjälä <[email protected]>

Reviewed-by Lucas De Marchi <[email protected]>

Lucas De Marchi

Quoted 89 lines

---
 drivers/gpu/drm/i915/gvt/handlers.c |  6 +++---
 drivers/gpu/drm/i915/i915_reg.h     |  9 +++++----
 drivers/gpu/drm/i915/intel_pm.c     | 13 ++++---------
 3 files changed, 12 insertions(+), 16 deletions(-)

drivers/gpu/drm/i915/gvt/handlers.c +3 -3

index b5475c91e2ef..2edab387221d 100644
@@ -2120,9 +2120,9 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
 	MMIO_D(PF_VSCALE(PIPE_C), D_ALL);
 	MMIO_D(PF_HSCALE(PIPE_C), D_ALL);
 
-	MMIO_D(WM0_PIPEA_ILK, D_ALL);
-	MMIO_D(WM0_PIPEB_ILK, D_ALL);
-	MMIO_D(WM0_PIPEC_IVB, D_ALL);
+	MMIO_D(WM0_PIPE_ILK(PIPE_A), D_ALL);
+	MMIO_D(WM0_PIPE_ILK(PIPE_B), D_ALL);
+	MMIO_D(WM0_PIPE_ILK(PIPE_C), D_ALL);
 	MMIO_D(WM1_LP_ILK, D_ALL);
 	MMIO_D(WM2_LP_ILK, D_ALL);
 	MMIO_D(WM3_LP_ILK, D_ALL);

drivers/gpu/drm/i915/i915_reg.h +5 -4

index ea9a664980a6..246e5e77e7c5 100644
@@ -5992,15 +5992,16 @@ enum {
 	_MMIO(_PLANE(plane, _PLANE_WM_TRANS_1(pipe), _PLANE_WM_TRANS_2(pipe)))
 
 /* define the Watermark register on Ironlake */
-#define WM0_PIPEA_ILK		_MMIO(0x45100)
+#define _WM0_PIPEA_ILK		0x45100
+#define _WM0_PIPEB_ILK		0x45104
+#define _WM0_PIPEC_IVB		0x45200
+#define WM0_PIPE_ILK(pipe)	_MMIO_PIPE3((pipe), _WM0_PIPEA_ILK, \
+					    _WM0_PIPEB_ILK, _WM0_PIPEC_IVB)
 #define  WM0_PIPE_PLANE_MASK	(0xffff << 16)
 #define  WM0_PIPE_PLANE_SHIFT	16
 #define  WM0_PIPE_SPRITE_MASK	(0xff << 8)
 #define  WM0_PIPE_SPRITE_SHIFT	8
 #define  WM0_PIPE_CURSOR_MASK	(0xff)
-
-#define WM0_PIPEB_ILK		_MMIO(0x45104)
-#define WM0_PIPEC_IVB		_MMIO(0x45200)
 #define WM1_LP_ILK		_MMIO(0x45108)
 #define  WM1_LP_SR_EN		(1 << 31)
 #define  WM1_LP_LATENCY_SHIFT	24

drivers/gpu/drm/i915/intel_pm.c +4 -9

index 6ebde7bbac4e..46f8c8728847 100644
@@ -3555,11 +3555,11 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
 	_ilk_disable_lp_wm(dev_priv, dirty);
 
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
-		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_A), results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
-		I915_WRITE(WM0_PIPEB_ILK, results->wm_pipe[1]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_B), results->wm_pipe[1]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_C))
-		I915_WRITE(WM0_PIPEC_IVB, results->wm_pipe[2]);
+		I915_WRITE(WM0_PIPE_ILK(PIPE_C), results->wm_pipe[2]);
 
 	if (dirty & WM_DIRTY_LINETIME(PIPE_A))
 		I915_WRITE(PIPE_WM_LINETIME(PIPE_A), results->wm_linetime[0]);

@@ -5647,13 +5647,8 @@ static void ilk_pipe_wm_get_hw_state(struct intel_crtc *crtc)
 	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
 	struct ilk_pipe_wm *active = &cstate->wm.ilk.optimal;
 	enum pipe pipe = crtc->pipe;
-	static const i915_reg_t wm0_pipe_reg[] = {
-		[PIPE_A] = WM0_PIPEA_ILK,
-		[PIPE_B] = WM0_PIPEB_ILK,
-		[PIPE_C] = WM0_PIPEC_IVB,
-	};
 
-	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
+	hw->wm_pipe[pipe] = I915_READ(WM0_PIPE_ILK(pipe));
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));

Signature · 6 lines

2.18.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-12-12 19:58 UTC #8 lore raw

✗ Fi.CI.BAT: failure for series starting with [1/6] drm/i915: Shrink ilk-bdw wm storage by using u16

In reply to Ville Syrjala

== Series Details ==

Series: series starting with [1/6] drm/i915: Shrink ilk-bdw wm storage by using u16
URL : https://patchwork.freedesktop.org/series/53962/
State : failure

== Summary ==

CALL scripts/checksyscalls.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/i915/gvt/handlers.o
drivers/gpu/drm/i915/gvt/handlers.c: In function ‘init_generic_mmio_info’:
drivers/gpu/drm/i915/gvt/handlers.c:2123:9: error: ‘WM0_PIPEA_ILK’ undeclared (first use in this function); did you mean ‘_WM0_PIPEA_ILK’?

  MMIO_D(WM0_PIPEA_ILK, D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1767:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~

drivers/gpu/drm/i915/gvt/handlers.c:2123:2: note: in expansion of macro ‘MMIO_D’
MMIO_D(WM0_PIPEA_ILK, D_ALL);
^~~~~~
drivers/gpu/drm/i915/gvt/handlers.c:2123:9: note: each undeclared identifier is reported only once for each function it appears in

  MMIO_D(WM0_PIPEA_ILK, D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1767:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~

drivers/gpu/drm/i915/gvt/handlers.c:2123:2: note: in expansion of macro ‘MMIO_D’
MMIO_D(WM0_PIPEA_ILK, D_ALL);
^~~~~~
drivers/gpu/drm/i915/gvt/handlers.c:2124:9: error: ‘WM0_PIPEB_ILK’ undeclared (first use in this function); did you mean ‘WM0_PIPEA_ILK’?

  MMIO_D(WM0_PIPEB_ILK, D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1767:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~

drivers/gpu/drm/i915/gvt/handlers.c:2124:2: note: in expansion of macro ‘MMIO_D’
MMIO_D(WM0_PIPEB_ILK, D_ALL);
^~~~~~
drivers/gpu/drm/i915/gvt/handlers.c:2125:9: error: ‘WM0_PIPEC_IVB’ undeclared (first use in this function); did you mean ‘_WM0_PIPEC_IVB’?

  MMIO_D(WM0_PIPEC_IVB, D_ALL);
         ^
drivers/gpu/drm/i915/gvt/handlers.c:1767:48: note: in definition of macro ‘MMIO_F’
  ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
                                                ^~~

drivers/gpu/drm/i915/gvt/handlers.c:2125:2: note: in expansion of macro ‘MMIO_D’
MMIO_D(WM0_PIPEC_IVB, D_ALL);
^~~~~~
scripts/Makefile.build:291: recipe for target 'drivers/gpu/drm/i915/gvt/handlers.o' failed make[4]: *** [drivers/gpu/drm/i915/gvt/handlers.o] Error 1 scripts/Makefile.build:516: recipe for target 'drivers/gpu/drm/i915' failed make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:516: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:516: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1060: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-12-12 21:27 UTC #9 lore raw

✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915: Shrink ilk-bdw wm storage by using u16 (rev2)

In reply to Ville Syrjala

== Series Details ==

Series: series starting with [1/6] drm/i915: Shrink ilk-bdw wm storage by using u16 (rev2)
URL : https://patchwork.freedesktop.org/series/53962/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Shrink ilk-bdw wm storage by using u16
+drivers/gpu/drm/i915/intel_pm.c:2439:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2455:16: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:2734:35: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:2734:35: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:2735:35: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:2735:35: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:2736:35: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_pm.c:2736:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2731:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2731:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2732:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2732:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2733:35: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_pm.c:2733:35: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_pm.c:4852:30: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_pm.c:4852:30: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_pm.c:6615:24: warning: too many warnings
+drivers/gpu/drm/i915/intel_pm.c:4852:30: warning: too many warnings

Commit: drm/i915: Rename ilk watermark structs/enums -O:drivers/gpu/drm/i915/intel_pm.c:3208:33: warning: expression using sizeof(void) -O:drivers/gpu/drm/i915/intel_pm.c:3208:33: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_pm.c:3207:33: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_pm.c:3207:33: warning: expression using sizeof(void)

Commit: drm/i915: Stash away the original SSKPD latency values -drivers/gpu/drm/i915/selftests/../i915_drv.h:3561:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3567:16: warning: expression using sizeof(void)

Commit: drm/i915: Remove gen6_check_mch_setup()
Okay!

Commit: drm/i915: Clean up SSKPD defines
Okay!

Commit: drm/i915: Use _MMIO_PIPE3() for ilk+ WM0_PIPE registers Okay!

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-12-12 21:46 UTC #10 lore raw

✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915: Shrink ilk-bdw wm storage by using u16 (rev2)

In reply to Ville Syrjala

== Series Details ==

Series: series starting with [1/6] drm/i915: Shrink ilk-bdw wm storage by using u16 (rev2)
URL : https://patchwork.freedesktop.org/series/53962/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5308 -> Patchwork_11080
====================================================

Summary
-------

**WARNING**

Minor unknown changes coming with Patchwork_11080 need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in Patchwork_11080, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

External URL: https://patchwork.freedesktop.org/api/1.0/series/53962/revisions/2/mbox/

Possible new issues
-------------------

Here are the unknown changes that may have been introduced in Patchwork_11080:

### IGT changes ###

#### Warnings ####

* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
- fi-kbl-7567u: PASS -> SKIP +33

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

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

### IGT changes ###

#### Issues hit ####

* igt@amdgpu/amd_basic@cs-compute:
- fi-kbl-8809g: NOTRUN -> FAIL [fdo#108094]

* igt@amdgpu/amd_prime@amd-to-i915:
- fi-kbl-8809g: NOTRUN -> FAIL [fdo#107341]

* igt@gem_exec_suspend@basic-s4-devices:
- fi-kbl-7500u: PASS -> DMESG-WARN [fdo#105128] / [fdo#107139]
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]

* igt@kms_pipe_crc_basic@read-crc-pipe-a:
- fi-byt-clapper: PASS -> FAIL [fdo#107362]

* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-skl-guc: PASS -> FAIL [fdo#103191] / [fdo#107362]

* igt@prime_vgem@basic-fence-flip:
- fi-ilk-650: PASS -> FAIL [fdo#104008]

* {igt@runner@aborted}:
- fi-icl-y: NOTRUN -> FAIL [fdo#108070]

#### Possible fixes ####

* igt@amdgpu/amd_basic@userptr:
- fi-kbl-8809g: DMESG-WARN [fdo#108965] -> PASS

* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: FAIL [fdo#108767] -> PASS

{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).

Participating hosts (48 -> 44)
------------------------------

Additional (1): fi-icl-y
Missing (5): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-ctg-p8600

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

* Linux: CI_DRM_5308 -> Patchwork_11080

CI_DRM_5308: e7cbffbd8fd1b6d713128ceb891d7d6205390ee4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4746: 2c793666d8c8328733f5769b16ae5858fee97f3f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11080: f542917d1f7c20c1fee9535302982f1ff7038b13 @ git://anongit.freedesktop.org/gfx-ci/linux

== Linux commits ==

f542917d1f7c drm/i915: Use _MMIO_PIPE3() for ilk+ WM0_PIPE registers a65c8a2538bb drm/i915: Clean up SSKPD defines 15479c03167b drm/i915: Remove gen6_check_mch_setup() 436a44ebac02 drm/i915: Stash away the original SSKPD latency values 633566de8c23 drm/i915: Rename ilk watermark structs/enums 4552b667487e drm/i915: Shrink ilk-bdw wm storage by using u16

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11080/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx