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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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@collabora.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