On 17-07-07 11:34:48, Chris Wilson wrote:
Quoting Ben Widawsky (2017-07-07 00:27:01)
drivers/gpu/drm/i915/i915_drv.c | 3 +++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_pci.c | 13 +++++++++----
include/uapi/drm/i915_drm.h | 8 ++++++++
4 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9167a73f3c69..26c27b6ae814 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -401,6 +401,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
if (!value)
return -ENODEV;
break;
+ case I915_PARAM_MOCS_TABLE_VERSION:
+ value = INTEL_INFO(dev_priv)->mocs_version;
If we use intel_mocs_get_table_version() we can put this magic number
in intel_mocs.c next to the tables, where we can keep its history and
hopefully be able to remember to update it.
Yeah, that seems like an improvement to me as well.
+/* What version of the MOCS table we have. For GEN9 GPUs, the PRM defined
+ * non-optimal settings for the MOCS table. As a result, we were required to use a
+ * small subset, and later add new settings. This param allows userspace to
+ * determine which settings are there.
+ */
+#define MOCS_TABLE_VERSION 1 /* Build time MOCS table version */
How are you planing to share this? When we update we bump this number,
and then mesa copies it across and uses it after verifying it as 0,1 on
an old kernel.
I don't think you want to expose the updated constant here, but symbolic
names for each version? (What would be the point?)
At least one thing wrong here is we would need per GEN constants, which is maybe
what you meant and I misunderstood. Assuming you had per GEN constants, which I
don't like, I believe everything works out fine. So, I'll remove this compile
time MOCS versioning.
Next question, why a version number and not just the number of entries
defined? Each index is defined by ABI once assigned, so the number of
entries still operates as a version number and allows easy checking.
if (advanced_cacheing_idx < kernel_max_mocs)
return advanced_cacheing_idx;
if (default_cacheing_idx < kernel_max_mocs)
return default_cacheing_idx;
return follow_pte_idx;
give or take the smarts to choose the preferred indices for any
particular scenario.
In the future, if we finally get user defined mocs, the table_size will
then give the start of the user modifiable indices (presming they want
to keep the predefined entries for compatibility?))
-Chris
Yes, I considered this as well. I see no difference really as to one versus the
other. In fact, if you're to support multiple table versions, I think it's
actually easier with a pure version:
switch (kernel_mocs_version) {
case 3:
return new_best_cacheing_index;
case 2:
return old_best_cacheing_index;
case 1:
return naive_best_index;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
