[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 4:57 PM
> To: Yang, Libin
> Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org;
> conselvan2 at gmail.com; jani.nikula at linux.intel.com;
> ville.syrjala at linux.intel.com; Vetter, Daniel
> Subject: Re: [PATCH] drm/i915: add audio_ptr pointer check
> 
> On Wed, 02 Mar 2016 08:59:23 +0100,
> Yang, Libin wrote:
> >
> > 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?).
> 
> There are several ways to deal with such a thing.  An easy one would
> be the memory barrier.

Yes, agree to use mb. It will be easier and safer.

I think wmb() should be enough for this case, right?

I will do the stress test on mb.

Regards,
Libin

> 
> 
> Takashi