Hi Tvrtko, Your review changes are incorporated in v2. Regards, Ankit > -----Original Message----- > From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] > Sent: Friday, September 21, 2018 6:36 PM > To: J Karanje, Kedar <kedar.j.kara...@intel.com>; intel- > g...@lists.freedesktop.org > Cc: Diwakar, Praveen <praveen.diwa...@intel.com>; Marathe, Yogesh > <yogesh.mara...@intel.com>; Navik, Ankit P <ankit.p.na...@intel.com>; > Muthukumar, Aravindan <aravindan.muthuku...@intel.com> > Subject: Re: [Intel-gfx] [PATCH 3/4] drm/i915: set optimum eu/slice/sub-slice > configuration based on load type > > > On 21/09/2018 10:13, kedar.j.kara...@intel.com wrote: > > From: Praveen Diwakar <praveen.diwa...@intel.com> > > > > This patch will select optimum eu/slice/sub-slice configuration based > > on type of load (low, medium, high) as input. > > Based on our readings and experiments we have predefined set of > > optimum configuration for each platform(CHT, KBL). > > i915_set_optimum_config will select optimum configuration from > > pre-defined optimum configuration table(opt_config). > > > > Change-Id: I3a6a2a6bdddd01b3d3c97995f5403aef3c6fa989 > > Signed-off-by: Praveen Diwakar <praveen.diwa...@intel.com> > > Signed-off-by: Yogesh Marathe <yogesh.mara...@intel.com> > > Signed-off-by: Aravindan Muthukumar <aravindan.muthuku...@intel.com> > > Signed-off-by: Kedar J Karanje <kedar.j.kara...@intel.com> > > Signed-off-by: Ankit Navik <ankit.p.na...@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem_context.c | 46 > +++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/i915_gem_context.h | 32 +++++++++++++++++++++++ > > 2 files changed, 78 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > > b/drivers/gpu/drm/i915/i915_gem_context.c > > index 2838c1d..1b76410 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -94,6 +94,32 @@ > > > > #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1 > > > > +static struct optimum_config > opt_config[TIER_VERSION_MAX][LOAD_TYPE_MAX] = { > > + { > > + /* Cherry trail low */ > > + { 1, 1, 4}, > > + /* Cherry trail medium */ > > + { 1, 1, 6}, > > + /* Cherry trail high */ > > + { 1, 2, 6} > > + }, > > + { > > + /* kbl gt2 low */ > > + { 2, 3, 4}, > > + /* kbl gt2 medium */ > > + { 2, 3, 6}, > > + /* kbl gt2 high */ > > + { 2, 3, 8} > > + }, > > + { > > + /* kbl gt3 low */ > > + { 2, 3, 4}, > > + /* kbl gt3 medium */ > > + { 2, 3, 6}, > > + /* kbl gt3 high */ > > + { 2, 3, 8} > > + } > > +}; > > static void lut_close(struct i915_gem_context *ctx) > > { > > struct i915_lut_handle *lut, *ln; > > @@ -393,10 +419,30 @@ i915_gem_create_context(struct drm_i915_private > *dev_priv, > > ctx->subslice_cnt = hweight8( > > INTEL_INFO(dev_priv)->sseu.subslice_mask[0]); > > ctx->eu_cnt = INTEL_INFO(dev_priv)->sseu.eu_per_subslice; > > + ctx->load_type = 0; > > + ctx->prev_load_type = 0; > > > > return ctx; > > } > > > > + > > +void i915_set_optimum_config(int type, struct i915_gem_context *ctx, > > Type for type should be enum. > > Name the function as i915_gem_context_set_load_type(ctx, type). > > > + enum gem_tier_versions version) > > +{ > > + struct intel_context *ce = &ctx->__engine[RCS]; > > + u32 *reg_state = ce->lrc_reg_state; > > + u32 rpcs_config = 0; > > Unused locals? > > > + /* Call opt_config to get correct configuration for eu,slice,subslice */ > > + ctx->slice_cnt = (u8)opt_config[version][type].slice; > > + ctx->subslice_cnt = (u8)opt_config[version][type].subslice; > > + ctx->eu_cnt = (u8)opt_config[version][type].eu; > > You could store the correct table for the device in dev_priv and use the > static > table to assign/populate on device init time. > > > + > > + /* Enabling this to update the rpcs */ > > + if (ctx->prev_load_type != type) > > + ctx->update_render_config = 1; > > ctx->update_render_config was unused in last patch. So patch ordering is > a bit suboptimal but I don't know. > > > + > > + ctx->prev_load_type = type; > > +} > > /** > > * i915_gem_context_create_gvt - create a GVT GEM context > > * @dev: drm device * > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h > b/drivers/gpu/drm/i915/i915_gem_context.h > > index 52e341c..50183e6 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.h > > +++ b/drivers/gpu/drm/i915/i915_gem_context.h > > @@ -53,6 +53,26 @@ struct intel_context_ops { > > void (*destroy)(struct intel_context *ce); > > }; > > > > +enum gem_load_type { > > + LOAD_TYPE_LOW, > > + LOAD_TYPE_MEDIUM, > > + LOAD_TYPE_HIGH, > > + LOAD_TYPE_MAX > > +}; > > + > > +enum gem_tier_versions { > > + CHERRYVIEW = 0, > > + KABYLAKE_GT2, > > + KABYLAKE_GT3, > > + TIER_VERSION_MAX > > +}; > > + > > +struct optimum_config { > > + int slice; > > + int subslice; > > + int eu; > > u8 would do global table definitely. > > > +}; > > + > > /** > > * struct i915_gem_context - client state > > * > > @@ -210,6 +230,16 @@ struct i915_gem_context { > > /** eu_cnt: used to set the # of eu to be enabled. */ > > u8 eu_cnt; > > > > + /** load_type: The designated load_type (high/medium/low) for a given > > + * number of pending commands in the command queue. > > + */ > > + u8 load_type; > > Unused? > > > + > > + /** prev_load_type: The earlier load type that the GPU was configured > > + * for (high/medium/low). > > + */ > > + u8 prev_load_type; > > Would pending_load_type sound more obvious? > > Types should be enums for both. > > Regards, > > Tvrtko > > > + > > /** update_render_config: to track the updates to the render > > * configuration (S/SS/EU Configuration on the GPU) > > */ > > @@ -342,6 +372,8 @@ int i915_gem_context_setparam_ioctl(struct > drm_device *dev, void *data, > > struct drm_file *file_priv); > > int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file); > > +void i915_set_optimum_config(int type, struct i915_gem_context *ctx, > > + enum gem_tier_versions version); > > > > struct i915_gem_context * > > i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio); > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
