[alsa-devel] New snd-hda warning spew

[Ville Syrjälä]

On Fri, Mar 18, 2016 at 07:51:43PM +0100, Takashi Iwai wrote:
> On Fri, 18 Mar 2016 18:49:19 +0100,
> Ville Syrjälä wrote:
> > 
> > On Fri, Mar 18, 2016 at 03:22:15PM +0100, Takashi Iwai wrote:
> > > On Fri, 18 Mar 2016 14:54:59 +0100,
> > > Ville Syrjälä wrote:
> > > > 
> > > > On Wed, Mar 16, 2016 at 04:04:20PM +0200, Ville Syrjälä wrote:
> > > > > On Tue, Mar 15, 2016 at 06:22:56PM +0100, Takashi Iwai wrote:
> > > > > > On Tue, 15 Mar 2016 17:02:07 +0100,
> > > > > > Ville Syrjälä wrote:
> > > > > > > 
> > > > > > > We have a few new WARN spews from snd-hda causing some grief in i915 CI.
> > > > > > > 
> > > > > > > This one happens on ILK and BYT. Looks like it happens 100% of the time on driver load:
> > > > > > > [   18.809850] ------------[ cut here ]------------
> > > > > > > [   18.809866] WARNING: CPU: 0 PID: 39 at sound/hda/hdac_i915.c:129 pin2port+0x25/0x30 [snd_hda_core]()
> > > > > > 
> > > > > > This is bad.  Basically we had a naive assumption of the fixed mapping
> > > > > > between the port number and the HD-audio widget, but it doesn't apply
> > > > > > properly to pre-HSW models.
> > > > > > 
> > > > > > The patch attached below disables the audio binding for pre-HSW
> > > > > > models.  I'm going to queue to for-linus branch.
> > > > > 
> > > > > That seems to eliminate the warn on my ILK.
> > > > 
> > > > Apparently it was less effective on BYT. We still get this:
> > > 
> > > Ouch, I forgot that Baytrail had already i915 component binding in the
> > > controller side.  I assumed too naively that all old models have no
> > > binding.
> > > 
> > > Below is the additional fix patch.
> > 
> > Still getting blasted at least via snd_hdac_sync_audio_rate()
> 
> That code path is a slightly different.  This is a kind of false
> positive, but now the function checks the validity of the passed
> argument more strictly, and starts grumbling.
> 
> The fix is attached below.
> 
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai at suse.de>
> Subject: [PATCH] ALSA: hda - Fix spurious kernel WARNING on Baytrail HDMI
> 
> snd_hdac_sync_audio_rate() call is mandatory only for HSW and later
> models, but we call the function unconditionally blindly assuming that
> the function doesn't do anything harmful.  But since recently, the
> function checks the validity of the passed pin NID, and eventually
> spews the warning if an unexpected pin is passed.  This is seen on old
> chips like Baytrail.
> 
> The fix is to limit the call of this function again only for the chips
> with the proper binding.  This can be identified by the same flag as
> the eld notifier.
> 
> Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: <stable at vger.kernel.org> # v4.5
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>  sound/pci/hda/patch_hdmi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index e7d9453ecd10..56d3575ee6cc 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1741,7 +1741,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>  
>  	/* Call sync_audio_rate to set the N/CTS/M manually if necessary */
>  	/* Todo: add DP1.2 MST audio support later */
> -	snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);
> +	if (codec_has_acomp(codec))
> +		snd_hdac_sync_audio_rate(&codec->bus->core, pin_nid, runtime->rate);

Yeah seems like it should do it.

Though I'm not entirely enthusiastic about maintaining two code paths
for audio stuff. So longer term I'd love to be able rip out the
hardware based ELD passing from i915 and just use the audio component
even for the oldest platforms.

>  
>  	non_pcm = check_non_pcm_per_cvt(codec, cvt_nid);
>  	mutex_lock(&per_pin->lock);
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC