Skip to content

Commit

Permalink
drm/i915: remove IS_ACTIVE
Browse files Browse the repository at this point in the history
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 babaab2 ("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>
  • Loading branch information
lucasdemarchi authored and intel-lab-lkp committed Oct 1, 2021
1 parent 7f6002e commit 5000604
Show file tree
Hide file tree
Showing 11 changed files with 18 additions and 31 deletions.
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,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;

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_mman.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/i915/gt/intel_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gt/intel_engine_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,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;
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/gt/intel_execlists_submission.c
Original file line number Diff line number Diff line change
Expand Up @@ -3341,7 +3341,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;
}
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions drivers/gpu/drm/i915/gt/selftest_execlists.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/i915_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/i915_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -1859,7 +1859,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;

Expand Down
13 changes: 0 additions & 13 deletions drivers/gpu/drm/i915/i915_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

0 comments on commit 5000604

Please sign in to comment.