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 <[email protected]>
---
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
On Mon, Oct 31, 2022 at 2:49 AM Takashi Iwai <[email protected]> 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 <[email protected]>
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
>