Re: [Intel-gfx] [PATCH v3] drm/i915: remove IS_ACTIVE

2021-10-07 Thread Masahiro Yamada
On Wed, Oct 6, 2021 at 2:21 AM Lucas De Marchi  wrote:
>
> When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't
> provide much value just encapsulating it in a boolean context. So I also
> added the support for handling undefined macros as the IS_ENABLED()
> counterpart. However the feedback received from Masahiro Yamada was that
> it is too ugly, not providing much value. And just wrapping in a boolean
> context is too dumb - we could simply open code it.
>
> As detailed in commit babaab2f4738 ("drm/i915: Encapsulate kconfig
> constant values inside boolean predicates"), the IS_ACTIVE macro was
> added to workaround a compilation warning. However after checking again
> our current uses of IS_ACTIVE it turned out there is only
> 1 case in which it triggers a warning in clang (due
> -Wconstant-logical-operand) and 2 in smatch. All the others
> can simply use the shorter version, without wrapping it in any macro.
>
> So here I'm dialing all the way back to simply removing the macro. That
> single case hit by clang can be changed to make the constant come first,
> so it doesn't think it's mask:
>
> -   if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
> +   if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)
>
> As talked with Dan Carpenter, that logic will be added in smatch as
> well, so it will also stop warning about it.
>
> Signed-off-by: Lucas De Marchi 
> Acked-by: Jani Nikula 

Reviewed-by: Masahiro Yamada 





-- 
Best Regards
Masahiro Yamada


[Intel-gfx] [PATCH v3] drm/i915: remove IS_ACTIVE

2021-10-05 Thread Lucas De Marchi
When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't
provide much value just encapsulating it in a boolean context. So I also
added the support for handling undefined macros as the IS_ENABLED()
counterpart. However the feedback received from Masahiro Yamada was that
it is too ugly, not providing much value. And just wrapping in a boolean
context is too dumb - we could simply open code it.

As detailed in commit babaab2f4738 ("drm/i915: Encapsulate kconfig
constant values inside boolean predicates"), the IS_ACTIVE macro was
added to workaround a compilation warning. However after checking again
our current uses of IS_ACTIVE it turned out there is only
1 case in which it triggers a warning in clang (due
-Wconstant-logical-operand) and 2 in smatch. All the others
can simply use the shorter version, without wrapping it in any macro.

So here I'm dialing all the way back to simply removing the macro. That
single case hit by clang can be changed to make the constant come first,
so it doesn't think it's mask:

-   if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
+   if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)

As talked with Dan Carpenter, that logic will be added in smatch as
well, so it will also stop warning about it.

Signed-off-by: Lucas De Marchi 
Acked-by: Jani Nikula 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c|  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_engine.h |  4 ++--
 drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c   |  2 +-
 drivers/gpu/drm/i915/gt/intel_engine_types.h   |  2 +-
 .../gpu/drm/i915/gt/intel_execlists_submission.c   |  2 +-
 .../gpu/drm/i915/gt/selftest_engine_heartbeat.c|  4 ++--
 drivers/gpu/drm/i915/gt/selftest_execlists.c   | 14 +++---
 drivers/gpu/drm/i915/i915_config.c |  2 +-
 drivers/gpu/drm/i915/i915_request.c|  2 +-
 drivers/gpu/drm/i915/i915_utils.h  | 13 -
 11 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 74e33a4cdfe8..d225d3dd0b40 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -804,7 +804,7 @@ static int intel_context_set_gem(struct intel_context *ce,
intel_engine_has_semaphores(ce->engine))
__set_bit(CONTEXT_USE_SEMAPHORES, &ce->flags);
 
-   if (IS_ACTIVE(CONFIG_DRM_I915_REQUEST_TIMEOUT) &&
+   if (CONFIG_DRM_I915_REQUEST_TIMEOUT &&
ctx->i915->params.request_timeout_ms) {
unsigned int timeout_ms = ctx->i915->params.request_timeout_ms;
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c 
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 5130e8ed9564..65fc6ff5f59d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -395,7 +395,7 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
/* Track the mmo associated with the fenced vma */
vma->mmo = mmo;
 
-   if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
+   if (CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
   
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h 
b/drivers/gpu/drm/i915/gt/intel_engine.h
index eed4634c08cd..452248884ef1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -275,7 +275,7 @@ static inline bool intel_engine_uses_guc(const struct 
intel_engine_cs *engine)
 static inline bool
 intel_engine_has_preempt_reset(const struct intel_engine_cs *engine)
 {
-   if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+   if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
return false;
 
return intel_engine_has_preemption(engine);
@@ -302,7 +302,7 @@ intel_virtual_engine_has_heartbeat(const struct 
intel_engine_cs *engine)
 static inline bool
 intel_engine_has_heartbeat(const struct intel_engine_cs *engine)
 {
-   if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
+   if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
return false;
 
if (intel_engine_is_virtual(engine))
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c 
b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 74775ae961b2..a3698f611f45 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -207,7 +207,7 @@ static void heartbeat(struct work_struct *wrk)
 
 void intel_engine_unpark_heartbeat(struct intel_engine_cs *engine)
 {
-   if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
+   if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
return;
 
next_heartbeat(engine);
diff --git a/drivers/gpu/