On Mon, 9 Dec 2024 at 11:25, Johan Hovold johan@kernel.org wrote:
Dmitry,
Looks like you just silently ignored my reviewed feedback, yet included my conditional reviewed-by tag. Repeating below.
Excuse me. I'll expand the commit message.
On Sun, Dec 08, 2024 at 07:29:11PM +0200, 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
Please also include the follow-on resume error. I'm seeing:
[dpu error]connector not connected 3 [drm:drm_mode_config_helper_resume [drm_kms_helper]] *ERROR* Failed to resume (-22)
and say something about that this can prevent *displays* from being enabled on resume in *some* setups (preferably with an explanation why if you have one).
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")
I noticed that the implementation had this status check also before 71174f362d67 ("drm/msm/dpu: move writeback's atomic_check to dpu_writeback.c").
Why did this not cause any trouble back then? Or is this not the right Fixes tag?
If I remember correctly, the encoder's atomic_check() is called only if the corresponding connector is a part of the new state, if there is a connected CRTC, etc, while the connector's atomic_check() is called both for old and new connectors.
Cc: stable@vger.kernel.org Reported-by: Leonard Lausen leonard@lausen.nl Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/57
Please include mine an György's reports here too.
Since this has dragged on for many months now, more people have run into this issue and have reported this to you. Giving them credit for this is the least you can do especially since you failed to include the corresponding details about how this manifests itself to users in the commit message:
Reported-by: György Kurucz me@kuruczgy.com Link: https://lore.kernel.org/all/b70a4d1d-f98f-4169-942c-cb9006a42b40@kuruczgy.co...
Reported-by: Johan Hovold johan+linaro@kernel.org Link: https://lore.kernel.org/all/ZzyYI8KkWK36FfXf@hovoldconsulting.com/
Tested-by: Leonard Lausen leonard@lausen.nl # on sc7180 lazor Tested-by: György Kurucz me@kuruczgy.com Reviewed-by: Johan Hovold johan+linaro@kernel.org Tested-by: Johan Hovold johan+linaro@kernel.org Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
Leonard Lausen reported an issue with suspend/resume of the sc7180 devices. Fix the WB atomic check, which caused the issue.
Changes in v3:
- Rebased on top of msm-fixes
- Link to v2: https://lore.kernel.org/r/20240802-dpu-fix-wb-v2-0-7eac9eb8e895@linaro.org
Johan