[RFC] drm/i915: Introduce documentation for register subfields

[Chris Wilson <chris@chris-wilson.co.uk>]

An interesting point was raised in some recent patches to document the
various widths of the register subfields. I disagreed with that patch in
that it transformed illegal values into some random potentially harmful
valid value (and not transforming an invalid value would end up writing
bits in other subfields, so equally bad). Most of our register usage is
with static values so unlikely to be of concern, but for those we can
document the register subfields and provide compile time checking.

The resulting error message is fairly impententrable though:

n file included from include/uapi/linux/stddef.h:1:0,
                 from include/linux/stddef.h:4,
                 from ./include/uapi/linux/posix_types.h:4,
                 from include/uapi/linux/types.h:13,
                 from include/linux/types.h:5,
                 from include/linux/mod_devicetable.h:11,
                 from include/linux/i2c.h:29,
                 from drivers/gpu/drm/i915/intel_dp.c:28:
In function ‘intel_dp_autotest_edid’,
    inlined from ‘intel_dp_handle_test_request’ at drivers/gpu/drm/i915/intel_dp.c:4230:14,
    inlined from ‘intel_dp_detect’ at drivers/gpu/drm/i915/intel_dp.c:4635:4:
include/linux/compiler.h:429:38: error: call to ‘__compiletime_assert_4173’ declared with attribute error: BUILD_BUG_ON failed: (5) & -(1<<2)
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                      ^
include/linux/compiler.h:412:4: note: in definition of macro ‘__compiletime_assert’
    prefix ## suffix();    \
    ^
include/linux/compiler.h:429:2: note: in expansion of macro ‘_compiletime_assert’
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
  ^
include/linux/bug.h:50:37: note: in expansion of macro ‘compiletime_assert’
 #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                     ^
include/linux/bug.h:74:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  ^
drivers/gpu/drm/i915/intel_dp.c:45:68: note: in expansion of macro ‘BUILD_BUG_ON’
 #define __FIELD__(x, shift, width) ({ if (__builtin_constant_p(x)) BUILD_BUG_ON((x) & -(1<<width)); ((x) << (shift)); })
                                                                    ^
drivers/gpu/drm/i915/intel_dp.c:47:32: note: in expansion of macro ‘__FIELD__’
 #define INTEL_DP_RESOLUTION(x) __FIELD__(x, INTEL_DP_RESOLUTION_SHIFT, 2)
                                ^
drivers/gpu/drm/i915/intel_dp.c:51:36: note: in expansion of macro ‘INTEL_DP_RESOLUTION’
 #define INTEL_DP_RESOLUTION_BROKEN INTEL_DP_RESOLUTION(5)
                                    ^
drivers/gpu/drm/i915/intel_dp.c:4173:36: note: in expansion of macro ‘INTEL_DP_RESOLUTION_BROKEN’
   intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_BROKEN;

We can extend the macro to WARN_ON for instances of runtime violation -
maybe useful, but it surely would bulk our code out dramatically.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f52eef138247..fe44b06adeb4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -42,10 +42,12 @@
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /* Compliance test status bits  */
-#define INTEL_DP_RESOLUTION_SHIFT_MASK	0
-#define INTEL_DP_RESOLUTION_PREFERRED	(1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
-#define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
-#define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define __FIELD__(x, shift, width) ({ if (__builtin_constant_p(x)) BUILD_BUG_ON((x) & -(1<<width)); ((x) << (shift)); })
+#define INTEL_DP_RESOLUTION_SHIFT 0
+#define INTEL_DP_RESOLUTION(x) __FIELD__(x, INTEL_DP_RESOLUTION_SHIFT, 2)
+#define INTEL_DP_RESOLUTION_PREFERRED	INTEL_DP_RESOLUTION(1)
+#define INTEL_DP_RESOLUTION_STANDARD	INTEL_DP_RESOLUTION(2)
+#define INTEL_DP_RESOLUTION_FAILSAFE	INTEL_DP_RESOLUTION(3)
 
 struct dp_link_dpll {
 	int link_bw;
-- 
2.1.4

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