drm: apple: audio: Rework audio service handling

'open' and 'close' the service/link in iomfb's power-on and shutdown and
on HPD deassert. This avoids leaking DCPAVAudioInterface services over
display power cycles and tears the service properly down.
For unknown reasons this is only observed with DCPs connected to atc
phys as for DP altmode and the HDMI ports on Macbook Pros.

Signed-off-by: Janne Grunau <j@jannau.net>

drm: apple: Rework audio service initialization

Signed-off-by: Janne Grunau <j@jannau.net>

Author: jannau

drivers/gpu/drm/apple/av.c

@@ -13,12 +13,14 @@
 
 #include "audio.h"
 #include "afk.h"
+#include "av.h"
 #include "dcp.h"
 #include "dcp-internal.h"
 
 struct dcp_av_audio_cmds {
 	/* commands in group 0*/
 	u32 open;
+	u32 close;
 	u32 prepare;
 	u32 start_link;
 	u32 stop_link;
@@ -30,6 +32,7 @@ struct dcp_av_audio_cmds {
 
 static const struct dcp_av_audio_cmds dcp_av_audio_cmds_v12_3 = {
 	.open = 6,
+	.close = 7,
 	.prepare = 8,
 	.start_link = 9,
 	.stop_link = 12,
@@ -40,6 +43,7 @@ static const struct dcp_av_audio_cmds dcp_av_audio_cmds_v12_3 = {
 
 static const struct dcp_av_audio_cmds dcp_av_audio_cmds_v13_5 = {
 	.open = 4,
+	.close = 5,
 	.prepare = 6,
 	.start_link = 7,
 	.stop_link = 10,
@@ -62,6 +66,7 @@ struct audiosrv_data {
 
 	bool warned_get_elements;
 	bool warned_get_product_attrs;
+	bool is_open;
 };
 
 static void av_interface_init(struct apple_epic_service *service, const char *name,
@@ -285,34 +290,89 @@ static const struct apple_epic_service_ops avep_ops[] = {
 	{}
 };
 
-static void av_work_service_start(struct work_struct *work)
+void av_service_connect(struct apple_dcp *dcp)
 {
+	struct apple_epic_service *service;
+	struct audiosrv_data *asrv = dcp->audiosrv;
 	int ret;
-	struct audiosrv_data *audiosrv_data;
-	struct apple_dcp *dcp;
 
-	audiosrv_data = container_of(work, struct audiosrv_data, start_av_service_wq);
-	if (!audiosrv_data->srv ||
-	    !audiosrv_data->srv->ep ||
-	    !audiosrv_data->srv->ep->dcp) {
-		pr_err("%s: dcp: av: NULL ptr during startup\n", __func__);
-		return;
+	scoped_guard(rwsem_write, &asrv->srv_rwsem) {
+		if (!asrv->srv)
+			return;
+		service = asrv->srv;
 	}
-	dcp = audiosrv_data->srv->ep->dcp;
 
 	/* open AV audio service */
-	dev_info(dcp->dev, "%s: starting audio service\n", __func__);
-	ret = afk_service_call(dcp->audiosrv->srv, 0, dcp->audiosrv->cmds.open,
-			       NULL, 0, 32, NULL, 0, 32);
+	dev_info(dcp->dev, "%s: starting audio service, plugged:%d\n", __func__,  asrv->plugged);
+	if (asrv->is_open)
+		return;
+
+	ret = afk_service_call(service, 0, asrv->cmds.open, NULL, 0, 32,
+			       NULL, 0, 32);
 	if (ret) {
 		dev_err(dcp->dev, "error opening audio service: %d\n", ret);
 		return;
 	}
+	mutex_lock(&asrv->plug_lock);
+	asrv->is_open = true;
 
-	mutex_lock(&dcp->audiosrv->plug_lock);
-	if (dcp->audiosrv->audio_dev)
-		dcpaud_connect(dcp->audiosrv->audio_dev, dcp->audiosrv->plugged);
-	mutex_unlock(&dcp->audiosrv->plug_lock);
+	if (asrv->audio_dev)
+		dcpaud_connect(asrv->audio_dev, asrv->plugged);
+	mutex_unlock(&asrv->plug_lock);
+}
+
+void av_service_disconnect(struct apple_dcp *dcp)
+{
+	struct apple_epic_service *service;
+	struct audiosrv_data *asrv = dcp->audiosrv;
+	int ret;
+
+	scoped_guard(rwsem_write, &asrv->srv_rwsem) {
+		if (!asrv->srv)
+			return;
+		service = asrv->srv;
+	}
+
+	/* close AV audio service */
+	dev_info(dcp->dev, "%s: stopping audio service\n", __func__);
+	if (!asrv->is_open)
+		return;
+
+	mutex_lock(&asrv->plug_lock);
+
+	if (asrv->audio_dev)
+		dcpaud_disconnect(asrv->audio_dev);
+
+	mutex_unlock(&asrv->plug_lock);
+
+	ret = afk_service_call(service, 0, asrv->cmds.close, NULL, 0, 16,
+			       NULL, 0, 16);
+	if (ret) {
+		dev_err(dcp->dev, "error closing audio service: %d\n", ret);
+	}
+	if (service->torndown)
+		service->enabled = false;
+	asrv->is_open = false;
+}
+
+static void av_work_service_start(struct work_struct *work)
+{
+	struct audiosrv_data *audiosrv_data;
+	struct apple_dcp *dcp;
+
+	audiosrv_data = container_of(work, struct audiosrv_data, start_av_service_wq);
+
+	scoped_guard(rwsem_read, &audiosrv_data->srv_rwsem) {
+		if (!audiosrv_data->srv ||
+		!audiosrv_data->srv->ep ||
+		!audiosrv_data->srv->ep->dcp) {
+			pr_err("%s: dcp: av: NULL ptr during startup\n", __func__);
+			return;
+		}
+		dcp = audiosrv_data->srv->ep->dcp;
+	}
+
+	av_service_connect(dcp);
 }
 
 int avep_init(struct apple_dcp *dcp)
@@ -339,9 +399,9 @@ int avep_init(struct apple_dcp *dcp)
 		dev_err(dcp->dev, "Audio not supported for firmware\n");
 		return -ENODEV;
 	}
-	INIT_WORK(&audiosrv_data->start_av_service_wq, av_work_service_start);
 
 	dcp->audiosrv = audiosrv_data;
+	INIT_WORK(&audiosrv_data->start_av_service_wq, av_work_service_start);
 
 	endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
 	if (endpoint) {

drivers/gpu/drm/apple/av.h

@@ -6,4 +6,7 @@
 //int avep_audiosrv_startlink(struct apple_dcp *dcp, struct dcp_sound_cookie *cookie);
 //int avep_audiosrv_stoplink(struct apple_dcp *dcp);
 
+void av_service_connect(struct apple_dcp *dcp);
+void av_service_disconnect(struct apple_dcp *dcp);
+
 #endif /* __AV_H__ */

drivers/gpu/drm/apple/dcp.c

@@ -31,6 +31,7 @@
 #include <drm/drm_vblank.h>
 
 #include "afk.h"
+#include "av.h"
 #include "dcp.h"
 #include "dcp-internal.h"
 #include "iomfb.h"
@@ -413,6 +414,9 @@ static int dcp_dptx_connect(struct apple_dcp *dcp, u32 port)
 	if (dcp->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
 		dptxport_set_hpd(dcp->dptxport[port].service, true);
 
+	if (dcp->avep)
+		av_service_connect(dcp);
+
 	return 0;
 
 out_unlock:
@@ -444,6 +448,8 @@ EXPORT_SYMBOL_GPL(dcp_dptx_connect_oob);
 int dcp_dptx_disconnect_oob(struct platform_device *pdev, u32 port)
 {
 	struct apple_dcp *dcp = platform_get_drvdata(pdev);
+	if (dcp->avep)
+		av_service_disconnect(dcp);
 	dptxport_set_hpd(dcp->dptxport[port].service, false);
 	return dcp_dptx_disconnect(dcp, port);
 }
@@ -629,13 +635,19 @@ void dcp_poweron(struct platform_device *pdev)
 		WARN_ONCE(true, "Unexpected firmware version: %u\n", dcp->fw_compat);
 		break;
 	}
+
+	if (dcp->avep)
+		av_service_connect(dcp);
 }
 EXPORT_SYMBOL(dcp_poweron);
 
 void dcp_poweroff(struct platform_device *pdev)
 {
 	struct apple_dcp *dcp = platform_get_drvdata(pdev);
 
+	if (dcp->avep)
+		av_service_disconnect(dcp);
+
 	switch (dcp->fw_compat) {
 	case DCP_FIRMWARE_V_12_3:
 		iomfb_poweroff_v12_3(dcp);
@@ -1074,6 +1086,7 @@ static void dcp_comp_unbind(struct device *dev, struct device *main, void *data)
 		disable_irq(dcp->hdmi_hpd_irq);
 
 	if (dcp->avep) {
+		av_service_disconnect(dcp);
 		afk_shutdown(dcp->avep);
 		dcp->avep = NULL;
 	}
@@ -1230,6 +1243,9 @@ static int dcp_platform_suspend(struct device *dev)
 {
 	struct apple_dcp *dcp = dev_get_drvdata(dev);
 
+	if (dcp->avep)
+		av_service_disconnect(dcp);
+
 	if (dcp->hdmi_hpd_irq) {
 		disable_irq(dcp->hdmi_hpd_irq);
 		dcp_dptx_disconnect(dcp, 0);
@@ -1258,6 +1274,9 @@ static int dcp_platform_resume(struct device *dev)
 			dcp_dptx_connect(dcp, 0);
 	}
 
+	if (dcp->avep)
+		av_service_connect(dcp);
+
 	return 0;
 }