| Message ID | 20190204084116.3013-5-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/12] drm/i915: Allow normal clients to always preempt idle priority clients | expand |
On 04/02/2019 08:41, Chris Wilson wrote: > Limit the NEWCLIENT boost to only give its small priority boost to fresh > clients only that have no dependencies. Needs some actual explaining/documenting on why we would want this. > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_request.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 7968875d0bed..69bc549e95d8 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -979,7 +979,7 @@ void i915_request_add(struct i915_request *request) > * Allow interactive/synchronous clients to jump ahead of > * the bulk clients. (FQ_CODEL) > */ > - if (!prev || i915_request_completed(prev)) > + if (list_empty(&request->sched.signalers_list)) Why are the !prev and completed checks gone? Seems in disagreement with the commit message text - they wouldn't have to be new clients any more, just have no dependencies. > attr.priority |= I915_PRIORITY_NEWCLIENT; > > engine->schedule(request, &attr); > Regards, Tvrtko
Quoting Tvrtko Ursulin (2019-02-04 12:11:41) > > On 04/02/2019 08:41, Chris Wilson wrote: > > Limit the NEWCLIENT boost to only give its small priority boost to fresh > > clients only that have no dependencies. > > Needs some actual explaining/documenting on why we would want this. > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_request.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > > index 7968875d0bed..69bc549e95d8 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -979,7 +979,7 @@ void i915_request_add(struct i915_request *request) > > * Allow interactive/synchronous clients to jump ahead of > > * the bulk clients. (FQ_CODEL) > > */ > > - if (!prev || i915_request_completed(prev)) > > + if (list_empty(&request->sched.signalers_list)) > > Why are the !prev and completed checks gone? Seems in disagreement with > the commit message text - they wouldn't have to be new clients any more, > just have no dependencies. What? signalers_list is a superset of !prev || i915_request_completed. -Chris
Quoting Tvrtko Ursulin (2019-02-04 12:11:41) > > On 04/02/2019 08:41, Chris Wilson wrote: > > Limit the NEWCLIENT boost to only give its small priority boost to fresh > > clients only that have no dependencies. > > Needs some actual explaining/documenting on why we would want this. Exactly the same reasoning as NEWCLIENT. -Chris
On 04/02/2019 12:26, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-02-04 12:11:41) >> >> On 04/02/2019 08:41, Chris Wilson wrote: >>> Limit the NEWCLIENT boost to only give its small priority boost to fresh >>> clients only that have no dependencies. >> >> Needs some actual explaining/documenting on why we would want this. >> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> --- >>> drivers/gpu/drm/i915/i915_request.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c >>> index 7968875d0bed..69bc549e95d8 100644 >>> --- a/drivers/gpu/drm/i915/i915_request.c >>> +++ b/drivers/gpu/drm/i915/i915_request.c >>> @@ -979,7 +979,7 @@ void i915_request_add(struct i915_request *request) >>> * Allow interactive/synchronous clients to jump ahead of >>> * the bulk clients. (FQ_CODEL) >>> */ >>> - if (!prev || i915_request_completed(prev)) >>> + if (list_empty(&request->sched.signalers_list)) >> >> Why are the !prev and completed checks gone? Seems in disagreement with >> the commit message text - they wouldn't have to be new clients any more, >> just have no dependencies. > > What? signalers_list is a superset of !prev || i915_request_completed. Ok my bad - I forgot about adding the previous unfinished request as a dependency. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 7968875d0bed..69bc549e95d8 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -979,7 +979,7 @@ void i915_request_add(struct i915_request *request) * Allow interactive/synchronous clients to jump ahead of * the bulk clients. (FQ_CODEL) */ - if (!prev || i915_request_completed(prev)) + if (list_empty(&request->sched.signalers_list)) attr.priority |= I915_PRIORITY_NEWCLIENT; engine->schedule(request, &attr);
Limit the NEWCLIENT boost to only give its small priority boost to fresh clients only that have no dependencies. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)