2017-08-25 17:10:17

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH] drm/i915: Mark wait_for_engine() as maybe_unused

The only call of wait_for_engine() is wrapped in a GEM_WARN_ON macro,
which confusingly suppresses the call unless CONFIG_DRM_I915_DEBUG_GEM
is set.

According to http://www.spinics.net/lists/intel-gfx/msg128768.html the
current behavior is correct, even though it's not obvious. Different
solutions to improve GEM_WARN_ON were discussed, but no conclusion
was reached.

Mark wait_for_engine() as maybe_unused to avoid a compiler warning,
according to the above discussion this is still needed evein if
GEM_WARN_ON is eventually refactored.

Reported-by: Nick Desaulniers <[email protected]>
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 969bac8404f1..52d0b7d0082b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3364,7 +3364,8 @@ static int wait_for_timeline(struct i915_gem_timeline *tl, unsigned int flags)
return 0;
}

-static int wait_for_engine(struct intel_engine_cs *engine, int timeout_ms)
+static __maybe_unused int wait_for_engine(
+ struct intel_engine_cs *engine, int timeout_ms)
{
return wait_for(intel_engine_is_idle(engine), timeout_ms);
}
--
2.14.1.342.g6490525c54-goog


2017-09-26 04:54:19

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/i915: Mark wait_for_engine() as maybe_unused

On Fri, Aug 25, 2017 at 10:09:45AM -0700, Matthias Kaehlcke wrote:
> The only call of wait_for_engine() is wrapped in a GEM_WARN_ON macro,
> which confusingly suppresses the call unless CONFIG_DRM_I915_DEBUG_GEM
> is set.
>
> According to http://www.spinics.net/lists/intel-gfx/msg128768.html the
> current behavior is correct, even though it's not obvious. Different
> solutions to improve GEM_WARN_ON were discussed, but no conclusion
> was reached.
>
> Mark wait_for_engine() as maybe_unused to avoid a compiler warning,
> according to the above discussion this is still needed evein if
> GEM_WARN_ON is eventually refactored.
>
> Reported-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Matthias Kaehlcke <[email protected]>

Seems to be taken care of already in

commit cad9946c2a4375386062131858881cfd30fc1b8f
Author: Chris Wilson <[email protected]>
Date: Sat Aug 26 12:09:33 2017 +0100

drm/i915: Always sanity check engine state upon idling

Cheers, Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 969bac8404f1..52d0b7d0082b 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3364,7 +3364,8 @@ static int wait_for_timeline(struct i915_gem_timeline *tl, unsigned int flags)
> return 0;
> }
>
> -static int wait_for_engine(struct intel_engine_cs *engine, int timeout_ms)
> +static __maybe_unused int wait_for_engine(
> + struct intel_engine_cs *engine, int timeout_ms)
> {
> return wait_for(intel_engine_is_idle(engine), timeout_ms);
> }
> --
> 2.14.1.342.g6490525c54-goog
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch