2022-12-07 01:19:07

by xurui

[permalink] [raw]
Subject: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got an HPD interrupt

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.

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.

Signed-off-by: xurui <[email protected]>
---
.../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


2022-12-07 15:49:39

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got an HPD interrupt

Hi xurui,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.1-rc8 next-20221207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/xurui/drm-amdgpu-Retry-DDC-probing-on-DVI-on-failure-if-we-got-an-HPD-interrupt/20221207-090523
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20221206073156.43453-1-xurui%40kylinos.cn
patch subject: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got an HPD interrupt
config: ia64-randconfig-r031-20221207
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/87950a1d2c0ebf78859951a92148a170ec692222
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review xurui/drm-amdgpu-Retry-DDC-probing-on-DVI-on-failure-if-we-got-an-HPD-interrupt/20221207-090523
git checkout 87950a1d2c0ebf78859951a92148a170ec692222
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/gpu/drm/amd/amdgpu/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c: In function 'amdgpu_connector_dvi_detect':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:1022:47: error: passing argument 1 of 'schedule_delayed_work' from incompatible pointer type [-Werror=incompatible-pointer-types]
1022 | schedule_delayed_work(&adev->hotplug_work,
| ^~~~~~~~~~~~~~~~~~~
| |
| struct work_struct *
In file included from include/linux/rhashtable-types.h:15,
from include/linux/ipc.h:7,
from include/uapi/linux/sem.h:5,
from include/linux/sem.h:5,
from include/linux/sched.h:15,
from include/linux/delay.h:23,
from include/drm/display/drm_dp_helper.h:26,
from drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:27:
include/linux/workqueue.h:667:63: note: expected 'struct delayed_work *' but argument is of type 'struct work_struct *'
667 | static inline bool schedule_delayed_work(struct delayed_work *dwork,
| ~~~~~~~~~~~~~~~~~~~~~^~~~~
cc1: some warnings being treated as errors


vim +/schedule_delayed_work +1022 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c

969
970 /*
971 * DVI is complicated
972 * Do a DDC probe, if DDC probe passes, get the full EDID so
973 * we can do analog/digital monitor detection at this point.
974 * If the monitor is an analog monitor or we got no DDC,
975 * we need to find the DAC encoder object for this connector.
976 * If we got no DDC, we do load detection on the DAC encoder object.
977 * If we got analog DDC or load detection passes on the DAC encoder
978 * we have to check if this analog encoder is shared with anyone else (TV)
979 * if its shared we have to set the other connector to disconnected.
980 */
981 static enum drm_connector_status
982 amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
983 {
984 struct drm_device *dev = connector->dev;
985 struct amdgpu_device *adev = drm_to_adev(dev);
986 struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
987 const struct drm_encoder_helper_funcs *encoder_funcs;
988 int r;
989 enum drm_connector_status ret = connector_status_disconnected;
990 bool dret = false, broken_edid = false;
991
992 if (!drm_kms_helper_is_poll_worker()) {
993 r = pm_runtime_get_sync(connector->dev->dev);
994 if (r < 0) {
995 pm_runtime_put_autosuspend(connector->dev->dev);
996 return connector_status_disconnected;
997 }
998 }
999
1000 if (amdgpu_connector->detected_hpd_without_ddc) {
1001 force = true;
1002 amdgpu_connector->detected_hpd_without_ddc = false;
1003 }
1004
1005 if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
1006 ret = connector->status;
1007 goto exit;
1008 }
1009
1010 if (amdgpu_connector->ddc_bus) {
1011 dret = amdgpu_display_ddc_probe(amdgpu_connector, false);
1012
1013 /* Sometimes the pins required for the DDC probe on DVI
1014 * connectors don't make contact at the same time that the ones
1015 * for HPD do. If the DDC probe fails even though we had an HPD
1016 * signal, try again later
1017 */
1018 if (!dret && !force &&
1019 amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd)) {
1020 DRM_DEBUG_KMS("hpd detected without ddc, retrying in 1 second\n");
1021 amdgpu_connector->detected_hpd_without_ddc = true;
> 1022 schedule_delayed_work(&adev->hotplug_work,
1023 msecs_to_jiffies(1000));
1024 goto exit;
1025 }
1026 }
1027 if (dret) {
1028 amdgpu_connector->detected_by_load = false;
1029 amdgpu_connector_free_edid(connector);
1030 amdgpu_connector_get_edid(connector);
1031
1032 if (!amdgpu_connector->edid) {
1033 DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
1034 connector->name);
1035 ret = connector_status_connected;
1036 broken_edid = true; /* defer use_digital to later */
1037 } else {
1038 amdgpu_connector->use_digital =
1039 !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
1040
1041 /* some oems have boards with separate digital and analog connectors
1042 * with a shared ddc line (often vga + hdmi)
1043 */
1044 if ((!amdgpu_connector->use_digital) && amdgpu_connector->shared_ddc) {
1045 amdgpu_connector_free_edid(connector);
1046 ret = connector_status_disconnected;
1047 } else {
1048 ret = connector_status_connected;
1049 }
1050
1051 /* This gets complicated. We have boards with VGA + HDMI with a
1052 * shared DDC line and we have boards with DVI-D + HDMI with a shared
1053 * DDC line. The latter is more complex because with DVI<->HDMI adapters
1054 * you don't really know what's connected to which port as both are digital.
1055 */
1056 if (amdgpu_connector->shared_ddc && (ret == connector_status_connected)) {
1057 struct drm_connector *list_connector;
1058 struct drm_connector_list_iter iter;
1059 struct amdgpu_connector *list_amdgpu_connector;
1060
1061 drm_connector_list_iter_begin(dev, &iter);
1062 drm_for_each_connector_iter(list_connector,
1063 &iter) {
1064 if (connector == list_connector)
1065 continue;
1066 list_amdgpu_connector = to_amdgpu_connector(list_connector);
1067 if (list_amdgpu_connector->shared_ddc &&
1068 (list_amdgpu_connector->ddc_bus->rec.i2c_id ==
1069 amdgpu_connector->ddc_bus->rec.i2c_id)) {
1070 /* cases where both connectors are digital */
1071 if (list_connector->connector_type != DRM_MODE_CONNECTOR_VGA) {
1072 /* hpd is our only option in this case */
1073 if (!amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd)) {
1074 amdgpu_connector_free_edid(connector);
1075 ret = connector_status_disconnected;
1076 }
1077 }
1078 }
1079 }
1080 drm_connector_list_iter_end(&iter);
1081 }
1082 }
1083 }
1084
1085 if ((ret == connector_status_connected) && (amdgpu_connector->use_digital == true))
1086 goto out;
1087
1088 /* DVI-D and HDMI-A are digital only */
1089 if ((connector->connector_type == DRM_MODE_CONNECTOR_DVID) ||
1090 (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA))
1091 goto out;
1092
1093 /* if we aren't forcing don't do destructive polling */
1094 if (!force) {
1095 /* only return the previous status if we last
1096 * detected a monitor via load.
1097 */
1098 if (amdgpu_connector->detected_by_load)
1099 ret = connector->status;
1100 goto out;
1101 }
1102
1103 /* find analog encoder */
1104 if (amdgpu_connector->dac_load_detect) {
1105 struct drm_encoder *encoder;
1106
1107 drm_connector_for_each_possible_encoder(connector, encoder) {
1108 if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
1109 encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
1110 continue;
1111
1112 encoder_funcs = encoder->helper_private;
1113 if (encoder_funcs->detect) {
1114 if (!broken_edid) {
1115 if (ret != connector_status_connected) {
1116 /* deal with analog monitors without DDC */
1117 ret = encoder_funcs->detect(encoder, connector);
1118 if (ret == connector_status_connected) {
1119 amdgpu_connector->use_digital = false;
1120 }
1121 if (ret != connector_status_disconnected)
1122 amdgpu_connector->detected_by_load = true;
1123 }
1124 } else {
1125 enum drm_connector_status lret;
1126 /* assume digital unless load detected otherwise */
1127 amdgpu_connector->use_digital = true;
1128 lret = encoder_funcs->detect(encoder, connector);
1129 DRM_DEBUG_KMS("load_detect %x returned: %x\n",encoder->encoder_type,lret);
1130 if (lret == connector_status_connected)
1131 amdgpu_connector->use_digital = false;
1132 }
1133 break;
1134 }
1135 }
1136 }
1137
1138 out:
1139 /* updated in get modes as well since we need to know if it's analog or digital */
1140 amdgpu_connector_update_scratch_regs(connector, ret);
1141
1142 exit:
1143 if (!drm_kms_helper_is_poll_worker()) {
1144 pm_runtime_mark_last_busy(connector->dev->dev);
1145 pm_runtime_put_autosuspend(connector->dev->dev);
1146 }
1147
1148 return ret;
1149 }
1150

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (10.73 kB)
config (139.98 kB)
Download all attachments

