[PATCH v2 3/3] drm/xe: Add CLOS specific initializations

[Mishra, Pallavi]

> -----Original Message-----
> From: Welty, Brian <brian.welty at intel.com>
> Sent: Friday, January 12, 2024 11:46 AM
> To: Mishra, Pallavi <pallavi.mishra at intel.com>; intel-xe at lists.freedesktop.org
> Cc: Naklicki, Mateusz <mateusz.naklicki at intel.com>; Vishwanathapura,
> Niranjana <niranjana.vishwanathapura at intel.com>
> Subject: Re: [PATCH v2 3/3] drm/xe: Add CLOS specific initializations
> 
> 
> 
> On 1/9/2024 3:57 PM, Pallavi Mishra wrote:
> > Handle CLOS specific initializations and PAT CLOS compatibility check.
> >
> > Signed-off-by: Pallavi Mishra <pallavi.mishra at intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_device.c | 15 ++++++++++++++
> >   drivers/gpu/drm/xe/xe_pat.c    | 36
> ++++++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/xe/xe_pat.h    |  9 +++++++++
> >   drivers/gpu/drm/xe/xe_vm.c     | 12 ++++++++++++
> >   4 files changed, 72 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > b/drivers/gpu/drm/xe/xe_device.c index 004e65544e8d..4e3d4f2c0f9b
> > 100644
> [snip]
> > diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
> > index 1ff6bc79e7d4..ccdefba0f2f3 100644
> > --- a/drivers/gpu/drm/xe/xe_pat.c
> > +++ b/drivers/gpu/drm/xe/xe_pat.c
> > @@ -45,6 +45,14 @@
> >   #define XELP_PAT_WC
> 	REG_FIELD_PREP(XELP_MEM_TYPE_MASK, 1)
> >   #define XELP_PAT_UC
> 	REG_FIELD_PREP(XELP_MEM_TYPE_MASK, 0)
> >
> > +#define XE2_PAT_CLOS1                          ((1 << 20)|(1 << 21)|(1 << 22)|(1 <<
> 23))
> > +#define XE2_PAT_CLOS2                          ((1 << 24)|(1 << 25)|(1 << 26)|(1 <<
> 27))
> > +#define XE2_PAT_CLOS3                          ((1 << 28)|(1 << 29)|(1 << 30)|(1 <<
> 31))
> > +
> > +#define XEPVC_PAT_CLOS1                                ((1 << 4)|(1 << 5))
> > +#define XEPVC_PAT_CLOS2                                ((1 << 6)|(1 << 7))
> > +
> > +
> >   static const char *XELP_MEM_TYPE_STR_MAP[] = { "UC", "WC", "WT",
> > "WB" };
> >
> >   struct xe_pat_ops {
> > @@ -148,6 +156,34 @@ u16 xe_pat_index_get_coh_mode(struct xe_device
> *xe, u16 pat_index)
> >   	return xe->pat.table[pat_index].coh_mode;
> >   }
> >
> > +int xe_pat_index_clos_check(struct xe_device *xe, u16 pat_index, u16
> > +clos_index) {
> > +	WARN_ON(pat_index >= xe->pat.n_entries);
> > +
> > +	int err = 0;
> > +
> > +	switch (clos_index) {
> > +	case 1:
> > +		if (!(((1 << pat_index) & XE2_PAT_CLOS1)
> > +			|| (1 << pat_index & XEPVC_PAT_CLOS1)))
> > +			err = -EINVAL;
> > +		break;
> > +	case 2:
> > +		if (!(((1 << pat_index) & XE2_PAT_CLOS2)
> > +			|| (1 << pat_index & XEPVC_PAT_CLOS2)))
> > +			err = -EINVAL;
> > +		break;
> > +	case 3:
> > +		if (!((1 << pat_index) & XE2_PAT_CLOS3))
> > +			err = -EINVAL;
> > +		break;
> > +	default:
> > +		drm_err(&xe->drm, "Unsupported CLOS value\n");
> > +			err = -EINVAL;
> > +	}
> > +	return err;
> > +}
> > +
> >   static void program_pat(struct xe_gt *gt, const struct xe_pat_table_entry
> table[],
> >   			int n_entries)
> >   {
> > diff --git a/drivers/gpu/drm/xe/xe_pat.h b/drivers/gpu/drm/xe/xe_pat.h
> > index fa0dfbe525cd..afac06bc425f 100644
> > --- a/drivers/gpu/drm/xe/xe_pat.h
> > +++ b/drivers/gpu/drm/xe/xe_pat.h
> > @@ -58,4 +58,13 @@ void xe_pat_dump(struct xe_gt *gt, struct
> drm_printer *p);
> >    */
> >   u16 xe_pat_index_get_coh_mode(struct xe_device *xe, u16 pat_index);
> >
> > +/**
> > + * xe_pat_index_clos_check - check whether clos has been reserved for
> > + * chosen pat_index.
> > + * @xe: xe device
> > + * @pat_index: The pat_index to query
> > + * @clos_index: clos index to compare  */ int
> > +xe_pat_index_clos_check(struct xe_device *xe, u16 pat_index, u16
> > +clos_index);
> > +
> >   #endif
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 1ca917b8315c..8e8c0302c8a0 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -2800,6 +2800,18 @@ static int vm_bind_ioctl_check_args(struct
> xe_device *xe,
> >   			err = -EINVAL;
> >   			goto free_bind_ops;
> >   		}
> > +
> > +		/* check whether Clos has been reserved for chosen pat */
> > +		if ((GRAPHICS_VER(xe) >= 20 && (pat_index > 19)) || (xe-
> >info.platform == XE_PVC && (pat_index > 3))) {
> > +			mutex_lock(&xe->cache_resv.clos_mutex);
> > +			err = xe_pat_index_clos_check(xe, pat_index,
> > +xe->cache_resv.clos_index);
> 
> Thanks for pointing out that xe->cache_resv.clos_index is used here.....
> But this doesn't seem correct.  That is simply the most recently reserved
> clos_index.  It might have been reserved by some other user.

I see. You are right. I seem to have considered a single user scenario.
> 
> I believe you need to test using the set of CLOS indexes reserved to
> this user.   So you need something like:
> 
>      struct xe_file *xef = to_xe_file(file);
> 
>      for each clos_index in xef->clos_resv.clos_mask
>          xe_pat_index_clos_check(xe, pat_index, clos_index);
>      confirm pat_index is valid for one of the reserved clos indexes
> 
> Right?

Agree. 

Thanks,
Pallavi
> 
> 
> > +			if (err) {
> > +				mutex_unlock(&xe->cache_resv.clos_mutex);
> > +				goto free_bind_ops;
> > +			}
> > +			mutex_unlock(&xe->cache_resv.clos_mutex);
> > +		}
> > +
> >   	}
> >
> >   	return 0;