]> asedeno.scripts.mit.edu Git - linux.git/commitdiff
drm/i915: Replace obj->pin_global with obj->frontbuffer
authorChris Wilson <chris@chris-wilson.co.uk>
Mon, 2 Sep 2019 04:02:47 +0000 (05:02 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Tue, 3 Sep 2019 04:39:37 +0000 (05:39 +0100)
obj->pin_global was originally used as a means to keep the shrinker off
the active scanout, but we use the vma->pin_count itself for that and
the obj->frontbuffer to delay shrinking active framebuffers. The other
role that obj->pin_global gained was for spotting display objects inside
GEM and working harder to keep those coherent; for which we can again
simply inspect obj->frontbuffer directly.

Coming up next, we will want to manipulate the pin_global counter
outside of the principle locks, so would need to make pin_global atomic.
However, since obj->frontbuffer is already managed atomically, it makes
sense to use that the primary key for display objects instead of having
pin_global.

Ville pointed out the principle difference is that obj->frontbuffer is
set for as long as an intel_framebuffer is attached to an object, but
obj->pin_global was only raised for as long as the object was active. In
practice, this means that we consider the object as being on the scanout
for longer than is strictly required, causing us to be more proactive in
flushing -- though it should be true that we would have flushed
eventually when the back became the front, except that on the flip path
that flush is async but when hit from another ioctl it will be
synchronous.

v2: i915_gem_object_is_framebuffer()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190902040303.14195-5-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/display/intel_display.c
drivers/gpu/drm/i915/display/intel_frontbuffer.c
drivers/gpu/drm/i915/gem/i915_gem_domain.c
drivers/gpu/drm/i915/gem/i915_gem_object.h
drivers/gpu/drm/i915/gem/i915_gem_object_types.h
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
drivers/gpu/drm/i915/i915_debugfs.c

index a03dc6eb61f841b882018afb29513fb398accea8..12a94c6e3d5173d892eefa33ba87a8bd12862035 100644 (file)
@@ -2080,6 +2080,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
        u32 alignment;
 
        WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+       if (WARN_ON(!i915_gem_object_is_framebuffer(obj)))
+               return ERR_PTR(-EINVAL);
 
        alignment = intel_surf_alignment(fb, 0);
 
index 719379774fa542e0b1426e169b581c1fc85f1552..fc40dc1fdbcc475badeae7800da860f4768d1a38 100644 (file)
@@ -220,11 +220,18 @@ static void frontbuffer_release(struct kref *ref)
 {
        struct intel_frontbuffer *front =
                container_of(ref, typeof(*front), ref);
+       struct drm_i915_gem_object *obj = front->obj;
+       struct i915_vma *vma;
 
-       front->obj->frontbuffer = NULL;
-       spin_unlock(&to_i915(front->obj->base.dev)->fb_tracking.lock);
+       spin_lock(&obj->vma.lock);
+       for_each_ggtt_vma(vma, obj)
+               vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
+       spin_unlock(&obj->vma.lock);
 
-       i915_gem_object_put(front->obj);
+       obj->frontbuffer = NULL;
+       spin_unlock(&to_i915(obj->base.dev)->fb_tracking.lock);
+
+       i915_gem_object_put(obj);
        kfree(front);
 }
 
index 9c58e8fac1d977c02dca65df0a4cb7b9ab715158..6af740a5e3db100be95dc8cf46f77ba46b4a6249 100644 (file)
@@ -27,7 +27,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
 
 void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj)
 {
-       if (!READ_ONCE(obj->pin_global))
+       if (!i915_gem_object_is_framebuffer(obj))
                return;
 
        i915_gem_object_lock(obj);
@@ -422,12 +422,8 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
        assert_object_held(obj);
 
-       /* Mark the global pin early so that we account for the
-        * display coherency whilst setting up the cache domains.
-        */
-       obj->pin_global++;
-
-       /* The display engine is not coherent with the LLC cache on gen6.  As
+       /*
+        * The display engine is not coherent with the LLC cache on gen6.  As
         * a result, we make sure that the pinning that is about to occur is
         * done with uncached PTEs. This is lowest common denominator for all
         * chipsets.
@@ -439,12 +435,11 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
        ret = i915_gem_object_set_cache_level(obj,
                                              HAS_WT(to_i915(obj->base.dev)) ?
                                              I915_CACHE_WT : I915_CACHE_NONE);
-       if (ret) {
-               vma = ERR_PTR(ret);
-               goto err_unpin_global;
-       }
+       if (ret)
+               return ERR_PTR(ret);
 
-       /* As the user may map the buffer once pinned in the display plane
+       /*
+        * As the user may map the buffer once pinned in the display plane
         * (e.g. libkms for the bootup splash), we have to ensure that we
         * always use map_and_fenceable for all scanout buffers. However,
         * it may simply be too big to fit into mappable, in which case
@@ -461,22 +456,19 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
        if (IS_ERR(vma))
                vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
        if (IS_ERR(vma))
-               goto err_unpin_global;
+               return vma;
 
        vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
 
        __i915_gem_object_flush_for_display(obj);
 
-       /* It should now be out of any other write domains, and we can update
+       /*
+        * It should now be out of any other write domains, and we can update
         * the domain values for our changes.
         */
        obj->read_domains |= I915_GEM_DOMAIN_GTT;
 
        return vma;
-
-err_unpin_global:
-       obj->pin_global--;
-       return vma;
 }
 
 static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
