[PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

[PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

[Daniel Vetter]

Otherwise we incur an unsightly WARNING. The mutex locking is a bit
overkill, but it curbs races and eventially we might grow a locking
check in the vblank wait code to make sure the right crtc lock is
held.

This is fallout from

commit 9393707190194eb8b42e412b444a03331db6862f
Author:     Jesse Barnes <jbarnes@virtuousgeek.org>
AuthorDate: Fri Apr 4 16:12:09 2014 -0700

    drm/i915: warn when a vblank wait times out

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79612
Tested-by: Guo Jinxian <jinxianx.guo@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 601caa88c092..cf94e9640627 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2927,11 +2927,16 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 	/* real source -> none transition */
 	if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
 		struct intel_pipe_crc_entry *entries;
+		struct intel_crtc *crtc =
+			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
 		DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
 				 pipe_name(pipe));
 
-		intel_wait_for_vblank(dev, pipe);
+		mutex_lock(&crtc->base.mutex);
+		if (crtc->active)
+			intel_wait_for_vblank(dev, pipe);
+		mutex_unlock(&crtc->base.mutex);
 
 		spin_lock_irq(&pipe_crc->lock);
 		entries = pipe_crc->entries;
-- 
2.0.0

Re: [PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

[Ville Syrjälä]

On Fri, Jun 06, 2014 at 08:22:08AM +0200, Daniel Vetter wrote:
> Otherwise we incur an unsightly WARNING. The mutex locking is a bit
> overkill, but it curbs races and eventially we might grow a locking
> check in the vblank wait code to make sure the right crtc lock is
> held.
> 
> This is fallout from
> 
> commit 9393707190194eb8b42e412b444a03331db6862f
> Author:     Jesse Barnes <jbarnes@virtuousgeek.org>
> AuthorDate: Fri Apr 4 16:12:09 2014 -0700
> 
>     drm/i915: warn when a vblank wait times out
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79612
> Tested-by: Guo Jinxian <jinxianx.guo@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 601caa88c092..cf94e9640627 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2927,11 +2927,16 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  	/* real source -> none transition */
>  	if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
>  		struct intel_pipe_crc_entry *entries;
> +		struct intel_crtc *crtc =
> +			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
                        ^^^^^^

What's that?

Otherwise:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  		DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
>  				 pipe_name(pipe));
>  
> -		intel_wait_for_vblank(dev, pipe);
> +		mutex_lock(&crtc->base.mutex);
> +		if (crtc->active)
> +			intel_wait_for_vblank(dev, pipe);
> +		mutex_unlock(&crtc->base.mutex);
>  
>  		spin_lock_irq(&pipe_crc->lock);
>  		entries = pipe_crc->entries;
> -- 
> 2.0.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: Only wait one vblank when disabling crc if the pipe is on

[Daniel Vetter]

On Fri, Jun 06, 2014 at 11:57:10AM +0300, Ville Syrjälä wrote:
> On Fri, Jun 06, 2014 at 08:22:08AM +0200, Daniel Vetter wrote:
> > Otherwise we incur an unsightly WARNING. The mutex locking is a bit
> > overkill, but it curbs races and eventially we might grow a locking
> > check in the vblank wait code to make sure the right crtc lock is
> > held.
> > 
> > This is fallout from
> > 
> > commit 9393707190194eb8b42e412b444a03331db6862f
> > Author:     Jesse Barnes <jbarnes@virtuousgeek.org>
> > AuthorDate: Fri Apr 4 16:12:09 2014 -0700
> > 
> >     drm/i915: warn when a vblank wait times out
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79612
> > Tested-by: Guo Jinxian <jinxianx.guo@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 601caa88c092..cf94e9640627 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2927,11 +2927,16 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
> >  	/* real source -> none transition */
> >  	if (source == INTEL_PIPE_CRC_SOURCE_NONE) {
> >  		struct intel_pipe_crc_entry *entries;
> > +		struct intel_crtc *crtc =
> > +			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>                         ^^^^^^
> 
> What's that?

Oops. Luckily patch still worked, C rules are bendy enough ;-)

> 
> Otherwise:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks,
-Daniel

> 
> >  
> >  		DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
> >  				 pipe_name(pipe));
> >  
> > -		intel_wait_for_vblank(dev, pipe);
> > +		mutex_lock(&crtc->base.mutex);
> > +		if (crtc->active)
> > +			intel_wait_for_vblank(dev, pipe);
> > +		mutex_unlock(&crtc->base.mutex);
> >  
> >  		spin_lock_irq(&pipe_crc->lock);
> >  		entries = pipe_crc->entries;
> > -- 
> > 2.0.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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

