Re: [PATCH 45/55] drm/i915: Convert intel_overlay to request tracking

Chris Wilson
2016-07-27T08:22:39Z

On Wed, Jul 27, 2016 at 11:12:32AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> >
> > -static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
> > +static void intel_overlay_submit_request(struct intel_overlay *overlay,
> >    struct drm_i915_gem_request *req,
> > -  void (*tail)(struct intel_overlay *))
> > +  void (*retire)(struct i915_gem_active *,
> > + struct drm_i915_gem_request *))
>
> Ugh, not such a beauty. We do not do callback typedefs?

Not often that we actually pass around function pointers.

> > -static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
> > +static void intel_overlay_release_old_vid_tail(struct i915_gem_active *active,
> > +        struct drm_i915_gem_request *req)
> >  {
> > + struct intel_overlay *overlay =
> > + container_of(active, typeof(*overlay), last_flip);
> >   struct drm_i915_gem_object *obj = overlay->old_vid_bo;
> >
> > + i915_gem_track_fb(obj, NULL,
> > +   INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
> > +
>
> Previously this call was after the below calls and old_vid_bo was
> always NULL, does this fix a bug?

Yes.
https://bugs.freedesktop.org/show_bug.cgi?id=93730
and a recent one.

> >   i915_gem_object_ggtt_unpin(obj);
> >   i915_gem_object_put(obj);
> >
> >   overlay->old_vid_bo = NULL;
> >  }
> >
> >
> >  /* Wait for pending overlay flip and release old frame.
> > @@ -452,13 +450,9 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
> >       intel_overlay_release_old_vid_tail);
> >   if (ret)
> >   return ret;
> > - }
> > + } else
> > + intel_overlay_release_old_vid_tail(&overlay->last_flip, NULL);
>
> Why you added else?

Habit?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre