Hi Takashi, > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Monday, March 21, 2016 11:30 PM > To: Yang, Libin > Cc: libin.yang@xxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > conselvan2@xxxxxxxxx; jani.nikula@xxxxxxxxxxxxxxx; > ville.syrjala@xxxxxxxxxxxxxxx; 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@xxxxxxx] > > > > > > Sent: Monday, March 21, 2016 6:45 PM > > > > > > To: libin.yang@xxxxxxxxxxxxxxx > > > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; conselvan2@xxxxxxxxx; > > > > > > jani.nikula@xxxxxxxxxxxxxxx; ville.syrjala@xxxxxxxxxxxxxxx; Vetter, > Daniel; > > > > > > tiwai@xxxxxxx; 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@xxxxxxxxxxxxxxx wrote: > > > > > > > > > > > > > > From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx> > > > > > > > > > > > > > > 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@xxxxxxxxxxxxxxx> > > > > > > > > > > > > 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@xxxxxxx> > 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@xxxxxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # v4.5 > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > --- > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
