public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP
@ 2022-10-31  5:10 Ashutosh Dixit
  0 siblings, 0 replies; 7+ messages in thread
From: Ashutosh Dixit @ 2022-10-31  5:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Andi Shyti, llvm, ndesaulniers, dri-devel

FIELD_PREP and REG_FIELD_PREP have checks requiring a compile time constant
mask. When the mask comes in as the argument of a function these checks can
can fail depending on the compiler (gcc vs clang), optimization level,
etc. Use a simpler local version of FIELD_PREP which skips these
checks. The checks are not needed because the mask is formed using
REG_GENMASK (so is actually a compile time constant).

Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7354
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_hwmon.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 9e97814930254..a3ec9a73a4e49 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -62,6 +62,12 @@ struct i915_hwmon {
 	int scl_shift_time;
 };
 
+/* FIELD_PREP and REG_FIELD_PREP require a compile time constant mask */
+static u32 hwm_field_prep(u32 mask, u32 val)
+{
+	return (val << __bf_shf(mask)) & mask;
+}
+
 static void
 hwm_locked_with_pm_intel_uncore_rmw(struct hwm_drvdata *ddat,
 				    i915_reg_t reg, u32 clear, u32 set)
@@ -112,7 +118,7 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
 	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
 
 	bits_to_clear = field_msk;
-	bits_to_set = FIELD_PREP(field_msk, nval);
+	bits_to_set = hwm_field_prep(field_msk, nval);
 
 	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
 					    bits_to_clear, bits_to_set);
-- 
2.38.0


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

* [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP
@ 2022-10-31 17:26 Ashutosh Dixit
  2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/hwmon: Don't use FIELD_PREP (rev2) Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Ashutosh Dixit @ 2022-10-31 17:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: Andi Shyti, llvm, ndesaulniers, dri-devel

FIELD_PREP and REG_FIELD_PREP have checks requiring a compile time constant
mask. When the mask comes in as the argument of a function these checks can
can fail depending on the compiler (gcc vs clang), optimization level,
etc. Use a simpler version of FIELD_PREP which skips these checks. The
checks are not needed because the mask is formed using REG_GENMASK (so is
actually a compile time constant).

v2: Split REG_FIELD_PREP into a macro with checks and one without and use
    the one without checks in i915_hwmon.c (Gwan-gyeong Mun)

Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7354
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_hwmon.c    |  2 +-
 drivers/gpu/drm/i915/i915_reg_defs.h | 17 +++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 9e97814930254..ae435b035229a 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -112,7 +112,7 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
 	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
 
 	bits_to_clear = field_msk;
-	bits_to_set = FIELD_PREP(field_msk, nval);
+	bits_to_set = __REG_FIELD_PREP(field_msk, nval);
 
 	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
 					    bits_to_clear, bits_to_set);
diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
index f1859046a9c48..dddacc8d48928 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -67,12 +67,17 @@
  *
  * @return: @__val masked and shifted into the field defined by @__mask.
  */
-#define REG_FIELD_PREP(__mask, __val)						\
-	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) +	\
-	       BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) +		\
-	       BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) +		\
-	       BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
-	       BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0))))
+#define __REG_FIELD_PREP_CHK(__mask, __val) \
+	(BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
+	 BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+	 BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+	 BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
+
+#define __REG_FIELD_PREP(__mask, __val) \
+	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask))))
+
+#define REG_FIELD_PREP(__mask, __val) \
+	(__REG_FIELD_PREP(__mask, __val) + __REG_FIELD_PREP_CHK(__mask, __val))
 
 /**
  * REG_FIELD_GET() - Extract a u32 bitfield value
-- 
2.38.0


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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/hwmon: Don't use FIELD_PREP (rev2)
  2022-10-31 17:26 [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Ashutosh Dixit
@ 2022-10-31 21:29 ` Patchwork
  2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2022-10-31 21:29 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/hwmon: Don't use FIELD_PREP (rev2)
URL   : https://patchwork.freedesktop.org/series/110301/
State : warning

== Summary ==

Error: dim checkpatch failed
6a7780f724ae drm/i915/hwmon: Don't use FIELD_PREP
-:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__mask' - possible side-effects?
#46: FILE: drivers/gpu/drm/i915/i915_reg_defs.h:70:
+#define __REG_FIELD_PREP_CHK(__mask, __val) \
+	(BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
+	 BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+	 BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+	 BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))

-:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__val' - possible side-effects?
#46: FILE: drivers/gpu/drm/i915/i915_reg_defs.h:70:
+#define __REG_FIELD_PREP_CHK(__mask, __val) \
+	(BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
+	 BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+	 BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+	 BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))

-:50: WARNING:LONG_LINE: line length of 121 exceeds 100 columns
#50: FILE: drivers/gpu/drm/i915/i915_reg_defs.h:74:
+	 BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))

-:52: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__mask' - possible side-effects?
#52: FILE: drivers/gpu/drm/i915/i915_reg_defs.h:76:
+#define __REG_FIELD_PREP(__mask, __val) \
+	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask))))

-:55: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__mask' - possible side-effects?
#55: FILE: drivers/gpu/drm/i915/i915_reg_defs.h:79:
+#define REG_FIELD_PREP(__mask, __val) \
+	(__REG_FIELD_PREP(__mask, __val) + __REG_FIELD_PREP_CHK(__mask, __val))

-:55: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__val' - possible side-effects?
#55: FILE: drivers/gpu/drm/i915/i915_reg_defs.h:79:
+#define REG_FIELD_PREP(__mask, __val) \
+	(__REG_FIELD_PREP(__mask, __val) + __REG_FIELD_PREP_CHK(__mask, __val))

total: 0 errors, 1 warnings, 5 checks, 31 lines checked



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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/hwmon: Don't use FIELD_PREP (rev2)
  2022-10-31 17:26 [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Ashutosh Dixit
  2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/hwmon: Don't use FIELD_PREP (rev2) Patchwork
@ 2022-10-31 21:29 ` Patchwork
  2022-10-31 21:53 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  2022-11-01 10:58 ` [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Jani Nikula
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2022-10-31 21:29 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/hwmon: Don't use FIELD_PREP (rev2)
URL   : https://patchwork.freedesktop.org/series/110301/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/hwmon: Don't use FIELD_PREP (rev2)
  2022-10-31 17:26 [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Ashutosh Dixit
  2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/hwmon: Don't use FIELD_PREP (rev2) Patchwork
  2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2022-10-31 21:53 ` Patchwork
  2022-11-01 10:58 ` [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Jani Nikula
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2022-10-31 21:53 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 12275 bytes --]

== Series Details ==

Series: drm/i915/hwmon: Don't use FIELD_PREP (rev2)
URL   : https://patchwork.freedesktop.org/series/110301/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12325 -> Patchwork_110301v2
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/index.html

Participating hosts (40 -> 40)
------------------------------

  Additional (2): fi-kbl-soraka fi-rkl-11600 
  Missing    (2): fi-cml-u2 fi-hsw-4770 

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gem_contexts:
    - fi-kbl-soraka:      NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-kbl-soraka/igt@i915_selftest@live@gem_contexts.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-apl-guc:         [PASS][2] -> [INCOMPLETE][3] ([i915#7073])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/fi-apl-guc/igt@core_hotunplug@unbind-rebind.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-apl-guc/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_gttfill@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][4] ([fdo#109271]) +8 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-kbl-soraka/igt@gem_exec_gttfill@basic.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#2190])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#2190])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4613]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][8] ([i915#4613]) +3 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][9] ([i915#3282])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-rkl-11600:       NOTRUN -> [SKIP][10] ([i915#3012])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][11] ([i915#1886])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@mman:
    - fi-rkl-guc:         [PASS][12] -> [INCOMPLETE][13] ([i915#6794])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/fi-rkl-guc/igt@i915_selftest@live@mman.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-guc/igt@i915_selftest@live@mman.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       NOTRUN -> [INCOMPLETE][14] ([i915#4817])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][15] ([fdo#111827]) +7 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][16] ([fdo#109271] / [fdo#111827]) +7 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-kbl-soraka/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - fi-rkl-11600:       NOTRUN -> [SKIP][17] ([i915#4103])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][18] ([fdo#109285] / [i915#4098])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-2:
    - fi-bdw-5557u:       [PASS][19] -> [INCOMPLETE][20] ([i915#146])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/fi-bdw-5557u/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-2.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-bdw-5557u/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-2.html

  * igt@kms_psr@primary_page_flip:
    - fi-rkl-11600:       NOTRUN -> [SKIP][21] ([i915#1072]) +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@kms_psr@primary_page_flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][22] ([i915#3555] / [i915#4098])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][23] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][24] ([fdo#109295] / [i915#3301] / [i915#3708])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0@lmem0:
    - {bat-dg2-11}:       [DMESG-FAIL][25] ([fdo#103375]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/bat-dg2-11/igt@gem_exec_suspend@basic-s0@lmem0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/bat-dg2-11/igt@gem_exec_suspend@basic-s0@lmem0.html

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-adlm-1}:       [DMESG-WARN][27] ([i915#2867]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@gem_exec_suspend@basic-s3@lmem0:
    - {bat-dg2-11}:       [FAIL][29] ([fdo#103375]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/bat-dg2-11/igt@gem_exec_suspend@basic-s3@lmem0.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/bat-dg2-11/igt@gem_exec_suspend@basic-s3@lmem0.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - {bat-rplp-1}:       [DMESG-WARN][31] ([i915#2867]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/bat-rplp-1/igt@gem_exec_suspend@basic-s3@smem.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/bat-rplp-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [FAIL][33] ([i915#6298]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-dp-3:
    - {bat-dg2-11}:       [FAIL][35] -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/bat-dg2-11/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-dp-3.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/bat-dg2-11/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-dp-3.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-dp-3:
    - {bat-dg2-11}:       [FAIL][37] ([i915#6818]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/bat-dg2-11/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-dp-3.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/bat-dg2-11/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-dp-3.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-2:
    - fi-icl-u2:          [DMESG-WARN][39] ([i915#2867]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12325/fi-icl-u2/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-2.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/fi-icl-u2/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-2.html

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

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5537]: https://gitlab.freedesktop.org/drm/intel/issues/5537
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#6818]: https://gitlab.freedesktop.org/drm/intel/issues/6818
  [i915#7073]: https://gitlab.freedesktop.org/drm/intel/issues/7073


Build changes
-------------

  * Linux: CI_DRM_12325 -> Patchwork_110301v2

  CI-20190529: 20190529
  CI_DRM_12325: 1a90222aa5e5bb86ffcbde5ba9611659a23f0df6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7032: 372c56225e12578a7a4a6bcc5b79eb40b643fcde @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_110301v2: 1a90222aa5e5bb86ffcbde5ba9611659a23f0df6 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

c346baa59d6e drm/i915/hwmon: Don't use FIELD_PREP

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110301v2/index.html

[-- Attachment #2: Type: text/html, Size: 14079 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP
  2022-10-31 17:26 [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Ashutosh Dixit
                   ` (2 preceding siblings ...)
  2022-10-31 21:53 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2022-11-01 10:58 ` Jani Nikula
  2022-11-02  6:17   ` Dixit, Ashutosh
  3 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2022-11-01 10:58 UTC (permalink / raw)
  To: Ashutosh Dixit, intel-gfx; +Cc: ndesaulniers, llvm, Andi Shyti, dri-devel

On Mon, 31 Oct 2022, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> FIELD_PREP and REG_FIELD_PREP have checks requiring a compile time constant
> mask. When the mask comes in as the argument of a function these checks can
> can fail depending on the compiler (gcc vs clang), optimization level,
> etc. Use a simpler version of FIELD_PREP which skips these checks. The
> checks are not needed because the mask is formed using REG_GENMASK (so is
> actually a compile time constant).
>
> v2: Split REG_FIELD_PREP into a macro with checks and one without and use
>     the one without checks in i915_hwmon.c (Gwan-gyeong Mun)

I frankly think you're solving the wrong problem here. See [1].

BR,
Jani.

[1] https://lore.kernel.org/r/87leov7yix.fsf@intel.com

>
> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7354
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_hwmon.c    |  2 +-
>  drivers/gpu/drm/i915/i915_reg_defs.h | 17 +++++++++++------
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 9e97814930254..ae435b035229a 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -112,7 +112,7 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
>  	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
>  
>  	bits_to_clear = field_msk;
> -	bits_to_set = FIELD_PREP(field_msk, nval);
> +	bits_to_set = __REG_FIELD_PREP(field_msk, nval);
>  
>  	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
>  					    bits_to_clear, bits_to_set);
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
> index f1859046a9c48..dddacc8d48928 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -67,12 +67,17 @@
>   *
>   * @return: @__val masked and shifted into the field defined by @__mask.
>   */
> -#define REG_FIELD_PREP(__mask, __val)						\
> -	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) +	\
> -	       BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) +		\
> -	       BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) +		\
> -	       BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
> -	       BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0))))
> +#define __REG_FIELD_PREP_CHK(__mask, __val) \
> +	(BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
> +	 BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
> +	 BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
> +	 BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
> +
> +#define __REG_FIELD_PREP(__mask, __val) \
> +	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask))))
> +
> +#define REG_FIELD_PREP(__mask, __val) \
> +	(__REG_FIELD_PREP(__mask, __val) + __REG_FIELD_PREP_CHK(__mask, __val))
>  
>  /**
>   * REG_FIELD_GET() - Extract a u32 bitfield value

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP
  2022-11-01 10:58 ` [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Jani Nikula
@ 2022-11-02  6:17   ` Dixit, Ashutosh
  0 siblings, 0 replies; 7+ messages in thread
From: Dixit, Ashutosh @ 2022-11-02  6:17 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Andi Shyti, intel-gfx, llvm, ndesaulniers, dri-devel

On Tue, 01 Nov 2022 03:58:13 -0700, Jani Nikula wrote:
>
> On Mon, 31 Oct 2022, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> > FIELD_PREP and REG_FIELD_PREP have checks requiring a compile time constant
> > mask. When the mask comes in as the argument of a function these checks can
> > can fail depending on the compiler (gcc vs clang), optimization level,
> > etc. Use a simpler version of FIELD_PREP which skips these checks. The
> > checks are not needed because the mask is formed using REG_GENMASK (so is
> > actually a compile time constant).
> >
> > v2: Split REG_FIELD_PREP into a macro with checks and one without and use
> >     the one without checks in i915_hwmon.c (Gwan-gyeong Mun)
>
> I frankly think you're solving the wrong problem here. See [1].

We can consider the sort of refactoring suggested in [1] in the future,
right now I thought I'll offer what in my opinion is the correct way to fix
the clang compile break incrementally with the current code. But otherwise
feel free to go with whatever you think is the correct course of action for
this issue. Even if we don't fix the issue the clang guys will (as they
have in the past).

Thanks.
--
Ashutosh

> [1] https://lore.kernel.org/r/87leov7yix.fsf@intel.com
>
> >
> > Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7354
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_hwmon.c    |  2 +-
> >  drivers/gpu/drm/i915/i915_reg_defs.h | 17 +++++++++++------
> >  2 files changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > index 9e97814930254..ae435b035229a 100644
> > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > @@ -112,7 +112,7 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> >	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
> >
> >	bits_to_clear = field_msk;
> > -	bits_to_set = FIELD_PREP(field_msk, nval);
> > +	bits_to_set = __REG_FIELD_PREP(field_msk, nval);
> >
> >	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
> >					    bits_to_clear, bits_to_set);
> > diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
> > index f1859046a9c48..dddacc8d48928 100644
> > --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> > +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> > @@ -67,12 +67,17 @@
> >   *
> >   * @return: @__val masked and shifted into the field defined by @__mask.
> >   */
> > -#define REG_FIELD_PREP(__mask, __val)						\
> > -	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) +	\
> > -	       BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) +		\
> > -	       BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) +		\
> > -	       BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
> > -	       BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0))))
> > +#define __REG_FIELD_PREP_CHK(__mask, __val) \
> > +	(BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
> > +	 BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
> > +	 BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
> > +	 BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
> > +
> > +#define __REG_FIELD_PREP(__mask, __val) \
> > +	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask))))
> > +
> > +#define REG_FIELD_PREP(__mask, __val) \
> > +	(__REG_FIELD_PREP(__mask, __val) + __REG_FIELD_PREP_CHK(__mask, __val))
> >
> >  /**
> >   * REG_FIELD_GET() - Extract a u32 bitfield value

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

end of thread, other threads:[~2022-11-02  6:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-31 17:26 [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Ashutosh Dixit
2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/hwmon: Don't use FIELD_PREP (rev2) Patchwork
2022-10-31 21:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-10-31 21:53 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-11-01 10:58 ` [Intel-gfx] [PATCH] drm/i915/hwmon: Don't use FIELD_PREP Jani Nikula
2022-11-02  6:17   ` Dixit, Ashutosh
  -- strict thread matches above, loose matches on Subject: below --
2022-10-31  5:10 Ashutosh Dixit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox