Bug 48157

Summary: [i965GM regression] HDMI audio on i965 in Dell Studio Hybrid 140g
Product: DRI Reporter: Bernard B <b-freedesktop>
Component: DRM/IntelAssignee: Daniel Vetter <daniel>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: medium CC: ben, chris, daniel, florian, jbarnes
Version: XOrg git   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 42991, 44622    
Attachments:
Description Flags
lspci output
none
dmesg from Ubuntu 3.2.0-20.33, without patch.
none
dmesg from Ubuntu 3.2.0-20.33, with patch (and working audio)
none
Patch which resolves the issue on Ubuntu's 3.2.0-20.33 kernel
none
separate the input/output sdvo timing
none
xrandr --verbose output
none
(hopefully) final patch for reference none

Description Bernard B 2012-04-01 05:19:36 UTC 
Created attachment 59333 [details]
lspci output

System environment: 
-- chipset: Intel Corporation Mobile GM965/GL960
-- system architecture: 32-bit
-- libdrm: 2.4.32-1ubuntu1
-- kernel: 2.6.36 to 3.2
-- Linux distribution: Ubuntu 12.04
-- Machine: Dell Studio Hybrid 140g
-- Display connector: HDMI

Somewhere between 2.6.35 and 2.6.36, HDMI audio to my connected TV went from working to not working - all the ALSA devices are still there with no changes, but in 2.6.36 is no audio from the TV. 24 hours with git bisect points to this commit: c74696b9c890074c1e1ee3d7496fc71eb3680ced

And in particular, this hunk:

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 093e914..62d22ae 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1122,11 +1123,9 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 
 	/* We have tried to get input timing in mode_fixup, and filled into
 	   adjusted_mode */
-	if (intel_sdvo->is_tv || intel_sdvo->is_lvds) {
-		intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
+	intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
+	if (intel_sdvo->is_tv || intel_sdvo->is_lvds)
 		input_dtd.part2.sdvo_flags = intel_sdvo->sdvo_flags;
-	} else
-		intel_sdvo_get_dtd_from_mode(&input_dtd, mode);
 
 	/* If it's a TV, we already set the output timing in mode_fixup.
 	 * Otherwise, the output timing is equal to the input timing.

Reverting that hunk against 2.6.36 brings back HDMI audio.

On a more recent Ubuntu precise kernel (3.2.0-21.34) which is based off 3.2, a similar patch fixes things for me:

--- drivers/gpu/drm/i915/intel_sdvo.c.orig	2012-04-01 12:30:31.000000000 +0100
+++ drivers/gpu/drm/i915/intel_sdvo.c	2012-04-01 12:49:36.000000000 +0100
@@ -1033,7 +1033,7 @@
 						  intel_sdvo->attached_output))
 			return;
 
-		intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
+		intel_sdvo_get_dtd_from_mode(&input_dtd, mode);
 		(void) intel_sdvo_set_output_timing(intel_sdvo, &input_dtd);
 	}
 

This is probably not the correct fix, as I honestly have no idea what the code is doing. But I am happy to test out any patches or provide any further information. There seem to be many people with this hardware who have experienced the same problem (e.g.  https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/793592)

Attached are dmesg output with drm.debug=0x06, from 3.2 with and without the above patch.
Comment 1 Bernard B 2012-04-01 05:20:38 UTC 
Created attachment 59335 [details]
dmesg from Ubuntu 3.2.0-20.33, without patch.
Comment 2 Bernard B 2012-04-01 05:21:12 UTC 
Created attachment 59336 [details]
dmesg from Ubuntu 3.2.0-20.33, with patch (and working audio)
Comment 3 Daniel Vetter 2012-04-01 05:57:27 UTC 
For reference, can you please attach the exact patch for 3.2 which fixes this issue for you?
Comment 4 Bernard B 2012-04-01 06:03:30 UTC 
Created attachment 59338 [details] [review]
Patch which resolves the issue on Ubuntu's 3.2.0-20.33 kernel

Attached.
Comment 5 Chris Wilson 2012-04-01 07:18:17 UTC 
So

intel_sdvo_get_mode_from_dtd() and intel_sdvo_get_dtd_from_mode() are not idempotent. We lose dtd.sdvo_flags and some bits from dtd.dtd_flags. I'm guessing that sdvo_flags is the relevant lossage in this case.
Comment 6 Daniel Vetter 2012-04-01 09:39:59 UTC 
> --- Comment #5 from Chris Wilson <chris@chris-wilson.co.uk> 2012-04-01 07:18:17 PDT ---
> So
> 
> intel_sdvo_get_mode_from_dtd() and intel_sdvo_get_dtd_from_mode() are not
> idempotent. We lose dtd.sdvo_flags and some bits from dtd.dtd_flags. I'm
> guessing that sdvo_flags is the relevant lossage in this case.

Hm, I've figured the only relevant difference between mode and adjusted
mode is the pixel clock multiplier adjustements at the end of
intel_sdvo_mode_fixup (so that the pixelclock we send to the sdvo card is
>= 100MHZ).

With some more code-reading we seem to have a decent confusion between
input and output modes (and timings). For is_tv || is_lvds we set the
output timings in mode_fixup (bad style), for everything else we do it in
mode_set, but with the adjusted mode (which I guess should be used only
for the input timings).

/me doesn't see through the mist, yet
Comment 7 Daniel Vetter 2012-04-01 10:31:36 UTC 
Created attachment 59346 [details] [review]
separate the input/output sdvo timing

Ok, I think I see what's going on here (hopefully). Please test the attached patch.
Comment 8 Daniel Vetter 2012-04-01 10:33:01 UTC 
If the patch works, please also test the for-bernard git branch from http://cgit.freedesktop.org/~danvet/drm/

It contains some additional cleanups and clarifications for this code.
Comment 9 Bernard B 2012-04-01 11:18:59 UTC 
Created attachment 59351 [details]
xrandr --verbose output
Comment 10 Daniel Vetter 2012-04-01 12:08:11 UTC 
Created attachment 59353 [details] [review]
(hopefully) final patch for reference

The thing is that if this patch is correct, things should have been broken on quite a few machines. Which means before I can submit this, I need to go bug-hunting and get more tested-bys on different machines.
Comment 11 Daniel Vetter 2012-04-10 11:46:30 UTC 
I think I've confused things here a bit. Can you please confirm whether the attached patch works for you or whether it does not work for you?
Comment 12 Bernard B 2012-04-18 08:38:17 UTC 
I can confirm that just applying attachment #59353 [details] [review] works. (tested on top of Ubuntu's 3.2.0-20.33 kernel, based on 3.2.13).
Comment 13 Daniel Vetter 2012-04-26 13:59:54 UTC 
Patch is merged to -fixes as:

commit 6651819b4b4fc3caa6964c5d825eb4bb996f3905
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sun Apr 1 19:16:18 2012 +0200

    drm/i915: handle input/output sdvo timings separately in mode_set

Thanks for reporting this regression and bearing with our slow turn-around time.
Comment 14 Florian Mickler 2012-11-05 23:09:09 UTC 
A patch referencing a commit referencing this bug report has been merged in Linux v3.7-rc3:

commit e751823da27d37d25e5543abef2dc031f8813bc8
Author: Egbert Eich <eich@suse.de>
Date:   Sat Oct 13 14:29:31 2012 +0200

    DRM/i915: Restore sdvo_flags after dtd->mode->dtd Roundrtrip.

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.