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

[Cristian Ciocaltea]

On 6/25/26 11:05 AM, Maxime Ripard wrote:
> 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.

Ack.

Thanks,
Cristian