Dear Dmitry,
Thank you for the patch. Unfortunately, the patch triggers a regression with
respect to DRM CRTC state handling. With the patch applied, suspending and
resuming a lazor sc7180 with external display connected, looses CRTC state on
resume and prevents applying a new CRTC state. Without the patch, CRTC state is
preserved across suspend and resume and it remains possible to change CRTC
settings after resume. This means the patch regresses the user experience,
preventing "Night Light" mode to work as expected. I've validated this on
v6.10.2 vs. v6.10.2 with this patch applied.
While the cause for the bug uncovered by this change is likely separate, given
it's impact, would it be prudent to delay the application of this patch until
the related bug is identified and fixed? Otherwise we would be fixing a dmesg
error message "[dpu error]connector not connected 3" that appears to do no harm
but thereby break more critical user visible behavior.
Best regards
Leonard
On 8/2/24 15:47, Dmitry Baryshkov wrote:
> During suspend/resume process all connectors are explicitly disabled and
> then reenabled. However resume fails because of the connector_status check:
>
> [ 1185.831970] [dpu error]connector not connected 3
>
> It doesn't make sense to check for the Writeback connected status (and
> other drivers don't perform such check), so drop the check.
>
> Fixes: 71174f362d67 ("drm/msm/dpu: move writeback's atomic_check to
> dpu_writeback.c")
> Cc: sta...@vger.kernel.org
> Reported-by: Leonard Lausen <leon...@lausen.nl>
> Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/57
> Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> index 16f144cbc0c9..8ff496082902 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
> @@ -42,9 +42,6 @@ static int dpu_wb_conn_atomic_check(struct drm_connector
> *connector,
> if (!conn_state || !conn_state->connector) {
> DPU_ERROR("invalid connector state\n");
> return -EINVAL;
> - } else if (conn_state->connector->status != connector_status_connected)
> {
> - DPU_ERROR("connector not connected %d\n",
> conn_state->connector->status);
> - return -EINVAL;
> }
>
> crtc = conn_state->crtc;
>
