[PATCH 1/2] drm/ttm, drm/vmwgfx: Have TTM support AMD SEV encryption
From: Thomas Hellstrom With SEV encryption, all DMA memory must be marked decrypted (AKA "shared") for devices to be able to read it. In the future we might want to be able to switch normal (encrypted) memory to decrypted in exactly the same way as we handle caching states, and that would require additional memory pools. But for now, rely on memory allocated with dma_alloc_coherent() which is already decrypted with SEV enabled. Set up the page protection accordingly. Drivers must detect SEV enabled and switch to the dma page pool if they don't want to bounce DMA though the SWIOTLB and implement proper syncing. Tested with vmwgfx and sev-es. Screen garbage without this patch and normal functionality with it. Cc: Christian König Cc: Thomas Lendacky Signed-off-by: Thomas Hellstrom --- drivers/gpu/drm/ttm/ttm_bo_util.c| 17 + drivers/gpu/drm/ttm/ttm_bo_vm.c | 17 +++-- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 +++ drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 6 -- include/drm/ttm/ttm_bo_driver.h | 8 +--- include/drm/ttm/ttm_tt.h | 1 + 6 files changed, 33 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 82ea26a49959..66d401935e0f 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -419,11 +419,13 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, page = i * dir + add; if (old_iomap == NULL) { pgprot_t prot = ttm_io_prot(old_mem->placement, + ttm->page_flags, PAGE_KERNEL); ret = ttm_copy_ttm_io_page(ttm, new_iomap, page, prot); } else if (new_iomap == NULL) { pgprot_t prot = ttm_io_prot(new_mem->placement, + ttm->page_flags, PAGE_KERNEL); ret = ttm_copy_io_ttm_page(ttm, old_iomap, page, prot); @@ -525,11 +527,11 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, return 0; } -pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) +pgprot_t ttm_io_prot(u32 caching_flags, u32 tt_page_flags, pgprot_t tmp) { /* Cached mappings need no adjustment */ if (caching_flags & TTM_PL_FLAG_CACHED) - return tmp; + goto check_encryption; #if defined(__i386__) || defined(__x86_64__) if (caching_flags & TTM_PL_FLAG_WC) @@ -547,6 +549,11 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) #if defined(__sparc__) tmp = pgprot_noncached(tmp); #endif + +check_encryption: + if (tt_page_flags & TTM_PAGE_FLAG_DECRYPTED) + tmp = pgprot_decrypted(tmp); + return tmp; } EXPORT_SYMBOL(ttm_io_prot); @@ -593,7 +600,8 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo, if (ret) return ret; - if (num_pages == 1 && (mem->placement & TTM_PL_FLAG_CACHED)) { + if (num_pages == 1 && (mem->placement & TTM_PL_FLAG_CACHED) && + !(ttm->page_flags & TTM_PAGE_FLAG_DECRYPTED)) { /* * We're mapping a single page, and the desired * page protection is consistent with the bo. @@ -607,7 +615,8 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo, * We need to use vmap to get the desired page protection * or to make the buffer object look contiguous. */ - prot = ttm_io_prot(mem->placement, PAGE_KERNEL); + prot = ttm_io_prot(mem->placement, ttm->page_flags, + PAGE_KERNEL); map->bo_kmap_type = ttm_bo_map_vmap; map->virtual = vmap(ttm->pages + start_page, num_pages, 0, prot); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index e93b1ad7828f..a7426f48c21d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -229,12 +229,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) * by mmap_sem in write mode. */ cvma = *vma; - cvma.vm_page_prot = vm_get_page_prot(cvma.vm_flags); - - if (bo->mem.bus.is_iomem) { - cvma.vm_page_prot = ttm_io_prot(bo->mem.placement, - cvma.vm_page_prot); - } else { + if (!bo->mem.bus.is_iomem) { struct ttm_operation_ctx ctx = { .interruptible = false, .no_wait_gpu = false, @@ -244,13 +239,15 @@ static vm_fault_t ttm_bo_vm_fault(struc
[PATCH 2/2] drm/ttm: Cache dma pool decrypted pages when AMD SEV is active
From: Thomas Hellstrom The TTM dma pool allocates coherent pages for use with TTM. When SEV is active, such allocations become very expensive since the linear kernel map has to be changed to mark the pages decrypted. So to avoid too many such allocations and frees, cache the decrypted pages even if they are in the normal cpu caching state, where otherwise the pool frees them immediately when unused. Tested with vmwgfx on SEV-ES. Cc: Christian König Cc: Thomas Lendacky Signed-off-by: Thomas Hellstrom --- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c index c7e223c4f26c..a4445a83bc96 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c @@ -999,7 +999,7 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) struct dma_pool *pool; struct dma_page *d_page, *next; enum pool_type type; - bool is_cached = false; + bool immediate_free = false; unsigned count, i, npages = 0; unsigned long irq_flags; @@ -1034,8 +1034,17 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) if (!pool) return; - is_cached = (ttm_dma_find_pool(pool->dev, -ttm_to_type(ttm->page_flags, tt_cached)) == pool); + /* +* If memory is cached and sev encryption is not active, allocating +* and freeing coherent memory is relatively cheap, so we can free +* it immediately. If sev encryption is active, allocating coherent +* memory involves a call to set_memory_decrypted() which is very +* expensive, so cache coherent pages is sev is active. +*/ + immediate_free = (ttm_dma_find_pool + (pool->dev, + ttm_to_type(ttm->page_flags, tt_cached)) == pool && + !sev_active()); /* make sure pages array match list and count number of pages */ count = 0; @@ -1050,13 +1059,13 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev) d_page->vaddr &= ~VADDR_FLAG_UPDATED_COUNT; } - if (is_cached) + if (immediate_free) ttm_dma_page_put(pool, d_page); } spin_lock_irqsave(&pool->lock, irq_flags); pool->npages_in_use -= count; - if (is_cached) { + if (immediate_free) { pool->nfrees += count; } else { pool->npages_free += count; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sti: Include the right header
The sti_hdmi.c file include despite not even using any GPIOs. What it does use is devm_ioremap_nocache() which comes from implicitly by including that header. Fix this up by including the right header instead. Cc: Benjamin Gaignard Cc: Vincent Abriou Signed-off-by: Linus Walleij --- drivers/gpu/drm/sti/sti_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index f03d617edc4c..4f1aaf222cb0 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/shmem: Do dma_unmap_sg before purging pages
On 19/08/2019 17:12, Rob Herring wrote: > Calling dma_unmap_sg() in drm_gem_shmem_free_object() is too late if the > backing pages have already been released by the shrinker. The result is > the following abort: > > Unable to handle kernel paging request at virtual address 898ed000 > Mem abort info: > ESR = 0x96000147 > Exception class = DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > Data abort info: > ISV = 0, ISS = 0x0147 > CM = 1, WnR = 1 > swapper pgtable: 4k pages, 48-bit VAs, pgdp=02f51000 > [898ed000] pgd=401f8003, pud=401f7003, > pmd=401b1003, pte=00e8098ed712 > Internal error: Oops: 96000147 [#1] SMP > Modules linked in: panfrost gpu_sched > CPU: 5 PID: 902 Comm: gnome-shell Not tainted 5.3.0-rc1+ #95 > Hardware name: 96boards Rock960 (DT) > pstate: 4005 (nZcv daif -PAN -UAO) > pc : __dma_inv_area+0x40/0x58 > lr : arch_sync_dma_for_cpu+0x28/0x30 > sp : 1321ba30 > x29: 1321ba30 x28: 1321bd08 > x27: x26: 0009 > x25: c1f86170 x24: > x23: x22: > x21: 00021000 x20: 80003bb2d810 > x19: 098ed000 x18: > x17: x16: 800023fd9480 > x15: x14: > x13: x12: > x11: fffb9fff x10: > x9 : x8 : 800023fd9c18 > x7 : x6 : > x5 : x4 : 00021000 > Purging 5693440 bytes > x3 : 003f x2 : 0040 > x1 : 8990e000 x0 : 898ed000 > Call trace: > __dma_inv_area+0x40/0x58 > dma_direct_sync_single_for_cpu+0x7c/0x80 > dma_direct_unmap_page+0x80/0x88 > dma_direct_unmap_sg+0x54/0x80 > drm_gem_shmem_free_object+0xfc/0x108 > panfrost_gem_free_object+0x118/0x128 [panfrost] > drm_gem_object_free+0x18/0x90 > drm_gem_object_put_unlocked+0x58/0x80 > drm_gem_object_handle_put_unlocked+0x64/0xb0 > drm_gem_object_release_handle+0x70/0x98 > drm_gem_handle_delete+0x64/0xb0 > drm_gem_close_ioctl+0x28/0x38 > drm_ioctl_kernel+0xb8/0x110 > drm_ioctl+0x244/0x3f0 > do_vfs_ioctl+0xbc/0x910 > ksys_ioctl+0x78/0xa8 > __arm64_sys_ioctl+0x1c/0x28 > el0_svc_common.constprop.0+0x88/0x150 > el0_svc_handler+0x28/0x78 > el0_svc+0x8/0xc > Code: 8a23 5460 d50b7e20 1402 (d5087620) > > Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers") > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Looks good to me: Reviewed-by: Steven Price > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index df8f2c8adb2b..5423ec56b535 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -390,6 +390,12 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object > *obj) > > WARN_ON(!drm_gem_shmem_is_purgeable(shmem)); > > + dma_unmap_sg(obj->dev->dev, shmem->sgt->sgl, > + shmem->sgt->nents, DMA_BIDIRECTIONAL); > + sg_free_table(shmem->sgt); > + kfree(shmem->sgt); > + shmem->sgt = NULL; > + > drm_gem_shmem_put_pages_locked(shmem); > > shmem->madv = -1; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/panfrost: Fix shrinker lockdep issues using drm_gem_shmem_purge()
On 19/08/2019 17:12, Rob Herring wrote: > This fixes 2 issues found by lockdep. First, drm_gem_shmem_purge() > now uses mutex_trylock for the pages_lock to avoid a circular > dependency. NIT: This is in the previous patch. > Second, it drops the call to panfrost_mmu_unmap() which takes several > locks due to runtime PM calls. The call is not necessary because the > unmapping is also called in panfrost_gem_close() already. I could be completely mistaken here, but don't we need to unmap the memory from the GPU here because the backing is free? The panfrost_gem_close() call could come significantly later, by which time a malicious user space could have run some jobs on the GPU to take a look at what those mappings now point to (quite likely some other processes memory). So this looks to me like a crafty way of observing 'random' memory in the system. Steve > Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") > Cc: Tomeu Vizoso > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring > --- > drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 15 ++- > 1 file changed, 2 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index d191632b6197..cc15005dc68f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -36,17 +36,6 @@ panfrost_gem_shrinker_count(struct shrinker *shrinker, > struct shrink_control *sc > return count; > } > > -static void panfrost_gem_purge(struct drm_gem_object *obj) > -{ > - struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > - mutex_lock(&shmem->pages_lock); > - > - panfrost_mmu_unmap(to_panfrost_bo(obj)); > - drm_gem_shmem_purge_locked(obj); > - > - mutex_unlock(&shmem->pages_lock); > -} > - > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control > *sc) > { > @@ -61,8 +50,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, > struct shrink_control *sc) > list_for_each_entry_safe(shmem, tmp, &pfdev->shrinker_list, madv_list) { > if (freed >= sc->nr_to_scan) > break; > - if (drm_gem_shmem_is_purgeable(shmem)) { > - panfrost_gem_purge(&shmem->base); > + if (drm_gem_shmem_is_purgeable(shmem) && > + drm_gem_shmem_purge(&shmem->base)) { > freed += shmem->base.size >> PAGE_SHIFT; > list_del_init(&shmem->madv_list); > } > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/panfrost: Fix sleeping while atomic in panfrost_gem_open
On 19/08/2019 17:12, Rob Herring wrote: > We can't hold the mm_lock spinlock as panfrost_mmu_map() can sleep: > > BUG: sleeping function called from invalid context at > kernel/locking/mutex.c:909 > in_atomic(): 1, irqs_disabled(): 0, pid: 974, name: glmark2-es2-drm > 1 lock held by glmark2-es2-drm/974: > CPU: 5 PID: 974 Comm: glmark2-es2-drm Tainted: GWL5.3.0-rc1+ > #94 > Hardware name: 96boards Rock960 (DT) > Call trace: > dump_backtrace+0x0/0x130 > show_stack+0x14/0x20 > dump_stack+0xc4/0x10c > ___might_sleep+0x158/0x228 > __might_sleep+0x50/0x88 > __mutex_lock+0x58/0x800 > mutex_lock_interruptible_nested+0x1c/0x28 > drm_gem_shmem_get_pages+0x24/0xa0 > drm_gem_shmem_get_pages_sgt+0x48/0xd0 > panfrost_mmu_map+0x38/0xf8 [panfrost] > panfrost_gem_open+0xc0/0xd8 [panfrost] > drm_gem_handle_create_tail+0xe8/0x198 > drm_gem_handle_create+0x3c/0x50 > panfrost_gem_create_with_handle+0x70/0xa0 [panfrost] > panfrost_ioctl_create_bo+0x48/0x80 [panfrost] > drm_ioctl_kernel+0xb8/0x110 > drm_ioctl+0x244/0x3f0 > do_vfs_ioctl+0xbc/0x910 > ksys_ioctl+0x78/0xa8 > __arm64_sys_ioctl+0x1c/0x28 > el0_svc_common.constprop.0+0x90/0x168 > el0_svc_handler+0x28/0x78 > el0_svc+0x8/0xc > > Fixes: 68337d0b8644 ("drm/panfrost: Restructure the GEM object creation") > Cc: Tomeu Vizoso > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c > b/drivers/gpu/drm/panfrost/panfrost_gem.c > index e084bc4e9083..acb07fe06580 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -65,16 +65,18 @@ static int panfrost_gem_open(struct drm_gem_object *obj, > struct drm_file *file_p > spin_lock(&priv->mm_lock); > ret = drm_mm_insert_node_generic(&priv->mm, &bo->node, >size >> PAGE_SHIFT, align, color, 0); > + spin_unlock(&priv->mm_lock); > if (ret) > - goto out; > + return ret; > > if (!bo->is_heap) { > ret = panfrost_mmu_map(bo); > - if (ret) > + if (ret) { > + spin_lock(&priv->mm_lock); > drm_mm_remove_node(&bo->node); > + spin_unlock(&priv->mm_lock); > + } > } > -out: > - spin_unlock(&priv->mm_lock); > return ret; > } > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/shmem: Use mutex_trylock in drm_gem_shmem_purge
On 19/08/2019 17:12, Rob Herring wrote: > Lockdep reports a circular locking dependency with pages_lock taken in > the shrinker callback. The deadlock can't actually happen with current > users at least as a BO will never be purgeable when pages_lock is held. > To be safe, let's use mutex_trylock() instead and bail if a BO is locked > already. > > WARNING: possible circular locking dependency detected > 5.3.0-rc1+ #100 Tainted: G L > -- > kswapd0/171 is trying to acquire lock: > 9b9823fd (&shmem->pages_lock){+.+.}, at: drm_gem_shmem_purge+0x20/0x40 > > but task is already holding lock: > f82369b6 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x40 > > which lock already depends on the new lock. > > the existing dependency chain (in reverse order) is: > > -> #1 (fs_reclaim){+.+.}: >fs_reclaim_acquire.part.18+0x34/0x40 >fs_reclaim_acquire+0x20/0x28 >__kmalloc_node+0x6c/0x4c0 >kvmalloc_node+0x38/0xa8 >drm_gem_get_pages+0x80/0x1d0 >drm_gem_shmem_get_pages+0x58/0xa0 >drm_gem_shmem_get_pages_sgt+0x48/0xd0 >panfrost_mmu_map+0x38/0xf8 [panfrost] >panfrost_gem_open+0xc0/0xe8 [panfrost] >drm_gem_handle_create_tail+0xe8/0x198 >drm_gem_handle_create+0x3c/0x50 >panfrost_gem_create_with_handle+0x70/0xa0 [panfrost] >panfrost_ioctl_create_bo+0x48/0x80 [panfrost] >drm_ioctl_kernel+0xb8/0x110 >drm_ioctl+0x244/0x3f0 >do_vfs_ioctl+0xbc/0x910 >ksys_ioctl+0x78/0xa8 >__arm64_sys_ioctl+0x1c/0x28 >el0_svc_common.constprop.0+0x90/0x168 >el0_svc_handler+0x28/0x78 >el0_svc+0x8/0xc > > -> #0 (&shmem->pages_lock){+.+.}: >__lock_acquire+0xa2c/0x1d70 >lock_acquire+0xdc/0x228 >__mutex_lock+0x8c/0x800 >mutex_lock_nested+0x1c/0x28 >drm_gem_shmem_purge+0x20/0x40 >panfrost_gem_shrinker_scan+0xc0/0x180 [panfrost] >do_shrink_slab+0x208/0x500 >shrink_slab+0x10c/0x2c0 >shrink_node+0x28c/0x4d8 >balance_pgdat+0x2c8/0x570 >kswapd+0x22c/0x638 >kthread+0x128/0x130 >ret_from_fork+0x10/0x18 > > other info that might help us debug this: > > Possible unsafe locking scenario: > >CPU0CPU1 > > lock(fs_reclaim); >lock(&shmem->pages_lock); >lock(fs_reclaim); > lock(&shmem->pages_lock); > > *** DEADLOCK *** > > 3 locks held by kswapd0/171: > #0: f82369b6 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x40 > #1: ceb37808 (shrinker_rwsem){}, at: shrink_slab+0xbc/0x2c0 > #2: f31efa81 (&pfdev->shrinker_lock){+.+.}, at: > panfrost_gem_shrinker_scan+0x34/0x180 [panfrost] > > Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers") > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Seems reasonable, like you state I don't think this can actually happen, but keeping lockdep happy is a good idea. Reviewed-by: Steven Price Steve > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 7 +-- > include/drm/drm_gem_shmem_helper.h | 2 +- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 5423ec56b535..f5918707672f 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -415,13 +415,16 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object > *obj) > } > EXPORT_SYMBOL(drm_gem_shmem_purge_locked); > > -void drm_gem_shmem_purge(struct drm_gem_object *obj) > +bool drm_gem_shmem_purge(struct drm_gem_object *obj) > { > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > > - mutex_lock(&shmem->pages_lock); > + if (!mutex_trylock(&shmem->pages_lock)) > + return false; > drm_gem_shmem_purge_locked(obj); > mutex_unlock(&shmem->pages_lock); > + > + return true; > } > EXPORT_SYMBOL(drm_gem_shmem_purge); > > diff --git a/include/drm/drm_gem_shmem_helper.h > b/include/drm/drm_gem_shmem_helper.h > index ce1600fdfc3e..01f514521687 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -134,7 +134,7 @@ static inline bool drm_gem_shmem_is_purgeable(struct > drm_gem_shmem_object *shmem > } > > void drm_gem_shmem_purge_locked(struct drm_gem_object *obj); > -void drm_gem_shmem_purge(struct drm_gem_object *obj); > +bool drm_gem_shmem_purge(struct drm_gem_object *obj); > > struct drm_gem_shmem_object * > drm_gem_shmem_create_with_handle(struct drm_file *file_priv, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.
[PATCH 3/6 v2] drm/msm/dpu: Drop unused GPIO code
The DPU has some kind of idea that it wants to be able to bring up power using GPIO lines. The struct dss_gpio is however completely unused and should this be done, it should be done using the GPIO descriptor framework rather than this API which relies on the global GPIO numberspace. Delete this code before anyone hurt themselves. The inclusion of was abused to get some OF and IRQ headers implicitly included into the DPU utilities, make these includes explicit and push them down into the actual implementation. Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Reviewed-by: Brian Masney Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h | 9 - drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c| 4 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c index 71b6987bff1e..27fbeb504362 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c @@ -7,6 +7,7 @@ #include #include #include +#include #include diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h index 09083e9f06bb..e6b5c772fa3b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h @@ -5,7 +5,6 @@ #ifndef __DPU_IO_UTIL_H__ #define __DPU_IO_UTIL_H__ -#include #include #include @@ -14,12 +13,6 @@ #define DEV_WARN(fmt, args...) pr_warn(fmt, ##args) #define DEV_ERR(fmt, args...) pr_err(fmt, ##args) -struct dss_gpio { - unsigned int gpio; - unsigned int value; - char gpio_name[32]; -}; - enum dss_clk_type { DSS_CLK_AHB, /* no set rate. rate controlled through rpm */ DSS_CLK_PCLK, @@ -34,8 +27,6 @@ struct dss_clk { }; struct dss_module_power { - unsigned int num_gpio; - struct dss_gpio *gpio_config; unsigned int num_clk; struct dss_clk *clk_config; }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index 986915bbbc02..c977baddfffd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -3,6 +3,10 @@ * Copyright (c) 2018, The Linux Foundation */ +#include +#include +#include +#include #include "dpu_kms.h" #include -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6 v2] drm/msm/hdmi: Convert to use GPIO descriptors
This switches the MSM HDMI code to use GPIO descriptors. Normally we would fetch the GPIOs from the device with the flags GPIOD_IN or GPIOD_OUT_[LOW|HIGH] to set up the lines immediately, but since the code seems eager to actively drive the lines high/low when turning HDMI on and off, we just fetch the GPIOs as-is and keep the code explicitly driving them. The old code would try legacy bindings (GPIOs without any "-gpios" suffix) but this has been moved to the gpiolib as a quirk by the previous patch. Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Reviewed-by: Brian Masney Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/hdmi/hdmi.c | 66 +++ drivers/gpu/drm/msm/hdmi/hdmi.h | 4 +- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 42 +-- 3 files changed, 45 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 0e4217be3f00..355afb936401 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -425,38 +425,6 @@ static const struct { { "qcom,hdmi-tx-mux-lpm", true, 1, "HDMI_MUX_LPM" }, }; -static int msm_hdmi_get_gpio(struct device_node *of_node, const char *name) -{ - int gpio; - - /* try with the gpio names as in the table (downstream bindings) */ - gpio = of_get_named_gpio(of_node, name, 0); - if (gpio < 0) { - char name2[32]; - - /* try with the gpio names as in the upstream bindings */ - snprintf(name2, sizeof(name2), "%s-gpios", name); - gpio = of_get_named_gpio(of_node, name2, 0); - if (gpio < 0) { - char name3[32]; - - /* -* try again after stripping out the "qcom,hdmi-tx" -* prefix. This is mainly to match "hpd-gpios" used -* in the upstream bindings -*/ - if (sscanf(name2, "qcom,hdmi-tx-%s", name3)) - gpio = of_get_named_gpio(of_node, name3, 0); - } - - if (gpio < 0) { - DBG("failed to get gpio: %s (%d)", name, gpio); - gpio = -1; - } - } - return gpio; -} - /* * HDMI audio codec callbacks */ @@ -582,11 +550,39 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) hdmi_cfg->qfprom_mmio_name = "qfprom_physical"; for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { - hdmi_cfg->gpios[i].num = msm_hdmi_get_gpio(of_node, - msm_hdmi_gpio_pdata[i].name); + const char *name = msm_hdmi_gpio_pdata[i].name; + struct gpio_desc *gpiod; + + /* +* We are fetching the GPIO lines "as is" since the connector +* code is enabling and disabling the lines. Until that point +* the power-on default value will be kept. +*/ + gpiod = devm_gpiod_get_optional(dev, name, GPIOD_ASIS); + /* This will catch e.g. -PROBE_DEFER */ + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + if (!gpiod) { + /* Try a second time, stripping down the name */ + char name3[32]; + + /* +* Try again after stripping out the "qcom,hdmi-tx" +* prefix. This is mainly to match "hpd-gpios" used +* in the upstream bindings. +*/ + if (sscanf(name, "qcom,hdmi-tx-%s", name3)) + gpiod = devm_gpiod_get_optional(dev, name3, GPIOD_ASIS); + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + if (!gpiod) + DBG("failed to get gpio: %s", name); + } + hdmi_cfg->gpios[i].gpiod = gpiod; + if (gpiod) + gpiod_set_consumer_name(gpiod, msm_hdmi_gpio_pdata[i].label); hdmi_cfg->gpios[i].output = msm_hdmi_gpio_pdata[i].output; hdmi_cfg->gpios[i].value = msm_hdmi_gpio_pdata[i].value; - hdmi_cfg->gpios[i].label = msm_hdmi_gpio_pdata[i].label; } dev->platform_data = hdmi_cfg; diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h index 982865866a29..bdac452b00fb 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.h +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "msm_drv.h" @@ -22,10 +23,9 @@ struct hdmi_phy; struct hdmi_platform
[PATCH 5/6 v2] drm/msm/hdmi: Bring up HDMI connector OFF
There is elaborate code in the HDMI connector handling to leave the connector in the state it was at power-on and only touch the GPIOs when the connector .enable() and .disable() callbacks are called. I don't think this is what we normally want, initialize the connector as OFF (possibly saving power?) using the appropriate GPIO descriptor flags. It will still be switched on/off in the enable()/disable() connector callback as before, but we can drop some strange surplus code. Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Reviewed-by: Brian Masney Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/hdmi/hdmi.c | 19 - drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 25 ++- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 355afb936401..5739eec65659 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -552,13 +552,22 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { const char *name = msm_hdmi_gpio_pdata[i].name; struct gpio_desc *gpiod; + enum gpiod_flags flags; /* -* We are fetching the GPIO lines "as is" since the connector -* code is enabling and disabling the lines. Until that point -* the power-on default value will be kept. +* Notice the inverse set up here: we initialize the connector +* to OFF state. */ - gpiod = devm_gpiod_get_optional(dev, name, GPIOD_ASIS); + if (msm_hdmi_gpio_pdata[i].output) { + if (msm_hdmi_gpio_pdata[i].value) + flags = GPIOD_OUT_LOW; + else + flags = GPIOD_OUT_HIGH; + } else { + flags = GPIOD_IN; + } + + gpiod = devm_gpiod_get_optional(dev, name, flags); /* This will catch e.g. -PROBE_DEFER */ if (IS_ERR(gpiod)) return PTR_ERR(gpiod); @@ -572,7 +581,7 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data) * in the upstream bindings. */ if (sscanf(name, "qcom,hdmi-tx-%s", name3)) - gpiod = devm_gpiod_get_optional(dev, name3, GPIOD_ASIS); + gpiod = devm_gpiod_get_optional(dev, name3, flags); if (IS_ERR(gpiod)) return PTR_ERR(gpiod); if (!gpiod) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index d0575d4f747d..f006682935e9 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -75,16 +75,9 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i]; - if (gpio.gpiod) { - if (gpio.output) { - gpiod_direction_output(gpio.gpiod, - gpio.value); - } else { - gpiod_direction_input(gpio.gpiod); - gpiod_set_value_cansleep(gpio.gpiod, -gpio.value); - } - } + /* The value indicates the value for turning things on */ + if (gpio.gpiod) + gpiod_set_value_cansleep(gpio.gpiod, gpio.value); } DBG("gpio on"); @@ -92,16 +85,10 @@ static int gpio_config(struct hdmi *hdmi, bool on) for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) { struct hdmi_gpio_data gpio = config->gpios[i]; - if (!gpio.gpiod) - continue; - - if (gpio.output) { - int value = gpio.value ? 0 : 1; - - gpiod_set_value_cansleep(gpio.gpiod, value); - } + /* The inverse value turns stuff off */ + if (gpio.gpiod && gpio.output) + gpiod_set_value_cansleep(gpio.gpiod, !gpio.value); }; - DBG("gpio off"); } -- 2.21.0
[PATCH 2/6 v2] drm/msm/dsi: Drop unused GPIO includes
This DSI driver uses the new descriptor API so these old GPIO API includes are surplus. Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Reviewed-by: Brian Masney Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index aa35d18ab43c..4b6f62138390 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -6,11 +6,9 @@ #include #include #include -#include #include #include #include -#include #include #include #include -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6 v2] drm/msm/mdp4: Drop unused GPIO include
This file is not using any symbols from so just drop this include. Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Reviewed-by: Brian Masney Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c index ecef4f5b9f26..9262ed2dc8c3 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c @@ -5,8 +5,6 @@ * Author: Vinay Simha */ -#include - #include "mdp4_kms.h" struct mdp4_lvds_connector { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6 v2] drm/msm/hdmi: Do not initialize HPD line value
After untangling the MSM HDMI GPIO code we see that the code is deliberately setting the output value of the HPD (hot plug detect) line to high, even though it is being used as input which is of course the only viable use of a HPD pin. This seems dubious: GPIO lines set up as input will have high impedance (tristate) and the typical electronic construction involves this line being used with a pull-down resistor around 10KOhm to keep it low (this is sometimes part of a levelshifter component) and then an inserted connector will pull it up to VDD and this asserts the HPD signal, as can be seen from the code reading the HPD GPIO. Stop try driving a value to the HPD input GPIO. Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Reviewed-by: Brian Masney Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Rebased on v5.3-rc1 - Collected review tag --- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index f006682935e9..bb1c49e3c9dd 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -76,7 +76,7 @@ static int gpio_config(struct hdmi *hdmi, bool on) struct hdmi_gpio_data gpio = config->gpios[i]; /* The value indicates the value for turning things on */ - if (gpio.gpiod) + if (gpio.gpiod && gpio.output) gpiod_set_value_cansleep(gpio.gpiod, gpio.value); } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #32 from Tomas --- Hi, I have applied the patch to drivers amdgpu-pro-19.30-855429-ubuntu-18.04, and something is wrong. The process doesn't find as seems some components? See output i got on the cli. Have I overlooked something? $ ./install-amdgpu-pro-19.30-855429-on-ubuntu19.04-v06.sh dpkg-deb: building package 'amdgpu-core' in 'amdgpu-core_19.30-855429_all.no_ub_ver_chk.deb'. dpkg-deb: building package 'amdgpu-pro-hwe' in 'amdgpu-pro-hwe_19.30-855429_amd64.no_ag-hwe_dep.deb'. dpkg-deb: building package 'amdgpu-pro-lib32' in 'amdgpu-pro-lib32_19.30-855429_amd64.no_ag-hwe_and_ag-lib32_dep.deb'. Creating local repository... Installing OpenGL PRO... [sudo] password for user: Reading package lists... Done Building dependency tree Reading state information... Done E: Unable to locate package amdgpu-pro-hwe E: Unable to locate package amdgpu-pro-lib32 Installing OpenCL PRO... Reading package lists... Done Building dependency tree Reading state information... Done E: Unable to locate package clinfo-amdgpu-pro E: Unable to locate package opencl-orca-amdgpu-pro-icd E: Unable to locate package opencl-orca-amdgpu-pro-icd:i386 E: Unable to locate package opencl-amdgpu-pro-icd Installing Vulkan PRO... Reading package lists... Done Building dependency tree Reading state information... Done E: Unable to locate package vulkan-amdgpu-pro E: Unable to locate package vulkan-amdgpu-pro:i386 All done, enjoy! This script was prepared for you by Andrew Shark, the author of LinuxComp Tutorial youtube channel. If you liked it, please let me know =) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #33 from Tomas --- System is Ubuntu 19.04, 5.0.0-25-generic #26-Ubuntu SMP Thu Aug 1 12:04:58 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: Fix port lookup for SDI output
On 21/08/2019 23:26, Aaro Koskinen wrote: Hi, On Wed, Aug 21, 2019 at 09:32:26PM +0300, Laurent Pinchart wrote: When refactoring port lookup for DSS outputs, commit d17eb4537a7e ("drm/omap: Factor out common init/cleanup code for output devices") incorrectly hardcoded usage of DT port 0. This breaks operation for SDI (which uses the DT port 1) and DPI outputs other than DPI0 (which are not used in mainline DT sources). Fix this by using the port number from the output omap_dss_device of_ports field. Fixes: d17eb4537a7e ("drm/omap: Factor out common init/cleanup code for output devices") Signed-off-by: Laurent Pinchart Tested-by: Aaro Koskinen Thanks, this fixes the display issue on N900. Thanks, pushed to drm-misc-fixes. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-next
Hi Dave and Daniel, (atempt v2: for some reason my dim didn't recognized the path apparently ) Here goes the final pull request targeting 5.4. It's important to highlight that we got a conflict on a backmerge yesterday which had already been solved on linux-next with a fix up patch: From: Stephen Rothwell Date: Wed, 14 Aug 2019 12:48:39 +1000 Subject: [PATCH] drm: fix up fallout from "dma-buf: rename reservation_object to dma_resv" Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/i915/gt/intel_engine_pool.c | 8 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pool.c b/drivers/gpu/drm/i915/gt/intel_engine_pool.c index 03d90b49584a..4cd54c569911 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pool.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pool.c @@ -43,12 +43,12 @@ static int pool_active(struct i915_active *ref) { struct intel_engine_pool_node *node = container_of(ref, typeof(*node), active); - struct reservation_object *resv = node->obj->base.resv; + struct dma_resv *resv = node->obj->base.resv; int err; - if (reservation_object_trylock(resv)) { - reservation_object_add_excl_fence(resv, NULL); - reservation_object_unlock(resv); + if (dma_resv_trylock(resv)) { + dma_resv_add_excl_fence(resv, NULL); + dma_resv_unlock(resv); } err = i915_gem_object_pin_pages(node->obj); which is a simplified version from a previous one which had: Reviewed-by: Christian König With this we could also remove the latest dma_resv fixup patch from drm_rerere. Unfortunately on this merge commit a ghost file reapeared probably by an amend mistake from my side. And consequently removed by Chris with an extra patch. I hope this doesn't cause any trouble for you, but if so the solution is simply go with the version that deletes the file for good. This pull request also includes Gvt stuff including several enhancements for command parser and batch buffer shadow, remove extra debugfs function return check, and other misc changes like typo, static check fix, etc. The rest is just as usual and split in 3 different drm-intel-next tags: drm-intel-next-2019-08-22: - More TGL enabling work (Michel, Jose, Lucas) - Fixes on DP MST (Ville) - More GTT and Execlists fixes and improvements (Chris) - Code style clean-up on hdmi and dp side (Jani) - Fix null pointer dereferrence (Xiong) - Fix a couple of missing serialization on selftests (Chris) - More vm locking rework (Chris) drm-intel-next-2019-08-20: - GuC and HuC related fixes and improvements (Daniele, Michal) - Improve debug with more engine information and rework on debugfs files (Chris, Stuart) - Simplify appearture address handling (Chris) - Other fixes and cleanups around engines and execlists (Chris) - Selftests fixes (Matt, Chris) - Gen11 cache flush related fixes and improvements (Mika) - More work around requests, timelines and locks to allow removal of struct_mutex (Chris) - Add missing CML PCI ID (Anusha) - More work on the new i915 buddy allocator (Matt) - More headers, files and directories reorg (Daniele) - Improvements on ggtt’s get pdp (Mika) - Fix GPU reset (Chris) - Fix GPIO pins on gen11 (Matt) - Fix HW readout for crtc_clock in HDMI mode (Imre) - Sanitize display Phy during unitit to workaround messages of HW state change during suspend (Imre) - Be defensive when starting vma activity (Chris) - More Tiger Lake enabling work (Michel, Daniele, Lucas) - Relax pd_used assertion (Chris) drm-intel-next-2019-08-13: - More Tiger Lake enabling work (Lucas, Jose, Tomasz, Michel, Jordan, Anusha, Vandita) - More selftest organization reworks, fixes and improvements (Lucas, Chris) - Simplifications on GEM code like context and cleanup_early (Chris, Daniele) - GuC and HuC related fixes and improvements (Daniele, Michal, Chris) - Some clean up and fixes on headers, Makefile, and generated files (Lucas, Jani) - MOCS setup clean up (Tvrtko) - More Elkhartlake enabling work (Jose, Matt) - Fix engine reset by clearing in flight execlists requests (Chris) - Fix possible memory leak on intel_hdcp_auth_downstream (Wei) - Introduce intel_gt_runtime_suspend/resume (Daniele) - PMU improvements (Tvrtko) - Flush extra hard after writing relocations through the GTT (Chris) - Documentations fixes (Michal, Chris) - Report dma_reserv allocation failure (Chris) - Improvements around shrinker (Chris) - More improvements around engine handling (Chris) - Also more s/dev_priv/i915 (Chris) - Abstract display suspend/resume operations (Rodrigo/Jani) - Drop VM_IO from GTT mappings (Chris) - Fix some NULL vs IS_ERR conditions (Dan) - General improvements on error state (Chris) - Isolate i915_getparam_iocrtl to its own file (Chris) - Perf OA object refactor (Umesh) - Ignore central i915->kernel_context and allocate it directly (Chris) - More fixes and improvements around wakerefs (Chris) - Clean-up and im
Re: [PATCH v7 2/6] drm: port definition is moved back into i915 header
On Thu, 22 Aug 2019, Ramalingam C wrote: > Handled the need for exposing enum port to mei_hdcp driver, by > converting the port into ddi index as per ME FW. > > Hence enum port definition moved into I915 driver itself. For future reference, please consider using the imperative style in the commit message. For example, "move port definition back to i915 header", "Hence move enum port definition into i915 driver itself", etc. There were some complaints from CI, please make sure this builds with CONFIG_DRM_I915_WERROR=y. Reviewed-by: Jani Nikula > > Signed-off-by: Ramalingam C > --- > drivers/gpu/drm/i915/display/intel_bios.h| 2 ++ > drivers/gpu/drm/i915/display/intel_display.h | 18 ++ > drivers/gpu/drm/i915/display/intel_dp.h | 1 + > include/drm/i915_drm.h | 18 -- > 4 files changed, 21 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h > b/drivers/gpu/drm/i915/display/intel_bios.h > index 4969189e620f..9415e22435ba 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.h > +++ b/drivers/gpu/drm/i915/display/intel_bios.h > @@ -34,6 +34,8 @@ > > #include > > +#include "intel_display.h" > + > struct drm_i915_private; > > enum intel_backlight_type { > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > b/drivers/gpu/drm/i915/display/intel_display.h > index e57e6969051d..40610d51327e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.h > +++ b/drivers/gpu/drm/i915/display/intel_display.h > @@ -182,6 +182,24 @@ enum plane_id { > for ((__p) = PLANE_PRIMARY; (__p) < I915_MAX_PLANES; (__p)++) \ > for_each_if((__crtc)->plane_ids_mask & BIT(__p)) > > +enum port { > + PORT_NONE = -1, > + > + PORT_A = 0, > + PORT_B, > + PORT_C, > + PORT_D, > + PORT_E, > + PORT_F, > + PORT_G, > + PORT_H, > + PORT_I, > + > + I915_MAX_PORTS > +}; > + > +#define port_name(p) ((p) + 'A') > + > /* > * Ports identifier referenced from other drivers. > * Expected to remain stable over time > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h > b/drivers/gpu/drm/i915/display/intel_dp.h > index 657bbb1f5ed0..ca05ae799d6e 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.h > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > @@ -11,6 +11,7 @@ > #include > > #include "i915_reg.h" > +#include "intel_display.h" > > enum pipe; > struct drm_connector_state; > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h > index 23274cf92712..6722005884db 100644 > --- a/include/drm/i915_drm.h > +++ b/include/drm/i915_drm.h > @@ -100,22 +100,4 @@ extern struct resource intel_graphics_stolen_res; > #define INTEL_GEN11_BSM_DW1 0xc4 > #define INTEL_BSM_MASK (-(1u << 20)) > > -enum port { > - PORT_NONE = -1, > - > - PORT_A = 0, > - PORT_B, > - PORT_C, > - PORT_D, > - PORT_E, > - PORT_F, > - PORT_G, > - PORT_H, > - PORT_I, > - > - I915_MAX_PORTS > -}; > - > -#define port_name(p) ((p) + 'A') > - > #endif /* _I915_DRM_H_ */ -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-next
Hi Dave and Daniel, Here goes the final pull request targeting 5.4. It's important to highlight that we got a conflict on a backmerge yesterday which had already been solved on linux-next with a fix up patch: From: Stephen Rothwell Date: Wed, 14 Aug 2019 12:48:39 +1000 Subject: [PATCH] drm: fix up fallout from "dma-buf: rename reservation_object to dma_resv" Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/i915/gt/intel_engine_pool.c | 8 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pool.c b/drivers/gpu/drm/i915/gt/intel_engine_pool.c index 03d90b49584a..4cd54c569911 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pool.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pool.c @@ -43,12 +43,12 @@ static int pool_active(struct i915_active *ref) { struct intel_engine_pool_node *node = container_of(ref, typeof(*node), active); - struct reservation_object *resv = node->obj->base.resv; + struct dma_resv *resv = node->obj->base.resv; int err; - if (reservation_object_trylock(resv)) { - reservation_object_add_excl_fence(resv, NULL); - reservation_object_unlock(resv); + if (dma_resv_trylock(resv)) { + dma_resv_add_excl_fence(resv, NULL); + dma_resv_unlock(resv); } err = i915_gem_object_pin_pages(node->obj); which is a simplified version from a previous one which had: Reviewed-by: Christian König With this we could also remove the latest dma_resv fixup patch from drm_rerere. Unfortunately on this merge commit a ghost file reapeared probably by an amend mistake from my side. And consequently removed by Chris with an extra patch. I hope this doesn't cause any trouble for you, but if so the solution is simply go with the version that deletes the file for good. This pull request also includes Gvt stuff including several enhancements for command parser and batch buffer shadow, remove extra debugfs function return check, and other misc changes like typo, static check fix, etc. The rest is just as usual and split in 3 different drm-intel-next tags: drm-intel-next-2019-08-22: - More TGL enabling work (Michel, Jose, Lucas) - Fixes on DP MST (Ville) - More GTT and Execlists fixes and improvements (Chris) - Code style clean-up on hdmi and dp side (Jani) - Fix null pointer dereferrence (Xiong) - Fix a couple of missing serialization on selftests (Chris) - More vm locking rework (Chris) drm-intel-next-2019-08-20: - GuC and HuC related fixes and improvements (Daniele, Michal) - Improve debug with more engine information and rework on debugfs files (Chris, Stuart) - Simplify appearture address handling (Chris) - Other fixes and cleanups around engines and execlists (Chris) - Selftests fixes (Matt, Chris) - Gen11 cache flush related fixes and improvements (Mika) - More work around requests, timelines and locks to allow removal of struct_mutex (Chris) - Add missing CML PCI ID (Anusha) - More work on the new i915 buddy allocator (Matt) - More headers, files and directories reorg (Daniele) - Improvements on ggtt’s get pdp (Mika) - Fix GPU reset (Chris) - Fix GPIO pins on gen11 (Matt) - Fix HW readout for crtc_clock in HDMI mode (Imre) - Sanitize display Phy during unitit to workaround messages of HW state change during suspend (Imre) - Be defensive when starting vma activity (Chris) - More Tiger Lake enabling work (Michel, Daniele, Lucas) - Relax pd_used assertion (Chris) drm-intel-next-2019-08-13: - More Tiger Lake enabling work (Lucas, Jose, Tomasz, Michel, Jordan, Anusha, Vandita) - More selftest organization reworks, fixes and improvements (Lucas, Chris) - Simplifications on GEM code like context and cleanup_early (Chris, Daniele) - GuC and HuC related fixes and improvements (Daniele, Michal, Chris) - Some clean up and fixes on headers, Makefile, and generated files (Lucas, Jani) - MOCS setup clean up (Tvrtko) - More Elkhartlake enabling work (Jose, Matt) - Fix engine reset by clearing in flight execlists requests (Chris) - Fix possible memory leak on intel_hdcp_auth_downstream (Wei) - Introduce intel_gt_runtime_suspend/resume (Daniele) - PMU improvements (Tvrtko) - Flush extra hard after writing relocations through the GTT (Chris) - Documentations fixes (Michal, Chris) - Report dma_reserv allocation failure (Chris) - Improvements around shrinker (Chris) - More improvements around engine handling (Chris) - Also more s/dev_priv/i915 (Chris) - Abstract display suspend/resume operations (Rodrigo/Jani) - Drop VM_IO from GTT mappings (Chris) - Fix some NULL vs IS_ERR conditions (Dan) - General improvements on error state (Chris) - Isolate i915_getparam_iocrtl to its own file (Chris) - Perf OA object refactor (Umesh) - Ignore central i915->kernel_context and allocate it directly (Chris) - More fixes and improvements around wakerefs (Chris) - Clean-up and improvements around debugfs (Chris) - Free the imported shmemfs file f
[PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions
From: Thomas Hellstrom The new header is intended to be used by drivers using the backdoor. Follow the kvm example using alternatives self-patching to choose between vmcall, vmmcall and io instructions. Also define two new CPU feature flags to indicate hypervisor support for vmcall- and vmmcall instructions. Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: "H. Peter Anvin" Cc: Cc: Signed-off-by: Thomas Hellstrom Reviewed-by: Doug Covelli --- MAINTAINERS| 1 + arch/x86/include/asm/cpufeatures.h | 2 ++ arch/x86/include/asm/vmware.h | 48 ++ arch/x86/kernel/cpu/vmware.c | 6 +++- 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 arch/x86/include/asm/vmware.h diff --git a/MAINTAINERS b/MAINTAINERS index c2d975da561f..5bf65a49fa19 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17203,6 +17203,7 @@ M: "VMware, Inc." L: virtualizat...@lists.linux-foundation.org S: Supported F: arch/x86/kernel/cpu/vmware.c +F: arch/x86/include/asm/vmware.h VMWARE PVRDMA DRIVER M: Adit Ranadive diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 998c2cc08363..55fa3b3f0bac 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -232,6 +232,8 @@ #define X86_FEATURE_VMMCALL( 8*32+15) /* Prefer VMMCALL to VMCALL */ #define X86_FEATURE_XENPV ( 8*32+16) /* "" Xen paravirtual guest */ #define X86_FEATURE_EPT_AD ( 8*32+17) /* Intel Extended Page Table access-dirty bit */ +#define X86_FEATURE_VMCALL ( 8*32+18) /* "" Hypervisor supports the VMCALL instruction */ +#define X86_FEATURE_VMW_VMMCALL( 8*32+19) /* "" VMware prefers VMMCALL hypercall instruction */ /* Intel-defined CPU features, CPUID level 0x0007:0 (EBX), word 9 */ #define X86_FEATURE_FSGSBASE ( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/ diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h new file mode 100644 index ..4b220e2bb3e8 --- /dev/null +++ b/arch/x86/include/asm/vmware.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0 or MIT */ +#ifndef _ASM_X86_VMWARE_H +#define _ASM_X86_VMWARE_H + +#include +#include + +/* + * The hypercall definitions differ in the low word of the edx argument in + * the following way: The old port base interface uses the port number to + * distinguish between high- and low bandwidth versions. The new vmcall + * interface instead uses a set of flags to select bandwidth mode and + * transfer direction. New driver code should strictly use the new + * definition of edx content. + */ + +/* Old port-based version */ +#define VMWARE_HYPERVISOR_PORT"0x5658" +#define VMWARE_HYPERVISOR_PORT_HB "0x5659" + +/* Current vmcall / vmmcall version */ +#define VMWARE_HYPERVISOR_HB BIT(0) +#define VMWARE_HYPERVISOR_OUT BIT(1) + +/* The low bandwidth call. The low word of edx is presumed clear. */ +#define VMWARE_HYPERCALL \ + ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT ", %%dx; inl (%%dx)", \ + "vmcall", X86_FEATURE_VMCALL, \ + "vmmcall", X86_FEATURE_VMW_VMMCALL) + +/* + * The high bandwidth out call. The low word of edx is presumed to have the + * HB and OUT bits set. + */ +#define VMWARE_HYPERCALL_HB_OUT \ + ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep outsb", \ + "vmcall", X86_FEATURE_VMCALL, \ + "vmmcall", X86_FEATURE_VMW_VMMCALL) + +/* + * The high bandwidth in call. The low word of edx is presumed to have the + * HB bit set. + */ +#define VMWARE_HYPERCALL_HB_IN \ + ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep insb", \ + "vmcall", X86_FEATURE_VMCALL, \ + "vmmcall", X86_FEATURE_VMW_VMMCALL) +#endif diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c index fcb84b1e099e..abaa1b27353c 100644 --- a/arch/x86/kernel/cpu/vmware.c +++ b/arch/x86/kernel/cpu/vmware.c @@ -31,6 +31,7 @@ #include #include #include +#include #undef pr_fmt #define pr_fmt(fmt)"vmware: " fmt @@ -41,7 +42,6 @@ #define CPUID_VMWARE_FEATURES_ECX_VMCALL BIT(1) #define VMWARE_HYPERVISOR_MAGIC0x564D5868 -#define VMWARE_HYPERVISOR_PORT 0x5658 #define VMWARE_CMD_GETVERSION10 #define VMWARE_CMD_GETHZ 45 @@ -165,6 +165,10 @@ static void __init vmware_set_capabilities(void) { setup_force_cpu_cap(X86_FEATURE_CONSTANT_TSC); setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE); + if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMCALL) + setup_force_cpu_cap(X86_FEATURE_VMCAL
[PATCH v2 4/4] input/vmmouse: Update the backdoor call with support for new instructions
From: Thomas Hellstrom Use the definition provided by include/asm/vmware.h CC: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: "H. Peter Anvin" Cc: Cc: Signed-off-by: Thomas Hellstrom Reviewed-by: Doug Covelli Acked-by: Dmitry Torokhov --- drivers/input/mouse/vmmouse.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c index 871e5b5ab129..148245c69be7 100644 --- a/drivers/input/mouse/vmmouse.c +++ b/drivers/input/mouse/vmmouse.c @@ -16,12 +16,12 @@ #include #include #include +#include #include "psmouse.h" #include "vmmouse.h" #define VMMOUSE_PROTO_MAGIC0x564D5868U -#define VMMOUSE_PROTO_PORT 0x5658 /* * Main commands supported by the vmmouse hypervisor port. @@ -84,7 +84,7 @@ struct vmmouse_data { #define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \ ({ \ unsigned long __dummy1, __dummy2; \ - __asm__ __volatile__ ("inl %%dx" : \ + __asm__ __volatile__ (VMWARE_HYPERCALL :\ "=a"(out1), \ "=b"(out2), \ "=c"(out3), \ @@ -94,7 +94,7 @@ struct vmmouse_data { "a"(VMMOUSE_PROTO_MAGIC), \ "b"(in1), \ "c"(VMMOUSE_PROTO_CMD_##cmd), \ - "d"(VMMOUSE_PROTO_PORT) : \ + "d"(0) :\ "memory"); \ }) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] drm/vmwgfx: Update the backdoor call with support for new instructions
From: Thomas Hellstrom Use the definition provided by include/asm/vmware.h Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: "H. Peter Anvin" Cc: Cc: Signed-off-by: Thomas Hellstrom Reviewed-by: Doug Covelli --- drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 21 + drivers/gpu/drm/vmwgfx/vmwgfx_msg.h | 35 +++-- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c index 81a86c3b77bc..1281e52898ee 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c @@ -45,8 +45,6 @@ #define RETRIES 3 #define VMW_HYPERVISOR_MAGIC0x564D5868 -#define VMW_HYPERVISOR_PORT 0x5658 -#define VMW_HYPERVISOR_HB_PORT 0x5659 #define VMW_PORT_CMD_MSG30 #define VMW_PORT_CMD_HB_MSG 0 @@ -92,7 +90,7 @@ static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol) VMW_PORT(VMW_PORT_CMD_OPEN_CHANNEL, (protocol | GUESTMSG_FLAG_COOKIE), si, di, - VMW_HYPERVISOR_PORT, + 0, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); @@ -125,7 +123,7 @@ static int vmw_close_channel(struct rpc_channel *channel) VMW_PORT(VMW_PORT_CMD_CLOSE_CHANNEL, 0, si, di, - (VMW_HYPERVISOR_PORT | (channel->channel_id << 16)), + channel->channel_id << 16, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); @@ -159,7 +157,8 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel, VMW_PORT_HB_OUT( (MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG, msg_len, si, di, - VMW_HYPERVISOR_HB_PORT | (channel->channel_id << 16), + VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) | + VMWARE_HYPERVISOR_OUT, VMW_HYPERVISOR_MAGIC, bp, eax, ebx, ecx, edx, si, di); @@ -180,7 +179,7 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel, VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_SENDPAYLOAD << 16), word, si, di, -VMW_HYPERVISOR_PORT | (channel->channel_id << 16), +channel->channel_id << 16, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); } @@ -212,7 +211,7 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply, VMW_PORT_HB_IN( (MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG, reply_len, si, di, - VMW_HYPERVISOR_HB_PORT | (channel->channel_id << 16), + VMWARE_HYPERVISOR_HB | (channel->channel_id << 16), VMW_HYPERVISOR_MAGIC, bp, eax, ebx, ecx, edx, si, di); @@ -229,7 +228,7 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply, VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_RECVPAYLOAD << 16), MESSAGE_STATUS_SUCCESS, si, di, -VMW_HYPERVISOR_PORT | (channel->channel_id << 16), +channel->channel_id << 16, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); @@ -268,7 +267,7 @@ static int vmw_send_msg(struct rpc_channel *channel, const char *msg) VMW_PORT(VMW_PORT_CMD_SENDSIZE, msg_len, si, di, - VMW_HYPERVISOR_PORT | (channel->channel_id << 16), + channel->channel_id << 16, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); @@ -326,7 +325,7 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg, VMW_PORT(VMW_PORT_CMD_RECVSIZE, 0, si, di, - (VMW_HYPERVISOR_PORT | (channel->channel_id << 16)), + channel->channel_id << 16, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); @@ -370,7 +369,7 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg, VMW_PORT(VMW_PORT_CMD_RECVSTATUS, MESSAGE_STATUS_SUCCESS, si, di, - (VMW_HYPERVISOR_PORT | (channel->channel_id << 16)), + channel->channel_id << 16, VMW_HYPERVISOR_MAGIC, eax, ebx, ecx, edx, si, di); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h index 4907e50fb20a..f685c7071dec 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h @@ -32,6 +32,7 @@
Re: [PATCH 3/3] drm/i915: switch to drm_fb_helper_remove_conflicting_pci_framebuffers
On Thu, Aug 22, 2019 at 03:13:14PM +0200, Daniel Vetter wrote: > On Thu, Aug 22, 2019 at 11:06:45AM +0200, Gerd Hoffmann wrote: > > No need for a home-grown version, the generic helper should work just > > fine. It also handles vgacon removal these days, see commit > > 1c74ca7a1a9a ("drm/fb-helper: call vga_remove_vgacon automatically."), > > so that can be removed too. > > > > Signed-off-by: Gerd Hoffmann > > Reviewed-by: Daniel Vetter > > I think I'd still wait until 5.4-rc1 with merging this one, just to give > another full release and people to test it before we pull the trigger. > Overabundance of caution and all that. Whole series or just the i915 patch? cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #34 from Andrew Shark --- @Tomas, it is strange that it did not asked a password after "Creating local repository..." which is before "Installing OpenGL PRO...". Probably you already had a previous version of repository (it happens if you try to install it with official installer script first). Try to force remove it (script provided as another attachment) and then re-run my installer script. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/i915: switch to drm_fb_helper_remove_conflicting_pci_framebuffers
On Fri, Aug 23, 2019 at 10:14 AM Gerd Hoffmann wrote: > On Thu, Aug 22, 2019 at 03:13:14PM +0200, Daniel Vetter wrote: > > On Thu, Aug 22, 2019 at 11:06:45AM +0200, Gerd Hoffmann wrote: > > > No need for a home-grown version, the generic helper should work just > > > fine. It also handles vgacon removal these days, see commit > > > 1c74ca7a1a9a ("drm/fb-helper: call vga_remove_vgacon automatically."), > > > so that can be removed too. > > > > > > Signed-off-by: Gerd Hoffmann > > > > Reviewed-by: Daniel Vetter > > > > I think I'd still wait until 5.4-rc1 with merging this one, just to give > > another full release and people to test it before we pull the trigger. > > Overabundance of caution and all that. > > Whole series or just the i915 patch? Ok I just checked and this all landed in 5.1 already, I thought it was more recent. I think that's good enough, push it all without more waiting. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] kernel.h: Add non_block_start/end()
On Fri, Aug 23, 2019 at 1:14 AM Andrew Morton wrote: > > On Tue, 20 Aug 2019 22:24:40 +0200 Daniel Vetter wrote: > > > Hi Peter, > > > > Iirc you've been involved at least somewhat in discussing this. -mm folks > > are a bit undecided whether these new non_block semantics are a good idea. > > Michal Hocko still is in support, but Andrew Morton and Jason Gunthorpe > > are less enthusiastic. Jason said he's ok with merging the hmm side of > > this if scheduler folks ack. If not, then I'll respin with the > > preempt_disable/enable instead like in v1. > > I became mollified once Michel explained the rationale. I think it's > OK. It's very specific to the oom reaper and hopefully won't be used > more widely(?). Yeah, no plans for that from me. And I hope the comment above them now explains why they exist, so people think twice before using it in random places. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Daniel, Dave, Here is what should be the final drm-misc-next PR for 5.4. Thanks! Maxime drm-misc-next-2019-08-23: drm-misc-next for 5.4: UAPI Changes: Cross-subsystem Changes: Core Changes: - dma-buf: dma-fence selftests Driver Changes: - kirin: Various cleanups and reworks - komeda: Add support for DT memory-regions - meson: Rely on the compatible to detect vpu features - omap: Implement alpha and pixel blend mode properties - panfrost: Implement per-fd address spaces, various fixes - rockchip: DSI DT binding rework - fbdev: Various cleanups The following changes since commit d777478599f781fc5162d1ae95dbee6e5ae05a41: drm/xen-front: Make structure fb_funcs constant (2019-08-19 08:32:52 +0300) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-08-23 for you to fetch changes up to e26ae7c0432101a924cf745b07470c8592de64cb: omapdrm: no need to check return value of debugfs_create functions (2019-08-23 10:21:03 +0300) drm-misc-next for 5.4: UAPI Changes: Cross-subsystem Changes: Core Changes: - dma-buf: dma-fence selftests Driver Changes: - kirin: Various cleanups and reworks - komeda: Add support for DT memory-regions - meson: Rely on the compatible to detect vpu features - omap: Implement alpha and pixel blend mode properties - panfrost: Implement per-fd address spaces, various fixes - rockchip: DSI DT binding rework - fbdev: Various cleanups Anders Roxell (1): video: fbdev: sh_mobile_lcdcfb: Mark expected switch fall-through Chris Wilson (5): dma-buf: Introduce selftesting framework dma-buf: Add selftests for dma-fence drm/i915: Select DMABUF_SELFTESTS for the default i915.ko debug build dma-buf: Use %zu for printing sizeof dmabuf: Mark up onstack timer for selftests Chuhong Yuan (2): video: fbdev: sm712fb: Use dev_get_drvdata video: fbdev: radeonfb: Use dev_get_drvdata Da Lv (1): drm: kirin: Fix for hikey620 display offset problem Dariusz Marcinkiewicz (2): drm: dw-hdmi: use cec_notifier_conn_(un)register dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Greg Kroah-Hartman (1): omapdrm: no need to check return value of debugfs_create functions Gustavo A. R. Silva (2): video: fbdev: pvr2fb: remove unnecessary comparison of unsigned integer with < 0 video: fbdev/mmp/core: Use struct_size() in kzalloc() Hans de Goede (1): efifb: BGRT: Improve efifb_bgrt_sanity_check Jani Nikula (1): drm: fix module name in edid_firmware log message Jean-Jacques Hiblot (1): drm/omap: Add 'alpha' and 'pixel blend mode' plane properties John Stultz (3): drm: kirin: Remove HISI_KIRIN_DW_DSI config option drm: kirin: Remove unreachable return drm: kirin: Move workqueue to ade_hw_ctx structure Jonathan Neuschäfer (1): drm/drv: Use // for comments in example code Julien Masson (1): drm: meson: use match data to detect vpu compatibility Mihail Atanassov (1): drm/komeda: Add support for 'memory-region' DT node property Nickey Yang (1): dt-bindings: display: rockchip: update DSI controller Nishka Dasgupta (1): udlfb: Make dlfb_ops constant Rob Herring (2): drm/panfrost: Implement per FD address spaces drm/panfrost: Fix sleeping while atomic in panfrost_gem_open Souptick Joarder (2): video: fbdev: aty[128]fb: Remove dead code video: fbdev: viafb: Remove dead code Steven Price (2): drm/panfrost: Enable devfreq to work without regulator drm/panfrost: Remove opp table when unloading Thierry Reding (1): drm/nouveau: Initialize GEM object before TTM object Wei Yongjun (1): drm/panfrost: Fix missing unlock on error in panfrost_mmu_map_fault_addr() Xu YiPing (21): drm: kirin: Remove uncessary parameter indirection drm: kirin: Remove out_format from ade_crtc drm: kirin: Rename ade_plane to kirin_plane drm: kirin: Rename ade_crtc to kirin_crtc drm: kirin: Dynamically allocate the hw_ctx drm: kirin: Move request irq handle in ade hw ctx alloc drm: kirin: Move kirin_crtc, kirin_plane, kirin_format to kirin_drm_drv.h drm: kirin: Reanme dc_ops to kirin_drm_data drm: kirin: Move ade crtc/plane help functions to driver_data drm: kirin: Move channel formats to driver data drm: kirin: Move mode config function to driver_data drm: kirin: Move plane number and primay plane in driver data drm: kirin: Move config max_width and max_height to driver data drm: kirin: Move drm driver to driver data drm: kirin: Add register connect helper functions in drm init drm: kirin: Rename plane_init and crtc_init drm: kirin: Fix dev->driver_data setting drm: kirin: Make driver_data variable non-global
Re: [PATCH 3/4] kernel.h: Add non_block_start/end()
On Tue, Aug 20, 2019 at 10:24:40PM +0200, Daniel Vetter wrote: > On Tue, Aug 20, 2019 at 10:19:01AM +0200, Daniel Vetter wrote: > > In some special cases we must not block, but there's not a > > spinlock, preempt-off, irqs-off or similar critical section already > > that arms the might_sleep() debug checks. Add a non_block_start/end() > > pair to annotate these. > > > > This will be used in the oom paths of mmu-notifiers, where blocking is > > not allowed to make sure there's forward progress. Quoting Michal: > > > > "The notifier is called from quite a restricted context - oom_reaper - > > which shouldn't depend on any locks or sleepable conditionals. The code > > should be swift as well but we mostly do care about it to make a forward > > progress. Checking for sleepable context is the best thing we could come > > up with that would describe these demands at least partially." > > > > Peter also asked whether we want to catch spinlocks on top, but Michal > > said those are less of a problem because spinlocks can't have an > > indirect dependency upon the page allocator and hence close the loop > > with the oom reaper. > > > > Suggested by Michal Hocko. > > > > v2: > > - Improve commit message (Michal) > > - Also check in schedule, not just might_sleep (Peter) > > > > v3: It works better when I actually squash in the fixup I had lying > > around :-/ > > > > v4: Pick the suggestion from Andrew Morton to give non_block_start/end > > some good kerneldoc comments. I added that other blocking calls like > > wait_event pose similar issues, since that's the other example we > > discussed. > > > > Cc: Jason Gunthorpe > > Cc: Peter Zijlstra > > Cc: Ingo Molnar > > Cc: Andrew Morton > > Cc: Michal Hocko > > Cc: David Rientjes > > Cc: "Christian König" > > Cc: Daniel Vetter > > Cc: "Jérôme Glisse" > > Cc: linux...@kvack.org > > Cc: Masahiro Yamada > > Cc: Wei Wang > > Cc: Andy Shevchenko > > Cc: Thomas Gleixner > > Cc: Jann Horn > > Cc: Feng Tang > > Cc: Kees Cook > > Cc: Randy Dunlap > > Cc: linux-ker...@vger.kernel.org > > Acked-by: Christian König (v1) > > Signed-off-by: Daniel Vetter > > Hi Peter, > > Iirc you've been involved at least somewhat in discussing this. -mm folks > are a bit undecided whether these new non_block semantics are a good idea. > Michal Hocko still is in support, but Andrew Morton and Jason Gunthorpe > are less enthusiastic. Jason said he's ok with merging the hmm side of > this if scheduler folks ack. If not, then I'll respin with the > preempt_disable/enable instead like in v1. > > So ack/nack for this from the scheduler side? Right, I had memories of seeing this before, and I just found a fairly long discussion on this elsewhere in the vacation inbox (*groan*). Yeah, this is something I can live with, Acked-by: Peter Zijlstra (Intel) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/i915: switch to drm_fb_helper_remove_conflicting_pci_framebuffers
On Fri, Aug 23, 2019 at 10:30:35AM +0200, Daniel Vetter wrote: > On Fri, Aug 23, 2019 at 10:14 AM Gerd Hoffmann wrote: > > > > Whole series or just the i915 patch? > > Ok I just checked and this all landed in 5.1 already, I thought it was > more recent. I think that's good enough, push it all without more > waiting. Pushed to drm-misc-next. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #35 from Etienne Lorrain --- (In reply to Andrew Shark from comment #31) > (In reply to Etienne Lorrain from comment #30) > I do not understand what problem you are talking about. Package just refuses > to install because of explicitly doing so in preinst script. My script > solves it. Sorry, it is some time ago and now that my system is working I am reluctant to break it again to see if I can re-install again. I use your script to install on 19.4, I just had: > but leads to a > Segmentation fault at address 0x0 of /usr/lib/xorg/Xorg seen in > .local/share/xorg/Xorg.0.log That I fixed by commenting out the line: ModulePath "/opt/amdgpu/lib/xorg/modules" in 00-amdgpu.conf I am not sure why I cannot use files in "/opt/amdgpu/lib/xorg/modules", I have to use the files in "/usr/lib/xorg/modules/" (same filenames, different files)... Cheers. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: Add Steven and Alyssa as panfrost reviewers
On 23/08/2019 03:33, Rob Herring wrote: > Add Steven Price and Alyssa Rosenzweig as reviewers as they have been the > primary reviewers already. > > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: Tomeu Vizoso > Signed-off-by: Rob Herring > --- > MAINTAINERS | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 67b75fc33c61..28f4a20940cb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1255,6 +1255,8 @@ F: Documentation/gpu/afbc.rst > ARM MALI PANFROST DRM DRIVER > M: Rob Herring > M: Tomeu Vizoso > +R: Steven Price > +R: Alyssa Rosenzweig > L: dri-devel@lists.freedesktop.org > S: Supported > T: git git://anongit.freedesktop.org/drm/drm-misc > Acked-by: Neil Armstrong ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panfrost: Add missing check for pfdev->regulator
On 23/08/2019 02:52, Rob Herring wrote: > On Thu, Aug 22, 2019 at 4:32 AM Steven Price wrote: >> >> When modifying panfrost_devfreq_target() to support a device without a >> regulator defined I missed the check on the error path. Let's add it. >> >> Reported-by: Dan Carpenter >> Fixes: e21dd290881b ("drm/panfrost: Enable devfreq to work without >> regulator") >> Signed-off-by: Steven Price >> --- >> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) > > Looks fine to me, but seems to be delayed getting to the list and > patchwork. I'm guessing you're not subscribed to dri-devel because all > your patches seem to get delayed. Ah, yes I'm subscribed with a different email address - hopefully now also subscribed with my @arm.com one. Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: Add Steven and Alyssa as panfrost reviewers
On 23/08/2019 02:33, Rob Herring wrote: > Add Steven Price and Alyssa Rosenzweig as reviewers as they have been the > primary reviewers already. > > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: Tomeu Vizoso > Signed-off-by: Rob Herring Acked-by: Steven Price Steve > --- > MAINTAINERS | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 67b75fc33c61..28f4a20940cb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1255,6 +1255,8 @@ F: Documentation/gpu/afbc.rst > ARM MALI PANFROST DRM DRIVER > M: Rob Herring > M: Tomeu Vizoso > +R: Steven Price > +R: Alyssa Rosenzweig > L: dri-devel@lists.freedesktop.org > S: Supported > T: git git://anongit.freedesktop.org/drm/drm-misc > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: Signed-off-by missing for commit in the drm tree
Hi all, Commit 88b703527ba7 ("drm/nouveau/kms/gf119-: add ctm property support") is missing a Signed-off-by from its committer. -- Cheers, Stephen Rothwell pgpFK0PXpb9VH.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/6] drm/i915/dp: Support for DP HDR outputs
Support for HDR10 video was introduced in DisplayPort 1.4. On GLK+ platform, in order to use DisplayPort HDR10, we need to support BT.2020 colorimetry and HDR Static metadata. It implements the CTA-861-G standard for transport of static HDR metadata. It enables writing of HDR metadata infoframe SDP to the panel. The HDR Metadata will be provided by userspace compositors, based on blending policies and passed to the driver through a blob property. And It refactors, renames and extends a function which handled vsc sdp header and data block setup for supporting colorimetry format. And It attaches the colorspace connector property and HDR metadata property to a DisplayPort connector. These patches tested on below test environment. Test Environment: - Tested System: GLK and Gen11 platform. - Monitor: Dell UP2718Q 4K HDR Monitor. - In order to test DP HDR10, test environment uses patched Kodi-gbm, patched Media driver and HDR10 video. You can find these on below. [patched Kodi-gbm] - repo: https://github.com/Kwiboo/xbmc/tree/drmprime-hdr [download 4K HDR video file] - link: https://4kmedia.org/lg-new-york-hdr-uhd-4k-demo/ [Media Driver for GLK] - repo https://gitlab.freedesktop.org/emersion/intel-vaapi-driver master branch [Media Driver for ICL] - repo: https://github.com/harishkrupo/media-driver/tree/p010_composite v2: - Add a missed blank line after function declaration. - Remove useless parentheses. - Minor style fix. Gwan-gyeong Mun (6): drm/i915/dp: Extend program of VSC Header and DB for Colorimetry Format drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA drm: Add DisplayPort colorspace property drm/i915/dp: Attach colorspace property drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata drm/i915/dp: Attach HDR metadata property to DP connector drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/i915/display/intel_ddi.c | 11 +- drivers/gpu/drm/i915/display/intel_display.h | 2 - .../drm/i915/display/intel_display_types.h| 3 + drivers/gpu/drm/i915/display/intel_dp.c | 191 -- drivers/gpu/drm/i915/display/intel_dp.h | 7 + drivers/gpu/drm/i915/i915_reg.h | 1 + 7 files changed, 199 insertions(+), 20 deletions(-) -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/6] drm/i915/dp: Program an Infoframe SDP Header and DB for HDR Static Metadata
Function intel_dp_setup_hdr_metadata_infoframe_sdp handles Infoframe SDP header and data block setup for HDR Static Metadata. It enables writing of HDR metadata infoframe SDP to panel. Support for HDR video was introduced in DisplayPort 1.4. It implements the CTA-861-G standard for transport of static HDR metadata. The HDR Metadata will be provided by userspace compositors, based on blending policies and passed to the driver through a blob property. Because each of GEN11 and prior GEN11 have different register size for HDR Metadata Infoframe SDP packet, it adds and uses different register size. Setup Infoframe SDP header and data block in function intel_dp_setup_hdr_metadata_infoframe_sdp for HDR Static Metadata as per dp 1.4 spec and CTA-861-F spec. As per DP 1.4 spec, 2.2.2.5 SDP Formats. It enables Dynamic Range and Mastering Infoframe for HDR content, which is defined in CTA-861-F spec. According to DP 1.4 spec and CEA-861-F spec Table 5, in order to transmit static HDR metadata, we have to use Non-audio INFOFRAME SDP v1.3. ++---+ | [ Packet Type Value ] | [ Packet Type ] | ++---+ | 80h + Non-audio INFOFRAME Type | CEA-861-F Non-audio INFOFRAME | ++---+ | [Transmission Timing] | ++ | As per CEA-861-F for INFOFRAME, including CEA-861.3 within | | which Dynamic Range and Mastering INFOFRAME are defined| ++ v2: Add a missed blank line after function declaration. Signed-off-by: Gwan-gyeong Mun --- drivers/gpu/drm/i915/display/intel_ddi.c | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 91 drivers/gpu/drm/i915/display/intel_dp.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 1 + 4 files changed, 96 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 5c42b58c1c2f..9f5bea941bcd 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3478,6 +3478,7 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder, intel_edp_backlight_on(crtc_state, conn_state); intel_psr_enable(intel_dp, crtc_state); intel_dp_vsc_enable(intel_dp, crtc_state, conn_state); + intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state); intel_edp_drrs_enable(intel_dp, crtc_state); if (crtc_state->has_audio) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7218e159f55d..00da8221eaba 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4554,6 +4554,85 @@ intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp, crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp)); } +static int +intel_dp_setup_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) +{ + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); + struct dp_sdp infoframe_sdp = {}; + struct hdmi_drm_infoframe drm_infoframe = {}; + const int infoframe_size = HDMI_INFOFRAME_HEADER_SIZE + HDMI_DRM_INFOFRAME_SIZE; + unsigned char buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_DRM_INFOFRAME_SIZE]; + ssize_t len; + int ret; + + ret = drm_hdmi_infoframe_set_hdr_metadata(&drm_infoframe, conn_state); + if (ret) { + DRM_DEBUG_KMS("couldn't set HDR metadata in infoframe\n"); + return ret; + } + + len = hdmi_drm_infoframe_pack_only(&drm_infoframe, buf, sizeof(buf)); + if (len < 0) { + DRM_DEBUG_KMS("buffer size is smaller than hdr metadata infoframe\n"); + return (int)len; + } + + if (len != infoframe_size) { + DRM_DEBUG_KMS("wrong static hdr metadata size\n"); + return -EINVAL; + } + + /* +* Set up the infoframe sdp packet for HDR static metadata. +* Prepare VSC Header for SU as per DP 1.4a spec, +* Table 2-100 and Table 2-101 +*/ + + /* Packet ID, 00h for non-Audio INFOFRAME */ + infoframe_sdp.sdp_header.HB0 = 0; + /* +* Packet Type 80h + Non-audio INFOFRAME Type value +* HDMI_INFOFRAME_TYPE_DRM: 0x87, +*/ + infoframe_sdp.sdp_header.HB1 = drm_infoframe.type; + /* +* Least Significant Eight Bits of (Data Byte Count – 1) +* infoframe_size - 1, +
[PATCH v2 1/6] drm/i915/dp: Extend program of VSC Header and DB for Colorimetry Format
It refactors and renames a function which handled vsc sdp header and data block setup for supporting colorimetry format. Function intel_dp_setup_vsc_sdp handles vsc sdp header and data block setup for pixel encoding / colorimetry format. In order to use colorspace information of a connector, it adds an argument of drm_connector_state type. Setup VSC header and data block in function intel_dp_setup_vsc_sdp for pixel encoding / colorimetry format as per dp 1.4a spec, section 2.2.5.7.1, table 2-119: VSC SDP Header Bytes, section 2.2.5.7.5, table 2-120: VSC SDP Payload for DB16 through DB18. Signed-off-by: Gwan-gyeong Mun --- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- drivers/gpu/drm/i915/display/intel_display.h | 2 - drivers/gpu/drm/i915/display/intel_dp.c | 68 drivers/gpu/drm/i915/display/intel_dp.h | 3 + 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 8eb2b3ec01ed..4f7ea0a35976 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3475,7 +3475,7 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder, intel_edp_backlight_on(crtc_state, conn_state); intel_psr_enable(intel_dp, crtc_state); - intel_dp_ycbcr_420_enable(intel_dp, crtc_state); + intel_dp_vsc_enable(intel_dp, crtc_state, conn_state); intel_edp_drrs_enable(intel_dp, crtc_state); if (crtc_state->has_audio) diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index e57e6969051d..7bd59241fc32 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -499,8 +499,6 @@ void intel_dp_get_m_n(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, enum link_m_n_set m_n); -void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, - const struct intel_crtc_state *crtc_state); int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n); bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, struct dpll *best_clock); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 5c45a3bb102d..55d5ab97061c 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4409,8 +4409,9 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, } static void -intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp, - const struct intel_crtc_state *crtc_state) +intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct dp_sdp vsc_sdp = {}; @@ -4431,13 +4432,55 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp, */ vsc_sdp.sdp_header.HB3 = 0x13; - /* -* YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h -* DB16[3:0] DP 1.4a spec, Table 2-120 -*/ - vsc_sdp.db[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/ - /* RGB->YCBCR color conversion uses the BT.709 color space. */ - vsc_sdp.db[16] |= 0x1; /* 0x1, ITU-R BT.709 */ + /* DP 1.4a spec, Table 2-120 */ + switch (crtc_state->output_format) { + case INTEL_OUTPUT_FORMAT_YCBCR444: + vsc_sdp.db[16] = 0x1 << 4; /* YCbCr 444 : DB16[7:4] = 1h */ + break; + case INTEL_OUTPUT_FORMAT_YCBCR420: + vsc_sdp.db[16] = 0x3 << 4; /* YCbCr 420 : DB16[7:4] = 3h */ + break; + case INTEL_OUTPUT_FORMAT_RGB: + default: + /* RGB: DB16[7:4] = 0h */ + break; + } + + switch (conn_state->colorspace) { + case DRM_MODE_COLORIMETRY_BT709_YCC: + vsc_sdp.db[16] |= 0x1; + break; + case DRM_MODE_COLORIMETRY_XVYCC_601: + vsc_sdp.db[16] |= 0x2; + break; + case DRM_MODE_COLORIMETRY_XVYCC_709: + vsc_sdp.db[16] |= 0x3; + break; + case DRM_MODE_COLORIMETRY_SYCC_601: + vsc_sdp.db[16] |= 0x4; + break; + case DRM_MODE_COLORIMETRY_OPYCC_601: + vsc_sdp.db[16] |= 0x5; + break; + case DRM_MODE_COLORIMETRY_BT2020_CYCC: + case DRM_MODE_COLORIMETRY_BT2020_RGB: + vsc_sdp.db[16] |= 0x6; + break; + case DRM_MODE_COLORIMETRY_BT2020_YCC: + vsc_sdp.db[16] |= 0x7; + break; + case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65: + case D
[PATCH v2 6/6] drm/i915/dp: Attach HDR metadata property to DP connector
It attaches HDR metadata property to DP connector on GLK+. It enables HDR metadata infoframe sdp on GLK+ to be used to send HDR metadata to DP sink. v2: Minor style fix Signed-off-by: Gwan-gyeong Mun --- drivers/gpu/drm/i915/display/intel_dp.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 00da8221eaba..899923ecd1c9 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -6495,6 +6495,11 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect intel_attach_colorspace_property(connector); + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11) + drm_object_attach_property(&connector->base, + connector->dev->mode_config.hdr_output_metadata_property, + 0); + if (intel_dp_is_edp(intel_dp)) { u32 allowed_scalers; -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/6] drm/i915/dp: Add support of BT.2020 Colorimetry to DP MSA
When BT.2020 Colorimetry output is used for DP, we should program BT.2020 Colorimetry to MSA and VSC SDP. It adds output_colorspace to intel_crtc_state struct as a place holder of pipe's output colorspace. In order to distinguish needed colorimetry for VSC SDP, it adds intel_dp_needs_vsc_colorimetry function. If the output colorspace requires vsc sdp or output format is YCbCr 4:2:0, it uses MSA with VSC SDP. As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication of Color Encoding Format and Content Color Gamut] while sending BT.2020 Colorimetry signals we should program MSA MISC1 fields which indicate VSC SDP for the Pixel Encoding/Colorimetry Format. v2: Remove useless parentheses Signed-off-by: Gwan-gyeong Mun --- drivers/gpu/drm/i915/display/intel_ddi.c | 8 +++--- .../drm/i915/display/intel_display_types.h| 3 +++ drivers/gpu/drm/i915/display/intel_dp.c | 25 ++- drivers/gpu/drm/i915/display/intel_dp.h | 1 + 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 4f7ea0a35976..5c42b58c1c2f 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1737,11 +1737,13 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state) /* * As per DP 1.4a spec section 2.2.4.3 [MSA Field for Indication * of Color Encoding Format and Content Color Gamut] while sending -* YCBCR 420 signals we should program MSA MISC1 fields which -* indicate VSC SDP for the Pixel Encoding/Colorimetry Format. +* YCBCR 420, HDR BT.2020 signals we should program MSA MISC1 fields +* which indicate VSC SDP for the Pixel Encoding/Colorimetry Format. */ - if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) + if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 || + intel_dp_needs_vsc_colorimetry(crtc_state->output_colorspace)) temp |= TRANS_MSA_USE_VSC_SDP; + I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp); } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 449abaea619f..9845abcf6f29 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -964,6 +964,9 @@ struct intel_crtc_state { /* Output format RGB/YCBCR etc */ enum intel_output_format output_format; + /* Output colorspace sRGB/BT.2020 etc */ + u32 output_colorspace; + /* Output down scaling is done in LSPCON device */ bool lspcon_downsampling; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 55d5ab97061c..295d5ed2be96 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2164,6 +2164,8 @@ intel_dp_compute_config(struct intel_encoder *encoder, pipe_config->has_pch_encoder = true; pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB; + pipe_config->output_colorspace = intel_conn_state->base.colorspace; + if (lspcon->active) lspcon_ycbcr420_config(&intel_connector->base, pipe_config); else @@ -4408,6 +4410,26 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, return 0; } +bool +intel_dp_needs_vsc_colorimetry(u32 colorspace) +{ + bool ret = false; + + switch (colorspace) { + case DRM_MODE_COLORIMETRY_SYCC_601: + case DRM_MODE_COLORIMETRY_OPYCC_601: + case DRM_MODE_COLORIMETRY_BT2020_YCC: + case DRM_MODE_COLORIMETRY_BT2020_RGB: + case DRM_MODE_COLORIMETRY_BT2020_CYCC: + ret = true; + break; + default: + break; + } + + return ret; +} + static void intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state, @@ -4536,7 +4558,8 @@ void intel_dp_vsc_enable(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state) { - if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420) + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 && + !intel_dp_needs_vsc_colorimetry(conn_state->colorspace)) return; intel_dp_setup_vsc_sdp(intel_dp, crtc_state, conn_state); diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h index 91a0ee6058fe..b2da7c9998f7 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.h +++ b/drivers/gpu/drm/i915/display/intel_dp.h @@ -111,6 +111,7 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp); bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp); int intel_dp_link_required(int pixel_cl
[PATCH v2 3/6] drm: Add DisplayPort colorspace property
In order to use colorspace property to Display Port connectors, it extends DRM_MODE_CONNECTOR_DisplayPort connector_type on drm_mode_create_colorspace_property function. Signed-off-by: Gwan-gyeong Mun --- drivers/gpu/drm/drm_connector.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 4c766624b20d..655ada9d4c16 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1703,7 +1703,9 @@ int drm_mode_create_colorspace_property(struct drm_connector *connector) struct drm_property *prop; if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || - connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) { + connector->connector_type == DRM_MODE_CONNECTOR_HDMIB || + connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, "Colorspace", hdmi_colorspaces, -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] drm/i915/dp: Attach colorspace property
It attaches the colorspace connector property to a DisplayPort connector. Based on colorspace change, modeset will be triggered to switch to a new colorspace. Based on colorspace property value create a VSC SDP packet with appropriate colorspace. This would help to enable wider color gamut like BT2020 on a sink device. Signed-off-by: Gwan-gyeong Mun --- drivers/gpu/drm/i915/display/intel_dp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 295d5ed2be96..7218e159f55d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -6402,6 +6402,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect else if (INTEL_GEN(dev_priv) >= 5) drm_connector_attach_max_bpc_property(connector, 6, 12); + intel_attach_colorspace_property(connector); + if (intel_dp_is_edp(intel_dp)) { u32 allowed_scalers; -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 03/18] drm/virtio: simplify cursor updates
No need to do the reservation dance, we can just wait on the fence directly. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_plane.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index a492ac3f4a7e..11539b66c6f2 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -186,7 +186,6 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, struct virtio_gpu_framebuffer *vgfb; struct virtio_gpu_object *bo = NULL; uint32_t handle; - int ret = 0; if (plane->state->crtc) output = drm_crtc_to_virtio_gpu_output(plane->state->crtc); @@ -210,15 +209,9 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, cpu_to_le32(plane->state->crtc_w), cpu_to_le32(plane->state->crtc_h), 0, 0, vgfb->fence); - ret = virtio_gpu_object_reserve(bo, false); - if (!ret) { - dma_resv_add_excl_fence(bo->tbo.base.resv, - &vgfb->fence->f); - dma_fence_put(&vgfb->fence->f); - vgfb->fence = NULL; - virtio_gpu_object_unreserve(bo); - virtio_gpu_object_wait(bo, false); - } + dma_fence_wait(&vgfb->fence->f, true); + dma_fence_put(&vgfb->fence->f); + vgfb->fence = NULL; } if (plane->state->fb != old_state->fb) { -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression
Hi Am 22.08.19 um 22:02 schrieb Dave Airlie: > On Fri, 23 Aug 2019 at 03:25, Thomas Zimmermann wrote: >> >> Hi >> >> I was traveling and could reply earlier. Sorry for taking so long. >> >> Am 13.08.19 um 11:36 schrieb Feng Tang: >>> Hi Thomas, >>> >>> On Mon, Aug 12, 2019 at 03:25:45PM +0800, Feng Tang wrote: Hi Thomas, On Fri, Aug 09, 2019 at 04:12:29PM +0800, Rong Chen wrote: > Hi, > >>> Actually we run the benchmark as a background process, do we need to >>> disable the cursor and test again? >> There's a worker thread that updates the display from the shadow buffer. >> The blinking cursor periodically triggers the worker thread, but the >> actual update is just the size of one character. >> >> The point of the test without output is to see if the regression comes > >from the buffer update (i.e., the memcpy from shadow buffer to VRAM), or > >from the worker thread. If the regression goes away after disabling the >> blinking cursor, then the worker thread is the problem. If it already >> goes away if there's simply no output from the test, the screen update >> is the problem. On my machine I have to disable the blinking cursor, so >> I think the worker causes the performance drop. > > We disabled redirecting stdout/stderr to /dev/kmsg, and the regression is > gone. > > commit: > f1f8555dfb9 drm/bochs: Use shadow buffer for bochs framebuffer console > 90f479ae51a drm/mgag200: Replace struct mga_fbdev with generic > framebuffer > emulation > > f1f8555dfb9a70a2 90f479ae51afa45efab97afdde testcase/testparams/testbox > -- --- > %stddev change %stddev > \ |\ > 43785 44481 > vm-scalability/300s-8T-anon-cow-seq-hugetlb/lkp-knm01 > 43785 44481GEO-MEAN > vm-scalability.median Till now, from Rong's tests: 1. Disabling cursor blinking doesn't cure the regression. 2. Disabling printint test results to console can workaround the regression. Also if we set the perfer_shadown to 0, the regression is also gone. >>> >>> We also did some further break down for the time consumed by the >>> new code. >>> >>> The drm_fb_helper_dirty_work() calls sequentially >>> 1. drm_client_buffer_vmap (290 us) >>> 2. drm_fb_helper_dirty_blit_real (19240 us) >>> 3. helper->fb->funcs->dirty()---> NULL for mgag200 driver >>> 4. drm_client_buffer_vunmap (215 us) >>> >> >> It's somewhat different to what I observed, but maybe I just couldn't >> reproduce the problem correctly. >> >>> The average run time is listed after the function names. >>> >>> From it, we can see drm_fb_helper_dirty_blit_real() takes too long >>> time (about 20ms for each run). I guess this is the root cause >>> of this regression, as the original code doesn't use this dirty worker. >> >> True, the original code uses a temporary buffer, but updates the display >> immediately. >> >> My guess is that this could be a caching problem. The worker runs on a >> different CPU, which doesn't have the shadow buffer in cache. >> >>> As said in last email, setting the prefer_shadow to 0 can avoid >>> the regrssion. Could it be an option? >> >> Unfortunately not. Without the shadow buffer, the console's display >> buffer permanently resides in video memory. It consumes significant >> amount of that memory (say 8 MiB out of 16 MiB). That doesn't leave >> enough room for anything else. >> >> The best option is to not print to the console. > > Wait a second, I thought the driver did an eviction on modeset of the > scanned out object, this was a deliberate design decision made when > writing those drivers, has this been removed in favour of gem and > generic code paths? Yes. We added back this feature for testing in [1]. It was only an improvement of ~1% compared to the original report. I wouldn't mind landing this patch set, but it probably doesn't make a difference either. Best regards Thomas [1] https://lists.freedesktop.org/archives/dri-devel/2019-August/228950.html > > Dave. > -- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg) signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 10/18] drm/virtio: rework virtio_gpu_transfer_from_host_ioctl fencing
Switch to the virtio_gpu_array_* helper workflow. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 40 ++ drivers/gpu/drm/virtio/virtgpu_vq.c| 8 -- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index c6ea65f6507e..fa568cbfdddc 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -323,9 +323,10 @@ void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev, struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, - uint32_t resource_id, uint32_t ctx_id, + uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, + struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, struct virtio_gpu_object *bo, diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 6baf43148e01..444696f73e96 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -340,9 +340,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv = file->driver_priv; struct drm_virtgpu_3d_transfer_from_host *args = data; - struct ttm_operation_ctx ctx = { true, false }; - struct drm_gem_object *gobj = NULL; - struct virtio_gpu_object *qobj = NULL; + struct virtio_gpu_object_array *objs; struct virtio_gpu_fence *fence; int ret; u32 offset = args->offset; @@ -351,39 +349,31 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, if (vgdev->has_virgl_3d == false) return -ENOSYS; - gobj = drm_gem_object_lookup(file, args->bo_handle); - if (gobj == NULL) + objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1); + if (objs == NULL) return -ENOENT; - qobj = gem_to_virtio_gpu_obj(gobj); - - ret = virtio_gpu_object_reserve(qobj); - if (ret) - goto out; - - ret = ttm_bo_validate(&qobj->tbo, &qobj->placement, &ctx); - if (unlikely(ret)) - goto out_unres; + ret = virtio_gpu_array_lock_resv(objs); + if (ret != 0) + goto err_put_free; convert_to_hw_box(&box, &args->box); fence = virtio_gpu_fence_alloc(vgdev); if (!fence) { ret = -ENOMEM; - goto out_unres; + goto err_unlock; } virtio_gpu_cmd_transfer_from_host_3d - (vgdev, qobj->hw_res_handle, -vfpriv->ctx_id, offset, args->level, -&box, fence); - dma_resv_add_excl_fence(qobj->tbo.base.resv, - &fence->f); - + (vgdev, vfpriv->ctx_id, offset, args->level, +&box, objs, fence); dma_fence_put(&fence->f); -out_unres: - virtio_gpu_object_unreserve(qobj); -out: - drm_gem_object_put_unlocked(gobj); + return 0; + +err_unlock: + virtio_gpu_array_unlock_resv(objs); +err_put_free: + virtio_gpu_array_put_free(objs); return ret; } diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 079907e8a596..59d32787944d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -929,20 +929,24 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, } void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, - uint32_t resource_id, uint32_t ctx_id, + uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, + struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence) { + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]); struct virtio_gpu_transfer_host_3d *cmd_p; struct virtio_gpu_vbuffer *vbuf; cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p)); memset(cmd_p, 0, sizeof(*cmd_p)); + vbuf->objs = objs; + cmd_p->hdr
[PATCH v8 01/18] drm/virtio: pass gem reservation object to ttm init
With this gem and ttm will use the same reservation object, so mixing and matching ttm / gem reservation helpers should work fine. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index b2da31310d24..242766d644a7 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -132,7 +132,8 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, virtio_gpu_init_ttm_placement(bo); ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, params->size, ttm_bo_type_device, &bo->placement, 0, - true, acc_size, NULL, NULL, + true, acc_size, NULL, + bo->gem_base.resv, &virtio_gpu_ttm_bo_destroy); /* ttm_bo_init failure will call the destroy */ if (ret != 0) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 04/18] drm/virtio: remove virtio_gpu_object_wait
No users left. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_drv.h| 1 - drivers/gpu/drm/virtio/virtgpu_object.c | 13 - 2 files changed, 14 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index e28829661724..3e0a53309c5b 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -364,7 +364,6 @@ int virtio_gpu_object_kmap(struct virtio_gpu_object *bo); int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, struct virtio_gpu_object *bo); void virtio_gpu_object_free_sg_table(struct virtio_gpu_object *bo); -int virtio_gpu_object_wait(struct virtio_gpu_object *bo, bool no_wait); /* virtgpu_prime.c */ struct sg_table *virtgpu_gem_prime_get_sg_table(struct drm_gem_object *obj); diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 242766d644a7..82bfbf983fd2 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -233,16 +233,3 @@ void virtio_gpu_object_free_sg_table(struct virtio_gpu_object *bo) kfree(bo->pages); bo->pages = NULL; } - -int virtio_gpu_object_wait(struct virtio_gpu_object *bo, bool no_wait) -{ - int r; - - r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL); - if (unlikely(r != 0)) - return r; - r = ttm_bo_wait(&bo->tbo, true, no_wait); - ttm_bo_unreserve(&bo->tbo); - return r; -} - -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 00/18] drm/virtio: switch from ttm to gem shmem helpers.
ttm increasingly gets into the way while hacking on virtio-gpu memory management. It also overkill for what virtio-gpu needs. Lets get rid of it. v8: - rebase to latest drm-misc-next, adapt to changes. v7: - rebase to latest drm-misc-next - reorder patches: switch all virtio commands to object array helpers first. then drop ttm, to make sure we don't release objects still in use. - misc fixes. v6: - largely rewrite fencing logic, using the virtio_gpu_array_* helpers - add more patches to the series. v5: - fence bugfixes. - minor optimizations. v4: - make gem array helpers private to virtio. - misc minor fixes. v3: - add gem array helpers. - rework fencing. please review. thanks, Gerd Gerd Hoffmann (18): drm/virtio: pass gem reservation object to ttm init drm/virtio: switch virtio_gpu_wait_ioctl() to gem helper. drm/virtio: simplify cursor updates drm/virtio: remove virtio_gpu_object_wait drm/virtio: drop no_wait argument from virtio_gpu_object_reserve drm/virtio: remove ttm calls from in virtio_gpu_object_{reserve,unreserve} drm/virtio: add virtio_gpu_object_array & helpers drm/virtio: rework virtio_gpu_execbuffer_ioctl fencing drm/virtio: rework virtio_gpu_object_create fencing drm/virtio: rework virtio_gpu_transfer_from_host_ioctl fencing drm/virtio: rework virtio_gpu_transfer_to_host_ioctl fencing drm/virtio: rework virtio_gpu_cmd_context_{attach,detach}_resource drm/virtio: drop virtio_gpu_object_list_validate/virtio_gpu_unref_list drm/virtio: switch from ttm to gem shmem helpers drm/virtio: remove virtio_gpu_alloc_object drm/virtio: drop virtio_gpu_object_{ref,unref} drm/virtio: drop virtio_gpu_object_{reserve,unreserve} drm/virtio: add fence sanity check drivers/gpu/drm/virtio/virtgpu_drv.h| 123 +++--- drivers/gpu/drm/virtio/virtgpu_drv.c| 20 +- drivers/gpu/drm/virtio/virtgpu_fence.c | 4 + drivers/gpu/drm/virtio/virtgpu_gem.c| 156 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 221 ++--- drivers/gpu/drm/virtio/virtgpu_kms.c| 9 - drivers/gpu/drm/virtio/virtgpu_object.c | 220 + drivers/gpu/drm/virtio/virtgpu_plane.c | 34 +-- drivers/gpu/drm/virtio/virtgpu_prime.c | 34 --- drivers/gpu/drm/virtio/virtgpu_ttm.c| 305 drivers/gpu/drm/virtio/virtgpu_vq.c | 78 -- drivers/gpu/drm/virtio/Kconfig | 2 +- drivers/gpu/drm/virtio/Makefile | 2 +- 13 files changed, 370 insertions(+), 838 deletions(-) delete mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 18/18] drm/virtio: add fence sanity check
Make sure we don't leak half-initialized fences outside the driver. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_fence.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index a0514f5bd006..a4b9881ca1d3 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -41,6 +41,10 @@ bool virtio_fence_signaled(struct dma_fence *f) { struct virtio_gpu_fence *fence = to_virtio_fence(f); + if (WARN_ON_ONCE(fence->f.seqno == 0)) + /* leaked fence outside driver before completing +* initialization with virtio_gpu_fence_emit */ + return false; if (atomic64_read(&fence->drv->last_seq) >= fence->f.seqno) return true; return false; -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 02/18] drm/virtio: switch virtio_gpu_wait_ioctl() to gem helper.
Use drm_gem_reservation_object_wait() in virtio_gpu_wait_ioctl(). This also makes the ioctl run lockless. v5: handle lookup failure. v2: use reservation_object_test_signaled_rcu for VIRTGPU_WAIT_NOWAIT. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 0a88ef11b9d3..74b6bad01d7f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -466,23 +466,20 @@ static int virtio_gpu_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { struct drm_virtgpu_3d_wait *args = data; - struct drm_gem_object *gobj = NULL; - struct virtio_gpu_object *qobj = NULL; + struct drm_gem_object *obj; + long timeout = 15 * HZ; int ret; - bool nowait = false; - gobj = drm_gem_object_lookup(file, args->handle); - if (gobj == NULL) - return -ENOENT; + if (args->flags & VIRTGPU_WAIT_NOWAIT) { + obj = drm_gem_object_lookup(file, args->handle); + if (obj == NULL) + return -ENOENT; + ret = dma_resv_test_signaled_rcu(obj->resv, true); + drm_gem_object_put_unlocked(obj); + return ret ? 0 : -EBUSY; + } - qobj = gem_to_virtio_gpu_obj(gobj); - - if (args->flags & VIRTGPU_WAIT_NOWAIT) - nowait = true; - ret = virtio_gpu_object_wait(qobj, nowait); - - drm_gem_object_put_unlocked(gobj); - return ret; + return drm_gem_dma_resv_wait(file, args->handle, true, timeout); } static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 17/18] drm/virtio: drop virtio_gpu_object_{reserve, unreserve}
No users left. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 21 - 1 file changed, 21 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 85f974a9837b..fb35831ed351 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -368,27 +368,6 @@ static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo) return drm_vma_node_offset_addr(&bo->base.base.vma_node); } -static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo) -{ - int r; - - r = dma_resv_lock_interruptible(bo->base.base.resv, NULL); - if (unlikely(r != 0)) { - if (r != -EINTR) { - struct virtio_gpu_device *qdev = - bo->base.base.dev->dev_private; - dev_err(qdev->dev, "%p reserve failed\n", bo); - } - return r; - } - return 0; -} - -static inline void virtio_gpu_object_unreserve(struct virtio_gpu_object *bo) -{ - dma_resv_unlock(bo->base.base.resv); -} - /* virgl debufs */ int virtio_gpu_debugfs_init(struct drm_minor *minor); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 09/18] drm/virtio: rework virtio_gpu_object_create fencing
Rework fencing workflow. Stop using ttm helpers, use the virtio_gpu_array_* helpers instead. Due to using the gem reservation object it is initialized and ready for use before calling ttm_bo_init. So we can simply use the standard fencing workflow and drop the tricky logic which checks whenever the command is in flight still. v6: rewrite most of the patch. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h| 2 + drivers/gpu/drm/virtio/virtgpu_object.c | 74 +++-- drivers/gpu/drm/virtio/virtgpu_vq.c | 4 ++ 3 files changed, 37 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index f4f843aec18a..c6ea65f6507e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -274,6 +274,7 @@ void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev); void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev, struct virtio_gpu_object *bo, struct virtio_gpu_object_params *params, + struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev, uint32_t resource_id); @@ -336,6 +337,7 @@ void virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev, struct virtio_gpu_object *bo, struct virtio_gpu_object_params *params, + struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_ctrl_ack(struct virtqueue *vq); void virtio_gpu_cursor_ack(struct virtqueue *vq); diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 82bfbf983fd2..2902487f051a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -97,6 +97,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, struct virtio_gpu_object **bo_ptr, struct virtio_gpu_fence *fence) { + struct virtio_gpu_object_array *objs = NULL; struct virtio_gpu_object *bo; size_t acc_size; int ret; @@ -110,23 +111,34 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, if (bo == NULL) return -ENOMEM; ret = virtio_gpu_resource_id_get(vgdev, &bo->hw_res_handle); - if (ret < 0) { - kfree(bo); - return ret; - } + if (ret < 0) + goto err_free_gem; + params->size = roundup(params->size, PAGE_SIZE); ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, params->size); - if (ret != 0) { - virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle); - kfree(bo); - return ret; - } + if (ret != 0) + goto err_put_id; + bo->dumb = params->dumb; + if (fence) { + ret = -ENOMEM; + objs = virtio_gpu_array_alloc(1); + if (!objs) + goto err_put_id; + virtio_gpu_array_add_obj(objs, &bo->gem_base); + + ret = virtio_gpu_array_lock_resv(objs); + if (ret != 0) + goto err_put_objs; + } + if (params->virgl) { - virtio_gpu_cmd_resource_create_3d(vgdev, bo, params, fence); + virtio_gpu_cmd_resource_create_3d(vgdev, bo, params, + objs, fence); } else { - virtio_gpu_cmd_create_resource(vgdev, bo, params, fence); + virtio_gpu_cmd_create_resource(vgdev, bo, params, + objs, fence); } virtio_gpu_init_ttm_placement(bo); @@ -139,40 +151,16 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, if (ret != 0) return ret; - if (fence) { - struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv; - struct list_head validate_list; - struct ttm_validate_buffer mainbuf; - struct ww_acquire_ctx ticket; - unsigned long irq_flags; - bool signaled; - - INIT_LIST_HEAD(&validate_list); - memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer)); - - /* use a gem reference since unref list undoes them */ - drm_gem_object_get(&bo->gem_base); - mainbuf.bo = &bo->tbo; - list_add(&mainbuf.head, &validate_list); - - ret = virtio_gpu_object_list_validate(&ticket, &validate_list); - if (ret == 0) { -
[PATCH v8 05/18] drm/virtio: drop no_wait argument from virtio_gpu_object_reserve
All callers pass no_wait = false. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_drv.h | 5 ++--- drivers/gpu/drm/virtio/virtgpu_gem.c | 4 ++-- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 3e0a53309c5b..d886c0e3502a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -398,12 +398,11 @@ static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo) return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } -static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo, -bool no_wait) +static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo) { int r; - r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL); + r = ttm_bo_reserve(&bo->tbo, true, false, NULL); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) { struct virtio_gpu_device *qdev = diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index 292566146814..6fe6f72f64d1 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -142,7 +142,7 @@ int virtio_gpu_gem_object_open(struct drm_gem_object *obj, if (!vgdev->has_virgl_3d) return 0; - r = virtio_gpu_object_reserve(qobj, false); + r = virtio_gpu_object_reserve(qobj); if (r) return r; @@ -163,7 +163,7 @@ void virtio_gpu_gem_object_close(struct drm_gem_object *obj, if (!vgdev->has_virgl_3d) return; - r = virtio_gpu_object_reserve(qobj, false); + r = virtio_gpu_object_reserve(qobj); if (r) return; diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 74b6bad01d7f..285824c42e19 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -377,7 +377,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, qobj = gem_to_virtio_gpu_obj(gobj); - ret = virtio_gpu_object_reserve(qobj, false); + ret = virtio_gpu_object_reserve(qobj); if (ret) goto out; @@ -427,7 +427,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, qobj = gem_to_virtio_gpu_obj(gobj); - ret = virtio_gpu_object_reserve(qobj, false); + ret = virtio_gpu_object_reserve(qobj); if (ret) goto out; -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 12/18] drm/virtio: rework virtio_gpu_cmd_context_{attach, detach}_resource
Switch to the virtio_gpu_array_* helper workflow. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++-- drivers/gpu/drm/virtio/virtgpu_gem.c | 24 +++- drivers/gpu/drm/virtio/virtgpu_vq.c | 12 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 4f54bf7c02af..d5ef2514d2bd 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -313,10 +313,10 @@ void virtio_gpu_cmd_context_destroy(struct virtio_gpu_device *vgdev, uint32_t id); void virtio_gpu_cmd_context_attach_resource(struct virtio_gpu_device *vgdev, uint32_t ctx_id, - uint32_t resource_id); + struct virtio_gpu_object_array *objs); void virtio_gpu_cmd_context_detach_resource(struct virtio_gpu_device *vgdev, uint32_t ctx_id, - uint32_t resource_id); + struct virtio_gpu_object_array *objs); void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev, void *data, uint32_t data_size, uint32_t ctx_id, diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index f3799f2e97cb..188c20aaae56 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -136,19 +136,18 @@ int virtio_gpu_gem_object_open(struct drm_gem_object *obj, { struct virtio_gpu_device *vgdev = obj->dev->dev_private; struct virtio_gpu_fpriv *vfpriv = file->driver_priv; - struct virtio_gpu_object *qobj = gem_to_virtio_gpu_obj(obj); - int r; + struct virtio_gpu_object_array *objs; if (!vgdev->has_virgl_3d) return 0; - r = virtio_gpu_object_reserve(qobj); - if (r) - return r; + objs = virtio_gpu_array_alloc(1); + if (!objs) + return -ENOMEM; + virtio_gpu_array_add_obj(objs, obj); virtio_gpu_cmd_context_attach_resource(vgdev, vfpriv->ctx_id, - qobj->hw_res_handle); - virtio_gpu_object_unreserve(qobj); + objs); return 0; } @@ -157,19 +156,18 @@ void virtio_gpu_gem_object_close(struct drm_gem_object *obj, { struct virtio_gpu_device *vgdev = obj->dev->dev_private; struct virtio_gpu_fpriv *vfpriv = file->driver_priv; - struct virtio_gpu_object *qobj = gem_to_virtio_gpu_obj(obj); - int r; + struct virtio_gpu_object_array *objs; if (!vgdev->has_virgl_3d) return; - r = virtio_gpu_object_reserve(qobj); - if (r) + objs = virtio_gpu_array_alloc(1); + if (!objs) return; + virtio_gpu_array_add_obj(objs, obj); virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id, - qobj->hw_res_handle); - virtio_gpu_object_unreserve(qobj); + objs); } struct virtio_gpu_object_array *virtio_gpu_array_alloc(u32 nents) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index e8f5670aadf2..7a316e92c783 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -838,34 +838,38 @@ void virtio_gpu_cmd_context_destroy(struct virtio_gpu_device *vgdev, void virtio_gpu_cmd_context_attach_resource(struct virtio_gpu_device *vgdev, uint32_t ctx_id, - uint32_t resource_id) + struct virtio_gpu_object_array *objs) { + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]); struct virtio_gpu_ctx_resource *cmd_p; struct virtio_gpu_vbuffer *vbuf; cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p)); memset(cmd_p, 0, sizeof(*cmd_p)); + vbuf->objs = objs; cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE); cmd_p->hdr.ctx_id = cpu_to_le32(ctx_id); - cmd_p->resource_id = cpu_to_le32(resource_id); + cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle); virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); } void virtio_gpu_cmd_context_detach_resource(struct virtio_gpu_device *vgdev, uint32_t ctx_id, - uint32_t resource_id) + struct virtio_gpu_object_array *objs) { + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
[PATCH v8 06/18] drm/virtio: remove ttm calls from in virtio_gpu_object_{reserve, unreserve}
Call reservation_object_* directly instead of using ttm_bo_{reserve,unreserve}. v4: check for EINTR only. v3: check for EINTR too. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_drv.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index d886c0e3502a..db57bbb36216 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -402,9 +402,9 @@ static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo) { int r; - r = ttm_bo_reserve(&bo->tbo, true, false, NULL); + r = dma_resv_lock_interruptible(bo->gem_base.resv, NULL); if (unlikely(r != 0)) { - if (r != -ERESTARTSYS) { + if (r != -EINTR) { struct virtio_gpu_device *qdev = bo->gem_base.dev->dev_private; dev_err(qdev->dev, "%p reserve failed\n", bo); @@ -416,7 +416,7 @@ static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo) static inline void virtio_gpu_object_unreserve(struct virtio_gpu_object *bo) { - ttm_bo_unreserve(&bo->tbo); + dma_resv_unlock(bo->gem_base.resv); } /* virgl debufs */ -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 15/18] drm/virtio: remove virtio_gpu_alloc_object
Thin wrapper around virtio_gpu_object_create(), but calling that directly works equally well. Signed-off-by: Gerd Hoffmann Acked-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_drv.h | 4 drivers/gpu/drm/virtio/virtgpu_gem.c | 23 --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 6 +++--- 3 files changed, 7 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index b67d23ef2b11..3e5b2d1db42d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -229,10 +229,6 @@ int virtio_gpu_gem_object_open(struct drm_gem_object *obj, struct drm_file *file); void virtio_gpu_gem_object_close(struct drm_gem_object *obj, struct drm_file *file); -struct virtio_gpu_object* -virtio_gpu_alloc_object(struct drm_device *dev, - struct virtio_gpu_object_params *params, - struct virtio_gpu_fence *fence); int virtio_gpu_mode_dumb_create(struct drm_file *file_priv, struct drm_device *dev, struct drm_mode_create_dumb *args); diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index 452ad7ac54a5..3a45a865f9c1 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -28,35 +28,20 @@ #include "virtgpu_drv.h" -struct virtio_gpu_object* -virtio_gpu_alloc_object(struct drm_device *dev, - struct virtio_gpu_object_params *params, - struct virtio_gpu_fence *fence) -{ - struct virtio_gpu_device *vgdev = dev->dev_private; - struct virtio_gpu_object *obj; - int ret; - - ret = virtio_gpu_object_create(vgdev, params, &obj, fence); - if (ret) - return ERR_PTR(ret); - - return obj; -} - int virtio_gpu_gem_create(struct drm_file *file, struct drm_device *dev, struct virtio_gpu_object_params *params, struct drm_gem_object **obj_p, uint32_t *handle_p) { + struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_object *obj; int ret; u32 handle; - obj = virtio_gpu_alloc_object(dev, params, NULL); - if (IS_ERR(obj)) - return PTR_ERR(obj); + ret = virtio_gpu_object_create(vgdev, params, &obj, NULL); + if (ret < 0) + return ret; ret = drm_gem_handle_create(file, &obj->base.base, &handle); if (ret) { diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 1e25c650870c..e10be4bcebaf 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -256,10 +256,10 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, fence = virtio_gpu_fence_alloc(vgdev); if (!fence) return -ENOMEM; - qobj = virtio_gpu_alloc_object(dev, ¶ms, fence); + ret = virtio_gpu_object_create(vgdev, ¶ms, &qobj, fence); dma_fence_put(&fence->f); - if (IS_ERR(qobj)) - return PTR_ERR(qobj); + if (ret < 0) + return ret; obj = &qobj->base.base; ret = drm_gem_handle_create(file_priv, obj, &handle); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 13/18] drm/virtio: drop virtio_gpu_object_list_validate/virtio_gpu_unref_list
No users left. Signed-off-by: Gerd Hoffmann Acked-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_drv.h | 3 -- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 39 -- 2 files changed, 42 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index d5ef2514d2bd..1266a8e64961 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -223,9 +223,6 @@ struct virtio_gpu_fpriv { /* virtio_ioctl.c */ #define DRM_VIRTIO_NUM_IOCTLS 10 extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS]; -int virtio_gpu_object_list_validate(struct ww_acquire_ctx *ticket, - struct list_head *head); -void virtio_gpu_unref_list(struct list_head *head); /* virtio_kms.c */ int virtio_gpu_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index cffde4760468..96eb9e59a8c8 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -56,45 +56,6 @@ static int virtio_gpu_map_ioctl(struct drm_device *dev, void *data, &virtio_gpu_map->offset); } -int virtio_gpu_object_list_validate(struct ww_acquire_ctx *ticket, - struct list_head *head) -{ - struct ttm_operation_ctx ctx = { false, false }; - struct ttm_validate_buffer *buf; - struct ttm_buffer_object *bo; - struct virtio_gpu_object *qobj; - int ret; - - ret = ttm_eu_reserve_buffers(ticket, head, true, NULL, true); - if (ret != 0) - return ret; - - list_for_each_entry(buf, head, head) { - bo = buf->bo; - qobj = container_of(bo, struct virtio_gpu_object, tbo); - ret = ttm_bo_validate(bo, &qobj->placement, &ctx); - if (ret) { - ttm_eu_backoff_reservation(ticket, head); - return ret; - } - } - return 0; -} - -void virtio_gpu_unref_list(struct list_head *head) -{ - struct ttm_validate_buffer *buf; - struct ttm_buffer_object *bo; - struct virtio_gpu_object *qobj; - - list_for_each_entry(buf, head, head) { - bo = buf->bo; - qobj = container_of(bo, struct virtio_gpu_object, tbo); - - drm_gem_object_put_unlocked(&qobj->gem_base); - } -} - /* * Usage of execbuffer: * Relocations need to take into account the full VIRTIO_GPUDrawable size. -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 08/18] drm/virtio: rework virtio_gpu_execbuffer_ioctl fencing
Rework fencing workflow, starting with virtio_gpu_execbuffer_ioctl. Stop using ttm helpers, use the virtio_gpu_array_* helpers (which work on the reservation objects directly) instead. Also store the object array in struct virtio_gpu_vbuffer, so we explicitly keep a reference of all buffers used instead of depending on ttm_bo_put() checking whenever the object is actually idle before releasing it. New workflow: (1) All gem objects needed by a command are added to a virtio_gpu_object_array. (2) All reservation objects will be locked (virtio_gpu_array_lock_resv). (3) virtio_gpu_fence_emit() completes fence initialization. (4) fence gets added to the objects, reservation objects are unlocked (virtio_gpu_array_add_fence, virtio_gpu_array_unlock_resv). (5) virtio command is submitted to the host. (6) The completion callback (virtio_gpu_dequeue_ctrl_func) will drop object references and free virtio_gpu_object_array. v6: rewrite most of the patch. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 6 ++- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 56 +- drivers/gpu/drm/virtio/virtgpu_vq.c| 21 +++--- 3 files changed, 38 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index b6bd2b1141fb..f4f843aec18a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -121,9 +121,9 @@ struct virtio_gpu_vbuffer { char *resp_buf; int resp_size; - virtio_gpu_resp_cb resp_cb; + struct virtio_gpu_object_array *objs; struct list_head list; }; @@ -318,7 +318,9 @@ void virtio_gpu_cmd_context_detach_resource(struct virtio_gpu_device *vgdev, uint32_t resource_id); void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev, void *data, uint32_t data_size, - uint32_t ctx_id, struct virtio_gpu_fence *fence); + uint32_t ctx_id, + struct virtio_gpu_object_array *objs, + struct virtio_gpu_fence *fence); void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, uint32_t resource_id, uint32_t ctx_id, uint64_t offset, uint32_t level, diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 285824c42e19..6baf43148e01 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -107,16 +107,11 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, struct drm_virtgpu_execbuffer *exbuf = data; struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv; - struct drm_gem_object *gobj; struct virtio_gpu_fence *out_fence; - struct virtio_gpu_object *qobj; int ret; uint32_t *bo_handles = NULL; void __user *user_bo_handles = NULL; - struct list_head validate_list; - struct ttm_validate_buffer *buflist = NULL; - int i; - struct ww_acquire_ctx ticket; + struct virtio_gpu_object_array *buflist = NULL; struct sync_file *sync_file; int in_fence_fd = exbuf->fence_fd; int out_fence_fd = -1; @@ -157,15 +152,10 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, return out_fence_fd; } - INIT_LIST_HEAD(&validate_list); if (exbuf->num_bo_handles) { - bo_handles = kvmalloc_array(exbuf->num_bo_handles, - sizeof(uint32_t), GFP_KERNEL); - buflist = kvmalloc_array(exbuf->num_bo_handles, - sizeof(struct ttm_validate_buffer), - GFP_KERNEL | __GFP_ZERO); - if (!bo_handles || !buflist) { + sizeof(uint32_t), GFP_KERNEL); + if (!bo_handles) { ret = -ENOMEM; goto out_unused_fd; } @@ -177,25 +167,21 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, goto out_unused_fd; } - for (i = 0; i < exbuf->num_bo_handles; i++) { - gobj = drm_gem_object_lookup(drm_file, bo_handles[i]); - if (!gobj) { - ret = -ENOENT; - goto out_unused_fd; - } - - qobj = gem_to_virtio_gpu_obj(gobj); - buflist[i].bo = &qobj->tbo; - - list_add(&buflist[i].head, &validate_list); + buflist = vir
[PATCH v8 16/18] drm/virtio: drop virtio_gpu_object_{ref,unref}
No users left. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 15 --- 1 file changed, 15 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 3e5b2d1db42d..85f974a9837b 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -363,21 +363,6 @@ struct drm_gem_object *virtgpu_gem_prime_import_sg_table( struct drm_device *dev, struct dma_buf_attachment *attach, struct sg_table *sgt); -static inline struct virtio_gpu_object* -virtio_gpu_object_ref(struct virtio_gpu_object *bo) -{ - drm_gem_object_get(&bo->base.base); - return bo; -} - -static inline void virtio_gpu_object_unref(struct virtio_gpu_object **bo) -{ - if ((*bo) == NULL) - return; - drm_gem_object_put(&(*bo)->base.base); - *bo = NULL; -} - static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo) { return drm_vma_node_offset_addr(&bo->base.base.vma_node); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 14/18] drm/virtio: switch from ttm to gem shmem helpers
virtio-gpu basically needs a sg_table for the bo, to tell the host where the backing pages for the object are. So the gem shmem helpers are a perfect fit. Some drm_gem_object_funcs need thin wrappers to update the host state, but otherwise the helpers handle everything just fine. Once the fencing was sorted the switch was surprisingly easy and for the most part just removing the ttm code. v4: fix drm_gem_object_funcs name. Signed-off-by: Gerd Hoffmann Acked-by: Daniel Vetter --- drivers/gpu/drm/virtio/virtgpu_drv.h| 52 +--- drivers/gpu/drm/virtio/virtgpu_drv.c| 20 +- drivers/gpu/drm/virtio/virtgpu_gem.c| 16 +- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +- drivers/gpu/drm/virtio/virtgpu_kms.c| 9 - drivers/gpu/drm/virtio/virtgpu_object.c | 146 drivers/gpu/drm/virtio/virtgpu_prime.c | 34 --- drivers/gpu/drm/virtio/virtgpu_ttm.c| 305 drivers/gpu/drm/virtio/virtgpu_vq.c | 24 +- drivers/gpu/drm/virtio/Kconfig | 2 +- drivers/gpu/drm/virtio/Makefile | 2 +- 11 files changed, 80 insertions(+), 535 deletions(-) delete mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 1266a8e64961..b67d23ef2b11 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -35,12 +35,9 @@ #include #include #include +#include #include #include -#include -#include -#include -#include #define DRIVER_NAME "virtio_gpu" #define DRIVER_DESC "virtio GPU" @@ -68,21 +65,16 @@ struct virtio_gpu_object_params { }; struct virtio_gpu_object { - struct drm_gem_object gem_base; + struct drm_gem_shmem_object base; uint32_t hw_res_handle; struct sg_table *pages; uint32_t mapped; - void *vmap; bool dumb; - struct ttm_placeplacement_code; - struct ttm_placementplacement; - struct ttm_buffer_objecttbo; - struct ttm_bo_kmap_obj kmap; bool created; }; #define gem_to_virtio_gpu_obj(gobj) \ - container_of((gobj), struct virtio_gpu_object, gem_base) + container_of((gobj), struct virtio_gpu_object, base.base) struct virtio_gpu_object_array { struct ww_acquire_ctx ticket; @@ -153,10 +145,6 @@ struct virtio_gpu_framebuffer { #define to_virtio_gpu_framebuffer(x) \ container_of(x, struct virtio_gpu_framebuffer, base) -struct virtio_gpu_mman { - struct ttm_bo_devicebdev; -}; - struct virtio_gpu_queue { struct virtqueue *vq; spinlock_t qlock; @@ -185,8 +173,6 @@ struct virtio_gpu_device { struct virtio_device *vdev; - struct virtio_gpu_mman mman; - struct virtio_gpu_output outputs[VIRTIO_GPU_MAX_SCANOUTS]; uint32_t num_scanouts; @@ -358,11 +344,6 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev, enum drm_plane_type type, int index); -/* virtio_gpu_ttm.c */ -int virtio_gpu_ttm_init(struct virtio_gpu_device *vgdev); -void virtio_gpu_ttm_fini(struct virtio_gpu_device *vgdev); -int virtio_gpu_mmap(struct file *filp, struct vm_area_struct *vma); - /* virtio_gpu_fence.c */ bool virtio_fence_signaled(struct dma_fence *f); struct virtio_gpu_fence *virtio_gpu_fence_alloc( @@ -374,58 +355,47 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev, u64 last_seq); /* virtio_gpu_object */ +struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev, + size_t size); int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, struct virtio_gpu_object_params *params, struct virtio_gpu_object **bo_ptr, struct virtio_gpu_fence *fence); -void virtio_gpu_object_kunmap(struct virtio_gpu_object *bo); -int virtio_gpu_object_kmap(struct virtio_gpu_object *bo); -int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, - struct virtio_gpu_object *bo); -void virtio_gpu_object_free_sg_table(struct virtio_gpu_object *bo); /* virtgpu_prime.c */ -struct sg_table *virtgpu_gem_prime_get_sg_table(struct drm_gem_object *obj); struct drm_gem_object *virtgpu_gem_prime_import_sg_table( struct drm_device *dev, struct dma_buf_attachment *attach, struct sg_table *sgt); -void *virtgpu_gem_prime_vmap(struct drm_gem_object *obj); -void virtgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); -int virtgpu_gem_prime_mmap(struct drm_gem_object *obj, - struct vm_area_struct *vma); static inline struct virtio_gpu_object* virtio_gpu_object_ref(struct virtio_gpu_object *bo) { - ttm_bo_get(&bo->tbo); + drm
[PATCH v8 07/18] drm/virtio: add virtio_gpu_object_array & helpers
Some helper functions to manage an array of gem objects. v6: - add ticket to struct virtio_gpu_object_array. - add virtio_gpu_array_{lock,unlock}_resv helpers. - add virtio_gpu_array_add_fence helper. v5: some small optimizations (Chia-I Wu). v4: make them virtio-private instead of generic helpers. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 17 + drivers/gpu/drm/virtio/virtgpu_gem.c | 93 2 files changed, 110 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index db57bbb36216..b6bd2b1141fb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -84,6 +84,12 @@ struct virtio_gpu_object { #define gem_to_virtio_gpu_obj(gobj) \ container_of((gobj), struct virtio_gpu_object, gem_base) +struct virtio_gpu_object_array { + struct ww_acquire_ctx ticket; + u32 nents, total; + struct drm_gem_object *objs[]; +}; + struct virtio_gpu_vbuffer; struct virtio_gpu_device; @@ -251,6 +257,17 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv, struct drm_device *dev, uint32_t handle, uint64_t *offset_p); +struct virtio_gpu_object_array *virtio_gpu_array_alloc(u32 nents); +struct virtio_gpu_object_array* +virtio_gpu_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents); +void virtio_gpu_array_add_obj(struct virtio_gpu_object_array *objs, + struct drm_gem_object *obj); +int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs); +void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs); +void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs, + struct dma_fence *fence); +void virtio_gpu_array_put_free(struct virtio_gpu_object_array *objs); + /* virtio vg */ int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev); void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev); diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index 6fe6f72f64d1..f3799f2e97cb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -171,3 +171,96 @@ void virtio_gpu_gem_object_close(struct drm_gem_object *obj, qobj->hw_res_handle); virtio_gpu_object_unreserve(qobj); } + +struct virtio_gpu_object_array *virtio_gpu_array_alloc(u32 nents) +{ + struct virtio_gpu_object_array *objs; + size_t size = sizeof(*objs) + sizeof(objs->objs[0]) * nents; + + objs = kmalloc(size, GFP_KERNEL); + if (!objs) + return NULL; + + objs->nents = 0; + objs->total = nents; + return objs; +} + +static void virtio_gpu_array_free(struct virtio_gpu_object_array *objs) +{ + kfree(objs); +} + +struct virtio_gpu_object_array* +virtio_gpu_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents) +{ + struct virtio_gpu_object_array *objs; + u32 i; + + objs = virtio_gpu_array_alloc(nents); + if (!objs) + return NULL; + + for (i = 0; i < nents; i++) { + objs->objs[i] = drm_gem_object_lookup(drm_file, handles[i]); + if (!objs->objs[i]) { + objs->nents = i; + virtio_gpu_array_put_free(objs); + return NULL; + } + } + objs->nents = i; + return objs; +} + +void virtio_gpu_array_add_obj(struct virtio_gpu_object_array *objs, + struct drm_gem_object *obj) +{ + if (WARN_ON_ONCE(objs->nents == objs->total)) + return; + + drm_gem_object_get(obj); + objs->objs[objs->nents] = obj; + objs->nents++; +} + +int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs) +{ + int ret; + + if (objs->nents == 1) { + ret = dma_resv_lock(objs->objs[0]->resv, NULL); + } else { + ret = drm_gem_lock_reservations(objs->objs, objs->nents, + &objs->ticket); + } + return ret; +} + +void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs) +{ + if (objs->nents == 1) { + dma_resv_unlock(objs->objs[0]->resv); + } else { + drm_gem_unlock_reservations(objs->objs, objs->nents, + &objs->ticket); + } +} + +void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs, + struct dma_fence *fence) +{ + int i; + + for (i = 0; i < objs->nents; i++) + dma_resv_add_excl_fence(objs->objs[i]->resv, fence); +} + +void virtio_gpu_array_put_free(struct virtio_gpu_object_array *objs) +{ + u32 i; + + for (i = 0; i < objs->nents; i++)
[PATCH v8 11/18] drm/virtio: rework virtio_gpu_transfer_to_host_ioctl fencing
Switch to the virtio_gpu_array_* helper workflow. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 4 +-- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 50 +++--- drivers/gpu/drm/virtio/virtgpu_plane.c | 21 --- drivers/gpu/drm/virtio/virtgpu_vq.c| 9 +++-- 4 files changed, 47 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index fa568cbfdddc..4f54bf7c02af 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -279,10 +279,10 @@ void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev, void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev, uint32_t resource_id); void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, - struct virtio_gpu_object *bo, uint64_t offset, __le32 width, __le32 height, __le32 x, __le32 y, + struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_resource_flush(struct virtio_gpu_device *vgdev, uint32_t resource_id, @@ -329,10 +329,10 @@ void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev, struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev, - struct virtio_gpu_object *bo, uint32_t ctx_id, uint64_t offset, uint32_t level, struct virtio_gpu_box *box, + struct virtio_gpu_object_array *objs, struct virtio_gpu_fence *fence); void virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev, diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 444696f73e96..cffde4760468 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -383,52 +383,44 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_fpriv *vfpriv = file->driver_priv; struct drm_virtgpu_3d_transfer_to_host *args = data; - struct ttm_operation_ctx ctx = { true, false }; - struct drm_gem_object *gobj = NULL; - struct virtio_gpu_object *qobj = NULL; + struct virtio_gpu_object_array *objs; struct virtio_gpu_fence *fence; struct virtio_gpu_box box; int ret; u32 offset = args->offset; - gobj = drm_gem_object_lookup(file, args->bo_handle); - if (gobj == NULL) + objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1); + if (objs == NULL) return -ENOENT; - qobj = gem_to_virtio_gpu_obj(gobj); - - ret = virtio_gpu_object_reserve(qobj); - if (ret) - goto out; - - ret = ttm_bo_validate(&qobj->tbo, &qobj->placement, &ctx); - if (unlikely(ret)) - goto out_unres; - convert_to_hw_box(&box, &args->box); if (!vgdev->has_virgl_3d) { virtio_gpu_cmd_transfer_to_host_2d - (vgdev, qobj, offset, -box.w, box.h, box.x, box.y, NULL); + (vgdev, offset, +box.w, box.h, box.x, box.y, +objs, NULL); } else { + ret = virtio_gpu_array_lock_resv(objs); + if (ret != 0) + goto err_put_free; + + ret = -ENOMEM; fence = virtio_gpu_fence_alloc(vgdev); - if (!fence) { - ret = -ENOMEM; - goto out_unres; - } + if (!fence) + goto err_unlock; + virtio_gpu_cmd_transfer_to_host_3d - (vgdev, qobj, + (vgdev, vfpriv ? vfpriv->ctx_id : 0, offset, -args->level, &box, fence); - dma_resv_add_excl_fence(qobj->tbo.base.resv, - &fence->f); +args->level, &box, objs, fence); dma_fence_put(&fence->f); } + return 0; -out_unres: - virtio_gpu_object_unreserve(qobj); -out: - drm_gem_object_put_unlocked(gobj); +err_unlock: + virtio_gpu_array_unlock_resv(objs); +e
Re: [PATCH 2/2] drm/bridge: panel: Use drm_panel.type instead of explicit connector_type
On Fri, Aug 23, 2019 at 3:40 AM Laurent Pinchart wrote: > The drm panel bridge creates a connector using a connector type explicit > passed by the display controller or bridge driver that instantiates the > panel bridge. Now that drm_panel reports its connector type, use it and > remove the connector_type argument to drm_panel_bridge_add() and > devm_drm_panel_bridge_add(). > > Signed-off-by: Laurent Pinchart Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/komeda: Reordered the komeda's de-init functions
On Tuesday, 20 August 2019 18:46:19 BST Ayan Halder wrote: > The de-init routine should be doing the following in order:- > 1. Unregister the drm device > 2. Shut down the crtcs - failing to do this might cause a connector leakage > See the 'commit 109c4d18e574 ("drm/arm/malidp: Ensure that the crtcs are > shutdown before removing any encoder/connector")' > 3. Disable the interrupts > 4. Unbind the components > 5. Free up DRM mode_config info > > Signed-off-by: Ayan Kumar Halder > --- > .../gpu/drm/arm/display/komeda/komeda_kms.c | 20 +-- > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > index 89191a555c84..e219d1b67100 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include Can we keep the include list in alphabetical order? > #include > #include > > @@ -304,24 +305,30 @@ struct komeda_kms_dev *komeda_kms_attach(struct > komeda_dev *mdev) > komeda_kms_irq_handler, IRQF_SHARED, > drm->driver->name, drm); > if (err) > - goto cleanup_mode_config; > + goto free_component_binding; > > err = mdev->funcs->enable_irq(mdev); > if (err) > - goto cleanup_mode_config; > + goto free_component_binding; > > drm->irq_enabled = true; > > err = drm_dev_register(drm, 0); > if (err) > - goto cleanup_mode_config; > + goto free_interrupts; > > return kms; > > -cleanup_mode_config: > +free_interrupts: > drm->irq_enabled = false; > + mdev->funcs->disable_irq(mdev); > +free_component_binding: > + component_unbind_all(mdev->dev, drm); > +cleanup_mode_config: > drm_mode_config_cleanup(drm); > komeda_kms_cleanup_private_objs(kms); > + drm->dev_private = NULL; > + drm_dev_put(drm); > free_kms: > kfree(kms); > return ERR_PTR(err); > @@ -332,12 +339,13 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) > struct drm_device *drm = &kms->base; > struct komeda_dev *mdev = drm->dev_private; > > + drm_dev_unregister(drm); > + drm_atomic_helper_shutdown(drm); > drm->irq_enabled = false; > mdev->funcs->disable_irq(mdev); > - drm_dev_unregister(drm); > component_unbind_all(mdev->dev, drm); > - komeda_kms_cleanup_private_objs(kms); > drm_mode_config_cleanup(drm); > + komeda_kms_cleanup_private_objs(kms); > drm->dev_private = NULL; > drm_dev_put(drm); > } > Thanks. See my include order comment above, with that fixed: Reviewed-by: Mihail Atanassov -- Mihail ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/8] drm/panfrost: Rework runtime PM initialization
On 23/08/2019 03:12, Rob Herring wrote: There's a few issues with the runtime PM initialization. The documentation states pm_runtime_set_active() should be called before pm_runtime_enable(). The pm_runtime_put_autosuspend() could suspend the GPU before panfrost_perfcnt_init() is called which touches the h/w. The autosuspend delay keeps things from breaking. There's no need explicitly power off the GPU only to wake back up with pm_runtime_get_sync(). Just delaying pm_runtime_enable to the end of probe is sufficient. Lets move all the runtime PM calls into the probe() function so they are all in one place and are done after all initialization. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch drivers/gpu/drm/panfrost/panfrost_device.c | 9 - drivers/gpu/drm/panfrost/panfrost_drv.c| 10 ++ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 4da71bb56c20..73805210834e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -5,7 +5,6 @@ #include #include #include -#include #include #include "panfrost_device.h" @@ -166,14 +165,6 @@ int panfrost_device_init(struct panfrost_device *pfdev) if (err) goto err_out4; - /* runtime PM will wake us up later */ - panfrost_gpu_power_off(pfdev); - - pm_runtime_set_active(pfdev->dev); - pm_runtime_get_sync(pfdev->dev); - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_put_autosuspend(pfdev->dev); - err = panfrost_perfcnt_init(pfdev); if (err) goto err_out5; diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index d74442d71048..f27e3d6aec12 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -523,10 +523,6 @@ static int panfrost_probe(struct platform_device *pdev) mutex_init(&pfdev->shrinker_lock); INIT_LIST_HEAD(&pfdev->shrinker_list); - pm_runtime_use_autosuspend(pfdev->dev); - pm_runtime_set_autosuspend_delay(pfdev->dev, 50); /* ~3 frames */ - pm_runtime_enable(pfdev->dev); - err = panfrost_device_init(pfdev); if (err) { if (err != -EPROBE_DEFER) @@ -541,6 +537,12 @@ static int panfrost_probe(struct platform_device *pdev) goto err_out1; } + pm_runtime_set_active(pfdev->dev); + pm_runtime_use_autosuspend(pfdev->dev); + pm_runtime_set_autosuspend_delay(pfdev->dev, 0); /* ~3 frames */ Hmm... different timeout, same comment - something seems amiss there, unless perhaps it's all within rounding error anyway? Other than that, the overall change looks sensible to me. Robin. + pm_runtime_mark_last_busy(pfdev->dev); + pm_runtime_enable(pfdev->dev); + /* * Register the DRM device with the core and the connectors with * sysfs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #114 from Yuxuan Shui --- (In reply to magist3r from comment #111) > (In reply to tempel.julian from comment #110) > > (In reply to magist3r from comment #109) > > > My patch fixes a bug that breaks this behavior when OverDrive mask is > > > enabled, nothing more. > > > > It unfortunately also forces my single display 1440p 75Hz into maximum VRAM > > clock. > > It's not caused by my patch. Try to disable overdrive mask, revert the patch > and you will see the same behavior. Can confirm your patch fixed the glitch for me. Also yes, my MCLK is stuck at the highest level without your patch anyways. I have 2 monitors at 60hz. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
Hi Nicolas, On Fri, Aug 23, 2019 at 07:30:07AM +, nicolas.fe...@microchip.com wrote: > On 23/08/2019 at 03:40, Laurent Pinchart wrote: > > Add a type field to the drm_panel structure to report the panel type, > > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS, > > eDP, DSI and DPI). This will be used to initialise the corresponding > > connector type. > > With Microchip/Atmel driver, we mainly (only) use the "Unknown" type of > connector because our hardware simply uses RGB wires in parallel. That's called DPI (Display Pixel Interface, sometimes also referred to as Display Parallel Interface) :-) > Should we move to another connector type (maybe now that it's created > and it was not, back when we chose the "Unknown" one)? I think DRM_MODE_CONNECTOR_DPI would be best, yes. > What would be the consequences if we move (silently?) to another type > and particularly on the command line argument like the ones we currently > use: "Unknown-1:800x480-16"? That will be nasty to handle :-( As much as I'd love to ignore that issue, I don't think we can. At the same time it shouldn't prevent us from moving forward and exposing the real connector type. One option could be to keep the extra type argument to drm_panel_bridge_add() to force the connector type, and add another variant of the function that would derive it automatically from the panel type. Drivers could then decide to switch to the new variant on a case-by-case basis. Still, that won't solve your issue, as I'm not sure how you would be able to decide which variant to call if we want newer systems to expose the right panel type while keeping backward compatibility. Another option would be to hack the command line argument parsing to convert Unknown-* to a panel type when there's no connector with an unknown type, but that seems fragile. Any other proposal ? :-) > > Update all panel drivers to fill the new field. > > > > Signed-off-by: Laurent Pinchart -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On 23/08/2019 03:12, Rob Herring wrote: There is no point in resuming the h/w just to do flush operations and doing so takes several locks which cause lockdep issues with the shrinker. Rework the flush operations to only happen when the h/w is already awake. This avoids taking any locks associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..ccf671a9c3fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) return SZ_2M; } +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, + struct panfrost_mmu *mmu, + u64 iova, size_t size) +{ + if (mmu->as < 0) + return; + + /* Flush the PTs only if we're already awake */ + if (!pm_runtime_get_if_in_use(pfdev->dev)) + return; Is the MMU guaranteed to be reset on resume such that the TLBs will always come up clean? Otherwise there are potentially corners where stale entries that we skip here might hang around if the hardware lies about powering things down. Robin. + + mmu_hw_do_operation(pfdev, mmu, iova, size, AS_COMMAND_FLUSH_PT); + + pm_runtime_mark_last_busy(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->dev); +} + static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, u64 iova, int prot, struct sg_table *sgt) { @@ -246,11 +263,10 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, } } - mmu_hw_do_operation(pfdev, mmu, start_iova, iova - start_iova, - AS_COMMAND_FLUSH_PT); - mutex_unlock(&mmu->lock); + panfrost_mmu_flush_range(pfdev, mmu, start_iova, iova - start_iova); + return 0; } @@ -259,7 +275,6 @@ int panfrost_mmu_map(struct panfrost_gem_object *bo) struct drm_gem_object *obj = &bo->base.base; struct panfrost_device *pfdev = to_panfrost_device(obj->dev); struct sg_table *sgt; - int ret; int prot = IOMMU_READ | IOMMU_WRITE; if (WARN_ON(bo->is_mapped)) @@ -272,14 +287,7 @@ int panfrost_mmu_map(struct panfrost_gem_object *bo) if (WARN_ON(IS_ERR(sgt))) return PTR_ERR(sgt); - ret = pm_runtime_get_sync(pfdev->dev); - if (ret < 0) - return ret; - mmu_map_sg(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT, prot, sgt); - - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_put_autosuspend(pfdev->dev); bo->is_mapped = true; return 0; @@ -293,17 +301,12 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo) u64 iova = bo->node.start << PAGE_SHIFT; size_t len = bo->node.size << PAGE_SHIFT; size_t unmapped_len = 0; - int ret; if (WARN_ON(!bo->is_mapped)) return; dev_dbg(pfdev->dev, "unmap: as=%d, iova=%llx, len=%zx", bo->mmu->as, iova, len); - ret = pm_runtime_get_sync(pfdev->dev); - if (ret < 0) - return; - mutex_lock(&bo->mmu->lock); while (unmapped_len < len) { @@ -318,13 +321,9 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo) unmapped_len += pgsize; } - mmu_hw_do_operation(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT, - bo->node.size << PAGE_SHIFT, AS_COMMAND_FLUSH_PT); - mutex_unlock(&bo->mmu->lock); - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_put_autosuspend(pfdev->dev); + panfrost_mmu_flush_range(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT, len); bo->is_mapped = false; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
On Fri, 23 Aug 2019 07:30:07 + wrote: > Hi Laurent, > > On 23/08/2019 at 03:40, Laurent Pinchart wrote: > > Add a type field to the drm_panel structure to report the panel type, > > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS, > > eDP, DSI and DPI). This will be used to initialise the corresponding > > connector type. > > With Microchip/Atmel driver, we mainly (only) use the "Unknown" type of > connector because our hardware simply uses RGB wires in parallel. > > Should we move to another connector type (maybe now that it's created > and it was not, back when we chose the "Unknown" one)? I confirm, DRM_MODE_ENCODER_DPI was not defined when I switched from _LVDS (which was wrong) to _Unknown. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
Hi Sam, On Fri, Aug 23, 2019 at 06:49:49AM +0200, Sam Ravnborg wrote: > Hi Laurent. > > Thanks for looking into this, but you will not be happy in a minute... Could be worse :-) > On Fri, Aug 23, 2019 at 04:40:32AM +0300, Laurent Pinchart wrote: > > Add a type field to the drm_panel structure to report the panel type, > > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS, > > eDP, DSI and DPI). > > > This will be used to initialise the corresponding connector type. > Ohh, that explains what this should be used for. > I had missed that we have the panel when we create the drm_connector. > > (I had seen we had to use the connector type to match a panel with a > connector or something like that). > > > diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c > > b/drivers/gpu/drm/panel/panel-arm-versatile.c > > index 5f72c922a04b..5c335fc1632b 100644 > > --- a/drivers/gpu/drm/panel/panel-arm-versatile.c > > +++ b/drivers/gpu/drm/panel/panel-arm-versatile.c > > @@ -353,6 +353,7 @@ static int versatile_panel_probe(struct platform_device > > *pdev) > > drm_panel_init(&vpanel->panel); > > vpanel->panel.dev = dev; > > vpanel->panel.funcs = &versatile_panel_drm_funcs; > > + vpanel->panel.type = DRM_MODE_CONNECTOR_DPI; > > The pattern where we call a simple init like here, > and then have a set of mandatory assignments right after > is not a good way to help the driver writer. > > We should instead do: > > drm_panel_init(&vpanel->panel, dev, &versatile_panel_drm_funcs, > DRM_MODE_CONNECTOR_DPI); > > Then all drm_panel users are forced to supply the init parameters, > and we do not secretly rely on some defaults for panels that do not have it. > Also new panels will fail to build if they do not specify the new field. I'll fix that. > This may also allow us to chatch that: > > > diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > > b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > > index b5b14aa059ea..cac074939e2c 100644 > > --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > > +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > > @@ -428,6 +428,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c, > > > > ts->base.dev = dev; > > ts->base.funcs = &rpi_touchscreen_funcs; > > + ts->base.type = DRM_MODE_CONNECTOR_DSI; > > forgets to call drm_panel_init() > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index 28fa6ba7b767..6d5d0c51e97e 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -94,6 +94,7 @@ struct panel_desc { > > > > u32 bus_format; > > u32 bus_flags; > > + unsigned int type; > > }; > > > > struct panel_simple { > > @@ -467,6 +468,7 @@ static int panel_simple_probe(struct device *dev, const > > struct panel_desc *desc) > > drm_panel_init(&panel->base); > > panel->base.dev = dev; > > panel->base.funcs = &panel_simple_funcs; > > + panel->base.type = desc->type ? desc->type : DRM_MODE_CONNECTOR_DPI; > > > > err = drm_panel_add(&panel->base); > > if (err < 0) > > @@ -833,6 +835,7 @@ static const struct panel_desc auo_g133han01 = { > > .unprepare = 1000, > > }, > > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, > > + .type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct display_timing auo_g185han01_timings = { > > @@ -862,6 +865,7 @@ static const struct panel_desc auo_g185han01 = { > > .unprepare = 1000, > > }, > > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, > > + .type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct display_timing auo_p320hvn03_timings = { > > @@ -890,6 +894,7 @@ static const struct panel_desc auo_p320hvn03 = { > > .unprepare = 500, > > }, > > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, > > + .type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode auo_t215hvn01_mode = { > > @@ -1205,6 +1210,7 @@ static const struct panel_desc dlc_dlc0700yzg_1 = { > > .disable = 200, > > }, > > .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > > + .type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct display_timing dlc_dlc1010gig_timing = { > > @@ -1235,6 +1241,7 @@ static const struct panel_desc dlc_dlc1010gig = { > > .unprepare = 60, > > }, > > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, > > + .type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode edt_et035012dm6_mode = { > > @@ -1501,6 +1508,7 @@ static const struct panel_desc hannstar_hsd070pww1 = { > > .height = 94, > > }, > > .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > > + .type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct display_timing hannstar_hsd100pxn1_timing = { > > @@ -1525,6 +1533,7 @@ stat
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #115 from tempel.jul...@gmail.com --- There is a new patch series that should allow memclock switching with same refreshrate multi monitor setup: https://lists.freedesktop.org/archives/amd-gfx/2019-August/038995.html -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/8] drm/panfrost: Rework runtime PM initialization
On Fri, Aug 23, 2019 at 5:54 AM Robin Murphy wrote: > > On 23/08/2019 03:12, Rob Herring wrote: > > There's a few issues with the runtime PM initialization. > > > > The documentation states pm_runtime_set_active() should be called before > > pm_runtime_enable(). The pm_runtime_put_autosuspend() could suspend the GPU > > before panfrost_perfcnt_init() is called which touches the h/w. The > > autosuspend delay keeps things from breaking. There's no need explicitly > > power off the GPU only to wake back up with pm_runtime_get_sync(). Just > > delaying pm_runtime_enable to the end of probe is sufficient. > > > > Lets move all the runtime PM calls into the probe() function so they are > > all in one place and are done after all initialization. > > > > Cc: Tomeu Vizoso > > Cc: Steven Price > > Cc: Alyssa Rosenzweig > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Rob Herring > > --- > > v2: new patch > > > > drivers/gpu/drm/panfrost/panfrost_device.c | 9 - > > drivers/gpu/drm/panfrost/panfrost_drv.c| 10 ++ > > 2 files changed, 6 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > > b/drivers/gpu/drm/panfrost/panfrost_device.c > > index 4da71bb56c20..73805210834e 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > > @@ -5,7 +5,6 @@ > > #include > > #include > > #include > > -#include > > #include > > > > #include "panfrost_device.h" > > @@ -166,14 +165,6 @@ int panfrost_device_init(struct panfrost_device *pfdev) > > if (err) > > goto err_out4; > > > > - /* runtime PM will wake us up later */ > > - panfrost_gpu_power_off(pfdev); > > - > > - pm_runtime_set_active(pfdev->dev); > > - pm_runtime_get_sync(pfdev->dev); > > - pm_runtime_mark_last_busy(pfdev->dev); > > - pm_runtime_put_autosuspend(pfdev->dev); > > - > > err = panfrost_perfcnt_init(pfdev); > > if (err) > > goto err_out5; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > > b/drivers/gpu/drm/panfrost/panfrost_drv.c > > index d74442d71048..f27e3d6aec12 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > @@ -523,10 +523,6 @@ static int panfrost_probe(struct platform_device *pdev) > > mutex_init(&pfdev->shrinker_lock); > > INIT_LIST_HEAD(&pfdev->shrinker_list); > > > > - pm_runtime_use_autosuspend(pfdev->dev); > > - pm_runtime_set_autosuspend_delay(pfdev->dev, 50); /* ~3 frames */ > > - pm_runtime_enable(pfdev->dev); > > - > > err = panfrost_device_init(pfdev); > > if (err) { > > if (err != -EPROBE_DEFER) > > @@ -541,6 +537,12 @@ static int panfrost_probe(struct platform_device *pdev) > > goto err_out1; > > } > > > > + pm_runtime_set_active(pfdev->dev); > > + pm_runtime_use_autosuspend(pfdev->dev); > > + pm_runtime_set_autosuspend_delay(pfdev->dev, 0); /* ~3 frames */ > > Hmm... different timeout, same comment - something seems amiss there, > unless perhaps it's all within rounding error anyway? Leftover debugging to force issues. It should be 50. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] kernel.h: Add non_block_start/end()
On Fri, Aug 23, 2019 at 09:12:34AM -0300, Jason Gunthorpe wrote: > I still haven't heard a satisfactory answer why a whole new scheme is > needed and a simple: > >if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_SLEEP)) > preempt_disable() > > isn't sufficient to catch the problematic cases during debugging?? > IMHO the fact preempt is changed by the above when debugging is not > material here. I think that information should be included in the > commit message at least. That has a much larger impact and actually changes behaviour, while the relatively simple patch Daniel proposed only adds a warning but doesn't affect behaviour. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: Add Steven and Alyssa as panfrost reviewers
On 8/23/19 3:33 AM, Rob Herring wrote: Add Steven Price and Alyssa Rosenzweig as reviewers as they have been the primary reviewers already. Cc: Steven Price Cc: Alyssa Rosenzweig Cc: Tomeu Vizoso Signed-off-by: Rob Herring Acked-by: Tomeu Vizoso --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 67b75fc33c61..28f4a20940cb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1255,6 +1255,8 @@ F:Documentation/gpu/afbc.rst ARM MALI PANFROST DRM DRIVER M:Rob Herring M:Tomeu Vizoso +R: Steven Price +R: Alyssa Rosenzweig L:dri-devel@lists.freedesktop.org S:Supported T:git git://anongit.freedesktop.org/drm/drm-misc ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context
On 23/08/2019 03:12, Rob Herring wrote: tlb_inv_context() hook is only called when freeing the page tables. There's no point in flushing only to free the page tables immediately following. FWIW, in general the point of flushing is *because* we're about to free the pagetables - if there's any possibility that the hardware could continue to issue translation table walks (speculative or otherwise) after those pages have been reused by someone else, TLB badness may ensue. For panfrost in particular I suspect we can probably get away without it, at least for the moment, but it might be worth moving the flush to mmu_disable() for complete peace of mind (which kind of preempts the sort of thing that per-process AS switching will want anyway). Robin. There is also a problem that we could be accessing the h/w when suspended. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index ccf671a9c3fb..9f85275a896c 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -328,11 +328,7 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo) } static void mmu_tlb_inv_context_s1(void *cookie) -{ - struct panfrost_file_priv *priv = cookie; - - mmu_hw_do_operation(priv->pfdev, &priv->mmu, 0, ~0UL, AS_COMMAND_FLUSH_MEM); -} +{} static void mmu_tlb_inv_range_nosync(unsigned long iova, size_t size, size_t granule, bool leaf, void *cookie) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context
On Fri, Aug 23, 2019 at 7:56 AM Robin Murphy wrote: > > On 23/08/2019 03:12, Rob Herring wrote: > > tlb_inv_context() hook is only called when freeing the page tables. There's > > no point in flushing only to free the page tables immediately following. > > FWIW, in general the point of flushing is *because* we're about to free > the pagetables - if there's any possibility that the hardware could > continue to issue translation table walks (speculative or otherwise) > after those pages have been reused by someone else, TLB badness may ensue. > > For panfrost in particular I suspect we can probably get away without > it, at least for the moment, but it might be worth moving the flush to > mmu_disable() for complete peace of mind (which kind of preempts the > sort of thing that per-process AS switching will want anyway). There's bigger problem that mmu_disable() is still only called for AS0 and only for driver unload. I guess we should fix that and then figure out where a flush is needed if at all. I would think changing the TTBR would be enough to quiesce the h/w and TLBs. That seems to be the case in my testing of switching address spaces. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] kernel.h: Add non_block_start/end()
On Fri, Aug 23, 2019 at 2:12 PM Jason Gunthorpe wrote: > > On Fri, Aug 23, 2019 at 10:34:01AM +0200, Daniel Vetter wrote: > > On Fri, Aug 23, 2019 at 1:14 AM Andrew Morton > > wrote: > > > > > > On Tue, 20 Aug 2019 22:24:40 +0200 Daniel Vetter wrote: > > > > > > > Hi Peter, > > > > > > > > Iirc you've been involved at least somewhat in discussing this. -mm > > > > folks > > > > are a bit undecided whether these new non_block semantics are a good > > > > idea. > > > > Michal Hocko still is in support, but Andrew Morton and Jason Gunthorpe > > > > are less enthusiastic. Jason said he's ok with merging the hmm side of > > > > this if scheduler folks ack. If not, then I'll respin with the > > > > preempt_disable/enable instead like in v1. > > > > > > I became mollified once Michel explained the rationale. I think it's > > > OK. It's very specific to the oom reaper and hopefully won't be used > > > more widely(?). > > > > Yeah, no plans for that from me. And I hope the comment above them now > > explains why they exist, so people think twice before using it in > > random places. > > I still haven't heard a satisfactory answer why a whole new scheme is > needed and a simple: > >if (IS_ENABLED(CONFIG_DEBUG_ATOMIC_SLEEP)) > preempt_disable() > > isn't sufficient to catch the problematic cases during debugging?? > IMHO the fact preempt is changed by the above when debugging is not > material here. I think that information should be included in the > commit message at least. > > But if sched people are happy then lets go ahead. Can you send a v2 > with the check encompassing the invalidate_range_end? Yes I will resend with this patch plus the next, amended as we discussed, plus the might_sleep annotations. I'm assuming the lockdep one will land, so not going to resend that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/komeda: Add missing of_node_get() call
On Tue, Aug 20, 2019 at 03:16:58PM +, Mihail Atanassov wrote: > komeda_pipeline_destroy has the matching of_node_put(). > > Fixes: 29e56aec911dd ("drm/komeda: Add DT parsing") > Signed-off-by: Mihail Atanassov > --- > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > index 0142ee991957..ca64a129c594 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > @@ -130,7 +130,7 @@ static int komeda_parse_pipe_dt(struct komeda_dev *mdev, > struct device_node *np) > of_graph_get_port_by_id(np, KOMEDA_OF_PORT_OUTPUT); > > pipe->dual_link = pipe->of_output_links[0] && pipe->of_output_links[1]; > - pipe->of_node = np; > + pipe->of_node = of_node_get(np); > Good catch. Reviewed-by: Ayan Kumar Halder > return 0; > } > -- > 2.22.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context
On 23/08/2019 14:18, Rob Herring wrote: On Fri, Aug 23, 2019 at 7:56 AM Robin Murphy wrote: On 23/08/2019 03:12, Rob Herring wrote: tlb_inv_context() hook is only called when freeing the page tables. There's no point in flushing only to free the page tables immediately following. FWIW, in general the point of flushing is *because* we're about to free the pagetables - if there's any possibility that the hardware could continue to issue translation table walks (speculative or otherwise) after those pages have been reused by someone else, TLB badness may ensue. For panfrost in particular I suspect we can probably get away without it, at least for the moment, but it might be worth moving the flush to mmu_disable() for complete peace of mind (which kind of preempts the sort of thing that per-process AS switching will want anyway). There's bigger problem that mmu_disable() is still only called for AS0 and only for driver unload. Sure, but AS0 is the only one we ever enable, and conceptually we do that once at probe time (AFAICS it stays nominally live through resets and resumes), so while it may be the least-clever AS usage possible it's at least self-consistent. And at the moment the only time we'll call .tlb_inv_context is from panfrost_mmu_fini() when we're unconditionally poking registers anyway, so either way I don't think there's any actual problem today - I'm viewing this patch as getting things straight in preparation for the future. I guess we should fix that and then figure out where a flush is needed if at all. I would think changing the TTBR would be enough to quiesce the h/w and TLBs. That seems to be the case in my testing of switching address spaces. From a quick scan through kbase, kbase_mmu_disable() appears to perform an full FLUSH_MEM before rewriting TRANSTAB, and it looks like that does get called when scheduling out a context. That's in line with what I was imagining, so unless we know better for sure, we may as well play it safe and follow the same pattern. Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] kernel.h: Add non_block_start/end()
On Fri, Aug 23, 2019 at 03:42:47PM +0200, Daniel Vetter wrote: > I'm assuming the lockdep one will land, so not going to resend that. I was assuming you'd wake the might_lock_nested() along with the i915 user through the i915/drm tree. If want me to take some or all of that, lemme know. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context
On Fri, Aug 23, 2019 at 9:05 AM Robin Murphy wrote: > > On 23/08/2019 14:18, Rob Herring wrote: > > On Fri, Aug 23, 2019 at 7:56 AM Robin Murphy wrote: > >> > >> On 23/08/2019 03:12, Rob Herring wrote: > >>> tlb_inv_context() hook is only called when freeing the page tables. > >>> There's > >>> no point in flushing only to free the page tables immediately following. > >> > >> FWIW, in general the point of flushing is *because* we're about to free > >> the pagetables - if there's any possibility that the hardware could > >> continue to issue translation table walks (speculative or otherwise) > >> after those pages have been reused by someone else, TLB badness may ensue. > >> > >> For panfrost in particular I suspect we can probably get away without > >> it, at least for the moment, but it might be worth moving the flush to > >> mmu_disable() for complete peace of mind (which kind of preempts the > >> sort of thing that per-process AS switching will want anyway). > > > > There's bigger problem that mmu_disable() is still only called for AS0 > > and only for driver unload. > > Sure, but AS0 is the only one we ever enable, and conceptually we do > that once at probe time (AFAICS it stays nominally live through resets > and resumes), so while it may be the least-clever AS usage possible it's > at least self-consistent. And at the moment the only time we'll call > .tlb_inv_context is from panfrost_mmu_fini() when we're unconditionally > poking registers anyway, so either way I don't think there's any actual > problem today - I'm viewing this patch as getting things straight in > preparation for the future. It was only AS0, but we now have per FD AS. > > I guess we should fix that and then figure > > out where a flush is needed if at all. I would think changing the TTBR > > would be enough to quiesce the h/w and TLBs. That seems to be the case > > in my testing of switching address spaces. > > From a quick scan through kbase, kbase_mmu_disable() appears to perform > an full FLUSH_MEM before rewriting TRANSTAB, and it looks like that does > get called when scheduling out a context. That's in line with what I was > imagining, so unless we know better for sure, we may as well play it > safe and follow the same pattern. Agreed. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/komeda: Add missing of_node_get() call
On Fri, Aug 23, 2019 at 01:43:49PM +, Ayan Halder wrote: > On Tue, Aug 20, 2019 at 03:16:58PM +, Mihail Atanassov wrote: > > komeda_pipeline_destroy has the matching of_node_put(). > > > > Fixes: 29e56aec911dd ("drm/komeda: Add DT parsing") > > Signed-off-by: Mihail Atanassov > > --- > > drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > index 0142ee991957..ca64a129c594 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > > @@ -130,7 +130,7 @@ static int komeda_parse_pipe_dt(struct komeda_dev > > *mdev, struct device_node *np) > > of_graph_get_port_by_id(np, KOMEDA_OF_PORT_OUTPUT); > > > > pipe->dual_link = pipe->of_output_links[0] && pipe->of_output_links[1]; > > - pipe->of_node = np; > > + pipe->of_node = of_node_get(np); > > > > Good catch. > Reviewed-by: Ayan Kumar Halder > > return 0; > > } > > -- Pushed to drm-misc-fixes - 51a44a28eefd0d4c1addeb23fc5a599ff1787dfd Apologies, I accidently pushed the gerrit change-id in the commit message. Surprisingly, "checkpatch.pl --strict" did not catch the issue. > > 2.22.0 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/8] drm/panfrost: Hold runtime PM reference until jobs complete
On 23/08/2019 03:12, Rob Herring wrote: Doing a pm_runtime_put as soon as a job is submitted is wrong as it should not happen until the job completes. It works because we are relying on the autosuspend timeout to keep the h/w enabled. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Nice find, but I'm a bit worried about one thing. --- v2: new patch drivers/gpu/drm/panfrost/panfrost_job.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 05c85f45a0de..80c9cab9a01b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -151,7 +151,7 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) return; if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js - goto end; + return; cfg = panfrost_mmu_as_get(pfdev, &job->file_priv->mmu); @@ -187,10 +187,6 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START); spin_unlock_irqrestore(&pfdev->hwaccess_lock, flags); - -end: - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_put_autosuspend(pfdev->dev); } static void panfrost_acquire_object_fences(struct drm_gem_object **bos, @@ -388,9 +384,13 @@ static void panfrost_job_timedout(struct drm_sched_job *sched_job) mutex_lock(&pfdev->reset_lock); - for (i = 0; i < NUM_JOB_SLOTS; i++) + for (i = 0; i < NUM_JOB_SLOTS; i++) { drm_sched_stop(&pfdev->js->queue[i].sched, sched_job); - + if (pfdev->jobs[i]) { + pm_runtime_put_noidle(pfdev->dev); + pfdev->jobs[i] = NULL; I can't see what prevents this racing with panfrost_job_irq_handler() - the job could be completing at the same time as we assign NULL. Then panfrost_job_irq_handler() will happily dereference the NULL pointer... Admittedly this patch is an improvement over the situation before :) Steve + } + } if (sched_job) drm_sched_increase_karma(sched_job); @@ -455,7 +455,9 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) pfdev->jobs[j] = NULL; panfrost_mmu_as_put(pfdev, &job->file_priv->mmu); panfrost_devfreq_record_transition(pfdev, j); + dma_fence_signal(job->done_fence); + pm_runtime_put_autosuspend(pfdev->dev); } status &= ~mask; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/8] drm/panfrost: Fix possible suspend in panfrost_remove
On 23/08/2019 03:12, Rob Herring wrote: Calls to panfrost_device_fini() access the h/w, but we already done a pm_runtime_put_sync_autosuspend() beforehand. This only works if the autosuspend delay is long enough. A 0ms delay will hang the system when removing the device. Fix this by moving the pm_runtime_put_sync_suspend() after the panfrost_device_fini() call. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price --- v2: new patch drivers/gpu/drm/panfrost/panfrost_drv.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 44a558c6e17e..d74442d71048 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -570,11 +570,13 @@ static int panfrost_remove(struct platform_device *pdev) drm_dev_unregister(ddev); panfrost_gem_shrinker_cleanup(ddev); + pm_runtime_get_sync(pfdev->dev); - pm_runtime_put_sync_autosuspend(pfdev->dev); - pm_runtime_disable(pfdev->dev); panfrost_devfreq_fini(pfdev); panfrost_device_fini(pfdev); + pm_runtime_put_sync_suspend(pfdev->dev); + pm_runtime_disable(pfdev->dev); + drm_dev_put(ddev); return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/8] drm/shmem: Use mutex_trylock in drm_gem_shmem_purge
On 23/08/2019 03:12, Rob Herring wrote: Lockdep reports a circular locking dependency with pages_lock taken in the shrinker callback. The deadlock can't actually happen with current users at least as a BO will never be purgeable when pages_lock is held. To be safe, let's use mutex_trylock() instead and bail if a BO is locked already. WARNING: possible circular locking dependency detected 5.3.0-rc1+ #100 Tainted: G L -- kswapd0/171 is trying to acquire lock: 9b9823fd (&shmem->pages_lock){+.+.}, at: drm_gem_shmem_purge+0x20/0x40 but task is already holding lock: f82369b6 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x40 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (fs_reclaim){+.+.}: fs_reclaim_acquire.part.18+0x34/0x40 fs_reclaim_acquire+0x20/0x28 __kmalloc_node+0x6c/0x4c0 kvmalloc_node+0x38/0xa8 drm_gem_get_pages+0x80/0x1d0 drm_gem_shmem_get_pages+0x58/0xa0 drm_gem_shmem_get_pages_sgt+0x48/0xd0 panfrost_mmu_map+0x38/0xf8 [panfrost] panfrost_gem_open+0xc0/0xe8 [panfrost] drm_gem_handle_create_tail+0xe8/0x198 drm_gem_handle_create+0x3c/0x50 panfrost_gem_create_with_handle+0x70/0xa0 [panfrost] panfrost_ioctl_create_bo+0x48/0x80 [panfrost] drm_ioctl_kernel+0xb8/0x110 drm_ioctl+0x244/0x3f0 do_vfs_ioctl+0xbc/0x910 ksys_ioctl+0x78/0xa8 __arm64_sys_ioctl+0x1c/0x28 el0_svc_common.constprop.0+0x90/0x168 el0_svc_handler+0x28/0x78 el0_svc+0x8/0xc -> #0 (&shmem->pages_lock){+.+.}: __lock_acquire+0xa2c/0x1d70 lock_acquire+0xdc/0x228 __mutex_lock+0x8c/0x800 mutex_lock_nested+0x1c/0x28 drm_gem_shmem_purge+0x20/0x40 panfrost_gem_shrinker_scan+0xc0/0x180 [panfrost] do_shrink_slab+0x208/0x500 shrink_slab+0x10c/0x2c0 shrink_node+0x28c/0x4d8 balance_pgdat+0x2c8/0x570 kswapd+0x22c/0x638 kthread+0x128/0x130 ret_from_fork+0x10/0x18 other info that might help us debug this: Possible unsafe locking scenario: CPU0CPU1 lock(fs_reclaim); lock(&shmem->pages_lock); lock(fs_reclaim); lock(&shmem->pages_lock); *** DEADLOCK *** 3 locks held by kswapd0/171: #0: f82369b6 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x40 #1: ceb37808 (shrinker_rwsem){}, at: shrink_slab+0xbc/0x2c0 #2: f31efa81 (&pfdev->shrinker_lock){+.+.}, at: panfrost_gem_shrinker_scan+0x34/0x180 [panfrost] Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers") Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring I thought I'd already given my R-b for this one, but just in case: Reviewed-by: Steven Price Steve --- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 +-- include/drm/drm_gem_shmem_helper.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 5423ec56b535..f5918707672f 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -415,13 +415,16 @@ void drm_gem_shmem_purge_locked(struct drm_gem_object *obj) } EXPORT_SYMBOL(drm_gem_shmem_purge_locked); -void drm_gem_shmem_purge(struct drm_gem_object *obj) +bool drm_gem_shmem_purge(struct drm_gem_object *obj) { struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); - mutex_lock(&shmem->pages_lock); + if (!mutex_trylock(&shmem->pages_lock)) + return false; drm_gem_shmem_purge_locked(obj); mutex_unlock(&shmem->pages_lock); + + return true; } EXPORT_SYMBOL(drm_gem_shmem_purge); diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h index ce1600fdfc3e..01f514521687 100644 --- a/include/drm/drm_gem_shmem_helper.h +++ b/include/drm/drm_gem_shmem_helper.h @@ -134,7 +134,7 @@ static inline bool drm_gem_shmem_is_purgeable(struct drm_gem_shmem_object *shmem } void drm_gem_shmem_purge_locked(struct drm_gem_object *obj); -void drm_gem_shmem_purge(struct drm_gem_object *obj); +bool drm_gem_shmem_purge(struct drm_gem_object *obj); struct drm_gem_shmem_object * drm_gem_shmem_create_with_handle(struct drm_file *file_priv, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 6/8] drm/panfrost: Use mutex_trylock in panfrost_gem_purge
On 23/08/2019 03:12, Rob Herring wrote: Lockdep reports a circular locking dependency with pages_lock taken in the shrinker callback. The deadlock can't actually happen with current users at least as a BO will never be purgeable when pages_lock is held. To be safe, let's use mutex_trylock() instead and bail if a BO is locked already. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price --- v2: new patch drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c index d191632b6197..458f0fa68111 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c @@ -36,15 +36,18 @@ panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc return count; } -static void panfrost_gem_purge(struct drm_gem_object *obj) +static bool panfrost_gem_purge(struct drm_gem_object *obj) { struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); - mutex_lock(&shmem->pages_lock); + + if (!mutex_trylock(&shmem->pages_lock)) + return false; panfrost_mmu_unmap(to_panfrost_bo(obj)); drm_gem_shmem_purge_locked(obj); mutex_unlock(&shmem->pages_lock); + return true; } static unsigned long @@ -61,8 +64,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) list_for_each_entry_safe(shmem, tmp, &pfdev->shrinker_list, madv_list) { if (freed >= sc->nr_to_scan) break; - if (drm_gem_shmem_is_purgeable(shmem)) { - panfrost_gem_purge(&shmem->base); + if (drm_gem_shmem_is_purgeable(shmem) && + panfrost_gem_purge(&shmem->base)) { freed += shmem->base.size >> PAGE_SHIFT; list_del_init(&shmem->madv_list); } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context
On 23/08/2019 15:26, Rob Herring wrote: On Fri, Aug 23, 2019 at 9:05 AM Robin Murphy wrote: On 23/08/2019 14:18, Rob Herring wrote: On Fri, Aug 23, 2019 at 7:56 AM Robin Murphy wrote: On 23/08/2019 03:12, Rob Herring wrote: tlb_inv_context() hook is only called when freeing the page tables. There's no point in flushing only to free the page tables immediately following. FWIW, in general the point of flushing is *because* we're about to free the pagetables - if there's any possibility that the hardware could continue to issue translation table walks (speculative or otherwise) after those pages have been reused by someone else, TLB badness may ensue. For panfrost in particular I suspect we can probably get away without it, at least for the moment, but it might be worth moving the flush to mmu_disable() for complete peace of mind (which kind of preempts the sort of thing that per-process AS switching will want anyway). There's bigger problem that mmu_disable() is still only called for AS0 and only for driver unload. Sure, but AS0 is the only one we ever enable, and conceptually we do that once at probe time (AFAICS it stays nominally live through resets and resumes), so while it may be the least-clever AS usage possible it's at least self-consistent. And at the moment the only time we'll call .tlb_inv_context is from panfrost_mmu_fini() when we're unconditionally poking registers anyway, so either way I don't think there's any actual problem today - I'm viewing this patch as getting things straight in preparation for the future. It was only AS0, but we now have per FD AS. Ah, silly me, I forgot to look at -next... I guess we should fix that and then figure out where a flush is needed if at all. I would think changing the TTBR would be enough to quiesce the h/w and TLBs. That seems to be the case in my testing of switching address spaces. From a quick scan through kbase, kbase_mmu_disable() appears to perform an full FLUSH_MEM before rewriting TRANSTAB, and it looks like that does get called when scheduling out a context. That's in line with what I was imagining, so unless we know better for sure, we may as well play it safe and follow the same pattern. Agreed. I guess in that case we probably want it in panfrost_mmu_as_put() when as_count falls to 0, to balance the corresponding enable in as_get(). If the tables are only freed later when the FD is closed and whichever AS is probably in use by some other job, that's even more reason that .tlb_inv_context is now the wrong place to be touching hardware. Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On 23/08/2019 12:11, Robin Murphy wrote: On 23/08/2019 03:12, Rob Herring wrote: There is no point in resuming the h/w just to do flush operations and doing so takes several locks which cause lockdep issues with the shrinker. Rework the flush operations to only happen when the h/w is already awake. This avoids taking any locks associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..ccf671a9c3fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) return SZ_2M; } +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, + struct panfrost_mmu *mmu, + u64 iova, size_t size) +{ + if (mmu->as < 0) + return; + + /* Flush the PTs only if we're already awake */ + if (!pm_runtime_get_if_in_use(pfdev->dev)) + return; Is the MMU guaranteed to be reset on resume such that the TLBs will always come up clean? Otherwise there are potentially corners where stale entries that we skip here might hang around if the hardware lies about powering things down. Assuming runtime PM has actually committed to the power off, then on power on panfrost_device_resume() will be called which performs a reset of the GPU which will clear the L2/TLBs so there shouldn't be any problem there. As far as I can see from the code that is how pm_runtime_get_if_in_use() works - it will return true unless the suspend() callback has been called. Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On 23/08/2019 03:12, Rob Herring wrote: There is no point in resuming the h/w just to do flush operations and doing so takes several locks which cause lockdep issues with the shrinker. Rework the flush operations to only happen when the h/w is already awake. This avoids taking any locks associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price But one comment below... --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..ccf671a9c3fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) return SZ_2M; } +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, + struct panfrost_mmu *mmu, + u64 iova, size_t size) +{ + if (mmu->as < 0) + return; + + /* Flush the PTs only if we're already awake */ + if (!pm_runtime_get_if_in_use(pfdev->dev)) + return; + + mmu_hw_do_operation(pfdev, mmu, iova, size, AS_COMMAND_FLUSH_PT); + + pm_runtime_mark_last_busy(pfdev->dev); This isn't really a change, but: I'm not sure why we want to signal we were busy just because we had to do some cache maintenance? We might actually be faster leaving the GPU off so there's no need to do extra flushes on the GPU? Steve + pm_runtime_put_autosuspend(pfdev->dev); +} + static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, u64 iova, int prot, struct sg_table *sgt) { @@ -246,11 +263,10 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, } } - mmu_hw_do_operation(pfdev, mmu, start_iova, iova - start_iova, - AS_COMMAND_FLUSH_PT); - mutex_unlock(&mmu->lock); + panfrost_mmu_flush_range(pfdev, mmu, start_iova, iova - start_iova); + return 0; } @@ -259,7 +275,6 @@ int panfrost_mmu_map(struct panfrost_gem_object *bo) struct drm_gem_object *obj = &bo->base.base; struct panfrost_device *pfdev = to_panfrost_device(obj->dev); struct sg_table *sgt; - int ret; int prot = IOMMU_READ | IOMMU_WRITE; if (WARN_ON(bo->is_mapped)) @@ -272,14 +287,7 @@ int panfrost_mmu_map(struct panfrost_gem_object *bo) if (WARN_ON(IS_ERR(sgt))) return PTR_ERR(sgt); - ret = pm_runtime_get_sync(pfdev->dev); - if (ret < 0) - return ret; - mmu_map_sg(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT, prot, sgt); - - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_put_autosuspend(pfdev->dev); bo->is_mapped = true; return 0; @@ -293,17 +301,12 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo) u64 iova = bo->node.start << PAGE_SHIFT; size_t len = bo->node.size << PAGE_SHIFT; size_t unmapped_len = 0; - int ret; if (WARN_ON(!bo->is_mapped)) return; dev_dbg(pfdev->dev, "unmap: as=%d, iova=%llx, len=%zx", bo->mmu->as, iova, len); - ret = pm_runtime_get_sync(pfdev->dev); - if (ret < 0) - return; - mutex_lock(&bo->mmu->lock); while (unmapped_len < len) { @@ -318,13 +321,9 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo) unmapped_len += pgsize; } - mmu_hw_do_operation(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT, - bo->node.size << PAGE_SHIFT, AS_COMMAND_FLUSH_PT); - mutex_unlock(&bo->mmu->lock); - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_put_autosuspend(pfdev->dev); + panfrost_mmu_flush_range(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT, len); bo->is_mapped = false; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context
On 23/08/2019 03:12, Rob Herring wrote: tlb_inv_context() hook is only called when freeing the page tables. There's no point in flushing only to free the page tables immediately following. There is also a problem that we could be accessing the h/w when suspended. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price Although it might be worth trying to capture the justification about this in a comment somewhere - there's been a fair bit of discussion about this... Steve --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index ccf671a9c3fb..9f85275a896c 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -328,11 +328,7 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo) } static void mmu_tlb_inv_context_s1(void *cookie) -{ - struct panfrost_file_priv *priv = cookie; - - mmu_hw_do_operation(priv->pfdev, &priv->mmu, 0, ~0UL, AS_COMMAND_FLUSH_MEM); -} +{} static void mmu_tlb_inv_range_nosync(unsigned long iova, size_t size, size_t granule, bool leaf, void *cookie) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/8] drm/panfrost: Hold runtime PM reference until jobs complete
On Fri, Aug 23, 2019 at 9:50 AM Steven Price wrote: > > On 23/08/2019 03:12, Rob Herring wrote: > > Doing a pm_runtime_put as soon as a job is submitted is wrong as it should > > not happen until the job completes. It works because we are relying on the > > autosuspend timeout to keep the h/w enabled. > > > > Cc: Tomeu Vizoso > > Cc: Steven Price > > Cc: Alyssa Rosenzweig > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Rob Herring > > Nice find, but I'm a bit worried about one thing. > > > --- > > v2: new patch > > > > drivers/gpu/drm/panfrost/panfrost_job.c | 16 +--- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c > > b/drivers/gpu/drm/panfrost/panfrost_job.c > > index 05c85f45a0de..80c9cab9a01b 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > > @@ -151,7 +151,7 @@ static void panfrost_job_hw_submit(struct panfrost_job > > *job, int js) > > return; > > > > if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js > > - goto end; > > + return; > > > > cfg = panfrost_mmu_as_get(pfdev, &job->file_priv->mmu); > > > > @@ -187,10 +187,6 @@ static void panfrost_job_hw_submit(struct panfrost_job > > *job, int js) > > job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START); > > > > spin_unlock_irqrestore(&pfdev->hwaccess_lock, flags); > > - > > -end: > > - pm_runtime_mark_last_busy(pfdev->dev); > > - pm_runtime_put_autosuspend(pfdev->dev); > > } > > > > static void panfrost_acquire_object_fences(struct drm_gem_object **bos, > > @@ -388,9 +384,13 @@ static void panfrost_job_timedout(struct drm_sched_job > > *sched_job) > > > > mutex_lock(&pfdev->reset_lock); > > > > - for (i = 0; i < NUM_JOB_SLOTS; i++) > > + for (i = 0; i < NUM_JOB_SLOTS; i++) { > > drm_sched_stop(&pfdev->js->queue[i].sched, sched_job); > > - > > + if (pfdev->jobs[i]) { > > + pm_runtime_put_noidle(pfdev->dev); > > + pfdev->jobs[i] = NULL; > > I can't see what prevents this racing with panfrost_job_irq_handler() - > the job could be completing at the same time as we assign NULL. Then > panfrost_job_irq_handler() will happily dereference the NULL pointer... The fact that 1 job's timeout handler is cleaning up all the other jobs is messy. I'm not sure if there's a better way... We could perhaps disable the job interrupt at the beginning though I think we can still have a race as we can't use disable_irq() with a shared irq. Or do this after the reset. > Admittedly this patch is an improvement over the situation before :) Yes, and I'd like to stop digging a deeper hole... Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] kernel.h: Add non_block_start/end()
On Fri, Aug 23, 2019 at 4:06 PM Peter Zijlstra wrote: > On Fri, Aug 23, 2019 at 03:42:47PM +0200, Daniel Vetter wrote: > > I'm assuming the lockdep one will land, so not going to resend that. > > I was assuming you'd wake the might_lock_nested() along with the i915 > user through the i915/drm tree. If want me to take some or all of that, > lemme know. might_lock_nested() is a different patch series, that one will indeed go in through the drm/i915 tree, thx for the ack there. What I meant here is some mmu notifier lockdep map in this series that Jason said he's going to pick up into hmm.git. I'm doing about 3 or 4 different lockdep annotations series in parallel right now :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] drm/imx: IPUv3 image converter fixes and improvements
Hi Dave, Daniel, please consider merging these image converter fixes for v5.4, which avoid image degradation due to intermediate downscaling in some tiled conversions and fix 1:1 conversions that are multiples of the hardware tile size. This also adds RGBX and BGRX format support now that these are defined for V4L2, and there is a patch to remove a leftover line from the Makefile. regards Philipp The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b: Linus 5.3-rc1 (2019-07-21 14:05:38 -0700) are available in the Git repository at: git://git.pengutronix.de/pza/linux.git tags/imx-drm-next-2019-08-23 for you to fetch changes up to 4d24376370fbfc87231d54434a683f2913abcce4: gpu: ipu-v3: image-convert: only sample into the next tile if necessary (2019-08-19 16:25:30 +0200) drm/imx: IPUv3 image converter fixes and improvements Fix image converter seam handling for 1024x1024 pixel hardware limitation at the main processing section input, improve error handling, and slightly optimize for 1:1 conversions. Add support for newly defined 32-bit RGB V4L2 pixel formats. Guido Günther (1): drm/imx: Drop unused imx-ipuv3-crtc.o build Philipp Zabel (9): gpu: ipu-v3: enable remaining 32-bit RGB V4L2 pixel formats gpu: ipu-v3: image-convert: enable V4L2_PIX_FMT_BGRX32 and _RGBX32 gpu: ipu-v3: image-convert: move output seam valid interval calculation into find_best_seam gpu: ipu-v3: image-convert: fix output seam valid interval gpu: ipu-v3: image-convert: limit input seam position to hardware requirements gpu: ipu-v3: image-convert: fix image downsize coefficients and tiling calculation gpu: ipu-v3: image-convert: bail on invalid tile sizes gpu: ipu-v3: image-convert: move tile burst alignment out of loop gpu: ipu-v3: image-convert: only sample into the next tile if necessary drivers/gpu/drm/imx/Makefile | 1 - drivers/gpu/ipu-v3/ipu-common.c| 16 ++- drivers/gpu/ipu-v3/ipu-cpmem.c | 26 +++- drivers/gpu/ipu-v3/ipu-image-convert.c | 230 - 4 files changed, 177 insertions(+), 96 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
Hi Brendan, On 8/20/19 5:20 PM, Brendan Higgins wrote: Add core facilities for defining unit tests; this provides a common way to define test cases, functions that execute code which is under test and determine whether the code under test behaves as expected; this also provides a way to group together related test cases in test suites (here we call them test_modules). Just define test cases and how to execute them for now; setting expectations on code will be defined later. Signed-off-by: Brendan Higgins Reviewed-by: Greg Kroah-Hartman Reviewed-by: Logan Gunthorpe Reviewed-by: Luis Chamberlain Reviewed-by: Stephen Boyd --- include/kunit/test.h | 179 kunit/Kconfig| 17 kunit/Makefile | 1 + kunit/test.c | 191 +++ 4 files changed, 388 insertions(+) create mode 100644 include/kunit/test.h create mode 100644 kunit/Kconfig create mode 100644 kunit/Makefile create mode 100644 kunit/test.c diff --git a/include/kunit/test.h b/include/kunit/test.h new file mode 100644 index 0..e0b34acb9ee4e --- /dev/null +++ b/include/kunit/test.h @@ -0,0 +1,179 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Base unit test (KUnit) API. + * + * Copyright (C) 2019, Google LLC. + * Author: Brendan Higgins + */ + +#ifndef _KUNIT_TEST_H +#define _KUNIT_TEST_H + +#include + +struct kunit; + +/** + * struct kunit_case - represents an individual test case. + * @run_case: the function representing the actual test case. + * @name: the name of the test case. + * + * A test case is a function with the signature, ``void (*)(struct kunit *)`` + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each + * test case is associated with a &struct kunit_suite and will be run after the + * suite's init function and followed by the suite's exit function. + * + * A test case should be static and should only be created with the KUNIT_CASE() + * macro; additionally, every array of test cases should be terminated with an + * empty test case. + * + * Example: Can you fix these line continuations. It makes it very hard to read. Sorry for this late comment. These comments lines are longer than 80 and wrap. There are several comment lines in the file that are way too long. thanks, -- Shuah ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v14 09/18] kunit: test: add support for test abort
Hi Brendan, On 8/20/19 5:20 PM, Brendan Higgins wrote: Add support for aborting/bailing out of test cases, which is needed for implementing assertions. An assertion is like an expectation, but bails out of the test case early if the assertion is not met. The idea with assertions is that you use them to state all the preconditions for your test. Logically speaking, these are the premises of the test case, so if a premise isn't true, there is no point in continuing the test case because there are no conclusions that can be drawn without the premises. Whereas, the expectation is the thing you are trying to prove. Signed-off-by: Brendan Higgins Reviewed-by: Greg Kroah-Hartman Reviewed-by: Logan Gunthorpe Reviewed-by: Stephen Boyd --- include/kunit/test.h | 2 + include/kunit/try-catch.h | 75 + kunit/Makefile| 3 +- kunit/test.c | 137 +- kunit/try-catch.c | 118 5 files changed, 319 insertions(+), 16 deletions(-) create mode 100644 include/kunit/try-catch.h create mode 100644 kunit/try-catch.c diff --git a/include/kunit/test.h b/include/kunit/test.h index 6917b186b737a..390ce02f717b6 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -10,6 +10,7 @@ #define _KUNIT_TEST_H #include +#include #include #include #include @@ -167,6 +168,7 @@ struct kunit { /* private: internal use only. */ const char *name; /* Read only after initialization! */ + struct kunit_try_catch try_catch; /* * success starts as true, and may only be set to false during a test * case; thus, it is safe to update this across multiple threads using diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h new file mode 100644 index 0..404f336cbdc85 --- /dev/null +++ b/include/kunit/try-catch.h @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * An API to allow a function, that may fail, to be executed, and recover in a + * controlled manner. + * + * Copyright (C) 2019, Google LLC. + * Author: Brendan Higgins + */ + +#ifndef _KUNIT_TRY_CATCH_H +#define _KUNIT_TRY_CATCH_H + +#include + +typedef void (*kunit_try_catch_func_t)(void *); + +struct completion; +struct kunit; + +/** + * struct kunit_try_catch - provides a generic way to run code which might fail. + * @test: The test case that is currently being executed. + * @try_completion: Completion that the control thread waits on while test runs. + * @try_result: Contains any errno obtained while running test case. + * @try: The function, the test case, to attempt to run. + * @catch: The function called if @try bails out. + * @context: used to pass user data to the try and catch functions. + * + * kunit_try_catch provides a generic, architecture independent way to execute + * an arbitrary function of type kunit_try_catch_func_t which may bail out by + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try + * is stopped at the site of invocation and @catch is called. + * + * struct kunit_try_catch provides a generic interface for the functionality + * needed to implement kunit->abort() which in turn is needed for implementing + * assertions. Assertions allow stating a precondition for a test simplifying + * how test cases are written and presented. + * + * Assertions are like expectations, except they abort (call + * kunit_try_catch_throw()) when the specified condition is not met. This is + * useful when you look at a test case as a logical statement about some piece + * of code, where assertions are the premises for the test case, and the + * conclusion is a set of predicates, rather expectations, that must all be + * true. If your premises are violated, it does not makes sense to continue. + */ +struct kunit_try_catch { + /* private: internal use only. */ + struct kunit *test; + struct completion *try_completion; + int try_result; + kunit_try_catch_func_t try; + kunit_try_catch_func_t catch; + void *context; +}; + +void kunit_try_catch_init(struct kunit_try_catch *try_catch, + struct kunit *test, + kunit_try_catch_func_t try, + kunit_try_catch_func_t catch); + +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context); + +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch); + +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch) +{ + return try_catch->try_result; +} + +/* + * Exposed for testing only. + */ +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch); + +#endif /* _KUNIT_TRY_CATCH_H */ diff --git a/kunit/Makefile b/kunit/Makefile index 4e46450bcb3a8..c9176c9c578c6 100644 --- a/kunit/Makefile +++ b/kunit/Makefile @@ -1,6 +1,7 @@ obj-$(CONFIG_KUNIT) +=test.o \
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On 23/08/2019 16:05, Steven Price wrote: On 23/08/2019 12:11, Robin Murphy wrote: On 23/08/2019 03:12, Rob Herring wrote: There is no point in resuming the h/w just to do flush operations and doing so takes several locks which cause lockdep issues with the shrinker. Rework the flush operations to only happen when the h/w is already awake. This avoids taking any locks associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..ccf671a9c3fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) return SZ_2M; } +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, + struct panfrost_mmu *mmu, + u64 iova, size_t size) +{ + if (mmu->as < 0) + return; + + /* Flush the PTs only if we're already awake */ + if (!pm_runtime_get_if_in_use(pfdev->dev)) + return; Is the MMU guaranteed to be reset on resume such that the TLBs will always come up clean? Otherwise there are potentially corners where stale entries that we skip here might hang around if the hardware lies about powering things down. Assuming runtime PM has actually committed to the power off, then on power on panfrost_device_resume() will be called which performs a reset of the GPU which will clear the L2/TLBs so there shouldn't be any problem there. OK, if panfrost_gpu_soft_reset() is sufficient to guarantee clean TLBs then this looks equivalent to what we did for arm-smmu, so I've no complaints in that regard. However on second look I've now noticed the panfrost_mmu_flush_range() calls being moved outside of mmu->lock protection. Forgive me if there's basic DRM knowledge I'm missing here, but is there any possibility for multiple threads to create/import/free objects simultaneously on the same FD such that two mmu_hw_do_operation() calls could race and interfere with each other in terms of the AS_LOCKADDR/AS_COMMAND/AS_STATUS dance? Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On Fri, Aug 23, 2019 at 10:09 AM Steven Price wrote: > > On 23/08/2019 03:12, Rob Herring wrote: > > There is no point in resuming the h/w just to do flush operations and > > doing so takes several locks which cause lockdep issues with the shrinker. > > Rework the flush operations to only happen when the h/w is already awake. > > This avoids taking any locks associated with resuming. > > > > Cc: Tomeu Vizoso > > Cc: Steven Price > > Cc: Alyssa Rosenzweig > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Rob Herring > > Reviewed-by: Steven Price > > But one comment below... > > > --- > > v2: new patch > > > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - > > 1 file changed, 20 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > index 842bdd7cf6be..ccf671a9c3fb 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) > > return SZ_2M; > > } > > > > +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, > > + struct panfrost_mmu *mmu, > > + u64 iova, size_t size) > > +{ > > + if (mmu->as < 0) > > + return; > > + > > + /* Flush the PTs only if we're already awake */ > > + if (!pm_runtime_get_if_in_use(pfdev->dev)) > > + return; > > + > > + mmu_hw_do_operation(pfdev, mmu, iova, size, AS_COMMAND_FLUSH_PT); > > + > > + pm_runtime_mark_last_busy(pfdev->dev); > > This isn't really a change, but: I'm not sure why we want to signal we > were busy just because we had to do some cache maintenance? We might > actually be faster leaving the GPU off so there's no need to do extra > flushes on the GPU? I don't know, good question. Powering up and down has its cost too. We only get here if we were already active. A flush on a map probably is going to be followed by a job. An unmap may be the end of activity or not. If we don't call pm_runtime_mark_last_busy, then maybe we also want to do a pm_runtime_put_suspend (i.e. suspend immediately) instead. That may only matter if we're the last one which could only happen during this get/put. I'm not sure what happens if a suspend is requested followed by an autosuspend. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111236] VA-API radeonsi SIGSEGV __memmove_avx_unaligned
https://bugs.freedesktop.org/show_bug.cgi?id=111236 --- Comment #6 from Julien Isorce --- Looks similar to https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/merge_requests/142 It is working fine with mesa 18.3.6 and st/va has not changed in 19.1: https://cgit.freedesktop.org/mesa/mesa/log/src/gallium/state_trackers/va?h=19.1 So the issues could be due to the recent changes in drivers/radeon, drivers/radeonsi or winsys/amdgpu, i.e. : https://cgit.freedesktop.org/mesa/mesa/log/src/gallium/drivers/radeon?h=19.1 https://cgit.freedesktop.org/mesa/mesa/log/src/gallium/drivers/radeonsi?h=19.1 https://cgit.freedesktop.org/mesa/mesa/log/src/gallium/winsys/amdgpu?h=19.1 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On Fri, Aug 23, 2019 at 10:44 AM Robin Murphy wrote: > > On 23/08/2019 16:05, Steven Price wrote: > > On 23/08/2019 12:11, Robin Murphy wrote: > >> On 23/08/2019 03:12, Rob Herring wrote: > >>> There is no point in resuming the h/w just to do flush operations and > >>> doing so takes several locks which cause lockdep issues with the > >>> shrinker. > >>> Rework the flush operations to only happen when the h/w is already > >>> awake. > >>> This avoids taking any locks associated with resuming. > >>> > >>> Cc: Tomeu Vizoso > >>> Cc: Steven Price > >>> Cc: Alyssa Rosenzweig > >>> Cc: David Airlie > >>> Cc: Daniel Vetter > >>> Signed-off-by: Rob Herring > >>> --- > >>> v2: new patch > >>> > >>> drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - > >>> 1 file changed, 20 insertions(+), 21 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > >>> b/drivers/gpu/drm/panfrost/panfrost_mmu.c > >>> index 842bdd7cf6be..ccf671a9c3fb 100644 > >>> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > >>> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > >>> @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) > >>> return SZ_2M; > >>> } > >>> +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, > >>> + struct panfrost_mmu *mmu, > >>> + u64 iova, size_t size) > >>> +{ > >>> +if (mmu->as < 0) > >>> +return; > >>> + > >>> +/* Flush the PTs only if we're already awake */ > >>> +if (!pm_runtime_get_if_in_use(pfdev->dev)) > >>> +return; > >> > >> Is the MMU guaranteed to be reset on resume such that the TLBs will > >> always come up clean? Otherwise there are potentially corners where > >> stale entries that we skip here might hang around if the hardware lies > >> about powering things down. > > > > Assuming runtime PM has actually committed to the power off, then on > > power on panfrost_device_resume() will be called which performs a reset > > of the GPU which will clear the L2/TLBs so there shouldn't be any > > problem there. > > OK, if panfrost_gpu_soft_reset() is sufficient to guarantee clean TLBs > then this looks equivalent to what we did for arm-smmu, so I've no > complaints in that regard. > > However on second look I've now noticed the panfrost_mmu_flush_range() > calls being moved outside of mmu->lock protection. Forgive me if there's > basic DRM knowledge I'm missing here, but is there any possibility for > multiple threads to create/import/free objects simultaneously on the > same FD such that two mmu_hw_do_operation() calls could race and > interfere with each other in terms of the > AS_LOCKADDR/AS_COMMAND/AS_STATUS dance? Yes, we could have multiple threads. Not really any good reason it's moved out of the mmu->lock other than just to avoid any nesting (though that seemed fine in testing). The newly added as_lock will serialize mmu_hw_do_operation(). So the mmu->lock is just serializing page table writes. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Add LTTPR defines for DP 1.4
On 2019-08-22 6:46 p.m., Siqueira, Rodrigo wrote: > DP 1.4 specification defines Link Training Tunable PHY Repeater (LTTPR) > which is required to add support for systems with Thunderbolt or other > repeater devices. > > Cc: Abdoulaye Berthe > Cc: Harry Wentland > Cc: Leo Li > Signed-off-by: Rodrigo Siqueira > Signed-off-by: Abdoulaye Berthe > --- > include/drm/drm_dp_helper.h | 26 ++ > 1 file changed, 26 insertions(+) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 8364502f92cf..8336d960da7f 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -134,6 +134,32 @@ > #define DP_SUPPORTED_LINK_RATES 0x010 /* eDP 1.4 */ > # define DP_MAX_SUPPORTED_RATES 8 /* 16-bit > little-endian */ > > +/** Link Training (LT)-tunable Physical Repeaters - DP 1.4 **/ > +#define DP_LTTPR_REV 0xf > +#define DP_LTTPR_MAX_LINK_RATE0xf0001 > +#define DP_LTTPR_REPEATER_CNT 0xf0002 > +#define DP_LTTPR_REPEATER_MODE0xf0003 > +#define DP_LTTPR_MAX_LANE_CNT 0xf0004 > +#define DP_LTTPR_EXTENDED_WAIT_TIMEOUT0xf0005 > +#define DP_LTTPR_TRAINING_PATTERN_SET_REPEATER1 0xf0010 > +#define DP_LTTPR_TRAINING_LANE0_SET_REPEATER1 0xf0011 > +#define DP_LTTPR_TRAINING_LANE1_SET_REPEATER1 0xf0012 > +#define DP_LTTPR_TRAINING_LANE2_SET_REPEATER1 0xf0013 > +#define DP_LTTPR_TRAINING_LANE3_SET_REPEATER1 0xf0014 Can we align the register naming with the spec and to align with the rest of this file? The seem to take the form of DP_ E.g. call this one DP_TRAINING_LANE3_SET_PHY_REPEATER1 Same for all other registers. Keeping the naming consistent makes it easier to search. > +#define DP_LTTPR_TRAINING_AUX_RD_INTERVAL_REPEATER1 0xf0020 > +#define DP_LTTPR_TRANSMITTER_CAPABILITY_REPEATER1 0xf0021 > +#define DP_LTTPR_LANE0_1_STATUS_REPEATER1 0xf0030 > +#define DP_LTTPR_LANE2_3_STATUS_REPEATER1 0xf0031 > +#define DP_LTTPR_LANE_ALIGN_STATUS_UPDATED_REPEATER1 0xf0032 > +#define DP_LTTPR_ADJUST_REQUEST_LANE0_1_REPEATER1 0xf0033 > +#define DP_LTTPR_ADJUST_REQUEST_LANE2_3_REPEATER1 0xf0034 > +#define DP_LTTPR_SYMBOL_ERROR_COUNT_LANE0_REPEATER1 0xf0035 > +#define DP_LTTPR_SYMBOL_ERROR_COUNT_LANE1_REPEATER1 0xf0037 > +#define DP_LTTPR_SYMBOL_ERROR_COUNT_LANE2_REPEATER1 0xf0039 > +#define DP_LTTPR_SYMBOL_ERROR_COUNT_LANE3_REPEATER1 0xf003b > +#define DP_REPEATER_CONFIGURATION_AND_STATUS_OFFSET 0x50 With a cursory search I can't seem to find this in the spec. Where is this coming from? Harry > +#define DP_LTTPR_FEC_STATUS_REPEATER1 0xf0290> + > /* Multiple stream transport */ > #define DP_FAUX_CAP 0x020 /* 1.2 */ > # define DP_FAUX_CAP_1 (1 << 0) > signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
On 23/08/2019 16:57, Rob Herring wrote: On Fri, Aug 23, 2019 at 10:44 AM Robin Murphy wrote: On 23/08/2019 16:05, Steven Price wrote: On 23/08/2019 12:11, Robin Murphy wrote: On 23/08/2019 03:12, Rob Herring wrote: There is no point in resuming the h/w just to do flush operations and doing so takes several locks which cause lockdep issues with the shrinker. Rework the flush operations to only happen when the h/w is already awake. This avoids taking any locks associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 842bdd7cf6be..ccf671a9c3fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size) return SZ_2M; } +void panfrost_mmu_flush_range(struct panfrost_device *pfdev, + struct panfrost_mmu *mmu, + u64 iova, size_t size) +{ +if (mmu->as < 0) +return; + +/* Flush the PTs only if we're already awake */ +if (!pm_runtime_get_if_in_use(pfdev->dev)) +return; Is the MMU guaranteed to be reset on resume such that the TLBs will always come up clean? Otherwise there are potentially corners where stale entries that we skip here might hang around if the hardware lies about powering things down. Assuming runtime PM has actually committed to the power off, then on power on panfrost_device_resume() will be called which performs a reset of the GPU which will clear the L2/TLBs so there shouldn't be any problem there. OK, if panfrost_gpu_soft_reset() is sufficient to guarantee clean TLBs then this looks equivalent to what we did for arm-smmu, so I've no complaints in that regard. However on second look I've now noticed the panfrost_mmu_flush_range() calls being moved outside of mmu->lock protection. Forgive me if there's basic DRM knowledge I'm missing here, but is there any possibility for multiple threads to create/import/free objects simultaneously on the same FD such that two mmu_hw_do_operation() calls could race and interfere with each other in terms of the AS_LOCKADDR/AS_COMMAND/AS_STATUS dance? Yes, we could have multiple threads. Not really any good reason it's moved out of the mmu->lock other than just to avoid any nesting (though that seemed fine in testing). The newly added as_lock will serialize mmu_hw_do_operation(). So the mmu->lock is just serializing page table writes. Urgh, sorry, once again I'd stopped looking at -next and was cross-referencing my current rc3-based working tree :( In that case, you may even be able to remove mmu->lock entirely, since io-pgtable-arm doesn't need external locking itself. And for this patch, Reviewed-by: Robin Murphy Cheers, Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel