Re: [ANNOUNCE] patch for INTEL HDMI codec

[Takashi Iwai <tiwai@suse.de>]

At Wed, 29 Oct 2008 14:07:55 +0800,
Wu, Fengguang wrote:
> 
> On Tue, Oct 28, 2008 at 06:31:09PM +0800, Takashi Iwai wrote:
> > > > 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 :)
> 
> Thank you!
> 
> In the long term, there will be kernel mode-setting code that takes
> charge of the EDID information; it can also do audio enabling on the
> video part, so that HDMI audio is available to console users.
> 
> > > > 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.
> 
> That's a good direction. Can you point the right place for EDID code,
> or I can simply put them in patch_intelhdmi.c for now?

You can put them as they are right now.
My plan is just like the grand unification theory :)


> > > > > 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().
> 
> OK, I'll do it after sorting out the basic functions.

Or, it can be procfs.  It won't be easy as well.
Choose the way you like.


> > > > > --- /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.
> 
> OK. I'll remove bit fields from struct hdmi_audio_infoframe and keep
> struct ELD_fixed_fields for internal use only.

That's great.

Thanks!

Takashi