2022-12-07 16:08:44

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got an HPD interrupt

Hi xurui,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.1-rc8 next-20221207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/xurui/drm-amdgpu-Retry-DDC-probing-on-DVI-on-failure-if-we-got-an-HPD-interrupt/20221207-090523
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20221206073156.43453-1-xurui%40kylinos.cn
patch subject: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got an HPD interrupt
config: riscv-randconfig-r035-20221207
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/87950a1d2c0ebf78859951a92148a170ec692222
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review xurui/drm-amdgpu-Retry-DDC-probing-on-DVI-on-failure-if-we-got-an-HPD-interrupt/20221207-090523
git checkout 87950a1d2c0ebf78859951a92148a170ec692222
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:1022:26: error: incompatible pointer types passing 'struct work_struct *' to parameter of type 'struct delayed_work *' [-Werror,-Wincompatible-pointer-types]
schedule_delayed_work(&adev->hotplug_work,
^~~~~~~~~~~~~~~~~~~
include/linux/workqueue.h:667:63: note: passing argument to parameter 'dwork' here
static inline bool schedule_delayed_work(struct delayed_work *dwork,
^
1 error generated.


vim +1022 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c

969
970 /*
971 * DVI is complicated
972 * Do a DDC probe, if DDC probe passes, get the full EDID so
973 * we can do analog/digital monitor detection at this point.
974 * If the monitor is an analog monitor or we got no DDC,
975 * we need to find the DAC encoder object for this connector.
976 * If we got no DDC, we do load detection on the DAC encoder object.
977 * If we got analog DDC or load detection passes on the DAC encoder
978 * we have to check if this analog encoder is shared with anyone else (TV)
979 * if its shared we have to set the other connector to disconnected.
980 */
981 static enum drm_connector_status
982 amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
983 {
984 struct drm_device *dev = connector->dev;
985 struct amdgpu_device *adev = drm_to_adev(dev);
986 struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
987 const struct drm_encoder_helper_funcs *encoder_funcs;
988 int r;
989 enum drm_connector_status ret = connector_status_disconnected;
990 bool dret = false, broken_edid = false;
991
992 if (!drm_kms_helper_is_poll_worker()) {
993 r = pm_runtime_get_sync(connector->dev->dev);
994 if (r < 0) {
995 pm_runtime_put_autosuspend(connector->dev->dev);
996 return connector_status_disconnected;
997 }
998 }
999
1000 if (amdgpu_connector->detected_hpd_without_ddc) {
1001 force = true;
1002 amdgpu_connector->detected_hpd_without_ddc = false;
1003 }
1004
1005 if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
1006 ret = connector->status;
1007 goto exit;
1008 }
1009
1010 if (amdgpu_connector->ddc_bus) {
1011 dret = amdgpu_display_ddc_probe(amdgpu_connector, false);
1012
1013 /* Sometimes the pins required for the DDC probe on DVI
1014 * connectors don't make contact at the same time that the ones
1015 * for HPD do. If the DDC probe fails even though we had an HPD
1016 * signal, try again later
1017 */
1018 if (!dret && !force &&
1019 amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd)) {
1020 DRM_DEBUG_KMS("hpd detected without ddc, retrying in 1 second\n");
1021 amdgpu_connector->detected_hpd_without_ddc = true;
> 1022 schedule_delayed_work(&adev->hotplug_work,
1023 msecs_to_jiffies(1000));
1024 goto exit;
1025 }
1026 }
1027 if (dret) {
1028 amdgpu_connector->detected_by_load = false;
1029 amdgpu_connector_free_edid(connector);
1030 amdgpu_connector_get_edid(connector);
1031
1032 if (!amdgpu_connector->edid) {
1033 DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
1034 connector->name);
1035 ret = connector_status_connected;
1036 broken_edid = true; /* defer use_digital to later */
1037 } else {
1038 amdgpu_connector->use_digital =
1039 !!(amdgpu_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
1040
1041 /* some oems have boards with separate digital and analog connectors
1042 * with a shared ddc line (often vga + hdmi)
1043 */
1044 if ((!amdgpu_connector->use_digital) && amdgpu_connector->shared_ddc) {
1045 amdgpu_connector_free_edid(connector);
1046 ret = connector_status_disconnected;
1047 } else {
1048 ret = connector_status_connected;
1049 }
1050
1051 /* This gets complicated. We have boards with VGA + HDMI with a
1052 * shared DDC line and we have boards with DVI-D + HDMI with a shared
1053 * DDC line. The latter is more complex because with DVI<->HDMI adapters
1054 * you don't really know what's connected to which port as both are digital.
1055 */
1056 if (amdgpu_connector->shared_ddc && (ret == connector_status_connected)) {
1057 struct drm_connector *list_connector;
1058 struct drm_connector_list_iter iter;
1059 struct amdgpu_connector *list_amdgpu_connector;
1060
1061 drm_connector_list_iter_begin(dev, &iter);
1062 drm_for_each_connector_iter(list_connector,
1063 &iter) {
1064 if (connector == list_connector)
1065 continue;
1066 list_amdgpu_connector = to_amdgpu_connector(list_connector);
1067 if (list_amdgpu_connector->shared_ddc &&
1068 (list_amdgpu_connector->ddc_bus->rec.i2c_id ==
1069 amdgpu_connector->ddc_bus->rec.i2c_id)) {
1070 /* cases where both connectors are digital */
1071 if (list_connector->connector_type != DRM_MODE_CONNECTOR_VGA) {
1072 /* hpd is our only option in this case */
1073 if (!amdgpu_display_hpd_sense(adev, amdgpu_connector->hpd.hpd)) {
1074 amdgpu_connector_free_edid(connector);
1075 ret = connector_status_disconnected;
1076 }
1077 }
1078 }
1079 }
1080 drm_connector_list_iter_end(&iter);
1081 }
1082 }
1083 }
1084
1085 if ((ret == connector_status_connected) && (amdgpu_connector->use_digital == true))
1086 goto out;
1087
1088 /* DVI-D and HDMI-A are digital only */
1089 if ((connector->connector_type == DRM_MODE_CONNECTOR_DVID) ||
1090 (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA))
1091 goto out;
1092
1093 /* if we aren't forcing don't do destructive polling */
1094 if (!force) {
1095 /* only return the previous status if we last
1096 * detected a monitor via load.
1097 */
1098 if (amdgpu_connector->detected_by_load)
1099 ret = connector->status;
1100 goto out;
1101 }
1102
1103 /* find analog encoder */
1104 if (amdgpu_connector->dac_load_detect) {
1105 struct drm_encoder *encoder;
1106
1107 drm_connector_for_each_possible_encoder(connector, encoder) {
1108 if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
1109 encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
1110 continue;
1111
1112 encoder_funcs = encoder->helper_private;
1113 if (encoder_funcs->detect) {
1114 if (!broken_edid) {
1115 if (ret != connector_status_connected) {
1116 /* deal with analog monitors without DDC */
1117 ret = encoder_funcs->detect(encoder, connector);
1118 if (ret == connector_status_connected) {
1119 amdgpu_connector->use_digital = false;
1120 }
1121 if (ret != connector_status_disconnected)
1122 amdgpu_connector->detected_by_load = true;
1123 }
1124 } else {
1125 enum drm_connector_status lret;
1126 /* assume digital unless load detected otherwise */
1127 amdgpu_connector->use_digital = true;
1128 lret = encoder_funcs->detect(encoder, connector);
1129 DRM_DEBUG_KMS("load_detect %x returned: %x\n",encoder->encoder_type,lret);
1130 if (lret == connector_status_connected)
1131 amdgpu_connector->use_digital = false;
1132 }
1133 break;
1134 }
1135 }
1136 }
1137
1138 out:
1139 /* updated in get modes as well since we need to know if it's analog or digital */
1140 amdgpu_connector_update_scratch_regs(connector, ret);
1141
1142 exit:
1143 if (!drm_kms_helper_is_poll_worker()) {
1144 pm_runtime_mark_last_busy(connector->dev->dev);
1145 pm_runtime_put_autosuspend(connector->dev->dev);
1146 }
1147
1148 return ret;
1149 }
1150

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (10.12 kB)
config (215.47 kB)
Download all attachments

2022-12-08 18:05:12

by Alex Deucher

[permalink] [raw]
Subject: Re: [PATCH] drm/amdgpu: Retry DDC probing on DVI on failure if we got an HPD interrupt

On Wed, Dec 7, 2022 at 3:09 AM xurui <[email protected]> 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 <[email protected]>
> ---
> .../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