[Intel-gfx] [PATCH] drm/i915/tgl: WaDisableGPGPUMidThreadPreemption

[Jason Ekstrand]

On Wed, Mar 4, 2020 at 11:01 AM Rafael Antognolli
<rafael.antognolli at intel.com> wrote:
>
> On Wed, Mar 04, 2020 at 04:24:13PM +0000, Tvrtko Ursulin wrote:
> >
> > On 04/03/2020 16:02, Rafael Antognolli wrote:
> > > On Wed, Mar 04, 2020 at 03:31:44PM +0000, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > > >
> > > > Enable FtrPerCtxtPreemptionGranularityControl bit and select thread-
> > > > group as the default preemption level.
> > > >
> > > > v2:
> > > >   * Remove register whitelisting (Rafael, Tony).
> > > >
> > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > > > Cc: Michał Winiarski <michal.winiarski at intel.com>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > > Cc: piotr.zdunowski at intel.com
> > > > Cc: michal.mrozek at intel.com
> > > > Cc: Tony Ye <tony.ye at intel.com>
> > > > Cc: Rafael Antognolli <rafael.antognolli at intel.com>
> > >
> > > Thanks for CC'ing me. I also saw a reply from Jason yesterday, but I
> > > don't see it in the list now (though my mail client a mess lately).
> >
> > I saw nothing from Jason, but there was an email from you asking about
> > interface descriptors and whitelisting which is why I copied you.
> >
> > > But he asked whether it's possible for Media and OpenCL drivers to
> > > also disable mid-thread preemption through the
> > > INTERFACE_DESCRIPTOR_DATA, instead of from the kernel side, so we could
> > > try to experiment with it in the future.
> > >
> > > Also, do you have an idea of how broken it is? Or is it just not tested
> > > because no driver is currently implementing it? And do you know if the
> > > windows 3D drivers implement it at all? I see code in the driver that
> > > seems to me that it's only disabled in certain cases...
> > >
> > > To summarize, I think we should either:
> > >     1) Disable mid-thread preemption from the kernel and not whitelist
> > >     the register (just like you do in this patch); or
> > >     2) Not do anything at all from the kernel, and let userspace disable
> > >     it if needed.
> > >
> > > I think 2) is better, if it's not an issue to the other userspace
> > > drivers (OpenCL and Media).
> >
> > I know it is somewhat broken like in
> > https://gitlab.freedesktop.org/drm/intel/issues/1293.
> >
> > And I know OpenCL and Media would prefer i915 to handle it, but that's
> > always the case. :) OpenCL and Media folks are on the thread so can comment
> > if they are okay with handling this themselves.
> >
> > Indeed a blanket ban in i915 means no one can try it out later without
> > further kernel changes.
>
> Well, based on your comment from the previous patch:
>
> "General thinking is, since MTP is considered not validated / broken /
> dangerous, i915 should default it off. But yes, whitelisting or not on
> top is open."
>
> Maybe we should simply ban it and be done. So this patch is:
>
> Acked-by: Rafael Antognolli <rafael.antognolli at intel.com>

Agreed.  If we think that it's broken or is likely to take additional
kernel work to enable it properly, we shouldn't allow userspace to
turn it on until we know the kernel is in good shape.  Just ban it
outright and we can figure out white-listing later if and when we get
it properly working.

Acked-by: Jason Ekstrand <jason at jlekstrand.net>