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 <[email protected]> Reviewed-by: Leo Li <[email protected]>
> Signed-off-by: Nicholas Kazlauskas <[email protected]> > --- > .../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 [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
