| Message ID | 1541477601-10883-2-git-send-email-ankit.p.navik@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Dynamic EU configuration of Slice/Subslice/EU. | expand |
On 06/11/2018 04:13, Ankit Navik wrote: > From: Praveen Diwakar <praveen.diwakar@intel.com> > > This patch gives us the active pending request count which is yet > to be submitted to the GPU > > Signed-off-by: Praveen Diwakar <praveen.diwakar@intel.com> > Signed-off-by: Yogesh Marathe <yogesh.marathe@intel.com> > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar@intel.com> > Signed-off-by: Kedar J Karanje <kedar.j.karanje@intel.com> > Signed-off-by: Ankit Navik <ankit.p.navik@intel.com> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > drivers/gpu/drm/i915/i915_gem_context.c | 1 + > drivers/gpu/drm/i915/i915_gem_context.h | 6 ++++++ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++++ > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++ > 6 files changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index f8cfd16..d37c46e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, > mutex_init(&dev_priv->av_mutex); > mutex_init(&dev_priv->wm.wm_mutex); > mutex_init(&dev_priv->pps_mutex); > + mutex_init(&dev_priv->pred_mutex); > > i915_memcpy_init_early(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4aca534..137ec33 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1609,6 +1609,11 @@ struct drm_i915_private { > * controller on different i2c buses. */ > struct mutex gmbus_mutex; > > + /** pred_mutex protects against councurrent usage of pending > + * request counter for multiple contexts > + */ > + struct mutex pred_mutex; > + > /** > * Base address of the gmbus and gpio block. > */ > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index b10770c..0bcbe32 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -387,6 +387,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, > } > > trace_i915_context_create(ctx); > + atomic_set(&ctx->req_cnt, 0); > > return ctx; > } > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h > index b116e49..04e3ff7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.h > +++ b/drivers/gpu/drm/i915/i915_gem_context.h > @@ -194,6 +194,12 @@ struct i915_gem_context { > * context close. > */ > struct list_head handles_list; > + > + /** req_cnt: tracks the pending commands, based on which we decide to > + * go for low/medium/high load configuration of the GPU, this is > + * controlled via a mutex > + */ > + atomic_t req_cnt; > }; > > static inline bool i915_gem_context_is_closed(const struct i915_gem_context *ctx) > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 3f0c612..8afa2a5 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -2178,6 +2178,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > struct drm_syncobj **fences) > { > struct i915_execbuffer eb; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct dma_fence *in_fence = NULL; > struct sync_file *out_fence = NULL; > int out_fence_fd = -1; > @@ -2390,6 +2391,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, > */ > eb.request->batch = eb.batch; > > + mutex_lock(&dev_priv->pred_mutex); > + atomic_inc(&eb.ctx->req_cnt); Point of going to atomic_t was to remove need for the mutex. > + mutex_unlock(&dev_priv->pred_mutex); > + > trace_i915_request_queue(eb.request, eb.batch_flags); > err = eb_submit(&eb); > err_request: > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 1744792..bcbb66b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -728,6 +728,12 @@ static void execlists_dequeue(struct intel_engine_cs *engine) > trace_i915_request_in(rq, port_index(port, execlists)); > last = rq; > submit = true; > + > + mutex_lock(&rq->i915->pred_mutex); > + if (atomic_read(&rq->gem_context->req_cnt) > 0) > + atomic_dec(&rq->gem_context->req_cnt); Hitting underflow is a hint accounting does not work as expected. I really think you need to fix it by gathering some ideas from the patches I've pointed at in the previous round. And there is also GuC to think about. Regards, Tvrtko > + > + mutex_unlock(&rq->i915->pred_mutex); > } > > rb_erase_cached(&p->node, &execlists->queue); >
Hi Tvrtko, > On Tue, Nov 6, 2018 at 3:14 PM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > > On 06/11/2018 04:13, Ankit Navik wrote: > > From: Praveen Diwakar <praveen.diwakar@intel.com> > > > > This patch gives us the active pending request count which is yet to > > be submitted to the GPU > > > > Signed-off-by: Praveen Diwakar <praveen.diwakar@intel.com> > > Signed-off-by: Yogesh Marathe <yogesh.marathe@intel.com> > > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar@intel.com> > > Signed-off-by: Kedar J Karanje <kedar.j.karanje@intel.com> > > Signed-off-by: Ankit Navik <ankit.p.navik@intel.com> > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 1 + > > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > > drivers/gpu/drm/i915/i915_gem_context.c | 1 + > > drivers/gpu/drm/i915/i915_gem_context.h | 6 ++++++ > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++++ > > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++ > > 6 files changed, 24 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c index f8cfd16..d37c46e 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct > drm_i915_private *dev_priv, > > mutex_init(&dev_priv->av_mutex); > > mutex_init(&dev_priv->wm.wm_mutex); > > mutex_init(&dev_priv->pps_mutex); > > + mutex_init(&dev_priv->pred_mutex); > > > > i915_memcpy_init_early(dev_priv); > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index 4aca534..137ec33 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1609,6 +1609,11 @@ struct drm_i915_private { > > * controller on different i2c buses. */ > > struct mutex gmbus_mutex; > > > > + /** pred_mutex protects against councurrent usage of pending > > + * request counter for multiple contexts > > + */ > > + struct mutex pred_mutex; > > + > > /** > > * Base address of the gmbus and gpio block. > > */ > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > > b/drivers/gpu/drm/i915/i915_gem_context.c > > index b10770c..0bcbe32 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -387,6 +387,7 @@ i915_gem_create_context(struct drm_i915_private > *dev_priv, > > } > > > > trace_i915_context_create(ctx); > > + atomic_set(&ctx->req_cnt, 0); > > > > return ctx; > > } > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h > > b/drivers/gpu/drm/i915/i915_gem_context.h > > index b116e49..04e3ff7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.h > > +++ b/drivers/gpu/drm/i915/i915_gem_context.h > > @@ -194,6 +194,12 @@ struct i915_gem_context { > > * context close. > > */ > > struct list_head handles_list; > > + > > + /** req_cnt: tracks the pending commands, based on which we decide > to > > + * go for low/medium/high load configuration of the GPU, this is > > + * controlled via a mutex > > + */ > > + atomic_t req_cnt; > > }; > > > > static inline bool i915_gem_context_is_closed(const struct > > i915_gem_context *ctx) diff --git > > a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index 3f0c612..8afa2a5 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -2178,6 +2178,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > struct drm_syncobj **fences) > > { > > struct i915_execbuffer eb; > > + struct drm_i915_private *dev_priv = to_i915(dev); > > struct dma_fence *in_fence = NULL; > > struct sync_file *out_fence = NULL; > > int out_fence_fd = -1; > > @@ -2390,6 +2391,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > */ > > eb.request->batch = eb.batch; > > > > + mutex_lock(&dev_priv->pred_mutex); > > + atomic_inc(&eb.ctx->req_cnt); > > Point of going to atomic_t was to remove need for the mutex. > > > + mutex_unlock(&dev_priv->pred_mutex); > > + > > trace_i915_request_queue(eb.request, eb.batch_flags); > > err = eb_submit(&eb); > > err_request: > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index 1744792..bcbb66b 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -728,6 +728,12 @@ static void execlists_dequeue(struct intel_engine_cs > *engine) > > trace_i915_request_in(rq, port_index(port, execlists)); > > last = rq; > > submit = true; > > + > > + mutex_lock(&rq->i915->pred_mutex); > > + if (atomic_read(&rq->gem_context->req_cnt) > 0) > > + atomic_dec(&rq->gem_context->req_cnt); > > Hitting underflow is a hint accounting does not work as expected. I really think > you need to fix it by gathering some ideas from the patches I've pointed at in the > previous round. Changes submitted in v3 patch. https://patchwork.freedesktop.org/patch/267386/ Regards, Ankit > > And there is also GuC to think about. > > Regards, > > Tvrtko > > > + > > + mutex_unlock(&rq->i915->pred_mutex); > > } > > > > rb_erase_cached(&p->node, &execlists->queue); > >
Hi Tvrtko, On Tue, Nov 6, 2018 at 3:14 PM Tvrtko Ursulin < tvrtko.ursulin@linux.intel.com> wrote: > > On 06/11/2018 04:13, Ankit Navik wrote: > > From: Praveen Diwakar <praveen.diwakar@intel.com> > > > > This patch gives us the active pending request count which is yet > > to be submitted to the GPU > > > > Signed-off-by: Praveen Diwakar <praveen.diwakar@intel.com> > > Signed-off-by: Yogesh Marathe <yogesh.marathe@intel.com> > > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar@intel.com> > > Signed-off-by: Kedar J Karanje <kedar.j.karanje@intel.com> > > Signed-off-by: Ankit Navik <ankit.p.navik@intel.com> > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 1 + > > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > > drivers/gpu/drm/i915/i915_gem_context.c | 1 + > > drivers/gpu/drm/i915/i915_gem_context.h | 6 ++++++ > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++++ > > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++ > > 6 files changed, 24 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > > index f8cfd16..d37c46e 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct > drm_i915_private *dev_priv, > > mutex_init(&dev_priv->av_mutex); > > mutex_init(&dev_priv->wm.wm_mutex); > > mutex_init(&dev_priv->pps_mutex); > > + mutex_init(&dev_priv->pred_mutex); > > > > i915_memcpy_init_early(dev_priv); > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > > index 4aca534..137ec33 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1609,6 +1609,11 @@ struct drm_i915_private { > > * controller on different i2c buses. */ > > struct mutex gmbus_mutex; > > > > + /** pred_mutex protects against councurrent usage of pending > > + * request counter for multiple contexts > > + */ > > + struct mutex pred_mutex; > > + > > /** > > * Base address of the gmbus and gpio block. > > */ > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > > index b10770c..0bcbe32 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -387,6 +387,7 @@ i915_gem_create_context(struct drm_i915_private > *dev_priv, > > } > > > > trace_i915_context_create(ctx); > > + atomic_set(&ctx->req_cnt, 0); > > > > return ctx; > > } > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h > b/drivers/gpu/drm/i915/i915_gem_context.h > > index b116e49..04e3ff7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.h > > +++ b/drivers/gpu/drm/i915/i915_gem_context.h > > @@ -194,6 +194,12 @@ struct i915_gem_context { > > * context close. > > */ > > struct list_head handles_list; > > + > > + /** req_cnt: tracks the pending commands, based on which we decide > to > > + * go for low/medium/high load configuration of the GPU, this is > > + * controlled via a mutex > > + */ > > + atomic_t req_cnt; > > }; > > > > static inline bool i915_gem_context_is_closed(const struct > i915_gem_context *ctx) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index 3f0c612..8afa2a5 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -2178,6 +2178,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > struct drm_syncobj **fences) > > { > > struct i915_execbuffer eb; > > + struct drm_i915_private *dev_priv = to_i915(dev); > > struct dma_fence *in_fence = NULL; > > struct sync_file *out_fence = NULL; > > int out_fence_fd = -1; > > @@ -2390,6 +2391,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > */ > > eb.request->batch = eb.batch; > > > > + mutex_lock(&dev_priv->pred_mutex); > > + atomic_inc(&eb.ctx->req_cnt); > > Point of going to atomic_t was to remove need for the mutex. > > > + mutex_unlock(&dev_priv->pred_mutex); > > + > > trace_i915_request_queue(eb.request, eb.batch_flags); > > err = eb_submit(&eb); > > err_request: > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > > index 1744792..bcbb66b 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -728,6 +728,12 @@ static void execlists_dequeue(struct > intel_engine_cs *engine) > > trace_i915_request_in(rq, port_index(port, > execlists)); > > last = rq; > > submit = true; > > + > > + mutex_lock(&rq->i915->pred_mutex); > > + if (atomic_read(&rq->gem_context->req_cnt) > 0) > > + atomic_dec(&rq->gem_context->req_cnt); > > Hitting underflow is a hint accounting does not work as expected. I > really think you need to fix it by gathering some ideas from the patches > I've pointed at in the previous round. > I have submitted the patch v4. I have tried with point which you have suggested, but didnt see much power benefit with that. Regards, Ankit > > And there is also GuC to think about. > > Regards, > > Tvrtko > > > + > > + mutex_unlock(&rq->i915->pred_mutex); > > } > > > > rb_erase_cached(&p->node, &execlists->queue); > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > <div dir="ltr"><div dir="ltr">Hi Tvrtko, </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 6, 2018 at 3:14 PM Tvrtko Ursulin <<a href="mailto:tvrtko.ursulin@linux.intel.com">tvrtko.ursulin@linux.intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br> On 06/11/2018 04:13, Ankit Navik wrote:<br> > From: Praveen Diwakar <<a href="mailto:praveen.diwakar@intel.com" target="_blank">praveen.diwakar@intel.com</a>><br> > <br> > This patch gives us the active pending request count which is yet<br> > to be submitted to the GPU<br> > <br> > Signed-off-by: Praveen Diwakar <<a href="mailto:praveen.diwakar@intel.com" target="_blank">praveen.diwakar@intel.com</a>><br> > Signed-off-by: Yogesh Marathe <<a href="mailto:yogesh.marathe@intel.com" target="_blank">yogesh.marathe@intel.com</a>><br> > Signed-off-by: Aravindan Muthukumar <<a href="mailto:aravindan.muthukumar@intel.com" target="_blank">aravindan.muthukumar@intel.com</a>><br> > Signed-off-by: Kedar J Karanje <<a href="mailto:kedar.j.karanje@intel.com" target="_blank">kedar.j.karanje@intel.com</a>><br> > Signed-off-by: Ankit Navik <<a href="mailto:ankit.p.navik@intel.com" target="_blank">ankit.p.navik@intel.com</a>><br> > Suggested-by: Tvrtko Ursulin <<a href="mailto:tvrtko.ursulin@linux.intel.com" target="_blank">tvrtko.ursulin@linux.intel.com</a>><br> > ---<br> > drivers/gpu/drm/i915/i915_drv.c | 1 +<br> > drivers/gpu/drm/i915/i915_drv.h | 5 +++++<br> > drivers/gpu/drm/i915/i915_gem_context.c | 1 +<br> > drivers/gpu/drm/i915/i915_gem_context.h | 6 ++++++<br> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++++<br> > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++++<br> > 6 files changed, 24 insertions(+)<br> > <br> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c<br> > index f8cfd16..d37c46e 100644<br> > --- a/drivers/gpu/drm/i915/i915_drv.c<br> > +++ b/drivers/gpu/drm/i915/i915_drv.c<br> > @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,<br> > mutex_init(&dev_priv->av_mutex);<br> > mutex_init(&dev_priv->wm.wm_mutex);<br> > mutex_init(&dev_priv->pps_mutex);<br> > + mutex_init(&dev_priv->pred_mutex);<br> > <br> > i915_memcpy_init_early(dev_priv);<br> > <br> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h<br> > index 4aca534..137ec33 100644<br> > --- a/drivers/gpu/drm/i915/i915_drv.h<br> > +++ b/drivers/gpu/drm/i915/i915_drv.h<br> > @@ -1609,6 +1609,11 @@ struct drm_i915_private {<br> > * controller on different i2c buses. */<br> > struct mutex gmbus_mutex;<br> > <br> > + /** pred_mutex protects against councurrent usage of pending<br> > + * request counter for multiple contexts<br> > + */<br> > + struct mutex pred_mutex;<br> > +<br> > /**<br> > * Base address of the gmbus and gpio block.<br> > */<br> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c<br> > index b10770c..0bcbe32 100644<br> > --- a/drivers/gpu/drm/i915/i915_gem_context.c<br> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c<br> > @@ -387,6 +387,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,<br> > }<br> > <br> > trace_i915_context_create(ctx);<br> > + atomic_set(&ctx->req_cnt, 0);<br> > <br> > return ctx;<br> > }<br> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h<br> > index b116e49..04e3ff7 100644<br> > --- a/drivers/gpu/drm/i915/i915_gem_context.h<br> > +++ b/drivers/gpu/drm/i915/i915_gem_context.h<br> > @@ -194,6 +194,12 @@ struct i915_gem_context {<br> > * context close.<br> > */<br> > struct list_head handles_list;<br> > +<br> > + /** req_cnt: tracks the pending commands, based on which we decide to<br> > + * go for low/medium/high load configuration of the GPU, this is<br> > + * controlled via a mutex<br> > + */<br> > + atomic_t req_cnt;<br> > };<br> > <br> > static inline bool i915_gem_context_is_closed(const struct i915_gem_context *ctx)<br> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c<br> > index 3f0c612..8afa2a5 100644<br> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c<br> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c<br> > @@ -2178,6 +2178,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,<br> > struct drm_syncobj **fences)<br> > {<br> > struct i915_execbuffer eb;<br> > + struct drm_i915_private *dev_priv = to_i915(dev);<br> > struct dma_fence *in_fence = NULL;<br> > struct sync_file *out_fence = NULL;<br> > int out_fence_fd = -1;<br> > @@ -2390,6 +2391,10 @@ i915_gem_do_execbuffer(struct drm_device *dev,<br> > */<br> > eb.request->batch = eb.batch;<br> > <br> > + mutex_lock(&dev_priv->pred_mutex);<br> > + atomic_inc(&eb.ctx->req_cnt);<br> <br> Point of going to atomic_t was to remove need for the mutex.<br> <br> > + mutex_unlock(&dev_priv->pred_mutex);<br> > +<br> > trace_i915_request_queue(eb.request, eb.batch_flags);<br> > err = eb_submit(&eb);<br> > err_request:<br> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c<br> > index 1744792..bcbb66b 100644<br> > --- a/drivers/gpu/drm/i915/intel_lrc.c<br> > +++ b/drivers/gpu/drm/i915/intel_lrc.c<br> > @@ -728,6 +728,12 @@ static void execlists_dequeue(struct intel_engine_cs *engine)<br> > trace_i915_request_in(rq, port_index(port, execlists));<br> > last = rq;<br> > submit = true;<br> > +<br> > + mutex_lock(&rq->i915->pred_mutex);<br> > + if (atomic_read(&rq->gem_context->req_cnt) > 0)<br> > + atomic_dec(&rq->gem_context->req_cnt);<br> <br> Hitting underflow is a hint accounting does not work as expected. I <br> really think you need to fix it by gathering some ideas from the patches <br> I've pointed at in the previous round.<br></blockquote><div><br></div><div>I have submitted the patch v4. <br>I have tried with point which you have suggested, but didnt see much</div><div>power benefit with that. </div><div><br></div><div>Regards, Ankit</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> And there is also GuC to think about.<br> <br> Regards,<br> <br> Tvrtko<br> <br> > +<br> > + mutex_unlock(&rq->i915->pred_mutex);<br> > }<br> > <br> > rb_erase_cached(&p->node, &execlists->queue);<br> > <br> _______________________________________________<br> Intel-gfx mailing list<br> <a href="mailto:Intel-gfx@lists.freedesktop.org" target="_blank">Intel-gfx@lists.freedesktop.org</a><br> <a href="https://lists.freedesktop.org/mailman/listinfo/intel-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br> </blockquote></div></div>
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f8cfd16..d37c46e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -903,6 +903,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, mutex_init(&dev_priv->av_mutex); mutex_init(&dev_priv->wm.wm_mutex); mutex_init(&dev_priv->pps_mutex); + mutex_init(&dev_priv->pred_mutex); i915_memcpy_init_early(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4aca534..137ec33 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1609,6 +1609,11 @@ struct drm_i915_private { * controller on different i2c buses. */ struct mutex gmbus_mutex; + /** pred_mutex protects against councurrent usage of pending + * request counter for multiple contexts + */ + struct mutex pred_mutex; + /** * Base address of the gmbus and gpio block. */ diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index b10770c..0bcbe32 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -387,6 +387,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, } trace_i915_context_create(ctx); + atomic_set(&ctx->req_cnt, 0); return ctx; } diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index b116e49..04e3ff7 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -194,6 +194,12 @@ struct i915_gem_context { * context close. */ struct list_head handles_list; + + /** req_cnt: tracks the pending commands, based on which we decide to + * go for low/medium/high load configuration of the GPU, this is + * controlled via a mutex + */ + atomic_t req_cnt; }; static inline bool i915_gem_context_is_closed(const struct i915_gem_context *ctx) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 3f0c612..8afa2a5 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2178,6 +2178,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, struct drm_syncobj **fences) { struct i915_execbuffer eb; + struct drm_i915_private *dev_priv = to_i915(dev); struct dma_fence *in_fence = NULL; struct sync_file *out_fence = NULL; int out_fence_fd = -1; @@ -2390,6 +2391,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, */ eb.request->batch = eb.batch; + mutex_lock(&dev_priv->pred_mutex); + atomic_inc(&eb.ctx->req_cnt); + mutex_unlock(&dev_priv->pred_mutex); + trace_i915_request_queue(eb.request, eb.batch_flags); err = eb_submit(&eb); err_request: diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 1744792..bcbb66b 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -728,6 +728,12 @@ static void execlists_dequeue(struct intel_engine_cs *engine) trace_i915_request_in(rq, port_index(port, execlists)); last = rq; submit = true; + + mutex_lock(&rq->i915->pred_mutex); + if (atomic_read(&rq->gem_context->req_cnt) > 0) + atomic_dec(&rq->gem_context->req_cnt); + + mutex_unlock(&rq->i915->pred_mutex); } rb_erase_cached(&p->node, &execlists->queue);