From 516198d317d81f33839ca850e83f6717b0d80e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Thu, 1 Sep 2022 13:49:30 +0100 Subject: [PATCH 01/19] drm/i915: audit bo->resource usage v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure we can at least move and alloc TT objects without backing store. v2: clear the tt object even when no resource is allocated. v3: add Matthews changes for i915 as well. Signed-off-by: Christian König Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230124125726.13323-1-christian.koenig@amd.com --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 27 ++++++++-- drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 56 +++++++++++++++++--- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index d409a77449a3..6649d18ed1c8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -271,8 +271,6 @@ static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo, { struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915), bdev); - struct ttm_resource_manager *man = - ttm_manager_type(bo->bdev, bo->resource->mem_type); struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo); unsigned long ccs_pages = 0; enum ttm_caching caching; @@ -286,8 +284,8 @@ static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo, if (!i915_tt) return NULL; - if (obj->flags & I915_BO_ALLOC_CPU_CLEAR && - man->use_tt) + if (obj->flags & I915_BO_ALLOC_CPU_CLEAR && (!bo->resource || + ttm_manager_type(bo->bdev, bo->resource->mem_type)->use_tt)) page_flags |= TTM_TT_FLAG_ZERO_ALLOC; caching = i915_ttm_select_tt_caching(obj); @@ -1051,7 +1049,26 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) return VM_FAULT_SIGBUS; } - if (!i915_ttm_resource_mappable(bo->resource)) { + /* + * This must be swapped out with shmem ttm_tt (pipeline-gutting). + * Calling ttm_bo_validate() here with TTM_PL_SYSTEM should only go as + * far as far doing a ttm_bo_move_null(), which should skip all the + * other junk. + */ + if (!bo->resource) { + struct ttm_operation_ctx ctx = { + .interruptible = true, + .no_wait_gpu = true, /* should be idle already */ + }; + + GEM_BUG_ON(!bo->ttm || !(bo->ttm->page_flags & TTM_TT_FLAG_SWAPPED)); + + ret = ttm_bo_validate(bo, i915_ttm_sys_placement(), &ctx); + if (ret) { + dma_resv_unlock(bo->base.resv); + return VM_FAULT_SIGBUS; + } + } else if (!i915_ttm_resource_mappable(bo->resource)) { int err = -ENODEV; int i; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c index 2ebaaf4d663c..76dd9e5e1a8b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c @@ -103,7 +103,27 @@ void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj) { struct ttm_buffer_object *bo = i915_gem_to_ttm(obj); unsigned int cache_level; + unsigned int mem_flags; unsigned int i; + int mem_type; + + /* + * We might have been purged (or swapped out) if the resource is NULL, + * in which case the SYSTEM placement is the closest match to describe + * the current domain. If the object is ever used in this state then we + * will require moving it again. + */ + if (!bo->resource) { + mem_flags = I915_BO_FLAG_STRUCT_PAGE; + mem_type = I915_PL_SYSTEM; + cache_level = I915_CACHE_NONE; + } else { + mem_flags = i915_ttm_cpu_maps_iomem(bo->resource) ? I915_BO_FLAG_IOMEM : + I915_BO_FLAG_STRUCT_PAGE; + mem_type = bo->resource->mem_type; + cache_level = i915_ttm_cache_level(to_i915(bo->base.dev), bo->resource, + bo->ttm); + } /* * If object was moved to an allowable region, update the object @@ -111,11 +131,11 @@ void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj) * in an allowable region, it's evicted and we don't update the * object region. */ - if (intel_region_to_ttm_type(obj->mm.region) != bo->resource->mem_type) { + if (intel_region_to_ttm_type(obj->mm.region) != mem_type) { for (i = 0; i < obj->mm.n_placements; ++i) { struct intel_memory_region *mr = obj->mm.placements[i]; - if (intel_region_to_ttm_type(mr) == bo->resource->mem_type && + if (intel_region_to_ttm_type(mr) == mem_type && mr != obj->mm.region) { i915_gem_object_release_memory_region(obj); i915_gem_object_init_memory_region(obj, mr); @@ -125,12 +145,8 @@ void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj) } obj->mem_flags &= ~(I915_BO_FLAG_STRUCT_PAGE | I915_BO_FLAG_IOMEM); + obj->mem_flags |= mem_flags; - obj->mem_flags |= i915_ttm_cpu_maps_iomem(bo->resource) ? I915_BO_FLAG_IOMEM : - I915_BO_FLAG_STRUCT_PAGE; - - cache_level = i915_ttm_cache_level(to_i915(bo->base.dev), bo->resource, - bo->ttm); i915_gem_object_set_cache_coherency(obj, cache_level); } @@ -565,6 +581,32 @@ int i915_ttm_move(struct ttm_buffer_object *bo, bool evict, return 0; } + if (!bo->resource) { + if (dst_mem->mem_type != TTM_PL_SYSTEM) { + hop->mem_type = TTM_PL_SYSTEM; + hop->flags = TTM_PL_FLAG_TEMPORARY; + return -EMULTIHOP; + } + + /* + * This is only reached when first creating the object, or if + * the object was purged or swapped out (pipeline-gutting). For + * the former we can safely skip all of the below since we are + * only using a dummy SYSTEM placement here. And with the latter + * we will always re-enter here with bo->resource set correctly + * (as per the above), since this is part of a multi-hop + * sequence, where at the end we can do the move for real. + * + * The special case here is when the dst_mem is TTM_PL_SYSTEM, + * which doens't require any kind of move, so it should be safe + * to skip all the below and call ttm_bo_move_null() here, where + * the caller in __i915_ttm_get_pages() will take care of the + * rest, since we should have a valid ttm_tt. + */ + ttm_bo_move_null(bo, dst_mem); + return 0; + } + ret = i915_ttm_move_notify(bo); if (ret) return ret; From 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Thu, 17 Feb 2022 16:29:43 +0100 Subject: [PATCH 02/19] drm/ttm: stop allocating dummy resources during BO creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That should not be necessary any more when drivers should at least be able to handle the move without a resource. Signed-off-by: Christian König Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230124125726.13323-2-christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 326a3d13a829..bb0c21c8caac 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -953,7 +953,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { - static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; int ret; kref_init(&bo->kref); @@ -970,12 +969,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->base.resv = &bo->base._resv; atomic_inc(&ttm_glob.bo_count); - ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); - if (unlikely(ret)) { - ttm_bo_put(bo); - return ret; - } - /* * For ttm_bo_type_device buffers, allocate * address space from the device. From 4110872b8115aab2adb3a52149c144d8465440de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Thu, 17 Feb 2022 16:51:44 +0100 Subject: [PATCH 03/19] drm/ttm: stop allocating a dummy resource for pipelined gutting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That should not be necessary any more when drivers should at least be able to handle a move without a resource. Signed-off-by: Christian König Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230124125726.13323-3-christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo_util.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index fca8b0e0e30a..17330fb4480a 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -704,30 +704,23 @@ EXPORT_SYMBOL(ttm_bo_move_sync_cleanup); */ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) { - static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; struct ttm_buffer_object *ghost; - struct ttm_resource *sys_res; struct ttm_tt *ttm; int ret; - ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); - if (ret) - return ret; - /* If already idle, no need for ghost object dance. */ if (dma_resv_test_signaled(bo->base.resv, DMA_RESV_USAGE_BOOKKEEP)) { if (!bo->ttm) { /* See comment below about clearing. */ ret = ttm_tt_create(bo, true); if (ret) - goto error_free_sys_mem; + return ret; } else { ttm_tt_unpopulate(bo->bdev, bo->ttm); if (bo->type == ttm_bo_type_device) ttm_tt_mark_for_clear(bo->ttm); } ttm_resource_free(bo, &bo->resource); - ttm_bo_assign_mem(bo, sys_res); return 0; } @@ -744,7 +737,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) ret = ttm_tt_create(bo, true); swap(bo->ttm, ttm); if (ret) - goto error_free_sys_mem; + return ret; ret = ttm_buffer_object_transfer(bo, &ghost); if (ret) @@ -760,13 +753,9 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); bo->ttm = ttm; - ttm_bo_assign_mem(bo, sys_res); return 0; error_destroy_tt: ttm_tt_destroy(bo->bdev, ttm); - -error_free_sys_mem: - ttm_resource_free(bo, &sys_res); return ret; } From b49323aa35d502b0d9a7950327f30a1a52eae534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Tue, 13 Dec 2022 10:07:36 +0100 Subject: [PATCH 04/19] drm/ttm: prevent moving of pinned BOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have checks for this in the individual drivers move callback, but it's probably better to generally forbid that on a higher level. Also stops exporting ttm_resource_compat() since that's not necessary any more after removing the extra checks in vmwgfx. Signed-off-by: Christian König Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230124125726.13323-4-christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ---- drivers/gpu/drm/nouveau/nouveau_bo.c | 3 --- drivers/gpu/drm/radeon/radeon_ttm.c | 4 ---- drivers/gpu/drm/ttm/ttm_bo.c | 20 ++++++++++++-------- drivers/gpu/drm/ttm/ttm_resource.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 19 ++----------------- 6 files changed, 14 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index c5ef7f7bdc15..2cd081cbf706 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -466,11 +466,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, return r; } - /* Can't move a pinned BO */ abo = ttm_to_amdgpu_bo(bo); - if (WARN_ON_ONCE(abo->tbo.pin_count > 0)) - return -EINVAL; - adev = amdgpu_ttm_adev(bo->bdev); if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 288eebc70a67..c2ec91cc845d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1015,9 +1015,6 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, if (ret) goto out_ntfy; - if (nvbo->bo.pin_count) - NV_WARN(drm, "Moving pinned object %p!\n", nvbo); - if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) { ret = nouveau_bo_vm_bind(bo, new_reg, &new_tile); if (ret) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 1e8e287e113c..67075c85f847 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -211,11 +211,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, if (r) return r; - /* Can't move a pinned BO */ rbo = container_of(bo, struct radeon_bo, tbo); - if (WARN_ON_ONCE(rbo->tbo.pin_count > 0)) - return -EINVAL; - rdev = radeon_get_rdev(bo->bdev); if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { ttm_bo_move_null(bo, new_mem); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bb0c21c8caac..33471e363ff4 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -894,14 +894,18 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, if (!placement->num_placement && !placement->num_busy_placement) return ttm_bo_pipeline_gutting(bo); - /* - * Check whether we need to move buffer. - */ - if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { - ret = ttm_bo_move_buffer(bo, placement, ctx); - if (ret) - return ret; - } + /* Check whether we need to move buffer. */ + if (bo->resource && ttm_resource_compat(bo->resource, placement)) + return 0; + + /* Moving of pinned BOs is forbidden */ + if (bo->pin_count) + return -EINVAL; + + ret = ttm_bo_move_buffer(bo, placement, ctx); + if (ret) + return ret; + /* * We might need to add a TTM. */ diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index b8a826a24fb2..7333f7a87a2f 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -361,7 +361,6 @@ bool ttm_resource_compat(struct ttm_resource *res, return false; } -EXPORT_SYMBOL(ttm_resource_compat); void ttm_resource_set_bo(struct ttm_resource *res, struct ttm_buffer_object *bo) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 321c551784a1..dbcef460c452 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -87,12 +87,7 @@ int vmw_bo_pin_in_placement(struct vmw_private *dev_priv, if (unlikely(ret != 0)) goto err; - if (buf->base.pin_count > 0) - ret = ttm_resource_compat(bo->resource, placement) - ? 0 : -EINVAL; - else - ret = ttm_bo_validate(bo, placement, &ctx); - + ret = ttm_bo_validate(bo, placement, &ctx); if (!ret) vmw_bo_pin_reserved(buf, true); @@ -128,12 +123,6 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv, if (unlikely(ret != 0)) goto err; - if (buf->base.pin_count > 0) { - ret = ttm_resource_compat(bo->resource, &vmw_vram_gmr_placement) - ? 0 : -EINVAL; - goto out_unreserve; - } - ret = ttm_bo_validate(bo, &vmw_vram_gmr_placement, &ctx); if (likely(ret == 0) || ret == -ERESTARTSYS) goto out_unreserve; @@ -218,11 +207,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private *dev_priv, (void) ttm_bo_validate(bo, &vmw_sys_placement, &ctx); } - if (buf->base.pin_count > 0) - ret = ttm_resource_compat(bo->resource, &placement) - ? 0 : -EINVAL; - else - ret = ttm_bo_validate(bo, &placement, &ctx); + ret = ttm_bo_validate(bo, &placement, &ctx); /* For some reason we didn't end up at the start of vram */ WARN_ON(ret == 0 && bo->resource->start != 0); From fc64adc56b8302ed21189607dd4b4f4719f2c78f Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 20 Jan 2023 09:28:42 +0000 Subject: [PATCH 05/19] accel/ivpu: Fix spelling mistake "tansition" -> "transition" There are spelling mistakes in two ivpu_err error messages. Fix them. Signed-off-by: Colin Ian King Reviewed-by: Stanislaw Gruszka Signed-off-by: Jacek Lawrynowicz Link: https://patchwork.freedesktop.org/patch/msgid/20230120092842.79238-1-colin.i.king@gmail.com --- drivers/accel/ivpu/ivpu_hw_mtl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c index b59b1f472b40..62bfaa9081c4 100644 --- a/drivers/accel/ivpu/ivpu_hw_mtl.c +++ b/drivers/accel/ivpu/ivpu_hw_mtl.c @@ -608,7 +608,7 @@ static int ivpu_boot_d0i3_drive(struct ivpu_device *vdev, bool enable) ret = REGB_POLL_FLD(MTL_BUTTRESS_VPU_D0I3_CONTROL, INPROGRESS, 0, TIMEOUT_US); if (ret) { - ivpu_err(vdev, "Failed to sync before D0i3 tansition: %d\n", ret); + ivpu_err(vdev, "Failed to sync before D0i3 transition: %d\n", ret); return ret; } @@ -621,7 +621,7 @@ static int ivpu_boot_d0i3_drive(struct ivpu_device *vdev, bool enable) ret = REGB_POLL_FLD(MTL_BUTTRESS_VPU_D0I3_CONTROL, INPROGRESS, 0, TIMEOUT_US); if (ret) - ivpu_err(vdev, "Failed to sync after D0i3 tansition: %d\n", ret); + ivpu_err(vdev, "Failed to sync after D0i3 transition: %d\n", ret); return ret; } From 2847a67d3aa518a29e8a8db01bbec630d3bd5011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Wed, 25 Jan 2023 17:14:37 +0100 Subject: [PATCH 06/19] drm/ttm: revert "prevent moving of pinned BOs" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b49323aa35d502b0d9a7950327f30a1a52eae534. This still seems to break i915. Signed-off-by: Christian König Acked-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230125155023.105584-1-christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++++ drivers/gpu/drm/nouveau/nouveau_bo.c | 3 +++ drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++++ drivers/gpu/drm/ttm/ttm_bo.c | 20 ++++++++------------ drivers/gpu/drm/ttm/ttm_resource.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 19 +++++++++++++++++-- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 2cd081cbf706..c5ef7f7bdc15 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -466,7 +466,11 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, return r; } + /* Can't move a pinned BO */ abo = ttm_to_amdgpu_bo(bo); + if (WARN_ON_ONCE(abo->tbo.pin_count > 0)) + return -EINVAL; + adev = amdgpu_ttm_adev(bo->bdev); if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index c2ec91cc845d..288eebc70a67 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1015,6 +1015,9 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, if (ret) goto out_ntfy; + if (nvbo->bo.pin_count) + NV_WARN(drm, "Moving pinned object %p!\n", nvbo); + if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) { ret = nouveau_bo_vm_bind(bo, new_reg, &new_tile); if (ret) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 67075c85f847..1e8e287e113c 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -211,7 +211,11 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, if (r) return r; + /* Can't move a pinned BO */ rbo = container_of(bo, struct radeon_bo, tbo); + if (WARN_ON_ONCE(rbo->tbo.pin_count > 0)) + return -EINVAL; + rdev = radeon_get_rdev(bo->bdev); if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { ttm_bo_move_null(bo, new_mem); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 33471e363ff4..bb0c21c8caac 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -894,18 +894,14 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, if (!placement->num_placement && !placement->num_busy_placement) return ttm_bo_pipeline_gutting(bo); - /* Check whether we need to move buffer. */ - if (bo->resource && ttm_resource_compat(bo->resource, placement)) - return 0; - - /* Moving of pinned BOs is forbidden */ - if (bo->pin_count) - return -EINVAL; - - ret = ttm_bo_move_buffer(bo, placement, ctx); - if (ret) - return ret; - + /* + * Check whether we need to move buffer. + */ + if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { + ret = ttm_bo_move_buffer(bo, placement, ctx); + if (ret) + return ret; + } /* * We might need to add a TTM. */ diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 7333f7a87a2f..b8a826a24fb2 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -361,6 +361,7 @@ bool ttm_resource_compat(struct ttm_resource *res, return false; } +EXPORT_SYMBOL(ttm_resource_compat); void ttm_resource_set_bo(struct ttm_resource *res, struct ttm_buffer_object *bo) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index dbcef460c452..321c551784a1 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -87,7 +87,12 @@ int vmw_bo_pin_in_placement(struct vmw_private *dev_priv, if (unlikely(ret != 0)) goto err; - ret = ttm_bo_validate(bo, placement, &ctx); + if (buf->base.pin_count > 0) + ret = ttm_resource_compat(bo->resource, placement) + ? 0 : -EINVAL; + else + ret = ttm_bo_validate(bo, placement, &ctx); + if (!ret) vmw_bo_pin_reserved(buf, true); @@ -123,6 +128,12 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv, if (unlikely(ret != 0)) goto err; + if (buf->base.pin_count > 0) { + ret = ttm_resource_compat(bo->resource, &vmw_vram_gmr_placement) + ? 0 : -EINVAL; + goto out_unreserve; + } + ret = ttm_bo_validate(bo, &vmw_vram_gmr_placement, &ctx); if (likely(ret == 0) || ret == -ERESTARTSYS) goto out_unreserve; @@ -207,7 +218,11 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private *dev_priv, (void) ttm_bo_validate(bo, &vmw_sys_placement, &ctx); } - ret = ttm_bo_validate(bo, &placement, &ctx); + if (buf->base.pin_count > 0) + ret = ttm_resource_compat(bo->resource, &placement) + ? 0 : -EINVAL; + else + ret = ttm_bo_validate(bo, &placement, &ctx); /* For some reason we didn't end up at the start of vram */ WARN_ON(ret == 0 && bo->resource->start != 0); From fc1137070b9c59f8f1772e632215da0ad5725661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Wed, 25 Jan 2023 16:48:36 +0100 Subject: [PATCH 07/19] drm/ttm: revert "stop allocating a dummy resource for pipelined gutting" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 4110872b8115aab2adb3a52149c144d8465440de. This still seems to break i915. Signed-off-by: Christian König Acked-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230125155023.105584-1-christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo_util.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 17330fb4480a..fca8b0e0e30a 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -704,23 +704,30 @@ EXPORT_SYMBOL(ttm_bo_move_sync_cleanup); */ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) { + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; struct ttm_buffer_object *ghost; + struct ttm_resource *sys_res; struct ttm_tt *ttm; int ret; + ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); + if (ret) + return ret; + /* If already idle, no need for ghost object dance. */ if (dma_resv_test_signaled(bo->base.resv, DMA_RESV_USAGE_BOOKKEEP)) { if (!bo->ttm) { /* See comment below about clearing. */ ret = ttm_tt_create(bo, true); if (ret) - return ret; + goto error_free_sys_mem; } else { ttm_tt_unpopulate(bo->bdev, bo->ttm); if (bo->type == ttm_bo_type_device) ttm_tt_mark_for_clear(bo->ttm); } ttm_resource_free(bo, &bo->resource); + ttm_bo_assign_mem(bo, sys_res); return 0; } @@ -737,7 +744,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) ret = ttm_tt_create(bo, true); swap(bo->ttm, ttm); if (ret) - return ret; + goto error_free_sys_mem; ret = ttm_buffer_object_transfer(bo, &ghost); if (ret) @@ -753,9 +760,13 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); bo->ttm = ttm; + ttm_bo_assign_mem(bo, sys_res); return 0; error_destroy_tt: ttm_tt_destroy(bo->bdev, ttm); + +error_free_sys_mem: + ttm_resource_free(bo, &sys_res); return ret; } From 0c8fb2469438256a9cbb690d538437db6f845df8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Wed, 25 Jan 2023 16:49:02 +0100 Subject: [PATCH 08/19] drm/ttm: revert "stop allocating dummy resources during BO creation" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 00984ad39599bb2a1e6ec5d4e9c75a749f7f45c9. It seems to still breka i915. Signed-off-by: Christian König Acked-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20230125155023.105584-1-christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bb0c21c8caac..326a3d13a829 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -953,6 +953,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { + static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; int ret; kref_init(&bo->kref); @@ -969,6 +970,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->base.resv = &bo->base._resv; atomic_inc(&ttm_glob.bo_count); + ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); + if (unlikely(ret)) { + ttm_bo_put(bo); + return ret; + } + /* * For ttm_bo_type_device buffers, allocate * address space from the device. From da2b1a0a40d8ea8f23f9d13e10ec3160ef92178d Mon Sep 17 00:00:00 2001 From: Deepak R Varma Date: Wed, 25 Jan 2023 20:37:14 +0530 Subject: [PATCH 09/19] drm/nouveau/devinit: Convert function disable() to be void The current design of callback function disable() of struct nvkm_devinit_func is defined to return a u64 value. In its implementation in the driver modules, the function always returns a fixed value 0. Hence the design and implementation of this function should be enhanced to return void instead of a fixed value. This change also eliminates untouched return variables. The change is identified using the returnvar.cocci Coccinelle semantic patch script. Signed-off-by: Deepak R Varma Reviewed-by: Lyude Paul Signed-off-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/Y9FFoooIXjlr+UP1@ubun2204.myguest.virtualbox.org --- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/base.c | 3 ++- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g84.c | 5 +---- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g98.c | 4 +--- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gf100.c | 4 +--- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gm107.c | 4 +--- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gt215.c | 4 +--- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/mcp89.c | 4 +--- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.c | 5 +---- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.h | 2 +- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/priv.h | 2 +- 10 files changed, 11 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/base.c index dd4981708fe4..3d9319c319c6 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/base.c @@ -51,7 +51,8 @@ u64 nvkm_devinit_disable(struct nvkm_devinit *init) { if (init && init->func->disable) - return init->func->disable(init); + init->func->disable(init); + return 0; } diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g84.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g84.c index c224702b7bed..00df7811dd10 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g84.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g84.c @@ -26,13 +26,12 @@ #include #include -static u64 +static void g84_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; u32 r001540 = nvkm_rd32(device, 0x001540); u32 r00154c = nvkm_rd32(device, 0x00154c); - u64 disable = 0ULL; if (!(r001540 & 0x40000000)) { nvkm_subdev_disable(device, NVKM_ENGINE_MPEG, 0); @@ -47,8 +46,6 @@ g84_devinit_disable(struct nvkm_devinit *init) nvkm_subdev_disable(device, NVKM_ENGINE_BSP, 0); if (!(r00154c & 0x00000040)) nvkm_subdev_disable(device, NVKM_ENGINE_CIPHER, 0); - - return disable; } static const struct nvkm_devinit_func diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g98.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g98.c index 8977483a9f42..54bee499b982 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g98.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/g98.c @@ -26,7 +26,7 @@ #include #include -static u64 +static void g98_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; @@ -45,8 +45,6 @@ g98_devinit_disable(struct nvkm_devinit *init) nvkm_subdev_disable(device, NVKM_ENGINE_MSVLD, 0); if (!(r00154c & 0x00000040)) nvkm_subdev_disable(device, NVKM_ENGINE_SEC, 0); - - return 0ULL; } static const struct nvkm_devinit_func diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gf100.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gf100.c index 5b7cb1fe7897..5368e705e7fd 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gf100.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gf100.c @@ -63,7 +63,7 @@ gf100_devinit_pll_set(struct nvkm_devinit *init, u32 type, u32 freq) return ret; } -static u64 +static void gf100_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; @@ -85,8 +85,6 @@ gf100_devinit_disable(struct nvkm_devinit *init) nvkm_subdev_disable(device, NVKM_ENGINE_CE, 0); if (r022500 & 0x00000200) nvkm_subdev_disable(device, NVKM_ENGINE_CE, 1); - - return 0ULL; } void diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gm107.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gm107.c index 8955af2704c7..7bcbc4895ec2 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gm107.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gm107.c @@ -26,7 +26,7 @@ #include #include -u64 +void gm107_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; @@ -39,8 +39,6 @@ gm107_devinit_disable(struct nvkm_devinit *init) nvkm_subdev_disable(device, NVKM_ENGINE_CE, 2); if (r021c04 & 0x00000001) nvkm_subdev_disable(device, NVKM_ENGINE_DISP, 0); - - return 0ULL; } static const struct nvkm_devinit_func diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gt215.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gt215.c index 3d0ab86c3115..dbca92318baf 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gt215.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/gt215.c @@ -62,7 +62,7 @@ gt215_devinit_pll_set(struct nvkm_devinit *init, u32 type, u32 freq) return ret; } -static u64 +static void gt215_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; @@ -80,8 +80,6 @@ gt215_devinit_disable(struct nvkm_devinit *init) nvkm_subdev_disable(device, NVKM_ENGINE_MSVLD, 0); if (!(r00154c & 0x00000200)) nvkm_subdev_disable(device, NVKM_ENGINE_CE, 0); - - return 0ULL; } static u32 diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/mcp89.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/mcp89.c index a9cdf2411187..a24bd2e7d7ce 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/mcp89.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/mcp89.c @@ -26,7 +26,7 @@ #include #include -static u64 +static void mcp89_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; @@ -46,8 +46,6 @@ mcp89_devinit_disable(struct nvkm_devinit *init) nvkm_subdev_disable(device, NVKM_ENGINE_VIC, 0); if (!(r00154c & 0x00000200)) nvkm_subdev_disable(device, NVKM_ENGINE_CE, 0); - - return 0; } static const struct nvkm_devinit_func diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.c b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.c index 380995d398b1..07ed8fd778b2 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.c @@ -77,17 +77,14 @@ nv50_devinit_pll_set(struct nvkm_devinit *init, u32 type, u32 freq) return 0; } -static u64 +static void nv50_devinit_disable(struct nvkm_devinit *init) { struct nvkm_device *device = init->subdev.device; u32 r001540 = nvkm_rd32(device, 0x001540); - u64 disable = 0ULL; if (!(r001540 & 0x40000000)) nvkm_subdev_disable(device, NVKM_ENGINE_MPEG, 0); - - return disable; } void diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.h b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.h index 987a7f478b84..8de409c084c1 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.h +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/nv50.h @@ -23,7 +23,7 @@ int gf100_devinit_ctor(struct nvkm_object *, struct nvkm_object *, int gf100_devinit_pll_set(struct nvkm_devinit *, u32, u32); void gf100_devinit_preinit(struct nvkm_devinit *); -u64 gm107_devinit_disable(struct nvkm_devinit *); +void gm107_devinit_disable(struct nvkm_devinit *); int gm200_devinit_post(struct nvkm_devinit *, bool); void gm200_devinit_preos(struct nv50_devinit *, bool); diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/priv.h index dd8b038a8cee..a648482d06e9 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/priv.h +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/priv.h @@ -12,7 +12,7 @@ struct nvkm_devinit_func { u32 (*mmio)(struct nvkm_devinit *, u32); void (*meminit)(struct nvkm_devinit *); int (*pll_set)(struct nvkm_devinit *, u32 type, u32 freq); - u64 (*disable)(struct nvkm_devinit *); + void (*disable)(struct nvkm_devinit *); }; void nvkm_devinit_ctor(const struct nvkm_devinit_func *, struct nvkm_device *, From c2bb3be64eb7182285846123219230375af61abd Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:06 +0100 Subject: [PATCH 10/19] drm/client: Test for connectors before sending hotplug event Test for connectors in the client code and remove a similar test from the generic fbdev emulation. Do nothing if the test fails. Not having connectors indicates a driver bug. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-2-tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 5 +++++ drivers/gpu/drm/drm_fbdev_generic.c | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 262ec64d4397..09ac191c202d 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev) if (!drm_core_check_feature(dev, DRIVER_MODESET)) return; + if (!dev->mode_config.num_connector) { + drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n"); + return; + } + mutex_lock(&dev->clientlist_mutex); list_for_each_entry(client, &dev->clientlist, list) { if (!client->funcs || !client->funcs->hotplug) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 0a4c160e0e58..3d455a2e3fb5 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) if (dev->fb_helper) return drm_fb_helper_hotplug_event(dev->fb_helper); - if (!dev->mode_config.num_connector) { - drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n"); - return 0; - } - drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); ret = drm_fb_helper_init(dev, fb_helper); From 6a9d5ad3af65a1e7af97f25bbf83ce97bcfbab72 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:07 +0100 Subject: [PATCH 11/19] drm/client: Add hotplug_failed flag Signal failed hotplugging with a flag in struct drm_client_dev. If set, the client helpers will not further try to set up the fbdev display. This used to be signalled with a combination of cleared pointers in struct drm_fb_helper, which prevents us from initializing these pointers early after allocation. The change also harmonizes behavior among DRM clients. Additional DRM clients will now handle failed hotplugging like fbdev does. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-3-tzimmermann@suse.de --- drivers/gpu/drm/drm_client.c | 5 +++++ drivers/gpu/drm/drm_fbdev_generic.c | 4 ---- include/drm/drm_client.h | 8 ++++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 09ac191c202d..009e7b10455c 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev) if (!client->funcs || !client->funcs->hotplug) continue; + if (client->hotplug_failed) + continue; + ret = client->funcs->hotplug(client); drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret); + if (ret) + client->hotplug_failed = true; } mutex_unlock(&dev->clientlist_mutex); } diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 3d455a2e3fb5..135d58b8007b 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) struct drm_device *dev = client->dev; int ret; - /* Setup is not retried if it has failed */ - if (!fb_helper->dev && fb_helper->funcs) - return 0; - if (dev->fb_helper) return drm_fb_helper_hotplug_event(dev->fb_helper); diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index 4fc8018eddda..39482527a775 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -106,6 +106,14 @@ struct drm_client_dev { * @modesets: CRTC configurations */ struct drm_mode_set *modesets; + + /** + * @hotplug failed: + * + * Set by client hotplug helpers if the hotplugging failed + * before. It is usually not tried again. + */ + bool hotplug_failed; }; int drm_client_init(struct drm_device *dev, struct drm_client_dev *client, From 4825797c36da5537706a020567cb64933568d1eb Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:08 +0100 Subject: [PATCH 12/19] drm/fb-helper: Introduce drm_fb_helper_unprepare() Move the fb-helper clean-up code into drm_fb_helper_unprepare(). No functional changes. v2: * declare as static inline (kernel test robot) Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-4-tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 14 +++++++++++++- include/drm/drm_fb_helper.h | 5 +++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 367fb8b2d5fa..7fc717ab69b7 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -433,6 +433,18 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, } EXPORT_SYMBOL(drm_fb_helper_prepare); +/** + * drm_fb_helper_unprepare - clean up a drm_fb_helper structure + * @fb_helper: driver-allocated fbdev helper structure to set up + * + * Cleans up the framebuffer helper. Inverse of drm_fb_helper_prepare(). + */ +void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper) +{ + mutex_destroy(&fb_helper->lock); +} +EXPORT_SYMBOL(drm_fb_helper_unprepare); + /** * drm_fb_helper_init - initialize a &struct drm_fb_helper * @dev: drm device @@ -559,7 +571,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper) } mutex_unlock(&kernel_fb_helper_lock); - mutex_destroy(&fb_helper->lock); + drm_fb_helper_unprepare(fb_helper); if (!fb_helper->client.funcs) drm_client_release(&fb_helper->client); diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index f443e1f11654..39710c570a04 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -230,6 +230,7 @@ drm_fb_helper_from_client(struct drm_client_dev *client) #ifdef CONFIG_DRM_FBDEV_EMULATION void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, const struct drm_fb_helper_funcs *funcs); +void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper); int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *helper); void drm_fb_helper_fini(struct drm_fb_helper *helper); int drm_fb_helper_blank(int blank, struct fb_info *info); @@ -296,6 +297,10 @@ static inline void drm_fb_helper_prepare(struct drm_device *dev, { } +static inline void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper) +{ +} + static inline int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *helper) { From f73ab51bfd3ac6b4d2b9d0bbbef3e0cc57a0f079 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:09 +0100 Subject: [PATCH 13/19] drm/fbdev-generic: Initialize fb-helper structure in generic setup Initialize the fb-helper structure immediately after its allocation in drm_fbdev_generic_setup(). That will make it easier to fill it with driver-specific values, such as the preferred BPP. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-5-tzimmermann@suse.de --- drivers/gpu/drm/drm_fbdev_generic.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 135d58b8007b..63f66325a8a5 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -385,8 +385,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) if (dev->fb_helper) return drm_fb_helper_hotplug_event(dev->fb_helper); - drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); - ret = drm_fb_helper_init(dev, fb_helper); if (ret) goto err; @@ -456,12 +454,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev, fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL); if (!fb_helper) return; + drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs); if (ret) { - kfree(fb_helper); drm_err(dev, "Failed to register client: %d\n", ret); - return; + goto err_drm_client_init; } /* @@ -484,5 +482,12 @@ void drm_fbdev_generic_setup(struct drm_device *dev, drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); drm_client_register(&fb_helper->client); + + return; + +err_drm_client_init: + drm_fb_helper_unprepare(fb_helper); + kfree(fb_helper); + return; } EXPORT_SYMBOL(drm_fbdev_generic_setup); From ec9361a1374f8f1ff23d5b6c217326648a8191de Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:10 +0100 Subject: [PATCH 14/19] drm/fb-helper: Remove preferred_bpp parameter from fbdev internals Store the console's preferred BPP value in struct drm_fb_helper and remove the respective function parameters from the internal fbdev code. The BPP value is only required as a fallback and will now always be available in the fb-helper instance. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-6-tzimmermann@suse.de --- drivers/gpu/drm/drm_fb_helper.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 7fc717ab69b7..b1cf58fb698d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1784,7 +1784,7 @@ static uint32_t drm_fb_helper_find_color_mode_format(struct drm_fb_helper *fb_he return drm_fb_helper_find_format(fb_helper, formats, format_count, bpp, depth); } -static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp, +static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, struct drm_fb_helper_surface_size *sizes) { struct drm_client_dev *client = &fb_helper->client; @@ -1829,7 +1829,7 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int prefe surface_format = drm_fb_helper_find_color_mode_format(fb_helper, plane->format_types, plane->format_count, - preferred_bpp); + fb_helper->preferred_bpp); if (surface_format != DRM_FORMAT_INVALID) break; /* found supported format */ } @@ -1901,7 +1901,7 @@ static int __drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int prefe return 0; } -static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferred_bpp, +static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, struct drm_fb_helper_surface_size *sizes) { struct drm_client_dev *client = &fb_helper->client; @@ -1910,7 +1910,7 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferr int ret; mutex_lock(&client->modeset_mutex); - ret = __drm_fb_helper_find_sizes(fb_helper, preferred_bpp, sizes); + ret = __drm_fb_helper_find_sizes(fb_helper, sizes); mutex_unlock(&client->modeset_mutex); if (ret) @@ -1932,14 +1932,13 @@ static int drm_fb_helper_find_sizes(struct drm_fb_helper *fb_helper, int preferr * Allocates the backing storage and sets up the fbdev info structure through * the ->fb_probe callback. */ -static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, - int preferred_bpp) +static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper) { struct drm_client_dev *client = &fb_helper->client; struct drm_fb_helper_surface_size sizes; int ret; - ret = drm_fb_helper_find_sizes(fb_helper, preferred_bpp, &sizes); + ret = drm_fb_helper_find_sizes(fb_helper, &sizes); if (ret) { /* First time: disable all crtc's.. */ if (!fb_helper->deferred_setup) @@ -2117,8 +2116,7 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper) /* Note: Drops fb_helper->lock before returning. */ static int -__drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper, - int bpp_sel) +__drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; struct fb_info *info; @@ -2129,10 +2127,9 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper, height = dev->mode_config.max_height; drm_client_modeset_probe(&fb_helper->client, width, height); - ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel); + ret = drm_fb_helper_single_fb_probe(fb_helper); if (ret < 0) { if (ret == -EAGAIN) { - fb_helper->preferred_bpp = bpp_sel; fb_helper->deferred_setup = true; ret = 0; } @@ -2223,8 +2220,10 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) if (!drm_fbdev_emulation) return 0; + fb_helper->preferred_bpp = bpp_sel; + mutex_lock(&fb_helper->lock); - ret = __drm_fb_helper_initial_config_and_unlock(fb_helper, bpp_sel); + ret = __drm_fb_helper_initial_config_and_unlock(fb_helper); return ret; } @@ -2260,8 +2259,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) mutex_lock(&fb_helper->lock); if (fb_helper->deferred_setup) { - err = __drm_fb_helper_initial_config_and_unlock(fb_helper, - fb_helper->preferred_bpp); + err = __drm_fb_helper_initial_config_and_unlock(fb_helper); return err; } From 6c80a93be62d398e1854d95069340b2e60f96166 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:11 +0100 Subject: [PATCH 15/19] drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function Initialize the fb-helper's preferred_bpp field early from within drm_fb_helper_prepare(); instead of the later client hot-plugging callback. This simplifies the generic fbdev setup function. No real changes, but all drivers' fbdev code has to be adapted. v3: * build with CONFIG_DRM_FBDEV_EMULATION unset (kernel test bot) Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-7-tzimmermann@suse.de --- drivers/gpu/drm/armada/armada_fbdev.c | 4 ++-- drivers/gpu/drm/drm_fb_helper.c | 22 ++++++++++++++++++---- drivers/gpu/drm/drm_fbdev_generic.c | 19 ++----------------- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 ++-- drivers/gpu/drm/gma500/framebuffer.c | 4 ++-- drivers/gpu/drm/i915/display/intel_fbdev.c | 11 ++++++----- drivers/gpu/drm/msm/msm_fbdev.c | 4 ++-- drivers/gpu/drm/omapdrm/omap_fbdev.c | 4 ++-- drivers/gpu/drm/radeon/radeon_fb.c | 4 ++-- drivers/gpu/drm/tegra/fb.c | 7 +++---- include/drm/drm_fb_helper.h | 11 ++++++----- 11 files changed, 47 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c index 584cee123bd8..07e410c62b7a 100644 --- a/drivers/gpu/drm/armada/armada_fbdev.c +++ b/drivers/gpu/drm/armada/armada_fbdev.c @@ -129,7 +129,7 @@ int armada_fbdev_init(struct drm_device *dev) priv->fbdev = fbh; - drm_fb_helper_prepare(dev, fbh, &armada_fb_helper_funcs); + drm_fb_helper_prepare(dev, fbh, 32, &armada_fb_helper_funcs); ret = drm_fb_helper_init(dev, fbh); if (ret) { @@ -137,7 +137,7 @@ int armada_fbdev_init(struct drm_device *dev) goto err_fb_helper; } - ret = drm_fb_helper_initial_config(fbh, 32); + ret = drm_fb_helper_initial_config(fbh); if (ret) { DRM_ERROR("failed to set initial config\n"); goto err_fb_setup; diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b1cf58fb698d..97b34f377fe8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -414,14 +414,30 @@ static void drm_fb_helper_damage_work(struct work_struct *work) * drm_fb_helper_prepare - setup a drm_fb_helper structure * @dev: DRM device * @helper: driver-allocated fbdev helper structure to set up + * @preferred_bpp: Preferred bits per pixel for the device. * @funcs: pointer to structure of functions associate with this helper * * Sets up the bare minimum to make the framebuffer helper usable. This is * useful to implement race-free initialization of the polling helpers. */ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, + unsigned int preferred_bpp, const struct drm_fb_helper_funcs *funcs) { + /* + * Pick a preferred bpp of 32 if no value has been given. This + * will select XRGB8888 for the framebuffer formats. All drivers + * have to support XRGB8888 for backwards compatibility with legacy + * userspace, so it's the safe choice here. + * + * TODO: Replace struct drm_mode_config.preferred_depth and this + * bpp value with a preferred format that is given as struct + * drm_format_info. Then derive all other values from the + * format. + */ + if (!preferred_bpp) + preferred_bpp = 32; + INIT_LIST_HEAD(&helper->kernel_fb_list); spin_lock_init(&helper->damage_lock); INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker); @@ -430,6 +446,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, mutex_init(&helper->lock); helper->funcs = funcs; helper->dev = dev; + helper->preferred_bpp = preferred_bpp; } EXPORT_SYMBOL(drm_fb_helper_prepare); @@ -2175,7 +2192,6 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper) /** * drm_fb_helper_initial_config - setup a sane initial connector configuration * @fb_helper: fb_helper device struct - * @bpp_sel: bpp value to use for the framebuffer configuration * * Scans the CRTCs and connectors and tries to put together an initial setup. * At the moment, this is a cloned configuration across all heads with @@ -2213,15 +2229,13 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper) * RETURNS: * Zero if everything went ok, nonzero otherwise. */ -int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) +int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper) { int ret; if (!drm_fbdev_emulation) return 0; - fb_helper->preferred_bpp = bpp_sel; - mutex_lock(&fb_helper->lock); ret = __drm_fb_helper_initial_config_and_unlock(fb_helper); diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 63f66325a8a5..6ae014040df3 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -392,7 +392,7 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) if (!drm_drv_uses_atomic_modeset(dev)) drm_helper_disable_unused_functions(dev); - ret = drm_fb_helper_initial_config(fb_helper, fb_helper->preferred_bpp); + ret = drm_fb_helper_initial_config(fb_helper); if (ret) goto err_cleanup; @@ -454,7 +454,7 @@ void drm_fbdev_generic_setup(struct drm_device *dev, fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL); if (!fb_helper) return; - drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); + drm_fb_helper_prepare(dev, fb_helper, preferred_bpp, &drm_fb_helper_generic_funcs); ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs); if (ret) { @@ -462,21 +462,6 @@ void drm_fbdev_generic_setup(struct drm_device *dev, goto err_drm_client_init; } - /* - * Pick a preferred bpp of 32 if no value has been given. This - * will select XRGB8888 for the framebuffer formats. All drivers - * have to support XRGB8888 for backwards compatibility with legacy - * userspace, so it's the safe choice here. - * - * TODO: Replace struct drm_mode_config.preferred_depth and this - * bpp value with a preferred format that is given as struct - * drm_format_info. Then derive all other values from the - * format. - */ - if (!preferred_bpp) - preferred_bpp = 32; - fb_helper->preferred_bpp = preferred_bpp; - ret = drm_fbdev_client_hotplug(&fb_helper->client); if (ret) drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 55c92372fca0..b89e33af8da8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -163,7 +163,7 @@ int exynos_drm_fbdev_init(struct drm_device *dev) private->fb_helper = helper = &fbdev->drm_fb_helper; - drm_fb_helper_prepare(dev, helper, &exynos_drm_fb_helper_funcs); + drm_fb_helper_prepare(dev, helper, PREFERRED_BPP, &exynos_drm_fb_helper_funcs); ret = drm_fb_helper_init(dev, helper); if (ret < 0) { @@ -172,7 +172,7 @@ int exynos_drm_fbdev_init(struct drm_device *dev) goto err_init; } - ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP); + ret = drm_fb_helper_initial_config(helper); if (ret < 0) { DRM_DEV_ERROR(dev->dev, "failed to set up hw configuration.\n"); diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 52ae3ade9a61..1f04c07ee180 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -409,7 +409,7 @@ int psb_fbdev_init(struct drm_device *dev) dev_priv->fb_helper = fb_helper; - drm_fb_helper_prepare(dev, fb_helper, &psb_fb_helper_funcs); + drm_fb_helper_prepare(dev, fb_helper, 32, &psb_fb_helper_funcs); ret = drm_fb_helper_init(dev, fb_helper); if (ret) @@ -418,7 +418,7 @@ int psb_fbdev_init(struct drm_device *dev) /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - ret = drm_fb_helper_initial_config(fb_helper, 32); + ret = drm_fb_helper_initial_config(fb_helper); if (ret) goto fini; diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index bbdb98d7c96e..b6f5c554b50f 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -520,10 +520,12 @@ int intel_fbdev_init(struct drm_device *dev) return -ENOMEM; mutex_init(&ifbdev->hpd_lock); - drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs); + drm_fb_helper_prepare(dev, &ifbdev->helper, 32, &intel_fb_helper_funcs); - if (!intel_fbdev_init_bios(dev, ifbdev)) - ifbdev->preferred_bpp = 32; + if (intel_fbdev_init_bios(dev, ifbdev)) + ifbdev->helper.preferred_bpp = ifbdev->preferred_bpp; + else + ifbdev->preferred_bpp = ifbdev->helper.preferred_bpp; ret = drm_fb_helper_init(dev, &ifbdev->helper); if (ret) { @@ -542,8 +544,7 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) struct intel_fbdev *ifbdev = data; /* Due to peculiar init order wrt to hpd handling this is separate. */ - if (drm_fb_helper_initial_config(&ifbdev->helper, - ifbdev->preferred_bpp)) + if (drm_fb_helper_initial_config(&ifbdev->helper)) intel_fbdev_unregister(to_i915(ifbdev->helper.dev)); } diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c index 31e1e30cb52a..915b213f3a5c 100644 --- a/drivers/gpu/drm/msm/msm_fbdev.c +++ b/drivers/gpu/drm/msm/msm_fbdev.c @@ -146,7 +146,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev) helper = &fbdev->base; - drm_fb_helper_prepare(dev, helper, &msm_fb_helper_funcs); + drm_fb_helper_prepare(dev, helper, 32, &msm_fb_helper_funcs); ret = drm_fb_helper_init(dev, helper); if (ret) { @@ -159,7 +159,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev) if (ret) goto fini; - ret = drm_fb_helper_initial_config(helper, 32); + ret = drm_fb_helper_initial_config(helper); if (ret) goto fini; diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c index 98d8758048fc..fc5f52d567c6 100644 --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -239,13 +239,13 @@ void omap_fbdev_init(struct drm_device *dev) helper = &fbdev->base; - drm_fb_helper_prepare(dev, helper, &omap_fb_helper_funcs); + drm_fb_helper_prepare(dev, helper, 32, &omap_fb_helper_funcs); ret = drm_fb_helper_init(dev, helper); if (ret) goto fail; - ret = drm_fb_helper_initial_config(helper, 32); + ret = drm_fb_helper_initial_config(helper); if (ret) goto fini; diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index fe4087bfdb3c..6e5eed0e157c 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -348,7 +348,7 @@ int radeon_fbdev_init(struct radeon_device *rdev) rfbdev->rdev = rdev; rdev->mode_info.rfbdev = rfbdev; - drm_fb_helper_prepare(rdev->ddev, &rfbdev->helper, + drm_fb_helper_prepare(rdev->ddev, &rfbdev->helper, bpp_sel, &radeon_fb_helper_funcs); ret = drm_fb_helper_init(rdev->ddev, &rfbdev->helper); @@ -358,7 +358,7 @@ int radeon_fbdev_init(struct radeon_device *rdev) /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(rdev->ddev); - ret = drm_fb_helper_initial_config(&rfbdev->helper, bpp_sel); + ret = drm_fb_helper_initial_config(&rfbdev->helper); if (ret) goto fini; diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c index a900300ae5bd..153c39c32c71 100644 --- a/drivers/gpu/drm/tegra/fb.c +++ b/drivers/gpu/drm/tegra/fb.c @@ -308,7 +308,7 @@ static struct tegra_fbdev *tegra_fbdev_create(struct drm_device *drm) return ERR_PTR(-ENOMEM); } - drm_fb_helper_prepare(drm, &fbdev->base, &tegra_fb_helper_funcs); + drm_fb_helper_prepare(drm, &fbdev->base, 32, &tegra_fb_helper_funcs); return fbdev; } @@ -319,7 +319,6 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev) } static int tegra_fbdev_init(struct tegra_fbdev *fbdev, - unsigned int preferred_bpp, unsigned int num_crtc, unsigned int max_connectors) { @@ -333,7 +332,7 @@ static int tegra_fbdev_init(struct tegra_fbdev *fbdev, return err; } - err = drm_fb_helper_initial_config(&fbdev->base, preferred_bpp); + err = drm_fb_helper_initial_config(&fbdev->base); if (err < 0) { dev_err(drm->dev, "failed to set initial configuration: %d\n", err); @@ -396,7 +395,7 @@ int tegra_drm_fb_init(struct drm_device *drm) struct tegra_drm *tegra = drm->dev_private; int err; - err = tegra_fbdev_init(tegra->fbdev, 32, drm->mode_config.num_crtc, + err = tegra_fbdev_init(tegra->fbdev, drm->mode_config.num_crtc, drm->mode_config.num_connector); if (err < 0) return err; diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 39710c570a04..93bc8f29f5a4 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -229,6 +229,7 @@ drm_fb_helper_from_client(struct drm_client_dev *client) #ifdef CONFIG_DRM_FBDEV_EMULATION void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, + unsigned int preferred_bpp, const struct drm_fb_helper_funcs *funcs); void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper); int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *helper); @@ -284,7 +285,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg); int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper); -int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel); +int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper); int drm_fb_helper_debug_enter(struct fb_info *info); int drm_fb_helper_debug_leave(struct fb_info *info); @@ -292,8 +293,9 @@ void drm_fb_helper_lastclose(struct drm_device *dev); void drm_fb_helper_output_poll_changed(struct drm_device *dev); #else static inline void drm_fb_helper_prepare(struct drm_device *dev, - struct drm_fb_helper *helper, - const struct drm_fb_helper_funcs *funcs) + struct drm_fb_helper *helper, + unsigned int preferred_bpp, + const struct drm_fb_helper_funcs *funcs) { } @@ -455,8 +457,7 @@ static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) return 0; } -static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, - int bpp_sel) +static inline int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper) { return 0; } From 643231b28380c9e9cba11675ef8f480016feaec3 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:12 +0100 Subject: [PATCH 16/19] drm/fbdev-generic: Minimize hotplug error handling Call drm_fb_helper_fini() in the generic-fbdev hotplug helper to revert the effects of drm_fb_helper_init(). No full cleanup is required. v3: * fix error in commit message (Javier) Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-8-tzimmermann@suse.de --- drivers/gpu/drm/drm_fbdev_generic.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 6ae014040df3..dd8be5e0f271 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -387,25 +387,21 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) ret = drm_fb_helper_init(dev, fb_helper); if (ret) - goto err; + goto err_drm_err; if (!drm_drv_uses_atomic_modeset(dev)) drm_helper_disable_unused_functions(dev); ret = drm_fb_helper_initial_config(fb_helper); if (ret) - goto err_cleanup; + goto err_drm_fb_helper_fini; return 0; -err_cleanup: - drm_fbdev_cleanup(fb_helper); -err: - fb_helper->dev = NULL; - fb_helper->info = NULL; - +err_drm_fb_helper_fini: + drm_fb_helper_fini(fb_helper); +err_drm_err: drm_err(dev, "fbdev: Failed to setup generic emulation (ret=%d)\n", ret); - return ret; } From 032116bbe152594f5528683c671a04259e6b4ed7 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:13 +0100 Subject: [PATCH 17/19] drm/fbdev-generic: Minimize client unregistering For uninitialized framebuffers, only release the DRM client and free the fbdev memory. Do not attempt to clean up the framebuffer. DRM fbdev clients have a two-step initialization: first create the DRM client; then create the framebuffer device on the first successful hotplug event. In cases where the client never creates the framebuffer, only the client state needs to be released. We can detect which case it is, full or client-only cleanup, by looking at the presence of fb_helper's info field. v3: * fix typo in commit message (Javier) * release client before unpreparing fbdev v2: * remove test for (fbi != NULL) in drm_fbdev_cleanup() (Sam) Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-9-tzimmermann@suse.de --- drivers/gpu/drm/drm_fbdev_generic.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index dd8be5e0f271..a9c519001019 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -51,12 +51,10 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper) if (!fb_helper->dev) return; - if (fbi) { - if (fbi->fbdefio) - fb_deferred_io_cleanup(fbi); - if (drm_fbdev_use_shadow_fb(fb_helper)) - shadow = fbi->screen_buffer; - } + if (fbi->fbdefio) + fb_deferred_io_cleanup(fbi); + if (drm_fbdev_use_shadow_fb(fb_helper)) + shadow = fbi->screen_buffer; drm_fb_helper_fini(fb_helper); @@ -362,11 +360,13 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client) { struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client); - if (fb_helper->info) - /* drm_fbdev_fb_destroy() takes care of cleanup */ + if (fb_helper->info) { drm_fb_helper_unregister_info(fb_helper); - else - drm_fbdev_release(fb_helper); + } else { + drm_client_release(&fb_helper->client); + drm_fb_helper_unprepare(fb_helper); + kfree(fb_helper); + } } static int drm_fbdev_client_restore(struct drm_client_dev *client) From 7f5fe873968d49aeb9d805235acf57641a691b8f Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:14 +0100 Subject: [PATCH 18/19] drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy() The fbdev framebuffer cleanup in drm_fbdev_fb_destroy() calls drm_fbdev_release() and drm_fbdev_cleanup(). Inline both into the caller. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-10-tzimmermann@suse.de --- drivers/gpu/drm/drm_fbdev_generic.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index a9c519001019..68ce652e3a14 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -43,8 +43,9 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) return 0; } -static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper) +static void drm_fbdev_fb_destroy(struct fb_info *info) { + struct drm_fb_helper *fb_helper = info->par; struct fb_info *fbi = fb_helper->info; void *shadow = NULL; @@ -64,24 +65,10 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper) drm_client_buffer_vunmap(fb_helper->buffer); drm_client_framebuffer_delete(fb_helper->buffer); -} - -static void drm_fbdev_release(struct drm_fb_helper *fb_helper) -{ - drm_fbdev_cleanup(fb_helper); drm_client_release(&fb_helper->client); kfree(fb_helper); } -/* - * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of - * unregister_framebuffer() or fb_release(). - */ -static void drm_fbdev_fb_destroy(struct fb_info *info) -{ - drm_fbdev_release(info->par); -} - static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) { struct drm_fb_helper *fb_helper = info->par; From 6ca80b9e5cc0120c37e2e7dd367b08e3e0eb8289 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Wed, 25 Jan 2023 21:04:15 +0100 Subject: [PATCH 19/19] drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info' The generic fbdev emulation names variables of type struct fb_info both 'fbi' and 'info'. The latter seems to be more common in fbdev code, so name fbi accordingly. Also replace the duplicate variable in drm_fbdev_fb_destroy(). Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Link: https://patchwork.freedesktop.org/patch/msgid/20230125200415.14123-11-tzimmermann@suse.de --- drivers/gpu/drm/drm_fbdev_generic.c | 47 ++++++++++++++--------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 68ce652e3a14..43f94aa9e015 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -46,16 +46,15 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) static void drm_fbdev_fb_destroy(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; - struct fb_info *fbi = fb_helper->info; void *shadow = NULL; if (!fb_helper->dev) return; - if (fbi->fbdefio) - fb_deferred_io_cleanup(fbi); + if (info->fbdefio) + fb_deferred_io_cleanup(info); if (drm_fbdev_use_shadow_fb(fb_helper)) - shadow = fbi->screen_buffer; + shadow = info->screen_buffer; drm_fb_helper_fini(fb_helper); @@ -171,7 +170,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, struct drm_device *dev = fb_helper->dev; struct drm_client_buffer *buffer; struct drm_framebuffer *fb; - struct fb_info *fbi; + struct fb_info *info; u32 format; struct iosys_map map; int ret; @@ -190,35 +189,35 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, fb_helper->fb = buffer->fb; fb = buffer->fb; - fbi = drm_fb_helper_alloc_info(fb_helper); - if (IS_ERR(fbi)) - return PTR_ERR(fbi); + info = drm_fb_helper_alloc_info(fb_helper); + if (IS_ERR(info)) + return PTR_ERR(info); - fbi->fbops = &drm_fbdev_fb_ops; - fbi->screen_size = sizes->surface_height * fb->pitches[0]; - fbi->fix.smem_len = fbi->screen_size; - fbi->flags = FBINFO_DEFAULT; + info->fbops = &drm_fbdev_fb_ops; + info->screen_size = sizes->surface_height * fb->pitches[0]; + info->fix.smem_len = info->screen_size; + info->flags = FBINFO_DEFAULT; - drm_fb_helper_fill_info(fbi, fb_helper, sizes); + drm_fb_helper_fill_info(info, fb_helper, sizes); if (drm_fbdev_use_shadow_fb(fb_helper)) { - fbi->screen_buffer = vzalloc(fbi->screen_size); - if (!fbi->screen_buffer) + info->screen_buffer = vzalloc(info->screen_size); + if (!info->screen_buffer) return -ENOMEM; - fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; + info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; - fbi->fbdefio = &drm_fbdev_defio; - fb_deferred_io_init(fbi); + info->fbdefio = &drm_fbdev_defio; + fb_deferred_io_init(info); } else { /* buffer is mapped for HW framebuffer */ ret = drm_client_buffer_vmap(fb_helper->buffer, &map); if (ret) return ret; if (map.is_iomem) { - fbi->screen_base = map.vaddr_iomem; + info->screen_base = map.vaddr_iomem; } else { - fbi->screen_buffer = map.vaddr; - fbi->flags |= FBINFO_VIRTFB; + info->screen_buffer = map.vaddr; + info->flags |= FBINFO_VIRTFB; } /* @@ -227,10 +226,10 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, * case. */ #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) - if (fb_helper->hint_leak_smem_start && fbi->fix.smem_start == 0 && + if (fb_helper->hint_leak_smem_start && info->fix.smem_start == 0 && !drm_WARN_ON_ONCE(dev, map.is_iomem)) - fbi->fix.smem_start = - page_to_phys(virt_to_page(fbi->screen_buffer)); + info->fix.smem_start = + page_to_phys(virt_to_page(info->screen_buffer)); #endif }