Re: [PATCH 1/2] ALSA: hda - Enable i915 binding for gen3/4 HDMI/DP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 01 Apr 2016 19:53:05 +0200,
Ville Syrjälä wrote:
> 
> On Thu, Mar 31, 2016 at 02:45:13PM +0200, Takashi Iwai wrote:
> > On Thu, 31 Mar 2016 14:30:26 +0200,
> > Ville Syrjälä wrote:
> > > 
> > > On Wed, Mar 30, 2016 at 05:46:01PM +0200, Takashi Iwai wrote:
> > > > This patch fills the holes and now all i915 HDMI/DP codecs are managed
> > > > over the audio ELD notifier, finally.  The old gen3/gen4 chips have
> > > > usually only a single pin/converter pair, and the digital port mapping
> > > > is fixed.
> > > > 
> > > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > > > ---
> > > >  sound/hda/hdac_i915.c      | 11 +++++++++++
> > > >  sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++------
> > > >  2 files changed, 26 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> > > > index d0da2508823e..c62a9f830b84 100644
> > > > --- a/sound/hda/hdac_i915.c
> > > > +++ b/sound/hda/hdac_i915.c
> > > > @@ -128,6 +128,9 @@ EXPORT_SYMBOL_GPL(snd_hdac_get_display_clk);
> > > >   * Pin Widget 4 - PORT B (port = 1 in i915 driver)
> > > >   * Pin Widget 5 - PORT C (port = 2 in i915 driver)
> > > >   * Pin Widget 6 - PORT D (port = 3 in i915 driver)
> > > > + *
> > > > + * on earlier models:
> > > > + * Pin Widget 3 - PORT B
> > > 
> > > Hmm. ctg/elk have potentially multiple HDMI ports. Although they only
> > > have one video DIP block so can send infoframes only to one of the ports
> > > at a time. I wonder how this relates to the audio part, as in does the
> > > pin widget 3 always map to the port that is getting infoframes
> > > currently? I do have one elk with two HDMI ports here so I could do some
> > > experiments if needed.
> > 
> > That's great.  I'd like to see how many audio pins are there.
> > Could you take alsa-info.sh snapshot without my patches?
> 
> Attached.

Thanks!  So it still has only a single audio pin.

> > If the port switching happens but it's not seen in the audio side,
> > we'd need to record the last active audio port in i915 side and let
> > i915 function taking it (e.g. when port = -1 is passed by callback).
> 
> I played around with the machine a bit and HDMI audio works as long as
> just one of the ports has the audio enable bit set. If the bit is set on
> multiple ports, there is no audio. Doesn't matter if I drive the ports
> with the same pipe or two different pipes. Oh, and I just found a note
> in the spec to confirm this behaviour. I'll need to rework the i915 HDMI
> code a bit to make this work correctly.
> 
> Also I tested sending video related infoframes to one port and audio to
> the other port, and that also worked. I verified that at least the video
> infoframes really went to the port I want by having the AVI infoframe
> indicate YCbCr 4:4:4 while actually sending RGB data, and checking that
> the colors distorted on the correct port. But I'm not sure if the audio
> infoframe gets sent to the right port in this case. Since I don't have
> any HDMI analyzers and such I think I need to try and make a similar
> expriment with the audio infoframe to see which ports gets it. But that
> involves reading the spec and some code to find something useful to
> tweak in the infoframe.

OK, so we'd need to track the active port in anyway, so the necessary
change in the audio side seems minimal.  At most, some extension of
component ops (e.g. allowing to pass port=-1) would be necessary.


> > 
> > > >   */
> > > >  static int pin2port(struct hdac_device *codec, hda_nid_t pin_nid)
> > > >  {
> > > > @@ -139,6 +142,14 @@ static int pin2port(struct hdac_device *codec, hda_nid_t pin_nid)
> > > >  	case 0x80862882: /* VLV */
> > > >  		base_nid = 3;
> > > >  		break;
> > > > +	case 0x80862801: /* Bearlake */
> > > 
> > > Hmm. I wonder if this is bearlake-C or something earlier. BLC doesn't
> > > actually exist AFAIK, and earlier bearlake is gen3 and doesn't even
> > > support native HDMI output so not sure what this is doing here.
> > 
> > Possibly wrongly named.  Looking at the history, it seems to be
> > labeled as "G45 DEVBLC" originally.
> 
> Yeah that sounds like the thing that doesn't exist.

Aha, interesting.  But keeping it wouldn't hurt so much :)


> > I only checked Cantiga and
> > Eaglelake, so far.
> > 
> > > > +	case 0x80862802: /* Cantiga */
> > > > +	case 0x80862803: /* Eaglelake */
> > > > +	case 0x80862880: /* CedarTrail */
> > > 
> > > Cedartrail is some powervr atom thing. Should have nothing to do with i915.
> > 
> > Thanks, this one should be removed.
> > 
> > > > +	case 0x808629fb: /* Crestline HDMI */
> > > 
> > > CL is gen4 and we don't support native HDMI output on those. Strangely
> > > enough our spec says CL/BW do support TMDS encoding on the SDVO/HDMI
> > > ports and that CL also has the video DIP block and the HDA registers.
> > > Not sure what to make of that really.
> > 
> > Hm, OK, in a safer side, we may revert for these entries.
> 
> Another interesting mess I noticed is that we set the audio enable bit
> in the SDVO/HDMI port register even in SDVO mode. That doesn't really
> make any sense as SDVO only deals with the video data and the ADD2 card
> should get the audio data from the HDA bus or via i2s/spdif.
> Unfortunately I have no proper HDMI capable ADD2 cards to play around
> with. The one I have seems totally bonkers as it doesn't even respond
> to the SDVO HDMI opcodes. Also that card has an spdif input connector
> for audio so I assume it doesn't hang around on the HDA bus.
> 
> I don't know if anyone has really ever tried to get SDVO and HDA
> working together properly.

OK, so maybe it's safer to start with only CTG and ELK.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux