On 7 July 2017 at 11:34, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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. > >> +/* 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?) > FWIW I have to agree with Chris here - having the value is of limited use. Furthermore it mostly confuses people when writing the user space parts. For example: Mesa implements v1 and uses the define. Kernel headers get updated to v2 and Mesa supporting v1 gets rebuild against them. Mesa stores/treats as the MOCS version has "v2" when the actual hardware/kernel supports "v1". The expected issues vary depending on the implementation, but I suspect it won't be fun :-) IMHO it's better if user space is explicit on the versions it supports and kernel should avoid exposing such defines unless really needed. HTH Emil _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
