[Intel-gfx] [PATCH 08/12] drm/i915: ensure the HW is powered when accessing the CRC HW block

[Daniel Vetter]

On Fri, Feb 12, 2016 at 06:55:17PM +0200, Imre Deak wrote:
> The assumption when adding the intel_display_power_is_enabled() checks
> was that if it returns success the power can't be turned off afterwards
> during the HW access, which is guaranteed by modeset locks. This isn't
> always true, so make sure we hold a dedicated reference for the time of
> the access.
> 
> Signed-off-by: Imre Deak <imre.deak at intel.com>

subject needs to be fixed to mention that this is for all the stuff in
i915_debugfs.c. With that fixed for patches 4-8:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>


> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ec0c2a05e..9e19cf0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -825,8 +825,11 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
>  		}
>  
>  		for_each_pipe(dev_priv, pipe) {
> -			if (!intel_display_power_is_enabled(dev_priv,
> -						POWER_DOMAIN_PIPE(pipe))) {
> +			enum intel_display_power_domain power_domain;
> +
> +			power_domain = POWER_DOMAIN_PIPE(pipe);
> +			if (!intel_display_power_get_if_enabled(dev_priv,
> +								power_domain)) {
>  				seq_printf(m, "Pipe %c power disabled\n",
>  					   pipe_name(pipe));
>  				continue;
> @@ -840,6 +843,8 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
>  			seq_printf(m, "Pipe %c IER:\t%08x\n",
>  				   pipe_name(pipe),
>  				   I915_READ(GEN8_DE_PIPE_IER(pipe)));
> +
> +			intel_display_power_put(dev_priv, power_domain);
>  		}
>  
>  		seq_printf(m, "Display Engine port interrupt mask:\t%08x\n",
> @@ -4004,6 +4009,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
>  	struct intel_crtc *crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev,
>  									pipe));
> +	enum intel_display_power_domain power_domain;
>  	u32 val = 0; /* shut up gcc */
>  	int ret;
>  
> @@ -4014,7 +4020,8 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  	if (pipe_crc->source && source)
>  		return -EINVAL;
>  
> -	if (!intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_PIPE(pipe))) {
> +	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>  		DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
>  		return -EIO;
>  	}
> @@ -4031,7 +4038,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  		ret = ivb_pipe_crc_ctl_reg(dev, pipe, &source, &val);
>  
>  	if (ret != 0)
> -		return ret;
> +		goto out;
>  
>  	/* none -> real source transition */
>  	if (source) {
> @@ -4043,8 +4050,10 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  		entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
>  				  sizeof(pipe_crc->entries[0]),
>  				  GFP_KERNEL);
> -		if (!entries)
> -			return -ENOMEM;
> +		if (!entries) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  
>  		/*
>  		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
> @@ -4100,7 +4109,12 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  		hsw_enable_ips(crtc);
>  	}
>  
> -	return 0;
> +	ret = 0;
> +
> +out:
> +	intel_display_power_put(dev_priv, power_domain);
> +
> +	return ret;
>  }
>  
>  /*
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch