Re: [PATCH v4 10/12] drm/i915/gen9: Turn DC handling into a power well

Daniel Vetter
2015-11-24T12:25:00Z

On Tue, Nov 24, 2015 at 01:09:03AM +0200, Imre Deak wrote:
> On Mon, 2015-11-23 at 14:58 -0800, Matt Roper wrote:
> > On Mon, Nov 16, 2015 at 04:20:01PM +0100, Patrik Jakobsson wrote:
> > > Handle DC off as a power well where enabling the power well will
> > > prevent
> > > the DMC to enter selected DC states (required around modesets and
> > > Aux
> > > A). Disabling the power well will allow DC states again. For now
> > > the
> > > highest DC state is DC6 for Skylake and DC5 for Broxton but will be
> > > configurable for Skylake in a later patch.
> > >
> > > v2: Check both DC5 and DC6 bits in power well enabled function
> > > (Ville)
> > > v3:
> > > - Remove unneeded DC_OFF case in skl_set_power_well() (Imre)
> > > - Add PW2 dependency to DC_OFF (Imre)
> > > v4: Put DC_OFF before PW2 in BXT power well array
> > >
> > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > > Reviewed-by: Imre Deak <imre.deak@intel.com>
> >
> > I've been seeing a BXT regression on recent di-nightly where DPMS off
> > causes the entire platform to power down[1] instead of just the
> > display;
> > my bisect lands on this commit as the culprit.  Any idea what the
> > cause
> > could be?  I can reproduce by either letting the system sit idle long
> > enough at an fb console, or by doing an "xset dpms force off" in X.
> > Unfortunately I don't have a functioning serial console on this
> > platform, so I can't get any messages that may show up around the
> > DPMS
> > operation.  I've attached my boot-time dmesg output in case that
> > helps.
> >
> > Subsequent commits seem to depend on the changes here, so I haven't
> > reverted this commit directly on di-nightly, but I confirmed that if
> > I
> > checkout this commit directly I see DPMS problems, whereas its HEAD~1
> > works as expected.
>
> The power well support on BXT is not stable atm, we need to apply at
> least a similar set of fixes as we did for SKL. So for now I would
> suggest disabling it, by booting with i915.disable_power_well=0 until
> things are fixed. This should've been made the default option earlier,
> I forgot about this. I will follow up with the patch to that extent.

I guess we should pull in that patch asap. Other problem is that current
igt tests aren't too good at obeying the dsi encoder/pipe restrictions, so
atm pm_rpm just skips. That needs to be fixed too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation