drm/amd/display: Fix update type for multiple planes

[Why]
determine_update_type_for_commit() uses pointers to single instance
of local variable to fill scaling/color info for all planes updates.
This is a bug, that leads to incorrect update type for commit in case
of multiple planes per crtc.
Each plane should refer to separate scaling/color data.

[How]
Use arrays for plane properties.
Bundle all properties into a single structure to simplify memory allocation.

Signed-off-by: Roman Li <roman.li@amd.com>
Reviewed-by: Nicholas Kazlauskas <Nicholas.Kazlauskas@amd.com>
Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
This commit is contained in:
Roman Li 2020-01-13 10:26:19 -05:00 committed by Alex Deucher
parent 022205ffbb
commit 7527791e1f

View File

@ -7759,24 +7759,27 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
struct drm_crtc_state *new_crtc_state, *old_crtc_state; struct drm_crtc_state *new_crtc_state, *old_crtc_state;
struct dm_crtc_state *new_dm_crtc_state, *old_dm_crtc_state; struct dm_crtc_state *new_dm_crtc_state, *old_dm_crtc_state;
struct dc_stream_status *status = NULL; struct dc_stream_status *status = NULL;
struct dc_surface_update *updates;
enum surface_update_type update_type = UPDATE_TYPE_FAST; enum surface_update_type update_type = UPDATE_TYPE_FAST;
struct surface_info_bundle {
struct dc_surface_update surface_updates[MAX_SURFACES];
struct dc_plane_info plane_infos[MAX_SURFACES];
struct dc_scaling_info scaling_infos[MAX_SURFACES];
struct dc_flip_addrs flip_addrs[MAX_SURFACES];
struct dc_stream_update stream_update;
} *bundle;
updates = kcalloc(MAX_SURFACES, sizeof(*updates), GFP_KERNEL); bundle = kzalloc(sizeof(*bundle), GFP_KERNEL);
if (!updates) { if (!bundle) {
DRM_ERROR("Failed to allocate plane updates\n"); DRM_ERROR("Failed to allocate update bundle\n");
/* Set type to FULL to avoid crashing in DC*/ /* Set type to FULL to avoid crashing in DC*/
update_type = UPDATE_TYPE_FULL; update_type = UPDATE_TYPE_FULL;
goto cleanup; goto cleanup;
} }
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
struct dc_scaling_info scaling_info;
struct dc_stream_update stream_update;
memset(&stream_update, 0, sizeof(stream_update)); memset(bundle, 0, sizeof(struct surface_info_bundle));
new_dm_crtc_state = to_dm_crtc_state(new_crtc_state); new_dm_crtc_state = to_dm_crtc_state(new_crtc_state);
old_dm_crtc_state = to_dm_crtc_state(old_crtc_state); old_dm_crtc_state = to_dm_crtc_state(old_crtc_state);
@ -7793,8 +7796,9 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, j) { for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, j) {
const struct amdgpu_framebuffer *amdgpu_fb = const struct amdgpu_framebuffer *amdgpu_fb =
to_amdgpu_framebuffer(new_plane_state->fb); to_amdgpu_framebuffer(new_plane_state->fb);
struct dc_plane_info plane_info; struct dc_plane_info *plane_info = &bundle->plane_infos[num_plane];
struct dc_flip_addrs flip_addr; 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; uint64_t tiling_flags;
new_plane_crtc = new_plane_state->crtc; new_plane_crtc = new_plane_state->crtc;
@ -7812,49 +7816,48 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
if (crtc != new_plane_crtc) if (crtc != new_plane_crtc)
continue; continue;
updates[num_plane].surface = new_dm_plane_state->dc_state; bundle->surface_updates[num_plane].surface =
new_dm_plane_state->dc_state;
if (new_crtc_state->mode_changed) { if (new_crtc_state->mode_changed) {
stream_update.dst = new_dm_crtc_state->stream->dst; bundle->stream_update.dst = new_dm_crtc_state->stream->dst;
stream_update.src = new_dm_crtc_state->stream->src; bundle->stream_update.src = new_dm_crtc_state->stream->src;
} }
if (new_crtc_state->color_mgmt_changed) { if (new_crtc_state->color_mgmt_changed) {
updates[num_plane].gamma = bundle->surface_updates[num_plane].gamma =
new_dm_plane_state->dc_state->gamma_correction; new_dm_plane_state->dc_state->gamma_correction;
updates[num_plane].in_transfer_func = bundle->surface_updates[num_plane].in_transfer_func =
new_dm_plane_state->dc_state->in_transfer_func; new_dm_plane_state->dc_state->in_transfer_func;
stream_update.gamut_remap = bundle->stream_update.gamut_remap =
&new_dm_crtc_state->stream->gamut_remap_matrix; &new_dm_crtc_state->stream->gamut_remap_matrix;
stream_update.output_csc_transform = bundle->stream_update.output_csc_transform =
&new_dm_crtc_state->stream->csc_color_matrix; &new_dm_crtc_state->stream->csc_color_matrix;
stream_update.out_transfer_func = bundle->stream_update.out_transfer_func =
new_dm_crtc_state->stream->out_transfer_func; new_dm_crtc_state->stream->out_transfer_func;
} }
ret = fill_dc_scaling_info(new_plane_state, ret = fill_dc_scaling_info(new_plane_state,
&scaling_info); scaling_info);
if (ret) if (ret)
goto cleanup; goto cleanup;
updates[num_plane].scaling_info = &scaling_info; bundle->surface_updates[num_plane].scaling_info = scaling_info;
if (amdgpu_fb) { if (amdgpu_fb) {
ret = get_fb_info(amdgpu_fb, &tiling_flags); ret = get_fb_info(amdgpu_fb, &tiling_flags);
if (ret) if (ret)
goto cleanup; goto cleanup;
memset(&flip_addr, 0, sizeof(flip_addr));
ret = fill_dc_plane_info_and_addr( ret = fill_dc_plane_info_and_addr(
dm->adev, new_plane_state, tiling_flags, dm->adev, new_plane_state, tiling_flags,
&plane_info, plane_info,
&flip_addr.address); &flip_addr->address);
if (ret) if (ret)
goto cleanup; goto cleanup;
updates[num_plane].plane_info = &plane_info; bundle->surface_updates[num_plane].plane_info = plane_info;
updates[num_plane].flip_addr = &flip_addr; bundle->surface_updates[num_plane].flip_addr = flip_addr;
} }
num_plane++; num_plane++;
@ -7875,14 +7878,15 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
status = dc_stream_get_status_from_state(old_dm_state->context, status = dc_stream_get_status_from_state(old_dm_state->context,
new_dm_crtc_state->stream); new_dm_crtc_state->stream);
stream_update.stream = new_dm_crtc_state->stream; bundle->stream_update.stream = new_dm_crtc_state->stream;
/* /*
* TODO: DC modifies the surface during this call so we need * TODO: DC modifies the surface during this call so we need
* to lock here - find a way to do this without locking. * to lock here - find a way to do this without locking.
*/ */
mutex_lock(&dm->dc_lock); mutex_lock(&dm->dc_lock);
update_type = dc_check_update_surfaces_for_stream(dc, updates, num_plane, update_type = dc_check_update_surfaces_for_stream(
&stream_update, status); dc, bundle->surface_updates, num_plane,
&bundle->stream_update, status);
mutex_unlock(&dm->dc_lock); mutex_unlock(&dm->dc_lock);
if (update_type > UPDATE_TYPE_MED) { if (update_type > UPDATE_TYPE_MED) {
@ -7892,7 +7896,7 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
} }
cleanup: cleanup:
kfree(updates); kfree(bundle);
*out_type = update_type; *out_type = update_type;
return ret; return ret;