Reviewed-by: Rodrigo Siqueira On 07/30, Nicholas Kazlauskas wrote: > [Why] > Store these in advance so we can reuse them later in commit_tail without > having to reserve the fbo again. > > These will also be used for checking for tiling changes when deciding > to reset the plane or not. > > [How] > This change should mostly be a refactor. Only commit check is affected > for now and I'll drop the get_fb_info calls in prepare_planes and > commit_tail after. > > This runs a prepass loop once we think that all planes have been added > to the context and replaces the get_fb_info calls with accessing the > dm_plane_state instead. > > Cc: Bhawanpreet Lakha > Cc: Rodrigo Siqueira > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 60 +++++++++++-------- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + > 2 files changed, 37 insertions(+), 25 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 8d64f5fde7e2..7cc5ab90ce13 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3700,8 +3700,17 @@ static int fill_dc_scaling_info(const struct drm_plane_state *state, > static int get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb, > uint64_t *tiling_flags, bool *tmz_surface) > { > - struct amdgpu_bo *rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]); > - int r = amdgpu_bo_reserve(rbo, false); > + struct amdgpu_bo *rbo; > + int r; > + > + if (!amdgpu_fb) { > + *tiling_flags = 0; > + *tmz_surface = false; > + return 0; > + } > + > + rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]); > + r = amdgpu_bo_reserve(rbo, false); > > if (unlikely(r)) { > /* Don't show error message when returning -ERESTARTSYS */ > @@ -4124,13 +4133,10 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev, > struct drm_crtc_state *crtc_state) > { > struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state); > - const struct amdgpu_framebuffer *amdgpu_fb = > - to_amdgpu_framebuffer(plane_state->fb); > + struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state); > struct dc_scaling_info scaling_info; > struct dc_plane_info plane_info; > - uint64_t tiling_flags; > int ret; > - bool tmz_surface = false; > bool force_disable_dcc = false; > > ret = fill_dc_scaling_info(plane_state, &scaling_info); > @@ -4142,15 +4148,12 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev, > dc_plane_state->clip_rect = scaling_info.clip_rect; > dc_plane_state->scaling_quality = scaling_info.scaling_quality; > > - ret = get_fb_info(amdgpu_fb, &tiling_flags, &tmz_surface); > - if (ret) > - return ret; > - > force_disable_dcc = adev->asic_type == CHIP_RAVEN && adev->in_suspend; > - ret = fill_dc_plane_info_and_addr(adev, plane_state, tiling_flags, > + ret = fill_dc_plane_info_and_addr(adev, plane_state, > + dm_plane_state->tiling_flags, > &plane_info, > &dc_plane_state->address, > - tmz_surface, > + dm_plane_state->tmz_surface, > force_disable_dcc); > if (ret) > return ret; > @@ -5753,6 +5756,10 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane) > dc_plane_state_retain(dm_plane_state->dc_state); > } > > + /* Framebuffer hasn't been updated yet, so retain old flags. */ > + dm_plane_state->tiling_flags = old_dm_plane_state->tiling_flags; > + dm_plane_state->tmz_surface = old_dm_plane_state->tmz_surface; > + > return &dm_plane_state->base; > } > > @@ -8557,13 +8564,9 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, > continue; > > for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, j) { > - const struct amdgpu_framebuffer *amdgpu_fb = > - to_amdgpu_framebuffer(new_plane_state->fb); > struct dc_plane_info *plane_info = &bundle->plane_infos[num_plane]; > struct dc_flip_addrs *flip_addr = &bundle->flip_addrs[num_plane]; > struct dc_scaling_info *scaling_info = &bundle->scaling_infos[num_plane]; > - uint64_t tiling_flags; > - bool tmz_surface = false; > > new_plane_crtc = new_plane_state->crtc; > new_dm_plane_state = to_dm_plane_state(new_plane_state); > @@ -8610,16 +8613,12 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, > > bundle->surface_updates[num_plane].scaling_info = scaling_info; > > - if (amdgpu_fb) { > - ret = get_fb_info(amdgpu_fb, &tiling_flags, &tmz_surface); > - if (ret) > - goto cleanup; > - > + if (new_plane_state->fb) { > ret = fill_dc_plane_info_and_addr( > - dm->adev, new_plane_state, tiling_flags, > - plane_info, > - &flip_addr->address, tmz_surface, > - false); > + dm->adev, new_plane_state, > + new_dm_plane_state->tiling_flags, > + plane_info, &flip_addr->address, > + new_dm_plane_state->tmz_surface, false); > if (ret) > goto cleanup; > > @@ -8833,6 +8832,17 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, > } > } > > + /* Prepass for updating tiling flags on new planes. */ > + for_each_new_plane_in_state(state, plane, new_plane_state, i) { > + struct dm_plane_state *new_dm_plane_state = to_dm_plane_state(new_plane_state); > + struct amdgpu_framebuffer *new_afb = to_amdgpu_framebuffer(new_plane_state->fb); > + > + ret = get_fb_info(new_afb, &new_dm_plane_state->tiling_flags, > + &new_dm_plane_state->tmz_surface); > + if (ret) > + goto fail; > + } > + > /* Remove exiting planes if they are modified */ > for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) { > ret = dm_update_plane_state(dc, state, plane, > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > index 5b6f879a108c..ad025f5cfd3e 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > @@ -409,6 +409,8 @@ struct dc_plane_state; > struct dm_plane_state { > struct drm_plane_state base; > struct dc_plane_state *dc_state; > + uint64_t tiling_flags; > + bool tmz_surface; > }; > > struct dm_crtc_state { > -- > 2.25.1 > -- Rodrigo Siqueira https://siqueira.tech