Hi Tvrtko,
On Tue, Nov 6, 2018 at 3:14 PM Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
On 06/11/2018 04:13, Ankit Navik wrote:
> From: Praveen Diwakar <praveen.diwakar@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Yogesh Marathe <yogesh.marathe@xxxxxxxxx>
> Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar@xxxxxxxxx>
> Signed-off-by: Kedar J Karanje <kedar.j.karanje@xxxxxxxxx>
> Signed-off-by: Ankit Navik <ankit.p.navik@xxxxxxxxx>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> ---
> 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
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
