From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754903Ab1KVKxX (ORCPT ); Tue, 22 Nov 2011 05:53:23 -0500 Received: from mga14.intel.com ([143.182.124.37]:23156 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752653Ab1KVKxV (ORCPT ); Tue, 22 Nov 2011 05:53:21 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.69,552,1315206000"; d="scan'208";a="77906910" Date: Tue, 22 Nov 2011 18:53:18 +0800 From: Wu Fengguang To: Takashi Iwai Cc: "alsa-devel@alsa-project.org" , LKML Subject: Re: [alsa-devel] [PATCH] hda - fail ELD reading early Message-ID: <20111122105318.GB5730@localhost> References: <20111122084623.GA30050@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 22, 2011 at 06:39:53PM +0800, Takashi Iwai wrote: > At Tue, 22 Nov 2011 16:46:23 +0800, > Wu Fengguang wrote: > > > > With the ELD repoll mechanism, we can (and should) fail the ELD reading > > immediately when find something obviously wrong and let the caller retry > > after some delay. > > > > Signed-off-by: Wu Fengguang > > --- > > sound/pci/hda/hda_eld.c | 28 +++++++++++++++++++--------- > > 1 file changed, 19 insertions(+), 9 deletions(-) > > > > --- linux.orig/sound/pci/hda/hda_eld.c 2011-11-22 16:02:58.000000000 +0800 > > +++ linux/sound/pci/hda/hda_eld.c 2011-11-22 16:36:10.000000000 +0800 > > @@ -347,18 +347,28 @@ int snd_hdmi_get_eld(struct hdmi_eld *el > > > > for (i = 0; i < size; i++) { > > unsigned int val = hdmi_get_eld_data(codec, nid, i); > > + /* > > + * Graphics driver might be writing to ELD buffer right now. > > + * Just abort. The caller will repoll after a while. > > + */ > > if (!(val & AC_ELDD_ELD_VALID)) { > > - if (!i) { > > - snd_printd(KERN_INFO > > - "HDMI: invalid ELD data\n"); > > - ret = -EINVAL; > > - goto error; > > - } > > snd_printd(KERN_INFO > > "HDMI: invalid ELD data byte %d\n", i); > > - val = 0; > > - } else > > - val &= AC_ELDD_ELD_DATA; > > + ret = -EINVAL; > > + goto error; > > + } > > + val &= AC_ELDD_ELD_DATA; > > + /* > > + * The first byte cannot be zero. This can happen on some DVI > > + * connections. Some Intel chips may also need some 250ms delay > > + * to return non-zero ELD data, even when the graphics driver > > + * correctly writes ELD content before setting ELD_valid bit. > > + */ > > + if (!val && !i) { > > + snd_printdd(KERN_INFO "HDMI: 0 ELD data\n"); > > + ret = -EINVAL; > > + goto error; > > + } > > Shouldn't this zero-check be before the valid-bit check? > Otherwise it'll never reach there. It does reach there: [ 1191.016746] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1 [ 1191.019309] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1 ==> [ 1191.021803] ALSA hda_eld.c:368 HDMI: 0 ELD data [ 1191.324661] HDMI status: Codec=3 Pin=6 Presence_Detect=1 ELD_Valid=1 [ 1191.333236] HDMI: detected monitor SONY TV at connection type HDMI [ 1191.335020] HDMI: available speakers: FL/FR [ 1191.335996] HDMI: supports coding type LPCM: channels = 2, rates = 32000 44100 48000, bits = 16 20 24 The funny thing is, it's reporting (invalid) 0 ELD data that has the AC_ELDD_ELD_VALID bit set. Thanks, Fengguang