[PATCH 0/6] drm/msm: Misc fixes and improvements

[PATCH 0/6] drm/msm: Misc fixes and improvements

[Archit Taneja]

Misc fixes for better stability on 8x96. One of the cursor patches fixes
the async plane update path, the other makes sure we don't restore
LM cursor registers if we aren't using it.

Archit Taneja (6):
  drm/msm/hdmi: Keep the HDMI_CTRL_ENABLE bitfield always on for 8x96
  drm/msm/hdmi: Switch to DRM_CONNECTOR_POLL_HPD
  drm/msm/mdp5: Prepare mdp5_pipe_assign for some rework
  drm/msm/mdp5: Update mdp5_pipe_assign to spit out both planes
  drm/msm/mdp5: mdp5_crtc: Restore cursor state only if LM cursors are
    enabled
  drm/msm/mdp5: Don't use async plane update path if plane visibility
    changes

 drivers/gpu/drm/msm/hdmi/hdmi.c           |  3 ++
 drivers/gpu/drm/msm/hdmi/hdmi.h           |  3 ++
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c |  3 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c  | 66 +++++++++++++++-------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  | 71 +++++++++++++++++++++++--------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  7 +--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 58 +++++++++++++++++--------
 7 files changed, 143 insertions(+), 68 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

[PATCH 1/6] drm/msm/hdmi: Keep the HDMI_CTRL_ENABLE bitfield always on for 8x96

[Archit Taneja]

The ENABLE field in REG_HDMI_CTRL is required to be set to detect hot plug
events on 8x96. We don't get any HPD interrupts when HDMI bridge is
disabled.

Keep it always on. Downstream also seems to do the same thing. Restrict
this quirk only to 8x96, since we're not entirely sure whether this is
a legitimate fix for other platforms or not.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 3 +++
 drivers/gpu/drm/msm/hdmi/hdmi.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index e63dc0fb55f8..747f69f8321a 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -39,6 +39,8 @@ void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on)
 		}
 	} else {
 		ctrl = HDMI_CTRL_HDMI;
+		if (hdmi->config->keep_ctrl_on)
+			ctrl |= HDMI_CTRL_ENABLE;
 	}
 
 	hdmi_write(hdmi, REG_HDMI_CTRL, ctrl);
@@ -406,6 +408,7 @@ static struct hdmi_platform_config hdmi_tx_8996_config = {
 		HDMI_CFG(pwr_clk, 8x74),
 		HDMI_CFG(hpd_clk, 8x74),
 		.hpd_freq      = hpd_clk_freq_8x74,
+		.keep_ctrl_on  = true,
 };
 
 static const struct {
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index accc9a61611d..64291551fff6 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -121,6 +121,9 @@ struct hdmi_platform_config {
 
 	/* gpio's: */
 	struct hdmi_gpio_data gpios[HDMI_MAX_NUM_GPIO];
+
+	/* quirks, etc. */
+	bool keep_ctrl_on;
 };
 
 void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 2/6] drm/msm/hdmi: Switch to DRM_CONNECTOR_POLL_HPD

[Archit Taneja]

We support HPD interrupts on all platofrms, let's start using it.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index c0848dfedd50..6293f45a7f79 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -465,8 +465,7 @@ struct drm_connector *msm_hdmi_connector_init(struct hdmi *hdmi)
 			DRM_MODE_CONNECTOR_HDMIA);
 	drm_connector_helper_add(connector, &msm_hdmi_connector_helper_funcs);
 
-	connector->polled = DRM_CONNECTOR_POLL_CONNECT |
-			DRM_CONNECTOR_POLL_DISCONNECT;
+	connector->polled = DRM_CONNECTOR_POLL_HPD;
 
 	connector->interlace_allowed = 0;
 	connector->doublescan_allowed = 0;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

[PATCH 3/6] drm/msm/mdp5: Prepare mdp5_pipe_assign for some rework

[Archit Taneja]

mdp5_pipe_assign currently returns the hwpipe pointer for the drm_plane.
Return it indirectly by setting a pointer passed as an argument. This
is needed because we want the func to find out the right hwpipe too.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  | 32 +++++++++++++++----------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  4 ++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 18 ++++++++---------
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
index 2bfac3712685..1ca9ecc46d91 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
@@ -17,19 +17,19 @@
 
 #include "mdp5_kms.h"
 
-struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
-		struct drm_plane *plane, uint32_t caps, uint32_t blkcfg)
+int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
+		     uint32_t caps, uint32_t blkcfg,
+		     struct mdp5_hw_pipe **hwpipe)
 {
 	struct msm_drm_private *priv = s->dev->dev_private;
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
 	struct mdp5_state *state;
 	struct mdp5_hw_pipe_state *old_state, *new_state;
-	struct mdp5_hw_pipe *hwpipe = NULL;
 	int i;
 
 	state = mdp5_get_state(s);
 	if (IS_ERR(state))
-		return ERR_CAST(state);
+		return PTR_ERR(state);
 
 	/* grab old_state after mdp5_get_state(), since now we hold lock: */
 	old_state = &mdp5_kms->state->hwpipe;
@@ -64,31 +64,31 @@ struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
 		/* possible candidate, take the one with the
 		 * fewest unneeded caps bits set:
 		 */
-		if (!hwpipe || (hweight_long(cur->caps & ~caps) <
-				hweight_long(hwpipe->caps & ~caps)))
-			hwpipe = cur;
+		if (!(*hwpipe) || (hweight_long(cur->caps & ~caps) <
+				   hweight_long((*hwpipe)->caps & ~caps)))
+			*hwpipe = cur;
 	}
 
-	if (!hwpipe)
-		return ERR_PTR(-ENOMEM);
+	if (!(*hwpipe))
+		return -ENOMEM;
 
 	if (mdp5_kms->smp) {
 		int ret;
 
-		DBG("%s: alloc SMP blocks", hwpipe->name);
+		DBG("%s: alloc SMP blocks", (*hwpipe)->name);
 		ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
-				hwpipe->pipe, blkcfg);
+				(*hwpipe)->pipe, blkcfg);
 		if (ret)
-			return ERR_PTR(-ENOMEM);
+			return -ENOMEM;
 
-		hwpipe->blkcfg = blkcfg;
+		(*hwpipe)->blkcfg = blkcfg;
 	}
 
 	DBG("%s: assign to plane %s for caps %x",
-			hwpipe->name, plane->name, caps);
-	new_state->hwpipe_to_plane[hwpipe->idx] = plane;
+			(*hwpipe)->name, plane->name, caps);
+	new_state->hwpipe_to_plane[(*hwpipe)->idx] = plane;
 
-	return hwpipe;
+	return 0;
 }
 
 void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
index 924c3e6f9517..aaa2bd4e9c32 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
@@ -44,9 +44,9 @@ struct mdp5_hw_pipe_state {
 	struct drm_plane *hwpipe_to_plane[SSPP_MAX];
 };
 
-struct mdp5_hw_pipe *__must_check
+int
 mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
-		uint32_t caps, uint32_t blkcfg);
+		 uint32_t caps, uint32_t blkcfg, struct mdp5_hw_pipe **hwpipe);
 void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe);
 
 struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 56287219d134..aec115e20053 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -394,21 +394,21 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 			struct mdp5_hw_pipe *old_right_hwpipe =
 							  mdp5_state->r_hwpipe;
 
-			mdp5_state->hwpipe = mdp5_pipe_assign(state->state,
-					plane, caps, blkcfg);
-			if (IS_ERR(mdp5_state->hwpipe)) {
+			ret = mdp5_pipe_assign(state->state, plane, caps,
+					       blkcfg, &mdp5_state->hwpipe);
+			if (ret) {
 				DBG("%s: failed to assign hwpipe!", plane->name);
-				return PTR_ERR(mdp5_state->hwpipe);
+				return ret;
 			}
 
 			if (need_right_hwpipe) {
-				mdp5_state->r_hwpipe =
-					mdp5_pipe_assign(state->state, plane,
-							 caps, blkcfg);
-				if (IS_ERR(mdp5_state->r_hwpipe)) {
+				ret = mdp5_pipe_assign(state->state, plane,
+						       caps, blkcfg,
+						       &mdp5_state->r_hwpipe);
+				if (ret) {
 					DBG("%s: failed to assign right hwpipe",
 					    plane->name);
-					return PTR_ERR(mdp5_state->r_hwpipe);
+					return ret;
 				}
 			} else {
 				/*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

[PATCH 4/6] drm/msm/mdp5: Update mdp5_pipe_assign to spit out both planes

[Archit Taneja]

We currently call mdp5_pipe_assign() twice to assign the left and right
hwpipes for our drm_plane. When merging 2 hwpipes, there are a few
constraints that we need to keep in mind:

- Only the same types of SSPPs are preferred. I.e, a RGB pipe should
  be paired with another RGB pipe, VIG with VIG etc.
- The hwpipe staged on the left should have a higher priority than
  the hwpipe staged on the right. The priorities are as follows:
  VIG0 > VIG1 > VIG2 > VIG3
  RGB0 > RGB1 > RGB2 > RGB3
  DMA0 > DMA1

We can't apply these constraints easily if mdp5_pipe_assign() is
called twice. Update mdp5_pipe_assign() to find both hwpipes in
one go, and add the extra constraints needed.

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  | 45 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  7 ++---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 25 +++++++++--------
 3 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
index 1ca9ecc46d91..ff52c49095f9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
@@ -19,13 +19,14 @@
 
 int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
 		     uint32_t caps, uint32_t blkcfg,
-		     struct mdp5_hw_pipe **hwpipe)
+		     struct mdp5_hw_pipe **hwpipe,
+		     struct mdp5_hw_pipe **r_hwpipe)
 {
 	struct msm_drm_private *priv = s->dev->dev_private;
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
 	struct mdp5_state *state;
 	struct mdp5_hw_pipe_state *old_state, *new_state;
-	int i;
+	int i, j;
 
 	state = mdp5_get_state(s);
 	if (IS_ERR(state))
@@ -65,16 +66,46 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
 		 * fewest unneeded caps bits set:
 		 */
 		if (!(*hwpipe) || (hweight_long(cur->caps & ~caps) <
-				   hweight_long((*hwpipe)->caps & ~caps)))
-			*hwpipe = cur;
+				   hweight_long((*hwpipe)->caps & ~caps))) {
+			bool r_found = false;
+
+			if (r_hwpipe) {
+				for (j = i + 1; j < mdp5_kms->num_hwpipes;
+				     j++) {
+					struct mdp5_hw_pipe *r_cur =
+							mdp5_kms->hwpipes[j];
+
+					/* reject different types of hwpipes */
+					if (r_cur->caps != cur->caps)
+						continue;
+
+					/* respect priority, eg. VIG0 > VIG1 */
+					if (cur->pipe > r_cur->pipe)
+						continue;
+
+					*r_hwpipe = r_cur;
+					r_found = true;
+					break;
+				}
+			}
+
+			if (!r_hwpipe || r_found)
+				*hwpipe = cur;
+		}
 	}
 
 	if (!(*hwpipe))
 		return -ENOMEM;
 
+	if (r_hwpipe && !(*r_hwpipe))
+		return -ENOMEM;
+
 	if (mdp5_kms->smp) {
 		int ret;
 
+		/* We don't support SMP and 2 hwpipes/plane together */
+		WARN_ON(r_hwpipe);
+
 		DBG("%s: alloc SMP blocks", (*hwpipe)->name);
 		ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
 				(*hwpipe)->pipe, blkcfg);
@@ -88,6 +119,12 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
 			(*hwpipe)->name, plane->name, caps);
 	new_state->hwpipe_to_plane[(*hwpipe)->idx] = plane;
 
+	if (r_hwpipe) {
+		DBG("%s: assign to right of plane %s for caps %x",
+		    (*r_hwpipe)->name, plane->name, caps);
+		new_state->hwpipe_to_plane[(*r_hwpipe)->idx] = plane;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
index aaa2bd4e9c32..bb2b0ac7aa2b 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
@@ -44,9 +44,10 @@ struct mdp5_hw_pipe_state {
 	struct drm_plane *hwpipe_to_plane[SSPP_MAX];
 };
 
-int
-mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
-		 uint32_t caps, uint32_t blkcfg, struct mdp5_hw_pipe **hwpipe);
+int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
+		     uint32_t caps, uint32_t blkcfg,
+		     struct mdp5_hw_pipe **hwpipe,
+		     struct mdp5_hw_pipe **r_hwpipe);
 void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe);
 
 struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index aec115e20053..f1cf367e853d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -393,31 +393,30 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 			struct mdp5_hw_pipe *old_hwpipe = mdp5_state->hwpipe;
 			struct mdp5_hw_pipe *old_right_hwpipe =
 							  mdp5_state->r_hwpipe;
+			struct mdp5_hw_pipe *new_hwpipe = NULL;
+			struct mdp5_hw_pipe *new_right_hwpipe = NULL;
 
 			ret = mdp5_pipe_assign(state->state, plane, caps,
-					       blkcfg, &mdp5_state->hwpipe);
+					       blkcfg, &new_hwpipe,
+					       need_right_hwpipe ?
+					       &new_right_hwpipe : NULL);
 			if (ret) {
-				DBG("%s: failed to assign hwpipe!", plane->name);
+				DBG("%s: failed to assign hwpipe(s)!",
+				    plane->name);
 				return ret;
 			}
 
-			if (need_right_hwpipe) {
-				ret = mdp5_pipe_assign(state->state, plane,
-						       caps, blkcfg,
-						       &mdp5_state->r_hwpipe);
-				if (ret) {
-					DBG("%s: failed to assign right hwpipe",
-					    plane->name);
-					return ret;
-				}
-			} else {
+			mdp5_state->hwpipe = new_hwpipe;
+			if (need_right_hwpipe)
+				mdp5_state->r_hwpipe = new_right_hwpipe;
+			else
 				/*
 				 * set it to NULL so that the driver knows we
 				 * don't have a right hwpipe when committing a
 				 * new state
 				 */
 				mdp5_state->r_hwpipe = NULL;
-			}
+
 
 			mdp5_pipe_release(state->state, old_hwpipe);
 			mdp5_pipe_release(state->state, old_right_hwpipe);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 5/6] drm/msm/mdp5: mdp5_crtc: Restore cursor state only if LM cursors are enabled

[Archit Taneja]

MDP5 on newer SoCs support cursor planes (i.e, cursor SSPPs). They are a
separate entity unlike the cursors within LM.

Do not try to restore the MDP5 LM cursor registers, or the corresponding
CTL bits if we are not using LM cursors.

Also, since we've introduced a new variable 'lm_cursor_enabled', we can
now use it to avoid creating a different sets of crtc_funcs for CRTCs
with LM cursors and CRTCs with cursor planes.

