On Thu, 06 Jun 2019, Daniel Vetter <daniel@xxxxxxxx> wrote: > 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> Like I explained in my reply, this essentially makes override/firmware EDID a connector force for the case where hotplug detect isn't used or reliable. That's a regression for another set of people... BR, Jani. > > >> --- >> 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 -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
