Re: [PATCH v2 1/1] drm/i915/gmbus: Add bit-banging debugfs control

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

Hi Jani,

Thanks for the review comments. Sure i will fix them in the next revision.

On 9/25/2026 4:00 PM, Jani Nikula wrote:
> On Thu, 24 Sep 2026, Naladala Ramanaidu <ramanaidu.naladala@intel.com> wrote:
>> Add a connector debugfs interface to enable or disable I2C
>> bit-banging for Intel GMBUS adapters.
>>
>> v1: Address below review comments: (Jani Nikula)
>>      - Remove HDMI-specific EDID read path changes.
>>      - Control bit-banging directly from debugfs via intel_gmbus_force_bit()
>>
>> Assisted-by: OpenAI Codex:GPT-5
>> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
>> ---
>>   .../drm/i915/display/intel_display_debugfs.c  |  3 +
>>   .../drm/i915/display/intel_display_types.h    |  1 +
>>   drivers/gpu/drm/i915/display/intel_gmbus.c    | 61 +++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_gmbus.h    |  3 +
>>   4 files changed, 68 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index 3e302f23f247..74f065175a72 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -40,6 +40,7 @@
>>   #include "intel_fb.h"
>>   #include "intel_fbc.h"
>>   #include "intel_fbdev.h"
>> +#include "intel_gmbus.h"
>>   #include "intel_hdcp.h"
>>   #include "intel_hdmi.h"
>>   #include "intel_hotplug.h"
>> @@ -1327,6 +1328,8 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>>   	if (!root)
>>   		return;
>>   
>> +	intel_gmbus_connector_debugfs_add(connector);
>> +
> Superfluous newline. All the other calls below are back to back.
>
>>   	intel_drrs_connector_debugfs_add(connector);
>>   	intel_hdcp_connector_debugfs_add(connector);
>>   	intel_pps_connector_debugfs_add(connector);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 79f30660c2b6..5cef2582c361 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -558,6 +558,7 @@ struct intel_connector {
>>   	u8 polled;
>>   
>>   	int force_joined_pipes;
>> +	bool force_bit_banging;
> This should be hidden in struct intel_gmbus next to force_bit, maybe as
> debugfs_force_bit or something.
>
>>   
>>   	struct {
>>   		struct drm_dp_aux *dsc_decompression_aux;
>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
>> index 60a70dea5d85..38d6d1148399 100644
>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
>> @@ -27,6 +27,7 @@
>>    *	Chris Wilson <chris@chris-wilson.co.uk>
>>    */
>>   
>> +#include <linux/debugfs.h>
>>   #include <linux/export.h>
>>   #include <linux/i2c-algo-bit.h>
>>   #include <linux/i2c.h>
>> @@ -871,6 +872,66 @@ static const struct i2c_algorithm gmbus_algorithm = {
>>   	.functionality	= gmbus_func
>>   };
>>   
>> +bool intel_gmbus_is_adapter(struct i2c_adapter *adapter)
> This should be static.
>
>> +{
>> +	return adapter->algo == &gmbus_algorithm;
>> +}
>> +
>> +static int intel_force_bit_banging_show(struct seq_file *m, void *data)
>> +{
>> +	struct intel_connector *connector = m->private;
>> +
>> +	seq_printf(m, "%u\n", connector->force_bit_banging);
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t intel_force_bit_banging_write(struct file *file,
>> +						     const char __user *ubuf,
>> +						     size_t len, loff_t *offp)
>> +{
>> +	struct seq_file *m = file->private_data;
>> +	struct intel_connector *connector = m->private;
>> +	bool force_bit_banging;
>> +	int ret;
>> +
>> +	ret = kstrtobool_from_user(ubuf, len, &force_bit_banging);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (force_bit_banging != connector->force_bit_banging) {
>> +		intel_gmbus_force_bit(connector->base.ddc, force_bit_banging);
>> +		connector->force_bit_banging = force_bit_banging;
>> +	}
>> +
>> +	*offp += len;
>> +
>> +	return len;
>> +}
>> +
>> +static int intel_force_bit_banging_open(struct inode *inode, struct file *file)
>> +{
>> +	return single_open(file, intel_force_bit_banging_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations intel_force_bit_banging_fops = {
>> +	.owner = THIS_MODULE,
>> +	.open = intel_force_bit_banging_open,
>> +	.read = seq_read,
>> +	.llseek = seq_lseek,
>> +	.release = single_release,
>> +	.write = intel_force_bit_banging_write,
>> +};
>> +
>> +void intel_gmbus_connector_debugfs_add(struct intel_connector *connector)
>> +{
>> +	if (connector->base.ddc &&
>> +	    intel_gmbus_is_adapter(connector->base.ddc))
> The connector->base.ddc != NULL check should be inside
> intel_gmbus_is_adapter().
>
>> +		debugfs_create_file("intel_force_bit_banging", 0644,
>> +				    connector->base.debugfs_entry, connector,
>> +				    &intel_force_bit_banging_fops);
>> +}
>> +
>>   static void gmbus_lock_bus(struct i2c_adapter *adapter,
>>   			   unsigned int flags)
>>   {
>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.h b/drivers/gpu/drm/i915/display/intel_gmbus.h
>> index 5fdeab1aa794..968f8cb42821 100644
>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.h
>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.h
>> @@ -9,6 +9,7 @@
>>   #include <linux/types.h>
>>   
>>   struct i2c_adapter;
>> +struct intel_connector;
>>   struct intel_display;
>>   
>>   #define GMBUS_PIN_DISABLED	0
>> @@ -41,6 +42,8 @@ int intel_gmbus_output_aksv(struct i2c_adapter *adapter);
>>   
>>   struct i2c_adapter *
>>   intel_gmbus_get_adapter(struct intel_display *display, unsigned int pin);
>> +bool intel_gmbus_is_adapter(struct i2c_adapter *adapter);
>> +void intel_gmbus_connector_debugfs_add(struct intel_connector *connector);
>>   void intel_gmbus_force_bit(struct i2c_adapter *adapter, bool force_bit);
>>   bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter);
>>   void intel_gmbus_reset(struct intel_display *display);