RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

2019-11-01 Thread Wu, Hersen

Reviewed-by: Hersen Wu 



-Original Message-
From: Liu, Zhan  
Sent: Friday, November 1, 2019 9:35 PM
To: Wu, Hersen ; amd-gfx@lists.freedesktop.org; 
Kazlauskas, Nicholas ; Lakha, Bhawanpreet 
; Li, Roman ; Siqueira, Rodrigo 
; Wentland, Harry ; Zuo, 
Jerry 
Cc: Yeh, Eagle ; Lazare, Jordan 
Subject: RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for 
Navi14

Thank you Hersen. Please check the updated patch:

From: Liu, Zhan 
Sent: Friday, November 1, 2019 9:18 PM
To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas 
; Lakha, Bhawanpreet ; 
Li, Roman ; Liu, Zhan ; Siqueira, Rodrigo 
; Wentland, Harry ; Wu, 
Hersen ; Zuo, Jerry 
Cc: Yeh, Eagle ; Lazare, Jordan 
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

From: Zhan liu 
Date: Fri, 1 Nov 2019 21:10:17 -0400
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

[Why]
Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is no 
ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related issues (e.g. 
HDMI S3 resume failure, HDMI pink screen on boot) will be observed.

[How]
If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.

Signed-off-by: Zhan liu 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 924c2e303588..cf886483e380 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -1152,6 +1152,11 @@ struct stream_encoder *dcn20_stream_encoder_create(
if (!enc1)
return NULL;

+   if (ASIC_REV_IS_NAVI14_M(ctx->asic_id.hw_internal_rev)) {
+   if (eng_id >= ENGINE_ID_DIGD)
+   eng_id++;
+   }
+
dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
&stream_enc_regs[eng_id],
&se_shift, &se_mask);
--
2.21.0

> -Original Message-
> From: Wu, Hersen 
> Sent: 2019/November/01, Friday 9:23 PM
> To: Liu, Zhan ; amd-gfx@lists.freedesktop.org; 
> Kazlauskas, Nicholas ; Lakha, Bhawanpreet 
> ; Li, Roman ; Siqueira, 
> Rodrigo ; Wentland, Harry 
> ; Zuo, Jerry 
> Cc: Yeh, Eagle ; Lazare, Jordan 
> 
> Subject: RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition 
> check for Navi14
> 
> Hi Zhan,
> 
> The function is shared by NV10,12,14.
> 
> Please add ASIC ID check  for the DIG D skip.
> 
> Thanks!
> Hersen
> 
> 
> -Original Message-
> From: Liu, Zhan 
> Sent: Friday, November 1, 2019 9:18 PM
> To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas 
> ; Lakha, Bhawanpreet 
> ; Li, Roman ; Liu, Zhan 
> ; Siqueira, Rodrigo ; 
> Wentland, Harry ; Wu, Hersen 
> ; Zuo, Jerry 
> Cc: Yeh, Eagle ; Lazare, Jordan 
> 
> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check 
> for Navi14
> 
> From: Zhan liu 
> Date: Fri, 1 Nov 2019 21:10:17 -0400
> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check 
> for Navi14
> 
> [Why]
> Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is 
> no ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related 
> issues (e.g. HDMI S3 resume failure, HDMI pink screen on boot) will be 
> observed.
> 
> [How]
> If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.
> 
> Signed-off-by: Zhan liu 
> ---
>  drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index 924c2e303588..cf886483e380 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -1152,6 +1152,9 @@ struct stream_encoder 
> *dcn20_stream_encoder_create(
> if (!enc1)
> return NULL;
> 
> +   if (eng_id >= ENGINE_ID_DIGD)
> +   eng_id++;
> +
> dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
> &stream_enc_regs[eng_id],
> &se_shift, &se_mask);
> --
> 2.21.0
> 
> ___
> 
> amd-gfx mailing list
> 
> amd-gfx@lists.freedesktop.org
> 
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

2019-11-01 Thread Liu, Zhan
Thank you Hersen. Please check the updated patch:

From: Liu, Zhan  
Sent: Friday, November 1, 2019 9:18 PM
To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas 
; Lakha, Bhawanpreet ; 
Li, Roman ; Liu, Zhan ; Siqueira, Rodrigo 
; Wentland, Harry ; Wu, 
Hersen ; Zuo, Jerry 
Cc: Yeh, Eagle ; Lazare, Jordan 
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

From: Zhan liu 
Date: Fri, 1 Nov 2019 21:10:17 -0400
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

[Why]
Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is no 
ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related issues (e.g. 
HDMI S3 resume failure, HDMI pink screen on boot) will be observed.

[How]
If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.

Signed-off-by: Zhan liu 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 924c2e303588..cf886483e380 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -1152,6 +1152,11 @@ struct stream_encoder *dcn20_stream_encoder_create(
if (!enc1)
return NULL;

+   if (ASIC_REV_IS_NAVI14_M(ctx->asic_id.hw_internal_rev)) {
+   if (eng_id >= ENGINE_ID_DIGD)
+   eng_id++;
+   }
+
dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
&stream_enc_regs[eng_id],
&se_shift, &se_mask);
--
2.21.0

> -Original Message-
> From: Wu, Hersen 
> Sent: 2019/November/01, Friday 9:23 PM
> To: Liu, Zhan ; amd-gfx@lists.freedesktop.org;
> Kazlauskas, Nicholas ; Lakha, Bhawanpreet
> ; Li, Roman ;
> Siqueira, Rodrigo ; Wentland, Harry
> ; Zuo, Jerry 
> Cc: Yeh, Eagle ; Lazare, Jordan
> 
> Subject: RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition
> check for Navi14
> 
> Hi Zhan,
> 
> The function is shared by NV10,12,14.
> 
> Please add ASIC ID check  for the DIG D skip.
> 
> Thanks!
> Hersen
> 
> 
> -Original Message-
> From: Liu, Zhan 
> Sent: Friday, November 1, 2019 9:18 PM
> To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas
> ; Lakha, Bhawanpreet
> ; Li, Roman ; Liu,
> Zhan ; Siqueira, Rodrigo ;
> Wentland, Harry ; Wu, Hersen
> ; Zuo, Jerry 
> Cc: Yeh, Eagle ; Lazare, Jordan
> 
> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check
> for Navi14
> 
> From: Zhan liu 
> Date: Fri, 1 Nov 2019 21:10:17 -0400
> Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check
> for Navi14
> 
> [Why]
> Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is no
> ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related issues
> (e.g. HDMI S3 resume failure, HDMI pink screen on boot) will be observed.
> 
> [How]
> If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.
> 
> Signed-off-by: Zhan liu 
> ---
>  drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index 924c2e303588..cf886483e380 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -1152,6 +1152,9 @@ struct stream_encoder
> *dcn20_stream_encoder_create(
> if (!enc1)
> return NULL;
> 
> +   if (eng_id >= ENGINE_ID_DIGD)
> +   eng_id++;
> +
> dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
> &stream_enc_regs[eng_id],
> &se_shift, &se_mask);
> --
> 2.21.0
> 
> ___
> 
> amd-gfx mailing list
> 
> amd-gfx@lists.freedesktop.org
> 
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

2019-11-01 Thread Wu, Hersen
Hi Zhan,

The function is shared by NV10,12,14.

Please add ASIC ID check  for the DIG D skip. 

Thanks!
Hersen


-Original Message-
From: Liu, Zhan  
Sent: Friday, November 1, 2019 9:18 PM
To: amd-gfx@lists.freedesktop.org; Kazlauskas, Nicholas 
; Lakha, Bhawanpreet ; 
Li, Roman ; Liu, Zhan ; Siqueira, Rodrigo 
; Wentland, Harry ; Wu, 
Hersen ; Zuo, Jerry 
Cc: Yeh, Eagle ; Lazare, Jordan 
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

From: Zhan liu 
Date: Fri, 1 Nov 2019 21:10:17 -0400
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

[Why]
Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is because there is no 
ENGINE_ID_DIGD in Navi14. Without this patch, many HDMI related issues (e.g. 
HDMI S3 resume failure, HDMI pink screen on boot) will be observed.

[How]
If eng_id is larger than ENGINE_ID_DIGD, then add eng_id by 1.

Signed-off-by: Zhan liu 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 924c2e303588..cf886483e380 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -1152,6 +1152,9 @@ struct stream_encoder *dcn20_stream_encoder_create(
if (!enc1)
return NULL;

+   if (eng_id >= ENGINE_ID_DIGD)
+   eng_id++;
+
dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
&stream_enc_regs[eng_id],
&se_shift, &se_mask);
--
2.21.0

___

amd-gfx mailing list

amd-gfx@lists.freedesktop.org

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

[PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

2019-11-01 Thread Liu, Zhan
From: Zhan liu 
Date: Fri, 1 Nov 2019 21:10:17 -0400
Subject: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14

[Why]
Navi10 has 6 PHY, but Navi14 only has 5 PHY, that is
because there is no ENGINE_ID_DIGD in Navi14. Without
this patch, many HDMI related issues (e.g. HDMI S3
resume failure, HDMI pink screen on boot) will be
observed.

[How]
If eng_id is larger than ENGINE_ID_DIGD, then
add eng_id by 1.

Signed-off-by: Zhan liu 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 924c2e303588..cf886483e380 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -1152,6 +1152,9 @@ struct stream_encoder *dcn20_stream_encoder_create(
if (!enc1)
return NULL;

+   if (eng_id >= ENGINE_ID_DIGD)
+   eng_id++;
+
dcn20_stream_encoder_construct(enc1, ctx, ctx->dc_bios, eng_id,
&stream_enc_regs[eng_id],
&se_shift, &se_mask);
--
2.21.0

___

amd-gfx mailing list

amd-gfx@lists.freedesktop.org

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

Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Zhao, Yong
> + /* only leave the offset segment */
> + vma->vm_pgoff &= (1ULL << (KFD_MMAP_GPU_ID_SHIFT - PAGE_SHIFT)) - 1;

You're now open-coding what used to be done by the
KFD_MMAP_OFFSET_VALUE_GET macro. I don't see how this is an improvement.
Maybe better to update the macro to do this.

I can definitely do that, but I think we'd better delete this line completely 
as it seems odd to change vm_pgoff. Moreover this vm_pgoff is not used at all 
in the following function calls. What do you think?

Regards,
Yong

From: Kuehling, Felix 
Sent: Friday, November 1, 2019 6:48 PM
To: Zhao, Yong ; amd-gfx@lists.freedesktop.org 

Subject: Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

On 2019-11-01 4:48 p.m., Zhao, Yong wrote:
> The new code is much cleaner and results in better readability.
>
> Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
> Signed-off-by: Yong Zhao 
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
>   4 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b91993753b82..590138727ca9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
> struct kfd_process *p,
>/* Return gpu_id as doorbell offset for mmap usage */
>args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
>args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
> - args->doorbell_offset <<= PAGE_SHIFT;
>if (KFD_IS_SOC15(dev->device_info->asic_family))
>/* On SOC15 ASICs, include the doorbell offset within the
> * process doorbell frame, which could be 1 page or 2 pages.
> @@ -1938,20 +1937,22 @@ static int kfd_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>   {
>struct kfd_process *process;
>struct kfd_dev *dev = NULL;
> - unsigned long vm_pgoff;
> + unsigned long mmap_offset;
>unsigned int gpu_id;
>
>process = kfd_get_process(current);
>if (IS_ERR(process))
>return PTR_ERR(process);
>
> - vm_pgoff = vma->vm_pgoff;
> - vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
> - gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
> + mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
> + gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
>if (gpu_id)
>dev = kfd_device_by_id(gpu_id);
>
> - switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
> + /* only leave the offset segment */
> + vma->vm_pgoff &= (1ULL << (KFD_MMAP_GPU_ID_SHIFT - PAGE_SHIFT)) - 1;

You're now open-coding what used to be done by the
KFD_MMAP_OFFSET_VALUE_GET macro. I don't see how this is an improvement.
Maybe better to update the macro to do this.


> +
> + switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
>case KFD_MMAP_TYPE_DOORBELL:
>if (!dev)
>return -ENODEV;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> index 908081c85de1..1f8365575b12 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> @@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
> kfd_process *p,
>ret = create_signal_event(devkfd, p, ev);
>if (!ret) {
>*event_page_offset = KFD_MMAP_TYPE_EVENTS;
> - *event_page_offset <<= PAGE_SHIFT;
>*event_slot_index = ev->event_id;
>}
>break;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 66bae8f2dad1..8eecd2cd1fd2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -59,24 +59,21 @@
>* NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
>*  defines are w.r.t to PAGE_SIZE
>*/
> -#define KFD_MMAP_TYPE_SHIFT  (62 - PAGE_SHIFT)
> +#define KFD_MMAP_TYPE_SHIFT  (62)
>   #define KFD_MMAP_TYPE_MASK  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_DOORBELL  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_EVENTS(0x2ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_RESERVED_MEM  (0x1ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_MMIO  (0x0ULL << KFD_MMAP_TYPE_SHIFT)
>
> -#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
> +#define KFD_MMAP_GPU_ID_SHIFT (46)
>   #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \
><< KFD_MMAP_GPU_ID_SHIFT)
>   #define KFD_MMAP_GPU_ID(gpu_id) uint64_t)gpu_id) << 
> KFD_MMAP_GPU_ID_SHIFT)\
> 

Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Kuehling, Felix
On 2019-11-01 4:48 p.m., Zhao, Yong wrote:
> The new code is much cleaner and results in better readability.
>
> Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
> Signed-off-by: Yong Zhao 
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
>   4 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b91993753b82..590138727ca9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
> struct kfd_process *p,
>   /* Return gpu_id as doorbell offset for mmap usage */
>   args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
>   args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
> - args->doorbell_offset <<= PAGE_SHIFT;
>   if (KFD_IS_SOC15(dev->device_info->asic_family))
>   /* On SOC15 ASICs, include the doorbell offset within the
>* process doorbell frame, which could be 1 page or 2 pages.
> @@ -1938,20 +1937,22 @@ static int kfd_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>   {
>   struct kfd_process *process;
>   struct kfd_dev *dev = NULL;
> - unsigned long vm_pgoff;
> + unsigned long mmap_offset;
>   unsigned int gpu_id;
>   
>   process = kfd_get_process(current);
>   if (IS_ERR(process))
>   return PTR_ERR(process);
>   
> - vm_pgoff = vma->vm_pgoff;
> - vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
> - gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
> + mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
> + gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
>   if (gpu_id)
>   dev = kfd_device_by_id(gpu_id);
>   
> - switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
> + /* only leave the offset segment */
> + vma->vm_pgoff &= (1ULL << (KFD_MMAP_GPU_ID_SHIFT - PAGE_SHIFT)) - 1;

You're now open-coding what used to be done by the 
KFD_MMAP_OFFSET_VALUE_GET macro. I don't see how this is an improvement. 
Maybe better to update the macro to do this.


> +
> + switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
>   case KFD_MMAP_TYPE_DOORBELL:
>   if (!dev)
>   return -ENODEV;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> index 908081c85de1..1f8365575b12 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> @@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
> kfd_process *p,
>   ret = create_signal_event(devkfd, p, ev);
>   if (!ret) {
>   *event_page_offset = KFD_MMAP_TYPE_EVENTS;
> - *event_page_offset <<= PAGE_SHIFT;
>   *event_slot_index = ev->event_id;
>   }
>   break;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 66bae8f2dad1..8eecd2cd1fd2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -59,24 +59,21 @@
>* NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
>*  defines are w.r.t to PAGE_SIZE
>*/
> -#define KFD_MMAP_TYPE_SHIFT  (62 - PAGE_SHIFT)
> +#define KFD_MMAP_TYPE_SHIFT  (62)
>   #define KFD_MMAP_TYPE_MASK  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_DOORBELL  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_EVENTS(0x2ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_RESERVED_MEM  (0x1ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_MMIO  (0x0ULL << KFD_MMAP_TYPE_SHIFT)
>   
> -#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
> +#define KFD_MMAP_GPU_ID_SHIFT (46)
>   #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \
>   << KFD_MMAP_GPU_ID_SHIFT)
>   #define KFD_MMAP_GPU_ID(gpu_id) uint64_t)gpu_id) << 
> KFD_MMAP_GPU_ID_SHIFT)\
>   & KFD_MMAP_GPU_ID_MASK)
> -#define KFD_MMAP_GPU_ID_GET(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
> +#define KFD_MMAP_GET_GPU_ID(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
>   >> KFD_MMAP_GPU_ID_SHIFT)
>   
> -#define KFD_MMAP_OFFSET_VALUE_MASK   (0x3FFFULL >> PAGE_SHIFT)
> -#define KFD_MMAP_OFFSET_VALUE_GET(offset) (offset & 
> KFD_MMAP_OFFSET_VALUE_MASK)

This macro is still useful. See above. I think you should just update 
the mask and continue using it for clarity.

Regards,
   Felix


> -
>   /*
>* When working with cp scheduler we should assign the HIQ manually or via
>* the amdgpu driver to a fixed hqd slot, here are the fixed HIQ hqd slot
> diff --git a/dr

Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Zhao, Yong
Hi Felix,

The PAGE_SHIFT was not deleted but merged into the KFD_*_SHIFT in kfd_priv.h. 
Because of that, this change is actually transparent to the thunk, and it only 
straightens up the bit shift operations in most cases.

Regards,
Yong

From: Kuehling, Felix 
Sent: Friday, November 1, 2019 5:13 PM
To: amd-gfx@lists.freedesktop.org ; Zhao, Yong 

Subject: Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

NAK. This won't work for several reasons.

The mmap_offset is used as offset parameter in the mmap system call. If
you check the man page of mmap, you'll see that "offset must be a
multiple of the page size". Therefore the PAGE_SHIFT is necessary.

In the case of doorbell offsets, the offset is parsed and processed by
the Thunk in user mode. On GFX9 GPUs the lower bits are used for the
offset of the doorbell within the doorbell page. On GFX8 the queue ID
was used, but on GFX9 we had to decoupled the doorbell ID from the queue
ID. If you remove the PAGE_SHIFT, you'll need to put those bits
somewhere else. But that change in the encoding would break the ABI with
the Thunk.

Regards,
   Felix

On 2019-11-01 4:48 p.m., Zhao, Yong wrote:
> The new code is much cleaner and results in better readability.
>
> Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
> Signed-off-by: Yong Zhao 
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
>   4 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b91993753b82..590138727ca9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
> struct kfd_process *p,
>/* Return gpu_id as doorbell offset for mmap usage */
>args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
>args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
> - args->doorbell_offset <<= PAGE_SHIFT;
>if (KFD_IS_SOC15(dev->device_info->asic_family))
>/* On SOC15 ASICs, include the doorbell offset within the
> * process doorbell frame, which could be 1 page or 2 pages.
> @@ -1938,20 +1937,22 @@ static int kfd_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>   {
>struct kfd_process *process;
>struct kfd_dev *dev = NULL;
> - unsigned long vm_pgoff;
> + unsigned long mmap_offset;
>unsigned int gpu_id;
>
>process = kfd_get_process(current);
>if (IS_ERR(process))
>return PTR_ERR(process);
>
> - vm_pgoff = vma->vm_pgoff;
> - vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
> - gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
> + mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
> + gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
>if (gpu_id)
>dev = kfd_device_by_id(gpu_id);
>
> - switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
> + /* only leave the offset segment */
> + vma->vm_pgoff &= (1ULL << (KFD_MMAP_GPU_ID_SHIFT - PAGE_SHIFT)) - 1;
> +
> + switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
>case KFD_MMAP_TYPE_DOORBELL:
>if (!dev)
>return -ENODEV;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> index 908081c85de1..1f8365575b12 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> @@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
> kfd_process *p,
>ret = create_signal_event(devkfd, p, ev);
>if (!ret) {
>*event_page_offset = KFD_MMAP_TYPE_EVENTS;
> - *event_page_offset <<= PAGE_SHIFT;
>*event_slot_index = ev->event_id;
>}
>break;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 66bae8f2dad1..8eecd2cd1fd2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -59,24 +59,21 @@
>* NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
>*  defines are w.r.t to PAGE_SIZE
>*/
> -#define KFD_MMAP_TYPE_SHIFT  (62 - PAGE_SHIFT)
> +#define KFD_MMAP_TYPE_SHIFT  (62)
>   #define KFD_MMAP_TYPE_MASK  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_DOORBELL  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_EVENTS(0x2ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_RESERVED_MEM  (0x1ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_MMIO  (0x0ULL << KFD_MMAP_TYPE_SHIFT)
>
> -#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
> +#defin

Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Kuehling, Felix
NAK. This won't work for several reasons.

The mmap_offset is used as offset parameter in the mmap system call. If 
you check the man page of mmap, you'll see that "offset must be a 
multiple of the page size". Therefore the PAGE_SHIFT is necessary.

In the case of doorbell offsets, the offset is parsed and processed by 
the Thunk in user mode. On GFX9 GPUs the lower bits are used for the 
offset of the doorbell within the doorbell page. On GFX8 the queue ID 
was used, but on GFX9 we had to decoupled the doorbell ID from the queue 
ID. If you remove the PAGE_SHIFT, you'll need to put those bits 
somewhere else. But that change in the encoding would break the ABI with 
the Thunk.

Regards,
   Felix

On 2019-11-01 4:48 p.m., Zhao, Yong wrote:
> The new code is much cleaner and results in better readability.
>
> Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
> Signed-off-by: Yong Zhao 
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
>   4 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b91993753b82..590138727ca9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
> struct kfd_process *p,
>   /* Return gpu_id as doorbell offset for mmap usage */
>   args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
>   args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
> - args->doorbell_offset <<= PAGE_SHIFT;
>   if (KFD_IS_SOC15(dev->device_info->asic_family))
>   /* On SOC15 ASICs, include the doorbell offset within the
>* process doorbell frame, which could be 1 page or 2 pages.
> @@ -1938,20 +1937,22 @@ static int kfd_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>   {
>   struct kfd_process *process;
>   struct kfd_dev *dev = NULL;
> - unsigned long vm_pgoff;
> + unsigned long mmap_offset;
>   unsigned int gpu_id;
>   
>   process = kfd_get_process(current);
>   if (IS_ERR(process))
>   return PTR_ERR(process);
>   
> - vm_pgoff = vma->vm_pgoff;
> - vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
> - gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
> + mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
> + gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
>   if (gpu_id)
>   dev = kfd_device_by_id(gpu_id);
>   
> - switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
> + /* only leave the offset segment */
> + vma->vm_pgoff &= (1ULL << (KFD_MMAP_GPU_ID_SHIFT - PAGE_SHIFT)) - 1;
> +
> + switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
>   case KFD_MMAP_TYPE_DOORBELL:
>   if (!dev)
>   return -ENODEV;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> index 908081c85de1..1f8365575b12 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> @@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
> kfd_process *p,
>   ret = create_signal_event(devkfd, p, ev);
>   if (!ret) {
>   *event_page_offset = KFD_MMAP_TYPE_EVENTS;
> - *event_page_offset <<= PAGE_SHIFT;
>   *event_slot_index = ev->event_id;
>   }
>   break;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 66bae8f2dad1..8eecd2cd1fd2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -59,24 +59,21 @@
>* NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
>*  defines are w.r.t to PAGE_SIZE
>*/
> -#define KFD_MMAP_TYPE_SHIFT  (62 - PAGE_SHIFT)
> +#define KFD_MMAP_TYPE_SHIFT  (62)
>   #define KFD_MMAP_TYPE_MASK  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_DOORBELL  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_EVENTS(0x2ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_RESERVED_MEM  (0x1ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_MMIO  (0x0ULL << KFD_MMAP_TYPE_SHIFT)
>   
> -#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
> +#define KFD_MMAP_GPU_ID_SHIFT (46)
>   #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \
>   << KFD_MMAP_GPU_ID_SHIFT)
>   #define KFD_MMAP_GPU_ID(gpu_id) uint64_t)gpu_id) << 
> KFD_MMAP_GPU_ID_SHIFT)\
>   & KFD_MMAP_GPU_ID_MASK)
> -#define KFD_MMAP_GPU_ID_GET(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
> +#define KFD_MMAP_GET_GPU_ID(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
>  

Re: [PATCH v2 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6

2019-11-01 Thread Harry Wentland
On 2019-11-01 3:38 p.m., Jerry (Fangzhi) Zuo wrote:
> DP 1.4 edid corruption test requires source DUT to write calculated
> CRC, not the corrupted CRC from reference sink.
> 
> Return the calculated CRC back, and initiate the required sequence.
> 
> -v2: Have separate routine for returning real CRC
> 
> Signed-off-by: Jerry (Fangzhi) Zuo 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 36 
>  drivers/gpu/drm/drm_edid.c  | 14 ++
>  include/drm/drm_connector.h |  7 +++
>  include/drm/drm_dp_helper.h |  3 +++
>  4 files changed, 60 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ffc68d305afe..75dbd30c62a7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> +/**
> +  * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
> +  * @aux: DisplayPort AUX channel
> +  * @bad_edid_checksum: real edid checksum for the last block
> +  *
> +  * Returns true on success
> +  */
> +bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
> +u8 bad_edid_checksum)
> +{
> +u8 link_edid_read = 0, auto_test_req = 0;
> +u8 test_resp = 0;
> +
> +drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 
> 1);
> +auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
> +
> +drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_edid_read, 1);
> +link_edid_read &= DP_TEST_LINK_EDID_READ;
> +
> +if (!auto_test_req || !link_edid_read) {
> +DRM_DEBUG_KMS("Source DUT does not support 
> TEST_EDID_READ\n");
> +return false;
> +}
> +
> +drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 
> 1);
> +
> +/* send back checksum for the last edid extension block data */
> +drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &bad_edid_checksum, 1);
> +
> +test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
> +drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1);
> +
> +return true;
> +}
> +EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
> +
>  /**
>   * drm_dp_link_probe() - probe a DisplayPort link for capabilities
>   * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..0598314e3f46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1354,6 +1354,17 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
>   return csum;
>  }
>  
> +static int drm_edid_block_real_checksum(const u8 *raw_edid)
> +{
> + int i;
> + u8 csum = 0;
> +
> + for (i = 0; i < EDID_LENGTH - 1; i++)
> + csum += raw_edid[i];
> +
> + return (0x100 - csum);
> +}
> +
>  static bool drm_edid_is_zero(const u8 *in_edid, int length)
>  {
>   if (memchr_inv(in_edid, 0, length))
> @@ -1572,6 +1583,9 @@ static void connector_bad_edid(struct drm_connector 
> *connector,
>  prefix, DUMP_PREFIX_NONE, 16, 1,
>  block, EDID_LENGTH, false);
>   }
> +
> + /* Calculate real checksum for the last edid extension block data */
> + connector->bad_edid_checksum = drm_edid_block_real_checksum(edid + 
> edid[0x7e] * EDID_LENGTH);
>  }
>  
>  /* Get override or firmware EDID */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8442461542b9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1345,6 +1345,13 @@ struct drm_connector {
>* rev1.1 4.2.2.6
>*/
>   bool edid_corrupt;
> + /**
> + * @bad_edid_checksum: real edid checksum value for corrupted edid 
> block.
> + * Required in Displayport 1.4 compliance testing
> + * rev1.1 4.2.2.6
> + */
> +uint8_t bad_edid_checksum;


This variable name confused me a bit. Maybe name this
"computed_edid_checksum" to clarify that this is the EDID checksum that
we've computed for the EDID, i.e. the correct one.

> +
>  
>   /** @debugfs_entry: debugfs directory for this connector */
>   struct dentry *debugfs_entry;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 5a795075d5da..2a7e54bebb18 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1383,6 +1383,9 @@ static inline ssize_t drm_dp_dpcd_writeb(struct 
> drm_dp_aux *aux,
>  int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>u8 status[DP_LINK_STATUS_SIZE]);
>  
> +bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,

Same as before, might be good to name this
drm_dp_send_computed_edid_checksum or something similar.

Harry

> + u8 bad_edid_checksum);
> +
> 

Re: [PATCH v2 00/15] Consolidate the mmu notifier interval_tree and locking

2019-11-01 Thread Ralph Campbell


On 10/28/19 1:10 PM, Jason Gunthorpe wrote:

From: Jason Gunthorpe 

8 of the mmu_notifier using drivers (i915_gem, radeon_mn, umem_odp, hfi1,
scif_dma, vhost, gntdev, hmm) drivers are using a common pattern where
they only use invalidate_range_start/end and immediately check the
invalidating range against some driver data structure to tell if the
driver is interested. Half of them use an interval_tree, the others are
simple linear search lists.

Of the ones I checked they largely seem to have various kinds of races,
bugs and poor implementation. This is a result of the complexity in how
the notifier interacts with get_user_pages(). It is extremely difficult to
use it correctly.

Consolidate all of this code together into the core mmu_notifier and
provide a locking scheme similar to hmm_mirror that allows the user to
safely use get_user_pages() and reliably know if the page list still
matches the mm.

This new arrangment plays nicely with the !blockable mode for
OOM. Scanning the interval tree is done such that the intersection test
will always succeed, and since there is no invalidate_range_end exposed to
drivers the scheme safely allows multiple drivers to be subscribed.

Four places are converted as an example of how the new API is used.
Four are left for future patches:
  - i915_gem has complex locking around destruction of a registration,
needs more study
  - hfi1 (2nd user) needs access to the rbtree
  - scif_dma has a complicated logic flow
  - vhost's mmu notifiers are already being rewritten

This series, and the other code it depends on is available on my github:

https://github.com/jgunthorpe/linux/commits/mmu_notifier

v2 changes:
- Add mmu_range_set_seq() to set the mrn sequence number under the driver
   lock and make the locking more understandable
- Add some additional comments around locking/READ_ONCe
- Make the WARN_ON flow in mn_itree_invalidate a bit easier to follow
- Fix wrong WARN_ON

Jason Gunthorpe (15):
   mm/mmu_notifier: define the header pre-processor parts even if
 disabled
   mm/mmu_notifier: add an interval tree notifier
   mm/hmm: allow hmm_range to be used with a mmu_range_notifier or
 hmm_mirror
   mm/hmm: define the pre-processor related parts of hmm.h even if
 disabled
   RDMA/odp: Use mmu_range_notifier_insert()
   RDMA/hfi1: Use mmu_range_notifier_inset for user_exp_rcv
   drm/radeon: use mmu_range_notifier_insert
   xen/gntdev: Use select for DMA_SHARED_BUFFER
   xen/gntdev: use mmu_range_notifier_insert
   nouveau: use mmu_notifier directly for invalidate_range_start
   nouveau: use mmu_range_notifier instead of hmm_mirror
   drm/amdgpu: Call find_vma under mmap_sem
   drm/amdgpu: Use mmu_range_insert instead of hmm_mirror
   drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror
   mm/hmm: remove hmm_mirror and related

  Documentation/vm/hmm.rst  | 105 +---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   2 +
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   9 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|  14 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|   1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 457 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h|  53 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|  13 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 111 ++--
  drivers/gpu/drm/nouveau/nouveau_svm.c | 231 +---
  drivers/gpu/drm/radeon/radeon.h   |   9 +-
  drivers/gpu/drm/radeon/radeon_mn.c| 219 ++-
  drivers/infiniband/core/device.c  |   1 -
  drivers/infiniband/core/umem_odp.c| 288 +
  drivers/infiniband/hw/hfi1/file_ops.c |   2 +-
  drivers/infiniband/hw/hfi1/hfi.h  |   2 +-
  drivers/infiniband/hw/hfi1/user_exp_rcv.c | 146 ++---
  drivers/infiniband/hw/hfi1/user_exp_rcv.h |   3 +-
  drivers/infiniband/hw/mlx5/mlx5_ib.h  |   7 +-
  drivers/infiniband/hw/mlx5/mr.c   |   3 +-
  drivers/infiniband/hw/mlx5/odp.c  |  50 +-
  drivers/xen/Kconfig   |   3 +-
  drivers/xen/gntdev-common.h   |   8 +-
  drivers/xen/gntdev.c  | 180 ++
  include/linux/hmm.h   | 195 +--
  include/linux/mmu_notifier.h  | 144 -
  include/rdma/ib_umem_odp.h|  65 +--
  include/rdma/ib_verbs.h   |   2 -
  kernel/fork.c |   1 -
  mm/Kconfig|   2 +-
  mm/hmm.c  | 275 +
  mm/mmu_notifier.c | 546 +-
  32 files changed, 1225 insertions(+), 1922 deletions(-)



You can add my Tested-by for the mm and nouveau changes.
IOW, patches 1-4, 10-11, and 15.

Tested-by: Ralph Campbell 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.

Re: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Zhao, Yong
Please discard this one and look for an update version.

Regards,
Yong

From: Zhao, Yong 
Sent: Friday, November 1, 2019 4:11 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Zhao, Yong 
Subject: [PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

The new code is much cleaner and results in better readability.

Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 --
 drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
 4 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b91993753b82..34078df36621 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
struct kfd_process *p,
 /* Return gpu_id as doorbell offset for mmap usage */
 args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
 args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
-   args->doorbell_offset <<= PAGE_SHIFT;
 if (KFD_IS_SOC15(dev->device_info->asic_family))
 /* On SOC15 ASICs, include the doorbell offset within the
  * process doorbell frame, which could be 1 page or 2 pages.
@@ -1938,20 +1937,19 @@ static int kfd_mmap(struct file *filp, struct 
vm_area_struct *vma)
 {
 struct kfd_process *process;
 struct kfd_dev *dev = NULL;
-   unsigned long vm_pgoff;
+   unsigned long mmap_offset;
 unsigned int gpu_id;

 process = kfd_get_process(current);
 if (IS_ERR(process))
 return PTR_ERR(process);

-   vm_pgoff = vma->vm_pgoff;
-   vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
-   gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
+   mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
+   gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
 if (gpu_id)
 dev = kfd_device_by_id(gpu_id);

-   switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
+   switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
 case KFD_MMAP_TYPE_DOORBELL:
 if (!dev)
 return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 908081c85de1..1f8365575b12 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
kfd_process *p,
 ret = create_signal_event(devkfd, p, ev);
 if (!ret) {
 *event_page_offset = KFD_MMAP_TYPE_EVENTS;
-   *event_page_offset <<= PAGE_SHIFT;
 *event_slot_index = ev->event_id;
 }
 break;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 66bae8f2dad1..8eecd2cd1fd2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -59,24 +59,21 @@
  * NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
  *  defines are w.r.t to PAGE_SIZE
  */
-#define KFD_MMAP_TYPE_SHIFT(62 - PAGE_SHIFT)
+#define KFD_MMAP_TYPE_SHIFT(62)
 #define KFD_MMAP_TYPE_MASK  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_DOORBELL  (0x3ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_EVENTS(0x2ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_RESERVED_MEM  (0x1ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_MMIO  (0x0ULL << KFD_MMAP_TYPE_SHIFT)

-#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
+#define KFD_MMAP_GPU_ID_SHIFT (46)
 #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \
 << KFD_MMAP_GPU_ID_SHIFT)
 #define KFD_MMAP_GPU_ID(gpu_id) uint64_t)gpu_id) << KFD_MMAP_GPU_ID_SHIFT)\
 & KFD_MMAP_GPU_ID_MASK)
-#define KFD_MMAP_GPU_ID_GET(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
+#define KFD_MMAP_GET_GPU_ID(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
 >> KFD_MMAP_GPU_ID_SHIFT)

-#define KFD_MMAP_OFFSET_VALUE_MASK (0x3FFFULL >> PAGE_SHIFT)
-#define KFD_MMAP_OFFSET_VALUE_GET(offset) (offset & KFD_MMAP_OFFSET_VALUE_MASK)
-
 /*
  * When working with cp scheduler we should assign the HIQ manually or via
  * the amdgpu driver to a fixed hqd slot, here are the fixed HIQ hqd slot
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 6abfb77ae540..39dc49b8fd85 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -554,8 +554,7 @@ static int kfd_process_init_cwsr_apu(struct kfd_process *p, 
struct file *fil

[PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Zhao, Yong
The new code is much cleaner and results in better readability.

Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 +++--
 drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
 4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b91993753b82..590138727ca9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
struct kfd_process *p,
/* Return gpu_id as doorbell offset for mmap usage */
args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
-   args->doorbell_offset <<= PAGE_SHIFT;
if (KFD_IS_SOC15(dev->device_info->asic_family))
/* On SOC15 ASICs, include the doorbell offset within the
 * process doorbell frame, which could be 1 page or 2 pages.
@@ -1938,20 +1937,22 @@ static int kfd_mmap(struct file *filp, struct 
vm_area_struct *vma)
 {
struct kfd_process *process;
struct kfd_dev *dev = NULL;
-   unsigned long vm_pgoff;
+   unsigned long mmap_offset;
unsigned int gpu_id;
 
process = kfd_get_process(current);
if (IS_ERR(process))
return PTR_ERR(process);
 
-   vm_pgoff = vma->vm_pgoff;
-   vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
-   gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
+   mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
+   gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
if (gpu_id)
dev = kfd_device_by_id(gpu_id);
 
-   switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
+   /* only leave the offset segment */
+   vma->vm_pgoff &= (1ULL << (KFD_MMAP_GPU_ID_SHIFT - PAGE_SHIFT)) - 1;
+
+   switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
case KFD_MMAP_TYPE_DOORBELL:
if (!dev)
return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 908081c85de1..1f8365575b12 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
kfd_process *p,
ret = create_signal_event(devkfd, p, ev);
if (!ret) {
*event_page_offset = KFD_MMAP_TYPE_EVENTS;
-   *event_page_offset <<= PAGE_SHIFT;
*event_slot_index = ev->event_id;
}
break;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 66bae8f2dad1..8eecd2cd1fd2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -59,24 +59,21 @@
  * NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
  *  defines are w.r.t to PAGE_SIZE
  */
-#define KFD_MMAP_TYPE_SHIFT(62 - PAGE_SHIFT)
+#define KFD_MMAP_TYPE_SHIFT(62)
 #define KFD_MMAP_TYPE_MASK (0x3ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_DOORBELL (0x3ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_EVENTS   (0x2ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_RESERVED_MEM (0x1ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_MMIO (0x0ULL << KFD_MMAP_TYPE_SHIFT)
 
-#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
+#define KFD_MMAP_GPU_ID_SHIFT (46)
 #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \
<< KFD_MMAP_GPU_ID_SHIFT)
 #define KFD_MMAP_GPU_ID(gpu_id) uint64_t)gpu_id) << KFD_MMAP_GPU_ID_SHIFT)\
& KFD_MMAP_GPU_ID_MASK)
-#define KFD_MMAP_GPU_ID_GET(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
+#define KFD_MMAP_GET_GPU_ID(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
>> KFD_MMAP_GPU_ID_SHIFT)
 
-#define KFD_MMAP_OFFSET_VALUE_MASK (0x3FFFULL >> PAGE_SHIFT)
-#define KFD_MMAP_OFFSET_VALUE_GET(offset) (offset & KFD_MMAP_OFFSET_VALUE_MASK)
-
 /*
  * When working with cp scheduler we should assign the HIQ manually or via
  * the amdgpu driver to a fixed hqd slot, here are the fixed HIQ hqd slot
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 6abfb77ae540..39dc49b8fd85 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -554,8 +554,7 @@ static int kfd_process_init_cwsr_apu(struct kfd_process *p, 
struct file *filep)
if (!dev->cwsr_enabled || qpd->cwsr_kaddr || qpd->cwsr_base)
continue;
 
-   offset = (KFD_MMAP_TYPE_RESERVED_MEM | KFD_MMAP_GPU_ID(d

Re: [PATCH] drm/amdgpu: Show resolution correctly in mode validation debug output

2019-11-01 Thread Harry Wentland
On 2019-11-01 11:47 a.m., Neil Mayhew wrote:
> On 2019-11-01 9:13 a.m., Harry Wentland wrote:
>> On 2019-10-30 2:58 p.m., n...@neil.mayhew.name wrote:
>>> From: Neil Mayhew 
>> This requires your Signed-off-by. See [1].
>>
>> With that fixed your change looks good and is
>> Reviewed-by: Harry Wentland 
>>
>> You can simply reply to this email with your Signed-off-by and I can add
>> it when merging, or you can send a v2 patch with your Signed-off-by.
> 
> Signed-off-by: Neil Mayhew 
> 

Thanks and thanks for your fix. Applied.

Harry

> Thanks, Harry.
> 
>> [1]
>> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
>>
>> Harry
>>
>>> ---
>>>  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 a52f0b13a2c8..f802c784e6f6 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -4114,8 +4114,8 @@ enum drm_mode_status 
>>> amdgpu_dm_connector_mode_valid(struct drm_connector *connec
>>> result = MODE_OK;
>>> else
>>> DRM_DEBUG_KMS("Mode %dx%d (clk %d) failed DC validation with 
>>> error %d\n",
>>> - mode->vdisplay,
>>>   mode->hdisplay,
>>> + mode->vdisplay,
>>>   mode->clock,
>>>   dc_result);
>>>  
>>>
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdkfd: Simplify the mmap offset related bit operations

2019-11-01 Thread Zhao, Yong
The new code is much cleaner and results in better readability.

Change-Id: I0c1f7cca7e24ddb7b4ffe1cb0fa71943828ae373
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 --
 drivers/gpu/drm/amd/amdkfd/kfd_events.c  |  1 -
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  9 +++--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +--
 4 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b91993753b82..34078df36621 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -298,7 +298,6 @@ static int kfd_ioctl_create_queue(struct file *filep, 
struct kfd_process *p,
/* Return gpu_id as doorbell offset for mmap usage */
args->doorbell_offset = KFD_MMAP_TYPE_DOORBELL;
args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
-   args->doorbell_offset <<= PAGE_SHIFT;
if (KFD_IS_SOC15(dev->device_info->asic_family))
/* On SOC15 ASICs, include the doorbell offset within the
 * process doorbell frame, which could be 1 page or 2 pages.
@@ -1938,20 +1937,19 @@ static int kfd_mmap(struct file *filp, struct 
vm_area_struct *vma)
 {
struct kfd_process *process;
struct kfd_dev *dev = NULL;
-   unsigned long vm_pgoff;
+   unsigned long mmap_offset;
unsigned int gpu_id;
 
process = kfd_get_process(current);
if (IS_ERR(process))
return PTR_ERR(process);
 
-   vm_pgoff = vma->vm_pgoff;
-   vma->vm_pgoff = KFD_MMAP_OFFSET_VALUE_GET(vm_pgoff);
-   gpu_id = KFD_MMAP_GPU_ID_GET(vm_pgoff);
+   mmap_offset = vma->vm_pgoff << PAGE_SHIFT;
+   gpu_id = KFD_MMAP_GET_GPU_ID(mmap_offset);
if (gpu_id)
dev = kfd_device_by_id(gpu_id);
 
-   switch (vm_pgoff & KFD_MMAP_TYPE_MASK) {
+   switch (mmap_offset & KFD_MMAP_TYPE_MASK) {
case KFD_MMAP_TYPE_DOORBELL:
if (!dev)
return -ENODEV;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 908081c85de1..1f8365575b12 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -346,7 +346,6 @@ int kfd_event_create(struct file *devkfd, struct 
kfd_process *p,
ret = create_signal_event(devkfd, p, ev);
if (!ret) {
*event_page_offset = KFD_MMAP_TYPE_EVENTS;
-   *event_page_offset <<= PAGE_SHIFT;
*event_slot_index = ev->event_id;
}
break;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 66bae8f2dad1..8eecd2cd1fd2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -59,24 +59,21 @@
  * NOTE: struct vm_area_struct.vm_pgoff uses offset in pages. Hence, these
  *  defines are w.r.t to PAGE_SIZE
  */
-#define KFD_MMAP_TYPE_SHIFT(62 - PAGE_SHIFT)
+#define KFD_MMAP_TYPE_SHIFT(62)
 #define KFD_MMAP_TYPE_MASK (0x3ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_DOORBELL (0x3ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_EVENTS   (0x2ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_RESERVED_MEM (0x1ULL << KFD_MMAP_TYPE_SHIFT)
 #define KFD_MMAP_TYPE_MMIO (0x0ULL << KFD_MMAP_TYPE_SHIFT)
 
-#define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
+#define KFD_MMAP_GPU_ID_SHIFT (46)
 #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \
<< KFD_MMAP_GPU_ID_SHIFT)
 #define KFD_MMAP_GPU_ID(gpu_id) uint64_t)gpu_id) << KFD_MMAP_GPU_ID_SHIFT)\
& KFD_MMAP_GPU_ID_MASK)
-#define KFD_MMAP_GPU_ID_GET(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
+#define KFD_MMAP_GET_GPU_ID(offset)((offset & KFD_MMAP_GPU_ID_MASK) \
>> KFD_MMAP_GPU_ID_SHIFT)
 
-#define KFD_MMAP_OFFSET_VALUE_MASK (0x3FFFULL >> PAGE_SHIFT)
-#define KFD_MMAP_OFFSET_VALUE_GET(offset) (offset & KFD_MMAP_OFFSET_VALUE_MASK)
-
 /*
  * When working with cp scheduler we should assign the HIQ manually or via
  * the amdgpu driver to a fixed hqd slot, here are the fixed HIQ hqd slot
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 6abfb77ae540..39dc49b8fd85 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -554,8 +554,7 @@ static int kfd_process_init_cwsr_apu(struct kfd_process *p, 
struct file *filep)
if (!dev->cwsr_enabled || qpd->cwsr_kaddr || qpd->cwsr_base)
continue;
 
-   offset = (KFD_MMAP_TYPE_RESERVED_MEM | KFD_MMAP_GPU_ID(dev->id))
-   << PAGE_SHIFT;
+   offset = KFD_MMAP_TYPE_RESERVED_MEM | KFD_MMAP_GPU_ID(dev->id);

[PATCH] drm/amdkfd: Rename create_cp_queue() to init_user_queue()

2019-11-01 Thread Zhao, Yong
create_cp_queue() could also work with SDMA queues, so we should rename
it.

Change-Id: I76cbaed8fa95dd9062d786cbc1dd037ff041da9d
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 48185d2957e9..ebb2f69b438c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -162,7 +162,7 @@ void pqm_uninit(struct process_queue_manager *pqm)
pqm->queue_slot_bitmap = NULL;
 }
 
-static int create_cp_queue(struct process_queue_manager *pqm,
+static int init_user_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev, struct queue **q,
struct queue_properties *q_properties,
struct file *f, unsigned int qid)
@@ -251,7 +251,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
goto err_create_queue;
}
 
-   retval = create_cp_queue(pqm, dev, &q, properties, f, *qid);
+   retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
if (retval != 0)
goto err_create_queue;
pqn->q = q;
@@ -272,7 +272,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
goto err_create_queue;
}
 
-   retval = create_cp_queue(pqm, dev, &q, properties, f, *qid);
+   retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
if (retval != 0)
goto err_create_queue;
pqn->q = q;
-- 
2.17.1

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

[PATCH 1/2] drm/amdkfd: Use better name to indicate the offset is in dwords

2019-11-01 Thread Zhao, Yong
Change-Id: I75da23bba90231762cf58da3170f5bb77ece45ed
Signed-off-by: Yong Zhao 
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c  | 14 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  8 
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 984c2f2b24b6..4503fb26fe5b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -170,7 +170,7 @@ static int allocate_doorbell(struct qcm_process_device 
*qpd, struct queue *q)
}
 
q->properties.doorbell_off =
-   kfd_doorbell_id_to_offset(dev, q->process,
+   kfd_get_doorbell_dw_offset_from_bar(dev, q->process,
  q->doorbell_id);
 
return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index ebe79bf00145..f904355c44a1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -91,7 +91,7 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
kfd->doorbell_base = kfd->shared_resources.doorbell_physical_address +
doorbell_start_offset;
 
-   kfd->doorbell_id_offset = doorbell_start_offset / sizeof(u32);
+   kfd->doorbell_base_dw_offset = doorbell_start_offset / sizeof(u32);
 
kfd->doorbell_kernel_ptr = ioremap(kfd->doorbell_base,
   kfd_doorbell_process_slice(kfd));
@@ -103,8 +103,8 @@ int kfd_doorbell_init(struct kfd_dev *kfd)
pr_debug("doorbell base   == 0x%08lX\n",
(uintptr_t)kfd->doorbell_base);
 
-   pr_debug("doorbell_id_offset  == 0x%08lX\n",
-   kfd->doorbell_id_offset);
+   pr_debug("doorbell_base_dw_offset  == 0x%08lX\n",
+   kfd->doorbell_base_dw_offset);
 
pr_debug("doorbell_process_limit  == 0x%08lX\n",
doorbell_process_limit);
@@ -185,7 +185,7 @@ void __iomem *kfd_get_kernel_doorbell(struct kfd_dev *kfd,
 * Calculating the kernel doorbell offset using the first
 * doorbell page.
 */
-   *doorbell_off = kfd->doorbell_id_offset + inx;
+   *doorbell_off = kfd->doorbell_base_dw_offset + inx;
 
pr_debug("Get kernel queue doorbell\n"
" doorbell offset   == 0x%08X\n"
@@ -225,17 +225,17 @@ void write_kernel_doorbell64(void __iomem *db, u64 value)
}
 }
 
-unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
+unsigned int kfd_get_doorbell_dw_offset_from_bar(struct kfd_dev *kfd,
struct kfd_process *process,
unsigned int doorbell_id)
 {
/*
-* doorbell_id_offset accounts for doorbells taken by KGD.
+* doorbell_base_dw_offset accounts for doorbells taken by KGD.
 * index * kfd_doorbell_process_slice/sizeof(u32) adjusts to
 * the process's doorbells. The offset returned is in dword
 * units regardless of the ASIC-dependent doorbell size.
 */
-   return kfd->doorbell_id_offset +
+   return kfd->doorbell_base_dw_offset +
process->doorbell_index
* kfd_doorbell_process_slice(kfd) / sizeof(u32) +
doorbell_id * kfd->device_info->doorbell_size / sizeof(u32);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 62db4d20ed32..7c561c98f2e2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -238,9 +238,9 @@ struct kfd_dev {
 * KFD. It is aligned for mapping
 * into user mode
 */
-   size_t doorbell_id_offset;  /* Doorbell offset (from KFD doorbell
-* to HW doorbell, GFX reserved some
-* at the start)
+   size_t doorbell_base_dw_offset; /* Doorbell dword offset (from KFD
+* doorbell to PCI doorbell bar,
+* GFX reserved some at the start)
 */
u32 __iomem *doorbell_kernel_ptr; /* This is a pointer for a doorbells
   * page used by kernel queue
@@ -821,7 +821,7 @@ void kfd_release_kernel_doorbell(struct kfd_dev *kfd, u32 
__iomem *db_addr);
 u32 read_kernel_doorbell(u32 __iomem *db);
 void write_kernel_doorbell(void __iomem *db, u32 value);
 void write_kernel_doorbell64(void __iomem *db, u64 value);
-unsigned int kfd_doorbell_id_to_offset(struct kfd_dev *kfd,
+uns

[PATCH 2/2] drm/amdkfd: Avoid using doorbell_off as offset in process doorbell pages

2019-11-01 Thread Zhao, Yong
dorbell_off in the queue properties is mainly used for the doorbell dw
offset in pci bar. We should not set it to the doorbell byte offset in
process doorbell pages. This makes the code much easier to read.

Change-Id: I553045ff9fcb3676900c92d10426f2ceb3660005
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 12 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c  |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h|  3 ++-
 .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c   |  8 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index d9e36dbf13d5..b91993753b82 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -258,6 +258,7 @@ static int kfd_ioctl_create_queue(struct file *filep, 
struct kfd_process *p,
unsigned int queue_id;
struct kfd_process_device *pdd;
struct queue_properties q_properties;
+   uint32_t doorbell_offset_in_process = 0;
 
memset(&q_properties, 0, sizeof(struct queue_properties));
 
@@ -286,7 +287,8 @@ static int kfd_ioctl_create_queue(struct file *filep, 
struct kfd_process *p,
p->pasid,
dev->id);
 
-   err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id);
+   err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id,
+   &doorbell_offset_in_process);
if (err != 0)
goto err_create_queue;
 
@@ -298,12 +300,10 @@ static int kfd_ioctl_create_queue(struct file *filep, 
struct kfd_process *p,
args->doorbell_offset |= KFD_MMAP_GPU_ID(args->gpu_id);
args->doorbell_offset <<= PAGE_SHIFT;
if (KFD_IS_SOC15(dev->device_info->asic_family))
-   /* On SOC15 ASICs, doorbell allocation must be
-* per-device, and independent from the per-process
-* queue_id. Return the doorbell offset within the
-* doorbell aperture to user mode.
+   /* On SOC15 ASICs, include the doorbell offset within the
+* process doorbell frame, which could be 1 page or 2 pages.
 */
-   args->doorbell_offset |= q_properties.doorbell_off;
+   args->doorbell_offset |= doorbell_offset_in_process;
 
mutex_unlock(&p->mutex);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
index d59f2cd056c6..1d33c4f25263 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
@@ -185,7 +185,7 @@ static int dbgdev_register_diq(struct kfd_dbgdev *dbgdev)
properties.type = KFD_QUEUE_TYPE_DIQ;
 
status = pqm_create_queue(dbgdev->pqm, dbgdev->dev, NULL,
-   &properties, &qid);
+   &properties, &qid, NULL);
 
if (status) {
pr_err("Failed to create DIQ\n");
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 7c561c98f2e2..66bae8f2dad1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -907,7 +907,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev,
struct file *f,
struct queue_properties *properties,
-   unsigned int *qid);
+   unsigned int *qid,
+   uint32_t *p_doorbell_offset_in_process);
 int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid);
 int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
struct queue_properties *p);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 8509814a6ff0..48185d2957e9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -192,7 +192,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
struct kfd_dev *dev,
struct file *f,
struct queue_properties *properties,
-   unsigned int *qid)
+   unsigned int *qid,
+   uint32_t *p_doorbell_offset_in_process)
 {
int retval;
struct kfd_process_device *pdd;
@@ -307,8 +308,11 @@ int pqm_create_queue(struct process_queue_manager *pqm,
/* Return the doorbell offset within the doorbell page
 * to the caller so it can be passed up to user mode
 * (in bytes).
+* There are always 1024 doorbells per process, so in case
+* of 8

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Yang, Philip
Sorry, resend patch, the one in previous email missed couple of lines 
duo to copy/paste.

On 2019-11-01 3:45 p.m., Yang, Philip wrote:
> 
> 
> On 2019-11-01 1:42 p.m., Jason Gunthorpe wrote:
>> On Fri, Nov 01, 2019 at 03:59:26PM +, Yang, Philip wrote:
 This test for range_blockable should be before mutex_lock, I can move
 it up

>>> yes, thanks.
>>
>> Okay, I wrote it like this:
>>
>>  if (mmu_notifier_range_blockable(range))
>>  mutex_lock(&adev->notifier_lock);
>>  else if (!mutex_trylock(&adev->notifier_lock))
>>  return false;
>>
 Also, do you know if notifier_lock is held while calling
 amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held'
 to amdgpu_ttm_tt_get_user_pages_done()?
>>>
>>> gpu side hold notifier_lock but kfd side doesn't. kfd side doesn't check
>>> amdgpu_ttm_tt_get_user_pages_done/mmu_range_read_retry return value but
>>> check mem->invalid flag which is updated from invalidate callback. It
>>> takes more time to change, I will come to another patch to fix it later.
>>
>> Ah.. confusing, OK, I'll let you sort that
>>
 However, this is all pre-existing bugs, so I'm OK go ahead with this
 patch as modified. I advise AMD to make a followup patch ..

>>> yes, I will.
>>
>> While you are here, this is also wrong:
>>
>> int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
>> {
>>  down_read(&mm->mmap_sem);
>>  r = hmm_range_fault(range, 0);
>>  up_read(&mm->mmap_sem);
>>  if (unlikely(r <= 0)) {
>>  if ((r == 0 || r == -EBUSY) && !time_after(jiffies, timeout))
>>  goto retry;
>>  goto out_free_pfns;
>>  }
>>
>>  for (i = 0; i < ttm->num_pages; i++) {
>>  pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);
>>
>> It is not allowed to read the results of hmm_range_fault() outside
>> locking, and in particular, we can't convert to a struct page.
>>
>> This must be done inside the notifier_lock, after checking
>> mmu_range_read_retry(), all handling of the struct page must be
>> structured like that.
>>
> Below change will fix this, then driver will call mmu_range_read_retry
> second time using same range->notifier_seq to check if range is
> invalidated inside amdgpu_cs_submit, this looks ok for me.
> 
@@ -797,6 +797,7 @@ static const uint64_t 
hmm_range_values[HMM_PFN_VALUE_MAX] = {
   */
  int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page 
**pages)
  {
+   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 struct ttm_tt *ttm = bo->tbo.ttm;
 struct amdgpu_ttm_tt *gtt = (void *)ttm;
 unsigned long start = gtt->userptr;
@@ -868,6 +869,13 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo 
*bo, struct page **pages)
 goto out_free_pfns;
 }

+   mutex_lock(&adev->notifier_lock);
+
+   if (mmu_range_read_retry(&bo->notifier, range->notifier_seq)) {
+   mutex_unlock(&adev->notifier_lock);
+   goto retry;
+   }
+
 for (i = 0; i < ttm->num_pages; i++) {
 pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);
 if (unlikely(!pages[i])) {
@@ -875,10 +883,12 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo 
*bo, struct page **pages)
i, range->pfns[i]);
 r = -ENOMEM;

+   mutex_unlock(&adev->notifier_lock);
 goto out_free_pfns;
 }
 }

+   mutex_unlock(&adev->notifier_lock);
 gtt->range = range;
 mmput(mm);

> 
> Philip
> 
> @@ -997,10 +1004,18 @@ static void amdgpu_ttm_tt_unpin_userptr(struct 
> ttm_tt *ttm)
>   sg_free_table(ttm->sg);
> 
> #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
> - if (gtt->range &&
> - ttm->pages[0] == hmm_device_entry_to_page(gtt->range,
> -   gtt->range->pfns[0]))
> - WARN_ONCE(1, "Missing get_user_page_done\n");
> + if (gtt->range) {
> + unsigned long i;
> +
> + for (i = 0; i < ttm->num_pages; i++) {
> + if (ttm->pages[i] !=
> + hmm_device_entry_to_page(gtt->range,
> +   gtt->range->pfns[i]))
> + break;
> + }
> +
> + WARN((i == ttm->num_pages), "Missing get_user_page_done\n");
> + }

 Is this related/necessary? I can put it in another patch if it is just
 debugging improvement? Please advise

>>> I see this WARN backtrace now, but I didn't see it before. This is
>>> somehow related.
>>
>> Hm, might be instructive to learn what is going on..
>>
>> Thanks,
>> Jason
>>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.fr

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Yang, Philip


On 2019-11-01 1:42 p.m., Jason Gunthorpe wrote:
> On Fri, Nov 01, 2019 at 03:59:26PM +, Yang, Philip wrote:
>>> This test for range_blockable should be before mutex_lock, I can move
>>> it up
>>>
>> yes, thanks.
> 
> Okay, I wrote it like this:
> 
>   if (mmu_notifier_range_blockable(range))
>   mutex_lock(&adev->notifier_lock);
>   else if (!mutex_trylock(&adev->notifier_lock))
>   return false;
> 
>>> Also, do you know if notifier_lock is held while calling
>>> amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held'
>>> to amdgpu_ttm_tt_get_user_pages_done()?
>>
>> gpu side hold notifier_lock but kfd side doesn't. kfd side doesn't check
>> amdgpu_ttm_tt_get_user_pages_done/mmu_range_read_retry return value but
>> check mem->invalid flag which is updated from invalidate callback. It
>> takes more time to change, I will come to another patch to fix it later.
> 
> Ah.. confusing, OK, I'll let you sort that
> 
>>> However, this is all pre-existing bugs, so I'm OK go ahead with this
>>> patch as modified. I advise AMD to make a followup patch ..
>>>
>> yes, I will.
> 
> While you are here, this is also wrong:
> 
> int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
> {
>   down_read(&mm->mmap_sem);
>   r = hmm_range_fault(range, 0);
>   up_read(&mm->mmap_sem);
>   if (unlikely(r <= 0)) {
>   if ((r == 0 || r == -EBUSY) && !time_after(jiffies, timeout))
>   goto retry;
>   goto out_free_pfns;
>   }
> 
>   for (i = 0; i < ttm->num_pages; i++) {
>   pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);
> 
> It is not allowed to read the results of hmm_range_fault() outside
> locking, and in particular, we can't convert to a struct page.
> 
> This must be done inside the notifier_lock, after checking
> mmu_range_read_retry(), all handling of the struct page must be
> structured like that.
> 
Below change will fix this, then driver will call mmu_range_read_retry 
second time using same range->notifier_seq to check if range is 
invalidated inside amdgpu_cs_submit, this looks ok for me.

@@ -868,6 +869,13 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo 
*bo, struct page **pages)
 goto out_free_pfns;
 }

+   mutex_lock(&adev->notifier_lock);
+
+   if (mmu_range_read_retry(&bo->notifier, range->notifier_seq)) {
+   mutex_unlock(&adev->notifier_lock);
+   goto retry;
+   }
+
 for (i = 0; i < ttm->num_pages; i++) {
 pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);
 if (unlikely(!pages[i])) {
@@ -875,10 +883,12 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo 
*bo, struct page **pages)
i, range->pfns[i]);
 r = -ENOMEM;

+   mutex_unlock(&adev->notifier_lock);
 goto out_free_pfns;
 }
 }

+   mutex_unlock(&adev->notifier_lock);
 gtt->range = range;
 mmput(mm);

Philip

 @@ -997,10 +1004,18 @@ static void amdgpu_ttm_tt_unpin_userptr(struct 
 ttm_tt *ttm)
sg_free_table(ttm->sg);

#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
 -  if (gtt->range &&
 -  ttm->pages[0] == hmm_device_entry_to_page(gtt->range,
 -gtt->range->pfns[0]))
 -  WARN_ONCE(1, "Missing get_user_page_done\n");
 +  if (gtt->range) {
 +  unsigned long i;
 +
 +  for (i = 0; i < ttm->num_pages; i++) {
 +  if (ttm->pages[i] !=
 +  hmm_device_entry_to_page(gtt->range,
 +gtt->range->pfns[i]))
 +  break;
 +  }
 +
 +  WARN((i == ttm->num_pages), "Missing get_user_page_done\n");
 +  }
>>>
>>> Is this related/necessary? I can put it in another patch if it is just
>>> debugging improvement? Please advise
>>>
>> I see this WARN backtrace now, but I didn't see it before. This is
>> somehow related.
> 
> Hm, might be instructive to learn what is going on..
> 
> Thanks,
> Jason
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6

2019-11-01 Thread Jerry (Fangzhi) Zuo
DP 1.4 edid corruption test requires source DUT to write calculated
CRC, not the corrupted CRC from reference sink.

Return the calculated CRC back, and initiate the required sequence.

-v2: Have separate routine for returning real CRC

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/drm_dp_helper.c | 36 
 drivers/gpu/drm/drm_edid.c  | 14 ++
 include/drm/drm_connector.h |  7 +++
 include/drm/drm_dp_helper.h |  3 +++
 4 files changed, 60 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffc68d305afe..75dbd30c62a7 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+/**
+  * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
+  * @aux: DisplayPort AUX channel
+  * @bad_edid_checksum: real edid checksum for the last block
+  *
+  * Returns true on success
+  */
+bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
+u8 bad_edid_checksum)
+{
+u8 link_edid_read = 0, auto_test_req = 0;
+u8 test_resp = 0;
+
+drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 1);
+auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_edid_read, 1);
+link_edid_read &= DP_TEST_LINK_EDID_READ;
+
+if (!auto_test_req || !link_edid_read) {
+DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n");
+return false;
+}
+
+drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 
1);
+
+/* send back checksum for the last edid extension block data */
+drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &bad_edid_checksum, 1);
+
+test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1);
+
+return true;
+}
+EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
+
 /**
  * drm_dp_link_probe() - probe a DisplayPort link for capabilities
  * @aux: DisplayPort AUX channel
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 82a4ceed3fcf..0598314e3f46 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1354,6 +1354,17 @@ static int drm_edid_block_checksum(const u8 *raw_edid)
return csum;
 }
 
+static int drm_edid_block_real_checksum(const u8 *raw_edid)
+{
+   int i;
+   u8 csum = 0;
+
+   for (i = 0; i < EDID_LENGTH - 1; i++)
+   csum += raw_edid[i];
+
+   return (0x100 - csum);
+}
+
 static bool drm_edid_is_zero(const u8 *in_edid, int length)
 {
if (memchr_inv(in_edid, 0, length))
@@ -1572,6 +1583,9 @@ static void connector_bad_edid(struct drm_connector 
*connector,
   prefix, DUMP_PREFIX_NONE, 16, 1,
   block, EDID_LENGTH, false);
}
+
+   /* Calculate real checksum for the last edid extension block data */
+   connector->bad_edid_checksum = drm_edid_block_real_checksum(edid + 
edid[0x7e] * EDID_LENGTH);
 }
 
 /* Get override or firmware EDID */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 681cb590f952..8442461542b9 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1345,6 +1345,13 @@ struct drm_connector {
 * rev1.1 4.2.2.6
 */
bool edid_corrupt;
+   /**
+ * @bad_edid_checksum: real edid checksum value for corrupted edid 
block.
+ * Required in Displayport 1.4 compliance testing
+ * rev1.1 4.2.2.6
+ */
+uint8_t bad_edid_checksum;
+
 
/** @debugfs_entry: debugfs directory for this connector */
struct dentry *debugfs_entry;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5a795075d5da..2a7e54bebb18 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1383,6 +1383,9 @@ static inline ssize_t drm_dp_dpcd_writeb(struct 
drm_dp_aux *aux,
 int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 u8 status[DP_LINK_STATUS_SIZE]);
 
+bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
+   u8 bad_edid_checksum);
+
 /*
  * DisplayPort link
  */
-- 
2.14.1

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

[pull] amdgpu, amdkfd drm-next-5.5

2019-11-01 Thread Alex Deucher
Hi Dave, Daniel,

More stuff for 5.5.  Mostly bug fixes and cleanups at this point.

The following changes since commit 0e04ad7d1857670944786a8465930a049aaf995f:

  drm/amdgpu/powerplay: use local renoir array sizes for clock fetching 
(2019-10-25 16:48:14 -0400)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux tags/drm-next-5.5-2019-11-01

for you to fetch changes up to 5ab5e4e60accd13b0a505a4a34b6feafde2c8fbf:

  drm/amd/display: Add a conversion function for transmitter and phy_id enums 
(2019-10-30 11:07:13 -0400)


drm-next-5.5-2019-11-01:

amdgpu:
- Add EEPROM support for Arcturus
- Enable VCN encode support for Arcturus
- Misc PSP fixes
- Misc DC fixes
- swSMU cleanup

amdkfd:
- Misc cleanups
- Fix typo in cu bitmap parsing


Aidan Yang (2):
  drm/amd/display: Don't use optimized gamma22 with eetf
  drm/amd/display: Allow inverted gamma

Alex Deucher (1):
  drm/amdgpu/gmc10: properly set BANK_SELECT and FRAGMENT_SIZE

Alex Sierra (1):
  drm/amdkfd: bug fix for out of bounds mem on gpu cache filling info

Alvin Lee (1):
  drm/amd/display: Update min dcfclk

Andrey Grodzovsky (6):
  drm/amd/powerplay: Add interface for I2C transactions to SMU.
  drm/amd/powerplay: Add EEPROM I2C read/write support to Arcturus.
  drm/amdgpu: Use ARCTURUS in RAS EEPROM.
  drm/amdgpu: Move amdgpu_ras_recovery_init to after SMU ready.
  drm/sched: Set error to s_fence if HW job submission failed.
  drm/amdgpu: If amdgpu_ib_schedule fails return back the error.

Anthony Koo (2):
  drm/amd/display: correctly populate dpp refclk in fpga
  drm/amd/display: Proper return of result when aux engine acquire fails

Aric Cyr (2):
  drm/amd/display: 3.2.55
  drm/amd/display: 3.2.56

Chenwandun (1):
  drm/amd/display: remove gcc warning Wunused-but-set-variable

Colin Ian King (1):
  drm/amdgpu/psp: fix spelling mistake "initliaze" -> "initialize"

Dmytro Laktyushkin (8):
  drm/amd/display: remove unused code
  drm/amd/display: split dcn20 fast validate into more functions
  drm/amd/display: correctly initialize dml odm variables
  drm/amd/display: move dispclk vco freq to clk mgr base
  drm/amd/display: remove unnecessary assert
  drm/amd/display: fix number of dcn21 dpm clock levels
  drm/amd/display: add embedded flag to dml
  drm/amd/display: fix avoid_split for dcn2+ validation

Eric Yang (2):
  drm/amd/display: move wm ranges reporting to end of init hw
  drm/amd/display: fix hubbub deadline programing

Evan Quan (6):
  drm/amd/powerplay: update Arcturus driver smu interface XGMI link part
  drm/amd/powerplay: add lock protection for swSMU APIs V2
  drm/amd/powerplay: split out those internal used swSMU APIs V2
  drm/amd/powerplay: clear the swSMU code layer
  drm/amd/powerplay: skip unsupported clock limit settings on Arcturus V2
  drm/amd/powerplay: correct current clock level label for Arcturus

Geert Uytterhoeven (1):
  drm/amdgpu: Remove superfluous void * cast in debugfs_create_file() call

Guchun Chen (2):
  drm/amdgpu: refine reboot debugfs operation in ras case (v3)
  drm/amdgpu: define macros for retire page reservation

HaiJun Chang (1):
  drm/amdgpu: fix gfx VF FLR test fail on navi

James Zhu (1):
  drm/amdgpu/vcn: Enable VCN2.5 encoding

Jane Jian (1):
  drm/amdgpu: add VCN0 and VCN1 needed headers

Jiange Zhao (1):
  drm/amdgpu/SRIOV: SRIOV VF doesn't support BACO

Jordan Lazare (1):
  drm/amd/display: Remove superfluous assert

Joshua Aberback (1):
  drm/amd/display: Apply vactive dram clock change workaround to dcn2 DMLv2

Jun Lei (4):
  drm/amd/display: add 50us buffer as WA for pstate switch in active
  drm/amd/display: add odm visual confirm
  drm/amd/display: add flag to allow diag to force enumerate edp
  drm/amd/display: do not synchronize "drr" displays

Krunoslav Kovac (1):
  drm/amd/display: Only use EETF when maxCL > max display

Kyle Mahlkuch (1):
  drm/radeon: Fix EEH during kexec

Le Ma (3):
  drm/amdgpu: clear UVD VCPU buffer when err_event_athub generated
  drm/amdgpu: bypass some cleanup work after err_event_athub (v2)
  drm/amdgpu: fix no ACK from LDS read during stress test for Arcturus

Leo Li (1):
  drm/amdgpu: Add DC feature mask to disable fractional pwm

Lewis Huang (1):
  drm/amd/display: take signal type from link

Marek Olšák (1):
  drm/amdgpu: Allow reading more status registers on si/cik

Michael Strauss (3):
  drm/amd/display: Fix MPO & pipe split on 3-pipe dcn2x
  drm/amd/display: Passive DP->HDMI dongle detection fix
  drm/amd/display: Disable force_single_disp_pipe_split on DCN2+

Nathan Chancellor (1):
  drm/amd/display: Add a conversion function for transmitter and phy_id 
enums

Nicholas

Re: [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert

2019-11-01 Thread Boris Ostrovsky
On 11/1/19 1:48 PM, Jason Gunthorpe wrote:
> On Wed, Oct 30, 2019 at 12:55:37PM -0400, Boris Ostrovsky wrote:
>> On 10/28/19 4:10 PM, Jason Gunthorpe wrote:
>>> From: Jason Gunthorpe 
>>>
>>> gntdev simply wants to monitor a specific VMA for any notifier events,
>>> this can be done straightforwardly using mmu_range_notifier_insert() over
>>> the VMA's VA range.
>>>
>>> The notifier should be attached until the original VMA is destroyed.
>>>
>>> It is unclear if any of this is even sane, but at least a lot of duplicate
>>> code is removed.
>> I didn't have a chance to look at the patch itself yet but as a heads-up
>> --- it crashes dom0.
> Thanks Boris. I spent a bit of time and got a VM running with a xen
> 4.9 hypervisor and a kernel with this patch series. It a ubuntu bionic
> VM with the distro's xen stuff.
>
> Can you give some guidance how you made it crash? 

It crashes trying to dereference mrn->ops->invalidate in
mn_itree_invalidate() when a guest exits.

I don't think you've initialized notifier ops. I don't see you using
gntdev_mmu_ops anywhere.

-boris


> I see the VM
> autoloaded gntdev:
>
> Module  Size  Used by
> xen_gntdev 24576  2
> xen_evtchn 16384  1
> xenfs  16384  1
> xen_privcmd24576  16 xenfs
>
> And lsof says several xen processes have the chardev open:
>
> xenstored  819 root   13u  CHR  10,53  
> 0t0  19595 /dev/xen/gntdev
> xenconsol  857 root8u  CHR  10,53  
> 0t0  19595 /dev/xen/gntdev
> xenconsol  857 860 root8u  CHR  10,53  
> 0t0  19595 /dev/xen/gntdev
>
> But no crashing..
>
> However, I wasn't able to get my usual debug kernel .config to boot
> with the xen hypervisor, it crashes on early boot with:
>
> (XEN) Dom0 has maximum 8 VCPUs
> (XEN) Scrubbing Free RAM on 1 nodes using 8 CPUs
> (XEN) .done.
> (XEN) Initial low memory virq threshold set at 0x1000 pages.
> (XEN) Std. Loglevel: All
> (XEN) Guest Loglevel: All
> (XEN) *** Serial input -> DOM0 (type 'CTRL-a' three times to switch input to 
> Xen)
> (XEN) Freed 468kB init memory
> (XEN) d0v0 Unhandled page fault fault/trap [#14, ec=0002]
> (XEN) Pagetable walk from fbfff0480fbe:
> (XEN)  L4[0x1f7] =  
> (XEN) domain_crash_sync called from entry.S: fault at 82d080348a06 
> entry.o#create_bounce_frame+0x135/0x15f
> (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
> (XEN) [ Xen-4.9.2  x86_64  debug=n   Not tainted ]
> (XEN) CPU:0
> (XEN) RIP:e033:[]
> (XEN) RFLAGS: 0296   EM: 1   CONTEXT: pv guest (d0v0)
> (XEN) rax: fbfff0480fbe   rbx:    rcx: c101
> (XEN) rdx:    rsi: 84026000   rdi: 82cb4a20
> (XEN) rbp: 82407ff8   rsp: 82407da0   r8:  
> (XEN) r9:     r10:    r11: 
> (XEN) r12:    r13: 10480fbe   r14: 
> (XEN) r15:    cr0: 8005003b   cr4: 003506e0
> (XEN) cr3: 34027000   cr2: fbfff0480fbe
> (XEN) fsb:    gsb: 82b61000   gss: 
> (XEN) ds:    es:    fs:    gs:    ss: e02b   cs: e033
>
> Which is surely some .config issue, but I didn't figure out what.
>
> Jason

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

Re: [PATCH v2 08/15] xen/gntdev: Use select for DMA_SHARED_BUFFER

2019-11-01 Thread Jason Gunthorpe
On Mon, Oct 28, 2019 at 05:10:25PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe 
> 
> DMA_SHARED_BUFFER can not be enabled by the user (it represents a library
> set in the kernel). The kconfig convention is to use select for such
> symbols so they are turned on implicitly when the user enables a kconfig
> that needs them.
> 
> Otherwise the XEN_GNTDEV_DMABUF kconfig is overly difficult to enable.
> 
> Fixes: 932d6562179e ("xen/gntdev: Add initial support for dma-buf UAPI")
> Cc: Oleksandr Andrushchenko 
> Cc: Boris Ostrovsky 
> Cc: xen-de...@lists.xenproject.org
> Cc: Juergen Gross 
> Cc: Stefano Stabellini 
> Reviewed-by: Juergen Gross 
> Reviewed-by: Oleksandr Andrushchenko 
> Signed-off-by: Jason Gunthorpe 
> ---
>  drivers/xen/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Juergen/Oleksandr/Xen Maintainers:

Would you take this patch through a xen related tree? The only reason
I had in this series is to make it easier to compile-test the gntdev
changes.

Since it is looking like the gntdev rework might not make it this
cycle it is probably best for you to take it.

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

[PATCH v2a 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Jason Gunthorpe
Convert the collision-retry lock around hmm_range_fault to use the one now
provided by the mmu_range notifier.

Although this driver does not seem to use the collision retry lock that
hmm provides correctly, it can still be converted over to use the
mmu_range_notifier api instead of hmm_mirror without too much trouble.

This also deletes another place where a driver is associating additional
data (struct amdgpu_mn) with a mmu_struct.

Signed-off-by: Philip Yang 
Reviewed-by: Philip Yang 
Tested-by: Philip Yang 
Signed-off-by: Jason Gunthorpe 
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|  14 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 150 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h|  49 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 116 +-
 5 files changed, 96 insertions(+), 237 deletions(-)

Philip, here is what it loos like after combining the two patches, thanks

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 47700302a08b7f..1bcedb9b477dce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1738,6 +1738,10 @@ static int update_invalid_user_pages(struct 
amdkfd_process_info *process_info,
return ret;
}
 
+   /*
+* FIXME: Cannot ignore the return code, must hold
+* notifier_lock
+*/
amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm);
 
/* Mark the BO as valid unless it was invalidated
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 82823d9a8ba887..22c989bca7514c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -603,8 +603,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
e->tv.num_shared = 2;
 
amdgpu_bo_list_get_list(p->bo_list, &p->validated);
-   if (p->bo_list->first_userptr != p->bo_list->num_entries)
-   p->mn = amdgpu_mn_get(p->adev, AMDGPU_MN_TYPE_GFX);
 
INIT_LIST_HEAD(&duplicates);
amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
@@ -1287,11 +1285,11 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
if (r)
goto error_unlock;
 
-   /* No memory allocation is allowed while holding the mn lock.
-* p->mn is hold until amdgpu_cs_submit is finished and fence is added
-* to BOs.
+   /* No memory allocation is allowed while holding the notifier lock.
+* The lock is held until amdgpu_cs_submit is finished and fence is
+* added to BOs.
 */
-   amdgpu_mn_lock(p->mn);
+   mutex_lock(&p->adev->notifier_lock);
 
/* If userptr are invalidated after amdgpu_cs_parser_bos(), return
 * -EAGAIN, drmIoctl in libdrm will restart the amdgpu_cs_ioctl.
@@ -1334,13 +1332,13 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
amdgpu_vm_move_to_lru_tail(p->adev, &fpriv->vm);
 
ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence);
-   amdgpu_mn_unlock(p->mn);
+   mutex_unlock(&p->adev->notifier_lock);
 
return 0;
 
 error_abort:
drm_sched_job_cleanup(&job->base);
-   amdgpu_mn_unlock(p->mn);
+   mutex_unlock(&p->adev->notifier_lock);
 
 error_unlock:
amdgpu_job_free(job);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index ac74320b71e4e7..f7be34907e54f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -50,28 +50,6 @@
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 
-/**
- * amdgpu_mn_lock - take the write side lock for this notifier
- *
- * @mn: our notifier
- */
-void amdgpu_mn_lock(struct amdgpu_mn *mn)
-{
-   if (mn)
-   down_write(&mn->lock);
-}
-
-/**
- * amdgpu_mn_unlock - drop the write side lock for this notifier
- *
- * @mn: our notifier
- */
-void amdgpu_mn_unlock(struct amdgpu_mn *mn)
-{
-   if (mn)
-   up_write(&mn->lock);
-}
-
 /**
  * amdgpu_mn_invalidate_gfx - callback to notify about mm change
  *
@@ -82,16 +60,20 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
  * potentially dirty.
  */
 static bool amdgpu_mn_invalidate_gfx(struct mmu_range_notifier *mrn,
-const struct mmu_notifier_range *range)
+const struct mmu_notifier_range *range,
+unsigned long cur_seq)
 {
struct amdgpu_bo *bo = container_of(mrn, struct amdgpu_bo, notifier);
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
long r;
 
-   if (!mmu_notifier_range_blockable(range))
+   if (mmu_notifier_range_blockable(range))
+   m

Re: [PATCH] drm/amdgpu: implement TMZ accessor

2019-11-01 Thread Tuikov, Luben
On 2019-11-01 14:14, Alex Deucher wrote:
> On Thu, Oct 31, 2019 at 8:15 PM Tuikov, Luben  wrote:
>>
>> On 2019-10-31 3:29 a.m., Christian König wrote:
>>> Am 31.10.19 um 00:43 schrieb Tuikov, Luben:
 Implement an accessor of adev->tmz.enabled. Let not
 code around access it as "if (adev->tmz.enabled)"
 as the organization may change. Instead...

 Recruit "bool amdgpu_is_tmz(adev)" to return
 exactly this Boolean value. That is, this function
 is now an accessor of an already initialized and
 set adev and adev->tmz.

 Add "void amdgpu_tmz_set(adev)" to check and set
 adev->tmz.* at initialization time. After which
 one uses "bool amdgpu_is_tmz(adev)" to query
 whether adev supports TMZ.
>>>
>>> Actually I would rather completely remove the amdgpu_tmz.[hc] files. See
>>> TMZ is a feature and not a hardware block.
>>>
>>> All that stuff should probably moved into the PSP handling, since the
>>> control of TMZ is enabled or disabled in the hardware is there.
>>
>> Hi Christian,
>>
>> Thanks for reviewing this. Sure, we can do that as well, should
>> there be consensus on it.
>>
>> I just saw myself needing to know if the device is TMZ (as well
>> as if a buffer is TMZ for which I used amdgpu_bo_encrypted())
>> and so it became natural to write (after this patch):
>>
>> if (!amdgpu_bo_encrypted(abo) || !amdgpu_is_tmz(adev)) {
>> /* normal processing */
>> } else {
>> /* TMZ processing */
>> }
>>
>> BTW, should we proceed as you suggested, do you see
>> those primitives going into psp_v12_0.c?
>>
> 
> They are not psp version specific.  the PSP handles the security, but
> it's not really involved much from a driver perspective; they are
> really more of a memory management thing.  I'd suggest putting them in
> struct amdgpu_gmc.

Thanks Alex. So then I'd get rid of the files and consolidate
into struct amdgpu_gmc, with the files disappearing as Christian
suggested.

Regards,
Luben

> 
> Alex
> 
>> Regards,
>> Luben
>>
>>>
>>> Regards,
>>> Christian.
>>>

 Also, remove circular header file include.

 Signed-off-by: Luben Tuikov 
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  5 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c| 23 +++---
   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h|  7 ++-
   4 files changed, 19 insertions(+), 18 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 index 7d1e528cc783..23bd81a76570 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 @@ -1249,5 +1249,10 @@ _name##_show(struct device *dev,
   \
  \
   static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)

 +static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 +{
 +return adev->tmz.enabled;
 +}
 +
   #endif

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 index 4eee40b9d0b0..f12b817480bb 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -1058,7 +1058,7 @@ static int amdgpu_device_check_arguments(struct 
 amdgpu_device *adev)

  adev->firmware.load_type = amdgpu_ucode_get_load_type(adev, 
 amdgpu_fw_load_type);

 -adev->tmz.enabled = amdgpu_is_tmz(adev);
 +amdgpu_tmz_set(adev);

  return ret;
   }
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
 index 823527a0fa47..518b9d335550 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
 @@ -27,26 +27,25 @@
   #include "amdgpu.h"
   #include "amdgpu_tmz.h"

 -
   /**
 - * amdgpu_is_tmz - validate trust memory zone
 - *
 + * amdgpu_tmz_set -- check and set if a device supports TMZ
* @adev: amdgpu_device pointer
*
 - * Return true if @dev supports trusted memory zones (TMZ), and return 
 false if
 - * @dev does not support TMZ.
 + * Check and set if an the device @adev supports Trusted Memory
 + * Zones (TMZ).
*/
 -bool amdgpu_is_tmz(struct amdgpu_device *adev)
 +void amdgpu_tmz_set(struct amdgpu_device *adev)
   {
  if (!amdgpu_tmz)
 -return false;
 +return;

 -if (adev->asic_type < CHIP_RAVEN || adev->asic_type == CHIP_ARCTURUS) 
 {
 -dev_warn(adev->dev, "doesn't support trusted memory zones 
 (TMZ)\n");
 -return false;
 +if (adev->asic_type < CHIP_RAVEN ||
 +adev->asic_type == CHIP_ARCTURUS) {
 + 

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Jason Gunthorpe
On Fri, Nov 01, 2019 at 02:44:51PM +, Yang, Philip wrote:
> @@ -854,12 +853,20 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, 
> struct page **pages)
>   r = -EPERM;
>   goto out_unlock;
>   }
> + up_read(&mm->mmap_sem);
> + timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> +
> +retry:
> + range->notifier_seq = mmu_range_read_begin(&bo->notifier);
>  
> + down_read(&mm->mmap_sem);
>   r = hmm_range_fault(range, 0);
>   up_read(&mm->mmap_sem);
> -
> - if (unlikely(r < 0))
> + if (unlikely(r <= 0)) {
> + if ((r == 0 || r == -EBUSY) && !time_after(jiffies, timeout))
> + goto retry;
>   goto out_free_pfns;
> + }

I was reflecting on why this suddently became necessary, and I think
what might be happening is that hmm_range_fault() is trigging
invalidations as it runs (ie it is faulting in pages or something) and
that in turn causes the mrn to need retry.

The hmm version of this had a bug where a full
invalidate_range_start/end pair would not trigger retry, so this this
didn't happen.

This is unfortunate as the retry is unnecessary, but at this time I
can't think of a good way to separate an ignorable synchronous
invalidation caused by hmm_range_fault from an async one that cannot
be ignored..

A basic fix would be to not update the mrq seq in the notifier if
the invalidate is triggered by hmm_range_fault, but that seems
difficult to determine..

Any thoughts Jerome?

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

Re: [PATCH] drm/amdgpu: implement TMZ accessor

2019-11-01 Thread Alex Deucher
On Thu, Oct 31, 2019 at 8:15 PM Tuikov, Luben  wrote:
>
> On 2019-10-31 3:29 a.m., Christian König wrote:
> > Am 31.10.19 um 00:43 schrieb Tuikov, Luben:
> >> Implement an accessor of adev->tmz.enabled. Let not
> >> code around access it as "if (adev->tmz.enabled)"
> >> as the organization may change. Instead...
> >>
> >> Recruit "bool amdgpu_is_tmz(adev)" to return
> >> exactly this Boolean value. That is, this function
> >> is now an accessor of an already initialized and
> >> set adev and adev->tmz.
> >>
> >> Add "void amdgpu_tmz_set(adev)" to check and set
> >> adev->tmz.* at initialization time. After which
> >> one uses "bool amdgpu_is_tmz(adev)" to query
> >> whether adev supports TMZ.
> >
> > Actually I would rather completely remove the amdgpu_tmz.[hc] files. See
> > TMZ is a feature and not a hardware block.
> >
> > All that stuff should probably moved into the PSP handling, since the
> > control of TMZ is enabled or disabled in the hardware is there.
>
> Hi Christian,
>
> Thanks for reviewing this. Sure, we can do that as well, should
> there be consensus on it.
>
> I just saw myself needing to know if the device is TMZ (as well
> as if a buffer is TMZ for which I used amdgpu_bo_encrypted())
> and so it became natural to write (after this patch):
>
> if (!amdgpu_bo_encrypted(abo) || !amdgpu_is_tmz(adev)) {
> /* normal processing */
> } else {
> /* TMZ processing */
> }
>
> BTW, should we proceed as you suggested, do you see
> those primitives going into psp_v12_0.c?
>

They are not psp version specific.  the PSP handles the security, but
it's not really involved much from a driver perspective; they are
really more of a memory management thing.  I'd suggest putting them in
struct amdgpu_gmc.

Alex

> Regards,
> Luben
>
> >
> > Regards,
> > Christian.
> >
> >>
> >> Also, remove circular header file include.
> >>
> >> Signed-off-by: Luben Tuikov 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  5 +
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c| 23 +++---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h|  7 ++-
> >>   4 files changed, 19 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index 7d1e528cc783..23bd81a76570 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -1249,5 +1249,10 @@ _name##_show(struct device *dev,
> >>   \
> >>  \
> >>   static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
> >>
> >> +static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
> >> +{
> >> +return adev->tmz.enabled;
> >> +}
> >> +
> >>   #endif
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 4eee40b9d0b0..f12b817480bb 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -1058,7 +1058,7 @@ static int amdgpu_device_check_arguments(struct 
> >> amdgpu_device *adev)
> >>
> >>  adev->firmware.load_type = amdgpu_ucode_get_load_type(adev, 
> >> amdgpu_fw_load_type);
> >>
> >> -adev->tmz.enabled = amdgpu_is_tmz(adev);
> >> +amdgpu_tmz_set(adev);
> >>
> >>  return ret;
> >>   }
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
> >> index 823527a0fa47..518b9d335550 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
> >> @@ -27,26 +27,25 @@
> >>   #include "amdgpu.h"
> >>   #include "amdgpu_tmz.h"
> >>
> >> -
> >>   /**
> >> - * amdgpu_is_tmz - validate trust memory zone
> >> - *
> >> + * amdgpu_tmz_set -- check and set if a device supports TMZ
> >>* @adev: amdgpu_device pointer
> >>*
> >> - * Return true if @dev supports trusted memory zones (TMZ), and return 
> >> false if
> >> - * @dev does not support TMZ.
> >> + * Check and set if an the device @adev supports Trusted Memory
> >> + * Zones (TMZ).
> >>*/
> >> -bool amdgpu_is_tmz(struct amdgpu_device *adev)
> >> +void amdgpu_tmz_set(struct amdgpu_device *adev)
> >>   {
> >>  if (!amdgpu_tmz)
> >> -return false;
> >> +return;
> >>
> >> -if (adev->asic_type < CHIP_RAVEN || adev->asic_type == CHIP_ARCTURUS) 
> >> {
> >> -dev_warn(adev->dev, "doesn't support trusted memory zones 
> >> (TMZ)\n");
> >> -return false;
> >> +if (adev->asic_type < CHIP_RAVEN ||
> >> +adev->asic_type == CHIP_ARCTURUS) {
> >> +dev_warn(adev->dev, "Trusted Memory Zone (TMZ) feature not 
> >> supported\n");
> >> +return;
> >>  }
> >>
> >> -dev_info(adev->dev, "TMZ feature is enabled\n");
> >> +adev->tmz.enabled = true;
> >>
> >> 

Re: [PATCH 3/3] drm/amd/display: rename DCN1_0 kconfig to DCN

2019-11-01 Thread Deucher, Alexander
Series is:
Reviewed-by: Alex Deucher 

From: amd-gfx  on behalf of Bhawanpreet 
Lakha 
Sent: Friday, November 1, 2019 2:05 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Lakha, Bhawanpreet 
Subject: [PATCH 3/3] drm/amd/display: rename DCN1_0 kconfig to DCN

Since dcn20 and dcn21 are under dcn1 it doesnt make sense to
have it named dcn1.

Change it to "dcn" to make it generic

Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/Kconfig   |  4 ++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  8 
 drivers/gpu/drm/amd/display/dc/Makefile   |  4 ++--
 .../display/dc/bios/command_table_helper2.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/calcs/Makefile |  2 +-
 .../gpu/drm/amd/display/dc/clk_mgr/Makefile   |  2 +-
 .../gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  8 
 .../gpu/drm/amd/display/dc/core/dc_debug.c|  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c |  2 +-
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 12 +--
 .../gpu/drm/amd/display/dc/core/dc_stream.c   |  4 ++--
 drivers/gpu/drm/amd/display/dc/dc.h   |  2 +-
 .../drm/amd/display/dc/dce/dce_clock_source.c |  2 +-
 .../drm/amd/display/dc/dce/dce_clock_source.h |  2 +-
 drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 10 +-
 .../amd/display/dc/dce/dce_stream_encoder.c   | 20 +--
 .../display/dc/dce110/dce110_hw_sequencer.c   |  2 +-
 .../gpu/drm/amd/display/dc/dcn10/dcn10_dwb.c  |  2 +-
 .../gpu/drm/amd/display/dc/dcn10/dcn10_dwb.h  |  2 +-
 drivers/gpu/drm/amd/display/dc/dml/Makefile   |  4 ++--
 drivers/gpu/drm/amd/display/dc/gpio/Makefile  |  2 +-
 .../gpu/drm/amd/display/dc/gpio/hw_factory.c  |  4 ++--
 .../drm/amd/display/dc/gpio/hw_translate.c|  4 ++--
 .../gpu/drm/amd/display/dc/inc/core_types.h   |  6 +++---
 drivers/gpu/drm/amd/display/dc/inc/hw/dwb.h   |  2 +-
 drivers/gpu/drm/amd/display/dc/irq/Makefile   |  2 +-
 .../gpu/drm/amd/display/dc/irq/irq_service.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/os_types.h |  2 +-
 29 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index b5a9bfe8998c..78f40690a109 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,13 +6,13 @@ config DRM_AMD_DC
 bool "AMD DC - Enable new display engine"
 default y
 select SND_HDA_COMPONENT if SND_HDA_CORE
-   select DRM_AMD_DC_DCN1_0 if X86 && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
+   select DRM_AMD_DC_DCN if X86 && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
 help
   Choose this option if you want to use the new display engine
   support for AMDGPU. This adds required support for Vega and
   Raven ASICs.

-config DRM_AMD_DC_DCN1_0
+config DRM_AMD_DC_DCN
 def_bool n
 help
   Raven, Navi and Renoir family support for display engine
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 441ad43ce9a9..72e7a1245bd8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -72,7 +72,7 @@
 #include 
 #include 

-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"

 #include "dcn/dcn_1_0_offset.h"
@@ -1866,7 +1866,7 @@ static int dce110_register_irq_handlers(struct 
amdgpu_device *adev)
 return 0;
 }

-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 /* Register IRQ sources and initialize IRQ callbacks */
 static int dcn10_register_irq_handlers(struct amdgpu_device *adev)
 {
@@ -2455,7 +2455,7 @@ static int amdgpu_dm_initialize_drm_device(struct 
amdgpu_device *adev)
 goto fail;
 }
 break;
-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 case CHIP_RAVEN:
 case CHIP_NAVI12:
 case CHIP_NAVI10:
@@ -2679,7 +2679,7 @@ static int dm_early_init(void *handle)
 adev->mode_info.num_hpd = 6;
 adev->mode_info.num_dig = 6;
 break;
-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 case CHIP_RAVEN:
 adev->mode_info.num_crtc = 4;
 adev->mode_info.num_hpd = 4;
diff --git a/drivers/gpu/drm/amd/display/dc/Makefile 
b/drivers/gpu/drm/amd/display/dc/Makefile
index 1feba4190284..ee9b83e5c51a 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -25,7 +25,7 @@

 DC_LIBS = basics bios calcs clk_mgr dce gpio irq virtual

-ifdef CONFIG_DRM_AMD_DC_DCN1_0
+ifdef CONFIG_DRM_AMD_DC_DCN
 DC_LIBS += dcn20
 DC_LIBS += dsc
 DC_LIBS += dcn10 dml
@@ -50,7 +50,7 @@ include $(AMD_DC)
 DISPLAY_CORE = dc.o dc

[PATCH 2/3] drm/amd/display: Drop CONFIG_DRM_AMD_DC_DCN2_1 flag

2019-11-01 Thread Bhawanpreet Lakha
[Why]

DCN21 is stable enough to be build by default. So drop the flags.

[How]

Remove them using the unifdef tool. The following commands were executed
in sequence:

$ find -name '*.c' -exec unifdef -m -DCONFIG_DRM_AMD_DC_DCN2_1 
-UCONFIG_TRIM_DRM_AMD_DC_DCN2_1 '{}' ';'
$ find -name '*.h' -exec unifdef -m -DCONFIG_DRM_AMD_DC_DCN2_1 
-UCONFIG_TRIM_DRM_AMD_DC_DCN2_1 '{}' ';'

In addition:

* Remove from kconfig, and replace any dependencies with DCN1_0.
* Remove from any makefiles.
* Fix and cleanup Renoir definitions in dal_asic_id.h
* Expand DCN1 ifdef to include DCN21 code in the following files:
* clk_mgr/clk_mgr.c: dc_clk_mgr_create()
* core/dc_resources.c: dc_create_resource_pool()
* gpio/hw_factory.c: dal_hw_factory_init()
* gpio/hw_translate.c: dal_hw_translate_init()

Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/Kconfig| 18 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  6 --
 .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c   |  4 
 drivers/gpu/drm/amd/display/dc/Makefile|  3 ---
 .../display/dc/bios/command_table_helper2.c|  2 --
 .../gpu/drm/amd/display/dc/clk_mgr/Makefile|  2 --
 .../gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c   |  4 
 drivers/gpu/drm/amd/display/dc/core/dc.c   |  2 --
 .../gpu/drm/amd/display/dc/core/dc_resource.c  |  6 --
 drivers/gpu/drm/amd/display/dc/dc.h|  2 --
 .../drm/amd/display/dc/dce/dce_clock_source.h  |  2 --
 drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c  |  6 --
 drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.h  |  2 --
 drivers/gpu/drm/amd/display/dc/dce/dce_hwseq.h |  4 
 .../drm/amd/display/dc/dcn10/dcn10_hubbub.h|  8 
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c  |  2 --
 .../gpu/drm/amd/display/dc/dcn20/dcn20_hubp.h  | 16 
 .../drm/amd/display/dc/dcn20/dcn20_resource.c  |  8 
 .../drm/amd/display/dc/dcn21/dcn21_resource.c  |  2 --
 drivers/gpu/drm/amd/display/dc/dm_pp_smu.h |  4 
 drivers/gpu/drm/amd/display/dc/dml/Makefile|  4 
 .../drm/amd/display/dc/dml/display_mode_lib.c  |  6 --
 .../drm/amd/display/dc/dml/display_mode_lib.h  |  2 --
 drivers/gpu/drm/amd/display/dc/gpio/Makefile   |  5 +++--
 .../display/dc/gpio/dcn21/hw_factory_dcn21.h   |  2 --
 .../display/dc/gpio/dcn21/hw_translate_dcn21.h |  2 --
 .../gpu/drm/amd/display/dc/gpio/hw_factory.c   |  4 
 .../gpu/drm/amd/display/dc/gpio/hw_translate.c |  4 
 .../gpu/drm/amd/display/dc/inc/core_types.h|  4 
 .../gpu/drm/amd/display/dc/inc/hw/clk_mgr.h|  6 --
 .../gpu/drm/amd/display/dc/inc/hw/mem_input.h  |  2 --
 .../gpu/drm/amd/display/dc/inc/hw_sequencer.h  |  2 --
 drivers/gpu/drm/amd/display/dc/irq/Makefile|  2 --
 .../gpu/drm/amd/display/include/dal_asic_id.h  |  2 --
 .../gpu/drm/amd/display/include/dal_types.h|  2 --
 35 files changed, 4 insertions(+), 148 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 0b4c71dc0447..b5a9bfe8998c 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -15,23 +15,7 @@ config DRM_AMD_DC
 config DRM_AMD_DC_DCN1_0
def_bool n
help
- RV and NV family support for display engine
-
-config DRM_AMD_DC_DCN2_1
-   bool "DCN 2.1 family"
-   depends on DRM_AMD_DC && X86
-   help
- Choose this option if you want to have
- Renoir support for display engine
-
-config DRM_AMD_DC_DSC_SUPPORT
-   bool "DSC support"
-   default y
-   depends on DRM_AMD_DC && X86
-   depends on DRM_AMD_DC_DCN1_0
-   help
- Choose this option if you want to have
- Dynamic Stream Compression support
+ Raven, Navi and Renoir family support for display engine
 
 config DRM_AMD_DC_HDCP
bool "Enable HDCP support in DC"
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 b0005313e9a9..441ad43ce9a9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2460,9 +2460,7 @@ static int amdgpu_dm_initialize_drm_device(struct 
amdgpu_device *adev)
case CHIP_NAVI12:
case CHIP_NAVI10:
case CHIP_NAVI14:
-#if defined(CONFIG_DRM_AMD_DC_DCN2_1)
case CHIP_RENOIR:
-#endif
if (dcn10_register_irq_handlers(dm->adev)) {
DRM_ERROR("DM: Failed to initialize IRQ\n");
goto fail;
@@ -2699,13 +2697,11 @@ static int dm_early_init(void *handle)
adev->mode_info.num_hpd = 5;
adev->mode_info.num_dig = 5;
break;
-#if defined(CONFIG_DRM_AMD_DC_DCN2_1)
case CHIP_RENOIR:
adev->mode_info.num_crtc = 4;
adev->mode_info.num_hpd = 4;
adev->mode_info.num_dig = 4;
break;
-#endif
default:
   

[PATCH 3/3] drm/amd/display: rename DCN1_0 kconfig to DCN

2019-11-01 Thread Bhawanpreet Lakha
Since dcn20 and dcn21 are under dcn1 it doesnt make sense to
have it named dcn1.

Change it to "dcn" to make it generic

Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/Kconfig   |  4 ++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  8 
 drivers/gpu/drm/amd/display/dc/Makefile   |  4 ++--
 .../display/dc/bios/command_table_helper2.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/calcs/Makefile |  2 +-
 .../gpu/drm/amd/display/dc/clk_mgr/Makefile   |  2 +-
 .../gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  8 
 .../gpu/drm/amd/display/dc/core/dc_debug.c|  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c |  2 +-
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 12 +--
 .../gpu/drm/amd/display/dc/core/dc_stream.c   |  4 ++--
 drivers/gpu/drm/amd/display/dc/dc.h   |  2 +-
 .../drm/amd/display/dc/dce/dce_clock_source.c |  2 +-
 .../drm/amd/display/dc/dce/dce_clock_source.h |  2 +-
 drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 10 +-
 .../amd/display/dc/dce/dce_stream_encoder.c   | 20 +--
 .../display/dc/dce110/dce110_hw_sequencer.c   |  2 +-
 .../gpu/drm/amd/display/dc/dcn10/dcn10_dwb.c  |  2 +-
 .../gpu/drm/amd/display/dc/dcn10/dcn10_dwb.h  |  2 +-
 drivers/gpu/drm/amd/display/dc/dml/Makefile   |  4 ++--
 drivers/gpu/drm/amd/display/dc/gpio/Makefile  |  2 +-
 .../gpu/drm/amd/display/dc/gpio/hw_factory.c  |  4 ++--
 .../drm/amd/display/dc/gpio/hw_translate.c|  4 ++--
 .../gpu/drm/amd/display/dc/inc/core_types.h   |  6 +++---
 drivers/gpu/drm/amd/display/dc/inc/hw/dwb.h   |  2 +-
 drivers/gpu/drm/amd/display/dc/irq/Makefile   |  2 +-
 .../gpu/drm/amd/display/dc/irq/irq_service.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/os_types.h |  2 +-
 29 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index b5a9bfe8998c..78f40690a109 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,13 +6,13 @@ config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
select SND_HDA_COMPONENT if SND_HDA_CORE
-   select DRM_AMD_DC_DCN1_0 if X86 && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
+   select DRM_AMD_DC_DCN if X86 && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and
  Raven ASICs.
 
-config DRM_AMD_DC_DCN1_0
+config DRM_AMD_DC_DCN
def_bool n
help
  Raven, Navi and Renoir family support for display engine
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 441ad43ce9a9..72e7a1245bd8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -72,7 +72,7 @@
 #include 
 #include 
 
-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
 
 #include "dcn/dcn_1_0_offset.h"
@@ -1866,7 +1866,7 @@ static int dce110_register_irq_handlers(struct 
amdgpu_device *adev)
return 0;
 }
 
-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
 /* Register IRQ sources and initialize IRQ callbacks */
 static int dcn10_register_irq_handlers(struct amdgpu_device *adev)
 {
@@ -2455,7 +2455,7 @@ static int amdgpu_dm_initialize_drm_device(struct 
amdgpu_device *adev)
goto fail;
}
break;
-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
case CHIP_RAVEN:
case CHIP_NAVI12:
case CHIP_NAVI10:
@@ -2679,7 +2679,7 @@ static int dm_early_init(void *handle)
adev->mode_info.num_hpd = 6;
adev->mode_info.num_dig = 6;
break;
-#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
+#if defined(CONFIG_DRM_AMD_DC_DCN)
case CHIP_RAVEN:
adev->mode_info.num_crtc = 4;
adev->mode_info.num_hpd = 4;
diff --git a/drivers/gpu/drm/amd/display/dc/Makefile 
b/drivers/gpu/drm/amd/display/dc/Makefile
index 1feba4190284..ee9b83e5c51a 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -25,7 +25,7 @@
 
 DC_LIBS = basics bios calcs clk_mgr dce gpio irq virtual
 
-ifdef CONFIG_DRM_AMD_DC_DCN1_0
+ifdef CONFIG_DRM_AMD_DC_DCN
 DC_LIBS += dcn20
 DC_LIBS += dsc
 DC_LIBS += dcn10 dml
@@ -50,7 +50,7 @@ include $(AMD_DC)
 DISPLAY_CORE = dc.o dc_link.o dc_resource.o dc_hw_sequencer.o dc_sink.o \
 dc_surface.o dc_link_hwss.o dc_link_dp.o dc_link_ddc.o dc_debug.o dc_stream.o
 
-ifdef CONFIG_DRM_AMD_DC_DCN1_0
+ifdef CONFIG_DRM_AMD_DC_DCN
 DISPLAY_CORE += dc_vm_helper.o
 endif
 
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_help

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Jason Gunthorpe
On Fri, Nov 01, 2019 at 03:59:26PM +, Yang, Philip wrote:
> > This test for range_blockable should be before mutex_lock, I can move
> > it up
> > 
> yes, thanks.

Okay, I wrote it like this:

if (mmu_notifier_range_blockable(range))
mutex_lock(&adev->notifier_lock);
else if (!mutex_trylock(&adev->notifier_lock))
return false;

> > Also, do you know if notifier_lock is held while calling
> > amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held'
> > to amdgpu_ttm_tt_get_user_pages_done()?
> 
> gpu side hold notifier_lock but kfd side doesn't. kfd side doesn't check 
> amdgpu_ttm_tt_get_user_pages_done/mmu_range_read_retry return value but 
> check mem->invalid flag which is updated from invalidate callback. It 
> takes more time to change, I will come to another patch to fix it later.

Ah.. confusing, OK, I'll let you sort that

> > However, this is all pre-existing bugs, so I'm OK go ahead with this
> > patch as modified. I advise AMD to make a followup patch ..
> > 
> yes, I will.

While you are here, this is also wrong:

int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
{
down_read(&mm->mmap_sem);
r = hmm_range_fault(range, 0);
up_read(&mm->mmap_sem);
if (unlikely(r <= 0)) {
if ((r == 0 || r == -EBUSY) && !time_after(jiffies, timeout))
goto retry;
goto out_free_pfns;
}

for (i = 0; i < ttm->num_pages; i++) {
pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);

It is not allowed to read the results of hmm_range_fault() outside
locking, and in particular, we can't convert to a struct page.

This must be done inside the notifier_lock, after checking
mmu_range_read_retry(), all handling of the struct page must be
structured like that.

> >> @@ -997,10 +1004,18 @@ static void amdgpu_ttm_tt_unpin_userptr(struct 
> >> ttm_tt *ttm)
> >>sg_free_table(ttm->sg);
> >>   
> >>   #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
> >> -  if (gtt->range &&
> >> -  ttm->pages[0] == hmm_device_entry_to_page(gtt->range,
> >> -gtt->range->pfns[0]))
> >> -  WARN_ONCE(1, "Missing get_user_page_done\n");
> >> +  if (gtt->range) {
> >> +  unsigned long i;
> >> +
> >> +  for (i = 0; i < ttm->num_pages; i++) {
> >> +  if (ttm->pages[i] !=
> >> +  hmm_device_entry_to_page(gtt->range,
> >> +gtt->range->pfns[i]))
> >> +  break;
> >> +  }
> >> +
> >> +  WARN((i == ttm->num_pages), "Missing get_user_page_done\n");
> >> +  }
> > 
> > Is this related/necessary? I can put it in another patch if it is just
> > debugging improvement? Please advise
> > 
> I see this WARN backtrace now, but I didn't see it before. This is 
> somehow related.

Hm, might be instructive to learn what is going on..

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

Re: [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert

2019-11-01 Thread Jason Gunthorpe
On Wed, Oct 30, 2019 at 12:55:37PM -0400, Boris Ostrovsky wrote:
> On 10/28/19 4:10 PM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe 
> >
> > gntdev simply wants to monitor a specific VMA for any notifier events,
> > this can be done straightforwardly using mmu_range_notifier_insert() over
> > the VMA's VA range.
> >
> > The notifier should be attached until the original VMA is destroyed.
> >
> > It is unclear if any of this is even sane, but at least a lot of duplicate
> > code is removed.
> 
> I didn't have a chance to look at the patch itself yet but as a heads-up
> --- it crashes dom0.

Thanks Boris. I spent a bit of time and got a VM running with a xen
4.9 hypervisor and a kernel with this patch series. It a ubuntu bionic
VM with the distro's xen stuff.

Can you give some guidance how you made it crash? I see the VM
autoloaded gntdev:

Module  Size  Used by
xen_gntdev 24576  2
xen_evtchn 16384  1
xenfs  16384  1
xen_privcmd24576  16 xenfs

And lsof says several xen processes have the chardev open:

xenstored  819 root   13u  CHR  10,53  0t0  
19595 /dev/xen/gntdev
xenconsol  857 root8u  CHR  10,53  0t0  
19595 /dev/xen/gntdev
xenconsol  857 860 root8u  CHR  10,53  0t0  
19595 /dev/xen/gntdev

But no crashing..

However, I wasn't able to get my usual debug kernel .config to boot
with the xen hypervisor, it crashes on early boot with:

(XEN) Dom0 has maximum 8 VCPUs
(XEN) Scrubbing Free RAM on 1 nodes using 8 CPUs
(XEN) .done.
(XEN) Initial low memory virq threshold set at 0x1000 pages.
(XEN) Std. Loglevel: All
(XEN) Guest Loglevel: All
(XEN) *** Serial input -> DOM0 (type 'CTRL-a' three times to switch input to 
Xen)
(XEN) Freed 468kB init memory
(XEN) d0v0 Unhandled page fault fault/trap [#14, ec=0002]
(XEN) Pagetable walk from fbfff0480fbe:
(XEN)  L4[0x1f7] =  
(XEN) domain_crash_sync called from entry.S: fault at 82d080348a06 
entry.o#create_bounce_frame+0x135/0x15f
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) [ Xen-4.9.2  x86_64  debug=n   Not tainted ]
(XEN) CPU:0
(XEN) RIP:e033:[]
(XEN) RFLAGS: 0296   EM: 1   CONTEXT: pv guest (d0v0)
(XEN) rax: fbfff0480fbe   rbx:    rcx: c101
(XEN) rdx:    rsi: 84026000   rdi: 82cb4a20
(XEN) rbp: 82407ff8   rsp: 82407da0   r8:  
(XEN) r9:     r10:    r11: 
(XEN) r12:    r13: 10480fbe   r14: 
(XEN) r15:    cr0: 8005003b   cr4: 003506e0
(XEN) cr3: 34027000   cr2: fbfff0480fbe
(XEN) fsb:    gsb: 82b61000   gss: 
(XEN) ds:    es:    fs:    gs:    ss: e02b   cs: e033

Which is surely some .config issue, but I didn't figure out what.

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

[PATCH] drivers: gpu: amdgpu: Remove @dev from amdgpu_gem_prime_export documentation

2019-11-01 Thread Jaskaran Singh
The function does not have a "dev" argument, so remove it from
the documentation. This fixes the following documentation
build warning:

./drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c:335:
warning: Excess function parameter 'dev' description in
'amdgpu_gem_prime_export'

Signed-off-by: Jaskaran Singh 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 61f108ec2b5c..4917b548b7f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -321,7 +321,6 @@ const struct dma_buf_ops amdgpu_dmabuf_ops = {
 
 /**
  * amdgpu_gem_prime_export - &drm_driver.gem_prime_export implementation
- * @dev: DRM device
  * @gobj: GEM BO
  * @flags: Flags such as DRM_CLOEXEC and DRM_RDWR.
  *
-- 
2.21.0

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

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Jason Gunthorpe
On Fri, Nov 01, 2019 at 02:44:51PM +, Yang, Philip wrote:
> 
> 
> On 2019-10-29 3:25 p.m., Jason Gunthorpe wrote:
> > On Tue, Oct 29, 2019 at 07:22:37PM +, Yang, Philip wrote:
> >> Hi Jason,
> >>
> >> I did quick test after merging amd-staging-drm-next with the
> >> mmu_notifier branch, which includes this set changes. The test result
> >> has different failures, app stuck intermittently, GUI no display etc. I
> >> am understanding the changes and will try to figure out the cause.
> > 
> > Thanks! I'm not surprised by this given how difficult this patch was
> > to make. Let me know if I can assist in any way
> > 
> > Please ensure to run with lockdep enabled.. Your symptops sounds sort
> > of like deadlocking?
> > 
> Hi Jason,
> 
> Attached patch fix several issues in amdgpu driver, maybe you can squash 
> this into patch 14. With this is done, patch 12, 13, 14 is Reviewed-by 
> and Tested-by Philip Yang 

Wow, this is great thanks! Can you clarify what the problems you found
were? Was the bug the 'return !r' below?

I'll also add your signed off by

Here are some remarks:

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> index cb718a064eb4..c8bbd06f1009 100644
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> @@ -67,21 +67,15 @@ static bool amdgpu_mn_invalidate_gfx(struct 
> mmu_range_notifier *mrn,
>   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>   long r;
>  
> - /*
> -  * FIXME: Must hold some lock shared with
> -  * amdgpu_ttm_tt_get_user_pages_done()
> -  */
> - mmu_range_set_seq(mrn, cur_seq);
> + mutex_lock(&adev->notifier_lock);
>  
> - /* FIXME: Is this necessary? */
> - if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, range->start,
> -   range->end))
> - return true;
> + mmu_range_set_seq(mrn, cur_seq);
>  
> - if (!mmu_notifier_range_blockable(range))
> + if (!mmu_notifier_range_blockable(range)) {
> + mutex_unlock(&adev->notifier_lock);
>   return false;

This test for range_blockable should be before mutex_lock, I can move
it up

Also, do you know if notifier_lock is held while calling
amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held'
to amdgpu_ttm_tt_get_user_pages_done()?

> @@ -854,12 +853,20 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, 
> struct page **pages)
>   r = -EPERM;
>   goto out_unlock;
>   }
> + up_read(&mm->mmap_sem);
> + timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> +
> +retry:
> + range->notifier_seq = mmu_range_read_begin(&bo->notifier);
>  
> + down_read(&mm->mmap_sem);
>   r = hmm_range_fault(range, 0);
>   up_read(&mm->mmap_sem);
> -
> - if (unlikely(r < 0))
> + if (unlikely(r <= 0)) {
> + if ((r == 0 || r == -EBUSY) && !time_after(jiffies, timeout))
> + goto retry;
>   goto out_free_pfns;
> + }

This isn't really right, a retry loop like this needs to go all the
way to mmu_range_read_retry() and done under the notifier_lock. ie
mmu_range_read_retry() can fail just as likely as hmm_range_fault()
can, and drivers are supposed to retry in both cases, with a single
timeout.

AFAICT it is a major bug that many places ignore the return code of
amdgpu_ttm_tt_get_user_pages_done() ???

However, this is all pre-existing bugs, so I'm OK go ahead with this
patch as modified. I advise AMD to make a followup patch ..

I'll add a FIXME note to this effect.

>   for (i = 0; i < ttm->num_pages; i++) {
>   pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);
> @@ -916,7 +923,7 @@ bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm)
>   gtt->range = NULL;
>   }
>  
> - return r;
> + return !r;

Ah is this the major error? hmm_range_valid() is inverted vs
mmu_range_read_retry()?

>  }
>  #endif
>  
> @@ -997,10 +1004,18 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt 
> *ttm)
>   sg_free_table(ttm->sg);
>  
>  #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
> - if (gtt->range &&
> - ttm->pages[0] == hmm_device_entry_to_page(gtt->range,
> -   gtt->range->pfns[0]))
> - WARN_ONCE(1, "Missing get_user_page_done\n");
> + if (gtt->range) {
> + unsigned long i;
> +
> + for (i = 0; i < ttm->num_pages; i++) {
> + if (ttm->pages[i] !=
> + hmm_device_entry_to_page(gtt->range,
> +   gtt->range->pfns[i]))
> + break;
> + }
> +
> + WARN((i == ttm->num_pages), "Missing get_user_page_done\n");
> + }

Is this related/necessary? I can put it in another patch if it is just
debugging improvement? Please advise

Thanks a lot,
Jason
__

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Yang, Philip


On 2019-11-01 11:12 a.m., Jason Gunthorpe wrote:
> On Fri, Nov 01, 2019 at 02:44:51PM +, Yang, Philip wrote:
>>
>>
>> On 2019-10-29 3:25 p.m., Jason Gunthorpe wrote:
>>> On Tue, Oct 29, 2019 at 07:22:37PM +, Yang, Philip wrote:
 Hi Jason,

 I did quick test after merging amd-staging-drm-next with the
 mmu_notifier branch, which includes this set changes. The test result
 has different failures, app stuck intermittently, GUI no display etc. I
 am understanding the changes and will try to figure out the cause.
>>>
>>> Thanks! I'm not surprised by this given how difficult this patch was
>>> to make. Let me know if I can assist in any way
>>>
>>> Please ensure to run with lockdep enabled.. Your symptops sounds sort
>>> of like deadlocking?
>>>
>> Hi Jason,
>>
>> Attached patch fix several issues in amdgpu driver, maybe you can squash
>> this into patch 14. With this is done, patch 12, 13, 14 is Reviewed-by
>> and Tested-by Philip Yang 
> 
> Wow, this is great thanks! Can you clarify what the problems you found
> were? Was the bug the 'return !r' below?
> 
Yes. return !r is critical one, and retry if hmm_range_fault return 
-EBUSY is needed too.

> I'll also add your signed off by
> 
> Here are some remarks:
> 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> index cb718a064eb4..c8bbd06f1009 100644
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> @@ -67,21 +67,15 @@ static bool amdgpu_mn_invalidate_gfx(struct 
>> mmu_range_notifier *mrn,
>>  struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>  long r;
>>   
>> -/*
>> - * FIXME: Must hold some lock shared with
>> - * amdgpu_ttm_tt_get_user_pages_done()
>> - */
>> -mmu_range_set_seq(mrn, cur_seq);
>> +mutex_lock(&adev->notifier_lock);
>>   
>> -/* FIXME: Is this necessary? */
>> -if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, range->start,
>> -  range->end))
>> -return true;
>> +mmu_range_set_seq(mrn, cur_seq);
>>   
>> -if (!mmu_notifier_range_blockable(range))
>> +if (!mmu_notifier_range_blockable(range)) {
>> +mutex_unlock(&adev->notifier_lock);
>>  return false;
> 
> This test for range_blockable should be before mutex_lock, I can move
> it up
> 
yes, thanks.
> Also, do you know if notifier_lock is held while calling
> amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held'
> to amdgpu_ttm_tt_get_user_pages_done()?
> 
gpu side hold notifier_lock but kfd side doesn't. kfd side doesn't check 
amdgpu_ttm_tt_get_user_pages_done/mmu_range_read_retry return value but 
check mem->invalid flag which is updated from invalidate callback. It 
takes more time to change, I will come to another patch to fix it later.

>> @@ -854,12 +853,20 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, 
>> struct page **pages)
>>  r = -EPERM;
>>  goto out_unlock;
>>  }
>> +up_read(&mm->mmap_sem);
>> +timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>> +
>> +retry:
>> +range->notifier_seq = mmu_range_read_begin(&bo->notifier);
>>   
>> +down_read(&mm->mmap_sem);
>>  r = hmm_range_fault(range, 0);
>>  up_read(&mm->mmap_sem);
>> -
>> -if (unlikely(r < 0))
>> +if (unlikely(r <= 0)) {
>> +if ((r == 0 || r == -EBUSY) && !time_after(jiffies, timeout))
>> +goto retry;
>>  goto out_free_pfns;
>> +}
> 
> This isn't really right, a retry loop like this needs to go all the
> way to mmu_range_read_retry() and done under the notifier_lock. ie
> mmu_range_read_retry() can fail just as likely as hmm_range_fault()
> can, and drivers are supposed to retry in both cases, with a single
> timeout.
> 
For gpu, check mmu_range_read_retry return value under the notifier_lock 
to do retry is in seperate location, not in same retry loop.

> AFAICT it is a major bug that many places ignore the return code of
> amdgpu_ttm_tt_get_user_pages_done() ???
>
For kfd, explained above.

> However, this is all pre-existing bugs, so I'm OK go ahead with this
> patch as modified. I advise AMD to make a followup patch ..
> 
yes, I will.
> I'll add a FIXME note to this effect.
> 
>>  for (i = 0; i < ttm->num_pages; i++) {
>>  pages[i] = hmm_device_entry_to_page(range, range->pfns[i]);
>> @@ -916,7 +923,7 @@ bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt 
>> *ttm)
>>  gtt->range = NULL;
>>  }
>>   
>> -return r;
>> +return !r;
> 
> Ah is this the major error? hmm_range_valid() is inverted vs
> mmu_range_read_retry()?
> 
yes.
>>   }
>>   #endif
>>   
>> @@ -997,10 +1004,18 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt 
>> *ttm)
>>  sg_free_table(ttm->sg);
>>   
>>   #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
>> -if (gtt->range &&
>> -ttm->pages[0] == hmm_device_entry_to_page(gtt->range,
>> -

Re: [PATCH] drm/amdgpu: Show resolution correctly in mode validation debug output

2019-11-01 Thread Neil Mayhew
On 2019-11-01 9:13 a.m., Harry Wentland wrote:
> On 2019-10-30 2:58 p.m., n...@neil.mayhew.name wrote:
>> From: Neil Mayhew 
> This requires your Signed-off-by. See [1].
>
> With that fixed your change looks good and is
> Reviewed-by: Harry Wentland 
>
> You can simply reply to this email with your Signed-off-by and I can add
> it when merging, or you can send a v2 patch with your Signed-off-by.

Signed-off-by: Neil Mayhew 

Thanks, Harry.

> [1]
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
>
> Harry
>
>> ---
>>  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 a52f0b13a2c8..f802c784e6f6 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4114,8 +4114,8 @@ enum drm_mode_status 
>> amdgpu_dm_connector_mode_valid(struct drm_connector *connec
>>  result = MODE_OK;
>>  else
>>  DRM_DEBUG_KMS("Mode %dx%d (clk %d) failed DC validation with 
>> error %d\n",
>> -  mode->vdisplay,
>>mode->hdisplay,
>> +  mode->vdisplay,
>>mode->clock,
>>dc_result);
>>  
>>

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

RE: [PATCH] drm/amdgpu: Need to free discovery memory

2019-11-01 Thread Zhang, Hawking
Besides, the bo for ip_discovery only need to be created and reserved for 
Navi10 and onwards, although it shouldn't be a big issue to reserve 64K memory 
in top vram.

Regards,
Hawking

-Original Message-
From: amd-gfx  On Behalf Of Yuan, Xiaojie
Sent: 2019年11月1日 23:14
To: Deng, Emily ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Need to free discovery memory

Hi Emily,

Thanks for catching this.

I think freeing Discovery TMR bo should be put at amdgpu_ttm_fini() instead of 
amdgpu_ttm_late_init() because unlike VGA stolen bo, touching PSP-protected 
Discovery TMR bo will cause GPU hang. Therefore, it should be reserved across 
the life-cycle of amdgpu driver.

Please kindly send v2 patch with this change.

BR,
Xiaojie


From: amd-gfx  on behalf of Emily Deng 

Sent: Friday, November 1, 2019 5:07 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deng, Emily
Subject: [PATCH] drm/amdgpu: Need to free discovery memory

When unloading driver, need to free discovery memory.

Signed-off-by: Emily Deng 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 9f2a893..50d6ed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1091,8 +1091,11 @@ static int gmc_v9_0_sw_fini(void *handle)
amdgpu_gem_force_release(adev);
amdgpu_vm_manager_fini(adev);

-   if (gmc_v9_0_keep_stolen_memory(adev))
+   if (gmc_v9_0_keep_stolen_memory(adev)) {
amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, 
&stolen_vga_buf);
+   /* return the IP Discovery TMR memory back to VRAM */
+   amdgpu_bo_free_kernel(&adev->discovery_memory, NULL, NULL);
+   }

amdgpu_gart_table_vram_free(adev);
amdgpu_bo_fini(adev);
--
2.7.4

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

Re: [PATCH] drm/amdgpu: Need to free discovery memory

2019-11-01 Thread Yuan, Xiaojie
Hi Emily,

Thanks for catching this.

I think freeing Discovery TMR bo should be put at amdgpu_ttm_fini() instead of 
amdgpu_ttm_late_init() because unlike VGA stolen bo, touching PSP-protected 
Discovery TMR bo will cause GPU hang. Therefore, it should be reserved across 
the life-cycle of amdgpu driver.

Please kindly send v2 patch with this change.

BR,
Xiaojie


From: amd-gfx  on behalf of Emily Deng 

Sent: Friday, November 1, 2019 5:07 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deng, Emily
Subject: [PATCH] drm/amdgpu: Need to free discovery memory

When unloading driver, need to free discovery memory.

Signed-off-by: Emily Deng 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 9f2a893..50d6ed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1091,8 +1091,11 @@ static int gmc_v9_0_sw_fini(void *handle)
amdgpu_gem_force_release(adev);
amdgpu_vm_manager_fini(adev);

-   if (gmc_v9_0_keep_stolen_memory(adev))
+   if (gmc_v9_0_keep_stolen_memory(adev)) {
amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, 
&stolen_vga_buf);
+   /* return the IP Discovery TMR memory back to VRAM */
+   amdgpu_bo_free_kernel(&adev->discovery_memory, NULL, NULL);
+   }

amdgpu_gart_table_vram_free(adev);
amdgpu_bo_fini(adev);
--
2.7.4

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

Re: [PATCH] drm/amdgpu: Show resolution correctly in mode validation debug output

2019-11-01 Thread Harry Wentland
On 2019-10-30 2:58 p.m., n...@neil.mayhew.name wrote:
> From: Neil Mayhew 

This requires your Signed-off-by. See [1].

With that fixed your change looks good and is
Reviewed-by: Harry Wentland 

You can simply reply to this email with your Signed-off-by and I can add
it when merging, or you can send a v2 patch with your Signed-off-by.

[1]
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Harry

> 
> ---
>  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 a52f0b13a2c8..f802c784e6f6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4114,8 +4114,8 @@ enum drm_mode_status 
> amdgpu_dm_connector_mode_valid(struct drm_connector *connec
>   result = MODE_OK;
>   else
>   DRM_DEBUG_KMS("Mode %dx%d (clk %d) failed DC validation with 
> error %d\n",
> -   mode->vdisplay,
> mode->hdisplay,
> +   mode->vdisplay,
> mode->clock,
> dc_result);
>  
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier instead of hmm_mirror

2019-11-01 Thread Yang, Philip


On 2019-10-29 3:25 p.m., Jason Gunthorpe wrote:
> On Tue, Oct 29, 2019 at 07:22:37PM +, Yang, Philip wrote:
>> Hi Jason,
>>
>> I did quick test after merging amd-staging-drm-next with the
>> mmu_notifier branch, which includes this set changes. The test result
>> has different failures, app stuck intermittently, GUI no display etc. I
>> am understanding the changes and will try to figure out the cause.
> 
> Thanks! I'm not surprised by this given how difficult this patch was
> to make. Let me know if I can assist in any way
> 
> Please ensure to run with lockdep enabled.. Your symptops sounds sort
> of like deadlocking?
> 
Hi Jason,

Attached patch fix several issues in amdgpu driver, maybe you can squash 
this into patch 14. With this is done, patch 12, 13, 14 is Reviewed-by 
and Tested-by Philip Yang 

Regards,
Philip

> Regards,
> Jason
> 
From 5a0bd4d8cef8472fe2904550142d288feed8cd81 Mon Sep 17 00:00:00 2001
From: Philip Yang 
Date: Thu, 31 Oct 2019 09:10:30 -0400
Subject: [PATCH] drm/amdgpu: issues with new mmu_range_notifier api

put mmu_range_set_seq under the same lock which is used to call
mmu_range_read_retry.

fix amdgpu_ttm_tt_get_user_pages_done return value, because
mmu_range_read_retry means !hmm_range_valid

retry if hmm_range_fault return -EBUSY

fix false WARN for missing get_user_page_done, we should check all
pages not just the first page, don't understand why this issue is
triggered by this change.

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 32 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 37 +
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index cb718a064eb4..c8bbd06f1009 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -67,21 +67,15 @@ static bool amdgpu_mn_invalidate_gfx(struct mmu_range_notifier *mrn,
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 	long r;
 
-	/*
-	 * FIXME: Must hold some lock shared with
-	 * amdgpu_ttm_tt_get_user_pages_done()
-	 */
-	mmu_range_set_seq(mrn, cur_seq);
+	mutex_lock(&adev->notifier_lock);
 
-	/* FIXME: Is this necessary? */
-	if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, range->start,
-	  range->end))
-		return true;
+	mmu_range_set_seq(mrn, cur_seq);
 
-	if (!mmu_notifier_range_blockable(range))
+	if (!mmu_notifier_range_blockable(range)) {
+		mutex_unlock(&adev->notifier_lock);
 		return false;
+	}
 
-	mutex_lock(&adev->notifier_lock);
 	r = dma_resv_wait_timeout_rcu(bo->tbo.base.resv, true, false,
   MAX_SCHEDULE_TIMEOUT);
 	mutex_unlock(&adev->notifier_lock);
@@ -110,21 +104,15 @@ static bool amdgpu_mn_invalidate_hsa(struct mmu_range_notifier *mrn,
 	struct amdgpu_bo *bo = container_of(mrn, struct amdgpu_bo, notifier);
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 
-	/*
-	 * FIXME: Must hold some lock shared with
-	 * amdgpu_ttm_tt_get_user_pages_done()
-	 */
-	mmu_range_set_seq(mrn, cur_seq);
+	mutex_lock(&adev->notifier_lock);
 
-	/* FIXME: Is this necessary? */
-	if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, range->start,
-	  range->end))
-		return true;
+	mmu_range_set_seq(mrn, cur_seq);
 
