[Intel-gfx] [PATCH 1/4] drm/i915/guc: Remove obsolete comments and remove unused variable

[Chris Wilson]

Quoting Chris Wilson (2017-09-12 15:21:06)
> Quoting Chris Wilson (2017-09-12 15:17:56)
> > Quoting Michał Winiarski (2017-09-12 13:47:23)
> > > Originally removed in:
> > > c1adab970348 ("drm/i915/guc: Remove failed doorbell stat from debugfs")
> > > f1448a62a103 ("drm/i915/guc: Remove last submission result from debugfs")
> > > 
> > > Were accidentaly restored in:
> > > 925344ccc91d ("BackMerge tag 'v4.12-rc5' into drm-next")
> > > 
> > > We can also remove unused variable and replace it with a WARN.
> > > 
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > > Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> > > Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
> > >  drivers/gpu/drm/i915/intel_uc.h            | 4 ----
> > >  2 files changed, 1 insertion(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > index 48a1e9349a2c..8a550785b257 100644
> > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > @@ -602,7 +602,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
> > >         struct intel_guc *guc = &rq->i915->guc;
> > >         struct i915_guc_client *client = guc->execbuf_client;
> > >         unsigned long flags;
> > > -       int b_ret;
> > >  
> > >         /* WA to flush out the pending GMADR writes to ring buffer. */
> > >         if (i915_vma_is_map_and_fenceable(rq->ring->vma))
> > > @@ -611,7 +610,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
> > >         spin_lock_irqsave(&client->wq_lock, flags);
> > >  
> > >         guc_wq_item_append(client, rq);
> > > -       b_ret = guc_ring_doorbell(client);
> > > +       WARN_ON(guc_ring_doorbell(client));
> > 
> > Hmm:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5658/fi-skl-6700k/igt@gem_exec_parallel@basic.html
> > 
> > Ok, time to dig.
> 
> Actually that's a compliment to your lockless implementation. We have
> multiple threads competing to ring the doorbell. So one more patch
> required...

I suggest another cmpxchg loop:

static void guc_ring_doorbell(struct i915_guc_client *client)
{
        struct guc_doorbell_info *db = __get_doorbell(client);
        u32 cookie;

        do {
                cookie = READ_ONCE(db->cookie);
        } while (cmpxchg(&db->cookie, cookie, cookie + 1) != cookie);
        GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
}

with associated cleanup.
-Chris