All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: introduce macros to define register contents
@ 2018-10-03 16:05 Jani Nikula
  2018-10-03 16:05 ` [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() " Jani Nikula
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Jani Nikula @ 2018-10-03 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Actual serious v1 after the RFC [1].

The major change is to add local wrappers or versions of the BIT(),
GENMASK(), and FIELD_PREP() macros for i915_reg.h to ensure u32 type and
integer constant expressions.

It's a bit of a meh but does provide an opportunity for unified local
naming of the macros. After some back and forth, REG_BIT(),
REG_FIELD_MASK() and REG_FIELD() are what I settled on, but I guess I
expect some bikeshedding...

The big loss is the build-time checking of the defined field values;
FIELD_PREP() does it in a way that prohibits macro use in e.g. case
labels.

bloat-o-meter indicates slight increase, unsurprising due to the added
masking to avoid the values overflowing their fields. The changed sample
is too small to make any big conclusions though.

add/remove: 0/0 grow/shrink: 3/0 up/down: 81/0 (81)
Function                                     old     new   delta
intel_dp_init_panel_power_sequencer_registers     863     912     +49
intel_pre_enable_lvds                        618     646     +28
intel_pps_readout_hw_state                   385     389      +4
Total: Before=1106128, After=1106209, chg +0.01%


BR,
Jani.


[1] https://patchwork.freedesktop.org/series/50267/


Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Jani Nikula (3):
  drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register
    contents
  drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
  drm/i915: introduce REG_FIELD() to define register field values

 drivers/gpu/drm/i915/i915_reg.h   | 129 ++++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c   |  42 +++++--------
 drivers/gpu/drm/i915/intel_lvds.c |  40 ++++++------
 3 files changed, 104 insertions(+), 107 deletions(-)

-- 
2.11.0

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

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

* [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
@ 2018-10-03 16:05 ` Jani Nikula
  2018-10-03 16:54   ` Rodrigo Vivi
  2018-10-03 18:26   ` Manasi Navare
  2018-10-03 16:05 ` [PATCH 2/3] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK Jani Nikula
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 11+ messages in thread
From: Jani Nikula @ 2018-10-03 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Introduce REG_BIT(n) to define register bits and REG_FIELD_MASK(h, l) to
define register field masks.

We define the above as wrappers to BIT() and GENMASK() respectively to
force u32 type to go with our register size. Additionally, the specified
type will be helpful with follow-up to define and use register field
values through bitfield operations, and the macro wrapper naming is
aligned as well.

The intention is that these are easier to get right and review against
the spec than hand rolled masks.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 68 +++++++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c |  2 +-
 2 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a71c507cfb9b..ac9258769435 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -25,6 +25,8 @@
 #ifndef _I915_REG_H_
 #define _I915_REG_H_
 
+#include <linux/bits.h>
+
 /**
  * DOC: The i915 register macro definition style guide
  *
@@ -59,15 +61,13 @@
  * significant to least significant bit. Indent the register content macros
  * using two extra spaces between ``#define`` and the macro name.
  *
- * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Define bit field
- * contents so that they are already shifted in place, and can be directly
- * OR'd. For convenience, function-like macros may be used to define bit fields,
- * but do note that the macros may be needed to read as well as write the
- * register contents.
+ * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Use
+ * ``REG_FIELD_MASK()`` to define _MASK. Define bit field contents so that they
+ * are already shifted in place, and can be directly OR'd. For convenience,
+ * function-like macros may be used to define bit fields, but do note that the
+ * macros may be needed to read as well as write the register contents.
  *
- * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change this in
- * the future, but this is the prevailing style. Do **not** add ``_BIT`` suffix
- * to the name.
+ * Define bits using ``REG_BIT(N)``. Do **not** add ``_BIT`` suffix to the name.
  *
  * Group the register and its contents together without blank lines, separate
  * from other registers and their contents with one blank line.
@@ -105,8 +105,8 @@
  *  #define _FOO_A                      0xf000
  *  #define _FOO_B                      0xf001
  *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
- *  #define   FOO_ENABLE                (1 << 31)
- *  #define   FOO_MODE_MASK             (0xf << 16)
+ *  #define   FOO_ENABLE                REG_BIT(31)
+ *  #define   FOO_MODE_MASK             REG_FIELD_MASK(19, 16)
  *  #define   FOO_MODE_SHIFT            16
  *  #define   FOO_MODE_BAR              (0 << 16)
  *  #define   FOO_MODE_BAZ              (1 << 16)
@@ -116,6 +116,17 @@
  *  #define GEN8_BAR                    _MMIO(0xb888)
  */
 
+/*
+ * Macro for defining register bits. Local wrapper for BIT() to force u32.
+ */
+#define REG_BIT(n)		((u32)BIT(n))
+
+/*
+ * Macro for defining register field masks. Local wrapper for GENMASK() to force
+ * u32.
+ */
+#define REG_FIELD_MASK(h, l)	((u32)GENMASK(h, l))
+
 typedef struct {
 	uint32_t reg;
 } i915_reg_t;
@@ -4612,7 +4623,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				REG_BIT(31)
 /*
  * Indicates that all dependencies of the panel are on:
  *
@@ -4620,14 +4631,14 @@ enum {
  * - pipe enabled
  * - LVDS/DVOB/DVOC on
  */
-#define   PP_READY			(1 << 30)
+#define   PP_READY			REG_BIT(30)
+#define   PP_SEQUENCE_MASK		REG_FIELD_MASK(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_CYCLE_DELAY_ACTIVE		REG_BIT(27)
+#define   PP_SEQUENCE_STATE_MASK	REG_FIELD_MASK(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,42 +4651,41 @@ enum {
 
 #define _PP_CONTROL			0x61204
 #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
+#define  PANEL_UNLOCK_MASK		REG_FIELD_MASK(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	REG_FIELD_MASK(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			REG_BIT(3)
+#define  EDP_BLC_ENABLE			REG_BIT(2)
+#define  PANEL_POWER_RESET		REG_BIT(1)
+#define  PANEL_POWER_ON			REG_BIT(0)
 
 #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		REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
 #define  PANEL_POWER_UP_DELAY_SHIFT	16
-#define  PANEL_LIGHT_ON_DELAY_MASK	0x1fff
+#define  PANEL_LIGHT_ON_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
 #define  PANEL_POWER_DOWN_DELAY_SHIFT	16
-#define  PANEL_LIGHT_OFF_DELAY_MASK	0x1fff
+#define  PANEL_LIGHT_OFF_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(31, 8)
 #define  PP_REFERENCE_DIVIDER_SHIFT	8
-#define  PANEL_POWER_CYCLE_DELAY_MASK	0x1f
+#define  PANEL_POWER_CYCLE_DELAY_MASK	REG_FIELD_MASK(4, 0)
 #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
 
 /* Panel fitting */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 15a981ef5966..31eef9b0e33b 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

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

* [PATCH 2/3] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
  2018-10-03 16:05 ` [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() " Jani Nikula
@ 2018-10-03 16:05 ` Jani Nikula
  2018-10-03 16:05 ` [PATCH 3/3] drm/i915: introduce REG_FIELD() to define register field values Jani Nikula
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2018-10-03 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   | 21 ++++++--------------
 drivers/gpu/drm/i915/intel_dp.c   | 40 ++++++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_lvds.c | 40 ++++++++++++++++++---------------------
 3 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ac9258769435..dce4a6ac394c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -25,6 +25,7 @@
 #ifndef _I915_REG_H_
 #define _I915_REG_H_
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 
 /**
@@ -61,11 +62,11 @@
  * significant to least significant bit. Indent the register content macros
  * using two extra spaces between ``#define`` and the macro name.
  *
- * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Use
- * ``REG_FIELD_MASK()`` to define _MASK. Define bit field contents so that they
- * are already shifted in place, and can be directly OR'd. For convenience,
- * function-like macros may be used to define bit fields, but do note that the
- * macros may be needed to read as well as write the register contents.
+ * Define bit fields using ``REG_FIELD_MASK(h, l)``. Define bit field contents
+ * so that they are already shifted in place, and can be directly OR'd. For
+ * convenience, function-like macros may be used to define bit fields, but do
+ * note that the macros may be needed to read as well as write the register
+ * contents.
  *
  * Define bits using ``REG_BIT(N)``. Do **not** add ``_BIT`` suffix to the name.
  *
@@ -107,7 +108,6 @@
  *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
  *  #define   FOO_ENABLE                REG_BIT(31)
  *  #define   FOO_MODE_MASK             REG_FIELD_MASK(19, 16)
- *  #define   FOO_MODE_SHIFT            16
  *  #define   FOO_MODE_BAR              (0 << 16)
  *  #define   FOO_MODE_BAZ              (1 << 16)
  *  #define   FOO_MODE_QUX_SNB          (2 << 16)
@@ -4636,7 +4636,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		REG_BIT(27)
 #define   PP_SEQUENCE_STATE_MASK	REG_FIELD_MASK(3, 0)
 #define   PP_SEQUENCE_STATE_OFF_IDLE	(0x0 << 0)
@@ -4654,7 +4653,6 @@ enum {
 #define  PANEL_UNLOCK_MASK		REG_FIELD_MASK(31, 16)
 #define  PANEL_UNLOCK_REGS		(0xabcd << 16)
 #define  BXT_POWER_CYCLE_DELAY_MASK	REG_FIELD_MASK(8, 4)
-#define  BXT_POWER_CYCLE_DELAY_SHIFT	4
 #define  EDP_FORCE_VDD			REG_BIT(3)
 #define  EDP_BLC_ENABLE			REG_BIT(2)
 #define  PANEL_POWER_RESET		REG_BIT(1)
@@ -4662,7 +4660,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		REG_FIELD_MASK(31, 30)
 #define  PANEL_PORT_SELECT_LVDS		(0 << 30)
 #define  PANEL_PORT_SELECT_DPA		(1 << 30)
@@ -4670,23 +4667,17 @@ enum {
 #define  PANEL_PORT_SELECT_DPD		(3 << 30)
 #define  PANEL_PORT_SELECT_VLV(port)	((port) << 30)
 #define  PANEL_POWER_UP_DELAY_MASK	REG_FIELD_MASK(28, 16)
-#define  PANEL_POWER_UP_DELAY_SHIFT	16
 #define  PANEL_LIGHT_ON_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
-#define  PANEL_POWER_DOWN_DELAY_SHIFT	16
 #define  PANEL_LIGHT_OFF_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(31, 8)
-#define  PP_REFERENCE_DIVIDER_SHIFT	8
 #define  PANEL_POWER_CYCLE_DELAY_MASK	REG_FIELD_MASK(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 31eef9b0e33b..848ce42d7770 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5767,25 +5767,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;
 	}
 }
 
@@ -5945,22 +5936,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..285aee9e58f1 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -160,24 +160,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 +210,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

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

* [PATCH 3/3] drm/i915: introduce REG_FIELD() to define register field values
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
  2018-10-03 16:05 ` [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() " Jani Nikula
  2018-10-03 16:05 ` [PATCH 2/3] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK Jani Nikula
@ 2018-10-03 16:05 ` Jani Nikula
  2018-10-03 16:13 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents Patchwork
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2018-10-03 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Slightly verbose, but does away with hand rolled shifts. Ties the field
values with the mask defining the field.

Unfortunately we don't get the build-time checks of FIELD_PREP() due to
it not evaluating to a constant expression.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 58 +++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dce4a6ac394c..4dfb0f6f9e60 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -63,10 +63,10 @@
  * using two extra spaces between ``#define`` and the macro name.
  *
  * Define bit fields using ``REG_FIELD_MASK(h, l)``. Define bit field contents
- * so that they are already shifted in place, and can be directly OR'd. For
- * convenience, function-like macros may be used to define bit fields, but do
- * note that the macros may be needed to read as well as write the register
- * contents.
+ * using ``REG_FIELD(mask, value)``. This will define the values already shifted
+ * in place, so they can be directly OR'd together. For convenience,
+ * function-like macros may be used to define bit fields, but do note that the
+ * macros may be needed to read as well as write the register contents.
  *
  * Define bits using ``REG_BIT(N)``. Do **not** add ``_BIT`` suffix to the name.
  *
@@ -108,9 +108,9 @@
  *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
  *  #define   FOO_ENABLE                REG_BIT(31)
  *  #define   FOO_MODE_MASK             REG_FIELD_MASK(19, 16)
- *  #define   FOO_MODE_BAR              (0 << 16)
- *  #define   FOO_MODE_BAZ              (1 << 16)
- *  #define   FOO_MODE_QUX_SNB          (2 << 16)
+ *  #define   FOO_MODE_BAR              REG_FIELD(FOO_MODE_MASK, 0)
+ *  #define   FOO_MODE_BAZ              REG_FIELD(FOO_MODE_MASK, 1)
+ *  #define   FOO_MODE_QUX_SNB          REG_FIELD(FOO_MODE_MASK, 2)
  *
  *  #define BAR                         _MMIO(0xb000)
  *  #define GEN8_BAR                    _MMIO(0xb888)
@@ -127,6 +127,14 @@
  */
 #define REG_FIELD_MASK(h, l)	((u32)GENMASK(h, l))
 
+/*
+ * Macro for defining register field values. Local version of FIELD_PREP() to
+ * evaluate to an integer constant expression to allow use in e.g. case
+ * labels. Unfortunately this loses build-time checks on mask and value.
+ */
+#define REG_FIELD(_mask, _val)					\
+	(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))
+
 typedef struct {
 	uint32_t reg;
 } i915_reg_t;
@@ -4633,25 +4641,25 @@ enum {
  */
 #define   PP_READY			REG_BIT(30)
 #define   PP_SEQUENCE_MASK		REG_FIELD_MASK(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		REG_FIELD(PP_SEQUENCE_MASK, 0)
+#define   PP_SEQUENCE_POWER_UP		REG_FIELD(PP_SEQUENCE_MASK, 1)
+#define   PP_SEQUENCE_POWER_DOWN	REG_FIELD(PP_SEQUENCE_MASK, 2)
 #define   PP_CYCLE_DELAY_ACTIVE		REG_BIT(27)
 #define   PP_SEQUENCE_STATE_MASK	REG_FIELD_MASK(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	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0x0)
+#define   PP_SEQUENCE_STATE_OFF_S0_1	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0x1)
+#define   PP_SEQUENCE_STATE_OFF_S0_2	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0x2)
+#define   PP_SEQUENCE_STATE_OFF_S0_3	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0x3)
+#define   PP_SEQUENCE_STATE_ON_IDLE	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0x8)
+#define   PP_SEQUENCE_STATE_ON_S1_0	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0x9)
+#define   PP_SEQUENCE_STATE_ON_S1_2	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0xa)
+#define   PP_SEQUENCE_STATE_ON_S1_3	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0xb)
+#define   PP_SEQUENCE_STATE_RESET	REG_FIELD(PP_SEQUENCE_STATE_MASK, 0xf)
 
 #define _PP_CONTROL			0x61204
 #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
 #define  PANEL_UNLOCK_MASK		REG_FIELD_MASK(31, 16)
-#define  PANEL_UNLOCK_REGS		(0xabcd << 16)
+#define  PANEL_UNLOCK_REGS		REG_FIELD(PANEL_UNLOCK_MASK, 0xabcd)
 #define  BXT_POWER_CYCLE_DELAY_MASK	REG_FIELD_MASK(8, 4)
 #define  EDP_FORCE_VDD			REG_BIT(3)
 #define  EDP_BLC_ENABLE			REG_BIT(2)
@@ -4661,11 +4669,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		REG_FIELD_MASK(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		REG_FIELD(PANEL_PORT_SELECT_MASK, 0)
+#define  PANEL_PORT_SELECT_DPA		REG_FIELD(PANEL_PORT_SELECT_MASK, 1)
+#define  PANEL_PORT_SELECT_DPC		REG_FIELD(PANEL_PORT_SELECT_MASK, 2)
+#define  PANEL_PORT_SELECT_DPD		REG_FIELD(PANEL_PORT_SELECT_MASK, 3)
+#define  PANEL_PORT_SELECT_VLV(port)	REG_FIELD(PANEL_PORT_SELECT_MASK, port)
 #define  PANEL_POWER_UP_DELAY_MASK	REG_FIELD_MASK(28, 16)
 #define  PANEL_LIGHT_ON_DELAY_MASK	REG_FIELD_MASK(12, 0)
 
-- 
2.11.0

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

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
                   ` (2 preceding siblings ...)
  2018-10-03 16:05 ` [PATCH 3/3] drm/i915: introduce REG_FIELD() to define register field values Jani Nikula
@ 2018-10-03 16:13 ` Patchwork
  2018-10-03 16:15 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-10-03 16:13 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: introduce macros to define register contents
URL   : https://patchwork.freedesktop.org/series/50513/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
fa2aca822675 drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents
9d10556d888e drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
e48993ff2850 drm/i915: introduce REG_FIELD() to define register field values
-:60: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_mask' - possible side-effects?
#60: FILE: drivers/gpu/drm/i915/i915_reg.h:135:
+#define REG_FIELD(_mask, _val)					\
+	(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))

total: 0 errors, 0 warnings, 1 checks, 94 lines checked

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

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

* ✗ Fi.CI.SPARSE: warning for drm/i915: introduce macros to define register contents
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
                   ` (3 preceding siblings ...)
  2018-10-03 16:13 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents Patchwork
@ 2018-10-03 16:15 ` Patchwork
  2018-10-03 16:36 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-10-04  7:37 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-10-03 16:15 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: introduce macros to define register contents
URL   : https://patchwork.freedesktop.org/series/50513/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents
Okay!

Commit: drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
Okay!

Commit: drm/i915: introduce REG_FIELD() to define register field values
+drivers/gpu/drm/i915/intel_display.c:1219:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/intel_display.c:1222:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/intel_display.c:1225:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/intel_display.c:1228:22: error: Expected constant expression in case statement

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

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

* ✓ Fi.CI.BAT: success for drm/i915: introduce macros to define register contents
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
                   ` (4 preceding siblings ...)
  2018-10-03 16:15 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-10-03 16:36 ` Patchwork
  2018-10-04  7:37 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-10-03 16:36 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: introduce macros to define register contents
URL   : https://patchwork.freedesktop.org/series/50513/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4922 -> Patchwork_10346 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-soraka:      NOTRUN -> INCOMPLETE (fdo#107774, fdo#107859, fdo#107556)
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-bdw-samus:       NOTRUN -> INCOMPLETE (fdo#107773)

    igt@gem_wait@basic-wait-all:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#100368)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       INCOMPLETE (fdo#107773) -> PASS

    igt@gem_tiled_fence_blits@basic:
      fi-glk-j4005:       DMESG-WARN (fdo#105719) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
  fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859


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

  Additional (3): fi-kbl-soraka fi-skl-guc fi-snb-2520m 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 fi-icl-u 


== Build changes ==

    * Linux: CI_DRM_4922 -> Patchwork_10346

  CI_DRM_4922: 97b376a5213463a70eb977282f5486ded096648f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4665: 267870165d9ef66b4ab423e4efe7bacba023d75e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10346: e48993ff28501495fe5483d19cbe53be5722a8a7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e48993ff2850 drm/i915: introduce REG_FIELD() to define register field values
9d10556d888e drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
fa2aca822675 drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents

== Logs ==

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

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

* Re: [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents
  2018-10-03 16:05 ` [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() " Jani Nikula
@ 2018-10-03 16:54   ` Rodrigo Vivi
  2018-10-03 18:26   ` Manasi Navare
  1 sibling, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2018-10-03 16:54 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, Oct 03, 2018 at 07:05:21PM +0300, Jani Nikula wrote:
> Introduce REG_BIT(n) to define register bits and REG_FIELD_MASK(h, l) to
> define register field masks.
> 
> We define the above as wrappers to BIT() and GENMASK() respectively to
> force u32 type to go with our register size. Additionally, the specified
> type will be helpful with follow-up to define and use register field
> values through bitfield operations, and the macro wrapper naming is
> aligned as well.
> 
> The intention is that these are easier to get right and review against
> the spec than hand rolled masks.

My first impression was that reg rage as mask would get harder to code and
review, but it is just because I'm used to the current style....
But when thinking the real case of looking to the spec and writing the code
and/or reviewing it, the range makes indeed clear.

> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 68 +++++++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_dp.c |  2 +-
>  2 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a71c507cfb9b..ac9258769435 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -25,6 +25,8 @@
>  #ifndef _I915_REG_H_
>  #define _I915_REG_H_
>  
> +#include <linux/bits.h>
> +
>  /**
>   * DOC: The i915 register macro definition style guide
>   *
> @@ -59,15 +61,13 @@
>   * significant to least significant bit. Indent the register content macros
>   * using two extra spaces between ``#define`` and the macro name.
>   *
> - * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Define bit field
> - * contents so that they are already shifted in place, and can be directly
> - * OR'd. For convenience, function-like macros may be used to define bit fields,
> - * but do note that the macros may be needed to read as well as write the
> - * register contents.
> + * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Use
> + * ``REG_FIELD_MASK()`` to define _MASK. Define bit field contents so that they
> + * are already shifted in place, and can be directly OR'd. For convenience,
> + * function-like macros may be used to define bit fields, but do note that the
> + * macros may be needed to read as well as write the register contents.

can we add a note that this is all inclusive bits on the range?

>   *
> - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change this in
> - * the future, but this is the prevailing style. Do **not** add ``_BIT`` suffix
> - * to the name.
> + * Define bits using ``REG_BIT(N)``. Do **not** add ``_BIT`` suffix to the name.
>   *
>   * Group the register and its contents together without blank lines, separate
>   * from other registers and their contents with one blank line.
> @@ -105,8 +105,8 @@
>   *  #define _FOO_A                      0xf000
>   *  #define _FOO_B                      0xf001
>   *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
> - *  #define   FOO_ENABLE                (1 << 31)
> - *  #define   FOO_MODE_MASK             (0xf << 16)
> + *  #define   FOO_ENABLE                REG_BIT(31)
> + *  #define   FOO_MODE_MASK             REG_FIELD_MASK(19, 16)
>   *  #define   FOO_MODE_SHIFT            16
>   *  #define   FOO_MODE_BAR              (0 << 16)
>   *  #define   FOO_MODE_BAZ              (1 << 16)
> @@ -116,6 +116,17 @@
>   *  #define GEN8_BAR                    _MMIO(0xb888)
>   */
>  
> +/*
> + * Macro for defining register bits. Local wrapper for BIT() to force u32.
> + */
> +#define REG_BIT(n)		((u32)BIT(n))
> +
> +/*
> + * Macro for defining register field masks. Local wrapper for GENMASK() to force
> + * u32.
> + */
> +#define REG_FIELD_MASK(h, l)	((u32)GENMASK(h, l))
> +
>  typedef struct {
>  	uint32_t reg;
>  } i915_reg_t;
> @@ -4612,7 +4623,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				REG_BIT(31)
>  /*
>   * Indicates that all dependencies of the panel are on:
>   *
> @@ -4620,14 +4631,14 @@ enum {
>   * - pipe enabled
>   * - LVDS/DVOB/DVOC on
>   */
> -#define   PP_READY			(1 << 30)
> +#define   PP_READY			REG_BIT(30)
> +#define   PP_SEQUENCE_MASK		REG_FIELD_MASK(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_CYCLE_DELAY_ACTIVE		REG_BIT(27)
> +#define   PP_SEQUENCE_STATE_MASK	REG_FIELD_MASK(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,42 +4651,41 @@ enum {
>  
>  #define _PP_CONTROL			0x61204
>  #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
> +#define  PANEL_UNLOCK_MASK		REG_FIELD_MASK(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	REG_FIELD_MASK(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			REG_BIT(3)
> +#define  EDP_BLC_ENABLE			REG_BIT(2)
> +#define  PANEL_POWER_RESET		REG_BIT(1)
> +#define  PANEL_POWER_ON			REG_BIT(0)
>  
>  #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		REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
>  #define  PANEL_POWER_UP_DELAY_SHIFT	16
> -#define  PANEL_LIGHT_ON_DELAY_MASK	0x1fff
> +#define  PANEL_LIGHT_ON_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
>  #define  PANEL_POWER_DOWN_DELAY_SHIFT	16
> -#define  PANEL_LIGHT_OFF_DELAY_MASK	0x1fff
> +#define  PANEL_LIGHT_OFF_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(31, 8)
>  #define  PP_REFERENCE_DIVIDER_SHIFT	8
> -#define  PANEL_POWER_CYCLE_DELAY_MASK	0x1f
> +#define  PANEL_POWER_CYCLE_DELAY_MASK	REG_FIELD_MASK(4, 0)
>  #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
>  
>  /* Panel fitting */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 15a981ef5966..31eef9b0e33b 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents
  2018-10-03 16:05 ` [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() " Jani Nikula
  2018-10-03 16:54   ` Rodrigo Vivi
@ 2018-10-03 18:26   ` Manasi Navare
  2018-10-04  6:38     ` Jani Nikula
  1 sibling, 1 reply; 11+ messages in thread
From: Manasi Navare @ 2018-10-03 18:26 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

This seems really useful for the DSC PPS bitfields in i915_reg.h
since its a lot of bitfileds mapped from the spec to the macros for
for MASKS and SHIFTS for 128 bytes of PPS data.

This patch set only updates them in case of few registers.
All the other MASKS and SHIFTS clean up for all i915 registers
as a follow up right?

Manasi

On Wed, Oct 03, 2018 at 07:05:21PM +0300, Jani Nikula wrote:
> Introduce REG_BIT(n) to define register bits and REG_FIELD_MASK(h, l) to
> define register field masks.
> 
> We define the above as wrappers to BIT() and GENMASK() respectively to
> force u32 type to go with our register size. Additionally, the specified
> type will be helpful with follow-up to define and use register field
> values through bitfield operations, and the macro wrapper naming is
> aligned as well.
> 
> The intention is that these are easier to get right and review against
> the spec than hand rolled masks.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 68 +++++++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_dp.c |  2 +-
>  2 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a71c507cfb9b..ac9258769435 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -25,6 +25,8 @@
>  #ifndef _I915_REG_H_
>  #define _I915_REG_H_
>  
> +#include <linux/bits.h>
> +
>  /**
>   * DOC: The i915 register macro definition style guide
>   *
> @@ -59,15 +61,13 @@
>   * significant to least significant bit. Indent the register content macros
>   * using two extra spaces between ``#define`` and the macro name.
>   *
> - * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Define bit field
> - * contents so that they are already shifted in place, and can be directly
> - * OR'd. For convenience, function-like macros may be used to define bit fields,
> - * but do note that the macros may be needed to read as well as write the
> - * register contents.
> + * For bit fields, define a ``_MASK`` and a ``_SHIFT`` macro. Use
> + * ``REG_FIELD_MASK()`` to define _MASK. Define bit field contents so that they
> + * are already shifted in place, and can be directly OR'd. For convenience,
> + * function-like macros may be used to define bit fields, but do note that the
> + * macros may be needed to read as well as write the register contents.
>   *
> - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change this in
> - * the future, but this is the prevailing style. Do **not** add ``_BIT`` suffix
> - * to the name.
> + * Define bits using ``REG_BIT(N)``. Do **not** add ``_BIT`` suffix to the name.
>   *
>   * Group the register and its contents together without blank lines, separate
>   * from other registers and their contents with one blank line.
> @@ -105,8 +105,8 @@
>   *  #define _FOO_A                      0xf000
>   *  #define _FOO_B                      0xf001
>   *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
> - *  #define   FOO_ENABLE                (1 << 31)
> - *  #define   FOO_MODE_MASK             (0xf << 16)
> + *  #define   FOO_ENABLE                REG_BIT(31)
> + *  #define   FOO_MODE_MASK             REG_FIELD_MASK(19, 16)
>   *  #define   FOO_MODE_SHIFT            16
>   *  #define   FOO_MODE_BAR              (0 << 16)
>   *  #define   FOO_MODE_BAZ              (1 << 16)
> @@ -116,6 +116,17 @@
>   *  #define GEN8_BAR                    _MMIO(0xb888)
>   */
>  
> +/*
> + * Macro for defining register bits. Local wrapper for BIT() to force u32.
> + */
> +#define REG_BIT(n)		((u32)BIT(n))
> +
> +/*
> + * Macro for defining register field masks. Local wrapper for GENMASK() to force
> + * u32.
> + */
> +#define REG_FIELD_MASK(h, l)	((u32)GENMASK(h, l))
> +
>  typedef struct {
>  	uint32_t reg;
>  } i915_reg_t;
> @@ -4612,7 +4623,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				REG_BIT(31)
>  /*
>   * Indicates that all dependencies of the panel are on:
>   *
> @@ -4620,14 +4631,14 @@ enum {
>   * - pipe enabled
>   * - LVDS/DVOB/DVOC on
>   */
> -#define   PP_READY			(1 << 30)
> +#define   PP_READY			REG_BIT(30)
> +#define   PP_SEQUENCE_MASK		REG_FIELD_MASK(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_CYCLE_DELAY_ACTIVE		REG_BIT(27)
> +#define   PP_SEQUENCE_STATE_MASK	REG_FIELD_MASK(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,42 +4651,41 @@ enum {
>  
>  #define _PP_CONTROL			0x61204
>  #define PP_CONTROL(pps_idx)		_MMIO_PPS(pps_idx, _PP_CONTROL)
> +#define  PANEL_UNLOCK_MASK		REG_FIELD_MASK(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	REG_FIELD_MASK(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			REG_BIT(3)
> +#define  EDP_BLC_ENABLE			REG_BIT(2)
> +#define  PANEL_POWER_RESET		REG_BIT(1)
> +#define  PANEL_POWER_ON			REG_BIT(0)
>  
>  #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		REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
>  #define  PANEL_POWER_UP_DELAY_SHIFT	16
> -#define  PANEL_LIGHT_ON_DELAY_MASK	0x1fff
> +#define  PANEL_LIGHT_ON_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(28, 16)
>  #define  PANEL_POWER_DOWN_DELAY_SHIFT	16
> -#define  PANEL_LIGHT_OFF_DELAY_MASK	0x1fff
> +#define  PANEL_LIGHT_OFF_DELAY_MASK	REG_FIELD_MASK(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	REG_FIELD_MASK(31, 8)
>  #define  PP_REFERENCE_DIVIDER_SHIFT	8
> -#define  PANEL_POWER_CYCLE_DELAY_MASK	0x1f
> +#define  PANEL_POWER_CYCLE_DELAY_MASK	REG_FIELD_MASK(4, 0)
>  #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
>  
>  /* Panel fitting */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 15a981ef5966..31eef9b0e33b 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() to define register contents
  2018-10-03 18:26   ` Manasi Navare
@ 2018-10-04  6:38     ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2018-10-04  6:38 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Wed, 03 Oct 2018, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This patch set only updates them in case of few registers.
> All the other MASKS and SHIFTS clean up for all i915 registers
> as a follow up right?

Let's see if we can agree this is the direction we want to go first.

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] 11+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: introduce macros to define register contents
  2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
                   ` (5 preceding siblings ...)
  2018-10-03 16:36 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-04  7:37 ` Patchwork
  6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-10-04  7:37 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: introduce macros to define register contents
URL   : https://patchwork.freedesktop.org/series/50513/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4922_full -> Patchwork_10346_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10346_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10346_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106886)

    igt@gem_exec_big:
      shard-hsw:          PASS -> TIMEOUT (fdo#107937)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039, fdo#108130)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-skl:          NOTRUN -> FAIL (fdo#106641)

    igt@kms_busy@extended-modeset-hang-newfb-render-b:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      shard-skl:          PASS -> FAIL (fdo#100368)

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
      shard-skl:          NOTRUN -> FAIL (fdo#105683)

    {igt@kms_plane_alpha_blend@pipe-b-alpha-7efc}:
      shard-skl:          NOTRUN -> FAIL (fdo#108146)

    {igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min}:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_setmode@basic:
      shard-hsw:          PASS -> FAIL (fdo#99912)
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_cursor_crc@cursor-256x85-onscreen:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-apl:          DMESG-FAIL (fdo#105602, fdo#103558, fdo#103166) -> PASS

    igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +48

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
    ==== Warnings ====

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> FAIL (fdo#105458, fdo#106510)

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-apl:          DMESG-FAIL (fdo#105602, fdo#103558, fdo#103166) -> FAIL (fdo#103166)

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

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105458 https://bugs.freedesktop.org/show_bug.cgi?id=105458
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683
  fdo#106510 https://bugs.freedesktop.org/show_bug.cgi?id=106510
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107937 https://bugs.freedesktop.org/show_bug.cgi?id=107937
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108130 https://bugs.freedesktop.org/show_bug.cgi?id=108130
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-glk 


== Build changes ==

    * Linux: CI_DRM_4922 -> Patchwork_10346

  CI_DRM_4922: 97b376a5213463a70eb977282f5486ded096648f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4665: 267870165d9ef66b4ab423e4efe7bacba023d75e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10346: e48993ff28501495fe5483d19cbe53be5722a8a7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

end of thread, other threads:[~2018-10-04  7:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 16:05 [PATCH 0/3] drm/i915: introduce macros to define register contents Jani Nikula
2018-10-03 16:05 ` [PATCH 1/3] drm/i915: introduce REG_BIT() and REG_FIELD_MASK() " Jani Nikula
2018-10-03 16:54   ` Rodrigo Vivi
2018-10-03 18:26   ` Manasi Navare
2018-10-04  6:38     ` Jani Nikula
2018-10-03 16:05 ` [PATCH 2/3] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK Jani Nikula
2018-10-03 16:05 ` [PATCH 3/3] drm/i915: introduce REG_FIELD() to define register field values Jani Nikula
2018-10-03 16:13 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents Patchwork
2018-10-03 16:15 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-03 16:36 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-04  7:37 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.