[Intel-gfx] [PATCH] drm/i915: intel_audio clear eld buf when disconnecting monitor

[Daniel Vetter]

On Tue, Mar 22, 2016 at 08:15:42AM +0100, Takashi Iwai wrote:
> On Tue, 22 Mar 2016 02:47:14 +0100,
> Yang, Libin wrote:
> > 
> > Hi Takashi,
> > 
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > Sent: Monday, March 21, 2016 11:30 PM
> > > To: Yang, Libin
> > > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> > > conselvan2 at gmail.com; jani.nikula at linux.intel.com;
> > > ville.syrjala at linux.intel.com; Vetter, Daniel
> > > Subject: Re: [PATCH] drm/i915: intel_audio clear eld buf when
> > > disconnecting monitor
> > > 
> > > On Mon, 21 Mar 2016 16:19:31 +0100,
> > > Takashi Iwai wrote:
> > > >
> > > > On Mon, 21 Mar 2016 16:12:05 +0100,
> > > > Takashi Iwai wrote:
> > > > >
> > > > > On Mon, 21 Mar 2016 16:00:21 +0100,
> > > > > Takashi Iwai wrote:
> > > > > >
> > > > > > On Mon, 21 Mar 2016 15:17:37 +0100,
> > > > > > Yang, Libin wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > > > > > > Sent: Monday, March 21, 2016 6:45 PM
> > > > > > > > To: libin.yang at linux.intel.com
> > > > > > > > Cc: intel-gfx at lists.freedesktop.org; conselvan2 at gmail.com;
> > > > > > > > jani.nikula at linux.intel.com; ville.syrjala at linux.intel.com; Vetter,
> > > Daniel;
> > > > > > > > tiwai at suse.de; Yang, Libin
> > > > > > > > Subject: Re: [PATCH] drm/i915: intel_audio clear eld buf when
> > > > > > > > disconnecting monitor
> > > > > > > >
> > > > > > > > On Mon, 21 Mar 2016 06:03:29 +0100,
> > > > > > > > libin.yang at linux.intel.com wrote:
> > > > > > > > >
> > > > > > > > > From: Libin Yang <libin.yang at linux.intel.com>
> > > > > > > > >
> > > > > > > > > When disconnecting monitor, dev_priv->dig_port_map[port]
> > > > > > > > > will be set NULL, which causes eld will not be updated in
> > > > > > > > > i915_audio_component_get_eld().
> > > > > > > > >
> > > > > > > > > This patch clears the eld buf when dev_priv-
> > > >dig_port_map[port]
> > > > > > > > > is NULL.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> > > > > > > >
> > > > > > > > While this isn't certainly bad, I don't think it's mandatory.  The
> > > > > > > > function returns zero, i.e. no data is copied.  So the caller
> > > > > > > > shouldn't expect that the buffer is cleared in this case.
> > > > > > >
> > > > > > > Without the patch, we find when unplug the monitor, the eld info
> > > > > > > will not be updated. The means the eld info in the procfs still
> > > remains
> > > > > > > the old info after the monitor is disconnected.
> > > > > >
> > > > > > Well, it's not about zero-clear but rather because the function
> > > > > > returns an error (-EINVAL), and the caller takes it too seriously.
> > > > > > Upon receiving an error code, the HDA driver doesn't read ELD at all,
> > > > > > so it won't help even if you do zero-clear there.
> > > > > >
> > > > > > The alternative fix patch is below.
> > > > >
> > > > > ... or we can fix it in HDA side like below, too.
> > > >
> > > > This patch won't be applied cleanly on the upstream tree as I wrote it
> > > > on the my working tree, sorry.  Below is the revised one that is
> > > > applicable to upstream tree.
> > > 
> > > Gah, a typo was included in this patch, too.  Sorry, the correct one
> > > is attached below.
> > 
> > Yes. The patch seems more reasonable than mine.
> > 
> > Do we still need the patch to set i915_audio_component_get_eld()
> > return value to 0? It seems -EINVAL makes sense.
> 
> Yes, we can drop i915 change.  I queued my fix for HD-audio side for
> the next pull request.

Awesome, thx everyone for tracking this down.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch