Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
Quoting Tvrtko Ursulin (2019-08-01 17:22:27) > > On 01/08/2019 17:00, Chris Wilson wrote: > Who kidnapped real Chris? :D We could merge the mask clearing and reduce > pin to one conditional and one and, shift, or. :) Don't worry in about 24 patches time, we can remove the branches. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
On 01/08/2019 17:00, Chris Wilson wrote:
Quoting Chris Wilson (2019-08-01 16:48:33)
Quoting Tvrtko Ursulin (2019-08-01 16:29:53)
For instance Icelake engine dependent stuff sneaked into
intel_lrc.c/lrc_desriptors at some point, which is also against the
spirit of caching. If we were to move the cached value in ce then we
would be able to remove that and have it once again minimal in there.
Well we can set all bits but hw_id/lrca at init time. How about if I run
that past you?
static u64
-lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
+base_lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
{
- struct i915_gem_context *ctx = ce->gem_context;
u64 desc;
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
@@ -426,18 +425,12 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
if (IS_GEN(engine->i915, 8))
desc |= GEN8_CTX_L3LLC_COHERENT;
- desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
- /* bits 12-31 */
/*
* The following 32bits are copied into the OA reports (dword 2).
* Consider updating oa_get_render_ctx_id in i915_perf.c when changing
* anything below.
*/
if (INTEL_GEN(engine->i915) >= 11) {
- GEM_BUG_ON(ctx->hw_id >= BIT(GEN11_SW_CTX_ID_WIDTH));
- desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
- /* bits 37-47 */
-
desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
/* bits 48-53
*/
@@ -445,8 +438,29 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
/* bits 61-63
*/
+ }
+
+ return desc;
+}
+
+static u64
+update_lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
+{
+ struct i915_gem_context *ctx = ce->gem_context;
+ u64 desc = ce->lrc_desc;
+
+ desc &= ~GENMASK_ULL(31, 12);
+ desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
+
+ if (INTEL_GEN(engine->i915) >= 11) {
+ GEM_BUG_ON(ctx->hw_id >= BIT(GEN11_SW_CTX_ID_WIDTH));
+
+ desc &= ~GENMASK_ULL(47, 37);
+ desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
} else {
GEM_BUG_ON(ctx->hw_id >= BIT(GEN8_CTX_ID_WIDTH));
+
+ desc &= ~GENMASK_ULL(52, 32);
desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-52
*/
}
@@ -1631,7 +1645,7 @@ __execlists_context_pin(struct intel_context *ce,
if (ret)
goto unpin_map;
- ce->lrc_desc = lrc_descriptor(ce, engine);
+ ce->lrc_desc = update_lrc_descriptor(ce, engine);
ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
__execlists_update_reg_state(ce, engine);
@@ -3126,6 +3140,8 @@ static int execlists_context_deferred_alloc(struct
intel_context *ce,
ce->ring = ring;
ce->state = vma;
+ ce->lrc_desc = base_lrc_descriptor(ce, engine);
+
return 0;
error_ring_free:
That's pretty much the same amount of work in context_pin. I'm not
convinced that caching between pins achieves very much.
Concur?
Who kidnapped real Chris? :D We could merge the mask clearing and reduce
pin to one conditional and one and, shift, or. :)
Okay, have it your way.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
Quoting Chris Wilson (2019-08-01 16:48:33)
> Quoting Tvrtko Ursulin (2019-08-01 16:29:53)
> > For instance Icelake engine dependent stuff sneaked into
> > intel_lrc.c/lrc_desriptors at some point, which is also against the
> > spirit of caching. If we were to move the cached value in ce then we
> > would be able to remove that and have it once again minimal in there.
>
> Well we can set all bits but hw_id/lrca at init time. How about if I run
> that past you?
static u64
-lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
+base_lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
{
- struct i915_gem_context *ctx = ce->gem_context;
u64 desc;
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
@@ -426,18 +425,12 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
if (IS_GEN(engine->i915, 8))
desc |= GEN8_CTX_L3LLC_COHERENT;
- desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
- /* bits 12-31 */
/*
* The following 32bits are copied into the OA reports (dword 2).
* Consider updating oa_get_render_ctx_id in i915_perf.c when changing
* anything below.
*/
if (INTEL_GEN(engine->i915) >= 11) {
- GEM_BUG_ON(ctx->hw_id >= BIT(GEN11_SW_CTX_ID_WIDTH));
- desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
- /* bits 37-47 */
-
desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
/* bits 48-53 */
@@ -445,8 +438,29 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
/* bits 61-63 */
+ }
+
+ return desc;
+}
+
+static u64
+update_lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
+{
+ struct i915_gem_context *ctx = ce->gem_context;
+ u64 desc = ce->lrc_desc;
+
+ desc &= ~GENMASK_ULL(31, 12);
+ desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
+
+ if (INTEL_GEN(engine->i915) >= 11) {
+ GEM_BUG_ON(ctx->hw_id >= BIT(GEN11_SW_CTX_ID_WIDTH));
+
+ desc &= ~GENMASK_ULL(47, 37);
+ desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
} else {
GEM_BUG_ON(ctx->hw_id >= BIT(GEN8_CTX_ID_WIDTH));
+
+ desc &= ~GENMASK_ULL(52, 32);
desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT; /* bits 32-52 */
}
@@ -1631,7 +1645,7 @@ __execlists_context_pin(struct intel_context *ce,
if (ret)
goto unpin_map;
- ce->lrc_desc = lrc_descriptor(ce, engine);
+ ce->lrc_desc = update_lrc_descriptor(ce, engine);
ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
__execlists_update_reg_state(ce, engine);
@@ -3126,6 +3140,8 @@ static int execlists_context_deferred_alloc(struct
intel_context *ce,
ce->ring = ring;
ce->state = vma;
+ ce->lrc_desc = base_lrc_descriptor(ce, engine);
+
return 0;
error_ring_free:
That's pretty much the same amount of work in context_pin. I'm not
convinced that caching between pins achieves very much.
Concur?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
Quoting Tvrtko Ursulin (2019-08-01 16:29:53) > > On 01/08/2019 12:13, Chris Wilson wrote: > > Quoting Chris Wilson (2019-08-01 11:57:06) > >> Quoting Tvrtko Ursulin (2019-08-01 09:53:15) > >>> We could store it in ce then. We already have well defined control > >>> points for when vm changes when all are updated. > >> > >> We are storing it in ce; it's not like we recompute it all that often, > >> and when we do it's because we have rebound the vma. > >> > >>> If done like this then it looks like assigning ctx->hw_id could also do > >>> the default_desc update, so that we can avoid even more work done at pin > >>> time. > >> > >> What ctx->hw_id? You are imagining things again :-p > >> > >> Remember that we only do this on first pin from idle, not every pin. > > > > Fwiw, I quickly looked at only doing it if the vma is rebound, but > > that's move branches just to save a couple. The low frequency at which > > we have to actually compute this (walk a few more branches inside an > > already branchy contxt_pin) doesn't seem to justify the extra storage for > > me. It's not like we are recomputing lrc_desc on every submit as it once > > was. > > On every submit if last request got retired in the meantime, no, for > instance bursty loads? Yeah it is very inconsequential but at some point > we made an effort to cache as much as possible what is invariant so it > saddens me a bit to remove that. Once we have hw_id out of the way, we only need to set the bottom 32b here. > For instance Icelake engine dependent stuff sneaked into > intel_lrc.c/lrc_desriptors at some point, which is also against the > spirit of caching. If we were to move the cached value in ce then we > would be able to remove that and have it once again minimal in there. Well we can set all bits but hw_id/lrca at init time. How about if I run that past you? > Not only just minimal, but not separated in two separate places. I guess > this patch improves things in that respect - consolidates the lrc_desc > computation once again. > > I did not get the part about VMA re-binding. I did not suggest to move > the lrca offset into cache as well. I was just thinking about the gen, > engine and vm dependent bits could naturally go into > i915_gem_context.c/default_desc_template. Just need to take (engine, > hw_id, vm). I'm just thinking about the bit that changes inside ce->lrc_desc. > And virtual engine would have to re-compute it when moving engines. Hm.. > we don't seem to do that? Only when pinning we set it up based on > sibling[0] so how it all works? We don't re-pin when moving engine I > thought. No. We don't. Whoops. Good job clearly nothing uses that then. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
On 01/08/2019 12:13, Chris Wilson wrote: Quoting Chris Wilson (2019-08-01 11:57:06) Quoting Tvrtko Ursulin (2019-08-01 09:53:15) We could store it in ce then. We already have well defined control points for when vm changes when all are updated. We are storing it in ce; it's not like we recompute it all that often, and when we do it's because we have rebound the vma. If done like this then it looks like assigning ctx->hw_id could also do the default_desc update, so that we can avoid even more work done at pin time. What ctx->hw_id? You are imagining things again :-p Remember that we only do this on first pin from idle, not every pin. Fwiw, I quickly looked at only doing it if the vma is rebound, but that's move branches just to save a couple. The low frequency at which we have to actually compute this (walk a few more branches inside an already branchy contxt_pin) doesn't seem to justify the extra storage for me. It's not like we are recomputing lrc_desc on every submit as it once was. On every submit if last request got retired in the meantime, no, for instance bursty loads? Yeah it is very inconsequential but at some point we made an effort to cache as much as possible what is invariant so it saddens me a bit to remove that. For instance Icelake engine dependent stuff sneaked into intel_lrc.c/lrc_desriptors at some point, which is also against the spirit of caching. If we were to move the cached value in ce then we would be able to remove that and have it once again minimal in there. Not only just minimal, but not separated in two separate places. I guess this patch improves things in that respect - consolidates the lrc_desc computation once again. I did not get the part about VMA re-binding. I did not suggest to move the lrca offset into cache as well. I was just thinking about the gen, engine and vm dependent bits could naturally go into i915_gem_context.c/default_desc_template. Just need to take (engine, hw_id, vm). And virtual engine would have to re-compute it when moving engines. Hm.. we don't seem to do that? Only when pinning we set it up based on sibling[0] so how it all works? We don't re-pin when moving engine I thought. Aside that, if you are still not convinced my argument makes sense, you can have my ack. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
Quoting Chris Wilson (2019-08-01 11:57:06) > Quoting Tvrtko Ursulin (2019-08-01 09:53:15) > > We could store it in ce then. We already have well defined control > > points for when vm changes when all are updated. > > We are storing it in ce; it's not like we recompute it all that often, > and when we do it's because we have rebound the vma. > > > If done like this then it looks like assigning ctx->hw_id could also do > > the default_desc update, so that we can avoid even more work done at pin > > time. > > What ctx->hw_id? You are imagining things again :-p > > Remember that we only do this on first pin from idle, not every pin. Fwiw, I quickly looked at only doing it if the vma is rebound, but that's move branches just to save a couple. The low frequency at which we have to actually compute this (walk a few more branches inside an already branchy contxt_pin) doesn't seem to justify the extra storage for me. It's not like we are recomputing lrc_desc on every submit as it once was. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
Quoting Tvrtko Ursulin (2019-08-01 09:53:15)
>
> On 01/08/2019 09:41, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-08-01 09:37:38)
> >>
> >> On 24/07/2019 10:20, Tvrtko Ursulin wrote:
> >>>
> >>> On 23/07/2019 19:38, Chris Wilson wrote:
> We only compute the lrc_descriptor() on pinning the context, i.e.
> infrequently, so we do not benefit from storing the template as the
> addressing mode is also fixed for the lifetime of the intel_context.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-
> .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
> drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +---
> drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
> 4 files changed, 10 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index b28c7ca681a8..1b3dc7258ef2 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context
> *ctx)
> i915_gem_context_put(ctx);
> }
> -static u32 default_desc_template(const struct drm_i915_private *i915,
> - const struct i915_address_space *vm)
> -{
> - u32 address_mode;
> - u32 desc;
> -
> - desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
> -
> - address_mode = INTEL_LEGACY_32B_CONTEXT;
> - if (vm && i915_vm_is_4lvl(vm))
> - address_mode = INTEL_LEGACY_64B_CONTEXT;
> - desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
> -
> - if (IS_GEN(i915, 8))
> - desc |= GEN8_CTX_L3LLC_COHERENT;
> -
> - /* TODO: WaDisableLiteRestore when we start using semaphore
> - * signalling between Command Streamers
> - * ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
> - */
> -
> - return desc;
> -}
> -
> static struct i915_gem_context *
> __create_context(struct drm_i915_private *i915)
> {
> @@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
> i915_gem_context_set_recoverable(ctx);
> ctx->ring_size = 4 * PAGE_SIZE;
> - ctx->desc_template = default_desc_template(i915, NULL);
> for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
> ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
> @@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct
> i915_address_space *vm)
> struct i915_gem_engines_iter it;
> struct intel_context *ce;
> + GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
> +
> ctx->vm = i915_vm_get(vm);
> - ctx->desc_template = default_desc_template(ctx->i915, vm);
> for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
> i915_vm_put(ce->vm);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> index 0ee61482ef94..a02d98494078 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> @@ -171,8 +171,6 @@ struct i915_gem_context {
> /** ring_size: size for allocating the per-engine ring buffer */
> u32 ring_size;
> - /** desc_template: invariant fields for the HW context descriptor */
> - u32 desc_template;
> /** guilty_count: How many times this context has caused a GPU
> hang. */
> atomic_t guilty_count;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 632344c163a8..5fdac40015cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -418,13 +418,17 @@ lrc_descriptor(struct intel_context *ce, struct
> intel_engine_cs *engine)
> BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
> BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID >
> (BIT(GEN11_SW_CTX_ID_WIDTH)));
> - desc = ctx->desc_template; /* bits 0-11 */
> - GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
> + desc = INTEL_LEGACY_32B_CONTEXT;
> + if (i915_vm_is_4lvl(ce->vm))
> + desc = INTEL_LEGACY_64B_CONTEXT;
> >>>
> >>> if-else now that the vm null check is gone.
> >>>
> + desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
> +
> + desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
> + if (IS_GEN(engine->i915, 8))
> + desc |= GEN8_CTX_L3LLC_COHERENT;
> >>>
> >>> Don't know.. it's nicer to keep it stored it both for Gen and context
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
On 01/08/2019 09:41, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-08-01 09:37:38)
On 24/07/2019 10:20, Tvrtko Ursulin wrote:
On 23/07/2019 19:38, Chris Wilson wrote:
We only compute the lrc_descriptor() on pinning the context, i.e.
infrequently, so we do not benefit from storing the template as the
addressing mode is also fixed for the lifetime of the intel_context.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +---
drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
4 files changed, 10 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index b28c7ca681a8..1b3dc7258ef2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context
*ctx)
i915_gem_context_put(ctx);
}
-static u32 default_desc_template(const struct drm_i915_private *i915,
- const struct i915_address_space *vm)
-{
- u32 address_mode;
- u32 desc;
-
- desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
-
- address_mode = INTEL_LEGACY_32B_CONTEXT;
- if (vm && i915_vm_is_4lvl(vm))
- address_mode = INTEL_LEGACY_64B_CONTEXT;
- desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
-
- if (IS_GEN(i915, 8))
- desc |= GEN8_CTX_L3LLC_COHERENT;
-
- /* TODO: WaDisableLiteRestore when we start using semaphore
- * signalling between Command Streamers
- * ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
- */
-
- return desc;
-}
-
static struct i915_gem_context *
__create_context(struct drm_i915_private *i915)
{
@@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
i915_gem_context_set_recoverable(ctx);
ctx->ring_size = 4 * PAGE_SIZE;
- ctx->desc_template = default_desc_template(i915, NULL);
for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
@@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct
i915_address_space *vm)
struct i915_gem_engines_iter it;
struct intel_context *ce;
+ GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
+
ctx->vm = i915_vm_get(vm);
- ctx->desc_template = default_desc_template(ctx->i915, vm);
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
i915_vm_put(ce->vm);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 0ee61482ef94..a02d98494078 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -171,8 +171,6 @@ struct i915_gem_context {
/** ring_size: size for allocating the per-engine ring buffer */
u32 ring_size;
- /** desc_template: invariant fields for the HW context descriptor */
- u32 desc_template;
/** guilty_count: How many times this context has caused a GPU
hang. */
atomic_t guilty_count;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 632344c163a8..5fdac40015cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -418,13 +418,17 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID >
(BIT(GEN11_SW_CTX_ID_WIDTH)));
- desc = ctx->desc_template; /* bits 0-11 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
+ desc = INTEL_LEGACY_32B_CONTEXT;
+ if (i915_vm_is_4lvl(ce->vm))
+ desc = INTEL_LEGACY_64B_CONTEXT;
if-else now that the vm null check is gone.
+ desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
+
+ desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
+ if (IS_GEN(engine->i915, 8))
+ desc |= GEN8_CTX_L3LLC_COHERENT;
Don't know.. it's nicer to keep it stored it both for Gen and context
state. What's the problem with it?
Ping.
There's no gem_context.
We could store it in ce then. We already have well defined control
points for when vm changes when all are updated.
If done like this then it looks like assigning ctx->hw_id could also do
the default_desc update, so that we can avoid even more work done at pin
time.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
Quoting Tvrtko Ursulin (2019-08-01 09:37:38)
>
> On 24/07/2019 10:20, Tvrtko Ursulin wrote:
> >
> > On 23/07/2019 19:38, Chris Wilson wrote:
> >> We only compute the lrc_descriptor() on pinning the context, i.e.
> >> infrequently, so we do not benefit from storing the template as the
> >> addressing mode is also fixed for the lifetime of the intel_context.
> >>
> >> Signed-off-by: Chris Wilson
> >> ---
> >> drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-
> >> .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
> >> drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +---
> >> drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
> >> 4 files changed, 10 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >> index b28c7ca681a8..1b3dc7258ef2 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >> @@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context
> >> *ctx)
> >> i915_gem_context_put(ctx);
> >> }
> >> -static u32 default_desc_template(const struct drm_i915_private *i915,
> >> - const struct i915_address_space *vm)
> >> -{
> >> - u32 address_mode;
> >> - u32 desc;
> >> -
> >> - desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
> >> -
> >> - address_mode = INTEL_LEGACY_32B_CONTEXT;
> >> - if (vm && i915_vm_is_4lvl(vm))
> >> - address_mode = INTEL_LEGACY_64B_CONTEXT;
> >> - desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
> >> -
> >> - if (IS_GEN(i915, 8))
> >> - desc |= GEN8_CTX_L3LLC_COHERENT;
> >> -
> >> - /* TODO: WaDisableLiteRestore when we start using semaphore
> >> - * signalling between Command Streamers
> >> - * ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
> >> - */
> >> -
> >> - return desc;
> >> -}
> >> -
> >> static struct i915_gem_context *
> >> __create_context(struct drm_i915_private *i915)
> >> {
> >> @@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
> >> i915_gem_context_set_recoverable(ctx);
> >> ctx->ring_size = 4 * PAGE_SIZE;
> >> - ctx->desc_template = default_desc_template(i915, NULL);
> >> for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
> >> ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
> >> @@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct
> >> i915_address_space *vm)
> >> struct i915_gem_engines_iter it;
> >> struct intel_context *ce;
> >> + GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
> >> +
> >> ctx->vm = i915_vm_get(vm);
> >> - ctx->desc_template = default_desc_template(ctx->i915, vm);
> >> for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
> >> i915_vm_put(ce->vm);
> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >> b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >> index 0ee61482ef94..a02d98494078 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >> @@ -171,8 +171,6 @@ struct i915_gem_context {
> >> /** ring_size: size for allocating the per-engine ring buffer */
> >> u32 ring_size;
> >> - /** desc_template: invariant fields for the HW context descriptor */
> >> - u32 desc_template;
> >> /** guilty_count: How many times this context has caused a GPU
> >> hang. */
> >> atomic_t guilty_count;
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> index 632344c163a8..5fdac40015cf 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >> @@ -418,13 +418,17 @@ lrc_descriptor(struct intel_context *ce, struct
> >> intel_engine_cs *engine)
> >> BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
> >> BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID >
> >> (BIT(GEN11_SW_CTX_ID_WIDTH)));
> >> - desc = ctx->desc_template; /* bits 0-11 */
> >> - GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
> >> + desc = INTEL_LEGACY_32B_CONTEXT;
> >> + if (i915_vm_is_4lvl(ce->vm))
> >> + desc = INTEL_LEGACY_64B_CONTEXT;
> >
> > if-else now that the vm null check is gone.
> >
> >> + desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
> >> +
> >> + desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
> >> + if (IS_GEN(engine->i915, 8))
> >> + desc |= GEN8_CTX_L3LLC_COHERENT;
> >
> > Don't know.. it's nicer to keep it stored it both for Gen and context
> > state. What's the problem with it?
>
> Ping.
There's no gem_context.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
On 24/07/2019 10:20, Tvrtko Ursulin wrote:
On 23/07/2019 19:38, Chris Wilson wrote:
We only compute the lrc_descriptor() on pinning the context, i.e.
infrequently, so we do not benefit from storing the template as the
addressing mode is also fixed for the lifetime of the intel_context.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +---
drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
4 files changed, 10 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index b28c7ca681a8..1b3dc7258ef2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context
*ctx)
i915_gem_context_put(ctx);
}
-static u32 default_desc_template(const struct drm_i915_private *i915,
- const struct i915_address_space *vm)
-{
- u32 address_mode;
- u32 desc;
-
- desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
-
- address_mode = INTEL_LEGACY_32B_CONTEXT;
- if (vm && i915_vm_is_4lvl(vm))
- address_mode = INTEL_LEGACY_64B_CONTEXT;
- desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
-
- if (IS_GEN(i915, 8))
- desc |= GEN8_CTX_L3LLC_COHERENT;
-
- /* TODO: WaDisableLiteRestore when we start using semaphore
- * signalling between Command Streamers
- * ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
- */
-
- return desc;
-}
-
static struct i915_gem_context *
__create_context(struct drm_i915_private *i915)
{
@@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
i915_gem_context_set_recoverable(ctx);
ctx->ring_size = 4 * PAGE_SIZE;
- ctx->desc_template = default_desc_template(i915, NULL);
for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
@@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct
i915_address_space *vm)
struct i915_gem_engines_iter it;
struct intel_context *ce;
+ GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
+
ctx->vm = i915_vm_get(vm);
- ctx->desc_template = default_desc_template(ctx->i915, vm);
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
i915_vm_put(ce->vm);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 0ee61482ef94..a02d98494078 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -171,8 +171,6 @@ struct i915_gem_context {
/** ring_size: size for allocating the per-engine ring buffer */
u32 ring_size;
- /** desc_template: invariant fields for the HW context descriptor */
- u32 desc_template;
/** guilty_count: How many times this context has caused a GPU
hang. */
atomic_t guilty_count;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 632344c163a8..5fdac40015cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -418,13 +418,17 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID >
(BIT(GEN11_SW_CTX_ID_WIDTH)));
- desc = ctx->desc_template; /* bits 0-11 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
+ desc = INTEL_LEGACY_32B_CONTEXT;
+ if (i915_vm_is_4lvl(ce->vm))
+ desc = INTEL_LEGACY_64B_CONTEXT;
if-else now that the vm null check is gone.
+ desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
+
+ desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
+ if (IS_GEN(engine->i915, 8))
+ desc |= GEN8_CTX_L3LLC_COHERENT;
Don't know.. it's nicer to keep it stored it both for Gen and context
state. What's the problem with it?
Ping.
Regards,
Tvrtko
desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
/* bits 12-31 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 32));
-
/*
* The following 32bits are copied into the OA reports (dword 2).
* Consider updating oa_get_render_ctx_id in i915_perf.c when
changing
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index f68798ab1e7c..4c018fb1359c 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -291,9 +291,6 @@ shadow_context_descriptor_update(struct
intel_context *ce,
* Update bits 0-11 of the context descriptor which includes flags
* like GEN8_CTX_* cached in desc_template
*/
- desc &= U64_MAX << 12;
- desc |=
Re: [Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
On 23/07/2019 19:38, Chris Wilson wrote:
We only compute the lrc_descriptor() on pinning the context, i.e.
infrequently, so we do not benefit from storing the template as the
addressing mode is also fixed for the lifetime of the intel_context.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +---
drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
4 files changed, 10 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index b28c7ca681a8..1b3dc7258ef2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context *ctx)
i915_gem_context_put(ctx);
}
-static u32 default_desc_template(const struct drm_i915_private *i915,
-const struct i915_address_space *vm)
-{
- u32 address_mode;
- u32 desc;
-
- desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
-
- address_mode = INTEL_LEGACY_32B_CONTEXT;
- if (vm && i915_vm_is_4lvl(vm))
- address_mode = INTEL_LEGACY_64B_CONTEXT;
- desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
-
- if (IS_GEN(i915, 8))
- desc |= GEN8_CTX_L3LLC_COHERENT;
-
- /* TODO: WaDisableLiteRestore when we start using semaphore
-* signalling between Command Streamers
-* ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
-*/
-
- return desc;
-}
-
static struct i915_gem_context *
__create_context(struct drm_i915_private *i915)
{
@@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
i915_gem_context_set_recoverable(ctx);
ctx->ring_size = 4 * PAGE_SIZE;
- ctx->desc_template = default_desc_template(i915, NULL);
for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
@@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct
i915_address_space *vm)
struct i915_gem_engines_iter it;
struct intel_context *ce;
+ GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
+
ctx->vm = i915_vm_get(vm);
- ctx->desc_template = default_desc_template(ctx->i915, vm);
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
i915_vm_put(ce->vm);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 0ee61482ef94..a02d98494078 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -171,8 +171,6 @@ struct i915_gem_context {
/** ring_size: size for allocating the per-engine ring buffer */
u32 ring_size;
- /** desc_template: invariant fields for the HW context descriptor */
- u32 desc_template;
/** guilty_count: How many times this context has caused a GPU hang. */
atomic_t guilty_count;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 632344c163a8..5fdac40015cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -418,13 +418,17 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > (BIT(GEN11_SW_CTX_ID_WIDTH)));
- desc = ctx->desc_template;/* bits 0-11 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
+ desc = INTEL_LEGACY_32B_CONTEXT;
+ if (i915_vm_is_4lvl(ce->vm))
+ desc = INTEL_LEGACY_64B_CONTEXT;
if-else now that the vm null check is gone.
+ desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
+
+ desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
+ if (IS_GEN(engine->i915, 8))
+ desc |= GEN8_CTX_L3LLC_COHERENT;
Don't know.. it's nicer to keep it stored it both for Gen and context
state. What's the problem with it?
Regards,
Tvrtko
desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
/* bits 12-31 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 32));
-
/*
* The following 32bits are copied into the OA reports (dword 2).
* Consider updating oa_get_render_ctx_id in i915_perf.c when changing
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index f68798ab1e7c..4c018fb1359c 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -291,9 +291,6 @@ shadow_context_descriptor_update(struct intel_context *ce,
* Update bits 0-11 of the context descriptor which
[Intel-gfx] [PATCH 03/23] drm/i915: Remove lrc default desc from GEM context
We only compute the lrc_descriptor() on pinning the context, i.e.
infrequently, so we do not benefit from storing the template as the
addressing mode is also fixed for the lifetime of the intel_context.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-
.../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +---
drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
4 files changed, 10 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index b28c7ca681a8..1b3dc7258ef2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context *ctx)
i915_gem_context_put(ctx);
}
-static u32 default_desc_template(const struct drm_i915_private *i915,
-const struct i915_address_space *vm)
-{
- u32 address_mode;
- u32 desc;
-
- desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
-
- address_mode = INTEL_LEGACY_32B_CONTEXT;
- if (vm && i915_vm_is_4lvl(vm))
- address_mode = INTEL_LEGACY_64B_CONTEXT;
- desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
-
- if (IS_GEN(i915, 8))
- desc |= GEN8_CTX_L3LLC_COHERENT;
-
- /* TODO: WaDisableLiteRestore when we start using semaphore
-* signalling between Command Streamers
-* ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
-*/
-
- return desc;
-}
-
static struct i915_gem_context *
__create_context(struct drm_i915_private *i915)
{
@@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
i915_gem_context_set_recoverable(ctx);
ctx->ring_size = 4 * PAGE_SIZE;
- ctx->desc_template = default_desc_template(i915, NULL);
for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
@@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct
i915_address_space *vm)
struct i915_gem_engines_iter it;
struct intel_context *ce;
+ GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
+
ctx->vm = i915_vm_get(vm);
- ctx->desc_template = default_desc_template(ctx->i915, vm);
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
i915_vm_put(ce->vm);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 0ee61482ef94..a02d98494078 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -171,8 +171,6 @@ struct i915_gem_context {
/** ring_size: size for allocating the per-engine ring buffer */
u32 ring_size;
- /** desc_template: invariant fields for the HW context descriptor */
- u32 desc_template;
/** guilty_count: How many times this context has caused a GPU hang. */
atomic_t guilty_count;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 632344c163a8..5fdac40015cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -418,13 +418,17 @@ lrc_descriptor(struct intel_context *ce, struct
intel_engine_cs *engine)
BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > (BIT(GEN11_SW_CTX_ID_WIDTH)));
- desc = ctx->desc_template; /* bits 0-11 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
+ desc = INTEL_LEGACY_32B_CONTEXT;
+ if (i915_vm_is_4lvl(ce->vm))
+ desc = INTEL_LEGACY_64B_CONTEXT;
+ desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
+
+ desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
+ if (IS_GEN(engine->i915, 8))
+ desc |= GEN8_CTX_L3LLC_COHERENT;
desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
/* bits 12-31 */
- GEM_BUG_ON(desc & GENMASK_ULL(63, 32));
-
/*
* The following 32bits are copied into the OA reports (dword 2).
* Consider updating oa_get_render_ctx_id in i915_perf.c when changing
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index f68798ab1e7c..4c018fb1359c 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -291,9 +291,6 @@ shadow_context_descriptor_update(struct intel_context *ce,
* Update bits 0-11 of the context descriptor which includes flags
* like GEN8_CTX_* cached in desc_template
*/
- desc &= U64_MAX << 12;
- desc |= ce->gem_context->desc_template & ((1ULL << 12) - 1);
-
