| Message ID | 20221031064918.14178-1-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/radeon: Use a local mutex for bind/unbind protection | expand |
On Mon, Oct 31, 2022 at 2:49 AM Takashi Iwai <tiwai@suse.de> wrote: > > We used drm_modeset_lock_all() for protecting the audio component > bind/unbind operations against the notification, but it seems leading > to kernel WARNING and Oops. Moreover, the use of > drm_modeset_lock_all() is rather an overkill only for this > protection. > > This patch introduces a new mutex that protects just the bind/unbind > and the notify calls and replaces the drm_modeset_lock with it. > > Fixes: 34d84636e5e0 ("drm/radeon: Add HD-audio component notifier support (v4)") > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1569 > Signed-off-by: Takashi Iwai <tiwai@suse.de> Applied. I will squash this in with 34d84636e5e0. Alex > --- > drivers/gpu/drm/radeon/radeon.h | 1 + > drivers/gpu/drm/radeon/radeon_audio.c | 21 ++++++++++++--------- > drivers/gpu/drm/radeon/radeon_device.c | 1 + > 3 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index d82424525f5a..2e7161acd443 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -1799,6 +1799,7 @@ struct r600_audio { > struct radeon_audio_basic_funcs *funcs; > struct drm_audio_component *component; > bool component_registered; > + struct mutex component_mutex; > }; > > /* > diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c > index 4ceb90556127..71b67d4efe08 100644 > --- a/drivers/gpu/drm/radeon/radeon_audio.c > +++ b/drivers/gpu/drm/radeon/radeon_audio.c > @@ -181,8 +181,7 @@ static struct radeon_audio_funcs dce6_dp_funcs = { > .dpms = evergreen_dp_enable, > }; > > -static void radeon_audio_component_notify(struct drm_audio_component *acomp, > - int port); > +static void radeon_audio_component_notify(struct radeon_device *rdev, int port); > > static void radeon_audio_enable(struct radeon_device *rdev, > struct r600_audio_pin *pin, u8 enable_mask) > @@ -212,7 +211,7 @@ static void radeon_audio_enable(struct radeon_device *rdev, > if (rdev->audio.funcs->enable) > rdev->audio.funcs->enable(rdev, pin, enable_mask); > > - radeon_audio_component_notify(rdev->audio.component, pin->id); > + radeon_audio_component_notify(rdev, pin->id); > } > > static void radeon_audio_interface_init(struct radeon_device *rdev) > @@ -731,12 +730,16 @@ unsigned int radeon_audio_decode_dfs_div(unsigned int div) > /* > * Audio component support > */ > -static void radeon_audio_component_notify(struct drm_audio_component *acomp, > - int port) > +static void radeon_audio_component_notify(struct radeon_device *rdev, int port) > { > + struct drm_audio_component *acomp; > + > + mutex_lock(&rdev->audio.component_mutex); > + acomp = rdev->audio.component; > if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) > acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, > port, -1); > + mutex_unlock(&rdev->audio.component_mutex); > } > > static int radeon_audio_component_get_eld(struct device *kdev, int port, > @@ -787,11 +790,11 @@ static int radeon_audio_component_bind(struct device *kdev, > if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS))) > return -ENOMEM; > > - drm_modeset_lock_all(dev); > + mutex_lock(&rdev->audio.component_mutex); > acomp->ops = &radeon_audio_component_ops; > acomp->dev = kdev; > rdev->audio.component = acomp; > - drm_modeset_unlock_all(dev); > + mutex_unlock(&rdev->audio.component_mutex); > > return 0; > } > @@ -805,11 +808,11 @@ static void radeon_audio_component_unbind(struct device *kdev, > > device_link_remove(hda_kdev, kdev); > > - drm_modeset_lock_all(dev); > + mutex_lock(&rdev->audio.component_mutex); > rdev->audio.component = NULL; > acomp->ops = NULL; > acomp->dev = NULL; > - drm_modeset_unlock_all(dev); > + mutex_unlock(&rdev->audio.component_mutex); > } > > static const struct component_ops radeon_audio_component_bind_ops = { > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c > index 17bfbbe906c8..2e13ce2b65d3 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -1312,6 +1312,7 @@ int radeon_device_init(struct radeon_device *rdev, > mutex_init(&rdev->pm.mutex); > mutex_init(&rdev->gpu_clock_mutex); > mutex_init(&rdev->srbm_mutex); > + mutex_init(&rdev->audio.component_mutex); > init_rwsem(&rdev->pm.mclk_lock); > init_rwsem(&rdev->exclusive_lock); > init_waitqueue_head(&rdev->irq.vblank_queue); > -- > 2.35.3 >
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index d82424525f5a..2e7161acd443 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1799,6 +1799,7 @@ struct r600_audio { struct radeon_audio_basic_funcs *funcs; struct drm_audio_component *component; bool component_registered; + struct mutex component_mutex; }; /* diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c index 4ceb90556127..71b67d4efe08 100644 --- a/drivers/gpu/drm/radeon/radeon_audio.c +++ b/drivers/gpu/drm/radeon/radeon_audio.c @@ -181,8 +181,7 @@ static struct radeon_audio_funcs dce6_dp_funcs = { .dpms = evergreen_dp_enable, }; -static void radeon_audio_component_notify(struct drm_audio_component *acomp, - int port); +static void radeon_audio_component_notify(struct radeon_device *rdev, int port); static void radeon_audio_enable(struct radeon_device *rdev, struct r600_audio_pin *pin, u8 enable_mask) @@ -212,7 +211,7 @@ static void radeon_audio_enable(struct radeon_device *rdev, if (rdev->audio.funcs->enable) rdev->audio.funcs->enable(rdev, pin, enable_mask); - radeon_audio_component_notify(rdev->audio.component, pin->id); + radeon_audio_component_notify(rdev, pin->id); } static void radeon_audio_interface_init(struct radeon_device *rdev) @@ -731,12 +730,16 @@ unsigned int radeon_audio_decode_dfs_div(unsigned int div) /* * Audio component support */ -static void radeon_audio_component_notify(struct drm_audio_component *acomp, - int port) +static void radeon_audio_component_notify(struct radeon_device *rdev, int port) { + struct drm_audio_component *acomp; + + mutex_lock(&rdev->audio.component_mutex); + acomp = rdev->audio.component; if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, port, -1); + mutex_unlock(&rdev->audio.component_mutex); } static int radeon_audio_component_get_eld(struct device *kdev, int port, @@ -787,11 +790,11 @@ static int radeon_audio_component_bind(struct device *kdev, if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS))) return -ENOMEM; - drm_modeset_lock_all(dev); + mutex_lock(&rdev->audio.component_mutex); acomp->ops = &radeon_audio_component_ops; acomp->dev = kdev; rdev->audio.component = acomp; - drm_modeset_unlock_all(dev); + mutex_unlock(&rdev->audio.component_mutex); return 0; } @@ -805,11 +808,11 @@ static void radeon_audio_component_unbind(struct device *kdev, device_link_remove(hda_kdev, kdev); - drm_modeset_lock_all(dev); + mutex_lock(&rdev->audio.component_mutex); rdev->audio.component = NULL; acomp->ops = NULL; acomp->dev = NULL; - drm_modeset_unlock_all(dev); + mutex_unlock(&rdev->audio.component_mutex); } static const struct component_ops radeon_audio_component_bind_ops = { diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 17bfbbe906c8..2e13ce2b65d3 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1312,6 +1312,7 @@ int radeon_device_init(struct radeon_device *rdev, mutex_init(&rdev->pm.mutex); mutex_init(&rdev->gpu_clock_mutex); mutex_init(&rdev->srbm_mutex); + mutex_init(&rdev->audio.component_mutex); init_rwsem(&rdev->pm.mclk_lock); init_rwsem(&rdev->exclusive_lock); init_waitqueue_head(&rdev->irq.vblank_queue);
We used drm_modeset_lock_all() for protecting the audio component bind/unbind operations against the notification, but it seems leading to kernel WARNING and Oops. Moreover, the use of drm_modeset_lock_all() is rather an overkill only for this protection. This patch introduces a new mutex that protects just the bind/unbind and the notify calls and replaces the drm_modeset_lock with it. Fixes: 34d84636e5e0 ("drm/radeon: Add HD-audio component notifier support (v4)") Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1569 Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_audio.c | 21 ++++++++++++--------- drivers/gpu/drm/radeon/radeon_device.c | 1 + 3 files changed, 14 insertions(+), 9 deletions(-)