-	if (!mmu_notifier_range_blockable(range))
+	if (!mmu_notifier_range_blockable(range)) {
+		mutex_unlock(&adev->notifier_lock);
 		return false;
+	}
 
-	mutex_lock(&adev->notifier_lock);
 	amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
 	mutex_unlock(&adev->notifier_lock);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index a38437fd290a..56fde43d5efa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -799,10 +799,11 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
 {
 	struct ttm_tt *ttm = bo->tbo.ttm;
 	struct amdgpu_ttm_tt *gtt = (void *)ttm;
-	struct mm_struct *mm;
-	struct hmm_range *range;
 	unsigned long start = gtt->userptr;
 	struct vm_area_struct *vma;
+	struct hmm_range *range;
+	unsigned long timeout;
+	struct mm_struct *mm;
 	unsigned long i;
 	int r = 0;
 
@@ -841,8 +842,6 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
 		goto out_free_ranges;
 	}
 
-	range->notifier_seq = mmu_range_read_begin(&bo->notifier);
-
 	down_read(&mm->mmap_sem);
 	vma = find_vma(mm, start);
 	if (unlikely(!vma || start < vma->vm_start)) {
@@ -854,12 +853,20 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
 		r = -EPERM;
 		goto out_unlock;
 	}
+	up_read(&mm->mmap_sem);
+	timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+
+retry:
+	range->notifier_seq = mmu_range_read_begin(&bo->notifier);
 
+	down_read(&mm->mmap_sem);
 	r = hmm_range_fault(range, 0);
 	up_read(&mm->mmap_sem);
-
-	if (unlikely(r < 0))
+	if (unlikely(r <= 0)) {
+		if ((r == 0 || r == -EBUSY) && !time_after(

Re: [PATCH 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6

2019-11-01 Thread Jani Nikula
On Wed, 30 Oct 2019, "Jerry (Fangzhi) Zuo"  wrote:
> DP 1.4 edid corruption test requires source DUT to write calculated
> CRC, not the corrupted CRC from reference sink.
>
> Return the calculated CRC back, and initiate the required sequence.
>
> Signed-off-by: Jerry (Fangzhi) Zuo 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 36 
>  drivers/gpu/drm/drm_edid.c  | 15 ---
>  include/drm/drm_connector.h |  7 +++
>  include/drm/drm_dp_helper.h |  3 +++
>  4 files changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ffc68d305afe..75dbd30c62a7 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> +/**
> +  * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
> +  * @aux: DisplayPort AUX channel
> +  * @bad_edid_checksum: real edid checksum for the last block
> +  *
> +  * Returns true on success
> +  */
> +bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
> +u8 bad_edid_checksum)
> +{
> +u8 link_edid_read = 0, auto_test_req = 0;
> +u8 test_resp = 0;
> +
> +drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 
> 1);
> +auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
> +
> +drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_edid_read, 1);
> +link_edid_read &= DP_TEST_LINK_EDID_READ;
> +
> +if (!auto_test_req || !link_edid_read) {
> +DRM_DEBUG_KMS("Source DUT does not support 
> TEST_EDID_READ\n");
> +return false;
> +}
> +
> +drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 
> 1);
> +
> +/* send back checksum for the last edid extension block data */
> +drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &bad_edid_checksum, 1);
> +
> +test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
> +drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1);
> +
> +return true;
> +}
> +EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
> +
>  /**
>   * drm_dp_link_probe() - probe a DisplayPort link for capabilities
>   * @aux: DisplayPort AUX channel
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 82a4ceed3fcf..400064dcc010 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1344,13 +1344,19 @@ static void drm_get_displayid(struct drm_connector 
> *connector,
> struct edid *edid);
>  static int validate_displayid(u8 *displayid, int length, int idx);
>  
> -static int drm_edid_block_checksum(const u8 *raw_edid)
> +static int drm_edid_block_checksum(const u8 *raw_edid, bool c)
>  {
>   int i;
>   u8 csum = 0;
> - for (i = 0; i < EDID_LENGTH; i++)
> + u8 len;
> +
> + len = c ? EDID_LENGTH : (EDID_LENGTH - 1);
> +
> + for (i = 0; i < len; i++)
>   csum += raw_edid[i];
>  
> + csum = c ? csum : (0x100 - csum);
> +
>   return csum;
>  }

I think a mysterious boolean 'c' argument just makes this more
confusing.

Please split the function to two, one that calculates the checksum (c ==
false case) and another that uses the function (c == true case). The
latter would return the same value as the above.

Please do this as a prep patch without any of the other modifications.

BR,
Jani.



>  
> @@ -1408,7 +1414,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
> print_bad_edid,
>   }
>   }
>  
> - csum = drm_edid_block_checksum(raw_edid);
> + csum = drm_edid_block_checksum(raw_edid, true);
>   if (csum) {
>   if (edid_corrupt)
>   *edid_corrupt = true;
> @@ -1572,6 +1578,9 @@ static void connector_bad_edid(struct drm_connector 
> *connector,
>  prefix, DUMP_PREFIX_NONE, 16, 1,
>  block, EDID_LENGTH, false);
>   }
> +
> + /* Calculate real checksum for the last edid extension block data */
> + connector->bad_edid_checksum = drm_edid_block_checksum(edid + 
> edid[0x7e] * EDID_LENGTH, false);
>  }
>  
>  /* Get override or firmware EDID */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8442461542b9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1345,6 +1345,13 @@ struct drm_connector {
>* rev1.1 4.2.2.6
>*/
>   bool edid_corrupt;
> + /**
> + * @bad_edid_checksum: real edid checksum value for corrupted edid 
> block.
> + * Required in Displayport 1.4 compliance testing
> + * rev1.1 4.2.2.6
> + */
> +uint8_t bad_edid_checksum;
> +
>  
>   /** @debugfs_entry: debugfs directory for this connector */
>   struct de

[PATCH 5/5] drm: drop DRM_AUTH from PRIME_TO/FROM_HANDLE ioctls

2019-11-01 Thread Emil Velikov
From: Emil Velikov 

As mentioned by Christian, for drivers which support only primary nodes
this changes the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS.

For others, this check in particular will be a noop. So let's remove it
as suggested by Christian.

Cc: Alex Deucher 
Cc: amd-gfx@lists.freedesktop.org
Cc: Daniel Vetter 
Cc: Sean Paul 
Acked-by: Christian König 
Signed-off-by: Emil Velikov 
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fcd728d7cf72..5afb39688b55 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -652,8 +652,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, 0),
 
-   DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, 
drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-   DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, 
drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, 
drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, 
drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW),
 
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 
0),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0),
-- 
2.23.0

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

[PATCH] drm/amdgpu: Need to free discovery memory

2019-11-01 Thread Emily Deng
When unloading driver, need to free discovery memory.

Signed-off-by: Emily Deng 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 9f2a893..50d6ed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1091,8 +1091,11 @@ static int gmc_v9_0_sw_fini(void *handle)
amdgpu_gem_force_release(adev);
amdgpu_vm_manager_fini(adev);
 
-   if (gmc_v9_0_keep_stolen_memory(adev))
+   if (gmc_v9_0_keep_stolen_memory(adev)) {
amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, 
&stolen_vga_buf);
+   /* return the IP Discovery TMR memory back to VRAM */
+   amdgpu_bo_free_kernel(&adev->discovery_memory, NULL, NULL);
+   }
 
amdgpu_gart_table_vram_free(adev);
amdgpu_bo_fini(adev);
-- 
2.7.4

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