[Intel-xe] [PATCH DONTMERGE] drm/xe: uapi review submission

[Matt Roper]

On Thu, Jul 06, 2023 at 08:14:52AM +0200, Thomas Hellström wrote:
> Hi, Matt,
> 
> Thanks for taking a look into this. Some comments below. I think we need to
> set up tasks for the various review concerns and assign people to have them
> resolved:
...
> > > +struct drm_xe_vm_bind {
> > Should there be a field in here to allow userspace to specify which PAT
> > index corresponds to the behavior (caching, coherency, CLOS, etc.) they
> > want on this binding?  The PAT should be a characteristic of the bind
> > rather than of the underlying object, right?
> Yes, given certain immutable restrictions given at buffer-object create
> time; For example the CPU caching mode will restrict the coherency mode used
> in PAT, and CPU caching mode needs to be specified at bo create time. The
> current suggestion is therefore that coherency mode also needs to be
> specified at bo create time, a slightly stricter limitation on the available
> PAT indices at VM_BIND time. Whether we want to include PAT in the main
> struct or as an extension hasn't been decided yet.

I'm not sure I fully understand this one; I thought CPU caching to be
mostly orthogonal to GPU coherency mode.  Do you mean we'd step in and
prevent combinations like CPU:WB + GPU:non-coherent since then the GPU
might not see updates done by the CPU that are still stuck in the CPU
cache and/or the CPU might read stale data from its cache after the GPU
has updated the buffer?  It seems like there are so many ways userspace
could already shoot itself in the foot if it picked nonsense settings
that I'm surprised we'd try to stop it here.  Even for the example given
there might be some cases where it still winds up being fine (e.g., the
CPU side always does clflushes at the necessary times and the GPU never
does any writes that would need to invalidate the CPU cache).

Am I overlooking something, or are the combinations that will get
restricted based on something else entirely?


Matt

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation