Re: [PATCH 2/2] drm/xe/display: Make panic support work on vram.

[Jocelyn Falempe <jfalempe@redhat.com>]

On 27/06/2025 17:13, Maarten Lankhorst wrote:
> Add a special path for VRAM using xe_res iterators to ensure a panic
> screen is shown on VRAM as well.
> 

I was considering using a different set_pixel() function for VRAM, but 
in fact both implementations share a lot of things, so it's better this way.

vmap and vaddr are a bit redundant, you can rewrite xe_panic_kunmap()
to use vmap instead, and remove vaddr, like this:

if (!panic->vmap.is_iomem && panic->vmap.vaddr) {
	drm_clflush_virt_range(panic->vmap.vaddr, PAGE_SIZE);
	kunmap_local(panic->vmap.vaddr);
	iosys_map_clear(&panic->vmap);
}

With this change:

Acked-by: Jocelyn Falempe <jfalempe@redhat.com>


Best regards,

-- 

Jocelyn


> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>   drivers/gpu/drm/xe/display/intel_bo.c | 36 +++++++++++++++++++++------
>   1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/intel_bo.c b/drivers/gpu/drm/xe/display/intel_bo.c
> index 910632f57c3d6..03be519207397 100644
> --- a/drivers/gpu/drm/xe/display/intel_bo.c
> +++ b/drivers/gpu/drm/xe/display/intel_bo.c
> @@ -9,6 +9,7 @@
>   #include "intel_display_types.h"
>   
>   #include "xe_bo.h"
> +#include "xe_res_cursor.h"
>   #include "intel_bo.h"
>   
>   bool intel_bo_is_tiled(struct drm_gem_object *obj)
> @@ -69,6 +70,9 @@ struct xe_panic_data {
>   	struct page **pages;
>   	int page;
>   	void *vaddr;
> +
> +	struct xe_res_cursor res;
> +	struct iosys_map vmap;
>   };
>   
>   struct xe_framebuffer {
> @@ -112,15 +116,29 @@ static void xe_panic_page_set_pixel(struct drm_scanout_buffer *sb, unsigned int
>   	new_page = offset >> PAGE_SHIFT;
>   	offset = offset % PAGE_SIZE;
>   	if (new_page != panic->page) {
> -		xe_panic_kunmap(panic);
> +		if (xe_bo_is_vram(bo)) {
> +			/* Display is always mapped on root tile */
> +			struct xe_vram_region *vram = &xe_bo_device(bo)->mem.vram;
> +
> +			if (panic->page < 0 || new_page < panic->page) {
> +				xe_res_first(bo->ttm.resource, new_page * PAGE_SIZE,
> +					     bo->ttm.base.size - new_page * PAGE_SIZE, &panic->res);
> +			} else {
> +				xe_res_next(&panic->res, PAGE_SIZE * (new_page - panic->page));
> +			}
> +			iosys_map_set_vaddr_iomem(&panic->vmap,
> +						  vram->mapping + panic->res.start);
> +		} else {
> +			xe_panic_kunmap(panic);
> +			panic->vaddr = ttm_bo_kmap_try_from_panic(&bo->ttm,
> +								  new_page);
> +			iosys_map_set_vaddr(&panic->vmap, panic->vaddr);
> +		}
>   		panic->page = new_page;
> -		panic->vaddr = ttm_bo_kmap_try_from_panic(&bo->ttm,
> -							  panic->page);
> -	}
> -	if (panic->vaddr) {
> -		u32 *pix = panic->vaddr + offset;
> -		*pix = color;
>   	}
> +
> +	if (iosys_map_is_set(&panic->vmap))
> +		iosys_map_wr(&panic->vmap, offset, u32, color);
>   }
>   
>   struct intel_framebuffer *intel_bo_alloc_framebuffer(void)
> @@ -137,6 +155,10 @@ int intel_bo_panic_setup(struct drm_scanout_buffer *sb)
>   {
>   	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
>   	struct xe_panic_data *panic = to_xe_panic_data(fb);
> +	struct xe_bo *bo = gem_to_xe_bo(intel_fb_bo(&fb->base));
> +
> +	if (xe_bo_is_vram(bo) && !xe_bo_is_visible_vram(bo))
> +		return -ENODEV;
>   
>   	panic->page = -1;
>   	sb->set_pixel = xe_panic_page_set_pixel;