linux-kernel - Re: [PATCH] drm: do not call detect for connectors which are forced on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221107123657.24vbgep3jqeklb2s@houat>
Date:   Mon, 7 Nov 2022 13:36:57 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Michael Rodin <mrodin@...adit-jv.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Alex Deucher <alexander.deucher@....com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Sean Paul <seanpaul@...omium.org>,
        Vincent Abriou <vincent.abriou@...com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        michael@...in.online, erosca@...adit-jv.com
Subject: Re: [PATCH] drm: do not call detect for connectors which are forced
 on

On Fri, Aug 26, 2022 at 11:11:21AM +0200, Michael Rodin wrote:
> "detect" should not be called and its return value shall not be used when a
> connector is forced as hinted in the commit d50ba256b5f1 ("drm/kms: start
> adding command line interface using fb.") and the commit 6fe14acd496e
> ("drm: Document drm_connector_funcs"). One negative side effect of doing
> this is observed on the RCar3 SoCs which use the dw-hdmi driver. It
> continues executing drm_helper_hpd_irq_event even if its connector is
> forced to ON. As consequence drm_helper_hpd_irq_event calls "detect" so the
> connector status is updated to "disconnected":
> 
> [  420.201527] [drm:drm_helper_hpd_irq_event] [CONNECTOR:76:HDMI-A-1] status updated from connected to disconnected
> 
> This status is corrected by drm_helper_probe_single_connector_modes shortly
> after this because this function checks if a connector is forced:
> 
> [  420.218703] [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:76:HDMI-A-1] status updated from disconnected to connected
> 
> To avoid similar issues this commit adapts functions which call "detect"
> so they check if a connector is forced and return the correct status.
> 
> Fixes: 949f08862d66 ("drm: Make the connector .detect() callback optional")
> Signed-off-by: Michael Rodin <mrodin@...adit-jv.com>
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index bb427c5a4f1f..1691047d0472 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -289,7 +289,12 @@ drm_helper_probe_detect_ctx(struct drm_connector *connector, bool force)
>  retry:
>  	ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, &ctx);
>  	if (!ret) {
> -		if (funcs->detect_ctx)
> +		if (connector->force == DRM_FORCE_ON ||
> +		    connector->force == DRM_FORCE_ON_DIGITAL)
> +			ret = connector_status_connected;
> +		else if (connector->force == DRM_FORCE_OFF)
> +			ret = connector_status_disconnected;
> +		else if (funcs->detect_ctx)
>  			ret = funcs->detect_ctx(connector, &ctx, force);
>  		else if (connector->funcs->detect)
>  			ret = connector->funcs->detect(connector, force);

Actually, I think this creates a regression on vc4 at least (and
possibly i915?), when the HDMI scrambling is on.

When a monitor is disconnected and then connected again, the mode won't
change by itself, but it will lose its scrambling status. The way we
dealt with it (and documented here:
https://lore.kernel.org/all/20220829134731.213478-9-maxime@cerno.tech/)
is to have detect enabling the scrambler again if it detects a
disconnection/connection transition.

If we force the status on all the time, then we will never get called
and we won't have the opportunity to enable the scrambler again.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)