[PATCH v2 0/4] drm/i915: introduce macros to define register contents

[PATCH v2 0/4] drm/i915: introduce macros to define register contents

[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

[PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use

[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

Re: [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use

[Mika Kuoppala]

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

Re: [PATCH v2 1/4] drm/i915/dp: remove PANEL_POWER_OFF macro and its use

[Jani Nikula]

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

[PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents

[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

Re: [PATCH v2 2/4] drm/i915: introduce _BIT() and _MASK() to define register contents

[Mika Kuoppala]

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

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

[Jani Nikula]

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

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

FIELD_PREP() can get more verbose than simply shifting in place, but it
does provide masking to ensure we don't overflow the mask, something we
usually don't bother with currently.

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

[PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values

[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

Re: [PATCH v2 4/4] drm/i915: introduce _FIELD() to define register field values

[Chris Wilson]

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

Re: [PATCH v2 0/4] drm/i915: introduce macros to define register contents

[Chris Wilson]

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

Re: [PATCH v2 0/4] drm/i915: introduce macros to define register contents

[Jani Nikula]

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

Re: [PATCH v2 0/4] drm/i915: introduce macros to define register contents

[Chris Wilson]

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: introduce macros to define register contents (rev2)

[Patchwork]

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

✗ Fi.CI.SPARSE: warning for drm/i915: introduce macros to define register contents (rev2)

[Patchwork]

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

✓ Fi.CI.BAT: success for drm/i915: introduce macros to define register contents (rev2)

[Patchwork]

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

✓ Fi.CI.IGT: success for drm/i915: introduce macros to define register contents (rev2)

[Patchwork]

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