All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-01  7:40 ` Lucas De Marchi
  0 siblings, 0 replies; 19+ messages in thread
From: Lucas De Marchi @ 2021-10-01  7:40 UTC (permalink / raw)
  To: intel-gfx
  Cc: dri-devel, Jani Nikula, Masahiro Yamada, Steven Price, Andrzej Hajda

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 would potentially trigger a warning. All the others
  can simply use the shorter version, without wrapping it in any macro.
And even that single one didn't trigger any warning in gcc 10.3.

So here I'm dialing all the way back to simply removing the macro. If it
triggers warnings in future we may change the few cases to check for > 0
or != 0. Another possibility would be to use the great "not not
operator" for all positive checks, which would allow us to maintain
consistency.  However let's try first the simplest form though, hopefully
we don't hit broken compilers spitting a warning:

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 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 8208fd5b72c3..be60bcf8069c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -761,7 +761,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 87579affb952..6aba239a10e8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -273,7 +273,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);
@@ -300,7 +300,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/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 5ae1207c363b..9167ce52487c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -556,7 +556,7 @@ intel_engine_has_semaphores(const struct intel_engine_cs *engine)
 static inline bool
 intel_engine_has_timeslices(const struct intel_engine_cs *engine)
 {
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return false;
 
 	return engine->flags & I915_ENGINE_HAS_TIMESLICES;
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 7147fe80919e..73a79c2acd3a 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -3339,7 +3339,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
 		engine->flags |= I915_ENGINE_HAS_SEMAPHORES;
 		if (can_preempt(engine)) {
 			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
-			if (IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+			if (CONFIG_DRM_I915_TIMESLICE_DURATION)
 				engine->flags |= I915_ENGINE_HAS_TIMESLICES;
 		}
 	}
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index 317eebf086c3..6e6e4d747cca 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -290,7 +290,7 @@ static int live_heartbeat_fast(void *arg)
 	int err = 0;
 
 	/* Check that the heartbeat ticks at the desired rate. */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
+	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
 		return 0;
 
 	for_each_engine(engine, gt, id) {
@@ -352,7 +352,7 @@ static int live_heartbeat_off(void *arg)
 	int err = 0;
 
 	/* Check that we can turn off heartbeat and not interrupt VIP */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
+	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
 		return 0;
 
 	for_each_engine(engine, gt, id) {
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index b3863abc51f5..25a8c4f62b0d 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -992,7 +992,7 @@ static int live_timeslice_preempt(void *arg)
 	 * need to preempt the current task and replace it with another
 	 * ready task.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
@@ -1122,7 +1122,7 @@ static int live_timeslice_rewind(void *arg)
 	 * but only a few of those requests, forcing us to rewind the
 	 * RING_TAIL of the original request.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	for_each_engine(engine, gt, id) {
@@ -1299,7 +1299,7 @@ static int live_timeslice_queue(void *arg)
 	 * ELSP[1] is already occupied, so must rely on timeslicing to
 	 * eject ELSP[0] in favour of the queue.)
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
@@ -1420,7 +1420,7 @@ static int live_timeslice_nopreempt(void *arg)
 	 * We should not timeslice into a request that is marked with
 	 * I915_REQUEST_NOPREEMPT.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	if (igt_spinner_init(&spin, gt))
@@ -2260,7 +2260,7 @@ static int __cancel_hostile(struct live_preempt_cancel *arg)
 	int err;
 
 	/* Preempt cancel non-preemptible spinner in ELSP0 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
 		return 0;
 
 	if (!intel_has_reset_engine(arg->engine->gt))
@@ -2316,7 +2316,7 @@ static int __cancel_fail(struct live_preempt_cancel *arg)
 	struct i915_request *rq;
 	int err;
 
-	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
 		return 0;
 
 	if (!intel_has_reset_engine(engine->gt))
@@ -3375,7 +3375,7 @@ static int live_preempt_timeout(void *arg)
 	 * Check that we force preemption to occur by cancelling the previous
 	 * context if it refuses to yield the GPU.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
 		return 0;
 
 	if (!intel_has_reset_engine(gt))
diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c
index b79b5f6d2cfa..ad228dd96a0b 100644
--- a/drivers/gpu/drm/i915/i915_config.c
+++ b/drivers/gpu/drm/i915/i915_config.c
@@ -8,7 +8,7 @@
 unsigned long
 i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
 {
-	if (context && IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT))
+	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
 		return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 79da5eca60af..91bd6f4e9909 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1852,7 +1852,7 @@ long i915_request_wait(struct i915_request *rq,
 	 * completion. That requires having a good predictor for the request
 	 * duration, which we currently lack.
 	 */
-	if (IS_ACTIVE(CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT) &&
+	if (CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT &&
 	    __i915_spin_request(rq, state))
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 5259edacde38..62f189e064a9 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -458,17 +458,4 @@ static inline bool timer_expired(const struct timer_list *t)
 	return timer_active(t) && !timer_pending(t);
 }
 
-/*
- * This is a lookalike for IS_ENABLED() that takes a kconfig value,
- * e.g. CONFIG_DRM_I915_SPIN_REQUEST, and evaluates whether it is non-zero
- * i.e. whether the configuration is active. Wrapping up the config inside
- * a boolean context prevents clang and smatch from complaining about potential
- * issues in confusing logical-&& with bitwise-& for constants.
- *
- * Sadly IS_ENABLED() itself does not work with kconfig values.
- *
- * Returns 0 if @config is 0, 1 if set to any value.
- */
-#define IS_ACTIVE(config) ((config) != 0)
-
 #endif /* !__I915_UTILS_H */
-- 
2.33.0


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

* [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-01  7:40 ` Lucas De Marchi
  0 siblings, 0 replies; 19+ messages in thread
From: Lucas De Marchi @ 2021-10-01  7:40 UTC (permalink / raw)
  To: intel-gfx
  Cc: dri-devel, Jani Nikula, Masahiro Yamada, Steven Price, Andrzej Hajda

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 would potentially trigger a warning. All the others
  can simply use the shorter version, without wrapping it in any macro.
And even that single one didn't trigger any warning in gcc 10.3.

