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@xxxxxxx>
Reviewed-by: Leo Li <sunpeng.li@xxxxxxx>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
> ---
> .../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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
