[PATCH V4] fbcon: fix fbcon out-of-bounds write in sys_imageblit
From: Haimin Zhang yres and vyres can be controlled by user mode parameters, and cause p->vrows to become a negative value. While this value be passed to real_y function, the ypos will be out of screen range.This is an out-of-bounds write bug. some driver will check xres and yres in fb_check_var callback,but some not so we add a common check after that callback. Signed-off-by: Haimin Zhang Signed-off-by: Tetsuo Handa --- drivers/video/fbdev/core/fbmem.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 1c85514..5599372 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1013,6 +1013,10 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var, if (ret) return ret; + /* virtual resolution cannot be smaller than visible resolution. */ + if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) + return -EINVAL; + if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW) return 0; -- 1.8.3.1
Re: [PATCH 2/3] drm/etnaviv: fix dma configuration of the virtual device
Re: [PATCH v3 0/3] Remove all strcpy() uses
Re: [Intel-gfx] [PATCH v10 03/11] drm/ttm: Add a generic TTM memcpy move for page-based iomem
Hi! On 8/30/21 8:16 AM, Christian König wrote: Am 30.08.21 um 03:54 schrieb Dave Airlie: I've just been talking with Ben about nouveau having some issues since this path, ttm_resource can be subclassed by drivers, and the code below that copies ttm_resources around pretty much seems to destroy that. + struct ttm_resource *src_mem = &bo->mem; + struct ttm_resource_manager *src_man = + ttm_manager_type(bdev, src_mem->mem_type); + struct ttm_resource src_copy = *src_mem; This here ^^ Mhm, that's most likely a rebase/merge conflict between my change to subclass ttm_resource which came in through the drm-misc-next tree and Thomas change here. Thomas can you take a look? Sure. /Thomas Thanks, Christian. + union { + struct ttm_kmap_iter_tt tt; + struct ttm_kmap_iter_linear_io io; + } _dst_iter, _src_iter; + struct ttm_kmap_iter *dst_iter, *src_iter; + int ret = 0; - /* - * TTM might be null for moves within the same region. - */ - if (ttm) { + if (ttm && ((ttm->page_flags & TTM_PAGE_FLAG_SWAPPED) || + dst_man->use_tt)) { ret = ttm_tt_populate(bdev, ttm, ctx); if (ret) - goto out1; + return ret; } - for (i = 0; i < new_mem->num_pages; ++i) { - if (old_iomap == NULL) { - pgprot_t prot = ttm_io_prot(bo, old_mem, PAGE_KERNEL); - ret = ttm_copy_ttm_io_page(ttm, new_iomap, i, - prot); - } else if (new_iomap == NULL) { - pgprot_t prot = ttm_io_prot(bo, new_mem, PAGE_KERNEL); - ret = ttm_copy_io_ttm_page(ttm, old_iomap, i, - prot); - } else { - ret = ttm_copy_io_page(new_iomap, old_iomap, i); - } - if (ret) - goto out1; + dst_iter = ttm_kmap_iter_linear_io_init(&_dst_iter.io, bdev, dst_mem); + if (PTR_ERR(dst_iter) == -EINVAL && dst_man->use_tt) + dst_iter = ttm_kmap_iter_tt_init(&_dst_iter.tt, bo->ttm); + if (IS_ERR(dst_iter)) + return PTR_ERR(dst_iter); + + src_iter = ttm_kmap_iter_linear_io_init(&_src_iter.io, bdev, src_mem); + if (PTR_ERR(src_iter) == -EINVAL && src_man->use_tt) + src_iter = ttm_kmap_iter_tt_init(&_src_iter.tt, bo->ttm); + if (IS_ERR(src_iter)) { + ret = PTR_ERR(src_iter); + goto out_src_iter; } - mb(); -out2: - old_copy = *old_mem; - ttm_bo_assign_mem(bo, new_mem); - - if (!man->use_tt) - ttm_bo_tt_destroy(bo); + ttm_move_memcpy(bo, dst_mem->num_pages, dst_iter, src_iter); + src_copy = *src_mem; + ttm_bo_move_sync_cleanup(bo, dst_mem); -out1: - ttm_resource_iounmap(bdev, old_mem, new_iomap); -out: - ttm_resource_iounmap(bdev, &old_copy, old_iomap); + if (!src_iter->ops->maps_tt) + ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, &src_copy); passes a copy into linear_io_fini which calls the driver io_mem_free without the subclass data. Dave.
[PATCH] drm/ttm: Fix ttm_bo_move_memcpy() for subclassed struct ttm_resource
The code was making a copy of a struct ttm_resource. However, recently the struct ttm_resources were allowed to be subclassed and also were allowed to be malloced, hence the driver could end up assuming the copy we handed it was subclassed and worse, the original could have been freed at this point. Fix this by using the original struct ttm_resource before it is potentially freed in ttm_bo_move_sync_cleanup() Reported-by: Ben Skeggs Reported-by: Dave Airlie Cc: Christian König Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based iomem") Signed-off-by: Thomas Hellström --- drivers/gpu/drm/ttm/ttm_bo_util.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 5c20d0541cc3..c893c3db2623 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -139,7 +139,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, struct ttm_resource *src_mem = bo->resource; struct ttm_resource_manager *src_man = ttm_manager_type(bdev, src_mem->mem_type); - struct ttm_resource src_copy = *src_mem; union { struct ttm_kmap_iter_tt tt; struct ttm_kmap_iter_linear_io io; @@ -173,11 +172,10 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, if (!(clear && ttm && !(ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC))) ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter); - src_copy = *src_mem; + if (!src_iter->ops->maps_tt) + ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem); ttm_bo_move_sync_cleanup(bo, dst_mem); - if (!src_iter->ops->maps_tt) - ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, &src_copy); out_src_iter: if (!dst_iter->ops->maps_tt) ttm_kmap_iter_linear_io_fini(&_dst_iter.io, bdev, dst_mem); -- 2.31.1
Re: [Intel-gfx] [PATCH v10 03/11] drm/ttm: Add a generic TTM memcpy move for page-based iomem
Hi! On 8/30/21 8:16 AM, Christian König wrote: Am 30.08.21 um 03:54 schrieb Dave Airlie: I've just been talking with Ben about nouveau having some issues since this path, ttm_resource can be subclassed by drivers, and the code below that copies ttm_resources around pretty much seems to destroy that. + struct ttm_resource *src_mem = &bo->mem; + struct ttm_resource_manager *src_man = + ttm_manager_type(bdev, src_mem->mem_type); + struct ttm_resource src_copy = *src_mem; This here ^^ Mhm, that's most likely a rebase/merge conflict between my change to subclass ttm_resource which came in through the drm-misc-next tree and Thomas change here. Thomas can you take a look? Thanks, Christian. Posted a patch. Ben, it would be great if you could help give it a try to verify that it works. I'll give it a try on vmwgfx later today if I can get it running. Looks like VMware mesa is broken on Fedora.. /Thomas
Re: [PATCH V4] fbcon: fix fbcon out-of-bounds write in sys_imageblit
On Mon, Aug 30, 2021 at 11:30:23AM +0800, tcs.ker...@gmail.com wrote: > From: Haimin Zhang > > yres and vyres can be controlled by user mode parameters, and cause > p->vrows to become a negative value. While this value be passed to real_y > function, the ypos will be out of screen range.This is an out-of-bounds > write bug. > some driver will check xres and yres in fb_check_var callback,but some not > so we add a common check after that callback. > > Signed-off-by: Haimin Zhang > Signed-off-by: Tetsuo Handa Does this fix a syzbot crash or how was this discovered? -Daniel > --- > drivers/video/fbdev/core/fbmem.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c > index 1c85514..5599372 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1013,6 +1013,10 @@ static int fb_check_caps(struct fb_info *info, struct > fb_var_screeninfo *var, > if (ret) > return ret; > > + /* virtual resolution cannot be smaller than visible resolution. */ > + if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) > + return -EINVAL; > + > if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW) > return 0; > > -- > 1.8.3.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/ttm: Fix ttm_bo_move_memcpy() for subclassed struct ttm_resource
Am 30.08.21 um 09:48 schrieb Thomas Hellström: The code was making a copy of a struct ttm_resource. However, recently the struct ttm_resources were allowed to be subclassed and also were allowed to be malloced, hence the driver could end up assuming the copy we handed it was subclassed and worse, the original could have been freed at this point. Fix this by using the original struct ttm_resource before it is potentially freed in ttm_bo_move_sync_cleanup() Reported-by: Ben Skeggs Reported-by: Dave Airlie Cc: Christian König Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based iomem") Signed-off-by: Thomas Hellström Reviewed-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo_util.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 5c20d0541cc3..c893c3db2623 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -139,7 +139,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, struct ttm_resource *src_mem = bo->resource; struct ttm_resource_manager *src_man = ttm_manager_type(bdev, src_mem->mem_type); - struct ttm_resource src_copy = *src_mem; union { struct ttm_kmap_iter_tt tt; struct ttm_kmap_iter_linear_io io; @@ -173,11 +172,10 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, if (!(clear && ttm && !(ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC))) ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter); - src_copy = *src_mem; + if (!src_iter->ops->maps_tt) + ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem); ttm_bo_move_sync_cleanup(bo, dst_mem); - if (!src_iter->ops->maps_tt) - ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, &src_copy); out_src_iter: if (!dst_iter->ops->maps_tt) ttm_kmap_iter_linear_io_fini(&_dst_iter.io, bdev, dst_mem);
Re: [Intel-gfx] [PATCH v2] drm/i915: Handle Intel igfx + Intel dgfx hybrid graphics setup
On Fri, Aug 27, 2021 at 03:44:42PM +0100, Tvrtko Ursulin wrote: > > On 27/08/2021 15:39, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin > > > > In short this makes i915 work for hybrid setups (DRI_PRIME=1 with Mesa) > > when rendering is done on Intel dgfx and scanout/composition on Intel > > igfx. > > > > Before this patch the driver was not quite ready for that setup, mainly > > because it was able to emit a semaphore wait between the two GPUs, which > > results in deadlocks because semaphore target location in HWSP is neither > > shared between the two, nor mapped in both GGTT spaces. > > > > To fix it the patch adds an additional check to a couple of relevant code > > paths in order to prevent using semaphores for inter-engine > > synchronisation between different driver instances. > > > > Patch also moves singly used i915_gem_object_last_write_engine to be > > private in its only calling unit (debugfs), while modifying it to only > > show activity belonging to the respective driver instance. > > > > What remains in this problem space is the question of the GEM busy ioctl. > > We have a somewhat ambigous comment there saying only status of native > > fences will be reported, which could be interpreted as either i915, or > > native to the drm fd. For now I have decided to leave that as is, meaning > > any i915 instance activity continues to be reported. > > > > v2: > > * Avoid adding rq->i915. (Chris) > > > > Signed-off-by: Tvrtko Ursulin Can't we just delete semaphore code and done? - GuC won't have it - media team benchmarked on top of softpin media driver, found no difference - pre-gen8 semaphore code was also silently ditched and no one cared Plus removing semaphore code would greatly simplify conversion to drm/sched. > > --- > > drivers/gpu/drm/i915/gem/i915_gem_object.h | 17 -- > > drivers/gpu/drm/i915/i915_debugfs.c| 39 -- > > drivers/gpu/drm/i915/i915_request.c| 12 ++- > > 3 files changed, 47 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > index 48112b9d76df..3043fcbd31bd 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > @@ -503,23 +503,6 @@ i915_gem_object_finish_access(struct > > drm_i915_gem_object *obj) > > i915_gem_object_unpin_pages(obj); > > } > > -static inline struct intel_engine_cs * > > -i915_gem_object_last_write_engine(struct drm_i915_gem_object *obj) > > -{ > > - struct intel_engine_cs *engine = NULL; > > - struct dma_fence *fence; > > - > > - rcu_read_lock(); > > - fence = dma_resv_get_excl_unlocked(obj->base.resv); > > - rcu_read_unlock(); > > - > > - if (fence && dma_fence_is_i915(fence) && !dma_fence_is_signaled(fence)) > > - engine = to_request(fence)->engine; > > - dma_fence_put(fence); > > - > > - return engine; > > -} > > - > > void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj, > > unsigned int cache_level); > > void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj); > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index 04351a851586..55fd6191eb32 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -135,13 +135,46 @@ static const char *stringify_vma_type(const struct > > i915_vma *vma) > > return "ppgtt"; > > } > > +static char * > > +last_write_engine(struct drm_i915_private *i915, > > + struct drm_i915_gem_object *obj) > > +{ > > + struct intel_engine_cs *engine; > > + struct dma_fence *fence; > > + char *res = NULL; > > + > > + rcu_read_lock(); > > + fence = dma_resv_get_excl_unlocked(obj->base.resv); > > + rcu_read_unlock(); > > + > > + if (!fence || dma_fence_is_signaled(fence)) > > + goto out; > > + > > + if (!dma_fence_is_i915(fence)) { > > + res = ""; > > + goto out; > > + } > > + > > + engine = to_request(fence)->engine; > > + if (engine->gt->i915 != i915) { > > + res = ""; > > + goto out; > > + } > > + > > + res = engine->name; > > + > > +out: > > + dma_fence_put(fence); > > + return res; > > +} > > + > > void > > i915_debugfs_describe_obj(struct seq_file *m, struct drm_i915_gem_object > > *obj) > > { > > struct drm_i915_private *dev_priv = to_i915(obj->base.dev); > > - struct intel_engine_cs *engine; > > struct i915_vma *vma; > > int pin_count = 0; > > + char *engine; > > seq_printf(m, "%pK: %c%c%c %8zdKiB %02x %02x %s%s%s", > >&obj->base, > > @@ -230,9 +263,9 @@ i915_debugfs_describe_obj(struct seq_file *m, struct > > drm_i915_gem_object *obj) > > if (i915_gem_object_is_framebuffer(obj)) > > seq_printf(m, " (fb)"); > > - engine = i915_gem_object_last_wr
Re: [PATCH] drm/ttm: Fix ttm_bo_move_memcpy() for subclassed struct ttm_resource
On Mon, 30 Aug 2021 at 17:48, Thomas Hellström wrote: > > The code was making a copy of a struct ttm_resource. However, > recently the struct ttm_resources were allowed to be subclassed and > also were allowed to be malloced, hence the driver could end up assuming > the copy we handed it was subclassed and worse, the original could have > been freed at this point. > > Fix this by using the original struct ttm_resource before it is > potentially freed in ttm_bo_move_sync_cleanup() > > Reported-by: Ben Skeggs > Reported-by: Dave Airlie > Cc: Christian König > Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based > iomem") > Signed-off-by: Thomas Hellström That's basically identical to what I came up with locally, so: Reviewed-by: Ben Skeggs > --- > drivers/gpu/drm/ttm/ttm_bo_util.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c > b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 5c20d0541cc3..c893c3db2623 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -139,7 +139,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, > struct ttm_resource *src_mem = bo->resource; > struct ttm_resource_manager *src_man = > ttm_manager_type(bdev, src_mem->mem_type); > - struct ttm_resource src_copy = *src_mem; > union { > struct ttm_kmap_iter_tt tt; > struct ttm_kmap_iter_linear_io io; > @@ -173,11 +172,10 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, > if (!(clear && ttm && !(ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC))) > ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, > src_iter); > > - src_copy = *src_mem; > + if (!src_iter->ops->maps_tt) > + ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem); > ttm_bo_move_sync_cleanup(bo, dst_mem); > > - if (!src_iter->ops->maps_tt) > - ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, &src_copy); > out_src_iter: > if (!dst_iter->ops->maps_tt) > ttm_kmap_iter_linear_io_fini(&_dst_iter.io, bdev, dst_mem); > -- > 2.31.1 >
[PATCH 02/12] drm/ttm: add back a reference to the bdev to the res manager
It is simply a lot cleaner to have this around instead of adding the device throughout the call chain. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c| 3 ++- drivers/gpu/drm/nouveau/nouveau_ttm.c | 4 ++-- drivers/gpu/drm/ttm/ttm_range_manager.c | 2 +- drivers/gpu/drm/ttm/ttm_resource.c | 3 +++ drivers/gpu/drm/ttm/ttm_sys_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 2 +- include/drm/ttm/ttm_resource.h | 8 10 files changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 77cfb64dd312..1dae68caa974 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -293,7 +293,8 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) man->use_tt = true; man->func = &amdgpu_gtt_mgr_func; - ttm_resource_manager_init(man, gtt_size >> PAGE_SHIFT); + ttm_resource_manager_init(man, &adev->mman.bdev, + gtt_size >> PAGE_SHIFT); start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS; size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c index ffddec08e931..6f7189d32f0a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c @@ -153,7 +153,7 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev) man->use_tt = true; man->func = &amdgpu_preempt_mgr_func; - ttm_resource_manager_init(man, (1 << 30)); + ttm_resource_manager_init(man, &adev->mman.bdev, (1 << 30)); atomic64_set(&mgr->used, 0); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index 0184210744a7..c1e97c865c2b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -699,7 +699,8 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev) struct amdgpu_vram_mgr *mgr = &adev->mman.vram_mgr; struct ttm_resource_manager *man = &mgr->manager; - ttm_resource_manager_init(man, adev->gmc.real_vram_size >> PAGE_SHIFT); + ttm_resource_manager_init(man, &adev->mman.bdev, + adev->gmc.real_vram_size >> PAGE_SHIFT); man->func = &amdgpu_vram_mgr_func; diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index 34421174ed59..6adee64b2332 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -164,7 +164,7 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm) man->func = &nouveau_vram_manager; - ttm_resource_manager_init(man, + ttm_resource_manager_init(man, &drm->ttm.bdev, drm->gem.vram_available >> PAGE_SHIFT); ttm_set_driver_manager(&drm->ttm.bdev, TTM_PL_VRAM, man); ttm_resource_manager_set_used(man, true); @@ -211,7 +211,7 @@ nouveau_ttm_init_gtt(struct nouveau_drm *drm) man->func = func; man->use_tt = true; - ttm_resource_manager_init(man, size_pages); + ttm_resource_manager_init(man, &drm->ttm.bdev, size_pages); ttm_set_driver_manager(&drm->ttm.bdev, TTM_PL_TT, man); ttm_resource_manager_set_used(man, true); return 0; diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c index 69962b5769c5..5c62cbb2a205 100644 --- a/drivers/gpu/drm/ttm/ttm_range_manager.c +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c @@ -156,7 +156,7 @@ int ttm_range_man_init(struct ttm_device *bdev, man->func = &ttm_range_manager_func; - ttm_resource_manager_init(man, p_size); + ttm_resource_manager_init(man, bdev, p_size); drm_mm_init(&rman->mm, 0, p_size); spin_lock_init(&rman->lock); diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 2e0db43ff99c..122f19e6968b 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -77,16 +77,19 @@ EXPORT_SYMBOL(ttm_resource_free); * ttm_resource_manager_init * * @man: memory manager object to init + * @bdev: ttm device this manager belongs to * @p_size: size managed area in pages. * * Initialise core parts of a manager object. */ void ttm_resource_manager_init(struct ttm_resource_manager *man, + struct ttm_device *bdev, unsigned long p_size) {
[PATCH 05/12] drm/ttm: move the LRU into resource handling
This way we finally fix the problem that new resource are not immediately evict-able after allocation. That has caused numerous problems including OOM on GDS handling and not being able to use TTM as general resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c| 105 ++-- drivers/gpu/drm/ttm/ttm_bo_util.c | 1 - drivers/gpu/drm/ttm/ttm_device.c| 8 +- drivers/gpu/drm/ttm/ttm_resource.c | 127 include/drm/ttm/ttm_bo_api.h| 16 --- include/drm/ttm/ttm_bo_driver.h | 29 +- include/drm/ttm/ttm_resource.h | 35 +++ 9 files changed, 181 insertions(+), 150 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6362e861a3f5..70d2cbb1dbb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (vm->bulk_moveable) { spin_lock(&adev->mman.bdev.lru_lock); - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move); + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); return; } - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move)); + ttm_lru_bulk_move_init(&vm->lru_bulk_move); spin_lock(&adev->mman.bdev.lru_lock); list_for_each_entry(bo_base, &vm->idle, vm_status) { @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (!bo->parent) continue; - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource, - &vm->lru_bulk_move); + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); if (shadow) ttm_bo_move_to_lru_tail(&shadow->tbo, - shadow->tbo.resource, &vm->lru_bulk_move); } spin_unlock(&adev->mman.bdev.lru_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e646aac9d7a4..41f0de841d72 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) bo->priority = I915_TTM_PRIO_NO_PAGES; } - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->lru_lock); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 49f4bc97c35a..d5c6e096fd31 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_del_init(&bo->lru); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos, -struct ttm_buffer_object *bo) -{ - if (!pos->first) - pos->first = bo; - pos->last = bo; -} - void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, -struct ttm_resource *mem, struct ttm_lru_bulk_move *bulk) { - struct ttm_device *bdev = bo->bdev; - struct ttm_resource_manager *man; - - if (!bo->deleted) - dma_resv_assert_held(bo->base.resv); - - if (bo->pin_count) { - ttm_bo_del_from_lru(bo); - return; - } - - if (!mem) - return; - - man = ttm_manager_type(bdev, mem->mem_type); - list_move_tail(&bo->lru, &man->lru[bo->priority]); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); - - if (bulk && !bo->pin_count) { - switch (bo->resource->mem_type) { - case TTM_PL_TT: - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo); - break; + dma_resv_assert_held(bo->base.resv); - case TTM_PL_VRAM: - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo); - break; - } - } + if (bo->resource) + ttm_resource_move_to_lru_tail(bo->resource, bulk); } EXPORT_SYMBOL(ttm_bo_move_to_lru_tail); -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk) -{ - unsigned i; - - for
[PATCH 04/12] drm/ttm: add common accounting to the resource mgr
It makes sense to have this in the common manager for debugging and accounting of how much resources are used. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_resource.c | 8 include/drm/ttm/ttm_resource.h | 18 ++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index a4c495da0040..426e6841fc89 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -33,6 +33,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo, const struct ttm_place *place, struct ttm_resource *res) { + struct ttm_resource_manager *man; + res->start = 0; res->num_pages = PFN_UP(bo->base.size); res->mem_type = place->mem_type; @@ -42,12 +44,16 @@ void ttm_resource_init(struct ttm_buffer_object *bo, res->bus.is_iomem = false; res->bus.caching = ttm_cached; res->bo = bo; + + man = ttm_manager_type(bo->bdev, place->mem_type); + atomic64_add(bo->base.size, &man->usage); } EXPORT_SYMBOL(ttm_resource_init); void ttm_resource_fini(struct ttm_resource_manager *man, struct ttm_resource *res) { + atomic64_sub(res->bo->base.size, &man->usage); } EXPORT_SYMBOL(ttm_resource_fini); @@ -100,6 +106,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man, spin_lock_init(&man->move_lock); man->bdev = bdev; man->size = p_size; + atomic64_set(&man->usage, 0); for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) INIT_LIST_HEAD(&man->lru[i]); @@ -172,6 +179,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man, drm_printf(p, " use_type: %d\n", man->use_type); drm_printf(p, " use_tt: %d\n", man->use_tt); drm_printf(p, " size: %llu\n", man->size); + drm_printf(p, " usage: %llu\n", atomic64_read(&man->usage)); if (man->func->debug) man->func->debug(man, p); } diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index e8080192cae4..526fe359c603 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -110,6 +111,7 @@ struct ttm_resource_manager_func { * static information. bdev::driver::io_mem_free is never used. * @lru: The lru list for this memory type. * @move: The fence of the last pipelined move operation. + * @usage: How much of the region is used. * * This structure is used to identify and manage memory types for a device. */ @@ -134,6 +136,9 @@ struct ttm_resource_manager { * Protected by @move_lock. */ struct dma_fence *move; + + /* Own protection */ + atomic64_t usage; }; /** @@ -260,6 +265,19 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man) man->move = NULL; } +/** + * ttm_resource_manager_usage + * + * @man: A memory manager object. + * + * Return how many resources are currently used. + */ +static inline uint64_t +ttm_resource_manager_usage(struct ttm_resource_manager *man) +{ + return atomic64_read(&man->usage); +} + void ttm_resource_init(struct ttm_buffer_object *bo, const struct ttm_place *place, struct ttm_resource *res); -- 2.25.1
[PATCH 06/12] drm/ttm: add resource iterator
Instead of duplicating that at different places add an iterator over all the resources in a resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 41 +++ drivers/gpu/drm/ttm/ttm_device.c | 34 -- drivers/gpu/drm/ttm/ttm_resource.c | 45 ++ include/drm/ttm/ttm_resource.h | 23 +++ 4 files changed, 103 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index d5c6e096fd31..8ca418d4a059 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -586,38 +586,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev, struct ww_acquire_ctx *ticket) { struct ttm_buffer_object *bo = NULL, *busy_bo = NULL; + struct ttm_resource_cursor cursor; struct ttm_resource *res; bool locked = false; - unsigned i; int ret; spin_lock(&bdev->lru_lock); - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - list_for_each_entry(res, &man->lru[i], lru) { - bool busy; - - bo = res->bo; - if (!ttm_bo_evict_swapout_allowable(bo, ctx, place, - &locked, &busy)) { - if (busy && !busy_bo && ticket != - dma_resv_locking_ctx(bo->base.resv)) - busy_bo = bo; - continue; - } - - if (!ttm_bo_get_unless_zero(bo)) { - if (locked) - dma_resv_unlock(bo->base.resv); - continue; - } - break; + ttm_resource_manager_for_each_res(man, &cursor, res) { + bool busy; + + if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place, + &locked, &busy)) { + if (busy && !busy_bo && ticket != + dma_resv_locking_ctx(bo->base.resv)) + busy_bo = res->bo; + continue; } - /* If the inner loop terminated early, we have our candidate */ - if (&res->lru != &man->lru[i]) - break; - - bo = NULL; + if (!ttm_bo_get_unless_zero(res->bo)) { + if (locked) + dma_resv_unlock(res->bo->base.resv); + continue; + } + bo = res->bo; } if (!bo) { diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 9e0dfceff68c..c019d8dcd82c 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -132,10 +132,11 @@ EXPORT_SYMBOL(ttm_global_swapout); int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { + struct ttm_resource_cursor cursor; struct ttm_resource_manager *man; struct ttm_buffer_object *bo; struct ttm_resource *res; - unsigned i, j; + unsigned i; int ret; spin_lock(&bdev->lru_lock); @@ -144,20 +145,23 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, if (!man || !man->use_tt) continue; - for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) { - list_for_each_entry(res, &man->lru[j], lru) { - uint32_t num_pages; - - bo = res->bo; - num_pages = PFN_UP(bo->base.size); - - ret = ttm_bo_swapout(bo, ctx, gfp_flags); - /* ttm_bo_swapout has dropped the lru_lock */ - if (!ret) - return num_pages; - if (ret != -EBUSY) - return ret; - } + ttm_resource_manager_for_each_res(man, &cursor, res) { + uint32_t num_pages; + + bo = res->bo; + num_pages = PFN_UP(bo->base.size); + if (!bo->ttm || + bo->ttm->page_flags & TTM_PAGE_FLAG_SG || + bo->ttm->page_flags & TTM_PAGE_FLAG_SWAPPED) + continue; + + num_pages = bo->ttm->num_pages; + ret = ttm_bo_swapout(bo, ctx, gfp_flags); + /* ttm_bo_swapout has dropped the lru_lock */ + if (!ret) +
[PATCH 03/12] drm/ttm: add a weak BO reference to the resource v3
Keep track for which BO a resource was allocated. This is necessary to move the LRU handling into the resources. A bit problematic is i915 since it tries to use the resource interface without a BO which is illegal from the conceptional point of view. v2: Document that this is a weak reference and add a workaround for i915 v3: further document that this is protected by ttm_device::lru_lock and clarify the i915 workaround Signed-off-by: Christian König --- drivers/gpu/drm/i915/intel_region_ttm.c | 22 -- drivers/gpu/drm/ttm/ttm_bo_util.c | 7 +-- drivers/gpu/drm/ttm/ttm_resource.c | 9 + include/drm/ttm/ttm_resource.h | 4 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c index 27fe0668d094..1b2a61b085f7 100644 --- a/drivers/gpu/drm/i915/intel_region_ttm.c +++ b/drivers/gpu/drm/i915/intel_region_ttm.c @@ -75,24 +75,26 @@ intel_region_ttm_node_reserve(struct intel_memory_region *mem, int ret; /* -* Having to use a mock_bo is unfortunate but stems from some -* drivers having private managers that insist to know what the -* allocate memory is intended for, using it to send private -* data to the manager. Also recently the bo has been used to send -* alignment info to the manager. Assume that apart from the latter, -* none of the managers we use will ever access the buffer object -* members, hoping we can pass the alignment info in the -* struct ttm_place in the future. +* This is essential an illegal use the of the TTM resource manager +* backend and should be fixed in the future. For now work around by +* using a mock_bo with at least the mandatory fields initialized. +* +* The resource should be ignored by TTM since it can't grab a +* reference to the BO during eviction. */ place.fpfn = offset >> PAGE_SHIFT; place.lpfn = place.fpfn + (size >> PAGE_SHIFT); mock_bo.base.size = size; + mock_bo.bdev = &mem->i915->bdev; ret = man->func->alloc(man, &mock_bo, &place, &res); if (ret == -ENOSPC) - ret = -ENXIO; + return ERR_PTR(-ENXIO); + if (ret) + return ERR_PTR(ret); - return ret ? ERR_PTR(ret) : res; + res->bo = NULL; + return 0; } /** diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 763fa6f4e07d..c5d02edaefc0 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -242,6 +242,11 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, if (bo->type != ttm_bo_type_sg) fbo->base.base.resv = &fbo->base.base._resv; + if (fbo->base.resource) { + ttm_resource_set_bo(fbo->base.resource, &fbo->base); + bo->resource = NULL; + } + dma_resv_init(&fbo->base.base._resv); fbo->base.base.dev = NULL; ret = dma_resv_trylock(&fbo->base.base._resv); @@ -510,7 +515,6 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo, ghost_obj->ttm = NULL; else bo->ttm = NULL; - bo->resource = NULL; dma_resv_unlock(&ghost_obj->base._resv); ttm_bo_put(ghost_obj); @@ -638,7 +642,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); bo->ttm = ttm; - bo->resource = NULL; ttm_bo_assign_mem(bo, sys_res); return 0; diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 122f19e6968b..a4c495da0040 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -41,6 +41,7 @@ void ttm_resource_init(struct ttm_buffer_object *bo, res->bus.offset = 0; res->bus.is_iomem = false; res->bus.caching = ttm_cached; + res->bo = bo; } EXPORT_SYMBOL(ttm_resource_init); @@ -73,6 +74,14 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res) } EXPORT_SYMBOL(ttm_resource_free); +void ttm_resource_set_bo(struct ttm_resource *res, +struct ttm_buffer_object *bo) +{ + spin_lock(&bo->bdev->lru_lock); + res->bo = bo; + spin_unlock(&bo->bdev->lru_lock); +} + /** * ttm_resource_manager_init * diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h index c004672789b6..e8080192cae4 100644 --- a/include/drm/ttm/ttm_resource.h +++ b/include/drm/ttm/ttm_resource.h @@ -160,6 +160,7 @@ struct ttm_bus_placement { * @mem_type: Resource type of the allocation. * @placement: Placement flags. * @bus: Placement on io bus accessible to the CPU + * @bo: weak reference to the BO, protected by ttm_device::lru_lock
[PATCH 01/12] drm/ttm: add ttm_resource_fini
Make sure we call the common cleanup function in all implementations of the resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c| 2 ++ drivers/gpu/drm/nouveau/nouveau_mem.c | 3 ++- drivers/gpu/drm/nouveau/nouveau_mem.h | 3 ++- drivers/gpu/drm/nouveau/nouveau_ttm.c | 9 + drivers/gpu/drm/ttm/ttm_range_manager.c | 2 ++ drivers/gpu/drm/ttm/ttm_resource.c | 6 ++ drivers/gpu/drm/ttm/ttm_sys_manager.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 3 ++- include/drm/ttm/ttm_resource.h | 3 +++ 12 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 543000304a1c..77cfb64dd312 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -169,6 +169,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, return 0; err_free: + ttm_resource_fini(man, &node->base.base); kfree(node); err_out: @@ -200,6 +201,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, if (!(res->placement & TTM_PL_FLAG_TEMPORARY)) atomic64_sub(res->num_pages, &mgr->used); + ttm_resource_fini(man, res); kfree(node); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c index d02c8637f909..ffddec08e931 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c @@ -95,6 +95,7 @@ static void amdgpu_preempt_mgr_del(struct ttm_resource_manager *man, struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man); atomic64_sub(res->num_pages, &mgr->used); + ttm_resource_fini(man, res); kfree(res); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index 2fd77c36a1ff..0184210744a7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -476,6 +476,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man, while (i--) drm_mm_remove_node(&node->mm_nodes[i]); spin_unlock(&mgr->lock); + ttm_resource_fini(man, &node->base); kvfree(node); error_sub: @@ -515,6 +516,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man, atomic64_sub(usage, &mgr->usage); atomic64_sub(vis_usage, &mgr->vis_usage); + ttm_resource_fini(man, res); kvfree(node); } diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.c b/drivers/gpu/drm/nouveau/nouveau_mem.c index 0de6549fb875..5b610906bec8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_mem.c +++ b/drivers/gpu/drm/nouveau/nouveau_mem.c @@ -175,11 +175,12 @@ nouveau_mem_vram(struct ttm_resource *reg, bool contig, u8 page) } void -nouveau_mem_del(struct ttm_resource *reg) +nouveau_mem_del(struct ttm_resource_manager *man, struct ttm_resource *reg) { struct nouveau_mem *mem = nouveau_mem(reg); nouveau_mem_fini(mem); + ttm_resource_fini(man, reg); kfree(mem); } diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.h b/drivers/gpu/drm/nouveau/nouveau_mem.h index 2c01166a90f2..325551eba5cd 100644 --- a/drivers/gpu/drm/nouveau/nouveau_mem.h +++ b/drivers/gpu/drm/nouveau/nouveau_mem.h @@ -23,7 +23,8 @@ nouveau_mem(struct ttm_resource *reg) int nouveau_mem_new(struct nouveau_cli *, u8 kind, u8 comp, struct ttm_resource **); -void nouveau_mem_del(struct ttm_resource *); +void nouveau_mem_del(struct ttm_resource_manager *man, +struct ttm_resource *); int nouveau_mem_vram(struct ttm_resource *, bool contig, u8 page); int nouveau_mem_host(struct ttm_resource *, struct ttm_tt *); void nouveau_mem_fini(struct nouveau_mem *); diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index f4c2e46b6fe1..34421174ed59 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -36,9 +36,10 @@ #include static void -nouveau_manager_del(struct ttm_resource_manager *man, struct ttm_resource *reg) +nouveau_manager_del(struct ttm_resource_manager *man, + struct ttm_resource *reg) { - nouveau_mem_del(reg); + nouveau_mem_del(man, reg); } static int @@ -62,7 +63,7 @@ nouveau_vram_manager_new(struct ttm_resource_manager *man, ret = nouveau_mem_vram(*res, nvbo->contig, nvbo->page); if (ret) { - nouveau_mem_del(*res); + nouveau_mem_del(man, *res); return ret; } @@ -118,7 +119,7
[PATCH 07/12] drm/radeon: use ttm_resource_manager_debug
Instead of calling the debug operation directly. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 7793249bc549..9c45d8eb8680 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -802,7 +802,7 @@ static int radeon_mm_vram_dump_table_show(struct seq_file *m, void *unused) TTM_PL_VRAM); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } @@ -820,7 +820,7 @@ static int radeon_mm_gtt_dump_table_show(struct seq_file *m, void *unused) TTM_PL_TT); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } -- 2.25.1
[PATCH 09/12] drm/amdgpu: use ttm_resource_manager_debug
Instead of calling the debug operation directly. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 489e22190e29..fdf4c46d2073 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -2045,7 +2045,7 @@ static int amdgpu_mm_vram_table_show(struct seq_file *m, void *unused) TTM_PL_VRAM); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } @@ -2063,7 +2063,7 @@ static int amdgpu_mm_tt_table_show(struct seq_file *m, void *unused) TTM_PL_TT); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } @@ -2074,7 +2074,7 @@ static int amdgpu_mm_gds_table_show(struct seq_file *m, void *unused) AMDGPU_PL_GDS); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } @@ -2085,7 +2085,7 @@ static int amdgpu_mm_gws_table_show(struct seq_file *m, void *unused) AMDGPU_PL_GWS); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } @@ -2096,7 +2096,7 @@ static int amdgpu_mm_oa_table_show(struct seq_file *m, void *unused) AMDGPU_PL_OA); struct drm_printer p = drm_seq_file_printer(m); - man->func->debug(man, &p); + ttm_resource_manager_debug(man, &p); return 0; } -- 2.25.1
[PATCH 08/12] drm/radeon: remove resource accounting
Use the one provided by TTM instead. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon.h| 2 -- drivers/gpu/drm/radeon/radeon_kms.c| 7 -- drivers/gpu/drm/radeon/radeon_object.c | 30 +++--- drivers/gpu/drm/radeon/radeon_object.h | 1 - drivers/gpu/drm/radeon/radeon_ttm.c| 18 ++-- 5 files changed, 10 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 895776c421d4..08f83bf2c330 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -2462,8 +2462,6 @@ struct radeon_device { struct radeon_vm_managervm_manager; struct mutexgpu_clock_mutex; /* memory stats */ - atomic64_t vram_usage; - atomic64_t gtt_usage; atomic64_t num_bytes_moved; atomic_tgpu_reset_counter; /* ACPI interface */ diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 0473583dcdac..e3643379b607 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) struct drm_radeon_info *info = data; struct radeon_mode_info *minfo = &rdev->mode_info; uint32_t *value, value_tmp, *value_ptr, value_size; + struct ttm_resource_manager *man; uint64_t value64; struct drm_crtc *crtc; int i, found; @@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) case RADEON_INFO_VRAM_USAGE: value = (uint32_t*)&value64; value_size = sizeof(uint64_t); - value64 = atomic64_read(&rdev->vram_usage); + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM); + value64 = ttm_resource_manager_usage(man); break; case RADEON_INFO_GTT_USAGE: value = (uint32_t*)&value64; value_size = sizeof(uint64_t); - value64 = atomic64_read(&rdev->gtt_usage); + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT); + value64 = ttm_resource_manager_usage(man); break; case RADEON_INFO_ACTIVE_CU_COUNT: if (rdev->family >= CHIP_BONAIRE) diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 56ede9d63b12..c9bbed2a25ad 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo *bo); * function are calling it. */ -static void radeon_update_memory_usage(struct ttm_buffer_object *bo, - unsigned int mem_type, int sign) -{ - struct radeon_device *rdev = radeon_get_rdev(bo->bdev); - - switch (mem_type) { - case TTM_PL_TT: - if (sign > 0) - atomic64_add(bo->base.size, &rdev->gtt_usage); - else - atomic64_sub(bo->base.size, &rdev->gtt_usage); - break; - case TTM_PL_VRAM: - if (sign > 0) - atomic64_add(bo->base.size, &rdev->vram_usage); - else - atomic64_sub(bo->base.size, &rdev->vram_usage); - break; - } -} - static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo) { struct radeon_bo *bo; @@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev) static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev) { u64 real_vram_size = rdev->mc.real_vram_size; - u64 vram_usage = atomic64_read(&rdev->vram_usage); + struct ttm_resource_manager *man = + ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM); + u64 vram_usage = ttm_resource_manager_usage(man); /* This function is based on the current VRAM usage. * @@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved, } void radeon_bo_move_notify(struct ttm_buffer_object *bo, - unsigned int old_type, struct ttm_resource *new_mem) { struct radeon_bo *rbo; - radeon_update_memory_usage(bo, old_type, -1); - if (new_mem) - radeon_update_memory_usage(bo, new_mem->mem_type, 1); - if (!radeon_ttm_bo_is_radeon_bo(bo)) return; diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index 1afc7992ef91..0b64e202577b 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -161,7 +161,6 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *b
[PATCH 11/12] drm/amdgpu: remove VRAM accounting
This is provided by TTM now. Also switch man->size to bytes instead of pages and fix the double printing of size and usage in debugfs. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 56 +++- 7 files changed, 26 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c index 7b46ba551cb2..c2d5aef2ae48 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c @@ -520,9 +520,10 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd, uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd) { struct amdgpu_device *adev = (struct amdgpu_device *)kgd; - struct ttm_resource_manager *vram_man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM); + struct ttm_resource_manager *vram_man = + ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM); - return amdgpu_vram_mgr_usage(vram_man); + return ttm_resource_manager_usage(vram_man); } uint64_t amdgpu_amdkfd_get_hive_id(struct kgd_dev *kgd) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index a152363b0254..c2f17d4df748 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -315,7 +315,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev, } total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size); - used_vram = amdgpu_vram_mgr_usage(vram_man); + used_vram = ttm_resource_manager_usage(vram_man); free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram; spin_lock(&adev->mm_stats.lock); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index bd8e28e9ca7c..9e9d4f4a8743 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -665,7 +665,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) ui64 = atomic64_read(&adev->num_vram_cpu_page_faults); return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0; case AMDGPU_INFO_VRAM_USAGE: - ui64 = amdgpu_vram_mgr_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM)); + ui64 = ttm_resource_manager_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM)); return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0; case AMDGPU_INFO_VIS_VRAM_USAGE: ui64 = amdgpu_vram_mgr_vis_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM)); @@ -711,8 +711,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) mem.vram.usable_heap_size = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size) - AMDGPU_VM_RESERVED_VRAM; - mem.vram.heap_usage = - amdgpu_vram_mgr_usage(vram_man); + mem.vram.heap_usage = ttm_resource_manager_usage(vram_man); mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4; mem.cpu_accessible_vram.total_heap_size = diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index fdf4c46d2073..f3222a396fd1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1872,7 +1872,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable) size = adev->gmc.real_vram_size; else size = adev->gmc.visible_vram_size; - man->size = size >> PAGE_SHIFT; + man->size = size; adev->mman.buffer_funcs_enabled = enable; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index af0faf9b36f0..ef84a1a78fef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -44,7 +44,6 @@ struct amdgpu_vram_mgr { spinlock_t lock; struct list_head reservations_pending; struct list_head reserved_pages; - atomic64_t usage; atomic64_t vis_usage; }; @@ -127,7 +126,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev, void amdgpu_vram_mgr_free_sgt(struct device *dev, enum dma_data_direction dir, struct sg_table *sgt); -uint64_t amdgpu_vram_mgr_usage(struct ttm_resource_manager *man); uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_res
[PATCH 10/12] drm/amdgpu: remove GTT accounting
This is provided by TTM now. Also switch man->size to bytes instead of pages and fix the double printing of size and usage in debugfs. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 50 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 - 3 files changed, 12 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 1dae68caa974..7085494c6a9f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev, struct ttm_resource_manager *man; man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT); - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE); + return sysfs_emit(buf, "%llu\n", man->size); } /** @@ -80,7 +80,7 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev, struct ttm_resource_manager *man; man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT); - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(man)); + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man)); } static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO, @@ -132,20 +132,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, struct amdgpu_gtt_node *node; int r; - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && - atomic64_add_return(num_pages, &mgr->used) > man->size) { - atomic64_sub(num_pages, &mgr->used); - return -ENOSPC; - } - node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL); - if (!node) { - r = -ENOMEM; - goto err_out; - } + if (!node) + return -ENOMEM; node->tbo = tbo; ttm_resource_init(tbo, place, &node->base.base); + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && + ttm_resource_manager_usage(man) > man->size) { + r = -ENOSPC; + goto err_free; + } if (place->lpfn) { spin_lock(&mgr->lock); @@ -171,11 +168,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, err_free: ttm_resource_fini(man, &node->base.base); kfree(node); - -err_out: - if (!(place->flags & TTM_PL_FLAG_TEMPORARY)) - atomic64_sub(num_pages, &mgr->used); - return r; } @@ -198,27 +190,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, drm_mm_remove_node(&node->base.mm_nodes[0]); spin_unlock(&mgr->lock); - if (!(res->placement & TTM_PL_FLAG_TEMPORARY)) - atomic64_sub(res->num_pages, &mgr->used); - ttm_resource_fini(man, res); kfree(node); } -/** - * amdgpu_gtt_mgr_usage - return usage of GTT domain - * - * @man: TTM memory type manager - * - * Return how many bytes are used in the GTT domain - */ -uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man) -{ - struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); - - return atomic64_read(&mgr->used) * PAGE_SIZE; -} - /** * amdgpu_gtt_mgr_recover - re-init gart * @@ -265,9 +240,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man, spin_lock(&mgr->lock); drm_mm_print(&mgr->mm, printer); spin_unlock(&mgr->lock); - - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n", - man->size, atomic64_read(&mgr->used)); } static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = { @@ -293,14 +265,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size) man->use_tt = true; man->func = &amdgpu_gtt_mgr_func; - ttm_resource_manager_init(man, &adev->mman.bdev, - gtt_size >> PAGE_SHIFT); + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size); start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS; size = (adev->gmc.gart_size >> PAGE_SHIFT) - start; drm_mm_init(&mgr->mm, start, size); spin_lock_init(&mgr->lock); - atomic64_set(&mgr->used, 0); ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager); ttm_resource_manager_set_used(man, true); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 20b049ad61c1..bd8e28e9ca7c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -671,7 +671,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) ui64 = amdgpu_vram_mgr_vis_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM)); return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0; case AMD
[PATCH 12/12] drm/amdgpu: drop amdgpu_gtt_node
We have the BO pointer in the base structure now as well. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 - 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 7085494c6a9f..22506e429a62 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -26,23 +26,12 @@ #include "amdgpu.h" -struct amdgpu_gtt_node { - struct ttm_buffer_object *tbo; - struct ttm_range_mgr_node base; -}; - static inline struct amdgpu_gtt_mgr * to_gtt_mgr(struct ttm_resource_manager *man) { return container_of(man, struct amdgpu_gtt_mgr, manager); } -static inline struct amdgpu_gtt_node * -to_amdgpu_gtt_node(struct ttm_resource *res) -{ - return container_of(res, struct amdgpu_gtt_node, base.base); -} - /** * DOC: mem_info_gtt_total * @@ -107,9 +96,9 @@ const struct attribute_group amdgpu_gtt_mgr_attr_group = { */ bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res) { - struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res); + struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res); - return drm_mm_node_allocated(&node->base.mm_nodes[0]); + return drm_mm_node_allocated(&node->mm_nodes[0]); } /** @@ -129,15 +118,14 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, { struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); uint32_t num_pages = PFN_UP(tbo->base.size); - struct amdgpu_gtt_node *node; + struct ttm_range_mgr_node *node; int r; - node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL); + node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL); if (!node) return -ENOMEM; - node->tbo = tbo; - ttm_resource_init(tbo, place, &node->base.base); + ttm_resource_init(tbo, place, &node->base); if (!(place->flags & TTM_PL_FLAG_TEMPORARY) && ttm_resource_manager_usage(man) > man->size) { r = -ENOSPC; @@ -146,8 +134,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, if (place->lpfn) { spin_lock(&mgr->lock); - r = drm_mm_insert_node_in_range(&mgr->mm, - &node->base.mm_nodes[0], + r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0], num_pages, tbo->page_alignment, 0, place->fpfn, place->lpfn, DRM_MM_INSERT_BEST); @@ -155,18 +142,18 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, if (unlikely(r)) goto err_free; - node->base.base.start = node->base.mm_nodes[0].start; + node->base.start = node->mm_nodes[0].start; } else { - node->base.mm_nodes[0].start = 0; - node->base.mm_nodes[0].size = node->base.base.num_pages; - node->base.base.start = AMDGPU_BO_INVALID_OFFSET; + node->mm_nodes[0].start = 0; + node->mm_nodes[0].size = node->base.num_pages; + node->base.start = AMDGPU_BO_INVALID_OFFSET; } - *res = &node->base.base; + *res = &node->base; return 0; err_free: - ttm_resource_fini(man, &node->base.base); + ttm_resource_fini(man, &node->base); kfree(node); return r; } @@ -182,12 +169,12 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man, static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, struct ttm_resource *res) { - struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res); + struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res); struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); spin_lock(&mgr->lock); - if (drm_mm_node_allocated(&node->base.mm_nodes[0])) - drm_mm_remove_node(&node->base.mm_nodes[0]); + if (drm_mm_node_allocated(&node->mm_nodes[0])) + drm_mm_remove_node(&node->mm_nodes[0]); spin_unlock(&mgr->lock); ttm_resource_fini(man, res); @@ -204,16 +191,16 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man, int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man) { struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man); + struct ttm_range_mgr_node *node; struct amdgpu_device *adev; - struct amdgpu_gtt_node *node; struct drm_mm_node *mm_node; int r = 0; adev = container_of(mgr, typeof(*adev), mman.gtt_mgr); spin_lock(&mgr->lock); drm_mm_for_each_node(mm_node, &mgr->mm) { - node = container_of(mm_node, typeof(*node), base.mm_nodes[0]); -
Re: [PATCH v3 1/4] drm/ttm: Create pinned list
Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky: This list will be used to capture all non VRAM BOs not on LRU so when device is hot unplugged we can iterate the list and unmap DMA mappings before device is removed. v2: Reanme function to ttm_bo_move_to_pinned v3: Move the pinned list to ttm device As far as I can see there is not list_del() remaining. So this won't work correctly. I suggest to rather rebase on top of the stuff I'm working on for a while to move the LRU into the resource instead. Just send out the latest patch set of this with you in CC. Christian. Signed-off-by: Andrey Grodzovsky Suggested-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++ drivers/gpu/drm/ttm/ttm_device.c | 1 + include/drm/ttm/ttm_device.h | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b950b45cf4b..1fedd0eb67ba 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) +{ + struct ttm_device *bdev = bo->bdev; + + list_move_tail(&bo->lru, &bdev->pinned); + + if (bdev->funcs->del_from_lru_notify) + bdev->funcs->del_from_lru_notify(bo); +} + +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { struct ttm_device *bdev = bo->bdev; @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, dma_resv_assert_held(bo->base.resv); if (bo->pin_count) { - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); return; } @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, return ret; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); list_del_init(&bo->ddestroy); spin_unlock(&bo->bdev->lru_lock); ttm_bo_cleanup_memtype_use(bo); @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, return 0; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); /* TODO: Cleanup the locking */ spin_unlock(&bo->bdev->lru_lock); diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 5f31acec3ad7..530a9c36be37 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs, INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue); spin_lock_init(&bdev->lru_lock); INIT_LIST_HEAD(&bdev->ddestroy); + INIT_LIST_HEAD(&bdev->pinned); bdev->dev_mapping = mapping; mutex_lock(&ttm_global_mutex); list_add_tail(&bdev->device_list, &glob->device_list); diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index cd592f8e941b..03fb44d061e0 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -265,6 +265,7 @@ struct ttm_device { */ spinlock_t lru_lock; struct list_head ddestroy; + struct list_head pinned; /* * Protected by load / firstopen / lastclose /unload sync.
Re: [PATCH v5 12/20] drm/msm: Use scheduler dependency handling
On Thu, Aug 05, 2021 at 12:46:57PM +0200, Daniel Vetter wrote: > drm_sched_job_init is already at the right place, so this boils down > to deleting code. > > Signed-off-by: Daniel Vetter > Cc: Rob Clark > Cc: Sean Paul > Cc: Sumit Semwal > Cc: "Christian König" > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > Cc: linux-me...@vger.kernel.org > Cc: linaro-mm-...@lists.linaro.org Merged up to this patch, except for etnaviv. -Daniel > --- > drivers/gpu/drm/msm/msm_gem.h| 5 - > drivers/gpu/drm/msm/msm_gem_submit.c | 19 +-- > drivers/gpu/drm/msm/msm_ringbuffer.c | 12 > 3 files changed, 5 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > index f9e3ffb2309a..8bf0ac707fd7 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -312,11 +312,6 @@ struct msm_gem_submit { > struct ww_acquire_ctx ticket; > uint32_t seqno; /* Sequence number of the submit on the ring */ > > - /* Array of struct dma_fence * to block on before submitting this job. > - */ > - struct xarray deps; > - unsigned long last_dep; > - > /* Hw fence, which is created when the scheduler executes the job, and >* is signaled when the hw finishes (via seqno write from cmdstream) >*/ > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > b/drivers/gpu/drm/msm/msm_gem_submit.c > index 96cea0ba4cfd..fb5a2eab27a2 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -52,8 +52,6 @@ static struct msm_gem_submit *submit_create(struct > drm_device *dev, > return ERR_PTR(ret); > } > > - xa_init_flags(&submit->deps, XA_FLAGS_ALLOC); > - > kref_init(&submit->ref); > submit->dev = dev; > submit->aspace = queue->ctx->aspace; > @@ -72,8 +70,6 @@ void __msm_gem_submit_destroy(struct kref *kref) > { > struct msm_gem_submit *submit = > container_of(kref, struct msm_gem_submit, ref); > - unsigned long index; > - struct dma_fence *fence; > unsigned i; > > if (submit->fence_id) { > @@ -82,12 +78,6 @@ void __msm_gem_submit_destroy(struct kref *kref) > mutex_unlock(&submit->queue->lock); > } > > - xa_for_each (&submit->deps, index, fence) { > - dma_fence_put(fence); > - } > - > - xa_destroy(&submit->deps); > - > dma_fence_put(submit->user_fence); > dma_fence_put(submit->hw_fence); > > @@ -343,8 +333,9 @@ static int submit_fence_sync(struct msm_gem_submit > *submit, bool no_implicit) > if (no_implicit) > continue; > > - ret = drm_gem_fence_array_add_implicit(&submit->deps, obj, > - write); > + ret = drm_sched_job_add_implicit_dependencies(&submit->base, > + obj, > + write); > if (ret) > break; > } > @@ -588,7 +579,7 @@ static struct drm_syncobj **msm_parse_deps(struct > msm_gem_submit *submit, > if (ret) > break; > > - ret = drm_gem_fence_array_add(&submit->deps, fence); > + ret = drm_sched_job_add_dependency(&submit->base, fence); > if (ret) > break; > > @@ -798,7 +789,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void > *data, > goto out_unlock; > } > > - ret = drm_gem_fence_array_add(&submit->deps, in_fence); > + ret = drm_sched_job_add_dependency(&submit->base, in_fence); > if (ret) > goto out_unlock; > } > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c > b/drivers/gpu/drm/msm/msm_ringbuffer.c > index bd54c1412649..652b1dedd7c1 100644 > --- a/drivers/gpu/drm/msm/msm_ringbuffer.c > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c > @@ -11,17 +11,6 @@ static uint num_hw_submissions = 8; > MODULE_PARM_DESC(num_hw_submissions, "The max # of jobs to write into > ringbuffer (default 8)"); > module_param(num_hw_submissions, uint, 0600); > > -static struct dma_fence *msm_job_dependency(struct drm_sched_job *job, > - struct drm_sched_entity *s_entity) > -{ > - struct msm_gem_submit *submit = to_msm_submit(job); > - > - if (!xa_empty(&submit->deps)) > - return xa_erase(&submit->deps, submit->last_dep++); > - > - return NULL; > -} > - > static struct dma_fence *msm_job_run(struct drm_sched_job *job) > { > struct msm_gem_submit *submit = to_msm_submit(job); > @@ -52,7 +41,6 @@ static void msm_job_free(struct drm_sched_job *job) > } > > const struct drm_sched_backend_ops msm_sched_ops = { > - .dependency = msm_job_dependency, > .run_job = msm_job
Re: [PATCH v5 16/20] drm/msm: Don't break exclusive fence ordering
On Thu, Aug 26, 2021 at 09:16:25AM -0700, Rob Clark wrote: > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter wrote: > > > > There's only one exclusive slot, and we must not break the ordering. > > > > Adding a new exclusive fence drops all previous fences from the > > dma_resv. To avoid violating the signalling order we err on the side of > > over-synchronizing by waiting for the existing fences, even if > > userspace asked us to ignore them. > > > > A better fix would be to us a dma_fence_chain or _array like e.g. > > amdgpu now uses, but > > - msm has a synchronous dma_fence_wait for anything from another > > context, so doesn't seem to care much, > > - and it probably makes sense to lift this into dma-resv.c code as a > > proper concept, so that drivers don't have to hack up their own > > solution each on their own. > > > > v2: Improve commit message per Lucas' suggestion. > > > > Cc: Lucas Stach > > Signed-off-by: Daniel Vetter > > Cc: Rob Clark > > Cc: Sean Paul > > Cc: linux-arm-...@vger.kernel.org > > Cc: freedr...@lists.freedesktop.org > > a-b Also pushed to drm-misc-next, thanks for review&testing. -Daniel > > > --- > > drivers/gpu/drm/msm/msm_gem_submit.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > index fb5a2eab27a2..66633dfd58a2 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -330,7 +330,8 @@ static int submit_fence_sync(struct msm_gem_submit > > *submit, bool no_implicit) > > return ret; > > } > > > > - if (no_implicit) > > + /* exclusive fences must be ordered */ > > + if (no_implicit && !write) > > continue; > > > > ret = drm_sched_job_add_implicit_dependencies(&submit->base, > > -- > > 2.32.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/ttm: Fix ttm_bo_move_memcpy() for subclassed struct ttm_resource
On Mon, Aug 30, 2021 at 09:48:35AM +0200, Thomas Hellström wrote: > The code was making a copy of a struct ttm_resource. However, > recently the struct ttm_resources were allowed to be subclassed and > also were allowed to be malloced, hence the driver could end up assuming > the copy we handed it was subclassed and worse, the original could have > been freed at this point. > > Fix this by using the original struct ttm_resource before it is > potentially freed in ttm_bo_move_sync_cleanup() > > Reported-by: Ben Skeggs > Reported-by: Dave Airlie > Cc: Christian König > Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based > iomem") > Signed-off-by: Thomas Hellström This doesn't apply cleanly to drm-misc-next-fixes, do we need a backmerge or something? Can you pls coordinate with Maarten and then also poke for a pull request so this isn't stuck? Also since 5.14 is released this needs cc: stable. -Daniel > --- > drivers/gpu/drm/ttm/ttm_bo_util.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c > b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 5c20d0541cc3..c893c3db2623 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -139,7 +139,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, > struct ttm_resource *src_mem = bo->resource; > struct ttm_resource_manager *src_man = > ttm_manager_type(bdev, src_mem->mem_type); > - struct ttm_resource src_copy = *src_mem; > union { > struct ttm_kmap_iter_tt tt; > struct ttm_kmap_iter_linear_io io; > @@ -173,11 +172,10 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, > if (!(clear && ttm && !(ttm->page_flags & TTM_PAGE_FLAG_ZERO_ALLOC))) > ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter); > > - src_copy = *src_mem; > + if (!src_iter->ops->maps_tt) > + ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem); > ttm_bo_move_sync_cleanup(bo, dst_mem); > > - if (!src_iter->ops->maps_tt) > - ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, &src_copy); > out_src_iter: > if (!dst_iter->ops->maps_tt) > ttm_kmap_iter_linear_io_fini(&_dst_iter.io, bdev, dst_mem); > -- > 2.31.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 2/2] drm/bridge: it66121: Wait for next bridge to be probed
On 27/08/2021 18:39, Paul Cercueil wrote: > If run before the next bridge is initialized, of_drm_find_bridge() will > give us a NULL pointer. > > If that's the case, return -EPROBE_DEFER; we may have more luck next > time. > Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver") > Signed-off-by: Paul Cercueil > --- > drivers/gpu/drm/bridge/ite-it66121.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/ite-it66121.c > b/drivers/gpu/drm/bridge/ite-it66121.c > index b130d01147c6..9dc41a7b9136 100644 > --- a/drivers/gpu/drm/bridge/ite-it66121.c > +++ b/drivers/gpu/drm/bridge/ite-it66121.c > @@ -924,6 +924,9 @@ static int it66121_probe(struct i2c_client *client, > ctx->next_bridge = of_drm_find_bridge(ep); > of_node_put(ep); > > + if (!ctx->next_bridge) > + return -EPROBE_DEFER; > + > i2c_set_clientdata(client, ctx); > mutex_init(&ctx->lock); > > Reviewed-by: Neil Armstrong Thanks, Neil
Re: [PATCH 1/2] drm/bridge: it66121: Initialize {device,vendor}_ids
On 27/08/2021 18:39, Paul Cercueil wrote: > These two arrays are populated with data read from the I2C device > through regmap_read(), and the data is then compared with hardcoded > vendor/product ID values of supported chips. > > However, the return value of regmap_read() was never checked. This is > fine, as long as the two arrays are zero-initialized, so that we don't > compare the vendor/product IDs against whatever garbage is left on the > stack. > > Address this issue by zero-initializing these two arrays. > Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver") > Signed-off-by: Paul Cercueil > --- > drivers/gpu/drm/bridge/ite-it66121.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/ite-it66121.c > b/drivers/gpu/drm/bridge/ite-it66121.c > index 2f2a09adb4bc..b130d01147c6 100644 > --- a/drivers/gpu/drm/bridge/ite-it66121.c > +++ b/drivers/gpu/drm/bridge/ite-it66121.c > @@ -889,7 +889,7 @@ static irqreturn_t it66121_irq_threaded_handler(int irq, > void *dev_id) > static int it66121_probe(struct i2c_client *client, >const struct i2c_device_id *id) > { > - u32 vendor_ids[2], device_ids[2], revision_id; > + u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 }; > struct device_node *ep; > int ret; > struct it66121_ctx *ctx; > Reviewed-by: Neil Armstrong Thanks, Neil
[PATCH v2 1/2] drm/probe-helper: Create a HPD IRQ event helper for a single connector
The drm_helper_hpd_irq_event() function is iterating over all the connectors when an hotplug event is detected. During that iteration, it will call each connector detect function and figure out if its status changed. Finally, if any connector changed, it will notify the user-space and the clients that something changed on the DRM device. This is supposed to be used for drivers that don't have a hotplug interrupt for individual connectors. However, drivers that can use an interrupt for a single connector are left in the dust and can either reimplement the logic used during the iteration for each connector or use that helper and iterate over all connectors all the time. Since both are suboptimal, let's create a helper that will only perform the status detection on a single connector. Signed-off-by: Maxime Ripard --- Changes from v1: - Rename the shared function - Move the hotplug event notification out of the shared function - Added missing locks - Improve the documentation - Switched to drm_dbg_kms --- drivers/gpu/drm/drm_probe_helper.c | 120 - include/drm/drm_probe_helper.h | 1 + 2 files changed, 86 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 5606bca3caa8..fcf32ec0b0c8 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -795,6 +795,86 @@ void drm_kms_helper_poll_fini(struct drm_device *dev) } EXPORT_SYMBOL(drm_kms_helper_poll_fini); +static bool check_connector_changed(struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + enum drm_connector_status old_status; + u64 old_epoch_counter; + bool changed = false; + + /* Only handle HPD capable connectors. */ + drm_WARN_ON(dev, !(connector->polled & DRM_CONNECTOR_POLL_HPD)); + + drm_WARN_ON(dev, !mutex_is_locked(&dev->mode_config.mutex)); + + old_status = connector->status; + old_epoch_counter = connector->epoch_counter; + + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Old epoch counter %llu\n", + connector->base.id, + connector->name, + old_epoch_counter); + + connector->status = drm_helper_probe_detect(connector, NULL, false); + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] status updated from %s to %s\n", + connector->base.id, + connector->name, + drm_get_connector_status_name(old_status), + drm_get_connector_status_name(connector->status)); + + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] New epoch counter %llu\n", + connector->base.id, + connector->name, + connector->epoch_counter); + + /* +* Check if epoch counter had changed, meaning that we need +* to send a uevent. +*/ + if (old_epoch_counter != connector->epoch_counter) + changed = true; + + return changed; +} + +/** + * drm_connector_helper_hpd_irq_event - hotplug processing + * @connector: drm_connector + * + * Drivers can use this helper function to run a detect cycle on a connector + * which has the DRM_CONNECTOR_POLL_HPD flag set in its &polled member. + * + * This helper function is useful for drivers which can track hotplug + * interrupts for a single connector. Drivers that want to send a + * hotplug event for all connectors or can't track hotplug interrupts + * per connector need to use drm_helper_hpd_irq_event(). + * + * This function must be called from process context with no mode + * setting locks held. + * + * Note that a connector can be both polled and probed from the hotplug + * handler, in case the hotplug interrupt is known to be unreliable. + */ +bool drm_connector_helper_hpd_irq_event(struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + bool changed; + + mutex_lock(&dev->mode_config.mutex); + changed = check_connector_changed(connector); + mutex_unlock(&dev->mode_config.mutex); + + if (changed) { + drm_kms_helper_hotplug_event(dev); + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Sent hotplug event\n", + connector->base.id, + connector->name); + } + + return changed; +} +EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event); + /** * drm_helper_hpd_irq_event - hotplug processing * @dev: drm_device @@ -808,9 +888,10 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini); * interrupts for each connector. * * Drivers which support hotplug interrupts for each connector individually and - * which have a more fine-grained detect logic should bypass this code and - * directly call drm_kms_helper_hotplug_event() in case the connector state - * changed. + * which have a more fine-grained detect logic can use + * drm_connector_helper_hpd_irq_event(). Alternatively, they sh
[PATCH] dma-buf: heaps: remove duplicated cache sync
From: Guangming Cao Current flow, one dmabuf maybe call cache sync many times if it has beed mapped more than one time. Is there any case that attachments of one dmabuf will points to different memory? If not, seems do sync only one time is more better. Signed-off-by: Guangming Cao --- drivers/dma-buf/heaps/system_heap.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 23a7e74ef966..909ef652a8c8 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -162,9 +162,10 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, invalidate_kernel_vmap_range(buffer->vaddr, buffer->len); list_for_each_entry(a, &buffer->attachments, list) { - if (!a->mapped) - continue; - dma_sync_sgtable_for_cpu(a->dev, a->table, direction); + if (a->mapped) { + dma_sync_sgtable_for_cpu(a->dev, a->table, direction); + break; + } } mutex_unlock(&buffer->lock); @@ -183,9 +184,10 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf, flush_kernel_vmap_range(buffer->vaddr, buffer->len); list_for_each_entry(a, &buffer->attachments, list) { - if (!a->mapped) - continue; - dma_sync_sgtable_for_device(a->dev, a->table, direction); + if (!a->mapped) { + dma_sync_sgtable_for_device(a->dev, a->table, direction); + break; + } } mutex_unlock(&buffer->lock); -- 2.17.1
Re: [PATCH V4] fbcon: fix fbcon out-of-bounds write in sys_imageblit
On 2021/08/30 17:16, Daniel Vetter wrote: > On Mon, Aug 30, 2021 at 11:30:23AM +0800, tcs.ker...@gmail.com wrote: >> From: Haimin Zhang >> >> yres and vyres can be controlled by user mode parameters, and cause >> p->vrows to become a negative value. While this value be passed to real_y >> function, the ypos will be out of screen range.This is an out-of-bounds >> write bug. >> some driver will check xres and yres in fb_check_var callback,but some not >> so we add a common check after that callback. >> >> Signed-off-by: Haimin Zhang >> Signed-off-by: Tetsuo Handa Please s/Signed-off-by: Tetsuo Handa/Suggested-by: Tetsuo Handa/ . It is Haimin who debugged this problem and wrote this patch. > > Does this fix a syzbot crash or how was this discovered? Yes, Haimin's team is running syzkaller locally and found this bug. Therefore, no Reported-by: syzbot tag for this patch. Forwarded Message Subject: Re: [PATCH v2] fbcon: fix Out-Of-Bounds write in sys_imageblit Message-ID: <33fc0e30-b94c-939f-a708-4b939af43...@gmail.com> Date: Mon, 2 Aug 2021 14:50:24 +0800 hi, Tetsuo Handa i made a test with your suggested code, it can block the out-of-bound bug. where to add this check logic, i suggest to add it after the driver's fb_check_var callback.because what we plan to add is a common check by framework,but some driver can fault-tolerant invalid parameters(like yres_virtual > yres) /* exist common check */ if (var->xres < 8 || var->yres < 8) return -EINVAL; /* callback to drivers, some driver can fix invalid virtual xres or virtual yres */ ret = info->fbops->fb_check_var(var, info); if (ret) return ret; /* we add a more check here, if some drivers can't fix invalid x,y virtual values, we return a -EINVAL */ if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) return -EINVAL; how about this fix ? i can make a v3 patch. diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 1c85514..9fb7e94 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1012,6 +1012,10 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var, if (ret) return ret; + + /* Virtual resolution cannot be smaller than visible resolution. */ + if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) + return -EINVAL; if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW) return 0; Forwarded Message Subject: Re: [PATCH]fbcon:fix Out-Of-Bounds write in sys_imageblit Message-ID: Date: Wed, 28 Jul 2021 11:10:52 +0800 this is my debug log: 1. environment linux kernel: git checkout a4c30b8691f26c6115db6e11ec837c1fb6073953 qemu version:QEMU emulator version 6.0.90 (v6.1.0-rc0-99-g76bf66b913) qemu-system-x86_64 -smp 4 -kernel linux_allyes/build/arch/x86/boot/bzImage -m 4G \ -hda /data/h_image/linux_debug/buster1.img -serial stdio \ -append "earlyprintk=serial console=ttyS0 root=/dev/sda nokaslr" \ -enable-kvm -cpu host -display none -net nic -net user,hostfwd=tcp::1236-:22 -s -S 1.kasan report: == BUG: KASAN: vmalloc-out-of-bounds in sys_imageblit+0x12f4/0x1430 Write of size 4 at addr c9000b8c1fe0 by task syz-executor.7/29927 CPU: 0 PID: 29927 Comm: syz-executor.7 Tainted: GE 5.13.0-rc5+ #2 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 Call Trace: dump_stack+0x141/0x1d7 print_address_description.constprop.0.cold+0x5/0x2f8 kasan_report.cold+0x7c/0xd8 sys_imageblit+0x12f4/0x1430 drm_fbdev_fb_imageblit+0x15c/0x350 soft_cursor+0x514/0xa30 bit_cursor+0xd07/0x1740 fbcon_cursor+0x51d/0x630 hide_cursor+0x85/0x280 putconsxy+0x1d/0x450 vcs_write+0x9cf/0xb90 vfs_write+0x28e/0xa30 ksys_write+0x12d/0x250 do_syscall_64+0x3a/0xb0 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x7f6c6d34cf59 Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48 RSP: 002b:7f6c6a832d98 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 0069c038 RCX: 7f6c6d34cf59 RDX: 008d RSI: 2040 RDI: 0003 RBP: 0069c038 R08: R09: R10: R11: 0246 R12: 0069c044 R13: 7ffd72091c8f R14: 0069c038 R15: 0001 Memory state around the buggy address: c9000b8c1e80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 c9000b8c1f00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 >c9000b8c1f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
[PATCH 0/5] drm/displayid: VESA vendor block and drm/i915 MSO use of it
We need the DisplayID VESA vendor block data for properly configuring eDP MSO (Multi-SST Operation) pixel overlap. I haven't actually tested this on a panel that requires the overlap, but this is all pretty straightforward to prepare for that use case. BR, Jani. Jani Nikula (5): drm/displayid: re-align data block macros drm/displayid: add DisplayID v2.0 data blocks and primary use cases drm/edid: parse the DisplayID v2.0 VESA vendor block for MSO drm/i915/edp: postpone MSO init until after EDID read drm/i915/edp: use MSO pixel overlap from DisplayID data drivers/gpu/drm/drm_edid.c | 63 drivers/gpu/drm/i915/display/intel_dp.c | 14 ++-- include/drm/drm_connector.h | 12 +++ include/drm/drm_displayid.h | 99 + 4 files changed, 154 insertions(+), 34 deletions(-) -- 2.20.1
[PATCH 1/5] drm/displayid: re-align data block macros
Make the values easier to read. Also add DisplayID Structure version and revision information (this is different from the spec version). Signed-off-by: Jani Nikula --- include/drm/drm_displayid.h | 57 +++-- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index ec64d141f578..0ed9445b5482 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -26,35 +26,36 @@ struct edid; -#define DATA_BLOCK_PRODUCT_ID 0x00 -#define DATA_BLOCK_DISPLAY_PARAMETERS 0x01 -#define DATA_BLOCK_COLOR_CHARACTERISTICS 0x02 -#define DATA_BLOCK_TYPE_1_DETAILED_TIMING 0x03 -#define DATA_BLOCK_TYPE_2_DETAILED_TIMING 0x04 -#define DATA_BLOCK_TYPE_3_SHORT_TIMING 0x05 -#define DATA_BLOCK_TYPE_4_DMT_TIMING 0x06 -#define DATA_BLOCK_VESA_TIMING 0x07 -#define DATA_BLOCK_CEA_TIMING 0x08 -#define DATA_BLOCK_VIDEO_TIMING_RANGE 0x09 -#define DATA_BLOCK_PRODUCT_SERIAL_NUMBER 0x0a -#define DATA_BLOCK_GP_ASCII_STRING 0x0b -#define DATA_BLOCK_DISPLAY_DEVICE_DATA 0x0c -#define DATA_BLOCK_INTERFACE_POWER_SEQUENCING 0x0d -#define DATA_BLOCK_TRANSFER_CHARACTERISTICS 0x0e -#define DATA_BLOCK_DISPLAY_INTERFACE 0x0f -#define DATA_BLOCK_STEREO_DISPLAY_INTERFACE 0x10 -#define DATA_BLOCK_TILED_DISPLAY 0x12 -#define DATA_BLOCK_CTA 0x81 +/* DisplayID Structure v1r2 Data Blocks */ +#define DATA_BLOCK_PRODUCT_ID 0x00 +#define DATA_BLOCK_DISPLAY_PARAMETERS 0x01 +#define DATA_BLOCK_COLOR_CHARACTERISTICS 0x02 +#define DATA_BLOCK_TYPE_1_DETAILED_TIMING 0x03 +#define DATA_BLOCK_TYPE_2_DETAILED_TIMING 0x04 +#define DATA_BLOCK_TYPE_3_SHORT_TIMING 0x05 +#define DATA_BLOCK_TYPE_4_DMT_TIMING 0x06 +#define DATA_BLOCK_VESA_TIMING 0x07 +#define DATA_BLOCK_CEA_TIMING 0x08 +#define DATA_BLOCK_VIDEO_TIMING_RANGE 0x09 +#define DATA_BLOCK_PRODUCT_SERIAL_NUMBER 0x0a +#define DATA_BLOCK_GP_ASCII_STRING 0x0b +#define DATA_BLOCK_DISPLAY_DEVICE_DATA 0x0c +#define DATA_BLOCK_INTERFACE_POWER_SEQUENCING 0x0d +#define DATA_BLOCK_TRANSFER_CHARACTERISTICS0x0e +#define DATA_BLOCK_DISPLAY_INTERFACE 0x0f +#define DATA_BLOCK_STEREO_DISPLAY_INTERFACE0x10 +#define DATA_BLOCK_TILED_DISPLAY 0x12 +#define DATA_BLOCK_VENDOR_SPECIFIC 0x7f +#define DATA_BLOCK_CTA 0x81 -#define DATA_BLOCK_VENDOR_SPECIFIC 0x7f - -#define PRODUCT_TYPE_EXTENSION 0 -#define PRODUCT_TYPE_TEST 1 -#define PRODUCT_TYPE_PANEL 2 -#define PRODUCT_TYPE_MONITOR 3 -#define PRODUCT_TYPE_TV 4 -#define PRODUCT_TYPE_REPEATER 5 -#define PRODUCT_TYPE_DIRECT_DRIVE 6 +/* DisplayID Structure v1r2 Product Type */ +#define PRODUCT_TYPE_EXTENSION 0 +#define PRODUCT_TYPE_TEST 1 +#define PRODUCT_TYPE_PANEL 2 +#define PRODUCT_TYPE_MONITOR 3 +#define PRODUCT_TYPE_TV4 +#define PRODUCT_TYPE_REPEATER 5 +#define PRODUCT_TYPE_DIRECT_DRIVE 6 struct displayid_header { u8 rev; -- 2.20.1
[PATCH 3/5] drm/edid: parse the DisplayID v2.0 VESA vendor block for MSO
The VESA Organization Vendor-Specific Data Block, defined in VESA DisplayID Standard v2.0, specifies the eDP Multi-SST Operation (MSO) stream count and segment pixel overlap. DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension, describing how DisplayID sections may be embedded in EDID extension blocks. DisplayID v2.0 does not have such a section, perhaps implying that DisplayID v2.0 data should not be included in EDID extensions, but rather in a "pure" DisplayID structure at its own DDC address pair A4h/A5h, as described in VESA E-DDC Standard v1.3 chapter 3. However, in practice, displays out in the field have embedded DisplayID v2.0 data blocks in EDID extensions, including, in particular, some eDP MSO displays, where a pure DisplayID structure is not available at all. Parse the MSO data from the DisplayID data block. Do it as part of drm_add_display_info(), extending it to parse also DisplayID data to avoid requiring extra calls to update the information. Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_edid.c | 63 + include/drm/drm_connector.h | 12 +++ include/drm/drm_displayid.h | 11 +++ 3 files changed, 86 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6325877c5fd6..7e8083068f3f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -28,6 +28,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include #include #include @@ -5148,6 +5149,62 @@ void drm_get_monitor_range(struct drm_connector *connector, info->monitor_range.max_vfreq); } +static void drm_parse_vesa_mso_data(struct drm_connector *connector, + const struct displayid_block *block) +{ + struct displayid_vesa_vendor_specific_block *vesa = + (struct displayid_vesa_vendor_specific_block *)block; + struct drm_display_info *info = &connector->display_info; + + if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { + drm_dbg_kms(connector->dev, "Unexpected VESA vendor block size\n"); + return; + } + + switch (FIELD_GET(DISPLAYID_VESA_MSO_MODE, vesa->mso)) { + default: + drm_dbg_kms(connector->dev, "Reserved MSO mode value\n"); + fallthrough; + case 0: + info->mso_stream_count = 0; + break; + case 1: + info->mso_stream_count = 2; /* 2 or 4 links */ + break; + case 2: + info->mso_stream_count = 4; /* 4 links */ + break; + } + + if (!info->mso_stream_count) { + info->mso_pixel_overlap = 0; + return; + } + + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso); + if (info->mso_pixel_overlap > 8) { + drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value %u\n", + info->mso_pixel_overlap); + info->mso_pixel_overlap = 8; + } + + drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n", + info->mso_stream_count, info->mso_pixel_overlap); +} + +static void drm_update_mso(struct drm_connector *connector, const struct edid *edid) +{ + const struct displayid_block *block; + struct displayid_iter iter; + + displayid_iter_edid_begin(edid, &iter); + displayid_iter_for_each(block, &iter) { + if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC) + drm_parse_vesa_mso_data(connector, block); + } + displayid_iter_end(&iter); +} + /* A connector has no EDID information, so we've got no EDID to compute quirks from. Reset * all of the values which would have been set from EDID */ @@ -5171,6 +5228,9 @@ drm_reset_display_info(struct drm_connector *connector) info->non_desktop = 0; memset(&info->monitor_range, 0, sizeof(info->monitor_range)); + + info->mso_stream_count = 0; + info->mso_pixel_overlap = 0; } u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) @@ -5249,6 +5309,9 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422) info->color_formats |= DRM_COLOR_FORMAT_YCRCB422; + + drm_update_mso(connector, edid); + return quirks; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 79fa34e5ccdb..379746d3266f 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -590,6 +590,18 @@ struct drm_display_info { * @monitor_range: Frequency range supported by monitor range descriptor */ struct drm_monitor_range_info monitor_range; + + /** +* @mso_stream_count: eDP Multi-SST Operation (
[PATCH 4/5] drm/i915/edp: postpone MSO init until after EDID read
MSO will require segment pixel overlap information from the EDID. Postpone MSO init until after we've read and cached the EDID. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7f8e8865048f..8e75543334c2 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2575,8 +2575,6 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp) */ intel_edp_init_source_oui(intel_dp, true); - intel_edp_mso_init(intel_dp); - return true; } @@ -5269,6 +5267,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, if (fixed_mode) downclock_mode = intel_dp_drrs_init(intel_connector, fixed_mode); + /* MSO requires information from the EDID */ + intel_edp_mso_init(intel_dp); + /* multiply the mode clock and horizontal timings for MSO */ intel_edp_mso_mode_fixup(intel_connector, fixed_mode); intel_edp_mso_mode_fixup(intel_connector, downclock_mode); -- 2.20.1
[PATCH 5/5] drm/i915/edp: use MSO pixel overlap from DisplayID data
Now that we have MSO pixel overlap in display info, use it. Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_dp.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 8e75543334c2..0d7c9eadca08 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2459,6 +2459,8 @@ static void intel_edp_mso_mode_fixup(struct intel_connector *connector, static void intel_edp_mso_init(struct intel_dp *intel_dp) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); + struct intel_connector *connector = intel_dp->attached_connector; + struct drm_display_info *info = &connector->base.display_info; u8 mso; if (intel_dp->edp_dpcd[0] < DP_EDP_14) @@ -2477,8 +2479,9 @@ static void intel_edp_mso_init(struct intel_dp *intel_dp) } if (mso) { - drm_dbg_kms(&i915->drm, "Sink MSO %ux%u configuration\n", - mso, drm_dp_max_lane_count(intel_dp->dpcd) / mso); + drm_dbg_kms(&i915->drm, "Sink MSO %ux%u configuration, pixel overlap %u\n", + mso, drm_dp_max_lane_count(intel_dp->dpcd) / mso, + info->mso_pixel_overlap); if (!HAS_MSO(i915)) { drm_err(&i915->drm, "No source MSO support, disabling\n"); mso = 0; @@ -2486,7 +2489,7 @@ static void intel_edp_mso_init(struct intel_dp *intel_dp) } intel_dp->mso_link_count = mso; - intel_dp->mso_pixel_overlap = 0; /* FIXME: read from DisplayID v2.0 */ + intel_dp->mso_pixel_overlap = mso ? info->mso_pixel_overlap : 0; } static bool -- 2.20.1
[PATCH 2/5] drm/displayid: add DisplayID v2.0 data blocks and primary use cases
DisplayID v2.0 changes the data block identifiers and product types (now called primary use cases). Signed-off-by: Jani Nikula --- include/drm/drm_displayid.h | 29 + 1 file changed, 29 insertions(+) diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 0ed9445b5482..79771091771a 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -26,6 +26,10 @@ struct edid; +/* DisplayID Structure versions */ +#define DISPLAY_ID_STRUCTURE_VER_120x12 +#define DISPLAY_ID_STRUCTURE_VER_200x20 + /* DisplayID Structure v1r2 Data Blocks */ #define DATA_BLOCK_PRODUCT_ID 0x00 #define DATA_BLOCK_DISPLAY_PARAMETERS 0x01 @@ -48,6 +52,20 @@ struct edid; #define DATA_BLOCK_VENDOR_SPECIFIC 0x7f #define DATA_BLOCK_CTA 0x81 +/* DisplayID Structure v2r0 Data Blocks */ +#define DATA_BLOCK_2_PRODUCT_ID0x20 +#define DATA_BLOCK_2_DISPLAY_PARAMETERS0x21 +#define DATA_BLOCK_2_TYPE_7_DETAILED_TIMING0x22 +#define DATA_BLOCK_2_TYPE_8_ENUMERATED_TIMING 0x23 +#define DATA_BLOCK_2_TYPE_9_FORMULA_TIMING 0x24 +#define DATA_BLOCK_2_DYNAMIC_VIDEO_TIMING 0x25 +#define DATA_BLOCK_2_DISPLAY_INTERFACE_FEATURES0x26 +#define DATA_BLOCK_2_STEREO_DISPLAY_INTERFACE 0x27 +#define DATA_BLOCK_2_TILED_DISPLAY_TOPOLOGY0x28 +#define DATA_BLOCK_2_CONTAINER_ID 0x29 +#define DATA_BLOCK_2_VENDOR_SPECIFIC 0x7e +#define DATA_BLOCK_2_CTA_DISPLAY_ID0x81 + /* DisplayID Structure v1r2 Product Type */ #define PRODUCT_TYPE_EXTENSION 0 #define PRODUCT_TYPE_TEST 1 @@ -57,6 +75,17 @@ struct edid; #define PRODUCT_TYPE_REPEATER 5 #define PRODUCT_TYPE_DIRECT_DRIVE 6 +/* DisplayID Structure v2r0 Display Product Primary Use Case (~Product Type) */ +#define PRIMARY_USE_EXTENSION 0 +#define PRIMARY_USE_TEST 1 +#define PRIMARY_USE_GENERIC2 +#define PRIMARY_USE_TV 3 +#define PRIMARY_USE_DESKTOP_PRODUCTIVITY 4 +#define PRIMARY_USE_DESKTOP_GAMING 5 +#define PRIMARY_USE_PRESENTATION 6 +#define PRIMARY_USE_HEAD_MOUNTED_VR7 +#define PRIMARY_USE_HEAD_MOUNTED_AR8 + struct displayid_header { u8 rev; u8 bytes; -- 2.20.1
Re: [PATCH 3/5] drm/edid: parse the DisplayID v2.0 VESA vendor block for MSO
On Mon, 30 Aug 2021, Jani Nikula wrote: > The VESA Organization Vendor-Specific Data Block, defined in VESA > DisplayID Standard v2.0, specifies the eDP Multi-SST Operation (MSO) > stream count and segment pixel overlap. > > DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension, > describing how DisplayID sections may be embedded in EDID extension > blocks. DisplayID v2.0 does not have such a section, perhaps implying > that DisplayID v2.0 data should not be included in EDID extensions, but > rather in a "pure" DisplayID structure at its own DDC address pair > A4h/A5h, as described in VESA E-DDC Standard v1.3 chapter 3. > > However, in practice, displays out in the field have embedded DisplayID > v2.0 data blocks in EDID extensions, including, in particular, some eDP > MSO displays, where a pure DisplayID structure is not available at all. > > Parse the MSO data from the DisplayID data block. Do it as part of > drm_add_display_info(), extending it to parse also DisplayID data to > avoid requiring extra calls to update the information. For reference, this is the EDID from a Lenovo ThinkPad X1 with eDP MSO display. AFAICT, the display does not respond on A4h/A5h at all, it only has the usual EDID at the usual DDC address. BR, Jani. edid-decode (hex): 00 ff ff ff ff ff ff 00 06 af 13 10 00 00 00 00 00 1c 01 04 a5 1c 13 78 02 ee 95 a3 54 4c 99 26 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 d5 2b 68 50 40 e0 2c 50 18 10 3a 00 1c bd 10 00 00 18 00 00 00 0f 00 00 00 00 00 00 00 00 00 00 00 00 00 20 00 00 00 fe 00 41 55 4f 0a 20 20 20 20 20 20 20 20 20 00 00 00 fe 00 42 31 33 35 51 41 4e 30 31 2e 30 20 0a 01 e1 70 20 08 06 00 7e 00 05 3a 02 92 00 20 61 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90 Block 0, Base EDID: EDID Structure Version & Revision: 1.4 Vendor & Product Identification: Manufacturer: AUO Model: 4115 Made in: 2018 Basic Display Parameters & Features: Digital display Bits per primary color channel: 8 DisplayPort interface Maximum image size: 28 cm x 19 cm Gamma: 2.20 Supported color formats: RGB 4:4:4 First detailed timing includes the native pixel format and preferred refresh rate Color Characteristics: Red : 0.6396, 0.3300 Green: 0.2998, 0.5996 Blue : 0.1503, 0.0595 White: 0.3134, 0.3291 Established Timings I & II: none Standard Timings: none Detailed Timing Descriptors: DTD 1: 1128x1504 60.006 Hz 3:492.889 kHz 112.210 MHz (284 mm x 189 mm) Hfront 24 Hsync 16 Hback 40 Hpol N Vfront3 Vsync 10 Vback 31 Vpol N Manufacturer-Specified Display Descriptor (0x0f): 00 0f 00 00 00 00 00 00 00 00 00 00 00 00 00 20 '... ' Alphanumeric Data String: 'AUO' Alphanumeric Data String: 'B135QAN01.0 ' Extension blocks: 1 Checksum: 0xe1 Block 1, DisplayID Extension Block: Version: 2.0 Extension Count: 0 Display Product Primary Use Case: Presentation display Vendor-Specific Data Block (VESA): Data Structure Type: eDP Default Colorspace and EOTF Handling: sRGB Number of Pixels in Hor Pix Cnt Overlapping an Adjacent Panel: 0 Multi-SST Operation: Two Streams (number of links shall be 2 or 4) Checksum: 0x61 Checksum: 0x90 -- Jani Nikula, Intel Open Source Graphics Center
[PATCH AUTOSEL 5.13 01/14] gpu: ipu-v3: Fix i.MX IPU-v3 offset calculations for (semi)planar U/V formats
From: Krzysztof Hałasa [ Upstream commit 7cca7c8096e2c8a4149405438329b5035d0744f0 ] Video captured in 1400x1050 resolution (bytesperline aka stride = 1408 bytes) is invalid. Fix it. Signed-off-by: Krzysztof Halasa Link: https://lore.kernel.org/r/m3y2bmq7a4@t19.piap.pl [p.za...@pengutronix.de: added "gpu: ipu-v3:" prefix to commit description] Signed-off-by: Philipp Zabel Signed-off-by: Sasha Levin --- drivers/gpu/ipu-v3/ipu-cpmem.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c index a1c85d1521f5..82b244cb313e 100644 --- a/drivers/gpu/ipu-v3/ipu-cpmem.c +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c @@ -585,21 +585,21 @@ static const struct ipu_rgb def_bgra_16 = { .bits_per_pixel = 16, }; -#define Y_OFFSET(pix, x, y)((x) + pix->width * (y)) -#define U_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define V_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * pix->height / 4) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define U2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * (y) / 2) + (x) / 2) -#define V2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * pix->height / 2) + \ -(pix->width * (y) / 2) + (x) / 2) -#define UV_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * ((y) / 2)) + (x)) -#define UV2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * y) + (x)) +#define Y_OFFSET(pix, x, y)((x) + pix->bytesperline * (y)) +#define U_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define V_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 4) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define U2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define V2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 2) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define UV_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2)) + (x)) +#define UV2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * y) + (x)) #define NUM_ALPHA_CHANNELS 7 -- 2.30.2
[PATCH AUTOSEL 5.10 01/11] gpu: ipu-v3: Fix i.MX IPU-v3 offset calculations for (semi)planar U/V formats
From: Krzysztof Hałasa [ Upstream commit 7cca7c8096e2c8a4149405438329b5035d0744f0 ] Video captured in 1400x1050 resolution (bytesperline aka stride = 1408 bytes) is invalid. Fix it. Signed-off-by: Krzysztof Halasa Link: https://lore.kernel.org/r/m3y2bmq7a4@t19.piap.pl [p.za...@pengutronix.de: added "gpu: ipu-v3:" prefix to commit description] Signed-off-by: Philipp Zabel Signed-off-by: Sasha Levin --- drivers/gpu/ipu-v3/ipu-cpmem.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c index a1c85d1521f5..82b244cb313e 100644 --- a/drivers/gpu/ipu-v3/ipu-cpmem.c +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c @@ -585,21 +585,21 @@ static const struct ipu_rgb def_bgra_16 = { .bits_per_pixel = 16, }; -#define Y_OFFSET(pix, x, y)((x) + pix->width * (y)) -#define U_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define V_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * pix->height / 4) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define U2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * (y) / 2) + (x) / 2) -#define V2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * pix->height / 2) + \ -(pix->width * (y) / 2) + (x) / 2) -#define UV_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * ((y) / 2)) + (x)) -#define UV2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * y) + (x)) +#define Y_OFFSET(pix, x, y)((x) + pix->bytesperline * (y)) +#define U_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define V_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 4) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define U2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define V2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 2) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define UV_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2)) + (x)) +#define UV2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * y) + (x)) #define NUM_ALPHA_CHANNELS 7 -- 2.30.2
[PATCH AUTOSEL 5.4 01/10] gpu: ipu-v3: Fix i.MX IPU-v3 offset calculations for (semi)planar U/V formats
From: Krzysztof Hałasa [ Upstream commit 7cca7c8096e2c8a4149405438329b5035d0744f0 ] Video captured in 1400x1050 resolution (bytesperline aka stride = 1408 bytes) is invalid. Fix it. Signed-off-by: Krzysztof Halasa Link: https://lore.kernel.org/r/m3y2bmq7a4@t19.piap.pl [p.za...@pengutronix.de: added "gpu: ipu-v3:" prefix to commit description] Signed-off-by: Philipp Zabel Signed-off-by: Sasha Levin --- drivers/gpu/ipu-v3/ipu-cpmem.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c index a1c85d1521f5..82b244cb313e 100644 --- a/drivers/gpu/ipu-v3/ipu-cpmem.c +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c @@ -585,21 +585,21 @@ static const struct ipu_rgb def_bgra_16 = { .bits_per_pixel = 16, }; -#define Y_OFFSET(pix, x, y)((x) + pix->width * (y)) -#define U_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define V_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * pix->height / 4) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define U2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * (y) / 2) + (x) / 2) -#define V2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * pix->height / 2) + \ -(pix->width * (y) / 2) + (x) / 2) -#define UV_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * ((y) / 2)) + (x)) -#define UV2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * y) + (x)) +#define Y_OFFSET(pix, x, y)((x) + pix->bytesperline * (y)) +#define U_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define V_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 4) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define U2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define V2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 2) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define UV_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2)) + (x)) +#define UV2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * y) + (x)) #define NUM_ALPHA_CHANNELS 7 -- 2.30.2
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
Hi Testsuo, On Mon, Aug 30, 2021 at 4:27 AM Tetsuo Handa wrote: > On 2021/08/30 9:24, Randy Dunlap wrote: > > Note that yres_virtual is set to 0x1000. Is there no practical limit > > (hence limit check) that can be used here? > > > > Also, in vga16fb_check_var(), beginning at line 404: > > > > 404if (yres > vyres) > > 405vyres = yres; > > 406if (vxres * vyres > maxmem) { > > 407vyres = maxmem / vxres; > > 408if (vyres < yres) > > 409return -ENOMEM; > > 410} > > > > At line 406, the product of vxres * vyres overflows 32 bits (is 0 in this > > case/example), so any protection from this block is lost. > > OK. Then, we can check overflow like below. > > diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c > index e2757ff1c23d..e483a3f5fd47 100644 > --- a/drivers/video/fbdev/vga16fb.c > +++ b/drivers/video/fbdev/vga16fb.c > @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo > *var, > > if (yres > vyres) > vyres = yres; > - if (vxres * vyres > maxmem) { > + if ((u64) vxres * vyres > (u64) maxmem) { Mindlessly changing the sizes is not the solution. Please use e.g. the array_size() helper from instead. > vyres = maxmem / vxres; > if (vyres < yres) > return -ENOMEM; > > But I think we can check overflow in the common code like below. (Both patch > fixed the oops.) > > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c > index 1c855145711b..8899679bbc46 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1008,6 +1008,11 @@ fb_set_var(struct fb_info *info, struct > fb_var_screeninfo *var) > if (var->xres < 8 || var->yres < 8) > return -EINVAL; > > + /* Don't allow u32 * u32 to overflow. */ > + if ((u64) var->xres * var->yres > (u64) UINT_MAX || > + (u64) var->xres_virtual * var->yres_virtual > (u64) UINT_MAX) > + return -EINVAL; > + Same comment here, of course. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
[PATCH AUTOSEL 4.19 1/8] gpu: ipu-v3: Fix i.MX IPU-v3 offset calculations for (semi)planar U/V formats
From: Krzysztof Hałasa [ Upstream commit 7cca7c8096e2c8a4149405438329b5035d0744f0 ] Video captured in 1400x1050 resolution (bytesperline aka stride = 1408 bytes) is invalid. Fix it. Signed-off-by: Krzysztof Halasa Link: https://lore.kernel.org/r/m3y2bmq7a4@t19.piap.pl [p.za...@pengutronix.de: added "gpu: ipu-v3:" prefix to commit description] Signed-off-by: Philipp Zabel Signed-off-by: Sasha Levin --- drivers/gpu/ipu-v3/ipu-cpmem.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c index a9d2501500a1..170371770dd4 100644 --- a/drivers/gpu/ipu-v3/ipu-cpmem.c +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c @@ -545,21 +545,21 @@ static const struct ipu_rgb def_bgra_16 = { .bits_per_pixel = 16, }; -#define Y_OFFSET(pix, x, y)((x) + pix->width * (y)) -#define U_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define V_OFFSET(pix, x, y)((pix->width * pix->height) + \ -(pix->width * pix->height / 4) + \ -(pix->width * ((y) / 2) / 2) + (x) / 2) -#define U2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * (y) / 2) + (x) / 2) -#define V2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * pix->height / 2) + \ -(pix->width * (y) / 2) + (x) / 2) -#define UV_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * ((y) / 2)) + (x)) -#define UV2_OFFSET(pix, x, y) ((pix->width * pix->height) + \ -(pix->width * y) + (x)) +#define Y_OFFSET(pix, x, y)((x) + pix->bytesperline * (y)) +#define U_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define V_OFFSET(pix, x, y)((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 4) + \ +(pix->bytesperline * ((y) / 2) / 2) + (x) / 2) +#define U2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define V2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * pix->height / 2) + \ +(pix->bytesperline * (y) / 2) + (x) / 2) +#define UV_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * ((y) / 2)) + (x)) +#define UV2_OFFSET(pix, x, y) ((pix->bytesperline * pix->height) + \ +(pix->bytesperline * y) + (x)) #define NUM_ALPHA_CHANNELS 7 -- 2.30.2
Re: [PATCH 2/7] drm/format-helper: Add drm_fb_xrgb8888_to_rgb332()
Den 17.08.2021 15.56, skrev Daniel Vetter: > On Tue, Aug 17, 2021 at 02:29:12PM +0200, Noralf Trønnes wrote: >> Add XRGB emulation support for devices that can only do RGB332. >> >> Cc: Thomas Zimmermann >> Signed-off-by: Noralf Trønnes >> --- >> drivers/gpu/drm/drm_format_helper.c | 47 + >> include/drm/drm_format_helper.h | 2 ++ >> 2 files changed, 49 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_format_helper.c >> b/drivers/gpu/drm/drm_format_helper.c >> index 5231104b1498..53b426da7467 100644 >> --- a/drivers/gpu/drm/drm_format_helper.c >> +++ b/drivers/gpu/drm/drm_format_helper.c >> @@ -135,6 +135,53 @@ void drm_fb_swab(void *dst, void *src, struct >> drm_framebuffer *fb, >> } >> EXPORT_SYMBOL(drm_fb_swab); >> >> +static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, u32 *sbuf, unsigned >> int pixels) >> +{ >> +unsigned int x; >> + >> +for (x = 0; x < pixels; x++) >> +dbuf[x] = ((sbuf[x] & 0x00e0) >> 16) | > > I think for 2/3 bits correct rounding would be useful, not just masking. > i.e. before you shift add 0x0010 here, and similar below. > Math isn't my strongest side and my brain failed to turn this into code. > Also I just realized we've totally ignored endianess on these, which is > not great, because strictly speaking all the drm_fourcc codes should be > little endian. But I'm also not sure that's worth fixing ... > Is it as simple as using le32_to_cpu()? static void drm_fb_xrgb_to_rgb332_line(u8 *dbuf, __le32 *sbuf, unsigned int pixels) { unsigned int x; u32 pix; for (x = 0; x < pixels; x++) { pix = le32_to_cpu(sbuf[x]); dbuf[x] = ((pix & 0x00e0) >> 16) | ((pix & 0xe000) >> 11) | ((pix & 0x00c0) >> 6); } } Noralf. > Either way, lgtm: > > Reviewed-by: Daniel Vetter > >> + ((sbuf[x] & 0xe000) >> 11) | >> + ((sbuf[x] & 0x00c0) >> 6); >> +} >> + >> +/** >> + * drm_fb_xrgb_to_rgb332 - Convert XRGB to RGB332 clip buffer >> + * @dst: RGB332 destination buffer >> + * @src: XRGB source buffer >> + * @fb: DRM framebuffer >> + * @clip: Clip rectangle area to copy >> + * >> + * Drivers can use this function for RGB332 devices that don't natively >> support XRGB. >> + * >> + * This function does not apply clipping on dst, i.e. the destination is a >> small buffer >> + * containing the clip rect only. >> + */ >> +void drm_fb_xrgb_to_rgb332(void *dst, void *src, struct drm_framebuffer >> *fb, >> + struct drm_rect *clip) >> +{ >> +size_t width = drm_rect_width(clip); >> +size_t src_len = width * sizeof(u32); >> +unsigned int y; >> +void *sbuf; >> + >> +/* Use a buffer to speed up access on buffers with uncached read >> mapping (i.e. WC) */ >> +sbuf = kmalloc(src_len, GFP_KERNEL); >> +if (!sbuf) >> +return; >> + >> +src += clip_offset(clip, fb->pitches[0], sizeof(u32)); >> +for (y = 0; y < drm_rect_height(clip); y++) { >> +memcpy(sbuf, src, src_len); >> +drm_fb_xrgb_to_rgb332_line(dst, sbuf, width); >> +src += fb->pitches[0]; >> +dst += width; >> +} >> + >> +kfree(sbuf); >> +} >> +EXPORT_SYMBOL(drm_fb_xrgb_to_rgb332); >> + >> static void drm_fb_xrgb_to_rgb565_line(u16 *dbuf, u32 *sbuf, >> unsigned int pixels, >> bool swab) >> diff --git a/include/drm/drm_format_helper.h >> b/include/drm/drm_format_helper.h >> index 4e0258a61311..d0809aff5cf8 100644 >> --- a/include/drm/drm_format_helper.h >> +++ b/include/drm/drm_format_helper.h >> @@ -16,6 +16,8 @@ void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int >> dst_pitch, void *vadd >> struct drm_rect *clip); >> void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, >> struct drm_rect *clip, bool cached); >> +void drm_fb_xrgb_to_rgb332(void *dst, void *vaddr, struct >> drm_framebuffer *fb, >> + struct drm_rect *clip); >> void drm_fb_xrgb_to_rgb565(void *dst, void *vaddr, >> struct drm_framebuffer *fb, >> struct drm_rect *clip, bool swab); >> -- >> 2.32.0 >> >
Re: [PATCH AUTOSEL 5.13 20/26] drm/nouveau: recognise GA107
On Tue, Aug 24, 2021 at 01:08:28PM -0400, Lyude Paul wrote: This is more hardware enablement, I'm not sure this should be going into stable either. Ben? We take this sort of hardware enablement patches (where the platform code is already there, and we just add quirks/ids/etc. -- Thanks, Sasha
Re: [PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace
On Tue, Aug 24, 2021 at 01:05:32PM -0400, Lyude Paul wrote: This isn't at all intended to be a fix to be backported, so I don't think this should be included. I don't know about 5/7, but I'll let Benjamin comment on that one I'll drop it, thanks! -- Thanks, Sasha
Re: [PATCH] drm/radeon: Make use of the helper macro SET_RUNTIME_PM_OPS()
On Sat, Aug 28, 2021 at 4:51 AM Cai Huoqing wrote: > > Use the helper macro SET_RUNTIME_PM_OPS() instead of the verbose > operators ".runtime_suspend/.runtime_resume/.runtime_idle", because > the SET_RUNTIME_PM_OPS() is a nice helper macro that could be brought > in to make code a little clearer, a little more concise. I don't personally think it really helps readability. Just seems to be code churn. Alex > > Signed-off-by: Cai Huoqing > --- > drivers/gpu/drm/radeon/radeon_drv.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > b/drivers/gpu/drm/radeon/radeon_drv.c > index b74cebca1f89..c2eb725e87f6 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -529,15 +529,14 @@ static long radeon_kms_compat_ioctl(struct file *filp, > unsigned int cmd, unsigne > #endif > > static const struct dev_pm_ops radeon_pm_ops = { > + SET_RUNTIME_PM_OPS(radeon_pmops_runtime_suspend, > + radeon_pmops_runtime_resume, > radeon_pmops_runtime_idle) > .suspend = radeon_pmops_suspend, > .resume = radeon_pmops_resume, > .freeze = radeon_pmops_freeze, > .thaw = radeon_pmops_thaw, > .poweroff = radeon_pmops_freeze, > .restore = radeon_pmops_resume, > - .runtime_suspend = radeon_pmops_runtime_suspend, > - .runtime_resume = radeon_pmops_runtime_resume, > - .runtime_idle = radeon_pmops_runtime_idle, > }; > > static const struct file_operations radeon_driver_kms_fops = { > -- > 2.25.1 >
Re: [PATCH 1/2] dt-bindings: panel: ili9341: correct indentation
On Mon, Aug 23, 2021 at 3:11 PM Sam Ravnborg wrote: > > On Thu, Aug 19, 2021 at 12:10:19PM +0200, Krzysztof Kozlowski wrote: > > Correct indentation warning: > > ilitek,ili9341.yaml:25:9: [warning] wrong indentation: expected 10 but > > found 8 (indentation) > > > > Signed-off-by: Krzysztof Kozlowski > > Thanks, applied to drm-misc-next, and the patch will show up in -next in > one to two weeks. This needs to go in fixes as it looks like commit 7dbdce806268 ("dt-bindings: display: panel: Add ilitek ili9341 panel bindings") is queued for 5.15. Rob
Re: [PATCH] drm/radeon: Make use of the helper macro SET_RUNTIME_PM_OPS()
On 30 Aug 21 08:21:52, Alex Deucher wrote: > On Sat, Aug 28, 2021 at 4:51 AM Cai Huoqing wrote: > > > > Use the helper macro SET_RUNTIME_PM_OPS() instead of the verbose > > operators ".runtime_suspend/.runtime_resume/.runtime_idle", because > > the SET_RUNTIME_PM_OPS() is a nice helper macro that could be brought > > in to make code a little clearer, a little more concise. > > I don't personally think it really helps readability. Just seems to > be code churn. > > Alex > agree, just code churn, the macro seems to do it. Cai > > > > > Signed-off-by: Cai Huoqing > > --- > > drivers/gpu/drm/radeon/radeon_drv.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > > b/drivers/gpu/drm/radeon/radeon_drv.c > > index b74cebca1f89..c2eb725e87f6 100644 > > --- a/drivers/gpu/drm/radeon/radeon_drv.c > > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > > @@ -529,15 +529,14 @@ static long radeon_kms_compat_ioctl(struct file > > *filp, unsigned int cmd, unsigne > > #endif > > > > static const struct dev_pm_ops radeon_pm_ops = { > > + SET_RUNTIME_PM_OPS(radeon_pmops_runtime_suspend, > > + radeon_pmops_runtime_resume, > > radeon_pmops_runtime_idle) > > .suspend = radeon_pmops_suspend, > > .resume = radeon_pmops_resume, > > .freeze = radeon_pmops_freeze, > > .thaw = radeon_pmops_thaw, > > .poweroff = radeon_pmops_freeze, > > .restore = radeon_pmops_resume, > > - .runtime_suspend = radeon_pmops_runtime_suspend, > > - .runtime_resume = radeon_pmops_runtime_resume, > > - .runtime_idle = radeon_pmops_runtime_idle, > > }; > > > > static const struct file_operations radeon_driver_kms_fops = { > > -- > > 2.25.1 > >
Re: [PATCH] dma-buf: heaps: remove duplicated cache sync
Am 30.08.21 um 12:01 schrieb guangming@mediatek.com: From: Guangming Cao Current flow, one dmabuf maybe call cache sync many times if it has beed mapped more than one time. Well I'm not an expert on DMA heaps, but this will most likely not work correctly. Is there any case that attachments of one dmabuf will points to different memory? If not, seems do sync only one time is more better. I think that this can happen, yes. Christian. Signed-off-by: Guangming Cao --- drivers/dma-buf/heaps/system_heap.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 23a7e74ef966..909ef652a8c8 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -162,9 +162,10 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, invalidate_kernel_vmap_range(buffer->vaddr, buffer->len); list_for_each_entry(a, &buffer->attachments, list) { - if (!a->mapped) - continue; - dma_sync_sgtable_for_cpu(a->dev, a->table, direction); + if (a->mapped) { + dma_sync_sgtable_for_cpu(a->dev, a->table, direction); + break; + } } mutex_unlock(&buffer->lock); @@ -183,9 +184,10 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf, flush_kernel_vmap_range(buffer->vaddr, buffer->len); list_for_each_entry(a, &buffer->attachments, list) { - if (!a->mapped) - continue; - dma_sync_sgtable_for_device(a->dev, a->table, direction); + if (!a->mapped) { + dma_sync_sgtable_for_device(a->dev, a->table, direction); + break; + } } mutex_unlock(&buffer->lock);
Re: [PATCH] drm/radeon: Make use of the helper macro SET_RUNTIME_PM_OPS()
Am 28.08.21 um 10:50 schrieb Cai Huoqing: Use the helper macro SET_RUNTIME_PM_OPS() instead of the verbose operators ".runtime_suspend/.runtime_resume/.runtime_idle", because the SET_RUNTIME_PM_OPS() is a nice helper macro that could be brought in to make code a little clearer, a little more concise. To be honest I don't think that this is cleaner at all. Christian. Signed-off-by: Cai Huoqing --- drivers/gpu/drm/radeon/radeon_drv.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index b74cebca1f89..c2eb725e87f6 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -529,15 +529,14 @@ static long radeon_kms_compat_ioctl(struct file *filp, unsigned int cmd, unsigne #endif static const struct dev_pm_ops radeon_pm_ops = { + SET_RUNTIME_PM_OPS(radeon_pmops_runtime_suspend, + radeon_pmops_runtime_resume, radeon_pmops_runtime_idle) .suspend = radeon_pmops_suspend, .resume = radeon_pmops_resume, .freeze = radeon_pmops_freeze, .thaw = radeon_pmops_thaw, .poweroff = radeon_pmops_freeze, .restore = radeon_pmops_resume, - .runtime_suspend = radeon_pmops_runtime_suspend, - .runtime_resume = radeon_pmops_runtime_resume, - .runtime_idle = radeon_pmops_runtime_idle, }; static const struct file_operations radeon_driver_kms_fops = {
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
On Mon, Aug 30, 2021 at 02:00:21PM +0200, Geert Uytterhoeven wrote: > Hi Testsuo, > > On Mon, Aug 30, 2021 at 4:27 AM Tetsuo Handa > wrote: > > On 2021/08/30 9:24, Randy Dunlap wrote: > > > Note that yres_virtual is set to 0x1000. Is there no practical limit > > > (hence limit check) that can be used here? > > > > > > Also, in vga16fb_check_var(), beginning at line 404: > > > > > > 404if (yres > vyres) > > > 405vyres = yres; > > > 406if (vxres * vyres > maxmem) { > > > 407vyres = maxmem / vxres; > > > 408if (vyres < yres) > > > 409return -ENOMEM; > > > 410} > > > > > > At line 406, the product of vxres * vyres overflows 32 bits (is 0 in this > > > case/example), so any protection from this block is lost. > > > > OK. Then, we can check overflow like below. > > > > diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c > > index e2757ff1c23d..e483a3f5fd47 100644 > > --- a/drivers/video/fbdev/vga16fb.c > > +++ b/drivers/video/fbdev/vga16fb.c > > @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo > > *var, > > > > if (yres > vyres) > > vyres = yres; > > - if (vxres * vyres > maxmem) { > > + if ((u64) vxres * vyres > (u64) maxmem) { > > Mindlessly changing the sizes is not the solution. > Please use e.g. the array_size() helper from > instead. On a 64bit system the array_size() macro is going to do the exact same casts? But I do think this code would be easier to understand if the integer overflow check were pull out separately and done first: if (array_size(vxres, vyres) >= UINT_MAX) return -EINVAL; if (vxres * vyres > maxmem) { ... The UINT_MAX is because vxres and vyres are u32. This would maybe be the first time anyone ever did an integer overflow check like this in the kernel. It's a new idiom. regards, dan carpenter
Re: [PATCH] drm/amdgpu: Make use of the helper macro SET_RUNTIME_PM_OPS()
On Sat, Aug 28, 2021 at 4:41 AM Cai Huoqing wrote: > > Use the helper macro SET_RUNTIME_PM_OPS() instead of the verbose > operators ".runtime_suspend/.runtime_resume/.runtime_idle", because > the SET_RUNTIME_PM_OPS() is a nice helper macro that could be brought > in to make code a little clearer, a little more concise. > I think this makes readability worse and just causes code churn. Alex > Signed-off-by: Cai Huoqing > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index b6640291f980..9e5fb8d2e0e0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -1699,6 +1699,8 @@ long amdgpu_drm_ioctl(struct file *filp, > } > > static const struct dev_pm_ops amdgpu_pm_ops = { > + SET_RUNTIME_PM_OPS(amdgpu_pmops_runtime_suspend, > + amdgpu_pmops_runtime_resume, > amdgpu_pmops_runtime_idle) > .prepare = amdgpu_pmops_prepare, > .complete = amdgpu_pmops_complete, > .suspend = amdgpu_pmops_suspend, > @@ -1707,9 +1709,6 @@ static const struct dev_pm_ops amdgpu_pm_ops = { > .thaw = amdgpu_pmops_thaw, > .poweroff = amdgpu_pmops_poweroff, > .restore = amdgpu_pmops_restore, > - .runtime_suspend = amdgpu_pmops_runtime_suspend, > - .runtime_resume = amdgpu_pmops_runtime_resume, > - .runtime_idle = amdgpu_pmops_runtime_idle, > }; > > static int amdgpu_flush(struct file *f, fl_owner_t id) > -- > 2.25.1 >
Re: [PATCH rdma-next v4 0/3] SG fix together with update to RDMA umem
On Mon, Aug 30, 2021 at 11:21:00AM +0300, Leon Romanovsky wrote: > On Tue, Aug 24, 2021 at 05:25:28PM +0300, Maor Gottlieb wrote: > > From: Maor Gottlieb > > > > Changelog: > > v4: > > * Unify sg_free_table_entries with __sg_free_table > > v3: https://lore.kernel.org/lkml/cover.1627551226.git.leo...@nvidia.com/ > > * Rewrote to new API suggestion > > * Split for more patches > > v2: https://lore.kernel.org/lkml/cover.1626605893.git.leo...@nvidia.com > > * Changed implementation of first patch, based on our discussion with > > * Christoph. > >https://lore.kernel.org/lkml/ynwavtt0qmqdx...@infradead.org/ > > v1: https://lore.kernel.org/lkml/cover.1624955710.git.leo...@nvidia.com/ > > * Fixed sg_page with a _dma_ API in the umem.c > > v0: https://lore.kernel.org/lkml/cover.1624361199.git.leo...@nvidia.com > > > > Maor Gottlieb (3): > > lib/scatterlist: Provide a dedicated function to support table append > > lib/scatterlist: Fix wrong update of orig_nents > > RDMA: Use the sg_table directly and remove the opencoded version from > > umem > > > > drivers/gpu/drm/drm_prime.c | 13 +- > > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 11 +- > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 14 +- > > drivers/infiniband/core/umem.c | 56 --- > > drivers/infiniband/core/umem_dmabuf.c | 5 +- > > drivers/infiniband/hw/hns/hns_roce_db.c | 4 +- > > drivers/infiniband/hw/irdma/verbs.c | 2 +- > > drivers/infiniband/hw/mlx4/doorbell.c | 3 +- > > drivers/infiniband/hw/mlx4/mr.c | 4 +- > > drivers/infiniband/hw/mlx5/doorbell.c | 3 +- > > drivers/infiniband/hw/mlx5/mr.c | 3 +- > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > drivers/infiniband/sw/rdmavt/mr.c | 2 +- > > drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- > > include/linux/scatterlist.h | 56 +-- > > include/rdma/ib_umem.h | 11 +- > > include/rdma/ib_verbs.h | 28 > > lib/scatterlist.c | 155 > > lib/sg_pool.c | 3 +- > > tools/testing/scatterlist/main.c| 38 +++-- > > 20 files changed, 258 insertions(+), 157 deletions(-) > > Jason, > > Did you add these patches to the -next? I can't find them. They sat in linux-next for awhile outside the rdma-next tree All synced up now Jason
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
On 2021/08/30 22:00, Dan Carpenter wrote: >>> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c >>> index e2757ff1c23d..e483a3f5fd47 100644 >>> --- a/drivers/video/fbdev/vga16fb.c >>> +++ b/drivers/video/fbdev/vga16fb.c >>> @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo >>> *var, >>> >>> if (yres > vyres) >>> vyres = yres; >>> - if (vxres * vyres > maxmem) { >>> + if ((u64) vxres * vyres > (u64) maxmem) { >> >> Mindlessly changing the sizes is not the solution. >> Please use e.g. the array_size() helper from >> instead. > > On a 64bit system the array_size() macro is going to do the exact same > casts? But I do think this code would be easier to understand if the > integer overflow check were pull out separately and done first: > > if (array_size(vxres, vyres) >= UINT_MAX) > return -EINVAL; This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). Comparing like "> (u64) UINT_MAX" is to detect only overflow. array_size() would be helpful for forcing memory allocation to fail (instead of allocating smaller than actually required). > > if (vxres * vyres > maxmem) { > ... > > The UINT_MAX is because vxres and vyres are u32. > > This would maybe be the first time anyone ever did an integer overflow > check like this in the kernel. It's a new idiom. > > regards, > dan carpenter >
[Bug 214029] [NAVI] Several memory leaks in amdgpu and ttm
https://bugzilla.kernel.org/show_bug.cgi?id=214029 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #298269|0 |1 is obsolete|| --- Comment #3 from Erhard F. (erhar...@mailbox.org) --- Created attachment 298525 --> https://bugzilla.kernel.org/attachment.cgi?id=298525&action=edit kernel .config (kernel 5.14, AMD FX-8370) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 214029] [NAVI] Several memory leaks in amdgpu and ttm
https://bugzilla.kernel.org/show_bug.cgi?id=214029 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #298265|0 |1 is obsolete|| --- Comment #4 from Erhard F. (erhar...@mailbox.org) --- Created attachment 298527 --> https://bugzilla.kernel.org/attachment.cgi?id=298527&action=edit kernel dmesg (kernel 5.14, AMD FX-8370) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
On Mon, Aug 30, 2021 at 10:37:31PM +0900, Tetsuo Handa wrote: > On 2021/08/30 22:00, Dan Carpenter wrote: > >>> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c > >>> index e2757ff1c23d..e483a3f5fd47 100644 > >>> --- a/drivers/video/fbdev/vga16fb.c > >>> +++ b/drivers/video/fbdev/vga16fb.c > >>> @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo > >>> *var, > >>> > >>> if (yres > vyres) > >>> vyres = yres; > >>> - if (vxres * vyres > maxmem) { > >>> + if ((u64) vxres * vyres > (u64) maxmem) { > >> > >> Mindlessly changing the sizes is not the solution. > >> Please use e.g. the array_size() helper from > >> instead. > > > > On a 64bit system the array_size() macro is going to do the exact same > > casts? But I do think this code would be easier to understand if the > > integer overflow check were pull out separately and done first: > > > > if (array_size(vxres, vyres) >= UINT_MAX) > > return -EINVAL; > > This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and > returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid > value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). Huh... I just assumed we didn't allow resolutions that high. > Comparing like "> (u64) UINT_MAX" is to detect only overflow. > Of course, that doesn't work on 32 bit systems. Also the cast isn't required because of type promotion. regards, dan carpenter
Re: [PATCH v3 0/2] GPD Win Max display fixes
Hi, On Tue, Aug 17, 2021 at 10:43 PM Anisse Astier wrote: > > This patch series is for making the GPD Win Max display usable with > Linux. > > The GPD Win Max is a small laptop, and its eDP panel does not send an > EDID over DPCD; the EDID is instead available in the intel opregion, in > mailbox #5 [1] > > The first patch is based on Jani's patch series [2] adding support for > the opregion, with changes. I've changed authorship, but I'd be glad to > revert it > > The second patch is just to fix the orientation of the panel. > > Changes since v1: > - rebased on drm-tip > - squashed patch 1 & 2 > - picked up Reviewed-by from Hans de Goede (thanks for the review) > > Changes since v2: > - rebased on drm-tip > - updated commit message > > When v2 was initially sent [3] Ville Syrjälä suggested that it might be > a good idea to use the ACPI _DDC method instead to get the EDID, to > cover a wider range of hardware. Unfortunately, it doesn't seem > available on GPD Win Max, so I think this work should be done > independently, and this patch series considered separately. > > [1]: https://gitlab.freedesktop.org/drm/intel/-/issues/3454 > [2]: > https://patchwork.kernel.org/project/intel-gfx/patch/20200828061941.17051-1-jani.nik...@intel.com/ > [3]: > https://patchwork.kernel.org/project/intel-gfx/patch/20210531204642.4907-2-ani...@astier.eu/ > > > Anisse Astier (2): > drm/i915/opregion: add support for mailbox #5 EDID > drm: Add orientation quirk for GPD Win Max > > .../gpu/drm/drm_panel_orientation_quirks.c| 6 ++ > drivers/gpu/drm/i915/display/intel_dp.c | 3 + > drivers/gpu/drm/i915/display/intel_opregion.c | 69 ++- > drivers/gpu/drm/i915/display/intel_opregion.h | 8 +++ > 4 files changed, 85 insertions(+), 1 deletion(-) Would it be possible to have this series reviewed ? Kind regards, Anisse
[Bug 214029] [NAVI] Several memory leaks in amdgpu and ttm
https://bugzilla.kernel.org/show_bug.cgi?id=214029 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #298267|0 |1 is obsolete|| --- Comment #5 from Erhard F. (erhar...@mailbox.org) --- Created attachment 298529 --> https://bugzilla.kernel.org/attachment.cgi?id=298529&action=edit output of kmemleak (kernel 5.14, AMD FX-8370) [...] unreferenced object 0x8881dd96f0c0 (size 216): comm "lxdm-greeter-gt", pid 619, jiffies 4294888177 (age 3729.577s) hex dump (first 32 bytes): 50 f1 96 dd 81 88 ff ff 20 8b 72 c0 ff ff ff ff P... .r. c0 3a 59 da 08 00 00 00 58 fd a6 08 00 c9 ff ff .:Y.X... backtrace: [] drm_sched_fence_create+0x1f/0x1d0 [gpu_sched] [] drm_sched_job_init+0x10e/0x240 [gpu_sched] [] amdgpu_job_submit+0x27/0x2d0 [amdgpu] [] amdgpu_copy_buffer+0x49e/0x700 [amdgpu] [] amdgpu_ttm_copy_mem_to_mem+0x5fa/0xf00 [amdgpu] [] amdgpu_bo_move+0x356/0x2180 [amdgpu] [] ttm_bo_handle_move_mem+0x1c7/0x620 [ttm] [] ttm_bo_validate+0x2c7/0x450 [ttm] [] amdgpu_bo_fault_reserve_notify+0x2a4/0x640 [amdgpu] [] amdgpu_gem_fault+0x123/0x2d0 [amdgpu] [] __do_fault+0xf3/0x3e0 [] __handle_mm_fault+0x1bcb/0x2ac0 [] handle_mm_fault+0x12a/0x490 [] do_user_addr_fault+0x259/0xb70 [] exc_page_fault+0x55/0xb0 [] asm_exc_page_fault+0x1b/0x20 unreferenced object 0x888120f77100 (size 72): comm "sdma0", pid 344, jiffies 4294888177 (age 3729.577s) hex dump (first 32 bytes): 30 f4 ec 3a 81 88 ff ff 20 8a f0 c1 ff ff ff ff 0..: ... a3 49 5d da 08 00 00 00 58 84 a3 0c 82 88 ff ff .I].X... backtrace: [] amdgpu_fence_emit+0x91/0x790 [amdgpu] [] amdgpu_ib_schedule+0x8cb/0x12f0 [amdgpu] [] amdgpu_job_run+0x35e/0x790 [amdgpu] [] drm_sched_main+0x64e/0xc60 [gpu_sched] [] kthread+0x342/0x410 [] ret_from_fork+0x22/0x30 unreferenced object 0x888246342640 (size 216): comm "mate-session-ch", pid 718, jiffies 4294890392 (age 3722.200s) hex dump (first 32 bytes): d0 26 34 46 82 88 ff ff 20 8b 72 c0 ff ff ff ff .&4F .r. 03 71 65 92 0a 00 00 00 58 fd a6 08 00 c9 ff ff .qe.X... backtrace: [] drm_sched_fence_create+0x1f/0x1d0 [gpu_sched] [] drm_sched_job_init+0x10e/0x240 [gpu_sched] [] amdgpu_job_submit+0x27/0x2d0 [amdgpu] [] amdgpu_copy_buffer+0x49e/0x700 [amdgpu] [] amdgpu_ttm_copy_mem_to_mem+0x5fa/0xf00 [amdgpu] [] amdgpu_bo_move+0x356/0x2180 [amdgpu] [] ttm_bo_handle_move_mem+0x1c7/0x620 [ttm] [] ttm_bo_validate+0x2c7/0x450 [ttm] [] amdgpu_bo_fault_reserve_notify+0x2a4/0x640 [amdgpu] [] amdgpu_gem_fault+0x123/0x2d0 [amdgpu] [] __do_fault+0xf3/0x3e0 [] __handle_mm_fault+0x1bcb/0x2ac0 [] handle_mm_fault+0x12a/0x490 [] do_user_addr_fault+0x259/0xb70 [] exc_page_fault+0x55/0xb0 [] asm_exc_page_fault+0x1b/0x20 [...] -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH rdma-next v4 0/3] SG fix together with update to RDMA umem
On Mon, Aug 30, 2021 at 10:31:28AM -0300, Jason Gunthorpe wrote: > On Mon, Aug 30, 2021 at 11:21:00AM +0300, Leon Romanovsky wrote: > > On Tue, Aug 24, 2021 at 05:25:28PM +0300, Maor Gottlieb wrote: > > > From: Maor Gottlieb > > > > > > Changelog: > > > v4: > > > * Unify sg_free_table_entries with __sg_free_table > > > v3: https://lore.kernel.org/lkml/cover.1627551226.git.leo...@nvidia.com/ > > > * Rewrote to new API suggestion > > > * Split for more patches > > > v2: https://lore.kernel.org/lkml/cover.1626605893.git.leo...@nvidia.com > > > * Changed implementation of first patch, based on our discussion with > > > * Christoph. > > >https://lore.kernel.org/lkml/ynwavtt0qmqdx...@infradead.org/ > > > v1: https://lore.kernel.org/lkml/cover.1624955710.git.leo...@nvidia.com/ > > > * Fixed sg_page with a _dma_ API in the umem.c > > > v0: https://lore.kernel.org/lkml/cover.1624361199.git.leo...@nvidia.com > > > > > > Maor Gottlieb (3): > > > lib/scatterlist: Provide a dedicated function to support table append > > > lib/scatterlist: Fix wrong update of orig_nents > > > RDMA: Use the sg_table directly and remove the opencoded version from > > > umem > > > > > > drivers/gpu/drm/drm_prime.c | 13 +- > > > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 11 +- > > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 14 +- > > > drivers/infiniband/core/umem.c | 56 --- > > > drivers/infiniband/core/umem_dmabuf.c | 5 +- > > > drivers/infiniband/hw/hns/hns_roce_db.c | 4 +- > > > drivers/infiniband/hw/irdma/verbs.c | 2 +- > > > drivers/infiniband/hw/mlx4/doorbell.c | 3 +- > > > drivers/infiniband/hw/mlx4/mr.c | 4 +- > > > drivers/infiniband/hw/mlx5/doorbell.c | 3 +- > > > drivers/infiniband/hw/mlx5/mr.c | 3 +- > > > drivers/infiniband/hw/qedr/verbs.c | 2 +- > > > drivers/infiniband/sw/rdmavt/mr.c | 2 +- > > > drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- > > > include/linux/scatterlist.h | 56 +-- > > > include/rdma/ib_umem.h | 11 +- > > > include/rdma/ib_verbs.h | 28 > > > lib/scatterlist.c | 155 > > > lib/sg_pool.c | 3 +- > > > tools/testing/scatterlist/main.c| 38 +++-- > > > 20 files changed, 258 insertions(+), 157 deletions(-) > > > > Jason, > > > > Did you add these patches to the -next? I can't find them. > > They sat in linux-next for awhile outside the rdma-next tree > > All synced up now Thanks > > Jason
Re: [PATCH rdma-next v4 0/3] SG fix together with update to RDMA umem
On Tue, Aug 24, 2021 at 05:25:28PM +0300, Maor Gottlieb wrote: > From: Maor Gottlieb > > Changelog: > v4: > * Unify sg_free_table_entries with __sg_free_table > v3: https://lore.kernel.org/lkml/cover.1627551226.git.leo...@nvidia.com/ > * Rewrote to new API suggestion > * Split for more patches > v2: https://lore.kernel.org/lkml/cover.1626605893.git.leo...@nvidia.com > * Changed implementation of first patch, based on our discussion with > * Christoph. >https://lore.kernel.org/lkml/ynwavtt0qmqdx...@infradead.org/ > v1: https://lore.kernel.org/lkml/cover.1624955710.git.leo...@nvidia.com/ > * Fixed sg_page with a _dma_ API in the umem.c > v0: https://lore.kernel.org/lkml/cover.1624361199.git.leo...@nvidia.com > > Maor Gottlieb (3): > lib/scatterlist: Provide a dedicated function to support table append > lib/scatterlist: Fix wrong update of orig_nents > RDMA: Use the sg_table directly and remove the opencoded version from > umem > > drivers/gpu/drm/drm_prime.c | 13 +- > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 11 +- > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 14 +- > drivers/infiniband/core/umem.c | 56 --- > drivers/infiniband/core/umem_dmabuf.c | 5 +- > drivers/infiniband/hw/hns/hns_roce_db.c | 4 +- > drivers/infiniband/hw/irdma/verbs.c | 2 +- > drivers/infiniband/hw/mlx4/doorbell.c | 3 +- > drivers/infiniband/hw/mlx4/mr.c | 4 +- > drivers/infiniband/hw/mlx5/doorbell.c | 3 +- > drivers/infiniband/hw/mlx5/mr.c | 3 +- > drivers/infiniband/hw/qedr/verbs.c | 2 +- > drivers/infiniband/sw/rdmavt/mr.c | 2 +- > drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- > include/linux/scatterlist.h | 56 +-- > include/rdma/ib_umem.h | 11 +- > include/rdma/ib_verbs.h | 28 > lib/scatterlist.c | 155 > lib/sg_pool.c | 3 +- > tools/testing/scatterlist/main.c| 38 +++-- > 20 files changed, 258 insertions(+), 157 deletions(-) Jason, Did you add these patches to the -next? I can't find them. Thanks > > -- > 2.25.4 >
Re: [PATCH v3 1/4] drm/ttm: Create pinned list
On 2021-08-30 4:58 a.m., Christian König wrote: Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky: This list will be used to capture all non VRAM BOs not on LRU so when device is hot unplugged we can iterate the list and unmap DMA mappings before device is removed. v2: Reanme function to ttm_bo_move_to_pinned v3: Move the pinned list to ttm device As far as I can see there is not list_del() remaining. So this won't work correctly. It's in ttm_bo_release, there was no code change there hence it's not captured in the patch. Andrey I suggest to rather rebase on top of the stuff I'm working on for a while to move the LRU into the resource instead. Just send out the latest patch set of this with you in CC. Christian. Signed-off-by: Andrey Grodzovsky Suggested-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++ drivers/gpu/drm/ttm/ttm_device.c | 1 + include/drm/ttm/ttm_device.h | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b950b45cf4b..1fedd0eb67ba 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) +{ + struct ttm_device *bdev = bo->bdev; + + list_move_tail(&bo->lru, &bdev->pinned); + + if (bdev->funcs->del_from_lru_notify) + bdev->funcs->del_from_lru_notify(bo); +} + +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { struct ttm_device *bdev = bo->bdev; @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, dma_resv_assert_held(bo->base.resv); if (bo->pin_count) { - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); return; } @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, return ret; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); list_del_init(&bo->ddestroy); spin_unlock(&bo->bdev->lru_lock); ttm_bo_cleanup_memtype_use(bo); @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, return 0; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); /* TODO: Cleanup the locking */ spin_unlock(&bo->bdev->lru_lock); diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 5f31acec3ad7..530a9c36be37 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs, INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue); spin_lock_init(&bdev->lru_lock); INIT_LIST_HEAD(&bdev->ddestroy); + INIT_LIST_HEAD(&bdev->pinned); bdev->dev_mapping = mapping; mutex_lock(&ttm_global_mutex); list_add_tail(&bdev->device_list, &glob->device_list); diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index cd592f8e941b..03fb44d061e0 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -265,6 +265,7 @@ struct ttm_device { */ spinlock_t lru_lock; struct list_head ddestroy; + struct list_head pinned; /* * Protected by load / firstopen / lastclose /unload sync.
Re: [PATCH 2/3] drm/msm/dsi: Use "ref" fw clock instead of global name for VCO parent
Hi Dmitry, On Mon, Aug 30, 2021 at 04:17:32AM +0300, Dmitry Baryshkov wrote: > On Mon, 30 Aug 2021 at 00:53, Marijn Suijten > wrote: > > > > Hi Dmitry, > > > > On 8/29/21 10:39 PM, Dmitry Baryshkov wrote: > > > Hi, > > > > > > On Sun, 29 Aug 2021 at 23:30, Marijn Suijten > > > wrote: > > >> > > >> All DSI PHY/PLL drivers were referencing their VCO parent clock by a > > >> global name, most of which don't exist or have been renamed. These > > >> clock drivers seem to function fine without that except the 14nm driver > > >> for the sdm6xx [1]. > > >> > > >> At the same time all DTs provide a "ref" clock as per the requirements > > >> of dsi-phy-common.yaml, but the clock is never used. This patchset puts > > >> that clock to use without relying on a global clock name, so that all > > >> dependencies are explicitly defined in DT (the firmware) in the end. > > > > > > msm8974 (28nm-hpm) does not define the "ref" clock. So you'd have to: > > > 1) add ref clock to the dtsi (should come in a separate patch). > > > > > > Thanks for double-checking and noticing this! I've queued up this patch > > for v2. > > > > > 2) add .name = "xo" as a fallback to the 28nm driver (to be compatible > > > with older devices) > > > > > > Are there msm8974 devices out there that might upgrade kernels, but not > > firmware (DT)? On other boards (sdm630) I'm removing these from various > > drivers as to not have any possibility of relying on global names, in > > favour of having the clock dependencies fully specified in the DT. > > IIUC it is a general policy of trying to be (somewhat) > backwards-compatible. For example because your dts might come from a > different source/be a part of different build process/etc. Good thinking; DT was after all intended to be used as firmware shipping on the device, when we're usually modifying and shipping it with the kernel in the end. Just to make sure other platforms aren't affected by these changes, every board currently providing a "ref" clock has done so since the DSI node was added, except these for these three patches that added them after the fact: 79e51645a1dd ("arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY") 6969d1d9c615 ("ARM: dts: qcom-apq8064: Set 'cxo_board' as ref clock of the DSI PHY") 0c0e72705a33 ("arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs") Their commit-messages confuse me. They make it seem like the "ref" clock was previously used when this doesn't seem to be the case (hence my patch). Has there possibly been a patchset like mine that removed the mentioned hardcoded clock, but ended up never being merged? Either way, perhaps it's worth mentioning those patches with Fixes: so that this commit can be backported (have to be careful that DT changes for the other drivers are also backported, or this patch is split per PHY file), and maybe it's worth cc-ing the original authors to ask for clarification or at least make them aware? - Marijn
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
On 2021/08/30 22:47, Dan Carpenter wrote: > On Mon, Aug 30, 2021 at 10:37:31PM +0900, Tetsuo Handa wrote: >> On 2021/08/30 22:00, Dan Carpenter wrote: > diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c > index e2757ff1c23d..e483a3f5fd47 100644 > --- a/drivers/video/fbdev/vga16fb.c > +++ b/drivers/video/fbdev/vga16fb.c > @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo > *var, > > if (yres > vyres) > vyres = yres; > - if (vxres * vyres > maxmem) { > + if ((u64) vxres * vyres > (u64) maxmem) { Mindlessly changing the sizes is not the solution. Please use e.g. the array_size() helper from instead. >>> >>> On a 64bit system the array_size() macro is going to do the exact same >>> casts? But I do think this code would be easier to understand if the >>> integer overflow check were pull out separately and done first: >>> >>> if (array_size(vxres, vyres) >= UINT_MAX) >>> return -EINVAL; >> >> This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and >> returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid >> value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). > > Huh... I just assumed we didn't allow resolutions that high. Of course, we don't allow resolutions that high. ;-) Since I don't know possible max resolutions, I chose UINT_MAX + 1 as a common limit for returning -EINVAL. Unless overflow happens, vga16fb_check_var() will return -ENOMEM on such high resolutions. > >> Comparing like "> (u64) UINT_MAX" is to detect only overflow. >> > > Of course, that doesn't work on 32 bit systems. Also the cast isn't > required because of type promotion. Indeed, "> UINT_MAX" seems to work on both 32bits and 64bits. -- #include #include int main(int argc, char *argv[]) { unsigned int w = 0x600; unsigned int h = 0x1000; if ((unsigned long long) w * h > UINT_MAX) printf("Overflowed\n"); else printf("No overflow\n"); return 0; } --
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
On Mon, Aug 30, 2021 at 11:25:51PM +0900, Tetsuo Handa wrote: > On 2021/08/30 22:47, Dan Carpenter wrote: > > On Mon, Aug 30, 2021 at 10:37:31PM +0900, Tetsuo Handa wrote: > >> On 2021/08/30 22:00, Dan Carpenter wrote: > > diff --git a/drivers/video/fbdev/vga16fb.c > > b/drivers/video/fbdev/vga16fb.c > > index e2757ff1c23d..e483a3f5fd47 100644 > > --- a/drivers/video/fbdev/vga16fb.c > > +++ b/drivers/video/fbdev/vga16fb.c > > @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct > > fb_var_screeninfo *var, > > > > if (yres > vyres) > > vyres = yres; > > - if (vxres * vyres > maxmem) { > > + if ((u64) vxres * vyres > (u64) maxmem) { > > Mindlessly changing the sizes is not the solution. > Please use e.g. the array_size() helper from > instead. > >>> > >>> On a 64bit system the array_size() macro is going to do the exact same > >>> casts? But I do think this code would be easier to understand if the > >>> integer overflow check were pull out separately and done first: > >>> > >>> if (array_size(vxres, vyres) >= UINT_MAX) > >>> return -EINVAL; > >> > >> This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and > >> returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid > >> value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). > > > > Huh... I just assumed we didn't allow resolutions that high. > > Of course, we don't allow resolutions that high. ;-) > > Since I don't know possible max resolutions, I chose UINT_MAX + 1 as a common > limit for returning -EINVAL. Unless overflow happens, vga16fb_check_var() will > return -ENOMEM on such high resolutions. > > > > >> Comparing like "> (u64) UINT_MAX" is to detect only overflow. > >> > > > > Of course, that doesn't work on 32 bit systems. Also the cast isn't > > required because of type promotion. > > Indeed, "> UINT_MAX" seems to work on both 32bits and 64bits. Sorry, for the confusion. I'm talking about array_size() which is size_t. Your approach using unsigned long long works. regards, dan carpenter
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
Hi Tetsuo, On Mon, Aug 30, 2021 at 4:26 PM Tetsuo Handa wrote: > On 2021/08/30 22:47, Dan Carpenter wrote: > > On Mon, Aug 30, 2021 at 10:37:31PM +0900, Tetsuo Handa wrote: > >> On 2021/08/30 22:00, Dan Carpenter wrote: > > diff --git a/drivers/video/fbdev/vga16fb.c > > b/drivers/video/fbdev/vga16fb.c > > index e2757ff1c23d..e483a3f5fd47 100644 > > --- a/drivers/video/fbdev/vga16fb.c > > +++ b/drivers/video/fbdev/vga16fb.c > > @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct > > fb_var_screeninfo *var, > > > > if (yres > vyres) > > vyres = yres; > > - if (vxres * vyres > maxmem) { > > + if ((u64) vxres * vyres > (u64) maxmem) { > > Mindlessly changing the sizes is not the solution. > Please use e.g. the array_size() helper from > instead. > >>> > >>> On a 64bit system the array_size() macro is going to do the exact same > >>> casts? But I do think this code would be easier to understand if the > >>> integer overflow check were pull out separately and done first: > >>> > >>> if (array_size(vxres, vyres) >= UINT_MAX) > >>> return -EINVAL; > >> > >> This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and > >> returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid > >> value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). > > > > Huh... I just assumed we didn't allow resolutions that high. > > Of course, we don't allow resolutions that high. ;-) > > Since I don't know possible max resolutions, I chose UINT_MAX + 1 as a common > limit for returning -EINVAL. Unless overflow happens, vga16fb_check_var() will > return -ENOMEM on such high resolutions. The highest possible value of maxmem inside vga16fb_check_var() is 65536. So I believe if (array_size(vxres, vyres) > maxmem) should work fine. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH] drm/tidss: Make use of the helper macro SET_RUNTIME_PM_OPS()
Hi Cai, On Sat, Aug 28, 2021 at 04:52:52PM +0800, Cai Huoqing wrote: > Use the helper macro SET_RUNTIME_PM_OPS() instead of the verbose > operators ".runtime_suspend/.runtime_resume", because the > SET_RUNTIME_PM_OPS() is a nice helper macro that could be brought > in to make code a little clearer, a little more concise. > > Signed-off-by: Cai Huoqing > --- > drivers/gpu/drm/tidss/tidss_drv.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_drv.c > b/drivers/gpu/drm/tidss/tidss_drv.c > index d620f35688da..57605b80b526 100644 > --- a/drivers/gpu/drm/tidss/tidss_drv.c > +++ b/drivers/gpu/drm/tidss/tidss_drv.c > @@ -91,9 +91,8 @@ static int __maybe_unused tidss_resume(struct device *dev) > #ifdef CONFIG_PM > > static const struct dev_pm_ops tidss_pm_ops = { > - .runtime_suspend = tidss_pm_runtime_suspend, > - .runtime_resume = tidss_pm_runtime_resume, > SET_SYSTEM_SLEEP_PM_OPS(tidss_suspend, tidss_resume) > + SET_RUNTIME_PM_OPS(tidss_pm_runtime_suspend, tidss_pm_runtime_resume, > NULL) > }; > > #endif /* CONFIG_PM */ This change would make sense if you: - dropped the ifdef CONFIG_PM as they are now included in the macros. - used pm_ptr() in the assignment in tidss_platform_driver If this way you killed all CONFIG_PM uses in the driver. I would do this in one patch - as it is all simple changes. Note: For drivers with no CONFIG_PM use I would not introduce the macros as in this cases they hurt readability. Sam
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
On 2021/08/30 23:30, Geert Uytterhoeven wrote: > The highest possible value of maxmem inside vga16fb_check_var() > is 65536. Yes. > > So I believe > > if (array_size(vxres, vyres) > maxmem) > > should work fine. My intent is to check at common path than individual module so that we don't need to add same check to every module. Same approach is proposed at https://lkml.kernel.org/r/1630294223-7225-1-git-send-email-tcs_ker...@tencent.com .
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
Hi Tetsuo, On Mon, Aug 30, 2021 at 4:38 PM Tetsuo Handa wrote: > On 2021/08/30 23:30, Geert Uytterhoeven wrote: > > The highest possible value of maxmem inside vga16fb_check_var() > > is 65536. > > Yes. > > > > > So I believe > > > > if (array_size(vxres, vyres) > maxmem) > > > > should work fine. > > My intent is to check at common path than individual module so that we don't > need to add same check to every module. Same approach is proposed at > https://lkml.kernel.org/r/1630294223-7225-1-git-send-email-tcs_ker...@tencent.com > . Which I believe is wrong. Thanks for the pointer, I will reply to the actual patch... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH V4] fbcon: fix fbcon out-of-bounds write in sys_imageblit
Hi Tetsuo, On Mon, Aug 30, 2021 at 12:25 PM Tetsuo Handa wrote: > On 2021/08/30 17:16, Daniel Vetter wrote: > > On Mon, Aug 30, 2021 at 11:30:23AM +0800, tcs.ker...@gmail.com wrote: > >> From: Haimin Zhang > >> > >> yres and vyres can be controlled by user mode parameters, and cause > >> p->vrows to become a negative value. While this value be passed to real_y > >> function, the ypos will be out of screen range.This is an out-of-bounds > >> write bug. > >> some driver will check xres and yres in fb_check_var callback,but some not > >> so we add a common check after that callback. > >> > >> Signed-off-by: Haimin Zhang > >> Signed-off-by: Tetsuo Handa > > Please s/Signed-off-by: Tetsuo Handa/Suggested-by: Tetsuo Handa/ . > It is Haimin who debugged this problem and wrote this patch. > > > > > Does this fix a syzbot crash or how was this discovered? > > Yes, Haimin's team is running syzkaller locally and found this bug. > Therefore, no Reported-by: syzbot tag for this patch. > > > Forwarded Message > Subject: Re: [PATCH v2] fbcon: fix Out-Of-Bounds write in sys_imageblit > Message-ID: <33fc0e30-b94c-939f-a708-4b939af43...@gmail.com> > Date: Mon, 2 Aug 2021 14:50:24 +0800 > > hi, Tetsuo Handa > > i made a test with your suggested code, it can block the out-of-bound bug. > > where to add this check logic, i suggest to add it after the driver's > fb_check_var callback.because what we plan to add is a common check by > framework,but some driver can fault-tolerant invalid parameters(like > yres_virtual > yres) > > /* exist common check */ > if (var->xres < 8 || var->yres < 8) > return -EINVAL; > > /* callback to drivers, some driver can fix invalid virtual > xres or virtual yres */ Yes. Fbdev drivers are supposed to round up invalid or unsupported values, or return -EINVAL. > ret = info->fbops->fb_check_var(var, info); > if (ret) > return ret; > /* we add a more check here, if some drivers can't fix invalid x,y > virtual values, we return a -EINVAL */ > if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) > return -EINVAL; > > how about this fix ? i can make a v3 patch. > > > > diff --git a/drivers/video/fbdev/core/fbmem.c > b/drivers/video/fbdev/core/fbmem.c > index 1c85514..9fb7e94 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1012,6 +1012,10 @@ static int fb_check_caps(struct fb_info *info, > struct fb_var_screeninfo *var, > > if (ret) > return ret; > + > + /* Virtual resolution cannot be smaller than visible resolution. */ > + if (var->yres_virtual < var->yres || var->xres_virtual < var->xres) > + return -EINVAL; So if this happens, it's a driver bug, not a userspace bug. > > if ((var->activate & FB_ACTIVATE_MASK) != FB_ACTIVATE_NOW) > return 0; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect
Hi Tetsuo, On Mon, Aug 30, 2021 at 4:53 PM Geert Uytterhoeven wrote: > On Mon, Aug 30, 2021 at 4:38 PM Tetsuo Handa > wrote: > > On 2021/08/30 23:30, Geert Uytterhoeven wrote: > > > The highest possible value of maxmem inside vga16fb_check_var() > > > is 65536. > > > > Yes. > > > > > > > > So I believe > > > > > > if (array_size(vxres, vyres) > maxmem) > > > > > > should work fine. > > > > My intent is to check at common path than individual module so that we don't > > need to add same check to every module. Same approach is proposed at > > https://lkml.kernel.org/r/1630294223-7225-1-git-send-email-tcs_ker...@tencent.com > > . > > Which I believe is wrong. > Thanks for the pointer, I will reply to the actual patch... Upon second look, that patch is not really wrong, as the check happens after calling into info->fbops->fb_check_var(). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 3/5] drm/edid: parse the DisplayID v2.0 VESA vendor block for MSO
On Mon, Aug 30, 2021 at 01:29:01PM +0300, Jani Nikula wrote: > The VESA Organization Vendor-Specific Data Block, defined in VESA > DisplayID Standard v2.0, specifies the eDP Multi-SST Operation (MSO) > stream count and segment pixel overlap. > > DisplayID v1.3 has Appendix B: DisplayID as an EDID Extension, > describing how DisplayID sections may be embedded in EDID extension > blocks. DisplayID v2.0 does not have such a section, perhaps implying > that DisplayID v2.0 data should not be included in EDID extensions, but > rather in a "pure" DisplayID structure at its own DDC address pair > A4h/A5h, as described in VESA E-DDC Standard v1.3 chapter 3. > > However, in practice, displays out in the field have embedded DisplayID > v2.0 data blocks in EDID extensions, including, in particular, some eDP > MSO displays, where a pure DisplayID structure is not available at all. > > Parse the MSO data from the DisplayID data block. Do it as part of > drm_add_display_info(), extending it to parse also DisplayID data to > avoid requiring extra calls to update the information. > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/drm_edid.c | 63 + > include/drm/drm_connector.h | 12 +++ > include/drm/drm_displayid.h | 11 +++ > 3 files changed, 86 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 6325877c5fd6..7e8083068f3f 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -28,6 +28,7 @@ > * DEALINGS IN THE SOFTWARE. > */ > > +#include > #include > #include > #include > @@ -5148,6 +5149,62 @@ void drm_get_monitor_range(struct drm_connector > *connector, > info->monitor_range.max_vfreq); > } > > +static void drm_parse_vesa_mso_data(struct drm_connector *connector, > + const struct displayid_block *block) > +{ > + struct displayid_vesa_vendor_specific_block *vesa = > + (struct displayid_vesa_vendor_specific_block *)block; > + struct drm_display_info *info = &connector->display_info; > + > + if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { > + drm_dbg_kms(connector->dev, "Unexpected VESA vendor block > size\n"); > + return; > + } > + > + switch (FIELD_GET(DISPLAYID_VESA_MSO_MODE, vesa->mso)) { > + default: > + drm_dbg_kms(connector->dev, "Reserved MSO mode value\n"); > + fallthrough; > + case 0: > + info->mso_stream_count = 0; > + break; > + case 1: > + info->mso_stream_count = 2; /* 2 or 4 links */ > + break; > + case 2: > + info->mso_stream_count = 4; /* 4 links */ > + break; > + } > + > + if (!info->mso_stream_count) { > + info->mso_pixel_overlap = 0; > + return; > + } > + > + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > + if (info->mso_pixel_overlap > 8) { > + drm_dbg_kms(connector->dev, "Reserved MSO pixel overlap value > %u\n", > + info->mso_pixel_overlap); > + info->mso_pixel_overlap = 8; > + } > + > + drm_dbg_kms(connector->dev, "MSO stream count %u, pixel overlap %u\n", > + info->mso_stream_count, info->mso_pixel_overlap); > +} > + > +static void drm_update_mso(struct drm_connector *connector, const struct > edid *edid) > +{ > + const struct displayid_block *block; > + struct displayid_iter iter; > + > + displayid_iter_edid_begin(edid, &iter); > + displayid_iter_for_each(block, &iter) { > + if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC) Don't we need to check the OUI to make sure the block is the right type? I don't have the v2 spec at hand atm, but I presume a vendor specific block could contain all kinds of different things? > + drm_parse_vesa_mso_data(connector, block); > + } > + displayid_iter_end(&iter); > +} > + > /* A connector has no EDID information, so we've got no EDID to compute > quirks from. Reset > * all of the values which would have been set from EDID > */ > @@ -5171,6 +5228,9 @@ drm_reset_display_info(struct drm_connector *connector) > > info->non_desktop = 0; > memset(&info->monitor_range, 0, sizeof(info->monitor_range)); > + > + info->mso_stream_count = 0; > + info->mso_pixel_overlap = 0; > } > > u32 drm_add_display_info(struct drm_connector *connector, const struct edid > *edid) > @@ -5249,6 +5309,9 @@ u32 drm_add_display_info(struct drm_connector > *connector, const struct edid *edi > info->color_formats |= DRM_COLOR_FORMAT_YCRCB444; > if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422) > info->color_formats |= DRM_COLOR_FORMAT_YCRCB422; > + > + drm_update_mso(connector, edid); > + > return quirks; > } > > diff --git a/include/drm/drm
Re: [PATCH v3] drm/dp_mst: Fix return code on sideband message failure
On 2021-08-25 09:26, Lyude Paul wrote: The patch was pushed yes (was part of drm-misc-next-2021-07-29), seems like it just hasn't trickled down to linus's branch quite yet. Hi Stephen B, Would you mind back porting this patch to V5.10 branch? It will have lots of helps for us to support display port MST case. Thanks, On Wed, 2021-08-25 at 09:06 -0700, khs...@codeaurora.org wrote: On 2021-07-27 15:44, Lyude Paul wrote: > Nice timing, you literally got me as I was 2 minutes away from leaving > work > for the day :P. I will go ahead and push it now. > Hi Lyude, Had you pushed this patch yet? We still did not see this patch at msm-nex and v5.10 branch. Thanks, > BTW - in the future I recommend using dim to add Fixes: tags as it'll > add Cc: > to stable as appropriate (this patch in particular should be Cc: > sta...@vger.kernel.org # v5.3+). will add these tags when I push it > > On Tue, 2021-07-27 at 15:41 -0700, khs...@codeaurora.org wrote: > > On 2021-07-27 12:21, Lyude Paul wrote: > > > On Thu, 2021-07-22 at 15:28 -0700, khs...@codeaurora.org wrote: > > > > > > > > It looks like this patch is good to go (mainlined). > > > > Anything needed from me to do? > > > > Thanks, > > > > > > Do you have access for pushing this patch? If not let me know and I > > > can > > > go > > > ahead and push it to drm-misc-next for you. > > no, I do not have access to drm-misc-next. > > Please push it for me. > > Thanks a lots. > >
Re: [RFC] drm/msm/dp: Allow attaching a drm_panel
Hi, On Sat, Aug 28, 2021 at 7:40 AM Bjorn Andersson wrote: > > On Fri 27 Aug 15:52 CDT 2021, Doug Anderson wrote: > > > Hi, > > > > On Mon, Jul 26, 2021 at 4:15 PM Bjorn Andersson > > wrote: > > > > > > +static int dp_parser_find_panel(struct dp_parser *parser) > > > +{ > > > + struct device_node *np = parser->pdev->dev.of_node; > > > + int rc; > > > + > > > + rc = drm_of_find_panel_or_bridge(np, 2, 0, &parser->drm_panel, > > > NULL); > > > > Why port 2? Shouldn't this just be port 1 always? The yaml says that > > port 1 is "Output endpoint of the controller". We should just use port > > 1 here, right? > > > > I thought port 1 was the link to the Type-C controller, didn't give it a > second thought and took the next available. > > But per the binding it makes sense that the panel is the "Output > endpoint of the controller" and I guess one will have either a Type-C > controller or a panel - even after the DP rework? Right, my understanding is that "port 1" is the output port irregardless of whether you're outputting to a panel or a DP connector. I think the only case it would make sense to add a new port is if it was possible for the output to be connected to both a panel and a DP port simultaneously. ...but that doesn't make sense. -Doug
[PATCH v2] GPU:DRM: returning ENOMEM
check the return value and pass the proper error code. Signed-off-by: F.A. SULAIMAN --- drivers/gpu/drm/udl/udl_connector.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c index cdc1c42e1669..857d2c97ef0e 100644 --- a/drivers/gpu/drm/udl/udl_connector.c +++ b/drivers/gpu/drm/udl/udl_connector.c @@ -23,7 +23,7 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block, read_buff = kmalloc(2, GFP_KERNEL); if (!read_buff) - return -1; + return -ENOMEM; for (i = 0; i < len; i++) { int bval = (i + block * EDID_LENGTH) << 8; @@ -31,11 +31,16 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block, usb_rcvctrlpipe(udl->udev, 0), (0x02), (0x80 | (0x02 << 5)), bval, 0xA1, read_buff, 2, HZ); - if (ret < 1) { + if (ret == 0) { + DRM_ERROR("Reading EDID block %d returned empty result\n", i); + kfree(read_buff); + return -EINVAL; + } else if (ret < 0) { DRM_ERROR("Read EDID byte %d failed err %x\n", i, ret); kfree(read_buff); - return -1; + return ret; } + buf[i] = read_buff[1]; } -- 2.17.1
[PATCH] fbmem: don't allow too huge resolutions
syzbot is reporting page fault at vga16fb_fillrect() [1], for vga16fb_check_var() is failing to detect multiplication overflow. if (vxres * vyres > maxmem) { vyres = maxmem / vxres; if (vyres < yres) return -ENOMEM; } Since no module would accept too huge resolutions where multiplication overflow happens, let's reject in the common path. This patch does not use array_size(), for array_size() is allowed to return UINT_MAX on 32bits even if overflow did not happen. We want to detect only overflow here, for individual module will recheck with more strict limits as needed. Link: https://syzkaller.appspot.com/bug?extid=04168c8063cfdde1db5e [1] Reported-by: syzbot Debugged-by: Randy Dunlap Signed-off-by: Tetsuo Handa Tested-by: syzbot --- drivers/video/fbdev/core/fbmem.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 1c855145711b..9f5075dc2345 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1008,6 +1008,11 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var) if (var->xres < 8 || var->yres < 8) return -EINVAL; + /* Don't allow u32 * u32 to overflow. */ + if ((u64) var->xres * var->yres > UINT_MAX || + (u64) var->xres_virtual * var->yres_virtual > UINT_MAX) + return -EINVAL; + ret = info->fbops->fb_check_var(var, info); if (ret) -- 2.18.4
Re: [PATCH v2] GPU:DRM: returning ENOMEM
Maybe the commit message can be improved a bit? Add a prefix to make it clear this is about the udl driver, make it clear this is about the udl_get_edid_block function. The new `return ret` statement may return something different from ENOMEM. This would give something among these lines: drm/udl: return -errno in udl_get_edid_block
Re: [PATCH] drm/amdgpu/swsmu: fix spelling mistake "minimun" -> "minimum"
Applied. Thanks! Alex On Fri, Aug 27, 2021 at 2:59 PM Colin King wrote: > > From: Colin Ian King > > There are three identical spelling mistakes in dev_err messages. > Fix these. > > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 2 +- > drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 2 +- > drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > index ad52f7ecfb87..629bb8e926fb 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c > @@ -1869,7 +1869,7 @@ static int vangogh_od_edit_dpm_table(struct smu_context > *smu, enum PP_OD_DPM_TAB > } else { > if (smu->gfx_actual_hard_min_freq > > smu->gfx_actual_soft_max_freq) { > dev_err(smu->adev->dev, > - "The setting minimun sclk (%d) MHz is > greater than the setting maximum sclk (%d) MHz\n", > + "The setting minimum sclk (%d) MHz is > greater than the setting maximum sclk (%d) MHz\n", > smu->gfx_actual_hard_min_freq, > smu->gfx_actual_soft_max_freq); > return -EINVAL; > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c > b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c > index b39138041141..5aa175e12a78 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c > @@ -426,7 +426,7 @@ static int renoir_od_edit_dpm_table(struct smu_context > *smu, > } else { > if (smu->gfx_actual_hard_min_freq > > smu->gfx_actual_soft_max_freq) { > dev_err(smu->adev->dev, > - "The setting minimun sclk (%d) MHz is > greater than the setting maximum sclk (%d) MHz\n", > + "The setting minimum sclk (%d) MHz is > greater than the setting maximum sclk (%d) MHz\n", > smu->gfx_actual_hard_min_freq, > smu->gfx_actual_soft_max_freq); > return -EINVAL; > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c > b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c > index 0f17c2522c85..627ba2eec7fd 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c > @@ -731,7 +731,7 @@ static int yellow_carp_od_edit_dpm_table(struct > smu_context *smu, enum PP_OD_DPM > } else { > if (smu->gfx_actual_hard_min_freq > > smu->gfx_actual_soft_max_freq) { > dev_err(smu->adev->dev, > - "The setting minimun sclk (%d) MHz is > greater than the setting maximum sclk (%d) MHz\n", > + "The setting minimum sclk (%d) MHz is > greater than the setting maximum sclk (%d) MHz\n", > smu->gfx_actual_hard_min_freq, > smu->gfx_actual_soft_max_freq); > return -EINVAL; > -- > 2.32.0 >
Re: [PATCH][next] drm/amd/display: Fix unused initialization of pointer sink
Applied. Thanks! Alex On Sun, Aug 29, 2021 at 12:46 PM Colin King wrote: > > From: Colin Ian King > > Pointer sink is being inintialized with a value that is never read, > it is later being re-assigned a new value. Remove the redundant > initialization. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index e1e57e7465a7..9331c19fe9cb 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -10917,7 +10917,7 @@ void amdgpu_dm_update_freesync_caps(struct > drm_connector *connector, > struct amdgpu_dm_connector *amdgpu_dm_connector = > to_amdgpu_dm_connector(connector); > struct dm_connector_state *dm_con_state = NULL; > - struct dc_sink *sink = amdgpu_dm_connector->dc_sink; > + struct dc_sink *sink; > > struct drm_device *dev = connector->dev; > struct amdgpu_device *adev = drm_to_adev(dev); > -- > 2.32.0 >
Re: [PATCH] drm: remove zxdrm driver
On Thu, Aug 26, 2021 at 4:22 AM Daniel Vetter wrote: > > On Mon, Aug 23, 2021 at 01:51:30PM -0500, Rob Herring wrote: > > On Sat, 21 Aug 2021 11:13:57 +0800, Zenghui Yu wrote: > > > The zte zx platform had been removed in commit 89d4f98ae90d ("ARM: remove > > > zte zx platform"), so this driver is no longer needed. > > > > > > Cc: Arnd Bergmann > > > Cc: Jun Nie > > > Cc: Shawn Guo > > > Signed-off-by: Zenghui Yu > > > --- > > > .../devicetree/bindings/display/zte,vou.txt | 120 --- > > > drivers/gpu/drm/Kconfig | 2 - > > > drivers/gpu/drm/Makefile | 1 - > > > drivers/gpu/drm/zte/Kconfig | 10 - > > > drivers/gpu/drm/zte/Makefile | 10 - > > > drivers/gpu/drm/zte/zx_common_regs.h | 28 - > > > drivers/gpu/drm/zte/zx_drm_drv.c | 190 > > > drivers/gpu/drm/zte/zx_drm_drv.h | 34 - > > > drivers/gpu/drm/zte/zx_hdmi.c | 760 --- > > > drivers/gpu/drm/zte/zx_hdmi_regs.h| 66 -- > > > drivers/gpu/drm/zte/zx_plane.c| 537 -- > > > drivers/gpu/drm/zte/zx_plane.h| 26 - > > > drivers/gpu/drm/zte/zx_plane_regs.h | 120 --- > > > drivers/gpu/drm/zte/zx_tvenc.c| 400 > > > drivers/gpu/drm/zte/zx_tvenc_regs.h | 27 - > > > drivers/gpu/drm/zte/zx_vga.c | 527 -- > > > drivers/gpu/drm/zte/zx_vga_regs.h | 33 - > > > drivers/gpu/drm/zte/zx_vou.c | 921 -- > > > drivers/gpu/drm/zte/zx_vou.h | 64 -- > > > drivers/gpu/drm/zte/zx_vou_regs.h | 212 > > > 20 files changed, 4088 deletions(-) > > > delete mode 100644 Documentation/devicetree/bindings/display/zte,vou.txt > > > delete mode 100644 drivers/gpu/drm/zte/Kconfig > > > delete mode 100644 drivers/gpu/drm/zte/Makefile > > > delete mode 100644 drivers/gpu/drm/zte/zx_common_regs.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_drm_drv.c > > > delete mode 100644 drivers/gpu/drm/zte/zx_drm_drv.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_hdmi.c > > > delete mode 100644 drivers/gpu/drm/zte/zx_hdmi_regs.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_plane.c > > > delete mode 100644 drivers/gpu/drm/zte/zx_plane.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_plane_regs.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_tvenc.c > > > delete mode 100644 drivers/gpu/drm/zte/zx_tvenc_regs.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_vga.c > > > delete mode 100644 drivers/gpu/drm/zte/zx_vga_regs.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_vou.c > > > delete mode 100644 drivers/gpu/drm/zte/zx_vou.h > > > delete mode 100644 drivers/gpu/drm/zte/zx_vou_regs.h > > > > > > > Acked-by: Rob Herring > > I just merged another patch to delete the zte driver. Unfortunately, that one missed the binding doc, so please send me a patch removing the binding doc. Rob
Re: [v4 3/4] drm/panel: support for BOE and INX video mode panel
Hi, On Sun, Aug 29, 2021 at 7:39 PM yangcong wrote: > > Support for these two panels fits in nicely with the existing > panel-boe-tv101wum-nl6 driver as suggested by Sam [1]. The main things > we needed to handle were: > a) These panels need slightly longer delays in two places. Since these > new delays aren't much longer, let's just unconditionally increase > them for the driver. > b) One of these two panels doesn't support DSI HS mode so this patch > adds a flag for a panel to disable that. > > [1] https://lore.kernel.org/r/yspasee6wd8dd...@ravnborg.org/ > > Signed-off-by: yangcong > --- > .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 915 +- > 1 file changed, 912 insertions(+), 3 deletions(-) This looks fine to me now. Reviewed-by: Douglas Anderson I'm happy to help land this series or for others to land it. For now my plan is to let them sit for a couple of weeks and if there is silence I will plan to land them in drm-misc-next. I'm happy to land earlier with an Ack or I'm happy to step back if someone else wants to take charge. ;-) -Doug
Re: [Intel-gfx] [PATCH v3] drm/i915/dp: Use max params for panels < eDP 1.4
On Thu, Aug 26, 2021 at 01:37:34PM -0400, Rodrigo Vivi wrote: > On Fri, Aug 20, 2021 at 08:26:14PM +0300, Ville Syrjälä wrote: > > On Fri, Aug 20, 2021 at 03:52:59PM +0800, Kai-Heng Feng wrote: > > > Users reported that after commit 2bbd6dba84d4 ("drm/i915: Try to use > > > fast+narrow link on eDP again and fall back to the old max strategy on > > > failure"), the screen starts to have wobbly effect. > > > > > > Commit a5c936add6a2 ("drm/i915/dp: Use slow and wide link training for > > > everything") doesn't help either, that means the affected eDP 1.2 panels > > > only work with max params. > > > > > > So use max params for panels < eDP 1.4 as Windows does to solve the > > > issue. > > > > > > v3: > > > - Do the eDP rev check in intel_edp_init_dpcd() > > > > > > v2: > > > - Check eDP 1.4 instead of DPCD 1.1 to apply max params > > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3714 > > > Fixes: 2bbd6dba84d4 ("drm/i915: Try to use fast+narrow link on eDP again > > > and fall back to the old max strategy on failure") > > > Fixes: a5c936add6a2 ("drm/i915/dp: Use slow and wide link training for > > > everything") > > > Suggested-by: Ville Syrjälä > > > Signed-off-by: Kai-Heng Feng > > > > Slapped a cc:stable on it and pushed to drm-intel-next. Thanks. > > Since I got a strange failure on CI_DIF_604 that I don't see on CI_DIF_603, > I'm avoiding the display patches. This one and also > dab1b47e57e0 ("drm/i915/dp: return proper DPRX link training result") > > I know, it is probably the other one, but I had to remove both patches for > now and I'm not confident the CI will allow me to test with this one alone. > > If we have -rc8 I will check again later. Otherwise we will have to send > to the stable mailing list later. CI didn't run on TGL again, so I couldn't send this patch last week. And 5.14 got released. If this is important for 5.14 or any other stable release, please confirm this is not the one breaking linking training on TGL and then please send it to the stable mailing list. https://www.kernel.org/doc/Documentation/process/stable-kernel-rules.rst Sorry, Rodrigo. > > > > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 5 - > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 75d4ebc669411..e0dbd35ae7bc0 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -2445,11 +2445,14 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp) > > >*/ > > > if (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, > > >intel_dp->edp_dpcd, sizeof(intel_dp->edp_dpcd)) == > > > - sizeof(intel_dp->edp_dpcd)) > > > + sizeof(intel_dp->edp_dpcd)) { > > > drm_dbg_kms(&dev_priv->drm, "eDP DPCD: %*ph\n", > > > (int)sizeof(intel_dp->edp_dpcd), > > > intel_dp->edp_dpcd); > > > > > > + intel_dp->use_max_params = intel_dp->edp_dpcd[0] < DP_EDP_14; > > > + } > > > + > > > /* > > >* This has to be called after intel_dp->edp_dpcd is filled, PSR checks > > >* for SET_POWER_CAPABLE bit in intel_dp->edp_dpcd[1] > > > -- > > > 2.32.0 > > > > -- > > Ville Syrjälä > > Intel
Re: [PATCH v3 1/4] drm/ttm: Create pinned list
Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky: On 2021-08-30 4:58 a.m., Christian König wrote: Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky: This list will be used to capture all non VRAM BOs not on LRU so when device is hot unplugged we can iterate the list and unmap DMA mappings before device is removed. v2: Reanme function to ttm_bo_move_to_pinned v3: Move the pinned list to ttm device As far as I can see there is not list_del() remaining. So this won't work correctly. It's in ttm_bo_release, there was no code change there hence it's not captured in the patch. Ah! So you keep the logic as is there. Sorry totally missed that. In this case the patch is Reviewed-by: Christian König Can you push this to drm-misc-next? Thanks, Christian. Andrey I suggest to rather rebase on top of the stuff I'm working on for a while to move the LRU into the resource instead. Just send out the latest patch set of this with you in CC. Christian. Signed-off-by: Andrey Grodzovsky Suggested-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++ drivers/gpu/drm/ttm/ttm_device.c | 1 + include/drm/ttm/ttm_device.h | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b950b45cf4b..1fedd0eb67ba 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) +{ + struct ttm_device *bdev = bo->bdev; + + list_move_tail(&bo->lru, &bdev->pinned); + + if (bdev->funcs->del_from_lru_notify) + bdev->funcs->del_from_lru_notify(bo); +} + +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { struct ttm_device *bdev = bo->bdev; @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, dma_resv_assert_held(bo->base.resv); if (bo->pin_count) { - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); return; } @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, return ret; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); list_del_init(&bo->ddestroy); spin_unlock(&bo->bdev->lru_lock); ttm_bo_cleanup_memtype_use(bo); @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, return 0; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); /* TODO: Cleanup the locking */ spin_unlock(&bo->bdev->lru_lock); diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 5f31acec3ad7..530a9c36be37 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs, INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue); spin_lock_init(&bdev->lru_lock); INIT_LIST_HEAD(&bdev->ddestroy); + INIT_LIST_HEAD(&bdev->pinned); bdev->dev_mapping = mapping; mutex_lock(&ttm_global_mutex); list_add_tail(&bdev->device_list, &glob->device_list); diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index cd592f8e941b..03fb44d061e0 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -265,6 +265,7 @@ struct ttm_device { */ spinlock_t lru_lock; struct list_head ddestroy; + struct list_head pinned; /* * Protected by load / firstopen / lastclose /unload sync.
Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
On 2021-08-30 4:57 a.m., Christian König wrote: This way we finally fix the problem that new resource are not immediately evict-able after allocation. That has caused numerous problems including OOM on GDS handling and not being able to use TTM as general resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c| 105 ++-- drivers/gpu/drm/ttm/ttm_bo_util.c | 1 - drivers/gpu/drm/ttm/ttm_device.c| 8 +- drivers/gpu/drm/ttm/ttm_resource.c | 127 include/drm/ttm/ttm_bo_api.h| 16 --- include/drm/ttm/ttm_bo_driver.h | 29 +- include/drm/ttm/ttm_resource.h | 35 +++ 9 files changed, 181 insertions(+), 150 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6362e861a3f5..70d2cbb1dbb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (vm->bulk_moveable) { spin_lock(&adev->mman.bdev.lru_lock); - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move); + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); return; } - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move)); + ttm_lru_bulk_move_init(&vm->lru_bulk_move); spin_lock(&adev->mman.bdev.lru_lock); list_for_each_entry(bo_base, &vm->idle, vm_status) { @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (!bo->parent) continue; - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource, - &vm->lru_bulk_move); + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); if (shadow) ttm_bo_move_to_lru_tail(&shadow->tbo, - shadow->tbo.resource, &vm->lru_bulk_move); } spin_unlock(&adev->mman.bdev.lru_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e646aac9d7a4..41f0de841d72 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) bo->priority = I915_TTM_PRIO_NO_PAGES; } - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->lru_lock); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 49f4bc97c35a..d5c6e096fd31 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_del_init(&bo->lru); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos, -struct ttm_buffer_object *bo) -{ - if (!pos->first) - pos->first = bo; - pos->last = bo; -} - void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, -struct ttm_resource *mem, struct ttm_lru_bulk_move *bulk) { - struct ttm_device *bdev = bo->bdev; - struct ttm_resource_manager *man; - - if (!bo->deleted) - dma_resv_assert_held(bo->base.resv); - - if (bo->pin_count) { - ttm_bo_del_from_lru(bo); - return; - } - - if (!mem) - return; - - man = ttm_manager_type(bdev, mem->mem_type); - list_move_tail(&bo->lru, &man->lru[bo->priority]); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); - - if (bulk && !bo->pin_count) { - switch (bo->resource->mem_type) { - case TTM_PL_TT: - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo); - break; + dma_resv_assert_held(bo->base.resv); - case TTM_PL_VRAM: - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo); - break; - } - } + if (bo->resource) + ttm_resource_move_to_lru_tail(bo->resource, bulk); } EXPORT_SYMBOL(ttm_bo_move_to_lru_tail); -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
Am 30.08.21 um 18:53 schrieb Andrey Grodzovsky: On 2021-08-30 4:57 a.m., Christian König wrote: This way we finally fix the problem that new resource are not immediately evict-able after allocation. That has caused numerous problems including OOM on GDS handling and not being able to use TTM as general resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 105 ++-- drivers/gpu/drm/ttm/ttm_bo_util.c | 1 - drivers/gpu/drm/ttm/ttm_device.c | 8 +- drivers/gpu/drm/ttm/ttm_resource.c | 127 include/drm/ttm/ttm_bo_api.h | 16 --- include/drm/ttm/ttm_bo_driver.h | 29 +- include/drm/ttm/ttm_resource.h | 35 +++ 9 files changed, 181 insertions(+), 150 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6362e861a3f5..70d2cbb1dbb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (vm->bulk_moveable) { spin_lock(&adev->mman.bdev.lru_lock); - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move); + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); return; } - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move)); + ttm_lru_bulk_move_init(&vm->lru_bulk_move); spin_lock(&adev->mman.bdev.lru_lock); list_for_each_entry(bo_base, &vm->idle, vm_status) { @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (!bo->parent) continue; - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource, - &vm->lru_bulk_move); + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); if (shadow) ttm_bo_move_to_lru_tail(&shadow->tbo, - shadow->tbo.resource, &vm->lru_bulk_move); } spin_unlock(&adev->mman.bdev.lru_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e646aac9d7a4..41f0de841d72 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) bo->priority = I915_TTM_PRIO_NO_PAGES; } - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->lru_lock); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 49f4bc97c35a..d5c6e096fd31 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_del_init(&bo->lru); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos, - struct ttm_buffer_object *bo) -{ - if (!pos->first) - pos->first = bo; - pos->last = bo; -} - void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, - struct ttm_resource *mem, struct ttm_lru_bulk_move *bulk) { - struct ttm_device *bdev = bo->bdev; - struct ttm_resource_manager *man; - - if (!bo->deleted) - dma_resv_assert_held(bo->base.resv); - - if (bo->pin_count) { - ttm_bo_del_from_lru(bo); - return; - } - - if (!mem) - return; - - man = ttm_manager_type(bdev, mem->mem_type); - list_move_tail(&bo->lru, &man->lru[bo->priority]); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); - - if (bulk && !bo->pin_count) { - switch (bo->resource->mem_type) { - case TTM_PL_TT: - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo); - break; + dma_resv_assert_held(bo->base.resv); - case TTM_PL_VRAM: - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo); - break; - } - } + if (bo->resource) + ttm_resource_move_to_lru_tail(bo->resource, bulk); } EXPORT_SYMBOL(ttm_bo_move_to_lru_tail); -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk) -{ - unsigned i; - - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i]; - struct ttm_resource_manager *man; - - if (!pos->first) - continue; - - dma_resv_assert_held(pos->first->base.resv); - dma_resv_assert_held(pos->last->base.resv);
Re: [PATCH v3] drm/dp_mst: Fix return code on sideband message failure
On Mon, 2021-08-30 at 08:56 -0700, khs...@codeaurora.org wrote: > On 2021-08-25 09:26, Lyude Paul wrote: > > The patch was pushed yes (was part of drm-misc-next-2021-07-29), seems > > like it > > just hasn't trickled down to linus's branch quite yet. > > Hi Stephen B, > > Would you mind back porting this patch to V5.10 branch? > It will have lots of helps for us to support display port MST case. > Thanks, I'm assuming you're talking to someone else? A little confused because I don't see a Stephen B in this thread > > > > > > > On Wed, 2021-08-25 at 09:06 -0700, khs...@codeaurora.org wrote: > > > On 2021-07-27 15:44, Lyude Paul wrote: > > > > Nice timing, you literally got me as I was 2 minutes away from leaving > > > > work > > > > for the day :P. I will go ahead and push it now. > > > > > > > Hi Lyude, > > > > > > Had you pushed this patch yet? > > > We still did not see this patch at msm-nex and v5.10 branch. > > > Thanks, > > > > > > > > > > BTW - in the future I recommend using dim to add Fixes: tags as it'll > > > > add Cc: > > > > to stable as appropriate (this patch in particular should be Cc: > > > > sta...@vger.kernel.org # v5.3+). will add these tags when I push it > > > > > > > > On Tue, 2021-07-27 at 15:41 -0700, khs...@codeaurora.org wrote: > > > > > On 2021-07-27 12:21, Lyude Paul wrote: > > > > > > On Thu, 2021-07-22 at 15:28 -0700, khs...@codeaurora.org wrote: > > > > > > > > > > > > > > It looks like this patch is good to go (mainlined). > > > > > > > Anything needed from me to do? > > > > > > > Thanks, > > > > > > > > > > > > Do you have access for pushing this patch? If not let me know and > > > > > > I > > > > > > can > > > > > > go > > > > > > ahead and push it to drm-misc-next for you. > > > > > no, I do not have access to drm-misc-next. > > > > > Please push it for me. > > > > > Thanks a lots. > > > > > > > > > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
On 2021-08-30 12:55 p.m., Christian König wrote: Am 30.08.21 um 18:53 schrieb Andrey Grodzovsky: On 2021-08-30 4:57 a.m., Christian König wrote: This way we finally fix the problem that new resource are not immediately evict-able after allocation. That has caused numerous problems including OOM on GDS handling and not being able to use TTM as general resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 105 ++-- drivers/gpu/drm/ttm/ttm_bo_util.c | 1 - drivers/gpu/drm/ttm/ttm_device.c | 8 +- drivers/gpu/drm/ttm/ttm_resource.c | 127 include/drm/ttm/ttm_bo_api.h | 16 --- include/drm/ttm/ttm_bo_driver.h | 29 +- include/drm/ttm/ttm_resource.h | 35 +++ 9 files changed, 181 insertions(+), 150 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6362e861a3f5..70d2cbb1dbb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (vm->bulk_moveable) { spin_lock(&adev->mman.bdev.lru_lock); - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move); + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); return; } - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move)); + ttm_lru_bulk_move_init(&vm->lru_bulk_move); spin_lock(&adev->mman.bdev.lru_lock); list_for_each_entry(bo_base, &vm->idle, vm_status) { @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (!bo->parent) continue; - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource, - &vm->lru_bulk_move); + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); if (shadow) ttm_bo_move_to_lru_tail(&shadow->tbo, - shadow->tbo.resource, &vm->lru_bulk_move); } spin_unlock(&adev->mman.bdev.lru_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e646aac9d7a4..41f0de841d72 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) bo->priority = I915_TTM_PRIO_NO_PAGES; } - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->lru_lock); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 49f4bc97c35a..d5c6e096fd31 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_del_init(&bo->lru); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos, - struct ttm_buffer_object *bo) -{ - if (!pos->first) - pos->first = bo; - pos->last = bo; -} - void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, - struct ttm_resource *mem, struct ttm_lru_bulk_move *bulk) { - struct ttm_device *bdev = bo->bdev; - struct ttm_resource_manager *man; - - if (!bo->deleted) - dma_resv_assert_held(bo->base.resv); - - if (bo->pin_count) { - ttm_bo_del_from_lru(bo); - return; - } - - if (!mem) - return; - - man = ttm_manager_type(bdev, mem->mem_type); - list_move_tail(&bo->lru, &man->lru[bo->priority]); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); - - if (bulk && !bo->pin_count) { - switch (bo->resource->mem_type) { - case TTM_PL_TT: - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo); - break; + dma_resv_assert_held(bo->base.resv); - case TTM_PL_VRAM: - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo); - break; - } - } + if (bo->resource) + ttm_resource_move_to_lru_tail(bo->resource, bulk); } EXPORT_SYMBOL(ttm_bo_move_to_lru_tail); -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk) -{ - unsigned i; - - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i]; - struct ttm_resource_manager *man; - - if (!pos->first) - continue; - - dma_resv_assert_held(pos->first->base.resv
Re: [PATCH 05/12] drm/ttm: move the LRU into resource handling
Am 30.08.21 um 19:00 schrieb Andrey Grodzovsky: On 2021-08-30 12:55 p.m., Christian König wrote: Am 30.08.21 um 18:53 schrieb Andrey Grodzovsky: On 2021-08-30 4:57 a.m., Christian König wrote: This way we finally fix the problem that new resource are not immediately evict-able after allocation. That has caused numerous problems including OOM on GDS handling and not being able to use TTM as general resource manager. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 105 ++-- drivers/gpu/drm/ttm/ttm_bo_util.c | 1 - drivers/gpu/drm/ttm/ttm_device.c | 8 +- drivers/gpu/drm/ttm/ttm_resource.c | 127 include/drm/ttm/ttm_bo_api.h | 16 --- include/drm/ttm/ttm_bo_driver.h | 29 +- include/drm/ttm/ttm_resource.h | 35 +++ 9 files changed, 181 insertions(+), 150 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 6362e861a3f5..70d2cbb1dbb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (vm->bulk_moveable) { spin_lock(&adev->mman.bdev.lru_lock); - ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move); + ttm_lru_bulk_move_tail(&vm->lru_bulk_move); spin_unlock(&adev->mman.bdev.lru_lock); return; } - memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move)); + ttm_lru_bulk_move_init(&vm->lru_bulk_move); spin_lock(&adev->mman.bdev.lru_lock); list_for_each_entry(bo_base, &vm->idle, vm_status) { @@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev, if (!bo->parent) continue; - ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource, - &vm->lru_bulk_move); + ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move); if (shadow) ttm_bo_move_to_lru_tail(&shadow->tbo, - shadow->tbo.resource, &vm->lru_bulk_move); } spin_unlock(&adev->mman.bdev.lru_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e646aac9d7a4..41f0de841d72 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -471,7 +471,7 @@ static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) bo->priority = I915_TTM_PRIO_NO_PAGES; } - ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->lru_lock); } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 49f4bc97c35a..d5c6e096fd31 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,98 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) -{ - struct ttm_device *bdev = bo->bdev; - - list_del_init(&bo->lru); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); -} - -static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos, - struct ttm_buffer_object *bo) -{ - if (!pos->first) - pos->first = bo; - pos->last = bo; -} - void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, - struct ttm_resource *mem, struct ttm_lru_bulk_move *bulk) { - struct ttm_device *bdev = bo->bdev; - struct ttm_resource_manager *man; - - if (!bo->deleted) - dma_resv_assert_held(bo->base.resv); - - if (bo->pin_count) { - ttm_bo_del_from_lru(bo); - return; - } - - if (!mem) - return; - - man = ttm_manager_type(bdev, mem->mem_type); - list_move_tail(&bo->lru, &man->lru[bo->priority]); - - if (bdev->funcs->del_from_lru_notify) - bdev->funcs->del_from_lru_notify(bo); - - if (bulk && !bo->pin_count) { - switch (bo->resource->mem_type) { - case TTM_PL_TT: - ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo); - break; + dma_resv_assert_held(bo->base.resv); - case TTM_PL_VRAM: - ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo); - break; - } - } + if (bo->resource) + ttm_resource_move_to_lru_tail(bo->resource, bulk); } EXPORT_SYMBOL(ttm_bo_move_to_lru_tail); -void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk) -{ - unsigned i; - - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) { - struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i]; - struct ttm_resource_manager *man; - - if (!pos->first) - continue; - - d
Re: [PATCH v3 1/4] drm/ttm: Create pinned list
Am 30.08.21 um 19:02 schrieb Andrey Grodzovsky: On 2021-08-30 12:51 p.m., Christian König wrote: Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky: On 2021-08-30 4:58 a.m., Christian König wrote: Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky: This list will be used to capture all non VRAM BOs not on LRU so when device is hot unplugged we can iterate the list and unmap DMA mappings before device is removed. v2: Reanme function to ttm_bo_move_to_pinned v3: Move the pinned list to ttm device As far as I can see there is not list_del() remaining. So this won't work correctly. It's in ttm_bo_release, there was no code change there hence it's not captured in the patch. Ah! So you keep the logic as is there. Sorry totally missed that. In this case the patch is Reviewed-by: Christian König Can you push this to drm-misc-next? Thanks, Christian. I think It's supposed to go on top of your changes you mention here which are not pushed yet. I will need to apply all the patches on top of yours and retest (I was doing everything in amd-staging-drm-next) until now. Works for me as well. Alternatively you can just push this patch here to drm-misc-next so that I can rebase on top and merge the rest through amd-staging-drm-next. The patch pushed to drm-misc-next should automatically fall out when Alex rebases his stuff on upstream the next time. Christian. Andrey Andrey I suggest to rather rebase on top of the stuff I'm working on for a while to move the LRU into the resource instead. Just send out the latest patch set of this with you in CC. Christian. Signed-off-by: Andrey Grodzovsky Suggested-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++ drivers/gpu/drm/ttm/ttm_device.c | 1 + include/drm/ttm/ttm_device.h | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b950b45cf4b..1fedd0eb67ba 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) +{ + struct ttm_device *bdev = bo->bdev; + + list_move_tail(&bo->lru, &bdev->pinned); + + if (bdev->funcs->del_from_lru_notify) + bdev->funcs->del_from_lru_notify(bo); +} + +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { struct ttm_device *bdev = bo->bdev; @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, dma_resv_assert_held(bo->base.resv); if (bo->pin_count) { - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); return; } @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, return ret; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); list_del_init(&bo->ddestroy); spin_unlock(&bo->bdev->lru_lock); ttm_bo_cleanup_memtype_use(bo); @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, return 0; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); /* TODO: Cleanup the locking */ spin_unlock(&bo->bdev->lru_lock); diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 5f31acec3ad7..530a9c36be37 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs, INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue); spin_lock_init(&bdev->lru_lock); INIT_LIST_HEAD(&bdev->ddestroy); + INIT_LIST_HEAD(&bdev->pinned); bdev->dev_mapping = mapping; mutex_lock(&ttm_global_mutex); list_add_tail(&bdev->device_list, &glob->device_list); diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index cd592f8e941b..03fb44d061e0 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -265,6 +265,7 @@ struct ttm_device { */ spinlock_t lru_lock; struct list_head ddestroy; + struct list_head pinned; /* * Protected by load / firstopen / lastclose /unload sync.
Re: [PATCH AUTOSEL 5.13 20/26] drm/nouveau: recognise GA107
ahhh-ok, that makes these patchs make a bit more sense then. If Ben doesn't have any objections I'd say these are fine to backport then On Mon, 2021-08-30 at 08:17 -0400, Sasha Levin wrote: > On Tue, Aug 24, 2021 at 01:08:28PM -0400, Lyude Paul wrote: > > This is more hardware enablement, I'm not sure this should be going into > > stable either. Ben? > > We take this sort of hardware enablement patches (where the platform > code is already there, and we just add quirks/ids/etc. > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH AUTOSEL 5.13 20/26] drm/nouveau: recognise GA107
oops-except for "drm/nouveau: block a bunch of classes from userspace" of course. the rest are fine though On Mon, 2021-08-30 at 13:08 -0400, Lyude Paul wrote: > ahhh-ok, that makes these patchs make a bit more sense then. If Ben doesn't > have any objections I'd say these are fine to backport then > > On Mon, 2021-08-30 at 08:17 -0400, Sasha Levin wrote: > > On Tue, Aug 24, 2021 at 01:08:28PM -0400, Lyude Paul wrote: > > > This is more hardware enablement, I'm not sure this should be going into > > > stable either. Ben? > > > > We take this sort of hardware enablement patches (where the platform > > code is already there, and we just add quirks/ids/etc. > > > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH v3 1/4] drm/ttm: Create pinned list
On 2021-08-30 12:51 p.m., Christian König wrote: Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky: On 2021-08-30 4:58 a.m., Christian König wrote: Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky: This list will be used to capture all non VRAM BOs not on LRU so when device is hot unplugged we can iterate the list and unmap DMA mappings before device is removed. v2: Reanme function to ttm_bo_move_to_pinned v3: Move the pinned list to ttm device As far as I can see there is not list_del() remaining. So this won't work correctly. It's in ttm_bo_release, there was no code change there hence it's not captured in the patch. Ah! So you keep the logic as is there. Sorry totally missed that. In this case the patch is Reviewed-by: Christian König Can you push this to drm-misc-next? Thanks, Christian. I think It's supposed to go on top of your changes you mention here which are not pushed yet. I will need to apply all the patches on top of yours and retest (I was doing everything in amd-staging-drm-next) until now. Andrey Andrey I suggest to rather rebase on top of the stuff I'm working on for a while to move the LRU into the resource instead. Just send out the latest patch set of this with you in CC. Christian. Signed-off-by: Andrey Grodzovsky Suggested-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++ drivers/gpu/drm/ttm/ttm_device.c | 1 + include/drm/ttm/ttm_device.h | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b950b45cf4b..1fedd0eb67ba 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) +{ + struct ttm_device *bdev = bo->bdev; + + list_move_tail(&bo->lru, &bdev->pinned); + + if (bdev->funcs->del_from_lru_notify) + bdev->funcs->del_from_lru_notify(bo); +} + +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { struct ttm_device *bdev = bo->bdev; @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, dma_resv_assert_held(bo->base.resv); if (bo->pin_count) { - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); return; } @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, return ret; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); list_del_init(&bo->ddestroy); spin_unlock(&bo->bdev->lru_lock); ttm_bo_cleanup_memtype_use(bo); @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, return 0; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); /* TODO: Cleanup the locking */ spin_unlock(&bo->bdev->lru_lock); diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 5f31acec3ad7..530a9c36be37 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs, INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue); spin_lock_init(&bdev->lru_lock); INIT_LIST_HEAD(&bdev->ddestroy); + INIT_LIST_HEAD(&bdev->pinned); bdev->dev_mapping = mapping; mutex_lock(&ttm_global_mutex); list_add_tail(&bdev->device_list, &glob->device_list); diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index cd592f8e941b..03fb44d061e0 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -265,6 +265,7 @@ struct ttm_device { */ spinlock_t lru_lock; struct list_head ddestroy; + struct list_head pinned; /* * Protected by load / firstopen / lastclose /unload sync.
Re: [PATCH v1 03/14] mm: add iomem vma selection for memory migration
Am 2021-08-30 um 4:28 a.m. schrieb Christoph Hellwig: > On Thu, Aug 26, 2021 at 06:27:31PM -0400, Felix Kuehling wrote: >> I think we're missing something here. As far as I can tell, all the work >> we did first with DEVICE_GENERIC and now DEVICE_PUBLIC always used >> normal pages. Are we missing something in our driver code that would >> make these PTEs special? I don't understand how that can be, because >> driver code is not really involved in updating the CPU mappings. Maybe >> it's something we need to do in the migration helpers. > It looks like I'm totally misunderstanding what you are adding here > then. Why do we need any special treatment at all for memory that > has normal struct pages and is part of the direct kernel map? The pages are like normal memory for purposes of mapping them in CPU page tables and for coherent access from the CPU. From an application perspective, we want file-backed and anonymous mappings to be able to use DEVICE_PUBLIC pages with coherent CPU access. The goal is to optimize performance for GPU heavy workloads while minimizing the need to migrate data back-and-forth between system memory and device memory. The pages are special in two ways: 1. The memory is managed not by the Linux buddy allocator, but by the GPU driver's TTM memory manager 2. We want to migrate data in response to GPU page faults and application hints using the migrate_vma helpers It's the second part that we're really trying to address with this patch series. Regards, Felix
Re: [PATCH v3 1/4] drm/ttm: Create pinned list
On 2021-08-30 1:05 p.m., Christian König wrote: Am 30.08.21 um 19:02 schrieb Andrey Grodzovsky: On 2021-08-30 12:51 p.m., Christian König wrote: Am 30.08.21 um 16:16 schrieb Andrey Grodzovsky: On 2021-08-30 4:58 a.m., Christian König wrote: Am 27.08.21 um 22:39 schrieb Andrey Grodzovsky: This list will be used to capture all non VRAM BOs not on LRU so when device is hot unplugged we can iterate the list and unmap DMA mappings before device is removed. v2: Reanme function to ttm_bo_move_to_pinned v3: Move the pinned list to ttm device As far as I can see there is not list_del() remaining. So this won't work correctly. It's in ttm_bo_release, there was no code change there hence it's not captured in the patch. Ah! So you keep the logic as is there. Sorry totally missed that. In this case the patch is Reviewed-by: Christian König Can you push this to drm-misc-next? Thanks, Christian. I think It's supposed to go on top of your changes you mention here which are not pushed yet. I will need to apply all the patches on top of yours and retest (I was doing everything in amd-staging-drm-next) until now. Works for me as well. Alternatively you can just push this patch here to drm-misc-next so that I can rebase on top and merge the rest through amd-staging-drm-next. The patch pushed to drm-misc-next should automatically fall out when Alex rebases his stuff on upstream the next time. Christian. So i can both push this specific patch to drm-misc-next and also push the entire 4 patch series to amd-stagin-drm-next (after rest of the patches RBed)? Andrey Andrey Andrey I suggest to rather rebase on top of the stuff I'm working on for a while to move the LRU into the resource instead. Just send out the latest patch set of this with you in CC. Christian. Signed-off-by: Andrey Grodzovsky Suggested-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++ drivers/gpu/drm/ttm/ttm_device.c | 1 + include/drm/ttm/ttm_device.h | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 1b950b45cf4b..1fedd0eb67ba 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -69,7 +69,17 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, } } -static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) +static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo) +{ + struct ttm_device *bdev = bo->bdev; + + list_move_tail(&bo->lru, &bdev->pinned); + + if (bdev->funcs->del_from_lru_notify) + bdev->funcs->del_from_lru_notify(bo); +} + +static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo) { struct ttm_device *bdev = bo->bdev; @@ -98,7 +108,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo, dma_resv_assert_held(bo->base.resv); if (bo->pin_count) { - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); return; } @@ -339,7 +349,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, return ret; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); list_del_init(&bo->ddestroy); spin_unlock(&bo->bdev->lru_lock); ttm_bo_cleanup_memtype_use(bo); @@ -1154,7 +1164,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, return 0; } - ttm_bo_del_from_lru(bo); + ttm_bo_move_to_pinned(bo); /* TODO: Cleanup the locking */ spin_unlock(&bo->bdev->lru_lock); diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index 5f31acec3ad7..530a9c36be37 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -208,6 +208,7 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs, INIT_DELAYED_WORK(&bdev->wq, ttm_device_delayed_workqueue); spin_lock_init(&bdev->lru_lock); INIT_LIST_HEAD(&bdev->ddestroy); + INIT_LIST_HEAD(&bdev->pinned); bdev->dev_mapping = mapping; mutex_lock(&ttm_global_mutex); list_add_tail(&bdev->device_list, &glob->device_list); diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index cd592f8e941b..03fb44d061e0 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -265,6 +265,7 @@ struct ttm_device { */ spinlock_t lru_lock; struct list_head ddestroy; + struct list_head pinned; /* * Protected by load / firstopen / lastclose /unload sync.
[PATCH v2 0/2] Use "ref" clocks from firmware for DSI PLL VCO parent
All DSI PHY/PLL drivers were referencing their VCO parent clock by a global name, most of which don't exist or have been renamed. These clock drivers seem to function fine without that except the 14nm driver for the sdm6xx [1]. At the same time all DTs provide a "ref" clock as per the requirements of dsi-phy-common.yaml, but the clock is never used. This patchset puts that clock to use without relying on a global clock name, so that all dependencies are explicitly defined in DT (the firmware) in the end. [1]: https://lore.kernel.org/linux-arm-msm/386db1a6-a1cd-3c7d-a88e-dc83f8a1b...@somainline.org/ Changes since v1: - Dropped "arm: dts: qcom: apq8064: Use 27MHz PXO clock as DSI PLL reference" which has made its way into 5.15-fixes in advance of this patchset landing in 5.16. - Added Fixes: tags for commits that added missing "ref" clocks to DT while this firmware clock was never used (until this patchset). - Documented missing/wrong and later-added clocks (by aforementioned patches) in patch 1/2 more clearly. Dmitry: I have not added the .name="xo" fallback to the 28nm-hpm driver for the missing "ref" clock in msm8974 yet. This patch is supposed to make it in for 5.16 while the missing clock should be added in 5.15, is that enough time? If not I'll gladly respin a v3 with that fallback, but I hope everyone can update their DT firmware before that time. Likewise Bjorn acknowledged that there is enough time for the same to happen on apq8064. Marijn Suijten (2): drm/msm/dsi: Use "ref" fw clock instead of global name for VCO parent clk: qcom: gcc-sdm660: Remove transient global "xo" clock drivers/clk/qcom/gcc-sdm660.c | 14 -- drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 4 +++- 6 files changed, 15 insertions(+), 19 deletions(-) -- 2.33.0
[PATCH v2 1/2] drm/msm/dsi: Use "ref" fw clock instead of global name for VCO parent
All DSI PHY/PLL drivers were referencing their VCO parent clock by a global name, most of which don't exist or have been renamed. These clock drivers seem to function fine without that except the 14nm driver for the sdm6xx [1]. At the same time all DTs provide a "ref" clock as per the requirements of dsi-phy-common.yaml, but the clock is never used. This patchset puts that clock to use without relying on a global clock name, so that all dependencies are explicitly defined in DT (the firmware) in the end. Note that msm8974 is the only board not providing this clock, and apq8064 was providing the wrong clock (19.2MHz cxo instead of 27MHz pxo). Both have been been addressed in separate patches that are supposed to land well in advance of this patchset. Furthermore not all board-DTs provided this clock initially but that deficiency has been addressed in followup patches (see the Fixes: below). Those commits seem to assume that the clock was used, while nothing in history indicates that this "ref" clock was ever retrieved. [1]: https://lore.kernel.org/linux-arm-msm/386db1a6-a1cd-3c7d-a88e-dc83f8a1b...@somainline.org/ Fixes: 79e51645a1dd ("arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY") Fixes: 6969d1d9c615 ("ARM: dts: qcom-apq8064: Set 'cxo_board' as ref clock of the DSI PHY") Fixes: 0c0e72705a33 ("arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs") Signed-off-by: Marijn Suijten --- drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 4 +++- drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 4 +++- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c index e46b10fc793a..3cbb1f1475e8 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c @@ -562,7 +562,9 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov char clk_name[32], parent[32], vco_name[32]; char parent2[32], parent3[32], parent4[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "xo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c index bb31230721bd..406470265408 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c @@ -804,7 +804,9 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov { char clk_name[32], parent[32], vco_name[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "xo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c index 2da673a2add6..8ee9c9c0548d 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c @@ -521,7 +521,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov { char clk_name[32], parent1[32], parent2[32], vco_name[32]; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "xo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .name = vco_name, .flags = CLK_IGNORE_UNUSED, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c index aaa37456f4ee..9662cb236468 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c @@ -385,7 +385,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov { char *clk_name, *parent_name, *vco_name; struct clk_init_data vco_init = { - .parent_names = (const char *[]){ "pxo" }, + .parent_data = &(const struct clk_parent_data) { + .fw_name = "ref", + }, .num_parents = 1, .flags = CLK_IGNORE_UNUSED, .ops = &clk_ops_dsi_pll_28nm_vco, diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c index 7c23d4c47338..c77c30628cca 100644 --- a/drivers/gpu/drm/msm/
[PATCH v2 2/2] clk: qcom: gcc-sdm660: Remove transient global "xo" clock
The DSI PHY/PLL was relying on a global "xo" clock to be found, but the real clock is named "xo_board" in the DT. The standard nowadays is to never use global clock names anymore but require the firmware (DT) to provide every clock binding explicitly with .fw_name. The DSI PLLs have since been converted to this mechanism (specifically 14nm for SDM660) and this transient clock can now be removed. This issue was originally discovered in: https://lore.kernel.org/linux-arm-msm/386db1a6-a1cd-3c7d-a88e-dc83f8a1b...@somainline.org/ and prevented the removal of "xo" at that time. Signed-off-by: Marijn Suijten --- drivers/clk/qcom/gcc-sdm660.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/drivers/clk/qcom/gcc-sdm660.c b/drivers/clk/qcom/gcc-sdm660.c index 9b97425008ce..16fd16351f95 100644 --- a/drivers/clk/qcom/gcc-sdm660.c +++ b/drivers/clk/qcom/gcc-sdm660.c @@ -37,19 +37,6 @@ enum { P_GPLL1_EARLY_DIV, }; -static struct clk_fixed_factor xo = { - .mult = 1, - .div = 1, - .hw.init = &(struct clk_init_data){ - .name = "xo", - .parent_data = &(const struct clk_parent_data) { - .fw_name = "xo" - }, - .num_parents = 1, - .ops = &clk_fixed_factor_ops, - }, -}; - static struct clk_alpha_pll gpll0_early = { .offset = 0x0, .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT], @@ -2281,7 +2268,6 @@ static struct gdsc pcie_0_gdsc = { }; static struct clk_hw *gcc_sdm660_hws[] = { - &xo.hw, &gpll0_early_div.hw, &gpll1_early_div.hw, }; -- 2.33.0
Re: [PATCH v5 20/20] dma-resv: Give the docs a do-over
On Thu, Aug 05, 2021 at 12:47:05PM +0200, Daniel Vetter wrote: > Specifically document the new/clarified rules around how the shared > fences do not have any ordering requirements against the exclusive > fence. > > But also document all the things a bit better, given how central > struct dma_resv to dynamic buffer management the docs have been very > inadequat. > > - Lots more links to other pieces of the puzzle. Unfortunately > ttm_buffer_object has no docs, so no links :-( > > - Explain/complain a bit about dma_resv_locking_ctx(). I still don't > like that one, but fixing the ttm call chains is going to be > horrible. Plus we want to plug in real slowpath locking when we do > that anyway. > > - Main part of the patch is some actual docs for struct dma_resv. > > Overall I think we still have a lot of bad naming in this area (e.g. > dma_resv.fence is singular, but contains the multiple shared fences), > but I think that's more indicative of how the semantics and rules are > just not great. > > Another thing that's real awkard is how chaining exclusive fences > right now means direct dma_resv.exclusive_fence pointer access with an > rcu_assign_pointer. Not so great either. > > v2: > - Fix a pile of typos (Matt, Jason) > - Hammer it in that breaking the rules leads to use-after-free issues > around dma-buf sharing (Christian) > > Reviewed-by: Christian König > Cc: Jason Ekstrand > Cc: Matthew Auld > Reviewed-by: Matthew Auld > Signed-off-by: Daniel Vetter > Cc: Sumit Semwal > Cc: "Christian König" > Cc: linux-me...@vger.kernel.org > Cc: linaro-mm-...@lists.linaro.org Also pushed to drm-misc-next. -Daniel > --- > drivers/dma-buf/dma-resv.c | 24 ++--- > include/linux/dma-buf.h| 7 +++ > include/linux/dma-resv.h | 104 +++-- > 3 files changed, 124 insertions(+), 11 deletions(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index e744fd87c63c..84fbe60629e3 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -48,6 +48,8 @@ > * write operations) or N shared fences (read operations). The RCU > * mechanism is used to protect read access to fences from locked > * write-side updates. > + * > + * See struct dma_resv for more details. > */ > > DEFINE_WD_CLASS(reservation_ww_class); > @@ -137,7 +139,11 @@ EXPORT_SYMBOL(dma_resv_fini); > * @num_fences: number of fences we want to add > * > * Should be called before dma_resv_add_shared_fence(). Must > - * be called with obj->lock held. > + * be called with @obj locked through dma_resv_lock(). > + * > + * Note that the preallocated slots need to be re-reserved if @obj is > unlocked > + * at any time before calling dma_resv_add_shared_fence(). This is validated > + * when CONFIG_DEBUG_MUTEXES is enabled. > * > * RETURNS > * Zero for success, or -errno > @@ -234,8 +240,10 @@ EXPORT_SYMBOL(dma_resv_reset_shared_max); > * @obj: the reservation object > * @fence: the shared fence to add > * > - * Add a fence to a shared slot, obj->lock must be held, and > + * Add a fence to a shared slot, @obj must be locked with dma_resv_lock(), > and > * dma_resv_reserve_shared() has been called. > + * > + * See also &dma_resv.fence for a discussion of the semantics. > */ > void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) > { > @@ -278,9 +286,11 @@ EXPORT_SYMBOL(dma_resv_add_shared_fence); > /** > * dma_resv_add_excl_fence - Add an exclusive fence. > * @obj: the reservation object > - * @fence: the shared fence to add > + * @fence: the exclusive fence to add > * > - * Add a fence to the exclusive slot. The obj->lock must be held. > + * Add a fence to the exclusive slot. @obj must be locked with > dma_resv_lock(). > + * Note that this function replaces all fences attached to @obj, see also > + * &dma_resv.fence_excl for a discussion of the semantics. > */ > void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) > { > @@ -609,9 +619,11 @@ static inline int dma_resv_test_signaled_single(struct > dma_fence *passed_fence) > * fence > * > * Callers are not required to hold specific locks, but maybe hold > - * dma_resv_lock() already > + * dma_resv_lock() already. > + * > * RETURNS > - * true if all fences signaled, else false > + * > + * True if all fences signaled, else false. > */ > bool dma_resv_test_signaled(struct dma_resv *obj, bool test_all) > { > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index 678b2006be78..fc62b5f9980c 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -420,6 +420,13 @@ struct dma_buf { >* - Dynamic importers should set fences for any access that they can't >* disable immediately from their &dma_buf_attach_ops.move_notify >* callback. > + * > + * IMPORTANT: > + * > + * All drivers must obey the struct dma_resv rules, specifically the >
[PATCH 1/8] drm/i915/gt: Lock intel_engine_apply_whitelist with uncore->lock
Refactor intel_engine_apply_whitelist into locked and unlocked versions so that a caller who already has the lock can apply whitelist. v2: Fix sparse warning v3: (Chris) - Drop prefix and suffix for static function - Use longest to shortest line ordering for variable declaration Signed-off-by: Umesh Nerlige Ramappa Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 46 ++--- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 94e1937f8d29..2a8cc0e2d1b1 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1248,7 +1248,8 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915) } static enum forcewake_domains -wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal) +wal_get_fw(struct intel_uncore *uncore, const struct i915_wa_list *wal, + unsigned int op) { enum forcewake_domains fw = 0; struct i915_wa *wa; @@ -1257,8 +1258,7 @@ wal_get_fw_for_rmw(struct intel_uncore *uncore, const struct i915_wa_list *wal) for (i = 0, wa = wal->list; i < wal->count; i++, wa++) fw |= intel_uncore_forcewake_for_reg(uncore, wa->reg, -FW_REG_READ | -FW_REG_WRITE); +op); return fw; } @@ -1289,7 +1289,7 @@ wa_list_apply(struct intel_gt *gt, const struct i915_wa_list *wal) if (!wal->count) return; - fw = wal_get_fw_for_rmw(uncore, wal); + fw = wal_get_fw(uncore, wal, FW_REG_READ | FW_REG_WRITE); spin_lock_irqsave(&uncore->lock, flags); intel_uncore_forcewake_get__locked(uncore, fw); @@ -1328,7 +1328,7 @@ static bool wa_list_verify(struct intel_gt *gt, unsigned int i; bool ok = true; - fw = wal_get_fw_for_rmw(uncore, wal); + fw = wal_get_fw(uncore, wal, FW_REG_READ | FW_REG_WRITE); spin_lock_irqsave(&uncore->lock, flags); intel_uncore_forcewake_get__locked(uncore, fw); @@ -1617,27 +1617,45 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine) wa_init_finish(w); } -void intel_engine_apply_whitelist(struct intel_engine_cs *engine) +static void __engine_apply_whitelist(struct intel_engine_cs *engine) { const struct i915_wa_list *wal = &engine->whitelist; struct intel_uncore *uncore = engine->uncore; const u32 base = engine->mmio_base; + enum forcewake_domains fw; struct i915_wa *wa; unsigned int i; - if (!wal->count) - return; + lockdep_assert_held(&uncore->lock); + + fw = wal_get_fw(uncore, wal, FW_REG_WRITE); + intel_uncore_forcewake_get__locked(uncore, fw); for (i = 0, wa = wal->list; i < wal->count; i++, wa++) - intel_uncore_write(uncore, - RING_FORCE_TO_NONPRIV(base, i), - i915_mmio_reg_offset(wa->reg)); + intel_uncore_write_fw(uncore, + RING_FORCE_TO_NONPRIV(base, i), + i915_mmio_reg_offset(wa->reg)); /* And clear the rest just in case of garbage */ for (; i < RING_MAX_NONPRIV_SLOTS; i++) - intel_uncore_write(uncore, - RING_FORCE_TO_NONPRIV(base, i), - i915_mmio_reg_offset(RING_NOPID(base))); + intel_uncore_write_fw(uncore, + RING_FORCE_TO_NONPRIV(base, i), + i915_mmio_reg_offset(RING_NOPID(base))); + + intel_uncore_forcewake_put__locked(uncore, fw); +} + +void intel_engine_apply_whitelist(struct intel_engine_cs *engine) +{ + const struct i915_wa_list *wal = &engine->whitelist; + unsigned long flags; + + if (!wal->count) + return; + + spin_lock_irqsave(&engine->uncore->lock, flags); + __engine_apply_whitelist(engine); + spin_unlock_irqrestore(&engine->uncore->lock, flags); } static void -- 2.20.1