[Intel-gfx] [PATCH] drm/i915: Handle runtime pm in the CRC setup code

[Damien Lespiau]

On Tue, Nov 25, 2014 at 02:08:10PM +0100, Daniel Vetter wrote:
> The crc code doesn't handle anything really that could drop the
> register state (by design so that we have less complexity). Which
> means userspace may only start crc capture once the pipe is fully set
> up.
> 
> With an i-g-t patch this will be the case, but there's still the
> problem that this results in obscure unclaimed register write
> failures. Which is a pain to debug.
> 
> So instead make sure we don't have the basic unclaimed register write
> failure by grabbing runtime pm references. And reject completely
> invalid requests with -EIO. This is still racy of course, but for a
> test library we don't really care - if userspace shuts down the pipe
> right afterwards the entire setup will be lost anyway.
> 
> v2: Put instead of get, spotted by Damien. Also explain the runtime pm
> dance.
> 
> v3: There's really no need for rpm get/put since power_is_enabled only
> checks software state (Damien).
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=86092
> Cc: Damien Lespiau <damien.lespiau at intel.com>
> Tested-by: lu hua <huax.lu at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau at intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index b50458360a2d..1da809fcc1df 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3344,6 +3344,11 @@ 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))) {
> +		DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
> +		return -EIO;
> +	}
> +
>  	if (IS_GEN2(dev))
>  		ret = i8xx_pipe_crc_ctl_reg(&source, &val);
>  	else if (INTEL_INFO(dev)->gen < 5)
> -- 
> 2.1.1
>