Re: [PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Christian König]

Am 07.11.18 um 06:15 schrieb Trigger Huang:

Currently, SDMA page queue is not used under SR-IOV VF, and this queue will
cause ring test failure in amdgpu module reload case. So just disable it.

Signed-off-by: Trigger Huang 
---
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index e39a09eb0f..4edc848 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
adev->sdma.has_page_queue = false;
} else {
adev->sdma.num_instances = 2;
-   if (adev->asic_type != CHIP_VEGA20 &&
+   if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
+   adev->sdma.has_page_queue = false;
+   else if (adev->asic_type != CHIP_VEGA20 &&


Please add a /* TODO: Page queue breaks driver reload under SRIOV */ 
comment.


With that done the patch is Reviewed-by: Christian König 
.


Regards,
Christian.


adev->asic_type != CHIP_VEGA12)
adev->sdma.has_page_queue = true;
}


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

Re: [PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Koenig, Christian]

Am 07.11.18 um 08:41 schrieb Zhang, Jerry(Junwei):
> On 11/7/18 3:29 PM, Koenig, Christian wrote:
>> Hi guys,
>>
>> this is necessary for recoverable page fault handling.
>>
>> When the normal SDMA queue is blocked because of a page fault the SDMA
>> firmware will switch to the paging queue so that we are able to handle
>> the fault.
> Thanks for your info.
>
> IIRC, page queue has higher priority than gfx queue(previously we were 
> using),
> so the PT update job on page queue will always be scheduled first in HW.

I think so, but that is not it's primary purpose. The key feature is 
that it still works even when the GFX or RLC queues are blocked because 
of fault handling.

> And (not 100% sure) page queue is designed for page migration?

Yes, well it is designed for page tables updates. Either while doing 
migration, fault handling or whatever reason you got.

> Anyway, we can disable it for SRIOV for their existing issues.

It would be nice to have for normal PD/PT updates under SRIOV as well, 
but as a short term workaround we can probably disable it.

Regards,
Christian.

>
> Regards,
> Jerry
>
>>
>> In general it should work on all Vega (but not Raven) components and we
>> are going to need it when we enable recoverable page faults.
>>
>> The only case I can see where we don't immediately need it is SRIOV,
>> because the current planning is to not support recoverable page faults
>> there.
>>
>> Christian.
>>
>> Am 07.11.18 um 08:21 schrieb Liu, Monk:
>>> Hi team
>>>
>>> Why we need this page_queue in amdgpu ?  can anyone share something 
>>> of its introduction to the kmd ?
>>> According to my understanding , gpu-scheduler already have couple 
>>> levels of priority for contexts/entities , thus the job page_queue 
>>> supposed to do (should be mapping/unmapping/moving) is already good 
>>> took care of by "KERNEL" priority entities, and all other 
>>> context/entity SDMA jobs will be handled after "KERNEL" jobs ...
>>>
>>> So there is no real benefit to introduce page_queue (also for 
>>> rlc_queue) to amdgpu with the existence of priority aware 
>>> gpu-scheduler ... unless we are going to remove the "KERNEL" 
>>> priority and always do the mapping/unmapping in page_queue ...
>>>
>>> /Monk
>>>
>>> -Original Message-
>>> From: amd-gfx  On Behalf Of 
>>> Zhang, Jerry(Junwei)
>>> Sent: Wednesday, November 7, 2018 1:26 PM
>>> To: Huang, Trigger ; 
>>> amd-gfx@lists.freedesktop.org; Deucher, Alexander 
>>> ; Koenig, Christian 
>>> ; Kuehling, Felix 
>>> Subject: Re: [PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF
>>>
>>> On 11/7/18 1:15 PM, Trigger Huang wrote:
 Currently, SDMA page queue is not used under SR-IOV VF, and this queue
 will cause ring test failure in amdgpu module reload case. So just 
 disable it.

 Signed-off-by: Trigger Huang 
>>> Looks we ran into several issues about it on vega.
>>> kfd also disabled vega10 for development.(but not sure the detail 
>>> issue for them)
>>>
>>> Thus, we may disable it for vega10 as well?
>>> any comment? Alex, Christian, Flex.
>>>
>>> Regards,
>>> Jerry
 ---
     drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
     1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
 b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
 index e39a09eb0f..4edc848 100644
 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
 +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
 @@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
     adev->sdma.has_page_queue = false;
     } else {
     adev->sdma.num_instances = 2;
 -    if (adev->asic_type != CHIP_VEGA20 &&
 +    if ((adev->asic_type == CHIP_VEGA10) && 
 amdgpu_sriov_vf((adev)))
 +    adev->sdma.has_page_queue = false;
 +    else if (adev->asic_type != CHIP_VEGA20 &&
     adev->asic_type != CHIP_VEGA12)
     adev->sdma.has_page_queue = true;
     }
>>> ___
>>> 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] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Zhang, Jerry(Junwei)]

On 11/7/18 3:29 PM, Koenig, Christian wrote:

Hi guys,

this is necessary for recoverable page fault handling.

When the normal SDMA queue is blocked because of a page fault the SDMA
firmware will switch to the paging queue so that we are able to handle
the fault.

Thanks for your info.

IIRC, page queue has higher priority than gfx queue(previously we were 
using),

so the PT update job on page queue will always be scheduled first in HW.

And (not 100% sure) page queue is designed for page migration?

Anyway, we can disable it for SRIOV for their existing issues.

Regards,
Jerry



In general it should work on all Vega (but not Raven) components and we
are going to need it when we enable recoverable page faults.

The only case I can see where we don't immediately need it is SRIOV,
because the current planning is to not support recoverable page faults
there.

Christian.

Am 07.11.18 um 08:21 schrieb Liu, Monk:

Hi team

Why we need this page_queue in amdgpu ?  can anyone share something of its 
introduction to the kmd ?
According to my understanding , gpu-scheduler already have couple levels of priority for 
contexts/entities , thus the job page_queue supposed to do (should be mapping/unmapping/moving) is 
already good took care of by "KERNEL" priority entities, and all other context/entity 
SDMA jobs will be handled after "KERNEL" jobs ...

So there is no real benefit to introduce page_queue (also for rlc_queue) to amdgpu with 
the existence of priority aware gpu-scheduler ... unless we are going to remove the 
"KERNEL" priority and always do the mapping/unmapping in page_queue ...

/Monk

-Original Message-
From: amd-gfx  On Behalf Of Zhang, 
Jerry(Junwei)
Sent: Wednesday, November 7, 2018 1:26 PM
To: Huang, Trigger ; amd-gfx@lists.freedesktop.org; Deucher, Alexander 
; Koenig, Christian ; Kuehling, Felix 

Subject: Re: [PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

On 11/7/18 1:15 PM, Trigger Huang wrote:

Currently, SDMA page queue is not used under SR-IOV VF, and this queue
will cause ring test failure in amdgpu module reload case. So just disable it.

Signed-off-by: Trigger Huang 

Looks we ran into several issues about it on vega.
kfd also disabled vega10 for development.(but not sure the detail issue for 
them)

Thus, we may disable it for vega10 as well?
any comment? Alex, Christian, Flex.

Regards,
Jerry

---
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index e39a09eb0f..4edc848 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
adev->sdma.has_page_queue = false;
} else {
adev->sdma.num_instances = 2;
-   if (adev->asic_type != CHIP_VEGA20 &&
+   if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
+   adev->sdma.has_page_queue = false;
+   else if (adev->asic_type != CHIP_VEGA20 &&
adev->asic_type != CHIP_VEGA12)
adev->sdma.has_page_queue = true;
}

___
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] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Koenig, Christian]

Hi guys,

this is necessary for recoverable page fault handling.

When the normal SDMA queue is blocked because of a page fault the SDMA 
firmware will switch to the paging queue so that we are able to handle 
the fault.

In general it should work on all Vega (but not Raven) components and we 
are going to need it when we enable recoverable page faults.

The only case I can see where we don't immediately need it is SRIOV, 
because the current planning is to not support recoverable page faults 
there.

Christian.

Am 07.11.18 um 08:21 schrieb Liu, Monk:
> Hi team
>
> Why we need this page_queue in amdgpu ?  can anyone share something of its 
> introduction to the kmd ?
> According to my understanding , gpu-scheduler already have couple levels of 
> priority for contexts/entities , thus the job page_queue supposed to do 
> (should be mapping/unmapping/moving) is already good took care of by "KERNEL" 
> priority entities, and all other context/entity SDMA jobs will be handled 
> after "KERNEL" jobs ...
>
> So there is no real benefit to introduce page_queue (also for rlc_queue) to 
> amdgpu with the existence of priority aware gpu-scheduler ... unless we are 
> going to remove the "KERNEL" priority and always do the mapping/unmapping in 
> page_queue ...
>
> /Monk
>
> -Original Message-
> From: amd-gfx  On Behalf Of Zhang, 
> Jerry(Junwei)
> Sent: Wednesday, November 7, 2018 1:26 PM
> To: Huang, Trigger ; amd-gfx@lists.freedesktop.org; 
> Deucher, Alexander ; Koenig, Christian 
> ; Kuehling, Felix 
> Subject: Re: [PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF
>
> On 11/7/18 1:15 PM, Trigger Huang wrote:
>> Currently, SDMA page queue is not used under SR-IOV VF, and this queue
>> will cause ring test failure in amdgpu module reload case. So just disable 
>> it.
>>
>> Signed-off-by: Trigger Huang 
> Looks we ran into several issues about it on vega.
> kfd also disabled vega10 for development.(but not sure the detail issue for 
> them)
>
> Thus, we may disable it for vega10 as well?
> any comment? Alex, Christian, Flex.
>
> Regards,
> Jerry
>> ---
>>drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
>>1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> index e39a09eb0f..4edc848 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> @@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
>>  adev->sdma.has_page_queue = false;
>>  } else {
>>  adev->sdma.num_instances = 2;
>> -if (adev->asic_type != CHIP_VEGA20 &&
>> +if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
>> +adev->sdma.has_page_queue = false;
>> +else if (adev->asic_type != CHIP_VEGA20 &&
>>  adev->asic_type != CHIP_VEGA12)
>>  adev->sdma.has_page_queue = true;
>>  }
> ___
> 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] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Liu, Monk]

Hi team

Why we need this page_queue in amdgpu ?  can anyone share something of its 
introduction to the kmd ?
According to my understanding , gpu-scheduler already have couple levels of 
priority for contexts/entities , thus the job page_queue supposed to do (should 
be mapping/unmapping/moving) is already good took care of by "KERNEL" priority 
entities, and all other context/entity SDMA jobs will be handled after "KERNEL" 
jobs ...

So there is no real benefit to introduce page_queue (also for rlc_queue) to 
amdgpu with the existence of priority aware gpu-scheduler ... unless we are 
going to remove the "KERNEL" priority and always do the mapping/unmapping in 
page_queue ...

/Monk

-Original Message-
From: amd-gfx  On Behalf Of Zhang, 
Jerry(Junwei)
Sent: Wednesday, November 7, 2018 1:26 PM
To: Huang, Trigger ; amd-gfx@lists.freedesktop.org; 
Deucher, Alexander ; Koenig, Christian 
; Kuehling, Felix 
Subject: Re: [PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

On 11/7/18 1:15 PM, Trigger Huang wrote:
> Currently, SDMA page queue is not used under SR-IOV VF, and this queue 
> will cause ring test failure in amdgpu module reload case. So just disable it.
>
> Signed-off-by: Trigger Huang 

Looks we ran into several issues about it on vega.
kfd also disabled vega10 for development.(but not sure the detail issue for 
them)

Thus, we may disable it for vega10 as well?
any comment? Alex, Christian, Flex.

Regards,
Jerry
> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index e39a09eb0f..4edc848 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
>   adev->sdma.has_page_queue = false;
>   } else {
>   adev->sdma.num_instances = 2;
> - if (adev->asic_type != CHIP_VEGA20 &&
> + if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
> + adev->sdma.has_page_queue = false;
> + else if (adev->asic_type != CHIP_VEGA20 &&
>   adev->asic_type != CHIP_VEGA12)
>   adev->sdma.has_page_queue = true;
>   }

___
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] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Zhang, Jerry(Junwei)]

On 11/7/18 1:15 PM, Trigger Huang wrote:

Currently, SDMA page queue is not used under SR-IOV VF, and this queue will
cause ring test failure in amdgpu module reload case. So just disable it.

Signed-off-by: Trigger Huang 


Looks we ran into several issues about it on vega.
kfd also disabled vega10 for development.(but not sure the detail issue 
for them)


Thus, we may disable it for vega10 as well?
any comment? Alex, Christian, Flex.

Regards,
Jerry

---
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index e39a09eb0f..4edc848 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
adev->sdma.has_page_queue = false;
} else {
adev->sdma.num_instances = 2;
-   if (adev->asic_type != CHIP_VEGA20 &&
+   if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
+   adev->sdma.has_page_queue = false;
+   else if (adev->asic_type != CHIP_VEGA20 &&
adev->asic_type != CHIP_VEGA12)
adev->sdma.has_page_queue = true;
}


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

[PATCH] drm/amdgpu: disable page queue on Vega10 SR-IOV VF

[Trigger Huang]

Currently, SDMA page queue is not used under SR-IOV VF, and this queue will
cause ring test failure in amdgpu module reload case. So just disable it.

Signed-off-by: Trigger Huang 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index e39a09eb0f..4edc848 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1451,7 +1451,9 @@ static int sdma_v4_0_early_init(void *handle)
adev->sdma.has_page_queue = false;
} else {
adev->sdma.num_instances = 2;
-   if (adev->asic_type != CHIP_VEGA20 &&
+   if ((adev->asic_type == CHIP_VEGA10) && amdgpu_sriov_vf((adev)))
+   adev->sdma.has_page_queue = false;
+   else if (adev->asic_type != CHIP_VEGA20 &&
adev->asic_type != CHIP_VEGA12)
adev->sdma.has_page_queue = true;
}
-- 
2.7.4

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

RE: [PATCH] drm/amdgpu: fix frame size of amdgpu_xgmi_add_devices excceed 1024 bytes

[Zhang, Hawking]

Fixed and pushed. Thanks.

Regards,
Hawking
From: Yuan, Xiaojie 
Sent: 2018年11月7日 11:51
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: fix frame size of amdgpu_xgmi_add_devices 
excceed 1024 bytes


Would be better to add a is-null check for 'tmp_topology' after kzalloc(). With 
that fixed, patch is

Reviewed-by: Xiaojie Yuan mailto:xiaojie.y...@amd.com>>



Regards,

Xiaojie


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Hawking Zhang 
mailto:hawking.zh...@amd.com>>
Sent: Wednesday, November 7, 2018 11:29 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking
Subject: [PATCH] drm/amdgpu: fix frame size of amdgpu_xgmi_add_devices excceed 
1024 bytes

Instead of stack-allocated psp_xgmi_topology_info in function
amdgpu_xgmi_add_device, dynamically allocated this structure to
avoid the frame size of this function excceed 1024 bytes.

Signed-off-by: Hawking Zhang 
mailto:hawking.zh...@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index e92b454..1c0ed13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -63,7 +63,7 @@ static struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct 
amdgpu_device *adev)

 int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 {
-   struct psp_xgmi_topology_info tmp_topology;
+   struct psp_xgmi_topology_info *tmp_topology;
 struct amdgpu_hive_info *hive;
 struct amdgpu_xgmi  *entry;
 struct amdgpu_device *tmp_adev;
@@ -76,7 +76,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 adev->gmc.xgmi.node_id = psp_xgmi_get_node_id(&adev->psp);
 adev->gmc.xgmi.hive_id = psp_xgmi_get_hive_id(&adev->psp);

-   memset(&tmp_topology, 0, sizeof(tmp_topology));
+   tmp_topology = kzalloc(sizeof(struct psp_xgmi_topology_info), 
GFP_KERNEL);
 mutex_lock(&xgmi_mutex);
 hive = amdgpu_get_xgmi_hive(adev);
 if (!hive)
@@ -84,9 +84,9 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)

 list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
 list_for_each_entry(entry, &hive->device_list, head)
-   tmp_topology.nodes[count++].node_id = entry->node_id;
+   tmp_topology->nodes[count++].node_id = entry->node_id;

-   ret = psp_xgmi_get_topology_info(&adev->psp, count, &tmp_topology);
+   ret = psp_xgmi_get_topology_info(&adev->psp, count, tmp_topology);
 if (ret) {
 dev_err(adev->dev,
 "XGMI: Get topology failure on device %llx, hive %llx, 
ret %d",
@@ -96,7 +96,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 }
 /* Each psp need to set the latest topology */
 list_for_each_entry(tmp_adev, &hive->device_list, gmc.xgmi.head) {
-   ret = psp_xgmi_set_topology_info(&tmp_adev->psp, count, 
&tmp_topology);
+   ret = psp_xgmi_set_topology_info(&tmp_adev->psp, count, 
tmp_topology);
 if (ret) {
 dev_err(tmp_adev->dev,
 "XGMI: Set topology failure on device %llx, 
hive %llx, ret %d",
@@ -113,5 +113,6 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)

 exit:
 mutex_unlock(&xgmi_mutex);
+   kfree(tmp_topology);
 return ret;
 }
--
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] drm/amdgpu: fix frame size of amdgpu_xgmi_add_devices excceed 1024 bytes

[Yuan, Xiaojie]

Would be better to add a is-null check for 'tmp_topology' after kzalloc(). With 
that fixed, patch is

Reviewed-by: Xiaojie Yuan 


Regards,

Xiaojie


From: amd-gfx  on behalf of Hawking 
Zhang 
Sent: Wednesday, November 7, 2018 11:29 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking
Subject: [PATCH] drm/amdgpu: fix frame size of amdgpu_xgmi_add_devices excceed 
1024 bytes

Instead of stack-allocated psp_xgmi_topology_info in function
amdgpu_xgmi_add_device, dynamically allocated this structure to
avoid the frame size of this function excceed 1024 bytes.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index e92b454..1c0ed13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -63,7 +63,7 @@ static struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct 
amdgpu_device *adev)

 int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 {
-   struct psp_xgmi_topology_info tmp_topology;
+   struct psp_xgmi_topology_info *tmp_topology;
 struct amdgpu_hive_info *hive;
 struct amdgpu_xgmi  *entry;
 struct amdgpu_device *tmp_adev;
@@ -76,7 +76,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 adev->gmc.xgmi.node_id = psp_xgmi_get_node_id(&adev->psp);
 adev->gmc.xgmi.hive_id = psp_xgmi_get_hive_id(&adev->psp);

-   memset(&tmp_topology, 0, sizeof(tmp_topology));
+   tmp_topology = kzalloc(sizeof(struct psp_xgmi_topology_info), 
GFP_KERNEL);
 mutex_lock(&xgmi_mutex);
 hive = amdgpu_get_xgmi_hive(adev);
 if (!hive)
@@ -84,9 +84,9 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)

 list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
 list_for_each_entry(entry, &hive->device_list, head)
-   tmp_topology.nodes[count++].node_id = entry->node_id;
+   tmp_topology->nodes[count++].node_id = entry->node_id;

-   ret = psp_xgmi_get_topology_info(&adev->psp, count, &tmp_topology);
+   ret = psp_xgmi_get_topology_info(&adev->psp, count, tmp_topology);
 if (ret) {
 dev_err(adev->dev,
 "XGMI: Get topology failure on device %llx, hive %llx, 
ret %d",
@@ -96,7 +96,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 }
 /* Each psp need to set the latest topology */
 list_for_each_entry(tmp_adev, &hive->device_list, gmc.xgmi.head) {
-   ret = psp_xgmi_set_topology_info(&tmp_adev->psp, count, 
&tmp_topology);
+   ret = psp_xgmi_set_topology_info(&tmp_adev->psp, count, 
tmp_topology);
 if (ret) {
 dev_err(tmp_adev->dev,
 "XGMI: Set topology failure on device %llx, 
hive %llx, ret %d",
@@ -113,5 +113,6 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)

 exit:
 mutex_unlock(&xgmi_mutex);
+   kfree(tmp_topology);
 return ret;
 }
--
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] drm/amdgpu: fix frame size of amdgpu_xgmi_add_devices excceed 1024 bytes

[Hawking Zhang]

Instead of stack-allocated psp_xgmi_topology_info in function
amdgpu_xgmi_add_device, dynamically allocated this structure to
avoid the frame size of this function excceed 1024 bytes.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index e92b454..1c0ed13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -63,7 +63,7 @@ static struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct 
amdgpu_device *adev)
 
 int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 {
-   struct psp_xgmi_topology_info tmp_topology;
+   struct psp_xgmi_topology_info *tmp_topology;
struct amdgpu_hive_info *hive;
struct amdgpu_xgmi  *entry;
struct amdgpu_device*tmp_adev;
@@ -76,7 +76,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
adev->gmc.xgmi.node_id = psp_xgmi_get_node_id(&adev->psp);
adev->gmc.xgmi.hive_id = psp_xgmi_get_hive_id(&adev->psp);
 
-   memset(&tmp_topology, 0, sizeof(tmp_topology));
+   tmp_topology = kzalloc(sizeof(struct psp_xgmi_topology_info), 
GFP_KERNEL);
mutex_lock(&xgmi_mutex);
hive = amdgpu_get_xgmi_hive(adev);
if (!hive)
@@ -84,9 +84,9 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 
list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
list_for_each_entry(entry, &hive->device_list, head)
-   tmp_topology.nodes[count++].node_id = entry->node_id;
+   tmp_topology->nodes[count++].node_id = entry->node_id;
 
-   ret = psp_xgmi_get_topology_info(&adev->psp, count, &tmp_topology);
+   ret = psp_xgmi_get_topology_info(&adev->psp, count, tmp_topology);
if (ret) {
dev_err(adev->dev,
"XGMI: Get topology failure on device %llx, hive %llx, 
ret %d",
@@ -96,7 +96,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
}
/* Each psp need to set the latest topology */
list_for_each_entry(tmp_adev, &hive->device_list, gmc.xgmi.head) {
-   ret = psp_xgmi_set_topology_info(&tmp_adev->psp, count, 
&tmp_topology);
+   ret = psp_xgmi_set_topology_info(&tmp_adev->psp, count, 
tmp_topology);
if (ret) {
dev_err(tmp_adev->dev,
"XGMI: Set topology failure on device %llx, 
hive %llx, ret %d",
@@ -113,5 +113,6 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
 
 exit:
mutex_unlock(&xgmi_mutex);
+   kfree(tmp_topology);
return ret;
 }
-- 
2.7.4

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

RE: [PATCH 1/3] drm/amd/powerplay: set a default fclk/gfxclk ratio

[Xu, Feifei]

Seriel is reviewed-by: Feifei Xu 

-Original Message-
From: amd-gfx  On Behalf Of Evan Quan
Sent: Wednesday, November 7, 2018 9:38 AM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan 
Subject: [PATCH 1/3] drm/amd/powerplay: set a default fclk/gfxclk ratio

Otherwise big gap between these two clocks may causes some hangs.

Change-Id: Ifa3fafe2ee619d6231d5ecab61d3c68faa34abb6
Signed-off-by: Evan Quan 
---
 .../gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c   | 16 
 .../gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h   |  1 +
 drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h |  3 ++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index d2da9e3fc827..4f0f444fd111 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -120,6 +120,7 @@ static void vega20_set_default_registry_data(struct 
pp_hwmgr *hwmgr)
data->registry_data.disable_auto_wattman = 1;
data->registry_data.auto_wattman_debug = 0;
data->registry_data.auto_wattman_sample_period = 100;
+   data->registry_data.fclk_gfxclk_ratio = 0x3F6D;
data->registry_data.auto_wattman_threshold = 50;
data->registry_data.gfxoff_controlled_by_driver = 1;
data->gfxoff_allowed = false;
@@ -829,6 +830,16 @@ static int vega20_enable_all_smu_features(struct pp_hwmgr 
*hwmgr)
return 0;
 }
 
+static int vega20_send_clock_ratio(struct pp_hwmgr *hwmgr) {
+   struct vega20_hwmgr *data =
+   (struct vega20_hwmgr *)(hwmgr->backend);
+
+   return smum_send_msg_to_smc_with_parameter(hwmgr,
+   PPSMC_MSG_SetFclkGfxClkRatio,
+   data->registry_data.fclk_gfxclk_ratio);
+}
+
 static int vega20_disable_all_smu_features(struct pp_hwmgr *hwmgr)  {
struct vega20_hwmgr *data =
@@ -1535,6 +1546,11 @@ static int vega20_enable_dpm_tasks(struct pp_hwmgr 
*hwmgr)
"[EnableDPMTasks] Failed to enable all smu features!",
return result);
 
+   result = vega20_send_clock_ratio(hwmgr);
+   PP_ASSERT_WITH_CODE(!result,
+   "[EnableDPMTasks] Failed to send clock ratio!",
+   return result);
+
/* Initialize UVD/VCE powergating state */
vega20_init_powergate_state(hwmgr);
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
index 56fe6a0d42e8..25faaa5c5b10 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
@@ -328,6 +328,7 @@ struct vega20_registry_data {
uint8_t   disable_auto_wattman;
uint32_t  auto_wattman_debug;
uint32_t  auto_wattman_sample_period;
+   uint32_t  fclk_gfxclk_ratio;
uint8_t   auto_wattman_threshold;
uint8_t   log_avfs_param;
uint8_t   enable_enginess;
diff --git a/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h 
b/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h
index 45d64a81e945..4f63a736ea0e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h
@@ -105,7 +105,8 @@
 #define PPSMC_MSG_SetSystemVirtualDramAddrHigh   0x4B
 #define PPSMC_MSG_SetSystemVirtualDramAddrLow0x4C
 #define PPSMC_MSG_WaflTest   0x4D
-// Unused ID 0x4E to 0x50
+#define PPSMC_MSG_SetFclkGfxClkRatio 0x4E
+// Unused ID 0x4F to 0x50
 #define PPSMC_MSG_AllowGfxOff0x51
 #define PPSMC_MSG_DisallowGfxOff 0x52
 #define PPSMC_MSG_GetPptLimit0x53
--
2.19.1

___
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 1/3] drm/amd/powerplay: set a default fclk/gfxclk ratio

[Evan Quan]

Otherwise big gap between these two clocks may causes
some hangs.

Change-Id: Ifa3fafe2ee619d6231d5ecab61d3c68faa34abb6
Signed-off-by: Evan Quan 
---
 .../gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c   | 16 
 .../gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h   |  1 +
 drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h |  3 ++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index d2da9e3fc827..4f0f444fd111 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -120,6 +120,7 @@ static void vega20_set_default_registry_data(struct 
pp_hwmgr *hwmgr)
data->registry_data.disable_auto_wattman = 1;
data->registry_data.auto_wattman_debug = 0;
data->registry_data.auto_wattman_sample_period = 100;
+   data->registry_data.fclk_gfxclk_ratio = 0x3F6D;
data->registry_data.auto_wattman_threshold = 50;
data->registry_data.gfxoff_controlled_by_driver = 1;
data->gfxoff_allowed = false;
@@ -829,6 +830,16 @@ static int vega20_enable_all_smu_features(struct pp_hwmgr 
*hwmgr)
return 0;
 }
 
+static int vega20_send_clock_ratio(struct pp_hwmgr *hwmgr)
+{
+   struct vega20_hwmgr *data =
+   (struct vega20_hwmgr *)(hwmgr->backend);
+
+   return smum_send_msg_to_smc_with_parameter(hwmgr,
+   PPSMC_MSG_SetFclkGfxClkRatio,
+   data->registry_data.fclk_gfxclk_ratio);
+}
+
 static int vega20_disable_all_smu_features(struct pp_hwmgr *hwmgr)
 {
struct vega20_hwmgr *data =
@@ -1535,6 +1546,11 @@ static int vega20_enable_dpm_tasks(struct pp_hwmgr 
*hwmgr)
"[EnableDPMTasks] Failed to enable all smu features!",
return result);
 
+   result = vega20_send_clock_ratio(hwmgr);
+   PP_ASSERT_WITH_CODE(!result,
+   "[EnableDPMTasks] Failed to send clock ratio!",
+   return result);
+
/* Initialize UVD/VCE powergating state */
vega20_init_powergate_state(hwmgr);
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
index 56fe6a0d42e8..25faaa5c5b10 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
@@ -328,6 +328,7 @@ struct vega20_registry_data {
uint8_t   disable_auto_wattman;
uint32_t  auto_wattman_debug;
uint32_t  auto_wattman_sample_period;
+   uint32_t  fclk_gfxclk_ratio;
uint8_t   auto_wattman_threshold;
uint8_t   log_avfs_param;
uint8_t   enable_enginess;
diff --git a/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h 
b/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h
index 45d64a81e945..4f63a736ea0e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/vega20_ppsmc.h
@@ -105,7 +105,8 @@
 #define PPSMC_MSG_SetSystemVirtualDramAddrHigh   0x4B
 #define PPSMC_MSG_SetSystemVirtualDramAddrLow0x4C
 #define PPSMC_MSG_WaflTest   0x4D
-// Unused ID 0x4E to 0x50
+#define PPSMC_MSG_SetFclkGfxClkRatio 0x4E
+// Unused ID 0x4F to 0x50
 #define PPSMC_MSG_AllowGfxOff0x51
 #define PPSMC_MSG_DisallowGfxOff 0x52
 #define PPSMC_MSG_GetPptLimit0x53
-- 
2.19.1

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

[PATCH 2/3] drm/amd/powerplay: always use fast UCLK switching when UCLK DPM enabled

[Evan Quan]

With UCLK DPM enabled, slow switching is not supported any more.

Change-Id: I6242e782441272487aebd161836868785a6f7ee8
Signed-off-by: Evan Quan 
---
 .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c| 37 +--
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 4f0f444fd111..91956471cd0f 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -830,6 +830,18 @@ static int vega20_enable_all_smu_features(struct pp_hwmgr 
*hwmgr)
return 0;
 }
 
+static int vega20_notify_smc_display_change(struct pp_hwmgr *hwmgr)
+{
+   struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
+
+   if (data->smu_features[GNLD_DPM_UCLK].enabled)
+   return smum_send_msg_to_smc_with_parameter(hwmgr,
+   PPSMC_MSG_SetUclkFastSwitch,
+   1);
+
+   return 0;
+}
+
 static int vega20_send_clock_ratio(struct pp_hwmgr *hwmgr)
 {
struct vega20_hwmgr *data =
@@ -1546,6 +1558,11 @@ static int vega20_enable_dpm_tasks(struct pp_hwmgr 
*hwmgr)
"[EnableDPMTasks] Failed to enable all smu features!",
return result);
 
+   result = vega20_notify_smc_display_change(hwmgr);
+   PP_ASSERT_WITH_CODE(!result,
+   "[EnableDPMTasks] Failed to notify smc display change!",
+   return result);
+
result = vega20_send_clock_ratio(hwmgr);
PP_ASSERT_WITH_CODE(!result,
"[EnableDPMTasks] Failed to send clock ratio!",
@@ -1991,19 +2008,6 @@ static int vega20_read_sensor(struct pp_hwmgr *hwmgr, 
int idx,
return ret;
 }
 
-static int vega20_notify_smc_display_change(struct pp_hwmgr *hwmgr,
-   bool has_disp)
-{
-   struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
-
-   if (data->smu_features[GNLD_DPM_UCLK].enabled)
-   return smum_send_msg_to_smc_with_parameter(hwmgr,
-   PPSMC_MSG_SetUclkFastSwitch,
-   has_disp ? 1 : 0);
-
-   return 0;
-}
-
 int vega20_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
struct pp_display_clock_request *clock_req)
 {
@@ -2063,13 +2067,6 @@ static int 
vega20_notify_smc_display_config_after_ps_adjustment(
struct pp_display_clock_request clock_req;
int ret = 0;
 
-   if ((hwmgr->display_config->num_display > 1) &&
-!hwmgr->display_config->multi_monitor_in_sync &&
-!hwmgr->display_config->nb_pstate_switch_disable)
-   vega20_notify_smc_display_change(hwmgr, false);
-   else
-   vega20_notify_smc_display_change(hwmgr, true);
-
min_clocks.dcefClock = hwmgr->display_config->min_dcef_set_clk;
min_clocks.dcefClockInSR = 
hwmgr->display_config->min_dcef_deep_sleep_set_clk;
min_clocks.memoryClock = hwmgr->display_config->min_mem_set_clock;
-- 
2.19.1

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

[PATCH 3/3] drm/amdgpu: set Vega20 LBPW as disabled at default

[Evan Quan]

For Vega20, LBPW feature is disabled at default.

Change-Id: I184520cbb03ab8cba9321cd94d1deb0ce38b7e17
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index dcf1f79ab347..5bb0cca5b71f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -2503,6 +2503,20 @@ static int gfx_v9_0_rlc_resume(struct amdgpu_device 
*adev)
return r;
}
 
+   if (amdgpu_lbpw == -1) {
+   switch (adev->asic_type) {
+   case CHIP_RAVEN:
+   amdgpu_lbpw = 1;
+   break;
+   case CHIP_VEGA20:
+   amdgpu_lbpw = 0;
+   break;
+   default:
+   amdgpu_lbpw = 0;
+   break;
+   }
+   }
+
if (adev->asic_type == CHIP_RAVEN ||
adev->asic_type == CHIP_VEGA20) {
if (amdgpu_lbpw != 0)
-- 
2.19.1

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

Re: [PATCH] drm/ttm: Fix bo_global and mem_global kfree error

[Zhang, Jerry(Junwei)]

On 11/6/18 7:59 PM, Christian König wrote:

Am 06.11.18 um 12:54 schrieb Trigger Huang:

ttm_bo_glob and ttm_mem_glob are defined as structure instance, while
not allocated by kzalloc, so kfree should not be invoked to release
them anymore. Otherwise, it will cause the following kernel BUG when
unloading amdgpu module

[   48.419294] kernel BUG at 
/build/linux-5s7Xkn/linux-4.15.0/mm/slub.c:3894!

[   48.419352] invalid opcode:  [#1] SMP PTI
[   48.419387] Modules linked in: amdgpu(OE-) amdchash(OE) amdttm(OE) 
amd_sched(OE) amdkcl(OE) amd_iommu_v2 drm_kms_helper drm i2c_algo_bit 
fb_sys_fops syscopyarea sysfillrect sysimgblt snd_hda_codec_generic 
snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep kvm_intel kvm 
irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_pcm 
snd_seq_midi snd_seq_midi_event snd_rawmidi pcbc snd_seq 
snd_seq_device snd_timer aesni_intel snd soundcore joydev aes_x86_64 
crypto_simd glue_helper cryptd input_leds mac_hid serio_raw 
binfmt_misc nfsd auth_rpcgss nfs_acl lockd grace sunrpc sch_fq_codel 
parport_pc ppdev lp parport ip_tables x_tables autofs4 8139too 
psmouse i2c_piix4 8139cp mii floppy pata_acpi
[   48.419782] CPU: 1 PID: 1281 Comm: modprobe Tainted: G   
OE    4.15.0-20-generic #21-Ubuntu
[   48.419838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014

[   48.419901] RIP: 0010:kfree+0x137/0x180
[   48.419934] RSP: 0018:b02101273bf8 EFLAGS: 00010246
[   48.419974] RAX: eee1418ad7e0 RBX: c075f100 RCX: 
8fed7fca7ed0
[   48.420025] RDX:  RSI: 0003440e RDI: 
2240
[   48.420073] RBP: b02101273c10 R08: 0010 R09: 
8fed7ffd3680
[   48.420121] R10: eee1418ad7c0 R11: 8fed7ffd3000 R12: 
c075e2c0
[   48.420169] R13: c074ec10 R14: 8fed73063900 R15: 
8fed737428e8
[   48.420216] FS:  7fdc912ec540() GS:8fed7fc8() 
knlGS:

[   48.420267] CS:  0010 DS:  ES:  CR0: 80050033
[   48.420308] CR2: 55fa40c30060 CR3: 00023470a006 CR4: 
003606e0
[   48.420358] DR0:  DR1:  DR2: 

[   48.420405] DR3:  DR6: fffe0ff0 DR7: 
0400

[   48.420452] Call Trace:
[   48.420485]  ttm_bo_global_kobj_release+0x20/0x30 [amdttm]
[   48.420528]  kobject_release+0x6a/0x180
[   48.420562]  kobject_put+0x28/0x50
[   48.420595]  ttm_bo_global_release+0x36/0x50 [amdttm]
[   48.420636]  amdttm_bo_device_release+0x119/0x180 [amdttm]
[   48.420678]  ? amdttm_bo_clean_mm+0xa6/0xf0 [amdttm]
[   48.420760]  amdgpu_ttm_fini+0xc9/0x180 [amdgpu]
[   48.420821]  amdgpu_bo_fini+0x12/0x40 [amdgpu]
[   48.420889]  gmc_v9_0_sw_fini+0x40/0x50 [amdgpu]
[   48.420947]  amdgpu_device_fini+0x36f/0x4c0 [amdgpu]
[   48.421007]  amdgpu_driver_unload_kms+0xb4/0x150 [amdgpu]
[   48.421058]  drm_dev_unregister+0x46/0xf0 [drm]
[   48.421102]  drm_dev_unplug+0x12/0x70 [drm]

Signed-off-by: Trigger Huang 


Reviewed-by: Christian König 


Reviewed-by: Junwei Zhang 




---
  drivers/gpu/drm/ttm/ttm_bo.c | 1 -
  drivers/gpu/drm/ttm/ttm_memory.c | 9 -
  2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index df02880..01c6d14 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1527,7 +1527,6 @@ static void ttm_bo_global_kobj_release(struct 
kobject *kobj)

  container_of(kobj, struct ttm_bo_global, kobj);
    __free_page(glob->dummy_read_page);
-    kfree(glob);
  }
    static void ttm_bo_global_release(void)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c 
b/drivers/gpu/drm/ttm/ttm_memory.c

index 7704e17..f1567c3 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -219,14 +219,6 @@ static ssize_t ttm_mem_global_store(struct 
kobject *kobj,

  return size;
  }
  -static void ttm_mem_global_kobj_release(struct kobject *kobj)
-{
-    struct ttm_mem_global *glob =
-    container_of(kobj, struct ttm_mem_global, kobj);
-
-    kfree(glob);
-}
-
  static struct attribute *ttm_mem_global_attrs[] = {
  &ttm_mem_global_lower_mem_limit,
  NULL
@@ -238,7 +230,6 @@ static const struct sysfs_ops ttm_mem_global_ops = {
  };
    static struct kobj_type ttm_mem_glob_kobj_type = {
-    .release = &ttm_mem_global_kobj_release,
  .sysfs_ops = &ttm_mem_global_ops,
  .default_attrs = ttm_mem_global_attrs,
  };


___
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/9] drm/amdgpu: KFD Restore process: Optimize waiting

[Alex Deucher]

On Mon, Nov 5, 2018 at 8:41 PM Kuehling, Felix  wrote:
>
> From: Harish Kasiviswanathan 
>
> Instead of waiting for each KFD BO after validation just wait for the
> last BO moving fence.
>
> Signed-off-by: Harish Kasiviswanathan 
> Reviewed-by: Felix Kuehling 
> Signed-off-by: Felix Kuehling 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index e124d2d..d005371 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -2084,7 +2084,12 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void 
> *info, struct dma_fence **ef)
> pr_debug("Memory eviction: Validate BOs failed. Try 
> again\n");
> goto validate_map_fail;
> }
> -
> +   ret = amdgpu_sync_fence(amdgpu_ttm_adev(bo->tbo.bdev),
> +   &sync_obj, bo->tbo.moving, false);
> +   if (ret) {
> +   pr_debug("Memory eviction: Sync BO fence failed. Try 
> again\n");
> +   goto validate_map_fail;
> +   }
> list_for_each_entry(bo_va_entry, &mem->bo_va_list,
> bo_list) {
> ret = update_gpuvm_pte((struct amdgpu_device *)
> @@ -2105,6 +2110,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, 
> struct dma_fence **ef)
> goto validate_map_fail;
> }
>
> +   /* Wait for validate and PT updates to finish */
> amdgpu_sync_wait(&sync_obj, false);
>
> /* Release old eviction fence and create new one, because fence only
> @@ -2123,10 +2129,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void 
> *info, struct dma_fence **ef)
> process_info->eviction_fence = new_fence;
> *ef = dma_fence_get(&new_fence->base);
>
> -   /* Wait for validate to finish and attach new eviction fence */
> -   list_for_each_entry(mem, &process_info->kfd_bo_list,
> -   validate_list.head)
> -   ttm_bo_wait(&mem->bo->tbo, false, false);
> +   /* Attach new eviction fence to all BOs */
> list_for_each_entry(mem, &process_info->kfd_bo_list,
> validate_list.head)
> amdgpu_bo_fence(mem->bo,
> --
> 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 5/9] drm/amdgpu: Remove explicit wait after VM validate

[Alex Deucher]

On Mon, Nov 5, 2018 at 8:41 PM Kuehling, Felix  wrote:
>
> From: Harish Kasiviswanathan 
>
> PD or PT might have to be moved during validation and this move has to be
> completed before updating it. If page table updates are done using SDMA
> then this serializing is done by SDMA command submission.
>
> And if PD/PT updates are done by CPU, then explicit waiting for PD/PT
> updates are done in amdgpu VM amdgpu_vm_wait_pd function.
>
> Sync to PD BO moving fence to handle corner case where none of the PTs
> are updated but PD is evicted.
>
> Signed-off-by: Harish Kasiviswanathan 
> Reviewed-by: Felix Kuehling 
> Signed-off-by: Felix Kuehling 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 31 
> ++--
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 9a1b2b2..e124d2d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -901,6 +901,26 @@ static int process_validate_vms(struct 
> amdkfd_process_info *process_info)
> return 0;
>  }
>
> +static int process_sync_pds_resv(struct amdkfd_process_info *process_info,
> +struct amdgpu_sync *sync)
> +{
> +   struct amdgpu_vm *peer_vm;
> +   int ret;
> +
> +   list_for_each_entry(peer_vm, &process_info->vm_list_head,
> +   vm_list_node) {
> +   struct amdgpu_bo *pd = peer_vm->root.base.bo;
> +
> +   ret = amdgpu_sync_resv(amdgpu_ttm_adev(pd->tbo.bdev),
> +   sync, pd->tbo.resv,
> +   AMDGPU_FENCE_OWNER_UNDEFINED, false);
> +   if (ret)
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +
>  static int process_update_pds(struct amdkfd_process_info *process_info,
>   struct amdgpu_sync *sync)
>  {
> @@ -2045,13 +2065,10 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void 
> *info, struct dma_fence **ef)
> if (ret)
> goto validate_map_fail;
>
> -   /* Wait for PD/PTs validate to finish */
> -   /* FIXME: I think this isn't needed */
> -   list_for_each_entry(peer_vm, &process_info->vm_list_head,
> -   vm_list_node) {
> -   struct amdgpu_bo *bo = peer_vm->root.base.bo;
> -
> -   ttm_bo_wait(&bo->tbo, false, false);
> +   ret = process_sync_pds_resv(process_info, &sync_obj);
> +   if (ret) {
> +   pr_debug("Memory eviction: Failed to sync to PD BO moving 
> fence. Try again\n");
> +   goto validate_map_fail;
> }
>
> /* Validate BOs and map them to GPUVM (update VM page tables). */
> --
> 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 4/9] drm/amdkfd: Workaround PASID missing in gfx9 interrupt payload under non HWS

[Alex Deucher]

On Mon, Nov 5, 2018 at 8:41 PM Kuehling, Felix  wrote:
>
> From: Yong Zhao 
>
> This is a known gfx9 HW issue, and this change can perfectly workaround
> the issue.
>
> Signed-off-by: Yong Zhao 
> Reviewed-by: Felix Kuehling 
> Signed-off-by: Felix Kuehling 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c | 25 
> ++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> index 8497864..a85904a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> @@ -23,7 +23,7 @@
>  #include "kfd_priv.h"
>  #include "kfd_events.h"
>  #include "soc15_int.h"
> -
> +#include "kfd_device_queue_manager.h"
>
>  static bool event_interrupt_isr_v9(struct kfd_dev *dev,
> const uint32_t *ih_ring_entry,
> @@ -43,14 +43,33 @@ static bool event_interrupt_isr_v9(struct kfd_dev *dev,
> client_id = SOC15_CLIENT_ID_FROM_IH_ENTRY(ih_ring_entry);
> pasid = SOC15_PASID_FROM_IH_ENTRY(ih_ring_entry);
>
> +   /* This is a known issue for gfx9. Under non HWS, pasid is not set
> +* in the interrupt payload, so we need to find out the pasid on our
> +* own.
> +*/
> +   if (!pasid && dev->dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS) {
> +   const uint32_t pasid_mask = 0x;
> +
> +   *patched_flag = true;
> +   memcpy(patched_ihre, ih_ring_entry,
> +   dev->device_info->ih_ring_entry_size);
> +
> +   pasid = dev->kfd2kgd->get_atc_vmid_pasid_mapping_pasid(
> +   dev->kgd, vmid);
> +
> +   /* Patch the pasid field */
> +   patched_ihre[3] = cpu_to_le32((le32_to_cpu(patched_ihre[3])
> +   & ~pasid_mask) | pasid);
> +   }
> +
> pr_debug("client id 0x%x, source id %d, vmid %d, pasid 0x%x. raw 
> data:\n",
>  client_id, source_id, vmid, pasid);
> pr_debug("%8X, %8X, %8X, %8X, %8X, %8X, %8X, %8X.\n",
>  data[0], data[1], data[2], data[3],
>  data[4], data[5], data[6], data[7]);
>
> -   /* If there is no valid PASID, it's likely a firmware bug */
> -   if (WARN_ONCE(pasid == 0, "FW bug: No PASID in KFD interrupt"))
> +   /* If there is no valid PASID, it's likely a bug */
> +   if (WARN_ONCE(pasid == 0, "Bug: No PASID in KFD interrupt"))
> return 0;
>
> /* Interrupt types we care about: various signals and faults.
> --
> 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 3/9] drm/amdkfd: Adjust the debug message in KFD ISR

[Alex Deucher]

On Mon, Nov 5, 2018 at 8:40 PM Kuehling, Felix  wrote:
>
> From: Yong Zhao 
>
> This makes debug message get printed even when there is early return.
>
> Signed-off-by: Yong Zhao 
> Reviewed-by: Felix Kuehling 
> Signed-off-by: Felix Kuehling 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> index f836897..8497864 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> @@ -39,20 +39,20 @@ static bool event_interrupt_isr_v9(struct kfd_dev *dev,
> vmid > dev->vm_info.last_vmid_kfd)
> return 0;
>
> -   /* If there is no valid PASID, it's likely a firmware bug */
> -   pasid = SOC15_PASID_FROM_IH_ENTRY(ih_ring_entry);
> -   if (WARN_ONCE(pasid == 0, "FW bug: No PASID in KFD interrupt"))
> -   return 0;
> -
> source_id = SOC15_SOURCE_ID_FROM_IH_ENTRY(ih_ring_entry);
> client_id = SOC15_CLIENT_ID_FROM_IH_ENTRY(ih_ring_entry);
> +   pasid = SOC15_PASID_FROM_IH_ENTRY(ih_ring_entry);
>
> -   pr_debug("client id 0x%x, source id %d, pasid 0x%x. raw data:\n",
> -client_id, source_id, pasid);
> +   pr_debug("client id 0x%x, source id %d, vmid %d, pasid 0x%x. raw 
> data:\n",
> +client_id, source_id, vmid, pasid);
> pr_debug("%8X, %8X, %8X, %8X, %8X, %8X, %8X, %8X.\n",
>  data[0], data[1], data[2], data[3],
>  data[4], data[5], data[6], data[7]);
>
> +   /* If there is no valid PASID, it's likely a firmware bug */
> +   if (WARN_ONCE(pasid == 0, "FW bug: No PASID in KFD interrupt"))
> +   return 0;
> +
> /* Interrupt types we care about: various signals and faults.
>  * They will be forwarded to a work queue (see below).
>  */
> --
> 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 2/9] drm/amdkfd: Added Vega12 and Polaris12 for KFD.

[Alex Deucher]

On Mon, Nov 5, 2018 at 8:41 PM Kuehling, Felix  wrote:
>
> From: Gang Ba 
>
> Add Vega12 and Polaris12 device info and device IDs to KFD.
>
> Signed-off-by: Gang Ba 
> Reviewed-by: Felix Kuehling 
> Signed-off-by: Felix Kuehling 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   |  3 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c  |  6 +++
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c| 45 
> ++
>  .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  |  2 +
>  drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c   |  2 +
>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c  |  2 +
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c   |  2 +
>  drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c|  2 +
>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c  |  2 +
>  10 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 60f9a87..a9c7597 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -73,9 +73,11 @@ void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev)
> case CHIP_FIJI:
> case CHIP_POLARIS10:
> case CHIP_POLARIS11:
> +   case CHIP_POLARIS12:
> kfd2kgd = amdgpu_amdkfd_gfx_8_0_get_functions();
> break;
> case CHIP_VEGA10:
> +   case CHIP_VEGA12:
> case CHIP_VEGA20:
> case CHIP_RAVEN:
> kfd2kgd = amdgpu_amdkfd_gfx_9_0_get_functions();
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index df0a059..9a1b2b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1199,7 +1199,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> byte_align = (adev->family == AMDGPU_FAMILY_VI &&
> adev->asic_type != CHIP_FIJI &&
> adev->asic_type != CHIP_POLARIS10 &&
> -   adev->asic_type != CHIP_POLARIS11) ?
> +   adev->asic_type != CHIP_POLARIS11 &&
> +   adev->asic_type != CHIP_POLARIS12) ?
> VI_BO_SIZE_ALIGN : 1;
>
> mapping_flags = AMDGPU_VM_PAGE_READABLE;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 3783d12..c02adbb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -133,6 +133,7 @@ static struct kfd_gpu_cache_info carrizo_cache_info[] = {
>  #define fiji_cache_info  carrizo_cache_info
>  #define polaris10_cache_info carrizo_cache_info
>  #define polaris11_cache_info carrizo_cache_info
> +#define polaris12_cache_info carrizo_cache_info
>  /* TODO - check & update Vega10 cache details */
>  #define vega10_cache_info carrizo_cache_info
>  #define raven_cache_info carrizo_cache_info
> @@ -647,7 +648,12 @@ static int kfd_fill_gpu_cache_info(struct kfd_dev *kdev,
> pcache_info = polaris11_cache_info;
> num_of_cache_types = ARRAY_SIZE(polaris11_cache_info);
> break;
> +   case CHIP_POLARIS12:
> +   pcache_info = polaris12_cache_info;
> +   num_of_cache_types = ARRAY_SIZE(polaris12_cache_info);
> +   break;
> case CHIP_VEGA10:
> +   case CHIP_VEGA12:
> case CHIP_VEGA20:
> pcache_info = vega10_cache_info;
> num_of_cache_types = ARRAY_SIZE(vega10_cache_info);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index c004647..9ed14a1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -205,6 +205,22 @@ static const struct kfd_device_info 
> polaris11_device_info = {
> .num_sdma_queues_per_engine = 2,
>  };
>
> +static const struct kfd_device_info polaris12_device_info = {
> +   .asic_family = CHIP_POLARIS12,
> +   .max_pasid_bits = 16,
> +   .max_no_of_hqd  = 24,
> +   .doorbell_size  = 4,
> +   .ih_ring_entry_size = 4 * sizeof(uint32_t),
> +   .event_interrupt_class = &event_interrupt_class_cik,
> +   .num_of_watch_points = 4,
> +   .mqd_size_aligned = MQD_SIZE_ALIGNED,
> +   .supports_cwsr = true,
> +   .needs_iommu_device = false,
> +   .needs_pci_atomics = true,
> +   .num_sdma_engines = 2,
> +   .num_sdma_queues_per_engine = 2,
> +};
> +
>  static const struct kfd_device_info vega10_device_info = {
> .asic_family = CHIP_VEGA10,
> .max_pasid_bits = 16,
> @@ -237,6 +253,22 @@ static const struct kfd_device_info 
> vega10_vf_device_info = {
> .num_sdma_queues_per_engine = 2,
>  };
>
> +static const struct kfd

Re: [PATCH 1/9] drm/amdkfd: Replace mqd with mqd_mgr as the variable name for mqd_manager

[Alex Deucher]

On Mon, Nov 5, 2018 at 8:40 PM Kuehling, Felix  wrote:
>
> From: Yong Zhao 
>
> This will make reading code much easier. This fixes a few spots missed in a
> previous commit with the same title.
>
> Signed-off-by: Yong Zhao 
> Reviewed-by: Felix Kuehling 
> Signed-off-by: Felix Kuehling 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> 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 fb9d66e..d38efbb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1547,7 +1547,7 @@ static int get_wave_state(struct device_queue_manager 
> *dqm,
>   u32 *ctl_stack_used_size,
>   u32 *save_area_used_size)
>  {
> -   struct mqd_manager *mqd;
> +   struct mqd_manager *mqd_mgr;
> int r;
>
> dqm_lock(dqm);
> @@ -1558,19 +1558,19 @@ static int get_wave_state(struct device_queue_manager 
> *dqm,
> goto dqm_unlock;
> }
>
> -   mqd = dqm->ops.get_mqd_manager(dqm, KFD_MQD_TYPE_COMPUTE);
> -   if (!mqd) {
> +   mqd_mgr = dqm->ops.get_mqd_manager(dqm, KFD_MQD_TYPE_COMPUTE);
> +   if (!mqd_mgr) {
> r = -ENOMEM;
> goto dqm_unlock;
> }
>
> -   if (!mqd->get_wave_state) {
> +   if (!mqd_mgr->get_wave_state) {
> r = -EINVAL;
> goto dqm_unlock;
> }
>
> -   r = mqd->get_wave_state(mqd, q->mqd, ctl_stack, ctl_stack_used_size,
> -   save_area_used_size);
> +   r = mqd_mgr->get_wave_state(mqd_mgr, q->mqd, ctl_stack,
> +   ctl_stack_used_size, save_area_used_size);
>
>  dqm_unlock:
> dqm_unlock(dqm);
> --
> 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 5/5] drm/amdgpu/display/dce11: only enable FBC when selected

[Wentland, Harry]

On 2018-11-06 3:29 p.m., Alex Deucher wrote:
> Causes a black screen on a Stoney laptop.
> 
> bug: https://bugs.freedesktop.org/show_bug.cgi?id=108577
> Signed-off-by: Alex Deucher 

Thanks.

Series is
Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> index 1a90e1a38055..e33d11785b1f 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> @@ -1354,7 +1354,8 @@ static bool construct(
>   pool->base.sw_i2cs[i] = NULL;
>   }
>  
> - dc->fbc_compressor = dce110_compressor_create(ctx);
> + if (dc->config.fbc_support)
> + dc->fbc_compressor = dce110_compressor_create(ctx);
>  
>   if (!underlay_create(ctx, &pool->base))
>   goto res_create_fail;
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/5] drm/amdgpu/display: check if fbc is available in set_static_screen_control (v2)

[Alex Deucher]

The value is dependent on whether fbc is available.

v2: only check if num_pipes is valid

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 4789270c29d9..9724a17e352b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1750,7 +1750,12 @@ static void set_static_screen_control(struct pipe_ctx 
**pipe_ctx,
if (events->force_trigger)
value |= 0x1;
 
-   value |= 0x84;
+   if (num_pipes) {
+   struct dc *dc = pipe_ctx[0]->stream->ctx->dc;
+
+   if (dc->fbc_compressor)
+   value |= 0x84;
+   }
 
for (i = 0; i < num_pipes; i++)
pipe_ctx[i]->stream_res.tg->funcs->
-- 
2.13.6

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

[PATCH 5/5] drm/amdgpu/display/dce11: only enable FBC when selected

[Alex Deucher]

Causes a black screen on a Stoney laptop.

bug: https://bugs.freedesktop.org/show_bug.cgi?id=108577
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
index 1a90e1a38055..e33d11785b1f 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
@@ -1354,7 +1354,8 @@ static bool construct(
pool->base.sw_i2cs[i] = NULL;
}
 
-   dc->fbc_compressor = dce110_compressor_create(ctx);
+   if (dc->config.fbc_support)
+   dc->fbc_compressor = dce110_compressor_create(ctx);
 
if (!underlay_create(ctx, &pool->base))
goto res_create_fail;
-- 
2.13.6

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

[PATCH 3/5] drm/amdgpu/display/dc: add FBC to dc_config

[Alex Deucher]

Add FBC to the list of features that can be enabled from the DM.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/dc/dc.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 02db008fb78f..d16a20c84792 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -169,6 +169,7 @@ struct link_training_settings;
 struct dc_config {
bool gpu_vm_support;
bool disable_disp_pll_sharing;
+   bool fbc_support;
 };
 
 enum visual_confirm {
-- 
2.13.6

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

[PATCH 2/5] drm/amdgpu: add DC feature mask module parameter

[Alex Deucher]

Similar to ppfeaturemask.  Allows you to selectively enable/disable
DC features.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 11 +++
 drivers/gpu/drm/amd/include/amd_shared.h |  4 
 3 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 9dbdda66c318..42f882c633ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -152,6 +152,7 @@ extern int amdgpu_compute_multipipe;
 extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
+extern uint amdgpu_dc_feature_mask;
 extern struct amdgpu_mgpu_info mgpu_info;
 
 #ifdef CONFIG_DRM_AMDGPU_SI
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 943dbf3c5da1..8de55f7f1a3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -127,6 +127,9 @@ int amdgpu_compute_multipipe = -1;
 int amdgpu_gpu_recovery = -1; /* auto */
 int amdgpu_emu_mode = 0;
 uint amdgpu_smu_memory_pool_size = 0;
+/* FBC (bit 0) disabled by default*/
+uint amdgpu_dc_feature_mask = 0;
+
 struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
 };
@@ -631,6 +634,14 @@ module_param(halt_if_hws_hang, int, 0644);
 MODULE_PARM_DESC(halt_if_hws_hang, "Halt if HWS hang is detected (0 = off 
(default), 1 = on)");
 #endif
 
+/**
+ * DOC: dcfeaturemask (uint)
+ * Override display features enabled. See enum DC_FEATURE_MASK in 
drivers/gpu/drm/amd/include/amd_shared.h.
+ * The default is the current set of stable display features.
+ */
+MODULE_PARM_DESC(dcfeaturemask, "all stable DC features enabled (default))");
+module_param_named(dcfeaturemask, amdgpu_dc_feature_mask, uint, 0444);
+
 static const struct pci_device_id pciidlist[] = {
 #ifdef  CONFIG_DRM_AMDGPU_SI
{0x1002, 0x6780, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TAHITI},
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
b/drivers/gpu/drm/amd/include/amd_shared.h
index 2083c308007c..470d7b89071a 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -133,6 +133,10 @@ enum PP_FEATURE_MASK {
PP_AVFS_MASK = 0x4,
 };
 
+enum DC_FEATURE_MASK {
+   DC_FBC_MASK = 0x1,
+};
+
 /**
  * struct amd_ip_funcs - general hooks for managing amdgpu IP Blocks
  */
-- 
2.13.6

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

[PATCH 4/5] drm/amdgpu/display/dm: handle FBC dc feature parameter

[Alex Deucher]

Set the dc_config properly when the option is enabled.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
 1 file changed, 3 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 8c647ec1572f..b16de3754ab9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -434,6 +434,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
adev->asic_type < CHIP_RAVEN)
init_data.flags.gpu_vm_support = true;
 
+   if (amdgpu_dc_feature_mask & DC_FBC_MASK)
+   init_data.flags.fbc_support = true;
+
/* Display Core create. */
adev->dm.dc = dc_create(&init_data);
 
-- 
2.13.6

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

[PATCH v6 4/5] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal

[Nicholas Kazlauskas]

When variable refresh rate is active the hardware counter can return
a position >= vtotal. This results in a vpos being returned from
amdgpu_display_get_crtc_scanoutpos that's a positive value. The
positive value indicates to the caller that the display is
currently in scanout when the display is actually still in vblank.

This is because the vfront porch duration is unknown with variable
refresh active and will end when either a page flip occurs or the
timeout specified by the driver/display is reached.

The behavior of the amdgpu_display_get_crtc_scanoutpos remains the
same when the position is below vtotal. When the position is above
vtotal the function will return a value that is effectively -vbl_end,
the size of the vback porch.

The only caller affected by this change is the DRM helper for
calculating vblank timestamps. This change corrects behavior for
calculating the page flip timestap from being the previous timestamp
to the calculation to the next timestamp when position >= vtotal.

Signed-off-by: Nicholas Kazlauskas 
Cc: Michel Dänzer 
Cc: Harry Wentland 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6748cd7fc129..cb331319f225 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -850,7 +850,12 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device 
*dev,
/* Inside "upper part" of vblank area? Apply corrective offset if so: */
if (in_vbl && (*vpos >= vbl_start)) {
vtotal = mode->crtc_vtotal;
-   *vpos = *vpos - vtotal;
+
+   /* With variable refresh rate displays the vpos can exceed
+* the vtotal value. Clamp to 0 to return -vbl_end instead
+* of guessing the remaining number of lines until scanout.
+*/
+   *vpos = (*vpos < vtotal) ? (*vpos - vtotal) : 0;
}
 
/* Correct for shifted end of vbl at vbl_end. */
-- 
2.19.1

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

[PATCH v6 5/5] drm/amdgpu: Set FreeSync state using drm VRR properties

[Nicholas Kazlauskas]

Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.

The notify_freesync and set_freesync_property functions are dropped
from amdgpu_display_funcs.

The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.

The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.

To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required stream updates when planes are flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would need to be hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 3 files changed, 138 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..0cbe867ec375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -295,13 +295,6 @@ struct amdgpu_display_funcs {
  uint16_t connector_object_id,
  struct amdgpu_hpd *hpd,
  struct amdgpu_router *router);
-   /* it is used to enter or exit into free sync mode */
-   int (*notify_freesync)(struct drm_device *dev, void *data,
-  struct drm_file *filp);
-   /* it is used to allow enablement of freesync mode */
-   int (*set_freesync_property)(struct drm_connector *connector,
-struct drm_property *property,
-uint64_t val);
 
 
 };
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 b0df6dc9a775..53eb3d16f75c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1809,72 +1809,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(&ctx, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = &ctx;
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
&acrtc->base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff(&ctx);
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
-   return ret;
-}
 
 static const

[PATCH v6 0/5] A DRM API for adaptive sync and variable refresh rate support

[Nicholas Kazlauskas]

These patches are part of a proposed new interface for supporting variable 
refresh rate via DRM properties.

=== Changes from v5 ===

drm changes:

* Updated documentation to define userspace expectations when variable refresh 
rate is enabled

amd changes:

* Added patch to fix vblank timestamp calculations when vpos > vtotal

=== Changes from v4 ===

amd changes:

* Removed unused FreeSync functions from amdgpu

=== Changes from v3 ===

drm changes:

* Docstring and formatting fixes

amd/display changes:

* Updated commit message and debug statements

=== Changes from v2 ===

The interface has changed substantially from the last revision and the cover 
letter has been updated accordingly.

drm changes:

* Most "variable_refresh" prefixes in code have been shortened to just "vrr". 
This was motivated after changes to the interface had function names close to 
80 characters long. Comments from the mailing list were already shortening 
these in discussion as well.
* Documentation for "Variable Refresh" has been added to the KMS properties 
subsection for DRM driver developers.
* The drm_connector property "variable_refresh_capable" has been renamed to 
"vrr_capable".
* The drm_crtc property "variable_refresh" has been been renamed "vrr_enabled" 
to better reflect its usage.
* The drm_connector "variable_refresh_enabled" property has been removed. 
Managing this is now up to userspace when it sets "vrr_enabled".
* The "vrr_capable" property no longer has any state in drm_connector_state 
associated with it. The value can now be updated with the 
drm_connector_set_vrr_capable_property() function. This better matches the 
immutable flag on the property.
* The "variable_refresh_changed" flag has been removed from atomic helpers 
based on feedback from the mailing list and updated interface usage in the amd 
kernel driver.

amd/display changes:

* Updated interface usage based on the new properties
* Updated VRR infopacket handling based on new xf86-video-amdgpu usage

=== Adaptive sync and variable refresh rate ===

Adaptive sync is part of the DisplayPort specification and allows for graphics 
adapters to drive displays with varying frame timings.

Variable refresh rate (VRR) is essentially the same, but defined for HDMI.

=== Use cases for variable refresh rate ===

Variable frame (flip) timings don't align well with fixed refresh rate 
displays. This results in stuttering, tearing and/or input lag. By adjusting 
the display refresh rate dynamically these issues can be reduced or eliminated.

However, not all content is suitable for dynamic refresh adaptation. The user 
may experience "flickering" from differences in panel luminance at different 
refresh rates. Content that flips at an infrequent rate or demand is more 
likely to cause large changes in refresh rate that result in flickering.

Userland needs a way to let the driver know when the screen content is suitable 
for variable refresh rates.

=== DRM API to support variable refresh rates ===

This patch introduces a new API via atomic properties on the DRM connector and 
CRTC.

The drm_connector has one new optional boolean property:

* bool vrr_capable - set by the driver if the hardware is capable of supporting 
variable refresh rates

The drm_crtc has one new default boolean property:

* bool vrr_enabled - set by userspace indicating that variable refresh rate 
should be enabled

== Overview for DRM driver developers ===

Driver developers can attach the "vrr_capable" property by calling 
drm_connector_attach_vrr_capable_property(). This should be done on connectors 
that could be capable of supporting variable refresh rates (such as DP or HDMI).

The "vrr_capable" can then be updated accordingly by calling 
drm_connector_set_vrr_capable_property().

The "vrr_enabled" property can be checked from the drm_crtc_state object.

=== Overview for Userland developers ==

The "vrr_enabled" property on the CRTC should be set to true when the CRTC is 
suitable for variable refresh rates.

To demonstrate the suitability of the API for variable refresh and dynamic 
adaptation there are additional patches using this API that implement adaptive 
variable refresh across kernel and userland projects:

* DRM (dri-devel)
* amdgpu DRM kernel driver (amd-gfx)
* xf86-video-amdgpu 
(https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu)
* mesa (mesa-dev)

These patches enable adaptive variable refresh on X for AMD hardware. Support 
for variable refresh is disabled by default in xf86-video-amdgpu and will 
require additional user configuration.

The patches have been tested as working on upstream userland with the GNOME 
desktop environment under a single monitor setup. They also work on KDE in a 
single monitor setup with the compositor disabled.

The patches require that suitable applications flip via the Present extension 
to xf86-video-amdgpu for the entire Screen. Some compositors may interfere with 
this if they are unable to unredirect the window.

Full i

[PATCH v6 3/5] drm: Document variable refresh properties

[Nicholas Kazlauskas]

These include the drm_connector 'vrr_capable' and the drm_crtc
'vrr_enabled' properties.

Signed-off-by: Nicholas Kazlauskas 
Cc: Harry Wentland 
Cc: Manasi Navare 
Cc: Pekka Paalanen 
Cc: Ville Syrjälä 
Cc: Michel Dänzer 
---
 Documentation/gpu/drm-kms.rst   |  7 
 drivers/gpu/drm/drm_connector.c | 61 +
 2 files changed, 68 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..8da2a178cf85 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -575,6 +575,13 @@ Explicit Fencing Properties
 .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
:doc: explicit fencing properties
 
+
+Variable Refresh Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: Variable refresh properties
+
 Existing KMS Properties
 ---
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 49290060ab7b..a6adf5450db3 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,67 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * DOC: Variable refresh properties
+ *
+ * Variable refresh rate capable displays can dynamically adjust their
+ * refresh rate by extending the duration of their vertical porch until
+ * page flip or timeout occurs. This can reduce or remove stuttering
+ * and latency in scenarios where the page flip does not align with the
+ * vblank interval.
+ *
+ * An example scenario would be an application flipping at a constant rate
+ * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank
+ * interval and the same contents will be displayed twice. This can be
+ * observed as stuttering for content with motion.
+ *
+ * If variable refresh rate was active on a display that supported a
+ * variable refresh range from 35Hz to 60Hz no stuttering would be observable
+ * for the example scenario. The minimum supported variable refresh rate of
+ * 35Hz is below the page flip frequency and the vertical front porch can
+ * be extended until the page flip occurs. The vblank interval will be
+ * directly aligned to the page flip rate.
+ *
+ * Userspace control for variable refresh rate is supported via properties
+ * on the &drm_connector and &drm_crtc objects.
+ *
+ * "vrr_capable":
+ * Optional &drm_connector boolean property that drivers should attach
+ * with drm_connector_attach_vrr_capable_property() on connectors that
+ * could support variable refresh rates. Drivers should update the
+ * property value by calling drm_connector_set_vrr_capable_property().
+ *
+ * Absence of the property should indicate absence of support.
+ *
+ * "vrr_enabled":
+ * Default &drm_crtc boolean property that notifies the driver that the
+ * content on the CRTC is suitable for variable refresh rate presentation.
+ * The driver will take this property as a hint to enable variable
+ * refresh rate support if the receiver supports it, ie. if the
+ * "vrr_capable" property is true on the &drm_connector object. The
+ * veritcal front porch duration will be extended until page-flip or
+ * timeout when enabled.
+ *
+ * The minimum vertical front porch duration is defined as the vertical
+ * front porch duration for the current mode.
+ *
+ * The maximum vertical front porch duration is greater than or equal to
+ * the minimum vertical front porch duration. The duration is derived
+ * from the minimum supported variable refresh rate for the connector.
+ *
+ * The driver may place further restrictions within these minimum
+ * and maximum bounds.
+ *
+ * Some displays may exhibit noticeable differences in brightness when
+ * varying vertical front porch duration.
+ *
+ * The semantics for the vertical blank timestamp differ when
+ * variable refresh rate is active. The vertical blank timestamp
+ * is defined to be an estimate using the current mode's fixed
+ * refresh rate timings. The semantics for the page-flip event
+ * timestamp remain the same.
+ */
+
 /**
  * drm_connector_attach_vrr_capable_property - creates the
  * vrr_capable property
-- 
2.19.1

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

[PATCH v6 1/5] drm: Add vrr_capable property to the drm connector

[Nicholas Kazlauskas]

Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.

Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().

The value should be updated based on driver and hardware capabiltiy
by using drm_connector_set_vrr_capable_property().

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Manasi Navare 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/drm_connector.c | 49 +
 include/drm/drm_connector.h | 15 ++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4943cef178be..49290060ab7b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,37 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->vrr_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "vrr_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->vrr_capable_property = prop;
+   drm_object_attach_property(&connector->base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
@@ -1583,6 +1614,24 @@ void drm_connector_set_link_status_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+   struct drm_connector *connector, bool capable)
+{
+   drm_object_property_set_value(&connector->base,
+ connector->vrr_capable_property,
+ capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
 /**
  * drm_connector_init_panel_orientation_property -
  * initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9ccad6b062f2..4e6befff153b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -959,6 +959,17 @@ struct drm_connector {
 */
struct drm_property *scaling_mode_property;
 
+   /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+   struct drm_property *vrr_capable_property;
+
/**
 * @content_protection_property: DRM ENUM property for content
 * protection. See drm_connector_attach_content_protection_property().
@@ -1250,6 +1261,8 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
   u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1266,6 +1279,8 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
   const struct edid *edid);
 void drm_connector_set_link_status_property(struct drm_connector *connector,
uint64_t link_statu

[PATCH v6 2/5] drm: Add vrr_enabled property to drm CRTC

[Nicholas Kazlauskas]

This patch introduces the 'vrr_enabled' CRTC property to allow
dynamic control over variable refresh rate support for a CRTC.

This property should be treated like a content hint to the driver -
if the hardware or driver is not capable of driving variable refresh
timings then this is not considered an error.

Capability for variable refresh rate support should be determined
by querying the vrr_capable drm connector property.

It is worth noting that while the property is intended for atomic use
it isn't filtered from legacy userspace queries. This allows for Xorg
userspace drivers to implement support.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
Cc: Manasi Navare 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 
 drivers/gpu/drm/drm_crtc.c| 2 ++
 drivers/gpu/drm/drm_mode_config.c | 6 ++
 include/drm/drm_crtc.h| 9 +
 include/drm/drm_mode_config.h | 5 +
 5 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f315098c..eec396a57b88 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -433,6 +433,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
*crtc,
ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
drm_property_blob_put(mode);
return ret;
+   } else if (property == config->prop_vrr_enabled) {
+   state->vrr_enabled = val;
} else if (property == config->degamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
&state->degamma_lut,
@@ -491,6 +493,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = state->active;
else if (property == config->prop_mode_id)
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+   else if (property == config->prop_vrr_enabled)
+   *val = state->vrr_enabled;
else if (property == config->degamma_lut_property)
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 268a182ae189..6f8ddfcfaba5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -340,6 +340,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
struct drm_crtc *crtc,
drm_object_attach_property(&crtc->base, config->prop_mode_id, 
0);
drm_object_attach_property(&crtc->base,
   config->prop_out_fence_ptr, 0);
+   drm_object_attach_property(&crtc->base,
+  config->prop_vrr_enabled, 0);
}
 
return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index ee80788f2c40..5670c67f28d4 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -310,6 +310,12 @@ static int drm_mode_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_mode_id = prop;
 
+   prop = drm_property_create_bool(dev, 0,
+   "VRR_ENABLED");
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.prop_vrr_enabled = prop;
+
prop = drm_property_create(dev,
DRM_MODE_PROP_BLOB,
"DEGAMMA_LUT", 0);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b21437bc95bf..39c3900aab3c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -290,6 +290,15 @@ struct drm_crtc_state {
 */
u32 pageflip_flags;
 
+   /**
+* @vrr_enabled:
+*
+* Indicates if variable refresh rate should be enabled for the CRTC.
+* Support for the requested vrr state will depend on driver and
+* hardware capabiltiy - lacking support is not treated as failure.
+*/
+   bool vrr_enabled;
+
/**
 * @event:
 *
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 928e4172a0bb..49f2fcfdb5fc 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -639,6 +639,11 @@ struct drm_mode_config {
 * connectors must be of and active must be set to disabled, too.
 */
struct drm_property *prop_mode_id;
+   /**
+* @prop_vrr_enabled: Default atomic CRTC property to indicate
+* whether variable refresh rate should be enabled on the CRTC.
+*/
+   struct drm_property *prop_vrr_enabled;
 
/**
 * @dvi_i_subconnector_property: Optional DVI-I property to
-- 
2.19.1

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

Re: [PATCH] drm/amdgpu/vega20: add CLK base offset

[Wentland, Harry]

On 2018-11-06 11:24 a.m., Alex Deucher wrote:
> In case we need to access CLK registers.
> 
> Signed-off-by: Alex Deucher 

Acked-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c 
> b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> index 2d4473557b0d..d13fc4fcb517 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
> @@ -49,6 +49,7 @@ int vega20_reg_base_init(struct amdgpu_device *adev)
>   adev->reg_offset[SMUIO_HWIP][i] = (uint32_t 
> *)(&(SMUIO_BASE.instance[i]));
>   adev->reg_offset[NBIF_HWIP][i] = (uint32_t 
> *)(&(NBIO_BASE.instance[i]));
>   adev->reg_offset[THM_HWIP][i] = (uint32_t 
> *)(&(THM_BASE.instance[i]));
> + adev->reg_offset[CLK_HWIP][i] = (uint32_t 
> *)(&(CLK_BASE.instance[i]));
>   }
>   return 0;
>  }
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd: Don't fail on backlight = 0

[Wentland, Harry]

On 2018-11-06 11:14 a.m., David Francis wrote:
> Amgpu's backlight update status function was
> returning 1 (an error value) when the backlight
> property was 0.  This breaks users that assume
> 0 is a valid backlight value (which is a
> correct assumption)
> 
> If the user passes in a backlight value of 0,
> tell them everything is fine, then write a value of
> 1 to hardware.
> 
> Signed-off-by: David Francis 

Reviewed-by: Harry Wentland 

Harry

> Bugzilla: https://bugs.freedesktop.org/108668
> Fixes: 416615ea9578 ("drm/amd/display: set backlight level limit to 1")
> Cc: suresh.gutt...@amd.com
> Cc: harry.wentl...@amd.com
> Cc: samant...@posteo.net
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> 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 f8ec8a146663..131e6483d442 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1567,21 +1567,23 @@ static int amdgpu_dm_backlight_update_status(struct 
> backlight_device *bd)
>  {
>   struct amdgpu_display_manager *dm = bl_get_data(bd);
>  
> + /* backlight_pwm_u16_16 parameter is in unsigned 32 bit, 16 bit integer
> +  * and 16 bit fractional, where 1.0 is max backlight value.
> +  * bd->props.brightness is 8 bit format and needs to be converted by
> +  * scaling via copy lower byte to upper byte of 16 bit value.
> +  */
> + uint32_t brightness = bd->props.brightness * 0x101;
> +
>   /*
>* PWM interperts 0 as 100% rather than 0% because of HW
> -  * limitation for level 0.So limiting minimum brightness level
> +  * limitation for level 0.  So limiting minimum brightness level
>* to 1.
>*/
>   if (bd->props.brightness < 1)
> - return 1;
> + brightness = 0x101;
>  
> - /* backlight_pwm_u16_16 parameter is in unsigned 32 bit, 16 bit integer
> -  * and 16 bit fractional, where 1.0 is max backlight value.
> -  * bd->props.brightness is 8 bit format and needs to be converted by
> -  * scaling via copy lower byte to upper byte of 16 bit value.
> -  */
>   if (dc_link_set_backlight_level(dm->backlight_link,
> - (bd->props.brightness * 0x101), 0, 0))
> + brightness, 0, 0))
>   return 0;
>   else
>   return 1;
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/vega20: add CLK base offset

[Alex Deucher]

In case we need to access CLK registers.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c 
b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
index 2d4473557b0d..d13fc4fcb517 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
@@ -49,6 +49,7 @@ int vega20_reg_base_init(struct amdgpu_device *adev)
adev->reg_offset[SMUIO_HWIP][i] = (uint32_t 
*)(&(SMUIO_BASE.instance[i]));
adev->reg_offset[NBIF_HWIP][i] = (uint32_t 
*)(&(NBIO_BASE.instance[i]));
adev->reg_offset[THM_HWIP][i] = (uint32_t 
*)(&(THM_BASE.instance[i]));
+   adev->reg_offset[CLK_HWIP][i] = (uint32_t 
*)(&(CLK_BASE.instance[i]));
}
return 0;
 }
-- 
2.13.6

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

Re: [PATCH] drm/amd: Don't fail on backlight = 0

[Deucher, Alexander]

Acked-by: Alex Deucher 


From: amd-gfx  on behalf of David 
Francis 
Sent: Tuesday, November 6, 2018 11:14:46 AM
To: amd-gfx@lists.freedesktop.org
Cc: Francis, David; samant...@posteo.net; Wentland, Harry; Guttula, Suresh
Subject: [PATCH] drm/amd: Don't fail on backlight = 0

Amgpu's backlight update status function was
returning 1 (an error value) when the backlight
property was 0.  This breaks users that assume
0 is a valid backlight value (which is a
correct assumption)

If the user passes in a backlight value of 0,
tell them everything is fine, then write a value of
1 to hardware.

Signed-off-by: David Francis 
Bugzilla: https://bugs.freedesktop.org/108668
Fixes: 416615ea9578 ("drm/amd/display: set backlight level limit to 1")
Cc: suresh.gutt...@amd.com
Cc: harry.wentl...@amd.com
Cc: samant...@posteo.net
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

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 f8ec8a146663..131e6483d442 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1567,21 +1567,23 @@ static int amdgpu_dm_backlight_update_status(struct 
backlight_device *bd)
 {
 struct amdgpu_display_manager *dm = bl_get_data(bd);

+   /* backlight_pwm_u16_16 parameter is in unsigned 32 bit, 16 bit integer
+* and 16 bit fractional, where 1.0 is max backlight value.
+* bd->props.brightness is 8 bit format and needs to be converted by
+* scaling via copy lower byte to upper byte of 16 bit value.
+*/
+   uint32_t brightness = bd->props.brightness * 0x101;
+
 /*
  * PWM interperts 0 as 100% rather than 0% because of HW
-* limitation for level 0.So limiting minimum brightness level
+* limitation for level 0.  So limiting minimum brightness level
  * to 1.
  */
 if (bd->props.brightness < 1)
-   return 1;
+   brightness = 0x101;

-   /* backlight_pwm_u16_16 parameter is in unsigned 32 bit, 16 bit integer
-* and 16 bit fractional, where 1.0 is max backlight value.
-* bd->props.brightness is 8 bit format and needs to be converted by
-* scaling via copy lower byte to upper byte of 16 bit value.
-*/
 if (dc_link_set_backlight_level(dm->backlight_link,
-   (bd->props.brightness * 0x101), 0, 0))
+   brightness, 0, 0))
 return 0;
 else
 return 1;
--
2.17.1

___
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] drm/amd: Don't fail on backlight = 0

[David Francis]

Amgpu's backlight update status function was
returning 1 (an error value) when the backlight
property was 0.  This breaks users that assume
0 is a valid backlight value (which is a
correct assumption)

If the user passes in a backlight value of 0,
tell them everything is fine, then write a value of
1 to hardware.

Signed-off-by: David Francis 
Bugzilla: https://bugs.freedesktop.org/108668
Fixes: 416615ea9578 ("drm/amd/display: set backlight level limit to 1")
Cc: suresh.gutt...@amd.com
Cc: harry.wentl...@amd.com
Cc: samant...@posteo.net
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

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 f8ec8a146663..131e6483d442 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1567,21 +1567,23 @@ static int amdgpu_dm_backlight_update_status(struct 
backlight_device *bd)
 {
struct amdgpu_display_manager *dm = bl_get_data(bd);
 
+   /* backlight_pwm_u16_16 parameter is in unsigned 32 bit, 16 bit integer
+* and 16 bit fractional, where 1.0 is max backlight value.
+* bd->props.brightness is 8 bit format and needs to be converted by
+* scaling via copy lower byte to upper byte of 16 bit value.
+*/
+   uint32_t brightness = bd->props.brightness * 0x101;
+
/*
 * PWM interperts 0 as 100% rather than 0% because of HW
-* limitation for level 0.So limiting minimum brightness level
+* limitation for level 0.  So limiting minimum brightness level
 * to 1.
 */
if (bd->props.brightness < 1)
-   return 1;
+   brightness = 0x101;
 
-   /* backlight_pwm_u16_16 parameter is in unsigned 32 bit, 16 bit integer
-* and 16 bit fractional, where 1.0 is max backlight value.
-* bd->props.brightness is 8 bit format and needs to be converted by
-* scaling via copy lower byte to upper byte of 16 bit value.
-*/
if (dc_link_set_backlight_level(dm->backlight_link,
-   (bd->props.brightness * 0x101), 0, 0))
+   brightness, 0, 0))
return 0;
else
return 1;
-- 
2.17.1

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

Re: [PATCH] drm/ttm: Fix bo_global and mem_global kfree error

[Christian König]

Am 06.11.18 um 12:54 schrieb Trigger Huang:

ttm_bo_glob and ttm_mem_glob are defined as structure instance, while
not allocated by kzalloc, so kfree should not be invoked to release
them anymore. Otherwise, it will cause the following kernel BUG when
unloading amdgpu module

[   48.419294] kernel BUG at /build/linux-5s7Xkn/linux-4.15.0/mm/slub.c:3894!
[   48.419352] invalid opcode:  [#1] SMP PTI
[   48.419387] Modules linked in: amdgpu(OE-) amdchash(OE) amdttm(OE) 
amd_sched(OE) amdkcl(OE) amd_iommu_v2 drm_kms_helper drm i2c_algo_bit 
fb_sys_fops syscopyarea sysfillrect sysimgblt snd_hda_codec_generic 
snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep kvm_intel kvm irqbypass 
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_pcm snd_seq_midi 
snd_seq_midi_event snd_rawmidi pcbc snd_seq snd_seq_device snd_timer 
aesni_intel snd soundcore joydev aes_x86_64 crypto_simd glue_helper cryptd 
input_leds mac_hid serio_raw binfmt_misc nfsd auth_rpcgss nfs_acl lockd grace 
sunrpc sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 
8139too psmouse i2c_piix4 8139cp mii floppy pata_acpi
[   48.419782] CPU: 1 PID: 1281 Comm: modprobe Tainted: G   OE
4.15.0-20-generic #21-Ubuntu
[   48.419838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
Ubuntu-1.8.2-1ubuntu1 04/01/2014
[   48.419901] RIP: 0010:kfree+0x137/0x180
[   48.419934] RSP: 0018:b02101273bf8 EFLAGS: 00010246
[   48.419974] RAX: eee1418ad7e0 RBX: c075f100 RCX: 8fed7fca7ed0
[   48.420025] RDX:  RSI: 0003440e RDI: 2240
[   48.420073] RBP: b02101273c10 R08: 0010 R09: 8fed7ffd3680
[   48.420121] R10: eee1418ad7c0 R11: 8fed7ffd3000 R12: c075e2c0
[   48.420169] R13: c074ec10 R14: 8fed73063900 R15: 8fed737428e8
[   48.420216] FS:  7fdc912ec540() GS:8fed7fc8() 
knlGS:
[   48.420267] CS:  0010 DS:  ES:  CR0: 80050033
[   48.420308] CR2: 55fa40c30060 CR3: 00023470a006 CR4: 003606e0
[   48.420358] DR0:  DR1:  DR2: 
[   48.420405] DR3:  DR6: fffe0ff0 DR7: 0400
[   48.420452] Call Trace:
[   48.420485]  ttm_bo_global_kobj_release+0x20/0x30 [amdttm]
[   48.420528]  kobject_release+0x6a/0x180
[   48.420562]  kobject_put+0x28/0x50
[   48.420595]  ttm_bo_global_release+0x36/0x50 [amdttm]
[   48.420636]  amdttm_bo_device_release+0x119/0x180 [amdttm]
[   48.420678]  ? amdttm_bo_clean_mm+0xa6/0xf0 [amdttm]
[   48.420760]  amdgpu_ttm_fini+0xc9/0x180 [amdgpu]
[   48.420821]  amdgpu_bo_fini+0x12/0x40 [amdgpu]
[   48.420889]  gmc_v9_0_sw_fini+0x40/0x50 [amdgpu]
[   48.420947]  amdgpu_device_fini+0x36f/0x4c0 [amdgpu]
[   48.421007]  amdgpu_driver_unload_kms+0xb4/0x150 [amdgpu]
[   48.421058]  drm_dev_unregister+0x46/0xf0 [drm]
[   48.421102]  drm_dev_unplug+0x12/0x70 [drm]

Signed-off-by: Trigger Huang 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/ttm/ttm_bo.c | 1 -
  drivers/gpu/drm/ttm/ttm_memory.c | 9 -
  2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index df02880..01c6d14 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1527,7 +1527,6 @@ static void ttm_bo_global_kobj_release(struct kobject 
*kobj)
container_of(kobj, struct ttm_bo_global, kobj);
  
  	__free_page(glob->dummy_read_page);

-   kfree(glob);
  }
  
  static void ttm_bo_global_release(void)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 7704e17..f1567c3 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -219,14 +219,6 @@ static ssize_t ttm_mem_global_store(struct kobject *kobj,
return size;
  }
  
-static void ttm_mem_global_kobj_release(struct kobject *kobj)

-{
-   struct ttm_mem_global *glob =
-   container_of(kobj, struct ttm_mem_global, kobj);
-
-   kfree(glob);
-}
-
  static struct attribute *ttm_mem_global_attrs[] = {
&ttm_mem_global_lower_mem_limit,
NULL
@@ -238,7 +230,6 @@ static const struct sysfs_ops ttm_mem_global_ops = {
  };
  
  static struct kobj_type ttm_mem_glob_kobj_type = {

-   .release = &ttm_mem_global_kobj_release,
.sysfs_ops = &ttm_mem_global_ops,
.default_attrs = ttm_mem_global_attrs,
  };


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

[PATCH] drm/ttm: Fix bo_global and mem_global kfree error

[Trigger Huang]

ttm_bo_glob and ttm_mem_glob are defined as structure instance, while
not allocated by kzalloc, so kfree should not be invoked to release
them anymore. Otherwise, it will cause the following kernel BUG when
unloading amdgpu module

[   48.419294] kernel BUG at /build/linux-5s7Xkn/linux-4.15.0/mm/slub.c:3894!
[   48.419352] invalid opcode:  [#1] SMP PTI
[   48.419387] Modules linked in: amdgpu(OE-) amdchash(OE) amdttm(OE) 
amd_sched(OE) amdkcl(OE) amd_iommu_v2 drm_kms_helper drm i2c_algo_bit 
fb_sys_fops syscopyarea sysfillrect sysimgblt snd_hda_codec_generic 
snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep kvm_intel kvm irqbypass 
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_pcm snd_seq_midi 
snd_seq_midi_event snd_rawmidi pcbc snd_seq snd_seq_device snd_timer 
aesni_intel snd soundcore joydev aes_x86_64 crypto_simd glue_helper cryptd 
input_leds mac_hid serio_raw binfmt_misc nfsd auth_rpcgss nfs_acl lockd grace 
sunrpc sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 
8139too psmouse i2c_piix4 8139cp mii floppy pata_acpi
[   48.419782] CPU: 1 PID: 1281 Comm: modprobe Tainted: G   OE
4.15.0-20-generic #21-Ubuntu
[   48.419838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
Ubuntu-1.8.2-1ubuntu1 04/01/2014
[   48.419901] RIP: 0010:kfree+0x137/0x180
[   48.419934] RSP: 0018:b02101273bf8 EFLAGS: 00010246
[   48.419974] RAX: eee1418ad7e0 RBX: c075f100 RCX: 8fed7fca7ed0
[   48.420025] RDX:  RSI: 0003440e RDI: 2240
[   48.420073] RBP: b02101273c10 R08: 0010 R09: 8fed7ffd3680
[   48.420121] R10: eee1418ad7c0 R11: 8fed7ffd3000 R12: c075e2c0
[   48.420169] R13: c074ec10 R14: 8fed73063900 R15: 8fed737428e8
[   48.420216] FS:  7fdc912ec540() GS:8fed7fc8() 
knlGS:
[   48.420267] CS:  0010 DS:  ES:  CR0: 80050033
[   48.420308] CR2: 55fa40c30060 CR3: 00023470a006 CR4: 003606e0
[   48.420358] DR0:  DR1:  DR2: 
[   48.420405] DR3:  DR6: fffe0ff0 DR7: 0400
[   48.420452] Call Trace:
[   48.420485]  ttm_bo_global_kobj_release+0x20/0x30 [amdttm]
[   48.420528]  kobject_release+0x6a/0x180
[   48.420562]  kobject_put+0x28/0x50
[   48.420595]  ttm_bo_global_release+0x36/0x50 [amdttm]
[   48.420636]  amdttm_bo_device_release+0x119/0x180 [amdttm]
[   48.420678]  ? amdttm_bo_clean_mm+0xa6/0xf0 [amdttm]
[   48.420760]  amdgpu_ttm_fini+0xc9/0x180 [amdgpu]
[   48.420821]  amdgpu_bo_fini+0x12/0x40 [amdgpu]
[   48.420889]  gmc_v9_0_sw_fini+0x40/0x50 [amdgpu]
[   48.420947]  amdgpu_device_fini+0x36f/0x4c0 [amdgpu]
[   48.421007]  amdgpu_driver_unload_kms+0xb4/0x150 [amdgpu]
[   48.421058]  drm_dev_unregister+0x46/0xf0 [drm]
[   48.421102]  drm_dev_unplug+0x12/0x70 [drm]

Signed-off-by: Trigger Huang 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 1 -
 drivers/gpu/drm/ttm/ttm_memory.c | 9 -
 2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index df02880..01c6d14 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1527,7 +1527,6 @@ static void ttm_bo_global_kobj_release(struct kobject 
*kobj)
container_of(kobj, struct ttm_bo_global, kobj);
 
__free_page(glob->dummy_read_page);
-   kfree(glob);
 }
 
 static void ttm_bo_global_release(void)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 7704e17..f1567c3 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -219,14 +219,6 @@ static ssize_t ttm_mem_global_store(struct kobject *kobj,
return size;
 }
 
-static void ttm_mem_global_kobj_release(struct kobject *kobj)
-{
-   struct ttm_mem_global *glob =
-   container_of(kobj, struct ttm_mem_global, kobj);
-
-   kfree(glob);
-}
-
 static struct attribute *ttm_mem_global_attrs[] = {
&ttm_mem_global_lower_mem_limit,
NULL
@@ -238,7 +230,6 @@ static const struct sysfs_ops ttm_mem_global_ops = {
 };
 
 static struct kobj_type ttm_mem_glob_kobj_type = {
-   .release = &ttm_mem_global_kobj_release,
.sysfs_ops = &ttm_mem_global_ops,
.default_attrs = ttm_mem_global_attrs,
 };
-- 
2.7.4

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

Re: [PATCH libdrm 5/5] [libdrm] add syncobj timeline tests

[Daniel Vetter]

On Mon, Nov 05, 2018 at 10:07:24AM +, Zhou, David(ChunMing) wrote:
> 
> 
> > -Original Message-
> > From: Daniel Vetter  On Behalf Of Daniel Vetter
> > Sent: Monday, November 05, 2018 5:39 PM
> > To: Zhou, David(ChunMing) 
> > Cc: dri-de...@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH libdrm 5/5] [libdrm] add syncobj timeline tests
> > 
> > On Fri, Nov 02, 2018 at 04:26:49PM +0800, Chunming Zhou wrote:
> > > Signed-off-by: Chunming Zhou 
> > > ---
> > >  tests/amdgpu/Makefile.am |   3 +-
> > >  tests/amdgpu/amdgpu_test.c   |  12 ++
> > >  tests/amdgpu/amdgpu_test.h   |  21 +++
> > >  tests/amdgpu/meson.build |   2 +-
> > >  tests/amdgpu/syncobj_tests.c | 263
> > > +++
> > >  5 files changed, 299 insertions(+), 2 deletions(-)  create mode
> > > 100644 tests/amdgpu/syncobj_tests.c
> > 
> > This testcase seems very much a happy sunday scenario, no tests at all for
> > corner cases, invalid input, and generally trying to pull the kernel over 
> > the
> > table. I think we need a lot more, and preferrably in igt, where we already
> > have a good baseline of drm_syncobj tests.
> Hi Daniel,
> 
> OK, if you insist on that, I would switch to implement a timeline test on IGT.
> Btw,  timeline syncobj test needs based on command submission, Can I write it 
> with amdgpu driver on IGT?
> And after that, where should I send igt patch to review? 

We have used vgem and sw_fence in the past to be able to have very
controlled timelines. This allows us to write generic testcases. The
current igt tests for syncobj are neither i915 nor amdgpu specific.

You might still want to have an amgpu test on top, we do have some i915
tests for the i915 execbuf integration of syncobj of course. But that's a
fairly minor part of the syncobj api.

For contributing to igt, see 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/CONTRIBUTING.md

> Last, if you are free, Could you also take a look the u/k interface of 
> timeline syncobj?

Looked reasonable, the detailed questions need userspace (and so probably
better answered by someone who has mesa/vk/gl clue). I can help review
corner cases of the uapi, but that's much easier to do once we have
test-cases to exercise them all. See existing igts for an idea of what
exhaustive testing of uapi corner cases looks like.
-Daniel


> 
> 
> Thanks,
> David Zhou
> > -Daniel
> > 
> > >
> > > diff --git a/tests/amdgpu/Makefile.am b/tests/amdgpu/Makefile.am index
> > > 447ff217..d3fbe2bb 100644
> > > --- a/tests/amdgpu/Makefile.am
> > > +++ b/tests/amdgpu/Makefile.am
> > > @@ -33,4 +33,5 @@ amdgpu_test_SOURCES = \
> > >   vcn_tests.c \
> > >   uve_ib.h \
> > >   deadlock_tests.c \
> > > - vm_tests.c
> > > + vm_tests.c \
> > > + syncobj_tests.c
> > > diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
> > > index 96fcd687..cdcb93a5 100644
> > > --- a/tests/amdgpu/amdgpu_test.c
> > > +++ b/tests/amdgpu/amdgpu_test.c
> > > @@ -56,6 +56,7 @@
> > >  #define UVD_ENC_TESTS_STR "UVD ENC Tests"
> > >  #define DEADLOCK_TESTS_STR "Deadlock Tests"
> > >  #define VM_TESTS_STR "VM Tests"
> > > +#define SYNCOBJ_TIMELINE_TESTS_STR "SYNCOBJ TIMELINE Tests"
> > >
> > >  /**
> > >   *  Open handles for amdgpu devices
> > > @@ -116,6 +117,12 @@ static CU_SuiteInfo suites[] = {
> > >   .pCleanupFunc = suite_vm_tests_clean,
> > >   .pTests = vm_tests,
> > >   },
> > > + {
> > > + .pName = SYNCOBJ_TIMELINE_TESTS_STR,
> > > + .pInitFunc = suite_syncobj_timeline_tests_init,
> > > + .pCleanupFunc = suite_syncobj_timeline_tests_clean,
> > > + .pTests = syncobj_timeline_tests,
> > > + },
> > >
> > >   CU_SUITE_INFO_NULL,
> > >  };
> > > @@ -165,6 +172,11 @@ static Suites_Active_Status suites_active_stat[] = {
> > >   .pName = VM_TESTS_STR,
> > >   .pActive = suite_vm_tests_enable,
> > >   },
> > > + {
> > > + .pName = SYNCOBJ_TIMELINE_TESTS_STR,
> > > + .pActive = suite_syncobj_timeline_tests_enable,
> > > + },
> > > +
> > >  };
> > >
> > >
> > > diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
> > > index 0609a74b..946e91c2 100644
> > > --- a/tests/amdgpu/amdgpu_test.h
> > > +++ b/tests/amdgpu/amdgpu_test.h
> > > @@ -194,6 +194,27 @@ CU_BOOL suite_vm_tests_enable(void);
> > >   */
> > >  extern CU_TestInfo vm_tests[];
> > >
> > > +/**
> > > + * Initialize syncobj timeline test suite  */ int
> > > +suite_syncobj_timeline_tests_init();
> > > +
> > > +/**
> > > + * Deinitialize syncobj timeline test suite  */ int
> > > +suite_syncobj_timeline_tests_clean();
> > > +
> > > +/**
> > > + * Decide if the suite is enabled by default or not.
> > > + */
> > > +CU_BOOL suite_syncobj_timeline_tests_enable(void);
> > > +
> > > +/**
> > > + * Tests in syncobj timeline test suite  */ extern CU_TestInfo
> > > +syncobj_timeline_tests[];
> > > +
> > > +
> > >  /**
> > >   * Helper