[PATCH 0/2] *** add scheduling policy ***

[PATCH 0/2] *** add scheduling policy ***

[Chunming Zhou]

Andres added high priority queue for userspace, which got me to think more about
scheduling policy, if high priority queue is always full and busy, low priority queue could starve.


Chunming Zhou (2):
  drm/amd/sched: revise priority number
  drm/amd/sched: add schuduling policy

 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 30 ++++++++++++++++++++++-----
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 11 ++++++----
 2 files changed, 32 insertions(+), 9 deletions(-)

-- 
1.9.1

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

[PATCH 1/2] drm/amd/sched: revise priority number

[Chunming Zhou]

big number is to high priority.

Change-Id: I1e94b3403d0cfd41a418d2bd741736cf7edc5d77
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 4 ++--
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 9 +++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 3ff25af..0f439dd 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -499,7 +499,7 @@ static void amd_sched_wakeup(struct amd_gpu_scheduler *sched)
 		return NULL;
 
 	/* Kernel run queue has higher priority than normal run queue*/
-	for (i = 0; i < AMD_SCHED_MAX_PRIORITY; i++) {
+	for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= AMD_SCHED_PRIORITY_MIN; i--) {
 		entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
 		if (entity)
 			break;
@@ -607,7 +607,7 @@ int amd_sched_init(struct amd_gpu_scheduler *sched,
 	sched->hw_submission_limit = hw_submission;
 	sched->name = name;
 	sched->timeout = timeout;
-	for (i = 0; i < AMD_SCHED_MAX_PRIORITY; i++)
+	for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
 		amd_sched_rq_init(&sched->sched_rq[i]);
 
 	init_waitqueue_head(&sched->wake_up_worker);
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 338d840..99f0240 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -111,9 +111,10 @@ struct amd_sched_backend_ops {
 };
 
 enum amd_sched_priority {
-	AMD_SCHED_PRIORITY_KERNEL = 0,
-	AMD_SCHED_PRIORITY_NORMAL,
-	AMD_SCHED_MAX_PRIORITY
+	AMD_SCHED_PRIORITY_MIN,
+	AMD_SCHED_PRIORITY_NORMAL = AMD_SCHED_PRIORITY_MIN,
+	AMD_SCHED_PRIORITY_KERNEL,
+	AMD_SCHED_PRIORITY_MAX
 };
 
 /**
@@ -124,7 +125,7 @@ struct amd_gpu_scheduler {
 	uint32_t			hw_submission_limit;
 	long				timeout;
 	const char			*name;
-	struct amd_sched_rq		sched_rq[AMD_SCHED_MAX_PRIORITY];
+	struct amd_sched_rq		sched_rq[AMD_SCHED_PRIORITY_MAX];
 	wait_queue_head_t		wake_up_worker;
 	wait_queue_head_t		job_scheduled;
 	atomic_t			hw_rq_count;
-- 
1.9.1

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

[PATCH 2/2] drm/amd/sched: add schuduling policy

[Chunming Zhou]

if high priority rq is full, then process with low priority could be starve.
Add policy for this problem, the high proiority can ahead of next priority queue,
the ratio is 2 : 1.

Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26 +++++++++++++++++++++++---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 0f439dd..4637b6f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -35,11 +35,16 @@
 static void amd_sched_process_job(struct fence *f, struct fence_cb *cb);
 
 /* Initialize a given run queue struct */
-static void amd_sched_rq_init(struct amd_sched_rq *rq)
+static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
+			      amd_sched_priority pri)
 {
+	struct amd_sched_rq *rq = &sched->sched_rq[pri];
+
 	spin_lock_init(&rq->lock);
 	INIT_LIST_HEAD(&rq->entities);
 	rq->current_entity = NULL;
+	rq->wait_base = pri * 2;
+	rq->wait = rq->wait_base;
 }
 
 static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
@@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct amd_gpu_scheduler *sched)
 {
 	struct amd_sched_entity *entity;
 	int i;
+	bool skip;
 
 	if (!amd_sched_ready(sched))
 		return NULL;
 
+retry:
+	skip = false;
 	/* Kernel run queue has higher priority than normal run queue*/
 	for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= AMD_SCHED_PRIORITY_MIN; i--) {
+		if ((i > AMD_SCHED_PRIORITY_MIN) &&
+		    (sched->sched_rq[i - 1].wait >= sched->sched_rq[i].wait_base)) {
+			sched->sched_rq[i - 1].wait = sched->sched_rq[i - 1].wait_base;
+			skip = true;
+			continue;
+		}
 		entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
-		if (entity)
+		if (entity) {
+			if (i > AMD_SCHED_PRIORITY_MIN)
+				sched->sched_rq[i - 1].wait++;
 			break;
+		}
 	}
 
+	if (!entity && skip)
+		goto retry;
+
 	return entity;
 }
 
@@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler *sched,
 	sched->name = name;
 	sched->timeout = timeout;
 	for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
