Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, May 26, 2016 at 09:54:56AM +0200, Daniel Vetter wrote:
> On Wed, May 25, 2016 at 02:11:02PM -0400, Lyude wrote:
> > Thanks to Ville Syrjälä for pointing me towards the cause of this issue.
> > 
> > Unfortunately one of the sideaffects of having the refclk for a DPLL set
> > to SSC is that as long as it's set to SSC, the GPU will prevent us from
> > powering down any of the pipes or transcoders using it. A couple of
> > BIOSes enable SSC in both PCH_DREF_CONTROL and in the DPLL
> > configurations. This causes issues on the first modeset, since we don't
> > expect SSC to be left on and as a result, can't successfully power down
> > the pipes or the transcoders using it. Here's an example from this Dell
> > OptiPlex 990:
> > 
> > [drm:intel_modeset_init] SSC enabled by BIOS, overriding VBT which says disabled
> > [drm:intel_modeset_init] 2 display pipes available.
> > [drm:intel_update_cdclk] Current CD clock rate: 400000 kHz
> > [drm:intel_update_max_cdclk] Max CD clock rate: 400000 kHz
> > [drm:intel_update_max_cdclk] Max dotclock rate: 360000 kHz
> > vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
> > [drm:intel_crt_reset] crt adpa set to 0xf40000
> > [drm:intel_dp_init_connector] Adding DP connector on port C
> > [drm:intel_dp_aux_init] registering DPDDC-C bus for card0-DP-1
> > [drm:ironlake_init_pch_refclk] has_panel 0 has_lvds 0 has_ck505 0
> > [drm:ironlake_init_pch_refclk] Disabling SSC entirely
> > … later we try committing the first modeset …
> > [drm:intel_dump_pipe_config] [CRTC:26][modeset] config ffff88041b02e800 for pipe A
> > [drm:intel_dump_pipe_config] cpu_transcoder: A
> > …
> > [drm:intel_dump_pipe_config] dpll_hw_state: dpll: 0xc4016001, dpll_md: 0x0, fp0: 0x20e08, fp1: 0x30d07
> > [drm:intel_dump_pipe_config] planes on this crtc
> > [drm:intel_dump_pipe_config] STANDARD PLANE:23 plane: 0.0 idx: 0 enabled
> > [drm:intel_dump_pipe_config]     FB:42, fb = 800x600 format = 0x34325258
> > [drm:intel_dump_pipe_config]     scaler:0 src (0, 0) 800x600 dst (0, 0) 800x600
> > [drm:intel_dump_pipe_config] CURSOR PLANE:25 plane: 0.1 idx: 1 disabled, scaler_id = 0
> > [drm:intel_dump_pipe_config] STANDARD PLANE:27 plane: 0.1 idx: 2 disabled, scaler_id = 0
> > [drm:intel_get_shared_dpll] CRTC:26 allocated PCH DPLL A
> > [drm:intel_get_shared_dpll] using PCH DPLL A for pipe A
> > [drm:ilk_audio_codec_disable] Disable audio codec on port C, pipe A
> > [drm:intel_disable_pipe] disabling pipe A
> > ------------[ cut here ]------------
> > WARNING: CPU: 1 PID: 130 at drivers/gpu/drm/i915/intel_display.c:1146 intel_disable_pipe+0x297/0x2d0 [i915]
> > pipe_off wait timed out
> > …
> > ---[ end trace 94fc8aa03ae139e8 ]---
> > [drm:intel_dp_link_down]
> > [drm:ironlake_crtc_disable [i915]] *ERROR* failed to disable transcoder A
> > 
> > Later modesets succeed since they reset the DPLL's configuration anyway,
> > but this is enough to get stuck with a big fat warning in dmesg.
> > 
> > A better solution would be to add refcounts for the SSC source, but for
> > now leaving the source clock on should suffice.
> > 
> > Changes since v3:
> >  - Move temp variable into loop
> >  - Move checks for using_ssc_source to after we've figured out has_ck505
> >  - Add using_ssc_source to debug output
> > Changes since v2:
> >  - Fix debug output for when we disable the CPU source
> > Changes since v1:
> >  - Leave the SSC source clock on instead of just shutting it off on all
> >    of the DPLL configurations.
> > 
> > Cc: stable@vger.kernel.org
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Lyude <cpaul@redhat.com>
> 
> Queued for -next, thanks for the patch.

