From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wu Fengguang Subject: Re: [PATCH v5] drm/i915: pass ELD to HDMI/DP audio driver Date: Fri, 11 Nov 2011 19:12:57 +0800 Message-ID: <20111111111257.GA7118@localhost> References: <20111110131753.GA14160@localhost> <20111110135150.GB17079@localhost> <20111111022925.GA10466@localhost> <20111111082241.GA29340@localhost> <20111111092421.GA6092@localhost> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="d6Gm4EdcadzBjdND" Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id EC0AE9E7E6 for ; Fri, 11 Nov 2011 03:13:00 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Takashi Iwai Cc: "Wang, Zhenyu Z" , "intel-gfx@lists.freedesktop.org" , "Barnes, Jesse" , Jeremy Bush , Christopher White , "Bossart, Pierre-louis" List-Id: intel-gfx@lists.freedesktop.org --d6Gm4EdcadzBjdND Content-Type: text/plain; charset=us-ascii Content-Disposition: inline (snip) > > > One note that we don't rely on PD bit because not all (non-Intel) > > > hardware report it correctly. > > > > Oops. Do you imply ELDV is reliable on all platforms? ;-) > > Oh hell, no :) > The driver tries to probe explicitly via GET_PIN_SENSE HD-audio verb. Yeah the below "HDMI status:..." line. Can we rely on it then? > > > > [ 91.777028] [drm:ironlake_write_eld], ELD on pipe B > > > > [ 91.778561] HDMI hot plug event: Codec=3 Pin=6 Presence_Detect=0 ELD_Valid=1 > > > > [ 91.783078] HDMI status: Codec=3 Pin=6 Presence_Detect=0 ELD_Valid=0 > > > > [ 91.783083] [drm:ironlake_write_eld], Audio directed to unknown port > > > > [ 91.783095] [drm:output_poll_execute], [CONNECTOR:12:HDMI-A-2] status updated from 1 to 2 > > > > > > > > The HDA spec even mentioned doing some timeout mechanism for the > > > > "Presence_Detect=1 ELD_Valid=0" state. Well it may help some corner > > > > cases, but perhaps not an urgent feature. > > > > > > Yeah, this sounds like the workaround for such a case. > > > > Yeah, your mentioned DVI case may be always in state > > > > "Presence_Detect=1 ELD_Valid=0" > > > > where audio playback should be denied. > > > > And there might be the error case that the 2nd event is lost or not > > generated at all for changing > > > > "Presence_Detect=1 ELD_Valid=0" > > to > > "Presence_Detect=1 ELD_Valid=1" > > > > > > > We might end up with some delayed probe with a dedicated work_struct > > > > > (because it's bad to have a too long delay in unsol event handler > > > > > that run on a single workq). > > > > > > > > Understand. What if the graphics driver can delay the ELD writing (I > > > > can try that), so that the audio driver only need to wait for > > > > something like 10ms? > > > > > > Or, we can introduce a dirty flag, and set it when ELD is changed, > > > but don't prase ELD contents yet. First upon the next access, the > > > driver updates the status, and clear the dirty flag. We may put a > > > small delay at this update, too. > > > > It should work fine for "cat /proc/asound/card0/eld*" and other > > interfaces, however it could still delay the printks' significantly. > > Well, this reminds me of another question -- do we need these printks > unconditionally? Maybe not. How about the attached patch to remove them all? > > And it feels not good that accessing ELD may be blocked for some time.. > > Understood. > > > So I now prefer to avoid the msleep totally and schedule a delayed > > work for parsing ELD. > > OK, let me know if you have some test material. OK! Thanks, Fengguang --d6Gm4EdcadzBjdND Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=alsa-hdmi-remove-printk Subject: alsa: hide HDMI/ELD printks unless in debug kernels Date: Fri Nov 11 19:09:36 CST 2011 Signed-off-by: Wu Fengguang --- sound/pci/hda/hda_eld.c | 6 +++--- sound/pci/hda/patch_hdmi.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) --- linux.orig/sound/pci/hda/hda_eld.c 2011-11-11 19:08:21.000000000 +0800 +++ linux/sound/pci/hda/hda_eld.c 2011-11-11 19:09:35.000000000 +0800 @@ -410,7 +410,7 @@ static void hdmi_show_short_audio_desc(s else buf2[0] = '\0'; - printk(KERN_INFO "HDMI: supports coding type %s:" + snd_printdd("HDMI: supports coding type %s:" " channels = %d, rates =%s%s\n", cea_audio_coding_type_names[a->format], a->channels, @@ -434,14 +434,14 @@ void snd_hdmi_show_eld(struct hdmi_eld * { int i; - printk(KERN_INFO "HDMI: detected monitor %s at connection type %s\n", + snd_printdd("HDMI: detected monitor %s at connection type %s\n", e->monitor_name, eld_connection_type_names[e->conn_type]); if (e->spk_alloc) { char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; snd_print_channel_allocation(e->spk_alloc, buf, sizeof(buf)); - printk(KERN_INFO "HDMI: available speakers:%s\n", buf); + snd_printdd("HDMI: available speakers:%s\n", buf); } for (i = 0; i < e->sad_count; i++) --- linux.orig/sound/pci/hda/patch_hdmi.c 2011-11-11 19:07:15.000000000 +0800 +++ linux/sound/pci/hda/patch_hdmi.c 2011-11-11 19:08:16.000000000 +0800 @@ -757,7 +757,7 @@ static void hdmi_intrinsic_event(struct int pin_idx; struct hdmi_eld *eld; - printk(KERN_INFO + snd_printdd( "HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, pd, eldv); @@ -989,7 +989,7 @@ static void hdmi_present_sense(struct hd else eld->eld_valid = 0; - printk(KERN_INFO + snd_printdd( "HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, eld->monitor_present, eld->eld_valid); --d6Gm4EdcadzBjdND Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --d6Gm4EdcadzBjdND--