[APPLIED] Re: Oneiric: [PATCH] (UBUNTU: Sauce) Match jack input devices and pcm devices for HDMI

[David Henningsson]

On 09/02/2011 07:49 PM, Leann Ogasawara wrote:
> On Fri, 2011-09-02 at 17:21 +0200, David Henningsson wrote:
>> On 08/31/2011 11:35 PM, Leann Ogasawara wrote:
>>> On Wed, 2011-08-31 at 09:18 +0200, David Henningsson wrote:
>>>> On 08/30/2011 03:48 PM, Tim Gardner wrote:
>>> [...]
>>>>> So, you're asking to have a patch applied that has general impact on all
>>>>> platforms with an HDMI port, which by your own admission has not had
>>>>> widespread testing, and has not been agreed to or fully reviewed by
>>>>> upstream maintainers.
>>>>>
>>>>> The kernel is in Beta freeze right now. What would your call be if you
>>>>> were in the same position?
>>>>
>>>> If I were in the same position:
>>>>
>>>> First and foremost, I would start with a warm, welcoming and encouraging
>>>> attitude towards patches. A patch sent here, means someone not only
>>>> wants to improve Linux in general, but also cares in special for Ubuntu,
>>>> and has taken the time and effort to backport a patch to suit Ubuntu's
>>>> particular kernel version. In this case, the patch author is a Canonical
>>>> employee so this is not surprising, but nevertheless, this is a really
>>>> good thing and should be encouraged.
>>>
>>> Let me just start by saying that we really appreciate all the work that
>>> you've done with regards to the audio subsystem.  Your interaction with
>>> upstream and the bug fixes you've provided for Ubuntu have been much
>>> appreciated and I encourage you to keep up the good work.
>>>
>>>> With that positive attitude in mind, I would do a review. The review
>>>> would, of course, take regression risk into account - a kernel bug can
>>>> cause the system to fail in the most horrifying ways - but also weigh
>>>> that against the possible positive outcomes of applying the patch, i e
>>>> why it's needed in the first place. If I'm not qualified to make a
>>>> review, I would find someone who is, to make the opinion for me.
>>>
>>> Because we do consider you one of our knowledge experts in the audio
>>> area, I believe Tim was wanting a bit more reassurance and confidence
>>> from you with regards to the patch.  Having recognized the potential for
>>> widespread impact, I do feel he was doing his job as one of the
>>> maintainers of the Ubuntu kernel to make sure we're not introducing
>>> regressions, especially being so close to Beta for Oneiric.
>>>
>>>> The review in this case would notice that it impacts most people with
>>>> working HDMI audio which means both higher risk and gain. OTOH, the code
>>>> path is simple, which means that successful testing on one machine would
>>>> make it unlikely to fail on another.
>>>>
>>>> If I'm still unsure of regressions even after having done the review, I
>>>> would make sure testing of the patch is done. If I have time, I do the
>>>> testing myself, or if I don't, ask for someone to test it for me, but do
>>>> what I can to help, e g by building a test kernel and provide
>>>> instructions as of how to do the test. If testing with positive result
>>>> is all that's needed before applying the patch, I would communicate that
>>>> clearly, among with a deadline explaining when the testing must be
>>>> completed.
>>>
>>> I've built a test kernel with your patch applied and placed it at the
>>> following location:
>>>
>>> http://people.canonical.com/~ogasawara/diwic/
>>>
>>> If you can, could you please test and confirm this resolves the issue at
>>> hand and also doesn't introduce any regressions on a few existing
>>> systems with working HDMI audio (whatever access to systems you have on
>>> hand would suffice).  Please keep in mind that Oneiric Kernel Freeze is
>>> Thurs Sept 15, so if I could get your feedback before then I'd be happy
>>> to try and get this applied before we hit the freeze date.
>>
>> Hi Leann and thank you for your kind words and clear instructions! :-)
>>
>> Here are the testing results (where 3.0.0-10 means your patched and
>> built test kernel installed, and 3.0.0-9 is what comes with 11.10 beta 1) :
>>
>> Radeon HD 4200 series:
>>
>> 3.0.0-9, radeon driver: no jack input device, no working HDMI audio.
>> 3.0.0-10, radeon driver: no jack input device, no working HDMI audio.
>> 3.0.0-9, fglrx driver: no jack input device, working HDMI audio.
>> 3.0.0-10, fglrx driver: no jack input device, working HDMI audio.
>>
>> Note: Since there is no jack input device, the code path is not run and
>> my patch does not make any difference. Might look into that later (why
>> there isn't a jack input device), but that is not on top of the priority
>> list ATM.
>>
>> Intel Arrandale:
>>
>> 3.0.0-9: one jack input device "HDA Intel HDMI/DP" (plugin working,
>> unplug does not work), working HDMI audio.
>> 3.0.0-10: one jack input device "HDA Intel HDMI/DP,pcm=3" (plugin
>> working but compiz crashed at one point, unplug does not work), working
>> HDMI audio.
>>
>> Note: Patch working as jack input device name changes successfully. The
>> compiz crash (reported as bug 839468) was a little strange, so I retried
>> but could not reproduce the crash under either kernel. I don't think it
>> has anything to do with the HDMI audio patch.
>>
>> Nvidia GT 430:
>>
>> 3.0.0-9, nouveau driver: four jack input devices "HDA NVidia HDMI/DP"
>> (none of them working), no working HDMI audio on any of the four PCMs.
>> 3.0.0-10, nouveau driver: four jack input devices "HDA NVidia
>> HDMI/DP,pcm=3", "HDA NVidia HDMI/DP,pcm=7", "HDA NVidia HDMI/DP,pcm=8",
>> "HDA NVidia HDMI/DP,pcm=9" (none of them working), no working HDMI audio
>> on any of the four PCMs.
>> 3.0.0-10, nvidia driver: four jack input devices "HDA NVidia
>> HDMI/DP,pcm=3", "HDA NVidia HDMI/DP,pcm=7", "HDA NVidia HDMI/DP,pcm=8",
>> "HDA NVidia HDMI/DP,pcm=9". The pcm=9 indicates working HDMI audio for
>> plughw:NVidia,9 (or hdmi:NVidia,3). \o/
>>
>> Note: I spent four hours trying to install nvidia binary driver before I
>> succeeded (thanks to Sarvatt, tjaalton, and tseliot for helping out),
>> and one more hour trying to install it under 3.0.0.9.
>>
>> Final conclusion: patch works as expected and no regressions found.
>
> Thanks for the testing and the feedback.  I've gone ahead and applied
> this to Oneiric.

Thanks! I saw you released it as well, excellent :-)

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic