[Intel-gfx] [PATCH v2 1/4] drm/i915: Get active pending request for given context

[Ankit Navik]

Hi Tvrtko,

On Tue, Nov 6, 2018 at 3:14 PM Tvrtko Ursulin <
tvrtko.ursulin at linux.intel.com> wrote:

>
> On 06/11/2018 04:13, Ankit Navik wrote:
> > From: Praveen Diwakar <praveen.diwakar at 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 at intel.com>
> > Signed-off-by: Yogesh Marathe <yogesh.marathe at intel.com>
> > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar at intel.com>
> > Signed-off-by: Kedar J Karanje <kedar.j.karanje at intel.com>
> > Signed-off-by: Ankit Navik <ankit.p.navik at intel.com>
> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20190314/b1e88694/attachment.html>