Re: [PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/04/2017 18:58, Chris Wilson wrote:
> We indirectly hold the runtime-pm for the intel_lrc_irq_handler() by
> virtue of dev_priv->gt.awake keeping a wakeref whilst the requests are
> busy. As this is not obvious from the code, add a comment.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0dc1cc4ad6e7..e16cc28dc783 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -515,6 +515,15 @@ static void intel_lrc_irq_handler(unsigned long data)
>  	struct execlist_port *port = engine->execlist_port;
>  	struct drm_i915_private *dev_priv = engine->i915;
>
> +	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
> +	 * on our behalf by the request (see i915_gem_mark_busy ()) and it will
> +	 * not be relinquished until the device is idle (see
> +	 * i915_gem_idle_work_handler()). As a precaution, we make sure
> +	 * that all ELSP are drained i.e. we have processed the CSB,
> +	 * before allowing ourselves to idle and calling intel_runtime_pm_put().
> +	 */
> +	GEM_BUG_ON(!dev_priv->gt.awake);
> +
>  	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
>
>  	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It feels a big to c&p, but not sure if it worth putting a comment "see 
comment in intel_lrc_irq_handler on why we don't have to " to 
i915_guc_irq_handler or maybe dequeue?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx