Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/mtl/UAPI: Disable GET/SET_CACHING IOCTL for MTL+

["Iddamsetty, Aravind" <aravind.iddamsetty@intel.com>]

On 07-12-2022 05:19, Matt Roper wrote:
> On Tue, Dec 06, 2022 at 01:57:39PM +0530, Aravind Iddamsetty wrote:
>> From: Pallavi Mishra <pallavi.mishra@intel.com>
>>
>> It's a noop on all new platforms starting from MTL.
> 
> To me, saying "it's a noop" implies that the ioctl will succeed and
> silently do nothing, which isn't the case in this patch.  We're
> explicitly rejecting attempts by userspace to use these ioctls.
> 
>> Refer: (e7737b67ab46) drm/i915/uapi: reject caching ioctls for discrete
> 
> While killing set_caching/get_caching is the way we want to go, I think
> we need a lot more explanation of how cache behavior in general is
> supposed to work now.  I believe the plan is that userspace will supply
> the specific PAT index that corresponds to the behavior they want via a
> vm_bind extension?  I'm not familiar with the details of how that will
> work...does that mean that the caching behavior will also be tied to the
> specific mapping of an object in the GTT rather than being tied to the
> object itself?  I.e., you can map the same object twice with two
> different caching behaviors?
Like i mentioned in other email part of this series. The current plan
atleast is to set the caching for an object during creation time
depending on the platform so for MTL it would be UNCACHED. The PAT index
setting via VM_BIND is yet not planned.

Thanks,
Aravind.
> 
> Is there a uapi RFC document available yet that describes the high-level
> view of how all this stuff fits together now?  If so, a reference to
> that would be good to include.
> 
> 
> Matt
> 
>>
>> v2:
>> 1. block get caching ioctl
>> 2. return ENODEV similar to DGFX
>> 3. update the doc in i915_drm.h
>>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>
>> Signed-off-by: Pallavi Mishra <pallavi.mishra@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 4 ++--
>>  include/uapi/drm/i915_drm.h                | 3 +++
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> index d44a152ce680..cf817ee0aa01 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> @@ -291,7 +291,7 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
>>  	struct drm_i915_gem_object *obj;
>>  	int err = 0;
>>  
>> -	if (IS_DGFX(to_i915(dev)))
>> +	if (IS_DGFX(to_i915(dev)) || GRAPHICS_VER_FULL(to_i915(dev)) >= IP_VER(12, 70))
>>  		return -ENODEV;
>>  
>>  	rcu_read_lock();
>> @@ -329,7 +329,7 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
>>  	enum i915_cache_level level;
>>  	int ret = 0;
>>  
>> -	if (IS_DGFX(i915))
>> +	if (IS_DGFX(i915) || GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>>  		return -ENODEV;
>>  
>>  	switch (args->caching) {
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 8df261c5ab9b..3467fd879427 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -1626,6 +1626,9 @@ struct drm_i915_gem_busy {
>>   *     - Everything else is always allocated and mapped as write-back, with the
>>   *       guarantee that everything is also coherent with the GPU.
>>   *
>> + * Starting from MTL even on integrated platforms set/get caching is no longer
>> + * supported and object will be mapped as write-combined only.
>> + *
>>   * Note that this is likely to change in the future again, where we might need
>>   * more flexibility on future devices, so making this all explicit as part of a
>>   * new &drm_i915_gem_create_ext extension is probable.
>> -- 
>> 2.25.1
>>
>