On 21/09/2018 10:13, [email protected] wrote:
From: Praveen Diwakar <[email protected]>

This patch will update power clock state register at runtime base on the
flag update_render_config which can set by any governor which computes load
and want to update rpcs register.
subsequent patches will have a timer based governor which computes pending
load/request.

Change-Id: I4e7d2f484b957d5bd496e1decc59a69e3bc6d186
Signed-off-by: Praveen Diwakar <[email protected]>
Signed-off-by: Yogesh Marathe <[email protected]>
Signed-off-by: Aravindan Muthukumar <[email protected]>
Signed-off-by: Kedar J Karanje <[email protected]>
Signed-off-by: Ankit Navik <[email protected]>
---
  drivers/gpu/drm/i915/i915_gem_context.c |  5 ++++
  drivers/gpu/drm/i915/i915_gem_context.h | 14 +++++++++++
  drivers/gpu/drm/i915/intel_lrc.c        | 41 +++++++++++++++++++++++++++++++++
  3 files changed, 60 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 30932d9..2838c1d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -388,6 +388,11 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
trace_i915_context_create(ctx);
        ctx->req_cnt = 0;
+       ctx->update_render_config = 0;
+       ctx->slice_cnt = hweight8(INTEL_INFO(dev_priv)->sseu.slice_mask);
+       ctx->subslice_cnt = hweight8(
+                       INTEL_INFO(dev_priv)->sseu.subslice_mask[0]);
+       ctx->eu_cnt = INTEL_INFO(dev_priv)->sseu.eu_per_subslice;
return ctx;
  }
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
b/drivers/gpu/drm/i915/i915_gem_context.h
index 243ea22..52e341c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -200,6 +200,20 @@ struct i915_gem_context {
         * controlled via a mutex
         */
        u64 req_cnt;
+
+       /** slice_cnt: used to set the # of slices to be enabled. */
+       u8 slice_cnt;
+
+       /** subslice_cnt: used to set the # of subslices to be enabled. */
+       u8 subslice_cnt;
+
+       /** eu_cnt: used to set the # of eu to be enabled. */
+       u8 eu_cnt;
+
+       /** update_render_config: to track the updates to the render
+        * configuration (S/SS/EU Configuration on the GPU)
+        */
+       bool update_render_config;
  };
static inline bool i915_gem_context_is_closed(const struct i915_gem_context *ctx)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 039fbdb..d2d0e7d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -364,6 +364,36 @@ execlists_unwind_incomplete_requests(struct 
intel_engine_execlists *execlists)
        spin_unlock_irqrestore(&engine->timeline.lock, flags);
  }
+static u32
+get_context_rpcs_config(struct i915_gem_context *ctx)
+{
+       struct drm_i915_private *dev_priv = ctx->i915;
+       u32 rpcs = 0;
+
+       if (INTEL_GEN(dev_priv) < 8)
+               return 0;
+
+       if (INTEL_INFO(dev_priv)->sseu.has_slice_pg) {
+               rpcs |= GEN8_RPCS_S_CNT_ENABLE;
+               rpcs |= ctx->slice_cnt << GEN8_RPCS_S_CNT_SHIFT;
+               rpcs |= GEN8_RPCS_ENABLE;
+       }
+
+       if (INTEL_INFO(dev_priv)->sseu.has_subslice_pg) {
+               rpcs |= GEN8_RPCS_SS_CNT_ENABLE;
+               rpcs |= ctx->subslice_cnt << GEN8_RPCS_SS_CNT_SHIFT;
+               rpcs |= GEN8_RPCS_ENABLE;
+       }
+
+       if (INTEL_INFO(dev_priv)->sseu.has_eu_pg) {
+               rpcs |= ctx->eu_cnt << GEN8_RPCS_EU_MIN_SHIFT;
+               rpcs |= ctx->eu_cnt << GEN8_RPCS_EU_MAX_SHIFT;
+               rpcs |= GEN8_RPCS_ENABLE;
+       }
+
+       return rpcs;
+}

This function is very similar to make_rpcs so I'd suggest to extract some commonality and share the code.

In any case you are likely to get overtaken by https://patchwork.freedesktop.org/series/48194/ after which you will be able to use struct intel_sseu to package the data you need in one ctx member, etc.

+
  static inline void
  execlists_context_status_change(struct i915_request *rq, unsigned long status)
  {
@@ -418,11 +448,22 @@ execlists_update_context_pdps(struct i915_hw_ppgtt 
*ppgtt, u32 *reg_state)
  static u64 execlists_update_context(struct i915_request *rq)
  {
        struct intel_context *ce = rq->hw_context;
+       struct i915_gem_context *ctx = rq->gem_context;
+       struct intel_engine_cs *engine = rq->engine;
        struct i915_hw_ppgtt *ppgtt =
                rq->gem_context->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
        u32 *reg_state = ce->lrc_reg_state;
+       u32 rpcs_config = 0;
reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
+       if (ctx->pid && ctx->name && (rq->engine->id == RCS) &&
+                       ctx->update_render_config) {
+               rpcs_config = get_context_rpcs_config(ctx);
+               reg_state[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
+               CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
+                               rpcs_config);

For a first submission this works, but when appending to a running context the RPCS value you write here will probably get overwritten by context save. At least I would be surprised if lite-restore would write to the register.

So if I am correct the problem you have is that your configuration changes will not always become live when you expect it to. And then if you hit lite-restore you don't try to re-configure it later unless there is a config change.

But I don't think there is a cheap/reasonable way to fix this fully. So maybe you just give up on this chunk and rely on context image to be updated on context pin like in https://patchwork.freedesktop.org/patch/250007/.

Regards,

Tvrtko

+               ctx->update_render_config = 0;
+       }
/* True 32b PPGTT with dynamic page allocation: update PDP
         * registers and point the unallocated PDPs to scratch page.

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to