On Thu, Jun 06, 2019 at 02:56:53PM +0300, Jani Nikula wrote: > 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. Hi Jani, Can you please give more details on which regression this patch may cause. Any specific test setup and IGT test would be helpful. I will re-work my patch to make sure it doesn't cause any regression. The CI BAT report didn't indicate any regressions for this patch. Thank You Harish. > > > > > > > >> --- > >> 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
