drm/i915: intel_audio clear eld buf when disconnecting monitor

Commit Message

Takashi Iwai March 21, 2016, 3:19 p.m. UTC

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:[e-mail]]
> > > > Sent: Monday, March 21, 2016 6:45 PM
> > > > To: [e-mail]
> > > > Cc: [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail]; Vetter, Daniel;
> > > > [e-mail]; Yang, Libin
> > > > Subject: Re: [PATCH] drm/i915: intel_audio clear eld buf when
> > > > disconnecting monitor
> > > > 
> > > > On Mon, 21 Mar 2016 06:03:29 +0100,
> > > > [e-mail] wrote:
> > > > >
> > > > > From: Libin Yang <[e-mail]>
> > > > >
> > > > > 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 <[e-mail]>
> > > > 
> > > > 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.


Takashi

-- 8< --
From: Takashi Iwai <[e-mail]>
Subject: [PATCH v2] ALSA: hda - Fix missing ELD update at unplugging

i915 get_eld ops may return an error when no encoder is connected, and
currently we regard the error as fatal and skip the whole ELD
handling.  This ended up with the missing ELD update at unplugging.

This patch fixes the issue by treating the error as the unplugged
state, instead of skipping the rest.

Reported-by: Libin Yang <[e-mail]>
Cc: <[e-mail]> # v4.5
Signed-off-by: Takashi Iwai <[e-mail]>
---
 sound/pci/hda/patch_hdmi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

kernel test robot March 21, 2016, 11:23 p.m. UTC | #1

Hi Takashi,

[auto build test ERROR on v4.5-rc7]
[also build test ERROR on next-20160321]
[cannot apply to sound/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Takashi-Iwai/ALSA-hda-Fix-missing-ELD-update-at-unplugging/20160322-045538
config: x86_64-randconfig-s5-03220710 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   sound/pci/hda/patch_hdmi.c: In function 'sync_eld_via_acomp':
>> sound/pci/hda/patch_hdmi.c:1668:2: error: invalid type argument of unary '*' (have 'int')
     *eld->monitor_present = false;
     ^
   sound/pci/hda/patch_hdmi.c:1690:2: warning: label 'unlock' defined but not used [-Wunused-label]
     unlock:
     ^

vim +1668 sound/pci/hda/patch_hdmi.c

  1662	{
  1663		struct hdmi_spec *spec = codec->spec;
  1664		struct hdmi_eld *eld = &spec->temp_eld;
  1665		int size;
  1666	
  1667		mutex_lock(&per_pin->lock);
> 1668		*eld->monitor_present = false;
  1669		size = snd_hdac_acomp_get_eld(&codec->bus->core, per_pin->pin_nid,
  1670					      &eld->monitor_present, eld->eld_buffer,
  1671					      ELD_MAX_SIZE);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 7ae614d27954..d9a557ea7a5b 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1484,11 +1484,10 @@  static void sync_eld_via_acomp(struct hda_codec *codec,
 	int size;
 
 	mutex_lock(&per_pin->lock);
+	*eld->monitor_present = false;
 	size = snd_hdac_acomp_get_eld(&codec->bus->core, per_pin->pin_nid,
 				      &eld->monitor_present, eld->eld_buffer,
 				      ELD_MAX_SIZE);
-	if (size < 0)
-		goto unlock;
 	if (size > 0) {
 		size = min(size, ELD_MAX_SIZE);
 		if (snd_hdmi_parse_eld(codec, &eld->info,