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

[Imre Deak]

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 at 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
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.

--Imre