linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org,
	Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
	Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Subject: Re: [RFC 6/8] drm/i915: Make some recently added vfuncs use full scheduling attribute
Date: Wed, 6 Oct 2021 10:12:29 -0700	[thread overview]
Message-ID: <20211006171228.GA7906@jons-linux-dev-box> (raw)
In-Reply-To: <20211004143650.699120-7-tvrtko.ursulin@linux.intel.com>

On Mon, Oct 04, 2021 at 03:36:48PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Code added in 71ed60112d5d ("drm/i915: Add kick_backend function to
> i915_sched_engine") and ee242ca704d3 ("drm/i915/guc: Implement GuC
> priority management") introduced some scheduling related vfuncs which
> take integer request priority as argument.
> 
> Make them instead take struct i915_sched_attr, which is the type
> encapsulating this information, so it probably aligns with the design
> better. It definitely enables extending the set of scheduling attributes.
> 

Understand the motivation here but the i915_scheduler is going to
disapear when we move to the DRM scheduler or at least its functionality
of priority inheritance will be pushed into the DRM scheduler. I'd be
very careful making any changes here as the priority in the DRM
scheduler is defined as single enum:

/* These are often used as an (initial) index
 * to an array, and as such should start at 0.
 */
enum drm_sched_priority {
        DRM_SCHED_PRIORITY_MIN,
        DRM_SCHED_PRIORITY_NORMAL,
        DRM_SCHED_PRIORITY_HIGH,
        DRM_SCHED_PRIORITY_KERNEL,

        DRM_SCHED_PRIORITY_COUNT,
        DRM_SCHED_PRIORITY_UNSET = -2
};

Adding a field to the i915_sched_attr is fairly easy as we already have
a structure but changing the DRM scheduler might be a tougher sell.
Anyway you can make this work without adding the 'nice' field to
i915_sched_attr? Might be worth exploring so when we move to the DRM
scheduler this feature drops in a little cleaner.

Matt

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 +++-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c    | 3 ++-
>  drivers/gpu/drm/i915/i915_scheduler.c                | 4 ++--
>  drivers/gpu/drm/i915/i915_scheduler_types.h          | 4 ++--
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 7147fe80919e..e91d803a6453 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -3216,11 +3216,13 @@ static bool can_preempt(struct intel_engine_cs *engine)
>  	return engine->class != RENDER_CLASS;
>  }
>  
> -static void kick_execlists(const struct i915_request *rq, int prio)
> +static void kick_execlists(const struct i915_request *rq,
> +			   const struct i915_sched_attr *attr)
>  {
>  	struct intel_engine_cs *engine = rq->engine;
>  	struct i915_sched_engine *sched_engine = engine->sched_engine;
>  	const struct i915_request *inflight;
> +	const int prio = attr->priority;
>  
>  	/*
>  	 * We only need to kick the tasklet once for the high priority
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index ba0de35f6323..b5883a4365ca 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -2414,9 +2414,10 @@ static void guc_init_breadcrumbs(struct intel_engine_cs *engine)
>  }
>  
>  static void guc_bump_inflight_request_prio(struct i915_request *rq,
> -					   int prio)
> +					   const struct i915_sched_attr *attr)
>  {
>  	struct intel_context *ce = rq->context;
> +	const int prio = attr->priority;
>  	u8 new_guc_prio = map_i915_prio_to_guc_prio(prio);
>  
>  	/* Short circuit function */
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index 762127dd56c5..534bab99fcdc 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -255,7 +255,7 @@ static void __i915_schedule(struct i915_sched_node *node,
>  
>  		/* Must be called before changing the nodes priority */
>  		if (sched_engine->bump_inflight_request_prio)
> -			sched_engine->bump_inflight_request_prio(from, prio);
> +			sched_engine->bump_inflight_request_prio(from, attr);
>  
>  		WRITE_ONCE(node->attr.priority, prio);
>  
> @@ -280,7 +280,7 @@ static void __i915_schedule(struct i915_sched_node *node,
>  
>  		/* Defer (tasklet) submission until after all of our updates. */
>  		if (sched_engine->kick_backend)
> -			sched_engine->kick_backend(node_to_request(node), prio);
> +			sched_engine->kick_backend(node_to_request(node), attr);
>  	}
>  
>  	spin_unlock(&sched_engine->lock);
> diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
> index b0a1b58c7893..24b9ac1c2ce2 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler_types.h
> +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
> @@ -177,13 +177,13 @@ struct i915_sched_engine {
>  	 * @kick_backend: kick backend after a request's priority has changed
>  	 */
>  	void	(*kick_backend)(const struct i915_request *rq,
> -				int prio);
> +				const struct i915_sched_attr *attr);
>  
>  	/**
>  	 * @bump_inflight_request_prio: update priority of an inflight request
>  	 */
>  	void	(*bump_inflight_request_prio)(struct i915_request *rq,
> -					      int prio);
> +					      const struct i915_sched_attr *attr);
>  
>  	/**
>  	 * @retire_inflight_request_prio: indicate request is retired to
> -- 
> 2.30.2
> 

  reply	other threads:[~2021-10-06 17:21 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04 14:36 [RFC v2 0/8] CPU + GPU synchronised priority scheduling Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 1/8] sched: Add nice value change notifier Tvrtko Ursulin
2021-10-06  4:10   ` Wanghui (John)
2021-10-06  7:58     ` Barry Song
2021-10-06 13:44       ` Tvrtko Ursulin
2021-10-06 20:21         ` Barry Song
2021-10-07  8:50           ` Tvrtko Ursulin
2021-10-07  9:09             ` Tvrtko Ursulin
2021-10-07 10:00               ` Barry Song
2021-10-04 14:36 ` [RFC 2/8] drm/i915: Explicitly track DRM clients Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 3/8] drm/i915: Make GEM contexts " Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 4/8] drm/i915: Track all user contexts per client Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 5/8] drm/i915: Keep track of registered clients indexed by task struct Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 6/8] drm/i915: Make some recently added vfuncs use full scheduling attribute Tvrtko Ursulin
2021-10-06 17:12   ` Matthew Brost [this message]
2021-10-06 19:06     ` Tvrtko Ursulin
2021-10-13 12:01     ` [Intel-gfx] " Daniel Vetter
2021-10-13 15:50       ` Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 7/8] drm/i915: Inherit process nice for context scheduling priority Tvrtko Ursulin
2021-10-06 17:16   ` [Intel-gfx] " Matthew Brost
2021-10-06 17:24   ` Matthew Brost
2021-10-06 18:42     ` Tvrtko Ursulin
2021-10-04 14:36 ` [RFC 8/8] drm/i915: Connect with the process nice change notifier Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211006171228.GA7906@jons-linux-dev-box \
    --to=matthew.brost@intel.com \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tvrtko.ursulin@intel.com \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).