On Tue, 22 Mar 2016 02:47:14 +0100, Yang, Libin wrote: > > 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. Yes, we can drop i915 change. I queued my fix for HD-audio side for the next pull request. thanks, Takashi _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
