RE: [enable VCN2.0 for NV12 SRIOV 6/6] drm/amdgpu: clear warning on unused var

[Deng, Emily]

[AMD Official Use Only - Internal Distribution Only]

Series reviewed-by: Emily Deng 

>-Original Message-
>From: amd-gfx  On Behalf Of
>Christian König
>Sent: Thursday, March 5, 2020 9:37 PM
>To: Liu, Monk ; amd-gfx@lists.freedesktop.org
>Subject: Re: [enable VCN2.0 for NV12 SRIOV 6/6] drm/amdgpu: clear warning
>on unused var
>
>Am 05.03.20 um 14:33 schrieb Monk Liu:
>> Signed-off-by: Monk Liu 
>
>Acked-by: Christian König 
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index ae9754f..a41272f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -493,7 +493,6 @@ static int amdgpu_vcn_dec_get_destroy_msg(struct
>amdgpu_ring *ring, uint32_t han
>>
>>   int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>>   {
>> -struct amdgpu_device *adev = ring->adev;
>>  struct dma_fence *fence;
>>  long r;
>>
>> @@ -655,7 +654,6 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct
>amdgpu_ring *ring, uint32_t han
>>
>>   int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>>   {
>> -struct amdgpu_device *adev = ring->adev;
>>  struct dma_fence *fence = NULL;
>>  struct amdgpu_bo *bo = NULL;
>>  long r;
>
>___
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
>eedesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfxdata=02%7C01%7CEmily.Deng%40amd.com%7C65554a78d47c44316
>3ee08d7c10a4c0d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63
>7190122293430826sdata=DeaDmgrUF9yqz1dTnjovIHU6UcjGX4rEy6lcyC
>hHxeA%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [enable VCN2.0 for NV12 SRIOV 6/6] drm/amdgpu: clear warning on unused var

[Liu, Monk]

No, this is an existed warning 

_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Liu, Leo  
Sent: Friday, March 6, 2020 12:17 AM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: [enable VCN2.0 for NV12 SRIOV 6/6] drm/amdgpu: clear warning on 
unused var

Is this warning introduced by your patch 4?

On 2020-03-05 8:33 a.m., Monk Liu wrote:
> Signed-off-by: Monk Liu 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index ae9754f..a41272f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -493,7 +493,6 @@ static int amdgpu_vcn_dec_get_destroy_msg(struct 
> amdgpu_ring *ring, uint32_t han
>   
>   int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   {
> - struct amdgpu_device *adev = ring->adev;
>   struct dma_fence *fence;
>   long r;
>   
> @@ -655,7 +654,6 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct 
> amdgpu_ring *ring, uint32_t han
>   
>   int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)
>   {
> - struct amdgpu_device *adev = ring->adev;
>   struct dma_fence *fence = NULL;
>   struct amdgpu_bo *bo = NULL;
>   long r;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [enable VCN2.0 for NV12 SRIOV 3/6] drm/amdgpu: implement initialization part on VCN2.0 for SRIOV

[Liu, Monk]

Okay, no problem 

_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Liu, Leo  
Sent: Friday, March 6, 2020 12:08 AM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: [enable VCN2.0 for NV12 SRIOV 3/6] drm/amdgpu: implement 
initialization part on VCN2.0 for SRIOV


On 2020-03-05 8:33 a.m., Monk Liu wrote:
> one dec ring and one enc ring
It seems more than that, you might add more messages.


>
> Signed-off-by: Monk Liu 
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 231 
> +-
>   1 file changed, 228 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index c387c81..421e5bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -29,6 +29,7 @@
>   #include "soc15d.h"
>   #include "amdgpu_pm.h"
>   #include "amdgpu_psp.h"
> +#include "mmsch_v2_0.h"
>   
>   #include "vcn/vcn_2_0_0_offset.h"
>   #include "vcn/vcn_2_0_0_sh_mask.h"
> @@ -54,7 +55,7 @@ static int vcn_v2_0_set_powergating_state(void *handle,
>   enum amd_powergating_state state);
>   static int vcn_v2_0_pause_dpg_mode(struct amdgpu_device *adev,
>   int inst_idx, struct dpg_pause_state 
> *new_state);
> -
> +static int vcn_v2_0_start_sriov(struct amdgpu_device *adev);

Please keep the empty line here.


>   /**
>* vcn_v2_0_early_init - set function pointers
>*
> @@ -67,7 +68,10 @@ static int vcn_v2_0_early_init(void *handle)
>   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
>   adev->vcn.num_vcn_inst = 1;
> - adev->vcn.num_enc_rings = 2;
> + if (amdgpu_sriov_vf(adev))
> + adev->vcn.num_enc_rings = 1;
> + else
> + adev->vcn.num_enc_rings = 2;
>   
>   vcn_v2_0_set_dec_ring_funcs(adev);
>   vcn_v2_0_set_enc_ring_funcs(adev);
> @@ -154,7 +158,10 @@ static int vcn_v2_0_sw_init(void *handle)
>   for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
>   ring = >vcn.inst->ring_enc[i];
>   ring->use_doorbell = true;
> - ring->doorbell_index = (adev->doorbell_index.vcn.vcn_ring0_1 << 
> 1) + 2 + i;
> + if (!amdgpu_sriov_vf(adev))
> + ring->doorbell_index = 
> (adev->doorbell_index.vcn.vcn_ring0_1 << 1) + 2 + i;
> + else
> + ring->doorbell_index = 
> (adev->doorbell_index.vcn.vcn_ring0_1 << 1) 
> ++ 1 + i;
>   sprintf(ring->name, "vcn_enc%d", i);
>   r = amdgpu_ring_init(adev, ring, 512, >vcn.inst->irq, 0);
>   if (r)
> @@ -163,6 +170,10 @@ static int vcn_v2_0_sw_init(void *handle)
>   
>   adev->vcn.pause_dpg_mode = vcn_v2_0_pause_dpg_mode;
>   
> + r = amdgpu_virt_alloc_mm_table(adev);
> + if (r)
> + return r;
> +

This is not needed for bare metal.


>   return 0;
>   }
>   
> @@ -178,6 +189,8 @@ static int vcn_v2_0_sw_fini(void *handle)
>   int r;
>   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
> + amdgpu_virt_free_mm_table(adev);
> +

same as above here.


Regards,

Leo



>   r = amdgpu_vcn_suspend(adev);
>   if (r)
>   return r;
> @@ -203,6 +216,9 @@ static int vcn_v2_0_hw_init(void *handle)
>   adev->nbio.funcs->vcn_doorbell_range(adev, ring->use_doorbell,
>ring->doorbell_index, 0);
>   
> + if (amdgpu_sriov_vf(adev))
> + vcn_v2_0_start_sriov(adev);
> +
>   r = amdgpu_ring_test_helper(ring);
>   if (r)
>   goto done;
> @@ -1680,6 +1696,215 @@ static int vcn_v2_0_set_powergating_state(void 
> *handle,
>   return ret;
>   }
>   
> +static int vcn_v2_0_start_mmsch(struct amdgpu_device *adev,
> + struct amdgpu_mm_table *table)
> +{
> + uint32_t data = 0, loop;
> + uint64_t addr = table->gpu_addr;
> + struct mmsch_v2_0_init_header *header;
> + uint32_t size;
> + int i;
> +
> + header = (struct mmsch_v2_0_init_header *)table->cpu_addr;
> + size = header->header_size + header->vcn_table_size;
> +
> + /* 1, write to vce_mmsch_vf_ctx_addr_lo/hi register with GPU mc addr
> +  * of memory descriptor location
> +  */
> + WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_ADDR_LO, lower_32_bits(addr));
> + WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_ADDR_HI, upper_32_bits(addr));
> +
> + /* 2, update vmid of descriptor */
> + data = RREG32_SOC15(UVD, 0, mmMMSCH_VF_VMID);
> + data &= ~MMSCH_VF_VMID__VF_CTX_VMID_MASK;
> + /* use domain0 for MM scheduler */
> + data |= (0 << MMSCH_VF_VMID__VF_CTX_VMID__SHIFT);
> + WREG32_SOC15(UVD, 0, mmMMSCH_VF_VMID, data);
> +
> + /* 3, notify mmsch about the size of this descriptor */
> + WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_SIZE, size);
> +
> + /* 4, set resp to zero */
> + 

RE: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

[Xu, Feifei]

[AMD Public Use]

Patch3: Acked-by: Feifei Xu 

Regards,
Feifei

From: amd-gfx  On Behalf Of Xu, Feifei
Sent: 2020年3月6日 9:54
To: Deucher, Alexander ; Alex Deucher 
; amd-gfx list 
Cc: Tawfik, Aly 
Subject: RE: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching


[AMD Public Use]

Patch1, Patch2 are :


Reviewed-by: Feifei Xu mailto:feifei...@amd.com>>


From: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>
Sent: 2020年3月6日 0:00
To: Xu, Feifei mailto:feifei...@amd.com>>; Alex Deucher 
mailto:alexdeuc...@gmail.com>>; amd-gfx list 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Tawfik, Aly mailto:aly.taw...@amd.com>>
Subject: Re: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching


[AMD Public Use]

Is this for the series, or just this patch?  Thanks!

Alex

From: Xu, Feifei mailto:feifei...@amd.com>>
Sent: Thursday, March 5, 2020 12:24 AM
To: Alex Deucher mailto:alexdeuc...@gmail.com>>; amd-gfx 
list mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; Tawfik, Aly 
mailto:aly.taw...@amd.com>>
Subject: RE: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

[AMD Official Use Only - Internal Distribution Only]



Reviewed-by: Feifei Xu mailto:feifei...@amd.com>>

-Original Message-
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Alex Deucher
Sent: 2020年3月5日 13:23
To: amd-gfx list 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; Tawfik, Aly 
mailto:aly.taw...@amd.com>>
Subject: Re: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

Ping?

On Tue, Feb 25, 2020 at 3:32 PM Alex Deucher 
mailto:alexdeuc...@gmail.com>> wrote:
>
> From: Aly-Tawfik mailto:altaw...@amd.com>>
>
> Use the pci revision id rather than the asic silicon revision id.
>
> Signed-off-by: Aly-Tawfik mailto:altaw...@amd.com>>
> Signed-off-by: Alex Deucher 
> mailto:alexander.deuc...@amd.com>>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 0c55d44c9d5b..92166150bf9f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -902,7 +902,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>
> init_data.asic_id.chip_family = adev->family;
>
> -   init_data.asic_id.pci_revision_id = adev->rev_id;
> +   init_data.asic_id.pci_revision_id = adev->pdev->revision;
> init_data.asic_id.hw_internal_rev = adev->external_rev_id;
>
> init_data.asic_id.vram_width = adev->gmc.vram_width;
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CFeifei.Xu%40amd.com%7Cdd49232261ac4267a33f08d7c0c54ad8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637189825908683804sdata=sN%2BGSvrmnz5Qdrf4JR0sFUDWMWyu7tsOh6ZG9lb9I5Y%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

[Xu, Feifei]

[AMD Public Use]

Patch1, Patch2 are :


Reviewed-by: Feifei Xu 


From: Deucher, Alexander 
Sent: 2020年3月6日 0:00
To: Xu, Feifei ; Alex Deucher ; 
amd-gfx list 
Cc: Tawfik, Aly 
Subject: Re: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching


[AMD Public Use]

Is this for the series, or just this patch?  Thanks!

Alex

From: Xu, Feifei mailto:feifei...@amd.com>>
Sent: Thursday, March 5, 2020 12:24 AM
To: Alex Deucher mailto:alexdeuc...@gmail.com>>; amd-gfx 
list mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; Tawfik, Aly 
mailto:aly.taw...@amd.com>>
Subject: RE: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

[AMD Official Use Only - Internal Distribution Only]



Reviewed-by: Feifei Xu mailto:feifei...@amd.com>>

-Original Message-
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Alex Deucher
Sent: 2020年3月5日 13:23
To: amd-gfx list 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; Tawfik, Aly 
mailto:aly.taw...@amd.com>>
Subject: Re: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

Ping?

On Tue, Feb 25, 2020 at 3:32 PM Alex Deucher 
mailto:alexdeuc...@gmail.com>> wrote:
>
> From: Aly-Tawfik mailto:altaw...@amd.com>>
>
> Use the pci revision id rather than the asic silicon revision id.
>
> Signed-off-by: Aly-Tawfik mailto:altaw...@amd.com>>
> Signed-off-by: Alex Deucher 
> mailto:alexander.deuc...@amd.com>>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 0c55d44c9d5b..92166150bf9f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -902,7 +902,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>
> init_data.asic_id.chip_family = adev->family;
>
> -   init_data.asic_id.pci_revision_id = adev->rev_id;
> +   init_data.asic_id.pci_revision_id = adev->pdev->revision;
> init_data.asic_id.hw_internal_rev = adev->external_rev_id;
>
> init_data.asic_id.vram_width = adev->gmc.vram_width;
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CFeifei.Xu%40amd.com%7Cdd49232261ac4267a33f08d7c0c54ad8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637189825908683804sdata=sN%2BGSvrmnz5Qdrf4JR0sFUDWMWyu7tsOh6ZG9lb9I5Y%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v6 1/1] drm/amdgpu: set compute queue priority at mqd_init

[Luben Tuikov]

On 2020-03-05 16:13, Nirmoy wrote:
> 
> On 3/5/20 7:42 PM, Luben Tuikov wrote:
>>
>>> A quick search leads me amdgpu_sched_ioctl() which is using 
>>> DRM_SCHED_PRIORITY_INVALID
>>>
>>> to indicate a invalid value from userspace. I don't know much about drm 
>>> api to suggest any useful
>>>
>>> changes regarding this. But again this isn't in the scope of this patch 
>>> series.
>> I'm not asking you to eliminate "DRM_SCHED_PRIORITY_INVALID".
>> Oh wait! You forgot what I suggested in a previous review on
>> how to fix enum drm_sched_priority, did you? :-D Search
>> for that email.
> 
> Let me quote[1]:
> 
> "
> 
> Also consider changing to this:
> 
> 
> enum drm_sched_priority {
>     DRM_SCHED_PRIORITY_UNSET,
> DRM_SCHED_PRIORITY_INVALID,<--- remove
>     DRM_SCHED_PRIORITY_MIN,
>     DRM_SCHED_PRIORITY_LOW = DRM_SCHED_PRIORITY_MIN,
>     DRM_SCHED_PRIORITY_NORMAL,
>     DRM_SCHED_PRIORITY_HIGH_SW,
>     DRM_SCHED_PRIORITY_HIGH_HW,
>     DRM_SCHED_PRIORITY_KERNEL,
>     DRM_SCHED_PRIORITY_MAX,
> };
> 
> We should never have an "invalid priority", just ignored priority. :-)
> Second, having 0 as UNSET gives you easy priority when you set
> map[0] = normal_priority, as memory usually comes memset to 0.
> IOW, you'd not need to check against UNSET, and simply use
> the default [0] which you'd set to normal priority.
> 
> "
> 

Excellent! You found the email.

Yes, there is no reason to represent and carry around
an "invalid" priority--it simply means that the *input*
was invalid, but the priority would always be set to
something valid and meaningful: the input was invalid,
not the priority.

If you encounter an invalid input, simply set the priority
to unset, or even better, minimum. From then on,
any further map look-ups, would be a trivial linear
transformation.

It is generally a good idea to represent 0, default software
memory initialization, to "*_NONE", in enums. Then start the
meaningful labels at 1, so one can distinguish between
"this data has never been touched by software" and "this was
set to something by someone after memory initialization".
So you should rename "_UNSET" to "_NONE". It *was* set (by memset()),
just not meaningfully so.

To look like this:

enum drm_sched_priority {
DRM_SCHED_PRIORITY_NONE,
DRM_SCHED_PRIORITY_LOW,
DRM_SCHED_PRIORITY_NORMAL,
DRM_SCHED_PRIORITY_HIGH,
DRM_SCHED_PRIORITY_HIGHER,
DRM_SCHED_PRIORITY_KERNEL,

DRM_SCHED_PRIORITY_SIZE  /* intentionally no comma */
};

Valid values are "*_NONE" to "*_KERNEL".
Meaningful values are "*_LOW" to "*_KERNEL". (i.e. intentional)
"*_SIZE" you use to allocate storage (array, for instance)
of size which can index all valid priorities.

Then you can do:

enum drm_sched_priority some_map[XYZ_DOMAIN_PRIO_SIZE] = {
[0 ... XYZ_DOMAIN_PRIO_SIZE - 1] = XYZ_DOMAIN_PRIO_DEFAULT,
[A]  = XYZ_DOMAIN_PRIO_NORMAL,
[B]  = XYZ_DOMAIN_PRIO_HIGH,
};

Where 0 <= A, B < XYZ_DOMAIN_PRIO_SIZE.

This also allows you to chain those transformations, similarly.

Regards,
Luben


> I guess understood it wrong.
> 
> 
> [1]https://www.mail-archive.com/amd-gfx@lists.freedesktop.org/msg45621.html 
> 
> 
> 
> Regards,
> 
> Nirmoy
> 

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

Re: [PATCH] drm/amd/display: Fix pageflip event race condition for DCN.

[Mario Kleiner]

On Mon, Mar 2, 2020 at 2:57 PM Kazlauskas, Nicholas
 wrote:
>
> On 2020-03-02 1:17 a.m., Mario Kleiner wrote:
> > Commit '16f17eda8bad ("drm/amd/display: Send vblank and user
> > events at vsartup for DCN")' introduces a new way of pageflip
> > completion handling for DCN, and some trouble.
> >
> > The current implementation introduces a race condition, which
> > can cause pageflip completion events to be sent out one vblank
> > too early, thereby confusing userspace and causing flicker:
> >
> > prepare_flip_isr():
> >
> > 1. Pageflip programming takes the ddev->event_lock.
> > 2. Sets acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED
> > 3. Releases ddev->event_lock.
> >
> > --> Deadline for surface address regs double-buffering passes on
> >  target pipe.
> >
> > 4. dc_commit_updates_for_stream() MMIO programs the new pageflip
> > into hw, but too late for current vblank.
> >
> > => pflip_status == AMDGPU_FLIP_SUBMITTED, but flip won't complete
> > in current vblank due to missing the double-buffering deadline
> > by a tiny bit.
> >
> > 5. VSTARTUP trigger point in vblank is reached, VSTARTUP irq fires,
> > dm_dcn_crtc_high_irq() gets called.
> >
> > 6. Detects pflip_status == AMDGPU_FLIP_SUBMITTED and assumes the
> > pageflip has been completed/will complete in this vblank and
> > sends out pageflip completion event to userspace and resets
> > pflip_status = AMDGPU_FLIP_NONE.
> >
> > => Flip completion event sent out one vblank too early.
> >
> > This behaviour has been observed during my testing with measurement
> > hardware a couple of time.
> >
> > The commit message says that the extra flip event code was added to
> > dm_dcn_crtc_high_irq() to prevent missing to send out pageflip events
> > in case the pflip irq doesn't fire, because the "DCH HUBP" component
> > is clock gated and doesn't fire pflip irqs in that state. Also that
> > this clock gating may happen if no planes are active. This suggests
> > that the problem addressed by that commit can't happen if planes
> > are active.
> >
> > The proposed solution is therefore to only execute the extra pflip
> > completion code iff the count of active planes is zero and otherwise
> > leave pflip completion handling to the pflip irq handler, for a
> > more race-free experience.
> >
> > Note that i don't know if this fixes the problem the original commit
> > tried to address, as i don't know what the test scenario was. It
> > does fix the observed too early pageflip events though and points
> > out the problem introduced.
>
> This looks like a valid race condition that should be addressed.

Indeed! And if possible in any way, before Linux 5.6 is released. For
my use cases, neuroscience/medical research, this is a serious problem
which would make DCN gpu's basically unusable for most work. Problems
affecting flip timing / flip completion / timestamping are always
worst case problems for my users.

>
> Unfortunately this also doesn't fix the problem the original commit was
> trying to address.
>
> HUBP interrupts only trigger when it's not clock gated. But there are
> cases (for example, PSR) where the HUBP can be clock gated but the
> active plane count is greater than zero.
>
> The clock gating switch can typically happens outside of x86 control
> flow so we're not really going to understand in advance whether or not
> we'll be able to receive the pflip IRQ.
>

Oh dear! So how can that happen? Could explain to me in more detail,
how this works? What's the job of HUBP, apart from (not) triggering
pflip interrupts reliably? Is the scenario here that the desktop is
detected idle for a while (how?) and PSR kicks in and HUBP gets clock
gated, but somehow vblank interrupts are still active? I thought panel
self refresh only enables on long idle display, so scanout from the
gpu can be basically disabled while the panel refreshes itself? Is a
programmed pageflip then automatically (no host cpu involvement)
putting the panel out of self refresh and turning scanout on and the
pageflip completion enables HUBP again, but HUBP doesn't trigger the
pflip irq because it somehow missed that due to being clock-gated at
time of flip completion?

I'd really like to understand in more detail how this stuff works on
your recent hw, and also which irqs / events / trigger points are
associated with what actions of the hw. I have the feeling there will
be more "fun" lingering in the future. I also wanted to experiment
more with some VRR ideas, so any details about which hw events happen
when and fire which irq's and which double-buffered registers
double-buffer when are greatly appreciated.

> While we do have plane status/flip pending bits available to check in
> the VSTARTUP IRQ handler, this obviously doesn't work for resolving the
> core of the issue - that we probably don't know whether or not the HUBP
> will be on before sending out the event.
>
> Maybe we can guess based on what features are enabled.>

Ok, that's your domain of knowledge. I 

[PATCH] drm/amd/display: Fix pageflip event race condition for DCN. (v2)

[Mario Kleiner]

Commit '16f17eda8bad ("drm/amd/display: Send vblank and user
events at vsartup for DCN")' introduces a new way of pageflip
completion handling for DCN, and some trouble.

The current implementation introduces a race condition, which
can cause pageflip completion events to be sent out one vblank
too early, thereby confusing userspace and causing flicker:

prepare_flip_isr():

1. Pageflip programming takes the ddev->event_lock.
2. Sets acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED
3. Releases ddev->event_lock.

--> Deadline for surface address regs double-buffering passes on
target pipe.

4. dc_commit_updates_for_stream() MMIO programs the new pageflip
   into hw, but too late for current vblank.

=> pflip_status == AMDGPU_FLIP_SUBMITTED, but flip won't complete
   in current vblank due to missing the double-buffering deadline
   by a tiny bit.

5. VSTARTUP trigger point in vblank is reached, VSTARTUP irq fires,
   dm_dcn_crtc_high_irq() gets called.

6. Detects pflip_status == AMDGPU_FLIP_SUBMITTED and assumes the
   pageflip has been completed/will complete in this vblank and
   sends out pageflip completion event to userspace and resets
   pflip_status = AMDGPU_FLIP_NONE.

=> Flip completion event sent out one vblank too early.

This behaviour has been observed during my testing with measurement
hardware a couple of time.

The commit message says that the extra flip event code was added to
dm_dcn_crtc_high_irq() to prevent missing to send out pageflip events
in case the pflip irq doesn't fire, because the "DCH HUBP" component
is clock gated and doesn't fire pflip irqs in that state. Also that
this clock gating may happen if no planes are active. According to
Nicholas, the clock gating can also happen if psr is active, and the
gating is controlled independently by the hardware, so difficult to
detect if and when the completion code in above commit is needed.

This patch tries the following solution: It only executes the extra pflip
completion code in dm_dcn_crtc_high_irq() iff the hardware reports
that there aren't any surface updated pending in the double-buffered
surface scanout address registers. Otherwise it leaves pflip completion
to the pflip irq handler, for a more race-free experience.

This would only guard against the order of events mentioned above.
If Step 5 (VSTARTUP trigger) happens before step 4 then this won't help
at all, because 1-3 + 5 might happen even without the hw being programmed
at all, ie. no surface update pending because none yet programmed into hw.

Therefore this patch also changes locking in amdgpu_dm_commit_planes(),
so that prepare_flip_isr() and dc_commit_updates_for_stream() are done
under event_lock protection within the same critical section.

v2: Take Nicholas comments into account, try a different solution.

Lightly tested on Polaris (locking) and Raven (the whole DCN stuff).
Seems to work without causing obvious new trouble.

Fixes: 16f17eda8bad ("drm/amd/display: Send vblank and user events at vsartup 
for DCN")
Signed-off-by: Mario Kleiner 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 ---
 1 file changed, 67 insertions(+), 13 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 d7df1a85e72f..aa4e941b276f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -287,6 +287,28 @@ static inline bool amdgpu_dm_vrr_active(struct 
dm_crtc_state *dm_state)
   dm_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED;
 }
 
+/**
+ * dm_crtc_is_flip_pending() - Is a pageflip pending on this crtc?
+ *
+ * Returns true if any plane on the crtc has a flip pending, false otherwise.
+ */
+static bool dm_crtc_is_flip_pending(struct dm_crtc_state *acrtc_state)
+{
+   struct dc_stream_status *status = 
dc_stream_get_status(acrtc_state->stream);
+   const struct dc_plane_status *plane_status;
+   int i;
+   bool pending = false;
+
+   for (i = 0; i < status->plane_count; i++) {
+   plane_status = dc_plane_get_status(status->plane_states[i]);
+   pending |= plane_status->is_flip_pending;
+   DRM_DEBUG_DRIVER("plane:%d, flip_pending=%d\n",
+i, plane_status->is_flip_pending);
+   }
+
+   return pending;
+}
+
 /**
  * dm_pflip_high_irq() - Handle pageflip interrupt
  * @interrupt_params: ignored
@@ -435,6 +457,11 @@ static void dm_vupdate_high_irq(void *interrupt_params)
spin_unlock_irqrestore(>ddev->event_lock, 
flags);
}
}
+
+   if (acrtc->pflip_status == AMDGPU_FLIP_SUBMITTED) {
+   DRM_DEBUG_DRIVER("%s:crtc:%d, flip_pending=%d\n", 
__func__,
+   acrtc->crtc_id, 
dm_crtc_is_flip_pending(acrtc_state));
+   }
}
 }
 
@@ -489,6 +516,11 @@ static 

Re: [PATCH v6 1/1] drm/amdgpu: set compute queue priority at mqd_init

[Nirmoy]

On 3/5/20 7:42 PM, Luben Tuikov wrote:



A quick search leads me amdgpu_sched_ioctl() which is using
DRM_SCHED_PRIORITY_INVALID

to indicate a invalid value from userspace. I don't know much about drm
api to suggest any useful

changes regarding this. But again this isn't in the scope of this patch
series.

I'm not asking you to eliminate "DRM_SCHED_PRIORITY_INVALID".
Oh wait! You forgot what I suggested in a previous review on
how to fix enum drm_sched_priority, did you? :-D Search
for that email.


Let me quote[1]:

"

Also consider changing to this:


enum drm_sched_priority {
    DRM_SCHED_PRIORITY_UNSET,
DRM_SCHED_PRIORITY_INVALID,<--- remove
    DRM_SCHED_PRIORITY_MIN,
    DRM_SCHED_PRIORITY_LOW = DRM_SCHED_PRIORITY_MIN,
    DRM_SCHED_PRIORITY_NORMAL,
    DRM_SCHED_PRIORITY_HIGH_SW,
    DRM_SCHED_PRIORITY_HIGH_HW,
    DRM_SCHED_PRIORITY_KERNEL,
    DRM_SCHED_PRIORITY_MAX,
};

We should never have an "invalid priority", just ignored priority. :-)
Second, having 0 as UNSET gives you easy priority when you set
map[0] = normal_priority, as memory usually comes memset to 0.
IOW, you'd not need to check against UNSET, and simply use
the default [0] which you'd set to normal priority.

"

I guess understood it wrong.


[1]https://www.mail-archive.com/amd-gfx@lists.freedesktop.org/msg45621.html 




Regards,

Nirmoy

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

Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

[Zhao, Yong]

[AMD Official Use Only - Internal Distribution Only]

Okay, I will change back to its original format.

Yong


From: Kuehling, Felix 
Sent: Thursday, March 5, 2020 3:49 PM
To: amd-gfx@lists.freedesktop.org ; Zhao, Yong 

Subject: Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

On 2020-03-04 3:21 p.m., Yong Zhao wrote:
> ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
> but they are interweavedly used in kernel driver, resulting in bad
> readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is totally
> not referenced in kernel, and it functions in the kernel through
> ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.
>
> Replace all occurrences of ALLOC_MEM_FLAGS_* by
> KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.
>
> Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
> Signed-off-by: Yong Zhao 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  9 +++--
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 38 +++
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 ---
>   .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
>   4 files changed, 36 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 726c91ab6761..affaa0d4b636 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -29,6 +29,7 @@
>   #include 
>   #include 
>   #include "amdgpu_xgmi.h"
> +#include 
>
>   static const unsigned int compute_vmid_bitmap = 0xFF00;
>
> @@ -500,11 +501,13 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, 
> int dma_buf_fd,
>r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
>   metadata_size, _flags);
>if (flags) {
> - *flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
> - ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
> + if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)
> + *flags = KFD_IOC_ALLOC_MEM_FLAGS_VRAM;
> + else
> + *flags = KFD_IOC_ALLOC_MEM_FLAGS_GTT;

You're sneaking in some personal preference (changing the trinary (cond
? a : b) operator to if-else) with the renaming change. Personally I
find the trinary operator just as readable and more concise.


>
>if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
> - *flags |= ALLOC_MEM_FLAGS_PUBLIC;
> + *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
>}
>
>   out_put:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index e4481caed648..c81fe7011e88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -29,6 +29,7 @@
>   #include "amdgpu_vm.h"
>   #include "amdgpu_amdkfd.h"
>   #include "amdgpu_dma_buf.h"
> +#include 
>
>   /* BO flag to indicate a KFD userptr BO */
>   #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
> @@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
> amdgpu_sync *sync)
>   static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem 
> *mem)
>   {
>struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
> - bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
> + bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
>uint32_t mapping_flags;
>
>mapping_flags = AMDGPU_VM_PAGE_READABLE;
> - if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
> + if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
>mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
> - if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
> + if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
>mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
>
>switch (adev->asic_type) {
>case CHIP_ARCTURUS:
> - if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
> + if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
>if (bo_adev == adev)
>mapping_flags |= coherent ?
>AMDGPU_VM_MTYPE_CC : 
> AMDGPU_VM_MTYPE_RW;
> @@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>/*
> * Check on which domain to allocate BO
> */
> - if (flags & ALLOC_MEM_FLAGS_VRAM) {
> + if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
>domain = alloc_domain = AMDGPU_GEM_DOMAIN_VRAM;
>alloc_flags = AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
> - alloc_flags |= (flags & ALLOC_MEM_FLAGS_PUBLIC) ?
> + alloc_flags |= (flags & KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC) ?
>

Re: [PATCH] drm/amdkfd: Consolidate duplicated bo alloc flags

[Felix Kuehling]

On 2020-03-04 3:21 p.m., Yong Zhao wrote:

ALLOC_MEM_FLAGS_* used are the same as the KFD_IOC_ALLOC_MEM_FLAGS_*,
but they are interweavedly used in kernel driver, resulting in bad
readability. For example, KFD_IOC_ALLOC_MEM_FLAGS_COHERENT is totally
not referenced in kernel, and it functions in the kernel through
ALLOC_MEM_FLAGS_COHERENT, causing unnecessary confusion.

Replace all occurrences of ALLOC_MEM_FLAGS_* by
KFD_IOC_ALLOC_MEM_FLAGS_* to solve the problem.

Change-Id: Iced6ed3698167296c97b14e7e4569883859d619c
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  9 +++--
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 38 +++
  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 13 ---
  .../gpu/drm/amd/include/kgd_kfd_interface.h   | 21 --
  4 files changed, 36 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 726c91ab6761..affaa0d4b636 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include "amdgpu_xgmi.h"
+#include 
  
  static const unsigned int compute_vmid_bitmap = 0xFF00;
  
@@ -500,11 +501,13 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,

r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
   metadata_size, _flags);
if (flags) {
-   *flags = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-   ALLOC_MEM_FLAGS_VRAM : ALLOC_MEM_FLAGS_GTT;
+   if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)
+   *flags = KFD_IOC_ALLOC_MEM_FLAGS_VRAM;
+   else
+   *flags = KFD_IOC_ALLOC_MEM_FLAGS_GTT;


You're sneaking in some personal preference (changing the trinary (cond 
? a : b) operator to if-else) with the renaming change. Personally I 
find the trinary operator just as readable and more concise.



  
  		if (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)

-   *flags |= ALLOC_MEM_FLAGS_PUBLIC;
+   *flags |= KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC;
}
  
  out_put:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e4481caed648..c81fe7011e88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
  #include "amdgpu_vm.h"
  #include "amdgpu_amdkfd.h"
  #include "amdgpu_dma_buf.h"
+#include 
  
  /* BO flag to indicate a KFD userptr BO */

  #define AMDGPU_AMDKFD_USERPTR_BO (1ULL << 63)
@@ -400,18 +401,18 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
  static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
  {
struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
-   bool coherent = mem->alloc_flags & ALLOC_MEM_FLAGS_COHERENT;
+   bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
uint32_t mapping_flags;
  
  	mapping_flags = AMDGPU_VM_PAGE_READABLE;

-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_WRITABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_EXECUTABLE)
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
  
  	switch (adev->asic_type) {

case CHIP_ARCTURUS:
-   if (mem->alloc_flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
if (bo_adev == adev)
mapping_flags |= coherent ?
AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1160,24 +1161,24 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
/*
 * Check on which domain to allocate BO
 */
-   if (flags & ALLOC_MEM_FLAGS_VRAM) {
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_VRAM;
alloc_flags = AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
-   alloc_flags |= (flags & ALLOC_MEM_FLAGS_PUBLIC) ?
+   alloc_flags |= (flags & KFD_IOC_ALLOC_MEM_FLAGS_PUBLIC) ?
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED :
AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
-   } else if (flags & ALLOC_MEM_FLAGS_GTT) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
domain = alloc_domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_flags = 0;
-   } else if (flags & ALLOC_MEM_FLAGS_USERPTR) {
+   } else if (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
domain = AMDGPU_GEM_DOMAIN_GTT;
   

Re: [PATCH 2/8] drm/radeon: don't use ttm bo->offset

[Luben Tuikov]

On 2020-03-05 14:37, Ho, Kenny wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> 
> I believe bo->tbo.mem.mem_type is of uint32_t type and not an enum, is the 
> index lookup method safe? (i.e., how do you deal with the possibility of 
> having value TTM_PL_PRIV or above or are you suggesting those are not 
> possible for this function.)

Hi Kenny,

Good question. I'm not suggesting that those are not possible
with this function. The driver provides this map.
The format of the map is set by the infrastructure (TTM),
it could be as simple as "u64 *mem_start_map; int map_size;" in an embedding
struture or something more fancy.
However the map array and values themselves are set by the driver when it 
answers
about its capabilities, etc, when it first registers with TTM.

Notice how badly the switch statment below is written. (Inverview question.)
It's missing a "default:" label! (it's why switch-case is powerful).

If you add a "default:" label, you see that the rest of the values
in the map are 0s.

While TTM knows only of TTM_PL_PRIV number of maps (3), the driver
can add more, allocate and initialize the map, and return it back
to TTM, guaranteeing at least TTM_PL_PRIV maps in indices 0, 1, and 2.

I think it is not a good idea to use a compare-branch for a
simple lookup like this. One naturally asks, "why isn't this
a simple map lookup?"

Consider the following thought experiment: we continue to add switch-case
and if-else for all sorts of simple things as we've seen being suggested
in recent patches. In a few years, we'll not be able to understand the code.

Regards,
Luben
P.S. Code obfuscation isn't necessarily a bad thing--it causes a complete
rewrite! :-D

> 
> Kenny
> --
> *From:* Tuikov, Luben 
> *Sent:* Thursday, March 5, 2020 2:19 PM
> *To:* Nirmoy Das ; dri-de...@lists.freedesktop.org 
> 
> *Cc:* Zhou, David(ChunMing) ; thellst...@vmware.com 
> ; airl...@linux.ie ; Ho, Kenny 
> ; brian.we...@intel.com ; 
> maarten.lankho...@linux.intel.com ; 
> amd-gfx@lists.freedesktop.org ; Das, Nirmoy 
> ; linux-graphics-maintai...@vmware.com 
> ; bske...@redhat.com 
> ; dan...@ffwll.ch ; Deucher, Alexander 
> ; s...@poorly.run ; Koenig, 
> Christian ; kra...@redhat.com 
> *Subject:* Re: [PATCH 2/8] drm/radeon: don't use ttm bo->offset
>  
> On 2020-03-05 08:29, Nirmoy Das wrote:
>> Calculate GPU offset in radeon_bo_gpu_offset without depending on
>> bo->offset.
>> 
>> Signed-off-by: Nirmoy Das 
>> Reviewed-and-tested-by: Christian König 
>> ---
>>  drivers/gpu/drm/radeon/radeon.h    |  1 +
>>  drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
>>  drivers/gpu/drm/radeon/radeon_ttm.c    |  4 +---
>>  3 files changed, 17 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/radeon/radeon.h 
>> b/drivers/gpu/drm/radeon/radeon.h
>> index 30e32adc1fc6..b7c3fb2bfb54 100644
>> --- a/drivers/gpu/drm/radeon/radeon.h
>> +++ b/drivers/gpu/drm/radeon/radeon.h
>> @@ -2828,6 +2828,7 @@ extern void radeon_ttm_set_active_vram_size(struct 
>> radeon_device *rdev, u64 size
>>  extern void radeon_program_register_sequence(struct radeon_device *rdev,
>> const u32 *registers,
>> const u32 array_size);
>> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
>> 
>>  /*
>>   * vm
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.h 
>> b/drivers/gpu/drm/radeon/radeon_object.h
>> index d23f2ed4126e..60275b822f79 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.h
>> +++ b/drivers/gpu/drm/radeon/radeon_object.h
>> @@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo 
>> *bo)
>>   */
>>  static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
>>  {
>> - return bo->tbo.offset;
>> + struct radeon_device *rdev;
>> + u64 start = 0;
>> +
>> + rdev = radeon_get_rdev(bo->tbo.bdev);
>> +
>> + switch (bo->tbo.mem.mem_type) {
>> + case TTM_PL_TT:
>> + start = 

Re: [PATCH 2/3] drm/dp_mst: Don't show connectors as connected before probing available PBN

[Lyude Paul]

On Thu, 2020-03-05 at 13:52 -0500, Lyude Paul wrote:
> On Thu, 2020-03-05 at 20:29 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 05, 2020 at 01:13:36PM -0500, Lyude Paul wrote:
> > > On Thu, 2020-03-05 at 15:11 +0200, Ville Syrjälä wrote:
> > > > On Wed, Mar 04, 2020 at 05:36:12PM -0500, Lyude Paul wrote:
> > > > > It's next to impossible for us to do connector probing on topologies
> > > > > without occasionally racing with userspace, since creating a
> > > > > connector
> > > > > itself causes a hotplug event which we have to send before probing
> > > > > the
> > > > > available PBN of a connector. Even if we didn't have this hotplug
> > > > > event
> > > > > sent, there's still always a chance that userspace started probing
> > > > > connectors before we finished probing the topology.
> > > > > 
> > > > > This can be a problem when validating a new MST state since the
> > > > > connector will be shown as connected briefly, but without any
> > > > > available
> > > > > PBN - causing any atomic state which would enable said connector to
> > > > > fail
> > > > > with -ENOSPC. So, let's simply workaround this by telling userspace
> > > > > new
> > > > > MST connectors are disconnected until we've finished probing their
> > > > > PBN.
> > > > > Since we always send a hotplug event at the end of the link address
> > > > > probing process, userspace will still know to reprobe the connector
> > > > > when
> > > > > we're ready.
> > > > > 
> > > > > Signed-off-by: Lyude Paul 
> > > > > Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to
> > > > > MST
> > > > > atomic check")
> > > > > Cc: Mikita Lipski 
> > > > > Cc: Alex Deucher 
> > > > > Cc: Sean Paul 
> > > > > Cc: Hans de Goede 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
> > > > >  1 file changed, 13 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index 207eef08d12c..7b0ff0cff954 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -4033,6 +4033,19 @@ drm_dp_mst_detect_port(struct drm_connector
> > > > > *connector,
> > > > >   ret = connector_status_connected;
> > > > >   break;
> > > > >   }
> > > > > +
> > > > > + /* We don't want to tell userspace the port is actually
> > > > > plugged into
> > > > > +  * anything until we've finished probing it's available_pbn,
> > > > > otherwise
> > > > 
> > > > "its"
> > > > 
> > > > Why is the connector even registered before we've finished the probe?
> > > > 
> > > Oops, I'm not sure how I did this by accident but the explanation I gave
> > > in
> > > the commit message was uh, completely wrong. I must have forgotten that
> > > I
> > > made
> > > sure we didn't expose connectors before probing their PBN back when I
> > > started
> > > my MST cleanup
> > > 
> > > So: despite what I said before it's not actually when new connectors are
> > > created, it's when downstream hotplugs happen which means that the
> > > conenctor's
> > > always going to be there before we probe the available_pbn.
> > 
> > Not sure I understand. You're saying this is going to change for already
> > existing connectors when something else gets plugged in, and either we
> > zero it out during the probe or it always was zero to begin with for
> > whatever reason?
> 
> So: you just made me realize that I'm not actually sure whether there's any
> point to us clearing port->available_pbn here since the available_pbn (at
> least the value that we cache on initial link address probing for bandwidth
> constraint checking) shouldn't actually change on a port just because of a
> hotplug. I bet this is probably causing more problems on it's own as well,
> since reprobing the available_pbn might actually give us a value that
> reflects
> allocations on other ports that are already in place.
> 
> So: I think what I'm going to do instead is make it so that we never clear
> port->available_pbn; mainly to make things less complicated during
> suspend/resume, since we want to make sure there's always some sort of PBN
> value populated even during the middle of reprobing the link address on
> resume. That way we don't have to pretend that it's ever disconnected
> either.
> Will send a respin in a bit.
Wait, nope, I believe I am the fool here - _supposedly_ available bw is
supposed to reflect the smallest link rate that occurs in a patch to a branch
device. I think, me and sean paul are looking at this a bit more closely. I
think I might need to do some more playing around with my hubs to make sure
this value is actually what we think it is because unfortunately the spec is
pretty vague on this from what I can tell :( 

> 
> > > I did just notice
> > > though that we send a hotplug on connection status notifications even
> > > before
> > > we've finished the PBN probe, so I might be able 

Re: [PATCH 2/8] drm/radeon: don't use ttm bo->offset

[Ho, Kenny]

[AMD Official Use Only - Internal Distribution Only]

I believe bo->tbo.mem.mem_type is of uint32_t type and not an enum, is the 
index lookup method safe? (i.e., how do you deal with the possibility of having 
value TTM_PL_PRIV or above or are you suggesting those are not possible for 
this function.)

Kenny

From: Tuikov, Luben 
Sent: Thursday, March 5, 2020 2:19 PM
To: Nirmoy Das ; dri-de...@lists.freedesktop.org 

Cc: Zhou, David(ChunMing) ; thellst...@vmware.com 
; airl...@linux.ie ; Ho, Kenny 
; brian.we...@intel.com ; 
maarten.lankho...@linux.intel.com ; 
amd-gfx@lists.freedesktop.org ; Das, Nirmoy 
; linux-graphics-maintai...@vmware.com 
; bske...@redhat.com 
; dan...@ffwll.ch ; Deucher, Alexander 
; s...@poorly.run ; Koenig, 
Christian ; kra...@redhat.com 
Subject: Re: [PATCH 2/8] drm/radeon: don't use ttm bo->offset

On 2020-03-05 08:29, Nirmoy Das wrote:
> Calculate GPU offset in radeon_bo_gpu_offset without depending on
> bo->offset.
>
> Signed-off-by: Nirmoy Das 
> Reviewed-and-tested-by: Christian König 
> ---
>  drivers/gpu/drm/radeon/radeon.h|  1 +
>  drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
>  drivers/gpu/drm/radeon/radeon_ttm.c|  4 +---
>  3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 30e32adc1fc6..b7c3fb2bfb54 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2828,6 +2828,7 @@ extern void radeon_ttm_set_active_vram_size(struct 
> radeon_device *rdev, u64 size
>  extern void radeon_program_register_sequence(struct radeon_device *rdev,
> const u32 *registers,
> const u32 array_size);
> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
>
>  /*
>   * vm
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h 
> b/drivers/gpu/drm/radeon/radeon_object.h
> index d23f2ed4126e..60275b822f79 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo 
> *bo)
>   */
>  static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
>  {
> - return bo->tbo.offset;
> + struct radeon_device *rdev;
> + u64 start = 0;
> +
> + rdev = radeon_get_rdev(bo->tbo.bdev);
> +
> + switch (bo->tbo.mem.mem_type) {
> + case TTM_PL_TT:
> + start = rdev->mc.gtt_start;
> + break;
> + case TTM_PL_VRAM:
> + start = rdev->mc.vram_start;
> + break;
> + }
> +
> + return (bo->tbo.mem.start << PAGE_SHIFT) + start;
>  }

You're removing a "return bo->tbo.offset" and adding a
switch-case statement. So, then, now instead of an instant
lookup, you're adding branching. You're adding comparison
and branching. Do you think that's better? Faster? Smaller?

I've written before about this for this patch: Why not create a map,
whose index is "mem_type" which references the desired
address? No comparison, no branching. Just an index-dereference
and a value:

return rdev->mc.mem_start_map[bo->tbo.mem.mem_type];

Obviously, you'll have to create "mem_start_map".

That's a NAK from me on this patch using comparison
and branching to return static data lookup value.

Regards,
Luben

>
>  static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
> b/drivers/gpu/drm/radeon/radeon_ttm.c
> index badf1b6d1549..1c8303468e8f 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -56,7 +56,7 @@
>  static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
>  static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
>
> -static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>  {
>struct radeon_mman *mman;
>struct radeon_device *rdev;
> @@ -82,7 +82,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
> uint32_t type,
>break;
>case TTM_PL_TT:
>man->func = _bo_manager_func;
> - man->gpu_offset = rdev->mc.gtt_start;
>man->available_caching = TTM_PL_MASK_CACHING;
>man->default_caching = TTM_PL_FLAG_CACHED;
>man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> @@ -104,7 +103,6 @@ static int radeon_init_mem_type(struct ttm_bo_device 
> *bdev, uint32_t type,
>case TTM_PL_VRAM:
>/* "On-card" video ram */
>man->func = _bo_manager_func;
> - man->gpu_offset = rdev->mc.vram_start;
>man->flags = TTM_MEMTYPE_FLAG_FIXED |
> TTM_MEMTYPE_FLAG_MAPPABLE;
>man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> --
> 

Re: [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3

[Luben Tuikov]

On 2020-03-05 08:29, Nirmoy Das wrote:
> Store ttm bo->offset in struct nouveau_bo instead.
> 
> Signed-off-by: Nirmoy Das 
> ---
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c |  6 +++---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 +++---
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndw.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_abi16.c |  8 
>  drivers/gpu/drm/nouveau/nouveau_bo.c|  8 
>  drivers/gpu/drm/nouveau/nouveau_bo.h|  3 +++
>  drivers/gpu/drm/nouveau/nouveau_chan.c  |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_dmem.c  |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_gem.c   | 10 +-
>  15 files changed, 35 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 1f08de4241e0..d06a93f2b38a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -845,7 +845,7 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
>   fb = nouveau_framebuffer(crtc->primary->fb);
>   }
>  
> - nv_crtc->fb.offset = fb->nvbo->bo.offset;
> + nv_crtc->fb.offset = fb->nvbo->offset;
>  
>   if (nv_crtc->lut.depth != drm_fb->format->depth) {
>   nv_crtc->lut.depth = drm_fb->format->depth;
> @@ -1013,7 +1013,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct 
> drm_file *file_priv,
>   nv04_cursor_upload(dev, cursor, nv_crtc->cursor.nvbo);
>  
>   nouveau_bo_unmap(cursor);
> - nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->bo.offset;
> + nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->offset;
>   nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.offset);
>   nv_crtc->cursor.show(nv_crtc, true);
>  out:
> @@ -1191,7 +1191,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct 
> drm_framebuffer *fb,
>   /* Initialize a page flip struct */
>   *s = (struct nv04_page_flip_state)
>   { { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
> -   new_bo->bo.offset };
> +   new_bo->offset };
>  
>   /* Keep vblanks on during flip, for the target crtc of this flip */
>   drm_crtc_vblank_get(crtc);
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index 44ee82d0c9b6..89a4ddfcc55f 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -151,7 +151,7 @@ nv04_display_init(struct drm_device *dev, bool resume, 
> bool runtime)
>   continue;
>  
>   if (nv_crtc->cursor.set_offset)
> - nv_crtc->cursor.set_offset(nv_crtc, 
> nv_crtc->cursor.nvbo->bo.offset);
> + nv_crtc->cursor.set_offset(nv_crtc, 
> nv_crtc->cursor.nvbo->offset);
>   nv_crtc->cursor.set_pos(nv_crtc, nv_crtc->cursor_saved_x,
>nv_crtc->cursor_saved_y);
>   }
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c 
> b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> index a3a0a73ae8ab..9529bd9053e7 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> @@ -150,7 +150,7 @@ nv10_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>   nvif_mask(dev, NV_PCRTC_ENGINE_CTRL + soff2, NV_CRTC_FSEL_OVERLAY, 0);
>  
>   nvif_wr32(dev, NV_PVIDEO_BASE(flip), 0);
> - nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset);
> + nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->offset);
>   nvif_wr32(dev, NV_PVIDEO_SIZE_IN(flip), src_h << 16 | src_w);
>   nvif_wr32(dev, NV_PVIDEO_POINT_IN(flip), src_y << 16 | src_x);
>   nvif_wr32(dev, NV_PVIDEO_DS_DX(flip), (src_w << 20) / crtc_w);
> @@ -172,7 +172,7 @@ nv10_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>   if (format & NV_PVIDEO_FORMAT_PLANAR) {
>   nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
>   nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
> - nv_fb->nvbo->bo.offset + fb->offsets[1]);
> + nv_fb->nvbo->offset + fb->offsets[1]);
>   }
>   nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
>   nvif_wr32(dev, NV_PVIDEO_STOP, 0);
> @@ -396,7 +396,7 @@ nv04_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>  
>   for (i = 0; i < 2; i++) {
>   nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i,
> -   nv_fb->nvbo->bo.offset);
> +   nv_fb->nvbo->offset);
>

Re: [PATCH 2/8] drm/radeon: don't use ttm bo->offset

[Luben Tuikov]

On 2020-03-05 08:29, Nirmoy Das wrote:
> Calculate GPU offset in radeon_bo_gpu_offset without depending on
> bo->offset.
> 
> Signed-off-by: Nirmoy Das 
> Reviewed-and-tested-by: Christian König 
> ---
>  drivers/gpu/drm/radeon/radeon.h|  1 +
>  drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
>  drivers/gpu/drm/radeon/radeon_ttm.c|  4 +---
>  3 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 30e32adc1fc6..b7c3fb2bfb54 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2828,6 +2828,7 @@ extern void radeon_ttm_set_active_vram_size(struct 
> radeon_device *rdev, u64 size
>  extern void radeon_program_register_sequence(struct radeon_device *rdev,
>const u32 *registers,
>const u32 array_size);
> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
> 
>  /*
>   * vm
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h 
> b/drivers/gpu/drm/radeon/radeon_object.h
> index d23f2ed4126e..60275b822f79 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo 
> *bo)
>   */
>  static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
>  {
> - return bo->tbo.offset;
> + struct radeon_device *rdev;
> + u64 start = 0;
> +
> + rdev = radeon_get_rdev(bo->tbo.bdev);
> +
> + switch (bo->tbo.mem.mem_type) {
> + case TTM_PL_TT:
> + start = rdev->mc.gtt_start;
> + break;
> + case TTM_PL_VRAM:
> + start = rdev->mc.vram_start;
> + break;
> + }
> +
> + return (bo->tbo.mem.start << PAGE_SHIFT) + start;
>  }

You're removing a "return bo->tbo.offset" and adding a
switch-case statement. So, then, now instead of an instant
lookup, you're adding branching. You're adding comparison
and branching. Do you think that's better? Faster? Smaller?

I've written before about this for this patch: Why not create a map,
whose index is "mem_type" which references the desired
address? No comparison, no branching. Just an index-dereference
and a value:

return rdev->mc.mem_start_map[bo->tbo.mem.mem_type];

Obviously, you'll have to create "mem_start_map".

That's a NAK from me on this patch using comparison
and branching to return static data lookup value.

Regards,
Luben

> 
>  static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
> b/drivers/gpu/drm/radeon/radeon_ttm.c
> index badf1b6d1549..1c8303468e8f 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -56,7 +56,7 @@
>  static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
>  static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
> 
> -static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>  {
>   struct radeon_mman *mman;
>   struct radeon_device *rdev;
> @@ -82,7 +82,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
> uint32_t type,
>   break;
>   case TTM_PL_TT:
>   man->func = _bo_manager_func;
> - man->gpu_offset = rdev->mc.gtt_start;
>   man->available_caching = TTM_PL_MASK_CACHING;
>   man->default_caching = TTM_PL_FLAG_CACHED;
>   man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> @@ -104,7 +103,6 @@ static int radeon_init_mem_type(struct ttm_bo_device 
> *bdev, uint32_t type,
>   case TTM_PL_VRAM:
>   /* "On-card" video ram */
>   man->func = _bo_manager_func;
> - man->gpu_offset = rdev->mc.vram_start;
>   man->flags = TTM_MEMTYPE_FLAG_FIXED |
>TTM_MEMTYPE_FLAG_MAPPABLE;
>   man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> --
> 2.25.0
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cluben.tuikov%40amd.com%7Cca6004a5ac7a400a030708d7c108bcde%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637190115619487827sdata=EkSy4vpUIbTE%2B75CSO37JWiULKbRTYbcZUSEtRpcrTk%3Dreserved=0
> 

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

Re: [PATCH 1/8] drm/amdgpu: move ttm bo->offset to amdgpu_bo

[Luben Tuikov]

On 2020-03-05 08:29, Nirmoy Das wrote:
> GPU address should belong to driver not in memory management.
> This patch moves ttm bo.offset and gpu_offset calculation to amdgpu driver.
> 
> Signed-off-by: Nirmoy Das 
> Acked-by: Huang Rui 
> Reviewed-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 29 --
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h|  1 +
>  4 files changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 1791c084787d..52c7e579f2d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -918,7 +918,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
> domain,
>   bo->pin_count++;
>  
>   if (max_offset != 0) {
> - u64 domain_start = 
> bo->tbo.bdev->man[mem_type].gpu_offset;
> + u64 domain_start = amdgpu_ttm_domain_start(adev, 
> mem_type);
>   WARN_ON_ONCE(max_offset <
>(amdgpu_bo_gpu_offset(bo) - domain_start));
>   }
> @@ -1483,7 +1483,25 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>   WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
>!(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
>  
> - return amdgpu_gmc_sign_extend(bo->tbo.offset);
> + return amdgpu_bo_gpu_offset_no_check(bo);
> +}
> +
> +/**
> + * amdgpu_bo_gpu_offset_no_check - return GPU offset of bo
> + * @bo:  amdgpu object for which we query the offset
> + *
> + * Returns:
> + * current GPU offset of the object without raising warnings.
> + */
> +u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
> +{
> + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> + uint64_t offset;
> +
> +offset = (bo->tbo.mem.start << PAGE_SHIFT) +
> +  amdgpu_ttm_domain_start(adev, bo->tbo.mem.mem_type);

Align the above assignment, "offset" is out of alignment, as is the line 
following it.

Regards,
Luben

> +
> + return amdgpu_gmc_sign_extend(offset);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 5e39ecd8cc28..32edd35d2ccf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -282,6 +282,7 @@ int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev, 
> struct dma_resv *resv,
>bool intr);
>  int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
>  u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
> +u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
>  int amdgpu_bo_validate(struct amdgpu_bo *bo);
>  int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
>struct dma_fence **fence);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index fe131c21e8a3..87781fabf5f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -96,7 +96,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
> uint32_t type,
>   case TTM_PL_TT:
>   /* GTT memory  */
>   man->func = _gtt_mgr_func;
> - man->gpu_offset = adev->gmc.gart_start;
>   man->available_caching = TTM_PL_MASK_CACHING;
>   man->default_caching = TTM_PL_FLAG_CACHED;
>   man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> @@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device 
> *bdev, uint32_t type,
>   case TTM_PL_VRAM:
>   /* "On-card" video ram */
>   man->func = _vram_mgr_func;
> - man->gpu_offset = adev->gmc.vram_start;
>   man->flags = TTM_MEMTYPE_FLAG_FIXED |
>TTM_MEMTYPE_FLAG_MAPPABLE;
>   man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> @@ -115,7 +113,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device 
> *bdev, uint32_t type,
>   case AMDGPU_PL_OA:
>   /* On-chip GDS memory*/
>   man->func = _bo_manager_func;
> - man->gpu_offset = 0;
>   man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
>   man->available_caching = TTM_PL_FLAG_UNCACHED;
>   man->default_caching = TTM_PL_FLAG_UNCACHED;
> @@ -263,7 +260,7 @@ static uint64_t amdgpu_mm_node_addr(struct 
> ttm_buffer_object *bo,
>  
>   if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
>   addr = mm_node->start << PAGE_SHIFT;
> - addr += bo->bdev->man[mem->mem_type].gpu_offset;
> + addr += 

[pull] amdgpu 5.6 fixes

[Alex Deucher]

Hi Dave, Daniel,

Fixes for 5.6.

The following changes since commit 70b8ea1ab1d3ff3ad5c7491bf8995c912506da6c:

  Merge tag 'mediatek-drm-fixes-5.6' of 
https://github.com/ckhu-mediatek/linux.git-tags into drm-fixes (2020-03-05 
12:59:44 +1000)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux tags/amd-drm-fixes-5.6-2020-03-05

for you to fetch changes up to 09ed6ba43e659474878b22d40b141a01d09ec857:

  drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume 
from s3 (v2) (2020-03-05 09:42:08 -0500)


amd-drm-fixes-5.6-2020-03-05:

amdgpu:
- Gfx reset fix for gfx9, 10
- Fix for gfx10
- DP MST fix
- DCC fix
- Renoir power fixes
- Navi power fix


Bhawanpreet Lakha (1):
  drm/amd/display: Clear link settings on MST disable connector

Hersen Wu (1):
  drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu 
resume from s3 (v2)

Josip Pavic (1):
  drm/amd/display: fix dcc swath size calculations on dcn1

Prike Liang (2):
  drm/amd/powerplay: fix pre-check condition for setting clock range
  drm/amd/powerplay: map mclk to fclk for COMBINATIONAL_BYPASS case

Tianci.Yin (1):
  drm/amdgpu: disable 3D pipe 1 on Navi1x

Yintian Tao (1):
  drm/amdgpu: clean wptr on wb when gpu recovery

 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 98 --
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |  1 +
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 69 +++
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c|  1 +
 .../gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c|  4 +-
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c |  2 +-
 drivers/gpu/drm/amd/powerplay/renoir_ppt.c |  6 +-
 drivers/gpu/drm/amd/powerplay/smu_v12_0.c  |  3 -
 8 files changed, 129 insertions(+), 55 deletions(-)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/4] drm/scheduler: implement a function to modify sched list

[Luben Tuikov]

On 2020-03-05 04:10, Nirmoy wrote:
> 
> On 3/4/20 11:00 PM, Luben Tuikov wrote:
>> struct drm_sched_entity *entity,
>>>void *owner);
>>> +void drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
>>> + struct drm_gpu_scheduler **sched_list,
>>> +  unsigned int num_sched_list);
>>> +
>> Again, the argument list here is unaligned. Please align it
> looks good with cat and vim. Not sure why git format-patch is acting 
> strange in this case

You mean, "cat" and "vim" are acting strange. Then don't use them.
Use Emacs. It's easy to use and you don't need to switch modes to
edit.

Regards,
Luben
P.S. Please surround your replied comments with empty lines.

> 
> 
> Nirmoy
> 

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

Re: [PATCH 2/3] drm/dp_mst: Don't show connectors as connected before probing available PBN

[Lyude Paul]

On Thu, 2020-03-05 at 20:29 +0200, Ville Syrjälä wrote:
> On Thu, Mar 05, 2020 at 01:13:36PM -0500, Lyude Paul wrote:
> > On Thu, 2020-03-05 at 15:11 +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 04, 2020 at 05:36:12PM -0500, Lyude Paul wrote:
> > > > It's next to impossible for us to do connector probing on topologies
> > > > without occasionally racing with userspace, since creating a connector
> > > > itself causes a hotplug event which we have to send before probing the
> > > > available PBN of a connector. Even if we didn't have this hotplug
> > > > event
> > > > sent, there's still always a chance that userspace started probing
> > > > connectors before we finished probing the topology.
> > > > 
> > > > This can be a problem when validating a new MST state since the
> > > > connector will be shown as connected briefly, but without any
> > > > available
> > > > PBN - causing any atomic state which would enable said connector to
> > > > fail
> > > > with -ENOSPC. So, let's simply workaround this by telling userspace
> > > > new
> > > > MST connectors are disconnected until we've finished probing their
> > > > PBN.
> > > > Since we always send a hotplug event at the end of the link address
> > > > probing process, userspace will still know to reprobe the connector
> > > > when
> > > > we're ready.
> > > > 
> > > > Signed-off-by: Lyude Paul 
> > > > Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to
> > > > MST
> > > > atomic check")
> > > > Cc: Mikita Lipski 
> > > > Cc: Alex Deucher 
> > > > Cc: Sean Paul 
> > > > Cc: Hans de Goede 
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
> > > >  1 file changed, 13 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index 207eef08d12c..7b0ff0cff954 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -4033,6 +4033,19 @@ drm_dp_mst_detect_port(struct drm_connector
> > > > *connector,
> > > > ret = connector_status_connected;
> > > > break;
> > > > }
> > > > +
> > > > +   /* We don't want to tell userspace the port is actually
> > > > plugged into
> > > > +* anything until we've finished probing it's available_pbn,
> > > > otherwise
> > > 
> > > "its"
> > > 
> > > Why is the connector even registered before we've finished the probe?
> > > 
> > Oops, I'm not sure how I did this by accident but the explanation I gave
> > in
> > the commit message was uh, completely wrong. I must have forgotten that I
> > made
> > sure we didn't expose connectors before probing their PBN back when I
> > started
> > my MST cleanup
> > 
> > So: despite what I said before it's not actually when new connectors are
> > created, it's when downstream hotplugs happen which means that the
> > conenctor's
> > always going to be there before we probe the available_pbn.
> 
> Not sure I understand. You're saying this is going to change for already
> existing connectors when something else gets plugged in, and either we
> zero it out during the probe or it always was zero to begin with for
> whatever reason?

So: you just made me realize that I'm not actually sure whether there's any
point to us clearing port->available_pbn here since the available_pbn (at
least the value that we cache on initial link address probing for bandwidth
constraint checking) shouldn't actually change on a port just because of a
hotplug. I bet this is probably causing more problems on it's own as well,
since reprobing the available_pbn might actually give us a value that reflects
allocations on other ports that are already in place.

So: I think what I'm going to do instead is make it so that we never clear
port->available_pbn; mainly to make things less complicated during
suspend/resume, since we want to make sure there's always some sort of PBN
value populated even during the middle of reprobing the link address on
resume. That way we don't have to pretend that it's ever disconnected either.
Will send a respin in a bit.

> 
> > I did just notice
> > though that we send a hotplug on connection status notifications even
> > before
> > we've finished the PBN probe, so I might be able to improve on that as
> > well.
> > We still definitely want to report the connector as disconnected before we
> > have the available PBN though, in case another probe was already going
> > before
> > we got the connection status notification.
> > 
> > I'll make sure to fixup the explanation in the commit message on the next
> > respin
> > 
> > > > +* userspace will see racy atomic check failures
> > > > +*
> > > > +* Since we always send a hotplug at the end of probing
> > > > topology
> > > > +* state, we can just let userspace reprobe this connector
> > > > later.
> > > > +*/
> > > > +   if (ret == connector_status_connected && 

Re: [PATCH v4 3/4] drm/amdgpu: change hw sched list on ctx priority override

[Luben Tuikov]

On 2020-03-05 01:28, Nirmoy wrote:
> 
> On 3/4/20 11:25 PM, Luben Tuikov wrote:
>> On 2020-03-03 7:50 a.m., Nirmoy Das wrote:
>>> Switch to appropriate sched list for an entity on priority override.
>>>
>>> Signed-off-by: Nirmoy Das 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 32 +
>>>   1 file changed, 28 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> index 8c52152e3a6e..a0bf14ab9d33 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> @@ -522,6 +522,32 @@ struct dma_fence *amdgpu_ctx_get_fence(struct 
>>> amdgpu_ctx *ctx,
>>> return fence;
>>>   }
>>>
>>> +static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
>>> +  struct amdgpu_ctx_entity *aentity,
>>> +  int hw_ip, enum drm_sched_priority priority)
>>> +{
>>> +   struct amdgpu_device *adev = ctx->adev;
>>> +   enum gfx_pipe_priority hw_prio;
>>> +   struct drm_gpu_scheduler **scheds = NULL;
>>> +   unsigned num_scheds;
>>> +
>>> +   /* set sw priority */
>>> +   drm_sched_entity_set_priority(>entity, priority);
>>> +
>>> +   /* set hw priority */
>>> +   switch (hw_ip) {
>>> +   case AMDGPU_HW_IP_COMPUTE:
>>> +   hw_prio = amdgpu_ctx_sched_prio_to_compute_prio(priority);
>>> +   scheds = adev->gfx.compute_prio_sched[hw_prio];
>>> +   num_scheds = adev->gfx.num_compute_sched[hw_prio];
>>> +   break;
>>> +   default:
>>> +   return;
>>> +   }
>>> +
>>> +   drm_sched_entity_modify_sched(>entity, scheds, num_scheds);
>>> +}
>> I'd rather this not be over-engineered (in expectations of more case labels,
>> and a simple if-else to do it. Over-engineering it "just in case" creates
>> difficult to maintain code. I believe there is a document about this 
>> somewhere
>> in Documentation/.
> 
> This switch is in expectation of extending it for VCN and GFX but I 
> guess that can wait for now,.

No, please, do not over-engineer. Don't make decisions on what the next
programmer and implementation would be.

If you search the LKML you'll see plenty of emails from Linus and
other Linux leaders about the drawbacks of over-engineering.

Just have an if statement as I wrote which you remove without
adding "[snip]". Please add "[snip]" if you're removing content
when you reply. Everyone does this. It's only common courtesy.

Here it is again, because you removed it, conveniently:

You don't need a break only to execute one statement, which you can pull
into the case: label. If you did this you'll see that you just want to do:

static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx,
   struct amdgpu_ctx_entity *aentity,
   int hw_ip, enum drm_sched_priority 
priority)
{

...

/* Set software priority.
 */
drm_sched_entity_set_priority(>entity, priority);

/* Set hardware priority.
 */
if (hw_ip == AMDGPU_HW_IP_COMPUTE) {
hw_prio = s2p_prio_map(priority - 2);  ## or perhaps from a 
static inline from a header file, so we wouldn't care for the - 2 here
scheds = adev->gfx.compute_prio_sched[hw_prio];
num_scheds = adev->gfx.num_compute_sched[hw_prio];
drm_sched_entity_modify_sched(>entity, scheds, 
num_scheds);
}
}

Regards,
Luben


> 
> 
> thanks,
> 
> Nirmoy
> 

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

Re: [PATCH v2 2/4] drm/scheduler: implement a function to modify sched list

[Luben Tuikov]

On 2020-03-05 01:23, Nirmoy wrote:
> 
> On 3/4/20 11:00 PM, Luben Tuikov wrote:
>> On 2020-03-03 7:50 a.m., Nirmoy Das wrote:
>>> implement drm_sched_entity_modify_sched() which can modify existing
>>> sched_list with a different one. This is going to be helpful when
>>> userspace changes priority of a ctx/entity then driver can switch to
>>> corresponding hw shced list for that priority
>>>
>> This commit message should start with a capitalized sentence:
>> "implement" --> "Implement".  "can modify" --> "modifies"
>>
>> Also a spell check should be done on it before committing:
>> "shced" --> "sched".
>>
>> "then the driver", "to the corresponding", "HW scheduler".
>>
>> And the commit message paragraph should end with a period.
> Thanks!
>>

A difficult to see comment of yours above?
Why not add an empty line before and above your comments?
Are you trying to make them hard to find?

>>> Signed-off-by: Nirmoy Das 
>>> Reviewed-by: Christian König 
>>> ---
>>>   drivers/gpu/drm/scheduler/sched_entity.c | 19 +++
>>>   include/drm/gpu_scheduler.h  |  4 
>>>   2 files changed, 23 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>> index 63bccd201b97..b94312154e56 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>> @@ -83,6 +83,25 @@ int drm_sched_entity_init(struct drm_sched_entity 
>>> *entity,
>>>   }
>>>   EXPORT_SYMBOL(drm_sched_entity_init);
>>>
>>> +/**
>>> + * drm_sched_entity_modify_sched - Modify sched of an entity
>>> + *
>> This empty line is unncessary.
>>
>>> + * @entity: scheduler entity to init
>>> + * @sched_list: the list of new drm scheds which will replace
>>> + * existing entity->sched_list
>>> + * @num_sched_list: number of drm sched in sched_list
>>> + */
>>> +void drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
>>> + struct drm_gpu_scheduler **sched_list,
>>> + unsigned int num_sched_list)
>>> +{
>>> +   WARN_ON(!num_sched_list || !sched_list);
>>> +
>>> +   entity->sched_list = sched_list;
>>> +   entity->num_sched_list = num_sched_list;
>>> +}
>>> +EXPORT_SYMBOL(drm_sched_entity_modify_sched);
>>> +
>>>   /**
>>>* drm_sched_entity_is_idle - Check if entity is idle
>>>*
>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>>> index 589be851f8a1..f70a84aaaf7a 100644
>>> --- a/include/drm/gpu_scheduler.h
>>> +++ b/include/drm/gpu_scheduler.h
>>> @@ -297,6 +297,10 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched);
>>>   int drm_sched_job_init(struct drm_sched_job *job,
>>>struct drm_sched_entity *entity,
>>>void *owner);
>>> +void drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
>>> + struct drm_gpu_scheduler **sched_list,
>>> +  unsigned int num_sched_list);
>>> +
> This generally doesn't happen with my current vi config. I will be extra 
> careful.
>> Again, the argument list here is unaligned. Please align it

Another hard to see and find comment from you. Why? Why not
take the extra care and diligence to add an empty line before
and after your comment, so that it is easy to see? Why do you do this?

I think it is "generally" unaligned coming from you as you can
see in the reviews in this list. Configure "vi" or use something
else, "Emacs", etc.

Regards,
Luben

>> correctly. The correct indentation would add the maximum number of
>> leading TAB chars followed by the 0 to 7 spaces, to align
>> the argument list to the first argument in the opening parenthesis.
>> The LKCS describes this and also has the settings for Emacs.
>> In Emacs, pressing the TAB key aligns, after which pressing it more
>> does nothing (on an aligned line).
>>
>> Regards,
>> Luben
>>
>>
>>>   void drm_sched_job_cleanup(struct drm_sched_job *job);
>>>   void drm_sched_wakeup(struct drm_gpu_scheduler *sched);
>>>   void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job 
>>> *bad);
>>> --
>>> 2.25.0
>>>
>>> ___
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cluben.tuikov%40amd.com%7Cbe8445945c194bd8b3cc08d7bf7109c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637188364557354217sdata=UAxV2NtkNcj6VXAPhDbk4GliUOnPEfuLOH4BVuOrqdw%3Dreserved=0
>>>

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

Re: [PATCH v6 1/1] drm/amdgpu: set compute queue priority at mqd_init

[Luben Tuikov]

It is very difficult to find your comment replies when
you do not add an empty line around them.

Do you not see how everyone responds and adds
an empty line around them?

Why don't you?

cont'd below

On 2020-03-05 01:21, Nirmoy wrote:
> 
> On 3/4/20 10:41 PM, Luben Tuikov wrote:
>> On 2020-03-03 7:50 a.m., Nirmoy Das wrote:
>>> We were changing compute ring priority while rings were being used
>>> before every job submission which is not recommended. This patch
>>> sets compute queue priority at mqd initialization for gfx8, gfx9 and
>>> gfx10.
>>>
>>> Policy: make queue 0 of each pipe as high priority compute queue
>>>
>>> High/normal priority compute sched lists are generated from set of 
>>> high/normal
>>> priority compute queues. At context creation, entity of compute queue
>>> get a sched list from high or normal priority depending on ctx->priority
>>>
>>> Signed-off-by: Nirmoy Das 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  4 --
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c  | 60 +---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c  |  8 
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  | 15 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c  |  6 ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 +
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 19 
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 23 +++--
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 
>>>   9 files changed, 135 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> index f397ff97b4e4..8304d0c87899 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> @@ -1205,7 +1205,6 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser 
>>> *p,
>>> struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>>> struct drm_sched_entity *entity = p->entity;
>>> enum drm_sched_priority priority;
>>> -   struct amdgpu_ring *ring;
>>> struct amdgpu_bo_list_entry *e;
>>> struct amdgpu_job *job;
>>> uint64_t seq;
>>> @@ -1258,9 +1257,6 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser 
>>> *p,
>>> priority = job->base.s_priority;
>>> drm_sched_entity_push_job(>base, entity);
>>>
>>> -   ring = to_amdgpu_ring(entity->rq->sched);
>>> -   amdgpu_ring_priority_get(ring, priority);
>>> -
>>> amdgpu_vm_move_to_lru_tail(p->adev, >vm);
>>>
>>> ttm_eu_fence_buffer_objects(>ticket, >validated, p->fence);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> index 94a6c42f29ea..4ad944f85672 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> @@ -61,12 +61,30 @@ static int amdgpu_ctx_priority_permit(struct drm_file 
>>> *filp,
>>> return -EACCES;
>>>   }
>>>
>>> +static enum gfx_pipe_priority amdgpu_ctx_sched_prio_to_compute_prio(enum 
>>> drm_sched_priority prio)
>>> +{
>>> +   switch(prio) {
>> LKCS wants a space after a keyword ("switch") and before parenthesis "(".
>> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces
>>
>> Please read the LKCS in you local Linux source code:
>> Documentation/process/coding-style.rst
>> so we don't have to point that out anymore.
> Yes this is happening more than often with me. I will be careful.
>>

See above? No empty lines around your reply.

Just read the LKCS document a few times.

>>> +   case DRM_SCHED_PRIORITY_MIN:
>>> +   case DRM_SCHED_PRIORITY_NORMAL:
>>> +   case DRM_SCHED_PRIORITY_HIGH_SW:
>>> +   return AMDGPU_GFX_PIPE_PRIO_NORMAL;
>> This is taken care of by the "default:" label and
>> unnecessary here (can be removed completely).
> Thanks!
>>
>>> +   case DRM_SCHED_PRIORITY_HIGH_HW:
>>> +   case DRM_SCHED_PRIORITY_KERNEL:
>>> +   return AMDGPU_GFX_PIPE_PRIO_HIGH;
>>> +   default:
>>> +   return AMDGPU_GFX_PIPE_PRIO_NORMAL;
>>> +   }
>> This can be a map. We're mapping from one integer
>> space to another. There is no reason for a jump switch.
>>
>> For instance,
>>
>> /* Map of the DRM scheduling priority to pipe
>>   * priority.
>>   */
>> const enum gfx_pipe_priority s2p_prio_map[] = {
>>  [0] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>>  [1] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>>  [2] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>>  [3] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>>  [4] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>>  [5] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>>  [6] = AMDGPU_GFX_PIPE_PRIO_HIGH,
>>  [7] = AMDGPU_GFX_PIPE_PRIO_HIGH,
>>  [8] = AMDGPU_GFX_PIPE_PRIO_NORMAL,
>> };
>>
>> /* Map it!
>>   */
>> pipe_prio = s2p_prio_map[sched_prio + 2];   ## You can view this as y = f(x 
>> + 2).
> 
> I think that would be over-engineering for not so much of extra benefit. 
> A switch statement here is  more expressive and
> 
> bit more immune to changes that might happen to DRM_SCHED_PRIORITY_*. I 
> 

Re: [PATCH 2/3] drm/dp_mst: Don't show connectors as connected before probing available PBN

[Ville Syrjälä]

On Thu, Mar 05, 2020 at 01:13:36PM -0500, Lyude Paul wrote:
> On Thu, 2020-03-05 at 15:11 +0200, Ville Syrjälä wrote:
> > On Wed, Mar 04, 2020 at 05:36:12PM -0500, Lyude Paul wrote:
> > > It's next to impossible for us to do connector probing on topologies
> > > without occasionally racing with userspace, since creating a connector
> > > itself causes a hotplug event which we have to send before probing the
> > > available PBN of a connector. Even if we didn't have this hotplug event
> > > sent, there's still always a chance that userspace started probing
> > > connectors before we finished probing the topology.
> > > 
> > > This can be a problem when validating a new MST state since the
> > > connector will be shown as connected briefly, but without any available
> > > PBN - causing any atomic state which would enable said connector to fail
> > > with -ENOSPC. So, let's simply workaround this by telling userspace new
> > > MST connectors are disconnected until we've finished probing their PBN.
> > > Since we always send a hotplug event at the end of the link address
> > > probing process, userspace will still know to reprobe the connector when
> > > we're ready.
> > > 
> > > Signed-off-by: Lyude Paul 
> > > Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST
> > > atomic check")
> > > Cc: Mikita Lipski 
> > > Cc: Alex Deucher 
> > > Cc: Sean Paul 
> > > Cc: Hans de Goede 
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 207eef08d12c..7b0ff0cff954 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -4033,6 +4033,19 @@ drm_dp_mst_detect_port(struct drm_connector
> > > *connector,
> > >   ret = connector_status_connected;
> > >   break;
> > >   }
> > > +
> > > + /* We don't want to tell userspace the port is actually plugged into
> > > +  * anything until we've finished probing it's available_pbn, otherwise
> > 
> > "its"
> > 
> > Why is the connector even registered before we've finished the probe?
> > 
> Oops, I'm not sure how I did this by accident but the explanation I gave in
> the commit message was uh, completely wrong. I must have forgotten that I made
> sure we didn't expose connectors before probing their PBN back when I started
> my MST cleanup
> 
> So: despite what I said before it's not actually when new connectors are
> created, it's when downstream hotplugs happen which means that the conenctor's
> always going to be there before we probe the available_pbn.

Not sure I understand. You're saying this is going to change for already
existing connectors when something else gets plugged in, and either we
zero it out during the probe or it always was zero to begin with for
whatever reason?

> I did just notice
> though that we send a hotplug on connection status notifications even before
> we've finished the PBN probe, so I might be able to improve on that as well.
> We still definitely want to report the connector as disconnected before we
> have the available PBN though, in case another probe was already going before
> we got the connection status notification.
> 
> I'll make sure to fixup the explanation in the commit message on the next
> respin
> 
> > > +  * userspace will see racy atomic check failures
> > > +  *
> > > +  * Since we always send a hotplug at the end of probing topology
> > > +  * state, we can just let userspace reprobe this connector later.
> > > +  */
> > > + if (ret == connector_status_connected && !port->available_pbn) {
> > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not ready yet (PBN not
> > > probed)\n",
> > > +   connector->base.id, connector->name);
> > > + ret = connector_status_disconnected;
> > > + }
> > >  out:
> > >   drm_dp_mst_topology_put_port(port);
> > >   return ret;
> > > -- 
> > > 2.24.1
> > > 
> > > ___
> > > dri-devel mailing list
> > > dri-de...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> -- 
> Cheers,
>   Lyude Paul (she/her)
>   Associate Software Engineer at Red Hat

-- 
Ville Syrjälä
Intel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/dp_mst: Don't show connectors as connected before probing available PBN

[Lyude Paul]

On Thu, 2020-03-05 at 15:11 +0200, Ville Syrjälä wrote:
> On Wed, Mar 04, 2020 at 05:36:12PM -0500, Lyude Paul wrote:
> > It's next to impossible for us to do connector probing on topologies
> > without occasionally racing with userspace, since creating a connector
> > itself causes a hotplug event which we have to send before probing the
> > available PBN of a connector. Even if we didn't have this hotplug event
> > sent, there's still always a chance that userspace started probing
> > connectors before we finished probing the topology.
> > 
> > This can be a problem when validating a new MST state since the
> > connector will be shown as connected briefly, but without any available
> > PBN - causing any atomic state which would enable said connector to fail
> > with -ENOSPC. So, let's simply workaround this by telling userspace new
> > MST connectors are disconnected until we've finished probing their PBN.
> > Since we always send a hotplug event at the end of the link address
> > probing process, userspace will still know to reprobe the connector when
> > we're ready.
> > 
> > Signed-off-by: Lyude Paul 
> > Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST
> > atomic check")
> > Cc: Mikita Lipski 
> > Cc: Alex Deucher 
> > Cc: Sean Paul 
> > Cc: Hans de Goede 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 207eef08d12c..7b0ff0cff954 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4033,6 +4033,19 @@ drm_dp_mst_detect_port(struct drm_connector
> > *connector,
> > ret = connector_status_connected;
> > break;
> > }
> > +
> > +   /* We don't want to tell userspace the port is actually plugged into
> > +* anything until we've finished probing it's available_pbn, otherwise
> 
> "its"
> 
> Why is the connector even registered before we've finished the probe?
> 
Oops, I'm not sure how I did this by accident but the explanation I gave in
the commit message was uh, completely wrong. I must have forgotten that I made
sure we didn't expose connectors before probing their PBN back when I started
my MST cleanup

So: despite what I said before it's not actually when new connectors are
created, it's when downstream hotplugs happen which means that the conenctor's
always going to be there before we probe the available_pbn. I did just notice
though that we send a hotplug on connection status notifications even before
we've finished the PBN probe, so I might be able to improve on that as well.
We still definitely want to report the connector as disconnected before we
have the available PBN though, in case another probe was already going before
we got the connection status notification.

I'll make sure to fixup the explanation in the commit message on the next
respin

> > +* userspace will see racy atomic check failures
> > +*
> > +* Since we always send a hotplug at the end of probing topology
> > +* state, we can just let userspace reprobe this connector later.
> > +*/
> > +   if (ret == connector_status_connected && !port->available_pbn) {
> > +   DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not ready yet (PBN not
> > probed)\n",
> > + connector->base.id, connector->name);
> > +   ret = connector_status_disconnected;
> > +   }
> >  out:
> > drm_dp_mst_topology_put_port(port);
> > return ret;
> > -- 
> > 2.24.1
> > 
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Cheers,
Lyude Paul (she/her)
Associate Software Engineer at Red Hat

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

[PATCH AUTOSEL 5.4 36/58] drm/amdgpu: fix memory leak during TDR test(v2)

[Sasha Levin]

From: Monk Liu 

[ Upstream commit 4829f89855f1d3a3d8014e74cceab51b421503db ]

fix system memory leak

v2:
fix coding style

Signed-off-by: Monk Liu 
Reviewed-by: Hawking Zhang 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index c5257ae3188a3..0922d9cd858a0 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -988,8 +988,12 @@ static int smu_v11_0_init_max_sustainable_clocks(struct 
smu_context *smu)
struct smu_11_0_max_sustainable_clocks *max_sustainable_clocks;
int ret = 0;
 
