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

[Matt Roper]

On Tue, Dec 06, 2022 at 01:57:39PM +0530, Aravind Iddamsetty wrote:
> From: Pallavi Mishra <pallavi.mishra at 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?

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 at intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> 
> Signed-off-by: Pallavi Mishra <pallavi.mishra at intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty at 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
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation