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

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 9/28/2026 5:49 PM, Naladala Ramanaidu 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()
>
> v2: Address below review comments: (Jani Nikula)
>      - Remove superfluous newline.
>      - Move debugfs bit-banging state from struct intel_connector to struct
>        intel_gmbus as debugfs_force_bit.
>      - Make intel_gmbus_is_adapter() static.
>      - Fold NULL adapter check into intel_gmbus_is_adapter().
>      - Remove unused intel_gmbus_is_adapter() declaration from header.
>
> Assisted-by: OpenAI Codex:GPT-5
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>   .../drm/i915/display/intel_display_debugfs.c  |  2 +
>   .../drm/i915/display/intel_display_types.h    |  1 -
>   drivers/gpu/drm/i915/display/intel_gmbus.c    | 67 +++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_gmbus.h    |  2 +
>   4 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 3e302f23f247..b8fd65414990 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,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>   	if (!root)
>   		return;
>   
> +	intel_gmbus_connector_debugfs_add(connector);
>   	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..1b8eb08d1096 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -558,7 +558,6 @@ struct intel_connector {
>   	u8 polled;
>   
>   	int force_joined_pipes;
> -


Nitpick : unwanted line removal.


>   	struct {
>   		struct drm_dp_aux *dsc_decompression_aux;
>   		u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 60a70dea5d85..be6d29508bea 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>
> @@ -47,6 +48,7 @@ struct intel_gmbus {
>   	struct i2c_adapter adapter;
>   #define GMBUS_FORCE_BIT_RETRY (1U << 31)
>   	u32 force_bit;
> +	bool debugfs_force_bit;
>   	u32 reg0;
>   	intel_reg_t gpio_reg;
>   	struct i2c_algo_bit_data bit_algo;
> @@ -871,6 +873,71 @@ static const struct i2c_algorithm gmbus_algorithm = {
>   	.functionality	= gmbus_func
>   };
>   
> +static bool intel_gmbus_is_adapter(struct intel_connector *connector)
> +{
> +	struct i2c_adapter *adapter = connector->base.ddc;
> +
> +	return adapter && adapter->algo == &gmbus_algorithm;
> +}
> +
> +static int intel_force_bit_banging_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = m->private;
> +	struct intel_gmbus *bus = container_of(connector->base.ddc,
> +					       struct intel_gmbus, adapter);
> +
> +	seq_printf(m, "%u\n", bus->debugfs_force_bit);
> +
> +	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;
> +	struct intel_gmbus *bus = container_of(connector->base.ddc,
> +					       struct intel_gmbus, adapter);
> +	bool force_bit_banging;
> +	int ret;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &force_bit_banging);
> +	if (ret)
> +		return ret;
> +
> +	if (force_bit_banging != bus->debugfs_force_bit) {
> +		intel_gmbus_force_bit(connector->base.ddc, force_bit_banging);
> +		bus->debugfs_force_bit = force_bit_banging;

I agree with Sashiko's comment here. Theoretically there can be multiple 
users trying to set the debugfs concurrently or along with the test.

for other debugfs tests it might not be a problem, but here problem is 
that the intel_gmbus_force_bit() ref count can get unintentionally 
remain > 0, causing GMBUS to always use bit bashing.

There would be no way to tell from the debugfs value itself that this 
has happened.

I think we can reuse the global mutex similar to how the force_bit is 
using. That will mean to avoid calling intel_gmbus_force_bit() (since it 
already takes mutex) and instead:

-take mutex lock

-check if debugfs_force_bit is changed. Update the force_bit, and the 
debug_force_bit

-unlock mutex

Same thing in show()

-take mutex lock

-save bus->debugfs_force_bit in variable force_bit_banging

-unlock mutex

-print the force_bit_banging.


Regards,

Ankit

> +	}
> +
> +	*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 (intel_gmbus_is_adapter(connector))
> +		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..5c03100ce46b 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,7 @@ int intel_gmbus_output_aksv(struct i2c_adapter *adapter);
>   
>   struct i2c_adapter *
>   intel_gmbus_get_adapter(struct intel_display *display, unsigned int pin);
> +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);