@@ -514,12 +506,6 @@ i915_gem_object_unpin_from_display_plane(struct i915_vma *vma)
 
        assert_object_held(obj);
 
-       if (WARN_ON(obj->pin_global == 0))
-               return;
-
-       if (--obj->pin_global == 0)
-               vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
-
        /* Bump the LRU to try and avoid premature eviction whilst flipping  */
        i915_gem_object_bump_inactive_ggtt(obj);
 
index 5efb9936e05b512fe9d7dbcd47cf8069ac844e2e..29b9eddc4c7f5aff670c5fa1934c1d4130643a88 100644 (file)
@@ -406,7 +406,8 @@ static inline bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
        if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE))
                return true;
 
-       return obj->pin_global; /* currently in use by HW, keep flushed */
+       /* Currently in use by HW (display engine)? Keep flushed. */
+       return i915_gem_object_is_framebuffer(obj);
 }
 
 static inline void __start_cpu_write(struct drm_i915_gem_object *obj)
index ede0eb4218a81b9f1c67390afe7b2433afdf7a49..13b9dc0e1a899f059401a7b52224cc9e362f0c1e 100644 (file)
@@ -152,8 +152,6 @@ struct drm_i915_gem_object {
 
        /** Count of VMA actually bound by this object */
        atomic_t bind_count;
-       /** Count of how many global VMA are currently pinned for use by HW */
-       unsigned int pin_global;
 
        struct {
                struct mutex lock; /* protects the pages and their use */
index edd21d14e64ffa729b3e6fc1189db3e9e5c419dd..4e55cfc2b0dc6cbd9bee17fea3e730ba4b4b8f43 100644 (file)
@@ -61,7 +61,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
        if (!i915_gem_object_is_shrinkable(obj))
                return false;
 
-       /* Only report true if by unbinding the object and putting its pages
+       /*
+        * Only report true if by unbinding the object and putting its pages
         * we can actually make forward progress towards freeing physical
         * pages.
         *
@@ -72,16 +73,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
        if (atomic_read(&obj->mm.pages_pin_count) > atomic_read(&obj->bind_count))
                return false;
 
-       /* If any vma are "permanently" pinned, it will prevent us from
-        * reclaiming the obj->mm.pages. We only allow scanout objects to claim
-        * a permanent pin, along with a few others like the context objects.
-        * To simplify the scan, and to avoid walking the list of vma under the
-        * object, we just check the count of its permanently pinned.
-        */
-       if (READ_ONCE(obj->pin_global))
-               return false;
-
-       /* We can only return physical pages to the system if we can either
+       /*
+        * We can only return physical pages to the system if we can either
         * discard the contents (because the user has marked them as being
         * purgeable) or if we can move their contents out to swap.
         */
index 97493691732965470442c3f30a594d2717497113..9798f27a697ac6707095356333fd2640cb1a09a4 100644 (file)
@@ -77,11 +77,6 @@ static int i915_capabilities(struct seq_file *m, void *data)
        return 0;
 }
 
-static char get_pin_flag(struct drm_i915_gem_object *obj)
-{
-       return obj->pin_global ? 'p' : ' ';
-}
-
 static char get_tiling_flag(struct drm_i915_gem_object *obj)
 {
        switch (i915_gem_object_get_tiling(obj)) {
@@ -140,9 +135,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
        struct i915_vma *vma;
        int pin_count = 0;
 
-       seq_printf(m, "%pK: %c%c%c%c %8zdKiB %02x %02x %s%s%s",
+       seq_printf(m, "%pK: %c%c%c %8zdKiB %02x %02x %s%s%s",
                   &obj->base,
-                  get_pin_flag(obj),
                   get_tiling_flag(obj),
                   get_global_flag(obj),
                   get_pin_mapped_flag(obj),
@@ -221,8 +215,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
        seq_printf(m, " (pinned x %d)", pin_count);
        if (obj->stolen)
                seq_printf(m, " (stolen: %08llx)", obj->stolen->start);
-       if (obj->pin_global)
-               seq_printf(m, " (global)");
+       if (i915_gem_object_is_framebuffer(obj))
+               seq_printf(m, " (fb)");
 
        engine = i915_gem_object_last_write_engine(obj);
        if (engine)