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>

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d4b6f83..c9e1575 100644 (file)
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -621,34 +621,27 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
        struct nouveau_drm *drm = nouveau_drm(drm_dev);
        struct drm_encoder *encoder;
        struct nouveau_encoder *nv_encoder;
-       struct drm_connector *connector;
        struct nouveau_crtc *nv_crtc;
-       struct drm_connector_list_iter conn_iter;
        int ret = 0;
 
        *enabled = false;
 
+       mutex_lock(&drm->audio.lock);
+
        drm_for_each_encoder(encoder, drm->dev) {
                struct nouveau_connector *nv_connector = NULL;
 
+               if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST)
+                       continue; /* TODO */
+
                nv_encoder = nouveau_encoder(encoder);
+               nv_connector = nouveau_connector(nv_encoder->audio.connector);
+               nv_crtc = nouveau_crtc(nv_encoder->crtc);
 
-               drm_connector_list_iter_begin(drm_dev, &conn_iter);
-               drm_for_each_connector_iter(connector, &conn_iter) {
-                       if (connector->state->best_encoder == encoder) {
-                               nv_connector = nouveau_connector(connector);
-                               break;
-                       }
-               }
-               drm_connector_list_iter_end(&conn_iter);
-               if (!nv_connector)
+               if (!nv_crtc || nv_encoder->or != port || nv_crtc->index != dev_id)
                        continue;
 
-               nv_crtc = nouveau_crtc(nv_encoder->crtc);
-               if (!nv_crtc || nv_encoder->or != port ||
-                   nv_crtc->index != dev_id)
-                       continue;
-               *enabled = nv_encoder->audio;
+               *enabled = nv_encoder->audio.enabled;
                if (*enabled) {
                        ret = drm_eld_size(nv_connector->base.eld);
                        memcpy(buf, nv_connector->base.eld,
@@ -657,6 +650,8 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
                break;
        }
 
+       mutex_unlock(&drm->audio.lock);
+
        return ret;
 }
 
@@ -706,17 +701,22 @@ static const struct component_ops nv50_audio_component_bind_ops = {
 static void
 nv50_audio_component_init(struct nouveau_drm *drm)
 {
-       if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
-               drm->audio.component_registered = true;
+       if (component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
+               return;
+
+       drm->audio.component_registered = true;
+       mutex_init(&drm->audio.lock);
 }
 
 static void
 nv50_audio_component_fini(struct nouveau_drm *drm)
 {
-       if (drm->audio.component_registered) {
-               component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
-               drm->audio.component_registered = false;
-       }
+       if (!drm->audio.component_registered)
+               return;
+
+       component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
+       drm->audio.component_registered = false;
+       mutex_destroy(&drm->audio.lock);
 }
 
 /******************************************************************************
@@ -739,11 +739,13 @@ nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
                                (0x0100 << nv_crtc->index),
        };
 
-       if (!nv_encoder->audio)
-               return;
-
-       nv_encoder->audio = false;
-       nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
+       mutex_lock(&drm->audio.lock);
+       if (nv_encoder->audio.enabled) {
+               nv_encoder->audio.enabled = false;
+               nv_encoder->audio.connector = NULL;
+               nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
+       }
+       mutex_unlock(&drm->audio.lock);
 
        nv50_audio_component_eld_notify(drm->audio.component, nv_encoder->or,
                                        nv_crtc->index);
@@ -774,11 +776,16 @@ nv50_audio_enable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc,
        if (!drm_detect_monitor_audio(nv_connector->edid))
                return;
 
+       mutex_lock(&drm->audio.lock);
+
        memcpy(args.data, nv_connector->base.eld, sizeof(args.data));
 
        nvif_mthd(&disp->disp->object, 0, &args,
                  sizeof(args.base) + drm_eld_size(args.data));
-       nv_encoder->audio = true;
+       nv_encoder->audio.enabled = true;
+       nv_encoder->audio.connector = &nv_connector->base;
+
+       mutex_unlock(&drm->audio.lock);
 
        nv50_audio_component_eld_notify(drm->audio.component, nv_encoder->or,
                                        nv_crtc->index);
@@ -1649,8 +1656,6 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
        struct drm_dp_aux *aux = &nv_connector->aux;
        u8 pwr;
 
-       nv_encoder->crtc = NULL;
-
        if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
                int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &pwr);
 
@@ -1665,6 +1670,7 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
        nv50_audio_disable(encoder, nv_crtc);
        nv50_hdmi_disable(&nv_encoder->base.base, nv_crtc);
        nv50_outp_release(nv_encoder);
+       nv_encoder->crtc = NULL;
 }
 
 static void

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index c802d3d..d28ee68 100644 (file)
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -221,6 +221,7 @@ struct nouveau_drm {
 
        struct {
                struct drm_audio_component *component;
+               struct mutex lock;
                bool component_registered;
        } audio;
 };

diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 0dea219..1ffcc0a 100644 (file)
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -53,7 +53,12 @@ struct nouveau_encoder {
         * actually programmed on the hw, not the proposed crtc */
        struct drm_crtc *crtc;
        u32 ctrl;
-       bool audio;
+
+       /* Protected by nouveau_drm.audio.lock */
+       struct {
+               bool enabled;
+               struct drm_connector *connector;
+       } audio;
 
        struct drm_display_mode mode;
        int last_dpms;