diff mbox

drm/i915: Name the anonymous per-engine context struct

Message ID 1458313256-18601-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin March 18, 2016, 3 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This anonymous struct was causing a good amount of overly
verbose code. Also, if we name it and cache the pointer locally
when there are multiple accesses to it, not only the code is
more readable, but the compiler manages to generate smaller
binary.

Along the way I also shortened access to dev_priv and eliminated
some unused variables and cached some where I spotted the
opportunity.

Name for the structure, intel_context_engine, and the local
variable name were borrowed from a similar patch by Chris Wilson.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c | 96 +++++++++++++++++++++-------------------
 2 files changed, 51 insertions(+), 47 deletions(-)

Comments

Chris Wilson March 18, 2016, 3:14 p.m. UTC | #1 
On Fri, Mar 18, 2016 at 03:00:56PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This anonymous struct was causing a good amount of overly
> verbose code. Also, if we name it and cache the pointer locally
> when there are multiple accesses to it, not only the code is
> more readable, but the compiler manages to generate smaller
> binary.
> 
> Along the way I also shortened access to dev_priv and eliminated
> some unused variables and cached some where I spotted the
> opportunity.
> 
> Name for the structure, intel_context_engine, and the local
> variable name were borrowed from a similar patch by Chris Wilson.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Looks like a patch I've been maintaining for an eon, and I think Mika
once did a similar thing.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

If I'm far enough away from it to be impartial?
-Chris
Tvrtko Ursulin March 22, 2016, 9:53 a.m. UTC | #2 
On 21/03/16 11:22, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: Name the anonymous per-engine context struct (rev2)
> URL   : https://patchwork.freedesktop.org/series/4635/
> State : warning
>
> == Summary ==
>
> Series 4635v2 drm/i915: Name the anonymous per-engine context struct
> http://patchwork.freedesktop.org/api/1.0/series/4635/revisions/2/mbox/
>
> Test gem_ringfill:
>          Subgroup basic-default-s3:
>                  dmesg-warn -> PASS       (bsw-nuc-2)
> Test kms_flip:
>          Subgroup basic-flip-vs-dpms:
>                  pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE

Sporadic ILK pipe underruns: 
https://bugs.freedesktop.org/show_bug.cgi?id=93787

>          Subgroup basic-flip-vs-wf_vblank:
>                  fail       -> PASS       (byt-nuc)
>          Subgroup basic-plain-flip:
>                  pass       -> DMESG-WARN (hsw-brixbox)

Unrelated intermittent device suspended while HW access: 
https://bugs.freedesktop.org/show_bug.cgi?id=94349

> Test kms_force_connector_basic:
>          Subgroup force-connector-state:
>                  pass       -> SKIP       (ilk-hp8440p)
> Test kms_pipe_crc_basic:
>          Subgroup nonblocking-crc-pipe-a:
>                  dmesg-warn -> PASS       (snb-x220t)
>          Subgroup nonblocking-crc-pipe-a-frame-sequence:
>                  pass       -> DMESG-WARN (snb-dellxps)

Same.

>          Subgroup suspend-read-crc-pipe-c:
>                  pass       -> DMESG-WARN (bsw-nuc-2)


Unrelated lockdep splat: https://bugs.freedesktop.org/show_bug.cgi?id=94350

>                  incomplete -> PASS       (hsw-gt2)
> Test pm_rpm:
>          Subgroup basic-pci-d3-state:
>                  fail       -> DMESG-FAIL (snb-x220t)

Device suspended while HW access again.

>                  dmesg-warn -> PASS       (snb-dellxps)
>
> bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12
> bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21
> bsw-nuc-2        total:194  pass:156  dwarn:1   dfail:0   fail:0   skip:37
> byt-nuc          total:194  pass:159  dwarn:0   dfail:0   fail:0   skip:35
> hsw-brixbox      total:194  pass:171  dwarn:1   dfail:0   fail:0   skip:22
> hsw-gt2          total:194  pass:176  dwarn:1   dfail:0   fail:0   skip:17
> ilk-hp8440p      total:194  pass:129  dwarn:1   dfail:0   fail:0   skip:64
> ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25
> skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23
> snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34
> snb-x220t        total:194  pass:160  dwarn:0   dfail:1   fail:0   skip:33
>
> Results at /archive/results/CI_IGT_test/Patchwork_1649/
>
> e7a7673e9840fe8b50a5a2894c75565ec7858a00 drm-intel-nightly: 2016y-03m-19d-10h-09m-53s UTC integration manifest
> a1c5f9b1e8b9cbfdab0fb71ccf7a5a0838b56069 drm/i915: Name the anonymous per-engine context struct

So looking good.

Chris, r-b on v2? It was just a revert of a hunk which changed one 
instance of ctx->i915->dev->struct_mutex to engine->dev->struct_mutex 
which the CI reminded me is not allowed in some places.

Regards,

Tvrtko
Chris Wilson March 22, 2016, 10:07 a.m. UTC | #3 
On Tue, Mar 22, 2016 at 09:53:25AM +0000, Tvrtko Ursulin wrote:
> 
> 
> On 21/03/16 11:22, Patchwork wrote:
> >== Series Details ==
> >
> >Series: drm/i915: Name the anonymous per-engine context struct (rev2)
> >URL   : https://patchwork.freedesktop.org/series/4635/
> >State : warning
> >
> >== Summary ==
> >
> >Series 4635v2 drm/i915: Name the anonymous per-engine context struct
> >http://patchwork.freedesktop.org/api/1.0/series/4635/revisions/2/mbox/
> >
> >Test gem_ringfill:
> >         Subgroup basic-default-s3:
> >                 dmesg-warn -> PASS       (bsw-nuc-2)
> >Test kms_flip:
> >         Subgroup basic-flip-vs-dpms:
> >                 pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
> 
> Sporadic ILK pipe underruns:
> https://bugs.freedesktop.org/show_bug.cgi?id=93787
> 
> >         Subgroup basic-flip-vs-wf_vblank:
> >                 fail       -> PASS       (byt-nuc)
> >         Subgroup basic-plain-flip:
> >                 pass       -> DMESG-WARN (hsw-brixbox)
> 
> Unrelated intermittent device suspended while HW access:
> https://bugs.freedesktop.org/show_bug.cgi?id=94349
> 
> >Test kms_force_connector_basic:
> >         Subgroup force-connector-state:
> >                 pass       -> SKIP       (ilk-hp8440p)
> >Test kms_pipe_crc_basic:
> >         Subgroup nonblocking-crc-pipe-a:
> >                 dmesg-warn -> PASS       (snb-x220t)
> >         Subgroup nonblocking-crc-pipe-a-frame-sequence:
> >                 pass       -> DMESG-WARN (snb-dellxps)
> 
> Same.
> 
> >         Subgroup suspend-read-crc-pipe-c:
> >                 pass       -> DMESG-WARN (bsw-nuc-2)
> 
> 
> Unrelated lockdep splat: https://bugs.freedesktop.org/show_bug.cgi?id=94350

Is it really the same one? There should be another lockdep chain that
isn't in bugzilla...
 
> >                 incomplete -> PASS       (hsw-gt2)
> >Test pm_rpm:
> >         Subgroup basic-pci-d3-state:
> >                 fail       -> DMESG-FAIL (snb-x220t)
> 
> Device suspended while HW access again.
> 
> >                 dmesg-warn -> PASS       (snb-dellxps)
> >
> >bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12
> >bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21
> >bsw-nuc-2        total:194  pass:156  dwarn:1   dfail:0   fail:0   skip:37
> >byt-nuc          total:194  pass:159  dwarn:0   dfail:0   fail:0   skip:35
> >hsw-brixbox      total:194  pass:171  dwarn:1   dfail:0   fail:0   skip:22
> >hsw-gt2          total:194  pass:176  dwarn:1   dfail:0   fail:0   skip:17
> >ilk-hp8440p      total:194  pass:129  dwarn:1   dfail:0   fail:0   skip:64
> >ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25
> >skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23
> >snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34
> >snb-x220t        total:194  pass:160  dwarn:0   dfail:1   fail:0   skip:33
> >
> >Results at /archive/results/CI_IGT_test/Patchwork_1649/
> >
> >e7a7673e9840fe8b50a5a2894c75565ec7858a00 drm-intel-nightly: 2016y-03m-19d-10h-09m-53s UTC integration manifest
> >a1c5f9b1e8b9cbfdab0fb71ccf7a5a0838b56069 drm/i915: Name the anonymous per-engine context struct
> 
> So looking good.
> 
> Chris, r-b on v2? It was just a revert of a hunk which changed one
> instance of ctx->i915->dev->struct_mutex to
> engine->dev->struct_mutex which the CI reminded me is not allowed in
> some places.

That one again! One day we will get engine init/fini sorted. Yes,
-Chris
Tvrtko Ursulin March 22, 2016, 10:27 a.m. UTC | #4 
On 22/03/16 10:07, Chris Wilson wrote:
> On Tue, Mar 22, 2016 at 09:53:25AM +0000, Tvrtko Ursulin wrote:
>>
>>
>> On 21/03/16 11:22, Patchwork wrote:
>>> == Series Details ==
>>>
>>> Series: drm/i915: Name the anonymous per-engine context struct (rev2)
>>> URL   : https://patchwork.freedesktop.org/series/4635/
>>> State : warning
>>>
>>> == Summary ==
>>>
>>> Series 4635v2 drm/i915: Name the anonymous per-engine context struct
>>> http://patchwork.freedesktop.org/api/1.0/series/4635/revisions/2/mbox/
>>>
>>> Test gem_ringfill:
>>>          Subgroup basic-default-s3:
>>>                  dmesg-warn -> PASS       (bsw-nuc-2)
>>> Test kms_flip:
>>>          Subgroup basic-flip-vs-dpms:
>>>                  pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
>>
>> Sporadic ILK pipe underruns:
>> https://bugs.freedesktop.org/show_bug.cgi?id=93787
>>
>>>          Subgroup basic-flip-vs-wf_vblank:
>>>                  fail       -> PASS       (byt-nuc)
>>>          Subgroup basic-plain-flip:
>>>                  pass       -> DMESG-WARN (hsw-brixbox)
>>
>> Unrelated intermittent device suspended while HW access:
>> https://bugs.freedesktop.org/show_bug.cgi?id=94349
>>
>>> Test kms_force_connector_basic:
>>>          Subgroup force-connector-state:
>>>                  pass       -> SKIP       (ilk-hp8440p)
>>> Test kms_pipe_crc_basic:
>>>          Subgroup nonblocking-crc-pipe-a:
>>>                  dmesg-warn -> PASS       (snb-x220t)
>>>          Subgroup nonblocking-crc-pipe-a-frame-sequence:
>>>                  pass       -> DMESG-WARN (snb-dellxps)
>>
>> Same.
>>
>>>          Subgroup suspend-read-crc-pipe-c:
>>>                  pass       -> DMESG-WARN (bsw-nuc-2)
>>
>>
>> Unrelated lockdep splat: https://bugs.freedesktop.org/show_bug.cgi?id=94350
>
> Is it really the same one? There should be another lockdep chain that
> isn't in bugzilla...

Looks the same to me:

Bz:

[  179.762863] rtcwake/5995 is trying to acquire lock:
[  179.762877]  (s_active#6){++++.+}, at: [<ffffffff8124ec70>] 
kernfs_remove_by_name_ns+0x40/0xa0
[  179.762878]
but task is already holding lock:
[  179.762885]  (cpu_hotplug.lock){+.+.+.}, at: [<ffffffff81078c4d>] 
cpu_hotplug_begin+0x6d/0xc0
[  179.762886]
which lock already depends on the new lock.

This CI run:

[  127.210522] rtcwake/5947 is trying to acquire lock:
[  127.210539]  (s_active#6){++++.+}, at: [<ffffffff81250740>] 
kernfs_remove_by_name_ns+0x40/0xa0
[  127.210540]
but task is already holding lock:
[  127.210549]  (cpu_hotplug.lock){+.+.+.}, at: [<ffffffff81078f5d>] 
cpu_hotplug_begin+0x6d/0xc0
[  127.210550]
which lock already depends on the new lock.

>
>>>                  incomplete -> PASS       (hsw-gt2)
>>> Test pm_rpm:
>>>          Subgroup basic-pci-d3-state:
>>>                  fail       -> DMESG-FAIL (snb-x220t)
>>
>> Device suspended while HW access again.
>>
>>>                  dmesg-warn -> PASS       (snb-dellxps)
>>>
>>> bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12
>>> bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21
>>> bsw-nuc-2        total:194  pass:156  dwarn:1   dfail:0   fail:0   skip:37
>>> byt-nuc          total:194  pass:159  dwarn:0   dfail:0   fail:0   skip:35
>>> hsw-brixbox      total:194  pass:171  dwarn:1   dfail:0   fail:0   skip:22
>>> hsw-gt2          total:194  pass:176  dwarn:1   dfail:0   fail:0   skip:17
>>> ilk-hp8440p      total:194  pass:129  dwarn:1   dfail:0   fail:0   skip:64
>>> ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25
>>> skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23
>>> snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34
>>> snb-x220t        total:194  pass:160  dwarn:0   dfail:1   fail:0   skip:33
>>>
>>> Results at /archive/results/CI_IGT_test/Patchwork_1649/
>>>
>>> e7a7673e9840fe8b50a5a2894c75565ec7858a00 drm-intel-nightly: 2016y-03m-19d-10h-09m-53s UTC integration manifest
>>> a1c5f9b1e8b9cbfdab0fb71ccf7a5a0838b56069 drm/i915: Name the anonymous per-engine context struct
>>
>> So looking good.
>>
>> Chris, r-b on v2? It was just a revert of a hunk which changed one
>> instance of ctx->i915->dev->struct_mutex to
>> engine->dev->struct_mutex which the CI reminded me is not allowed in
>> some places.
>
> That one again! One day we will get engine init/fini sorted. Yes,

Yes r-b, just to be really sure?

Regards,

Tvrtko
Chris Wilson March 22, 2016, 10:32 a.m. UTC | #5 
On Tue, Mar 22, 2016 at 10:27:30AM +0000, Tvrtko Ursulin wrote:
> 
> On 22/03/16 10:07, Chris Wilson wrote:
> >Is it really the same one? There should be another lockdep chain that
> >isn't in bugzilla...
> 
> Looks the same to me:
> 
> Bz:
> 
> [  179.762863] rtcwake/5995 is trying to acquire lock:
> [  179.762877]  (s_active#6){++++.+}, at: [<ffffffff8124ec70>]
> kernfs_remove_by_name_ns+0x40/0xa0
> [  179.762878]
> but task is already holding lock:
> [  179.762885]  (cpu_hotplug.lock){+.+.+.}, at: [<ffffffff81078c4d>]
> cpu_hotplug_begin+0x6d/0xc0
> [  179.762886]
> which lock already depends on the new lock.
> 
> This CI run:
> 
> [  127.210522] rtcwake/5947 is trying to acquire lock:
> [  127.210539]  (s_active#6){++++.+}, at: [<ffffffff81250740>]
> kernfs_remove_by_name_ns+0x40/0xa0
> [  127.210540]
> but task is already holding lock:
> [  127.210549]  (cpu_hotplug.lock){+.+.+.}, at: [<ffffffff81078f5d>]
> cpu_hotplug_begin+0x6d/0xc0
> [  127.210550]
> which lock already depends on the new lock.

Ok, the chain I'm looking for doesn't involve the kernfs link.

> >
> >>>                 incomplete -> PASS       (hsw-gt2)
> >>>Test pm_rpm:
> >>>         Subgroup basic-pci-d3-state:
> >>>                 fail       -> DMESG-FAIL (snb-x220t)
> >>
> >>Device suspended while HW access again.
> >>
> >>>                 dmesg-warn -> PASS       (snb-dellxps)
> >>>
> >>>bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12
> >>>bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21
> >>>bsw-nuc-2        total:194  pass:156  dwarn:1   dfail:0   fail:0   skip:37
> >>>byt-nuc          total:194  pass:159  dwarn:0   dfail:0   fail:0   skip:35
> >>>hsw-brixbox      total:194  pass:171  dwarn:1   dfail:0   fail:0   skip:22
> >>>hsw-gt2          total:194  pass:176  dwarn:1   dfail:0   fail:0   skip:17
> >>>ilk-hp8440p      total:194  pass:129  dwarn:1   dfail:0   fail:0   skip:64
> >>>ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25
> >>>skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23
> >>>snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34
> >>>snb-x220t        total:194  pass:160  dwarn:0   dfail:1   fail:0   skip:33
> >>>
> >>>Results at /archive/results/CI_IGT_test/Patchwork_1649/
> >>>
> >>>e7a7673e9840fe8b50a5a2894c75565ec7858a00 drm-intel-nightly: 2016y-03m-19d-10h-09m-53s UTC integration manifest
> >>>a1c5f9b1e8b9cbfdab0fb71ccf7a5a0838b56069 drm/i915: Name the anonymous per-engine context struct
> >>
> >>So looking good.
> >>
> >>Chris, r-b on v2? It was just a revert of a hunk which changed one
> >>instance of ctx->i915->dev->struct_mutex to
> >>engine->dev->struct_mutex which the CI reminded me is not allowed in
> >>some places.
> >
> >That one again! One day we will get engine init/fini sorted. Yes,
> 
> Yes r-b, just to be really sure?

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 00c41a4bde2a..480639c39543 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -840,7 +840,7 @@  struct intel_context {
 	} legacy_hw_ctx;
 
 	/* Execlists */
-	struct {
+	struct intel_context_engine {
 		struct drm_i915_gem_object *state;
 		struct intel_ringbuffer *ringbuf;
 		int pin_count;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3a23b9549f7b..c61f679a40ee 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -316,16 +316,16 @@  static void
 intel_lr_context_descriptor_update(struct intel_context *ctx,
 				   struct intel_engine_cs *engine)
 {
+	struct intel_context_engine *ce = &ctx->engine[engine->id];
 	uint64_t lrca, desc;
 
-	lrca = ctx->engine[engine->id].lrc_vma->node.start +
-	       LRC_PPHWSP_PN * PAGE_SIZE;
+	lrca = ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
 
-	desc = engine->ctx_desc_template;			   /* bits  0-11 */
+	desc = engine->ctx_desc_template;		   /* bits  0-11 */
 	desc |= lrca;					   /* bits 12-31 */
 	desc |= (lrca >> PAGE_SHIFT) << GEN8_CTX_ID_SHIFT; /* bits 32-51 */
 
-	ctx->engine[engine->id].lrc_desc = desc;
+	ce->lrc_desc = desc;
 }
 
 uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
@@ -361,8 +361,7 @@  static void execlists_elsp_write(struct drm_i915_gem_request *rq0,
 {
 
 	struct intel_engine_cs *engine = rq0->engine;
-	struct drm_device *dev = engine->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = rq0->i915;
 	uint64_t desc[2];
 
 	if (rq1) {
@@ -670,7 +669,8 @@  static int logical_ring_invalidate_all_caches(struct drm_i915_gem_request *req)
 static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
 				 struct list_head *vmas)
 {
-	const unsigned other_rings = ~intel_engine_flag(req->engine);
+	struct intel_engine_cs *engine = req->engine;
+	const unsigned other_rings = ~intel_engine_flag(engine);
 	struct i915_vma *vma;
 	uint32_t flush_domains = 0;
 	bool flush_chipset = false;
@@ -680,7 +680,7 @@  static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
 		struct drm_i915_gem_object *obj = vma->obj;
 
 		if (obj->active & other_rings) {
-			ret = i915_gem_object_sync(obj, req->engine, &req);
+			ret = i915_gem_object_sync(obj, engine, &req);
 			if (ret)
 				return ret;
 		}
@@ -782,6 +782,7 @@  intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
 	struct intel_ringbuffer *ringbuf = request->ringbuf;
 	struct drm_i915_private *dev_priv = request->i915;
 	struct intel_engine_cs *engine = request->engine;
+	struct intel_context *ctx = request->ctx;
 
 	intel_logical_ring_advance(ringbuf);
 	request->tail = ringbuf->tail;
@@ -799,12 +800,12 @@  intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
 	if (intel_engine_stopped(engine))
 		return 0;
 
-	if (engine->last_context != request->ctx) {
+	if (engine->last_context != ctx) {
 		if (engine->last_context)
 			intel_lr_context_unpin(engine->last_context, engine);
-		if (request->ctx != request->i915->kernel_context) {
-			intel_lr_context_pin(request->ctx, engine);
-			engine->last_context = request->ctx;
+		if (ctx != dev_priv->kernel_context) {
+			intel_lr_context_pin(ctx, engine);
+			engine->last_context = ctx;
 		} else {
 			engine->last_context = NULL;
 		}
@@ -949,9 +950,8 @@  int intel_execlists_submission(struct i915_execbuffer_params *params,
 			       struct drm_i915_gem_execbuffer2 *args,
 			       struct list_head *vmas)
 {
-	struct drm_device       *dev = params->dev;
 	struct intel_engine_cs *engine = params->engine;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(params->dev);
 	struct intel_ringbuffer *ringbuf = params->ctx->engine[engine->id].ringbuf;
 	u64 exec_start;
 	int instp_mode;
@@ -1092,17 +1092,18 @@  static int intel_lr_context_do_pin(struct intel_context *ctx,
 				   struct intel_engine_cs *engine)
 {
 	struct drm_device *dev = engine->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
-	struct intel_ringbuffer *ringbuf = ctx->engine[engine->id].ringbuf;
+	struct drm_i915_private *dev_priv = ctx->i915;
+	struct intel_context_engine *ce = &ctx->engine[engine->id];
+	struct drm_i915_gem_object *ctx_obj = ce->state;
+	struct intel_ringbuffer *ringbuf = ce->ringbuf;
 	struct page *lrc_state_page;
 	uint32_t *lrc_reg_state;
 	int ret;
 
-	WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
 	ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
-			PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+				    PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
 	if (ret)
 		return ret;
 
@@ -1112,15 +1113,15 @@  static int intel_lr_context_do_pin(struct intel_context *ctx,
 		goto unpin_ctx_obj;
 	}
 
-	ret = intel_pin_and_map_ringbuffer_obj(engine->dev, ringbuf);
+	ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
 	if (ret)
 		goto unpin_ctx_obj;
 
-	ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
+	ce->lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
 	intel_lr_context_descriptor_update(ctx, engine);
 	lrc_reg_state = kmap(lrc_state_page);
 	lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start;
-	ctx->engine[engine->id].lrc_reg_state = lrc_reg_state;
+	ce->lrc_reg_state = lrc_reg_state;
 	ctx_obj->dirty = true;
 
 	/* Invalidate GuC TLB. */
@@ -1138,9 +1139,10 @@  unpin_ctx_obj:
 static int intel_lr_context_pin(struct intel_context *ctx,
 				struct intel_engine_cs *engine)
 {
+	struct intel_context_engine *ce = &ctx->engine[engine->id];
 	int ret = 0;
 
-	if (ctx->engine[engine->id].pin_count++ == 0) {
+	if (ce->pin_count++ == 0) {
 		ret = intel_lr_context_do_pin(ctx, engine);
 		if (ret)
 			goto reset_pin_count;
@@ -1150,23 +1152,25 @@  static int intel_lr_context_pin(struct intel_context *ctx,
 	return ret;
 
 reset_pin_count:
-	ctx->engine[engine->id].pin_count = 0;
+	ce->pin_count = 0;
 	return ret;
 }
 
 void intel_lr_context_unpin(struct intel_context *ctx,
 			    struct intel_engine_cs *engine)
 {
-	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
+	struct intel_context_engine *ce = &ctx->engine[engine->id];
+	struct drm_i915_gem_object *ctx_obj = ce->state;
 
-	WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
-	if (--ctx->engine[engine->id].pin_count == 0) {
-		kunmap(kmap_to_page(ctx->engine[engine->id].lrc_reg_state));
-		intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf);
+	WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
+
+	if (--ce->pin_count == 0) {
+		kunmap(kmap_to_page(ce->lrc_reg_state));
+		intel_unpin_ringbuffer_obj(ce->ringbuf);
 		i915_gem_object_ggtt_unpin(ctx_obj);
-		ctx->engine[engine->id].lrc_vma = NULL;
-		ctx->engine[engine->id].lrc_desc = 0;
-		ctx->engine[engine->id].lrc_reg_state = NULL;
+		ce->lrc_vma = NULL;
+		ce->lrc_desc = 0;
+		ce->lrc_reg_state = NULL;
 
 		i915_gem_context_unreference(ctx);
 	}
@@ -1177,8 +1181,7 @@  static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
 	int ret, i;
 	struct intel_engine_cs *engine = req->engine;
 	struct intel_ringbuffer *ringbuf = req->ringbuf;
-	struct drm_device *dev = engine->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = req->i915;
 	struct i915_workarounds *w = &dev_priv->workarounds;
 
 	if (w->count == 0)
@@ -2513,8 +2516,9 @@  void intel_lr_context_free(struct intel_context *ctx)
 	int i;
 
 	for (i = I915_NUM_ENGINES; --i >= 0; ) {
-		struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
-		struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
+		struct intel_context_engine *ce = &ctx->engine[i];
+		struct intel_ringbuffer *ringbuf = ce->ringbuf;
+		struct drm_i915_gem_object *ctx_obj = ce->state;
 
 		if (!ctx_obj)
 			continue;
@@ -2524,7 +2528,7 @@  void intel_lr_context_free(struct intel_context *ctx)
 			i915_gem_object_ggtt_unpin(ctx_obj);
 		}
 
-		WARN_ON(ctx->engine[i].pin_count);
+		WARN_ON(ce->pin_count);
 		intel_ringbuffer_free(ringbuf);
 		drm_gem_object_unreference(&ctx_obj->base);
 	}
@@ -2604,13 +2608,14 @@  int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 				    struct intel_engine_cs *engine)
 {
 	struct drm_device *dev = engine->dev;
+	struct intel_context_engine *ce = &ctx->engine[engine->id];
 	struct drm_i915_gem_object *ctx_obj;
 	uint32_t context_size;
 	struct intel_ringbuffer *ringbuf;
 	int ret;
 
 	WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL);
-	WARN_ON(ctx->engine[engine->id].state);
+	WARN_ON(ce->state);
 
 	context_size = round_up(intel_lr_context_size(engine), 4096);
 
@@ -2635,8 +2640,8 @@  int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 		goto error_ringbuf;
 	}
 
-	ctx->engine[engine->id].ringbuf = ringbuf;
-	ctx->engine[engine->id].state = ctx_obj;
+	ce->ringbuf = ringbuf;
+	ce->state = ctx_obj;
 
 	if (ctx != ctx->i915->kernel_context && engine->init_context) {
 		struct drm_i915_gem_request *req;
@@ -2663,8 +2668,8 @@  error_ringbuf:
 	intel_ringbuffer_free(ringbuf);
 error_deref_obj:
 	drm_gem_object_unreference(&ctx_obj->base);
-	ctx->engine[engine->id].ringbuf = NULL;
-	ctx->engine[engine->id].state = NULL;
+	ce->ringbuf = NULL;
+	ce->state = NULL;
 	return ret;
 }
 
@@ -2676,10 +2681,9 @@  void intel_lr_context_reset(struct drm_device *dev,
 	int i;
 
 	for_each_engine(engine, dev_priv, i) {
-		struct drm_i915_gem_object *ctx_obj =
-				ctx->engine[engine->id].state;
-		struct intel_ringbuffer *ringbuf =
-				ctx->engine[engine->id].ringbuf;
+		struct intel_context_engine *ce = &ctx->engine[engine->id];
+		struct drm_i915_gem_object *ctx_obj = ce->state;
+		struct intel_ringbuffer *ringbuf = ce->ringbuf;
 		uint32_t *reg_state;
 		struct page *page;