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

["Naladala, Ramanaidu" <Ramanaidu.naladala@intel.com>]

Hi Jani,

Thanks for the review.

On 8/31/2026 7:32 PM, Jani Nikula wrote:
> 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.
Sure. I will fold this patches into single patch.
>
>> 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.

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