-		amd_sched_rq_init(&sched->sched_rq[i]);
+		amd_sched_rq_init(sched, i);
 
 	init_waitqueue_head(&sched->wake_up_worker);
 	init_waitqueue_head(&sched->job_scheduled);
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 99f0240..4caed30 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -64,6 +64,8 @@ struct amd_sched_rq {
 	spinlock_t		lock;
 	struct list_head	entities;
 	struct amd_sched_entity	*current_entity;
+	int wait_base;
+	int wait;
 };
 
 struct amd_sched_fence {
-- 
1.9.1

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

Re: [PATCH 1/2] drm/amd/sched: revise priority number

[Christian König]

Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
> big number is to high priority.
>
> Change-Id: I1e94b3403d0cfd41a418d2bd741736cf7edc5d77
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 4 ++--
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 9 +++++----
>   2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 3ff25af..0f439dd 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -499,7 +499,7 @@ static void amd_sched_wakeup(struct amd_gpu_scheduler *sched)
>   		return NULL;
>   
>   	/* Kernel run queue has higher priority than normal run queue*/
> -	for (i = 0; i < AMD_SCHED_MAX_PRIORITY; i++) {
> +	for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= AMD_SCHED_PRIORITY_MIN; i--) {
>   		entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>   		if (entity)
>   			break;
> @@ -607,7 +607,7 @@ int amd_sched_init(struct amd_gpu_scheduler *sched,
>   	sched->hw_submission_limit = hw_submission;
>   	sched->name = name;
>   	sched->timeout = timeout;
> -	for (i = 0; i < AMD_SCHED_MAX_PRIORITY; i++)
> +	for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
>   		amd_sched_rq_init(&sched->sched_rq[i]);
>   
>   	init_waitqueue_head(&sched->wake_up_worker);
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> index 338d840..99f0240 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> @@ -111,9 +111,10 @@ struct amd_sched_backend_ops {
>   };
>   
>   enum amd_sched_priority {
> -	AMD_SCHED_PRIORITY_KERNEL = 0,
> -	AMD_SCHED_PRIORITY_NORMAL,
> -	AMD_SCHED_MAX_PRIORITY
> +	AMD_SCHED_PRIORITY_MIN,
> +	AMD_SCHED_PRIORITY_NORMAL = AMD_SCHED_PRIORITY_MIN,
> +	AMD_SCHED_PRIORITY_KERNEL,
> +	AMD_SCHED_PRIORITY_MAX
>   };
>   
>   /**
> @@ -124,7 +125,7 @@ struct amd_gpu_scheduler {
>   	uint32_t			hw_submission_limit;
>   	long				timeout;
>   	const char			*name;
> -	struct amd_sched_rq		sched_rq[AMD_SCHED_MAX_PRIORITY];
> +	struct amd_sched_rq		sched_rq[AMD_SCHED_PRIORITY_MAX];
>   	wait_queue_head_t		wake_up_worker;
>   	wait_queue_head_t		job_scheduled;
>   	atomic_t			hw_rq_count;


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

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Christian König]

Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
> if high priority rq is full, then process with low priority could be starve.
> Add policy for this problem, the high proiority can ahead of next priority queue,
> the ratio is 2 : 1.
>
> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>

Well, the idea behind the high priority queues is to actually starve the 
low priority queues to a certain amount.

At least for the kernel queue that is really desired.

Christian.

> ---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26 +++++++++++++++++++++++---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>   2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 0f439dd..4637b6f 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -35,11 +35,16 @@
>   static void amd_sched_process_job(struct fence *f, struct fence_cb *cb);
>   
>   /* Initialize a given run queue struct */
> -static void amd_sched_rq_init(struct amd_sched_rq *rq)
> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
> +			      amd_sched_priority pri)
>   {
> +	struct amd_sched_rq *rq = &sched->sched_rq[pri];
> +
>   	spin_lock_init(&rq->lock);
>   	INIT_LIST_HEAD(&rq->entities);
>   	rq->current_entity = NULL;
> +	rq->wait_base = pri * 2;
> +	rq->wait = rq->wait_base;
>   }
>   
>   static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct amd_gpu_scheduler *sched)
>   {
>   	struct amd_sched_entity *entity;
>   	int i;
> +	bool skip;
>   
>   	if (!amd_sched_ready(sched))
>   		return NULL;
>   
> +retry:
> +	skip = false;
>   	/* Kernel run queue has higher priority than normal run queue*/
>   	for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= AMD_SCHED_PRIORITY_MIN; i--) {
> +		if ((i > AMD_SCHED_PRIORITY_MIN) &&
> +		    (sched->sched_rq[i - 1].wait >= sched->sched_rq[i].wait_base)) {
> +			sched->sched_rq[i - 1].wait = sched->sched_rq[i - 1].wait_base;
> +			skip = true;
> +			continue;
> +		}
>   		entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
> -		if (entity)
> +		if (entity) {
> +			if (i > AMD_SCHED_PRIORITY_MIN)
> +				sched->sched_rq[i - 1].wait++;
>   			break;
> +		}
>   	}
>   
> +	if (!entity && skip)
> +		goto retry;
> +
>   	return entity;
>   }
>   
> @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler *sched,
>   	sched->name = name;
>   	sched->timeout = timeout;
>   	for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
> -		amd_sched_rq_init(&sched->sched_rq[i]);
> +		amd_sched_rq_init(sched, i);
>   
>   	init_waitqueue_head(&sched->wake_up_worker);
>   	init_waitqueue_head(&sched->job_scheduled);
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> index 99f0240..4caed30 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>   	spinlock_t		lock;
>   	struct list_head	entities;
>   	struct amd_sched_entity	*current_entity;
> +	int wait_base;
> +	int wait;
>   };
>   
>   struct amd_sched_fence {


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

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[zhoucm1]

On 2017年03月16日 17:10, Christian König wrote:
> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
>> if high priority rq is full, then process with low priority could be 
>> starve.
>> Add policy for this problem, the high proiority can ahead of next 
>> priority queue,
>> the ratio is 2 : 1.
>>
>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>
> Well, the idea behind the high priority queues is to actually starve 
> the low priority queues to a certain amount.
>
> At least for the kernel queue that is really desired.
Yes, agree, but we certainly don't want  low priority queue is totally 
dead, which doesn't have chance to run if high priority queue is always 
full and busy.
If without Andres changes, it doesn't matter. But after Andres changes 
upstream, we need scheduling policy.

Regards,
David Zhou
>
> Christian.
>
>> ---
>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26 
>> +++++++++++++++++++++++---
>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c 
>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>> index 0f439dd..4637b6f 100644
>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>> @@ -35,11 +35,16 @@
>>   static void amd_sched_process_job(struct fence *f, struct fence_cb 
>> *cb);
>>     /* Initialize a given run queue struct */
>> -static void amd_sched_rq_init(struct amd_sched_rq *rq)
>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
>> +                  amd_sched_priority pri)
>>   {
>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
>> +
>>       spin_lock_init(&rq->lock);
>>       INIT_LIST_HEAD(&rq->entities);
>>       rq->current_entity = NULL;
>> +    rq->wait_base = pri * 2;
>> +    rq->wait = rq->wait_base;
>>   }
>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct 
>> amd_gpu_scheduler *sched)
>>   {
>>       struct amd_sched_entity *entity;
>>       int i;
>> +    bool skip;
>>         if (!amd_sched_ready(sched))
>>           return NULL;
>>   +retry:
>> +    skip = false;
>>       /* Kernel run queue has higher priority than normal run queue*/
>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= 
>> AMD_SCHED_PRIORITY_MIN; i--) {
>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
>> +            (sched->sched_rq[i - 1].wait >= 
>> sched->sched_rq[i].wait_base)) {
>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i - 
>> 1].wait_base;
>> +            skip = true;
>> +            continue;
>> +        }
>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>> -        if (entity)
>> +        if (entity) {
>> +            if (i > AMD_SCHED_PRIORITY_MIN)
>> +                sched->sched_rq[i - 1].wait++;
>>               break;
>> +        }
>>       }
>>   +    if (!entity && skip)
>> +        goto retry;
>> +
>>       return entity;
>>   }
>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler 
>> *sched,
>>       sched->name = name;
>>       sched->timeout = timeout;
>>       for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
>> -        amd_sched_rq_init(&sched->sched_rq[i]);
>> +        amd_sched_rq_init(sched, i);
>>         init_waitqueue_head(&sched->wake_up_worker);
>>       init_waitqueue_head(&sched->job_scheduled);
>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h 
>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> index 99f0240..4caed30 100644
>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>>       spinlock_t        lock;
>>       struct list_head    entities;
>>       struct amd_sched_entity    *current_entity;
>> +    int wait_base;
>> +    int wait;
>>   };
>>     struct amd_sched_fence {
>
>

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

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Andres Rodriguez]

On 2017-03-16 05:15 AM, zhoucm1 wrote:
>
>
> On 2017年03月16日 17:10, Christian König wrote:
>> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
>>> if high priority rq is full, then process with low priority could be
>>> starve.
>>> Add policy for this problem, the high proiority can ahead of next
>>> priority queue,
>>> the ratio is 2 : 1.
>>>
>>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>
>> Well, the idea behind the high priority queues is to actually starve
>> the low priority queues to a certain amount.
>>
>> At least for the kernel queue that is really desired.
> Yes, agree, but we certainly don't want  low priority queue is totally
> dead, which doesn't have chance to run if high priority queue is always
> full and busy.
> If without Andres changes, it doesn't matter. But after Andres changes
> upstream, we need scheduling policy.
>
> Regards,
> David Zhou

Hey David,

The desired behavior is actually starvation. This is why allocating a 
high priority context is locked behind root privileges at the moment.

High priority work includes many different features intended for the 
safety of the user wearing the headset. These include:
     - stopping the user from tripping over furniture
     - preventing motion sickness

We cannot compromise these safety features in order to run non-critical 
tasks.

The current approach also has the disadvantage of heavily interleaving 
work. This is going to have two undesired side effects. First, there 
will be a performance overhead from having the queue context switch so 
often.

Second, this approach improves concurrency of work in a ring with mixed 
priority work, but actually decreases overall system concurrency. When a 
ring's HW queue has any high priority work committed, the whole ring 
will be executing at high priority. So interleaving the work will result 
in extending the time a ring spends in high priority mode. This 
effectively extends time that a ring might spend with CU's reserved 
which will have a performance impact on other rings.

The best approach here is to make sure we don't map high priority work 
and regular priority work to the same ring. This is why the LRU policy 
for userspace ring ids to kernel ring ids was introduced. This policy 
provides the guarantee as a side effect. But further work there could be 
useful to actually check context priorities when doing the ring mapping.

By placing work on different rings, we let the hardware actually 
dispatch work according to wave parameters like VGPR usage, etc. Trying 
to deduce all this information in the GPU scheduler will get very 
complicated.

This is NAK'd by me.

Regards,
Andres


>>
>> Christian.
>>
>>> ---
>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26
>>> +++++++++++++++++++++++---
>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>> index 0f439dd..4637b6f 100644
>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>> @@ -35,11 +35,16 @@
>>>   static void amd_sched_process_job(struct fence *f, struct fence_cb
>>> *cb);
>>>     /* Initialize a given run queue struct */
>>> -static void amd_sched_rq_init(struct amd_sched_rq *rq)
>>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
>>> +                  amd_sched_priority pri)
>>>   {
>>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
>>> +
>>>       spin_lock_init(&rq->lock);
>>>       INIT_LIST_HEAD(&rq->entities);
>>>       rq->current_entity = NULL;
>>> +    rq->wait_base = pri * 2;
>>> +    rq->wait = rq->wait_base;
>>>   }
>>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
>>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct
>>> amd_gpu_scheduler *sched)
>>>   {
>>>       struct amd_sched_entity *entity;
>>>       int i;
>>> +    bool skip;
>>>         if (!amd_sched_ready(sched))
>>>           return NULL;
>>>   +retry:
>>> +    skip = false;
>>>       /* Kernel run queue has higher priority than normal run queue*/
>>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >=
>>> AMD_SCHED_PRIORITY_MIN; i--) {
>>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
>>> +            (sched->sched_rq[i - 1].wait >=
>>> sched->sched_rq[i].wait_base)) {
>>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i -
>>> 1].wait_base;
>>> +            skip = true;
>>> +            continue;
>>> +        }
>>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>>> -        if (entity)
>>> +        if (entity) {
>>> +            if (i > AMD_SCHED_PRIORITY_MIN)
>>> +                sched->sched_rq[i - 1].wait++;
>>>               break;
>>> +        }
>>>       }
>>>   +    if (!entity && skip)
>>> +        goto retry;
>>> +
>>>       return entity;
>>>   }
>>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler
>>> *sched,
>>>       sched->name = name;
>>>       sched->timeout = timeout;
>>>       for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
>>> -        amd_sched_rq_init(&sched->sched_rq[i]);
>>> +        amd_sched_rq_init(sched, i);
>>>         init_waitqueue_head(&sched->wake_up_worker);
>>>       init_waitqueue_head(&sched->job_scheduled);
>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>> index 99f0240..4caed30 100644
>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>>>       spinlock_t        lock;
>>>       struct list_head    entities;
>>>       struct amd_sched_entity    *current_entity;
>>> +    int wait_base;
>>> +    int wait;
>>>   };
>>>     struct amd_sched_fence {
>>
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Andres Rodriguez]

On 2017-03-16 11:13 AM, Andres Rodriguez wrote:
>
>
> On 2017-03-16 05:15 AM, zhoucm1 wrote:
>>
>>
>> On 2017年03月16日 17:10, Christian König wrote:
>>> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
>>>> if high priority rq is full, then process with low priority could be
>>>> starve.
>>>> Add policy for this problem, the high proiority can ahead of next
>>>> priority queue,
>>>> the ratio is 2 : 1.
>>>>
>>>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>
>>> Well, the idea behind the high priority queues is to actually starve
>>> the low priority queues to a certain amount.
>>>
>>> At least for the kernel queue that is really desired.
>> Yes, agree, but we certainly don't want  low priority queue is totally
>> dead, which doesn't have chance to run if high priority queue is always
>> full and busy.
>> If without Andres changes, it doesn't matter. But after Andres changes
>> upstream, we need scheduling policy.
>>
>> Regards,
>> David Zhou
>
> Hey David,
>
> The desired behavior is actually starvation. This is why allocating a
> high priority context is locked behind root privileges at the moment.
>
> High priority work includes many different features intended for the
> safety of the user wearing the headset. These include:
>     - stopping the user from tripping over furniture
>     - preventing motion sickness
>
> We cannot compromise these safety features in order to run non-critical
> tasks.
>
> The current approach also has the disadvantage of heavily interleaving
> work. This is going to have two undesired side effects. First, there
> will be a performance overhead from having the queue context switch so
> often.
>
> Second, this approach improves concurrency of work in a ring with mixed
> priority work, but actually decreases overall system concurrency. When a
> ring's HW queue has any high priority work committed, the whole ring
> will be executing at high priority. So interleaving the work will result
> in extending the time a ring spends in high priority mode. This
> effectively extends time that a ring might spend with CU's reserved
> which will have a performance impact on other rings.
>
> The best approach here is to make sure we don't map high priority work
> and regular priority work to the same ring. This is why the LRU policy
> for userspace ring ids to kernel ring ids was introduced. This policy
> provides the guarantee as a side effect.

Wanted to correct myself here. The LRU policy doesn't actually provide a 
guarantee. It approximates it.

Regards,
Andres

> But further work there could be
> useful to actually check context priorities when doing the ring mapping.
>
> By placing work on different rings, we let the hardware actually
> dispatch work according to wave parameters like VGPR usage, etc. Trying
> to deduce all this information in the GPU scheduler will get very
> complicated.
>
> This is NAK'd by me.
>
> Regards,
> Andres
>
>
>>>
>>> Christian.
>>>
>>>> ---
>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26
>>>> +++++++++++++++++++++++---
>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>>>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>> index 0f439dd..4637b6f 100644
>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>> @@ -35,11 +35,16 @@
>>>>   static void amd_sched_process_job(struct fence *f, struct fence_cb
>>>> *cb);
>>>>     /* Initialize a given run queue struct */
>>>> -static void amd_sched_rq_init(struct amd_sched_rq *rq)
>>>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
>>>> +                  amd_sched_priority pri)
>>>>   {
>>>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
>>>> +
>>>>       spin_lock_init(&rq->lock);
>>>>       INIT_LIST_HEAD(&rq->entities);
>>>>       rq->current_entity = NULL;
>>>> +    rq->wait_base = pri * 2;
>>>> +    rq->wait = rq->wait_base;
>>>>   }
>>>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
>>>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct
>>>> amd_gpu_scheduler *sched)
>>>>   {
>>>>       struct amd_sched_entity *entity;
>>>>       int i;
>>>> +    bool skip;
>>>>         if (!amd_sched_ready(sched))
>>>>           return NULL;
>>>>   +retry:
>>>> +    skip = false;
>>>>       /* Kernel run queue has higher priority than normal run queue*/
>>>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >=
>>>> AMD_SCHED_PRIORITY_MIN; i--) {
>>>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
>>>> +            (sched->sched_rq[i - 1].wait >=
>>>> sched->sched_rq[i].wait_base)) {
>>>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i -
>>>> 1].wait_base;
>>>> +            skip = true;
>>>> +            continue;
>>>> +        }
>>>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>>>> -        if (entity)
>>>> +        if (entity) {
>>>> +            if (i > AMD_SCHED_PRIORITY_MIN)
>>>> +                sched->sched_rq[i - 1].wait++;
>>>>               break;
>>>> +        }
>>>>       }
>>>>   +    if (!entity && skip)
>>>> +        goto retry;
>>>> +
>>>>       return entity;
>>>>   }
>>>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler
>>>> *sched,
>>>>       sched->name = name;
>>>>       sched->timeout = timeout;
>>>>       for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
>>>> -        amd_sched_rq_init(&sched->sched_rq[i]);
>>>> +        amd_sched_rq_init(sched, i);
>>>>         init_waitqueue_head(&sched->wake_up_worker);
>>>>       init_waitqueue_head(&sched->job_scheduled);
>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>> index 99f0240..4caed30 100644
>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>>>>       spinlock_t        lock;
>>>>       struct list_head    entities;
>>>>       struct amd_sched_entity    *current_entity;
>>>> +    int wait_base;
>>>> +    int wait;
>>>>   };
>>>>     struct amd_sched_fence {
>>>
>>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Christian König]

Am 16.03.2017 um 16:31 schrieb Andres Rodriguez:
>
>
> On 2017-03-16 11:13 AM, Andres Rodriguez wrote:
>>
>>
>> On 2017-03-16 05:15 AM, zhoucm1 wrote:
>>>
>>>
>>> On 2017年03月16日 17:10, Christian König wrote:
>>>> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
>>>>> if high priority rq is full, then process with low priority could be
>>>>> starve.
>>>>> Add policy for this problem, the high proiority can ahead of next
>>>>> priority queue,
>>>>> the ratio is 2 : 1.
>>>>>
>>>>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
>>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>>
>>>> Well, the idea behind the high priority queues is to actually starve
>>>> the low priority queues to a certain amount.
>>>>
>>>> At least for the kernel queue that is really desired.
>>> Yes, agree, but we certainly don't want  low priority queue is totally
>>> dead, which doesn't have chance to run if high priority queue is always
>>> full and busy.
>>> If without Andres changes, it doesn't matter. But after Andres changes
>>> upstream, we need scheduling policy.
>>>
>>> Regards,
>>> David Zhou
>>
>> Hey David,
>>
>> The desired behavior is actually starvation. This is why allocating a
>> high priority context is locked behind root privileges at the moment.

Thanks for the quick response, and yes that's what I was expecting as 
requirement for that feature as well.

>>
>> High priority work includes many different features intended for the
>> safety of the user wearing the headset. These include:
>>     - stopping the user from tripping over furniture
>>     - preventing motion sickness
>>
>> We cannot compromise these safety features in order to run non-critical
>> tasks.
>>
>> The current approach also has the disadvantage of heavily interleaving
>> work. This is going to have two undesired side effects. First, there
>> will be a performance overhead from having the queue context switch so
>> often.
>>
>> Second, this approach improves concurrency of work in a ring with mixed
>> priority work, but actually decreases overall system concurrency. When a
>> ring's HW queue has any high priority work committed, the whole ring
>> will be executing at high priority. So interleaving the work will result
>> in extending the time a ring spends in high priority mode. This
>> effectively extends time that a ring might spend with CU's reserved
>> which will have a performance impact on other rings.
>>
>> The best approach here is to make sure we don't map high priority work
>> and regular priority work to the same ring. This is why the LRU policy
>> for userspace ring ids to kernel ring ids was introduced. This policy
>> provides the guarantee as a side effect.
>
> Wanted to correct myself here. The LRU policy doesn't actually provide 
> a guarantee. It approximates it.

What David is perfectly right with is that this has the potential for 
undesired side effects.

But I completely agree that deadline or fair queue scheduling is tricky 
to implement even when it is desired.

So letting a submission dominate all other is perfectly ok for me as 
long as it is limited to a certain set of processes somehow.

Christian.

>
> Regards,
> Andres
>
>> But further work there could be
>> useful to actually check context priorities when doing the ring mapping.
>>
>> By placing work on different rings, we let the hardware actually
>> dispatch work according to wave parameters like VGPR usage, etc. Trying
>> to deduce all this information in the GPU scheduler will get very
>> complicated.
>>
>> This is NAK'd by me.
>>
>> Regards,
>> Andres
>>
>>
>>>>
>>>> Christian.
>>>>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26
>>>>> +++++++++++++++++++++++---
>>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>>>>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>> index 0f439dd..4637b6f 100644
>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>> @@ -35,11 +35,16 @@
>>>>>   static void amd_sched_process_job(struct fence *f, struct fence_cb
>>>>> *cb);
>>>>>     /* Initialize a given run queue struct */
>>>>> -static void amd_sched_rq_init(struct amd_sched_rq *rq)
>>>>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
>>>>> +                  amd_sched_priority pri)
>>>>>   {
>>>>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
>>>>> +
>>>>>       spin_lock_init(&rq->lock);
>>>>>       INIT_LIST_HEAD(&rq->entities);
>>>>>       rq->current_entity = NULL;
>>>>> +    rq->wait_base = pri * 2;
>>>>> +    rq->wait = rq->wait_base;
>>>>>   }
>>>>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
>>>>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct
>>>>> amd_gpu_scheduler *sched)
>>>>>   {
>>>>>       struct amd_sched_entity *entity;
>>>>>       int i;
>>>>> +    bool skip;
>>>>>         if (!amd_sched_ready(sched))
>>>>>           return NULL;
>>>>>   +retry:
>>>>> +    skip = false;
>>>>>       /* Kernel run queue has higher priority than normal run queue*/
>>>>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >=
>>>>> AMD_SCHED_PRIORITY_MIN; i--) {
>>>>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
>>>>> +            (sched->sched_rq[i - 1].wait >=
>>>>> sched->sched_rq[i].wait_base)) {
>>>>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i -
>>>>> 1].wait_base;
>>>>> +            skip = true;
>>>>> +            continue;
>>>>> +        }
>>>>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>>>>> -        if (entity)
>>>>> +        if (entity) {
>>>>> +            if (i > AMD_SCHED_PRIORITY_MIN)
>>>>> +                sched->sched_rq[i - 1].wait++;
>>>>>               break;
>>>>> +        }
>>>>>       }
>>>>>   +    if (!entity && skip)
>>>>> +        goto retry;
>>>>> +
>>>>>       return entity;
>>>>>   }
>>>>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler
>>>>> *sched,
>>>>>       sched->name = name;
>>>>>       sched->timeout = timeout;
>>>>>       for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; 
>>>>> i++)
>>>>> -        amd_sched_rq_init(&sched->sched_rq[i]);
>>>>> +        amd_sched_rq_init(sched, i);
>>>>> init_waitqueue_head(&sched->wake_up_worker);
>>>>>       init_waitqueue_head(&sched->job_scheduled);
>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>> index 99f0240..4caed30 100644
>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>>>>>       spinlock_t        lock;
>>>>>       struct list_head    entities;
>>>>>       struct amd_sched_entity    *current_entity;
>>>>> +    int wait_base;
>>>>> +    int wait;
>>>>>   };
>>>>>     struct amd_sched_fence {
>>>>
>>>>
>>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Andres Rodriguez]

On 2017-03-16 12:01 PM, Christian König wrote:
> Am 16.03.2017 um 16:31 schrieb Andres Rodriguez:
>>
>>
>> On 2017-03-16 11:13 AM, Andres Rodriguez wrote:
>>>
>>>
>>> On 2017-03-16 05:15 AM, zhoucm1 wrote:
>>>>
>>>>
>>>> On 2017年03月16日 17:10, Christian König wrote:
>>>>> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
>>>>>> if high priority rq is full, then process with low priority could be
>>>>>> starve.
>>>>>> Add policy for this problem, the high proiority can ahead of next
>>>>>> priority queue,
>>>>>> the ratio is 2 : 1.
>>>>>>
>>>>>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
>>>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>>>
>>>>> Well, the idea behind the high priority queues is to actually starve
>>>>> the low priority queues to a certain amount.
>>>>>
>>>>> At least for the kernel queue that is really desired.
>>>> Yes, agree, but we certainly don't want  low priority queue is totally
>>>> dead, which doesn't have chance to run if high priority queue is always
>>>> full and busy.
>>>> If without Andres changes, it doesn't matter. But after Andres changes
>>>> upstream, we need scheduling policy.
>>>>
>>>> Regards,
>>>> David Zhou
>>>
>>> Hey David,
>>>
>>> The desired behavior is actually starvation. This is why allocating a
>>> high priority context is locked behind root privileges at the moment.
>
> Thanks for the quick response, and yes that's what I was expecting as
> requirement for that feature as well.
>
>>>
>>> High priority work includes many different features intended for the
>>> safety of the user wearing the headset. These include:
>>>     - stopping the user from tripping over furniture
>>>     - preventing motion sickness
>>>
>>> We cannot compromise these safety features in order to run non-critical
>>> tasks.
>>>
>>> The current approach also has the disadvantage of heavily interleaving
>>> work. This is going to have two undesired side effects. First, there
>>> will be a performance overhead from having the queue context switch so
>>> often.
>>>
>>> Second, this approach improves concurrency of work in a ring with mixed
>>> priority work, but actually decreases overall system concurrency. When a
>>> ring's HW queue has any high priority work committed, the whole ring
>>> will be executing at high priority. So interleaving the work will result
>>> in extending the time a ring spends in high priority mode. This
>>> effectively extends time that a ring might spend with CU's reserved
>>> which will have a performance impact on other rings.
>>>
>>> The best approach here is to make sure we don't map high priority work
>>> and regular priority work to the same ring. This is why the LRU policy
>>> for userspace ring ids to kernel ring ids was introduced. This policy
>>> provides the guarantee as a side effect.
>>
>> Wanted to correct myself here. The LRU policy doesn't actually provide
>> a guarantee. It approximates it.
>
> What David is perfectly right with is that this has the potential for
> undesired side effects.
>
> But I completely agree that deadline or fair queue scheduling is tricky
> to implement even when it is desired.
>
> So letting a submission dominate all other is perfectly ok for me as
> long as it is limited to a certain set of processes somehow.
>
> Christian.

I also wanted to comment that I think policies for the gpu_scheduler are 
something interesting to explore. Not just at single ring level, but 
also policies that tie all the gpu_schedulers for a single ASIC 
together. Currently they all operate independently, but technically they 
share the underlying HW so there is some resource aliasing.

There has been a lot of experimentation on this area in the VR world by 
other OS vendors. Enhancing the policies provided by the SPI through SW 
can have some pretty great benefits. Although because of restrictions 
these kind of changes are only activated for certain HMDs.

I think we have a good opportunity here to make the linux version of VR 
better for all HMDs, and not just the ones associated with the OS vendor.

Regards,
Andres

>
>>
>> Regards,
>> Andres
>>
>>> But further work there could be
>>> useful to actually check context priorities when doing the ring mapping.
>>>
>>> By placing work on different rings, we let the hardware actually
>>> dispatch work according to wave parameters like VGPR usage, etc. Trying
>>> to deduce all this information in the GPU scheduler will get very
>>> complicated.
>>>
>>> This is NAK'd by me.
>>>
>>> Regards,
>>> Andres
>>>
>>>
>>>>>
>>>>> Christian.
>>>>>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26
>>>>>> +++++++++++++++++++++++---
>>>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>>>>>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> index 0f439dd..4637b6f 100644
>>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> @@ -35,11 +35,16 @@
>>>>>>   static void amd_sched_process_job(struct fence *f, struct fence_cb
>>>>>> *cb);
>>>>>>     /* Initialize a given run queue struct */
>>>>>> -static void amd_sched_rq_init(struct amd_sched_rq *rq)
>>>>>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
>>>>>> +                  amd_sched_priority pri)
>>>>>>   {
>>>>>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
>>>>>> +
>>>>>>       spin_lock_init(&rq->lock);
>>>>>>       INIT_LIST_HEAD(&rq->entities);
>>>>>>       rq->current_entity = NULL;
>>>>>> +    rq->wait_base = pri * 2;
>>>>>> +    rq->wait = rq->wait_base;
>>>>>>   }
>>>>>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
>>>>>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct
>>>>>> amd_gpu_scheduler *sched)
>>>>>>   {
>>>>>>       struct amd_sched_entity *entity;
>>>>>>       int i;
>>>>>> +    bool skip;
>>>>>>         if (!amd_sched_ready(sched))
>>>>>>           return NULL;
>>>>>>   +retry:
>>>>>> +    skip = false;
>>>>>>       /* Kernel run queue has higher priority than normal run queue*/
>>>>>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >=
>>>>>> AMD_SCHED_PRIORITY_MIN; i--) {
>>>>>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
>>>>>> +            (sched->sched_rq[i - 1].wait >=
>>>>>> sched->sched_rq[i].wait_base)) {
>>>>>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i -
>>>>>> 1].wait_base;
>>>>>> +            skip = true;
>>>>>> +            continue;
>>>>>> +        }
>>>>>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>>>>>> -        if (entity)
>>>>>> +        if (entity) {
>>>>>> +            if (i > AMD_SCHED_PRIORITY_MIN)
>>>>>> +                sched->sched_rq[i - 1].wait++;
>>>>>>               break;
>>>>>> +        }
>>>>>>       }
>>>>>>   +    if (!entity && skip)
>>>>>> +        goto retry;
>>>>>> +
>>>>>>       return entity;
>>>>>>   }
>>>>>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler
>>>>>> *sched,
>>>>>>       sched->name = name;
>>>>>>       sched->timeout = timeout;
>>>>>>       for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX;
>>>>>> i++)
>>>>>> -        amd_sched_rq_init(&sched->sched_rq[i]);
>>>>>> +        amd_sched_rq_init(sched, i);
>>>>>> init_waitqueue_head(&sched->wake_up_worker);
>>>>>>       init_waitqueue_head(&sched->job_scheduled);
>>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> index 99f0240..4caed30 100644
>>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>>>>>>       spinlock_t        lock;
>>>>>>       struct list_head    entities;
>>>>>>       struct amd_sched_entity    *current_entity;
>>>>>> +    int wait_base;
>>>>>> +    int wait;
>>>>>>   };
>>>>>>     struct amd_sched_fence {
>>>>>
>>>>>
>>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Zhou, David(ChunMing)]

Hi Christian, Andres,

I understand your mean, especially high priority queue from Andres is for VR, which is liking an engine to himself, but our scheduler is a generic thing, which is used by multiple case.
From scheduler perspective, scheduling policy is popular, e.g. CPU scheduler, it provides different time piece for different priority queue, high priority has long time to execute, low has short time, but everyone has the chance to run, just sooner or later.

The proposal I send is same goal for gpu scheduler, we don't desire to one engine is occupied by one person, high priority can have more chance to run, low priority has less, but low priority must to have. The ratio is power of 2 in current implementation, we can adjust it for more different situations if necessary. Even we can put a parameter to be adusted convenience. 
For Andres case, he can pass a big time piece to it so that the efficiency is same with occupied previous.

Regards,
David Zhou


-----Original Message-----
From: Andres Rodriguez [mailto:andresx7@gmail.com] 
Sent: Friday, March 17, 2017 12:32 AM
To: Christian König <deathsimple@vodafone.de>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amd/sched: add schuduling policy



On 2017-03-16 12:01 PM, Christian König wrote:
> Am 16.03.2017 um 16:31 schrieb Andres Rodriguez:
>>
>>
>> On 2017-03-16 11:13 AM, Andres Rodriguez wrote:
>>>
>>>
>>> On 2017-03-16 05:15 AM, zhoucm1 wrote:
>>>>
>>>>
>>>> On 2017年03月16日 17:10, Christian König wrote:
>>>>> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
>>>>>> if high priority rq is full, then process with low priority could 
>>>>>> be starve.
>>>>>> Add policy for this problem, the high proiority can ahead of next 
>>>>>> priority queue, the ratio is 2 : 1.
>>>>>>
>>>>>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
>>>>>> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
>>>>>
>>>>> Well, the idea behind the high priority queues is to actually 
>>>>> starve the low priority queues to a certain amount.
>>>>>
>>>>> At least for the kernel queue that is really desired.
>>>> Yes, agree, but we certainly don't want  low priority queue is 
>>>> totally dead, which doesn't have chance to run if high priority 
>>>> queue is always full and busy.
>>>> If without Andres changes, it doesn't matter. But after Andres 
>>>> changes upstream, we need scheduling policy.
>>>>
>>>> Regards,
>>>> David Zhou
>>>
>>> Hey David,
>>>
>>> The desired behavior is actually starvation. This is why allocating 
>>> a high priority context is locked behind root privileges at the moment.
>
> Thanks for the quick response, and yes that's what I was expecting as 
> requirement for that feature as well.
>
>>>
>>> High priority work includes many different features intended for the 
>>> safety of the user wearing the headset. These include:
>>>     - stopping the user from tripping over furniture
>>>     - preventing motion sickness
>>>
>>> We cannot compromise these safety features in order to run 
>>> non-critical tasks.
>>>
>>> The current approach also has the disadvantage of heavily 
>>> interleaving work. This is going to have two undesired side effects. 
>>> First, there will be a performance overhead from having the queue 
>>> context switch so often.
>>>
>>> Second, this approach improves concurrency of work in a ring with 
>>> mixed priority work, but actually decreases overall system 
>>> concurrency. When a ring's HW queue has any high priority work 
>>> committed, the whole ring will be executing at high priority. So 
>>> interleaving the work will result in extending the time a ring 
>>> spends in high priority mode. This effectively extends time that a 
>>> ring might spend with CU's reserved which will have a performance impact on other rings.
>>>
>>> The best approach here is to make sure we don't map high priority 
>>> work and regular priority work to the same ring. This is why the LRU 
>>> policy for userspace ring ids to kernel ring ids was introduced. 
>>> This policy provides the guarantee as a side effect.
>>
>> Wanted to correct myself here. The LRU policy doesn't actually 
>> provide a guarantee. It approximates it.
>
> What David is perfectly right with is that this has the potential for 
> undesired side effects.
>
> But I completely agree that deadline or fair queue scheduling is 
> tricky to implement even when it is desired.
>
> So letting a submission dominate all other is perfectly ok for me as 
> long as it is limited to a certain set of processes somehow.
>
> Christian.

I also wanted to comment that I think policies for the gpu_scheduler are something interesting to explore. Not just at single ring level, but also policies that tie all the gpu_schedulers for a single ASIC together. Currently they all operate independently, but technically they share the underlying HW so there is some resource aliasing.

There has been a lot of experimentation on this area in the VR world by other OS vendors. Enhancing the policies provided by the SPI through SW can have some pretty great benefits. Although because of restrictions these kind of changes are only activated for certain HMDs.

I think we have a good opportunity here to make the linux version of VR better for all HMDs, and not just the ones associated with the OS vendor.

Regards,
Andres

>
>>
>> Regards,
>> Andres
>>
>>> But further work there could be
>>> useful to actually check context priorities when doing the ring mapping.
>>>
>>> By placing work on different rings, we let the hardware actually 
>>> dispatch work according to wave parameters like VGPR usage, etc. 
>>> Trying to deduce all this information in the GPU scheduler will get 
>>> very complicated.
>>>
>>> This is NAK'd by me.
>>>
>>> Regards,
>>> Andres
>>>
>>>
>>>>>
>>>>> Christian.
>>>>>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26
>>>>>> +++++++++++++++++++++++---
>>>>>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
>>>>>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> index 0f439dd..4637b6f 100644
>>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
>>>>>> @@ -35,11 +35,16 @@
>>>>>>   static void amd_sched_process_job(struct fence *f, struct 
>>>>>> fence_cb *cb);
>>>>>>     /* Initialize a given run queue struct */ -static void 
>>>>>> amd_sched_rq_init(struct amd_sched_rq *rq)
>>>>>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, enum
>>>>>> +                  amd_sched_priority pri)
>>>>>>   {
>>>>>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
>>>>>> +
>>>>>>       spin_lock_init(&rq->lock);
>>>>>>       INIT_LIST_HEAD(&rq->entities);
>>>>>>       rq->current_entity = NULL;
>>>>>> +    rq->wait_base = pri * 2;
>>>>>> +    rq->wait = rq->wait_base;
>>>>>>   }
>>>>>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq, 
>>>>>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct 
>>>>>> amd_gpu_scheduler *sched)
>>>>>>   {
>>>>>>       struct amd_sched_entity *entity;
>>>>>>       int i;
>>>>>> +    bool skip;
>>>>>>         if (!amd_sched_ready(sched))
>>>>>>           return NULL;
>>>>>>   +retry:
>>>>>> +    skip = false;
>>>>>>       /* Kernel run queue has higher priority than normal run queue*/
>>>>>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= 
>>>>>> AMD_SCHED_PRIORITY_MIN; i--) {
>>>>>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
>>>>>> +            (sched->sched_rq[i - 1].wait >=
>>>>>> sched->sched_rq[i].wait_base)) {
>>>>>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i -
>>>>>> 1].wait_base;
>>>>>> +            skip = true;
>>>>>> +            continue;
>>>>>> +        }
>>>>>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
>>>>>> -        if (entity)
>>>>>> +        if (entity) {
>>>>>> +            if (i > AMD_SCHED_PRIORITY_MIN)
>>>>>> +                sched->sched_rq[i - 1].wait++;
>>>>>>               break;
>>>>>> +        }
>>>>>>       }
>>>>>>   +    if (!entity && skip)
>>>>>> +        goto retry;
>>>>>> +
>>>>>>       return entity;
>>>>>>   }
>>>>>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler 
>>>>>> *sched,
>>>>>>       sched->name = name;
>>>>>>       sched->timeout = timeout;
>>>>>>       for (i = AMD_SCHED_PRIORITY_MIN; i < 
>>>>>> AMD_SCHED_PRIORITY_MAX;
>>>>>> i++)
>>>>>> -        amd_sched_rq_init(&sched->sched_rq[i]);
>>>>>> +        amd_sched_rq_init(sched, i);
>>>>>> init_waitqueue_head(&sched->wake_up_worker);
>>>>>>       init_waitqueue_head(&sched->job_scheduled);
>>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> index 99f0240..4caed30 100644
>>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>>>>>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
>>>>>>       spinlock_t        lock;
>>>>>>       struct list_head    entities;
>>>>>>       struct amd_sched_entity    *current_entity;
>>>>>> +    int wait_base;
>>>>>> +    int wait;
>>>>>>   };
>>>>>>     struct amd_sched_fence {
>>>>>
>>>>>
>>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Michel Dänzer]

On 17/03/17 11:35 AM, Zhou, David(ChunMing) wrote:
> Hi Christian, Andres,
> 
> I understand your mean, especially high priority queue from Andres is
> for VR, which is liking an engine to himself, but our scheduler is a
> generic thing, which is used by multiple case. From scheduler
> perspective, scheduling policy is popular, e.g. CPU scheduler, it
> provides different time piece for different priority queue, high
> priority has long time to execute, low has short time, but everyone
> has the chance to run, just sooner or later.

The CPU scheduler supports the SCHED_FIFO scheduling policy, which
doesn't preempt a running process. It's only available to root.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/sched: add schuduling policy

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 10573 bytes --]

Am 17.03.2017 um 03:35 schrieb Zhou, David(ChunMing):
> Hi Christian, Andres,
>
> I understand your mean, especially high priority queue from Andres is 
> for VR, which is liking an engine to himself, but our scheduler is a 
> generic thing, which is used by multiple case.
> From scheduler perspective, scheduling policy is popular, e.g. CPU 
> scheduler, it provides different time piece for different priority 
> queue, high priority has long time to execute, low has short time, but 
> everyone has the chance to run, just sooner or later.
>

As Michel pointed out even the CPU scheduler allows for a process to 
dominate everyone else when you have the right privileges and is useful 
for the use case.

> The proposal I send is same goal for gpu scheduler, we don't desire to 
> one engine is occupied by one person, high priority can have more 
> chance to run, low priority has less, but low priority must to have. 
> The ratio is power of 2 in current implementation, we can adjust it 
> for more different situations if necessary. Even we can put a 
> parameter to be adusted convenience.
> For Andres case, he can pass a big time piece to it so that the 
> efficiency is same with occupied previous.

We perfectly understand what your patch is doing, it's just not that we 
want this behavior.

When there is high priority work available it should completely dominate 
everything else.

Regards,
Christian.

>
> Regards,
> David Zhou
>
>
> -----Original Message-----
> From: Andres Rodriguez [mailto:andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> Sent: Friday, March 17, 2017 12:32 AM
> To: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>; Zhou, David(ChunMing) 
> <David1.Zhou-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; 
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Subject: Re: [PATCH 2/2] drm/amd/sched: add schuduling policy
>
>
>
> On 2017-03-16 12:01 PM, Christian König wrote:
> > Am 16.03.2017 um 16:31 schrieb Andres Rodriguez:
> >>
> >>
> >> On 2017-03-16 11:13 AM, Andres Rodriguez wrote:
> >>>
> >>>
> >>> On 2017-03-16 05:15 AM, zhoucm1 wrote:
> >>>>
> >>>>
> >>>> On 2017年03月16日 17:10, Christian König wrote:
> >>>>> Am 16.03.2017 um 10:00 schrieb Chunming Zhou:
> >>>>>> if high priority rq is full, then process with low priority could
> >>>>>> be starve.
> >>>>>> Add policy for this problem, the high proiority can ahead of next
> >>>>>> priority queue, the ratio is 2 : 1.
> >>>>>>
> >>>>>> Change-Id: I58f4a6b9cdce8689b18dd8e83dd6e2cf5f99d5fb
> >>>>>> Signed-off-by: Chunming Zhou <David1.Zhou-5C7GfCeVMHo@public.gmane.org>
> >>>>>
> >>>>> Well, the idea behind the high priority queues is to actually
> >>>>> starve the low priority queues to a certain amount.
> >>>>>
> >>>>> At least for the kernel queue that is really desired.
> >>>> Yes, agree, but we certainly don't want low priority queue is
> >>>> totally dead, which doesn't have chance to run if high priority
> >>>> queue is always full and busy.
> >>>> If without Andres changes, it doesn't matter. But after Andres
> >>>> changes upstream, we need scheduling policy.
> >>>>
> >>>> Regards,
> >>>> David Zhou
> >>>
> >>> Hey David,
> >>>
> >>> The desired behavior is actually starvation. This is why allocating
> >>> a high priority context is locked behind root privileges at the 
> moment.
> >
> > Thanks for the quick response, and yes that's what I was expecting as
> > requirement for that feature as well.
> >
> >>>
> >>> High priority work includes many different features intended for the
> >>> safety of the user wearing the headset. These include:
> >>>     - stopping the user from tripping over furniture
> >>>     - preventing motion sickness
> >>>
> >>> We cannot compromise these safety features in order to run
> >>> non-critical tasks.
> >>>
> >>> The current approach also has the disadvantage of heavily
> >>> interleaving work. This is going to have two undesired side effects.
> >>> First, there will be a performance overhead from having the queue
> >>> context switch so often.
> >>>
> >>> Second, this approach improves concurrency of work in a ring with
> >>> mixed priority work, but actually decreases overall system
> >>> concurrency. When a ring's HW queue has any high priority work
> >>> committed, the whole ring will be executing at high priority. So
> >>> interleaving the work will result in extending the time a ring
> >>> spends in high priority mode. This effectively extends time that a
> >>> ring might spend with CU's reserved which will have a performance 
> impact on other rings.
> >>>
> >>> The best approach here is to make sure we don't map high priority
> >>> work and regular priority work to the same ring. This is why the LRU
> >>> policy for userspace ring ids to kernel ring ids was introduced.
> >>> This policy provides the guarantee as a side effect.
> >>
> >> Wanted to correct myself here. The LRU policy doesn't actually
> >> provide a guarantee. It approximates it.
> >
> > What David is perfectly right with is that this has the potential for
> > undesired side effects.
> >
> > But I completely agree that deadline or fair queue scheduling is
> > tricky to implement even when it is desired.
> >
> > So letting a submission dominate all other is perfectly ok for me as
> > long as it is limited to a certain set of processes somehow.
> >
> > Christian.
>
> I also wanted to comment that I think policies for the gpu_scheduler 
> are something interesting to explore. Not just at single ring level, 
> but also policies that tie all the gpu_schedulers for a single ASIC 
> together. Currently they all operate independently, but technically 
> they share the underlying HW so there is some resource aliasing.
>
> There has been a lot of experimentation on this area in the VR world 
> by other OS vendors. Enhancing the policies provided by the SPI 
> through SW can have some pretty great benefits. Although because of 
> restrictions these kind of changes are only activated for certain HMDs.
>
> I think we have a good opportunity here to make the linux version of 
> VR better for all HMDs, and not just the ones associated with the OS 
> vendor.
>
> Regards,
> Andres
>
> >
> >>
> >> Regards,
> >> Andres
> >>
> >>> But further work there could be
> >>> useful to actually check context priorities when doing the ring 
> mapping.
> >>>
> >>> By placing work on different rings, we let the hardware actually
> >>> dispatch work according to wave parameters like VGPR usage, etc.
> >>> Trying to deduce all this information in the GPU scheduler will get
> >>> very complicated.
> >>>
> >>> This is NAK'd by me.
> >>>
> >>> Regards,
> >>> Andres
> >>>
> >>>
> >>>>>
> >>>>> Christian.
> >>>>>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 26
> >>>>>> +++++++++++++++++++++++---
> >>>>>> drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  2 ++
> >>>>>>   2 files changed, 25 insertions(+), 3 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> >>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> >>>>>> index 0f439dd..4637b6f 100644
> >>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> >>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> >>>>>> @@ -35,11 +35,16 @@
> >>>>>>   static void amd_sched_process_job(struct fence *f, struct
> >>>>>> fence_cb *cb);
> >>>>>>     /* Initialize a given run queue struct */ -static void
> >>>>>> amd_sched_rq_init(struct amd_sched_rq *rq)
> >>>>>> +static void amd_sched_rq_init(struct amd_gpu_scheduler *sched, 
> enum
> >>>>>> + amd_sched_priority pri)
> >>>>>>   {
> >>>>>> +    struct amd_sched_rq *rq = &sched->sched_rq[pri];
> >>>>>> +
> >>>>>> spin_lock_init(&rq->lock);
> >>>>>> INIT_LIST_HEAD(&rq->entities);
> >>>>>>       rq->current_entity = NULL;
> >>>>>> +    rq->wait_base = pri * 2;
> >>>>>> +    rq->wait = rq->wait_base;
> >>>>>>   }
> >>>>>>     static void amd_sched_rq_add_entity(struct amd_sched_rq *rq,
> >>>>>> @@ -494,17 +499,32 @@ static void amd_sched_wakeup(struct
> >>>>>> amd_gpu_scheduler *sched)
> >>>>>>   {
> >>>>>>       struct amd_sched_entity *entity;
> >>>>>>       int i;
> >>>>>> +    bool skip;
> >>>>>>         if (!amd_sched_ready(sched))
> >>>>>>           return NULL;
> >>>>>>   +retry:
> >>>>>> +    skip = false;
> >>>>>>       /* Kernel run queue has higher priority than normal run 
> queue*/
> >>>>>>       for (i = AMD_SCHED_PRIORITY_MAX - 1; i >=
> >>>>>> AMD_SCHED_PRIORITY_MIN; i--) {
> >>>>>> +        if ((i > AMD_SCHED_PRIORITY_MIN) &&
> >>>>>> +            (sched->sched_rq[i - 1].wait >=
> >>>>>> sched->sched_rq[i].wait_base)) {
> >>>>>> +            sched->sched_rq[i - 1].wait = sched->sched_rq[i -
> >>>>>> 1].wait_base;
> >>>>>> +            skip = true;
> >>>>>> +            continue;
> >>>>>> +        }
> >>>>>>           entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
> >>>>>> -        if (entity)
> >>>>>> +        if (entity) {
> >>>>>> +            if (i > AMD_SCHED_PRIORITY_MIN)
> >>>>>> + sched->sched_rq[i - 1].wait++;
> >>>>>>               break;
> >>>>>> +        }
> >>>>>>       }
> >>>>>>   +    if (!entity && skip)
> >>>>>> +        goto retry;
> >>>>>> +
> >>>>>>       return entity;
> >>>>>>   }
> >>>>>>   @@ -608,7 +628,7 @@ int amd_sched_init(struct amd_gpu_scheduler
> >>>>>> *sched,
> >>>>>>       sched->name = name;
> >>>>>>       sched->timeout = timeout;
> >>>>>>       for (i = AMD_SCHED_PRIORITY_MIN; i <
> >>>>>> AMD_SCHED_PRIORITY_MAX;
> >>>>>> i++)
> >>>>>> - amd_sched_rq_init(&sched->sched_rq[i]);
> >>>>>> +        amd_sched_rq_init(sched, i);
> >>>>>> init_waitqueue_head(&sched->wake_up_worker);
> >>>>>> init_waitqueue_head(&sched->job_scheduled);
> >>>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> >>>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> >>>>>> index 99f0240..4caed30 100644
> >>>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> >>>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
> >>>>>> @@ -64,6 +64,8 @@ struct amd_sched_rq {
> >>>>>>       spinlock_t        lock;
> >>>>>>       struct list_head    entities;
> >>>>>>       struct amd_sched_entity *current_entity;
> >>>>>> +    int wait_base;
> >>>>>> +    int wait;
> >>>>>>   };
> >>>>>>     struct amd_sched_fence {
> >>>>>
> >>>>>
> >>>>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> >



[-- Attachment #1.2: Type: text/html, Size: 20557 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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