<div dir="ltr"><div dir="ltr">Hi Tvrtko,</div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Dec 11, 2018 at 6:06 PM Tvrtko Ursulin <<a href="mailto:tvrtko.ursulin@linux.intel.com">tvrtko.ursulin@linux.intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> On 11/12/2018 10:14, Ankit Navik wrote: > From: Praveen Diwakar <<a href="mailto:praveen.diwakar@intel.com" target="_blank">praveen.diwakar@intel.com</a>> > > This patch will update power clock state register at runtime base on the > flag 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. > > V2: > * Reuse make_rpcs to get rpcs config. (Tvrtko Ursulin) > > V3: > * Rebase. > > Cc: Aravindan Muthukumar <<a href="mailto:aravindan.muthukumar@intel.com" target="_blank">aravindan.muthukumar@intel.com</a>> > Cc: Kedar J Karanje <<a href="mailto:kedar.j.karanje@intel.com" target="_blank">kedar.j.karanje@intel.com</a>> > Cc: Yogesh Marathe <<a href="mailto:yogesh.marathe@intel.com" target="_blank">yogesh.marathe@intel.com</a>> > Reviewed-by: Tvrtko Ursulin <<a href="mailto:tvrtko.ursulin@linux.intel.com" target="_blank">tvrtko.ursulin@linux.intel.com</a>> Again, I did not give an r-b for this! > Signed-off-by: Praveen Diwakar <<a href="mailto:praveen.diwakar@intel.com" target="_blank">praveen.diwakar@intel.com</a>> > Signed-off-by: Ankit Navik <<a href="mailto:ankit.p.navik@intel.com" target="_blank">ankit.p.navik@intel.com</a>> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 4 ++++ > drivers/gpu/drm/i915/i915_gem_context.h | 9 +++++++++ > drivers/gpu/drm/i915/intel_lrc.c | 12 +++++++++++- > 3 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 0bcbe32..d040858 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -388,6 +388,10 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, > > trace_i915_context_create(ctx); > atomic_set(&ctx->req_cnt, 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 e824b15..e000530 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.h > +++ b/drivers/gpu/drm/i915/i915_gem_context.h > @@ -199,6 +199,15 @@ struct i915_gem_context { > * go for low/medium/high load configuration of the GPU. > */ > atomic_t 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; > }; > > 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 d33f5ac..a17f676 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -171,6 +171,7 @@ static void execlists_init_reg_state(u32 *reg_state, > struct i915_gem_context *ctx, > struct intel_engine_cs *engine, > struct intel_ring *ring); > +static u32 make_rpcs(struct drm_i915_private *dev_priv); > > static inline struct i915_priolist *to_priolist(struct rb_node *rb) > { > @@ -417,12 +418,21 @@ execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state) > > static u64 execlists_update_context(struct i915_request *rq) > { > + u32 rpcs_config = 0; Move to if block where it is used. > struct intel_context *ce = rq->hw_context; > + u32 *reg_state = ce->lrc_reg_state; No need to touch this. > + struct i915_gem_context *ctx = rq->gem_context; Can go under the if block as well. > struct i915_hw_ppgtt *ppgtt = > rq->gem_context->ppgtt ?: rq->i915->mm.aliasing_ppgtt; > - u32 *reg_state = ce->lrc_reg_state; > > reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail); > + /* FIXME: To avoid stale rpcs config, move it to context_pin */ This FIXME remains unresolved by the whole series, so that really cannot be like that. I suggest you add <a href="https://patchwork.freedesktop.org/patch/261560/" rel="noreferrer" target="_blank">https://patchwork.freedesktop.org/patch/261560/</a> to your series and rebase this on top. Which would actually make this patch not do very much and you should probably squash it with the one which actually uses the added fields. </blockquote><div> </div><div>submitted v4 patch and reverted to previous function</div><div>i.e., get_context_rpcs_config as make_rpcs is changed in mainline. </div><div> </div><div>Regards, Ankit </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > + if (ctx->pid && ctx->name && (rq->engine->id == RCS)) { Why name and pid? You are using them as proxy for something.. but for what and why? The answer may hold a hint to the proper solution. > + rpcs_config = make_rpcs(ctx->i915); > + 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); > + } > > /* True 32b PPGTT with dynamic page allocation: update PDP > * registers and point the unallocated PDPs to scratch page. > Regards, Tvrtko _______________________________________________ Intel-gfx mailing list <a href="mailto:Intel-gfx@lists.freedesktop.org" target="_blank">Intel-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/intel-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</a> </blockquote></div></div>