[PATCH v3 1/4] drm/amdgpu: Fix KFD oversubscription by tracking queues correctly

[PATCH v3 1/4] drm/amdgpu: Fix KFD oversubscription by tracking queues correctly

[Jay Cornwall]

The number of compute queues available to the KFD was erroneously
calculated as 64. Only the first MEC can execute compute queues and
it has 32 queue slots.

This caused the oversubscription limit to be calculated incorrectly,
leading to a missing chained runlist command at the end of an
oversubscribed runlist.

v2: Remove unused num_mec field to avoid duplicate logic
v3: Separate num_mec removal into separate patches

Change-Id: I9e7bba2cc1928b624e3eeb1edb06fdb602e5294f
Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 7060daf..aa4006a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -140,7 +140,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
 		/* According to linux/bitmap.h we shouldn't use bitmap_clear if
 		 * nbits is not compile time constant
 		 */
-		last_valid_bit = adev->gfx.mec.num_mec
+		last_valid_bit = 1 /* only first MEC can have compute queues */
 				* adev->gfx.mec.num_pipe_per_mec
 				* adev->gfx.mec.num_queue_per_pipe;
 		for (i = last_valid_bit; i < KGD_MAX_QUEUES; ++i)
-- 
2.7.4

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

[PATCH 2/4] drm/amdkfd: Remove unused references to shared_resources.num_mec

[Jay Cornwall]

Dead code.

Change-Id: Ic0bb1bcca87e96bc5e8fa9894727b0de152e8818
Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c               | 4 ----
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 7 -------
 2 files changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 1cf00d4..95f9396 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -494,10 +494,6 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 	} else
 		kfd->max_proc_per_quantum = hws_max_conc_proc;
 
-	/* We only use the first MEC */
-	if (kfd->shared_resources.num_mec > 1)
-		kfd->shared_resources.num_mec = 1;
-
 	/* calculate max size of mqds needed for queues */
 	size = max_num_of_queues_per_device *
 			kfd->device_info->mqd_size_aligned;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 7607989..306144f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -82,13 +82,6 @@ static bool is_pipe_enabled(struct device_queue_manager *dqm, int mec, int pipe)
 	return false;
 }
 
-unsigned int get_mec_num(struct device_queue_manager *dqm)
-{
-	BUG_ON(!dqm || !dqm->dev);
-
-	return dqm->dev->shared_resources.num_mec;
-}
-
 unsigned int get_queues_num(struct device_queue_manager *dqm)
 {
 	BUG_ON(!dqm || !dqm->dev);
-- 
2.7.4

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

[PATCH 3/4] drm/radeon: Remove initialization of shared_resources.num_mec

[Jay Cornwall]

Dead code.

Change-Id: I2383e0b541ed55288570b6a0ec8a0d49cdd4df89
Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
---
 drivers/gpu/drm/radeon/radeon_kfd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
index 719ea51..8f8c7c1 100644
--- a/drivers/gpu/drm/radeon/radeon_kfd.c
+++ b/drivers/gpu/drm/radeon/radeon_kfd.c
@@ -251,7 +251,6 @@ void radeon_kfd_device_init(struct radeon_device *rdev)
 	if (rdev->kfd) {
 		struct kgd2kfd_shared_resources gpu_resources = {
 			.compute_vmid_bitmap = 0xFF00,
-			.num_mec = 1,
 			.num_pipe_per_mec = 4,
 			.num_queue_per_pipe = 8,
 			.gpuvm_size = (uint64_t)radeon_vm_size << 30
-- 
2.7.4

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

[PATCH 4/4] drm/amdgpu: Remove unused field kgd2kfd_shared_resources.num_mec

[Jay Cornwall]

Dead code.

Change-Id: I9575aa73b5741b80dc340f953cc773385c92b2be
Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c      | 1 -
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index aa4006a..8c710f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -116,7 +116,6 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
 	if (adev->kfd) {
 		struct kgd2kfd_shared_resources gpu_resources = {
 			.compute_vmid_bitmap = global_compute_vmid_bitmap,
-			.num_mec = adev->gfx.mec.num_mec,
 			.num_pipe_per_mec = adev->gfx.mec.num_pipe_per_mec,
 			.num_queue_per_pipe = adev->gfx.mec.num_queue_per_pipe,
 			.gpuvm_size = (uint64_t)amdgpu_vm_size << 30
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index a4d2fee..10794b3 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -107,9 +107,6 @@ struct kgd2kfd_shared_resources {
 	/* Bit n == 1 means VMID n is available for KFD. */
 	unsigned int compute_vmid_bitmap;
 
-	/* number of mec available from the hardware */
-	uint32_t num_mec;
-
 	/* number of pipes per mec */
 	uint32_t num_pipe_per_mec;
 
-- 
2.7.4

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

Re: [PATCH v3 1/4] drm/amdgpu: Fix KFD oversubscription by tracking queues correctly

[Alex Deucher]

On Thu, Jul 13, 2017 at 9:21 PM, Jay Cornwall <Jay.Cornwall@amd.com> wrote:
> The number of compute queues available to the KFD was erroneously
> calculated as 64. Only the first MEC can execute compute queues and
> it has 32 queue slots.
>
> This caused the oversubscription limit to be calculated incorrectly,
> leading to a missing chained runlist command at the end of an
> oversubscribed runlist.
>
> v2: Remove unused num_mec field to avoid duplicate logic
> v3: Separate num_mec removal into separate patches
>
> Change-Id: I9e7bba2cc1928b624e3eeb1edb06fdb602e5294f
> Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 7060daf..aa4006a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -140,7 +140,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
>                 /* According to linux/bitmap.h we shouldn't use bitmap_clear if
>                  * nbits is not compile time constant
>                  */
> -               last_valid_bit = adev->gfx.mec.num_mec
> +               last_valid_bit = 1 /* only first MEC can have compute queues */
>                                 * adev->gfx.mec.num_pipe_per_mec
>                                 * adev->gfx.mec.num_queue_per_pipe;
>                 for (i = last_valid_bit; i < KGD_MAX_QUEUES; ++i)
> --
> 2.7.4
>
> _______________________________________________
> 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 v3 1/4] drm/amdgpu: Fix KFD oversubscription by tracking queues correctly

[Oded Gabbay]

On Fri, Jul 14, 2017 at 7:24 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Jul 13, 2017 at 9:21 PM, Jay Cornwall <Jay.Cornwall@amd.com> wrote:
>> The number of compute queues available to the KFD was erroneously
>> calculated as 64. Only the first MEC can execute compute queues and
>> it has 32 queue slots.
>>
>> This caused the oversubscription limit to be calculated incorrectly,
>> leading to a missing chained runlist command at the end of an
>> oversubscribed runlist.
>>
>> v2: Remove unused num_mec field to avoid duplicate logic
>> v3: Separate num_mec removal into separate patches
>>
>> Change-Id: I9e7bba2cc1928b624e3eeb1edb06fdb602e5294f
>> Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
>
> Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
Hi Jay,
Thanks for the patches, I applied them to amdkfd-fixes (after rebasing
them over 4.13-rc1)

Oded

>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> index 7060daf..aa4006a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> @@ -140,7 +140,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
>>                 /* According to linux/bitmap.h we shouldn't use bitmap_clear if
>>                  * nbits is not compile time constant
>>                  */
>> -               last_valid_bit = adev->gfx.mec.num_mec
>> +               last_valid_bit = 1 /* only first MEC can have compute queues */
>>                                 * adev->gfx.mec.num_pipe_per_mec
>>                                 * adev->gfx.mec.num_queue_per_pipe;
>>                 for (i = last_valid_bit; i < KGD_MAX_QUEUES; ++i)
>> --
>> 2.7.4
>>
>> _______________________________________________
>> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/4] drm/amdkfd: Remove unused references to shared_resources.num_mec

[Oded Gabbay]

On Fri, Jul 14, 2017 at 4:21 AM, Jay Cornwall <Jay.Cornwall@amd.com> wrote:
> Dead code.
>
> Change-Id: Ic0bb1bcca87e96bc5e8fa9894727b0de152e8818
> Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c               | 4 ----
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 7 -------
>  2 files changed, 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 1cf00d4..95f9396 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -494,10 +494,6 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>         } else
>                 kfd->max_proc_per_quantum = hws_max_conc_proc;
>
> -       /* We only use the first MEC */
> -       if (kfd->shared_resources.num_mec > 1)
> -               kfd->shared_resources.num_mec = 1;
> -
>         /* calculate max size of mqds needed for queues */
>         size = max_num_of_queues_per_device *
>                         kfd->device_info->mqd_size_aligned;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 7607989..306144f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -82,13 +82,6 @@ static bool is_pipe_enabled(struct device_queue_manager *dqm, int mec, int pipe)
>         return false;
>  }
>
> -unsigned int get_mec_num(struct device_queue_manager *dqm)
> -{
> -       BUG_ON(!dqm || !dqm->dev);
> -
> -       return dqm->dev->shared_resources.num_mec;
> -}
> -

FYI, I removed also the declaration of get_mec_num() in the header file
Oded

>  unsigned int get_queues_num(struct device_queue_manager *dqm)
>  {
>         BUG_ON(!dqm || !dqm->dev);
> --
> 2.7.4
>
> _______________________________________________
> 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 v3 1/4] drm/amdgpu: Fix KFD oversubscription by tracking queues correctly

[Felix Kuehling]

Hi Alex,

This patch series went into amd-kfd-staging. I'd like to also push it
into amd-staging-4.11 as I'm just working to minimize any unnecessary
differences between the branches before the big KFD history rework.

I rebased it, resolved some contlicts, and removed the declaration of
get_mec_num from kfd_device_queue_manager.h. Do you want me to push that
rebased patch series?

Thanks,
  Felix


On 17-07-17 11:52 AM, Oded Gabbay wrote:
> On Fri, Jul 14, 2017 at 7:24 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Thu, Jul 13, 2017 at 9:21 PM, Jay Cornwall <Jay.Cornwall@amd.com> wrote:
>>> The number of compute queues available to the KFD was erroneously
>>> calculated as 64. Only the first MEC can execute compute queues and
>>> it has 32 queue slots.
>>>
>>> This caused the oversubscription limit to be calculated incorrectly,
>>> leading to a missing chained runlist command at the end of an
>>> oversubscribed runlist.
>>>
>>> v2: Remove unused num_mec field to avoid duplicate logic
>>> v3: Separate num_mec removal into separate patches
>>>
>>> Change-Id: I9e7bba2cc1928b624e3eeb1edb06fdb602e5294f
>>> Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
>> Series is:
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
> Hi Jay,
> Thanks for the patches, I applied them to amdkfd-fixes (after rebasing
> them over 4.13-rc1)
>
> Oded
>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> index 7060daf..aa4006a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> @@ -140,7 +140,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
>>>                 /* According to linux/bitmap.h we shouldn't use bitmap_clear if
>>>                  * nbits is not compile time constant
>>>                  */
>>> -               last_valid_bit = adev->gfx.mec.num_mec
>>> +               last_valid_bit = 1 /* only first MEC can have compute queues */
>>>                                 * adev->gfx.mec.num_pipe_per_mec
>>>                                 * adev->gfx.mec.num_queue_per_pipe;
>>>                 for (i = last_valid_bit; i < KGD_MAX_QUEUES; ++i)
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> 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
> _______________________________________________
> 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 v3 1/4] drm/amdgpu: Fix KFD oversubscription by tracking queues correctly

[Deucher, Alexander]

> -----Original Message-----
> From: Kuehling, Felix
> Sent: Tuesday, July 18, 2017 12:41 AM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> Subject: Re: [PATCH v3 1/4] drm/amdgpu: Fix KFD oversubscription by
> tracking queues correctly
> 
> Hi Alex,
> 
> This patch series went into amd-kfd-staging. I'd like to also push it
> into amd-staging-4.11 as I'm just working to minimize any unnecessary
> differences between the branches before the big KFD history rework.
> 
> I rebased it, resolved some contlicts, and removed the declaration of
> get_mec_num from kfd_device_queue_manager.h. Do you want me to
> push that
> rebased patch series?

Sure.  Sounds good.

Alex

> 
> Thanks,
>   Felix
> 
> 
> On 17-07-17 11:52 AM, Oded Gabbay wrote:
> > On Fri, Jul 14, 2017 at 7:24 PM, Alex Deucher <alexdeucher@gmail.com>
> wrote:
> >> On Thu, Jul 13, 2017 at 9:21 PM, Jay Cornwall <Jay.Cornwall@amd.com>
> wrote:
> >>> The number of compute queues available to the KFD was erroneously
> >>> calculated as 64. Only the first MEC can execute compute queues and
> >>> it has 32 queue slots.
> >>>
> >>> This caused the oversubscription limit to be calculated incorrectly,
> >>> leading to a missing chained runlist command at the end of an
> >>> oversubscribed runlist.
> >>>
> >>> v2: Remove unused num_mec field to avoid duplicate logic
> >>> v3: Separate num_mec removal into separate patches
> >>>
> >>> Change-Id: I9e7bba2cc1928b624e3eeb1edb06fdb602e5294f
> >>> Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
> >> Series is:
> >> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >>
> > Hi Jay,
> > Thanks for the patches, I applied them to amdkfd-fixes (after rebasing
> > them over 4.13-rc1)
> >
> > Oded
> >
> >>> ---
> >>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >>> index 7060daf..aa4006a 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >>> @@ -140,7 +140,7 @@ void amdgpu_amdkfd_device_init(struct
> amdgpu_device *adev)
> >>>                 /* According to linux/bitmap.h we shouldn't use bitmap_clear if
> >>>                  * nbits is not compile time constant
> >>>                  */
> >>> -               last_valid_bit = adev->gfx.mec.num_mec
> >>> +               last_valid_bit = 1 /* only first MEC can have compute queues */
> >>>                                 * adev->gfx.mec.num_pipe_per_mec
> >>>                                 * adev->gfx.mec.num_queue_per_pipe;
> >>>                 for (i = last_valid_bit; i < KGD_MAX_QUEUES; ++i)
> >>> --
> >>> 2.7.4
> >>>
> >>> _______________________________________________
> >>> 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
> > _______________________________________________
> > 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