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

Imre Deak
2014-10-27T18:08:22Z

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>