On 5 April 2016 at 11:54, Ezequiel Garcia <ezequiel@xxxxxxxxxxxxxxxxxxxx> wrote: > (Adding Jani again, who got dropped for some reason) > > On 1 April 2016 at 16:50, Ezequiel Garcia <ezequiel@xxxxxxxxxxxxxxxxxxxx> wrote: >> On 01 Apr 06:46 PM, Ville Syrjälä wrote: >>> On Fri, Apr 01, 2016 at 12:38:11PM -0300, Ezequiel Garcia wrote: >>> > El abr. 1, 2016 11:47 AM, "Ville Syrjälä" <ville.syrjala@xxxxxxxxxxxxxxx> >>> > escribió: >>> > > >>> > > On Thu, Mar 31, 2016 at 05:55:03PM -0300, Ezequiel Garcia wrote: >>> > > > Currently, our implementation of drm_connector_funcs.detect is >>> > > > based on getting a valid EDID. >>> > > > >>> > > > This requirement makes the driver fail to detect connected >>> > > > connectors in case of EDID corruption, which prevents from falling >>> > > > back to modes provided by builtin or user-provided EDIDs. >>> > > >>> > > So why are you getting corrupted EDIDs? >>> > > >>> > >>> > Does it matter? >>> >>> Yes. We should fix the real cause (if possible) instead of adding >>> more duct tape. >>> >> >> So, there are two things involved in this patch: >> >> 1. >> There are several reasons why EDID can get screwed, this is >> documented at length [1], and it's the motivation for >> CONFIG_DRM_LOAD_EDID_FIRMWARE to exist. >> >> You can find lots of reports on the internet of people getting >> corrupt EDID from their monitors. For instance, here's one [2]. >> >> And even if no firmware is provided using CONFIG_DRM_LOAD_EDID_FIRMWARE, >> the DRM core will provide a 1024x768 fallback mode: >> >> int drm_helper_probe_single_connector_modes(struct drm_connector *connector, >> uint32_t maxX, uint32_t maxY) >> { >> [..] >> if (count == 0 && connector->status == connector_status_connected) >> count = drm_add_modes_noedid(connector, 1024, 768); >> >> But, this only works if the connector is detected. >> >> Since I'm interested in backporting this patch to apply it on the kernels >> I maintain (which are currently deployed on hundreds of machines), I tried >> to find a simple solution. Hence, this patch. >> >> There's no issue to fix here, because broken hardware is a fact of life, >> and not something we can fix or ignore [3]. >> >> 2. >> On the other side, the i915 implementation looks suspicious. IMHO, >> drm_connector_funcs.detect should not try to read a valid EDID, >> and just try to detect if the connector is connected or disconnected. >> >> The EDID can be read in drm_connector_helper_funcs.get_modes, as other >> drm/connector drivers are doing (tda998x, tfp410, tegra). >> >> However, I think it's safer to get a simple fix now, and do this >> as follow-up patches. >> >> How does it sound? >> Are there any other comments regarding this patch? If at all possible, I'd like to see this merged, or otherwise a proposal for an alternative solution. >> [1] Documentation/EDID/HOWTO.txt >> [2] http://www.blaicher.com/2012/06/howto-fixing-a-broken-edid-eeprom-with-a-bus-pirate-v4/ >> [3] https://marc.info/?l=linux-kernel&m=112838038415265&w=4 Thanks, -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel