| Message ID | 20210820224446.30620-4-matthew.brost@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Parallel submission aka multi-bb execbuf | expand |
On 8/20/2021 15:44, Matthew Brost wrote: > For testing purposes it may make sense to reduce the number of guc_ids > available to be allocated. Add debugfs support for setting the number of > guc_ids. > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> It 'may make sense'? Is there an actual IGT/selftest test that uses this and needs to in order to not take several hours to run or something? Seems wrong to be adding a debugfs interface for something that would only be used for testing by internal developers who can quite easy just edit a define in the code to simplify their testing. John. > --- > .../gpu/drm/i915/gt/uc/intel_guc_debugfs.c | 31 +++++++++++++++++++ > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 +- > 2 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c > index 887c8c8f35db..b88d343ee432 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c > @@ -71,12 +71,43 @@ static bool intel_eval_slpc_support(void *data) > return intel_guc_slpc_is_used(guc); > } > > +static int guc_num_id_get(void *data, u64 *val) > +{ > + struct intel_guc *guc = data; > + > + if (!intel_guc_submission_is_used(guc)) > + return -ENODEV; > + > + *val = guc->num_guc_ids; > + > + return 0; > +} > + > +static int guc_num_id_set(void *data, u64 val) > +{ > + struct intel_guc *guc = data; > + > + if (!intel_guc_submission_is_used(guc)) > + return -ENODEV; > + > + if (val > guc->max_guc_ids) > + val = guc->max_guc_ids; > + else if (val < 256) > + val = 256; > + > + guc->num_guc_ids = val; > + > + return 0; > +} > +DEFINE_SIMPLE_ATTRIBUTE(guc_num_id_fops, guc_num_id_get, guc_num_id_set, "%lld\n"); > + > void intel_guc_debugfs_register(struct intel_guc *guc, struct dentry *root) > { > static const struct debugfs_gt_file files[] = { > { "guc_info", &guc_info_fops, NULL }, > { "guc_registered_contexts", &guc_registered_contexts_fops, NULL }, > { "guc_slpc_info", &guc_slpc_info_fops, &intel_eval_slpc_support}, > + { "guc_num_id", &guc_num_id_fops, NULL }, > }; > > if (!intel_guc_is_supported(guc)) > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 8235e49bb347..68742b612692 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -2716,7 +2716,8 @@ g2h_context_lookup(struct intel_guc *guc, u32 desc_idx) > > if (unlikely(desc_idx >= guc->max_guc_ids)) { > drm_err(&guc_to_gt(guc)->i915->drm, > - "Invalid desc_idx %u", desc_idx); > + "Invalid desc_idx %u, max %u", > + desc_idx, guc->max_guc_ids); > return NULL; > } >
On Thu, Sep 09, 2021 at 03:16:22PM -0700, John Harrison wrote: > On 8/20/2021 15:44, Matthew Brost wrote: > > For testing purposes it may make sense to reduce the number of guc_ids > > available to be allocated. Add debugfs support for setting the number of > > guc_ids. > > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > It 'may make sense'? Is there an actual IGT/selftest test that uses this and > needs to in order to not take several hours to run or something? Seems wrong > to be adding a debugfs interface for something that would only be used for > testing by internal developers who can quite easy just edit a define in the > code to simplify their testing. > Had an IGT (gem_exec_parallel) to toggle this to hit certain conditions that are very hard to hit (guc_id exhaustion) but this was we punted this problem to the user. Now we likely don't need that test and it is probably ok to just drop this patch. Matt > John. > > > > --- > > .../gpu/drm/i915/gt/uc/intel_guc_debugfs.c | 31 +++++++++++++++++++ > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 +- > > 2 files changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c > > index 887c8c8f35db..b88d343ee432 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c > > @@ -71,12 +71,43 @@ static bool intel_eval_slpc_support(void *data) > > return intel_guc_slpc_is_used(guc); > > } > > +static int guc_num_id_get(void *data, u64 *val) > > +{ > > + struct intel_guc *guc = data; > > + > > + if (!intel_guc_submission_is_used(guc)) > > + return -ENODEV; > > + > > + *val = guc->num_guc_ids; > > + > > + return 0; > > +} > > + > > +static int guc_num_id_set(void *data, u64 val) > > +{ > > + struct intel_guc *guc = data; > > + > > + if (!intel_guc_submission_is_used(guc)) > > + return -ENODEV; > > + > > + if (val > guc->max_guc_ids) > > + val = guc->max_guc_ids; > > + else if (val < 256) > > + val = 256; > > + > > + guc->num_guc_ids = val; > > + > > + return 0; > > +} > > +DEFINE_SIMPLE_ATTRIBUTE(guc_num_id_fops, guc_num_id_get, guc_num_id_set, "%lld\n"); > > + > > void intel_guc_debugfs_register(struct intel_guc *guc, struct dentry *root) > > { > > static const struct debugfs_gt_file files[] = { > > { "guc_info", &guc_info_fops, NULL }, > > { "guc_registered_contexts", &guc_registered_contexts_fops, NULL }, > > { "guc_slpc_info", &guc_slpc_info_fops, &intel_eval_slpc_support}, > > + { "guc_num_id", &guc_num_id_fops, NULL }, > > }; > > if (!intel_guc_is_supported(guc)) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > index 8235e49bb347..68742b612692 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -2716,7 +2716,8 @@ g2h_context_lookup(struct intel_guc *guc, u32 desc_idx) > > if (unlikely(desc_idx >= guc->max_guc_ids)) { > > drm_err(&guc_to_gt(guc)->i915->drm, > > - "Invalid desc_idx %u", desc_idx); > > + "Invalid desc_idx %u, max %u", > > + desc_idx, guc->max_guc_ids); > > return NULL; > > } >
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c index 887c8c8f35db..b88d343ee432 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c @@ -71,12 +71,43 @@ static bool intel_eval_slpc_support(void *data) return intel_guc_slpc_is_used(guc); } +static int guc_num_id_get(void *data, u64 *val) +{ + struct intel_guc *guc = data; + + if (!intel_guc_submission_is_used(guc)) + return -ENODEV; + + *val = guc->num_guc_ids; + + return 0; +} + +static int guc_num_id_set(void *data, u64 val) +{ + struct intel_guc *guc = data; + + if (!intel_guc_submission_is_used(guc)) + return -ENODEV; + + if (val > guc->max_guc_ids) + val = guc->max_guc_ids; + else if (val < 256) + val = 256; + + guc->num_guc_ids = val; + + return 0; +} +DEFINE_SIMPLE_ATTRIBUTE(guc_num_id_fops, guc_num_id_get, guc_num_id_set, "%lld\n"); + void intel_guc_debugfs_register(struct intel_guc *guc, struct dentry *root) { static const struct debugfs_gt_file files[] = { { "guc_info", &guc_info_fops, NULL }, { "guc_registered_contexts", &guc_registered_contexts_fops, NULL }, { "guc_slpc_info", &guc_slpc_info_fops, &intel_eval_slpc_support}, + { "guc_num_id", &guc_num_id_fops, NULL }, }; if (!intel_guc_is_supported(guc)) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 8235e49bb347..68742b612692 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -2716,7 +2716,8 @@ g2h_context_lookup(struct intel_guc *guc, u32 desc_idx) if (unlikely(desc_idx >= guc->max_guc_ids)) { drm_err(&guc_to_gt(guc)->i915->drm, - "Invalid desc_idx %u", desc_idx); + "Invalid desc_idx %u, max %u", + desc_idx, guc->max_guc_ids); return NULL; }
For testing purposes it may make sense to reduce the number of guc_ids available to be allocated. Add debugfs support for setting the number of guc_ids. Signed-off-by: Matthew Brost <matthew.brost@intel.com> --- .../gpu/drm/i915/gt/uc/intel_guc_debugfs.c | 31 +++++++++++++++++++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 +- 2 files changed, 33 insertions(+), 1 deletion(-)