Re: [RFC 2/2] drm/i915/display: Protect pipe_update against dc3co exit

[Anshuman Gupta <anshuman.gupta@intel.com>]

On 2020-12-04 at 17:51:34 +0200, Ville Syrjälä wrote:
> 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@intel.com>
> > > > Cc: <stable@vger.kernel.org>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@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?
Hi Ville / Imre ,
Above suggested chnages is not helping to fix the display glitches.
Could you please provide your inputs to debug the possible root cause considering the patch https://patchwork.freedesktop.org/patch/405585/?series=84394&rev=2
fixes the glitch.
Thanks,
Anshuman Gupta.
 
> 
> -- 
> Ville Syrjälä
> Intel