| Message ID | 20170706232703.14229-4-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 6, 2017 at 4:27 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > We don't yet have optimal MOCS settings, but we have enough to know how > to at least determine when we might have non-optimal settings within our > driver. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > src/intel/vulkan/anv_device.c | 12 ++++++++++++ > src/intel/vulkan/anv_private.h | 2 ++ > src/mesa/drivers/dri/i915/intel_context.c | 7 ++++++- > src/mesa/drivers/dri/i965/intel_screen.c | 14 ++++++++++++++ > src/mesa/drivers/dri/i965/intel_screen.h | 2 ++ > 5 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c > index 3dc55dbb8d..8e180dbf18 100644 > --- a/src/intel/vulkan/anv_device.c > +++ b/src/intel/vulkan/anv_device.c > @@ -368,6 +368,18 @@ anv_physical_device_init(struct anv_physical_device > *device, > device->info.max_cs_threads = max_cs_threads; > } > > + if (device->info.gen >= 9) { > + device->mocs_version = anv_gem_get_param(fd, > + > I915_PARAM_MOCS_TABLE_VERSION); > + switch (device->mocs_version) { > + default: > + anv_perf_warn("Kernel exposes newer MOCS table\n"); > A perf_warn here seems reasonable though it makes more sense to me to make it if (device->mocs_version > ANV_MAX_KNOWN_MOCS_VERSION) anv_perf_warn("..."); > + case 1: > + case 0: > + device->mocs_version = MOCS_TABLE_VERSION; > Why are we stomping device->mocs_version to MOCS_TABLE_VERSION? Are you just trying to avoid the version 0? If so, why not just have /* If the MOCS_TABLE_VERSION query fails, assume version 1 */ if (device->mocs_version == 0) device->mocs_version = 1; I don't think we want to have it dependent on a #define in an external header file. What if someone updates it for i965 and doesn't update anv or vice-versa? > + } > + } > + > brw_process_intel_debug_variable(); > > device->compiler = brw_compiler_create(NULL, &device->info); > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_ > private.h > index 573778dad5..b8241a9b22 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -684,6 +684,8 @@ struct anv_physical_device { > uint32_t eu_total; > uint32_t subslice_total; > > + uint8_t mocs_version; > + > struct { > uint32_t type_count; > struct anv_memory_type > types[VK_MAX_MEMORY_TYPES]; > diff --git a/src/mesa/drivers/dri/i915/intel_context.c > b/src/mesa/drivers/dri/i915/intel_context.c > index e0766a0e3f..9169ea650e 100644 > --- a/src/mesa/drivers/dri/i915/intel_context.c > +++ b/src/mesa/drivers/dri/i915/intel_context.c > @@ -521,8 +521,13 @@ intelInitContext(struct intel_context *intel, > INTEL_DEBUG = parse_debug_string(getenv("INTEL_DEBUG"), > debug_control); > if (INTEL_DEBUG & DEBUG_BUFMGR) > dri_bufmgr_set_debug(intel->bufmgr, true); > - if (INTEL_DEBUG & DEBUG_PERF) > + if (INTEL_DEBUG & DEBUG_PERF) { > intel->perf_debug = true; > + if (screen->mocs_version > MOCS_TABLE_VERSION) { > + fprintf(stderr, "Kernel exposes newer MOCS table\n"); > + screen->mocs_version = MOCS_TABLE_VERSION; > + } > + } > > if (INTEL_DEBUG & DEBUG_AUB) > drm_intel_bufmgr_gem_set_aub_dump(intel->bufmgr, true); > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c > b/src/mesa/drivers/dri/i965/intel_screen.c > index c75f2125d4..c53f133d49 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.c > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > @@ -2301,6 +2301,20 @@ __DRIconfig **intelInitScreen2(__DRIscreen > *dri_screen) > (ret != -1 || errno != EINVAL); > } > > + if (devinfo->gen >= 9) { > + screen->mocs_version = intel_get_integer(screen, > + > I915_PARAM_MOCS_TABLE_VERSION); > + switch (screen->mocs_version) { > + case 1: > + case 0: > + screen->mocs_version = MOCS_TABLE_VERSION; > Same comments apply here. > + break; > + default: > + /* We want to perf debug, but we can't yet */ > + break; > + } > + } > + > dri_screen->extensions = !screen->has_context_reset_notification > ? screenExtensions : intelRobustScreenExtensions; > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.h > b/src/mesa/drivers/dri/i965/intel_screen.h > index f78b3e8f74..eb801f8155 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.h > +++ b/src/mesa/drivers/dri/i965/intel_screen.h > @@ -112,6 +112,8 @@ struct intel_screen > bool mesa_format_supports_texture[MESA_FORMAT_COUNT]; > bool mesa_format_supports_render[MESA_FORMAT_COUNT]; > enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT]; > + > + unsigned mocs_version; > }; > > extern void intelDestroyContext(__DRIcontext * driContextPriv); > -- > 2.13.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On 17-07-07 09:28:08, Jason Ekstrand wrote: >On Thu, Jul 6, 2017 at 4:27 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > >> We don't yet have optimal MOCS settings, but we have enough to know how >> to at least determine when we might have non-optimal settings within our >> driver. >> >> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >> --- >> src/intel/vulkan/anv_device.c | 12 ++++++++++++ >> src/intel/vulkan/anv_private.h | 2 ++ >> src/mesa/drivers/dri/i915/intel_context.c | 7 ++++++- >> src/mesa/drivers/dri/i965/intel_screen.c | 14 ++++++++++++++ >> src/mesa/drivers/dri/i965/intel_screen.h | 2 ++ >> 5 files changed, 36 insertions(+), 1 deletion(-) >> >> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c >> index 3dc55dbb8d..8e180dbf18 100644 >> --- a/src/intel/vulkan/anv_device.c >> +++ b/src/intel/vulkan/anv_device.c >> @@ -368,6 +368,18 @@ anv_physical_device_init(struct anv_physical_device >> *device, >> device->info.max_cs_threads = max_cs_threads; >> } >> >> + if (device->info.gen >= 9) { >> + device->mocs_version = anv_gem_get_param(fd, >> + >> I915_PARAM_MOCS_TABLE_VERSION); >> + switch (device->mocs_version) { >> + default: >> + anv_perf_warn("Kernel exposes newer MOCS table\n"); >> > >A perf_warn here seems reasonable though it makes more sense to me to make >it > >if (device->mocs_version > ANV_MAX_KNOWN_MOCS_VERSION) > anv_perf_warn("..."); > > One thing to keep in mind: the max MOCS version can vary by platform (hopefully it doesn't). >> + case 1: >> + case 0: >> + device->mocs_version = MOCS_TABLE_VERSION; >> > >Why are we stomping device->mocs_version to MOCS_TABLE_VERSION? Are you >just trying to avoid the version 0? If so, why not just have > >/* If the MOCS_TABLE_VERSION query fails, assume version 1 */ >if (device->mocs_version == 0) > device->mocs_version = 1; > I think the switch looks better, especially as the versions increase. >I don't think we want to have it dependent on a #define in an external >header file. What if someone updates it for i965 and doesn't update anv or >vice-versa? > > Yeah, I am removing that external define as mentioned in the other thread. I think it was a bad idea that I jammed in at the last minute. >> + } >> + } >> + >> brw_process_intel_debug_variable(); >> >> device->compiler = brw_compiler_create(NULL, &device->info); >> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_ >> private.h >> index 573778dad5..b8241a9b22 100644 >> --- a/src/intel/vulkan/anv_private.h >> +++ b/src/intel/vulkan/anv_private.h >> @@ -684,6 +684,8 @@ struct anv_physical_device { >> uint32_t eu_total; >> uint32_t subslice_total; >> >> + uint8_t mocs_version; >> + >> struct { >> uint32_t type_count; >> struct anv_memory_type >> types[VK_MAX_MEMORY_TYPES]; >> diff --git a/src/mesa/drivers/dri/i915/intel_context.c >> b/src/mesa/drivers/dri/i915/intel_context.c >> index e0766a0e3f..9169ea650e 100644 >> --- a/src/mesa/drivers/dri/i915/intel_context.c >> +++ b/src/mesa/drivers/dri/i915/intel_context.c >> @@ -521,8 +521,13 @@ intelInitContext(struct intel_context *intel, >> INTEL_DEBUG = parse_debug_string(getenv("INTEL_DEBUG"), >> debug_control); >> if (INTEL_DEBUG & DEBUG_BUFMGR) >> dri_bufmgr_set_debug(intel->bufmgr, true); >> - if (INTEL_DEBUG & DEBUG_PERF) >> + if (INTEL_DEBUG & DEBUG_PERF) { >> intel->perf_debug = true; >> + if (screen->mocs_version > MOCS_TABLE_VERSION) { >> + fprintf(stderr, "Kernel exposes newer MOCS table\n"); >> + screen->mocs_version = MOCS_TABLE_VERSION; >> + } >> + } >> >> if (INTEL_DEBUG & DEBUG_AUB) >> drm_intel_bufmgr_gem_set_aub_dump(intel->bufmgr, true); >> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c >> b/src/mesa/drivers/dri/i965/intel_screen.c >> index c75f2125d4..c53f133d49 100644 >> --- a/src/mesa/drivers/dri/i965/intel_screen.c >> +++ b/src/mesa/drivers/dri/i965/intel_screen.c >> @@ -2301,6 +2301,20 @@ __DRIconfig **intelInitScreen2(__DRIscreen >> *dri_screen) >> (ret != -1 || errno != EINVAL); >> } >> >> + if (devinfo->gen >= 9) { >> + screen->mocs_version = intel_get_integer(screen, >> + >> I915_PARAM_MOCS_TABLE_VERSION); >> + switch (screen->mocs_version) { >> + case 1: >> + case 0: >> + screen->mocs_version = MOCS_TABLE_VERSION; >> > >Same comments apply here. > > >> + break; >> + default: >> + /* We want to perf debug, but we can't yet */ >> + break; >> + } >> + } >> + >> dri_screen->extensions = !screen->has_context_reset_notification >> ? screenExtensions : intelRobustScreenExtensions; >> >> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h >> b/src/mesa/drivers/dri/i965/intel_screen.h >> index f78b3e8f74..eb801f8155 100644 >> --- a/src/mesa/drivers/dri/i965/intel_screen.h >> +++ b/src/mesa/drivers/dri/i965/intel_screen.h >> @@ -112,6 +112,8 @@ struct intel_screen >> bool mesa_format_supports_texture[MESA_FORMAT_COUNT]; >> bool mesa_format_supports_render[MESA_FORMAT_COUNT]; >> enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT]; >> + >> + unsigned mocs_version; >> }; >> >> extern void intelDestroyContext(__DRIcontext * driContextPriv); If you feel strongly about moving to if/else, I can do it. Let me know.
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c index 3dc55dbb8d..8e180dbf18 100644 --- a/src/intel/vulkan/anv_device.c +++ b/src/intel/vulkan/anv_device.c @@ -368,6 +368,18 @@ anv_physical_device_init(struct anv_physical_device *device, device->info.max_cs_threads = max_cs_threads; } + if (device->info.gen >= 9) { + device->mocs_version = anv_gem_get_param(fd, + I915_PARAM_MOCS_TABLE_VERSION); + switch (device->mocs_version) { + default: + anv_perf_warn("Kernel exposes newer MOCS table\n"); + case 1: + case 0: + device->mocs_version = MOCS_TABLE_VERSION; + } + } + brw_process_intel_debug_variable(); device->compiler = brw_compiler_create(NULL, &device->info); diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 573778dad5..b8241a9b22 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -684,6 +684,8 @@ struct anv_physical_device { uint32_t eu_total; uint32_t subslice_total; + uint8_t mocs_version; + struct { uint32_t type_count; struct anv_memory_type types[VK_MAX_MEMORY_TYPES]; diff --git a/src/mesa/drivers/dri/i915/intel_context.c b/src/mesa/drivers/dri/i915/intel_context.c index e0766a0e3f..9169ea650e 100644 --- a/src/mesa/drivers/dri/i915/intel_context.c +++ b/src/mesa/drivers/dri/i915/intel_context.c @@ -521,8 +521,13 @@ intelInitContext(struct intel_context *intel, INTEL_DEBUG = parse_debug_string(getenv("INTEL_DEBUG"), debug_control); if (INTEL_DEBUG & DEBUG_BUFMGR) dri_bufmgr_set_debug(intel->bufmgr, true); - if (INTEL_DEBUG & DEBUG_PERF) + if (INTEL_DEBUG & DEBUG_PERF) { intel->perf_debug = true; + if (screen->mocs_version > MOCS_TABLE_VERSION) { + fprintf(stderr, "Kernel exposes newer MOCS table\n"); + screen->mocs_version = MOCS_TABLE_VERSION; + } + } if (INTEL_DEBUG & DEBUG_AUB) drm_intel_bufmgr_gem_set_aub_dump(intel->bufmgr, true); diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index c75f2125d4..c53f133d49 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -2301,6 +2301,20 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen) (ret != -1 || errno != EINVAL); } + if (devinfo->gen >= 9) { + screen->mocs_version = intel_get_integer(screen, + I915_PARAM_MOCS_TABLE_VERSION); + switch (screen->mocs_version) { + case 1: + case 0: + screen->mocs_version = MOCS_TABLE_VERSION; + break; + default: + /* We want to perf debug, but we can't yet */ + break; + } + } + dri_screen->extensions = !screen->has_context_reset_notification ? screenExtensions : intelRobustScreenExtensions; diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h index f78b3e8f74..eb801f8155 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.h +++ b/src/mesa/drivers/dri/i965/intel_screen.h @@ -112,6 +112,8 @@ struct intel_screen bool mesa_format_supports_texture[MESA_FORMAT_COUNT]; bool mesa_format_supports_render[MESA_FORMAT_COUNT]; enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT]; + + unsigned mocs_version; }; extern void intelDestroyContext(__DRIcontext * driContextPriv);
We don't yet have optimal MOCS settings, but we have enough to know how to at least determine when we might have non-optimal settings within our driver. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- src/intel/vulkan/anv_device.c | 12 ++++++++++++ src/intel/vulkan/anv_private.h | 2 ++ src/mesa/drivers/dri/i915/intel_context.c | 7 ++++++- src/mesa/drivers/dri/i965/intel_screen.c | 14 ++++++++++++++ src/mesa/drivers/dri/i965/intel_screen.h | 2 ++ 5 files changed, 36 insertions(+), 1 deletion(-)