[PATCH] drm/i915: Enable vblank interrupts for CRC generation

[PATCH] drm/i915: Enable vblank interrupts for CRC generation

[Chris Wilson]

Pineview requires this. But this changes the debug API...

References: https://bugs.freedesktop.org/show_bug.cgi?id=82280
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d76a298a0eb7..65024a9bc95f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2713,10 +2713,15 @@ static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
 	struct pipe_crc_info *info = inode->i_private;
 	struct drm_i915_private *dev_priv = info->dev->dev_private;
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
+	int ret;
 
 	if (info->pipe >= INTEL_INFO(info->dev)->num_pipes)
 		return -ENODEV;
 
+	ret = drm_vblank_get(dev_priv->dev, info->pipe);
+	if (ret)
+		return ret;
+
 	spin_lock_irq(&pipe_crc->lock);
 
 	if (pipe_crc->opened) {
@@ -2742,6 +2747,8 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
 	pipe_crc->opened = false;
 	spin_unlock_irq(&pipe_crc->lock);
 
+	drm_vblank_put(dev_priv->dev, info->pipe);
+
 	return 0;
 }
 
-- 
2.1.0

Re: [PATCH] drm/i915: Enable vblank interrupts for CRC generation

[Ville Syrjälä]

On Mon, Aug 25, 2014 at 01:28:11PM +0100, Chris Wilson wrote:
> Pineview requires this. But this changes the debug API...
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=82280

Isn't this just the same old igt_display bug that it doesn't check
that the pipe<->port combination is valid?

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d76a298a0eb7..65024a9bc95f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2713,10 +2713,15 @@ static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
>  	struct pipe_crc_info *info = inode->i_private;
>  	struct drm_i915_private *dev_priv = info->dev->dev_private;
>  	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
> +	int ret;
>  
>  	if (info->pipe >= INTEL_INFO(info->dev)->num_pipes)
>  		return -ENODEV;
>  
> +	ret = drm_vblank_get(dev_priv->dev, info->pipe);
> +	if (ret)
> +		return ret;
> +
>  	spin_lock_irq(&pipe_crc->lock);
>  
>  	if (pipe_crc->opened) {
> @@ -2742,6 +2747,8 @@ static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
>  	pipe_crc->opened = false;
>  	spin_unlock_irq(&pipe_crc->lock);
>  
> +	drm_vblank_put(dev_priv->dev, info->pipe);
> +
>  	return 0;
>  }
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Enable vblank interrupts for CRC generation

[Chris Wilson]

On Mon, Aug 25, 2014 at 04:24:56PM +0300, Ville Syrjälä wrote:
> On Mon, Aug 25, 2014 at 01:28:11PM +0100, Chris Wilson wrote:
> > Pineview requires this. But this changes the debug API...
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=82280
> 
> Isn't this just the same old igt_display bug that it doesn't check
> that the pipe<->port combination is valid?

It appeared to be both. I had to set it to use the right pipe for LVDS
and I had to enable interrupts for it to start reading back the CRC.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Enable vblank interrupts for CRC generation

[Daniel Vetter]

On Mon, Aug 25, 2014 at 02:47:17PM +0100, Chris Wilson wrote:
> On Mon, Aug 25, 2014 at 04:24:56PM +0300, Ville Syrjälä wrote:
> > On Mon, Aug 25, 2014 at 01:28:11PM +0100, Chris Wilson wrote:
> > > Pineview requires this. But this changes the debug API...
> > > 
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=82280
> > 
> > Isn't this just the same old igt_display bug that it doesn't check
> > that the pipe<->port combination is valid?
> 
> It appeared to be both. I had to set it to use the right pipe for LVDS
> and I had to enable interrupts for it to start reading back the CRC.

What does kms_pipe_crc_basic say on pnv? Otoh I've just tried to run that
and it completely falls over on my i945gm box, so that much for the state
of our test infrastructure.

If we can confirm that we need vblank interrupts to get the pipe crc
delivered I'd like to restrict this to just pnv (or whatever else is
affected) and add a comment explaining why we need vblank interrupts
enabled.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Enable vblank interrupts for CRC generation

[Chris Wilson]

On Wed, Aug 27, 2014 at 08:12:23AM +0200, Daniel Vetter wrote:
> On Mon, Aug 25, 2014 at 02:47:17PM +0100, Chris Wilson wrote:
> > On Mon, Aug 25, 2014 at 04:24:56PM +0300, Ville Syrjälä wrote:
> > > On Mon, Aug 25, 2014 at 01:28:11PM +0100, Chris Wilson wrote:
> > > > Pineview requires this. But this changes the debug API...
> > > > 
> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=82280
> > > 
> > > Isn't this just the same old igt_display bug that it doesn't check
> > > that the pipe<->port combination is valid?
> > 
> > It appeared to be both. I had to set it to use the right pipe for LVDS
> > and I had to enable interrupts for it to start reading back the CRC.
> 
> What does kms_pipe_crc_basic say on pnv? Otoh I've just tried to run that
> and it completely falls over on my i945gm box, so that much for the state
> of our test infrastructure.

Given both of your skepticism, I cleaned out my igt and tested afresh. It
works without the extra vblanks. Apologies for the noise.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre