Re: [PATCH v3] drm/i915: Simplify and fix object to display tracking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

On 03/31/2015 02:10 PM, Joonas Lahtinen wrote:
On ti, 2015-03-31 at 13:55 +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Purpose of this tracking is to know when to flush the cache between
the CPU and the non-coherent display engine. Prior to:

    commit 121920faf2ccce9aa66a7e2588415c9647b66104
    Author: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
    Date:   Mon Mar 23 11:10:37 2015 +0000

        drm/i915/skl: Query display address through a wrapper

This worked by a mix of direct flag manipulation and checking for
existence of a pinned GGTT VMA.

With the introduction of rotated display mappings this approach is
no longer correct.

New simpler approach is to just keep this count over calls which pin
and unpin objects to and from display, at the slight cost of extra
space in every bo.

(Inspired and extracted code from a larger rework by Chris Wilson.)

v2: Remove the limit since it is not well defined. (Chris Wilson, Ville Syrjälä)
v3: Commit message corrections. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_drv.h |  3 ++-
  drivers/gpu/drm/i915/i915_gem.c | 30 ++++++------------------------
  2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4ef320c..37abd58 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1969,7 +1969,6 @@ struct drm_i915_gem_object {
  	 */
  	unsigned int fault_mappable:1;
  	unsigned int pin_mappable:1;
-	unsigned int pin_display:1;

  	/*
  	 * Is the object to be mapped as read-only to the GPU
@@ -1983,6 +1982,8 @@ struct drm_i915_gem_object {

  	unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;

+	unsigned int pin_display;
+

Here. Comment below.

  	struct sg_table *pages;
  	int pages_pin_count;


How about naming it like the variable just below it (pages_pin_count).
So let it be "display_pin_count", and make it just "int" too, I don't
think the 1 bit reduction in pin count matters really, but having sign
makes it useful for detecting negative pin count errors? At least I'd
make it signed integer and make some of the spots WARN_ON(xxx--- < 0).

It can't go negative due check in i915_gem_object_unpin_from_display_plane.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux