Re: [PATCH 27/53] drm/i915: Update overlay code to do explicit request management

Tomas Elf
2015-03-05T18:51:59Z

On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The overlay update code path to do explicit request creation and submission
> rather than relying on the OLR to do the right thing.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/intel_overlay.c | 64 +++++++++++++++++++++++++---------
> 1 file changed, 48 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index f93dfc1..dc209bf 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -209,19 +209,19 @@ static void intel_overlay_unmap_regs(struct intel_overlay *overlay,
> }
>
> static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
> + struct drm_i915_gem_request *req,
> void (*tail)(struct intel_overlay *))
> {
> struct drm_device *dev = overlay->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_engine_cs *ring = &dev_priv->ring[RCS];
> int ret;
>
> BUG_ON(overlay->last_flip_req);
> - i915_gem_request_assign(&overlay->last_flip_req,
> - ring->outstanding_lazy_request);
> - ret = i915_add_request(ring);
> - if (ret)
> + i915_gem_request_assign(&overlay->last_flip_req, req);
> + ret = i915_add_request(req->ring);
> + if (ret) {
> + i915_gem_request_unreference(req);
> return ret;
> + }
>
> overlay->flip_tail = tail;
> ret = i915_wait_request(overlay->last_flip_req);
> @@ -239,6 +239,7 @@ static int intel_overlay_on(struct intel_overlay *overlay)
> struct drm_device *dev = overlay->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring = &dev_priv->ring[RCS];
> + struct drm_i915_gem_request *req;
> int ret;
>
> BUG_ON(overlay->active);
> @@ -246,17 +247,23 @@ static int intel_overlay_on(struct intel_overlay *overlay)
>
> WARN_ON(IS_I830(dev) && !(dev_priv->quirks & QUIRK_PIPEA_FORCE));
>
> - ret = intel_ring_begin(ring, 4);
> + ret = dev_priv->gt.alloc_request(ring, ring->default_context, &req);
> if (ret)
> return ret;
>
> + ret = intel_ring_begin(ring, 4);
> + if (ret) {
> + i915_gem_request_unreference(req);
> + return ret;
> + }
> +
> intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_ON);
> intel_ring_emit(ring, overlay->flip_addr | OFC_UPDATE);
> intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
> intel_ring_emit(ring, MI_NOOP);
> intel_ring_advance(ring);
>
> - return intel_overlay_do_wait_request(overlay, NULL);
> + return intel_overlay_do_wait_request(overlay, req, NULL);
> }
>
> /* overlay needs to be enabled in OCMD reg */
> @@ -266,6 +273,7 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
> struct drm_device *dev = overlay->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring = &dev_priv->ring[RCS];
> + struct drm_i915_gem_request *req;
> u32 flip_addr = overlay->flip_addr;
> u32 tmp;
> int ret;
> @@ -280,18 +288,27 @@ static int intel_overlay_continue(struct intel_overlay *overlay,
> if (tmp & (1 << 17))
> DRM_DEBUG("overlay underrun, DOVSTA: %x\n", tmp);
>
> - ret = intel_ring_begin(ring, 2);
> + ret = dev_priv->gt.alloc_request(ring, ring->default_context, &req);
> if (ret)
> return ret;
>
> + ret = intel_ring_begin(ring, 2);
> + if (ret) {
> + i915_gem_request_unreference(req);
> + return ret;
> + }
> +
> intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);
> intel_ring_emit(ring, flip_addr);
> intel_ring_advance(ring);
>
> WARN_ON(overlay->last_flip_req);
> - i915_gem_request_assign(&overlay->last_flip_req,
> - ring->outstanding_lazy_request);
> - return i915_add_request(ring);
> + i915_gem_request_assign(&overlay->last_flip_req, req);
> + ret = i915_add_request(req->ring);
> + if (ret)
> + i915_gem_request_unreference(req);
> +
> + return ret;
> }
>
> static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
> @@ -326,6 +343,7 @@ static int intel_overlay_off(struct intel_overlay *overlay)
> struct drm_device *dev = overlay->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring = &dev_priv->ring[RCS];
> + struct drm_i915_gem_request *req;
> u32 flip_addr = overlay->flip_addr;
> int ret;
>
> @@ -337,10 +355,16 @@ static int intel_overlay_off(struct intel_overlay *overlay)
> * of the hw. Do it in both cases */
> flip_addr |= OFC_UPDATE;
>
> - ret = intel_ring_begin(ring, 6);
> + ret = dev_priv->gt.alloc_request(ring, ring->default_context, &req);
> if (ret)
> return ret;
>
> + ret = intel_ring_begin(ring, 6);
> + if (ret) {
> + i915_gem_request_unreference(req);
> + return ret;
> + }
> +
> /* wait for overlay to go idle */
> intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);
> intel_ring_emit(ring, flip_addr);
> @@ -359,7 +383,7 @@ static int intel_overlay_off(struct intel_overlay *overlay)
> }
> intel_ring_advance(ring);
>
> - return intel_overlay_do_wait_request(overlay, intel_overlay_off_tail);
> + return intel_overlay_do_wait_request(overlay, req, intel_overlay_off_tail);
> }
>
> /* recover from an interruption due to a signal
> @@ -404,15 +428,23 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>
> if (I915_READ(ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT) {
> /* synchronous slowpath */
> - ret = intel_ring_begin(ring, 2);
> + struct drm_i915_gem_request *req;
> +
> + ret = dev_priv->gt.alloc_request(ring, ring->default_context, &req)
> if (ret)
> return ret;
>
> + ret = intel_ring_begin(ring, 2);
> + if (ret) {
> + i915_gem_request_unreference(req);
> + return ret;
> + }
> +
> intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
> intel_ring_emit(ring, MI_NOOP);
> intel_ring_advance(ring);
>
> - ret = intel_overlay_do_wait_request(overlay,
> + ret = intel_overlay_do_wait_request(overlay, req,
> intel_overlay_release_old_vid_tail);
> if (ret)
> return ret;
>

Reviewed-by: Tomas Elf <tomas.elf@intel.com>

Thanks,
Tomas