Bug 72352 - [HSW] Incorrect i915.ko audio enabling code, bad vblank wait
Summary: [HSW] Incorrect i915.ko audio enabling code, bad vblank wait
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Paulo Zanoni
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 72211
  Show dependency treegraph
 
Reported: 2013-12-05 13:16 UTC by Paulo Zanoni
Modified: 2017-07-24 22:56 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Paulo Zanoni 2013-12-05 13:16:22 UTC
Hi

(this bug was reported somewhere else, and I was asked by the Audio team  to move it to freedesktop.org)

The function haswell_write_eld() from intel_display.c is responsible for enabling Audio on Haswell. This function is called by intel_write_eld(), which is called at mode_set time. When these functions are called, we're still at the very beginning of the mode set sequence, so the pipes, ports and planes are all stopped.

One of the problems I see is that this function calls intel_wait_for_vblank. Since the pipes are stopped, there's no vblank to wait, so what happens is that we sleep for the full 50ms timeout that's inside intel_wait_for_vblank, and we get a "vblank wait timed out" message every time. Waiting for a vblank at this point of the mode set sequence doesn't really make any sense.

I can imagine two possible solutions for this problem, but I'm not sure which one is correct:

Possible solution 1:
The docs suggests that Audio should only be enabled after the whole video mode set sequence is done, so we could perhaps call intel_write_eld just after the pipe is really running. I don't really know if we need to move more code besides intel_write_eld.

Possible solution 2:
If we conclude that intel_write_eld is called at the right place, we should consider replacing the intel_wait_for_vblank call with something that makes more sense and waits just for the amount of time we actually need, not for full 50ms. We should really take into consideration that the pipe is stopped.

One of the goals of the i915.ko driver is to startup as fast as possible, so useless 50ms waits should really be removed.

Here's a sample backtrace from haswell_write_eld():
[ 5.447629] [<ffffffffa00a9d48>] haswell_write_eld+0x58/0x420 [i915]
[ 5.447666] [<ffffffffa00b3e42>] intel_write_eld+0xc2/0xe0 [i915]
[ 5.447703] [<ffffffffa00bf959>] intel_ddi_mode_set+0x1a9/0x1b0 [i915]
[ 5.447733] [<ffffffffa00b1eea>] __intel_set_mode+0x77a/0x1470 [i915]
[ 5.447762] [<ffffffffa00b50f6>] intel_set_mode+0x16/0x30 [i915]
[ 5.447788] [<ffffffffa00b5a06>] intel_crtc_set_config+0x7f6/0x9f0 [i915]


I also sent a patch on September to remove the wait since it doesn't make sense: http://lists.freedesktop.org/archives/intel-gfx/2013-September/033454.html , but the patch was not merged.

Thanks,
Paulo
Comment 1 Jocelyn Li 2013-12-06 00:49:42 UTC
Mengdong,

Could you take a look at this?

Thanks,
Jocelyn
Comment 2 Mengdong Lin 2014-02-07 09:04:29 UTC
Hi Paulo,

We're checking with HW owner, Joseph and Abid, about the audio enable sequence position and whether it's okay to remove intel_wait_for_vblank. 

We've including you in that mail - "Need your clarfication about position of audio codec enable sequence in HSW/BDW b-spec".


Thanks
Mengdong
Comment 3 Mengdong Lin 2014-02-20 07:59:11 UTC
Hi Paulo,

I think your patch below is okay:
http://lists.freedesktop.org/archives/intel-gfx/2013-September/033454.html

And I tested it on HSW/BDW and found no regression.


We don't want to move intel_write_eld() atm, because it's called after the port:pipe mapping is established. Although we enables audio before pipe and port are finally enabled, the unsol responses of the eld valid/presence detect are not lost to the audio driver.
Comment 4 Paulo Zanoni 2014-04-29 13:20:27 UTC
Looks like we merged "drm/i915: Remove vblank wait from haswell_write_eld" from Daniel instead of my patch. Closing bug.


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.