On Sat, 15 Jun 2019, <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > The patch below does not apply to the 5.1-stable tree. > If someone wants it applied there, or to any other stable or longterm > tree, then please email the backport, including the original git commit > id to <stable@xxxxxxxxxxxxxxx>. > > thanks, > > greg k-h > > ------------------ original commit in Linus's tree ------------------ > > From 48eaeb7664c76139438724d520a1ea4a84a3ed92 Mon Sep 17 00:00:00 2001 > From: Jani Nikula <jani.nikula@xxxxxxxxx> > Date: Mon, 10 Jun 2019 12:30:54 +0300 > Subject: [PATCH] drm: add fallback override/firmware EDID modes workaround > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > We've moved the override and firmware EDID (simply "override EDID" from > now on) handling to the low level drm_do_get_edid() function in order to > transparently use the override throughout the stack. The idea is that > you get the override EDID via the ->get_modes() hook. > > Unfortunately, there are scenarios where the DDC probe in drm_get_edid() > called via ->get_modes() fails, although the preceding ->detect() > succeeds. > > In the case reported by Paul Wise, the ->detect() hook, > intel_crt_detect(), relies on hotplug detect, bypassing the DDC. In the > case reported by Ilpo Järvinen, there is no ->detect() hook, which is > interpreted as connected. The subsequent DDC probe reached via > ->get_modes() fails, and we don't even look at the override EDID, > resulting in no modes being added. > > Because drm_get_edid() is used via ->detect() all over the place, we > can't trivially remove the DDC probe, as it leads to override EDID > effectively meaning connector forcing. The goal is that connector > forcing and override EDID remain orthogonal. > > Generally, the underlying problem here is the conflation of ->detect() > and ->get_modes() via drm_get_edid(). The former should just detect, and > the latter should just get the modes, typically via reading the EDID. As > long as drm_get_edid() is used in ->detect(), it needs to retain the DDC > probe. Or such users need to have a separate DDC probe step first. > > The EDID caching between ->detect() and ->get_modes() done by some > drivers is a further complication that prevents us from making > drm_do_get_edid() adapt to the two cases. > > Work around the regression by falling back to a separate attempt at > getting the override EDID at drm_helper_probe_single_connector_modes() > level. With a working DDC and override EDID, it'll never be called; the > override EDID will come via ->get_modes(). There will still be a failing > DDC probe attempt in the cases that require the fallback. > > v2: > - Call drm_connector_update_edid_property (Paul) > - Update commit message about EDID caching (Daniel) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107583 > Reported-by: Paul Wise <pabs3@xxxxxxxxxxxxx> > Cc: Paul Wise <pabs3@xxxxxxxxxxxxx> > References: http://mid.mail-archive.com/alpine.DEB.2.20.1905262211270.24390@xxxxxxxxxxxxxxxxxxxxx > Reported-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxx> > Cc: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxx> > Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > References: 15f080f08d48 ("drm/edid: respect connector force for drm_get_edid ddc probe") > Fixes: 53fd40a90f3c ("drm: handle override and firmware EDID at drm_do_get_edid() level") > Cc: <stable@xxxxxxxxxxxxxxx> # v4.15+ 56a2b7f2a39a drm/edid: abstract override/firmware EDID retrieval I was hoping this dependency description would suffice; does it not work if you pick that up first? Ditto for the other stable kernels. BR, Jani. > Cc: <stable@xxxxxxxxxxxxxxx> # v4.15+ > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Harish Chegondi <harish.chegondi@xxxxxxxxx> > Tested-by: Paul Wise <pabs3@xxxxxxxxxxxxx> > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > Link: https://patchwork.freedesktop.org/patch/msgid/20190610093054.28445-1-jani.nikula@xxxxxxxxx > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index c1952b6e5747..e804ac5dec02 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1584,6 +1584,36 @@ static struct edid *drm_get_override_edid(struct drm_connector *connector) > return IS_ERR(override) ? NULL : override; > } > > +/** > + * drm_add_override_edid_modes - add modes from override/firmware EDID > + * @connector: connector we're probing > + * > + * Add modes from the override/firmware EDID, if available. Only to be used from > + * drm_helper_probe_single_connector_modes() as a fallback for when DDC probe > + * failed during drm_get_edid() and caused the override/firmware EDID to be > + * skipped. > + * > + * Return: The number of modes added or 0 if we couldn't find any. > + */ > +int drm_add_override_edid_modes(struct drm_connector *connector) > +{ > + struct edid *override; > + int num_modes = 0; > + > + override = drm_get_override_edid(connector); > + if (override) { > + drm_connector_update_edid_property(connector, override); > + num_modes = drm_add_edid_modes(connector, override); > + kfree(override); > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] adding %d modes via fallback override/firmware EDID\n", > + connector->base.id, connector->name, num_modes); > + } > + > + return num_modes; > +} > +EXPORT_SYMBOL(drm_add_override_edid_modes); > + > /** > * drm_do_get_edid - get EDID data using a custom EDID block read function > * @connector: connector we're probing > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 6fd08e04b323..dd427c7ff967 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -479,6 +479,13 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, > > count = (*connector_funcs->get_modes)(connector); > > + /* > + * Fallback for when DDC probe failed in drm_get_edid() and thus skipped > + * override/firmware EDID. > + */ > + if (count == 0 && connector->status == connector_status_connected) > + count = drm_add_override_edid_modes(connector); > + > if (count == 0 && connector->status == connector_status_connected) > count = drm_add_modes_noedid(connector, 1024, 768); > count += drm_helper_probe_add_cmdline_mode(connector); > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > index 9d3b5b93102c..c9ca0be54d9a 100644 > --- a/include/drm/drm_edid.h > +++ b/include/drm/drm_edid.h > @@ -471,6 +471,7 @@ struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, > struct i2c_adapter *adapter); > struct edid *drm_edid_duplicate(const struct edid *edid); > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid); > +int drm_add_override_edid_modes(struct drm_connector *connector); > > u8 drm_match_cea_mode(const struct drm_display_mode *to_match); > enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code); > -- Jani Nikula, Intel Open Source Graphics Center
