Re: [PATCH v7 04/30] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
> +static int drm_scdc_reset_crtc(struct drm_connector *connector,
> +			       struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_crtc *crtc;
> +	u8 config;
> +	int ret;
> +
> +	if (!ctx)
> +		return 0;
> +
> +	/*
> +	 * This is normally part of .detect_ctx() call path, which already holds
> +	 * connection_mutex through @ctx.  However, re-acquiring it with the
> +	 * same context is a no-op and makes the helper safe under any caller.
> +	 */
> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
> +	if (ret)
> +		return ret;
> +
> +	if (!connector->state)
> +		return 0;
> +
> +	crtc = connector->state->crtc;
> +	if (!crtc)
> +		return 0;
> +
> +	ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> +	if (ret) {
> +		drm_scdc_dbg(connector, "Failed to read TMDS config: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if ((config & SCDC_SCRAMBLING_ENABLE) &&
> +	    (config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40))
> +		return 0;

I don't think we should care about the bit clock ratio here. From a
technical standpoint, and since you ignore low rates for the scrambler
for now, they are indeed equivalents. But semantically, scrambler can be
enabled for any rate, and the bit clock ratio should be changed only for
rates higher than 340MHz.

This function makes sure to trigger a modeset when the scrambler is
enabled or not. From a spec point of view, it's somewhat orthogonal to
the bit clock ratio. From a function name point of view, it's
misleading.

So yeah, I'd just drop the check on the bit clock ratio here. Drivers
will then get to enable either when the commit actually happens.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]