Lyude Paul
2018-Aug-23 22:27 UTC
[Nouveau] [PATCH] drm/nouveau: Fix nouveau_connector_ddc_detect()
It looks like that when we moved over to using drm_connector_for_each_possible_encoder() in nouveau, that one rather important part of this function got dropped by accident: /* Right v here */ for (i = 0; nv_encoder = NULL, i < DRM_CONNECTOR_MAX_ENCODER; i++) { int id = connector->encoder_ids[i]; if (id == 0) break; Since it's rather difficult to notice: the conditional in this loop is actually: nv_encoder = NULL, i < DRM_CONNECTOR_MAX_ENCODER Meaning that all early breaks result in nv_encoder keeping it's value, otherwise nv_encoder = NULL. Ugh. Since this got dropped, nouveau_connector_ddc_detect() now returns an encoder for every single connector, regardless of whether or not it's detected: [ 1780.056185] nouveau 0000:01:00.0: DRM: DDC responded, but no EDID for DP-2 So: fix this to ensure we only return an encoder if we actually found one, and clean up the rest of the function while we're at it since it's nearly impossible to read properly. Signed-off-by: Lyude Paul <lyude at redhat.com> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com> Fixes: ddba766dd07e ("drm/nouveau: Use drm_connector_for_each_possible_encoder()") --- drivers/gpu/drm/nouveau/nouveau_connector.c | 51 ++++++++++++--------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 3b288be9c3f7..51b6f762f1e1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -414,9 +414,9 @@ nouveau_connector_ddc_detect(struct drm_connector *connector) struct nouveau_connector *nv_connector = nouveau_connector(connector); struct nouveau_drm *drm = nouveau_drm(dev); struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device); - struct nouveau_encoder *nv_encoder = NULL; + struct nouveau_encoder *nv_encoder = NULL, *found = NULL; struct drm_encoder *encoder; - int i, panel = -ENODEV; + int i, ret, panel = -ENODEV; /* eDP panels need powering on by us (if the VBIOS doesn't default it * to on) before doing any AUX channel transactions. LVDS panel power @@ -433,37 +433,46 @@ nouveau_connector_ddc_detect(struct drm_connector *connector) drm_connector_for_each_possible_encoder(connector, encoder, i) { nv_encoder = nouveau_encoder(encoder); - if (nv_encoder->dcb->type == DCB_OUTPUT_DP) { - int ret = nouveau_dp_detect(nv_encoder); + switch (nv_encoder->dcb->type) { + case DCB_OUTPUT_DP: + ret = nouveau_dp_detect(nv_encoder); if (ret == NOUVEAU_DP_MST) return NULL; - if (ret == NOUVEAU_DP_SST) - break; - } else - if ((vga_switcheroo_handler_flags() & - VGA_SWITCHEROO_CAN_SWITCH_DDC) && - nv_encoder->dcb->type == DCB_OUTPUT_LVDS && - nv_encoder->i2c) { - int ret; + else if (ret == NOUVEAU_DP_SST) + found = nv_encoder; + + break; + case DCB_OUTPUT_LVDS: + /* Can we actually probe this LVDS display? */ + if (!((vga_switcheroo_handler_flags() & + VGA_SWITCHEROO_CAN_SWITCH_DDC) && + nv_encoder->i2c)) + continue; + vga_switcheroo_lock_ddc(dev->pdev); - ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50); - vga_switcheroo_unlock_ddc(dev->pdev); - if (ret) - break; - } else - if (nv_encoder->i2c) { if (nvkm_probe_i2c(nv_encoder->i2c, 0x50)) - break; + found = nv_encoder; + vga_switcheroo_unlock_ddc(dev->pdev); + + break; + default: + if (nv_encoder->i2c && + nvkm_probe_i2c(nv_encoder->i2c, 0x50)) + found = nv_encoder; + + break; } + if (found) + break; } /* eDP panel not detected, restore panel power GPIO to previous * state to avoid confusing the SOR for other output types. */ - if (!nv_encoder && panel == 0) + if (!found && panel == 0) nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel); - return nv_encoder; + return found; } static struct nouveau_encoder * -- 2.17.1
Apparently Analagous Threads
- [PATCH v2] drm/nouveau: Fix nouveau_connector_ddc_detect()
- [PATCH v2] drm/nouveau: Fix nouveau_connector_ddc_detect()
- [PATCH v2 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()
- [PATCH v2 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()
- [PATCH v5 07/12] drm/nouveau: Switch DDC when reading the EDID