[RFC 0/4] drm/i915/registers: use standard bits.h and bitfield.h macros

[RFC 0/4] drm/i915/registers: use standard bits.h and bitfield.h macros

[Jani Nikula]

This is an RFC to get input on how people feel about moving towards
using <linux/bits.h> and <linux/bitfield.h> macros for register field
definitions and manipulation:

* BIT()
* GENMASK()
* FIELD_GET()
* FIELD_PREP()

I'm not necessarily proposing pushing the patches in this series;
they're more of a piece-by-piece transformation of the power sequencer
macros and code to use the above macros, to give an idea what the end
result would look like.

I'm also not proposing mass conversions that are error prone and cause
unnecessary conflicts. At least not outside of i915_reg.h.

I'd like to figure out the *direction* where we'd like to go with what
we're adding in the future.


BR,
Jani.


Jani Nikula (4):
  drm/i915/registers: prefer GENMASK() over hand rolled masks
  drm/i915/registers: prefer BIT() for single bits
  drm/i915/registers: deprecate _SHIFT in favor of FIELD_GET() and _MASK
  drm/i915/registers: define field values using FIELD_PREP()

 drivers/gpu/drm/i915/i915_reg.h   | 82 +++++++++++++++++----------------------
 drivers/gpu/drm/i915/intel_dp.c   | 43 +++++++++-----------
 drivers/gpu/drm/i915/intel_lvds.c | 41 +++++++++-----------
 3 files changed, 73 insertions(+), 93 deletions(-)

-- 
2.11.0

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

[RFC 1/4] drm/i915/registers: prefer GENMASK() over hand rolled masks

[Jani Nikula]

GENMASK() is much easier to get right and review against the specs than
hand rolled masks.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 27e650fe591b..25efce1eded7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4621,13 +4621,13 @@ enum {
  * - LVDS/DVOB/DVOC on
  */
 #define   PP_READY			(1 << 30)
+#define   PP_SEQUENCE_MASK		GENMASK(29, 28)
 #define   PP_SEQUENCE_NONE		(0 << 28)
 #define   PP_SEQUENCE_POWER_UP		(1 << 28)
 #define   PP_SEQUENCE_POWER_DOWN	(2 << 28)
-#define   PP_SEQUENCE_MASK		(3 << 28)
 #define   PP_SEQUENCE_SHIFT		28
 #define   PP_CYCLE_DELAY_ACTIVE		(1 << 27)
-#define   PP_SEQUENCE_STATE_MASK	0x0000000f
+#define   PP_SEQUENCE_STATE_MASK	GENMASK(3, 0)
 #define   PP_SEQUENCE_STATE_OFF_IDLE	(0x0 << 0)
 #define   PP_SEQUENCE_STATE_OFF_S0_1	(0x1 << 0)
 #define   PP_SEQUENCE_STATE_OFF_S0_2	(0x2 << 0)
@@ -4640,9 +4640,9 @@ enum {
 
 #define _PP_CONTROL			0x61204
 #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
+#define  PANEL_UNLOCK_MASK		GENMASK(31, 16)
 #define  PANEL_UNLOCK_REGS		(0xabcd << 16)
-#define  PANEL_UNLOCK_MASK		(0xffff << 16)
-#define  BXT_POWER_CYCLE_DELAY_MASK	0x1f0
+#define  BXT_POWER_CYCLE_DELAY_MASK	GENMASK(8, 4)
 #define  BXT_POWER_CYCLE_DELAY_SHIFT	4
 #define  EDP_FORCE_VDD			(1 << 3)
 #define  EDP_BLC_ENABLE			(1 << 2)
@@ -4653,29 +4653,29 @@ 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		(3 << 30)
+#define  PANEL_PORT_SELECT_MASK		GENMASK(31, 30)
 #define  PANEL_PORT_SELECT_LVDS		(0 << 30)
 #define  PANEL_PORT_SELECT_DPA		(1 << 30)
 #define  PANEL_PORT_SELECT_DPC		(2 << 30)
 #define  PANEL_PORT_SELECT_DPD		(3 << 30)
 #define  PANEL_PORT_SELECT_VLV(port)	((port) << 30)
-#define  PANEL_POWER_UP_DELAY_MASK	0x1fff0000
+#define  PANEL_POWER_UP_DELAY_MASK	GENMASK(28, 16)
 #define  PANEL_POWER_UP_DELAY_SHIFT	16
-#define  PANEL_LIGHT_ON_DELAY_MASK	0x1fff
+#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	0x1fff0000
+#define  PANEL_POWER_DOWN_DELAY_MASK	GENMASK(28, 16)
 #define  PANEL_POWER_DOWN_DELAY_SHIFT	16
-#define  PANEL_LIGHT_OFF_DELAY_MASK	0x1fff
+#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	0xffffff00
+#define  PP_REFERENCE_DIVIDER_MASK	GENMASK(31, 8)
 #define  PP_REFERENCE_DIVIDER_SHIFT	8
-#define  PANEL_POWER_CYCLE_DELAY_MASK	0x1f
+#define  PANEL_POWER_CYCLE_DELAY_MASK	GENMASK(4, 0)
 #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
 
 /* Panel fitting */
-- 
2.11.0

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

[RFC 2/4] drm/i915/registers: prefer BIT() for single bits

[Jani Nikula]

BIT() is the preferred way of defining bits in the kernel.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 15 +++++++--------
 drivers/gpu/drm/i915/intel_dp.c |  2 +-
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 25efce1eded7..893628c86a8d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4612,7 +4612,7 @@ enum {
 
 #define _PP_STATUS			0x61200
 #define PP_STATUS(pps_idx)		_MMIO_PPS(pps_idx, _PP_STATUS)
-#define   PP_ON				(1 << 31)
+#define   PP_ON				BIT(31)
 /*
  * Indicates that all dependencies of the panel are on:
  *
@@ -4620,13 +4620,13 @@ enum {
  * - pipe enabled
  * - LVDS/DVOB/DVOC on
  */
-#define   PP_READY			(1 << 30)
+#define   PP_READY			BIT(30)
 #define   PP_SEQUENCE_MASK		GENMASK(29, 28)
 #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		(1 << 27)
+#define   PP_CYCLE_DELAY_ACTIVE		BIT(27)
 #define   PP_SEQUENCE_STATE_MASK	GENMASK(3, 0)
 #define   PP_SEQUENCE_STATE_OFF_IDLE	(0x0 << 0)
 #define   PP_SEQUENCE_STATE_OFF_S0_1	(0x1 << 0)
@@ -4644,11 +4644,10 @@ enum {
 #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			(1 << 3)
-#define  EDP_BLC_ENABLE			(1 << 2)
-#define  PANEL_POWER_RESET		(1 << 1)
-#define  PANEL_POWER_OFF		(0 << 0)
-#define  PANEL_POWER_ON			(1 << 0)
+#define  EDP_FORCE_VDD			BIT(3)
+#define  EDP_BLC_ENABLE			BIT(2)
+#define  PANEL_POWER_RESET		BIT(1)
+#define  PANEL_POWER_ON			BIT(0)
 
 #define _PP_ON_DELAYS			0x61208
 #define PP_ON_DELAYS(pps_idx)		_MMIO_PPS(pps_idx, _PP_ON_DELAYS)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6b4c19123f2a..ed06876457b7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1040,7 +1040,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
 
 		/* 0x1F write to PP_DIV_REG sets max cycle delay */
 		I915_WRITE(pp_div_reg, pp_div | 0x1F);
-		I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+		I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS);
 		msleep(intel_dp->panel_power_cycle_delay);
 	}
 
-- 
2.11.0

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

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

[Jani Nikula]

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@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

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

[RFC 4/4] drm/i915/registers: define field values using FIELD_PREP()

[Jani Nikula]

Slightly verbose, but does away with hand rolled shifts and provides
static checking that the values fit the mask.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 52aa007c54c5..d1fc08038e54 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4622,25 +4622,25 @@ enum {
  */
 #define   PP_READY			BIT(30)
 #define   PP_SEQUENCE_MASK		GENMASK(29, 28)
-#define   PP_SEQUENCE_NONE		(0 << 28)
-#define   PP_SEQUENCE_POWER_UP		(1 << 28)
-#define   PP_SEQUENCE_POWER_DOWN	(2 << 28)
+#define   PP_SEQUENCE_NONE		FIELD_PREP(PP_SEQUENCE_MASK, 0)
+#define   PP_SEQUENCE_POWER_UP		FIELD_PREP(PP_SEQUENCE_MASK, 1)
+#define   PP_SEQUENCE_POWER_DOWN	FIELD_PREP(PP_SEQUENCE_MASK, 2)
 #define   PP_CYCLE_DELAY_ACTIVE		BIT(27)
 #define   PP_SEQUENCE_STATE_MASK	GENMASK(3, 0)
-#define   PP_SEQUENCE_STATE_OFF_IDLE	(0x0 << 0)
-#define   PP_SEQUENCE_STATE_OFF_S0_1	(0x1 << 0)
-#define   PP_SEQUENCE_STATE_OFF_S0_2	(0x2 << 0)
-#define   PP_SEQUENCE_STATE_OFF_S0_3	(0x3 << 0)
-#define   PP_SEQUENCE_STATE_ON_IDLE	(0x8 << 0)
-#define   PP_SEQUENCE_STATE_ON_S1_0	(0x9 << 0)
-#define   PP_SEQUENCE_STATE_ON_S1_2	(0xa << 0)
-#define   PP_SEQUENCE_STATE_ON_S1_3	(0xb << 0)
-#define   PP_SEQUENCE_STATE_RESET	(0xf << 0)
+#define   PP_SEQUENCE_STATE_OFF_IDLE	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0x0)
+#define   PP_SEQUENCE_STATE_OFF_S0_1	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0x1)
+#define   PP_SEQUENCE_STATE_OFF_S0_2	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0x2)
+#define   PP_SEQUENCE_STATE_OFF_S0_3	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0x3)
+#define   PP_SEQUENCE_STATE_ON_IDLE	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0x8)
+#define   PP_SEQUENCE_STATE_ON_S1_0	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0x9)
+#define   PP_SEQUENCE_STATE_ON_S1_2	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0xa)
+#define   PP_SEQUENCE_STATE_ON_S1_3	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0xb)
+#define   PP_SEQUENCE_STATE_RESET	FIELD_PREP(PP_SEQUENCE_STATE_MASK, 0xf)
 
 #define _PP_CONTROL			0x61204
 #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
 #define  PANEL_UNLOCK_MASK		GENMASK(31, 16)
-#define  PANEL_UNLOCK_REGS		(0xabcd << 16)
+#define  PANEL_UNLOCK_REGS		FIELD_PREP(PANEL_UNLOCK_MASK, 0xabcd)
 #define  BXT_POWER_CYCLE_DELAY_MASK	GENMASK(8, 4)
 #define  EDP_FORCE_VDD			BIT(3)
 #define  EDP_BLC_ENABLE			BIT(2)
@@ -4650,11 +4650,11 @@ enum {
 #define _PP_ON_DELAYS			0x61208
 #define PP_ON_DELAYS(pps_idx)		_MMIO_PPS(pps_idx, _PP_ON_DELAYS)
 #define  PANEL_PORT_SELECT_MASK		GENMASK(31, 30)
-#define  PANEL_PORT_SELECT_LVDS		(0 << 30)
-#define  PANEL_PORT_SELECT_DPA		(1 << 30)
-#define  PANEL_PORT_SELECT_DPC		(2 << 30)
-#define  PANEL_PORT_SELECT_DPD		(3 << 30)
-#define  PANEL_PORT_SELECT_VLV(port)	((port) << 30)
+#define  PANEL_PORT_SELECT_LVDS		FIELD_PREP(PANEL_PORT_SELECT_MASK, 0)
+#define  PANEL_PORT_SELECT_DPA		FIELD_PREP(PANEL_PORT_SELECT_MASK, 1)
+#define  PANEL_PORT_SELECT_DPC		FIELD_PREP(PANEL_PORT_SELECT_MASK, 2)
+#define  PANEL_PORT_SELECT_DPD		FIELD_PREP(PANEL_PORT_SELECT_MASK, 3)
+#define  PANEL_PORT_SELECT_VLV(port)	FIELD_PREP(PANEL_PORT_SELECT_MASK, port)
 #define  PANEL_POWER_UP_DELAY_MASK	GENMASK(28, 16)
 #define  PANEL_LIGHT_ON_DELAY_MASK	GENMASK(12, 0)
 
-- 
2.11.0

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

Re: [RFC 0/4] drm/i915/registers: use standard bits.h and bitfield.h macros

[Jani Nikula]

On Thu, 27 Sep 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> I'm not necessarily proposing pushing the patches in this series;
> they're more of a piece-by-piece transformation of the power sequencer
> macros and code to use the above macros, to give an idea what the end
> result would look like.

And if it isn't clear from the above, I'm *not* soliciting detailed
patch review on the series. More like meta review, what worries do you
have for wider usage, etc.

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

Re: [RFC 4/4] drm/i915/registers: define field values using FIELD_PREP()

[Chris Wilson]

Quoting Jani Nikula (2018-09-27 10:40:23)
> Slightly verbose, but does away with hand rolled shifts and provides
> static checking that the values fit the mask.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> @@ -4650,11 +4650,11 @@ enum {
>  #define _PP_ON_DELAYS                  0x61208
>  #define PP_ON_DELAYS(pps_idx)          _MMIO_PPS(pps_idx, _PP_ON_DELAYS)
>  #define  PANEL_PORT_SELECT_MASK                GENMASK(31, 30)
> -#define  PANEL_PORT_SELECT_LVDS                (0 << 30)
> -#define  PANEL_PORT_SELECT_DPA         (1 << 30)
> -#define  PANEL_PORT_SELECT_DPC         (2 << 30)
> -#define  PANEL_PORT_SELECT_DPD         (3 << 30)
> -#define  PANEL_PORT_SELECT_VLV(port)   ((port) << 30)
> +#define  PANEL_PORT_SELECT_LVDS                FIELD_PREP(PANEL_PORT_SELECT_MASK, 0)
> +#define  PANEL_PORT_SELECT_DPA         FIELD_PREP(PANEL_PORT_SELECT_MASK, 1)
> +#define  PANEL_PORT_SELECT_DPC         FIELD_PREP(PANEL_PORT_SELECT_MASK, 2)
> +#define  PANEL_PORT_SELECT_DPD         FIELD_PREP(PANEL_PORT_SELECT_MASK, 3)
> +#define  PANEL_PORT_SELECT_VLV(port)   FIELD_PREP(PANEL_PORT_SELECT_MASK, port)

Maybe verbose, but it reads far better as giving each field a distinct
name ties together all the individual options.

Before seeing this I was sceptical about FIELD_PREP, no longer.

Under this construct we aren't using masks per se, but giving a name to
a group of bits within the register (a field). So I think

#define PANEL_PORT_SELECT	GENMASK(31, 30)
#define PANEL_PORT_SELECT_LVDS	FIELD_PREP(PANEL_PORT_SELECT, 0)

is intuitive. Did you do a quick bloatometer to see if gcc code
generation is affected?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] drm/i915/registers: use standard bits.h and bitfield.h macros

[Michal Wajdeczko]

On Thu, 27 Sep 2018 11:40:19 +0200, Jani Nikula <jani.nikula@intel.com>  
wrote:

> This is an RFC to get input on how people feel about moving towards
> using <linux/bits.h> and <linux/bitfield.h> macros for register field
> definitions and manipulation:
>
> * BIT()
> * GENMASK()

BIT/GENMASK macros assumes 'unsigned long' type (64b) while our registers
(and some of our temporary variables) are 'unsigned int' (32b).

It was reported [1] that use of plain BIT(0) may cause compilation issues.

Michal

[1]  
https://lists.freedesktop.org/archives/intel-gfx/2018-September/176704.html

> * FIELD_GET()
> * FIELD_PREP()


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

Re: [RFC 0/4] drm/i915/registers: use standard bits.h and bitfield.h macros

[Jani Nikula]

On Thu, 27 Sep 2018, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> On Thu, 27 Sep 2018 11:40:19 +0200, Jani Nikula <jani.nikula@intel.com>  
> wrote:
>
>> This is an RFC to get input on how people feel about moving towards
>> using <linux/bits.h> and <linux/bitfield.h> macros for register field
>> definitions and manipulation:
>>
>> * BIT()
>> * GENMASK()
>
> BIT/GENMASK macros assumes 'unsigned long' type (64b) while our registers
> (and some of our temporary variables) are 'unsigned int' (32b).

I don't see a problem with that as long as we stick to unsigned types.

> It was reported [1] that use of plain BIT(0) may cause compilation issues.

That mixes signed and unsigned types.


BR,
Jani.

>
> Michal
>
> [1]  
> https://lists.freedesktop.org/archives/intel-gfx/2018-September/176704.html
>
>> * FIELD_GET()
>> * FIELD_PREP()
>
>

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

Re: [RFC 0/4] drm/i915/registers: use standard bits.h and bitfield.h macros

[Jani Nikula]

On Thu, 27 Sep 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 27 Sep 2018, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
>> On Thu, 27 Sep 2018 11:40:19 +0200, Jani Nikula <jani.nikula@intel.com>  
>> wrote:
>>
>>> This is an RFC to get input on how people feel about moving towards
>>> using <linux/bits.h> and <linux/bitfield.h> macros for register field
>>> definitions and manipulation:
>>>
>>> * BIT()
>>> * GENMASK()
>>
>> BIT/GENMASK macros assumes 'unsigned long' type (64b) while our registers
>> (and some of our temporary variables) are 'unsigned int' (32b).
>
> I don't see a problem with that as long as we stick to unsigned types.

Okay, there's a problem with mixing (1 << n) with GENMASK, as the former
is signed. Using BIT() and GENMASK() together works better.

Another slight annoyance is printf formats, which are currently written
for unsigned ints, and passing GENMASK or BIT to prints will make them
require unsigned longs. I don't think we use that a whole lot, but it's
a consideration.

If all else fails, we can write our own wrappers for u32, though it's a
bit meh.

BR,
Jani.



>
>> It was reported [1] that use of plain BIT(0) may cause compilation issues.
>
> That mixes signed and unsigned types.
>
>
> BR,
> Jani.
>
>>
>> Michal
>>
>> [1]  
>> https://lists.freedesktop.org/archives/intel-gfx/2018-September/176704.html
>>
>>> * FIELD_GET()
>>> * FIELD_PREP()
>>
>>

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

Re: [RFC 4/4] drm/i915/registers: define field values using FIELD_PREP()

[Jani Nikula]

On Thu, 27 Sep 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2018-09-27 10:40:23)
>> Slightly verbose, but does away with hand rolled shifts and provides
>> static checking that the values fit the mask.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> @@ -4650,11 +4650,11 @@ enum {
>>  #define _PP_ON_DELAYS                  0x61208
>>  #define PP_ON_DELAYS(pps_idx)          _MMIO_PPS(pps_idx, _PP_ON_DELAYS)
>>  #define  PANEL_PORT_SELECT_MASK                GENMASK(31, 30)
>> -#define  PANEL_PORT_SELECT_LVDS                (0 << 30)
>> -#define  PANEL_PORT_SELECT_DPA         (1 << 30)
>> -#define  PANEL_PORT_SELECT_DPC         (2 << 30)
>> -#define  PANEL_PORT_SELECT_DPD         (3 << 30)
>> -#define  PANEL_PORT_SELECT_VLV(port)   ((port) << 30)
>> +#define  PANEL_PORT_SELECT_LVDS                FIELD_PREP(PANEL_PORT_SELECT_MASK, 0)
>> +#define  PANEL_PORT_SELECT_DPA         FIELD_PREP(PANEL_PORT_SELECT_MASK, 1)
>> +#define  PANEL_PORT_SELECT_DPC         FIELD_PREP(PANEL_PORT_SELECT_MASK, 2)
>> +#define  PANEL_PORT_SELECT_DPD         FIELD_PREP(PANEL_PORT_SELECT_MASK, 3)
>> +#define  PANEL_PORT_SELECT_VLV(port)   FIELD_PREP(PANEL_PORT_SELECT_MASK, port)
>
> Maybe verbose, but it reads far better as giving each field a distinct
> name ties together all the individual options.
>
> Before seeing this I was sceptical about FIELD_PREP, no longer.
>
> Under this construct we aren't using masks per se, but giving a name to
> a group of bits within the register (a field). So I think
>
> #define PANEL_PORT_SELECT	GENMASK(31, 30)
> #define PANEL_PORT_SELECT_LVDS	FIELD_PREP(PANEL_PORT_SELECT, 0)

Argh, I've screwed up. I must have failed to build after adding the last
patch. You know, RFC and all. This results in:

drivers/gpu/drm/i915/intel_display.c: In function ‘assert_panel_unlocked’:
drivers/gpu/drm/i915/intel_display.c:1219:3: error: case label does not reduce to an integer constant
   case PANEL_PORT_SELECT_LVDS:

Fail.

However, taking just the mask&shift part from FIELD_PREP works:

	(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))

But then we'll lose the static checks, unless we come up with something
ingenious ourselves.

BR,
Jani.


>
> is intuitive. Did you do a quick bloatometer to see if gcc code
> generation is affected?
> -Chris

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

Re: [RFC 4/4] drm/i915/registers: define field values using FIELD_PREP()

[Joonas Lahtinen]

Quoting Chris Wilson (2018-09-27 13:35:47)
> Quoting Jani Nikula (2018-09-27 10:40:23)
> > Slightly verbose, but does away with hand rolled shifts and provides
> > static checking that the values fit the mask.
> > 
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> > @@ -4650,11 +4650,11 @@ enum {
> >  #define _PP_ON_DELAYS                  0x61208
> >  #define PP_ON_DELAYS(pps_idx)          _MMIO_PPS(pps_idx, _PP_ON_DELAYS)
> >  #define  PANEL_PORT_SELECT_MASK                GENMASK(31, 30)
> > -#define  PANEL_PORT_SELECT_LVDS                (0 << 30)
> > -#define  PANEL_PORT_SELECT_DPA         (1 << 30)
> > -#define  PANEL_PORT_SELECT_DPC         (2 << 30)
> > -#define  PANEL_PORT_SELECT_DPD         (3 << 30)
> > -#define  PANEL_PORT_SELECT_VLV(port)   ((port) << 30)
> > +#define  PANEL_PORT_SELECT_LVDS                FIELD_PREP(PANEL_PORT_SELECT_MASK, 0)
> > +#define  PANEL_PORT_SELECT_DPA         FIELD_PREP(PANEL_PORT_SELECT_MASK, 1)
> > +#define  PANEL_PORT_SELECT_DPC         FIELD_PREP(PANEL_PORT_SELECT_MASK, 2)
> > +#define  PANEL_PORT_SELECT_DPD         FIELD_PREP(PANEL_PORT_SELECT_MASK, 3)
> > +#define  PANEL_PORT_SELECT_VLV(port)   FIELD_PREP(PANEL_PORT_SELECT_MASK, port)
> 
> Maybe verbose, but it reads far better as giving each field a distinct
> name ties together all the individual options.
> 
> Before seeing this I was sceptical about FIELD_PREP, no longer.
> 
> Under this construct we aren't using masks per se, but giving a name to
> a group of bits within the register (a field). So I think
> 
> #define PANEL_PORT_SELECT       GENMASK(31, 30)
> #define PANEL_PORT_SELECT_LVDS  FIELD_PREP(PANEL_PORT_SELECT, 0)
> 
> is intuitive. Did you do a quick bloatometer to see if gcc code
> generation is affected?

+1 from me for the change as big and for the above detail suggested by
Chris.

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

Re: [RFC 4/4] drm/i915/registers: define field values using FIELD_PREP()

[Jani Nikula]

On Thu, 27 Sep 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 27 Sep 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Quoting Jani Nikula (2018-09-27 10:40:23)
>>> Slightly verbose, but does away with hand rolled shifts and provides
>>> static checking that the values fit the mask.
>>> 
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>> @@ -4650,11 +4650,11 @@ enum {
>>>  #define _PP_ON_DELAYS                  0x61208
>>>  #define PP_ON_DELAYS(pps_idx)          _MMIO_PPS(pps_idx, _PP_ON_DELAYS)
>>>  #define  PANEL_PORT_SELECT_MASK                GENMASK(31, 30)
>>> -#define  PANEL_PORT_SELECT_LVDS                (0 << 30)
>>> -#define  PANEL_PORT_SELECT_DPA         (1 << 30)
>>> -#define  PANEL_PORT_SELECT_DPC         (2 << 30)
>>> -#define  PANEL_PORT_SELECT_DPD         (3 << 30)
>>> -#define  PANEL_PORT_SELECT_VLV(port)   ((port) << 30)
>>> +#define  PANEL_PORT_SELECT_LVDS                FIELD_PREP(PANEL_PORT_SELECT_MASK, 0)
>>> +#define  PANEL_PORT_SELECT_DPA         FIELD_PREP(PANEL_PORT_SELECT_MASK, 1)
>>> +#define  PANEL_PORT_SELECT_DPC         FIELD_PREP(PANEL_PORT_SELECT_MASK, 2)
>>> +#define  PANEL_PORT_SELECT_DPD         FIELD_PREP(PANEL_PORT_SELECT_MASK, 3)
>>> +#define  PANEL_PORT_SELECT_VLV(port)   FIELD_PREP(PANEL_PORT_SELECT_MASK, port)
>>
>> Maybe verbose, but it reads far better as giving each field a distinct
>> name ties together all the individual options.
>>
>> Before seeing this I was sceptical about FIELD_PREP, no longer.
>>
>> Under this construct we aren't using masks per se, but giving a name to
>> a group of bits within the register (a field). So I think
>>
>> #define PANEL_PORT_SELECT	GENMASK(31, 30)
>> #define PANEL_PORT_SELECT_LVDS	FIELD_PREP(PANEL_PORT_SELECT, 0)
>
> Argh, I've screwed up. I must have failed to build after adding the last
> patch. You know, RFC and all. This results in:
>
> drivers/gpu/drm/i915/intel_display.c: In function ‘assert_panel_unlocked’:
> drivers/gpu/drm/i915/intel_display.c:1219:3: error: case label does not reduce to an integer constant
>    case PANEL_PORT_SELECT_LVDS:
>
> Fail.
>
> However, taking just the mask&shift part from FIELD_PREP works:
>
> 	(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))
>
> But then we'll lose the static checks, unless we come up with something
> ingenious ourselves.

If my grep-fu serves me right, we have 186 macros defined in i915_reg.h
that are used as case labels, and almost all of them would be candidates
for FIELD_PREP().

Is an alternative FIELD_PREP() worth it without the static checks?

BR,
Jani.


>
> BR,
> Jani.
>
>
>>
>> is intuitive. Did you do a quick bloatometer to see if gcc code
>> generation is affected?
>> -Chris

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

✗ Fi.CI.BAT: failure for drm/i915/registers: use standard bits.h and bitfield.h macros

[Patchwork]

== Series Details ==

Series: drm/i915/registers: use standard bits.h and bitfield.h macros
URL   : https://patchwork.freedesktop.org/series/50267/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/intel_display.o
In file included from drivers/gpu/drm/i915/i915_drv.h:58:0,
                 from drivers/gpu/drm/i915/intel_drv.h:33,
                 from drivers/gpu/drm/i915/intel_display.c:36:
drivers/gpu/drm/i915/intel_display.c: In function ‘assert_panel_unlocked’:
drivers/gpu/drm/i915/i915_reg.h:4653:34: error: implicit declaration of function ‘FIELD_PREP’; did you mean ‘CLK_PRE’? [-Werror=implicit-function-declaration]
 #define  PANEL_PORT_SELECT_LVDS  FIELD_PREP(PANEL_PORT_SELECT_MASK, 0)
                                  ^
drivers/gpu/drm/i915/intel_display.c:1219:8: note: in expansion of macro ‘PANEL_PORT_SELECT_LVDS’
   case PANEL_PORT_SELECT_LVDS:
        ^~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:1219:3: error: case label does not reduce to an integer constant
   case PANEL_PORT_SELECT_LVDS:
   ^~~~
drivers/gpu/drm/i915/intel_display.c:1222:3: error: case label does not reduce to an integer constant
   case PANEL_PORT_SELECT_DPA:
   ^~~~
drivers/gpu/drm/i915/intel_display.c:1225:3: error: case label does not reduce to an integer constant
   case PANEL_PORT_SELECT_DPC:
   ^~~~
drivers/gpu/drm/i915/intel_display.c:1228:3: error: case label does not reduce to an integer constant
   case PANEL_PORT_SELECT_DPD:
   ^~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:305: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1050: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [RFC 1/4] drm/i915/registers: prefer GENMASK() over hand rolled masks

[Mika Kuoppala]

Jani Nikula <jani.nikula@intel.com> writes:

> GENMASK() is much easier to get right and review against the specs than
> hand rolled masks.
>
+1

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