-   max_sustainable_clocks = kzalloc(sizeof(struct 
smu_11_0_max_sustainable_clocks),
+   if (!smu->smu_table.max_sustainable_clocks)
+   max_sustainable_clocks = kzalloc(sizeof(struct 
smu_11_0_max_sustainable_clocks),
 GFP_KERNEL);
+   else
+   max_sustainable_clocks = smu->smu_table.max_sustainable_clocks;
+
smu->smu_table.max_sustainable_clocks = (void *)max_sustainable_clocks;
 
max_sustainable_clocks->uclock = smu->smu_table.boot_values.uclk / 100;
-- 
2.20.1

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

[PATCH AUTOSEL 5.5 41/67] drm/amdgpu: fix memory leak during TDR test(v2)

[Sasha Levin]

From: Monk Liu 

[ Upstream commit 4829f89855f1d3a3d8014e74cceab51b421503db ]

fix system memory leak

v2:
fix coding style

Signed-off-by: Monk Liu 
Reviewed-by: Hawking Zhang 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 8b13d18c6414c..e4149e6b68b39 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -948,8 +948,12 @@ int smu_v11_0_init_max_sustainable_clocks(struct 
smu_context *smu)
struct smu_11_0_max_sustainable_clocks *max_sustainable_clocks;
int ret = 0;
 
-   max_sustainable_clocks = kzalloc(sizeof(struct 
smu_11_0_max_sustainable_clocks),
+   if (!smu->smu_table.max_sustainable_clocks)
+   max_sustainable_clocks = kzalloc(sizeof(struct 
smu_11_0_max_sustainable_clocks),
 GFP_KERNEL);
+   else
+   max_sustainable_clocks = smu->smu_table.max_sustainable_clocks;
+
smu->smu_table.max_sustainable_clocks = (void *)max_sustainable_clocks;
 
max_sustainable_clocks->uclock = smu->smu_table.boot_values.uclk / 100;
-- 
2.20.1

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

Re: [PATCH 0/3] drm/dp_mst: Fix bandwidth checking regressions from DSC patches

[Hans de Goede]

Hi Lyude,

On 3/4/20 11:36 PM, Lyude Paul wrote:

AMD's patch series for adding DSC support to the MST helpers
unfortunately introduced a few regressions into the kernel that I didn't
get around to fixing until just now. I would have reverted the changes
earlier, but seeing as that would have reverted all of amd's DSC support
+ everything that was done on top of that I really wanted to avoid
doing that.

Anyway, this should fix everything as far as I can tell. Note that I
don't have any DSC displays locally yet, so if someone from AMD could
sanity check this I would appreciate it ♥.


Thank you for trying to fix this, unfortunately for me this is not fixed,
with this series. My setup:

5.6-rc4 + your 3 patches (+ some unrelated patches outside of drm)

-Lenovo x1 7th gen +
 Lenovo TB3 dock gen 2 +
 2 external 1920x1080@60 monitors connected to the 2 HDMI interfaces on the dock
-System booted with the LID closed, so that the firmware/BIOS has already
 initialized both monitors when the kernel boots

This should be fairly easy to reproduce on a similar setup, other
users are seeing similar problems when connecting more then 1 monitor
to DP-MST docks, see e.g. :

https://bugzilla.redhat.com/show_bug.cgi?id=1809681
https://bugzilla.redhat.com/show_bug.cgi?id=1810070

Let me know if you want me to collect some drm.debug logs, I guess
if you do, you want me to use drm.debug=0x114 ?

Regards,

Hans







Cc: Mikita Lipski 
Cc: Alex Deucher 
Cc: Sean Paul 
Cc: Hans de Goede 

Lyude Paul (3):
   drm/dp_mst: Rename drm_dp_mst_is_dp_mst_end_device() to be less
 redundant
   drm/dp_mst: Don't show connectors as connected before probing
 available PBN
   drm/dp_mst: Rewrite and fix bandwidth limit checks

  drivers/gpu/drm/drm_dp_mst_topology.c | 124 --
  1 file changed, 96 insertions(+), 28 deletions(-)



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

Re: [enable VCN2.0 for NV12 SRIOV 6/6] drm/amdgpu: clear warning on unused var

[Leo Liu]

Is this warning introduced by your patch 4?

On 2020-03-05 8:33 a.m., Monk Liu wrote:

Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index ae9754f..a41272f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -493,7 +493,6 @@ static int amdgpu_vcn_dec_get_destroy_msg(struct 
amdgpu_ring *ring, uint32_t han
  
  int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout)

  {
-   struct amdgpu_device *adev = ring->adev;
struct dma_fence *fence;
long r;
  
@@ -655,7 +654,6 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t han
  
  int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)

  {
-   struct amdgpu_device *adev = ring->adev;
struct dma_fence *fence = NULL;
struct amdgpu_bo *bo = NULL;
long r;

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

Re: [enable VCN2.0 for NV12 SRIOV 5/6] drm/amdgpu: disable clock/power gating for SRIOV

[Leo Liu]

This patch is:

Acked-by: Leo Liu 

On 2020-03-05 8:33 a.m., Monk Liu wrote:

and disable MC resum in VCN2.0 as well

those are not concerned by VF driver

Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 23 +++
  1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index dd500d1..f2745fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -320,6 +320,9 @@ static void vcn_v2_0_mc_resume(struct amdgpu_device *adev)
uint32_t size = AMDGPU_GPU_PAGE_ALIGN(adev->vcn.fw->size + 4);
uint32_t offset;
  
+	if (amdgpu_sriov_vf(adev))

+   return;
+
/* cache window 0: fw */
if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
WREG32_SOC15(UVD, 0, mmUVD_LMI_VCPU_CACHE_64BIT_BAR_LOW,
@@ -464,6 +467,9 @@ static void vcn_v2_0_disable_clock_gating(struct 
amdgpu_device *adev)
  {
uint32_t data;
  
+	if (amdgpu_sriov_vf(adev))

+   return;
+
/* UVD disable CGC */
data = RREG32_SOC15(VCN, 0, mmUVD_CGC_CTRL);
if (adev->cg_flags & AMD_CG_SUPPORT_VCN_MGCG)
@@ -622,6 +628,9 @@ static void vcn_v2_0_enable_clock_gating(struct 
amdgpu_device *adev)
  {
uint32_t data = 0;
  
+	if (amdgpu_sriov_vf(adev))

+   return;
+
/* enable UVD CGC */
data = RREG32_SOC15(VCN, 0, mmUVD_CGC_CTRL);
if (adev->cg_flags & AMD_CG_SUPPORT_VCN_MGCG)
@@ -674,6 +683,9 @@ static void vcn_v2_0_disable_static_power_gating(struct 
amdgpu_device *adev)
uint32_t data = 0;
int ret;
  
+	if (amdgpu_sriov_vf(adev))

+   return;
+
if (adev->pg_flags & AMD_PG_SUPPORT_VCN) {
data = (1 << UVD_PGFSM_CONFIG__UVDM_PWR_CONFIG__SHIFT
| 1 << UVD_PGFSM_CONFIG__UVDU_PWR_CONFIG__SHIFT
@@ -721,6 +733,9 @@ static void vcn_v2_0_enable_static_power_gating(struct 
amdgpu_device *adev)
uint32_t data = 0;
int ret;
  
+	if (amdgpu_sriov_vf(adev))

+   return;
+
if (adev->pg_flags & AMD_PG_SUPPORT_VCN) {
/* Before power off, this indicator has to be turned on */
data = RREG32_SOC15(VCN, 0, mmUVD_POWER_STATUS);
@@ -1231,6 +1246,9 @@ static int vcn_v2_0_set_clockgating_state(void *handle,
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
bool enable = (state == AMD_CG_STATE_GATE);
  
+	if (amdgpu_sriov_vf(adev))

+   return 0;
+
if (enable) {
/* wait for STATUS to clear */
if (vcn_v2_0_is_idle(handle))
@@ -1686,6 +1704,11 @@ static int vcn_v2_0_set_powergating_state(void *handle,
int ret;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
  
+	if (amdgpu_sriov_vf(adev)) {

+   adev->vcn.cur_state = AMD_PG_STATE_UNGATE;
+   return 0;
+   }
+
if (state == adev->vcn.cur_state)
return 0;
  

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

Re: [enable VCN2.0 for NV12 SRIOV 4/6] drm/amdgpu: cleanup ring/ib test for SRIOV vcn2.0

[Leo Liu]

This patch is:

Reviewed-by: Leo Liu 

On 2020-03-05 8:33 a.m., Monk Liu wrote:

support IB test on dec/enc ring
disable ring test on dec/enc ring (MMSCH limitation)

Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 11 +++
  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c   |  3 +++
  2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..ae9754f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -497,10 +497,6 @@ int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
struct dma_fence *fence;
long r;
  
-	/* temporarily disable ib test for sriov */

-   if (amdgpu_sriov_vf(adev))
-   return 0;
-
r = amdgpu_vcn_dec_get_create_msg(ring, 1, NULL);
if (r)
goto error;
@@ -527,6 +523,9 @@ int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring *ring)
unsigned i;
int r;
  
+	if (amdgpu_sriov_vf(adev))

+   return 0;
+
r = amdgpu_ring_alloc(ring, 16);
if (r)
return r;
@@ -661,10 +660,6 @@ int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
struct amdgpu_bo *bo = NULL;
long r;
  
-	/* temporarily disable ib test for sriov */

-   if (amdgpu_sriov_vf(adev))
-   return 0;
-
r = amdgpu_bo_create_reserved(ring->adev, 128 * 1024, PAGE_SIZE,
  AMDGPU_GEM_DOMAIN_VRAM,
  , NULL, NULL);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index 421e5bf..dd500d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -1647,6 +1647,9 @@ int vcn_v2_0_dec_ring_test_ring(struct amdgpu_ring *ring)
unsigned i;
int r;
  
+	if (amdgpu_sriov_vf(adev))

+   return 0;
+
WREG32(adev->vcn.inst[ring->me].external.scratch9, 0xCAFEDEAD);
r = amdgpu_ring_alloc(ring, 4);
if (r)

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

Re: [enable VCN2.0 for NV12 SRIOV 3/6] drm/amdgpu: implement initialization part on VCN2.0 for SRIOV

[Leo Liu]

On 2020-03-05 8:33 a.m., Monk Liu wrote:

one dec ring and one enc ring

It seems more than that, you might add more messages.




Signed-off-by: Monk Liu 
---
  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 231 +-
  1 file changed, 228 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index c387c81..421e5bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -29,6 +29,7 @@
  #include "soc15d.h"
  #include "amdgpu_pm.h"
  #include "amdgpu_psp.h"
+#include "mmsch_v2_0.h"
  
  #include "vcn/vcn_2_0_0_offset.h"

  #include "vcn/vcn_2_0_0_sh_mask.h"
@@ -54,7 +55,7 @@ static int vcn_v2_0_set_powergating_state(void *handle,
enum amd_powergating_state state);
  static int vcn_v2_0_pause_dpg_mode(struct amdgpu_device *adev,
int inst_idx, struct dpg_pause_state 
*new_state);
-
+static int vcn_v2_0_start_sriov(struct amdgpu_device *adev);


Please keep the empty line here.



  /**
   * vcn_v2_0_early_init - set function pointers
   *
@@ -67,7 +68,10 @@ static int vcn_v2_0_early_init(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
  
  	adev->vcn.num_vcn_inst = 1;

-   adev->vcn.num_enc_rings = 2;
+   if (amdgpu_sriov_vf(adev))
+   adev->vcn.num_enc_rings = 1;
+   else
+   adev->vcn.num_enc_rings = 2;
  
  	vcn_v2_0_set_dec_ring_funcs(adev);

vcn_v2_0_set_enc_ring_funcs(adev);
@@ -154,7 +158,10 @@ static int vcn_v2_0_sw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst->ring_enc[i];
ring->use_doorbell = true;
-   ring->doorbell_index = (adev->doorbell_index.vcn.vcn_ring0_1 << 
1) + 2 + i;
+   if (!amdgpu_sriov_vf(adev))
+   ring->doorbell_index = (adev->doorbell_index.vcn.vcn_ring0_1 
<< 1) + 2 + i;
+   else
+   ring->doorbell_index = (adev->doorbell_index.vcn.vcn_ring0_1 
<< 1) + 1 + i;
sprintf(ring->name, "vcn_enc%d", i);
r = amdgpu_ring_init(adev, ring, 512, >vcn.inst->irq, 0);
if (r)
@@ -163,6 +170,10 @@ static int vcn_v2_0_sw_init(void *handle)
  
  	adev->vcn.pause_dpg_mode = vcn_v2_0_pause_dpg_mode;
  
+	r = amdgpu_virt_alloc_mm_table(adev);

+   if (r)
+   return r;
+


This is not needed for bare metal.



return 0;
  }
  
@@ -178,6 +189,8 @@ static int vcn_v2_0_sw_fini(void *handle)

int r;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
  
+	amdgpu_virt_free_mm_table(adev);

+


same as above here.


Regards,

Leo




r = amdgpu_vcn_suspend(adev);
if (r)
return r;
@@ -203,6 +216,9 @@ static int vcn_v2_0_hw_init(void *handle)
adev->nbio.funcs->vcn_doorbell_range(adev, ring->use_doorbell,
 ring->doorbell_index, 0);
  
+	if (amdgpu_sriov_vf(adev))

+   vcn_v2_0_start_sriov(adev);
+
r = amdgpu_ring_test_helper(ring);
if (r)
goto done;
@@ -1680,6 +1696,215 @@ static int vcn_v2_0_set_powergating_state(void *handle,
return ret;
  }
  
+static int vcn_v2_0_start_mmsch(struct amdgpu_device *adev,

+   struct amdgpu_mm_table *table)
+{
+   uint32_t data = 0, loop;
+   uint64_t addr = table->gpu_addr;
+   struct mmsch_v2_0_init_header *header;
+   uint32_t size;
+   int i;
+
+   header = (struct mmsch_v2_0_init_header *)table->cpu_addr;
+   size = header->header_size + header->vcn_table_size;
+
+   /* 1, write to vce_mmsch_vf_ctx_addr_lo/hi register with GPU mc addr
+* of memory descriptor location
+*/
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_ADDR_LO, lower_32_bits(addr));
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_ADDR_HI, upper_32_bits(addr));
+
+   /* 2, update vmid of descriptor */
+   data = RREG32_SOC15(UVD, 0, mmMMSCH_VF_VMID);
+   data &= ~MMSCH_VF_VMID__VF_CTX_VMID_MASK;
+   /* use domain0 for MM scheduler */
+   data |= (0 << MMSCH_VF_VMID__VF_CTX_VMID__SHIFT);
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_VMID, data);
+
+   /* 3, notify mmsch about the size of this descriptor */
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_SIZE, size);
+
+   /* 4, set resp to zero */
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_MAILBOX_RESP, 0);
+
+   adev->vcn.inst->ring_dec.wptr = 0;
+   adev->vcn.inst->ring_dec.wptr_old = 0;
+   vcn_v2_0_dec_ring_set_wptr(>vcn.inst->ring_dec);
+
+   for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
+   adev->vcn.inst->ring_enc[i].wptr = 0;
+   adev->vcn.inst->ring_enc[i].wptr_old = 0;
+   vcn_v2_0_enc_ring_set_wptr(>vcn.inst->ring_enc[i]);
+   }
+
+   /* 5, kick off 

Re: [PATCH 2/2] drm/amdkfd: Signal eviction fence on process destruction (v2)

[Felix Kuehling]

[moving to public mailing list]

Thank you. I'll also apply patch 2/2 to amd-staging-drm-next. It's not 
fixing a memory leak there, but it should make cleanup after process 
termination more efficient by avoiding delayed delete of BOs.


Regards,
  Felix

On 2020-03-04 10:46 p.m., Pan, Xinhui wrote:

Series is Reviewed-by: xinhui pan 


2020年3月5日 05:50，Kuehling, Felix  写道：

Otherwise BOs may wait for the fence indefinitely and never be destroyed.

v2: Signal the fence right after destroying queues to avoid unnecessary
delaye-delete in kfd_process_wq_release

Signed-off-by: Felix Kuehling 
---
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 5 +
1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index d5d4660221af..26f7f178b66d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -625,6 +625,11 @@ static void kfd_process_notifier_release(struct 
mmu_notifier *mn,

/* Indicate to other users that MM is no longer valid */
p->mm = NULL;
+   /* Signal the eviction fence after user mode queues are
+* destroyed. This allows any BOs to be freed without
+* triggering pointless evictions or waiting for fences.
+*/
+   dma_fence_signal(p->ef);

mutex_unlock(>mutex);

--
2.25.1


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

Re: [enable VCN2.0 for NV12 SRIOV 2/6] drm/amdgpu: disable jpeg block for SRIOV

[Leo Liu]

On 2020-03-05 8:39 a.m., Liu, Monk wrote:

This is not supported by MMSCH FW...


With this added to commit message, this patch is:

Reviewed-by: Leo Liu 





_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Christian König 
Sent: Thursday, March 5, 2020 9:38 PM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: [enable VCN2.0 for NV12 SRIOV 2/6] drm/amdgpu: disable jpeg block 
for SRIOV

A commit message explaining why we disable it and if it could be enabled again 
or if this is permanent would be nice to have.

Christian.

Am 05.03.20 um 14:33 schrieb Monk Liu:

Signed-off-by: Monk Liu 
---
   drivers/gpu/drm/amd/amdgpu/nv.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
b/drivers/gpu/drm/amd/amdgpu/nv.c index 2d1bebd..033cbbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -516,7 +516,8 @@ int nv_set_ip_blocks(struct amdgpu_device *adev)
!amdgpu_sriov_vf(adev))
amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
amdgpu_device_ip_block_add(adev, _v2_0_ip_block);
-   amdgpu_device_ip_block_add(adev, _v2_0_ip_block);
+   if (!amdgpu_sriov_vf(adev))
+   amdgpu_device_ip_block_add(adev, _v2_0_ip_block);
break;
default:
return -EINVAL;

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cleo.liu%40amd.com%7C73f28c93e88241bc4c5908d7c10a96e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637190123555130366sdata=CVcac9dEWc3mR0oNkcrkOTtxXqvdtjzEN78c%2FBYty8E%3Dreserved=0

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

Re: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

[Deucher, Alexander]

[AMD Public Use]

Is this for the series, or just this patch?  Thanks!

Alex

From: Xu, Feifei 
Sent: Thursday, March 5, 2020 12:24 AM
To: Alex Deucher ; amd-gfx list 

Cc: Deucher, Alexander ; Tawfik, Aly 

Subject: RE: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

[AMD Official Use Only - Internal Distribution Only]



Reviewed-by: Feifei Xu 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: 2020年3月5日 13:23
To: amd-gfx list 
Cc: Deucher, Alexander ; Tawfik, Aly 

Subject: Re: [PATCH 1/2] drm/amdgpu/display: fix pci revision id fetching

Ping?

On Tue, Feb 25, 2020 at 3:32 PM Alex Deucher  wrote:
>
> From: Aly-Tawfik 
>
> Use the pci revision id rather than the asic silicon revision id.
>
> Signed-off-by: Aly-Tawfik 
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 0c55d44c9d5b..92166150bf9f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -902,7 +902,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>
> init_data.asic_id.chip_family = adev->family;
>
> -   init_data.asic_id.pci_revision_id = adev->rev_id;
> +   init_data.asic_id.pci_revision_id = adev->pdev->revision;
> init_data.asic_id.hw_internal_rev = adev->external_rev_id;
>
> init_data.asic_id.vram_width = adev->gmc.vram_width;
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CFeifei.Xu%40amd.com%7Cdd49232261ac4267a33f08d7c0c54ad8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637189825908683804sdata=sN%2BGSvrmnz5Qdrf4JR0sFUDWMWyu7tsOh6ZG9lb9I5Y%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start

[James Zhu]

On 2020-03-05 6:27 a.m., Christian König wrote:

Am 05.03.20 um 12:25 schrieb Christian König:

Am 04.03.20 um 17:34 schrieb James Zhu:

Fix race condition issue when multiple vcn starts are called.

v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.

Signed-off-by: James Zhu 


Reviewed-by: Christian König 


One thing worth noting is that in theory you could run into the issue 
that one ring restarts the timer while another ring is still preparing 
the engine for usage.


Yes, you are right, The current timeout setting is 1 sec to guarantee 
one dec/enc can be finished for all formats.  the preparing process 
should nuch less than this setting.


otherwise there are some other bugs that needs to be fixed.

By the way, maybe we shouldn't just rely on fence check to determine if 
any job is left. We can maintain a dec/enc submission count(patch 2 
already added for enc). how do you think?


Best Regards!

James



So the timeout should be large enough to guarantee that this never 
causes problems.


Regards,
Christian.




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
  2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c

index f96464e..8a8406b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
  int i, r;
    INIT_DELAYED_WORK(>vcn.idle_work, 
amdgpu_vcn_idle_work_handler);

+    mutex_init(>vcn.vcn_pg_lock);
    switch (adev->asic_type) {
  case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
  }
    release_firmware(adev->vcn.fw);
+    mutex_destroy(>vcn.vcn_pg_lock);
    return 0;
  }
@@ -319,13 +321,13 @@ static void 
amdgpu_vcn_idle_work_handler(struct work_struct *work)

  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
  {
  struct amdgpu_device *adev = ring->adev;
-    bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
  -    if (set_clocks) {
-    amdgpu_gfx_off_ctrl(adev, false);
-    amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,

-   AMD_PG_STATE_UNGATE);
-    }
+    cancel_delayed_work_sync(>vcn.idle_work);
+
+    mutex_lock(>vcn.vcn_pg_lock);
+    amdgpu_gfx_off_ctrl(adev, false);
+    amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,

+   AMD_PG_STATE_UNGATE);
    if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
  struct dpg_pause_state new_state;
@@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct 
amdgpu_ring *ring)

    adev->vcn.pause_dpg_mode(adev, ring->me, _state);
  }
+    mutex_unlock(>vcn.vcn_pg_lock);
  }
    void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
  struct drm_gpu_scheduler 
*vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];

  uint32_t num_vcn_enc_sched;
  uint32_t num_vcn_dec_sched;
+    struct mutex vcn_pg_lock;
    unsigned    harvest_config;
  int (*pause_dpg_mode)(struct amdgpu_device *adev,





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

Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v3

[Christian König]

Am 05.03.20 um 15:35 schrieb Nirmoy:



On 3/5/20 3:07 PM, Gerd Hoffmann wrote:

On Thu, Mar 05, 2020 at 02:29:08PM +0100, Nirmoy Das wrote:

Calculate GEM VRAM bo's offset within vram-helper without depending on
bo->offset.

Signed-off-by: Nirmoy Das
Reviewed-by: Daniel Vetter
---
  drivers/gpu/drm/drm_gem_vram_helper.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 92a11bb42365..2749c2d25ac4 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -198,6 +198,13 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object 
*gbo)
  }
  EXPORT_SYMBOL(drm_gem_vram_mmap_offset);

+static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
+{
+   if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
+   return 0;

returns 0 on error.


I am not sure if we should call this an error. This patch series 
removes below offset calculation from ttm_bo.c.


-   if (bo->mem.mm_node)
-   bo->offset = (bo->mem.start << PAGE_SHIFT) +
-   bdev->man[bo->mem.mem_type].gpu_offset;
-   else
-   bo->offset = 0;
-

Most of the driver sets "bo->mem.mm_node". Thomas suggested to use 
this "return 0" in case some driver depends on bo->offset = 0.




We should probably add a code comment here to explain why we do this.

Something like "Keep TTM behavior for now, remove when drivers are audited".

Regards,
Christian.


+   return gbo->bo.mem.start;
+}
+
  /**
   * drm_gem_vram_offset() - \
Returns a GEM VRAM object's offset in video memory
@@ -214,7 +221,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
  {
if (WARN_ON_ONCE(!gbo->pin_count))
return (s64)-ENODEV;

returns -errno on error.


-   return gbo->bo.offset;
+   return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;

And given that one calls the other behavior on error should better be
consistent ...

cheers,
   Gerd



Regards,

Nirmoy


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


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

Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v3

[Nirmoy]

On 3/5/20 3:07 PM, Gerd Hoffmann wrote:

On Thu, Mar 05, 2020 at 02:29:08PM +0100, Nirmoy Das wrote:

Calculate GEM VRAM bo's offset within vram-helper without depending on
bo->offset.

Signed-off-by: Nirmoy Das 
Reviewed-by: Daniel Vetter 
---
  drivers/gpu/drm/drm_gem_vram_helper.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 92a11bb42365..2749c2d25ac4 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -198,6 +198,13 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object 
*gbo)
  }
  EXPORT_SYMBOL(drm_gem_vram_mmap_offset);

+static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
+{
+   if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
+   return 0;

returns 0 on error.


I am not sure if we should call this an error. This patch series removes 
below offset calculation from ttm_bo.c.


-   if (bo->mem.mm_node)
-   bo->offset = (bo->mem.start << PAGE_SHIFT) +
-   bdev->man[bo->mem.mem_type].gpu_offset;
-   else
-   bo->offset = 0;
-

Most of the driver sets "bo->mem.mm_node". Thomas suggested to use this 
"return 0" in case some driver depends on bo->offset = 0.





+   return gbo->bo.mem.start;
+}
+
  /**
   * drm_gem_vram_offset() - \
Returns a GEM VRAM object's offset in video memory
@@ -214,7 +221,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
  {
if (WARN_ON_ONCE(!gbo->pin_count))
return (s64)-ENODEV;

returns -errno on error.


-   return gbo->bo.offset;
+   return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;

And given that one calls the other behavior on error should better be
consistent ...

cheers,
   Gerd



Regards,

Nirmoy

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

Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v3

[Thomas Zimmermann]

Hi

Am 05.03.20 um 15:07 schrieb Gerd Hoffmann:
> On Thu, Mar 05, 2020 at 02:29:08PM +0100, Nirmoy Das wrote:
>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>> bo->offset.
>>
>> Signed-off-by: Nirmoy Das 
>> Reviewed-by: Daniel Vetter 
>> ---
>>  drivers/gpu/drm/drm_gem_vram_helper.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 92a11bb42365..2749c2d25ac4 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -198,6 +198,13 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object 
>> *gbo)
>>  }
>>  EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>
>> +static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>> +{
>> +if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>> +return 0;
> 
> returns 0 on error.
> 
>> +return gbo->bo.mem.start;
>> +}
>> +
>>  /**
>>   * drm_gem_vram_offset() - \
>>  Returns a GEM VRAM object's offset in video memory
>> @@ -214,7 +221,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
>>  {
>>  if (WARN_ON_ONCE(!gbo->pin_count))
>>  return (s64)-ENODEV;
> 
> returns -errno on error.
> 
>> -return gbo->bo.offset;
>> +return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
> 
> And given that one calls the other behavior on error should better be
> consistent ...

It is expected that the offset is valid if pin_count is positive.
Anything else would be a massive ref-counting bug. And that's been the
behavior of the old code as well.

But I agree that the current patch is inconsistent. I suggest changing
the return type of drm_gem_vram_pg_offset() to u64.

Best regards
Thomas

> 
> cheers,
>   Gerd
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



signature.asc
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v3

[Gerd Hoffmann]

On Thu, Mar 05, 2020 at 02:29:08PM +0100, Nirmoy Das wrote:
> Calculate GEM VRAM bo's offset within vram-helper without depending on
> bo->offset.
> 
> Signed-off-by: Nirmoy Das 
> Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 92a11bb42365..2749c2d25ac4 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -198,6 +198,13 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object 
> *gbo)
>  }
>  EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
> 
> +static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
> +{
> + if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
> + return 0;

returns 0 on error.

> + return gbo->bo.mem.start;
> +}
> +
>  /**
>   * drm_gem_vram_offset() - \
>   Returns a GEM VRAM object's offset in video memory
> @@ -214,7 +221,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
>  {
>   if (WARN_ON_ONCE(!gbo->pin_count))
>   return (s64)-ENODEV;

returns -errno on error.

> - return gbo->bo.offset;
> + return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;

And given that one calls the other behavior on error should better be
consistent ...

cheers,
  Gerd

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

RE: [enable VCN2.0 for NV12 SRIOV 2/6] drm/amdgpu: disable jpeg block for SRIOV

[Liu, Monk]

This is not supported by MMSCH FW... 

_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Christian König  
Sent: Thursday, March 5, 2020 9:38 PM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: [enable VCN2.0 for NV12 SRIOV 2/6] drm/amdgpu: disable jpeg block 
for SRIOV

A commit message explaining why we disable it and if it could be enabled again 
or if this is permanent would be nice to have.

Christian.

Am 05.03.20 um 14:33 schrieb Monk Liu:
> Signed-off-by: Monk Liu 
> ---
>   drivers/gpu/drm/amd/amdgpu/nv.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
> b/drivers/gpu/drm/amd/amdgpu/nv.c index 2d1bebd..033cbbc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -516,7 +516,8 @@ int nv_set_ip_blocks(struct amdgpu_device *adev)
>   !amdgpu_sriov_vf(adev))
>   amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
>   amdgpu_device_ip_block_add(adev, _v2_0_ip_block);
> - amdgpu_device_ip_block_add(adev, _v2_0_ip_block);
> + if (!amdgpu_sriov_vf(adev))
> + amdgpu_device_ip_block_add(adev, _v2_0_ip_block);
>   break;
>   default:
>   return -EINVAL;

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

Re: [enable VCN2.0 for NV12 SRIOV 6/6] drm/amdgpu: clear warning on unused var

[Christian König]

Am 05.03.20 um 14:33 schrieb Monk Liu:

Signed-off-by: Monk Liu 


Acked-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index ae9754f..a41272f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -493,7 +493,6 @@ static int amdgpu_vcn_dec_get_destroy_msg(struct 
amdgpu_ring *ring, uint32_t han
  
  int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout)

  {
-   struct amdgpu_device *adev = ring->adev;
struct dma_fence *fence;
long r;
  
@@ -655,7 +654,6 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t han
  
  int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long timeout)

  {
-   struct amdgpu_device *adev = ring->adev;
struct dma_fence *fence = NULL;
struct amdgpu_bo *bo = NULL;
long r;


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

Re: [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses

[Christian König]

Am 05.03.20 um 14:29 schrieb Nirmoy Das:

GPU address handling is device specific and should be handle by its device
driver.

Signed-off-by: Nirmoy Das 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/ttm/ttm_bo.c| 7 ---
  include/drm/ttm/ttm_bo_api.h| 2 --
  include/drm/ttm/ttm_bo_driver.h | 1 -
  3 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6d1e91be9c78..9f24fb287d71 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -85,7 +85,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, 
struct drm_printer *p
drm_printf(p, "has_type: %d\n", man->has_type);
drm_printf(p, "use_type: %d\n", man->use_type);
drm_printf(p, "flags: 0x%08X\n", man->flags);
-   drm_printf(p, "gpu_offset: 0x%08llX\n", man->gpu_offset);
drm_printf(p, "size: %llu\n", man->size);
drm_printf(p, "available_caching: 0x%08X\n", 
man->available_caching);
drm_printf(p, "default_caching: 0x%08X\n", man->default_caching);
@@ -345,12 +344,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object 
*bo,
  moved:
bo->evicted = false;
  
-	if (bo->mem.mm_node)

-   bo->offset = (bo->mem.start << PAGE_SHIFT) +
-   bdev->man[bo->mem.mem_type].gpu_offset;
-   else
-   bo->offset = 0;
-
ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
  
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h

index b9bc1b00142e..d6f39ee5bf5d 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -213,8 +213,6 @@ struct ttm_buffer_object {
 * either of these locks held.
 */
  
-	uint64_t offset; /* GPU address space is independent of CPU word size */

-
struct sg_table *sg;
  };
  
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h

index c9e0fd09f4b2..c8ce6c181abe 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -177,7 +177,6 @@ struct ttm_mem_type_manager {
bool has_type;
bool use_type;
uint32_t flags;
-   uint64_t gpu_offset; /* GPU address space is independent of CPU word 
size */
uint64_t size;
uint32_t available_caching;
uint32_t default_caching;


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

[enable VCN2.0 for NV12 SRIOV 5/6] drm/amdgpu: disable clock/power gating for SRIOV

[Monk Liu]

and disable MC resum in VCN2.0 as well

those are not concerned by VF driver

Signed-off-by: Monk Liu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index dd500d1..f2745fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -320,6 +320,9 @@ static void vcn_v2_0_mc_resume(struct amdgpu_device *adev)
uint32_t size = AMDGPU_GPU_PAGE_ALIGN(adev->vcn.fw->size + 4);
uint32_t offset;
 
+   if (amdgpu_sriov_vf(adev))
+   return;
+
/* cache window 0: fw */
if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
WREG32_SOC15(UVD, 0, mmUVD_LMI_VCPU_CACHE_64BIT_BAR_LOW,
@@ -464,6 +467,9 @@ static void vcn_v2_0_disable_clock_gating(struct 
amdgpu_device *adev)
 {
uint32_t data;
 
+   if (amdgpu_sriov_vf(adev))
+   return;
+
/* UVD disable CGC */
data = RREG32_SOC15(VCN, 0, mmUVD_CGC_CTRL);
if (adev->cg_flags & AMD_CG_SUPPORT_VCN_MGCG)
@@ -622,6 +628,9 @@ static void vcn_v2_0_enable_clock_gating(struct 
amdgpu_device *adev)
 {
uint32_t data = 0;
 
+   if (amdgpu_sriov_vf(adev))
+   return;
+
/* enable UVD CGC */
data = RREG32_SOC15(VCN, 0, mmUVD_CGC_CTRL);
if (adev->cg_flags & AMD_CG_SUPPORT_VCN_MGCG)
@@ -674,6 +683,9 @@ static void vcn_v2_0_disable_static_power_gating(struct 
amdgpu_device *adev)
uint32_t data = 0;
int ret;
 
+   if (amdgpu_sriov_vf(adev))
+   return;
+
if (adev->pg_flags & AMD_PG_SUPPORT_VCN) {
data = (1 << UVD_PGFSM_CONFIG__UVDM_PWR_CONFIG__SHIFT
| 1 << UVD_PGFSM_CONFIG__UVDU_PWR_CONFIG__SHIFT
@@ -721,6 +733,9 @@ static void vcn_v2_0_enable_static_power_gating(struct 
amdgpu_device *adev)
uint32_t data = 0;
int ret;
 
+   if (amdgpu_sriov_vf(adev))
+   return;
+
if (adev->pg_flags & AMD_PG_SUPPORT_VCN) {
/* Before power off, this indicator has to be turned on */
data = RREG32_SOC15(VCN, 0, mmUVD_POWER_STATUS);
@@ -1231,6 +1246,9 @@ static int vcn_v2_0_set_clockgating_state(void *handle,
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
bool enable = (state == AMD_CG_STATE_GATE);
 
+   if (amdgpu_sriov_vf(adev))
+   return 0;
+
if (enable) {
/* wait for STATUS to clear */
if (vcn_v2_0_is_idle(handle))
@@ -1686,6 +1704,11 @@ static int vcn_v2_0_set_powergating_state(void *handle,
int ret;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+   if (amdgpu_sriov_vf(adev)) {
+   adev->vcn.cur_state = AMD_PG_STATE_UNGATE;
+   return 0;
+   }
+
if (state == adev->vcn.cur_state)
return 0;
 
-- 
2.7.4

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

Re: [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3

[Christian König]

Am 05.03.20 um 14:29 schrieb Nirmoy Das:

Store ttm bo->offset in struct nouveau_bo instead.

Signed-off-by: Nirmoy Das 


Looks like this is the only patch without an rb or at least acked-by. 
Can anybody comment or at least throw a quick tested-by on it?


With that done I would say I would pick this series up for inclusion 
into drm-misc-next.


Regards,
Christian.


---
  drivers/gpu/drm/nouveau/dispnv04/crtc.c |  6 +++---
  drivers/gpu/drm/nouveau/dispnv04/disp.c |  2 +-
  drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 +++---
  drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
  drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
  drivers/gpu/drm/nouveau/dispnv50/wndw.c |  2 +-
  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
  drivers/gpu/drm/nouveau/nouveau_abi16.c |  8 
  drivers/gpu/drm/nouveau/nouveau_bo.c|  8 
  drivers/gpu/drm/nouveau/nouveau_bo.h|  3 +++
  drivers/gpu/drm/nouveau/nouveau_chan.c  |  2 +-
  drivers/gpu/drm/nouveau/nouveau_dmem.c  |  2 +-
  drivers/gpu/drm/nouveau/nouveau_fbcon.c |  2 +-
  drivers/gpu/drm/nouveau/nouveau_gem.c   | 10 +-
  15 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 1f08de4241e0..d06a93f2b38a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -845,7 +845,7 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
fb = nouveau_framebuffer(crtc->primary->fb);
}
  
-	nv_crtc->fb.offset = fb->nvbo->bo.offset;

+   nv_crtc->fb.offset = fb->nvbo->offset;
  
  	if (nv_crtc->lut.depth != drm_fb->format->depth) {

nv_crtc->lut.depth = drm_fb->format->depth;
@@ -1013,7 +1013,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct 
drm_file *file_priv,
nv04_cursor_upload(dev, cursor, nv_crtc->cursor.nvbo);
  
  	nouveau_bo_unmap(cursor);

-   nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->bo.offset;
+   nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->offset;
nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.offset);
nv_crtc->cursor.show(nv_crtc, true);
  out:
@@ -1191,7 +1191,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct 
drm_framebuffer *fb,
/* Initialize a page flip struct */
*s = (struct nv04_page_flip_state)
{ { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
- new_bo->bo.offset };
+ new_bo->offset };
  
  	/* Keep vblanks on during flip, for the target crtc of this flip */

drm_crtc_vblank_get(crtc);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 44ee82d0c9b6..89a4ddfcc55f 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -151,7 +151,7 @@ nv04_display_init(struct drm_device *dev, bool resume, bool 
runtime)
continue;
  
  		if (nv_crtc->cursor.set_offset)

-   nv_crtc->cursor.set_offset(nv_crtc, 
nv_crtc->cursor.nvbo->bo.offset);
+   nv_crtc->cursor.set_offset(nv_crtc, 
nv_crtc->cursor.nvbo->offset);
nv_crtc->cursor.set_pos(nv_crtc, nv_crtc->cursor_saved_x,
 nv_crtc->cursor_saved_y);
}
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c 
b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index a3a0a73ae8ab..9529bd9053e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -150,7 +150,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
nvif_mask(dev, NV_PCRTC_ENGINE_CTRL + soff2, NV_CRTC_FSEL_OVERLAY, 0);
  
  	nvif_wr32(dev, NV_PVIDEO_BASE(flip), 0);

-   nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset);
+   nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_SIZE_IN(flip), src_h << 16 | src_w);
nvif_wr32(dev, NV_PVIDEO_POINT_IN(flip), src_y << 16 | src_x);
nvif_wr32(dev, NV_PVIDEO_DS_DX(flip), (src_w << 20) / crtc_w);
@@ -172,7 +172,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
if (format & NV_PVIDEO_FORMAT_PLANAR) {
nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
-   nv_fb->nvbo->bo.offset + fb->offsets[1]);
+   nv_fb->nvbo->offset + fb->offsets[1]);
}
nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_STOP, 0);
@@ -396,7 +396,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
  
  	for (i = 0; i < 2; i++) {

nvif_wr32(dev, 

[enable VCN2.0 for NV12 SRIOV 3/6] drm/amdgpu: implement initialization part on VCN2.0 for SRIOV

[Monk Liu]

one dec ring and one enc ring

Signed-off-by: Monk Liu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 231 +-
 1 file changed, 228 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index c387c81..421e5bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -29,6 +29,7 @@
 #include "soc15d.h"
 #include "amdgpu_pm.h"
 #include "amdgpu_psp.h"
+#include "mmsch_v2_0.h"
 
 #include "vcn/vcn_2_0_0_offset.h"
 #include "vcn/vcn_2_0_0_sh_mask.h"
@@ -54,7 +55,7 @@ static int vcn_v2_0_set_powergating_state(void *handle,
enum amd_powergating_state state);
 static int vcn_v2_0_pause_dpg_mode(struct amdgpu_device *adev,
int inst_idx, struct dpg_pause_state 
*new_state);
-
+static int vcn_v2_0_start_sriov(struct amdgpu_device *adev);
 /**
  * vcn_v2_0_early_init - set function pointers
  *
@@ -67,7 +68,10 @@ static int vcn_v2_0_early_init(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
adev->vcn.num_vcn_inst = 1;
-   adev->vcn.num_enc_rings = 2;
+   if (amdgpu_sriov_vf(adev))
+   adev->vcn.num_enc_rings = 1;
+   else
+   adev->vcn.num_enc_rings = 2;
 
vcn_v2_0_set_dec_ring_funcs(adev);
vcn_v2_0_set_enc_ring_funcs(adev);
@@ -154,7 +158,10 @@ static int vcn_v2_0_sw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst->ring_enc[i];
ring->use_doorbell = true;
-   ring->doorbell_index = (adev->doorbell_index.vcn.vcn_ring0_1 << 
1) + 2 + i;
+   if (!amdgpu_sriov_vf(adev))
+   ring->doorbell_index = 
(adev->doorbell_index.vcn.vcn_ring0_1 << 1) + 2 + i;
+   else
+   ring->doorbell_index = 
(adev->doorbell_index.vcn.vcn_ring0_1 << 1) + 1 + i;
sprintf(ring->name, "vcn_enc%d", i);
r = amdgpu_ring_init(adev, ring, 512, >vcn.inst->irq, 0);
if (r)
@@ -163,6 +170,10 @@ static int vcn_v2_0_sw_init(void *handle)
 
adev->vcn.pause_dpg_mode = vcn_v2_0_pause_dpg_mode;
 
+   r = amdgpu_virt_alloc_mm_table(adev);
+   if (r)
+   return r;
+
return 0;
 }
 
@@ -178,6 +189,8 @@ static int vcn_v2_0_sw_fini(void *handle)
int r;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+   amdgpu_virt_free_mm_table(adev);
+
r = amdgpu_vcn_suspend(adev);
if (r)
return r;
@@ -203,6 +216,9 @@ static int vcn_v2_0_hw_init(void *handle)
adev->nbio.funcs->vcn_doorbell_range(adev, ring->use_doorbell,
 ring->doorbell_index, 0);
 
+   if (amdgpu_sriov_vf(adev))
+   vcn_v2_0_start_sriov(adev);
+
r = amdgpu_ring_test_helper(ring);
if (r)
goto done;
@@ -1680,6 +1696,215 @@ static int vcn_v2_0_set_powergating_state(void *handle,
return ret;
 }
 
+static int vcn_v2_0_start_mmsch(struct amdgpu_device *adev,
+   struct amdgpu_mm_table *table)
+{
+   uint32_t data = 0, loop;
+   uint64_t addr = table->gpu_addr;
+   struct mmsch_v2_0_init_header *header;
+   uint32_t size;
+   int i;
+
+   header = (struct mmsch_v2_0_init_header *)table->cpu_addr;
+   size = header->header_size + header->vcn_table_size;
+
+   /* 1, write to vce_mmsch_vf_ctx_addr_lo/hi register with GPU mc addr
+* of memory descriptor location
+*/
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_ADDR_LO, lower_32_bits(addr));
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_ADDR_HI, upper_32_bits(addr));
+
+   /* 2, update vmid of descriptor */
+   data = RREG32_SOC15(UVD, 0, mmMMSCH_VF_VMID);
+   data &= ~MMSCH_VF_VMID__VF_CTX_VMID_MASK;
+   /* use domain0 for MM scheduler */
+   data |= (0 << MMSCH_VF_VMID__VF_CTX_VMID__SHIFT);
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_VMID, data);
+
+   /* 3, notify mmsch about the size of this descriptor */
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_CTX_SIZE, size);
+
+   /* 4, set resp to zero */
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_MAILBOX_RESP, 0);
+
+   adev->vcn.inst->ring_dec.wptr = 0;
+   adev->vcn.inst->ring_dec.wptr_old = 0;
+   vcn_v2_0_dec_ring_set_wptr(>vcn.inst->ring_dec);
+
+   for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
+   adev->vcn.inst->ring_enc[i].wptr = 0;
+   adev->vcn.inst->ring_enc[i].wptr_old = 0;
+   vcn_v2_0_enc_ring_set_wptr(>vcn.inst->ring_enc[i]);
+   }
+
+   /* 5, kick off the initialization and wait until
+* VCE_MMSCH_VF_MAILBOX_RESP becomes non-zero
+*/
+   WREG32_SOC15(UVD, 0, mmMMSCH_VF_MAILBOX_HOST, 0x1001);
+
+   data = RREG32_SOC15(UVD, 0, 

[PATCH 1/8] drm/amdgpu: move ttm bo->offset to amdgpu_bo

[Nirmoy Das]

GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu driver.

Signed-off-by: Nirmoy Das 
Acked-by: Huang Rui 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 29 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h|  1 +
 4 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 1791c084787d..52c7e579f2d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -918,7 +918,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
domain,
bo->pin_count++;
 
if (max_offset != 0) {
-   u64 domain_start = 
bo->tbo.bdev->man[mem_type].gpu_offset;
+   u64 domain_start = amdgpu_ttm_domain_start(adev, 
mem_type);
WARN_ON_ONCE(max_offset <
 (amdgpu_bo_gpu_offset(bo) - domain_start));
}
@@ -1483,7 +1483,25 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
 !(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
 
-   return amdgpu_gmc_sign_extend(bo->tbo.offset);
+   return amdgpu_bo_gpu_offset_no_check(bo);
+}
+
+/**
+ * amdgpu_bo_gpu_offset_no_check - return GPU offset of bo
+ * @bo:amdgpu object for which we query the offset
+ *
+ * Returns:
+ * current GPU offset of the object without raising warnings.
+ */
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
+{
+   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+   uint64_t offset;
+
+offset = (bo->tbo.mem.start << PAGE_SHIFT) +
+amdgpu_ttm_domain_start(adev, bo->tbo.mem.mem_type);
+
+   return amdgpu_gmc_sign_extend(offset);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 5e39ecd8cc28..32edd35d2ccf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -282,6 +282,7 @@ int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev, 
struct dma_resv *resv,
 bool intr);
 int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
 u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
 int amdgpu_bo_validate(struct amdgpu_bo *bo);
 int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 struct dma_fence **fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fe131c21e8a3..87781fabf5f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -96,7 +96,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_TT:
/* GTT memory  */
man->func = _gtt_mgr_func;
-   man->gpu_offset = adev->gmc.gart_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _vram_mgr_func;
-   man->gpu_offset = adev->gmc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
@@ -115,7 +113,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case AMDGPU_PL_OA:
/* On-chip GDS memory*/
man->func = _bo_manager_func;
-   man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
man->available_caching = TTM_PL_FLAG_UNCACHED;
man->default_caching = TTM_PL_FLAG_UNCACHED;
@@ -263,7 +260,7 @@ static uint64_t amdgpu_mm_node_addr(struct 
ttm_buffer_object *bo,
 
if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
addr = mm_node->start << PAGE_SHIFT;
-   addr += bo->bdev->man[mem->mem_type].gpu_offset;
+   addr += amdgpu_ttm_domain_start(amdgpu_ttm_adev(bo->bdev), 
mem->mem_type);
}
return addr;
 }
@@ -750,6 +747,27 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct 
ttm_buffer_object *bo,
(offset >> PAGE_SHIFT);
 }
 
+/**
+ * amdgpu_ttm_domain_start - Returns GPU start address
+ * @adev: 

[PATCH v4 0/8] do not store GPU address in TTM

[Nirmoy Das]

With this patch series I am trying to remove GPU address dependency in
TTM and moving GPU address calculation to individual drm drivers. This
cleanup will simplify introduction of drm_mem_region/domain work started
by Brian Welty[1].


It would be nice if someone test this for nouveau. Rest of the drivers
are already tested.

v2:
* set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL

v3:
* catch return value of drm_gem_vram_offset() in drm/bochs
* introduce drm_gem_vram_pg_offset() in vram helper
* improve nbo->offset calculation for nouveau

v4:
* minor coding style fixes in amdgpu and radeon
* remove unnecessary kerneldoc for internal function

Nirmoy Das (8):
  drm/amdgpu: move ttm bo->offset to amdgpu_bo
  drm/radeon: don't use ttm bo->offset
  drm/vmwgfx: don't use ttm bo->offset
  drm/nouveau: don't use ttm bo->offset v3
  drm/qxl: don't use ttm bo->offset
  drm/vram-helper: don't use ttm bo->offset v3
  drm/bochs: use drm_gem_vram_offset to get bo offset v2
  drm/ttm: do not keep GPU dependent addresses

 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 +
 drivers/gpu/drm/bochs/bochs_kms.c   |  7 -
 drivers/gpu/drm/drm_gem_vram_helper.c   |  9 ++-
 drivers/gpu/drm/nouveau/dispnv04/crtc.c |  6 ++---
 drivers/gpu/drm/nouveau/dispnv04/disp.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
 drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndw.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_abi16.c |  8 +++---
 drivers/gpu/drm/nouveau/nouveau_bo.c|  8 ++
 drivers/gpu/drm/nouveau/nouveau_bo.h|  3 +++
 drivers/gpu/drm/nouveau/nouveau_chan.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_dmem.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c   | 10 +++
 drivers/gpu/drm/qxl/qxl_drv.h   |  6 ++---
 drivers/gpu/drm/qxl/qxl_kms.c   |  5 ++--
 drivers/gpu/drm/qxl/qxl_object.h|  5 
 drivers/gpu/drm/qxl/qxl_ttm.c   |  9 ---
 drivers/gpu/drm/radeon/radeon.h |  1 +
 drivers/gpu/drm/radeon/radeon_object.h  | 16 +++-
 drivers/gpu/drm/radeon/radeon_ttm.c |  4 +--
 drivers/gpu/drm/ttm/ttm_bo.c|  7 -
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c  |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c|  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
 include/drm/ttm/ttm_bo_api.h|  2 --
 include/drm/ttm/ttm_bo_driver.h |  1 -
 35 files changed, 118 insertions(+), 76 deletions(-)

--
2.25.0

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

[PATCH 2/8] drm/radeon: don't use ttm bo->offset

[Nirmoy Das]

Calculate GPU offset in radeon_bo_gpu_offset without depending on
bo->offset.

Signed-off-by: Nirmoy Das 
Reviewed-and-tested-by: Christian König 
---
 drivers/gpu/drm/radeon/radeon.h|  1 +
 drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
 drivers/gpu/drm/radeon/radeon_ttm.c|  4 +---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 30e32adc1fc6..b7c3fb2bfb54 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2828,6 +2828,7 @@ extern void radeon_ttm_set_active_vram_size(struct 
radeon_device *rdev, u64 size
 extern void radeon_program_register_sequence(struct radeon_device *rdev,
 const u32 *registers,
 const u32 array_size);
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);

 /*
  * vm
diff --git a/drivers/gpu/drm/radeon/radeon_object.h 
b/drivers/gpu/drm/radeon/radeon_object.h
index d23f2ed4126e..60275b822f79 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
  */
 static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
 {
-   return bo->tbo.offset;
+   struct radeon_device *rdev;
+   u64 start = 0;
+
+   rdev = radeon_get_rdev(bo->tbo.bdev);
+
+   switch (bo->tbo.mem.mem_type) {
+   case TTM_PL_TT:
+   start = rdev->mc.gtt_start;
+   break;
+   case TTM_PL_VRAM:
+   start = rdev->mc.vram_start;
+   break;
+   }
+
+   return (bo->tbo.mem.start << PAGE_SHIFT) + start;
 }

 static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index badf1b6d1549..1c8303468e8f 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -56,7 +56,7 @@
 static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
 static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);

-static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
 {
struct radeon_mman *mman;
struct radeon_device *rdev;
@@ -82,7 +82,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
break;
case TTM_PL_TT:
man->func = _bo_manager_func;
-   man->gpu_offset = rdev->mc.gtt_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -104,7 +103,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
-   man->gpu_offset = rdev->mc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
--
2.25.0

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

[PATCH 3/8] drm/vmwgfx: don't use ttm bo->offset

[Nirmoy Das]

Calculate GPU offset within vmwgfx driver itself without depending on
bo->offset.

Signed-off-by: Nirmoy Das 
Acked-by: Christian König 
Acked-by: Thomas Hellstrom 
Tested-by: Thomas Hellstrom 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c| 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c   | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 8b71bf6b58ef..1e59c019affa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -258,7 +258,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private 
*dev_priv,
ret = ttm_bo_validate(bo, , );

/* For some reason we didn't end up at the start of vram */
-   WARN_ON(ret == 0 && bo->offset != 0);
+   WARN_ON(ret == 0 && bo->mem.start != 0);
if (!ret)
vmw_bo_pin_reserved(buf, true);

@@ -317,7 +317,7 @@ void vmw_bo_get_guest_ptr(const struct ttm_buffer_object 
*bo,
 {
if (bo->mem.mem_type == TTM_PL_VRAM) {
ptr->gmrId = SVGA_GMR_FRAMEBUFFER;
-   ptr->offset = bo->offset;
+   ptr->offset = bo->mem.start << PAGE_SHIFT;
} else {
ptr->gmrId = bo->mem.start;
ptr->offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 73489a45decb..72c2cf4053df 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3313,7 +3313,7 @@ static void vmw_apply_relocations(struct vmw_sw_context 
*sw_context)
bo = >vbo->base;
switch (bo->mem.mem_type) {
case TTM_PL_VRAM:
-   reloc->location->offset += bo->offset;
+   reloc->location->offset += bo->mem.start << PAGE_SHIFT;
reloc->location->gmrId = SVGA_GMR_FRAMEBUFFER;
break;
case VMW_PL_GMR:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
index e5252ef3812f..1cdc445b24c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -612,7 +612,7 @@ static int vmw_fifo_emit_dummy_legacy_query(struct 
vmw_private *dev_priv,

if (bo->mem.mem_type == TTM_PL_VRAM) {
cmd->body.guestResult.gmrId = SVGA_GMR_FRAMEBUFFER;
-   cmd->body.guestResult.offset = bo->offset;
+   cmd->body.guestResult.offset = bo->mem.start << PAGE_SHIFT;
} else {
cmd->body.guestResult.gmrId = bo->mem.start;
cmd->body.guestResult.offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 3f3b2c7a208a..e7134aebeb81 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -750,7 +750,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
-   man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
man->default_caching = TTM_PL_FLAG_CACHED;
@@ -763,7 +762,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
 *  slots as well as the bo size.
 */
man->func = _gmrid_manager_func;
-   man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
man->default_caching = TTM_PL_FLAG_CACHED;
--
2.25.0

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

[PATCH 8/8] drm/ttm: do not keep GPU dependent addresses

[Nirmoy Das]

GPU address handling is device specific and should be handle by its device
driver.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/ttm/ttm_bo.c| 7 ---
 include/drm/ttm/ttm_bo_api.h| 2 --
 include/drm/ttm/ttm_bo_driver.h | 1 -
 3 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6d1e91be9c78..9f24fb287d71 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -85,7 +85,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, 
struct drm_printer *p
drm_printf(p, "has_type: %d\n", man->has_type);
drm_printf(p, "use_type: %d\n", man->use_type);
drm_printf(p, "flags: 0x%08X\n", man->flags);
-   drm_printf(p, "gpu_offset: 0x%08llX\n", man->gpu_offset);
drm_printf(p, "size: %llu\n", man->size);
drm_printf(p, "available_caching: 0x%08X\n", 
man->available_caching);
drm_printf(p, "default_caching: 0x%08X\n", man->default_caching);
@@ -345,12 +344,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object 
*bo,
 moved:
bo->evicted = false;
 
-   if (bo->mem.mm_node)
-   bo->offset = (bo->mem.start << PAGE_SHIFT) +
-   bdev->man[bo->mem.mem_type].gpu_offset;
-   else
-   bo->offset = 0;
-
ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
 
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index b9bc1b00142e..d6f39ee5bf5d 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -213,8 +213,6 @@ struct ttm_buffer_object {
 * either of these locks held.
 */
 
-   uint64_t offset; /* GPU address space is independent of CPU word size */
-
struct sg_table *sg;
 };
 
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index c9e0fd09f4b2..c8ce6c181abe 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -177,7 +177,6 @@ struct ttm_mem_type_manager {
bool has_type;
bool use_type;
uint32_t flags;
-   uint64_t gpu_offset; /* GPU address space is independent of CPU word 
size */
uint64_t size;
uint32_t available_caching;
uint32_t default_caching;
-- 
2.25.0

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

[PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3

[Nirmoy Das]

Store ttm bo->offset in struct nouveau_bo instead.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c |  6 +++---
 drivers/gpu/drm/nouveau/dispnv04/disp.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 +++---
 drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndw.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_abi16.c |  8 
 drivers/gpu/drm/nouveau/nouveau_bo.c|  8 
 drivers/gpu/drm/nouveau/nouveau_bo.h|  3 +++
 drivers/gpu/drm/nouveau/nouveau_chan.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_dmem.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c   | 10 +-
 15 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 1f08de4241e0..d06a93f2b38a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -845,7 +845,7 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
fb = nouveau_framebuffer(crtc->primary->fb);
}
 
-   nv_crtc->fb.offset = fb->nvbo->bo.offset;
+   nv_crtc->fb.offset = fb->nvbo->offset;
 
if (nv_crtc->lut.depth != drm_fb->format->depth) {
nv_crtc->lut.depth = drm_fb->format->depth;
@@ -1013,7 +1013,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct 
drm_file *file_priv,
nv04_cursor_upload(dev, cursor, nv_crtc->cursor.nvbo);
 
nouveau_bo_unmap(cursor);
-   nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->bo.offset;
+   nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->offset;
nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.offset);
nv_crtc->cursor.show(nv_crtc, true);
 out:
@@ -1191,7 +1191,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct 
drm_framebuffer *fb,
/* Initialize a page flip struct */
*s = (struct nv04_page_flip_state)
{ { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
- new_bo->bo.offset };
+ new_bo->offset };
 
/* Keep vblanks on during flip, for the target crtc of this flip */
drm_crtc_vblank_get(crtc);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 44ee82d0c9b6..89a4ddfcc55f 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -151,7 +151,7 @@ nv04_display_init(struct drm_device *dev, bool resume, bool 
runtime)
continue;
 
if (nv_crtc->cursor.set_offset)
-   nv_crtc->cursor.set_offset(nv_crtc, 
nv_crtc->cursor.nvbo->bo.offset);
+   nv_crtc->cursor.set_offset(nv_crtc, 
nv_crtc->cursor.nvbo->offset);
nv_crtc->cursor.set_pos(nv_crtc, nv_crtc->cursor_saved_x,
 nv_crtc->cursor_saved_y);
}
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c 
b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index a3a0a73ae8ab..9529bd9053e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -150,7 +150,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
nvif_mask(dev, NV_PCRTC_ENGINE_CTRL + soff2, NV_CRTC_FSEL_OVERLAY, 0);
 
nvif_wr32(dev, NV_PVIDEO_BASE(flip), 0);
-   nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset);
+   nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_SIZE_IN(flip), src_h << 16 | src_w);
nvif_wr32(dev, NV_PVIDEO_POINT_IN(flip), src_y << 16 | src_x);
nvif_wr32(dev, NV_PVIDEO_DS_DX(flip), (src_w << 20) / crtc_w);
@@ -172,7 +172,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
if (format & NV_PVIDEO_FORMAT_PLANAR) {
nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
-   nv_fb->nvbo->bo.offset + fb->offsets[1]);
+   nv_fb->nvbo->offset + fb->offsets[1]);
}
nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_STOP, 0);
@@ -396,7 +396,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
 
for (i = 0; i < 2; i++) {
nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i,
- nv_fb->nvbo->bo.offset);
+ nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i,
  fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_BUFF0_OFFSET + 4 

[PATCH 5/8] drm/qxl: don't use ttm bo->offset

[Nirmoy Das]

This patch removes slot->gpu_offset which is not required as
VRAM and PRIV slot are in separate PCI bar.

This patch also removes unused qxl_bo_gpu_offset()

Signed-off-by: Nirmoy Das 
Acked-by: Christian König 
Acked-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.h| 6 ++
 drivers/gpu/drm/qxl/qxl_kms.c| 5 ++---
 drivers/gpu/drm/qxl/qxl_object.h | 5 -
 drivers/gpu/drm/qxl/qxl_ttm.c| 9 -
 4 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 27e45a2d6b52..df581f0e6699 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -134,7 +134,6 @@ struct qxl_memslot {
uint64_tstart_phys_addr;
uint64_tsize;
uint64_thigh_bits;
-   uint64_tgpu_offset;
 };

 enum {
@@ -311,10 +310,9 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct 
qxl_bo *bo,
(bo->tbo.mem.mem_type == TTM_PL_VRAM)
? >main_slot : >surfaces_slot;

-   WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
+   /* TODO - need to hold one of the locks to read bo->tbo.mem.start */

-   /* TODO - need to hold one of the locks to read tbo.offset */
-   return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
+   return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + offset);
 }

 /* qxl_display.c */
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 70b20ee4741a..7a5bf544f34d 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -86,11 +86,10 @@ static void setup_slot(struct qxl_device *qdev,
high_bits <<= (64 - (qdev->rom->slot_gen_bits + 
qdev->rom->slot_id_bits));
slot->high_bits = high_bits;

-   DRM_INFO("slot %d (%s): base 0x%08lx, size 0x%08lx, gpu_offset 0x%lx\n",
+   DRM_INFO("slot %d (%s): base 0x%08lx, size 0x%08lx\n",
 slot->index, slot->name,
 (unsigned long)slot->start_phys_addr,
-(unsigned long)slot->size,
-(unsigned long)slot->gpu_offset);
+(unsigned long)slot->size);
 }

 void qxl_reinit_memslots(struct qxl_device *qdev)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..21fa81048f4f 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -48,11 +48,6 @@ static inline void qxl_bo_unreserve(struct qxl_bo *bo)
ttm_bo_unreserve(>tbo);
 }

-static inline u64 qxl_bo_gpu_offset(struct qxl_bo *bo)
-{
-   return bo->tbo.offset;
-}
-
 static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
 {
return bo->tbo.num_pages << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 62a5e424971b..635d000e7934 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -51,11 +51,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device 
*bdev)
 static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 struct ttm_mem_type_manager *man)
 {
-   struct qxl_device *qdev = qxl_get_qdev(bdev);
-   unsigned int gpu_offset_shift =
-   64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits + 8);
-   struct qxl_memslot *slot;
-
switch (type) {
case TTM_PL_SYSTEM:
/* System memory */
@@ -66,11 +61,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_VRAM:
case TTM_PL_PRIV:
/* "On-card" video ram */
-   slot = (type == TTM_PL_VRAM) ?
-   >main_slot : >surfaces_slot;
-   slot->gpu_offset = (uint64_t)type << gpu_offset_shift;
man->func = _bo_manager_func;
-   man->gpu_offset = slot->gpu_offset;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_MASK_CACHING;
--
2.25.0

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

[PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset v2

[Nirmoy Das]

Switch over to GEM VRAM's implementation to retrieve bo->offset.

Signed-off-by: Nirmoy Das 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 8066d7d370d5..18d2ec34534d 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -29,16 +29,21 @@ static void bochs_plane_update(struct bochs_device *bochs,
   struct drm_plane_state *state)
 {
struct drm_gem_vram_object *gbo;
+   s64 gpu_addr;

if (!state->fb || !bochs->stride)
return;

gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
+   gpu_addr = drm_gem_vram_offset(gbo);
+   if (WARN_ON_ONCE(gpu_addr < 0))
+   return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
+
bochs_hw_setbase(bochs,
 state->crtc_x,
 state->crtc_y,
 state->fb->pitches[0],
-state->fb->offsets[0] + gbo->bo.offset);
+state->fb->offsets[0] + gpu_addr);
bochs_hw_setformat(bochs, state->fb->format);
 }

--
2.25.0

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

[PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v3

[Nirmoy Das]

Calculate GEM VRAM bo's offset within vram-helper without depending on
bo->offset.

Signed-off-by: Nirmoy Das 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 92a11bb42365..2749c2d25ac4 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -198,6 +198,13 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object 
*gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_mmap_offset);

+static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
+{
+   if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
+   return 0;
+   return gbo->bo.mem.start;
+}
+
 /**
  * drm_gem_vram_offset() - \
Returns a GEM VRAM object's offset in video memory
@@ -214,7 +221,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
 {
if (WARN_ON_ONCE(!gbo->pin_count))
return (s64)-ENODEV;
-   return gbo->bo.offset;
+   return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
 }
 EXPORT_SYMBOL(drm_gem_vram_offset);

--
2.25.0

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

Re: [PATCH 2/3] drm/dp_mst: Don't show connectors as connected before probing available PBN

[Ville Syrjälä]

On Wed, Mar 04, 2020 at 05:36:12PM -0500, Lyude Paul wrote:
> It's next to impossible for us to do connector probing on topologies
> without occasionally racing with userspace, since creating a connector
> itself causes a hotplug event which we have to send before probing the
> available PBN of a connector. Even if we didn't have this hotplug event
> sent, there's still always a chance that userspace started probing
> connectors before we finished probing the topology.
> 
> This can be a problem when validating a new MST state since the
> connector will be shown as connected briefly, but without any available
> PBN - causing any atomic state which would enable said connector to fail
> with -ENOSPC. So, let's simply workaround this by telling userspace new
> MST connectors are disconnected until we've finished probing their PBN.
> Since we always send a hotplug event at the end of the link address
> probing process, userspace will still know to reprobe the connector when
> we're ready.
> 
> Signed-off-by: Lyude Paul 
> Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST 
> atomic check")
> Cc: Mikita Lipski 
> Cc: Alex Deucher 
> Cc: Sean Paul 
> Cc: Hans de Goede 
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 207eef08d12c..7b0ff0cff954 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4033,6 +4033,19 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
>   ret = connector_status_connected;
>   break;
>   }
> +
> + /* We don't want to tell userspace the port is actually plugged into
> +  * anything until we've finished probing it's available_pbn, otherwise

"its"

Why is the connector even registered before we've finished the probe?

> +  * userspace will see racy atomic check failures
> +  *
> +  * Since we always send a hotplug at the end of probing topology
> +  * state, we can just let userspace reprobe this connector later.
> +  */
> + if (ret == connector_status_connected && !port->available_pbn) {
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not ready yet (PBN not 
> probed)\n",
> +   connector->base.id, connector->name);
> + ret = connector_status_disconnected;
> + }
>  out:
>   drm_dp_mst_topology_put_port(port);
>   return ret;
> -- 
> 2.24.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update page retirement sequence

[Chen, Guchun]

[AMD Public Use]

We'd better keep original comment "/* skip error address process if -ENOMEM 
*/", if err_addr is not allocated successfully.

Regards,
Guchun

From: Zhang, Hawking 
Sent: Thursday, March 5, 2020 7:23 PM
To: Clements, John ; amd-gfx@lists.freedesktop.org; Li, 
Dennis ; Zhou1, Tao ; Chen, Guchun 

Subject: RE: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

I see. So it's the following programming that is in risk to corrupt data for 
other instances.

/* clear umc status */
WREG64_PCIE((mc_umc_status_addr + umc_reg_offset) * 4, 0x0ULL);

For error injection, everytime it should just have one instance had the error 
status record. Therefore, it make sense to me that we only clear the status 
register once. As discussed, we shall also follow up with umc team on the 
potential issue with index mode programming.

Please also add some comments in code for this unexpected behavior that we 
shall follow up. Other than that, the patch is

Reviewed-by: Hawking Zhang mailto:hawking.zh...@amd.com>>

Regards,
Hawking
From: Clements, John mailto:john.cleme...@amd.com>>
Sent: Thursday, March 5, 2020 18:19
To: Zhang, Hawking mailto:hawking.zh...@amd.com>>; 
amd-gfx@lists.freedesktop.org; Li, Dennis 
mailto:dennis...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

In the original sequence, if the key bits are not set in the mca_status, the 
page retirement will not happen and the status register will be cleared.
If there is a UMC UE, that register will be cleared erroneously 31 times.

If MCA Status == 0 already from the beginning there is no reason to press 
forward with the rest of the checks and clear the register.

From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Thursday, March 5, 2020 5:56 PM
To: Clements, John mailto:john.cleme...@amd.com>>; 
amd-gfx@lists.freedesktop.org; Li, Dennis 
mailto:dennis...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

Hi John,

Can you please explain more on the differences between (a). exit immediately 
when mca_status is 0 and (b). exit when some of critical field in mca_status is 
0?

Regards,
Hawking
From: Clements, John mailto:john.cleme...@amd.com>>
Sent: Thursday, March 5, 2020 17:40
To: amd-gfx@lists.freedesktop.org; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>; Li, Dennis 
mailto:dennis...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

check UMC status and exit prior to making and erroneus register access
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid

[Christian König]

Am 05.03.20 um 13:08 schrieb Jacob He:

SPM access the video memory according to SPM_VMID. It should be updated
with the job's vmid right before the job is scheduled. SPM_VMID is a
global resource

Change-Id: Id3881908960398f87e7c95026a54ff83ff826700
Signed-off-by: Jacob He 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 
  1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c00696f3017e..73398831196f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1080,8 +1080,12 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
amdgpu_job *job,
struct dma_fence *fence = NULL;
bool pasid_mapping_needed = false;
unsigned patch_offset = 0;
+   bool update_spm_vmid_needed = (job->vm && 
(job->vm->reserved_vmid[vmhub] != NULL));
int r;
  
+	if (update_spm_vmid_needed && adev->gfx.rlc.funcs->update_spm_vmid)

