On Mon, 02 Dec 2024, Matthew Auld <[email protected]> wrote: > On 02/12/2024 11:43, Jani Nikula wrote: >> On Tue, 26 Nov 2024, Matthew Brost <[email protected]> wrote: >>> Don't open code vmap of a BO, use ttm_bo_access helper which is safe for >>> non-contiguous BOs and non-visible BOs. >>> >>> Suggested-by: Matthew Auld <[email protected]> >>> Signed-off-by: Matthew Brost <[email protected]> >>> Reviewed-by: Matthew Auld <[email protected]> >> >> I've seen a few cases of [1] lately, and Thomas tipped me off to this >> change. We get: >> >> <4> [374.262965] xe 0000:03:00.0: [drm] drm_WARN_ON(ret) >> <4> [374.262983] WARNING: CPU: 8 PID: 5462 at >> drivers/gpu/drm/i915/display/intel_display.c:7637 >> intel_atomic_commit_tail+0x16c7/0x17f0 [xe] >> >> and that's intel_atomic_prepare_plane_clear_colors(): >> >> ret = intel_bo_read_from_page(intel_fb_bo(fb), >> fb->offsets[cc_plane] + 16, >> &plane_state->ccval, >> sizeof(plane_state->ccval)); >> /* The above could only fail if the FB obj has an unexpected >> backing store type. */ >> drm_WARN_ON(&i915->drm, ret); >> >> >> So I don't have any conclusive evidence, but could this be the reason? > > @@ -40,8 +40,13 @@ int intel_bo_fb_mmap(struct drm_gem_object *obj, > struct vm_area_struct *vma) > int intel_bo_read_from_page(struct drm_gem_object *obj, u64 offset, > void *dst, int size) > { > struct xe_bo *bo = gem_to_xe_bo(obj); > + int ret; > > - return ttm_bo_access(&bo->ttm, offset, dst, size, 0); > + ret = ttm_bo_access(&bo->ttm, offset, dst, size, 0); > + if (ret == size) > + ret = 0; > + > + return ret; > } > > I think we somehow missed that bo_access is returning @size on success?
On a another look, we got the warn in the CI results [1] for the series. :( BR, Jani. [1] https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-140200v7/shard-dg2-435/igt@kms_plane@[email protected]#dmesg-warnings69 > >> >> BR, >> Jani. >> >> >> >> [1] >> https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-138070v8/shard-dg2-434/igt@kms_flip_tiling@[email protected] >> >> >>> --- >>> drivers/gpu/drm/xe/display/intel_bo.c | 25 +------------------------ >>> 1 file changed, 1 insertion(+), 24 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/xe/display/intel_bo.c >>> b/drivers/gpu/drm/xe/display/intel_bo.c >>> index 9f54fad0f1c0..43141964f6f2 100644 >>> --- a/drivers/gpu/drm/xe/display/intel_bo.c >>> +++ b/drivers/gpu/drm/xe/display/intel_bo.c >>> @@ -40,31 +40,8 @@ int intel_bo_fb_mmap(struct drm_gem_object *obj, struct >>> vm_area_struct *vma) >>> int intel_bo_read_from_page(struct drm_gem_object *obj, u64 offset, void >>> *dst, int size) >>> { >>> struct xe_bo *bo = gem_to_xe_bo(obj); >>> - struct ttm_bo_kmap_obj map; >>> - void *src; >>> - bool is_iomem; >>> - int ret; >>> >>> - ret = xe_bo_lock(bo, true); >>> - if (ret) >>> - return ret; >>> - >>> - ret = ttm_bo_kmap(&bo->ttm, offset >> PAGE_SHIFT, 1, &map); >>> - if (ret) >>> - goto out_unlock; >>> - >>> - offset &= ~PAGE_MASK; >>> - src = ttm_kmap_obj_virtual(&map, &is_iomem); >>> - src += offset; >>> - if (is_iomem) >>> - memcpy_fromio(dst, (void __iomem *)src, size); >>> - else >>> - memcpy(dst, src, size); >>> - >>> - ttm_bo_kunmap(&map); >>> -out_unlock: >>> - xe_bo_unlock(bo); >>> - return ret; >>> + return ttm_bo_access(&bo->ttm, offset, dst, size, 0); >>> } >>> >>> struct intel_frontbuffer *intel_bo_get_frontbuffer(struct drm_gem_object >>> *obj) >> > -- Jani Nikula, Intel
