[Intel-gfx] [PATCH] drm/i915: disable IPS while getting the pipe CRCs.

[Daniel Vetter]

On Fri, Oct 17, 2014 at 06:42:03PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> For some yet-undiscovered reason, when IPS gets enabled, the pipe CRC
> changes. Since hsw_enable_ips() doesn't really guarantees to enable
> IPS (it depends on package C-states), we can't really predict if IPS
> is enabled or disabled while running our CRC tests, so let's just
> completely disable IPS while pipe CRCs are being used.
> 
> If we find a way to make IPS not change the pipe CRC result, we may
> want to fix IPS and then revert this patch. While this doesn't happen,
> let's merge this patch, so every IGT test relying on the CRCs can
> work on pipe A.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72864
> Testcase: igt/kms_cursor_crc (and others)
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Apparently we don't yet fully understand what's going on. But while the hw
ppl chase this, let's make the testcases work again. I think your little
hack here is a bit too ugly though, especially in the brave new world
where we have atomic updates. I'm thinking of something similar like we
have for the hsw pfit wa. We'd need that for the sprite ips workaround
that we have already too.

But that flag should be in the plane config, so right now we can't really
do that. So I've merged your patch as a stopgap.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index da4036d..cd1bc8f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3255,6 +3255,8 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	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));
>  	u32 val = 0; /* shut up gcc */
>  	int ret;
>  
> @@ -3290,6 +3292,14 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  		if (!pipe_crc->entries)
>  			return -ENOMEM;
>  
> +		/*
> +		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
> +		 * enabled and disabled dynamically based on package C states,
> +		 * user space can't make reliable use of the CRCs, so let's just
> +		 * completely disable it.
> +		 */
> +		hsw_disable_ips(crtc);
> +
>  		spin_lock_irq(&pipe_crc->lock);
>  		pipe_crc->head = 0;
>  		pipe_crc->tail = 0;
> @@ -3328,6 +3338,8 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  			vlv_undo_pipe_scramble_reset(dev, pipe);
>  		else if (IS_HASWELL(dev) && pipe == PIPE_A)
>  			hsw_undo_trans_edp_pipe_A_crc_wa(dev);
> +
> +		hsw_enable_ips(crtc);
>  	}
>  
>  	return 0;
> -- 
> 2.1.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch