Re: [PATCH v1 2/2] drm/i915/hdmi: Read EDID over GPIO when the debugfs flag is set

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 26 Aug 2026, Naladala Ramanaidu <ramanaidu.naladala@intel.com> 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.

>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>  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.


>  	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);

-- 
Jani Nikula, Intel