Re: [PATCH v18 1/9] drm/xe: Rename XE_BO_FLAG_SCANOUT to XE_BO_FLAG_FORCE_WC

Tvrtko Ursulin
2026-03-04T14:23:27Z

On 04/03/2026 13:50, Rodrigo Vivi wrote:
> On Wed, Mar 04, 2026 at 01:03:06PM +0000, Tvrtko Ursulin wrote:
>> Rename XE_BO_FLAG_SCANOUT to XE_BO_FLAG_FORCE_WC so that the usage of the
>> flag can legitimately be expanded to more than just the actual frame-
>> buffer objects.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/xe/display/intel_fb_bo.c | 6 +++---
>> drivers/gpu/drm/xe/display/intel_fbdev_fb.c | 12 ++++++++----
>> drivers/gpu/drm/xe/display/xe_dsb_buffer.c | 4 +++-
>> drivers/gpu/drm/xe/display/xe_fb_pin.c | 2 +-
>> drivers/gpu/drm/xe/display/xe_initial_plane.c | 2 +-
>> drivers/gpu/drm/xe/xe_bo.c | 16 +++++++++++-----
>> drivers/gpu/drm/xe/xe_bo.h | 2 +-
>> 7 files changed, 28 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> index db8b1a27b4de..d2e72dc5abd9 100644
>> --- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> +++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> @@ -45,9 +45,9 @@ int intel_fb_bo_framebuffer_init(struct drm_gem_object *obj,
>> if (ret)
>> goto err;
>>
>> - if (!(bo->flags & XE_BO_FLAG_SCANOUT)) {
>> + if (!(bo->flags & XE_BO_FLAG_FORCE_WC)) {
>> /*
>> - * XE_BO_FLAG_SCANOUT should ideally be set at creation, or is
>> + * XE_BO_FLAG_FORCE_WC should ideally be set at creation, or is
>> * automatically set when creating FB. We cannot change caching
>> * mode when the bo is VM_BINDed, so we can only set
>> * coherency with display when unbound.
>> @@ -57,7 +57,7 @@ int intel_fb_bo_framebuffer_init(struct drm_gem_object *obj,
>> ret = -EINVAL;
>> goto err;
>> }
>> - bo->flags |= XE_BO_FLAG_SCANOUT;
>> + bo->flags |= XE_BO_FLAG_FORCE_WC;
>> }
>> ttm_bo_unreserve(&bo->ttm);
>> return 0;
>> diff --git a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
>> index 87af5646c938..d7030e4d814c 100644
>> --- a/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
>> +++ b/drivers/gpu/drm/xe/display/intel_fbdev_fb.c
>> @@ -56,9 +56,11 @@ struct drm_gem_object *intel_fbdev_fb_bo_create(struct drm_device *drm, int size
>> if (intel_fbdev_fb_prefer_stolen(drm, size)) {
>> obj = xe_bo_create_pin_map_novm(xe, xe_device_get_root_tile(xe),
>> size,
>> - ttm_bo_type_kernel, XE_BO_FLAG_SCANOUT |
>> + ttm_bo_type_kernel,
>> + XE_BO_FLAG_FORCE_WC |
>> XE_BO_FLAG_STOLEN |
>> - XE_BO_FLAG_GGTT, false);
>> + XE_BO_FLAG_GGTT,
>> + false);
>> if (!IS_ERR(obj))
>> drm_info(&xe->drm, "Allocated fbdev into stolen\n");
>> else
>> @@ -69,9 +71,11 @@ struct drm_gem_object *intel_fbdev_fb_bo_create(struct drm_device *drm, int size
>>
>> if (IS_ERR(obj)) {
>> obj = xe_bo_create_pin_map_novm(xe, xe_device_get_root_tile(xe), size,
>> - ttm_bo_type_kernel, XE_BO_FLAG_SCANOUT |
>> + ttm_bo_type_kernel,
>> + XE_BO_FLAG_FORCE_WC |
>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) |
>> - XE_BO_FLAG_GGTT, false);
>> + XE_BO_FLAG_GGTT,
>> + false);
>> }
>>
>> if (IS_ERR(obj)) {
>> diff --git a/drivers/gpu/drm/xe/display/xe_dsb_buffer.c b/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
>> index 1c67a950c6ad..a7158c73a14c 100644
>> --- a/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
>> +++ b/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
>> @@ -54,7 +54,9 @@ static struct intel_dsb_buffer *xe_dsb_buffer_create(struct drm_device *drm, siz
>> PAGE_ALIGN(size),
>> ttm_bo_type_kernel,
>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) |
>> - XE_BO_FLAG_SCANOUT | XE_BO_FLAG_GGTT, false);
>> + XE_BO_FLAG_FORCE_WC |
>> + XE_BO_FLAG_GGTT,
>> + false);
>> if (IS_ERR(obj)) {
>> ret = PTR_ERR(obj);
>> goto err_pin_map;
>> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> index dbbc61032b7f..d4a9eb550cae 100644
>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> @@ -429,7 +429,7 @@ int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
>> return 0;
>>
>> /* We reject creating !SCANOUT fb's, so this is weird.. */
>> - drm_WARN_ON(bo->ttm.base.dev, !(bo->flags & XE_BO_FLAG_SCANOUT));
>> + drm_WARN_ON(bo->ttm.base.dev, !(bo->flags & XE_BO_FLAG_FORCE_WC));
>>
>> vma = __xe_pin_fb_vma(intel_fb, &new_plane_state->view.gtt, alignment);
>>
>> diff --git a/drivers/gpu/drm/xe/display/xe_initial_plane.c b/drivers/gpu/drm/xe/display/xe_initial_plane.c
>> index 65cc0b0c934b..8bcae552dddc 100644
>> --- a/drivers/gpu/drm/xe/display/xe_initial_plane.c
>> +++ b/drivers/gpu/drm/xe/display/xe_initial_plane.c
>> @@ -48,7 +48,7 @@ initial_plane_bo(struct xe_device *xe,
>> if (plane_config->size == 0)
>> return NULL;
>>
>> - flags = XE_BO_FLAG_SCANOUT | XE_BO_FLAG_GGTT;
>> + flags = XE_BO_FLAG_FORCE_WC | XE_BO_FLAG_GGTT;
>>
>> base = round_down(plane_config->base, page_size);
>> if (IS_DGFX(xe)) {
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index 8ff193600443..fe560b0a980a 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -515,7 +515,7 @@ static struct ttm_tt *xe_ttm_tt_create(struct ttm_buffer_object *ttm_bo,
>> * For Xe_LPG and beyond up to NVL-P (excluding), PPGTT PTE
>> * lookups are also non-coherent and require a CPU:WC mapping.
>> */
>> - if ((!bo->cpu_caching && bo->flags & XE_BO_FLAG_SCANOUT) ||
>> + if ((!bo->cpu_caching && bo->flags & XE_BO_FLAG_FORCE_WC) ||
>> (!xe->info.has_cached_pt && bo->flags & XE_BO_FLAG_PAGETABLE))
>> caching = ttm_write_combined;
>> }
>> @@ -3196,8 +3196,14 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING)
>> bo_flags |= XE_BO_FLAG_DEFER_BACKING;
>>
>> + /*
>> + * Display scanout is always non-coherent with the CPU cache.
>> + *
>> + * For Xe_LPG and beyond up to NVL-P (excluding), PPGTT PTE
>> + * lookups are also non-coherent and require a CPU:WC mapping.
>> + */
>
> I believe this comment should be now removed from the other place.
> But it doesn't hurt if you decide to keep in both places.

My bad, I thought you were suggesting a new comment text to add, and
while touching the flag in xe_ttm_tt_create apparently I suffered from
tunnel vision and did no see the same comment there. I will remove that
one but will hold of the respin until more of the series gets comments.

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thank you!

Regards,

Tvrtko

>
>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT)
>> - bo_flags |= XE_BO_FLAG_SCANOUT;
>> + bo_flags |= XE_BO_FLAG_FORCE_WC;
>>
>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) {
>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20))
>> @@ -3209,7 +3215,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>>
>> /* CCS formats need physical placement at a 64K alignment in VRAM. */
>> if ((bo_flags & XE_BO_FLAG_VRAM_MASK) &&
>> - (bo_flags & XE_BO_FLAG_SCANOUT) &&
>> + (args->flags & XE_BO_FLAG_FORCE_WC) &&
>> !(xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K) &&
>> IS_ALIGNED(args->size, SZ_64K))
>> bo_flags |= XE_BO_FLAG_NEEDS_64K;
>> @@ -3229,7 +3235,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>> args->cpu_caching != DRM_XE_GEM_CPU_CACHING_WC))
>> return -EINVAL;
>>
>> - if (XE_IOCTL_DBG(xe, bo_flags & XE_BO_FLAG_SCANOUT &&
>> + if (XE_IOCTL_DBG(xe, bo_flags & XE_BO_FLAG_FORCE_WC &&
>> args->cpu_caching == DRM_XE_GEM_CPU_CACHING_WB))
>> return -EINVAL;
>>
>> @@ -3642,7 +3648,7 @@ int xe_bo_dumb_create(struct drm_file *file_priv,
>> bo = xe_bo_create_user(xe, NULL, args->size,
>> DRM_XE_GEM_CPU_CACHING_WC,
>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) |
>> - XE_BO_FLAG_SCANOUT |
>> + XE_BO_FLAG_FORCE_WC |
>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL);
>> if (IS_ERR(bo))
>> return PTR_ERR(bo);
>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
>> index c914ab719f20..af9a6669c872 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.h
>> +++ b/drivers/gpu/drm/xe/xe_bo.h
>> @@ -35,7 +35,7 @@
>> #define XE_BO_FLAG_PINNED BIT(7)
>> #define XE_BO_FLAG_NO_RESV_EVICT BIT(8)
>> #define XE_BO_FLAG_DEFER_BACKING BIT(9)
>> -#define XE_BO_FLAG_SCANOUT BIT(10)
>> +#define XE_BO_FLAG_FORCE_WC BIT(10)
>> #define XE_BO_FLAG_FIXED_PLACEMENT BIT(11)
>> #define XE_BO_FLAG_PAGETABLE BIT(12)
>> #define XE_BO_FLAG_NEEDS_CPU_ACCESS BIT(13)
>> --
>> 2.52.0
>>