* Re: [PATCH v2] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-05-24 13:14 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2016-05-24 13:14 UTC (permalink / raw)
To: Lyude
Cc: intel-gfx, open list:INTEL DRM DRIVERS (excluding Poulsbo,
Moorestow...), linux-kernel@vger.kernel.org (open list),
Daniel Vetter, stable
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@vger.kernel.org
> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2] drm/i915/ilk: Don't disable SSC source if it's in use
2016-05-24 13:14 ` Ville Syrjälä
(?)
@ 2016-05-24 14:08 ` Lyude Paul
-1 siblings, 0 replies; 23+ messages in thread
From: Lyude Paul @ 2016-05-24 14:08 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, open list:INTEL DRM DRIVERS (excluding Poulsbo,
Moorestow...), linux-kernel@vger.kernel.org (open list),
Daniel Vetter, stable
Huh… so I'm going to have to take back what I said before about this. On further
observation, it looks like the "Don't disable SSC source if it's in use" patch
actually got rid of the underruns entirely, with or without the wait for a
vblank here.
On Tue, 2016-05-24 at 16:14 +0300, Ville Syrjälä wrote:
> 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@vger.kernel.org
> > Signed-off-by: Lyude <cpaul@redhat.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v3] drm/i915/ilk: Don't disable SSC source if it's in use
2016-05-24 13:14 ` Ville Syrjälä
@ 2016-05-24 14:27 ` Lyude
-1 siblings, 0 replies; 23+ messages in thread
From: Lyude @ 2016-05-24 14:27 UTC (permalink / raw)
To: intel-gfx, Ville Syrjälä
Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie,
open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
linux-kernel@vger.kernel.org (open list)
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 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
Signed-off-by: Lyude <cpaul@redhat.com>
---
As for the diagram you mentioned feel free to add that since you probably have
a better understanding of how that works then I do ;)
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..4937b68 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 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-05-24 14:27 ` Lyude
0 siblings, 0 replies; 23+ messages in thread
From: Lyude @ 2016-05-24 14:27 UTC (permalink / raw)
To: intel-gfx, Ville Syrjälä
Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list
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 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
Signed-off-by: Lyude <cpaul@redhat.com>
---
As for the diagram you mentioned feel free to add that since you probably have
a better understanding of how that works then I do ;)
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..4937b68 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 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3] drm/i915/ilk: Don't disable SSC source if it's in use
2016-05-24 14:27 ` Lyude
@ 2016-05-24 18:59 ` Ville Syrjälä
-1 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2016-05-24 18:59 UTC (permalink / raw)
To: Lyude
Cc: intel-gfx, stable, Daniel Vetter, Jani Nikula, David Airlie,
open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
linux-kernel@vger.kernel.org (open list)
On Tue, May 24, 2016 at 10:27:03AM -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 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
> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
> As for the diagram you mentioned feel free to add that since you probably have
> a better understanding of how that works then I do ;)
>
> 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..4937b68 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;
'temp' is not needed outside the loop, so can be moved there.
> 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;
> + }
> + }
Can you move this code a bit earlier so that it's not mixed in with the
code that figures out 'final'. I'd suggest doing it right after figuring
out has_ck505/can_ssc. Might even advertize 'using_ssc_source' alongside
the other bools in the debug print.
With those this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> +
> + 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
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-05-24 18:59 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2016-05-24 18:59 UTC (permalink / raw)
To: Lyude
Cc: David Airlie, intel-gfx,
open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
linux-kernel@vger.kernel.org (open list),
Daniel Vetter, stable
On Tue, May 24, 2016 at 10:27:03AM -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 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
> Signed-off-by: Lyude <cpaul@redhat.com>
> ---
> As for the diagram you mentioned feel free to add that since you probably have
> a better understanding of how that works then I do ;)
>
> 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..4937b68 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;
'temp' is not needed outside the loop, so can be moved there.
> 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;
> + }
> + }
Can you move this code a bit earlier so that it's not mixed in with the
code that figures out 'final'. I'd suggest doing it right after figuring
out has_ck505/can_ssc. Might even advertize 'using_ssc_source' alongside
the other bools in the debug print.
With those this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> +
> + 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
2016-05-24 18:59 ` Ville Syrjälä
@ 2016-05-25 18:11 ` Lyude
-1 siblings, 0 replies; 23+ messages in thread
From: Lyude @ 2016-05-25 18:11 UTC (permalink / raw)
To: Ville Syrjälä, intel-gfx
Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie,
open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...),
linux-kernel@vger.kernel.org (open list)
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>
---
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-05-25 18:11 ` Lyude
0 siblings, 0 replies; 23+ messages in thread
From: Lyude @ 2016-05-25 18:11 UTC (permalink / raw)
To: Ville Syrjälä, intel-gfx
Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list
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>
---
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
2016-05-25 18:11 ` Lyude
@ 2016-05-26 7:54 ` Daniel Vetter
-1 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2016-05-26 7:54 UTC (permalink / raw)
To: Lyude
Cc: Ville Syrjälä,
intel-gfx, David Airlie, stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, Daniel Vetter
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.
-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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-05-26 7:54 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2016-05-26 7:54 UTC (permalink / raw)
To: Lyude
Cc: David Airlie, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, stable, Daniel Vetter
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.
-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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
2016-05-26 7:54 ` Daniel Vetter
(?)
@ 2016-06-06 11:30 ` Ville Syrjälä
2016-06-06 19:29 ` Lyude Paul
-1 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjälä @ 2016-06-06 11:30 UTC (permalink / raw)
To: Daniel Vetter
Cc: Lyude, intel-gfx, David Airlie, stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, Daniel Vetter
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
2016-06-06 11:30 ` [Intel-gfx] " Ville Syrjälä
@ 2016-06-06 19:29 ` Lyude Paul
2016-06-07 7:02 ` Jani Nikula
0 siblings, 1 reply; 23+ messages in thread
From: Lyude Paul @ 2016-06-06 19:29 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: intel-gfx, David Airlie, stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, Daniel Vetter
On Mon, 2016-06-06 at 14:30 +0300, Ville Syrjälä wrote:
> 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().
>
Managed to find a machine at the office here that I could reproduce this 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 charge of it
just went on PTO until the end of the week. For the time being don't merge this
patch until I can get access to that machine again and make sure the fixed
version of this patch still works on the 990. I'll send the new version once I'm
able to test it.
Cheers,
Lyude
>
> >
> > -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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
2016-06-06 19:29 ` Lyude Paul
@ 2016-06-07 7:02 ` Jani Nikula
0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2016-06-07 7:02 UTC (permalink / raw)
To: Lyude Paul, Ville Syrjälä, Daniel Vetter
Cc: Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, stable
On Mon, 06 Jun 2016, Lyude Paul <cpaul@redhat.com> wrote:
> On Mon, 2016-06-06 at 14:30 +0300, Ville Syrjälä wrote:
>> On Thu, May 26, 2016 at 09:54:56AM +0200, Daniel Vetter wrote:
>> >
>> > 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().
>>
> Managed to find a machine at the office here that I could reproduce this 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 charge of it
> just went on PTO until the end of the week. For the time being don't merge this
> patch until I can get access to that machine again and make sure the fixed
> version of this patch still works on the 990. I'll send the new version once I'm
> able to test it.
Daniel already merged it, should we revert?
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-06-07 7:02 ` Jani Nikula
0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2016-06-07 7:02 UTC (permalink / raw)
To: Lyude Paul, Ville Syrjälä, Daniel Vetter
Cc: Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, stable
On Mon, 06 Jun 2016, Lyude Paul <cpaul@redhat.com> wrote:
> On Mon, 2016-06-06 at 14:30 +0300, Ville Syrjälä wrote:
>> On Thu, May 26, 2016 at 09:54:56AM +0200, Daniel Vetter wrote:
>> >
>> > 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().
>>
> Managed to find a machine at the office here that I could reproduce this 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 charge of it
> just went on PTO until the end of the week. For the time being don't merge this
> patch until I can get access to that machine again and make sure the fixed
> version of this patch still works on the 990. I'll send the new version once I'm
> able to test it.
Daniel already merged it, should we revert?
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
2016-06-07 7:02 ` Jani Nikula
@ 2016-06-07 19:37 ` Daniel Vetter
-1 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2016-06-07 19:37 UTC (permalink / raw)
To: Jani Nikula
Cc: Lyude Paul, Ville Syrjälä,
Daniel Vetter, Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, stable
On Tue, Jun 07, 2016 at 10:02:35AM +0300, Jani Nikula wrote:
> On Mon, 06 Jun 2016, Lyude Paul <cpaul@redhat.com> wrote:
> > On Mon, 2016-06-06 at 14:30 +0300, Ville Syrj�l� wrote:
> >> On Thu, May 26, 2016 at 09:54:56AM +0200, Daniel Vetter wrote:
> >> >
> >> > 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().
> >>
> > Managed to find a machine at the office here that I could reproduce this 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 charge of it
> > just went on PTO until the end of the week. For the time being don't merge this
> > patch until I can get access to that machine again and make sure the fixed
> > version of this patch still works on the 990. I'll send the new version once I'm
> > able to test it.
>
> Daniel already merged it, should we revert?
Oops, can do. Or if I have a fixup right away we can merge that, too.
Waiting otoh isn't cool. Lyude?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/ilk: Don't disable SSC source if it's in use
@ 2016-06-07 19:37 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2016-06-07 19:37 UTC (permalink / raw)
To: Jani Nikula
Cc: Lyude Paul, Ville Syrjälä,
Daniel Vetter, Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow...,
linux-kernel@vger.kernel.org open list, stable
On Tue, Jun 07, 2016 at 10:02:35AM +0300, Jani Nikula wrote:
> On Mon, 06 Jun 2016, Lyude Paul <cpaul@redhat.com> wrote:
> > On Mon, 2016-06-06 at 14:30 +0300, Ville Syrjälä wrote:
> >> On Thu, May 26, 2016 at 09:54:56AM +0200, Daniel Vetter wrote:
> >> >
> >> > 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().
> >>
> > Managed to find a machine at the office here that I could reproduce this 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 charge of it
> > just went on PTO until the end of the week. For the time being don't merge this
> > patch until I can get access to that machine again and make sure the fixed
> > version of this patch still works on the 990. I'll send the new version once I'm
> > able to test it.
>
> Daniel already merged it, should we revert?
Oops, can do. Or if I have a fixup right away we can merge that, too.
Waiting otoh isn't cool. Lyude?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 23+ messages in thread