On Wed, Mar 1, 2017 at 1:00 PM, Matthew Auld <matthew.william.auld@xxxxxxxxx> wrote: > On 02/22, Robert Bragg wrote: >> These are auto generated from an XML description of metric sets, >> currently maintained in gputop, ref: >> >> https://github.com/rib/gputop >> > gputop-data/oa-*.xml >> > scripts/i915-perf-kernelgen.py >> >> $ make -C gputop-data -f Makefile.xml >> >> Signed-off-by: Robert Bragg <robert@xxxxxxxxxxxxx> >> --- > > <SNIP> > >> + >> +static const struct i915_oa_reg * >> +get_compute_extended_mux_config(struct drm_i915_private *dev_priv, >> + int *len) >> +{ >> + if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x01) { >> + *len = ARRAY_SIZE(mux_config_compute_extended_1_0_subslices_0x01); >> + return mux_config_compute_extended_1_0_subslices_0x01; >> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x08) { >> + *len = ARRAY_SIZE(mux_config_compute_extended_1_1_subslices_0x08); >> + return mux_config_compute_extended_1_1_subslices_0x08; >> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x02) { >> + *len = ARRAY_SIZE(mux_config_compute_extended_1_2_subslices_0x02); >> + return mux_config_compute_extended_1_2_subslices_0x02; >> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x10) { >> + *len = ARRAY_SIZE(mux_config_compute_extended_1_3_subslices_0x10); >> + return mux_config_compute_extended_1_3_subslices_0x10; >> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x04) { >> + *len = ARRAY_SIZE(mux_config_compute_extended_1_4_subslices_0x04); >> + return mux_config_compute_extended_1_4_subslices_0x04; >> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x20) { >> + *len = ARRAY_SIZE(mux_config_compute_extended_1_5_subslices_0x20); >> + return mux_config_compute_extended_1_5_subslices_0x20; >> + *len = ARRAY_SIZE(mux_config_compute_extended); >> + return mux_config_compute_extended; > It looks like your script doesn't properly handle the unconditional mux > config here. Very well spotted! This is a pretty ambiguous case and I'm not entirely sure what the right thing to do here is and have asked the folks that came up with the original OA config for their feedback. I *think* in this case it's supposed to combine both as if the unconditional config were logically concatenated at the end of all the conditional configs. Locally I've at least updated the i915-perf-kernelgen.py script to print a loud warning out for this so I could double check in case there were other similar cases I'd missed - this BDW config seems to be the only one with this ambiguity though. This makes me think that I should update the mdapi-xml-convert.py script to just hide this special case from our oa-*.xml file by explicitly concatenating with the conditional configs if that's appropriate - then we don't need to support multiple NOA configs in the i915 perf implementation. The scripts can then have strict assertions that this doesn't change in the future. > > <SNIP> > >> + >> +static const struct i915_oa_reg * >> +get_test_oa_mux_config(struct drm_i915_private *dev_priv, >> + int *len) >> +{ >> + *len = ARRAY_SIZE(mux_config_test_oa); >> + return mux_config_test_oa; >> +} >> + >> +int i915_oa_select_metric_set_bdw(struct drm_i915_private *dev_priv) >> +{ >> + dev_priv->perf.oa.mux_regs = NULL; >> + dev_priv->perf.oa.mux_regs_len = 0; >> + dev_priv->perf.oa.b_counter_regs = NULL; >> + dev_priv->perf.oa.b_counter_regs_len = 0; >> + dev_priv->perf.oa.flex_regs = NULL; >> + dev_priv->perf.oa.flex_regs_len = 0; >> + >> + switch (dev_priv->perf.oa.metrics_set) { >> + case METRIC_SET_ID_RENDER_BASIC: >> + dev_priv->perf.oa.mux_regs = >> + get_render_basic_mux_config(dev_priv, >> + &dev_priv->perf.oa.mux_regs_len); >> + if (!dev_priv->perf.oa.mux_regs) { >> + DRM_DEBUG_DRIVER("No suitable MUX config for \"RENDER_BASIC\" metric set"); > Your script also needs to output a newline for DRM_DEBUG_DRIVER. Ah yep, fixed locally thanks, > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
