Re: [PATCH v16 14/16] tee, arm64: untag user pointers in tee_shm_register
On Mon, Jun 3, 2019 at 6:56 PM Andrey Konovalov wrote: > > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided > user pointers for vma lookups (via __check_mem_type()), which can only by > done with untagged pointers. > > Untag user pointers in this function. > > Signed-off-by: Andrey Konovalov Acked-by: Jens Wiklander > --- > drivers/tee/tee_shm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > index 49fd7312e2aa..96945f4cefb8 100644 > --- a/drivers/tee/tee_shm.c > +++ b/drivers/tee/tee_shm.c > @@ -263,6 +263,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, > unsigned long addr, > shm->teedev = teedev; > shm->ctx = ctx; > shm->id = -1; > + addr = untagged_addr(addr); > start = rounddown(addr, PAGE_SIZE); > shm->offset = addr - start; > shm->size = length; > -- > 2.22.0.rc1.311.g5d7573a151-goog >
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
...
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 8e7403f081f44a..547002f56a163d 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
...
> @@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
> mm->hmm = NULL;
> spin_unlock(>page_table_lock);
>
> - kfree(hmm);
> + mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
It occurred to me to wonder if it is best to use the MMU notifier's
instance of srcu, instead of creating a separate instance for HMM.
But this really does seem appropriate, since we are after all using
this to synchronize with MMU notifier callbacks. So, fine.
> }
>
> static inline void hmm_put(struct hmm *hmm)
> @@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
>
> static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> {
> - struct hmm *hmm = mm_get_hmm(mm);
> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> struct hmm_range *range;
>
> + /* hmm is in progress to free */
Well, sometimes, yes. :)
Maybe this wording is clearer (if we need any comment at all):
/* Bail out if hmm is in the process of being freed */
> + if (!kref_get_unless_zero(>kref))
> + return;
> +
> /* Report this HMM as dying. */
> hmm->dead = true;
>
> @@ -194,13 +203,15 @@ static void hmm_release(struct mmu_notifier *mn, struct
> mm_struct *mm)
> static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> const struct mmu_notifier_range *nrange)
> {
> - struct hmm *hmm = mm_get_hmm(nrange->mm);
> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> struct hmm_update update;
> struct hmm_range *range;
> int ret = 0;
>
> - VM_BUG_ON(!hmm);
> + /* hmm is in progress to free */
Same here.
> + if (!kref_get_unless_zero(>kref))
> + return 0;
>
> update.start = nrange->start;
> update.end = nrange->end;
> @@ -245,9 +256,11 @@ static int hmm_invalidate_range_start(struct
> mmu_notifier *mn,
> static void hmm_invalidate_range_end(struct mmu_notifier *mn,
> const struct mmu_notifier_range *nrange)
> {
> - struct hmm *hmm = mm_get_hmm(nrange->mm);
> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
>
> - VM_BUG_ON(!hmm);
> + /* hmm is in progress to free */
And here.
> + if (!kref_get_unless_zero(>kref))
> + return;
>
> mutex_lock(>lock);
> hmm->notifiers--;
>
Elegant fix. Regardless of the above chatter I added, you can add:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> So long a a struct hmm pointer exists, so should the struct mm it is
> linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
> once the hmm refcount goes to zero.
>
> Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
> mm->hmm delete the hmm_hmm_destroy().
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
> ---
> v2:
> - Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
> ---
> include/linux/hmm.h | 3 ---
> kernel/fork.c | 1 -
> mm/hmm.c| 22 --
> 3 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 2d519797cb134a..4ee3acabe5ed22 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror
> *mirror,
> }
>
> /* Below are for HMM internal use only! Not to be used by device driver! */
> -void hmm_mm_destroy(struct mm_struct *mm);
> -
> static inline void hmm_mm_init(struct mm_struct *mm)
> {
> mm->hmm = NULL;
> }
> #else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> -static inline void hmm_mm_destroy(struct mm_struct *mm) {}
> static inline void hmm_mm_init(struct mm_struct *mm) {}
> #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b2b87d450b80b5..588c768ae72451 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
> WARN_ON_ONCE(mm == current->active_mm);
> mm_free_pgd(mm);
> destroy_context(mm);
> - hmm_mm_destroy(mm);
This is particularly welcome, not to have an "HMM is special" case
in such a core part of process/mm code.
> mmu_notifier_mm_destroy(mm);
> check_mm(mm);
> put_user_ns(mm->user_ns);
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 8796447299023c..cc7c26fda3300e 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -82,6 +83,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> hmm->notifiers = 0;
> hmm->dead = false;
> hmm->mm = mm;
> + mmgrab(hmm->mm);
>
> spin_lock(>page_table_lock);
> if (!mm->hmm)
> @@ -109,6 +111,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> mm->hmm = NULL;
> spin_unlock(>page_table_lock);
> error:
> + mmdrop(hmm->mm);
> kfree(hmm);
> return NULL;
> }
> @@ -130,6 +133,7 @@ static void hmm_free(struct kref *kref)
> mm->hmm = NULL;
> spin_unlock(>page_table_lock);
>
> + mmdrop(hmm->mm);
> mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
> }
>
> @@ -138,24 +142,6 @@ static inline void hmm_put(struct hmm *hmm)
> kref_put(>kref, hmm_free);
> }
>
> -void hmm_mm_destroy(struct mm_struct *mm)
> -{
> - struct hmm *hmm;
> -
> - spin_lock(>page_table_lock);
> - hmm = mm_get_hmm(mm);
> - mm->hmm = NULL;
> - if (hmm) {
> - hmm->mm = NULL;
> - hmm->dead = true;
> - spin_unlock(>page_table_lock);
> - hmm_put(hmm);
> - return;
> - }
> -
> - spin_unlock(>page_table_lock);
> -}
> -
> static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
>
Failed to find any problems with this. :)
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> Ralph observes that hmm_range_register() can only be called by a driver
> while a mirror is registered. Make this clear in the API by passing in the
> mirror structure as a parameter.
>
> This also simplifies understanding the lifetime model for struct hmm, as
> the hmm pointer must be valid as part of a registered mirror so all we
> need in hmm_register_range() is a simple kref_get.
>
> Suggested-by: Ralph Campbell
> Signed-off-by: Jason Gunthorpe
> ---
> v2
> - Include the oneline patch to nouveau_svm.c
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
> include/linux/hmm.h | 7 ---
> mm/hmm.c | 15 ++-
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 93ed43c413f0bb..8c92374afcf227 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
> range.values = nouveau_svm_pfn_values;
> range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
> again:
> - ret = hmm_vma_fault(, true);
> + ret = hmm_vma_fault(>mirror, , true);
> if (ret == 0) {
> mutex_lock(>mutex);
> if (!hmm_vma_range_done()) {
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 688c5ca7068795..2d519797cb134a 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct
> hmm_mirror *mirror)
> * Please see Documentation/vm/hmm.rst for how to use the range API.
> */
> int hmm_range_register(struct hmm_range *range,
> -struct mm_struct *mm,
> +struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift);
> @@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct hmm_range
> *range)
> }
>
> /* This is a temporary helper to avoid merge conflict between trees. */
> -static inline int hmm_vma_fault(struct hmm_range *range, bool block)
> +static inline int hmm_vma_fault(struct hmm_mirror *mirror,
> + struct hmm_range *range, bool block)
> {
> long ret;
>
> @@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range *range,
> bool block)
> range->default_flags = 0;
> range->pfn_flags_mask = -1UL;
>
> - ret = hmm_range_register(range, range->vma->vm_mm,
> + ret = hmm_range_register(range, mirror,
>range->start, range->end,
>PAGE_SHIFT);
> if (ret)
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 547002f56a163d..8796447299023c 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
> * Track updates to the CPU page table see include/linux/hmm.h
> */
> int hmm_range_register(struct hmm_range *range,
> -struct mm_struct *mm,
> +struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift)
> {
> unsigned long mask = ((1UL << page_shift) - 1UL);
> - struct hmm *hmm;
> + struct hmm *hmm = mirror->hmm;
>
> range->valid = false;
> range->hmm = NULL;
> @@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
> range->start = start;
> range->end = end;
>
> - hmm = hmm_get_or_create(mm);
> - if (!hmm)
> - return -EFAULT;
> -
> /* Check if hmm_mm_destroy() was call. */
> - if (hmm->mm == NULL || hmm->dead) {
> - hmm_put(hmm);
> + if (hmm->mm == NULL || hmm->dead)
> return -EFAULT;
> - }
> +
> + range->hmm = hmm;
> + kref_get(>kref);
>
> /* Initialize range to track CPU page table updates. */
> mutex_lock(>lock);
>
I'm not a qualified Nouveau reviewer, but this looks obviously correct,
so:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdkfd: Initialize dqm earlier
On 2019-06-06 5:51 p.m., Zeng, Oak wrote:
> dqm is referenced in function kfd_toplogy_add_device.
> Move dqm initialization up to avoid NULL pointer reference.
This addresses a pretty unlikely race condition where someone looks at
/sys/kernel/debug/kfd/hqds during the device initialization.
We add devices do the topology before their initialization is
successfully completed. If it fails, we remove the device again. Having
devices in the topology that are not completely initialized yet seems to
be the real issue. A cleaner solution would move
kfd_topoglogy_add_device to the end of kgd2kfd_device_init, so that we
only add a device to the topology after they are successfully and
completely initialized. Not sure if there are any dependencies in the
init sequence that would be broken by this, though.
Regards,
Felix
>
> Change-Id: Id6cb2541af129826b7621ceaa8e06e638c7bb122
> Signed-off-by: Oak Zeng
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 9d1b026..e7e24fe 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -603,6 +603,12 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> if (kfd->kfd2kgd->get_hive_id)
> kfd->hive_id = kfd->kfd2kgd->get_hive_id(kfd->kgd);
>
> + kfd->dqm = device_queue_manager_init(kfd);
> + if (!kfd->dqm) {
> + dev_err(kfd_device, "Error initializing queue manager\n");
> + goto device_queue_manager_error;
> + }
> +
> if (kfd_topology_add_device(kfd)) {
> dev_err(kfd_device, "Error adding device to topology\n");
> goto kfd_topology_add_device_error;
> @@ -613,12 +619,6 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> goto kfd_interrupt_error;
> }
>
> - kfd->dqm = device_queue_manager_init(kfd);
> - if (!kfd->dqm) {
> - dev_err(kfd_device, "Error initializing queue manager\n");
> - goto device_queue_manager_error;
> - }
> -
> if (kfd_iommu_device_init(kfd)) {
> dev_err(kfd_device, "Error initializing iommuv2\n");
> goto device_iommu_error;
> @@ -642,12 +642,12 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>
> kfd_resume_error:
> device_iommu_error:
> - device_queue_manager_uninit(kfd->dqm);
> -device_queue_manager_error:
> kfd_interrupt_exit(kfd);
> kfd_interrupt_error:
> kfd_topology_remove_device(kfd);
> kfd_topology_add_device_error:
> + device_queue_manager_uninit(kfd->dqm);
> +device_queue_manager_error:
> kfd_doorbell_fini(kfd);
> kfd_doorbell_error:
> kfd_gtt_sa_fini(kfd);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 4/6] drm/amdkfd: Separate mqd allocation and initialization
On 2019-06-06 5:40 p.m., Zeng, Oak wrote:
> Introduce a new mqd allocation interface and split the original
> init_mqd function into two functions: allocate_mqd and init_mqd.
> Also renamed uninit_mqd to free_mqd. This is preparation work to
> fix a circular lock dependency.
>
> Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
> Signed-off-by: Oak Zeng
Reviewed-by: Felix Kuehling
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 40 +-
> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 16 ++--
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c | 4 +-
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 18 +++--
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 78 +++
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 78 +++
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c| 88
> --
> 7 files changed, 129 insertions(+), 193 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 3c042eb..e5b3fb9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -319,11 +319,13 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> if (retval)
> goto out_deallocate_hqd;
>
> - retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
> - >gart_mqd_addr, >properties);
> - if (retval)
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
> + if (!q->mqd_mem_obj) {
> + retval = -ENOMEM;
> goto out_deallocate_doorbell;
> -
> + }
> + mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
> + >gart_mqd_addr, >properties);
> if (q->properties.is_active) {
>
> if (WARN(q->process->mm != current->mm,
> @@ -333,7 +335,7 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
> q->queue, >properties, current->mm);
> if (retval)
> - goto out_uninit_mqd;
> + goto out_free_mqd;
> }
>
> list_add(>list, >queues_list);
> @@ -355,8 +357,8 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> dqm->total_queue_count);
> goto out_unlock;
>
> -out_uninit_mqd:
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> +out_free_mqd:
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> out_deallocate_doorbell:
> deallocate_doorbell(qpd, q);
> out_deallocate_hqd:
> @@ -450,7 +452,7 @@ static int destroy_queue_nocpsch_locked(struct
> device_queue_manager *dqm,
> if (retval == -ETIME)
> qpd->reset_wavefronts = true;
>
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>
> list_del(>list);
> if (list_empty(>queues_list)) {
> @@ -489,7 +491,7 @@ static int destroy_queue_nocpsch(struct
> device_queue_manager *dqm,
>
> static int update_queue(struct device_queue_manager *dqm, struct queue *q)
> {
> - int retval;
> + int retval = 0;
> struct mqd_manager *mqd_mgr;
> struct kfd_process_device *pdd;
> bool prev_active = false;
> @@ -527,7 +529,7 @@ static int update_queue(struct device_queue_manager *dqm,
> struct queue *q)
> }
> }
>
> - retval = mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
> + mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
>
> /*
>* check active state vs. the previous state and modify
> @@ -1160,11 +1162,13 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
> dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
> q->properties.tba_addr = qpd->tba_addr;
> q->properties.tma_addr = qpd->tma_addr;
> - retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
> - >gart_mqd_addr, >properties);
> - if (retval)
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
> + if (!q->mqd_mem_obj) {
> + retval = -ENOMEM;
> goto out_deallocate_doorbell;
> -
> + }
> + mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
> + >gart_mqd_addr, >properties);
> dqm_lock(dqm);
>
> list_add(>list, >queues_list);
> @@ -1373,8 +1377,8 @@ static int destroy_queue_cpsch(struct
> device_queue_manager *dqm,
>
> dqm_unlock(dqm);
>
> - /* Do uninit_mqd after dqm_unlock(dqm) to avoid circular locking */
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
> +
[PATCH] drm/amdkfd: Initialize dqm earlier
dqm is referenced in function kfd_toplogy_add_device.
Move dqm initialization up to avoid NULL pointer reference.
Change-Id: Id6cb2541af129826b7621ceaa8e06e638c7bb122
Signed-off-by: Oak Zeng
---
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 9d1b026..e7e24fe 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -603,6 +603,12 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
if (kfd->kfd2kgd->get_hive_id)
kfd->hive_id = kfd->kfd2kgd->get_hive_id(kfd->kgd);
+ kfd->dqm = device_queue_manager_init(kfd);
+ if (!kfd->dqm) {
+ dev_err(kfd_device, "Error initializing queue manager\n");
+ goto device_queue_manager_error;
+ }
+
if (kfd_topology_add_device(kfd)) {
dev_err(kfd_device, "Error adding device to topology\n");
goto kfd_topology_add_device_error;
@@ -613,12 +619,6 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
goto kfd_interrupt_error;
}
- kfd->dqm = device_queue_manager_init(kfd);
- if (!kfd->dqm) {
- dev_err(kfd_device, "Error initializing queue manager\n");
- goto device_queue_manager_error;
- }
-
if (kfd_iommu_device_init(kfd)) {
dev_err(kfd_device, "Error initializing iommuv2\n");
goto device_iommu_error;
@@ -642,12 +642,12 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
kfd_resume_error:
device_iommu_error:
- device_queue_manager_uninit(kfd->dqm);
-device_queue_manager_error:
kfd_interrupt_exit(kfd);
kfd_interrupt_error:
kfd_topology_remove_device(kfd);
kfd_topology_add_device_error:
+ device_queue_manager_uninit(kfd->dqm);
+device_queue_manager_error:
kfd_doorbell_fini(kfd);
kfd_doorbell_error:
kfd_gtt_sa_fini(kfd);
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/6] drm/amdkfd: Separate mqd allocation and initialization
Introduce a new mqd allocation interface and split the original
init_mqd function into two functions: allocate_mqd and init_mqd.
Also renamed uninit_mqd to free_mqd. This is preparation work to
fix a circular lock dependency.
Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
Signed-off-by: Oak Zeng
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 40 +-
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 16 ++--
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c | 4 +-
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 18 +++--
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 78 +++
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 78 +++
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c| 88 --
7 files changed, 129 insertions(+), 193 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 3c042eb..e5b3fb9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -319,11 +319,13 @@ static int create_queue_nocpsch(struct
device_queue_manager *dqm,
if (retval)
goto out_deallocate_hqd;
- retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
- >gart_mqd_addr, >properties);
- if (retval)
+ q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
+ if (!q->mqd_mem_obj) {
+ retval = -ENOMEM;
goto out_deallocate_doorbell;
-
+ }
+ mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
+ >gart_mqd_addr, >properties);
if (q->properties.is_active) {
if (WARN(q->process->mm != current->mm,
@@ -333,7 +335,7 @@ static int create_queue_nocpsch(struct device_queue_manager
*dqm,
retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
q->queue, >properties, current->mm);
if (retval)
- goto out_uninit_mqd;
+ goto out_free_mqd;
}
list_add(>list, >queues_list);
@@ -355,8 +357,8 @@ static int create_queue_nocpsch(struct device_queue_manager
*dqm,
dqm->total_queue_count);
goto out_unlock;
-out_uninit_mqd:
- mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+out_free_mqd:
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
out_deallocate_doorbell:
deallocate_doorbell(qpd, q);
out_deallocate_hqd:
@@ -450,7 +452,7 @@ static int destroy_queue_nocpsch_locked(struct
device_queue_manager *dqm,
if (retval == -ETIME)
qpd->reset_wavefronts = true;
- mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
list_del(>list);
if (list_empty(>queues_list)) {
@@ -489,7 +491,7 @@ static int destroy_queue_nocpsch(struct
device_queue_manager *dqm,
static int update_queue(struct device_queue_manager *dqm, struct queue *q)
{
- int retval;
+ int retval = 0;
struct mqd_manager *mqd_mgr;
struct kfd_process_device *pdd;
bool prev_active = false;
@@ -527,7 +529,7 @@ static int update_queue(struct device_queue_manager *dqm,
struct queue *q)
}
}
- retval = mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
+ mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
/*
* check active state vs. the previous state and modify
@@ -1160,11 +1162,13 @@ static int create_queue_cpsch(struct
device_queue_manager *dqm, struct queue *q,
dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
q->properties.tba_addr = qpd->tba_addr;
q->properties.tma_addr = qpd->tma_addr;
- retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
- >gart_mqd_addr, >properties);
- if (retval)
+ q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
+ if (!q->mqd_mem_obj) {
+ retval = -ENOMEM;
goto out_deallocate_doorbell;
-
+ }
+ mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
+ >gart_mqd_addr, >properties);
dqm_lock(dqm);
list_add(>list, >queues_list);
@@ -1373,8 +1377,8 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
dqm_unlock(dqm);
- /* Do uninit_mqd after dqm_unlock(dqm) to avoid circular locking */
- mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+ /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
return retval;
@@ -1615,14 +1619,14 @@ static int process_termination_cpsch(struct
device_queue_manager *dqm,
Re: [PATCH 2/7] drm/dp_mst: Register AUX devices for MST ports
On Thu, 2019-06-06 at 19:41 +, Li, Sun peng (Leo) wrote: > > On 2019-06-03 3:28 p.m., Lyude Paul wrote: > > > I'm reproducing this just by reloading i915 on a machine with some MST > > > displays connected. I uploaded a copy of the script that I use to do > > > this > > > here: > > > > > > https://people.freedesktop.org/~lyudess/archive/06-03-2019/unloadgpumod.sh > > oops-almost forgot to mention. The argument you pass to make it reload > > instead > > of just unloading is --reload > > > > Thanks for the script! > > So, the warning has to do with: > > 1. Having the aux device as a child of connector device, and > 2. During driver unload, drm_connector_unregister() is called before > drm_dp_mst_topology_put_port() > > Which means that connector_unregister() will recursively remove the > child aux device, before put_port() can properly unregister it. Any > further attempts to remove after the first will throw a "not found" warning. > > Call-stacks for reference: > >*drm_connector_unregister*+0x37/0x60 [drm] >drm_connector_unregister_all+0x30/0x60 [drm] >drm_modeset_unregister_all+0xe/0x30 [drm] >drm_dev_unregister+0x9c/0xb0 [drm] >i915_driver_unload+0x73/0x120 [i915] > >drm_dp_aux_unregister_devnode+0xf5/0x180 [drm_kms_helper] >*drm_dp_mst_topology_put_port*+0x4e/0xf0 [drm_kms_helper] >drm_dp_mst_topology_put_mstb+0x91/0x160 [drm_kms_helper] >drm_dp_mst_topology_mgr_set_mst+0x12b/0x2b0 [drm_kms_helper] >? __finish_swait+0x10/0x40 >drm_dp_mst_topology_mgr_destroy+0x11/0xa0 [drm_kms_helper] >intel_dp_encoder_flush_work+0x32/0xb0 [i915] >intel_ddi_encoder_destroy+0x32/0x60 [i915] >drm_mode_config_cleanup+0x51/0x2e0 [drm] >intel_modeset_cleanup+0xc8/0x140 [i915] >i915_driver_unload+0xa0/0x120 [i915] > > A solution is to unregister the aux device immediately before the > connector device is unregistered - if we are to keep the aux device as a > child. Following current scheme with SST, it looks like > drm_connector_funcs->early_unregister() is the right place to do so. > To keep the balance, aux registration will then happen in > drm_connector_funcs->late_register(). This will leave the SDP > transaction handling part in DRM still, but pass the responsibility of > creating and removing remote (fake) aux devices to the driver. > > I have a WIP patch here for you to take a look. It should apply on top > of the existing patchset: > https://pastebin.com/1YJZhL4C > This approach seems fine to me! I was thinking we might end up doing something like this. My only thought is that we can probably write up a drm_dp_mst_connector_late_register() and drm_dp_mst_connector_early_unregister() that drivers can call from their →late_register()/→early_unregister() callbacks that handles printing the debug message, setting the parent and registering/unregistering the aux device. I'd imagine someday in the future we'll probably have more things to add to those callbacks for mst as well. > I'd like to hear your thoughts, before I go and modify other drivers :) > > Thanks, > Leo -- Cheers, Lyude Paul ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 24/24] drm/amd/display: set link->dongle_max_pix_clk to 0 on a disconnect
From: Samson Tam
[Why]
Found issue in EDID Emulation where if we connect a display using
a passive HDMI-DP dongle, disconnect it and then try to emulate
a display using DP, we could not see 4K modes. This was because
on a disconnect, dongle_max_pix_clk was still set so when we
emulate using DP, in dc_link_validate_mode_timing(), it would
think we were still using a dongle and limit the modes we support.
[How]
In dc_link_detect(), set dongle_max_pix_clk to 0 when we detect
a hotplug out ( if new_connection_type = dc_connection_none ).
Signed-off-by: Samson Tam
Reviewed-by: Jun Lei
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 10807fa46ad6..202e092f8ecf 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -984,6 +984,12 @@ bool dc_link_detect(struct dc_link *link, enum
dc_detect_reason reason)
link->type = dc_connection_none;
sink_caps.signal = SIGNAL_TYPE_NONE;
+ /* When we unplug a passive DP-HDMI dongle connection,
dongle_max_pix_clk
+* is not cleared. If we emulate a DP signal on this
connection, it thinks
+* the dongle is still there and limits the number of modes we
can emulate.
+* Clear dongle_max_pix_clk on disconnect to fix this
+*/
+ link->dongle_max_pix_clk = 0;
}
LINK_INFO("link=%d, dc_sink_in=%p is now %s prev_sink=%p dpcd same=%d
edid same=%d\n",
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 23/24] drm/amd/display: make clk_mgr call enable_pme_wa
From: Su Sung Chung
refactor a code so we will call clk_mgr's enable_pme_wa function so we
can use pme_wa for future asics. This way we don't need to worry about
different ASIC since clk_mgr already have that information
Signed-off-by: Su Sung Chung
Reviewed-by: Eric Yang
Acked-by: Bhawanpreet Lakha
---
.../display/dc/clk_mgr/dcn10/rv1_clk_mgr.c| 14 +++
.../display/dc/dce110/dce110_hw_sequencer.c | 25 +--
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
index 31db9b55e11a..183ca39ce5a1 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr.c
@@ -215,9 +215,23 @@ static void rv1_update_clocks(struct clk_mgr *clk_mgr_base,
}
}
+static void rv1_enable_pme_wa(struct clk_mgr *clk_mgr_base)
+{
+ struct clk_mgr_internal *clk_mgr = TO_CLK_MGR_INTERNAL(clk_mgr_base);
+ struct pp_smu_funcs_rv *pp_smu = NULL;
+
+ if (clk_mgr->pp_smu) {
+ pp_smu = _mgr->pp_smu->rv_funcs;
+
+ if (pp_smu->set_pme_wa_enable)
+ pp_smu->set_pme_wa_enable(_smu->pp_smu);
+ }
+}
+
static struct clk_mgr_funcs rv1_clk_funcs = {
.get_dp_ref_clk_frequency = dce12_get_dp_ref_freq_khz,
.update_clocks = rv1_update_clocks,
+ .enable_pme_wa = rv1_enable_pme_wa,
};
static struct clk_mgr_internal_funcs rv1_clk_internal_funcs = {
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 2a7ac452d458..5a831410bc55 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -939,26 +939,12 @@ void hwss_edp_backlight_control(
edp_receiver_ready_T9(link);
}
-// Static helper function which calls the correct function
-// based on pp_smu version
-static void set_pme_wa_enable_by_version(struct dc *dc)
-{
- struct pp_smu_funcs *pp_smu = NULL;
-
- if (dc->res_pool->pp_smu)
- pp_smu = dc->res_pool->pp_smu;
-
- if (pp_smu) {
- if (pp_smu->ctx.ver == PP_SMU_VER_RV &&
pp_smu->rv_funcs.set_pme_wa_enable)
- pp_smu->rv_funcs.set_pme_wa_enable(&(pp_smu->ctx));
- }
-}
-
void dce110_enable_audio_stream(struct pipe_ctx *pipe_ctx)
{
/* notify audio driver for audio modes of monitor */
struct dc *core_dc = pipe_ctx->stream->ctx->dc;
struct pp_smu_funcs *pp_smu = NULL;
+ struct clk_mgr *clk_mgr = core_dc->clk_mgr;
unsigned int i, num_audio = 1;
if (pipe_ctx->stream_res.audio && pipe_ctx->stream_res.audio->enabled
== true)
@@ -976,9 +962,9 @@ void dce110_enable_audio_stream(struct pipe_ctx *pipe_ctx)
pipe_ctx->stream_res.audio->funcs->az_enable(pipe_ctx->stream_res.audio);
- if (num_audio >= 1 && pp_smu != NULL)
+ if (num_audio >= 1 && clk_mgr->funcs->enable_pme_wa)
/*this is the first audio. apply the PME w/a in order
to wake AZ from D3*/
- set_pme_wa_enable_by_version(core_dc);
+ clk_mgr->funcs->enable_pme_wa(clk_mgr);
/* un-mute audio */
/* TODO: audio should be per stream rather than per link */
pipe_ctx->stream_res.stream_enc->funcs->audio_mute_control(
@@ -992,6 +978,7 @@ void dce110_disable_audio_stream(struct pipe_ctx *pipe_ctx,
int option)
{
struct dc *dc = pipe_ctx->stream->ctx->dc;
struct pp_smu_funcs *pp_smu = NULL;
+ struct clk_mgr *clk_mgr = dc->clk_mgr;
if (pipe_ctx->stream_res.audio && pipe_ctx->stream_res.audio->enabled
== false)
return;
@@ -1020,9 +1007,9 @@ void dce110_disable_audio_stream(struct pipe_ctx
*pipe_ctx, int option)
update_audio_usage(>current_state->res_ctx,
dc->res_pool, pipe_ctx->stream_res.audio, false);
pipe_ctx->stream_res.audio = NULL;
}
- if (pp_smu != NULL)
+ if (clk_mgr->funcs->enable_pme_wa)
/*this is the first audio. apply the PME w/a in order
to wake AZ from D3*/
- set_pme_wa_enable_by_version(dc);
+ clk_mgr->funcs->enable_pme_wa(clk_mgr);
/* TODO: notify audio driver for if audio modes list changed
* add audio mode list change flag */
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 22/24] drm/amd/display: Do not grant POST_LT_ADJ when TPS4 is used
From: abdoulaye berthe
[Description]
The spec does not allow POST_LT_ADJ_GRANTED to be set when TPS4 is used.
Signed-off-by: abdoulaye berthe
Acked-by: Bhawanpreet Lakha
---
.../gpu/drm/amd/display/dc/core/dc_link_dp.c | 56 ++-
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 2d519e5fc3ea..a1187274dbed 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -89,6 +89,29 @@ static void dpcd_set_training_pattern(
dpcd_pattern.v1_4.TRAINING_PATTERN_SET);
}
+static enum hw_dp_training_pattern get_supported_tp(struct dc_link *link)
+{
+ enum hw_dp_training_pattern highest_tp = HW_DP_TRAINING_PATTERN_2;
+ struct encoder_feature_support *features = >link_enc->features;
+ struct dpcd_caps *dpcd_caps = >dpcd_caps;
+
+ if (features->flags.bits.IS_TPS3_CAPABLE)
+ highest_tp = HW_DP_TRAINING_PATTERN_3;
+
+ if (features->flags.bits.IS_TPS4_CAPABLE)
+ highest_tp = HW_DP_TRAINING_PATTERN_4;
+
+ if (dpcd_caps->max_down_spread.bits.TPS4_SUPPORTED &&
+ highest_tp >= HW_DP_TRAINING_PATTERN_4)
+ return HW_DP_TRAINING_PATTERN_4;
+
+ if (dpcd_caps->max_ln_count.bits.TPS3_SUPPORTED &&
+ highest_tp >= HW_DP_TRAINING_PATTERN_3)
+ return HW_DP_TRAINING_PATTERN_3;
+
+ return HW_DP_TRAINING_PATTERN_2;
+}
+
static void dpcd_set_link_settings(
struct dc_link *link,
const struct link_training_settings *lt_settings)
@@ -97,6 +120,7 @@ static void dpcd_set_link_settings(
union down_spread_ctrl downspread = { {0} };
union lane_count_set lane_count_set = { {0} };
+ enum hw_dp_training_pattern hw_tr_pattern;
downspread.raw = (uint8_t)
(lt_settings->link_settings.link_spread);
@@ -106,8 +130,13 @@ static void dpcd_set_link_settings(
lane_count_set.bits.ENHANCED_FRAMING = 1;
- lane_count_set.bits.POST_LT_ADJ_REQ_GRANTED =
- link->dpcd_caps.max_ln_count.bits.POST_LT_ADJ_REQ_SUPPORTED;
+ lane_count_set.bits.POST_LT_ADJ_REQ_GRANTED = 0;
+
+ hw_tr_pattern = get_supported_tp(link);
+ if (hw_tr_pattern != HW_DP_TRAINING_PATTERN_4) {
+ lane_count_set.bits.POST_LT_ADJ_REQ_GRANTED =
+
link->dpcd_caps.max_ln_count.bits.POST_LT_ADJ_REQ_SUPPORTED;
+ }
core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
, sizeof(downspread));
@@ -698,29 +727,6 @@ static bool perform_post_lt_adj_req_sequence(
}
-static enum hw_dp_training_pattern get_supported_tp(struct dc_link *link)
-{
- enum hw_dp_training_pattern highest_tp = HW_DP_TRAINING_PATTERN_2;
- struct encoder_feature_support *features = >link_enc->features;
- struct dpcd_caps *dpcd_caps = >dpcd_caps;
-
- if (features->flags.bits.IS_TPS3_CAPABLE)
- highest_tp = HW_DP_TRAINING_PATTERN_3;
-
- if (features->flags.bits.IS_TPS4_CAPABLE)
- highest_tp = HW_DP_TRAINING_PATTERN_4;
-
- if (dpcd_caps->max_down_spread.bits.TPS4_SUPPORTED &&
- highest_tp >= HW_DP_TRAINING_PATTERN_4)
- return HW_DP_TRAINING_PATTERN_4;
-
- if (dpcd_caps->max_ln_count.bits.TPS3_SUPPORTED &&
- highest_tp >= HW_DP_TRAINING_PATTERN_3)
- return HW_DP_TRAINING_PATTERN_3;
-
- return HW_DP_TRAINING_PATTERN_2;
-}
-
static enum link_training_result get_cr_failure(enum dc_lane_count ln_count,
union lane_status *dpcd_lane_status)
{
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 11/24] drm/amd/display: fix issue with eDP not detected on driver load
From: Anthony Koo
[Why]
HPD not going to be high if Panel VDD is off
And all AUX transaction will fail :(
[How]
1. Power on VDD before attempting detection if it isn't already on
2. Improve the robustness by having a retry mechanism on the
first DPCD read after VDD on. If a particular board always holds
HPD high incorrectly, the AUX access may fail, so we can retry
in those scenarios. This change would only improve logic
since it prevents AUX failure leading to bad resolution on internal
panel.
3. We should never need to re-detect internal panel, so logic
is re-arranged a bit to skip earlier.
Signed-off-by: Anthony Koo
Reviewed-by: Jun Lei
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 42 ++-
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index c9e0b126777b..c5dc809f17d6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -217,8 +217,11 @@ bool dc_link_detect_sink(struct dc_link *link, enum
dc_connection_type *type)
return true;
}
- if (link->connector_signal == SIGNAL_TYPE_EDP)
+ if (link->connector_signal == SIGNAL_TYPE_EDP) {
+ /*in case it is not on*/
+ link->dc->hwss.edp_power_control(link, true);
link->dc->hwss.edp_wait_for_hpd_ready(link, true);
+ }
/* todo: may need to lock gpio access */
hpd_pin = get_hpd_gpio(link->ctx->dc_bios, link->link_id,
link->ctx->gpio_service);
@@ -520,11 +523,31 @@ static void
read_edp_current_link_settings_on_detect(struct dc_link *link)
union lane_count_set lane_count_set = { {0} };
uint8_t link_bw_set;
uint8_t link_rate_set;
+ uint32_t read_dpcd_retry_cnt = 10;
+ enum dc_status status = DC_ERROR_UNEXPECTED;
+ int i;
// Read DPCD 00101h to find out the number of lanes currently set
- core_link_read_dpcd(link, DP_LANE_COUNT_SET,
- _count_set.raw, sizeof(lane_count_set));
- link->cur_link_settings.lane_count = lane_count_set.bits.LANE_COUNT_SET;
+ for (i = 0; i < read_dpcd_retry_cnt; i++) {
+ status = core_link_read_dpcd(
+ link,
+ DP_LANE_COUNT_SET,
+ _count_set.raw,
+ sizeof(lane_count_set));
+ /* First DPCD read after VDD ON can fail if the particular board
+* does not have HPD pin wired correctly. So if DPCD read fails,
+* which it should never happen, retry a few times. Target worst
+* case scenario of 80 ms.
+*/
+ if (status == DC_OK) {
+ link->cur_link_settings.lane_count =
lane_count_set.bits.LANE_COUNT_SET;
+ break;
+ }
+
+ udelay(8000);
+ }
+
+ ASSERT(status == DC_OK);
// Read DPCD 00100h to find if standard link rates are set
core_link_read_dpcd(link, DP_LINK_BW_SET,
@@ -678,6 +701,11 @@ bool dc_link_detect(struct dc_link *link, enum
dc_detect_reason reason)
if (dc_is_virtual_signal(link->connector_signal))
return false;
+ if ((link->connector_signal == SIGNAL_TYPE_LVDS ||
+ link->connector_signal == SIGNAL_TYPE_EDP) &&
+ link->local_sink)
+ return true;
+
if (false == dc_link_detect_sink(link, _connection_type)) {
BREAK_TO_DEBUGGER();
return false;
@@ -688,14 +716,8 @@ bool dc_link_detect(struct dc_link *link, enum
dc_detect_reason reason)
* up to date, especially if link was powered on by GOP.
*/
read_edp_current_link_settings_on_detect(link);
- if (link->local_sink)
- return true;
}
- if (link->connector_signal == SIGNAL_TYPE_LVDS &&
- link->local_sink)
- return true;
-
prev_sink = link->local_sink;
if (prev_sink != NULL) {
dc_sink_retain(prev_sink);
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 10/24] drm/amd/display: Rework CRTC color management
From: Nicholas Kazlauskas
[Why]
To prepare for the upcoming DRM plane color management properties
we need to correct a lot of wrong behavior and assumptions made for
CRTC color management.
The documentation added by this commit in amdgpu_dm_color explains
how the HW color pipeline works and its limitations with the DRM
interface.
The current implementation does the following wrong:
- Implicit sRGB DGM when no CRTC DGM is set
- Implicit sRGB RGM when no CRTC RGM is set
- No way to specify a non-linear DGM matrix that produces correct output
- No way to specify a correct RGM when a linear DGM is used
We had workarounds for passing kms_color tests but not all of the
behavior we had wrong was covered by these tests (especially when
it comes to non-linear DGM). Testing both DGM and RGM at the same time
isn't something kms_color tests well either.
[How]
The specifics for how color management works in AMDGPU and the new
behavior can be found by reading the documentation added to
amdgpu_dm_color.c from this patch.
All of the incorrect cases from the old implementation have been
addressed for the atomic interface, but there still a few TODOs for
the legacy one.
Note: this does cause regressions for kms_color@pipe-a-ctm-* over HDMI.
The result looks correct from visual inspection but the CRC no longer
matches. For reference, the test was previously doing the following:
linear degamma -> CTM -> sRGB regamma -> RGB to YUV (709) -> ...
Now the test is doing:
linear degamma -> CTM -> linear regamma -> RGB to YUV (709) -> ...
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Sun peng Li
Acked-by: Bhawanpreet Lakha
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 10 +-
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 473 --
3 files changed, 356 insertions(+), 159 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 36ee8fe52f38..d267a9cc840f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2930,6 +2930,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device
*adev,
struct drm_plane_state *plane_state,
struct drm_crtc_state *crtc_state)
{
+ struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
const struct amdgpu_framebuffer *amdgpu_fb =
to_amdgpu_framebuffer(plane_state->fb);
struct dc_scaling_info scaling_info;
@@ -2974,13 +2975,11 @@ static int fill_dc_plane_attributes(struct
amdgpu_device *adev,
* Always set input transfer function, since plane state is refreshed
* every time.
*/
- ret = amdgpu_dm_set_degamma_lut(crtc_state, dc_plane_state);
- if (ret) {
- dc_transfer_func_release(dc_plane_state->in_transfer_func);
- dc_plane_state->in_transfer_func = NULL;
- }
+ ret = amdgpu_dm_update_plane_color_mgmt(dm_crtc_state, dc_plane_state);
+ if (ret)
+ return ret;
- return ret;
+ return 0;
}
static void update_stream_scaling_settings(const struct drm_display_mode *mode,
@@ -3552,6 +3551,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->vrr_supported = cur->vrr_supported;
state->freesync_config = cur->freesync_config;
state->crc_enabled = cur->crc_enabled;
+ state->cm_has_degamma = cur->cm_has_degamma;
+ state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
/* TODO Duplicate dc_stream after objects are stream object is
flattened */
@@ -5595,8 +5596,18 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
bundle->stream_update.dst = acrtc_state->stream->dst;
}
- if (new_pcrtc_state->color_mgmt_changed)
- bundle->stream_update.out_transfer_func =
acrtc_state->stream->out_transfer_func;
+ if (new_pcrtc_state->color_mgmt_changed) {
+ /*
+* TODO: This isn't fully correct since we've actually
+* already modified the stream in place.
+*/
+ bundle->stream_update.gamut_remap =
+ _state->stream->gamut_remap_matrix;
+ bundle->stream_update.output_csc_transform =
+ _state->stream->csc_color_matrix;
+ bundle->stream_update.out_transfer_func =
+ acrtc_state->stream->out_transfer_func;
+ }
acrtc_state->stream->abm_level = acrtc_state->abm_level;
if (acrtc_state->abm_level != dm_old_crtc_state->abm_level)
@@ -6426,10 +6437,9 @@ static int dm_update_crtc_state(struct
amdgpu_display_manager *dm,
[PATCH 03/24] drm/amd/display: Copy stream updates onto streams
From: Nicholas Kazlauskas
[Why]
Almost every function in DC that works with stream state expects that
the current state on the stream is the one that it should be writing
out. These functions are typically triggered by specifying a particular
stream update - but the actual contents of the stream update itself
are ignored, leaving it to the DM to actually update the stream state
itself.
The problem with doing this in DM is a matter of timing. On Linux
most of this is incorrectly done in atomic check, when we actually want
it to be done during atomic commit tail while access to DC is locked.
To give an example, a commit requesting to modify color management
state for DM could come in, be rejected, but still have modified
the actual system state for the stream since it's shared memory. The
next time color management gets programmed it'll use the rejected
color management info - which might not even still be around if it's
a custom transfer function.
So a reasonable place to perform this is within DC itself and this is
the model that's currently in use for surface updates. DC can even
compare the current system state to the incoming surface update to
determine update level, something that can't currnetly be done with the
framework for stream updates.
[How]
Duplicate the framework used for surface updates for stream updates
as well. Copy all the updates after checking the update type.
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Harry Wentland
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 69
1 file changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 0bff546d3727..45b542b5d920 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1607,6 +1607,73 @@ static void copy_surface_update_to_plane(
*srf_update->coeff_reduction_factor;
}
+static void copy_stream_update_to_stream(struct dc *dc,
+struct dc_state *context,
+struct dc_stream_state *stream,
+const struct dc_stream_update *update)
+{
+ if (update == NULL || stream == NULL)
+ return;
+
+ if (update->src.height && update->src.width)
+ stream->src = update->src;
+
+ if (update->dst.height && update->dst.width)
+ stream->dst = update->dst;
+
+ if (update->out_transfer_func &&
+ stream->out_transfer_func != update->out_transfer_func) {
+ stream->out_transfer_func->sdr_ref_white_level =
+ update->out_transfer_func->sdr_ref_white_level;
+ stream->out_transfer_func->tf = update->out_transfer_func->tf;
+ stream->out_transfer_func->type =
+ update->out_transfer_func->type;
+ memcpy(>out_transfer_func->tf_pts,
+ >out_transfer_func->tf_pts,
+ sizeof(struct dc_transfer_func_distributed_points));
+ }
+
+ if (update->hdr_static_metadata)
+ stream->hdr_static_metadata = *update->hdr_static_metadata;
+
+ if (update->abm_level)
+ stream->abm_level = *update->abm_level;
+
+ if (update->periodic_interrupt0)
+ stream->periodic_interrupt0 = *update->periodic_interrupt0;
+
+ if (update->periodic_interrupt1)
+ stream->periodic_interrupt1 = *update->periodic_interrupt1;
+
+ if (update->gamut_remap)
+ stream->gamut_remap_matrix = *update->gamut_remap;
+
+ /* Note: this being updated after mode set is currently not a use case
+* however if it arises OCSC would need to be reprogrammed at the
+* minimum
+*/
+ if (update->output_color_space)
+ stream->output_color_space = *update->output_color_space;
+
+ if (update->output_csc_transform)
+ stream->csc_color_matrix = *update->output_csc_transform;
+
+ if (update->vrr_infopacket)
+ stream->vrr_infopacket = *update->vrr_infopacket;
+
+ if (update->dpms_off)
+ stream->dpms_off = *update->dpms_off;
+
+ if (update->vsc_infopacket)
+ stream->vsc_infopacket = *update->vsc_infopacket;
+
+ if (update->vsp_infopacket)
+ stream->vsp_infopacket = *update->vsp_infopacket;
+
+ if (update->dither_option)
+ stream->dither_option = *update->dither_option;
+}
+
static void commit_planes_do_stream_update(struct dc *dc,
struct dc_stream_state *stream,
struct dc_stream_update *stream_update,
@@ -1857,6 +1924,8 @@ void dc_commit_updates_for_stream(struct dc *dc,
}
}
+ copy_stream_update_to_stream(dc, context, stream, stream_update);
+
commit_planes_for_stream(
[PATCH 16/24] drm/amd/display: Remove superflous error message
From: Jordan Lazare
[Why]
VBios sometimes reports incorrect object type as encoder instead of
connector
[How]
Change error message to debug message
Signed-off-by: Jordan Lazare
Reviewed-by: Harry Wentland
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 9f32ddfde41e..10807fa46ad6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1180,7 +1180,7 @@ static bool construct(
link->link_id = bios->funcs->get_connector_id(bios,
init_params->connector_index);
if (link->link_id.type != OBJECT_TYPE_CONNECTOR) {
- dm_error("%s: Invalid Connector ObjectID from Adapter Service
for connector index:%d! type %d expected %d\n",
+ dm_output_to_console("%s: Invalid Connector ObjectID from
Adapter Service for connector index:%d! type %d expected %d\n",
__func__, init_params->connector_index,
link->link_id.type, OBJECT_TYPE_CONNECTOR);
goto create_fail;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 18/24] drm/amd/display: Add Underflow Asserts to dc
From: Thomas Lim
[Why]
For debugging underflow issues it can be useful to have asserts when the
underflow initially occurs.
[How]
Read the underflow status registers after actions that have a high risk
of causing underflow and assert that no underflow occurred. If underflow
occurred, clear the bit.
Signed-off-by: Thomas Lim
Reviewed-by: Eric Yang
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 32 ++-
.../amd/display/dc/dcn10/dcn10_hw_sequencer.h | 2 ++
.../drm/amd/display/dc/dcn10/dcn10_resource.c | 4 ++-
.../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 1 +
5 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h
b/drivers/gpu/drm/amd/display/dc/dc.h
index ffd1fe5df99e..9aa01bf8c64d 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -329,6 +329,7 @@ struct dc_debug_options {
int sr_exit_time_ns;
int sr_enter_plus_exit_time_ns;
int urgent_latency_ns;
+ uint32_t underflow_assert_delay_us;
int percent_of_ideal_drambw;
int dram_clock_change_latency_ns;
bool optimized_watermark;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index be245d4fe5c2..f334756c1ce3 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -360,6 +360,23 @@ void dcn10_log_hw_state(struct dc *dc,
DTN_INFO_END();
}
+bool dcn10_did_underflow_occur(struct dc *dc, struct pipe_ctx *pipe_ctx)
+{
+ struct hubp *hubp = pipe_ctx->plane_res.hubp;
+ struct timing_generator *tg = pipe_ctx->stream_res.tg;
+
+ if (tg->funcs->is_optc_underflow_occurred(tg)) {
+ tg->funcs->clear_optc_underflow(tg);
+ return true;
+ }
+
+ if (hubp->funcs->hubp_get_underflow_status(hubp)) {
+ hubp->funcs->hubp_clear_underflow(hubp);
+ return true;
+ }
+ return false;
+}
+
static void enable_power_gating_plane(
struct dce_hwseq *hws,
bool enable)
@@ -2332,6 +2349,7 @@ static void dcn10_apply_ctx_for_surface(
{
int i;
struct timing_generator *tg;
+ uint32_t underflow_check_delay_us;
bool removed_pipe[4] = { false };
bool interdependent_update = false;
struct pipe_ctx *top_pipe_to_program =
@@ -2346,11 +2364,22 @@ static void dcn10_apply_ctx_for_surface(
interdependent_update = top_pipe_to_program->plane_state &&
top_pipe_to_program->plane_state->update_flags.bits.full_update;
+ underflow_check_delay_us = dc->debug.underflow_assert_delay_us;
+
+ if (underflow_check_delay_us != 0x &&
dc->hwss.did_underflow_occur)
+ ASSERT(dc->hwss.did_underflow_occur(dc, top_pipe_to_program));
+
if (interdependent_update)
lock_all_pipes(dc, context, true);
else
dcn10_pipe_control_lock(dc, top_pipe_to_program, true);
+ if (underflow_check_delay_us != 0x)
+ udelay(underflow_check_delay_us);
+
+ if (underflow_check_delay_us != 0x &&
dc->hwss.did_underflow_occur)
+ ASSERT(dc->hwss.did_underflow_occur(dc, top_pipe_to_program));
+
if (num_planes == 0) {
/* OTG blank before remove all front end */
dc->hwss.blank_pixel_data(dc, top_pipe_to_program, true);
@@ -3022,7 +3051,8 @@ static const struct hw_sequencer_funcs dcn10_funcs = {
.disable_stream_gating = NULL,
.enable_stream_gating = NULL,
.setup_periodic_interrupt = dcn10_setup_periodic_interrupt,
- .setup_vupdate_interrupt = dcn10_setup_vupdate_interrupt
+ .setup_vupdate_interrupt = dcn10_setup_vupdate_interrupt,
+ .did_underflow_occur = dcn10_did_underflow_occur
};
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
index ef94d6b15843..d3616b1948cc 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
@@ -71,6 +71,8 @@ void dcn10_get_hdr_visual_confirm_color(
struct pipe_ctx *pipe_ctx,
struct tg_color *color);
+bool dcn10_did_underflow_occur(struct dc *dc, struct pipe_ctx *pipe_ctx);
+
void update_dchubp_dpp(
struct dc *dc,
struct pipe_ctx *pipe_ctx,
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index f6004bc53dce..29fd3cb9422b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -560,6 +560,7 @@ static const struct dc_debug_options debug_defaults_drv
[PATCH 08/24] drm/amd/display: add audio related regs
From: Charlene Liu
Signed-off-by: Charlene Liu
Reviewed-by: Chris Park
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 4 +---
drivers/gpu/drm/amd/display/dc/dce/dce_audio.h | 7 +++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
index 7f6d724686f1..d43d5d924c19 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
@@ -22,7 +22,7 @@
* Authors: AMD
*
*/
-
+#include "../dc.h"
#include "reg_helper.h"
#include "dce_audio.h"
#include "dce/dce_11_0_d.h"
@@ -841,8 +841,6 @@ void dce_aud_wall_dto_setup(
REG_UPDATE(DCCG_AUDIO_DTO_SOURCE,
DCCG_AUDIO_DTO_SEL, 1);
- REG_UPDATE(DCCG_AUDIO_DTO_SOURCE,
- DCCG_AUDIO_DTO_SEL, 1);
/* DCCG_AUDIO_DTO2_USE_512FBR_DTO, 1)
* Select 512fs for DP TODO: web register definition
* does not match register header file
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
index 0dc5ff137c7a..a0d5724aab31 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
@@ -49,6 +49,8 @@
SF(DCCG_AUDIO_DTO_SOURCE, DCCG_AUDIO_DTO0_SOURCE_SEL, mask_sh),\
SF(DCCG_AUDIO_DTO_SOURCE, DCCG_AUDIO_DTO_SEL, mask_sh),\
SF(DCCG_AUDIO_DTO_SOURCE, DCCG_AUDIO_DTO2_USE_512FBR_DTO,
mask_sh),\
+ SF(DCCG_AUDIO_DTO_SOURCE, DCCG_AUDIO_DTO0_USE_512FBR_DTO,
mask_sh),\
+ SF(DCCG_AUDIO_DTO_SOURCE, DCCG_AUDIO_DTO1_USE_512FBR_DTO,
mask_sh),\
SF(DCCG_AUDIO_DTO0_MODULE, DCCG_AUDIO_DTO0_MODULE, mask_sh),\
SF(DCCG_AUDIO_DTO0_PHASE, DCCG_AUDIO_DTO0_PHASE, mask_sh),\
SF(DCCG_AUDIO_DTO1_MODULE, DCCG_AUDIO_DTO1_MODULE, mask_sh),\
@@ -95,6 +97,8 @@ struct dce_audio_shift {
uint8_t DCCG_AUDIO_DTO1_MODULE;
uint8_t DCCG_AUDIO_DTO1_PHASE;
uint8_t DCCG_AUDIO_DTO2_USE_512FBR_DTO;
+ uint32_t DCCG_AUDIO_DTO0_USE_512FBR_DTO;
+ uint32_t DCCG_AUDIO_DTO1_USE_512FBR_DTO;
};
struct dce_aduio_mask {
@@ -112,6 +116,9 @@ struct dce_aduio_mask {
uint32_t DCCG_AUDIO_DTO1_MODULE;
uint32_t DCCG_AUDIO_DTO1_PHASE;
uint32_t DCCG_AUDIO_DTO2_USE_512FBR_DTO;
+ uint32_t DCCG_AUDIO_DTO0_USE_512FBR_DTO;
+ uint32_t DCCG_AUDIO_DTO1_USE_512FBR_DTO;
+
};
struct dce_audio {
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 13/24] drm/amd/display: 3.2.34
From: Aric Cyr Signed-off-by: Aric Cyr Reviewed-by: Aric Cyr Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 3f22212437ca..59a9e1ea806d 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -39,7 +39,7 @@ #include "inc/hw/dmcu.h" #include "dml/display_mode_lib.h" -#define DC_VER "3.2.33" +#define DC_VER "3.2.34" #define MAX_SURFACES 3 #define MAX_PLANES 6 -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 17/24] drm/amd/display: move vmid determination logic out of dc
From: Dmytro Laktyushkin
Currently vmid is decided internally inside dc. This makes it
difficult to use vmid use with external components.
This change moves vmid logic outside dc and allowing vmid to be
passed in as a parameter to DC.
Signed-off-by: Dmytro Laktyushkin
Reviewed-by: Charlene Liu
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/Makefile | 4 +-
.../drm/amd/display/dc/core/dc_vm_helper.c| 123 --
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
drivers/gpu/drm/amd/display/dc/dc_hw_types.h | 2 +
.../gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c | 3 +-
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 3 +-
drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h | 3 +-
drivers/gpu/drm/amd/display/dc/inc/hw/vmid.h | 1 +
.../gpu/drm/amd/display/dc/inc/vm_helper.h| 16 +--
9 files changed, 13 insertions(+), 143 deletions(-)
delete mode 100644 drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
diff --git a/drivers/gpu/drm/amd/display/dc/Makefile
b/drivers/gpu/drm/amd/display/dc/Makefile
index 6da4e4f844b2..f581d5394caa 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -41,8 +41,8 @@ AMD_DC = $(addsuffix /Makefile, $(addprefix
$(FULL_AMD_DISPLAY_PATH)/dc/,$(DC_LI
include $(AMD_DC)
DISPLAY_CORE = dc.o dc_link.o dc_resource.o dc_hw_sequencer.o dc_sink.o \
-dc_surface.o dc_link_hwss.o dc_link_dp.o dc_link_ddc.o dc_debug.o dc_stream.o \
-dc_vm_helper.o
+dc_surface.o dc_link_hwss.o dc_link_dp.o dc_link_ddc.o dc_debug.o dc_stream.o
+
AMD_DISPLAY_CORE = $(addprefix $(AMDDALPATH)/dc/core/,$(DISPLAY_CORE))
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
b/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
deleted file mode 100644
index 6ce87b682a32..
--- a/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
+++ /dev/null
@@ -1,123 +0,0 @@
-/*
- * Copyright 2018 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.
- *
- * Authors: AMD
- *
- */
-
-#include "vm_helper.h"
-
-static void mark_vmid_used(struct vm_helper *vm_helper, unsigned int pos,
uint8_t hubp_idx)
-{
- struct vmid_usage vmids = vm_helper->hubp_vmid_usage[hubp_idx];
-
- vmids.vmid_usage[0] = vmids.vmid_usage[1];
- vmids.vmid_usage[1] = 1 << pos;
-}
-
-static void add_ptb_to_table(struct vm_helper *vm_helper, unsigned int vmid,
uint64_t ptb)
-{
- vm_helper->ptb_assigned_to_vmid[vmid] = ptb;
- vm_helper->num_vmids_available--;
-}
-
-static void clear_entry_from_vmid_table(struct vm_helper *vm_helper, unsigned
int vmid)
-{
- vm_helper->ptb_assigned_to_vmid[vmid] = 0;
- vm_helper->num_vmids_available++;
-}
-
-static void evict_vmids(struct vm_helper *vm_helper)
-{
- int i;
- uint16_t ord = 0;
-
- for (i = 0; i < vm_helper->num_vmid; i++)
- ord |= vm_helper->hubp_vmid_usage[i].vmid_usage[0] |
vm_helper->hubp_vmid_usage[i].vmid_usage[1];
-
- // At this point any positions with value 0 are unused vmids, evict them
- for (i = 1; i < vm_helper->num_vmid; i++) {
- if (ord & (1u << i))
- clear_entry_from_vmid_table(vm_helper, i);
- }
-}
-
-// Return value of -1 indicates vmid table unitialized or ptb dne in the table
-static int get_existing_vmid_for_ptb(struct vm_helper *vm_helper, uint64_t ptb)
-{
- int i;
-
- for (i = 0; i < vm_helper->num_vmid; i++) {
- if (vm_helper->ptb_assigned_to_vmid[i] == ptb)
- return i;
- }
-
- return -1;
-}
-
-// Expected to be called only when there's an available vmid
-static int get_next_available_vmid(struct vm_helper *vm_helper)
-{
- int i;
-
- for (i = 1; i < vm_helper->num_vmid; i++) {
- if (vm_helper->ptb_assigned_to_vmid[i] == 0)
- return i;
- }
-
- return -1;
-}
-
[PATCH 07/24] drm/amd/display: add i2c_hw_Status check to make sure as HW I2c in use
From: Derek Lai
1. Add i2c_hw_Status check to make sure when HW i2c is in use.
2. Don't reset HW engine in is_hw_busy() and instead do this in
process_transaction() because SW i2c does not check if hw i2c is in use
Signed-off-by: Derek Lai
Reviewed-by: Charlene Liu
Acked-by: Bhawanpreet Lakha
---
.../gpu/drm/amd/display/dc/dce/dce_i2c_hw.c | 65 +++
.../gpu/drm/amd/display/dc/dce/dce_i2c_hw.h | 5 ++
2 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
index 526aab438374..7f2460caa2a6 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
@@ -149,6 +149,36 @@ static void process_channel_reply(
}
}
+static bool is_engine_available(struct dce_i2c_hw *dce_i2c_hw)
+{
+ unsigned int arbitrate;
+ unsigned int i2c_hw_status;
+
+ REG_GET(HW_STATUS, DC_I2C_DDC1_HW_STATUS, _hw_status);
+ if (i2c_hw_status == DC_I2C_STATUS__DC_I2C_STATUS_USED_BY_HW)
+ return false;
+
+ REG_GET(DC_I2C_ARBITRATION, DC_I2C_REG_RW_CNTL_STATUS, );
+ if (arbitrate == DC_I2C_REG_RW_CNTL_STATUS_DMCU_ONLY)
+ return false;
+
+ return true;
+}
+
+static bool is_hw_busy(struct dce_i2c_hw *dce_i2c_hw)
+{
+ uint32_t i2c_sw_status = 0;
+
+ REG_GET(DC_I2C_SW_STATUS, DC_I2C_SW_STATUS, _sw_status);
+ if (i2c_sw_status == DC_I2C_STATUS__DC_I2C_STATUS_IDLE)
+ return false;
+
+ if (is_engine_available(dce_i2c_hw))
+ return false;
+
+ return true;
+}
+
static bool process_transaction(
struct dce_i2c_hw *dce_i2c_hw,
struct i2c_request_transaction_data *request)
@@ -159,6 +189,11 @@ static bool process_transaction(
bool last_transaction = false;
uint32_t value = 0;
+ if (is_hw_busy(dce_i2c_hw)) {
+ request->status = I2C_CHANNEL_OPERATION_ENGINE_BUSY;
+ return false;
+ }
+
last_transaction = ((dce_i2c_hw->transaction_count == 3) ||
(request->action ==
DCE_I2C_TRANSACTION_ACTION_I2C_WRITE) ||
(request->action &
DCE_I2C_TRANSACTION_ACTION_I2C_READ));
@@ -294,27 +329,12 @@ static bool setup_engine(
* Enable restart of SW I2C that was interrupted by HW
* disable queuing of software while I2C is in use by HW
*/
- REG_UPDATE_2(DC_I2C_ARBITRATION,
-DC_I2C_NO_QUEUED_SW_GO, 0,
-DC_I2C_SW_PRIORITY,
DC_I2C_ARBITRATION__DC_I2C_SW_PRIORITY_NORMAL);
+ REG_UPDATE(DC_I2C_ARBITRATION,
+ DC_I2C_NO_QUEUED_SW_GO, 0);
return true;
}
-static bool is_hw_busy(struct dce_i2c_hw *dce_i2c_hw)
-{
- uint32_t i2c_sw_status = 0;
-
- REG_GET(DC_I2C_SW_STATUS, DC_I2C_SW_STATUS, _sw_status);
- if (i2c_sw_status == DC_I2C_STATUS__DC_I2C_STATUS_IDLE)
- return false;
-
- reset_hw_engine(dce_i2c_hw);
-
- REG_GET(DC_I2C_SW_STATUS, DC_I2C_SW_STATUS, _sw_status);
- return i2c_sw_status != DC_I2C_STATUS__DC_I2C_STATUS_IDLE;
-}
-
static void release_engine(
struct dce_i2c_hw *dce_i2c_hw)
{
@@ -349,16 +369,6 @@ static void release_engine(
}
-static bool is_engine_available(struct dce_i2c_hw *dce_i2c_hw)
-{
- unsigned int arbitrate;
-
- REG_GET(DC_I2C_ARBITRATION, DC_I2C_REG_RW_CNTL_STATUS, );
- if (arbitrate == DC_I2C_REG_RW_CNTL_STATUS_DMCU_ONLY)
- return false;
- return true;
-}
-
struct dce_i2c_hw *acquire_i2c_hw_engine(
struct resource_pool *pool,
struct ddc *ddc)
@@ -456,6 +466,7 @@ static void submit_channel_request_hw(
request->status = I2C_CHANNEL_OPERATION_ENGINE_BUSY;
return;
}
+ reset_hw_engine(dce_i2c_hw);
execute_transaction(dce_i2c_hw);
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.h
b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.h
index f718e3d396f2..a633632f625b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.h
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.h
@@ -84,6 +84,7 @@ enum {
#define I2C_HW_ENGINE_COMMON_REG_LIST(id)\
SRI(SETUP, DC_I2C_DDC, id),\
SRI(SPEED, DC_I2C_DDC, id),\
+ SRI(HW_STATUS, DC_I2C_DDC, id),\
SR(DC_I2C_ARBITRATION),\
SR(DC_I2C_CONTROL),\
SR(DC_I2C_SW_STATUS),\
@@ -105,6 +106,7 @@ enum {
I2C_SF(DC_I2C_DDC1_SETUP, DC_I2C_DDC1_DATA_DRIVE_SEL, mask_sh),\
I2C_SF(DC_I2C_DDC1_SETUP, DC_I2C_DDC1_INTRA_TRANSACTION_DELAY,
mask_sh),\
I2C_SF(DC_I2C_DDC1_SETUP, DC_I2C_DDC1_INTRA_BYTE_DELAY, mask_sh),\
+ I2C_SF(DC_I2C_DDC1_HW_STATUS, DC_I2C_DDC1_HW_STATUS, mask_sh),\
I2C_SF(DC_I2C_ARBITRATION, DC_I2C_SW_USE_I2C_REG_REQ, mask_sh),\
I2C_SF(DC_I2C_ARBITRATION, DC_I2C_SW_DONE_USING_I2C_REG, mask_sh),\
[PATCH 06/24] drm/amd/display: Dont aser if DP_DPHY_INTERNAL_CTRL
From: Eric Bernstein
No need to assert just return
Signed-off-by: Eric Bernstein
Reviewed-by: Charlene Liu
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
index 3396e499090d..9427a461bb26 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c
@@ -229,7 +229,9 @@ static void setup_panel_mode(
{
uint32_t value;
- ASSERT(REG(DP_DPHY_INTERNAL_CTRL));
+ if (!REG(DP_DPHY_INTERNAL_CTRL))
+ return;
+
value = REG_READ(DP_DPHY_INTERNAL_CTRL);
switch (panel_mode) {
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 05/24] drm/amd/display: 3.2.33
From: Aric Cyr Signed-off-by: Aric Cyr Reviewed-by: Aric Cyr Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 7ec6884acee4..3f22212437ca 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -39,7 +39,7 @@ #include "inc/hw/dmcu.h" #include "dml/display_mode_lib.h" -#define DC_VER "3.2.32" +#define DC_VER "3.2.33" #define MAX_SURFACES 3 #define MAX_PLANES 6 -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 12/24] drm/amd/display: fix gamma logic breaking driver unload
From: Krunoslav Kovac
Using this logic breaks driver unload, this is a temporary fix
a followup patch will properly fix this
Signed-off-by: Krunoslav Kovac
Reviewed-by: Aric Cyr
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
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 89a65e1d8317..8601d371776e 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1569,15 +1569,13 @@ bool mod_color_calculate_regamma_params(struct
dc_transfer_func *output_tf,
output_tf->tf == TRANSFER_FUNCTION_SRGB) {
if (ramp == NULL)
return true;
- if ((ramp->is_identity && ramp->type != GAMMA_CS_TFM_1D) ||
- (!mapUserRamp && ramp->type == GAMMA_RGB_256))
+ if (ramp->is_identity || (!mapUserRamp && ramp->type ==
GAMMA_RGB_256))
return true;
}
output_tf->type = TF_TYPE_DISTRIBUTED_POINTS;
- if (ramp && ramp->type != GAMMA_CS_TFM_1D &&
- (mapUserRamp || ramp->type != GAMMA_RGB_256)) {
+ if (ramp && (mapUserRamp || ramp->type != GAMMA_RGB_256)) {
rgb_user = kvcalloc(ramp->num_entries + _EXTRA_POINTS,
sizeof(*rgb_user),
GFP_KERNEL);
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 20/24] drm/amd/display: Use stream opp_id instead of hubp
From: Wesley Chalmers [WHY] By the time output csc matrix is being programmed, stream connection to OPP has been established, but this information has not been relayed back to HUBP. Signed-off-by: Wesley Chalmers Reviewed-by: Anthony Koo Acked-by: Bhawanpreet Lakha Acked-by: Krunoslav Kovac --- drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 45b542b5d920..bf64a73f1482 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -457,7 +457,7 @@ bool dc_stream_program_csc_matrix(struct dc *dc, struct dc_stream_state *stream) pipes, stream->output_color_space, stream->csc_color_matrix.matrix, - pipes->plane_res.hubp ? pipes->plane_res.hubp->opp_id : 0); + pipes->stream_res.opp->inst); ret = true; } } diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index f334756c1ce3..d2352949c06e 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -2169,7 +2169,7 @@ void update_dchubp_dpp( pipe_ctx, pipe_ctx->stream->output_color_space, pipe_ctx->stream->csc_color_matrix.matrix, - hubp->opp_id); + pipe_ctx->stream_res.opp->inst); } if (plane_state->update_flags.bits.full_update || -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 14/24] drm/amd/display: 3.2.35
From: Aric Cyr Signed-off-by: Aric Cyr Reviewed-by: Aric Cyr Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 59a9e1ea806d..c4bd9216dd61 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -39,7 +39,7 @@ #include "inc/hw/dmcu.h" #include "dml/display_mode_lib.h" -#define DC_VER "3.2.34" +#define DC_VER "3.2.35" #define MAX_SURFACES 3 #define MAX_PLANES 6 -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 15/24] drm/amd/display: Clean up scdc_test_data struct
From: Chris Park
These are no longer needed, Also added RESERVED bits.
Signed-off-by: Chris Park
Reviewed-by: Charlene Liu
Acked-by: Bhawanpreet Lakha
Acked-by: Harry Wentland
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 1 -
drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 14 ++
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index c5dc809f17d6..9f32ddfde41e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -3004,4 +3004,3 @@ const struct dc_link_settings *dc_link_get_link_cap(
return >preferred_link_setting;
return >verified_link_cap;
}
-
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
index f02092a0dc76..94064d8ce303 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
@@ -91,6 +91,8 @@ union hdmi_scdc_status_flags_data {
uint8_t CH2_LOCKED:1;
uint8_t RESERVED:4;
uint8_t RESERVED2:8;
+ uint8_t RESERVED3:8;
+
} fields;
};
@@ -107,14 +109,10 @@ union hdmi_scdc_ced_data {
uint8_t CH2_7HIGH:7;
uint8_t CH2_VALID:1;
uint8_t CHECKSUM:8;
- } fields;
-};
-
-union hdmi_scdc_test_config_Data {
- uint8_t byte;
- struct {
- uint8_t TEST_READ_REQUEST_DELAY:7;
- uint8_t TEST_READ_REQUEST: 1;
+ uint8_t RESERVED:8;
+ uint8_t RESERVED2:8;
+ uint8_t RESERVED3:8;
+ uint8_t RESERVED4:4;
} fields;
};
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 19/24] drm/amd/display: Gamma logic limitations causing unintended use of RAM over ROM.
From: Harmanprit Tatla
[Why]
Our existing logic in deciding whether to use RAM or ROM
depends on whether we are dealing with an identity gamma ramp.
[How]
In addition to the is_identity flag
a new is_logical_identity flag has been
added. The is_identity flag now denotes
whether the OS gamma is an RGB256 identity
and the new logical identity will inidicate
that the given gamma ramp regardless of its
type is identity.
Signed-off-by: Harmanprit Tatla
Reviewed-by: Krunoslav Kovac
Acked-by: Anthony Koo
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/dc_hw_types.h| 4
drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 3 ++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
index 479c5f8352f9..821d4f260764 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -453,7 +453,11 @@ struct dc_gamma {
/* private to DC core */
struct dc_context *ctx;
+ /* is_identity is used for RGB256 gamma identity which can also be
programmed in INPUT_LUT.
+* is_logical_identity indicates the given gamma ramp regardless of
type is identity.
+*/
bool is_identity;
+ bool is_logical_identity;
};
/* Used by both ipp amd opp functions*/
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 8601d371776e..3f413fb9f2ce 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1569,7 +1569,8 @@ bool mod_color_calculate_regamma_params(struct
dc_transfer_func *output_tf,
output_tf->tf == TRANSFER_FUNCTION_SRGB) {
if (ramp == NULL)
return true;
- if (ramp->is_identity || (!mapUserRamp && ramp->type ==
GAMMA_RGB_256))
+ if ((ramp->is_logical_identity) ||
+ (!mapUserRamp && ramp->type == GAMMA_RGB_256))
return true;
}
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 09/24] drm/amd/display: Use macro for invalid OPP ID
From: Wesley Chalmers
[WHY]
This is meant to make it clearer that 0xf is not a valid OPP ID, and
that code making use of OPP IDs should not accept this value.
Signed-off-by: Wesley Chalmers
Reviewed-by: Charlene Liu
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c | 4 ++--
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 4 ++--
drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h | 2 ++
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
index 54b219a710d8..aea2b63db137 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
@@ -63,7 +63,7 @@ void hubp1_set_blank(struct hubp *hubp, bool blank)
}
hubp->mpcc_id = 0xf;
- hubp->opp_id = 0xf;
+ hubp->opp_id = OPP_ID_INVALID;
}
}
@@ -1226,7 +1226,7 @@ void dcn10_hubp_construct(
hubp1->hubp_shift = hubp_shift;
hubp1->hubp_mask = hubp_mask;
hubp1->base.inst = inst;
- hubp1->base.opp_id = 0xf;
+ hubp1->base.opp_id = OPP_ID_INVALID;
hubp1->base.mpcc_id = 0xf;
}
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 821a280eb481..d9edf7f84cfa 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -1025,7 +1025,7 @@ static void dcn10_init_pipes(struct dc *dc, struct
dc_state *context)
pipe_ctx->plane_res.dpp = dpp;
pipe_ctx->plane_res.mpcc_inst = dpp->inst;
hubp->mpcc_id = dpp->inst;
- hubp->opp_id = 0xf;
+ hubp->opp_id = OPP_ID_INVALID;
hubp->power_gated = false;
dc->res_pool->opps[i]->mpc_tree_params.opp_id =
dc->res_pool->opps[i]->inst;
@@ -2371,7 +2371,7 @@ static void dcn10_apply_ctx_for_surface(
if (pipe_ctx->plane_state && !old_pipe_ctx->plane_state) {
if (old_pipe_ctx->stream_res.tg == tg &&
old_pipe_ctx->plane_res.hubp &&
- old_pipe_ctx->plane_res.hubp->opp_id != 0xf)
+ old_pipe_ctx->plane_res.hubp->opp_id !=
OPP_ID_INVALID)
dcn10_disable_plane(dc, old_pipe_ctx);
}
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h
b/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h
index 455df4999797..5420ad2da96f 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h
@@ -28,6 +28,8 @@
#include "mem_input.h"
+#define OPP_ID_INVALID 0xf
+
enum cursor_pitch {
CURSOR_PITCH_64_PIXELS = 0,
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 21/24] drm/amd/display: S3 Resume time increase after decoupling DPMS from fast boot
From: SivapiriyanKumarasamy
[Why]
We incorrectly began powering down the display at boot/resume whenever
fast boot was not possible. This should not be done in the case where there
exists a stream for the eDP since this implies that we want to turn it on.
[How]
Add check for eDP stream to decide whether to power off edp.
Signed-off-by: SivapiriyanKumarasamy
Reviewed-by: Anthony Koo
Acked-by: Bhawanpreet Lakha
Acked-by: Reza Amini
---
.../display/dc/dce110/dce110_hw_sequencer.c | 35 ++-
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 3042741b165a..2a7ac452d458 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1507,6 +1507,18 @@ static void disable_vga_and_power_gate_all_controllers(
}
}
+
+static struct dc_stream_state *get_edp_stream(struct dc_state *context)
+{
+ int i;
+
+ for (i = 0; i < context->stream_count; i++) {
+ if (context->streams[i]->signal == SIGNAL_TYPE_EDP)
+ return context->streams[i];
+ }
+ return NULL;
+}
+
static struct dc_link *get_edp_link(struct dc *dc)
{
int i;
@@ -1550,12 +1562,16 @@ void dce110_enable_accelerated_mode(struct dc *dc,
struct dc_state *context)
int i;
struct dc_link *edp_link_with_sink = get_edp_link_with_sink(dc,
context);
struct dc_link *edp_link = get_edp_link(dc);
+ struct dc_stream_state *edp_stream = NULL;
bool can_apply_edp_fast_boot = false;
bool can_apply_seamless_boot = false;
+ bool keep_edp_vdd_on = false;
if (dc->hwss.init_pipes)
dc->hwss.init_pipes(dc, context);
+ edp_stream = get_edp_stream(context);
+
// Check fastboot support, disable on DCE8 because of blank screens
if (edp_link && dc->ctx->dce_version != DCE_VERSION_8_0 &&
dc->ctx->dce_version != DCE_VERSION_8_1 &&
@@ -1563,15 +1579,16 @@ void dce110_enable_accelerated_mode(struct dc *dc,
struct dc_state *context)
// enable fastboot if backend is enabled on eDP
if
(edp_link->link_enc->funcs->is_dig_enabled(edp_link->link_enc)) {
- /* Find eDP stream and set optimization flag */
- for (i = 0; i < context->stream_count; i++) {
- if (context->streams[i]->signal ==
SIGNAL_TYPE_EDP) {
-
context->streams[i]->apply_edp_fast_boot_optimization = true;
- can_apply_edp_fast_boot = true;
- break;
- }
+ /* Set optimization flag on eDP stream*/
+ if (edp_stream) {
+ edp_stream->apply_edp_fast_boot_optimization =
true;
+ can_apply_edp_fast_boot = true;
}
}
+
+ // We are trying to enable eDP, don't power down VDD
+ if (edp_stream)
+ keep_edp_vdd_on = true;
}
// Check seamless boot support
@@ -1586,14 +1603,14 @@ void dce110_enable_accelerated_mode(struct dc *dc,
struct dc_state *context)
* it should get turned off
*/
if (!can_apply_edp_fast_boot && !can_apply_seamless_boot) {
- if (edp_link_with_sink) {
+ if (edp_link_with_sink && !keep_edp_vdd_on) {
/*turn off backlight before DP_blank and encoder
powered down*/
dc->hwss.edp_backlight_control(edp_link_with_sink,
false);
}
/*resume from S3, no vbios posting, no need to power down
again*/
power_down_all_hw_blocks(dc);
disable_vga_and_power_gate_all_controllers(dc);
- if (edp_link_with_sink)
+ if (edp_link_with_sink && !keep_edp_vdd_on)
dc->hwss.edp_power_control(edp_link_with_sink, false);
}
bios_set_scratch_acc_mode_change(dc->ctx->dc_bios);
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 02/24] drm/amd/display: Update link rate from DPCD 10
From: Wesley Chalmers
[WHY]
Some panels return a link rate of 0 (unknown) in DPCD 0. In this case,
an appropriate mode cannot be set, and certain panels will show
corruption as they are forced to use a mode they do not support.
[HOW]
Read DPCD 10 in the case where supported link rate from DPCD 0 is
unknown, and pass that value on to the reported link rate.
This re-introduces behaviour present in previous versions that appears
to have been accidentally removed.
Signed-off-by: Wesley Chalmers
Reviewed-by: Anthony Koo
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 65d6caedbd82..2d519e5fc3ea 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -1624,8 +1624,7 @@ static bool decide_edp_link_settings(struct dc_link
*link, struct dc_link_settin
uint32_t link_bw;
if (link->dpcd_caps.dpcd_rev.raw < DPCD_REV_14 ||
- link->dpcd_caps.edp_supported_link_rates_count == 0 ||
- link->dc->config.optimize_edp_link_rate == false) {
+ link->dpcd_caps.edp_supported_link_rates_count == 0) {
*link_setting = link->verified_link_cap;
return true;
}
@@ -2609,7 +2608,8 @@ void detect_edp_sink_caps(struct dc_link *link)
memset(supported_link_rates, 0, sizeof(supported_link_rates));
if (link->dpcd_caps.dpcd_rev.raw >= DPCD_REV_14 &&
- link->dc->config.optimize_edp_link_rate) {
+ (link->dc->config.optimize_edp_link_rate ||
+ link->reported_link_cap.link_rate ==
LINK_RATE_UNKNOWN)) {
// Read DPCD 00010h - 0001Fh 16 bytes at one shot
core_link_read_dpcd(link, DP_SUPPORTED_LINK_RATES,
supported_link_rates,
sizeof(supported_link_rates));
@@ -2624,6 +2624,9 @@ void detect_edp_sink_caps(struct dc_link *link)
link_rate =
linkRateInKHzToLinkRateMultiplier(link_rate_in_khz);
link->dpcd_caps.edp_supported_link_rates[link->dpcd_caps.edp_supported_link_rates_count]
= link_rate;
link->dpcd_caps.edp_supported_link_rates_count++;
+
+ if (link->reported_link_cap.link_rate <
link_rate)
+ link->reported_link_cap.link_rate =
link_rate;
}
}
}
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 01/24] drm/amd/display: fix resource saving missing when power state switch
From: "Tao.Huang"
Signed-off-by: Tao.Huang
Reviewed-by: Jun Lei
Acked-by: Bhawanpreet Lakha
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index d89a29bd8785..0bff546d3727 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1930,6 +1930,12 @@ void dc_set_power_state(
enum dc_acpi_cm_power_state power_state)
{
struct kref refcount;
+ struct display_mode_lib *dml = kzalloc(sizeof(struct display_mode_lib),
+ GFP_KERNEL);
+
+ ASSERT(dml);
+ if (!dml)
+ return;
switch (power_state) {
case DC_ACPI_CM_POWER_STATE_D0:
@@ -1946,15 +1952,20 @@ void dc_set_power_state(
/* Preserve refcount */
refcount = dc->current_state->refcount;
+ /* Preserve display mode lib */
+ memcpy(dml, >current_state->bw_ctx.dml, sizeof(struct
display_mode_lib));
+
dc_resource_state_destruct(dc->current_state);
memset(dc->current_state, 0,
sizeof(*dc->current_state));
dc->current_state->refcount = refcount;
+ dc->current_state->bw_ctx.dml = *dml;
break;
}
+ kfree(dml);
}
void dc_resume(struct dc *dc)
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 00/24] DC Patches 06 Jun 2019
Summary Of Changes *Rework CRTC color management *Add underflow asserts *i2c fix *gamma fixes Anthony Koo (1): drm/amd/display: fix issue with eDP not detected on driver load Aric Cyr (3): drm/amd/display: 3.2.33 drm/amd/display: 3.2.34 drm/amd/display: 3.2.35 Charlene Liu (2): drm/amd/display: add some math functions for dcn_calc_math drm/amd/display: add audio related regs Chris Park (1): drm/amd/display: Clean up scdc_test_data struct Derek Lai (1): drm/amd/display: add i2c_hw_Status check to make sure as HW I2c in use Dmytro Laktyushkin (1): drm/amd/display: move vmid determination logic out of dc Eric Bernstein (1): drm/amd/display: Dont aser if DP_DPHY_INTERNAL_CTRL Harmanprit Tatla (1): drm/amd/display: Gamma logic limitations causing unintended use of RAM over ROM. Jordan Lazare (1): drm/amd/display: Remove superflous error message Krunoslav Kovac (1): drm/amd/display: fix gamma logic breaking driver unload Nicholas Kazlauskas (2): drm/amd/display: Copy stream updates onto streams drm/amd/display: Rework CRTC color management Samson Tam (1): drm/amd/display: set link->dongle_max_pix_clk to 0 on a disconnect SivapiriyanKumarasamy (1): drm/amd/display: S3 Resume time increase after decoupling DPMS from fast boot Su Sung Chung (1): drm/amd/display: make clk_mgr call enable_pme_wa Tao.Huang (1): drm/amd/display: fix resource saving missing when power state switch Thomas Lim (1): drm/amd/display: Add Underflow Asserts to dc Wesley Chalmers (3): drm/amd/display: Update link rate from DPCD 10 drm/amd/display: Use macro for invalid OPP ID drm/amd/display: Use stream opp_id instead of hubp abdoulaye berthe (1): drm/amd/display: Do not grant POST_LT_ADJ when TPS4 is used .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 10 +- .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 473 -- drivers/gpu/drm/amd/display/dc/Makefile | 4 +- .../drm/amd/display/dc/calcs/dcn_calc_math.c | 20 + .../drm/amd/display/dc/calcs/dcn_calc_math.h | 3 + .../display/dc/clk_mgr/dcn10/rv1_clk_mgr.c| 14 + drivers/gpu/drm/amd/display/dc/core/dc.c | 82 ++- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 51 +- .../gpu/drm/amd/display/dc/core/dc_link_ddc.c | 14 +- .../gpu/drm/amd/display/dc/core/dc_link_dp.c | 65 +-- .../drm/amd/display/dc/core/dc_vm_helper.c| 123 - drivers/gpu/drm/amd/display/dc/dc.h | 4 +- drivers/gpu/drm/amd/display/dc/dc_hw_types.h | 6 + .../gpu/drm/amd/display/dc/dce/dce_audio.c| 4 +- .../gpu/drm/amd/display/dc/dce/dce_audio.h| 7 + .../gpu/drm/amd/display/dc/dce/dce_i2c_hw.c | 65 ++- .../gpu/drm/amd/display/dc/dce/dce_i2c_hw.h | 5 + .../display/dc/dce110/dce110_hw_sequencer.c | 60 +-- .../gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c | 7 +- .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 41 +- .../amd/display/dc/dcn10/dcn10_hw_sequencer.h | 2 + .../amd/display/dc/dcn10/dcn10_link_encoder.c | 4 +- .../drm/amd/display/dc/dcn10/dcn10_resource.c | 4 +- .../drm/amd/display/dc/dml/dml_inline_defs.h | 8 + drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h | 5 +- drivers/gpu/drm/amd/display/dc/inc/hw/vmid.h | 1 + .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 1 + .../gpu/drm/amd/display/dc/inc/vm_helper.h| 16 +- .../amd/display/modules/color/color_gamma.c | 5 +- 30 files changed, 715 insertions(+), 421 deletions(-) delete mode 100644 drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 04/24] drm/amd/display: add some math functions for dcn_calc_math
From: Charlene Liu
Implement floor, ceil, and fabs
Signed-off-by: Charlene Liu
Reviewed-by: Charlene Liu
Acked-by: Bhawanpreet Lakha
---
.../drm/amd/display/dc/calcs/dcn_calc_math.c | 20 +++
.../drm/amd/display/dc/calcs/dcn_calc_math.h | 3 +++
.../drm/amd/display/dc/dml/dml_inline_defs.h | 8
3 files changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c
b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c
index 7600a4a4abc7..07d18e78de49 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c
@@ -73,6 +73,17 @@ float dcn_bw_floor2(const float arg, const float
significance)
return 0;
return ((int) (arg / significance)) * significance;
}
+float dcn_bw_floor(const float arg)
+{
+ return ((int) (arg));
+}
+
+float dcn_bw_ceil(const float arg)
+{
+ float flr = dcn_bw_floor2(arg, 1);
+
+ return flr + 0.1 >= arg ? arg : flr + 1;
+}
float dcn_bw_ceil2(const float arg, const float significance)
{
@@ -109,6 +120,15 @@ float dcn_bw_pow(float a, float exp)
}
}
+double dcn_bw_fabs(double a)
+{
+ if (a > 0)
+ return (a);
+ else
+ return (-a);
+}
+
+
float dcn_bw_log(float a, float b)
{
int * const exp_ptr = (int *)();
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.h
b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.h
index f46ab0e24ca1..45a07eeffbb6 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.h
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.h
@@ -31,10 +31,13 @@ float dcn_bw_min2(const float arg1, const float arg2);
unsigned int dcn_bw_max(const unsigned int arg1, const unsigned int arg2);
float dcn_bw_max2(const float arg1, const float arg2);
float dcn_bw_floor2(const float arg, const float significance);
+float dcn_bw_floor(const float arg);
float dcn_bw_ceil2(const float arg, const float significance);
+float dcn_bw_ceil(const float arg);
float dcn_bw_max3(float v1, float v2, float v3);
float dcn_bw_max5(float v1, float v2, float v3, float v4, float v5);
float dcn_bw_pow(float a, float exp);
float dcn_bw_log(float a, float b);
+double dcn_bw_fabs(double a);
#endif /* _DCN_CALC_MATH_H_ */
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dml_inline_defs.h
b/drivers/gpu/drm/amd/display/dc/dml/dml_inline_defs.h
index e8ce08567cd8..eca140da13d8 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dml_inline_defs.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dml_inline_defs.h
@@ -129,4 +129,12 @@ static inline unsigned int dml_round_to_multiple(unsigned
int num,
else
return (num - remainder);
}
+static inline double dml_abs(double a)
+{
+ if (a > 0)
+ return a;
+ else
+ return (a*(-1));
+}
+
#endif
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH revert] Revert "drm/amdgpu: use GMC v9 KIQ workaround only for the GFXHUB"
Not sure whether it is related to this change, when I boot a system with amdgpu blacklisted, then modprobe amdgpu, I followed issue. Failing path is gmc_v9_0_flush_gpu_tlb calling amdgpu_virt_kiq_req_write_reg_wait. If I boot w/o amdgpu blacklisted, then it is fine (load amdgpu directly during boot). [ 94.778231] amdgpu :08:00.0: ring uvd_enc_0.1 uses VM inv eng 6 on hub 1 [ 94.778234] amdgpu :08:00.0: ring vce0 uses VM inv eng 7 on hub 1 [ 94.778236] amdgpu :08:00.0: ring vce1 uses VM inv eng 8 on hub 1 [ 94.778239] amdgpu :08:00.0: ring vce2 uses VM inv eng 9 on hub 1 [ 94.778242] [drm] ECC is not present. [ 94.778245] [drm] SRAM ECC is not present. [ 94.779441] [drm] Initialized amdgpu 3.32.0 20150101 for :08:00.0 on minor 0 [ 96.307042] rfkill: input handler enabled [ 97.101621] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101647] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101653] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101663] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101668] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101673] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013D [ 97.101681] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101686] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101691] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101699] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101704] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101708] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101717] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101722] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101726] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101735] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101740] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101744] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101753] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101757] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101762] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101770] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101775] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101779] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101788] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101793] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101797] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 97.101806] amdgpu :08:00.0: [mmhub] no-retry page fault (src_id:0 ring:158 vmid:0 pasid:0, for process pid 0 thread pid 0) [ 97.101810] amdgpu :08:00.0: in page starting at address 0x0057 from 18 [ 97.101815] amdgpu :08:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x013C [ 98.711247] failed to write reg 28b4 wait reg 28c6 [ 100.315395] failed to write reg 1a6f4 wait reg 1a706 [ 101.943593] failed to write reg 28b4 wait reg 28c6 Regards, Oak -Original Message- From: amd-gfx On Behalf Of Chengming Gui Sent: Friday, November 16, 2018 3:21 AM To: amd-gfx@lists.freedesktop.org Cc: Gui, Jack Subject: [PATCH revert] Revert "drm/amdgpu: use GMC v9 KIQ workaround only for the GFXHUB" With GFXOFF enabled, this patch will cause PCO amdgpu_test failed, but GFXOFF is necessary for PCO, so revert the patch. This reverts commit b83761bb0b09ec11c924afe9d88e458cb16a0372. Signed-off-by: Jack Gui --- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index 811231e..14ca4d8 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -338,9 +338,9 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, struct amdgpu_vmhub *hub = >vmhub[i]; u32 tmp =
Re: [PATCH 6/6] drm/amdkfd: Fix sdma queue allocate race condition
Patches 5 and 6 are Reviewed-by: Felix Kuehling
On 2019-06-06 2:25 p.m., Zeng, Oak wrote:
> SDMA queue allocation requires the dqm lock at it modify
> the global dqm members. Move up the dqm_lock so sdma
> queue allocation is enclosed in the critical section. Move
> mqd allocation out of critical section to avoid circular
> lock dependency.
>
> Change-Id: I96abd42eae6e77c82a5ba1b8e600af3efe8d791d
> Signed-off-by: Oak Zeng
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 24
> +++---
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 166636c..cd259b8 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1133,23 +1133,27 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
> if (dqm->total_queue_count >= max_num_of_queues_per_device) {
> pr_warn("Can't create new usermode queue because %d queues were
> already created\n",
> dqm->total_queue_count);
> - retval = -EPERM;
> - goto out;
> + return -EPERM;
> }
>
> + mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> + q->properties.type)];
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
> + if (!q->mqd_mem_obj)
> + return -ENOMEM;
> +
> + dqm_lock(dqm);
> if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
> q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
> retval = allocate_sdma_queue(dqm, q);
> if (retval)
> - goto out;
> + goto out_unlock;
> }
>
> retval = allocate_doorbell(qpd, q);
> if (retval)
> goto out_deallocate_sdma_queue;
>
> - mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> - q->properties.type)];
> /*
>* Eviction state logic: mark all queues as evicted, even ones
>* not currently active. Restoring inactive queues later only
> @@ -1161,12 +1165,8 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
> dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
> q->properties.tba_addr = qpd->tba_addr;
> q->properties.tma_addr = qpd->tma_addr;
> - q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
> - if (!q->mqd_mem_obj)
> - goto out_deallocate_doorbell;
> mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
> >gart_mqd_addr, >properties);
> - dqm_lock(dqm);
>
> list_add(>list, >queues_list);
> qpd->queue_count++;
> @@ -1192,13 +1192,13 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
> dqm_unlock(dqm);
> return retval;
>
> -out_deallocate_doorbell:
> - deallocate_doorbell(qpd, q);
> out_deallocate_sdma_queue:
> if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
> q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
> deallocate_sdma_queue(dqm, q);
> -out:
> +out_unlock:
> + dqm_unlock(dqm);
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> return retval;
> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 4/6] drm/amdkfd: Separate mqd allocation and initialization
On 2019-06-06 2:25 p.m., Zeng, Oak wrote:
> Introduce a new mqd allocation interface and split the original
> init_mqd function into two functions: allocate_mqd and init_mqd.
> Also renamed uninit_mqd to free_mqd. This is preparation work to
> fix a circular lock dependency.
>
> Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
> Signed-off-by: Oak Zeng
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 34 -
> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 16 ++--
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c | 4 +-
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 18 +++--
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 78 +++
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 78 +++
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c| 88
> --
> 7 files changed, 124 insertions(+), 192 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 3c042eb..10d4f4f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -319,11 +319,11 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> if (retval)
> goto out_deallocate_hqd;
>
> - retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
> - >gart_mqd_addr, >properties);
> - if (retval)
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
> + if (!q->mqd_mem_obj)
> goto out_deallocate_doorbell;
You need to set retval = -ENOMEM here.
> -
> + mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
> + >gart_mqd_addr, >properties);
> if (q->properties.is_active) {
>
> if (WARN(q->process->mm != current->mm,
> @@ -333,7 +333,7 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
> q->queue, >properties, current->mm);
> if (retval)
> - goto out_uninit_mqd;
> + goto out_free_mqd;
> }
>
> list_add(>list, >queues_list);
> @@ -355,8 +355,8 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> dqm->total_queue_count);
> goto out_unlock;
>
> -out_uninit_mqd:
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> +out_free_mqd:
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> out_deallocate_doorbell:
> deallocate_doorbell(qpd, q);
> out_deallocate_hqd:
> @@ -450,7 +450,7 @@ static int destroy_queue_nocpsch_locked(struct
> device_queue_manager *dqm,
> if (retval == -ETIME)
> qpd->reset_wavefronts = true;
>
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>
> list_del(>list);
> if (list_empty(>queues_list)) {
> @@ -527,7 +527,7 @@ static int update_queue(struct device_queue_manager *dqm,
> struct queue *q)
> }
> }
>
> - retval = mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
> + mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
I think there is a chance now that retval will remain uninitialized in
this function (in the non-HWS case if a queue is inactive). You should
initialized it to 0 at the start of the function.
Check the other functions as well. Check for compiler warnings. Most of
these types of problems should be caught by the compiler.
>
> /*
>* check active state vs. the previous state and modify
> @@ -1160,11 +1160,11 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
> dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
> q->properties.tba_addr = qpd->tba_addr;
> q->properties.tma_addr = qpd->tma_addr;
> - retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
> - >gart_mqd_addr, >properties);
> - if (retval)
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
> + if (!q->mqd_mem_obj)
> goto out_deallocate_doorbell;
retval = -ENOMEM;
> -
> + mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
> + >gart_mqd_addr, >properties);
> dqm_lock(dqm);
>
> list_add(>list, >queues_list);
> @@ -1373,8 +1373,8 @@ static int destroy_queue_cpsch(struct
> device_queue_manager *dqm,
>
> dqm_unlock(dqm);
>
> - /* Do uninit_mqd after dqm_unlock(dqm) to avoid circular locking */
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>
>
Re: [PATCH 2/7] drm/dp_mst: Register AUX devices for MST ports
On 2019-06-03 3:28 p.m., Lyude Paul wrote: >> I'm reproducing this just by reloading i915 on a machine with some MST >> displays connected. I uploaded a copy of the script that I use to do this >> here: >> >> https://people.freedesktop.org/~lyudess/archive/06-03-2019/unloadgpumod.sh > oops-almost forgot to mention. The argument you pass to make it reload instead > of just unloading is --reload > Thanks for the script! So, the warning has to do with: 1. Having the aux device as a child of connector device, and 2. During driver unload, drm_connector_unregister() is called before drm_dp_mst_topology_put_port() Which means that connector_unregister() will recursively remove the child aux device, before put_port() can properly unregister it. Any further attempts to remove after the first will throw a "not found" warning. Call-stacks for reference: *drm_connector_unregister*+0x37/0x60 [drm] drm_connector_unregister_all+0x30/0x60 [drm] drm_modeset_unregister_all+0xe/0x30 [drm] drm_dev_unregister+0x9c/0xb0 [drm] i915_driver_unload+0x73/0x120 [i915] drm_dp_aux_unregister_devnode+0xf5/0x180 [drm_kms_helper] *drm_dp_mst_topology_put_port*+0x4e/0xf0 [drm_kms_helper] drm_dp_mst_topology_put_mstb+0x91/0x160 [drm_kms_helper] drm_dp_mst_topology_mgr_set_mst+0x12b/0x2b0 [drm_kms_helper] ? __finish_swait+0x10/0x40 drm_dp_mst_topology_mgr_destroy+0x11/0xa0 [drm_kms_helper] intel_dp_encoder_flush_work+0x32/0xb0 [i915] intel_ddi_encoder_destroy+0x32/0x60 [i915] drm_mode_config_cleanup+0x51/0x2e0 [drm] intel_modeset_cleanup+0xc8/0x140 [i915] i915_driver_unload+0xa0/0x120 [i915] A solution is to unregister the aux device immediately before the connector device is unregistered - if we are to keep the aux device as a child. Following current scheme with SST, it looks like drm_connector_funcs->early_unregister() is the right place to do so. To keep the balance, aux registration will then happen in drm_connector_funcs->late_register(). This will leave the SDP transaction handling part in DRM still, but pass the responsibility of creating and removing remote (fake) aux devices to the driver. I have a WIP patch here for you to take a look. It should apply on top of the existing patchset: https://pastebin.com/1YJZhL4C I'd like to hear your thoughts, before I go and modify other drivers :) Thanks, Leo ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 0/2] Two bug-fixes for HMM
On Thu, Jun 06, 2019 at 07:04:46PM +, Kuehling, Felix wrote: > On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote: > > On Fri, May 10, 2019 at 07:53:21PM +, Kuehling, Felix wrote: > >> These problems were found in AMD-internal testing as we're working on > >> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get > >> them applied to a mainline Linux kernel as well as drm-next and > >> amd-staging-drm-next sooner rather than later. > >> > >> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, > >> but the driver changes for HMM are expected to land in 5.2 and will need to > >> be rebased on those HMM changes. > >> > >> I'd like to work out a flow between Jerome, Dave, Alex and myself that > >> allows us to test the latest version of HMM on amd-staging-drm-next so > >> that ideally everything comes together in master without much need for > >> rebasing and retesting. > > I think we have that now, I'm running a hmm.git that is clean and can > > be used for merging into DRM related trees (and RDMA). I've commited > > to send this tree to Linus at the start of the merge window. > > > > See here: > > > > https://lore.kernel.org/lkml/20190524124455.gb16...@ziepe.ca/ > > > > The tree is here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm > > > > However please consult with me before making a merge commit to be > > co-ordinated. Thanks > > > > I see in this thread that AMDGPU missed 5.2 beacause of the > > co-ordination problems this tree is intended to solve, so I'm very > > hopeful this will help your work move into 5.3! > > Thanks Jason. Our two patches below were already included in the MM tree > (https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, > does that mean HMM fixes and changes will no longer go through Andrew > Morton but directly through your tree to Linus? I belive so, that is what we agreed to in the RFC. At least for this cycle. I already noticed the duplication and sent Andrew a separate note.. It will be best if most of the things touching mm/hmm.c go to hmm.git to avoid conflicts for Linus. > We have also applied the two patches to our internal tree which is > currently based on 5.2-rc1 so we can make progress. Makes sense, this is is also why this shared tree should be very helpful.. I intend to run it as a clean and stable non-rebasing tree, ah probably starting tomorrow since I see there is still another fixup. :) > Alex, I think merging hmm would be an extra step every time you rebase > amd-staging-drm-next. We could probably also merge hmm at other times as > needed. Do you think this would cause trouble or confusion for > upstreaming through drm-next? I'm not sure what the workflow the amd tree uses, but.. Broadly: If the AMD tree is rebasing then likely you can simply rebase your AMD tree on top of hmm.git (or maybe hmm.git merge'd into DRM). Most likely we will want to send a PR for hmm.git to main DRM tree prior to merging AMD's tree, but I'm also rather relying on DRM folks to help build the workflow they want in their world.. There are quite a few options depending on people's preferences and workflow. Thanks, Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 0/2] Two bug-fixes for HMM
[resent with correct address for Alex] On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote: > On Fri, May 10, 2019 at 07:53:21PM +, Kuehling, Felix wrote: >> These problems were found in AMD-internal testing as we're working on >> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like >> to get >> them applied to a mainline Linux kernel as well as drm-next and >> amd-staging-drm-next sooner rather than later. >> >> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, >> but the driver changes for HMM are expected to land in 5.2 and will >> need to >> be rebased on those HMM changes. >> >> I'd like to work out a flow between Jerome, Dave, Alex and myself that >> allows us to test the latest version of HMM on amd-staging-drm-next so >> that ideally everything comes together in master without much need for >> rebasing and retesting. > I think we have that now, I'm running a hmm.git that is clean and can > be used for merging into DRM related trees (and RDMA). I've commited > to send this tree to Linus at the start of the merge window. > > See here: > > https://lore.kernel.org/lkml/20190524124455.gb16...@ziepe.ca/ > > The tree is here: > > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm > > However please consult with me before making a merge commit to be > co-ordinated. Thanks > > I see in this thread that AMDGPU missed 5.2 beacause of the > co-ordination problems this tree is intended to solve, so I'm very > hopeful this will help your work move into 5.3! Thanks Jason. Our two patches below were already included in the MM tree (https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, does that mean HMM fixes and changes will no longer go through Andrew Morton but directly through your tree to Linus? We have also applied the two patches to our internal tree which is currently based on 5.2-rc1 so we can make progress. Alex, I think merging hmm would be an extra step every time you rebase amd-staging-drm-next. We could probably also merge hmm at other times as needed. Do you think this would cause trouble or confusion for upstreaming through drm-next? Regards, Felix > >> Maybe having Jerome's latest HMM changes in drm-next. However, that may >> create dependencies where Jerome and Dave need to coordinate their pull- >> requests for master. >> >> Felix Kuehling (1): >> mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking >> >> Philip Yang (1): >> mm/hmm: support automatic NUMA balancing > I've applied both of these patches with Jerome's Reviewed-by to > hmm.git and added the missed Signed-off-by > > If you test and confirm I think this branch would be ready for merging > toward the AMD tree. > Regards, > Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 0/2] Two bug-fixes for HMM
On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote: > On Fri, May 10, 2019 at 07:53:21PM +, Kuehling, Felix wrote: >> These problems were found in AMD-internal testing as we're working on >> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get >> them applied to a mainline Linux kernel as well as drm-next and >> amd-staging-drm-next sooner rather than later. >> >> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, >> but the driver changes for HMM are expected to land in 5.2 and will need to >> be rebased on those HMM changes. >> >> I'd like to work out a flow between Jerome, Dave, Alex and myself that >> allows us to test the latest version of HMM on amd-staging-drm-next so >> that ideally everything comes together in master without much need for >> rebasing and retesting. > I think we have that now, I'm running a hmm.git that is clean and can > be used for merging into DRM related trees (and RDMA). I've commited > to send this tree to Linus at the start of the merge window. > > See here: > > https://lore.kernel.org/lkml/20190524124455.gb16...@ziepe.ca/ > > The tree is here: > > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm > > However please consult with me before making a merge commit to be > co-ordinated. Thanks > > I see in this thread that AMDGPU missed 5.2 beacause of the > co-ordination problems this tree is intended to solve, so I'm very > hopeful this will help your work move into 5.3! Thanks Jason. Our two patches below were already included in the MM tree (https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, does that mean HMM fixes and changes will no longer go through Andrew Morton but directly through your tree to Linus? We have also applied the two patches to our internal tree which is currently based on 5.2-rc1 so we can make progress. Alex, I think merging hmm would be an extra step every time you rebase amd-staging-drm-next. We could probably also merge hmm at other times as needed. Do you think this would cause trouble or confusion for upstreaming through drm-next? Regards, Felix > >> Maybe having Jerome's latest HMM changes in drm-next. However, that may >> create dependencies where Jerome and Dave need to coordinate their pull- >> requests for master. >> >> Felix Kuehling (1): >>mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking >> >> Philip Yang (1): >>mm/hmm: support automatic NUMA balancing > I've applied both of these patches with Jerome's Reviewed-by to > hmm.git and added the missed Signed-off-by > > If you test and confirm I think this branch would be ready for merging > toward the AMD tree. > Regards, > Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
From: Jason Gunthorpe
hmm_release() is called exactly once per hmm. ops->release() cannot
accidentally trigger any action that would recurse back onto
hmm->mirrors_sem.
This fixes a use after-free race of the form:
CPU0 CPU1
hmm_release()
up_write(>mirrors_sem);
hmm_mirror_unregister(mirror)
down_write(>mirrors_sem);
up_write(>mirrors_sem);
kfree(mirror)
mirror->ops->release(mirror)
The only user we have today for ops->release is an empty function, so this
is unambiguously safe.
As a consequence of plugging this race drivers are not allowed to
register/unregister mirrors from within a release op.
Signed-off-by: Jason Gunthorpe
---
mm/hmm.c | 28 +---
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index 709d138dd49027..3a45dd3d778248 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -136,26 +136,16 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
WARN_ON(!list_empty(>ranges));
mutex_unlock(>lock);
- down_write(>mirrors_sem);
- mirror = list_first_entry_or_null(>mirrors, struct hmm_mirror,
- list);
- while (mirror) {
- list_del_init(>list);
- if (mirror->ops->release) {
- /*
-* Drop mirrors_sem so the release callback can wait
-* on any pending work that might itself trigger a
-* mmu_notifier callback and thus would deadlock with
-* us.
-*/
- up_write(>mirrors_sem);
+ down_read(>mirrors_sem);
+ list_for_each_entry(mirror, >mirrors, list) {
+ /*
+* Note: The driver is not allowed to trigger
+* hmm_mirror_unregister() from this thread.
+*/
+ if (mirror->ops->release)
mirror->ops->release(mirror);
- down_write(>mirrors_sem);
- }
- mirror = list_first_entry_or_null(>mirrors,
- struct hmm_mirror, list);
}
- up_write(>mirrors_sem);
+ up_read(>mirrors_sem);
hmm_put(hmm);
}
@@ -287,7 +277,7 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror)
struct hmm *hmm = mirror->hmm;
down_write(>mirrors_sem);
- list_del_init(>list);
+ list_del(>list);
up_write(>mirrors_sem);
hmm_put(hmm);
memset(>hmm, POISON_INUSE, sizeof(mirror->hmm));
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 06/11] mm/hmm: Hold on to the mmget for the lifetime of the range
From: Jason Gunthorpe
Range functions like hmm_range_snapshot() and hmm_range_fault() call
find_vma, which requires hodling the mmget() and the mmap_sem for the mm.
Make this simpler for the callers by holding the mmget() inside the range
for the lifetime of the range. Other functions that accept a range should
only be called if the range is registered.
This has the side effect of directly preventing hmm_release() from
happening while a range is registered. That means range->dead cannot be
false during the lifetime of the range, so remove dead and
hmm_mirror_mm_is_alive() entirely.
Signed-off-by: Jason Gunthorpe
---
v2:
- Use Jerome's idea of just holding the mmget() for the range lifetime,
rework the patch to use that as as simplification to remove dead in
one step
---
include/linux/hmm.h | 26 --
mm/hmm.c| 28 ++--
2 files changed, 10 insertions(+), 44 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2ab35b40992b24..0e20566802967a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -91,7 +91,6 @@
* @mirrors_sem: read/write semaphore protecting the mirrors list
* @wq: wait queue for user waiting on a range invalidation
* @notifiers: count of active mmu notifiers
- * @dead: is the mm dead ?
*/
struct hmm {
struct mm_struct*mm;
@@ -104,7 +103,6 @@ struct hmm {
wait_queue_head_t wq;
struct rcu_head rcu;
longnotifiers;
- booldead;
};
/*
@@ -469,30 +467,6 @@ struct hmm_mirror {
int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm);
void hmm_mirror_unregister(struct hmm_mirror *mirror);
-/*
- * hmm_mirror_mm_is_alive() - test if mm is still alive
- * @mirror: the HMM mm mirror for which we want to lock the mmap_sem
- * Return: false if the mm is dead, true otherwise
- *
- * This is an optimization, it will not always accurately return false if the
- * mm is dead; i.e., there can be false negatives (process is being killed but
- * HMM is not yet informed of that). It is only intended to be used to optimize
- * out cases where the driver is about to do something time consuming and it
- * would be better to skip it if the mm is dead.
- */
-static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror *mirror)
-{
- struct mm_struct *mm;
-
- if (!mirror || !mirror->hmm)
- return false;
- mm = READ_ONCE(mirror->hmm->mm);
- if (mirror->hmm->dead || !mm)
- return false;
-
- return true;
-}
-
/*
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
diff --git a/mm/hmm.c b/mm/hmm.c
index dc30edad9a8a02..f67ba32983d9f1 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -80,7 +80,6 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
mutex_init(>lock);
kref_init(>kref);
hmm->notifiers = 0;
- hmm->dead = false;
hmm->mm = mm;
hmm->mmu_notifier.ops = _mmu_notifier_ops;
@@ -124,20 +123,17 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
- struct hmm_range *range;
/* hmm is in progress to free */
if (!kref_get_unless_zero(>kref))
return;
- /* Report this HMM as dying. */
- hmm->dead = true;
-
- /* Wake-up everyone waiting on any range. */
mutex_lock(>lock);
- list_for_each_entry(range, >ranges, list)
- range->valid = false;
- wake_up_all(>wq);
+ /*
+* Since hmm_range_register() holds the mmget() lock hmm_release() is
+* prevented as long as a range exists.
+*/
+ WARN_ON(!list_empty(>ranges));
mutex_unlock(>lock);
down_write(>mirrors_sem);
@@ -909,8 +905,8 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- /* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead)
+ /* Prevent hmm_release() from running while the range is valid */
+ if (!mmget_not_zero(hmm->mm))
return -EFAULT;
range->hmm = hmm;
@@ -955,6 +951,7 @@ void hmm_range_unregister(struct hmm_range *range)
/* Drop reference taken by hmm_range_register() */
range->valid = false;
+ mmput(hmm->mm);
hmm_put(hmm);
range->hmm = NULL;
}
@@ -982,10 +979,7 @@ long hmm_range_snapshot(struct hmm_range *range)
struct vm_area_struct *vma;
struct mm_walk mm_walk;
- /* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead)
- return -EFAULT;
-
+ lockdep_assert_held(>mm->mmap_sem);
do {
/* If range is no longer valid force retry. */
if (!range->valid)
@@ -1080,9 +1074,7
[PATCH v2 hmm 07/11] mm/hmm: Use lockdep instead of comments
From: Jason Gunthorpe
So we can check locking at runtime.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
v2
- Fix missing & in lockdeps (Jason)
---
mm/hmm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index f67ba32983d9f1..c702cd72651b53 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -254,11 +254,11 @@ static const struct mmu_notifier_ops hmm_mmu_notifier_ops
= {
*
* To start mirroring a process address space, the device driver must register
* an HMM mirror struct.
- *
- * THE mm->mmap_sem MUST BE HELD IN WRITE MODE !
*/
int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm)
{
+ lockdep_assert_held_exclusive(>mmap_sem);
+
/* Sanity check */
if (!mm || !mirror || !mirror->ops)
return -EINVAL;
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
From: Jason Gunthorpe
Ralph observes that hmm_range_register() can only be called by a driver
while a mirror is registered. Make this clear in the API by passing in the
mirror structure as a parameter.
This also simplifies understanding the lifetime model for struct hmm, as
the hmm pointer must be valid as part of a registered mirror so all we
need in hmm_register_range() is a simple kref_get.
Suggested-by: Ralph Campbell
Signed-off-by: Jason Gunthorpe
---
v2
- Include the oneline patch to nouveau_svm.c
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
include/linux/hmm.h | 7 ---
mm/hmm.c | 15 ++-
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 93ed43c413f0bb..8c92374afcf227 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
range.values = nouveau_svm_pfn_values;
range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
again:
- ret = hmm_vma_fault(, true);
+ ret = hmm_vma_fault(>mirror, , true);
if (ret == 0) {
mutex_lock(>mutex);
if (!hmm_vma_range_done()) {
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 688c5ca7068795..2d519797cb134a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror
*mirror)
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift);
@@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct hmm_range
*range)
}
/* This is a temporary helper to avoid merge conflict between trees. */
-static inline int hmm_vma_fault(struct hmm_range *range, bool block)
+static inline int hmm_vma_fault(struct hmm_mirror *mirror,
+ struct hmm_range *range, bool block)
{
long ret;
@@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range *range,
bool block)
range->default_flags = 0;
range->pfn_flags_mask = -1UL;
- ret = hmm_range_register(range, range->vma->vm_mm,
+ ret = hmm_range_register(range, mirror,
range->start, range->end,
PAGE_SHIFT);
if (ret)
diff --git a/mm/hmm.c b/mm/hmm.c
index 547002f56a163d..8796447299023c 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
* Track updates to the CPU page table see include/linux/hmm.h
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift)
{
unsigned long mask = ((1UL << page_shift) - 1UL);
- struct hmm *hmm;
+ struct hmm *hmm = mirror->hmm;
range->valid = false;
range->hmm = NULL;
@@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- hmm = hmm_get_or_create(mm);
- if (!hmm)
- return -EFAULT;
-
/* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead) {
- hmm_put(hmm);
+ if (hmm->mm == NULL || hmm->dead)
return -EFAULT;
- }
+
+ range->hmm = hmm;
+ kref_get(>kref);
/* Initialize range to track CPU page table updates. */
mutex_lock(>lock);
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 08/11] mm/hmm: Remove racy protection against double-unregistration
From: Jason Gunthorpe
No other register/unregister kernel API attempts to provide this kind of
protection as it is inherently racy, so just drop it.
Callers should provide their own protection, it appears nouveau already
does, but just in case drop a debugging POISON.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
mm/hmm.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index c702cd72651b53..6802de7080d172 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -284,18 +284,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
*/
void hmm_mirror_unregister(struct hmm_mirror *mirror)
{
- struct hmm *hmm = READ_ONCE(mirror->hmm);
-
- if (hmm == NULL)
- return;
+ struct hmm *hmm = mirror->hmm;
down_write(>mirrors_sem);
list_del_init(>list);
- /* To protect us against double unregister ... */
- mirror->hmm = NULL;
up_write(>mirrors_sem);
-
hmm_put(hmm);
+ memset(>hmm, POISON_INUSE, sizeof(mirror->hmm));
}
EXPORT_SYMBOL(hmm_mirror_unregister);
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 04/11] mm/hmm: Simplify hmm_get_or_create and make it reliable
From: Jason Gunthorpe
As coded this function can false-fail in various racy situations. Make it
reliable by running only under the write side of the mmap_sem and avoiding
the false-failing compare/exchange pattern.
Also make the locking very easy to understand by only ever reading or
writing mm->hmm while holding the write side of the mmap_sem.
Signed-off-by: Jason Gunthorpe
---
v2:
- Fix error unwind of mmgrab (Jerome)
- Use hmm local instead of 2nd container_of (Jerome)
---
mm/hmm.c | 80
1 file changed, 29 insertions(+), 51 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index cc7c26fda3300e..dc30edad9a8a02 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -40,16 +40,6 @@
#if IS_ENABLED(CONFIG_HMM_MIRROR)
static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
-static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
-{
- struct hmm *hmm = READ_ONCE(mm->hmm);
-
- if (hmm && kref_get_unless_zero(>kref))
- return hmm;
-
- return NULL;
-}
-
/**
* hmm_get_or_create - register HMM against an mm (HMM internal)
*
@@ -64,11 +54,20 @@ static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
*/
static struct hmm *hmm_get_or_create(struct mm_struct *mm)
{
- struct hmm *hmm = mm_get_hmm(mm);
- bool cleanup = false;
+ struct hmm *hmm;
- if (hmm)
- return hmm;
+ lockdep_assert_held_exclusive(>mmap_sem);
+
+ if (mm->hmm) {
+ if (kref_get_unless_zero(>hmm->kref))
+ return mm->hmm;
+ /*
+* The hmm is being freed by some other CPU and is pending a
+* RCU grace period, but this CPU can NULL now it since we
+* have the mmap_sem.
+*/
+ mm->hmm = NULL;
+ }
hmm = kmalloc(sizeof(*hmm), GFP_KERNEL);
if (!hmm)
@@ -83,57 +82,36 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
hmm->notifiers = 0;
hmm->dead = false;
hmm->mm = mm;
- mmgrab(hmm->mm);
-
- spin_lock(>page_table_lock);
- if (!mm->hmm)
- mm->hmm = hmm;
- else
- cleanup = true;
- spin_unlock(>page_table_lock);
- if (cleanup)
- goto error;
-
- /*
-* We should only get here if hold the mmap_sem in write mode ie on
-* registration of first mirror through hmm_mirror_register()
-*/
hmm->mmu_notifier.ops = _mmu_notifier_ops;
- if (__mmu_notifier_register(>mmu_notifier, mm))
- goto error_mm;
+ if (__mmu_notifier_register(>mmu_notifier, mm)) {
+ kfree(hmm);
+ return NULL;
+ }
+ mmgrab(hmm->mm);
+ mm->hmm = hmm;
return hmm;
-
-error_mm:
- spin_lock(>page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(>page_table_lock);
-error:
- mmdrop(hmm->mm);
- kfree(hmm);
- return NULL;
}
static void hmm_free_rcu(struct rcu_head *rcu)
{
- kfree(container_of(rcu, struct hmm, rcu));
+ struct hmm *hmm = container_of(rcu, struct hmm, rcu);
+
+ down_write(>mm->mmap_sem);
+ if (hmm->mm->hmm == hmm)
+ hmm->mm->hmm = NULL;
+ up_write(>mm->mmap_sem);
+ mmdrop(hmm->mm);
+
+ kfree(hmm);
}
static void hmm_free(struct kref *kref)
{
struct hmm *hmm = container_of(kref, struct hmm, kref);
- struct mm_struct *mm = hmm->mm;
-
- mmu_notifier_unregister_no_release(>mmu_notifier, mm);
- spin_lock(>page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(>page_table_lock);
-
- mmdrop(hmm->mm);
+ mmu_notifier_unregister_no_release(>mmu_notifier, hmm->mm);
mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
}
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
From: Jason Gunthorpe
The wait_event_timeout macro already tests the condition as its first
action, so there is no reason to open code another version of this, all
that does is skip the might_sleep() debugging in common cases, which is
not helpful.
Further, based on prior patches, we can no simplify the required condition
test:
- If range is valid memory then so is range->hmm
- If hmm_release() has run then range->valid is set to false
at the same time as dead, so no reason to check both.
- A valid hmm has a valid hmm->mm.
Also, add the READ_ONCE for range->valid as there is no lock held here.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
include/linux/hmm.h | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 4ee3acabe5ed22..2ab35b40992b24 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
struct hmm_range *range)
static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
unsigned long timeout)
{
- /* Check if mm is dead ? */
- if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
- range->valid = false;
- return false;
- }
- if (range->valid)
- return true;
- wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
+ wait_event_timeout(range->hmm->wq, range->valid,
msecs_to_jiffies(timeout));
- /* Return current valid status just in case we get lucky */
- return range->valid;
+ return READ_ONCE(range->valid);
}
/*
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
From: Jason Gunthorpe Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON and poison bytes to detect this condition. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse --- v2 - Keep range start/end valid after unregistration (Jerome) --- mm/hmm.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 6802de7080d172..c2fecb3ecb11e1 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) struct hmm *hmm = range->hmm; /* Sanity check this really should not happen. */ - if (hmm == NULL || range->end <= range->start) + if (WARN_ON(range->end <= range->start)) return; mutex_lock(>lock); @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) range->valid = false; mmput(hmm->mm); hmm_put(hmm); - range->hmm = NULL; + + /* The range is now invalid, leave it poisoned. */ + range->valid = false; + memset(>hmm, POISON_INUSE, sizeof(range->hmm)); } EXPORT_SYMBOL(hmm_range_unregister); -- 2.21.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 00/11] Various revisions from a locking/code review
From: Jason Gunthorpe For hmm.git: This patch series arised out of discussions with Jerome when looking at the ODP changes, particularly informed by use after free races we have already found and fixed in the ODP code (thanks to syzkaller) working with mmu notifiers, and the discussion with Ralph on how to resolve the lifetime model. Overall this brings in a simplified locking scheme and easy to explain lifetime model: If a hmm_range is valid, then the hmm is valid, if a hmm is valid then the mm is allocated memory. If the mm needs to still be alive (ie to lock the mmap_sem, find a vma, etc) then the mmget must be obtained via mmget_not_zero(). Locking of mm->hmm is shifted to use the mmap_sem consistently for all read/write and unlocked accesses are removed. The use unlocked reads on 'hmm->dead' are also eliminated in favour of using standard mmget() locking to prevent the mm from being released. Many of the debugging checks of !range->hmm and !hmm->mm are dropped in favour of poison - which is much clearer as to the lifetime intent. The trailing patches are just some random cleanups I noticed when reviewing this code. This v2 incorporates alot of the good off list changes & feedback Jerome had, and all the on-list comments too. However, now that we have the shared git I have kept the one line change to nouveau_svm.c rather than the compat funtions. I believe we can resolve this merge in the DRM tree now and keep the core mm/hmm.c clean. DRM maintainers, please correct me if I'm wrong. It is on top of hmm.git, and I have a git tree of this series to ease testing here: https://github.com/jgunthorpe/linux/tree/hmm There are still some open locking issues, as I think this remains unaddressed: https://lore.kernel.org/linux-mm/20190527195829.gb18...@mellanox.com/ I'm looking for some more acks, reviews and tests so this can move ahead to hmm.git. Detailed notes on the v2 changes are in each patch. The big changes: - mmget is held so long as the range is registered - the last patch 'Remove confusing comment and logic from hmm_release' is new Thanks everyone, Jason Jason Gunthorpe (11): mm/hmm: fix use after free with struct hmm in the mmu notifiers mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register mm/hmm: Hold a mmgrab from hmm to mm mm/hmm: Simplify hmm_get_or_create and make it reliable mm/hmm: Remove duplicate condition test before wait_event_timeout mm/hmm: Hold on to the mmget for the lifetime of the range mm/hmm: Use lockdep instead of comments mm/hmm: Remove racy protection against double-unregistration mm/hmm: Poison hmm_range during unregister mm/hmm: Do not use list*_rcu() for hmm->ranges mm/hmm: Remove confusing comment and logic from hmm_release drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/hmm.h | 49 +-- kernel/fork.c | 1 - mm/hmm.c | 204 ++ 4 files changed, 87 insertions(+), 169 deletions(-) -- 2.21.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
From: Jason Gunthorpe
So long a a struct hmm pointer exists, so should the struct mm it is
linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
once the hmm refcount goes to zero.
Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
mm->hmm delete the hmm_hmm_destroy().
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
v2:
- Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
---
include/linux/hmm.h | 3 ---
kernel/fork.c | 1 -
mm/hmm.c| 22 --
3 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2d519797cb134a..4ee3acabe5ed22 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror *mirror,
}
/* Below are for HMM internal use only! Not to be used by device driver! */
-void hmm_mm_destroy(struct mm_struct *mm);
-
static inline void hmm_mm_init(struct mm_struct *mm)
{
mm->hmm = NULL;
}
#else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-static inline void hmm_mm_destroy(struct mm_struct *mm) {}
static inline void hmm_mm_init(struct mm_struct *mm) {}
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
diff --git a/kernel/fork.c b/kernel/fork.c
index b2b87d450b80b5..588c768ae72451 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
WARN_ON_ONCE(mm == current->active_mm);
mm_free_pgd(mm);
destroy_context(mm);
- hmm_mm_destroy(mm);
mmu_notifier_mm_destroy(mm);
check_mm(mm);
put_user_ns(mm->user_ns);
diff --git a/mm/hmm.c b/mm/hmm.c
index 8796447299023c..cc7c26fda3300e 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -82,6 +83,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
hmm->notifiers = 0;
hmm->dead = false;
hmm->mm = mm;
+ mmgrab(hmm->mm);
spin_lock(>page_table_lock);
if (!mm->hmm)
@@ -109,6 +111,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
mm->hmm = NULL;
spin_unlock(>page_table_lock);
error:
+ mmdrop(hmm->mm);
kfree(hmm);
return NULL;
}
@@ -130,6 +133,7 @@ static void hmm_free(struct kref *kref)
mm->hmm = NULL;
spin_unlock(>page_table_lock);
+ mmdrop(hmm->mm);
mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
}
@@ -138,24 +142,6 @@ static inline void hmm_put(struct hmm *hmm)
kref_put(>kref, hmm_free);
}
-void hmm_mm_destroy(struct mm_struct *mm)
-{
- struct hmm *hmm;
-
- spin_lock(>page_table_lock);
- hmm = mm_get_hmm(mm);
- mm->hmm = NULL;
- if (hmm) {
- hmm->mm = NULL;
- hmm->dead = true;
- spin_unlock(>page_table_lock);
- hmm_put(hmm);
- return;
- }
-
- spin_unlock(>page_table_lock);
-}
-
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
From: Jason Gunthorpe
mmu_notifier_unregister_no_release() is not a fence and the mmu_notifier
system will continue to reference hmm->mn until the srcu grace period
expires.
Resulting in use after free races like this:
CPU0 CPU1
__mmu_notifier_invalidate_range_start()
srcu_read_lock
hlist_for_each ()
// mn == hmm->mn
hmm_mirror_unregister()
hmm_put()
hmm_free()
mmu_notifier_unregister_no_release()
hlist_del_init_rcu(hmm-mn->list)
mn->ops->invalidate_range_start(mn, range);
mm_get_hmm()
mm->hmm = NULL;
kfree(hmm)
mutex_lock(>lock);
Use SRCU to kfree the hmm memory so that the notifiers can rely on hmm
existing. Get the now-safe hmm struct through container_of and directly
check kref_get_unless_zero to lock it against free.
Signed-off-by: Jason Gunthorpe
---
v2:
- Spell 'free' properly (Jerome/Ralph)
---
include/linux/hmm.h | 1 +
mm/hmm.c| 25 +++--
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 092f0234bfe917..688c5ca7068795 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -102,6 +102,7 @@ struct hmm {
struct mmu_notifier mmu_notifier;
struct rw_semaphore mirrors_sem;
wait_queue_head_t wq;
+ struct rcu_head rcu;
longnotifiers;
booldead;
};
diff --git a/mm/hmm.c b/mm/hmm.c
index 8e7403f081f44a..547002f56a163d 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -113,6 +113,11 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
return NULL;
}
+static void hmm_free_rcu(struct rcu_head *rcu)
+{
+ kfree(container_of(rcu, struct hmm, rcu));
+}
+
static void hmm_free(struct kref *kref)
{
struct hmm *hmm = container_of(kref, struct hmm, kref);
@@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
mm->hmm = NULL;
spin_unlock(>page_table_lock);
- kfree(hmm);
+ mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
}
static inline void hmm_put(struct hmm *hmm)
@@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
- struct hmm *hmm = mm_get_hmm(mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
struct hmm_range *range;
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(>kref))
+ return;
+
/* Report this HMM as dying. */
hmm->dead = true;
@@ -194,13 +203,15 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
static int hmm_invalidate_range_start(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
- struct hmm *hmm = mm_get_hmm(nrange->mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
struct hmm_update update;
struct hmm_range *range;
int ret = 0;
- VM_BUG_ON(!hmm);
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(>kref))
+ return 0;
update.start = nrange->start;
update.end = nrange->end;
@@ -245,9 +256,11 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
static void hmm_invalidate_range_end(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
- struct hmm *hmm = mm_get_hmm(nrange->mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
- VM_BUG_ON(!hmm);
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(>kref))
+ return;
mutex_lock(>lock);
hmm->notifiers--;
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 hmm 10/11] mm/hmm: Do not use list*_rcu() for hmm->ranges
From: Jason Gunthorpe This list is always read and written while holding hmm->lock so there is no need for the confusing _rcu annotations. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse --- mm/hmm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index c2fecb3ecb11e1..709d138dd49027 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -911,7 +911,7 @@ int hmm_range_register(struct hmm_range *range, mutex_lock(>lock); range->hmm = hmm; - list_add_rcu(>list, >ranges); + list_add(>list, >ranges); /* * If there are any concurrent notifiers we have to wait for them for @@ -941,7 +941,7 @@ void hmm_range_unregister(struct hmm_range *range) return; mutex_lock(>lock); - list_del_rcu(>list); + list_del(>list); mutex_unlock(>lock); /* Drop reference taken by hmm_range_register() */ -- 2.21.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 5/6] drm/amdkfd: Fix a circular lock dependency
The idea to break the circular lock dependency is to move allocate_mqd
out of dqm lock protection. See callstack #1 below.
[ 59.510149] [drm] Initialized amdgpu 3.30.0 20150101 for :04:00.0 on
minor 0
[ 513.604034] ==
[ 513.604205] WARNING: possible circular locking dependency detected
[ 513.604375] 4.18.0-kfd-root #2 Tainted: GW
[ 513.604530] --
[ 513.604699] kswapd0/611 is trying to acquire lock:
[ 513.604840] d254022e (>lock_hidden){+.+.}, at:
evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[ 513.605150]
but task is already holding lock:
[ 513.605307] 961547fc (_vma->rwsem){}, at:
page_lock_anon_vma_read+0xe4/0x250
[ 513.605540]
which lock already depends on the new lock.
[ 513.605747]
the existing dependency chain (in reverse order) is:
[ 513.605944]
-> #4 (_vma->rwsem){}:
[ 513.606106]__vma_adjust+0x147/0x7f0
[ 513.606231]__split_vma+0x179/0x190
[ 513.606353]mprotect_fixup+0x217/0x260
[ 513.606553]do_mprotect_pkey+0x211/0x380
[ 513.606752]__x64_sys_mprotect+0x1b/0x20
[ 513.606954]do_syscall_64+0x50/0x1a0
[ 513.607149]entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 513.607380]
-> #3 (>i_mmap_rwsem){}:
[ 513.607678]rmap_walk_file+0x1f0/0x280
[ 513.607887]page_referenced+0xdd/0x180
[ 513.608081]shrink_page_list+0x853/0xcb0
[ 513.608279]shrink_inactive_list+0x33b/0x700
[ 513.608483]shrink_node_memcg+0x37a/0x7f0
[ 513.608682]shrink_node+0xd8/0x490
[ 513.608869]balance_pgdat+0x18b/0x3b0
[ 513.609062]kswapd+0x203/0x5c0
[ 513.609241]kthread+0x100/0x140
[ 513.609420]ret_from_fork+0x24/0x30
[ 513.609607]
-> #2 (fs_reclaim){+.+.}:
[ 513.609883]kmem_cache_alloc_trace+0x34/0x2e0
[ 513.610093]reservation_object_reserve_shared+0x139/0x300
[ 513.610326]ttm_bo_init_reserved+0x291/0x480 [ttm]
[ 513.610567]amdgpu_bo_do_create+0x1d2/0x650 [amdgpu]
[ 513.610811]amdgpu_bo_create+0x40/0x1f0 [amdgpu]
[ 513.611041]amdgpu_bo_create_reserved+0x249/0x2d0 [amdgpu]
[ 513.611290]amdgpu_bo_create_kernel+0x12/0x70 [amdgpu]
[ 513.611584]amdgpu_ttm_init+0x2cb/0x560 [amdgpu]
[ 513.611823]gmc_v9_0_sw_init+0x400/0x750 [amdgpu]
[ 513.612491]amdgpu_device_init+0x14eb/0x1990 [amdgpu]
[ 513.612730]amdgpu_driver_load_kms+0x78/0x290 [amdgpu]
[ 513.612958]drm_dev_register+0x111/0x1a0
[ 513.613171]amdgpu_pci_probe+0x11c/0x1e0 [amdgpu]
[ 513.613389]local_pci_probe+0x3f/0x90
[ 513.613581]pci_device_probe+0x102/0x1c0
[ 513.613779]driver_probe_device+0x2a7/0x480
[ 513.613984]__driver_attach+0x10a/0x110
[ 513.614179]bus_for_each_dev+0x67/0xc0
[ 513.614372]bus_add_driver+0x1eb/0x260
[ 513.614565]driver_register+0x5b/0xe0
[ 513.614756]do_one_initcall+0xac/0x357
[ 513.614952]do_init_module+0x5b/0x213
[ 513.615145]load_module+0x2542/0x2d30
[ 513.615337]__do_sys_finit_module+0xd2/0x100
[ 513.615541]do_syscall_64+0x50/0x1a0
[ 513.615731]entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 513.615963]
-> #1 (reservation_ww_class_mutex){+.+.}:
[ 513.616293]amdgpu_amdkfd_alloc_gtt_mem+0xcf/0x2c0 [amdgpu]
[ 513.616554]init_mqd+0x223/0x260 [amdgpu]
[ 513.616779]create_queue_nocpsch+0x4d9/0x600 [amdgpu]
[ 513.617031]pqm_create_queue+0x37c/0x520 [amdgpu]
[ 513.617270]kfd_ioctl_create_queue+0x2f9/0x650 [amdgpu]
[ 513.617522]kfd_ioctl+0x202/0x350 [amdgpu]
[ 513.617724]do_vfs_ioctl+0x9f/0x6c0
[ 513.617914]ksys_ioctl+0x66/0x70
[ 513.618095]__x64_sys_ioctl+0x16/0x20
[ 513.618286]do_syscall_64+0x50/0x1a0
[ 513.618476]entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 513.618695]
-> #0 (>lock_hidden){+.+.}:
[ 513.618984]__mutex_lock+0x98/0x970
[ 513.619197]evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[ 513.619459]kfd_process_evict_queues+0x3b/0xb0 [amdgpu]
[ 513.619710]kgd2kfd_quiesce_mm+0x1c/0x40 [amdgpu]
[ 513.620103]amdgpu_amdkfd_evict_userptr+0x38/0x70 [amdgpu]
[ 513.620363]amdgpu_mn_invalidate_range_start_hsa+0xa6/0xc0 [amdgpu]
[ 513.620614]__mmu_notifier_invalidate_range_start+0x70/0xb0
[ 513.620851]try_to_unmap_one+0x7fc/0x8f0
[ 513.621049]rmap_walk_anon+0x121/0x290
[ 513.621242]try_to_unmap+0x93/0xf0
[ 513.621428]shrink_page_list+0x606/0xcb0
[ 513.621625]shrink_inactive_list+0x33b/0x700
[ 513.621835]shrink_node_memcg+0x37a/0x7f0
[ 513.622034]shrink_node+0xd8/0x490
[ 513.622219]
[PATCH 6/6] drm/amdkfd: Fix sdma queue allocate race condition
SDMA queue allocation requires the dqm lock at it modify
the global dqm members. Move up the dqm_lock so sdma
queue allocation is enclosed in the critical section. Move
mqd allocation out of critical section to avoid circular
lock dependency.
Change-Id: I96abd42eae6e77c82a5ba1b8e600af3efe8d791d
Signed-off-by: Oak Zeng
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 24 +++---
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 166636c..cd259b8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1133,23 +1133,27 @@ static int create_queue_cpsch(struct
device_queue_manager *dqm, struct queue *q,
if (dqm->total_queue_count >= max_num_of_queues_per_device) {
pr_warn("Can't create new usermode queue because %d queues were
already created\n",
dqm->total_queue_count);
- retval = -EPERM;
- goto out;
+ return -EPERM;
}
+ mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
+ q->properties.type)];
+ q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
+ if (!q->mqd_mem_obj)
+ return -ENOMEM;
+
+ dqm_lock(dqm);
if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
retval = allocate_sdma_queue(dqm, q);
if (retval)
- goto out;
+ goto out_unlock;
}
retval = allocate_doorbell(qpd, q);
if (retval)
goto out_deallocate_sdma_queue;
- mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
- q->properties.type)];
/*
* Eviction state logic: mark all queues as evicted, even ones
* not currently active. Restoring inactive queues later only
@@ -1161,12 +1165,8 @@ static int create_queue_cpsch(struct
device_queue_manager *dqm, struct queue *q,
dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
q->properties.tba_addr = qpd->tba_addr;
q->properties.tma_addr = qpd->tma_addr;
- q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
- if (!q->mqd_mem_obj)
- goto out_deallocate_doorbell;
mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
>gart_mqd_addr, >properties);
- dqm_lock(dqm);
list_add(>list, >queues_list);
qpd->queue_count++;
@@ -1192,13 +1192,13 @@ static int create_queue_cpsch(struct
device_queue_manager *dqm, struct queue *q,
dqm_unlock(dqm);
return retval;
-out_deallocate_doorbell:
- deallocate_doorbell(qpd, q);
out_deallocate_sdma_queue:
if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
deallocate_sdma_queue(dqm, q);
-out:
+out_unlock:
+ dqm_unlock(dqm);
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
return retval;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/6] drm/amdkfd: Separate mqd allocation and initialization
Introduce a new mqd allocation interface and split the original
init_mqd function into two functions: allocate_mqd and init_mqd.
Also renamed uninit_mqd to free_mqd. This is preparation work to
fix a circular lock dependency.
Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
Signed-off-by: Oak Zeng
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 34 -
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 16 ++--
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c | 4 +-
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 18 +++--
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 78 +++
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c| 78 +++
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c| 88 --
7 files changed, 124 insertions(+), 192 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 3c042eb..10d4f4f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -319,11 +319,11 @@ static int create_queue_nocpsch(struct
device_queue_manager *dqm,
if (retval)
goto out_deallocate_hqd;
- retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
- >gart_mqd_addr, >properties);
- if (retval)
+ q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
+ if (!q->mqd_mem_obj)
goto out_deallocate_doorbell;
-
+ mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
+ >gart_mqd_addr, >properties);
if (q->properties.is_active) {
if (WARN(q->process->mm != current->mm,
@@ -333,7 +333,7 @@ static int create_queue_nocpsch(struct device_queue_manager
*dqm,
retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
q->queue, >properties, current->mm);
if (retval)
- goto out_uninit_mqd;
+ goto out_free_mqd;
}
list_add(>list, >queues_list);
@@ -355,8 +355,8 @@ static int create_queue_nocpsch(struct device_queue_manager
*dqm,
dqm->total_queue_count);
goto out_unlock;
-out_uninit_mqd:
- mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+out_free_mqd:
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
out_deallocate_doorbell:
deallocate_doorbell(qpd, q);
out_deallocate_hqd:
@@ -450,7 +450,7 @@ static int destroy_queue_nocpsch_locked(struct
device_queue_manager *dqm,
if (retval == -ETIME)
qpd->reset_wavefronts = true;
- mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
list_del(>list);
if (list_empty(>queues_list)) {
@@ -527,7 +527,7 @@ static int update_queue(struct device_queue_manager *dqm,
struct queue *q)
}
}
- retval = mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
+ mqd_mgr->update_mqd(mqd_mgr, q->mqd, >properties);
/*
* check active state vs. the previous state and modify
@@ -1160,11 +1160,11 @@ static int create_queue_cpsch(struct
device_queue_manager *dqm, struct queue *q,
dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
q->properties.tba_addr = qpd->tba_addr;
q->properties.tma_addr = qpd->tma_addr;
- retval = mqd_mgr->init_mqd(mqd_mgr, >mqd, >mqd_mem_obj,
- >gart_mqd_addr, >properties);
- if (retval)
+ q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, >properties);
+ if (!q->mqd_mem_obj)
goto out_deallocate_doorbell;
-
+ mqd_mgr->init_mqd(mqd_mgr, >mqd, q->mqd_mem_obj,
+ >gart_mqd_addr, >properties);
dqm_lock(dqm);
list_add(>list, >queues_list);
@@ -1373,8 +1373,8 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
dqm_unlock(dqm);
- /* Do uninit_mqd after dqm_unlock(dqm) to avoid circular locking */
- mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+ /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
+ mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
return retval;
@@ -1615,14 +1615,14 @@ static int process_termination_cpsch(struct
device_queue_manager *dqm,
kfd_dec_compute_active(dqm->dev);
/* Lastly, free mqd resources.
-* Do uninit_mqd() after dqm_unlock to avoid circular locking.
+* Do free_mqd() after dqm_unlock to avoid circular locking.
*/
list_for_each_entry_safe(q, next, >queues_list, list) {
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
Re: [PATCH 2/2] drm/amd/display: Set default ABM level to module parameter
Series is Reviewed-by: David Francis From: amd-gfx on behalf of Nicholas Kazlauskas Sent: June 6, 2019 9:02:13 AM To: amd-gfx@lists.freedesktop.org Cc: Francis, David; Wentland, Harry; Kazlauskas, Nicholas Subject: [PATCH 2/2] drm/amd/display: Set default ABM level to module parameter [Why] The module parameter to specify the default ABM level is now defined, so hook it up in DM. [How] On connector reset specify the default level. DC will program this as part of the modeset since it gets passed onto the stream in dm_update_crtc_state. It's only set for eDP connectors, but it doesn't matter if this is specified for connectors or hardware that doesn't support ABM. It's DC's responsibility to check that ABM can be set or adjusted, and DC does check that the DMCU firmware is running and if there's backlight control available. Cc: Harry Wentland Cc: David Francis Signed-off-by: Nicholas Kazlauskas --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index a698c8f272ed..f0c216d78a07 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3821,6 +3821,9 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->underscan_hborder = 0; state->underscan_vborder = 0; + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) + state->abm_level = amdgpu_dm_abm_level; + __drm_atomic_helper_connector_reset(connector, >base); } } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 0/2] Two bug-fixes for HMM
On Fri, May 10, 2019 at 07:53:21PM +, Kuehling, Felix wrote: > These problems were found in AMD-internal testing as we're working on > adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get > them applied to a mainline Linux kernel as well as drm-next and > amd-staging-drm-next sooner rather than later. > > Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, > but the driver changes for HMM are expected to land in 5.2 and will need to > be rebased on those HMM changes. > > I'd like to work out a flow between Jerome, Dave, Alex and myself that > allows us to test the latest version of HMM on amd-staging-drm-next so > that ideally everything comes together in master without much need for > rebasing and retesting. I think we have that now, I'm running a hmm.git that is clean and can be used for merging into DRM related trees (and RDMA). I've commited to send this tree to Linus at the start of the merge window. See here: https://lore.kernel.org/lkml/20190524124455.gb16...@ziepe.ca/ The tree is here: https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm However please consult with me before making a merge commit to be co-ordinated. Thanks I see in this thread that AMDGPU missed 5.2 beacause of the co-ordination problems this tree is intended to solve, so I'm very hopeful this will help your work move into 5.3! > Maybe having Jerome's latest HMM changes in drm-next. However, that may > create dependencies where Jerome and Dave need to coordinate their pull- > requests for master. > > Felix Kuehling (1): > mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking > > Philip Yang (1): > mm/hmm: support automatic NUMA balancing I've applied both of these patches with Jerome's Reviewed-by to hmm.git and added the missed Signed-off-by If you test and confirm I think this branch would be ready for merging toward the AMD tree. Regards, Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: FW: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload issue
It's upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bd4264112f93045704731850c5e4d85db981cd85
and in drm-next:
https://cgit.freedesktop.org/drm/drm/commit/?id=bd4264112f93045704731850c5e4d85db981cd85
and in amd-staging-drm-next:
https://cgit.freedesktop.org/~agd5f/linux/commit/?h=amd-staging-drm-next=bd4264112f93045704731850c5e4d85db981cd85
Where are you seeing it missing?
Alex
From: Christian K?nig
Sent: Thursday, June 6, 2019 10:00 AM
To: Koenig, Christian; Liu, Monk; amd-gfx@lists.freedesktop.org; Deucher,
Alexander
Subject: Re: FW: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload
issue
It is part of amd-staging-drm-next and has Alex Signed-of by tag.
So it should definitely be upstream, Alex any idea why that patch isn't
in drm-next?
Christian.
Am 05.06.19 um 20:10 schrieb Koenig, Christian:
> Mhm, looks like that somehow got dropped during rebase.
>
> Going to dig up where that actually ended up tomorrow.
>
> Christian.
>
> Am 05.06.19 um 16:44 schrieb Liu, Monk:
>> Strange, I get the latest "drm-next" branch and didn't see the change landed
>>
>>
>> /Monk
>>
>> -Original Message-
>> From: Koenig, Christian
>> Sent: Wednesday, June 5, 2019 7:11 PM
>> To: Liu, Monk ; amd-gfx@lists.freedesktop.org
>> Subject: Re: FW: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload
>> issue
>>
>> This should already be fixed by patch "drm/ttm: fix re-init of global
>> structures".
>>
>> Christian.
>>
>> Am 05.06.19 um 09:29 schrieb Liu, Monk:
>>> -Original Message-
>>> From: Monk Liu
>>> Sent: Wednesday, June 5, 2019 2:45 PM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Liu, Monk
>>> Subject: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload
>>> issue
>>>
>>> need to clear bo glob and mem glob during their release otherwise their
>>> member value would be wrongly used in the next glob init stage and lead to
>>> wild pointer access problems:
>>>
>>> 1) kobj.state_initialized is 1
>>> 2) ttm_bo_glob.bo_count isn't cleared and referenced via it
>>> on member "swap_lru" would hit out of bound array accessing
>>> bug
>>>
>>> Signed-off-by: Monk Liu
>>> ---
>>> drivers/gpu/drm/ttm/ttm_bo.c | 2 ++
>>> drivers/gpu/drm/ttm/ttm_memory.c | 8
>>> 2 files changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>> b/drivers/gpu/drm/ttm/ttm_bo.c index c7de667..6434eac 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -1604,6 +1604,8 @@ static void ttm_bo_global_kobj_release(struct kobject
>>> *kobj)
>>> container_of(kobj, struct ttm_bo_global, kobj);
>>>
>>> __free_page(glob->dummy_read_page);
>>> +
>>> + memset(glob, 0, sizeof(*glob));
>>> }
>>>
>>> static void ttm_bo_global_release(void) diff --git
>>> a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
>>> index 8617958..7128bbf 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>> @@ -229,9 +229,17 @@ static const struct sysfs_ops ttm_mem_global_ops = {
>>> .store = _mem_global_store,
>>> };
>>>
>>> +void ttm_mem_glob_kobj_release(struct kobject *kobj) {
>>> + struct ttm_mem_global *glob = container_of(kobj, struct
>>> +ttm_mem_global, kobj);
>>> +
>>> + memset(glob, 0, sizeof(*glob));
>>> +}
>>> +
>>> static struct kobj_type ttm_mem_glob_kobj_type = {
>>> .sysfs_ops = _mem_global_ops,
>>> .default_attrs = ttm_mem_global_attrs,
>>> + .release = ttm_mem_glob_kobj_release,
>>> };
>>>
>>> static bool ttm_zones_above_swap_target(struct ttm_mem_global *glob,
>>> --
>>> 2.7.4
>>>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: FW: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload issue
It is part of amd-staging-drm-next and has Alex Signed-of by tag.
So it should definitely be upstream, Alex any idea why that patch isn't
in drm-next?
Christian.
Am 05.06.19 um 20:10 schrieb Koenig, Christian:
Mhm, looks like that somehow got dropped during rebase.
Going to dig up where that actually ended up tomorrow.
Christian.
Am 05.06.19 um 16:44 schrieb Liu, Monk:
Strange, I get the latest "drm-next" branch and didn't see the change landed
/Monk
-Original Message-
From: Koenig, Christian
Sent: Wednesday, June 5, 2019 7:11 PM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: FW: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload
issue
This should already be fixed by patch "drm/ttm: fix re-init of global
structures".
Christian.
Am 05.06.19 um 09:29 schrieb Liu, Monk:
-Original Message-
From: Monk Liu
Sent: Wednesday, June 5, 2019 2:45 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Monk
Subject: [PATCH] drm/ttm: fix ttm client driver (e.g. amdgpu) reload
issue
need to clear bo glob and mem glob during their release otherwise their member
value would be wrongly used in the next glob init stage and lead to wild
pointer access problems:
1) kobj.state_initialized is 1
2) ttm_bo_glob.bo_count isn't cleared and referenced via it
on member "swap_lru" would hit out of bound array accessing
bug
Signed-off-by: Monk Liu
---
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++
drivers/gpu/drm/ttm/ttm_memory.c | 8
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c index c7de667..6434eac 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1604,6 +1604,8 @@ static void ttm_bo_global_kobj_release(struct kobject
*kobj)
container_of(kobj, struct ttm_bo_global, kobj);
__free_page(glob->dummy_read_page);
+
+ memset(glob, 0, sizeof(*glob));
}
static void ttm_bo_global_release(void) diff --git
a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 8617958..7128bbf 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -229,9 +229,17 @@ static const struct sysfs_ops ttm_mem_global_ops = {
.store = _mem_global_store,
};
+void ttm_mem_glob_kobj_release(struct kobject *kobj) {
+ struct ttm_mem_global *glob = container_of(kobj, struct
+ttm_mem_global, kobj);
+
+ memset(glob, 0, sizeof(*glob));
+}
+
static struct kobj_type ttm_mem_glob_kobj_type = {
.sysfs_ops = _mem_global_ops,
.default_attrs = ttm_mem_global_attrs,
+ .release = ttm_mem_glob_kobj_release,
};
static bool ttm_zones_above_swap_target(struct ttm_mem_global *glob,
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amd/display: Set default ABM level to module parameter
[Why] The module parameter to specify the default ABM level is now defined, so hook it up in DM. [How] On connector reset specify the default level. DC will program this as part of the modeset since it gets passed onto the stream in dm_update_crtc_state. It's only set for eDP connectors, but it doesn't matter if this is specified for connectors or hardware that doesn't support ABM. It's DC's responsibility to check that ABM can be set or adjusted, and DC does check that the DMCU firmware is running and if there's backlight control available. Cc: Harry Wentland Cc: David Francis Signed-off-by: Nicholas Kazlauskas --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index a698c8f272ed..f0c216d78a07 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3821,6 +3821,9 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->underscan_hborder = 0; state->underscan_vborder = 0; + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) + state->abm_level = amdgpu_dm_abm_level; + __drm_atomic_helper_connector_reset(connector, >base); } } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amd/amdgpu: Add module parameter for specifying default ABM level
[Why]
It's non trivial to configure or specify an ABM reduction level for
userspace outside of X. There is also no method to specify the default
ABM value at boot time.
A parameter should be added to configure this.
[How]
Expose a module parameter that can specify the default ABM level to
use for eDP connectors on DC enabled hardware that loads the DMCU
firmware.
The default is still disabled (0), but levels can range from 1-4. Levels
control how much the backlight can be reduced, with being the least
amount of reduction and four being the most reduction.
Signed-off-by: Nicholas Kazlauskas
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 16
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 19a00282e34c..4beb26738e1e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -155,6 +155,7 @@ extern int amdgpu_gpu_recovery;
extern int amdgpu_emu_mode;
extern uint amdgpu_smu_memory_pool_size;
extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_dm_abm_level;
extern struct amdgpu_mgpu_info mgpu_info;
extern int amdgpu_ras_enable;
extern uint amdgpu_ras_mask;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1f38d6fc1fe3..dde53f8e230b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -686,6 +686,22 @@ MODULE_PARM_DESC(hws_gws_support, "MEC FW support gws
barriers (false = not supp
MODULE_PARM_DESC(dcfeaturemask, "all stable DC features enabled (default))");
module_param_named(dcfeaturemask, amdgpu_dc_feature_mask, uint, 0444);
+/**
+ * DOC: abmlevel (uint)
+ * Override the default ABM (Adaptive Backlight Management) level used for DC
+ * enabled hardware. Requires DMCU to be supported and loaded.
+ * Valid levels are 0-4. A value of 0 indicates that ABM should be disabled by
+ * default. Values 1-4 control the maximum allowable brightness reduction via
+ * the ABM algorithm, with 1 being the least reduction and 4 being the most
+ * reduction.
+ *
+ * Defaults to 0, or disabled. Userspace can still override this level later
+ * after boot.
+ */
+uint amdgpu_dm_abm_level = 0;
+MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight
reduction level) ");
+module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
+
static const struct pci_device_id pciidlist[] = {
#ifdef CONFIG_DRM_AMDGPU_SI
{0x1002, 0x6780, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TAHITI},
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v3)
Am 06.06.19 um 13:51 schrieb StDenis, Tom:
On 2019-06-06 7:49 a.m., Christian König wrote:
Am 06.06.19 um 12:50 schrieb StDenis, Tom:
This option is no longer needed. The default code paths
are now the only option.
v2: Add HPAGE support and a default for non contiguous maps
v3: Misread 512 pages as MiB ...
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
4 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 19a00282e34c..e54f31865f60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
extern char *amdgpu_disable_cu;
extern char *amdgpu_virtual_display;
extern uint amdgpu_pp_feature_mask;
-extern int amdgpu_vram_page_split;
extern int amdgpu_ngg;
extern int amdgpu_prim_buf_per_se;
extern int amdgpu_pos_buf_per_se;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0212c9ee317c..2e13b8ef6681 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
amdgpu_device *adev)
amdgpu_device_check_block_size(adev);
- if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split <
16 ||
- !is_power_of_2(amdgpu_vram_page_split))) {
- dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
- amdgpu_vram_page_split);
- amdgpu_vram_page_split = 1024;
- }
-
ret = amdgpu_device_get_job_timeout_settings(adev);
if (ret) {
dev_err(adev->dev, "invalid lockup_timeout parameter
syntax\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1f38d6fc1fe3..ef22a2a25448 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
int amdgpu_vm_block_size = -1;
int amdgpu_vm_fault_stop = 0;
int amdgpu_vm_debug = 0;
-int amdgpu_vram_page_split = 512;
int amdgpu_vm_update_mode = -1;
int amdgpu_exp_hw_support = 0;
int amdgpu_dc = -1;
@@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug,
int, 0644);
MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never
(default except for large BAR(LB)), 1 = Graphics only, 2 = Compute
only (default for LB), 3 = Both");
module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
-/**
- * DOC: vram_page_split (int)
- * Override the number of pages after we split VRAM allocations
(default 512, -1 = disable). The default is 512.
- */
-MODULE_PARM_DESC(vram_page_split, "Number of pages after we split
VRAM allocations (default 512, -1 = disable)");
-module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
-
/**
* DOC: exp_hw_support (int)
* Enable experimental hw support (1 = enable). The default is 0
(disabled).
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index ec9ea3fdbb4a..8aea2f21b202 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
ttm_mem_type_manager *man,
if (!lpfn)
lpfn = man->size;
- if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
- amdgpu_vram_page_split == -1) {
+ if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
} else {
- pages_per_node = max((uint32_t)amdgpu_vram_page_split,
- mem->page_alignment);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ pages_per_node = HPAGE_PMD_NR;
+#else
+ /* default to 2MB */
+ pages_per_node = (2UL << (20UL - PAGE_SHIFT));
+#endif
+ pages_per_node = max((uint32_t)pages_per_node,
mem->page_alignment);
num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
}
- nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
+ nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
You can probably drop that cast here, apart from that the patch is
Reviewed-by: Christian König .
Without the cast it produces a somewhat lengthy warning diagnostic. The
original code had the cast as well.
Strange, the original code hat the case because amdgpu_vram_page_split
was a signed int, but num_nodes should be unsigned anyway.
So that shouldn't be necessary any more. Anyway keeping the cast works
for me as well.
Do you want to look into test/benchmark as well?
Sure, is the plan to cleave all that out too?
Well first of all dig a bit
Re: [PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v3)
On 2019-06-06 7:49 a.m., Christian König wrote:
> Am 06.06.19 um 12:50 schrieb StDenis, Tom:
>> This option is no longer needed. The default code paths
>> are now the only option.
>>
>> v2: Add HPAGE support and a default for non contiguous maps
>> v3: Misread 512 pages as MiB ...
>>
>> Signed-off-by: Tom St Denis
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
>> 4 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 19a00282e34c..e54f31865f60 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
>> extern char *amdgpu_disable_cu;
>> extern char *amdgpu_virtual_display;
>> extern uint amdgpu_pp_feature_mask;
>> -extern int amdgpu_vram_page_split;
>> extern int amdgpu_ngg;
>> extern int amdgpu_prim_buf_per_se;
>> extern int amdgpu_pos_buf_per_se;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 0212c9ee317c..2e13b8ef6681 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
>> amdgpu_device *adev)
>> amdgpu_device_check_block_size(adev);
>> - if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split <
>> 16 ||
>> - !is_power_of_2(amdgpu_vram_page_split))) {
>> - dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
>> - amdgpu_vram_page_split);
>> - amdgpu_vram_page_split = 1024;
>> - }
>> -
>> ret = amdgpu_device_get_job_timeout_settings(adev);
>> if (ret) {
>> dev_err(adev->dev, "invalid lockup_timeout parameter
>> syntax\n");
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 1f38d6fc1fe3..ef22a2a25448 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
>> int amdgpu_vm_block_size = -1;
>> int amdgpu_vm_fault_stop = 0;
>> int amdgpu_vm_debug = 0;
>> -int amdgpu_vram_page_split = 512;
>> int amdgpu_vm_update_mode = -1;
>> int amdgpu_exp_hw_support = 0;
>> int amdgpu_dc = -1;
>> @@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug,
>> int, 0644);
>> MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never
>> (default except for large BAR(LB)), 1 = Graphics only, 2 = Compute
>> only (default for LB), 3 = Both");
>> module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
>> -/**
>> - * DOC: vram_page_split (int)
>> - * Override the number of pages after we split VRAM allocations
>> (default 512, -1 = disable). The default is 512.
>> - */
>> -MODULE_PARM_DESC(vram_page_split, "Number of pages after we split
>> VRAM allocations (default 512, -1 = disable)");
>> -module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
>> -
>> /**
>> * DOC: exp_hw_support (int)
>> * Enable experimental hw support (1 = enable). The default is 0
>> (disabled).
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index ec9ea3fdbb4a..8aea2f21b202 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
>> ttm_mem_type_manager *man,
>> if (!lpfn)
>> lpfn = man->size;
>> - if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
>> - amdgpu_vram_page_split == -1) {
>> + if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>> pages_per_node = ~0ul;
>> num_nodes = 1;
>> } else {
>> - pages_per_node = max((uint32_t)amdgpu_vram_page_split,
>> - mem->page_alignment);
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> + pages_per_node = HPAGE_PMD_NR;
>> +#else
>> + /* default to 2MB */
>> + pages_per_node = (2UL << (20UL - PAGE_SHIFT));
>> +#endif
>> + pages_per_node = max((uint32_t)pages_per_node,
>> mem->page_alignment);
>> num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
>> }
>> - nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
>> + nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
>
> You can probably drop that cast here, apart from that the patch is
> Reviewed-by: Christian König .
Without the cast it produces a somewhat lengthy warning diagnostic. The
original code had the cast as well.
>
> Do you want to look into test/benchmark as well?
Sure, is the plan to cleave all that
Re: [PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v3)
Am 06.06.19 um 12:50 schrieb StDenis, Tom:
This option is no longer needed. The default code paths
are now the only option.
v2: Add HPAGE support and a default for non contiguous maps
v3: Misread 512 pages as MiB ...
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
4 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 19a00282e34c..e54f31865f60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
extern char *amdgpu_disable_cu;
extern char *amdgpu_virtual_display;
extern uint amdgpu_pp_feature_mask;
-extern int amdgpu_vram_page_split;
extern int amdgpu_ngg;
extern int amdgpu_prim_buf_per_se;
extern int amdgpu_pos_buf_per_se;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0212c9ee317c..2e13b8ef6681 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
amdgpu_device *adev)
amdgpu_device_check_block_size(adev);
- if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split < 16 ||
- !is_power_of_2(amdgpu_vram_page_split))) {
- dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
-amdgpu_vram_page_split);
- amdgpu_vram_page_split = 1024;
- }
-
ret = amdgpu_device_get_job_timeout_settings(adev);
if (ret) {
dev_err(adev->dev, "invalid lockup_timeout parameter syntax\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1f38d6fc1fe3..ef22a2a25448 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
int amdgpu_vm_block_size = -1;
int amdgpu_vm_fault_stop = 0;
int amdgpu_vm_debug = 0;
-int amdgpu_vram_page_split = 512;
int amdgpu_vm_update_mode = -1;
int amdgpu_exp_hw_support = 0;
int amdgpu_dc = -1;
@@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug, int, 0644);
MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never (default except
for large BAR(LB)), 1 = Graphics only, 2 = Compute only (default for LB), 3 = Both");
module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
-/**
- * DOC: vram_page_split (int)
- * Override the number of pages after we split VRAM allocations (default 512,
-1 = disable). The default is 512.
- */
-MODULE_PARM_DESC(vram_page_split, "Number of pages after we split VRAM allocations
(default 512, -1 = disable)");
-module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
-
/**
* DOC: exp_hw_support (int)
* Enable experimental hw support (1 = enable). The default is 0 (disabled).
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index ec9ea3fdbb4a..8aea2f21b202 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
ttm_mem_type_manager *man,
if (!lpfn)
lpfn = man->size;
- if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
- amdgpu_vram_page_split == -1) {
+ if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
} else {
- pages_per_node = max((uint32_t)amdgpu_vram_page_split,
-mem->page_alignment);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ pages_per_node = HPAGE_PMD_NR;
+#else
+ /* default to 2MB */
+ pages_per_node = (2UL << (20UL - PAGE_SHIFT));
+#endif
+ pages_per_node = max((uint32_t)pages_per_node,
mem->page_alignment);
num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
}
- nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
+ nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
You can probably drop that cast here, apart from that the patch is
Reviewed-by: Christian König .
Do you want to look into test/benchmark as well?
Thanks,
Christian.
GFP_KERNEL | __GFP_ZERO);
if (!nodes)
return -ENOMEM;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v2)
Ah ya I misread the original default as MiB instead of pages.
Tom
On 2019-06-06 6:35 a.m., Christian König wrote:
> Am 04.06.19 um 19:15 schrieb StDenis, Tom:
>> This option is no longer needed. The default code paths
>> are now the only option.
>>
>> v2: Add HPAGE support and a default for non contiguous maps
>>
>> Signed-off-by: Tom St Denis
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
>> 4 files changed, 9 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 19a00282e34c..e54f31865f60 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
>> extern char *amdgpu_disable_cu;
>> extern char *amdgpu_virtual_display;
>> extern uint amdgpu_pp_feature_mask;
>> -extern int amdgpu_vram_page_split;
>> extern int amdgpu_ngg;
>> extern int amdgpu_prim_buf_per_se;
>> extern int amdgpu_pos_buf_per_se;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index d00fd5dd307a..ef7d99ebe92d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
>> amdgpu_device *adev)
>> amdgpu_device_check_block_size(adev);
>> - if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split <
>> 16 ||
>> - !is_power_of_2(amdgpu_vram_page_split))) {
>> - dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
>> - amdgpu_vram_page_split);
>> - amdgpu_vram_page_split = 1024;
>> - }
>> -
>> ret = amdgpu_device_get_job_timeout_settings(adev);
>> if (ret) {
>> dev_err(adev->dev, "invalid lockup_timeout parameter
>> syntax\n");
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 1f38d6fc1fe3..ef22a2a25448 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
>> int amdgpu_vm_block_size = -1;
>> int amdgpu_vm_fault_stop = 0;
>> int amdgpu_vm_debug = 0;
>> -int amdgpu_vram_page_split = 512;
>> int amdgpu_vm_update_mode = -1;
>> int amdgpu_exp_hw_support = 0;
>> int amdgpu_dc = -1;
>> @@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug,
>> int, 0644);
>> MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never
>> (default except for large BAR(LB)), 1 = Graphics only, 2 = Compute
>> only (default for LB), 3 = Both");
>> module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
>> -/**
>> - * DOC: vram_page_split (int)
>> - * Override the number of pages after we split VRAM allocations
>> (default 512, -1 = disable). The default is 512.
>> - */
>> -MODULE_PARM_DESC(vram_page_split, "Number of pages after we split
>> VRAM allocations (default 512, -1 = disable)");
>> -module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
>> -
>> /**
>> * DOC: exp_hw_support (int)
>> * Enable experimental hw support (1 = enable). The default is 0
>> (disabled).
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index ec9ea3fdbb4a..0bc01e25a0b4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
>> ttm_mem_type_manager *man,
>> if (!lpfn)
>> lpfn = man->size;
>> - if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
>> - amdgpu_vram_page_split == -1) {
>> + if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>> pages_per_node = ~0ul;
>> num_nodes = 1;
>> } else {
>> - pages_per_node = max((uint32_t)amdgpu_vram_page_split,
>> - mem->page_alignment);
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> + pages_per_node = HPAGE_PMD_NR;
>> +#else
>> + /* default to 512MB */
>> + pages_per_node = (512UL << (20UL - PAGE_SHIFT));
>
> That is way to large, the fallback should be only 2MB (512 pages).
>
> Christian.
>
>> +#endif
>> + pages_per_node = max((uint32_t)pages_per_node,
>> mem->page_alignment);
>> num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
>> }
>> - nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
>> + nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
>> GFP_KERNEL | __GFP_ZERO);
>> if (!nodes)
>> return -ENOMEM;
>
___
amd-gfx mailing list
[PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v3)
This option is no longer needed. The default code paths
are now the only option.
v2: Add HPAGE support and a default for non contiguous maps
v3: Misread 512 pages as MiB ...
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
4 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 19a00282e34c..e54f31865f60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
extern char *amdgpu_disable_cu;
extern char *amdgpu_virtual_display;
extern uint amdgpu_pp_feature_mask;
-extern int amdgpu_vram_page_split;
extern int amdgpu_ngg;
extern int amdgpu_prim_buf_per_se;
extern int amdgpu_pos_buf_per_se;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0212c9ee317c..2e13b8ef6681 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
amdgpu_device *adev)
amdgpu_device_check_block_size(adev);
- if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split < 16 ||
- !is_power_of_2(amdgpu_vram_page_split))) {
- dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
-amdgpu_vram_page_split);
- amdgpu_vram_page_split = 1024;
- }
-
ret = amdgpu_device_get_job_timeout_settings(adev);
if (ret) {
dev_err(adev->dev, "invalid lockup_timeout parameter syntax\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1f38d6fc1fe3..ef22a2a25448 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
int amdgpu_vm_block_size = -1;
int amdgpu_vm_fault_stop = 0;
int amdgpu_vm_debug = 0;
-int amdgpu_vram_page_split = 512;
int amdgpu_vm_update_mode = -1;
int amdgpu_exp_hw_support = 0;
int amdgpu_dc = -1;
@@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug, int, 0644);
MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never (default
except for large BAR(LB)), 1 = Graphics only, 2 = Compute only (default for
LB), 3 = Both");
module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
-/**
- * DOC: vram_page_split (int)
- * Override the number of pages after we split VRAM allocations (default 512,
-1 = disable). The default is 512.
- */
-MODULE_PARM_DESC(vram_page_split, "Number of pages after we split VRAM
allocations (default 512, -1 = disable)");
-module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
-
/**
* DOC: exp_hw_support (int)
* Enable experimental hw support (1 = enable). The default is 0 (disabled).
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index ec9ea3fdbb4a..8aea2f21b202 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
ttm_mem_type_manager *man,
if (!lpfn)
lpfn = man->size;
- if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
- amdgpu_vram_page_split == -1) {
+ if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
} else {
- pages_per_node = max((uint32_t)amdgpu_vram_page_split,
-mem->page_alignment);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ pages_per_node = HPAGE_PMD_NR;
+#else
+ /* default to 2MB */
+ pages_per_node = (2UL << (20UL - PAGE_SHIFT));
+#endif
+ pages_per_node = max((uint32_t)pages_per_node,
mem->page_alignment);
num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
}
- nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
+ nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
GFP_KERNEL | __GFP_ZERO);
if (!nodes)
return -ENOMEM;
--
2.21.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v2)
Am 04.06.19 um 19:15 schrieb StDenis, Tom:
This option is no longer needed. The default code paths
are now the only option.
v2: Add HPAGE support and a default for non contiguous maps
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
4 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 19a00282e34c..e54f31865f60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
extern char *amdgpu_disable_cu;
extern char *amdgpu_virtual_display;
extern uint amdgpu_pp_feature_mask;
-extern int amdgpu_vram_page_split;
extern int amdgpu_ngg;
extern int amdgpu_prim_buf_per_se;
extern int amdgpu_pos_buf_per_se;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index d00fd5dd307a..ef7d99ebe92d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
amdgpu_device *adev)
amdgpu_device_check_block_size(adev);
- if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split < 16 ||
- !is_power_of_2(amdgpu_vram_page_split))) {
- dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
-amdgpu_vram_page_split);
- amdgpu_vram_page_split = 1024;
- }
-
ret = amdgpu_device_get_job_timeout_settings(adev);
if (ret) {
dev_err(adev->dev, "invalid lockup_timeout parameter syntax\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1f38d6fc1fe3..ef22a2a25448 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
int amdgpu_vm_block_size = -1;
int amdgpu_vm_fault_stop = 0;
int amdgpu_vm_debug = 0;
-int amdgpu_vram_page_split = 512;
int amdgpu_vm_update_mode = -1;
int amdgpu_exp_hw_support = 0;
int amdgpu_dc = -1;
@@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug, int, 0644);
MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never (default except
for large BAR(LB)), 1 = Graphics only, 2 = Compute only (default for LB), 3 = Both");
module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
-/**
- * DOC: vram_page_split (int)
- * Override the number of pages after we split VRAM allocations (default 512,
-1 = disable). The default is 512.
- */
-MODULE_PARM_DESC(vram_page_split, "Number of pages after we split VRAM allocations
(default 512, -1 = disable)");
-module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
-
/**
* DOC: exp_hw_support (int)
* Enable experimental hw support (1 = enable). The default is 0 (disabled).
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index ec9ea3fdbb4a..0bc01e25a0b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
ttm_mem_type_manager *man,
if (!lpfn)
lpfn = man->size;
- if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
- amdgpu_vram_page_split == -1) {
+ if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
} else {
- pages_per_node = max((uint32_t)amdgpu_vram_page_split,
-mem->page_alignment);
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ pages_per_node = HPAGE_PMD_NR;
+#else
+ /* default to 512MB */
+ pages_per_node = (512UL << (20UL - PAGE_SHIFT));
That is way to large, the fallback should be only 2MB (512 pages).
Christian.
+#endif
+ pages_per_node = max((uint32_t)pages_per_node,
mem->page_alignment);
num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
}
- nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
+ nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
GFP_KERNEL | __GFP_ZERO);
if (!nodes)
return -ENOMEM;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH][next] drm/amd/display: remove redundant assignment to status
On Fri, May 31, 2019 at 08:19:03PM +, Harry Wentland wrote:
> On 2019-05-30 12:12 p.m., Colin King wrote:
> > From: Colin Ian King
> >
> > The variable status is initialized with a value that is never read
> > and status is reassigned several statements later. This initialization
> > is redundant and can be removed.
> >
> > Addresses-Coverity: ("Unused value")
> > Signed-off-by: Colin Ian King
> > ---
> > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > index 65d6caedbd82..cf6166a1be53 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > @@ -2367,7 +2367,7 @@ static bool retrieve_link_cap(struct dc_link *link)
> > union down_stream_port_count down_strm_port_count;
> > union edp_configuration_cap edp_config_cap;
> > union dp_downstream_port_present ds_port = { 0 };
> > - enum dc_status status = DC_ERROR_UNEXPECTED;
> > + enum dc_status status;
>
> Not sure this improves the situation.
>
> I'd prefer to have a default here in case someone changes the code below
> and forgets to set the status.
The dead code confuses human readers, because people naturally assume it
is not dead.
GCC has a feature to warn about uninitialized variables and we're
randomly initializing status to a bogus value to disable static
analysis...
regards,
dan carpenter
Re: [PATCH] drm/amd/amdgpu: remove vram_page_split kernel option (v2)
ping?
On Tue, Jun 4, 2019 at 1:15 PM StDenis, Tom wrote:
> This option is no longer needed. The default code paths
> are now the only option.
>
> v2: Add HPAGE support and a default for non contiguous maps
>
> Signed-off-by: Tom St Denis
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 14 +-
> 4 files changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 19a00282e34c..e54f31865f60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -142,7 +142,6 @@ extern uint amdgpu_sdma_phase_quantum;
> extern char *amdgpu_disable_cu;
> extern char *amdgpu_virtual_display;
> extern uint amdgpu_pp_feature_mask;
> -extern int amdgpu_vram_page_split;
> extern int amdgpu_ngg;
> extern int amdgpu_prim_buf_per_se;
> extern int amdgpu_pos_buf_per_se;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index d00fd5dd307a..ef7d99ebe92d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -973,13 +973,6 @@ static int amdgpu_device_check_arguments(struct
> amdgpu_device *adev)
>
> amdgpu_device_check_block_size(adev);
>
> - if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split < 16 ||
> - !is_power_of_2(amdgpu_vram_page_split))) {
> - dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
> -amdgpu_vram_page_split);
> - amdgpu_vram_page_split = 1024;
> - }
> -
> ret = amdgpu_device_get_job_timeout_settings(adev);
> if (ret) {
> dev_err(adev->dev, "invalid lockup_timeout parameter
> syntax\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 1f38d6fc1fe3..ef22a2a25448 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -108,7 +108,6 @@ int amdgpu_vm_fragment_size = -1;
> int amdgpu_vm_block_size = -1;
> int amdgpu_vm_fault_stop = 0;
> int amdgpu_vm_debug = 0;
> -int amdgpu_vram_page_split = 512;
> int amdgpu_vm_update_mode = -1;
> int amdgpu_exp_hw_support = 0;
> int amdgpu_dc = -1;
> @@ -342,13 +341,6 @@ module_param_named(vm_debug, amdgpu_vm_debug, int,
> 0644);
> MODULE_PARM_DESC(vm_update_mode, "VM update using CPU (0 = never (default
> except for large BAR(LB)), 1 = Graphics only, 2 = Compute only (default for
> LB), 3 = Both");
> module_param_named(vm_update_mode, amdgpu_vm_update_mode, int, 0444);
>
> -/**
> - * DOC: vram_page_split (int)
> - * Override the number of pages after we split VRAM allocations (default
> 512, -1 = disable). The default is 512.
> - */
> -MODULE_PARM_DESC(vram_page_split, "Number of pages after we split VRAM
> allocations (default 512, -1 = disable)");
> -module_param_named(vram_page_split, amdgpu_vram_page_split, int, 0444);
> -
> /**
> * DOC: exp_hw_support (int)
> * Enable experimental hw support (1 = enable). The default is 0
> (disabled).
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index ec9ea3fdbb4a..0bc01e25a0b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -284,17 +284,21 @@ static int amdgpu_vram_mgr_new(struct
> ttm_mem_type_manager *man,
> if (!lpfn)
> lpfn = man->size;
>
> - if (place->flags & TTM_PL_FLAG_CONTIGUOUS ||
> - amdgpu_vram_page_split == -1) {
> + if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
> pages_per_node = ~0ul;
> num_nodes = 1;
> } else {
> - pages_per_node = max((uint32_t)amdgpu_vram_page_split,
> -mem->page_alignment);
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> + pages_per_node = HPAGE_PMD_NR;
> +#else
> + /* default to 512MB */
> + pages_per_node = (512UL << (20UL - PAGE_SHIFT));
> +#endif
> + pages_per_node = max((uint32_t)pages_per_node,
> mem->page_alignment);
> num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
> }
>
> - nodes = kvmalloc_array(num_nodes, sizeof(*nodes),
> + nodes = kvmalloc_array((uint32_t)num_nodes, sizeof(*nodes),
>GFP_KERNEL | __GFP_ZERO);
> if (!nodes)
> return -ENOMEM;
> --
> 2.21.0
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
