Re: [PATCH v2 12/23] drm/exynos: don't track enabled state at exynos_crtc

[Joonyoung Shim <jy0922.shim@samsung.com>]

+Cc Andrzej,

On 07/07/2015 02:41 AM, Daniel Vetter wrote:
> On Mon, Jul 06, 2015 at 11:20:13AM -0300, Gustavo Padovan wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>
>> struct drm_crtc already stores the enabled state of the crtc
>> thus we don't need to replicate enabled in exynos_drm_crtc.
>>

I think exynos_crtc->enabled can replace flags for power state of each
hw driver(e.g. "powered" of mixer driver, "suspended" of fimd driver).
Further, we can add other flag bit for instead of using special flag
in hw driver like vsync state("int_en" of mixer driver, "irq_flags" of
fimd driver.)

>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Note that exynos_crtc->enabled doesn't match drm_crtc->enabled perfectly
> since the exynos one reflect hw state (including dpms). You want to look
> at crtc->state->active instead on functions enabling stuff, and
> old_crtc_state->active on functions disabling stuff (we don't wire that
> through everywhere yet).
> -Daniel
> 
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 16 ----------------
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  1 -
>>  2 files changed, 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> index 9bc2353..5ab8972 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> @@ -26,14 +26,9 @@ static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
>>  {
>>  	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
>>  
>> -	if (exynos_crtc->enabled)
>> -		return;
>> -
>>  	if (exynos_crtc->ops->enable)
>>  		exynos_crtc->ops->enable(exynos_crtc);
>>  
>> -	exynos_crtc->enabled = true;
>> -
>>  	drm_crtc_vblank_on(crtc);
>>  }
>>  
>> @@ -41,9 +36,6 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
>>  {
>>  	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
>>  
>> -	if (!exynos_crtc->enabled)
>> -		return;
>> -
>>  	/* wait for the completion of page flip. */
>>  	if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
>>  				(exynos_crtc->event == NULL), HZ/20))
>> @@ -53,8 +45,6 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
>>  
>>  	if (exynos_crtc->ops->disable)
>>  		exynos_crtc->ops->disable(exynos_crtc);
>> -
>> -	exynos_crtc->enabled = false;
>>  }
>>  
>>  static bool
>> @@ -171,9 +161,6 @@ int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe)
>>  	struct exynos_drm_crtc *exynos_crtc =
>>  		to_exynos_crtc(private->crtc[pipe]);
>>  
>> -	if (!exynos_crtc->enabled)
>> -		return -EPERM;
>> -
>>  	if (exynos_crtc->ops->enable_vblank)
>>  		return exynos_crtc->ops->enable_vblank(exynos_crtc);
>>  
>> @@ -186,9 +173,6 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe)
>>  	struct exynos_drm_crtc *exynos_crtc =
>>  		to_exynos_crtc(private->crtc[pipe]);
>>  
>> -	if (!exynos_crtc->enabled)
>> -		return;
>> -
>>  	if (exynos_crtc->ops->disable_vblank)
>>  		exynos_crtc->ops->disable_vblank(exynos_crtc);
>>  }
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 5bd1d3c..d3a8f0a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -185,7 +185,6 @@ struct exynos_drm_crtc {
>>  	struct drm_crtc			base;
>>  	enum exynos_drm_output_type	type;
>>  	unsigned int			pipe;
>> -	bool				enabled;
>>  	wait_queue_head_t		pending_flip_queue;
>>  	struct drm_pending_vblank_event	*event;
>>  	const struct exynos_drm_crtc_ops	*ops;
>> -- 
>> 2.1.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel