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

[Takashi Iwai]

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.


Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] drm/i915: Don't handle NULL encoder as an error in get_eld
 notifier

Since we fixed the dig_port_map[] assignment rather in a dynamic
manner by the commit [2f791908a70e: drm/i915: Fix bogus dig_port_map[]
assignment for pre-HSW], the NULL encoder is no longer an error but it
just indicates that the unconnected state.  However, in the caller
(HD-audio) side, it's taken as a serious error, and it leads to the
missing update of ELD after unplugging.

As a fix, this patch changes get_eld notifier to return zero and
initialize the enabled argument properly as false when no encoder is
mapped for indicating the unconnected state.

Fixes: 2f791908a70e ('drm/i915: Fix bogus dig_port_map[] assignment for pre-HSW')
Cc: <stable at vger.kernel.org> # v4.5
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 drivers/gpu/drm/i915/intel_audio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 31f6d212fb1b..63493a63500e 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -717,13 +717,13 @@ static int i915_audio_component_get_eld(struct device *dev, int port,
 	struct intel_encoder *intel_encoder;
 	struct intel_digital_port *intel_dig_port;
 	const u8 *eld;
-	int ret = -EINVAL;
+	int ret = 0;
 
+	*enabled = false;
 	mutex_lock(&dev_priv->av_mutex);
 	intel_encoder = dev_priv->dig_port_map[port];
 	/* intel_encoder might be NULL for DP MST */
 	if (intel_encoder) {
-		ret = 0;
 		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
 		*enabled = intel_dig_port->audio_connector != NULL;
 		if (*enabled) {
-- 
2.7.4