On Fri, Mar 20, 2015 at 11:29:25AM +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 04:50:22PM +0000, Chris Wilson wrote:
> > On Thu, Mar 19, 2015 at 05:35:17PM +0100, Daniel Vetter wrote:
> > > On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > > > The existing ABI says that scanouts are pinned into the mappable region
> > > > so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> > > > into the scanout through a GTT mapping. However if the surface does not
> > > > fit into the mappable region, we are better off just trying to fit it
> > > > anywhere and hoping for the best. (Any userspace that is cappable of
> > > > using ginormous scanouts is also likely not to rely on pure GTT
> > > > updates.) In the future, there may even be a kernel mediated method for
> > > > the legacy clients.
> > > >
> > > > v2: Skip fence pinning when not mappable.
> > > >
> > > > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > > > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanan...@intel.com>
> > > > Cc: Deepak S <deepa...@linux.intel.com>
> > > > Cc: Damien Lespiau <damien.lesp...@intel.com>
> > > > Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
> > > > drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
> > > > 2 files changed, 19 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > index 9e498e0bbf22..9a1de848e450 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct
> > > > drm_i915_gem_object *obj,
> > > >
> > > > /* 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.
> > > > + * always use map_and_fenceable for all scanout buffers.
> > > > However,
> > > > + * it may simply be too big to fit into mappable, in which case
> > > > + * put it anyway and hope that userspace can cope (but always
> > > > first
> > > > + * try to preserve the existing ABI).
> > > > */
> > > > ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
> > > > if (ret)
> > > > + ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> > > > + if (ret)
> > > > goto err_unpin_display;
> > > >
> > > > i915_gem_object_flush_cpu_write_domain(obj);
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index d621ebecd33e..628aace63b43 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane
> > > > *plane,
> > > > if (ret)
> > > > goto err_interruptible;
> > > >
> > > > - /* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > > - * fence, whereas 965+ only requires a fence if using
> > > > - * framebuffer compression. For simplicity, we always install
> > > > - * a fence as the cost is not that onerous.
> > > > - */
> > > > - ret = i915_gem_object_get_fence(obj);
> > > > - if (ret)
> > > > - goto err_unpin;
> > > > + if (obj->map_and_fenceable) {
> > > > + /* Install a fence for tiled scan-out. Pre-i965 always
> > > > needs a
> > > > + * fence, whereas 965+ only requires a fence if using
> > > > + * framebuffer compression. For simplicity, we always,
> > > > when
> > > > + * possible, install a fence as the cost is not that
> > > > onerous.
> > > > + */
> > > > + ret = i915_gem_object_get_fence(obj);
> > > > + if (ret)
> > > > + goto err_unpin;
> > >
> > > FBC still assumes that a fence is there (and with Paulo's recent rework
> > > that's made even more explicit). I think we need a change in the fbc
> > > frontbuffer tracking integration to not filter out GTT invalidates if the
> > > buffer isn't mappable. Paulo?
> >
> > I checked that we have such a check in the fbc enable code. I think if
> > we have a framebuffer that won't fit in the GTT, we are reasonably sure
> > it won't be FBC compatible. On the other hand, if we have 4
> > framebuffers...
> >
> > if (obj->tiling_mode != I915_TILING_X ||
> > obj->fence_reg == I915_FENCE_REG_NONE) {
> > if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
> > DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling
> > compression\n");
> >
> > I think it is preferrable that the system continues to run in a degraded
> > mode in such circumstances than fail entirely.
>
> Oh right I've forgotten that fbc hw only works with X tiled and that we
> use the fence_reg as a proxy. Adding a comment would be useful though.
SKL supports FBC with Y tiled as well.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
