At Tue, 28 Oct 2008 17:50:16 +0800, Wu, Fengguang wrote:
On Tue, Oct 28, 2008 at 02:01:08AM -0700, Takashi Iwai wrote:
At Tue, 28 Oct 2008 16:07:39 +0800, Wu Fengguang wrote:
Hello,
We have recently enabled sound output for Intel HDMI codecs, and would like to share the good news and some early code :-)
Great!
The audio enabling code includes both ALSA and X.Org patches. Attached is one single patch against today's sound-2.6 tree. It contains very basic code for handling - audio infoframe - ELD(E-EDID Like Data) - unsolicited response
The patch is full enough to produce stereo sound via HDMI. But it still lacks features and contains many 'defined but unused' staffs.
Yeah I see.
It was tested OK on ASUS P5E-VM board and HP 2230s notebook. It _should_ also work on Intel DG45ID board.
Does the ALSA patch work alone without changes in x.org side?
P5E-VM: works in console, ALSA patch is enough
HP 2230s: needs intel video driver to produce sound at all
DG45ID: (if I remember it right)
- produces noise in console since booting(this is why I call hdmi_disable_output() in intel_hdmi_init(), will comment this case)
- sound OK both with vesa/intel video drivers
Thanks for detailed information. If the ALSA patch doesn't do anything harmful, I'd happily apply it :)
Takashi, I can prepare a trimmed-down patch series, in doing so I'd like to know if you are interested in the basic DIP/ELD/UNSOL routines?
Yes. I wanted to write it by myself, but forgot for long time since I have no HDMI hardware for testing :) Good to see someone finally adding it!
Yeah, hardware is critical. To do this work, we collected several HDMI boxes and monitors :-)
Wow.
Right now my colleagues have some HDMI devices for testing, but they are ATI graphic boards (no surprise :)
Or should I only submit bare code comparable to the current patch_atihdmi.c?
If the codec is supposed to be (almost) compatible, it'd be a good idea. Otherwise (if you are uncertain), we can keep it separated as your original patch. I don't mind much about it.
OK. The main logic for ati/nv/intel HDMI codecs should be the same, except some possible quirks. One fact is, we migrated SiI1392 HDMI support from patch_atihdmi.c to patch_intelhdmi.c to make it work for our P5E-VM board.
Right. I think SII1392 never worked properly with the current code. So, it's no problem to move it.
And, maybe we'll need to move EDID handling to the common place, anyway.
Yes. But what if the intel driver evolve/expand fast enough to be the one real driver for HDMI/Display Port codecs? ;-)
Heh, my grand plan is to have a perfect generic codec-parser. IOW, the codec-specific code should be merged into the generic parser in future.
PS: the patch can now produce the following ELD information:
[10340.656719] eldv = 1, pinp = 1 [10340.660711] ELD buffer size is 64 [10340.660716] ELD baseline len is 10*4 [10340.660720] vendor block len is 20 [10340.660724] ELD version is CEA-861D or below [10340.660728] CEA-EDID version is CEA-861-B, C or D [10340.660732] manufacture name is 0x4544 [10340.660736] product code is 0xa02c [10340.660740] port id is 0x0 [10340.660743] HDCP support is 0 [10340.660747] AI support is 0 [10340.660751] SAD count is 0 [10340.660754] audio sync delay is 0 [10340.660758] connection type is HDMI [10340.660763] speaker allocations: [10340.660766] monitor name is DELL 2408WFP
Looks good.
I think those kernel messages would be valuable informations for both developers and end users. Such information are also good candidates for sysfs(or procfs) exports, I guess.
I thought of that, too. If you need a sysfs entry for a specific codec, you can easily attach to hwdep sysfs directory. See hda_hwdep.c:snd_hda_hwdep_add_sysfs().
--- /dev/null +++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
(snip)
+struct ELD_fixed_fields {
- __u8 rsv_0 :3;
- __u8 ELD_ver :5;
We should avoid bitfields if it's used for communication with the hardware in general for portability reason. Simply take it as bytes, and use normal bit shift ops.
This structure is for communication with intel's video driver. ie. intel video driver reuses that struct for filling the ELD buffer, so I think it should be OK.
Hm... Then this should work, although I don't agree fully with bitfields for any communication protocols.
Anyway, it's no big problem. This can be fixed if we see any incompatibilities.
Also, no reason to use "__" prefix there.
OK.
+static void hdmi_debug_slot_mapping(struct hda_codec *codec) +{ +#ifdef CONFIG_SND_DEBUG
I'd use CONFIG_SND_DEBUG_VERBOSE here. Most distros turn on CONFIG_SND_DEBUG=y, and this will be annoying.
Will use it - it wasn't only because I cannot find the _VERBOSE version at the time ;-)
That was renamed from CONFIG_SND_DEBUG_DETECT.
--- sound-2.6.orig/sound/pci/hda/hda_intel.c +++ sound-2.6/sound/pci/hda/hda_intel.c @@ -1196,7 +1196,7 @@ static unsigned int azx_max_codecs[AZX_N
- report wrongly the non-existing 4th slot availability
*/ static unsigned int azx_default_codecs[AZX_NUM_DRIVERS] __devinitdata = {
- [AZX_DRIVER_ICH] = 3,
- [AZX_DRIVER_ICH] = 4, [AZX_DRIVER_ATI] = 3,
Well, this change should be done in a separate patch.
OK, it's a temp solution - I read about the same patch from you in latest linux-2.6 tree :-)
Oh yes, there were some issues around it.
I guess your hardware has the HDMI code on the 4th slot, and the audio codec on other?
It's required for DG45ID, however I no longer have access to it now :-(
This number 3 was basically to avoid lock-up by a wrong detection on some hardwares such as thinkpad X60 with some old BIOS. I remember there were others, too.
I guess, however, this workaround is no longer necessary because we clear the slot bits beforehand properly. If any, a user can add probe_mask option manually, and we can put it in the blacklist.
So, I'd say, let's remove this azx_default_codecs[] stuff completely now.
Agreed, I wondered why probe_mask and max limits exist side by side...
OK, I'll fix that now.
thanks,
Takashi