On Thu, 06 Jun 2019, Harish Chegondi <harish.chegondi@xxxxxxxxx> wrote: > This would allow the EDID override to be handled correctly in > drm_do_get_edid() for cases where EDID data is missing or corrupt. > > All drm_probe_ddc() does is call drm_do_probe_ddc_edid( , , , 1) > which probes the display by reading 1 byte of EDID data via I2C. > This patch removes the call to drm_probe_ddc() from drm_get_edid() > but drm_get_edid() calls drm_do_get_edid() which first handles > the EDID override case and then calls > drm_do_probe_ddc_edid( , , ,EDID_LENGTH) via function pointer > argument get_edid_block. So, the display device is still being > probed by reading EDID_LENGTH bytes of EDID data via I2C. > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Signed-off-by: Harish Chegondi <harish.chegondi@xxxxxxxxx> > References: https://bugs.freedesktop.org/show_bug.cgi?id=107583 > --- > drivers/gpu/drm/drm_edid.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index d87f574feeca..41c420706532 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1724,9 +1724,6 @@ struct edid *drm_get_edid(struct drm_connector *connector, > if (connector->force == DRM_FORCE_OFF) > return NULL; > > - if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter)) > - return NULL; > - > edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter); > if (edid) > drm_get_displayid(connector, edid); We've had the drm_probe_ddc() since 61e57a8d72f2 ("drm/edid: Fix secondary block fetch.") in 2010. The commit message does not shed any light on why this was added, though I assume early and fast bail out when the DDC isn't there. In any case, this patch solves one problem, but creates another. It loses the ability to use DDC probe to detect display presence when using firmware/override EDID, i.e. the firmware/override EDID effectively leads to connector forcing. In the referenced bug, the problem is that hotplug detect works, but DDC does not. Using connector forcing to work around the issue leads to losing hotplug detect. IMO the override/firmware EDID should be orthogonal to hotplug detect, regardless of whether the detect is achieved via source hardware detect or DDC. If you need to allow for cases where one or the other hotplug detect methods is bust, the only viable solution is to add another level of connector force to bypass drm_probe_ddc() above, yet rely on hardware hotplug detect. Say, DRM_FORCE_ON_DDC. It does mean the folks affected by the referenced bug would need to work around their experienced regression by adding a connector force, *but* we are talking about folks with broken displays. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel