[PATCH 0/4] drm_audio_component support for AMD/ATI HDMI codecs

[PATCH 0/4] drm_audio_component support for AMD/ATI HDMI codecs

[Takashi Iwai]

Hi,

this is a patch set to add the support of drm_audio_component for
AMD/ATI HDMI codecs.  With the drm_audio_component, the HDMI/DP audio
hotplug and ELD read-out can be achieved directly without the hardware
access.  The best point by that is that it makes the hotplug
notification working even during runtime suspend.

The support is totally optional and dynamic, hence it still works even
if either HD-audio or DRM side isn't patched, and it'll fall back to
the existing method.

The current patch supports only radeon and a part of amdgpu; the DC
support isn't included yet.


Takashi

===

Takashi Iwai (4):
  ALSA: hda/hdmi: Use single mutex unlock in error paths
  ALSA: hda/hdmi: Allow audio component for AMD/ATI HDMI
  drm/radeon: Add audio component support
  drm/amdgpu: Add audio component support

 drivers/gpu/drm/Kconfig                   |   2 +
 drivers/gpu/drm/amd/amdgpu/Makefile       |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |   4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c |  97 ++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   3 +
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   6 +
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   6 +
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   6 +
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   6 +
 drivers/gpu/drm/radeon/radeon.h           |   3 +
 drivers/gpu/drm/radeon/radeon_audio.c     |  79 ++++++++
 sound/pci/hda/patch_hdmi.c                | 209 +++++++++++++++++-----
 12 files changed, 374 insertions(+), 49 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c

-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] ALSA: hda/hdmi: Use single mutex unlock in error paths

[Takashi Iwai]

Instead of calling mutex_unlock() at each error path multiple times,
take the standard goto-and-a-single-unlock approach.  This will
simplify the code and make easier to find the unbalanced mutex locks.

No functional changes, but only the code readability improvement as a
preliminary work for further changes.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_hdmi.c | 67 +++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 1de5491fb9bf..6750318d1c82 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -339,13 +339,13 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
 	if (!per_pin) {
 		/* no pin is bound to the pcm */
 		uinfo->count = 0;
-		mutex_unlock(&spec->pcm_lock);
-		return 0;
+		goto unlock;
 	}
 	eld = &per_pin->sink_eld;
 	uinfo->count = eld->eld_valid ? eld->eld_size : 0;
-	mutex_unlock(&spec->pcm_lock);
 
+ unlock:
+	mutex_unlock(&spec->pcm_lock);
 	return 0;
 }
 
@@ -357,6 +357,7 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
 	struct hdmi_spec_per_pin *per_pin;
 	struct hdmi_eld *eld;
 	int pcm_idx;
+	int err = 0;
 
 	pcm_idx = kcontrol->private_value;
 	mutex_lock(&spec->pcm_lock);
@@ -365,16 +366,15 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
 		/* no pin is bound to the pcm */
 		memset(ucontrol->value.bytes.data, 0,
 		       ARRAY_SIZE(ucontrol->value.bytes.data));
-		mutex_unlock(&spec->pcm_lock);
-		return 0;
+		goto unlock;
 	}
-	eld = &per_pin->sink_eld;
 
+	eld = &per_pin->sink_eld;
 	if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data) ||
 	    eld->eld_size > ELD_MAX_SIZE) {
-		mutex_unlock(&spec->pcm_lock);
 		snd_BUG();
-		return -EINVAL;
+		err = -EINVAL;
+		goto unlock;
 	}
 
 	memset(ucontrol->value.bytes.data, 0,
@@ -382,9 +382,10 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
 	if (eld->eld_valid)
 		memcpy(ucontrol->value.bytes.data, eld->eld_buffer,
 		       eld->eld_size);
-	mutex_unlock(&spec->pcm_lock);
 
-	return 0;
+ unlock:
+	mutex_unlock(&spec->pcm_lock);
+	return err;
 }
 
 static const struct snd_kcontrol_new eld_bytes_ctl = {
@@ -1209,8 +1210,8 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 	pin_idx = hinfo_to_pin_index(codec, hinfo);
 	if (!spec->dyn_pcm_assign) {
 		if (snd_BUG_ON(pin_idx < 0)) {
-			mutex_unlock(&spec->pcm_lock);
-			return -EINVAL;
+			err = -EINVAL;
+			goto unlock;
 		}
 	} else {
 		/* no pin is assigned to the PCM
@@ -1218,16 +1219,13 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 		 */
 		if (pin_idx < 0) {
 			err = hdmi_pcm_open_no_pin(hinfo, codec, substream);
-			mutex_unlock(&spec->pcm_lock);
-			return err;
+			goto unlock;
 		}
 	}
 
 	err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx);
-	if (err < 0) {
-		mutex_unlock(&spec->pcm_lock);
-		return err;
-	}
+	if (err < 0)
+		goto unlock;
 
 	per_cvt = get_cvt(spec, cvt_idx);
 	/* Claim converter */
@@ -1264,12 +1262,11 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 			per_cvt->assigned = 0;
 			hinfo->nid = 0;
 			snd_hda_spdif_ctls_unassign(codec, pcm_idx);
-			mutex_unlock(&spec->pcm_lock);
-			return -ENODEV;
+			err = -ENODEV;
+			goto unlock;
 		}
 	}
 
-	mutex_unlock(&spec->pcm_lock);
 	/* Store the updated parameters */
 	runtime->hw.channels_min = hinfo->channels_min;
 	runtime->hw.channels_max = hinfo->channels_max;
@@ -1278,7 +1275,9 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 
 	snd_pcm_hw_constraint_step(substream->runtime, 0,
 				   SNDRV_PCM_HW_PARAM_CHANNELS, 2);
-	return 0;
+ unlock:
+	mutex_unlock(&spec->pcm_lock);
+	return err;
 }
 
 /*
@@ -1867,7 +1866,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	bool non_pcm;
 	int pinctl;
-	int err;
+	int err = 0;
 
 	mutex_lock(&spec->pcm_lock);
 	pin_idx = hinfo_to_pin_index(codec, hinfo);
@@ -1879,13 +1878,12 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 		pin_cvt_fixup(codec, NULL, cvt_nid);
 		snd_hda_codec_setup_stream(codec, cvt_nid,
 					stream_tag, 0, format);
-		mutex_unlock(&spec->pcm_lock);
-		return 0;
+		goto unlock;
 	}
 
 	if (snd_BUG_ON(pin_idx < 0)) {
-		mutex_unlock(&spec->pcm_lock);
-		return -EINVAL;
+		err = -EINVAL;
+		goto unlock;
 	}
 	per_pin = get_pin(spec, pin_idx);
 	pin_nid = per_pin->pin_nid;
@@ -1924,6 +1922,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 	/* snd_hda_set_dev_select() has been called before */
 	err = spec->ops.setup_stream(codec, cvt_nid, pin_nid,
 				 stream_tag, format);
+ unlock:
 	mutex_unlock(&spec->pcm_lock);
 	return err;
 }
@@ -1945,6 +1944,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 	struct hdmi_spec_per_cvt *per_cvt;
 	struct hdmi_spec_per_pin *per_pin;
 	int pinctl;
+	int err = 0;
 
 	if (hinfo->nid) {
 		pcm_idx = hinfo_to_pcm_index(codec, hinfo);
@@ -1963,14 +1963,12 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 		snd_hda_spdif_ctls_unassign(codec, pcm_idx);
 		clear_bit(pcm_idx, &spec->pcm_in_use);
 		pin_idx = hinfo_to_pin_index(codec, hinfo);
-		if (spec->dyn_pcm_assign && pin_idx < 0) {
-			mutex_unlock(&spec->pcm_lock);
-			return 0;
-		}
+		if (spec->dyn_pcm_assign && pin_idx < 0)
+			goto unlock;
 
 		if (snd_BUG_ON(pin_idx < 0)) {
-			mutex_unlock(&spec->pcm_lock);
-			return -EINVAL;
+			err = -EINVAL;
+			goto unlock;
 		}
 		per_pin = get_pin(spec, pin_idx);
 
@@ -1989,10 +1987,11 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 		per_pin->setup = false;
 		per_pin->channels = 0;
 		mutex_unlock(&per_pin->lock);
+	unlock:
 		mutex_unlock(&spec->pcm_lock);
 	}
 
-	return 0;
+	return err;
 }
 
 static const struct hda_pcm_ops generic_ops = {
-- 
2.18.0

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

[PATCH 2/4] ALSA: hda/hdmi: Allow audio component for AMD/ATI HDMI

[Takashi Iwai]

AMD/ATI HDMI codec drivers didn't have the audio component binding
like i915, but it worked only with the traditional HD-audio
unsolicited event for the HDMI hotplug detection and the ELD read-up
thereafter.  This has been a problem in many ways: first of all, it
goes through the hardware event transition (from GPU register write,
HD-audio controller trigger, and finally to HD-audio unsolicited event
handling), which is often unreliable and may miss some opportunities.
Second, each unsol event handling and ELD read-up need the explicit
power up / down when the codec is in the runtime suspend.  Last but
not least, which is the most important, the hotplug wakeup may be
missed when the HD-audio controller is in runtime suspend.  Especially
the last point is a big problem due to the recent change relevant with
vga_switcheroo that forcibly enables the runtime PM for AMD HDMI
controllers.

These issues are solved by introducing the audio component; the
hotplug notification is done by a direct function callback, which is
more accurate and reliable, and it can be processed without the actual
hardware access, i.e. no runtime PM trigger is needed, and the
HD-audio gets the event even if it's in runtime suspend.  The same for
ELD query, as it's read directly from the cached ELD bytes stored in
the DRM driver, hence the whole hardware access can be skipped.

So here it is: this patch implements the audio component binding with
AMD/ATI DRM driver.  The biggest difference from i915 implementation
is that this binding is fully optional and it can be enabled
asynchronously on the fly.  That is, the driver will switch from the
HD-audio unsolicited event to the notify callback once when the DRM
component gets bound. Similarly, when DRM driver gets unloaded, the
HDMI event handling returns to the legacy mode, too.

Also, another difference from i915 is that AMD HDMI registers the
component in the codec driver, while i915 HDMI codec assumes the
component binding was already done.  Hence AMD code does de-register
the component binding at the codec exit, too.

Some other details:
- The match component ops assumes that both VGA and HD-audio
  controller PCI entries belong to the same PCI bus, and only accepts
  such an entry.

- The pin2port audio_ops is implemented with assumption of the widget
  NID starting from 3, with step 2 (3, 5, 7, ...).

As of this patch, the DRM component in radeon and amdgpu DRM driver
sides isn't implemented yet, so this change won't give any benefit
alone.  By the following changes in DRM sides, the mission will be
completed.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_hdmi.c | 142 +++++++++++++++++++++++++++++++++----
 1 file changed, 128 insertions(+), 14 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 6750318d1c82..298d40876201 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -31,6 +31,7 @@
 
 #include <linux/init.h>
 #include <linux/delay.h>
+#include <linux/pci.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
@@ -131,6 +132,7 @@ struct hdmi_pcm {
 };
 
 struct hdmi_spec {
+	struct hda_codec *codec;
 	int num_cvts;
 	struct snd_array cvts; /* struct hdmi_spec_per_cvt */
 	hda_nid_t cvt_nids[4]; /* only for haswell fix */
@@ -175,8 +177,9 @@ struct hdmi_spec {
 	struct hda_multi_out multiout;
 	struct hda_pcm_stream pcm_playback;
 
-	/* i915/powerwell (Haswell+/Valleyview+) specific */
-	bool use_acomp_notifier; /* use i915 eld_notify callback for hotplug */
+	bool use_jack_detect; /* jack detection enabled */
+	bool use_acomp_notifier; /* use eld_notify callback for hotplug */
+	bool acomp_registered; /* audio component registered in this driver */
 	struct drm_audio_component_audio_ops drm_audio_ops;
 
 	struct hdac_chmap chmap;
@@ -775,6 +778,10 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid,
 static void jack_callback(struct hda_codec *codec,
 			  struct hda_jack_callback *jack)
 {
+	/* stop polling when notification is enabled */
+	if (codec_has_acomp(codec))
+		return;
+
 	/* hda_jack don't support DP MST */
 	check_presence_and_report(codec, jack->nid, 0);
 }
@@ -833,6 +840,9 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res)
 	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
 	int subtag = (res & AC_UNSOL_RES_SUBTAG) >> AC_UNSOL_RES_SUBTAG_SHIFT;
 
+	if (codec_has_acomp(codec))
+		return;
+
 	if (!snd_hda_jack_tbl_get_from_tag(codec, tag)) {
 		codec_dbg(codec, "Unexpected HDMI event tag 0x%x\n", tag);
 		return;
@@ -1639,18 +1649,13 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 			snd_hda_power_down_pm(codec);
 			return false;
 		}
-	}
-
-	if (codec_has_acomp(codec)) {
+		ret = hdmi_present_sense_via_verbs(per_pin, repoll);
+		snd_hda_power_down_pm(codec);
+	} else {
 		sync_eld_via_acomp(codec, per_pin);
 		ret = false; /* don't call snd_hda_jack_report_sync() */
-	} else {
-		ret = hdmi_present_sense_via_verbs(per_pin, repoll);
 	}
 
-	if (!codec_has_acomp(codec))
-		snd_hda_power_down_pm(codec);
-
 	return ret;
 }
 
@@ -2242,6 +2247,8 @@ static int generic_hdmi_init(struct hda_codec *codec)
 	struct hdmi_spec *spec = codec->spec;
 	int pin_idx;
 
+	mutex_lock(&spec->pcm_lock);
+	spec->use_jack_detect = !codec->jackpoll_interval;
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
 		hda_nid_t pin_nid = per_pin->pin_nid;
@@ -2249,11 +2256,15 @@ static int generic_hdmi_init(struct hda_codec *codec)
 
 		snd_hda_set_dev_select(codec, pin_nid, dev_id);
 		hdmi_init_pin(codec, pin_nid);
-		if (!codec_has_acomp(codec))
+		if (codec_has_acomp(codec))
+			continue;
+		if (spec->use_jack_detect)
+			snd_hda_jack_detect_enable(codec, pin_nid);
+		else
 			snd_hda_jack_detect_enable_callback(codec, pin_nid,
-				codec->jackpoll_interval > 0 ?
-				jack_callback : NULL);
+							    jack_callback);
 	}
+	mutex_unlock(&spec->pcm_lock);
 	return 0;
 }
 
@@ -2286,7 +2297,9 @@ static void generic_hdmi_free(struct hda_codec *codec)
 	struct hdmi_spec *spec = codec->spec;
 	int pin_idx, pcm_idx;
 
-	if (codec_has_acomp(codec))
+	if (spec->acomp_registered)
+		snd_hdac_acomp_exit(&codec->bus->core);
+	else if (codec_has_acomp(codec))
 		snd_hdac_acomp_register_notifier(&codec->bus->core, NULL);
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
@@ -2352,6 +2365,7 @@ static int alloc_generic_hdmi(struct hda_codec *codec)
 	if (!spec)
 		return -ENOMEM;
 
+	spec->codec = codec;
 	spec->ops = generic_standard_hdmi_ops;
 	spec->dev_num = 1;	/* initialize to 1 */
 	mutex_init(&spec->pcm_lock);
@@ -3459,6 +3473,63 @@ static int patch_tegra_hdmi(struct hda_codec *codec)
 #define ATI_HBR_CAPABLE 0x01
 #define ATI_HBR_ENABLE 0x10
 
+/* turn on / off the unsol event jack detection dynamically */
+static void reprogram_jack_detect(struct hda_codec *codec, hda_nid_t nid,
+				  bool use_acomp)
+{
+	struct hda_jack_tbl *tbl;
+
+	tbl = snd_hda_jack_tbl_get(codec, nid);
+	if (tbl) {
+		/* clear unsol even if component notifier is used, or re-enable
+		 * if notifier is cleared
+		 */
+		unsigned int val = use_acomp ? 0 : (AC_USRSP_EN | tbl->tag);
+		snd_hda_codec_write_cache(codec, nid, 0,
+					  AC_VERB_SET_UNSOLICITED_ENABLE, val);
+	} else {
+		/* if no jack entry was defined beforehand, create a new one
+		 * at need (i.e. only when notifier is cleared)
+		 */
+		if (!use_acomp)
+			snd_hda_jack_detect_enable(codec, nid);
+	}
+}
+
+/* set up / clear component notifier dynamically */
+static void acomp_notifier_set(struct drm_audio_component *acomp,
+			       bool use_acomp)
+{
+	struct hdmi_spec *spec;
+	int i;
+
+	spec = container_of(acomp->audio_ops, struct hdmi_spec, drm_audio_ops);
+	mutex_lock(&spec->pcm_lock);
+	spec->use_acomp_notifier = use_acomp;
+	/* reprogram each jack detection logic depending on the notifier */
+	if (spec->use_jack_detect) {
+		for (i = 0; i < spec->num_pins; i++)
+			reprogram_jack_detect(spec->codec,
+					      get_pin(spec, i)->pin_nid,
+					      use_acomp);
+	}
+	mutex_unlock(&spec->pcm_lock);
+}
+
+/* enable / disable the notifier via master bind / unbind */
+static int atihdmi_master_bind(struct device *dev,
+			       struct drm_audio_component *acomp)
+{
+	acomp_notifier_set(acomp, true);
+	return 0;
+}
+
+static void atihdmi_master_unbind(struct device *dev,
+				  struct drm_audio_component *acomp)
+{
+	acomp_notifier_set(acomp, false);
+}
+
 static int atihdmi_pin_get_eld(struct hda_codec *codec, hda_nid_t nid,
 			   unsigned char *buf, int *eld_size)
 {
@@ -3731,6 +3802,39 @@ static int atihdmi_init(struct hda_codec *codec)
 	return 0;
 }
 
+/* check whether both HD-audio and DRM PCI devices belong to the same bus */
+static int match_bound_vga(struct device *dev, void *data)
+{
+	struct hdac_bus *bus = data;
+	struct pci_dev *pci, *master;
+
+	if (!dev_is_pci(dev) || !dev_is_pci(bus->dev))
+		return 0;
+	master = to_pci_dev(bus->dev);
+	pci = to_pci_dev(dev);
+	return master->bus == pci->bus;
+}
+
+/* map from pin NID to port; port is 0-based */
+static int atihdmi_pin2port(void *audio_ptr, int pin_nid)
+{
+	return pin_nid / 2 - 1;
+}
+
+/* audio component notifier for AMD codecs */
+static void atihdmi_pin_eld_notify(void *audio_ptr, int port, int dev_id)
+{
+	struct hda_codec *codec = audio_ptr;
+	hda_nid_t pin_nid = port * 2 + 3; /* 0-based port# */
+
+	/* treat notification only when system is already running and
+	 * not during PM suspend/resume process itself
+	 */
+	if (snd_power_get_state(codec->card) == SNDRV_CTL_POWER_D0 &&
+	    !atomic_read(&(codec)->core.in_pm))
+		check_presence_and_report(codec, pin_nid, dev_id);
+}
+
 static int patch_atihdmi(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec;
@@ -3779,6 +3883,16 @@ static int patch_atihdmi(struct hda_codec *codec)
 	 */
 	codec->link_down_at_suspend = 1;
 
+	/* set up audio component binding */
+	spec->drm_audio_ops.audio_ptr = codec;
+	spec->drm_audio_ops.pin2port = atihdmi_pin2port;
+	spec->drm_audio_ops.pin_eld_notify = atihdmi_pin_eld_notify;
+	spec->drm_audio_ops.master_bind = atihdmi_master_bind;
+	spec->drm_audio_ops.master_unbind = atihdmi_master_unbind;
+	if (!snd_hdac_acomp_init(&codec->bus->core, &spec->drm_audio_ops,
+				 match_bound_vga, 0))
+		spec->acomp_registered = true;
+
 	return 0;
 }
 
-- 
2.18.0

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

[PATCH 3/4] drm/radeon: Add audio component support

[Takashi Iwai]

This patch introduces the HDMI audio component binding like what i915
does to radeon driver.  Unlike i915, we need only the hotplug
notification and the ELD query, hence the code is relatively simple,
just adding the hook at radeon_audio_enable() and giving the eld query
by copying the connector->eld contents.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/Kconfig               |  1 +
 drivers/gpu/drm/radeon/radeon.h       |  3 +
 drivers/gpu/drm/radeon/radeon_audio.c | 79 +++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2a72d2feb76d..2c7112ddfed4 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -170,6 +170,7 @@ config DRM_RADEON
 	select BACKLIGHT_CLASS_DEVICE
 	select BACKLIGHT_LCD_SUPPORT
 	select INTERVAL_TREE
+	select SND_HDA_COMPONENT if SND_HDA_CORE
 	help
 	  Choose this option if you have an ATI Radeon graphics card.  There
 	  are both PCI and AGP versions.  You don't need to choose this to
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 4a2eb409aacc..fad58df0bf19 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -75,6 +75,7 @@
 #include <drm/ttm/ttm_execbuf_util.h>
 
 #include <drm/drm_gem.h>
+#include <drm/drm_audio_component.h>
 
 #include "radeon_family.h"
 #include "radeon_mode.h"
@@ -1760,6 +1761,8 @@ struct r600_audio {
 	struct radeon_audio_funcs *hdmi_funcs;
 	struct radeon_audio_funcs *dp_funcs;
 	struct radeon_audio_basic_funcs *funcs;
+	struct drm_audio_component *component;
+	bool component_registered;
 };
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 770e31f5fd1b..58e149b7a7a4 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -23,6 +23,7 @@
  */
 
 #include <linux/gcd.h>
+#include <linux/component.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include "radeon.h"
@@ -248,6 +249,7 @@ static void radeon_audio_enable(struct radeon_device *rdev,
 	struct drm_encoder *encoder;
 	struct radeon_encoder *radeon_encoder;
 	struct radeon_encoder_atom_dig *dig;
+	struct drm_audio_component *acomp = rdev->audio.component;
 	int pin_count = 0;
 
 	if (!pin)
@@ -269,6 +271,10 @@ static void radeon_audio_enable(struct radeon_device *rdev,
 
 	if (rdev->audio.funcs->enable)
 		rdev->audio.funcs->enable(rdev, pin, enable_mask);
+
+	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+						 pin->id, -1);
 }
 
 static void radeon_audio_interface_init(struct radeon_device *rdev)
@@ -292,6 +298,71 @@ static void radeon_audio_interface_init(struct radeon_device *rdev)
 	}
 }
 
+static int radeon_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 drm_encoder *encoder;
+	struct radeon_encoder *radeon_encoder;
+	struct radeon_encoder_atom_dig *dig;
+	struct drm_connector *connector;
+	int ret = 0;
+
+	*enabled = 0;
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+		if (!radeon_encoder_is_digital(encoder))
+			continue;
+		radeon_encoder = to_radeon_encoder(encoder);
+		dig = radeon_encoder->enc_priv;
+		if (!dig->pin || dig->pin->id != port)
+			continue;
+		connector = radeon_get_connector_for_encoder(encoder);
+		*enabled = !!connector;
+		if (connector) {
+			ret = drm_eld_size(connector->eld);
+			memcpy(buf, connector->eld, min(max_bytes, ret));
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static const struct drm_audio_component_ops radeon_audio_component_ops = {
+	.get_eld = radeon_audio_component_get_eld,
+};
+
+static int radeon_audio_component_bind(struct device *kdev,
+				       struct device *hda_kdev, void *data)
+{
+	struct drm_device *dev = dev_get_drvdata(kdev);
+	struct radeon_device *rdev = dev->dev_private;
+	struct drm_audio_component *acomp = data;
+
+	acomp->ops = &radeon_audio_component_ops;
+	acomp->dev = kdev;
+	rdev->audio.component = acomp;
+	return 0;
+}
+
+static void radeon_audio_component_unbind(struct device *kdev,
+					  struct device *hda_kdev, void *data)
+{
+	struct drm_device *dev = dev_get_drvdata(kdev);
+	struct radeon_device *rdev = dev->dev_private;
+	struct drm_audio_component *acomp = data;
+
+	acomp->ops = NULL;
+	acomp->dev = NULL;
+	rdev->audio.component = NULL;
+}
+
+static const struct component_ops radeon_audio_component_bind_ops = {
+	.bind	= radeon_audio_component_bind,
+	.unbind	= radeon_audio_component_unbind,
+};
+
 static int radeon_audio_chipset_supported(struct radeon_device *rdev)
 {
 	return ASIC_IS_DCE2(rdev) && !ASIC_IS_NODCE(rdev);
@@ -338,6 +409,9 @@ int radeon_audio_init(struct radeon_device *rdev)
 	for (i = 0; i < rdev->audio.num_pins; i++)
 		radeon_audio_enable(rdev, &rdev->audio.pin[i], 0);
 
+	if (!component_add(rdev->dev, &radeon_audio_component_bind_ops))
+		rdev->audio.component_registered = true;
+
 	return 0;
 }
 
@@ -490,6 +564,11 @@ void radeon_audio_fini(struct radeon_device *rdev)
 		radeon_audio_enable(rdev, &rdev->audio.pin[i], 0);
 
 	rdev->audio.enabled = false;
+
+	if (rdev->audio.component_registered) {
+		component_del(rdev->dev, &radeon_audio_component_bind_ops);
+		rdev->audio.component_registered = false;
+	}
 }
 
 static void radeon_audio_set_dto(struct drm_encoder *encoder, unsigned int clock)
-- 
2.18.0

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

[PATCH 4/4] drm/amdgpu: Add audio component support

[Takashi Iwai]

This patch introduces the HDMI audio component binding like what i915
does to amdgpu driver.  Unlike i915, we need only the hotplug
notification and the ELD query, hence the code is relatively simple,
just adding the hook at each *_audio_enable() call and giving the eld
query by copying the connector->eld contents.

This patch currently doesn't contain the component support for the new
DC codes yet.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/Kconfig                   |  1 +
 drivers/gpu/drm/amd/amdgpu/Makefile       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c | 97 +++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  3 +
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  6 ++
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  6 ++
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  6 ++
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  6 ++
 9 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2c7112ddfed4..fbe7216c5c56 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -193,6 +193,7 @@ config DRM_AMDGPU
 	select BACKLIGHT_LCD_SUPPORT
 	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.
 
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index bfd332c95b61..9c26facddb17 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -52,7 +52,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
 	amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_atomfirmware.o \
 	amdgpu_queue_mgr.o amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o \
-	amdgpu_ids.o
+	amdgpu_ids.o amdgpu_audio.o
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a59c07590cee..203d2584c989 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1957,5 +1957,9 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev );
 static inline int amdgpu_dm_display_resume(struct amdgpu_device *adev) { return 0; }
 #endif
 
+int amdgpu_audio_component_init(struct amdgpu_device *adev);
+void amdgpu_audio_component_fini(struct amdgpu_device *adev);
+void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin);
+
 #include "amdgpu_object.h"
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
new file mode 100644
index 000000000000..39256e2f84b3
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: MIT
+
+#include <linux/component.h>
+#include "amdgpu.h"
+
+static int amdgpu_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 drm_encoder *encoder;
+	struct amdgpu_encoder *amdgpu_encoder;
+	struct amdgpu_encoder_atom_dig *dig;
+	struct drm_connector *connector;
+	int ret = 0;
+
+	*enabled = 0;
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+		amdgpu_encoder = to_amdgpu_encoder(encoder);
+		dig = amdgpu_encoder->enc_priv;
+		if (!dig || !dig->afmt || !dig->afmt->enabled)
+			continue;
+		if (!dig->afmt->pin || dig->afmt->pin->id != port)
+			continue;
+		connector = amdgpu_get_connector_for_encoder(encoder);
+		*enabled = !!connector;
+		if (connector) {
+			ret = drm_eld_size(connector->eld);
+			memcpy(buf, connector->eld, min(max_bytes, ret));
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static const struct drm_audio_component_ops amdgpu_audio_component_ops = {
+	.get_eld = amdgpu_audio_component_get_eld,
+};
+
+static int amdgpu_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_audio_component_ops;
+	acomp->dev = kdev;
+	adev->mode_info.audio.component = acomp;
+	return 0;
+}
+
+static void amdgpu_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->mode_info.audio.component = NULL;
+}
+
+static const struct component_ops amdgpu_audio_component_bind_ops = {
+	.bind	= amdgpu_audio_component_bind,
+	.unbind	= amdgpu_audio_component_unbind,
+};
+
+int amdgpu_audio_component_init(struct amdgpu_device *adev)
+{
+	int err;
+
+	err = component_add(adev->dev, &amdgpu_audio_component_bind_ops);
+	if (err < 0)
+		return err;
+	adev->mode_info.audio.component_registered = true;
+	return 0;
+}
+
+void amdgpu_audio_component_fini(struct amdgpu_device *adev)
+{
+	if (adev->mode_info.audio.component_registered) {
+		component_del(adev->dev, &amdgpu_audio_component_bind_ops);
+		adev->mode_info.audio.component_registered = false;
+	}
+}
+
+void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin)
+{
+	struct drm_audio_component *acomp = adev->mode_info.audio.component;
+
+	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+						 pin, -1);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..7cb1aa97e522 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -39,6 +39,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_audio_component.h>
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
 #include <linux/hrtimer.h>
@@ -260,6 +261,8 @@ struct amdgpu_audio {
 	bool enabled;
 	struct amdgpu_audio_pin pin[AMDGPU_MAX_AFMT_BLOCKS];
 	int num_pins;
+	bool component_registered;
+	struct drm_audio_component *component;
 };
 
 struct amdgpu_display_funcs {
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index ada241bfeee9..c8471a59930f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1392,6 +1392,8 @@ static void dce_v10_0_audio_enable(struct amdgpu_device *adev,
 
 	WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
 			   enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+	amdgpu_audio_eld_notify(adev, pin->id);
 }
 
 static const u32 pin_offsets[] =
@@ -1430,6 +1432,8 @@ static int dce_v10_0_audio_init(struct amdgpu_device *adev)
 		dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 	}
 
+	amdgpu_audio_component_init(adev);
+
 	return 0;
 }
 
@@ -1443,6 +1447,8 @@ static void dce_v10_0_audio_fini(struct amdgpu_device *adev)
 	if (!adev->mode_info.audio.enabled)
 		return;
 
+	amdgpu_audio_component_fini(adev);
+
 	for (i = 0; i < adev->mode_info.audio.num_pins; i++)
 		dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index a5b96eac3033..49edb74725a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1418,6 +1418,8 @@ static void dce_v11_0_audio_enable(struct amdgpu_device *adev,
 
 	WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
 			   enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+	amdgpu_audio_eld_notify(adev, pin->id);
 }
 
 static const u32 pin_offsets[] =
@@ -1472,6 +1474,8 @@ static int dce_v11_0_audio_init(struct amdgpu_device *adev)
 		dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 	}
 
+	amdgpu_audio_component_init(adev);
+
 	return 0;
 }
 
@@ -1485,6 +1489,8 @@ static void dce_v11_0_audio_fini(struct amdgpu_device *adev)
 	if (!adev->mode_info.audio.enabled)
 		return;
 
+	amdgpu_audio_component_fini(adev);
+
 	for (i = 0; i < adev->mode_info.audio.num_pins; i++)
 		dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 394cc1e8fe20..2b7468eaacca 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1297,6 +1297,8 @@ static void dce_v6_0_audio_enable(struct amdgpu_device *adev,
 
 	WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
 			enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+	amdgpu_audio_eld_notify(adev, pin->id);
 }
 
 static const u32 pin_offsets[7] =
@@ -1343,6 +1345,8 @@ static int dce_v6_0_audio_init(struct amdgpu_device *adev)
 		dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 	}
 
+	amdgpu_audio_component_init(adev);
+
 	return 0;
 }
 
@@ -1356,6 +1360,8 @@ static void dce_v6_0_audio_fini(struct amdgpu_device *adev)
 	if (!adev->mode_info.audio.enabled)
 		return;
 
+	amdgpu_audio_component_fini(adev);
+
 	for (i = 0; i < adev->mode_info.audio.num_pins; i++)
 		dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index c9b9ab8f1b05..c2370ea1f64f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1348,6 +1348,8 @@ static void dce_v8_0_audio_enable(struct amdgpu_device *adev,
 
 	WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
 		enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+	amdgpu_audio_eld_notify(adev, pin->id);
 }
 
 static const u32 pin_offsets[7] =
@@ -1395,6 +1397,8 @@ static int dce_v8_0_audio_init(struct amdgpu_device *adev)
 		dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 	}
 
+	amdgpu_audio_component_init(adev);
+
 	return 0;
 }
 
@@ -1408,6 +1412,8 @@ static void dce_v8_0_audio_fini(struct amdgpu_device *adev)
 	if (!adev->mode_info.audio.enabled)
 		return;
 
+	amdgpu_audio_component_fini(adev);
+
 	for (i = 0; i < adev->mode_info.audio.num_pins; i++)
 		dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
 
-- 
2.18.0

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

Re: [PATCH 0/4] drm_audio_component support for AMD/ATI HDMI codecs

[Alex Deucher]

On Mon, Jul 23, 2018 at 10:50 AM, Takashi Iwai <tiwai@suse.de> wrote:
> Hi,
>
> this is a patch set to add the support of drm_audio_component for
> AMD/ATI HDMI codecs.  With the drm_audio_component, the HDMI/DP audio
> hotplug and ELD read-out can be achieved directly without the hardware
> access.  The best point by that is that it makes the hotplug
> notification working even during runtime suspend.
>
> The support is totally optional and dynamic, hence it still works even
> if either HD-audio or DRM side isn't patched, and it'll fall back to
> the existing method.

I'm still getting my head around how the new callbacks work so bear
with me.  It seems like we'd want to set the ELD and report whether
the display is attached in when we detect the displays or fetch the
EDID rather than at modeset time when we enable the audio stream.

Alex


>
> The current patch supports only radeon and a part of amdgpu; the DC
> support isn't included yet.
>
>
> Takashi
>
> ===
>
> Takashi Iwai (4):
>   ALSA: hda/hdmi: Use single mutex unlock in error paths
>   ALSA: hda/hdmi: Allow audio component for AMD/ATI HDMI
>   drm/radeon: Add audio component support
>   drm/amdgpu: Add audio component support
>
>  drivers/gpu/drm/Kconfig                   |   2 +
>  drivers/gpu/drm/amd/amdgpu/Makefile       |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |   4 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c |  97 ++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   3 +
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   6 +
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   6 +
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   6 +
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   6 +
>  drivers/gpu/drm/radeon/radeon.h           |   3 +
>  drivers/gpu/drm/radeon/radeon_audio.c     |  79 ++++++++
>  sound/pci/hda/patch_hdmi.c                | 209 +++++++++++++++++-----
>  12 files changed, 374 insertions(+), 49 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>
> --
> 2.18.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/4] drm_audio_component support for AMD/ATI HDMI codecs

[Takashi Iwai]

On Mon, 23 Jul 2018 22:53:08 +0200,
Alex Deucher wrote:
> 
> On Mon, Jul 23, 2018 at 10:50 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > Hi,
> >
> > this is a patch set to add the support of drm_audio_component for
> > AMD/ATI HDMI codecs.  With the drm_audio_component, the HDMI/DP audio
> > hotplug and ELD read-out can be achieved directly without the hardware
> > access.  The best point by that is that it makes the hotplug
> > notification working even during runtime suspend.
> >
> > The support is totally optional and dynamic, hence it still works even
> > if either HD-audio or DRM side isn't patched, and it'll fall back to
> > the existing method.
> 
> I'm still getting my head around how the new callbacks work so bear
> with me.  It seems like we'd want to set the ELD and report whether
> the display is attached in when we detect the displays or fetch the
> EDID rather than at modeset time when we enable the audio stream.

Basically the callbacks are just replacements of the existing
mechanism with the direct calls.

In the traditional model, from GPU to HD-audio, we trigger a hotplug
event via writing a dedicated GPU register.  In HD-audio side, it's
transmitted as an unsolicited event via HD-audio bus, and HD-audio
driver receives it.  And for passing ELD, we write some bytes to GPU
registers in DRM driver.  These are read by HD-audio driver in hotplug
handler after receiving the event.

In the callback model, GPU calls audio_ops.pin_eld_notify() with the
pin index.  This callback is set by HD-audio.  Then HD-audio reads
back ELD bytes in return by calling ops.get_eld() with the given pin
index.  This callback is set by DRM.

For registration and de-registration, DRM gives the component bind /
unbind to set / clear its ops.


thanks,

Takashi

> 
> Alex
> 
> 
> >
> > The current patch supports only radeon and a part of amdgpu; the DC
> > support isn't included yet.
> >
> >
> > Takashi
> >
> > ===
> >
> > Takashi Iwai (4):
> >   ALSA: hda/hdmi: Use single mutex unlock in error paths
> >   ALSA: hda/hdmi: Allow audio component for AMD/ATI HDMI
> >   drm/radeon: Add audio component support
> >   drm/amdgpu: Add audio component support
> >
> >  drivers/gpu/drm/Kconfig                   |   2 +
> >  drivers/gpu/drm/amd/amdgpu/Makefile       |   2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |   4 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c |  97 ++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   3 +
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   6 +
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   6 +
> >  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   6 +
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   6 +
> >  drivers/gpu/drm/radeon/radeon.h           |   3 +
> >  drivers/gpu/drm/radeon/radeon_audio.c     |  79 ++++++++
> >  sound/pci/hda/patch_hdmi.c                | 209 +++++++++++++++++-----
> >  12 files changed, 374 insertions(+), 49 deletions(-)
> >  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
> >
> > --
> > 2.18.0
> >
> > _______________________________________________
> > 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

答复: [PATCH 3/4] drm/radeon: Add audio component support

[Qu, Jim]

Hi Takashi lwai,

Sorry, I have to use outlook again, since my mail client had some problems that did not receive these series patches.

See the commands in line.

Thanks
JimQu

________________________________________
发件人: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> 代表 Takashi Iwai <tiwai@suse.de>
发送时间: 2018年7月23日 22:50
收件人: alsa-devel@alsa-project.org
抄送: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
主题: [PATCH 3/4] drm/radeon: Add audio component support

This patch introduces the HDMI audio component binding like what i915
does to radeon driver.  Unlike i915, we need only the hotplug
notification and the ELD query, hence the code is relatively simple,
just adding the hook at radeon_audio_enable() and giving the eld query
by copying the connector->eld contents.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/Kconfig               |  1 +
 drivers/gpu/drm/radeon/radeon.h       |  3 +
 drivers/gpu/drm/radeon/radeon_audio.c | 79 +++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2a72d2feb76d..2c7112ddfed4 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -170,6 +170,7 @@ config DRM_RADEON
        select BACKLIGHT_CLASS_DEVICE
        select BACKLIGHT_LCD_SUPPORT
        select INTERVAL_TREE
+       select SND_HDA_COMPONENT if SND_HDA_CORE
        help
          Choose this option if you have an ATI Radeon graphics card.  There
          are both PCI and AGP versions.  You don't need to choose this to
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 4a2eb409aacc..fad58df0bf19 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -75,6 +75,7 @@
 #include <drm/ttm/ttm_execbuf_util.h>

 #include <drm/drm_gem.h>
+#include <drm/drm_audio_component.h>

 #include "radeon_family.h"
 #include "radeon_mode.h"
@@ -1760,6 +1761,8 @@ struct r600_audio {
        struct radeon_audio_funcs *hdmi_funcs;
        struct radeon_audio_funcs *dp_funcs;
        struct radeon_audio_basic_funcs *funcs;
+       struct drm_audio_component *component;
+       bool component_registered;
 };

 /*
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 770e31f5fd1b..58e149b7a7a4 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -23,6 +23,7 @@
  */

 #include <linux/gcd.h>
+#include <linux/component.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include "radeon.h"
@@ -248,6 +249,7 @@ static void radeon_audio_enable(struct radeon_device *rdev,
        struct drm_encoder *encoder;
        struct radeon_encoder *radeon_encoder;
        struct radeon_encoder_atom_dig *dig;
+       struct drm_audio_component *acomp = rdev->audio.component;
        int pin_count = 0;

        if (!pin)
@@ -269,6 +271,10 @@ static void radeon_audio_enable(struct radeon_device *rdev,

        if (rdev->audio.funcs->enable)
                rdev->audio.funcs->enable(rdev, pin, enable_mask);
+
+       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+                                                pin->id, -1);

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
Jim: radeon_audio_enable() can enable audios and also disable them, so eld noitfy callback should be called when enable_mask is true.

JimQu
Thanks
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

 }

 static void radeon_audio_interface_init(struct radeon_device *rdev)
@@ -292,6 +298,71 @@ static void radeon_audio_interface_init(struct radeon_device *rdev)
        }
 }

+static int radeon_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 drm_encoder *encoder;
+       struct radeon_encoder *radeon_encoder;
+       struct radeon_encoder_atom_dig *dig;
+       struct drm_connector *connector;
+       int ret = 0;
+
+       *enabled = 0;
+       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+               if (!radeon_encoder_is_digital(encoder))
+                       continue;
+               radeon_encoder = to_radeon_encoder(encoder);
+               dig = radeon_encoder->enc_priv;
+               if (!dig->pin || dig->pin->id != port)
+                       continue;
+               connector = radeon_get_connector_for_encoder(encoder);
+               *enabled = !!connector;
+               if (connector) {
+                       ret = drm_eld_size(connector->eld);
+                       memcpy(buf, connector->eld, min(max_bytes, ret));
+                       break;
+               }
+       }
+
+       return ret;
+}
+
+static const struct drm_audio_component_ops radeon_audio_component_ops = {
+       .get_eld = radeon_audio_component_get_eld,
+};
+
+static int radeon_audio_component_bind(struct device *kdev,
+                                      struct device *hda_kdev, void *data)
+{
+       struct drm_device *dev = dev_get_drvdata(kdev);
+       struct radeon_device *rdev = dev->dev_private;
+       struct drm_audio_component *acomp = data;
+
+       acomp->ops = &radeon_audio_component_ops;
+       acomp->dev = kdev;
+       rdev->audio.component = acomp;
+       return 0;
+}
+
+static void radeon_audio_component_unbind(struct device *kdev,
+                                         struct device *hda_kdev, void *data)
+{
+       struct drm_device *dev = dev_get_drvdata(kdev);
+       struct radeon_device *rdev = dev->dev_private;
+       struct drm_audio_component *acomp = data;
+
+       acomp->ops = NULL;
+       acomp->dev = NULL;
+       rdev->audio.component = NULL;
+}
+
+static const struct component_ops radeon_audio_component_bind_ops = {
+       .bind   = radeon_audio_component_bind,
+       .unbind = radeon_audio_component_unbind,
+};
+
 static int radeon_audio_chipset_supported(struct radeon_device *rdev)
 {
        return ASIC_IS_DCE2(rdev) && !ASIC_IS_NODCE(rdev);
@@ -338,6 +409,9 @@ int radeon_audio_init(struct radeon_device *rdev)
        for (i = 0; i < rdev->audio.num_pins; i++)
                radeon_audio_enable(rdev, &rdev->audio.pin[i], 0);

+       if (!component_add(rdev->dev, &radeon_audio_component_bind_ops))
+               rdev->audio.component_registered = true;
+
        return 0;
 }

@@ -490,6 +564,11 @@ void radeon_audio_fini(struct radeon_device *rdev)
                radeon_audio_enable(rdev, &rdev->audio.pin[i], 0);

        rdev->audio.enabled = false;
+
+       if (rdev->audio.component_registered) {
+               component_del(rdev->dev, &radeon_audio_component_bind_ops);
+               rdev->audio.component_registered = false;
+       }
 }

 static void radeon_audio_set_dto(struct drm_encoder *encoder, unsigned int clock)
--
2.18.0

_______________________________________________
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 3/4] drm/radeon: Add audio component support

[Takashi Iwai]

On Wed, 25 Jul 2018 05:32:52 +0200,
Qu, Jim wrote:
> @@ -269,6 +271,10 @@ static void radeon_audio_enable(struct radeon_device *rdev,
> 
>         if (rdev->audio.funcs->enable)
>                 rdev->audio.funcs->enable(rdev, pin, enable_mask);
> +
> +       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> +               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> +                                                pin->id, -1);
> 
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> Jim: radeon_audio_enable() can enable audios and also disable them, so eld noitfy callback should be called when enable_mask is true.

It's intentional.  The notifier needs to be called at disablement
time, too.  The audio driver has to follow the HDMI audio disablement,
and notifier receives both on and off case.

The actual state is inquired via get_eld call by HD-audio followed
after the notification.


thanks,

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

答复: [PATCH 4/4] drm/amdgpu: Add audio component support

[Qu, Jim]

See comments in line.

Thanks
JimQu

________________________________________
发件人: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> 代表 Takashi Iwai <tiwai@suse.de>
发送时间: 2018年7月23日 22:50
收件人: alsa-devel@alsa-project.org
抄送: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
主题: [PATCH 4/4] drm/amdgpu: Add audio component support

This patch introduces the HDMI audio component binding like what i915
does to amdgpu driver.  Unlike i915, we need only the hotplug
notification and the ELD query, hence the code is relatively simple,
just adding the hook at each *_audio_enable() call and giving the eld
query by copying the connector->eld contents.

This patch currently doesn't contain the component support for the new
DC codes yet.

Signed-off-by: Takashi Iwai <tiwai@suse.de>

**********************************************************************************************************
Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.

IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.

With None-DC driver, the detect() of connectors will to be preformed in drm_helper_hpd_irq_event() when there is device hotplug. However, amdgpu_connector_xxxx_detect() maybe need to be modified to fit your change.

Thanks
JimQu
************************************************************************************************************
---
 drivers/gpu/drm/Kconfig                   |  1 +
 drivers/gpu/drm/amd/amdgpu/Makefile       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c | 97 +++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  3 +
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  6 ++
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  6 ++
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  6 ++
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  6 ++
 9 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2c7112ddfed4..fbe7216c5c56 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -193,6 +193,7 @@ config DRM_AMDGPU
        select BACKLIGHT_LCD_SUPPORT
        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.

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index bfd332c95b61..9c26facddb17 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -52,7 +52,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
        amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
        amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_atomfirmware.o \
        amdgpu_queue_mgr.o amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o \
-       amdgpu_ids.o
+       amdgpu_ids.o amdgpu_audio.o

 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a59c07590cee..203d2584c989 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1957,5 +1957,9 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev );
 static inline int amdgpu_dm_display_resume(struct amdgpu_device *adev) { return 0; }
 #endif

+int amdgpu_audio_component_init(struct amdgpu_device *adev);
+void amdgpu_audio_component_fini(struct amdgpu_device *adev);
+void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin);
+
 #include "amdgpu_object.h"
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
new file mode 100644
index 000000000000..39256e2f84b3
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: MIT
+
+#include <linux/component.h>
+#include "amdgpu.h"
+
+static int amdgpu_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 drm_encoder *encoder;
+       struct amdgpu_encoder *amdgpu_encoder;
+       struct amdgpu_encoder_atom_dig *dig;
+       struct drm_connector *connector;
+       int ret = 0;
+
+       *enabled = 0;
+       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+               amdgpu_encoder = to_amdgpu_encoder(encoder);
+               dig = amdgpu_encoder->enc_priv;
+               if (!dig || !dig->afmt || !dig->afmt->enabled)
+                       continue;
+               if (!dig->afmt->pin || dig->afmt->pin->id != port)
+                       continue;
+               connector = amdgpu_get_connector_for_encoder(encoder);
+               *enabled = !!connector;
+               if (connector) {
+                       ret = drm_eld_size(connector->eld);
+                       memcpy(buf, connector->eld, min(max_bytes, ret));
+                       break;
+               }
+       }
+
+       return ret;
+}
+
+static const struct drm_audio_component_ops amdgpu_audio_component_ops = {
+       .get_eld = amdgpu_audio_component_get_eld,
+};
+
+static int amdgpu_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_audio_component_ops;
+       acomp->dev = kdev;
+       adev->mode_info.audio.component = acomp;
+       return 0;
+}
+
+static void amdgpu_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->mode_info.audio.component = NULL;
+}
+
+static const struct component_ops amdgpu_audio_component_bind_ops = {
+       .bind   = amdgpu_audio_component_bind,
+       .unbind = amdgpu_audio_component_unbind,
+};
+
+int amdgpu_audio_component_init(struct amdgpu_device *adev)
+{
+       int err;
+
+       err = component_add(adev->dev, &amdgpu_audio_component_bind_ops);
+       if (err < 0)
+               return err;
+       adev->mode_info.audio.component_registered = true;
+       return 0;
+}
+
+void amdgpu_audio_component_fini(struct amdgpu_device *adev)
+{
+       if (adev->mode_info.audio.component_registered) {
+               component_del(adev->dev, &amdgpu_audio_component_bind_ops);
+               adev->mode_info.audio.component_registered = false;
+       }
+}
+
+void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin)
+{
+       struct drm_audio_component *acomp = adev->mode_info.audio.component;
+
+       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+                                                pin, -1);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..7cb1aa97e522 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -39,6 +39,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_audio_component.h>
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
 #include <linux/hrtimer.h>
@@ -260,6 +261,8 @@ struct amdgpu_audio {
        bool enabled;
        struct amdgpu_audio_pin pin[AMDGPU_MAX_AFMT_BLOCKS];
        int num_pins;
+       bool component_registered;
+       struct drm_audio_component *component;
 };

 struct amdgpu_display_funcs {
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index ada241bfeee9..c8471a59930f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1392,6 +1392,8 @@ static void dce_v10_0_audio_enable(struct amdgpu_device *adev,

        WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
                           enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+       amdgpu_audio_eld_notify(adev, pin->id);
 }

 static const u32 pin_offsets[] =
@@ -1430,6 +1432,8 @@ static int dce_v10_0_audio_init(struct amdgpu_device *adev)
                dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
        }

+       amdgpu_audio_component_init(adev);
+
        return 0;
 }

@@ -1443,6 +1447,8 @@ static void dce_v10_0_audio_fini(struct amdgpu_device *adev)
        if (!adev->mode_info.audio.enabled)
                return;

+       amdgpu_audio_component_fini(adev);
+
        for (i = 0; i < adev->mode_info.audio.num_pins; i++)
                dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index a5b96eac3033..49edb74725a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1418,6 +1418,8 @@ static void dce_v11_0_audio_enable(struct amdgpu_device *adev,

        WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
                           enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+       amdgpu_audio_eld_notify(adev, pin->id);
 }

 static const u32 pin_offsets[] =
@@ -1472,6 +1474,8 @@ static int dce_v11_0_audio_init(struct amdgpu_device *adev)
                dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
        }

+       amdgpu_audio_component_init(adev);
+
        return 0;
 }

@@ -1485,6 +1489,8 @@ static void dce_v11_0_audio_fini(struct amdgpu_device *adev)
        if (!adev->mode_info.audio.enabled)
                return;

+       amdgpu_audio_component_fini(adev);
+
        for (i = 0; i < adev->mode_info.audio.num_pins; i++)
                dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 394cc1e8fe20..2b7468eaacca 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1297,6 +1297,8 @@ static void dce_v6_0_audio_enable(struct amdgpu_device *adev,

        WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
                        enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+       amdgpu_audio_eld_notify(adev, pin->id);
 }

 static const u32 pin_offsets[7] =
@@ -1343,6 +1345,8 @@ static int dce_v6_0_audio_init(struct amdgpu_device *adev)
                dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
        }

+       amdgpu_audio_component_init(adev);
+
        return 0;
 }

@@ -1356,6 +1360,8 @@ static void dce_v6_0_audio_fini(struct amdgpu_device *adev)
        if (!adev->mode_info.audio.enabled)
                return;

+       amdgpu_audio_component_fini(adev);
+
        for (i = 0; i < adev->mode_info.audio.num_pins; i++)
                dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index c9b9ab8f1b05..c2370ea1f64f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1348,6 +1348,8 @@ static void dce_v8_0_audio_enable(struct amdgpu_device *adev,

        WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
                enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
+
+       amdgpu_audio_eld_notify(adev, pin->id);
 }

 static const u32 pin_offsets[7] =
@@ -1395,6 +1397,8 @@ static int dce_v8_0_audio_init(struct amdgpu_device *adev)
                dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
        }

+       amdgpu_audio_component_init(adev);
+
        return 0;
 }

@@ -1408,6 +1412,8 @@ static void dce_v8_0_audio_fini(struct amdgpu_device *adev)
        if (!adev->mode_info.audio.enabled)
                return;

+       amdgpu_audio_component_fini(adev);
+
        for (i = 0; i < adev->mode_info.audio.num_pins; i++)
                dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);

--
2.18.0

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

Re: 答复: [PATCH 3/4] drm/radeon: Add audio component support

[jimqu]

On 2018年07月25日 13:28, Takashi Iwai wrote:
> On Wed, 25 Jul 2018 05:32:52 +0200,
> Qu, Jim wrote:
>> @@ -269,6 +271,10 @@ static void radeon_audio_enable(struct radeon_device *rdev,
>>
>>          if (rdev->audio.funcs->enable)
>>                  rdev->audio.funcs->enable(rdev, pin, enable_mask);
>> +
>> +       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
>> +               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
>> +                                                pin->id, -1);
>>
>> Jim: radeon_audio_enable() can enable audios and also disable them, so eld noitfy callback should be called when enable_mask is true.
> It's intentional.  The notifier needs to be called at disablement
> time, too.  The audio driver has to follow the HDMI audio disablement,
> and notifier receives both on and off case.
>
> The actual state is inquired via get_eld call by HD-audio followed
> after the notification.
>
>
> thanks,
>
> Takashi

OK, I got your point.

Thanks
JimQu


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

Re: 答复: [PATCH 4/4] drm/amdgpu: Add audio component support

[Takashi Iwai]

On Wed, 25 Jul 2018 07:38:37 +0200,
Qu, Jim wrote:
> 
> Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.
> 
> IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.

Hm, but by the modesetting, it actually enables / disables the audio
as well, no?  If so, the notifier is exactly for that purpose.  The
audio driver needs to know not only about the physical connection but
whether the audio can be actually driven.

That is, even if the monitor is connected, the audio won't come out if
the mode is off.  That is equivalent with the unplugged state for the
audio driver.

The i915 driver deals with the notifier just like the above, so the
behavior is intentional.


thanks,

Takashi

> With None-DC driver, the detect() of connectors will to be preformed in drm_helper_hpd_irq_event() when there is device hotplug. However, amdgpu_connector_xxxx_detect() maybe need to be modified to fit your change.
> 
> Thanks
> JimQu
> ************************************************************************************************************
> ---
>  drivers/gpu/drm/Kconfig                   |  1 +
>  drivers/gpu/drm/amd/amdgpu/Makefile       |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c | 97 +++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  3 +
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  6 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  6 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  6 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  6 ++
>  9 files changed, 130 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 2c7112ddfed4..fbe7216c5c56 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -193,6 +193,7 @@ config DRM_AMDGPU
>         select BACKLIGHT_LCD_SUPPORT
>         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.
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index bfd332c95b61..9c26facddb17 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -52,7 +52,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>         amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
>         amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_atomfirmware.o \
>         amdgpu_queue_mgr.o amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o \
> -       amdgpu_ids.o
> +       amdgpu_ids.o amdgpu_audio.o
> 
>  # add asic specific block
>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a59c07590cee..203d2584c989 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1957,5 +1957,9 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev );
>  static inline int amdgpu_dm_display_resume(struct amdgpu_device *adev) { return 0; }
>  #endif
> 
> +int amdgpu_audio_component_init(struct amdgpu_device *adev);
> +void amdgpu_audio_component_fini(struct amdgpu_device *adev);
> +void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin);
> +
>  #include "amdgpu_object.h"
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
> new file mode 100644
> index 000000000000..39256e2f84b3
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: MIT
> +
> +#include <linux/component.h>
> +#include "amdgpu.h"
> +
> +static int amdgpu_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 drm_encoder *encoder;
> +       struct amdgpu_encoder *amdgpu_encoder;
> +       struct amdgpu_encoder_atom_dig *dig;
> +       struct drm_connector *connector;
> +       int ret = 0;
> +
> +       *enabled = 0;
> +       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
> +               amdgpu_encoder = to_amdgpu_encoder(encoder);
> +               dig = amdgpu_encoder->enc_priv;
> +               if (!dig || !dig->afmt || !dig->afmt->enabled)
> +                       continue;
> +               if (!dig->afmt->pin || dig->afmt->pin->id != port)
> +                       continue;
> +               connector = amdgpu_get_connector_for_encoder(encoder);
> +               *enabled = !!connector;
> +               if (connector) {
> +                       ret = drm_eld_size(connector->eld);
> +                       memcpy(buf, connector->eld, min(max_bytes, ret));
> +                       break;
> +               }
> +       }
> +
> +       return ret;
> +}
> +
> +static const struct drm_audio_component_ops amdgpu_audio_component_ops = {
> +       .get_eld = amdgpu_audio_component_get_eld,
> +};
> +
> +static int amdgpu_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_audio_component_ops;
> +       acomp->dev = kdev;
> +       adev->mode_info.audio.component = acomp;
> +       return 0;
> +}
> +
> +static void amdgpu_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->mode_info.audio.component = NULL;
> +}
> +
> +static const struct component_ops amdgpu_audio_component_bind_ops = {
> +       .bind   = amdgpu_audio_component_bind,
> +       .unbind = amdgpu_audio_component_unbind,
> +};
> +
> +int amdgpu_audio_component_init(struct amdgpu_device *adev)
> +{
> +       int err;
> +
> +       err = component_add(adev->dev, &amdgpu_audio_component_bind_ops);
> +       if (err < 0)
> +               return err;
> +       adev->mode_info.audio.component_registered = true;
> +       return 0;
> +}
> +
> +void amdgpu_audio_component_fini(struct amdgpu_device *adev)
> +{
> +       if (adev->mode_info.audio.component_registered) {
> +               component_del(adev->dev, &amdgpu_audio_component_bind_ops);
> +               adev->mode_info.audio.component_registered = false;
> +       }
> +}
> +
> +void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin)
> +{
> +       struct drm_audio_component *acomp = adev->mode_info.audio.component;
> +
> +       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> +               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> +                                                pin, -1);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index b9e9e8b02fb7..7cb1aa97e522 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -39,6 +39,7 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_fb_helper.h>
> +#include <drm/drm_audio_component.h>
>  #include <linux/i2c.h>
>  #include <linux/i2c-algo-bit.h>
>  #include <linux/hrtimer.h>
> @@ -260,6 +261,8 @@ struct amdgpu_audio {
>         bool enabled;
>         struct amdgpu_audio_pin pin[AMDGPU_MAX_AFMT_BLOCKS];
>         int num_pins;
> +       bool component_registered;
> +       struct drm_audio_component *component;
>  };
> 
>  struct amdgpu_display_funcs {
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index ada241bfeee9..c8471a59930f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1392,6 +1392,8 @@ static void dce_v10_0_audio_enable(struct amdgpu_device *adev,
> 
>         WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>                            enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
> +
> +       amdgpu_audio_eld_notify(adev, pin->id);
>  }
> 
>  static const u32 pin_offsets[] =
> @@ -1430,6 +1432,8 @@ static int dce_v10_0_audio_init(struct amdgpu_device *adev)
>                 dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>         }
> 
> +       amdgpu_audio_component_init(adev);
> +
>         return 0;
>  }
> 
> @@ -1443,6 +1447,8 @@ static void dce_v10_0_audio_fini(struct amdgpu_device *adev)
>         if (!adev->mode_info.audio.enabled)
>                 return;
> 
> +       amdgpu_audio_component_fini(adev);
> +
>         for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>                 dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index a5b96eac3033..49edb74725a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1418,6 +1418,8 @@ static void dce_v11_0_audio_enable(struct amdgpu_device *adev,
> 
>         WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>                            enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
> +
> +       amdgpu_audio_eld_notify(adev, pin->id);
>  }
> 
>  static const u32 pin_offsets[] =
> @@ -1472,6 +1474,8 @@ static int dce_v11_0_audio_init(struct amdgpu_device *adev)
>                 dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>         }
> 
> +       amdgpu_audio_component_init(adev);
> +
>         return 0;
>  }
> 
> @@ -1485,6 +1489,8 @@ static void dce_v11_0_audio_fini(struct amdgpu_device *adev)
>         if (!adev->mode_info.audio.enabled)
>                 return;
> 
> +       amdgpu_audio_component_fini(adev);
> +
>         for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>                 dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 394cc1e8fe20..2b7468eaacca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1297,6 +1297,8 @@ static void dce_v6_0_audio_enable(struct amdgpu_device *adev,
> 
>         WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>                         enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
> +
> +       amdgpu_audio_eld_notify(adev, pin->id);
>  }
> 
>  static const u32 pin_offsets[7] =
> @@ -1343,6 +1345,8 @@ static int dce_v6_0_audio_init(struct amdgpu_device *adev)
>                 dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>         }
> 
> +       amdgpu_audio_component_init(adev);
> +
>         return 0;
>  }
> 
> @@ -1356,6 +1360,8 @@ static void dce_v6_0_audio_fini(struct amdgpu_device *adev)
>         if (!adev->mode_info.audio.enabled)
>                 return;
> 
> +       amdgpu_audio_component_fini(adev);
> +
>         for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>                 dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index c9b9ab8f1b05..c2370ea1f64f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1348,6 +1348,8 @@ static void dce_v8_0_audio_enable(struct amdgpu_device *adev,
> 
>         WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>                 enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
> +
> +       amdgpu_audio_eld_notify(adev, pin->id);
>  }
> 
>  static const u32 pin_offsets[7] =
> @@ -1395,6 +1397,8 @@ static int dce_v8_0_audio_init(struct amdgpu_device *adev)
>                 dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>         }
> 
> +       amdgpu_audio_component_init(adev);
> +
>         return 0;
>  }
> 
> @@ -1408,6 +1412,8 @@ static void dce_v8_0_audio_fini(struct amdgpu_device *adev)
>         if (!adev->mode_info.audio.enabled)
>                 return;
> 
> +       amdgpu_audio_component_fini(adev);
> +
>         for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>                 dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
> 
> --
> 2.18.0
> 
> _______________________________________________
> 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 4/4] drm/amdgpu: Add audio component support

[jimqu]

On 2018年07月25日 13:46, Takashi Iwai wrote:
> On Wed, 25 Jul 2018 07:38:37 +0200,
> Qu, Jim wrote:
>> Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.
>>
>> IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.
> Hm, but by the modesetting, it actually enables / disables the audio
> as well, no?  If so, the notifier is exactly for that purpose.  The
> audio driver needs to know not only about the physical connection but
> whether the audio can be actually driven.
>
> That is, even if the monitor is connected, the audio won't come out if
> the mode is off.  That is equivalent with the unplugged state for the
> audio driver.
>
> The i915 driver deals with the notifier just like the above, so the
> behavior is intentional.
>
>
> thanks,
>
> Takashi

I am afraid if device hotplug out, how is audio state if it follow up 
eld info? Since the modesetting is never performed for the display which 
is plugged out, so there is no notify() call on it.

Thanks
JimQu
>> With None-DC driver, the detect() of connectors will to be preformed in drm_helper_hpd_irq_event() when there is device hotplug. However, amdgpu_connector_xxxx_detect() maybe need to be modified to fit your change.
>>
>> Thanks
>> JimQu
>> ************************************************************************************************************
>> ---
>>   drivers/gpu/drm/Kconfig                   |  1 +
>>   drivers/gpu/drm/amd/amdgpu/Makefile       |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c | 97 +++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  3 +
>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  6 ++
>>   drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  6 ++
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  6 ++
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  6 ++
>>   9 files changed, 130 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 2c7112ddfed4..fbe7216c5c56 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -193,6 +193,7 @@ config DRM_AMDGPU
>>          select BACKLIGHT_LCD_SUPPORT
>>          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.
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index bfd332c95b61..9c26facddb17 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -52,7 +52,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>>          amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
>>          amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_atomfirmware.o \
>>          amdgpu_queue_mgr.o amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o \
>> -       amdgpu_ids.o
>> +       amdgpu_ids.o amdgpu_audio.o
>>
>>   # add asic specific block
>>   amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index a59c07590cee..203d2584c989 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1957,5 +1957,9 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev );
>>   static inline int amdgpu_dm_display_resume(struct amdgpu_device *adev) { return 0; }
>>   #endif
>>
>> +int amdgpu_audio_component_init(struct amdgpu_device *adev);
>> +void amdgpu_audio_component_fini(struct amdgpu_device *adev);
>> +void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin);
>> +
>>   #include "amdgpu_object.h"
>>   #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>> new file mode 100644
>> index 000000000000..39256e2f84b3
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_audio.c
>> @@ -0,0 +1,97 @@
>> +// SPDX-License-Identifier: MIT
>> +
>> +#include <linux/component.h>
>> +#include "amdgpu.h"
>> +
>> +static int amdgpu_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 drm_encoder *encoder;
>> +       struct amdgpu_encoder *amdgpu_encoder;
>> +       struct amdgpu_encoder_atom_dig *dig;
>> +       struct drm_connector *connector;
>> +       int ret = 0;
>> +
>> +       *enabled = 0;
>> +       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
>> +               amdgpu_encoder = to_amdgpu_encoder(encoder);
>> +               dig = amdgpu_encoder->enc_priv;
>> +               if (!dig || !dig->afmt || !dig->afmt->enabled)
>> +                       continue;
>> +               if (!dig->afmt->pin || dig->afmt->pin->id != port)
>> +                       continue;
>> +               connector = amdgpu_get_connector_for_encoder(encoder);
>> +               *enabled = !!connector;
>> +               if (connector) {
>> +                       ret = drm_eld_size(connector->eld);
>> +                       memcpy(buf, connector->eld, min(max_bytes, ret));
>> +                       break;
>> +               }
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +static const struct drm_audio_component_ops amdgpu_audio_component_ops = {
>> +       .get_eld = amdgpu_audio_component_get_eld,
>> +};
>> +
>> +static int amdgpu_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_audio_component_ops;
>> +       acomp->dev = kdev;
>> +       adev->mode_info.audio.component = acomp;
>> +       return 0;
>> +}
>> +
>> +static void amdgpu_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->mode_info.audio.component = NULL;
>> +}
>> +
>> +static const struct component_ops amdgpu_audio_component_bind_ops = {
>> +       .bind   = amdgpu_audio_component_bind,
>> +       .unbind = amdgpu_audio_component_unbind,
>> +};
>> +
>> +int amdgpu_audio_component_init(struct amdgpu_device *adev)
>> +{
>> +       int err;
>> +
>> +       err = component_add(adev->dev, &amdgpu_audio_component_bind_ops);
>> +       if (err < 0)
>> +               return err;
>> +       adev->mode_info.audio.component_registered = true;
>> +       return 0;
>> +}
>> +
>> +void amdgpu_audio_component_fini(struct amdgpu_device *adev)
>> +{
>> +       if (adev->mode_info.audio.component_registered) {
>> +               component_del(adev->dev, &amdgpu_audio_component_bind_ops);
>> +               adev->mode_info.audio.component_registered = false;
>> +       }
>> +}
>> +
>> +void amdgpu_audio_eld_notify(struct amdgpu_device *adev, int pin)
>> +{
>> +       struct drm_audio_component *acomp = adev->mode_info.audio.component;
>> +
>> +       if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
>> +               acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
>> +                                                pin, -1);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> index b9e9e8b02fb7..7cb1aa97e522 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> @@ -39,6 +39,7 @@
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_plane_helper.h>
>>   #include <drm/drm_fb_helper.h>
>> +#include <drm/drm_audio_component.h>
>>   #include <linux/i2c.h>
>>   #include <linux/i2c-algo-bit.h>
>>   #include <linux/hrtimer.h>
>> @@ -260,6 +261,8 @@ struct amdgpu_audio {
>>          bool enabled;
>>          struct amdgpu_audio_pin pin[AMDGPU_MAX_AFMT_BLOCKS];
>>          int num_pins;
>> +       bool component_registered;
>> +       struct drm_audio_component *component;
>>   };
>>
>>   struct amdgpu_display_funcs {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> index ada241bfeee9..c8471a59930f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> @@ -1392,6 +1392,8 @@ static void dce_v10_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                             enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[] =
>> @@ -1430,6 +1432,8 @@ static int dce_v10_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1443,6 +1447,8 @@ static void dce_v10_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v10_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> index a5b96eac3033..49edb74725a7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> @@ -1418,6 +1418,8 @@ static void dce_v11_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                             enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[] =
>> @@ -1472,6 +1474,8 @@ static int dce_v11_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1485,6 +1489,8 @@ static void dce_v11_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v11_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> index 394cc1e8fe20..2b7468eaacca 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
>> @@ -1297,6 +1297,8 @@ static void dce_v6_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                          enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[7] =
>> @@ -1343,6 +1345,8 @@ static int dce_v6_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1356,6 +1360,8 @@ static void dce_v6_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v6_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> index c9b9ab8f1b05..c2370ea1f64f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> @@ -1348,6 +1348,8 @@ static void dce_v8_0_audio_enable(struct amdgpu_device *adev,
>>
>>          WREG32_AUDIO_ENDPT(pin->offset, ixAZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL,
>>                  enable ? AZALIA_F0_CODEC_PIN_CONTROL_HOT_PLUG_CONTROL__AUDIO_ENABLED_MASK : 0);
>> +
>> +       amdgpu_audio_eld_notify(adev, pin->id);
>>   }
>>
>>   static const u32 pin_offsets[7] =
>> @@ -1395,6 +1397,8 @@ static int dce_v8_0_audio_init(struct amdgpu_device *adev)
>>                  dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>          }
>>
>> +       amdgpu_audio_component_init(adev);
>> +
>>          return 0;
>>   }
>>
>> @@ -1408,6 +1412,8 @@ static void dce_v8_0_audio_fini(struct amdgpu_device *adev)
>>          if (!adev->mode_info.audio.enabled)
>>                  return;
>>
>> +       amdgpu_audio_component_fini(adev);
>> +
>>          for (i = 0; i < adev->mode_info.audio.num_pins; i++)
>>                  dce_v8_0_audio_enable(adev, &adev->mode_info.audio.pin[i], false);
>>
>> --
>> 2.18.0
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: y4T�TCH 4/4] drm/amdgpu: Add audio component support

[Takashi Iwai]

On Wed, 25 Jul 2018 10:02:34 +0200,
jimqu wrote:
> 
> 
> 
> On 2018年07月25日 13:46, Takashi Iwai wrote:
> > On Wed, 25 Jul 2018 07:38:37 +0200,
> > Qu, Jim wrote:
> >> Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.
> >>
> >> IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.
> > Hm, but by the modesetting, it actually enables / disables the audio
> > as well, no?  If so, the notifier is exactly for that purpose.  The
> > audio driver needs to know not only about the physical connection but
> > whether the audio can be actually driven.
> >
> > That is, even if the monitor is connected, the audio won't come out if
> > the mode is off.  That is equivalent with the unplugged state for the
> > audio driver.
> >
> > The i915 driver deals with the notifier just like the above, so the
> > behavior is intentional.
> >
> >
> > thanks,
> >
> > Takashi
> 
> I am afraid if device hotplug out, how is audio state if it follow up
> eld info? Since the modesetting is never performed for the display
> which is plugged out, so there is no notify() call on it.

In principle, the HDMI audio just needs to follows the video state,
and it doesn't need to care actual physical connections.  As long as
video can go out, it's fine, audio can, too.  When video is disabled
(even if connected), audio can't be used as well, so it must follow to
off, too.  The notifier is used to follow this video state change.

Practically seen, the user-space shall switch off the video
accordingly upon hot unplug, then the audio notifier is sent, and the
audio gets off, too.


thanks,

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

Re: y4T�TCH 4/4] drm/amdgpu: Add audio component support

[jimqu]

On 2018年07月25日 16:20, Takashi Iwai wrote:
> On Wed, 25 Jul 2018 10:02:34 +0200,
> jimqu wrote:
>>
>>
>> On 2018年07月25日 13:46, Takashi Iwai wrote:
>>> On Wed, 25 Jul 2018 07:38:37 +0200,
>>> Qu, Jim wrote:
>>>> Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.
>>>>
>>>> IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.
>>> Hm, but by the modesetting, it actually enables / disables the audio
>>> as well, no?  If so, the notifier is exactly for that purpose.  The
>>> audio driver needs to know not only about the physical connection but
>>> whether the audio can be actually driven.
>>>
>>> That is, even if the monitor is connected, the audio won't come out if
>>> the mode is off.  That is equivalent with the unplugged state for the
>>> audio driver.
>>>
>>> The i915 driver deals with the notifier just like the above, so the
>>> behavior is intentional.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>> I am afraid if device hotplug out, how is audio state if it follow up
>> eld info? Since the modesetting is never performed for the display
>> which is plugged out, so there is no notify() call on it.
> In principle, the HDMI audio just needs to follows the video state,
> and it doesn't need to care actual physical connections.  As long as
> video can go out, it's fine, audio can, too.  When video is disabled
> (even if connected), audio can't be used as well, so it must follow to
> off, too.  The notifier is used to follow this video state change.
>
> Practically seen, the user-space shall switch off the video
> accordingly upon hot unplug, then the audio notifier is sent, and the
> audio gets off, too.
>
>
> thanks,
>
> Takashi

Ok, that means there are other code pathes to update audio state. 
Anyway, for patch 3/4 Acked-by: Jim Qu <Jim.Qu@amd.com>
There may be other concerns from Alex/Christian.

Thanks
JimQu

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel