[Intel-gfx] [PATCH 17/20] drm/i915/uapi: add NEEDS_CPU_ACCESS hint
If set, force the allocation to be placed in the mappable portion of LMEM. One big restriction here is that system memory must be given as a potential placement for the object, that way we can always spill the object into system memory if we can't make space. XXX: Still very much WIP and needs IGTs. Including now just for the sake of having more complete picture. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 28 --- include/uapi/drm/i915_drm.h| 31 +- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index e7456443f163..98d63cb21e94 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -238,6 +238,7 @@ struct create_ext { struct drm_i915_private *i915; struct intel_memory_region *placements[INTEL_REGION_UNKNOWN]; unsigned int n_placements; + unsigned int placement_mask; unsigned long flags; }; @@ -334,6 +335,7 @@ static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, for (i = 0; i < args->num_regions; i++) ext_data->placements[i] = placements[i]; + ext_data->placement_mask = mask; return 0; out_dump: @@ -408,7 +410,7 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, struct drm_i915_gem_object *obj; int ret; - if (args->flags) + if (args->flags & ~I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS) return -EINVAL; ret = i915_user_extensions(u64_to_user_ptr(args->extensions), @@ -424,14 +426,22 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, ext_data.n_placements = 1; } - /* -* TODO: add a userspace hint to force CPU_ACCESS for the object, which -* can override this. -*/ - if (!IS_DG1(i915) && (ext_data.n_placements > 1 || - ext_data.placements[0]->type != - INTEL_MEMORY_SYSTEM)) - ext_data.flags |= I915_BO_ALLOC_TOPDOWN; + if (args->flags & I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS) { + if (ext_data.n_placements == 1) + return -EINVAL; + + /* +* We always need to be able to spill to system memory, if we +* can't place in the mappable part of LMEM. +*/ + if (!(ext_data.placement_mask & BIT(INTEL_REGION_SMEM))) + return -EINVAL; + } else { + if (!IS_DG1(i915) && (ext_data.n_placements > 1 || + ext_data.placements[0]->type != + INTEL_MEMORY_SYSTEM)) + ext_data.flags |= I915_BO_ALLOC_TOPDOWN; + } obj = __i915_gem_object_create_user_ext(i915, args->size, ext_data.placements, diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 914ebd9290e5..ecfa805549a7 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3157,7 +3157,36 @@ struct drm_i915_gem_create_ext { * Object handles are nonzero. */ __u32 handle; - /** @flags: MBZ */ + /** +* @flags: Optional flags. +* +* Supported values: +* +* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that +* the object will need to be accessed via the CPU. +* +* Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and +* only strictly required on platforms where only some of the device +* memory is directly visible or mappable through the CPU, like on DG2+. +* +* One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to +* ensure we can always spill the allocation to system memory, if we +* can't place the object in the mappable part of +* I915_MEMORY_CLASS_DEVICE. +* +* Note that buffers that need to be captured with EXEC_OBJECT_CAPTURE, +* will need to enable this hint, if the object can also be placed in +* I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call will +* throw an error otherwise. This also means that such objects will need +* I915_MEMORY_CLASS_SYSTEM set as a possible placement. +* +* Without this hint, the kernel will assume that non-mappable +* I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that the +* kernel can still migrate the object to the mappable part, as a last +* resort, if userspace ever CPU faults this object, but this might be +* expensive, and so ideally should be avoided. +*/ +#define I915_GEM_CREATE_
[Intel-gfx] [PATCH 13/20] drm/i915/ttm: mappable migration on fault
The end goal is to have userspace tell the kernel what buffers will require CPU access, however if we ever reach the CPU fault handler, and the current resource is not mappable, then we should attempt to migrate the buffer to the mappable portion of LMEM, or even system memory, if the allowable placements permit it. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 58 ++--- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 8376e4c3d290..7299053fb1ec 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -636,11 +636,25 @@ static void i915_ttm_swap_notify(struct ttm_buffer_object *bo) i915_ttm_purge(obj); } +static bool i915_ttm_resource_mappable(struct ttm_resource *res) +{ + struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res); + + if (!i915_ttm_cpu_maps_iomem(res)) + return true; + + return bman_res->used_visible_size == bman_res->base.num_pages; +} + static int i915_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource *mem) { + if (!i915_ttm_cpu_maps_iomem(mem)) return 0; + if (!i915_ttm_resource_mappable(mem)) + return -EINVAL; + mem->bus.caching = ttm_write_combined; mem->bus.is_iomem = true; @@ -779,14 +793,15 @@ static int i915_ttm_get_pages(struct drm_i915_gem_object *obj) * Gem forced migration using the i915_ttm_migrate() op, is allowed even * to regions that are not in the object's list of allowable placements. */ -static int i915_ttm_migrate(struct drm_i915_gem_object *obj, - struct intel_memory_region *mr) +static int __i915_ttm_migrate(struct drm_i915_gem_object *obj, + struct intel_memory_region *mr, + unsigned int flags) { struct ttm_place requested; struct ttm_placement placement; int ret; - i915_ttm_place_from_region(mr, &requested, obj->flags); + i915_ttm_place_from_region(mr, &requested, flags); placement.num_placement = 1; placement.num_busy_placement = 1; placement.placement = &requested; @@ -809,6 +824,12 @@ static int i915_ttm_migrate(struct drm_i915_gem_object *obj, return 0; } +static int i915_ttm_migrate(struct drm_i915_gem_object *obj, + struct intel_memory_region *mr) +{ + return __i915_ttm_migrate(obj, mr, obj->flags); +} + static void i915_ttm_put_pages(struct drm_i915_gem_object *obj, struct sg_table *st) { @@ -940,6 +961,10 @@ static void i915_ttm_delayed_free(struct drm_i915_gem_object *obj) ttm_bo_put(i915_gem_to_ttm(obj)); } +static int __i915_ttm_migrate(struct drm_i915_gem_object *obj, + struct intel_memory_region *mr, + unsigned int flags); + static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) { struct vm_area_struct *area = vmf->vma; @@ -953,9 +978,6 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) if (!obj) return VM_FAULT_SIGBUS; - if (obj->flags & I915_BO_ALLOC_TOPDOWN) - return -EINVAL; - /* Sanity check that we allow writing into this object */ if (unlikely(i915_gem_object_is_readonly(obj) && area->vm_flags & VM_WRITE)) @@ -970,6 +992,30 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) return VM_FAULT_SIGBUS; } + if (!i915_ttm_resource_mappable(bo->resource)) { + int err = -ENODEV; + int i; + + for (i = 0; i < obj->mm.n_placements; i++) { + struct intel_memory_region *mr = obj->mm.placements[i]; + unsigned int flags; + + if (!mr->io_size && mr->type != INTEL_MEMORY_SYSTEM) + continue; + + flags = obj->flags; + flags &= ~I915_BO_ALLOC_TOPDOWN; + err = __i915_ttm_migrate(obj, mr, flags); + if (!err) + break; + } + + if (err) { + dma_resv_unlock(bo->base.resv); + return VM_FAULT_SIGBUS; + } + } + if (drm_dev_enter(dev, &idx)) { ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot, TTM_BO_VM_NUM_PREFAULT); -- 2.34.1
[Intel-gfx] [PATCH 16/20] drm/i915/create: apply ALLOC_TOPDOWN by default
Starting from DG2+, when dealing with LMEM, we assume that by default all userspace allocations should be placed in the non-mappable portion of LMEM. Note that dumb buffers are not included here, since these are not "GPU accelerated" and likely need CPU access. In a later patch userspace will be able to provide a hint if CPU access to the buffer is needed. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 9402d4bf4ffc..e7456443f163 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -424,6 +424,15 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, ext_data.n_placements = 1; } + /* +* TODO: add a userspace hint to force CPU_ACCESS for the object, which +* can override this. +*/ + if (!IS_DG1(i915) && (ext_data.n_placements > 1 || + ext_data.placements[0]->type != + INTEL_MEMORY_SYSTEM)) + ext_data.flags |= I915_BO_ALLOC_TOPDOWN; + obj = __i915_gem_object_create_user_ext(i915, args->size, ext_data.placements, ext_data.n_placements, -- 2.34.1
[Intel-gfx] [PATCH 19/20] drm/i915/lmem: don't treat small BAR as an error
Just pass along the probed io_size. The backend should be able to utilize the entire range here, even if some of it is non-mappable. It does leave open with what to do with stolen local-memory. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gt/intel_region_lmem.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c index 2c7ec7ff79fd..b788fc2b3df8 100644 --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c @@ -200,6 +200,7 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) struct intel_memory_region *mem; resource_size_t min_page_size; resource_size_t io_start; + resource_size_t io_size; resource_size_t lmem_size; int err; @@ -210,7 +211,8 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) lmem_size = intel_uncore_read64(uncore, GEN12_GSMBASE); io_start = pci_resource_start(pdev, 2); - if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2))) + io_size = min(pci_resource_len(pdev, 2), lmem_size); + if (!io_size) return ERR_PTR(-ENODEV); min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K : @@ -220,7 +222,7 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) lmem_size, min_page_size, io_start, -lmem_size, +io_size, INTEL_MEMORY_LOCAL, 0, &intel_region_lmem_ops); -- 2.34.1
[Intel-gfx] [PATCH 15/20] drm/i915/selftests: handle allocation failures
If we have to contend with non-mappable LMEM, then we need to ensure the object fits within the mappable portion, like in the selftests, where we later try to CPU access the pages. However if it can't then we need to gracefully handle this, without throwing an error. Also it looks like TTM will return -ENOMEM if the object can't be placed. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/selftests/intel_memory_region.c | 8 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index 42db9cd30978..3caa178bbd07 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -1344,7 +1344,7 @@ static int igt_ppgtt_smoke_huge(void *arg) err = i915_gem_object_pin_pages_unlocked(obj); if (err) { - if (err == -ENXIO || err == -E2BIG) { + if (err == -ENXIO || err == -E2BIG || err == -ENOMEM) { i915_gem_object_put(obj); size >>= 1; goto try_again; diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c index 04ae29779206..87bff7f83554 100644 --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c @@ -822,8 +822,14 @@ static int igt_lmem_create_with_ps(void *arg) i915_gem_object_lock(obj, NULL); err = i915_gem_object_pin_pages(obj); - if (err) + if (err) { + if (err == -ENXIO || err == -E2BIG || err == -ENOMEM) { + pr_info("%s not enough lmem for ps(%u) err=%d\n", + __func__, ps, err); + err = 0; + } goto out_put; + } daddr = i915_gem_object_get_dma_address(obj, 0); if (!IS_ALIGNED(daddr, ps)) { -- 2.34.1
[Intel-gfx] [PATCH 18/20] drm/i915/uapi: forbid ALLOC_TOPDOWN for error capture
On platforms where there might be non-mappable LMEM, force userspace to mark the buffers with the correct hint. When dumping the BO contents during capture we need CPU access. Note this only applies to buffers that can be placed in LMEM, and also doesn't impact DG1. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 498b458fd784..3c8083852620 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1965,7 +1965,7 @@ eb_find_first_request_added(struct i915_execbuffer *eb) #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) /* Stage with GFP_KERNEL allocations before we enter the signaling critical path */ -static void eb_capture_stage(struct i915_execbuffer *eb) +static int eb_capture_stage(struct i915_execbuffer *eb) { const unsigned int count = eb->buffer_count; unsigned int i = count, j; @@ -1978,6 +1978,9 @@ static void eb_capture_stage(struct i915_execbuffer *eb) if (!(flags & EXEC_OBJECT_CAPTURE)) continue; + if (vma->obj->flags & I915_BO_ALLOC_TOPDOWN) + return -EINVAL; + for_each_batch_create_order(eb, j) { struct i915_capture_list *capture; @@ -1990,6 +1993,8 @@ static void eb_capture_stage(struct i915_execbuffer *eb) eb->capture_lists[j] = capture; } } + + return 0; } /* Commit once we're in the critical path */ @@ -3418,7 +3423,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, } ww_acquire_done(&eb.ww.ctx); - eb_capture_stage(&eb); + err = eb_capture_stage(&eb); + if (err) + goto err_vma; out_fence = eb_requests_create(&eb, in_fence, out_fence_fd); if (IS_ERR(out_fence)) { -- 2.34.1
[Intel-gfx] [PATCH 14/20] drm/i915/selftests: exercise mmap migration
Exercise each of the migration scenarios, verifying that the final placement and buffer contents match our expectations. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- .../drm/i915/gem/selftests/i915_gem_mman.c| 306 ++ 1 file changed, 306 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index ba29767348be..d2c1071df98a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -10,6 +10,7 @@ #include "gt/intel_gpu_commands.h" #include "gt/intel_gt.h" #include "gt/intel_gt_pm.h" +#include "gt/intel_migrate.h" #include "gem/i915_gem_region.h" #include "huge_gem_object.h" #include "i915_selftest.h" @@ -999,6 +1000,310 @@ static int igt_mmap(void *arg) return 0; } +static void igt_close_objects(struct drm_i915_private *i915, + struct list_head *objects) +{ + struct drm_i915_gem_object *obj, *on; + + list_for_each_entry_safe(obj, on, objects, st_link) { + i915_gem_object_lock(obj, NULL); + if (i915_gem_object_has_pinned_pages(obj)) + i915_gem_object_unpin_pages(obj); + /* No polluting the memory region between tests */ + __i915_gem_object_put_pages(obj); + i915_gem_object_unlock(obj); + list_del(&obj->st_link); + i915_gem_object_put(obj); + } + + cond_resched(); + + i915_gem_drain_freed_objects(i915); +} + +static void igt_make_evictable(struct list_head *objects) +{ + struct drm_i915_gem_object *obj; + + list_for_each_entry(obj, objects, st_link) { + i915_gem_object_lock(obj, NULL); + if (i915_gem_object_has_pinned_pages(obj)) + i915_gem_object_unpin_pages(obj); + i915_gem_object_unlock(obj); + } + + cond_resched(); +} + +static int igt_fill_mappable(struct intel_memory_region *mr, +struct list_head *objects) +{ + u64 size, total; + int err; + + total = 0; + size = mr->io_size; + do { + struct drm_i915_gem_object *obj; + + obj = i915_gem_object_create_region(mr, size, 0, 0); + if (IS_ERR(obj)) { + err = PTR_ERR(obj); + goto err_close; + } + + list_add(&obj->st_link, objects); + + err = i915_gem_object_pin_pages_unlocked(obj); + if (err) { + if (err != -ENXIO && err != -ENOMEM) + goto err_close; + + if (size == mr->min_page_size) { + err = 0; + break; + } + + size >>= 1; + continue; + } + + total += obj->base.size; + } while (1); + + pr_info("%s filled=%lluMiB\n", __func__, total >> 20); + return 0; + +err_close: + igt_close_objects(mr->i915, objects); + return err; +} + +static int ___igt_mmap_migrate(struct drm_i915_private *i915, + struct drm_i915_gem_object *obj, + unsigned long addr, + bool unfaultable) +{ + struct vm_area_struct *area; + int err = 0, i; + + pr_info("igt_mmap(%s, %d) @ %lx\n", +obj->mm.region->name, I915_MMAP_TYPE_FIXED, addr); + + mmap_read_lock(current->mm); + area = vma_lookup(current->mm, addr); + mmap_read_unlock(current->mm); + if (!area) { + pr_err("%s: Did not create a vm_area_struct for the mmap\n", + obj->mm.region->name); + err = -EINVAL; + goto out_unmap; + } + + for (i = 0; i < obj->base.size / sizeof(u32); i++) { + u32 __user *ux = u64_to_user_ptr((u64)(addr + i * sizeof(*ux))); + u32 x; + + if (get_user(x, ux)) { + err = -EFAULT; + if (!unfaultable) { + pr_err("%s: Unable to read from mmap, offset:%zd\n", + obj->mm.region->name, i * sizeof(x)); + goto out_unmap; + } + + continue; + } + + if (unfaultable) { + pr_err("%s: Faulted unmappable memory\n", + obj->mm.region->name); + err = -EINVAL; + goto out_unmap; + } + + if (x != expand32(POISON_INUSE)) { + pr_err("%s: Read incorrect value from mmap, offset:%zd, found:%x, expected:%x\n", +
[Intel-gfx] [PATCH 09/20] drm/i915/buddy: tweak 2big check
Otherwise we get -EINVAL, instead of the more useful -E2BIG if the allocation doesn't fit within the pfn range, like with mappable lmem. The hugepages selftest, for example, needs this to know if a smaller size is needed. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index bc725a92fc5c..7c24cc6608e3 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -81,7 +81,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, lpfn = pages; } - if (size > mm->size) { + if (size > lpfn << PAGE_SHIFT) { err = -E2BIG; goto err_free_res; } -- 2.34.1
[Intel-gfx] [PATCH 12/20] drm/i915/ttm: make eviction mappable aware
If we need to make room for some some mappable object, then we should only victimize objects that have one or pages that occupy the visible portion of LMEM. Let's also create a new priority hint for objects that are placed in mappable memory, where we know that CPU access was requested, that way we hopefully victimize these last. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 65 - 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e4cd6ccf5ab1..8376e4c3d290 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -5,8 +5,10 @@ #include #include +#include #include "i915_drv.h" +#include "i915_ttm_buddy_manager.h" #include "intel_memory_region.h" #include "intel_region_ttm.h" @@ -20,6 +22,7 @@ #define I915_TTM_PRIO_PURGE 0 #define I915_TTM_PRIO_NO_PAGES 1 #define I915_TTM_PRIO_HAS_PAGES 2 +#define I915_TTM_PRIO_NEEDS_CPU_ACCESS 3 /* * Size of struct ttm_place vector in on-stack struct ttm_placement allocs @@ -337,6 +340,7 @@ static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo, const struct ttm_place *place) { struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo); + struct ttm_resource *res = bo->resource; if (!obj) return false; @@ -350,7 +354,48 @@ static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo, return false; /* Will do for now. Our pinned objects are still on TTM's LRU lists */ - return i915_gem_object_evictable(obj); + if (!i915_gem_object_evictable(obj)) + return false; + + switch (res->mem_type) { + case TTM_PL_PRIV: { + struct ttm_resource_manager *man = + ttm_manager_type(bo->bdev, res->mem_type); + struct i915_ttm_buddy_resource *bman_res = + to_ttm_buddy_resource(res); + struct drm_buddy *mm = bman_res->mm; + struct drm_buddy_block *block; + + if (!place->fpfn && !place->lpfn) + return true; + + GEM_BUG_ON(!place->lpfn); + + /* +* If we just want something mappable then we can quickly check +* if the current victim resource is using any of the CPU +* visible portion. +*/ + if (!place->fpfn && + place->lpfn == i915_ttm_buddy_man_visible_size(man)) + return bman_res->used_visible_size > 0; + + /* Real range allocation */ + list_for_each_entry(block, &bman_res->blocks, link) { + unsigned long fpfn = + drm_buddy_block_offset(block) >> PAGE_SHIFT; + unsigned long lpfn = fpfn + + (drm_buddy_block_size(mm, block) >> PAGE_SHIFT); + + if (place->fpfn < lpfn && place->lpfn > fpfn) + return true; + } + return false; + } default: + break; + } + + return true; } static void i915_ttm_evict_flags(struct ttm_buffer_object *bo, @@ -850,7 +895,23 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) } else if (!i915_gem_object_has_pages(obj)) { bo->priority = I915_TTM_PRIO_NO_PAGES; } else { - bo->priority = I915_TTM_PRIO_HAS_PAGES; + struct ttm_resource_manager *man = + ttm_manager_type(bo->bdev, bo->resource->mem_type); + + /* +* If we need to place an LMEM resource which doesn't need CPU +* access then we should try not to victimize mappable objects +* first, since we likely end up stealing more of the mappable +* portion. And likewise when we try to find space for a mappble +* object, we know not to ever victimize objects that don't +* occupy any mappable pages. +*/ + if (i915_ttm_cpu_maps_iomem(bo->resource) && + i915_ttm_buddy_man_visible_size(man) < man->size && + !(obj->flags & I915_BO_ALLOC_TOPDOWN)) + bo->priority = I915_TTM_PRIO_NEEDS_CPU_ACCESS; + else + bo->priority = I915_TTM_PRIO_HAS_PAGES; } ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); -- 2.34.1
[Intel-gfx] [PATCH 08/20] drm/i915/buddy: adjust res->start
Differentiate between mappable vs non-mappable resources, also if this is an actual range allocation ensure we set res->start as the starting pfn. Later when we need to do non-mappable -> mappable moves then we want TTM to see that the current placement is not compatible, which should result in an actual move, instead of being turned into a noop. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index 6e5842155898..bc725a92fc5c 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -140,6 +140,13 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, mutex_unlock(&bman->lock); } + if (place->lpfn - place->fpfn == n_pages) + bman_res->base.start = place->fpfn; + else if (lpfn <= bman->visible_size) + bman_res->base.start = 0; + else + bman_res->base.start = bman->visible_size; + *res = &bman_res->base; return 0; -- 2.34.1
[Intel-gfx] [PATCH 11/20] drm/i915/ttm: tweak priority hint selection
For some reason we are selecting PRIO_HAS_PAGES when we don't have mm.pages, and vice versa. Perhaps something else is going on here. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e60b677ecd54..e4cd6ccf5ab1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -848,11 +848,9 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj) } else if (obj->mm.madv != I915_MADV_WILLNEED) { bo->priority = I915_TTM_PRIO_PURGE; } else if (!i915_gem_object_has_pages(obj)) { - if (bo->priority < I915_TTM_PRIO_HAS_PAGES) - bo->priority = I915_TTM_PRIO_HAS_PAGES; + bo->priority = I915_TTM_PRIO_NO_PAGES; } else { - if (bo->priority > I915_TTM_PRIO_NO_PAGES) - bo->priority = I915_TTM_PRIO_NO_PAGES; + bo->priority = I915_TTM_PRIO_HAS_PAGES; } ttm_bo_move_to_lru_tail(bo, bo->resource, NULL); -- 2.34.1
[Intel-gfx] [PATCH 10/20] drm/i915/selftests: mock test io_size
Check that mappable vs non-mappable matches our expectations. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- .../drm/i915/selftests/intel_memory_region.c | 143 ++ 1 file changed, 143 insertions(+) diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c index 247f65f02bbf..04ae29779206 100644 --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c @@ -17,6 +17,7 @@ #include "gem/i915_gem_context.h" #include "gem/i915_gem_lmem.h" #include "gem/i915_gem_region.h" +#include "gem/i915_gem_ttm.h" #include "gem/selftests/igt_gem_utils.h" #include "gem/selftests/mock_context.h" #include "gt/intel_engine_pm.h" @@ -512,6 +513,147 @@ static int igt_mock_max_segment(void *arg) return err; } +static u64 igt_object_mappable_total(struct drm_i915_gem_object *obj) +{ + struct intel_memory_region *mr = obj->mm.region; + struct i915_ttm_buddy_resource *bman_res = + to_ttm_buddy_resource(obj->mm.res); + struct drm_buddy *mm = bman_res->mm; + struct drm_buddy_block *block; + u64 total; + + total = 0; + list_for_each_entry(block, &bman_res->blocks, link) { + u64 start = drm_buddy_block_offset(block); + u64 end = start + drm_buddy_block_size(mm, block); + + if (start < mr->io_size) + total += min_t(u64, end, mr->io_size) - start; + } + + return total; +} + +static int igt_mock_io_size(void *arg) +{ + struct intel_memory_region *mr = arg; + struct drm_i915_private *i915 = mr->i915; + struct drm_i915_gem_object *obj; + u64 mappable_theft_total; + u64 io_size; + u64 total; + u64 ps; + u64 rem; + u64 size; + I915_RND_STATE(prng); + LIST_HEAD(objects); + int err = 0; + + ps = SZ_4K; + if (i915_prandom_u64_state(&prng) & 1) + ps = SZ_64K; /* For something like DG2 */ + + div64_u64_rem(i915_prandom_u64_state(&prng), SZ_8G, &total); + total = round_down(total, ps); + total = max_t(u64, total, SZ_1G); + + div64_u64_rem(i915_prandom_u64_state(&prng), total - ps, &io_size); + io_size = round_down(io_size, ps); + io_size = max_t(u64, io_size, SZ_256M); /* 256M seems to be the common lower limit */ + + pr_info("%s with ps=%llx, io_size=%llx, total=%llx\n", + __func__, ps, io_size, total); + + mr = mock_region_create(i915, 0, total, ps, 0, io_size); + if (IS_ERR(mr)) { + err = PTR_ERR(mr); + goto out_err; + } + + mappable_theft_total = 0; + rem = total - io_size; + do { + div64_u64_rem(i915_prandom_u64_state(&prng), rem, &size); + size = round_down(size, ps); + size = max(size, ps); + + obj = igt_object_create(mr, &objects, size, + I915_BO_ALLOC_TOPDOWN); + if (IS_ERR(obj)) { + pr_err("%s TOPDOWN failed with rem=%llx, size=%llx\n", + __func__, rem, size); + err = PTR_ERR(obj); + goto out_close; + } + + mappable_theft_total += igt_object_mappable_total(obj); + rem -= size; + } while (rem); + + pr_info("%s mappable theft=(%lluMiB/%lluMiB), total=%lluMiB\n", + __func__, + (u64)mappable_theft_total >> 20, + (u64)io_size >> 20, + (u64)total >> 20); + + /* +* Even if we allocate all of the non-mappable portion, we should still +* be able to dip into the mappable portion. +*/ + obj = igt_object_create(mr, &objects, io_size, + I915_BO_ALLOC_TOPDOWN); + if (IS_ERR(obj)) { + pr_err("%s allocation unexpectedly failed\n", __func__); + err = PTR_ERR(obj); + goto out_close; + } + + close_objects(mr, &objects); + + rem = io_size; + do { + div64_u64_rem(i915_prandom_u64_state(&prng), rem, &size); + size = round_down(size, ps); + size = max(size, ps); + + obj = igt_object_create(mr, &objects, size, 0); + if (IS_ERR(obj)) { + pr_err("%s MAPPABLE failed with rem=%llx, size=%llx\n", + __func__, rem, size); + err = PTR_ERR(obj); + goto out_close; + } + + if (igt_object_mappable_total(obj) != size) { + pr_err("%s allocation is not mappable(size=%llx)\n", + __func__, size); + err = -EINVAL; + goto out_close; +
[Intel-gfx] [PATCH 04/20] drm/i915: add io_size plumbing
With small LMEM-BAR we need to be able to differentiate between the total size of LMEM, and how much of it is CPU mappable. The end goal is to be able to utilize the entire range, even if part of is it not CPU accessible. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 2 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 +--- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/gt/intel_region_lmem.c | 6 +- drivers/gpu/drm/i915/intel_memory_region.c | 6 +- drivers/gpu/drm/i915/intel_memory_region.h | 2 ++ drivers/gpu/drm/i915/selftests/intel_memory_region.c | 8 drivers/gpu/drm/i915/selftests/mock_region.c | 6 -- drivers/gpu/drm/i915/selftests/mock_region.h | 3 ++- 10 files changed, 30 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 6c57b0a79c8a..a9aca11cedbb 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -696,7 +696,7 @@ struct intel_memory_region *i915_gem_shmem_setup(struct drm_i915_private *i915, { return intel_memory_region_create(i915, 0, totalram_pages() << PAGE_SHIFT, - PAGE_SIZE, 0, + PAGE_SIZE, 0, 0, type, instance, &shmem_region_ops); } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index 26975d857776..387b48686851 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -490,6 +490,7 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem) /* Exclude the reserved region from driver use */ mem->region.end = reserved_base - 1; + mem->io_size = resource_size(&mem->region); /* It is possible for the reserved area to end before the end of stolen * memory, so just consider the start. */ @@ -746,7 +747,7 @@ static int init_stolen_lmem(struct intel_memory_region *mem) if (!io_mapping_init_wc(&mem->iomap, mem->io_start, - resource_size(&mem->region))) + mem->io_size)) return -EIO; /* @@ -801,7 +802,8 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type, I915_GTT_PAGE_SIZE_4K; mem = intel_memory_region_create(i915, lmem_base, lmem_size, -min_page_size, io_start, +min_page_size, +io_start, lmem_size, type, instance, &i915_region_stolen_lmem_ops); if (IS_ERR(mem)) @@ -832,7 +834,7 @@ i915_gem_stolen_smem_setup(struct drm_i915_private *i915, u16 type, mem = intel_memory_region_create(i915, intel_graphics_stolen_res.start, resource_size(&intel_graphics_stolen_res), -PAGE_SIZE, 0, type, instance, +PAGE_SIZE, 0, 0, type, instance, &i915_region_stolen_smem_ops); if (IS_ERR(mem)) return mem; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 84cae740b4a5..e1140ca3d9a0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1103,7 +1103,7 @@ i915_gem_ttm_system_setup(struct drm_i915_private *i915, mr = intel_memory_region_create(i915, 0, totalram_pages() << PAGE_SHIFT, - PAGE_SIZE, 0, + PAGE_SIZE, 0, 0, type, instance, &ttm_system_region_ops); if (IS_ERR(mr)) diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index f36191ebf964..42db9cd30978 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -499,7 +499,7 @@ static int igt_mock_memory_region_huge_pages(void *arg) int bit; int err = 0; - mem = mock_region_create(i915, 0, SZ_2G, I915_GTT_PAGE_SIZE_4K, 0); + mem = mock_region_create(i915, 0, SZ_2G, I915_GTT_PAGE_SIZE_4K, 0, 0); if (IS_ERR(mem)) {
[Intel-gfx] [PATCH 06/20] drm/i915: add I915_BO_ALLOC_TOPDOWN
If the user doesn't require CPU access for the buffer, then ALLOC_TOPDOWN should be used, in order to prioritise allocating in the non-mappable portion of LMEM. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 15 +++ drivers/gpu/drm/i915/gem/i915_gem_pages.c| 3 +++ drivers/gpu/drm/i915/gem/i915_gem_region.c | 5 + drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 13 ++--- drivers/gpu/drm/i915/gt/intel_gt.c | 4 +++- drivers/gpu/drm/i915/i915_vma.c | 3 +++ drivers/gpu/drm/i915/intel_region_ttm.c | 11 --- drivers/gpu/drm/i915/selftests/mock_region.c | 7 +-- 8 files changed, 44 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 71e778ecaeb8..29285aaf0477 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -319,15 +319,22 @@ struct drm_i915_gem_object { #define I915_BO_ALLOC_PM_VOLATILE BIT(4) /* Object needs to be restored early using memcpy during resume */ #define I915_BO_ALLOC_PM_EARLYBIT(5) +/* + * Object is likely never accessed by the CPU. This will prioritise the BO to be + * allocated in the non-mappable portion of lmem. This is merely a hint, and if + * dealing with userspace objects the CPU fault handler is free to ignore this. + */ +#define I915_BO_ALLOC_TOPDOWNBIT(6) #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \ I915_BO_ALLOC_VOLATILE | \ I915_BO_ALLOC_CPU_CLEAR | \ I915_BO_ALLOC_USER | \ I915_BO_ALLOC_PM_VOLATILE | \ -I915_BO_ALLOC_PM_EARLY) -#define I915_BO_READONLY BIT(6) -#define I915_TILING_QUIRK_BIT 7 /* unknown swizzling; do not release! */ -#define I915_BO_PROTECTED BIT(8) +I915_BO_ALLOC_PM_EARLY | \ +I915_BO_ALLOC_TOPDOWN) +#define I915_BO_READONLY BIT(7) +#define I915_TILING_QUIRK_BIT 8 /* unknown swizzling; do not release! */ +#define I915_BO_PROTECTED BIT(9) /** * @mem_flags - Mutable placement-related flags * diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index 7d2211fbe548..a95b4d72619f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -346,6 +346,9 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, !i915_gem_object_has_iomem(obj)) return ERR_PTR(-ENXIO); + if (WARN_ON_ONCE(obj->flags & I915_BO_ALLOC_TOPDOWN)) + return ERR_PTR(-EINVAL); + assert_object_held(obj); pinned = !(type & I915_MAP_OVERRIDE); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c b/drivers/gpu/drm/i915/gem/i915_gem_region.c index a4350227e9ae..f91e5a9c759d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_region.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c @@ -45,6 +45,11 @@ i915_gem_object_create_region(struct intel_memory_region *mem, GEM_BUG_ON(flags & ~I915_BO_ALLOC_FLAGS); + if (WARN_ON_ONCE(flags & I915_BO_ALLOC_TOPDOWN && +(flags & I915_BO_ALLOC_CPU_CLEAR || + flags & I915_BO_ALLOC_PM_EARLY))) + return ERR_PTR(-EINVAL); + if (!mem) return ERR_PTR(-ENODEV); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index d9a04c7d41b1..e60b677ecd54 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -127,10 +127,14 @@ i915_ttm_place_from_region(const struct intel_memory_region *mr, place->mem_type = intel_region_to_ttm_type(mr); if (flags & I915_BO_ALLOC_CONTIGUOUS) - place->flags = TTM_PL_FLAG_CONTIGUOUS; + place->flags |= TTM_PL_FLAG_CONTIGUOUS; if (mr->io_size && mr->io_size < mr->total) { - place->fpfn = 0; - place->lpfn = mr->io_size >> PAGE_SHIFT; + if (flags & I915_BO_ALLOC_TOPDOWN) { + place->flags |= TTM_PL_FLAG_TOPDOWN; + } else { + place->fpfn = 0; + place->lpfn = mr->io_size >> PAGE_SHIFT; + } } } @@ -890,6 +894,9 @@ static vm_fault_t vm_fault_ttm(struct vm_fault *vmf) if (!obj) return VM_FAULT_SIGBUS; + if (obj->flags & I915_BO_ALLOC_TOPDOWN) + return -EINVAL; + /* Sanity check that we allow writing into this object */ if (unlikely(i915_gem_object_is_readonly(obj) && area->vm_flags & VM_WRITE)) diff --git a/drive
[Intel-gfx] [PATCH 07/20] drm/i915/buddy: track available visible size
Track the total amount of available visible memory, and also track per-resource the amount of used visible memory. For now this is useful for our debug output, and deciding if it is even worth calling into the buddy allocator. In the future tracking the per-resource visible usage will be useful for when deciding if we should attempt to evict certain buffers. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 55 ++- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 8 ++- drivers/gpu/drm/i915/intel_region_ttm.c | 1 + 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index 53eb100688a6..6e5842155898 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -19,6 +19,8 @@ struct i915_ttm_buddy_manager { struct drm_buddy mm; struct list_head reserved; struct mutex lock; + unsigned long visible_size; + unsigned long visible_avail; u64 default_page_size; }; @@ -87,6 +89,13 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, n_pages = size >> ilog2(mm->chunk_size); mutex_lock(&bman->lock); + if (place->lpfn && place->lpfn <= bman->visible_size && + n_pages > bman->visible_avail) { + mutex_unlock(&bman->lock); + err = -ENOSPC; + goto err_free_res; + } + err = drm_buddy_alloc_blocks(mm, (u64)place->fpfn << PAGE_SHIFT, (u64)lpfn << PAGE_SHIFT, (u64)n_pages << PAGE_SHIFT, @@ -107,6 +116,30 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, mutex_unlock(&bman->lock); } + if (place->lpfn && place->lpfn <= bman->visible_size) { + bman_res->used_visible_size = bman_res->base.num_pages; + } else { + struct drm_buddy_block *block; + + list_for_each_entry(block, &bman_res->blocks, link) { + unsigned long start = + drm_buddy_block_offset(block) >> PAGE_SHIFT; + unsigned long end = start + + (drm_buddy_block_size(mm, block) >> PAGE_SHIFT); + + if (start < bman->visible_size) { + bman_res->used_visible_size += + min(end, bman->visible_size) - start; + } + } + } + + if (bman_res->used_visible_size) { + mutex_lock(&bman->lock); + bman->visible_avail -= bman_res->used_visible_size; + mutex_unlock(&bman->lock); + } + *res = &bman_res->base; return 0; @@ -127,6 +160,7 @@ static void i915_ttm_buddy_man_free(struct ttm_resource_manager *man, mutex_lock(&bman->lock); drm_buddy_free_list(&bman->mm, &bman_res->blocks); + bman->visible_avail += bman_res->used_visible_size; mutex_unlock(&bman->lock); kfree(bman_res); @@ -141,6 +175,10 @@ static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man, mutex_lock(&bman->lock); drm_printf(printer, "default_page_size: %lluKiB\n", bman->default_page_size >> 10); + drm_printf(printer, "visible_avail: %luMiB\n", + bman->visible_avail << PAGE_SHIFT >> 20); + drm_printf(printer, "visible_size: %luMiB\n", + bman->visible_size << PAGE_SHIFT >> 20); drm_buddy_print(&bman->mm, printer); @@ -162,6 +200,7 @@ static const struct ttm_resource_manager_func i915_ttm_buddy_manager_func = { * @type: Memory type we want to manage * @use_tt: Set use_tt for the manager * @size: The size in bytes to manage + * @visible_size: The CPU visible size in bytes to manage * @default_page_size: The default minimum page size in bytes for allocations, * this must be at least as large as @chunk_size, and can be overridden by * setting the BO page_alignment, to be larger or smaller as needed. @@ -185,7 +224,7 @@ static const struct ttm_resource_manager_func i915_ttm_buddy_manager_func = { */ int i915_ttm_buddy_man_init(struct ttm_device *bdev, unsigned int type, bool use_tt, - u64 size, u64 default_page_size, + u64 size, u64 visible_size, u64 default_page_size, u64 chunk_size) { struct ttm_resource_manager *man; @@ -204,6 +243,8 @@ int i915_ttm_buddy_man_init(struct ttm_device *bdev, INIT_LIST_HEAD(&bman->reserved); GEM_BUG_ON(default_page_size < chunk_size); bman->default_page_size = default_page_size; + bman->visible_size = visible_size >> PAGE_SHIFT; +
[Intel-gfx] [PATCH 05/20] drm/i915/ttm: require mappable by default
On devices with non-mappable LMEM ensure we always allocate the pages within the mappable portion. For now we assume that all LMEM buffers will require CPU access, which is also inline with pretty much all current kernel internal users. In the next patch we will introduce a new flag to override this behaviour. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4 drivers/gpu/drm/i915/intel_region_ttm.c | 5 + 2 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e1140ca3d9a0..d9a04c7d41b1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -128,6 +128,10 @@ i915_ttm_place_from_region(const struct intel_memory_region *mr, if (flags & I915_BO_ALLOC_CONTIGUOUS) place->flags = TTM_PL_FLAG_CONTIGUOUS; + if (mr->io_size && mr->io_size < mr->total) { + place->fpfn = 0; + place->lpfn = mr->io_size >> PAGE_SHIFT; + } } static void diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c index f2b888c16958..4689192d5e8d 100644 --- a/drivers/gpu/drm/i915/intel_region_ttm.c +++ b/drivers/gpu/drm/i915/intel_region_ttm.c @@ -199,6 +199,11 @@ intel_region_ttm_resource_alloc(struct intel_memory_region *mem, struct ttm_resource *res; int ret; + if (mem->io_size && mem->io_size < mem->total) { + place.fpfn = 0; + place.lpfn = mem->io_size >> PAGE_SHIFT; + } + mock_bo.base.size = size; place.flags = flags; -- 2.34.1
[Intel-gfx] [PATCH 03/20] drm: implement a method to free unused pages
From: Arunpravin On contiguous allocation, we round up the size to the *next* power of 2, implement a function to free the unused pages after the newly allocate block. v2(Matthew Auld): - replace function name 'drm_buddy_free_unused_pages' with drm_buddy_block_trim - replace input argument name 'actual_size' with 'new_size' - add more validation checks for input arguments - add overlaps check to avoid needless searching and splitting - merged the below patch to see the feature in action - add free unused pages support to i915 driver - lock drm_buddy_block_trim() function as it calls mark_free/mark_split are all globally visible v3(Matthew Auld): - remove trim method error handling as we address the failure case at drm_buddy_block_trim() function v4: - in case of trim, at __alloc_range() split_block failure path marks the block as free and removes it from the original list, potentially also freeing it, to overcome this problem, we turn the drm_buddy_block_trim() input node into a temporary node to prevent recursively freeing itself, but still retain the un-splitting/freeing of the other nodes(Matthew Auld) - modify the drm_buddy_block_trim() function return type v5(Matthew Auld): - revert drm_buddy_block_trim() function return type changes in v4 - modify drm_buddy_block_trim() passing argument n_pages to original_size as n_pages has already been rounded up to the next power-of-two and passing n_pages results noop v6: - fix warnings reported by kernel test robot Signed-off-by: Arunpravin --- drivers/gpu/drm/drm_buddy.c | 65 +++ drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 10 +++ include/drm/drm_buddy.h | 4 ++ 3 files changed, 79 insertions(+) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index 6aa5c1ce25bf..c5902a81b8c5 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -546,6 +546,71 @@ static int __drm_buddy_alloc_range(struct drm_buddy *mm, return __alloc_range(mm, &dfs, start, size, blocks); } +/** + * drm_buddy_block_trim - free unused pages + * + * @mm: DRM buddy manager + * @new_size: original size requested + * @blocks: output list head to add allocated blocks + * + * For contiguous allocation, we round up the size to the nearest + * power of two value, drivers consume *actual* size, so remaining + * portions are unused and it can be freed. + * + * Returns: + * 0 on success, error code on failure. + */ +int drm_buddy_block_trim(struct drm_buddy *mm, +u64 new_size, +struct list_head *blocks) +{ + struct drm_buddy_block *parent; + struct drm_buddy_block *block; + LIST_HEAD(dfs); + u64 new_start; + int err; + + if (!list_is_singular(blocks)) + return -EINVAL; + + block = list_first_entry(blocks, +struct drm_buddy_block, +link); + + if (!drm_buddy_block_is_allocated(block)) + return -EINVAL; + + if (new_size > drm_buddy_block_size(mm, block)) + return -EINVAL; + + if (!new_size && !IS_ALIGNED(new_size, mm->chunk_size)) + return -EINVAL; + + if (new_size == drm_buddy_block_size(mm, block)) + return 0; + + list_del(&block->link); + mark_free(mm, block); + mm->avail += drm_buddy_block_size(mm, block); + + /* Prevent recursively freeing this node */ + parent = block->parent; + block->parent = NULL; + + new_start = drm_buddy_block_offset(block); + list_add(&block->tmp_link, &dfs); + err = __alloc_range(mm, &dfs, new_start, new_size, blocks); + if (err) { + mark_allocated(block); + mm->avail -= drm_buddy_block_size(mm, block); + list_add(&block->link, blocks); + } + + block->parent = parent; + return err; +} +EXPORT_SYMBOL(drm_buddy_block_trim); + /** * drm_buddy_alloc_blocks - allocate power-of-two blocks * diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index 3662434b64bb..53eb100688a6 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -97,6 +97,16 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, if (unlikely(err)) goto err_free_blocks; + if (place->flags & TTM_PL_FLAG_CONTIGUOUS) { + u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT; + + mutex_lock(&bman->lock); + drm_buddy_block_trim(mm, +original_size, +&bman_res->blocks); + mutex_unlock(&bman->lock); + } + *res = &bman_res->base; return 0; diff
[Intel-gfx] [PATCH 02/20] drm: implement top-down allocation method
From: Arunpravin Implemented a function which walk through the order list, compares the offset and returns the maximum offset block, this method is unpredictable in obtaining the high range address blocks which depends on allocation and deallocation. for instance, if driver requests address at a low specific range, allocator traverses from the root block and splits the larger blocks until it reaches the specific block and in the process of splitting, lower orders in the freelist are occupied with low range address blocks and for the subsequent TOPDOWN memory request we may return the low range blocks.To overcome this issue, we may go with the below approach. The other approach, sorting each order list entries in ascending order and compares the last entry of each order list in the freelist and return the max block. This creates sorting overhead on every drm_buddy_free() request and split up of larger blocks for a single page request. v2: - Fix alignment issues(Matthew Auld) - Remove unnecessary list_empty check(Matthew Auld) - merged the below patch to see the feature in action - add top-down alloc support to i915 driver Signed-off-by: Arunpravin --- drivers/gpu/drm/drm_buddy.c | 36 --- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 3 ++ include/drm/drm_buddy.h | 1 + 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index 954e31962c74..6aa5c1ce25bf 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -371,6 +371,26 @@ alloc_range_bias(struct drm_buddy *mm, return ERR_PTR(err); } +static struct drm_buddy_block * +get_maxblock(struct list_head *head) +{ + struct drm_buddy_block *max_block = NULL, *node; + + max_block = list_first_entry_or_null(head, +struct drm_buddy_block, +link); + if (!max_block) + return NULL; + + list_for_each_entry(node, head, link) { + if (drm_buddy_block_offset(node) > + drm_buddy_block_offset(max_block)) + max_block = node; + } + + return max_block; +} + static struct drm_buddy_block * alloc_from_freelist(struct drm_buddy *mm, unsigned int order, @@ -381,11 +401,17 @@ alloc_from_freelist(struct drm_buddy *mm, int err; for (i = order; i <= mm->max_order; ++i) { - block = list_first_entry_or_null(&mm->free_list[i], -struct drm_buddy_block, -link); - if (block) - break; + if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) { + block = get_maxblock(&mm->free_list[i]); + if (block) + break; + } else { + block = list_first_entry_or_null(&mm->free_list[i], +struct drm_buddy_block, +link); + if (block) + break; + } } if (!block) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index 1411f4cf1f21..3662434b64bb 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -53,6 +53,9 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, INIT_LIST_HEAD(&bman_res->blocks); bman_res->mm = mm; + if (place->flags & TTM_PL_FLAG_TOPDOWN) + bman_res->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION; + if (place->fpfn || lpfn != man->size) bman_res->flags |= DRM_BUDDY_RANGE_ALLOCATION; diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h index 865664b90a8a..424fc443115e 100644 --- a/include/drm/drm_buddy.h +++ b/include/drm/drm_buddy.h @@ -28,6 +28,7 @@ }) #define DRM_BUDDY_RANGE_ALLOCATION (1 << 0) +#define DRM_BUDDY_TOPDOWN_ALLOCATION (1 << 1) struct drm_buddy_block { #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12) -- 2.34.1
[Intel-gfx] [PATCH 01/20] drm: improve drm_buddy_alloc function
From: Arunpravin - Make drm_buddy_alloc a single function to handle range allocation and non-range allocation demands - Implemented a new function alloc_range() which allocates the requested power-of-two block comply with range limitations - Moved order computation and memory alignment logic from i915 driver to drm buddy v2: merged below changes to keep the build unbroken - drm_buddy_alloc_range() becomes obsolete and may be removed - enable ttm range allocation (fpfn / lpfn) support in i915 driver - apply enhanced drm_buddy_alloc() function to i915 driver v3(Matthew Auld): - Fix alignment issues and remove unnecessary list_empty check - add more validation checks for input arguments - make alloc_range() block allocations as bottom-up - optimize order computation logic - replace uint64_t with u64, which is preferred in the kernel v4(Matthew Auld): - keep drm_buddy_alloc_range() function implementation for generic actual range allocations - keep alloc_range() implementation for end bias allocations v5(Matthew Auld): - modify drm_buddy_alloc() passing argument place->lpfn to lpfn as place->lpfn will currently always be zero for i915 v6(Matthew Auld): - fixup potential uaf - If we are unlucky and can't allocate enough memory when splitting blocks, where we temporarily end up with the given block and its buddy on the respective free list, then we need to ensure we delete both blocks, and no just the buddy, before potentially freeing them - fix warnings reported by kernel test robot Signed-off-by: Arunpravin --- drivers/gpu/drm/drm_buddy.c | 326 +- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 67 ++-- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 2 + include/drm/drm_buddy.h | 22 +- 4 files changed, 293 insertions(+), 124 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index d60878bc9c20..954e31962c74 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -282,23 +282,99 @@ void drm_buddy_free_list(struct drm_buddy *mm, struct list_head *objects) } EXPORT_SYMBOL(drm_buddy_free_list); -/** - * drm_buddy_alloc_blocks - allocate power-of-two blocks - * - * @mm: DRM buddy manager to allocate from - * @order: size of the allocation - * - * The order value here translates to: - * - * 0 = 2^0 * mm->chunk_size - * 1 = 2^1 * mm->chunk_size - * 2 = 2^2 * mm->chunk_size - * - * Returns: - * allocated ptr to the &drm_buddy_block on success - */ -struct drm_buddy_block * -drm_buddy_alloc_blocks(struct drm_buddy *mm, unsigned int order) +static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2) +{ + return s1 <= e2 && e1 >= s2; +} + +static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2) +{ + return s1 <= s2 && e1 >= e2; +} + +static struct drm_buddy_block * +alloc_range_bias(struct drm_buddy *mm, +u64 start, u64 end, +unsigned int order) +{ + struct drm_buddy_block *block; + struct drm_buddy_block *buddy; + LIST_HEAD(dfs); + int err; + int i; + + end = end - 1; + + for (i = 0; i < mm->n_roots; ++i) + list_add_tail(&mm->roots[i]->tmp_link, &dfs); + + do { + u64 block_start; + u64 block_end; + + block = list_first_entry_or_null(&dfs, +struct drm_buddy_block, +tmp_link); + if (!block) + break; + + list_del(&block->tmp_link); + + if (drm_buddy_block_order(block) < order) + continue; + + block_start = drm_buddy_block_offset(block); + block_end = block_start + drm_buddy_block_size(mm, block) - 1; + + if (!overlaps(start, end, block_start, block_end)) + continue; + + if (drm_buddy_block_is_allocated(block)) + continue; + + if (contains(start, end, block_start, block_end) && + order == drm_buddy_block_order(block)) { + /* +* Find the free block within the range. +*/ + if (drm_buddy_block_is_free(block)) + return block; + + continue; + } + + if (!drm_buddy_block_is_split(block)) { + err = split_block(mm, block); + if (unlikely(err)) + goto err_undo; + } + + list_add(&block->right->tmp_link, &dfs); + list_add(&block->left->tmp_link, &dfs); + } while (1); + + return ERR_PTR(-ENOSPC); + +err_undo: + /* +* We really don't want to leave around a bunch of split blocks
[Intel-gfx] [PATCH 00/20] Initial support for small BAR recovery
Starting from DG2 we will have resizable BAR support for device local-memory, but in some cases the final BAR size might still be smaller than the total local-memory size. In such cases only part of local-memory will be CPU accessible, while the remainder is only accessible via the GPU. This series adds the basic enablers needed to ensure that the entire local-memory range is usable. Patches 1-3 are taken directly from Arun' in-progress series[1], which reworks part of the allocator, and for example, allows us to allocate memory within a sub-range, and is needed when allocating mappable memory. These patches are only included here for the benefit of CI testing. [1] https://patchwork.freedesktop.org/series/98979/ -- 2.34.1
Re: [Intel-gfx] [PATCH v2 09/11] drm: Convert open-coded yes/no strings to yesno()
On Wed, 26 Jan 2022, Lucas De Marchi wrote: > linux/string_helpers.h provides a helper to return "yes"/"no" strings. > Replace the open coded versions with str_yes_no(). The places were > identified with the following semantic patch: > > @@ > expression b; > @@ > > - b ? "yes" : "no" > + str_yes_no(b) > > Then the includes were added, so we include-what-we-use, and parenthesis > adjusted in drivers/gpu/drm/v3d/v3d_debugfs.c. After the conversion we > still see the same binary sizes: > >textdata bss dec hex filename > 511493295 212 54656d580 virtio/virtio-gpu.ko.old > 511493295 212 54656d580 virtio/virtio-gpu.ko > 1441491 60340 800 1502631 16eda7 radeon/radeon.ko.old > 1441491 60340 800 1502631 16eda7 radeon/radeon.ko > 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko.old > 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko > 411986 104906176 428652 68a6c drm.ko.old > 411986 104906176 428652 68a6c drm.ko > 981291636 264 100029 186bd dp/drm_dp_helper.ko.old > 981291636 264 100029 186bd dp/drm_dp_helper.ko > 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko.old > 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko > > Signed-off-by: Lucas De Marchi Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/amd/amdgpu/atom.c | 4 +++- > drivers/gpu/drm/dp/drm_dp.c | 3 ++- > drivers/gpu/drm/drm_client_modeset.c | 3 ++- > drivers/gpu/drm/drm_gem.c | 3 ++- > drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 5 - > drivers/gpu/drm/radeon/atom.c | 3 ++- > drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++- > drivers/gpu/drm/virtio/virtgpu_debugfs.c | 4 +++- > 8 files changed, 24 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c > b/drivers/gpu/drm/amd/amdgpu/atom.c > index 6fa2229b7229..1c5d9388ad0b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atom.c > +++ b/drivers/gpu/drm/amd/amdgpu/atom.c > @@ -25,6 +25,8 @@ > #include > #include > #include > +#include > + > #include > > #include > @@ -740,7 +742,7 @@ static void atom_op_jump(atom_exec_context *ctx, int > *ptr, int arg) > break; > } > if (arg != ATOM_COND_ALWAYS) > - SDEBUG(" taken: %s\n", execute ? "yes" : "no"); > + SDEBUG(" taken: %s\n", str_yes_no(execute)); > SDEBUG(" target: 0x%04X\n", target); > if (execute) { > if (ctx->last_jump == (ctx->start + target)) { > diff --git a/drivers/gpu/drm/dp/drm_dp.c b/drivers/gpu/drm/dp/drm_dp.c > index 6d43325acca5..c43577c8ac4d 100644 > --- a/drivers/gpu/drm/dp/drm_dp.c > +++ b/drivers/gpu/drm/dp/drm_dp.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1239,7 +1240,7 @@ void drm_dp_downstream_debug(struct seq_file *m, > bool branch_device = drm_dp_is_branch(dpcd); > > seq_printf(m, "\tDP branch device present: %s\n", > -branch_device ? "yes" : "no"); > +str_yes_no(branch_device)); > > if (!branch_device) > return; > diff --git a/drivers/gpu/drm/drm_client_modeset.c > b/drivers/gpu/drm/drm_client_modeset.c > index ced09c7c06f9..e6346a67cd98 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -241,7 +242,7 @@ static void drm_client_connectors_enabled(struct > drm_connector **connectors, > connector = connectors[i]; > enabled[i] = drm_connector_enabled(connector, true); > DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id, > - connector->display_info.non_desktop ? "non > desktop" : enabled[i] ? "yes" : "no"); > + connector->display_info.non_desktop ? "non > desktop" : str_yes_no(enabled[i])); > > any_enabled |= enabled[i]; > } > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 21631c22b374..3c888db59ea4 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -37,6 +37,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -1145,7 +1146,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned > int indent, > drm_vma_node_start(&obj->vma_node)); > drm_printf_indent(p, indent, "size=%zu\n", obj->size); > drm_printf_indent(p, indent, "imported=%s\n", > - obj->import_attach ? "yes" : "no"); > + str_yes_no(obj->import_attach)); > > if (obj->funcs->print_info) > obj->funcs->print_info(p, indent, obj); > diff --git a/drivers/gpu/drm/no
Re: [Intel-gfx] [PATCH v2 08/11] drm/gem: Sort includes alphabetically
On Wed, 26 Jan 2022, Lucas De Marchi wrote: > Sort includes alphabetically so it's easier to add/remove includes and > know when that is needed. > > Signed-off-by: Lucas De Marchi Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/drm_gem.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 4dcdec6487bb..21631c22b374 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -25,20 +25,20 @@ > * > */ > > -#include > -#include > -#include > -#include > -#include > +#include > +#include > #include > -#include > +#include > +#include > +#include > #include > +#include > #include > -#include > -#include > -#include > -#include > #include > +#include > +#include > +#include > +#include > > #include > #include -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2 02/11] drm/i915: Fix trailing semicolon
On Wed, 26 Jan 2022, Lucas De Marchi wrote: > Remove the trailing semicolon, as correctly warned by checkpatch: > > -:1189: WARNING:TRAILING_SEMICOLON: macros should not use a trailing > semicolon > #1189: FILE: drivers/gpu/drm/i915/intel_device_info.c:119: > +#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, > yesno(info->display.name)); > > Signed-off-by: Lucas De Marchi Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_device_info.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c > b/drivers/gpu/drm/i915/intel_device_info.c > index 93b251b25aba..94da5aa37391 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -114,7 +114,7 @@ void intel_device_info_print_static(const struct > intel_device_info *info, > DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG); > #undef PRINT_FLAG > > -#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, > yesno(info->display.name)); > +#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, > yesno(info->display.name)) > DEV_INFO_DISPLAY_FOR_EACH_FLAG(PRINT_FLAG); > #undef PRINT_FLAG > } -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up pixel_rate vs. clock confusion in wm calculations
On Thu, 09 Dec 2021, Ville Syrjala wrote: > From: Ville Syrjälä > > Use pixel_rate rather than crtc_clock in the watermark calculations. > These are actually identical on gmch platforms for now since > we don't adjust the pixel rate based on pfit downscaling. But > pixel_rate is the thing we are actually interested here so use > the proper name for it. > > Signed-off-by: Ville Syrjälä On the series, Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_pm.c | 52 ++--- > 1 file changed, 22 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 434b1f8b7fe3..b5d5b625a321 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -915,15 +915,13 @@ static void pnv_update_wm(struct drm_i915_private > *dev_priv) > > crtc = single_enabled_crtc(dev_priv); > if (crtc) { > - const struct drm_display_mode *pipe_mode = > - &crtc->config->hw.pipe_mode; > const struct drm_framebuffer *fb = > crtc->base.primary->state->fb; > + int pixel_rate = crtc->config->pixel_rate; > int cpp = fb->format->cpp[0]; > - int clock = pipe_mode->crtc_clock; > > /* Display SR */ > - wm = intel_calculate_wm(clock, &pnv_display_wm, > + wm = intel_calculate_wm(pixel_rate, &pnv_display_wm, > pnv_display_wm.fifo_size, > cpp, latency->display_sr); > reg = intel_uncore_read(&dev_priv->uncore, DSPFW1); > @@ -933,7 +931,7 @@ static void pnv_update_wm(struct drm_i915_private > *dev_priv) > drm_dbg_kms(&dev_priv->drm, "DSPFW1 register is %x\n", reg); > > /* cursor SR */ > - wm = intel_calculate_wm(clock, &pnv_cursor_wm, > + wm = intel_calculate_wm(pixel_rate, &pnv_cursor_wm, > pnv_display_wm.fifo_size, > 4, latency->cursor_sr); > reg = intel_uncore_read(&dev_priv->uncore, DSPFW3); > @@ -942,7 +940,7 @@ static void pnv_update_wm(struct drm_i915_private > *dev_priv) > intel_uncore_write(&dev_priv->uncore, DSPFW3, reg); > > /* Display HPLL off SR */ > - wm = intel_calculate_wm(clock, &pnv_display_hplloff_wm, > + wm = intel_calculate_wm(pixel_rate, &pnv_display_hplloff_wm, > pnv_display_hplloff_wm.fifo_size, > cpp, latency->display_hpll_disable); > reg = intel_uncore_read(&dev_priv->uncore, DSPFW3); > @@ -951,7 +949,7 @@ static void pnv_update_wm(struct drm_i915_private > *dev_priv) > intel_uncore_write(&dev_priv->uncore, DSPFW3, reg); > > /* cursor HPLL off SR */ > - wm = intel_calculate_wm(clock, &pnv_cursor_hplloff_wm, > + wm = intel_calculate_wm(pixel_rate, &pnv_cursor_hplloff_wm, > pnv_display_hplloff_wm.fifo_size, > 4, latency->cursor_hpll_disable); > reg = intel_uncore_read(&dev_priv->uncore, DSPFW3); > @@ -1154,7 +1152,7 @@ static u16 g4x_compute_wm(const struct intel_crtc_state > *crtc_state, > const struct drm_display_mode *pipe_mode = > &crtc_state->hw.pipe_mode; > unsigned int latency = dev_priv->wm.pri_latency[level] * 10; > - unsigned int clock, htotal, cpp, width, wm; > + unsigned int pixel_rate, htotal, cpp, width, wm; > > if (latency == 0) > return USHRT_MAX; > @@ -1175,21 +1173,21 @@ static u16 g4x_compute_wm(const struct > intel_crtc_state *crtc_state, > level != G4X_WM_LEVEL_NORMAL) > cpp = max(cpp, 4u); > > - clock = pipe_mode->crtc_clock; > + pixel_rate = crtc_state->pixel_rate; > htotal = pipe_mode->crtc_htotal; > > width = drm_rect_width(&plane_state->uapi.dst); > > if (plane->id == PLANE_CURSOR) { > - wm = intel_wm_method2(clock, htotal, width, cpp, latency); > + wm = intel_wm_method2(pixel_rate, htotal, width, cpp, latency); > } else if (plane->id == PLANE_PRIMARY && > level == G4X_WM_LEVEL_NORMAL) { > - wm = intel_wm_method1(clock, cpp, latency); > + wm = intel_wm_method1(pixel_rate, cpp, latency); > } else { > unsigned int small, large; > > - small = intel_wm_method1(clock, cpp, latency); > - large = intel_wm_method2(clock, htotal, width, cpp, latency); > + small = intel_wm_method1(pixel_rate, cpp, latency); > + large = intel_wm_method2(pixel_rate, htotal, width, cpp, > latency); > > wm = min(small, large); > } > @@ -1674,7 +1672,7 @@ stati
Re: [Intel-gfx] [PATCH 6/9] drm/i915: Clean up PIPESRC defines
On Fri, 12 Nov 2021, Ville Syrjala wrote: > From: Ville Syrjälä > > Use REG_GENMASK() & co. when dealing with PIPESRC. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/display/i9xx_plane.c| 4 ++-- > drivers/gpu/drm/i915/display/intel_display.c | 7 --- > drivers/gpu/drm/i915/i915_reg.h | 4 > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c > b/drivers/gpu/drm/i915/display/i9xx_plane.c > index 2194f74101ae..f586e39cb378 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c > @@ -1048,8 +1048,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, > plane_config->base = base; > > val = intel_de_read(dev_priv, PIPESRC(pipe)); > - fb->width = ((val >> 16) & 0xfff) + 1; > - fb->height = ((val >> 0) & 0xfff) + 1; I guess the mask width change is worth noting in the commit message. Reviewed-by: Jani Nikula > + fb->width = REG_FIELD_GET(PIPESRC_WIDTH_MASK, val) + 1; > + fb->height = REG_FIELD_GET(PIPESRC_HEIGHT_MASK, val) + 1; > > val = intel_de_read(dev_priv, DSPSTRIDE(i9xx_plane)); > fb->pitches[0] = val & 0xffc0; > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 4e29032b29d6..e1959a17805c 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3236,7 +3236,8 @@ static void intel_set_pipe_src_size(const struct > intel_crtc_state *crtc_state) >* always be the user's requested size. >*/ > intel_de_write(dev_priv, PIPESRC(pipe), > -((crtc_state->pipe_src_w - 1) << 16) | > (crtc_state->pipe_src_h - 1)); > +PIPESRC_WIDTH(crtc_state->pipe_src_w - 1) | > +PIPESRC_HEIGHT(crtc_state->pipe_src_h - 1)); > } > > static bool intel_pipe_is_interlaced(const struct intel_crtc_state > *crtc_state) > @@ -3307,8 +3308,8 @@ static void intel_get_pipe_src_size(struct intel_crtc > *crtc, > u32 tmp; > > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe)); > - pipe_config->pipe_src_h = (tmp & 0x) + 1; > - pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1; > + pipe_config->pipe_src_w = REG_FIELD_GET(PIPESRC_WIDTH_MASK, tmp) + 1; > + pipe_config->pipe_src_h = REG_FIELD_GET(PIPESRC_HEIGHT_MASK, tmp) + 1; > } > > static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state) > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index eea009e76e15..211e2b415e50 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4476,6 +4476,10 @@ enum { > #define _VSYNC_A 0x60014 > #define _EXITLINE_A 0x60018 > #define _PIPEASRC0x6001c > +#define PIPESRC_WIDTH_MASK REG_GENMASK(31, 16) > +#define PIPESRC_WIDTH(w) REG_FIELD_PREP(PIPESRC_WIDTH_MASK, (w)) > +#define PIPESRC_HEIGHT_MASKREG_GENMASK(15, 0) > +#define PIPESRC_HEIGHT(h) REG_FIELD_PREP(PIPESRC_HEIGHT_MASK, (h)) > #define _BCLRPAT_A 0x60020 > #define _VSYNCSHIFT_A0x60028 > #define _PIPE_MULT_A 0x6002c -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 5/9] drm/i915: Clean up PCH_TRANSCONF/TRANS_DP_CTL bit defines
On Fri, 12 Nov 2021, Ville Syrjala wrote: > From: Ville Syrjälä > > Use REG_BIT & co. for PCH_TRANSCONF/TRANS_DP_CTL bits, and > adjust the naming a some bits to be more consistent. > > Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula > --- > .../gpu/drm/i915/display/intel_pch_display.c | 13 +++-- > drivers/gpu/drm/i915/i915_reg.h | 58 +-- > 2 files changed, 33 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_pch_display.c > b/drivers/gpu/drm/i915/display/intel_pch_display.c > index 81ab761251ae..155c2d19a6bb 100644 > --- a/drivers/gpu/drm/i915/display/intel_pch_display.c > +++ b/drivers/gpu/drm/i915/display/intel_pch_display.c > @@ -166,11 +166,11 @@ static void ilk_enable_pch_transcoder(const struct > intel_crtc_state *crtc_state) > if ((pipeconf_val & PIPECONF_INTERLACE_MASK_ILK) == > PIPECONF_INTERLACE_IF_ID_ILK) { > if (HAS_PCH_IBX(dev_priv) && > intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO)) > - val |= TRANS_LEGACY_INTERLACED_ILK; > + val |= TRANS_INTERLACE_LEGACY_VSYNC_IBX; > else > - val |= TRANS_INTERLACED; > + val |= TRANS_INTERLACE_INTERLACED; > } else { > - val |= TRANS_PROGRESSIVE; > + val |= TRANS_INTERLACE_PROGRESSIVE; > } > > intel_de_write(dev_priv, reg, val | TRANS_ENABLE); > @@ -279,7 +279,8 @@ void ilk_pch_enable(struct intel_atomic_state *state, > > temp = intel_de_read(dev_priv, reg); > temp &= ~(TRANS_DP_PORT_SEL_MASK | > - TRANS_DP_SYNC_MASK | > + TRANS_DP_VSYNC_ACTIVE_HIGH | > + TRANS_DP_HSYNC_ACTIVE_HIGH | > TRANS_DP_BPC_MASK); > temp |= TRANS_DP_OUTPUT_ENABLE; > temp |= bpc << 9; /* same format but at 11:9 */ > @@ -423,9 +424,9 @@ static void lpt_enable_pch_transcoder(struct > drm_i915_private *dev_priv, > pipeconf_val = intel_de_read(dev_priv, PIPECONF(cpu_transcoder)); > > if ((pipeconf_val & PIPECONF_INTERLACE_MASK_HSW) == > PIPECONF_INTERLACE_IF_ID_ILK) > - val |= TRANS_INTERLACED; > + val |= TRANS_INTERLACE_INTERLACED; > else > - val |= TRANS_PROGRESSIVE; > + val |= TRANS_INTERLACE_PROGRESSIVE; > > intel_de_write(dev_priv, LPT_TRANSCONF, val); > if (intel_de_wait_for_set(dev_priv, LPT_TRANSCONF, > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d2d5b2fa2a4a..eea009e76e15 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8994,22 +8994,19 @@ enum { > #define _PCH_TRANSBCONF 0xf1008 > #define PCH_TRANSCONF(pipe) _MMIO_PIPE(pipe, _PCH_TRANSACONF, > _PCH_TRANSBCONF) > #define LPT_TRANSCONFPCH_TRANSCONF(PIPE_A) /* lpt has only > one transcoder */ > -#define TRANS_DISABLE (0 << 31) > -#define TRANS_ENABLE (1 << 31) > -#define TRANS_STATE_MASK (1 << 30) > -#define TRANS_STATE_DISABLE(0 << 30) > -#define TRANS_STATE_ENABLE (1 << 30) > -#define TRANS_FRAME_START_DELAY_MASK(3 << 27) /* ibx */ > -#define TRANS_FRAME_START_DELAY(x) ((x) << 27) /* ibx: 0-3 */ > -#define TRANS_INTERLACE_MASK (7 << 21) > -#define TRANS_PROGRESSIVE (0 << 21) > -#define TRANS_INTERLACED (3 << 21) > -#define TRANS_LEGACY_INTERLACED_ILK (2 << 21) > -#define TRANS_8BPC (0 << 5) > -#define TRANS_10BPC(1 << 5) > -#define TRANS_6BPC (2 << 5) > -#define TRANS_12BPC(3 << 5) > - > +#define TRANS_ENABLEREG_BIT(31) > +#define TRANS_STATE_ENABLE REG_BIT(30) > +#define TRANS_FRAME_START_DELAY_MASKREG_GENMASK(28, 27) /* ibx */ > +#define TRANS_FRAME_START_DELAY(x) > REG_FIELD_PREP(TRANS_FRAME_START_DELAY_MASK, (x)) /* ibx: 0-3 */ > +#define TRANS_INTERLACE_MASKREG_GENMASK(23, 21) > +#define TRANS_INTERLACE_PROGRESSIVE REG_FIELD_PREP(TRANS_INTERLACE_MASK, 0) > +#define TRANS_INTERLACE_LEGACY_VSYNC_IBX > REG_FIELD_PREP(TRANS_INTERLACE_MASK, 2) /* ibx */ > +#define TRANS_INTERLACE_INTERLACED REG_FIELD_PREP(TRANS_INTERLACE_MASK, 3) > +#define TRANS_BPC_MASK REG_GENMASK(7, 5) /* ibx */ > +#define TRANS_BPC_8 REG_FIELD_PREP(TRANS_BPC_MASK, 0) > +#define TRANS_BPC_10REG_FIELD_PREP(TRANS_BPC_MASK, > 1) > +#define TRANS_BPC_6 REG_FIELD_PREP(TRANS_BPC_MASK, 2) > +#define TRANS_BPC_12REG_FIELD_PREP(TRANS_BPC_MASK, > 3) > #define _TRANSA_CHICKEN1 0xf0060 > #define _TRANSB_CHICKEN1 0xf1060 > #define TRANS_CHICKEN1(pipe) _MMIO_PIPE(pipe, _TRANSA_CHICKEN1, > _TRANSB_CHICKEN1) > @@ -9219,22 +9216,19 @@ enum { > #define _TRANS_DP_CT
Re: [Intel-gfx] [PATCH 2/9] drm/i915: Clean up PIPEMISC register defines
On Fri, 12 Nov 2021, Ville Syrjala wrote: > From: Ville Syrjälä > > Use REG_BIT() & co. for PIPEMISC* bits, and while at it > fill in the missing dithering bits since we already had some > of them defined. > > Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/display/intel_display.c | 18 +- > drivers/gpu/drm/i915/i915_reg.h | 35 +++- > 2 files changed, 28 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 6073f94632ab..e293241450b1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3724,18 +3724,18 @@ static void bdw_set_pipemisc(const struct > intel_crtc_state *crtc_state) > > switch (crtc_state->pipe_bpp) { > case 18: > - val |= PIPEMISC_6_BPC; > + val |= PIPEMISC_BPC_6; > break; > case 24: > - val |= PIPEMISC_8_BPC; > + val |= PIPEMISC_BPC_8; > break; > case 30: > - val |= PIPEMISC_10_BPC; > + val |= PIPEMISC_BPC_10; > break; > case 36: > /* Port output 12BPC defined for ADLP+ */ > if (DISPLAY_VER(dev_priv) > 12) > - val |= PIPEMISC_12_BPC_ADLP; > + val |= PIPEMISC_BPC_12_ADLP; > break; > default: > MISSING_CASE(crtc_state->pipe_bpp); > @@ -3771,7 +3771,7 @@ static void bdw_set_pipemisc(const struct > intel_crtc_state *crtc_state) > } > > intel_de_rmw(dev_priv, PIPE_MISC2(crtc->pipe), > - PIPE_MISC2_UNDERRUN_BUBBLE_COUNTER_MASK, > + PIPE_MISC2_BUBBLE_COUNTER_MASK, >scaler_in_use ? > PIPE_MISC2_BUBBLE_COUNTER_SCALER_EN : >PIPE_MISC2_BUBBLE_COUNTER_SCALER_DIS); > } > @@ -3787,11 +3787,11 @@ int bdw_get_pipemisc_bpp(struct intel_crtc *crtc) > tmp = intel_de_read(dev_priv, PIPEMISC(crtc->pipe)); > > switch (tmp & PIPEMISC_BPC_MASK) { > - case PIPEMISC_6_BPC: > + case PIPEMISC_BPC_6: > return 18; > - case PIPEMISC_8_BPC: > + case PIPEMISC_BPC_8: > return 24; > - case PIPEMISC_10_BPC: > + case PIPEMISC_BPC_10: > return 30; > /* >* PORT OUTPUT 12 BPC defined for ADLP+. > @@ -3803,7 +3803,7 @@ int bdw_get_pipemisc_bpp(struct intel_crtc *crtc) >* on older platforms, need to find a workaround for 12 BPC >* MIPI DSI HW readout. >*/ > - case PIPEMISC_12_BPC_ADLP: > + case PIPEMISC_BPC_12_ADLP: > if (DISPLAY_VER(dev_priv) > 12) > return 36; > fallthrough; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f5d54ed2efc1..e300a202ce2d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6308,32 +6308,35 @@ enum { > > #define _PIPE_MISC_A 0x70030 > #define _PIPE_MISC_B 0x71030 > -#define PIPEMISC_YUV420_ENABLE (1 << 27) /* glk+ */ > -#define PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26) /* glk+ */ > -#define PIPEMISC_HDR_MODE_PRECISION(1 << 23) /* icl+ */ > -#define PIPEMISC_OUTPUT_COLORSPACE_YUV (1 << 11) > -#define PIPEMISC_PIXEL_ROUNDING_TRUNC REG_BIT(8) /* tgl+ */ > +#define PIPEMISC_YUV420_ENABLE REG_BIT(27) /* glk+ */ > +#define PIPEMISC_YUV420_MODE_FULL_BLENDREG_BIT(26) /* glk+ */ > +#define PIPEMISC_HDR_MODE_PRECISIONREG_BIT(23) /* icl+ */ > +#define PIPEMISC_OUTPUT_COLORSPACE_YUV REG_BIT(11) > +#define PIPEMISC_PIXEL_ROUNDING_TRUNC REG_BIT(8) /* tgl+ */ > /* > * For Display < 13, Bits 5-7 of PIPE MISC represent DITHER BPC with > * valid values of: 6, 8, 10 BPC. > * ADLP+, the bits 5-7 represent PORT OUTPUT BPC with valid values of: > * 6, 8, 10, 12 BPC. > */ > -#define PIPEMISC_BPC_MASK (7 << 5) > -#define PIPEMISC_8_BPC (0 << 5) > -#define PIPEMISC_10_BPC(1 << 5) > -#define PIPEMISC_6_BPC (2 << 5) > -#define PIPEMISC_12_BPC_ADLP (4 << 5) /* adlp+ */ > -#define PIPEMISC_DITHER_ENABLE (1 << 4) > -#define PIPEMISC_DITHER_TYPE_MASK (3 << 2) > -#define PIPEMISC_DITHER_TYPE_SP(0 << 2) > +#define PIPEMISC_BPC_MASK REG_GENMASK(7, 5) > +#define PIPEMISC_BPC_8 > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 0) > +#define PIPEMISC_BPC_10 > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 1) > +#define PIPEMISC_BPC_6 > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 2) > +#define PIPEMISC_BPC_12_ADLP > REG_FIELD_PREP(PIPEMISC_BPC_MASK, 4) /* adlp+ */ > +#define PI
Re: [Intel-gfx] [PATCH 1/9] drm/i915: Bump DSL linemask to 20 bits
On Fri, 19 Nov 2021, Ville Syrjälä wrote: > On Mon, Nov 15, 2021 at 02:05:00PM -0500, Rodrigo Vivi wrote: >> On Fri, Nov 12, 2021 at 09:38:05PM +0200, Ville Syrjala wrote: >> > From: Ville Syrjälä >> > >> > Since tgl PIPE_DSL has 20 bits for the scanline. Let's bump our >> > definition to match. And while at it let's also add the define >> > for the current field readback. >> > >> > We can also get rid of the gen2 vs. gen3+ nonsense since none >> > of the extra bits ever did anything and just always read >> > as zero. >> >> You are stepping over reserved bits on older platforms here. >> >> I understand that must probably hw is not using this for anything >> and the reads are only zero. But I'm always afraid of opening >> precedence for this kind of assumptions and end up stepping >> over some reserved bit that hw is using for something else >> but not documented. > > We do this in other places too in order to keep the code > simple. I think it's fine for cases where all old platforms > had a smaller bitfield which is extended in later platforms. > That is, assuming all the bits were unused (and read as zero) > in the old platforms, which is the case here. > > The thing we probably shouldn't do is make the bitfield larger > proactively for future platforms since we can't know if some of > the currently unused bits might end up being used for something > else in the future. > > I really hope we don't have any undocumented bits anywhere since > those can screw us up in a lot more ways than this. If we do find > any undocuemnted bits we really need to file bspec issues for those. I guess I'd record some of this in the commit message while applying, in case this blows up. Other than that, Reviewed-by: Jani Nikula -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 4/9] drm/i915: Clean up PIPECONF bit defines
On Fri, 12 Nov 2021, Ville Syrjala wrote: > From: Ville Syrjälä > > Use REG_BIT() & co. for PIPECONF bits, and adjust the > naming of various bits to be more consistent. > > Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/display/icl_dsi.c| 4 +- > drivers/gpu/drm/i915/display/intel_display.c | 60 +- > .../gpu/drm/i915/display/intel_pch_display.c | 7 +- > drivers/gpu/drm/i915/gvt/display.c| 4 +- > drivers/gpu/drm/i915/gvt/handlers.c | 4 +- > drivers/gpu/drm/i915/i915_reg.h | 108 +- > 6 files changed, 89 insertions(+), 98 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c > b/drivers/gpu/drm/i915/display/icl_dsi.c > index c05fb861f10c..0f6587bef106 100644 > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > @@ -1048,7 +1048,7 @@ static void gen11_dsi_enable_transcoder(struct > intel_encoder *encoder) > > /* wait for transcoder to be enabled */ > if (intel_de_wait_for_set(dev_priv, PIPECONF(dsi_trans), > - I965_PIPECONF_ACTIVE, 10)) > + PIPECONF_STATE_ENABLE, 10)) > drm_err(&dev_priv->drm, > "DSI transcoder not enabled\n"); > } > @@ -1317,7 +1317,7 @@ static void gen11_dsi_disable_transcoder(struct > intel_encoder *encoder) > > /* wait for transcoder to be disabled */ > if (intel_de_wait_for_clear(dev_priv, PIPECONF(dsi_trans), > - I965_PIPECONF_ACTIVE, 50)) > + PIPECONF_STATE_ENABLE, 50)) > drm_err(&dev_priv->drm, > "DSI trancoder not disabled\n"); > } > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index e293241450b1..4e29032b29d6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -386,13 +386,11 @@ intel_wait_for_pipe_off(const struct intel_crtc_state > *old_crtc_state) > > if (DISPLAY_VER(dev_priv) >= 4) { > enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder; > - i915_reg_t reg = PIPECONF(cpu_transcoder); > > /* Wait for the Pipe State to go off */ > - if (intel_de_wait_for_clear(dev_priv, reg, > - I965_PIPECONF_ACTIVE, 100)) > - drm_WARN(&dev_priv->drm, 1, > - "pipe_off wait timed out\n"); > + if (intel_de_wait_for_clear(dev_priv, PIPECONF(cpu_transcoder), > + PIPECONF_STATE_ENABLE, 100)) > + drm_WARN(&dev_priv->drm, 1, "pipe_off wait timed > out\n"); > } else { > intel_wait_for_pipe_scanline_stopped(crtc); > } > @@ -3338,13 +3336,13 @@ static void i9xx_set_pipeconf(const struct > intel_crtc_state *crtc_state) > > switch (crtc_state->pipe_bpp) { > case 18: > - pipeconf |= PIPECONF_6BPC; > + pipeconf |= PIPECONF_BPC_6; > break; > case 24: > - pipeconf |= PIPECONF_8BPC; > + pipeconf |= PIPECONF_BPC_8; > break; > case 30: > - pipeconf |= PIPECONF_10BPC; > + pipeconf |= PIPECONF_BPC_10; > break; > default: > /* Case prevented by intel_choose_pipe_bpp_dither. */ > @@ -3359,7 +3357,7 @@ static void i9xx_set_pipeconf(const struct > intel_crtc_state *crtc_state) > else > pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT; > } else { > - pipeconf |= PIPECONF_PROGRESSIVE; > + pipeconf |= PIPECONF_INTERLACE_PROGRESSIVE; > } > > if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) && > @@ -3537,16 +3535,17 @@ static bool i9xx_get_pipe_config(struct intel_crtc > *crtc, > if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) || > IS_CHERRYVIEW(dev_priv)) { > switch (tmp & PIPECONF_BPC_MASK) { > - case PIPECONF_6BPC: > + case PIPECONF_BPC_6: > pipe_config->pipe_bpp = 18; > break; > - case PIPECONF_8BPC: > + case PIPECONF_BPC_8: > pipe_config->pipe_bpp = 24; > break; > - case PIPECONF_10BPC: > + case PIPECONF_BPC_10: > pipe_config->pipe_bpp = 30; > break; > default: > + MISSING_CASE(tmp); >
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Clean up SKL_BOTTOM_COLOR defines
On Fri, 12 Nov 2021, Ville Syrjala wrote: > From: Ville Syrjälä > > Use REG_BIT() for SKL_BOTTOM_COLOR. > > Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index e300a202ce2d..8b227dabb10c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6341,8 +6341,8 @@ enum { > > /* Skylake+ pipe bottom (background) color */ > #define _SKL_BOTTOM_COLOR_A 0x70034 > -#define SKL_BOTTOM_COLOR_GAMMA_ENABLE (1 << 31) > -#define SKL_BOTTOM_COLOR_CSC_ENABLE(1 << 30) > +#define SKL_BOTTOM_COLOR_GAMMA_ENABLE REG_BIT(31) > +#define SKL_BOTTOM_COLOR_CSC_ENABLEREG_BIT(30) > #define SKL_BOTTOM_COLOR(pipe) _MMIO_PIPE2(pipe, > _SKL_BOTTOM_COLOR_A) > > #define _ICL_PIPE_A_STATUS 0x70058 -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v5 5/5] drm/i915/uapi: document behaviour for DG2 64K support
On 1/25/22 20:35, Robert Beckett wrote: From: Matthew Auld On discrete platforms like DG2, we need to support a minimum page size of 64K when dealing with device local-memory. This is quite tricky for various reasons, so try to document the new implicit uapi for this. v3: fix typos and less emphasis v2: Fixed suggestions on formatting [Daniel] Signed-off-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Acked-by: Jordan Justen Reviewed-by: Ramalingam C cc: Simon Ser cc: Pekka Paalanen Cc: Jordan Justen Cc: Kenneth Graunke Cc: mesa-...@lists.freedesktop.org Cc: Tony Ye Cc: Slawomir Milczarek --- Reviewed-by: Thomas Hellström
Re: [Intel-gfx] [PATCH v5 4/5] drm/i915: add gtt misalignment test
On 1/25/22 20:35, Robert Beckett wrote: add test to check handling of misaligned offsets and sizes v4: * remove spurious blank lines * explicitly cast intel_region_id to intel_memory_type in misaligned_pin Reported-by: kernel test robot Signed-off-by: Robert Beckett --- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 128 ++ 1 file changed, 128 insertions(+) diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index b80788a2b7f9..f082b5ff3b5e 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -22,10 +22,12 @@ * */ +#include "gt/intel_gtt.h" #include #include #include "gem/i915_gem_context.h" +#include "gem/i915_gem_region.h" #include "gem/selftests/mock_context.h" #include "gt/intel_context.h" #include "gt/intel_gpu_commands.h" @@ -1067,6 +1069,120 @@ static int shrink_boom(struct i915_address_space *vm, return err; } +static int misaligned_case(struct i915_address_space *vm, struct intel_memory_region *mr, + u64 addr, u64 size, unsigned long flags) +{ + struct drm_i915_gem_object *obj; + struct i915_vma *vma; + int err = 0; + u64 expected_vma_size, expected_node_size; + + obj = i915_gem_object_create_region(mr, size, 0, 0); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + vma = i915_vma_instance(obj, vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto err_put; + } + + err = i915_vma_pin(vma, 0, 0, addr | flags); + if (err) + goto err_put; + i915_vma_unpin(vma); + + if (!drm_mm_node_allocated(&vma->node)) { + err = -EINVAL; + goto err_put; + } + + if (i915_vma_misplaced(vma, 0, 0, addr | flags)) { + err = -EINVAL; + goto err_put; + } + + expected_vma_size = round_up(size, 1 << (ffs(vma->resource->page_sizes_gtt) - 1)); + expected_node_size = expected_vma_size; + + if (IS_DG2(vm->i915) && i915_gem_object_is_lmem(obj)) { + /* dg2 should expand lmem node to 2MB */ Should this test be NEEDS_COMPACT_PT()? Otherwise LGTM. Reviewed-by: Thomas Hellström
Re: [Intel-gfx] [PATCH v5 1/5] drm/i915: add needs_compact_pt flag
On 1/25/22 20:35, Robert Beckett wrote: From: Ramalingam C Add a new platform flag, needs_compact_pt, to mark the requirement of compact pt layout support for the ppGTT when using 64K GTT pages. With this flag has_64k_pages will only indicate requirement of 64K GTT page sizes or larger for device local memory access. Suggested-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett --- drivers/gpu/drm/i915/i915_drv.h | 10 +++--- drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 44c1f98144b4..1258b7779705 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1512,12 +1512,16 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, /* * Set this flag, when platform requires 64K GTT page sizes or larger for - * device local memory access. Also this flag implies that we require or - * at least support the compact PT layout for the ppGTT when using the 64K - * GTT pages. Why do we remove these comment lines? + * device local memory access. */ #define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages) +/* Set this flag when platform doesn't allow both 64k pages and 4k pages in First line of multi-line comments should be empty. + * the same PT. this flag means we need to support compact PT layout for the + * ppGTT when using the 64K GTT pages. + */ +#define NEEDS_COMPACT_PT(dev_priv) (INTEL_INFO(dev_priv)->needs_compact_pt) + #define HAS_IPC(dev_priv) (INTEL_INFO(dev_priv)->display.has_ipc) #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 4081fd50ba9d..799b56569ef5 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1028,6 +1028,7 @@ static const struct intel_device_info xehpsdv_info = { PLATFORM(INTEL_XEHPSDV), .display = { }, .has_64k_pages = 1, + .needs_compact_pt = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) | @@ -1045,6 +1046,7 @@ static const struct intel_device_info dg2_info = { .media.rel = 55, PLATFORM(INTEL_DG2), .has_64k_pages = 1, + .needs_compact_pt = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 3699b1c539ea..c8aaf646430c 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -130,6 +130,7 @@ enum intel_ppgtt_type { /* Keep has_* in alphabetical order */ \ func(has_64bit_reloc); \ func(has_64k_pages); \ + func(needs_compact_pt); \ func(gpu_reset_clobbers_display); \ func(has_reset_engine); \ func(has_global_mocs); \
Re: [Intel-gfx] [PATCH] drm/i915/adlp: Fix TypeC PHY-ready status readout
On Wed, 2022-01-26 at 12:43 +0200, Imre Deak wrote: > The TCSS_DDI_STATUS register is indexed by tc_port not by the FIA port > index, fix this up. This only caused an issue on TC#3/4 ports in legacy > mode, as in all other cases the two indices either match (on TC#1/2) or > the TCSS_DDI_STATUS_READY flag is set regardless of something being > connected or not (on TC#1/2/3/4 in dp-alt and tbt-alt modes). > Reviewed-by: José Roberto de Souza > Reported-and-tested-by: Chia-Lin Kao (AceLan) > Fixes: 55ce306c2aa1 ("drm/i915/adl_p: Implement TC sequences") > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4698 > Cc: José Roberto de Souza > Cc: # v5.14+ > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/display/intel_tc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > b/drivers/gpu/drm/i915/display/intel_tc.c > index 4eefe7b0bb263..3291124a99e5a 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -346,10 +346,11 @@ static bool icl_tc_phy_status_complete(struct > intel_digital_port *dig_port) > static bool adl_tc_phy_status_complete(struct intel_digital_port *dig_port) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > + enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port); > struct intel_uncore *uncore = &i915->uncore; > u32 val; > > - val = intel_uncore_read(uncore, > TCSS_DDI_STATUS(dig_port->tc_phy_fia_idx)); > + val = intel_uncore_read(uncore, TCSS_DDI_STATUS(tc_port)); > if (val == 0x) { > drm_dbg_kms(&i915->drm, > "Port %s: PHY in TCCOLD, assuming not complete\n",
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix oops due to missing stack depot
== Series Details == Series: series starting with [1/2] drm/i915: Fix oops due to missing stack depot URL : https://patchwork.freedesktop.org/series/99353/ State : success == Summary == CI Bug Log - changes from CI_DRM_11141 -> Patchwork_22109 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/index.html Participating hosts (44 -> 43) -- Additional (2): fi-kbl-soraka bat-jsl-2 Missing(3): fi-ctg-p8600 fi-bsw-cyan fi-hsw-4200u Known issues Here are the changes found in Patchwork_22109 that come from known issues: ### IGT changes ### Issues hit * igt@amdgpu/amd_cs_nop@sync-fork-compute0: - fi-snb-2600:NOTRUN -> [SKIP][1] ([fdo#109271]) +17 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-snb-2600/igt@amdgpu/amd_cs_...@sync-fork-compute0.html * igt@amdgpu/amd_prime@amd-to-i915: - fi-pnv-d510:NOTRUN -> [SKIP][2] ([fdo#109271]) +17 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-pnv-d510/igt@amdgpu/amd_pr...@amd-to-i915.html * igt@gem_exec_fence@basic-busy@bcs0: - fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271]) +8 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-kbl-soraka/igt@gem_exec_fence@basic-b...@bcs0.html * igt@gem_huc_copy@huc-copy: - fi-kbl-soraka: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#2190]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-kbl-soraka/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-kbl-soraka/igt@gem_lmem_swapp...@basic.html * igt@i915_selftest@live@execlists: - fi-bsw-n3050: [PASS][6] -> [INCOMPLETE][7] ([i915#2940]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11141/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-bsw-n3050/igt@i915_selftest@l...@execlists.html * igt@i915_selftest@live@gt_pm: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][8] ([i915#1886] / [i915#2291]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html * igt@i915_selftest@live@hangcheck: - fi-hsw-4770:[PASS][9] -> [INCOMPLETE][10] ([i915#3303]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11141/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html * igt@kms_chamelium@common-hpd-after-suspend: - fi-kbl-soraka: NOTRUN -> [SKIP][11] ([fdo#109271] / [fdo#111827]) +8 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-kbl-soraka/igt@kms_chamel...@common-hpd-after-suspend.html * igt@kms_frontbuffer_tracking@basic: - fi-cml-u2: [PASS][12] -> [DMESG-WARN][13] ([i915#4269]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11141/fi-cml-u2/igt@kms_frontbuffer_track...@basic.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-cml-u2/igt@kms_frontbuffer_track...@basic.html * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d: - fi-kbl-soraka: NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#533]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-kbl-soraka/igt@kms_pipe_crc_ba...@compare-crc-sanitycheck-pipe-d.html * igt@runner@aborted: - fi-hsw-4770:NOTRUN -> [FAIL][15] ([fdo#109271] / [i915#1436] / [i915#4312]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-hsw-4770/igt@run...@aborted.html - fi-bsw-n3050: NOTRUN -> [FAIL][16] ([fdo#109271] / [i915#1436] / [i915#2722] / [i915#3428] / [i915#4312]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-bsw-n3050/igt@run...@aborted.html Possible fixes * igt@i915_selftest@live@hangcheck: - fi-snb-2600:[INCOMPLETE][17] ([i915#3921]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11141/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html * igt@i915_selftest@live@requests: - fi-pnv-d510:[DMESG-FAIL][19] ([i915#2927] / [i915#4528]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11141/fi-pnv-d510/igt@i915_selftest@l...@requests.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22109/fi-pnv-d510/igt@i915_selftest@l...@requests.html {name}: This element is suppressed. This means it is ignored when computing t
[Intel-gfx] [RFC PATCH] drm/i915: Remove all frontbuffer tracking calls from the gem code
We should now rely on userspace doing dirtyfb. There is no need to have separate frontbuffer tracking hooks in gem code. This patch is removing all frontbuffer tracking calls from the gem code. Cc: Ville Syrjälä Cc: Jani Nikula Cc: Daniel Vetter Signed-off-by: Jouni Högander --- drivers/gpu/drm/i915/display/intel_overlay.c | 2 -- drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 2 -- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 5 drivers/gpu/drm/i915/gem/i915_gem_object.c | 24 drivers/gpu/drm/i915/gem/i915_gem_object.h | 16 - drivers/gpu/drm/i915/gem/i915_gem_phys.c | 7 -- drivers/gpu/drm/i915/i915_gem.c | 5 7 files changed, 61 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c index 5358f03b52db..fc2691dac278 100644 --- a/drivers/gpu/drm/i915/display/intel_overlay.c +++ b/drivers/gpu/drm/i915/display/intel_overlay.c @@ -809,8 +809,6 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay, goto out_pin_section; } - i915_gem_object_flush_frontbuffer(new_bo, ORIGIN_DIRTYFB); - if (!overlay->active) { const struct intel_crtc_state *crtc_state = overlay->crtc->config; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c index 8a248003dfae..115e6d877e38 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c @@ -20,8 +20,6 @@ static void __do_clflush(struct drm_i915_gem_object *obj) { GEM_BUG_ON(!i915_gem_object_has_pages(obj)); drm_clflush_sg(obj->mm.pages); - - i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU); } static void clflush_work(struct dma_fence_work *base) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index 26532c07d467..ab1fc2d930e1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -63,7 +63,6 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains) } spin_unlock(&obj->vma.lock); - i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU); break; case I915_GEM_DOMAIN_WC: @@ -615,9 +614,6 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, out_unlock: i915_gem_object_unlock(obj); - if (!err && write_domain) - i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU); - out: i915_gem_object_put(obj); return err; @@ -728,7 +724,6 @@ int i915_gem_object_prepare_write(struct drm_i915_gem_object *obj, } out: - i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU); obj->mm.dirty = true; /* return with the pages pinned */ return 0; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 1a9e1f940a7d..f7ba66deb923 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -394,30 +394,6 @@ static void i915_gem_free_object(struct drm_gem_object *gem_obj) queue_delayed_work(i915->wq, &i915->mm.free_work, 0); } -void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, -enum fb_op_origin origin) -{ - struct intel_frontbuffer *front; - - front = __intel_frontbuffer_get(obj); - if (front) { - intel_frontbuffer_flush(front, origin); - intel_frontbuffer_put(front); - } -} - -void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, - enum fb_op_origin origin) -{ - struct intel_frontbuffer *front; - - front = __intel_frontbuffer_get(obj); - if (front) { - intel_frontbuffer_invalidate(front, origin); - intel_frontbuffer_put(front); - } -} - static void i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 02c37fe4a535..d7a08172b239 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -578,22 +578,6 @@ void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, enum fb_op_origin origin); -static inline void -i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, - enum fb_op_origin origin) -{ - if (unlikely(rcu_access_pointer(obj->frontbuffer))) - __i915_gem_object_flush_frontbuffer
Re: [Intel-gfx] [PATCH] drm/i915: Lock dpt_obj around set_cache_level, v2.
On 1/26/22 9:37 AM, Maarten Lankhorst wrote: set_cache_level may unbind the object, which will result in the below lockdep splat: <6> [184.578145] [IGT] kms_addfb_basic: starting subtest addfb25-framebuffer-vs-set-tiling <4> [184.578220] [ cut here ] <4> [184.578221] WARN_ON(debug_locks && !(lock_is_held(&(&((obj)->base.resv)->lock.base)->dep_map) != 0)) <4> [184.578237] WARNING: CPU: 6 PID: 5544 at drivers/gpu/drm/i915/i915_gem.c:123 i915_gem_object_unbind+0x4a9/0x510 [i915] <4> [184.578323] Modules linked in: vgem drm_shmem_helper snd_hda_codec_hdmi i915 mei_hdcp x86_pkg_temp_thermal snd_hda_intel coretemp crct10dif_pclmul snd_intel_dspcfg crc32_pclmul ttm snd_hda_codec ghash_clmulni_intel snd_hwdep drm_kms_helper snd_hda_core e1000e mei_me syscopyarea ptp snd_pcm sysfillrect mei pps_core sysimgblt fb_sys_fops prime_numbers intel_lpss_pci smsc75xx usbnet mii <4> [184.578349] CPU: 6 PID: 5544 Comm: kms_addfb_basic Not tainted 5.16.0-CI-Patchwork_22006+ #1 <4> [184.578351] Hardware name: Intel Corporation Alder Lake Client Platform/AlderLake-P DDR4 RVP, BIOS ADLPFWI1.R00.2422.A00.2110131104 10/13/2021 <4> [184.578352] RIP: 0010:i915_gem_object_unbind+0x4a9/0x510 [i915] <4> [184.578424] Code: 00 be ff ff ff ff 48 8d 78 68 e8 a2 6e 2b e1 85 c0 0f 85 b1 fb ff ff 48 c7 c6 48 37 9e a0 48 c7 c7 d9 fc a1 a0 e8 a3 54 26 e1 <0f> 0b e9 97 fb ff ff 31 ed 48 8b 5c 24 58 65 48 33 1c 25 28 00 00 <4> [184.578426] RSP: 0018:c900013b3b68 EFLAGS: 00010286 <4> [184.578428] RAX: RBX: c900013b3bb0 RCX: 0001 <4> [184.578429] RDX: 8001 RSI: 8230b42d RDI: <4> [184.578430] RBP: 888120e1 R08: R09: c0007fff <4> [184.578431] R10: 0001 R11: c900013b3980 R12: 8881176ea740 <4> [184.578432] R13: 888120e1 R14: R15: 0001 <4> [184.578433] FS: 7f65074f5e40() GS:8f30() knlGS: <4> [184.578435] CS: 0010 DS: ES: CR0: 80050033 <4> [184.578436] CR2: 7fff4420ede8 CR3: 00010c2f2005 CR4: 00770ee0 <4> [184.578437] PKRU: 5554 <4> [184.578438] Call Trace: <4> [184.578439] <4> [184.578440] ? dma_resv_iter_first_unlocked+0x78/0xf0 <4> [184.578447] intel_dpt_create+0x88/0x220 [i915] <4> [184.578530] intel_framebuffer_init+0x5b8/0x620 [i915] <4> [184.578612] intel_framebuffer_create+0x3d/0x60 [i915] <4> [184.578691] intel_user_framebuffer_create+0x18f/0x2c0 [i915] <4> [184.578775] drm_internal_framebuffer_create+0x36d/0x4c0 <4> [184.578779] drm_mode_addfb2+0x2f/0xd0 <4> [184.578781] ? drm_mode_addfb_ioctl+0x10/0x10 <4> [184.578784] drm_ioctl_kernel+0xac/0x140 <4> [184.578787] drm_ioctl+0x201/0x3d0 <4> [184.578789] ? drm_mode_addfb_ioctl+0x10/0x10 <4> [184.578796] __x64_sys_ioctl+0x6a/0xa0 <4> [184.578800] do_syscall_64+0x37/0xb0 <4> [184.578803] entry_SYSCALL_64_after_hwframe+0x44/0xae <4> [184.578805] RIP: 0033:0x7f6506736317 <4> [184.578807] Code: b3 66 90 48 8b 05 71 4b 2d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 41 4b 2d 00 f7 d8 64 89 01 48 <4> [184.578808] RSP: 002b:7fff44211a98 EFLAGS: 0246 ORIG_RAX: 0010 <4> [184.578810] RAX: ffda RBX: 0006 RCX: 7f6506736317 <4> [184.578811] RDX: 7fff44211b30 RSI: c06864b8 RDI: 0006 <4> [184.578812] RBP: 7fff44211b30 R08: 7fff44311170 R09: <4> [184.578813] R10: 0008 R11: 0246 R12: c06864b8 <4> [184.578813] R13: 0006 R14: R15: <4> [184.578819] <4> [184.578820] irq event stamp: 47931 <4> [184.578821] hardirqs last enabled at (47937): [] __up_console_sem+0x62/0x70 <4> [184.578824] hardirqs last disabled at (47942): [] __up_console_sem+0x47/0x70 <4> [184.578826] softirqs last enabled at (47340): [] __do_softirq+0x32d/0x493 <4> [184.578828] softirqs last disabled at (47335): [] irq_exit_rcu+0xa6/0xe0 <4> [184.578830] ---[ end trace f17ec219f892c7d4 ]--- Changes since v1: - Fix intel_pin_fb_obj_dpt too. Fixes: 0f341974cbc2 ("drm/i915: Add i915_vma_unbind_unlocked, and take obj lock for i915_vma_unbind, v2.") Signed-off-by: Maarten Lankhorst Testcase: kms_addfb_basic --- drivers/gpu/drm/i915/display/intel_dpt.c| 6 +- drivers/gpu/drm/i915/display/intel_fb_pin.c | 6 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c index 63a83d5f85a1..c2f8f853db90 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -253,7 +253,11 @@ intel_dpt_create(struct intel_framebuffer *fb) if (IS_ERR(dpt_obj)) return ERR_CAST(dpt_obj); - ret = i915_gem_object_set_cache_l
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Enable rpm wakeref tracking whether runtime pm is enabled or not
On Wed, Jan 26, 2022 at 10:15:39AM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > Don't see why we should skip the wakeref tracking when the > platform doesn't support runtime pm. We still want all the > code to be 100% leak free so let's track this unconditionally. > > Cc: Vlastimil Babka > Cc: Dmitry Vyukov > Cc: Marco Elver # stackdepot > Cc: Chris Wilson > Cc: Imre Deak > Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 64c2708efc9e..3293ac71bcf8 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -77,9 +77,6 @@ track_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm) > depot_stack_handle_t stack, *stacks; > unsigned long flags; > > - if (!rpm->available) > - return -1; > - > stack = __save_depot_stack(); > if (!stack) > return -1; > -- > 2.34.1 >
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix oops due to missing stack depot
On Wed, Jan 26, 2022 at 10:15:38AM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > We call __save_depot_stack() unconditionally so the stack depot > must always be initialized or else we'll oops on platforms without > runtime pm support. > > Presumably we've not seen this in CI due to stack_depot_init() > already getting called via drm_mm_init()+CONFIG_DRM_DEBUG_MM. > > Cc: Vlastimil Babka > Cc: Dmitry Vyukov > Cc: Marco Elver # stackdepot > Cc: Chris Wilson > Cc: Imre Deak > Fixes: 2dba5eb1c73b ("lib/stackdepot: allow optional init and stack_table > allocation by kvmalloc()") > Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 53f1ccb78849..64c2708efc9e 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -68,9 +68,7 @@ static noinline depot_stack_handle_t > __save_depot_stack(void) > static void init_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm) > { > spin_lock_init(&rpm->debug.lock); > - > - if (rpm->available) > - stack_depot_init(); > + stack_depot_init(); > } > > static noinline depot_stack_handle_t > -- > 2.34.1 >
Re: [Intel-gfx] [PATCH v2 09/11] drm: Convert open-coded yes/no strings to yesno()
On Wed, Jan 26, 2022 at 02:43:45AM -0800, Lucas De Marchi wrote: > On Wed, Jan 26, 2022 at 12:12:50PM +0200, Andy Shevchenko wrote: > > On Wed, Jan 26, 2022 at 11:39 AM Lucas De Marchi > > wrote: ... > > > 411986 104906176 428652 68a6c drm.ko.old > > > 411986 104906176 428652 68a6c drm.ko > > > 981291636 264 100029 186bd dp/drm_dp_helper.ko.old > > > 981291636 264 100029 186bd dp/drm_dp_helper.ko > > > 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko.old > > > 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko > > > > This probably won't change for modules, but if you compile in the > > linker may try to optimize it. Would be nice to see the old-new for > > `make allyesconfig` or equivalent. > > just like it would already do, no? I can try and see what happens, but > my feeling is that we won't have any change. Maybe not or maybe a small win. Depends how compiler puts / linker sees that in two cases. (Yeah, likely it should be no differences if all instances are already caught by linker) -- With Best Regards, Andy Shevchenko
[Intel-gfx] [PATCH v5 04/10] drm/i915/guc: Add Gen9 registers for GuC error state capture.
Abstract out a Gen9 register list as the default for all other platforms we don't yet formally support GuC submission on. Signed-off-by: Alan Previn --- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 74 +++ 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 19719daffed4..70d2ee841289 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -19,15 +19,24 @@ * NOTE: For engine-registers, GuC only needs the register offsets * from the engine-mmio-base */ +#define COMMON_BASE_GLOBAL() \ + {FORCEWAKE_MT, 0, 0, "FORCEWAKE_MT"} + +#define COMMON_GEN9BASE_GLOBAL() \ + {GEN8_FAULT_TLB_DATA0, 0, 0, "GEN8_FAULT_TLB_DATA0"}, \ + {GEN8_FAULT_TLB_DATA1, 0, 0, "GEN8_FAULT_TLB_DATA1"}, \ + {ERROR_GEN6, 0, 0, "ERROR_GEN6"}, \ + {DONE_REG, 0, 0, "DONE_REG"}, \ + {HSW_GTT_CACHE_EN, 0, 0, "HSW_GTT_CACHE_EN"} + #define COMMON_GEN12BASE_GLOBAL() \ {GEN12_FAULT_TLB_DATA0,0, 0, "GEN12_FAULT_TLB_DATA0"}, \ {GEN12_FAULT_TLB_DATA1,0, 0, "GEN12_FAULT_TLB_DATA1"}, \ - {FORCEWAKE_MT, 0, 0, "FORCEWAKE_MT"}, \ {GEN12_AUX_ERR_DBG,0, 0, "GEN12_AUX_ERR_DBG"}, \ {GEN12_GAM_DONE, 0, 0, "GEN12_GAM_DONE"}, \ {GEN12_RING_FAULT_REG, 0, 0, "GEN12_RING_FAULT_REG"} -#define COMMON_GEN12BASE_ENGINE_INSTANCE() \ +#define COMMON_BASE_ENGINE_INSTANCE() \ {RING_PSMI_CTL(0), 0, 0, "RING_PSMI_CTL"}, \ {RING_ESR(0), 0, 0, "RING_ESR"}, \ {RING_DMA_FADD(0), 0, 0, "RING_DMA_FADD_LOW32"}, \ @@ -61,11 +70,13 @@ {GEN8_RING_PDP_LDW(0, 3), 0, 0, "GEN8_RING_PDP3_LDW"}, \ {GEN8_RING_PDP_UDW(0, 3), 0, 0, "GEN8_RING_PDP3_UDW"} -#define COMMON_GEN12BASE_HAS_EU() \ +#define COMMON_BASE_HAS_EU() \ {EIR, 0, 0, "EIR"} +#define COMMON_BASE_RENDER() \ + {GEN7_SC_INSTDONE, 0, 0, "GEN7_SC_INSTDONE"} + #define COMMON_GEN12BASE_RENDER() \ - {GEN7_SC_INSTDONE, 0, 0, "GEN7_SC_INSTDONE"}, \ {GEN12_SC_INSTDONE_EXTRA, 0, 0, "GEN12_SC_INSTDONE_EXTRA"}, \ {GEN12_SC_INSTDONE_EXTRA2, 0, 0, "GEN12_SC_INSTDONE_EXTRA2"} @@ -77,23 +88,26 @@ /* XE_LPD - Global */ static struct __guc_mmio_reg_descr xe_lpd_global_regs[] = { + COMMON_BASE_GLOBAL(), + COMMON_GEN9BASE_GLOBAL(), COMMON_GEN12BASE_GLOBAL(), }; /* XE_LPD - Render / Compute Per-Class */ static struct __guc_mmio_reg_descr xe_lpd_rc_class_regs[] = { - COMMON_GEN12BASE_HAS_EU(), + COMMON_BASE_HAS_EU(), + COMMON_BASE_RENDER(), COMMON_GEN12BASE_RENDER(), }; -/* XE_LPD - Render / Compute Per-Engine-Instance */ +/* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */ static struct __guc_mmio_reg_descr xe_lpd_rc_inst_regs[] = { - COMMON_GEN12BASE_ENGINE_INSTANCE(), + COMMON_BASE_ENGINE_INSTANCE(), }; -/* XE_LPD - Media Decode/Encode Per-Engine-Instance */ +/* GEN9/XE_LPD - Media Decode/Encode Per-Engine-Instance */ static struct __guc_mmio_reg_descr xe_lpd_vd_inst_regs[] = { - COMMON_GEN12BASE_ENGINE_INSTANCE(), + COMMON_BASE_ENGINE_INSTANCE(), }; /* XE_LPD - Video Enhancement Per-Class */ @@ -101,18 +115,33 @@ static struct __guc_mmio_reg_descr xe_lpd_vec_class_regs[] = { COMMON_GEN12BASE_VEC(), }; -/* XE_LPD - Video Enhancement Per-Engine-Instance */ +/* GEN9/XE_LPD - Video Enhancement Per-Engine-Instance */ static struct __guc_mmio_reg_descr xe_lpd_vec_inst_regs[] = { - COMMON_GEN12BASE_ENGINE_INSTANCE(), + COMMON_BASE_ENGINE_INSTANCE(), }; -/* XE_LPD - Blitter Per-Engine-Instance */ +/* GEN9/XE_LPD - Blitter Per-Engine-Instance */ static struct __guc_mmio_reg_descr xe_lpd_blt_inst_regs[] = { - COMMON_GEN12BASE_ENGINE_INSTANCE(), + COMMON_BASE_ENGINE_INSTANCE(), +}; + +/* GEN9 - Global */ +static struct __guc_mmio_reg_descr default_global_regs[] = { + COMMON_BASE_GLOBAL(), + COMMON_GEN9BASE_GLOBAL(), }; -/* XE_LPD - Blitter Per-Class */ -/* XE_LPD - Media Decode/Encode Per-Class */ +static struct __guc_mmio_reg_descr default_rc_class_regs[] = { + COMMON_BASE_HAS_EU(), + COMMON_BASE_RENDER(), +}; + +/* + * Empty lists: + * GEN9/XE_LPD - Blitter-Class + * GEN9/XE_LPD - Media Class + * GEN9 - VEC Class + */ static struct __guc_mmio_reg_descr empty_regs_list[] = { }; @@ -130,6 +159,18 @@ static struct __guc_mmio_reg_descr empty_regs_list[] = { } /* List of lists */ +static struct __guc_mmio_reg_descr_group default_lists[] = { + MAKE_REGLIST(default_global_regs, PF, GLOBAL, 0), + MAKE_REGLIST(default_rc_class_regs, PF, E
[Intel-gfx] [PATCH v5 08/10] drm/i915/guc: Plumb GuC-capture into gpu_coredump
Add a flags parameter through all of the coredump creation functions. Add a bitmask flag to indicate if the top level gpu_coredump event is triggered in response to a GuC context reset notification. Using that flag, ensure all coredump functions that read or print mmio-register values related to work submission or command-streamer engines are skipped and replaced with a calls guc-capture module equivalent functions to retrieve or print the register dump. While here, split out display related register reading and printing into its own function that is called agnostic to whether GuC had triggered the reset. For now, introduce an empty printing function that can filled in on a subsequent patch just to handle formatting. Signed-off-by: Alan Previn --- .../drm/i915/gt/intel_execlists_submission.c | 4 +- drivers/gpu/drm/i915/gt/intel_reset.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 78 ++ .../gpu/drm/i915/gt/uc/intel_guc_capture.h| 11 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 263 -- drivers/gpu/drm/i915/i915_gpu_error.h | 31 ++- 8 files changed, 295 insertions(+), 98 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 960a9aaf4f3a..c8bb43863461 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2230,11 +2230,11 @@ static struct execlists_capture *capture_regs(struct intel_engine_cs *engine) if (!cap->error) goto err_cap; - cap->error->gt = intel_gt_coredump_alloc(engine->gt, gfp); + cap->error->gt = intel_gt_coredump_alloc(engine->gt, gfp, CORE_DUMP_FLAG_NONE); if (!cap->error->gt) goto err_gpu; - cap->error->gt->engine = intel_engine_coredump_alloc(engine, gfp); + cap->error->gt->engine = intel_engine_coredump_alloc(engine, gfp, CORE_DUMP_FLAG_NONE); if (!cap->error->gt->engine) goto err_gt; diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 6f2821cca409..c1dc3f8b1108 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -1305,7 +1305,7 @@ void intel_gt_handle_error(struct intel_gt *gt, engine_mask &= gt->info.engine_mask; if (flags & I915_ERROR_CAPTURE) { - i915_capture_error_state(gt, engine_mask); + i915_capture_error_state(gt, engine_mask, CORE_DUMP_FLAG_NONE); intel_gt_clear_error_registers(gt, engine_mask); } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 0b6d743712a6..2f5dc413dddc 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -728,6 +728,17 @@ int intel_guc_capture_output_min_size_est(struct intel_guc *guc) * instance). This node id added to a linked list stored in * guc->capture->priv for matchup and printout when triggered by * i915_gpu_coredump and err_print_gt (via error capture sysfs) later. + * + * GUC --> notify context reset: + * - + * --> G2H CONTEXT RESET + * L--> guc_handle_context_reset --> i915_capture_error_state + * L--> i915_gpu_coredump(..IS_GUC_CAPTURE) --> gt_record_engines + * --> capture_engine(..IS_GUC_CAPTURE) + * L--> detach C from internal linked list and add into + * intel_engine_coredump struct (if the context and + * engine of the event notification matches a node + * in the link list) */ static int guc_capture_buf_cnt(struct __guc_capture_bufstate *buf) @@ -1203,6 +1214,73 @@ static void __guc_capture_store_snapshot_work(struct intel_guc *guc) mutex_unlock(&guc->log_state[GUC_CAPTURE_LOG_BUFFER].lock); } +#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) + +int intel_guc_capture_print_engine_node(struct drm_i915_error_state_buf *ebuf, + const struct intel_engine_coredump *ee) +{ + return 0; +} + +#endif //CONFIG_DRM_I915_DEBUG_GUC + +void intel_guc_capture_free_node(struct intel_engine_coredump *ee) +{ + int i; + + if (!ee) + return; + if (!ee->guc_capture_node) + return; + for (i = GUC_CAPTURE_LIST_TYPE_GLOBAL; i < GUC_CAPTURE_LIST_TYPE_MAX; ++i) { + if (ee->guc_capture_node->reginfo[i].regs) + kfree(ee->guc_capture_node->reginfo[i].regs); + } + kfree(e
[Intel-gfx] [PATCH v5 06/10] drm/i915/guc: Update GuC's log-buffer-state access for error capture.
GuC log buffer regions for debug-log-events, crash-dumps and error-state-capture are all a single bo allocation that includes the guc_log_buffer_state structures. Since the error-capture region is accessed with high priority at non- deterministic times (as part of gpu coredump) while the debug-log-event region is populated and accessed with different priorities, timings and consumers, let's split out separate locks for buffer-state accesses of each region. Also, ensure a global mapping is made up front for the entire bo throughout GuC operation so that dynamic mapping and unmapping isn't required for error capture log access if relay-logging isn't running. Additionally, while here, make some readibility improvements: 1. change previous function names with "capture_logs" to "copy_debug_logs" to help make the distinction clearer. 2. Update the guc log region mapping comments to order them according to the enum definition as per the GuC interface. Signed-off-by: Alan Previn --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 + .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 47 ++ .../gpu/drm/i915/gt/uc/intel_guc_capture.h| 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_log.c| 135 +++--- drivers/gpu/drm/i915/gt/uc/intel_guc_log.h| 16 ++- 5 files changed, 141 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 4e819853ec2e..be1ad7fa2bf8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -34,6 +34,8 @@ struct intel_guc { struct intel_uc_fw fw; /** @log: sub-structure containing GuC log related data and objects */ struct intel_guc_log log; + /** @log_state: states and locks for each subregion of GuC's log buffer */ + struct intel_guc_log_stats log_state[GUC_MAX_LOG_BUFFER]; /** @ct: the command transport communication channel */ struct intel_guc_ct ct; /** @slpc: sub-structure containing SLPC related data and objects */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 70d2ee841289..e7f99d051636 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -651,6 +651,53 @@ int intel_guc_capture_prep_lists(struct intel_guc *guc, struct guc_ads *blob, u3 return PAGE_ALIGN(alloc_size); } +#define GUC_CAPTURE_OVERBUFFER_MULTIPLIER 3 +int intel_guc_capture_output_min_size_est(struct intel_guc *guc) +{ + struct intel_gt *gt = guc_to_gt(guc); + struct intel_engine_cs *engine; + enum intel_engine_id id; + int worst_min_size = 0, num_regs = 0; + u16 tmp = 0; + + /* +* If every single engine-instance suffered a failure in quick succession but +* were all unrelated, then a burst of multiple error-capture events would dump +* registers for every one engine instance, one at a time. In this case, GuC +* would even dump the global-registers repeatedly. +* +* For each engine instance, there would be 1 x guc_state_capture_group_t output +* followed by 3 x guc_state_capture_t lists. The latter is how the register +* dumps are split across different register types (where the '3' are global vs class +* vs instance). Finally, let's multiply the whole thing by 3x (just so we are +* not limited to just 1 round of data in a worst case full register dump log) +* +* NOTE: intel_guc_log that allocates the log buffer would round this size up to +* a power of two. +*/ + + for_each_engine(engine, gt, id) { + worst_min_size += sizeof(struct guc_state_capture_group_header_t) + + (3 * sizeof(struct guc_state_capture_header_t)); + + if (!guc_capture_list_count(guc, 0, GUC_CAPTURE_LIST_TYPE_GLOBAL, 0, &tmp)) + num_regs += tmp; + + if (!guc_capture_list_count(guc, 0, GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS, + engine->class, &tmp)) { + num_regs += tmp; + } + if (!guc_capture_list_count(guc, 0, GUC_CAPTURE_LIST_TYPE_ENGINE_INSTANCE, + engine->class, &tmp)) { + num_regs += tmp; + } + } + + worst_min_size += (num_regs * sizeof(struct guc_mmio_reg)); + + return (worst_min_size * GUC_CAPTURE_OVERBUFFER_MULTIPLIER); +} + void intel_guc_capture_destroy(struct intel_guc *guc) { guc_capture_clear_ext_regs(guc->capture.priv->reglists); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h index 6b5594ca529d..4d3e5221128c 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h +++ b/drivers
[Intel-gfx] [PATCH v5 10/10] drm/i915/guc: Print the GuC error capture output register list.
Print the GuC captured error state register list (string names and values) when gpu_coredump_state printout is invoked via the i915 debugfs for flushing the gpu error-state that was captured prior. Since GuC could have reported multiple engine register dumps in a single notification event, parse the captured data (appearing as a stream of structures) to identify each dump as a different 'engine-capture-group-output'. Finally, for each 'engine-capture-group-output' that is found, verify if the engine register dump corresponds to the engine_coredump content that was previously populated by the i915_gpu_coredump function. That function would have copied the context's vma's including the bacth buffer during the G2H-context-reset notification that occurred earlier. Perform this verification check by comparing guc_id, lrca and engine- instance obtained from the 'engine-capture-group-output' vs a copy of that same info taken during i915_gpu_coredump. If they match, then print those vma's as well (such as the batch buffers). Signed-off-by: Alan Previn --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 +- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 169 ++ .../gpu/drm/i915/gt/uc/intel_guc_capture.h| 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 1 + drivers/gpu/drm/i915/i915_gpu_error.c | 16 +- drivers/gpu/drm/i915/i915_gpu_error.h | 5 + 6 files changed, 185 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 4317ae5e525b..47c0c32d9b86 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1628,9 +1628,7 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine, drm_printf(m, "\tIPEHR: 0x%08x\n", ENGINE_READ(engine, IPEHR)); } - if (intel_engine_uses_guc(engine)) { - /* nothing to print yet */ - } else if (HAS_EXECLISTS(dev_priv)) { + if (HAS_EXECLISTS(dev_priv) && !intel_engine_uses_guc(engine)) { struct i915_request * const *port, *rq; const u32 *hws = &engine->status_page.addr[I915_HWS_CSB_BUF0_INDEX]; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 506496058daf..5cb24098747e 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -739,6 +739,15 @@ int intel_guc_capture_output_min_size_est(struct intel_guc *guc) * intel_engine_coredump struct (if the context and * engine of the event notification matches a node * in the link list) + * + * User Sysfs / Debugfs + * + * --> i915_gpu_coredump_copy_to_buffer-> + * L--> err_print_to_sgl --> err_print_gt + *L--> error_print_guc_captures + * L--> intel_guc_capture_print_node prints the + * register lists values of the attached node + * on the error-engine-dump being reported. */ static int guc_capture_buf_cnt(struct __guc_capture_bufstate *buf) @@ -1216,12 +1225,172 @@ static void __guc_capture_store_snapshot_work(struct intel_guc *guc) #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) +static const char * +guc_capture_reg_to_str(const struct intel_guc *guc, u32 owner, u32 type, + u32 class, u32 id, u32 offset, u32 *is_ext) +{ + struct __guc_mmio_reg_descr_group *reglists = guc->capture.priv->reglists; + struct __guc_mmio_reg_descr_group *match; + int num_regs, j; + + *is_ext = 0; + if (!reglists) + return NULL; + + match = guc_capture_get_one_list(reglists, owner, type, id); + + if (match) { + for (num_regs = match->num_regs, j = 0; j < num_regs; ++j) { + if (offset == match->list[j].reg.reg) + return match->list[j].regname; + } + } + if (match->ext) { + for (num_regs = match->num_ext, j = 0; j < num_regs; ++j) { + if (offset == match->ext[j].reg.reg) { + *is_ext = 1; + return match->ext[j].regname; + } + } + } + + return NULL; +} + +#ifdef CONFIG_DRM_I915_DEBUG_GUC +#define guc_capt_err_print(a, b, ...) \ + do { \ + drm_warn(a, __VA_ARGS__); \ + if (b) \ + i915_error_printf(b, __VA_ARGS__); \ + } while (0) +#else +#define guc_capt_err_print(a, b, ...) \ + do { \ + if (b) \ + i915_error_printf(b, __VA_ARGS__); \ +
[Intel-gfx] [PATCH v5 05/10] drm/i915/guc: Add GuC's error state capture output structures.
Add GuC's error capture output structures and definitions as how they would appear in GuC log buffer's error capture subregion after an error state capture G2H event notification. Signed-off-by: Alan Previn Reviewed-by: Matthew Brost --- drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 35 +++ 1 file changed, 35 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h index a2f97d04ff18..495cdb0228c6 100644 --- a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h @@ -28,6 +28,41 @@ struct __guc_mmio_reg_descr_group { struct __guc_mmio_reg_descr *ext; }; +struct guc_state_capture_header_t { + u32 reserved1; + u32 info; + #define CAP_HDR_CAPTURE_TYPE GENMASK(3, 0) /* see enum guc_capture_type */ + #define CAP_HDR_ENGINE_CLASS GENMASK(7, 4) /* see GUC_MAX_ENGINE_CLASSES */ + #define CAP_HDR_ENGINE_INSTANCE GENMASK(11, 8) + u32 lrca; /* if type-instance, LRCA (address) that hung, else set to ~0 */ + u32 guc_id; /* if type-instance, context index of hung context, else set to ~0 */ + u32 num_mmios; + #define CAP_HDR_NUM_MMIOS GENMASK(9, 0) +} __packed; + +struct guc_state_capture_t { + struct guc_state_capture_header_t header; + struct guc_mmio_reg mmio_entries[0]; +} __packed; + +enum guc_capture_group_types { + GUC_STATE_CAPTURE_GROUP_TYPE_FULL, + GUC_STATE_CAPTURE_GROUP_TYPE_PARTIAL, + GUC_STATE_CAPTURE_GROUP_TYPE_MAX, +}; + +struct guc_state_capture_group_header_t { + u32 reserved1; + u32 info; + #define CAP_GRP_HDR_NUM_CAPTURES GENMASK(7, 0) + #define CAP_GRP_HDR_CAPTURE_TYPE GENMASK(15, 8) /* guc_capture_group_types */ +} __packed; + +struct guc_state_capture_group_t { + struct guc_state_capture_group_header_t grp_header; + struct guc_state_capture_t capture_entries[0]; +} __packed; + struct __guc_state_capture_priv { struct __guc_mmio_reg_descr_group *reglists; u16 num_instance_regs[GUC_CAPTURE_LIST_INDEX_MAX][GUC_MAX_ENGINE_CLASSES]; -- 2.25.1
[Intel-gfx] [PATCH v5 07/10] drm/i915/guc: Extract GuC error capture lists on G2H notification.
- Upon the G2H Notify-Err-Capture event, parse through the GuC Log Buffer (error-capture-region) and dynamically allocate capture-nodes, A single node represents a single "engine- instance-capture-dump" and contains at least 3 register lists: global, engine-class and engine-instance. An internal link list is maintained to store one or more nodes. - The G2G error-capture notification event happens before the corresponding G2H context-reset that triggers the i915_gpu_coredump (where we want to avoid memory allocation moving forward). - Because the link-list node allocations happen before the call to i915_gpu_codedump, duplicate global and engine-class register lists for each engine-instance register dump if we find dependent-engine resets in a engine-capture-group. - Later when i915_gpu_coredump calls into capture_engine, (in the subsequent patch) we dettach the matching node (guc-id, LRCA, etc) from the link list above and attach it to i915_gpu_coredump's intel_engine_coredump structure when have matching LRCA/guc-id/engine-instance. - Finally, when we reset GuC submission lets also parse all outstanding capture data here too. Signed-off-by: Alan Previn --- .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 7 + drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 52 +- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 517 +- .../gpu/drm/i915/gt/uc/intel_guc_capture.h| 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_log.c| 26 +- drivers/gpu/drm/i915/gt/uc/intel_guc_log.h| 4 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 14 +- 7 files changed, 608 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index 7afdadc7656f..82a69f54cddb 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -173,4 +173,11 @@ enum intel_guc_sleep_state_status { #define GUC_LOG_CONTROL_VERBOSITY_MASK (0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT) #define GUC_LOG_CONTROL_DEFAULT_LOGGING(1 << 8) +enum intel_guc_state_capture_event_status { + INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_SUCCESS = 0x0, + INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_NOSPACE = 0x1, +}; + +#define INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_MASK 0x1 + #endif /* _ABI_GUC_ACTIONS_ABI_H */ diff --git a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h index 495cdb0228c6..14c497f12621 100644 --- a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h @@ -29,7 +29,8 @@ struct __guc_mmio_reg_descr_group { }; struct guc_state_capture_header_t { - u32 reserved1; + u32 owner; + #define CAP_HDR_CAPTURE_VFID GENMASK(7, 0) u32 info; #define CAP_HDR_CAPTURE_TYPE GENMASK(3, 0) /* see enum guc_capture_type */ #define CAP_HDR_ENGINE_CLASS GENMASK(7, 4) /* see GUC_MAX_ENGINE_CLASSES */ @@ -52,7 +53,8 @@ enum guc_capture_group_types { }; struct guc_state_capture_group_header_t { - u32 reserved1; + u32 owner; + #define CAP_GRP_HDR_CAPTURE_VFID GENMASK(7, 0) u32 info; #define CAP_GRP_HDR_NUM_CAPTURES GENMASK(7, 0) #define CAP_GRP_HDR_CAPTURE_TYPE GENMASK(15, 8) /* guc_capture_group_types */ @@ -63,11 +65,57 @@ struct guc_state_capture_group_t { struct guc_state_capture_t capture_entries[0]; } __packed; +struct __guc_capture_parsed_output { + /* +* a single set of 3 capture lists: a global-list +* an engine-class-list and an engine-instance list. +* outlist in __guc_capture_parsed_output will keep +* a linked list of these nodes that will eventually +* be detached from outlist and attached into to +* i915_gpu_codedump in response to a context reset +*/ + struct list_head link; + bool is_partial; + u32 eng_class; + u32 eng_inst; + u32 guc_id; + u32 lrca; + struct gcap_reg_list_info { + u32 vfid; + u32 num; + struct guc_mmio_reg *regs; + } reginfo[GUC_CAPTURE_LIST_TYPE_MAX]; + #define GCAP_PARSED_REGLIST_INDEX_GLOBAL BIT(GUC_CAPTURE_LIST_TYPE_GLOBAL) + #define GCAP_PARSED_REGLIST_INDEX_ENGCLASS BIT(GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS) + #define GCAP_PARSED_REGLIST_INDEX_ENGINST BIT(GUC_CAPTURE_LIST_TYPE_ENGINE_INSTANCE) +}; + +#define MAX_NODE_LINKLIST_THRESHOLD 24 + /* The maximum number of allocated __guc_capture_parsed_output nodes +* that we shall keep in outlist. If we receive an error-capture +* notification and need to allocate another node but have hit this +* threshold, we shall free the oldest entry and add a new one (FIFO). +*/ + +struct __guc_capture_bufstate { + unsigned int size; +
[Intel-gfx] [PATCH v5 09/10] drm/i915/guc: Follow legacy register names
Before we print the GuC provided register dumps, modify the register tables to use string names as per the legacy error capture execlist codes. Signed-off-by: Alan Previn --- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 70 +-- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 2f5dc413dddc..506496058daf 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -22,7 +22,7 @@ * from the engine-mmio-base */ #define COMMON_BASE_GLOBAL() \ - {FORCEWAKE_MT, 0, 0, "FORCEWAKE_MT"} + {FORCEWAKE_MT, 0, 0, "FORCEWAKE"} #define COMMON_GEN9BASE_GLOBAL() \ {GEN8_FAULT_TLB_DATA0, 0, 0, "GEN8_FAULT_TLB_DATA0"}, \ @@ -34,43 +34,43 @@ #define COMMON_GEN12BASE_GLOBAL() \ {GEN12_FAULT_TLB_DATA0,0, 0, "GEN12_FAULT_TLB_DATA0"}, \ {GEN12_FAULT_TLB_DATA1,0, 0, "GEN12_FAULT_TLB_DATA1"}, \ - {GEN12_AUX_ERR_DBG,0, 0, "GEN12_AUX_ERR_DBG"}, \ - {GEN12_GAM_DONE, 0, 0, "GEN12_GAM_DONE"}, \ - {GEN12_RING_FAULT_REG, 0, 0, "GEN12_RING_FAULT_REG"} + {GEN12_AUX_ERR_DBG,0, 0, "AUX_ERR_DBG"}, \ + {GEN12_GAM_DONE, 0, 0, "GAM_DONE"}, \ + {GEN12_RING_FAULT_REG, 0, 0, "FAULT_REG"} #define COMMON_BASE_ENGINE_INSTANCE() \ - {RING_PSMI_CTL(0), 0, 0, "RING_PSMI_CTL"}, \ - {RING_ESR(0), 0, 0, "RING_ESR"}, \ - {RING_DMA_FADD(0), 0, 0, "RING_DMA_FADD_LOW32"}, \ - {RING_DMA_FADD_UDW(0), 0, 0, "RING_DMA_FADD_UP32"}, \ - {RING_IPEIR(0),0, 0, "RING_IPEIR"}, \ - {RING_IPEHR(0),0, 0, "RING_IPEHR"}, \ - {RING_INSTPS(0), 0, 0, "RING_INSTPS"}, \ + {RING_PSMI_CTL(0), 0, 0, "RC PSMI"}, \ + {RING_ESR(0), 0, 0, "ESR"}, \ + {RING_DMA_FADD(0), 0, 0, "RING_DMA_FADD_LDW"}, \ + {RING_DMA_FADD_UDW(0), 0, 0, "RING_DMA_FADD_UDW"}, \ + {RING_IPEIR(0),0, 0, "IPEIR"}, \ + {RING_IPEHR(0),0, 0, "IPEHR"}, \ + {RING_INSTPS(0), 0, 0, "INSTPS"}, \ {RING_BBADDR(0), 0, 0, "RING_BBADDR_LOW32"}, \ {RING_BBADDR_UDW(0), 0, 0, "RING_BBADDR_UP32"}, \ - {RING_BBSTATE(0), 0, 0, "RING_BBSTATE"}, \ + {RING_BBSTATE(0), 0, 0, "BB_STATE"}, \ {CCID(0), 0, 0, "CCID"}, \ - {RING_ACTHD(0),0, 0, "RING_ACTHD_LOW32"}, \ - {RING_ACTHD_UDW(0),0, 0, "RING_ACTHD_UP32"}, \ - {RING_INSTPM(0), 0, 0, "RING_INSTPM"}, \ + {RING_ACTHD(0),0, 0, "ACTHD_LDW"}, \ + {RING_ACTHD_UDW(0),0, 0, "ACTHD_UDW"}, \ + {RING_INSTPM(0), 0, 0, "INSTPM"}, \ + {RING_INSTDONE(0), 0, 0, "INSTDONE"}, \ {RING_NOPID(0),0, 0, "RING_NOPID"}, \ - {RING_START(0),0, 0, "RING_START"}, \ - {RING_HEAD(0), 0, 0, "RING_HEAD"}, \ - {RING_TAIL(0), 0, 0, "RING_TAIL"}, \ - {RING_CTL(0), 0, 0, "RING_CTL"}, \ - {RING_MI_MODE(0), 0, 0, "RING_MI_MODE"}, \ + {RING_START(0),0, 0, "START"}, \ + {RING_HEAD(0), 0, 0, "HEAD"}, \ + {RING_TAIL(0), 0, 0, "TAIL"}, \ + {RING_CTL(0), 0, 0, "CTL"}, \ + {RING_MI_MODE(0), 0, 0, "MODE"}, \ {RING_CONTEXT_CONTROL(0), 0, 0, "RING_CONTEXT_CONTROL"}, \ - {RING_INSTDONE(0), 0, 0, "RING_INSTDONE"}, \ - {RING_HWS_PGA(0), 0, 0, "RING_HWS_PGA"}, \ - {RING_MODE_GEN7(0),0, 0, "RING_MODE_GEN7"}, \ - {GEN8_RING_PDP_LDW(0, 0), 0, 0, "GEN8_RING_PDP0_LDW"}, \ - {GEN8_RING_PDP_UDW(0, 0), 0, 0, "GEN8_RING_PDP0_UDW"}, \ - {GEN8_RING_PDP_LDW(0, 1), 0, 0, "GEN8_RING_PDP1_LDW"}, \ - {GEN8_RING_PDP_UDW(0, 1), 0, 0, "GEN8_RING_PDP1_UDW"}, \ - {GEN8_RING_PDP_LDW(0, 2), 0, 0, "GEN8_RING_PDP2_LDW"}, \ - {GEN8_RING_PDP_UDW(0, 2), 0, 0, "GEN8_RING_PDP2_UDW"}, \ - {GEN8_RING_PDP_LDW(0, 3), 0, 0, "GEN8_RING_PDP3_LDW"}, \ - {GEN8_RING_PDP_UDW(0, 3), 0, 0, "GEN8_RING_PDP3_UDW"} + {RING_HWS_PGA(0), 0, 0, "HWS"}, \ + {RING_MODE_GEN7(0),0, 0, "GFX_MODE"}, \ + {GEN8_RING_PDP_LDW(0, 0), 0, 0, "PDP0_LDW"}, \ + {GEN8_RING_PDP_UDW(0, 0), 0, 0, "PDP0_UDW"}, \ + {GEN8_RING_PDP_LDW(0, 1), 0, 0, "PDP1_LDW"}, \ + {GEN8_RING_PDP_UD
[Intel-gfx] [PATCH v5 00/10] Add GuC Error Capture Support
This series: 1. Enables support of GuC to execute error- state-capture based on a list of MMIO registers the driver registers and GuC will dump and report back right before a GuC triggered engine-reset event. 2. Updates the ADS blob creation to register lists of global and engine registers with GuC. 3. Defines tables of register lists that are global or engine class or engine instance in scope. 4. Separates GuC log-buffer access locks for relay logging vs the new region for the error state capture data. 5. Allocates an additional interim circular buffer store to copy snapshots of new GuC reported error-state-capture dumps in response to the G2H notification. 6. Connects the i915_gpu_coredump reporting function to the GuC error capture module to print all GuC error state capture dumps that is reported. This is the 5th rev of this series with the first 3 revs labelled as RFC. Prior receipts of rvb's: - Patch #5 has received R-v-b from Matthew Brost Changes from prior revs: v5: - Added Gen9->Gen11 register list for CI coverage that included Gen9 with GuC submission. - Redesigned the extraction of the GuC error-capture dumps by grouping them into complete per-engine-reset nodes. Complete here means each node includes the global, engine-class and engine-instance register lists in a single structure. - Extraction is decoupled from the print-out. We now do the extraction immediately when receiving the G2H for error-capture notification. A link list of nodes is maintained with a FIFO based threshold while awaiting retrieval from i915_gpu_coredump's capture_engine function. - Added new plumbing through the i915_gpu_coredump allocation and capture functions to include a flag that is used indicate that GuC had triggered the reset. This new plumbing guarantees an exact match from i915_gpu_coredump's per-engine vma recording and node-retrieval from the guc-error-capture. - Broke the coredump gt_global capture and recording functions into smaller subsets so we can reuse as much of the existing legacy register reading + printing functions and only rely on GuC error-capture for the smaller subset of registers that are tied to engine workload execution. - Updated the register list to follow the legacy execlist format of printout. v4: - Rebased on latest drm-tip that has been merged with the support of GuC firmware version 69.0.3 that is required for GuC error-state-catpure to work. - Added register list for DG2 which is the same as XE_LP except an additional steering register set. - Fixed a bug in the end of capture parsing loop in intel_guc_capture_out_print_next_group that was not properly comparing the engine-instance and engine- class being parsed against the one that triggered the i915_gpu_coredump. v3: - Fixed all review comments from rev2 except the following: - Michal Wajdeczko proposed adding a seperate function to lookup register string nameslookup (based on offset) but decided against it because of offset conflicts and the current table layout is easier to maintain. - Last set of checkpatch errors pertaining to "COMPLEX MACROS" should be fixed on next rev. - Abstracted internal-to-guc-capture information into a new __guc_state_capture_priv structure that allows the exclusion of intel_guc.h and intel_guc_fwif.h from intel_guc_capture.h. Now, only the first 2 patches have a wider build time impact because of the changes to intel_guc_fwif.h but subsequent changes to guc-capture internal structures or firmware interfaces used solely by guc-capture module shoudn't impact the rest of the driver build. - Added missing Gen12LP registers and added slice+subslice indices when reporting extended steered registers. - Add additional checks to ensure that the GuC reported error capture information matches the i915_gpu_coredump that is being printed before we print out the corresponding VMA dumps such as the batch buffer. v2: - Ignore - failed CI retest. Alan Previn (10): drm/i915/guc: Update GuC ADS size for error capture lists drm/i915/guc: Add XE_LP registers for GuC error state capture. drm/i915/guc: Add DG2 registers for GuC error state capture. drm/i915/guc: Add Gen9 registers for GuC error state capture. drm/i915/guc: Add GuC's error state capture output structures. drm/i915/guc: Update GuC's log-buffer-state access for error capture. drm/i915/guc: Extract GuC error capture lists on G2H notification. drm/i915/guc: Plumb GuC-capture into gpu_coredump drm/i915/guc: Fol
[Intel-gfx] [PATCH v5 02/10] drm/i915/guc: Add XE_LP registers for GuC error state capture.
Add device specific tables and register lists to cover different engines class types for GuC error state capture for XE_LP products. Also, add runtime allocation and freeing of extended register lists for registers that need steering identifiers that depend on the detected HW config. Signed-off-by: Alan Previn --- drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 2 + .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 207 +++--- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 4 +- 3 files changed, 180 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h index 15b8c02b8a76..a2f97d04ff18 100644 --- a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h @@ -24,6 +24,8 @@ struct __guc_mmio_reg_descr_group { u32 owner; /* see enum guc_capture_owner */ u32 type; /* see enum guc_capture_type */ u32 engine; /* as per MAX_ENGINE_CLASS */ + int num_ext; + struct __guc_mmio_reg_descr *ext; }; struct __guc_state_capture_priv { diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index 06873d617b8b..b6882074fc8d 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -19,40 +19,101 @@ * NOTE: For engine-registers, GuC only needs the register offsets * from the engine-mmio-base */ +#define COMMON_GEN12BASE_GLOBAL() \ + {GEN12_FAULT_TLB_DATA0,0, 0, "GEN12_FAULT_TLB_DATA0"}, \ + {GEN12_FAULT_TLB_DATA1,0, 0, "GEN12_FAULT_TLB_DATA1"}, \ + {FORCEWAKE_MT, 0, 0, "FORCEWAKE_MT"}, \ + {GEN12_AUX_ERR_DBG,0, 0, "GEN12_AUX_ERR_DBG"}, \ + {GEN12_GAM_DONE, 0, 0, "GEN12_GAM_DONE"}, \ + {GEN12_RING_FAULT_REG, 0, 0, "GEN12_RING_FAULT_REG"} + +#define COMMON_GEN12BASE_ENGINE_INSTANCE() \ + {RING_PSMI_CTL(0), 0, 0, "RING_PSMI_CTL"}, \ + {RING_ESR(0), 0, 0, "RING_ESR"}, \ + {RING_DMA_FADD(0), 0, 0, "RING_DMA_FADD_LOW32"}, \ + {RING_DMA_FADD_UDW(0), 0, 0, "RING_DMA_FADD_UP32"}, \ + {RING_IPEIR(0),0, 0, "RING_IPEIR"}, \ + {RING_IPEHR(0),0, 0, "RING_IPEHR"}, \ + {RING_INSTPS(0), 0, 0, "RING_INSTPS"}, \ + {RING_BBADDR(0), 0, 0, "RING_BBADDR_LOW32"}, \ + {RING_BBADDR_UDW(0), 0, 0, "RING_BBADDR_UP32"}, \ + {RING_BBSTATE(0), 0, 0, "RING_BBSTATE"}, \ + {CCID(0), 0, 0, "CCID"}, \ + {RING_ACTHD(0),0, 0, "RING_ACTHD_LOW32"}, \ + {RING_ACTHD_UDW(0),0, 0, "RING_ACTHD_UP32"}, \ + {RING_INSTPM(0), 0, 0, "RING_INSTPM"}, \ + {RING_NOPID(0),0, 0, "RING_NOPID"}, \ + {RING_START(0),0, 0, "RING_START"}, \ + {RING_HEAD(0), 0, 0, "RING_HEAD"}, \ + {RING_TAIL(0), 0, 0, "RING_TAIL"}, \ + {RING_CTL(0), 0, 0, "RING_CTL"}, \ + {RING_MI_MODE(0), 0, 0, "RING_MI_MODE"}, \ + {RING_CONTEXT_CONTROL(0), 0, 0, "RING_CONTEXT_CONTROL"}, \ + {RING_INSTDONE(0), 0, 0, "RING_INSTDONE"}, \ + {RING_HWS_PGA(0), 0, 0, "RING_HWS_PGA"}, \ + {RING_MODE_GEN7(0),0, 0, "RING_MODE_GEN7"}, \ + {GEN8_RING_PDP_LDW(0, 0), 0, 0, "GEN8_RING_PDP0_LDW"}, \ + {GEN8_RING_PDP_UDW(0, 0), 0, 0, "GEN8_RING_PDP0_UDW"}, \ + {GEN8_RING_PDP_LDW(0, 1), 0, 0, "GEN8_RING_PDP1_LDW"}, \ + {GEN8_RING_PDP_UDW(0, 1), 0, 0, "GEN8_RING_PDP1_UDW"}, \ + {GEN8_RING_PDP_LDW(0, 2), 0, 0, "GEN8_RING_PDP2_LDW"}, \ + {GEN8_RING_PDP_UDW(0, 2), 0, 0, "GEN8_RING_PDP2_UDW"}, \ + {GEN8_RING_PDP_LDW(0, 3), 0, 0, "GEN8_RING_PDP3_LDW"}, \ + {GEN8_RING_PDP_UDW(0, 3), 0, 0, "GEN8_RING_PDP3_UDW"} + +#define COMMON_GEN12BASE_HAS_EU() \ + {EIR, 0, 0, "EIR"} + +#define COMMON_GEN12BASE_RENDER() \ + {GEN7_SC_INSTDONE, 0, 0, "GEN7_SC_INSTDONE"}, \ + {GEN12_SC_INSTDONE_EXTRA, 0, 0, "GEN12_SC_INSTDONE_EXTRA"}, \ + {GEN12_SC_INSTDONE_EXTRA2, 0, 0, "GEN12_SC_INSTDONE_EXTRA2"} + +#define COMMON_GEN12BASE_VEC() \ + {GEN12_SFC_DONE(0),0, 0, "GEN12_SFC_DONE0"}, \ + {GEN12_SFC_DONE(1),0, 0, "GEN12_SFC_DONE1"}, \ + {GEN12_SFC_DONE(2),0, 0, "GEN12_SFC_DONE2"}, \ + {GEN12_SFC_DONE(3),0, 0, "GEN12_SFC_DONE3"} + /* XE_LPD - Global */ static struct __guc_mmio_reg_descr xe_lpd_global_regs[] = { - {GEN12_RING_FAULT_REG, 0, 0, "GEN12_RING_FAULT_REG"} + COMMON_GEN12BASE_GLOBAL(), }; /*
[Intel-gfx] [PATCH v5 03/10] drm/i915/guc: Add DG2 registers for GuC error state capture.
Add additional DG2 registers for GuC error state capture. Signed-off-by: Alan Previn --- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 64 ++- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c index b6882074fc8d..19719daffed4 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c @@ -179,19 +179,23 @@ static struct __ext_steer_reg xelpd_extregs[] = { {"GEN7_ROW_INSTDONE", GEN7_ROW_INSTDONE} }; +static struct __ext_steer_reg xehpg_extregs[] = { + {"XEHPG_INSTDONE_GEOM_SVG", XEHPG_INSTDONE_GEOM_SVG} +}; + static void -guc_capture_alloc_steered_list_xelpd(struct intel_guc *guc, -struct __guc_mmio_reg_descr_group *lists) +guc_capture_alloc_steered_list_xe_lpd_hpg(struct intel_guc *guc, + struct __guc_mmio_reg_descr_group *lists, + u32 ipver) { struct intel_gt *gt = guc_to_gt(guc); struct drm_i915_private *i915 = guc_to_gt(guc)->i915; struct sseu_dev_info *sseu; - int slice, subslice, i, num_tot_regs = 0; + int slice, subslice, i, iter, num_steer_regs, num_tot_regs = 0; struct __guc_mmio_reg_descr_group *list; struct __guc_mmio_reg_descr *extarray; - int num_steer_regs = ARRAY_SIZE(xelpd_extregs); - /* In XE_LP we only care about render-class steering registers during error-capture */ + /* In XE_LP / HPG we only have render-class steering registers during error-capture */ list = guc_capture_get_one_list(lists, GUC_CAPTURE_LIST_INDEX_PF, GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS, GUC_RENDER_CLASS); if (!list) @@ -200,10 +204,21 @@ guc_capture_alloc_steered_list_xelpd(struct intel_guc *guc, if (list->ext) return; /* already populated */ + num_steer_regs = ARRAY_SIZE(xelpd_extregs); + if (ipver >= IP_VER(12, 55)) + num_steer_regs += ARRAY_SIZE(xehpg_extregs); + sseu = >->info.sseu; - for_each_instdone_slice_subslice(i915, sseu, slice, subslice) { - num_tot_regs += num_steer_regs; + if (ipver >= IP_VER(12, 50)) { + for_each_instdone_gslice_dss_xehp(i915, sseu, iter, slice, subslice) { + num_tot_regs += num_steer_regs; + } + } else { + for_each_instdone_slice_subslice(i915, sseu, slice, subslice) { + num_tot_regs += num_steer_regs; + } } + if (!num_tot_regs) return; @@ -212,15 +227,31 @@ guc_capture_alloc_steered_list_xelpd(struct intel_guc *guc, return; extarray = list->ext; - for_each_instdone_slice_subslice(i915, sseu, slice, subslice) { - for (i = 0; i < num_steer_regs; i++) { - extarray->reg = xelpd_extregs[i].reg; - extarray->flags = FIELD_PREP(GUC_REGSET_STEERING_GROUP, slice); - extarray->flags |= FIELD_PREP(GUC_REGSET_STEERING_INSTANCE, subslice); - extarray->regname = xelpd_extregs[i].name; - ++extarray; + +#define POPULATE_NEXT_EXTREG(ext, list, idx, slicenum, subslicenum) \ + { \ + (ext)->reg = list[idx].reg; \ + (ext)->flags = FIELD_PREP(GUC_REGSET_STEERING_GROUP, slicenum); \ + (ext)->flags |= FIELD_PREP(GUC_REGSET_STEERING_INSTANCE, subslicenum); \ + (ext)->regname = xelpd_extregs[i].name; \ + ++(ext); \ + } + if (ipver >= IP_VER(12, 50)) { + for_each_instdone_gslice_dss_xehp(i915, sseu, iter, slice, subslice) { + for (i = 0; i < ARRAY_SIZE(xelpd_extregs); i++) + POPULATE_NEXT_EXTREG(extarray, xelpd_extregs, i, slice, subslice) + for (i = 0; i < ARRAY_SIZE(xehpg_extregs) && ipver >= IP_VER(12, 55); +i++) + POPULATE_NEXT_EXTREG(extarray, xehpg_extregs, i, slice, subslice) + } + } else { + for_each_instdone_slice_subslice(i915, sseu, slice, subslice) { + for (i = 0; i < num_steer_regs; i++) + POPULATE_NEXT_EXTREG(extarray, xelpd_extregs, i, slice, subslice) } } +#undef POPULATE_NEXT_EXTREG + list->num_ext = num_tot_regs; } @@ -237,7 +268,10 @@ guc_capture_get_device_reglist(struct intel_guc *guc) * these at init time based on hw config add it as an extension * list at the end of the pre-populated render list. */ - guc_capture_alloc_steered_list_xelpd(guc, xe_lpd_l
[Intel-gfx] [PATCH v5 01/10] drm/i915/guc: Update GuC ADS size for error capture lists
Update GuC ADS size allocation to include space for the lists of error state capture register descriptors. Also, populate the lists of registers we want GuC to report back to Host on engine reset events. This list should include global, engine-class and engine-instance registers for every engine-class type on the current hardware. NOTE: Start with a sample table of register lists to layout the framework before adding real registers in subsequent patch. Signed-off-by: Alan Previn --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 36 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.c| 13 +- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 11 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 36 +- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 450 ++ .../gpu/drm/i915/gt/uc/intel_guc_capture.h| 20 + drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 17 + 8 files changed, 555 insertions(+), 29 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index a26e6736bebb..236bcd6cd8ea 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -183,6 +183,7 @@ i915-y += gt/uc/intel_uc.o \ gt/uc/intel_uc_fw.o \ gt/uc/intel_guc.o \ gt/uc/intel_guc_ads.o \ + gt/uc/intel_guc_capture.o \ gt/uc/intel_guc_ct.o \ gt/uc/intel_guc_debugfs.o \ gt/uc/intel_guc_fw.o \ diff --git a/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h new file mode 100644 index ..15b8c02b8a76 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2021-2021 Intel Corporation + */ + +#ifndef _INTEL_GUC_CAPTURE_FWIF_H +#define _INTEL_GUC_CAPTURE_FWIF_H + +#include +#include "intel_guc_fwif.h" + +struct intel_guc; + +struct __guc_mmio_reg_descr { + i915_reg_t reg; + u32 flags; + u32 mask; + const char *regname; +}; + +struct __guc_mmio_reg_descr_group { + struct __guc_mmio_reg_descr *list; + u32 num_regs; + u32 owner; /* see enum guc_capture_owner */ + u32 type; /* see enum guc_capture_type */ + u32 engine; /* as per MAX_ENGINE_CLASS */ +}; + +struct __guc_state_capture_priv { + struct __guc_mmio_reg_descr_group *reglists; + u16 num_instance_regs[GUC_CAPTURE_LIST_INDEX_MAX][GUC_MAX_ENGINE_CLASSES]; + u16 num_class_regs[GUC_CAPTURE_LIST_INDEX_MAX][GUC_MAX_ENGINE_CLASSES]; + u16 num_global_regs[GUC_CAPTURE_LIST_INDEX_MAX]; +}; + +#endif /* _INTEL_GUC_CAPTURE_FWIF_H */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c index ba2a67f9e500..d035a3ba8700 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c @@ -8,8 +8,9 @@ #include "gt/intel_gt_irq.h" #include "gt/intel_gt_pm_irq.h" #include "intel_guc.h" -#include "intel_guc_slpc.h" #include "intel_guc_ads.h" +#include "intel_guc_capture.h" +#include "intel_guc_slpc.h" #include "intel_guc_submission.h" #include "i915_drv.h" #include "i915_irq.h" @@ -361,9 +362,14 @@ int intel_guc_init(struct intel_guc *guc) if (ret) goto err_fw; - ret = intel_guc_ads_create(guc); + ret = intel_guc_capture_init(guc); if (ret) goto err_log; + + ret = intel_guc_ads_create(guc); + if (ret) + goto err_capture; + GEM_BUG_ON(!guc->ads_vma); ret = intel_guc_ct_init(&guc->ct); @@ -402,6 +408,8 @@ int intel_guc_init(struct intel_guc *guc) intel_guc_ct_fini(&guc->ct); err_ads: intel_guc_ads_destroy(guc); +err_capture: + intel_guc_capture_destroy(guc); err_log: intel_guc_log_destroy(&guc->log); err_fw: @@ -429,6 +437,7 @@ void intel_guc_fini(struct intel_guc *guc) intel_guc_ct_fini(&guc->ct); intel_guc_ads_destroy(guc); + intel_guc_capture_destroy(guc); intel_guc_log_destroy(&guc->log); intel_uc_fw_fini(&guc->fw); } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 697d9d66acef..4e819853ec2e 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -9,18 +9,19 @@ #include #include -#include "intel_uncore.h" +#include "intel_guc_ct.h" #include "intel_guc_fw.h" #include "intel_guc_fwif.h" -#include "intel_guc_ct.h" #include "intel_guc_log.h" #include "intel_guc_reg.h" #include "intel_guc_slpc_types.h" #include "intel_uc_fw.h" +#include "intel_uncore.h" #include "i915_utils.h" #include "i915_vma.h" struct __guc_ads_blob; +struct __guc_state_c
[Intel-gfx] [PATCH] drm/i915/adlp: Fix TypeC PHY-ready status readout
The TCSS_DDI_STATUS register is indexed by tc_port not by the FIA port index, fix this up. This only caused an issue on TC#3/4 ports in legacy mode, as in all other cases the two indices either match (on TC#1/2) or the TCSS_DDI_STATUS_READY flag is set regardless of something being connected or not (on TC#1/2/3/4 in dp-alt and tbt-alt modes). Reported-and-tested-by: Chia-Lin Kao (AceLan) Fixes: 55ce306c2aa1 ("drm/i915/adl_p: Implement TC sequences") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4698 Cc: José Roberto de Souza Cc: # v5.14+ Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/display/intel_tc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 4eefe7b0bb263..3291124a99e5a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -346,10 +346,11 @@ static bool icl_tc_phy_status_complete(struct intel_digital_port *dig_port) static bool adl_tc_phy_status_complete(struct intel_digital_port *dig_port) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + enum tc_port tc_port = intel_port_to_tc(i915, dig_port->base.port); struct intel_uncore *uncore = &i915->uncore; u32 val; - val = intel_uncore_read(uncore, TCSS_DDI_STATUS(dig_port->tc_phy_fia_idx)); + val = intel_uncore_read(uncore, TCSS_DDI_STATUS(tc_port)); if (val == 0x) { drm_dbg_kms(&i915->drm, "Port %s: PHY in TCCOLD, assuming not complete\n", -- 2.27.0
Re: [Intel-gfx] [PATCH v2 09/11] drm: Convert open-coded yes/no strings to yesno()
On Wed, Jan 26, 2022 at 12:12:50PM +0200, Andy Shevchenko wrote: On Wed, Jan 26, 2022 at 11:39 AM Lucas De Marchi wrote: linux/string_helpers.h provides a helper to return "yes"/"no" strings. Replace the open coded versions with str_yes_no(). The places were oops, I replaced yesno() here but forgot to do so in the title identified with the following semantic patch: @@ expression b; @@ - b ? "yes" : "no" + str_yes_no(b) Then the includes were added, so we include-what-we-use, and parenthesis adjusted in drivers/gpu/drm/v3d/v3d_debugfs.c. After the conversion we still see the same binary sizes: textdata bss dec hex filename 511493295 212 54656d580 virtio/virtio-gpu.ko.old 511493295 212 54656d580 virtio/virtio-gpu.ko 1441491 60340 800 1502631 16eda7 radeon/radeon.ko.old 1441491 60340 800 1502631 16eda7 radeon/radeon.ko 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko.old 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko 411986 104906176 428652 68a6c drm.ko.old 411986 104906176 428652 68a6c drm.ko 981291636 264 100029 186bd dp/drm_dp_helper.ko.old 981291636 264 100029 186bd dp/drm_dp_helper.ko 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko.old 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko This probably won't change for modules, but if you compile in the linker may try to optimize it. Would be nice to see the old-new for `make allyesconfig` or equivalent. just like it would already do, no? I can try and see what happens, but my feeling is that we won't have any change. ... seq_printf(m, "\tDP branch device present: %s\n", - branch_device ? "yes" : "no"); + str_yes_no(branch_device)); Can it be now on one line? Same Q for all similar cases in the entire series. I saw that question in the previous version. I think those are very subjective is they all go a little bit over 80 chars. Some maintainers may prefer one way or the other. Here we are reducing just 3 chars so I assumed that is the preferred style here. Also keeping it as is helps with the mass conversion since it's easily repeatable if another iteration is needed. thanks Lucas De Marchi
Re: [Intel-gfx] [PATCH 3/4] drm/i915/execlists: Fix execlists request cancellation corner case
On 25/01/2022 16:32, Matthew Brost wrote: On Tue, Jan 25, 2022 at 03:27:31PM +, Tvrtko Ursulin wrote: On 24/01/2022 15:01, Matthew Brost wrote: More than 1 request can be submitted to a single ELSP at a time if multiple requests are ready run to on the same context. When a request is canceled it is marked bad, an idle pulse is triggered to the engine (high priority kernel request), the execlists scheduler sees that running request is bad and sets preemption timeout to minimum value (1 ms). This fails to work if multiple requests are combined on the ELSP as only the most recent request is stored in the execlists schedule (the request stored in the ELSP isn't marked bad, thus preemption timeout isn't set to the minimum value). If the preempt timeout is configured to zero, the engine is permanently hung. This is shown by an upcoming selftest. To work around this, mark the idle pulse with a flag to force a preempt with the minimum value. A couple of quick questions first before I find time to dig deeper. First about the "permanently hung" statement. How permanent? Does the heartbeat eventually resolve it and if not why not? Naive view is that missed heartbeats would identify the stuck non-preemptible request and then engine reset would skip over it. Yes, if the heartbeat is enabled then the heartbeat would eventually recover the engine. It is not always enabled though... If it does resolve, then the problem is only that request timeout works less well if someone set preempt timeout to zero? Which may not be as bad, since request timeout was never about any time guarantees. Yes, if the heartbeat is enabled the problem isn't as bad. Good, so commit message needs some work to be accurate. On the technical side of the patch it looks reasonable to me. And the idea that cancellation pulse is made special also sounds plausible. Question is whether we want to add code to support this considering the opens I have: 1) Do we care about request cancellation working for non-preemptible batches, *if* someone explicitly disabled both preemption timeout and hearbteats? 2) Do we care to improve the responsiveness of request cancellation if only preempt timeout was disabled? Conclusions here will also dictate whether Fixes: tag is warranted. Best to avoid hairy backports if we decide it is not really needed. Also, in the next patch you change one selftest to only run with preempt timeout disabled. I think it makes sense to have this test run in the default config (preempt timeout enabled) to reflect the typical configuration. You may add a second pass with it disabled to execise the corner case, again, depending on conclusions after above questions. Regards, Tvrtko Matt Regards, Tvrtko Fixes: 38b237eab2bc7 ("drm/i915: Individual request cancellation") Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gt/intel_engine_heartbeat.c | 23 +++ .../gpu/drm/i915/gt/intel_engine_heartbeat.h | 1 + .../drm/i915/gt/intel_execlists_submission.c | 18 ++- drivers/gpu/drm/i915/i915_request.h | 6 + 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c index a3698f611f457..efd1c719b4072 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c @@ -243,7 +243,8 @@ void intel_engine_init_heartbeat(struct intel_engine_cs *engine) INIT_DELAYED_WORK(&engine->heartbeat.work, heartbeat); } -static int __intel_engine_pulse(struct intel_engine_cs *engine) +static int __intel_engine_pulse(struct intel_engine_cs *engine, + bool force_preempt) { struct i915_sched_attr attr = { .priority = I915_PRIORITY_BARRIER }; struct intel_context *ce = engine->kernel_context; @@ -258,6 +259,8 @@ static int __intel_engine_pulse(struct intel_engine_cs *engine) return PTR_ERR(rq); __set_bit(I915_FENCE_FLAG_SENTINEL, &rq->fence.flags); + if (force_preempt) + __set_bit(I915_FENCE_FLAG_FORCE_PREEMPT, &rq->fence.flags); heartbeat_commit(rq, &attr); GEM_BUG_ON(rq->sched.attr.priority < I915_PRIORITY_BARRIER); @@ -299,7 +302,7 @@ int intel_engine_set_heartbeat(struct intel_engine_cs *engine, /* recheck current execution */ if (intel_engine_has_preemption(engine)) { - err = __intel_engine_pulse(engine); + err = __intel_engine_pulse(engine, false); if (err) set_heartbeat(engine, saved); } @@ -312,7 +315,8 @@ int intel_engine_set_heartbeat(struct intel_engine_cs *engine, return err; } -int intel_engine_pulse(struct intel_engine_cs *engine) +static int _intel_engine_pulse(struct intel_engine_cs *engine, +
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Add request cancel low level trace point
On 25/01/2022 16:39, Matthew Brost wrote: On Tue, Jan 25, 2022 at 12:27:43PM +, Tvrtko Ursulin wrote: On 24/01/2022 15:01, Matthew Brost wrote: Add request cancel trace point guarded by CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINT. Okay-ish I guess (There is pr_notice with the only real caller, but I suppose you want it for selftests? Oh yes, why is missing from the commit message.), but I'd emit it from i915_request_cancel since that's what the tracepoint is called so it fits. I had this tracepoint at one point but somehow during the upstreaming it got lost. Noticed when debugging the below issue this tracepoint wasn't present, so I brought it back in. I generally use low level tracepoints for debug, so a pr_notice doesn't really help there. Link: https://gitlab.freedesktop.org/drm/intel/-/issues/4960 This was a GuC backend bug right? And log shows this: <7> [275.431050] [drm:eb_lookup_vmas [i915]] EINVAL at eb_validate_vma:514 <5> [295.433920] Fence expiration time out i915-:03:00.0:kms_vblank[1038]:2! <3> [332.736763] INFO: task kworker/2:1:55 blocked for more than 30 seconds. So pr_notice worked. I am not opposed to the tracepoint just put a solid why in the commit message and if the tracepoint is called i915_request_cancel it should be emitted from i915_request_cancel(). Regards, Tvrtko Matt Regards, Tvrtko Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_context.h | 1 + drivers/gpu/drm/i915/i915_trace.h | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index d8c74bbf9aae2..3aed4d77f116c 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -124,6 +124,7 @@ intel_context_is_pinned(struct intel_context *ce) static inline void intel_context_cancel_request(struct intel_context *ce, struct i915_request *rq) { + trace_i915_request_cancel(rq); GEM_BUG_ON(!ce->ops->cancel_request); return ce->ops->cancel_request(ce, rq); } diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 37b5c9e9d260e..d0a11a8bb0ca3 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -324,6 +324,11 @@ DEFINE_EVENT(i915_request, i915_request_add, ); #if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS) +DEFINE_EVENT(i915_request, i915_request_cancel, +TP_PROTO(struct i915_request *rq), +TP_ARGS(rq) +); + DEFINE_EVENT(i915_request, i915_request_guc_submit, TP_PROTO(struct i915_request *rq), TP_ARGS(rq) @@ -497,6 +502,11 @@ DEFINE_EVENT(intel_context, intel_context_do_unpin, #else #if !defined(TRACE_HEADER_MULTI_READ) +static inline void +trace_i915_request_cancel(struct i915_request *rq) +{ +} + static inline void trace_i915_request_guc_submit(struct i915_request *rq) {
Re: [Intel-gfx] [PATCH v2 00/11] lib/string_helpers: Add a few string helpers
On Wed, Jan 26, 2022 at 11:39 AM Lucas De Marchi wrote: > > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nik...@intel.com/ > 2) > https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevche...@linux.intel.com/#t > > Now there is also the v1 of this same patch series: > https://lore.kernel.org/all/20220119072450.2890107-1-lucas.demar...@intel.com/ > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > a. This version should be a drop-in replacement for what is currently in >the tree, with no change in behavior or binary size. For binary >size what I checked was that the linked objects in the end have the >same size (gcc 11). From comments in the previous attempts this seems >also the case for earlier compiler versions > > b. I didn't change the function name to choice_* as suggested by Andrew >Morton in 20191023155619.43e0013f0c8c673a5c508...@linux-foundation.org >because other people argumented in favor of shorter names for these >simple helpers - if they are long and people simply not use due to >that, we failed. However as pointed out in v1 of this patchseries, >onoff(), yesno(), enabledisable(), enableddisabled() have some >issues: the last 2 are hard to read and for the first 2 it would not >be hard to have the symbol to clash with variable names. >From comments in v1, most people were in favor (or at least not >opposed) to using str_on_off(), str_yes_no(), str_enable_disable() >and str_enabled_disabled(). > > c. Use string_helper.h for these helpers - pulling string.h in the >compilations units was one of the concerns and I think re-using this >already existing header is better than creating a new string-choice.h > > d. One alternative to all of this suggested by Christian König >(43456ba7-c372-84cc-4949-dcb817188...@amd.com) would be to add a >printk format. But besides the comment, he also seemed to like >the common function. This brought the argument from others that the >simple yesno()/enabledisable() already used in the code (or new >renamed version) is easier to remember and use than e.g. %py[DOY] I do not see any impediments to this series to be pulled. Thanks for the work you've done! > Changes in v2: > > - Use str_ prefix and separate other words with underscore: it's a > little bit longer, but should improve readability > > - Patches we re-split due to the rename: first patch adds all the new > functions, then additional patches try to do one conversion at a > time. While doing so, there were some fixes for issues already > present along the way > > - Style suggestions from v1 were adopted > > In v1 it was suggested to apply this in drm-misc. I will leave this to > maintainers to decide: maybe it would be simpler to merge the first > patches on drm-intel-next, wait for the back merge and merge the rest > through drm-misc - my fear is a big conflict with other work going in > drm-intel-next since the bulk of the rename is there. > > I tried to figure out acks and reviews from v1 and apply them to how the > patches are now split. > > thanks > Lucas De Marchi > > Lucas De Marchi (11): > lib/string_helpers: Consolidate string helpers implementation > drm/i915: Fix trailing semicolon > drm/i915: Use str_yes_no() > drm/i915: Use str_enable_disable() > drm/i915: Use str_enabled_disabled() > drm/i915: Use str_on_off() > drm/amd/display: Use str_yes_no() > drm/gem: Sort includes alphabetically > drm: Convert open-coded yes/no strings to yesno() > tomoyo: Use str_yes_no() > cxgb4: Use str_yes_no() > > drivers/gpu/drm/amd/amdgpu/atom.c | 4 +- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 14 +- > drivers/gpu/drm/dp/drm_dp.c | 3 +- > drivers/gpu/drm/drm_client_modeset.c | 3 +- > drivers/gpu/drm/drm_gem.c | 23 +- > drivers/gpu/drm/i915/display/g4x_dp.c | 6 +- > .../gpu/drm/i915/display/intel_backlight.c| 3 +- > drivers/gpu/drm/i915/display/intel_ddi.c | 4 +- > drivers/gpu/drm/i915/display/intel_display.c | 46 ++-- > .../drm/i915/display/intel_display_debugfs.c | 74 +++--- > .../drm/i915/display/intel_display_power.c| 4 +- > .../drm/i915/display/intel_display_trace.h| 9 +- > drivers/gpu/drm/i915/display/intel_dp.c | 20 +- > drivers/gpu/drm/i915/display/intel_dpll.c | 3 +- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 +- > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 7 +- > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +- > drivers/gpu/drm/i915/display/intel_fdi.c | 8 +- > drivers/gpu/
Re: [Intel-gfx] [PATCH 1/2] drm/i915/pmu: Use PM timestamp instead of RING TIMESTAMP for reference
On 25/01/2022 05:47, Jani Nikula wrote: On Mon, 24 Jan 2022, Umesh Nerlige Ramappa wrote: All timestamps returned by GuC for GuC PMU busyness are captured from GUC PM TIMESTAMP. Since this timestamp does not tick when GuC goes idle, kmd uses RING_TIMESTAMP to measure busyness of an engine with an active context. In further stress testing, the MMIO read of the RING_TIMESTAMP is seen to cause a rare hang. Resolve the issue by using gt specific timestamp from PM which is in sync with the GuC PM timestamp. Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to pmu") Signed-off-by: Umesh Nerlige Ramappa Reviewed-by: Alan Previn --- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 5 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 56 ++- drivers/gpu/drm/i915/i915_reg.h | 3 +- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index d59bbf49d1c2..697d9d66acef 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -215,6 +215,11 @@ struct intel_guc { * context usage for overflows. */ struct delayed_work work; + + /** +* @shift: Right shift value for the gpm timestamp +*/ + u32 shift; } timestamp; #ifdef CONFIG_DRM_I915_SELFTEST diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 1331ff91c5b0..66760f5df0c1 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1150,23 +1150,51 @@ static void guc_update_engine_gt_clks(struct intel_engine_cs *engine) } } -static void guc_update_pm_timestamp(struct intel_guc *guc, - struct intel_engine_cs *engine, - ktime_t *now) +static u32 gpm_timestamp_shift(struct intel_gt *gt) { - u32 gt_stamp_now, gt_stamp_hi; + intel_wakeref_t wakeref; + u32 reg, shift; + + with_intel_runtime_pm(gt->uncore->rpm, wakeref) + reg = intel_uncore_read(gt->uncore, RPM_CONFIG0); + + shift = (reg & GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK) >> + GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT; + + return 3 - shift; +} + +static u64 gpm_timestamp(struct intel_gt *gt) +{ + u32 lo, hi, old_hi, loop = 0; + + hi = intel_uncore_read(gt->uncore, MISC_STATUS1); + do { + lo = intel_uncore_read(gt->uncore, MISC_STATUS0); + old_hi = hi; + hi = intel_uncore_read(gt->uncore, MISC_STATUS1); + } while (old_hi != hi && loop++ < 2); + + return ((u64)hi << 32) | lo; +} See intel_uncore_read64_2x32(). This was a good suggestion - since the patch was merged regardless I think a follow up cleanup is in order so we don't accumulate duplicated code. Regards, Tvrtko + +static void guc_update_pm_timestamp(struct intel_guc *guc, ktime_t *now) +{ + struct intel_gt *gt = guc_to_gt(guc); + u32 gt_stamp_lo, gt_stamp_hi; + u64 gpm_ts; lockdep_assert_held(&guc->timestamp.lock); gt_stamp_hi = upper_32_bits(guc->timestamp.gt_stamp); - gt_stamp_now = intel_uncore_read(engine->uncore, -RING_TIMESTAMP(engine->mmio_base)); + gpm_ts = gpm_timestamp(gt) >> guc->timestamp.shift; + gt_stamp_lo = lower_32_bits(gpm_ts); *now = ktime_get(); - if (gt_stamp_now < lower_32_bits(guc->timestamp.gt_stamp)) + if (gt_stamp_lo < lower_32_bits(guc->timestamp.gt_stamp)) gt_stamp_hi++; - guc->timestamp.gt_stamp = ((u64)gt_stamp_hi << 32) | gt_stamp_now; + guc->timestamp.gt_stamp = ((u64)gt_stamp_hi << 32) | gt_stamp_lo; } /* @@ -1210,7 +1238,7 @@ static ktime_t guc_engine_busyness(struct intel_engine_cs *engine, ktime_t *now) stats_saved = *stats; gt_stamp_saved = guc->timestamp.gt_stamp; guc_update_engine_gt_clks(engine); - guc_update_pm_timestamp(guc, engine, now); + guc_update_pm_timestamp(guc, now); intel_gt_pm_put_async(gt); if (i915_reset_count(gpu_error) != reset_count) { *stats = stats_saved; @@ -1242,8 +1270,8 @@ static void __reset_guc_busyness_stats(struct intel_guc *guc) spin_lock_irqsave(&guc->timestamp.lock, flags); + guc_update_pm_timestamp(guc, &unused); for_each_engine(engine, gt, id) { - guc_update_pm_timestamp(guc, engine, &unused); guc_update_engine_gt_clks(engine); engine->stats.guc.prev_total = 0; } @@ -1260,10 +1288,11 @@ static void __update_guc_busyness_stats(struct intel_guc *guc) ktime_t
Re: [Intel-gfx] [PATCH v2 09/11] drm: Convert open-coded yes/no strings to yesno()
On Wed, Jan 26, 2022 at 11:39 AM Lucas De Marchi wrote: > > linux/string_helpers.h provides a helper to return "yes"/"no" strings. > Replace the open coded versions with str_yes_no(). The places were > identified with the following semantic patch: > > @@ > expression b; > @@ > > - b ? "yes" : "no" > + str_yes_no(b) > > Then the includes were added, so we include-what-we-use, and parenthesis > adjusted in drivers/gpu/drm/v3d/v3d_debugfs.c. After the conversion we > still see the same binary sizes: > >textdata bss dec hex filename > 511493295 212 54656d580 virtio/virtio-gpu.ko.old > 511493295 212 54656d580 virtio/virtio-gpu.ko > 1441491 60340 800 1502631 16eda7 radeon/radeon.ko.old > 1441491 60340 800 1502631 16eda7 radeon/radeon.ko > 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko.old > 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko > 411986 104906176 428652 68a6c drm.ko.old > 411986 104906176 428652 68a6c drm.ko > 981291636 264 100029 186bd dp/drm_dp_helper.ko.old > 981291636 264 100029 186bd dp/drm_dp_helper.ko > 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko.old > 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko This probably won't change for modules, but if you compile in the linker may try to optimize it. Would be nice to see the old-new for `make allyesconfig` or equivalent. ... > seq_printf(m, "\tDP branch device present: %s\n", > - branch_device ? "yes" : "no"); > + str_yes_no(branch_device)); Can it be now on one line? Same Q for all similar cases in the entire series. -- With Best Regards, Andy Shevchenko
Re: [Intel-gfx] [PATCH][next] drm/i915/guc: Use struct_size() helper in kmalloc()
On 25/01/2022 18:07, Gustavo A. R. Silva wrote: Make use of the struct_size() helper instead of an open-coded version, in order to avoid any potential type mistakes or integer overflows that, in the worst scenario, could lead to heap overflows. Also, address the following sparse warnings: drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:792:23: warning: using sizeof on a flexible structure Link: https://github.com/KSPP/linux/issues/174 Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index aa6dd6415202..e352a1aad228 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -789,7 +789,7 @@ static struct ct_incoming_msg *ct_alloc_msg(u32 num_dwords) { struct ct_incoming_msg *msg; - msg = kmalloc(sizeof(*msg) + sizeof(u32) * num_dwords, GFP_ATOMIC); + msg = kmalloc(struct_size(msg, msg, num_dwords), GFP_ATOMIC); if (msg) msg->size = num_dwords; return msg; Reviewed-by: Tvrtko Ursulin Thanks for the patch, will merge shortly. Regards, Tvrtko
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/5] drm/i915: Skip dsc readout if the transcoder is disabled (rev3)
== Series Details == Series: series starting with [1/5] drm/i915: Skip dsc readout if the transcoder is disabled (rev3) URL : https://patchwork.freedesktop.org/series/99276/ State : success == Summary == CI Bug Log - changes from CI_DRM_11137_full -> Patchwork_22107_full Summary --- **SUCCESS** No regressions found. Participating hosts (10 -> 10) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_22107_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_balancer@parallel-balancer: - shard-iclb: [PASS][1] -> [SKIP][2] ([i915#4525]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/shard-iclb4/igt@gem_exec_balan...@parallel-balancer.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-iclb6/igt@gem_exec_balan...@parallel-balancer.html * igt@gem_exec_fair@basic-pace-share@rcs0: - shard-glk: [PASS][3] -> [FAIL][4] ([i915#2842]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/shard-glk8/igt@gem_exec_fair@basic-pace-sh...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-glk3/igt@gem_exec_fair@basic-pace-sh...@rcs0.html * igt@gem_exec_fair@basic-pace@vcs1: - shard-iclb: NOTRUN -> [FAIL][5] ([i915#2842]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-iclb2/igt@gem_exec_fair@basic-p...@vcs1.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-iclb: [PASS][6] -> [FAIL][7] ([i915#2849]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_suspend@basic-s3@smem: - shard-kbl: NOTRUN -> [DMESG-WARN][8] ([i915#180]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-kbl7/igt@gem_exec_suspend@basic...@smem.html * igt@gem_huc_copy@huc-copy: - shard-tglb: [PASS][9] -> [SKIP][10] ([i915#2190]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/shard-tglb3/igt@gem_huc_c...@huc-copy.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-tglb6/igt@gem_huc_c...@huc-copy.html * igt@gem_lmem_swapping@random: - shard-apl: NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4613]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-apl1/igt@gem_lmem_swapp...@random.html * igt@gem_pxp@protected-raw-src-copy-not-readible: - shard-tglb: NOTRUN -> [SKIP][12] ([i915#4270]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-tglb5/igt@gem_...@protected-raw-src-copy-not-readible.html * igt@gem_render_copy@linear-to-vebox-y-tiled: - shard-apl: NOTRUN -> [SKIP][13] ([fdo#109271]) +57 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-apl2/igt@gem_render_c...@linear-to-vebox-y-tiled.html * igt@gem_userptr_blits@coherency-sync: - shard-tglb: NOTRUN -> [SKIP][14] ([fdo#110542]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-tglb5/igt@gem_userptr_bl...@coherency-sync.html * igt@gem_userptr_blits@input-checking: - shard-apl: NOTRUN -> [DMESG-WARN][15] ([i915#4990]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-apl1/igt@gem_userptr_bl...@input-checking.html * igt@gem_userptr_blits@readonly-unsync: - shard-tglb: NOTRUN -> [SKIP][16] ([i915#3297]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-tglb5/igt@gem_userptr_bl...@readonly-unsync.html * igt@gem_userptr_blits@vma-merge: - shard-apl: NOTRUN -> [FAIL][17] ([i915#3318]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-apl2/igt@gem_userptr_bl...@vma-merge.html - shard-kbl: NOTRUN -> [FAIL][18] ([i915#3318]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-kbl4/igt@gem_userptr_bl...@vma-merge.html * igt@gem_workarounds@suspend-resume-context: - shard-apl: [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +2 similar issues [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/shard-apl6/igt@gem_workarou...@suspend-resume-context.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-apl2/igt@gem_workarou...@suspend-resume-context.html * igt@gen9_exec_parse@batch-zero-length: - shard-tglb: NOTRUN -> [SKIP][21] ([i915#2527] / [i915#2856]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22107/shard-tglb5/igt@gen9_exec_pa...@batch-zero-length.html * igt@i915_pm_dc@dc6-psr: - shard-iclb: [PASS][22] -> [FAIL][23] ([i915#454]) [22]: htt
Re: [Intel-gfx] [PATCH 17/54] gpu: drm: replace cpumask_weight with cpumask_empty where appropriate
On 25/01/2022 18:16, Yury Norov wrote: On Tue, Jan 25, 2022 at 1:28 AM Tvrtko Ursulin wrote: On 23/01/2022 18:38, Yury Norov wrote: i915_pmu_cpu_online() calls cpumask_weight() to check if any bit of a given cpumask is set. We can do it more efficiently with cpumask_empty() because cpumask_empty() stops traversing the cpumask as soon as it finds first set bit, while cpumask_weight() counts all bits unconditionally. Signed-off-by: Yury Norov --- drivers/gpu/drm/i915/i915_pmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index ea655161793e..1894c876b31d 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -1048,7 +1048,7 @@ static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node) GEM_BUG_ON(!pmu->base.event_init); /* Select the first online CPU as a designated reader. */ - if (!cpumask_weight(&i915_pmu_cpumask)) + if (cpumask_empty(&i915_pmu_cpumask)) cpumask_set_cpu(cpu, &i915_pmu_cpumask); return 0; Reviewed-by: Tvrtko Ursulin I see it's a large series which only partially appeared on our mailing lists. The series is here: https://lkml.org/lkml/2022/1/23/223 The branch: https://github.com/norov/linux/tree/bitmap-20220123 So for instance it hasn't got tested by our automated CI. (Not that I expect any problems in this patch.) Would be great if you give a test for the whole series, thanks! Can't really test the whole series for you, but if you want to send just the i915 patch standalone to the intel-gfx mailing list, that would trigger the CI run and if that passes we can merge that single one. What are the plans in terms of which tree will it get merged through? For the patches that will not be merged by maintainers of corresponding subsystems, I'll use my bitmap branch and send it to linux-next. Or I guess we can wait for them to trickle back to us this way. Regards, Tvrtko
[Intel-gfx] [PATCH v2 10/11] tomoyo: Use str_yes_no()
Remove the local yesno() implementation and adopt the str_yes_no() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi Reviewed-by: Sakari Ailus --- security/tomoyo/audit.c | 2 +- security/tomoyo/common.c | 19 +-- security/tomoyo/common.h | 1 - 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c index d79bf07e16be..023bedd9dfa3 100644 --- a/security/tomoyo/audit.c +++ b/security/tomoyo/audit.c @@ -166,7 +166,7 @@ static char *tomoyo_print_header(struct tomoyo_request_info *r) "#%04u/%02u/%02u %02u:%02u:%02u# profile=%u mode=%s granted=%s (global-pid=%u) task={ pid=%u ppid=%u uid=%u gid=%u euid=%u egid=%u suid=%u sgid=%u fsuid=%u fsgid=%u }", stamp.year, stamp.month, stamp.day, stamp.hour, stamp.min, stamp.sec, r->profile, tomoyo_mode[r->mode], - tomoyo_yesno(r->granted), gpid, tomoyo_sys_getpid(), + str_yes_no(r->granted), gpid, tomoyo_sys_getpid(), tomoyo_sys_getppid(), from_kuid(&init_user_ns, current_uid()), from_kgid(&init_user_ns, current_gid()), diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index 5c64927bf2b3..ff17abc96e5c 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "common.h" /* String table for operation mode. */ @@ -174,16 +175,6 @@ static bool tomoyo_manage_by_non_root; /* Utility functions. */ -/** - * tomoyo_yesno - Return "yes" or "no". - * - * @value: Bool value. - */ -const char *tomoyo_yesno(const unsigned int value) -{ - return value ? "yes" : "no"; -} - /** * tomoyo_addprintf - strncat()-like-snprintf(). * @@ -730,8 +721,8 @@ static void tomoyo_print_config(struct tomoyo_io_buffer *head, const u8 config) { tomoyo_io_printf(head, "={ mode=%s grant_log=%s reject_log=%s }\n", tomoyo_mode[config & 3], -tomoyo_yesno(config & TOMOYO_CONFIG_WANT_GRANT_LOG), -tomoyo_yesno(config & TOMOYO_CONFIG_WANT_REJECT_LOG)); +str_yes_no(config & TOMOYO_CONFIG_WANT_GRANT_LOG), +str_yes_no(config & TOMOYO_CONFIG_WANT_REJECT_LOG)); } /** @@ -1354,8 +1345,8 @@ static bool tomoyo_print_condition(struct tomoyo_io_buffer *head, case 3: if (cond->grant_log != TOMOYO_GRANTLOG_AUTO) tomoyo_io_printf(head, " grant_log=%s", -tomoyo_yesno(cond->grant_log == - TOMOYO_GRANTLOG_YES)); +str_yes_no(cond->grant_log == + TOMOYO_GRANTLOG_YES)); tomoyo_set_lf(head); return true; } diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h index 85246b9df7ca..ca285f362705 100644 --- a/security/tomoyo/common.h +++ b/security/tomoyo/common.h @@ -959,7 +959,6 @@ char *tomoyo_read_token(struct tomoyo_acl_param *param); char *tomoyo_realpath_from_path(const struct path *path); char *tomoyo_realpath_nofollow(const char *pathname); const char *tomoyo_get_exe(void); -const char *tomoyo_yesno(const unsigned int value); const struct tomoyo_path_info *tomoyo_compare_name_union (const struct tomoyo_path_info *name, const struct tomoyo_name_union *ptr); const struct tomoyo_path_info *tomoyo_get_domainname -- 2.34.1
[Intel-gfx] [PATCH v2 11/11] cxgb4: Use str_yes_no()
Remove the local yesno() implementation and adopt the str_yes_no() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi --- .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c| 249 ++ 1 file changed, 137 insertions(+), 112 deletions(-) diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c index 7d49fd4edc9e..f0d9842962ab 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c @@ -2015,17 +2015,6 @@ static const struct file_operations rss_debugfs_fops = { /* RSS Configuration. */ -/* Small utility function to return the strings "yes" or "no" if the supplied - * argument is non-zero. - */ -static const char *yesno(int x) -{ - static const char *yes = "yes"; - static const char *no = "no"; - - return x ? yes : no; -} - static int rss_config_show(struct seq_file *seq, void *v) { struct adapter *adapter = seq->private; @@ -2039,51 +2028,64 @@ static int rss_config_show(struct seq_file *seq, void *v) rssconf = t4_read_reg(adapter, TP_RSS_CONFIG_A); seq_printf(seq, "TP_RSS_CONFIG: %#x\n", rssconf); - seq_printf(seq, " Tnl4TupEnIpv6: %3s\n", yesno(rssconf & - TNL4TUPENIPV6_F)); - seq_printf(seq, " Tnl2TupEnIpv6: %3s\n", yesno(rssconf & - TNL2TUPENIPV6_F)); - seq_printf(seq, " Tnl4TupEnIpv4: %3s\n", yesno(rssconf & - TNL4TUPENIPV4_F)); - seq_printf(seq, " Tnl2TupEnIpv4: %3s\n", yesno(rssconf & - TNL2TUPENIPV4_F)); - seq_printf(seq, " TnlTcpSel: %3s\n", yesno(rssconf & TNLTCPSEL_F)); - seq_printf(seq, " TnlIp6Sel: %3s\n", yesno(rssconf & TNLIP6SEL_F)); - seq_printf(seq, " TnlVrtSel: %3s\n", yesno(rssconf & TNLVRTSEL_F)); - seq_printf(seq, " TnlMapEn: %3s\n", yesno(rssconf & TNLMAPEN_F)); - seq_printf(seq, " OfdHashSave: %3s\n", yesno(rssconf & - OFDHASHSAVE_F)); - seq_printf(seq, " OfdVrtSel: %3s\n", yesno(rssconf & OFDVRTSEL_F)); - seq_printf(seq, " OfdMapEn: %3s\n", yesno(rssconf & OFDMAPEN_F)); - seq_printf(seq, " OfdLkpEn: %3s\n", yesno(rssconf & OFDLKPEN_F)); - seq_printf(seq, " Syn4TupEnIpv6: %3s\n", yesno(rssconf & - SYN4TUPENIPV6_F)); - seq_printf(seq, " Syn2TupEnIpv6: %3s\n", yesno(rssconf & - SYN2TUPENIPV6_F)); - seq_printf(seq, " Syn4TupEnIpv4: %3s\n", yesno(rssconf & - SYN4TUPENIPV4_F)); - seq_printf(seq, " Syn2TupEnIpv4: %3s\n", yesno(rssconf & - SYN2TUPENIPV4_F)); - seq_printf(seq, " Syn4TupEnIpv6: %3s\n", yesno(rssconf & - SYN4TUPENIPV6_F)); - seq_printf(seq, " SynIp6Sel: %3s\n", yesno(rssconf & SYNIP6SEL_F)); - seq_printf(seq, " SynVrt6Sel:%3s\n", yesno(rssconf & SYNVRTSEL_F)); - seq_printf(seq, " SynMapEn: %3s\n", yesno(rssconf & SYNMAPEN_F)); - seq_printf(seq, " SynLkpEn: %3s\n", yesno(rssconf & SYNLKPEN_F)); - seq_printf(seq, " ChnEn: %3s\n", yesno(rssconf & - CHANNELENABLE_F)); - seq_printf(seq, " PrtEn: %3s\n", yesno(rssconf & - PORTENABLE_F)); - seq_printf(seq, " TnlAllLkp: %3s\n", yesno(rssconf & - TNLALLLOOKUP_F)); - seq_printf(seq, " VrtEn: %3s\n", yesno(rssconf & - VIRTENABLE_F)); - seq_printf(seq, " CngEn: %3s\n", yesno(rssconf & - CONGESTIONENABLE_F)); - seq_printf(seq, " HashToeplitz: %3s\n", yesno(rssconf & - HASHTOEPLITZ_F)); - seq_printf(seq, " Udp4En:%3s\n", yesno(rssconf & UDPENABLE_F)); - seq_printf(seq, " Disable: %3s\n", yesno(rssconf & DISABLE_F)); + seq_printf(seq, " Tnl4TupEnIpv6: %3s\n", + str_yes_no(rssconf & TNL4TUPENIPV6_F)); + seq_printf(seq, " Tnl2TupEnIpv6: %3s\n", + str_yes_no(rssconf & TNL2TUPENIPV6_F)); + seq_printf(seq, " Tnl4TupEnIpv4: %3s\n", + str_yes_no(rssconf & TNL4TUPENIPV4_F)); + seq_printf(seq, " Tnl2TupEnIpv4: %3s\n", + str_yes_no(rssconf & TNL2TUPENIPV4_F)); + seq_printf(seq, " TnlTcpSel: %3s\n", +
[Intel-gfx] [PATCH v2 08/11] drm/gem: Sort includes alphabetically
Sort includes alphabetically so it's easier to add/remove includes and know when that is needed. Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/drm_gem.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 4dcdec6487bb..21631c22b374 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -25,20 +25,20 @@ * */ -#include -#include -#include -#include -#include +#include +#include #include -#include +#include +#include +#include #include +#include #include -#include -#include -#include -#include #include +#include +#include +#include +#include #include #include -- 2.34.1
[Intel-gfx] [PATCH v2 06/11] drm/i915: Use str_on_off()
Remove the local onoff() implementation and adopt the str_on_off() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi Acked-by: Daniel Vetter Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/g4x_dp.c | 6 -- drivers/gpu/drm/i915/display/intel_display.c | 7 --- drivers/gpu/drm/i915/display/intel_display_trace.h | 3 ++- drivers/gpu/drm/i915/display/intel_dpll.c | 3 ++- drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 +-- drivers/gpu/drm/i915/display/intel_fdi.c | 8 +--- drivers/gpu/drm/i915/display/vlv_dsi_pll.c | 3 ++- drivers/gpu/drm/i915/gt/intel_rc6.c| 5 +++-- drivers/gpu/drm/i915/i915_utils.h | 5 - drivers/gpu/drm/i915/vlv_suspend.c | 3 ++- 10 files changed, 29 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c index f37677df6ebf..3e729bff1232 100644 --- a/drivers/gpu/drm/i915/display/g4x_dp.c +++ b/drivers/gpu/drm/i915/display/g4x_dp.c @@ -5,6 +5,8 @@ * DisplayPort support for G4x,ILK,SNB,IVB,VLV,CHV (HSW+ handled by the DDI code). */ +#include + #include "g4x_dp.h" #include "intel_audio.h" #include "intel_backlight.h" @@ -191,7 +193,7 @@ static void assert_dp_port(struct intel_dp *intel_dp, bool state) I915_STATE_WARN(cur_state != state, "[ENCODER:%d:%s] state assertion failure (expected %s, current %s)\n", dig_port->base.base.base.id, dig_port->base.base.name, - onoff(state), onoff(cur_state)); + str_on_off(state), str_on_off(cur_state)); } #define assert_dp_port_disabled(d) assert_dp_port((d), false) @@ -201,7 +203,7 @@ static void assert_edp_pll(struct drm_i915_private *dev_priv, bool state) I915_STATE_WARN(cur_state != state, "eDP PLL state assertion failure (expected %s, current %s)\n", - onoff(state), onoff(cur_state)); + str_on_off(state), str_on_off(cur_state)); } #define assert_edp_pll_enabled(d) assert_edp_pll((d), true) #define assert_edp_pll_disabled(d) assert_edp_pll((d), false) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 8920bdb53b7b..49f994f36fce 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -377,7 +377,7 @@ static void wait_for_pipe_scanline_moving(struct intel_crtc *crtc, bool state) if (wait_for(pipe_scanline_is_moving(dev_priv, pipe) == state, 100)) drm_err(&dev_priv->drm, "pipe %c scanline %s wait timed out\n", - pipe_name(pipe), onoff(state)); + pipe_name(pipe), str_on_off(state)); } static void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc) @@ -435,7 +435,7 @@ void assert_transcoder(struct drm_i915_private *dev_priv, I915_STATE_WARN(cur_state != state, "transcoder %s assertion failure (expected %s, current %s)\n", transcoder_name(cpu_transcoder), - onoff(state), onoff(cur_state)); + str_on_off(state), str_on_off(cur_state)); } static void assert_plane(struct intel_plane *plane, bool state) @@ -447,7 +447,8 @@ static void assert_plane(struct intel_plane *plane, bool state) I915_STATE_WARN(cur_state != state, "%s assertion failure (expected %s, current %s)\n", - plane->base.name, onoff(state), onoff(cur_state)); + plane->base.name, str_on_off(state), + str_on_off(cur_state)); } #define assert_plane_enabled(p) assert_plane(p, true) diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h index dcdd242fffd9..2dd5a4b7f5d8 100644 --- a/drivers/gpu/drm/i915/display/intel_display_trace.h +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h @@ -9,6 +9,7 @@ #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ) #define __INTEL_DISPLAY_TRACE_H__ +#include #include #include @@ -161,7 +162,7 @@ TRACE_EVENT(intel_memory_cxsr, ), TP_printk("%s->%s, pipe A: frame=%u, scanline=%u, pipe B: frame=%u, scanline=%u, pipe C: frame=%u, scanline=%u", - onoff(__entry->old), onoff(__entry->new), + str_on_off(__entry->old), str_on_off(__entry->new), __entry->frame[PIPE_A], __entry->scanline[PIPE_A], __entry->frame[PIPE_B], __entry->scanline[PIPE_B], __entry->frame[PIPE_C], __entry->scanline[PIPE_C]) diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c index 1
[Intel-gfx] [PATCH v2 07/11] drm/amd/display: Use str_yes_no()
Remove the local yesno() implementation and adopt the str_yes_no() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi --- .../drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index 26719efa5396..5ff1076b9130 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c @@ -23,6 +23,7 @@ * */ +#include #include #include "dc.h" @@ -49,11 +50,6 @@ struct dmub_debugfs_trace_entry { uint32_t param1; }; -static inline const char *yesno(bool v) -{ - return v ? "yes" : "no"; -} - /* parse_write_buffer_into_params - Helper function to parse debugfs write buffer into an array * * Function takes in attributes passed to debugfs write entry @@ -853,12 +849,12 @@ static int psr_capability_show(struct seq_file *m, void *data) if (!(link->connector_signal & SIGNAL_TYPE_EDP)) return -ENODEV; - seq_printf(m, "Sink support: %s", yesno(link->dpcd_caps.psr_caps.psr_version != 0)); + seq_printf(m, "Sink support: %s", str_yes_no(link->dpcd_caps.psr_caps.psr_version != 0)); if (link->dpcd_caps.psr_caps.psr_version) seq_printf(m, " [0x%02x]", link->dpcd_caps.psr_caps.psr_version); seq_puts(m, "\n"); - seq_printf(m, "Driver support: %s", yesno(link->psr_settings.psr_feature_enabled)); + seq_printf(m, "Driver support: %s", str_yes_no(link->psr_settings.psr_feature_enabled)); if (link->psr_settings.psr_version) seq_printf(m, " [0x%02x]", link->psr_settings.psr_version); seq_puts(m, "\n"); @@ -1207,8 +1203,8 @@ static int dp_dsc_fec_support_show(struct seq_file *m, void *data) drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); - seq_printf(m, "FEC_Sink_Support: %s\n", yesno(is_fec_supported)); - seq_printf(m, "DSC_Sink_Support: %s\n", yesno(is_dsc_supported)); + seq_printf(m, "FEC_Sink_Support: %s\n", str_yes_no(is_fec_supported)); + seq_printf(m, "DSC_Sink_Support: %s\n", str_yes_no(is_dsc_supported)); return ret; } -- 2.34.1
[Intel-gfx] [PATCH v2 05/11] drm/i915: Use str_enabled_disabled()
Remove the local enableddisabled() implementation and adopt the str_enabled_disabled() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi Acked-by: Daniel Vetter Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_backlight.c | 3 ++- drivers/gpu/drm/i915/display/intel_display.c | 16 .../gpu/drm/i915/display/intel_display_debugfs.c | 8 drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 7 --- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 ++- drivers/gpu/drm/i915/gt/intel_engine_cs.c| 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c| 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c| 2 +- drivers/gpu/drm/i915/gt/uc/intel_uc.c| 4 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_driver.c | 4 +++- drivers/gpu/drm/i915/i915_utils.h| 6 +- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 14 files changed, 33 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 98f7ea44042f..c8e1fc53a881 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -5,6 +5,7 @@ #include #include +#include #include "intel_backlight.h" #include "intel_connector.h" @@ -1633,7 +1634,7 @@ int intel_backlight_setup(struct intel_connector *connector, enum pipe pipe) drm_dbg_kms(&dev_priv->drm, "Connector %s backlight initialized, %s, brightness %u/%u\n", connector->base.name, - enableddisabled(panel->backlight.enabled), + str_enabled_disabled(panel->backlight.enabled), panel->backlight.level, panel->backlight.max); return 0; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index bd453861088e..8920bdb53b7b 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3110,8 +3110,8 @@ static void intel_panel_sanitize_ssc(struct drm_i915_private *dev_priv) if (dev_priv->vbt.lvds_use_ssc != bios_lvds_use_ssc) { drm_dbg_kms(&dev_priv->drm, "SSC %s by BIOS, overriding VBT which says %s\n", - enableddisabled(bios_lvds_use_ssc), - enableddisabled(dev_priv->vbt.lvds_use_ssc)); + str_enabled_disabled(bios_lvds_use_ssc), + str_enabled_disabled(dev_priv->vbt.lvds_use_ssc)); dev_priv->vbt.lvds_use_ssc = bios_lvds_use_ssc; } } @@ -5648,7 +5648,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, pipe_config->bigjoiner ? "master" : "no"); drm_dbg_kms(&dev_priv->drm, "splitter: %s, link count %d, overlap %d\n", - enableddisabled(pipe_config->splitter.enable), + str_enabled_disabled(pipe_config->splitter.enable), pipe_config->splitter.link_count, pipe_config->splitter.pixel_overlap); @@ -5736,7 +5736,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, drm_dbg_kms(&dev_priv->drm, "pch pfit: " DRM_RECT_FMT ", %s, force thru: %s\n", DRM_RECT_ARG(&pipe_config->pch_pfit.dst), - enableddisabled(pipe_config->pch_pfit.enabled), + str_enabled_disabled(pipe_config->pch_pfit.enabled), str_yes_no(pipe_config->pch_pfit.force_thru)); drm_dbg_kms(&dev_priv->drm, "ips: %i, double wide: %i\n", @@ -10300,7 +10300,7 @@ static void readout_plane_state(struct drm_i915_private *dev_priv) drm_dbg_kms(&dev_priv->drm, "[PLANE:%d:%s] hw state readout: %s, pipe %c\n", plane->base.base.id, plane->base.name, - enableddisabled(visible), pipe_name(pipe)); + str_enabled_disabled(visible), pipe_name(pipe)); } for_each_intel_crtc(&dev_priv->drm, crtc) { @@ -10346,7 +10346,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s] hw state readout: %s\n", crtc->base.base.id, crtc->base.name, - enableddisabled(crtc_state->hw.active)); + str_enabled_disabled(crtc_state->hw.active)); } cdclk_state->active_pipes = dbuf_state->active_pipes
[Intel-gfx] [PATCH v2 00/11] lib/string_helpers: Add a few string helpers
Add some helpers under lib/string_helpers.h so they can be used throughout the kernel. When I started doing this there were 2 other previous attempts I know of, not counting the iterations each of them had: 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nik...@intel.com/ 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevche...@linux.intel.com/#t Now there is also the v1 of this same patch series: https://lore.kernel.org/all/20220119072450.2890107-1-lucas.demar...@intel.com/ Going through the comments I tried to find some common ground and justification for what is in here, addressing some of the concerns raised. a. This version should be a drop-in replacement for what is currently in the tree, with no change in behavior or binary size. For binary size what I checked was that the linked objects in the end have the same size (gcc 11). From comments in the previous attempts this seems also the case for earlier compiler versions b. I didn't change the function name to choice_* as suggested by Andrew Morton in 20191023155619.43e0013f0c8c673a5c508...@linux-foundation.org because other people argumented in favor of shorter names for these simple helpers - if they are long and people simply not use due to that, we failed. However as pointed out in v1 of this patchseries, onoff(), yesno(), enabledisable(), enableddisabled() have some issues: the last 2 are hard to read and for the first 2 it would not be hard to have the symbol to clash with variable names. From comments in v1, most people were in favor (or at least not opposed) to using str_on_off(), str_yes_no(), str_enable_disable() and str_enabled_disabled(). c. Use string_helper.h for these helpers - pulling string.h in the compilations units was one of the concerns and I think re-using this already existing header is better than creating a new string-choice.h d. One alternative to all of this suggested by Christian König (43456ba7-c372-84cc-4949-dcb817188...@amd.com) would be to add a printk format. But besides the comment, he also seemed to like the common function. This brought the argument from others that the simple yesno()/enabledisable() already used in the code (or new renamed version) is easier to remember and use than e.g. %py[DOY] Changes in v2: - Use str_ prefix and separate other words with underscore: it's a little bit longer, but should improve readability - Patches we re-split due to the rename: first patch adds all the new functions, then additional patches try to do one conversion at a time. While doing so, there were some fixes for issues already present along the way - Style suggestions from v1 were adopted In v1 it was suggested to apply this in drm-misc. I will leave this to maintainers to decide: maybe it would be simpler to merge the first patches on drm-intel-next, wait for the back merge and merge the rest through drm-misc - my fear is a big conflict with other work going in drm-intel-next since the bulk of the rename is there. I tried to figure out acks and reviews from v1 and apply them to how the patches are now split. thanks Lucas De Marchi Lucas De Marchi (11): lib/string_helpers: Consolidate string helpers implementation drm/i915: Fix trailing semicolon drm/i915: Use str_yes_no() drm/i915: Use str_enable_disable() drm/i915: Use str_enabled_disabled() drm/i915: Use str_on_off() drm/amd/display: Use str_yes_no() drm/gem: Sort includes alphabetically drm: Convert open-coded yes/no strings to yesno() tomoyo: Use str_yes_no() cxgb4: Use str_yes_no() drivers/gpu/drm/amd/amdgpu/atom.c | 4 +- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 14 +- drivers/gpu/drm/dp/drm_dp.c | 3 +- drivers/gpu/drm/drm_client_modeset.c | 3 +- drivers/gpu/drm/drm_gem.c | 23 +- drivers/gpu/drm/i915/display/g4x_dp.c | 6 +- .../gpu/drm/i915/display/intel_backlight.c| 3 +- drivers/gpu/drm/i915/display/intel_ddi.c | 4 +- drivers/gpu/drm/i915/display/intel_display.c | 46 ++-- .../drm/i915/display/intel_display_debugfs.c | 74 +++--- .../drm/i915/display/intel_display_power.c| 4 +- .../drm/i915/display/intel_display_trace.h| 9 +- drivers/gpu/drm/i915/display/intel_dp.c | 20 +- drivers/gpu/drm/i915/display/intel_dpll.c | 3 +- drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 7 +- drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 7 +- drivers/gpu/drm/i915/display/intel_fbc.c | 4 +- drivers/gpu/drm/i915/display/intel_fdi.c | 8 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +- drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- drivers/gpu/drm/i915/display/vlv_dsi_pll.c| 3 +- .../gpu/drm/i915/gem/selftests/huge_pages.c | 9 +- .../drm/i915/gem/selftests/i915_gem_context.c | 7 +- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 +- drivers/gpu
[Intel-gfx] [PATCH v2 01/11] lib/string_helpers: Consolidate string helpers implementation
There are a few implementations of string helpers in the tree like yesno() that just returns "yes" or "no" depending on a boolean argument. Those are helpful to output strings to the user or log. In order to consolidate them, prefix all of them str_ prefix to make it clear what they are about and avoid symbol clashes. Taking the commoon `val ? "yes" : "no"` implementation, quite a few users of open coded yesno() could later be converted to the new function: $ git grep '?\s*"yes"\s*' | wc -l 286 $ git grep '?\s*"no"\s*' | wc -l 20 The inlined function should keep the const strings local to each compilation unit, the same way it's now, thus not changing the current behavior. Signed-off-by: Lucas De Marchi Reviewed-by: Andy Shevchenko Acked-by: Jani Nikula Acked-by: Daniel Vetter --- include/linux/string_helpers.h | 20 1 file changed, 20 insertions(+) diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h index 7a22921c9db7..4d72258d42fd 100644 --- a/include/linux/string_helpers.h +++ b/include/linux/string_helpers.h @@ -106,4 +106,24 @@ void kfree_strarray(char **array, size_t n); char **devm_kasprintf_strarray(struct device *dev, const char *prefix, size_t n); +static inline const char *str_yes_no(bool v) +{ + return v ? "yes" : "no"; +} + +static inline const char *str_on_off(bool v) +{ + return v ? "on" : "off"; +} + +static inline const char *str_enable_disable(bool v) +{ + return v ? "enable" : "disable"; +} + +static inline const char *str_enabled_disabled(bool v) +{ + return v ? "enabled" : "disabled"; +} + #endif -- 2.34.1
[Intel-gfx] [PATCH v2 02/11] drm/i915: Fix trailing semicolon
Remove the trailing semicolon, as correctly warned by checkpatch: -:1189: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon #1189: FILE: drivers/gpu/drm/i915/intel_device_info.c:119: +#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, yesno(info->display.name)); Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/intel_device_info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 93b251b25aba..94da5aa37391 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -114,7 +114,7 @@ void intel_device_info_print_static(const struct intel_device_info *info, DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG); #undef PRINT_FLAG -#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, yesno(info->display.name)); +#define PRINT_FLAG(name) drm_printf(p, "%s: %s\n", #name, yesno(info->display.name)) DEV_INFO_DISPLAY_FOR_EACH_FLAG(PRINT_FLAG); #undef PRINT_FLAG } -- 2.34.1
[Intel-gfx] [PATCH v2 09/11] drm: Convert open-coded yes/no strings to yesno()
linux/string_helpers.h provides a helper to return "yes"/"no" strings. Replace the open coded versions with str_yes_no(). The places were identified with the following semantic patch: @@ expression b; @@ - b ? "yes" : "no" + str_yes_no(b) Then the includes were added, so we include-what-we-use, and parenthesis adjusted in drivers/gpu/drm/v3d/v3d_debugfs.c. After the conversion we still see the same binary sizes: textdata bss dec hex filename 511493295 212 54656d580 virtio/virtio-gpu.ko.old 511493295 212 54656d580 virtio/virtio-gpu.ko 1441491 60340 800 1502631 16eda7 radeon/radeon.ko.old 1441491 60340 800 1502631 16eda7 radeon/radeon.ko 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko.old 6125369 328538 34000 6487907 62ff63 amd/amdgpu/amdgpu.ko 411986 104906176 428652 68a6c drm.ko.old 411986 104906176 428652 68a6c drm.ko 981291636 264 100029 186bd dp/drm_dp_helper.ko.old 981291636 264 100029 186bd dp/drm_dp_helper.ko 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko.old 1973432 1096402352 2085424 1fd230 nouveau/nouveau.ko Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/amd/amdgpu/atom.c | 4 +++- drivers/gpu/drm/dp/drm_dp.c | 3 ++- drivers/gpu/drm/drm_client_modeset.c | 3 ++- drivers/gpu/drm/drm_gem.c | 3 ++- drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c | 5 - drivers/gpu/drm/radeon/atom.c | 3 ++- drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++- drivers/gpu/drm/virtio/virtgpu_debugfs.c | 4 +++- 8 files changed, 24 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c index 6fa2229b7229..1c5d9388ad0b 100644 --- a/drivers/gpu/drm/amd/amdgpu/atom.c +++ b/drivers/gpu/drm/amd/amdgpu/atom.c @@ -25,6 +25,8 @@ #include #include #include +#include + #include #include @@ -740,7 +742,7 @@ static void atom_op_jump(atom_exec_context *ctx, int *ptr, int arg) break; } if (arg != ATOM_COND_ALWAYS) - SDEBUG(" taken: %s\n", execute ? "yes" : "no"); + SDEBUG(" taken: %s\n", str_yes_no(execute)); SDEBUG(" target: 0x%04X\n", target); if (execute) { if (ctx->last_jump == (ctx->start + target)) { diff --git a/drivers/gpu/drm/dp/drm_dp.c b/drivers/gpu/drm/dp/drm_dp.c index 6d43325acca5..c43577c8ac4d 100644 --- a/drivers/gpu/drm/dp/drm_dp.c +++ b/drivers/gpu/drm/dp/drm_dp.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -1239,7 +1240,7 @@ void drm_dp_downstream_debug(struct seq_file *m, bool branch_device = drm_dp_is_branch(dpcd); seq_printf(m, "\tDP branch device present: %s\n", - branch_device ? "yes" : "no"); + str_yes_no(branch_device)); if (!branch_device) return; diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c index ced09c7c06f9..e6346a67cd98 100644 --- a/drivers/gpu/drm/drm_client_modeset.c +++ b/drivers/gpu/drm/drm_client_modeset.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -241,7 +242,7 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors, connector = connectors[i]; enabled[i] = drm_connector_enabled(connector, true); DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id, - connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no"); + connector->display_info.non_desktop ? "non desktop" : str_yes_no(enabled[i])); any_enabled |= enabled[i]; } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 21631c22b374..3c888db59ea4 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -1145,7 +1146,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent, drm_vma_node_start(&obj->vma_node)); drm_printf_indent(p, indent, "size=%zu\n", obj->size); drm_printf_indent(p, indent, "imported=%s\n", - obj->import_attach ? "yes" : "no"); + str_yes_no(obj->import_attach)); if (obj->funcs->print_info) obj->funcs->print_info(p, indent, obj); diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c index a11637b0f6cc..d063d0dc13c5 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c @@ -21,6 +21,9 @@ * * Authors: Ben Skeggs
[Intel-gfx] [PATCH v2 04/11] drm/i915: Use str_enable_disable()
Remove the local enabledisable() implementation and adopt the str_enable_disable() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi Acked-by: Daniel Vetter Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_ddi.c | 4 +++- drivers/gpu/drm/i915/display/intel_display_power.c | 4 +++- drivers/gpu/drm/i915/display/intel_dp.c| 8 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 3 ++- drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 4 +++- drivers/gpu/drm/i915/i915_utils.h | 5 - 6 files changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 2f20abc5122d..4b35a8597632 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -25,6 +25,8 @@ * */ +#include + #include #include @@ -2152,7 +2154,7 @@ static void intel_dp_sink_set_msa_timing_par_ignore_state(struct intel_dp *intel enable ? DP_MSA_TIMING_PAR_IGNORE_EN : 0) <= 0) drm_dbg_kms(&i915->drm, "Failed to %s MSA_TIMING_PAR_IGNORE in the sink\n", - enabledisable(enable)); + str_enable_disable(enable)); } static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp, diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 369317805d24..1f77cb9edddf 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -3,6 +3,8 @@ * Copyright © 2019 Intel Corporation */ +#include + #include "i915_drv.h" #include "i915_irq.h" #include "intel_cdclk.h" @@ -5302,7 +5304,7 @@ static void gen9_dbuf_slice_set(struct drm_i915_private *dev_priv, state = intel_de_read(dev_priv, reg) & DBUF_POWER_STATE; drm_WARN(&dev_priv->drm, enable != state, "DBuf slice %d power %s timeout!\n", -slice, enabledisable(enable)); +slice, str_enable_disable(enable)); } void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 62c1535d696d..933fc316ea53 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1987,7 +1987,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp, if (ret < 0) drm_dbg_kms(&i915->drm, "Failed to %s sink decompression state\n", - enabledisable(enable)); + str_enable_disable(enable)); } static void @@ -2463,7 +2463,7 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp, if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_PROTOCOL_CONVERTER_CONTROL_0, tmp) != 1) drm_dbg_kms(&i915->drm, "Failed to %s protocol converter HDMI mode\n", - enabledisable(intel_dp->has_hdmi_sink)); + str_enable_disable(intel_dp->has_hdmi_sink)); tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 && intel_dp->dfp.ycbcr_444_to_420 ? DP_CONVERSION_TO_YCBCR420_ENABLE : 0; @@ -2472,7 +2472,7 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp, DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1) drm_dbg_kms(&i915->drm, "Failed to %s protocol converter YCbCr 4:2:0 conversion mode\n", - enabledisable(intel_dp->dfp.ycbcr_444_to_420)); + str_enable_disable(intel_dp->dfp.ycbcr_444_to_420)); tmp = 0; if (intel_dp->dfp.rgb_to_ycbcr) { @@ -2510,7 +2510,7 @@ void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp, if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) < 0) drm_dbg_kms(&i915->drm, "Failed to %s protocol converter RGB->YCbCr conversion mode\n", - enabledisable(tmp)); + str_enable_disable(tmp)); } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index de89d40abd38..31c3c3bceb95 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "i915_drv.h" @@ -170,7 +171,7 @@ static int ct_control_enable(struct intel_guc_ct *ct, bool enable) GUC_CTB_CONTROL_ENABLE : GUC_CTB_CONTROL_DISABLE); if (unlikely(err)) CT_PROBE_ERROR(ct, "Failed to control/%s CTB (%pe)\n", -
[Intel-gfx] [PATCH v2 03/11] drm/i915: Use str_yes_no()
Remove the local yesno() implementation and adopt the str_yes_no() from linux/string_helpers.h. Signed-off-by: Lucas De Marchi Acked-by: Daniel Vetter Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_display.c | 23 +++ .../drm/i915/display/intel_display_debugfs.c | 66 +++ .../drm/i915/display/intel_display_trace.h| 6 +- drivers/gpu/drm/i915/display/intel_dp.c | 12 ++-- drivers/gpu/drm/i915/display/intel_fbc.c | 4 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +- drivers/gpu/drm/i915/display/intel_sprite.c | 6 +- .../gpu/drm/i915/gem/selftests/huge_pages.c | 9 +-- .../drm/i915/gem/selftests/i915_gem_context.c | 7 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 +-- .../drm/i915/gt/intel_execlists_submission.c | 7 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 3 +- drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 52 --- drivers/gpu/drm/i915/gt/intel_reset.c | 3 +- drivers/gpu/drm/i915/gt/intel_rps.c | 13 ++-- drivers/gpu/drm/i915/gt/intel_sseu.c | 9 ++- drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 10 +-- drivers/gpu/drm/i915/gt/selftest_timeline.c | 3 +- drivers/gpu/drm/i915/gt/uc/intel_guc_log.c| 3 +- drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 4 +- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 +-- drivers/gpu/drm/i915/gt/uc/intel_uc_debugfs.c | 20 +++--- drivers/gpu/drm/i915/i915_debugfs.c | 15 +++-- drivers/gpu/drm/i915/i915_gpu_error.c | 9 +-- drivers/gpu/drm/i915/i915_params.c| 5 +- drivers/gpu/drm/i915/i915_utils.h | 5 -- drivers/gpu/drm/i915/intel_device_info.c | 8 ++- drivers/gpu/drm/i915/intel_dram.c | 10 +-- drivers/gpu/drm/i915/intel_pm.c | 10 +-- drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 4 +- drivers/gpu/drm/i915/selftests/i915_active.c | 3 +- 31 files changed, 203 insertions(+), 148 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 80bc52425e47..bd453861088e 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -3008,7 +3009,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, drm_dbg_kms(&dev_priv->drm, "requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n", pipe_mode->crtc_clock, clock_limit, - yesno(pipe_config->double_wide)); + str_yes_no(pipe_config->double_wide)); return -EINVAL; } @@ -5586,7 +5587,7 @@ static void intel_dump_plane_state(const struct intel_plane_state *plane_state) drm_dbg_kms(&i915->drm, "[PLANE:%d:%s] fb: [NOFB], visible: %s\n", plane->base.base.id, plane->base.name, - yesno(plane_state->uapi.visible)); + str_yes_no(plane_state->uapi.visible)); return; } @@ -5594,7 +5595,7 @@ static void intel_dump_plane_state(const struct intel_plane_state *plane_state) "[PLANE:%d:%s] fb: [FB:%d] %ux%u format = %p4cc modifier = 0x%llx, visible: %s\n", plane->base.base.id, plane->base.name, fb->base.id, fb->width, fb->height, &fb->format->format, - fb->modifier, yesno(plane_state->uapi.visible)); + fb->modifier, str_yes_no(plane_state->uapi.visible)); drm_dbg_kms(&i915->drm, "\trotation: 0x%x, scaler: %d\n", plane_state->hw.rotation, plane_state->scaler_id); if (plane_state->uapi.visible) @@ -5617,7 +5618,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s] enable: %s %s\n", crtc->base.base.id, crtc->base.name, - yesno(pipe_config->hw.enable), context); + str_yes_no(pipe_config->hw.enable), context); if (!pipe_config->hw.enable) goto dump_planes; @@ -5625,7 +5626,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, snprintf_output_types(buf, sizeof(buf), pipe_config->output_types); drm_dbg_kms(&dev_priv->drm, "active: %s, output_types: %s (0x%x), output format: %s\n", - yesno(pipe_config->hw.active), + str_yes_no(pipe_config->hw.active), buf, pipe_config->output_types, output_formats(pipe_config->output_format)); @@ -5694,7 +5695,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, intel_dump_dp_vsc_sd
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix oops due to missing stack depot
On 1/26/22 09:15, Ville Syrjala wrote: > From: Ville Syrjälä > > We call __save_depot_stack() unconditionally so the stack depot Ah, in __untrack_all_wakerefs()? Looks like I missed it, sorry. > must always be initialized or else we'll oops on platforms without > runtime pm support. > > Presumably we've not seen this in CI due to stack_depot_init() > already getting called via drm_mm_init()+CONFIG_DRM_DEBUG_MM. > > Cc: Vlastimil Babka > Cc: Dmitry Vyukov > Cc: Marco Elver # stackdepot > Cc: Chris Wilson > Cc: Imre Deak > Fixes: 2dba5eb1c73b ("lib/stackdepot: allow optional init and stack_table > allocation by kvmalloc()") > Signed-off-by: Ville Syrjälä Acked-by: Vlastimil Babka Thanks! > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 53f1ccb78849..64c2708efc9e 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -68,9 +68,7 @@ static noinline depot_stack_handle_t > __save_depot_stack(void) > static void init_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm) > { > spin_lock_init(&rpm->debug.lock); > - > - if (rpm->available) > - stack_depot_init(); > + stack_depot_init(); > } > > static noinline depot_stack_handle_t
Re: [Intel-gfx] [PATCH 3/3] drm: Convert open yes/no strings to yesno()
On Wed, Jan 19, 2022 at 09:30:47PM +0200, Andy Shevchenko wrote: On Tue, Jan 18, 2022 at 11:24:50PM -0800, Lucas De Marchi wrote: linux/string_helpers.h provides a helper to return "yes"/"no" strings. Replace the open coded versions with yesno(). The places were identified with the following semantic patch: @@ expression b; @@ - b ? "yes" : "no" + yesno(b) Then the includes were added, so we include-what-we-use, and parenthesis adjusted in drivers/gpu/drm/v3d/v3d_debugfs.c. After the conversion we still see the same binary sizes: textdata bss dec hex filename 1442171 60344 800 1503315 16f053 ./drivers/gpu/drm/radeon/radeon.ko 1442171 60344 800 1503315 16f053 ./drivers/gpu/drm/radeon/radeon.ko.old 5985991 324439 33808 6344238 60ce2e ./drivers/gpu/drm/amd/amdgpu/amdgpu.ko 5985991 324439 33808 6344238 60ce2e ./drivers/gpu/drm/amd/amdgpu/amdgpu.ko.old 411986 104906176 428652 68a6c ./drivers/gpu/drm/drm.ko 411986 104906176 428652 68a6c ./drivers/gpu/drm/drm.ko.old 1970292 1095152352 2082159 1fc56f ./drivers/gpu/drm/nouveau/nouveau.ko 1970292 1095152352 2082159 1fc56f ./drivers/gpu/drm/nouveau/nouveau.ko.old ... #include #include #include +#include + blank line? +#include ... seq_printf(m, "\tDP branch device present: %s\n", - branch_device ? "yes" : "no"); + yesno(branch_device)); Now it's possible to keep this on one line. ... drm_printf_indent(p, indent, "imported=%s\n", - obj->import_attach ? "yes" : "no"); + yesno(obj->import_attach)); 81 here, but anyway, ditto! ... */ +blank line here? +#include + #include "aux.h" #include "pad.h" ... seq_printf(m, "MMU:%s\n", - (ident2 & V3D_HUB_IDENT2_WITH_MMU) ? "yes" : "no"); + yesno(ident2 & V3D_HUB_IDENT2_WITH_MMU)); seq_printf(m, "TFU:%s\n", - (ident1 & V3D_HUB_IDENT1_WITH_TFU) ? "yes" : "no"); + yesno(ident1 & V3D_HUB_IDENT1_WITH_TFU)); seq_printf(m, "TSY:%s\n", - (ident1 & V3D_HUB_IDENT1_WITH_TSY) ? "yes" : "no"); + yesno(ident1 & V3D_HUB_IDENT1_WITH_TSY)); seq_printf(m, "MSO:%s\n", - (ident1 & V3D_HUB_IDENT1_WITH_MSO) ? "yes" : "no"); + yesno(ident1 & V3D_HUB_IDENT1_WITH_MSO)); seq_printf(m, "L3C:%s (%dkb)\n", - (ident1 & V3D_HUB_IDENT1_WITH_L3C) ? "yes" : "no", + yesno(ident1 & V3D_HUB_IDENT1_WITH_L3C), V3D_GET_FIELD(ident2, V3D_HUB_IDENT2_L3C_NKB)); I believe it's fine to join back to have less LOCs (yes, it will be 83 or so, but I believe in these cases it's very much okay). now that we are converting to str_yes_no(), we will have a few more chars. Some maintainers may be more strict on the 80 or 100 chars. I will assume whatever is in the code base is the preferred form. thanks Lucas De Marchi -- With Best Regards, Andy Shevchenko
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Lock dpt_obj around set_cache_level, v2.
== Series Details == Series: drm/i915: Lock dpt_obj around set_cache_level, v2. URL : https://patchwork.freedesktop.org/series/99350/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11137 -> Patchwork_22108 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_22108 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_22108, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/index.html Participating hosts (48 -> 43) -- Missing(5): fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510 fi-bdw-samus Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_22108: ### IGT changes ### Possible regressions * igt@kms_busy@basic@modeset: - bat-adlp-4: NOTRUN -> [DMESG-WARN][1] +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/bat-adlp-4/igt@kms_busy@ba...@modeset.html Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@kms_busy@basic@flip: - {bat-adlp-6}: NOTRUN -> [DMESG-WARN][2] +1 similar issue [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/bat-adlp-6/igt@kms_busy@ba...@flip.html Known issues Here are the changes found in Patchwork_22108 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_suspend@basic-s3: - fi-skl-6600u: NOTRUN -> [INCOMPLETE][3] ([i915#4547]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/fi-skl-6600u/igt@gem_exec_susp...@basic-s3.html * igt@runner@aborted: - fi-skl-6600u: NOTRUN -> [FAIL][4] ([i915#2722] / [i915#4312]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/fi-skl-6600u/igt@run...@aborted.html Possible fixes * igt@kms_addfb_basic@addfb25-framebuffer-vs-set-tiling: - bat-adlp-4: [DMESG-WARN][5] -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/bat-adlp-4/igt@kms_addfb_ba...@addfb25-framebuffer-vs-set-tiling.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/bat-adlp-4/igt@kms_addfb_ba...@addfb25-framebuffer-vs-set-tiling.html - {bat-adlp-6}: [DMESG-WARN][7] -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/bat-adlp-6/igt@kms_addfb_ba...@addfb25-framebuffer-vs-set-tiling.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/bat-adlp-6/igt@kms_addfb_ba...@addfb25-framebuffer-vs-set-tiling.html * igt@kms_pipe_crc_basic@read-crc-pipe-c: - {fi-tgl-dsi}: [DMESG-WARN][9] ([i915#1982]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11137/fi-tgl-dsi/igt@kms_pipe_crc_ba...@read-crc-pipe-c.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/fi-tgl-dsi/igt@kms_pipe_crc_ba...@read-crc-pipe-c.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547 Build changes - * Linux: CI_DRM_11137 -> Patchwork_22108 CI-20190529: 20190529 CI_DRM_11137: 4d22013ea4c52e5dd3625861c9c65fb8027f53a6 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6332: 27c9c3f5181a840c777399be7681d2cadd7940cd @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_22108: 6d1423b45148508bf31792fd561fd8a28051f19c @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 6d1423b45148 drm/i915: Lock dpt_obj around set_cache_level, v2. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22108/index.html
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Lock dpt_obj around set_cache_level, v2.
== Series Details == Series: drm/i915: Lock dpt_obj around set_cache_level, v2. URL : https://patchwork.freedesktop.org/series/99350/ State : warning == Summary == $ dim checkpatch origin/drm-tip 6d1423b45148 drm/i915: Lock dpt_obj around set_cache_level, v2. -:8: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #8: <6> [184.578145] [IGT] kms_addfb_basic: starting subtest addfb25-framebuffer-vs-set-tiling total: 0 errors, 1 warnings, 0 checks, 24 lines checked
[Intel-gfx] [PATCH 1/2] drm/i915: Fix oops due to missing stack depot
From: Ville Syrjälä We call __save_depot_stack() unconditionally so the stack depot must always be initialized or else we'll oops on platforms without runtime pm support. Presumably we've not seen this in CI due to stack_depot_init() already getting called via drm_mm_init()+CONFIG_DRM_DEBUG_MM. Cc: Vlastimil Babka Cc: Dmitry Vyukov Cc: Marco Elver # stackdepot Cc: Chris Wilson Cc: Imre Deak Fixes: 2dba5eb1c73b ("lib/stackdepot: allow optional init and stack_table allocation by kvmalloc()") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 53f1ccb78849..64c2708efc9e 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -68,9 +68,7 @@ static noinline depot_stack_handle_t __save_depot_stack(void) static void init_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm) { spin_lock_init(&rpm->debug.lock); - - if (rpm->available) - stack_depot_init(); + stack_depot_init(); } static noinline depot_stack_handle_t -- 2.34.1
[Intel-gfx] [PATCH 2/2] drm/i915: Enable rpm wakeref tracking whether runtime pm is enabled or not
From: Ville Syrjälä Don't see why we should skip the wakeref tracking when the platform doesn't support runtime pm. We still want all the code to be 100% leak free so let's track this unconditionally. Cc: Vlastimil Babka Cc: Dmitry Vyukov Cc: Marco Elver # stackdepot Cc: Chris Wilson Cc: Imre Deak Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_runtime_pm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 64c2708efc9e..3293ac71bcf8 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -77,9 +77,6 @@ track_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm) depot_stack_handle_t stack, *stacks; unsigned long flags; - if (!rpm->available) - return -1; - stack = __save_depot_stack(); if (!stack) return -1; -- 2.34.1