Bug 104931 - PSR reactivation logic seems a bit dubious
Summary: PSR reactivation logic seems a bit dubious
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Intel GFX Bugs mailing list
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-03 17:31 UTC by Andy Lutomirski
Modified: 2018-04-24 06:53 UTC (History)
3 users (show)

See Also:
i915 platform: SKL
i915 features: display/PSR


Attachments
Hackish patch (3.54 KB, patch)
2018-02-03 17:31 UTC, Andy Lutomirski
no flags Details | Splinter Review
dmesg (7.14 MB, text/plain)
2018-02-09 07:42 UTC, Rodrigo Vivi
no flags Details
debug.patch (3.47 KB, patch)
2018-02-09 07:42 UTC, Rodrigo Vivi
no flags Details | Splinter Review
Wait for PSR idle before and after PSR exit (10.06 KB, patch)
2018-02-09 23:15 UTC, Dhinakaran Pandiyan
no flags Details | Splinter Review

Description Andy Lutomirski 2018-02-03 17:31:38 UTC
I'm running Fedora 27 with a gnome-shell Wayland session.  I think I'm using a software cursor, but I'm not at all confident about that.  I seem to have small amounts of cursor lag when I move the cursor after a bit of idle time.

On inspection of the code, I see nothing that keeps PSR off for any particular amount of time or number of vblanks after intel_psr_flush() gets called.  Adding some tracing confirms that intel_psr_work() can reactivate PSR almost immediately after intel_psr_flush().  In fact, it seems that PSR gets reactivated about every 100ms even when the display contents never stop changing at all.

The attached hackish patch at least improves the trace, and it might be improving the cursor lag too, but the latter could be the placebo effect.  Nonetheless, I think the code could be improved.

My patch is a big gross.  I think a better solution would be to use mod_timer() and/or to track whatever condition, if any, is needed to ensure that the panel is up to date before reactivating PSR.
Comment 1 Andy Lutomirski 2018-02-03 17:31:56 UTC
Created attachment 137156 [details] [review]
Hackish patch
Comment 2 Rodrigo Vivi 2018-02-09 07:42:01 UTC
Created attachment 137239 [details]
dmesg

dmesg of my debug attempt
Comment 3 Rodrigo Vivi 2018-02-09 07:42:58 UTC
Created attachment 137240 [details] [review]
debug.patch

my hack to debug the hack
Comment 4 Rodrigo Vivi 2018-02-09 07:46:39 UTC
not a bug, just an ugly hack :(

more explanation on:

https://lists.freedesktop.org/archives/intel-gfx/2018-February/155044.html
Comment 5 Andy Lutomirski 2018-02-09 17:56:51 UTC
Reopening for now.  Your debug log seems to be showing exactly the bug I'm describing.
Comment 6 Dhinakaran Pandiyan 2018-02-09 23:15:05 UTC
Created attachment 137251 [details] [review]
Wait for PSR idle before and after PSR exit

Can you please check if this patch fixes the cursor lag issue when applied on top of the https://patchwork.freedesktop.org/series/37598/ [1,7,8,9,10] ?

Seems to work for me.
Comment 7 Jani Saarinen 2018-03-29 07:10:39 UTC
First of all. Sorry about spam.
This is mass update for our bugs. 

Sorry if you feel this annoying but with this trying to understand if bug still valid or not.
If bug investigation still in progress, please ignore this and I apologize!

If you think this is not anymore valid, please comment to the bug that can be closed.
If you haven't tested with our latest pre-upstream tree(drm-tip), can you do that also to see if issue is valid there still and if you cannot see issue there, please comment to the bug.
Comment 8 Jani Saarinen 2018-04-24 06:53:53 UTC
Resolving and closing, please re-open if still occurs.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.