[Bug 72352] New: [HSW] Incorrect i915.ko audio enabling code, bad vblank wait

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=72352

          Priority: medium
            Bug ID: 72352
                CC: intel-gfx-bugs at lists.freedesktop.org
          Assignee: intel-gfx-bugs at lists.freedesktop.org
           Summary: [HSW] Incorrect i915.ko audio enabling code, bad
                    vblank wait
        QA Contact: intel-gfx-bugs at lists.freedesktop.org
          Severity: normal
    Classification: Unclassified
                OS: Linux (All)
          Reporter: przanoni at gmail.com
          Hardware: Other
            Status: NEW
           Version: unspecified
         Component: DRM/Intel
           Product: DRI

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

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are on the CC list for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx-bugs/attachments/20131205/76cad168/attachment.html>