Re: [PATCH] drm/i915: Improve PSR activation timing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> writes:

> On Sat, Feb 10, 2018 at 09:43:59AM +0000, Chris Wilson wrote:
>> Quoting Andy Lutomirski (2018-02-09 17:55:38)
>> > On Fri, Feb 9, 2018 at 7:39 AM, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
>> > > Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> writes:
>> > > So, I move the hacked scheduled to 10s and forced psr_activate 10ms
>> > > after that and the result is this:
>> > >
>> > >
>> > > [   11.757909] [drm:intel_psr_enable [i915]] *ERROR* I915-DEBUG:
>> > > Scheduling 10s
>> > > [   11.778586] [drm:intel_psr_flush [i915]] *ERROR* I915-DEBUG: Work busy!
>> > > ...
>> > > a bunch more of Work busy
>> > > ...
>> > > [   21.980643] [drm:intel_psr_flush [i915]] *ERROR* I915-DEBUG: Work busy!
>> > 
>> > This like ("Work busy!") is intel_psr_flush() noticing that
>> > work_busy() returns true (which it does indeed do when the work is
>> > scheduled for the future).  But this means that intel_psr_flush()
>> > wants to keep PSR off for a little bit (10s with your patch applied)
>> > because the screen isn't idle.
>> > 
>> > > [   21.983060] [drm:intel_psr_work [i915]] *ERROR* I915-DEBUG: Work running
>> > 
>> > And here's intel_psr_work() turning PSR back on 3ms later.
>> > 
>> > So I think you're seeing exactly the bug I described.
>
> Well, not in the way you described though.
>
> As we can see on the log work running get executed only 10 seconds after
> intel_psr_enable wanted. None of the thousand flushes happening there will
> trigger psr activate before the wanted 10s.
>
> So the hack indeed work as expected.
>
> Given that description I proved that it works as expected and move away.
> But since you insisted I step back and re-read your patch like if you had
> never mentioned the hack on intel_psr_enable ever, but only focusing
> on the regular calls from intel_psr_flush and you are right,
> we do have a bug that would affect VLV/CHV here.
>
> In the current code the psr_activate can happen any moment from 0ms to 100ms
> after the flush.
>
> This 100ms would just reduce the amount of fast exit-activate cases,
> but not eliminate the possibility of enabling before 100ms.
>
> On core platforms it shouldn't be any problem because HW tracking would
> take care of waiting the extra idle frames.
>
> This could be a particular problem for VLV/CHV platforms where I believe
> we should wait few frames before really activating it back.
>
> So, probably we can go ahead with your patch so we get this covered for
> VLV/CHV, but I'd like a version without mentioning the intel_psr_enable
> case that is already proven work as expected.
>
> But meanwhile I will check back on the hack and see if we can already
> remove that since many things got fixed and improved around eDP link training
> and psr itself.
>
> Also I will discuss with DK and check the possibility of a immediate activate
> on platforms with HW tracking. Specially now that we are moving towards more
> use of HW tracking.
>
> In this case we would reduce this scheduled/timer based solution only for VLV/CHV.
>
> And then on VLV/CHV front there are even more issues to sync with VBLANK etc in
> a platform that probably went to the market without any single unit with PSR panels
> that was to expensive when those platforms got released.
>
> I even consider totally removing VLV/CHV code completely.
>
>> 
>> It's also evident in the tests that PSR isn't being disabled as
>> often/quick as we need for frontbuffer updates to be seen.
>
> Yeap, it is evident we have many bugs there, but I'm afraid this is not
> the answer for the lag.
> DK found some promising ones...

Ok. I was wrong. This also explains and fixes some cases here like
fbcon/fbdev... So:

Tested-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>

I'd still want a version without mention to the schedule case at
intel_psr_enable to avoid confusion here. But since I'm now
basing my patches on top of this:

Acked-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>

>
>> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux