diff mbox series

[01/12] ALSA: hda: intel: Reduce CONFIG_PM dependencies

Message ID 20240506161359.6960-2-tiwai@suse.de (mailing list archive)
State Accepted
Commit 32d7c6cdc98f7131f13c41251e87837df81e0b31
Headers show
Series ALSA: hda: Reduce CONFIG_PM dependencies | expand

Commit Message

Takashi Iwai May 6, 2024, 4:13 p.m. UTC
snd-hda-intel contains lots of CONFIG_PM dependent code although
CONFIG_PM is almost mandatory nowadays, and it makes the code
unnecessarily complex.

Let's reduce the dependencies of CONFIG_PM in snd-hda-intel driver
code.  I left a few module options to be dependent on CONFIG_PM (which
are visible to users), but other places are either enabled or
optimized by compiler automatically.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_intel.c       | 46 ++++++++++-----------------------
 sound/pci/hda/hda_intel_trace.h |  2 --
 2 files changed, 14 insertions(+), 34 deletions(-)

Comments

Andy Shevchenko May 8, 2024, 5:45 p.m. UTC | #1
On Mon, May 06, 2024 at 06:13:44PM +0200, Takashi Iwai wrote:
> snd-hda-intel contains lots of CONFIG_PM dependent code although
> CONFIG_PM is almost mandatory nowadays, and it makes the code
> unnecessarily complex.
> 
> Let's reduce the dependencies of CONFIG_PM in snd-hda-intel driver
> code.  I left a few module options to be dependent on CONFIG_PM (which
> are visible to users), but other places are either enabled or
> optimized by compiler automatically.

...

> +static int __maybe_unused azx_resume(struct device *dev)

__maybe)unused is discouraged nowadays.
We have new PM macros (w/o SET_ prefix) along with pm_ptr() / pm_sleep_ptr()
macros. They are preferred. In complicated cases the PTR_IF() can be used
directly.
Takashi Iwai May 9, 2024, 7:05 a.m. UTC | #2
On Wed, 08 May 2024 19:45:42 +0200,
Andy Shevchenko wrote:
> 
> On Mon, May 06, 2024 at 06:13:44PM +0200, Takashi Iwai wrote:
> > snd-hda-intel contains lots of CONFIG_PM dependent code although
> > CONFIG_PM is almost mandatory nowadays, and it makes the code
> > unnecessarily complex.
> > 
> > Let's reduce the dependencies of CONFIG_PM in snd-hda-intel driver
> > code.  I left a few module options to be dependent on CONFIG_PM (which
> > are visible to users), but other places are either enabled or
> > optimized by compiler automatically.
> 
> ...
> 
> > +static int __maybe_unused azx_resume(struct device *dev)
> 
> __maybe)unused is discouraged nowadays.
> We have new PM macros (w/o SET_ prefix) along with pm_ptr() / pm_sleep_ptr()
> macros. They are preferred. In complicated cases the PTR_IF() can be used
> directly.

Yeah, it was a dilemma there.  There seems no standard macro to use
pm_ptr() for runtime_suspend (there is only RUNTIME_PM_OPS()), so for
avoiding __maybe_unused, I'd have to expand them manually instead.


Takashi
Andy Shevchenko May 10, 2024, 2:30 p.m. UTC | #3
On Thu, May 09, 2024 at 09:05:58AM +0200, Takashi Iwai wrote:
> On Wed, 08 May 2024 19:45:42 +0200,
> Andy Shevchenko wrote:
> > On Mon, May 06, 2024 at 06:13:44PM +0200, Takashi Iwai wrote:

> > > snd-hda-intel contains lots of CONFIG_PM dependent code although
> > > CONFIG_PM is almost mandatory nowadays, and it makes the code
> > > unnecessarily complex.
> > > 
> > > Let's reduce the dependencies of CONFIG_PM in snd-hda-intel driver
> > > code.  I left a few module options to be dependent on CONFIG_PM (which
> > > are visible to users), but other places are either enabled or
> > > optimized by compiler automatically.

...

> > > +static int __maybe_unused azx_resume(struct device *dev)
> > 
> > __maybe)unused is discouraged nowadays.
> > We have new PM macros (w/o SET_ prefix) along with pm_ptr() / pm_sleep_ptr()
> > macros. They are preferred. In complicated cases the PTR_IF() can be used
> > directly.
> 
> Yeah, it was a dilemma there.  There seems no standard macro to use
> pm_ptr() for runtime_suspend (there is only RUNTIME_PM_OPS()), so for
> avoiding __maybe_unused, I'd have to expand them manually instead.

I'm not sure I got the use case. If we have runtime PM, we use pm_ptr(),
otherwise pm_sleep_ptr().
diff mbox series

Patch

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 1b550c42db09..31b0c954b0c8 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -186,8 +186,10 @@  MODULE_PARM_DESC(pm_blacklist, "Enable power-management denylist");
 static bool power_save_controller = 1;
 module_param(power_save_controller, bool, 0644);
 MODULE_PARM_DESC(power_save_controller, "Reset controller in power save mode.");
-#else
+#else /* CONFIG_PM */
 #define power_save	0
+#define pm_blacklist	false
+#define power_save_controller	false
 #endif /* CONFIG_PM */
 
 static int align_buffer_size = -1;
@@ -890,7 +892,6 @@  static void __azx_shutdown_chip(struct azx *chip, bool skip_link_reset)
 	display_power(chip, false);
 }
 
-#ifdef CONFIG_PM
 static DEFINE_MUTEX(card_list_lock);
 static LIST_HEAD(card_list);
 
@@ -916,7 +917,7 @@  static void azx_del_card_list(struct azx *chip)
 }
 
 /* trigger power-save check at writing parameter */
-static int param_set_xint(const char *val, const struct kernel_param *kp)
+static int __maybe_unused param_set_xint(const char *val, const struct kernel_param *kp)
 {
 	struct hda_intel *hda;
 	struct azx *chip;
@@ -987,7 +988,6 @@  static void __azx_runtime_resume(struct azx *chip)
 		display_power(chip, false);
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int azx_prepare(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
@@ -1046,7 +1046,7 @@  static int azx_suspend(struct device *dev)
 	return 0;
 }
 
-static int azx_resume(struct device *dev)
+static int __maybe_unused azx_resume(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
 	struct azx *chip;
@@ -1097,9 +1097,8 @@  static int azx_thaw_noirq(struct device *dev)
 
 	return 0;
 }
-#endif /* CONFIG_PM_SLEEP */
 
-static int azx_runtime_suspend(struct device *dev)
+static int __maybe_unused azx_runtime_suspend(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
 	struct azx *chip;
@@ -1116,7 +1115,7 @@  static int azx_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int azx_runtime_resume(struct device *dev)
+static int __maybe_unused azx_runtime_resume(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
 	struct azx *chip;
@@ -1133,7 +1132,7 @@  static int azx_runtime_resume(struct device *dev)
 	return 0;
 }
 
-static int azx_runtime_idle(struct device *dev)
+static int __maybe_unused azx_runtime_idle(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
 	struct azx *chip;
@@ -1159,23 +1158,14 @@  static int azx_runtime_idle(struct device *dev)
 }
 
 static const struct dev_pm_ops azx_pm = {
-	SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
-#ifdef CONFIG_PM_SLEEP
-	.prepare = azx_prepare,
-	.complete = azx_complete,
-	.freeze_noirq = azx_freeze_noirq,
-	.thaw_noirq = azx_thaw_noirq,
-#endif
+	SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
+	.prepare = pm_sleep_ptr(azx_prepare),
+	.complete = pm_sleep_ptr(azx_complete),
+	.freeze_noirq = pm_sleep_ptr(azx_freeze_noirq),
+	.thaw_noirq = pm_sleep_ptr(azx_thaw_noirq),
 	SET_RUNTIME_PM_OPS(azx_runtime_suspend, azx_runtime_resume, azx_runtime_idle)
 };
 
-#define AZX_PM_OPS	&azx_pm
-#else
-#define azx_add_card_list(chip) /* NOP */
-#define azx_del_card_list(chip) /* NOP */
-#define AZX_PM_OPS	NULL
-#endif /* CONFIG_PM */
-
 
 static int azx_probe_continue(struct azx *chip);
 
@@ -2206,7 +2196,6 @@  static int azx_probe(struct pci_dev *pci,
 	return err;
 }
 
-#ifdef CONFIG_PM
 /* On some boards setting power_save to a non 0 value leads to clicking /
  * popping sounds when ever we enter/leave powersaving mode. Ideally we would
  * figure out how to avoid these sounds, but that is not always feasible.
@@ -2248,13 +2237,11 @@  static const struct snd_pci_quirk power_save_denylist[] = {
 	SND_PCI_QUIRK(0x1734, 0x1232, "KONTRON SinglePC", 0),
 	{}
 };
-#endif /* CONFIG_PM */
 
 static void set_default_power_save(struct azx *chip)
 {
 	int val = power_save;
 
-#ifdef CONFIG_PM
 	if (pm_blacklist) {
 		const struct snd_pci_quirk *q;
 
@@ -2265,7 +2252,6 @@  static void set_default_power_save(struct azx *chip)
 			val = 0;
 		}
 	}
-#endif /* CONFIG_PM */
 	snd_hda_set_power_save(&chip->bus, val * 1000);
 }
 
@@ -2321,10 +2307,6 @@  static int azx_probe_continue(struct azx *chip)
 					 chip->fw->data);
 		if (err < 0)
 			goto out_free;
-#ifndef CONFIG_PM
-		release_firmware(chip->fw); /* no longer needed */
-		chip->fw = NULL;
-#endif
 	}
 #endif
 
@@ -2765,7 +2747,7 @@  static struct pci_driver azx_driver = {
 	.remove = azx_remove,
 	.shutdown = azx_shutdown,
 	.driver = {
-		.pm = AZX_PM_OPS,
+		.pm = &azx_pm,
 	},
 };
 
diff --git a/sound/pci/hda/hda_intel_trace.h b/sound/pci/hda/hda_intel_trace.h
index 73a7adfa192d..2775fa81a500 100644
--- a/sound/pci/hda/hda_intel_trace.h
+++ b/sound/pci/hda/hda_intel_trace.h
@@ -34,7 +34,6 @@  DEFINE_EVENT(hda_pm, azx_resume,
 	TP_ARGS(chip)
 );
 
-#ifdef CONFIG_PM
 DEFINE_EVENT(hda_pm, azx_runtime_suspend,
 	TP_PROTO(struct azx *chip),
 	TP_ARGS(chip)
@@ -44,7 +43,6 @@  DEFINE_EVENT(hda_pm, azx_runtime_resume,
 	TP_PROTO(struct azx *chip),
 	TP_ARGS(chip)
 );
-#endif
 
 #endif /* _TRACE_HDA_INTEL_H */