drm/amdgpu: always initialize job->base.sched

Submitted by Zhou, David(ChunMing) on July 17, 2018, 7:16 a.m.

Details

Message ID BY1PR12MB050253AA5BB29EAAF8AB0D00B45C0@BY1PR12MB0502.namprd12.prod.outlook.com
State New
Headers show
Series "drm/amdgpu: always initialize job->base.sched" ( rev: 2 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Zhou, David(ChunMing) July 17, 2018, 7:16 a.m.
Acked-by: Chunming Zhou <david1.zhou@amd.com>, but I think it isn't a nice evaluation although there is comment in code.



-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Christian K?nig

Sent: Tuesday, July 17, 2018 3:05 PM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: always initialize job->base.sched

Otherwise we can't clean up the job if we run into an error before it is pushed to the scheduler.

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

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 +++++
 1 file changed, 5 insertions(+)

--
2.14.1

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

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 024efb7ea6d6..42a4764d728e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -54,6 +54,11 @@  int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 	if (!*job)
 		return -ENOMEM;
 
+	/*
+	 * Initialize the scheduler to at least some ring so that we always
+	 * have a pointer to adev.
+	 */
+	(*job)->base.sched = &adev->rings[0]->sched;
 	(*job)->vm = vm;
 	(*job)->ibs = (void *)&(*job)[1];
 	(*job)->num_ibs = num_ibs;

Comments

Am 17.07.2018 um 09:16 schrieb Zhou, David(ChunMing):
> Acked-by: Chunming Zhou <david1.zhou@amd.com>, but I think it isn't a nice evaluation although there is comment in code.

Yeah, I didn't thought about the possibility that we need to free the 
job before it is submitted (in other words before the scheduler is 
determined).

Alternatively we could provide the adev manually to amdgpu_job_free() 
and amdgpu_job_free_resources().

Regards,
Christian.

>
>
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Christian K?nig
> Sent: Tuesday, July 17, 2018 3:05 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: always initialize job->base.sched
>
> Otherwise we can't clean up the job if we run into an error before it is pushed to the scheduler.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 024efb7ea6d6..42a4764d728e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -54,6 +54,11 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>   	if (!*job)
>   		return -ENOMEM;
>   
> +	/*
> +	 * Initialize the scheduler to at least some ring so that we always
> +	 * have a pointer to adev.
> +	 */
> +	(*job)->base.sched = &adev->rings[0]->sched;
>   	(*job)->vm = vm;
>   	(*job)->ibs = (void *)&(*job)[1];
>   	(*job)->num_ibs = num_ibs;
> --
> 2.14.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 2018年07月17日 15:26, Christian König wrote:
> Am 17.07.2018 um 09:16 schrieb Zhou, David(ChunMing):
>> Acked-by: Chunming Zhou <david1.zhou@amd.com>, but I think it isn't a 
>> nice evaluation although there is comment in code.
>
> Yeah, I didn't thought about the possibility that we need to free the 
> job before it is submitted (in other words before the scheduler is 
> determined).
>
> Alternatively we could provide the adev manually to amdgpu_job_free() 
> and amdgpu_job_free_resources().
not a big deal, you can still go ahead with this patch.

David
>
> Regards,
> Christian.
>
>>
>>
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On 
>> Behalf Of Christian K?nig
>> Sent: Tuesday, July 17, 2018 3:05 PM
>> To: amd-gfx@lists.freedesktop.org
>> Subject: [PATCH] drm/amdgpu: always initialize job->base.sched
>>
>> Otherwise we can't clean up the job if we run into an error before it 
>> is pushed to the scheduler.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> index 024efb7ea6d6..42a4764d728e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> @@ -54,6 +54,11 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, 
>> unsigned num_ibs,
>>       if (!*job)
>>           return -ENOMEM;
>>   +    /*
>> +     * Initialize the scheduler to at least some ring so that we always
>> +     * have a pointer to adev.
>> +     */
>> +    (*job)->base.sched = &adev->rings[0]->sched;
>>       (*job)->vm = vm;
>>       (*job)->ibs = (void *)&(*job)[1];
>>       (*job)->num_ibs = num_ibs;
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>