From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use Date: Mon, 06 Jun 2016 15:29:21 -0400 Message-ID: <1465241361.7166.2.camel@redhat.com> References: <20160524185924.GI4329@intel.com> <1464199863-9397-1-git-send-email-cpaul@redhat.com> <20160526075456.GM27098@phenom.ffwll.local> <20160606113022.GE4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160606113022.GE4329@intel.com> Sender: stable-owner@vger.kernel.org To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= , Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, David Airlie , stable@vger.kernel.org, "open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list" , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org On Mon, 2016-06-06 at 14:30 +0300, Ville Syrj=C3=A4l=C3=A4 wrote: > On Thu, May 26, 2016 at 09:54:56AM +0200, Daniel Vetter wrote: > >=20 > > On Wed, May 25, 2016 at 02:11:02PM -0400, Lyude wrote: > > >=20 > > > Thanks to Ville Syrj=C3=A4l=C3=A4 for pointing me towards the cau= se of this issue. > > >=20 > > > Unfortunately one of the sideaffects of having the refclk for a D= PLL set > > > to SSC is that as long as it's set to SSC, the GPU will prevent u= s 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 powe= r down > > > the pipes or the transcoders using it. Here's an example from thi= s Dell > > > OptiPlex 990: > > >=20 > > > [drm:intel_modeset_init] SSC enabled by BIOS, overriding VBT whic= h 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=3Dio+mem,decodes=3Dio+mem:owns=3Dio+m= em > > > [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 > > > =E2=80=A6 later we try committing the first modeset =E2=80=A6 > > > [drm:intel_dump_pipe_config] [CRTC:26][modeset] config ffff88041b= 02e800 > > > for pipe A > > > [drm:intel_dump_pipe_config] cpu_transcoder: A > > > =E2=80=A6 > > > [drm:intel_dump_pipe_config] dpll_hw_state: dpll: 0xc4016001, dpl= l_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]=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0FB:42, = fb =3D 800x600 format =3D 0x34325258 > > > [drm:intel_dump_pipe_config]=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0scaler:= 0 src (0, 0) 800x600 dst (0, 0) > > > 800x600 > > > [drm:intel_dump_pipe_config] CURSOR PLANE:25 plane: 0.1 idx: 1 di= sabled, > > > scaler_id =3D 0 > > > [drm:intel_dump_pipe_config] STANDARD PLANE:27 plane: 0.1 idx: 2 = disabled, > > > scaler_id =3D 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 > > > =E2=80=A6 > > > ---[ end trace 94fc8aa03ae139e8 ]--- > > > [drm:intel_dp_link_down] > > > [drm:ironlake_crtc_disable [i915]] *ERROR* failed to disable tran= scoder A > > >=20 > > > 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. > > >=20 > > > A better solution would be to add refcounts for the SSC source, b= ut for > > > now leaving the source clock on should suffice. > > >=20 > > > Changes since v3: > > > =C2=A0- Move temp variable into loop > > > =C2=A0- Move checks for using_ssc_source to after we've figured o= ut has_ck505 > > > =C2=A0- Add using_ssc_source to debug output > > > Changes since v2: > > > =C2=A0- Fix debug output for when we disable the CPU source > > > Changes since v1: > > > =C2=A0- Leave the SSC source clock on instead of just shutting it= off on all > > > =C2=A0=C2=A0=C2=A0of the DPLL configurations. > > >=20 > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Ville Syrj=C3=A4l=C3=A4 > > > Signed-off-by: Lyude > > Queued for -next, thanks for the patch. > Looks like this one broke one of the ILKs in CI. >=20 > [=C2=A0=C2=A0=C2=A013.100979] [drm:ironlake_init_pch_refclk] has_pane= l 1 has_lvds 1 has_ck505 > 0 using_ssc_source 1 > [=C2=A0=C2=A0=C2=A013.101413] ------------[ cut here ]------------ > [=C2=A0=C2=A0=C2=A013.101429] kernel BUG at drivers/gpu/drm/i915/inte= l_display.c:8528! >=20 > which is the 'BUG_ON(val !=3D final)' at the end of ironlake_init_pch= _refclk(). >=20 Managed to find a machine at the office here that I could reproduce thi= s on and figured out the problem with the patch. Unfortunately, the Optiplex 990= that I was seeing this issue on already left the office and the person in char= ge of it just went on PTO until the end of the week. For the time being don't me= rge this patch until I can get access to that machine again and make sure the fi= xed version of this patch still works on the 990. I'll send the new version= once I'm able to test it. Cheers, Lyude >=20 > >=20 > > -Daniel > >=20 > > >=20 > > > --- > > > =C2=A0drivers/gpu/drm/i915/intel_display.c | 49 +++++++++++++++++= +++++++++----- > > > ----- > > > =C2=A01 file changed, 36 insertions(+), 13 deletions(-) > > >=20 > > > 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(stru= ct > > > drm_device *dev) > > > =C2=A0{ > > > =C2=A0 struct drm_i915_private *dev_priv =3D dev->dev_private; > > > =C2=A0 struct intel_encoder *encoder; > > > + int i; > > > =C2=A0 u32 val, final; > > > =C2=A0 bool has_lvds =3D false; > > > =C2=A0 bool has_cpu_edp =3D false; > > > =C2=A0 bool has_panel =3D false; > > > =C2=A0 bool has_ck505 =3D false; > > > =C2=A0 bool can_ssc =3D false; > > > + bool using_ssc_source =3D false; > > > =C2=A0 > > > =C2=A0 /* We need to take the global config into account */ > > > =C2=A0 for_each_intel_encoder(dev, encoder) { > > > @@ -8262,8 +8264,22 @@ static void ironlake_init_pch_refclk(struc= t > > > drm_device *dev) > > > =C2=A0 can_ssc =3D true; > > > =C2=A0 } > > > =C2=A0 > > > - DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d\n", > > > - =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0has_panel, has_lvds, has_c= k505); > > > + /* Check if any DPLLs are using the SSC source */ > > > + for (i =3D 0; i < dev_priv->num_shared_dpll; i++) { > > > + u32 temp =3D I915_READ(PCH_DPLL(i)); > > > + > > > + if (!(temp & DPLL_VCO_ENABLE)) > > > + continue; > > > + > > > + if ((temp & PLL_REF_INPUT_MASK) =3D=3D > > > + =C2=A0=C2=A0=C2=A0=C2=A0PLLB_REF_INPUT_SPREADSPECTRUMIN) { > > > + using_ssc_source =3D true; > > > + break; > > > + } > > > + } > > > + > > > + DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d > > > using_ssc_source %d\n", > > > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0has_panel, has_lvds, has_c= k505, using_ssc_source); > > > =C2=A0 > > > =C2=A0 /* Ironlake: try to setup display ref clock before DPLL > > > =C2=A0 =C2=A0* enabling. This is only under driver's control afte= r > > > @@ -8283,9 +8299,12 @@ static void ironlake_init_pch_refclk(struc= t > > > drm_device *dev) > > > =C2=A0 else > > > =C2=A0 final |=3D DREF_NONSPREAD_SOURCE_ENABLE; > > > =C2=A0 > > > - final &=3D ~DREF_SSC_SOURCE_MASK; > > > =C2=A0 final &=3D ~DREF_CPU_SOURCE_OUTPUT_MASK; > > > - final &=3D ~DREF_SSC1_ENABLE; > > > + > > > + if (!using_ssc_source) { > > > + final &=3D ~DREF_SSC_SOURCE_MASK; > > > + final &=3D ~DREF_SSC1_ENABLE; > > > + } > > > =C2=A0 > > > =C2=A0 if (has_panel) { > > > =C2=A0 final |=3D DREF_SSC_SOURCE_ENABLE; > > > @@ -8348,7 +8367,7 @@ static void ironlake_init_pch_refclk(struct > > > drm_device *dev) > > > =C2=A0 POSTING_READ(PCH_DREF_CONTROL); > > > =C2=A0 udelay(200); > > > =C2=A0 } else { > > > - DRM_DEBUG_KMS("Disabling SSC entirely\n"); > > > + DRM_DEBUG_KMS("Disabling CPU source output\n"); > > > =C2=A0 > > > =C2=A0 val &=3D ~DREF_CPU_SOURCE_OUTPUT_MASK; > > > =C2=A0 > > > @@ -8359,16 +8378,20 @@ static void ironlake_init_pch_refclk(stru= ct > > > drm_device *dev) > > > =C2=A0 POSTING_READ(PCH_DREF_CONTROL); > > > =C2=A0 udelay(200); > > > =C2=A0 > > > - /* Turn off the SSC source */ > > > - val &=3D ~DREF_SSC_SOURCE_MASK; > > > - val |=3D DREF_SSC_SOURCE_DISABLE; > > > + if (!using_ssc_source) { > > > + DRM_DEBUG_KMS("Disabling SSC source\n"); > > > =C2=A0 > > > - /* Turn off SSC1 */ > > > - val &=3D ~DREF_SSC1_ENABLE; > > > + /* Turn off the SSC source */ > > > + val &=3D ~DREF_SSC_SOURCE_MASK; > > > + val |=3D DREF_SSC_SOURCE_DISABLE; > > > =C2=A0 > > > - I915_WRITE(PCH_DREF_CONTROL, val); > > > - POSTING_READ(PCH_DREF_CONTROL); > > > - udelay(200); > > > + /* Turn off SSC1 */ > > > + val &=3D ~DREF_SSC1_ENABLE; > > > + > > > + I915_WRITE(PCH_DREF_CONTROL, val); > > > + POSTING_READ(PCH_DREF_CONTROL); > > > + udelay(200); > > > + } > > > =C2=A0 } > > > =C2=A0 > > > =C2=A0 BUG_ON(val !=3D final); > > > --=C2=A0 > > > 2.5.5 > > >=20 > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > --=C2=A0 > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch