On 2017-11-16 10:32 AM, Harry Wentland wrote:
> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>
> Within atomic check, dm_update_crtcs_state is called twice. First to
> remove from the dc_state, and subsequently to add to it.
>
> In both calls, a secondary mode-change check is done using dc-level
> states. We shouldn't be doing this while removing, since a new
> dc_stream_state has not been created to do the necessary comparison.
> Because of this, the mode_changed flag within the DRM state can be
> mistakenly set to false. Doing so only when adding prevents this.
>
> We are also guaranteed that a call to add will come after remove, or
> else the atomic check fails (and a commit will not happen).
>
> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
> Reviewed-by: Tony Cheng <Tony.Cheng at amd.com>
> Acked-by: Harry Wentland <harry.wentland at amd.com>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 d10f8c09469b..05bc98d74a33 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4535,7 +4535,7 @@ static int dm_update_crtcs_state(struct dc *dc,
> }
> }
>
> - if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
> + if (enable && dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
> dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
>
> new_crtc_state->mode_changed = false;
Why just not move this logic into the if (aconnector && enable) {} close
immediately above ? Seems this call has no meaning unless new_stream is
initialized anyway.
Thanks,
Andrey
