Re: [RFC] drm/i915: Temporarily go realtime when polling PCODE

[Imre Deak <imre.deak@intel.com>]

On Tue, Feb 21, 2017 at 05:01:58PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Elevate task scheduling policy to realtime when polling on PCODE
> to guarantee a good poll rate before falling back to busy wait.
> 
> We only do this for tasks with normal policy and priority in
> order  to simplify policy restore and also assuming that for
> tasks which either made themselves low or high priority it makes
> less sense to do so.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> This was my idea as mentioned in the other thread.
> 
> Deadline scheduling policy seems trickier to restore from so
> I thought SCHED_FIFO should be good enough.
> 
> Briefly tested but couldn't reproduce the timeout condition.

Hm, I thought you wanted this instead of the preempt-disable poll. The
first preempt-enable poll is what's based on the spec, which only
requires two requests 3ms apart, so no requirement on the number of
requests there. That works most of the time and the preempt-disable part
is needed only rarely. So do we want to increase the priority for the
normal case?

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 33 +++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_pm.c  |  4 +++-
>  2 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 821c57cab406..70033bdd183e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -51,9 +51,24 @@
>   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>   * added.
>   */
> -#define _wait_for(COND, US, W) ({ \
> +#define _wait_for(COND, US, W, DEADLINE) ({ \
>  	unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;	\
> -	int ret__;							\
> +	int sched__, ret__;						\
> +									\
> +	if ((DEADLINE) && (W)) {					\
> +		struct task_struct *t = current;			\
> +									\
> +		if (t->policy != 0 || task_nice(t) != 0) {		\
> +			sched__ = -EINVAL;				\
> +		} else {						\
> +			struct sched_param param =			\
> +				{ .sched_priority = MAX_RT_PRIO - 1 };	\
> +			sched__ = sched_setscheduler_nocheck(t,		\
> +							     SCHED_FIFO,\
> +							     &param);	\
> +		}							\
> +	}								\
> +									\
>  	for (;;) {							\
>  		bool expired__ = time_after(jiffies, timeout__);	\
>  		if (COND) {						\
> @@ -70,10 +85,20 @@
>  			cpu_relax();					\
>  		}							\
>  	}								\
> +									\
> +	if ((DEADLINE) && (W) && sched__ == 0) {			\
> +		struct task_struct *t = current;			\
> +		struct sched_param param =				\
> +				{ .sched_priority = 0 };		\
> +									\
> +		WARN_ON(sched_setscheduler_nocheck(t, SCHED_NORMAL,	\
> +						   &param));		\
> +	}								\
> +									\
>  	ret__;								\
>  })
>  
> -#define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000)
> +#define wait_for(COND, MS)	  	_wait_for((COND), (MS) * 1000, 1000, 0)
>  
>  /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
>  #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
> @@ -123,7 +148,7 @@
>  	int ret__; \
>  	BUILD_BUG_ON(!__builtin_constant_p(US)); \
>  	if ((US) > 10) \
> -		ret__ = _wait_for((COND), (US), 10); \
> +		ret__ = _wait_for((COND), (US), 10, 0); \
>  	else \
>  		ret__ = _wait_for_atomic((COND), (US), 0); \
>  	ret__; \
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 169c4908ad5b..215b1a9fd214 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7939,7 +7939,7 @@ int skl_pcode_request(struct drm_i915_private *dev_priv, u32 mbox, u32 request,
>  		ret = 0;
>  		goto out;
>  	}
> -	ret = _wait_for(COND, timeout_base_ms * 1000, 10);
> +	ret = _wait_for(COND, timeout_base_ms * 1000, 10, 1);
>  	if (!ret)
>  		goto out;
>  
> @@ -7957,6 +7957,8 @@ int skl_pcode_request(struct drm_i915_private *dev_priv, u32 mbox, u32 request,
>  	preempt_disable();
>  	ret = wait_for_atomic(COND, 10);
>  	preempt_enable();
> +	if (ret == 0)
> +		DRM_DEBUG_KMS("PCODE success after timeout\n");
>  
>  out:
>  	return ret ? ret : status;
> -- 
> 2.9.3
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx