| Message ID | 20200508092933.738-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] drm/i915: Ignore submit-fences on the same timeline | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > As a means for a small code consolidation, but primarily to start > thinking more carefully about internal-vs-external linkage, pull the > pair of i915_sw_fence_await_dma_fence() calls into a common routine. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_request.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index be2ce9065a29..94189c7d43cd 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) > return 0; > } > > +static int > +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) > +{ > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, > + fence->context ? I915_FENCE_TIMEOUT : 0, > + I915_FENCE_GFP); > +} > + > int > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > { > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > if (dma_fence_is_i915(fence)) > ret = i915_request_await_request(rq, to_request(fence)); > else > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > - fence->context ? I915_FENCE_TIMEOUT : 0, > - I915_FENCE_GFP); > + ret = i915_request_await_external(rq, fence); For us (rq, rq), for external (rq, fence). It looks neat for a reader. But then, how can external fence have a context? -Mika > if (ret < 0) > return ret; > > @@ -1255,9 +1261,7 @@ i915_request_await_execution(struct i915_request *rq, > to_request(fence), > hook); > else > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > - I915_FENCE_TIMEOUT, > - GFP_KERNEL); > + ret = i915_request_await_external(rq, fence); > if (ret < 0) > return ret; > } while (--nchild); > -- > 2.20.1
Quoting Mika Kuoppala (2020-05-08 11:19:25) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > As a means for a small code consolidation, but primarily to start > > thinking more carefully about internal-vs-external linkage, pull the > > pair of i915_sw_fence_await_dma_fence() calls into a common routine. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_request.c | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > > index be2ce9065a29..94189c7d43cd 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) > > return 0; > > } > > > > +static int > > +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) > > +{ > > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, > > + fence->context ? I915_FENCE_TIMEOUT : 0, > > + I915_FENCE_GFP); > > +} > > + > > int > > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > > { > > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > > if (dma_fence_is_i915(fence)) > > ret = i915_request_await_request(rq, to_request(fence)); > > else > > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > > - fence->context ? I915_FENCE_TIMEOUT : 0, > > - I915_FENCE_GFP); > > + ret = i915_request_await_external(rq, fence); > > For us (rq, rq), for external (rq, fence). > > It looks neat for a reader. But then, how can external fence have > a context? The 'context' here is the dma_fence timeline context. All dma_fences have a context:seqno tuple which denotes their timeline and position along the timeline. -Chris
Quoting Mika Kuoppala (2020-05-08 11:19:25) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > As a means for a small code consolidation, but primarily to start > > thinking more carefully about internal-vs-external linkage, pull the > > pair of i915_sw_fence_await_dma_fence() calls into a common routine. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_request.c | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > > index be2ce9065a29..94189c7d43cd 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) > > return 0; > > } > > > > +static int > > +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) > > +{ > > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, > > + fence->context ? I915_FENCE_TIMEOUT : 0, > > + I915_FENCE_GFP); > > +} > > + > > int > > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > > { > > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) > > if (dma_fence_is_i915(fence)) > > ret = i915_request_await_request(rq, to_request(fence)); > > else > > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, > > - fence->context ? I915_FENCE_TIMEOUT : 0, > > - I915_FENCE_GFP); > > + ret = i915_request_await_external(rq, fence); > > For us (rq, rq), for external (rq, fence). > > It looks neat for a reader. But then, how can external fence have > a context? How about s/fence/dma/? -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2020-05-08 11:19:25) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > As a means for a small code consolidation, but primarily to start >> > thinking more carefully about internal-vs-external linkage, pull the >> > pair of i915_sw_fence_await_dma_fence() calls into a common routine. >> > >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > --- >> > drivers/gpu/drm/i915/i915_request.c | 16 ++++++++++------ >> > 1 file changed, 10 insertions(+), 6 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c >> > index be2ce9065a29..94189c7d43cd 100644 >> > --- a/drivers/gpu/drm/i915/i915_request.c >> > +++ b/drivers/gpu/drm/i915/i915_request.c >> > @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) >> > return 0; >> > } >> > >> > +static int >> > +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) >> > +{ >> > + return i915_sw_fence_await_dma_fence(&rq->submit, fence, >> > + fence->context ? I915_FENCE_TIMEOUT : 0, >> > + I915_FENCE_GFP); >> > +} >> > + >> > int >> > i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) >> > { >> > @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) >> > if (dma_fence_is_i915(fence)) >> > ret = i915_request_await_request(rq, to_request(fence)); >> > else >> > - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, >> > - fence->context ? I915_FENCE_TIMEOUT : 0, >> > - I915_FENCE_GFP); >> > + ret = i915_request_await_external(rq, fence); >> >> For us (rq, rq), for external (rq, fence). >> >> It looks neat for a reader. But then, how can external fence have >> a context? > > How about s/fence/dma/? It is fine like it is. I was just so confused that we piggyback our context along the fence. But yeah this is the fence->context and zero is a special. (thanks for explaining in irc) Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > -Chris
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index be2ce9065a29..94189c7d43cd 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1067,6 +1067,14 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) return 0; } +static int +i915_request_await_external(struct i915_request *rq, struct dma_fence *fence) +{ + return i915_sw_fence_await_dma_fence(&rq->submit, fence, + fence->context ? I915_FENCE_TIMEOUT : 0, + I915_FENCE_GFP); +} + int i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) { @@ -1114,9 +1122,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence) if (dma_fence_is_i915(fence)) ret = i915_request_await_request(rq, to_request(fence)); else - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - fence->context ? I915_FENCE_TIMEOUT : 0, - I915_FENCE_GFP); + ret = i915_request_await_external(rq, fence); if (ret < 0) return ret; @@ -1255,9 +1261,7 @@ i915_request_await_execution(struct i915_request *rq, to_request(fence), hook); else - ret = i915_sw_fence_await_dma_fence(&rq->submit, fence, - I915_FENCE_TIMEOUT, - GFP_KERNEL); + ret = i915_request_await_external(rq, fence); if (ret < 0) return ret; } while (--nchild);
As a means for a small code consolidation, but primarily to start thinking more carefully about internal-vs-external linkage, pull the pair of i915_sw_fence_await_dma_fence() calls into a common routine. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_request.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)