2022-09-22 23:33:52

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH] drm/bridge: anx7625: Notify audio framework of connector status changes

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 <[email protected]>

---

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;
}

+static void
+anx7625_audio_update_connector_status(struct anx7625_data *ctx,
+ enum drm_connector_status status)
+{
+ if (ctx->plugged_cb && ctx->codec_dev) {
+ ctx->plugged_cb(ctx->codec_dev,
+ status == connector_status_connected);
+ }
+}
+
+static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
+{
+ struct device *dev = &ctx->client->dev;
+
+ DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
+
+ if (ctx->pdata.panel_bridge)
+ return connector_status_connected;
+
+ return ctx->hpd_status ? connector_status_connected :
+ connector_status_disconnected;
+}
+
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));
+
if (ctx->bridge_attached)
drm_helper_hpd_irq_event(ctx->bridge.dev);

@@ -1790,19 +1815,6 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
return (struct edid *)edid;
}

-static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
-{
- struct device *dev = &ctx->client->dev;
-
- DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
-
- if (ctx->pdata.panel_bridge)
- return connector_status_connected;
-
- return ctx->hpd_status ? connector_status_connected :
- connector_status_disconnected;
-}
-
static int anx7625_audio_hw_params(struct device *dev, void *data,
struct hdmi_codec_daifmt *fmt,
struct hdmi_codec_params *params)
@@ -1951,16 +1963,6 @@ static int anx7625_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
return 0;
}

-static void
-anx7625_audio_update_connector_status(struct anx7625_data *ctx,
- enum drm_connector_status status)
-{
- if (ctx->plugged_cb && ctx->codec_dev) {
- ctx->plugged_cb(ctx->codec_dev,
- status == connector_status_connected);
- }
-}
-
static int anx7625_audio_hook_plugged_cb(struct device *dev, void *data,
hdmi_codec_plugged_cb fn,
struct device *codec_dev)
--
2.37.3


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 <[email protected]>
>
> ---
>
> 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