linux-kernel - Re: [PATCH] drm/bridge: anx7625: Notify audio framework of connector status changes

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] [day] [month] [year] [list]

Message-ID: <3fac3cdc-2c05-57b8-d854-a1f517616706@collabora.com>
Date:   Fri, 23 Sep 2022 10:51:57 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Nícolas F. R. A. Prado 
        <nfraprado@...labora.com>, Robert Foss <robert.foss@...aro.org>
Cc:     kernel@...labora.com, Andrzej Hajda <andrzej.hajda@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Javier Martinez Canillas <javierm@...hat.com>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Jonas Karlman <jonas@...boo.se>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Maxime Ripard <maxime@...no.tech>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Xin Ji <xji@...logixsemi.com>, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: anx7625: Notify audio framework of connector
 status changes

Il 23/09/22 01:21, Nícolas F. R. A. Prado ha scritto:
> We already have anx7625_audio_update_connector_status() to notify the
> audio framework when HDMI is (dis)connected, but up until now this
> function was only called during the callback setup. Add a call for it
> to the HPD change handling logic.
> 
> A couple functions needed to be hoisted as part of this.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> 
> ---
> 
>   drivers/gpu/drm/bridge/analogix/anx7625.c | 48 ++++++++++++-----------
>   1 file changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index b0ff1ecb80a5..da54d6a61e07 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1588,6 +1588,29 @@ static int anx7625_hpd_change_detect(struct anx7625_data *ctx)
>   	return 0;
>   }
>   

..snip..

> +
>   static void anx7625_work_func(struct work_struct *work)
>   {
>   	int event;
> @@ -1603,6 +1626,8 @@ static void anx7625_work_func(struct work_struct *work)
>   	if (event < 0)
>   		goto unlock;
>   
> +	anx7625_audio_update_connector_status(ctx, anx7625_sink_detect(ctx));

I think that this can be done in a smarter way.

In this worker, you have a call to anx7625_hpd_change_detect(): this is checking
the status of the hotplug detect signal (see it like a GPIO, as that's just a pin
in the DVI/HDMI/DP physical connectors going LOW or HIGH for "presence" detection),
so you should be able to make that decision in function dp_hpd_change_handler().

(Please correct me if I'm wrong!)

If HPD is LOW, this means that the external display was unplugged, so you should
be able to do something like:

static void dp_hpd_change_handler(struct .....)
{
	....
	if (on == 0) {
		anx7625_audio_update_connector_status(ctx, on);
		remove_edid(), stop_dp()
	} else {
		start_dp, adjust_swing,
		and7625_audio_update_connector_status(ctx, on);
	}
}

That's to make sure that we send unplug events *before* stopping the DP.

What do you think?

Cheers,
Angelo