Re: [PATCH 2/2] drm: get lock before accessing vblank refcount

[Daniel Vetter <daniel@ffwll.ch>]

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.Li@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