[PATCH 3/5] drm/amdgpu:implement guilty pointer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Yeah, already did so, please check the amd-gfx

-----Original Message-----
From: Koenig, Christian 
Sent: 2017å¹´10æ??23æ?¥ 15:17
To: Liu, Monk <Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 3/5] drm/amdgpu:implement guilty pointer

Am 23.10.2017 um 06:26 schrieb Liu, Monk:
> Don't set this directly, make it a parameter of amd_sched_entity_init().
>
> And please split up the patches differently.
>
> First all changes to the scheduler, including the guilty marking.
>
> Then the changes to amdgpu.
>
>
> [ML] that way the first patch will break kernel from compiling because 
> you change the interface of entity_init without changing all place 
> Calling this interface

You obviously need to adjust the calls to entity_init and at least provide NULL as value for the new parameter.

> I suggest just use one patch

I prefer to separate that, but not a strict must have when you don't have time.

The scheduler and amdgpu using it should essentially be two different components.

Regards,
Christian.

>
> -----Original Message-----
> From: Christian König [mailto:ckoenig.leichtzumerken at gmail.com]
> Sent: 2017å¹´10æ??20æ?¥ 17:09
> To: Liu, Monk <Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 3/5] drm/amdgpu:implement guilty pointer
>
> Am 20.10.2017 um 05:33 schrieb Monk Liu:
>> for user context there will be a guilty pointer in entity that points 
>> to the guilty member of its context, thus we cant track if a given 
>> entity is not from kernel ctx and if it is already marked guilty.
>>
>> Change-Id: Ie0a30830d89f52a6c4a514e67206140698a46367
>> Signed-off-by: Monk Liu <Monk.Liu at amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 1 +
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c       | 1 +
>>    drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 1 +
>>    3 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 774edc1..6a4178b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -744,6 +744,7 @@ struct amdgpu_ctx {
>>    	enum amd_sched_priority init_priority;
>>    	enum amd_sched_priority override_priority;
>>    	struct mutex            lock;
>> +	atomic_t	guilty;
>>    };
>>    
>>    struct amdgpu_ctx_mgr {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index c184468..d822e95 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -93,6 +93,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
>>    					  rq, amdgpu_sched_jobs);
>>    		if (r)
>>    			goto failed;
>> +		ctx->rings[i].entity.guilty = &ctx->guilty;
> Don't set this directly, make it a parameter of amd_sched_entity_init().
>
> And please split up the patches differently.
>
> First all changes to the scheduler, including the guilty marking.
>
> Then the changes to amdgpu.
>
> Regards,
> Christian.
>
>>    	}
>>    
>>    	r = amdgpu_queue_mgr_init(adev, &ctx->queue_mgr); diff --git 
>> a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> index 1dac7bc..2d59fc5 100644
>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> @@ -50,6 +50,7 @@ struct amd_sched_entity {
>>    
>>    	struct dma_fence		*dependency;
>>    	struct dma_fence_cb		cb;
>> +	atomic_t	*guilty; /* points to ctx's guilty */
>>    };
>>    
>>    /**
>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux