On 10/06/2015 05:49 PM, Daniel Vetter wrote:
On Tue, Oct 06, 2015 at 02:41:44PM +0300, Ville Syrjälä wrote:
On Tue, Oct 06, 2015 at 01:19:52PM +0200, Daniel Vetter wrote:
On Tue, Oct 06, 2015 at 04:33:43PM +0530, Kumar, Shobhit wrote:
On 10/06/2015 04:11 PM, Imre Deak wrote:
On ti, 2015-10-06 at 15:26 +0530, Kumar, Shobhit wrote:
On 10/05/2015 09:05 PM, Imre Deak wrote:
On ma, 2015-10-05 at 20:52 +0530, Shobhit Kumar wrote:
Mostly reuse what is programmed by pre-os, but in case there is no
pre-os initialization, init the cdclk with the default value.
Cc: Imre Deak <imre.deak@xxxxxxxxx>
Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_ddi.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2d3cc82..675c60d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2947,10 +2947,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
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);
+
+ skl_init_cdclk(dev_priv);
How does this prevent changing the clock if BIOS did enable some output?
We shouldn't change the clock in that case.
In that case it will try to re-apply the same clock that BIOS enabled.
Not sure if this is allowed, but I checked the cdclock change sequence
and it is mostly followed in skl_init_cdclk.
In my tests where BIOS does enable this, I faced no issues in
initializing again in driver.
The first step in that sequence:
"Disable all display engine functions using the full mode set disable
sequence on all pipes, ports, and planes."
Oh, yeah, I again made mistake of assuming that display is not enabled in
the first place. You are right, though it works if I change the clock again.
So the problem is not that the PLL itself may be enabled here (as BIOS
left it), but that some output is also enabled.
Yes.
I have noticed on some pre-os this value is programmed correctly except
for the decimal part. That causes AUX transactions to fail on SKl. That
is what triggered this patch actually. So other way is to completely
validate the value in get_display_clock_speed instead of bit[28:26] and
then if wrong then only do the cdclk init.
I think we'd need to detect at this point if outputs are enabled and
only attempt to work around the above BIOS problem if this is not the
case. Alternatively you could also disable the active outputs as a first
step.
Ok, let me detect if any output is enabled by BIOS and accordingly
initialize cdclk.
These kind of fixiups should be done after the hw state readout. We
already have sanitize_crtc/pll/encoder functions, probably best if we add
a sanitize_cdclk or similar for this at the very end of the hw state
sanitize sequence.
Can't be done if we already need a somewhat sane cdclk for the
eDP AUX probing and whatnot.
For actually enabling the cdclk for pushing pixels, we wouldn't need
to do anything except actually plug ia a calc_cdclk for SKL. No idea
why we're not doing that currently. Some extra care may be needed
due to the eDP DPLL0 usag IIRC.
Hm right, cdlck is in the top-level power domain. Added fun is that with
dmc the firmware is supposed to handle it. Messy :(
Yes, exactly. How about just adding verify_cdclk and calling in
get_display_clock_speed to check if cdclk is programmed correctly along
with related DPLL0 VCO settings for now. If all looks good, then skip
else initialize. Now in that case if we have to initialize where do we
get the cdclock to initialize with at this point ? Any default in VBT ?
Or go with minimum by default and it can be bumped up later if needed.
Regards
Shobhit
-Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
