On Thu, 22 May 2014 09:51:22 +0300 Imre Deak <imre.deak@xxxxxxxxx> 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.syrjala@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx