intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: Matthew Brost <matthew.brost@intel.com>, <igt-dev@lists.freedesktop.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 6/7] i915/gem_scheduler: Make gem_scheduler understand static priority mapping
Date: Wed, 28 Jul 2021 18:51:29 -0700	[thread overview]
Message-ID: <e46a72c7-3443-bbcd-273e-6939b743fc2b@intel.com> (raw)
In-Reply-To: <20210727152202.9527-7-matthew.brost@intel.com>



On 7/27/2021 8:22 AM, Matthew Brost wrote:
> The i915 currently has 2k visible priority levels which are currently
> unique. This is changing to statically map these 2k levels into 3
> buckets:
>
> low: < 0
> mid: 0
> high: > 0
>
> Update gem_scheduler to understand this. This entails updating promotion
> test to use 3 levels that will map into different buckets and also
> delete a racey check. Also skip any tests that rely on having more than

Can you elaborate on why the check is racey? I'm sure you've already 
explained it to me in the past but can't spot it right now.

> 3 priority levels.
>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>   lib/i915/gem_scheduler.c       | 13 ++++++++++
>   lib/i915/gem_scheduler.h       |  1 +
>   tests/i915/gem_exec_schedule.c | 47 ++++++++++++++++++++--------------
>   3 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/lib/i915/gem_scheduler.c b/lib/i915/gem_scheduler.c
> index cdddf42ad..bec2e485a 100644
> --- a/lib/i915/gem_scheduler.c
> +++ b/lib/i915/gem_scheduler.c
> @@ -90,6 +90,19 @@ bool gem_scheduler_has_ctx_priority(int fd)
>   		I915_SCHEDULER_CAP_PRIORITY;
>   }
>   
> +/**
> + * gem_scheduler_has_ctx_priority:

s/ctx/static/

LGTM apart from this.

Daniele

> + * @fd: open i915 drm file descriptor
> + *
> + * Feature test macro to query whether the driver supports priority assigned
> + * from user space are statically mapping into 3 buckets.
> + */
> +bool gem_scheduler_has_static_priority(int fd)
> +{
> +	return gem_scheduler_capability(fd) &
> +		I915_SCHEDULER_CAP_STATIC_PRIORITY_MAP;
> +}
> +
>   /**
>    * gem_scheduler_has_preemption:
>    * @fd: open i915 drm file descriptor
> diff --git a/lib/i915/gem_scheduler.h b/lib/i915/gem_scheduler.h
> index d43e84bd2..b00804f70 100644
> --- a/lib/i915/gem_scheduler.h
> +++ b/lib/i915/gem_scheduler.h
> @@ -29,6 +29,7 @@
>   unsigned gem_scheduler_capability(int fd);
>   bool gem_scheduler_enabled(int fd);
>   bool gem_scheduler_has_ctx_priority(int fd);
> +bool gem_scheduler_has_static_priority(int fd);
>   bool gem_scheduler_has_preemption(int fd);
>   bool gem_scheduler_has_semaphores(int fd);
>   bool gem_scheduler_has_engine_busy_stats(int fd);
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index e5fb45982..f03842478 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -1344,8 +1344,7 @@ static void reorder(int fd, const intel_ctx_cfg_t *cfg,
>   static void promotion(int fd, const intel_ctx_cfg_t *cfg, unsigned ring)
>   {
>   	IGT_CORK_FENCE(cork);
> -	uint32_t result, dep;
> -	uint32_t result_read, dep_read;
> +	uint32_t result, dep, dep_read;
>   	const intel_ctx_t *ctx[3];
>   	int fence;
>   
> @@ -1353,10 +1352,10 @@ static void promotion(int fd, const intel_ctx_cfg_t *cfg, unsigned ring)
>   	gem_context_set_priority(fd, ctx[LO]->id, MIN_PRIO);
>   
>   	ctx[HI] = intel_ctx_create(fd, cfg);
> -	gem_context_set_priority(fd, ctx[HI]->id, 0);
> +	gem_context_set_priority(fd, ctx[HI]->id, MAX_PRIO);
>   
>   	ctx[NOISE] = intel_ctx_create(fd, cfg);
> -	gem_context_set_priority(fd, ctx[NOISE]->id, MIN_PRIO/2);
> +	gem_context_set_priority(fd, ctx[NOISE]->id, 0);
>   
>   	result = gem_create(fd, 4096);
>   	dep = gem_create(fd, 4096);
> @@ -1383,11 +1382,9 @@ static void promotion(int fd, const intel_ctx_cfg_t *cfg, unsigned ring)
>   	dep_read = __sync_read_u32(fd, dep, 0);
>   	gem_close(fd, dep);
>   
> -	result_read = __sync_read_u32(fd, result, 0);
>   	gem_close(fd, result);
>   
>   	igt_assert_eq_u32(dep_read, ctx[HI]->id);
> -	igt_assert_eq_u32(result_read, ctx[NOISE]->id);
>   
>   	intel_ctx_destroy(fd, ctx[NOISE]);
>   	intel_ctx_destroy(fd, ctx[LO]);
> @@ -2963,19 +2960,25 @@ igt_main
>   			test_each_engine_store("preempt-other-chain", fd, ctx, e)
>   				preempt_other(fd, &ctx->cfg, e->flags, CHAIN);
>   
> -			test_each_engine_store("preempt-queue", fd, ctx, e)
> -				preempt_queue(fd, &ctx->cfg, e->flags, 0);
> +			test_each_engine_store("preempt-engines", fd, ctx, e)
> +				preempt_engines(fd, e, 0);
>   
> -			test_each_engine_store("preempt-queue-chain", fd, ctx, e)
> -				preempt_queue(fd, &ctx->cfg, e->flags, CHAIN);
> -			test_each_engine_store("preempt-queue-contexts", fd, ctx, e)
> -				preempt_queue(fd, &ctx->cfg, e->flags, CONTEXTS);
> +			igt_subtest_group {
> +				igt_fixture {
> +					igt_require(!gem_scheduler_has_static_priority(fd));
> +				}
>   
> -			test_each_engine_store("preempt-queue-contexts-chain", fd, ctx, e)
> -				preempt_queue(fd, &ctx->cfg, e->flags, CONTEXTS | CHAIN);
> +				test_each_engine_store("preempt-queue", fd, ctx, e)
> +					preempt_queue(fd, &ctx->cfg, e->flags, 0);
>   
> -			test_each_engine_store("preempt-engines", fd, ctx, e)
> -				preempt_engines(fd, e, 0);
> +				test_each_engine_store("preempt-queue-chain", fd, ctx, e)
> +					preempt_queue(fd, &ctx->cfg, e->flags, CHAIN);
> +				test_each_engine_store("preempt-queue-contexts", fd, ctx, e)
> +					preempt_queue(fd, &ctx->cfg, e->flags, CONTEXTS);
> +
> +				test_each_engine_store("preempt-queue-contexts-chain", fd, ctx, e)
> +					preempt_queue(fd, &ctx->cfg, e->flags, CONTEXTS | CHAIN);
> +			}
>   
>   			igt_subtest_group {
>   				igt_hang_t hang;
> @@ -3017,11 +3020,17 @@ igt_main
>   		test_each_engine_store("wide", fd, ctx, e)
>   			wide(fd, &ctx->cfg, e->flags);
>   
> -		test_each_engine_store("reorder-wide", fd, ctx, e)
> -			reorder_wide(fd, &ctx->cfg, e->flags);
> -
>   		test_each_engine_store("smoketest", fd, ctx, e)
>   			smoketest(fd, &ctx->cfg, e->flags, 5);
> +
> +		igt_subtest_group {
> +			igt_fixture {
> +				igt_require(!gem_scheduler_has_static_priority(fd));
> +			}
> +
> +			test_each_engine_store("reorder-wide", fd, ctx, e)
> +				reorder_wide(fd, &ctx->cfg, e->flags);
> +		}
>   	}
>   
>   	igt_subtest_group {

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-07-29  1:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 15:21 [Intel-gfx] [PATCH i-g-t 0/7] Updates for GuC & parallel submission Matthew Brost
2021-07-27 15:21 ` [Intel-gfx] [PATCH i-g-t 1/7] include/drm-uapi: Add parallel context configuration uAPI Matthew Brost
2021-07-27 15:21 ` [Intel-gfx] [PATCH i-g-t 2/7] include/drm-uapi: Add logical mapping uAPI Matthew Brost
2021-07-27 15:21 ` [Intel-gfx] [PATCH i-g-t 3/7] lib/intel_ctx: Add support for parallel contexts to intel_ctx library Matthew Brost
2021-07-27 15:21 ` [Intel-gfx] [PATCH i-g-t 4/7] i915/gem_exec_balancer: Test parallel execbuf Matthew Brost
2021-07-27 15:22 ` [Intel-gfx] [PATCH i-g-t 5/7] include/drm-uapi: Add static priority mapping UAPI Matthew Brost
2021-07-27 15:22 ` [Intel-gfx] [PATCH i-g-t 6/7] i915/gem_scheduler: Make gem_scheduler understand static priority mapping Matthew Brost
2021-07-29  1:51   ` Daniele Ceraolo Spurio [this message]
2021-07-29 23:35     ` [Intel-gfx] [igt-dev] " Matthew Brost
2021-07-27 15:22 ` [Intel-gfx] [PATCH i-g-t 7/7] i915/gem_ctx_shared: Make gem_ctx_shared " Matthew Brost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e46a72c7-3443-bbcd-273e-6939b743fc2b@intel.com \
    --to=daniele.ceraolospurio@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).