Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221206073156.43453-1-xurui@kylinos.cn>
In-Reply-To: <20221206073156.43453-1-xurui@kylinos.cn>
From:   Alex Deucher <alexdeucher@gmail.com>
Date:   Thu, 8 Dec 2022 12:50:30 -0500
Message-ID: <CADnq5_ONUkteQrhgMVfst5UCK5+GjVQaj-=-ABo8bYczeGCRLg@mail.gmail.com>
Subject: Re: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got
 an HPD interrupt
To:     xurui <xurui@kylinos.cn>
Cc:     alexander.deucher@amd.com, tzimmermann@suse.de,
        guchun.chen@amd.com, Xinhui.Pan@amd.com,
        linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org,
        sam@ravnborg.org, aurabindo.pillai@amd.com, cssk@net-c.es,
        dri-devel@lists.freedesktop.org, christian.koenig@amd.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Dec 7, 2022 at 3:09 AM xurui <xurui@kylinos.cn> wrote:
>
> HPD signals on DVI ports can be fired off before the pins required for
> DDC probing actually make contact, due to the pins for HPD making
> contact first. This results in a HPD signal being asserted but DDC
> probing failing, resulting in hotplugging occasionally failing.

It seems like DP should get a similar fix.

>
> Rescheduling the hotplug work for a second when we run into an HPD
> signal with a failing DDC probe usually gives enough time for the rest
> of the connector's pins to make contact, and fixes this issue.

This looks reasonable.  Please address the kernel test robot reports.

Thanks,

Alex

>
> Signed-off-by: xurui <xurui@kylinos.cn>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 22 ++++++++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h      |  1 +
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index cfb262911bfc..dd8d414249a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -997,13 +997,33 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
>                 }
>         }
>
> +       if (amdgpu_connector->detected_hpd_without_ddc) {
> +               force = true;
> +               amdgpu_connector->detected_hpd_without_ddc = false;
> +       }
> +
>         if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
>                 ret = connector->status;
>                 goto exit;
>         }
>
> -       if (amdgpu_connector->ddc_bus)
> +       if (amdgpu_connector->ddc_bus) {
>                 dret = amdgpu_display_ddc_probe(amdgpu_connector, false);
> +
> +               /* Sometimes the pins required for the DDC probe on DVI
> +                * connectors don't make contact at the same time that the ones
> +                * for HPD do. If the DDC probe fails even though we had an HPD
> +                * signal, try again later
> +                */
> +               if (!dret && !force &&
> +                   amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd)) {
> +                       DRM_DEBUG_KMS("hpd detected without ddc, retrying in 1 second\n");
> +                       amdgpu_connector->detected_hpd_without_ddc = true;
> +                       schedule_delayed_work(&adev->hotplug_work,
> +                                             msecs_to_jiffies(1000));
> +                       goto exit;
> +               }
> +       }
>         if (dret) {
>                 amdgpu_connector->detected_by_load = false;
>                 amdgpu_connector_free_edid(connector);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 37322550d750..bf009de59710 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -535,6 +535,7 @@ struct amdgpu_connector {
>         void *con_priv;
>         bool dac_load_detect;
>         bool detected_by_load; /* if the connection status was determined by load */
> +       bool detected_hpd_without_ddc; /* if an HPD signal was detected on DVI, but ddc probing failed */
>         uint16_t connector_object_id;
>         struct amdgpu_hpd hpd;
>         struct amdgpu_router router;
> --
> 2.25.1
>
>
> No virus found
>                 Checked by Hillstone Network AntiVirus