[1/8] drm/msm: dpu: Move pm_runtime_(get|put) from vblank_enable

Submitted by Sean Paul on Nov. 13, 2018, 8:52 p.m.

Details

Message ID 20181113205257.170707-1-sean@poorly.run
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Freedreno

Not browsing as part of any series.

Commit Message

Sean Paul Nov. 13, 2018, 8:52 p.m.
From: Sean Paul <seanpaul@chromium.org>

There are 4 times that _dpu_crtc_vblank_enable_no_lock() is called:

1- crtc enable
2- crtc disable
3- crtc vblank enable
4- crtc vblank disable

When we enable or disable the crtc, we call drm_crtc_vblank_on and
drm_crtc_vblank_off respectively. That will gate vblank enables and
disables to only being called when the crtc is active. That means that
we can just enable/disable pm runtime in crtc enable/disable. This will
be beneficial in trying to eliminate blocking calls from the vblank call
chain.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 80de5289ada3..aa2f20c05dd7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -777,8 +777,6 @@  static void _dpu_crtc_vblank_enable_no_lock(
 	struct drm_encoder *enc;
 
 	if (enable) {
-		pm_runtime_get_sync(dev->dev);
-
 		list_for_each_entry(enc, &dev->mode_config.encoder_list, head) {
 			if (enc->crtc != crtc)
 				continue;
@@ -801,8 +799,6 @@  static void _dpu_crtc_vblank_enable_no_lock(
 
 			dpu_encoder_register_vblank_callback(enc, NULL, NULL);
 		}
-
-		pm_runtime_put_sync(dev->dev);
 	}
 }
 
@@ -902,6 +898,8 @@  static void dpu_crtc_disable(struct drm_crtc *crtc)
 		crtc->state->event = NULL;
 		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
 	}
+
+	pm_runtime_put_sync(crtc->dev->dev);
 }
 
 static void dpu_crtc_enable(struct drm_crtc *crtc,
@@ -917,6 +915,8 @@  static void dpu_crtc_enable(struct drm_crtc *crtc,
 	}
 	priv = crtc->dev->dev_private;
 
+	pm_runtime_get_sync(crtc->dev->dev);
+
 	DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
 	dpu_crtc = to_dpu_crtc(crtc);
 

Comments

On Tue, Nov 13, 2018 at 03:52:44PM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>

I neglected to add --cover-letter to send-email, so pasting my cover here
instead:

Hi all,
So I kept digging into the locking and dependencies around encoder/crtc and this
is the latest series to pop out. I think things are a bit more clear and safe
at the end. We have now gotten rid of the crtc_lock and the pm_runtime sync
operations in vblank enable/disable, so we should be able to completely turf the
display thread/worker/etc!

Note that this series applies on top of [1].

Thanks!

Sean

[1]- https://lists.freedesktop.org/archives/dri-devel/2018-November/196170.html



> 
> There are 4 times that _dpu_crtc_vblank_enable_no_lock() is called:
> 
> 1- crtc enable
> 2- crtc disable
> 3- crtc vblank enable
> 4- crtc vblank disable
> 
> When we enable or disable the crtc, we call drm_crtc_vblank_on and
> drm_crtc_vblank_off respectively. That will gate vblank enables and
> disables to only being called when the crtc is active. That means that
> we can just enable/disable pm runtime in crtc enable/disable. This will
> be beneficial in trying to eliminate blocking calls from the vblank call
> chain.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 80de5289ada3..aa2f20c05dd7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -777,8 +777,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
>  	struct drm_encoder *enc;
>  
>  	if (enable) {
> -		pm_runtime_get_sync(dev->dev);
> -
>  		list_for_each_entry(enc, &dev->mode_config.encoder_list, head) {
>  			if (enc->crtc != crtc)
>  				continue;
> @@ -801,8 +799,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
>  
>  			dpu_encoder_register_vblank_callback(enc, NULL, NULL);
>  		}
> -
> -		pm_runtime_put_sync(dev->dev);
>  	}
>  }
>  
> @@ -902,6 +898,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
>  		crtc->state->event = NULL;
>  		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>  	}
> +
> +	pm_runtime_put_sync(crtc->dev->dev);
>  }
>  
>  static void dpu_crtc_enable(struct drm_crtc *crtc,
> @@ -917,6 +915,8 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
>  	}
>  	priv = crtc->dev->dev_private;
>  
> +	pm_runtime_get_sync(crtc->dev->dev);
> +
>  	DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
>  	dpu_crtc = to_dpu_crtc(crtc);
>  
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
>