intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Auld <matthew.auld@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	dri-devel@lists.freedesktop.org,
	"Kenneth Graunke" <kenneth@whitecape.org>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>
Subject: [Intel-gfx] [PATCH 1/4] drm/i915/uapi: reject caching ioctls for discrete
Date: Thu, 15 Jul 2021 11:15:33 +0100	[thread overview]
Message-ID: <20210715101536.2606307-2-matthew.auld@intel.com> (raw)
In-Reply-To: <20210715101536.2606307-1-matthew.auld@intel.com>

It's a noop on DG1, and in the future when need to support other devices
which let us control the coherency, then it should be an immutable
creation time property for the BO. This will likely be controlled
through a new gem_create_ext extension.

v2: add some kernel doc for the discrete changes, and document the
    implicit rules

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c |  6 +++++
 include/uapi/drm/i915_drm.h                | 29 ++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 7d1400b13429..43004bef55cb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -268,6 +268,9 @@ 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)))
+		return -ENODEV;
+
 	rcu_read_lock();
 	obj = i915_gem_object_lookup_rcu(file, args->handle);
 	if (!obj) {
@@ -303,6 +306,9 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
 	enum i915_cache_level level;
 	int ret = 0;
 
+	if (IS_DGFX(i915))
+		return -ENODEV;
+
 	switch (args->caching) {
 	case I915_CACHING_NONE:
 		level = I915_CACHE_NONE;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e54f9efaead0..868c2ee7be60 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1395,6 +1395,35 @@ struct drm_i915_gem_busy {
  * ppGTT support, or if the object is used for scanout). Note that this might
  * require unbinding the object from the GTT first, if its current caching value
  * doesn't match.
+ *
+ * Note that this all changes on discrete platforms, starting from DG1, the
+ * set/get caching is no longer supported, and is now rejected.  Instead the CPU
+ * caching attributes(WB vs WC) will become an immutable creation time property
+ * for the object, along with the GTT caching level. For now we don't expose any
+ * new uAPI for this, instead on DG1 this is all implicit, although this largely
+ * shouldn't matter since DG1 is coherent by default(without any way of
+ * controlling it).
+ *
+ * Implicit caching rules, starting from DG1:
+ *
+ *     - If any of the object placements (see &drm_i915_gem_create_ext_memory_regions)
+ *       contain I915_MEMORY_CLASS_DEVICE then the object will be allocated and
+ *       mapped as write-combined only.
+ *
+ *     - Everything else is always allocated and mapped as write-back, with the
+ *       guarantee that everything is also coherent with the GPU.
+ *
+ * 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.
+ *
+ * Side note: Part of the reason for this is that changing the at-allocation-time CPU
+ * caching attributes for the pages might be required(and is expensive) if we
+ * need to then CPU map the pages later with different caching attributes. This
+ * inconsistent caching behaviour, while supported on x86, is not universally
+ * supported on other architectures. So for simplicity we opt for setting
+ * everything at creation time, whilst also making it immutable, on discrete
+ * platforms.
  */
 struct drm_i915_gem_caching {
 	/**
-- 
2.26.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-07-15 10:16 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 10:15 [Intel-gfx] [PATCH 0/4] Some DG1 uAPI cleanup Matthew Auld
2021-07-15 10:15 ` Matthew Auld [this message]
2021-07-15 10:15 ` [Intel-gfx] [PATCH 2/4] drm/i915/uapi: convert drm_i915_gem_userptr to kernel doc Matthew Auld
2021-07-15 12:09   ` Maarten Lankhorst
2021-07-15 10:15 ` [Intel-gfx] [PATCH 3/4] drm/i915/userptr: Probe existence of backing struct pages upon creation Matthew Auld
2021-07-15 10:33   ` Tvrtko Ursulin
2021-07-15 11:07     ` Daniel Vetter
2021-07-15 11:27       ` Tvrtko Ursulin
2021-07-15 18:21         ` Kenneth Graunke
2021-07-15 19:18           ` Jason Ekstrand
2021-07-16  8:20           ` Tvrtko Ursulin
2021-07-16 14:50           ` Daniel Vetter
2021-07-15 11:09     ` Matthew Auld
2021-07-15 11:33       ` Tvrtko Ursulin
2021-07-16 14:36   ` Tvrtko Ursulin
2021-07-21 19:18   ` Kenneth Graunke
2021-07-21 19:21   ` Kenneth Graunke
2021-07-21 20:27   ` Jason Ekstrand
2021-07-22  8:43     ` Matthew Auld
2021-07-22 13:29       ` Jason Ekstrand
2021-07-15 10:15 ` [Intel-gfx] [PATCH 4/4] drm/i915/uapi: reject set_domain for discrete Matthew Auld
2021-07-16 14:52   ` Tvrtko Ursulin
2021-07-16 15:23     ` Jason Ekstrand
2021-07-19  9:00       ` Tvrtko Ursulin
2021-07-19  9:09       ` Matthew Auld
2021-07-19 19:57         ` Jason Ekstrand
2021-07-16 19:33 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Some DG1 uAPI cleanup Patchwork
2021-07-17  1:36 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210715101536.2606307-2-matthew.auld@intel.com \
    --to=matthew.auld@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=kenneth@whitecape.org \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).