[Intel-gfx] [PATCH 15/15] drm/i915: Enable PSR for Baytrail and Braswell.

[Daniel Vetter]

On Mon, Nov 17, 2014 at 10:30:58AM -0800, Rodrigo Vivi wrote:
> On Mon, Nov 17, 2014 at 10:18 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> > On Fri, Nov 14, 2014 at 08:52:41AM -0800, Rodrigo Vivi wrote:
> >> This patch is the last in series of VLV/CHV PSR,
> >> that finnaly enable psr by adding it to HAS_PSR
> >> and calling the proper enable and disable
> >> functions on the right places.
> >>
> >> Although it is still disabled by default.
> >>
> >> v2: Rebase over intel_psr and merge Durgadoss's fixes.
> >>
> >> Cc: Durgadoss R <durgadoss.r at intel.com>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >
> > where's the patch to enable PSR by default again?
> 
> I'm trying to be extremely careful with this. I'm not enabling it by
> default while I'm not 100% sure
> I'm not breaking things hard for end users again.

The 3.19 window is pretty much gone by now, so we can be lenient with
enabling.

> > Also I've heard noises
> > that the sink crc stuff is busted :( And we still have the tasklist to fix
> > up the functional igt testcases.
> 
> Yes. In VLV/CHV the sink CRC is not working at all. This is one of the
> reasons I'm not trying to
> enable it by default again.

Bummer :( Have you tried switching panels (if you have an sdv with the
panel separate ofc and not a form-factor). Also do things still work on
hsw/bdw? If so that's really strange since this shouldn't depend in any
way on the source platform, it's "just" dp aux.

And if vlv/chv somehow block dp aux when psr is active then:
- We need a testcase for this (reading edid while psr is active should do
  that).
- We need to throw a synchronous psr exit into the dp aux transfer
  function on vlv/chv.

If it's something else we should track this down, too.

> > I don't want to block merging this, but we need a clear task list of
> > what's left to do and full commitment of the necessary engineer-time to
> > actually make it happen. PSR with the frontbuffer tracking code is
> > invasive and will simply keep on bitrotting if we don't enable it (atomic
> > modeset already almost broke it completely by accident), which would
> > really be sad given all the time we've invested. Please chat with
> > Paul/Gavin to make sure this is tracked.
> 
> Sure. I'm not just trowing and abandoning it. I'll continue investing
> time to get it properly to get enabled by default.
> I have some Jira tasks for that and we can chat more later.
> 
> > I don't want to carry essentially dead code around - the illusion that it
> > will get magically rebased and keep working is really just that.
> 
> I don't believe this is deadcode at all. Every progress is better than
> none. This is why parameters exist.
> We are just protecting all kind of end users but still giving the
> choice to try some power savings.

The problem is that if we don't enable this by default it bitrots really
quickly, and we'll end up spending more time keeping it alive than the
final push to make it work would cost. I've thought that for hsw/bdw we've
had patches to make it work well? But somehow they never landed, or we've
forgotten to throw the switch to "enabled by default" again ...

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