[Intel-gfx] [PATCH] drm/i915: add audio_ptr pointer check

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Wednesday, March 02, 2016 3:44 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: add audio_ptr pointer check
> 
> On Wed, 02 Mar 2016 03:24:31 +0100,
> libin.yang at linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang at linux.intel.com>
> >
> > check to make sure audio_ptr is not NULL before
> > using it.
> 
> I don't think non-NULL is mandatory.  If any invalid access is seen,
> it should be fixed rather in the audio side.

When I enable MST with other patches (not submitted so far),
I found system may be random hang when boot up.

After apply this patch and another patch (I sent the patch in
ALSA Mail list: ALSA: hda - hdmi defer to register acomp eld notifier),
the hang issue is fixed.

I make the patch because in sound driver side, setting audio_ptr and
audio_ops is not in the lock. I'm wondering that the compiler or CPU
may optimize and change the setting order (the gfx code and alsa code
may run parallelly?).

Regards,
Libin

> 
> 
> thanks,
> 
> Takashi
> 
> >
> > Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index 31f6d21..667596d 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -529,7 +529,8 @@ void intel_audio_codec_enable(struct
> intel_encoder *intel_encoder)
> >  	intel_dig_port->audio_connector = connector;
> >  	mutex_unlock(&dev_priv->av_mutex);
> >
> > -	if (acomp && acomp->audio_ops && acomp->audio_ops-
> >pin_eld_notify)
> > +	if (acomp && acomp->audio_ops && acomp->audio_ops-
> >pin_eld_notify &&
> > +	    acomp->audio_ops->audio_ptr)
> >  		acomp->audio_ops->pin_eld_notify(acomp->audio_ops-
> >audio_ptr, (int) port);
> >  }
> >
> > @@ -556,7 +557,8 @@ void intel_audio_codec_disable(struct
> intel_encoder *intel_encoder)
> >  	intel_dig_port->audio_connector = NULL;
> >  	mutex_unlock(&dev_priv->av_mutex);
> >
> > -	if (acomp && acomp->audio_ops && acomp->audio_ops-
> >pin_eld_notify)
> > +	if (acomp && acomp->audio_ops && acomp->audio_ops-
> >pin_eld_notify &&
> > +	    acomp->audio_ops->audio_ptr)
> >  		acomp->audio_ops->pin_eld_notify(acomp->audio_ops-
> >audio_ptr, (int) port);
> >  }
> >
> > --
> > 1.9.1
> >