Commit c7302f20 authored by Chris Wilson's avatar Chris Wilson
Browse files

drm/i915: Defer final intel_wakeref_put to process context 

As we need to acquire a mutex to serialise the final
intel_wakeref_put, we need to ensure that we are in process context at
that time. However, we want to allow operation on the intel_wakeref from
inside timer and other hardirq context, which means that need to defer
that final put to a workqueue.

Inside the final wakeref puts, we are safe to operate in any context, as
we are simply marking up the HW and state tracking for the potential
sleep. It's only the serialisation with the potential sleeping getting
that requires careful wait avoidance. This allows us to retain the
immediate processing as before (we only need to sleep over the same
races as the current mutex_lock).

v2: Add a selftest to ensure we exercise the code while lockdep watches.
v3: That test was extremely loud and complained about many things!
v4: Not a whale!

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111295
References: https://bugs.freedesktop.org/show_bug.cgi?id=111245
References: https://bugs.freedesktop.org/show_bug.cgi?id=111256


Fixes: 18398904 ("drm/i915: Only recover active engines")
Fixes: 51fbd8de ("drm/i915/pmu: Atomically acquire the gt_pm wakeref")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190808202758.10453-1-chris@chris-wilson.co.uk
parent cbb153c5

drivers/gpu/drm/i915/gem/i915_gem_pm.c

+3 −10
Original line number Diff line number Diff line
@@ -130,7 +130,9 @@ static bool switch_to_kernel_context_sync(struct intel_gt *gt) 
		}

	} while (i915_retire_requests(gt->i915) && result);



	GEM_BUG_ON(gt->awake);

	if (intel_gt_pm_wait_for_idle(gt))

		result = false;



	return result;

}
@@ -161,13 +163,6 @@ void i915_gem_suspend(struct drm_i915_private *i915) 


	mutex_unlock(&i915->drm.struct_mutex);



	/*

	 * Assert that we successfully flushed all the work and

	 * reset the GPU back to its idle, low power state.

	 */

	GEM_BUG_ON(i915->gt.awake);

	flush_work(&i915->gem.idle_work);



	cancel_delayed_work_sync(&i915->gt.hangcheck.work);



	i915_gem_drain_freed_objects(i915);
@@ -244,8 +239,6 @@ void i915_gem_resume(struct drm_i915_private *i915) 
{

	GEM_TRACE("\n");



	WARN_ON(i915->gt.awake);



	mutex_lock(&i915->drm.struct_mutex);

	intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL);

drivers/gpu/drm/i915/gt/intel_engine_cs.c

+1 −0
Original line number Diff line number Diff line
@@ -1577,5 +1577,6 @@ intel_engine_find_active_request(struct intel_engine_cs *engine) 


#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)

#include "mock_engine.c"

#include "selftest_engine.c"

#include "selftest_engine_cs.c"

#endif

drivers/gpu/drm/i915/gt/intel_engine_pm.c

+11 −27
Original line number Diff line number Diff line
@@ -37,28 +37,6 @@ static int __engine_unpark(struct intel_wakeref *wf) 
	return 0;

}



void intel_engine_pm_get(struct intel_engine_cs *engine)

{

	intel_wakeref_get(&engine->i915->runtime_pm, &engine->wakeref, __engine_unpark);

}



void intel_engine_park(struct intel_engine_cs *engine)

{

	/*

	 * We are committed now to parking this engine, make sure there

	 * will be no more interrupts arriving later and the engine

	 * is truly idle.

	 */

	if (wait_for(intel_engine_is_idle(engine), 10)) {

		struct drm_printer p = drm_debug_printer(__func__);



		dev_err(engine->i915->drm.dev,

			"%s is not idle before parking\n",

			engine->name);

		intel_engine_dump(engine, &p, NULL);

	}

}



static bool switch_to_kernel_context(struct intel_engine_cs *engine)

{

	struct i915_request *rq;
@@ -136,12 +114,18 @@ static int __engine_park(struct intel_wakeref *wf) 
	return 0;

}



void intel_engine_pm_put(struct intel_engine_cs *engine)

{

	intel_wakeref_put(&engine->i915->runtime_pm, &engine->wakeref, __engine_park);

}

static const struct intel_wakeref_ops wf_ops = {

	.get = __engine_unpark,

	.put = __engine_park,

};



void intel_engine_init__pm(struct intel_engine_cs *engine)

{

	intel_wakeref_init(&engine->wakeref);

	struct intel_runtime_pm *rpm = &engine->i915->runtime_pm;



	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);

}



#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)

#include "selftest_engine_pm.c"

#endif

drivers/gpu/drm/i915/gt/intel_engine_pm.h

+10 −8
Original line number Diff line number Diff line
@@ -10,24 +10,26 @@ 
#include "intel_engine_types.h"

#include "intel_wakeref.h"



struct drm_i915_private;



void intel_engine_pm_get(struct intel_engine_cs *engine);

void intel_engine_pm_put(struct intel_engine_cs *engine);



static inline bool

intel_engine_pm_is_awake(const struct intel_engine_cs *engine)

{

	return intel_wakeref_is_active(&engine->wakeref);

}



static inline bool

intel_engine_pm_get_if_awake(struct intel_engine_cs *engine)

static inline void intel_engine_pm_get(struct intel_engine_cs *engine)

{

	intel_wakeref_get(&engine->wakeref);

}



static inline bool intel_engine_pm_get_if_awake(struct intel_engine_cs *engine)

{

	return intel_wakeref_get_if_active(&engine->wakeref);

}



void intel_engine_park(struct intel_engine_cs *engine);

static inline void intel_engine_pm_put(struct intel_engine_cs *engine)

{

	intel_wakeref_put(&engine->wakeref);

}



void intel_engine_init__pm(struct intel_engine_cs *engine);

drivers/gpu/drm/i915/gt/intel_gt_pm.c

+12 −16
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@ static void pm_notify(struct drm_i915_private *i915, int state) 
	blocking_notifier_call_chain(&i915->gt.pm_notifications, state, i915);

}



static int intel_gt_unpark(struct intel_wakeref *wf)

static int __gt_unpark(struct intel_wakeref *wf)

{

	struct intel_gt *gt = container_of(wf, typeof(*gt), wakeref);

	struct drm_i915_private *i915 = gt->i915;
@@ -53,14 +53,7 @@ static int intel_gt_unpark(struct intel_wakeref *wf) 
	return 0;

}



void intel_gt_pm_get(struct intel_gt *gt)

{

	struct intel_runtime_pm *rpm = &gt->i915->runtime_pm;



	intel_wakeref_get(rpm, &gt->wakeref, intel_gt_unpark);

}



static int intel_gt_park(struct intel_wakeref *wf)

static int __gt_park(struct intel_wakeref *wf)

{

	struct drm_i915_private *i915 =

		container_of(wf, typeof(*i915), gt.wakeref);
@@ -74,22 +67,25 @@ static int intel_gt_park(struct intel_wakeref *wf) 
	if (INTEL_GEN(i915) >= 6)

		gen6_rps_idle(i915);



	/* Everything switched off, flush any residual interrupt just in case */

	intel_synchronize_irq(i915);



	GEM_BUG_ON(!wakeref);

	intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ, wakeref);



	return 0;

}



void intel_gt_pm_put(struct intel_gt *gt)

{

	struct intel_runtime_pm *rpm = &gt->i915->runtime_pm;



	intel_wakeref_put(rpm, &gt->wakeref, intel_gt_park);

}

static const struct intel_wakeref_ops wf_ops = {

	.get = __gt_unpark,

	.put = __gt_park,

	.flags = INTEL_WAKEREF_PUT_ASYNC,

};



void intel_gt_pm_init_early(struct intel_gt *gt)

{

	intel_wakeref_init(&gt->wakeref);

	intel_wakeref_init(&gt->wakeref, &gt->i915->runtime_pm, &wf_ops);



	BLOCKING_INIT_NOTIFIER_HEAD(&gt->pm_notifications);

}