[Intel-gfx] [PATCH v10] drm/i915: Extend LRC pinning to cover GPU context writeback

[Chris Wilson]

On Thu, Jan 14, 2016 at 11:56:07AM +0000, Nick Hoath wrote:
> On 14/01/2016 11:36, Chris Wilson wrote:
> >On Wed, Jan 13, 2016 at 04:19:45PM +0000, Nick Hoath wrote:
> >>+	if (ctx->engine[ring->id].dirty) {
> >>+		struct drm_i915_gem_request *req = NULL;
> >>+
> >>+		/**
> >>+		 * If there is already a request pending on
> >>+		 * this ring, wait for that to complete,
> >>+		 * otherwise create a switch to idle request
> >>+		 */
> >>+		if (list_empty(&ring->request_list)) {
> >>+			int ret;
> >>+
> >>+			ret = i915_gem_request_alloc(
> >>+					ring,
> >>+					ring->default_context,
> >>+					&req);
> >>+			if (!ret)
> >>+				i915_add_request(req);
> >>+			else
> >>+				DRM_DEBUG("Failed to ensure context saved");
> >>+		} else {
> >>+			req = list_first_entry(
> >>+					&ring->request_list,
> >>+					typeof(*req), list);
> >>+		}
> >>+		if (req) {
> >>+			ret = i915_wait_request(req);
> >>+			if (ret != 0) {
> >>+				/**
> >>+				 * If we get here, there's probably been a ring
> >>+				 * reset, so we just clean up the dirty flag.&
> >>+				 * pin count.
> >>+				 */
> >>+				ctx->engine[ring->id].dirty = false;
> >>+				__intel_lr_context_unpin(
> >>+					ring,
> >>+					ctx);
> >>+			}
> >>+		}
> >
> >If you were to take a lr_context_pin on the last_context, and only
> >release that pin when you change to a new context, you do not need to
> 
> That what this patch does.
> 
> >introduce a blocking context-close, nor do you need to introduce the
> >usage of default_context.
> 
> The use of default_context here is to stop a context hanging around
> after it is no longer needed.

By blocking, which is not acceptable. Also we can eliminate the
default_context and so pinning that opposed to the last_context serves
no purpose other than by chance having a more preferrable position when
it comes to defragmentation. But you don't enable that anyway and we
have alternative strategies now that avoid the issue with fragmentation
of the mappable aperture.

> >(lr_context_pin should take a reference on the ctx to prevent early
> >freeeing ofc).
> 
> You can't clear the reference on the ctx in an interrupt context.

The execlists submission should moved out of the interrupt context, for
the very simple reason that it is causing machine panics. userspace
submits a workload, machine lockups....
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre