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

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

On Fri, Nov 11, 2011 at 04:49:57PM +0800, Takashi Iwai wrote:
> At Fri, 11 Nov 2011 16:22:41 +0800,
> Wu Fengguang wrote:
> > 
> > On Fri, Nov 11, 2011 at 03:40:37PM +0800, Takashi Iwai wrote:
> > > At Fri, 11 Nov 2011 10:29:25 +0800,
> > > Wu Fengguang wrote:
> > > > 
> > > > On Thu, Nov 10, 2011 at 10:28:19PM +0800, Takashi Iwai wrote:
> > > > > At Thu, 10 Nov 2011 21:51:50 +0800,
> > > > > Wu Fengguang wrote:
> > > > > > 
> > > > > > > > > > So maybe the hardware is in some state that is unable to provide the
> > > > > > > > > > real ELD content?
> > > > > > > > > That's my guess as well. I think the hardware may still be doing some 
> > > > > > > > > form of data negotiation with the HDMI display device at that stage, and 
> > > > > > > > > doesn't have the copy of the EDID+ELD buffer until a tiny bit later. 
> > > > > > > > > Possibly?
> > > > > > > > 
> > > > > > > > Look at the below dmesg. The ELD seem to available immediately after the DPMS
> > > > > > > > state setting..
> > > > > > > 
> > > > > > > Interesting.  Does HDMI audio work at all while HDMI DPMS off?
> > > > > > > It clears SDVO_ENABLE bit, so this might turn off both video and
> > > > > > > audio?
> > > > > > 
> > > > > > We normally see transient blank screen and silence of audio when
> > > > > > switching the video mode.
> > > > > 
> > > > > Well, what I suspected is that ELD won't be transferred while
> > > > > SDVO_ENABLE is cleared.
> > > > 
> > > > It's not about SDVO_ENABLE. The transient ELD invalid state I see in
> > > > dmesg is caused by the graphics driver doing
> > > > 
> > > >         ELD_Valid = 0           => trigger 1st unsolicited event
> > > 
> > > But why this triggers at *plugging*?  Wasn't it zero beforehand?
> > > If I understand correctly, changing AUD_CNTL_ST register triggers the
> > > HD-audio codec unsol event.  Writing even the same value matters?
> > 
> > Sorry I assumed the "mode switching" context.
> 
> Ah, I see.  But this could be suppressed by your patch
>    drm/i915: don't trigger hotplug events on unchanged ELD
> no?

Mostly. The ELD may still change if the mode switching changes the
av-sync-delay field, or suppresses the bandwidth available to audio
samples.

> (snip)
> > > > Depending on the timing, the 1st unsolicited event may see
> > > > ELD_Valid=0 (if it's fast enough) or ELD_Valid=1 (if the event
> > > > handling is delayed after the graphics driver sets ELD_Valid=1).
> > > > 
> > > > I know that because I literally saw both cases happening in dmesg.
> > > > The 1st hot plug event itself will send ELD_Valid=0, however the audio
> > > > driver is not trusting this and always do a status query (the HDMI
> > > > status line) whose result depends on the timing.
> > > 
> > > The problem in this procedure is that this looks as if you are
> > > re-connecting the HDMI from the audio-codec POV.
> > 
> > The re-connecting events can be distinguished from the
> > video-mode-switching events by the Presence_Detect bit.
> 
> Hm, OK, so the codec driver can simply ignore (or postpone) the case
> when the connection is kept but ELD is invalidated.

Yes.

> > Here is one hot removal event (I just wrote a patch to trigger this),
> > with Presence_Detect=0:
> 
> 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? ;-)

> > [   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.

And it feels not good that accessing ELD may be blocked for some time..

So I now prefer to avoid the msleep totally and schedule a delayed
work for parsing ELD.

Thanks,
Fengguang