Re: [patch 00/13] preempt: Make preempt count unconditional
On Mon, Sep 14, 2020 at 11:24 PM Herbert Xu wrote: > > On Tue, Sep 15, 2020 at 09:20:59AM +0300, Ard Biesheuvel wrote: > > > > The documentation of kmap_atomic() states the following: > > > > * The use of kmap_atomic/kunmap_atomic is discouraged - kmap/kunmap > > * gives a more generic (and caching) interface. But kmap_atomic can > > * be used in IRQ contexts, so in some (very limited) cases we need > > * it. > > > > so if this is no longer accurate, perhaps we should fix it? > > This hasn't been accurate for at least ten years :) Yeah, that used to be true a long long time ago, but the comment is very stale. > > But another reason I tried to avoid kmap_atomic() is that it disables > > preemption unconditionally, even on 64-bit architectures where HIGHMEM > > is irrelevant. So using kmap_atomic() here means that the bulk of > > WireGuard packet encryption runs with preemption disabled, essentially > > for legacy reasons. > > Agreed. We should definitely fix that. Well, honestly, one big reason for that is debugging. The *semantics* of the kmap_atomic() is in the name - you can't sleep in between it and the kunmap_atomic(). On any sane architecture, kmap_atomic() ends up being a no-op from an implementation standpoint, and sleeping would work just fine. But we very much want to make sure that people don't then write code that doesn't work on the bad old 32-bit machines where it really needs that sequence to be safe from preemption. So it's mostly a debug thing. I say "mostly", because there might be small other details too, like shared code, and perhaps even a couple of users out in the wild that depend on the pagefault_disable() inherent in the current kmap_atomic(), who knows.. So no, the preemption disabling isn't inherent in the operation itself. But it does have some argument for it. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 00/13] preempt: Make preempt count unconditional
On Tue, 15 Sep 2020 at 01:43, Linus Torvalds wrote: > > On Mon, Sep 14, 2020 at 3:24 PM Linus Torvalds > wrote: > > > > Ard and Herbert added to participants: see > > chacha20poly1305_crypt_sg_inplace(), which does > > > > flags = SG_MITER_TO_SG; > > if (!preemptible()) > > flags |= SG_MITER_ATOMIC; > > > > introduced in commit d95312a3ccc0 ("crypto: lib/chacha20poly1305 - > > reimplement crypt_from_sg() routine"). > > As far as I can tell, the only reason for this all is to try to use > "kmap()" rather than "kmap_atomic()". > > And kmap() actually has the much more complex "might_sleep()" tests, > and apparently the "preemptible()" check wasn't even the proper full > debug check, it was just a complete hack to catch the one that > triggered. > This was not driven by a failing check. The documentation of kmap_atomic() states the following: * The use of kmap_atomic/kunmap_atomic is discouraged - kmap/kunmap * gives a more generic (and caching) interface. But kmap_atomic can * be used in IRQ contexts, so in some (very limited) cases we need * it. so if this is no longer accurate, perhaps we should fix it? But another reason I tried to avoid kmap_atomic() is that it disables preemption unconditionally, even on 64-bit architectures where HIGHMEM is irrelevant. So using kmap_atomic() here means that the bulk of WireGuard packet encryption runs with preemption disabled, essentially for legacy reasons. > From a quick look, that code should probably just get rid of > SG_MITER_ATOMIC entirely, and alwayse use kmap_atomic(). > > kmap_atomic() is actually the faster and proper interface to use > anyway (never mind that any of this matters on any sane hardware). The > old kmap() and kunmap() interfaces should generally be avoided like > the plague - yes, they allow sleeping in the middle and that is > sometimes required, but if you don't need that, you should never ever > use them. > > We used to have a very nasty kmap_atomic() that required people to be > very careful and know exactly which atomic entry to use, and that was > admitedly quite nasty. > > So it _looks_ like this code started using kmap() - probably back when > kmap_atomic() was so cumbersome to use - and was then converted > (conditionally) to kmap_atomic() rather than just changed whole-sale. > Is there actually something that wants to use those sg_miter functions > and sleep? > > Because if there is, that choice should come from the outside, not > from inside lib/scatterlist.c trying to make some bad guess based on > the wrong thing entirely. > > Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
some half-baked ttm ideas
Hi Christian, I've been trying to move towards the idea of not having TTM manage the global TT, I'm still not sure what the result would look like so I've been randomly trying out a direction or two, There are some patches in : https://github.com/airlied/linux/commits/ttm-half-baked-ideas a) it splits use_tt into two flags, one for system memory backing and one for binding, and tries to use them correctly. b) adds cpu_pin/unpin hooks for the amdgpu/radeon drivers to use for userptr. I sort of envision being able to set the use_tt flag to false for drivers who don't want TTM to maintain their global TT, but I'm still not certain how that would look, I'd welcome any input there. Thanks, Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/ttm: move ttm binding/unbinding out of ttm_tt paths.
From: Dave Airlie Move these up to the bo level, moving ttm_tt to just being backing store. Next step is to move the bound flag out. Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/nouveau/nouveau_bo.c| 2 +- drivers/gpu/drm/radeon/radeon_mn.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c| 2 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 33 +-- drivers/gpu/drm/ttm/ttm_tt.c| 31 -- include/drm/ttm/ttm_bo_driver.h | 28 include/drm/ttm/ttm_tt.h| 35 - 9 files changed, 64 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index dc73cce515b2..76a796eab901 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -554,7 +554,7 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict, } /* Bind the memory to the GTT space */ - r = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem); + r = ttm_bo_tt_bind(bo, &tmp_mem); if (unlikely(r)) { goto out_cleanup; } diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 3a1032d01808..ec995215de97 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -908,7 +908,7 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr, if (ret) goto out; - ret = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg); + ret = ttm_bo_tt_bind(bo, &tmp_reg); if (ret) goto out; diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c index b6293fb91030..eb46d2220236 100644 --- a/drivers/gpu/drm/radeon/radeon_mn.c +++ b/drivers/gpu/drm/radeon/radeon_mn.c @@ -53,7 +53,7 @@ static bool radeon_mn_invalidate(struct mmu_interval_notifier *mn, struct ttm_operation_ctx ctx = { false, false }; long r; - if (!bo->tbo.ttm || !ttm_tt_is_bound(bo->tbo.ttm)) + if (!bo->tbo.ttm || !ttm_bo_tt_is_bound(&bo->tbo)) return true; if (!mmu_notifier_range_blockable(range)) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 2bc6991fb7e9..228175790457 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -242,7 +242,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, goto out_cleanup; } - r = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem); + r = ttm_bo_tt_bind(bo, &tmp_mem); if (unlikely(r)) { goto out_cleanup; } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 32eaf809b7c9..abc88f713fc6 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -264,7 +264,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (ret) goto out_err; - ret = ttm_tt_bind(bdev, bo->ttm, mem); + ret = ttm_bo_tt_bind(bo, mem); if (ret) goto out_err; } diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 2354046bda9a..4c5c9a333c74 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -67,7 +67,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo, return ret; } - ttm_tt_unbind(bo->bdev, ttm); + ttm_bo_tt_unbind(bo); ttm_bo_free_old_node(bo); old_mem->mem_type = TTM_PL_SYSTEM; } @@ -82,7 +82,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo, if (unlikely(ret != 0)) return ret; - ret = ttm_tt_bind(bo->bdev, ttm, new_mem); + ret = ttm_bo_tt_bind(bo, new_mem); if (unlikely(ret != 0)) return ret; } @@ -706,6 +706,35 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo) { if (bo->ttm == NULL) return; + + ttm_bo_tt_unbind(bo); ttm_tt_destroy(bo->bdev, bo->ttm); bo->ttm = NULL; } + +int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem) +{ + int ret; + + if (!bo->ttm) + return -EINVAL; + + if (ttm_bo_tt_is_bound(bo)) + return 0; + + ret = bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem); + if (unlikely(ret != 0)) + return ret; + + ttm_bo_tt_set_bound(bo); + return 0; +} +EXPORT_SYMBOL(ttm_bo_tt_bind); + +void ttm_bo_tt_unbind(struct ttm_bu
[PATCH 1/7] drm/ttm/tt: add wrappers to set tt state.
From: Dave Airlie This adds 2 getters and 4 setters, however unbound and populated are currently the same thing, this will change, it also drops a BUG_ON that seems not that useful. Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 4 +-- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 +-- drivers/gpu/drm/radeon/radeon_mn.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c| 4 +-- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- drivers/gpu/drm/ttm/ttm_page_alloc.c | 6 ++-- drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 6 ++-- drivers/gpu/drm/ttm/ttm_tt.c | 20 ++ drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 +- include/drm/ttm/ttm_tt.h | 32 +- 11 files changed, 57 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 6fc3af082f6f..568cb75900a3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1305,7 +1305,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_bo_device *bdev, return -ENOMEM; ttm->page_flags |= TTM_PAGE_FLAG_SG; - ttm->state = tt_unbound; + ttm_tt_set_populated(ttm); return 0; } @@ -1325,7 +1325,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_bo_device *bdev, drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, gtt->ttm.dma_address, ttm->num_pages); - ttm->state = tt_unbound; + ttm_tt_set_populated(ttm); return 0; } diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 97e1908eada0..3ab9397fac40 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1282,14 +1282,14 @@ nouveau_ttm_tt_populate(struct ttm_bo_device *bdev, struct device *dev; bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG); - if (ttm->state != tt_unpopulated) + if (ttm_tt_is_populated(ttm)) return 0; if (slave && ttm->sg) { /* make userspace faulting work */ drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, ttm_dma->dma_address, ttm->num_pages); - ttm->state = tt_unbound; + ttm_tt_set_populated(ttm); return 0; } diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c index f93829f08a4d..b6293fb91030 100644 --- a/drivers/gpu/drm/radeon/radeon_mn.c +++ b/drivers/gpu/drm/radeon/radeon_mn.c @@ -53,7 +53,7 @@ static bool radeon_mn_invalidate(struct mmu_interval_notifier *mn, struct ttm_operation_ctx ctx = { false, false }; long r; - if (!bo->tbo.ttm || bo->tbo.ttm->state != tt_bound) + if (!bo->tbo.ttm || !ttm_tt_is_bound(bo->tbo.ttm)) return true; if (!mmu_notifier_range_blockable(range)) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 602a591a53dc..7aa3d03ddeb9 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -615,14 +615,14 @@ static int radeon_ttm_tt_populate(struct ttm_bo_device *bdev, return -ENOMEM; ttm->page_flags |= TTM_PAGE_FLAG_SG; - ttm->state = tt_unbound; + ttm_tt_set_populated(ttm); return 0; } if (slave && ttm->sg) { drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, gtt->ttm.dma_address, ttm->num_pages); - ttm->state = tt_unbound; + ttm_tt_set_populated(ttm); return 0; } diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 1b56432dfa43..44b47ccdeaf7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -249,7 +249,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, * Don't move nonexistent data. Clear destination instead. */ if (old_iomap == NULL && - (ttm == NULL || (ttm->state == tt_unpopulated && + (ttm == NULL || (!ttm_tt_is_populated(ttm) && !(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED { memset_io(new_iomap, 0, new_mem->num_pages*PAGE_SIZE); goto out2; diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c index b40a4678c296..14660f723f71 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c @@ -1044,7 +1044,7 @@
[no subject]
The goal here is to make the ttm_tt object just represent a memory backing store, and now whether the store is bound to a global translation table. It moves binding up to the bo level. There's a lot more work on removing the global TT from the core of TTM, but this seems like a good start. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] drm/ttm: split populate out from binding.
From: Dave Airlie Drivers have to call populate themselves now before binding. Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 ++- drivers/gpu/drm/nouveau/nouveau_bo.c| 6 +- drivers/gpu/drm/radeon/radeon_ttm.c | 7 ++- drivers/gpu/drm/ttm/ttm_bo.c| 6 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 7 ++- drivers/gpu/drm/ttm/ttm_tt.c| 11 +-- include/drm/ttm/ttm_tt.h| 3 +-- 7 files changed, 34 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 568cb75900a3..dc73cce515b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -548,8 +548,13 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object *bo, bool evict, goto out_cleanup; } + r = ttm_tt_populate(bo->bdev, bo->ttm, ctx); + if (unlikely(r)) { + goto out_cleanup; + } + /* Bind the memory to the GTT space */ - r = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem, ctx); + r = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem); if (unlikely(r)) { goto out_cleanup; } diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 3ab9397fac40..3a1032d01808 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -904,7 +904,11 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr, if (ret) return ret; - ret = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg, &ctx); + ret = ttm_tt_populate(bo->bdev, bo->ttm, &ctx); + if (ret) + goto out; + + ret = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_reg); if (ret) goto out; diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 7aa3d03ddeb9..2bc6991fb7e9 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -237,7 +237,12 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, goto out_cleanup; } - r = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem, &ctx); + r = ttm_tt_populate(bo->bdev, bo->ttm, &ctx); + if (unlikely(r)) { + goto out_cleanup; + } + + r = ttm_tt_bind(bo->bdev, bo->ttm, &tmp_mem); if (unlikely(r)) { goto out_cleanup; } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 9aae9e1bd8e8..32eaf809b7c9 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -260,7 +260,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, goto out_err; if (mem->mem_type != TTM_PL_SYSTEM) { - ret = ttm_tt_bind(bdev, bo->ttm, mem, ctx); + ret = ttm_tt_populate(bdev, bo->ttm, ctx); + if (ret) + goto out_err; + + ret = ttm_tt_bind(bdev, bo->ttm, mem); if (ret) goto out_err; } diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index f3452a1624fd..2354046bda9a 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -77,7 +77,12 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo, return ret; if (new_mem->mem_type != TTM_PL_SYSTEM) { - ret = ttm_tt_bind(bo->bdev, ttm, new_mem, ctx); + + ret = ttm_tt_populate(bo->bdev, ttm, ctx); + if (unlikely(ret != 0)) + return ret; + + ret = ttm_tt_bind(bo->bdev, ttm, new_mem); if (unlikely(ret != 0)) return ret; } diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 381face3cedb..93d65e5e4205 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -312,8 +312,7 @@ void ttm_tt_unbind(struct ttm_bo_device *bdev, struct ttm_tt *ttm) } int ttm_tt_bind(struct ttm_bo_device *bdev, - struct ttm_tt *ttm, struct ttm_resource *bo_mem, - struct ttm_operation_ctx *ctx) + struct ttm_tt *ttm, struct ttm_resource *bo_mem) { int ret = 0; @@ -323,10 +322,6 @@ int ttm_tt_bind(struct ttm_bo_device *bdev, if (ttm_tt_is_bound(ttm)) return 0; - ret = ttm_tt_populate(bdev, ttm, ctx); - if (ret) - return ret; - ret = bdev->driver->ttm_tt_bind(bdev, ttm, bo_mem); if (unlikely(ret != 0)) return ret; @@ -455,6 +450,9 @@ int ttm_tt_populate(struct ttm_bo_device *bdev, { int ret; + if (!ttm) + return -EINVAL; +
[PATCH 2/7] drm/ttm: wrap tt destroy.
From: Dave Airlie All places this was called was using bo->ttm either direct or indirectly. Signed-off-by: Dave Airlie --- drivers/gpu/drm/ttm/ttm_bo.c | 9 +++-- drivers/gpu/drm/ttm/ttm_bo_util.c | 24 include/drm/ttm/ttm_bo_driver.h | 5 + 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index e2bfe3a13c63..9aae9e1bd8e8 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -301,10 +301,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, out_err: new_man = ttm_manager_type(bdev, bo->mem.mem_type); - if (!new_man->use_tt) { - ttm_tt_destroy(bdev, bo->ttm); - bo->ttm = NULL; - } + if (!new_man->use_tt) + ttm_bo_tt_destroy(bo); return ret; } @@ -322,8 +320,7 @@ static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo) if (bo->bdev->driver->move_notify) bo->bdev->driver->move_notify(bo, false, NULL); - ttm_tt_destroy(bo->bdev, bo->ttm); - bo->ttm = NULL; + ttm_bo_tt_destroy(bo); ttm_resource_free(bo, &bo->mem); } diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 44b47ccdeaf7..0ddaaa1ddafd 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -297,10 +297,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, *old_mem = *new_mem; new_mem->mm_node = NULL; - if (!man->use_tt) { - ttm_tt_destroy(bdev, ttm); - bo->ttm = NULL; - } + if (!man->use_tt) + ttm_bo_tt_destroy(bo); out1: ttm_resource_iounmap(bdev, old_mem, new_iomap); @@ -542,10 +540,8 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, if (ret) return ret; - if (!man->use_tt) { - ttm_tt_destroy(bdev, bo->ttm); - bo->ttm = NULL; - } + if (!man->use_tt) + ttm_bo_tt_destroy(bo); ttm_bo_free_old_node(bo); } else { /** @@ -665,10 +661,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo, if (ret) return ret; - if (!to->use_tt) { - ttm_tt_destroy(bdev, bo->ttm); - bo->ttm = NULL; - } + if (!to->use_tt) + ttm_bo_tt_destroy(bo); ttm_bo_free_old_node(bo); } @@ -702,3 +696,9 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) return 0; } + +void ttm_bo_tt_destroy(struct ttm_buffer_object *bo) +{ + ttm_tt_destroy(bo->bdev, bo->ttm); + bo->ttm = NULL; +} diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 303a89d1066d..c2e93f04d0ad 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -684,6 +684,11 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo); */ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp); +/** + * ttm_bo_tt_destroy. + */ +void ttm_bo_tt_destroy(struct ttm_buffer_object *bo); + /** * ttm_range_man_init * -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] drm/ttm: tt destroy move null check to outer function.
From: Dave Airlie This just makes things easier later. Signed-off-by: Dave Airlie --- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 ++ drivers/gpu/drm/ttm/ttm_tt.c | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 0ddaaa1ddafd..f3452a1624fd 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -699,6 +699,8 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) void ttm_bo_tt_destroy(struct ttm_buffer_object *bo) { + if (bo->ttm == NULL) + return; ttm_tt_destroy(bo->bdev, bo->ttm); bo->ttm = NULL; } diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index ff3d953aa90e..381face3cedb 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -209,9 +209,6 @@ EXPORT_SYMBOL(ttm_tt_set_placement_caching); void ttm_tt_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm) { - if (ttm == NULL) - return; - ttm_tt_unbind(bdev, ttm); ttm_tt_unpopulate(bdev, ttm); -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/ttm: move populated state into page flags
From: Dave Airlie Just use the top bit of page flags to store the populated state. Signed-off-by: Dave Airlie --- include/drm/ttm/ttm_tt.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h index 94e16238c93d..c777b72063db 100644 --- a/include/drm/ttm/ttm_tt.h +++ b/include/drm/ttm/ttm_tt.h @@ -42,6 +42,8 @@ struct ttm_operation_ctx; #define TTM_PAGE_FLAG_SG (1 << 8) #define TTM_PAGE_FLAG_NO_RETRY (1 << 9) +#define TTM_PAGE_FLAG_PRIV_POPULATED (1 << 31) + enum ttm_caching_state { tt_uncached, tt_wc, @@ -70,22 +72,21 @@ struct ttm_tt { struct sg_table *sg; /* for SG objects via dma-buf */ struct file *swap_storage; enum ttm_caching_state caching_state; - bool populated; }; static inline bool ttm_tt_is_populated(struct ttm_tt *tt) { - return tt->populated; + return tt->page_flags & TTM_PAGE_FLAG_PRIV_POPULATED; } static inline void ttm_tt_set_unpopulated(struct ttm_tt *tt) { - tt->populated = false; + tt->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED; } static inline void ttm_tt_set_populated(struct ttm_tt *tt) { - tt->populated = true; + tt->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED; } /** -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] drm/ttm: split bound/populated flags.
From: Dave Airlie Move bound up into the bo object, and keep populated with the tt object. The ghost object handling needs to follow the flags at the bo level now instead of it being part of the ttm tt object. Signed-off-by: Dave Airlie --- drivers/gpu/drm/ttm/ttm_bo_util.c | 15 +++ include/drm/ttm/ttm_bo_api.h | 1 + include/drm/ttm/ttm_bo_driver.h | 6 +++--- include/drm/ttm/ttm_tt.h | 12 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 4c5c9a333c74..6b0320252a60 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -572,10 +572,13 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo, * bo to be unbound and destroyed. */ - if (man->use_tt) + if (man->use_tt) { ghost_obj->ttm = NULL; - else + ttm_bo_tt_set_unbound(ghost_obj); + } else { bo->ttm = NULL; + ttm_bo_tt_set_unbound(bo); + } dma_resv_unlock(&ghost_obj->base._resv); ttm_bo_put(ghost_obj); @@ -628,10 +631,13 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo, * bo to be unbound and destroyed. */ - if (to->use_tt) + if (to->use_tt) { ghost_obj->ttm = NULL; - else + ttm_bo_tt_set_unbound(ghost_obj); + } else { bo->ttm = NULL; + ttm_bo_tt_set_unbound(bo); + } dma_resv_unlock(&ghost_obj->base._resv); ttm_bo_put(ghost_obj); @@ -695,6 +701,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) memset(&bo->mem, 0, sizeof(bo->mem)); bo->mem.mem_type = TTM_PL_SYSTEM; bo->ttm = NULL; + ttm_bo_tt_set_unbound(bo); dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 36ff64e2736c..1d20a7f15a7a 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -141,6 +141,7 @@ struct ttm_buffer_object { struct ttm_resource mem; struct file *persistent_swap_storage; struct ttm_tt *ttm; + bool ttm_bound; bool evicted; bool deleted; diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 0112619f6172..0677e2582ff8 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -700,17 +700,17 @@ void ttm_bo_tt_unbind(struct ttm_buffer_object *bo); static inline bool ttm_bo_tt_is_bound(struct ttm_buffer_object *bo) { - return bo->ttm->_state == tt_bound; + return bo->ttm_bound; } static inline void ttm_bo_tt_set_unbound(struct ttm_buffer_object *bo) { - bo->ttm->_state = tt_unbound; + bo->ttm_bound = false; } static inline void ttm_bo_tt_set_bound(struct ttm_buffer_object *bo) { - bo->ttm->_state = tt_bound; + bo->ttm_bound = true; } /** * ttm_bo_tt_destroy. diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h index 1ac56730d952..94e16238c93d 100644 --- a/include/drm/ttm/ttm_tt.h +++ b/include/drm/ttm/ttm_tt.h @@ -70,26 +70,22 @@ struct ttm_tt { struct sg_table *sg; /* for SG objects via dma-buf */ struct file *swap_storage; enum ttm_caching_state caching_state; - enum { - tt_bound, - tt_unbound, - tt_unpopulated, - } _state; + bool populated; }; static inline bool ttm_tt_is_populated(struct ttm_tt *tt) { - return tt->_state != tt_unpopulated; + return tt->populated; } static inline void ttm_tt_set_unpopulated(struct ttm_tt *tt) { - tt->_state = tt_unpopulated; + tt->populated = false; } static inline void ttm_tt_set_populated(struct ttm_tt *tt) { - tt->_state = tt_unbound; + tt->populated = true; } /** -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/panel: samsung: make vint_table static
Hi Jason, Thank you for the patch. On Sat, Sep 12, 2020 at 11:38:17AM +0800, Jason Yan wrote: > This eliminates the following sparse warning: > > drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c:217:15: warning: symbol > 'vint_table' was not declared. Should it be static? > > Reported-by: Hulk Robot > Signed-off-by: Jason Yan > --- > drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c > b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c > index 1d1c79a18613..b3f5797c23e0 100644 > --- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c > +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c > @@ -214,7 +214,7 @@ static const u8 > gamma_tbl[S6E3HA2_NUM_GAMMA_STEPS][S6E3HA2_GAMMA_CMD_CNT] = { > 0x00, 0x00 } > }; > > -unsigned char vint_table[S6E3HA2_VINT_STATUS_MAX] = { > +static unsigned char vint_table[S6E3HA2_VINT_STATUS_MAX] = { Shouldn't it be const, while at it ? > 0x18, 0x19, 0x1a, 0x1b, 0x1c, > 0x1d, 0x1e, 0x1f, 0x20, 0x21 > }; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data
Hi Jyri, Thank you for the patch. On Mon, Sep 14, 2020 at 11:34:50AM +0300, Jyri Sarha wrote: > We already have a private data member for maximum display width so > let's use it and get rid of the redundant tilcdc_crtc_max_width(). You may want to add a sentence to explain why OF parsing is moved. > Signed-off-by: Jyri Sarha > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +--- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++--- > 3 files changed, 26 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 29f263e1975a..27ea529d74b8 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -754,20 +754,6 @@ static const struct drm_crtc_funcs tilcdc_crtc_funcs = { > .disable_vblank = tilcdc_crtc_disable_vblank, > }; > > -int tilcdc_crtc_max_width(struct drm_crtc *crtc) > -{ > - struct drm_device *dev = crtc->dev; > - struct tilcdc_drm_private *priv = dev->dev_private; > - int max_width = 0; > - > - if (priv->rev == 1) > - max_width = 1024; > - else if (priv->rev == 2) > - max_width = 2048; > - > - return max_width; > -} > - > static enum drm_mode_status > tilcdc_crtc_mode_valid(struct drm_crtc *crtc, > const struct drm_display_mode *mode) > @@ -780,7 +766,7 @@ tilcdc_crtc_mode_valid(struct drm_crtc *crtc, >* check to see if the width is within the range that >* the LCD Controller physically supports >*/ > - if (mode->hdisplay > tilcdc_crtc_max_width(crtc)) > + if (mode->hdisplay > priv->max_width) > return MODE_VIRTUAL_X; > > /* width must be multiple of 16 */ > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > index 4f5fc3e87383..866b33b40eff 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > @@ -105,7 +105,7 @@ static void modeset_init(struct drm_device *dev) > > dev->mode_config.min_width = 0; > dev->mode_config.min_height = 0; > - dev->mode_config.max_width = tilcdc_crtc_max_width(priv->crtc); > + dev->mode_config.max_width = priv->max_width; > dev->mode_config.max_height = 2048; > dev->mode_config.funcs = &mode_config_funcs; > } > @@ -218,22 +218,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) > goto init_failed; > } > > - if (of_property_read_u32(node, "max-bandwidth", &priv->max_bandwidth)) > - priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; > - > - DBG("Maximum Bandwidth Value %d", priv->max_bandwidth); > - > - if (of_property_read_u32(node, "max-width", &priv->max_width)) > - priv->max_width = TILCDC_DEFAULT_MAX_WIDTH; > - > - DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width); > - > - if (of_property_read_u32(node, "max-pixelclock", > - &priv->max_pixelclock)) > - priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK; > - > - DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); > - > pm_runtime_enable(dev); > > /* Determine LCD IP Version */ > @@ -287,6 +271,26 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) > } > } > > + if (of_property_read_u32(node, "max-bandwidth", &priv->max_bandwidth)) > + priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; > + > + DBG("Maximum Bandwidth Value %d", priv->max_bandwidth); > + > + if (of_property_read_u32(node, "max-width", &priv->max_width)) { > + if (priv->rev == 1) > + priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V1; > + else > + priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V2; > + } > + > + DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width); > + > + if (of_property_read_u32(node, "max-pixelclock", > + &priv->max_pixelclock)) > + priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK; > + > + DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); > + > ret = tilcdc_crtc_create(ddev); > if (ret < 0) { > dev_err(dev, "failed to create crtc\n"); > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h > b/drivers/gpu/drm/tilcdc/tilcdc_drv.h > index 18815e75ca4f..76adf87fec4e 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h > @@ -28,8 +28,10 @@ struct drm_plane; > > /* Defaulting to pixel clock defined on AM335x */ > #define TILCDC_DEFAULT_MAX_PIXELCLOCK 126000 > -/* Defaulting to max width as defined on AM335x */ > -#define TILCDC_DEFAULT_MAX_WIDTH 2048 > +/* Maximum width as defined on AM335x */ > +#define TILCDC_DEFAULT_MAX_
Re: [PATCH 1/2] drm/tilcdc: Do not keep vblank interrupts enabled all the time
Hi Jyri, Thank you for the patch. On Mon, Sep 14, 2020 at 11:34:49AM +0300, Jyri Sarha wrote: > END_OF_FRAME interrupts have been enabled all the time since the > beginning of this driver. It is about time to add this feature. > > Signed-off-by: Jyri Sarha Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 36 +--- > 1 file changed, 33 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 1856962411c7..29f263e1975a 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -147,12 +147,9 @@ static void tilcdc_crtc_enable_irqs(struct drm_device > *dev) > tilcdc_set(dev, LCDC_RASTER_CTRL_REG, > LCDC_V1_SYNC_LOST_INT_ENA | LCDC_V1_FRAME_DONE_INT_ENA | > LCDC_V1_UNDERFLOW_INT_ENA); > - tilcdc_set(dev, LCDC_DMA_CTRL_REG, > - LCDC_V1_END_OF_FRAME_INT_ENA); > } else { > tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, > LCDC_V2_UNDERFLOW_INT_ENA | > - LCDC_V2_END_OF_FRAME0_INT_ENA | > LCDC_FRAME_DONE | LCDC_SYNC_LOST); > } > } > @@ -678,11 +675,44 @@ static int tilcdc_crtc_atomic_check(struct drm_crtc > *crtc, > > static int tilcdc_crtc_enable_vblank(struct drm_crtc *crtc) > { > + struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); > + struct drm_device *dev = crtc->dev; > + struct tilcdc_drm_private *priv = dev->dev_private; > + unsigned long flags; > + > + spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); > + > + tilcdc_clear_irqstatus(dev, LCDC_END_OF_FRAME0); > + > + if (priv->rev == 1) > + tilcdc_set(dev, LCDC_DMA_CTRL_REG, > +LCDC_V1_END_OF_FRAME_INT_ENA); > + else > + tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, > +LCDC_V2_END_OF_FRAME0_INT_ENA); > + > + spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); > + > return 0; > } > > static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc) > { > + struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); > + struct drm_device *dev = crtc->dev; > + struct tilcdc_drm_private *priv = dev->dev_private; > + unsigned long flags; > + > + spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); > + > + if (priv->rev == 1) > + tilcdc_clear(dev, LCDC_DMA_CTRL_REG, > + LCDC_V1_END_OF_FRAME_INT_ENA); > + else > + tilcdc_clear(dev, LCDC_INT_ENABLE_SET_REG, > + LCDC_V2_END_OF_FRAME0_INT_ENA); > + > + spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); > } > > static void tilcdc_crtc_reset(struct drm_crtc *crtc) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/23] virtio: Add get_shm_region method
On Thu, Sep 10, 2020 at 2:56 AM Miklos Szeredi wrote: > On Thu, Sep 10, 2020 at 2:28 AM Gurchetan Singh > wrote: > > > That sounds like an excellent plan ! > > > > I will send out blob v3 (incorporating kraxel@'s feedback) once the > topic pull request (it seems Miklos will do this?) for the shm region > patches has been merged into drm-misc-next. > > I split out the three patches into: > > git:// > git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtio-shm Thanks, pull request sent. > > > Thanks, > Miklos > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: add missing put_device() call in mtk_hdmi_dt_parse_pdata()
Hi, Yu Kuai: Yu Kuai 於 2020年9月11日 週五 下午7:22寫道: > > if of_find_device_by_node() succeed, mtk_drm_kms_init() doesn't have > a corresponding put_device(). Thus add jump target to fix the exception > handling for this function implementation. Reviewed-by: Chun-Kuang Hu > > Fixes: 8f83f26891e1 ("drm/mediatek: Add HDMI support") > Signed-off-by: Yu Kuai > --- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++ > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c > b/drivers/gpu/drm/mediatek/mtk_hdmi.c > index f2e9b429960b..a97725680d4e 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c > @@ -1507,25 +1507,30 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi > *hdmi, > dev_err(dev, > "Failed to get system configuration registers: %d\n", > ret); > - return ret; > + goto put_device; > } > hdmi->sys_regmap = regmap; > > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > hdmi->regs = devm_ioremap_resource(dev, mem); > - if (IS_ERR(hdmi->regs)) > - return PTR_ERR(hdmi->regs); > + if (IS_ERR(hdmi->regs)) { > + ret = PTR_ERR(hdmi->regs); > + goto put_device; > + } > > remote = of_graph_get_remote_node(np, 1, 0); > - if (!remote) > - return -EINVAL; > + if (!remote) { > + ret = -EINVAL; > + goto put_device; > + } > > if (!of_device_is_compatible(remote, "hdmi-connector")) { > hdmi->next_bridge = of_drm_find_bridge(remote); > if (!hdmi->next_bridge) { > dev_err(dev, "Waiting for external bridge\n"); > of_node_put(remote); > - return -EPROBE_DEFER; > + ret = -EPROBE_DEFER; > + goto put_device; > } > } > > @@ -1534,7 +1539,8 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi > *hdmi, > dev_err(dev, "Failed to find ddc-i2c-bus node in %pOF\n", > remote); > of_node_put(remote); > - return -EINVAL; > + ret = -EINVAL; > + goto put_device; > } > of_node_put(remote); > > @@ -1542,10 +1548,14 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi > *hdmi, > of_node_put(i2c_np); > if (!hdmi->ddc_adpt) { > dev_err(dev, "Failed to get ddc i2c adapter by node\n"); > - return -EINVAL; > + ret = -EINVAL; > + goto put_device; > } > > return 0; > +put_device: > + put_device(hdmi->cec_dev); > + return ret; > } > > /* > -- > 2.25.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: add missing put_device() call in mtk_drm_kms_init()
Hi, Yu Kuai: Yu Kuai 於 2020年9月11日 週五 下午7:22寫道: > > if of_find_device_by_node() succeed, mtk_drm_kms_init() doesn't have > a corresponding put_device(). Thus add jump target to fix the exception > handling for this function implementation. Reviewed-by: Chun-Kuang Hu > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") > Signed-off-by: Yu Kuai > --- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index 040a8f393fe2..7aceace94ebf 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -165,7 +165,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) > > ret = drmm_mode_config_init(drm); > if (ret) > - return ret; > + goto put_mutex_dev; > > drm->mode_config.min_width = 64; > drm->mode_config.min_height = 64; > @@ -182,7 +182,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) > > ret = component_bind_all(drm->dev, drm); > if (ret) > - return ret; > + goto put_mutex_dev; > > /* > * We currently support two fixed data streams, each optional, > @@ -229,7 +229,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) > } > if (!dma_dev->dma_parms) { > ret = -ENOMEM; > - goto err_component_unbind; > + goto put_dma_dev; > } > > ret = dma_set_max_seg_size(dma_dev, (unsigned int)DMA_BIT_MASK(32)); > @@ -256,9 +256,12 @@ static int mtk_drm_kms_init(struct drm_device *drm) > err_unset_dma_parms: > if (private->dma_parms_allocated) > dma_dev->dma_parms = NULL; > +put_dma_dev: > + put_device(private->dma_dev); > err_component_unbind: > component_unbind_all(drm->dev, drm); > - > +put_mutex_dev: > + put_device(private->mutex_dev); > return ret; > } > > -- > 2.25.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] virtio-shm region
Hi, This set of changes are required for zero-copy virtio-gpu. The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5: Linux 5.9-rc1 (2020-08-16 13:04:57 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git virtio-shm for you to fetch changes up to 38e895487afc2ed42c11045853cbb3fa20b52b6e: virtio: Implement get_shm_region for MMIO transport (2020-09-10 10:05:58 +0200) Sebastien Boeuf (3): virtio: Add get_shm_region method virtio: Implement get_shm_region for PCI transport virtio: Implement get_shm_region for MMIO transport drivers/virtio/virtio_mmio.c | 31 + drivers/virtio/virtio_pci_modern.c | 95 ++ include/linux/virtio_config.h | 17 +++ include/uapi/linux/virtio_mmio.h | 11 + include/uapi/linux/virtio_pci.h| 11 - 5 files changed, 164 insertions(+), 1 deletion(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: add exception handing in mtk_drm_probe() if component init fail
Hi, Yu Kuai: Yu Kuai 於 2020年9月9日 週三 下午4:50寫道: > > mtk_ddp_comp_init() is called in a loop in mtk_drm_probe(), if it > fail, previous successive init component is not proccessed. > > Thus uninitialize valid component and put their device if component > init failed. Reviewed-by: Chun-Kuang Hu > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") > Signed-off-by: Yu Kuai > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index 040a8f393fe2..75a6cf231fd7 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -544,8 +544,13 @@ static int mtk_drm_probe(struct platform_device *pdev) > pm_runtime_disable(dev); > err_node: > of_node_put(private->mutex_node); > - for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) > + for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) { > of_node_put(private->comp_node[i]); > + if (private->ddp_comp[i]) { > + put_device(private->ddp_comp[i]->larb_dev); > + private->ddp_comp[i] = NULL; > + } > + } > return ret; > } > > -- > 2.25.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: add missing put_device() call in mtk_ddp_comp_init()
Hi, Yu Kuai: Yu Kuai 於 2020年9月5日 週六 下午4:31寫道: > > if of_find_device_by_node() succeed, mtk_ddp_comp_init() doesn't have > a corresponding put_device(). Thus add put_device() to fix the exception > handling for this function implementation. > Reviewed-by: Chun-Kuang Hu > Fixes: d0afe37f5209 ("drm/mediatek: support CMDQ interface in ddp component") > Signed-off-by: Yu Kuai > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index 57c88de9a329..526648885b97 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -496,6 +496,7 @@ int mtk_ddp_comp_init(struct device *dev, struct > device_node *node, > #if IS_REACHABLE(CONFIG_MTK_CMDQ) > if (of_address_to_resource(node, 0, &res) != 0) { > dev_err(dev, "Missing reg in %s node\n", node->full_name); > + put_device(&larb_pdev->dev); > return -EINVAL; > } > comp->regs_pa = res.start; > -- > 2.25.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] mediatek drm next for 5.10
Hi, Dave & Daniel: This includes: 1. Move Mediatek HDMI PHY driver from DRM folder to PHY folder 2. Convert mtk-dpi to drm_bridge API 3. Disable tmds on mt2701 Regards, Chun-Kuang. The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5: Linux 5.9-rc1 (2020-08-16 13:04:57 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git tags/mediatek-drm-next-5.10 for you to fetch changes up to 09e872d558ba6a7f4468c4e8cdf0cd5a99bfc175: drm/mediatek: Disable tmds on mt2701 (2020-09-14 23:05:23 +0800) Mediatek DRM Next for Linux 5.10 1. Move Mediatek HDMI PHY driver from DRM folder to PHY folder 2. Convert mtk-dpi to drm_bridge API 3. Disable tmds on mt2701 CK Hu (3): drm/mediatek: Move tz_disabled from mtk_hdmi_phy to mtk_hdmi driver drm/mediatek: Separate mtk_hdmi_phy to an independent module phy: mediatek: Move mtk_hdmi_phy driver into drivers/phy/mediatek folder Chun-Kuang Hu (1): MAINTAINERS: add files for Mediatek DRM drivers Enric Balletbo i Serra (2): drm/mediatek: mtk_dpi: Rename bridge to next_bridge drm/mediatek: mtk_dpi: Convert to bridge driver Frank Wunderlich (2): dt-bindings: mediatek: add mt7623 display-nodes drm/mediatek: Add ddp routing for mt7623 Stu Hsieh (1): drm/mediatek: dpi/dsi: Change the getting possible_crtc way chunhui dai (1): drm/mediatek: Disable tmds on mt2701 Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt | 2 +- Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt | 2 +- Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt | 4 ++-- Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.txt | 4 MAINTAINERS | 1 + drivers/gpu/drm/mediatek/Kconfig | 2 +- drivers/gpu/drm/mediatek/Makefile | 5 + drivers/gpu/drm/mediatek/mtk_dpi.c | 80 ++-- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 38 ++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 2 ++ drivers/gpu/drm/mediatek/mtk_drm_drv.c | 23 +++ drivers/gpu/drm/mediatek/mtk_dsi.c | 6 +- drivers/gpu/drm/mediatek/mtk_hdmi.c | 21 + drivers/gpu/drm/mediatek/mtk_hdmi.h | 1 - drivers/phy/mediatek/Kconfig | 7 +++ drivers/phy/mediatek/Makefile | 5 + drivers/{gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c => phy/mediatek/phy-mtk-hdmi-mt2701.c} | 4 ++-- drivers/{gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c => phy/mediatek/phy-mtk-hdmi-mt8173.c} | 2 +- drivers/{gpu/drm/mediatek/mtk_hdmi_phy.c => phy/mediatek/phy-mtk-hdmi.c} | 6 +- drivers/{gpu/drm/mediatek/mtk_hdmi_phy.h => phy/mediatek/phy-mtk-hdmi.h} | 3 +-- 20 files changed, 159 insertions(+), 59 deletions(-) rename drivers/{gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c => phy/mediatek/phy-mtk-hdmi-mt2701.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c => phy/mediatek/phy-mtk-hdmi-mt8173.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_hdmi_phy.c => phy/mediatek/phy-mtk-hdmi.c} (96%) rename drivers/{gpu/drm/mediatek/mtk_hdmi_phy.h => phy/mediatek/phy-mtk-hdmi.h} (95%) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add Yes Optoelectronics
On Fri, 04 Sep 2020 23:38:19 +0530, Jagan Teki wrote: > Add vendor dt-bindings for Yes Optoelectronics Co.,Ltd. > > Signed-off-by: Jagan Teki > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] dt-bindings: display: simple: Add YTC700TLAG-05-201C
On Fri, 04 Sep 2020 23:38:20 +0530, Jagan Teki wrote: > Add dt-bindings for YTC700TLAG-05-201C 7" TFT LCD panel from > Yes Optoelectronics Co.,Ltd. > > Signed-off-by: Jagan Teki > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/13] dt-bindings: mfd: rohm,bd71847-pmic: Add common clock-names
On Fri, Sep 04, 2020 at 04:53:06PM +0200, Krzysztof Kozlowski wrote: > Add common 'clock-names' property which might appear in DTSes. This > makes it consistent with rohm,bd71837-pmic dtschema. > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml > b/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml > index 5d531051a153..6328163fc32a 100644 > --- a/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml > @@ -35,6 +35,9 @@ properties: >clocks: > maxItems: 1 > > + clock-names: > +maxItems: 1 Need to define the name. > + >"#clock-cells": > const: 0 > > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/13] dt-bindings: mfd: rohm,bd71837-pmic: Add common properties
On Fri, Sep 04, 2020 at 04:53:05PM +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (clock-names, > clock-output-names) to fix dtbs_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: > pmic@4b: 'clock-names', 'clock-output-names', do not match any of the > regexes: 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > --- > .../devicetree/bindings/mfd/rohm,bd71837-pmic.yaml | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.yaml > b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.yaml > index 65018a019e1d..ecce0d5e3a95 100644 > --- a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.yaml > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.yaml > @@ -32,9 +32,15 @@ properties: >clocks: > maxItems: 1 > > + clock-names: > +maxItems: 1 Needs to define what the name is. > + >"#clock-cells": > const: 0 > > + clock-output-names: > +maxItems: 1 Ideally this one too, but we've been more flexible on it. > + > # The BD718x7 supports two different HW states as reset target states. States > # are called as SNVS and READY. At READY state all the PMIC power outputs go > # down and OTP is reload. At the SNVS state all other logic and external > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/msm: a6xx: Use WHERE_AM_I for eligible targets
Support the WHERE_AM_I opcode for the A618, A630 and A640 GPUs if the microcode supports it. The WHERE_AM_I opcode allows the RPTR shadow to be updated in priviliged memory which protects the shadow from being read or written from user submissions. A650 already supports extended APRIV have built in hardware support for to access privilged memory from the CP and can go back to using the hardware RPTR shadow feature. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 87 ++- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 9 +++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index a3a8d6fd06bb..9cce2b01b1a7 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -51,9 +51,20 @@ bool a6xx_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring) static void a6xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring) { + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); uint32_t wptr; unsigned long flags; + /* Expanded APRIV doesn't need to issue the WHERE_AM_I opcode */ + if (a6xx_gpu->has_whereami && !adreno_gpu->base.hw_apriv) { + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); + + OUT_PKT7(ring, CP_WHERE_AM_I, 2); + OUT_RING(ring, lower_32_bits(shadowptr(a6xx_gpu, ring))); + OUT_RING(ring, upper_32_bits(shadowptr(a6xx_gpu, ring))); + } + spin_lock_irqsave(&ring->lock, flags); /* Copy the shadow to the actual register */ @@ -508,6 +519,30 @@ static int a6xx_cp_init(struct msm_gpu *gpu) return a6xx_idle(gpu, ring) ? 0 : -EINVAL; } +static void a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu, + struct drm_gem_object *obj) +{ + u32 *buf = msm_gem_get_vaddr_active(obj); + + if (IS_ERR(buf)) + return; + + /* +* If the lowest nibble is 0xa that is an indication that this microcode +* has been patched. The actual version is in dword [3] but we only care +* about the patchlevel which is the lowest nibble of dword [3] +* +* Otherwise check that the firmware is greater than or equal to 1.90 +* which was the first version that had this fix built in +*/ + if (((buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1) + a6xx_gpu->has_whereami = true; + else if ((buf[0] & 0xfff) > 0x190) + a6xx_gpu->has_whereami = true; + + msm_gem_put_vaddr(obj); +} + static int a6xx_ucode_init(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); @@ -528,6 +563,7 @@ static int a6xx_ucode_init(struct msm_gpu *gpu) } msm_gem_object_set_name(a6xx_gpu->sqe_bo, "sqefw"); + a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo); } gpu_write64(gpu, REG_A6XX_CP_SQE_INSTR_BASE_LO, @@ -743,8 +779,37 @@ static int a6xx_hw_init(struct msm_gpu *gpu) gpu_write64(gpu, REG_A6XX_CP_RB_BASE, REG_A6XX_CP_RB_BASE_HI, gpu->rb[0]->iova); - gpu_write(gpu, REG_A6XX_CP_RB_CNTL, - MSM_GPU_RB_CNTL_DEFAULT | AXXX_CP_RB_CNTL_NO_UPDATE); + /* Targets that support extended APRIV can use the RPTR shadow from +* hardware but all the other ones need to disable the feature. Targets +* that support the WHERE_AM_I opcode can use that instead +*/ + if (adreno_gpu->base.hw_apriv) + gpu_write(gpu, REG_A6XX_CP_RB_CNTL, MSM_GPU_RB_CNTL_DEFAULT); + else + gpu_write(gpu, REG_A6XX_CP_RB_CNTL, + MSM_GPU_RB_CNTL_DEFAULT | AXXX_CP_RB_CNTL_NO_UPDATE); + + /* +* Expanded APRIV and targets that support WHERE_AM_I both need a +* privileged buffer to store the RPTR shadow +*/ + + if (adreno_gpu->base.hw_apriv || a6xx_gpu->has_whereami) { + if (!a6xx_gpu->shadow_bo) { + a6xx_gpu->shadow = msm_gem_kernel_new_locked(gpu->dev, + sizeof(u32) * gpu->nr_rings, + MSM_BO_UNCACHED | MSM_BO_MAP_PRIV, + gpu->aspace, &a6xx_gpu->shadow_bo, + &a6xx_gpu->shadow_iova); + + if (IS_ERR(a6xx_gpu->shadow)) + return PTR_ERR(a6xx_gpu->shadow); + } + + gpu_write64(gpu, REG_A6XX_CP_RB_RPTR_ADDR_LO, + REG_A6XX_CP_RB_RPTR_ADDR_HI, + shadowptr(a6xx_gpu, gpu->rb[0])); + } /* Always come up on rb 0 */ a6xx_gpu->cur_ring = gpu->rb[0]; @@ -1033,6 +1098,11 @@ static void a6xx_destroy(struct msm_gpu *gpu) drm_gem_object_put(a6xx_gpu-
[PATCH 1/3] drm/msm: Allow a5xx to mark the RPTR shadow as privileged
Newer microcode versions have support for the CP_WHERE_AM_I opcode which allows the RPTR shadow memory to be marked as privileged to protect it from corruption. Move the RPTR shadow into its own buffer and protect it it if the current microcode version supports the new feature. We can also re-enable preemption for those targets that support CP_WHERE_AM_I. Start out by preemptively assuming that we can enable preemption and disable it in a5xx_hw_init if the microcode version comes back as too old. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 96 ++--- drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 12 +++ drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 5 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 5 ++ drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h | 1 + drivers/gpu/drm/msm/msm_gpu.h | 1 + 7 files changed, 109 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 616d9e798058..835aaef72b00 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -18,13 +18,24 @@ static void a5xx_dump(struct msm_gpu *gpu); #define GPU_PAS_ID 13 -static void a5xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring) +void a5xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring, + bool sync) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu); uint32_t wptr; unsigned long flags; + /* +* Most flush operations need to issue a WHERE_AM_I opcode to sync up +* the rptr shadow +*/ + if (a5xx_gpu->has_whereami && sync) { + OUT_PKT7(ring, CP_WHERE_AM_I, 2); + OUT_RING(ring, lower_32_bits(shadowptr(a5xx_gpu, ring))); + OUT_RING(ring, upper_32_bits(shadowptr(a5xx_gpu, ring))); + } + spin_lock_irqsave(&ring->lock, flags); /* Copy the shadow to the actual register */ @@ -90,7 +101,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit } } - a5xx_flush(gpu, ring); + a5xx_flush(gpu, ring, true); a5xx_preempt_trigger(gpu); /* we might not necessarily have a cmd from userspace to @@ -204,7 +215,8 @@ static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) /* Set bit 0 to trigger an interrupt on preempt complete */ OUT_RING(ring, 0x01); - a5xx_flush(gpu, ring); + /* A WHERE_AM_I packet is not needed after a YIELD */ + a5xx_flush(gpu, ring, false); /* Check to see if we need to start preemption */ a5xx_preempt_trigger(gpu); @@ -363,7 +375,7 @@ static int a5xx_me_init(struct msm_gpu *gpu) OUT_RING(ring, 0x); OUT_RING(ring, 0x); - gpu->funcs->flush(gpu, ring); + a5xx_flush(gpu, ring, true); return a5xx_idle(gpu, ring) ? 0 : -EINVAL; } @@ -405,11 +417,31 @@ static int a5xx_preempt_start(struct msm_gpu *gpu) OUT_RING(ring, 0x01); OUT_RING(ring, 0x01); - gpu->funcs->flush(gpu, ring); + /* The WHERE_AMI_I packet is not needed after a YIELD is issued */ + a5xx_flush(gpu, ring, false); return a5xx_idle(gpu, ring) ? 0 : -EINVAL; } +static void a5xx_ucode_check_version(struct a5xx_gpu *a5xx_gpu, + struct drm_gem_object *obj) +{ + u32 *buf = msm_gem_get_vaddr_active(obj); + + if (IS_ERR(buf)) + return; + + /* +* If the lowest nibble is 0xa that is an indication that this microcode +* has been patched. The actual version is in dword [3] but we only care +* about the patchlevel which is the lowest nibble of dword [3] +*/ + if (((buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1) + a5xx_gpu->has_whereami = true; + + msm_gem_put_vaddr(obj); +} + static int a5xx_ucode_init(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); @@ -445,6 +477,7 @@ static int a5xx_ucode_init(struct msm_gpu *gpu) } msm_gem_object_set_name(a5xx_gpu->pfp_bo, "pfpfw"); + a5xx_ucode_check_version(a5xx_gpu, a5xx_gpu->pfp_bo); } gpu_write64(gpu, REG_A5XX_CP_ME_INSTR_BASE_LO, @@ -504,6 +537,7 @@ static int a5xx_zap_shader_init(struct msm_gpu *gpu) static int a5xx_hw_init(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); + struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu); int ret; gpu_write(gpu, REG_A5XX_VBIF_ROUND_ROBIN_QOS_ARB, 0x0003); @@ -712,9 +746,36 @@ static int a5xx_hw_init(struct msm_gpu *gpu) gpu_write64(gpu, REG_A5XX_CP_RB_BASE, REG_A5XX_CP_RB_BASE_HI, gpu->rb[0]->iova); +
[PATCH 3/3] drm/msm: Get rid of the REG_ADRENO offsets
As newer GPU families are added it makes less sense to maintain a "generic" version functions for older families. Move adreno_submit() and get_rptr() into the target specific code for a2xx, a3xx and a4xx. Add a parameter to adreno_flush to pass the target specific WPTR register instead of relying on the generic register. All of this gets rid of the last of the REG_ADRENO offsets so remove all all the register definitions and infrastructure. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 65 +++- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 77 ++- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 82 ++--- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 12 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 13 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 82 + drivers/gpu/drm/msm/adreno/adreno_gpu.h | 81 +--- 7 files changed, 178 insertions(+), 234 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c index 48fa49f69d6d..7e82c41a85f1 100644 --- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c @@ -10,6 +10,48 @@ extern bool hang_debug; static void a2xx_dump(struct msm_gpu *gpu); static bool a2xx_idle(struct msm_gpu *gpu); +static void a2xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) +{ + struct msm_drm_private *priv = gpu->dev->dev_private; + struct msm_ringbuffer *ring = submit->ring; + unsigned int i; + + for (i = 0; i < submit->nr_cmds; i++) { + switch (submit->cmd[i].type) { + case MSM_SUBMIT_CMD_IB_TARGET_BUF: + /* ignore IB-targets */ + break; + case MSM_SUBMIT_CMD_CTX_RESTORE_BUF: + /* ignore if there has not been a ctx switch: */ + if (priv->lastctx == submit->queue->ctx) + break; + fallthrough; + case MSM_SUBMIT_CMD_BUF: + OUT_PKT3(ring, CP_INDIRECT_BUFFER_PFD, 2); + OUT_RING(ring, lower_32_bits(submit->cmd[i].iova)); + OUT_RING(ring, submit->cmd[i].size); + OUT_PKT2(ring); + break; + } + } + + OUT_PKT0(ring, REG_AXXX_CP_SCRATCH_REG2, 1); + OUT_RING(ring, submit->seqno); + + /* wait for idle before cache flush/interrupt */ + OUT_PKT3(ring, CP_WAIT_FOR_IDLE, 1); + OUT_RING(ring, 0x); + + OUT_PKT3(ring, CP_EVENT_WRITE, 3); + OUT_RING(ring, CACHE_FLUSH_TS); + OUT_RING(ring, rbmemptr(ring, fence)); + OUT_RING(ring, submit->seqno); + OUT_PKT3(ring, CP_INTERRUPT, 1); + OUT_RING(ring, 0x8000); + + adreno_flush(gpu, ring, REG_AXXX_CP_RB_WPTR); +} + static bool a2xx_me_init(struct msm_gpu *gpu) { struct msm_ringbuffer *ring = gpu->rb[0]; @@ -53,7 +95,7 @@ static bool a2xx_me_init(struct msm_gpu *gpu) OUT_PKT3(ring, CP_SET_PROTECTED_MODE, 1); OUT_RING(ring, 1); - gpu->funcs->flush(gpu, ring); + adreno_flush(gpu, ring, REG_AXXX_CP_RB_WPTR); return a2xx_idle(gpu); } @@ -421,16 +463,11 @@ a2xx_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev) return aspace; } -/* Register offset defines for A2XX - copy of A3XX */ -static const unsigned int a2xx_register_offsets[REG_ADRENO_REGISTER_MAX] = { - REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE), - REG_ADRENO_SKIP(REG_ADRENO_CP_RB_BASE_HI), - REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_RPTR_ADDR, REG_AXXX_CP_RB_RPTR_ADDR), - REG_ADRENO_SKIP(REG_ADRENO_CP_RB_RPTR_ADDR_HI), - REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_RPTR, REG_AXXX_CP_RB_RPTR), - REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_WPTR, REG_AXXX_CP_RB_WPTR), - REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_CNTL, REG_AXXX_CP_RB_CNTL), -}; +static u32 a2xx_get_rptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring) +{ + ring->memptrs->rptr = gpu_read(gpu, REG_AXXX_CP_RB_RPTR); + return ring->memptrs->rptr; +} static const struct adreno_gpu_funcs funcs = { .base = { @@ -439,8 +476,7 @@ static const struct adreno_gpu_funcs funcs = { .pm_suspend = msm_gpu_pm_suspend, .pm_resume = msm_gpu_pm_resume, .recover = a2xx_recover, - .submit = adreno_submit, - .flush = adreno_flush, + .submit = a2xx_submit, .active_ring = adreno_active_ring, .irq = a2xx_irq, .destroy = a2xx_destroy, @@ -450,6 +486,7 @@ static const struct adreno_gpu_funcs funcs = { .gpu_state_get = a2xx_gpu_state_get, .gpu_state_put = adreno_gpu_state_put, .create_address_space = a2xx_create_address_space, +
[PATCH 0/3] drm/msm: Add support for the WHERE_AM_I opcode
The microcode in linux-firmware has been updated to 1.87.01 for a5xx 1.77.01 for a6xx [1]. These microcode versions support a new opcode called WHERE_AM_I that takes the place of the hardware RPTR shadow and enables the microcode to update the RPTR shadow in privileged memory so it is protected against the user. This patch series re-enables the RPTR shadow and preemption for a5xx and older versions of a6xx if the WHERE_AM_I opcode is available. Newer a6xx targets (starting with a650) have automatic privileged protection so the hardware RPTR shadow can be renabled for those targets too. If any of the needed dependencies aren't met then the RPTR shadow will remain disabled (along with preemption on 5xx). This stack is bsed on https://gitlab.freedesktop.org/drm/msm.git msm-next-pgtables as there are some minor dependencies on the reorganized code in the pgtable stack. Jordan [1] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/?id=f48fec44127f88ce83ea1bcaf5824de4146ca2f9 Jordan Crouse (3): drm/msm: Allow a5xx to mark the RPTR shadow as privileged drm/msm: a6xx: Use WHERE_AM_I for eligible targets drm/msm: Get rid of the REG_ADRENO offsets drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 65 +--- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 77 +++--- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 82 +++ drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 108 drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 12 +++ drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 5 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 100 +++--- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 9 ++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 81 +-- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 81 +-- drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h | 1 + drivers/gpu/drm/msm/msm_gpu.h | 1 + 13 files changed, 377 insertions(+), 247 deletions(-) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 00/13] preempt: Make preempt count unconditional
On Mon, Sep 14, 2020 at 3:24 PM Linus Torvalds wrote: > > Ard and Herbert added to participants: see > chacha20poly1305_crypt_sg_inplace(), which does > > flags = SG_MITER_TO_SG; > if (!preemptible()) > flags |= SG_MITER_ATOMIC; > > introduced in commit d95312a3ccc0 ("crypto: lib/chacha20poly1305 - > reimplement crypt_from_sg() routine"). As far as I can tell, the only reason for this all is to try to use "kmap()" rather than "kmap_atomic()". And kmap() actually has the much more complex "might_sleep()" tests, and apparently the "preemptible()" check wasn't even the proper full debug check, it was just a complete hack to catch the one that triggered. >From a quick look, that code should probably just get rid of SG_MITER_ATOMIC entirely, and alwayse use kmap_atomic(). kmap_atomic() is actually the faster and proper interface to use anyway (never mind that any of this matters on any sane hardware). The old kmap() and kunmap() interfaces should generally be avoided like the plague - yes, they allow sleeping in the middle and that is sometimes required, but if you don't need that, you should never ever use them. We used to have a very nasty kmap_atomic() that required people to be very careful and know exactly which atomic entry to use, and that was admitedly quite nasty. So it _looks_ like this code started using kmap() - probably back when kmap_atomic() was so cumbersome to use - and was then converted (conditionally) to kmap_atomic() rather than just changed whole-sale. Is there actually something that wants to use those sg_miter functions and sleep? Because if there is, that choice should come from the outside, not from inside lib/scatterlist.c trying to make some bad guess based on the wrong thing entirely. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/13] dt-bindings: arm: fsl: Fix matching Purism Librem5 phones
On Fri, 04 Sep 2020 16:53:02 +0200, Krzysztof Kozlowski wrote: > All Purism Librem5 phones have three compatibles so they need their own > entry to fix dbts_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: /: > compatible: ['purism,librem5r2', 'purism,librem5', 'fsl,imx8mq'] is not > valid under any of the given schemas > > arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: /: > compatible: ['purism,librem5r2', 'purism,librem5', 'fsl,imx8mq'] is too > long > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > Reviewed-by: Rob Herring I expect Shawn to pick this one up as this file gets touched a fair amount. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/13] dt-bindings: gpu: vivante, gc: Remove trailing whitespace
On Fri, 04 Sep 2020 16:53:04 +0200, Krzysztof Kozlowski wrote: > Remove whitespace at the end of line. > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/gpu/vivante,gc.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] dt-bindings: gpu: vivante, gc: Add common properties
On Fri, 04 Sep 2020 16:53:03 +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (cooling-cells, assigned-clocks > and others) to fix dtbs_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mq-evk.dt.yaml: gpu@3800: > '#cooling-cells', 'assigned-clock-parents', 'assigned-clock-rates', > 'assigned-clocks' do not match any of the regexes: 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/gpu/vivante,gc.yaml | 7 +++ > 1 file changed, 7 insertions(+) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] dt-bindings: power: fsl, imx-gpcv2: Document interrupt controller properties
On Fri, 04 Sep 2020 16:53:00 +0200, Krzysztof Kozlowski wrote: > The i.MX General Power Controller v2 is also an interrupt controller so > document additional properties to fix dtbs_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mq-evk.dt.yaml: gpc@303a: > '#interrupt-cells', 'interrupt-controller' do not match any of the > regexes: 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml | 4 > 1 file changed, 4 insertions(+) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/13] dt-bindings: display: bridge: nwl-dsi: Add common properties
On Fri, 04 Sep 2020 16:53:01 +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (assigned-clocks and others) to > fix dtbs_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mq-evk.dt.yaml: mipi-dsi@30a0: > 'assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' do > not match any of the regexes: '^panel@[0-9]+$', 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml | 4 > 1 file changed, 4 insertions(+) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 00/13] preempt: Make preempt count unconditional
On Mon, Sep 14, 2020 at 2:55 PM Thomas Gleixner wrote: > > Yes it does generate better code, but I tried hard to spot a difference > in various metrics exposed by perf. It's all in the noise and I only > can spot a difference when the actual preemption check after the > decrement I'm somewhat more worried about the small-device case. That said, the diffstat certainly has its very clear charm, and I do agree that it makes things simpler. I'm just not convinced people should ever EVER do things like that "if (preemptible())" garbage. It sounds like somebody is doing seriously bad things. The chacha20poly1305 code does look fundamentally broken. But no, the fix is not to make "preemptible" work with spinlocks, the fix is to not *do* that kind of broken things. Those things would be broken even if you changed the semantics of preemptible. There's no way that it's valid to say "use this debug flag to decide if we should do atomic allocations or not". It smells like "I got a debug failure, so I'm papering it over by checking the thing the debug code checks for". The debug check is to catch the obvious bad cases. It's not the _only_ bad cases, so copying the debug check test is just completely wrong. Ard and Herbert added to participants: see chacha20poly1305_crypt_sg_inplace(), which does flags = SG_MITER_TO_SG; if (!preemptible()) flags |= SG_MITER_ATOMIC; introduced in commit d95312a3ccc0 ("crypto: lib/chacha20poly1305 - reimplement crypt_from_sg() routine"). You *fundamentally* cannot do that. Seriously. It's completely wrong. Pick one or the other, or have the caller *tell* you what the context is. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 1/4] drm/mediatek: disable tmds on mt2701
Hi, Frank: Frank Wunderlich 於 2020年9月4日 週五 下午7:01寫道: > > From: chunhui dai > > Without that patch if you use specific resolutions like 1280x1024, > I can see distortion in the output. It seems as if the > frequency for updating the pixel of the image is out of sync. > > For initialization tmds needs to be active, but can be disabled after init > to fix blurry display As discussed with Jitao offline, he agree this patch, so I applied this patch to mediatek-drm-next [1], thanks. [1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next Regards, Chun-Kuang. > > Signed-off-by: chunhui dai > Signed-off-by: Frank Wunderlich > Tested-by: Frank Wunderlich > --- > drivers/phy/mediatek/phy-mtk-hdmi-mt2701.c | 1 + > drivers/phy/mediatek/phy-mtk-hdmi.c| 3 +++ > drivers/phy/mediatek/phy-mtk-hdmi.h| 1 + > 3 files changed, 5 insertions(+) > > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt2701.c > b/drivers/phy/mediatek/phy-mtk-hdmi-mt2701.c > index a6cb1dea3d0c..b74c65a1762c 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt2701.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt2701.c > @@ -238,6 +238,7 @@ static void mtk_hdmi_phy_disable_tmds(struct mtk_hdmi_phy > *hdmi_phy) > > struct mtk_hdmi_phy_conf mtk_hdmi_phy_2701_conf = { > .flags = CLK_SET_RATE_GATE, > + .pll_default_off = true, > .hdmi_phy_clk_ops = &mtk_hdmi_phy_pll_ops, > .hdmi_phy_enable_tmds = mtk_hdmi_phy_enable_tmds, > .hdmi_phy_disable_tmds = mtk_hdmi_phy_disable_tmds, > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi.c > b/drivers/phy/mediatek/phy-mtk-hdmi.c > index 8fc83f01a720..47c029d4b270 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi.c > @@ -184,6 +184,9 @@ static int mtk_hdmi_phy_probe(struct platform_device > *pdev) > return PTR_ERR(phy_provider); > } > > + if (hdmi_phy->conf->pll_default_off) > + hdmi_phy->conf->hdmi_phy_disable_tmds(hdmi_phy); > + > return of_clk_add_provider(dev->of_node, of_clk_src_simple_get, >hdmi_phy->pll); > } > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi.h > b/drivers/phy/mediatek/phy-mtk-hdmi.h > index b13e1d5f8e78..dcf9bb13699b 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi.h > +++ b/drivers/phy/mediatek/phy-mtk-hdmi.h > @@ -21,6 +21,7 @@ struct mtk_hdmi_phy; > > struct mtk_hdmi_phy_conf { > unsigned long flags; > + bool pll_default_off; > const struct clk_ops *hdmi_phy_clk_ops; > void (*hdmi_phy_enable_tmds)(struct mtk_hdmi_phy *hdmi_phy); > void (*hdmi_phy_disable_tmds)(struct mtk_hdmi_phy *hdmi_phy); > -- > 2.25.1 > > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/ttm: remove available_caching
For both: Reviewed-by: Dave Airlie On Sat, 12 Sep 2020 at 01:24, Christian König wrote: > > Instead of letting TTM make an educated guess based on > some mask all drivers should just specify what caching > they want for their CPU mappings. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 1 - > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 - > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 -- > drivers/gpu/drm/drm_gem_vram_helper.c | 1 - > drivers/gpu/drm/nouveau/nouveau_ttm.c | 9 ++--- > drivers/gpu/drm/qxl/qxl_ttm.c | 3 +-- > drivers/gpu/drm/radeon/radeon_ttm.c | 2 -- > drivers/gpu/drm/ttm/ttm_bo.c | 8 ++-- > drivers/gpu/drm/ttm/ttm_range_manager.c | 5 + > drivers/gpu/drm/ttm/ttm_resource.c| 1 - > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 + > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 1 - > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 10 +++--- > include/drm/ttm/ttm_bo_driver.h | 5 + > include/drm/ttm/ttm_resource.h| 3 --- > 15 files changed, 11 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > index b2ca693b8285..73fd2335e468 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > @@ -93,7 +93,6 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, > uint64_t gtt_size) > > man->use_tt = true; > man->func = &amdgpu_gtt_mgr_func; > - man->available_caching = TTM_PL_MASK_CACHING; > > ttm_resource_manager_init(man, gtt_size >> PAGE_SHIFT); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 96aa8fcb9115..a761cc9cf422 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -68,7 +68,6 @@ static int amdgpu_ttm_init_on_chip(struct amdgpu_device > *adev, > uint64_t size) > { > return ttm_range_man_init(&adev->mman.bdev, type, > - TTM_PL_FLAG_UNCACHED, > false, size >> PAGE_SHIFT); > } > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > index 8b4a9ab66586..76f1d8db0c85 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > @@ -177,8 +177,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev) > struct ttm_resource_manager *man = &mgr->manager; > int ret; > > - man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC; > - > ttm_resource_manager_init(man, adev->gmc.real_vram_size >> > PAGE_SHIFT); > > man->func = &amdgpu_vram_mgr_func; > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c > b/drivers/gpu/drm/drm_gem_vram_helper.c > index 34dc665eb891..e42cc361fe90 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -1105,7 +1105,6 @@ static int drm_vram_mm_init(struct drm_vram_mm *vmm, > struct drm_device *dev, > return ret; > > ret = ttm_range_man_init(&vmm->bdev, TTM_PL_VRAM, > -TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC, > false, vram_size >> PAGE_SHIFT); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c > b/drivers/gpu/drm/nouveau/nouveau_ttm.c > index 1b00f32b3849..427341753441 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c > @@ -200,7 +200,6 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) > if (!man) > return -ENOMEM; > > - man->available_caching = TTM_PL_FLAG_UNCACHED | > TTM_PL_FLAG_WC; > man->func = &nouveau_vram_manager; > > ttm_resource_manager_init(man, > @@ -209,9 +208,7 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) > ttm_resource_manager_set_used(man, true); > return 0; > } else { > - return ttm_range_man_init(&drm->ttm.bdev, TTM_PL_VRAM, > - TTM_PL_FLAG_UNCACHED | > TTM_PL_FLAG_WC, > - false, > + return ttm_range_man_init(&drm->ttm.bdev, TTM_PL_VRAM, false, > drm->gem.vram_available >> > PAGE_SHIFT); > } > } > @@ -243,8 +240,7 @@ nouveau_ttm_init_gtt(struct nouveau_drm *drm) > else if (!drm->agp.bridge) > func = &nv04_gart_manager; > else > - return ttm_range_man_init(&drm->ttm.bdev, TTM_PL_TT, > - TT
Re: [PATCH] drm/ttm: remove default caching
Reviewed-by: Dave Airlie On Fri, 11 Sep 2020 at 23:10, Christian König wrote: > > As far as I can tell this was never used either and we just > always fallback to the order cached > wc > uncached anyway. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 1 - > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 1 - > drivers/gpu/drm/drm_gem_vram_helper.c | 3 +-- > drivers/gpu/drm/nouveau/nouveau_ttm.c | 21 +++ > drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- > drivers/gpu/drm/radeon/radeon_ttm.c | 6 ++ > drivers/gpu/drm/ttm/ttm_bo.c | 3 --- > drivers/gpu/drm/ttm/ttm_range_manager.c | 2 -- > drivers/gpu/drm/ttm/ttm_resource.c| 1 - > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 1 - > drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 1 - > include/drm/ttm/ttm_bo_driver.h | 2 -- > include/drm/ttm/ttm_resource.h| 3 --- > 15 files changed, 14 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > index 697bc2c6fdb2..b2ca693b8285 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > @@ -94,7 +94,6 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, > uint64_t gtt_size) > man->use_tt = true; > man->func = &amdgpu_gtt_mgr_func; > man->available_caching = TTM_PL_MASK_CACHING; > - man->default_caching = TTM_PL_FLAG_CACHED; > > ttm_resource_manager_init(man, gtt_size >> PAGE_SHIFT); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 651365183e75..96aa8fcb9115 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -68,7 +68,7 @@ static int amdgpu_ttm_init_on_chip(struct amdgpu_device > *adev, > uint64_t size) > { > return ttm_range_man_init(&adev->mman.bdev, type, > - TTM_PL_FLAG_UNCACHED, TTM_PL_FLAG_UNCACHED, > + TTM_PL_FLAG_UNCACHED, > false, size >> PAGE_SHIFT); > } > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > index 7574be6cd7a0..8b4a9ab66586 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c > @@ -178,7 +178,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev) > int ret; > > man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC; > - man->default_caching = TTM_PL_FLAG_WC; > > ttm_resource_manager_init(man, adev->gmc.real_vram_size >> > PAGE_SHIFT); > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c > b/drivers/gpu/drm/drm_gem_vram_helper.c > index 5f10aa7aa099..34dc665eb891 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -1106,8 +1106,7 @@ static int drm_vram_mm_init(struct drm_vram_mm *vmm, > struct drm_device *dev, > > ret = ttm_range_man_init(&vmm->bdev, TTM_PL_VRAM, > TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC, > -TTM_PL_FLAG_WC, false, > -vram_size >> PAGE_SHIFT); > +false, vram_size >> PAGE_SHIFT); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c > b/drivers/gpu/drm/nouveau/nouveau_ttm.c > index a62f37b1131c..cf18f75cd0f1 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c > @@ -203,12 +203,9 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) > return -ENOMEM; > > man->available_caching = TTM_PL_FLAG_UNCACHED | > TTM_PL_FLAG_WC; > - man->default_caching = TTM_PL_FLAG_WC; > > - if (type & NVIF_MEM_UNCACHED) { > + if (type & NVIF_MEM_UNCACHED) > man->available_caching = TTM_PL_FLAG_UNCACHED; > - man->default_caching = TTM_PL_FLAG_UNCACHED; > - } > > man->func = &nouveau_vram_manager; > > @@ -220,7 +217,7 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) > } else { > return ttm_range_man_init(&drm->ttm.bdev, TTM_PL_VRAM, > TTM_PL_FLAG_UNCACHED | > TTM_PL_FLAG_WC, > - TTM_PL_FLAG_WC, false, > + false, > drm->gem.vram_available >> > PAGE_SHIFT); > } > } > @@ -245,16 +2
Re: [PATCH 1/1] drm/amdgpu: Convert to using devm_drm_dev_alloc()
On Fri, Sep 11, 2020 at 4:50 PM Luben Tuikov wrote: > > On 2020-09-08 16:09, Luben Tuikov wrote: > > On 2020-09-07 04:07, Daniel Vetter wrote: > >> On Mon, Sep 07, 2020 at 10:06:08AM +0200, Daniel Vetter wrote: > >>> On Sat, Sep 05, 2020 at 11:50:05AM -0400, Alex Deucher wrote: > On Thu, Sep 3, 2020 at 9:22 PM Luben Tuikov wrote: > > > > Convert to using devm_drm_dev_alloc(), > > as drm_dev_init() is going away. > > > > Signed-off-by: Luben Tuikov > > I think we can drop the final drm_put in the error case? I think the > unwinding in current devm code should take care of it. > >>> > >>> Same applies for the pci remove hook too. > >> > >> KASAN run with unload should have caught this. > > > > But it didn't? Why? > > Could it be that drm_dev_put() actually got > > the kref to 0 and then drm_dev_release() > > was called which did a kfree()? > > > > Could you try that same unload KASAN run but > > with your suggestion of removing drm_dev_put() from > > amdgpu_pci_remove()? What do you get then? > > Hi Daniel, > > Have you had a chance to run this unload KASAN run with > your suggestion of removing drm_dev_put() from > the PCI release hook? > > If it "should have caught this", but it didn't, > perhaps it did catch it when you removed the drm_dev_put() > hook from the PCI release hook, when you did a KASAN unload run? > Showing that drm_dev_put() is still necessary, since, > 1) we're still using kref, > 2) kref is kref-init-ed under devm_drm_dev_alloc() as I pointed >out in my reply to Alex in this thread. > > I believe KASAN (and logic) show this patch to be solid. > > > > >> I strongly recommend doing > >> that for any changes to the unload code, it's way to easy to mix up > >> something and release it in the wrong order or from the wrong callback or > >> with the wrong managed (devm_ vs drmm_) functions. > > > > Sorry, I don't understand what you mean by "doing that"? Do > > you mean "not calling drm_dev_put()"? Sure, but what > > are we supposed to call instead? > > > > I also don't understand what you mean by "easy to mix up something > > and release it in wrong order or from the wrong callback..." etc. > > > > If you want things to happen in certain order, > > you can either put the correct-order-sequence > > behind the non-zero-->0 transition of kref, say in > > drm_dev_release() as it is right now, > > > > static void drm_dev_release(struct kref *ref) > > { > > struct drm_device *dev = container_of(ref, struct drm_device, ref); > > > > if (dev->driver->release) > > dev->driver->release(dev); > > > > drm_managed_release(dev); > > > > kfree(dev->managed.final_kfree); > > } > > > > Or you can remove kref from DRM dev (which I do not > > recommend), and stipulate the release sequence > > as I asked in Message-ID: <165961bb-3b5b-cedc-2fc0-838b7999d...@amd.com>, > > "Re: [PATCH] drm/managed: Cleanup of unused functions and polishing docs". > > > > Then we can follow that and submit patches to conform. > > Eagerly awaiting your response on this so that we can conform > to the direction you're setting forth. > > Are you removing kref (release() cb) from DRM and if so, > what function should we call in order to do the "final" > (although without kref, the notion of "final" is obviated) > free, OR kref stays in and this patch, which conforms > to using devm_drm_dev_alloc(), as postulated by you, > can go in. devm_drm_dev_init() calls devm_add_action() which adds devm_drm_dev_init_release() as the function which gets called for resource unwinding. That calls drm_dev_put() which handles the ref counting and clean up, so I don't think we need to call drm_dev_put() in any of our unwinding paths anymore. All of the drm bits are handled for us. Alex > > Regards, > Luben > > > > > Regards, > > Luben > > > > > > > >> -Daniel > >> > >>> -Daniel > > Alex > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++ > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > index 146a85c8df1c..06d994187c24 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > @@ -1142,18 +1142,13 @@ static int amdgpu_pci_probe(struct pci_dev > > *pdev, > > if (ret) > > return ret; > > > > - adev = kzalloc(sizeof(*adev), GFP_KERNEL); > > - if (!adev) > > - return -ENOMEM; > > + adev = devm_drm_dev_alloc(&pdev->dev, &kms_driver, > > typeof(*adev), ddev); > > + if (IS_ERR(adev)) > > + return PTR_ERR(adev); > > > > adev->dev = &pdev->dev; > > adev->pdev = pdev; > > ddev = adev_to_drm(adev); > > - ret = drm_dev_init(ddev, &kms_driver, &pdev->dev); >
Re: [PATCH v2 10/16] drm/exynos: implement a drm bridge
Hi Marek, Michael, On 14.09.2020 22:01, Michael Tretter wrote: > Hi, > > On Mon, 14 Sep 2020 14:31:19 +0200, Marek Szyprowski wrote: >> On 14.09.2020 10:29, Marek Szyprowski wrote: >>> On 11.09.2020 15:54, Michael Tretter wrote: Make the exynos_dsi driver a full drm bridge that can be found and used from other drivers. Other drivers can only attach to the bridge, if a mipi dsi device already attached to the bridge. This allows to defer the probe of the display pipe until the downstream bridges are available, too. Signed-off-by: Michael Tretter >>> This one (and the whole series applied) still fails on Exynos boards: >>> >>> [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations >>> exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) >>> OF: graph: no port node found in /soc/dsi@11c8 >>> 8<--- cut here --- >>> Unable to handle kernel NULL pointer dereference at virtual address 0084 >>> pgd = (ptrval) >>> [0084] *pgd= >>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM >>> Modules linked in: >>> CPU: 1 PID: 1 Comm: swapper/0 Not tainted >>> 5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608 >>> Hardware name: Samsung Exynos (Flattened Device Tree) >>> PC is at drm_bridge_attach+0x18/0x164 >>> LR is at exynos_dsi_bind+0x88/0xa8 >>> pc : [] lr : [] psr: 2013 >>> sp : ef0dfca8 ip : 0002 fp : c13190e0 >>> r10: r9 : ee46d580 r8 : c13190e0 >>> r7 : ee438800 r6 : 0018 r5 : ef253810 r4 : ef39e840 >>> r3 : r2 : 0018 r1 : ef39e888 r0 : ef39e840 >>> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >>> Control: 10c5387d Table: 4000404a DAC: 0051 >>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) >>> Stack: (0xef0dfca8 to 0xef0e) >>> ... >>> [] (drm_bridge_attach) from [] >>> (exynos_dsi_bind+0x88/0xa8) >>> [] (exynos_dsi_bind) from [] >>> (component_bind_all+0xfc/0x290) >>> [] (component_bind_all) from [] >>> (exynos_drm_bind+0xe4/0x19c) >>> [] (exynos_drm_bind) from [] >>> (try_to_bring_up_master+0x1e4/0x2c4) >>> [] (try_to_bring_up_master) from [] >>> (component_master_add_with_match+0xd4/0x108) >>> [] (component_master_add_with_match) from [] >>> (exynos_drm_platform_probe+0xe4/0x110) >>> [] (exynos_drm_platform_probe) from [] >>> (platform_drv_probe+0x6c/0xa4) >>> [] (platform_drv_probe) from [] >>> (really_probe+0x200/0x4fc) >>> [] (really_probe) from [] >>> (driver_probe_device+0x78/0x1fc) >>> [] (driver_probe_device) from [] >>> (device_driver_attach+0x58/0x60) >>> [] (device_driver_attach) from [] >>> (__driver_attach+0xdc/0x174) >>> [] (__driver_attach) from [] >>> (bus_for_each_dev+0x68/0xb4) >>> [] (bus_for_each_dev) from [] >>> (bus_add_driver+0x158/0x214) >>> [] (bus_add_driver) from [] (driver_register+0x78/0x110) >>> [] (driver_register) from [] >>> (exynos_drm_init+0xe4/0x118) >>> [] (exynos_drm_init) from [] >>> (do_one_initcall+0x8c/0x42c) >>> [] (do_one_initcall) from [] >>> (kernel_init_freeable+0x190/0x1dc) >>> [] (kernel_init_freeable) from [] >>> (kernel_init+0x8/0x118) >>> [] (kernel_init) from [] (ret_from_fork+0x14/0x20) >>> Exception stack(0xef0dffb0 to 0xef0dfff8) >>> ... >>> ---[ end trace ee27f313f9ed9da1 ]--- >>> >>> # arm-linux-gnueabi-addr2line -e vmlinux c0628c08 >>> drivers/gpu/drm/drm_bridge.c:184 (discriminator 1) >>> >>> I will try to debug it a bit more today. >> The above crash has been caused by lack of in_bridge initialization to >> NULL in exynos_dsi_bind() in this patch. However, fixing it reveals >> another issue: >> >> [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations >> exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) >> OF: graph: no port node found in /soc/dsi@11c8 >> 8<--- cut here --- >> Unable to handle kernel NULL pointer dereference at virtual address 0280 >> pgd = (ptrval) >> [0280] *pgd= >> Internal error: Oops: 5 [#1] PREEMPT SMP ARM >> Modules linked in: >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted >> 5.9.0-rc4-next-20200911-00010-g417dc70d70ec-dirty #1613 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> PC is at __mutex_lock+0x54/0xb18 >> LR is at lock_is_held_type+0x80/0x138 >> pc : [] lr : [] psr: 6013 >> sp : ef0dfd30 ip : 33937b74 fp : c13193c8 >> r10: c1208eec r9 : r8 : ee45f808 >> r7 : c19561a4 r6 : r5 : r4 : 024c >> r3 : r2 : 00204140 r1 : c124f13c r0 : >> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >> Control: 10c5387d Table: 4000404a DAC: 0051 >> Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) >> Stack: (0xef0dfd30 to 0xef0e) >> ... >> [] (__mutex_lock) from [] (mutex_lock_nested+0x1c/0x24) >> [] (mutex_lock_nested) from [] >> (__exynos_dsi_host_attach+0x20/0x6c) >> [] (__exynos_dsi_host_attach) from [] >> (exynos_dsi_host_attach+0x70/0x194) >> [] (exynos_dsi_host_attach) from [] >> (s6e8aa0_probe+0x1b0/0x21
Re: [patch 00/13] preempt: Make preempt count unconditional
On Mon, Sep 14, 2020 at 1:45 PM Thomas Gleixner wrote: > > Recently merged code does: > > gfp = preemptible() ? GFP_KERNEL : GFP_ATOMIC; > > Looks obviously correct, except for the fact that preemptible() is > unconditionally false for CONFIF_PREEMPT_COUNT=n, i.e. all allocations in > that code use GFP_ATOMIC on such kernels. I don't think this is a good reason to entirely get rid of the no-preempt thing. The above is just garbage. It's bogus. You can't do it. Blaming the no-preempt code for this bug is extremely unfair, imho. And the no-preempt code does help make for much better code generation for simple spinlocks. Where is that horribly buggy recent code? It's not in that exact format, certainly, since 'grep' doesn't find it. Linus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 00/13] preempt: Make preempt count unconditional
On Mon, 14 Sep 2020 22:42:09 +0200 Thomas Gleixner wrote: > 21 files changed, 23 insertions(+), 92 deletions(-) This alone makes it look promising, and hopefully acceptable by Linus :-) -- Steve ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/6] dt-bindings: gpu: arm, mali-utgard: Correct Maxime's email
On Thu, 03 Sep 2020 21:14:35 +0200, Krzysztof Kozlowski wrote: > Update the address of Maxime Ripard as one in @free-electrons.com does > not work. > > Cc: Maxime Ripard > Signed-off-by: Krzysztof Kozlowski > Acked-by: Maxime Ripard > > --- > > Changes since v1: > 1. Add Ack > --- > Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/6] dt-bindings: gpu: samsung-rotator: Add missing properties
On Thu, Sep 03, 2020 at 09:14:36PM +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (iommus, power-domains) to fix > dtbs_check warnings like: > > arch/arm/boot/dts/exynos4210-i9100.dt.yaml: rotator@1281: > 'iommus', 'power-domains' do not match any of the regexes: > 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v1: > 1. Add properties instead of using unevaluated > --- > Documentation/devicetree/bindings/gpu/samsung-rotator.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > index 665c6e3b31d3..f480174fe0d3 100644 > --- a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml > @@ -22,6 +22,9 @@ properties: >interrupts: > maxItems: 1 > > + iommus: true > + power-domains: true These need to define how many. I assume 1, so 'maxItems: 1'. > + >clocks: > maxItems: 1 > > -- > 2.17.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/6] dt-bindings: gpu: arm, mali-utgard: Add missing properties
On Thu, 03 Sep 2020 21:14:34 +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (opp-table) to fix dtbs_check > warnings like: > > arch/arm/boot/dts/exynos4210-i9100.dt.yaml: gpu@1300: > 'opp-table' does not match any of the regexes: 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v1: > 1. Add properties instead of using unevaluated > --- > Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/6] dt-bindings: gpu: arm, mali-midgard: Add missing properties
On Thu, 03 Sep 2020 21:14:33 +0200, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (opp-table) to fix dtbs_check > warnings like: > > arch/arm64/boot/dts/exynos/exynos5433-tm2.dt.yaml: gpu@14ac: > 'opp-table' does not match any of the regexes: 'pinctrl-[0-9]+' > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v1: > 1. Add properties instead of using unevaluated > --- > Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml | 1 + > 1 file changed, 1 insertion(+) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/16] drm/exynos: implement a drm bridge
Hi, On Mon, 14 Sep 2020 14:31:19 +0200, Marek Szyprowski wrote: > On 14.09.2020 10:29, Marek Szyprowski wrote: > > On 11.09.2020 15:54, Michael Tretter wrote: > >> Make the exynos_dsi driver a full drm bridge that can be found and used > >> from other drivers. > >> > >> Other drivers can only attach to the bridge, if a mipi dsi device > >> already attached to the bridge. This allows to defer the probe of the > >> display pipe until the downstream bridges are available, too. > >> > >> Signed-off-by: Michael Tretter > > This one (and the whole series applied) still fails on Exynos boards: > > > > [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations > > exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) > > OF: graph: no port node found in /soc/dsi@11c8 > > 8<--- cut here --- > > Unable to handle kernel NULL pointer dereference at virtual address 0084 > > pgd = (ptrval) > > [0084] *pgd= > > Internal error: Oops: 5 [#1] PREEMPT SMP ARM > > Modules linked in: > > CPU: 1 PID: 1 Comm: swapper/0 Not tainted > > 5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608 > > Hardware name: Samsung Exynos (Flattened Device Tree) > > PC is at drm_bridge_attach+0x18/0x164 > > LR is at exynos_dsi_bind+0x88/0xa8 > > pc : [] lr : [] psr: 2013 > > sp : ef0dfca8 ip : 0002 fp : c13190e0 > > r10: r9 : ee46d580 r8 : c13190e0 > > r7 : ee438800 r6 : 0018 r5 : ef253810 r4 : ef39e840 > > r3 : r2 : 0018 r1 : ef39e888 r0 : ef39e840 > > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > > Control: 10c5387d Table: 4000404a DAC: 0051 > > Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) > > Stack: (0xef0dfca8 to 0xef0e) > > ... > > [] (drm_bridge_attach) from [] > > (exynos_dsi_bind+0x88/0xa8) > > [] (exynos_dsi_bind) from [] > > (component_bind_all+0xfc/0x290) > > [] (component_bind_all) from [] > > (exynos_drm_bind+0xe4/0x19c) > > [] (exynos_drm_bind) from [] > > (try_to_bring_up_master+0x1e4/0x2c4) > > [] (try_to_bring_up_master) from [] > > (component_master_add_with_match+0xd4/0x108) > > [] (component_master_add_with_match) from [] > > (exynos_drm_platform_probe+0xe4/0x110) > > [] (exynos_drm_platform_probe) from [] > > (platform_drv_probe+0x6c/0xa4) > > [] (platform_drv_probe) from [] > > (really_probe+0x200/0x4fc) > > [] (really_probe) from [] > > (driver_probe_device+0x78/0x1fc) > > [] (driver_probe_device) from [] > > (device_driver_attach+0x58/0x60) > > [] (device_driver_attach) from [] > > (__driver_attach+0xdc/0x174) > > [] (__driver_attach) from [] > > (bus_for_each_dev+0x68/0xb4) > > [] (bus_for_each_dev) from [] > > (bus_add_driver+0x158/0x214) > > [] (bus_add_driver) from [] (driver_register+0x78/0x110) > > [] (driver_register) from [] > > (exynos_drm_init+0xe4/0x118) > > [] (exynos_drm_init) from [] > > (do_one_initcall+0x8c/0x42c) > > [] (do_one_initcall) from [] > > (kernel_init_freeable+0x190/0x1dc) > > [] (kernel_init_freeable) from [] > > (kernel_init+0x8/0x118) > > [] (kernel_init) from [] (ret_from_fork+0x14/0x20) > > Exception stack(0xef0dffb0 to 0xef0dfff8) > > ... > > ---[ end trace ee27f313f9ed9da1 ]--- > > > > # arm-linux-gnueabi-addr2line -e vmlinux c0628c08 > > drivers/gpu/drm/drm_bridge.c:184 (discriminator 1) > > > > I will try to debug it a bit more today. > > The above crash has been caused by lack of in_bridge initialization to > NULL in exynos_dsi_bind() in this patch. However, fixing it reveals > another issue: > > [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) > OF: graph: no port node found in /soc/dsi@11c8 > 8<--- cut here --- > Unable to handle kernel NULL pointer dereference at virtual address 0280 > pgd = (ptrval) > [0280] *pgd= > Internal error: Oops: 5 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 5.9.0-rc4-next-20200911-00010-g417dc70d70ec-dirty #1613 > Hardware name: Samsung Exynos (Flattened Device Tree) > PC is at __mutex_lock+0x54/0xb18 > LR is at lock_is_held_type+0x80/0x138 > pc : [] lr : [] psr: 6013 > sp : ef0dfd30 ip : 33937b74 fp : c13193c8 > r10: c1208eec r9 : r8 : ee45f808 > r7 : c19561a4 r6 : r5 : r4 : 024c > r3 : r2 : 00204140 r1 : c124f13c r0 : > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 4000404a DAC: 0051 > Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) > Stack: (0xef0dfd30 to 0xef0e) > ... > [] (__mutex_lock) from [] (mutex_lock_nested+0x1c/0x24) > [] (mutex_lock_nested) from [] > (__exynos_dsi_host_attach+0x20/0x6c) > [] (__exynos_dsi_host_attach) from [] > (exynos_dsi_host_attach+0x70/0x194) > [] (exynos_dsi_host_attach) from [] > (s6e8aa0_probe+0x1b0/0x218) > [] (s6e8aa0_probe) from [] (really_probe+0x200/0x4fc) > [] (really_probe) from [] > (driv
Re: [Intel-gfx] [PATCH 2/3] drm/atomic-helper: Remove the timestamping constant update from drm_atomic_helper_update_legacy_modeset_state()
On Mon, Sep 07, 2020 at 08:12:56PM +0200, Daniel Vetter wrote: > On Mon, Sep 07, 2020 at 03:00:25PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > The timestamping constants have nothing to do with any legacy state > > so should not be updated from > > drm_atomic_helper_update_legacy_modeset_state(). > > > > Let's make everyone call drm_atomic_helper_calc_timestamping_constants() > > directly instead of relying on > > drm_atomic_helper_update_legacy_modeset_state() to call it. > > > > @@ > > expression S; > > @@ > > - drm_atomic_helper_calc_timestamping_constants(S); > > > > @@ > > expression D, S; > > @@ > > drm_atomic_helper_update_legacy_modeset_state(D, S); > > + drm_atomic_helper_calc_timestamping_constants(S); > > > > Signed-off-by: Ville Syrjälä > > I think the kerneldoc for > drm_crtc_vblank_helper_get_vblank/_timestamp_internal (both of them) also > needs to be updated to mention the new function. With that fixed: > > Reviewed-by: Daniel Vetter Fixed the docs while applying. Thanks for the review. > > > > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 + > > drivers/gpu/drm/drm_atomic_helper.c | 7 ++- > > drivers/gpu/drm/i915/display/intel_display.c | 1 + > > drivers/gpu/drm/nouveau/dispnv50/disp.c | 1 + > > 4 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > index 490684787cff..0511097343da 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > @@ -7397,6 +7397,7 @@ static void amdgpu_dm_atomic_commit_tail(struct > > drm_atomic_state *state) > > int crtc_disable_count = 0; > > > > drm_atomic_helper_update_legacy_modeset_state(dev, state); > > + drm_atomic_helper_calc_timestamping_constants(state); > > > > dm_state = dm_atomic_get_new_state(state); > > if (dm_state && dm_state->context) { > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index 673e3fc282d9..45ee613c8efd 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -1115,9 +1115,7 @@ disable_outputs(struct drm_device *dev, struct > > drm_atomic_state *old_state) > > * @old_state: atomic state object with old state structures > > * > > * This function updates all the various legacy modeset state pointers in > > - * connectors, encoders and CRTCs. It also updates the timestamping > > constants > > - * used for precise vblank timestamps by calling > > - * drm_calc_timestamping_constants(). > > + * connectors, encoders and CRTCs. > > * > > * Drivers can use this for building their own atomic commit if they don't > > have > > * a pure helper-based modeset implementation. > > @@ -1187,8 +1185,6 @@ drm_atomic_helper_update_legacy_modeset_state(struct > > drm_device *dev, > > crtc->y = new_plane_state->src_y >> 16; > > } > > } > > - > > - drm_atomic_helper_calc_timestamping_constants(old_state); > > } > > EXPORT_SYMBOL(drm_atomic_helper_update_legacy_modeset_state); > > > > @@ -1296,6 +1292,7 @@ void drm_atomic_helper_commit_modeset_disables(struct > > drm_device *dev, > > disable_outputs(dev, old_state); > > > > drm_atomic_helper_update_legacy_modeset_state(dev, old_state); > > + drm_atomic_helper_calc_timestamping_constants(old_state); > > > > crtc_set_mode(dev, old_state); > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index ec148a8da2c2..035840ce3825 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -15578,6 +15578,7 @@ static void intel_atomic_commit_tail(struct > > intel_atomic_state *state) > > > > if (state->modeset) { > > drm_atomic_helper_update_legacy_modeset_state(dev, > > &state->base); > > + drm_atomic_helper_calc_timestamping_constants(&state->base); > > > > intel_set_cdclk_pre_plane_update(state); > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c > > b/drivers/gpu/drm/nouveau/dispnv50/disp.c > > index 7799530e07c1..b6d1b926bc5e 100644 > > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > > @@ -2069,6 +2069,7 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state > > *state) > > drm_atomic_helper_wait_for_fences(dev, state, false); > > drm_atomic_helper_wait_for_dependencies(state); > > drm_atomic_helper_update_legacy_modeset_state(dev, state); > > + drm_atomic_helper_calc_timestamping_constants(state); > > > > if (atom->lock_core) > > mutex_lock(&disp->mutex); > > -- > > 2.26.2 > > > > ___ > > Intel-gfx mailing list > >
[Bug 208825] lspci triggers NULL pointer dereference on AMD Renoir 4800H/5600M laptop
https://bugzilla.kernel.org/show_bug.cgi?id=208825 --- Comment #3 from Jon Tourville (jontourvi...@me.com) --- I am now unable to reproduce even on versions <5.8.6, which I know still had the problem. So I am thinking it may have been a firmware update or something else that resolved the issue for me. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 2/3] drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge
Hi Swapnil, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v5.9-rc5 next-20200914] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Swapnil-Jakhade/drm-Add-support-for-Cadence-MHDP8546-DPI-DP-bridge-and-J721E-wrapper/20200914-205242 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: x86_64-allyesconfig (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce (this is a W=1 build): # save the attached .config to linux build tree make W=1 ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_fw_activate': >> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:749:10: warning: >> conversion from 'long unsigned int' to 'unsigned int' changes value from >> '18446744073709551613' to '4294967293' [-Woverflow] 749 | writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_fill_host_caps': drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:1359:30: error: 'struct phy_attrs' has no member named 'max_link_rate' 1359 | link_rate = mhdp->phy->attrs.max_link_rate; | ^ drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_attach': drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:1690:10: warning: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551613' to '4294967293' [-Woverflow] 1690 | writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_bridge_hpd_enable': drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:2123:10: warning: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551613' to '4294967293' [-Woverflow] 2123 | writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, # https://github.com/0day-ci/linux/commit/747d94aa1dc15ccc7712116aa9161477b0ff3e85 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Swapnil-Jakhade/drm-Add-support-for-Cadence-MHDP8546-DPI-DP-bridge-and-J721E-wrapper/20200914-205242 git checkout 747d94aa1dc15ccc7712116aa9161477b0ff3e85 vim +749 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 692 693 static int cdns_mhdp_fw_activate(const struct firmware *fw, 694 struct cdns_mhdp_device *mhdp) 695 { 696 unsigned int reg; 697 int ret; 698 699 /* Release uCPU reset and stall it. */ 700 writel(CDNS_CPU_STALL, mhdp->regs + CDNS_APB_CTRL); 701 702 memcpy_toio(mhdp->regs + CDNS_MHDP_IMEM, fw->data, fw->size); 703 704 /* Leave debug mode, release stall */ 705 writel(0, mhdp->regs + CDNS_APB_CTRL); 706 707 /* 708 * Wait for the KEEP_ALIVE "message" on the first 8 bits. 709 * Updated each sched "tick" (~2ms) 710 */ 711 ret = readl_poll_timeout(mhdp->regs + CDNS_KEEP_ALIVE, reg, 712 reg & CDNS_KEEP_ALIVE_MASK, 500, 713 CDNS_KEEP_ALIVE_TIMEOUT); 714 if (ret) { 715 dev_err(mhdp->dev, 716 "device didn't give any life sign: reg %d\n", reg); 717 return ret; 718 } 719 720 ret = cdns_mhdp_check_fw_version(mhdp); 721 if (ret) 722 return ret; 723 724 /* Init events to 0 as it's not cleared by FW at boot but on read */ 725 readl(mhdp->regs + CDNS_SW_EVENT0); 726 readl(mhdp->regs + CDNS_SW_EVENT1); 727 readl(mhdp->regs + CDNS_SW_EVENT2); 728 readl(mhdp->regs + CDNS_SW_EVENT3); 729 730 /* Activate uCPU */ 731 ret = cdns_mhdp_set_firmware_active(mhdp, true); 732 if (ret) 733 return ret; 734 735 spin_lock(&mhdp->start_lock); 736 737 mhdp->hw_state = MHDP_HW_READY; 738 739 /* 740 * Here we must keep the lock while enabling the interrupts
Re: Changing vma->vm_file in dma_buf_mmap()
Am 14.09.20 um 16:06 schrieb Jason Gunthorpe: On Mon, Sep 14, 2020 at 03:30:47PM +0200, Christian König wrote: Am 14.09.20 um 15:29 schrieb Christian König: Hi Andrew, I'm the new DMA-buf maintainer and Daniel and others came up with patches extending the use of the dma_buf_mmap() function. Now this function is doing something a bit odd by changing the vma->vm_file while installing a VMA in the mmap() system call It doesn't look obviously safe as mmap_region() has an interesting mix of file and vma->file Eg it calls mapping_unmap_writable() using both routes Thanks for the hint, going to take a look at that code tomorrow. What about security? Is it OK that some other random file, maybe in another process, is being linked to this mmap? Good question, I have no idea. That's why I send out this mail. The background here is that DMA-buf allows device drivers to export buffer which are then imported into another device driver. The mmap() handler of the importing device driver then find that the pgoff belongs to the exporting device and so redirects the mmap() call there. So the pgoff is some virtualized thing? Yes, absolutely. Christian. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] dt-bindings: display: samsung, amoled-mipi-dsi: Do not require enable-gpios on samsung, s6e63j0x03
On Sat, 29 Aug 2020 19:25:29 +0200, Krzysztof Kozlowski wrote: > The samsung,s6e63j0x03 does not have enable GPIO, so do not require it. > This fixes dtbs_check warning: > > arch/arm/boot/dts/exynos3250-rinato.dt.yaml: panel@0: 'enable-gpios' is a > required property > > Signed-off-by: Krzysztof Kozlowski > --- > .../display/panel/samsung,amoled-mipi-dsi.yaml | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/10] dt-bindings: connector: Convert Samsung 11-pin USB bindings to dtschema
On Sat, 29 Aug 2020 16:24:56 +0200, Krzysztof Kozlowski wrote: > Add Samsung 11-pin USB-C connector into standard dtschema bindings file. > > Signed-off-by: Krzysztof Kozlowski > --- > .../connector/samsung,usb-connector-11pin.txt | 49 --- > .../bindings/connector/usb-connector.yaml | 44 + > 2 files changed, 44 insertions(+), 49 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt > Applied, thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/20] drm/amdgpu: Introduce GEM object functions
Am 14.09.20 um 17:05 schrieb Thomas Zimmermann: Hi Am 13.08.20 um 12:22 schrieb Christian König: Am 13.08.20 um 10:36 schrieb Thomas Zimmermann: GEM object functions deprecate several similar callback interfaces in struct drm_driver. This patch replaces the per-driver callbacks with per-instance callbacks in amdgpu. The only exception is gem_prime_mmap, which is non-trivial to convert. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 81a79760ca61..51525b8774c9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = { .lastclose = amdgpu_driver_lastclose_kms, .irq_handler = amdgpu_irq_handler, .ioctls = amdgpu_ioctls_kms, - .gem_free_object_unlocked = amdgpu_gem_object_free, - .gem_open_object = amdgpu_gem_object_open, - .gem_close_object = amdgpu_gem_object_close, .dumb_create = amdgpu_mode_dumb_create, .dumb_map_offset = amdgpu_mode_dumb_mmap, .fops = &amdgpu_driver_kms_fops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_export = amdgpu_gem_prime_export, .gem_prime_import = amdgpu_gem_prime_import, - .gem_prime_vmap = amdgpu_gem_prime_vmap, - .gem_prime_vunmap = amdgpu_gem_prime_vunmap, .gem_prime_mmap = amdgpu_gem_prime_mmap, .name = DRIVER_NAME, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 43f4966331dd..ca2b79f94e99 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -36,6 +36,7 @@ #include #include #include "amdgpu.h" +#include "amdgpu_dma_buf.h" #include "amdgpu_trace.h" #include "amdgpu_amdkfd.h" @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) #endif } +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = { + .free = amdgpu_gem_object_free, + .open = amdgpu_gem_object_open, + .close = amdgpu_gem_object_close, + .export = amdgpu_gem_prime_export, + .vmap = amdgpu_gem_prime_vmap, + .vunmap = amdgpu_gem_prime_vunmap, +}; + Wrong file, this belongs into amdgpu_gem.c static int amdgpu_bo_do_create(struct amdgpu_device *adev, struct amdgpu_bo_param *bp, struct amdgpu_bo **bo_ptr) @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); if (bo == NULL) return -ENOMEM; + + bo->tbo.base.funcs = &amdgpu_gem_object_funcs; And this should probably go into amdgpu_gem_object_create(). I'm trying to understand what amdgpu does. What about all the places where amdgpu calls amdgpu_bo_create() internally? Wouldn't these miss the free callback for the GEM object? Those shouldn't have a GEM object in the first place. Or otherwise we would have a reference counting issue. Regards, Christian. Best regards Thomas Apart from that looks like a good idea to me. Christian. drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size); INIT_LIST_HEAD(&bo->shadow_list); bo->vm_bo = NULL; ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 0/5] Add support for DisplayPort driver on SnapDragon
On 2020-09-12 11:25, Rob Clark wrote: Fyi, I've pushed this series and the dp-compliance bits to msm-next-dp[1] I didn't include the dp audio series yet, which seems to need some minor rebasing. (And a small request, when resending, cc freedr...@lists.freedesktop.org, so it shows up in the patchwork instance[2] I use) You might want to double check that I got the correct versions of the series, etc. And that nothing else (other than audio) is missing. Thanks Rob for pulling changes in msm-next-dp branch. I confirm that changes you pointed above are latest changes and nothing else is missing in driver. Sure, we will make sure to send changes in freedreno list as well. Thanks. BR, -R [1] https://gitlab.freedesktop.org/drm/msm/-/commits/msm-next-dp [2] https://patchwork.freedesktop.org/project/freedreno On Thu, Aug 27, 2020 at 2:17 PM Tanmay Shah wrote: These patches add Display-Port driver on SnapDragon/msm hardware. This series also contains device-tree bindings for msm DP driver. It also contains Makefile and Kconfig changes to compile msm DP driver. The block diagram of DP driver is shown below: +-+ |DRM FRAMEWORK| +--+--+ | +v+ | DP DRM | +++ | +v+ ++| DP+--++--+ ++---+| DISPLAY |+---+ | | |++-+-+-+| | | || | | | | | || | | | | | || | | | | | vv v v v v v +--+ +--+ +---+ ++ ++ +---+ +-+ | DP | | DP | |DP | | DP | | DP | |DP | | DP | |PARSER| | HPD | |AUX| |LINK| |CTRL| |PHY| |POWER| +--+---+ +---+--+ +---+ ++ +--+-+ +-+-+ +-+ | | | +--v---+ +v-v+ |DEVICE| | DP | | TREE | |CATALOG| +--+ +---+---+ | +---v+ |CTRL/PHY| | HW | ++ Changes in v12: -- Add support of pm ops in display port driver -- Clear bpp depth bits before writing to MISC register -- Fix edid read Previous change log: https://lkml.kernel.org/lkml/20200818051137.21478-1-tan...@codeaurora.org/ Chandan Uddaraju (4): dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon drm: add constant N value in helper file drm/msm/dp: add displayPort driver support drm/msm/dp: add support for DP PLL driver Jeykumar Sankaran (1): drm/msm/dpu: add display port support in DPU Tanmay Shah (1): drm/msm/dp: Add Display Port HPD feature drivers/gpu/drm/i915/display/intel_display.c |2 +- drivers/gpu/drm/msm/Kconfig |9 + drivers/gpu/drm/msm/Makefile | 14 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c |8 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 83 +- drivers/gpu/drm/msm/dp/dp_aux.c | 535 ++ drivers/gpu/drm/msm/dp/dp_aux.h | 30 + drivers/gpu/drm/msm/dp/dp_catalog.c | 1045 ++ drivers/gpu/drm/msm/dp/dp_catalog.h | 105 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 1693 + drivers/gpu/drm/msm/dp/dp_ctrl.h | 35 + drivers/gpu/drm/msm/dp/dp_display.c | 1046 ++ drivers/gpu/drm/msm/dp/dp_display.h | 31 + drivers/gpu/drm/msm/dp/dp_drm.c | 168 ++ drivers/gpu/drm/msm/dp/dp_drm.h | 18 + drivers/gpu/drm/msm/dp/dp_hpd.c | 69 + drivers/gpu/drm/msm/dp/dp_hpd.h | 79 + drivers/gpu/drm/msm/dp/dp_link.c | 1214 drivers/gpu/drm/msm/dp/dp_link.h | 132 ++ drivers/gpu/drm/msm/dp/dp_panel.c | 486 + drivers/gpu/drm/msm/dp/dp_panel.h | 95 + drivers/gpu/drm/msm/dp/dp_parser.c| 267 +++ drivers/gpu/drm/msm/dp/dp_parser.h| 138 ++ drivers/gpu/drm/msm/dp/dp_pll.c | 99 + drivers/gpu/drm/msm/dp/dp_pll.h | 61 + drivers/gpu/drm/msm/dp/dp_pll_10nm.c | 930 + drivers/gpu/drm/msm/dp/dp_pll_private.h | 89 + drivers/gpu/drm/msm/dp/dp_power.c | 373 drivers/gpu/drm/msm/dp/dp_power.h | 103 + drivers/gpu/drm/msm/dp/dp_reg.h | 518 + drivers/gpu/drm/msm/msm_drv.c |2 + drivers/gpu/drm/msm/msm_drv.h | 59 +- include/drm/drm_dp_helper.h |1 + 34 fi
Re: [Intel-gfx] [PATCH] drm/i915: Fix the race between the GEM close and debugfs
On 14/09/2020 12:00, Nikunj A. Dadhania wrote: As we close GEM object and set file_priv to -EBADF which is protected by ctx->mutex, populating the GEM debugfs info is not protected and results in the crash shown below. Make sure to protect the access to file_priv using ctx->mutex to avoid race. BUG: unable to handle page fault for address: RIP: 0010:i915_gem_object_info+0x26b/0x3eb Code: 89 44 24 48 48 89 44 24 40 48 89 44 24 38 48 89 44 24 30 48 89 44 24 28 48 89 44 24 20 49 8b 46 f0 48 89 44 24 20 49 8b 46 a0 <48> 8b 58 08 b9 0a 00 00 00 48 b8 aa aa aa aa aa aa aa aa 48 8d bc RSP: 0018:ac81c14cfc30 EFLAGS: 00010246 RAX: fff7 RBX: 95094429c218 RCX: 95096756c740 RDX: RSI: 919b93ee RDI: 95094429c218 RBP: ac81c14cfd58 R08: 9509746fab80 R09: R10: 0001 R11: R12: 9509753f8e80 R13: ac81c14cfc98 R14: 95094429c268 R15: ac81c14cfc88 FS: 7a1bdcd52900() GS:950977e0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00026b4e CR4: 00340ef0 Call Trace: seq_read+0x162/0x3ca full_proxy_read+0x5b/0x8d __vfs_read+0x45/0x1b9 vfs_read+0xc9/0x15e ksys_read+0x7e/0xde do_syscall_64+0x54/0x7e entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7a1bdd34cf03 Signed-off-by: Nikunj A. Dadhania --- drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 784219962193..ea469168cd44 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m, } i915_gem_context_unlock_engines(ctx); + mutex_lock(&ctx->mutex); if (!IS_ERR_OR_NULL(ctx->file_priv)) { struct file_stats stats = { .vm = rcu_access_pointer(ctx->vm), @@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m, print_file_stats(m, name, stats); } + mutex_unlock(&ctx->mutex); spin_lock(&i915->gem.contexts.lock); list_safe_reset_next(ctx, cn, link); Fix is correct, but it looked familiar and indeed I found a fix for the same issues back from July. Copied you on that one which now has an r-b. This one can have it as well but please also copy stable. Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 3/3] drm/i915/gem: Serialise debugfs i915_gem_objects with ctx->mutex
On 23/07/2020 18:21, Chris Wilson wrote: Since the debugfs may peek into the GEM contexts as the corresponding client/fd is being closed, we may try and follow a dangling pointer. However, the context closure itself is serialised with the ctx->mutex, so if we hold that mutex as we inspect the state coupled in the context, we know the pointers within the context are stable and will remain valid as we inspect their tables. Signed-off-by: Chris Wilson Cc: CQ Tang Cc: Daniel Vetter Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 784219962193..ea469168cd44 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -326,6 +326,7 @@ static void print_context_stats(struct seq_file *m, } i915_gem_context_unlock_engines(ctx); + mutex_lock(&ctx->mutex); if (!IS_ERR_OR_NULL(ctx->file_priv)) { struct file_stats stats = { .vm = rcu_access_pointer(ctx->vm), @@ -346,6 +347,7 @@ static void print_context_stats(struct seq_file *m, print_file_stats(m, name, stats); } + mutex_unlock(&ctx->mutex); spin_lock(&i915->gem.contexts.lock); list_safe_reset_next(ctx, cn, link); Hm this apparently never got it's r-b and so got re-discovered in the field. +Nikunj Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/a6xx: fix a potential overflow issue
On Sat, Sep 12, 2020 at 06:25:58PM +0800, Zhenzhong Duan wrote: > It's allocating an array of a6xx_gpu_state_obj structure rathor than > its pointers. > > This patch fix it. > > Signed-off-by: Zhenzhong Duan LGTM but should have a Fixes: tag for the stable trees Fixes: d6852b4b2d01 ("drm/msm/a6xx: Track and manage a6xx state memory") Reviewed-by: Jordan Crouse > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > index b12f5b4..e9ede19 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > @@ -875,7 +875,7 @@ static void a6xx_get_indexed_registers(struct msm_gpu > *gpu, > int i; > > a6xx_state->indexed_regs = state_kcalloc(a6xx_state, count, > - sizeof(a6xx_state->indexed_regs)); > + sizeof(*a6xx_state->indexed_regs)); > if (!a6xx_state->indexed_regs) > return; > > -- > 1.8.3.1 > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
On 2020-09-14 5:33 p.m., Kazlauskas, Nicholas wrote: On 2020-09-14 11:22 a.m., Michel Dänzer wrote: On 2020-09-14 4:37 p.m., Kazlauskas, Nicholas wrote: On 2020-09-14 3:52 a.m., Michel Dänzer wrote: P.S. Since DCN doesn't make a distinction between primary or overlay planes in hardware, could ChromiumOS achieve the same effect with only the primary plane instead of only an overlay plane? If so, maybe there's no need for the "black plane hack". I only know that atomictest tries to enable this configuration. Not sure if ChromiumOS or other "real" userspace tries this configuration. Someone mentioned that ChromiumOS uses this for video playback with black bars (when the video aspect ratio doesn't match the display's). We only expose support for NV12 on the primary plane so we wouldn't be hitting this case at least. Interesting, so if we're lucky this really won't affect any real-world apps. We can always go back to lying to userspace about the cursor being visible if the commit fails in that case I guess [...] I'm not sure what you mean by that / how it could work. Dropping the check you added in this patch: + if (state->enable && + !(state->plane_mask & drm_plane_mask(crtc->primary))) return -EINVAL; That way we can still allow the cursor plane to be enabled while primary/overlay are not, it's just not going to be actually visible on the screen. It'll fail some IGT tests but nothing really uses this configuration as more than just a temporary state. As Daniel pointed out in <20200901075432.GW2352366@phenom.ffwll.local> in the v1 patch thread, that won't fly, since atomic userspace can legitimately expect the cursor plane to be visible in that case. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 2/3] drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge
Hi Swapnil, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on linus/master v5.9-rc5 next-20200914] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Swapnil-Jakhade/drm-Add-support-for-Cadence-MHDP8546-DPI-DP-bridge-and-J721E-wrapper/20200914-205242 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: s390-allyesconfig (attached as .config) compiler: s390-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_fw_activate': >> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:749:10: warning: >> conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes >> value from '18446744073709551613' to '4294967293' [-Woverflow] 749 | writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_fill_host_caps': drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:1359:30: error: 'struct phy_attrs' has no member named 'max_link_rate' 1359 | link_rate = mhdp->phy->attrs.max_link_rate; | ^ drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_attach': drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:1690:10: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551613' to '4294967293' [-Woverflow] 1690 | writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c: In function 'cdns_mhdp_bridge_hpd_enable': drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:2123:10: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551613' to '4294967293' [-Woverflow] 2123 | writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, # https://github.com/0day-ci/linux/commit/747d94aa1dc15ccc7712116aa9161477b0ff3e85 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Swapnil-Jakhade/drm-Add-support-for-Cadence-MHDP8546-DPI-DP-bridge-and-J721E-wrapper/20200914-205242 git checkout 747d94aa1dc15ccc7712116aa9161477b0ff3e85 vim +749 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 692 693 static int cdns_mhdp_fw_activate(const struct firmware *fw, 694 struct cdns_mhdp_device *mhdp) 695 { 696 unsigned int reg; 697 int ret; 698 699 /* Release uCPU reset and stall it. */ 700 writel(CDNS_CPU_STALL, mhdp->regs + CDNS_APB_CTRL); 701 702 memcpy_toio(mhdp->regs + CDNS_MHDP_IMEM, fw->data, fw->size); 703 704 /* Leave debug mode, release stall */ 705 writel(0, mhdp->regs + CDNS_APB_CTRL); 706 707 /* 708 * Wait for the KEEP_ALIVE "message" on the first 8 bits. 709 * Updated each sched "tick" (~2ms) 710 */ 711 ret = readl_poll_timeout(mhdp->regs + CDNS_KEEP_ALIVE, reg, 712 reg & CDNS_KEEP_ALIVE_MASK, 500, 713 CDNS_KEEP_ALIVE_TIMEOUT); 714 if (ret) { 715 dev_err(mhdp->dev, 716 "device didn't give any life sign: reg %d\n", reg); 717 return ret; 718 } 719 720 ret = cdns_mhdp_check_fw_version(mhdp); 721 if (ret) 722 return ret; 723 724 /* Init events to 0 as it's not cleared by FW at boot but on read */ 725 readl(mhdp->regs + CDNS_SW_EVENT0); 726 readl(mhdp->regs + CDNS_SW_EVENT1); 727 readl(mhdp->regs + CDNS_SW_EVENT2); 728 readl(mhdp->regs + CDNS_SW_EVENT3); 729 730 /* Activate uCPU */ 731 ret = cdns_mhdp_set_firmware_active(mhdp, true); 732 if (ret) 733
Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
On 2020-09-14 11:22 a.m., Michel Dänzer wrote: On 2020-09-14 4:37 p.m., Kazlauskas, Nicholas wrote: On 2020-09-14 3:52 a.m., Michel Dänzer wrote: On 2020-09-07 9:57 a.m., Daniel Vetter wrote: On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote: From: Michel Dänzer Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * &drm_mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Signed-off-by: Michel Dänzer Commit message agrees with my understand of this maze now, so: Acked-by: Daniel Vetter Thanks Daniel! Nick / Harry, any more feedback? If not, can you apply this? P.S. Since DCN doesn't make a distinction between primary or overlay planes in hardware, could ChromiumOS achieve the same effect with only the primary plane instead of only an overlay plane? If so, maybe there's no need for the "black plane hack". I only know that atomictest tries to enable this configuration. Not sure if ChromiumOS or other "real" userspace tries this configuration. Someone mentioned that ChromiumOS uses this for video playback with black bars (when the video aspect ratio doesn't match the display's). We only expose support for NV12 on the primary plane so we wouldn't be hitting this case at least. Maybe for now this can go in and if something breaks we can deal with the fallout then. We can always go back to lying to userspace about the cursor being visible if the commit fails in that case I guess [...] I'm not sure what you mean by that / how it could work. Dropping the check you added in this patch: + if (state->enable && + !(state->plane_mask & drm_plane_mask(crtc->primary))) return -EINVAL; That way we can still allow the cursor plane to be enabled while primary/overlay are not, it's just not going to be actually visible on the screen. It'll fail some IGT tests but nothing really uses this configuration as more than just a temporary state. Regards, Nicholas Kazlauskas Reviewed-by: Nicholas Kazlauskas Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
On 2020-09-14 4:37 p.m., Kazlauskas, Nicholas wrote: On 2020-09-14 3:52 a.m., Michel Dänzer wrote: On 2020-09-07 9:57 a.m., Daniel Vetter wrote: On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote: From: Michel Dänzer Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * &drm_mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Signed-off-by: Michel Dänzer Commit message agrees with my understand of this maze now, so: Acked-by: Daniel Vetter Thanks Daniel! Nick / Harry, any more feedback? If not, can you apply this? P.S. Since DCN doesn't make a distinction between primary or overlay planes in hardware, could ChromiumOS achieve the same effect with only the primary plane instead of only an overlay plane? If so, maybe there's no need for the "black plane hack". I only know that atomictest tries to enable this configuration. Not sure if ChromiumOS or other "real" userspace tries this configuration. Someone mentioned that ChromiumOS uses this for video playback with black bars (when the video aspect ratio doesn't match the display's). Maybe for now this can go in and if something breaks we can deal with the fallout then. We can always go back to lying to userspace about the cursor being visible if the commit fails in that case I guess [...] I'm not sure what you mean by that / how it could work. Reviewed-by: Nicholas Kazlauskas Thanks! -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: per-plane LUTs and CSCs?
On Mon, Sep 14, 2020 at 10:38:24AM -0400, Alex Deucher wrote: > On Mon, Sep 14, 2020 at 9:32 AM Ville Syrjälä > wrote: > > > > On Mon, Sep 14, 2020 at 02:13:09AM -0400, Alex Deucher wrote: > > > On Thu, Sep 10, 2020 at 4:29 AM Simon Ser wrote: > > > > > > > > On Thursday, September 10, 2020 10:18 AM, Daniel Vetter > > > > wrote: > > > > > > > > > On Thu, Sep 10, 2020 at 07:50:59AM +, Simon Ser wrote: > > > > > > > > > > > On Wednesday, September 9, 2020 12:57 PM, Laurentiu Palcu > > > > > > laurentiu.pa...@oss.nxp.com wrote: > > > > > > > > > > > > > Hi all, > > > > > > > I was wondering whether you could give me an advise on how to > > > > > > > proceed further > > > > > > > with the following issue as I'm preparing to upstream the next > > > > > > > set of patches > > > > > > > for the iMX8MQ display controller(DCSS). The display controller > > > > > > > has 3 planes, > > > > > > > each with 2 CSCs and one degamma LUT. The CSCs are in front and > > > > > > > after the LUT > > > > > > > respectively. Then the output from those 3 pipes is blended > > > > > > > together and then > > > > > > > gamma correction is applied using a linear-to-nonlinear LUT and > > > > > > > another CSC, if > > > > > > > needed. > > > > > > > Currently, downstream, we have all those CSCs and LUTs hard-coded > > > > > > > into a header > > > > > > > file. Based on the colorspace, range, gamut selected for the > > > > > > > output and/or > > > > > > > plane input, we pick up the right CSCs and LUTs from that header > > > > > > > file to > > > > > > > configure our pipes... I guess this solution does the job, > > > > > > > userspace doesn't > > > > > > > need to care much about how to generate those tables. But, it's > > > > > > > also not very > > > > > > > flexible in case there is an app smart enough to know and > > > > > > > actually generate > > > > > > > their own custom tables. :/ > > > > > > > Looking through the dri-devel archives, I've seen that there was > > > > > > > a tentative to > > > > > > > implement a more or less generic per-plane LUT/CSC solution but > > > > > > > it didn't make > > > > > > > it in due to lack of userspace consumers... > > > > > > > > > > > > Apart from full color management mentioned by Pekka, are there other > > > > > > use-cases for these per-plane props? > > > > > > One thing I can think of is that some drivers annoyingly only apply > > > > > > the > > > > > > per-CRTC gamma LUT to the primary plane. I think it would make more > > > > > > sense to not attach a gamma prop to the CRTC and instead only > > > > > > attach it > > > > > > to the primary plane to make that clear. But I guess that would also > > > > > > break existing user-space? > > > > > > > > > > Uh, which drivers? This would be a driver bug (and there's been > > > > > plenty of > > > > > those where the cursor has the wrong color temp and fun stuff like > > > > > that). > > > > > We need to fix these driver issues instead of lamenting in userspace > > > > > that > > > > > it's all kinda broken :-) > > > > > > > > Apparently this is bug with the old radeon driver [1]. It works fine on > > > > all i915 setups I've tried, and also works fine on amdgpu (with DC). > > > > > > > > I've opened a radeon bug. > > > > > > > > [1]: https://github.com/swaywm/wlroots/issues/968 > > > > > > This is a hardware limitation. I suspend all hardware of a certain > > > age had this same limitation. Old stuff didn't have multiple planes. > > > > That doesn't sound right to me. mach64 vt/gt and rage128 had an > > overlay plane already. I even vaguely remeber staring at some > > radeon overlay code at some point thinking "that stuff looks > > identical to the rage128 stuff, wonder why it's not shared code?". > > > > Ah, yeah, sorry, I forgot about that. We don't expose that via KMS. > Actually r128 is almost identical to some of the early radeons. I > actually had a ddx branch years ago which added r128 support to > xf86-video-ati: > https://cgit.freedesktop.org/~agd5f/xf86-video-ati/log/?h=r128-support > That overlay plane went away in the r300 time frame IIRC as everyone > moved to using the 3D engine for CSC and scaling. Right. Windows didn't have use for overlay planes so a lot of hw lost them around that time I guess. Intel hw didn't quite lose them, but they did lose a bunch of features such as scaling and planar YCbCr formats. And at least in our case most of the lost stuff has been reintroduced in recent years after Android/Windows started to use them again. Which is fine by me since I can often use ancient hw to bring up some of the "new" features in the latest hw ;) -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/20] drm/amdgpu: Introduce GEM object functions
Hi Am 13.08.20 um 12:22 schrieb Christian König: > Am 13.08.20 um 10:36 schrieb Thomas Zimmermann: >> GEM object functions deprecate several similar callback interfaces in >> struct drm_driver. This patch replaces the per-driver callbacks with >> per-instance callbacks in amdgpu. The only exception is gem_prime_mmap, >> which is non-trivial to convert. >> >> Signed-off-by: Thomas Zimmermann >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 -- >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 >> 2 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> index 81a79760ca61..51525b8774c9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = { >> .lastclose = amdgpu_driver_lastclose_kms, >> .irq_handler = amdgpu_irq_handler, >> .ioctls = amdgpu_ioctls_kms, >> - .gem_free_object_unlocked = amdgpu_gem_object_free, >> - .gem_open_object = amdgpu_gem_object_open, >> - .gem_close_object = amdgpu_gem_object_close, >> .dumb_create = amdgpu_mode_dumb_create, >> .dumb_map_offset = amdgpu_mode_dumb_mmap, >> .fops = &amdgpu_driver_kms_fops, >> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, >> .prime_fd_to_handle = drm_gem_prime_fd_to_handle, >> - .gem_prime_export = amdgpu_gem_prime_export, >> .gem_prime_import = amdgpu_gem_prime_import, >> - .gem_prime_vmap = amdgpu_gem_prime_vmap, >> - .gem_prime_vunmap = amdgpu_gem_prime_vunmap, >> .gem_prime_mmap = amdgpu_gem_prime_mmap, >> .name = DRIVER_NAME, >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> index 43f4966331dd..ca2b79f94e99 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> @@ -36,6 +36,7 @@ >> #include >> #include >> #include "amdgpu.h" >> +#include "amdgpu_dma_buf.h" >> #include "amdgpu_trace.h" >> #include "amdgpu_amdkfd.h" >> @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) >> #endif >> } >> +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = { >> + .free = amdgpu_gem_object_free, >> + .open = amdgpu_gem_object_open, >> + .close = amdgpu_gem_object_close, >> + .export = amdgpu_gem_prime_export, >> + .vmap = amdgpu_gem_prime_vmap, >> + .vunmap = amdgpu_gem_prime_vunmap, >> +}; >> + > > Wrong file, this belongs into amdgpu_gem.c > >> static int amdgpu_bo_do_create(struct amdgpu_device *adev, >> struct amdgpu_bo_param *bp, >> struct amdgpu_bo **bo_ptr) >> @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct >> amdgpu_device *adev, >> bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); >> if (bo == NULL) >> return -ENOMEM; >> + >> + bo->tbo.base.funcs = &amdgpu_gem_object_funcs; > > And this should probably go into amdgpu_gem_object_create(). I'm trying to understand what amdgpu does. What about all the places where amdgpu calls amdgpu_bo_create() internally? Wouldn't these miss the free callback for the GEM object? Best regards Thomas > > Apart from that looks like a good idea to me. > > Christian. > >> drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size); >> INIT_LIST_HEAD(&bo->shadow_list); >> bo->vm_bo = NULL; > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: per-plane LUTs and CSCs?
On Mon, Sep 14, 2020 at 9:32 AM Ville Syrjälä wrote: > > On Mon, Sep 14, 2020 at 02:13:09AM -0400, Alex Deucher wrote: > > On Thu, Sep 10, 2020 at 4:29 AM Simon Ser wrote: > > > > > > On Thursday, September 10, 2020 10:18 AM, Daniel Vetter > > > wrote: > > > > > > > On Thu, Sep 10, 2020 at 07:50:59AM +, Simon Ser wrote: > > > > > > > > > On Wednesday, September 9, 2020 12:57 PM, Laurentiu Palcu > > > > > laurentiu.pa...@oss.nxp.com wrote: > > > > > > > > > > > Hi all, > > > > > > I was wondering whether you could give me an advise on how to > > > > > > proceed further > > > > > > with the following issue as I'm preparing to upstream the next set > > > > > > of patches > > > > > > for the iMX8MQ display controller(DCSS). The display controller has > > > > > > 3 planes, > > > > > > each with 2 CSCs and one degamma LUT. The CSCs are in front and > > > > > > after the LUT > > > > > > respectively. Then the output from those 3 pipes is blended > > > > > > together and then > > > > > > gamma correction is applied using a linear-to-nonlinear LUT and > > > > > > another CSC, if > > > > > > needed. > > > > > > Currently, downstream, we have all those CSCs and LUTs hard-coded > > > > > > into a header > > > > > > file. Based on the colorspace, range, gamut selected for the output > > > > > > and/or > > > > > > plane input, we pick up the right CSCs and LUTs from that header > > > > > > file to > > > > > > configure our pipes... I guess this solution does the job, > > > > > > userspace doesn't > > > > > > need to care much about how to generate those tables. But, it's > > > > > > also not very > > > > > > flexible in case there is an app smart enough to know and actually > > > > > > generate > > > > > > their own custom tables. :/ > > > > > > Looking through the dri-devel archives, I've seen that there was a > > > > > > tentative to > > > > > > implement a more or less generic per-plane LUT/CSC solution but it > > > > > > didn't make > > > > > > it in due to lack of userspace consumers... > > > > > > > > > > Apart from full color management mentioned by Pekka, are there other > > > > > use-cases for these per-plane props? > > > > > One thing I can think of is that some drivers annoyingly only apply > > > > > the > > > > > per-CRTC gamma LUT to the primary plane. I think it would make more > > > > > sense to not attach a gamma prop to the CRTC and instead only attach > > > > > it > > > > > to the primary plane to make that clear. But I guess that would also > > > > > break existing user-space? > > > > > > > > Uh, which drivers? This would be a driver bug (and there's been plenty > > > > of > > > > those where the cursor has the wrong color temp and fun stuff like > > > > that). > > > > We need to fix these driver issues instead of lamenting in userspace > > > > that > > > > it's all kinda broken :-) > > > > > > Apparently this is bug with the old radeon driver [1]. It works fine on > > > all i915 setups I've tried, and also works fine on amdgpu (with DC). > > > > > > I've opened a radeon bug. > > > > > > [1]: https://github.com/swaywm/wlroots/issues/968 > > > > This is a hardware limitation. I suspend all hardware of a certain > > age had this same limitation. Old stuff didn't have multiple planes. > > That doesn't sound right to me. mach64 vt/gt and rage128 had an > overlay plane already. I even vaguely remeber staring at some > radeon overlay code at some point thinking "that stuff looks > identical to the rage128 stuff, wonder why it's not shared code?". > Ah, yeah, sorry, I forgot about that. We don't expose that via KMS. Actually r128 is almost identical to some of the early radeons. I actually had a ddx branch years ago which added r128 support to xf86-video-ati: https://cgit.freedesktop.org/~agd5f/xf86-video-ati/log/?h=r128-support That overlay plane went away in the r300 time frame IIRC as everyone moved to using the 3D engine for CSC and scaling. Alex > > It had a primary plane and a cursor and gamma didn't apply to the > > cursor. > > That last part I can believe. > > -- > Ville Syrjälä > Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
On 2020-09-14 3:52 a.m., Michel Dänzer wrote: On 2020-09-07 9:57 a.m., Daniel Vetter wrote: On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote: From: Michel Dänzer Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * &drm_mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Signed-off-by: Michel Dänzer Commit message agrees with my understand of this maze now, so: Acked-by: Daniel Vetter Thanks Daniel! Nick / Harry, any more feedback? If not, can you apply this? P.S. Since DCN doesn't make a distinction between primary or overlay planes in hardware, could ChromiumOS achieve the same effect with only the primary plane instead of only an overlay plane? If so, maybe there's no need for the "black plane hack". I only know that atomictest tries to enable this configuration. Not sure if ChromiumOS or other "real" userspace tries this configuration. Maybe for now this can go in and if something breaks we can deal with the fallout then. We can always go back to lying to userspace about the cursor being visible if the commit fails in that case I guess since the blank plane hack is going to add a significant amount of complexity to DM. Reviewed-by: Nicholas Kazlauskas Regards, Nicholas Kazlauskas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: imx8m: Failed to attach bridge
On Mon, Sep 14, 2020 at 10:52 AM Fabio Estevam wrote: > > Hi Martin and Guido, > > I am trying to get MIPI DSI panel to work on an imx8mq-evk board. > > Here are the changes I did against linux-next 20200914 following what > was done on imx8mq-librem5-devkit.dts: > https://pastebin.com/raw/GXazRyNx > > The config I am using is this one: > https://pastebin.com/raw/UGAjTQyg > > This is what I see in dmesg: > > [1.666974] mxsfb 3032.lcd-controller: [drm:mxsfb_probe] > *ERROR* failed to attach bridge: -517 > [1.667309] sdhci-esdhc-imx 30b5.mmc: Got CD GPIO > [1.675977] mxsfb 3032.lcd-controller: Cannot connect bridge: -517 > [1.689620] mxsfb 3032.lcd-controller: [drm:mxsfb_probe] > *ERROR* failed to attach bridge: -517 > [1.692019] mmc0: SDHCI controller on 30b4.mmc [30b4.mmc] using > ADMA > [1.698606] mxsfb 3032.lcd-controller: Cannot connect bridge: -517 > [1.711321] mmc1: SDHCI controller on 30b5.mmc [30b5.mmc] using > ADMA > [1.721418] mxsfb 3032.lcd-controller: [drm:mxsfb_probe] > *ERROR* failed to attach bridge: -517 > [1.730422] mxsfb 3032.lcd-controller: Cannot connect bridge: -517 > > Any ideas as to what I am missing? Nevermind. I got it to work. The 'dsi-lanes' property for the panel was missing. I will submit a patch later. Thanks ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
imx8m: Failed to attach bridge
Hi Martin and Guido, I am trying to get MIPI DSI panel to work on an imx8mq-evk board. Here are the changes I did against linux-next 20200914 following what was done on imx8mq-librem5-devkit.dts: https://pastebin.com/raw/GXazRyNx The config I am using is this one: https://pastebin.com/raw/UGAjTQyg This is what I see in dmesg: [1.666974] mxsfb 3032.lcd-controller: [drm:mxsfb_probe] *ERROR* failed to attach bridge: -517 [1.667309] sdhci-esdhc-imx 30b5.mmc: Got CD GPIO [1.675977] mxsfb 3032.lcd-controller: Cannot connect bridge: -517 [1.689620] mxsfb 3032.lcd-controller: [drm:mxsfb_probe] *ERROR* failed to attach bridge: -517 [1.692019] mmc0: SDHCI controller on 30b4.mmc [30b4.mmc] using ADMA [1.698606] mxsfb 3032.lcd-controller: Cannot connect bridge: -517 [1.711321] mmc1: SDHCI controller on 30b5.mmc [30b5.mmc] using ADMA [1.721418] mxsfb 3032.lcd-controller: [drm:mxsfb_probe] *ERROR* failed to attach bridge: -517 [1.730422] mxsfb 3032.lcd-controller: Cannot connect bridge: -517 Any ideas as to what I am missing? Thanks, Fabio Estevam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: per-plane LUTs and CSCs?
On Mon, Sep 14, 2020 at 02:13:09AM -0400, Alex Deucher wrote: > On Thu, Sep 10, 2020 at 4:29 AM Simon Ser wrote: > > > > On Thursday, September 10, 2020 10:18 AM, Daniel Vetter > > wrote: > > > > > On Thu, Sep 10, 2020 at 07:50:59AM +, Simon Ser wrote: > > > > > > > On Wednesday, September 9, 2020 12:57 PM, Laurentiu Palcu > > > > laurentiu.pa...@oss.nxp.com wrote: > > > > > > > > > Hi all, > > > > > I was wondering whether you could give me an advise on how to proceed > > > > > further > > > > > with the following issue as I'm preparing to upstream the next set of > > > > > patches > > > > > for the iMX8MQ display controller(DCSS). The display controller has 3 > > > > > planes, > > > > > each with 2 CSCs and one degamma LUT. The CSCs are in front and after > > > > > the LUT > > > > > respectively. Then the output from those 3 pipes is blended together > > > > > and then > > > > > gamma correction is applied using a linear-to-nonlinear LUT and > > > > > another CSC, if > > > > > needed. > > > > > Currently, downstream, we have all those CSCs and LUTs hard-coded > > > > > into a header > > > > > file. Based on the colorspace, range, gamut selected for the output > > > > > and/or > > > > > plane input, we pick up the right CSCs and LUTs from that header file > > > > > to > > > > > configure our pipes... I guess this solution does the job, userspace > > > > > doesn't > > > > > need to care much about how to generate those tables. But, it's also > > > > > not very > > > > > flexible in case there is an app smart enough to know and actually > > > > > generate > > > > > their own custom tables. :/ > > > > > Looking through the dri-devel archives, I've seen that there was a > > > > > tentative to > > > > > implement a more or less generic per-plane LUT/CSC solution but it > > > > > didn't make > > > > > it in due to lack of userspace consumers... > > > > > > > > Apart from full color management mentioned by Pekka, are there other > > > > use-cases for these per-plane props? > > > > One thing I can think of is that some drivers annoyingly only apply the > > > > per-CRTC gamma LUT to the primary plane. I think it would make more > > > > sense to not attach a gamma prop to the CRTC and instead only attach it > > > > to the primary plane to make that clear. But I guess that would also > > > > break existing user-space? > > > > > > Uh, which drivers? This would be a driver bug (and there's been plenty of > > > those where the cursor has the wrong color temp and fun stuff like that). > > > We need to fix these driver issues instead of lamenting in userspace that > > > it's all kinda broken :-) > > > > Apparently this is bug with the old radeon driver [1]. It works fine on > > all i915 setups I've tried, and also works fine on amdgpu (with DC). > > > > I've opened a radeon bug. > > > > [1]: https://github.com/swaywm/wlroots/issues/968 > > This is a hardware limitation. I suspend all hardware of a certain > age had this same limitation. Old stuff didn't have multiple planes. That doesn't sound right to me. mach64 vt/gt and rage128 had an overlay plane already. I even vaguely remeber staring at some radeon overlay code at some point thinking "that stuff looks identical to the rage128 stuff, wonder why it's not shared code?". > It had a primary plane and a cursor and gamma didn't apply to the > cursor. That last part I can believe. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Changing vma->vm_file in dma_buf_mmap()
Am 14.09.20 um 15:29 schrieb Christian König: Hi Andrew, Sorry forgot to add Daniel as well. I'm the new DMA-buf maintainer and Daniel and others came up with patches extending the use of the dma_buf_mmap() function. Now this function is doing something a bit odd by changing the vma->vm_file while installing a VMA in the mmap() system call The background here is that DMA-buf allows device drivers to export buffer which are then imported into another device driver. The mmap() handler of the importing device driver then find that the pgoff belongs to the exporting device and so redirects the mmap() call there. In other words user space calls mmap() on one file descriptor, but get a different one mapped into your virtual address space. My question is now: Is that legal or can you think of something which breaks here? If it's not legal we should probably block any new users of the dma_buf_mmap() function and consider what should happen with the two existing ones. If that is legal I would like to document this by adding a new vma_set_file() function which does the necessary reference count dance. Thanks in advance, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Changing vma->vm_file in dma_buf_mmap()
Hi Andrew, I'm the new DMA-buf maintainer and Daniel and others came up with patches extending the use of the dma_buf_mmap() function. Now this function is doing something a bit odd by changing the vma->vm_file while installing a VMA in the mmap() system call The background here is that DMA-buf allows device drivers to export buffer which are then imported into another device driver. The mmap() handler of the importing device driver then find that the pgoff belongs to the exporting device and so redirects the mmap() call there. In other words user space calls mmap() on one file descriptor, but get a different one mapped into your virtual address space. My question is now: Is that legal or can you think of something which breaks here? If it's not legal we should probably block any new users of the dma_buf_mmap() function and consider what should happen with the two existing ones. If that is legal I would like to document this by adding a new vma_set_file() function which does the necessary reference count dance. Thanks in advance, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/shmem-helpers: revert "Redirect mmap for imported dma-buf"
This reverts commit 26d3ac3cb04d171a861952e89324e347598a347f. We need to figure out if dma_buf_mmap() is valid or not first. Signed-off-by: Christian König --- drivers/gpu/drm/drm_gem_shmem_helper.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 0a952f27c184..cd727343f72b 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -594,9 +594,6 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) /* Remove the fake offset */ vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); - if (obj->import_attach) - return dma_buf_mmap(obj->dma_buf, vma, 0); - shmem = to_drm_gem_shmem_obj(obj); ret = drm_gem_shmem_get_pages(shmem); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] mm: introduce vma_set_file function
Add the new vma_set_file() function to allow changing vma->vm_file with the necessary refcount dance. Signed-off-by: Christian König --- drivers/dma-buf/dma-buf.c | 16 +--- include/linux/mm.h| 2 ++ mm/mmap.c | 16 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 1699a8e309ef..672f3525ba74 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1163,20 +1163,14 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, return -EINVAL; /* readjust the vma */ - get_file(dmabuf->file); - oldfile = vma->vm_file; - vma->vm_file = dmabuf->file; + oldfile = vma_set_file(vma, dmabuf->file); vma->vm_pgoff = pgoff; ret = dmabuf->ops->mmap(dmabuf, vma); - if (ret) { - /* restore old parameters on failure */ - vma->vm_file = oldfile; - fput(dmabuf->file); - } else { - if (oldfile) - fput(oldfile); - } + /* restore old parameters on failure */ + if (ret) + vma_set_file(vma, oldfile); + return ret; } diff --git a/include/linux/mm.h b/include/linux/mm.h index 1983e08f5906..398a6fdaad1e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2688,6 +2688,8 @@ static inline void vma_set_page_prot(struct vm_area_struct *vma) } #endif +struct file *vma_set_file(struct vm_area_struct *vma, struct file *file); + #ifdef CONFIG_NUMA_BALANCING unsigned long change_prot_numa(struct vm_area_struct *vma, unsigned long start, unsigned long end); diff --git a/mm/mmap.c b/mm/mmap.c index 40248d84ad5f..d3c3c510f643 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -136,6 +136,22 @@ void vma_set_page_prot(struct vm_area_struct *vma) WRITE_ONCE(vma->vm_page_prot, vm_page_prot); } +/* + * Change backing file, only valid to use during initial VMA setup. + */ +struct file *vma_set_file(struct vm_area_struct *vma, struct file *file) +{ + if (file) + get_file(file); + + swap(vma->vm_file, file); + + if (file) + fput(file); + + return file; +} + /* * Requires inode->i_mapping->i_mmap_rwsem */ -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/tilcdc: Couple of minor feature improvements
On 14/09/2020 11:34, Jyri Sarha wrote: > The vblank interrupts have been always on when the display is on for a > very long time, so I decided that it is about time to fix it. Then the > following patch is just a cleanup. > > BR, > Jyri > > Jyri Sarha (2): > drm/tilcdc: Do not keep vblank interrupts enabled all the time > drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data > > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 52 ++-- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++-- > 3 files changed, 59 insertions(+), 38 deletions(-) > For both patches: Reviewed-by: Tomi Valkeinen 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
[PATCH 0/2] drm/tilcdc: Couple of minor feature improvements
The vblank interrupts have been always on when the display is on for a very long time, so I decided that it is about time to fix it. Then the following patch is just a cleanup. BR, Jyri Jyri Sarha (2): drm/tilcdc: Do not keep vblank interrupts enabled all the time drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 52 ++-- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++-- 3 files changed, 59 insertions(+), 38 deletions(-) -- 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
[PATCH 2/2] drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data
We already have a private data member for maximum display width so let's use it and get rid of the redundant tilcdc_crtc_max_width(). Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +--- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 38 +++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 7 ++--- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 29f263e1975a..27ea529d74b8 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -754,20 +754,6 @@ static const struct drm_crtc_funcs tilcdc_crtc_funcs = { .disable_vblank = tilcdc_crtc_disable_vblank, }; -int tilcdc_crtc_max_width(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct tilcdc_drm_private *priv = dev->dev_private; - int max_width = 0; - - if (priv->rev == 1) - max_width = 1024; - else if (priv->rev == 2) - max_width = 2048; - - return max_width; -} - static enum drm_mode_status tilcdc_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode) @@ -780,7 +766,7 @@ tilcdc_crtc_mode_valid(struct drm_crtc *crtc, * check to see if the width is within the range that * the LCD Controller physically supports */ - if (mode->hdisplay > tilcdc_crtc_max_width(crtc)) + if (mode->hdisplay > priv->max_width) return MODE_VIRTUAL_X; /* width must be multiple of 16 */ diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 4f5fc3e87383..866b33b40eff 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -105,7 +105,7 @@ static void modeset_init(struct drm_device *dev) dev->mode_config.min_width = 0; dev->mode_config.min_height = 0; - dev->mode_config.max_width = tilcdc_crtc_max_width(priv->crtc); + dev->mode_config.max_width = priv->max_width; dev->mode_config.max_height = 2048; dev->mode_config.funcs = &mode_config_funcs; } @@ -218,22 +218,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) goto init_failed; } - if (of_property_read_u32(node, "max-bandwidth", &priv->max_bandwidth)) - priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; - - DBG("Maximum Bandwidth Value %d", priv->max_bandwidth); - - if (of_property_read_u32(node, "max-width", &priv->max_width)) - priv->max_width = TILCDC_DEFAULT_MAX_WIDTH; - - DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width); - - if (of_property_read_u32(node, "max-pixelclock", - &priv->max_pixelclock)) - priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK; - - DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); - pm_runtime_enable(dev); /* Determine LCD IP Version */ @@ -287,6 +271,26 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) } } + if (of_property_read_u32(node, "max-bandwidth", &priv->max_bandwidth)) + priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH; + + DBG("Maximum Bandwidth Value %d", priv->max_bandwidth); + + if (of_property_read_u32(node, "max-width", &priv->max_width)) { + if (priv->rev == 1) + priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V1; + else + priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V2; + } + + DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width); + + if (of_property_read_u32(node, "max-pixelclock", + &priv->max_pixelclock)) + priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK; + + DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock); + ret = tilcdc_crtc_create(ddev); if (ret < 0) { dev_err(dev, "failed to create crtc\n"); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index 18815e75ca4f..76adf87fec4e 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -28,8 +28,10 @@ struct drm_plane; /* Defaulting to pixel clock defined on AM335x */ #define TILCDC_DEFAULT_MAX_PIXELCLOCK 126000 -/* Defaulting to max width as defined on AM335x */ -#define TILCDC_DEFAULT_MAX_WIDTH 2048 +/* Maximum width as defined on AM335x */ +#define TILCDC_DEFAULT_MAX_WIDTH_V2 2048 +/* ... and for V1 LCDC: */ +#define TILCDC_DEFAULT_MAX_WIDTH_V1 1024 /* * This may need some tweaking, but want to allow at least 1280x1024@60 * with optimized DDR & EMIF settings tweaked 1920x1080@24 appears to @@ -158,7 +160,6 @@ void tilcdc_crtc_set_panel_info(struct drm_c
[PATCH 1/2] drm/tilcdc: Do not keep vblank interrupts enabled all the time
END_OF_FRAME interrupts have been enabled all the time since the beginning of this driver. It is about time to add this feature. Signed-off-by: Jyri Sarha --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 1856962411c7..29f263e1975a 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -147,12 +147,9 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev) tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_SYNC_LOST_INT_ENA | LCDC_V1_FRAME_DONE_INT_ENA | LCDC_V1_UNDERFLOW_INT_ENA); - tilcdc_set(dev, LCDC_DMA_CTRL_REG, - LCDC_V1_END_OF_FRAME_INT_ENA); } else { tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_UNDERFLOW_INT_ENA | - LCDC_V2_END_OF_FRAME0_INT_ENA | LCDC_FRAME_DONE | LCDC_SYNC_LOST); } } @@ -678,11 +675,44 @@ static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc, static int tilcdc_crtc_enable_vblank(struct drm_crtc *crtc) { + struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); + struct drm_device *dev = crtc->dev; + struct tilcdc_drm_private *priv = dev->dev_private; + unsigned long flags; + + spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); + + tilcdc_clear_irqstatus(dev, LCDC_END_OF_FRAME0); + + if (priv->rev == 1) + tilcdc_set(dev, LCDC_DMA_CTRL_REG, + LCDC_V1_END_OF_FRAME_INT_ENA); + else + tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, + LCDC_V2_END_OF_FRAME0_INT_ENA); + + spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); + return 0; } static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc) { + struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); + struct drm_device *dev = crtc->dev; + struct tilcdc_drm_private *priv = dev->dev_private; + unsigned long flags; + + spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags); + + if (priv->rev == 1) + tilcdc_clear(dev, LCDC_DMA_CTRL_REG, +LCDC_V1_END_OF_FRAME_INT_ENA); + else + tilcdc_clear(dev, LCDC_INT_ENABLE_SET_REG, +LCDC_V2_END_OF_FRAME0_INT_ENA); + + spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags); } static void tilcdc_crtc_reset(struct drm_crtc *crtc) -- 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
Re: [PATCH v2 10/16] drm/exynos: implement a drm bridge
Hi, On 14.09.2020 10:29, Marek Szyprowski wrote: > On 11.09.2020 15:54, Michael Tretter wrote: >> Make the exynos_dsi driver a full drm bridge that can be found and used >> from other drivers. >> >> Other drivers can only attach to the bridge, if a mipi dsi device >> already attached to the bridge. This allows to defer the probe of the >> display pipe until the downstream bridges are available, too. >> >> Signed-off-by: Michael Tretter > This one (and the whole series applied) still fails on Exynos boards: > > [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations > exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) > OF: graph: no port node found in /soc/dsi@11c8 > 8<--- cut here --- > Unable to handle kernel NULL pointer dereference at virtual address 0084 > pgd = (ptrval) > [0084] *pgd= > Internal error: Oops: 5 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 1 PID: 1 Comm: swapper/0 Not tainted > 5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608 > Hardware name: Samsung Exynos (Flattened Device Tree) > PC is at drm_bridge_attach+0x18/0x164 > LR is at exynos_dsi_bind+0x88/0xa8 > pc : [] lr : [] psr: 2013 > sp : ef0dfca8 ip : 0002 fp : c13190e0 > r10: r9 : ee46d580 r8 : c13190e0 > r7 : ee438800 r6 : 0018 r5 : ef253810 r4 : ef39e840 > r3 : r2 : 0018 r1 : ef39e888 r0 : ef39e840 > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 4000404a DAC: 0051 > Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) > Stack: (0xef0dfca8 to 0xef0e) > ... > [] (drm_bridge_attach) from [] > (exynos_dsi_bind+0x88/0xa8) > [] (exynos_dsi_bind) from [] > (component_bind_all+0xfc/0x290) > [] (component_bind_all) from [] > (exynos_drm_bind+0xe4/0x19c) > [] (exynos_drm_bind) from [] > (try_to_bring_up_master+0x1e4/0x2c4) > [] (try_to_bring_up_master) from [] > (component_master_add_with_match+0xd4/0x108) > [] (component_master_add_with_match) from [] > (exynos_drm_platform_probe+0xe4/0x110) > [] (exynos_drm_platform_probe) from [] > (platform_drv_probe+0x6c/0xa4) > [] (platform_drv_probe) from [] > (really_probe+0x200/0x4fc) > [] (really_probe) from [] > (driver_probe_device+0x78/0x1fc) > [] (driver_probe_device) from [] > (device_driver_attach+0x58/0x60) > [] (device_driver_attach) from [] > (__driver_attach+0xdc/0x174) > [] (__driver_attach) from [] > (bus_for_each_dev+0x68/0xb4) > [] (bus_for_each_dev) from [] > (bus_add_driver+0x158/0x214) > [] (bus_add_driver) from [] (driver_register+0x78/0x110) > [] (driver_register) from [] > (exynos_drm_init+0xe4/0x118) > [] (exynos_drm_init) from [] > (do_one_initcall+0x8c/0x42c) > [] (do_one_initcall) from [] > (kernel_init_freeable+0x190/0x1dc) > [] (kernel_init_freeable) from [] > (kernel_init+0x8/0x118) > [] (kernel_init) from [] (ret_from_fork+0x14/0x20) > Exception stack(0xef0dffb0 to 0xef0dfff8) > ... > ---[ end trace ee27f313f9ed9da1 ]--- > > # arm-linux-gnueabi-addr2line -e vmlinux c0628c08 > drivers/gpu/drm/drm_bridge.c:184 (discriminator 1) > > I will try to debug it a bit more today. The above crash has been caused by lack of in_bridge initialization to NULL in exynos_dsi_bind() in this patch. However, fixing it reveals another issue: [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) OF: graph: no port node found in /soc/dsi@11c8 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 0280 pgd = (ptrval) [0280] *pgd= Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4-next-20200911-00010-g417dc70d70ec-dirty #1613 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at __mutex_lock+0x54/0xb18 LR is at lock_is_held_type+0x80/0x138 pc : [] lr : [] psr: 6013 sp : ef0dfd30 ip : 33937b74 fp : c13193c8 r10: c1208eec r9 : r8 : ee45f808 r7 : c19561a4 r6 : r5 : r4 : 024c r3 : r2 : 00204140 r1 : c124f13c r0 : Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000404a DAC: 0051 Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xef0dfd30 to 0xef0e) ... [] (__mutex_lock) from [] (mutex_lock_nested+0x1c/0x24) [] (mutex_lock_nested) from [] (__exynos_dsi_host_attach+0x20/0x6c) [] (__exynos_dsi_host_attach) from [] (exynos_dsi_host_attach+0x70/0x194) [] (exynos_dsi_host_attach) from [] (s6e8aa0_probe+0x1b0/0x218) [] (s6e8aa0_probe) from [] (really_probe+0x200/0x4fc) [] (really_probe) from [] (driver_probe_device+0x78/0x1fc) [] (driver_probe_device) from [] (device_driver_attach+0x58/0x60) [] (device_driver_attach) from [] (__driver_attach+0xdc/0x174) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0xb4) [] (bus_for_each_dev) from [] (bus_add_driver+0x158/0x214) [] (bus_add_driver)
Re: [PATCH] MAINTAINERS: make linux-aspeed list remarks consistent
On Sun, 13 Sep 2020 at 09:57, Lukas Bulwahn wrote: > > > > On Sat, 12 Sep 2020, Lukas Bulwahn wrote: > > > Commit f15a3ea80391 ("MAINTAINERS: Add ASPEED BMC GFX DRM driver entry") > > does not mention that linux-asp...@lists.ozlabs.org is moderated for > > non-subscribers, but the other three entries for > > linux-asp...@lists.ozlabs.org do. > > > > By 'majority vote' among entries, let us assume it was just missed here and > > adjust it to be consistent with others. > > > > Signed-off-by: Lukas Bulwahn > > --- > > applies cleanly on master and next-20200911 > > > > Joel, please ack. > > David, Daniel, please pick this minor non-urgent clean-up patch. > > > > This patch submission will also show me if linux-aspeed is moderated or > > not. I have not subscribed to linux-aspeed and if it shows up quickly in > > the archive, the list is probably not moderated; and if it takes longer, > > it is moderated, and hence, validating the patch. > > > > I did quickly get back an moderation email that my email is being held > back. So, that response validates my patch. The bmc related lists (openbmc@, linux-aspeed@, linux-fsi@) on ozlabs.org that I own have a soft-moderation policy. The first time you post a patch I add you to a whitelist. Given the low volume on these lists this works for me. I don't know if this necessitates marking the lists as moderated in MAINTINERS, but if you find that helpful then that's fine with me. Acked-by: Joel Stanley Cheers, Joel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] dma-buf: Flag vmap'ed memory as system or I/O memory
Dma-buf provides vmap() and vunmap() for retrieving and releasing mappings of dma-buf memory in kernel address space. The functions operate with plain addresses and the assumption is that the memory can be accessed with load and store operations. This is not the case on some architectures (e.g., sparc64) where I/O memory can only be accessed with dedicated instructions. This patchset introduces struct dma_buf_map, which contains the address of a buffer and a flag that tells whether system- or I/O-memory instructions are required. Some background: updating the DRM framebuffer console on sparc64 makes the kernel panic. This is because the framebuffer memory cannot be accessed with system-memory instructions. We currently employ a workaround in DRM to address this specific problem. [1] To resolve the problem, we'd like to address it at the most common point, which is the dma-buf framework. The dma-buf mapping ideally knows if I/O instructions are required and exports this information to it's users. The new structure struct dma_buf_map stores the buffer address and a flag that signals I/O memory. Affected users of the buffer (e.g., drivers, frameworks) can then access the memory accordingly. This patchset only introduces struct dma_buf_map, and updates struct dma_buf and it's interfaces. Further patches can update dma-buf users. For example, there's a prototype patchset for DRM that fixes the framebuffer problem. [2] Further work: TTM, one of DRM's memory managers, already exports an is_iomem flag of its own. It could later be switched over to exporting struct dma_buf_map, thus simplifying some code. Several DRM drivers expect their fbdev console to operate on I/O memory. These could possibly be switched over to the generic fbdev emulation, as soon as the generic code uses struct dma_buf_map. [1] https://lore.kernel.org/dri-devel/20200725191012.ga434...@ravnborg.org/ [2] https://lore.kernel.org/dri-devel/20200806085239.4606-1-tzimmerm...@suse.de/ Thomas Zimmermann (3): dma-buf: Add struct dma-buf-map for storing struct dma_buf.vaddr_ptr dma-buf: Use struct dma_buf_map in dma_buf_vmap() interfaces dma-buf: Use struct dma_buf_map in dma_buf_vunmap() interfaces Documentation/driver-api/dma-buf.rst | 3 + drivers/dma-buf/dma-buf.c | 40 +++--- drivers/gpu/drm/drm_gem_cma_helper.c | 16 ++- drivers/gpu/drm/drm_gem_shmem_helper.c| 17 ++- drivers/gpu/drm/drm_prime.c | 14 +- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 13 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 18 ++- drivers/gpu/drm/tegra/gem.c | 23 ++-- .../common/videobuf2/videobuf2-dma-contig.c | 17 ++- .../media/common/videobuf2/videobuf2-dma-sg.c | 19 ++- .../common/videobuf2/videobuf2-vmalloc.c | 21 ++- include/drm/drm_prime.h | 5 +- include/linux/dma-buf-map.h | 126 ++ include/linux/dma-buf.h | 11 +- 15 files changed, 274 insertions(+), 82 deletions(-) create mode 100644 include/linux/dma-buf-map.h -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] dma-buf: Add struct dma-buf-map for storing struct dma_buf.vaddr_ptr
The new type struct dma_buf_map represents a mapping of dma-buf memory into kernel space. It contains a flag, is_iomem, that signals users to access the mapped memory with I/O operations instead of regular loads and stores. It was assumed that DMA buffer memory can be accessed with regular load and store operations. Some architectures, such as sparc64, require the use of I/O operations to access dma-map buffers that are located in I/O memory. Providing struct dma_buf_map allows drivers to implement this. This was specifically a problem when refreshing the grahpics framebuffer on such systems. [Link 1] As the first step, struct dma_buf stores an instance of struct dma_buf_map internally. Afterwards, dma-buf's vmap and vunmap interfaces are be converted. Finally, affected drivers can be fixed. Signed-off-by: Thomas Zimmermann Link: https://lore.kernel.org/dri-devel/20200725191012.ga434...@ravnborg.org/ --- Documentation/driver-api/dma-buf.rst | 3 + drivers/dma-buf/dma-buf.c| 14 ++--- include/linux/dma-buf-map.h | 87 include/linux/dma-buf.h | 3 +- 4 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 include/linux/dma-buf-map.h diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index 13ea0cc0a3fa..3244c600a9a1 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -115,6 +115,9 @@ Kernel Functions and Structures Reference .. kernel-doc:: include/linux/dma-buf.h :internal: +.. kernel-doc:: include/linux/dma-buf-map.h + :internal: + Reservation Objects --- diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 58564d82a3a2..5e849ca241a0 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1207,12 +1207,12 @@ void *dma_buf_vmap(struct dma_buf *dmabuf) mutex_lock(&dmabuf->lock); if (dmabuf->vmapping_counter) { dmabuf->vmapping_counter++; - BUG_ON(!dmabuf->vmap_ptr); - ptr = dmabuf->vmap_ptr; + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); + ptr = dmabuf->vmap_ptr.vaddr; goto out_unlock; } - BUG_ON(dmabuf->vmap_ptr); + BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr)); ptr = dmabuf->ops->vmap(dmabuf); if (WARN_ON_ONCE(IS_ERR(ptr))) @@ -1220,7 +1220,7 @@ void *dma_buf_vmap(struct dma_buf *dmabuf) if (!ptr) goto out_unlock; - dmabuf->vmap_ptr = ptr; + dmabuf->vmap_ptr.vaddr = ptr; dmabuf->vmapping_counter = 1; out_unlock: @@ -1239,15 +1239,15 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) if (WARN_ON(!dmabuf)) return; - BUG_ON(!dmabuf->vmap_ptr); + BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); BUG_ON(dmabuf->vmapping_counter == 0); - BUG_ON(dmabuf->vmap_ptr != vaddr); + BUG_ON(!dma_buf_map_is_vaddr(&dmabuf->vmap_ptr, vaddr)); mutex_lock(&dmabuf->lock); if (--dmabuf->vmapping_counter == 0) { if (dmabuf->ops->vunmap) dmabuf->ops->vunmap(dmabuf, vaddr); - dmabuf->vmap_ptr = NULL; + dma_buf_map_clear(&dmabuf->vmap_ptr); } mutex_unlock(&dmabuf->lock); } diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h new file mode 100644 index ..d4b1bb3cc4b0 --- /dev/null +++ b/include/linux/dma-buf-map.h @@ -0,0 +1,87 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Pointer to dma-buf-mapped memory, plus helpers. + */ + +#ifndef __DMA_BUF_MAP_H__ +#define __DMA_BUF_MAP_H__ + +#include + +/** + * struct dma_buf_map - Pointer to vmap'ed dma-buf memory. + * @vaddr_iomem: The buffer's address if in I/O memory + * @vaddr: The buffer's address if in system memory + * @is_iomem: True if the dma-buf memory is located in I/O + * memory, or false otherwise. + * + * Calling dma-buf's vmap operation returns a pointer to the buffer. + * Depending on the location of the buffer, users may have to access it + * with I/O operations or memory load/store operations. struct dma_buf_map + * stores the buffer address and a flag that signals the required access. + */ +struct dma_buf_map { + union { + void __iomem *vaddr_iomem; + void *vaddr; + }; + bool is_iomem; +}; + +/* API transition helper */ +static inline bool dma_buf_map_is_vaddr(const struct dma_buf_map *map, const void *vaddr) +{ + return !map->is_iomem && (map->vaddr == vaddr); +} + +/** + * dma_buf_map_is_null - Tests for a dma-buf mapping to be NULL + * @map: The dma-buf mapping structure + * + * Depending on the state of struct dma_buf_map.is_iomem, tests if the + * mapping is NULL. + * + * Returns: + * True if the mapping is NULL, or false otherwise. +
[PATCH 2/3] dma-buf: Use struct dma_buf_map in dma_buf_vmap() interfaces
This patch updates dma_buf_vmap() and dma-buf's vmap callback to use struct dma_buf_map. The interfaces used to return a buffer address. This address now gets stored in an instance of the structure that is given as an additional argument. The functions return an errno code on errors. Users of the functions are updated accordingly. This is only an interface change. It is currently expected that dma-buf memory can be accessed with system memory load/store operations. Signed-off-by: Thomas Zimmermann --- drivers/dma-buf/dma-buf.c | 26 ++- drivers/gpu/drm/drm_gem_cma_helper.c | 13 +- drivers/gpu/drm/drm_gem_shmem_helper.c| 14 ++ drivers/gpu/drm/drm_prime.c | 8 +++--- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 11 ++-- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 12 ++--- drivers/gpu/drm/tegra/gem.c | 18 - .../common/videobuf2/videobuf2-dma-contig.c | 14 +++--- .../media/common/videobuf2/videobuf2-dma-sg.c | 16 .../common/videobuf2/videobuf2-vmalloc.c | 15 --- include/drm/drm_prime.h | 3 ++- include/linux/dma-buf-map.h | 13 ++ include/linux/dma-buf.h | 6 ++--- 14 files changed, 122 insertions(+), 55 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 5e849ca241a0..c99e3577aa2f 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1186,46 +1186,48 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap); * dma_buf_vmap - Create virtual mapping for the buffer object into kernel * address space. Same restrictions as for vmap and friends apply. * @dmabuf:[in]buffer to vmap + * @map: [out] returns the vmap pointer * * This call may fail due to lack of virtual mapping address space. * These calls are optional in drivers. The intended use for them * is for mapping objects linear in kernel space for high use objects. * Please attempt to use kmap/kunmap before thinking about these interfaces. * - * Returns NULL on error. + * Returns 0 on success, or a negative errno code otherwise. */ -void *dma_buf_vmap(struct dma_buf *dmabuf) +int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map) { - void *ptr; + struct dma_buf_map ptr; + int ret = 0; if (WARN_ON(!dmabuf)) - return NULL; + return -EINVAL; if (!dmabuf->ops->vmap) - return NULL; + return -EINVAL; mutex_lock(&dmabuf->lock); if (dmabuf->vmapping_counter) { dmabuf->vmapping_counter++; BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); - ptr = dmabuf->vmap_ptr.vaddr; + *map = dmabuf->vmap_ptr; goto out_unlock; } BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr)); - ptr = dmabuf->ops->vmap(dmabuf); - if (WARN_ON_ONCE(IS_ERR(ptr))) - ptr = NULL; - if (!ptr) + ret = dmabuf->ops->vmap(dmabuf, &ptr); + if (WARN_ON_ONCE(ret)) goto out_unlock; - dmabuf->vmap_ptr.vaddr = ptr; + dmabuf->vmap_ptr = ptr; dmabuf->vmapping_counter = 1; + *map = dmabuf->vmap_ptr; + out_unlock: mutex_unlock(&dmabuf->lock); - return ptr; + return ret; } EXPORT_SYMBOL_GPL(dma_buf_vmap); diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 822edeadbab3..062315c25c12 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -634,22 +634,23 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev, { struct drm_gem_cma_object *cma_obj; struct drm_gem_object *obj; - void *vaddr; + struct dma_buf_map map; + int ret; - vaddr = dma_buf_vmap(attach->dmabuf); - if (!vaddr) { + ret = dma_buf_vmap(attach->dmabuf, &map); + if (ret) { DRM_ERROR("Failed to vmap PRIME buffer\n"); - return ERR_PTR(-ENOMEM); + return ERR_PTR(ret); } obj = drm_gem_cma_prime_import_sg_table(dev, attach, sgt); if (IS_ERR(obj)) { - dma_buf_vunmap(attach->dmabuf, vaddr); + dma_buf_vunmap(attach->dmabuf, map.vaddr); return obj; } cma_obj = to_drm_gem_cma_obj(obj); - cma_obj->vaddr = vaddr; + cma_obj->vaddr = map.vaddr; return obj; } diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 0a952f27c184..ad10a57cfece 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -261,13 +261,16 @@ EXPORT_SYMBOL(drm_gem_shmem_unpin); static void *drm_gem_shmem_vmap_locked(
[PATCH 3/3] dma-buf: Use struct dma_buf_map in dma_buf_vunmap() interfaces
This patch updates dma_buf_vunmap() and dma-buf's vunmap callback to use struct dma_buf_map. The interfaces used to receive a buffer address. This address is now given in an instance of the structure. Users of the functions are updated accordingly. This is only an interface change. It is currently expected that dma-buf memory can be accessed with system memory load/store operations. Signed-off-by: Thomas Zimmermann --- drivers/dma-buf/dma-buf.c | 8 ++--- drivers/gpu/drm/drm_gem_cma_helper.c | 5 +-- drivers/gpu/drm/drm_gem_shmem_helper.c| 3 +- drivers/gpu/drm/drm_prime.c | 6 ++-- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 5 +-- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c| 2 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 6 ++-- drivers/gpu/drm/tegra/gem.c | 5 +-- .../common/videobuf2/videobuf2-dma-contig.c | 3 +- .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- .../common/videobuf2/videobuf2-vmalloc.c | 6 ++-- include/drm/drm_prime.h | 2 +- include/linux/dma-buf-map.h | 32 +-- include/linux/dma-buf.h | 4 +-- 14 files changed, 62 insertions(+), 28 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index c99e3577aa2f..90022003b065 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1234,21 +1234,21 @@ EXPORT_SYMBOL_GPL(dma_buf_vmap); /** * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap. * @dmabuf:[in]buffer to vunmap - * @vaddr: [in]vmap to vunmap + * @map: [in]vmap pointer to vunmap */ -void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) +void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map) { if (WARN_ON(!dmabuf)) return; BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr)); BUG_ON(dmabuf->vmapping_counter == 0); - BUG_ON(!dma_buf_map_is_vaddr(&dmabuf->vmap_ptr, vaddr)); + BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map)); mutex_lock(&dmabuf->lock); if (--dmabuf->vmapping_counter == 0) { if (dmabuf->ops->vunmap) - dmabuf->ops->vunmap(dmabuf, vaddr); + dmabuf->ops->vunmap(dmabuf, map); dma_buf_map_clear(&dmabuf->vmap_ptr); } mutex_unlock(&dmabuf->lock); diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 062315c25c12..55f500c85fe6 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -176,12 +176,13 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv, void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) { struct drm_gem_cma_object *cma_obj; + struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(cma_obj->vaddr); cma_obj = to_drm_gem_cma_obj(gem_obj); if (gem_obj->import_attach) { if (cma_obj->vaddr) - dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); + dma_buf_vunmap(gem_obj->import_attach->dmabuf, &map); drm_prime_gem_destroy(gem_obj, cma_obj->sgt); } else if (cma_obj->vaddr) { dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, @@ -645,7 +646,7 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev, obj = drm_gem_cma_prime_import_sg_table(dev, attach, sgt); if (IS_ERR(obj)) { - dma_buf_vunmap(attach->dmabuf, map.vaddr); + dma_buf_vunmap(attach->dmabuf, &map); return obj; } diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index ad10a57cfece..3c2e8cde69a8 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -337,6 +337,7 @@ EXPORT_SYMBOL(drm_gem_shmem_vmap); static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem) { struct drm_gem_object *obj = &shmem->base; + struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(shmem->vaddr); if (WARN_ON_ONCE(!shmem->vmap_use_count)) return; @@ -345,7 +346,7 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem) return; if (obj->import_attach) - dma_buf_vunmap(obj->import_attach->dmabuf, shmem->vaddr); + dma_buf_vunmap(obj->import_attach->dmabuf, &map); else vunmap(shmem->vaddr); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 1b7d86c7842d..8a02b739924f 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -686,16 +686,16 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap); /** * drm_gem_dmabuf_vunmap - dma_buf vunmap implementation for GEM * @dma_buf: buffer to be unmapped - * @vaddr: t
Re: [PATCH 0/4] Add support for video pattern DP CTS to MSM DP
On Thu, 03 Sep 2020, Abhinav Kumar wrote: > Add support for video pattern Display Port Compliance tests to > MSM DP driver. FWIW, I find it useful to prefix the cover letter subject line with the usual git log prefixes, e.g. "drm/msm/dp", so people can filter their mails better. BR, Jani. > Abhinav Kumar (4): > drm/msm/dp: add debugfs support to DP driver > drm/msm/dp: move debugfs node to /sys/kernel/debug/dri/*/ > drm/msm/dp: add debugfs nodes for video pattern tests > drm/msm/dp: remove mode hard-coding in case of DP CTS > > drivers/gpu/drm/msm/Makefile| 3 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 7 + > drivers/gpu/drm/msm/dp/dp_debug.c | 485 > drivers/gpu/drm/msm/dp/dp_debug.h | 74 > drivers/gpu/drm/msm/dp/dp_display.c | 28 +- > drivers/gpu/drm/msm/dp/dp_link.c| 2 +- > drivers/gpu/drm/msm/dp/dp_link.h| 23 ++ > drivers/gpu/drm/msm/dp/dp_panel.c | 46 +-- > drivers/gpu/drm/msm/msm_drv.h | 2 + > 9 files changed, 617 insertions(+), 53 deletions(-) > create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.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
Re: [PATCH v5 00/80] drm/vc4: Support BCM2711 Display Pipeline
Hi Maxime, On 9/8/20 9:00 PM, Maxime Ripard wrote: > Hi Hoegeun, > > On Mon, Sep 07, 2020 at 08:49:12PM +0900, Hoegeun Kwon wrote: >> On 9/3/20 5:00 PM, Maxime Ripard wrote: >>> Hi everyone, >>> >>> Here's a (pretty long) series to introduce support in the VC4 DRM driver >>> for the display pipeline found in the BCM2711 (and thus the RaspberryPi 4). >>> >>> The main differences are that there's two HDMI controllers and that there's >>> more pixelvalve now. Those pixelvalve come with a mux in the HVS that still >>> have only 3 FIFOs. Both of those differences are breaking a bunch of >>> expectations in the driver, so we first need a good bunch of cleanup and >>> reworks to introduce support for the new controllers. >>> >>> Similarly, the HDMI controller has all its registers shuffled and split in >>> multiple controllers now, so we need a bunch of changes to support this as >>> well. >>> >>> Only the HDMI support is enabled for now (even though the DPI and DSI >>> outputs have been tested too). >>> >>> Let me know if you have any comments >>> Maxime >>> >>> Cc: bcm-kernel-feedback-l...@broadcom.com >>> Cc: devicet...@vger.kernel.org >>> Cc: Kamal Dasu >>> Cc: Philipp Zabel >>> Cc: Rob Herring >>> Cc: Stephen Boyd >>> >>> Changes from v4: >>> - Rebased on top of next-20200828 >>> - Collected the various tags >>> - Fixed some issues with 4k support and dual output (thanks Hoegeun!) >> Thanks for your v5 patchset. >> >> I tested all patches based on the next-20200812. > Thanks again for testing all the patches > >> Everything else is fine, but the dual hdmi modetest doesn't work well in my >> environment... >> >> In my environment, dsi is not connected, I have seen your answer[1]. > Can you share a bit more your setup? What monitors are being connected > to each HDMI port? Do you hotplug any? Yes, Monitors are being connected to each HDMI ports. (did not use hotplug) When booting, both HDMI-0 and 1 are recognized and the kernel log is output. But after run modetest on HDMI-0(works) and modetest on HDMI-1(works), crtc timed out occurs on HDMI-0 and does not work. When HDMI-0 is not working we do a modetest on HDMI-0, it will work agin after about 40 sec. Below is the log for modetest. root:~> modetest -Mvc4 -s 32:1280x720 - HDMI-0 works setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64 failed to set gamma: Invalid argument root:~> modetest -Mvc4 -s 32:1280x720 - HDMI-0 works setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64 failed to set gamma: Invalid argument root:~> modetest -Mvc4 -s 38:1280x720 - HDMI-1 works setting mode 1280x720-60Hz@XR24 on connectors 38, crtc 69 failed to set gamma: Invalid argument - Crtc timed out occurs on HDMI-0 and does not work. [ 71.134283] [drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:64:crtc-3] flip_done timed out [ 81.374296] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:64:crtc-3] flip_done timed out [ 91.618380] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:32:HDMI-A-1] flip_done timed out [ 101.854274] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:60:plane-3] flip_done timed out [ 112.094271] [drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:64:crtc-3] flip_done timed out [ 122.590311] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:64:crtc-3] flip_done timed out root:~> modetest -Mvc4 -s 32:1280x720 [ 132.830309] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:32:HDMI-A-1] flip_done timed out [ 143.070307] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:60:plane-3] flip_done timed out [ 153.310303] [drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:64:crtc-3] flip_done timed out setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64 [ 163.550340] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:64:crtc-3] flip_done timed out [ 173.790277] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:32:HDMI-A-1] flip_done timed out [ 184.030286] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:60:plane-3] flip_done timed out failed to set gamma: Invalid argument - HDMI-0 works Best regards, Hoegeun ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 10/16] drm/exynos: implement a drm bridge
Hi Michael, On 11.09.2020 15:54, Michael Tretter wrote: > Make the exynos_dsi driver a full drm bridge that can be found and used > from other drivers. > > Other drivers can only attach to the bridge, if a mipi dsi device > already attached to the bridge. This allows to defer the probe of the > display pipe until the downstream bridges are available, too. > > Signed-off-by: Michael Tretter This one (and the whole series applied) still fails on Exynos boards: [drm] Exynos DRM: using 11c0.fimd device for DMA mapping operations exynos-drm exynos-drm: bound 11c0.fimd (ops fimd_component_ops) OF: graph: no port node found in /soc/dsi@11c8 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 0084 pgd = (ptrval) [0084] *pgd= Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at drm_bridge_attach+0x18/0x164 LR is at exynos_dsi_bind+0x88/0xa8 pc : [] lr : [] psr: 2013 sp : ef0dfca8 ip : 0002 fp : c13190e0 r10: r9 : ee46d580 r8 : c13190e0 r7 : ee438800 r6 : 0018 r5 : ef253810 r4 : ef39e840 r3 : r2 : 0018 r1 : ef39e888 r0 : ef39e840 Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000404a DAC: 0051 Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xef0dfca8 to 0xef0e) ... [] (drm_bridge_attach) from [] (exynos_dsi_bind+0x88/0xa8) [] (exynos_dsi_bind) from [] (component_bind_all+0xfc/0x290) [] (component_bind_all) from [] (exynos_drm_bind+0xe4/0x19c) [] (exynos_drm_bind) from [] (try_to_bring_up_master+0x1e4/0x2c4) [] (try_to_bring_up_master) from [] (component_master_add_with_match+0xd4/0x108) [] (component_master_add_with_match) from [] (exynos_drm_platform_probe+0xe4/0x110) [] (exynos_drm_platform_probe) from [] (platform_drv_probe+0x6c/0xa4) [] (platform_drv_probe) from [] (really_probe+0x200/0x4fc) [] (really_probe) from [] (driver_probe_device+0x78/0x1fc) [] (driver_probe_device) from [] (device_driver_attach+0x58/0x60) [] (device_driver_attach) from [] (__driver_attach+0xdc/0x174) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0xb4) [] (bus_for_each_dev) from [] (bus_add_driver+0x158/0x214) [] (bus_add_driver) from [] (driver_register+0x78/0x110) [] (driver_register) from [] (exynos_drm_init+0xe4/0x118) [] (exynos_drm_init) from [] (do_one_initcall+0x8c/0x42c) [] (do_one_initcall) from [] (kernel_init_freeable+0x190/0x1dc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x118) [] (kernel_init) from [] (ret_from_fork+0x14/0x20) Exception stack(0xef0dffb0 to 0xef0dfff8) ... ---[ end trace ee27f313f9ed9da1 ]--- # arm-linux-gnueabi-addr2line -e vmlinux c0628c08 drivers/gpu/drm/drm_bridge.c:184 (discriminator 1) I will try to debug it a bit more today. > --- > v2: > - move attach of out_bridge to bridge_attach > - add bridge_detach > - don't cleanup encoder if create_connector failed > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 103 +--- > 1 file changed, 75 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index 2d75f9877dc0..5e7c1a99a3ee 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -266,6 +266,7 @@ struct exynos_dsi_driver_data { > > struct exynos_dsi { > struct drm_encoder encoder; > + struct drm_bridge bridge; > struct mipi_dsi_host dsi_host; > struct drm_connector connector; > struct drm_panel *panel; > @@ -1602,6 +1603,60 @@ static const struct drm_encoder_helper_funcs > exynos_dsi_encoder_helper_funcs = { > .disable = exynos_dsi_disable, > }; > > +static int exynos_dsi_bridge_attach(struct drm_bridge *bridge, > + enum drm_bridge_attach_flags flags) > +{ > + struct exynos_dsi *dsi = bridge->driver_private; > + struct drm_encoder *encoder = bridge->encoder; > + int ret; > + > + if (!dsi->out_bridge && !dsi->panel) > + return -EPROBE_DEFER; > + > + if (dsi->out_bridge) { > + ret = drm_bridge_attach(encoder, dsi->out_bridge, > + bridge, flags); > + if (ret) > + return ret; > + list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain); > + } else { > + ret = exynos_dsi_create_connector(encoder); > + if (ret) > + return ret; > + > + if (dsi->panel) { > + dsi->connector.status = connector_status_connected; > + } > + } > + > + return 0; > +} > + > +static void exynos_dsi_bridge_detach(struct drm_bridge *bridge) > +{ > + struct exynos_dsi *dsi = bridge->driver_private; > +
Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
On 2020-09-07 9:57 a.m., Daniel Vetter wrote: On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote: From: Michel Dänzer Don't check drm_crtc_state::active for this either, per its documentation in include/drm/drm_crtc.h: * Hence drivers must not consult @active in their various * &drm_mode_config_funcs.atomic_check callback to reject an atomic * commit. atomic_remove_fb disables the CRTC as needed for disabling the primary plane. This prevents at least the following problems if the primary plane gets disabled (e.g. due to destroying the FB assigned to the primary plane, as happens e.g. with mutter in Wayland mode): * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID (which enables the cursor plane). * If the cursor plane was enabled, changing the legacy DPMS property value from off to on returned EINVAL. v2: * Minor changes to code comment and commit log, per review feedback. GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 Suggested-by: Daniel Vetter Signed-off-by: Michel Dänzer Commit message agrees with my understand of this maze now, so: Acked-by: Daniel Vetter Thanks Daniel! Nick / Harry, any more feedback? If not, can you apply this? P.S. Since DCN doesn't make a distinction between primary or overlay planes in hardware, could ChromiumOS achieve the same effect with only the primary plane instead of only an overlay plane? If so, maybe there's no need for the "black plane hack". -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/4] drm/ast: Set format registers in primary plane's update
The atomic modesetting code tried to distinguish format changes from full modesetting operations. But the implementation was buggy and the format registers were often updated even for simple pageflips. Fix this problem by handling format changes in the primary plane's update function. v3: * program format in primary plane's update function Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_mode.c | 44 +- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 62fe682a7de6..2323fe0b71bd 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -562,13 +562,24 @@ ast_primary_plane_helper_atomic_update(struct drm_plane *plane, struct drm_plane_state *state = plane->state; struct drm_gem_vram_object *gbo; s64 gpu_addr; + struct drm_framebuffer *fb = state->fb; + struct drm_framebuffer *old_fb = old_state->fb; + + if (!old_fb || (fb->format != old_fb->format)) { + struct drm_crtc_state *crtc_state = state->crtc->state; + struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state); + struct ast_vbios_mode_info *vbios_mode_info = &ast_crtc_state->vbios_mode_info; + + ast_set_color_reg(ast, fb->format); + ast_set_vbios_color_reg(ast, fb->format, vbios_mode_info); + } - gbo = drm_gem_vram_of_gem(state->fb->obj[0]); + gbo = drm_gem_vram_of_gem(fb->obj[0]); gpu_addr = drm_gem_vram_offset(gbo); if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */ - ast_set_offset_reg(ast, state->fb); + ast_set_offset_reg(ast, fb); ast_set_start_address_crt1(ast, (u32)gpu_addr); ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x00); @@ -733,6 +744,7 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode) static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) { + struct drm_device *dev = crtc->dev; struct ast_crtc_state *ast_state; const struct drm_format_info *format; bool succ; @@ -743,8 +755,8 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc, ast_state = to_ast_crtc_state(state); format = ast_state->format; - if (!format) - return 0; + if (drm_WARN_ON_ONCE(dev, !format)) + return -EINVAL; /* BUG: We didn't set format in primary check(). */ succ = ast_get_vbios_mode_info(format, &state->mode, &state->adjusted_mode, @@ -768,27 +780,15 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct ast_private *ast = to_ast_private(dev); - struct ast_crtc_state *ast_state; - const struct drm_format_info *format; - struct ast_vbios_mode_info *vbios_mode_info; - struct drm_display_mode *adjusted_mode; + struct drm_crtc_state *crtc_state = crtc->state; + struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state); + struct ast_vbios_mode_info *vbios_mode_info = + &ast_crtc_state->vbios_mode_info; + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; - ast_state = to_ast_crtc_state(crtc->state); - - format = ast_state->format; - if (!format) - return; - - vbios_mode_info = &ast_state->vbios_mode_info; - - ast_set_color_reg(ast, format); - ast_set_vbios_color_reg(ast, format, vbios_mode_info); - - if (!crtc->state->mode_changed) + if (!drm_atomic_crtc_needs_modeset(crtc_state)) return; - adjusted_mode = &crtc->state->adjusted_mode; - ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info); ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06); ast_set_std_reg(ast, adjusted_mode, vbios_mode_info); -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND][PATCH v3 0/4] drm/ast: Disable HW cursor when switching modes
Since converting the ast driver to atomic modesetting, modesetting occationally locks up the graphics hardware and turns the display permanently dark. This happens once or twice per 10 mode switches. Investigation shows that the ast hardware presumably requires the HW cursor to be disabled while the modeswitch takes place. This patchset fixes the problem by disabling planes before programming the CRTC mode. After the switch, the planes gets re-enabled if they were enabled before. For mere pageflip operations, nothing changes. Patches #1 makes format changes work as intended: format registers are only updated if necessary. They used to be changed on each pageflip. Patches #2 to #4 change the modesetting logic such that planes will be disabled with the CRTC, then the CRTC's new mode is being programmed, and finally planes are reenabled. The primary plane is enabled before the cursor plane. With this setup, the cursor plane always has a valid mode and framebuffer available. Tested on AST2100 HW. The issue is not 100% reproducible, but does not show up after applying the patchset. I think the problem has been fixed. Thomas Zimmermann (4): drm/ast: Set format registers in primary plane's update drm/ast: Disable planes while switching display modes drm/ast: Program display mode in CRTC's atomic_enable() drm/ast: Enable CRTC before planes drivers/gpu/drm/ast/ast_drv.h | 2 + drivers/gpu/drm/ast/ast_mode.c | 103 - 2 files changed, 64 insertions(+), 41 deletions(-) -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] drm/ast: Disable planes while switching display modes
The ast HW cursor requires the primary plane and CRTC to display at a valid mode and format. This is not the case while switching display modes, which can lead to the screen turing permanently dark. As a workaround, the ast driver now disables active planes while the mode or format switch takes place. It also synchronizes with the vertical refresh to give CRTC and planes some time to catch up on each other. The active planes planes (primary or cursor) will be re-enabled by each plane's atomic_update() function. v3: * move the logic into the CRTC's atomic_disable function v2: * move the logic into the commit-tail function Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_drv.h | 2 ++ drivers/gpu/drm/ast/ast_mode.c | 31 +++ 2 files changed, 33 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index c1af6b725933..467049ca8430 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -177,6 +177,8 @@ struct ast_private *ast_device_create(struct drm_driver *drv, #define AST_IO_MM_OFFSET (0x380) +#define AST_IO_VGAIR1_VREFRESH BIT(3) + #define __ast_read(x) \ static inline u##x ast_read##x(struct ast_private *ast, u32 reg) { \ u##x val = 0;\ diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 2323fe0b71bd..5b45b57c3d17 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -514,6 +514,16 @@ static void ast_set_start_address_crt1(struct ast_private *ast, } +static void ast_wait_for_vretrace(struct ast_private *ast) +{ + unsigned long timeout = jiffies + HZ; + u8 vgair1; + + do { + vgair1 = ast_io_read8(ast, AST_IO_INPUT_STATUS1_READ); + } while (!(vgair1 & AST_IO_VGAIR1_VREFRESH) && time_before(jiffies, timeout)); +} + /* * Primary plane */ @@ -809,7 +819,28 @@ static void ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { + struct drm_device *dev = crtc->dev; + struct ast_private *ast = to_ast_private(dev); + ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); + + /* +* HW cursors require the underlying primary plane and CRTC to +* display a valid mode and image. This is not the case during +* full modeset operations. So we temporarily disable any active +* plane, including the HW cursor. Each plane's atomic_update() +* helper will re-enable it if necessary. +* +* We only do this during *full* modesets. It does not affect +* simple pageflips on the planes. +*/ + drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false); + + /* +* Ensure that no scanout takes place before reprogramming mode +* and format registers. +*/ + ast_wait_for_vretrace(ast); } static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = { -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/4] drm/ast: Program display mode in CRTC's atomic_enable()
This change simplifies ast's modesetting code. The display mode is now programmed from within the CRTC's atomic_enable(), which only runs if we actually want to program the mode. Corresponding code in atomic_flush() is being removed. Also removed is atomic_begin(), which serves no purpose at all. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_mode.c | 23 +++ 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 5b45b57c3d17..fd0127f48fb9 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -777,16 +777,9 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc, return 0; } -static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc, -struct drm_crtc_state *old_crtc_state) -{ - struct ast_private *ast = to_ast_private(crtc->dev); - - ast_open_key(ast); -} - -static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, -struct drm_crtc_state *old_crtc_state) +static void +ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, + struct drm_crtc_state *old_crtc_state) { struct drm_device *dev = crtc->dev; struct ast_private *ast = to_ast_private(dev); @@ -796,9 +789,6 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, &ast_crtc_state->vbios_mode_info; struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; - if (!drm_atomic_crtc_needs_modeset(crtc_state)) - return; - ast_set_vbios_mode_reg(ast, adjusted_mode, vbios_mode_info); ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06); ast_set_std_reg(ast, adjusted_mode, vbios_mode_info); @@ -806,12 +796,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, ast_set_dclk_reg(ast, adjusted_mode, vbios_mode_info); ast_set_crtthd_reg(ast); ast_set_sync_reg(ast, adjusted_mode, vbios_mode_info); -} -static void -ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, - struct drm_crtc_state *old_crtc_state) -{ ast_crtc_dpms(crtc, DRM_MODE_DPMS_ON); } @@ -845,8 +830,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = { .atomic_check = ast_crtc_helper_atomic_check, - .atomic_begin = ast_crtc_helper_atomic_begin, - .atomic_flush = ast_crtc_helper_atomic_flush, .atomic_enable = ast_crtc_helper_atomic_enable, .atomic_disable = ast_crtc_helper_atomic_disable, }; -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/4] drm/ast: Enable CRTC before planes
An active cursor plane requires a valid display mode. Change the commit_tail callback, so that it sets up the CRTC's mode before updating planes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_mode.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index fd0127f48fb9..834a156e3a75 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1068,6 +1068,11 @@ static int ast_connector_init(struct drm_device *dev) * Mode config */ +static const struct drm_mode_config_helper_funcs +ast_mode_config_helper_funcs = { + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, +}; + static const struct drm_mode_config_funcs ast_mode_config_funcs = { .fb_create = drm_gem_fb_create, .mode_valid = drm_vram_helper_mode_valid, @@ -1107,6 +1112,8 @@ int ast_mode_config_init(struct ast_private *ast) dev->mode_config.max_height = 1200; } + dev->mode_config.helper_private = &ast_mode_config_helper_funcs; + memset(&ast->primary_plane, 0, sizeof(ast->primary_plane)); ret = drm_universal_plane_init(dev, &ast->primary_plane, 0x01, &ast_primary_plane_funcs, -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces
Hi Am 11.09.20 um 16:07 schrieb Hans de Goede: > Hi, > > On 9/11/20 9:59 AM, Thomas Zimmermann wrote: >> VRAM helpers support ref counting for pin and vmap operations, no need >> to avoid these operations, by employing the internal kmap interface. Just >> use drm_gem_vram_vmap() and let it handle the details. >> >> Also unexport the kmap interfaces from VRAM helpers. Vboxvideo was the >> last user of these internal functions. >> >> Signed-off-by: Thomas Zimmermann > > Nice cleanup. > > I've given this a test-run in an actual VirtualBox vm, focussing on > cursor sprite changes and I don't see any regressions, so: > > Tested-by: Hans de Goede > > Thomas, I assume that you will push this upstream yourself? It's merged now. Thanks for testing the change. Best regards Thomas > > Regards, > > Hans > > >> --- >> drivers/gpu/drm/drm_gem_vram_helper.c | 56 +-- >> drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++-- >> include/drm/drm_gem_vram_helper.h | 3 -- >> 3 files changed, 8 insertions(+), 61 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c >> b/drivers/gpu/drm/drm_gem_vram_helper.c >> index 07447abb4134..0e3cdc40379c 100644 >> --- a/drivers/gpu/drm/drm_gem_vram_helper.c >> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c >> @@ -97,8 +97,8 @@ static const struct drm_gem_object_funcs >> drm_gem_vram_object_funcs; >> * hardware's draing engine. >> * >> * To access a buffer object's memory from the DRM driver, call >> - * drm_gem_vram_kmap(). It (optionally) maps the buffer into kernel >> address >> - * space and returns the memory address. Use drm_gem_vram_kunmap() to >> + * drm_gem_vram_vmap(). It maps the buffer into kernel address >> + * space and returns the memory address. Use drm_gem_vram_vunmap() to >> * release the mapping. >> */ >> @@ -436,39 +436,6 @@ static void *drm_gem_vram_kmap_locked(struct >> drm_gem_vram_object *gbo, >> return kmap->virtual; >> } >> -/** >> - * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address >> space >> - * @gbo: the GEM VRAM object >> - * @map: establish a mapping if necessary >> - * @is_iomem: returns true if the mapped memory is I/O memory, or >> false \ >> - otherwise; can be NULL >> - * >> - * This function maps the buffer object into the kernel's address space >> - * or returns the current mapping. If the parameter map is false, the >> - * function only queries the current mapping, but does not establish a >> - * new one. >> - * >> - * Returns: >> - * The buffers virtual address if mapped, or >> - * NULL if not mapped, or >> - * an ERR_PTR()-encoded error code otherwise. >> - */ >> -void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map, >> - bool *is_iomem) >> -{ >> - int ret; >> - void *virtual; >> - >> - ret = ttm_bo_reserve(&gbo->bo, true, false, NULL); >> - if (ret) >> - return ERR_PTR(ret); >> - virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem); >> - ttm_bo_unreserve(&gbo->bo); >> - >> - return virtual; >> -} >> -EXPORT_SYMBOL(drm_gem_vram_kmap); >> - >> static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo) >> { >> if (WARN_ON_ONCE(!gbo->kmap_use_count)) >> @@ -484,22 +451,6 @@ static void drm_gem_vram_kunmap_locked(struct >> drm_gem_vram_object *gbo) >> */ >> } >> -/** >> - * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object >> - * @gbo: the GEM VRAM object >> - */ >> -void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo) >> -{ >> - int ret; >> - >> - ret = ttm_bo_reserve(&gbo->bo, false, false, NULL); >> - if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret)) >> - return; >> - drm_gem_vram_kunmap_locked(gbo); >> - ttm_bo_unreserve(&gbo->bo); >> -} >> -EXPORT_SYMBOL(drm_gem_vram_kunmap); >> - >> /** >> * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel >> address >> * space >> @@ -511,9 +462,6 @@ EXPORT_SYMBOL(drm_gem_vram_kunmap); >> * permanently. Call drm_gem_vram_vunmap() with the returned address to >> * unmap and unpin the GEM VRAM object. >> * >> - * If you have special requirements for the pinning or mapping >> operations, >> - * call drm_gem_vram_pin() and drm_gem_vram_kmap() directly. >> - * >> * Returns: >> * The buffer's virtual address on success, or >> * an ERR_PTR()-encoded error code otherwise. >> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c >> b/drivers/gpu/drm/vboxvideo/vbox_mode.c >> index d9a5af62af89..4fcc0a542b8a 100644 >> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c >> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c >> @@ -397,11 +397,13 @@ static void vbox_cursor_atomic_update(struct >> drm_plane *plane, >> vbox_crtc->cursor_enabled = true; >> - /* pinning is done in prepare/cleanup framebuffer */ >> - src = drm_gem_vram_kmap(gbo, true, NULL); >> + src = drm_gem_vram_vmap(gbo); >> if (IS_ERR(src)) { >> +
Re: [PATCH] drm/vc4: Handing the return value of drm_universal_plane_init
Hi Am 11.09.20 um 10:09 schrieb Tian Tao: > Handing the return value of drm_universal_plane_init to fix the following > W=1 kernel build warning(s): > vc4_plane.c: In function ‘vc4_plane_init’: > vc4_plane.c:1340:6: warning: variable ‘ret’ set but not > used [-Wunused-but-set-variable] > > Signed-off-by: Tian Tao Reviewed-by: Thomas Zimmermann Thanks! > --- > drivers/gpu/drm/vc4/vc4_plane.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 24d7e6d..65c1205 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -1361,6 +1361,8 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev, > &vc4_plane_funcs, > formats, ARRAY_SIZE(formats), > modifiers, type, NULL); > + if (ret) > + return ERR_PTR(ret); > > drm_plane_helper_add(plane, &vc4_plane_helper_funcs); > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] Add support for SM8150 and SM8250 DSI.
Note I haven't tested SM8150 recently, but DSI is almost identical to SM8250. v2: - added workaround for 5GHz max_rate overflowing in 32-bit builds (based on robclark's suggestion) - Updated Kconfig option to mention SM8250 and not just SM8150 Jonathan Marek (3): drm/msm/dsi: remove unused clk_pre/clk_post in msm_dsi_dphy_timing drm/msm/dsi: add DSI config for sm8150 and sm8250 drm/msm/dsi: add support for 7nm DSI PHY/PLL .../devicetree/bindings/display/msm/dsi.txt | 6 +- drivers/gpu/drm/msm/Kconfig | 8 + drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/dsi/dsi.h | 2 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 423 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 5 +- drivers/gpu/drm/msm/dsi/dsi_cfg.h | 2 + drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 102 ++ drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 6 +- drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 255 + drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 4 + drivers/gpu/drm/msm/dsi/pll/dsi_pll.h | 10 + drivers/gpu/drm/msm/dsi/pll/dsi_pll_7nm.c | 904 ++ 13 files changed, 1724 insertions(+), 5 deletions(-) create mode 100644 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c create mode 100644 drivers/gpu/drm/msm/dsi/pll/dsi_pll_7nm.c -- 2.26.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mediatek: add missing put_device() call in mtk_drm_kms_init()
if of_find_device_by_node() succeed, mtk_drm_kms_init() doesn't have a corresponding put_device(). Thus add jump target to fix the exception handling for this function implementation. Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") Signed-off-by: Yu Kuai --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 040a8f393fe2..7aceace94ebf 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -165,7 +165,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) ret = drmm_mode_config_init(drm); if (ret) - return ret; + goto put_mutex_dev; drm->mode_config.min_width = 64; drm->mode_config.min_height = 64; @@ -182,7 +182,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) ret = component_bind_all(drm->dev, drm); if (ret) - return ret; + goto put_mutex_dev; /* * We currently support two fixed data streams, each optional, @@ -229,7 +229,7 @@ static int mtk_drm_kms_init(struct drm_device *drm) } if (!dma_dev->dma_parms) { ret = -ENOMEM; - goto err_component_unbind; + goto put_dma_dev; } ret = dma_set_max_seg_size(dma_dev, (unsigned int)DMA_BIT_MASK(32)); @@ -256,9 +256,12 @@ static int mtk_drm_kms_init(struct drm_device *drm) err_unset_dma_parms: if (private->dma_parms_allocated) dma_dev->dma_parms = NULL; +put_dma_dev: + put_device(private->dma_dev); err_component_unbind: component_unbind_all(drm->dev, drm); - +put_mutex_dev: + put_device(private->mutex_dev); return ret; } -- 2.25.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 10/17] WIP: gpu: host1x: Add no-recovery mode
05.09.2020 13:34, Mikko Perttunen пишет: > + } else { > + struct host1x_job *failed_job = job; > + > + host1x_job_dump(dev, job); > + > + host1x_syncpt_set_locked(job->syncpt); > + failed_job->cancelled = true; > + > + list_for_each_entry_continue(job, &cdma->sync_queue, list) { > + unsigned int i; > + > + if (job->syncpt != failed_job->syncpt) > + continue; > + > + for (i = 0; i < job->num_slots; i++) { > + unsigned int slot = (job->first_get/8 + i) % > + HOST1X_PUSHBUFFER_SLOTS; > + u32 *mapped = cdma->push_buffer.mapped; > + > + mapped[2*slot+0] = 0x1bad; > + mapped[2*slot+1] = 0x1bad; The 0x1bad is a valid memory address on Tegra20. The 0x6000 is invalid phys address for all hardware generations. It's used by grate-kernel [1] and VDE driver [2]. Note that the 0x6 << 28 is also invalid Host1x opcode, while 0x1 should break CDMA parser during of PB debug-dumping. [1] https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/gem.h#L16 [2] https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/staging/media/tegra-vde/iommu.c#L99 The VDE driver reserves the trapping IOVA addresses, I assume the Host1x driver should do the same. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv4 6/6] iommu: arm-smmu-impl: Remove unwanted extra blank lines
On 2020-09-11 21:37, Will Deacon wrote: On Fri, Sep 11, 2020 at 05:03:06PM +0100, Robin Murphy wrote: BTW am I supposed to have received 3 copies of everything? Because I did... Yeah, this seems to be happening for all of Sai's emails :/ Sorry, I am not sure what went wrong as I only sent this once and there are no recent changes to any of my configs, I'll check it further. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv4 2/6] iommu/arm-smmu: Add domain attribute for system cache
Add iommu domain attribute for using system cache aka last level cache by client drivers like GPU to set right attributes for caching the hardware pagetables into the system cache. Signed-off-by: Sai Prakash Ranjan --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 17 + drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + include/linux/iommu.h | 1 + 3 files changed, 19 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 1f06ab219819..d449c895ba16 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -789,6 +789,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, if (smmu_domain->non_strict) pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT; + if (smmu_domain->sys_cache) + pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_SYS_CACHE; + pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain); if (!pgtbl_ops) { ret = -ENOMEM; @@ -1513,6 +1516,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain, case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE: *(int *)data = smmu_domain->non_strict; return 0; + case DOMAIN_ATTR_SYS_CACHE: + *((int *)data) = smmu_domain->sys_cache; + return 0; default: return -ENODEV; } @@ -1544,6 +1550,17 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, else smmu_domain->stage = ARM_SMMU_DOMAIN_S1; break; + case DOMAIN_ATTR_SYS_CACHE: + if (smmu_domain->smmu) { + ret = -EPERM; + goto out_unlock; + } + + if (*((int *)data)) + smmu_domain->sys_cache = true; + else + smmu_domain->sys_cache = false; + break; default: ret = -ENODEV; } diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h index ddf2ca4c923d..93593e164e44 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h @@ -373,6 +373,7 @@ struct arm_smmu_domain { struct mutexinit_mutex; /* Protects smmu pointer */ spinlock_t cb_lock; /* Serialises ATS1* ops and TLB syncs */ struct iommu_domain domain; + boolsys_cache; }; struct arm_smmu_master_cfg { diff --git a/include/linux/iommu.h b/include/linux/iommu.h index fee209efb756..a580dfe9c68d 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -118,6 +118,7 @@ enum iommu_attr { DOMAIN_ATTR_FSL_PAMUV1, DOMAIN_ATTR_NESTING,/* two stages of translation */ DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, + DOMAIN_ATTR_SYS_CACHE, DOMAIN_ATTR_MAX, }; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel