[Intel-gfx] [PATCH] drm/i915: Only allocate preempt context when required

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-01-30 00:24:04)
> <snip>
> 
> > @@ -979,17 +979,19 @@ static int guc_clients_create(struct intel_guc *guc)
> >       }
> >       guc->execbuf_client = client;
> >   
> > -     client = guc_client_alloc(dev_priv,
> > -                               INTEL_INFO(dev_priv)->ring_mask,
> > -                               GUC_CLIENT_PRIORITY_KMD_HIGH,
> > -                               dev_priv->preempt_context);
> > -     if (IS_ERR(client)) {
> > -             DRM_ERROR("Failed to create GuC client for preemption!\n");
> > -             guc_client_free(guc->execbuf_client);
> > -             guc->execbuf_client = NULL;
> > -             return PTR_ERR(client);
> > +     if (dev_priv->preempt_context) {
> > +             client = guc_client_alloc(dev_priv,
> > +                                       INTEL_INFO(dev_priv)->ring_mask,
> > +                                       GUC_CLIENT_PRIORITY_KMD_HIGH,
> > +                                       dev_priv->preempt_context);
> > +             if (IS_ERR(client)) {
> > +                     DRM_ERROR("Failed to create GuC client for preemption!\n");
> > +                     guc_client_free(guc->execbuf_client);
> > +                     guc->execbuf_client = NULL;
> > +                     return PTR_ERR(client);
> > +             }
> > +             guc->preempt_client = client;
> >       }
> 
> I was having another look at this patch while rebasing my patches on top 
> of it and I noticed that although guc->preempt_client is now not always 
> created other code still assumes that it is always there. All platforms 
> with GuC also have preemption, so i think the only case in which we can 
> have issues is when we fail to create the preempt context and keep going 
> with the driver loading assuming preemption disabled.

At the time we landed guc preemption we intended for it to be defensive
and allow us to disable it if need by throwing a single switch
(has_logical_ring_preemption = false).

When creating this patch, I thought that was still true; we could use
the propagation of guc->preempt_client = NULL /
dev_priv->preempt_context = NULL to simply not trigger preemption.
-Chris