Re: [PATCH v1 1/2] drm/i915/hdmi: Add debugfs support to force EDID reads over GPIO

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

On Wed, 26 Aug 2026, Naladala Ramanaidu <ramanaidu.naladala@intel.com> wrote:
> Add an i915_hdmi_force_bit_banging debugfs control for HDMI
> connectors to allow EDID reads to be forced over GPIO bit-banging.

The primary question the commit message *must* answer is *why*.

It takes me under a minute to look at the patch and deduce the *what*,
and even that does not match the commit message. There is no support for
actually forcing anything here, it's just the non-functional debugfs
being added.

I still have no clue why.

>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  |  1 +
>  .../drm/i915/display/intel_display_types.h    |  3 ++
>  drivers/gpu/drm/i915/display/intel_hdmi.c     | 51 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_hdmi.h     |  2 +
>  4 files changed, 57 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..c991f224b4be 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1335,6 +1335,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  	intel_dp_link_training_debugfs_add(connector);
>  	intel_dp_link_caps_debugfs_add(connector);
>  	intel_link_bw_connector_debugfs_add(connector);
> +	intel_hdmi_connector_debugfs_add(connector);
>  
>  	if (DISPLAY_VER(display) >= 11 &&
>  	    ((connector_type == DRM_MODE_CONNECTOR_DisplayPort && !connector->mst.dp) ||
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 20a07ea06b5e..58983e31c5ef 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1689,6 +1689,9 @@ struct intel_hdmi {
>  	} dp_dual_mode;
>  	struct intel_connector *attached_connector;
>  	struct cec_notifier *cec_notifier;
> +
> +	/* Debugfs knob to force EDID reads over GPIO bit-banging. */

Is that comment helpful?

> +	bool force_bit_banging;
>  };
>  
>  struct intel_dp_mst_encoder;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 9b637e38a1a5..38915f19d3e5 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -26,6 +26,7 @@
>   *	Jesse Barnes <jesse.barnes@intel.com>
>   */
>  
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/hdmi.h>
>  #include <linux/i2c.h>
> @@ -3112,6 +3113,56 @@ void intel_infoframe_init(struct intel_digital_port *dig_port)
>  	}
>  }
>  
> +static int i915_hdmi_force_bit_banging_show(void *data, u64 *val)

Please don't use i915_ naming.

> +{
> +	struct intel_connector *connector = to_intel_connector(data);
> +
> +	*val = READ_ONCE(intel_attached_hdmi(connector)->force_bit_banging);
> +
> +	return 0;
> +}
> +
> +static int i915_hdmi_force_bit_banging_write(void *data, u64 val)
> +{
> +	struct intel_connector *connector = to_intel_connector(data);
> +	struct intel_display *display = to_intel_display(connector);
> +	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> +
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	drm_dbg_kms(display->drm,
> +		    "[CONNECTOR:%d:%s] %sabling forced GPIO bit-banging for EDID reads\n",
> +		    connector->base.base.id, connector->base.name,
> +		    val ? "en" : "dis");

I see this cute "%sabling" and val ? "en" : "dis" but it's just too
clever for its own good. str_enable_disable() is close enough.

> +
> +	WRITE_ONCE(intel_hdmi->force_bit_banging, val);
> +
> +	return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(i915_hdmi_force_bit_banging_fops,
> +			 i915_hdmi_force_bit_banging_show,
> +			 i915_hdmi_force_bit_banging_write, "%llu\n");
> +
> +/**
> + * intel_hdmi_connector_debugfs_add - add HDMI specific connector debugfs files
> + * @connector: pointer to a registered intel_connector
> + *
> + * Cleanup will be done by drm_connector_unregister() through a call to
> + * drm_debugfs_connector_remove().
> + */
> +void intel_hdmi_connector_debugfs_add(struct intel_connector *connector)
> +{
> +	struct dentry *root = connector->base.debugfs_entry;
> +
> +	if (connector->base.connector_type != DRM_MODE_CONNECTOR_HDMIA &&
> +	    connector->base.connector_type != DRM_MODE_CONNECTOR_HDMIB)
> +		return;
> +
> +	debugfs_create_file("i915_hdmi_force_bit_banging", 0644, root,
> +			    connector, &i915_hdmi_force_bit_banging_fops);

Please no new i915_ prefixed naming for shared display code. Please just
use intel_.

> +}
> +
>  bool intel_hdmi_init_connector(struct intel_digital_port *dig_port,
>  			       struct intel_connector *intel_connector)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
> index c95ed37bcc0c..752cccf5ccee 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> @@ -78,4 +78,6 @@ void intel_hdmi_poll_for_scrambling_enable(const struct intel_crtc_state *crtc_s
>  int intel_hdmi_sink_max_frl_rate(struct drm_connector *connector);
>  int intel_hdmi_sink_dsc_max_frl_rate(struct drm_connector *connector);
>  
> +void intel_hdmi_connector_debugfs_add(struct intel_connector *connector);
> +
>  #endif /* __INTEL_HDMI_H__ */

-- 
Jani Nikula, Intel