Yes, I agree. On Tue, Jun 16, 2020 at 3:08 AM Alex Deucher <alexdeucher@xxxxxxxxx> wrote: > > On Mon, Jun 15, 2020 at 5:30 AM Aaron Chou <zhoubb.aaron@xxxxxxxxx> wrote: > > > > About one month ago, I have asked a question about HDMI hotplug, the link is: > > > > https://gitlab.freedesktop.org/drm/amd/-/issues/1135#note_492460 > > > > And I have put one patch to fix this, as follows: > > > > 39 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > 40 index f355d9a752d2..ee657db9a228 100644 > > 41 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > 42 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > 43 @@ -973,7 +973,7 @@ amdgpu_connector_dvi_detect(struct > > drm_connector *connector, bool force) > > 44 const struct drm_encoder_helper_funcs *encoder_funcs; > > 45 int r; > > 46 enum drm_connector_status ret = > > connector_status_disconnected; > > 47 - bool dret = false, broken_edid = false; > > 48 + bool dret = false, broken_edid = false, undefined_flag = > > false; > > 49 > > 50 if (!drm_kms_helper_is_poll_worker()) { > > 51 r = pm_runtime_get_sync(connector->dev->dev); > > 52 @@ -988,7 +988,12 @@ amdgpu_connector_dvi_detect(struct > > drm_connector *connector, bool force) > > 53 > > 54 if (amdgpu_connector->ddc_bus) > > 55 dret = amdgpu_display_ddc_probe(amdgpu_connector, > > false); > > 56 - if (dret) { > > 57 + > > 58 + /* Check the HDMI HPD pin status again */ > > 59 + if (!amdgpu_display_hpd_sense(adev, > > amdgpu_connector->hpd.hpd)) > > 60 + undefined_flag = true; > > 61 + > > 62 + if (dret && !undefined_flag) { > > 63 amdgpu_connector->detected_by_load = false; > > 64 amdgpu_connector_free_edid(connector); > > 65 amdgpu_connector_get_edid(connector); > > > > Maybe the fix is sloppy, so I write the another patch: > > > > 16 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > 17 index c770d73352a7..bb59ebc9a6c8 100644 > > 18 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > 19 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > > 20 @@ -991,6 +991,7 @@ amdgpu_connector_dvi_detect(struct > > drm_connector *connector, bool force) > > 21 if (amdgpu_connector->ddc_bus) > > 22 dret = amdgpu_display_ddc_probe(amdgpu_connector, > > false); > > 23 if (dret) { > > 24 + schedule_work(&adev->hotplug_work); > > 25 amdgpu_connector->detected_by_load = false; > > 26 amdgpu_connector_free_edid(connector); > > 27 amdgpu_connector_get_edid(connector); > > > > Which is better, or neither？ > > As per the last discussion: > > "This is the part I don't understand. The logic already checks the HPD > status in amdgpu_connector_check_hpd_status_unchanged(). Does it > still report connected at that point? After that it tries to read the > EDID in amdgpu_display_ddc_probe(). If the monitor is disconnected, > there should be no EDID so dret should be false. We should try and > figure out why the first HPD check reports connected and the EDID > probe returns true." > > There is already an HPD probe in the current logic. We should try and > understand why we need a second one rather than just adding one. Does > a delay at the top of that function help? > > Alex > > > > > -- > > Regards, > > Aaron > > _______________________________________________ > > amd-gfx mailing list > > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
