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@xxxxxxxxx>
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> >
> > 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
