drm/i915: Use atomics to manipulate obj->frontbuffer_bits

The individual bits inside obj->frontbuffer_bits are protected by each
plane->mutex, but the whole bitfield may be accessed by multiple KMS
operations simultaneously and so the RMW need to be under atomics.
However, for updating the single field we do not need to mandate that it
be under the struct_mutex, one more step towards its removal as the de
facto BKL.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470324762-2545-21-git-send-email-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8d47d1bf7b8533841ed8c776d22098dfd8faa042..9796b07bdb0d341434bdf1bce9b4dc468ad16f81 100644 (file)
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -138,6 +138,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
        struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
        struct intel_engine_cs *engine;
        struct i915_vma *vma;
+       unsigned int frontbuffer_bits;
        int pin_count = 0;
        enum intel_engine_id id;
 
@@ -204,8 +205,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
        if (engine)
                seq_printf(m, " (%s)", engine->name);
 
-       if (obj->frontbuffer_bits)
-               seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits);
+       frontbuffer_bits = atomic_read(&obj->frontbuffer_bits);
+       if (frontbuffer_bits)
+               seq_printf(m, " (frontbuffer: 0x%03x)", frontbuffer_bits);
 }
 
 static int i915_gem_object_list_info(struct seq_file *m, void *data)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b26f5b1f466bd8138f58585eb696d6c01fd95325..3de75e82ca76e479eac65ff51d937ea5428eba5b 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2127,8 +2127,6 @@ struct drm_i915_gem_object_ops {
  */
 #define INTEL_MAX_SPRITE_BITS_PER_PIPE 5
 #define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
-#define INTEL_FRONTBUFFER_BITS \
-       (INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES)
 #define INTEL_FRONTBUFFER_PRIMARY(pipe) \
        (1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
 #define INTEL_FRONTBUFFER_CURSOR(pipe) \
@@ -2216,7 +2214,7 @@ struct drm_i915_gem_object {
        unsigned int cache_level:3;
        unsigned int cache_dirty:1;
 
-       unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;
+       atomic_t frontbuffer_bits;
 
        unsigned int has_wc_mmap;
        /** Count of VMA actually bound by this object */

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 68110b9d98b7ec1a0282e454bd5ed8e0efaf23ea..03eb0946a878b4452ecd345d30b372a4900852bf 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4040,7 +4040,7 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
        if (obj->stolen)
                i915_gem_object_unpin_pages(obj);
 
-       WARN_ON(obj->frontbuffer_bits);
+       WARN_ON(atomic_read(&obj->frontbuffer_bits));
 
        if (obj->pages && obj->madv == I915_MADV_WILLNEED &&
            dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES &&
@@ -4557,16 +4557,23 @@ void i915_gem_track_fb(struct drm_i915_gem_object *old,
                       struct drm_i915_gem_object *new,
                       unsigned frontbuffer_bits)
 {
+       /* Control of individual bits within the mask are guarded by
+        * the owning plane->mutex, i.e. we can never see concurrent
+        * manipulation of individual bits. But since the bitfield as a whole
+        * is updated using RMW, we need to use atomics in order to update
+        * the bits.
+        */
+       BUILD_BUG_ON(INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES >
+                    sizeof(atomic_t) * BITS_PER_BYTE);
+
        if (old) {
-               WARN_ON(!mutex_is_locked(&old->base.dev->struct_mutex));
-               WARN_ON(!(old->frontbuffer_bits & frontbuffer_bits));
-               old->frontbuffer_bits &= ~frontbuffer_bits;
+               WARN_ON(!(atomic_read(&old->frontbuffer_bits) & frontbuffer_bits));
+               atomic_andnot(frontbuffer_bits, &old->frontbuffer_bits);
        }
 
        if (new) {
-               WARN_ON(!mutex_is_locked(&new->base.dev->struct_mutex));
-               WARN_ON(new->frontbuffer_bits & frontbuffer_bits);
-               new->frontbuffer_bits |= frontbuffer_bits;
+               WARN_ON(atomic_read(&new->frontbuffer_bits) & frontbuffer_bits);
+               atomic_or(frontbuffer_bits, &new->frontbuffer_bits);
        }
 }
 

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 229b1c471df3c17884cfb5b7f3b3994fc19716bb..5bc82064b2198fd3802439fab568a931028632e7 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2601,7 +2601,8 @@ valid_fb:
        primary->fb = primary->state->fb = fb;
        primary->crtc = primary->state->crtc = &intel_crtc->base;
        intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
-       obj->frontbuffer_bits |= to_intel_plane(primary)->frontbuffer_bit;
+       atomic_or(to_intel_plane(primary)->frontbuffer_bit,
+                 &obj->frontbuffer_bits);
 }
 
 static void i9xx_update_primary_plane(struct drm_plane *primary,
@@ -13810,19 +13811,12 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
 {
        struct drm_plane_state *old_plane_state;
        struct drm_plane *plane;
-       struct drm_i915_gem_object *obj, *old_obj;
-       struct intel_plane *intel_plane;
        int i;
 
-       mutex_lock(&state->dev->struct_mutex);
-       for_each_plane_in_state(state, plane, old_plane_state, i) {
-               obj = intel_fb_obj(plane->state->fb);
-               old_obj = intel_fb_obj(old_plane_state->fb);
-               intel_plane = to_intel_plane(plane);
-
-               i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
-       }
-       mutex_unlock(&state->dev->struct_mutex);
+       for_each_plane_in_state(state, plane, old_plane_state, i)
+               i915_gem_track_fb(intel_fb_obj(old_plane_state->fb),
+                                 intel_fb_obj(plane->state->fb),
+                                 to_intel_plane(plane)->frontbuffer_bit);
 }
 
 /**

diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index f15486aee7f819cb12711f6c1e2e9879dcffc06f..0e5da902473c80e77a5eef0d70762fbf65af84bd 100644 (file)
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -67,23 +67,22 @@
 #include "i915_drv.h"
 
 void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
-                              enum fb_op_origin origin)
+                              enum fb_op_origin origin,
+                              unsigned int frontbuffer_bits)
 {
        struct drm_device *dev = obj->base.dev;
        struct drm_i915_private *dev_priv = to_i915(dev);
 
-       WARN_ON(!mutex_is_locked(&dev->struct_mutex));
-
        if (origin == ORIGIN_CS) {
                spin_lock(&dev_priv->fb_tracking.lock);
-               dev_priv->fb_tracking.busy_bits |= obj->frontbuffer_bits;
-               dev_priv->fb_tracking.flip_bits &= ~obj->frontbuffer_bits;
+               dev_priv->fb_tracking.busy_bits |= frontbuffer_bits;
+               dev_priv->fb_tracking.flip_bits &= ~frontbuffer_bits;
                spin_unlock(&dev_priv->fb_tracking.lock);
        }
 
-       intel_psr_invalidate(dev, obj->frontbuffer_bits);
-       intel_edp_drrs_invalidate(dev, obj->frontbuffer_bits);
-       intel_fbc_invalidate(dev_priv, obj->frontbuffer_bits, origin);
+       intel_psr_invalidate(dev, frontbuffer_bits);
+       intel_edp_drrs_invalidate(dev, frontbuffer_bits);
+       intel_fbc_invalidate(dev_priv, frontbuffer_bits, origin);
 }
 
 /**
@@ -119,15 +118,11 @@ static void intel_frontbuffer_flush(struct drm_device *dev,
 
 void __intel_fb_obj_flush(struct drm_i915_gem_object *obj,
                          bool retire,
-                         enum fb_op_origin origin)
+                         enum fb_op_origin origin,
+                         unsigned int frontbuffer_bits)
 {
        struct drm_device *dev = obj->base.dev;
        struct drm_i915_private *dev_priv = to_i915(dev);
-       unsigned frontbuffer_bits;
-
-       WARN_ON(!mutex_is_locked(&dev->struct_mutex));
-
-       frontbuffer_bits = obj->frontbuffer_bits;
 
        if (retire) {
                spin_lock(&dev_priv->fb_tracking.lock);

diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.h b/drivers/gpu/drm/i915/intel_frontbuffer.h
index 60a0ec179108e94792bb56cae7353a037e8f5c58..0c85b205d902d46b324e5ba2fd5f3c69e476fcd5 100644 (file)
--- a/drivers/gpu/drm/i915/intel_frontbuffer.h
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.h
@@ -36,10 +36,12 @@ void intel_frontbuffer_flip(struct drm_device *dev,
                            unsigned frontbuffer_bits);
 
 void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
-                              enum fb_op_origin origin);
+                              enum fb_op_origin origin,
+                              unsigned int frontbuffer_bits);
 void __intel_fb_obj_flush(struct drm_i915_gem_object *obj,
                          bool retire,
-                         enum fb_op_origin origin);
+                         enum fb_op_origin origin,
+                         unsigned int frontbuffer_bits);
 
 /**
  * intel_fb_obj_invalidate - invalidate frontbuffer object
@@ -55,10 +57,13 @@ void __intel_fb_obj_flush(struct drm_i915_gem_object *obj,
 static inline void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
                                           enum fb_op_origin origin)
 {
-       if (!obj->frontbuffer_bits)
+       unsigned int frontbuffer_bits;
+
+       frontbuffer_bits = atomic_read(&obj->frontbuffer_bits);
+       if (!frontbuffer_bits)
                return;
 
-       __intel_fb_obj_invalidate(obj, origin);
+       __intel_fb_obj_invalidate(obj, origin, frontbuffer_bits);
 }
 
 /**
@@ -75,10 +80,13 @@ static inline void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
                                      bool retire,
                                      enum fb_op_origin origin)
 {
-       if (!obj->frontbuffer_bits)
+       unsigned int frontbuffer_bits;
+
+       frontbuffer_bits = atomic_read(&obj->frontbuffer_bits);
+       if (!frontbuffer_bits)
                return;
 
-       __intel_fb_obj_flush(obj, retire, origin);
+       __intel_fb_obj_flush(obj, retire, origin, frontbuffer_bits);
 }
 
 #endif /* __INTEL_FRONTBUFFER_H__ */