Re: [PATCH 00/17] drm/i915: Fix vlv/chv panel power sequencer

Daniel Vetter
2014-10-28T08:31:48Z

On Mon, Oct 27, 2014 at 07:56:50PM +0200, Imre Deak wrote:
> On Thu, 2014-10-16 at 21:27 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > After weeks or months of beating on the hardware I finally managed
> > to figure out how to kick the vlv/chv power sequencer in a reasonably
> > light way after changing the pipe<->port mapping.
> >
> > Contrary to my earlier comments we don't actually need to kick it
> > when dealing with regular DP ports. I'm not sure if my previous BSW
> > was just a bit funky or if I just overlooked something because I clearly
> > remember having to do that. Anyway now it seems enough to kick eDP
> > ports only and with the refined kick procedure (which doesn't involve
> > panel power on+off cycles anymore) it should be a reasonably fast
> > operation too. The power sequencer on one pipe can control any eDP
> > port even if that pipe otherwise is driving another non-eDP port.
> >
> > I've tested this this on BYT eDP+DP, BSW eDP+DP, ILK eDP+DP, IVB DP
> > and HSW eDP and everything appears to working perfectly now. I've
> > been beating on it for a few days now trying all kinds combinations
> > of pipes and ports and swapping them around in various ways.
> >
> > I also simulated a dual eDP system on the BYT and BSW machines by
> > making the code pretend that the external DP port is also eDP. That
> > way I could test that the power sequencer code really would work
> > in dual eDP machines. Obviously I was unable to test that VDD would
> > actually get enabled for both ports since one of them was DP but
> > at least the registers pretend that is, and the power sequencer
> > didn't cause the port to gets stuck at any point, and the AUX stuff
> > worked every single time on the actual eDP port so at least there
> > VDD got applied appropriately.
> >
> > So I'm quite confident that this vlv/chv power seqeuencer problem
> > is now solved. Well, until someone breaks it that is. I'm not sure
> > we can do any kind of satisfactory tests for this stuff since it
> > depends on eDP and some of trickier interaction even requires dual
> > eDP. Also some of the failures can manifest as something as benign
> > as failed OUI reads which doesn't even flag an error in dmesg. So
> > I don't have particularly good ideas on how we can prevent regressions
> > to this code.
> >
> > Ville Syrjälä (17):
> > drm/i915: Warn if trying to register eDP on port != B/C on vlv/chv
> > drm/i915: Warn if stealing power sequencer from an active eDP port
> > drm/i915: Remove high level intel_edp_vdd_{on,off}() from hpd/detect
> > drm/i915: Store power sequencer delays in intel_dp
> > drm/i915: Don't initialize power seqeuencer delays more than once
> > drm/i915: Split power sequencer panel on/off functions to locked and
> > unlocked variants
> > drm/i915: Hold the pps mutex across the whole panel power enable
> > sequence
> > drm/i915: Wait for PHY port ready before link training on VLV/CHV
> > drm/i915: Fix eDP link training when switching pipes on VLV/CHV
> > drm/i915: Kick the power sequencer before AUX transactions
> > drm/i915: Make sure DPLL is enabled when kicking the power sequencer
> > on VLV/CHV
> > drm/i915: Don't kick the power seqeuncer just to check if we have
> > vdd/panel power
> > drm/i915: Clear PPS port select when giving up the power sequencer
> > drm/i915: Warn if stealing non pipe A/B power sequencer
> > drm/i915: Steal power sequencer in vlv_power_sequencer_pipe()
> > drm/i915: Improve VDD/PPS debugs
> > drm/i915: Warn if panel power is already on when enabling it
> >
> > drivers/gpu/drm/i915/intel_display.c | 84 +++++----
> > drivers/gpu/drm/i915/intel_dp.c | 347 ++++++++++++++++++++++++-----------
> > drivers/gpu/drm/i915/intel_drv.h | 16 ++
> > 3 files changed, 304 insertions(+), 143 deletions(-)
>
> Looks good to me, I guess it could fix a bunch of VLV modeset related
> bugs we have open. I tested this on my BYT-M, eDP/DP with basic xrandr
> on/off sequences and igt/kms_flip and it works nicely, so on the entire
> series:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Ok, pulled in the entire series except for two patches where I want to
wait for Ville's reply first.

Thanks for patches&review,
Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch