drm/omap: Fix drm_handle_vblank() handling for command mode panels
diff mbox series

Message ID 20200225183733.50875-1-tony@atomide.com
State New
Headers show
Series
  • drm/omap: Fix drm_handle_vblank() handling for command mode panels
Related show

Commit Message

Tony Lindgren Feb. 25, 2020, 6:37 p.m. UTC
When trying to run weston on droid4 with the updated sgx blobs, the LCD
is just black and not updating. Weston also displays the following on
startup:

Warning: computed repaint delay is insane: -205475 msec

Weston runs fine on the HDMI alone though, and the issue was narrowed
down to an issue with vblank as suggested by ruleh <ruleh@gmx.de>.

Turns out that for command mode displays, we're not currently calling
drm_handle_vblank() at all since omap_irq_handler() won't do it for
us since we get no vblank interrupts. Let's fix the issue by calling
drm_handle_vblank() and omap_crtc_vblank_irq() for command mode
displays from omap_crtc_framedone_irq(). And we can now remove the
old partial handling in omap_crtc_framedone_irq().

For reference, below is my current weston.ini. The repaint-window is
maxed out to force immediate repaint instead of the default 7 ms.
Otherwise it seems that the repaint is happening at about 60 fps with
es2gears_wayland compared to about 130 fps where it seems to max out.

[core]
xwayland=true
backend=drm-backend.so
repaint-window=1000
pageflip-timeout=1000

[libinput]
rotation=0

[output]
name=DSI-1
transform=90

[output]
name=HDMI-1
mode=1024x768@60

Fixes: 47103a80f55a ("drm/omap: add framedone interrupt support")
Cc: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: ruleh <ruleh@gmx.de>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

Only lightly tested so far, please test. Also, I'm not sure if we
should get the id from somewhere for drm_handle_vblank() instead of
just using 0?

---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Tony Lindgren Feb. 25, 2020, 7:52 p.m. UTC | #1
* Tony Lindgren <tony@atomide.com> [200225 18:38]:
> Only lightly tested so far, please test. Also, I'm not sure if we
> should get the id from somewhere for drm_handle_vblank() instead of
> just using 0?

Also looks like we can now get WARN_ON(omap_crtc->pending)
in omap_crtc_arm_event(), so obviously some changes are
needed.

Regards,

Tony
Tony Lindgren Feb. 25, 2020, 10:34 p.m. UTC | #2
* Tony Lindgren <tony@atomide.com> [200225 19:53]:
> * Tony Lindgren <tony@atomide.com> [200225 18:38]:
> > Only lightly tested so far, please test. Also, I'm not sure if we
> > should get the id from somewhere for drm_handle_vblank() instead of
> > just using 0?
> 
> Also looks like we can now get WARN_ON(omap_crtc->pending)
> in omap_crtc_arm_event(), so obviously some changes are
> needed.

Below is an updated version that just returns early for
omap_crtc_arm_event() for manual update display if already
armed.

I'm now also passing the id from the interrupt handler, maybe
it's available somewhere locally but I just did not notice.

Regards,

Tony

8< -----------------------
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index d0bc9614588f9..515b5c1520b98 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -325,23 +325,21 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 	DBG("%s: apply done", omap_crtc->name);
 }
 
-void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, int id, uint32_t irqstatus)
 {
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	if (!omap_crtc->framedone_handler)
 		return;
 
-	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
-
-	spin_lock(&crtc->dev->event_lock);
-	/* Send the vblank event if one has been requested. */
-	if (omap_crtc->event) {
-		drm_crtc_send_vblank_event(crtc, omap_crtc->event);
-		omap_crtc->event = NULL;
+	if (omap_state->manually_updated) {
+		drm_handle_vblank(dev, id);
+		omap_crtc_vblank_irq(crtc);
 	}
-	omap_crtc->pending = false;
-	spin_unlock(&crtc->dev->event_lock);
+
+	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
 
 	/* Wake up omap_atomic_complete. */
 	wake_up(&omap_crtc->pending_wait);
@@ -439,8 +437,12 @@ static void omap_crtc_destroy(struct drm_crtc *crtc)
 
 static void omap_crtc_arm_event(struct drm_crtc *crtc)
 {
+	struct omap_crtc_state *omap_crtc_state = to_omap_crtc_state(crtc->state);
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
+	if (omap_crtc->pending && omap_crtc_state->manually_updated)
+		return;
+
 	WARN_ON(omap_crtc->pending);
 	omap_crtc->pending = true;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
index 2fd57751ae2b1..9221f8ef2e87f 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -30,7 +30,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 int omap_crtc_wait_pending(struct drm_crtc *crtc);
 void omap_crtc_error_irq(struct drm_crtc *crtc, u32 irqstatus);
 void omap_crtc_vblank_irq(struct drm_crtc *crtc);
-void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus);
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, int id, uint32_t irqstatus);
 void omap_crtc_flush(struct drm_crtc *crtc);
 
 #endif /* __OMAPDRM_CRTC_H__ */
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 382bcdc72ac06..068ea4fdf445c 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -232,7 +232,7 @@ static irqreturn_t omap_irq_handler(int irq, void *arg)
 			omap_crtc_error_irq(crtc, irqstatus);
 
 		if (irqstatus & priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel))
-			omap_crtc_framedone_irq(crtc, irqstatus);
+			omap_crtc_framedone_irq(crtc, id, irqstatus);
 	}
 
 	omap_irq_ocp_error_handler(dev, irqstatus);
Tony Lindgren Feb. 25, 2020, 10:59 p.m. UTC | #3
* Tony Lindgren <tony@atomide.com> [200225 22:35]:
> * Tony Lindgren <tony@atomide.com> [200225 19:53]:
> > * Tony Lindgren <tony@atomide.com> [200225 18:38]:
> > > Only lightly tested so far, please test. Also, I'm not sure if we
> > > should get the id from somewhere for drm_handle_vblank() instead of
> > > just using 0?
> > 
> > Also looks like we can now get WARN_ON(omap_crtc->pending)
> > in omap_crtc_arm_event(), so obviously some changes are
> > needed.
> 
> Below is an updated version that just returns early for
> omap_crtc_arm_event() for manual update display if already
> armed.
> 
> I'm now also passing the id from the interrupt handler, maybe
> it's available somewhere locally but I just did not notice.

And we need to have drm_crtc_vblank_get and put paired to
avoid more warnings.. Best to just have a single path for
handling vblank interrupt and manual mode panels.

Updated patch below again, this one seems to behave for
me so far based on the issues I noticed with earlier
attempts.

Regards,

Tony

8< ---------------------------
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index d0bc9614588f9..ed4abfef0536d 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -325,23 +325,21 @@ void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 	DBG("%s: apply done", omap_crtc->name);
 }
 
-void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, int id, uint32_t irqstatus)
 {
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	if (!omap_crtc->framedone_handler)
 		return;
 
-	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
-
-	spin_lock(&crtc->dev->event_lock);
-	/* Send the vblank event if one has been requested. */
-	if (omap_crtc->event) {
-		drm_crtc_send_vblank_event(crtc, omap_crtc->event);
-		omap_crtc->event = NULL;
+	if (omap_state->manually_updated) {
+		drm_handle_vblank(dev, id);
+		omap_crtc_vblank_irq(crtc);
 	}
-	omap_crtc->pending = false;
-	spin_unlock(&crtc->dev->event_lock);
+
+	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
 
 	/* Wake up omap_atomic_complete. */
 	wake_up(&omap_crtc->pending_wait);
@@ -439,8 +437,12 @@ static void omap_crtc_destroy(struct drm_crtc *crtc)
 
 static void omap_crtc_arm_event(struct drm_crtc *crtc)
 {
+	struct omap_crtc_state *omap_crtc_state = to_omap_crtc_state(crtc->state);
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
+	if (omap_crtc->pending && omap_crtc_state->manually_updated)
+		return;
+
 	WARN_ON(omap_crtc->pending);
 	omap_crtc->pending = true;
 
@@ -455,17 +457,12 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	int ret;
 
 	DBG("%s", omap_crtc->name);
 
 	priv->dispc_ops->runtime_get(priv->dispc);
 
-	/* manual updated display will not trigger vsync irq */
-	if (omap_state->manually_updated)
-		return;
-
 	spin_lock_irq(&crtc->dev->event_lock);
 	drm_crtc_vblank_on(crtc);
 	ret = drm_crtc_vblank_get(crtc);
@@ -646,20 +643,14 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	DBG("%s: GO", omap_crtc->name);
 
-	if (omap_crtc_state->manually_updated) {
-		/* send new image for page flips and modeset changes */
-		spin_lock_irq(&crtc->dev->event_lock);
-		omap_crtc_flush(crtc);
-		omap_crtc_arm_event(crtc);
-		spin_unlock_irq(&crtc->dev->event_lock);
-		return;
-	}
-
 	ret = drm_crtc_vblank_get(crtc);
 	WARN_ON(ret != 0);
 
 	spin_lock_irq(&crtc->dev->event_lock);
-	priv->dispc_ops->mgr_go(priv->dispc, omap_crtc->channel);
+	if (omap_crtc_state->manually_updated)
+		omap_crtc_flush(crtc);
+	else
+		priv->dispc_ops->mgr_go(priv->dispc, omap_crtc->channel);
 	omap_crtc_arm_event(crtc);
 	spin_unlock_irq(&crtc->dev->event_lock);
 }
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h
index 2fd57751ae2b1..9221f8ef2e87f 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.h
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.h
@@ -30,7 +30,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 int omap_crtc_wait_pending(struct drm_crtc *crtc);
 void omap_crtc_error_irq(struct drm_crtc *crtc, u32 irqstatus);
 void omap_crtc_vblank_irq(struct drm_crtc *crtc);
-void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus);
+void omap_crtc_framedone_irq(struct drm_crtc *crtc, int id, uint32_t irqstatus);
 void omap_crtc_flush(struct drm_crtc *crtc);
 
 #endif /* __OMAPDRM_CRTC_H__ */
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 382bcdc72ac06..068ea4fdf445c 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -232,7 +232,7 @@ static irqreturn_t omap_irq_handler(int irq, void *arg)
 			omap_crtc_error_irq(crtc, irqstatus);
 
 		if (irqstatus & priv->dispc_ops->mgr_get_framedone_irq(priv->dispc, channel))
-			omap_crtc_framedone_irq(crtc, irqstatus);
+			omap_crtc_framedone_irq(crtc, id, irqstatus);
 	}
 
 	omap_irq_ocp_error_handler(dev, irqstatus);

Patch
diff mbox series

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -326,21 +326,19 @@  void omap_crtc_vblank_irq(struct drm_crtc *crtc)
 
 void omap_crtc_framedone_irq(struct drm_crtc *crtc, uint32_t irqstatus)
 {
+	struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state);
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
 
 	if (!omap_crtc->framedone_handler)
 		return;
 
-	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
-
-	spin_lock(&crtc->dev->event_lock);
-	/* Send the vblank event if one has been requested. */
-	if (omap_crtc->event) {
-		drm_crtc_send_vblank_event(crtc, omap_crtc->event);
-		omap_crtc->event = NULL;
+	if (omap_state->manually_updated) {
+		drm_handle_vblank(dev, 0);
+		omap_crtc_vblank_irq(crtc);
 	}
-	omap_crtc->pending = false;
-	spin_unlock(&crtc->dev->event_lock);
+
+	omap_crtc->framedone_handler(omap_crtc->framedone_handler_data);
 
 	/* Wake up omap_atomic_complete. */
 	wake_up(&omap_crtc->pending_wait);