On Mon, May 23, 2016 at 03:56:36PM -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 v1: > - Leave the SSC source clock on instead of just shutting it off on all > of the DPLL configurations. > > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Lyude <cpaul@xxxxxxxxxx> > --- > Sorry about that! I misread danvet's suggestion as "disable the SSC" instead of > "don't disable the SSC". > > drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++--------- > 1 file changed, 35 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d500e6f..dff8511 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; > - u32 val, final; > + int i; > + u32 val, temp, 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) { > @@ -8283,9 +8285,26 @@ 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; > + > + /* Check if any DPLLs are using the SSC source */ > + for (i = 0; i < dev_priv->num_shared_dpll; i++) { > + 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; > + } > + } > + > + 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 SSC CPU output\n"); The CPU source isn't necessarily SSC. It can come from SSC1 or from the non-spread source. IIRC the clock tree looks something like this: integrated \ -> non-spread -> dpll non-spread / \ ck505 -> cpu source -> edp pll / integrated -> ssc -> ssc1 -> dpll ssc integrated -> super ssc -> ssc4 -> dpll super ssc Maybe we should slap that diagram somewhere as a comment? Anyways, othewise the patch looks OK I think. As far as the original code goes, I don't actually understand it. For LVDS it can enable the SSC source but leave SSC1 modulator disabled. Not sure what's the point in using the SSC source w/ SSC1 disabled. I'm not sure that's even legal TBH. For eDP it can select the non-spread source but still leave the SSC source enabled for some reason. Also I don't understand why the has_ssc depends on ck505 on IBX. We only use ck505 as the non-spread source, so not really sure why has_ssc depends on the presence of ck505. Maybe you can only get ssc+non-spread sources enabled at the same time if one of them comes from ck505? > 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 -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html