[PATCH 0/7] Runtime PM support (hda/tegra)

[PATCH 0/7] Runtime PM support (hda/tegra)

[Sameer Pujar]

Background
==========
The device power management operations can be controlled with the help of
runtime power management (runtime PM) framework. In this case hda driver
can register runtime PM callbacks with the power management core (PM core).
Idea is to put the device in low power mode, when it is not getting used.
The clocks and power domains can be turned off when no use case is running
on the device. Current patch series adds necessary support to manage hda
device at runtime.

Change log
==========
v1:
----
  Patch-1: "ALSA: hda/tegra: runtime power management support"
    * runtime PM enable/disable added in device probe/remove
    * runtime PM calls are forbidden if AZX_DCAPS_PM_RUNTIME is not enabled
      in driver flags
    * worker thread uses pm_runtime_get_sync to invoke ->runtime_resume and
      pm_runtime_put to invoke ->runtime_suspend. The callbacks are added
      in subsequent patches
 
  Patch-2: "ALSA: hda/tegra: get clock handles early in probe"
    * Clock handles required for hda are acquired early in the probe. This
      is done to make use of runtime PM framework to enable/disable clocks
      in the callbacks.

  Patch-3: "ALSA: hda/tegra: add runtime PM callbacks"
    * adds runtime PM callbacks for ->runtime_suspend(), ->runtime_resume()
    * only skeleton for the callbacks is added, implementation is done in
      the subsequent patches

  Patch-4: "ALSA: hda/tegra: remove redundant clock enable API"
    * As worked thread is calling pm_runtime_get_sync() now, no need to
      enable the clock explicitly. This can be managed from callback.
    * Idea is to have clock enable and disable functionalities in runtime
      PM callbacks and the driver can make use of the above. Any device
      access should be preceded by ->runtime_resume() call.

  Patch-5: "ALSA: hda/tegra: implement runtime suspend/resume"
    * clock enable and controller initialization is moved to
      ->runtime_reesume() and the reverse is done during
      ->runtime_suspend()
    * Also during system wide power transitions, pm_runtime_force_resume()
      is invoked in system resume() and similarly for suspend case.

  Patch-6: "ALSA: hda/tegra: fix kernel panic"
    * kernel panic is happening befoe controller init is done
    * added a flag "probed" to indicate the completion of device init
    * this flag is checked before doing any device access.

  Patch-7: "ALSA: hda/tegra: add driver flag for runtime PM"
    * driver flag AZX_DCAPS_PM_RUNTIME is added if CONFIG_PM is enabled.
    * otherwise runtime PM calls will be forbidden and act as dummy calls

=========

Sameer Pujar (7):
  ALSA: hda/tegra: runtime power management support
  ALSA: hda/tegra: get clock handles early in probe
  ALSA: hda/tegra: add runtime PM callbacks
  ALSA: hda/tegra: remove redundant clock enable API
  ALSA: hda/tegra: implement runtime suspend/resume
  ALSA: hda/tegra: fix kernel panic
  ALSA: hda/tegra: add driver flag for runtime PM

 sound/pci/hda/hda_tegra.c | 120 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 88 insertions(+), 32 deletions(-)

-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 1/7] ALSA: hda/tegra: runtime power management support

[Sameer Pujar]

This patch enables runtime power management(runtime PM) support for
hda. pm_runtime_enable() and pm_runtime_disable() are added during
device probe and remove respectively. The runtime PM callbacks will
be forbidden if hda controller does not have support for runtime PM.
pm_runtime_get_sync() and pm_runtime_put() are added for hda register
access. The callbacks for above will be added in subsequent patches.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 97a176d..2f9dd23 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -32,6 +32,7 @@
 #include <linux/slab.h>
 #include <linux/time.h>
 #include <linux/string.h>
+#include <linux/pm_runtime.h>
 
 #include <sound/core.h>
 #include <sound/initval.h>
@@ -512,6 +513,11 @@ static int hda_tegra_probe(struct platform_device *pdev)
 	card->private_data = chip;
 
 	dev_set_drvdata(&pdev->dev, card);
+
+	pm_runtime_enable(hda->dev);
+	if (!azx_has_pm_runtime(chip))
+		pm_runtime_forbid(hda->dev);
+
 	schedule_work(&hda->probe_work);
 
 	return 0;
