On Fri, Jul 22, 2022 at 05:52:34PM -0400, Yunxiang Li wrote:
> Acquire vbl_lock before accessing vblank refcount in drm_vblank_put,
> just like everywhere else that access the refcount.
> 
> Signed-off-by: Yunxiang Li <yunxiang...@amd.com>

The entire point of using atomic for the refcount is that we can check it
lockless, so I'm not sure what you're trying to fix here?

For the first patch I think it's clear that the bug needs to be fixed in
amdgpu dc code already.
-Daniel
> ---
>  drivers/gpu/drm/drm_vblank.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 159d13b5d97b..77b8c40fc7ba 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1203,15 +1203,22 @@ EXPORT_SYMBOL(drm_crtc_vblank_get);
>  void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
>  {
>       struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> +     unsigned long irqflags;
> +     int ret;
>  
>       if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>               return;
>  
> -     if (drm_WARN_ON(dev, atomic_read(&vblank->refcount) == 0))
> +     spin_lock_irqsave(&dev->vbl_lock, irqflags);
> +     if (drm_WARN_ON(dev, atomic_read(&vblank->refcount) == 0)) {
> +             spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
>               return;
> +     }
>  
>       /* Last user schedules interrupt disable */
> -     if (atomic_dec_and_test(&vblank->refcount)) {
> +     ret = atomic_dec_and_test(&vblank->refcount);
> +     spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> +     if (ret) {
>               if (drm_vblank_offdelay == 0)
>                       return;
>               else if (drm_vblank_offdelay < 0)
> -- 
> 2.37.1
> 

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

Reply via email to