* [PATCH] drm/i915: add audio_ptr pointer check
@ 2016-03-02 2:24 libin.yang
2016-03-02 7:44 ` Takashi Iwai
2016-03-02 8:31 ` ✗ Fi.CI.BAT: warning for " Patchwork
0 siblings, 2 replies; 7+ messages in thread
From: libin.yang @ 2016-03-02 2:24 UTC (permalink / raw)
To: intel-gfx, conselvan2, jani.nikula, ville.syrjala, daniel.vetter,
tiwai
Cc: Libin Yang
From: Libin Yang <libin.yang@linux.intel.com>
check to make sure audio_ptr is not NULL before
using it.
Signed-off-by: Libin Yang <libin.yang@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: add audio_ptr pointer check
2016-03-02 2:24 [PATCH] drm/i915: add audio_ptr pointer check libin.yang
@ 2016-03-02 7:44 ` Takashi Iwai
2016-03-02 7:59 ` Yang, Libin
2016-03-02 8:00 ` Jani Nikula
2016-03-02 8:31 ` ✗ Fi.CI.BAT: warning for " Patchwork
1 sibling, 2 replies; 7+ messages in thread
From: Takashi Iwai @ 2016-03-02 7:44 UTC (permalink / raw)
To: libin.yang; +Cc: intel-gfx, tiwai, daniel.vetter
On Wed, 02 Mar 2016 03:24:31 +0100,
libin.yang@linux.intel.com wrote:
>
> From: Libin Yang <libin.yang@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.
thanks,
Takashi
>
> Signed-off-by: Libin Yang <libin.yang@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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: add audio_ptr pointer check
2016-03-02 7:44 ` Takashi Iwai
@ 2016-03-02 7:59 ` Yang, Libin
2016-03-02 8:56 ` Takashi Iwai
2016-03-02 8:00 ` Jani Nikula
1 sibling, 1 reply; 7+ messages in thread
From: Yang, Libin @ 2016-03-02 7:59 UTC (permalink / raw)
To: Takashi Iwai, libin.yang@linux.intel.com
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, March 02, 2016 3:44 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: add audio_ptr pointer check
>
> On Wed, 02 Mar 2016 03:24:31 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@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@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
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: add audio_ptr pointer check
2016-03-02 7:59 ` Yang, Libin
@ 2016-03-02 8:56 ` Takashi Iwai
2016-03-03 3:01 ` Yang, Libin
0 siblings, 1 reply; 7+ messages in thread
From: Takashi Iwai @ 2016-03-02 8:56 UTC (permalink / raw)
To: Yang, Libin
Cc: intel-gfx@lists.freedesktop.org, Vetter, Daniel,
libin.yang@linux.intel.com
On Wed, 02 Mar 2016 08:59:23 +0100,
Yang, Libin wrote:
>
> Hi Takashi,
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Wednesday, March 02, 2016 3:44 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: add audio_ptr pointer check
> >
> > On Wed, 02 Mar 2016 03:24:31 +0100,
> > libin.yang@linux.intel.com wrote:
> > >
> > > From: Libin Yang <libin.yang@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.
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: add audio_ptr pointer check
2016-03-02 8:56 ` Takashi Iwai
@ 2016-03-03 3:01 ` Yang, Libin
0 siblings, 0 replies; 7+ messages in thread
From: Yang, Libin @ 2016-03-03 3:01 UTC (permalink / raw)
To: Takashi Iwai
Cc: intel-gfx@lists.freedesktop.org, Vetter, Daniel,
libin.yang@linux.intel.com
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, March 02, 2016 4:57 PM
> To: Yang, Libin
> Cc: libin.yang@linux.intel.com; intel-gfx@lists.freedesktop.org;
> conselvan2@gmail.com; jani.nikula@linux.intel.com;
> ville.syrjala@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@suse.de]
> > > Sent: Wednesday, March 02, 2016 3:44 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: add audio_ptr pointer check
> > >
> > > On Wed, 02 Mar 2016 03:24:31 +0100,
> > > libin.yang@linux.intel.com wrote:
> > > >
> > > > From: Libin Yang <libin.yang@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: add audio_ptr pointer check
2016-03-02 7:44 ` Takashi Iwai
2016-03-02 7:59 ` Yang, Libin
@ 2016-03-02 8:00 ` Jani Nikula
1 sibling, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2016-03-02 8:00 UTC (permalink / raw)
To: libin.yang; +Cc: intel-gfx, tiwai, daniel.vetter
On Wed, 02 Mar 2016, Takashi Iwai <tiwai@suse.de> wrote:
> On Wed, 02 Mar 2016 03:24:31 +0100,
> libin.yang@linux.intel.com wrote:
>>
>> From: Libin Yang <libin.yang@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.
Agreed.
BR,
Jani
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: add audio_ptr pointer check
2016-03-02 2:24 [PATCH] drm/i915: add audio_ptr pointer check libin.yang
2016-03-02 7:44 ` Takashi Iwai
@ 2016-03-02 8:31 ` Patchwork
1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2016-03-02 8:31 UTC (permalink / raw)
To: libin.yang; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: add audio_ptr pointer check
URL : https://patchwork.freedesktop.org/series/4002/
State : warning
== Summary ==
Series 4002v1 drm/i915: add audio_ptr pointer check
http://patchwork.freedesktop.org/api/1.0/series/4002/revisions/1/mbox/
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> DMESG-WARN (bdw-ultra)
pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (snb-x220t)
Subgroup basic-plain-flip:
pass -> DMESG-WARN (hsw-brixbox)
Test kms_force_connector_basic:
Subgroup force-load-detect:
fail -> SKIP (ilk-hp8440p)
Subgroup prune-stale-modes:
pass -> SKIP (ilk-hp8440p)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> DMESG-WARN (hsw-brixbox)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
pass -> DMESG-WARN (hsw-gt2)
Test pm_rpm:
Subgroup basic-pci-d3-state:
dmesg-fail -> FAIL (snb-x220t)
pass -> DMESG-WARN (snb-dellxps)
Subgroup basic-rte:
pass -> DMESG-WARN (snb-x220t)
bdw-nuci7 total:169 pass:158 dwarn:0 dfail:0 fail:0 skip:11
bdw-ultra total:169 pass:154 dwarn:1 dfail:0 fail:0 skip:14
bsw-nuc-2 total:169 pass:137 dwarn:1 dfail:0 fail:1 skip:30
byt-nuc total:169 pass:144 dwarn:0 dfail:0 fail:0 skip:25
hsw-brixbox total:169 pass:152 dwarn:2 dfail:0 fail:0 skip:15
hsw-gt2 total:169 pass:156 dwarn:2 dfail:0 fail:1 skip:10
ilk-hp8440p total:169 pass:115 dwarn:2 dfail:0 fail:0 skip:52
ivb-t430s total:169 pass:153 dwarn:0 dfail:0 fail:1 skip:15
skl-i5k-2 total:169 pass:153 dwarn:0 dfail:0 fail:0 skip:16
skl-i7k-2 total:169 pass:153 dwarn:0 dfail:0 fail:0 skip:16
snb-dellxps total:169 pass:143 dwarn:2 dfail:0 fail:1 skip:23
snb-x220t total:169 pass:144 dwarn:1 dfail:0 fail:2 skip:22
Results at /archive/results/CI_IGT_test/Patchwork_1510/
f9cadb616ff17d482312fba07db772b6604ce799 drm-intel-nightly: 2016y-03m-01d-17h-16m-32s UTC integration manifest
fe32b47cd9d0b86c2f46da80b4e02cbbccc9cbee drm/i915: add audio_ptr pointer check
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-03-03 3:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-02 2:24 [PATCH] drm/i915: add audio_ptr pointer check libin.yang
2016-03-02 7:44 ` Takashi Iwai
2016-03-02 7:59 ` Yang, Libin
2016-03-02 8:56 ` Takashi Iwai
2016-03-03 3:01 ` Yang, Libin
2016-03-02 8:00 ` Jani Nikula
2016-03-02 8:31 ` ✗ Fi.CI.BAT: warning for " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox