On Tue, Dec 06, 2022 at 01:57:39PM +0530, Aravind Iddamsetty wrote:
> From: Pallavi Mishra <pallavi.mishra@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
>
> Signed-off-by: Pallavi Mishra <pallavi.mishra@xxxxxxxxx>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx>
> ---
> 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
