Re: [PATCH 1/6] drm/i915: Fail if we can't get a fence for gen2/3 tiled scanout

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 21, 2018 at 09:52:04PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-02-21 16:02:30)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Gen2/3 display engine depends on the fence for tiled scanout. So if we
> > fail to get a fence fail the entire operation.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 5d46771d58f6..66b269bc24b9 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2123,6 +2123,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> >                 goto err;
> >  
> >         if (i915_vma_is_map_and_fenceable(vma)) {
> > +               int ret;
> > +
> >                 /* 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
> > @@ -2139,7 +2141,13 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> >                  * something and try to run the system in a "less than optimal"
> >                  * mode that matches the user configuration.
> >                  */
> > -               if (i915_vma_pin_fence(vma) == 0 && vma->fence)
> > +               ret = i915_vma_pin_fence(vma);
> > +               if (ret != 0 && INTEL_GEN(dev_priv) < 4) {
> > +                       vma = ERR_PTR(ret);
> > +                       goto err;
> > +               }
> > +
> > +               if (ret == 0 && vma->fence)
> >                         *out_flags |= PLANE_HAS_FENCE;
> >         }
> 
> Ok, I'd like to see INTEL_GEN(dev_priv) < 4 be replaced with say
> needs_fence (and may be passed in from the caller like wants_fence?).

I had that earlier, but then I didn't have the uses_fence. Maybe I
cook up some kind of input flags thing here with PLANE_NEEDS_FENCE
and PLANE_WANTS_FENCE (maybe with a better naming scheme to
distinguish from the output flags, or should we just share the
same namespace?).

And should we then move the gmch check out and instead have something
like PLANE_NEEDS_MAPPABLE?

> Then I'm wondering if a 
> 	if (WARN_ON(needs_fence && !(*flags & PLANE_HAS_FENCE))
> makes sense.

Just to make sure i915_vma_pin_fence() did its job correctly?

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx