On Thu, Jun 6, 2019 at 9:38 AM 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 Since it's a regression we need to annotate this correctly, for the next version please include: Fixes: 53fd40a90f3c ("drm: handle override and firmware EDID at drm_do_get_edid() level") Cc: <stable@xxxxxxxxxxxxxxx> # v4.15+ So there's a pile more drm_probe_ddc calls all around in drivers, but I reviewed them all, and they're all in ->detect callbacks. So not affecting the regression we're discussing here. Looking at drm_do_get_edid this should also not result in more failures. The only thing this changes is that drm_do_get_edid will retry a bunch more times if nothing is connected (4 times, instead of just the one probe that drm_probe_ddc does). I guess we can restore that if anyone cares, should at least mention it in the commit message. Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > 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; Trouble is there's a lot more drm_probe_ddc calls all over, and a lot of these > - > edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter); > if (edid) > drm_get_displayid(connector, edid); > -- > 2.21.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
