On 12/18/2018 10:26 AM, [email protected] wrote: > From: Leo Li <[email protected]> > > drm_atomic_helper_check_planes() calls the crtc atomic check helpers. In > an attempt to better align with the DRM framework, we can move the > entire dm_update dance to the crtc check helper (since it essentially > checks that we can align DC states to what DRM is requesting)
What if DRM submits a request for plane update without any CRTC in the new atomic state - like some type of plane update request e.g. disable plane) - Can this approach handle this scenario ? Andrey > Signed-off-by: Leo Li <[email protected]> > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 145 > ++++++++++++---------- > 1 file changed, 80 insertions(+), 65 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 a629544..b4dd65b 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3859,15 +3859,88 @@ static bool dm_lock_and_validation_needed(struct > drm_atomic_state *state) > } > > static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, > - struct drm_crtc_state *state) > + struct drm_crtc_state *new_crtc_state) > { > struct amdgpu_device *adev = crtc->dev->dev_private; > struct dc *dc = adev->dm.dc; > - struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state); > + struct drm_atomic_state *state = new_crtc_state->state; > + struct drm_plane *plane; > + struct drm_plane_state *old_plane_state, *new_plane_state; > + struct drm_crtc_state *old_crtc_state = > drm_atomic_get_old_crtc_state(state, crtc); > + struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(new_crtc_state); > int ret = -EINVAL; > + int i; > + > + /* > + * Add affected connectors and planes if a connector or plane change > + * affects the DC stream. > + * > + * The additional include of affected connectors shouldn't be required > + * outside of what is already done in drm_atomic_helper_check_modeset(). > + * We currently do this because dm_update_crtcs_state() requires the new > + * affected connector state in order to construct a new, updated stream. > + * See create_stream_for_sink(). > + */ > + if (new_crtc_state->enable && > + (new_crtc_state->color_mgmt_changed || > + new_crtc_state->vrr_enabled)) { > + ret = drm_atomic_add_affected_connectors(state, crtc); > + if (ret) > + return ret; > + > + ret = drm_atomic_add_affected_planes(state, crtc); > + if (ret) > + return ret; > + } > + > + /* > + * Remove exiting planes for this CRTC, if they are modified or removed > + */ > + for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, > new_plane_state, i) { > + if (!(new_crtc_state->plane_mask & drm_plane_mask(plane)) && > + !(old_crtc_state->plane_mask & drm_plane_mask(plane))) > + continue; > + > + ret = dm_update_planes_state(dc, state, plane, > + old_plane_state, > + new_plane_state, > + false); > + if (ret) > + return ret; > + } > + > + /* Disable this crtc, if required*/ > + ret = dm_update_crtcs_state(&adev->dm, state, crtc, > + old_crtc_state, > + new_crtc_state, > + false); > + if (ret) > + return ret; > + > + /* Enable this crtc, if required*/ > + ret = dm_update_crtcs_state(&adev->dm, state, crtc, > + old_crtc_state, > + new_crtc_state, > + true); > + if (ret) > + return ret; > + > + /* Add new or add back modified planes */ > + for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, > new_plane_state, i) { > + if (!(new_crtc_state->plane_mask & drm_plane_mask(plane)) && > + !(old_crtc_state->plane_mask & drm_plane_mask(plane))) > + continue; > + > + ret = dm_update_planes_state(dc, state, plane, > + old_plane_state, > + new_plane_state, > + true); > + if (ret) > + return ret; > + } > > if (unlikely(!dm_crtc_state->stream && > - modeset_required(state, NULL, dm_crtc_state->stream))) { > + modeset_required(new_crtc_state, NULL, > dm_crtc_state->stream))) { > WARN_ON(1); > return ret; > } > @@ -4077,6 +4150,9 @@ static int dm_plane_atomic_check(struct drm_plane > *plane, > struct dc *dc = adev->dm.dc; > struct dm_plane_state *dm_plane_state = to_dm_plane_state(state); > > + if (drm_atomic_plane_disabling(plane->state, state)) > + return 0; > + > if (!dm_plane_state->dc_state) > return 0; > > @@ -5911,10 +5987,6 @@ static int amdgpu_dm_atomic_check(struct drm_device > *dev, > struct dc *dc = adev->dm.dc; > struct drm_connector *connector; > struct drm_connector_state *old_con_state, *new_con_state; > - struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state, *new_crtc_state; > - struct drm_plane *plane; > - struct drm_plane_state *old_plane_state, *new_plane_state; > enum surface_update_type update_type = UPDATE_TYPE_FAST; > enum surface_update_type overall_update_type = UPDATE_TYPE_FAST; > > @@ -5926,68 +5998,11 @@ static int amdgpu_dm_atomic_check(struct drm_device > *dev, > */ > bool lock_and_validation_needed = false; > > + > ret = drm_atomic_helper_check_modeset(dev, state); > if (ret) > goto fail; > > - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > - if (!drm_atomic_crtc_needs_modeset(new_crtc_state) && > - !new_crtc_state->color_mgmt_changed && > - !new_crtc_state->vrr_enabled) > - continue; > - > - if (!new_crtc_state->enable) > - continue; > - > - ret = drm_atomic_add_affected_connectors(state, crtc); > - if (ret) > - return ret; > - > - ret = drm_atomic_add_affected_planes(state, crtc); > - 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_planes_state(dc, state, plane, > - old_plane_state, > - new_plane_state, > - false); > - if (ret) > - goto fail; > - } > - > - /* Disable all crtcs which require disable */ > - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > - ret = dm_update_crtcs_state(&adev->dm, state, crtc, > - old_crtc_state, > - new_crtc_state, > - false); > - if (ret) > - goto fail; > - } > - > - /* Enable all crtcs which require enable */ > - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > - ret = dm_update_crtcs_state(&adev->dm, state, crtc, > - old_crtc_state, > - new_crtc_state, > - true); > - if (ret) > - goto fail; > - } > - > - /* Add new/modified planes */ > - for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, > new_plane_state, i) { > - ret = dm_update_planes_state(dc, state, plane, > - old_plane_state, > - new_plane_state, > - true); > - if (ret) > - goto fail; > - } > - > /* Run this here since we want to validate the streams we created */ > ret = drm_atomic_helper_check_planes(dev, state); > if (ret) _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
