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

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

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

-- 
Jani Nikula, Intel