On Sat, Mar 1, 2014 at 6:10 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > 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. agree > 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 see... > 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. so, what do you suggest? ioctls for exit? > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx