On Thu, Jan 07, 2016 at 11:40:22AM +0200, Jani Nikula wrote: > On Thu, 07 Jan 2016, Daniel Vetter <daniel@xxxxxxxx> wrote: > > On Thu, Jan 07, 2016 at 10:15:00AM +0200, Jani Nikula wrote: > >> On Wed, 06 Jan 2016, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote: > >> > Our attempts save/restore panel power state in i915_suspend.c are > >> > causing unclaimed register warnings on BXT since the registers for this > >> > platform differ from older platforms. > >> > > >> > The big hammer suspend/resume shouldn't be necessary for PP since the > >> > connector/encoder hooks should already handle this. In theory we could > >> > remove this for all platforms, but in practice it's likely that would > >> > cause some regressions since older platforms with LVDS may have > >> > incomplete PP handling. For now we'll leave the PCH save/restore alone > >> > and change the non-PCH branch to only operate on gen <= 4 so that BXT > >> > and future platforms aren't included. > >> > > >> > v2: Typo fix: s/||/&&/ > >> > > >> > v3: Change non-PCH condition to a gen <= 4 test rather than listing > >> > VLV/CHV/BXT as specific platforms to exclude; should be more > >> > future-proof as we add new platforms. (Daniel) > >> > > >> > Cc: Vandana Kannan <vandana.kannan@xxxxxxxxx> > >> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > >> > Cc: Daniel Vetter <daniel@xxxxxxxx> > >> > Cc: drm-intel-fixes@xxxxxxxxxxxxxxxxxxxxx > >> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > >> > --- > >> > Although it's the direction we want to move, I'm not brave enough to blow away > >> > the entire PP save/restore for all platforms since I don't have the HW > >> > necessary to deal with the regressions that might pop up. The consensus on IRC > >> > is that there probably will be a few regressions when we do that, so I'd rather > >> > have people with appropriate platform access make those changes. > >> > >> This is the first step we want to take anyway. > >> > >> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > > > Just looked at this some more, and the code here is the only code that > > restores PP registers. Except for vlv/chv, where we have a call to > > intel_dp_init_panel_power_sequencer_registers in vlv_power_sequencer_pipe. > > > > I think we first need to sprinkle more calls to restore PP registers > > around before we can disable this here for bxt. This patch here just > > papers over a legit bug, without fixing it really. > > No, this patch doesn't paper over anything. The code that gets run for > Broxton before this patch doesn't save/restore any meaningful registers > that could possibly help with PP. This fixes unclaimed register > read/writes, and it's a valid fix as such. > > The commit message should probably be fixed to say that we're not there > yet for Broxton with the hooks. But IMO this patch is a valid fix and > orthogonal to the issue of fixing the hooks. The hooks are good enough, at least they work for vlv/chv. If we can't fix up PP restoring for bxt right away because it's too much work I think we should at least put int a FIXME. Otherwise we'll promptly forget about this again, invalidating the value unclaimed register access provides as a hint that we haven't yet completed some crucial bxt enabling work. Also, we need a jira to keep track of this. Adding Imre/Annie for that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
