Re: [PATCH 6/8] drm/amd/display: enable GPU VM support

[Christian König]

Am 27.10.2017 um 17:40 schrieb Harry Wentland:

On 2017-10-27 10:48 AM, Christian König wrote:

Am 27.10.2017 um 16:37 schrieb Harry Wentland:

On 2017-10-26 12:06 PM, Christian König wrote:

From: Christian König 

Just set the bit so that DC does the hardware programming.

Signed-off-by: Christian König 
---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2188f20..ed4351a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
     init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
   +    init_data.flags.gpu_vm_support = true;
+

Hi Christian,

what ASIC has this been tested on?

Carizzo.


I'm not sure that this won't break dGPU. dGPU doesn't have vm_support but I 
think DC will still try to write the same registers as for CZ on dGPU if we 
enable this. We at least need to make sure it doesn't blow up.

Yeah, as Tom already stumbled over as well it causes a warning on Tonga. 
Currently giving it a spin on Polaris.


As for Raven we use this flag on Windows but only tested the case where VM is 
actually programmed. If VM isn't programmed it might still work but we'd need 
to give that a spin.

Without this flag tilling fails and only displays a quite unusual pattern.

I would actually prefer if we can completely remove this flag and instead 
always enable the VM programming on supported hardware.

Is there any downside on doing so?


The way this flag is currently used is for the base driver (amdgpu on Linux, 
kmd on Windows) to let us know whether VM is being used or not. The options are

1) amdgpu lets us know whether the framebuffer is in virtual memory or not
2) we assume amdgpu always uses virtual memory on ASICs that support it, set 
this flag to true (as in this patch) and make sure DC only programs it on ASICs 
that support it

I'd somewhat prefer option 2 as it's simpler from an amdgpu perspective and 
also keeps DC consistent.


I agree that option 2 sounds better. In this case please provide DC 
patches to fix the warning and clean things up.


BTW: I've also get a bunch of warning from the DRM core about things DC 
does. You're probably aware of those problem, but I just wanted to note 
once more that the backtraces in the log are kind of annoying.


Regards,
Christian.



Harry


Christian.


Harry


   if (amdgpu_dc_log)
   init_data.log_mask = DC_DEFAULT_LOG_MASK;
   else


___
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 6/8] drm/amd/display: enable GPU VM support

[Harry Wentland]

On 2017-10-27 10:48 AM, Christian König wrote:
> Am 27.10.2017 um 16:37 schrieb Harry Wentland:
>> On 2017-10-26 12:06 PM, Christian König wrote:
>>> From: Christian König 
>>>
>>> Just set the bit so that DC does the hardware programming.
>>>
>>> Signed-off-by: Christian König 
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index 2188f20..ed4351a 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>>>     init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>   +    init_data.flags.gpu_vm_support = true;
>>> +
>> Hi Christian,
>>
>> what ASIC has this been tested on?
> 
> Carizzo.
> 
>> I'm not sure that this won't break dGPU. dGPU doesn't have vm_support but I 
>> think DC will still try to write the same registers as for CZ on dGPU if we 
>> enable this. We at least need to make sure it doesn't blow up.
> 
> Yeah, as Tom already stumbled over as well it causes a warning on Tonga. 
> Currently giving it a spin on Polaris.
> 
>> As for Raven we use this flag on Windows but only tested the case where VM 
>> is actually programmed. If VM isn't programmed it might still work but we'd 
>> need to give that a spin.
> 
> Without this flag tilling fails and only displays a quite unusual pattern.
> 
> I would actually prefer if we can completely remove this flag and instead 
> always enable the VM programming on supported hardware.
> 
> Is there any downside on doing so?
> 

The way this flag is currently used is for the base driver (amdgpu on Linux, 
kmd on Windows) to let us know whether VM is being used or not. The options are

1) amdgpu lets us know whether the framebuffer is in virtual memory or not
2) we assume amdgpu always uses virtual memory on ASICs that support it, set 
this flag to true (as in this patch) and make sure DC only programs it on ASICs 
that support it

I'd somewhat prefer option 2 as it's simpler from an amdgpu perspective and 
also keeps DC consistent.

Harry

> Christian.
> 
>>
>> Harry
>>
>>>   if (amdgpu_dc_log)
>>>   init_data.log_mask = DC_DEFAULT_LOG_MASK;
>>>   else
>>>
>> ___
>> 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 6/8] drm/amd/display: enable GPU VM support

[Christian König]

Am 27.10.2017 um 16:58 schrieb Michel Dänzer:

On 27/10/17 04:48 PM, Christian König wrote:

Am 27.10.2017 um 16:37 schrieb Harry Wentland:

On 2017-10-26 12:06 PM, Christian König wrote:

From: Christian König 

Just set the bit so that DC does the hardware programming.

Signed-off-by: Christian König 
---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2188f20..ed4351a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device
*adev)
     init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
   +    init_data.flags.gpu_vm_support = true;
+

Hi Christian,

what ASIC has this been tested on?

Carizzo.


I'm not sure that this won't break dGPU. dGPU doesn't have vm_support
but I think DC will still try to write the same registers as for CZ on
dGPU if we enable this. We at least need to make sure it doesn't blow up.

Yeah, as Tom already stumbled over as well it causes a warning on Tonga.
Currently giving it a spin on Polaris.


As for Raven we use this flag on Windows but only tested the case
where VM is actually programmed. If VM isn't programmed it might still
work but we'd need to give that a spin.

Without this flag tilling fails and only displays a quite unusual pattern.

I would actually prefer if we can completely remove this flag and
instead always enable the VM programming on supported hardware.

Is there any downside on doing so?

Is there a particular use-case for scanout from system memory with a
dGPU? Otherwise I suspect it's better not to bother, because the failure
mode might be pretty bad if the PCIe link can't keep up with scanout.


That's true and I actually didn't want to suggest that we scanout from 
system memory on dGPUs.


But setting up the tilling information in the DCE/DCN VM block anyway 
looks harmless to me.


And scanning out from scattered VRAM at some point still seems to make 
sense to me.


Regards,
Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/amd/display: enable GPU VM support

[Michel Dänzer]

On 27/10/17 04:48 PM, Christian König wrote:
> Am 27.10.2017 um 16:37 schrieb Harry Wentland:
>> On 2017-10-26 12:06 PM, Christian König wrote:
>>> From: Christian König 
>>>
>>> Just set the bit so that DC does the hardware programming.
>>>
>>> Signed-off-by: Christian König 
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index 2188f20..ed4351a 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device
>>> *adev)
>>>     init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>>>   +    init_data.flags.gpu_vm_support = true;
>>> +
>> Hi Christian,
>>
>> what ASIC has this been tested on?
> 
> Carizzo.
> 
>> I'm not sure that this won't break dGPU. dGPU doesn't have vm_support
>> but I think DC will still try to write the same registers as for CZ on
>> dGPU if we enable this. We at least need to make sure it doesn't blow up.
> 
> Yeah, as Tom already stumbled over as well it causes a warning on Tonga.
> Currently giving it a spin on Polaris.
> 
>> As for Raven we use this flag on Windows but only tested the case
>> where VM is actually programmed. If VM isn't programmed it might still
>> work but we'd need to give that a spin.
> 
> Without this flag tilling fails and only displays a quite unusual pattern.
> 
> I would actually prefer if we can completely remove this flag and
> instead always enable the VM programming on supported hardware.
> 
> Is there any downside on doing so?

Is there a particular use-case for scanout from system memory with a
dGPU? Otherwise I suspect it's better not to bother, because the failure
mode might be pretty bad if the PCIe link can't keep up with scanout.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/amd/display: enable GPU VM support

[Tom St Denis]

On 27/10/17 10:48 AM, Christian König wrote:

Am 27.10.2017 um 16:37 schrieb Harry Wentland:

On 2017-10-26 12:06 PM, Christian König wrote:

From: Christian König 

Just set the bit so that DC does the hardware programming.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index 2188f20..ed4351a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device 
*adev)

  init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
+    init_data.flags.gpu_vm_support = true;
+

Hi Christian,

what ASIC has this been tested on?


Carizzo.

I'm not sure that this won't break dGPU. dGPU doesn't have vm_support 
but I think DC will still try to write the same registers as for CZ on 
dGPU if we enable this. We at least need to make sure it doesn't blow up.


Yeah, as Tom already stumbled over as well it causes a warning on Tonga. 
Currently giving it a spin on Polaris.


I saw the warnings on my CZ+P10 setup but I didn't note which device was 
complaining.


And I'd like to think I journeyed into it on my meandering path of 
testing new commits on the hodgepodge of gear I have :-).


Tom

As for Raven we use this flag on Windows but only tested the case 
where VM is actually programmed. If VM isn't programmed it might still 
work but we'd need to give that a spin.


Without this flag tilling fails and only displays a quite unusual pattern.

I would actually prefer if we can completely remove this flag and 
instead always enable the VM programming on supported hardware.


Is there any downside on doing so?

Christian.



Harry


  if (amdgpu_dc_log)
  init_data.log_mask = DC_DEFAULT_LOG_MASK;
  else


___
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 6/8] drm/amd/display: enable GPU VM support

[Christian König]

Am 27.10.2017 um 16:37 schrieb Harry Wentland:

On 2017-10-26 12:06 PM, Christian König wrote:

From: Christian König 

Just set the bit so that DC does the hardware programming.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2188f20..ed4351a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
  
  	init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
  
+	init_data.flags.gpu_vm_support = true;

+

Hi Christian,

what ASIC has this been tested on?


Carizzo.


I'm not sure that this won't break dGPU. dGPU doesn't have vm_support but I 
think DC will still try to write the same registers as for CZ on dGPU if we 
enable this. We at least need to make sure it doesn't blow up.


Yeah, as Tom already stumbled over as well it causes a warning on Tonga. 
Currently giving it a spin on Polaris.



As for Raven we use this flag on Windows but only tested the case where VM is 
actually programmed. If VM isn't programmed it might still work but we'd need 
to give that a spin.


Without this flag tilling fails and only displays a quite unusual pattern.

I would actually prefer if we can completely remove this flag and 
instead always enable the VM programming on supported hardware.


Is there any downside on doing so?

Christian.



Harry


if (amdgpu_dc_log)
init_data.log_mask = DC_DEFAULT_LOG_MASK;
else


___
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 6/8] drm/amd/display: enable GPU VM support

[Harry Wentland]

On 2017-10-26 12:06 PM, Christian König wrote:
> From: Christian König 
> 
> Just set the bit so that DC does the hardware programming.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 2188f20..ed4351a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>  
>   init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
>  
> + init_data.flags.gpu_vm_support = true;
> +

Hi Christian,

what ASIC has this been tested on?

I'm not sure that this won't break dGPU. dGPU doesn't have vm_support but I 
think DC will still try to write the same registers as for CZ on dGPU if we 
enable this. We at least need to make sure it doesn't blow up.

As for Raven we use this flag on Windows but only tested the case where VM is 
actually programmed. If VM isn't programmed it might still work but we'd need 
to give that a spin.

Harry

>   if (amdgpu_dc_log)
>   init_data.log_mask = DC_DEFAULT_LOG_MASK;
>   else
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/amd/display: enable GPU VM support

[Andrey Grodzovsky]

Reviewed-by: Andrey Grodzovsky


On 2017-10-26 12:06 PM, Christian König wrote:

From: Christian König 

Just set the bit so that DC does the hardware programming.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2188f20..ed4351a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
  
  	init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
  
+	init_data.flags.gpu_vm_support = true;

+
if (amdgpu_dc_log)
init_data.log_mask = DC_DEFAULT_LOG_MASK;
else


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

RE: [PATCH 6/8] drm/amd/display: enable GPU VM support

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Thursday, October 26, 2017 12:06 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH 6/8] drm/amd/display: enable GPU VM support
> 
> From: Christian König <christian.koe...@amd.com>
> 
> Just set the bit so that DC does the hardware programming.
> 
> Signed-off-by: Christian König <christian.koe...@amd.com>

Acked-by: Alex Deucher <alexander.deuc...@amd.com>

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 2188f20..ed4351a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device
> *adev)
> 
>   init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
> 
> + init_data.flags.gpu_vm_support = true;
> +
>   if (amdgpu_dc_log)
>   init_data.log_mask = DC_DEFAULT_LOG_MASK;
>   else
> --
> 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

[PATCH 6/8] drm/amd/display: enable GPU VM support

[Christian König]

From: Christian König 

Just set the bit so that DC does the hardware programming.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2188f20..ed4351a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -417,6 +417,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
init_data.dce_environment = DCE_ENV_PRODUCTION_DRV;
 
+   init_data.flags.gpu_vm_support = true;
+
if (amdgpu_dc_log)
init_data.log_mask = DC_DEFAULT_LOG_MASK;
else
-- 
2.7.4

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