[Intel-xe] [PATCH v3 0/3] Enable remapped display page table on Xe

[Intel-xe] [PATCH v3 0/3] Enable remapped display page table on Xe

[Juha-Pekka Heikkila]

Here added writing of remapped display page table and taken into use
where needed.

v2. couple of things fixed after forgot to run checkpatch

v3. rebase

/Juha-Pekka

Juha-Pekka Heikkila (3):
  drm/xe/display: Add writing of remapped dpt
  drm/xe/display: Enable remapped display page tables
  drm/xe/display: Don't try to use vram if not available

 drivers/gpu/drm/xe/display/xe_fb_pin.c | 63 +++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 11 deletions(-)

-- 
2.25.1

[Intel-xe] [PATCH v3 1/3] drm/xe/display: Add writing of remapped dpt

[Juha-Pekka Heikkila]

Xe need to use remapped display page table for tiled framebuffers
on anywhere else than DG2. Here add function to write such dpt

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 drivers/gpu/drm/xe/display/xe_fb_pin.c | 31 ++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
index b7a04fba3585..8a1cb6c28eb0 100644
--- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
@@ -45,6 +45,37 @@ write_dpt_rotated(struct xe_bo *bo, struct iosys_map *map, u32 *dpt_ofs, u32 bo_
 	*dpt_ofs = ALIGN(*dpt_ofs, 4096);
 }
 
+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)
+{
+	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,
+			     enum xe_cache_level cache) = 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_CACHE_WB));
+
+			*dpt_ofs += 8;
+			src_idx++;
+		}
+
+		/* The DE ignores the PTEs for the padding tiles */
+		*dpt_ofs += (dst_stride - width) * 8;
+	}
+
+	/* Align to next page */
+	*dpt_ofs = ALIGN(*dpt_ofs, 4096);
+}
+
 static int __xe_pin_fb_vma_dpt(struct intel_framebuffer *fb,
 			       const struct i915_gtt_view *view,
 			       struct i915_vma *vma)
-- 
2.25.1

[Intel-xe] [PATCH v3 2/3] drm/xe/display: Enable remapped display page tables

[Juha-Pekka Heikkila]

Enable usage of remapped display page tables where needed.

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 drivers/gpu/drm/xe/display/xe_fb_pin.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
index 8a1cb6c28eb0..e8e38091c8e6 100644
--- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
@@ -88,6 +88,9 @@ static int __xe_pin_fb_vma_dpt(struct intel_framebuffer *fb,
 
 	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,
@@ -119,6 +122,18 @@ static int __xe_pin_fb_vma_dpt(struct intel_framebuffer *fb,
 
 			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;
@@ -240,12 +255,6 @@ static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
 	if (!vma)
 		return ERR_PTR(-ENODEV);
 
-	/* Remapped view is only required on ADL-P, which xe doesn't support. */
-	if (XE_WARN_ON(view->type == I915_GTT_VIEW_REMAPPED)) {
-		ret = -ENODEV;
-		goto err;
-	}
-
 	if (IS_DGFX(to_xe_device(bo->ttm.base.dev)) &&
 	    intel_fb_rc_ccs_cc_plane(&fb->base) >= 0 &&
 	    !(bo->flags & XE_BO_NEEDS_CPU_ACCESS)) {
-- 
2.25.1

[Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if not available

[Juha-Pekka Heikkila]

Trying to get bo from vram when vram not available will cause
WARN_ON() hence avoid touching vram if not available.

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 drivers/gpu/drm/xe/display/xe_fb_pin.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
index e8e38091c8e6..a79c2416579f 100644
--- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
@@ -83,7 +83,7 @@ static int __xe_pin_fb_vma_dpt(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 xe_bo *bo = intel_fb_obj(&fb->base), *dpt;
+	struct xe_bo *bo = intel_fb_obj(&fb->base), *dpt = ERR_PTR(-EINVAL);
 	u32 dpt_size, size = bo->ttm.base.size;
 
 	if (view->type == I915_GTT_VIEW_NORMAL)
@@ -96,10 +96,11 @@ static int __xe_pin_fb_vma_dpt(struct intel_framebuffer *fb,
 		dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
 				 XE_PAGE_SIZE);
 
-	dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
-				  ttm_bo_type_kernel,
-				  XE_BO_CREATE_VRAM0_BIT |
-				  XE_BO_CREATE_GGTT_BIT);
+	if (tile0->mem.vram.usable_size)
+		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
+					   ttm_bo_type_kernel,
+					   XE_BO_CREATE_VRAM0_BIT |
+					   XE_BO_CREATE_GGTT_BIT);
 	if (IS_ERR(dpt))
 		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
 					   ttm_bo_type_kernel,
-- 
2.25.1

Re: [Intel-xe] [PATCH v3 0/3] Enable remapped display page table on Xe

[Souza, Jose]

Thanks, this fixes https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/606
Can you please close this bug when this gets merged?

Tested-by: José Roberto de Souza <jose.souza@intel.com>

On Thu, 2023-10-05 at 21:34 +0300, Juha-Pekka Heikkila wrote:
> Here added writing of remapped display page table and taken into use
> where needed.
> 
> v2. couple of things fixed after forgot to run checkpatch
> 
> v3. rebase
> 
> /Juha-Pekka
> 
> Juha-Pekka Heikkila (3):
>   drm/xe/display: Add writing of remapped dpt
>   drm/xe/display: Enable remapped display page tables
>   drm/xe/display: Don't try to use vram if not available
> 
>  drivers/gpu/drm/xe/display/xe_fb_pin.c | 63 +++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 11 deletions(-)
> 

Re: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if not available

[Ruhl, Michael J]

>-----Original Message-----
>From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Juha-
>Pekka Heikkila
>Sent: Thursday, October 5, 2023 2:34 PM
>To: intel-xe@lists.freedesktop.org
>Subject: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if not
>available
>
>Trying to get bo from vram when vram not available will cause
>WARN_ON() hence avoid touching vram if not available.
>
>Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>---
> drivers/gpu/drm/xe/display/xe_fb_pin.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>index e8e38091c8e6..a79c2416579f 100644
>--- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>@@ -83,7 +83,7 @@ static int __xe_pin_fb_vma_dpt(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 xe_bo *bo = intel_fb_obj(&fb->base), *dpt;
>+	struct xe_bo *bo = intel_fb_obj(&fb->base), *dpt = ERR_PTR(-EINVAL);
> 	u32 dpt_size, size = bo->ttm.base.size;
>
> 	if (view->type == I915_GTT_VIEW_NORMAL)
>@@ -96,10 +96,11 @@ static int __xe_pin_fb_vma_dpt(struct
>intel_framebuffer *fb,
> 		dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
> 				 XE_PAGE_SIZE);
>
>-	dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>-				  ttm_bo_type_kernel,
>-				  XE_BO_CREATE_VRAM0_BIT |
>-				  XE_BO_CREATE_GGTT_BIT);
>+	if (tile0->mem.vram.usable_size)
>+		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>+					   ttm_bo_type_kernel,
>+					   XE_BO_CREATE_VRAM0_BIT |
>+					   XE_BO_CREATE_GGTT_BIT);
> 	if (IS_ERR(dpt))

Hmm, seems like just 'else' here would be sufficient (if _size) vram0 else sysmem) ?

It also seems like maybe this check should be in the xe_bo_create_pin_map function?
I.e. validate that the requested region bit is valid before creating the bo?

That would let you keep the pattern for the IS_ERR without the init..

Just some thoughts.

M

> 		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
> 					   ttm_bo_type_kernel,
>--
>2.25.1

Re: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if not available

[Juha-Pekka Heikkila]

On 5.10.2023 21.56, Ruhl, Michael J wrote:
>> -----Original Message-----
>> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Juha-
>> Pekka Heikkila
>> Sent: Thursday, October 5, 2023 2:34 PM
>> To: intel-xe@lists.freedesktop.org
>> Subject: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if not
>> available
>>
>> Trying to get bo from vram when vram not available will cause
>> WARN_ON() hence avoid touching vram if not available.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>> drivers/gpu/drm/xe/display/xe_fb_pin.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> index e8e38091c8e6..a79c2416579f 100644
>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>> @@ -83,7 +83,7 @@ static int __xe_pin_fb_vma_dpt(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 xe_bo *bo = intel_fb_obj(&fb->base), *dpt;
>> +	struct xe_bo *bo = intel_fb_obj(&fb->base), *dpt = ERR_PTR(-EINVAL);
>> 	u32 dpt_size, size = bo->ttm.base.size;
>>
>> 	if (view->type == I915_GTT_VIEW_NORMAL)
>> @@ -96,10 +96,11 @@ static int __xe_pin_fb_vma_dpt(struct
>> intel_framebuffer *fb,
>> 		dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
>> 				 XE_PAGE_SIZE);
>>
>> -	dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>> -				  ttm_bo_type_kernel,
>> -				  XE_BO_CREATE_VRAM0_BIT |
>> -				  XE_BO_CREATE_GGTT_BIT);
>> +	if (tile0->mem.vram.usable_size)
>> +		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>> +					   ttm_bo_type_kernel,
>> +					   XE_BO_CREATE_VRAM0_BIT |
>> +					   XE_BO_CREATE_GGTT_BIT);
>> 	if (IS_ERR(dpt))
> 
> Hmm, seems like just 'else' here would be sufficient (if _size) vram0 else sysmem) ?

That's good idea, I'll send new version.

> 
> It also seems like maybe this check should be in the xe_bo_create_pin_map function?
> I.e. validate that the requested region bit is valid before creating the bo?
> 
> That would let you keep the pattern for the IS_ERR without the init..

This I didn't quite understand, I'm still new to how things work on Xe.

/Juha-Pekka

> 
> Just some thoughts.
> 
> M
> 
>> 		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>> 					   ttm_bo_type_kernel,
>> --
>> 2.25.1
> 

Re: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if not available

[Ruhl, Michael J]

>-----Original Message-----
>From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>Sent: Friday, October 6, 2023 9:43 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>; intel-xe@lists.freedesktop.org
>Subject: Re: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if
>not available
>
>On 5.10.2023 21.56, Ruhl, Michael J wrote:
>>> -----Original Message-----
>>> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Juha-
>>> Pekka Heikkila
>>> Sent: Thursday, October 5, 2023 2:34 PM
>>> To: intel-xe@lists.freedesktop.org
>>> Subject: [Intel-xe] [PATCH v3 3/3] drm/xe/display: Don't try to use vram if
>not
>>> available
>>>
>>> Trying to get bo from vram when vram not available will cause
>>> WARN_ON() hence avoid touching vram if not available.
>>>
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>> ---
>>> drivers/gpu/drm/xe/display/xe_fb_pin.c | 11 ++++++-----
>>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> index e8e38091c8e6..a79c2416579f 100644
>>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>> @@ -83,7 +83,7 @@ static int __xe_pin_fb_vma_dpt(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 xe_bo *bo = intel_fb_obj(&fb->base), *dpt;
>>> +	struct xe_bo *bo = intel_fb_obj(&fb->base), *dpt = ERR_PTR(-EINVAL);
>>> 	u32 dpt_size, size = bo->ttm.base.size;
>>>
>>> 	if (view->type == I915_GTT_VIEW_NORMAL)
>>> @@ -96,10 +96,11 @@ static int __xe_pin_fb_vma_dpt(struct
>>> intel_framebuffer *fb,
>>> 		dpt_size = ALIGN(intel_rotation_info_size(&view->rotated) * 8,
>>> 				 XE_PAGE_SIZE);
>>>
>>> -	dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>> -				  ttm_bo_type_kernel,
>>> -				  XE_BO_CREATE_VRAM0_BIT |
>>> -				  XE_BO_CREATE_GGTT_BIT);
>>> +	if (tile0->mem.vram.usable_size)
>>> +		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>> +					   ttm_bo_type_kernel,
>>> +					   XE_BO_CREATE_VRAM0_BIT |
>>> +					   XE_BO_CREATE_GGTT_BIT);
>>> 	if (IS_ERR(dpt))
>>
>> Hmm, seems like just 'else' here would be sufficient (if _size) vram0 else
>sysmem) ?
>
>That's good idea, I'll send new version.
>
>>
>> It also seems like maybe this check should be in the xe_bo_create_pin_map
>function?
>> I.e. validate that the requested region bit is valid before creating the bo?
>>
>> That would let you keep the pattern for the IS_ERR without the init..
>
>This I didn't quite understand, I'm still new to how things work on Xe.

I was suggesting that xe_bo_create_pin_map() should validate that the VRAM0/1 bit was valid
for the tile and return  an error if not.

So you would have no change here, but eh xe_bo_create_pin_map() would get updated...

No worries. 😊

I will take a look at your refresh...

M

>/Juha-Pekka
>
>>
>> Just some thoughts.
>>
>> M
>>
>>> 		dpt = xe_bo_create_pin_map(xe, tile0, NULL, dpt_size,
>>> 					   ttm_bo_type_kernel,
>>> --
>>> 2.25.1
>>