Re: [PATCH v10 45/69] drm/vc4: hdmi: Convert to common HDMI 2.0 scrambling infrastructure

On Fri, Jul 31, 2026 at 07:19:52PM +0300, Cristian Ciocaltea wrote:
> Replace driver local scrambling logic with the newly introduced common
> HDMI 2.0 scrambling support:
> 
> - Advertise source-side scrambling capability by setting supported HDMI
>   version based on the variant's max_pixel_clock.
> 
> - Provide minimal .scrambler_{enable,disable} connector callbacks that
>   only toggle the VC5 HDMI_SCRAMBLER_CTL register, while delegating
>   sink-side SCDC programming and periodic status monitoring to
>   drm_connector_hdmi_{enable,disable}_scrambling().
> 
> - Drop vc4_hdmi_reset_link() and vc4_hdmi_handle_hotplug(), switching
>   the .detect_ctx() path to drm_atomic_helper_connector_hdmi_hotplug()
>   which internally calls drm_connector_hdmi_sync_scdc() to trigger a
>   CRTC reset on reconnection.
> 
> - Drop the local scrambling_work delayed workqueue and scdc_enabled
>   flag, now tracked by the common drm_connector_hdmi layer, as well as
>   the now unused output_{bpc,color_format} driver data members.
> 
> No functional change intended.
> 
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 230 +++++------------------------------------
>  drivers/gpu/drm/vc4/vc4_hdmi.h |  21 ----
>  2 files changed, 28 insertions(+), 223 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index d599957fee3a..ec0c1f4224d7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -35,7 +35,6 @@
>  #include <drm/display/drm_hdmi_cec_helper.h>
>  #include <drm/display/drm_hdmi_helper.h>
>  #include <drm/display/drm_hdmi_state_helper.h>
> -#include <drm/display/drm_scdc_helper.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_edid.h>
> @@ -114,22 +113,6 @@
>  #define HSM_MIN_CLOCK_FREQ	120000000
>  #define CEC_CLOCK_FREQ 40000
>  
> -static bool vc4_hdmi_supports_scrambling(struct vc4_hdmi *vc4_hdmi)
> -{
> -	struct drm_display_info *display = &vc4_hdmi->connector.display_info;
> -
> -	lockdep_assert_held(&vc4_hdmi->mutex);
> -
> -	if (!display->is_hdmi)
> -		return false;
> -
> -	if (!display->hdmi.scdc.supported ||
> -	    !display->hdmi.scdc.scrambling.supported)
> -		return false;
> -
> -	return true;
> -}
> -
>  static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
>  {
>  	struct drm_debugfs_entry *entry = m->private;
> @@ -263,115 +246,6 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
>  static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
>  #endif
>  
> -static int vc4_hdmi_reset_link(struct drm_connector *connector,
> -			       struct drm_modeset_acquire_ctx *ctx)
> -{
> -	struct drm_device *drm;
> -	struct vc4_hdmi *vc4_hdmi;
> -	struct drm_connector_state *conn_state;
> -	struct drm_crtc_state *crtc_state;
> -	struct drm_crtc *crtc;
> -	bool scrambling_needed;
> -	u8 config;
> -	int ret;
> -
> -	if (!connector)
> -		return 0;
> -
> -	drm = connector->dev;
> -	ret = drm_modeset_lock(&drm->mode_config.connection_mutex, ctx);
> -	if (ret)
> -		return ret;
> -
> -	conn_state = connector->state;
> -	crtc = conn_state->crtc;
> -	if (!crtc)
> -		return 0;
> -
> -	ret = drm_modeset_lock(&crtc->mutex, ctx);
> -	if (ret)
> -		return ret;
> -
> -	crtc_state = crtc->state;
> -	if (!crtc_state->active)
> -		return 0;
> -
> -	vc4_hdmi = connector_to_vc4_hdmi(connector);
> -	mutex_lock(&vc4_hdmi->mutex);
> -
> -	if (!vc4_hdmi_supports_scrambling(vc4_hdmi)) {
> -		mutex_unlock(&vc4_hdmi->mutex);
> -		return 0;
> -	}
> -
> -	scrambling_needed = drm_hdmi_mode_needs_scrambling(&vc4_hdmi->saved_adjusted_mode,
> -							   vc4_hdmi->output_bpc,
> -							   vc4_hdmi->output_format);
> -	if (!scrambling_needed) {
> -		mutex_unlock(&vc4_hdmi->mutex);
> -		return 0;
> -	}
> -
> -	if (conn_state->commit &&
> -	    !try_wait_for_completion(&conn_state->commit->hw_done)) {
> -		mutex_unlock(&vc4_hdmi->mutex);
> -		return 0;
> -	}
> -
> -	ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> -	if (ret < 0) {
> -		drm_err(drm, "Failed to read TMDS config: %d\n", ret);
> -		mutex_unlock(&vc4_hdmi->mutex);
> -		return 0;
> -	}
> -
> -	if (!!(config & SCDC_SCRAMBLING_ENABLE) == scrambling_needed) {
> -		mutex_unlock(&vc4_hdmi->mutex);
> -		return 0;
> -	}
> -
> -	mutex_unlock(&vc4_hdmi->mutex);
> -
> -	/*
> -	 * HDMI 2.0 says that one should not send scrambled data
> -	 * prior to configuring the sink scrambling, and that
> -	 * TMDS clock/data transmission should be suspended when
> -	 * changing the TMDS clock rate in the sink. So let's
> -	 * just do a full modeset here, even though some sinks
> -	 * would be perfectly happy if were to just reconfigure
> -	 * the SCDC settings on the fly.
> -	 */
> -	return drm_atomic_helper_reset_crtc(crtc, ctx);
> -}
> -
> -static int vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> -				   struct drm_modeset_acquire_ctx *ctx,
> -				   enum drm_connector_status status)
> -{
> -	struct drm_connector *connector = &vc4_hdmi->connector;
> -
> -	/*
> -	 * NOTE: This function should really be called with vc4_hdmi->mutex
> -	 * held, but doing so results in reentrancy issues since
> -	 * cec_s_phys_addr() might call .adap_enable, which leads to that
> -	 * funtion being called with our mutex held.
> -	 *
> -	 * A similar situation occurs with vc4_hdmi_reset_link() that
> -	 * will call into our KMS hooks if the scrambling was enabled.
> -	 *
> -	 * Concurrency isn't an issue at the moment since we don't share
> -	 * any state with any of the other frameworks so we can ignore
> -	 * the lock for now.
> -	 */
> -
> -	drm_atomic_helper_connector_hdmi_hotplug(connector, ctx, status);
> -
> -	if (status != connector_status_connected)
> -		return 0;
> -
> -	return vc4_hdmi_reset_link(connector, ctx);
> -}
> -
>  static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
>  					 struct drm_modeset_acquire_ctx *ctx,
>  					 bool force)
> @@ -383,8 +257,8 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
>  	/*
>  	 * NOTE: This function should really take vc4_hdmi->mutex, but
>  	 * doing so results in reentrancy issues since
> -	 * vc4_hdmi_handle_hotplug() can call into other functions that
> -	 * would take the mutex while it's held here.
> +	 * drm_atomic_helper_connector_hdmi_hotplug() can call into other
> +	 * functions that would take the mutex while it's held here.
>  	 *
>  	 * Concurrency isn't an issue at the moment since we don't share
>  	 * any state with any of the other frameworks so we can ignore
> @@ -407,7 +281,8 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
>  			status = connector_status_connected;
>  	}
>  
> -	ret = vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
> +	ret = drm_atomic_helper_connector_hdmi_hotplug(connector, ctx, status);
> +
>  	pm_runtime_put(&vc4_hdmi->pdev->dev);
>  
>  	return ret == -EDEADLK ? ret : status;
> @@ -534,6 +409,14 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
>  
>  	drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
>  
> +	/*
> +	 * Since we don't know the state of the controller and its
> +	 * display (if any), let's assume it's always enabled.
> +	 * drm_connector_hdmi_disable_scrambling() will thus run at boot,
> +	 * make sure it's disabled, and avoid any inconsistency.
> +	 */
> +	connector->hdmi.scrambler_enabled = drm_connector_hdmi_scrambler_supported(connector);
> +
>  	/*
>  	 * Some of the properties below require access to state, like bpc.
>  	 * Allocate some default initial connector state with our reset helper.
> @@ -759,32 +642,15 @@ static int vc4_hdmi_write_spd_infoframe(struct drm_connector *connector,
>  					buffer, len);
>  }
>  
> -#define SCRAMBLING_POLLING_DELAY_MS	1000
> -
> -static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
> +static int vc4_hdmi_scrambler_enable(struct drm_connector *connector)
>  {
> -	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> -	struct drm_connector *connector = &vc4_hdmi->connector;
> +	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
>  	struct drm_device *drm = connector->dev;
> -	const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
>  	unsigned long flags;
>  	int idx;
>  
> -	lockdep_assert_held(&vc4_hdmi->mutex);
> -
> -	if (!vc4_hdmi_supports_scrambling(vc4_hdmi))
> -		return;
> -
> -	if (!drm_hdmi_mode_needs_scrambling(mode,
> -					    vc4_hdmi->output_bpc,
> -					    vc4_hdmi->output_format))
> -		return;
> -
>  	if (!drm_dev_enter(drm, &idx))
> -		return;
> -
> -	drm_scdc_set_high_tmds_clock_ratio(connector, true);
> -	drm_scdc_set_scrambling(connector, true);
> +		return -ENODEV;

Not being able to acquire the device isn't an error, it's something we should ignore.

>  
>  	spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
>  	HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |
> @@ -793,59 +659,27 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
>  
>  	drm_dev_exit(idx);
>  
> -	vc4_hdmi->scdc_enabled = true;
> -
> -	queue_delayed_work(system_percpu_wq, &vc4_hdmi->scrambling_work,
> -			   msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
> +	return 0;
>  }
>  
> -static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
> +static int vc4_hdmi_scrambler_disable(struct drm_connector *connector)
>  {
> -	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> -	struct drm_connector *connector = &vc4_hdmi->connector;
> +	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
>  	struct drm_device *drm = connector->dev;
>  	unsigned long flags;
>  	int idx;
>  
> -	lockdep_assert_held(&vc4_hdmi->mutex);
> -
> -	if (!vc4_hdmi->scdc_enabled)
> -		return;
> -
> -	vc4_hdmi->scdc_enabled = false;
> -
> -	if (delayed_work_pending(&vc4_hdmi->scrambling_work))
> -		cancel_delayed_work_sync(&vc4_hdmi->scrambling_work);
> -
>  	if (!drm_dev_enter(drm, &idx))
> -		return;
> +		return -ENODEV;

Ditto

Maxime

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

-----BEGIN PGP SIGNATURE-----

iJUEABMJAB0WIQTkHFbLp4ejekA/qfgnX84Zoj2+dgUCaobj1wAKCRAnX84Zoj2+
dmo7AYC6qAWiFTzzVaM68fskflGcMwmLOcb9y0OYwWOsoasV6dPr3yy6NGu6fTNY
CiCZjB4Bf1PC5Y4/LulW8HtYxYjWkKyysCkpCLKgcRlR05xvvWqJYpopGgcFM6BA
3jG2X32FJw==
=ce98
-----END PGP SIGNATURE-----