Fixes: "drm/msm/mdp5: restore cursor state when enabling crtc"
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 66 ++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index 562853eb4be2..e414850dbbda 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -55,6 +55,8 @@ struct mdp5_crtc {
 
 	struct completion pp_completion;
 
+	bool lm_cursor_enabled;
+
 	struct {
 		/* protect REG_MDP5_LM_CURSOR* registers and cursor scanout_bo*/
 		spinlock_t lock;
@@ -457,19 +459,24 @@ static void mdp5_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	pm_runtime_get_sync(dev);
 
-	/* Restore cursor state, as it might have been lost with suspend: */
-	if (mdp5_crtc->cursor.iova) {
-		unsigned long flags;
-
-		spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
-		mdp5_crtc_restore_cursor(crtc);
-		spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
-
-		mdp5_ctl_set_cursor(mdp5_cstate->ctl,
-			&mdp5_cstate->pipeline, 0, true);
-	} else {
-		mdp5_ctl_set_cursor(mdp5_cstate->ctl,
-			&mdp5_cstate->pipeline, 0, false);
+	if (mdp5_crtc->lm_cursor_enabled) {
+		/*
+		 * Restore LM cursor state, as it might have been lost
+		 * with suspend:
+		 */
+		if (mdp5_crtc->cursor.iova) {
+			unsigned long flags;
+
+			spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
+			mdp5_crtc_restore_cursor(crtc);
+			spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
+
+			mdp5_ctl_set_cursor(mdp5_cstate->ctl,
+					    &mdp5_cstate->pipeline, 0, true);
+		} else {
+			mdp5_ctl_set_cursor(mdp5_cstate->ctl,
+					    &mdp5_cstate->pipeline, 0, false);
+		}
 	}
 
 	/* Restore vblank irq handling after power is enabled */
@@ -817,6 +824,12 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,
 	bool cursor_enable = true;
 	unsigned long flags;
 
+	if (!mdp5_crtc->lm_cursor_enabled) {
+		dev_warn(dev->dev,
+			 "cursor_set is deprecated with cursor planes\n");
+		return -EINVAL;
+	}
+
 	if ((width > CURSOR_WIDTH) || (height > CURSOR_HEIGHT)) {
 		dev_err(dev->dev, "bad cursor size: %dx%d\n", width, height);
 		return -EINVAL;
@@ -886,10 +899,17 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
 	struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
 	uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
+	struct drm_device *dev = crtc->dev;
 	uint32_t roi_w;
 	uint32_t roi_h;
 	unsigned long flags;
 
+	if (!mdp5_crtc->lm_cursor_enabled) {
+		dev_warn(dev->dev,
+			 "cursor_move is deprecated with cursor planes\n");
+		return -EINVAL;
+	}
+
 	/* don't support LM cursors when we we have source split enabled */
 	if (mdp5_cstate->pipeline.r_mixer)
 		return -EINVAL;
@@ -991,16 +1011,6 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = {
 	.atomic_print_state = mdp5_crtc_atomic_print_state,
 };
 
-static const struct drm_crtc_funcs mdp5_crtc_no_lm_cursor_funcs = {
-	.set_config = drm_atomic_helper_set_config,
-	.destroy = mdp5_crtc_destroy,
-	.page_flip = drm_atomic_helper_page_flip,
-	.reset = mdp5_crtc_reset,
-	.atomic_duplicate_state = mdp5_crtc_duplicate_state,
-	.atomic_destroy_state = mdp5_crtc_destroy_state,
-	.atomic_print_state = mdp5_crtc_atomic_print_state,
-};
-
 static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = {
 	.mode_set_nofb = mdp5_crtc_mode_set_nofb,
 	.atomic_check = mdp5_crtc_atomic_check,
@@ -1169,12 +1179,10 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
 	mdp5_crtc->err.irq = mdp5_crtc_err_irq;
 	mdp5_crtc->pp_done.irq = mdp5_crtc_pp_done_irq;
 
-	if (cursor_plane)
-		drm_crtc_init_with_planes(dev, crtc, plane, cursor_plane,
-					  &mdp5_crtc_no_lm_cursor_funcs, NULL);
-	else
-		drm_crtc_init_with_planes(dev, crtc, plane, NULL,
-					  &mdp5_crtc_funcs, NULL);
+	mdp5_crtc->lm_cursor_enabled = cursor_plane ? false : true;
+
+	drm_crtc_init_with_planes(dev, crtc, plane, cursor_plane,
+				  &mdp5_crtc_funcs, NULL);
 
 	drm_flip_work_init(&mdp5_crtc->unref_cursor_work,
 			"unref cursor", unref_cursor_worker);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 6/6] drm/msm/mdp5: Don't use async plane update path if plane visibility changes

[Archit Taneja]

When a plane moves out of bounds (i.e, outside the crtc clip region), the
plane state's "visible" parameter changes to false. When this happens, we
(a) release the hwpipe resources away from it, and
(b) unstage the corresponding hwpipe(s) from the Layer Mixers in the CRTC.

(a) requires use to acquire the global atomic state and assign a new
hwpipe. (b) requires us to re-configure the Layer Mixer, which is done in
the CRTC. We don't want to do these things in the async plane update path,
so return an error if the new state's "visible" isn't the same as the
current state's "visible".

Cc: Gustavo Padovan <gustavo.padovan@collabora.com>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index f1cf367e853d..be50445f9901 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -470,6 +470,9 @@ static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
 {
 	struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
 	struct drm_crtc_state *crtc_state;
+	struct drm_rect clip;
+	int min_scale, max_scale;
+	int ret;
 
 	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
 							state->crtc);
@@ -495,6 +498,28 @@ static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
 	    plane->state->fb != state->fb)
 		return -EINVAL;
 
+	clip.x1 = 0;
+	clip.y1 = 0;
+	clip.x2 = crtc_state->adjusted_mode.hdisplay;
+	clip.y2 = crtc_state->adjusted_mode.vdisplay;
+	min_scale = FRAC_16_16(1, 8);
+	max_scale = FRAC_16_16(8, 1);
+
+	ret = drm_plane_helper_check_state(state, &clip, min_scale,
+					   max_scale, true, true);
+	if (ret)
+		return ret;
+
+	/*
+	 * if the visibility of the plane changes (i.e, if the cursor is
+	 * clipped out completely, we can't take the async path because
+	 * we need to stage/unstage the plane from the Layer Mixer(s). We
+	 * also assign/unassign the hwpipe(s) tied to the plane. We avoid
+	 * taking the fast path for both these reasons.
+	 */
+	if (state->visible != plane->state->visible)
+		return -EINVAL;
+
 	return 0;
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 6/6] drm/msm/mdp5: Don't use async plane update path if plane visibility changes

[Gustavo Padovan]

Hi Archit,

2017-10-27 Archit Taneja <architt@codeaurora.org>:

> When a plane moves out of bounds (i.e, outside the crtc clip region), the
> plane state's "visible" parameter changes to false. When this happens, we
> (a) release the hwpipe resources away from it, and
> (b) unstage the corresponding hwpipe(s) from the Layer Mixers in the CRTC.
> 
> (a) requires use to acquire the global atomic state and assign a new
> hwpipe. (b) requires us to re-configure the Layer Mixer, which is done in
> the CRTC. We don't want to do these things in the async plane update path,
> so return an error if the new state's "visible" isn't the same as the
> current state's "visible".
> 
> Cc: Gustavo Padovan <gustavo.padovan@collabora.com>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index f1cf367e853d..be50445f9901 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -470,6 +470,9 @@ static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
>  {
>  	struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
>  	struct drm_crtc_state *crtc_state;
> +	struct drm_rect clip;
> +	int min_scale, max_scale;
> +	int ret;
>  
>  	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
>  							state->crtc);
> @@ -495,6 +498,28 @@ static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
>  	    plane->state->fb != state->fb)
>  		return -EINVAL;
>  
> +	clip.x1 = 0;
> +	clip.y1 = 0;
> +	clip.x2 = crtc_state->adjusted_mode.hdisplay;
> +	clip.y2 = crtc_state->adjusted_mode.vdisplay;
> +	min_scale = FRAC_16_16(1, 8);
> +	max_scale = FRAC_16_16(8, 1);
> +
> +	ret = drm_plane_helper_check_state(state, &clip, min_scale,
> +					   max_scale, true, true);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * if the visibility of the plane changes (i.e, if the cursor is
> +	 * clipped out completely, we can't take the async path because
> +	 * we need to stage/unstage the plane from the Layer Mixer(s). We
> +	 * also assign/unassign the hwpipe(s) tied to the plane. We avoid
> +	 * taking the fast path for both these reasons.
> +	 */
> +	if (state->visible != plane->state->visible)
> +		return -EINVAL;
> +
>  	return 0;
>  }

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.com>

Gustavo