+   adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid);
+
if (amdgpu_vmid_had_gpu_reset(adev, id)) {
gds_switch_needed = true;
vm_flush_needed = true;
@@ -3213,6 +3217,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   long timeout = msecs_to_jiffies(2000);
int r;
  
  	switch (args->in.op) {

@@ -3224,6 +3229,21 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
return r;
break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
+   if (amdgpu_sriov_runtime(adev))
+   timeout = 8 * timeout;
+
+   /* Wait vm idle to make sure the vmid set in SPM_VMID is
+* not referenced anymore.
+*/
+   r = amdgpu_bo_reserve(fpriv->vm.root.base.bo, true);
+   if (r)
+   return r;
+
+   r = amdgpu_vm_wait_idle(>vm, timeout);
+   if (r < 0)
+   return r;
+
+   amdgpu_bo_unreserve(fpriv->vm.root.base.bo);
amdgpu_vmid_free_reserved(adev, >vm, AMDGPU_GFXHUB_0);
break;
default:


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

[PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid

[Jacob He]

SPM access the video memory according to SPM_VMID. It should be updated
with the job's vmid right before the job is scheduled. SPM_VMID is a
global resource

Change-Id: Id3881908960398f87e7c95026a54ff83ff826700
Signed-off-by: Jacob He 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c00696f3017e..73398831196f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1080,8 +1080,12 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
amdgpu_job *job,
struct dma_fence *fence = NULL;
bool pasid_mapping_needed = false;
unsigned patch_offset = 0;
+   bool update_spm_vmid_needed = (job->vm && 
(job->vm->reserved_vmid[vmhub] != NULL));
int r;
 
+   if (update_spm_vmid_needed && adev->gfx.rlc.funcs->update_spm_vmid)
+   adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid);
+
if (amdgpu_vmid_had_gpu_reset(adev, id)) {
gds_switch_needed = true;
vm_flush_needed = true;
@@ -3213,6 +3217,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   long timeout = msecs_to_jiffies(2000);
int r;
 
switch (args->in.op) {
@@ -3224,6 +3229,21 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
return r;
break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
+   if (amdgpu_sriov_runtime(adev))
+   timeout = 8 * timeout;
+
+   /* Wait vm idle to make sure the vmid set in SPM_VMID is
+* not referenced anymore.
+*/
+   r = amdgpu_bo_reserve(fpriv->vm.root.base.bo, true);
+   if (r)
+   return r;
+
+   r = amdgpu_vm_wait_idle(>vm, timeout);
+   if (r < 0)
+   return r;
+
+   amdgpu_bo_unreserve(fpriv->vm.root.base.bo);
amdgpu_vmid_free_reserved(adev, >vm, AMDGPU_GFXHUB_0);
break;
default:
-- 
2.17.1

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

RE: [PATCH] drm/amdgpu: update page retirement sequence

[Zhou1, Tao]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Tao Zhou mailto:tao.zh...@amd.com>>

From: Clements, John 
Sent: 2020年3月5日 17:40
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking ; Li, 
Dennis ; Zhou1, Tao ; Chen, Guchun 

Subject: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

check UMC status and exit prior to making and erroneus register access
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Use better names to reflect it is CP MQD buffer

[Christian König]

Am 04.03.20 um 21:35 schrieb Yong Zhao:

Add "CP" to AMDGPU_GEM_CREATE_MQD_GFX9 to indicate it is only for CP MQD.


You should probably note that we can do this because it was always 
illegal for userspace to use the flag.



Change-Id: Ie69cd3ba477e4bac161ea5b20ec2919a35f3528e
Signed-off-by: Yong Zhao 


Acked-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 6 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 7 +--
  include/uapi/drm/amdgpu_drm.h  | 2 +-
  3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index bc2e72a66db9..726c91ab6761 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -224,7 +224,7 @@ void amdgpu_amdkfd_gpu_reset(struct kgd_dev *kgd)
  
  int amdgpu_amdkfd_alloc_gtt_mem(struct kgd_dev *kgd, size_t size,

void **mem_obj, uint64_t *gpu_addr,
-   void **cpu_ptr, bool mqd_gfx9)
+   void **cpu_ptr, bool cp_mqd_gfx9)
  {
struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
struct amdgpu_bo *bo = NULL;
@@ -240,8 +240,8 @@ int amdgpu_amdkfd_alloc_gtt_mem(struct kgd_dev *kgd, size_t 
size,
bp.type = ttm_bo_type_kernel;
bp.resv = NULL;
  
-	if (mqd_gfx9)

-   bp.flags |= AMDGPU_GEM_CREATE_MQD_GFX9;
+   if (cp_mqd_gfx9)
+   bp.flags |= AMDGPU_GEM_CREATE_CP_MQD_GFX9;
  
  	r = amdgpu_bo_create(adev, , );

if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fca87bafe174..665db2353a78 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1043,7 +1043,7 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (amdgpu_bo_encrypted(abo))
flags |= AMDGPU_PTE_TMZ;
  
-	if (abo->flags & AMDGPU_GEM_CREATE_MQD_GFX9) {

+   if (abo->flags & AMDGPU_GEM_CREATE_CP_MQD_GFX9) {
uint64_t page_idx = 1;
  
  		r = amdgpu_gart_bind(adev, gtt->offset, page_idx,

@@ -1051,7 +1051,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
  
-		/* Patch mtype of the second part BO */

+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
  
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h

index eaf94a421901..1e59c0146531 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -130,7 +130,7 @@ extern "C" {
  /* Flag that indicates allocating MQD gart on GFX9, where the mtype
   * for the second page onward should be set to NC.
   */
-#define AMDGPU_GEM_CREATE_MQD_GFX9 (1 << 8)
+#define AMDGPU_GEM_CREATE_CP_MQD_GFX9  (1 << 8)
  /* Flag that BO may contain sensitive data that must be wiped before
   * releasing the memory
   */


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

Re: [PATCH] drm/amdkfd: Add more comments on GFX9 user CP queue MQD workaround

[Christian König]

Am 04.03.20 um 20:40 schrieb Yong Zhao:

Because too many things are involved in this workaround, we need more
comments to avoid pitfalls.

Change-Id: I5d7917296dd5f5edb45921118cf8e7d778d40de1
Signed-off-by: Yong Zhao 


Acked-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  5 -
  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 18 +++---
  2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1947a326de57..10f6f4b21b44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1041,7 +1041,10 @@ int amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
if (r)
goto gart_bind_fail;
  
-		/* Patch mtype of the second part BO */

+   /* The memory type of the first page defaults to UC. Now
+* modify the memory type to NC from the second page of
+* the BO onward.
+*/
flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
flags |= AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC);
  
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c

index 436b7f518979..5b11190ff6e6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -87,9 +87,21 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
int retval;
struct kfd_mem_obj *mqd_mem_obj = NULL;
  
-	/* From V9,  for CWSR, the control stack is located on the next page

-* boundary after the mqd, we will use the gtt allocation function
-* instead of sub-allocation function.
+   /* For V9 only, due to a HW bug, the control stack of a user mode
+* compute queue needs to be allocated just behind the page boundary
+* of its regular MQD buffer. So we allocate an enlarged MQD buffer:
+* the first page of the buffer serves as the regular MQD buffer
+* purpose and the remaining is for control stack. Although the two
+* parts are in the same buffer object, they need different memory
+* types: MQD part needs UC (uncached) as usual, while control stack
+* needs NC (non coherent), which is different from the UC type which
+* is used when control stack is allocated in user space.
+*
+* Because of all those, we use the gtt allocation function instead
+* of sub-allocation function for this enlarged MQD buffer. Moreover,
+* in order to achieve two memory types in a single buffer object, we
+* pass a special bo flag AMDGPU_GEM_CREATE_MQD_GFX9 to instruct
+* amdgpu memory functions to do so.
 */
if (kfd->cwsr_enabled && (q->type == KFD_QUEUE_TYPE_COMPUTE)) {
mqd_mem_obj = kzalloc(sizeof(struct kfd_mem_obj), GFP_KERNEL);


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

Re: [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start

[Christian König]

Am 05.03.20 um 12:25 schrieb Christian König:

Am 04.03.20 um 17:34 schrieb James Zhu:

Fix race condition issue when multiple vcn starts are called.

v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.

Signed-off-by: James Zhu 


Reviewed-by: Christian König 


One thing worth noting is that in theory you could run into the issue 
that one ring restarts the timer while another ring is still preparing 
the engine for usage.


So the timeout should be large enough to guarantee that this never 
causes problems.


Regards,
Christian.




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
  2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c

index f96464e..8a8406b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
  int i, r;
    INIT_DELAYED_WORK(>vcn.idle_work, 
amdgpu_vcn_idle_work_handler);

+    mutex_init(>vcn.vcn_pg_lock);
    switch (adev->asic_type) {
  case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
  }
    release_firmware(adev->vcn.fw);
+    mutex_destroy(>vcn.vcn_pg_lock);
    return 0;
  }
@@ -319,13 +321,13 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)

  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
  {
  struct amdgpu_device *adev = ring->adev;
-    bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
  -    if (set_clocks) {
-    amdgpu_gfx_off_ctrl(adev, false);
-    amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,

-   AMD_PG_STATE_UNGATE);
-    }
+    cancel_delayed_work_sync(>vcn.idle_work);
+
+    mutex_lock(>vcn.vcn_pg_lock);
+    amdgpu_gfx_off_ctrl(adev, false);
+    amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+   AMD_PG_STATE_UNGATE);
    if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)    {
  struct dpg_pause_state new_state;
@@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring 
*ring)

    adev->vcn.pause_dpg_mode(adev, ring->me, _state);
  }
+    mutex_unlock(>vcn.vcn_pg_lock);
  }
    void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
  struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
  uint32_t num_vcn_enc_sched;
  uint32_t num_vcn_dec_sched;
+    struct mutex vcn_pg_lock;
    unsigned    harvest_config;
  int (*pause_dpg_mode)(struct amdgpu_device *adev,




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

Re: [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start

[Christian König]

Am 04.03.20 um 17:34 schrieb James Zhu:

Fix race condition issue when multiple vcn starts are called.

v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.

Signed-off-by: James Zhu 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
  2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..8a8406b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
int i, r;
  
  	INIT_DELAYED_WORK(>vcn.idle_work, amdgpu_vcn_idle_work_handler);

+   mutex_init(>vcn.vcn_pg_lock);
  
  	switch (adev->asic_type) {

case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
}
  
  	release_firmware(adev->vcn.fw);

+   mutex_destroy(>vcn.vcn_pg_lock);
  
  	return 0;

  }
@@ -319,13 +321,13 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)
  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
  {
struct amdgpu_device *adev = ring->adev;
-   bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
  
-	if (set_clocks) {

-   amdgpu_gfx_off_ctrl(adev, false);
-   amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
-  AMD_PG_STATE_UNGATE);
-   }
+   cancel_delayed_work_sync(>vcn.idle_work);
+
+   mutex_lock(>vcn.vcn_pg_lock);
+   amdgpu_gfx_off_ctrl(adev, false);
+   amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+  AMD_PG_STATE_UNGATE);
  
  	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)	{

struct dpg_pause_state new_state;
@@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
  
  		adev->vcn.pause_dpg_mode(adev, ring->me, _state);

}
+   mutex_unlock(>vcn.vcn_pg_lock);
  }
  
  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
uint32_t num_vcn_enc_sched;
uint32_t num_vcn_dec_sched;
+   struct mutex vcn_pg_lock;
  
  	unsigned	harvest_config;

int (*pause_dpg_mode)(struct amdgpu_device *adev,


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

RE: [PATCH] drm/amdgpu: update page retirement sequence

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

I see. So it's the following programming that is in risk to corrupt data for 
other instances.

/* clear umc status */
WREG64_PCIE((mc_umc_status_addr + umc_reg_offset) * 4, 0x0ULL);

For error injection, everytime it should just have one instance had the error 
status record. Therefore, it make sense to me that we only clear the status 
register once. As discussed, we shall also follow up with umc team on the 
potential issue with index mode programming.

Please also add some comments in code for this unexpected behavior that we 
shall follow up. Other than that, the patch is

Reviewed-by: Hawking Zhang 

Regards,
Hawking
From: Clements, John 
Sent: Thursday, March 5, 2020 18:19
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org; Li, 
Dennis ; Zhou1, Tao ; Chen, Guchun 

Subject: RE: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

In the original sequence, if the key bits are not set in the mca_status, the 
page retirement will not happen and the status register will be cleared.
If there is a UMC UE, that register will be cleared erroneously 31 times.

If MCA Status == 0 already from the beginning there is no reason to press 
forward with the rest of the checks and clear the register.

From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Thursday, March 5, 2020 5:56 PM
To: Clements, John mailto:john.cleme...@amd.com>>; 
amd-gfx@lists.freedesktop.org; Li, Dennis 
mailto:dennis...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

Hi John,

Can you please explain more on the differences between (a). exit immediately 
when mca_status is 0 and (b). exit when some of critical field in mca_status is 
0?

Regards,
Hawking
From: Clements, John mailto:john.cleme...@amd.com>>
Sent: Thursday, March 5, 2020 17:40
To: amd-gfx@lists.freedesktop.org; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>; Li, Dennis 
mailto:dennis...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

check UMC status and exit prior to making and erroneus register access
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid

[Christian König]

Am 04.03.20 um 05:06 schrieb Jacob He:

SPM access the video memory according to SPM_VMID. It should be updated
with the job's vmid right before the job is scheduled. SPM_VMID is a
global resource

Change-Id: Id3881908960398f87e7c95026a54ff83ff826700
Signed-off-by: Jacob He 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c00696f3017e..f08effb033a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1080,8 +1080,12 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
amdgpu_job *job,
struct dma_fence *fence = NULL;
bool pasid_mapping_needed = false;
unsigned patch_offset = 0;
+   bool update_spm_vmid_needed = (job->vm && 
(job->vm->reserved_vmid[vmhub] != NULL));
int r;
  
+	if (update_spm_vmid_needed && adev->gfx.rlc.funcs->update_spm_vmid)

+   adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid);
+
if (amdgpu_vmid_had_gpu_reset(adev, id)) {
gds_switch_needed = true;
vm_flush_needed = true;
@@ -3213,6 +3217,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   long timeout = msecs_to_jiffies(2000);
int r;
  
  	switch (args->in.op) {

@@ -3224,6 +3229,15 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
return r;
break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
+   if (amdgpu_sriov_runtime(adev))
+   timeout = 8 * timeout;
+
+   /* Wait vm idle to make sure the vmid set in SPM_VMID is
+* not referenced anymore.
+*/
+   r = amdgpu_vm_wait_idle(>vm, timeout);
+   if (r < 0)
+   return r;


You also need to lock/unlock the whole VM around that. Otherwise 
amdgpu_vm_wait_idle() could crash.


E.g. call amdgpu_bo_reserve() on vm->root.base.bo and after you are done 
amdgpu_bo_unreserver().


Apart from that it looks good to me,
Christian.


amdgpu_vmid_free_reserved(adev, >vm, AMDGPU_GFXHUB_0);
break;
default:


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

RE: [PATCH] drm/amdgpu: update page retirement sequence

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

In the original sequence, if the key bits are not set in the mca_status, the 
page retirement will not happen and the status register will be cleared.
If there is a UMC UE, that register will be cleared erroneously 31 times.

If MCA Status == 0 already from the beginning there is no reason to press 
forward with the rest of the checks and clear the register.

From: Zhang, Hawking 
Sent: Thursday, March 5, 2020 5:56 PM
To: Clements, John ; amd-gfx@lists.freedesktop.org; Li, 
Dennis ; Zhou1, Tao ; Chen, Guchun 

Subject: RE: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

Hi John,

Can you please explain more on the differences between (a). exit immediately 
when mca_status is 0 and (b). exit when some of critical field in mca_status is 
0?

Regards,
Hawking
From: Clements, John mailto:john.cleme...@amd.com>>
Sent: Thursday, March 5, 2020 17:40
To: amd-gfx@lists.freedesktop.org; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>; Li, Dennis 
mailto:dennis...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

check UMC status and exit prior to making and erroneus register access
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update page retirement sequence

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Hi John,

Can you please explain more on the differences between (a). exit immediately 
when mca_status is 0 and (b). exit when some of critical field in mca_status is 
0?

Regards,
Hawking
From: Clements, John 
Sent: Thursday, March 5, 2020 17:40
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking ; Li, 
Dennis ; Zhou1, Tao ; Chen, Guchun 

Subject: [PATCH] drm/amdgpu: update page retirement sequence


[AMD Official Use Only - Internal Distribution Only]

check UMC status and exit prior to making and erroneus register access
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: enable vcn 2.0 for SRIOV on NV12

[Monk Liu]

this a patch that port from SRIOV project branch
to fix those IB/RING test fail on VCN 2.0 rings

Signed-off-by: Darlington Opara 
Signed-off-by: Jiange Zhao 
Signed-off-by: Monk Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c|  14 +-
 drivers/gpu/drm/amd/amdgpu/mmsch_v2_0.h| 141 ++
 drivers/gpu/drm/amd/amdgpu/nv.c|   3 +-
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c  | 303 -
 .../amd/include/asic_reg/vcn/vcn_2_0_0_offset.h| 197 ++
 6 files changed, 637 insertions(+), 24 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/mmsch_v2_0.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
index 5727f00a..0120130 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
@@ -181,6 +181,9 @@ int amdgpu_jpeg_dec_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
struct dma_fence *fence = NULL;
long r = 0;
 
+   if (amdgpu_sriov_vf(adev))
+   return 0;
+
r = amdgpu_jpeg_dec_set_reg(ring, 1, );
if (r)
goto error;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..ca7c9a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -359,6 +359,9 @@ int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
unsigned i;
int r;
 
+   if (amdgpu_sriov_vf(adev))
+   return 0;
+
WREG32(adev->vcn.inst[ring->me].external.scratch9, 0xCAFEDEAD);
r = amdgpu_ring_alloc(ring, 3);
if (r)
@@ -497,10 +500,6 @@ int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
struct dma_fence *fence;
long r;
 
-   /* temporarily disable ib test for sriov */
-   if (amdgpu_sriov_vf(adev))
-   return 0;
-
r = amdgpu_vcn_dec_get_create_msg(ring, 1, NULL);
if (r)
goto error;
@@ -527,6 +526,9 @@ int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring *ring)
unsigned i;
int r;
 
+   if (amdgpu_sriov_vf(adev))
+   return 0;
+
r = amdgpu_ring_alloc(ring, 16);
if (r)
return r;
@@ -661,10 +663,6 @@ int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
struct amdgpu_bo *bo = NULL;
long r;
 
-   /* temporarily disable ib test for sriov */
-   if (amdgpu_sriov_vf(adev))
-   return 0;
-
r = amdgpu_bo_create_reserved(ring->adev, 128 * 1024, PAGE_SIZE,
  AMDGPU_GEM_DOMAIN_VRAM,
  , NULL, NULL);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmsch_v2_0.h 
b/drivers/gpu/drm/amd/amdgpu/mmsch_v2_0.h
new file mode 100644
index 000..ad99e92
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/mmsch_v2_0.h
@@ -0,0 +1,141 @@
+/*
+ * Copyright 2019 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __MMSCH_V2_0_H__
+#define __MMSCH_V2_0_H__
+
+#define MMSCH_VERSION_MAJOR2
+#define MMSCH_VERSION_MINOR0
+#define MMSCH_VERSION  (MMSCH_VERSION_MAJOR << 16 | MMSCH_VERSION_MINOR)
+
+enum mmsch_v2_0_command_type {
+   MMSCH_COMMAND__DIRECT_REG_WRITE = 0,
+   MMSCH_COMMAND__DIRECT_REG_POLLING = 2,
+   MMSCH_COMMAND__DIRECT_REG_READ_MODIFY_WRITE = 3,
+   MMSCH_COMMAND__INDIRECT_REG_WRITE = 8,
+   MMSCH_COMMAND__END = 0xf
+};
+
+struct mmsch_v2_0_init_header {
+   uint32_t version;
+   uint32_t header_size;
+   uint32_t vcn_init_status;
+   uint32_t vcn_table_offset;
+   uint32_t vcn_table_size;
+};
+
+struct mmsch_v2_0_cmd_direct_reg_header {
+   uint32_t reg_offset   : 28;
+   uint32_t command_type : 4;
+};

RE: [PATCH] drm/amdgpu: toggle DF-Cstate when accessing UMC ras error related registers

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: John Clements 

-Original Message-
From: Chen, Guchun  
Sent: Wednesday, March 4, 2020 10:47 PM
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking ; Li, 
Dennis ; Zhou1, Tao ; Clements, John 

Cc: Chen, Guchun 
Subject: [PATCH] drm/amdgpu: toggle DF-Cstate when accessing UMC ras error 
related registers

On arcturus, DF-Cstate needs to be toggled off/on before and after accessing 
UMC error counter and error address registers, otherwise, clearing such 
registers may fail.

Signed-off-by: Guchun Chen 
---
 drivers/gpu/drm/amd/amdgpu/umc_v6_1.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
index e6866f038209..77122a7282e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
@@ -186,6 +186,10 @@ static void umc_v6_1_query_ras_error_count(struct 
amdgpu_device *adev,
if (rsmu_umc_index_state)
umc_v6_1_disable_umc_index_mode(adev);
 
+   if ((adev->asic_type == CHIP_ARCTURUS) &&
+   amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_DISALLOW))
+   DRM_WARN("Fail to disable DF-Cstate.\n");
+
LOOP_UMC_INST_AND_CH(umc_inst, ch_inst) {
umc_reg_offset = get_umc_6_reg_offset(adev,
  umc_inst,
@@ -199,6 +203,10 @@ static void umc_v6_1_query_ras_error_count(struct 
amdgpu_device *adev,
  
&(err_data->ue_count));
}
 
+   if ((adev->asic_type == CHIP_ARCTURUS) &&
+   amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW))
+   DRM_WARN("Fail to enable DF-Cstate\n");
+
if (rsmu_umc_index_state)
umc_v6_1_enable_umc_index_mode(adev);
 }
@@ -288,6 +296,10 @@ static void umc_v6_1_query_ras_error_address(struct 
amdgpu_device *adev,
if (rsmu_umc_index_state)
umc_v6_1_disable_umc_index_mode(adev);
 
+   if ((adev->asic_type == CHIP_ARCTURUS) &&
+   amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_DISALLOW))
+   DRM_WARN("Fail to disable DF-Cstate.\n");
+
LOOP_UMC_INST_AND_CH(umc_inst, ch_inst) {
umc_reg_offset = get_umc_6_reg_offset(adev,
  umc_inst,
@@ -300,6 +312,10 @@ static void umc_v6_1_query_ras_error_address(struct 
amdgpu_device *adev,
 umc_inst);
}
 
+   if ((adev->asic_type == CHIP_ARCTURUS) &&
+   amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW))
+   DRM_WARN("Fail to enable DF-Cstate\n");
+
if (rsmu_umc_index_state)
umc_v6_1_enable_umc_index_mode(adev);
 }
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: increase atombios cmd timeout

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
From: Clements, John 
Sent: Thursday, March 5, 2020 17:39
To: amd-gfx@lists.freedesktop.org; Zhang, Hawking ; Li, 
Dennis ; Zhou1, Tao ; Chen, Guchun 

Subject: [PATCH] drm/amdgpu: increase atombios cmd timeout


[AMD Official Use Only - Internal Distribution Only]

mitigates race condition on BACO reset between GPU bootcode and driver reload
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: update page retirement sequence

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

check UMC status and exit prior to making and erroneus register access


0001-drm-amdgpu-update-page-retirement-sequence.patch
Description: 0001-drm-amdgpu-update-page-retirement-sequence.patch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: increase atombios cmd timeout

[Clements, John]

[AMD Official Use Only - Internal Distribution Only]

mitigates race condition on BACO reset between GPU bootcode and driver reload


0001-drm-amdgpu-increase-atombios-cmd-timeout.patch
Description: 0001-drm-amdgpu-increase-atombios-cmd-timeout.patch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/4] drm/scheduler: implement a function to modify sched list

[Nirmoy]

On 3/4/20 11:00 PM, Luben Tuikov wrote:

struct drm_sched_entity *entity,

   void *owner);
+void drm_sched_entity_modify_sched(struct drm_sched_entity *entity,
+ struct drm_gpu_scheduler **sched_list,
+  unsigned int num_sched_list);
+

Again, the argument list here is unaligned. Please align it
looks good with cat and vim. Not sure why git format-patch is acting 
strange in this case



Nirmoy

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