Re: [PATCH 2/4] drm/xe: store bind time pat index to xe_bo

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jan 19, 2024 at 03:45:22PM +0000, Matthew Auld wrote:
> On 18/01/2024 15:27, Juha-Pekka Heikkila wrote:
> > Store pat index from xe_vma to xe_bo
> > 
> > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > ---
> >   drivers/gpu/drm/xe/xe_pt.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index de1030a47588..4b76db698878 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -1252,6 +1252,10 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
> >   		return ERR_PTR(-ENOMEM);
> >   	}
> >   
> > +	if (xe_vma_bo(vma)) {
> > +		xe_vma_bo(vma)->pat_index = vma->pat_index;
> 
> Multiple mappings will trash this I think. Is that OK for your usecase? 
> It can be useful to map the same resource as compressed and uncompressed 
> to facilitate in-place decompression/compression.

I thought the pat_index is set for the entire bo? The
cache_level->pat_index stuff doesn't really work otherwise
I don't think (assuming it works at all).

So dunno why this is doing anything using vmas. I think
what we probably need is to check/set the bo pat_index
at fb create time, and lock it into place (if there's
some mechanism by which a random userspace client could
change it after the fact, and thus screw up everything).

> 
> Also would be good to be clear about what happens if the KMD doesn't do 
> anything to prevent compression with non-tile4? Is it just a bit of 
> display corruption or something much worse that we need to prevent? Is 
> this just a best effort check to help userspace? Otherwise it is hard to 
> evaluate how solid we need to be here in our checking to prevent this 
> scenario. For example how is binding vs display races handled? What 
> happens if the bind appears after the display check?
> 
> > +	}
> > +
> >   	fence = xe_migrate_update_pgtables(tile->migrate,
> >   					   vm, xe_vma_bo(vma), q,
> >   					   entries, num_entries,

-- 
Ville Syrjälä
Intel