[Intel-gfx] [RFC 2/2] drm/i915/display: Protect pipe_update against dc3co exit

[Ville Syrjälä]

On Fri, Dec 04, 2020 at 01:40:03PM +0530, Anshuman Gupta wrote:
> On 2020-11-30 at 17:28:32 +0200, Imre Deak wrote:
> > On Mon, Nov 30, 2020 at 02:46:46PM +0530, Anshuman Gupta wrote:
> > > At usual case DC3CO exit happen automatically by DMC f/w whenever
> > > PSR2 clears idle. This happens smoothly by DMC f/w to work with flips.
> > > But there are certain scenario where DC3CO  Disallowed by driver
> > > asynchronous with flips. In such scenario display engine could
> > > be already in DC3CO state and driver has disallowed it,
> > > It initiates DC3CO exit sequence in DMC f/w which requires a
> > > dc3co exit delay of 200us in driver.
> > > It requires to protect intel_pipe_update_{update_end} with
> > > dc3co exit delay.
> > > 
> > > Cc: Imre Deak <imre.deak at intel.com>
> > > Cc: <stable at vger.kernel.org>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > 
> > To make sure that it doesn't hide the root cause (or affects unrelated
> > platforms), I'd only add locking around DC3co changes with a new lock,
> > using lock/unlock helpers in intel_display_power.c called from
> > intel_pipe_update_start/end.
> > 
> > Also please submit this patch separately, w/o the optimization in patch
> > 1/2, so we know that this change fixes the problem.
> This patch doesn't seems to fix the issue.
> Looks like there is some other set of display register updates before
> completing the dc3co exit delay beyond intel_pipe_update_start/end causing this issue.

Not really sure I understand the DC3CO issue here, nor how grabbing a
mutex across the update could help.

But anyways, maybe we should just:
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2e2dd746921f..96276f0feddc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16268,8 +16268,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
 	drm_atomic_helper_wait_for_dependencies(&state->base);
 
-	if (state->modeset)
-		wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
+	wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
@@ -16415,8 +16414,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		 * the culprit.
 		 */
 		intel_uncore_arm_unclaimed_mmio_detection(&dev_priv->uncore);
-		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET, wakeref);
 	}
+	intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET, wakeref);
 	intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
 
 	/*

To get the DMC out of equation entirely for all plane updates?

-- 
Ville Syrjälä
Intel