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.
Let's fix this by improving the detection, with a DDC probe, if the current EDID-based detection failed.
Note that a better way of dealing with this could calling drm_probe_ddc in drm_connector_funcs.detect, and do the EDID full reading and parsing in drm_connector_helper_funcs.get_modes, when it's actually needed.
However, this would be more invasive and thus more error-prone. The current commit is an attempt to get some uninvasive fix, and allow for easier backporting.
Signed-off-by: Ezequiel Garcia ezequiel@vanguardiasur.com.ar --- drivers/gpu/drm/i915/intel_hdmi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index a0d8daed2470..c079206e6681 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1428,6 +1428,20 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) } else status = connector_status_disconnected;
+ /* + * The above call to intel_hdmi_set_edid() checked for a valid EDID. + * However, the EDID can get corrupted for several reasons, resulting + * in a disconnected status despite the connector being connected. + * Hence, let's try one more time, by only probing the DDC. + * + * This allows the DRM core to fallback to builtin or user-provided + * EDID firmware, e.g. in drm_helper_probe_single_connector_modes. + */ + if (status == connector_status_disconnected) + if (drm_probe_ddc(intel_gmbus_get_adapter(dev_priv, + intel_hdmi->ddc_bus))) + status = connector_status_connected; + intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
return status;
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?
Let's fix this by improving the detection, with a DDC probe, if the current EDID-based detection failed.
Note that a better way of dealing with this could calling drm_probe_ddc in drm_connector_funcs.detect, and do the EDID full reading and parsing in drm_connector_helper_funcs.get_modes, when it's actually needed.
However, this would be more invasive and thus more error-prone. The current commit is an attempt to get some uninvasive fix, and allow for easier backporting.
Signed-off-by: Ezequiel Garcia ezequiel@vanguardiasur.com.ar
drivers/gpu/drm/i915/intel_hdmi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index a0d8daed2470..c079206e6681 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1428,6 +1428,20 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) } else status = connector_status_disconnected;
/*
* The above call to intel_hdmi_set_edid() checked for a valid EDID.* However, the EDID can get corrupted for several reasons, resulting* in a disconnected status despite the connector being connected.* Hence, let's try one more time, by only probing the DDC.** This allows the DRM core to fallback to builtin or user-provided* EDID firmware, e.g. in drm_helper_probe_single_connector_modes.*/if (status == connector_status_disconnected)
if (drm_probe_ddc(intel_gmbus_get_adapter(dev_priv,intel_hdmi->ddc_bus)))status = connector_status_connected;intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
return status;
-- 2.7.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
El abr. 1, 2016 11:47 AM, "Ville Syrjälä" ville.syrjala@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?
Let's fix this by improving the detection, with a DDC probe, if the current EDID-based detection failed.
Note that a better way of dealing with this could calling drm_probe_ddc in drm_connector_funcs.detect, and do the EDID full reading and parsing in drm_connector_helper_funcs.get_modes, when it's actually needed.
However, this would be more invasive and thus more error-prone. The current commit is an attempt to get some uninvasive fix, and allow for easier backporting.
Signed-off-by: Ezequiel Garcia ezequiel@vanguardiasur.com.ar
drivers/gpu/drm/i915/intel_hdmi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a0d8daed2470..c079206e6681 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1428,6 +1428,20 @@ intel_hdmi_detect(struct drm_connector
*connector, bool force)
} else status = connector_status_disconnected;
/** The above call to intel_hdmi_set_edid() checked for a valid
EDID.
* However, the EDID can get corrupted for several reasons,
resulting
* in a disconnected status despite the connector being connected.* Hence, let's try one more time, by only probing the DDC.** This allows the DRM core to fallback to builtin or
user-provided
* EDID firmware, e.g. in drm_helper_probe_single_connector_modes.*/if (status == connector_status_disconnected)if (drm_probe_ddc(intel_gmbus_get_adapter(dev_priv,intel_hdmi->ddc_bus)))status = connector_status_connected;intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); return status;-- 2.7.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Ville Syrjälä Intel OTC
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@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.
Let's fix this by improving the detection, with a DDC probe, if the current EDID-based detection failed.
Note that a better way of dealing with this could calling drm_probe_ddc in drm_connector_funcs.detect, and do the EDID full reading and parsing in drm_connector_helper_funcs.get_modes, when it's actually needed.
However, this would be more invasive and thus more error-prone. The current commit is an attempt to get some uninvasive fix, and allow for easier backporting.
Signed-off-by: Ezequiel Garcia ezequiel@vanguardiasur.com.ar
drivers/gpu/drm/i915/intel_hdmi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a0d8daed2470..c079206e6681 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1428,6 +1428,20 @@ intel_hdmi_detect(struct drm_connector
*connector, bool force)
} else status = connector_status_disconnected;
/** The above call to intel_hdmi_set_edid() checked for a validEDID.
* However, the EDID can get corrupted for several reasons,resulting
* in a disconnected status despite the connector being connected.* Hence, let's try one more time, by only probing the DDC.** This allows the DRM core to fallback to builtin oruser-provided
* EDID firmware, e.g. in drm_helper_probe_single_connector_modes.*/if (status == connector_status_disconnected)if (drm_probe_ddc(intel_gmbus_get_adapter(dev_priv,intel_hdmi->ddc_bus)))status = connector_status_connected;intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); return status;-- 2.7.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Ville Syrjälä Intel OTC
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@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?
[1] Documentation/EDID/HOWTO.txt [2] http://www.blaicher.com/2012/06/howto-fixing-a-broken-edid-eeprom-with-a-bus... [3] https://marc.info/?l=linux-kernel&m=112838038415265&w=4
(Adding Jani again, who got dropped for some reason)
On 1 April 2016 at 16:50, Ezequiel Garcia ezequiel@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@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:
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].
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?
[1] Documentation/EDID/HOWTO.txt [2] http://www.blaicher.com/2012/06/howto-fixing-a-broken-edid-eeprom-with-a-bus... [3] https://marc.info/?l=linux-kernel&m=112838038415265&w=4 -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar
On 5 April 2016 at 11:54, Ezequiel Garcia ezequiel@vanguardiasur.com.ar wrote:
(Adding Jani again, who got dropped for some reason)
On 1 April 2016 at 16:50, Ezequiel Garcia ezequiel@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@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:
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].
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... [3] https://marc.info/?l=linux-kernel&m=112838038415265&w=4
Thanks,
dri-devel@lists.freedesktop.org
-
Ezequiel Garcia -
Ville Syrjälä