diff mbox series

[01/10] drm/sched: move calling drm_sched_entity_select_rq

Message ID 20220714103902.7084-2-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [01/10] drm/sched: move calling drm_sched_entity_select_rq | expand

Commit Message

Christian König July 14, 2022, 10:38 a.m. UTC
We already discussed that the call to drm_sched_entity_select_rq() needs
to move to drm_sched_job_arm() to be able to set a new scheduler list
between _init() and _arm(). This was just not applied for some reason.

Signed-off-by: Christian König <christian.koenig@amd.com>
CC: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
CC: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/scheduler/sched_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Andrey Grodzovsky July 14, 2022, 3:43 p.m. UTC | #1
Can you please remind me of the use case that requires this ? I browsed 
through
related mails in the past but haven't found when is that needed. For amdgpu
drm_sched_job_init and drm_sched_job_arm are called together and amdgpu
is the only one who supports modifying entity priority on the fly as far 
as i see.

Andrey

On 2022-07-14 06:38, Christian König wrote:
> We already discussed that the call to drm_sched_entity_select_rq() needs
> to move to drm_sched_job_arm() to be able to set a new scheduler list
> between _init() and _arm(). This was just not applied for some reason.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> CC: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> CC: dri-devel@lists.freedesktop.org
> ---
>   drivers/gpu/drm/scheduler/sched_main.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 68317d3a7a27..e0ab14e0fb6b 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -592,7 +592,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>   		       struct drm_sched_entity *entity,
>   		       void *owner)
>   {
> -	drm_sched_entity_select_rq(entity);
>   	if (!entity->rq)
>   		return -ENOENT;
>   
> @@ -628,7 +627,7 @@ void drm_sched_job_arm(struct drm_sched_job *job)
>   	struct drm_sched_entity *entity = job->entity;
>   
>   	BUG_ON(!entity);
> -
> +	drm_sched_entity_select_rq(entity);
>   	sched = entity->rq->sched;
>   
>   	job->sched = sched;
Christian König July 14, 2022, 4:26 p.m. UTC | #2
We need this for limiting codecs like AV1 to the first instance for VCN3.

Essentially the idea is that we first initialize the job with entity, id 
etc... and before we submit it we select a new rq for the entity. In the 
meantime the VCN3 inline parse will have modified the available rqs for 
the entity.

See the patch "revert "fix limiting AV1 to the first instance on VCN3"" 
as well.

Christian.

Am 14.07.22 um 17:43 schrieb Andrey Grodzovsky:
> Can you please remind me of the use case that requires this ? I 
> browsed through
> related mails in the past but haven't found when is that needed. For 
> amdgpu
> drm_sched_job_init and drm_sched_job_arm are called together and amdgpu
> is the only one who supports modifying entity priority on the fly as 
> far as i see.
>
> Andrey
>
> On 2022-07-14 06:38, Christian König wrote:
>> We already discussed that the call to drm_sched_entity_select_rq() needs
>> to move to drm_sched_job_arm() to be able to set a new scheduler list
>> between _init() and _arm(). This was just not applied for some reason.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> CC: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> CC: dri-devel@lists.freedesktop.org
>> ---
>>   drivers/gpu/drm/scheduler/sched_main.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
>> b/drivers/gpu/drm/scheduler/sched_main.c
>> index 68317d3a7a27..e0ab14e0fb6b 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -592,7 +592,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>>                  struct drm_sched_entity *entity,
>>                  void *owner)
>>   {
>> -    drm_sched_entity_select_rq(entity);
>>       if (!entity->rq)
>>           return -ENOENT;
>>   @@ -628,7 +627,7 @@ void drm_sched_job_arm(struct drm_sched_job *job)
>>       struct drm_sched_entity *entity = job->entity;
>>         BUG_ON(!entity);
>> -
>> +    drm_sched_entity_select_rq(entity);
>>       sched = entity->rq->sched;
>>         job->sched = sched;
Andrey Grodzovsky July 14, 2022, 6:25 p.m. UTC | #3
Found the new use case from the 5/10 of reordering CS ioctl.

Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

Andrey

On 2022-07-14 12:26, Christian König wrote:
> We need this for limiting codecs like AV1 to the first instance for VCN3.
>
> Essentially the idea is that we first initialize the job with entity, 
> id etc... and before we submit it we select a new rq for the entity. 
> In the meantime the VCN3 inline parse will have modified the available 
> rqs for the entity.
>
> See the patch "revert "fix limiting AV1 to the first instance on 
> VCN3"" as well.
>
> Christian.
>
> Am 14.07.22 um 17:43 schrieb Andrey Grodzovsky:
>> Can you please remind me of the use case that requires this ? I 
>> browsed through
>> related mails in the past but haven't found when is that needed. For 
>> amdgpu
>> drm_sched_job_init and drm_sched_job_arm are called together and amdgpu
>> is the only one who supports modifying entity priority on the fly as 
>> far as i see.
>>
>> Andrey
>>
>> On 2022-07-14 06:38, Christian König wrote:
>>> We already discussed that the call to drm_sched_entity_select_rq() 
>>> needs
>>> to move to drm_sched_job_arm() to be able to set a new scheduler list
>>> between _init() and _arm(). This was just not applied for some reason.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> CC: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> CC: dri-devel@lists.freedesktop.org
>>> ---
>>>   drivers/gpu/drm/scheduler/sched_main.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
>>> b/drivers/gpu/drm/scheduler/sched_main.c
>>> index 68317d3a7a27..e0ab14e0fb6b 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -592,7 +592,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>>>                  struct drm_sched_entity *entity,
>>>                  void *owner)
>>>   {
>>> -    drm_sched_entity_select_rq(entity);
>>>       if (!entity->rq)
>>>           return -ENOENT;
>>>   @@ -628,7 +627,7 @@ void drm_sched_job_arm(struct drm_sched_job *job)
>>>       struct drm_sched_entity *entity = job->entity;
>>>         BUG_ON(!entity);
>>> -
>>> +    drm_sched_entity_select_rq(entity);
>>>       sched = entity->rq->sched;
>>>         job->sched = sched;
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 68317d3a7a27..e0ab14e0fb6b 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -592,7 +592,6 @@  int drm_sched_job_init(struct drm_sched_job *job,
 		       struct drm_sched_entity *entity,
 		       void *owner)
 {
-	drm_sched_entity_select_rq(entity);
 	if (!entity->rq)
 		return -ENOENT;
 
@@ -628,7 +627,7 @@  void drm_sched_job_arm(struct drm_sched_job *job)
 	struct drm_sched_entity *entity = job->entity;
 
 	BUG_ON(!entity);
-
+	drm_sched_entity_select_rq(entity);
 	sched = entity->rq->sched;
 
 	job->sched = sched;