Re: [Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
On 12/14/2021 09:05, Matthew Brost wrote: Testing the stealing of guc ids is hard from user space as we have 64k guc_ids. Add a selftest, which artificially reduces the number of guc ids, and forces a steal. The test creates a spinner which is used to block all subsequent submissions until it completes. Next, a loop creates a context and a NOP request each iteration until the guc_ids are exhausted (request creation returns -EAGAIN). The spinner is ended, unblocking all requests created in the loop. At this point all guc_ids are exhausted but are available to steal. Try to create another request which should successfully steal a guc_id. Wait on last request to complete, idle GPU, verify a guc_id was stolen via a counter, and exit the test. Test also artificially reduces the number of guc_ids so the test runs in a timely manner. v2: (John Harrison) - s/stole/stolen - Fix some wording in test description - Rework indexing into context array - Add test description to commit message - Fix typo in commit message (Checkpatch) - s/guc/(guc) in NUMBER_MULTI_LRC_GUC_ID v3: (John Harrison) - Set array value to NULL after extracting error - Fix a few typos in comments / error messages - Delete redundant comment in commit message Signed-off-by: Matthew Brost Reviewed-by: John Harrison --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 +- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 173 ++ 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1cb46098030d..f9240d4baa69 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -94,6 +94,11 @@ struct intel_guc { * @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @num_guc_ids: Number of guc_ids, selftest feature to be able +* to reduce this number while testing. +*/ + int num_guc_ids; /** * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc */ @@ -202,6 +207,13 @@ struct intel_guc { */ struct delayed_work work; } timestamp; + +#ifdef CONFIG_DRM_I915_SELFTEST + /** +* @number_guc_id_stolen: The number of guc_ids that have been stolen +*/ + int number_guc_id_stolen; +#endif }; static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) 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 96fcf869e3ff..99414b49ca6d 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -145,7 +145,8 @@ guc_create_parallel(struct intel_engine_cs **engines, * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for * multi-lrc. */ -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) +#define NUMBER_MULTI_LRC_GUC_ID(guc) \ + ((guc)->submission_state.num_guc_ids / 16) /* * Below is a set of functions which control the GuC scheduling state which @@ -1775,7 +1776,7 @@ int intel_guc_submission_init(struct intel_guc *guc) destroyed_worker_func); guc->submission_state.guc_ids_bitmap = - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) return -ENOMEM; @@ -1869,13 +1870,13 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) if (intel_context_is_parent(ce)) ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, - NUMBER_MULTI_LRC_GUC_ID, + NUMBER_MULTI_LRC_GUC_ID(guc), order_base_2(ce->parallel.number_children + 1)); else ret = ida_simple_get(&guc->submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID, -GUC_MAX_LRC_DESCRIPTORS, +NUMBER_MULTI_LRC_GUC_ID(guc), +guc->submission_state.num_guc_ids, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) @@ -1941,6 +1942,10 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce) set_context_guc_id_invalid(cn); +#ifdef CONFIG_DRM_I915_SELFTEST + guc->number_guc_id
[Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
Testing the stealing of guc ids is hard from user space as we have 64k guc_ids. Add a selftest, which artificially reduces the number of guc ids, and forces a steal. The test creates a spinner which is used to block all subsequent submissions until it completes. Next, a loop creates a context and a NOP request each iteration until the guc_ids are exhausted (request creation returns -EAGAIN). The spinner is ended, unblocking all requests created in the loop. At this point all guc_ids are exhausted but are available to steal. Try to create another request which should successfully steal a guc_id. Wait on last request to complete, idle GPU, verify a guc_id was stolen via a counter, and exit the test. Test also artificially reduces the number of guc_ids so the test runs in a timely manner. v2: (John Harrison) - s/stole/stolen - Fix some wording in test description - Rework indexing into context array - Add test description to commit message - Fix typo in commit message (Checkpatch) - s/guc/(guc) in NUMBER_MULTI_LRC_GUC_ID v3: (John Harrison) - Set array value to NULL after extracting error - Fix a few typos in comments / error messages - Delete redundant comment in commit message Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 +- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 173 ++ 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1cb46098030d..f9240d4baa69 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -94,6 +94,11 @@ struct intel_guc { * @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @num_guc_ids: Number of guc_ids, selftest feature to be able +* to reduce this number while testing. +*/ + int num_guc_ids; /** * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc */ @@ -202,6 +207,13 @@ struct intel_guc { */ struct delayed_work work; } timestamp; + +#ifdef CONFIG_DRM_I915_SELFTEST + /** +* @number_guc_id_stolen: The number of guc_ids that have been stolen +*/ + int number_guc_id_stolen; +#endif }; static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) 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 96fcf869e3ff..99414b49ca6d 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -145,7 +145,8 @@ guc_create_parallel(struct intel_engine_cs **engines, * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for * multi-lrc. */ -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) +#define NUMBER_MULTI_LRC_GUC_ID(guc) \ + ((guc)->submission_state.num_guc_ids / 16) /* * Below is a set of functions which control the GuC scheduling state which @@ -1775,7 +1776,7 @@ int intel_guc_submission_init(struct intel_guc *guc) destroyed_worker_func); guc->submission_state.guc_ids_bitmap = - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) return -ENOMEM; @@ -1869,13 +1870,13 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) if (intel_context_is_parent(ce)) ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, - NUMBER_MULTI_LRC_GUC_ID, + NUMBER_MULTI_LRC_GUC_ID(guc), order_base_2(ce->parallel.number_children + 1)); else ret = ida_simple_get(&guc->submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID, -GUC_MAX_LRC_DESCRIPTORS, +NUMBER_MULTI_LRC_GUC_ID(guc), +guc->submission_state.num_guc_ids, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) @@ -1941,6 +1942,10 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce) set_context_guc_id_invalid(cn); +#ifdef CONFIG_DRM_I915_SELFTEST + guc->number_guc_id_stolen++; +#endif + return 0; } else { return
Re: [Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
On Mon, Dec 13, 2021 at 04:26:07PM -0800, John Harrison wrote: > On 12/11/2021 09:35, Matthew Brost wrote: > > Testing the stealing of guc ids is hard from user space as we have 64k > > guc_ids. Add a selftest, which artificially reduces the number of guc > > ids, and forces a steal. Description of test below. > Last sentence seems redundant. > Will delete. > > > > The test creates a spinner which is used to block all subsequent > > submissions until it completes. Next, a loop creates a context and a NOP > > request each iteration until the guc_ids are exhausted (request creation > > returns -EAGAIN). The spinner is ended, unblocking all requests created > > in the loop. At this point all guc_ids are exhausted but are available > > to steal. Try to create another request which should successfully steal > > a guc_id. Wait on last request to complete, idle GPU, verify a guc_id > > was stolen via a counter, and exit the test. Test also artificially > > reduces the number of guc_ids so the test runs in a timely manner. > > > > v2: > > (John Harrison) > >- s/stole/stolen > >- Fix some wording in test description > >- Rework indexing into context array > >- Add test description to commit message > >- Fix typo in commit message > > (Checkpatch) > >- s/guc/(guc) in NUMBER_MULTI_LRC_GUC_ID > > > > Signed-off-by: Matthew Brost > > --- > > drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 +- > > drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 173 ++ > > 3 files changed, 196 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > index 1cb46098030d..f9240d4baa69 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > @@ -94,6 +94,11 @@ struct intel_guc { > > * @guc_ids: used to allocate new guc_ids, single-lrc > > */ > > struct ida guc_ids; > > + /** > > +* @num_guc_ids: Number of guc_ids, selftest feature to be able > > +* to reduce this number while testing. > > +*/ > > + int num_guc_ids; > > /** > > * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc > > */ > > @@ -202,6 +207,13 @@ struct intel_guc { > > */ > > struct delayed_work work; > > } timestamp; > > + > > +#ifdef CONFIG_DRM_I915_SELFTEST > > + /** > > +* @number_guc_id_stolen: The number of guc_ids that have been stolen > > +*/ > > + int number_guc_id_stolen; > > +#endif > > }; > > static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) > > 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 96fcf869e3ff..99414b49ca6d 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -145,7 +145,8 @@ guc_create_parallel(struct intel_engine_cs **engines, > >* use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids > > for > >* multi-lrc. > >*/ > > -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) > > +#define NUMBER_MULTI_LRC_GUC_ID(guc) \ > > + ((guc)->submission_state.num_guc_ids / 16) > > /* > >* Below is a set of functions which control the GuC scheduling state > > which > > @@ -1775,7 +1776,7 @@ int intel_guc_submission_init(struct intel_guc *guc) > > destroyed_worker_func); > > guc->submission_state.guc_ids_bitmap = > > - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); > > + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); > > if (!guc->submission_state.guc_ids_bitmap) > > return -ENOMEM; > > @@ -1869,13 +1870,13 @@ static int new_guc_id(struct intel_guc *guc, struct > > intel_context *ce) > > if (intel_context_is_parent(ce)) > > ret = > > bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, > > - NUMBER_MULTI_LRC_GUC_ID, > > + NUMBER_MULTI_LRC_GUC_ID(guc), > > > > order_base_2(ce->parallel.number_children > >+ 1)); > > else > > ret = ida_simple_get(&guc->submission_state.guc_ids, > > -NUMBER_MULTI_LRC_GUC_ID, > > -GUC_MAX_LRC_DESCRIPTORS, > > +NUMBER_MULTI_LRC_GUC_ID(guc), > > +guc->submission_state.num_guc_ids, > > GFP_KERNEL | __GFP_RETRY_MAYFAIL | > > __GFP_NOWARN); > > if (unlikely(ret < 0)) > > @@ -1941,6 +1942,10
Re: [Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
On 12/11/2021 09:35, Matthew Brost wrote: Testing the stealing of guc ids is hard from user space as we have 64k guc_ids. Add a selftest, which artificially reduces the number of guc ids, and forces a steal. Description of test below. Last sentence seems redundant. The test creates a spinner which is used to block all subsequent submissions until it completes. Next, a loop creates a context and a NOP request each iteration until the guc_ids are exhausted (request creation returns -EAGAIN). The spinner is ended, unblocking all requests created in the loop. At this point all guc_ids are exhausted but are available to steal. Try to create another request which should successfully steal a guc_id. Wait on last request to complete, idle GPU, verify a guc_id was stolen via a counter, and exit the test. Test also artificially reduces the number of guc_ids so the test runs in a timely manner. v2: (John Harrison) - s/stole/stolen - Fix some wording in test description - Rework indexing into context array - Add test description to commit message - Fix typo in commit message (Checkpatch) - s/guc/(guc) in NUMBER_MULTI_LRC_GUC_ID Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 +- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 173 ++ 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1cb46098030d..f9240d4baa69 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -94,6 +94,11 @@ struct intel_guc { * @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @num_guc_ids: Number of guc_ids, selftest feature to be able +* to reduce this number while testing. +*/ + int num_guc_ids; /** * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc */ @@ -202,6 +207,13 @@ struct intel_guc { */ struct delayed_work work; } timestamp; + +#ifdef CONFIG_DRM_I915_SELFTEST + /** +* @number_guc_id_stolen: The number of guc_ids that have been stolen +*/ + int number_guc_id_stolen; +#endif }; static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) 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 96fcf869e3ff..99414b49ca6d 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -145,7 +145,8 @@ guc_create_parallel(struct intel_engine_cs **engines, * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for * multi-lrc. */ -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) +#define NUMBER_MULTI_LRC_GUC_ID(guc) \ + ((guc)->submission_state.num_guc_ids / 16) /* * Below is a set of functions which control the GuC scheduling state which @@ -1775,7 +1776,7 @@ int intel_guc_submission_init(struct intel_guc *guc) destroyed_worker_func); guc->submission_state.guc_ids_bitmap = - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) return -ENOMEM; @@ -1869,13 +1870,13 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) if (intel_context_is_parent(ce)) ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, - NUMBER_MULTI_LRC_GUC_ID, + NUMBER_MULTI_LRC_GUC_ID(guc), order_base_2(ce->parallel.number_children + 1)); else ret = ida_simple_get(&guc->submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID, -GUC_MAX_LRC_DESCRIPTORS, +NUMBER_MULTI_LRC_GUC_ID(guc), +guc->submission_state.num_guc_ids, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) @@ -1941,6 +1942,10 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce) set_context_guc_id_invalid(cn); +#ifdef CONFIG_DRM_I915_SELFTEST + guc->number_guc_id_stolen++; +#endif + return 0; } else { return -EAGAIN; @@ -3779,6 +3784,7 @@ static bool __guc_submis
[Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
Testing the stealing of guc ids is hard from user space as we have 64k guc_ids. Add a selftest, which artificially reduces the number of guc ids, and forces a steal. Description of test below. The test creates a spinner which is used to block all subsequent submissions until it completes. Next, a loop creates a context and a NOP request each iteration until the guc_ids are exhausted (request creation returns -EAGAIN). The spinner is ended, unblocking all requests created in the loop. At this point all guc_ids are exhausted but are available to steal. Try to create another request which should successfully steal a guc_id. Wait on last request to complete, idle GPU, verify a guc_id was stolen via a counter, and exit the test. Test also artificially reduces the number of guc_ids so the test runs in a timely manner. v2: (John Harrison) - s/stole/stolen - Fix some wording in test description - Rework indexing into context array - Add test description to commit message - Fix typo in commit message (Checkpatch) - s/guc/(guc) in NUMBER_MULTI_LRC_GUC_ID Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 16 +- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 173 ++ 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1cb46098030d..f9240d4baa69 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -94,6 +94,11 @@ struct intel_guc { * @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @num_guc_ids: Number of guc_ids, selftest feature to be able +* to reduce this number while testing. +*/ + int num_guc_ids; /** * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc */ @@ -202,6 +207,13 @@ struct intel_guc { */ struct delayed_work work; } timestamp; + +#ifdef CONFIG_DRM_I915_SELFTEST + /** +* @number_guc_id_stolen: The number of guc_ids that have been stolen +*/ + int number_guc_id_stolen; +#endif }; static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) 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 96fcf869e3ff..99414b49ca6d 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -145,7 +145,8 @@ guc_create_parallel(struct intel_engine_cs **engines, * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for * multi-lrc. */ -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) +#define NUMBER_MULTI_LRC_GUC_ID(guc) \ + ((guc)->submission_state.num_guc_ids / 16) /* * Below is a set of functions which control the GuC scheduling state which @@ -1775,7 +1776,7 @@ int intel_guc_submission_init(struct intel_guc *guc) destroyed_worker_func); guc->submission_state.guc_ids_bitmap = - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) return -ENOMEM; @@ -1869,13 +1870,13 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) if (intel_context_is_parent(ce)) ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, - NUMBER_MULTI_LRC_GUC_ID, + NUMBER_MULTI_LRC_GUC_ID(guc), order_base_2(ce->parallel.number_children + 1)); else ret = ida_simple_get(&guc->submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID, -GUC_MAX_LRC_DESCRIPTORS, +NUMBER_MULTI_LRC_GUC_ID(guc), +guc->submission_state.num_guc_ids, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) @@ -1941,6 +1942,10 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce) set_context_guc_id_invalid(cn); +#ifdef CONFIG_DRM_I915_SELFTEST + guc->number_guc_id_stolen++; +#endif + return 0; } else { return -EAGAIN; @@ -3779,6 +3784,7 @@ static bool __guc_submission_selected(struct intel_guc *guc) void intel_guc_submission_init_early(struct i
Re: [Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
On Fri, Dec 10, 2021 at 05:33:02PM -0800, John Harrison wrote: > On 12/10/2021 16:56, Matthew Brost wrote: > > Testing the stealing of guc ids is hard from user spaec as we have 64k > spaec -> space > > > guc_ids. Add a selftest, which artificially reduces the number of guc > > ids, and forces a steal. Details of test has comment in code so will not > has -> are > > But would a copy&paste really be so hard? It is useful to be able to read > what a patch does from the commit log and not have to delve inside every > time. > > > > repeat here. > > > > Signed-off-by: Matthew Brost > > --- > > drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +- > > drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 171 ++ > > 3 files changed, 193 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > index 1cb46098030d..307380a2e2ff 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > @@ -94,6 +94,11 @@ struct intel_guc { > > * @guc_ids: used to allocate new guc_ids, single-lrc > > */ > > struct ida guc_ids; > > + /** > > +* @num_guc_ids: Number of guc_ids, selftest feature to be able > > +* to reduce this number of test. > of test -> while testing > > Should have a CONFIG_SELFTEST around it? And define a wrapper that is > GUC_MAX_LRC_DESCRIPTORS or num_guc_ids as appropriate. > Missed this. Basically decided again a SELFTEST wrapper because SRIOV needs this anyways. Can hide this if you insist. Matt > > > +*/ > > + int num_guc_ids; > > /** > > * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc > > */ > > @@ -202,6 +207,13 @@ struct intel_guc { > > */ > > struct delayed_work work; > > } timestamp; > > + > > +#ifdef CONFIG_DRM_I915_SELFTEST > > + /** > > +* @number_guc_id_stole: The number of guc_ids that have been stole > > +*/ > > + int number_guc_id_stole; > stole -> stolen (in all three cases) > > > +#endif > > }; > > static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) > > 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 96fcf869e3ff..57019b190bfb 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -145,7 +145,7 @@ guc_create_parallel(struct intel_engine_cs **engines, > >* use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids > > for > >* multi-lrc. > >*/ > > -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) > > +#define NUMBER_MULTI_LRC_GUC_ID(guc) > > (guc->submission_state.num_guc_ids / 16) > And keep the original definition for the non CONFIG_SELFTEST case? > > > /* > >* Below is a set of functions which control the GuC scheduling state > > which > > @@ -1775,7 +1775,7 @@ int intel_guc_submission_init(struct intel_guc *guc) > > destroyed_worker_func); > > guc->submission_state.guc_ids_bitmap = > > - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); > > + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); > > if (!guc->submission_state.guc_ids_bitmap) > > return -ENOMEM; > > @@ -1869,13 +1869,13 @@ static int new_guc_id(struct intel_guc *guc, struct > > intel_context *ce) > > if (intel_context_is_parent(ce)) > > ret = > > bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, > > - NUMBER_MULTI_LRC_GUC_ID, > > + NUMBER_MULTI_LRC_GUC_ID(guc), > > > > order_base_2(ce->parallel.number_children > >+ 1)); > > else > > ret = ida_simple_get(&guc->submission_state.guc_ids, > > -NUMBER_MULTI_LRC_GUC_ID, > > -GUC_MAX_LRC_DESCRIPTORS, > > +NUMBER_MULTI_LRC_GUC_ID(guc), > > +guc->submission_state.num_guc_ids, > > GFP_KERNEL | __GFP_RETRY_MAYFAIL | > > __GFP_NOWARN); > > if (unlikely(ret < 0)) > > @@ -1941,6 +1941,10 @@ static int steal_guc_id(struct intel_guc *guc, > > struct intel_context *ce) > > set_context_guc_id_invalid(cn); > > +#ifdef CONFIG_DRM_I915_SELFTEST > > + guc->number_guc_id_stole++; > > +#endif > > + > > return 0; > > } else { > > return -EAGAIN; > > @@ -3779,6 +3783,7 @@ static bool __guc_submission_selected(struct > > intel_guc *guc
Re: [Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
On Fri, Dec 10, 2021 at 05:33:02PM -0800, John Harrison wrote: > On 12/10/2021 16:56, Matthew Brost wrote: > > Testing the stealing of guc ids is hard from user spaec as we have 64k > spaec -> space > > > guc_ids. Add a selftest, which artificially reduces the number of guc > > ids, and forces a steal. Details of test has comment in code so will not > has -> are > Yep. > But would a copy&paste really be so hard? It is useful to be able to read > what a patch does from the commit log and not have to delve inside every > time. > Will c & p. > > > repeat here. > > > > Signed-off-by: Matthew Brost > > --- > > drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +- > > drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 171 ++ > > 3 files changed, 193 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > index 1cb46098030d..307380a2e2ff 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > @@ -94,6 +94,11 @@ struct intel_guc { > > * @guc_ids: used to allocate new guc_ids, single-lrc > > */ > > struct ida guc_ids; > > + /** > > +* @num_guc_ids: Number of guc_ids, selftest feature to be able > > +* to reduce this number of test. > of test -> while testing > > Should have a CONFIG_SELFTEST around it? And define a wrapper that is > GUC_MAX_LRC_DESCRIPTORS or num_guc_ids as appropriate. > > > > +*/ > > + int num_guc_ids; > > /** > > * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc > > */ > > @@ -202,6 +207,13 @@ struct intel_guc { > > */ > > struct delayed_work work; > > } timestamp; > > + > > +#ifdef CONFIG_DRM_I915_SELFTEST > > + /** > > +* @number_guc_id_stole: The number of guc_ids that have been stole > > +*/ > > + int number_guc_id_stole; > stole -> stolen (in all three cases) > Sure. > > +#endif > > }; > > static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) > > 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 96fcf869e3ff..57019b190bfb 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -145,7 +145,7 @@ guc_create_parallel(struct intel_engine_cs **engines, > >* use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids > > for > >* multi-lrc. > >*/ > > -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) > > +#define NUMBER_MULTI_LRC_GUC_ID(guc) > > (guc->submission_state.num_guc_ids / 16) > And keep the original definition for the non CONFIG_SELFTEST case? > Probably could hide submission_state.num_guc_ids behind a CONFIG_SELFTEST define but SRIOV needs this anyways so didn't bother. If you insist can I hide this. > > /* > >* Below is a set of functions which control the GuC scheduling state > > which > > @@ -1775,7 +1775,7 @@ int intel_guc_submission_init(struct intel_guc *guc) > > destroyed_worker_func); > > guc->submission_state.guc_ids_bitmap = > > - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); > > + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); > > if (!guc->submission_state.guc_ids_bitmap) > > return -ENOMEM; > > @@ -1869,13 +1869,13 @@ static int new_guc_id(struct intel_guc *guc, struct > > intel_context *ce) > > if (intel_context_is_parent(ce)) > > ret = > > bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, > > - NUMBER_MULTI_LRC_GUC_ID, > > + NUMBER_MULTI_LRC_GUC_ID(guc), > > > > order_base_2(ce->parallel.number_children > >+ 1)); > > else > > ret = ida_simple_get(&guc->submission_state.guc_ids, > > -NUMBER_MULTI_LRC_GUC_ID, > > -GUC_MAX_LRC_DESCRIPTORS, > > +NUMBER_MULTI_LRC_GUC_ID(guc), > > +guc->submission_state.num_guc_ids, > > GFP_KERNEL | __GFP_RETRY_MAYFAIL | > > __GFP_NOWARN); > > if (unlikely(ret < 0)) > > @@ -1941,6 +1941,10 @@ static int steal_guc_id(struct intel_guc *guc, > > struct intel_context *ce) > > set_context_guc_id_invalid(cn); > > +#ifdef CONFIG_DRM_I915_SELFTEST > > + guc->number_guc_id_stole++; > > +#endif > > + > > return 0; > > } else { > > return -EAGAIN; > > @@ -3779,6 +3783,7 @@ s
Re: [Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
On 12/10/2021 16:56, Matthew Brost wrote: Testing the stealing of guc ids is hard from user spaec as we have 64k spaec -> space guc_ids. Add a selftest, which artificially reduces the number of guc ids, and forces a steal. Details of test has comment in code so will not has -> are But would a copy&paste really be so hard? It is useful to be able to read what a patch does from the commit log and not have to delve inside every time. repeat here. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 171 ++ 3 files changed, 193 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1cb46098030d..307380a2e2ff 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -94,6 +94,11 @@ struct intel_guc { * @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @num_guc_ids: Number of guc_ids, selftest feature to be able +* to reduce this number of test. of test -> while testing Should have a CONFIG_SELFTEST around it? And define a wrapper that is GUC_MAX_LRC_DESCRIPTORS or num_guc_ids as appropriate. +*/ + int num_guc_ids; /** * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc */ @@ -202,6 +207,13 @@ struct intel_guc { */ struct delayed_work work; } timestamp; + +#ifdef CONFIG_DRM_I915_SELFTEST + /** +* @number_guc_id_stole: The number of guc_ids that have been stole +*/ + int number_guc_id_stole; stole -> stolen (in all three cases) +#endif }; static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) 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 96fcf869e3ff..57019b190bfb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -145,7 +145,7 @@ guc_create_parallel(struct intel_engine_cs **engines, * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for * multi-lrc. */ -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) +#define NUMBER_MULTI_LRC_GUC_ID(guc) (guc->submission_state.num_guc_ids / 16) And keep the original definition for the non CONFIG_SELFTEST case? /* * Below is a set of functions which control the GuC scheduling state which @@ -1775,7 +1775,7 @@ int intel_guc_submission_init(struct intel_guc *guc) destroyed_worker_func); guc->submission_state.guc_ids_bitmap = - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) return -ENOMEM; @@ -1869,13 +1869,13 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) if (intel_context_is_parent(ce)) ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, - NUMBER_MULTI_LRC_GUC_ID, + NUMBER_MULTI_LRC_GUC_ID(guc), order_base_2(ce->parallel.number_children + 1)); else ret = ida_simple_get(&guc->submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID, -GUC_MAX_LRC_DESCRIPTORS, +NUMBER_MULTI_LRC_GUC_ID(guc), +guc->submission_state.num_guc_ids, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) @@ -1941,6 +1941,10 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce) set_context_guc_id_invalid(cn); +#ifdef CONFIG_DRM_I915_SELFTEST + guc->number_guc_id_stole++; +#endif + return 0; } else { return -EAGAIN; @@ -3779,6 +3783,7 @@ static bool __guc_submission_selected(struct intel_guc *guc) void intel_guc_submission_init_early(struct intel_guc *guc) { + guc->submission_state.num_guc_ids = GUC_MAX_LRC_DESCRIPTORS; guc->submission_supported = __guc_submission_supported(guc); guc->submission_selected = __guc_submission_selected(guc); } diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c index fb0e4a7bd8ca..9ab355e64b3f 100644 -
[Intel-gfx] [PATCH 7/7] drm/i915/guc: Selftest for stealing of guc ids
Testing the stealing of guc ids is hard from user spaec as we have 64k guc_ids. Add a selftest, which artificially reduces the number of guc ids, and forces a steal. Details of test has comment in code so will not repeat here. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 12 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +- drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 171 ++ 3 files changed, 193 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1cb46098030d..307380a2e2ff 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -94,6 +94,11 @@ struct intel_guc { * @guc_ids: used to allocate new guc_ids, single-lrc */ struct ida guc_ids; + /** +* @num_guc_ids: Number of guc_ids, selftest feature to be able +* to reduce this number of test. +*/ + int num_guc_ids; /** * @guc_ids_bitmap: used to allocate new guc_ids, multi-lrc */ @@ -202,6 +207,13 @@ struct intel_guc { */ struct delayed_work work; } timestamp; + +#ifdef CONFIG_DRM_I915_SELFTEST + /** +* @number_guc_id_stole: The number of guc_ids that have been stole +*/ + int number_guc_id_stole; +#endif }; static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) 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 96fcf869e3ff..57019b190bfb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -145,7 +145,7 @@ guc_create_parallel(struct intel_engine_cs **engines, * use should be low and 1/16 should be sufficient. Minimum of 32 guc_ids for * multi-lrc. */ -#define NUMBER_MULTI_LRC_GUC_ID(GUC_MAX_LRC_DESCRIPTORS / 16) +#define NUMBER_MULTI_LRC_GUC_ID(guc) (guc->submission_state.num_guc_ids / 16) /* * Below is a set of functions which control the GuC scheduling state which @@ -1775,7 +1775,7 @@ int intel_guc_submission_init(struct intel_guc *guc) destroyed_worker_func); guc->submission_state.guc_ids_bitmap = - bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID, GFP_KERNEL); + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) return -ENOMEM; @@ -1869,13 +1869,13 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) if (intel_context_is_parent(ce)) ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap, - NUMBER_MULTI_LRC_GUC_ID, + NUMBER_MULTI_LRC_GUC_ID(guc), order_base_2(ce->parallel.number_children + 1)); else ret = ida_simple_get(&guc->submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID, -GUC_MAX_LRC_DESCRIPTORS, +NUMBER_MULTI_LRC_GUC_ID(guc), +guc->submission_state.num_guc_ids, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) @@ -1941,6 +1941,10 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce) set_context_guc_id_invalid(cn); +#ifdef CONFIG_DRM_I915_SELFTEST + guc->number_guc_id_stole++; +#endif + return 0; } else { return -EAGAIN; @@ -3779,6 +3783,7 @@ static bool __guc_submission_selected(struct intel_guc *guc) void intel_guc_submission_init_early(struct intel_guc *guc) { + guc->submission_state.num_guc_ids = GUC_MAX_LRC_DESCRIPTORS; guc->submission_supported = __guc_submission_supported(guc); guc->submission_selected = __guc_submission_selected(guc); } diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c index fb0e4a7bd8ca..9ab355e64b3f 100644 --- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c @@ -3,8 +3,21 @@ * Copyright �� 2021 Intel Corporation */ +#include "selftests/igt_spinner.h" #include "selftests/intel_scheduler_helpers.h" +static int request_add_spin(struct i915_request *rq, struct igt_spinner *spin) +{ + int err = 0; + + i915_request_get(rq); + i915_request_add(rq); + if (spin && !igt_wait_for_spinner(spin, rq)) + err = -ETIMEDOUT; + +