diff mbox series

[v11,50/74] drm/bridge: adv7511: Advertise HDMI 1.2 capabilities

Message ID 20260901-dw-hdmi-qp-scramb-v11-50-bc12954a0688@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea Sept. 1, 2026, 6:51 p.m. UTC 
Bridges using DRM_BRIDGE_OP_HDMI are now expected to also provide a
supported HDMI version.  This is not yet enforced, but will become
mandatory once the migration to the new caps-based HDMI connector
initialization API is completed.

Since max_mode_clock_khz does not exceed 165 MHz for any of the chip
variants handled by the driver, advertise HDMI 1.2 as the supported
version and set the bridge's max_tmds_char_rate so the connector limit
inferred from supported_hdmi_ver is overridden by the chip-specific
hardware constraints.

drm_bridge_connector_atomic_check() runs
drm_atomic_helper_connector_hdmi_check(), which rejects modes whose
computed TMDS character rate exceeds the connector limit before invoking
the driver's .tmds_char_rate_valid() hook, making
adv7511_bridge_hdmi_tmds_char_rate_valid() redundant. Drop it.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 1, 2026, 8:17 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing IRQ disablement and workqueue cancellation in driver removal and probe error paths.
--

commit e3d22519d26e84f17c45f704d4c2a1fc871e1036
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/bridge: adv7511: Advertise HDMI 1.2 capabilities

Bridges using DRM_BRIDGE_OP_HDMI are now expected to provide a supported
HDMI version. This commit advertises HDMI 1.2 as the supported version
and sets the bridge's max_tmds_char_rate. It also drops the redundant
adv7511_bridge_hdmi_tmds_char_rate_valid() hook.

> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 038b852405b54..c3ef44074477b 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c

[ ... ]

> @@ -1359,6 +1341,8 @@ static int adv7511_probe(struct i2c_client *i2c)
>  
>  	adv7511->bridge.vendor = "Analog";
>  	adv7511->bridge.product = adv7511->info->name;
> +	adv7511->bridge.supported_hdmi_ver = HDMI_VERSION_1_2;
> +	adv7511->bridge.max_tmds_char_rate = 1000ULL * adv7511->info->max_mode_clock_khz;
>  
>  #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
use-after-free in the error handling and driver removal paths of this driver.

In adv7511_probe(), the driver registers a shared interrupt via
devm_request_threaded_irq() and initializes adv7511->hpd_work.
However, in the err_unregister_audio probe error path, the devm-managed
IRQ can still fire because the resources are manually freed before the
devm cleanup runs:

adv7511_probe() {
    ...
err_unregister_audio:
	drm_bridge_remove(&adv7511->bridge);
	i2c_unregister_device(adv7511->i2c_cec);
	clk_disable_unprepare(adv7511->cec_clk);
err_i2c_unregister_packet:
	i2c_unregister_device(adv7511->i2c_packet);
    ...
}

Similarly, adv7511_remove() manually unregisters I2C devices while the
devm-managed IRQ remains active until devres cleanup:

adv7511_remove() {
    ...
	i2c_unregister_device(adv7511->i2c_cec);
	clk_disable_unprepare(adv7511->cec_clk);

	i2c_unregister_device(adv7511->i2c_packet);
	i2c_unregister_device(adv7511->i2c_edid);
    ...
}

Could a hotplug or CEC interrupt firing during or just after these
cleanup paths attempt to access the freed I2C clients?

Furthermore, since cancel_work_sync() is never called for hpd_work, could
any scheduled work execute after the adv7511 structure has been freed by
devres, leading to memory corruption or a kernel panic?
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 038b852405b5..c3ef44074477 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -8,6 +8,7 @@ 
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
+#include <linux/hdmi.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/slab.h>
@@ -775,11 +776,6 @@  static int adv7511_connector_init(struct adv7511 *adv)
  * DRM Bridge Operations
  */
 
-static const struct adv7511 *bridge_to_adv7511_const(const struct drm_bridge *bridge)
-{
-	return container_of(bridge, struct adv7511, bridge);
-}
-
 static void adv7511_bridge_atomic_enable(struct drm_bridge *bridge,
 					 struct drm_atomic_commit *state)
 {
@@ -817,19 +813,6 @@  static void adv7511_bridge_atomic_disable(struct drm_bridge *bridge,
 	adv7511_power_off(adv);
 }
 
-static enum drm_mode_status
-adv7511_bridge_hdmi_tmds_char_rate_valid(const struct drm_bridge *bridge,
-					 const struct drm_display_mode *mode,
-					 unsigned long long tmds_rate)
-{
-	const struct adv7511 *adv = bridge_to_adv7511_const(bridge);
-
-	if (tmds_rate > 1000ULL * adv->info->max_mode_clock_khz)
-		return MODE_CLOCK_HIGH;
-
-	return MODE_OK;
-}
-
 static enum drm_mode_status adv7511_bridge_mode_valid(struct drm_bridge *bridge,
 						      const struct drm_display_info *info,
 		const struct drm_display_mode *mode)
@@ -1006,7 +989,6 @@  static const struct drm_bridge_funcs adv7511_bridge_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_create_state = drm_atomic_helper_bridge_create_state,
 
-	.hdmi_tmds_char_rate_valid = adv7511_bridge_hdmi_tmds_char_rate_valid,
 	.hdmi_clear_audio_infoframe = adv7511_bridge_hdmi_clear_audio_infoframe,
 	.hdmi_write_audio_infoframe = adv7511_bridge_hdmi_write_audio_infoframe,
 	.hdmi_clear_avi_infoframe = adv7511_bridge_hdmi_clear_avi_infoframe,
@@ -1359,6 +1341,8 @@  static int adv7511_probe(struct i2c_client *i2c)
 
 	adv7511->bridge.vendor = "Analog";
 	adv7511->bridge.product = adv7511->info->name;
+	adv7511->bridge.supported_hdmi_ver = HDMI_VERSION_1_2;
+	adv7511->bridge.max_tmds_char_rate = 1000ULL * adv7511->info->max_mode_clock_khz;
 
 #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
 	adv7511->bridge.ops |= DRM_BRIDGE_OP_HDMI_AUDIO;