Re: [PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

[Damien Lespiau]

On Fri, Jun 06, 2014 at 08:22:08AM +0200, Daniel Vetter wrote:
> +		mutex_lock(&crtc->base.mutex);
> +		if (crtc->active)
> +			intel_wait_for_vblank(dev, pipe);
> +		mutex_unlock(&crtc->base.mutex);

drivers/gpu/drm/i915/i915_debugfs.c: In function ‘pipe_crc_set_source’:
drivers/gpu/drm/i915/i915_debugfs.c:2932:3: warning: passing argument 1 of ‘mutex_lock’ from incompatible pointer type [enabled by default]
   mutex_lock(&crtc->base.mutex);
   ^
In file included from include/linux/seq_file.h:7:0,
                 from drivers/gpu/drm/i915/i915_debugfs.c:29:
include/linux/mutex.h:158:13: note: expected ‘struct mutex *’ but argument is of type ‘struct drm_modeset_lock *’
 extern void mutex_lock(struct mutex *lock);
             ^

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

[Daniel Vetter]

On Fri, Jun 06, 2014 at 06:18:01PM +0100, Damien Lespiau wrote:
> On Fri, Jun 06, 2014 at 08:22:08AM +0200, Daniel Vetter wrote:
> > +		mutex_lock(&crtc->base.mutex);
> > +		if (crtc->active)
> > +			intel_wait_for_vblank(dev, pipe);
> > +		mutex_unlock(&crtc->base.mutex);
> 
> drivers/gpu/drm/i915/i915_debugfs.c: In function ‘pipe_crc_set_source’:
> drivers/gpu/drm/i915/i915_debugfs.c:2932:3: warning: passing argument 1 of ‘mutex_lock’ from incompatible pointer type [enabled by default]
>    mutex_lock(&crtc->base.mutex);
>    ^
> In file included from include/linux/seq_file.h:7:0,
>                  from drivers/gpu/drm/i915/i915_debugfs.c:29:
> include/linux/mutex.h:158:13: note: expected ‘struct mutex *’ but argument is of type ‘struct drm_modeset_lock *’
>  extern void mutex_lock(struct mutex *lock);

Oh dang, rebase fail. Thanks for helping the blind, will fix asap.
-Daniel

>              ^
> 
> -- 
> Damien

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Only wait one vblank when disabling crc if the pipe is on

[Jani Nikula]

On Fri, 06 Jun 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jun 06, 2014 at 06:18:01PM +0100, Damien Lespiau wrote:
>> On Fri, Jun 06, 2014 at 08:22:08AM +0200, Daniel Vetter wrote:
>> > +		mutex_lock(&crtc->base.mutex);
>> > +		if (crtc->active)
>> > +			intel_wait_for_vblank(dev, pipe);
>> > +		mutex_unlock(&crtc->base.mutex);
>> 
>> drivers/gpu/drm/i915/i915_debugfs.c: In function ‘pipe_crc_set_source’:
>> drivers/gpu/drm/i915/i915_debugfs.c:2932:3: warning: passing argument 1 of ‘mutex_lock’ from incompatible pointer type [enabled by default]
>>    mutex_lock(&crtc->base.mutex);
>>    ^
>> In file included from include/linux/seq_file.h:7:0,
>>                  from drivers/gpu/drm/i915/i915_debugfs.c:29:
>> include/linux/mutex.h:158:13: note: expected ‘struct mutex *’ but argument is of type ‘struct drm_modeset_lock *’
>>  extern void mutex_lock(struct mutex *lock);
>
> Oh dang, rebase fail. Thanks for helping the blind, will fix asap.


drivers/gpu/drm/i915/i915_debugfs.c: In function ‘pipe_crc_set_source’:
drivers/gpu/drm/i915/i915_debugfs.c:2932:3: warning: passing argument 1 of ‘mutex_lock’ from incompatible pointer type [enabled by default]
   mutex_lock(&crtc->base.mutex);
   ^
In file included from include/linux/seq_file.h:7:0,
                 from drivers/gpu/drm/i915/i915_debugfs.c:29:
include/linux/mutex.h:158:13: note: expected ‘struct mutex *’ but argument is of type ‘struct drm_modeset_lock *’
 extern void mutex_lock(struct mutex *lock);
             ^
drivers/gpu/drm/i915/i915_debugfs.c:2935:3: warning: passing argument 1 of ‘mutex_unlock’ from incompatible pointer type [enabled by default]
   mutex_unlock(&crtc->base.mutex);
   ^
In file included from include/linux/seq_file.h:7:0,
                 from drivers/gpu/drm/i915/i915_debugfs.c:29:
include/linux/mutex.h:175:13: note: expected ‘struct mutex *’ but argument is of type ‘struct drm_modeset_lock *’
 extern void mutex_unlock(struct mutex *lock);
             ^


33b1879a1038d87d82cd4de82ebedd95ee88e529 is the first bad commit
commit 33b1879a1038d87d82cd4de82ebedd95ee88e529
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Jun 6 08:22:08 2014 +0200

    drm/i915: Only wait one vblank when disabling crc if the pipe is on
    
    Otherwise we incur an unsightly WARNING. The mutex locking is a bit
    overkill, but it curbs races and eventially we might grow a locking
    check in the vblank wait code to make sure the right crtc lock is
    held.
    
    This is fallout from
    
    commit 9393707190194eb8b42e412b444a03331db6862f
    Author:     Jesse Barnes <jbarnes@virtuousgeek.org>
    AuthorDate: Fri Apr 4 16:12:09 2014 -0700
    
        drm/i915: warn when a vblank wait times out
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79612
    Tested-by: Guo Jinxian <jinxianx.guo@intel.com>
    Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

:040000 040000 ec8ee719363b4eb6f1d90950605694bbe1bb2cf3 b3d5f21458cc6ba201e73a5effb003edadb644f5 M	drivers



BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx