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: stable@vger.kernel.org Reported-by: Leonard Lausen leonard@lausen.nl Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/57 Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@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;