On Sat, Mar 01, 2014 at 03:29:41PM -0300, Rodrigo Vivi wrote:
> On Sat, Mar 1, 2014 at 5:45 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, Feb 28, 2014 at 08:44:45PM -0300, Rodrigo Vivi wrote:
> >> Baytrail cannot easily detect screen updates and force PSR exit.
> >> So we inactivate it on {busy_ioctl, set_domain, sw_finish and mark_busy}
> >> and update to enable it back it later with a delayed workqueue.
> >
> > Why are we not checking if the object being accessed is indeed being
> > used for PSR? In set-domain, you only care about writes.
>
> Ok, so here you are suggesting that I on trigger psr_exit if it is
> write domain, right?
> I agree.
>
> > sw-finish and
> > busy are too late for psr_exit, the damage has already been done and
> > presumably the content may already be corrupted?
>
> I also agree but it is better late than never... or than letting
> userspace fully control the psr exit.
> Most of the cases are coverred by psr_exit at set_domain.
> The remaining cases where userspace set domain once, sleep for while
> (like 10 seconds) and than write something
> was coverred by my test that checked crc and it was different from crc
> base already.
>
> > Can you please explain
> > that it is safe to do an psr_exit after the damage is already in the scanount
> > based on the panel refresh cycle.
>
> The perfect solution is the hardware tracking the changes and doing
> psr_exit by itself,
> but unfortunatelly se don't have this scenario so we can live with what we have.
>
> If the idea is to allow userspace to let kernel know when to exit psr
> I'm in favor of a more generic
> ioctl called pre_damage to or a front_buffer_rend something to warn
> when some damage will occur
> and we can disable psr, fbc and do whatever we want before the damage
>
> if you prefer I can remove this patch and add the patch with ioclts
> that let psr_exit full control to userspace.
> These patches are ready already. I just really don't like this option
> because I don't like the idea to depend
> on userspace to get this hardware feature working.
This is the level of detail that I want in the changelog and in nearby
comments. Keeping track of the weaknesses of the implementation is
vital.
For example there are problems with this approach if userspace flips
between two GTT mmapped buffers, it currently has no reason to call
set-domain again (and it never calls sw-finish along this path). So PSR
is enabled again, and userspace may continue to directly write into the
scanout without the kernel ever detecting and issuing the psr_exit().
I do not believe there is a way to change the coherency semantics of the
GTT domain without userspace being aware, or else run afoul of a popular
corner case.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
