Re: [PATCH 06/14] drm/i915/perf: rework mux configurations queries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> ---
>  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 ?

Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>

>  			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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux