[Intel-gfx] [PATCH 18/37] drm/exynos: Drop drm_vblank_cleanup

[Inki Dae]

2017년 05월 31일 17:45에 Daniel Vetter 이(가) 쓴 글:
> On Tue, May 30, 2017 at 09:03:34AM +0900, Inki Dae wrote:
>> Hi Daniel,
>>
>> 2017년 05월 24일 23:51에 Daniel Vetter 이(가) 쓴 글:
>>> Only in the load failure path, where the hardware is quiet anyway.
>>>
>>> Cc: Inki Dae <inki.dae at samsung.com>
>>> Cc: Joonyoung Shim <jy0922.shim at samsung.com>
>>> Cc: Seung-Woo Kim <sw0312.kim at samsung.com>
>>> Cc: Kyungmin Park <kyungmin.park at samsung.com>
>>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> index 50294a7bd29d..1c814b9342af 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> @@ -376,7 +376,7 @@ static int exynos_drm_bind(struct device *dev)
>>>  	/* Probe non kms sub drivers and virtual display driver. */
>>>  	ret = exynos_drm_device_subdrv_probe(drm);
>>>  	if (ret)
>>> -		goto err_cleanup_vblank;
>>> +		goto err_unbind_all;
>>
>> With this change shouldn't you post the patch to remove drm_vblank_init and setup vblank stuff in drm_crtc_init together?
>> I couldn't find the relevant patch on your patch series[1].
> 
> No, drm_vblank_cleanup is already called in the core. The only reason to
> call it in the driver is to fall back from kms to ums when irq setup
> somehow failed, then you need to disable vblank support again.
> 
> The only driver which ever needed this was radeon, and radeon long ago
> lost ums support. drm_vblank_cleanup is already called for you, and most
> drivers don't even do this cleanup call. But somehow a lot of people
> copied from radeon without understanding what it does.
> 
> Looking at the last patch in this series might help a bit in understanding
> the history here. Can you pls reevaluate the patch?


Thanks for explaination. Confirmed.

Reviewed-by: Inki Dae <inki.dae at samsung.com>

Thanks,
Inki Dae

> 
> Thanks, Daniel
> 
>> As of now, I think resource leak would happen with this patch only.
>>
>> Thanks,
>> Inki Dae
>>
>> [1] http://www.spinics.net/lists/dri-devel/msg142387.html
>>
>>>  
>>>  	drm_mode_config_reset(drm);
>>>  
>>> @@ -407,8 +407,6 @@ static int exynos_drm_bind(struct device *dev)
>>>  	exynos_drm_fbdev_fini(drm);
>>>  	drm_kms_helper_poll_fini(drm);
>>>  	exynos_drm_device_subdrv_remove(drm);
>>> -err_cleanup_vblank:
>>> -	drm_vblank_cleanup(drm);
>>>  err_unbind_all:
>>>  	component_unbind_all(drm->dev, drm);
>>>  err_mode_config_cleanup:
>>>
>