Re: [Freedreno] [PATCH v3 3/4] drm/msm/dpu: Remove suspend state tracking from crtc
On Fri, Oct 05, 2018 at 05:04:02PM -0400, Bruce Wang wrote:
> Since drm core's modeset locks serialize atomic commits, we don't need to
> track whether or not we're in a suspended state from inside the crtc for
> dpu_crtc_enable/disable. This patch removes the suspend logic from the crtc
> and
> removes the relevant tracing from dpu_trace. Since we removed all calls
> to dpu_kms_is_suspend_state, we can remove that function and the
> suspend_state field of dpu_kms as well.
>
> v2: added patch to patchset
> v3: reworded commit body and moved deletion of dpu_kms_is_suspend_state and
> suspend_state to this patch
>
> Signed-off-by: Bruce Wang
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 48 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 16
> drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +--
> 4 files changed, 6 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..ac604055a824 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -815,35 +815,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
> }
> }
>
> -/**
> - * _dpu_crtc_set_suspend - notify crtc of suspend enable/disable
> - * @crtc: Pointer to drm crtc object
> - * @enable: true to enable suspend, false to indicate resume
> - */
> -static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
> -{
> - struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> -
> - DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
> -
> - mutex_lock(_crtc->crtc_lock);
> -
> - /*
> - * If the vblank is enabled, release a power reference on suspend
> - * and take it back during resume (if it is still enabled).
> - */
> - trace_dpu_crtc_set_suspend(DRMID(_crtc->base), enable, dpu_crtc);
> - if (dpu_crtc->suspend == enable)
> - DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n",
> - crtc->base.id, enable);
> - else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
> - _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
> - }
> -
> - dpu_crtc->suspend = enable;
> - mutex_unlock(_crtc->crtc_lock);
> -}
> -
> /**
> * dpu_crtc_duplicate_state - state duplicate hook
> * @crtc: Pointer to drm crtc structure
> @@ -951,9 +922,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
>
> DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
>
> - if (dpu_kms_is_suspend_state(crtc->dev))
> - _dpu_crtc_set_suspend(crtc, true);
> -
> /* Disable/save vblank irq handling */
> drm_crtc_vblank_off(crtc);
>
> @@ -966,8 +934,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> atomic_read(_crtc->frame_pending));
>
> trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
> - if (dpu_crtc->enabled && !dpu_crtc->suspend &&
> - dpu_crtc->vblank_requested) {
> + if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
> _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
> }
> dpu_crtc->enabled = false;
> @@ -1033,8 +1000,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
>
> mutex_lock(_crtc->crtc_lock);
> trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
> - if (!dpu_crtc->enabled && !dpu_crtc->suspend &&
> - dpu_crtc->vblank_requested) {
> + if (!dpu_crtc->enabled && dpu_crtc->vblank_requested) {
> _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
> }
> dpu_crtc->enabled = true;
> @@ -1289,17 +1255,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc
> *crtc,
>
> int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> {
> - struct dpu_crtc *dpu_crtc;
> -
> - if (!crtc) {
> - DPU_ERROR("invalid crtc\n");
> - return -EINVAL;
> - }
> - dpu_crtc = to_dpu_crtc(crtc);
> + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>
> mutex_lock(_crtc->crtc_lock);
> trace_dpu_crtc_vblank(DRMID(_crtc->base), en, dpu_crtc);
> - if (dpu_crtc->enabled && !dpu_crtc->suspend) {
> + if (dpu_crtc->enabled) {
> _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
> }
> dpu_crtc->vblank_requested = en;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 3723b4830335..7dc181aabb4d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -133,7 +133,6 @@ struct dpu_crtc_frame_event {
> * @play_count: frame count between crtc enable and disable
> * @vblank_cb_time : ktime at vblank count reset
> * @vblank_requested : whether the user has requested vblank events
> - * @suspend : whether
[Freedreno] [PATCH v3 3/4] drm/msm/dpu: Remove suspend state tracking from crtc
Since drm core's modeset locks serialize atomic commits, we don't need to
track whether or not we're in a suspended state from inside the crtc for
dpu_crtc_enable/disable. This patch removes the suspend logic from the crtc and
removes the relevant tracing from dpu_trace. Since we removed all calls
to dpu_kms_is_suspend_state, we can remove that function and the
suspend_state field of dpu_kms as well.
v2: added patch to patchset
v3: reworded commit body and moved deletion of dpu_kms_is_suspend_state and
suspend_state to this patch
Signed-off-by: Bruce Wang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 48 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 16
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 15 +--
4 files changed, 6 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d4530d60767b..ac604055a824 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -815,35 +815,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
}
}
-/**
- * _dpu_crtc_set_suspend - notify crtc of suspend enable/disable
- * @crtc: Pointer to drm crtc object
- * @enable: true to enable suspend, false to indicate resume
- */
-static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
-{
- struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
-
- DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
-
- mutex_lock(_crtc->crtc_lock);
-
- /*
-* If the vblank is enabled, release a power reference on suspend
-* and take it back during resume (if it is still enabled).
-*/
- trace_dpu_crtc_set_suspend(DRMID(_crtc->base), enable, dpu_crtc);
- if (dpu_crtc->suspend == enable)
- DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n",
- crtc->base.id, enable);
- else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
- _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
- }
-
- dpu_crtc->suspend = enable;
- mutex_unlock(_crtc->crtc_lock);
-}
-
/**
* dpu_crtc_duplicate_state - state duplicate hook
* @crtc: Pointer to drm crtc structure
@@ -951,9 +922,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
- if (dpu_kms_is_suspend_state(crtc->dev))
- _dpu_crtc_set_suspend(crtc, true);
-
/* Disable/save vblank irq handling */
drm_crtc_vblank_off(crtc);
@@ -966,8 +934,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
atomic_read(_crtc->frame_pending));
trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
- if (dpu_crtc->enabled && !dpu_crtc->suspend &&
- dpu_crtc->vblank_requested) {
+ if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
_dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
}
dpu_crtc->enabled = false;
@@ -1033,8 +1000,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
- if (!dpu_crtc->enabled && !dpu_crtc->suspend &&
- dpu_crtc->vblank_requested) {
+ if (!dpu_crtc->enabled && dpu_crtc->vblank_requested) {
_dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
}
dpu_crtc->enabled = true;
@@ -1289,17 +1255,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
{
- struct dpu_crtc *dpu_crtc;
-
- if (!crtc) {
- DPU_ERROR("invalid crtc\n");
- return -EINVAL;
- }
- dpu_crtc = to_dpu_crtc(crtc);
+ struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_vblank(DRMID(_crtc->base), en, dpu_crtc);
- if (dpu_crtc->enabled && !dpu_crtc->suspend) {
+ if (dpu_crtc->enabled) {
_dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
}
dpu_crtc->vblank_requested = en;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 3723b4830335..7dc181aabb4d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -133,7 +133,6 @@ struct dpu_crtc_frame_event {
* @play_count: frame count between crtc enable and disable
* @vblank_cb_time : ktime at vblank count reset
* @vblank_requested : whether the user has requested vblank events
- * @suspend : whether or not a suspend operation is in progress
* @enabled : whether the DPU CRTC is currently enabled. updated in the
* commit-thread, not state-swap time which is earlier, so
* safe to
