Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Wed, 21 May 2014 20:54:03 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: + if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable) + deassert cmnreset } If those measures aren't enough, then I think this patch would be OK. But right now I have this nagging feeling that we may have shot ourselves in the foot and we're about to pile workarounds on top of our own bug. I'll add these changes, with the additon that we need to do an actual toggle at boot and init. Simply asserting in suspend or gate and then de-asserting isn't enough to re-calibrate, since we tried that on the Chromebooks across suspend/resume and it wasn't enough to get things going again after the chip had lost power. Across simple power well transitions it ought to be enough though, and that's easy enough to verify in testing. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Wed, 2014-05-21 at 21:43 +0300, Ville Syrjälä wrote: On Wed, May 21, 2014 at 11:11:15AM -0700, Jesse Barnes wrote: And to answer more specifically... On Wed, 21 May 2014 20:54:03 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, +false); + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, +true); Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in with this. We should definitely rip that out regardless. Yeah we can rip that out. That's just an ungate, and it assumes the BIOS has already done the reset toggle for us. Well I'm assumign the system may boot/resume with the wells powered down. And we definitely don't have the cri/ref clocks set up at this point. So if they're needed to complete the resets the PHY may get stuck or something. Also it just ungates everything at once, if the wells need some ordering in bringup we have just messed things up here. I agree, there doesn't seem to be anything that needs those power wells until the power domain init code runs. I can send a patch for this after some testing. Another thing I'm wodering is did the BIOS/hw really power on the common lane, or did we do that outselves? If the latter, then I wonder if we simply do that too early. Or more precisely do we need to make sure the cri clock and/or refclock are enabled before we power on the common lane? Depends on the platform. It looks like the right thing happens at boot time on most machines (i.e. the BIOS does a full toggle which causes a reset), but on suspend/resume it's up to us. And of course on machines without video init at boot time, we need to do it ourselves as well. I don't think the cri or refclk is required at this point, at least not if the docs we have are correct (the PHY reset is supposed to be the very first thing). The timing diagrams do show some kind of relationship between the cri/ref clocks and the reset signals getting deasserted. Under 4.3 Clock Signal List there is a hint about this saying that the cri and pll1 ref clocks are needed for the init time calibration of the whole PHY. The calibration is started by CMN reset, which should occur after all the needed lanes are powered on. But this matches what we do already now, except that toggling of the CMN reset signal is not guaranteed if it was already deasserted by the BIOS. This would be fixed by Jesse's v1 patch (but not v2). And third is that do we need to enable the TX wells before the CMN well? Currently we do the opposite which could also explain why this CMN well toggle fixes things. I don't think that matters, but we should ask the PHY guys. The lack of symmetry between the gate and ungate bothers me too. My theory here is that the cmn and/or side reset needs to propagate to the data lanes to propery initialize them. If the lanes are powered down when the reset occurs things go bad. I agree that the CMN reset should happen after all the lanes are powered on, but this is what we do already now, except guaranteeing that it toggles. It's not clear what triggers the side reset signal, I assume Punit deasserts it after powering on the CMN lane. Also not clear if this really needs to happen after powering on the TX lanes. And looks like the Dynamic Power Gating section in the phy cspec pretty much says as much. That is you need to fully reset the entire phy if you want to ungate even one of the blocks. What does full reset mean? In my opinion toggling the CMN reset line is enough, but I guess you suggest that we also need to power off/on the CMN lane first. It would be good if Jesse could try if toggling CMN reset alone is enough to get rid of the problem he saw. So based on that I think pretty much everything I said is needed to make stuff work correctly. I'd be also happy to get some clarification from the HW people about our assumptions, but your changes look sensible anyway, since handling the CMN reset and the cri/ref clocks do belong logically to the power well code. Also I think currently we power gate the TX blocks whenever the port gets disabled. If we want to keep doing that, we need to adjust valleyview_modeset_global_resources() to fully reset the phy whenever previously power down wells need to be brought up. The alternative would be to model the entire PHY as a single power well, which would definitely be the simpler option. Yes this is correct and it needs fixing. I took it for granted that we can turn on/off parts of the PHY independently from the rest of it, but now it looks like it's not possible on VLV. (As I understand it's possible on CHV.) I planned to come up with some fix after figuring out how things would work with fastboot, where based on this constraint we
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Thu, May 22, 2014 at 09:51:22AM +0300, Imre Deak wrote: On Wed, 2014-05-21 at 21:43 +0300, Ville Syrjälä wrote: On Wed, May 21, 2014 at 11:11:15AM -0700, Jesse Barnes wrote: And to answer more specifically... On Wed, 21 May 2014 20:54:03 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + false); + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + true); Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in with this. We should definitely rip that out regardless. Yeah we can rip that out. That's just an ungate, and it assumes the BIOS has already done the reset toggle for us. Well I'm assumign the system may boot/resume with the wells powered down. And we definitely don't have the cri/ref clocks set up at this point. So if they're needed to complete the resets the PHY may get stuck or something. Also it just ungates everything at once, if the wells need some ordering in bringup we have just messed things up here. I agree, there doesn't seem to be anything that needs those power wells until the power domain init code runs. I can send a patch for this after some testing. Another thing I'm wodering is did the BIOS/hw really power on the common lane, or did we do that outselves? If the latter, then I wonder if we simply do that too early. Or more precisely do we need to make sure the cri clock and/or refclock are enabled before we power on the common lane? Depends on the platform. It looks like the right thing happens at boot time on most machines (i.e. the BIOS does a full toggle which causes a reset), but on suspend/resume it's up to us. And of course on machines without video init at boot time, we need to do it ourselves as well. I don't think the cri or refclk is required at this point, at least not if the docs we have are correct (the PHY reset is supposed to be the very first thing). The timing diagrams do show some kind of relationship between the cri/ref clocks and the reset signals getting deasserted. Under 4.3 Clock Signal List there is a hint about this saying that the cri and pll1 ref clocks are needed for the init time calibration of the whole PHY. The calibration is started by CMN reset, which should occur after all the needed lanes are powered on. But this matches what we do already now, except that toggling of the CMN reset signal is not guaranteed if it was already deasserted by the BIOS. This would be fixed by Jesse's v1 patch (but not v2). My main issue with Jesse's patch is that it may fix the resume time problem. But things should still fail in the exact same way if we power gate the TX lanes and then try to bring them back. So it should fail by simply disabling and re-enabling one port while keeping the common lane powered up eg. by having the other port enabled all the time. And we don't assert cmnreset when powergating the cmnlane, so that too may mess up the cmn lane power up since cmnreset is now deasserted before sidereset. So even if we power down the common lane as well, things may go bad. And third is that do we need to enable the TX wells before the CMN well? Currently we do the opposite which could also explain why this CMN well toggle fixes things. I don't think that matters, but we should ask the PHY guys. The lack of symmetry between the gate and ungate bothers me too. My theory here is that the cmn and/or side reset needs to propagate to the data lanes to propery initialize them. If the lanes are powered down when the reset occurs things go bad. I agree that the CMN reset should happen after all the lanes are powered on, but this is what we do already now, except guaranteeing that it toggles. It's not clear what triggers the side reset signal, I assume Punit deasserts it after powering on the CMN lane. Also not clear if this really needs to happen after powering on the TX lanes. And looks like the Dynamic Power Gating section in the phy cspec pretty much says as much. That is you need to fully reset the entire phy if you want to ungate even one of the blocks. What does full reset mean? In my opinion toggling the CMN reset line is enough, but I guess you suggest that we also need to power off/on the CMN lane first. It would be good if Jesse could try if toggling CMN reset alone is enough to get rid of the problem he saw. IIRC the docs said sidereset needs to be reasserted too. And AFAICS the only way to do that is by toggling the power to the common lane. So based on that I think pretty much everything I said is needed to make stuff work correctly. I'd be also happy to get some clarification from the HW people about our assumptions, but your changes
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Thu, 22 May 2014 09:51:22 +0300 Imre Deak imre.d...@intel.com wrote: On Wed, 2014-05-21 at 21:43 +0300, Ville Syrjälä wrote: On Wed, May 21, 2014 at 11:11:15AM -0700, Jesse Barnes wrote: And to answer more specifically... On Wed, 21 May 2014 20:54:03 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + false); + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + true); Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in with this. We should definitely rip that out regardless. Yeah we can rip that out. That's just an ungate, and it assumes the BIOS has already done the reset toggle for us. Well I'm assumign the system may boot/resume with the wells powered down. And we definitely don't have the cri/ref clocks set up at this point. So if they're needed to complete the resets the PHY may get stuck or something. Also it just ungates everything at once, if the wells need some ordering in bringup we have just messed things up here. I agree, there doesn't seem to be anything that needs those power wells until the power domain init code runs. I can send a patch for this after some testing. Another thing I'm wodering is did the BIOS/hw really power on the common lane, or did we do that outselves? If the latter, then I wonder if we simply do that too early. Or more precisely do we need to make sure the cri clock and/or refclock are enabled before we power on the common lane? Depends on the platform. It looks like the right thing happens at boot time on most machines (i.e. the BIOS does a full toggle which causes a reset), but on suspend/resume it's up to us. And of course on machines without video init at boot time, we need to do it ourselves as well. I don't think the cri or refclk is required at this point, at least not if the docs we have are correct (the PHY reset is supposed to be the very first thing). The timing diagrams do show some kind of relationship between the cri/ref clocks and the reset signals getting deasserted. Under 4.3 Clock Signal List there is a hint about this saying that the cri and pll1 ref clocks are needed for the init time calibration of the whole PHY. The calibration is started by CMN reset, which should occur after all the needed lanes are powered on. But this matches what we do already now, except that toggling of the CMN reset signal is not guaranteed if it was already deasserted by the BIOS. This would be fixed by Jesse's v1 patch (but not v2). I guess the earlier sequence docs aren't clear on this, but it makes sense. What does full reset mean? In my opinion toggling the CMN reset line is enough, but I guess you suggest that we also need to power off/on the CMN lane first. It would be good if Jesse could try if toggling CMN reset alone is enough to get rid of the problem he saw. Well, that's the problem. I don't have access to the failing machines anymore, and we only ever had one that was reliably failing, so this is a really tough one to verify. The only one that was actually tested was the one I sent out at the very beginning. Also I think currently we power gate the TX blocks whenever the port gets disabled. If we want to keep doing that, we need to adjust valleyview_modeset_global_resources() to fully reset the phy whenever previously power down wells need to be brought up. The alternative would be to model the entire PHY as a single power well, which would definitely be the simpler option. Yes this is correct and it needs fixing. I took it for granted that we can turn on/off parts of the PHY independently from the rest of it, but now it looks like it's not possible on VLV. (As I understand it's possible on CHV.) I planned to come up with some fix after figuring out how things would work with fastboot, where based on this constraint we also have to avoid disabling/enabling any PHY lanes during booting. Currently we enable all the lanes at that time. I guess we could just leave the PHY configuration as-is until we read out the HW configuration and then leave the PHY config unchanged if there are active DP/HDMI/VGA outputs and disable the whole PHY otherwise. Yeah if the display is already active we should assume we don't have to perform a full reset. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Tue, May 20, 2014 at 11:09:03AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on boot or
resume to properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
v2: extract simpler set_power_well function for use in reset_dpio (Imre)
move to reset_dpio (Daniel Ville)
v3: don't reset if DPIO reset is already de-asserted (Imre)
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 19 +++
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index df58afc..bdb4624 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
} else {
/*
+ * If DPIO has already been reset, e.g. by BIOS, just skip all
+ * this.
+ */
+ if (I915_READ(DPIO_CTL) DPIO_CMNRST)
+ return;
+
+ /*
+ * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+ * Need to assert and de-assert PHY SB reset by gating the
+ * common lane power, then un-gating it.
+ * Simply ungating isn't enough to reset the PHY enough to get
+ * ports and lanes running.
+ */
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+ false);
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+ true);
Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in
with this. We should definitely rip that out regardless.
Another thing I'm wodering is did the BIOS/hw really power on the
common lane, or did we do that outselves? If the latter, then I wonder
if we simply do that too early. Or more precisely do we need to make
sure the cri clock and/or refclock are enabled before we power on the
common lane?
And third is that do we need to enable the TX wells before the CMN
well? Currently we do the opposite which could also explain why this
CMN well toggle fixes things.
And finally we should make sure the cmnreset assert/deassert happens
corecctly wrt. to the power well transitions.
So I'd really like to see the following measures taken:
- kill power well hack from intel_uncore_sanitize()
- move the DPLL register setup from intel_reset_dpio() into
vlv_set_power_well() like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable) {
+ I915_WRITE(DPLL, ...);
+ POSTING_READ(DPLL);
+ udelay(1); /* 10ns for cmnreset, 0ns for sidereset */
+ }
really enable the power well
}
- reorder vlv_power_wells[] to bring up TX wells before CMN.
Although maybe the DPLL stuff should then be done already before
the TX wells are brought up? But maybe not.
- move the cmnreset assert/deassert into the power well code like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC !enable)
+ assert cmnreset
do stuff
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable)
+ deassert cmnreset
}
If those measures aren't enough, then I think this patch would be OK.
But right now I have this nagging feeling that we may have shot
ourselves in the foot and we're about to pile workarounds on top of our
own bug.
+
+ /*
* From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
* 6. De-assert cmn_reset/side_reset. Same as VLV X0.
* a. GUnit 0x2110 bit[0] set to 1 (def 0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 0ef2777..feb6165 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -966,7 +966,8 @@ void intel_runtime_pm_put(struct drm_i915_private
*dev_priv);
void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
void ilk_wm_get_hw_state(struct drm_device *dev);
-
+void __vlv_set_power_well(struct drm_i915_private *dev_priv,
+ enum punit_power_well power_well_id, bool enable);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool
is_sdvob);
diff --git a/drivers/gpu/drm/i915/intel_pm.c
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Wed, 21 May 2014 20:54:03 +0300
Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Tue, May 20, 2014 at 11:09:03AM -0700, Jesse Barnes wrote:
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on boot or
resume to properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
v2: extract simpler set_power_well function for use in reset_dpio (Imre)
move to reset_dpio (Daniel Ville)
v3: don't reset if DPIO reset is already de-asserted (Imre)
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 19 +++
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index df58afc..bdb4624 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
} else {
/*
+* If DPIO has already been reset, e.g. by BIOS, just skip all
+* this.
+*/
+ if (I915_READ(DPIO_CTL) DPIO_CMNRST)
+ return;
+
+ /*
+* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the
+* common lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+false);
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+true);
Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in
with this. We should definitely rip that out regardless.
Another thing I'm wodering is did the BIOS/hw really power on the
common lane, or did we do that outselves? If the latter, then I wonder
if we simply do that too early. Or more precisely do we need to make
sure the cri clock and/or refclock are enabled before we power on the
common lane?
And third is that do we need to enable the TX wells before the CMN
well? Currently we do the opposite which could also explain why this
CMN well toggle fixes things.
And finally we should make sure the cmnreset assert/deassert happens
corecctly wrt. to the power well transitions.
So I'd really like to see the following measures taken:
- kill power well hack from intel_uncore_sanitize()
- move the DPLL register setup from intel_reset_dpio() into
vlv_set_power_well() like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable) {
+ I915_WRITE(DPLL, ...);
+ POSTING_READ(DPLL);
+ udelay(1); /* 10ns for cmnreset, 0ns for sidereset */
+ }
really enable the power well
}
- reorder vlv_power_wells[] to bring up TX wells before CMN.
Although maybe the DPLL stuff should then be done already before
the TX wells are brought up? But maybe not.
- move the cmnreset assert/deassert into the power well code like so:
vlv_set_power_well()
{
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC !enable)
+ assert cmnreset
do stuff
+ if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC enable)
+ deassert cmnreset
}
If those measures aren't enough, then I think this patch would be OK.
But right now I have this nagging feeling that we may have shot
ourselves in the foot and we're about to pile workarounds on top of our
own bug.
I worry about this too, but on the flip side we only have the
incomplete doc to look at, and these bugs can be really hard to find
and test fixes for (this reset one in particular bit only some
machines on only some boots for example).
So going with something untested like the above may make more sense,
but may also give us a false sense of security wrt this issue.
Can you ping the PHY guys with the above and see what they have to
say? Maybe in the context of the updated programming doc stuff I sent
out? Ideally we could get firm answers, update the docs, then update
the code.
In the meantime, upstream will be broken without a tested fix on these
machines.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
On Wed, May 21, 2014 at 11:11:15AM -0700, Jesse Barnes wrote: And to answer more specifically... On Wed, 21 May 2014 20:54:03 +0300 Ville Syrjälä ville.syrj...@linux.intel.com wrote: + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + false); + __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC, + true); Hmm. I wonder how the power well hack in intel_uncore_sanitize() ties in with this. We should definitely rip that out regardless. Yeah we can rip that out. That's just an ungate, and it assumes the BIOS has already done the reset toggle for us. Well I'm assumign the system may boot/resume with the wells powered down. And we definitely don't have the cri/ref clocks set up at this point. So if they're needed to complete the resets the PHY may get stuck or something. Also it just ungates everything at once, if the wells need some ordering in bringup we have just messed things up here. Another thing I'm wodering is did the BIOS/hw really power on the common lane, or did we do that outselves? If the latter, then I wonder if we simply do that too early. Or more precisely do we need to make sure the cri clock and/or refclock are enabled before we power on the common lane? Depends on the platform. It looks like the right thing happens at boot time on most machines (i.e. the BIOS does a full toggle which causes a reset), but on suspend/resume it's up to us. And of course on machines without video init at boot time, we need to do it ourselves as well. I don't think the cri or refclk is required at this point, at least not if the docs we have are correct (the PHY reset is supposed to be the very first thing). The timing diagrams do show some kind of relationship between the cri/ref clocks and the reset signals getting deasserted. And third is that do we need to enable the TX wells before the CMN well? Currently we do the opposite which could also explain why this CMN well toggle fixes things. I don't think that matters, but we should ask the PHY guys. The lack of symmetry between the gate and ungate bothers me too. My theory here is that the cmn and/or side reset needs to propagate to the data lanes to propery initialize them. If the lanes are powered down when the reset occurs things go bad. And looks like the Dynamic Power Gating section in the phy cspec pretty much says as much. That is you need to fully reset the entire phy if you want to ungate even one of the blocks. So based on that I think pretty much everything I said is needed to make stuff work correctly. Also I think currently we power gate the TX blocks whenever the port gets disabled. If we want to keep doing that, we need to adjust valleyview_modeset_global_resources() to fully reset the phy whenever previously power down wells need to be brought up. The alternative would be to model the entire PHY as a single power well, which would definitely be the simpler option. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY. This is
required on machines where the BIOS doesn't do this for us on boot or
resume to properly re-calibrate and get the PHY ready to transmit data.
Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.
v2: extract simpler set_power_well function for use in reset_dpio (Imre)
move to reset_dpio (Daniel Ville)
v3: don't reset if DPIO reset is already de-asserted (Imre)
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 19 +++
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_pm.c | 13 ++---
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index df58afc..bdb4624 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
} else {
/*
+* If DPIO has already been reset, e.g. by BIOS, just skip all
+* this.
+*/
+ if (I915_READ(DPIO_CTL) DPIO_CMNRST)
+ return;
+
+ /*
+* From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+* Need to assert and de-assert PHY SB reset by gating the
+* common lane power, then un-gating it.
+* Simply ungating isn't enough to reset the PHY enough to get
+* ports and lanes running.
+*/
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+false);
+ __vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+true);
+
+ /*
* From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
* 6. De-assert cmn_reset/side_reset. Same as VLV X0.
* a. GUnit 0x2110 bit[0] set to 1 (def 0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0ef2777..feb6165 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -966,7 +966,8 @@ void intel_runtime_pm_put(struct drm_i915_private
*dev_priv);
void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
void ilk_wm_get_hw_state(struct drm_device *dev);
-
+void __vlv_set_power_well(struct drm_i915_private *dev_priv,
+ enum punit_power_well power_well_id, bool enable);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b59e8ab..8f7dbb9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5724,10 +5724,9 @@ static bool i9xx_always_on_power_well_enabled(struct
drm_i915_private *dev_priv,
return true;
}
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
- struct i915_power_well *power_well, bool enable)
+void __vlv_set_power_well(struct drm_i915_private *dev_priv,
+ enum punit_power_well power_well_id, bool enable)
{
- enum punit_power_well power_well_id = power_well-data;
u32 mask;
u32 state;
u32 ctrl;
@@ -5760,6 +5759,14 @@ out:
mutex_unlock(dev_priv-rps.hw_lock);
}
+static void vlv_set_power_well(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well, bool enable)
+{
+ enum punit_power_well power_well_id = power_well-data;
+
+ __vlv_set_power_well(dev_priv, power_well_id, enable);
+}
+
static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
