| Message ID | 1537521230-22904-4-git-send-email-kedar.j.karanje@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Dynamic EU configuration of Slice/Subslice/EU. | expand |
On 21/09/2018 10:13, kedar.j.karanje@intel.com wrote: > From: Praveen Diwakar <praveen.diwakar@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@intel.com> > Signed-off-by: Yogesh Marathe <yogesh.marathe@intel.com> > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar@intel.com> > Signed-off-by: Kedar J Karanje <kedar.j.karanje@intel.com> > Signed-off-by: Ankit Navik <ankit.p.navik@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); >
Hi Praveen,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.19-rc4 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/kedar-j-karanje-intel-com/drm-i915-Get-active-pending-request-for-given-context/20180923-012250
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x014-201838 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/i915_gem_context.c: In function 'i915_set_optimum_config':
>> drivers/gpu/drm/i915/i915_gem_context.c:434:6: error: unused variable 'rpcs_config' [-Werror=unused-variable]
u32 rpcs_config = 0;
^~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_gem_context.c:433:7: error: unused variable 'reg_state' [-Werror=unused-variable]
u32 *reg_state = ce->lrc_reg_state;
^~~~~~~~~
cc1: all warnings being treated as errors
vim +/rpcs_config +434 drivers/gpu/drm/i915/i915_gem_context.c
427
428
429 void i915_set_optimum_config(int type, struct i915_gem_context *ctx,
430 enum gem_tier_versions version)
431 {
432 struct intel_context *ce = &ctx->__engine[RCS];
> 433 u32 *reg_state = ce->lrc_reg_state;
> 434 u32 rpcs_config = 0;
435 /* Call opt_config to get correct configuration for eu,slice,subslice */
436 ctx->slice_cnt = (u8)opt_config[version][type].slice;
437 ctx->subslice_cnt = (u8)opt_config[version][type].subslice;
438 ctx->eu_cnt = (u8)opt_config[version][type].eu;
439
440 /* Enabling this to update the rpcs */
441 if (ctx->prev_load_type != type)
442 ctx->update_render_config = 1;
443
444 ctx->prev_load_type = type;
445 }
446 /**
447 * i915_gem_context_create_gvt - create a GVT GEM context
448 * @dev: drm device *
449 *
450 * This function is used to create a GVT specific GEM context.
451 *
452 * Returns:
453 * pointer to i915_gem_context on success, error pointer if failed
454 *
455 */
456 struct i915_gem_context *
457 i915_gem_context_create_gvt(struct drm_device *dev)
458 {
459 struct i915_gem_context *ctx;
460 int ret;
461
462 if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
463 return ERR_PTR(-ENODEV);
464
465 ret = i915_mutex_lock_interruptible(dev);
466 if (ret)
467 return ERR_PTR(ret);
468
469 ctx = __create_hw_context(to_i915(dev), NULL);
470 if (IS_ERR(ctx))
471 goto out;
472
473 ctx->file_priv = ERR_PTR(-EBADF);
474 i915_gem_context_set_closed(ctx); /* not user accessible */
475 i915_gem_context_clear_bannable(ctx);
476 i915_gem_context_set_force_single_submission(ctx);
477 if (!USES_GUC_SUBMISSION(to_i915(dev)))
478 ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
479
480 GEM_BUG_ON(i915_gem_context_is_kernel(ctx));
481 out:
482 mutex_unlock(&dev->struct_mutex);
483 return ctx;
484 }
485
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Tvrtko, Your review changes are incorporated in v2. Regards, Ankit > -----Original Message----- > From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] > Sent: Friday, September 21, 2018 6:36 PM > To: J Karanje, Kedar <kedar.j.karanje@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Diwakar, Praveen <praveen.diwakar@intel.com>; Marathe, Yogesh > <yogesh.marathe@intel.com>; Navik, Ankit P <ankit.p.navik@intel.com>; > Muthukumar, Aravindan <aravindan.muthukumar@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@intel.com wrote: > > From: Praveen Diwakar <praveen.diwakar@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@intel.com> > > Signed-off-by: Yogesh Marathe <yogesh.marathe@intel.com> > > Signed-off-by: Aravindan Muthukumar <aravindan.muthukumar@intel.com> > > Signed-off-by: Kedar J Karanje <kedar.j.karanje@intel.com> > > Signed-off-by: Ankit Navik <ankit.p.navik@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); > >
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, + enum gem_tier_versions version) +{ + struct intel_context *ce = &ctx->__engine[RCS]; + u32 *reg_state = ce->lrc_reg_state; + u32 rpcs_config = 0; + /* 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; + + /* Enabling this to update the rpcs */ + if (ctx->prev_load_type != type) + ctx->update_render_config = 1; + + 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; +}; + /** * 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; + + /** prev_load_type: The earlier load type that the GPU was configured + * for (high/medium/low). + */ + u8 prev_load_type; + /** 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);