[PATCH 2/2] drm/xe/display: Unify display page table mapping
Juha-Pekka Heikkilä
juhapekka.heikkila at gmail.com
Tue Feb 4 12:10:38 UTC 2025
On Tue, Feb 4, 2025 at 1:38 PM Tvrtko Ursulin <tursulin at ursulin.net> wrote:
>
>
> On 04/02/2025 11:25, Tvrtko Ursulin wrote:
> >
> > On 14/01/2025 18:04, Juha-Pekka Heikkila wrote:
> >> Unify writing of remapped and straight DPTs. Take out writing of
> >> rotated DPT since Xe doesn't support any platform that does
> >> 90 degree rotated framebuffers.
> >
> > Consider giving my series which fixes AuxCSS a look instead? Given there
> > is external interest to have all that work on ADL-P. I could respin with
> > rotated cleaned up in a similar fashion I did for remapped. Maintenance
> > cost is minimal since it is all very contained.
>
> Oh ADL-P does not support hardware rotation. I missed the fact it is
> display version 13..
>
> But the point regarding changes to remapping code are still valid I
> think, since to suport aux plane the refactoring from this patch looks
> in the wrong direction.
>
Yea, I kinda agree here with Tvrtko. I did earlier mention this to
JSaa. That aux thing will need to be checked.
Though, how far are we with your set Tvrtko? It did seem lot kms_ccs
tests failed.
https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-144186v1/shards-all.html?testfilter=kms_ccs.*crc.*gen12&hosts=adlp
Did you check you had compression when you had clean image on your
machine? (aux surface was not zeroes) Those failures on my link ci are
with failing crc which mean the picture on screen was not correct.
/Juha-Pekka
> >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> >> ---
> >> drivers/gpu/drm/xe/display/xe_fb_pin.c | 141 +++++++++----------------
> >> 1 file changed, 48 insertions(+), 93 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >> b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >> index c28885316986..70322f28eee5 100644
> >> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> >> @@ -14,66 +14,44 @@
> >> #include "xe_ggtt.h"
> >> #include "xe_pm.h"
> >> -static void
> >> -write_dpt_rotated(struct xe_bo *bo, struct iosys_map *map, u32
> >> *dpt_ofs, u32 bo_ofs,
> >> - u32 width, u32 height, u32 src_stride, u32 dst_stride)
> >> +static void encode_and_write_pte(struct xe_bo *bo, struct iosys_map
> >> *map,
> >> + u32 *ofs, u32 src_idx, struct xe_device *xe)
> >> {
> >> - struct xe_device *xe = xe_bo_device(bo);
> >> struct xe_ggtt *ggtt = xe_device_get_root_tile(xe)->mem.ggtt;
> >> - u32 column, row;
> >> -
> >> - /* TODO: Maybe rewrite so we can traverse the bo addresses
> >> sequentially,
> >> - * by writing dpt/ggtt in a different order?
> >> - */
> >> -
> >> - for (column = 0; column < width; column++) {
> >> - u32 src_idx = src_stride * (height - 1) + column + bo_ofs;
> >> -
> >> - for (row = 0; row < height; row++) {
> >> - u64 pte = ggtt->pt_ops->pte_encode_bo(bo, src_idx *
> >> XE_PAGE_SIZE,
> >> - xe->pat.idx[XE_CACHE_NONE]);
> >> -
> >> - iosys_map_wr(map, *dpt_ofs, u64, pte);
> >> - *dpt_ofs += 8;
> >> - src_idx -= src_stride;
> >> - }
> >> -
> >> - /* The DE ignores the PTEs for the padding tiles */
> >> - *dpt_ofs += (dst_stride - height) * 8;
> >> - }
> >> -
> >> - /* Align to next page */
> >> - *dpt_ofs = ALIGN(*dpt_ofs, 4096);
> >> + u64 pte = ggtt->pt_ops->pte_encode_bo(bo, src_idx * XE_PAGE_SIZE,
> >> + xe->pat.idx[XE_CACHE_NONE]);
> >> + iosys_map_wr(map, *ofs, u64, pte);
> >> + *ofs += 8;
> >> }
> >> -static void
> >> -write_dpt_remapped(struct xe_bo *bo, struct iosys_map *map, u32
> >> *dpt_ofs,
> >> - u32 bo_ofs, u32 width, u32 height, u32 src_stride,
> >> - u32 dst_stride)
> >> +static void write_dpt(struct xe_bo *bo, struct iosys_map *map, u32
> >> *dpt_ofs,
> >> + const struct intel_remapped_plane_info *plane,
> >> + enum i915_gtt_view_type type)
> >> {
> >> struct xe_device *xe = xe_bo_device(bo);
> >> - struct xe_ggtt *ggtt = xe_device_get_root_tile(xe)->mem.ggtt;
> >> - u64 (*pte_encode_bo)(struct xe_bo *bo, u64 bo_offset, u16 pat_index)
> >> - = ggtt->pt_ops->pte_encode_bo;
> >> - u32 column, row;
> >> -
> >> - for (row = 0; row < height; row++) {
> >> - u32 src_idx = src_stride * row + bo_ofs;
> >> -
> >> - for (column = 0; column < width; column++) {
> >> - iosys_map_wr(map, *dpt_ofs, u64,
> >> - pte_encode_bo(bo, src_idx * XE_PAGE_SIZE,
> >> - xe->pat.idx[XE_CACHE_NONE]));
> >> -
> >> - *dpt_ofs += 8;
> >> - src_idx++;
> >> + const u32 dpt_even = (plane->dst_stride - plane->width) * 8;
> >> + const u32 dest_width = plane->width;
> >> + const u32 dest_height = plane->height;
> >> + u32 src_idx;
> >> +
> >> + for (u32 row = 0; row < dest_height; ++row) {
> >> + for (u32 column = 0; column < dest_width; ++column) {
> >> + switch (type) {
> >> + case I915_GTT_VIEW_NORMAL:
> >> + src_idx = plane->offset + column;
> >> + break;
> >> + case I915_GTT_VIEW_REMAPPED:
> >> + src_idx = plane->offset +
> >> + row * plane->src_stride + column;
> >> + break;
> >> + default:
> >> + WARN(1, "Unsupported GTT view type: %d", type);
> >> + return;
> >> + }
> >> + encode_and_write_pte(bo, map, dpt_ofs, src_idx, xe);
> >> }
> >> -
> >> - /* The DE ignores the PTEs for the padding tiles */
> >> - *dpt_ofs += (dst_stride - width) * 8;
> >> + *dpt_ofs += dpt_even;
> >> }
> >> -
> >> - /* Align to next page */
> >> *dpt_ofs = ALIGN(*dpt_ofs, 4096);
> >> }
> >> @@ -125,59 +103,36 @@ static int __xe_pin_fb_vma_dpt(const struct
> >> intel_framebuffer *fb,
> >> {
> >> struct xe_device *xe = to_xe_device(fb->base.dev);
> >> struct xe_tile *tile0 = xe_device_get_root_tile(xe);
> >> - struct xe_ggtt *ggtt = tile0->mem.ggtt;
> >> struct drm_gem_object *obj = intel_fb_bo(&fb->base);
> >> struct xe_bo *bo = gem_to_xe_bo(obj), *dpt;
> >> u32 dpt_size, size = bo->ttm.base.size;
> >> + const struct intel_remapped_plane_info *plane;
> >> + u32 i, plane_count, dpt_ofs = 0;
> >> + struct intel_remapped_plane_info normal_plane;
> >> - if (view->type == I915_GTT_VIEW_NORMAL)
> >> + if (view->type == I915_GTT_VIEW_NORMAL) {
> >> dpt_size = ALIGN(size / XE_PAGE_SIZE * 8, XE_PAGE_SIZE);
> >> - else if (view->type == I915_GTT_VIEW_REMAPPED)
> >> - dpt_size =
> >> ALIGN(intel_remapped_info_size(&fb->remapped_view.gtt.remapped) * 8,
> >> - XE_PAGE_SIZE);
> >> - else
> >> - /* display uses 4K tiles instead of bytes here, convert to
> >> entries.. */
> >> - dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
> >> + normal_plane.offset = 0;
> >> + normal_plane.width = size / XE_PAGE_SIZE;
> >> + normal_plane.height = 1;
> >> + normal_plane.src_stride = size / XE_PAGE_SIZE;
> >> + normal_plane.dst_stride = size / XE_PAGE_SIZE;
> >> + plane = &normal_plane;
> >> + plane_count = 1;
> >> + } else {
> >> + dpt_size = ALIGN(intel_remapped_info_size(&view->remapped) * 8,
> >> XE_PAGE_SIZE);
> >> + plane = view->remapped.plane;
> >> + plane_count = ARRAY_SIZE(view->remapped.plane);
> >> + }
> >> dpt = xe_alloc_dpt_bo(xe, tile0, dpt_size, physical_alignment);
> >> if (IS_ERR(dpt))
> >> return PTR_ERR(dpt);
> >> - if (view->type == I915_GTT_VIEW_NORMAL) {
> >> - u32 x;
> >> -
> >> - for (x = 0; x < size / XE_PAGE_SIZE; x++) {
> >> - u64 pte = ggtt->pt_ops->pte_encode_bo(bo, x * XE_PAGE_SIZE,
> >> - xe->pat.idx[XE_CACHE_NONE]);
> >> -
> >> - iosys_map_wr(&dpt->vmap, x * 8, u64, pte);
> >> - }
> >> - } else if (view->type == I915_GTT_VIEW_REMAPPED) {
> >> - const struct intel_remapped_info *remap_info = &view->remapped;
> >> - u32 i, dpt_ofs = 0;
> >> -
> >> - for (i = 0; i < ARRAY_SIZE(remap_info->plane); i++)
> >> - write_dpt_remapped(bo, &dpt->vmap, &dpt_ofs,
> >> - remap_info->plane[i].offset,
> >> - remap_info->plane[i].width,
> >> - remap_info->plane[i].height,
> >> - remap_info->plane[i].src_stride,
> >> - remap_info->plane[i].dst_stride);
> >> -
> >> - } else {
> >> - const struct intel_rotation_info *rot_info = &view->rotated;
> >> - u32 i, dpt_ofs = 0;
> >> -
> >> - for (i = 0; i < ARRAY_SIZE(rot_info->plane); i++)
> >> - write_dpt_rotated(bo, &dpt->vmap, &dpt_ofs,
> >> - rot_info->plane[i].offset,
> >> - rot_info->plane[i].width,
> >> - rot_info->plane[i].height,
> >> - rot_info->plane[i].src_stride,
> >> - rot_info->plane[i].dst_stride);
> >> - }
> >> + for (i = 0; i < plane_count; i++)
> >> + write_dpt(bo, &dpt->vmap, &dpt_ofs, &plane[i], view->type);
> >> vma->dpt = dpt;
> >> vma->node = dpt->ggtt_node[tile0->id];
More information about the Intel-xe
mailing list