[Intel-gfx] [RFC 3/4] drm/i915/registers: deprecate _SHIFT in favor of FIELD_GET() and _MASK

Jani Nikula jani.nikula at intel.com
Thu Sep 27 09:40:22 UTC 2018


bitfield.h defines FIELD_GET() and FIELD_PREP() macros to access
bitfields using the mask alone, with no need for separate shift. Indeed,
the shift is redundant.

For the most part, FIELD_GET() is shorter than masking followed by
shift, and arguably has more clarity.

FIELD_PREP() can get more verbose than simply shifting in place, but it
does provide masking to ensure we don't overflow the mask, something we
usually don't bother with currently. For build time constants there's a
build error for this.

Signed-off-by: Jani Nikula <jani.nikula at intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  9 ---------
 drivers/gpu/drm/i915/intel_dp.c   | 41 ++++++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_lvds.c | 41 ++++++++++++++++++---------------------
 3 files changed, 36 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 893628c86a8d..52aa007c54c5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4625,7 +4625,6 @@ enum {
 #define   PP_SEQUENCE_NONE		(0 << 28)
 #define   PP_SEQUENCE_POWER_UP		(1 << 28)
 #define   PP_SEQUENCE_POWER_DOWN	(2 << 28)
-#define   PP_SEQUENCE_SHIFT		28
 #define   PP_CYCLE_DELAY_ACTIVE		BIT(27)
 #define   PP_SEQUENCE_STATE_MASK	GENMASK(3, 0)
 #define   PP_SEQUENCE_STATE_OFF_IDLE	(0x0 << 0)
@@ -4643,7 +4642,6 @@ enum {
 #define  PANEL_UNLOCK_MASK		GENMASK(31, 16)
 #define  PANEL_UNLOCK_REGS		(0xabcd << 16)
 #define  BXT_POWER_CYCLE_DELAY_MASK	GENMASK(8, 4)
-#define  BXT_POWER_CYCLE_DELAY_SHIFT	4
 #define  EDP_FORCE_VDD			BIT(3)
 #define  EDP_BLC_ENABLE			BIT(2)
 #define  PANEL_POWER_RESET		BIT(1)
@@ -4651,7 +4649,6 @@ enum {
 
 #define _PP_ON_DELAYS			0x61208
 #define PP_ON_DELAYS(pps_idx)		_MMIO_PPS(pps_idx, _PP_ON_DELAYS)
-#define  PANEL_PORT_SELECT_SHIFT	30
 #define  PANEL_PORT_SELECT_MASK		GENMASK(31, 30)
 #define  PANEL_PORT_SELECT_LVDS		(0 << 30)
 #define  PANEL_PORT_SELECT_DPA		(1 << 30)
@@ -4659,23 +4656,17 @@ enum {
 #define  PANEL_PORT_SELECT_DPD		(3 << 30)
 #define  PANEL_PORT_SELECT_VLV(port)	((port) << 30)
 #define  PANEL_POWER_UP_DELAY_MASK	GENMASK(28, 16)
-#define  PANEL_POWER_UP_DELAY_SHIFT	16
 #define  PANEL_LIGHT_ON_DELAY_MASK	GENMASK(12, 0)
-#define  PANEL_LIGHT_ON_DELAY_SHIFT	0
 
 #define _PP_OFF_DELAYS			0x6120C
 #define PP_OFF_DELAYS(pps_idx)		_MMIO_PPS(pps_idx, _PP_OFF_DELAYS)
 #define  PANEL_POWER_DOWN_DELAY_MASK	GENMASK(28, 16)
-#define  PANEL_POWER_DOWN_DELAY_SHIFT	16
 #define  PANEL_LIGHT_OFF_DELAY_MASK	GENMASK(12, 0)
-#define  PANEL_LIGHT_OFF_DELAY_SHIFT	0
 
 #define _PP_DIVISOR			0x61210
 #define PP_DIVISOR(pps_idx)		_MMIO_PPS(pps_idx, _PP_DIVISOR)
 #define  PP_REFERENCE_DIVIDER_MASK	GENMASK(31, 8)
-#define  PP_REFERENCE_DIVIDER_SHIFT	8
 #define  PANEL_POWER_CYCLE_DELAY_MASK	GENMASK(4, 0)
-#define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
 
 /* Panel fitting */
 #define PFIT_CONTROL	_MMIO(dev_priv->info.display_mmio_offset + 0x61230)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ed06876457b7..6f6c1e621387 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -31,6 +31,7 @@
 #include <linux/types.h>
 #include <linux/notifier.h>
 #include <linux/reboot.h>
+#include <linux/bitfield.h>
 #include <asm/byteorder.h>
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
@@ -5763,25 +5764,16 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, struct edp_power_seq *seq)
 	}
 
 	/* Pull timing values out of registers */
-	seq->t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
-		     PANEL_POWER_UP_DELAY_SHIFT;
-
-	seq->t8 = (pp_on & PANEL_LIGHT_ON_DELAY_MASK) >>
-		  PANEL_LIGHT_ON_DELAY_SHIFT;
-
-	seq->t9 = (pp_off & PANEL_LIGHT_OFF_DELAY_MASK) >>
-		  PANEL_LIGHT_OFF_DELAY_SHIFT;
-
-	seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
-		   PANEL_POWER_DOWN_DELAY_SHIFT;
+	seq->t1_t3 = FIELD_GET(PANEL_POWER_UP_DELAY_MASK, pp_on);
+	seq->t8 = FIELD_GET(PANEL_LIGHT_ON_DELAY_MASK, pp_on);
+	seq->t9 = FIELD_GET(PANEL_LIGHT_OFF_DELAY_MASK, pp_off);
+	seq->t10 = FIELD_GET(PANEL_POWER_DOWN_DELAY_MASK, pp_off);
 
 	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
 	    HAS_PCH_ICP(dev_priv)) {
-		seq->t11_t12 = ((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
-				BXT_POWER_CYCLE_DELAY_SHIFT) * 1000;
+		seq->t11_t12 = FIELD_GET(BXT_POWER_CYCLE_DELAY_MASK, pp_ctl) * 1000;
 	} else {
-		seq->t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
-		       PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
+		seq->t11_t12 = FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, pp_div) * 1000;
 	}
 }
 
@@ -5941,22 +5933,23 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
 		I915_WRITE(regs.pp_ctrl, pp);
 	}
 
-	pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) |
-		(seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT);
-	pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) |
-		 (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
+	pp_on = FIELD_PREP(PANEL_POWER_UP_DELAY_MASK, seq->t1_t3) |
+		FIELD_PREP(PANEL_LIGHT_ON_DELAY_MASK, seq->t8);
+	pp_off = FIELD_PREP(PANEL_LIGHT_OFF_DELAY_MASK, seq->t9) |
+		FIELD_PREP(PANEL_POWER_DOWN_DELAY_MASK, seq->t10);
 	/* Compute the divisor for the pp clock, simply match the Bspec
 	 * formula. */
 	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
 	    HAS_PCH_ICP(dev_priv)) {
 		pp_div = I915_READ(regs.pp_ctrl);
 		pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
-		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
-				<< BXT_POWER_CYCLE_DELAY_SHIFT);
+		pp_div |= FIELD_PREP(BXT_POWER_CYCLE_DELAY_MASK,
+				     DIV_ROUND_UP(seq->t11_t12, 1000));
 	} else {
-		pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
-		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
-				<< PANEL_POWER_CYCLE_DELAY_SHIFT);
+		pp_div = FIELD_PREP(PP_REFERENCE_DIVIDER_MASK,
+				    (100 * div) / 2 - 1);
+		pp_div |= FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK,
+				     DIV_ROUND_UP(seq->t11_t12, 1000));
 	}
 
 	/* Haswell doesn't have any port selection bits for the panel
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index f9f3b0885ba5..315eb2af29c8 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -28,6 +28,7 @@
  */
 
 #include <acpi/button.h>
+#include <linux/bitfield.h>
 #include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
@@ -160,24 +161,17 @@ static void intel_lvds_pps_get_hw_state(struct drm_i915_private *dev_priv,
 	pps->powerdown_on_reset = I915_READ(PP_CONTROL(0)) & PANEL_POWER_RESET;
 
 	val = I915_READ(PP_ON_DELAYS(0));
-	pps->port = (val & PANEL_PORT_SELECT_MASK) >>
-		    PANEL_PORT_SELECT_SHIFT;
-	pps->t1_t2 = (val & PANEL_POWER_UP_DELAY_MASK) >>
-		     PANEL_POWER_UP_DELAY_SHIFT;
-	pps->t5 = (val & PANEL_LIGHT_ON_DELAY_MASK) >>
-		  PANEL_LIGHT_ON_DELAY_SHIFT;
+	pps->port = FIELD_GET(PANEL_PORT_SELECT_MASK, val);
+	pps->t1_t2 = FIELD_GET(PANEL_POWER_UP_DELAY_MASK, val);
+	pps->t5 = FIELD_GET(PANEL_LIGHT_ON_DELAY_MASK, val);
 
 	val = I915_READ(PP_OFF_DELAYS(0));
-	pps->t3 = (val & PANEL_POWER_DOWN_DELAY_MASK) >>
-		  PANEL_POWER_DOWN_DELAY_SHIFT;
-	pps->tx = (val & PANEL_LIGHT_OFF_DELAY_MASK) >>
-		  PANEL_LIGHT_OFF_DELAY_SHIFT;
+	pps->t3 = FIELD_GET(PANEL_POWER_DOWN_DELAY_MASK, val);
+	pps->tx = FIELD_GET(PANEL_LIGHT_OFF_DELAY_MASK, val);
 
 	val = I915_READ(PP_DIVISOR(0));
-	pps->divider = (val & PP_REFERENCE_DIVIDER_MASK) >>
-		       PP_REFERENCE_DIVIDER_SHIFT;
-	val = (val & PANEL_POWER_CYCLE_DELAY_MASK) >>
-	      PANEL_POWER_CYCLE_DELAY_SHIFT;
+	pps->divider = FIELD_GET(PP_REFERENCE_DIVIDER_MASK, val);
+	val = FIELD_GET(PANEL_POWER_CYCLE_DELAY_MASK, val);
 	/*
 	 * Remove the BSpec specified +1 (100ms) offset that accounts for a
 	 * too short power-cycle delay due to the asynchronous programming of
@@ -217,15 +211,18 @@ static void intel_lvds_pps_init_hw(struct drm_i915_private *dev_priv,
 		val |= PANEL_POWER_RESET;
 	I915_WRITE(PP_CONTROL(0), val);
 
-	I915_WRITE(PP_ON_DELAYS(0), (pps->port << PANEL_PORT_SELECT_SHIFT) |
-				    (pps->t1_t2 << PANEL_POWER_UP_DELAY_SHIFT) |
-				    (pps->t5 << PANEL_LIGHT_ON_DELAY_SHIFT));
-	I915_WRITE(PP_OFF_DELAYS(0), (pps->t3 << PANEL_POWER_DOWN_DELAY_SHIFT) |
-				     (pps->tx << PANEL_LIGHT_OFF_DELAY_SHIFT));
+	val = FIELD_PREP(PANEL_PORT_SELECT_MASK, pps->port) |
+		FIELD_PREP(PANEL_POWER_UP_DELAY_MASK, pps->t1_t2) |
+		FIELD_PREP(PANEL_LIGHT_ON_DELAY_MASK, pps->t5);
+	I915_WRITE(PP_ON_DELAYS(0), val);
 
-	val = pps->divider << PP_REFERENCE_DIVIDER_SHIFT;
-	val |= (DIV_ROUND_UP(pps->t4, 1000) + 1) <<
-	       PANEL_POWER_CYCLE_DELAY_SHIFT;
+	val = FIELD_PREP(PANEL_POWER_DOWN_DELAY_MASK, pps->t3) |
+		FIELD_PREP(PANEL_LIGHT_OFF_DELAY_MASK, pps->tx);
+	I915_WRITE(PP_OFF_DELAYS(0), val);
+
+	val = FIELD_PREP(PP_REFERENCE_DIVIDER_MASK, pps->divider) |
+		FIELD_PREP(PANEL_POWER_CYCLE_DELAY_MASK,
+			   DIV_ROUND_UP(pps->t4, 1000) + 1);
 	I915_WRITE(PP_DIVISOR(0), val);
 }
 
-- 
2.11.0



More information about the Intel-gfx mailing list