Looks like this one broke one of the ILKs in CI.

[   13.100979] [drm:ironlake_init_pch_refclk] has_panel 1 has_lvds 1 has_ck505 0 using_ssc_source 1
[   13.101413] ------------[ cut here ]------------
[   13.101429] kernel BUG at drivers/gpu/drm/i915/intel_display.c:8528!

which is the 'BUG_ON(val != final)' at the end of ironlake_init_pch_refclk().


> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++++++++++----------
> >  1 file changed, 36 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d500e6f..471e3a8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8230,12 +8230,14 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_encoder *encoder;
> > +	int i;
> >  	u32 val, final;
> >  	bool has_lvds = false;
> >  	bool has_cpu_edp = false;
> >  	bool has_panel = false;
> >  	bool has_ck505 = false;
> >  	bool can_ssc = false;
> > +	bool using_ssc_source = false;
> >  
> >  	/* We need to take the global config into account */
> >  	for_each_intel_encoder(dev, encoder) {
> > @@ -8262,8 +8264,22 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> >  		can_ssc = true;
> >  	}
> >  
> > -	DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d\n",
> > -		      has_panel, has_lvds, has_ck505);
> > +	/* Check if any DPLLs are using the SSC source */
> > +	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> > +		u32 temp = I915_READ(PCH_DPLL(i));
> > +
> > +		if (!(temp & DPLL_VCO_ENABLE))
> > +			continue;
> > +
> > +		if ((temp & PLL_REF_INPUT_MASK) ==
> > +		    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
> > +			using_ssc_source = true;
> > +			break;
> > +		}
> > +	}
> > +
> > +	DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d using_ssc_source %d\n",
> > +		      has_panel, has_lvds, has_ck505, using_ssc_source);
> >  
> >  	/* Ironlake: try to setup display ref clock before DPLL
> >  	 * enabling. This is only under driver's control after
> > @@ -8283,9 +8299,12 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> >  	else
> >  		final |= DREF_NONSPREAD_SOURCE_ENABLE;
> >  
> > -	final &= ~DREF_SSC_SOURCE_MASK;
> >  	final &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> > -	final &= ~DREF_SSC1_ENABLE;
> > +
> > +	if (!using_ssc_source) {
> > +		final &= ~DREF_SSC_SOURCE_MASK;
> > +		final &= ~DREF_SSC1_ENABLE;
> > +	}
> >  
> >  	if (has_panel) {
> >  		final |= DREF_SSC_SOURCE_ENABLE;
> > @@ -8348,7 +8367,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> >  		POSTING_READ(PCH_DREF_CONTROL);
> >  		udelay(200);
> >  	} else {
> > -		DRM_DEBUG_KMS("Disabling SSC entirely\n");
> > +		DRM_DEBUG_KMS("Disabling CPU source output\n");
> >  
> >  		val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> >  
> > @@ -8359,16 +8378,20 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> >  		POSTING_READ(PCH_DREF_CONTROL);
> >  		udelay(200);
> >  
> > -		/* Turn off the SSC source */
> > -		val &= ~DREF_SSC_SOURCE_MASK;
> > -		val |= DREF_SSC_SOURCE_DISABLE;
> > +		if (!using_ssc_source) {
> > +			DRM_DEBUG_KMS("Disabling SSC source\n");
> >  
> > -		/* Turn off SSC1 */
> > -		val &= ~DREF_SSC1_ENABLE;
> > +			/* Turn off the SSC source */
> > +			val &= ~DREF_SSC_SOURCE_MASK;
> > +			val |= DREF_SSC_SOURCE_DISABLE;
> >  
> > -		I915_WRITE(PCH_DREF_CONTROL, val);
> > -		POSTING_READ(PCH_DREF_CONTROL);
> > -		udelay(200);
> > +			/* Turn off SSC1 */
> > +			val &= ~DREF_SSC1_ENABLE;
> > +
> > +			I915_WRITE(PCH_DREF_CONTROL, val);
> > +			POSTING_READ(PCH_DREF_CONTROL);
> > +			udelay(200);
> > +		}
> >  	}
> >  
> >  	BUG_ON(val != final);
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC