* [PATCH v2 0/4] drm/i915: introduce macros to define register contents
@ 2019-01-17 12:13 Jani Nikula
2019-01-17 12:14 ` [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use Jani Nikula
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: Jani Nikula @ 2019-01-17 12:13 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
The v1 [1] kind of died down because the FIELD_PREP() build-time checks
were lost as it didn't evaluate to integer constant expression. I looked
at this again, and managed to include the checks in the local copy by
using BUILD_BUG_ON_ZERO() instead.
On the naming bikeshedding department, I noticed a clash with regmap.h
REG_FIELD() and, since it all looked pretty verbose anyway, decided to
see if local _BIT(), _MASK(), and _FIELD() would stick.
BR,
Jani.
[1] http://mid.mail-archive.com/cover.1538582156.git.jani.nikula@intel.com
Jani Nikula (4):
drm/i915/dp: remove PANEL_POWER_OFF macro and its use
drm/i915: introduce _BIT() and _MASK() to define register contents
drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
drm/i915: introduce _FIELD() to define register field values
drivers/gpu/drm/i915/i915_reg.h | 155 +++++++++++++++++-------------
drivers/gpu/drm/i915/intel_dp.c | 43 ++++-----
drivers/gpu/drm/i915/intel_lvds.c | 40 ++++----
3 files changed, 124 insertions(+), 114 deletions(-)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
@ 2019-01-17 12:14 ` Jani Nikula
2019-01-17 12:25 ` Mika Kuoppala
2019-01-17 12:14 ` [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents Jani Nikula
` (7 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2019-01-17 12:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
It's superfluous.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 1 -
drivers/gpu/drm/i915/intel_dp.c | 3 +--
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9a1340cfda6c..93cbd057c07a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4687,7 +4687,6 @@ enum {
#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 _PP_ON_DELAYS 0x61208
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 808ccdae15b8..f7d5314e3395 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1000,8 +1000,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.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
2019-01-17 12:14 ` [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use Jani Nikula
@ 2019-01-17 12:14 ` Jani Nikula
2019-01-17 13:02 ` Mika Kuoppala
2019-01-17 12:14 ` [PATCH v2 3/4] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK Jani Nikula
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2019-01-17 12:14 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Introduce _BIT(n) to define register bits and _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.
The intention is that these are easier to get right and review against
the spec than hand rolled masks.
v2: rename macros to just _BIT() and _MASK() to reduce verbosity
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 | 83 ++++++++++++++++++++-------------
1 file changed, 50 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 93cbd057c07a..b6cc06b42c1a 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 ``_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 ``_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 _BIT(31)
+ * #define FOO_MODE_MASK _MASK(19, 16)
* #define FOO_MODE_SHIFT 16
* #define FOO_MODE_BAR (0 << 16)
* #define FOO_MODE_BAZ (1 << 16)
@@ -116,6 +116,23 @@
* #define GEN8_BAR _MMIO(0xb888)
*/
+/*
+ * Macro for defining register bits. Bit number is 0-based.
+ *
+ * Local wrapper for BIT() to force u32. Do *not* use outside of this file. Use
+ * BIT() instead.
+ */
+#define _BIT(__n) ((u32)BIT(__n))
+
+/*
+ * Macro for defining register field masks. Bit numbers are 0-based and the mask
+ * includes the high and low bits.
+ *
+ * Local wrapper for GENMASK() to force u32. Do *not* use outside of this
+ * file. Use GENMASK() instead.
+ */
+#define _MASK(__high, __low) ((u32)GENMASK(__high, __low))
+
typedef struct {
u32 reg;
} i915_reg_t;
@@ -4641,18 +4658,18 @@ 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)
#define _PP_CONTROL_1 0xc7204
#define _PP_CONTROL_2 0xc7304
#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
_PP_CONTROL_2)
-#define POWER_CYCLE_DELAY_MASK (0x1f << 4)
+#define POWER_CYCLE_DELAY_MASK _MASK(8, 4)
#define POWER_CYCLE_DELAY_SHIFT 4
-#define VDD_OVERRIDE_FORCE (1 << 3)
-#define BACKLIGHT_ENABLE (1 << 2)
-#define PWR_DOWN_ON_RESET (1 << 1)
-#define PWR_STATE_TARGET (1 << 0)
+#define VDD_OVERRIDE_FORCE _BIT(3)
+#define BACKLIGHT_ENABLE _BIT(2)
+#define PWR_DOWN_ON_RESET _BIT(1)
+#define PWR_STATE_TARGET _BIT(0)
/*
* Indicates that all dependencies of the panel are on:
*
@@ -4660,14 +4677,14 @@ enum {
* - pipe enabled
* - LVDS/DVOB/DVOC on
*/
-#define PP_READY (1 << 30)
+#define PP_READY _BIT(30)
+#define PP_SEQUENCE_MASK _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 _BIT(27)
+#define PP_SEQUENCE_STATE_MASK _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)
@@ -4680,41 +4697,41 @@ enum {
#define _PP_CONTROL 0x61204
#define PP_CONTROL(pps_idx) _MMIO_PPS(pps_idx, _PP_CONTROL)
+#define PANEL_UNLOCK_MASK _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 _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_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)
#define PANEL_PORT_SELECT_SHIFT 30
-#define PANEL_PORT_SELECT_MASK (3 << 30)
+#define PANEL_PORT_SELECT_MASK _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 _MASK(28, 16)
#define PANEL_POWER_UP_DELAY_SHIFT 16
-#define PANEL_LIGHT_ON_DELAY_MASK 0x1fff
+#define PANEL_LIGHT_ON_DELAY_MASK _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 _MASK(28, 16)
#define PANEL_POWER_DOWN_DELAY_SHIFT 16
-#define PANEL_LIGHT_OFF_DELAY_MASK 0x1fff
+#define PANEL_LIGHT_OFF_DELAY_MASK _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 _MASK(31, 8)
#define PP_REFERENCE_DIVIDER_SHIFT 8
-#define PANEL_POWER_CYCLE_DELAY_MASK 0x1f
+#define PANEL_POWER_CYCLE_DELAY_MASK _MASK(4, 0)
#define PANEL_POWER_CYCLE_DELAY_SHIFT 0
/* Panel fitting */
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/4] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
2019-01-17 12:14 ` [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use Jani Nikula
2019-01-17 12:14 ` [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents Jani Nikula
@ 2019-01-17 12:14 ` Jani Nikula
2019-01-17 12:14 ` [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values Jani Nikula
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2019-01-17 12:14 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, 39 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b6cc06b42c1a..1b5cbae9c11d 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,10 @@
* 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 ``_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 ``_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 ``_BIT(N)``. Do **not** add ``_BIT`` suffix to the name.
*
@@ -107,7 +107,6 @@
* #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
* #define FOO_ENABLE _BIT(31)
* #define FOO_MODE_MASK _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)
@@ -4665,7 +4664,6 @@ enum {
#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
_PP_CONTROL_2)
#define POWER_CYCLE_DELAY_MASK _MASK(8, 4)
-#define POWER_CYCLE_DELAY_SHIFT 4
#define VDD_OVERRIDE_FORCE _BIT(3)
#define BACKLIGHT_ENABLE _BIT(2)
#define PWR_DOWN_ON_RESET _BIT(1)
@@ -4682,7 +4680,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 _MASK(3, 0)
#define PP_SEQUENCE_STATE_OFF_IDLE (0x0 << 0)
@@ -4700,7 +4697,6 @@ enum {
#define PANEL_UNLOCK_MASK _MASK(31, 16)
#define PANEL_UNLOCK_REGS (0xabcd << 16)
#define BXT_POWER_CYCLE_DELAY_MASK _MASK(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)
@@ -4708,7 +4704,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 _MASK(31, 30)
#define PANEL_PORT_SELECT_LVDS (0 << 30)
#define PANEL_PORT_SELECT_DPA (1 << 30)
@@ -4716,23 +4711,17 @@ enum {
#define PANEL_PORT_SELECT_DPD (3 << 30)
#define PANEL_PORT_SELECT_VLV(port) ((port) << 30)
#define PANEL_POWER_UP_DELAY_MASK _MASK(28, 16)
-#define PANEL_POWER_UP_DELAY_SHIFT 16
#define PANEL_LIGHT_ON_DELAY_MASK _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 _MASK(28, 16)
-#define PANEL_POWER_DOWN_DELAY_SHIFT 16
#define PANEL_LIGHT_OFF_DELAY_MASK _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 _MASK(31, 8)
-#define PP_REFERENCE_DIVIDER_SHIFT 8
#define PANEL_POWER_CYCLE_DELAY_MASK _MASK(4, 0)
-#define PANEL_POWER_CYCLE_DELAY_SHIFT 0
/* Panel fitting */
#define PFIT_CONTROL _MMIO(DISPLAY_MMIO_BASE(dev_priv) + 0x61230)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f7d5314e3395..971e2624cd79 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6086,25 +6086,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;
}
}
@@ -6264,22 +6255,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 46a5dfd5cdf7..95a639a76f05 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -152,24 +152,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
@@ -209,15 +202,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.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
` (2 preceding siblings ...)
2019-01-17 12:14 ` [PATCH v2 3/4] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK Jani Nikula
@ 2019-01-17 12:14 ` Jani Nikula
2019-01-17 12:58 ` Chris Wilson
2019-01-17 12:52 ` [PATCH v2 0/4] drm/i915: introduce macros to define register contents Chris Wilson
` (4 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2019-01-17 12:14 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 have to make a local copy of FIELD_PREP() to evaluate
to a constant expression. But with this, we can ensure the mask is
non-zero, power of 2, fits u32, and the value fits the mask (when the
value is a constant expression).
v2:
- add build-time checks with BUILD_BUG_ON_ZERO()
- rename to just _FIELD() due to regmap.h REG_FIELD() clash
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 | 64 +++++++++++++++++++++------------
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1b5cbae9c11d..a07b61cec078 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -62,8 +62,9 @@
* significant to least significant bit. Indent the register content macros
* using two extra spaces between ``#define`` and the macro name.
*
- * Define bit fields using ``_MASK(h, l)``. Define bit field contents so that
- * they are already shifted in place, and can be directly OR'd. For convenience,
+ * Define bit fields using ``_MASK(h, l)``. Define bit field contents using
+ * ``_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.
*
@@ -107,9 +108,9 @@
* #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
* #define FOO_ENABLE _BIT(31)
* #define FOO_MODE_MASK _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 _FIELD(FOO_MODE_MASK, 0)
+ * #define FOO_MODE_BAZ _FIELD(FOO_MODE_MASK, 1)
+ * #define FOO_MODE_QUX_SNB _FIELD(FOO_MODE_MASK, 2)
*
* #define BAR _MMIO(0xb000)
* #define GEN8_BAR _MMIO(0xb888)
@@ -132,6 +133,23 @@
*/
#define _MASK(__high, __low) ((u32)GENMASK(__high, __low))
+#define __POWER_OF_2(x) ((x) && (((x) & ((x) - 1)) == 0))
+
+/*
+ * Macro for defining register field values. Shifts the value to the register
+ * field defined by mask.
+ *
+ * Local version of FIELD_PREP() to evaluate to an integer constant expression
+ * to allow use in e.g. case labels. Do *not* use outside of this file. Use
+ * FIELD_PREP() instead.
+ */
+#define _FIELD(__mask, __val) \
+ ((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) + \
+ BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
+ BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+ BUILD_BUG_ON_ZERO(!__POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+ BUILD_BUG_ON_ZERO(__builtin_choose_expr(__builtin_constant_p(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
+
typedef struct {
u32 reg;
} i915_reg_t;
@@ -4677,25 +4695,25 @@ enum {
*/
#define PP_READY _BIT(30)
#define PP_SEQUENCE_MASK _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 _FIELD(PP_SEQUENCE_MASK, 0)
+#define PP_SEQUENCE_POWER_UP _FIELD(PP_SEQUENCE_MASK, 1)
+#define PP_SEQUENCE_POWER_DOWN _FIELD(PP_SEQUENCE_MASK, 2)
#define PP_CYCLE_DELAY_ACTIVE _BIT(27)
#define PP_SEQUENCE_STATE_MASK _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 _FIELD(PP_SEQUENCE_STATE_MASK, 0x0)
+#define PP_SEQUENCE_STATE_OFF_S0_1 _FIELD(PP_SEQUENCE_STATE_MASK, 0x1)
+#define PP_SEQUENCE_STATE_OFF_S0_2 _FIELD(PP_SEQUENCE_STATE_MASK, 0x2)
+#define PP_SEQUENCE_STATE_OFF_S0_3 _FIELD(PP_SEQUENCE_STATE_MASK, 0x3)
+#define PP_SEQUENCE_STATE_ON_IDLE _FIELD(PP_SEQUENCE_STATE_MASK, 0x8)
+#define PP_SEQUENCE_STATE_ON_S1_0 _FIELD(PP_SEQUENCE_STATE_MASK, 0x9)
+#define PP_SEQUENCE_STATE_ON_S1_2 _FIELD(PP_SEQUENCE_STATE_MASK, 0xa)
+#define PP_SEQUENCE_STATE_ON_S1_3 _FIELD(PP_SEQUENCE_STATE_MASK, 0xb)
+#define PP_SEQUENCE_STATE_RESET _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 _MASK(31, 16)
-#define PANEL_UNLOCK_REGS (0xabcd << 16)
+#define PANEL_UNLOCK_REGS _FIELD(PANEL_UNLOCK_MASK, 0xabcd)
#define BXT_POWER_CYCLE_DELAY_MASK _MASK(8, 4)
#define EDP_FORCE_VDD _BIT(3)
#define EDP_BLC_ENABLE _BIT(2)
@@ -4705,11 +4723,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 _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 _FIELD(PANEL_PORT_SELECT_MASK, 0)
+#define PANEL_PORT_SELECT_DPA _FIELD(PANEL_PORT_SELECT_MASK, 1)
+#define PANEL_PORT_SELECT_DPC _FIELD(PANEL_PORT_SELECT_MASK, 2)
+#define PANEL_PORT_SELECT_DPD _FIELD(PANEL_PORT_SELECT_MASK, 3)
+#define PANEL_PORT_SELECT_VLV(port) _FIELD(PANEL_PORT_SELECT_MASK, port)
#define PANEL_POWER_UP_DELAY_MASK _MASK(28, 16)
#define PANEL_LIGHT_ON_DELAY_MASK _MASK(12, 0)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use
2019-01-17 12:14 ` [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use Jani Nikula
@ 2019-01-17 12:25 ` Mika Kuoppala
2019-01-18 13:42 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2019-01-17 12:25 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Jani Nikula <jani.nikula@intel.com> writes:
> It's superfluous.
One could argue that it has a minor documentative purpose.
But there is comment for that.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 -
> drivers/gpu/drm/i915/intel_dp.c | 3 +--
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9a1340cfda6c..93cbd057c07a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4687,7 +4687,6 @@ enum {
> #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 _PP_ON_DELAYS 0x61208
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 808ccdae15b8..f7d5314e3395 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1000,8 +1000,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.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/4] drm/i915: introduce macros to define register contents
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
` (3 preceding siblings ...)
2019-01-17 12:14 ` [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values Jani Nikula
@ 2019-01-17 12:52 ` Chris Wilson
2019-02-27 11:01 ` Jani Nikula
2019-01-17 12:54 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents (rev2) Patchwork
` (3 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2019-01-17 12:52 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Quoting Jani Nikula (2019-01-17 12:13:59)
> The v1 [1] kind of died down because the FIELD_PREP() build-time checks
> were lost as it didn't evaluate to integer constant expression. I looked
> at this again, and managed to include the checks in the local copy by
> using BUILD_BUG_ON_ZERO() instead.
>
> On the naming bikeshedding department, I noticed a clash with regmap.h
> REG_FIELD() and, since it all looked pretty verbose anyway, decided to
> see if local _BIT(), _MASK(), and _FIELD() would stick.
MMIO_BIT, MMIO_MASK, MMIO_FIELD
#define MMIO_PREP(mask, val) FIELD_PREP(mask, val)
?
The leading underscore and the inconsistency with FIELD_PREP is getting
to me.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents (rev2)
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
` (4 preceding siblings ...)
2019-01-17 12:52 ` [PATCH v2 0/4] drm/i915: introduce macros to define register contents Chris Wilson
@ 2019-01-17 12:54 ` Patchwork
2019-01-17 12:57 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-01-17 12:54 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: introduce macros to define register contents (rev2)
URL : https://patchwork.freedesktop.org/series/50513/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
53519fbcbe40 drm/i915/dp: remove PANEL_POWER_OFF macro and its use
b64ca6410c69 drm/i915: introduce _BIT() and _MASK() to define register contents
dc4e11b1eaf0 drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
9edbb60abd1b drm/i915: introduce _FIELD() to define register field values
-:57: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'x' - possible side-effects?
#57: FILE: drivers/gpu/drm/i915/i915_reg.h:136:
+#define __POWER_OF_2(x) ((x) && (((x) & ((x) - 1)) == 0))
-:67: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__mask' - possible side-effects?
#67: FILE: drivers/gpu/drm/i915/i915_reg.h:146:
+#define _FIELD(__mask, __val) \
+ ((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) + \
+ BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
+ BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+ BUILD_BUG_ON_ZERO(!__POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+ BUILD_BUG_ON_ZERO(__builtin_choose_expr(__builtin_constant_p(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
-:67: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__val' - possible side-effects?
#67: FILE: drivers/gpu/drm/i915/i915_reg.h:146:
+#define _FIELD(__mask, __val) \
+ ((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) + \
+ BUILD_BUG_ON_ZERO(!__builtin_constant_p(__mask)) + \
+ BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+ BUILD_BUG_ON_ZERO(!__POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+ BUILD_BUG_ON_ZERO(__builtin_choose_expr(__builtin_constant_p(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
-:72: WARNING:LONG_LINE: line over 100 characters
#72: FILE: drivers/gpu/drm/i915/i915_reg.h:151:
+ BUILD_BUG_ON_ZERO(__builtin_choose_expr(__builtin_constant_p(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
total: 0 errors, 1 warnings, 3 checks, 100 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: introduce macros to define register contents (rev2)
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
` (5 preceding siblings ...)
2019-01-17 12:54 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents (rev2) Patchwork
@ 2019-01-17 12:57 ` Patchwork
2019-01-17 13:13 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-17 21:33 ` ✓ Fi.CI.IGT: " Patchwork
8 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-01-17 12:57 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: introduce macros to define register contents (rev2)
URL : https://patchwork.freedesktop.org/series/50513/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/dp: remove PANEL_POWER_OFF macro and its use
Okay!
Commit: drm/i915: introduce _BIT() and _MASK() to define register contents
Okay!
Commit: drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
Okay!
Commit: drm/i915: introduce _FIELD() to define register field values
+drivers/gpu/drm/i915/intel_display.c:1156:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/intel_display.c:1159:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/intel_display.c:1162:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/intel_display.c:1165:22: error: Expected constant expression in case statement
-drivers/gpu/drm/i915/intel_display.c:2362:13: warning: call with no type!
-./include/linux/reservation.h:220:20: warning: dereference of noderef expression
-./include/linux/reservation.h:220:20: warning: dereference of noderef expression
-./include/linux/reservation.h:220:20: warning: dereference of noderef expression
-./include/linux/reservation.h:220:45: warning: dereference of noderef expression
-./include/linux/reservation.h:220:45: warning: dereference of noderef expression
-./include/linux/reservation.h:220:45: warning: dereference of noderef expression
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values
2019-01-17 12:14 ` [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values Jani Nikula
@ 2019-01-17 12:58 ` Chris Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2019-01-17 12:58 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Quoting Jani Nikula (2019-01-17 12:14:03)
> #define _MASK(__high, __low) ((u32)GENMASK(__high, __low))
>
> +#define __POWER_OF_2(x) ((x) && (((x) & ((x) - 1)) == 0))
Fwiw, I'd go with IS_POWER_OF_2(x) /* compile-time constant version of is_power_of_2() */
For constexpr!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents
2019-01-17 12:14 ` [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents Jani Nikula
@ 2019-01-17 13:02 ` Mika Kuoppala
0 siblings, 0 replies; 16+ messages in thread
From: Mika Kuoppala @ 2019-01-17 13:02 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
Jani Nikula <jani.nikula@intel.com> writes:
> Introduce _BIT(n) to define register bits and _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.
>
> The intention is that these are easier to get right and review against
> the spec than hand rolled masks.
>
> v2: rename macros to just _BIT() and _MASK() to reduce verbosity
>
> 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 | 83 ++++++++++++++++++++-------------
> 1 file changed, 50 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 93cbd057c07a..b6cc06b42c1a 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 ``_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 ``_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 _BIT(31)
> + * #define FOO_MODE_MASK _MASK(19, 16)
> * #define FOO_MODE_SHIFT 16
> * #define FOO_MODE_BAR (0 << 16)
> * #define FOO_MODE_BAZ (1 << 16)
> @@ -116,6 +116,23 @@
> * #define GEN8_BAR _MMIO(0xb888)
> */
>
> +/*
> + * Macro for defining register bits. Bit number is 0-based.
> + *
> + * Local wrapper for BIT() to force u32. Do *not* use outside of this file. Use
> + * BIT() instead.
> + */
> +#define _BIT(__n) ((u32)BIT(__n))
> +
> +/*
> + * Macro for defining register field masks. Bit numbers are 0-based and the mask
> + * includes the high and low bits.
> + *
> + * Local wrapper for GENMASK() to force u32. Do *not* use outside of this
> + * file. Use GENMASK() instead.
> + */
> +#define _MASK(__high, __low) ((u32)GENMASK(__high, __low))
Just pondering here if BUILD_BUG_ON(__low > __high) would
pay itself off.
-Mika
> +
> typedef struct {
> u32 reg;
> } i915_reg_t;
> @@ -4641,18 +4658,18 @@ 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)
>
> #define _PP_CONTROL_1 0xc7204
> #define _PP_CONTROL_2 0xc7304
> #define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \
> _PP_CONTROL_2)
> -#define POWER_CYCLE_DELAY_MASK (0x1f << 4)
> +#define POWER_CYCLE_DELAY_MASK _MASK(8, 4)
> #define POWER_CYCLE_DELAY_SHIFT 4
> -#define VDD_OVERRIDE_FORCE (1 << 3)
> -#define BACKLIGHT_ENABLE (1 << 2)
> -#define PWR_DOWN_ON_RESET (1 << 1)
> -#define PWR_STATE_TARGET (1 << 0)
> +#define VDD_OVERRIDE_FORCE _BIT(3)
> +#define BACKLIGHT_ENABLE _BIT(2)
> +#define PWR_DOWN_ON_RESET _BIT(1)
> +#define PWR_STATE_TARGET _BIT(0)
> /*
> * Indicates that all dependencies of the panel are on:
> *
> @@ -4660,14 +4677,14 @@ enum {
> * - pipe enabled
> * - LVDS/DVOB/DVOC on
> */
> -#define PP_READY (1 << 30)
> +#define PP_READY _BIT(30)
> +#define PP_SEQUENCE_MASK _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 _BIT(27)
> +#define PP_SEQUENCE_STATE_MASK _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)
> @@ -4680,41 +4697,41 @@ enum {
>
> #define _PP_CONTROL 0x61204
> #define PP_CONTROL(pps_idx) _MMIO_PPS(pps_idx, _PP_CONTROL)
> +#define PANEL_UNLOCK_MASK _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 _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_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)
> #define PANEL_PORT_SELECT_SHIFT 30
> -#define PANEL_PORT_SELECT_MASK (3 << 30)
> +#define PANEL_PORT_SELECT_MASK _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 _MASK(28, 16)
> #define PANEL_POWER_UP_DELAY_SHIFT 16
> -#define PANEL_LIGHT_ON_DELAY_MASK 0x1fff
> +#define PANEL_LIGHT_ON_DELAY_MASK _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 _MASK(28, 16)
> #define PANEL_POWER_DOWN_DELAY_SHIFT 16
> -#define PANEL_LIGHT_OFF_DELAY_MASK 0x1fff
> +#define PANEL_LIGHT_OFF_DELAY_MASK _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 _MASK(31, 8)
> #define PP_REFERENCE_DIVIDER_SHIFT 8
> -#define PANEL_POWER_CYCLE_DELAY_MASK 0x1f
> +#define PANEL_POWER_CYCLE_DELAY_MASK _MASK(4, 0)
> #define PANEL_POWER_CYCLE_DELAY_SHIFT 0
>
> /* Panel fitting */
> --
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: introduce macros to define register contents (rev2)
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
` (6 preceding siblings ...)
2019-01-17 12:57 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-01-17 13:13 ` Patchwork
2019-01-17 21:33 ` ✓ Fi.CI.IGT: " Patchwork
8 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-01-17 13:13 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: introduce macros to define register contents (rev2)
URL : https://patchwork.freedesktop.org/series/50513/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5440 -> Patchwork_11969
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/50513/revisions/2/mbox/
Known issues
------------
Here are the changes found in Patchwork_11969 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload-with-fault-injection:
- fi-kbl-7567u: NOTRUN -> DMESG-WARN [fdo#105602] / [fdo#108529] +1
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7567u: NOTRUN -> DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@pm_rpm@module-reload:
- fi-kbl-7567u: NOTRUN -> DMESG-WARN [fdo#108529]
#### Possible fixes ####
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u3: FAIL [fdo#103167] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
Participating hosts (45 -> 41)
------------------------------
Additional (2): fi-kbl-7567u fi-glk-j4005
Missing (6): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-snb-2600
Build changes
-------------
* Linux: CI_DRM_5440 -> Patchwork_11969
CI_DRM_5440: b36a89b5ab74fd49a4369e6df0d2c02bc464a474 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4777: 8614d5eb114a660c3bd7ff77eab8bed53424cd30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11969: 9edbb60abd1bfa0d247e6dd70aac322fcec951dc @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
9edbb60abd1b drm/i915: introduce _FIELD() to define register field values
dc4e11b1eaf0 drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK
b64ca6410c69 drm/i915: introduce _BIT() and _MASK() to define register contents
53519fbcbe40 drm/i915/dp: remove PANEL_POWER_OFF macro and its use
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11969/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: introduce macros to define register contents (rev2)
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
` (7 preceding siblings ...)
2019-01-17 13:13 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-01-17 21:33 ` Patchwork
8 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-01-17 21:33 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: introduce macros to define register contents (rev2)
URL : https://patchwork.freedesktop.org/series/50513/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5440_full -> Patchwork_11969_full
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_11969_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11969_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_11969_full:
### IGT changes ###
#### Warnings ####
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-glk: DMESG-FAIL [fdo#105763] / [fdo#106538] -> FAIL
Known issues
------------
Here are the changes found in Patchwork_11969_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@pi-ringfull-vebox:
- shard-skl: NOTRUN -> FAIL [fdo#103158]
* igt@i915_suspend@shrink:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#107886] / [fdo#109244]
- shard-iclb: NOTRUN -> DMESG-WARN [fdo#107886] / [fdo#109244]
* igt@kms_addfb_basic@too-wide:
- shard-apl: PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +16
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
- shard-iclb: NOTRUN -> DMESG-WARN [fdo#107956]
- shard-skl: NOTRUN -> DMESG-WARN [fdo#107956] +2
* igt@kms_ccs@pipe-a-crc-primary-basic:
- shard-iclb: NOTRUN -> FAIL [fdo#107725]
* igt@kms_color@pipe-c-ctm-max:
- shard-apl: PASS -> FAIL [fdo#108147]
* igt@kms_cursor_crc@cursor-128x42-onscreen:
- shard-apl: PASS -> FAIL [fdo#103232] +4
* igt@kms_cursor_crc@cursor-64x64-sliding:
- shard-glk: PASS -> FAIL [fdo#103232] +1
* igt@kms_draw_crc@draw-method-xrgb2101010-blt-ytiled:
- shard-skl: PASS -> FAIL [fdo#103184]
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: PASS -> FAIL [fdo#105363]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
- shard-apl: PASS -> FAIL [fdo#103167] +1
* igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-wc:
- shard-skl: NOTRUN -> FAIL [fdo#103167] +3
* igt@kms_frontbuffer_tracking@fbc-stridechange:
- shard-skl: NOTRUN -> FAIL [fdo#105683]
* igt@kms_frontbuffer_tracking@fbc-tilingchange:
- shard-skl: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-wc:
- shard-skl: NOTRUN -> FAIL [fdo#105682] +1
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
- shard-iclb: PASS -> FAIL [fdo#103167] +6
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- shard-skl: NOTRUN -> FAIL [fdo#103191] / [fdo#107362]
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#106885]
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: NOTRUN -> FAIL [fdo#108145] +1
* igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
- shard-skl: PASS -> FAIL [fdo#107815] / [fdo#108145]
* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- shard-iclb: PASS -> FAIL [fdo#103166] +3
* igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
- shard-glk: PASS -> FAIL [fdo#103166] +1
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-apl: PASS -> DMESG-FAIL [fdo#108950]
* igt@kms_setmode@basic:
- shard-apl: PASS -> FAIL [fdo#99912]
* igt@pm_rpm@modeset-lpsp-stress-no-wait:
- shard-iclb: PASS -> INCOMPLETE [fdo#108840]
* igt@sw_sync@sync_busy_fork:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#108889]
#### Possible fixes ####
* igt@debugfs_test@read_all_entries_display_off:
- shard-skl: INCOMPLETE [fdo#104108] -> PASS
* igt@kms_color@pipe-a-gamma:
- shard-skl: FAIL [fdo#104782] -> PASS
* igt@kms_color@pipe-a-legacy-gamma:
- shard-apl: FAIL [fdo#104782] / [fdo#108145] -> PASS
* igt@kms_cursor_crc@cursor-128x128-suspend:
- shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS
* igt@kms_cursor_crc@cursor-128x42-sliding:
- shard-glk: FAIL [fdo#103232] -> PASS +2
* igt@kms_cursor_crc@cursor-64x21-random:
- shard-apl: FAIL [fdo#103232] -> PASS +1
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
- shard-hsw: FAIL [fdo#105767] -> PASS
* igt@kms_draw_crc@draw-method-rgb565-mmap-cpu-xtiled:
- shard-skl: FAIL [fdo#103184] -> PASS
* igt@kms_flip_event_leak:
- shard-kbl: DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +4
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-apl: FAIL [fdo#103167] -> PASS +1
- shard-iclb: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
- shard-glk: FAIL [fdo#103167] -> PASS +2
* igt@kms_frontbuffer_tracking@fbc-2p-rte:
- shard-glk: FAIL [fdo#103167] / [fdo#105682] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
- shard-skl: FAIL [fdo#103167] -> PASS +2
* igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-render:
- shard-skl: FAIL [fdo#105682] -> PASS +1
* igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-apl: FAIL [fdo#103166] -> PASS +1
* igt@pm_rpm@gem-execbuf-stress:
- shard-skl: INCOMPLETE [fdo#107803] / [fdo#107807] -> PASS
#### Warnings ####
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-apl: FAIL [fdo#108948] -> DMESG-FAIL [fdo#103558] / [fdo#105602]
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-iclb: FAIL -> DMESG-FAIL [fdo#107724]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
[fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
[fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
[fdo#107886]: https://bugs.freedesktop.org/show_bug.cgi?id=107886
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
[fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
[fdo#108889]: https://bugs.freedesktop.org/show_bug.cgi?id=108889
[fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
[fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
[fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109275]: https://bugs.freedesktop.org/show_bug.cgi?id=109275
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109286]: https://bugs.freedesktop.org/show_bug.cgi?id=109286
[fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_5440 -> Patchwork_11969
CI_DRM_5440: b36a89b5ab74fd49a4369e6df0d2c02bc464a474 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4777: 8614d5eb114a660c3bd7ff77eab8bed53424cd30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11969: 9edbb60abd1bfa0d247e6dd70aac322fcec951dc @ 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_11969/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use
2019-01-17 12:25 ` Mika Kuoppala
@ 2019-01-18 13:42 ` Jani Nikula
0 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2019-01-18 13:42 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
On Thu, 17 Jan 2019, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Jani Nikula <jani.nikula@intel.com> writes:
>
>> It's superfluous.
>
> One could argue that it has a minor documentative purpose.
> But there is comment for that.
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Thanks, pushed this one.
BR,
Jani.
>
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 1 -
>> drivers/gpu/drm/i915/intel_dp.c | 3 +--
>> 2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 9a1340cfda6c..93cbd057c07a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4687,7 +4687,6 @@ enum {
>> #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 _PP_ON_DELAYS 0x61208
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 808ccdae15b8..f7d5314e3395 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1000,8 +1000,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.20.1
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 16+ messages in thread
* Re: [PATCH v2 0/4] drm/i915: introduce macros to define register contents
2019-01-17 12:52 ` [PATCH v2 0/4] drm/i915: introduce macros to define register contents Chris Wilson
@ 2019-02-27 11:01 ` Jani Nikula
2019-02-27 11:20 ` Chris Wilson
0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2019-02-27 11:01 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Thu, 17 Jan 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2019-01-17 12:13:59)
>> The v1 [1] kind of died down because the FIELD_PREP() build-time checks
>> were lost as it didn't evaluate to integer constant expression. I looked
>> at this again, and managed to include the checks in the local copy by
>> using BUILD_BUG_ON_ZERO() instead.
>>
>> On the naming bikeshedding department, I noticed a clash with regmap.h
>> REG_FIELD() and, since it all looked pretty verbose anyway, decided to
>> see if local _BIT(), _MASK(), and _FIELD() would stick.
>
> MMIO_BIT, MMIO_MASK, MMIO_FIELD
> #define MMIO_PREP(mask, val) FIELD_PREP(mask, val)
This would mean two wrappers/implementations for FIELD_PREP(), plus the
original.
> ?
>
> The leading underscore and the inconsistency with FIELD_PREP is getting
> to me.
Fair enough.
So we need constant expression and/or u32 implementations or wrappers
for BIT(), GENMASK(), and FIELD_PREP(). Do we want to use wrappers for
FIELD_PREP() and FIELD_GET() in code outside of the macro definitions?
Might be nice for consistency.
I'm not fond of the MMIO prefix here, mostly because these are not
strictly related to MMIO. Feels like conflating too much.
BIT
-> REG_BIT
-> I915_BIT
GENMASK
-> REG_MASK
-> REG_GENMASK
-> I915_MASK
-> I915_GENMASK
FIELD_PREP
-> REG_BITFIELD
-> REG_FIELD_PREP
-> I915_FIELD
-> I915_FIELD_PREP
-> I915_BITFIELD
FIELD_GET (not strictly needed)
-> REG_FIELD_GET
-> I915_FIELD_GET
Dunno, I kind of liked the short underscored versions, but if we're
going to make them longer, why not just prefix all the originals with
REG_ or I915_ and be done with it?
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] 16+ messages in thread
* Re: [PATCH v2 0/4] drm/i915: introduce macros to define register contents
2019-02-27 11:01 ` Jani Nikula
@ 2019-02-27 11:20 ` Chris Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2019-02-27 11:20 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
Quoting Jani Nikula (2019-02-27 11:01:58)
> On Thu, 17 Jan 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Jani Nikula (2019-01-17 12:13:59)
> >> The v1 [1] kind of died down because the FIELD_PREP() build-time checks
> >> were lost as it didn't evaluate to integer constant expression. I looked
> >> at this again, and managed to include the checks in the local copy by
> >> using BUILD_BUG_ON_ZERO() instead.
> >>
> >> On the naming bikeshedding department, I noticed a clash with regmap.h
> >> REG_FIELD() and, since it all looked pretty verbose anyway, decided to
> >> see if local _BIT(), _MASK(), and _FIELD() would stick.
> >
> > MMIO_BIT, MMIO_MASK, MMIO_FIELD
> > #define MMIO_PREP(mask, val) FIELD_PREP(mask, val)
>
> This would mean two wrappers/implementations for FIELD_PREP(), plus the
> original.
>
> > ?
> >
> > The leading underscore and the inconsistency with FIELD_PREP is getting
> > to me.
>
> Fair enough.
>
> So we need constant expression and/or u32 implementations or wrappers
> for BIT(), GENMASK(), and FIELD_PREP(). Do we want to use wrappers for
> FIELD_PREP() and FIELD_GET() in code outside of the macro definitions?
> Might be nice for consistency.
>
> I'm not fond of the MMIO prefix here, mostly because these are not
> strictly related to MMIO. Feels like conflating too much.
>
> BIT
> -> REG_BIT
> -> I915_BIT
>
> GENMASK
> -> REG_MASK
> -> REG_GENMASK
> -> I915_MASK
> -> I915_GENMASK
>
> FIELD_PREP
> -> REG_BITFIELD
> -> REG_FIELD_PREP
> -> I915_FIELD
> -> I915_FIELD_PREP
> -> I915_BITFIELD
>
> FIELD_GET (not strictly needed)
> -> REG_FIELD_GET
> -> I915_FIELD_GET
>
> Dunno, I kind of liked the short underscored versions, but if we're
> going to make them longer, why not just prefix all the originals with
> REG_ or I915_ and be done with it?
REG_* works for me. I prefer REG over I915 as I think using I915 implies
that this is something special for our hw, whereas we are just trying to
interact with registers. And I think there will be no harm in us making
this include/linux/regfield.h with a nod to bitfield.h
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-02-27 11:20 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17 12:13 [PATCH v2 0/4] drm/i915: introduce macros to define register contents Jani Nikula
2019-01-17 12:14 ` [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use Jani Nikula
2019-01-17 12:25 ` Mika Kuoppala
2019-01-18 13:42 ` Jani Nikula
2019-01-17 12:14 ` [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents Jani Nikula
2019-01-17 13:02 ` Mika Kuoppala
2019-01-17 12:14 ` [PATCH v2 3/4] drm/i915: deprecate _SHIFT in favor of FIELD_GET() and _MASK Jani Nikula
2019-01-17 12:14 ` [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values Jani Nikula
2019-01-17 12:58 ` Chris Wilson
2019-01-17 12:52 ` [PATCH v2 0/4] drm/i915: introduce macros to define register contents Chris Wilson
2019-02-27 11:01 ` Jani Nikula
2019-02-27 11:20 ` Chris Wilson
2019-01-17 12:54 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents (rev2) Patchwork
2019-01-17 12:57 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-01-17 13:13 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-17 21:33 ` ✓ 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.