Commit 029d92c2 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

ALSA: hda: Refactor display power management 

The current HD-audio code manages the DRM audio power via too complex
redirections, and this seems even still unbalanced in a corner case as
Intel DRM CI has been intermittently reporting.  This patch is a big
surgery for addressing the complexity and the possible unbalance.

Basically the patch changes the display PM in the following ways:

- Both HD-audio controller and codec drivers call a single helper,
  snd_hdac_display_power().  (Formerly, the display power control from
  a codec was done indirectly via link_power bus ops.)

- snd_hdac_display_power() receives the codec address index.  For
  turning on/off from the controller, pass HDA_CODEC_IDX_CONTROLLER.

- snd_hdac_display_power() doesn't manage refcounts any longer, but
  keeps the power status in bitmap.  If any of controller or codecs is
  turned on, the function updates the DRM power state via get_power()
  or put_power().

Also this refactor allows us more cleanup:

- The link_power bus ops is dropped, so there is no longer indirect
  management, as mentioned in the above.

- hdac_device link_power_control flag is moved to hda_codec
  display_power_control flag, as it's only for HDA legacy.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106525


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 3baffc4a

include/sound/hda_codec.h

+1 −0
Original line number Diff line number Diff line
@@ -236,6 +236,7 @@ struct hda_codec { 
	/* misc flags */

	unsigned int in_freeing:1; /* being released */

	unsigned int registered:1; /* codec was registered */

	unsigned int display_power_control:1; /* needs display power */

	unsigned int spdif_status_reset :1; /* needs to toggle SPDIF for each

					     * status change

					     * (e.g. Realtek codecs)

include/sound/hda_component.h

+8 −2
Original line number Diff line number Diff line
@@ -5,10 +5,15 @@ 
#define __SOUND_HDA_COMPONENT_H



#include <drm/drm_audio_component.h>

#include <sound/hdaudio.h>



/* virtual idx for controller */

#define HDA_CODEC_IDX_CONTROLLER	HDA_MAX_CODECS



#ifdef CONFIG_SND_HDA_COMPONENT

int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable);

int snd_hdac_display_power(struct hdac_bus *bus, bool enable);

int snd_hdac_display_power(struct hdac_bus *bus, unsigned int idx,

			   bool enable);

int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,

			     int dev_id, int rate);

int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
@@ -25,7 +30,8 @@ static inline int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable) 
{

	return 0;

}

static inline int snd_hdac_display_power(struct hdac_bus *bus, bool enable)

static inline int snd_hdac_display_power(struct hdac_bus *bus,

					 unsigned int idx, bool enable)

{

	return 0;

}

include/sound/hdaudio.h

+2 −5
Original line number Diff line number Diff line
@@ -79,7 +79,6 @@ struct hdac_device { 


	/* misc flags */

	atomic_t in_pm;		/* suspend/resume being performed */

	bool  link_power_control:1;



	/* sysfs */

	struct hdac_widget_tree *widgets;
@@ -237,8 +236,6 @@ struct hdac_bus_ops { 
	/* get a response from the last command */

	int (*get_response)(struct hdac_bus *bus, unsigned int addr,

			    unsigned int *res);

	/* control the link power  */

	int (*link_power)(struct hdac_bus *bus, bool enable);

};



/*
@@ -363,7 +360,8 @@ struct hdac_bus { 


	/* DRM component interface */

	struct drm_audio_component *audio_component;

	int drm_power_refcount;

	long display_power_status;

	bool display_power_active;



	/* parameters required for enhanced capabilities */

	int num_streams;
@@ -404,7 +402,6 @@ int snd_hdac_bus_send_cmd(struct hdac_bus *bus, unsigned int val); 
int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,

			      unsigned int *res);

int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus);

int snd_hdac_link_power(struct hdac_device *codec, bool enable);



bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset);

void snd_hdac_bus_stop_chip(struct hdac_bus *bus);

sound/hda/hdac_component.c

+22 −13
Original line number Diff line number Diff line
@@ -54,38 +54,45 @@ EXPORT_SYMBOL_GPL(snd_hdac_set_codec_wakeup); 
/**

 * snd_hdac_display_power - Power up / down the power refcount

 * @bus: HDA core bus

 * @idx: HDA codec address, pass HDA_CODEC_IDX_CONTROLLER for controller

 * @enable: power up or down

 *

 * This function is supposed to be used only by a HD-audio controller

 * driver that needs the interaction with graphics driver.

 * This function is used by either HD-audio controller or codec driver that

 * needs the interaction with graphics driver.

 *

 * This function manages a refcount and calls the get_power() and

 * This function updates the power status, and calls the get_power() and

 * put_power() ops accordingly, toggling the codec wakeup, too.

 *

 * Returns zero for success or a negative error code.

 */

int snd_hdac_display_power(struct hdac_bus *bus, bool enable)

int snd_hdac_display_power(struct hdac_bus *bus, unsigned int idx, bool enable)

{

	struct drm_audio_component *acomp = bus->audio_component;



	if (!acomp || !acomp->ops)

		return -ENODEV;



	dev_dbg(bus->dev, "display power %s\n",

		enable ? "enable" : "disable");

	if (enable)

		set_bit(idx, &bus->display_power_status);

	else

		clear_bit(idx, &bus->display_power_status);



	if (enable) {

		if (!bus->drm_power_refcount++) {

	if (!acomp || !acomp->ops)

		return 0;



	if (bus->display_power_status) {

		if (!bus->display_power_active) {

			if (acomp->ops->get_power)

				acomp->ops->get_power(acomp->dev);

			snd_hdac_set_codec_wakeup(bus, true);

			snd_hdac_set_codec_wakeup(bus, false);

			bus->display_power_active = true;

		}

	} else {

		WARN_ON(!bus->drm_power_refcount);

		if (!--bus->drm_power_refcount)

		if (bus->display_power_active) {

			if (acomp->ops->put_power)

				acomp->ops->put_power(acomp->dev);

			bus->display_power_active = false;

		}

	}



	return 0;
@@ -321,10 +328,12 @@ int snd_hdac_acomp_exit(struct hdac_bus *bus) 
	if (!acomp)

		return 0;



	WARN_ON(bus->drm_power_refcount);

	if (bus->drm_power_refcount > 0 && acomp->ops)

	if (WARN_ON(bus->display_power_active) && acomp->ops)

		acomp->ops->put_power(acomp->dev);



	bus->display_power_active = false;

	bus->display_power_status = 0;



	component_master_del(dev, &hdac_component_master_ops);



	bus->audio_component = NULL;

sound/hda/hdac_device.c

+0 −17
Original line number Diff line number Diff line
@@ -622,23 +622,6 @@ int snd_hdac_power_down_pm(struct hdac_device *codec) 
EXPORT_SYMBOL_GPL(snd_hdac_power_down_pm);

#endif



/**

 * snd_hdac_link_power - Enable/disable the link power for a codec

 * @codec: the codec object

 * @bool: enable or disable the link power

 */

int snd_hdac_link_power(struct hdac_device *codec, bool enable)

{

	if  (!codec->link_power_control)

		return 0;



	if  (codec->bus->ops->link_power)

		return codec->bus->ops->link_power(codec->bus, enable);

	else

		return -EINVAL;

}

EXPORT_SYMBOL_GPL(snd_hdac_link_power);



/* codec vendor labels */

struct hda_vendor_id {

	unsigned int id;

CRA Git | Maintained and supported by SUSTech CRA and CCSE