[Intel-gfx] [PATCH] drm/i915/hdmi: Fix weak connector detection

[Ezequiel Garcia]

On 5 April 2016 at 11:54, Ezequiel Garcia <ezequiel at vanguardiasur.com.ar> wrote:
> (Adding Jani again, who got dropped for some reason)
>
> On 1 April 2016 at 16:50, Ezequiel Garcia <ezequiel at vanguardiasur.com.ar> 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 at linux.intel.com>
>>> > 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