Hi Jani,
Thanks for the review.
On 8/31/2026 7:32 PM, Jani Nikula wrote:
On Wed, 26 Aug 2026, Naladala Ramanaidu <[email protected]> wrote:
Normally the EDID is read over GMBUS, and GPIO bit-banging is only used
if that read fails. Now the debugfs flag is checked first. If it is set,
the DDC adapter is put in bit-banging mode before the read and put back
after it.
The flag is read once with READ_ONCE(), so the enable and the restore
always match even if the flag changes at the same time. The GMBUS retry
is not needed in this case, because the read already used GPIO.
This should just be folded to one patch that actually does something
with the debugfs.
Sure. I will fold this patches into single patch.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Naladala Ramanaidu <[email protected]>
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 38915f19d3e5..462ccfc840d4 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2555,13 +2555,23 @@ intel_hdmi_set_edid(struct drm_connector *_connector)
struct i2c_adapter *ddc = connector->base.ddc;
struct ref_tracker *wakeref;
const struct drm_edid *drm_edid;
+ bool force_bit_banging = READ_ONCE(intel_hdmi->force_bit_banging);
bool connected = false;
wakeref = intel_display_power_get(display, POWER_DOMAIN_GMBUS);
+ if (force_bit_banging) {
+ drm_dbg_kms(display->drm,
+ "[CONNECTOR:%d:%s] HDMI EDID read forced to GPIO
bit-banging\n",
+ connector->base.base.id, connector->base.name);
+ intel_gmbus_force_bit(ddc, true);
+ }
+
Okay, now that I see the implementation... why is any of this HDMI
specific? You could make it independent of the connector type. There are
other connectors that use intel_gmbus_force_bit().
And you could just call intel_gmbus_force_bit() directly from the
debugfs handler. It already supports nesting with a count. You don't
have to modify any of this code, at all. The debug logging is there in
intel_gmbus_force_bit(). Everything is taken care of.
BR,
Jani.
The original intent was to debug a specific HDMI EDID read failure on
newer platforms, where GMBUS reads were intermittently unreliable. To
enable quick validation of the fix, I added a debugfs knob that forces
GPIO bit-banging for the affected connector. As a result, the
implementation was scoped around the immediate HDMIA/HDMIB use case
rather than developed as a generic DDC/GMBUS debugging aid.
I agree with the suggestion. Since intel_gmbus_force_bit() applies to
any GMBUS-backed DDC adapter, limiting the interface to HDMI connectors
is unnecessarily restrictive and reduces its value for VGA/DVI debugging
without any clear benefit. I'll generalize the implementation in the
next revision.
drm_edid = drm_edid_read_ddc(&connector->base, ddc);
- if (!drm_edid && !intel_gmbus_is_forced_bit(ddc)) {
+ if (force_bit_banging)
+ intel_gmbus_force_bit(ddc, false);
+ else if (!drm_edid && !intel_gmbus_is_forced_bit(ddc)) {
drm_dbg_kms(display->drm,
"HDMI GMBUS EDID read failed, retry using GPIO
bit-banging\n");
intel_gmbus_force_bit(ddc, true);
