[PATCH 1/2] drm/amd/display: Expose audio inst from DC to DM

[PATCH 1/2] drm/amd/display: Expose audio inst from DC to DM

[Nicholas Kazlauskas]

[Why]
In order to give pin notifications to the sound driver from DM we need
to know whether audio is enabled on a stream and what pin it's using
from DC.

[How]
Expose the instance via stream status if it's a mapped resource for
the stream. It will be -1 if there's no audio mapped.

Cc: Leo Li <sunpeng.li@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 3 +++
 drivers/gpu/drm/amd/display/dc/dc_stream.h        | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index d10ebfd33a60..5b85139fb3ce 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2008,6 +2008,9 @@ enum dc_status resource_map_pool_resources(
 		if (context->streams[i] == stream) {
 			context->stream_status[i].primary_otg_inst = pipe_ctx->stream_res.tg->inst;
 			context->stream_status[i].stream_enc_inst = pipe_ctx->stream_res.stream_enc->id;
+			context->stream_status[i].audio_inst =
+				pipe_ctx->stream_res.audio ? pipe_ctx->stream_res.audio->inst : -1;
+
 			return DC_OK;
 		}
 
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index e253a5c591f6..0fa1c26bc20d 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -42,6 +42,7 @@ struct dc_stream_status {
 	int primary_otg_inst;
 	int stream_enc_inst;
 	int plane_count;
+	int audio_inst;
 	struct timing_sync_info timing_sync_info;
 	struct dc_plane_state *plane_states[MAX_SURFACE_NUM];
 };
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Nicholas Kazlauskas]

[Why]
The drm_audio_component can be used to give pin ELD notifications
directly to the sound driver. This fixes audio endpoints disappearing
due to missing unsolicited notifications.

[How]
Send the notification via the audio component whenever we enable or
disable audio state on a stream. This matches what i915 does with
their drm_audio_component and what Takashi Iwai's proposed hack for
radeon/amdpgu did.

This is a bit delayed in when the notification actually occurs, however.
We wait until after all the programming is complete rather than sending
the notification mid sequence.

Particular care is needed for the get ELD callback since it can happen
outside the locking and fencing DRM does for atomic commits.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 222 ++++++++++++++++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  25 ++
 2 files changed, 247 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f26c7b348aa9..9ef785497eb5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -55,6 +55,7 @@
 #include <linux/types.h>
 #include <linux/pm_runtime.h>
 #include <linux/firmware.h>
+#include <linux/component.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
@@ -63,6 +64,7 @@
 #include <drm/drm_dp_mst_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_audio_component.h>
 
 #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
 #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
@@ -506,6 +508,139 @@ static void amdgpu_dm_fbc_init(struct drm_connector *connector)
 
 }
 
+static int amdgpu_dm_audio_component_get_eld(struct device *kdev, int port,
+					  int pipe, bool *enabled,
+					  unsigned char *buf, int max_bytes)
+{
+	struct drm_device *dev = dev_get_drvdata(kdev);
+	struct amdgpu_device *adev = dev->dev_private;
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	struct amdgpu_dm_connector *aconnector;
+	int ret = 0;
+
+	*enabled = false;
+
+	mutex_lock(&adev->dm.audio_lock);
+
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		aconnector = to_amdgpu_dm_connector(connector);
+		if (aconnector->audio_inst != port)
+			continue;
+
+		*enabled = true;
+		ret = drm_eld_size(connector->eld);
+		memcpy(buf, connector->eld, min(max_bytes, ret));
+
+		break;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+
+	mutex_unlock(&adev->dm.audio_lock);
+
+	DRM_DEBUG_KMS("Get ELD : idx=%d ret=%d en=%d\n", port, ret, *enabled);
+
+	return ret;
+}
+
+static const struct drm_audio_component_ops amdgpu_dm_audio_component_ops = {
+	.get_eld = amdgpu_dm_audio_component_get_eld,
+};
+
+static int amdgpu_dm_audio_component_bind(struct device *kdev,
+				       struct device *hda_kdev, void *data)
+{
+	struct drm_device *dev = dev_get_drvdata(kdev);
+	struct amdgpu_device *adev = dev->dev_private;
+	struct drm_audio_component *acomp = data;
+
+	acomp->ops = &amdgpu_dm_audio_component_ops;
+	acomp->dev = kdev;
+	adev->dm.audio_component = acomp;
+
+	return 0;
+}
+
+static void amdgpu_dm_audio_component_unbind(struct device *kdev,
+					  struct device *hda_kdev, void *data)
+{
+	struct drm_device *dev = dev_get_drvdata(kdev);
+	struct amdgpu_device *adev = dev->dev_private;
+	struct drm_audio_component *acomp = data;
+
+	acomp->ops = NULL;
+	acomp->dev = NULL;
+	adev->dm.audio_component = NULL;
+}
+
+static const struct component_ops amdgpu_dm_audio_component_bind_ops = {
+	.bind	= amdgpu_dm_audio_component_bind,
+	.unbind	= amdgpu_dm_audio_component_unbind,
+};
+
+static int amdgpu_dm_audio_init(struct amdgpu_device *adev)
+{
+	int i, ret;
+
+	if (!amdgpu_audio)
+		return 0;
+
+	adev->mode_info.audio.enabled = true;
+
+	adev->mode_info.audio.num_pins = adev->dm.dc->res_pool->audio_count;
+
+	for (i = 0; i < adev->mode_info.audio.num_pins; i++) {
+		adev->mode_info.audio.pin[i].channels = -1;
+		adev->mode_info.audio.pin[i].rate = -1;
+		adev->mode_info.audio.pin[i].bits_per_sample = -1;
+		adev->mode_info.audio.pin[i].status_bits = 0;
+		adev->mode_info.audio.pin[i].category_code = 0;
+		adev->mode_info.audio.pin[i].connected = false;
+		adev->mode_info.audio.pin[i].id =
+			adev->dm.dc->res_pool->audios[i]->inst;
+		adev->mode_info.audio.pin[i].offset = 0;
+	}
+
+	ret = component_add(adev->dev, &amdgpu_dm_audio_component_bind_ops);
+	if (ret < 0)
+		return ret;
+
+	adev->dm.audio_registered = true;
+
+	return 0;
+}
+
+static void amdgpu_dm_audio_fini(struct amdgpu_device *adev)
+{
+	if (!amdgpu_audio)
+		return;
+
+	if (!adev->mode_info.audio.enabled)
+		return;
+
+	if (adev->dm.audio_registered) {
+		component_del(adev->dev, &amdgpu_dm_audio_component_bind_ops);
+		adev->dm.audio_registered = false;
+	}
+
+	/* TODO: Disable audio? */
+
+	adev->mode_info.audio.enabled = false;
+}
+
+void amdgpu_dm_audio_eld_notify(struct amdgpu_device *adev, int pin)
+{
+	struct drm_audio_component *acomp = adev->dm.audio_component;
+
+	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+		DRM_DEBUG_KMS("Notify ELD: %d\n", pin);
+
+		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+						 pin, -1);
+	}
+}
+
 static int amdgpu_dm_init(struct amdgpu_device *adev)
 {
 	struct dc_init_data init_data;
@@ -516,6 +651,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 	memset(&init_data, 0, sizeof(init_data));
 
 	mutex_init(&adev->dm.dc_lock);
+	mutex_init(&adev->dm.audio_lock);
 
 	if(amdgpu_dm_irq_init(adev)) {
 		DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n");
@@ -619,6 +755,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
 static void amdgpu_dm_fini(struct amdgpu_device *adev)
 {
+	amdgpu_dm_audio_fini(adev);
+
 	amdgpu_dm_destroy_drm_device(&adev->dm);
 
 	/* DC Destroy TODO: Replace destroy DAL */
@@ -639,6 +777,7 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
 		adev->dm.freesync_module = NULL;
 	}
 
+	mutex_destroy(&adev->dm.audio_lock);
 	mutex_destroy(&adev->dm.dc_lock);
 
 	return;
@@ -1887,6 +2026,10 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
 	if (r)
 		return r;
 
+	r = amdgpu_dm_audio_init(adev);
+	if (r)
+		return r;
+
 	return 0;
 }
 
@@ -4803,6 +4946,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 	aconnector->base.stereo_allowed = false;
 	aconnector->base.dpms = DRM_MODE_DPMS_OFF;
 	aconnector->hpd.hpd = AMDGPU_HPD_NONE; /* not used */
+	aconnector->audio_inst = -1;
 	mutex_init(&aconnector->hpd_lock);
 
 	/*
@@ -5697,6 +5841,81 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
 	kfree(bundle);
 }
 
+static void amdgpu_dm_commit_audio(struct drm_device *dev,
+				   struct drm_atomic_state *state)
+{
+	struct amdgpu_device *adev = dev->dev_private;
+	struct amdgpu_dm_connector *aconnector;
+	struct drm_connector *connector;
+	struct drm_connector_state *old_con_state, *new_con_state;
+	struct drm_crtc_state *new_crtc_state;
+	struct dm_crtc_state *new_dm_crtc_state;
+	const struct dc_stream_status *status;
+	int i, inst;
+
+	/* Notify device removals. */
+	for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) {
+		if (old_con_state->crtc != new_con_state->crtc) {
+			/* CRTC changes require notification. */
+			goto notify;
+		}
+
+		if (!new_con_state->crtc)
+			continue;
+
+		new_crtc_state = drm_atomic_get_new_crtc_state(
+			state, new_con_state->crtc);
+
+		if (!new_crtc_state)
+			continue;
+
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
+			continue;
+
+	notify:
+		aconnector = to_amdgpu_dm_connector(connector);
+
+		mutex_lock(&adev->dm.audio_lock);
+		inst = aconnector->audio_inst;
+		aconnector->audio_inst = -1;
+		mutex_unlock(&adev->dm.audio_lock);
+
+		amdgpu_dm_audio_eld_notify(adev, inst);
+	}
+
+	/* Notify audio device additions. */
+	for_each_new_connector_in_state(state, connector, new_con_state, i) {
+		if (!new_con_state->crtc)
+			continue;
+
+		new_crtc_state = drm_atomic_get_new_crtc_state(
+			state, new_con_state->crtc);
+
+		if (!new_crtc_state)
+			continue;
+
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
+			continue;
+
+		new_dm_crtc_state = to_dm_crtc_state(new_crtc_state);
+		if (!new_dm_crtc_state->stream)
+			continue;
+
+		status = dc_stream_get_status(new_dm_crtc_state->stream);
+		if (!status)
+			continue;
+
+		aconnector = to_amdgpu_dm_connector(connector);
+
+		mutex_lock(&adev->dm.audio_lock);
+		inst = status->audio_inst;
+		aconnector->audio_inst = inst;
+		mutex_unlock(&adev->dm.audio_lock);
+
+		amdgpu_dm_audio_eld_notify(adev, inst);
+	}
+}
+
 /*
  * Enable interrupts on CRTCs that are newly active, undergone
  * a modeset, or have active planes again.
@@ -6060,6 +6279,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 	/* Enable interrupts for CRTCs going from 0 to n active planes. */
 	amdgpu_dm_enable_crtc_interrupts(dev, state, false);
 
+	/* Update audio instances for each connector. */
+	amdgpu_dm_commit_audio(dev, state);
+
 	/*
 	 * send vblank event on all events not handled in flip and
 	 * mark consumed event for drm_atomic_helper_commit_hw_done
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 1d498e6dc1fe..c7cd0cd4a85c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -140,6 +140,28 @@ struct amdgpu_display_manager {
 	 */
 	struct mutex dc_lock;
 
+	/**
+	 * @audio_lock:
+	 *
+	 * Guards access to audio instance changes.
+	 */
+	struct mutex audio_lock;
+
+	/**
+	 * @audio_component:
+	 *
+	 * Used to notify ELD changes to sound driver.
+	 */
+	struct drm_audio_component *audio_component;
+
+	/**
+	 * @audio_registered:
+	 *
+	 * True if the audio component has been registered
+	 * successfully, false otherwise.
+	 */
+	bool audio_registered;
+
 	/**
 	 * @irq_handler_list_low_tab:
 	 *
@@ -251,6 +273,9 @@ struct amdgpu_dm_connector {
 	int max_vfreq ;
 	int pixel_clock_mhz;
 
+	/* Audio instance - protected by audio_lock. */
+	int audio_inst;
+
 	struct mutex hpd_lock;
 
 	bool fake_enable;
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Alex Deucher]

On Tue, Jul 9, 2019 at 12:30 PM Nicholas Kazlauskas
<nicholas.kazlauskas@amd.com> wrote:
>
> [Why]
> The drm_audio_component can be used to give pin ELD notifications
> directly to the sound driver. This fixes audio endpoints disappearing
> due to missing unsolicited notifications.
>
> [How]
> Send the notification via the audio component whenever we enable or
> disable audio state on a stream. This matches what i915 does with
> their drm_audio_component and what Takashi Iwai's proposed hack for
> radeon/amdpgu did.
>
> This is a bit delayed in when the notification actually occurs, however.
> We wait until after all the programming is complete rather than sending
> the notification mid sequence.
>
> Particular care is needed for the get ELD callback since it can happen
> outside the locking and fencing DRM does for atomic commits.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 222 ++++++++++++++++++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  25 ++
>  2 files changed, 247 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index f26c7b348aa9..9ef785497eb5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -55,6 +55,7 @@
>  #include <linux/types.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/firmware.h>
> +#include <linux/component.h>
>
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic.h>
> @@ -63,6 +64,7 @@
>  #include <drm/drm_dp_mst_helper.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_audio_component.h>
>
>  #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
>  #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
> @@ -506,6 +508,139 @@ static void amdgpu_dm_fbc_init(struct drm_connector *connector)
>
>  }
>
> +static int amdgpu_dm_audio_component_get_eld(struct device *kdev, int port,
> +                                         int pipe, bool *enabled,
> +                                         unsigned char *buf, int max_bytes)
> +{
> +       struct drm_device *dev = dev_get_drvdata(kdev);
> +       struct amdgpu_device *adev = dev->dev_private;
> +       struct drm_connector *connector;
> +       struct drm_connector_list_iter conn_iter;
> +       struct amdgpu_dm_connector *aconnector;
> +       int ret = 0;
> +
> +       *enabled = false;
> +
> +       mutex_lock(&adev->dm.audio_lock);
> +
> +       drm_connector_list_iter_begin(dev, &conn_iter);
> +       drm_for_each_connector_iter(connector, &conn_iter) {
> +               aconnector = to_amdgpu_dm_connector(connector);
> +               if (aconnector->audio_inst != port)
> +                       continue;
> +
> +               *enabled = true;
> +               ret = drm_eld_size(connector->eld);
> +               memcpy(buf, connector->eld, min(max_bytes, ret));
> +
> +               break;
> +       }
> +       drm_connector_list_iter_end(&conn_iter);
> +
> +       mutex_unlock(&adev->dm.audio_lock);
> +
> +       DRM_DEBUG_KMS("Get ELD : idx=%d ret=%d en=%d\n", port, ret, *enabled);
> +
> +       return ret;
> +}
> +
> +static const struct drm_audio_component_ops amdgpu_dm_audio_component_ops = {
> +       .get_eld = amdgpu_dm_audio_component_get_eld,
> +};
> +
> +static int amdgpu_dm_audio_component_bind(struct device *kdev,
> +                                      struct device *hda_kdev, void *data)
> +{
> +       struct drm_device *dev = dev_get_drvdata(kdev);
> +       struct amdgpu_device *adev = dev->dev_private;
> +       struct drm_audio_component *acomp = data;
> +
> +       acomp->ops = &amdgpu_dm_audio_component_ops;
> +       acomp->dev = kdev;
> +       adev->dm.audio_component = acomp;
> +
> +       return 0;
> +}
> +
> +static void amdgpu_dm_audio_component_unbind(struct device *kdev,
> +                                         struct device *hda_kdev, void *data)
> +{
> +       struct drm_device *dev = dev_get_drvdata(kdev);
> +       struct amdgpu_device *adev = dev->dev_private;
> +       struct drm_audio_component *acomp = data;
> +
> +       acomp->ops = NULL;
> +       acomp->dev = NULL;
> +       adev->dm.audio_component = NULL;
> +}
> +
> +static const struct component_ops amdgpu_dm_audio_component_bind_ops = {
> +       .bind   = amdgpu_dm_audio_component_bind,
> +       .unbind = amdgpu_dm_audio_component_unbind,
> +};
> +
> +static int amdgpu_dm_audio_init(struct amdgpu_device *adev)
> +{
> +       int i, ret;
> +
> +       if (!amdgpu_audio)
> +               return 0;
> +
> +       adev->mode_info.audio.enabled = true;
> +
> +       adev->mode_info.audio.num_pins = adev->dm.dc->res_pool->audio_count;
> +
> +       for (i = 0; i < adev->mode_info.audio.num_pins; i++) {
> +               adev->mode_info.audio.pin[i].channels = -1;
> +               adev->mode_info.audio.pin[i].rate = -1;
> +               adev->mode_info.audio.pin[i].bits_per_sample = -1;
> +               adev->mode_info.audio.pin[i].status_bits = 0;
> +               adev->mode_info.audio.pin[i].category_code = 0;
> +               adev->mode_info.audio.pin[i].connected = false;
> +               adev->mode_info.audio.pin[i].id =
> +                       adev->dm.dc->res_pool->audios[i]->inst;
> +               adev->mode_info.audio.pin[i].offset = 0;
> +       }
> +
> +       ret = component_add(adev->dev, &amdgpu_dm_audio_component_bind_ops);
> +       if (ret < 0)
> +               return ret;
> +
> +       adev->dm.audio_registered = true;
> +
> +       return 0;
> +}
> +
> +static void amdgpu_dm_audio_fini(struct amdgpu_device *adev)
> +{
> +       if (!amdgpu_audio)
> +               return;
> +
> +       if (!adev->mode_info.audio.enabled)
> +               return;
> +
> +       if (adev->dm.audio_registered) {
> +               component_del(adev->dev, &amdgpu_dm_audio_component_bind_ops);
> +               adev->dm.audio_registered = false;
> +       }
> +
> +       /* TODO: Disable audio? */
> +
> +       adev->mode_info.audio.enabled = false;
> +}
> +
> +void amdgpu_dm_audio_eld_notify(struct amdgpu_device *adev, int pin)
> +{
> +       struct drm_audio_component *acomp = adev->dm.audio_component;
> +
> +       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
> +               DRM_DEBUG_KMS("Notify ELD: %d\n", pin);
> +
> +               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> +                                                pin, -1);
> +       }
> +}
> +
>  static int amdgpu_dm_init(struct amdgpu_device *adev)
>  {
>         struct dc_init_data init_data;
> @@ -516,6 +651,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>         memset(&init_data, 0, sizeof(init_data));
>
>         mutex_init(&adev->dm.dc_lock);
> +       mutex_init(&adev->dm.audio_lock);
>
>         if(amdgpu_dm_irq_init(adev)) {
>                 DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n");
> @@ -619,6 +755,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>
>  static void amdgpu_dm_fini(struct amdgpu_device *adev)
>  {
> +       amdgpu_dm_audio_fini(adev);
> +
>         amdgpu_dm_destroy_drm_device(&adev->dm);
>
>         /* DC Destroy TODO: Replace destroy DAL */
> @@ -639,6 +777,7 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
>                 adev->dm.freesync_module = NULL;
>         }
>
> +       mutex_destroy(&adev->dm.audio_lock);
>         mutex_destroy(&adev->dm.dc_lock);
>
>         return;
> @@ -1887,6 +2026,10 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev)
>         if (r)
>                 return r;
>
> +       r = amdgpu_dm_audio_init(adev);
> +       if (r)
> +               return r;
> +
>         return 0;
>  }
>
> @@ -4803,6 +4946,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
>         aconnector->base.stereo_allowed = false;
>         aconnector->base.dpms = DRM_MODE_DPMS_OFF;
>         aconnector->hpd.hpd = AMDGPU_HPD_NONE; /* not used */
> +       aconnector->audio_inst = -1;
>         mutex_init(&aconnector->hpd_lock);
>
>         /*
> @@ -5697,6 +5841,81 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>         kfree(bundle);
>  }
>
> +static void amdgpu_dm_commit_audio(struct drm_device *dev,
> +                                  struct drm_atomic_state *state)
> +{
> +       struct amdgpu_device *adev = dev->dev_private;
> +       struct amdgpu_dm_connector *aconnector;
> +       struct drm_connector *connector;
> +       struct drm_connector_state *old_con_state, *new_con_state;
> +       struct drm_crtc_state *new_crtc_state;
> +       struct dm_crtc_state *new_dm_crtc_state;
> +       const struct dc_stream_status *status;
> +       int i, inst;
> +
> +       /* Notify device removals. */
> +       for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) {
> +               if (old_con_state->crtc != new_con_state->crtc) {
> +                       /* CRTC changes require notification. */
> +                       goto notify;
> +               }
> +
> +               if (!new_con_state->crtc)
> +                       continue;
> +
> +               new_crtc_state = drm_atomic_get_new_crtc_state(
> +                       state, new_con_state->crtc);
> +
> +               if (!new_crtc_state)
> +                       continue;
> +
> +               if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> +                       continue;
> +
> +       notify:
> +               aconnector = to_amdgpu_dm_connector(connector);
> +
> +               mutex_lock(&adev->dm.audio_lock);
> +               inst = aconnector->audio_inst;
> +               aconnector->audio_inst = -1;
> +               mutex_unlock(&adev->dm.audio_lock);
> +
> +               amdgpu_dm_audio_eld_notify(adev, inst);
> +       }
> +
> +       /* Notify audio device additions. */
> +       for_each_new_connector_in_state(state, connector, new_con_state, i) {
> +               if (!new_con_state->crtc)
> +                       continue;
> +
> +               new_crtc_state = drm_atomic_get_new_crtc_state(
> +                       state, new_con_state->crtc);
> +
> +               if (!new_crtc_state)
> +                       continue;
> +
> +               if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> +                       continue;
> +
> +               new_dm_crtc_state = to_dm_crtc_state(new_crtc_state);
> +               if (!new_dm_crtc_state->stream)
> +                       continue;
> +
> +               status = dc_stream_get_status(new_dm_crtc_state->stream);
> +               if (!status)
> +                       continue;
> +
> +               aconnector = to_amdgpu_dm_connector(connector);
> +
> +               mutex_lock(&adev->dm.audio_lock);
> +               inst = status->audio_inst;
> +               aconnector->audio_inst = inst;
> +               mutex_unlock(&adev->dm.audio_lock);
> +
> +               amdgpu_dm_audio_eld_notify(adev, inst);
> +       }
> +}
> +
>  /*
>   * Enable interrupts on CRTCs that are newly active, undergone
>   * a modeset, or have active planes again.
> @@ -6060,6 +6279,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
>         /* Enable interrupts for CRTCs going from 0 to n active planes. */
>         amdgpu_dm_enable_crtc_interrupts(dev, state, false);
>
> +       /* Update audio instances for each connector. */
> +       amdgpu_dm_commit_audio(dev, state);
> +
>         /*
>          * send vblank event on all events not handled in flip and
>          * mark consumed event for drm_atomic_helper_commit_hw_done
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 1d498e6dc1fe..c7cd0cd4a85c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -140,6 +140,28 @@ struct amdgpu_display_manager {
>          */
>         struct mutex dc_lock;
>
> +       /**
> +        * @audio_lock:
> +        *
> +        * Guards access to audio instance changes.
> +        */
> +       struct mutex audio_lock;
> +
> +       /**
> +        * @audio_component:
> +        *
> +        * Used to notify ELD changes to sound driver.
> +        */
> +       struct drm_audio_component *audio_component;
> +
> +       /**
> +        * @audio_registered:
> +        *
> +        * True if the audio component has been registered
> +        * successfully, false otherwise.
> +        */
> +       bool audio_registered;
> +
>         /**
>          * @irq_handler_list_low_tab:
>          *
> @@ -251,6 +273,9 @@ struct amdgpu_dm_connector {
>         int max_vfreq ;
>         int pixel_clock_mhz;
>
> +       /* Audio instance - protected by audio_lock. */
> +       int audio_inst;
> +
>         struct mutex hpd_lock;
>
>         bool fake_enable;
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Takashi Iwai]

On Tue, 09 Jul 2019 18:30:19 +0200,
Nicholas Kazlauskas wrote:
> 
> [Why]
> The drm_audio_component can be used to give pin ELD notifications
> directly to the sound driver. This fixes audio endpoints disappearing
> due to missing unsolicited notifications.
> 
> [How]
> Send the notification via the audio component whenever we enable or
> disable audio state on a stream. This matches what i915 does with
> their drm_audio_component and what Takashi Iwai's proposed hack for
> radeon/amdpgu did.
> 
> This is a bit delayed in when the notification actually occurs, however.
> We wait until after all the programming is complete rather than sending
> the notification mid sequence.
> 
> Particular care is needed for the get ELD callback since it can happen
> outside the locking and fencing DRM does for atomic commits.
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Thanks for the patch, this has been a long-standing TODO for me, too!

Do you have the patch for HD-audio part as well?  Or you tested with
my old patch?  Then I'll resurrect my patch set as well.

This patch itself looks almost good.  One caveat is that you might
want the reverse select of CONFIG_SND_HDA_COMPONENT, something like:

================================================================
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -223,6 +223,7 @@ config DRM_AMDGPU
 	select BACKLIGHT_CLASS_DEVICE
 	select INTERVAL_TREE
 	select CHASH
+	select SND_HDA_COMPONENT if SND_HDA_CORE
 	help
 	  Choose this option if you have a recent AMD Radeon graphics card.
 
================================================================

Other than that, feel free to take my r-b tag:
  Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Kazlauskas, Nicholas]

On 7/10/19 9:48 AM, Takashi Iwai wrote:
> On Tue, 09 Jul 2019 18:30:19 +0200,
> Nicholas Kazlauskas wrote:
>>
>> [Why]
>> The drm_audio_component can be used to give pin ELD notifications
>> directly to the sound driver. This fixes audio endpoints disappearing
>> due to missing unsolicited notifications.
>>
>> [How]
>> Send the notification via the audio component whenever we enable or
>> disable audio state on a stream. This matches what i915 does with
>> their drm_audio_component and what Takashi Iwai's proposed hack for
>> radeon/amdpgu did.
>>
>> This is a bit delayed in when the notification actually occurs, however.
>> We wait until after all the programming is complete rather than sending
>> the notification mid sequence.
>>
>> Particular care is needed for the get ELD callback since it can happen
>> outside the locking and fencing DRM does for atomic commits.
>>
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Cc: Leo Li <sunpeng.li@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> Thanks for the patch, this has been a long-standing TODO for me, too!
> 
> Do you have the patch for HD-audio part as well?  Or you tested with
> my old patch?  Then I'll resurrect my patch set as well.

I've tested this series with and without that patch. The notifications 
work and the driver will query back with get_eld as expected.

Without the patch it should just retain the existing behavior.

> 
> This patch itself looks almost good.  One caveat is that you might
> want the reverse select of CONFIG_SND_HDA_COMPONENT, something like:

I can try adding this select based on DC for now (since it's the only 
part that actually uses it at the moment).

> 
> ================================================================
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -223,6 +223,7 @@ config DRM_AMDGPU
>   	select BACKLIGHT_CLASS_DEVICE
>   	select INTERVAL_TREE
>   	select CHASH
> +	select SND_HDA_COMPONENT if SND_HDA_CORE
>   	help
>   	  Choose this option if you have a recent AMD Radeon graphics card.
>   
> ================================================================
> 
> Other than that, feel free to take my r-b tag:
>    Reviewed-by: Takashi Iwai <tiwai@suse.de>

Thanks for the review!

Just to note, audio is currently broken after hotplug at the moment for 
AMDGPU with DC enabled, but this is unrelated to this particular series. 
I have a patch that resolves that issue that should show up sometime 
soon on amdgfx.

Nicholas Kazlauskas

> 
> 
> thanks,
> 
> Takashi
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Takashi Iwai]

On Wed, 10 Jul 2019 16:31:40 +0200,
Kazlauskas, Nicholas wrote:
> 
> On 7/10/19 9:48 AM, Takashi Iwai wrote:
> > On Tue, 09 Jul 2019 18:30:19 +0200,
> > Nicholas Kazlauskas wrote:
> >>
> >> [Why]
> >> The drm_audio_component can be used to give pin ELD notifications
> >> directly to the sound driver. This fixes audio endpoints disappearing
> >> due to missing unsolicited notifications.
> >>
> >> [How]
> >> Send the notification via the audio component whenever we enable or
> >> disable audio state on a stream. This matches what i915 does with
> >> their drm_audio_component and what Takashi Iwai's proposed hack for
> >> radeon/amdpgu did.
> >>
> >> This is a bit delayed in when the notification actually occurs, however.
> >> We wait until after all the programming is complete rather than sending
> >> the notification mid sequence.
> >>
> >> Particular care is needed for the get ELD callback since it can happen
> >> outside the locking and fencing DRM does for atomic commits.
> >>
> >> Cc: Takashi Iwai <tiwai@suse.de>
> >> Cc: Leo Li <sunpeng.li@amd.com>
> >> Cc: Harry Wentland <harry.wentland@amd.com>
> >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > 
> > Thanks for the patch, this has been a long-standing TODO for me, too!
> > 
> > Do you have the patch for HD-audio part as well?  Or you tested with
> > my old patch?  Then I'll resurrect my patch set as well.
> 
> I've tested this series with and without that patch. The notifications 
> work and the driver will query back with get_eld as expected.

OK, then I'm going to prepare a branch including that patch.

> Without the patch it should just retain the existing behavior.
> 
> > This patch itself looks almost good.  One caveat is that you might
> > want the reverse select of CONFIG_SND_HDA_COMPONENT, something like:
> 
> I can try adding this select based on DC for now (since it's the only 
> part that actually uses it at the moment).

Right, that's better.

> Just to note, audio is currently broken after hotplug at the moment for 
> AMDGPU with DC enabled, but this is unrelated to this particular series. 
> I have a patch that resolves that issue that should show up sometime 
> soon on amdgfx.

Thanks for the information!


Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Takashi Iwai]

On Wed, 10 Jul 2019 16:36:19 +0200,
Takashi Iwai wrote:
> 
> On Wed, 10 Jul 2019 16:31:40 +0200,
> Kazlauskas, Nicholas wrote:
> > 
> > On 7/10/19 9:48 AM, Takashi Iwai wrote:
> > > On Tue, 09 Jul 2019 18:30:19 +0200,
> > > Nicholas Kazlauskas wrote:
> > >>
> > >> [Why]
> > >> The drm_audio_component can be used to give pin ELD notifications
> > >> directly to the sound driver. This fixes audio endpoints disappearing
> > >> due to missing unsolicited notifications.
> > >>
> > >> [How]
> > >> Send the notification via the audio component whenever we enable or
> > >> disable audio state on a stream. This matches what i915 does with
> > >> their drm_audio_component and what Takashi Iwai's proposed hack for
> > >> radeon/amdpgu did.
> > >>
> > >> This is a bit delayed in when the notification actually occurs, however.
> > >> We wait until after all the programming is complete rather than sending
> > >> the notification mid sequence.
> > >>
> > >> Particular care is needed for the get ELD callback since it can happen
> > >> outside the locking and fencing DRM does for atomic commits.
> > >>
> > >> Cc: Takashi Iwai <tiwai@suse.de>
> > >> Cc: Leo Li <sunpeng.li@amd.com>
> > >> Cc: Harry Wentland <harry.wentland@amd.com>
> > >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > > 
> > > Thanks for the patch, this has been a long-standing TODO for me, too!
> > > 
> > > Do you have the patch for HD-audio part as well?  Or you tested with
> > > my old patch?  Then I'll resurrect my patch set as well.
> > 
> > I've tested this series with and without that patch. The notifications 
> > work and the driver will query back with get_eld as expected.
> 
> OK, then I'm going to prepare a branch including that patch.

FWIW, my latest patches are found in topic/hda-acomp branch on sound
git tree:
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git topic/hda-acomp

There have been a slight refactoring in HD-audio side, and now it
supports both AMD and Nvidia audio component support.  The branch
contains the patches for radeon and nouveau support.

Note that the branch isn't permanent one, it'll be rebased later.
Once after I submit and get the review for the HD-audio patches, I'll
create an immutable branch based on 5.3-rc1 containing those HD-audio
patches (but not DRM ones), so that you can pull into your drm tree
for the completeness.


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/display: Add drm_audio_component support to amdgpu_dm

[Takashi Iwai]

On Wed, 17 Jul 2019 15:32:55 +0200,
Takashi Iwai wrote:
> 
> On Wed, 10 Jul 2019 16:36:19 +0200,
> Takashi Iwai wrote:
> > 
> > On Wed, 10 Jul 2019 16:31:40 +0200,
> > Kazlauskas, Nicholas wrote:
> > > 
> > > On 7/10/19 9:48 AM, Takashi Iwai wrote:
> > > > On Tue, 09 Jul 2019 18:30:19 +0200,
> > > > Nicholas Kazlauskas wrote:
> > > >>
> > > >> [Why]
> > > >> The drm_audio_component can be used to give pin ELD notifications
> > > >> directly to the sound driver. This fixes audio endpoints disappearing
> > > >> due to missing unsolicited notifications.
> > > >>
> > > >> [How]
> > > >> Send the notification via the audio component whenever we enable or
> > > >> disable audio state on a stream. This matches what i915 does with
> > > >> their drm_audio_component and what Takashi Iwai's proposed hack for
> > > >> radeon/amdpgu did.
> > > >>
> > > >> This is a bit delayed in when the notification actually occurs, however.
> > > >> We wait until after all the programming is complete rather than sending
> > > >> the notification mid sequence.
> > > >>
> > > >> Particular care is needed for the get ELD callback since it can happen
> > > >> outside the locking and fencing DRM does for atomic commits.
> > > >>
> > > >> Cc: Takashi Iwai <tiwai@suse.de>
> > > >> Cc: Leo Li <sunpeng.li@amd.com>
> > > >> Cc: Harry Wentland <harry.wentland@amd.com>
> > > >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > > > 
> > > > Thanks for the patch, this has been a long-standing TODO for me, too!
> > > > 
> > > > Do you have the patch for HD-audio part as well?  Or you tested with
> > > > my old patch?  Then I'll resurrect my patch set as well.
> > > 
> > > I've tested this series with and without that patch. The notifications 
> > > work and the driver will query back with get_eld as expected.
> > 
> > OK, then I'm going to prepare a branch including that patch.
> 
> FWIW, my latest patches are found in topic/hda-acomp branch on sound
> git tree:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git topic/hda-acomp
> 
> There have been a slight refactoring in HD-audio side, and now it
> supports both AMD and Nvidia audio component support.  The branch
> contains the patches for radeon and nouveau support.
> 
> Note that the branch isn't permanent one, it'll be rebased later.
> Once after I submit and get the review for the HD-audio patches, I'll
> create an immutable branch based on 5.3-rc1 containing those HD-audio
> patches (but not DRM ones), so that you can pull into your drm tree
> for the completeness.

BTW, one thing I forgot to add: i915 audio component recently added
the device-link for assuring the PM call order.  More specifically, in
the component bind and unbind callbacks, device_link_add() and
device_link_del() get called respectively.  I guess the similar calls
are needed for AMDGPU driver as well.


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx