| Message ID | 1458536609-70400-1-git-send-email-libin.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 21 Mar 2016 06:03:29 +0100, libin.yang@linux.intel.com wrote: > > From: Libin Yang <libin.yang@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@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. Actually, if we do zero-clear, a safer fix would be to do it at the beginning. The similar "leak" can be seen, as we do memcpy() only for drm_eld_size() bytes at max. thanks, Takashi > --- > drivers/gpu/drm/i915/intel_audio.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index fdc8b2a..9d3879e 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -732,6 +732,13 @@ static int i915_audio_component_get_eld(struct device *dev, int port, > ret = drm_eld_size(eld); > memcpy(buf, eld, min(max_bytes, ret)); > } > + } else { > + /* dev_priv->dig_port_map[port] will be set NULL in > + * intel_audio_codec_disable when disconnecting monitor. > + * We need clear the eld buf. > + */ > + memset(buf, 0, max_bytes); > + ret = 0; > } > > mutex_unlock(&dev_priv->av_mutex); > -- > 1.9.1 >
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Monday, March 21, 2016 6:45 PM > To: libin.yang@linux.intel.com > Cc: intel-gfx@lists.freedesktop.org; conselvan2@gmail.com; > jani.nikula@linux.intel.com; ville.syrjala@linux.intel.com; Vetter, Daniel; > tiwai@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@linux.intel.com wrote: > > > > From: Libin Yang <libin.yang@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@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. > > Actually, if we do zero-clear, a safer fix would be to do it at the > beginning. The similar "leak" can be seen, as we do memcpy() only for > drm_eld_size() bytes at max. OK. I will clear at the beginning. > > > thanks, > > Takashi > > > --- > > drivers/gpu/drm/i915/intel_audio.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > b/drivers/gpu/drm/i915/intel_audio.c > > index fdc8b2a..9d3879e 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -732,6 +732,13 @@ static int > i915_audio_component_get_eld(struct device *dev, int port, > > ret = drm_eld_size(eld); > > memcpy(buf, eld, min(max_bytes, ret)); > > } > > + } else { > > + /* dev_priv->dig_port_map[port] will be set NULL in > > + * intel_audio_codec_disable when disconnecting > monitor. > > + * We need clear the eld buf. > > + */ > > + memset(buf, 0, max_bytes); > > + ret = 0; > > } > > > > mutex_unlock(&dev_priv->av_mutex); > > -- > > 1.9.1 > >
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index fdc8b2a..9d3879e 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -732,6 +732,13 @@ static int i915_audio_component_get_eld(struct device *dev, int port, ret = drm_eld_size(eld); memcpy(buf, eld, min(max_bytes, ret)); } + } else { + /* dev_priv->dig_port_map[port] will be set NULL in + * intel_audio_codec_disable when disconnecting monitor. + * We need clear the eld buf. + */ + memset(buf, 0, max_bytes); + ret = 0; } mutex_unlock(&dev_priv->av_mutex);