@@ -528,6 +534,7 @@ static void hda_tegra_probe_work(struct work_struct *work)
 	struct platform_device *pdev = to_platform_device(hda->dev);
 	int err;
 
+	pm_runtime_get_sync(hda->dev);
 	err = hda_tegra_first_init(chip, pdev);
 	if (err < 0)
 		goto out_free;
@@ -549,12 +556,18 @@ static void hda_tegra_probe_work(struct work_struct *work)
 	snd_hda_set_power_save(&chip->bus, power_save * 1000);
 
  out_free:
+	pm_runtime_put(hda->dev);
 	return; /* no error return from async probe */
 }
 
 static int hda_tegra_remove(struct platform_device *pdev)
 {
-	return snd_card_free(dev_get_drvdata(&pdev->dev));
+	int ret;
+
+	ret = snd_card_free(dev_get_drvdata(&pdev->dev));
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
 }
 
 static void hda_tegra_shutdown(struct platform_device *pdev)
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 2/7] ALSA: hda/tegra: get clock handles early in probe

[Sameer Pujar]

Moved devm_clk_get() API calls to a separate function and the same
can be called early in the probe. This is done before runtime PM
for the device is enabled. The runtime resume/suspend callbacks can
later enable/disable clocks respectively(the support would be added
in subsequent patches). Clock handles should be available by the
time runtime suspend/resume calls can happen.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 2f9dd23..28e1656 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -306,22 +306,6 @@ static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
 	struct resource *res;
 	int err;
 
-	hda->hda_clk = devm_clk_get(dev, "hda");
-	if (IS_ERR(hda->hda_clk)) {
-		dev_err(dev, "failed to get hda clock\n");
-		return PTR_ERR(hda->hda_clk);
-	}
-	hda->hda2codec_2x_clk = devm_clk_get(dev, "hda2codec_2x");
-	if (IS_ERR(hda->hda2codec_2x_clk)) {
-		dev_err(dev, "failed to get hda2codec_2x clock\n");
-		return PTR_ERR(hda->hda2codec_2x_clk);
-	}
-	hda->hda2hdmi_clk = devm_clk_get(dev, "hda2hdmi");
-	if (IS_ERR(hda->hda2hdmi_clk)) {
-		dev_err(dev, "failed to get hda2hdmi clock\n");
-		return PTR_ERR(hda->hda2hdmi_clk);
-	}
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hda->regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(hda->regs))
@@ -341,6 +325,29 @@ static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
 	return 0;
 }
 
+static int hda_tegra_init_clk(struct hda_tegra *hda)
+{
+	struct device *dev = hda->dev;
+
+	hda->hda_clk = devm_clk_get(dev, "hda");
+	if (IS_ERR(hda->hda_clk)) {
+		dev_err(dev, "failed to get hda clock\n");
+		return PTR_ERR(hda->hda_clk);
+	}
+	hda->hda2codec_2x_clk = devm_clk_get(dev, "hda2codec_2x");
+	if (IS_ERR(hda->hda2codec_2x_clk)) {
+		dev_err(dev, "failed to get hda2codec_2x clock\n");
+		return PTR_ERR(hda->hda2codec_2x_clk);
+	}
+	hda->hda2hdmi_clk = devm_clk_get(dev, "hda2hdmi");
+	if (IS_ERR(hda->hda2hdmi_clk)) {
+		dev_err(dev, "failed to get hda2hdmi clock\n");
+		return PTR_ERR(hda->hda2hdmi_clk);
+	}
+
+	return 0;
+}
+
 static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
 {
 	struct hdac_bus *bus = azx_bus(chip);
@@ -507,6 +514,10 @@ static int hda_tegra_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	err = hda_tegra_init_clk(hda);
+	if (err < 0)
+		goto out_free;
+
 	err = hda_tegra_create(card, driver_flags, hda);
 	if (err < 0)
 		goto out_free;
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 3/7] ALSA: hda/tegra: add runtime PM callbacks

[Sameer Pujar]

This patch adds skeleton of runtime suspend and resume callbacks.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 28e1656..1189f97 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -265,8 +265,23 @@ static int hda_tegra_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
+#ifdef CONFIG_PM
+static int hda_tegra_runtime_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int hda_tegra_runtime_resume(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM */
+
 static const struct dev_pm_ops hda_tegra_pm = {
 	SET_SYSTEM_SLEEP_PM_OPS(hda_tegra_suspend, hda_tegra_resume)
+	SET_RUNTIME_PM_OPS(hda_tegra_runtime_suspend,
+			   hda_tegra_runtime_resume,
+			   NULL)
 };
 
 static int hda_tegra_dev_disconnect(struct snd_device *device)
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 4/7] ALSA: hda/tegra: remove redundant clock enable API

[Sameer Pujar]

Explicit clock enable is not required during probe, as this would be
managed by runtime PM calls. Clock can be enabled/disabled in runtime
resume/suspend. This way it is easier to balance clock enable/disable
counts.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 1189f97..f068b1e 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -319,7 +319,6 @@ static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
 	struct hdac_bus *bus = azx_bus(chip);
 	struct device *dev = hda->dev;
 	struct resource *res;
-	int err;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hda->regs = devm_ioremap_resource(dev, res);
@@ -329,12 +328,6 @@ static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
 	bus->remap_addr = hda->regs + HDA_BAR0;
 	bus->addr = res->start + HDA_BAR0;
 
-	err = hda_tegra_enable_clocks(hda);
-	if (err) {
-		dev_err(dev, "failed to get enable clocks\n");
-		return err;
-	}
-
 	hda_tegra_init(hda);
 
 	return 0;
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 5/7] ALSA: hda/tegra: implement runtime suspend/resume

[Sameer Pujar]

This patch moves clock enable/disable from system resume/suspend to
runtime resume/suspend respectively. Along with this hda controller
chip init or stop is also moved. System resume/suspend can invoke
runtime callbacks and do necessary setup.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index f068b1e..5546e29 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -233,32 +233,24 @@ static void hda_tegra_disable_clocks(struct hda_tegra *data)
 static int hda_tegra_suspend(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
-	struct azx *chip = card->private_data;
-	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
-	struct hdac_bus *bus = azx_bus(chip);
+	int rc;
 
+	rc = pm_runtime_force_suspend(dev);
+	if (rc < 0)
+		return rc;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
 
-	azx_stop_chip(chip);
-	synchronize_irq(bus->irq);
-	azx_enter_link_reset(chip);
-	hda_tegra_disable_clocks(hda);
-
 	return 0;
 }
 
 static int hda_tegra_resume(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
-	struct azx *chip = card->private_data;
-	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
-
-	hda_tegra_enable_clocks(hda);
-
-	hda_tegra_init(hda);
-
-	azx_init_chip(chip, 1);
+	int rc;
 
+	rc = pm_runtime_force_resume(dev);
+	if (rc < 0)
+		return rc;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
 
 	return 0;
@@ -268,11 +260,32 @@ static int hda_tegra_resume(struct device *dev)
 #ifdef CONFIG_PM
 static int hda_tegra_runtime_suspend(struct device *dev)
 {
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
+	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
+	struct hdac_bus *bus = azx_bus(chip);
+
+	azx_stop_chip(chip);
+	synchronize_irq(bus->irq);
+	azx_enter_link_reset(chip);
+	hda_tegra_disable_clocks(hda);
+
 	return 0;
 }
 
 static int hda_tegra_runtime_resume(struct device *dev)
 {
+	struct snd_card *card = dev_get_drvdata(dev);
+	struct azx *chip = card->private_data;
+	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
+	int rc;
+
+	rc = hda_tegra_enable_clocks(hda);
+	if (rc != 0)
+		return rc;
+	hda_tegra_init(hda);
+	azx_init_chip(chip, 1);
+
 	return 0;
 }
 #endif /* CONFIG_PM */
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 6/7] ALSA: hda/tegra: fix kernel panic

[Sameer Pujar]

Kernel panic is seen during device boot. It appears that before the
probe completes, runtime callbacks happen and the device setup is not
done yet. This could be initiated from framework through exposed
callbacks. This issue can be fixed by having a flag to indicate
completion of device probe. Hence 'probed' flag is introduced to notify
completion of probe and runtime callbacks can check this flag before
doing any device access.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 5546e29..80c4042 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -76,6 +76,7 @@ struct hda_tegra {
 	struct clk *hda2hdmi_clk;
 	void __iomem *regs;
 	struct work_struct probe_work;
+	bool probed;
 };
 
 #ifdef CONFIG_PM
@@ -265,9 +266,11 @@ static int hda_tegra_runtime_suspend(struct device *dev)
 	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
 	struct hdac_bus *bus = azx_bus(chip);
 
-	azx_stop_chip(chip);
-	synchronize_irq(bus->irq);
-	azx_enter_link_reset(chip);
+	if (hda->probed) {
+		azx_stop_chip(chip);
+		synchronize_irq(bus->irq);
+		azx_enter_link_reset(chip);
+	}
 	hda_tegra_disable_clocks(hda);
 
 	return 0;
@@ -283,8 +286,10 @@ static int hda_tegra_runtime_resume(struct device *dev)
 	rc = hda_tegra_enable_clocks(hda);
 	if (rc != 0)
 		return rc;
-	hda_tegra_init(hda);
-	azx_init_chip(chip, 1);
+	if (hda->probed) {
+		hda_tegra_init(hda);
+		azx_init_chip(chip, 1);
+	}
 
 	return 0;
 }
@@ -527,6 +532,7 @@ static int hda_tegra_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	hda->dev = &pdev->dev;
 	chip = &hda->chip;
+	hda->probed = false;
 
 	err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1,
 			   THIS_MODULE, 0, &card);
@@ -585,6 +591,7 @@ static void hda_tegra_probe_work(struct work_struct *work)
 		goto out_free;
 
 	chip->running = 1;
+	hda->probed = true;
 	snd_hda_set_power_save(&chip->bus, power_save * 1000);
 
  out_free:
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH 7/7] ALSA: hda/tegra: add driver flag for runtime PM

[Sameer Pujar]

AZX_DCAPS_PM_RUNTIME flag is added to indicate support for runtime PM.
azx_has_pm_runtime() can be called to check if above is enabled. The
flag is put under CONFIG_PM check.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 80c4042..80b7a3b 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -521,12 +521,16 @@ MODULE_DEVICE_TABLE(of, hda_tegra_match);
 
 static int hda_tegra_probe(struct platform_device *pdev)
 {
-	const unsigned int driver_flags = AZX_DCAPS_CORBRP_SELF_CLEAR;
+	unsigned int driver_flags = AZX_DCAPS_CORBRP_SELF_CLEAR;
 	struct snd_card *card;
 	struct azx *chip;
 	struct hda_tegra *hda;
 	int err;
 
+#ifdef CONFIG_PM
+	driver_flags |= AZX_DCAPS_PM_RUNTIME;
+#endif
+
 	hda = devm_kzalloc(&pdev->dev, sizeof(*hda), GFP_KERNEL);
 	if (!hda)
 		return -ENOMEM;
-- 
2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 6/7] ALSA: hda/tegra: fix kernel panic

[Takashi Iwai]

On Mon, 21 Jan 2019 18:41:36 +0100,
Sameer Pujar wrote:
> 
> Kernel panic is seen during device boot. It appears that before the
> probe completes, runtime callbacks happen and the device setup is not
> done yet. This could be initiated from framework through exposed
> callbacks. This issue can be fixed by having a flag to indicate
> completion of device probe. Hence 'probed' flag is introduced to notify
> completion of probe and runtime callbacks can check this flag before
> doing any device access.

Such a fix should be rather folded into the previous, especially if
you know it's already broken :)

And, IMO, it's better to put such a check into runtime_idle callback
instead of doing in each (runtime_)suspend/resume callback.


thanks,

Takashi

Re: [PATCH 7/7] ALSA: hda/tegra: add driver flag for runtime PM

[Takashi Iwai]

On Mon, 21 Jan 2019 18:41:37 +0100,
Sameer Pujar wrote:
> 
> AZX_DCAPS_PM_RUNTIME flag is added to indicate support for runtime PM.
> azx_has_pm_runtime() can be called to check if above is enabled. The
> flag is put under CONFIG_PM check.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
> Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>

This can be more simply done in hda_controller.h.  Namely, a change
like:

--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -49,7 +49,11 @@
 #define AZX_DCAPS_4K_BDLE_BOUNDARY (1 << 23)	/* BDLE in 4k boundary */
 /* 24 unused */
 #define AZX_DCAPS_COUNT_LPIB_DELAY  (1 << 25)	/* Take LPIB as delay */
+#ifdef CONFIG_PM
 #define AZX_DCAPS_PM_RUNTIME	(1 << 26)	/* runtime PM support */
+#else
+#define AZX_DCAPS_PM_RUNTIME	0		/* N/A */
+#endif
 /* 27 unused */
 #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)	/* CORBRP clears itself after reset */
 #define AZX_DCAPS_NO_MSI64      (1 << 29)	/* Stick to 32-bit MSIs */


thanks,

Takashi

Re: [PATCH 7/7] ALSA: hda/tegra: add driver flag for runtime PM

[Sameer Pujar]

On 1/22/2019 3:01 AM, Takashi Iwai wrote:
> On Mon, 21 Jan 2019 18:41:37 +0100,
> Sameer Pujar wrote:
>> AZX_DCAPS_PM_RUNTIME flag is added to indicate support for runtime PM.
>> azx_has_pm_runtime() can be called to check if above is enabled. The
>> flag is put under CONFIG_PM check.
>>
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
>> Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
> This can be more simply done in hda_controller.h.  Namely, a change
> like:
>
> --- a/sound/pci/hda/hda_controller.h
> +++ b/sound/pci/hda/hda_controller.h
> @@ -49,7 +49,11 @@
>   #define AZX_DCAPS_4K_BDLE_BOUNDARY (1 << 23)	/* BDLE in 4k boundary */
>   /* 24 unused */
>   #define AZX_DCAPS_COUNT_LPIB_DELAY  (1 << 25)	/* Take LPIB as delay */
> +#ifdef CONFIG_PM
>   #define AZX_DCAPS_PM_RUNTIME	(1 << 26)	/* runtime PM support */
> +#else
> +#define AZX_DCAPS_PM_RUNTIME	0		/* N/A */
> +#endif
>   /* 27 unused */
>   #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)	/* CORBRP clears itself after reset */
>   #define AZX_DCAPS_NO_MSI64      (1 << 29)	/* Stick to 32-bit MSIs */

In one of the patches in the series, I am using azx_has_pm_runtime() to 
forbid runtime PM calls.
I can use above as you suggested. Along with this, I would need to 
populate chip->driver_caps with
above flag. Something like below,

--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -521,7 +521,8 @@ MODULE_DEVICE_TABLE(of, hda_tegra_match);

  static int hda_tegra_probe(struct platform_device *pdev)
  {
-       unsigned int driver_flags = AZX_DCAPS_CORBRP_SELF_CLEAR;
+       unsigned int driver_flags = AZX_DCAPS_CORBRP_SELF_CLEAR |
+                                   AZX_DCAPS_PM_RUNTIME;
         struct snd_card *card;
         struct azx *chip;
         struct hda_tegra *hda;

Thanks,
Sameer.

>
> thanks,
>
> Takashi
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 6/7] ALSA: hda/tegra: fix kernel panic

[Sameer Pujar]

On 1/22/2019 2:58 AM, Takashi Iwai wrote:
> On Mon, 21 Jan 2019 18:41:36 +0100,
> Sameer Pujar wrote:
>> Kernel panic is seen during device boot. It appears that before the
>> probe completes, runtime callbacks happen and the device setup is not
>> done yet. This could be initiated from framework through exposed
>> callbacks. This issue can be fixed by having a flag to indicate
>> completion of device probe. Hence 'probed' flag is introduced to notify
>> completion of probe and runtime callbacks can check this flag before
>> doing any device access.
> Such a fix should be rather folded into the previous, especially if
> you know it's already broken :)
Ok, I will meld this with previous commit.
>
> And, IMO, it's better to put such a check into runtime_idle callback
> instead of doing in each (runtime_)suspend/resume callback.
Check in runtime_idle would take care of suspend path. But how the check in
runtime_resume can be avoided? Kernel panic happened during 
runtime_resume().
>
>
> thanks,
>
> Takashi

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 6/7] ALSA: hda/tegra: fix kernel panic

[Takashi Iwai]

On Tue, 22 Jan 2019 04:41:16 +0100,
Sameer Pujar wrote:
> 
> 
> On 1/22/2019 2:58 AM, Takashi Iwai wrote:
> > On Mon, 21 Jan 2019 18:41:36 +0100,
> > Sameer Pujar wrote:
> >> Kernel panic is seen during device boot. It appears that before the
> >> probe completes, runtime callbacks happen and the device setup is not
> >> done yet. This could be initiated from framework through exposed
> >> callbacks. This issue can be fixed by having a flag to indicate
> >> completion of device probe. Hence 'probed' flag is introduced to notify
> >> completion of probe and runtime callbacks can check this flag before
> >> doing any device access.
> > Such a fix should be rather folded into the previous, especially if
> > you know it's already broken :)
> Ok, I will meld this with previous commit.
> >
> > And, IMO, it's better to put such a check into runtime_idle callback
> > instead of doing in each (runtime_)suspend/resume callback.
> Check in runtime_idle would take care of suspend path. But how the check in
> runtime_resume can be avoided? Kernel panic happened during
> runtime_resume().

Hm, right, we need the check in both places, since runtime PM is
called from the system PM, too.

BTW, I guess we can check chip->running instead of adding a new flag.


thanks,

Takashi

Re: [PATCH 7/7] ALSA: hda/tegra: add driver flag for runtime PM

[Takashi Iwai]

On Tue, 22 Jan 2019 03:04:21 +0100,
Sameer Pujar wrote:
> 
> 
> On 1/22/2019 3:01 AM, Takashi Iwai wrote:
> > On Mon, 21 Jan 2019 18:41:37 +0100,
> > Sameer Pujar wrote:
> >> AZX_DCAPS_PM_RUNTIME flag is added to indicate support for runtime PM.
> >> azx_has_pm_runtime() can be called to check if above is enabled. The
> >> flag is put under CONFIG_PM check.
> >>
> >> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> >> Reviewed-by: Ravindra Lokhande <rlokhande@nvidia.com>
> >> Reviewed-by: Mohan Kumar D <mkumard@nvidia.com>
> > This can be more simply done in hda_controller.h.  Namely, a change
> > like:
> >
> > --- a/sound/pci/hda/hda_controller.h
> > +++ b/sound/pci/hda/hda_controller.h
> > @@ -49,7 +49,11 @@
> >   #define AZX_DCAPS_4K_BDLE_BOUNDARY (1 << 23)	/* BDLE in 4k boundary */
> >   /* 24 unused */
> >   #define AZX_DCAPS_COUNT_LPIB_DELAY  (1 << 25)	/* Take LPIB as delay */
> > +#ifdef CONFIG_PM
> >   #define AZX_DCAPS_PM_RUNTIME	(1 << 26)	/* runtime PM support */
> > +#else
> > +#define AZX_DCAPS_PM_RUNTIME	0		/* N/A */
> > +#endif
> >   /* 27 unused */
> >   #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28)	/* CORBRP clears itself after reset */
> >   #define AZX_DCAPS_NO_MSI64      (1 << 29)	/* Stick to 32-bit MSIs */
> 
> In one of the patches in the series, I am using azx_has_pm_runtime()
> to forbid runtime PM calls.
> I can use above as you suggested. Along with this, I would need to
> populate chip->driver_caps with
> above flag. Something like below,
> 
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -521,7 +521,8 @@ MODULE_DEVICE_TABLE(of, hda_tegra_match);
> 
>  static int hda_tegra_probe(struct platform_device *pdev)
>  {
> -       unsigned int driver_flags = AZX_DCAPS_CORBRP_SELF_CLEAR;
> +       unsigned int driver_flags = AZX_DCAPS_CORBRP_SELF_CLEAR |
> +                                   AZX_DCAPS_PM_RUNTIME;
>         struct snd_card *card;
>         struct azx *chip;
>         struct hda_tegra *hda;

I see.  On the second thought, the ifdef CONFIG_PM is likely
superfluous.  The whole pm_runtime_*() calls become no-ops without
CONFIG_PM, so the compiler would optimize in anyway.

That said, I'd drop ugly ifdef but unconditionally add
AZX_DCAPS_PM_RUNTIME in an oneliner above.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel