Re: [PATCH v3 2/4] drm/i915/perf: remove redundant oa buffer initialization

From: Lucas De Marchi <lucas.de.marchi@gmail.com>
To: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 2/4] drm/i915/perf: remove redundant oa buffer initialization
Date: Mon, 15 Oct 2018 11:48:46 -0700	[thread overview]
Message-ID: <20181015184844.GB4402@ldmartin-desk.jf.intel.com> (raw)
In-Reply-To: <20181015155959.28038-3-lionel.g.landwerlin@intel.com>

On Mon, Oct 15, 2018 at 04:59:57PM +0100, Lionel Landwerlin wrote:
> We initialize the OA buffer everytime we enable the OA unit (first call in
> gen[78]_oa_enable), so we don't need to initialize when preparing the metric
> set.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 17 -----------------
>  drivers/gpu/drm/i915/i915_perf.c |  6 +-----
>  2 files changed, 1 insertion(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 63ce0da4e723..eef7c811bd8f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1529,23 +1529,6 @@ struct i915_oa_ops {
>  	 */
>  	bool (*is_valid_flex_reg)(struct drm_i915_private *dev_priv, u32 addr);
>  
> -	/**
> -	 * @init_oa_buffer: Resets the head and tail pointers of the
> -	 * circular buffer for periodic OA reports.
> -	 *
> -	 * Called when first opening a stream for OA metrics, but also may be
> -	 * called in response to an OA buffer overflow or other error
> -	 * condition.
> -	 *
> -	 * Note it may be necessary to clear the full OA buffer here as part of
> -	 * maintaining the invariable that new reports must be written to
> -	 * zeroed memory for us to be able to reliable detect if an expected
> -	 * report has not yet landed in memory.  (At least on Haswell the OA
> -	 * buffer tail pointer is not synchronized with reports being visible
> -	 * to the CPU)
> -	 */
> -	void (*init_oa_buffer)(struct drm_i915_private *dev_priv);
> -
>  	/**
>  	 * @enable_metric_set: Selects and applies any MUX configuration to set
>  	 * up the Boolean and Custom (B/C) counters that are part of the
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 30911efd2cf7..14f7d03aabcf 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1530,8 +1530,6 @@ static int alloc_oa_buffer(struct drm_i915_private *dev_priv)
>  		goto err_unpin;
>  	}
>  
> -	dev_priv->perf.oa.ops.init_oa_buffer(dev_priv);
> -
>  	DRM_DEBUG_DRIVER("OA Buffer initialized, gtt offset = 0x%x, vaddr = %p\n",
>  			 i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma),
>  			 dev_priv->perf.oa.oa_buffer.vaddr);
> @@ -2000,7 +1998,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>  		return -EINVAL;
>  	}
>  
> -	if (!dev_priv->perf.oa.ops.init_oa_buffer) {
> +	if (!dev_priv->perf.oa.ops.enable_metric_set) {
>  		DRM_DEBUG("OA unit not supported\n");
>  		return -ENODEV;
>  	}
> @@ -3389,7 +3387,6 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
>  		dev_priv->perf.oa.ops.is_valid_mux_reg =
>  			hsw_is_valid_mux_addr;
>  		dev_priv->perf.oa.ops.is_valid_flex_reg = NULL;
> -		dev_priv->perf.oa.ops.init_oa_buffer = gen7_init_oa_buffer;
>  		dev_priv->perf.oa.ops.enable_metric_set = hsw_enable_metric_set;
>  		dev_priv->perf.oa.ops.disable_metric_set = hsw_disable_metric_set;
>  		dev_priv->perf.oa.ops.oa_enable = gen7_oa_enable;
> @@ -3408,7 +3405,6 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
>  		 */
>  		dev_priv->perf.oa.oa_formats = gen8_plus_oa_formats;
>  
> -		dev_priv->perf.oa.ops.init_oa_buffer = gen8_init_oa_buffer;
>  		dev_priv->perf.oa.ops.oa_enable = gen8_oa_enable;
>  		dev_priv->perf.oa.ops.oa_disable = gen8_oa_disable;
>  		dev_priv->perf.oa.ops.read = gen8_oa_read;
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx