Re: [PATCH 3/3] drm/i915: Cache LRC state page in the context

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/3] drm/i915: Cache LRC state page in the context
Date: Fri, 15 Jan 2016 16:17:04 +0000	[thread overview]
Message-ID: <20160115161704.GA27877@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <569918C4.8020503@linux.intel.com>

On Fri, Jan 15, 2016 at 04:05:24PM +0000, Tvrtko Ursulin wrote:
> 
> On 15/01/16 16:01, Chris Wilson wrote:
> >On Fri, Jan 15, 2016 at 03:10:29PM +0000, Tvrtko Ursulin wrote:
> >>@@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request *rq)
> >>+	if (--rq->ctx->engine[ring->id].pin_count == 0) {
> >>+		lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj,
> >>+								LRC_STATE_PN);
> >
> >Interesting choice. We called set_page_dirty() when we took the mapping.
> >Should that page flag be preserved whilst we hold the kmap - I think so,
> >i.e. the mm cannot flush the page whilst it has an elevated mapcount. So
> >calling set_page_dirty() again is redundant, right?
> 
> If you call mindless copy & paste interesting. :D
> 
> Any other concerns or I can respin with that only?

No. I was quibbling over the excess clearing of state on unpinning :)

Pity we have to respin even for innoculous changes just to get a CI
tick.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx