ALSA: hda: Expose codec organization functions

PATCH Applied Cezary Rojewski 2 participants 7 messages Feb 14, 2022 – Feb 21, 2022

Subsystems (2)

Sound ASoC

lore

Share & tools

lore

Cezary Rojewski

author

2022-02-14 10:13 UTC #0 lore raw

v2 cover ALSA: hda: Expose codec organization functions

Changes expose several function that are currently unavailable for HDA-DSP drivers for use. Those functions are:

snd_hda_codec_cleanup_for_unbind()
snd_hda_codec_set_power_save()
snd_hda_codec_register()
snd_hda_codec_unregister()
snd_hda_codec_device_init()

This allows upcoming AVS driver [1] to re-use even mode of HDA related code that is currently available in sound/pci/hda and sound/hda and prevent any code duplication within avs-driver that would otherwise had to happen.

Last patch in the series provides snd_hdac_ext_bus_link_at() - a helper function which allows for retrieval of HDA segment (link) based on codec address directly. This is simpler than parsing codec-name first to extract the address what is the case for snd_hdac_ext_bus_get_link(). The latter function is updated to re-use newly added one so core logic is not duplicated after the addition.

Changes v1 -> v2:
- snd_hda_codec_new() now calls snd_hda_codec_device_new() with 'snddev_managed' param set to 'true', reported by Kai

[1]: https://lore.kernel.org/all/[email protected]/

Cezary Rojewski (4):
ALSA: hda: Add snd_hdac_ext_bus_link_at() helper
ALSA: hda: Update and expose snd_hda_codec_device_init()
ALSA: hda: Update and expose codec register procedures
ALSA: hda: Expose codec cleanup and power-save functions

 include/sound/hda_codec.h           | 11 +++-
 include/sound/hdaudio_ext.h         |  1 +
 sound/hda/ext/hdac_ext_controller.c | 31 +++++++---
 sound/pci/hda/hda_codec.c           | 94 ++++++++++++++++++++---------
 sound/pci/hda/hda_local.h           |  2 -
 sound/soc/codecs/hdac_hda.c         |  2 +-
 6 files changed, 100 insertions(+), 41 deletions(-)

Signature · 1 line

2.25.1

Cezary Rojewski

author

2022-02-14 10:14 UTC #1 lore raw

PATCH v2 1/4 ALSA: hda: Add snd_hdac_ext_bus_link_at() helper

This patch exposes a new helper to directly retrieve the link from the codec address, and makes use of this helper when retrieving the link from the codec name.

Signed-off-by Cezary Rojewski <[email protected]>

---
 include/sound/hdaudio_ext.h         |  1 +
 sound/hda/ext/hdac_ext_controller.c | 31 +++++++++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

include/sound/hdaudio_ext.h +1 -0

index 77123c3e4095..b0c8e4936168 100644
@@ -28,6 +28,7 @@ void snd_hdac_ext_stream_spbcap_enable(struct hdac_bus *chip,
 				 bool enable, int index);
 
 int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_bus *bus);
+struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr);
 struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus,
 						const char *codec_name);

sound/hda/ext/hdac_ext_controller.c +21 ~~-10~~

index b2df7b4f9227..b072392725c7 100644
@@ -132,6 +132,26 @@ void snd_hdac_link_free_all(struct hdac_bus *bus)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_link_free_all);
 
+/**
+ * snd_hdac_ext_bus_link_at - get link at specified address
+ * @bus: link's parent bus device
+ * @addr: codec device address
+ *
+ * Returns link object or NULL if matching link is not found.
+ */
+struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr)
+{
+	struct hdac_ext_link *hlink;
+	int i;
+
+	list_for_each_entry(hlink, &bus->hlink_list, list)
+		for (i = 0; i < HDA_MAX_CODECS; i++)
+			if (hlink->lsdiid & (0x1 << addr))
+				return hlink;
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_at);
+
 /**
  * snd_hdac_ext_bus_get_link - get link based on codec name
  * @bus: the pointer to HDAC bus object

@@ -140,8 +160,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_link_free_all);
 struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus,
 						 const char *codec_name)
 {
-	int i;
-	struct hdac_ext_link *hlink = NULL;
 	int bus_idx, addr;
 
 	if (sscanf(codec_name, "ehdaudio%dD%d", &bus_idx, &addr) != 2)

@@ -151,14 +169,7 @@ struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus,
 	if (addr < 0 || addr > 31)
 		return NULL;
 
-	list_for_each_entry(hlink, &bus->hlink_list, list) {
-		for (i = 0; i < HDA_MAX_CODECS; i++) {
-			if (hlink->lsdiid & (0x1 << addr))
-				return hlink;
-		}
-	}
-
-	return NULL;
+	return snd_hdac_ext_bus_link_at(bus, addr);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_link);

Signature · 1 line

2.25.1

Cezary Rojewski

author

2022-02-14 10:14 UTC #2 lore raw

PATCH v2 2/4 ALSA: hda: Update and expose snd_hda_codec_device_init()

In reply to Cezary Rojewski

With few changes, snd_hda_codec_device_init() can be re-used by ASoC drivers. While at it, provide kernel doc for the exposed function.

Signed-off-by Cezary Rojewski <[email protected]>

---
 include/sound/hda_codec.h |  3 +++
 sound/pci/hda/hda_codec.c | 45 +++++++++++++++++++++++++--------------
 2 files changed, 32 insertions(+), 16 deletions(-)

include/sound/hda_codec.h +3 -0

index 82d9daa17851..5e3cbcca42f0 100644
@@ -306,6 +306,9 @@ struct hda_codec {
 /*
  * constructors
  */
+__printf(3, 4) struct hda_codec *
+snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
+			  const char *fmt, ...);
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		      unsigned int codec_addr, struct hda_codec **codecp);
 int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,

sound/pci/hda/hda_codec.c +29 ~~-16~~

83 lines not on this page · raw message

Signature · 1 line

2.25.1

Cezary Rojewski

author

2022-02-14 10:14 UTC #3 lore raw

PATCH v2 3/4 ALSA: hda: Update and expose codec register procedures

In reply to Cezary Rojewski

With few changes, snd_hda_codec_register() and its unregister-counterpart can be re-used by ASoC drivers. While at it, provide kernel doc for the exposed functions.

Due to ALSA-device vs ASoC-component organization differences, new 'snddev_managed' argument is specified allowing for better control over codec registration process.

Signed-off-by Cezary Rojewski <[email protected]>

---

Changes v1 -> v2:
- snd_hda_codec_new() now calls snd_hda_codec_device_new() with 'snddev_managed' param set to 'true', reported by Kai

 include/sound/hda_codec.h   |  5 ++++-
 sound/pci/hda/hda_codec.c   | 35 ++++++++++++++++++++++++++---------
 sound/pci/hda/hda_local.h   |  1 -
 sound/soc/codecs/hdac_hda.c |  2 +-
 4 files changed, 31 insertions(+), 12 deletions(-)

include/sound/hda_codec.h +4 -1

index 5e3cbcca42f0..f74abc13414f 100644
@@ -312,9 +312,12 @@ snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		      unsigned int codec_addr, struct hda_codec **codecp);
 int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
-		      unsigned int codec_addr, struct hda_codec *codec);
+		      unsigned int codec_addr, struct hda_codec *codec,
+		      bool snddev_managed);
 int snd_hda_codec_configure(struct hda_codec *codec);
 int snd_hda_codec_update_widgets(struct hda_codec *codec);
+void snd_hda_codec_register(struct hda_codec *codec);
+void snd_hda_codec_unregister(struct hda_codec *codec);
 
 /*
  * low level functions

sound/pci/hda/hda_codec.c +26 -9

index 3787060ad77f..4afdc3b68bd6 100644
@@ -813,7 +813,12 @@ void snd_hda_codec_display_power(struct hda_codec *codec, bool enable)
 		snd_hdac_display_power(&codec->bus->core, codec->addr, enable);
 }
 
-/* also called from hda_bind.c */
+/**
+ * snd_hda_codec_register - Finalize codec initialization
+ * @codec: codec device to register
+ *
+ * Also called from hda_bind.c
+ */
 void snd_hda_codec_register(struct hda_codec *codec)
 {
 	if (codec->registered)

@@ -826,6 +831,7 @@ void snd_hda_codec_register(struct hda_codec *codec)
 		codec->registered = 1;
 	}
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_register);
 
 static int snd_hda_codec_dev_register(struct snd_device *device)
 {

@@ -833,10 +839,12 @@ static int snd_hda_codec_dev_register(struct snd_device *device)
 	return 0;
 }
 
-static int snd_hda_codec_dev_free(struct snd_device *device)
+/**
+ * snd_hda_codec_unregister - Unregister specified codec device
+ * @codec: codec device to unregister
+ */
+void snd_hda_codec_unregister(struct hda_codec *codec)
 {
-	struct hda_codec *codec = device->device_data;
-
 	codec->in_freeing = 1;
 	/*
 	 * snd_hda_codec_device_new() is used by legacy HDA and ASoC driver.

@@ -853,7 +861,12 @@ static int snd_hda_codec_dev_free(struct snd_device *device)
 	 */
 	if (codec->core.type == HDA_DEV_LEGACY)
 		put_device(hda_codec_dev(codec));
+}
+EXPORT_SYMBOL_GPL(snd_hda_codec_unregister);
 
+static int snd_hda_codec_dev_free(struct snd_device *device)
+{
+	snd_hda_codec_unregister(device->device_data);
 	return 0;
 }

@@ -940,12 +953,13 @@ int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		return PTR_ERR(codec);
 	*codecp = codec;
 
-	return snd_hda_codec_device_new(bus, card, codec_addr, *codecp);
+	return snd_hda_codec_device_new(bus, card, codec_addr, *codecp, true);
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_new);
 
 int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
-			unsigned int codec_addr, struct hda_codec *codec)
+			unsigned int codec_addr, struct hda_codec *codec,
+			bool snddev_managed)
 {
 	char component[31];
 	hda_nid_t fg;

@@ -1020,9 +1034,12 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
 		codec->core.subsystem_id, codec->core.revision_id);
 	snd_component_add(card, component);
 
-	err = snd_device_new(card, SNDRV_DEV_CODEC, codec, &dev_ops);
-	if (err < 0)
-		goto error;
+	if (snddev_managed) {
+		/* ASoC features component management instead */
+		err = snd_device_new(card, SNDRV_DEV_CODEC, codec, &dev_ops);
+		if (err < 0)
+			goto error;
+	}
 
 	/* PM runtime needs to be enabled later after binding codec */
 	pm_runtime_forbid(&codec->core.dev);

sound/pci/hda/hda_local.h +0 -1

index 8621f576446b..4c52dfb615bc 100644
@@ -135,7 +135,6 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name,
 #define snd_hda_add_vmaster(codec, name, tlv, followers, suffix, access) \
 	__snd_hda_add_vmaster(codec, name, tlv, followers, suffix, true, access, NULL)
 int snd_hda_codec_reset(struct hda_codec *codec);
-void snd_hda_codec_register(struct hda_codec *codec);
 void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec);
 void snd_hda_codec_disconnect_pcms(struct hda_codec *codec);

sound/soc/codecs/hdac_hda.c +1 -1

index de5955db0a5f..667f3df239c7 100644
@@ -413,7 +413,7 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
 				       HDA_CODEC_IDX_CONTROLLER, true);
 
 	ret = snd_hda_codec_device_new(hcodec->bus, component->card->snd_card,
-				       hdev->addr, hcodec);
+				       hdev->addr, hcodec, true);
 	if (ret < 0) {
 		dev_err(&hdev->dev, "failed to create hda codec %d\n", ret);
 		goto error_no_pm;

Signature · 1 line

2.25.1

Cezary Rojewski

author

2022-02-14 10:15 UTC #4 lore raw

PATCH v2 4/4 ALSA: hda: Expose codec cleanup and power-save functions

In reply to Cezary Rojewski

With few changes, snd_hda_codec_set_power_save() and snd_hda_codec_cleanup_for_unbind() can be re-used by ASoC drivers. While at it, provide kernel doc for the exposed functions.

Signed-off-by Cezary Rojewski <[email protected]>

---
 include/sound/hda_codec.h |  3 +++
 sound/pci/hda/hda_codec.c | 14 ++++++++++++--
 sound/pci/hda/hda_local.h |  1 -
 3 files changed, 15 insertions(+), 3 deletions(-)

include/sound/hda_codec.h +3 -0

index f74abc13414f..77426ff58338 100644
@@ -318,6 +318,7 @@ int snd_hda_codec_configure(struct hda_codec *codec);
 int snd_hda_codec_update_widgets(struct hda_codec *codec);
 void snd_hda_codec_register(struct hda_codec *codec);
 void snd_hda_codec_unregister(struct hda_codec *codec);
+void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec);
 
 /*
  * low level functions

@@ -496,9 +497,11 @@ int hda_call_check_power_status(struct hda_codec *codec, hda_nid_t nid)
 #define snd_hda_power_down(codec)	snd_hdac_power_down(&(codec)->core)
 #define snd_hda_power_down_pm(codec)	snd_hdac_power_down_pm(&(codec)->core)
 #ifdef CONFIG_PM
+void snd_hda_codec_set_power_save(struct hda_codec *codec, int delay);
 void snd_hda_set_power_save(struct hda_bus *bus, int delay);
 void snd_hda_update_power_acct(struct hda_codec *codec);
 #else
+static inline void snd_hda_codec_set_power_save(struct hda_codec *codec, int delay) {}
 static inline void snd_hda_set_power_save(struct hda_bus *bus, int delay) {}
 #endif

sound/pci/hda/hda_codec.c +12 -2

index 4afdc3b68bd6..748022da0c18 100644
@@ -766,6 +766,10 @@ static void codec_release_pcms(struct hda_codec *codec)
 	}
 }
 
+/**
+ * snd_hda_codec_cleanup_for_unbind - Prepare codec for removal
+ * @codec: codec device to cleanup
+ */
 void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
 {
 	if (codec->registered) {

@@ -3397,7 +3401,12 @@ int snd_hda_add_new_ctls(struct hda_codec *codec,
 EXPORT_SYMBOL_GPL(snd_hda_add_new_ctls);
 
 #ifdef CONFIG_PM
-static void codec_set_power_save(struct hda_codec *codec, int delay)
+/**
+ * snd_hda_codec_set_power_save - Configure codec's runtime PM
+ * @codec: codec device to configure
+ * @delay: autosuspend delay
+ */
+void snd_hda_codec_set_power_save(struct hda_codec *codec, int delay)
 {
 	struct device *dev = hda_codec_dev(codec);

@@ -3415,6 +3424,7 @@ static void codec_set_power_save(struct hda_codec *codec, int delay)
 		pm_runtime_forbid(dev);
 	}
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_set_power_save);
 
 /**
  * snd_hda_set_power_save - reprogram autosuspend for the given delay

@@ -3428,7 +3438,7 @@ void snd_hda_set_power_save(struct hda_bus *bus, int delay)
 	struct hda_codec *c;
 
 	list_for_each_codec(c, bus)
-		codec_set_power_save(c, delay);
+		snd_hda_codec_set_power_save(c, delay);
 }
 EXPORT_SYMBOL_GPL(snd_hda_set_power_save);

sound/pci/hda/hda_local.h +0 -1

index 4c52dfb615bc..aca592651870 100644
@@ -135,7 +135,6 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name,
 #define snd_hda_add_vmaster(codec, name, tlv, followers, suffix, access) \
 	__snd_hda_add_vmaster(codec, name, tlv, followers, suffix, true, access, NULL)
 int snd_hda_codec_reset(struct hda_codec *codec);
-void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec);
 void snd_hda_codec_disconnect_pcms(struct hda_codec *codec);
 
 #define snd_hda_regmap_sync(codec)	snd_hdac_regmap_sync(&(codec)->core)

Signature · 1 line

2.25.1

Cezary Rojewski

author

2022-02-21 09:52 UTC #5 lore raw

In reply to Cezary Rojewski

Quoted 22 lines · Cezary Rojewski

Changes expose several function that are currently unavailable for HDA-DSP drivers for use. Those functions are:
snd_hda_codec_cleanup_for_unbind()
snd_hda_codec_set_power_save()
snd_hda_codec_register()
snd_hda_codec_unregister()
snd_hda_codec_device_init()
This allows upcoming AVS driver [1] to re-use even mode of HDA related code that is currently available in sound/pci/hda and sound/hda and prevent any code duplication within avs-driver that would otherwise had to happen.
Last patch in the series provides snd_hdac_ext_bus_link_at() - a helper function which allows for retrieval of HDA segment (link) based on codec address directly. This is simpler than parsing codec-name first to extract the address what is the case for snd_hdac_ext_bus_get_link(). The latter function is updated to re-use newly added one so core logic is not duplicated after the addition.

Quoted 3 lines

Changes v1 -> v2:
- snd_hda_codec_new() now calls snd_hda_codec_device_new() with 'snddev_managed' param set to 'true', reported by Kai

Hello Takashi,

Is there anything else I could help with or explain to help get this series merged?

Regards,
Czarek

Takashi Iwai

Applied

2022-02-21 10:08 UTC #6 lore raw

Quoted 24 lines · Cezary Rojewski

Quoted 22 lines · Cezary Rojewski
Changes expose several function that are currently unavailable for HDA-DSP drivers for use. Those functions are:
snd_hda_codec_cleanup_for_unbind()
snd_hda_codec_set_power_save()
snd_hda_codec_register()
snd_hda_codec_unregister()
snd_hda_codec_device_init()
This allows upcoming AVS driver [1] to re-use even mode of HDA related code that is currently available in sound/pci/hda and sound/hda and prevent any code duplication within avs-driver that would otherwise had to happen.
Last patch in the series provides snd_hdac_ext_bus_link_at() - a helper function which allows for retrieval of HDA segment (link) based on codec address directly. This is simpler than parsing codec-name first to extract the address what is the case for snd_hdac_ext_bus_get_link(). The latter function is updated to re-use newly added one so core logic is not duplicated after the addition.

Quoted 8 lines

Quoted 3 lines
Changes v1 -> v2:
- snd_hda_codec_new() now calls snd_hda_codec_device_new() with 'snddev_managed' param set to 'true', reported by Kai
Hello Takashi,
Is there anything else I could help with or explain to help get this series merged?

I merged all patches now to for-next branch.

thanks,

Takashi