On 10/08/2015 05:54 PM, Ville Syrjälä wrote:
On Thu, Oct 08, 2015 at 05:43:41PM +0530, Kumar, Shobhit wrote:
On 10/08/2015 04:59 PM, Imre Deak wrote:
On to, 2015-10-08 at 09:58 +0530, Shobhit Kumar wrote:
Reuse what is programmed by pre-os, but in case there is no pre-os
initialization, init the cdclk with the max value by default untill
dynamic cdclk support comes.
v2: Check if BIOS programmed correctly rather than always calling init
- Do validation of programmed cdctl and what it is expected
- Only do slk_init_cdclk if validation failed else reuse BIOS
programmed value
Cc: Imre Deak <imre.deak@xxxxxxxxx>
Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_ddi.c | 18 ++++++++++++-----
drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++---------
2 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2d3cc82..3ec5618 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2946,11 +2946,19 @@ void intel_ddi_pll_init(struct drm_device *dev)
int cdclk_freq;
cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
- dev_priv->skl_boot_cdclk = cdclk_freq;
- if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
- DRM_ERROR("LCPLL1 is disabled\n");
- else
- intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+
+ /* Invalid CDCLK from BIOS ? */
+ if (cdclk_freq < 0) {
+ /* program to maximum cdclk till we have dynamic cdclk support */
+ dev_priv->skl_boot_cdclk = 675000;
+ skl_init_cdclk(dev_priv);
This would still try to reprogram CDCLK if BIOS enabled an output with
an incorrect CDCLK decimal part. Isn't this the exact scenario you're
seeing? As said before reprogramming CDCLK in that case would require
disabling the outputs first.
I went with the hypothesis that if VBIOS/GOP was loaded it would have to
correct the cdclock, with wrong decimal value display cannot be enabled.
For example AUX will fail on SKL. So for correct display output enabled
cdclock has to be correctly programmed. If it is wrong display was not
enabled at all.
The scenario which I am seeing is VBIOS/GOP is not loaded at all, and
the pre-os is not leaving cdclock in correct state.
But it still enabled DPLL0? Why does it do that if it doesn't set up
cdclk?
So I had discussion with some folks in BIOS team and came to know that
on some boards even if higher cdclocks are supported, we might want to
limit maximum cdclock. This is done by BIOS programming the cdclock to
allowed maximum and VBIOS/GOP uses that as the max value rather than
the real possible max. So the BIOS is supposed to program cdclock
correctly and hence DPLL0 as well. My only assumption is in this case it
has not correctly programmed due to some bug. I know if I work with them
and ensure that BIOS programs correctly, the problem will be solved, but
do you think it is worth while to remove the tight dependency altogether ?
We would also need to call skl_init_cdclk if the BIOS hasn't enabled the
PLL for some reason. But I guess that could be a separate change.
+ } else {
+ dev_priv->skl_boot_cdclk = cdclk_freq;
+ if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
+ DRM_ERROR("LCPLL1 is disabled\n");
+ else
+ intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+ }
} else if (IS_BROXTON(dev)) {
broxton_init_cdclk(dev);
broxton_ddi_phy_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bbeb6d3..f734410 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6634,12 +6634,15 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
uint32_t cdctl = I915_READ(CDCLK_CTL);
uint32_t linkrate;
+ int freq;
if (!(lcpll1 & LCPLL_PLL_ENABLE))
return 24000; /* 24MHz is the cd freq with NSSC ref */
- if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540)
- return 540000;
+ if ((cdctl & CDCLK_FREQ_SEL_MASK) == CDCLK_FREQ_540) {
+ freq = 540000;
+ goto verify;
+ }
linkrate = (I915_READ(DPLL_CTRL1) &
DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) >> 1;
@@ -6649,30 +6652,46 @@ static int skylake_get_display_clock_speed(struct drm_device *dev)
/* vco 8640 */
switch (cdctl & CDCLK_FREQ_SEL_MASK) {
case CDCLK_FREQ_450_432:
- return 432000;
+ freq = 432000;
+ break;
case CDCLK_FREQ_337_308:
- return 308570;
+ freq = 308570;
+ break;
case CDCLK_FREQ_675_617:
- return 617140;
+ freq = 617140;
+ break;
default:
WARN(1, "Unknown cd freq selection\n");
+ return -EINVAL;
}
} else {
/* vco 8100 */
switch (cdctl & CDCLK_FREQ_SEL_MASK) {
case CDCLK_FREQ_450_432:
- return 450000;
+ freq = 450000;
+ break;
case CDCLK_FREQ_337_308:
- return 337500;
+ freq = 337500;
+ break;
case CDCLK_FREQ_675_617:
- return 675000;
+ freq = 675000;
+ break;
default:
WARN(1, "Unknown cd freq selection\n");
+ return -EINVAL;
}
}
- /* error case, do as if DPLL0 isn't enabled */
- return 24000;
+verify:
+ /*
+ * Noticed in some instances that the freq selection is correct but
+ * decimal part is programmed wrong from BIOS where pre-os does not
+ * enable display. Verify the same as well.
+ */
+ if (cdctl == ((cdctl & CDCLK_FREQ_SEL_MASK) | skl_cdclk_decimal(freq)))
+ return freq;
+ else
+ return -EINVAL;
}
static int broxton_get_display_clock_speed(struct drm_device *dev)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
