[Intel-gfx] [PATCH 06/14] drm/i915/perf: rework mux configurations queries
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Mon May 22 17:50:44 UTC 2017
On 22/05/17 17:51, Matthew Auld wrote:
> On 05/17, Lionel Landwerlin wrote:
>> Gen8+ might have mux configurations per slices/subslices. Depending on
>> whether slices/subslices have been fused off, only part of the
>> configuration needs to be applied. This change reworks the mux
>> configurations query mechanism to allow more than one set of registers
>> to be programmed.
> So the previous behaviour of applying only one mux config depending on
> the slices/sublices was wrong? Since now we seem to concatenate the mux
> configs.
Yes that was incorrect previously.
>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 5 +-
>> drivers/gpu/drm/i915/i915_oa_hsw.c | 211 ++++++++++++++++++++++++-------------
>> drivers/gpu/drm/i915/i915_perf.c | 7 +-
>> 3 files changed, 144 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 18e12e61949b..56ed5a0651e2 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2353,8 +2353,9 @@ struct drm_i915_private {
>>
>> int metrics_set;
>>
>> - const struct i915_oa_reg *mux_regs;
>> - int mux_regs_len;
>> + const struct i915_oa_reg *mux_regs[1];
>> + int mux_regs_lens[1];
>> + int n_mux_regs;
> So this is more like n_mux_configs ?
Sure, thanks for looking through those patches again :)
>
> Reviewed-by: Matthew Auld <matthew.auld at intel.com>
>
>> const struct i915_oa_reg *b_counter_regs;
>> int b_counter_regs_len;
>>
>> diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c b/drivers/gpu/drm/i915/i915_oa_hsw.c
>> index 4ddf756add31..ccd6e5124992 100644
>> --- a/drivers/gpu/drm/i915/i915_oa_hsw.c
>> +++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
>> @@ -109,12 +109,21 @@ static const struct i915_oa_reg mux_config_render_basic[] = {
>> { _MMIO(0x25428), 0x00042049 },
>> };
>>
>> -static const struct i915_oa_reg *
>> +static int
>> get_render_basic_mux_config(struct drm_i915_private *dev_priv,
>> - int *len)
>> + const struct i915_oa_reg **regs,
>> + int *lens)
>> {
>> - *len = ARRAY_SIZE(mux_config_render_basic);
>> - return mux_config_render_basic;
>> + int n = 0;
>> +
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs) < 1);
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens) < 1);
>> +
>> + regs[n] = mux_config_render_basic;
>> + lens[n] = ARRAY_SIZE(mux_config_render_basic);
>> + n++;
>> +
>> + return n;
>> }
>>
>> static const struct i915_oa_reg b_counter_config_compute_basic[] = {
>> @@ -172,12 +181,21 @@ static const struct i915_oa_reg mux_config_compute_basic[] = {
>> { _MMIO(0x25428), 0x00000c03 },
>> };
>>
>> -static const struct i915_oa_reg *
>> +static int
>> get_compute_basic_mux_config(struct drm_i915_private *dev_priv,
>> - int *len)
>> + const struct i915_oa_reg **regs,
>> + int *lens)
>> {
>> - *len = ARRAY_SIZE(mux_config_compute_basic);
>> - return mux_config_compute_basic;
>> + int n = 0;
>> +
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs) < 1);
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens) < 1);
>> +
>> + regs[n] = mux_config_compute_basic;
>> + lens[n] = ARRAY_SIZE(mux_config_compute_basic);
>> + n++;
>> +
>> + return n;
>> }
>>
>> static const struct i915_oa_reg b_counter_config_compute_extended[] = {
>> @@ -221,12 +239,21 @@ static const struct i915_oa_reg mux_config_compute_extended[] = {
>> { _MMIO(0x25428), 0x00000000 },
>> };
>>
>> -static const struct i915_oa_reg *
>> +static int
>> get_compute_extended_mux_config(struct drm_i915_private *dev_priv,
>> - int *len)
>> + const struct i915_oa_reg **regs,
>> + int *lens)
>> {
>> - *len = ARRAY_SIZE(mux_config_compute_extended);
>> - return mux_config_compute_extended;
>> + int n = 0;
>> +
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs) < 1);
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens) < 1);
>> +
>> + regs[n] = mux_config_compute_extended;
>> + lens[n] = ARRAY_SIZE(mux_config_compute_extended);
>> + n++;
>> +
>> + return n;
>> }
>>
>> static const struct i915_oa_reg b_counter_config_memory_reads[] = {
>> @@ -281,12 +308,21 @@ static const struct i915_oa_reg mux_config_memory_reads[] = {
>> { _MMIO(0x25428), 0x00000000 },
>> };
>>
>> -static const struct i915_oa_reg *
>> +static int
>> get_memory_reads_mux_config(struct drm_i915_private *dev_priv,
>> - int *len)
>> + const struct i915_oa_reg **regs,
>> + int *lens)
>> {
>> - *len = ARRAY_SIZE(mux_config_memory_reads);
>> - return mux_config_memory_reads;
>> + int n = 0;
>> +
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs) < 1);
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens) < 1);
>> +
>> + regs[n] = mux_config_memory_reads;
>> + lens[n] = ARRAY_SIZE(mux_config_memory_reads);
>> + n++;
>> +
>> + return n;
>> }
>>
>> static const struct i915_oa_reg b_counter_config_memory_writes[] = {
>> @@ -341,12 +377,21 @@ static const struct i915_oa_reg mux_config_memory_writes[] = {
>> { _MMIO(0x25428), 0x00000000 },
>> };
>>
>> -static const struct i915_oa_reg *
>> +static int
>> get_memory_writes_mux_config(struct drm_i915_private *dev_priv,
>> - int *len)
>> + const struct i915_oa_reg **regs,
>> + int *lens)
>> {
>> - *len = ARRAY_SIZE(mux_config_memory_writes);
>> - return mux_config_memory_writes;
>> + int n = 0;
>> +
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs) < 1);
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens) < 1);
>> +
>> + regs[n] = mux_config_memory_writes;
>> + lens[n] = ARRAY_SIZE(mux_config_memory_writes);
>> + n++;
>> +
>> + return n;
>> }
>>
>> static const struct i915_oa_reg b_counter_config_sampler_balance[] = {
>> @@ -401,31 +446,40 @@ static const struct i915_oa_reg mux_config_sampler_balance[] = {
>> { _MMIO(0x25428), 0x0004a54a },
>> };
>>
>> -static const struct i915_oa_reg *
>> +static int
>> get_sampler_balance_mux_config(struct drm_i915_private *dev_priv,
>> - int *len)
>> + const struct i915_oa_reg **regs,
>> + int *lens)
>> {
>> - *len = ARRAY_SIZE(mux_config_sampler_balance);
>> - return mux_config_sampler_balance;
>> + int n = 0;
>> +
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs) < 1);
>> + BUILD_BUG_ON(ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens) < 1);
>> +
>> + regs[n] = mux_config_sampler_balance;
>> + lens[n] = ARRAY_SIZE(mux_config_sampler_balance);
>> + n++;
>> +
>> + return n;
>> }
>>
>> int i915_oa_select_metric_set_hsw(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.n_mux_regs = 0;
>> dev_priv->perf.oa.b_counter_regs = NULL;
>> dev_priv->perf.oa.b_counter_regs_len = 0;
>>
>> switch (dev_priv->perf.oa.metrics_set) {
>> case METRIC_SET_ID_RENDER_BASIC:
>> - dev_priv->perf.oa.mux_regs =
>> + dev_priv->perf.oa.n_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");
>> + dev_priv->perf.oa.mux_regs,
>> + dev_priv->perf.oa.mux_regs_lens);
>> + if (dev_priv->perf.oa.n_mux_regs == 0) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for \"RENDER_BASIC\" metric set\n");
>>
>> /* EINVAL because *_register_sysfs already checked this
>> - * and so it wouldn't have been advertised so userspace and
>> + * and so it wouldn't have been advertised to userspace and
>> * so shouldn't have been requested
>> */
>> return -EINVAL;
>> @@ -438,14 +492,15 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private *dev_priv)
>>
>> return 0;
>> case METRIC_SET_ID_COMPUTE_BASIC:
>> - dev_priv->perf.oa.mux_regs =
>> + dev_priv->perf.oa.n_mux_regs =
>> get_compute_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 \"COMPUTE_BASIC\" metric set");
>> + dev_priv->perf.oa.mux_regs,
>> + dev_priv->perf.oa.mux_regs_lens);
>> + if (dev_priv->perf.oa.n_mux_regs == 0) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for \"COMPUTE_BASIC\" metric set\n");
>>
>> /* EINVAL because *_register_sysfs already checked this
>> - * and so it wouldn't have been advertised so userspace and
>> + * and so it wouldn't have been advertised to userspace and
>> * so shouldn't have been requested
>> */
>> return -EINVAL;
>> @@ -458,14 +513,15 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private *dev_priv)
>>
>> return 0;
>> case METRIC_SET_ID_COMPUTE_EXTENDED:
>> - dev_priv->perf.oa.mux_regs =
>> + dev_priv->perf.oa.n_mux_regs =
>> get_compute_extended_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 \"COMPUTE_EXTENDED\" metric set");
>> + dev_priv->perf.oa.mux_regs,
>> + dev_priv->perf.oa.mux_regs_lens);
>> + if (dev_priv->perf.oa.n_mux_regs == 0) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for \"COMPUTE_EXTENDED\" metric set\n");
>>
>> /* EINVAL because *_register_sysfs already checked this
>> - * and so it wouldn't have been advertised so userspace and
>> + * and so it wouldn't have been advertised to userspace and
>> * so shouldn't have been requested
>> */
>> return -EINVAL;
>> @@ -478,14 +534,15 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private *dev_priv)
>>
>> return 0;
>> case METRIC_SET_ID_MEMORY_READS:
>> - dev_priv->perf.oa.mux_regs =
>> + dev_priv->perf.oa.n_mux_regs =
>> get_memory_reads_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 \"MEMORY_READS\" metric set");
>> + dev_priv->perf.oa.mux_regs,
>> + dev_priv->perf.oa.mux_regs_lens);
>> + if (dev_priv->perf.oa.n_mux_regs == 0) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for \"MEMORY_READS\" metric set\n");
>>
>> /* EINVAL because *_register_sysfs already checked this
>> - * and so it wouldn't have been advertised so userspace and
>> + * and so it wouldn't have been advertised to userspace and
>> * so shouldn't have been requested
>> */
>> return -EINVAL;
>> @@ -498,14 +555,15 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private *dev_priv)
>>
>> return 0;
>> case METRIC_SET_ID_MEMORY_WRITES:
>> - dev_priv->perf.oa.mux_regs =
>> + dev_priv->perf.oa.n_mux_regs =
>> get_memory_writes_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 \"MEMORY_WRITES\" metric set");
>> + dev_priv->perf.oa.mux_regs,
>> + dev_priv->perf.oa.mux_regs_lens);
>> + if (dev_priv->perf.oa.n_mux_regs == 0) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for \"MEMORY_WRITES\" metric set\n");
>>
>> /* EINVAL because *_register_sysfs already checked this
>> - * and so it wouldn't have been advertised so userspace and
>> + * and so it wouldn't have been advertised to userspace and
>> * so shouldn't have been requested
>> */
>> return -EINVAL;
>> @@ -518,14 +576,15 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private *dev_priv)
>>
>> return 0;
>> case METRIC_SET_ID_SAMPLER_BALANCE:
>> - dev_priv->perf.oa.mux_regs =
>> + dev_priv->perf.oa.n_mux_regs =
>> get_sampler_balance_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 \"SAMPLER_BALANCE\" metric set");
>> + dev_priv->perf.oa.mux_regs,
>> + dev_priv->perf.oa.mux_regs_lens);
>> + if (dev_priv->perf.oa.n_mux_regs == 0) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for \"SAMPLER_BALANCE\" metric set\n");
>>
>> /* EINVAL because *_register_sysfs already checked this
>> - * and so it wouldn't have been advertised so userspace and
>> + * and so it wouldn't have been advertised to userspace and
>> * so shouldn't have been requested
>> */
>> return -EINVAL;
>> @@ -677,35 +736,36 @@ static struct attribute_group group_sampler_balance = {
>> int
>> i915_perf_register_sysfs_hsw(struct drm_i915_private *dev_priv)
>> {
>> - int mux_len;
>> + const struct i915_oa_reg *mux_regs[ARRAY_SIZE(dev_priv->perf.oa.mux_regs)];
>> + int mux_lens[ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens)];
>> int ret = 0;
>>
>> - if (get_render_basic_mux_config(dev_priv, &mux_len)) {
>> + if (get_render_basic_mux_config(dev_priv, mux_regs, mux_lens)) {
>> ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_render_basic);
>> if (ret)
>> goto error_render_basic;
>> }
>> - if (get_compute_basic_mux_config(dev_priv, &mux_len)) {
>> + if (get_compute_basic_mux_config(dev_priv, mux_regs, mux_lens)) {
>> ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_compute_basic);
>> if (ret)
>> goto error_compute_basic;
>> }
>> - if (get_compute_extended_mux_config(dev_priv, &mux_len)) {
>> + if (get_compute_extended_mux_config(dev_priv, mux_regs, mux_lens)) {
>> ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_compute_extended);
>> if (ret)
>> goto error_compute_extended;
>> }
>> - if (get_memory_reads_mux_config(dev_priv, &mux_len)) {
>> + if (get_memory_reads_mux_config(dev_priv, mux_regs, mux_lens)) {
>> ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_memory_reads);
>> if (ret)
>> goto error_memory_reads;
>> }
>> - if (get_memory_writes_mux_config(dev_priv, &mux_len)) {
>> + if (get_memory_writes_mux_config(dev_priv, mux_regs, mux_lens)) {
>> ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_memory_writes);
>> if (ret)
>> goto error_memory_writes;
>> }
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len)) {
>> + if (get_sampler_balance_mux_config(dev_priv, mux_regs, mux_lens)) {
>> ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_sampler_balance);
>> if (ret)
>> goto error_sampler_balance;
>> @@ -714,19 +774,19 @@ i915_perf_register_sysfs_hsw(struct drm_i915_private *dev_priv)
>> return 0;
>>
>> error_sampler_balance:
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len))
>> + if (get_memory_writes_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_memory_writes);
>> error_memory_writes:
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len))
>> + if (get_memory_reads_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_memory_reads);
>> error_memory_reads:
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len))
>> + if (get_compute_extended_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_compute_extended);
>> error_compute_extended:
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len))
>> + if (get_compute_basic_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_compute_basic);
>> error_compute_basic:
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len))
>> + if (get_render_basic_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_render_basic);
>> error_render_basic:
>> return ret;
>> @@ -735,18 +795,19 @@ i915_perf_register_sysfs_hsw(struct drm_i915_private *dev_priv)
>> void
>> i915_perf_unregister_sysfs_hsw(struct drm_i915_private *dev_priv)
>> {
>> - int mux_len;
>> + const struct i915_oa_reg *mux_regs[ARRAY_SIZE(dev_priv->perf.oa.mux_regs)];
>> + int mux_lens[ARRAY_SIZE(dev_priv->perf.oa.mux_regs_lens)];
>>
>> - if (get_render_basic_mux_config(dev_priv, &mux_len))
>> + if (get_render_basic_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_render_basic);
>> - if (get_compute_basic_mux_config(dev_priv, &mux_len))
>> + if (get_compute_basic_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_compute_basic);
>> - if (get_compute_extended_mux_config(dev_priv, &mux_len))
>> + if (get_compute_extended_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_compute_extended);
>> - if (get_memory_reads_mux_config(dev_priv, &mux_len))
>> + if (get_memory_reads_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_memory_reads);
>> - if (get_memory_writes_mux_config(dev_priv, &mux_len))
>> + if (get_memory_writes_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_memory_writes);
>> - if (get_sampler_balance_mux_config(dev_priv, &mux_len))
>> + if (get_sampler_balance_mux_config(dev_priv, mux_regs, mux_lens))
>> sysfs_remove_group(dev_priv->perf.metrics_kobj, &group_sampler_balance);
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index d20cceef93d0..2ba0bd36b40c 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1049,6 +1049,7 @@ static void config_oa_regs(struct drm_i915_private *dev_priv,
>> static int hsw_enable_metric_set(struct drm_i915_private *dev_priv)
>> {
>> int ret = i915_oa_select_metric_set_hsw(dev_priv);
>> + int i;
>>
>> if (ret)
>> return ret;
>> @@ -1070,8 +1071,10 @@ static int hsw_enable_metric_set(struct drm_i915_private *dev_priv)
>> I915_WRITE(GEN6_UCGCTL1, (I915_READ(GEN6_UCGCTL1) |
>> GEN6_CSUNIT_CLOCK_GATE_DISABLE));
>>
>> - config_oa_regs(dev_priv, dev_priv->perf.oa.mux_regs,
>> - dev_priv->perf.oa.mux_regs_len);
>> + for (i = 0; i < dev_priv->perf.oa.n_mux_regs; i++) {
>> + config_oa_regs(dev_priv, dev_priv->perf.oa.mux_regs[i],
>> + dev_priv->perf.oa.mux_regs_lens[i]);
>> + }
>>
>> /* It apparently takes a fairly long time for a new MUX
>> * configuration to be be applied after these register writes.
>> --
>> 2.11.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list