Skip to content

Commit 9125e24

Browse files
LyudeBen Skeggs
authored andcommitted
drm/nouveau/kms/nv50-: Fix locking for audio callbacks
Noticed that I wasn't paying close enough attention the last time I looked at our audio callbacks, as I completely missed the fact that we were figuring out which audio-enabled connector goes to each encoder by checking it's state, but without grabbing any of the appropriate modesetting locks to do so. That being said however: trying to grab modesetting locks in our audio callbacks would be very painful due to the potential for locking inversion between HDA and DRM. So, let's instead just copy what i915 does again - add our own audio lock to protect audio related state, and store each audio enabled connector in each nouveau_encoder struct so that we don't need to check any atomic states. Signed-off-by: Lyude Paul <lyude@redhat.com> Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
1 parent b2b4027 commit 9125e24

File tree

3 files changed

+43
-31
lines changed

3 files changed

+43
-31
lines changed

drivers/gpu/drm/nouveau/dispnv50/disp.c

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -621,34 +621,27 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
621621
struct nouveau_drm *drm = nouveau_drm(drm_dev);
622622
struct drm_encoder *encoder;
623623
struct nouveau_encoder *nv_encoder;
624-
struct drm_connector *connector;
625624
struct nouveau_crtc *nv_crtc;
626-
struct drm_connector_list_iter conn_iter;
627625
int ret = 0;
628626

629627
*enabled = false;
630628

629+
mutex_lock(&drm->audio.lock);
630+
631631
drm_for_each_encoder(encoder, drm->dev) {
632632
struct nouveau_connector *nv_connector = NULL;
633633

634+
if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST)
635+
continue; /* TODO */
636+
634637
nv_encoder = nouveau_encoder(encoder);
638+
nv_connector = nouveau_connector(nv_encoder->audio.connector);
639+
nv_crtc = nouveau_crtc(nv_encoder->crtc);
635640

636-
drm_connector_list_iter_begin(drm_dev, &conn_iter);
637-
drm_for_each_connector_iter(connector, &conn_iter) {
638-
if (connector->state->best_encoder == encoder) {
639-
nv_connector = nouveau_connector(connector);
640-
break;
641-
}
642-
}
643-
drm_connector_list_iter_end(&conn_iter);
644-
if (!nv_connector)
641+
if (!nv_crtc || nv_encoder->or != port || nv_crtc->index != dev_id)
645642
continue;
646643

647-
nv_crtc = nouveau_crtc(nv_encoder->crtc);
648-
if (!nv_crtc || nv_encoder->or != port ||
649-
nv_crtc->index != dev_id)
650-
continue;
651-
*enabled = nv_encoder->audio;
644+
*enabled = nv_encoder->audio.enabled;
652645
if (*enabled) {
653646
ret = drm_eld_size(nv_connector->base.eld);
654647
memcpy(buf, nv_connector->base.eld,
@@ -657,6 +650,8 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
657650
break;
658651
}
659652

653+
mutex_unlock(&drm->audio.lock);
654+
660655
return ret;
661656
}
662657

@@ -706,17 +701,22 @@ static const struct component_ops nv50_audio_component_bind_ops = {
706701
static void
707702
nv50_audio_component_init(struct nouveau_drm *drm)
708703
{
709-
if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
710-
drm->audio.component_registered = true;
704+
if (component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
705+
return;
706+
707+
drm->audio.component_registered = true;
708+
mutex_init(&drm->audio.lock);
711709
}
712710

713711
static void
714712
nv50_audio_component_fini(struct nouveau_drm *drm)
715713
{
716-
if (drm->audio.component_registered) {
717-
component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
718-
drm->audio.component_registered = false;
719-
}
714+
if (!drm->audio.component_registered)
715+
return;
716+
717+
component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
718+
drm->audio.component_registered = false;
719+
mutex_destroy(&drm->audio.lock);
720720
}
721721

722722
/******************************************************************************
@@ -739,11 +739,13 @@ nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
739739
(0x0100 << nv_crtc->index),
740740
};
741741

742-
if (!nv_encoder->audio)
743-
return;
744-
745-
nv_encoder->audio = false;
746-
nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
742+
mutex_lock(&drm->audio.lock);
743+
if (nv_encoder->audio.enabled) {
744+
nv_encoder->audio.enabled = false;
745+
nv_encoder->audio.connector = NULL;
746+
nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
747+
}
748+
mutex_unlock(&drm->audio.lock);
747749

748750
nv50_audio_component_eld_notify(drm->audio.component, nv_encoder->or,
749751
nv_crtc->index);
@@ -774,11 +776,16 @@ nv50_audio_enable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc,
774776
if (!drm_detect_monitor_audio(nv_connector->edid))
775777
return;
776778

779+
mutex_lock(&drm->audio.lock);
780+
777781
memcpy(args.data, nv_connector->base.eld, sizeof(args.data));
778782

779783
nvif_mthd(&disp->disp->object, 0, &args,
780784
sizeof(args.base) + drm_eld_size(args.data));
781-
nv_encoder->audio = true;
785+
nv_encoder->audio.enabled = true;
786+
nv_encoder->audio.connector = &nv_connector->base;
787+
788+
mutex_unlock(&drm->audio.lock);
782789

783790
nv50_audio_component_eld_notify(drm->audio.component, nv_encoder->or,
784791
nv_crtc->index);
@@ -1649,8 +1656,6 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
16491656
struct drm_dp_aux *aux = &nv_connector->aux;
16501657
u8 pwr;
16511658

1652-
nv_encoder->crtc = NULL;
1653-
16541659
if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
16551660
int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &pwr);
16561661

@@ -1665,6 +1670,7 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
16651670
nv50_audio_disable(encoder, nv_crtc);
16661671
nv50_hdmi_disable(&nv_encoder->base.base, nv_crtc);
16671672
nv50_outp_release(nv_encoder);
1673+
nv_encoder->crtc = NULL;
16681674
}
16691675

16701676
static void

drivers/gpu/drm/nouveau/nouveau_drv.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ struct nouveau_drm {
221221

222222
struct {
223223
struct drm_audio_component *component;
224+
struct mutex lock;
224225
bool component_registered;
225226
} audio;
226227
};

drivers/gpu/drm/nouveau/nouveau_encoder.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,12 @@ struct nouveau_encoder {
5353
* actually programmed on the hw, not the proposed crtc */
5454
struct drm_crtc *crtc;
5555
u32 ctrl;
56-
bool audio;
56+
57+
/* Protected by nouveau_drm.audio.lock */
58+
struct {
59+
bool enabled;
60+
struct drm_connector *connector;
61+
} audio;
5762

5863
struct drm_display_mode mode;
5964
int last_dpms;

0 commit comments

Comments
 (0)