| Message ID | 1360149028-13531-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 06, 2013 at 11:10:21AM +0000, Chris Wilson wrote: > Modifying the clock sources (via the DREF control on the PCH) is a slow > multi-stage process as we need to let the clocks stabilise between each > stage. If we are not actually changing the clock sources, then we can > return early. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> One idea I've been pondering for the refclock stuff is to simply shovel this into the ->modeset_global_resources callback. That way no clever tricks are required, and we'd avoid this for all cases where fastboot works complety. Presumably ofc that the bios didn't set up a config which does not work. As a bonus, we could only set up the refclocks the new configuration actually needs, i.e. filter for encoder->crtc != NULL ... A bit a risky patch, but might uncover some subtle bugs if we do the refclock adjusting more often. Too insane? -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++--------- > 1 file changed, 61 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d75c6a0..f1dbd01 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_mode_config *mode_config = &dev->mode_config; > struct intel_encoder *encoder; > - u32 temp; > + u32 val, final; > bool has_lvds = false; > bool has_cpu_edp = false; > bool has_pch_edp = false; > @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > * PCH B stepping, previous chipset stepping should be > * ignoring this setting. > */ > - temp = I915_READ(PCH_DREF_CONTROL); > + val = I915_READ(PCH_DREF_CONTROL); > + > + /* As we must carefully and slowly disable/enable each source in turn, > + * compute the final state we want first and check if we need to > + * make any changes at all. > + */ > + final = val; > + final &= ~DREF_NONSPREAD_SOURCE_MASK; > + if (has_ck505) > + final |= DREF_NONSPREAD_CK505_ENABLE; > + else > + final |= DREF_NONSPREAD_SOURCE_ENABLE; > + > + final &= ~DREF_SSC_SOURCE_MASK; > + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + final &= ~DREF_SSC1_ENABLE; > + > + if (has_panel) { > + final |= DREF_SSC_SOURCE_ENABLE; > + > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > + final |= DREF_SSC1_ENABLE; > + > + if (has_cpu_edp) { > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > + else > + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > + } else > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + } else { > + final |= DREF_SSC_SOURCE_DISABLE; > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + } > + > + if (final == val) > + return; > + > /* Always enable nonspread source */ > - temp &= ~DREF_NONSPREAD_SOURCE_MASK; > + val &= ~DREF_NONSPREAD_SOURCE_MASK; > > if (has_ck505) > - temp |= DREF_NONSPREAD_CK505_ENABLE; > + val |= DREF_NONSPREAD_CK505_ENABLE; > else > - temp |= DREF_NONSPREAD_SOURCE_ENABLE; > + val |= DREF_NONSPREAD_SOURCE_ENABLE; > > if (has_panel) { > - temp &= ~DREF_SSC_SOURCE_MASK; > - temp |= DREF_SSC_SOURCE_ENABLE; > + val &= ~DREF_SSC_SOURCE_MASK; > + val |= DREF_SSC_SOURCE_ENABLE; > > /* SSC must be turned on before enabling the CPU output */ > if (intel_panel_use_ssc(dev_priv) && can_ssc) { > DRM_DEBUG_KMS("Using SSC on panel\n"); > - temp |= DREF_SSC1_ENABLE; > + val |= DREF_SSC1_ENABLE; > } else > - temp &= ~DREF_SSC1_ENABLE; > + val &= ~DREF_SSC1_ENABLE; > > /* Get SSC going before enabling the outputs */ > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > > - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > /* Enable CPU source on CPU attached eDP */ > if (has_cpu_edp) { > if (intel_panel_use_ssc(dev_priv) && can_ssc) { > DRM_DEBUG_KMS("Using SSC on eDP\n"); > - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > } > else > - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > } else > - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > } else { > DRM_DEBUG_KMS("Disabling SSC entirely\n"); > > - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > /* Turn off CPU output */ > - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > > /* Turn off the SSC source */ > - temp &= ~DREF_SSC_SOURCE_MASK; > - temp |= DREF_SSC_SOURCE_DISABLE; > + val &= ~DREF_SSC_SOURCE_MASK; > + val |= DREF_SSC_SOURCE_DISABLE; > > /* Turn off SSC1 */ > - temp &= ~ DREF_SSC1_ENABLE; > + val &= ~ DREF_SSC1_ENABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > } > + > + BUG_ON(val != final); > } > > /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */ > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi 2013/2/6 Chris Wilson <chris@chris-wilson.co.uk>: > Modifying the clock sources (via the DREF control on the PCH) is a slow > multi-stage process as we need to let the clocks stabilise between each > stage. If we are not actually changing the clock sources, then we can > return early. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> The patch looks correct, so: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> But this patch only saves 400us. I thought this was on Daniel's "don't care" range (e.g., on "drm/i915: don't send DP "idle" pattern before "normal" on HSW PORT_A" I was asked to keep a potentially bigger delay that's not needed at all). > --- > drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++--------- > 1 file changed, 61 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d75c6a0..f1dbd01 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_mode_config *mode_config = &dev->mode_config; > struct intel_encoder *encoder; > - u32 temp; > + u32 val, final; > bool has_lvds = false; > bool has_cpu_edp = false; > bool has_pch_edp = false; > @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > * PCH B stepping, previous chipset stepping should be > * ignoring this setting. > */ > - temp = I915_READ(PCH_DREF_CONTROL); > + val = I915_READ(PCH_DREF_CONTROL); > + > + /* As we must carefully and slowly disable/enable each source in turn, > + * compute the final state we want first and check if we need to > + * make any changes at all. > + */ > + final = val; > + final &= ~DREF_NONSPREAD_SOURCE_MASK; > + if (has_ck505) > + final |= DREF_NONSPREAD_CK505_ENABLE; > + else > + final |= DREF_NONSPREAD_SOURCE_ENABLE; > + > + final &= ~DREF_SSC_SOURCE_MASK; > + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + final &= ~DREF_SSC1_ENABLE; > + > + if (has_panel) { > + final |= DREF_SSC_SOURCE_ENABLE; > + > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > + final |= DREF_SSC1_ENABLE; > + > + if (has_cpu_edp) { > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > + else > + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > + } else > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + } else { > + final |= DREF_SSC_SOURCE_DISABLE; > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + } > + > + if (final == val) > + return; > + > /* Always enable nonspread source */ > - temp &= ~DREF_NONSPREAD_SOURCE_MASK; > + val &= ~DREF_NONSPREAD_SOURCE_MASK; > > if (has_ck505) > - temp |= DREF_NONSPREAD_CK505_ENABLE; > + val |= DREF_NONSPREAD_CK505_ENABLE; > else > - temp |= DREF_NONSPREAD_SOURCE_ENABLE; > + val |= DREF_NONSPREAD_SOURCE_ENABLE; > > if (has_panel) { > - temp &= ~DREF_SSC_SOURCE_MASK; > - temp |= DREF_SSC_SOURCE_ENABLE; > + val &= ~DREF_SSC_SOURCE_MASK; > + val |= DREF_SSC_SOURCE_ENABLE; > > /* SSC must be turned on before enabling the CPU output */ > if (intel_panel_use_ssc(dev_priv) && can_ssc) { > DRM_DEBUG_KMS("Using SSC on panel\n"); > - temp |= DREF_SSC1_ENABLE; > + val |= DREF_SSC1_ENABLE; > } else > - temp &= ~DREF_SSC1_ENABLE; > + val &= ~DREF_SSC1_ENABLE; > > /* Get SSC going before enabling the outputs */ > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > > - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > /* Enable CPU source on CPU attached eDP */ > if (has_cpu_edp) { > if (intel_panel_use_ssc(dev_priv) && can_ssc) { > DRM_DEBUG_KMS("Using SSC on eDP\n"); > - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > } > else > - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > } else > - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > } else { > DRM_DEBUG_KMS("Disabling SSC entirely\n"); > > - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > /* Turn off CPU output */ > - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > > /* Turn off the SSC source */ > - temp &= ~DREF_SSC_SOURCE_MASK; > - temp |= DREF_SSC_SOURCE_DISABLE; > + val &= ~DREF_SSC_SOURCE_MASK; > + val |= DREF_SSC_SOURCE_DISABLE; > > /* Turn off SSC1 */ > - temp &= ~ DREF_SSC1_ENABLE; > + val &= ~ DREF_SSC1_ENABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > } > + > + BUG_ON(val != final); > } > > /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */ > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Feb 7, 2013 at 1:45 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > 2013/2/6 Chris Wilson <chris@chris-wilson.co.uk>: >> Modifying the clock sources (via the DREF control on the PCH) is a slow >> multi-stage process as we need to let the clocks stabilise between each >> stage. If we are not actually changing the clock sources, then we can >> return early. >> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > The patch looks correct, so: > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > But this patch only saves 400us. I thought this was on Daniel's "don't > care" range (e.g., on "drm/i915: don't send DP "idle" pattern before > "normal" on HSW PORT_A" I was asked to keep a potentially bigger delay > that's not needed at all). For modeset operations it's on my don't care list, since that'll take much longer anyway. But this tries to avoid it in boot-up, where fastboot aims to completely avoid the modeset. And I'm toying around with pushing all the edid reading to workqueues. So this could very much be in the "I care" bucket ;-) Hence also my proposal to simply push this into the modeset sequence instead of making the code more complicated ... -Daniel
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote: > Modifying the clock sources (via the DREF control on the PCH) is a slow > multi-stage process as we need to let the clocks stabilise between each > stage. If we are not actually changing the clock sources, then we can > return early. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++--------- > 1 file changed, 61 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d75c6a0..f1dbd01 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_mode_config *mode_config = &dev->mode_config; > struct intel_encoder *encoder; > - u32 temp; > + u32 val, final; > bool has_lvds = false; > bool has_cpu_edp = false; > bool has_pch_edp = false; > @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) > * PCH B stepping, previous chipset stepping should be > * ignoring this setting. > */ > - temp = I915_READ(PCH_DREF_CONTROL); > + val = I915_READ(PCH_DREF_CONTROL); > + > + /* As we must carefully and slowly disable/enable each source in turn, > + * compute the final state we want first and check if we need to > + * make any changes at all. > + */ > + final = val; > + final &= ~DREF_NONSPREAD_SOURCE_MASK; > + if (has_ck505) > + final |= DREF_NONSPREAD_CK505_ENABLE; > + else > + final |= DREF_NONSPREAD_SOURCE_ENABLE; > + > + final &= ~DREF_SSC_SOURCE_MASK; > + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + final &= ~DREF_SSC1_ENABLE; > + > + if (has_panel) { > + final |= DREF_SSC_SOURCE_ENABLE; > + > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > + final |= DREF_SSC1_ENABLE; > + > + if (has_cpu_edp) { > + if (intel_panel_use_ssc(dev_priv) && can_ssc) > + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > + else > + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > + } else > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + } else { > + final |= DREF_SSC_SOURCE_DISABLE; > + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + } > + > + if (final == val) > + return; > + Nitpick: this would be clearer in a seperate function with a 'check_only' param, then the below could be removed. > /* Always enable nonspread source */ > - temp &= ~DREF_NONSPREAD_SOURCE_MASK; > + val &= ~DREF_NONSPREAD_SOURCE_MASK; > > if (has_ck505) > - temp |= DREF_NONSPREAD_CK505_ENABLE; > + val |= DREF_NONSPREAD_CK505_ENABLE; > else > - temp |= DREF_NONSPREAD_SOURCE_ENABLE; > + val |= DREF_NONSPREAD_SOURCE_ENABLE; > > if (has_panel) { > - temp &= ~DREF_SSC_SOURCE_MASK; > - temp |= DREF_SSC_SOURCE_ENABLE; > + val &= ~DREF_SSC_SOURCE_MASK; > + val |= DREF_SSC_SOURCE_ENABLE; > > /* SSC must be turned on before enabling the CPU output */ > if (intel_panel_use_ssc(dev_priv) && can_ssc) { > DRM_DEBUG_KMS("Using SSC on panel\n"); > - temp |= DREF_SSC1_ENABLE; > + val |= DREF_SSC1_ENABLE; > } else > - temp &= ~DREF_SSC1_ENABLE; > + val &= ~DREF_SSC1_ENABLE; > > /* Get SSC going before enabling the outputs */ > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > > - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > /* Enable CPU source on CPU attached eDP */ > if (has_cpu_edp) { > if (intel_panel_use_ssc(dev_priv) && can_ssc) { > DRM_DEBUG_KMS("Using SSC on eDP\n"); > - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; > } > else > - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; > } else > - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > } else { > DRM_DEBUG_KMS("Disabling SSC entirely\n"); > > - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; > > /* Turn off CPU output */ > - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > > /* Turn off the SSC source */ > - temp &= ~DREF_SSC_SOURCE_MASK; > - temp |= DREF_SSC_SOURCE_DISABLE; > + val &= ~DREF_SSC_SOURCE_MASK; > + val |= DREF_SSC_SOURCE_DISABLE; > > /* Turn off SSC1 */ > - temp &= ~ DREF_SSC1_ENABLE; > + val &= ~ DREF_SSC1_ENABLE; > > - I915_WRITE(PCH_DREF_CONTROL, temp); > + I915_WRITE(PCH_DREF_CONTROL, val); > POSTING_READ(PCH_DREF_CONTROL); > udelay(200); > } > + > + BUG_ON(val != final); > } > > /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d75c6a0..f1dbd01 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct drm_mode_config *mode_config = &dev->mode_config; struct intel_encoder *encoder; - u32 temp; + u32 val, final; bool has_lvds = false; bool has_cpu_edp = false; bool has_pch_edp = false; @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) * PCH B stepping, previous chipset stepping should be * ignoring this setting. */ - temp = I915_READ(PCH_DREF_CONTROL); + val = I915_READ(PCH_DREF_CONTROL); + + /* As we must carefully and slowly disable/enable each source in turn, + * compute the final state we want first and check if we need to + * make any changes at all. + */ + final = val; + final &= ~DREF_NONSPREAD_SOURCE_MASK; + if (has_ck505) + final |= DREF_NONSPREAD_CK505_ENABLE; + else + final |= DREF_NONSPREAD_SOURCE_ENABLE; + + final &= ~DREF_SSC_SOURCE_MASK; + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK; + final &= ~DREF_SSC1_ENABLE; + + if (has_panel) { + final |= DREF_SSC_SOURCE_ENABLE; + + if (intel_panel_use_ssc(dev_priv) && can_ssc) + final |= DREF_SSC1_ENABLE; + + if (has_cpu_edp) { + if (intel_panel_use_ssc(dev_priv) && can_ssc) + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; + else + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; + } else + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; + } else { + final |= DREF_SSC_SOURCE_DISABLE; + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE; + } + + if (final == val) + return; + /* Always enable nonspread source */ - temp &= ~DREF_NONSPREAD_SOURCE_MASK; + val &= ~DREF_NONSPREAD_SOURCE_MASK; if (has_ck505) - temp |= DREF_NONSPREAD_CK505_ENABLE; + val |= DREF_NONSPREAD_CK505_ENABLE; else - temp |= DREF_NONSPREAD_SOURCE_ENABLE; + val |= DREF_NONSPREAD_SOURCE_ENABLE; if (has_panel) { - temp &= ~DREF_SSC_SOURCE_MASK; - temp |= DREF_SSC_SOURCE_ENABLE; + val &= ~DREF_SSC_SOURCE_MASK; + val |= DREF_SSC_SOURCE_ENABLE; /* SSC must be turned on before enabling the CPU output */ if (intel_panel_use_ssc(dev_priv) && can_ssc) { DRM_DEBUG_KMS("Using SSC on panel\n"); - temp |= DREF_SSC1_ENABLE; + val |= DREF_SSC1_ENABLE; } else - temp &= ~DREF_SSC1_ENABLE; + val &= ~DREF_SSC1_ENABLE; /* Get SSC going before enabling the outputs */ - I915_WRITE(PCH_DREF_CONTROL, temp); + I915_WRITE(PCH_DREF_CONTROL, val); POSTING_READ(PCH_DREF_CONTROL); udelay(200); - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; /* Enable CPU source on CPU attached eDP */ if (has_cpu_edp) { if (intel_panel_use_ssc(dev_priv) && can_ssc) { DRM_DEBUG_KMS("Using SSC on eDP\n"); - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD; } else - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD; } else - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; - I915_WRITE(PCH_DREF_CONTROL, temp); + I915_WRITE(PCH_DREF_CONTROL, val); POSTING_READ(PCH_DREF_CONTROL); udelay(200); } else { DRM_DEBUG_KMS("Disabling SSC entirely\n"); - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK; + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK; /* Turn off CPU output */ - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE; + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE; - I915_WRITE(PCH_DREF_CONTROL, temp); + I915_WRITE(PCH_DREF_CONTROL, val); POSTING_READ(PCH_DREF_CONTROL); udelay(200); /* Turn off the SSC source */ - temp &= ~DREF_SSC_SOURCE_MASK; - temp |= DREF_SSC_SOURCE_DISABLE; + val &= ~DREF_SSC_SOURCE_MASK; + val |= DREF_SSC_SOURCE_DISABLE; /* Turn off SSC1 */ - temp &= ~ DREF_SSC1_ENABLE; + val &= ~ DREF_SSC1_ENABLE; - I915_WRITE(PCH_DREF_CONTROL, temp); + I915_WRITE(PCH_DREF_CONTROL, val); POSTING_READ(PCH_DREF_CONTROL); udelay(200); } + + BUG_ON(val != final); } /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
Modifying the clock sources (via the DREF control on the PCH) is a slow multi-stage process as we need to let the clocks stabilise between each stage. If we are not actually changing the clock sources, then we can return early. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 22 deletions(-)