[RFC] drm: Do not call drm_probe_ddc() when connector force isn't specified

[Jani Nikula]

On Thu, 06 Jun 2019, Harish Chegondi <harish.chegondi at intel.com> 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 at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Harish Chegondi <harish.chegondi at intel.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107583
> ---
>  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;
> -
>  	edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
>  	if (edid)
>  		drm_get_displayid(connector, edid);

We've had the drm_probe_ddc() since 61e57a8d72f2 ("drm/edid: Fix
secondary block fetch.") in 2010. The commit message does not shed any
light on why this was added, though I assume early and fast bail out
when the DDC isn't there.

In any case, this patch solves one problem, but creates another. It
loses the ability to use DDC probe to detect display presence when using
firmware/override EDID, i.e. the firmware/override EDID effectively
leads to connector forcing.

In the referenced bug, the problem is that hotplug detect works, but DDC
does not. Using connector forcing to work around the issue leads to
losing hotplug detect.

IMO the override/firmware EDID should be orthogonal to hotplug detect,
regardless of whether the detect is achieved via source hardware detect
or DDC.

If you need to allow for cases where one or the other hotplug detect
methods is bust, the only viable solution is to add another level of
connector force to bypass drm_probe_ddc() above, yet rely on hardware
hotplug detect. Say, DRM_FORCE_ON_DDC.

It does mean the folks affected by the referenced bug would need to work
around their experienced regression by adding a connector force, *but*
we are talking about folks with broken displays.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center