The inherent defect of the AMDGPU driver about hotplug

Ernst Sjöstrand ernstp at gmail.com
Tue Jun 16 07:09:08 UTC 2020


Have you tried a much later kernel btw? I saw you mentioned 4.19.

Den tis 16 juni 2020 kl 02:50 skrev Aaron Chou <zhoubb.aaron at gmail.com>:

> Yes, I agree.
>
> On Tue, Jun 16, 2020 at 3:08 AM Alex Deucher <alexdeucher at gmail.com>
> wrote:
> >
> > On Mon, Jun 15, 2020 at 5:30 AM Aaron Chou <zhoubb.aaron at gmail.com>
> 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 at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200616/46e66fe7/attachment-0001.htm>


More information about the amd-gfx mailing list