[Intel-gfx] [PATCH 3/4] drm/i915: set optimum eu/slice/sub-slice configuration based on load type
Navik, Ankit P
ankit.p.navik at intel.com
Tue Nov 6 04:25:17 UTC 2018
Hi Tvrtko,
Your review changes are incorporated in v2.
Regards, Ankit
> -----Original Message-----
> From: Tvrtko Ursulin [mailto:tvrtko.ursulin at linux.intel.com]
> Sent: Friday, September 21, 2018 6:36 PM
> To: J Karanje, Kedar <kedar.j.karanje at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Diwakar, Praveen <praveen.diwakar at intel.com>; Marathe, Yogesh
> <yogesh.marathe at intel.com>; Navik, Ankit P <ankit.p.navik at intel.com>;
> Muthukumar, Aravindan <aravindan.muthukumar at 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.karanje at intel.com wrote:
> > From: Praveen Diwakar <praveen.diwakar at 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.diwakar at intel.com>
> > Signed-off-by: Yogesh Marathe <yogesh.marathe at intel.com>
> > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar at intel.com>
> > Signed-off-by: Kedar J Karanje <kedar.j.karanje at intel.com>
> > Signed-off-by: Ankit Navik <ankit.p.navik at 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);
> >
More information about the Intel-gfx
mailing list