Re: [DPU PATCH 01/11] drm/msm: Skip seamless disables in crtc/encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-02-28 11:18, Sean Paul wrote:
Instead of duplicating whole swaths of atomic helper functions (which
are already out-of-date), just skip the encoder/crtc disables in the
.disable hooks.

Change-Id: I7bd9183ae60624204fb1de9550656b776efc7202
Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

Can you consider getting rid of these checks?

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |   8 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |   8 +
 drivers/gpu/drm/msm/msm_atomic.c            | 185 +-------------------
 3 files changed, 17 insertions(+), 184 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 3cdf1e3d9d96..a3ab6ed2bf1d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -3393,6 +3393,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
 {
 	struct dpu_crtc *dpu_crtc;
 	struct dpu_crtc_state *cstate;
+	struct drm_display_mode *mode;
 	struct drm_encoder *encoder;
 	struct msm_drm_private *priv;
 	unsigned long flags;
@@ -3407,8 +3408,15 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
 	}
 	dpu_crtc = to_dpu_crtc(crtc);
 	cstate = to_dpu_crtc_state(crtc->state);
+	mode = &cstate->base.adjusted_mode;
 	priv = crtc->dev->dev_private;

+	if (msm_is_mode_seamless(mode) || msm_is_mode_seamless_vrr(mode)
||
+	    msm_is_mode_seamless_dms(mode)) {
+		DPU_DEBUG("Seamless mode is being applied, skip
disable\n");
+		return;
+	}
+
Another topic of discussion which should be brought up with dri-devel.

May not be common in PC world, but there are a handful of mobile OEM's
using panels which supports more than one resolution. Primary use cases
involve "seamless" switching to optimized display resolution when
streaming content changes resolutions or rendering lossless data.

Jeykumar S.

 	DPU_DEBUG("crtc%d\n", crtc->base.id);

 	if (dpu_kms_is_suspend_state(crtc->dev))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3d168fa09f3f..28ceb589ee40 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2183,6 +2183,7 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
 	struct dpu_encoder_virt *dpu_enc = NULL;
 	struct msm_drm_private *priv;
 	struct dpu_kms *dpu_kms;
+	struct drm_display_mode *mode;
 	int i = 0;

 	if (!drm_enc) {
@@ -2196,6 +2197,13 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
 		return;
 	}

+	mode = &drm_enc->crtc->state->adjusted_mode;
+	if (msm_is_mode_seamless(mode) || msm_is_mode_seamless_vrr(mode)
||
+	    msm_is_mode_seamless_dms(mode)) {
+		DPU_DEBUG("Seamless mode is being applied, skip
disable\n");
+		return;
+	}
+
 	dpu_enc = to_dpu_encoder_virt(drm_enc);
 	DPU_DEBUG_ENC(dpu_enc, "\n");

diff --git a/drivers/gpu/drm/msm/msm_atomic.c
b/drivers/gpu/drm/msm/msm_atomic.c
index 46536edb72ee..5cfb80345052 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -84,189 +84,6 @@ static void msm_atomic_wait_for_commit_done(
 	}
 }

-static void
-msm_disable_outputs(struct drm_device *dev, struct drm_atomic_state
*old_state)
-{
-	struct drm_connector *connector;
-	struct drm_connector_state *old_conn_state, *new_conn_state;
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-	int i;
-
-	for_each_oldnew_connector_in_state(old_state, connector,
old_conn_state, new_conn_state, i) {
-		const struct drm_encoder_helper_funcs *funcs;
-		struct drm_encoder *encoder;
-		struct drm_crtc_state *old_crtc_state;
-		unsigned int crtc_idx;
-
-		/*
-		 * Shut down everything that's in the changeset and
currently
-		 * still on. So need to check the old, saved state.
-		 */
-		if (!old_conn_state->crtc)
-			continue;
-
-		crtc_idx = drm_crtc_index(old_conn_state->crtc);
-		old_crtc_state = drm_atomic_get_old_crtc_state(old_state,
-
old_conn_state->crtc);
-
-		if (!old_crtc_state->active ||
-
!drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
-			continue;
-
-		encoder = old_conn_state->best_encoder;
-
-		/* We shouldn't get this far if we didn't previously have
-		 * an encoder.. but WARN_ON() rather than explode.
-		 */
-		if (WARN_ON(!encoder))
-			continue;
-
-		if (msm_is_mode_seamless(
-			&connector->encoder->crtc->state->mode) ||
-			msm_is_mode_seamless_vrr(
-			&connector->encoder->crtc->state->adjusted_mode))
-			continue;
-
-		if (msm_is_mode_seamless_dms(
-			&connector->encoder->crtc->state->adjusted_mode))
-			continue;
-
-		funcs = encoder->helper_private;
-
-		DRM_DEBUG_ATOMIC("disabling [ENCODER:%d:%s]\n",
-				 encoder->base.id, encoder->name);
-
-		/*
-		 * Each encoder has at most one connector (since we always
steal
-		 * it away), so we won't call disable hooks twice.
-		 */
-		drm_bridge_disable(encoder->bridge);
-
-		/* Right function depends upon target state. */
-		if (new_conn_state->crtc && funcs->prepare)
-			funcs->prepare(encoder);
-		else if (funcs->disable)
-			funcs->disable(encoder);
-		else
-			funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
-
-		drm_bridge_post_disable(encoder->bridge);
-	}
-
-	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
new_crtc_state, i) {
-		const struct drm_crtc_helper_funcs *funcs;
-
-		/* Shut down everything that needs a full modeset. */
-		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
-			continue;
-
-		if (!old_crtc_state->active)
-			continue;
-
-		if (msm_is_mode_seamless(&crtc->state->mode) ||
-
msm_is_mode_seamless_vrr(&crtc->state->adjusted_mode))
-			continue;
-
-		if (msm_is_mode_seamless_dms(&crtc->state->adjusted_mode))
-			continue;
-
-		funcs = crtc->helper_private;
-
-		DRM_DEBUG_ATOMIC("disabling [CRTC:%d]\n",
-				 crtc->base.id);
-
-		/* Right function depends upon target state. */
-		if (new_crtc_state->enable && funcs->prepare)
-			funcs->prepare(crtc);
-		else if (funcs->disable)
-			funcs->disable(crtc);
-		else
-			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
-	}
-}
-
-static void
-msm_crtc_set_mode(struct drm_device *dev, struct drm_atomic_state
*old_state)
-{
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *new_crtc_state;
-	struct drm_connector *connector;
-	struct drm_connector_state *new_conn_state;
-	int i;
-
-	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
-		const struct drm_crtc_helper_funcs *funcs;
-
-		if (!new_crtc_state->mode_changed)
-			continue;
-
-		funcs = crtc->helper_private;
-
-		if (new_crtc_state->enable && funcs->mode_set_nofb) {
-			DRM_DEBUG_ATOMIC("modeset on [CRTC:%d]\n",
-					 crtc->base.id);
-
-			funcs->mode_set_nofb(crtc);
-		}
-	}
-
-	for_each_new_connector_in_state(old_state, connector,
new_conn_state, i) {
-		const struct drm_encoder_helper_funcs *funcs;
-		struct drm_crtc_state *new_crtc_state;
-		struct drm_encoder *encoder;
-		struct drm_display_mode *mode, *adjusted_mode;
-
-		if (!new_conn_state->best_encoder)
-			continue;
-
-		encoder = new_conn_state->best_encoder;
-		funcs = encoder->helper_private;
-		new_crtc_state = new_conn_state->crtc->state;
-		mode = &new_crtc_state->mode;
-		adjusted_mode = &new_crtc_state->adjusted_mode;
-
-		if (!new_crtc_state->mode_changed)
-			continue;
-
-		DRM_DEBUG_ATOMIC("modeset on [ENCODER:%d:%s]\n",
-				 encoder->base.id, encoder->name);
-
-		/*
-		 * Each encoder has at most one connector (since we always
steal
-		 * it away), so we won't call mode_set hooks twice.
-		 */
-		if (funcs->mode_set)
-			funcs->mode_set(encoder, mode, adjusted_mode);
-
-		drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
-	}
-}
-
-/**
- * msm_atomic_helper_commit_modeset_disables - modeset commit to disable
outputs
- * @dev: DRM device
- * @old_state: atomic state object with old state structures
- *
- * This function shuts down all the outputs that need to be shut down and
- * prepares them (if required) with the new mode.
- *
- * For compatibility with legacy crtc helpers this should be called
before
- * drm_atomic_helper_commit_planes(), which is what the default commit
function
- * does. But drivers with different needs can group the modeset commits
together
- * and do the plane commits at the end. This is useful for drivers doing
runtime
- * PM since planes updates then only happen when the CRTC is actually
enabled.
- */
-void msm_atomic_helper_commit_modeset_disables(struct drm_device *dev,
-		struct drm_atomic_state *old_state)
-{
-	msm_disable_outputs(dev, old_state);
-
-	drm_atomic_helper_update_legacy_modeset_state(dev, old_state);
-
-	msm_crtc_set_mode(dev, old_state);
-}
-
 /**
  * msm_atomic_helper_commit_modeset_enables - modeset commit to enable
outputs
  * @dev: DRM device
@@ -406,7 +223,7 @@ static void complete_commit(struct msm_commit *c)

 	kms->funcs->prepare_commit(kms, state);

-	msm_atomic_helper_commit_modeset_disables(dev, state);
+	drm_atomic_helper_commit_modeset_disables(dev, state);

 	drm_atomic_helper_commit_planes(dev, state, 0);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux