RE: [PATCH] drm/amd/display: Add ENGINE_ID_DIGD condition check for Navi14
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
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
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
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
> + /* 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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[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
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
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
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
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
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
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
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
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
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
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
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
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
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
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