On 08/09/2025 14:15, Luiz Otavio Mello wrote:
Move legacy BKL struct_mutex from drm_device to drm_i915_private, which
is the last remaining user.

Signed-off-by: Luiz Otavio Mello <luiz.me...@estudante.ufscar.br>
Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com>
---
  drivers/gpu/drm/drm_drv.c                  |  2 --
  drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  4 ++--
  drivers/gpu/drm/i915/i915_driver.c         |  2 ++
  drivers/gpu/drm/i915/i915_drv.h            | 11 +++++++++++
  drivers/gpu/drm/i915/i915_irq.c            |  4 ++--
  include/drm/drm_device.h                   | 10 ----------
  6 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index cdd591b11488..ad3aee354ba3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -694,7 +694,6 @@ static void drm_dev_init_release(struct drm_device *dev, 
void *res)
        mutex_destroy(&dev->master_mutex);
        mutex_destroy(&dev->clientlist_mutex);
        mutex_destroy(&dev->filelist_mutex);
-       mutex_destroy(&dev->struct_mutex);
  }
static int drm_dev_init(struct drm_device *dev,
@@ -735,7 +734,6 @@ static int drm_dev_init(struct drm_device *dev,
        INIT_LIST_HEAD(&dev->vblank_event_list);
spin_lock_init(&dev->event_lock);
-       mutex_init(&dev->struct_mutex);
        mutex_init(&dev->filelist_mutex);
        mutex_init(&dev->clientlist_mutex);
        mutex_init(&dev->master_mutex);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index 09a64f224c49..65ffcaeee4b9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -677,7 +677,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 
level)
        if (level < GUC_LOG_LEVEL_DISABLED || level > GUC_LOG_LEVEL_MAX)
                return -EINVAL;
- mutex_lock(&i915->drm.struct_mutex);
+       mutex_lock(&i915->struct_mutex);
if (log->level == level)
                goto out_unlock;
@@ -695,7 +695,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 
level)
        log->level = level;
out_unlock:
-       mutex_unlock(&i915->drm.struct_mutex);
+       mutex_unlock(&i915->struct_mutex);
return ret;
  }
diff --git a/drivers/gpu/drm/i915/i915_driver.c 
b/drivers/gpu/drm/i915/i915_driver.c
index c6263c6d3384..d1559fd8ad3d 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -233,6 +233,7 @@ static int i915_driver_early_probe(struct drm_i915_private 
*dev_priv)
intel_sbi_init(display);
        vlv_iosf_sb_init(dev_priv);
+       mutex_init(&dev_priv->struct_mutex);
        mutex_init(&dev_priv->sb_lock);
i915_memcpy_init_early(dev_priv);
@@ -291,6 +292,7 @@ static void i915_driver_late_release(struct 
drm_i915_private *dev_priv)
        i915_workqueues_cleanup(dev_priv);
mutex_destroy(&dev_priv->sb_lock);
+       mutex_destroy(&dev_priv->struct_mutex);
        vlv_iosf_sb_fini(dev_priv);
        intel_sbi_fini(display);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4e4e89746aa6..15f66a7d496d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -222,6 +222,17 @@ struct drm_i915_private {
bool irqs_enabled; + /*
+        * Currently, struct_mutex is only used by the i915 driver as a 
replacement
+        * for BKL.

It's not a replacement for the BKL (i915 does not use the "good old" BKL since who knows when), but just a mutex (ab)used by the GuC logging and IVB parity interrupt work.

At least the comment should be improved, or even better, if not too much work, consider splitting into two separate mutexes. One could probably live in the i915->l3_parity and other in struct intel_guc_log.

[comes back later]

Never mind, I see that by the end of the series you get exactly there. I'd say series is a bit churny and typically you wouldn't be adding and changing so much just to remove it, but I can live with it in this instance.

Regards,

Tvrtko

+        *
+        * For this reason, it is no longer part of struct drm_device.
+        */
+       struct mutex struct_mutex;
+
+       /* LPT/WPT IOSF sideband protection */
+       struct mutex sbi_lock;
+
        /* VLV/CHV IOSF sideband */
        struct {
                struct mutex lock; /* protect sideband access */
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 191ed8bb1d9c..cdfb09464134 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -167,7 +167,7 @@ static void ivb_parity_work(struct work_struct *work)
         * In order to prevent a get/put style interface, acquire struct mutex
         * any time we access those registers.
         */
-       mutex_lock(&dev_priv->drm.struct_mutex);
+       mutex_lock(&dev_priv->struct_mutex);
/* If we've screwed up tracking, just let the interrupt fire again */
        if (drm_WARN_ON(&dev_priv->drm, !dev_priv->l3_parity.which_slice))
@@ -225,7 +225,7 @@ static void ivb_parity_work(struct work_struct *work)
        gen5_gt_enable_irq(gt, GT_PARITY_ERROR(dev_priv));
        spin_unlock_irq(gt->irq_lock);
- mutex_unlock(&dev_priv->drm.struct_mutex);
+       mutex_unlock(&dev_priv->struct_mutex);
  }
static irqreturn_t valleyview_irq_handler(int irq, void *arg)
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index a33aedd5e9ec..016df5529d46 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -188,16 +188,6 @@ struct drm_device {
        /** @unique: Unique name of the device */
        char *unique;
- /**
-        * @struct_mutex:
-        *
-        * Lock for others (not &drm_minor.master and &drm_file.is_master)
-        *
-        * TODO: This lock used to be the BKL of the DRM subsystem. Move the
-        *       lock into i915, which is the only remaining user.
-        */
-       struct mutex struct_mutex;
-
        /**
         * @master_mutex:
         *

Reply via email to