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

Joonas Lahtinen
2016-07-27T08:12:35Z

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?

> -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?

>   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?

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation