[PATCH] drm/amd/display: Don't fail atomic check in MST S3 topology change

[Lyude Paul]

JFYI: In the future patches like this should also be sent to 
dri-devel at lists.freedesktop.org

Anyway, sorry but NAK

It's usually OK to set crtc_state->mode_changed = true, but it's never okay to
set it to false from atomic check code. This is because atomic helpers will
sometimes force modesets in situations where they're required but might not
otherwise happen (in fact-this is something we'll be doing in the MST helpers
once we've implemented fallback link retraining support), so if you unset
mode_changed you're breaking part of the atomic state and those helpers.

Additionally, I'm not sure what problem this is supposed to fix? We already
have fixes for the suspend/resume issues on amdgpu, and I made amdgpu not fail
the suspend/resume process if the atomic check fails since that's the behavior
other drivers follow as well. We do eventually want to come up with a solution
for atomic checks failing mid suspend/resume, but since I don't know of any
cases this has ever broken userspace it's not really a priority right now.
Fixing this is a lot more difficult then it seems: we can't change the state
that userspace has set, but we also can't just "turn off" atomic checks per-
say for states that aren't physically possible anymore after resume due to
connector changes and that sort of thing.

On Thu, 2019-01-10 at 16:17 -0500, Jerry (Fangzhi) Zuo wrote:
> In MST S3 topology change, dc_sink is created in .get_mode hook
> later than the resume sequence. No stream created in atomic check
> is normal in MST S3 resume. In such case, mark false to
> mode_changed flag to skip the check in
> dm_crtc_helper_atomic_check(), instead of returning error.
> 
> Signed-off-by: Jerry (Fangzhi) Zuo <Jerry.Zuo at amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 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 a9ed225d2ae0..6e03b2cf0adb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5628,10 +5628,9 @@ static int dm_update_crtc_state(struct
> amdgpu_display_manager *dm,
>  		 */
>  
>  		if (!new_stream) {
> -			DRM_DEBUG_DRIVER("%s: Failed to create new stream for
> crtc %d\n",
> -					__func__, acrtc->base.base.id);
> -			ret = -ENOMEM;
> -			goto fail;
> +			new_crtc_state->mode_changed = false;
> +			DRM_DEBUG_DRIVER("Mode change not expected on %s\n",
> aconnector->base.name);
> +			goto next_crtc;
>  		}
>  
>  		dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
-- 
Cheers,
	Lyude Paul