All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: add automatic per asic settings for gart_size
@ 2017-08-21 16:00 Alex Deucher
       [not found] ` <1503331233-20371-1-git-send-email-alexander.deucher-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2017-08-21 16:00 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Deucher

We need a larger gart for asics that do not support GPUVM on all
engines (e.g., MM) to make sure we have enough space for all
gtt buffers in physical mode.  Change the default size based on
the asic type.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c   | 16 +++++++++++++++-
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ffb98bb..97d50cd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -77,7 +77,7 @@
 extern int amdgpu_modeset;
 extern int amdgpu_vram_limit;
 extern int amdgpu_vis_vram_limit;
-extern unsigned amdgpu_gart_size;
+extern int amdgpu_gart_size;
 extern int amdgpu_gtt_size;
 extern int amdgpu_moverate;
 extern int amdgpu_benchmarking;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1e66eda8..3c062a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1063,11 +1063,11 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
 		amdgpu_sched_jobs = roundup_pow_of_two(amdgpu_sched_jobs);
 	}
 
-	if (amdgpu_gart_size < 32) {
+	if (amdgpu_gart_size != -1 && amdgpu_gart_size < 32) {
 		/* gart size must be greater or equal to 32M */
 		dev_warn(adev->dev, "gart size (%d) too small\n",
 			 amdgpu_gart_size);
-		amdgpu_gart_size = 32;
+		amdgpu_gart_size = -1;
 	}
 
 	if (amdgpu_gtt_size != -1 && amdgpu_gtt_size < 32) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3eafe39..3f2143d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -76,7 +76,7 @@
 
 int amdgpu_vram_limit = 0;
 int amdgpu_vis_vram_limit = 0;
-unsigned amdgpu_gart_size = 256;
+int amdgpu_gart_size = -1; /* auto */
 int amdgpu_gtt_size = -1; /* auto */
 int amdgpu_moverate = -1; /* auto */
 int amdgpu_benchmarking = 0;
@@ -129,7 +129,7 @@ module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
 MODULE_PARM_DESC(vis_vramlimit, "Restrict visible VRAM for testing, in megabytes");
 module_param_named(vis_vramlimit, amdgpu_vis_vram_limit, int, 0444);
 
-MODULE_PARM_DESC(gartsize, "Size of PCIE/IGP gart to setup in megabytes (32, 64, etc.)");
+MODULE_PARM_DESC(gartsize, "Size of gart to setup in megabytes (32, 64, etc., -1=auto)");
 module_param_named(gartsize, amdgpu_gart_size, uint, 0600);
 
 MODULE_PARM_DESC(gttsize, "Size of the GTT domain in megabytes (-1 = auto)");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 94c1e2e..b9b9f68 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -65,7 +65,21 @@
  */
 void amdgpu_gart_set_defaults(struct amdgpu_device *adev)
 {
-	adev->mc.gart_size = (uint64_t)amdgpu_gart_size << 20;
+	u64 gart_size;
+
+	if (amdgpu_gart_size == -1) {
+		/* make the GART larger for chips that
+		 * dont' support VM for all rings
+		 */
+		if (adev->asic_type <= CHIP_STONEY)
+			gart_size = 1024;
+		else
+			gart_size = 256;
+	} else {
+		gart_size = amdgpu_gart_size;
+	}
+
+	adev->mc.gart_size = gart_size << 20;
 }
 
 /**
-- 
2.5.5

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: add automatic per asic settings for gart_size
       [not found] ` <1503331233-20371-1-git-send-email-alexander.deucher-5C7GfCeVMHo@public.gmane.org>
@ 2017-08-21 20:48   ` Felix Kuehling
       [not found]     ` <875401f3-fb37-f9b5-c14c-62c034e0cddc-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Felix Kuehling @ 2017-08-21 20:48 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On 2017-08-21 12:00 PM, Alex Deucher wrote:
> We need a larger gart for asics that do not support GPUVM on all
> engines (e.g., MM) to make sure we have enough space for all
> gtt buffers in physical mode.  Change the default size based on
> the asic type.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c   | 16 +++++++++++++++-
>  4 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index ffb98bb..97d50cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -77,7 +77,7 @@
>  extern int amdgpu_modeset;
>  extern int amdgpu_vram_limit;
>  extern int amdgpu_vis_vram_limit;
> -extern unsigned amdgpu_gart_size;
> +extern int amdgpu_gart_size;
>  extern int amdgpu_gtt_size;
>  extern int amdgpu_moverate;
>  extern int amdgpu_benchmarking;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1e66eda8..3c062a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1063,11 +1063,11 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
>  		amdgpu_sched_jobs = roundup_pow_of_two(amdgpu_sched_jobs);
>  	}
>  
> -	if (amdgpu_gart_size < 32) {
> +	if (amdgpu_gart_size != -1 && amdgpu_gart_size < 32) {
>  		/* gart size must be greater or equal to 32M */
>  		dev_warn(adev->dev, "gart size (%d) too small\n",
>  			 amdgpu_gart_size);
> -		amdgpu_gart_size = 32;
> +		amdgpu_gart_size = -1;
>  	}
>  
>  	if (amdgpu_gtt_size != -1 && amdgpu_gtt_size < 32) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 3eafe39..3f2143d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -76,7 +76,7 @@
>  
>  int amdgpu_vram_limit = 0;
>  int amdgpu_vis_vram_limit = 0;
> -unsigned amdgpu_gart_size = 256;
> +int amdgpu_gart_size = -1; /* auto */
>  int amdgpu_gtt_size = -1; /* auto */
>  int amdgpu_moverate = -1; /* auto */
>  int amdgpu_benchmarking = 0;
> @@ -129,7 +129,7 @@ module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
>  MODULE_PARM_DESC(vis_vramlimit, "Restrict visible VRAM for testing, in megabytes");
>  module_param_named(vis_vramlimit, amdgpu_vis_vram_limit, int, 0444);
>  
> -MODULE_PARM_DESC(gartsize, "Size of PCIE/IGP gart to setup in megabytes (32, 64, etc.)");
> +MODULE_PARM_DESC(gartsize, "Size of gart to setup in megabytes (32, 64, etc., -1=auto)");

I would spell GART in capitals here.

>  module_param_named(gartsize, amdgpu_gart_size, uint, 0600);
>  
>  MODULE_PARM_DESC(gttsize, "Size of the GTT domain in megabytes (-1 = auto)");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 94c1e2e..b9b9f68 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -65,7 +65,21 @@
>   */
>  void amdgpu_gart_set_defaults(struct amdgpu_device *adev)
>  {
> -	adev->mc.gart_size = (uint64_t)amdgpu_gart_size << 20;
> +	u64 gart_size;
> +
> +	if (amdgpu_gart_size == -1) {
> +		/* make the GART larger for chips that
> +		 * dont' support VM for all rings
> +		 */
> +		if (adev->asic_type <= CHIP_STONEY)

I guess that means Polaris10 and later support VM for multimedia? I
needed to go look at amd_shared.h to find that out. It would be more
obvious like this:

    if (adev->asic_type < CHIP_POLARIS10) ...

Other than that, this patch is Reviewed-by: Felix Kuehling
<Felix.Kuehling@amd.com>

Regards,
  Felix

> +			gart_size = 1024;
> +		else
> +			gart_size = 256;
> +	} else {
> +		gart_size = amdgpu_gart_size;
> +	}
> +
> +	adev->mc.gart_size = gart_size << 20;
>  }
>  
>  /**

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: add automatic per asic settings for gart_size
       [not found]     ` <875401f3-fb37-f9b5-c14c-62c034e0cddc-5C7GfCeVMHo@public.gmane.org>
@ 2017-08-22  8:48       ` Christian König
  0 siblings, 0 replies; 3+ messages in thread
From: Christian König @ 2017-08-22  8:48 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 21.08.2017 um 22:48 schrieb Felix Kuehling:
> On 2017-08-21 12:00 PM, Alex Deucher wrote:
>> We need a larger gart for asics that do not support GPUVM on all
>> engines (e.g., MM) to make sure we have enough space for all
>> gtt buffers in physical mode.  Change the default size based on
>> the asic type.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 ++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c   | 16 +++++++++++++++-
>>   4 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index ffb98bb..97d50cd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -77,7 +77,7 @@
>>   extern int amdgpu_modeset;
>>   extern int amdgpu_vram_limit;
>>   extern int amdgpu_vis_vram_limit;
>> -extern unsigned amdgpu_gart_size;
>> +extern int amdgpu_gart_size;
>>   extern int amdgpu_gtt_size;
>>   extern int amdgpu_moverate;
>>   extern int amdgpu_benchmarking;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 1e66eda8..3c062a7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1063,11 +1063,11 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
>>   		amdgpu_sched_jobs = roundup_pow_of_two(amdgpu_sched_jobs);
>>   	}
>>   
>> -	if (amdgpu_gart_size < 32) {
>> +	if (amdgpu_gart_size != -1 && amdgpu_gart_size < 32) {
>>   		/* gart size must be greater or equal to 32M */
>>   		dev_warn(adev->dev, "gart size (%d) too small\n",
>>   			 amdgpu_gart_size);
>> -		amdgpu_gart_size = 32;
>> +		amdgpu_gart_size = -1;
>>   	}
>>   
>>   	if (amdgpu_gtt_size != -1 && amdgpu_gtt_size < 32) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 3eafe39..3f2143d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -76,7 +76,7 @@
>>   
>>   int amdgpu_vram_limit = 0;
>>   int amdgpu_vis_vram_limit = 0;
>> -unsigned amdgpu_gart_size = 256;
>> +int amdgpu_gart_size = -1; /* auto */
>>   int amdgpu_gtt_size = -1; /* auto */
>>   int amdgpu_moverate = -1; /* auto */
>>   int amdgpu_benchmarking = 0;
>> @@ -129,7 +129,7 @@ module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
>>   MODULE_PARM_DESC(vis_vramlimit, "Restrict visible VRAM for testing, in megabytes");
>>   module_param_named(vis_vramlimit, amdgpu_vis_vram_limit, int, 0444);
>>   
>> -MODULE_PARM_DESC(gartsize, "Size of PCIE/IGP gart to setup in megabytes (32, 64, etc.)");
>> +MODULE_PARM_DESC(gartsize, "Size of gart to setup in megabytes (32, 64, etc., -1=auto)");
> I would spell GART in capitals here.
>
>>   module_param_named(gartsize, amdgpu_gart_size, uint, 0600);
>>   
>>   MODULE_PARM_DESC(gttsize, "Size of the GTT domain in megabytes (-1 = auto)");
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
>> index 94c1e2e..b9b9f68 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
>> @@ -65,7 +65,21 @@
>>    */
>>   void amdgpu_gart_set_defaults(struct amdgpu_device *adev)
>>   {
>> -	adev->mc.gart_size = (uint64_t)amdgpu_gart_size << 20;
>> +	u64 gart_size;
>> +
>> +	if (amdgpu_gart_size == -1) {
>> +		/* make the GART larger for chips that
>> +		 * dont' support VM for all rings
>> +		 */
>> +		if (adev->asic_type <= CHIP_STONEY)
> I guess that means Polaris10 and later support VM for multimedia? I
> needed to go look at amd_shared.h to find that out. It would be more
> obvious like this:
>
>      if (adev->asic_type < CHIP_POLARIS10) ...
>
> Other than that, this patch is Reviewed-by: Felix Kuehling
> <Felix.Kuehling@amd.com>

Yeah, the code in uvd_v6_0.c to decide what to do is:
> static void uvd_v6_0_set_ring_funcs(struct amdgpu_device *adev)
> {
>         if (adev->asic_type >= CHIP_POLARIS10) {
>                 adev->uvd.ring.funcs = &uvd_v6_0_ring_vm_funcs;
>                 DRM_INFO("UVD is enabled in VM mode\n");
>         } else {
>                 adev->uvd.ring.funcs = &uvd_v6_0_ring_phys_funcs;
>                 DRM_INFO("UVD is enabled in physical mode\n");
>         }
> }

Not 100% sure about VCE, but Monk also pointed out internally that we 
should put that in a single place.

A check in amdgpu_uvd.h and one in amdgpu_vce.h sound reasonable to me.

Christian.

>
> Regards,
>    Felix
>
>> +			gart_size = 1024;
>> +		else
>> +			gart_size = 256;
>> +	} else {
>> +		gart_size = amdgpu_gart_size;
>> +	}
>> +
>> +	adev->mc.gart_size = gart_size << 20;
>>   }
>>   
>>   /**
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-22  8:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21 16:00 [PATCH] drm/amdgpu: add automatic per asic settings for gart_size Alex Deucher
     [not found] ` <1503331233-20371-1-git-send-email-alexander.deucher-5C7GfCeVMHo@public.gmane.org>
2017-08-21 20:48   ` Felix Kuehling
     [not found]     ` <875401f3-fb37-f9b5-c14c-62c034e0cddc-5C7GfCeVMHo@public.gmane.org>
2017-08-22  8:48       ` Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.