Re: [PATCH] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 22 May 2014 09:51:22 +0300
Imre Deak <imre.deak@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.syrjala@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