Re: [PATCH v5] drm/i915: pass ELD to HDMI/DP audio driver

[Wu Fengguang <fengguang.wu@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

(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

[-- Attachment #2: alsa-hdmi-remove-printk --]
[-- Type: text/plain, Size: 1970 bytes --]

Subject: alsa: hide HDMI/ELD printks unless in debug kernels
Date: Fri Nov 11 19:09:36 CST 2011


Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 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);
 

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx