On Wed, Mar 02, 2016 at 07:13:07PM +0200, Imre Deak wrote: > On Fri, 2016-02-26 at 10:02 -0800, Rodrigo Vivi wrote: > > [...] > > Well, I have this tree: > > https://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=rpm-domains-psr-vblank-counter-full > > with mainly: > > 1 - vblank domain on pre-enable post-disable vblanks hooks as Ville > > had suggested > > 2 - psr domain so we just enable dc state when screen is really in > > idle. > > 3 - restore counter on vblank enable. > > > > From what I understood so far of this problem, only the patch 1 > > should > > be enough, but with only this one I don't get the screen frozen but > > the typying is so slow that is visible that we have something > > wrong.... Maybe dc state transition with mutexes there are slow? > > I'm not aware of any big latencies caused by toggling DC states alone. According to bspec, any MMIO access when in DC6 have a considerably higher latency. The recommendation from bspec is to disable DC states around longer sequences of MMIO. I don't know if this is the case here but going from DC6 -> DC3 is a quite heavy operation (enable PG0, PG1, CDCLK and do CSR for these). I guess we could measure it to see how long it actually takes? > > > Patch 2 by iitself also doesn't solve this and I still have frozen > > screens, but when combined to patch 1 everything works really > > well... > > In the point that I believe we really don't need patch 3. > > I think something like 1 and 3 is a good idea (and both are needed). > About 2, it's strange that you have to disable DC states when enabling > the panel. Since the pipe is active it should prevent DC5 (and hence > DC6). We wouldn't waste any power with your changes, since you re- > enable DC states before entering PSR, but imo we should find out why > exactly this is needed. > > Some notes/ideas about the patches: > - intel_display_power_get() is called from page_flip_completed(), which > is bad since we can be in interrupt context. > - There is a drm_crtc_vblank_get() in intel_crtc_page_flip(), but there > is no corresponding intel_display_power_get() for it. > - The same goes for the FBC, CRC code, couldn't you just call the new > vblank hooks from drm_vblank_get/put()? > - PIPE_FLIPCOUNT_G4X is also read-only, so it could get corrupted the > same way as the frame counter register. page_flip_finished() depends on > PIPE_FLIPCOUNT_G4X, so isn't this a problem? > > I haven't checked this in detail, but it could be that we need to exit > PSR explicitly when waiting for a vblank or doing a flip. In PSR mode > the pipe may not be running, so I'm not sure how the vblank and flip > interrupts would be delivered. > > --Imre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- --- Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
