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 */ >> }; >> >> /** >
