Re: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to psp_check_pmfw_centralized_cstate_management
On Wed, Oct 13, 2021 at 10:29 PM Quan, Evan wrote:
>
> [AMD Official Use Only]
>
>
>
> > -Original Message-
> > From: Alex Deucher
> > Sent: Thursday, October 14, 2021 10:00 AM
> > To: Quan, Evan
> > Cc: Deucher, Alexander ; amd-
> > g...@lists.freedesktop.org
> > Subject: Re: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to
> > psp_check_pmfw_centralized_cstate_management
> >
> > On Wed, Oct 13, 2021 at 9:41 PM Quan, Evan wrote:
> > >
> > > [AMD Official Use Only]
> > >
> > > I assume IP_VERSION(11, 0, 0) and IP_VERSION(11, 0, 5) are for Navi10 and
> > Navi14 respectively.
> > > Then according to the code comment that "
> > pmfw_centralized_cstate_management support is available for Navi12 and
> > onwards only", I think they should be handled by "default" branch. That
> > means this patch seems unnecessary.
> > >
> >
> > The original code was this:
> >if ((adev->asic_type >= CHIP_ARCTURUS) ||
> >(adev->asic_type >= CHIP_NAVI12))
> > psp->pmfw_centralized_cstate_management = true; So navi10
> > and
> > 14 were included. Not sure whether they should have been or not.
> [Quan, Evan] OK. That will make sense. Series is reviewed-by: Evan Quan
>
> Dig a little more about the history.
> It seems at first the centralized_cstate_management was limited to ARCTURUS
> or >= CHIP_NAIV12. Then it was expanded to all ASICs >= CHIP_ ARCTURUS.
> But the code comment was left outdated. Can you get that updated on code
> submit?
> @@ -65,7 +65,6 @@ static int psp_securedisplay_terminate(struct psp_context
> *psp);
> *
> * This new sequence is required for
> * - Arcturus and onwards
> - * - Navi12 and onwards
> */
Will do. Thanks!
Alex
>
> BR
> Evan
> >
> > Alex
> >
> >
> > > Patch1 and 2 are reviewed-by: Evan Quan
> > >
> > > > -Original Message-
> > > > From: amd-gfx On Behalf Of
> > > > Alex Deucher
> > > > Sent: Tuesday, October 12, 2021 11:53 PM
> > > > To: amd-gfx@lists.freedesktop.org
> > > > Cc: Deucher, Alexander
> > > > Subject: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to
> > > > psp_check_pmfw_centralized_cstate_management
> > > >
> > > > Missed a few asics.
> > > >
> > > > Fixes: 82d05736c47b19 ("drm/amdgpu/amdgpu_psp: convert to IP
> > version
> > > > checking")
> > > > Signed-off-by: Alex Deucher
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > > index 6b39e6c02dd8..51620f2fc43a 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > > @@ -77,7 +77,9 @@ static void
> > > > psp_check_pmfw_centralized_cstate_management(struct psp_context
> > *psp
> > > > }
> > > >
> > > > switch (adev->ip_versions[MP0_HWIP][0]) {
> > > > + case IP_VERSION(11, 0, 0):
> > > > case IP_VERSION(11, 0, 4):
> > > > + case IP_VERSION(11, 0, 5):
> > > > case IP_VERSION(11, 0, 7):
> > > > case IP_VERSION(11, 0, 9):
> > > > case IP_VERSION(11, 0, 11):
> > > > --
> > > > 2.31.1
RE: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to psp_check_pmfw_centralized_cstate_management
[AMD Official Use Only]
> -Original Message-
> From: Alex Deucher
> Sent: Thursday, October 14, 2021 10:00 AM
> To: Quan, Evan
> Cc: Deucher, Alexander ; amd-
> g...@lists.freedesktop.org
> Subject: Re: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to
> psp_check_pmfw_centralized_cstate_management
>
> On Wed, Oct 13, 2021 at 9:41 PM Quan, Evan wrote:
> >
> > [AMD Official Use Only]
> >
> > I assume IP_VERSION(11, 0, 0) and IP_VERSION(11, 0, 5) are for Navi10 and
> Navi14 respectively.
> > Then according to the code comment that "
> pmfw_centralized_cstate_management support is available for Navi12 and
> onwards only", I think they should be handled by "default" branch. That
> means this patch seems unnecessary.
> >
>
> The original code was this:
>if ((adev->asic_type >= CHIP_ARCTURUS) ||
>(adev->asic_type >= CHIP_NAVI12))
> psp->pmfw_centralized_cstate_management = true; So navi10 and
> 14 were included. Not sure whether they should have been or not.
[Quan, Evan] OK. That will make sense. Series is reviewed-by: Evan Quan
Dig a little more about the history.
It seems at first the centralized_cstate_management was limited to ARCTURUS or
>= CHIP_NAIV12. Then it was expanded to all ASICs >= CHIP_ ARCTURUS.
But the code comment was left outdated. Can you get that updated on code submit?
@@ -65,7 +65,6 @@ static int psp_securedisplay_terminate(struct psp_context
*psp);
*
* This new sequence is required for
* - Arcturus and onwards
- * - Navi12 and onwards
*/
BR
Evan
>
> Alex
>
>
> > Patch1 and 2 are reviewed-by: Evan Quan
> >
> > > -Original Message-
> > > From: amd-gfx On Behalf Of
> > > Alex Deucher
> > > Sent: Tuesday, October 12, 2021 11:53 PM
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander
> > > Subject: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to
> > > psp_check_pmfw_centralized_cstate_management
> > >
> > > Missed a few asics.
> > >
> > > Fixes: 82d05736c47b19 ("drm/amdgpu/amdgpu_psp: convert to IP
> version
> > > checking")
> > > Signed-off-by: Alex Deucher
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > index 6b39e6c02dd8..51620f2fc43a 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > @@ -77,7 +77,9 @@ static void
> > > psp_check_pmfw_centralized_cstate_management(struct psp_context
> *psp
> > > }
> > >
> > > switch (adev->ip_versions[MP0_HWIP][0]) {
> > > + case IP_VERSION(11, 0, 0):
> > > case IP_VERSION(11, 0, 4):
> > > + case IP_VERSION(11, 0, 5):
> > > case IP_VERSION(11, 0, 7):
> > > case IP_VERSION(11, 0, 9):
> > > case IP_VERSION(11, 0, 11):
> > > --
> > > 2.31.1
RE: bf756fb833cb ("drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend")
[AMD Official Use Only]
> -Original Message-
> From: Borislav Petkov
> Sent: Wednesday, October 13, 2021 5:26 PM
> To: Quan, Evan
> Cc: Alex Deucher ; amd-gfx list g...@lists.freedesktop.org>; LKML ; Deucher,
> Alexander ; Pan, Xinhui
> ; Chen, Guchun
> Subject: Re: bf756fb833cb ("drm/amdgpu: add missing cleanups for Polaris12
> UVD/VCE on suspend")
>
> On Wed, Oct 13, 2021 at 09:19:45AM +, Quan, Evan wrote:
> > So, I need your help to confirm the last two patches(I sent you) do not
> affect the fix for the bug above.
> > Please follow the steps below to verify it:
> > 1. Launch a video playing
> > 2. open another terminal and issue "sudo pm-suspend" command to force
> > suspending 3. verify the system can suspend and resume back correctly
> > without errors or hangs
>
> Just to confirm: you want me to do that with the last two patches applied?
[Quan, Evan] Yes, but not(apply them) at the same time. One by one as you did
before.
- try the patch1 first
- undo the changes of patch1 and try patch2
BR
Evan
>
> --
> Regards/Gruss,
> Boris.
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeo
> ple.kernel.org%2Ftglx%2Fnotes-about-
> netiquettedata=04%7C01%7CEvan.Quan%40amd.com%7Cbcabc6c3a5
> 07426172a308d98e2b8235%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> C0%7C637697139813202149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
> p;sdata=7%2F3TXqlIld%2BdSocRyLsgZBeaFcsEDiI5GEwJ5AHaLSk%3Dre
> served=0
Re: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to psp_check_pmfw_centralized_cstate_management
On Wed, Oct 13, 2021 at 9:41 PM Quan, Evan wrote:
>
> [AMD Official Use Only]
>
> I assume IP_VERSION(11, 0, 0) and IP_VERSION(11, 0, 5) are for Navi10 and
> Navi14 respectively.
> Then according to the code comment that " pmfw_centralized_cstate_management
> support is available for Navi12 and onwards only", I think they should be
> handled by "default" branch. That means this patch seems unnecessary.
>
The original code was this:
if ((adev->asic_type >= CHIP_ARCTURUS) ||
(adev->asic_type >= CHIP_NAVI12))
psp->pmfw_centralized_cstate_management = true;
So navi10 and 14 were included. Not sure whether they should have been or not.
Alex
> Patch1 and 2 are reviewed-by: Evan Quan
>
> > -Original Message-
> > From: amd-gfx On Behalf Of Alex
> > Deucher
> > Sent: Tuesday, October 12, 2021 11:53 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander
> > Subject: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to
> > psp_check_pmfw_centralized_cstate_management
> >
> > Missed a few asics.
> >
> > Fixes: 82d05736c47b19 ("drm/amdgpu/amdgpu_psp: convert to IP version
> > checking")
> > Signed-off-by: Alex Deucher
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index 6b39e6c02dd8..51620f2fc43a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -77,7 +77,9 @@ static void
> > psp_check_pmfw_centralized_cstate_management(struct psp_context
> > *psp
> > }
> >
> > switch (adev->ip_versions[MP0_HWIP][0]) {
> > + case IP_VERSION(11, 0, 0):
> > case IP_VERSION(11, 0, 4):
> > + case IP_VERSION(11, 0, 5):
> > case IP_VERSION(11, 0, 7):
> > case IP_VERSION(11, 0, 9):
> > case IP_VERSION(11, 0, 11):
> > --
> > 2.31.1
RE: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to psp_check_pmfw_centralized_cstate_management
[AMD Official Use Only]
I assume IP_VERSION(11, 0, 0) and IP_VERSION(11, 0, 5) are for Navi10 and
Navi14 respectively.
Then according to the code comment that " pmfw_centralized_cstate_management
support is available for Navi12 and onwards only", I think they should be
handled by "default" branch. That means this patch seems unnecessary.
Patch1 and 2 are reviewed-by: Evan Quan
> -Original Message-
> From: amd-gfx On Behalf Of Alex
> Deucher
> Sent: Tuesday, October 12, 2021 11:53 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander
> Subject: [PATCH 3/3] drm/amdgpu/psp: add some missing cases to
> psp_check_pmfw_centralized_cstate_management
>
> Missed a few asics.
>
> Fixes: 82d05736c47b19 ("drm/amdgpu/amdgpu_psp: convert to IP version
> checking")
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 6b39e6c02dd8..51620f2fc43a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -77,7 +77,9 @@ static void
> psp_check_pmfw_centralized_cstate_management(struct psp_context
> *psp
> }
>
> switch (adev->ip_versions[MP0_HWIP][0]) {
> + case IP_VERSION(11, 0, 0):
> case IP_VERSION(11, 0, 4):
> + case IP_VERSION(11, 0, 5):
> case IP_VERSION(11, 0, 7):
> case IP_VERSION(11, 0, 9):
> case IP_VERSION(11, 0, 11):
> --
> 2.31.1
Re: [PATCH v4] drm/amdkfd: unregistered svm range not overlap with TTM range
Am 2021-10-13 um 7:18 p.m. schrieb Philip Yang:
> When creating unregistered new svm range to recover retry fault, avoid
> new svm range to overlap with ranges or userptr ranges managed by TTM,
> otherwise svm migration will trigger TTM or userptr eviction, to evict
> user queues unexpectedly.
>
> Change helper amdgpu_ttm_tt_affect_userptr to return userptr which is
> inside the range. Add helper svm_range_check_vm_userptr to scan all
> userptr of the vm, and return overlap userptr bo start, last.
>
> Signed-off-by: Philip Yang
Reviewed-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c| 95 +++--
> 3 files changed, 94 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index bd5dda8066fa..d784f8d3a834 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1220,7 +1220,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct
> ttm_tt *ttm)
> *
> */
> bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
> - unsigned long end)
> + unsigned long end, unsigned long *userptr)
> {
> struct amdgpu_ttm_tt *gtt = (void *)ttm;
> unsigned long size;
> @@ -1235,6 +1235,8 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm,
> unsigned long start,
> if (gtt->userptr > end || gtt->userptr + size <= start)
> return false;
>
> + if (userptr)
> + *userptr = gtt->userptr;
> return true;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index ba5c864b8de1..91a087f9dc7c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -182,7 +182,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object
> *bo,
> bool amdgpu_ttm_tt_has_userptr(struct ttm_tt *ttm);
> struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm);
> bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
> - unsigned long end);
> + unsigned long end, unsigned long *userptr);
> bool amdgpu_ttm_tt_userptr_invalidated(struct ttm_tt *ttm,
> int *last_invalidated);
> bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 49c92713c2ad..b691c8495d66 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -50,7 +50,9 @@ static bool
> svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
> const struct mmu_notifier_range *range,
> unsigned long cur_seq);
> -
> +static int
> +svm_range_check_vm(struct kfd_process *p, uint64_t start, uint64_t last,
> +uint64_t *bo_s, uint64_t *bo_l);
> static const struct mmu_interval_notifier_ops svm_range_mn_ops = {
> .invalidate = svm_range_cpu_invalidate_pagetables,
> };
> @@ -2308,6 +2310,7 @@ svm_range_best_restore_location(struct svm_range
> *prange,
>
> return -1;
> }
> +
> static int
> svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
> unsigned long *start, unsigned long *last)
> @@ -2355,8 +2358,59 @@ svm_range_get_range_boundaries(struct kfd_process *p,
> int64_t addr,
> vma->vm_end >> PAGE_SHIFT, *last);
>
> return 0;
> +}
> +
> +static int
> +svm_range_check_vm_userptr(struct kfd_process *p, uint64_t start, uint64_t
> last,
> +uint64_t *bo_s, uint64_t *bo_l)
> +{
> + struct amdgpu_bo_va_mapping *mapping;
> + struct interval_tree_node *node;
> + struct amdgpu_bo *bo = NULL;
> + unsigned long userptr;
> + uint32_t i;
> + int r;
>
> + for (i = 0; i < p->n_pdds; i++) {
> + struct amdgpu_vm *vm;
> +
> + if (!p->pdds[i]->drm_priv)
> + continue;
> +
> + vm = drm_priv_to_vm(p->pdds[i]->drm_priv);
> + r = amdgpu_bo_reserve(vm->root.bo, false);
> + if (r)
> + return r;
> +
> + /* Check userptr by searching entire vm->va interval tree */
> + node = interval_tree_iter_first(>va, 0, ~0ULL);
> + while (node) {
> + mapping = container_of((struct rb_node *)node,
> +struct amdgpu_bo_va_mapping, rb);
> + bo = mapping->bo_va->base.bo;
> +
> + if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
> + start <<
[PATCH v4] drm/amdkfd: unregistered svm range not overlap with TTM range
When creating unregistered new svm range to recover retry fault, avoid
new svm range to overlap with ranges or userptr ranges managed by TTM,
otherwise svm migration will trigger TTM or userptr eviction, to evict
user queues unexpectedly.
Change helper amdgpu_ttm_tt_affect_userptr to return userptr which is
inside the range. Add helper svm_range_check_vm_userptr to scan all
userptr of the vm, and return overlap userptr bo start, last.
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c| 95 +++--
3 files changed, 94 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index bd5dda8066fa..d784f8d3a834 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1220,7 +1220,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt
*ttm)
*
*/
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
- unsigned long end)
+ unsigned long end, unsigned long *userptr)
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
unsigned long size;
@@ -1235,6 +1235,8 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm,
unsigned long start,
if (gtt->userptr > end || gtt->userptr + size <= start)
return false;
+ if (userptr)
+ *userptr = gtt->userptr;
return true;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index ba5c864b8de1..91a087f9dc7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -182,7 +182,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
bool amdgpu_ttm_tt_has_userptr(struct ttm_tt *ttm);
struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm);
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
- unsigned long end);
+ unsigned long end, unsigned long *userptr);
bool amdgpu_ttm_tt_userptr_invalidated(struct ttm_tt *ttm,
int *last_invalidated);
bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 49c92713c2ad..b691c8495d66 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -50,7 +50,9 @@ static bool
svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
const struct mmu_notifier_range *range,
unsigned long cur_seq);
-
+static int
+svm_range_check_vm(struct kfd_process *p, uint64_t start, uint64_t last,
+ uint64_t *bo_s, uint64_t *bo_l);
static const struct mmu_interval_notifier_ops svm_range_mn_ops = {
.invalidate = svm_range_cpu_invalidate_pagetables,
};
@@ -2308,6 +2310,7 @@ svm_range_best_restore_location(struct svm_range *prange,
return -1;
}
+
static int
svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
unsigned long *start, unsigned long *last)
@@ -2355,8 +2358,59 @@ svm_range_get_range_boundaries(struct kfd_process *p,
int64_t addr,
vma->vm_end >> PAGE_SHIFT, *last);
return 0;
+}
+
+static int
+svm_range_check_vm_userptr(struct kfd_process *p, uint64_t start, uint64_t
last,
+ uint64_t *bo_s, uint64_t *bo_l)
+{
+ struct amdgpu_bo_va_mapping *mapping;
+ struct interval_tree_node *node;
+ struct amdgpu_bo *bo = NULL;
+ unsigned long userptr;
+ uint32_t i;
+ int r;
+ for (i = 0; i < p->n_pdds; i++) {
+ struct amdgpu_vm *vm;
+
+ if (!p->pdds[i]->drm_priv)
+ continue;
+
+ vm = drm_priv_to_vm(p->pdds[i]->drm_priv);
+ r = amdgpu_bo_reserve(vm->root.bo, false);
+ if (r)
+ return r;
+
+ /* Check userptr by searching entire vm->va interval tree */
+ node = interval_tree_iter_first(>va, 0, ~0ULL);
+ while (node) {
+ mapping = container_of((struct rb_node *)node,
+ struct amdgpu_bo_va_mapping, rb);
+ bo = mapping->bo_va->base.bo;
+
+ if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
+start << PAGE_SHIFT,
+last << PAGE_SHIFT,
+)) {
+ node = interval_tree_iter_next(node, 0, ~0ULL);
+
Re: [PATCH v3] drm/amdkfd: unregistered svm range not overlap with TTM range
On 2021-10-13 6:33 p.m., Felix Kuehling
wrote:
Am 2021-10-13 um 6:05 p.m. schrieb Philip Yang:
When creating unregistered new svm range to recover retry fault, avoid
new svm range to overlap with ranges or userptr ranges managed by TTM,
otherwise svm migration will trigger TTM or userptr eviction, to evict
user queues unexpectedly.
Change helper amdgpu_ttm_tt_affect_userptr to return userptr which is
inside the range. Add helper svm_range_check_vm_userptr to scan all
userptr of the vm, and return overlap userptr bo start, last.
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c| 93 +++--
3 files changed, 92 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index bd5dda8066fa..d784f8d3a834 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1220,7 +1220,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm)
*
*/
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
- unsigned long end)
+ unsigned long end, unsigned long *userptr)
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
unsigned long size;
@@ -1235,6 +1235,8 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
if (gtt->userptr > end || gtt->userptr + size <= start)
return false;
+ if (userptr)
+ *userptr = gtt->userptr;
return true;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index ba5c864b8de1..91a087f9dc7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -182,7 +182,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
bool amdgpu_ttm_tt_has_userptr(struct ttm_tt *ttm);
struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm);
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
- unsigned long end);
+ unsigned long end, unsigned long *userptr);
bool amdgpu_ttm_tt_userptr_invalidated(struct ttm_tt *ttm,
int *last_invalidated);
bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 49c92713c2ad..95d018fe2287 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -50,7 +50,9 @@ static bool
svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
const struct mmu_notifier_range *range,
unsigned long cur_seq);
-
+static int
+svm_range_check_vm(struct kfd_process *p, uint64_t start, uint64_t last,
+ uint64_t *bo_s, uint64_t *bo_l);
static const struct mmu_interval_notifier_ops svm_range_mn_ops = {
.invalidate = svm_range_cpu_invalidate_pagetables,
};
@@ -2308,6 +2310,7 @@ svm_range_best_restore_location(struct svm_range *prange,
return -1;
}
+
static int
svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
unsigned long *start, unsigned long *last)
@@ -2355,8 +2358,59 @@ svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
vma->vm_end >> PAGE_SHIFT, *last);
return 0;
+}
+
+static int
+svm_range_check_vm_userptr(struct kfd_process *p, uint64_t start, uint64_t last,
+ uint64_t *bo_s, uint64_t *bo_l)
+{
+ struct amdgpu_bo_va_mapping *mapping;
+ struct interval_tree_node *node;
+ struct amdgpu_bo *bo = NULL;
+ unsigned long userptr;
+ uint32_t i;
+ int r;
+ for (i = 0; i < p->n_pdds; i++) {
+ struct amdgpu_vm *vm;
+
+ if (!p->pdds[i]->drm_priv)
+ continue;
+
+ vm = drm_priv_to_vm(p->pdds[i]->drm_priv);
+ r = amdgpu_bo_reserve(vm->root.bo, false);
+ if (r)
+ return r;
+
+ /* Check userptr by searching entire vm->va interval tree */
+ node = interval_tree_iter_first(>va, 0, ~0ULL);
+ while (node) {
+ mapping = container_of((struct rb_node *)node,
+ struct amdgpu_bo_va_mapping, rb);
+ bo = mapping->bo_va->base.bo;
+
+ if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
+ start << PAGE_SHIFT,
+ last << PAGE_SHIFT,
+ )) {
+node = interval_tree_iter_next(node, 0, ~0ULL);
+continue;
+ }
+
+ pr_debug("[0x%llx 0x%llx] already userptr mapped\n",
+ start, last);
+ if (bo_s && bo_l) {
+*bo_s = userptr >> PAGE_SHIFT;
+*bo_l = *bo_s + bo->tbo.ttm->num_pages - 1;
+ }
+ amdgpu_bo_unreserve(vm->root.bo);
+ return -EADDRINUSE;
+ }
+ amdgpu_bo_unreserve(vm->root.bo);
+ }
+ return 0;
}
+
static struct
svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
struct kfd_process *p,
@@ -2366,10 +2420,24 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
struct svm_range *prange = NULL;
unsigned long start, last;
uint32_t gpuid, gpuidx;
+
Re: [PATCH v3] drm/amdkfd: unregistered svm range not overlap with TTM range
Am 2021-10-13 um 6:05 p.m. schrieb Philip Yang:
> When creating unregistered new svm range to recover retry fault, avoid
> new svm range to overlap with ranges or userptr ranges managed by TTM,
> otherwise svm migration will trigger TTM or userptr eviction, to evict
> user queues unexpectedly.
>
> Change helper amdgpu_ttm_tt_affect_userptr to return userptr which is
> inside the range. Add helper svm_range_check_vm_userptr to scan all
> userptr of the vm, and return overlap userptr bo start, last.
>
> Signed-off-by: Philip Yang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c| 93 +++--
> 3 files changed, 92 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index bd5dda8066fa..d784f8d3a834 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1220,7 +1220,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct
> ttm_tt *ttm)
> *
> */
> bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
> - unsigned long end)
> + unsigned long end, unsigned long *userptr)
> {
> struct amdgpu_ttm_tt *gtt = (void *)ttm;
> unsigned long size;
> @@ -1235,6 +1235,8 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm,
> unsigned long start,
> if (gtt->userptr > end || gtt->userptr + size <= start)
> return false;
>
> + if (userptr)
> + *userptr = gtt->userptr;
> return true;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index ba5c864b8de1..91a087f9dc7c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -182,7 +182,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object
> *bo,
> bool amdgpu_ttm_tt_has_userptr(struct ttm_tt *ttm);
> struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm);
> bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
> - unsigned long end);
> + unsigned long end, unsigned long *userptr);
> bool amdgpu_ttm_tt_userptr_invalidated(struct ttm_tt *ttm,
> int *last_invalidated);
> bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 49c92713c2ad..95d018fe2287 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -50,7 +50,9 @@ static bool
> svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
> const struct mmu_notifier_range *range,
> unsigned long cur_seq);
> -
> +static int
> +svm_range_check_vm(struct kfd_process *p, uint64_t start, uint64_t last,
> +uint64_t *bo_s, uint64_t *bo_l);
> static const struct mmu_interval_notifier_ops svm_range_mn_ops = {
> .invalidate = svm_range_cpu_invalidate_pagetables,
> };
> @@ -2308,6 +2310,7 @@ svm_range_best_restore_location(struct svm_range
> *prange,
>
> return -1;
> }
> +
> static int
> svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
> unsigned long *start, unsigned long *last)
> @@ -2355,8 +2358,59 @@ svm_range_get_range_boundaries(struct kfd_process *p,
> int64_t addr,
> vma->vm_end >> PAGE_SHIFT, *last);
>
> return 0;
> +}
> +
> +static int
> +svm_range_check_vm_userptr(struct kfd_process *p, uint64_t start, uint64_t
> last,
> +uint64_t *bo_s, uint64_t *bo_l)
> +{
> + struct amdgpu_bo_va_mapping *mapping;
> + struct interval_tree_node *node;
> + struct amdgpu_bo *bo = NULL;
> + unsigned long userptr;
> + uint32_t i;
> + int r;
>
> + for (i = 0; i < p->n_pdds; i++) {
> + struct amdgpu_vm *vm;
> +
> + if (!p->pdds[i]->drm_priv)
> + continue;
> +
> + vm = drm_priv_to_vm(p->pdds[i]->drm_priv);
> + r = amdgpu_bo_reserve(vm->root.bo, false);
> + if (r)
> + return r;
> +
> + /* Check userptr by searching entire vm->va interval tree */
> + node = interval_tree_iter_first(>va, 0, ~0ULL);
> + while (node) {
> + mapping = container_of((struct rb_node *)node,
> +struct amdgpu_bo_va_mapping, rb);
> + bo = mapping->bo_va->base.bo;
> +
> + if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
> + start << PAGE_SHIFT,
> +
[PATCH v3] drm/amdkfd: unregistered svm range not overlap with TTM range
When creating unregistered new svm range to recover retry fault, avoid
new svm range to overlap with ranges or userptr ranges managed by TTM,
otherwise svm migration will trigger TTM or userptr eviction, to evict
user queues unexpectedly.
Change helper amdgpu_ttm_tt_affect_userptr to return userptr which is
inside the range. Add helper svm_range_check_vm_userptr to scan all
userptr of the vm, and return overlap userptr bo start, last.
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c| 93 +++--
3 files changed, 92 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index bd5dda8066fa..d784f8d3a834 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1220,7 +1220,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt
*ttm)
*
*/
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
- unsigned long end)
+ unsigned long end, unsigned long *userptr)
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
unsigned long size;
@@ -1235,6 +1235,8 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm,
unsigned long start,
if (gtt->userptr > end || gtt->userptr + size <= start)
return false;
+ if (userptr)
+ *userptr = gtt->userptr;
return true;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index ba5c864b8de1..91a087f9dc7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -182,7 +182,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
bool amdgpu_ttm_tt_has_userptr(struct ttm_tt *ttm);
struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm);
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
- unsigned long end);
+ unsigned long end, unsigned long *userptr);
bool amdgpu_ttm_tt_userptr_invalidated(struct ttm_tt *ttm,
int *last_invalidated);
bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 49c92713c2ad..95d018fe2287 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -50,7 +50,9 @@ static bool
svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
const struct mmu_notifier_range *range,
unsigned long cur_seq);
-
+static int
+svm_range_check_vm(struct kfd_process *p, uint64_t start, uint64_t last,
+ uint64_t *bo_s, uint64_t *bo_l);
static const struct mmu_interval_notifier_ops svm_range_mn_ops = {
.invalidate = svm_range_cpu_invalidate_pagetables,
};
@@ -2308,6 +2310,7 @@ svm_range_best_restore_location(struct svm_range *prange,
return -1;
}
+
static int
svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
unsigned long *start, unsigned long *last)
@@ -2355,8 +2358,59 @@ svm_range_get_range_boundaries(struct kfd_process *p,
int64_t addr,
vma->vm_end >> PAGE_SHIFT, *last);
return 0;
+}
+
+static int
+svm_range_check_vm_userptr(struct kfd_process *p, uint64_t start, uint64_t
last,
+ uint64_t *bo_s, uint64_t *bo_l)
+{
+ struct amdgpu_bo_va_mapping *mapping;
+ struct interval_tree_node *node;
+ struct amdgpu_bo *bo = NULL;
+ unsigned long userptr;
+ uint32_t i;
+ int r;
+ for (i = 0; i < p->n_pdds; i++) {
+ struct amdgpu_vm *vm;
+
+ if (!p->pdds[i]->drm_priv)
+ continue;
+
+ vm = drm_priv_to_vm(p->pdds[i]->drm_priv);
+ r = amdgpu_bo_reserve(vm->root.bo, false);
+ if (r)
+ return r;
+
+ /* Check userptr by searching entire vm->va interval tree */
+ node = interval_tree_iter_first(>va, 0, ~0ULL);
+ while (node) {
+ mapping = container_of((struct rb_node *)node,
+ struct amdgpu_bo_va_mapping, rb);
+ bo = mapping->bo_va->base.bo;
+
+ if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
+start << PAGE_SHIFT,
+last << PAGE_SHIFT,
+)) {
+ node = interval_tree_iter_next(node, 0, ~0ULL);
+
[PATCH v4 20/20] drm: cleanup: remove acquire_ctx from drm_mode_config
The previous patch removed drm_modeset_{lock,unlock}_all, which were the
only users of this field inside the drm_mode_config structure.
Signed-off-by: Fernando Ramos
---
include/drm/drm_mode_config.h | 10 --
1 file changed, 10 deletions(-)
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..b214b07157f2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -383,16 +383,6 @@ struct drm_mode_config {
*/
struct drm_modeset_lock connection_mutex;
- /**
-* @acquire_ctx:
-*
-* Global implicit acquire context used by atomic drivers for legacy
-* IOCTLs. Deprecated, since implicit locking contexts make it
-* impossible to use driver-private drm_modeset_lock. Users of
-* this must hold @mutex.
-*/
- struct drm_modeset_acquire_ctx *acquire_ctx;
-
/**
* @idr_mutex:
*
--
2.33.0
[PATCH v4 19/20] drm: cleanup: remove drm_modeset_(un)lock_all()
Functions drm_modeset_lock_all() and drm_modeset_unlock_all() are no
longer used anywhere and can be removed.
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/drm_modeset_lock.c | 94 +-
include/drm/drm_modeset_lock.h | 2 -
2 files changed, 3 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/drm_modeset_lock.c
b/drivers/gpu/drm/drm_modeset_lock.c
index 4d32b61fa1fd..b2b84ca2b738 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -117,93 +117,6 @@ static void __stack_depot_print(depot_stack_handle_t
stack_depot)
}
#endif /* CONFIG_DRM_DEBUG_MODESET_LOCK */
-/**
- * drm_modeset_lock_all - take all modeset locks
- * @dev: DRM device
- *
- * This function takes all modeset locks, suitable where a more fine-grained
- * scheme isn't (yet) implemented. Locks must be dropped by calling the
- * drm_modeset_unlock_all() function.
- *
- * This function is deprecated. It allocates a lock acquisition context and
- * stores it in _device.mode_config. This facilitate conversion of
- * existing code because it removes the need to manually deal with the
- * acquisition context, but it is also brittle because the context is global
- * and care must be taken not to nest calls. New code should use the
- * drm_modeset_lock_all_ctx() function and pass in the context explicitly.
- */
-void drm_modeset_lock_all(struct drm_device *dev)
-{
- struct drm_mode_config *config = >mode_config;
- struct drm_modeset_acquire_ctx *ctx;
- int ret;
-
- ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
- if (WARN_ON(!ctx))
- return;
-
- mutex_lock(>mutex);
-
- drm_modeset_acquire_init(ctx, 0);
-
-retry:
- ret = drm_modeset_lock_all_ctx(dev, ctx);
- if (ret < 0) {
- if (ret == -EDEADLK) {
- drm_modeset_backoff(ctx);
- goto retry;
- }
-
- drm_modeset_acquire_fini(ctx);
- kfree(ctx);
- return;
- }
- ww_acquire_done(>ww_ctx);
-
- WARN_ON(config->acquire_ctx);
-
- /*
-* We hold the locks now, so it is safe to stash the acquisition
-* context for drm_modeset_unlock_all().
-*/
- config->acquire_ctx = ctx;
-
- drm_warn_on_modeset_not_all_locked(dev);
-}
-EXPORT_SYMBOL(drm_modeset_lock_all);
-
-/**
- * drm_modeset_unlock_all - drop all modeset locks
- * @dev: DRM device
- *
- * This function drops all modeset locks taken by a previous call to the
- * drm_modeset_lock_all() function.
- *
- * This function is deprecated. It uses the lock acquisition context stored
- * in _device.mode_config. This facilitates conversion of existing
- * code because it removes the need to manually deal with the acquisition
- * context, but it is also brittle because the context is global and care must
- * be taken not to nest calls. New code should pass the acquisition context
- * directly to the drm_modeset_drop_locks() function.
- */
-void drm_modeset_unlock_all(struct drm_device *dev)
-{
- struct drm_mode_config *config = >mode_config;
- struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
-
- if (WARN_ON(!ctx))
- return;
-
- config->acquire_ctx = NULL;
- drm_modeset_drop_locks(ctx);
- drm_modeset_acquire_fini(ctx);
-
- kfree(ctx);
-
- mutex_unlock(>mode_config.mutex);
-}
-EXPORT_SYMBOL(drm_modeset_unlock_all);
-
/**
* drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
* @dev: device
@@ -425,10 +338,9 @@ EXPORT_SYMBOL(drm_modeset_unlock);
* This function takes all modeset locks, suitable where a more fine-grained
* scheme isn't (yet) implemented.
*
- * Unlike drm_modeset_lock_all(), it doesn't take the _mode_config.mutex
- * since that lock isn't required for modeset state changes. Callers which
- * need to grab that lock too need to do so outside of the acquire context
- * @ctx.
+ * It doesn't take the _mode_config.mutex since that lock isn't required
for
+ * modeset state changes. Callers which need to grab that lock too need to do
so
+ * outside of the acquire context @ctx.
*
* Locks acquired with this function should be released by calling the
* drm_modeset_drop_locks() function on @ctx.
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index b84693fbd2b5..96b853530120 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -140,8 +140,6 @@ struct drm_device;
struct drm_crtc;
struct drm_plane;
-void drm_modeset_lock_all(struct drm_device *dev);
-void drm_modeset_unlock_all(struct drm_device *dev);
void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
int drm_modeset_lock_all_ctx(struct drm_device *dev,
--
2.33.0
[PATCH v4 18/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
NOTE:
While this change is similar to the one done two commits ago, it
contains an important extra nuances that I'm going to explain next.
The only difference between the old drm_modeset_{lock,unlock}_all()
functions and the new DRM_MODESET_LOCK_ALL_{BEGIN,END}() macros is that
the former use a global context stored in dev->mode_config.acquire_ctx
while the latter depend on a user provided one (typically in the stack).
This means that as long as no one accesses the global
dev->mode_config.acquire_ctx context in the block that runs between
lock/BEGIN and unlock/END, the code should be equivalent before and
after my changes.
Turns out that, while not obvious at first sight, the call to
dm_restore_drm_connector_state() done between drm_modset_lock_all() and
drm_modeset_unlock_all() ends up using that global context structure
stored in dev.
To fix this we need to update some function prototypes to accept the
new stack allocated variable as an argument.
Signed-off-by: Fernando Ramos
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 27 ---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 ++-
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 13 ++---
3 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 10ed1f8ad514..7a3c5def9fb9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -81,6 +81,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_DRM_AMD_DC_DCN)
#include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
@@ -2906,6 +2907,8 @@ static void handle_hpd_irq_helper(struct
amdgpu_dm_connector *aconnector)
struct amdgpu_device *adev = drm_to_adev(dev);
struct dm_connector_state *dm_con_state =
to_dm_connector_state(connector->state);
struct dm_crtc_state *dm_crtc_state = NULL;
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
if (adev->dm.disable_hpd_irq)
return;
@@ -2947,9 +2950,9 @@ static void handle_hpd_irq_helper(struct
amdgpu_dm_connector *aconnector)
goto out;
}
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+ dm_restore_drm_connector_state(dev, connector, );
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
drm_kms_helper_hotplug_event(dev);
@@ -3070,6 +3073,7 @@ static void handle_hpd_rx_irq(void *param)
struct drm_connector *connector = >base;
struct drm_device *dev = connector->dev;
struct dc_link *dc_link = aconnector->dc_link;
+ struct drm_modeset_acquire_ctx ctx;
bool is_mst_root_connector = aconnector->mst_mgr.mst_state;
bool result = false;
enum dc_connection_type new_connection_type = dc_connection_none;
@@ -3079,6 +3083,7 @@ static void handle_hpd_rx_irq(void *param)
bool has_left_work = false;
int idx = aconnector->base.index;
struct hpd_rx_irq_offload_work_queue *offload_wq =
>dm.hpd_rx_offload_wq[idx];
+ int ret;
memset(_irq_data, 0, sizeof(hpd_irq_data));
@@ -3153,9 +3158,9 @@ static void handle_hpd_rx_irq(void *param)
goto finish;
}
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+ dm_restore_drm_connector_state(dev, connector, );
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
drm_kms_helper_hotplug_event(dev);
}
@@ -9703,7 +9708,8 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
}
-static int dm_force_atomic_commit(struct drm_connector *connector)
+static int dm_force_atomic_commit(struct drm_connector *connector,
+ struct drm_modeset_acquire_ctx *ctx)
{
int ret = 0;
struct drm_device *ddev = connector->dev;
@@ -9717,7 +9723,7 @@ static int dm_force_atomic_commit(struct drm_connector
*connector)
if (!state)
return -ENOMEM;
- state->acquire_ctx = ddev->mode_config.acquire_ctx;
+ state->acquire_ctx = ctx;
/* Construct an atomic state to restore previous display setting */
@@ -9764,7 +9770,8 @@ static int dm_force_atomic_commit(struct drm_connector
*connector)
* same port and when running without usermode desktop manager supprot
*/
void dm_restore_drm_connector_state(struct drm_device *dev,
-
[PATCH v4 17/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 2]
Refactor places using drm_modeset_{lock,unlock}_all() so that they only
appear once per function.
This is needed so that in the next commit I can replace those functions
by the new macros (which use labels that can only appear once per
function).
Signed-off-by: Fernando Ramos
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 43 ---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 18 +++-
2 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f35561b5a465..10ed1f8ad514 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2935,13 +2935,6 @@ static void handle_hpd_irq_helper(struct
amdgpu_dm_connector *aconnector)
if (aconnector->base.force && new_connection_type ==
dc_connection_none) {
emulated_link_detect(aconnector->dc_link);
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
-
- if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
- drm_kms_helper_hotplug_event(dev);
-
} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
if (new_connection_type == dc_connection_none &&
aconnector->dc_link->type == dc_connection_none &&
@@ -2950,13 +2943,18 @@ static void handle_hpd_irq_helper(struct
amdgpu_dm_connector *aconnector)
amdgpu_dm_update_connector_after_detect(aconnector);
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
-
- if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
- drm_kms_helper_hotplug_event(dev);
+ } else {
+ goto out;
}
+
+ drm_modeset_lock_all(dev);
+ dm_restore_drm_connector_state(dev, connector);
+ drm_modeset_unlock_all(dev);
+
+ if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
+ drm_kms_helper_hotplug_event(dev);
+
+out:
mutex_unlock(>hpd_lock);
}
@@ -3144,12 +3142,6 @@ static void handle_hpd_rx_irq(void *param)
amdgpu_dm_update_connector_after_detect(aconnector);
-
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
-
- drm_kms_helper_hotplug_event(dev);
} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
if (aconnector->fake_enable)
@@ -3157,14 +3149,17 @@ static void handle_hpd_rx_irq(void *param)
amdgpu_dm_update_connector_after_detect(aconnector);
+ } else {
+ goto finish;
+ }
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
+ drm_modeset_lock_all(dev);
+ dm_restore_drm_connector_state(dev, connector);
+ drm_modeset_unlock_all(dev);
- drm_kms_helper_hotplug_event(dev);
- }
+ drm_kms_helper_hotplug_event(dev);
}
+finish:
#ifdef CONFIG_DRM_AMD_DC_HDCP
if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
if (adev->dm.hdcp_workqueue)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 814f67d86a3c..7751038d5788 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1237,12 +1237,6 @@ static ssize_t trigger_hotplug(struct file *f, const
char __user *buf,
goto unlock;
amdgpu_dm_update_connector_after_detect(aconnector);
-
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
-
- drm_kms_helper_hotplug_event(dev);
} else if (param[0] == 0) {
if (!aconnector->dc_link)
goto unlock;
@@ -1260,13 +1254,15 @@ static ssize_t trigger_hotplug(struct file *f, const
char __user *buf,
amdgpu_dm_update_connector_after_detect(aconnector);
- drm_modeset_lock_all(dev);
- dm_restore_drm_connector_state(dev, connector);
- drm_modeset_unlock_all(dev);
-
- drm_kms_helper_hotplug_event(dev);
+ } else {
+ goto unlock;
}
+ drm_modeset_lock_all(dev);
+ dm_restore_drm_connector_state(dev, connector);
+
[PATCH v4 16/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index dc50c05f23fc..0ea7bdbc8482 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
static void amdgpu_display_flip_callback(struct dma_fence *f,
struct dma_fence_cb *cb)
@@ -1574,16 +1575,21 @@ int amdgpu_display_suspend_helper(struct amdgpu_device
*adev)
struct drm_crtc *crtc;
struct drm_connector *connector;
struct drm_connector_list_iter iter;
- int r;
+ struct drm_modeset_acquire_ctx ctx;
+ int r, ret;
/* turn off display hw */
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_connector_list_iter_begin(dev, );
drm_for_each_connector_iter(connector, )
drm_helper_connector_dpms(connector,
DRM_MODE_DPMS_OFF);
drm_connector_list_iter_end();
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+ if (ret)
+ return ret;
+
/* unpin the front buffers and cursors */
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
@@ -1621,7 +1627,8 @@ int amdgpu_display_resume_helper(struct amdgpu_device
*adev)
struct drm_connector *connector;
struct drm_connector_list_iter iter;
struct drm_crtc *crtc;
- int r;
+ struct drm_modeset_acquire_ctx ctx;
+ int r, ret;
/* pin cursors */
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
@@ -1643,7 +1650,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device
*adev)
drm_helper_resume_force_mode(dev);
/* turn on display hw */
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_connector_list_iter_begin(dev, );
drm_for_each_connector_iter(connector, )
@@ -1651,8 +1658,8 @@ int amdgpu_display_resume_helper(struct amdgpu_device
*adev)
DRM_MODE_DPMS_ON);
drm_connector_list_iter_end();
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
- return 0;
+ return ret;
}
--
2.33.0
[PATCH v4 15/20] drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/gma500/psb_device.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_device.c
b/drivers/gpu/drm/gma500/psb_device.c
index 3030f18ba022..021a7238508f 100644
--- a/drivers/gpu/drm/gma500/psb_device.c
+++ b/drivers/gpu/drm/gma500/psb_device.c
@@ -8,6 +8,7 @@
#include
#include
+#include
#include "gma_device.h"
#include "intel_bios.h"
@@ -169,8 +170,10 @@ static int psb_save_display_registers(struct drm_device
*dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct drm_crtc *crtc;
+ struct drm_modeset_acquire_ctx ctx;
struct gma_connector *connector;
struct psb_state *regs = _priv->regs.psb;
+ int ret;
/* Display arbitration control + watermarks */
regs->saveDSPARB = PSB_RVDC32(DSPARB);
@@ -183,7 +186,7 @@ static int psb_save_display_registers(struct drm_device
*dev)
regs->saveCHICKENBIT = PSB_RVDC32(DSPCHICKENBIT);
/* Save crtc and output state */
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
if (drm_helper_crtc_in_use(crtc))
dev_priv->ops->save_crtc(crtc);
@@ -193,8 +196,9 @@ static int psb_save_display_registers(struct drm_device
*dev)
if (connector->save)
connector->save(>base);
- drm_modeset_unlock_all(dev);
- return 0;
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+ return ret;
}
/**
@@ -207,8 +211,10 @@ static int psb_restore_display_registers(struct drm_device
*dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct drm_crtc *crtc;
+ struct drm_modeset_acquire_ctx ctx;
struct gma_connector *connector;
struct psb_state *regs = _priv->regs.psb;
+ int ret;
/* Display arbitration + watermarks */
PSB_WVDC32(regs->saveDSPARB, DSPARB);
@@ -223,7 +229,7 @@ static int psb_restore_display_registers(struct drm_device
*dev)
/*make sure VGA plane is off. it initializes to on after reset!*/
PSB_WVDC32(0x8000, VGACNTRL);
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(crtc, >mode_config.crtc_list, head)
if (drm_helper_crtc_in_use(crtc))
dev_priv->ops->restore_crtc(crtc);
@@ -232,8 +238,8 @@ static int psb_restore_display_registers(struct drm_device
*dev)
if (connector->restore)
connector->restore(>base);
- drm_modeset_unlock_all(dev);
- return 0;
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+ return ret;
}
static int psb_power_down(struct drm_device *dev)
--
2.33.0
[PATCH v4 14/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
NOTE:
While the previous two commits were a simple "search and replace", this
time I had to do a bit of refactoring as only one call to
DRM_MODESET_LOCK_ALL_BEGIN() is allowed inside one same function.
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/i915/display/intel_overlay.c | 40 ++--
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c
b/drivers/gpu/drm/i915/display/intel_overlay.c
index c0ee135e5499..c623738c59c8 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1105,6 +1105,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev,
void *data,
struct drm_crtc *drmmode_crtc;
struct intel_crtc *crtc;
struct drm_i915_gem_object *new_bo;
+ struct drm_modeset_acquire_ctx ctx;
int ret;
overlay = dev_priv->overlay;
@@ -1113,24 +1114,24 @@ int intel_overlay_put_image_ioctl(struct drm_device
*dev, void *data,
return -ENODEV;
}
- if (!(params->flags & I915_OVERLAY_ENABLE)) {
- drm_modeset_lock_all(dev);
- ret = intel_overlay_switch_off(overlay);
- drm_modeset_unlock_all(dev);
+ if (params->flags & I915_OVERLAY_ENABLE) {
- return ret;
- }
+ drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
+ if (!drmmode_crtc)
+ return -ENOENT;
+ crtc = to_intel_crtc(drmmode_crtc);
- drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
- if (!drmmode_crtc)
- return -ENOENT;
- crtc = to_intel_crtc(drmmode_crtc);
+ new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
+ if (!new_bo)
+ return -ENOENT;
+ }
- new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
- if (!new_bo)
- return -ENOENT;
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
- drm_modeset_lock_all(dev);
+ if (!(params->flags & I915_OVERLAY_ENABLE)) {
+ ret = intel_overlay_switch_off(overlay);
+ goto out_unlock;
+ }
if (i915_gem_object_is_tiled(new_bo)) {
drm_dbg_kms(_priv->drm,
@@ -1195,14 +1196,11 @@ int intel_overlay_put_image_ioctl(struct drm_device
*dev, void *data,
if (ret != 0)
goto out_unlock;
- drm_modeset_unlock_all(dev);
- i915_gem_object_put(new_bo);
-
- return 0;
-
out_unlock:
- drm_modeset_unlock_all(dev);
- i915_gem_object_put(new_bo);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+ if (params->flags & I915_OVERLAY_ENABLE)
+ i915_gem_object_put(new_bo);
return ret;
}
--
2.33.0
[PATCH v4 13/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 2]
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
NOTE:
I separated this change from the rest of modifications to the i915
driver to point out something special explained next.
The only difference between the old drm_modeset_{lock,unlock}_all()
functions and the new DRM_MODESET_LOCK_ALL_{BEGIN,END}() macros is that
the former use a global context stored in dev->mode_config.acquire_ctx
while the latter depend on a user provided one (typically in the stack).
This means that as long as no one accesses the global
dev->mode_config.acquire_ctx context in the block that runs between
lock/BEGIN and unlock/END, the code should be equivalent before and
after my changes.
The only place where I had to take special action to preserve this
condition was here, where I need to modify the old call to
intel_modeset_setup_hw_state() to use the new stack allocated context
structure instead of the global one.
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/i915/display/intel_display.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index c7d9a58f9f98..75b45c01c573 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10992,6 +10992,7 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
int intel_modeset_init_nogem(struct drm_i915_private *i915)
{
struct drm_device *dev = >drm;
+ struct drm_modeset_acquire_ctx ctx;
enum pipe pipe;
struct intel_crtc *crtc;
int ret;
@@ -11043,10 +11044,10 @@ int intel_modeset_init_nogem(struct drm_i915_private
*i915)
intel_vga_disable(i915);
intel_setup_outputs(i915);
- drm_modeset_lock_all(dev);
- intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+ intel_modeset_setup_hw_state(dev, );
intel_acpi_assign_connector_fwnodes(i915);
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
for_each_intel_crtc(dev, crtc) {
if (!to_intel_crtc_state(crtc->base.state)->uapi.active)
--
2.33.0
[PATCH v4 12/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/i915/display/intel_audio.c| 16 ---
.../drm/i915/display/intel_display_debugfs.c | 46 ---
drivers/gpu/drm/i915/display/intel_overlay.c | 6 ++-
drivers/gpu/drm/i915/display/intel_pipe_crc.c | 7 ++-
drivers/gpu/drm/i915/i915_drv.c | 13 --
5 files changed, 58 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
b/drivers/gpu/drm/i915/display/intel_audio.c
index 03e8c05a74f6..37699f13b21f 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -26,6 +26,7 @@
#include
#include
+#include
#include "i915_drv.h"
#include "intel_atomic.h"
@@ -1225,7 +1226,8 @@ static int i915_audio_component_bind(struct device
*i915_kdev,
{
struct i915_audio_component *acomp = data;
struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
- int i;
+ struct drm_modeset_acquire_ctx ctx;
+ int i, ret;
if (drm_WARN_ON(_priv->drm, acomp->base.ops || acomp->base.dev))
return -EEXIST;
@@ -1235,16 +1237,16 @@ static int i915_audio_component_bind(struct device
*i915_kdev,
DL_FLAG_STATELESS)))
return -ENOMEM;
- drm_modeset_lock_all(_priv->drm);
+ DRM_MODESET_LOCK_ALL_BEGIN((_priv->drm), ctx, 0, ret);
acomp->base.ops = _audio_component_ops;
acomp->base.dev = i915_kdev;
BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
acomp->aud_sample_rate[i] = 0;
dev_priv->audio_component = acomp;
- drm_modeset_unlock_all(_priv->drm);
+ DRM_MODESET_LOCK_ALL_END((_priv->drm), ctx, ret);
- return 0;
+ return ret;
}
static void i915_audio_component_unbind(struct device *i915_kdev,
@@ -1252,12 +1254,14 @@ static void i915_audio_component_unbind(struct device
*i915_kdev,
{
struct i915_audio_component *acomp = data;
struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
- drm_modeset_lock_all(_priv->drm);
+ DRM_MODESET_LOCK_ALL_BEGIN((_priv->drm), ctx, 0, ret);
acomp->base.ops = NULL;
acomp->base.dev = NULL;
dev_priv->audio_component = NULL;
- drm_modeset_unlock_all(_priv->drm);
+ DRM_MODESET_LOCK_ALL_END((_priv->drm), ctx, ret);
device_link_remove(hda_kdev, i915_kdev);
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index bc5113589f0a..3205ceb0ab70 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include "i915_debugfs.h"
#include "intel_de.h"
@@ -1059,11 +1060,13 @@ static int i915_display_info(struct seq_file *m, void
*unused)
struct intel_crtc *crtc;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
+ struct drm_modeset_acquire_ctx ctx;
intel_wakeref_t wakeref;
+ int ret;
wakeref = intel_runtime_pm_get(_priv->runtime_pm);
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
seq_printf(m, "CRTC info\n");
seq_printf(m, "-\n");
@@ -1078,20 +1081,21 @@ static int i915_display_info(struct seq_file *m, void
*unused)
intel_connector_info(m, connector);
drm_connector_list_iter_end(_iter);
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
intel_runtime_pm_put(_priv->runtime_pm, wakeref);
- return 0;
+ return ret;
}
static int i915_shared_dplls_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
struct drm_device *dev = _priv->drm;
- int i;
+ struct drm_modeset_acquire_ctx ctx;
+ int i, ret;
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
seq_printf(m, "PLL refclks: non-SSC: %d kHz, SSC: %d kHz\n",
dev_priv->dpll.ref_clks.nssc,
@@ -1134,9 +1138,9 @@ static int i915_shared_dplls_info(struct seq_file *m,
void *unused)
seq_printf(m, " mg_pll_tdc_coldst_bias: 0x%08x\n",
pll->state.hw_state.mg_pll_tdc_coldst_bias);
}
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
- return 0;
+ return ret;
}
static int i915_ipc_status_show(struct seq_file *m, void *data)
@@ -1195,13 +1199,15 @@ static int i915_ddb_info(struct seq_file
[PATCH v4 11/20] drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and DRM_MODESET_LOCK_ALL_END() Signed-off-by: Fernando Ramos --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 768012243b44..b89687074890 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "dpu_kms.h" #include "dpu_hw_lm.h" @@ -1172,14 +1173,15 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data) struct drm_display_mode *mode; struct drm_framebuffer *fb; struct drm_plane_state *state; + struct drm_modeset_acquire_ctx ctx; struct dpu_crtc_state *cstate; - int i, out_width; + int i, out_width, ret; dpu_crtc = s->private; crtc = _crtc->base; - drm_modeset_lock_all(crtc->dev); + DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret); cstate = to_dpu_crtc_state(crtc->state); mode = >state->adjusted_mode; @@ -1263,9 +1265,9 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data) dpu_crtc->vblank_cb_time = ktime_set(0, 0); } - drm_modeset_unlock_all(crtc->dev); + DRM_MODESET_LOCK_ALL_END(crtc->dev, ctx, ret); - return 0; + return ret; } DEFINE_SHOW_ATTRIBUTE(_dpu_debugfs_status); -- 2.33.0
[PATCH v4 10/20] drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..86e18a844953 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include
@@ -667,16 +668,18 @@ nv50_audio_component_bind(struct device *kdev, struct
device *hda_kdev,
struct drm_device *drm_dev = dev_get_drvdata(kdev);
struct nouveau_drm *drm = nouveau_drm(drm_dev);
struct drm_audio_component *acomp = data;
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
return -ENOMEM;
- drm_modeset_lock_all(drm_dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
acomp->ops = _audio_component_ops;
acomp->dev = kdev;
drm->audio.component = acomp;
- drm_modeset_unlock_all(drm_dev);
- return 0;
+ DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
+ return ret;
}
static void
@@ -686,12 +689,14 @@ nv50_audio_component_unbind(struct device *kdev, struct
device *hda_kdev,
struct drm_device *drm_dev = dev_get_drvdata(kdev);
struct nouveau_drm *drm = nouveau_drm(drm_dev);
struct drm_audio_component *acomp = data;
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
- drm_modeset_lock_all(drm_dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
drm->audio.component = NULL;
acomp->ops = NULL;
acomp->dev = NULL;
- drm_modeset_unlock_all(drm_dev);
+ DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
}
static const struct component_ops nv50_audio_component_bind_ops = {
--
2.33.0
[PATCH v4 09/20] drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
---
drivers/gpu/drm/omapdrm/omap_fb.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
b/drivers/gpu/drm/omapdrm/omap_fb.c
index 190afc564914..fa7636c13c19 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "omap_dmm_tiler.h"
#include "omap_drv.h"
@@ -62,15 +63,17 @@ static int omap_framebuffer_dirty(struct drm_framebuffer
*fb,
unsigned num_clips)
{
struct drm_crtc *crtc;
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
- drm_modeset_lock_all(fb->dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(fb->dev, ctx, 0, ret);
drm_for_each_crtc(crtc, fb->dev)
omap_crtc_flush(crtc);
- drm_modeset_unlock_all(fb->dev);
+ DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
- return 0;
+ return ret;
}
static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
--
2.33.0
[PATCH v4 08/20] drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
---
drivers/gpu/drm/radeon/radeon_device.c | 21 +++--
drivers/gpu/drm/radeon/radeon_dp_mst.c | 10 ++
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 4f0fbf667431..7e31e5ce7f61 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1559,7 +1560,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool
suspend,
struct pci_dev *pdev;
struct drm_crtc *crtc;
struct drm_connector *connector;
- int i, r;
+ struct drm_modeset_acquire_ctx ctx;
+ int i, r, ret;
if (dev == NULL || dev->dev_private == NULL) {
return -ENODEV;
@@ -1573,12 +1575,15 @@ int radeon_suspend_kms(struct drm_device *dev, bool
suspend,
drm_kms_helper_poll_disable(dev);
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
/* turn off display hw */
list_for_each_entry(connector, >mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
}
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+ if (ret)
+ return ret;
/* unpin the front buffers and cursors */
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
@@ -1663,7 +1668,8 @@ int radeon_resume_kms(struct drm_device *dev, bool
resume, bool fbcon)
struct radeon_device *rdev = dev->dev_private;
struct pci_dev *pdev = to_pci_dev(dev->dev);
struct drm_crtc *crtc;
- int r;
+ struct drm_modeset_acquire_ctx ctx;
+ int r, ret;
if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
@@ -1741,11 +1747,14 @@ int radeon_resume_kms(struct drm_device *dev, bool
resume, bool fbcon)
if (fbcon) {
drm_helper_resume_force_mode(dev);
/* turn on display hw */
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(connector,
>mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
}
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+ if (ret)
+ return ret;
}
drm_kms_helper_poll_enable(dev);
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index ec867fa880a4..3f83ee75b100 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include "atom.h"
#include "ni_reg.h"
@@ -737,11 +738,12 @@ static int radeon_debugfs_mst_info_show(struct seq_file
*m, void *unused)
struct radeon_device *rdev = (struct radeon_device *)m->private;
struct drm_device *dev = rdev->ddev;
struct drm_connector *connector;
+ struct drm_modeset_acquire_ctx ctx;
struct radeon_connector *radeon_connector;
struct radeon_connector_atom_dig *dig_connector;
- int i;
+ int i, ret;
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(connector, >mode_config.connector_list, head) {
if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
continue;
@@ -759,8 +761,8 @@ static int radeon_debugfs_mst_info_show(struct seq_file *m,
void *unused)
radeon_connector->cur_stream_attribs[i].fe,
radeon_connector->cur_stream_attribs[i].slots);
}
- drm_modeset_unlock_all(dev);
- return 0;
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+ return ret;
}
DEFINE_SHOW_ATTRIBUTE(radeon_debugfs_mst_info);
--
2.33.0
[PATCH v4 07/20] drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
---
drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 80078a9fd7f6..1877feff2e6b 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -156,10 +156,12 @@ static int shmob_drm_pm_suspend(struct device *dev)
static int shmob_drm_pm_resume(struct device *dev)
{
struct shmob_drm_device *sdev = dev_get_drvdata(dev);
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
- drm_modeset_lock_all(sdev->ddev);
+ DRM_MODESET_LOCK_ALL_BEGIN(sdev->ddev, ctx, 0, ret);
shmob_drm_crtc_resume(>crtc);
- drm_modeset_unlock_all(sdev->ddev);
+ DRM_MODESET_LOCK_ALL_END(sdev->ddev, ctx, ret);
drm_kms_helper_poll_enable(sdev->ddev);
return 0;
--
2.33.0
[PATCH v4 06/20] drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
Reported-by: kernel test robot
---
drivers/gpu/drm/tegra/dsi.c | 6 --
drivers/gpu/drm/tegra/hdmi.c | 6 --
drivers/gpu/drm/tegra/sor.c | 11 +++
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index f46d377f0c30..28050c188c1c 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "dc.h"
#include "drm.h"
@@ -202,10 +203,11 @@ static int tegra_dsi_show_regs(struct seq_file *s, void
*data)
struct tegra_dsi *dsi = node->info_ent->data;
struct drm_crtc *crtc = dsi->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+ struct drm_modeset_acquire_ctx ctx;
unsigned int i;
int err = 0;
- drm_modeset_lock_all(drm);
+ DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -220,7 +222,7 @@ static int tegra_dsi_show_regs(struct seq_file *s, void
*data)
}
unlock:
- drm_modeset_unlock_all(drm);
+ DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
}
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..a62de7f92414 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include "hda.h"
#include "hdmi.h"
@@ -1031,10 +1032,11 @@ static int tegra_hdmi_show_regs(struct seq_file *s,
void *data)
struct tegra_hdmi *hdmi = node->info_ent->data;
struct drm_crtc *crtc = hdmi->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+ struct drm_modeset_acquire_ctx ctx;
unsigned int i;
int err = 0;
- drm_modeset_lock_all(drm);
+ DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -1049,7 +1051,7 @@ static int tegra_hdmi_show_regs(struct seq_file *s, void
*data)
}
unlock:
- drm_modeset_unlock_all(drm);
+ DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
}
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 0ea320c1092b..3d1c8b3d1358 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "dc.h"
#include "dp.h"
@@ -1490,10 +1491,11 @@ static int tegra_sor_show_crc(struct seq_file *s, void
*data)
struct tegra_sor *sor = node->info_ent->data;
struct drm_crtc *crtc = sor->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+ struct drm_modeset_acquire_ctx ctx;
int err = 0;
u32 value;
- drm_modeset_lock_all(drm);
+ DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -1522,7 +1524,7 @@ static int tegra_sor_show_crc(struct seq_file *s, void
*data)
seq_printf(s, "%08x\n", value);
unlock:
- drm_modeset_unlock_all(drm);
+ DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
}
@@ -1652,10 +1654,11 @@ static int tegra_sor_show_regs(struct seq_file *s, void
*data)
struct tegra_sor *sor = node->info_ent->data;
struct drm_crtc *crtc = sor->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+ struct drm_modeset_acquire_ctx ctx;
unsigned int i;
int err = 0;
- drm_modeset_lock_all(drm);
+ DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -1670,7 +1673,7 @@ static int tegra_sor_show_regs(struct seq_file *s, void
*data)
}
unlock:
- drm_modeset_unlock_all(drm);
+ DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
}
--
2.33.0
[PATCH v4 05/20] drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
---
drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 11 +++
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index 28af34ab6ed6..7df35c6f1458 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -28,6 +28,7 @@
#include "vmwgfx_drv.h"
#include "vmwgfx_devcaps.h"
#include
+#include
#include "vmwgfx_kms.h"
int vmw_getparam_ioctl(struct drm_device *dev, void *data,
@@ -172,6 +173,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
struct drm_vmw_rect __user *clips_ptr;
struct drm_vmw_rect *clips = NULL;
struct drm_framebuffer *fb;
+ struct drm_modeset_acquire_ctx ctx;
struct vmw_framebuffer *vfb;
struct vmw_resource *res;
uint32_t num_clips;
@@ -203,7 +205,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
goto out_no_copy;
}
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
if (!fb) {
@@ -231,7 +233,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
out_no_surface:
drm_framebuffer_put(fb);
out_no_fb:
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
out_no_copy:
kfree(clips);
out_clips:
@@ -250,6 +252,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void
*data,
struct drm_vmw_rect __user *clips_ptr;
struct drm_vmw_rect *clips = NULL;
struct drm_framebuffer *fb;
+ struct drm_modeset_acquire_ctx ctx;
struct vmw_framebuffer *vfb;
uint32_t num_clips;
int ret;
@@ -280,7 +283,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void
*data,
goto out_no_copy;
}
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
if (!fb) {
@@ -303,7 +306,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void
*data,
out_no_ttm_lock:
drm_framebuffer_put(fb);
out_no_fb:
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
out_no_copy:
kfree(clips);
out_clips:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 74fa41909213..268095cb8c84 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "vmwgfx_kms.h"
@@ -243,15 +244,17 @@ void vmw_kms_legacy_hotspot_clear(struct vmw_private
*dev_priv)
struct drm_device *dev = _priv->drm;
struct vmw_display_unit *du;
struct drm_crtc *crtc;
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_for_each_crtc(crtc, dev) {
du = vmw_crtc_to_du(crtc);
du->hotspot_x = 0;
du->hotspot_y = 0;
}
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
}
void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
@@ -1012,9 +1015,10 @@ static int vmw_framebuffer_bo_dirty(struct
drm_framebuffer *framebuffer,
struct vmw_framebuffer_bo *vfbd =
vmw_framebuffer_to_vfbd(framebuffer);
struct drm_clip_rect norect;
+ struct drm_modeset_acquire_ctx ctx;
int ret, increment = 1;
- drm_modeset_lock_all(_priv->drm);
+ DRM_MODESET_LOCK_ALL_BEGIN((_priv->drm), ctx, 0, ret);
if (!num_clips) {
num_clips = 1;
@@ -1040,7 +1044,7 @@ static int vmw_framebuffer_bo_dirty(struct
drm_framebuffer *framebuffer,
vmw_cmd_flush(dev_priv, false);
- drm_modeset_unlock_all(_priv->drm);
+ DRM_MODESET_LOCK_ALL_END((_priv->drm), ctx, ret);
return ret;
}
--
2.33.0
[PATCH v4 04/20] drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
---
drivers/gpu/drm/drm_client_modeset.c | 5 +++--
drivers/gpu/drm/drm_crtc_helper.c| 18 --
drivers/gpu/drm/drm_fb_helper.c | 10 ++
drivers/gpu/drm/drm_framebuffer.c| 6 --
4 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 5f5184f071ed..43f772543d2a 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1062,9 +1062,10 @@ static int drm_client_modeset_commit_legacy(struct
drm_client_dev *client)
struct drm_device *dev = client->dev;
struct drm_mode_set *mode_set;
struct drm_plane *plane;
+ struct drm_modeset_acquire_ctx ctx;
int ret = 0;
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_for_each_plane(plane, dev) {
if (plane->type != DRM_PLANE_TYPE_PRIMARY)
drm_plane_force_disable(plane);
@@ -1093,7 +1094,7 @@ static int drm_client_modeset_commit_legacy(struct
drm_client_dev *client)
goto out;
}
out:
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
return ret;
}
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index bff917531f33..f3ce073dff79 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -218,11 +218,14 @@ static void __drm_helper_disable_unused_functions(struct
drm_device *dev)
*/
void drm_helper_disable_unused_functions(struct drm_device *dev)
{
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
+
WARN_ON(drm_drv_uses_atomic_modeset(dev));
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
__drm_helper_disable_unused_functions(dev);
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
}
EXPORT_SYMBOL(drm_helper_disable_unused_functions);
@@ -942,12 +945,14 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
struct drm_crtc *crtc;
struct drm_encoder *encoder;
const struct drm_crtc_helper_funcs *crtc_funcs;
+ struct drm_modeset_acquire_ctx ctx;
int encoder_dpms;
bool ret;
+ int err;
WARN_ON(drm_drv_uses_atomic_modeset(dev));
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
drm_for_each_crtc(crtc, dev) {
if (!crtc->enabled)
@@ -982,7 +987,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
/* disable the unused connectors while restoring the modesetting */
__drm_helper_disable_unused_functions(dev);
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
}
EXPORT_SYMBOL(drm_helper_resume_force_mode);
@@ -1002,9 +1007,10 @@ EXPORT_SYMBOL(drm_helper_resume_force_mode);
int drm_helper_force_disable_all(struct drm_device *dev)
{
struct drm_crtc *crtc;
+ struct drm_modeset_acquire_ctx ctx;
int ret = 0;
- drm_modeset_lock_all(dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_for_each_crtc(crtc, dev)
if (crtc->enabled) {
struct drm_mode_set set = {
@@ -1016,7 +1022,7 @@ int drm_helper_force_disable_all(struct drm_device *dev)
goto out;
}
out:
- drm_modeset_unlock_all(dev);
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
return ret;
}
EXPORT_SYMBOL(drm_helper_force_disable_all);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8e7a124d6c5a..3b5661cf6c2b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -940,10 +940,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct
fb_info *info)
struct drm_fb_helper *fb_helper = info->par;
struct drm_mode_set *modeset;
struct drm_crtc *crtc;
+ struct drm_modeset_acquire_ctx ctx;
u16 *r, *g, *b;
int ret = 0;
- drm_modeset_lock_all(fb_helper->dev);
+ DRM_MODESET_LOCK_ALL_BEGIN(fb_helper->dev, ctx, 0, ret);
drm_client_for_each_modeset(modeset, _helper->client) {
crtc = modeset->crtc;
if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
@@ -970,7 +971,7 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct
fb_info *info)
goto out;
}
out:
- drm_modeset_unlock_all(fb_helper->dev);
+ DRM_MODESET_LOCK_ALL_END(fb_helper->dev, ctx, ret);
return ret;
}
@@ -1441,10 +1442,11 @@ static int pan_display_legacy(struct
[PATCH v4 03/20] drm/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
Reported-by: kernel test robot
---
drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
index cabe15190ec1..abda52f09b09 100644
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
@@ -5,6 +5,8 @@
#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
+#include
+
#include "msm_disp_snapshot.h"
static void msm_disp_state_dump_regs(u32 **reg, u32 aligned_len, void __iomem
*base_addr)
@@ -99,20 +101,18 @@ static void msm_disp_capture_atomic_state(struct
msm_disp_state *disp_state)
{
struct drm_device *ddev;
struct drm_modeset_acquire_ctx ctx;
+ int ret;
disp_state->timestamp = ktime_get();
ddev = disp_state->drm_dev;
- drm_modeset_acquire_init(, 0);
-
- while (drm_modeset_lock_all_ctx(ddev, ) != 0)
- drm_modeset_backoff();
+ DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
disp_state->atomic_state = drm_atomic_helper_duplicate_state(ddev,
);
- drm_modeset_drop_locks();
- drm_modeset_acquire_fini();
+
+ DRM_MODESET_LOCK_ALL_END(ddev, ctx, ret);
}
void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state)
--
2.33.0
[PATCH v4 02/20] drm/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()
Signed-off-by: Fernando Ramos
Reviewed-by: Sean Paul
---
drivers/gpu/drm/i915/display/intel_display.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 9cf987ee143d..c7d9a58f9f98 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -43,6 +43,7 @@
#include
#include
#include
+#include
#include "display/intel_audio.h"
#include "display/intel_crt.h"
@@ -11923,22 +11924,13 @@ void intel_display_resume(struct drm_device *dev)
if (state)
state->acquire_ctx =
- drm_modeset_acquire_init(, 0);
-
- while (1) {
- ret = drm_modeset_lock_all_ctx(dev, );
- if (ret != -EDEADLK)
- break;
-
- drm_modeset_backoff();
- }
+ DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
- if (!ret)
- ret = __intel_display_resume(dev, state, );
+ ret = __intel_display_resume(dev, state, );
intel_enable_ipc(dev_priv);
- drm_modeset_drop_locks();
- drm_modeset_acquire_fini();
+
+ DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
if (ret)
drm_err(_priv->drm,
--
2.33.0
[PATCH v4 01/20] drm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()
As requested in Documentation/gpu/todo.rst, replace the boilerplate code surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and DRM_MODESET_LOCK_ALL_END() Signed-off-by: Fernando Ramos --- drivers/gpu/drm/drm_client_modeset.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..5f5184f071ed 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -574,6 +574,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, int num_connectors_detected = 0; int num_tiled_conns = 0; struct drm_modeset_acquire_ctx ctx; + int err; if (!drm_drv_uses_atomic_modeset(dev)) return false; @@ -585,10 +586,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, if (!save_enabled) return false; - drm_modeset_acquire_init(, 0); - - while (drm_modeset_lock_all_ctx(dev, ) != 0) - drm_modeset_backoff(); + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err); memcpy(save_enabled, enabled, count); mask = GENMASK(count - 1, 0); @@ -743,8 +741,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client, ret = false; } - drm_modeset_drop_locks(); - drm_modeset_acquire_fini(); + DRM_MODESET_LOCK_ALL_END(dev, ctx, err); kfree(save_enabled); return ret; -- 2.33.0
[PATCH v4 00/20] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers
Hi all,
One of the things in the DRM TODO list ("Documentation/gpu/todo.rst") was to
"use DRM_MODESET_LOCAL_ALL_* helpers instead of boilerplate". That's what this
patch series is about.
You will find two types of changes here:
- Replacing "drm_modeset_lock_all_ctx()" (and surrounding boilerplate) with
"DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has
already been done in previous commits such as b7ea04d2)
- Replacing "drm_modeset_lock_all()" with "DRM_MODESET_LOCK_ALL_BEGIN()/END()"
in the remaining places (as it has already been done in previous commits
such as 57037094)
Most of the changes are straight forward, except for a few cases in the "amd"
and "i915" drivers where some extra dancing was needed to overcome the
limitation that the DRM_MODESET_LOCK_ALL_BEGIN()/END() macros can only be used
once inside the same function (the reason being that the macro expansion
includes *labels*, and you can not have two labels named the same inside one
function)
Notice that, even after this patch series, some places remain where
"drm_modeset_lock_all()" and "drm_modeset_lock_all_ctx()" are still present,
all inside drm core (which makes sense), except for two (in "amd" and "i915")
which cannot be replaced due to the way they are being used.
Changes in v2:
- Fix commit message typo
- Use the value returned by DRM_MODESET_LOCK_ALL_END when possible
- Split drm/i915 patch into two simpler ones
- Remove drm_modeset_(un)lock_all()
- Fix build problems in non-x86 platforms
Changes in v3:
- Fix in drm/i915 driver to make sure global context is no longer used
- Fix in drm/amdgpu driver to make sure global context is no longer used
- Split amdgpu driver to make it easier to understand
- Remove acquire_ctx from drm_mode_config
- Rebase on top of drm-tip
- WARNING: There is some discussion going on regarding whether the new macros
should be used (or not) in the i915 driver, as a different set of functions
has been proposed in the past (see here:
https://lore.kernel.org/dri-devel/yvrizxceipbug...@intel.com/).
In that case I will need to create a v4 where i915 files are left unchanged.
Let me know your thoughts regarding this.
Changes in v4:
- Fix missing "Signed-off-by" in one commit
- No extra comments received in one week
- Rebase on top of drm-tip
Fernando Ramos (20):
drm: cleanup: drm_modeset_lock_all_ctx() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/i915: cleanup: drm_modeset_lock_all_ctx() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/msm: cleanup: drm_modeset_lock_all_ctx() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
drm/vmwgfx: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/tegra: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/shmobile: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/radeon: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/omapdrm: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/nouveau: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/msm: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/i915: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/i915: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 2]
drm/i915: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
drm/gma500: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/amd: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
drm/amd: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 2]
drm/amd: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
drm: cleanup: remove drm_modeset_(un)lock_all()
drm: cleanup: remove acquire_ctx from drm_mode_config
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 21 +++--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 58 ++--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 25 ++---
drivers/gpu/drm/drm_client_modeset.c | 14 ++-
drivers/gpu/drm/drm_crtc_helper.c | 18 ++--
drivers/gpu/drm/drm_fb_helper.c | 10 +-
drivers/gpu/drm/drm_framebuffer.c | 6 +-
drivers/gpu/drm/drm_modeset_lock.c| 94 +--
drivers/gpu/drm/gma500/psb_device.c | 18 ++--
drivers/gpu/drm/i915/display/intel_audio.c| 16 ++--
drivers/gpu/drm/i915/display/intel_display.c | 25 ++---
.../drm/i915/display/intel_display_debugfs.c | 46 +
drivers/gpu/drm/i915/display/intel_overlay.c | 46 -
drivers/gpu/drm/i915/display/intel_pipe_crc.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 13 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 10
Re: [PATCH] drm/amdgpu: fix out of bounds write
On Wed, Oct 13, 2021 at 4:04 PM T. Williams wrote:
>
The description and s-o-b should go here and the patch seems to be
mangled. I've manually applied this. Please fix up your mailer in
the future.
Thanks for the fix.
Alex
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 87daa78a32b8..17f2756a64dc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -263,7 +263,7 @@ static ssize_t dp_link_settings_write(struct file *f,
> const char __user *buf,
> if (!wr_buf)
> return -ENOSPC;
>
> - if (parse_write_buffer_into_params(wr_buf, size,
> + if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
>(long *)param, buf,
>max_param_num,
>_nums)) {
> --
>
> Size can be any value and is user controlled resulting in overwriting the 40
> byte array wr_buf with an arbitrary length of data from buf.
>
> Signed-off-by: Thelford Williams
[PATCH] drm/amdgpu: fix out of bounds write
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 87daa78a32b8..17f2756a64dc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -263,7 +263,7 @@ static ssize_t dp_link_settings_write(struct file *f,
const char __user *buf,
if (!wr_buf)
return -ENOSPC;
- if (parse_write_buffer_into_params(wr_buf, size,
+ if (parse_write_buffer_into_params(wr_buf, wr_buf_size,
(long *)param, buf,
max_param_num,
_nums)) {
--
Size can be any value and is user controlled resulting in overwriting the
40 byte array wr_buf with an arbitrary length of data from buf.
Signed-off-by: Thelford Williams
Re: [PATCH] drm/amdgpu/gfx10: fix typo in gfx_v10_0_update_gfx_clock_gating()
Ping.
On Tue, Oct 12, 2021 at 12:40 PM Alex Deucher wrote:
>
> Check was incorrectly converted to IP version checking.
>
> Fixes: 4b0ad8425498ba ("drm/amdgpu/gfx10: convert to IP version checking")
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 71bb3c0dc1da..8cec03949835 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -8238,8 +8238,9 @@ static int gfx_v10_0_update_gfx_clock_gating(struct
> amdgpu_device *adev,
> /* === CGCG + CGLS === */
> gfx_v10_0_update_coarse_grain_clock_gating(adev, enable);
>
> - if ((adev->ip_versions[GC_HWIP][0] >= IP_VERSION(10, 1, 10))
> &&
> -(adev->ip_versions[GC_HWIP][0] <= IP_VERSION(10, 1, 2)))
> + if ((adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 1, 10))
> ||
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 1, 1)) ||
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 1, 2)))
>
> gfx_v10_0_apply_medium_grain_clock_gating_workaround(adev);
> } else {
> /* CGCG/CGLS should be disabled before MGCG/MGLS
> --
> 2.31.1
>
Re: [PATCH 2/3] drm/amdgpu/swsmu: fix is_support_sw_smu() for VEGA20
Ping on this series.
On Tue, Oct 12, 2021 at 11:53 AM Alex Deucher wrote:
>
> VEGA20 is 11.0.2, but it's handled by powerplay, not
> swsmu.
>
> Fixes: a8967967f6a554 ("drm/amdgpu/amdgpu_smu: convert to IP version
> checking")
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 4ea7e90ef60d..f5bf3ab0ebad 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -455,6 +455,10 @@ static int smu_get_power_num_states(void *handle,
>
> bool is_support_sw_smu(struct amdgpu_device *adev)
> {
> + /* vega20 is 11.0.2, but it's supported via the powerplay code */
> + if (adev->asic_type == CHIP_VEGA20)
> + return false;
> +
> if (adev->ip_versions[MP1_HWIP][0] >= IP_VERSION(11, 0, 0))
> return true;
>
> --
> 2.31.1
>
Re: [PATCH 1/5] drm/amdkfd: protect hawaii_device_info with CONFIG_DRM_AMDGPU_CIK
Ping on this series.
On Mon, Oct 11, 2021 at 11:02 AM Alex Deucher wrote:
>
> hawaii_device_info is not used when CONFIG_DRM_AMDGPU_CIK is not
> set.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 064d42acd54e..31e255ba15ed 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -114,6 +114,7 @@ static const struct kfd_device_info raven_device_info = {
> .num_sdma_queues_per_engine = 2,
> };
>
> +#ifdef CONFIG_DRM_AMDGPU_CIK
> static const struct kfd_device_info hawaii_device_info = {
> .asic_family = CHIP_HAWAII,
> .asic_name = "hawaii",
> @@ -133,6 +134,7 @@ static const struct kfd_device_info hawaii_device_info = {
> .num_xgmi_sdma_engines = 0,
> .num_sdma_queues_per_engine = 2,
> };
> +#endif
>
> static const struct kfd_device_info tonga_device_info = {
> .asic_family = CHIP_TONGA,
> --
> 2.31.1
>
[PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
From: Mark Yacoub
[Why]
1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
or Degamma props in the new CRTC state, allowing any invalid size to
be passed on.
2. Each driver has its own LUT size, which could also be different for
legacy users.
[How]
1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
assigned by the driver when it's initializing its color and CTM
management.
2. Create drm_atomic_helper_check_crtc which is called by
drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
they match the sizes in the new CRTC state.
3. Rename older lut checks that test for the color channels to indicate
it's a channel check. It's not included in drm_atomic_helper_check_crtc
as it's hardware specific and is to be called by the driver.
4. As the LUT size check now happens in drm_atomic_helper_check, remove
the lut check in intel_color.c
Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
Tested on Zork(amdgpu) and Jacuzzi(mediatek), volteer(TGL)
v1:
1. Fix typos
2. Remove the LUT size check from intel driver
3. Rename old LUT check to indicate it's a channel change
Signed-off-by: Mark Yacoub
---
drivers/gpu/drm/drm_atomic_helper.c| 60 ++
drivers/gpu/drm/drm_color_mgmt.c | 14 ++---
drivers/gpu/drm/i915/display/intel_color.c | 14 ++---
include/drm/drm_atomic_helper.h| 1 +
include/drm/drm_color_mgmt.h | 7 +--
include/drm/drm_crtc.h | 11
6 files changed, 89 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index bc3487964fb5e..5feb2ad0209c3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -929,6 +929,62 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_helper_check_planes);
+/**
+ * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
+ * @state: the driver state object
+ *
+ * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
+ * state holds them.
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *new_crtc_state;
+ int i;
+
+ for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
+ if (new_crtc_state->color_mgmt_changed &&
+ new_crtc_state->gamma_lut) {
+ uint64_t supported_lut_size = crtc->gamma_lut_size;
+ uint32_t supported_legacy_lut_size = crtc->gamma_size;
+ uint32_t new_state_lut_size =
+ drm_color_lut_size(new_crtc_state->gamma_lut);
+
+ if (new_state_lut_size != supported_lut_size &&
+ new_state_lut_size != supported_legacy_lut_size) {
+ drm_dbg_state(
+ state->dev,
+ "Invalid Gamma LUT size. Should be %u
(or %u for legacy) but got %u.\n",
+ supported_lut_size,
+ supported_legacy_lut_size,
+ new_state_lut_size);
+ return -EINVAL;
+ }
+ }
+
+ if (new_crtc_state->color_mgmt_changed &&
+ new_crtc_state->degamma_lut) {
+ uint32_t new_state_lut_size =
+ drm_color_lut_size(new_crtc_state->degamma_lut);
+ uint64_t supported_lut_size = crtc->degamma_lut_size;
+
+ if (new_state_lut_size != supported_lut_size) {
+ drm_dbg_state(
+ state->dev,
+ "Invalid Degamma LUT size. Should be %u
but got %u.\n",
+ supported_lut_size, new_state_lut_size);
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
+
/**
* drm_atomic_helper_check - validate state object
* @dev: DRM device
@@ -974,6 +1030,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_atomic_helper_check_crtcs(state);
+ if (ret)
+ return ret;
+
if (state->legacy_cursor_update)
state->async_update = !drm_atomic_helper_async_check(dev,
state);
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index bb14f488c8f6c..e5b820ce823bf 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct
[PATCH 2/2] amd/amdgpu_dm: Verify Gamma and Degamma LUT sizes using DRM Core check
From: Mark Yacoub
[Why]
drm_atomic_helper_check_crtc now verifies both legacy and non-legacy LUT
sizes. There is no need to check it within amdgpu_dm_atomic_check.
[How]
Remove the local call to verify LUT sizes and use DRM Core function
instead.
Tested on ChromeOS Zork.
v1:
Remove amdgpu_dm_verify_lut_sizes everywhere.
Signed-off-by: Mark Yacoub
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 -
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 35 ---
3 files changed, 4 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f74663b6b046e..47f8de1cfc3a5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10244,6 +10244,10 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
}
}
#endif
+ ret = drm_atomic_helper_check_crtcs(state);
+ if (ret)
+ return ret;
+
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
@@ -10253,10 +10257,6 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
dm_old_crtc_state->dsc_force_changed == false)
continue;
- ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
- if (ret)
- goto fail;
-
if (!new_crtc_state->enable)
continue;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index fcb9c4a629c32..22730e5542092 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -617,7 +617,6 @@ void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
#define MAX_COLOR_LEGACY_LUT_ENTRIES 256
void amdgpu_dm_init_color_mod(void);
-int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
struct dc_plane_state *dc_plane_state);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index a022e5bb30a5c..319f8a8a89835 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -284,37 +284,6 @@ static int __set_input_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
}
-/**
- * Verifies that the Degamma and Gamma LUTs attached to the |crtc_state| are of
- * the expected size.
- * Returns 0 on success.
- */
-int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state)
-{
- const struct drm_color_lut *lut = NULL;
- uint32_t size = 0;
-
- lut = __extract_blob_lut(crtc_state->degamma_lut, );
- if (lut && size != MAX_COLOR_LUT_ENTRIES) {
- DRM_DEBUG_DRIVER(
- "Invalid Degamma LUT size. Should be %u but got %u.\n",
- MAX_COLOR_LUT_ENTRIES, size);
- return -EINVAL;
- }
-
- lut = __extract_blob_lut(crtc_state->gamma_lut, );
- if (lut && size != MAX_COLOR_LUT_ENTRIES &&
- size != MAX_COLOR_LEGACY_LUT_ENTRIES) {
- DRM_DEBUG_DRIVER(
- "Invalid Gamma LUT size. Should be %u (or %u for
legacy) but got %u.\n",
- MAX_COLOR_LUT_ENTRIES, MAX_COLOR_LEGACY_LUT_ENTRIES,
- size);
- return -EINVAL;
- }
-
- return 0;
-}
-
/**
* amdgpu_dm_update_crtc_color_mgmt: Maps DRM color management to DC stream.
* @crtc: amdgpu_dm crtc state
@@ -348,10 +317,6 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state
*crtc)
bool is_legacy;
int r;
- r = amdgpu_dm_verify_lut_sizes(>base);
- if (r)
- return r;
-
degamma_lut = __extract_blob_lut(crtc->base.degamma_lut, _size);
regamma_lut = __extract_blob_lut(crtc->base.gamma_lut, _size);
--
2.33.0.882.g93a45727a2-goog
Re: [PATCH 1/2] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Fri, Oct 1, 2021 at 4:34 PM Sean Paul wrote:
>
> On Wed, Sep 29, 2021 at 03:39:25PM -0400, Mark Yacoub wrote:
> > From: Mark Yacoub
> >
> > [Why]
> > 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> > or Degamma props in the new CRTC state, allowing any invalid size to
> > be passed on.
> > 2. Each driver has its own LUT size, which could also be different for
> > legacy users.
> >
> > [How]
> > 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> > assigned by the driver when it's initializing its color and CTM
> > management.
> > 2. Create drm_atomic_helper_check_crtc which is called by
> > drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> > they match the sizes in the new CRTC state.
> >
>
> Did you consider extending drm_color_lut_check() with the size checks?
renamed it to be specific to channels. It's HW specific so i thought
of keeping it a separate check if the driver chooses to check it.
Removed the LUT size check that intel uses though.
>
> > Fixes: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> > Tested on Zork(amdgpu) and Jacuzzi(mediatek)
> >
> > Signed-off-by: Mark Yacoub
>
> nit: missing a space between name and email
>
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 56 +
> > drivers/gpu/drm/drm_color_mgmt.c| 2 ++
> > include/drm/drm_atomic_helper.h | 1 +
> > include/drm/drm_crtc.h | 11 ++
> > 4 files changed, 70 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2c0c6ec928200..265b9747250d1 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -930,6 +930,58 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> >
> > +/**
> > + * drm_atomic_helper_check_planes - validate state object for CRTC changes
>
> Ctrl+c/Ctrl+v error here
>
> > + * @state: the driver state object
> > + *
> > + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the
> > new
>
> Are there missing words between "object" and "such"?
>
not really. I was thinking of how to reword it without being too
verbose and nothing sounded good.
I mean I'm checking the object, such as the LUT which is part of this object.
> > + * state holds them.
> > + *
> > + * RETURNS:
> > + * Zero for success or -errno
> > + */
> > +int drm_atomic_helper_check_crtc(struct drm_atomic_state *state)
>
> drm_atomic_helper_check_crtcs to be consistent with
> drm_atomic_helper_check_planes
>
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *new_crtc_state;
> > + int i;
> > +
> > + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
>
> no space before (
>
> > + if (new_crtc_state->gamma_lut) {
>
> Perhaps gate these with a check of state->color_mgmt_changed first?
done . did it for each check so you can easily expand in the future
and squeeze in more things around those checks as it loops the CRTC
states.
>
> > + uint64_t supported_lut_size = crtc->gamma_lut_size;
> > + uint32_t supported_legacy_lut_size = crtc->gamma_size;
> > + uint32_t new_state_lut_size =
> > + drm_color_lut_size(new_crtc_state->gamma_lut);
>
> nit: new_state_lut_size and supported_lut_size can be pulled out to top level
> scope
> to avoid re-instantiation on each iteration
>
CRTC is an iterator, so it changes within the loop.
> > +
> > + if (new_state_lut_size != supported_lut_size &&
> > + new_state_lut_size != supported_legacy_lut_size) {
>
> According to the docbook, "If drivers support multiple LUT sizes then they
> should publish the largest size, and sub-sample smaller sized LUTs". So
> should this check be > instead of != ?
>
so IGT tests see it differently, they check for a very specific size,
rather than a range. so if the legacy size is 256 and regular is 1024,
1000 isn't a valid size.
> > + DRM_DEBUG_DRIVER(
>
> drm_dbg_state() is probably more appropriate
>
> > + "Invalid Gamma LUT size. Should be %u
> > (or %u for legacy) but got %u.\n",
> > + supported_lut_size,
> > + supported_legacy_lut_size,
> > + new_state_lut_size);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (new_crtc_state->degamma_lut) {
> > + uint32_t new_state_lut_size =
> > +
> > drm_color_lut_size(new_crtc_state->degamma_lut);
> > + uint64_t supported_lut_size = crtc->degamma_lut_size;
> > +
> > + if (new_state_lut_size !=
Re: [PATCH Review 1/1] drm/ttm: fix debugfs node create failed
On 10/13/2021 2:29 PM, Christian König wrote:
Am 12.10.21 um 15:12 schrieb Das, Nirmoy:
On 10/12/2021 1:58 PM, Stanley.Yang wrote:
Test scenario:
modprobe amdgpu -> rmmod amdgpu -> modprobe amdgpu
Error log:
[ 54.396807] debugfs: File 'page_pool' in directory 'amdttm'
already present!
[ 54.396833] debugfs: File 'page_pool_shrink' in directory
'amdttm' already present!
[ 54.396848] debugfs: File 'buffer_objects' in directory
'amdttm' already present!
We should instead add a check if those debugfs files already
exist/created in ttm debugfs dir using debugfs_lookup() before creating.
No, IIRC the Intel guys had fixed that already by adding/removing the
debugfs file on module load/unload.
Adding/removing on ttm module load/unload is nicer.
Nirmoy
Christian.
Regards,
Nirmoy
Reason:
page_pool, page_pool_shrink and buffer_objects can be removed when
rmmod amdttm, in the above test scenario only rmmod amdgpu, so
those
debugfs node will not be removed, this caused file create failed.
Soultion:
create ttm_page directory under ttm_root directory when insmod
amdgpu,
page_pool, page_pool_shrink and buffer_objects are stored in
ttm_page directiry,
remove ttm_page directory when do rmmod amdgpu, this can fix
above issue.
Signed-off-by: Stanley.Yang
---
drivers/gpu/drm/ttm/ttm_device.c | 12 +++-
drivers/gpu/drm/ttm/ttm_module.c | 1 +
drivers/gpu/drm/ttm/ttm_module.h | 1 +
drivers/gpu/drm/ttm/ttm_pool.c | 4 ++--
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c
b/drivers/gpu/drm/ttm/ttm_device.c
index 1de23edbc182..ad170328f0c8 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -55,6 +55,10 @@ static void ttm_global_release(void)
ttm_pool_mgr_fini();
+#ifdef CONFIG_DEBUG_FS
+ debugfs_remove(ttm_debugfs_page);
+#endif
+
__free_page(glob->dummy_read_page);
memset(glob, 0, sizeof(*glob));
out:
@@ -85,6 +89,10 @@ static int ttm_global_init(void)
>> PAGE_SHIFT;
num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT));
+#ifdef CONFIG_DEBUG_FS
+ ttm_debugfs_page = debugfs_create_dir("ttm_page",
ttm_debugfs_root);
+#endif
+
ttm_pool_mgr_init(num_pages);
ttm_tt_mgr_init(num_pages, num_dma32);
@@ -98,8 +106,10 @@ static int ttm_global_init(void)
INIT_LIST_HEAD(>device_list);
atomic_set(>bo_count, 0);
- debugfs_create_atomic_t("buffer_objects", 0444,
ttm_debugfs_root,
+#ifdef CONFIG_DEBUG_FS
+ debugfs_create_atomic_t("buffer_objects", 0444, ttm_debugfs_page,
>bo_count);
+#endif
out:
mutex_unlock(_global_mutex);
return ret;
diff --git a/drivers/gpu/drm/ttm/ttm_module.c
b/drivers/gpu/drm/ttm/ttm_module.c
index 88970a6b8e32..66595e6e7087 100644
--- a/drivers/gpu/drm/ttm/ttm_module.c
+++ b/drivers/gpu/drm/ttm/ttm_module.c
@@ -38,6 +38,7 @@
#include "ttm_module.h"
struct dentry *ttm_debugfs_root;
+struct dentry *ttm_debugfs_page;
static int __init ttm_init(void)
{
diff --git a/drivers/gpu/drm/ttm/ttm_module.h
b/drivers/gpu/drm/ttm/ttm_module.h
index d7cac5d4b835..6007dc66f44e 100644
--- a/drivers/gpu/drm/ttm/ttm_module.h
+++ b/drivers/gpu/drm/ttm/ttm_module.h
@@ -36,5 +36,6 @@
struct dentry;
extern struct dentry *ttm_debugfs_root;
+extern struct dentry *ttm_debugfs_page;
#endif /* _TTM_MODULE_H_ */
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
b/drivers/gpu/drm/ttm/ttm_pool.c
index 8be7fd7161fd..ecb33daad7b5 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -709,9 +709,9 @@ int ttm_pool_mgr_init(unsigned long num_pages)
}
#ifdef CONFIG_DEBUG_FS
- debugfs_create_file("page_pool", 0444, ttm_debugfs_root, NULL,
+ debugfs_create_file("page_pool", 0444, ttm_debugfs_page, NULL,
_pool_debugfs_globals_fops);
- debugfs_create_file("page_pool_shrink", 0400, ttm_debugfs_root,
NULL,
+ debugfs_create_file("page_pool_shrink", 0400, ttm_debugfs_page,
NULL,
_pool_debugfs_shrink_fops);
#endif
[PATCH 1/3] drm:Enable buddy allocator support
Port Intel buddy manager to drm root folder
Implemented range allocation support for the provided order
Implemented TOP-DOWN support
Implemented freeing up unused pages on contiguous allocation
Moved range allocation and freelist pickup into a single function
Signed-off-by: Arunpravin
---
drivers/gpu/drm/Makefile| 2 +-
drivers/gpu/drm/drm_buddy.c | 705
drivers/gpu/drm/drm_drv.c | 3 +
include/drm/drm_buddy.h | 157
4 files changed, 866 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_buddy.c
create mode 100644 include/drm/drm_buddy.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index a118692a6df7..fe1a2fc09675 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,7 +18,7 @@ drm-y :=drm_aperture.o drm_auth.o drm_cache.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o \
- drm_managed.o drm_vblank_work.o
+ drm_managed.o drm_vblank_work.o drm_buddy.o
drm-$(CONFIG_DRM_LEGACY) += drm_agpsupport.o drm_bufs.o drm_context.o
drm_dma.o \
drm_legacy_misc.o drm_lock.o drm_memory.o
drm_scatter.o \
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
new file mode 100644
index ..8cd118574665
--- /dev/null
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -0,0 +1,705 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include
+#include
+
+#include
+
+static struct kmem_cache *slab_blocks;
+
+static struct drm_buddy_block *drm_block_alloc(struct drm_buddy_mm *mm,
+ struct drm_buddy_block *parent,
+ unsigned int order,
+ u64 offset)
+{
+ struct drm_buddy_block *block;
+
+ BUG_ON(order > DRM_BUDDY_MAX_ORDER);
+
+ block = kmem_cache_zalloc(slab_blocks, GFP_KERNEL);
+ if (!block)
+ return NULL;
+
+ block->header = offset;
+ block->header |= order;
+ block->parent = parent;
+
+ BUG_ON(block->header & DRM_BUDDY_HEADER_UNUSED);
+ return block;
+}
+
+static void drm_block_free(struct drm_buddy_mm *mm,
+ struct drm_buddy_block *block)
+{
+ kmem_cache_free(slab_blocks, block);
+}
+
+static void mark_allocated(struct drm_buddy_block *block)
+{
+ block->header &= ~DRM_BUDDY_HEADER_STATE;
+ block->header |= DRM_BUDDY_ALLOCATED;
+
+ list_del(>link);
+}
+
+static void mark_free(struct drm_buddy_mm *mm,
+ struct drm_buddy_block *block)
+{
+ block->header &= ~DRM_BUDDY_HEADER_STATE;
+ block->header |= DRM_BUDDY_FREE;
+
+ list_add(>link,
+ >free_list[drm_buddy_block_order(block)]);
+}
+
+static void mark_split(struct drm_buddy_block *block)
+{
+ block->header &= ~DRM_BUDDY_HEADER_STATE;
+ block->header |= DRM_BUDDY_SPLIT;
+
+ list_del(>link);
+}
+
+/**
+ * drm_buddy_init - init memory manager
+ *
+ * @mm: DRM buddy manager to initialize
+ * @size: size in bytes to manage
+ * @chunk_size: minimum page size in bytes for our allocations
+ *
+ * Initializes the memory manager and its resources.
+ *
+ * Returns:
+ * 0 on success, error code on failure.
+ */
+int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
+{
+ unsigned int i;
+ u64 offset;
+
+ if (size < chunk_size)
+ return -EINVAL;
+
+ if (chunk_size < PAGE_SIZE)
+ return -EINVAL;
+
+ if (!is_power_of_2(chunk_size))
+ return -EINVAL;
+
+ size = round_down(size, chunk_size);
+
+ mm->size = size;
+ mm->avail = size;
+ mm->chunk_size = chunk_size;
+ mm->max_order = ilog2(size) - ilog2(chunk_size);
+
+ BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
+
+ mm->free_list = kmalloc_array(mm->max_order + 1,
+ sizeof(struct list_head),
+ GFP_KERNEL);
+ if (!mm->free_list)
+ return -ENOMEM;
+
+ for (i = 0; i <= mm->max_order; ++i)
+ INIT_LIST_HEAD(>free_list[i]);
+
+ mm->n_roots = hweight64(size);
+
+ mm->roots = kmalloc_array(mm->n_roots,
+ sizeof(struct drm_buddy_block *),
+ GFP_KERNEL);
+ if (!mm->roots)
+ goto out_free_list;
+
+ offset = 0;
+ i = 0;
+
+ /*
+* Split into power-of-two blocks, in case we are given a size that is
+* not itself a power-of-two.
+*/
+ do {
+ struct drm_buddy_block *root;
+ unsigned int order;
+ u64 root_size;
+
+
Re: [PATCH 0/5] 0 MHz is not a valid current frequency
On Wed, Oct 13, 2021 at 1:06 PM Lazar, Lijo wrote: > > [AMD Official Use Only] > > > >Or maybe just a list without default hint, i.e. no asterisk? > > I think this is also fine meaning we are having trouble in determining the > current frequency or DPM level. Evan/Alex? Don't know if this will crash the > tools. > That seems reasonable to me. Alex > Thanks, > Lijo > > From: Tuikov, Luben > Sent: Wednesday, October 13, 2021 9:52:09 PM > To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org > > Cc: Deucher, Alexander > Subject: Re: [PATCH 0/5] 0 MHz is not a valid current frequency > > On 2021-10-13 00:14, Lazar, Lijo wrote: > > > > On 10/13/2021 8:40 AM, Luben Tuikov wrote: > >> Some ASIC support low-power functionality for the whole ASIC or just > >> an IP block. When in such low-power mode, some sysfs interfaces would > >> report a frequency of 0, e.g., > >> > >> $cat /sys/class/drm/card0/device/pp_dpm_sclk > >> 0: 500Mhz > >> 1: 0Mhz * > >> 2: 2200Mhz > >> $_ > >> > >> An operating frequency of 0 MHz doesn't make sense, and this interface > >> is designed to report only operating clock frequencies, i.e. non-zero, > >> and possibly the current one. > >> > >> When in this low-power state, round to the smallest > >> operating frequency, for this interface, as follows, > >> > > Would rather avoid this - > > > > 1) It is manipulating FW reported value. If at all there is an uncaught > > issue in FW reporting of frequency values, that is masked here. > > 2) Otherwise, if 0MHz is described as GFX power gated case, this > > provides a convenient interface to check if GFX is power gated. > > > > If seeing a '0' is not pleasing, consider changing to something like > >"NA" - not available (frequency cannot be fetched at the moment). > > There's a ROCm tool which literally asserts if the values are not ordered in > increasing order. Now since 0 < 550, but 0 is listed as the second entry, the > tool simply asserts and crashes. > > It is not clear what you'd rather see here: > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > 0: 550Mhz > 1: N/A * > 2: 2200MHz > $_ > > Is this what you want to see? (That'll crash other tools which expect %uMhz.) > > Or maybe just a list without default hint, i.e. no asterisk? > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > 0: 550Mhz > 1: 2200MHz > $_ > > What should the output be? > > We want to avoid showing 0, but still show numbers. > > Regards, > Luben > > > > > Thanks, > > Lijo > > > >> $cat /sys/class/drm/card0/device/pp_dpm_sclk > >> 0: 500Mhz * > >> 1: 2200Mhz > >> $_ > >> > >> Luben Tuikov (5): > >>drm/amd/pm: Slight function rename > >>drm/amd/pm: Rename cur_value to curr_value > >>drm/amd/pm: Rename freq_values --> freq_value > >>dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency > >>dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency > >> > >> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- > >> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 --- > >> 2 files changed, 86 insertions(+), 47 deletions(-) > >> >
Re: [PATCH 1/3] drm/amdgpu/smu11: fix firmware version check for vangogh
On Wed, Oct 13, 2021 at 3:12 AM Quan, Evan wrote:
>
> [AMD Official Use Only]
>
> Reviewed-by: Evan Quan
Is this for just this patch or the series?
Alex
>
> > -Original Message-
> > From: amd-gfx On Behalf Of Alex
> > Deucher
> > Sent: Tuesday, October 12, 2021 11:53 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander
> > Subject: [PATCH 1/3] drm/amdgpu/smu11: fix firmware version check for
> > vangogh
> >
> > Was missed in the conversion to IP version checking.
> >
> > Fixes: af3b89d3a639d5 ("drm/amdgpu/smu11.0: convert to IP version
> > checking")
> > Signed-off-by: Alex Deucher
> > ---
> > drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > index 3470c33ee09d..6d008e9c2f65 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > @@ -255,7 +255,7 @@ int smu_v11_0_check_fw_version(struct
> > smu_context *smu)
> > case IP_VERSION(11, 0, 11):
> > smu->smc_driver_if_version =
> > SMU11_DRIVER_IF_VERSION_Navy_Flounder;
> > break;
> > - case CHIP_VANGOGH:
> > + case IP_VERSION(11, 5, 0):
> > smu->smc_driver_if_version =
> > SMU11_DRIVER_IF_VERSION_VANGOGH;
> > break;
> > case IP_VERSION(11, 0, 12):
> > --
> > 2.31.1
Re: [PATCH 0/5] 0 MHz is not a valid current frequency
[AMD Official Use Only] >Or maybe just a list without default hint, i.e. no asterisk? I think this is also fine meaning we are having trouble in determining the current frequency or DPM level. Evan/Alex? Don't know if this will crash the tools. Thanks, Lijo From: Tuikov, Luben Sent: Wednesday, October 13, 2021 9:52:09 PM To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander Subject: Re: [PATCH 0/5] 0 MHz is not a valid current frequency On 2021-10-13 00:14, Lazar, Lijo wrote: > > On 10/13/2021 8:40 AM, Luben Tuikov wrote: >> Some ASIC support low-power functionality for the whole ASIC or just >> an IP block. When in such low-power mode, some sysfs interfaces would >> report a frequency of 0, e.g., >> >> $cat /sys/class/drm/card0/device/pp_dpm_sclk >> 0: 500Mhz >> 1: 0Mhz * >> 2: 2200Mhz >> $_ >> >> An operating frequency of 0 MHz doesn't make sense, and this interface >> is designed to report only operating clock frequencies, i.e. non-zero, >> and possibly the current one. >> >> When in this low-power state, round to the smallest >> operating frequency, for this interface, as follows, >> > Would rather avoid this - > > 1) It is manipulating FW reported value. If at all there is an uncaught > issue in FW reporting of frequency values, that is masked here. > 2) Otherwise, if 0MHz is described as GFX power gated case, this > provides a convenient interface to check if GFX is power gated. > > If seeing a '0' is not pleasing, consider changing to something like >"NA" - not available (frequency cannot be fetched at the moment). There's a ROCm tool which literally asserts if the values are not ordered in increasing order. Now since 0 < 550, but 0 is listed as the second entry, the tool simply asserts and crashes. It is not clear what you'd rather see here: $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 550Mhz 1: N/A * 2: 2200MHz $_ Is this what you want to see? (That'll crash other tools which expect %uMhz.) Or maybe just a list without default hint, i.e. no asterisk? $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 550Mhz 1: 2200MHz $_ What should the output be? We want to avoid showing 0, but still show numbers. Regards, Luben > > Thanks, > Lijo > >> $cat /sys/class/drm/card0/device/pp_dpm_sclk >> 0: 500Mhz * >> 1: 2200Mhz >> $_ >> >> Luben Tuikov (5): >>drm/amd/pm: Slight function rename >>drm/amd/pm: Rename cur_value to curr_value >>drm/amd/pm: Rename freq_values --> freq_value >>dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency >>dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency >> >> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- >> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 --- >> 2 files changed, 86 insertions(+), 47 deletions(-) >>
Re: [PATCH 0/5] 0 MHz is not a valid current frequency
On 2021-10-13 00:14, Lazar, Lijo wrote: > > On 10/13/2021 8:40 AM, Luben Tuikov wrote: >> Some ASIC support low-power functionality for the whole ASIC or just >> an IP block. When in such low-power mode, some sysfs interfaces would >> report a frequency of 0, e.g., >> >> $cat /sys/class/drm/card0/device/pp_dpm_sclk >> 0: 500Mhz >> 1: 0Mhz * >> 2: 2200Mhz >> $_ >> >> An operating frequency of 0 MHz doesn't make sense, and this interface >> is designed to report only operating clock frequencies, i.e. non-zero, >> and possibly the current one. >> >> When in this low-power state, round to the smallest >> operating frequency, for this interface, as follows, >> > Would rather avoid this - > > 1) It is manipulating FW reported value. If at all there is an uncaught > issue in FW reporting of frequency values, that is masked here. > 2) Otherwise, if 0MHz is described as GFX power gated case, this > provides a convenient interface to check if GFX is power gated. > > If seeing a '0' is not pleasing, consider changing to something like > "NA" - not available (frequency cannot be fetched at the moment). There's a ROCm tool which literally asserts if the values are not ordered in increasing order. Now since 0 < 550, but 0 is listed as the second entry, the tool simply asserts and crashes. It is not clear what you'd rather see here: $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 550Mhz 1: N/A * 2: 2200MHz $_ Is this what you want to see? (That'll crash other tools which expect %uMhz.) Or maybe just a list without default hint, i.e. no asterisk? $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 550Mhz 1: 2200MHz $_ What should the output be? We want to avoid showing 0, but still show numbers. Regards, Luben > > Thanks, > Lijo > >> $cat /sys/class/drm/card0/device/pp_dpm_sclk >> 0: 500Mhz * >> 1: 2200Mhz >> $_ >> >> Luben Tuikov (5): >>drm/amd/pm: Slight function rename >>drm/amd/pm: Rename cur_value to curr_value >>drm/amd/pm: Rename freq_values --> freq_value >>dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency >>dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency >> >> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- >> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 --- >> 2 files changed, 86 insertions(+), 47 deletions(-) >>
[PATCH 3/5] drm/amd/pm: Rename freq_values --> freq_value
By usage: read freq_values[x] to freq_value[x].
Cc: Alex Deucher
Signed-off-by: Luben Tuikov
---
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c| 16
.../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c| 18 +-
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index f810549df493d5..646e9bbf8af42a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1268,7 +1268,7 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
uint16_t *curve_settings;
int i, size = 0, ret = 0;
uint32_t curr_value = 0, value = 0, count = 0;
- uint32_t freq_values[3] = {0};
+ uint32_t freq_value[3] = {0, 0, 0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = >smu_table;
uint32_t gen_speed, lane_width;
@@ -1310,21 +1310,21 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_values[0]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_value[0]);
if (ret)
return size;
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type,
count - 1, _values[2]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type,
count - 1, _value[2]);
if (ret)
return size;
- freq_values[1] = curr_value;
- mark_index = curr_value == freq_values[0] ? 0 :
-curr_value == freq_values[2] ? 2 : 1;
+ freq_value[1] = curr_value;
+ mark_index = curr_value == freq_value[0] ? 0 :
+curr_value == freq_value[2] ? 2 : 1;
if (mark_index != 1)
- freq_values[1] = (freq_values[0] +
freq_values[2]) / 2;
+ freq_value[1] = (freq_value[0] + freq_value[2])
/ 2;
for (i = 0; i < 3; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, freq_values[i],
+ size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, freq_value[i],
i == mark_index ? "*" : "");
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3ebded3a99b5f2..f630d5e928ccfe 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1053,7 +1053,7 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
uint32_t curr_value = 0, value = 0, count = 0;
- uint32_t freq_values[3] = {0};
+ uint32_t freq_value[3] = {0, 0, 0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
uint32_t min_value, max_value;
@@ -1096,26 +1096,26 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_values[0]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_value[0]);
if (ret)
goto print_clk_out;
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type,
count - 1, _values[2]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type,
count - 1, _value[2]);
if (ret)
goto print_clk_out;
- freq_values[1] = curr_value;
- mark_index = curr_value == freq_values[0] ? 0 :
-curr_value == freq_values[2] ? 2 : 1;
+ freq_value[1] = curr_value;
+ mark_index = curr_value == freq_value[0] ? 0 :
+curr_value == freq_value[2] ? 2 : 1;
count = 3;
if (mark_index != 1) {
count = 2;
- freq_values[1] = freq_values[2];
+ freq_value[1] = freq_value[2];
}
for (i = 0; i < count; i++) {
-
[PATCH 5/5] dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency
A current value of a clock frequency of 0, means
that the IP block is in some kind of low power
state. Ignore it and don't report it here. Here we
only report the possible operating (non-zero)
frequencies of the block requested. So, if the
current clock value is 0, then report as the
current clock the minimum operating one, which is
non-zero.
Cc: Alex Deucher
Signed-off-by: Luben Tuikov
---
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 45 ---
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 646e9bbf8af42a..de1a558dc81047 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1269,7 +1269,6 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
int i, size = 0, ret = 0;
uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_value[3] = {0, 0, 0};
- uint32_t mark_index = 0;
struct smu_table_context *table_context = >smu_table;
uint32_t gen_speed, lane_width;
struct smu_dpm_context *smu_dpm = >smu_dpm;
@@ -1279,6 +1278,7 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
(OverDriveTable_t *)table_context->overdrive_table;
struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
uint32_t min_value, max_value;
+ bool fine_grained;
smu_cmn_get_sysfs_buf(, );
@@ -1296,12 +1296,23 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
if (ret)
return size;
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
+ _value[0]);
+ if (ret)
+ return size;
+
+ if (curr_value == 0)
+ curr_value = freq_value[0];
+
ret = smu_v11_0_get_dpm_level_count(smu, clk_type, );
if (ret)
return size;
- if (!navi10_supports_fine_grained_dpm(smu, clk_type)) {
- for (i = 0; i < count; i++) {
+ fine_grained = navi10_supports_fine_grained_dpm(smu, clk_type);
+ if (!fine_grained) {
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0,
freq_value[0],
+ curr_value == freq_value[0] ? "*"
: "");
+ for (i = 1; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu,
clk_type, i, );
if (ret)
return size;
@@ -1310,24 +1321,28 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_value[0]);
- if (ret)
- return size;
+ freq_value[1] = curr_value;
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type,
count - 1, _value[2]);
if (ret)
return size;
- freq_value[1] = curr_value;
- mark_index = curr_value == freq_value[0] ? 0 :
-curr_value == freq_value[2] ? 2 : 1;
- if (mark_index != 1)
- freq_value[1] = (freq_value[0] + freq_value[2])
/ 2;
-
- for (i = 0; i < 3; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, freq_value[i],
- i == mark_index ? "*" : "");
+ if (freq_value[1] == freq_value[0]) {
+ i = 1;
+ count = 3;
+ } else if (freq_value[1] == freq_value[2]) {
+ i = 0;
+ count = 2;
+ } else {
+ i = 0;
+ count = 3;
}
+ for ( ; i < count; i++) {
+ size += sysfs_emit_at(buf, size,
+ "%d: %uMhz %s\n",
+ i, freq_value[i],
+ curr_value ==
freq_value[i] ? "*" : "");
+ }
}
break;
case SMU_PCIE:
--
2.33.1.558.g2bd2f258f4
[PATCH 4/5] dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency
A current value of a clock frequency of 0, means
that the IP block is in some kind of low power
state. Ignore it and don't report it here. Here we
only report the possible operating (non-zero)
frequencies of the block requested. So, if the
current clock value is 0, then report as the
current clock the minimum operating one, which is
non-zero.
Cc: Alex Deucher
Signed-off-by: Luben Tuikov
---
.../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 57 +--
1 file changed, 39 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index f630d5e928ccfe..00be2b851baf93 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1054,10 +1054,10 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
int i, size = 0, ret = 0;
uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_value[3] = {0, 0, 0};
- uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
uint32_t min_value, max_value;
uint32_t smu_version;
+ bool fine_grained;
smu_cmn_get_sysfs_buf(, );
@@ -1077,6 +1077,22 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
if (ret)
goto print_clk_out;
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
+ _value[0]);
+ if (ret)
+ goto print_clk_out;
+
+ /* A current value of a clock frequency of 0, means
+* that the IP block is in some kind of low power
+* state. Ignore it and don't report it here. Here we
+* only report the possible operating (non-zero)
+* frequencies of the block requested. So, if the
+* current clock value is 0, then report as the
+* current clock the minimum operating one, which is
+* non-zero.
+*/
+ if (curr_value == 0)
+ curr_value = freq_value[0];
/* no need to disable gfxoff when retrieving the current gfxclk
*/
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
@@ -1086,38 +1102,43 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
if (ret)
goto print_clk_out;
- if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
- for (i = 0; i < count; i++) {
+ fine_grained = sienna_cichlid_supports_fine_grained_dpm(smu,
clk_type);
+ if (!fine_grained) {
+ /* We already got the 0-th index--print it
+* here and continue thereafter.
+*/
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0,
freq_value[0],
+ curr_value == freq_value[0] ? "*"
: "");
+ for (i = 1; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu,
clk_type, i, );
if (ret)
goto print_clk_out;
-
size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, value,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_value[0]);
- if (ret)
- goto print_clk_out;
+ freq_value[1] = curr_value;
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type,
count - 1, _value[2]);
if (ret)
goto print_clk_out;
- freq_value[1] = curr_value;
- mark_index = curr_value == freq_value[0] ? 0 :
-curr_value == freq_value[2] ? 2 : 1;
-
- count = 3;
- if (mark_index != 1) {
+ if (freq_value[1] == freq_value[0]) {
+ i = 1;
+ count = 3;
+ } else if (freq_value[1] == freq_value[2]) {
+ i = 0;
count = 2;
- freq_value[1] = freq_value[2];
+ } else {
+ i = 0;
+ count = 3;
}
- for (i = 0; i < count; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, freq_value[i],
-
[PATCH 0/5] 0 MHz is not a valid current frequency (v2)
Some ASIC support low-power functionality for the whole ASIC or just an IP block. When in such low-power mode, some sysfs interfaces would report a frequency of 0, e.g., $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 500Mhz 1: 0Mhz * 2: 2200Mhz $_ An operating frequency of 0 MHz doesn't make sense, and this interface is designed to report only operating clock frequencies, i.e. non-zero, and possibly the current one. When in this low-power state, round to the smallest operating frequency, for this interface, as follows, $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 500Mhz * 1: 2200Mhz $_ v2: Fix description to reflect change in patch 1--add an 's'. Luben Tuikov (5): drm/amd/pm: Slight function rename (v2) drm/amd/pm: Rename cur_value to curr_value drm/amd/pm: Rename freq_values --> freq_value dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 --- 2 files changed, 86 insertions(+), 47 deletions(-) -- 2.33.1.558.g2bd2f258f4
[PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value
Rename "cur_value", which stands for "cursor
value" to "curr_value", which stands for "current
value".
Cc: Alex Deucher
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 12 ++--
.../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ---
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 0fe9790f67f5af..f810549df493d5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1267,7 +1267,7 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
{
uint16_t *curve_settings;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = >smu_table;
@@ -1292,7 +1292,7 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
case SMU_VCLK:
case SMU_DCLK:
case SMU_DCEFCLK:
- ret = navi10_get_current_clk_freq_by_table(smu, clk_type,
_value);
+ ret = navi10_get_current_clk_freq_by_table(smu, clk_type,
_value);
if (ret)
return size;
@@ -1307,7 +1307,7 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
return size;
size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_values[0]);
@@ -1317,9 +1317,9 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
if (ret)
return size;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
-cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+curr_value == freq_values[2] ? 2 : 1;
if (mark_index != 1)
freq_values[1] = (freq_values[0] +
freq_values[2]) / 2;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3f5721baa5ff50..3ebded3a99b5f2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1052,7 +1052,7 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
OverDriveTable_t *od_table =
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
@@ -1073,10 +1073,11 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
case SMU_DCLK:
case SMU_DCLK1:
case SMU_DCEFCLK:
- ret = sienna_cichlid_get_current_clk_freq_by_table(smu,
clk_type, _value);
+ ret = sienna_cichlid_get_current_clk_freq_by_table(smu,
clk_type, _value);
if (ret)
goto print_clk_out;
+
/* no need to disable gfxoff when retrieving the current gfxclk
*/
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
amdgpu_gfx_off_ctrl(adev, false);
@@ -1092,7 +1093,7 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
goto print_clk_out;
size += sysfs_emit_at(buf, size, "%d: %uMhz
%s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
_values[0]);
@@ -1102,9 +1103,9 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
if (ret)
goto print_clk_out;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
-cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+
[PATCH 1/5] drm/amd/pm: Slight function rename (v2)
Rename
sienna_cichlid_is_support_fine_grained_dpm() to
sienna_cichlid_supports_fine_grained_dpm().
Rename
navi10_is_support_fine_grained_dpm() to
navi10_supports_fine_grained_dpm().
v2: Fix function name in commit message to reflect
the change being done: add a missing 's'.
Cc: Alex Deucher
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 7 ---
drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 7 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 71161f6b78fea9..0fe9790f67f5af 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1231,7 +1231,8 @@ static int navi10_get_current_clk_freq_by_table(struct
smu_context *smu,
value);
}
-static bool navi10_is_support_fine_grained_dpm(struct smu_context *smu, enum
smu_clk_type clk_type)
+static bool navi10_supports_fine_grained_dpm(struct smu_context *smu,
+enum smu_clk_type clk_type)
{
PPTable_t *pptable = smu->smu_table.driver_pptable;
DpmDescriptor_t *dpm_desc = NULL;
@@ -1299,7 +1300,7 @@ static int navi10_print_clk_levels(struct smu_context
*smu,
if (ret)
return size;
- if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (!navi10_supports_fine_grained_dpm(smu, clk_type)) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu,
clk_type, i, );
if (ret)
@@ -1465,7 +1466,7 @@ static int navi10_force_clk_levels(struct smu_context
*smu,
case SMU_UCLK:
case SMU_FCLK:
/* There is only 2 levels for fine grained DPM */
- if (navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (navi10_supports_fine_grained_dpm(smu, clk_type)) {
soft_max_level = (soft_max_level >= 1 ? 1 : 0);
soft_min_level = (soft_min_level >= 1 ? 1 : 0);
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 15e66e1912de33..3f5721baa5ff50 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1006,7 +1006,8 @@ static int
sienna_cichlid_get_current_clk_freq_by_table(struct smu_context *smu,
}
-static bool sienna_cichlid_is_support_fine_grained_dpm(struct smu_context
*smu, enum smu_clk_type clk_type)
+static bool sienna_cichlid_supports_fine_grained_dpm(struct smu_context *smu,
+enum smu_clk_type clk_type)
{
DpmDescriptor_t *dpm_desc = NULL;
DpmDescriptor_t *table_member;
@@ -1084,7 +1085,7 @@ static int sienna_cichlid_print_clk_levels(struct
smu_context *smu,
if (ret)
goto print_clk_out;
- if (!sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type))
{
+ if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu,
clk_type, i, );
if (ret)
@@ -1235,7 +1236,7 @@ static int sienna_cichlid_force_clk_levels(struct
smu_context *smu,
case SMU_UCLK:
case SMU_FCLK:
/* There is only 2 levels for fine grained DPM */
- if (sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
soft_max_level = (soft_max_level >= 1 ? 1 : 0);
soft_min_level = (soft_min_level >= 1 ? 1 : 0);
}
--
2.33.1.558.g2bd2f258f4
Re: [PATCH] drm/amdgpu/nbio2.3: use original HDP_FLUSH bits for navi1x
On Wed, Oct 13, 2021 at 11:06 AM Alex Deucher wrote:
>
> The extended bits were not available for use on navi1x, but
> navi2x only have 2 sdma instances so we won't conflict with
This should say navi1x. Fixed locally.
Alex
> firmware anyway.
>
> Fixes: 468e994c41ecb3 ("drm/amdgpu/nbio2.3: don't use GPU_HDP_FLUSH bit 12")
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 -
> drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c| 15 +++
> drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h| 1 +
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 4228c7964175..9645b95b9c42 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -1133,12 +1133,15 @@ int amdgpu_discovery_set_ip_blocks(struct
> amdgpu_device *adev)
> case IP_VERSION(2, 3, 0):
> case IP_VERSION(2, 3, 1):
> case IP_VERSION(2, 3, 2):
> + adev->nbio.funcs = _v2_3_funcs;
> + adev->nbio.hdp_flush_reg = _v2_3_hdp_flush_reg;
> + break;
> case IP_VERSION(3, 3, 0):
> case IP_VERSION(3, 3, 1):
> case IP_VERSION(3, 3, 2):
> case IP_VERSION(3, 3, 3):
> adev->nbio.funcs = _v2_3_funcs;
> - adev->nbio.hdp_flush_reg = _v2_3_hdp_flush_reg;
> + adev->nbio.hdp_flush_reg = _v2_3_hdp_flush_reg_sc;
> break;
> default:
> break;
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> index 79bf6b381862..4ecd2b5808ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> @@ -314,6 +314,21 @@ static u32 nbio_v2_3_get_pcie_data_offset(struct
> amdgpu_device *adev)
> }
>
> const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg = {
> + .ref_and_mask_cp0 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP0_MASK,
> + .ref_and_mask_cp1 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP1_MASK,
> + .ref_and_mask_cp2 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP2_MASK,
> + .ref_and_mask_cp3 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP3_MASK,
> + .ref_and_mask_cp4 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP4_MASK,
> + .ref_and_mask_cp5 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP5_MASK,
> + .ref_and_mask_cp6 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP6_MASK,
> + .ref_and_mask_cp7 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP7_MASK,
> + .ref_and_mask_cp8 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP8_MASK,
> + .ref_and_mask_cp9 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP9_MASK,
> + .ref_and_mask_sdma0 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__SDMA0_MASK,
> + .ref_and_mask_sdma1 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__SDMA1_MASK,
> +};
> +
> +const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg_sc = {
> .ref_and_mask_cp0 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP0_MASK,
> .ref_and_mask_cp1 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP1_MASK,
> .ref_and_mask_cp2 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP2_MASK,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
> b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
> index a43b60acf7f6..6074dd3a1ed8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
> @@ -27,6 +27,7 @@
> #include "soc15_common.h"
>
> extern const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg;
> +extern const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg_sc;
> extern const struct amdgpu_nbio_funcs nbio_v2_3_funcs;
>
> #endif
> --
> 2.31.1
>
Re: [PATCH] MAINTAINERS: Add Siqueira for AMD DC
Hi, Acked-by: Rodrigo Siqueira Thanks a lot, I'm going to apply this patch. On 2021-10-08 5:21 p.m., Harry Wentland wrote: He's been helping maintain it for quite a while now. Make it official. Signed-off-by: Harry Wentland --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 24d520c4b157..b107ddb306de 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -876,6 +876,7 @@ F: include/uapi/linux/psp-sev.h AMD DISPLAY CORE M:Harry Wentland M:Leo Li +M: Rodrigo Siqueira L:amd-gfx@lists.freedesktop.org S:Supported T:git https://gitlab.freedesktop.org/agd5f/linux.git
Re: [PATCH] drm/amdkfd: Separate pinned BOs destruction from general routine
Am 2021-10-11 um 4:58 a.m. schrieb Lang Yu:
> Currently, all kfd BOs use same destruction routine. But pinned
> BOs are not unpinned properly. Separate them from general routine.
>
> Signed-off-by: Lang Yu
I think the general idea is right. However, we need another safeguard
for the signal BO, which is allocated by user mode and can be freed by
user mode at any time. We can solve this in one of two ways:
1. Add special handling for the signal BO in
kfd_ioctl_free_memory_of_gpu to kunmap the BO and make sure the
signal handling code is aware of it
2. Fail kfd_ioctl_free_memory_of_gpu for signal BOs and only allow them
to be destroyed at process termination
I think #2 is easier, and is consistent with what current user mode does.
A few more comment inline ...
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 2 +
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 ++
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 +
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 +
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 125 ++
> 5 files changed, 114 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 69de31754907..751557af09bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -279,6 +279,8 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
> struct kgd_dev *kgd, struct kgd_mem *mem, bool intr);
> int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct kgd_dev *kgd,
> struct kgd_mem *mem, void **kptr, uint64_t *size);
> +void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct kgd_dev *kgd,
> struct kgd_mem *mem);
> +
> int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
> struct dma_fence **ef);
> int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 054c1a224def..6acc78b02bdc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1871,6 +1871,16 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct
> kgd_dev *kgd,
> return ret;
> }
>
> +void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct kgd_dev *kgd,
> struct kgd_mem *mem)
> +{
> + struct amdgpu_bo *bo = mem->bo;
> +
> + amdgpu_bo_reserve(bo, true);
> + amdgpu_bo_kunmap(bo);
> + amdgpu_bo_unpin(bo);
> + amdgpu_bo_unreserve(bo);
> +}
> +
> int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
> struct kfd_vm_fault_info *mem)
> {
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index f1e7edeb4e6b..0db48ac10fde 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1051,6 +1051,9 @@ static int kfd_ioctl_create_event(struct file *filp,
> struct kfd_process *p,
> pr_err("Failed to set event page\n");
Need to kunmap the signal BO here.
> return err;
> }
> +
> + p->signal_handle = args->event_page_offset;
> +
> }
>
> err = kfd_event_create(filp, p, args->event_type,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 6d8f9bb2d905..30f08f1606bb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -608,12 +608,14 @@ struct qcm_process_device {
> uint32_t sh_hidden_private_base;
>
> /* CWSR memory */
> + struct kgd_mem *cwsr_mem;
> void *cwsr_kaddr;
> uint64_t cwsr_base;
> uint64_t tba_addr;
> uint64_t tma_addr;
>
> /* IB memory */
> + struct kgd_mem *ib_mem;
> uint64_t ib_base;
> void *ib_kaddr;
>
> @@ -808,6 +810,7 @@ struct kfd_process {
> /* Event ID allocator and lookup */
> struct idr event_idr;
> /* Event page */
> + u64 signal_handle;
> struct kfd_signal_page *signal_page;
> size_t signal_mapped_size;
> size_t signal_event_count;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 21ec8a18cad2..c024f2e2efaa 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -72,6 +72,8 @@ static int kfd_process_init_cwsr_apu(struct kfd_process *p,
> struct file *filep);
> static void evict_process_worker(struct work_struct *work);
> static void restore_process_worker(struct work_struct *work);
>
> +static void kfd_process_device_destroy_cwsr_dgpu(struct kfd_process_device
> *pdd);
> +
> struct kfd_procfs_tree {
> struct kobject *kobj;
> };
> @@ -685,10 +687,15 @@ void
Re: [PATCH 1/1] drm/amdgpu: release gtt bo after each move test
Please ignore this!
On 10/13/2021 5:04 PM, Nirmoy Das wrote:
When gart size is < gtt size this test will fail with
-ENOMEM as we are not freeing gtt bo after each move test.
This is generally not an issue when gart size >= gtt size.
Reported-by: zhang
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
index 909d830b513e..0cf2a560d673 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
@@ -212,7 +212,6 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset 0x%llx\n",
gart_addr - adev->gmc.gart_start);
- continue;
out_lclean_unpin:
amdgpu_bo_unpin(gtt_obj[i]);
@@ -220,6 +219,7 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
amdgpu_bo_unreserve(gtt_obj[i]);
out_lclean_unref:
amdgpu_bo_unref(_obj[i]);
+ continue;
out_lclean:
for (--i; i >= 0; --i) {
amdgpu_bo_unpin(gtt_obj[i]);
[PATCH 1/1] drm/amdgpu: fix BO leak after successful move test
GTT BO cleanup code is with in the test for loop and
we would skip cleaning up GTT BO on success.
Reported-by: zhang
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 25
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
index 909d830b513e..5fe7ff680c29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
@@ -35,6 +35,7 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
struct amdgpu_bo *vram_obj = NULL;
struct amdgpu_bo **gtt_obj = NULL;
struct amdgpu_bo_param bp;
+ struct dma_fence *fence = NULL;
uint64_t gart_addr, vram_addr;
unsigned n, size;
int i, r;
@@ -82,7 +83,6 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
void *gtt_map, *vram_map;
void **gart_start, **gart_end;
void **vram_start, **vram_end;
- struct dma_fence *fence = NULL;
bp.domain = AMDGPU_GEM_DOMAIN_GTT;
r = amdgpu_bo_create(adev, , gtt_obj + i);
@@ -212,24 +212,23 @@ static void amdgpu_do_test_moves(struct amdgpu_device
*adev)
DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset
0x%llx\n",
gart_addr - adev->gmc.gart_start);
- continue;
+ }
+ --i;
out_lclean_unpin:
- amdgpu_bo_unpin(gtt_obj[i]);
+ amdgpu_bo_unpin(gtt_obj[i]);
out_lclean_unres:
- amdgpu_bo_unreserve(gtt_obj[i]);
+ amdgpu_bo_unreserve(gtt_obj[i]);
out_lclean_unref:
- amdgpu_bo_unref(_obj[i]);
+ amdgpu_bo_unref(_obj[i]);
out_lclean:
- for (--i; i >= 0; --i) {
- amdgpu_bo_unpin(gtt_obj[i]);
- amdgpu_bo_unreserve(gtt_obj[i]);
- amdgpu_bo_unref(_obj[i]);
- }
- if (fence)
- dma_fence_put(fence);
- break;
+ for (--i; i >= 0; --i) {
+ amdgpu_bo_unpin(gtt_obj[i]);
+ amdgpu_bo_unreserve(gtt_obj[i]);
+ amdgpu_bo_unref(_obj[i]);
}
+ if (fence)
+ dma_fence_put(fence);
amdgpu_bo_unpin(vram_obj);
out_unres:
--
2.32.0
Re: [PATCH 1/1] drm/amdgpu: release gtt bo after each move test
On 10/13/2021 12:42 PM, Das, Nirmoy wrote:
On 10/13/2021 3:22 AM, zhang wrote:
Hi . Nirmoy
If you let continue to unpin. this will allways test a same va for gtt
I think we should rafresh calculate the value n
Right, I guess then the test should only run till gart size.
Actually the test size calculation was fine, it is just that we wouldn't
release BO after a successful test as the cleanup code is inside the
test for loop.
Regards,
Nirmoy
Regards,
Nirmoy
On 2021/10/12 20:10, Nirmoy Das wrote:
When gart size is < gtt size this test will fail with
-ENOMEM as we are not freeing gtt bo after each move test.
This is generally not an issue when gart size >= gtt size.
Reported-by: zhang
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
index 909d830b513e..0cf2a560d673 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
@@ -212,7 +212,6 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset 0x%llx\n",
gart_addr - adev->gmc.gart_start);
- continue;
out_lclean_unpin:
amdgpu_bo_unpin(gtt_obj[i]);
@@ -220,6 +219,7 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
amdgpu_bo_unreserve(gtt_obj[i]);
out_lclean_unref:
amdgpu_bo_unref(_obj[i]);
+ continue;
out_lclean:
for (--i; i >= 0; --i) {
amdgpu_bo_unpin(gtt_obj[i]);
[PATCH] drm/amdgpu/nbio2.3: use original HDP_FLUSH bits for navi1x
The extended bits were not available for use on navi1x, but
navi2x only have 2 sdma instances so we won't conflict with
firmware anyway.
Fixes: 468e994c41ecb3 ("drm/amdgpu/nbio2.3: don't use GPU_HDP_FLUSH bit 12")
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 -
drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c| 15 +++
drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h| 1 +
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 4228c7964175..9645b95b9c42 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1133,12 +1133,15 @@ int amdgpu_discovery_set_ip_blocks(struct amdgpu_device
*adev)
case IP_VERSION(2, 3, 0):
case IP_VERSION(2, 3, 1):
case IP_VERSION(2, 3, 2):
+ adev->nbio.funcs = _v2_3_funcs;
+ adev->nbio.hdp_flush_reg = _v2_3_hdp_flush_reg;
+ break;
case IP_VERSION(3, 3, 0):
case IP_VERSION(3, 3, 1):
case IP_VERSION(3, 3, 2):
case IP_VERSION(3, 3, 3):
adev->nbio.funcs = _v2_3_funcs;
- adev->nbio.hdp_flush_reg = _v2_3_hdp_flush_reg;
+ adev->nbio.hdp_flush_reg = _v2_3_hdp_flush_reg_sc;
break;
default:
break;
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
index 79bf6b381862..4ecd2b5808ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
@@ -314,6 +314,21 @@ static u32 nbio_v2_3_get_pcie_data_offset(struct
amdgpu_device *adev)
}
const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg = {
+ .ref_and_mask_cp0 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP0_MASK,
+ .ref_and_mask_cp1 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP1_MASK,
+ .ref_and_mask_cp2 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP2_MASK,
+ .ref_and_mask_cp3 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP3_MASK,
+ .ref_and_mask_cp4 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP4_MASK,
+ .ref_and_mask_cp5 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP5_MASK,
+ .ref_and_mask_cp6 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP6_MASK,
+ .ref_and_mask_cp7 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP7_MASK,
+ .ref_and_mask_cp8 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP8_MASK,
+ .ref_and_mask_cp9 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP9_MASK,
+ .ref_and_mask_sdma0 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__SDMA0_MASK,
+ .ref_and_mask_sdma1 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__SDMA1_MASK,
+};
+
+const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg_sc = {
.ref_and_mask_cp0 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP0_MASK,
.ref_and_mask_cp1 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP1_MASK,
.ref_and_mask_cp2 = BIF_BX_PF_GPU_HDP_FLUSH_DONE__CP2_MASK,
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
index a43b60acf7f6..6074dd3a1ed8 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.h
@@ -27,6 +27,7 @@
#include "soc15_common.h"
extern const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg;
+extern const struct nbio_hdp_flush_reg nbio_v2_3_hdp_flush_reg_sc;
extern const struct amdgpu_nbio_funcs nbio_v2_3_funcs;
#endif
--
2.31.1
[PATCH 1/1] drm/amdgpu: release gtt bo after each move test
When gart size is < gtt size this test will fail with
-ENOMEM as we are not freeing gtt bo after each move test.
This is generally not an issue when gart size >= gtt size.
Reported-by: zhang
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
index 909d830b513e..0cf2a560d673 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
@@ -212,7 +212,6 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset
0x%llx\n",
gart_addr - adev->gmc.gart_start);
- continue;
out_lclean_unpin:
amdgpu_bo_unpin(gtt_obj[i]);
@@ -220,6 +219,7 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
amdgpu_bo_unreserve(gtt_obj[i]);
out_lclean_unref:
amdgpu_bo_unref(_obj[i]);
+ continue;
out_lclean:
for (--i; i >= 0; --i) {
amdgpu_bo_unpin(gtt_obj[i]);
--
2.32.0
Re: [PATCH 1/5] drm/amd/pm: Slight function rename
On 2021-10-13 10:50, Russell, Kent wrote:
> [AMD Official Use Only]
>
> Pedantic tiny thing:
>
>> -Original Message-
>> From: amd-gfx On Behalf Of Luben
>> Tuikov
>> Sent: Tuesday, October 12, 2021 11:11 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander ; Tuikov, Luben
>>
>> Subject: [PATCH 1/5] drm/amd/pm: Slight function rename
>>
>> Rename
>> sienna_cichlid_is_support_fine_grained_dpm() to
>> sienna_cichlid_support_fine_grained_dpm().
> ^You would want cichlid_supports_fine_grained_dpm . The function is correct
> below, but anyone grepping git logs would miss it.
Ah, yes, thank you Kent--I'll fix the description to add the missing 's'.
Regards,
Luben
>
> Kent
>
>> Rename
>> navi10_is_support_fine_grained_dpm() to
>> navi10_supports_fine_grained_dpm().
>>
>> Signed-off-by: Luben Tuikov
>> ---
>> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 7 ---
>> drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 7 ---
>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> index 71161f6b78fea9..0fe9790f67f5af 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> @@ -1231,7 +1231,8 @@ static int navi10_get_current_clk_freq_by_table(struct
>> smu_context *smu,
>> value);
>> }
>>
>> -static bool navi10_is_support_fine_grained_dpm(struct smu_context *smu, enum
>> smu_clk_type clk_type)
>> +static bool navi10_supports_fine_grained_dpm(struct smu_context *smu,
>> + enum smu_clk_type clk_type)
>> {
>> PPTable_t *pptable = smu->smu_table.driver_pptable;
>> DpmDescriptor_t *dpm_desc = NULL;
>> @@ -1299,7 +1300,7 @@ static int navi10_print_clk_levels(struct smu_context
>> *smu,
>> if (ret)
>> return size;
>>
>> -if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
>> +if (!navi10_supports_fine_grained_dpm(smu, clk_type)) {
>> for (i = 0; i < count; i++) {
>> ret = smu_v11_0_get_dpm_freq_by_index(smu,
>> clk_type, i,
>> );
>> if (ret)
>> @@ -1465,7 +1466,7 @@ static int navi10_force_clk_levels(struct smu_context
>> *smu,
>> case SMU_UCLK:
>> case SMU_FCLK:
>> /* There is only 2 levels for fine grained DPM */
>> -if (navi10_is_support_fine_grained_dpm(smu, clk_type)) {
>> +if (navi10_supports_fine_grained_dpm(smu, clk_type)) {
>> soft_max_level = (soft_max_level >= 1 ? 1 : 0);
>> soft_min_level = (soft_min_level >= 1 ? 1 : 0);
>> }
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> index 15e66e1912de33..3f5721baa5ff50 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> @@ -1006,7 +1006,8 @@ static int
>> sienna_cichlid_get_current_clk_freq_by_table(struct
>> smu_context *smu,
>>
>> }
>>
>> -static bool sienna_cichlid_is_support_fine_grained_dpm(struct smu_context
>> *smu, enum
>> smu_clk_type clk_type)
>> +static bool sienna_cichlid_supports_fine_grained_dpm(struct smu_context
>> *smu,
>> + enum smu_clk_type clk_type)
>> {
>> DpmDescriptor_t *dpm_desc = NULL;
>> DpmDescriptor_t *table_member;
>> @@ -1084,7 +1085,7 @@ static int sienna_cichlid_print_clk_levels(struct
>> smu_context
>> *smu,
>> if (ret)
>> goto print_clk_out;
>>
>> -if (!sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type))
>> {
>> +if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
>> for (i = 0; i < count; i++) {
>> ret = smu_v11_0_get_dpm_freq_by_index(smu,
>> clk_type, i,
>> );
>> if (ret)
>> @@ -1235,7 +1236,7 @@ static int sienna_cichlid_force_clk_levels(struct
>> smu_context
>> *smu,
>> case SMU_UCLK:
>> case SMU_FCLK:
>> /* There is only 2 levels for fine grained DPM */
>> -if (sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type)) {
>> +if (sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
>> soft_max_level = (soft_max_level >= 1 ? 1 : 0);
>> soft_min_level = (soft_min_level >= 1 ? 1 : 0);
>> }
>> --
>> 2.33.1.558.g2bd2f258f4
RE: [PATCH 1/5] drm/amd/pm: Slight function rename
[AMD Official Use Only]
Pedantic tiny thing:
> -Original Message-
> From: amd-gfx On Behalf Of Luben
> Tuikov
> Sent: Tuesday, October 12, 2021 11:11 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Tuikov, Luben
>
> Subject: [PATCH 1/5] drm/amd/pm: Slight function rename
>
> Rename
> sienna_cichlid_is_support_fine_grained_dpm() to
> sienna_cichlid_support_fine_grained_dpm().
^You would want cichlid_supports_fine_grained_dpm . The function is correct
below, but anyone grepping git logs would miss it.
Kent
>
> Rename
> navi10_is_support_fine_grained_dpm() to
> navi10_supports_fine_grained_dpm().
>
> Signed-off-by: Luben Tuikov
> ---
> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 7 ---
> drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 7 ---
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 71161f6b78fea9..0fe9790f67f5af 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -1231,7 +1231,8 @@ static int navi10_get_current_clk_freq_by_table(struct
> smu_context *smu,
> value);
> }
>
> -static bool navi10_is_support_fine_grained_dpm(struct smu_context *smu, enum
> smu_clk_type clk_type)
> +static bool navi10_supports_fine_grained_dpm(struct smu_context *smu,
> + enum smu_clk_type clk_type)
> {
> PPTable_t *pptable = smu->smu_table.driver_pptable;
> DpmDescriptor_t *dpm_desc = NULL;
> @@ -1299,7 +1300,7 @@ static int navi10_print_clk_levels(struct smu_context
> *smu,
> if (ret)
> return size;
>
> - if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + if (!navi10_supports_fine_grained_dpm(smu, clk_type)) {
> for (i = 0; i < count; i++) {
> ret = smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, i,
> );
> if (ret)
> @@ -1465,7 +1466,7 @@ static int navi10_force_clk_levels(struct smu_context
> *smu,
> case SMU_UCLK:
> case SMU_FCLK:
> /* There is only 2 levels for fine grained DPM */
> - if (navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + if (navi10_supports_fine_grained_dpm(smu, clk_type)) {
> soft_max_level = (soft_max_level >= 1 ? 1 : 0);
> soft_min_level = (soft_min_level >= 1 ? 1 : 0);
> }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index 15e66e1912de33..3f5721baa5ff50 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -1006,7 +1006,8 @@ static int
> sienna_cichlid_get_current_clk_freq_by_table(struct
> smu_context *smu,
>
> }
>
> -static bool sienna_cichlid_is_support_fine_grained_dpm(struct smu_context
> *smu, enum
> smu_clk_type clk_type)
> +static bool sienna_cichlid_supports_fine_grained_dpm(struct smu_context *smu,
> + enum smu_clk_type clk_type)
> {
> DpmDescriptor_t *dpm_desc = NULL;
> DpmDescriptor_t *table_member;
> @@ -1084,7 +1085,7 @@ static int sienna_cichlid_print_clk_levels(struct
> smu_context
> *smu,
> if (ret)
> goto print_clk_out;
>
> - if (!sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type))
> {
> + if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
> for (i = 0; i < count; i++) {
> ret = smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, i,
> );
> if (ret)
> @@ -1235,7 +1236,7 @@ static int sienna_cichlid_force_clk_levels(struct
> smu_context
> *smu,
> case SMU_UCLK:
> case SMU_FCLK:
> /* There is only 2 levels for fine grained DPM */
> - if (sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type)) {
> + if (sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
> soft_max_level = (soft_max_level >= 1 ? 1 : 0);
> soft_min_level = (soft_min_level >= 1 ? 1 : 0);
> }
> --
> 2.33.1.558.g2bd2f258f4
[PATCH] drm/amd/display: fix apply_degamma_for_user_regamma() warning
From: Arnd Bergmann
It appears that the wrong argument was removed in this call:
drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c: In function
'apply_degamma_for_user_regamma':
drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1694:36:
error: implicit conversion from 'enum ' to 'enum
dc_transfer_func_predefined' [-Werror=enum-conversion]
1694 | build_coefficients(, true);
Fixes: 9b3d76527f6e ("drm/amd/display: Revert adding degamma coefficients")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
index 64a38f08f497..4cb6617059ae 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1691,7 +1691,7 @@ static void apply_degamma_for_user_regamma(struct
pwl_float_data_ex *rgb_regamma
struct pwl_float_data_ex *rgb = rgb_regamma;
const struct hw_x_point *coord_x = coordinates_x;
- build_coefficients(, true);
+ build_coefficients(, TRANSFER_FUNCTION_SRGB);
i = 0;
while (i != hw_points_num + 1) {
--
2.29.2
Re: [PATCH] drm/amdkfd: Fix an inappropriate error handling in allloc memory of gpu
Am 2021-10-13 um 3:28 a.m. schrieb Lang Yu:
> We should unreference a gem object instead of an amdgpu bo here.
>
> Fixes: 5ae0283e831a ("drm/amdgpu: Add userptr support for KFD")
I think the Fixes tag is incorrect. At that time there was no gobj in
this function.
If anything I'd attribute the problem to fd9a9f8801de ("drm/amdgpu: Use
GEM obj reference for KFD BOs"), where I missed the error handling code
path.
Other than that, the patch is
Reviewed-by: Felix Kuehling
>
> Signed-off-by: Lang Yu
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 054c1a224def..cdf46bd0d8d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1503,7 +1503,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
> drm_vma_node_revoke(>vma_node, drm_priv);
> err_node_allow:
> - amdgpu_bo_unref();
> + drm_gem_object_put(gobj);
> /* Don't unreserve system mem limit twice */
> goto err_reserve_limit;
> err_bo_create:
Re: [PATCH 0/5] 0 MHz is not a valid current frequency
On 10/13/2021 7:28 PM, Alex Deucher wrote: On Wed, Oct 13, 2021 at 12:14 AM Lazar, Lijo wrote: On 10/13/2021 8:40 AM, Luben Tuikov wrote: Some ASIC support low-power functionality for the whole ASIC or just an IP block. When in such low-power mode, some sysfs interfaces would report a frequency of 0, e.g., $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 500Mhz 1: 0Mhz * 2: 2200Mhz $_ An operating frequency of 0 MHz doesn't make sense, and this interface is designed to report only operating clock frequencies, i.e. non-zero, and possibly the current one. When in this low-power state, round to the smallest operating frequency, for this interface, as follows, Would rather avoid this - 1) It is manipulating FW reported value. If at all there is an uncaught issue in FW reporting of frequency values, that is masked here. 2) Otherwise, if 0MHz is described as GFX power gated case, this provides a convenient interface to check if GFX is power gated. If seeing a '0' is not pleasing, consider changing to something like "NA" - not available (frequency cannot be fetched at the moment). I don't think this interface is really supposed to be the way to get the current clock, although some use it that way. It's supposed to show the DPM states and which are active. conflating the interface with other information confuses things in my opinion. Why is the current clock less than the minimum clock? Whether or not an IP is turned off or in deep sleep or not is independent of DPM states. When the IP is active, it will never go below the minimum DPM level. If we want to query deep sleep or gfxoff we can use the amdgpu_pm_info debugfs interface or add some other new interface. The idea of DPM level is deprecated with fine grained clock which provides only min and max. For fine grained clock, we fetch the current clock frequency and show it as an artificial DPM level between min/max. That itself should have confused users but it is not which means the users use the * to fetch the current frequency and not really bothered about the DPM level per se. Also, some ASICs define 'min' as as the least possible freq (that happens during a throttle) and not the DPM level 0 min in the traditional sense (that is defined as idle frequency which doesn't come into min/max levels). It's usually from the idle frequency that GFX gets power gated. Showing a * against min in such cases would be confusing because that could be misinterpreted as a throttle scenario. Thanks, Lijo Alex Thanks, Lijo $cat /sys/class/drm/card0/device/pp_dpm_sclk 0: 500Mhz * 1: 2200Mhz $_ Luben Tuikov (5): drm/amd/pm: Slight function rename drm/amd/pm: Rename cur_value to curr_value drm/amd/pm: Rename freq_values --> freq_value dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 --- 2 files changed, 86 insertions(+), 47 deletions(-)
Re: [PATCH 0/5] 0 MHz is not a valid current frequency
On Wed, Oct 13, 2021 at 12:14 AM Lazar, Lijo wrote: > > > > On 10/13/2021 8:40 AM, Luben Tuikov wrote: > > Some ASIC support low-power functionality for the whole ASIC or just > > an IP block. When in such low-power mode, some sysfs interfaces would > > report a frequency of 0, e.g., > > > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > > 0: 500Mhz > > 1: 0Mhz * > > 2: 2200Mhz > > $_ > > > > An operating frequency of 0 MHz doesn't make sense, and this interface > > is designed to report only operating clock frequencies, i.e. non-zero, > > and possibly the current one. > > > > When in this low-power state, round to the smallest > > operating frequency, for this interface, as follows, > > > > Would rather avoid this - > > 1) It is manipulating FW reported value. If at all there is an uncaught > issue in FW reporting of frequency values, that is masked here. > 2) Otherwise, if 0MHz is described as GFX power gated case, this > provides a convenient interface to check if GFX is power gated. > > If seeing a '0' is not pleasing, consider changing to something like > "NA" - not available (frequency cannot be fetched at the moment). > I don't think this interface is really supposed to be the way to get the current clock, although some use it that way. It's supposed to show the DPM states and which are active. conflating the interface with other information confuses things in my opinion. Why is the current clock less than the minimum clock? Whether or not an IP is turned off or in deep sleep or not is independent of DPM states. When the IP is active, it will never go below the minimum DPM level. If we want to query deep sleep or gfxoff we can use the amdgpu_pm_info debugfs interface or add some other new interface. Alex > Thanks, > Lijo > > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > > 0: 500Mhz * > > 1: 2200Mhz > > $_ > > > > Luben Tuikov (5): > >drm/amd/pm: Slight function rename > >drm/amd/pm: Rename cur_value to curr_value > >drm/amd/pm: Rename freq_values --> freq_value > >dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency > >dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency > > > > .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- > > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 --- > > 2 files changed, 86 insertions(+), 47 deletions(-) > >
Re: [PATCH 1/3] drm:Enable buddy allocator support
On Wed, Oct 13, 2021 at 07:05:34PM +0530, Arunpravin wrote:
> Port Intel buddy manager to drm root folder
One patch to move it 1:1, then follow-up patches to change it. Not
everything in one.
Also i915 needs to be adopted to use this too, or this just doesn't make
sense.
I'm also wondering whether we shouldn't have a ttm helper for this
readymade so it just glues all in?
-Daniel
> Implemented range allocation support for the provided order
> Implemented TOP-DOWN support
> Implemented freeing up unused pages on contiguous allocation
> Moved range allocation and freelist pickup into a single function
>
> Signed-off-by: Arunpravin
> ---
> drivers/gpu/drm/Makefile| 2 +-
> drivers/gpu/drm/drm_buddy.c | 705
> drivers/gpu/drm/drm_drv.c | 3 +
> include/drm/drm_buddy.h | 157
> 4 files changed, 866 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/drm_buddy.c
> create mode 100644 include/drm/drm_buddy.h
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index a118692a6df7..fe1a2fc09675 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -18,7 +18,7 @@ drm-y := drm_aperture.o drm_auth.o drm_cache.o \
> drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o \
> - drm_managed.o drm_vblank_work.o
> + drm_managed.o drm_vblank_work.o drm_buddy.o
>
> drm-$(CONFIG_DRM_LEGACY) += drm_agpsupport.o drm_bufs.o drm_context.o
> drm_dma.o \
> drm_legacy_misc.o drm_lock.o drm_memory.o
> drm_scatter.o \
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> new file mode 100644
> index ..8cd118574665
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -0,0 +1,705 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include
> +#include
> +
> +#include
> +
> +static struct kmem_cache *slab_blocks;
> +
> +static struct drm_buddy_block *drm_block_alloc(struct drm_buddy_mm *mm,
> +struct drm_buddy_block *parent,
> +unsigned int order,
> +u64 offset)
> +{
> + struct drm_buddy_block *block;
> +
> + BUG_ON(order > DRM_BUDDY_MAX_ORDER);
> +
> + block = kmem_cache_zalloc(slab_blocks, GFP_KERNEL);
> + if (!block)
> + return NULL;
> +
> + block->header = offset;
> + block->header |= order;
> + block->parent = parent;
> +
> + BUG_ON(block->header & DRM_BUDDY_HEADER_UNUSED);
> + return block;
> +}
> +
> +static void drm_block_free(struct drm_buddy_mm *mm,
> +struct drm_buddy_block *block)
> +{
> + kmem_cache_free(slab_blocks, block);
> +}
> +
> +static void mark_allocated(struct drm_buddy_block *block)
> +{
> + block->header &= ~DRM_BUDDY_HEADER_STATE;
> + block->header |= DRM_BUDDY_ALLOCATED;
> +
> + list_del(>link);
> +}
> +
> +static void mark_free(struct drm_buddy_mm *mm,
> + struct drm_buddy_block *block)
> +{
> + block->header &= ~DRM_BUDDY_HEADER_STATE;
> + block->header |= DRM_BUDDY_FREE;
> +
> + list_add(>link,
> + >free_list[drm_buddy_block_order(block)]);
> +}
> +
> +static void mark_split(struct drm_buddy_block *block)
> +{
> + block->header &= ~DRM_BUDDY_HEADER_STATE;
> + block->header |= DRM_BUDDY_SPLIT;
> +
> + list_del(>link);
> +}
> +
> +/**
> + * drm_buddy_init - init memory manager
> + *
> + * @mm: DRM buddy manager to initialize
> + * @size: size in bytes to manage
> + * @chunk_size: minimum page size in bytes for our allocations
> + *
> + * Initializes the memory manager and its resources.
> + *
> + * Returns:
> + * 0 on success, error code on failure.
> + */
> +int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
> +{
> + unsigned int i;
> + u64 offset;
> +
> + if (size < chunk_size)
> + return -EINVAL;
> +
> + if (chunk_size < PAGE_SIZE)
> + return -EINVAL;
> +
> + if (!is_power_of_2(chunk_size))
> + return -EINVAL;
> +
> + size = round_down(size, chunk_size);
> +
> + mm->size = size;
> + mm->avail = size;
> + mm->chunk_size = chunk_size;
> + mm->max_order = ilog2(size) - ilog2(chunk_size);
> +
> + BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
> +
> + mm->free_list = kmalloc_array(mm->max_order + 1,
> + sizeof(struct list_head),
> + GFP_KERNEL);
> + if (!mm->free_list)
> + return -ENOMEM;
> +
> + for (i = 0; i <= mm->max_order; ++i)
> + INIT_LIST_HEAD(>free_list[i]);
> +
> + mm->n_roots = hweight64(size);
> +
> +
[PATCH 3/3] drm/amdgpu: Replace drm_mm with drm buddy manager
Add drm buddy allocator support for vram memory management
Signed-off-by: Arunpravin
---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 97 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 251 ++
3 files changed, 217 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index acfa207cf970..2c17e948355e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -30,12 +30,15 @@
#include
#include
+#include "amdgpu_vram_mgr.h"
+
/* state back for walking over vram_mgr and gtt_mgr allocations */
struct amdgpu_res_cursor {
uint64_tstart;
uint64_tsize;
uint64_tremaining;
- struct drm_mm_node *node;
+ void*node;
+ uint32_tmem_type;
};
/**
@@ -52,27 +55,63 @@ static inline void amdgpu_res_first(struct ttm_resource
*res,
uint64_t start, uint64_t size,
struct amdgpu_res_cursor *cur)
{
+ struct drm_buddy_block *block;
+ struct list_head *head, *next;
struct drm_mm_node *node;
- if (!res || res->mem_type == TTM_PL_SYSTEM) {
- cur->start = start;
- cur->size = size;
- cur->remaining = size;
- cur->node = NULL;
- WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
- return;
- }
+ if (!res)
+ goto err_out;
BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
- node = to_ttm_range_mgr_node(res)->mm_nodes;
- while (start >= node->size << PAGE_SHIFT)
- start -= node++->size << PAGE_SHIFT;
+ cur->mem_type = res->mem_type;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ head = _amdgpu_vram_mgr_node(res)->blocks;
+
+ block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+link);
+ if (!block)
+ goto err_out;
+
+ while (start >= node_size(block)) {
+ start -= node_size(block);
+
+ next = block->link.next;
+ if (next != head)
+ block = list_entry(next, struct
drm_buddy_block, link);
+ }
+
+ cur->start = node_start(block) + start;
+ cur->size = min(node_size(block) - start, size);
+ cur->remaining = size;
+ cur->node = block;
+ break;
+ case TTM_PL_TT:
+ node = to_ttm_range_mgr_node(res)->mm_nodes;
+ while (start >= node->size << PAGE_SHIFT)
+ start -= node++->size << PAGE_SHIFT;
+
+ cur->start = (node->start << PAGE_SHIFT) + start;
+ cur->size = min((node->size << PAGE_SHIFT) - start, size);
+ cur->remaining = size;
+ cur->node = node;
+ break;
+ default:
+ goto err_out;
+ }
- cur->start = (node->start << PAGE_SHIFT) + start;
- cur->size = min((node->size << PAGE_SHIFT) - start, size);
+ return;
+
+err_out:
+ cur->start = start;
+ cur->size = size;
cur->remaining = size;
- cur->node = node;
+ cur->node = NULL;
+ WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
+ return;
}
/**
@@ -85,7 +124,9 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
*/
static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t
size)
{
- struct drm_mm_node *node = cur->node;
+ struct drm_buddy_block *block;
+ struct drm_mm_node *node;
+ struct list_head *next;
BUG_ON(size > cur->remaining);
@@ -99,9 +140,27 @@ static inline void amdgpu_res_next(struct amdgpu_res_cursor
*cur, uint64_t size)
return;
}
- cur->node = ++node;
- cur->start = node->start << PAGE_SHIFT;
- cur->size = min(node->size << PAGE_SHIFT, cur->remaining);
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ next = block->link.next;
+ block = list_entry(next, struct drm_buddy_block, link);
+
+ cur->node = block;
+ cur->start = node_start(block);
+ cur->size = min(node_size(block), cur->remaining);
+ break;
+ case TTM_PL_TT:
+ node = cur->node;
+
+ cur->node = ++node;
+ cur->start = node->start << PAGE_SHIFT;
+ cur->size = min(node->size << PAGE_SHIFT,
[PATCH 2/3] drm/amdgpu:move vram manager defines into a header file
Move vram related defines and inline functions into
a separate header file
Signed-off-by: Arunpravin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 72
1 file changed, 72 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
new file mode 100644
index ..fcab6475ccbb
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: MIT
+ * Copyright 2021 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __AMDGPU_VRAM_MGR_H__
+#define __AMDGPU_VRAM_MGR_H__
+
+#include
+
+struct amdgpu_vram_mgr_node {
+ struct ttm_resource base;
+ struct list_head blocks;
+ unsigned long flags;
+};
+
+struct amdgpu_vram_reservation {
+ uint64_t start;
+ uint64_t size;
+ uint64_t min_size;
+ unsigned long flags;
+ struct list_head block;
+ struct list_head node;
+};
+
+static inline uint64_t node_start(struct drm_buddy_block *block)
+{
+ return drm_buddy_block_offset(block);
+}
+
+static inline uint64_t node_size(struct drm_buddy_block *block)
+{
+ return PAGE_SIZE << drm_buddy_block_order(block);
+}
+
+static inline struct amdgpu_vram_mgr_node *
+to_amdgpu_vram_mgr_node(struct ttm_resource *res)
+{
+ return container_of(res, struct amdgpu_vram_mgr_node, base);
+}
+
+static inline struct amdgpu_vram_mgr *
+to_vram_mgr(struct ttm_resource_manager *man)
+{
+ return container_of(man, struct amdgpu_vram_mgr, manager);
+}
+
+static inline struct amdgpu_device *
+to_amdgpu_device(struct amdgpu_vram_mgr *mgr)
+{
+ return container_of(mgr, struct amdgpu_device, mman.vram_mgr);
+}
+
+#endif
--
2.25.1
Re: [PATCH v1 00/12] MEMORY_DEVICE_COHERENT for CPU-accessible coherent device memory
On Tue, Oct 12, 2021 at 03:56:29PM -0300, Jason Gunthorpe wrote: > On Tue, Oct 12, 2021 at 11:39:57AM -0700, Andrew Morton wrote: > > On Tue, 12 Oct 2021 12:12:35 -0500 Alex Sierra wrote: > > > > > This patch series introduces MEMORY_DEVICE_COHERENT, a type of memory > > > owned by a device that can be mapped into CPU page tables like > > > MEMORY_DEVICE_GENERIC and can also be migrated like MEMORY_DEVICE_PRIVATE. > > > With MEMORY_DEVICE_COHERENT, we isolate the new memory type from other > > > subsystems as far as possible, though there are some small changes to > > > other subsystems such as filesystem DAX, to handle the new memory type > > > appropriately. > > > > > > We use ZONE_DEVICE for this instead of NUMA so that the amdgpu > > > allocator can manage it without conflicting with core mm for non-unified > > > memory use cases. > > > > > > How it works: The system BIOS advertises the GPU device memory (aka VRAM) > > > as SPM (special purpose memory) in the UEFI system address map. > > > The amdgpu driver registers the memory with devmap as > > > MEMORY_DEVICE_COHERENT using devm_memremap_pages. > > > > > > The initial user for this hardware page migration capability will be > > > the Frontier supercomputer project. > > > > To what other uses will this infrastructure be put? > > > > Because I must ask: if this feature is for one single computer which > > presumably has a custom kernel, why add it to mainline Linux? > > Well, it certainly isn't just "one single computer". Overall I know of > about, hmm, ~10 *datacenters* worth of installations that are using > similar technology underpinnings. > > "Frontier" is the code name for a specific installation but as the > technology is proven out there will be many copies made of that same > approach. > > The previous program "Summit" was done with NVIDIA GPUs and PowerPC > CPUs and also included a very similar capability. I think this is a > good sign that this coherently attached accelerator will continue to > be a theme in computing going foward. IIRC this was done using out of > tree kernel patches and NUMA localities. > > Specifically with CXL now being standardized and on a path to ubiquity > I think we will see an explosion in deployments of coherently attached > accelerator memory. This is the high end trickling down to wider > usage. > > I strongly think many CXL accelerators are going to want to manage > their on-accelerator memory in this way as it makes universal sense to > want to carefully manage memory access locality to optimize for > performance. Yeah with CXL this will be used by a lot more drivers/devices, not even including nvidia's blob. I guess if you want make sure get an ack on this from CXL folks, so that we don't end up with a mess. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: bf756fb833cb ("drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend")
On Wed, Oct 13, 2021 at 09:19:45AM +, Quan, Evan wrote: > So, I need your help to confirm the last two patches(I sent you) do not > affect the fix for the bug above. > Please follow the steps below to verify it: > 1. Launch a video playing > 2. open another terminal and issue "sudo pm-suspend" command to force > suspending > 3. verify the system can suspend and resume back correctly without errors or > hangs Just to confirm: you want me to do that with the last two patches applied? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/amdkfd: Fix an inappropriate error handling in allloc memory of gpu
LGTM as we create a gem object 1st and retrieve amdgpu_bo from the gem
object.
Acked-by: Nirmoy Das
On 10/13/2021 9:28 AM, Lang Yu wrote:
We should unreference a gem object instead of an amdgpu bo here.
Fixes: 5ae0283e831a ("drm/amdgpu: Add userptr support for KFD")
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 054c1a224def..cdf46bd0d8d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1503,7 +1503,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
drm_vma_node_revoke(>vma_node, drm_priv);
err_node_allow:
- amdgpu_bo_unref();
+ drm_gem_object_put(gobj);
/* Don't unreserve system mem limit twice */
goto err_reserve_limit;
err_bo_create:
Re: [PATCH v2 1/2] drm/amdkfd: fix boot failure when iommu is disabled in Picasso.
Reviewed-by:JamesZhufortheseries. When IOMMU disabled in sbios and kfd in iommuv2 path, iommuv2 init will fail. But this failure should not block amdgpu driver init. Reported-by: youling Tested-by: youling Signed-off-by: Yifan Zhang --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 drivers/gpu/drm/amd/amdkfd/kfd_device.c| 3 +++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 52b24334a19e..ef467216ff8a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2397,10 +2397,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) if (!adev->gmc.xgmi.pending_reset) amdgpu_amdkfd_device_init(adev); - r = amdgpu_amdkfd_resume_iommu(adev); - if (r) - goto init_failed; - amdgpu_fru_get_product_info(adev); init_failed: diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c index 064d42acd54e..08eedbc6699d 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c @@ -1029,6 +1029,9 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd, svm_migrate_init((struct amdgpu_device *)kfd->kgd); + if(kgd2kfd_resume_iommu(kfd)) + goto device_iommu_error; + if (kfd_resume(kfd)) goto kfd_resume_error;
Re: [PATCH Review 1/1] drm/ttm: fix debugfs node create failed
Am 12.10.21 um 15:12 schrieb Das, Nirmoy:
On 10/12/2021 1:58 PM, Stanley.Yang wrote:
Test scenario:
modprobe amdgpu -> rmmod amdgpu -> modprobe amdgpu
Error log:
[ 54.396807] debugfs: File 'page_pool' in directory 'amdttm'
already present!
[ 54.396833] debugfs: File 'page_pool_shrink' in directory
'amdttm' already present!
[ 54.396848] debugfs: File 'buffer_objects' in directory
'amdttm' already present!
We should instead add a check if those debugfs files already
exist/created in ttm debugfs dir using debugfs_lookup() before creating.
No, IIRC the Intel guys had fixed that already by adding/removing the
debugfs file on module load/unload.
Christian.
Regards,
Nirmoy
Reason:
page_pool, page_pool_shrink and buffer_objects can be removed when
rmmod amdttm, in the above test scenario only rmmod amdgpu, so
those
debugfs node will not be removed, this caused file create failed.
Soultion:
create ttm_page directory under ttm_root directory when insmod
amdgpu,
page_pool, page_pool_shrink and buffer_objects are stored in
ttm_page directiry,
remove ttm_page directory when do rmmod amdgpu, this can fix
above issue.
Signed-off-by: Stanley.Yang
---
drivers/gpu/drm/ttm/ttm_device.c | 12 +++-
drivers/gpu/drm/ttm/ttm_module.c | 1 +
drivers/gpu/drm/ttm/ttm_module.h | 1 +
drivers/gpu/drm/ttm/ttm_pool.c | 4 ++--
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c
b/drivers/gpu/drm/ttm/ttm_device.c
index 1de23edbc182..ad170328f0c8 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -55,6 +55,10 @@ static void ttm_global_release(void)
ttm_pool_mgr_fini();
+#ifdef CONFIG_DEBUG_FS
+ debugfs_remove(ttm_debugfs_page);
+#endif
+
__free_page(glob->dummy_read_page);
memset(glob, 0, sizeof(*glob));
out:
@@ -85,6 +89,10 @@ static int ttm_global_init(void)
>> PAGE_SHIFT;
num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT));
+#ifdef CONFIG_DEBUG_FS
+ ttm_debugfs_page = debugfs_create_dir("ttm_page",
ttm_debugfs_root);
+#endif
+
ttm_pool_mgr_init(num_pages);
ttm_tt_mgr_init(num_pages, num_dma32);
@@ -98,8 +106,10 @@ static int ttm_global_init(void)
INIT_LIST_HEAD(>device_list);
atomic_set(>bo_count, 0);
- debugfs_create_atomic_t("buffer_objects", 0444, ttm_debugfs_root,
+#ifdef CONFIG_DEBUG_FS
+ debugfs_create_atomic_t("buffer_objects", 0444, ttm_debugfs_page,
>bo_count);
+#endif
out:
mutex_unlock(_global_mutex);
return ret;
diff --git a/drivers/gpu/drm/ttm/ttm_module.c
b/drivers/gpu/drm/ttm/ttm_module.c
index 88970a6b8e32..66595e6e7087 100644
--- a/drivers/gpu/drm/ttm/ttm_module.c
+++ b/drivers/gpu/drm/ttm/ttm_module.c
@@ -38,6 +38,7 @@
#include "ttm_module.h"
struct dentry *ttm_debugfs_root;
+struct dentry *ttm_debugfs_page;
static int __init ttm_init(void)
{
diff --git a/drivers/gpu/drm/ttm/ttm_module.h
b/drivers/gpu/drm/ttm/ttm_module.h
index d7cac5d4b835..6007dc66f44e 100644
--- a/drivers/gpu/drm/ttm/ttm_module.h
+++ b/drivers/gpu/drm/ttm/ttm_module.h
@@ -36,5 +36,6 @@
struct dentry;
extern struct dentry *ttm_debugfs_root;
+extern struct dentry *ttm_debugfs_page;
#endif /* _TTM_MODULE_H_ */
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
b/drivers/gpu/drm/ttm/ttm_pool.c
index 8be7fd7161fd..ecb33daad7b5 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -709,9 +709,9 @@ int ttm_pool_mgr_init(unsigned long num_pages)
}
#ifdef CONFIG_DEBUG_FS
- debugfs_create_file("page_pool", 0444, ttm_debugfs_root, NULL,
+ debugfs_create_file("page_pool", 0444, ttm_debugfs_page, NULL,
_pool_debugfs_globals_fops);
- debugfs_create_file("page_pool_shrink", 0400, ttm_debugfs_root,
NULL,
+ debugfs_create_file("page_pool_shrink", 0400, ttm_debugfs_page,
NULL,
_pool_debugfs_shrink_fops);
#endif
Re: [PATCH 1/1] drm/amdgpu: release gtt bo after each move test
On 10/13/2021 3:22 AM, zhang wrote:
Hi . Nirmoy
If you let continue to unpin. this will allways test a same va for gtt
I think we should rafresh calculate the value n
Right, I guess then the test should only run till gart size.
Regards,
Nirmoy
On 2021/10/12 20:10, Nirmoy Das wrote:
When gart size is < gtt size this test will fail with
-ENOMEM as we are not freeing gtt bo after each move test.
This is generally not an issue when gart size >= gtt size.
Reported-by: zhang
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
index 909d830b513e..0cf2a560d673 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
@@ -212,7 +212,6 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset 0x%llx\n",
gart_addr - adev->gmc.gart_start);
- continue;
out_lclean_unpin:
amdgpu_bo_unpin(gtt_obj[i]);
@@ -220,6 +219,7 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
amdgpu_bo_unreserve(gtt_obj[i]);
out_lclean_unref:
amdgpu_bo_unref(_obj[i]);
+ continue;
out_lclean:
for (--i; i >= 0; --i) {
amdgpu_bo_unpin(gtt_obj[i]);
RE: bf756fb833cb ("drm/amdgpu: add missing cleanups for Polaris12 UVD/VCE on suspend")
[AMD Official Use Only]
Thanks Boris.
There is another thing which needs your help.
The change of bf756fb833cb was introduced to fix the bug below:
"some hangs observed on suspending when UVD/VCE still using".
So, I need your help to confirm the last two patches(I sent you) do not affect
the fix for the bug above.
Please follow the steps below to verify it:
1. Launch a video playing
2. open another terminal and issue "sudo pm-suspend" command to force suspending
3. verify the system can suspend and resume back correctly without errors or
hangs
BR
Evan
> -Original Message-
> From: Borislav Petkov
> Sent: Tuesday, October 12, 2021 1:09 AM
> To: Quan, Evan
> Cc: Alex Deucher ; amd-gfx list g...@lists.freedesktop.org>; LKML ; Deucher,
> Alexander ; Pan, Xinhui
> ; Chen, Guchun
> Subject: Re: bf756fb833cb ("drm/amdgpu: add missing cleanups for Polaris12
> UVD/VCE on suspend")
>
> On Mon, Oct 11, 2021 at 08:03:51AM +, Quan, Evan wrote:
> > OK... Then forget about previous patches. Let's try to narrow down the
> > issue first. Please try the attached patch1 first. If it works,
>
> It does.
>
> > please undo the changes of patch1 and try patch2 to narrow down further.
>
> It does too.
>
> :-)
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeo
> ple.kernel.org%2Ftglx%2Fnotes-about-
> netiquettedata=04%7C01%7CEvan.Quan%40amd.com%7C7d523770cb
> a84f8d23a108d98cd9c5c0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C
> 0%7C637695689245101874%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000
> sdata=TVYDHI74dYSJl68Z3ZjypBhr6jDYAIUgsMQcoreQrhk%3Dreserve
> d=0
RE: [PATCH] drm/amdkfd: Fix a __user pointer dereference in create_signal_event
[AMD Official Use Only]
>-Original Message-
>From: Lazar, Lijo
>Sent: Wednesday, October 13, 2021 4:07 PM
>To: Yu, Lang ; amd-gfx@lists.freedesktop.org; Kuehling,
>Felix
>Cc: Deucher, Alexander ; Huang, Ray
>
>Subject: Re: [PATCH] drm/amdkfd: Fix a __user pointer dereference in
>create_signal_event
>
>
>
>On 10/13/2021 1:03 PM, Lang Yu wrote:
>> We should not dereference __user pointers directly.
>> https://yarchive.net/comp/linux/user_pointers.html
>>
>> Fixes: 482f07775cf5
>> ("drm/amdkfd: Simplify event ID and signal slot management")
>>
>> Signed-off-by: Lang Yu
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_events.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> index 3eea4edee355..74d3bdcfe341 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> @@ -201,7 +201,7 @@ static int create_signal_event(struct file
>> *devkfd,
>>
>> p->signal_event_count++;
>>
>> -ev->user_signal_address = >signal_page->user_address[ev-
>>event_id];
>
>This is interesting. I thought this wouldn't dereference.
>
>See here -
>
>https://en.cppreference.com/w/c/language/operator_member_access
>
>"If the operand is an array index expression, no action is taken other than the
>array-to-pointer conversion and the addition, so [N] is valid for an array
>of size
>N (obtaining a pointer one past the end is okay, dereferencing it is not, but
>dereference cancels out in this expression)"
Thanks for your clarification about this. I got it.
Regards,
Lang
>Thanks,
>Lijo
>
>
>> +ev->user_signal_address = p->signal_page->user_address +
>> +ev->event_id;
>> pr_debug("Signal event number %zu created with id %d, address %p\n",
>> p->signal_event_count, ev->event_id,
>> ev->user_signal_address);
>>
Re: [PATCH] drm/amdkfd: Fix a __user pointer dereference in create_signal_event
On 10/13/2021 1:03 PM, Lang Yu wrote:
We should not dereference __user pointers directly.
https://yarchive.net/comp/linux/user_pointers.html
Fixes: 482f07775cf5
("drm/amdkfd: Simplify event ID and signal slot management")
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdkfd/kfd_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 3eea4edee355..74d3bdcfe341 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -201,7 +201,7 @@ static int create_signal_event(struct file *devkfd,
p->signal_event_count++;
- ev->user_signal_address = >signal_page->user_address[ev->event_id];
This is interesting. I thought this wouldn't dereference.
See here -
https://en.cppreference.com/w/c/language/operator_member_access
"If the operand is an array index expression, no action is taken other
than the array-to-pointer conversion and the addition, so [N] is valid
for an array of size N (obtaining a pointer one past the end is okay,
dereferencing it is not, but dereference cancels out in this expression)"
Thanks,
Lijo
+ ev->user_signal_address = p->signal_page->user_address + ev->event_id;
pr_debug("Signal event number %zu created with id %d, address %p\n",
p->signal_event_count, ev->event_id,
ev->user_signal_address);
[PATCH] drm/amdkfd: Fix a __user pointer dereference in create_signal_event
We should not dereference __user pointers directly.
https://yarchive.net/comp/linux/user_pointers.html
Fixes: 482f07775cf5
("drm/amdkfd: Simplify event ID and signal slot management")
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdkfd/kfd_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 3eea4edee355..74d3bdcfe341 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -201,7 +201,7 @@ static int create_signal_event(struct file *devkfd,
p->signal_event_count++;
- ev->user_signal_address = >signal_page->user_address[ev->event_id];
+ ev->user_signal_address = p->signal_page->user_address + ev->event_id;
pr_debug("Signal event number %zu created with id %d, address %p\n",
p->signal_event_count, ev->event_id,
ev->user_signal_address);
--
2.25.1
[PATCH] drm/amdkfd: Fix an inappropriate error handling in allloc memory of gpu
We should unreference a gem object instead of an amdgpu bo here.
Fixes: 5ae0283e831a ("drm/amdgpu: Add userptr support for KFD")
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 054c1a224def..cdf46bd0d8d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1503,7 +1503,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
drm_vma_node_revoke(>vma_node, drm_priv);
err_node_allow:
- amdgpu_bo_unref();
+ drm_gem_object_put(gobj);
/* Don't unreserve system mem limit twice */
goto err_reserve_limit;
err_bo_create:
--
2.25.1
RE: [PATCH 1/3] drm/amdgpu/smu11: fix firmware version check for vangogh
[AMD Official Use Only]
Reviewed-by: Evan Quan
> -Original Message-
> From: amd-gfx On Behalf Of Alex
> Deucher
> Sent: Tuesday, October 12, 2021 11:53 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander
> Subject: [PATCH 1/3] drm/amdgpu/smu11: fix firmware version check for
> vangogh
>
> Was missed in the conversion to IP version checking.
>
> Fixes: af3b89d3a639d5 ("drm/amdgpu/smu11.0: convert to IP version
> checking")
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 3470c33ee09d..6d008e9c2f65 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -255,7 +255,7 @@ int smu_v11_0_check_fw_version(struct
> smu_context *smu)
> case IP_VERSION(11, 0, 11):
> smu->smc_driver_if_version =
> SMU11_DRIVER_IF_VERSION_Navy_Flounder;
> break;
> - case CHIP_VANGOGH:
> + case IP_VERSION(11, 5, 0):
> smu->smc_driver_if_version =
> SMU11_DRIVER_IF_VERSION_VANGOGH;
> break;
> case IP_VERSION(11, 0, 12):
> --
> 2.31.1
RE: [PATCH 0/5] 0 MHz is not a valid current frequency
[AMD Official Use Only] I agree with Lijo. Reporting a "round to the smallest operating frequency" just makes user more confusing. As per designed, the frequency marked with "*" should reflect the current clock frequency. BR Evan > -Original Message- > From: amd-gfx On Behalf Of > Lazar, Lijo > Sent: Wednesday, October 13, 2021 12:14 PM > To: Tuikov, Luben ; amd- > g...@lists.freedesktop.org > Cc: Deucher, Alexander > Subject: Re: [PATCH 0/5] 0 MHz is not a valid current frequency > > > > On 10/13/2021 8:40 AM, Luben Tuikov wrote: > > Some ASIC support low-power functionality for the whole ASIC or just > > an IP block. When in such low-power mode, some sysfs interfaces would > > report a frequency of 0, e.g., > > > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > > 0: 500Mhz > > 1: 0Mhz * > > 2: 2200Mhz > > $_ > > > > An operating frequency of 0 MHz doesn't make sense, and this interface > > is designed to report only operating clock frequencies, i.e. non-zero, > > and possibly the current one. > > > > When in this low-power state, round to the smallest operating > > frequency, for this interface, as follows, > > > > Would rather avoid this - > > 1) It is manipulating FW reported value. If at all there is an uncaught issue > in > FW reporting of frequency values, that is masked here. > 2) Otherwise, if 0MHz is described as GFX power gated case, this provides a > convenient interface to check if GFX is power gated. > > If seeing a '0' is not pleasing, consider changing to something like > "NA" - not available (frequency cannot be fetched at the moment). > > Thanks, > Lijo > > > $cat /sys/class/drm/card0/device/pp_dpm_sclk > > 0: 500Mhz * > > 1: 2200Mhz > > $_ > > > > Luben Tuikov (5): > >drm/amd/pm: Slight function rename > >drm/amd/pm: Rename cur_value to curr_value > >drm/amd/pm: Rename freq_values --> freq_value > >dpm/amd/pm: Sienna: 0 MHz is not a current clock frequency > >dpm/amd/pm: Navi10: 0 MHz is not a current clock frequency > > > > .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 60 +-- > > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 73 - > -- > > 2 files changed, 86 insertions(+), 47 deletions(-) > >
[PATCH v2 2/2] drm/amdkfd: fix resume error when iommu disabled in Picasso
When IOMMU disabled in sbios and kfd in iommuv2 path,
IOMMU resume failure blocks system resume. Don't allow kfd to
use iommu v2 when iommu is disabled.
Reported-by: youling
Tested-by: youling
Signed-off-by: Yifan Zhang
---
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 08eedbc6699d..99d2b9c875ea 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -1021,6 +1021,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
kfd_double_confirm_iommu_support(kfd);
if (kfd_iommu_device_init(kfd)) {
+ kfd->use_iommu_v2 = false;
dev_err(kfd_device, "Error initializing iommuv2\n");
goto device_iommu_error;
}
--
2.25.1
[PATCH v2 1/2] drm/amdkfd: fix boot failure when iommu is disabled in Picasso.
When IOMMU disabled in sbios and kfd in iommuv2 path, iommuv2 init will fail. But this failure should not block amdgpu driver init. Reported-by: youling Tested-by: youling Signed-off-by: Yifan Zhang --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 drivers/gpu/drm/amd/amdkfd/kfd_device.c| 3 +++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 52b24334a19e..ef467216ff8a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2397,10 +2397,6 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) if (!adev->gmc.xgmi.pending_reset) amdgpu_amdkfd_device_init(adev); - r = amdgpu_amdkfd_resume_iommu(adev); - if (r) - goto init_failed; - amdgpu_fru_get_product_info(adev); init_failed: diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c index 064d42acd54e..08eedbc6699d 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c @@ -1029,6 +1029,9 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd, svm_migrate_init((struct amdgpu_device *)kfd->kgd); + if(kgd2kfd_resume_iommu(kfd)) + goto device_iommu_error; + if (kfd_resume(kfd)) goto kfd_resume_error; -- 2.25.1
