All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: Fix plane address updates for video surface formats
@ 2019-03-11 13:38 Nicholas Kazlauskas
       [not found] ` <20190311133835.32721-1-nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Nicholas Kazlauskas @ 2019-03-11 13:38 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Leo Li, Nicholas Kazlauskas

[Why]
For new DC planes the correct plane address fields are filled based
on whether the plane had a graphics or video format.

However, when we perform stream and plane updates using DC we only ever
fill in the graphics format fields. This causing corruption and hangs
when using video surface formats like NV12 for planes.

[How]
Use the same logic everywhere we update dc_plane_address - always
fill in the correct fields based on the surface format type.

There are 3 places this is done:

- Atomic check, during DC plane creation
- Atomic commit, during plane prepare_fb
- Atomic commit tail, during amdgpu_dm_commit_planes

We use the fill_plane_tiling_attributes in all 3 locations and it
already needs the address to update DCC attributes, so the surface
address update logic can be moved into this helper.

Cc: Leo Li <sunpeng.li@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 57 +++++++++----------
 1 file changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 59a8045c9e2a..e0c0621f40d4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2457,6 +2457,27 @@ fill_plane_tiling_attributes(struct amdgpu_device *adev,
 
 	memset(tiling_info, 0, sizeof(*tiling_info));
 	memset(dcc, 0, sizeof(*dcc));
+	memset(address, 0, sizeof(*address));
+
+	if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
+		address->type = PLN_ADDR_TYPE_GRAPHICS;
+		address->grph.addr.low_part = lower_32_bits(afb->address);
+		address->grph.addr.high_part = upper_32_bits(afb->address);
+	} else {
+		const struct drm_framebuffer *fb = &afb->base;
+		uint64_t awidth = ALIGN(fb->width, 64);
+		uint64_t chroma_addr = afb->address + awidth * fb->height;
+
+		address->type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
+		address->video_progressive.luma_addr.low_part =
+			lower_32_bits(afb->address);
+		address->video_progressive.luma_addr.high_part =
+			upper_32_bits(afb->address);
+		address->video_progressive.chroma_addr.low_part =
+			lower_32_bits(chroma_addr);
+		address->video_progressive.chroma_addr.high_part =
+			upper_32_bits(chroma_addr);
+	}
 
 	/* Fill GFX8 params */
 	if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) == DC_ARRAY_2D_TILED_THIN1) {
@@ -2571,7 +2592,6 @@ static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
 	memset(&plane_state->address, 0, sizeof(plane_state->address));
 
 	if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
-		plane_state->address.type = PLN_ADDR_TYPE_GRAPHICS;
 		plane_state->plane_size.grph.surface_size.x = 0;
 		plane_state->plane_size.grph.surface_size.y = 0;
 		plane_state->plane_size.grph.surface_size.width = fb->width;
@@ -2583,7 +2603,7 @@ static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
 
 	} else {
 		awidth = ALIGN(fb->width, 64);
-		plane_state->address.type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
+
 		plane_state->plane_size.video.luma_size.x = 0;
 		plane_state->plane_size.video.luma_size.y = 0;
 		plane_state->plane_size.video.luma_size.width = awidth;
@@ -3738,10 +3758,8 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
 	struct drm_gem_object *obj;
 	struct amdgpu_device *adev;
 	struct amdgpu_bo *rbo;
-	uint64_t chroma_addr = 0;
 	struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
-	uint64_t tiling_flags, dcc_address;
-	unsigned int awidth;
+	uint64_t tiling_flags;
 	uint32_t domain;
 	int r;
 
@@ -3794,29 +3812,9 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
 			dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) {
 		struct dc_plane_state *plane_state = dm_plane_state_new->dc_state;
 
-		if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
-			plane_state->address.grph.addr.low_part = lower_32_bits(afb->address);
-			plane_state->address.grph.addr.high_part = upper_32_bits(afb->address);
-
-			dcc_address =
-				get_dcc_address(afb->address, tiling_flags);
-			plane_state->address.grph.meta_addr.low_part =
-				lower_32_bits(dcc_address);
-			plane_state->address.grph.meta_addr.high_part =
-				upper_32_bits(dcc_address);
-		} else {
-			awidth = ALIGN(new_state->fb->width, 64);
-			plane_state->address.type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
-			plane_state->address.video_progressive.luma_addr.low_part
-							= lower_32_bits(afb->address);
-			plane_state->address.video_progressive.luma_addr.high_part
-							= upper_32_bits(afb->address);
-			chroma_addr = afb->address + (u64)awidth * new_state->fb->height;
-			plane_state->address.video_progressive.chroma_addr.low_part
-							= lower_32_bits(chroma_addr);
-			plane_state->address.video_progressive.chroma_addr.high_part
-							= upper_32_bits(chroma_addr);
-		}
+		fill_plane_tiling_attributes(
+			adev, afb, plane_state, &plane_state->tiling_info,
+			&plane_state->dcc, &plane_state->address, tiling_flags);
 	}
 
 	return 0;
@@ -4878,9 +4876,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
 
 		amdgpu_bo_unreserve(abo);
 
-		bundle->flip_addrs[planes_count].address.grph.addr.low_part = lower_32_bits(afb->address);
-		bundle->flip_addrs[planes_count].address.grph.addr.high_part = upper_32_bits(afb->address);
-
 		fill_plane_tiling_attributes(dm->adev, afb, dc_plane,
 			&bundle->plane_infos[planes_count].tiling_info,
 			&bundle->plane_infos[planes_count].dcc,
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amd/display: Fix plane address updates for video surface formats
       [not found] ` <20190311133835.32721-1-nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>
@ 2019-03-11 13:52   ` Li, Sun peng (Leo)
  0 siblings, 0 replies; 2+ messages in thread
From: Li, Sun peng (Leo) @ 2019-03-11 13:52 UTC (permalink / raw)
  To: Kazlauskas, Nicholas, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW



On 2019-03-11 9:38 a.m., Nicholas Kazlauskas wrote:
> [Why]
> For new DC planes the correct plane address fields are filled based
> on whether the plane had a graphics or video format.
> 
> However, when we perform stream and plane updates using DC we only ever
> fill in the graphics format fields. This causing corruption and hangs
> when using video surface formats like NV12 for planes.
> 
> [How]
> Use the same logic everywhere we update dc_plane_address - always
> fill in the correct fields based on the surface format type.
> 
> There are 3 places this is done:
> 
> - Atomic check, during DC plane creation
> - Atomic commit, during plane prepare_fb
> - Atomic commit tail, during amdgpu_dm_commit_planes
> 
> We use the fill_plane_tiling_attributes in all 3 locations and it
> already needs the address to update DCC attributes, so the surface
> address update logic can be moved into this helper.
> 
> Cc: Leo Li <sunpeng.li@amd.com>
Reviewed-by: Leo Li <sunpeng.li@amd.com>

> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 57 +++++++++----------
>   1 file changed, 26 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 59a8045c9e2a..e0c0621f40d4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2457,6 +2457,27 @@ fill_plane_tiling_attributes(struct amdgpu_device *adev,
>   
>   	memset(tiling_info, 0, sizeof(*tiling_info));
>   	memset(dcc, 0, sizeof(*dcc));
> +	memset(address, 0, sizeof(*address));
> +
> +	if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
> +		address->type = PLN_ADDR_TYPE_GRAPHICS;
> +		address->grph.addr.low_part = lower_32_bits(afb->address);
> +		address->grph.addr.high_part = upper_32_bits(afb->address);
> +	} else {
> +		const struct drm_framebuffer *fb = &afb->base;
> +		uint64_t awidth = ALIGN(fb->width, 64);
> +		uint64_t chroma_addr = afb->address + awidth * fb->height;
> +
> +		address->type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
> +		address->video_progressive.luma_addr.low_part =
> +			lower_32_bits(afb->address);
> +		address->video_progressive.luma_addr.high_part =
> +			upper_32_bits(afb->address);
> +		address->video_progressive.chroma_addr.low_part =
> +			lower_32_bits(chroma_addr);
> +		address->video_progressive.chroma_addr.high_part =
> +			upper_32_bits(chroma_addr);
> +	}
>   
>   	/* Fill GFX8 params */
>   	if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) == DC_ARRAY_2D_TILED_THIN1) {
> @@ -2571,7 +2592,6 @@ static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
>   	memset(&plane_state->address, 0, sizeof(plane_state->address));
>   
>   	if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
> -		plane_state->address.type = PLN_ADDR_TYPE_GRAPHICS;
>   		plane_state->plane_size.grph.surface_size.x = 0;
>   		plane_state->plane_size.grph.surface_size.y = 0;
>   		plane_state->plane_size.grph.surface_size.width = fb->width;
> @@ -2583,7 +2603,7 @@ static int fill_plane_attributes_from_fb(struct amdgpu_device *adev,
>   
>   	} else {
>   		awidth = ALIGN(fb->width, 64);
> -		plane_state->address.type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
> +
>   		plane_state->plane_size.video.luma_size.x = 0;
>   		plane_state->plane_size.video.luma_size.y = 0;
>   		plane_state->plane_size.video.luma_size.width = awidth;
> @@ -3738,10 +3758,8 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>   	struct drm_gem_object *obj;
>   	struct amdgpu_device *adev;
>   	struct amdgpu_bo *rbo;
> -	uint64_t chroma_addr = 0;
>   	struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
> -	uint64_t tiling_flags, dcc_address;
> -	unsigned int awidth;
> +	uint64_t tiling_flags;
>   	uint32_t domain;
>   	int r;
>   
> @@ -3794,29 +3812,9 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>   			dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) {
>   		struct dc_plane_state *plane_state = dm_plane_state_new->dc_state;
>   
> -		if (plane_state->format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
> -			plane_state->address.grph.addr.low_part = lower_32_bits(afb->address);
> -			plane_state->address.grph.addr.high_part = upper_32_bits(afb->address);
> -
> -			dcc_address =
> -				get_dcc_address(afb->address, tiling_flags);
> -			plane_state->address.grph.meta_addr.low_part =
> -				lower_32_bits(dcc_address);
> -			plane_state->address.grph.meta_addr.high_part =
> -				upper_32_bits(dcc_address);
> -		} else {
> -			awidth = ALIGN(new_state->fb->width, 64);
> -			plane_state->address.type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
> -			plane_state->address.video_progressive.luma_addr.low_part
> -							= lower_32_bits(afb->address);
> -			plane_state->address.video_progressive.luma_addr.high_part
> -							= upper_32_bits(afb->address);
> -			chroma_addr = afb->address + (u64)awidth * new_state->fb->height;
> -			plane_state->address.video_progressive.chroma_addr.low_part
> -							= lower_32_bits(chroma_addr);
> -			plane_state->address.video_progressive.chroma_addr.high_part
> -							= upper_32_bits(chroma_addr);
> -		}
> +		fill_plane_tiling_attributes(
> +			adev, afb, plane_state, &plane_state->tiling_info,
> +			&plane_state->dcc, &plane_state->address, tiling_flags);
>   	}
>   
>   	return 0;
> @@ -4878,9 +4876,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   
>   		amdgpu_bo_unreserve(abo);
>   
> -		bundle->flip_addrs[planes_count].address.grph.addr.low_part = lower_32_bits(afb->address);
> -		bundle->flip_addrs[planes_count].address.grph.addr.high_part = upper_32_bits(afb->address);
> -
>   		fill_plane_tiling_attributes(dm->adev, afb, dc_plane,
>   			&bundle->plane_infos[planes_count].tiling_info,
>   			&bundle->plane_infos[planes_count].dcc,
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-03-11 13:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 13:38 [PATCH] drm/amd/display: Fix plane address updates for video surface formats Nicholas Kazlauskas
     [not found] ` <20190311133835.32721-1-nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>
2019-03-11 13:52   ` Li, Sun peng (Leo)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.