So here I'm dialing all the way back to simply removing the macro. If it
triggers warnings in future we may change the few cases to check for > 0
or != 0. Another possibility would be to use the great "not not
operator" for all positive checks, which would allow us to maintain
consistency.  However let's try first the simplest form though, hopefully
we don't hit broken compilers spitting a warning:

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 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 8208fd5b72c3..be60bcf8069c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -761,7 +761,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 87579affb952..6aba239a10e8 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -273,7 +273,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);
@@ -300,7 +300,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/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 5ae1207c363b..9167ce52487c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -556,7 +556,7 @@ intel_engine_has_semaphores(const struct intel_engine_cs *engine)
 static inline bool
 intel_engine_has_timeslices(const struct intel_engine_cs *engine)
 {
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return false;
 
 	return engine->flags & I915_ENGINE_HAS_TIMESLICES;
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 7147fe80919e..73a79c2acd3a 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -3339,7 +3339,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
 		engine->flags |= I915_ENGINE_HAS_SEMAPHORES;
 		if (can_preempt(engine)) {
 			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
-			if (IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+			if (CONFIG_DRM_I915_TIMESLICE_DURATION)
 				engine->flags |= I915_ENGINE_HAS_TIMESLICES;
 		}
 	}
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index 317eebf086c3..6e6e4d747cca 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -290,7 +290,7 @@ static int live_heartbeat_fast(void *arg)
 	int err = 0;
 
 	/* Check that the heartbeat ticks at the desired rate. */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
+	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
 		return 0;
 
 	for_each_engine(engine, gt, id) {
@@ -352,7 +352,7 @@ static int live_heartbeat_off(void *arg)
 	int err = 0;
 
 	/* Check that we can turn off heartbeat and not interrupt VIP */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
+	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
 		return 0;
 
 	for_each_engine(engine, gt, id) {
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index b3863abc51f5..25a8c4f62b0d 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -992,7 +992,7 @@ static int live_timeslice_preempt(void *arg)
 	 * need to preempt the current task and replace it with another
 	 * ready task.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
@@ -1122,7 +1122,7 @@ static int live_timeslice_rewind(void *arg)
 	 * but only a few of those requests, forcing us to rewind the
 	 * RING_TAIL of the original request.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	for_each_engine(engine, gt, id) {
@@ -1299,7 +1299,7 @@ static int live_timeslice_queue(void *arg)
 	 * ELSP[1] is already occupied, so must rely on timeslicing to
 	 * eject ELSP[0] in favour of the queue.)
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
@@ -1420,7 +1420,7 @@ static int live_timeslice_nopreempt(void *arg)
 	 * We should not timeslice into a request that is marked with
 	 * I915_REQUEST_NOPREEMPT.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
+	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
 		return 0;
 
 	if (igt_spinner_init(&spin, gt))
@@ -2260,7 +2260,7 @@ static int __cancel_hostile(struct live_preempt_cancel *arg)
 	int err;
 
 	/* Preempt cancel non-preemptible spinner in ELSP0 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
 		return 0;
 
 	if (!intel_has_reset_engine(arg->engine->gt))
@@ -2316,7 +2316,7 @@ static int __cancel_fail(struct live_preempt_cancel *arg)
 	struct i915_request *rq;
 	int err;
 
-	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
 		return 0;
 
 	if (!intel_has_reset_engine(engine->gt))
@@ -3375,7 +3375,7 @@ static int live_preempt_timeout(void *arg)
 	 * Check that we force preemption to occur by cancelling the previous
 	 * context if it refuses to yield the GPU.
 	 */
-	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
+	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
 		return 0;
 
 	if (!intel_has_reset_engine(gt))
diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c
index b79b5f6d2cfa..ad228dd96a0b 100644
--- a/drivers/gpu/drm/i915/i915_config.c
+++ b/drivers/gpu/drm/i915/i915_config.c
@@ -8,7 +8,7 @@
 unsigned long
 i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
 {
-	if (context && IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT))
+	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
 		return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 79da5eca60af..91bd6f4e9909 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1852,7 +1852,7 @@ long i915_request_wait(struct i915_request *rq,
 	 * completion. That requires having a good predictor for the request
 	 * duration, which we currently lack.
 	 */
-	if (IS_ACTIVE(CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT) &&
+	if (CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT &&
 	    __i915_spin_request(rq, state))
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 5259edacde38..62f189e064a9 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -458,17 +458,4 @@ static inline bool timer_expired(const struct timer_list *t)
 	return timer_active(t) && !timer_pending(t);
 }
 
-/*
- * This is a lookalike for IS_ENABLED() that takes a kconfig value,
- * e.g. CONFIG_DRM_I915_SPIN_REQUEST, and evaluates whether it is non-zero
- * i.e. whether the configuration is active. Wrapping up the config inside
- * a boolean context prevents clang and smatch from complaining about potential
- * issues in confusing logical-&& with bitwise-& for constants.
- *
- * Sadly IS_ENABLED() itself does not work with kconfig values.
- *
- * Returns 0 if @config is 0, 1 if set to any value.
- */
-#define IS_ACTIVE(config) ((config) != 0)
-
 #endif /* !__I915_UTILS_H */
-- 
2.33.0


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

* Re: [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
@ 2021-10-01  7:46   ` Jani Nikula
  -1 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2021-10-01  7:46 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx
  Cc: dri-devel, Masahiro Yamada, Steven Price, Andrzej Hajda

On Fri, 01 Oct 2021, Lucas De Marchi <lucas.demarchi@intel.com> 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 would potentially trigger a warning. All the others
>   can simply use the shorter version, without wrapping it in any macro.
> And even that single one didn't trigger any warning in gcc 10.3.
>
> So here I'm dialing all the way back to simply removing the macro. If it
> triggers warnings in future we may change the few cases to check for > 0
> or != 0. Another possibility would be to use the great "not not
> operator" for all positive checks, which would allow us to maintain
> consistency.  However let's try first the simplest form though, hopefully
> we don't hit broken compilers spitting a warning:
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  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 8208fd5b72c3..be60bcf8069c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -761,7 +761,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 87579affb952..6aba239a10e8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -273,7 +273,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);
> @@ -300,7 +300,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/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 5ae1207c363b..9167ce52487c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -556,7 +556,7 @@ intel_engine_has_semaphores(const struct intel_engine_cs *engine)
>  static inline bool
>  intel_engine_has_timeslices(const struct intel_engine_cs *engine)
>  {
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return false;
>  
>  	return engine->flags & I915_ENGINE_HAS_TIMESLICES;
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 7147fe80919e..73a79c2acd3a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -3339,7 +3339,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
>  		engine->flags |= I915_ENGINE_HAS_SEMAPHORES;
>  		if (can_preempt(engine)) {
>  			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
> -			if (IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +			if (CONFIG_DRM_I915_TIMESLICE_DURATION)
>  				engine->flags |= I915_ENGINE_HAS_TIMESLICES;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> index 317eebf086c3..6e6e4d747cca 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> @@ -290,7 +290,7 @@ static int live_heartbeat_fast(void *arg)
>  	int err = 0;
>  
>  	/* Check that the heartbeat ticks at the desired rate. */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
> +	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
>  		return 0;
>  
>  	for_each_engine(engine, gt, id) {
> @@ -352,7 +352,7 @@ static int live_heartbeat_off(void *arg)
>  	int err = 0;
>  
>  	/* Check that we can turn off heartbeat and not interrupt VIP */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
> +	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
>  		return 0;
>  
>  	for_each_engine(engine, gt, id) {
> diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> index b3863abc51f5..25a8c4f62b0d 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> @@ -992,7 +992,7 @@ static int live_timeslice_preempt(void *arg)
>  	 * need to preempt the current task and replace it with another
>  	 * ready task.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
> @@ -1122,7 +1122,7 @@ static int live_timeslice_rewind(void *arg)
>  	 * but only a few of those requests, forcing us to rewind the
>  	 * RING_TAIL of the original request.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	for_each_engine(engine, gt, id) {
> @@ -1299,7 +1299,7 @@ static int live_timeslice_queue(void *arg)
>  	 * ELSP[1] is already occupied, so must rely on timeslicing to
>  	 * eject ELSP[0] in favour of the queue.)
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
> @@ -1420,7 +1420,7 @@ static int live_timeslice_nopreempt(void *arg)
>  	 * We should not timeslice into a request that is marked with
>  	 * I915_REQUEST_NOPREEMPT.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	if (igt_spinner_init(&spin, gt))
> @@ -2260,7 +2260,7 @@ static int __cancel_hostile(struct live_preempt_cancel *arg)
>  	int err;
>  
>  	/* Preempt cancel non-preemptible spinner in ELSP0 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
> +	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
>  		return 0;
>  
>  	if (!intel_has_reset_engine(arg->engine->gt))
> @@ -2316,7 +2316,7 @@ static int __cancel_fail(struct live_preempt_cancel *arg)
>  	struct i915_request *rq;
>  	int err;
>  
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
> +	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
>  		return 0;
>  
>  	if (!intel_has_reset_engine(engine->gt))
> @@ -3375,7 +3375,7 @@ static int live_preempt_timeout(void *arg)
>  	 * Check that we force preemption to occur by cancelling the previous
>  	 * context if it refuses to yield the GPU.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
> +	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
>  		return 0;
>  
>  	if (!intel_has_reset_engine(gt))
> diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c
> index b79b5f6d2cfa..ad228dd96a0b 100644
> --- a/drivers/gpu/drm/i915/i915_config.c
> +++ b/drivers/gpu/drm/i915/i915_config.c
> @@ -8,7 +8,7 @@
>  unsigned long
>  i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
>  {
> -	if (context && IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT))
> +	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
>  		return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 79da5eca60af..91bd6f4e9909 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1852,7 +1852,7 @@ long i915_request_wait(struct i915_request *rq,
>  	 * completion. That requires having a good predictor for the request
>  	 * duration, which we currently lack.
>  	 */
> -	if (IS_ACTIVE(CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT) &&
> +	if (CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT &&
>  	    __i915_spin_request(rq, state))
>  		goto out;
>  
> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> index 5259edacde38..62f189e064a9 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -458,17 +458,4 @@ static inline bool timer_expired(const struct timer_list *t)
>  	return timer_active(t) && !timer_pending(t);
>  }
>  
> -/*
> - * This is a lookalike for IS_ENABLED() that takes a kconfig value,
> - * e.g. CONFIG_DRM_I915_SPIN_REQUEST, and evaluates whether it is non-zero
> - * i.e. whether the configuration is active. Wrapping up the config inside
> - * a boolean context prevents clang and smatch from complaining about potential
> - * issues in confusing logical-&& with bitwise-& for constants.
> - *
> - * Sadly IS_ENABLED() itself does not work with kconfig values.
> - *
> - * Returns 0 if @config is 0, 1 if set to any value.
> - */
> -#define IS_ACTIVE(config) ((config) != 0)
> -
>  #endif /* !__I915_UTILS_H */

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-01  7:46   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2021-10-01  7:46 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx
  Cc: dri-devel, Masahiro Yamada, Steven Price, Andrzej Hajda

On Fri, 01 Oct 2021, Lucas De Marchi <lucas.demarchi@intel.com> 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 would potentially trigger a warning. All the others
>   can simply use the shorter version, without wrapping it in any macro.
> And even that single one didn't trigger any warning in gcc 10.3.
>
> So here I'm dialing all the way back to simply removing the macro. If it
> triggers warnings in future we may change the few cases to check for > 0
> or != 0. Another possibility would be to use the great "not not
> operator" for all positive checks, which would allow us to maintain
> consistency.  However let's try first the simplest form though, hopefully
> we don't hit broken compilers spitting a warning:
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  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 8208fd5b72c3..be60bcf8069c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -761,7 +761,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 87579affb952..6aba239a10e8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -273,7 +273,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);
> @@ -300,7 +300,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/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 5ae1207c363b..9167ce52487c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -556,7 +556,7 @@ intel_engine_has_semaphores(const struct intel_engine_cs *engine)
>  static inline bool
>  intel_engine_has_timeslices(const struct intel_engine_cs *engine)
>  {
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return false;
>  
>  	return engine->flags & I915_ENGINE_HAS_TIMESLICES;
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 7147fe80919e..73a79c2acd3a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -3339,7 +3339,7 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
>  		engine->flags |= I915_ENGINE_HAS_SEMAPHORES;
>  		if (can_preempt(engine)) {
>  			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
> -			if (IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +			if (CONFIG_DRM_I915_TIMESLICE_DURATION)
>  				engine->flags |= I915_ENGINE_HAS_TIMESLICES;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> index 317eebf086c3..6e6e4d747cca 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> @@ -290,7 +290,7 @@ static int live_heartbeat_fast(void *arg)
>  	int err = 0;
>  
>  	/* Check that the heartbeat ticks at the desired rate. */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
> +	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
>  		return 0;
>  
>  	for_each_engine(engine, gt, id) {
> @@ -352,7 +352,7 @@ static int live_heartbeat_off(void *arg)
>  	int err = 0;
>  
>  	/* Check that we can turn off heartbeat and not interrupt VIP */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_HEARTBEAT_INTERVAL))
> +	if (!CONFIG_DRM_I915_HEARTBEAT_INTERVAL)
>  		return 0;
>  
>  	for_each_engine(engine, gt, id) {
> diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> index b3863abc51f5..25a8c4f62b0d 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> @@ -992,7 +992,7 @@ static int live_timeslice_preempt(void *arg)
>  	 * need to preempt the current task and replace it with another
>  	 * ready task.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
> @@ -1122,7 +1122,7 @@ static int live_timeslice_rewind(void *arg)
>  	 * but only a few of those requests, forcing us to rewind the
>  	 * RING_TAIL of the original request.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	for_each_engine(engine, gt, id) {
> @@ -1299,7 +1299,7 @@ static int live_timeslice_queue(void *arg)
>  	 * ELSP[1] is already occupied, so must rely on timeslicing to
>  	 * eject ELSP[0] in favour of the queue.)
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
> @@ -1420,7 +1420,7 @@ static int live_timeslice_nopreempt(void *arg)
>  	 * We should not timeslice into a request that is marked with
>  	 * I915_REQUEST_NOPREEMPT.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
> +	if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
>  		return 0;
>  
>  	if (igt_spinner_init(&spin, gt))
> @@ -2260,7 +2260,7 @@ static int __cancel_hostile(struct live_preempt_cancel *arg)
>  	int err;
>  
>  	/* Preempt cancel non-preemptible spinner in ELSP0 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
> +	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
>  		return 0;
>  
>  	if (!intel_has_reset_engine(arg->engine->gt))
> @@ -2316,7 +2316,7 @@ static int __cancel_fail(struct live_preempt_cancel *arg)
>  	struct i915_request *rq;
>  	int err;
>  
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
> +	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
>  		return 0;
>  
>  	if (!intel_has_reset_engine(engine->gt))
> @@ -3375,7 +3375,7 @@ static int live_preempt_timeout(void *arg)
>  	 * Check that we force preemption to occur by cancelling the previous
>  	 * context if it refuses to yield the GPU.
>  	 */
> -	if (!IS_ACTIVE(CONFIG_DRM_I915_PREEMPT_TIMEOUT))
> +	if (!CONFIG_DRM_I915_PREEMPT_TIMEOUT)
>  		return 0;
>  
>  	if (!intel_has_reset_engine(gt))
> diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c
> index b79b5f6d2cfa..ad228dd96a0b 100644
> --- a/drivers/gpu/drm/i915/i915_config.c
> +++ b/drivers/gpu/drm/i915/i915_config.c
> @@ -8,7 +8,7 @@
>  unsigned long
>  i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
>  {
> -	if (context && IS_ACTIVE(CONFIG_DRM_I915_FENCE_TIMEOUT))
> +	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
>  		return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 79da5eca60af..91bd6f4e9909 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1852,7 +1852,7 @@ long i915_request_wait(struct i915_request *rq,
>  	 * completion. That requires having a good predictor for the request
>  	 * duration, which we currently lack.
>  	 */
> -	if (IS_ACTIVE(CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT) &&
> +	if (CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT &&
>  	    __i915_spin_request(rq, state))
>  		goto out;
>  
> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> index 5259edacde38..62f189e064a9 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -458,17 +458,4 @@ static inline bool timer_expired(const struct timer_list *t)
>  	return timer_active(t) && !timer_pending(t);
>  }
>  
> -/*
> - * This is a lookalike for IS_ENABLED() that takes a kconfig value,
> - * e.g. CONFIG_DRM_I915_SPIN_REQUEST, and evaluates whether it is non-zero
> - * i.e. whether the configuration is active. Wrapping up the config inside
> - * a boolean context prevents clang and smatch from complaining about potential
> - * issues in confusing logical-&& with bitwise-& for constants.
> - *
> - * Sadly IS_ENABLED() itself does not work with kconfig values.
> - *
> - * Returns 0 if @config is 0, 1 if set to any value.
> - */
> -#define IS_ACTIVE(config) ((config) != 0)
> -
>  #endif /* !__I915_UTILS_H */

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: remove IS_ACTIVE
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
  (?)
  (?)
@ 2021-10-01  9:11 ` Patchwork
  -1 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2021-10-01  9:11 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915: remove IS_ACTIVE
URL   : https://patchwork.freedesktop.org/series/95312/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10671 -> Patchwork_21214
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][1] ([fdo#109315])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-kbl-soraka/igt@amdgpu/amd_basic@query-info.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][3] ([fdo#109315] / [i915#2575]) +16 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][4] ([i915#1888])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][5] ([i915#2190])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][6] ([i915#1155])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@i915_pm_backlight@basic-brightness.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][7] ([fdo#111827]) +8 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][8] ([i915#4103]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][9] ([fdo#109285])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][10] ([i915#1072]) +3 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][11] ([i915#3301])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/fi-tgl-1115g4/igt@prime_vgem@basic-userptr.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103


Participating hosts (31 -> 28)
------------------------------

  Additional (1): fi-tgl-1115g4 
  Missing    (4): fi-bsw-cyan bat-jsl-1 bat-dg1-6 bat-adlp-4 


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

  * Linux: CI_DRM_10671 -> Patchwork_21214

  CI-20190529: 20190529
  CI_DRM_10671: 0c56a95d6dcf174353231175cb56dfbead9aa287 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6228: 22643ce4014a0b2dc52ce7916b2f657e2a7757c3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21214: acba9196789a9bd61f33432e9ba9847b58c678d8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

acba9196789a drm/i915: remove IS_ACTIVE

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
                   ` (2 preceding siblings ...)
  (?)
@ 2021-10-01  9:29 ` Chris Wilson
  2021-10-01  9:57   ` Jani Nikula
  -1 siblings, 1 reply; 19+ messages in thread
From: Chris Wilson @ 2021-10-01  9:29 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: dri-devel, Jani Nikula, Masahiro Yamada, Steven Price, Andrzej Hajda

Quoting Lucas De Marchi (2021-10-01 08:40:41)
> 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 would potentially trigger a warning. All the others
>   can simply use the shorter version, without wrapping it in any macro.
> And even that single one didn't trigger any warning in gcc 10.3.
> 
> So here I'm dialing all the way back to simply removing the macro. If it
> triggers warnings in future we may change the few cases to check for > 0
> or != 0. Another possibility would be to use the great "not not
> operator" for all positive checks, which would allow us to maintain
> consistency.  However let's try first the simplest form though, hopefully
> we don't hit broken compilers spitting a warning:

You didn't prevent the compilation warning this re-introduces.

drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?
drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op?
-Chris

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  9:29 ` [Intel-gfx] [PATCH] " Chris Wilson
@ 2021-10-01  9:57   ` Jani Nikula
  2021-10-01 16:17     ` Lucas De Marchi
  2021-10-04 20:52     ` Lucas De Marchi
  0 siblings, 2 replies; 19+ messages in thread
From: Jani Nikula @ 2021-10-01  9:57 UTC (permalink / raw)
  To: Chris Wilson, Lucas De Marchi
  Cc: dri-devel, Masahiro Yamada, Steven Price, Andrzej Hajda,
	intel-gfx, Sarvela, Tomi P

On Fri, 01 Oct 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Lucas De Marchi (2021-10-01 08:40:41)
>> 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 would potentially trigger a warning. All the others
>>   can simply use the shorter version, without wrapping it in any macro.
>> And even that single one didn't trigger any warning in gcc 10.3.
>> 
>> So here I'm dialing all the way back to simply removing the macro. If it
>> triggers warnings in future we may change the few cases to check for > 0
>> or != 0. Another possibility would be to use the great "not not
>> operator" for all positive checks, which would allow us to maintain
>> consistency.  However let's try first the simplest form though, hopefully
>> we don't hit broken compilers spitting a warning:
>
> You didn't prevent the compilation warning this re-introduces.
>
> drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?
> drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op?

Looks like that's a Smatch warning. The immediate fix would be to just
add the != 0 in the relevant places. But this is stuff that's just going
to get broken again unless we add Smatch to CI. Most people aren't
running it on a regular basis.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: remove IS_ACTIVE
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
                   ` (3 preceding siblings ...)
  (?)
@ 2021-10-01 10:51 ` Patchwork
  -1 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2021-10-01 10:51 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915: remove IS_ACTIVE
URL   : https://patchwork.freedesktop.org/series/95312/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10671_full -> Patchwork_21214_full
====================================================

Summary
-------

  **FAILURE**

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

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-kbl:          NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@display-4x:
    - shard-tglb:         NOTRUN -> [SKIP][2] ([i915#1839])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@feature_discovery@display-4x.html

  * igt@gem_ctx_persistence@smoketest:
    - shard-tglb:         [PASS][3] -> [FAIL][4] ([i915#2896])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb6/igt@gem_ctx_persistence@smoketest.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb7/igt@gem_ctx_persistence@smoketest.html

  * igt@gem_ctx_sseu@engines:
    - shard-tglb:         NOTRUN -> [SKIP][5] ([i915#280])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@gem_ctx_sseu@engines.html

  * igt@gem_eio@in-flight-suspend:
    - shard-tglb:         [PASS][6] -> [INCOMPLETE][7] ([i915#456]) +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb3/igt@gem_eio@in-flight-suspend.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb7/igt@gem_eio@in-flight-suspend.html

  * igt@gem_eio@unwedge-stress:
    - shard-tglb:         NOTRUN -> [TIMEOUT][8] ([i915#2369] / [i915#3063] / [i915#3648])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb6/igt@gem_eio@unwedge-stress.html
    - shard-iclb:         [PASS][9] -> [TIMEOUT][10] ([i915#2369] / [i915#2481] / [i915#3070])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb3/igt@gem_eio@unwedge-stress.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb3/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([i915#2842]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb2/igt@gem_exec_fair@basic-none-share@rcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb5/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][13] ([i915#2842])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb1/igt@gem_exec_fair@basic-none@vcs1.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-tglb:         [PASS][14] -> [FAIL][15] ([i915#2842])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb6/igt@gem_exec_fair@basic-pace@bcs0.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb7/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [PASS][16] -> [FAIL][17] ([i915#2842])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-kbl4/igt@gem_exec_fair@basic-pace@vecs0.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl7/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_exec_flush@basic-batch-kernel-default-cmd:
    - shard-tglb:         NOTRUN -> [SKIP][18] ([fdo#109313])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb5/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html

  * igt@gem_exec_params@no-bsd:
    - shard-tglb:         NOTRUN -> [SKIP][19] ([fdo#109283])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb6/igt@gem_exec_params@no-bsd.html

  * igt@gem_exec_whisper@basic-queues-forked-all:
    - shard-glk:          [PASS][20] -> [DMESG-WARN][21] ([i915#118] / [i915#95])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-glk8/igt@gem_exec_whisper@basic-queues-forked-all.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk7/igt@gem_exec_whisper@basic-queues-forked-all.html

  * igt@gem_huc_copy@huc-copy:
    - shard-apl:          NOTRUN -> [SKIP][22] ([fdo#109271] / [i915#2190])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl7/igt@gem_huc_copy@huc-copy.html

  * igt@gem_pread@exhaustion:
    - shard-apl:          NOTRUN -> [WARN][23] ([i915#2658])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl2/igt@gem_pread@exhaustion.html
    - shard-kbl:          NOTRUN -> [WARN][24] ([i915#2658])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl2/igt@gem_pread@exhaustion.html

  * igt@gem_userptr_blits@coherency-sync:
    - shard-tglb:         NOTRUN -> [SKIP][25] ([fdo#110542])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@gem_userptr_blits@coherency-sync.html

  * igt@gem_userptr_blits@unsync-overlap:
    - shard-tglb:         NOTRUN -> [SKIP][26] ([i915#3297])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb5/igt@gem_userptr_blits@unsync-overlap.html

  * igt@gem_userptr_blits@vma-merge:
    - shard-skl:          NOTRUN -> [FAIL][27] ([i915#3318])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl1/igt@gem_userptr_blits@vma-merge.html

  * igt@gen3_mixed_blits:
    - shard-tglb:         NOTRUN -> [SKIP][28] ([fdo#109289])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@gen3_mixed_blits.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][29] -> [DMESG-WARN][30] ([i915#1436] / [i915#716])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl1/igt@gen9_exec_parse@allowed-single.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl2/igt@gen9_exec_parse@allowed-single.html

  * igt@gen9_exec_parse@bb-start-out:
    - shard-tglb:         NOTRUN -> [SKIP][31] ([i915#2856])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb8/igt@gen9_exec_parse@bb-start-out.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp:
    - shard-kbl:          NOTRUN -> [SKIP][32] ([fdo#109271] / [i915#1937])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl3/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html

  * igt@i915_pm_rc6_residency@rc6-fence:
    - shard-tglb:         NOTRUN -> [WARN][33] ([i915#2681])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb6/igt@i915_pm_rc6_residency@rc6-fence.html

  * igt@i915_pm_rpm@modeset-pc8-residency-stress:
    - shard-tglb:         NOTRUN -> [SKIP][34] ([fdo#109506] / [i915#2411])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@i915_pm_rpm@modeset-pc8-residency-stress.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-tglb:         [PASS][35] -> [INCOMPLETE][36] ([i915#2411] / [i915#456] / [i915#750])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb1/igt@i915_pm_rpm@system-suspend-execbuf.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb7/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@i915_query@query-topology-known-pci-ids:
    - shard-tglb:         NOTRUN -> [SKIP][37] ([fdo#109303])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@i915_query@query-topology-known-pci-ids.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-glk:          NOTRUN -> [SKIP][38] ([fdo#109271] / [i915#3777])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk4/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@yf-tiled-8bpp-rotate-90:
    - shard-tglb:         NOTRUN -> [SKIP][39] ([fdo#111615]) +2 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_big_fb@yf-tiled-8bpp-rotate-90.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-apl:          NOTRUN -> [SKIP][40] ([fdo#109271] / [i915#3777])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl2/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
    - shard-apl:          NOTRUN -> [SKIP][41] ([fdo#109271]) +130 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl6/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html

  * igt@kms_big_joiner@invalid-modeset:
    - shard-tglb:         NOTRUN -> [SKIP][42] ([i915#2705])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb5/igt@kms_big_joiner@invalid-modeset.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-kbl:          NOTRUN -> [SKIP][43] ([fdo#109271] / [i915#3886]) +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl2/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][44] ([fdo#109271] / [i915#3886]) +7 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl7/igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][45] ([fdo#109271] / [i915#3886])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk4/igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_mc_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][46] ([i915#3689] / [i915#3886]) +1 similar issue
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-kbl:          NOTRUN -> [SKIP][47] ([fdo#109271]) +60 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl2/igt@kms_ccs@pipe-d-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-random-ccs-data-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][48] ([i915#3689])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_ccs@pipe-d-random-ccs-data-yf_tiled_ccs.html

  * igt@kms_chamelium@dp-crc-multiple:
    - shard-skl:          NOTRUN -> [SKIP][49] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl1/igt@kms_chamelium@dp-crc-multiple.html

  * igt@kms_chamelium@dp-hpd-enable-disable-mode:
    - shard-tglb:         NOTRUN -> [SKIP][50] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_chamelium@dp-hpd-enable-disable-mode.html

  * igt@kms_chamelium@vga-hpd-for-each-pipe:
    - shard-glk:          NOTRUN -> [SKIP][51] ([fdo#109271] / [fdo#111827]) +5 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk4/igt@kms_chamelium@vga-hpd-for-each-pipe.html

  * igt@kms_color@pipe-b-ctm-0-25:
    - shard-skl:          NOTRUN -> [DMESG-WARN][52] ([i915#1982])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl1/igt@kms_color@pipe-b-ctm-0-25.html

  * igt@kms_color@pipe-c-ctm-negative:
    - shard-skl:          [PASS][53] -> [DMESG-WARN][54] ([i915#1982])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl9/igt@kms_color@pipe-c-ctm-negative.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl8/igt@kms_color@pipe-c-ctm-negative.html

  * igt@kms_color@pipe-d-ctm-max:
    - shard-skl:          NOTRUN -> [SKIP][55] ([fdo#109271]) +65 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl1/igt@kms_color@pipe-d-ctm-max.html

  * igt@kms_color_chamelium@pipe-b-degamma:
    - shard-kbl:          NOTRUN -> [SKIP][56] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl3/igt@kms_color_chamelium@pipe-b-degamma.html

  * igt@kms_color_chamelium@pipe-d-ctm-0-25:
    - shard-apl:          NOTRUN -> [SKIP][57] ([fdo#109271] / [fdo#111827]) +10 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl7/igt@kms_color_chamelium@pipe-d-ctm-0-25.html

  * igt@kms_content_protection@uevent:
    - shard-tglb:         NOTRUN -> [SKIP][58] ([fdo#111828])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_content_protection@uevent.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][59] -> [DMESG-WARN][60] ([i915#180]) +4 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
    - shard-skl:          [PASS][61] -> [INCOMPLETE][62] ([i915#146] / [i915#2828] / [i915#300])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl3/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-max-size-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][63] ([i915#3359]) +3 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_cursor_crc@pipe-c-cursor-max-size-rapid-movement.html

  * igt@kms_cursor_crc@pipe-d-cursor-256x85-rapid-movement:
    - shard-glk:          NOTRUN -> [SKIP][64] ([fdo#109271]) +35 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk4/igt@kms_cursor_crc@pipe-d-cursor-256x85-rapid-movement.html

  * igt@kms_cursor_crc@pipe-d-cursor-512x512-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][65] ([fdo#109279] / [i915#3359]) +2 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb5/igt@kms_cursor_crc@pipe-d-cursor-512x512-rapid-movement.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-varying-size:
    - shard-tglb:         NOTRUN -> [SKIP][66] ([i915#4103]) +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-varying-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][67] -> [FAIL][68] ([i915#2346])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [PASS][69] -> [FAIL][70] ([i915#2346] / [i915#533])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_dp_tiled_display@basic-test-pattern:
    - shard-tglb:         NOTRUN -> [SKIP][71] ([i915#426])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_dp_tiled_display@basic-test-pattern.html

  * igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][72] -> [FAIL][73] ([i915#79])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-apl:          NOTRUN -> [DMESG-WARN][74] ([i915#180])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl6/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_flip@plain-flip-fb-recreate@a-dp1:
    - shard-apl:          [PASS][75] -> [FAIL][76] ([i915#2122])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-apl3/igt@kms_flip@plain-flip-fb-recreate@a-dp1.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl6/igt@kms_flip@plain-flip-fb-recreate@a-dp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs:
    - shard-tglb:         NOTRUN -> [SKIP][77] ([i915#2587])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb8/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-render:
    - shard-tglb:         NOTRUN -> [SKIP][78] ([fdo#111825]) +17 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb8/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-render.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - shard-skl:          NOTRUN -> [SKIP][79] ([fdo#109271] / [i915#533]) +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl1/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:
    - shard-kbl:          NOTRUN -> [FAIL][80] ([i915#265])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl3/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          NOTRUN -> [FAIL][81] ([fdo#108145] / [i915#265])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> [FAIL][82] ([fdo#108145] / [i915#265])
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl2/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html
    - shard-apl:          NOTRUN -> [FAIL][83] ([fdo#108145] / [i915#265]) +1 similar issue
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl2/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][84] -> [FAIL][85] ([fdo#108145] / [i915#265])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-none:
    - shard-tglb:         NOTRUN -> [SKIP][86] ([i915#3536]) +3 similar issues
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb8/igt@kms_plane_lowres@pipe-a-tiling-none.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-1:
    - shard-kbl:          NOTRUN -> [SKIP][87] ([fdo#109271] / [i915#658])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl3/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-1.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3:
    - shard-glk:          NOTRUN -> [SKIP][88] ([fdo#109271] / [i915#658])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk4/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-5:
    - shard-apl:          NOTRUN -> [SKIP][89] ([fdo#109271] / [i915#658])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl6/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-5.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-3:
    - shard-tglb:         NOTRUN -> [SKIP][90] ([i915#2920]) +1 similar issue
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-3.html

  * igt@kms_psr@psr2_dpms:
    - shard-tglb:         NOTRUN -> [FAIL][91] ([i915#132] / [i915#3467]) +1 similar issue
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb6/igt@kms_psr@psr2_dpms.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][92] -> [SKIP][93] ([fdo#109441]) +3 similar issues
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_sysfs_edid_timing:
    - shard-kbl:          NOTRUN -> [FAIL][94] ([IGT#2])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl3/igt@kms_sysfs_edid_timing.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          [PASS][95] -> [INCOMPLETE][96] ([i915#198] / [i915#2828])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl8/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl8/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@kms_writeback@writeback-fb-id:
    - shard-skl:          NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#2437])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl6/igt@kms_writeback@writeback-fb-id.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-tglb:         NOTRUN -> [SKIP][98] ([i915#2437])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb5/igt@kms_writeback@writeback-invalid-parameters.html

  * igt@nouveau_crc@pipe-d-source-outp-inactive:
    - shard-tglb:         NOTRUN -> [SKIP][99] ([i915#2530])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@nouveau_crc@pipe-d-source-outp-inactive.html

  * igt@perf_pmu@rc6-suspend:
    - shard-apl:          [PASS][100] -> [DMESG-WARN][101] ([i915#180])
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-apl8/igt@perf_pmu@rc6-suspend.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl6/igt@perf_pmu@rc6-suspend.html

  * igt@prime_nv_api@i915_self_import:
    - shard-tglb:         NOTRUN -> [SKIP][102] ([fdo#109291]) +3 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@prime_nv_api@i915_self_import.html

  * igt@sysfs_clients@create:
    - shard-apl:          NOTRUN -> [SKIP][103] ([fdo#109271] / [i915#2994]) +2 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl7/igt@sysfs_clients@create.html

  * igt@sysfs_clients@split-50:
    - shard-kbl:          NOTRUN -> [SKIP][104] ([fdo#109271] / [i915#2994])
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl2/igt@sysfs_clients@split-50.html

  * igt@sysfs_heartbeat_interval@precise@vecs0:
    - shard-iclb:         [PASS][105] -> [FAIL][106] ([i915#1755])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb5/igt@sysfs_heartbeat_interval@precise@vecs0.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb4/igt@sysfs_heartbeat_interval@precise@vecs0.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-contexts-10ms:
    - shard-iclb:         [TIMEOUT][107] ([i915#3070]) -> [PASS][108]
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb5/igt@gem_eio@in-flight-contexts-10ms.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb1/igt@gem_eio@in-flight-contexts-10ms.html

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-glk:          [FAIL][109] ([i915#2842]) -> [PASS][110] +1 similar issue
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-glk4/igt@gem_exec_fair@basic-none-rrul@rcs0.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk2/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [FAIL][111] ([i915#2842]) -> [PASS][112]
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb2/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb3/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-iclb:         [FAIL][113] ([i915#2842]) -> [PASS][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb7/igt@gem_exec_fair@basic-pace@bcs0.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb8/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-kbl:          [SKIP][115] ([fdo#109271]) -> [PASS][116]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-kbl4/igt@gem_exec_fair@basic-pace@vcs0.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl7/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-kbl:          [FAIL][117] ([i915#2842]) -> [PASS][118]
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-kbl4/igt@gem_exec_fair@basic-pace@vcs1.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl7/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@gem_huc_copy@huc-copy:
    - shard-tglb:         [SKIP][119] ([i915#2190]) -> [PASS][120]
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb7/igt@gem_huc_copy@huc-copy.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb8/igt@gem_huc_copy@huc-copy.html

  * igt@i915_module_load@reload-no-display:
    - shard-iclb:         [DMESG-WARN][121] ([i915#2867]) -> [PASS][122]
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-iclb3/igt@i915_module_load@reload-no-display.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-iclb3/igt@i915_module_load@reload-no-display.html

  * igt@i915_suspend@forcewake:
    - shard-tglb:         [INCOMPLETE][123] ([i915#2411] / [i915#456]) -> [PASS][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb7/igt@i915_suspend@forcewake.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb8/igt@i915_suspend@forcewake.html

  * igt@i915_suspend@sysfs-reader:
    - shard-skl:          [INCOMPLETE][125] ([i915#198]) -> [PASS][126]
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl2/igt@i915_suspend@sysfs-reader.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl6/igt@i915_suspend@sysfs-reader.html

  * igt@kms_color@pipe-b-ctm-0-75:
    - shard-skl:          [DMESG-WARN][127] ([i915#1982]) -> [PASS][128]
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-skl6/igt@kms_color@pipe-b-ctm-0-75.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-skl6/igt@kms_color@pipe-b-ctm-0-75.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-tglb:         [INCOMPLETE][129] ([i915#2411] / [i915#2828] / [i915#456]) -> [PASS][130]
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-tglb7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-tglb5/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][131] ([i915#180]) -> [PASS][132] +3 similar issues
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2:
    - shard-glk:          [FAIL][133] ([i915#2122]) -> [PASS][134]
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-glk5/igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-glk6/igt@kms_flip@2x-plain-flip-ts-check@bc-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-expired-vblank@a-dp1:
    - shard-kbl:          [FAIL][135] ([i915#2122]) -> [PASS][136]
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-kbl4/igt@kms_flip@flip-vs-expired-vblank@a-dp1.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-kbl2/igt@kms_flip@flip-vs-expired-vblank@a-dp1.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes:
    - shard-apl:          [DMESG-WARN][137] ([i915#180]) -> [PASS][138]
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10671/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21214/shard-apl

== Logs ==

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

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

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

* Re: [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
  (?)
@ 2021-10-01 12:48   ` kernel test robot
  -1 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-01 12:48 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx
  Cc: llvm, kbuild-all, dri-devel, Jani Nikula, Masahiro Yamada,
	Steven Price, Andrzej Hajda

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

Hi Lucas,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.15-rc3 next-20210922]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a015-20211001 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/50006042f1d264599bd1be1942f9958112e15c01
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
        git checkout 50006042f1d264599bd1be1942f9958112e15c01
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_config.c:11:14: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_config.c:11:14: note: use '&' for a bitwise operation
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^~
                       &
   drivers/gpu/drm/i915/i915_config.c:11:14: note: remove constant to silence this warning
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +11 drivers/gpu/drm/i915/i915_config.c

     7	
     8	unsigned long
     9	i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
    10	{
  > 11		if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40607 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-01 12:48   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-01 12:48 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx
  Cc: llvm, kbuild-all, dri-devel, Jani Nikula, Masahiro Yamada,
	Steven Price, Andrzej Hajda

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

Hi Lucas,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.15-rc3 next-20210922]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a015-20211001 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/50006042f1d264599bd1be1942f9958112e15c01
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
        git checkout 50006042f1d264599bd1be1942f9958112e15c01
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_config.c:11:14: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_config.c:11:14: note: use '&' for a bitwise operation
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^~
                       &
   drivers/gpu/drm/i915/i915_config.c:11:14: note: remove constant to silence this warning
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +11 drivers/gpu/drm/i915/i915_config.c

     7	
     8	unsigned long
     9	i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
    10	{
  > 11		if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40607 bytes --]

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

* Re: [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-01 12:48   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-01 12:48 UTC (permalink / raw)
  To: kbuild-all

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

Hi Lucas,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.15-rc3 next-20210922]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a015-20211001 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/50006042f1d264599bd1be1942f9958112e15c01
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
        git checkout 50006042f1d264599bd1be1942f9958112e15c01
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_config.c:11:14: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_config.c:11:14: note: use '&' for a bitwise operation
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^~
                       &
   drivers/gpu/drm/i915/i915_config.c:11:14: note: remove constant to silence this warning
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +11 drivers/gpu/drm/i915/i915_config.c

     7	
     8	unsigned long
     9	i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
    10	{
  > 11		if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 40607 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  9:57   ` Jani Nikula
@ 2021-10-01 16:17     ` Lucas De Marchi
  2021-10-04 20:52     ` Lucas De Marchi
  1 sibling, 0 replies; 19+ messages in thread
From: Lucas De Marchi @ 2021-10-01 16:17 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Chris Wilson, dri-devel, Masahiro Yamada, Steven Price,
	Andrzej Hajda, intel-gfx, Sarvela, Tomi P

On Fri, Oct 01, 2021 at 12:57:13PM +0300, Jani Nikula wrote:
>On Fri, 01 Oct 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Quoting Lucas De Marchi (2021-10-01 08:40:41)
>>> 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 would potentially trigger a warning. All the others
>>>   can simply use the shorter version, without wrapping it in any macro.
>>> And even that single one didn't trigger any warning in gcc 10.3.
>>>
>>> So here I'm dialing all the way back to simply removing the macro. If it
>>> triggers warnings in future we may change the few cases to check for > 0
>>> or != 0. Another possibility would be to use the great "not not
>>> operator" for all positive checks, which would allow us to maintain
>>> consistency.  However let's try first the simplest form though, hopefully
>>> we don't hit broken compilers spitting a warning:
>>
>> You didn't prevent the compilation warning this re-introduces.
>>
>> drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?
>> drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op?
>
>Looks like that's a Smatch warning. The immediate fix would be to just
>add the != 0 in the relevant places. But this is stuff that's just going
>to get broken again unless we add Smatch to CI. Most people aren't
>running it on a regular basis.

it looks like the warning is also triggered with clang an as per
0day test.  What if we change all the positive caes to use !!?  That
would make it  consistent and IMO not as ugly as the != 0.

what about this?

-----8<----
diff --git a/drivers/gpu/drm/i915/i915_config.c b/drivers/gpu/drm/i915/i915_config.c
index ad228dd96a0b..0df3efa2e72d 100644
--- a/drivers/gpu/drm/i915/i915_config.c
+++ b/drivers/gpu/drm/i915/i915_config.c
@@ -8,7 +8,7 @@
  unsigned long
  i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
  {
-	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
+	if (context && !!CONFIG_DRM_I915_FENCE_TIMEOUT)
  		return msecs_to_jiffies_timeout(CONFIG_DRM_I915_FENCE_TIMEOUT);
  
  	return 0;
-----8<----

Lucas De Marchi

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

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: remove IS_ACTIVE (rev2)
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
                   ` (5 preceding siblings ...)
  (?)
@ 2021-10-01 22:07 ` Patchwork
  -1 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2021-10-01 22:07 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: remove IS_ACTIVE (rev2)
URL   : https://patchwork.freedesktop.org/series/95312/
State : failure

== Summary ==

Applying: drm/i915: remove IS_ACTIVE
error: patch failed: drivers/gpu/drm/i915/i915_config.c:8
error: drivers/gpu/drm/i915/i915_config.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_config.c
Patch failed at 0001 drm/i915: remove IS_ACTIVE
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



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

* Re: [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
  (?)
@ 2021-10-02  5:47   ` kernel test robot
  -1 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-02  5:47 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx
  Cc: llvm, kbuild-all, dri-devel, Jani Nikula, Masahiro Yamada,
	Steven Price, Andrzej Hajda

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

Hi Lucas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.15-rc3 next-20210823]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-r024-20211001 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/50006042f1d264599bd1be1942f9958112e15c01
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
        git checkout 50006042f1d264599bd1be1942f9958112e15c01
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_config.c:11:14: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand]
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_config.c:11:14: note: use '&' for a bitwise operation
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^~
                       &
   drivers/gpu/drm/i915/i915_config.c:11:14: note: remove constant to silence this warning
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +11 drivers/gpu/drm/i915/i915_config.c

     7	
     8	unsigned long
     9	i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
    10	{
  > 11		if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36679 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-02  5:47   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-02  5:47 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx
  Cc: llvm, kbuild-all, dri-devel, Jani Nikula, Masahiro Yamada,
	Steven Price, Andrzej Hajda

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

Hi Lucas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.15-rc3 next-20210823]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-r024-20211001 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/50006042f1d264599bd1be1942f9958112e15c01
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
        git checkout 50006042f1d264599bd1be1942f9958112e15c01
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_config.c:11:14: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand]
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_config.c:11:14: note: use '&' for a bitwise operation
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^~
                       &
   drivers/gpu/drm/i915/i915_config.c:11:14: note: remove constant to silence this warning
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +11 drivers/gpu/drm/i915/i915_config.c

     7	
     8	unsigned long
     9	i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
    10	{
  > 11		if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36679 bytes --]

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

* Re: [PATCH] drm/i915: remove IS_ACTIVE
@ 2021-10-02  5:47   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-02  5:47 UTC (permalink / raw)
  To: kbuild-all

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

Hi Lucas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.15-rc3 next-20210823]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-r024-20211001 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/50006042f1d264599bd1be1942f9958112e15c01
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lucas-De-Marchi/drm-i915-remove-IS_ACTIVE/20211001-154226
        git checkout 50006042f1d264599bd1be1942f9958112e15c01
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/i915_config.c:11:14: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand]
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/i915_config.c:11:14: note: use '&' for a bitwise operation
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                       ^~
                       &
   drivers/gpu/drm/i915/i915_config.c:11:14: note: remove constant to silence this warning
           if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +11 drivers/gpu/drm/i915/i915_config.c

     7	
     8	unsigned long
     9	i915_fence_context_timeout(const struct drm_i915_private *i915, u64 context)
    10	{
  > 11		if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36679 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-01  9:57   ` Jani Nikula
  2021-10-01 16:17     ` Lucas De Marchi
@ 2021-10-04 20:52     ` Lucas De Marchi
  2021-10-05  6:19       ` Dan Carpenter
  1 sibling, 1 reply; 19+ messages in thread
From: Lucas De Marchi @ 2021-10-04 20:52 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Chris Wilson, dri-devel, Masahiro Yamada, Steven Price,
	Andrzej Hajda, intel-gfx, Sarvela, Tomi P, Dan Carpenter

Cc'ing Dan Carpenter

On Fri, Oct 01, 2021 at 12:57:13PM +0300, Jani Nikula wrote:
>On Fri, 01 Oct 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Quoting Lucas De Marchi (2021-10-01 08:40:41)
>>> 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 would potentially trigger a warning. All the others
>>>   can simply use the shorter version, without wrapping it in any macro.
>>> And even that single one didn't trigger any warning in gcc 10.3.
>>>
>>> So here I'm dialing all the way back to simply removing the macro. If it
>>> triggers warnings in future we may change the few cases to check for > 0
>>> or != 0. Another possibility would be to use the great "not not
>>> operator" for all positive checks, which would allow us to maintain
>>> consistency.  However let's try first the simplest form though, hopefully
>>> we don't hit broken compilers spitting a warning:
>>
>> You didn't prevent the compilation warning this re-introduces.
>>
>> drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?
>> drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op?
>
>Looks like that's a Smatch warning. The immediate fix would be to just
>add the != 0 in the relevant places. But this is stuff that's just going
>to get broken again unless we add Smatch to CI. Most people aren't
>running it on a regular basis.

clang gives a warning only in drivers/gpu/drm/i915/i915_config.c and the
warning is gone if the condition swapped:

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

which would make sense if we think about shortcutting the if condition.
However smatch still reports the warning and an additional one
in drivers/gpu/drm/i915/i915_request.c. The ways I found to stop the
false positives with smatch are:

if (context && CONFIG_DRM_I915_FENCE_TIMEOUT != 0)
or
if (context && !!CONFIG_DRM_I915_FENCE_TIMEOUT)
or
if (context && CONFIG_DRM_I915_FENCE_TIMEOUT > 0)

Dan, anything else that we could do here?  This is about this kind of
code:

	f (context && CONFIG_DRM_I915_FENCE_TIMEOUT)

in which context is a u64 variable, that gives this warning:

drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?

thanks
Lucas De Marchi

>
>BR,
>Jani.
>
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-04 20:52     ` Lucas De Marchi
@ 2021-10-05  6:19       ` Dan Carpenter
  2021-10-05  7:13         ` Lucas De Marchi
  0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2021-10-05  6:19 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Jani Nikula, Chris Wilson, dri-devel, Masahiro Yamada,
	Steven Price, Andrzej Hajda, intel-gfx, Sarvela, Tomi P

On Mon, Oct 04, 2021 at 01:52:27PM -0700, Lucas De Marchi wrote:
> Cc'ing Dan Carpenter
> 
> On Fri, Oct 01, 2021 at 12:57:13PM +0300, Jani Nikula wrote:
> > On Fri, 01 Oct 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > Quoting Lucas De Marchi (2021-10-01 08:40:41)
> > > > 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 would potentially trigger a warning. All the others
> > > >   can simply use the shorter version, without wrapping it in any macro.
> > > > And even that single one didn't trigger any warning in gcc 10.3.
> > > > 
> > > > So here I'm dialing all the way back to simply removing the macro. If it
> > > > triggers warnings in future we may change the few cases to check for > 0
> > > > or != 0. Another possibility would be to use the great "not not
> > > > operator" for all positive checks, which would allow us to maintain
> > > > consistency.  However let's try first the simplest form though, hopefully
> > > > we don't hit broken compilers spitting a warning:
> > > 
> > > You didn't prevent the compilation warning this re-introduces.
> > > 
> > > drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?
> > > drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op?
> > 
> > Looks like that's a Smatch warning. The immediate fix would be to just
> > add the != 0 in the relevant places. But this is stuff that's just going
> > to get broken again unless we add Smatch to CI. Most people aren't
> > running it on a regular basis.

I would really prefer that instead of ensuring that code doesn't
generate Smatch warnings, people just look over the warnings and then
mass mark them all as false positives and never look at them again.

It let's us warn about more complicated things without worrying so much
about being perfect.  When code is fresh in your head then warnings are
not a big deal to review and you want to warn about every possible issue
After a year then they take forever and so you really want them to be
correct or it's a huge waste of time.  I'd prefer Smatch live in the
space where people run it when the code is fresh.

You would have received some automated emails about this Smatch warning
but I look over the zero day output and filter the results.

> 
> clang gives a warning only in drivers/gpu/drm/i915/i915_config.c and the
> warning is gone if the condition swapped:
> 
> -	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
> +	if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)

I like this rule that when the constant is on the left it's not a mask.
That makes sense.  I will add that.

> 
> which would make sense if we think about shortcutting the if condition.
> However smatch still reports the warning and an additional one
> in drivers/gpu/drm/i915/i915_request.c. The ways I found to stop the
> false positives with smatch are:
> 
> if (context && CONFIG_DRM_I915_FENCE_TIMEOUT != 0)
> or
> if (context && !!CONFIG_DRM_I915_FENCE_TIMEOUT)
> or
> if (context && CONFIG_DRM_I915_FENCE_TIMEOUT > 0)
> 

I guess I prefer the first and third but I'll add the rule that Clang
uses to silence the warning.

regards,
dan carpenter


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

* Re: [Intel-gfx] [PATCH] drm/i915: remove IS_ACTIVE
  2021-10-05  6:19       ` Dan Carpenter
@ 2021-10-05  7:13         ` Lucas De Marchi
  0 siblings, 0 replies; 19+ messages in thread
From: Lucas De Marchi @ 2021-10-05  7:13 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Jani Nikula, Chris Wilson, dri-devel, Masahiro Yamada,
	Steven Price, Andrzej Hajda, intel-gfx, Sarvela, Tomi P

On Tue, Oct 05, 2021 at 09:19:39AM +0300, Dan Carpenter wrote:
>On Mon, Oct 04, 2021 at 01:52:27PM -0700, Lucas De Marchi wrote:
>> Cc'ing Dan Carpenter
>>
>> On Fri, Oct 01, 2021 at 12:57:13PM +0300, Jani Nikula wrote:
>> > On Fri, 01 Oct 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > > Quoting Lucas De Marchi (2021-10-01 08:40:41)
>> > > > 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 would potentially trigger a warning. All the others
>> > > >   can simply use the shorter version, without wrapping it in any macro.
>> > > > And even that single one didn't trigger any warning in gcc 10.3.
>> > > >
>> > > > So here I'm dialing all the way back to simply removing the macro. If it
>> > > > triggers warnings in future we may change the few cases to check for > 0
>> > > > or != 0. Another possibility would be to use the great "not not
>> > > > operator" for all positive checks, which would allow us to maintain
>> > > > consistency.  However let's try first the simplest form though, hopefully
>> > > > we don't hit broken compilers spitting a warning:
>> > >
>> > > You didn't prevent the compilation warning this re-introduces.
>> > >
>> > > drivers/gpu/drm/i915/i915_config.c:11 i915_fence_context_timeout() warn: should this be a bitwise op?
>> > > drivers/gpu/drm/i915/i915_request.c:1679 i915_request_wait() warn: should this be a bitwise op?
>> >
>> > Looks like that's a Smatch warning. The immediate fix would be to just
>> > add the != 0 in the relevant places. But this is stuff that's just going
>> > to get broken again unless we add Smatch to CI. Most people aren't
>> > running it on a regular basis.
>
>I would really prefer that instead of ensuring that code doesn't
>generate Smatch warnings, people just look over the warnings and then
>mass mark them all as false positives and never look at them again.
>
>It let's us warn about more complicated things without worrying so much
>about being perfect.  When code is fresh in your head then warnings are
>not a big deal to review and you want to warn about every possible issue
>After a year then they take forever and so you really want them to be
>correct or it's a huge waste of time.  I'd prefer Smatch live in the
>space where people run it when the code is fresh.
>
>You would have received some automated emails about this Smatch warning
>but I look over the zero day output and filter the results.
>
>>
>> clang gives a warning only in drivers/gpu/drm/i915/i915_config.c and the
>> warning is gone if the condition swapped:
>>
>> -	if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
>> +	if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)
>
>I like this rule that when the constant is on the left it's not a mask.
>That makes sense.  I will add that.

thanks, that would be great, so we can really get rid of the macro by
sticking this rule since it works for smatch and clang (and gcc doesn't
give this warning).


thanks
Lucas De Marchi


>
>>
>> which would make sense if we think about shortcutting the if condition.
>> However smatch still reports the warning and an additional one
>> in drivers/gpu/drm/i915/i915_request.c. The ways I found to stop the
>> false positives with smatch are:
>>
>> if (context && CONFIG_DRM_I915_FENCE_TIMEOUT != 0)
>> or
>> if (context && !!CONFIG_DRM_I915_FENCE_TIMEOUT)
>> or
>> if (context && CONFIG_DRM_I915_FENCE_TIMEOUT > 0)
>>
>
>I guess I prefer the first and third but I'll add the rule that Clang
>uses to silence the warning.
>
>regards,
>dan carpenter
>

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

end of thread, other threads:[~2021-10-05  7:13 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-01  7:40 [PATCH] drm/i915: remove IS_ACTIVE Lucas De Marchi
2021-10-01  7:40 ` [Intel-gfx] " Lucas De Marchi
2021-10-01  7:46 ` Jani Nikula
2021-10-01  7:46   ` [Intel-gfx] " Jani Nikula
2021-10-01  9:11 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2021-10-01  9:29 ` [Intel-gfx] [PATCH] " Chris Wilson
2021-10-01  9:57   ` Jani Nikula
2021-10-01 16:17     ` Lucas De Marchi
2021-10-04 20:52     ` Lucas De Marchi
2021-10-05  6:19       ` Dan Carpenter
2021-10-05  7:13         ` Lucas De Marchi
2021-10-01 10:51 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for " Patchwork
2021-10-01 12:48 ` [PATCH] " kernel test robot
2021-10-01 12:48   ` kernel test robot
2021-10-01 12:48   ` [Intel-gfx] " kernel test robot
2021-10-01 22:07 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: remove IS_ACTIVE (rev2) Patchwork
2021-10-02  5:47 ` [PATCH] drm/i915: remove IS_ACTIVE kernel test robot
2021-10-02  5:47   ` kernel test robot
2021-10-02  5:47   ` [Intel-gfx] " kernel test robot

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.