On Tue, Mar 31, 2015 at 04:10:05PM +0300, 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).
We already have the counter underflow detection. We could just as well
make pages_pin_count unsigned. I blame my own laziness.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
