diff mbox

[RFCv2,2/3] drm/i915: Introduce private PAT management

Message ID 1503452653-30903-3-git-send-email-zhi.a.wang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wang, Zhi A Aug. 23, 2017, 1:44 a.m. UTC
The private PAT management is to support both static and dynamic PPAT
entry manipulation. During the initialization, the PPAT indexes with
specific PPAT values could be reserved and set by intel_ppat_reserve.
The unused PPAT entries can be allocated/freed later at runtime. Two
APIs are introduced for dynamically managing PPAT entries: intel_ppat_get
and intel_ppat_set.

intel_ppat_get will search for an existing PPAT entry which perfectly
matches the required PPAT value. If not, it will try to allocate or
return a partially matched PPAT entry if there is any available PPAT
indexes or not.

intel_ppat_put will put back the PPAT entry which comes from
intel_ppat_get. If it's dynamically allocated, the reference count will
be decreased. If the reference count turns into zero, the PPAT index is
freed again.

Besides, another two callbacks are introduced to support the private PAT
management framework. One is ppat->update(), which writes the PPAT
configurations in ppat->entries into HW. Another one is ppat->match, which
will return a score to show how two PPAT values match with each other.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |   2 +
 drivers/gpu/drm/i915/i915_gem_gtt.c | 129 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.h |  36 ++++++++++
 3 files changed, 167 insertions(+)

Comments

Chris Wilson Aug. 22, 2017, 6:01 p.m. UTC | #1
Quoting Zhi Wang (2017-08-23 02:44:12)
> The private PAT management is to support both static and dynamic PPAT
> entry manipulation. During the initialization, the PPAT indexes with
> specific PPAT values could be reserved and set by intel_ppat_reserve.
> The unused PPAT entries can be allocated/freed later at runtime. Two
> APIs are introduced for dynamically managing PPAT entries: intel_ppat_get
> and intel_ppat_set.

What's the use case for reserved? Once assigned, a new allocation
doesn't evict, so reservation is just another form of assignment.
-Chris
Chris Wilson Aug. 22, 2017, 6:07 p.m. UTC | #2
Quoting Chris Wilson (2017-08-22 19:01:11)
> Quoting Zhi Wang (2017-08-23 02:44:12)
> > The private PAT management is to support both static and dynamic PPAT
> > entry manipulation. During the initialization, the PPAT indexes with
> > specific PPAT values could be reserved and set by intel_ppat_reserve.
> > The unused PPAT entries can be allocated/freed later at runtime. Two
> > APIs are introduced for dynamically managing PPAT entries: intel_ppat_get
> > and intel_ppat_set.
> 
> What's the use case for reserved? Once assigned, a new allocation
> doesn't evict, so reservation is just another form of assignment.

Or rather, I can see the differentiation you want for init, but I can't
see why you want to export it (since it ignores the ppat controller) or
why you need to have a reserved bit, since you can just elevate the
refcount.
-Chris
Wang, Zhi A Aug. 22, 2017, 6:25 p.m. UTC | #3
The "reserved" PPAT indexes is for keeping current i915 logics unchanged since I don't want to cause regression. I can remove "reserved" PPAT indexes actually.

-----Original Message-----
From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of Chris Wilson

Sent: Tuesday, August 22, 2017 9:01 PM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
Cc: joonas.lahtinen@linux.intel.com; Wang, Zhi A <zhi.a.wang@intel.com>; zhenyuw@linux.intel.com
Subject: Re: [RFCv2 2/3] drm/i915: Introduce private PAT management

Quoting Zhi Wang (2017-08-23 02:44:12)
> The private PAT management is to support both static and dynamic PPAT 

> entry manipulation. During the initialization, the PPAT indexes with 

> specific PPAT values could be reserved and set by intel_ppat_reserve.

> The unused PPAT entries can be allocated/freed later at runtime. Two 

> APIs are introduced for dynamically managing PPAT entries: 

> intel_ppat_get and intel_ppat_set.


What's the use case for reserved? Once assigned, a new allocation doesn't evict, so reservation is just another form of assignment.
-Chris
_______________________________________________
intel-gvt-dev mailing list
intel-gvt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
Wang, Zhi A Aug. 22, 2017, 6:30 p.m. UTC | #4
How about I don't export "reserve" APIs in the next version? The "reserve" stuff is totally for init and keeping current logic unchanged. I'm scared of regression. :(

-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk] 

Sent: Tuesday, August 22, 2017 9:08 PM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
Cc: joonas.lahtinen@linux.intel.com; zhenyuw@linux.intel.com; Wang, Zhi A <zhi.a.wang@intel.com>
Subject: Re: [RFCv2 2/3] drm/i915: Introduce private PAT management

Quoting Chris Wilson (2017-08-22 19:01:11)
> Quoting Zhi Wang (2017-08-23 02:44:12)

> > The private PAT management is to support both static and dynamic 

> > PPAT entry manipulation. During the initialization, the PPAT indexes 

> > with specific PPAT values could be reserved and set by intel_ppat_reserve.

> > The unused PPAT entries can be allocated/freed later at runtime. Two 

> > APIs are introduced for dynamically managing PPAT entries: 

> > intel_ppat_get and intel_ppat_set.

> 

> What's the use case for reserved? Once assigned, a new allocation 

> doesn't evict, so reservation is just another form of assignment.


Or rather, I can see the differentiation you want for init, but I can't see why you want to export it (since it ignores the ppat controller) or why you need to have a reserved bit, since you can just elevate the refcount.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 60267e3..97b46f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2307,6 +2307,8 @@  struct drm_i915_private {
 	DECLARE_HASHTABLE(mm_structs, 7);
 	struct mutex mm_lock;
 
+	struct intel_ppat ppat;
+
 	/* Kernel Modesetting */
 
 	struct intel_crtc *plane_to_crtc_mapping[I915_MAX_PIPES];
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09d1d48..2a521b6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2742,6 +2742,121 @@  static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
 	return 0;
 }
 
+/**
+ * intel_ppat_reserve - reserve a PPAT index and set the PPAT value
+ * @ppat: i915 PPAT instance
+ * @index: the PPAT index needs to be reserved
+ * @value: the PPAT value needs to be set
+ *
+ * A PPAT entry sits on the reserved PPAT index will not be modified by PPAT
+ * management.
+ */
+void intel_ppat_reserve(struct intel_ppat *ppat, unsigned int index, u8 value)
+{
+	GEM_BUG_ON(index >= ppat->max_entries);
+	GEM_BUG_ON(test_bit(index, ppat->reserved));
+
+	ppat->entries[index].value = value;
+	set_bit(index, ppat->reserved);
+	set_bit(index, ppat->used);
+}
+
+/**
+ * intel_ppat_get - dynamically get a usable PPAT entry
+ * @dev_priv: i915 device instance
+ * @value: the PPAT value required by the caller
+ *
+ * The function tries to search if there is an existing PPAT entry which
+ * matches with the required value. If perfectly matched, the existing PPAT
+ * entry will be used. If only partially matched, it will try to check if
+ * there is any available PPAT index. If yes, it will allocate a new PPAT
+ * index for the required entry and update the HW. If not, the partially
+ * matched entry will be used.
+ */
+struct intel_ppat_entry *intel_ppat_get(struct drm_i915_private *dev_priv,
+					u8 value)
+{
+	struct intel_ppat *ppat = &dev_priv->ppat;
+	struct intel_ppat_entry *entry;
+	int i, used;
+	unsigned int score, best_score;
+
+	score = best_score = 0;
+	used = 0;
+
+	/* First, find a suitable value from available entries */
+	for_each_clear_bit(i, ppat->used, ppat->max_entries) {
+		score = ppat->match(ppat->entries[i].value, value);
+		/* Perfect match */
+		if (score == ~0) {
+			entry = &ppat->entries[i];
+			kref_get(&entry->ref_count);
+			return entry;
+		}
+
+		if (score > best_score) {
+			entry = &ppat->entries[i];
+			best_score = score;
+		}
+		used++;
+	}
+
+	/* No matched entry and we can't allocate a new entry. */
+	if (!best_score && used == ppat->max_entries) {
+		DRM_ERROR("Fail to get PPAT entry\n");
+		return ERR_PTR(-ENOSPC);
+	}
+
+	/*
+	 * Found a matched entry which is not perfect,
+	 * and we can't allocate a new entry.
+	 */
+	if (best_score && used == ppat->max_entries) {
+		kref_get(&entry->ref_count);
+		return entry;
+	}
+
+	/* Allocate a new entry */
+	i = find_first_zero_bit(ppat->used, ppat->max_entries);
+	set_bit(i, ppat->used);
+
+	entry = &ppat->entries[i];
+	entry->value = value;
+	kref_init(&entry->ref_count);
+
+	ppat->update(dev_priv);
+	return entry;
+}
+
+static void put_ppat(struct kref *kref)
+{
+	struct intel_ppat_entry *entry =
+		container_of(kref, struct intel_ppat_entry, ref_count);
+	struct intel_ppat *ppat = entry->ppat;
+	struct drm_i915_private *dev_priv = ppat->dev_priv;
+	int index = entry - ppat->entries;
+
+	if (test_bit(index, ppat->reserved))
+		return;
+
+	entry->value = ppat->dummy_value;
+	clear_bit(index, ppat->used);
+	ppat->update(dev_priv);
+}
+
+/**
+ * intel_ppat_put - put back the PPAT entry got from intel_ppat_get()
+ * @entry: an intel PPAT entry
+ *
+ * Put back the PPAT entry got from intel_ppat_get(). If the PPAT index of the
+ * entry is dynamically allocated, its reference count will be decreased. Once
+ * the reference count becomes into zero, the PPAT index becomes free again.
+ */
+void intel_ppat_put(struct intel_ppat_entry *entry)
+{
+	kref_put(&entry->ref_count, put_ppat);
+}
+
 static void cnl_setup_private_ppat(struct drm_i915_private *dev_priv)
 {
 	/* XXX: spec is unclear if this is still needed for CNL+ */
@@ -2843,12 +2958,26 @@  static void gen6_gmch_remove(struct i915_address_space *vm)
 
 static void setup_private_pat(struct drm_i915_private *dev_priv)
 {
+	struct intel_ppat *ppat = &dev_priv->ppat;
+	int i;
+
+	ppat->dev_priv = dev_priv;
+
 	if (INTEL_GEN(dev_priv) >= 10)
 		cnl_setup_private_ppat(dev_priv);
 	else if (IS_CHERRYVIEW(dev_priv) || IS_GEN9_LP(dev_priv))
 		chv_setup_private_ppat(dev_priv);
 	else
 		bdw_setup_private_ppat(dev_priv);
+
+	GEM_BUG_ON(ppat->max_entries > INTEL_MAX_PPAT_ENTRIES);
+
+	for_each_clear_bit(i, ppat->reserved, ppat->max_entries) {
+		ppat->entries[i].value = ppat->dummy_value;
+		ppat->entries[i].ppat = ppat;
+	}
+
+	ppat->update(dev_priv);
 }
 
 static int gen8_gmch_probe(struct i915_ggtt *ggtt)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b4e3aa7..eeb1307 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -536,6 +536,42 @@  i915_vm_to_ggtt(struct i915_address_space *vm)
 	return container_of(vm, struct i915_ggtt, base);
 }
 
+#define INTEL_MAX_PPAT_ENTRIES 8
+
+struct intel_ppat;
+
+struct intel_ppat_entry {
+	struct intel_ppat *ppat;
+	struct kref ref_count;
+	u8 value;
+};
+
+struct intel_ppat {
+	struct intel_ppat_entry entries[INTEL_MAX_PPAT_ENTRIES];
+	DECLARE_BITMAP(reserved, INTEL_MAX_PPAT_ENTRIES);
+	DECLARE_BITMAP(used, INTEL_MAX_PPAT_ENTRIES);
+
+	unsigned int max_entries;
+
+	u8 dummy_value;
+	/*
+	 * Return a score to show how two PPAT values match,
+	 * a ~0 indicates a perfect match
+	 */
+	unsigned int (*match)(u8 src, u8 dst);
+	/*
+	 * Write the PPAT configuration into HW.
+	 */
+	void (*update)(struct drm_i915_private *dev_priv);
+
+	struct drm_i915_private *dev_priv;
+};
+
+void intel_ppat_reserve(struct intel_ppat *ppat, unsigned int index, u8 value);
+struct intel_ppat_entry *intel_ppat_get(struct drm_i915_private *i915,
+					u8 value);
+void intel_ppat_put(struct intel_ppat_entry *entry);
+
 int i915_gem_init_aliasing_ppgtt(struct drm_i915_private *i915);
 void i915_gem_fini_aliasing_ppgtt(struct drm_i915_private *i915);