diff mbox series

[1/2] drm/bridge: anx7625: Drop EDID cache on bridge power off

Message ID 20240924035231.1163670-2-treapking@chromium.org (mailing list archive)
State New
Headers show
Series Drop EDID cache for it6505/anx7625 when the bridge is powered off | expand

Commit Message

Pin-yen Lin Sept. 24, 2024, 3:51 a.m. UTC
The bridge might miss the display change events when it's powered off.
This happens when a user changes the external monitor when the system
is suspended and the embedded controller doesn't not wake AP up.

It's also observed that one DP-to-HDMI bridge doesn't work correctly
when there is no EDID read after it is powered on.

Drop the cache to force an EDID read after system resume to fix this.

Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP")
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---

 drivers/gpu/drm/bridge/analogix/anx7625.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Dmitry Baryshkov Sept. 24, 2024, 6:53 a.m. UTC | #1
On Tue, Sep 24, 2024 at 11:51:46AM GMT, Pin-yen Lin wrote:
> The bridge might miss the display change events when it's powered off.
> This happens when a user changes the external monitor when the system
> is suspended and the embedded controller doesn't not wake AP up.
> 
> It's also observed that one DP-to-HDMI bridge doesn't work correctly
> when there is no EDID read after it is powered on.
> 
> Drop the cache to force an EDID read after system resume to fix this.
> 
> Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP")
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> ---
> 
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Doug Anderson Sept. 24, 2024, 4:42 p.m. UTC | #2
Hi,

On Mon, Sep 23, 2024 at 8:53 PM Pin-yen Lin <treapking@chromium.org> wrote:
>
> The bridge might miss the display change events when it's powered off.
> This happens when a user changes the external monitor when the system
> is suspended and the embedded controller doesn't not wake AP up.
>
> It's also observed that one DP-to-HDMI bridge doesn't work correctly
> when there is no EDID read after it is powered on.
>
> Drop the cache to force an EDID read after system resume to fix this.
>
> Fixes: 8bdfc5dae4e3 ("drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP")
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> ---
>
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 1 +
>  1 file changed, 1 insertion(+)

I'm not totally sure if it matters, but I wonder if you should change
this to only delete the EDID cache if you're in DP mode and not eDP
mode? For eDP mode the panel is not allowed to change and re-reading
it needlessly seems like it would slow down things like
suspend/resume. I think this would only matter if someone were using
eDP panels in the "old" way (not under the aux-bus) because we don't
set the "DRM_BRIDGE_OP_EDID" when we see "aux-bus", so maybe we don't
care that much but still...

Other than that, I know that there have been discussions in the past
about EDID caches but I can't quite remember all the details. I know
that panel-edp.c still caches it, so we must have concluded that it's
at least fine/reasonable for panels. I don't remember whether caching
is encouraged / suggested for external displays, though. Do you happen
to know if it even makes a difference there (in other words, do you
actually see multiple calls to read the EDID when you plug in a DP
display)?

-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 88e4aa5830f3..b4c1f00f1c3d 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2561,6 +2561,7 @@  static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
 	mutex_lock(&ctx->lock);
 
 	anx7625_stop_dp_work(ctx);
+	anx7625_remove_edid(ctx);
 	anx7625_power_standby(ctx);
 
 	mutex_unlock(&ctx->lock);