Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit acb868d3 authored by Chris Wilson's avatar Chris Wilson Committed by Daniel Vetter
Browse files

drm/i915: Disallow preallocation of requests 



The intention was to allow the caller to avoid a failure to queue a
request having already written commands to the ring. However, this is a
moot point as the i915_add_request() can fail for other reasons than a
mere allocation failure and those failure cases are more likely than
ENOMEM. So the overlay code already had to handle i915_add_request()
failures, and due to

commit 3bb73aba
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jul 20 12:40:59 2012 +0100

    drm/i915: Allow late allocation of request for i915_add_request()

the error handling code in intel_overlay.c was subject to causing
double-frees, as found by coverity.

Rather than further complicate i915_add_request() and callers, realise
the battle is lost and adapt intel_overlay.c to take advantage of the
late allocation of requests.

v2: Handle callers passing in a NULL seqno.
v3: Ditto. This time for sure.

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 26b6e44a

drivers/gpu/drm/i915/i915_drv.h

+1 −1
Original line number Diff line number Diff line
@@ -1427,7 +1427,7 @@ int __must_check i915_gpu_idle(struct drm_device *dev); 
int __must_check i915_gem_idle(struct drm_device *dev);

int i915_add_request(struct intel_ring_buffer *ring,

		     struct drm_file *file,

		     struct drm_i915_gem_request *request);

		     u32 *seqno);

int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,

				 uint32_t seqno);

int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);

drivers/gpu/drm/i915/i915_gem.c

+8 −7
Original line number Diff line number Diff line
@@ -1956,11 +1956,12 @@ i915_gem_next_request_seqno(struct intel_ring_buffer *ring) 
int

i915_add_request(struct intel_ring_buffer *ring,

		 struct drm_file *file,

		 struct drm_i915_gem_request *request)

		 u32 *out_seqno)

{

	drm_i915_private_t *dev_priv = ring->dev->dev_private;

	uint32_t seqno;

	struct drm_i915_gem_request *request;

	u32 request_ring_position;

	u32 seqno;

	int was_empty;

	int ret;
@@ -1975,11 +1976,9 @@ i915_add_request(struct intel_ring_buffer *ring, 
	if (ret)

		return ret;



	if (request == NULL) {

	request = kmalloc(sizeof(*request), GFP_KERNEL);

	if (request == NULL)

		return -ENOMEM;

	}



	seqno = i915_gem_next_request_seqno(ring);
@@ -2031,6 +2030,8 @@ i915_add_request(struct intel_ring_buffer *ring, 
		}

	}



	if (out_seqno)

		*out_seqno = seqno;

	return 0;

}

drivers/gpu/drm/i915/intel_overlay.c

+15 −57
Original line number Diff line number Diff line
@@ -210,7 +210,6 @@ 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 *request,

					 void (*tail)(struct intel_overlay *))

{

	struct drm_device *dev = overlay->dev;
@@ -219,12 +218,10 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay, 
	int ret;



	BUG_ON(overlay->last_flip_req);

	ret = i915_add_request(ring, NULL, request);

	if (ret) {

	    kfree(request);

	ret = i915_add_request(ring, NULL, &overlay->last_flip_req);

	if (ret)

		return ret;

	}

	overlay->last_flip_req = request->seqno;



	overlay->flip_tail = tail;

	ret = i915_wait_seqno(ring, overlay->last_flip_req);

	if (ret)
@@ -241,7 +238,6 @@ 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_ring_buffer *ring = &dev_priv->ring[RCS];

	struct drm_i915_gem_request *request;

	int ret;



	BUG_ON(overlay->active);
@@ -249,17 +245,9 @@ static int intel_overlay_on(struct intel_overlay *overlay) 


	WARN_ON(IS_I830(dev) && !(dev_priv->quirks & QUIRK_PIPEA_FORCE));



	request = kzalloc(sizeof(*request), GFP_KERNEL);

	if (request == NULL) {

		ret = -ENOMEM;

		goto out;

	}



	ret = intel_ring_begin(ring, 4);

	if (ret) {

		kfree(request);

		goto out;

	}

	if (ret)

		return ret;



	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_ON);

	intel_ring_emit(ring, overlay->flip_addr | OFC_UPDATE);
@@ -267,9 +255,7 @@ static int intel_overlay_on(struct intel_overlay *overlay) 
	intel_ring_emit(ring, MI_NOOP);

	intel_ring_advance(ring);



	ret = intel_overlay_do_wait_request(overlay, request, NULL);

out:

	return ret;

	return intel_overlay_do_wait_request(overlay, NULL);

}



/* overlay needs to be enabled in OCMD reg */
@@ -279,17 +265,12 @@ static int intel_overlay_continue(struct intel_overlay *overlay, 
	struct drm_device *dev = overlay->dev;

	drm_i915_private_t *dev_priv = dev->dev_private;

	struct intel_ring_buffer *ring = &dev_priv->ring[RCS];

	struct drm_i915_gem_request *request;

	u32 flip_addr = overlay->flip_addr;

	u32 tmp;

	int ret;



	BUG_ON(!overlay->active);



	request = kzalloc(sizeof(*request), GFP_KERNEL);

	if (request == NULL)

		return -ENOMEM;



	if (load_polyphase_filter)

		flip_addr |= OFC_UPDATE;
@@ -299,22 +280,14 @@ static int intel_overlay_continue(struct intel_overlay *overlay, 
		DRM_DEBUG("overlay underrun, DOVSTA: %x\n", tmp);



	ret = intel_ring_begin(ring, 2);

	if (ret) {

		kfree(request);

	if (ret)

		return ret;

	}



	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);

	intel_ring_emit(ring, flip_addr);

	intel_ring_advance(ring);



	ret = i915_add_request(ring, NULL, request);

	if (ret) {

		kfree(request);

		return ret;

	}



	overlay->last_flip_req = request->seqno;

	return 0;

	return i915_add_request(ring, NULL, &overlay->last_flip_req);

}



static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
@@ -350,15 +323,10 @@ static int intel_overlay_off(struct intel_overlay *overlay) 
	struct drm_i915_private *dev_priv = dev->dev_private;

	struct intel_ring_buffer *ring = &dev_priv->ring[RCS];

	u32 flip_addr = overlay->flip_addr;

	struct drm_i915_gem_request *request;

	int ret;



	BUG_ON(!overlay->active);



	request = kzalloc(sizeof(*request), GFP_KERNEL);

	if (request == NULL)

		return -ENOMEM;



	/* According to intel docs the overlay hw may hang (when switching

	 * off) without loading the filter coeffs. It is however unclear whether

	 * this applies to the disabling of the overlay or to the switching off
@@ -366,10 +334,9 @@ static int intel_overlay_off(struct intel_overlay *overlay) 
	flip_addr |= OFC_UPDATE;



	ret = intel_ring_begin(ring, 6);

	if (ret) {

		kfree(request);

	if (ret)

		return ret;

	}



	/* wait for overlay to go idle */

	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);

	intel_ring_emit(ring, flip_addr);
@@ -380,8 +347,7 @@ static int intel_overlay_off(struct intel_overlay *overlay) 
	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);

	intel_ring_advance(ring);



	return intel_overlay_do_wait_request(overlay, request,

					     intel_overlay_off_tail);

	return intel_overlay_do_wait_request(overlay, intel_overlay_off_tail);

}



/* recover from an interruption due to a signal
@@ -426,24 +392,16 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay) 
		return 0;



	if (I915_READ(ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT) {

		struct drm_i915_gem_request *request;



		/* synchronous slowpath */

		request = kzalloc(sizeof(*request), GFP_KERNEL);

		if (request == NULL)

			return -ENOMEM;



		ret = intel_ring_begin(ring, 2);

		if (ret) {

			kfree(request);

		if (ret)

			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, request,

		ret = intel_overlay_do_wait_request(overlay,

						    intel_overlay_release_old_vid_tail);

		if (ret)

			return ret;