Hi Tvrtko,
On Tue, Dec 11, 2018 at 6:06 PM Tvrtko Ursulin <
tvrtko.ursu...@linux.intel.com> wrote:
>
> On 11/12/2018 10:14, Ankit Navik wrote:
> > From: Praveen Diwakar <praveen.diwa...@intel.com>
> >
> > 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 <aravindan.muthuku...@intel.com>
> > Cc: Kedar J Karanje <kedar.j.kara...@intel.com>
> > Cc: Yogesh Marathe <yogesh.mara...@intel.com>
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
>
> Again, I did not give an r-b for this!
>
> > Signed-off-by: Praveen Diwakar <praveen.diwa...@intel.com>
> > Signed-off-by: Ankit Navik <ankit.p.na...@intel.com>
> > ---
> > 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 https://patchwork.freedesktop.org/patch/261560/ 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.
>
submitted v4 patch and reverted to previous function
i.e., get_context_rpcs_config as make_rpcs is changed in mainline.
Regards, Ankit
>
> > + 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
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
