* [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
@ 2017-12-31 22:34 ` Stefan Brüns
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Brüns @ 2017-12-31 22:34 UTC (permalink / raw)
To: dri-devel
Cc: Stefan Brüns, Jani Nikula, intel-gfx, Rodrigo Vivi,
linux-kernel, David Airlie, Joonas Lahtinen
The ACK/NACK implementation as found in e.g. the G965 has the falling
clock edge and the release of the data line after the ACK for the received
byte happen at the same time.
This is conformant with the I2C specification, which allows a zero hold
time, see footnote [3]: "A device must internally provide a hold time of
at least 300 ns for the SDA signal (with respect to the V IH(min) of the
SCL signal) to bridge the undefined region of the falling edge of SCL."
Some HDMI-to-VGA converters apparently fail to adhere to this requirement
and latch SDA at the falling clock edge, so instead of an ACK
sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the
transfer.
The bitbanging releases the data line for the ACK only 1/4 bit time after
the falling clock edge, so a slave will see the correct value no matter
if it samples at the rising or the falling clock edge or in the center.
Fallback to bitbanging is already done for the CRT connector.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92685
Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
Changes in v2:
- Fix/enhance commit message, no code changes
drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4dea833f9d1b..847cda4c017c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1573,12 +1573,20 @@ intel_hdmi_set_edid(struct drm_connector *connector)
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
struct edid *edid;
bool connected = false;
+ struct i2c_adapter *i2c;
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
- edid = drm_get_edid(connector,
- intel_gmbus_get_adapter(dev_priv,
- intel_hdmi->ddc_bus));
+ i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
+
+ edid = drm_get_edid(connector, i2c);
+
+ if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
+ DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
+ intel_gmbus_force_bit(i2c, true);
+ edid = drm_get_edid(connector, i2c);
+ intel_gmbus_force_bit(i2c, false);
+ }
intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
--
2.15.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Try EDID bitbanging on HDMI after failed read (rev2)
2017-12-31 22:34 ` Stefan Brüns
(?)
@ 2018-01-02 10:28 ` Patchwork
-1 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-01-02 10:28 UTC (permalink / raw)
To: Stefan Brüns; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Try EDID bitbanging on HDMI after failed read (rev2)
URL : https://patchwork.freedesktop.org/series/35770/
State : success
== Summary ==
Series 35770v2 drm/i915: Try EDID bitbanging on HDMI after failed read
https://patchwork.freedesktop.org/api/1.0/series/35770/revisions/2/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-fail -> DMESG-WARN (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
fail -> PASS (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> PASS (fi-kbl-r) fdo#104172 +1
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:436s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:440s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:383s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:494s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:276s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:494s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:493s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:482s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:471s
fi-elk-e7500 total:224 pass:163 dwarn:15 dfail:0 fail:0 skip:45
fi-gdg-551 total:288 pass:180 dwarn:0 dfail:0 fail:0 skip:108 time:264s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:528s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:403s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:411s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:478s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:432s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:474s
fi-kbl-7560u total:288 pass:268 dwarn:1 dfail:0 fail:0 skip:19 time:521s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:468s
fi-kbl-r total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:525s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:588s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:454s
fi-skl-6600u total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:534s
fi-skl-6700hq total:288 pass:261 dwarn:1 dfail:0 fail:0 skip:26 time:557s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:503s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:505s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:449s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:561s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:417s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:586s
fi-cnl-y total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:628s
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:489s
16432d39f2cbdc7a8798df3ebb4f7c882fb23132 drm-tip: 2017y-12m-30d-02h-25m-26s UTC integration manifest
9e520a26a878 drm/i915: Try EDID bitbanging on HDMI after failed read
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7594/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
2017-12-31 22:34 ` Stefan Brüns
@ 2018-01-02 19:12 ` Rodrigo Vivi
-1 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2018-01-02 19:12 UTC (permalink / raw)
To: Stefan Brüns; +Cc: David Airlie, intel-gfx, linux-kernel, dri-devel
On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
> The ACK/NACK implementation as found in e.g. the G965 has the falling
> clock edge and the release of the data line after the ACK for the received
> byte happen at the same time.
>
> This is conformant with the I2C specification, which allows a zero hold
> time, see footnote [3]: "A device must internally provide a hold time of
> at least 300 ns for the SDA signal (with respect to the V IH(min) of the
> SCL signal) to bridge the undefined region of the falling edge of SCL."
>
> Some HDMI-to-VGA converters apparently fail to adhere to this requirement
> and latch SDA at the falling clock edge, so instead of an ACK
> sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the
> transfer.
>
> The bitbanging releases the data line for the ACK only 1/4 bit time after
> the falling clock edge, so a slave will see the correct value no matter
> if it samples at the rising or the falling clock edge or in the center.
>
> Fallback to bitbanging is already done for the CRT connector.
>
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92685
s/Bug:/Bugzilla:
Did we get the confirmation that this also fix the Skylake issue
initially reported?
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
>
> ---
>
> Changes in v2:
> - Fix/enhance commit message, no code changes
>
> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4dea833f9d1b..847cda4c017c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1573,12 +1573,20 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct edid *edid;
> bool connected = false;
> + struct i2c_adapter *i2c;
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> - edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> +
> + edid = drm_get_edid(connector, i2c);
> +
> + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> + intel_gmbus_force_bit(i2c, true);
> + edid = drm_get_edid(connector, i2c);
> + intel_gmbus_force_bit(i2c, false);
> + }
Approach seems fine for this case.
I just wonder what would be the risks of forcing this bit and edid read when nothing is present on the other end?
>
> intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
@ 2018-01-02 19:12 ` Rodrigo Vivi
0 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2018-01-02 19:12 UTC (permalink / raw)
To: Stefan Brüns
Cc: dri-devel, David Airlie, intel-gfx, Joonas Lahtinen, linux-kernel
On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
> The ACK/NACK implementation as found in e.g. the G965 has the falling
> clock edge and the release of the data line after the ACK for the received
> byte happen at the same time.
>
> This is conformant with the I2C specification, which allows a zero hold
> time, see footnote [3]: "A device must internally provide a hold time of
> at least 300 ns for the SDA signal (with respect to the V IH(min) of the
> SCL signal) to bridge the undefined region of the falling edge of SCL."
>
> Some HDMI-to-VGA converters apparently fail to adhere to this requirement
> and latch SDA at the falling clock edge, so instead of an ACK
> sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the
> transfer.
>
> The bitbanging releases the data line for the ACK only 1/4 bit time after
> the falling clock edge, so a slave will see the correct value no matter
> if it samples at the rising or the falling clock edge or in the center.
>
> Fallback to bitbanging is already done for the CRT connector.
>
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92685
s/Bug:/Bugzilla:
Did we get the confirmation that this also fix the Skylake issue
initially reported?
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
>
> ---
>
> Changes in v2:
> - Fix/enhance commit message, no code changes
>
> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4dea833f9d1b..847cda4c017c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1573,12 +1573,20 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct edid *edid;
> bool connected = false;
> + struct i2c_adapter *i2c;
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> - edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> +
> + edid = drm_get_edid(connector, i2c);
> +
> + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> + intel_gmbus_force_bit(i2c, true);
> + edid = drm_get_edid(connector, i2c);
> + intel_gmbus_force_bit(i2c, false);
> + }
Approach seems fine for this case.
I just wonder what would be the risks of forcing this bit and edid read when nothing is present on the other end?
>
> intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
2018-01-02 19:12 ` Rodrigo Vivi
@ 2018-01-02 19:24 ` Chris Wilson
-1 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2018-01-02 19:24 UTC (permalink / raw)
To: Rodrigo Vivi, Stefan Brüns
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel
Quoting Rodrigo Vivi (2018-01-02 19:12:18)
> On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
> > + edid = drm_get_edid(connector, i2c);
> > +
> > + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> > + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> > + intel_gmbus_force_bit(i2c, true);
> > + edid = drm_get_edid(connector, i2c);
> > + intel_gmbus_force_bit(i2c, false);
> > + }
>
> Approach seems fine for this case.
> I just wonder what would be the risks of forcing this bit and edid read when nothing is present on the other end?
Should be no more risky than using GMBUS as the bit-banging is the
underlying HW protocol; it should just be adding an extra delay to
the disconnected probe. Offset against the chance that it fixes
detection of borderline devices.
I would say that given the explanation above, the question is why not
apply it universally? (Bonus points for including the explanation as
comments.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Intel-gfx] [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
@ 2018-01-02 19:24 ` Chris Wilson
0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2018-01-02 19:24 UTC (permalink / raw)
To: Rodrigo Vivi, Stefan Brüns
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel
Quoting Rodrigo Vivi (2018-01-02 19:12:18)
> On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
> > + edid = drm_get_edid(connector, i2c);
> > +
> > + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> > + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> > + intel_gmbus_force_bit(i2c, true);
> > + edid = drm_get_edid(connector, i2c);
> > + intel_gmbus_force_bit(i2c, false);
> > + }
>
> Approach seems fine for this case.
> I just wonder what would be the risks of forcing this bit and edid read when nothing is present on the other end?
Should be no more risky than using GMBUS as the bit-banging is the
underlying HW protocol; it should just be adding an extra delay to
the disconnected probe. Offset against the chance that it fixes
detection of borderline devices.
I would say that given the explanation above, the question is why not
apply it universally? (Bonus points for including the explanation as
comments.)
-Chris
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
2018-01-02 19:24 ` [Intel-gfx] " Chris Wilson
@ 2018-01-03 7:14 ` Jani Nikula
-1 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2018-01-03 7:14 UTC (permalink / raw)
To: Chris Wilson, Rodrigo Vivi, Stefan Brüns
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel
On Tue, 02 Jan 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Rodrigo Vivi (2018-01-02 19:12:18)
>> On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
>> > + edid = drm_get_edid(connector, i2c);
>> > +
>> > + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
>> > + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
>> > + intel_gmbus_force_bit(i2c, true);
>> > + edid = drm_get_edid(connector, i2c);
>> > + intel_gmbus_force_bit(i2c, false);
>> > + }
>>
>> Approach seems fine for this case.
>> I just wonder what would be the risks of forcing this bit and edid read when nothing is present on the other end?
>
> Should be no more risky than using GMBUS as the bit-banging is the
> underlying HW protocol; it should just be adding an extra delay to
> the disconnected probe. Offset against the chance that it fixes
> detection of borderline devices.
>
> I would say that given the explanation above, the question is why not
> apply it universally? (Bonus points for including the explanation as
> comments.)
I'm wondering, is gmbus too fast for the adapters, does gmbus generally
have different timing for the ack/nak as described in the commit message
than bit banging, or are the adapters just plain buggy? Do we have any
control over gmbus timings (don't have the time to peruse the bpsec just
now)?
BR,
Jani.
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Intel-gfx] [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
@ 2018-01-03 7:14 ` Jani Nikula
0 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2018-01-03 7:14 UTC (permalink / raw)
To: Chris Wilson, Rodrigo Vivi, Stefan Brüns
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel
On Tue, 02 Jan 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Rodrigo Vivi (2018-01-02 19:12:18)
>> On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
>> > + edid = drm_get_edid(connector, i2c);
>> > +
>> > + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
>> > + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
>> > + intel_gmbus_force_bit(i2c, true);
>> > + edid = drm_get_edid(connector, i2c);
>> > + intel_gmbus_force_bit(i2c, false);
>> > + }
>>
>> Approach seems fine for this case.
>> I just wonder what would be the risks of forcing this bit and edid read when nothing is present on the other end?
>
> Should be no more risky than using GMBUS as the bit-banging is the
> underlying HW protocol; it should just be adding an extra delay to
> the disconnected probe. Offset against the chance that it fixes
> detection of borderline devices.
>
> I would say that given the explanation above, the question is why not
> apply it universally? (Bonus points for including the explanation as
> comments.)
I'm wondering, is gmbus too fast for the adapters, does gmbus generally
have different timing for the ack/nak as described in the commit message
than bit banging, or are the adapters just plain buggy? Do we have any
control over gmbus timings (don't have the time to peruse the bpsec just
now)?
BR,
Jani.
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
2018-01-03 7:14 ` [Intel-gfx] " Jani Nikula
@ 2018-01-03 9:23 ` Stefan Brüns
-1 siblings, 0 replies; 13+ messages in thread
From: Stefan Brüns @ 2018-01-03 9:23 UTC (permalink / raw)
To: Jani Nikula
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
[-- Attachment #1.1: Type: text/plain, Size: 2728 bytes --]
On Wednesday, January 3, 2018 8:14:47 AM CET Jani Nikula wrote:
> On Tue, 02 Jan 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Rodrigo Vivi (2018-01-02 19:12:18)
> >
> >> On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
> >> > + edid = drm_get_edid(connector, i2c);
> >> > +
> >> > + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> >> > + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using
> >> > GPIO bit-banging\n"); + intel_gmbus_force_bit(i2c, true);
> >> > + edid = drm_get_edid(connector, i2c);
> >> > + intel_gmbus_force_bit(i2c, false);
> >> > + }
> >>
> >> Approach seems fine for this case.
> >> I just wonder what would be the risks of forcing this bit and edid read
> >> when nothing is present on the other end?>
> > Should be no more risky than using GMBUS as the bit-banging is the
> > underlying HW protocol; it should just be adding an extra delay to
> > the disconnected probe. Offset against the chance that it fixes
> > detection of borderline devices.
> >
> > I would say that given the explanation above, the question is why not
> > apply it universally? (Bonus points for including the explanation as
> > comments.)
>
> I'm wondering, is gmbus too fast for the adapters, does gmbus generally
> have different timing for the ack/nak as described in the commit message
> than bit banging, or are the adapters just plain buggy? Do we have any
> control over gmbus timings (don't have the time to peruse the bpsec just
> now)?
I have seen two different behaviours, one on the ~2009 GM965, the other on the
~2013 Haswell. The Haswell provides a 250..500ns hold time, the other does
not.
There is a flag in the GMBUS0 register, GMBUS_HOLD_EXT, "300ns hold time, rsvd
on Pineview". The driver does not set this flag. Possibly it is always set/
implied on the Haswell (which is post-Pineview), and should be set for
anything older than Pineview.
There is another odd fact with the GM965, according to the register setting it
should run at 100 kBit/s, but it only runs at 30 kBit/s. The Haswell runs at
100 kBit/s, as specified. As there are also idle periods ever 8 bytes, the
EDID read takes 270ms before it fails.
The bitbanging code, running at 45 kBit/s (2 * 20us per clock cycle plus
overhead) on the other hand just needs 58 ms, but keeps one core busy
(udelay).
Unfortunately I currently have no older system than the Haswell available, so
I can not check if the GMBUS_HOLD_EXT flag has any effect.
Kind regards,
Stefan
--
Stefan Brüns / Bergstraße 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Intel-gfx] [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
@ 2018-01-03 9:23 ` Stefan Brüns
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Brüns @ 2018-01-03 9:23 UTC (permalink / raw)
To: Jani Nikula
Cc: Chris Wilson, Rodrigo Vivi, David Airlie, intel-gfx, linux-kernel,
dri-devel
[-- Attachment #1: Type: text/plain, Size: 2728 bytes --]
On Wednesday, January 3, 2018 8:14:47 AM CET Jani Nikula wrote:
> On Tue, 02 Jan 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Rodrigo Vivi (2018-01-02 19:12:18)
> >
> >> On Sun, Dec 31, 2017 at 10:34:54PM +0000, Stefan Brüns wrote:
> >> > + edid = drm_get_edid(connector, i2c);
> >> > +
> >> > + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> >> > + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using
> >> > GPIO bit-banging\n"); + intel_gmbus_force_bit(i2c, true);
> >> > + edid = drm_get_edid(connector, i2c);
> >> > + intel_gmbus_force_bit(i2c, false);
> >> > + }
> >>
> >> Approach seems fine for this case.
> >> I just wonder what would be the risks of forcing this bit and edid read
> >> when nothing is present on the other end?>
> > Should be no more risky than using GMBUS as the bit-banging is the
> > underlying HW protocol; it should just be adding an extra delay to
> > the disconnected probe. Offset against the chance that it fixes
> > detection of borderline devices.
> >
> > I would say that given the explanation above, the question is why not
> > apply it universally? (Bonus points for including the explanation as
> > comments.)
>
> I'm wondering, is gmbus too fast for the adapters, does gmbus generally
> have different timing for the ack/nak as described in the commit message
> than bit banging, or are the adapters just plain buggy? Do we have any
> control over gmbus timings (don't have the time to peruse the bpsec just
> now)?
I have seen two different behaviours, one on the ~2009 GM965, the other on the
~2013 Haswell. The Haswell provides a 250..500ns hold time, the other does
not.
There is a flag in the GMBUS0 register, GMBUS_HOLD_EXT, "300ns hold time, rsvd
on Pineview". The driver does not set this flag. Possibly it is always set/
implied on the Haswell (which is post-Pineview), and should be set for
anything older than Pineview.
There is another odd fact with the GM965, according to the register setting it
should run at 100 kBit/s, but it only runs at 30 kBit/s. The Haswell runs at
100 kBit/s, as specified. As there are also idle periods ever 8 bytes, the
EDID read takes 270ms before it fails.
The bitbanging code, running at 45 kBit/s (2 * 20us per clock cycle plus
overhead) on the other hand just needs 58 ms, but keeps one core busy
(udelay).
Unfortunately I currently have no older system than the Haswell available, so
I can not check if the GMBUS_HOLD_EXT flag has any effect.
Kind regards,
Stefan
--
Stefan Brüns / Bergstraße 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
2017-12-31 22:34 ` Stefan Brüns
@ 2018-01-09 9:06 ` Daniel Vetter
-1 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-01-09 9:06 UTC (permalink / raw)
To: Stefan Brüns
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Sun, Dec 31, 2017 at 11:34:54PM +0100, Stefan Brüns wrote:
> The ACK/NACK implementation as found in e.g. the G965 has the falling
> clock edge and the release of the data line after the ACK for the received
> byte happen at the same time.
>
> This is conformant with the I2C specification, which allows a zero hold
> time, see footnote [3]: "A device must internally provide a hold time of
> at least 300 ns for the SDA signal (with respect to the V IH(min) of the
> SCL signal) to bridge the undefined region of the falling edge of SCL."
>
> Some HDMI-to-VGA converters apparently fail to adhere to this requirement
> and latch SDA at the falling clock edge, so instead of an ACK
> sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the
> transfer.
>
> The bitbanging releases the data line for the ACK only 1/4 bit time after
> the falling clock edge, so a slave will see the correct value no matter
> if it samples at the rising or the falling clock edge or in the center.
>
> Fallback to bitbanging is already done for the CRT connector.
>
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92685
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
>
> ---
>
> Changes in v2:
> - Fix/enhance commit message, no code changes
Ok, found v2, merged this one instead.
-Daniel
>
> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4dea833f9d1b..847cda4c017c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1573,12 +1573,20 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct edid *edid;
> bool connected = false;
> + struct i2c_adapter *i2c;
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> - edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> +
> + edid = drm_get_edid(connector, i2c);
> +
> + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> + intel_gmbus_force_bit(i2c, true);
> + edid = drm_get_edid(connector, i2c);
> + intel_gmbus_force_bit(i2c, false);
> + }
>
> intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Try EDID bitbanging on HDMI after failed read
@ 2018-01-09 9:06 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-01-09 9:06 UTC (permalink / raw)
To: Stefan Brüns
Cc: dri-devel, David Airlie, intel-gfx, Joonas Lahtinen, linux-kernel,
Rodrigo Vivi
On Sun, Dec 31, 2017 at 11:34:54PM +0100, Stefan Brüns wrote:
> The ACK/NACK implementation as found in e.g. the G965 has the falling
> clock edge and the release of the data line after the ACK for the received
> byte happen at the same time.
>
> This is conformant with the I2C specification, which allows a zero hold
> time, see footnote [3]: "A device must internally provide a hold time of
> at least 300 ns for the SDA signal (with respect to the V IH(min) of the
> SCL signal) to bridge the undefined region of the falling edge of SCL."
>
> Some HDMI-to-VGA converters apparently fail to adhere to this requirement
> and latch SDA at the falling clock edge, so instead of an ACK
> sometimes a NACK is read and the slave (i.e. the EDID ROM) ends the
> transfer.
>
> The bitbanging releases the data line for the ACK only 1/4 bit time after
> the falling clock edge, so a slave will see the correct value no matter
> if it samples at the rising or the falling clock edge or in the center.
>
> Fallback to bitbanging is already done for the CRT connector.
>
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92685
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
>
> ---
>
> Changes in v2:
> - Fix/enhance commit message, no code changes
Ok, found v2, merged this one instead.
-Daniel
>
> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4dea833f9d1b..847cda4c017c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1573,12 +1573,20 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct edid *edid;
> bool connected = false;
> + struct i2c_adapter *i2c;
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>
> - edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
> +
> + edid = drm_get_edid(connector, i2c);
> +
> + if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
> + DRM_DEBUG_KMS("HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
> + intel_gmbus_force_bit(i2c, true);
> + edid = drm_get_edid(connector, i2c);
> + intel_gmbus_force_bit(i2c, false);
> + }
>
> intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 13+ messages in thread