On 26/01/2024 21:08, Juha-Pekka Heikkila wrote:
Store pat index from xe_vma to xe_bo and check if bo was pinned
as framebuffer and verify pat index is not changing for pinned
framebuffers.

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikk...@gmail.com>
---
  drivers/gpu/drm/xe/xe_pt.c | 23 +++++++++++++++++++----
  1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index de1030a47588..0a5d7c7543b1 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -1208,10 +1208,11 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma 
*vma, struct xe_exec_queue
        struct dma_fence *fence;
        struct invalidation_fence *ifence = NULL;
        struct xe_range_fence *rfence;
+       struct xe_bo *bo = xe_vma_bo(vma);
        int err;
bind_pt_update.locked = false;
-       xe_bo_assert_held(xe_vma_bo(vma));
+       xe_bo_assert_held(bo);
        xe_vm_assert_held(vm);
vm_dbg(&xe_vma_vm(vma)->xe->drm,
@@ -1252,8 +1253,22 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma 
*vma, struct xe_exec_queue
                return ERR_PTR(-ENOMEM);
        }
+ /*
+        * BO which has XE_BO_SCANOUT_BIT set and was pinned as framebuffer
+        * before with different PAT index cannot be bound with different PAT
+        * index. This is to prevent switching CCS on/off from framebuffers
+        * on the fly.
+        */
+       if (bo) {
+               if (bo->flags & XE_BO_SCANOUT_BIT && bo->pat_index_scanout &&

Note that pat_index = 0 is usually a valid index...

+                   bo->pat_index_scanout != vma->pat_index)
+                       return ERR_PTR(-EINVAL);
+
+               bo->pat_index = vma->pat_index;
+       }

...what about something like:

if (bo.has_sealed_pat_index && bo.sealed_pat_index != vma->pat_index)
    return ERR_PTR();
else if (SCANOUT) {
    bo.has_sealed_pat_index = true;
    bo.sealed_pat_index = vma->pat_index;
}

Also, this and the previous patch should probably be squashed together? Other question is if we should only apply this on xe2?

+
        fence = xe_migrate_update_pgtables(tile->migrate,
-                                          vm, xe_vma_bo(vma), q,
+                                          vm, bo, q,
                                           entries, num_entries,
                                           syncs, num_syncs,
                                           &bind_pt_update.base);
@@ -1287,8 +1302,8 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma 
*vma, struct xe_exec_queue
                                   DMA_RESV_USAGE_KERNEL :
                                   DMA_RESV_USAGE_BOOKKEEP);
- if (!xe_vma_has_no_bo(vma) && !xe_vma_bo(vma)->vm)
-                       dma_resv_add_fence(xe_vma_bo(vma)->ttm.base.resv, fence,
+               if (!xe_vma_has_no_bo(vma) && !bo->vm)
+                       dma_resv_add_fence(bo->ttm.base.resv, fence,
                                           DMA_RESV_USAGE_BOOKKEEP);
                xe_pt_commit_bind(vma, entries, num_entries, rebind,
                                  bind_pt_update.locked ? &deferred : NULL);

Reply via email to