On Tue, Jul 12, 2016 at 03:30:25PM +0100, Tvrtko Ursulin wrote: > On 07/07/16 09:41, Chris Wilson wrote: > >@@ -3684,6 +3684,9 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > > old_read_domains, > > old_write_domain); > > > >+ /* Increment the pages_pin_count to guard against the shrinker */ > >+ obj->pages_pin_count++; > > Would it be clearer to look at obj->pin_display in the shrinker? > Although it looks like special casing out of the cleanliness of the > design in both case so I am not sure. Yeah. I liked the mechanism of telling the shrinker to avoid certain pages by only having to control the pages_pin_count. It feels easier to explain to others "the shrinker may reap any object that hasn't pinned its pages" (explicitly called i915_gem_object_pin_pages for its own use) rather than that + plus a list of exceptions known by the shrinker. > >@@ -82,7 +67,7 @@ static bool can_release_pages(struct drm_i915_gem_object *obj) > > * to the GPU, simply unbinding from the GPU is not going to succeed > > * in releasing our pin count on the pages themselves. > > */ > >- if (obj->pages_pin_count != num_vma_bound(obj)) > >+ if (obj->pages_pin_count != obj->bind_count) > > Would this be clearer as obj->pages_pin_count > obj->bind_count ? Ok. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx