From: Andrey Grodzovsky Date: Mon, 22 May 2017 21:55:38 +0000 (-0400) Subject: drm/amd/display: Fix handling of scaling and underscan. X-Git-Tag: v5.15~9960^2~23^2~461 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ccaa73897109c521a9fda82a50c988e9cba947e7;p=platform%2Fkernel%2Flinux-starfive.git drm/amd/display: Fix handling of scaling and underscan. Summury of changes: 1: Both in check and commit Connector properties were handled as part of for_each(crtc) loop while they shoud have been handled in a dedicated for_each(connector) loop since they are connector properties. Moved. 2: Removed hacky plane add in amdgpu_dm_connector_atomic_set_property to force iteration on plane forconnector property. This was causing double call to commit_surface_for_stream both in crtc loop and plane loop. 3: Remove middleman DC interface and call dc_commit_surfaces_to_stream directly to increase code clarity. Remove it from atomic_commit. Signed-off-by: Andrey Grodzovsky Reviewed-by: Harry Wentland Signed-off-by: Alex Deucher --- diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index 3c1baa2..4da5a71 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c @@ -682,9 +682,8 @@ struct amdgpu_connector *aconnector_from_drm_crtc_id( static void update_stream_scaling_settings( const struct drm_display_mode *mode, const struct dm_connector_state *dm_state, - const struct dc_stream *stream) + struct dc_stream *stream) { - struct amdgpu_device *adev = dm_state->base.crtc->dev->dev_private; enum amdgpu_rmx_type rmx_type; struct rect src = { 0 }; /* viewport in composition space*/ @@ -726,7 +725,8 @@ static void update_stream_scaling_settings( dst.height -= dm_state->underscan_vborder; } - adev->dm.dc->stream_funcs.stream_update_scaling(adev->dm.dc, stream, &src, &dst); + stream->src = src; + stream->dst = dst; DRM_DEBUG_KMS("Destination Rectangle x:%d y:%d width:%d height:%d\n", dst.x, dst.y, dst.width, dst.height); @@ -1135,9 +1135,6 @@ int amdgpu_dm_connector_atomic_set_property( struct dm_connector_state *dm_new_state = to_dm_connector_state(connector_state); - struct drm_crtc_state *new_crtc_state; - struct drm_crtc *crtc; - int i; int ret = -EINVAL; if (property == dev->mode_config.scaling_mode_property) { @@ -1175,32 +1172,6 @@ int amdgpu_dm_connector_atomic_set_property( ret = 0; } - for_each_crtc_in_state( - connector_state->state, - crtc, - new_crtc_state, - i) { - - if (crtc == connector_state->crtc) { - struct drm_plane_state *plane_state; - - /* - * Bit of magic done here. We need to ensure - * that planes get update after mode is set. - * So, we need to add primary plane to state, - * and this way atomic_update would be called - * for it - */ - plane_state = - drm_atomic_get_plane_state( - connector_state->state, - crtc->primary); - - if (!plane_state) - return -EINVAL; - } - } - return ret; } @@ -2401,28 +2372,19 @@ static void amdgpu_dm_commit_surfaces(struct drm_atomic_state *state, struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(crtc); struct drm_framebuffer *fb = plane_state->fb; struct drm_connector *connector; - struct dm_connector_state *dm_state = NULL; - - enum dm_commit_action action; + struct dm_connector_state *con_state = NULL; bool pflip_needed; if (!fb || !crtc || !crtc->state->active) continue; - action = get_dm_commit_action(crtc->state); - - /* - * TODO - TO decide if it's a flip or surface update - * stop relying on allow_modeset flag and query DC - * using dc_check_update_surfaces_for_stream. - */ pflip_needed = !state->allow_modeset; if (!pflip_needed) { list_for_each_entry(connector, &dev->mode_config.connector_list, head) { if (connector->state->crtc == crtc) { - dm_state = to_dm_connector_state( + con_state = to_dm_connector_state( connector->state); break; } @@ -2442,8 +2404,10 @@ static void amdgpu_dm_commit_surfaces(struct drm_atomic_state *state, * Also it should be needed when used with actual * drm_atomic_commit ioctl in future */ - if (!dm_state) + if (!con_state) continue; + + if (crtc == pcrtc) { add_surface(dm->dc, crtc, plane, &dc_surfaces_constructed[planes_count]); @@ -2495,7 +2459,8 @@ void amdgpu_dm_atomic_commit_tail( const struct dc_stream *new_stream; unsigned long flags; bool wait_for_vblank = true; - + struct drm_connector *connector; + struct drm_connector_state *old_conn_state; drm_atomic_helper_update_legacy_modeset_state(dev, state); @@ -2575,21 +2540,6 @@ void amdgpu_dm_atomic_commit_tail( break; } - - case DM_COMMIT_ACTION_NOTHING: { - struct dm_connector_state *dm_state = NULL; - - if (!aconnector) - break; - - dm_state = to_dm_connector_state(aconnector->base.state); - - /* Scaling update */ - update_stream_scaling_settings(&crtc->state->mode, - dm_state, acrtc->stream); - - break; - } case DM_COMMIT_ACTION_DPMS_OFF: case DM_COMMIT_ACTION_RESET: DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc); @@ -2597,9 +2547,48 @@ void amdgpu_dm_atomic_commit_tail( if (acrtc->stream) remove_stream(adev, acrtc); break; + + /*TODO retire */ + case DM_COMMIT_ACTION_NOTHING: + continue; } /* switch() */ } /* for_each_crtc_in_state() */ + /* Handle scaling and undersacn changes*/ + for_each_connector_in_state(state, connector, old_conn_state, i) { + struct amdgpu_connector *aconnector = to_amdgpu_connector(connector); + struct dm_connector_state *con_new_state = + to_dm_connector_state(aconnector->base.state); + struct dm_connector_state *con_old_state = + to_dm_connector_state(old_conn_state); + struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); + const struct dc_stream_status *status = NULL; + + /* Skip any modesets/resets */ + if (!acrtc || + get_dm_commit_action(acrtc->base.state) != DM_COMMIT_ACTION_NOTHING) + continue; + + /* Skip any thing not scale or underscan chnages */ + if (!is_scaling_state_different(con_new_state, con_old_state)) + continue; + + update_stream_scaling_settings(&con_new_state->base.crtc->mode, + con_new_state, (struct dc_stream *)acrtc->stream); + + status = dc_stream_get_status(acrtc->stream); + WARN_ON(!status); + WARN_ON(!status->surface_count); + + /*TODO How it works with MPO ?*/ + if (!dc_commit_surfaces_to_stream( + dm->dc, + (const struct dc_surface **)status->surfaces, + status->surface_count, + acrtc->stream)) + dm_error("%s: Failed to update stream scaling!\n", __func__); + } + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); @@ -2937,6 +2926,8 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, struct dc *dc = adev->dm.dc; bool need_to_validate = false; struct validate_context *context; + struct drm_connector *connector; + struct drm_connector_state *conn_state; /* * This bool will be set for true for any modeset/reset * or surface update which implies non fast surfae update. @@ -3022,52 +3013,6 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, break; } - case DM_COMMIT_ACTION_NOTHING: { - const struct drm_connector *drm_connector = NULL; - struct drm_connector_state *conn_state = NULL; - struct dm_connector_state *dm_state = NULL; - struct dm_connector_state *old_dm_state = NULL; - struct dc_stream *new_stream; - - if (!aconnector) - break; - - for_each_connector_in_state( - state, drm_connector, conn_state, j) { - if (&aconnector->base == drm_connector) - break; - } - - old_dm_state = to_dm_connector_state(drm_connector->state); - dm_state = to_dm_connector_state(conn_state); - - /* Support underscan adjustment*/ - if (!is_scaling_state_different(dm_state, old_dm_state)) - break; - - new_stream = create_stream_for_sink(aconnector, &crtc_state->mode, dm_state); - - if (!new_stream) { - DRM_ERROR("%s: Failed to create new stream for crtc %d\n", - __func__, acrtc->base.base.id); - break; - } - - new_streams[new_stream_count] = new_stream; - set_count = update_in_val_sets_stream( - set, - crtc_set, - set_count, - acrtc->stream, - new_stream, - crtc); - - new_stream_count++; - need_to_validate = true; - wait_for_prev_commits = true; - - break; - } case DM_COMMIT_ACTION_DPMS_OFF: case DM_COMMIT_ACTION_RESET: /* i.e. reset mode */ @@ -3079,6 +3024,10 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, wait_for_prev_commits = true; } break; + + /*TODO retire */ + case DM_COMMIT_ACTION_NOTHING: + continue; } /* @@ -3095,6 +3044,50 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, ret = -EINVAL; } + /* Check scaling and undersacn changes*/ + for_each_connector_in_state(state, connector, conn_state, i) { + struct amdgpu_connector *aconnector = to_amdgpu_connector(connector); + struct dm_connector_state *con_old_state = + to_dm_connector_state(aconnector->base.state); + struct dm_connector_state *con_new_state = + to_dm_connector_state(conn_state); + struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); + struct dc_stream *new_stream; + + /* Skip any modesets/resets */ + if (!acrtc || + get_dm_commit_action(acrtc->base.state) != DM_COMMIT_ACTION_NOTHING) + continue; + + /* Skip any thing not scale or underscan chnages */ + if (!is_scaling_state_different(con_new_state, con_old_state)) + continue; + + new_stream = create_stream_for_sink( + aconnector, + &acrtc->base.state->mode, + con_new_state); + + if (!new_stream) { + DRM_ERROR("%s: Failed to create new stream for crtc %d\n", + __func__, acrtc->base.base.id); + continue; + } + + new_streams[new_stream_count] = new_stream; + set_count = update_in_val_sets_stream( + set, + crtc_set, + set_count, + acrtc->stream, + new_stream, + &acrtc->base); + + new_stream_count++; + need_to_validate = true; + wait_for_prev_commits = true; + } + for (i = 0; i < set_count; i++) { for_each_plane_in_state(state, plane, plane_state, j) { struct drm_crtc *crtc = plane_state->crtc;