diff mbox series

[03/27] drm/i915/guc: Connect the number of guc_ids to debugfs

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

Commit Message

Matthew Brost Aug. 20, 2021, 10:44 p.m. UTC
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(-)

Comments

John Harrison Sept. 9, 2021, 10:16 p.m. UTC | #1
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;
>   	}
>
Matthew Brost Sept. 10, 2021, 12:16 a.m. UTC | #2
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 mbox series

Patch

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;
 	}