* [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON
@ 2017-04-11 13:57 Michal Wajdeczko
2017-04-11 15:28 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-04-12 11:13 ` [RFC] " Tvrtko Ursulin
0 siblings, 2 replies; 5+ messages in thread
From: Michal Wajdeczko @ 2017-04-11 13:57 UTC (permalink / raw)
To: intel-gfx
This is slightly weaker version of MAYBE_BUILD_BUG_ON that allows
us to avoid adding implicit BUG but still detect as much as possible
during the build. With this new macro we can fix the problem with
GCC 4.4.7 that wrongly triggers build break in wait_for_atomic()
when invoked with non-const parameter.
Fixes: 1d1a9774 ("drm/i915: Extend intel_wait_for_register_fw() with fast timeout")
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.h | 9 +++++++++
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 5a49487..ce23cf1 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -42,6 +42,15 @@
#define GEM_DEBUG_BUG_ON(expr)
#endif
+#define GEM_MAYBE_BUILD_BUG_ON(expr) \
+ do { \
+ if (__builtin_constant_p((expr))) \
+ BUILD_BUG_ON(expr); \
+ else \
+ GEM_BUG_ON(expr); \
+ } while (0)
+
+
#define I915_NUM_ENGINES 5
#endif /* __I915_GEM_H__ */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7bc0c25..ce50ec4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -88,7 +88,7 @@
int cpu, ret, timeout = (US) * 1000; \
u64 base; \
_WAIT_FOR_ATOMIC_CHECK(ATOMIC); \
- BUILD_BUG_ON((US) > 50000); \
+ GEM_MAYBE_BUILD_BUG_ON((US) > 50000); \
if (!(ATOMIC)) { \
preempt_disable(); \
cpu = smp_processor_id(); \
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON 2017-04-11 13:57 [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON Michal Wajdeczko @ 2017-04-11 15:28 ` Patchwork 2017-04-12 11:13 ` [RFC] " Tvrtko Ursulin 1 sibling, 0 replies; 5+ messages in thread From: Patchwork @ 2017-04-11 15:28 UTC (permalink / raw) To: Michal Wajdeczko; +Cc: intel-gfx == Series Details == Series: drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON URL : https://patchwork.freedesktop.org/series/22857/ State : success == Summary == Series 22857v1 drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON https://patchwork.freedesktop.org/api/1.0/series/22857/revisions/1/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:433s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:429s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:574s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:480s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:481s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:422s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:490s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:467s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:452s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:564s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:461s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:576s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:460s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:492s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:430s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:526s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:403s fi-bxt-j4205 failed to collect. IGT log at Patchwork_4474/fi-bxt-j4205/igt.log 1a8653e657e1154a337956033af17740ef5f9dda drm-tip: 2017y-04m-11d-14h-18m-13s UTC integration manifest 205ac62 drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4474/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON 2017-04-11 13:57 [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON Michal Wajdeczko 2017-04-11 15:28 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2017-04-12 11:13 ` Tvrtko Ursulin 2017-04-12 11:29 ` Chris Wilson 2017-04-12 12:58 ` Michal Wajdeczko 1 sibling, 2 replies; 5+ messages in thread From: Tvrtko Ursulin @ 2017-04-12 11:13 UTC (permalink / raw) To: Michal Wajdeczko, intel-gfx On 11/04/2017 14:57, Michal Wajdeczko wrote: > This is slightly weaker version of MAYBE_BUILD_BUG_ON that allows > us to avoid adding implicit BUG but still detect as much as possible > during the build. With this new macro we can fix the problem with > GCC 4.4.7 that wrongly triggers build break in wait_for_atomic() > when invoked with non-const parameter. > > Fixes: 1d1a9774 ("drm/i915: Extend intel_wait_for_register_fw() with fast timeout") > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem.h | 9 +++++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > index 5a49487..ce23cf1 100644 > --- a/drivers/gpu/drm/i915/i915_gem.h > +++ b/drivers/gpu/drm/i915/i915_gem.h > @@ -42,6 +42,15 @@ > #define GEM_DEBUG_BUG_ON(expr) > #endif > > +#define GEM_MAYBE_BUILD_BUG_ON(expr) \ > + do { \ > + if (__builtin_constant_p((expr))) \ > + BUILD_BUG_ON(expr); \ > + else \ > + GEM_BUG_ON(expr); \ > + } while (0) > + > + > #define I915_NUM_ENGINES 5 > > #endif /* __I915_GEM_H__ */ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 7bc0c25..ce50ec4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -88,7 +88,7 @@ > int cpu, ret, timeout = (US) * 1000; \ > u64 base; \ > _WAIT_FOR_ATOMIC_CHECK(ATOMIC); \ > - BUILD_BUG_ON((US) > 50000); \ > + GEM_MAYBE_BUILD_BUG_ON((US) > 50000); \ > if (!(ATOMIC)) { \ > preempt_disable(); \ > cpu = smp_processor_id(); \ > The choice is between this and this: -#define wait_for_atomic(COND, MS) _wait_for_atomic((COND), (MS) * 1000, 1) -#define wait_for_atomic_us(COND, US) _wait_for_atomic((COND), (US), 1) +#define wait_for_atomic(COND, MS) \ +({ \ + int ret__; \ + BUILD_BUG_ON((MS) > 50); \ + ret__ = _wait_for_atomic((COND), (MS) * 1000, 1); \ + ret__; \ +}) + +#define wait_for_atomic_us(COND, US) \ +({ \ + int ret__; \ + BUILD_BUG_ON((US) > 50000); \ + ret__ = _wait_for_atomic((COND), (US), 1); \ + ret__; \ +}) Correct? Both would fix the GCC 4.4 issue? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON 2017-04-12 11:13 ` [RFC] " Tvrtko Ursulin @ 2017-04-12 11:29 ` Chris Wilson 2017-04-12 12:58 ` Michal Wajdeczko 1 sibling, 0 replies; 5+ messages in thread From: Chris Wilson @ 2017-04-12 11:29 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx On Wed, Apr 12, 2017 at 12:13:51PM +0100, Tvrtko Ursulin wrote: > > On 11/04/2017 14:57, Michal Wajdeczko wrote: > > This is slightly weaker version of MAYBE_BUILD_BUG_ON that allows > > us to avoid adding implicit BUG but still detect as much as possible > > during the build. With this new macro we can fix the problem with > > GCC 4.4.7 that wrongly triggers build break in wait_for_atomic() > > when invoked with non-const parameter. > > > > Fixes: 1d1a9774 ("drm/i915: Extend intel_wait_for_register_fw() with fast timeout") > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_gem.h | 9 +++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > > index 5a49487..ce23cf1 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.h > > +++ b/drivers/gpu/drm/i915/i915_gem.h > > @@ -42,6 +42,15 @@ > > #define GEM_DEBUG_BUG_ON(expr) > > #endif > > > > +#define GEM_MAYBE_BUILD_BUG_ON(expr) \ > > + do { \ > > + if (__builtin_constant_p((expr))) \ > > + BUILD_BUG_ON(expr); \ > > + else \ > > + GEM_BUG_ON(expr); \ > > + } while (0) > > + > > + > > #define I915_NUM_ENGINES 5 > > > > #endif /* __I915_GEM_H__ */ > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 7bc0c25..ce50ec4 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -88,7 +88,7 @@ > > int cpu, ret, timeout = (US) * 1000; \ > > u64 base; \ > > _WAIT_FOR_ATOMIC_CHECK(ATOMIC); \ > > - BUILD_BUG_ON((US) > 50000); \ > > + GEM_MAYBE_BUILD_BUG_ON((US) > 50000); \ > > if (!(ATOMIC)) { \ > > preempt_disable(); \ > > cpu = smp_processor_id(); \ > > > > The choice is between this and this: > > -#define wait_for_atomic(COND, MS) _wait_for_atomic((COND), (MS) * 1000, 1) > -#define wait_for_atomic_us(COND, US) _wait_for_atomic((COND), (US), 1) > +#define wait_for_atomic(COND, MS) \ > +({ \ > + int ret__; \ > + BUILD_BUG_ON((MS) > 50); \ > + ret__ = _wait_for_atomic((COND), (MS) * 1000, 1); \ > + ret__; \ > +}) > + > +#define wait_for_atomic_us(COND, US) \ > +({ \ > + int ret__; \ > + BUILD_BUG_ON((US) > 50000); \ > + ret__ = _wait_for_atomic((COND), (US), 1); \ > + ret__; \ > +}) > > Correct? Both would fix the GCC 4.4 issue? I was thinking this was the direction we should take as well. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON 2017-04-12 11:13 ` [RFC] " Tvrtko Ursulin 2017-04-12 11:29 ` Chris Wilson @ 2017-04-12 12:58 ` Michal Wajdeczko 1 sibling, 0 replies; 5+ messages in thread From: Michal Wajdeczko @ 2017-04-12 12:58 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx On Wed, Apr 12, 2017 at 12:13:51PM +0100, Tvrtko Ursulin wrote: > > On 11/04/2017 14:57, Michal Wajdeczko wrote: > > This is slightly weaker version of MAYBE_BUILD_BUG_ON that allows > > us to avoid adding implicit BUG but still detect as much as possible > > during the build. With this new macro we can fix the problem with > > GCC 4.4.7 that wrongly triggers build break in wait_for_atomic() > > when invoked with non-const parameter. > > > > Fixes: 1d1a9774 ("drm/i915: Extend intel_wait_for_register_fw() with fast timeout") > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_gem.h | 9 +++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > > index 5a49487..ce23cf1 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.h > > +++ b/drivers/gpu/drm/i915/i915_gem.h > > @@ -42,6 +42,15 @@ > > #define GEM_DEBUG_BUG_ON(expr) > > #endif > > > > +#define GEM_MAYBE_BUILD_BUG_ON(expr) \ > > + do { \ > > + if (__builtin_constant_p((expr))) \ > > + BUILD_BUG_ON(expr); \ > > + else \ > > + GEM_BUG_ON(expr); \ > > + } while (0) > > + > > + > > #define I915_NUM_ENGINES 5 > > > > #endif /* __I915_GEM_H__ */ > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 7bc0c25..ce50ec4 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -88,7 +88,7 @@ > > int cpu, ret, timeout = (US) * 1000; \ > > u64 base; \ > > _WAIT_FOR_ATOMIC_CHECK(ATOMIC); \ > > - BUILD_BUG_ON((US) > 50000); \ > > + GEM_MAYBE_BUILD_BUG_ON((US) > 50000); \ > > if (!(ATOMIC)) { \ > > preempt_disable(); \ > > cpu = smp_processor_id(); \ > > > > The choice is between this and this: > > -#define wait_for_atomic(COND, MS) _wait_for_atomic((COND), (MS) * 1000, 1) > -#define wait_for_atomic_us(COND, US) _wait_for_atomic((COND), (US), 1) > +#define wait_for_atomic(COND, MS) \ > +({ \ > + int ret__; \ > + BUILD_BUG_ON((MS) > 50); \ > + ret__ = _wait_for_atomic((COND), (MS) * 1000, 1); \ > + ret__; \ > +}) > + > +#define wait_for_atomic_us(COND, US) \ > +({ \ > + int ret__; \ > + BUILD_BUG_ON((US) > 50000); \ > + ret__ = _wait_for_atomic((COND), (US), 1); \ > + ret__; \ > +}) > > Correct? Both would fix the GCC 4.4 issue? First choice should fix both current issue and shold also survive any new case when someone decides to pass var to the wait_for_atomic() macro. The second one, should fix current build break on GCC 4.4 as we are passing var only to internal variant of _wait_for_atomic macro, but may fail again on any non-const US/MS param passed to public wait_for_atomic()... ... which maybe is acceptable if we also add: #define wait_for_atomic_us(COND, US) \ ({ \ int ret__; \ + BUILD_BUG_ON(!__builtin_constant_p(US)); \ BUILD_BUG_ON((US) > 50000); \ ret__ = _wait_for_atomic((COND), (US), 1); \ ret__; \ }) -Michal ps. as 50ms == 50000us then we can have only one macro with implementation: #define wait_for_atomic(COND, MS) wait_for_atomic_us((COND), (MS) * 1000) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-12 12:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-11 13:57 [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON Michal Wajdeczko 2017-04-11 15:28 ` ✓ Fi.CI.BAT: success for " Patchwork 2017-04-12 11:13 ` [RFC] " Tvrtko Ursulin 2017-04-12 11:29 ` Chris Wilson 2017-04-12 12:58 ` Michal Wajdeczko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.