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

[Yang, Libin]

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.

Regards,
Libin

> 
> 
> Takashi -- obviously needs a coffee break
> 
> -- 8< --
> From: Takashi Iwai <tiwai at suse.de>
> Subject: [PATCH v3] 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 <libin.yang at linux.intel.com>
> Cc: <stable at vger.kernel.org> # v4.5
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>  sound/pci/hda/patch_hdmi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 7ae614d27954..5af372d01834 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,
> --
> 2.7.4