target/linux/bcm27xx/patches-5.15/950-0008-drm-vc4-hdmi-Use-a-mutex-to-prevent-concurrent-frame.patch

   1 From 5976ef1d81c8d474eddb55103f29a686f84f22f1 Mon Sep 17 00:00:00 2001
   2 From: Maxime Ripard <maxime@cerno.tech>
   3 Date: Mon, 25 Oct 2021 16:11:09 +0200
   4 Subject: [PATCH] drm/vc4: hdmi: Use a mutex to prevent concurrent
   5  framework access
   6
   7 The vc4 HDMI controller registers into the KMS, CEC and ALSA
   8 frameworks.
   9
  10 However, no particular care is done to prevent the concurrent execution
  11 of different framework hooks from happening at the same time.
  12
  13 In order to protect against that scenario, let's introduce a mutex that
  14 relevant ALSA and KMS hooks will need to take to prevent concurrent
  15 execution.
  16
  17 CEC is left out at the moment though, since the .get_modes and .detect
  18 KMS hooks, when running cec_s_phys_addr_from_edid, can end up calling
  19 CEC's .adap_enable hook. This introduces some reentrancy that isn't easy
  20 to deal with properly.
  21
  22 The CEC hooks also don't share much state with the rest of the driver:
  23 the registers are entirely separate, we don't share any variable, the
  24 only thing that can conflict is the CEC clock divider setup that can be
  25 affected by a mode set.
  26
  27 However, after discussing it, it looks like CEC should be able to
  28 recover from this if it was to happen.
  29
  30 Link: https://lore.kernel.org/r/20211025141113.702757-6-maxime@cerno.tech
  31 Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
  32 Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
  33 Signed-off-by: Maxime Ripard <maxime@cerno.tech>
  34 ---
  35  drivers/gpu/drm/vc4/vc4_hdmi.c | 118 +++++++++++++++++++++++++++++++--
  36  drivers/gpu/drm/vc4/vc4_hdmi.h |  14 ++++
  37  2 files changed, 126 insertions(+), 6 deletions(-)
  38
  39 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
  40 +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
  41 @@ -192,6 +192,8 @@ vc4_hdmi_connector_detect(struct drm_con
  42         struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
  43         bool connected = false;
  44
  45 +       mutex_lock(&vc4_hdmi->mutex);
  46 +
  47         WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
  48
  49         if (vc4_hdmi->hpd_gpio) {
  50 @@ -222,11 +224,13 @@ vc4_hdmi_connector_detect(struct drm_con
  51
  52                 vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
  53                 pm_runtime_put(&vc4_hdmi->pdev->dev);
  54 +               mutex_unlock(&vc4_hdmi->mutex);
  55                 return connector_status_connected;
  56         }
  57
  58         cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
  59         pm_runtime_put(&vc4_hdmi->pdev->dev);
  60 +       mutex_unlock(&vc4_hdmi->mutex);
  61         return connector_status_disconnected;
  62  }
  63
  64 @@ -243,10 +247,14 @@ static int vc4_hdmi_connector_get_modes(
  65         int ret = 0;
  66         struct edid *edid;
  67
  68 +       mutex_lock(&vc4_hdmi->mutex);
  69 +
  70         edid = drm_get_edid(connector, vc4_hdmi->ddc);
  71         cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
  72 -       if (!edid)
  73 -               return -ENODEV;
  74 +       if (!edid) {
  75 +               ret = -ENODEV;
  76 +               goto out;
  77 +       }
  78
  79         vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
  80
  81 @@ -266,6 +274,9 @@ static int vc4_hdmi_connector_get_modes(
  82                 }
  83         }
  84
  85 +out:
  86 +       mutex_unlock(&vc4_hdmi->mutex);
  87 +
  88         return ret;
  89  }
  90
  91 @@ -482,6 +493,8 @@ static void vc4_hdmi_set_avi_infoframe(s
  92         union hdmi_infoframe frame;
  93         int ret;
  94
  95 +       lockdep_assert_held(&vc4_hdmi->mutex);
  96 +
  97         ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
  98                                                        connector, mode);
  99         if (ret < 0) {
 100 @@ -533,6 +546,8 @@ static void vc4_hdmi_set_hdr_infoframe(s
 101         struct drm_connector_state *conn_state = connector->state;
 102         union hdmi_infoframe frame;
 103
 104 +       lockdep_assert_held(&vc4_hdmi->mutex);
 105 +
 106         if (!vc4_hdmi->variant->supports_hdr)
 107                 return;
 108
 109 @@ -549,6 +564,8 @@ static void vc4_hdmi_set_infoframes(stru
 110  {
 111         struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 112
 113 +       lockdep_assert_held(&vc4_hdmi->mutex);
 114 +
 115         vc4_hdmi_set_avi_infoframe(encoder);
 116         vc4_hdmi_set_spd_infoframe(encoder);
 117         /*
 118 @@ -568,6 +585,8 @@ static bool vc4_hdmi_supports_scrambling
 119         struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 120         struct drm_display_info *display = &vc4_hdmi->connector.display_info;
 121
 122 +       lockdep_assert_held(&vc4_hdmi->mutex);
 123 +
 124         if (!vc4_encoder->hdmi_monitor)
 125                 return false;
 126
 127 @@ -586,6 +605,8 @@ static void vc4_hdmi_enable_scrambling(s
 128         struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 129         unsigned long flags;
 130
 131 +       lockdep_assert_held(&vc4_hdmi->mutex);
 132 +
 133         if (!vc4_hdmi_supports_scrambling(encoder, mode))
 134                 return;
 135
 136 @@ -655,6 +676,8 @@ static void vc4_hdmi_encoder_post_crtc_d
 137         struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 138         unsigned long flags;
 139
 140 +       mutex_lock(&vc4_hdmi->mutex);
 141 +
 142         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 143
 144         HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, 0);
 145 @@ -671,6 +694,8 @@ static void vc4_hdmi_encoder_post_crtc_d
 146         spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
 147
 148         vc4_hdmi_disable_scrambling(encoder);
 149 +
 150 +       mutex_unlock(&vc4_hdmi->mutex);
 151  }
 152
 153  static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder,
 154 @@ -680,6 +705,8 @@ static void vc4_hdmi_encoder_post_crtc_p
 155         unsigned long flags;
 156         int ret;
 157
 158 +       mutex_lock(&vc4_hdmi->mutex);
 159 +
 160         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 161         HDMI_WRITE(HDMI_VID_CTL,
 162                    HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX);
 163 @@ -694,6 +721,8 @@ static void vc4_hdmi_encoder_post_crtc_p
 164         ret = pm_runtime_put(&vc4_hdmi->pdev->dev);
 165         if (ret < 0)
 166                 DRM_ERROR("Failed to release power domain: %d\n", ret);
 167 +
 168 +       mutex_unlock(&vc4_hdmi->mutex);
 169  }
 170
 171  static void vc4_hdmi_encoder_disable(struct drm_encoder *encoder)
 172 @@ -996,6 +1025,8 @@ static void vc4_hdmi_encoder_pre_crtc_co
 173         unsigned long flags;
 174         int ret;
 175
 176 +       mutex_lock(&vc4_hdmi->mutex);
 177 +
 178         /*
 179          * As stated in RPi's vc4 firmware "HDMI state machine (HSM) clock must
 180          * be faster than pixel clock, infinitesimally faster, tested in
 181 @@ -1016,13 +1047,13 @@ static void vc4_hdmi_encoder_pre_crtc_co
 182         ret = clk_set_min_rate(vc4_hdmi->hsm_clock, hsm_rate);
 183         if (ret) {
 184                 DRM_ERROR("Failed to set HSM clock rate: %d\n", ret);
 185 -               return;
 186 +               goto out;
 187         }
 188
 189         ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
 190         if (ret < 0) {
 191                 DRM_ERROR("Failed to retain power domain: %d\n", ret);
 192 -               return;
 193 +               goto out;
 194         }
 195
 196         ret = clk_set_rate(vc4_hdmi->pixel_clock, pixel_rate);
 197 @@ -1074,13 +1105,16 @@ static void vc4_hdmi_encoder_pre_crtc_co
 198         if (vc4_hdmi->variant->set_timings)
 199                 vc4_hdmi->variant->set_timings(vc4_hdmi, conn_state, mode);
 200
 201 +       mutex_unlock(&vc4_hdmi->mutex);
 202 +
 203         return;
 204
 205  err_disable_pixel_clock:
 206         clk_disable_unprepare(vc4_hdmi->pixel_clock);
 207  err_put_runtime_pm:
 208         pm_runtime_put(&vc4_hdmi->pdev->dev);
 209 -
 210 +out:
 211 +       mutex_unlock(&vc4_hdmi->mutex);
 212         return;
 213  }
 214
 215 @@ -1092,6 +1126,8 @@ static void vc4_hdmi_encoder_pre_crtc_en
 216         struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 217         unsigned long flags;
 218
 219 +       mutex_lock(&vc4_hdmi->mutex);
 220 +
 221         if (vc4_encoder->hdmi_monitor &&
 222             drm_default_rgb_quant_range(mode) == HDMI_QUANTIZATION_RANGE_LIMITED) {
 223                 if (vc4_hdmi->variant->csc_setup)
 224 @@ -1108,6 +1144,8 @@ static void vc4_hdmi_encoder_pre_crtc_en
 225         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 226         HDMI_WRITE(HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N);
 227         spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
 228 +
 229 +       mutex_unlock(&vc4_hdmi->mutex);
 230  }
 231
 232  static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
 233 @@ -1121,6 +1159,8 @@ static void vc4_hdmi_encoder_post_crtc_e
 234         unsigned long flags;
 235         int ret;
 236
 237 +       mutex_lock(&vc4_hdmi->mutex);
 238 +
 239         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 240
 241         HDMI_WRITE(HDMI_VID_CTL,
 242 @@ -1180,6 +1220,8 @@ static void vc4_hdmi_encoder_post_crtc_e
 243
 244         vc4_hdmi_recenter_fifo(vc4_hdmi);
 245         vc4_hdmi_enable_scrambling(encoder);
 246 +
 247 +       mutex_unlock(&vc4_hdmi->mutex);
 248  }
 249
 250  static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
 251 @@ -1323,6 +1365,7 @@ static void vc4_hdmi_set_n_cts(struct vc
 252         u32 n, cts;
 253         u64 tmp;
 254
 255 +       lockdep_assert_held(&vc4_hdmi->mutex);
 256         lockdep_assert_held(&vc4_hdmi->hw_lock);
 257
 258         n = 128 * samplerate / 1000;
 259 @@ -1356,13 +1399,17 @@ static int vc4_hdmi_audio_startup(struct
 260         struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 261         unsigned long flags;
 262
 263 +       mutex_lock(&vc4_hdmi->mutex);
 264 +
 265         /*
 266          * If the HDMI encoder hasn't probed, or the encoder is
 267          * currently in DVI mode, treat the codec dai as missing.
 268          */
 269         if (!encoder->crtc || !(HDMI_READ(HDMI_RAM_PACKET_CONFIG) &
 270 -                               VC4_HDMI_RAM_PACKET_ENABLE))
 271 +                               VC4_HDMI_RAM_PACKET_ENABLE)) {
 272 +               mutex_unlock(&vc4_hdmi->mutex);
 273                 return -ENODEV;
 274 +       }
 275
 276         vc4_hdmi->audio.streaming = true;
 277
 278 @@ -1378,6 +1425,8 @@ static int vc4_hdmi_audio_startup(struct
 279         if (vc4_hdmi->variant->phy_rng_enable)
 280                 vc4_hdmi->variant->phy_rng_enable(vc4_hdmi);
 281
 282 +       mutex_unlock(&vc4_hdmi->mutex);
 283 +
 284         return 0;
 285  }
 286
 287 @@ -1388,6 +1437,8 @@ static void vc4_hdmi_audio_reset(struct
 288         unsigned long flags;
 289         int ret;
 290
 291 +       lockdep_assert_held(&vc4_hdmi->mutex);
 292 +
 293         vc4_hdmi->audio.streaming = false;
 294         ret = vc4_hdmi_stop_packet(encoder, HDMI_INFOFRAME_TYPE_AUDIO, false);
 295         if (ret)
 296 @@ -1407,6 +1458,8 @@ static void vc4_hdmi_audio_shutdown(stru
 297         struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
 298         unsigned long flags;
 299
 300 +       mutex_lock(&vc4_hdmi->mutex);
 301 +
 302         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 303
 304         HDMI_WRITE(HDMI_MAI_CTL,
 305 @@ -1421,6 +1474,8 @@ static void vc4_hdmi_audio_shutdown(stru
 306
 307         vc4_hdmi->audio.streaming = false;
 308         vc4_hdmi_audio_reset(vc4_hdmi);
 309 +
 310 +       mutex_unlock(&vc4_hdmi->mutex);
 311  }
 312
 313  static int sample_rate_to_mai_fmt(int samplerate)
 314 @@ -1479,6 +1534,8 @@ static int vc4_hdmi_audio_prepare(struct
 315         dev_dbg(dev, "%s: %u Hz, %d bit, %d channels\n", __func__,
 316                 sample_rate, params->sample_width, channels);
 317
 318 +       mutex_lock(&vc4_hdmi->mutex);
 319 +
 320         vc4_hdmi_audio_set_mai_clock(vc4_hdmi, sample_rate);
 321
 322         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 323 @@ -1533,6 +1590,8 @@ static int vc4_hdmi_audio_prepare(struct
 324         memcpy(&vc4_hdmi->audio.infoframe, &params->cea, sizeof(params->cea));
 325         vc4_hdmi_set_audio_infoframe(encoder);
 326
 327 +       mutex_unlock(&vc4_hdmi->mutex);
 328 +
 329         return 0;
 330  }
 331
 332 @@ -1575,7 +1634,9 @@ static int vc4_hdmi_audio_get_eld(struct
 333         struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
 334         struct drm_connector *connector = &vc4_hdmi->connector;
 335
 336 +       mutex_lock(&vc4_hdmi->mutex);
 337         memcpy(buf, connector->eld, min(sizeof(connector->eld), len));
 338 +       mutex_unlock(&vc4_hdmi->mutex);
 339
 340         return 0;
 341  }
 342 @@ -1912,6 +1973,17 @@ static int vc4_hdmi_cec_enable(struct ce
 343         u32 val;
 344         int ret;
 345
 346 +       /*
 347 +        * NOTE: This function should really take vc4_hdmi->mutex, but doing so
 348 +        * results in a reentrancy since cec_s_phys_addr_from_edid() called in
 349 +        * .detect or .get_modes might call .adap_enable, which leads to this
 350 +        * function being called with that mutex held.
 351 +        *
 352 +        * Concurrency is not an issue for the moment since we don't share any
 353 +        * state with KMS, so we can ignore the lock for now, but we need to
 354 +        * keep it in mind if we were to change that assumption.
 355 +        */
 356 +
 357         ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
 358         if (ret)
 359                 return ret;
 360 @@ -1958,6 +2030,17 @@ static int vc4_hdmi_cec_disable(struct c
 361         struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
 362         unsigned long flags;
 363
 364 +       /*
 365 +        * NOTE: This function should really take vc4_hdmi->mutex, but doing so
 366 +        * results in a reentrancy since cec_s_phys_addr_from_edid() called in
 367 +        * .detect or .get_modes might call .adap_enable, which leads to this
 368 +        * function being called with that mutex held.
 369 +        *
 370 +        * Concurrency is not an issue for the moment since we don't share any
 371 +        * state with KMS, so we can ignore the lock for now, but we need to
 372 +        * keep it in mind if we were to change that assumption.
 373 +        */
 374 +
 375         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 376
 377         if (!vc4_hdmi->variant->external_irq_controller)
 378 @@ -1986,6 +2069,17 @@ static int vc4_hdmi_cec_adap_log_addr(st
 379         struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
 380         unsigned long flags;
 381
 382 +       /*
 383 +        * NOTE: This function should really take vc4_hdmi->mutex, but doing so
 384 +        * results in a reentrancy since cec_s_phys_addr_from_edid() called in
 385 +        * .detect or .get_modes might call .adap_enable, which leads to this
 386 +        * function being called with that mutex held.
 387 +        *
 388 +        * Concurrency is not an issue for the moment since we don't share any
 389 +        * state with KMS, so we can ignore the lock for now, but we need to
 390 +        * keep it in mind if we were to change that assumption.
 391 +        */
 392 +
 393         spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
 394         HDMI_WRITE(HDMI_CEC_CNTRL_1,
 395                    (HDMI_READ(HDMI_CEC_CNTRL_1) & ~VC4_HDMI_CEC_ADDR_MASK) |
 396 @@ -2004,6 +2098,17 @@ static int vc4_hdmi_cec_adap_transmit(st
 397         u32 val;
 398         unsigned int i;
 399
 400 +       /*
 401 +        * NOTE: This function should really take vc4_hdmi->mutex, but doing so
 402 +        * results in a reentrancy since cec_s_phys_addr_from_edid() called in
 403 +        * .detect or .get_modes might call .adap_enable, which leads to this
 404 +        * function being called with that mutex held.
 405 +        *
 406 +        * Concurrency is not an issue for the moment since we don't share any
 407 +        * state with KMS, so we can ignore the lock for now, but we need to
 408 +        * keep it in mind if we were to change that assumption.
 409 +        */
 410 +
 411         if (msg->len > 16) {
 412                 drm_err(dev, "Attempting to transmit too much data (%d)\n", msg->len);
 413                 return -ENOMEM;
 414 @@ -2360,6 +2465,7 @@ static int vc4_hdmi_bind(struct device *
 415         vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
 416         if (!vc4_hdmi)
 417                 return -ENOMEM;
 418 +       mutex_init(&vc4_hdmi->mutex);
 419         spin_lock_init(&vc4_hdmi->hw_lock);
 420         INIT_DELAYED_WORK(&vc4_hdmi->scrambling_work, vc4_hdmi_scrambling_wq);
 421
 422 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
 423 +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
 424 @@ -184,6 +184,20 @@ struct vc4_hdmi {
 425          * @hw_lock: Spinlock protecting device register access.
 426          */
 427         spinlock_t hw_lock;
 428 +
 429 +       /**
 430 +        * @mutex: Mutex protecting the driver access across multiple
 431 +        * frameworks (KMS, ALSA).
 432 +        *
 433 +        * NOTE: While supported, CEC has been left out since
 434 +        * cec_s_phys_addr_from_edid() might call .adap_enable and lead to a
 435 +        * reentrancy issue between .get_modes (or .detect) and .adap_enable.
 436 +        * Since we don't share any state between the CEC hooks and KMS', it's
 437 +        * not a big deal. The only trouble might come from updating the CEC
 438 +        * clock divider which might be affected by a modeset, but CEC should
 439 +        * be resilient to that.
 440 +        */
 441 +       struct mutex mutex;
 442  };
 443
 444  static inline struct vc4_hdmi *