Bug 104856

Summary: `intel_hpd_cancel_work` takes around 1 s during suspend on Dell XPS 13 9360
Product: DRI Reporter: Paul Menzel <pmenzel+bugs.freedesktop.org>
Component: DRM/IntelAssignee: Intel GFX Bugs mailing list <intel-gfx-bugs>
Status: CLOSED WORKSFORME QA Contact: Intel GFX Bugs mailing list <intel-gfx-bugs>
Severity: normal    
Priority: medium CC: intel-gfx-bugs, mario_limonciello, pmenzel+bugs.freedesktop.org
Version: XOrg git   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: KBL i915 features: power/suspend-resume
Attachments:
Description Flags
Linux messages
none
Screenshot of timings for `intel_hpd_cancel_work`
none
Linux 4.15.2 messages none

Description Paul Menzel 2018-01-30 12:56:57 UTC 
There seems to be a regression on the Dell XPS 13 9360, that during suspend the method `intel_hpd_cancel_work` takes almost one second. `sleepgraph.py` [1] was used to instrument this.

I’ll add the logs later.

[1] https://01.org/suspendresume
[2] https://github.com/01org/pm-graph
Comment 1 Paul Menzel 2018-01-30 17:14:25 UTC 
Created attachment 137054 [details]
Linux messages
Comment 2 Paul Menzel 2018-01-30 17:18:28 UTC 
Created attachment 137055 [details]
Screenshot of timings for `intel_hpd_cancel_work`

Please see how `flush_work` lasts 998.321 ms.
Comment 3 Jani Nikula 2018-02-07 12:11:48 UTC 
Any idea when the regression happened? Bisect would be awesome, but some rough idea would be a good start.
Comment 4 Jani Nikula 2018-02-07 12:17:49 UTC 
Also, dmesg with drm.debug=14 all the way from boot to the problem might prove useful.
Comment 5 Paul Menzel 2018-02-09 14:35:31 UTC 
Created attachment 137244 [details]
Linux 4.15.2 messages

(In reply to Jani Nikula from comment #4)
> Also, dmesg with drm.debug=14 all the way from boot to the problem might
> prove useful.

Please find the more elaborate messages attached.
Comment 6 Imre Deak 2018-02-09 15:06:39 UTC 
The problem is that AUX interrupts get disabled too early during suspend, after which the driver still performs AUX transfers. Without interrupts these transfers will be slow (each taking a 10ms timeout overhead). The solution would be to do more fine-grained interrupt disabling during suspend.
Comment 7 Paul Menzel 2018-02-09 15:18:19 UTC 
(In reply to Imre Deak from comment #6)
> The problem is that AUX interrupts get disabled too early during suspend,
> after which the driver still performs AUX transfers. Without interrupts
> these transfers will be slow (each taking a 10ms timeout overhead). The
> solution would be to do more fine-grained interrupt disabling during suspend.

What commit introduced this regression?
Comment 8 Imre Deak 2018-02-09 17:29:57 UTC 
(In reply to Paul Menzel from comment #7)
> (In reply to Imre Deak from comment #6)
> > The problem is that AUX interrupts get disabled too early during suspend,
> > after which the driver still performs AUX transfers. Without interrupts
> > these transfers will be slow (each taking a 10ms timeout overhead). The
> > solution would be to do more fine-grained interrupt disabling during suspend.
> 
> What commit introduced this regression?

I don't think this is a regression, we never had support for keeping only AUX interrupts enabled. So it needs to be implemented.
Comment 9 Paul Menzel 2018-02-10 09:34:25 UTC 
(In reply to Imre Deak from comment #8)
> (In reply to Paul Menzel from comment #7)
> > (In reply to Imre Deak from comment #6)
> > > The problem is that AUX interrupts get disabled too early during suspend,
> > > after which the driver still performs AUX transfers. Without interrupts
> > > these transfers will be slow (each taking a 10ms timeout overhead). The
> > > solution would be to do more fine-grained interrupt disabling during suspend.
> > 
> > What commit introduced this regression?
> 
> I don't think this is a regression, we never had support for keeping only
> AUX interrupts enabled. So it needs to be implemented.

I can only say, that the delay was around 350 ms in the past, see bug #99650 [1]. So there is a regression in 4.15 in my opinion.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=99650
Comment 10 Paul Menzel 2018-02-10 09:39:54 UTC 
(In reply to Paul Menzel from comment #9)
> (In reply to Imre Deak from comment #8)
> > (In reply to Paul Menzel from comment #7)
> > > (In reply to Imre Deak from comment #6)
> > > > The problem is that AUX interrupts get disabled too early during suspend,
> > > > after which the driver still performs AUX transfers. Without interrupts
> > > > these transfers will be slow (each taking a 10ms timeout overhead). The
> > > > solution would be to do more fine-grained interrupt disabling during suspend.
> > > 
> > > What commit introduced this regression?
> > 
> > I don't think this is a regression, we never had support for keeping only
> > AUX interrupts enabled. So it needs to be implemented.
> 
> I can only say, that the delay was around 350 ms in the past, see bug #99650
> [1]. So there is a regression in 4.15 in my opinion.

Sorry, I remembered incorrectly. *Resume* takes up 350 ms, and bug #99650 shows that suspend always took over a second. So it is *not* a regression. Sorry about that.

> [1] https://bugs.freedesktop.org/show_bug.cgi?id=99650
Comment 11 Jani Saarinen 2018-03-29 07:10:20 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 12 Jani Saarinen 2018-04-25 11:10:08 UTC 
Closing, please re-open is issue still exists.

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.