* [PATCH v8 2/5] drm/i915: Introduce private PAT management
2017-09-08 9:05 [PATCH v8 1/5] drm/i915: Factor out setup_private_pat() Zhi Wang
@ 2017-09-08 9:05 ` Zhi Wang
2017-09-08 9:05 ` [PATCH v8 3/5] drm/i915: Remove the "INDEX" suffix from PPAT marcos Zhi Wang
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Zhi Wang @ 2017-09-08 9:05 UTC (permalink / raw)
To: intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
The private PAT management is to support PPAT entry manipulation. Two
APIs are introduced for dynamically managing PPAT entries: intel_ppat_get
and intel_ppat_put.
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_hw(), 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.
v7:
- Keep all the register writes unchanged in this patch. (Joonas)
v6:
- Address all comments from Chris:
http://www.spinics.net/lists/intel-gfx/msg136850.html
- Address all comments from Joonas:
http://www.spinics.net/lists/intel-gfx/msg136845.html
v5:
- Add check and warnnings for those platforms which don't have PPAT.
v3:
- Introduce dirty bitmap for PPAT registers. (Chris)
- Change the name of the pointer "dev_priv" to "i915". (Chris)
- intel_ppat_{get, put} returns/takes a const intel_ppat_entry *. (Chris)
v2:
- API re-design. (Chris)
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> v7
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 | 279 +++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/i915_gem_gtt.h | 36 +++++
3 files changed, 268 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 63ca2ff..3c10b82 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2347,6 +2347,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 2fc0656..8cecd90 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2816,41 +2816,203 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
return 0;
}
-static void cnl_setup_private_ppat(struct drm_i915_private *dev_priv)
+static struct intel_ppat_entry *
+__alloc_ppat_entry(struct intel_ppat *ppat, unsigned int index, u8 value)
{
+ struct intel_ppat_entry *entry = &ppat->entries[index];
+
+ GEM_BUG_ON(index >= ppat->max_entries);
+ GEM_BUG_ON(test_bit(index, ppat->used));
+
+ entry->ppat = ppat;
+ entry->value = value;
+ kref_init(&entry->ref);
+ set_bit(index, ppat->used);
+ set_bit(index, ppat->dirty);
+
+ return entry;
+}
+
+static void __free_ppat_entry(struct intel_ppat_entry *entry)
+{
+ struct intel_ppat *ppat = entry->ppat;
+ unsigned int index = entry - ppat->entries;
+
+ GEM_BUG_ON(index >= ppat->max_entries);
+ GEM_BUG_ON(!test_bit(index, ppat->used));
+
+ entry->value = ppat->clear_value;
+ clear_bit(index, ppat->used);
+ set_bit(index, ppat->dirty);
+}
+
+/**
+ * intel_ppat_get - get a usable PPAT entry
+ * @i915: 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.
+ */
+const struct intel_ppat_entry *
+intel_ppat_get(struct drm_i915_private *i915, u8 value)
+{
+ struct intel_ppat *ppat = &i915->ppat;
+ struct intel_ppat_entry *entry;
+ unsigned int scanned, best_score;
+ int i;
+
+ GEM_BUG_ON(!ppat->max_entries);
+
+ scanned = best_score = 0;
+
+ for_each_set_bit(i, ppat->used, ppat->max_entries) {
+ unsigned int score;
+
+ entry = &ppat->entries[i];
+ score = ppat->match(entry->value, value);
+ if (score > best_score) {
+ if (score == INTEL_PPAT_PERFECT_MATCH) {
+ kref_get(&entry->ref);
+ return entry;
+ }
+ best_score = score;
+ }
+ scanned++;
+ }
+
+ if (scanned == ppat->max_entries) {
+ if (!best_score)
+ return ERR_PTR(-ENOSPC);
+
+ kref_get(&entry->ref);
+ return entry;
+ }
+
+ i = find_first_zero_bit(ppat->used, ppat->max_entries);
+ entry = __alloc_ppat_entry(ppat, i, value);
+ ppat->update_hw(i915);
+ return entry;
+}
+
+static void release_ppat(struct kref *kref)
+{
+ struct intel_ppat_entry *entry =
+ container_of(kref, struct intel_ppat_entry, ref);
+ struct drm_i915_private *i915 = entry->ppat->i915;
+
+ __free_ppat_entry(entry);
+ entry->ppat->update_hw(i915);
+}
+
+/**
+ * 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(const struct intel_ppat_entry *entry)
+{
+ struct intel_ppat *ppat = entry->ppat;
+ unsigned int index = entry - ppat->entries;
+
+ GEM_BUG_ON(!ppat->max_entries);
+
+ kref_put(&ppat->entries[index].ref, release_ppat);
+}
+
+static void cnl_private_pat_update_hw(struct drm_i915_private *dev_priv)
+{
+ struct intel_ppat *ppat = &dev_priv->ppat;
+ int i;
+
+ for_each_set_bit(i, ppat->dirty, ppat->max_entries) {
+ I915_WRITE(GEN10_PAT_INDEX(i), ppat->entries[i].value);
+ clear_bit(i, ppat->dirty);
+ }
+}
+
+static void bdw_private_pat_update_hw(struct drm_i915_private *dev_priv)
+{
+ struct intel_ppat *ppat = &dev_priv->ppat;
+ u64 pat = 0;
+ int i;
+
+ for (i = 0; i < ppat->max_entries; i++)
+ pat |= GEN8_PPAT(i, ppat->entries[i].value);
+
+ bitmap_clear(ppat->dirty, 0, ppat->max_entries);
+
+ I915_WRITE(GEN8_PRIVATE_PAT_LO, lower_32_bits(pat));
+ I915_WRITE(GEN8_PRIVATE_PAT_HI, upper_32_bits(pat));
+}
+
+static unsigned int bdw_private_pat_match(u8 src, u8 dst)
+{
+ unsigned int score = 0;
+
+ /* Cache attribute has to be matched. */
+ if (GEN8_PPAT_GET_CA(src) != GEN8_PPAT_GET_CA(dst))
+ return 0;
+
+ if (GEN8_PPAT_GET_TC(src) == GEN8_PPAT_GET_TC(dst))
+ score += 2;
+
+ if (GEN8_PPAT_GET_AGE(src) == GEN8_PPAT_GET_AGE(dst))
+ score += 1;
+
+ if (score == 3)
+ return INTEL_PPAT_PERFECT_MATCH;
+
+ return score;
+}
+
+static unsigned int chv_private_pat_match(u8 src, u8 dst)
+{
+ return (CHV_PPAT_GET_SNOOP(src) == CHV_PPAT_GET_SNOOP(dst)) ?
+ INTEL_PPAT_PERFECT_MATCH : 0;
+}
+
+static void cnl_setup_private_ppat(struct intel_ppat *ppat)
+{
+ ppat->max_entries = 8;
+ ppat->update_hw = cnl_private_pat_update_hw;
+ ppat->match = bdw_private_pat_match;
+ ppat->clear_value = GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3);
+
/* XXX: spec is unclear if this is still needed for CNL+ */
- if (!USES_PPGTT(dev_priv)) {
- I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_UC);
+ if (!USES_PPGTT(ppat->i915)) {
+ __alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC);
return;
}
- I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_WB | GEN8_PPAT_LLC);
- I915_WRITE(GEN10_PAT_INDEX(1), GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);
- I915_WRITE(GEN10_PAT_INDEX(2), GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
- I915_WRITE(GEN10_PAT_INDEX(3), GEN8_PPAT_UC);
- I915_WRITE(GEN10_PAT_INDEX(4), GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
- I915_WRITE(GEN10_PAT_INDEX(5), GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
- I915_WRITE(GEN10_PAT_INDEX(6), GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2));
- I915_WRITE(GEN10_PAT_INDEX(7), GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
+ __alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);
+ __alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);
+ __alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
+ __alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);
+ __alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
+ __alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
+ __alloc_ppat_entry(ppat, 6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2));
+ __alloc_ppat_entry(ppat, 7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
}
/* The GGTT and PPGTT need a private PPAT setup in order to handle cacheability
* bits. When using advanced contexts each context stores its own PAT, but
* writing this data shouldn't be harmful even in those cases. */
-static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv)
+static void bdw_setup_private_ppat(struct intel_ppat *ppat)
{
- u64 pat;
+ ppat->max_entries = 8;
+ ppat->update_hw = bdw_private_pat_update_hw;
+ ppat->match = bdw_private_pat_match;
+ ppat->clear_value = GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3);
- pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC) | /* for normal objects, no eLLC */
- GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for something pointing to ptes? */
- GEN8_PPAT(2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC) | /* for scanout with eLLC */
- GEN8_PPAT(3, GEN8_PPAT_UC) | /* Uncached objects, mostly for scanout */
- GEN8_PPAT(4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)) |
- GEN8_PPAT(5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1)) |
- GEN8_PPAT(6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2)) |
- GEN8_PPAT(7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
-
- if (!USES_PPGTT(dev_priv))
+ if (!USES_PPGTT(ppat->i915)) {
/* Spec: "For GGTT, there is NO pat_sel[2:0] from the entry,
* so RTL will always use the value corresponding to
* pat_sel = 000".
@@ -2864,17 +3026,26 @@ static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv)
* So we can still hold onto all our assumptions wrt cpu
* clflushing on LLC machines.
*/
- pat = GEN8_PPAT(0, GEN8_PPAT_UC);
+ __alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC);
+ return;
+ }
- /* XXX: spec defines this as 2 distinct registers. It's unclear if a 64b
- * write would work. */
- I915_WRITE(GEN8_PRIVATE_PAT_LO, pat);
- I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32);
+ __alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* for normal objects, no eLLC */
+ __alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC); /* for something pointing to ptes? */
+ __alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for scanout with eLLC */
+ __alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* Uncached objects, mostly for scanout */
+ __alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
+ __alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
+ __alloc_ppat_entry(ppat, 6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2));
+ __alloc_ppat_entry(ppat, 7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
}
-static void chv_setup_private_ppat(struct drm_i915_private *dev_priv)
+static void chv_setup_private_ppat(struct intel_ppat *ppat)
{
- u64 pat;
+ ppat->max_entries = 8;
+ ppat->update_hw = bdw_private_pat_update_hw;
+ ppat->match = chv_private_pat_match;
+ ppat->clear_value = CHV_PPAT_SNOOP;
/*
* Map WB on BDW to snooped on CHV.
@@ -2894,17 +3065,15 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv)
* Which means we must set the snoop bit in PAT entry 0
* in order to keep the global status page working.
*/
- pat = GEN8_PPAT(0, CHV_PPAT_SNOOP) |
- GEN8_PPAT(1, 0) |
- GEN8_PPAT(2, 0) |
- GEN8_PPAT(3, 0) |
- GEN8_PPAT(4, CHV_PPAT_SNOOP) |
- GEN8_PPAT(5, CHV_PPAT_SNOOP) |
- GEN8_PPAT(6, CHV_PPAT_SNOOP) |
- GEN8_PPAT(7, CHV_PPAT_SNOOP);
- I915_WRITE(GEN8_PRIVATE_PAT_LO, pat);
- I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32);
+ __alloc_ppat_entry(ppat, 0, CHV_PPAT_SNOOP);
+ __alloc_ppat_entry(ppat, 1, 0);
+ __alloc_ppat_entry(ppat, 2, 0);
+ __alloc_ppat_entry(ppat, 3, 0);
+ __alloc_ppat_entry(ppat, 4, CHV_PPAT_SNOOP);
+ __alloc_ppat_entry(ppat, 5, CHV_PPAT_SNOOP);
+ __alloc_ppat_entry(ppat, 6, CHV_PPAT_SNOOP);
+ __alloc_ppat_entry(ppat, 7, CHV_PPAT_SNOOP);
}
static void gen6_gmch_remove(struct i915_address_space *vm)
@@ -2917,12 +3086,27 @@ 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->i915 = dev_priv;
+
if (INTEL_GEN(dev_priv) >= 10)
- cnl_setup_private_ppat(dev_priv);
+ cnl_setup_private_ppat(ppat);
else if (IS_CHERRYVIEW(dev_priv) || IS_GEN9_LP(dev_priv))
- chv_setup_private_ppat(dev_priv);
+ chv_setup_private_ppat(ppat);
else
- bdw_setup_private_ppat(dev_priv);
+ bdw_setup_private_ppat(ppat);
+
+ GEM_BUG_ON(ppat->max_entries > INTEL_MAX_PPAT_ENTRIES);
+
+ for_each_clear_bit(i, ppat->used, ppat->max_entries) {
+ ppat->entries[i].value = ppat->clear_value;
+ ppat->entries[i].ppat = ppat;
+ set_bit(i, ppat->dirty);
+ }
+
+ ppat->update_hw(dev_priv);
}
static int gen8_gmch_probe(struct i915_ggtt *ggtt)
@@ -3236,13 +3420,10 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
ggtt->base.closed = false;
if (INTEL_GEN(dev_priv) >= 8) {
- 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);
+ struct intel_ppat *ppat = &dev_priv->ppat;
+ bitmap_set(ppat->dirty, 0, ppat->max_entries);
+ dev_priv->ppat.update_hw(dev_priv);
return;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b4e3aa7..e10ca89 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -143,6 +143,11 @@ typedef u64 gen8_ppgtt_pml4e_t;
#define GEN8_PPAT_ELLC_OVERRIDE (0<<2)
#define GEN8_PPAT(i, x) ((u64)(x) << ((i) * 8))
+#define GEN8_PPAT_GET_CA(x) ((x) & 3)
+#define GEN8_PPAT_GET_TC(x) ((x) & (3 << 2))
+#define GEN8_PPAT_GET_AGE(x) ((x) & (3 << 4))
+#define CHV_PPAT_GET_SNOOP(x) ((x) & (1 << 6))
+
struct sg_table;
struct intel_rotation_info {
@@ -536,6 +541,37 @@ i915_vm_to_ggtt(struct i915_address_space *vm)
return container_of(vm, struct i915_ggtt, base);
}
+#define INTEL_MAX_PPAT_ENTRIES 8
+#define INTEL_PPAT_PERFECT_MATCH (~0U)
+
+struct intel_ppat;
+
+struct intel_ppat_entry {
+ struct intel_ppat *ppat;
+ struct kref ref;
+ u8 value;
+};
+
+struct intel_ppat {
+ struct intel_ppat_entry entries[INTEL_MAX_PPAT_ENTRIES];
+ DECLARE_BITMAP(used, INTEL_MAX_PPAT_ENTRIES);
+ DECLARE_BITMAP(dirty, INTEL_MAX_PPAT_ENTRIES);
+ unsigned int max_entries;
+ u8 clear_value;
+ /*
+ * Return a score to show how two PPAT values match,
+ * a INTEL_PPAT_PERFECT_MATCH indicates a perfect match
+ */
+ unsigned int (*match)(u8 src, u8 dst);
+ void (*update_hw)(struct drm_i915_private *i915);
+
+ struct drm_i915_private *i915;
+};
+
+const struct intel_ppat_entry *
+intel_ppat_get(struct drm_i915_private *i915, u8 value);
+void intel_ppat_put(const 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);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 3/5] drm/i915: Remove the "INDEX" suffix from PPAT marcos
2017-09-08 9:05 [PATCH v8 1/5] drm/i915: Factor out setup_private_pat() Zhi Wang
2017-09-08 9:05 ` [PATCH v8 2/5] drm/i915: Introduce private PAT management Zhi Wang
@ 2017-09-08 9:05 ` Zhi Wang
2017-09-08 14:18 ` Chris Wilson
2017-09-08 9:05 ` [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries Zhi Wang
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2017-09-08 9:05 UTC (permalink / raw)
To: intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
Remove the "INDEX" suffix from PPAT marcos as they are bits actually, not
indexes.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/gvt/gtt.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +++++-----
drivers/gpu/drm/i915/i915_gem_gtt.h | 8 ++++----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 0bd028f..2801d70 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -1971,7 +1971,7 @@ static int alloc_scratch_pages(struct intel_vgpu *vgpu,
*/
se.val64 |= _PAGE_PRESENT | _PAGE_RW;
if (type == GTT_TYPE_PPGTT_PDE_PT)
- se.val64 |= PPAT_CACHED_INDEX;
+ se.val64 |= PPAT_CACHED;
for (i = 0; i < page_entry_num; i++)
ops->set_entry(scratch_pt, &se, i, false, 0, vgpu);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8cecd90..d8d2b4a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -230,13 +230,13 @@ static gen8_pte_t gen8_pte_encode(dma_addr_t addr,
switch (level) {
case I915_CACHE_NONE:
- pte |= PPAT_UNCACHED_INDEX;
+ pte |= PPAT_UNCACHED;
break;
case I915_CACHE_WT:
- pte |= PPAT_DISPLAY_ELLC_INDEX;
+ pte |= PPAT_DISPLAY_ELLC;
break;
default:
- pte |= PPAT_CACHED_INDEX;
+ pte |= PPAT_CACHED;
break;
}
@@ -249,9 +249,9 @@ static gen8_pde_t gen8_pde_encode(const dma_addr_t addr,
gen8_pde_t pde = _PAGE_PRESENT | _PAGE_RW;
pde |= addr;
if (level != I915_CACHE_NONE)
- pde |= PPAT_CACHED_PDE_INDEX;
+ pde |= PPAT_CACHED_PDE;
else
- pde |= PPAT_UNCACHED_INDEX;
+ pde |= PPAT_UNCACHED;
return pde;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index e10ca89..0178416 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -126,10 +126,10 @@ typedef u64 gen8_ppgtt_pml4e_t;
* tables */
#define GEN8_PDPE_MASK 0x1ff
-#define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD)
-#define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */
-#define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */
-#define PPAT_DISPLAY_ELLC_INDEX _PAGE_PCD /* WT eLLC */
+#define PPAT_UNCACHED (_PAGE_PWT | _PAGE_PCD)
+#define PPAT_CACHED_PDE 0 /* WB LLC */
+#define PPAT_CACHED _PAGE_PAT /* WB LLCeLLC */
+#define PPAT_DISPLAY_ELLC _PAGE_PCD /* WT eLLC */
#define CHV_PPAT_SNOOP (1<<6)
#define GEN8_PPAT_AGE(x) (x<<4)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries
2017-09-08 9:05 [PATCH v8 1/5] drm/i915: Factor out setup_private_pat() Zhi Wang
2017-09-08 9:05 ` [PATCH v8 2/5] drm/i915: Introduce private PAT management Zhi Wang
2017-09-08 9:05 ` [PATCH v8 3/5] drm/i915: Remove the "INDEX" suffix from PPAT marcos Zhi Wang
@ 2017-09-08 9:05 ` Zhi Wang
2017-09-08 14:27 ` Chris Wilson
2017-09-08 9:05 ` [PATCH v8 5/5] drm/i915/selftests: Introduce live tests of private PAT management Zhi Wang
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2017-09-08 9:05 UTC (permalink / raw)
To: intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
Only PPAT entries 0/2/3/4 are using. Remove extra PPAT entry allocation
during initialization.
v8:
- Move ppat_index() into i915_gem_gtt.c. (Chris)
- Change the name of ppat_bits_to_index to ppat_index.
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 53 +++++++++++++++++++------------------
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d8d2b4a..82cb97b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2979,6 +2979,13 @@ static unsigned int chv_private_pat_match(u8 src, u8 dst)
INTEL_PPAT_PERFECT_MATCH : 0;
}
+/* PPAT index = 4 * PAT + 2 * PCD + PWT */
+static inline unsigned int ppat_index(unsigned int bits)
+{
+ return (4 * !!(bits & _PAGE_PAT) + 2 * !!(bits & _PAGE_PCD)
+ + !!(bits & _PAGE_PWT));
+}
+
static void cnl_setup_private_ppat(struct intel_ppat *ppat)
{
ppat->max_entries = 8;
@@ -2988,18 +2995,15 @@ static void cnl_setup_private_ppat(struct intel_ppat *ppat)
/* XXX: spec is unclear if this is still needed for CNL+ */
if (!USES_PPGTT(ppat->i915)) {
- __alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED_PDE), GEN8_PPAT_UC);
return;
}
- __alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);
- __alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);
- __alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
- __alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);
- __alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
- __alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
- __alloc_ppat_entry(ppat, 6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2));
- __alloc_ppat_entry(ppat, 7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
+ /* See gen8_pte_encode() for the mapping from cache-level to PPAT */
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED_PDE), GEN8_PPAT_WB | GEN8_PPAT_LLC);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_DISPLAY_ELLC), GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_UNCACHED), GEN8_PPAT_UC);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED), GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
}
/* The GGTT and PPGTT need a private PPAT setup in order to handle cacheability
@@ -3026,18 +3030,18 @@ static void bdw_setup_private_ppat(struct intel_ppat *ppat)
* So we can still hold onto all our assumptions wrt cpu
* clflushing on LLC machines.
*/
- __alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED_PDE), GEN8_PPAT_UC);
return;
}
- __alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* for normal objects, no eLLC */
- __alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC); /* for something pointing to ptes? */
- __alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for scanout with eLLC */
- __alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* Uncached objects, mostly for scanout */
- __alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
- __alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1));
- __alloc_ppat_entry(ppat, 6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2));
- __alloc_ppat_entry(ppat, 7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3));
+ /* See gen8_pte_encode() for the mapping from cache-level to PPAT */
+ /* for normal objects, no eLLC */
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED_PDE), GEN8_PPAT_WB | GEN8_PPAT_LLC);
+ /* for scanout with eLLC */
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_DISPLAY_ELLC), GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);
+ /* Uncached objects, mostly for scanout */
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_UNCACHED), GEN8_PPAT_UC);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED), GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0));
}
static void chv_setup_private_ppat(struct intel_ppat *ppat)
@@ -3066,14 +3070,11 @@ static void chv_setup_private_ppat(struct intel_ppat *ppat)
* in order to keep the global status page working.
*/
- __alloc_ppat_entry(ppat, 0, CHV_PPAT_SNOOP);
- __alloc_ppat_entry(ppat, 1, 0);
- __alloc_ppat_entry(ppat, 2, 0);
- __alloc_ppat_entry(ppat, 3, 0);
- __alloc_ppat_entry(ppat, 4, CHV_PPAT_SNOOP);
- __alloc_ppat_entry(ppat, 5, CHV_PPAT_SNOOP);
- __alloc_ppat_entry(ppat, 6, CHV_PPAT_SNOOP);
- __alloc_ppat_entry(ppat, 7, CHV_PPAT_SNOOP);
+ /* See gen8_pte_encode() for the mapping from cache-level to PPAT */
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED_PDE), CHV_PPAT_SNOOP);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_DISPLAY_ELLC), 0);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_UNCACHED), 0);
+ __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED), CHV_PPAT_SNOOP);
}
static void gen6_gmch_remove(struct i915_address_space *vm)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries
2017-09-08 9:05 ` [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries Zhi Wang
@ 2017-09-08 14:27 ` Chris Wilson
2017-09-08 16:30 ` Wang, Zhi A
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-09-08 14:27 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
Quoting Zhi Wang (2017-09-08 10:05:52)
> Only PPAT entries 0/2/3/4 are using. Remove extra PPAT entry allocation
> during initialization.
>
> v8:
>
> - Move ppat_index() into i915_gem_gtt.c. (Chris)
> - Change the name of ppat_bits_to_index to ppat_index.
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 53 +++++++++++++++++++------------------
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d8d2b4a..82cb97b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2979,6 +2979,13 @@ static unsigned int chv_private_pat_match(u8 src, u8 dst)
> INTEL_PPAT_PERFECT_MATCH : 0;
> }
>
> +/* PPAT index = 4 * PAT + 2 * PCD + PWT */
> +static inline unsigned int ppat_index(unsigned int bits)
> +{
> + return (4 * !!(bits & _PAGE_PAT) + 2 * !!(bits & _PAGE_PCD)
> + + !!(bits & _PAGE_PWT));
I'm feeling very dumb, having quickly grepped the bspec for why those
bits map to a particular PAT entry. Clue for enlightenment?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries
2017-09-08 14:27 ` Chris Wilson
@ 2017-09-08 16:30 ` Wang, Zhi A
0 siblings, 0 replies; 12+ messages in thread
From: Wang, Zhi A @ 2017-09-08 16:30 UTC (permalink / raw)
To: Chris Wilson, intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org
Cc: Vivi, Rodrigo, Widawsky, Benjamin
Check BSpec>Memory Views>Memory Types and Cache Interface>Memory Type [SKL+]>PAT (IA32e) [SKL+]
-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
Sent: Friday, September 8, 2017 5:27 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>; Widawsky, Benjamin <benjamin.widawsky@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries
Quoting Zhi Wang (2017-09-08 10:05:52)
> Only PPAT entries 0/2/3/4 are using. Remove extra PPAT entry
> allocation during initialization.
>
> v8:
>
> - Move ppat_index() into i915_gem_gtt.c. (Chris)
> - Change the name of ppat_bits_to_index to ppat_index.
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 53
> +++++++++++++++++++------------------
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d8d2b4a..82cb97b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2979,6 +2979,13 @@ static unsigned int chv_private_pat_match(u8 src, u8 dst)
> INTEL_PPAT_PERFECT_MATCH : 0; }
>
> +/* PPAT index = 4 * PAT + 2 * PCD + PWT */ static inline unsigned int
> +ppat_index(unsigned int bits) {
> + return (4 * !!(bits & _PAGE_PAT) + 2 * !!(bits & _PAGE_PCD)
> + + !!(bits & _PAGE_PWT));
I'm feeling very dumb, having quickly grepped the bspec for why those bits map to a particular PAT entry. Clue for enlightenment?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 5/5] drm/i915/selftests: Introduce live tests of private PAT management
2017-09-08 9:05 [PATCH v8 1/5] drm/i915: Factor out setup_private_pat() Zhi Wang
` (2 preceding siblings ...)
2017-09-08 9:05 ` [PATCH v8 4/5] drm/i915: Do not allocate unused PPAT entries Zhi Wang
@ 2017-09-08 9:05 ` Zhi Wang
2017-09-08 14:42 ` Chris Wilson
2017-09-08 9:51 ` ✗ Fi.CI.BAT: failure for series starting with [v8,1/5] drm/i915: Factor out setup_private_pat() Patchwork
2017-09-08 11:54 ` [PATCH v8 1/5] " Joonas Lahtinen
5 siblings, 1 reply; 12+ messages in thread
From: Zhi Wang @ 2017-09-08 9:05 UTC (permalink / raw)
To: intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
Introduce two live tests of private PAT management:
igt_ppat_init - This test is to check if all the PPAT configuration is
written into HW.
igt_ppat_get - This test performs several sub-tests on intel_ppat_get()
and intel_ppat_put().
The "perfect match" test case will try to get a PPAT entry with an existing
value, then check if the returned PPAT entry is the same one.
The "alloc entries" test case will run out of PPAT table, and check if all
the requested values are put into the newly allocated PPAT entries.
The negative test case will try to generate a new PPAT value, and get it
when PPAT table is full.
The "partial match" test case will generate a parital matched value from
the existing PPAT table and try to match it.
The "put entries" test case will free all the PPAT entries that allocated
in "alloc entries" test case. It will check if the values of freed PPAT
entries turn into ppat->clear_value.
v8:
- Remove noisy output. (Chris)
- Add negative test case. (Chris)
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 276 ++++++++++++++++++++++++++
1 file changed, 276 insertions(+)
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 6b132ca..c5179ce 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1094,6 +1094,280 @@ static int igt_ggtt_page(void *arg)
return err;
}
+static int check_cnl_ppat(struct drm_i915_private *dev_priv)
+{
+ struct intel_ppat *ppat = &dev_priv->ppat;
+ int i;
+
+ for (i = 0; i < ppat->max_entries; i++) {
+ u32 value = I915_READ(GEN10_PAT_INDEX(i));
+
+ if (value != ppat->entries[i].value) {
+ pr_err("expected PPAT value isn't written into HW\n");
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+
+static int check_bdw_ppat(struct drm_i915_private *dev_priv)
+{
+ struct intel_ppat *ppat = &dev_priv->ppat;
+ u64 pat, hw_pat;
+ int i;
+
+ pat = hw_pat = 0;
+
+ for (i = 0; i < ppat->max_entries; i++)
+ pat |= GEN8_PPAT(i, ppat->entries[i].value);
+
+ hw_pat = I915_READ(GEN8_PRIVATE_PAT_HI);
+ hw_pat <<= 32;
+ hw_pat |= I915_READ(GEN8_PRIVATE_PAT_LO);
+
+ if (pat != hw_pat) {
+ pr_err("expected PPAT value isn't written into HW\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int igt_ppat_check(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ int ret;
+
+ if (!i915->ppat.max_entries)
+ return 0;
+
+ if (INTEL_GEN(i915) >= 10)
+ ret = check_cnl_ppat(i915);
+ else
+ ret = check_bdw_ppat(i915);
+
+ return ret;
+}
+
+static u8 generate_new_value(struct intel_ppat *ppat, bool partial,
+ bool negative)
+{
+ u8 ca[] = { GEN8_PPAT_WB, GEN8_PPAT_WT, GEN8_PPAT_UC, GEN8_PPAT_WC };
+ u8 tc[] = { GEN8_PPAT_LLC, GEN8_PPAT_LLCELLC, GEN8_PPAT_LLCeLLC };
+ u8 age[] = { GEN8_PPAT_AGE(3), GEN8_PPAT_AGE(2), GEN8_PPAT_AGE(1),
+ GEN8_PPAT_AGE(0) };
+ u8 value = 0;
+ bool same;
+ int ca_index, tc_index, age_index, i;
+
+#define for_each_ppat_attr(ca_index, tc_index, age_index) \
+ for ((ca_index) = 0 ; (ca_index) < ARRAY_SIZE(ca); (ca_index)++) \
+ for ((tc_index) = 0; (tc_index) < ARRAY_SIZE(tc); (tc_index)++) \
+ for ((age_index) = 0; (age_index) < ARRAY_SIZE(age); (age_index)++)
+
+ for_each_ppat_attr(ca_index, tc_index, age_index) {
+ value = age[age_index] | ca[ca_index] | tc[tc_index];
+ same = false;
+
+ for_each_set_bit(i, ppat->used, ppat->max_entries) {
+ if (value != ppat->entries[i].value)
+ continue;
+
+ same = true;
+ break;
+ }
+
+ if (same)
+ continue;
+
+ if (!partial && !negative)
+ return value;
+
+ if (partial) {
+ /* cache attribute has to be the same. */
+ for_each_set_bit(i, ppat->used, ppat->max_entries) {
+ if (GEN8_PPAT_GET_CA(value) !=
+ GEN8_PPAT_GET_CA(ppat->entries[i].value))
+ continue;
+
+ return value;
+ }
+ }
+
+ if (negative) {
+ /* cache attribute has to be new. */
+ same = false;
+ for_each_set_bit(i, ppat->used, ppat->max_entries) {
+ if (GEN8_PPAT_GET_CA(value) ==
+ GEN8_PPAT_GET_CA(ppat->entries[i].value)) {
+ same = true;
+ break;
+ }
+ }
+ if (same)
+ continue;
+ return value;
+ }
+ }
+#undef for_each_ppat_attr
+ return 0;
+}
+
+static inline bool ppat_table_is_full(struct intel_ppat *ppat)
+{
+ return bitmap_weight(ppat->used, ppat->max_entries) ==
+ ppat->max_entries;
+}
+
+static int igt_ppat_get(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct intel_ppat *ppat = &i915->ppat;
+ const struct intel_ppat_entry **entries;
+ const struct intel_ppat_entry *entry;
+ unsigned int size = 0;
+ u8 value;
+ int i, ret;
+
+ if (!ppat->max_entries)
+ return 0;
+
+ ret = igt_ppat_check(i915);
+ if (ret)
+ return ret;
+
+ /* case 1: perfect match */
+ entry = intel_ppat_get(i915, ppat->entries[0].value);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+
+ if (entry != &ppat->entries[0]) {
+ pr_err("not expected entry\n");
+ intel_ppat_put(entry);
+ return -EINVAL;
+ }
+
+ intel_ppat_put(entry);
+
+ /* case 2: alloc new entries */
+ entries = NULL;
+ ret = 0;
+
+ while (!ppat_table_is_full(ppat)) {
+ const struct intel_ppat_entry **p;
+ DECLARE_BITMAP(used, INTEL_MAX_PPAT_ENTRIES);
+
+ bitmap_copy(used, ppat->used, ppat->max_entries);
+
+ p = krealloc(entries, (size + 1) *
+ sizeof(struct intel_ppat_entry *),
+ GFP_KERNEL);
+ if (!p) {
+ ret = -ENOSPC;
+ break;
+ }
+ entries = p;
+
+ p = &entries[size++];
+ *p = NULL;
+
+ value = generate_new_value(ppat, false, false);
+ if (!value) {
+ pr_err("cannot fill the unused PPAT entries?\n");
+ ret = -EINVAL;
+ break;
+ }
+
+ *p = entry = intel_ppat_get(i915, value);
+ if (IS_ERR(entry)) {
+ pr_err("fail to get new entry\n");
+ ret = PTR_ERR(entry);
+ break;
+ }
+
+ if (entry->value != value) {
+ pr_err("fail to get expected new value\n");
+ ret = -EINVAL;
+ break;
+ }
+
+ if (bitmap_equal(used, ppat->used, ppat->max_entries)) {
+ pr_err("fail to alloc a new entry\n");
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+ if (ret)
+ goto ppat_put;
+
+ /* case 3: negative test, suppose PPAT table is full now */
+ value = generate_new_value(ppat, false, true);
+ if (!value) {
+ pr_err("fail to get new value\n");
+ ret = -EINVAL;
+ goto ppat_put;
+ }
+
+ entry = intel_ppat_get(i915, value);
+ if (!IS_ERR(entry) || PTR_ERR(entry) != -ENOSPC) {
+ pr_err("fail on negative test\n");
+ ret = -EINVAL;
+ goto ppat_put;
+ }
+
+ ret = 0;
+
+ /* case 4: partial match */
+ value = generate_new_value(ppat, true, false);
+ if (!value) {
+ pr_err("fail to get new value\n");
+ ret = -EINVAL;
+ goto ppat_put;
+ }
+
+ entry = intel_ppat_get(i915, value);
+ if (IS_ERR(entry)) {
+ pr_err("fail to get new entry\n");
+ ret = PTR_ERR(entry);
+ goto ppat_put;
+ }
+
+ if (!(entry->value != value &&
+ GEN8_PPAT_GET_CA(entry->value) == GEN8_PPAT_GET_CA(value))) {
+ pr_err("fail to get expected value\n");
+ ret = -EINVAL;
+ }
+
+ intel_ppat_put(entry);
+
+ppat_put:
+ if (entries) {
+ for (i = 0; i < size; i++) {
+ if (IS_ERR(entries[i]) || !entries[i])
+ continue;
+
+ intel_ppat_put(entries[i]);
+
+ if (entries[i]->value != ppat->clear_value) {
+ pr_err("fail to put ppat value\n");
+ ret = -EINVAL;
+ break;
+ }
+ }
+ kfree(entries);
+ entries = NULL;
+ }
+
+ if (ret)
+ return ret;
+
+ ret = igt_ppat_check(i915);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static void track_vma_bind(struct i915_vma *vma)
{
struct drm_i915_gem_object *obj = vma->obj;
@@ -1560,6 +1834,8 @@ int i915_gem_gtt_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_ggtt_pot),
SUBTEST(igt_ggtt_fill),
SUBTEST(igt_ggtt_page),
+ SUBTEST(igt_ppat_check),
+ SUBTEST(igt_ppat_get),
};
GEM_BUG_ON(offset_in_page(i915->ggtt.base.total));
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v8 5/5] drm/i915/selftests: Introduce live tests of private PAT management
2017-09-08 9:05 ` [PATCH v8 5/5] drm/i915/selftests: Introduce live tests of private PAT management Zhi Wang
@ 2017-09-08 14:42 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-09-08 14:42 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
Quoting Zhi Wang (2017-09-08 10:05:53)
> Introduce two live tests of private PAT management:
>
> igt_ppat_init - This test is to check if all the PPAT configuration is
> written into HW.
>
> igt_ppat_get - This test performs several sub-tests on intel_ppat_get()
> and intel_ppat_put().
>
> The "perfect match" test case will try to get a PPAT entry with an existing
> value, then check if the returned PPAT entry is the same one.
>
> The "alloc entries" test case will run out of PPAT table, and check if all
> the requested values are put into the newly allocated PPAT entries.
>
> The negative test case will try to generate a new PPAT value, and get it
> when PPAT table is full.
>
> The "partial match" test case will generate a parital matched value from
> the existing PPAT table and try to match it.
>
> The "put entries" test case will free all the PPAT entries that allocated
> in "alloc entries" test case. It will check if the values of freed PPAT
> entries turn into ppat->clear_value.
>
> v8:
>
> - Remove noisy output. (Chris)
> - Add negative test case. (Chris)
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 276 ++++++++++++++++++++++++++
> 1 file changed, 276 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 6b132ca..c5179ce 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -1094,6 +1094,280 @@ static int igt_ggtt_page(void *arg)
> return err;
> }
>
> +static int check_cnl_ppat(struct drm_i915_private *dev_priv)
> +{
> + struct intel_ppat *ppat = &dev_priv->ppat;
> + int i;
> +
> + for (i = 0; i < ppat->max_entries; i++) {
> + u32 value = I915_READ(GEN10_PAT_INDEX(i));
> +
> + if (value != ppat->entries[i].value) {
> + pr_err("expected PPAT value isn't written into HW\n");
Always helpful to include details such as expected and found values.
> + return -EINVAL;
> + }
> + }
> + return 0;
> +}
> +
> +static int check_bdw_ppat(struct drm_i915_private *dev_priv)
> +{
> + struct intel_ppat *ppat = &dev_priv->ppat;
> + u64 pat, hw_pat;
> + int i;
> +
> + pat = hw_pat = 0;
> +
> + for (i = 0; i < ppat->max_entries; i++)
> + pat |= GEN8_PPAT(i, ppat->entries[i].value);
> +
> + hw_pat = I915_READ(GEN8_PRIVATE_PAT_HI);
> + hw_pat <<= 32;
> + hw_pat |= I915_READ(GEN8_PRIVATE_PAT_LO);
> +
> + if (pat != hw_pat) {
> + pr_err("expected PPAT value isn't written into HW\n");
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int igt_ppat_check(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + int ret;
> +
> + if (!i915->ppat.max_entries)
> + return 0;
> +
> + if (INTEL_GEN(i915) >= 10)
> + ret = check_cnl_ppat(i915);
> + else
> + ret = check_bdw_ppat(i915);
> +
> + return ret;
> +}
Excellent little checker.
> +
> +static u8 generate_new_value(struct intel_ppat *ppat, bool partial,
> + bool negative)
More than one bool parameter is a recipe for confusion :)
> +{
> + u8 ca[] = { GEN8_PPAT_WB, GEN8_PPAT_WT, GEN8_PPAT_UC, GEN8_PPAT_WC };
> + u8 tc[] = { GEN8_PPAT_LLC, GEN8_PPAT_LLCELLC, GEN8_PPAT_LLCeLLC };
> + u8 age[] = { GEN8_PPAT_AGE(3), GEN8_PPAT_AGE(2), GEN8_PPAT_AGE(1),
> + GEN8_PPAT_AGE(0) };
> + u8 value = 0;
> + bool same;
> + int ca_index, tc_index, age_index, i;
> +
> +#define for_each_ppat_attr(ca_index, tc_index, age_index) \
> + for ((ca_index) = 0 ; (ca_index) < ARRAY_SIZE(ca); (ca_index)++) \
> + for ((tc_index) = 0; (tc_index) < ARRAY_SIZE(tc); (tc_index)++) \
> + for ((age_index) = 0; (age_index) < ARRAY_SIZE(age); (age_index)++)
> +
> + for_each_ppat_attr(ca_index, tc_index, age_index) {
> + value = age[age_index] | ca[ca_index] | tc[tc_index];
> + same = false;
> +
> + for_each_set_bit(i, ppat->used, ppat->max_entries) {
> + if (value != ppat->entries[i].value)
> + continue;
> +
> + same = true;
> + break;
> + }
> +
> + if (same)
> + continue;
> +
> + if (!partial && !negative)
> + return value;
> +
> + if (partial) {
> + /* cache attribute has to be the same. */
> + for_each_set_bit(i, ppat->used, ppat->max_entries) {
> + if (GEN8_PPAT_GET_CA(value) !=
> + GEN8_PPAT_GET_CA(ppat->entries[i].value))
> + continue;
> +
> + return value;
> + }
> + }
> +
> + if (negative) {
> + /* cache attribute has to be new. */
> + same = false;
> + for_each_set_bit(i, ppat->used, ppat->max_entries) {
> + if (GEN8_PPAT_GET_CA(value) ==
> + GEN8_PPAT_GET_CA(ppat->entries[i].value)) {
> + same = true;
> + break;
> + }
> + }
> + if (same)
> + continue;
> + return value;
> + }
> + }
> +#undef for_each_ppat_attr
> + return 0;
> +}
> +
> +static inline bool ppat_table_is_full(struct intel_ppat *ppat)
> +{
> + return bitmap_weight(ppat->used, ppat->max_entries) ==
> + ppat->max_entries;
return bitmap_full();
> +}
> +
> +static int igt_ppat_get(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct intel_ppat *ppat = &i915->ppat;
> + const struct intel_ppat_entry **entries;
> + const struct intel_ppat_entry *entry;
> + unsigned int size = 0;
> + u8 value;
> + int i, ret;
> +
> + if (!ppat->max_entries)
> + return 0;
> +
> + ret = igt_ppat_check(i915);
> + if (ret)
> + return ret;
> +
> + /* case 1: perfect match */
> + entry = intel_ppat_get(i915, ppat->entries[0].value);
> + if (IS_ERR(entry))
> + return PTR_ERR(entry);
> +
> + if (entry != &ppat->entries[0]) {
> + pr_err("not expected entry\n");
> + intel_ppat_put(entry);
> + return -EINVAL;
> + }
Ok, but finding entries[0] is easy. ;)
for_each_bit(bit, &used) {
entry = intel_ppat_get(i915, ppat->entries[bit].value);
if (IS_ERR(entry)) {
"found nothing for index %d, value=%x\n"
}
if (entry->value != ppat->entries[bit].value)) {
"lookup failed"
}
}
> +
> + intel_ppat_put(entry);
> +
> + /* case 2: alloc new entries */
> + entries = NULL;
> + ret = 0;
> +
> + while (!ppat_table_is_full(ppat)) {
> + const struct intel_ppat_entry **p;
> + DECLARE_BITMAP(used, INTEL_MAX_PPAT_ENTRIES);
> +
> + bitmap_copy(used, ppat->used, ppat->max_entries);
> +
> + p = krealloc(entries, (size + 1) *
> + sizeof(struct intel_ppat_entry *),
> + GFP_KERNEL);
> + if (!p) {
> + ret = -ENOSPC;
> + break;
> + }
> + entries = p;
> +
> + p = &entries[size++];
> + *p = NULL;
> +
> + value = generate_new_value(ppat, false, false);
> + if (!value) {
> + pr_err("cannot fill the unused PPAT entries?\n");
> + ret = -EINVAL;
> + break;
> + }
> +
> + *p = entry = intel_ppat_get(i915, value);
> + if (IS_ERR(entry)) {
> + pr_err("fail to get new entry\n");
> + ret = PTR_ERR(entry);
> + break;
> + }
> +
> + if (entry->value != value) {
> + pr_err("fail to get expected new value\n");
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (bitmap_equal(used, ppat->used, ppat->max_entries)) {
> + pr_err("fail to alloc a new entry\n");
> + ret = -EINVAL;
> + break;
> + }
> + }
Ok.
> +
> + if (ret)
> + goto ppat_put;
> +
> + /* case 3: negative test, suppose PPAT table is full now */
> + value = generate_new_value(ppat, false, true);
> + if (!value) {
> + pr_err("fail to get new value\n");
> + ret = -EINVAL;
> + goto ppat_put;
> + }
> +
> + entry = intel_ppat_get(i915, value);
> + if (!IS_ERR(entry) || PTR_ERR(entry) != -ENOSPC) {
> + pr_err("fail on negative test\n");
> + ret = -EINVAL;
> + goto ppat_put;
> + }
Ok.
> +
> + ret = 0;
> +
> + /* case 4: partial match */
> + value = generate_new_value(ppat, true, false);
> + if (!value) {
> + pr_err("fail to get new value\n");
> + ret = -EINVAL;
> + goto ppat_put;
> + }
> +
> + entry = intel_ppat_get(i915, value);
> + if (IS_ERR(entry)) {
> + pr_err("fail to get new entry\n");
> + ret = PTR_ERR(entry);
> + goto ppat_put;
> + }
> +
> + if (!(entry->value != value &&
> + GEN8_PPAT_GET_CA(entry->value) == GEN8_PPAT_GET_CA(value))) {
> + pr_err("fail to get expected value\n");
> + ret = -EINVAL;
> + }
> +
> + intel_ppat_put(entry);
Now check that you can allocate a new entry, the table is still full at
this point.
> +
> +ppat_put:
> + if (entries) {
> + for (i = 0; i < size; i++) {
> + if (IS_ERR(entries[i]) || !entries[i])
> + continue;
> +
> + intel_ppat_put(entries[i]);
> +
> + if (entries[i]->value != ppat->clear_value) {
> + pr_err("fail to put ppat value\n");
> + ret = -EINVAL;
> + break;
> + }
> + }
> + kfree(entries);
> + entries = NULL;
> + }
> +
> + if (ret)
> + return ret;
> +
> + ret = igt_ppat_check(i915);
> + if (ret)
> + return ret;
Makes sense to run this after every phase. Especially after filling and
then after freeing.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [v8,1/5] drm/i915: Factor out setup_private_pat()
2017-09-08 9:05 [PATCH v8 1/5] drm/i915: Factor out setup_private_pat() Zhi Wang
` (3 preceding siblings ...)
2017-09-08 9:05 ` [PATCH v8 5/5] drm/i915/selftests: Introduce live tests of private PAT management Zhi Wang
@ 2017-09-08 9:51 ` Patchwork
2017-09-08 11:54 ` [PATCH v8 1/5] " Joonas Lahtinen
5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-09-08 9:51 UTC (permalink / raw)
To: Zhi Wang; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v8,1/5] drm/i915: Factor out setup_private_pat()
URL : https://patchwork.freedesktop.org/series/30004/
State : failure
== Summary ==
Series 30004v1 series starting with [v8,1/5] drm/i915: Factor out setup_private_pat()
https://patchwork.freedesktop.org/api/1.0/series/30004/revisions/1/mbox/
Test gem_exec_basic:
Subgroup basic-vebox:
skip -> INCOMPLETE (fi-byt-j1900)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-skl-x1585l) fdo#101781
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:458s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:449s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:363s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:557s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:257s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:521s
fi-byt-j1900 total:39 pass:27 dwarn:0 dfail:0 fail:0 skip:11
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:511s
fi-cfl-s total:289 pass:250 dwarn:4 dfail:0 fail:0 skip:35 time:462s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:443s
fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:605s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:448s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:429s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:423s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:509s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:479s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:509s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:598s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:608s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:545s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:469s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:533s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:522s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:447s
fi-skl-x1585l total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:492s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:558s
fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:407s
16fef66706a3fcdd1d87009beab7e09de1f32807 drm-tip: 2017y-09m-08d-08h-41m-49s UTC integration manifest
c494b7902a47 drm/i915/selftests: Introduce live tests of private PAT management
7024001b7f2a drm/i915: Do not allocate unused PPAT entries
85194e2bbed1 drm/i915: Remove the "INDEX" suffix from PPAT marcos
019dee958972 drm/i915: Introduce private PAT management
a66e6a7232df drm/i915: Factor out setup_private_pat()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5614/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v8 1/5] drm/i915: Factor out setup_private_pat()
2017-09-08 9:05 [PATCH v8 1/5] drm/i915: Factor out setup_private_pat() Zhi Wang
` (4 preceding siblings ...)
2017-09-08 9:51 ` ✗ Fi.CI.BAT: failure for series starting with [v8,1/5] drm/i915: Factor out setup_private_pat() Patchwork
@ 2017-09-08 11:54 ` Joonas Lahtinen
2017-09-08 16:21 ` Wang, Zhi A
5 siblings, 1 reply; 12+ messages in thread
From: Joonas Lahtinen @ 2017-09-08 11:54 UTC (permalink / raw)
To: Zhi Wang, intel-gfx, intel-gvt-dev; +Cc: Rodrigo Vivi, Ben Widawsky
On Fri, 2017-09-08 at 17:05 +0800, Zhi Wang wrote:
> Factor out setup_private_pat() for introducing the following patches.
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> v7
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
Keep this in chronological order when you add things, so R-b after S-o-
b. Only add the version after R-b *if* the review might have been
invalidated due to some changes, in this case there are no changes and
no changelog, so you can keep the R-b.
If there was substantial enough changes, changelog would be added and
#v7 added to previous reviews to indicate they do not apply to latest
changes.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v8 1/5] drm/i915: Factor out setup_private_pat()
2017-09-08 11:54 ` [PATCH v8 1/5] " Joonas Lahtinen
@ 2017-09-08 16:21 ` Wang, Zhi A
0 siblings, 0 replies; 12+ messages in thread
From: Wang, Zhi A @ 2017-09-08 16:21 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org
Cc: Vivi, Rodrigo, Widawsky, Benjamin
Thanks! Learned. :)
-----Original Message-----
From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
Sent: Friday, September 8, 2017 2:54 PM
To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
Cc: chris@chris-wilson.co.uk; zhenyuw@linux.intel.com; Widawsky, Benjamin <benjamin.widawsky@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v8 1/5] drm/i915: Factor out setup_private_pat()
On Fri, 2017-09-08 at 17:05 +0800, Zhi Wang wrote:
> Factor out setup_private_pat() for introducing the following patches.
>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> v7
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
Keep this in chronological order when you add things, so R-b after S-o- b. Only add the version after R-b *if* the review might have been invalidated due to some changes, in this case there are no changes and no changelog, so you can keep the R-b.
If there was substantial enough changes, changelog would be added and
#v7 added to previous reviews to indicate they do not apply to latest changes.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread