[PATCH 1/2] drm/amdgpu: create a func to check vm size

[PATCH 1/2] drm/amdgpu: create a func to check vm size

[Junwei Zhang]

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 51 ++++++++++++++++++------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 3500da3..1ad1113 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1078,6 +1078,36 @@ static void amdgpu_get_block_size(struct amdgpu_device *adev)
 	}
 }
 
+static void amdgpu_check_vm_size(struct amdgpu_device *adev)
+{
+	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
+		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
+			 amdgpu_vm_size);
+		goto def_value;
+	}
+
+	if (amdgpu_vm_size < 1) {
+		dev_warn(adev->dev, "VM size (%d) too small, min is 1GB\n",
+			 amdgpu_vm_size);
+		goto def_value;
+	}
+
+	/*
+	 * Max GPUVM size for Cayman, SI, CI VI are 40 bits.
+	 */
+	if (amdgpu_vm_size > 1024) {
+		dev_warn(adev->dev, "VM size (%d) too large, max is 1TB\n",
+			 amdgpu_vm_size);
+		goto def_value;
+	}
+
+	return;
+
+def_value:
+	amdgpu_vm_size = 8;
+	dev_info(adev->dev, "set default VM size %dGB\n", amdgpu_vm_size);
+}
+
 /**
  * amdgpu_check_arguments - validate module params
  *
@@ -1107,26 +1137,7 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
 		}
 	}
 
-	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
-		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
-			 amdgpu_vm_size);
-		amdgpu_vm_size = 8;
-	}
-
-	if (amdgpu_vm_size < 1) {
-		dev_warn(adev->dev, "VM size (%d) too small, min is 1GB\n",
-			 amdgpu_vm_size);
-		amdgpu_vm_size = 8;
-	}
-
-	/*
-	 * Max GPUVM size for Cayman, SI and CI are 40 bits.
-	 */
-	if (amdgpu_vm_size > 1024) {
-		dev_warn(adev->dev, "VM size (%d) too large, max is 1TB\n",
-			 amdgpu_vm_size);
-		amdgpu_vm_size = 8;
-	}
+	amdgpu_check_vm_size(adev);
 
 	amdgpu_get_block_size(adev);
 
-- 
1.9.1

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

[PATCH 2/2] drm/amdgpu: set vm size as 256TB for Vega10

[Junwei Zhang]

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1ad1113..4af4ea0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1080,6 +1080,11 @@ static void amdgpu_get_block_size(struct amdgpu_device *adev)
 
 static void amdgpu_check_vm_size(struct amdgpu_device *adev)
 {
+	if (adev->asic_type >= CHIP_VEGA10) {
+		amdgpu_vm_size = 1U << 18; /* 256TB */
+		return;
+	}
+
 	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
 		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
 			 amdgpu_vm_size);
-- 
1.9.1

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

Re: [PATCH 1/2] drm/amdgpu: create a func to check vm size

[zhoucm1]

Reviewed-by: Chunming Zhou <david1.zhou@amd.com> for both.

On 2017年03月29日 16:08, Junwei Zhang wrote:
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 51 ++++++++++++++++++------------
>   1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 3500da3..1ad1113 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1078,6 +1078,36 @@ static void amdgpu_get_block_size(struct amdgpu_device *adev)
>   	}
>   }
>   
> +static void amdgpu_check_vm_size(struct amdgpu_device *adev)
> +{
> +	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
> +		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
> +			 amdgpu_vm_size);
> +		goto def_value;
> +	}
> +
> +	if (amdgpu_vm_size < 1) {
> +		dev_warn(adev->dev, "VM size (%d) too small, min is 1GB\n",
> +			 amdgpu_vm_size);
> +		goto def_value;
> +	}
> +
> +	/*
> +	 * Max GPUVM size for Cayman, SI, CI VI are 40 bits.
> +	 */
> +	if (amdgpu_vm_size > 1024) {
> +		dev_warn(adev->dev, "VM size (%d) too large, max is 1TB\n",
> +			 amdgpu_vm_size);
> +		goto def_value;
> +	}
> +
> +	return;
> +
> +def_value:
> +	amdgpu_vm_size = 8;
> +	dev_info(adev->dev, "set default VM size %dGB\n", amdgpu_vm_size);
> +}
> +
>   /**
>    * amdgpu_check_arguments - validate module params
>    *
> @@ -1107,26 +1137,7 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
>   		}
>   	}
>   
> -	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
> -		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
> -			 amdgpu_vm_size);
> -		amdgpu_vm_size = 8;
> -	}
> -
> -	if (amdgpu_vm_size < 1) {
> -		dev_warn(adev->dev, "VM size (%d) too small, min is 1GB\n",
> -			 amdgpu_vm_size);
> -		amdgpu_vm_size = 8;
> -	}
> -
> -	/*
> -	 * Max GPUVM size for Cayman, SI and CI are 40 bits.
> -	 */
> -	if (amdgpu_vm_size > 1024) {
> -		dev_warn(adev->dev, "VM size (%d) too large, max is 1TB\n",
> -			 amdgpu_vm_size);
> -		amdgpu_vm_size = 8;
> -	}
> +	amdgpu_check_vm_size(adev);
>   
>   	amdgpu_get_block_size(adev);
>   

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

Re: [PATCH 1/2] drm/amdgpu: create a func to check vm size

[Christian König]

Am 29.03.2017 um 10:08 schrieb Junwei Zhang:
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for only this patch.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 51 ++++++++++++++++++------------
>   1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 3500da3..1ad1113 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1078,6 +1078,36 @@ static void amdgpu_get_block_size(struct amdgpu_device *adev)
>   	}
>   }
>   
> +static void amdgpu_check_vm_size(struct amdgpu_device *adev)
> +{
> +	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
> +		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
> +			 amdgpu_vm_size);
> +		goto def_value;
> +	}
> +
> +	if (amdgpu_vm_size < 1) {
> +		dev_warn(adev->dev, "VM size (%d) too small, min is 1GB\n",
> +			 amdgpu_vm_size);
> +		goto def_value;
> +	}
> +
> +	/*
> +	 * Max GPUVM size for Cayman, SI, CI VI are 40 bits.
> +	 */
> +	if (amdgpu_vm_size > 1024) {
> +		dev_warn(adev->dev, "VM size (%d) too large, max is 1TB\n",
> +			 amdgpu_vm_size);
> +		goto def_value;
> +	}
> +
> +	return;
> +
> +def_value:
> +	amdgpu_vm_size = 8;
> +	dev_info(adev->dev, "set default VM size %dGB\n", amdgpu_vm_size);
> +}
> +
>   /**
>    * amdgpu_check_arguments - validate module params
>    *
> @@ -1107,26 +1137,7 @@ static void amdgpu_check_arguments(struct amdgpu_device *adev)
>   		}
>   	}
>   
> -	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
> -		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
> -			 amdgpu_vm_size);
> -		amdgpu_vm_size = 8;
> -	}
> -
> -	if (amdgpu_vm_size < 1) {
> -		dev_warn(adev->dev, "VM size (%d) too small, min is 1GB\n",
> -			 amdgpu_vm_size);
> -		amdgpu_vm_size = 8;
> -	}
> -
> -	/*
> -	 * Max GPUVM size for Cayman, SI and CI are 40 bits.
> -	 */
> -	if (amdgpu_vm_size > 1024) {
> -		dev_warn(adev->dev, "VM size (%d) too large, max is 1TB\n",
> -			 amdgpu_vm_size);
> -		amdgpu_vm_size = 8;
> -	}
> +	amdgpu_check_vm_size(adev);
>   
>   	amdgpu_get_block_size(adev);
>   


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

Re: [PATCH 2/2] drm/amdgpu: set vm size as 256TB for Vega10

[Christian König]

Am 29.03.2017 um 10:08 schrieb Junwei Zhang:
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1ad1113..4af4ea0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1080,6 +1080,11 @@ static void amdgpu_get_block_size(struct amdgpu_device *adev)
>   
>   static void amdgpu_check_vm_size(struct amdgpu_device *adev)
>   {
> +	if (adev->asic_type >= CHIP_VEGA10) {
> +		amdgpu_vm_size = 1U << 18; /* 256TB */
> +		return;
> +	}
> +

NAK, that would break in a mixed environment with APU+Vega10.

Instead just go into gmc_v9_0_sw_init() and set max_pfn = 1ULL << 36 and 
add a comment there why we hardcode this value for Vega10 and later.

Christian.

>   	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
>   		dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
>   			 amdgpu_vm_size);


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

Re: [PATCH 2/2] drm/amdgpu: set vm size as 256TB for Vega10

[Alex Deucher]

On Wed, Mar 29, 2017 at 4:47 AM, Christian König
<christian.koenig@amd.com> wrote:
> Am 29.03.2017 um 10:08 schrieb Junwei Zhang:
>>
>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 1ad1113..4af4ea0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1080,6 +1080,11 @@ static void amdgpu_get_block_size(struct
>> amdgpu_device *adev)
>>     static void amdgpu_check_vm_size(struct amdgpu_device *adev)
>>   {
>> +       if (adev->asic_type >= CHIP_VEGA10) {
>> +               amdgpu_vm_size = 1U << 18; /* 256TB */
>> +               return;
>> +       }
>> +
>
>
> NAK, that would break in a mixed environment with APU+Vega10.
>
> Instead just go into gmc_v9_0_sw_init() and set max_pfn = 1ULL << 36 and add
> a comment there why we hardcode this value for Vega10 and later.
>

We should fix this for real and move these variables into the driver
vm structure and then set those based on the global parameter
variables in the various gmc modules.

Alex

> Christian.
>
>>         if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
>>                 dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
>>                          amdgpu_vm_size);
>
>
>
> _______________________________________________
> 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/2] drm/amdgpu: set vm size as 256TB for Vega10

[Christian König]

Am 29.03.2017 um 16:17 schrieb Alex Deucher:
> On Wed, Mar 29, 2017 at 4:47 AM, Christian König
> <christian.koenig@amd.com> wrote:
>> Am 29.03.2017 um 10:08 schrieb Junwei Zhang:
>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 1ad1113..4af4ea0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -1080,6 +1080,11 @@ static void amdgpu_get_block_size(struct
>>> amdgpu_device *adev)
>>>      static void amdgpu_check_vm_size(struct amdgpu_device *adev)
>>>    {
>>> +       if (adev->asic_type >= CHIP_VEGA10) {
>>> +               amdgpu_vm_size = 1U << 18; /* 256TB */
>>> +               return;
>>> +       }
>>> +
>>
>> NAK, that would break in a mixed environment with APU+Vega10.
>>
>> Instead just go into gmc_v9_0_sw_init() and set max_pfn = 1ULL << 36 and add
>> a comment there why we hardcode this value for Vega10 and later.
>>
> We should fix this for real and move these variables into the driver
> vm structure and then set those based on the global parameter
> variables in the various gmc modules.

Yeah, that's exactly what I've suggested as well.

max_pfn is already part of the vm manager structure, so setting this 
different on Vega10 than on previous generations shouldn't be a problem.

We just need to add the block size there as well and should be done with it.

Christian.

>
> Alex
>
>> Christian.
>>
>>>          if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
>>>                  dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
>>>                           amdgpu_vm_size);
>>
>>
>> _______________________________________________
>> 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/2] drm/amdgpu: set vm size as 256TB for Vega10

[Zhang, Jerry (Junwei)]

On 03/29/2017 10:27 PM, Christian König wrote:
> Am 29.03.2017 um 16:17 schrieb Alex Deucher:
>> On Wed, Mar 29, 2017 at 4:47 AM, Christian König
>> <christian.koenig@amd.com> wrote:
>>> Am 29.03.2017 um 10:08 schrieb Junwei Zhang:
>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index 1ad1113..4af4ea0 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -1080,6 +1080,11 @@ static void amdgpu_get_block_size(struct
>>>> amdgpu_device *adev)
>>>>      static void amdgpu_check_vm_size(struct amdgpu_device *adev)
>>>>    {
>>>> +       if (adev->asic_type >= CHIP_VEGA10) {
>>>> +               amdgpu_vm_size = 1U << 18; /* 256TB */
>>>> +               return;
>>>> +       }
>>>> +
>>>
>>> NAK, that would break in a mixed environment with APU+Vega10.
>>>
>>> Instead just go into gmc_v9_0_sw_init() and set max_pfn = 1ULL << 36 and add
>>> a comment there why we hardcode this value for Vega10 and later.
>>>
>> We should fix this for real and move these variables into the driver
>> vm structure and then set those based on the global parameter
>> variables in the various gmc modules.
>
> Yeah, that's exactly what I've suggested as well.
>
> max_pfn is already part of the vm manager structure, so setting this different
> on Vega10 than on previous generations shouldn't be a problem.
>
> We just need to add the block size there as well and should be done with it.

Thanks for all the comments above.
Prepared another patch to fix it.

Jerry.

>
> Christian.
>
>>
>> Alex
>>
>>> Christian.
>>>
>>>>          if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
>>>>                  dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
>>>>                           amdgpu_vm_size);
>>>
>>>
>>> _______________________________________________
>>> 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