Skip to contentSkip to conversation

drm/msm/mdp5: fix missing CTL flush

PATCH In review Rob Clark 2 participants 2 messages Jul 3, 2018

Subsystems (3)
lore
Share & tools
lore

Rob Clark

author
#0 lore raw

PATCH 1/1 drm/msm/mdp5: fix missing CTL flush

f9cb8d8d836e fixed various race conditions with CTL flush, in particular flushing and sending the START signal before encoder state was updated. But it did this a little too well in some cases that don't trigger encoder->enable(), and CTL[n].FLUSH would never be set. When page flips happen it would paper over the bug, since the first plag flip would flush out the state to the hardware.

The issue could be reproduced with, for example, modetest (without the '-v' argument).

Fixes f9cb8d8d836e drm/msm/mdp5: rework CTL START signal handling
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c +11 -1
index 9af94e35f678..fcd44d1d1068 100644
@@ -319,7 +319,17 @@ static int mdp5_encoder_atomic_check(struct drm_encoder *encoder,
 
 	mdp5_cstate->ctl = ctl;
 	mdp5_cstate->pipeline.intf = intf;
-	mdp5_cstate->defer_start = true;
+
+	/*
+	 * This is a bit awkward, but we want to flush the CTL and hit the
+	 * START bit at most once for an atomic update.  In the non-full-
+	 * modeset case, this is done from crtc->atomic_flush(), but that
+	 * is too early in the case of full modeset, in which case we
+	 * defer to encoder->enable().  But we need to *know* whether
+	 * encoder->enable() will be called to do this:
+	 */
+	if (drm_atomic_crtc_needs_modeset(crtc_state))
+		mdp5_cstate->defer_start = true;
 
 	return 0;
 }
Signature · 6 lines
2.17.1

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Rob Clark

f9cb8d8d836e fixed various race conditions with CTL flush, in particular flushing and sending the START signal before encoder state was updated. But it did this a little too well in some cases that don't trigger encoder->enable(), and CTL[n].FLUSH would never be set. When page flips happen it would paper over the bug, since the first plag flip would flush out the state to the hardware.

Quoted 5 lines

The issue could be reproduced with, for example, modetest (without the '-v' argument).

Fixes f9cb8d8d836e drm/msm/mdp5: rework CTL START signal handling
Quoted 30 lines
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c +11 -1
index 9af94e35f678..fcd44d1d1068 100644
@@ -319,7 +319,17 @@ static int mdp5_encoder_atomic_check(struct drm_encoder *encoder,
 
 	mdp5_cstate->ctl = ctl;
 	mdp5_cstate->pipeline.intf = intf;
-	mdp5_cstate->defer_start = true;
+
+	/*
+	 * This is a bit awkward, but we want to flush the CTL and hit the
+	 * START bit at most once for an atomic update.  In the non-full-
+	 * modeset case, this is done from crtc->atomic_flush(), but that
+	 * is too early in the case of full modeset, in which case we
+	 * defer to encoder->enable().  But we need to *know* whether
+	 * encoder->enable() will be called to do this:
+	 */
+	if (drm_atomic_crtc_needs_modeset(crtc_state))
+		mdp5_cstate->defer_start = true;
 
 	return 0;
 }
Signature · 1 line
2.17.1
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno