Re: [PATCH 15/15] drm/i915: Enable PSR for Baytrail and Braswell.

From: Rodrigo Vivi <rodrigo.vivi@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 15/15] drm/i915: Enable PSR for Baytrail and Braswell.
Date: Mon, 17 Nov 2014 11:12:30 -0800	[thread overview]
Message-ID: <CABVU7+tPyr-2mw5tENOOuH=x_CKBoWvdQu5Xr8ROD_3bK-_dag@mail.gmail.com> (raw)
In-Reply-To: <20141117185140.GL25711@phenom.ffwll.local>

On Mon, Nov 17, 2014 at 10:51 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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@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@intel.com>
>> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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).

yes

> Also do things still work on
> hsw/bdw?

Yes, on hsw sdp ultrabook passing 100% of time.
On BDW acrilic sdp passing after I removed that VBT.skip aux on exit.

> If so that's really strange since this shouldn't depend in any
> way on the source platform, it's "just" dp aux.

indeed.

>
> 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.

good ideas, I;ll check that
Thanks.

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

I'll continue the investigation.

>
>> > 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 ...

I'll do my best to speed up this work and get this safelly enabled by
default soon.

But with your point in mind I got worried about FBC. One of my tasks
that should be easily here was to put FBC on SKL on the same stage
that we have currently on previous platforms. Should I just skip that
while we don't really have FBC enabled by default?

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

-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx