All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Make the audio component binding more generic
@ 2018-07-16 13:48 Takashi Iwai
  2018-07-16 13:48 ` [PATCH 1/3] drm/i915: Split audio component to a generic type Takashi Iwai
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 13:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Jim Qu, intel-gfx, dri-devel

Hi,

this is a preliminiary patch set to convert the existing i915 /
HD-audio component binding to be applicable to other drivers like
radeon / amdgpu.  This patchset itself doesn't change the
functionality but only renames and split to a new drm_audio_component
stuff from i915_audio_component.

The actual usage of the new API will follow once after this one gets
reviewed / accepted.  The whole patches (including this patchset) are
found in topic/hda-acomp branch of sound.git tree.

BTW, since the whole stuff is about the audio binding, I suppose these
will go through sound git tree.  Let me know if anyone has concerns.


Thanks!

Takashi

===

Takashi Iwai (3):
  drm/i915: Split audio component to a generic type
  ALSA: hda/i915: Associate audio component with devres
  ALSA: hda: Make audio component support more generic

 drivers/gpu/drm/i915/Kconfig       |   1 +
 drivers/gpu/drm/i915/intel_audio.c |  22 +-
 include/drm/drm_audio_component.h  | 138 ++++++++++++
 include/drm/i915_component.h       |  85 +-------
 include/sound/hda_component.h      |  61 ++++++
 include/sound/hda_i915.h           |  37 +---
 include/sound/hdaudio.h            |   6 +-
 sound/hda/Kconfig                  |   7 +-
 sound/hda/Makefile                 |   1 +
 sound/hda/hdac_component.c         | 332 ++++++++++++++++++++++++++++
 sound/hda/hdac_i915.c              | 335 ++---------------------------
 sound/pci/hda/patch_hdmi.c         |  57 +++--
 sound/soc/codecs/hdac_hdmi.c       |  10 +-
 13 files changed, 624 insertions(+), 468 deletions(-)
 create mode 100644 include/drm/drm_audio_component.h
 create mode 100644 include/sound/hda_component.h
 create mode 100644 sound/hda/hdac_component.c

-- 
2.18.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] drm/i915: Split audio component to a generic type
  2018-07-16 13:48 [PATCH 0/3] Make the audio component binding more generic Takashi Iwai
@ 2018-07-16 13:48 ` Takashi Iwai
  2018-07-16 17:22   ` [Intel-gfx] " Rodrigo Vivi
  2018-07-16 13:48 ` [PATCH 2/3] ALSA: hda/i915: Associate audio component with devres Takashi Iwai
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 13:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Jim Qu, intel-gfx, dri-devel

For allowing other drivers to use the DRM audio component, rename the
i915_audio_component_* with drm_audio_component_*, and split the
generic part into drm_audio_component.h.  The i915 specific stuff
remains in struct i915_audio_component, which contains
drm_audio_component as the base.

This is a preliminary change for further development, and no
functional changes by this patch itself, merely code-split and
renames.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/intel_audio.c |  22 +++---
 include/drm/drm_audio_component.h  | 115 +++++++++++++++++++++++++++++
 include/drm/i915_component.h       |  85 ++-------------------
 include/sound/hda_i915.h           |   6 +-
 include/sound/hdaudio.h            |   6 +-
 sound/hda/hdac_i915.c              |  40 +++++-----
 sound/pci/hda/patch_hdmi.c         |   8 +-
 sound/soc/codecs/hdac_hdmi.c       |   2 +-
 8 files changed, 164 insertions(+), 120 deletions(-)
 create mode 100644 include/drm/drm_audio_component.h

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..7dd5605d94ae 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -639,11 +639,12 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
 	dev_priv->av_enc_map[pipe] = encoder;
 	mutex_unlock(&dev_priv->av_mutex);
 
-	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+	if (acomp && acomp->base.audio_ops &&
+	    acomp->base.audio_ops->pin_eld_notify) {
 		/* audio drivers expect pipe = -1 to indicate Non-MST cases */
 		if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
 			pipe = -1;
-		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+		acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
 						 (int) port, (int) pipe);
 	}
 
@@ -681,11 +682,12 @@ void intel_audio_codec_disable(struct intel_encoder *encoder,
 	dev_priv->av_enc_map[pipe] = NULL;
 	mutex_unlock(&dev_priv->av_mutex);
 
-	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+	if (acomp && acomp->base.audio_ops &&
+	    acomp->base.audio_ops->pin_eld_notify) {
 		/* audio drivers expect pipe = -1 to indicate Non-MST cases */
 		if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
 			pipe = -1;
-		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+		acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
 						 (int) port, (int) pipe);
 	}
 
@@ -880,7 +882,7 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
 	return ret;
 }
 
-static const struct i915_audio_component_ops i915_audio_component_ops = {
+static const struct drm_audio_component_ops i915_audio_component_ops = {
 	.owner		= THIS_MODULE,
 	.get_power	= i915_audio_component_get_power,
 	.put_power	= i915_audio_component_put_power,
@@ -897,12 +899,12 @@ static int i915_audio_component_bind(struct device *i915_kdev,
 	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
 	int i;
 
-	if (WARN_ON(acomp->ops || acomp->dev))
+	if (WARN_ON(acomp->base.ops || acomp->base.dev))
 		return -EEXIST;
 
 	drm_modeset_lock_all(&dev_priv->drm);
-	acomp->ops = &i915_audio_component_ops;
-	acomp->dev = i915_kdev;
+	acomp->base.ops = &i915_audio_component_ops;
+	acomp->base.dev = i915_kdev;
 	BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
 	for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
 		acomp->aud_sample_rate[i] = 0;
@@ -919,8 +921,8 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
 	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
 
 	drm_modeset_lock_all(&dev_priv->drm);
-	acomp->ops = NULL;
-	acomp->dev = NULL;
+	acomp->base.ops = NULL;
+	acomp->base.dev = NULL;
 	dev_priv->audio_component = NULL;
 	drm_modeset_unlock_all(&dev_priv->drm);
 }
diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h
new file mode 100644
index 000000000000..f310b28404e8
--- /dev/null
+++ b/include/drm/drm_audio_component.h
@@ -0,0 +1,115 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef _DRM_AUDIO_COMPONENT_H_
+#define _DRM_AUDIO_COMPONENT_H_
+
+/**
+ * struct drm_audio_component_ops - Ops implemented by DRM driver, called by hda driver
+ */
+struct drm_audio_component_ops {
+	/**
+	 * @owner: i915 module
+	 */
+	struct module *owner;
+	/**
+	 * @get_power: get the POWER_DOMAIN_AUDIO power well
+	 *
+	 * Request the power well to be turned on.
+	 */
+	void (*get_power)(struct device *);
+	/**
+	 * @put_power: put the POWER_DOMAIN_AUDIO power well
+	 *
+	 * Allow the power well to be turned off.
+	 */
+	void (*put_power)(struct device *);
+	/**
+	 * @codec_wake_override: Enable/disable codec wake signal
+	 */
+	void (*codec_wake_override)(struct device *, bool enable);
+	/**
+	 * @get_cdclk_freq: Get the Core Display Clock in kHz
+	 */
+	int (*get_cdclk_freq)(struct device *);
+	/**
+	 * @sync_audio_rate: set n/cts based on the sample rate
+	 *
+	 * Called from audio driver. After audio driver sets the
+	 * sample rate, it will call this function to set n/cts
+	 */
+	int (*sync_audio_rate)(struct device *, int port, int pipe, int rate);
+	/**
+	 * @get_eld: fill the audio state and ELD bytes for the given port
+	 *
+	 * Called from audio driver to get the HDMI/DP audio state of the given
+	 * digital port, and also fetch ELD bytes to the given pointer.
+	 *
+	 * It returns the byte size of the original ELD (not the actually
+	 * copied size), zero for an invalid ELD, or a negative error code.
+	 *
+	 * Note that the returned size may be over @max_bytes.  Then it
+	 * implies that only a part of ELD has been copied to the buffer.
+	 */
+	int (*get_eld)(struct device *, int port, int pipe, bool *enabled,
+		       unsigned char *buf, int max_bytes);
+};
+
+/**
+ * struct drm_audio_component_audio_ops - Ops implemented by hda driver, called by DRM driver
+ */
+struct drm_audio_component_audio_ops {
+	/**
+	 * @audio_ptr: Pointer to be used in call to pin_eld_notify
+	 */
+	void *audio_ptr;
+	/**
+	 * @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD information has changed
+	 *
+	 * Called when the DRM driver has set up audio pipeline or has just
+	 * begun to tear it down. This allows the HDA driver to update its
+	 * status accordingly (even when the HDA controller is in power save
+	 * mode).
+	 */
+	void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
+};
+
+/**
+ * struct drm_audio_component - Used for direct communication between DRM and hda drivers
+ */
+struct drm_audio_component {
+	/**
+	 * @dev: DRM device, used as parameter for ops
+	 */
+	struct device *dev;
+	/**
+	 * @ops: Ops implemented by DRM driver, called by hda driver
+	 */
+	const struct drm_audio_component_ops *ops;
+	/**
+	 * @audio_ops: Ops implemented by hda driver, called by DRM driver
+	 */
+	const struct drm_audio_component_audio_ops *audio_ops;
+};
+
+#endif /* _DRM_AUDIO_COMPONENT_H_ */
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index 346b1f5cb180..fca22d463e1b 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -24,101 +24,26 @@
 #ifndef _I915_COMPONENT_H_
 #define _I915_COMPONENT_H_
 
+#include "drm_audio_component.h"
+
 /* MAX_PORT is the number of port
  * It must be sync with I915_MAX_PORTS defined i915_drv.h
  */
 #define MAX_PORTS 6
 
-/**
- * struct i915_audio_component_ops - Ops implemented by i915 driver, called by hda driver
- */
-struct i915_audio_component_ops {
-	/**
-	 * @owner: i915 module
-	 */
-	struct module *owner;
-	/**
-	 * @get_power: get the POWER_DOMAIN_AUDIO power well
-	 *
-	 * Request the power well to be turned on.
-	 */
-	void (*get_power)(struct device *);
-	/**
-	 * @put_power: put the POWER_DOMAIN_AUDIO power well
-	 *
-	 * Allow the power well to be turned off.
-	 */
-	void (*put_power)(struct device *);
-	/**
-	 * @codec_wake_override: Enable/disable codec wake signal
-	 */
-	void (*codec_wake_override)(struct device *, bool enable);
-	/**
-	 * @get_cdclk_freq: Get the Core Display Clock in kHz
-	 */
-	int (*get_cdclk_freq)(struct device *);
-	/**
-	 * @sync_audio_rate: set n/cts based on the sample rate
-	 *
-	 * Called from audio driver. After audio driver sets the
-	 * sample rate, it will call this function to set n/cts
-	 */
-	int (*sync_audio_rate)(struct device *, int port, int pipe, int rate);
-	/**
-	 * @get_eld: fill the audio state and ELD bytes for the given port
-	 *
-	 * Called from audio driver to get the HDMI/DP audio state of the given
-	 * digital port, and also fetch ELD bytes to the given pointer.
-	 *
-	 * It returns the byte size of the original ELD (not the actually
-	 * copied size), zero for an invalid ELD, or a negative error code.
-	 *
-	 * Note that the returned size may be over @max_bytes.  Then it
-	 * implies that only a part of ELD has been copied to the buffer.
-	 */
-	int (*get_eld)(struct device *, int port, int pipe, bool *enabled,
-		       unsigned char *buf, int max_bytes);
-};
-
-/**
- * struct i915_audio_component_audio_ops - Ops implemented by hda driver, called by i915 driver
- */
-struct i915_audio_component_audio_ops {
-	/**
-	 * @audio_ptr: Pointer to be used in call to pin_eld_notify
-	 */
-	void *audio_ptr;
-	/**
-	 * @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD information has changed
-	 *
-	 * Called when the i915 driver has set up audio pipeline or has just
-	 * begun to tear it down. This allows the HDA driver to update its
-	 * status accordingly (even when the HDA controller is in power save
-	 * mode).
-	 */
-	void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
-};
-
 /**
  * struct i915_audio_component - Used for direct communication between i915 and hda drivers
  */
 struct i915_audio_component {
 	/**
-	 * @dev: i915 device, used as parameter for ops
+	 * @base: the drm_audio_component base class
 	 */
-	struct device *dev;
+	struct drm_audio_component	base;
+
 	/**
 	 * @aud_sample_rate: the array of audio sample rate per port
 	 */
 	int aud_sample_rate[MAX_PORTS];
-	/**
-	 * @ops: Ops implemented by i915 driver, called by hda driver
-	 */
-	const struct i915_audio_component_ops *ops;
-	/**
-	 * @audio_ops: Ops implemented by hda driver, called by i915 driver
-	 */
-	const struct i915_audio_component_audio_ops *audio_ops;
 };
 
 #endif /* _I915_COMPONENT_H_ */
diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
index a94f5b6f92ac..f69ea84e7b65 100644
--- a/include/sound/hda_i915.h
+++ b/include/sound/hda_i915.h
@@ -5,7 +5,7 @@
 #ifndef __SOUND_HDA_I915_H
 #define __SOUND_HDA_I915_H
 
-#include <drm/i915_component.h>
+#include <drm/drm_audio_component.h>
 
 #ifdef CONFIG_SND_HDA_I915
 int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable);
@@ -17,7 +17,7 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
 			   bool *audio_enabled, char *buffer, int max_bytes);
 int snd_hdac_i915_init(struct hdac_bus *bus);
 int snd_hdac_i915_exit(struct hdac_bus *bus);
-int snd_hdac_i915_register_notifier(const struct i915_audio_component_audio_ops *);
+int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *);
 #else
 static inline int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
 {
@@ -49,7 +49,7 @@ static inline int snd_hdac_i915_exit(struct hdac_bus *bus)
 {
 	return 0;
 }
-static inline int snd_hdac_i915_register_notifier(const struct i915_audio_component_audio_ops *ops)
+static inline int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *ops)
 {
 	return -ENODEV;
 }
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index f1baaa88e766..ab5ee3ef2198 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -333,9 +333,9 @@ struct hdac_bus {
 	spinlock_t reg_lock;
 	struct mutex cmd_mutex;
 
-	/* i915 component interface */
-	struct i915_audio_component *audio_component;
-	int i915_power_refcount;
+	/* DRM component interface */
+	struct drm_audio_component *audio_component;
+	int drm_power_refcount;
 
 	/* parameters required for enhanced capabilities */
 	int num_streams;
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index cbe818eda336..1a88c1aaf9bb 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -16,13 +16,13 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/component.h>
-#include <drm/i915_component.h>
+#include <drm/drm_audio_component.h>
 #include <sound/core.h>
 #include <sound/hdaudio.h>
 #include <sound/hda_i915.h>
 #include <sound/hda_register.h>
 
-static struct i915_audio_component *hdac_acomp;
+static struct drm_audio_component *hdac_acomp;
 
 /**
  * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
@@ -39,7 +39,7 @@ static struct i915_audio_component *hdac_acomp;
  */
 int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
 {
-	struct i915_audio_component *acomp = bus->audio_component;
+	struct drm_audio_component *acomp = bus->audio_component;
 
 	if (!acomp || !acomp->ops)
 		return -ENODEV;
@@ -74,7 +74,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_set_codec_wakeup);
  */
 int snd_hdac_display_power(struct hdac_bus *bus, bool enable)
 {
-	struct i915_audio_component *acomp = bus->audio_component;
+	struct drm_audio_component *acomp = bus->audio_component;
 
 	if (!acomp || !acomp->ops)
 		return -ENODEV;
@@ -83,14 +83,14 @@ int snd_hdac_display_power(struct hdac_bus *bus, bool enable)
 		enable ? "enable" : "disable");
 
 	if (enable) {
-		if (!bus->i915_power_refcount++) {
+		if (!bus->drm_power_refcount++) {
 			acomp->ops->get_power(acomp->dev);
 			snd_hdac_set_codec_wakeup(bus, true);
 			snd_hdac_set_codec_wakeup(bus, false);
 		}
 	} else {
-		WARN_ON(!bus->i915_power_refcount);
-		if (!--bus->i915_power_refcount)
+		WARN_ON(!bus->drm_power_refcount);
+		if (!--bus->drm_power_refcount)
 			acomp->ops->put_power(acomp->dev);
 	}
 
@@ -119,7 +119,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_display_power);
  */
 void snd_hdac_i915_set_bclk(struct hdac_bus *bus)
 {
-	struct i915_audio_component *acomp = bus->audio_component;
+	struct drm_audio_component *acomp = bus->audio_component;
 	struct pci_dev *pci = to_pci_dev(bus->dev);
 	int cdclk_freq;
 	unsigned int bclk_m, bclk_n;
@@ -206,7 +206,7 @@ int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
 			     int dev_id, int rate)
 {
 	struct hdac_bus *bus = codec->bus;
-	struct i915_audio_component *acomp = bus->audio_component;
+	struct drm_audio_component *acomp = bus->audio_component;
 	int port, pipe;
 
 	if (!acomp || !acomp->ops || !acomp->ops->sync_audio_rate)
@@ -244,7 +244,7 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
 			   bool *audio_enabled, char *buffer, int max_bytes)
 {
 	struct hdac_bus *bus = codec->bus;
-	struct i915_audio_component *acomp = bus->audio_component;
+	struct drm_audio_component *acomp = bus->audio_component;
 	int port, pipe;
 
 	if (!acomp || !acomp->ops || !acomp->ops->get_eld)
@@ -262,7 +262,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
 
 static int hdac_component_master_bind(struct device *dev)
 {
-	struct i915_audio_component *acomp = hdac_acomp;
+	struct drm_audio_component *acomp = hdac_acomp;
 	int ret;
 
 	ret = component_bind_all(dev, acomp);
@@ -294,7 +294,7 @@ static int hdac_component_master_bind(struct device *dev)
 
 static void hdac_component_master_unbind(struct device *dev)
 {
-	struct i915_audio_component *acomp = hdac_acomp;
+	struct drm_audio_component *acomp = hdac_acomp;
 
 	module_put(acomp->ops->owner);
 	component_unbind_all(dev, acomp);
@@ -323,7 +323,7 @@ static int hdac_component_master_match(struct device *dev, void *data)
  *
  * Returns zero for success or a negative error code.
  */
-int snd_hdac_i915_register_notifier(const struct i915_audio_component_audio_ops *aops)
+int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *aops)
 {
 	if (!hdac_acomp)
 		return -ENODEV;
@@ -361,7 +361,8 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
 {
 	struct component_match *match = NULL;
 	struct device *dev = bus->dev;
-	struct i915_audio_component *acomp;
+	struct i915_audio_component *i915_acomp;
+	struct drm_audio_component *acomp;
 	int ret;
 
 	if (WARN_ON(hdac_acomp))
@@ -370,9 +371,10 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
 	if (!i915_gfx_present())
 		return -ENODEV;
 
-	acomp = kzalloc(sizeof(*acomp), GFP_KERNEL);
-	if (!acomp)
+	i915_acomp = kzalloc(sizeof(*i915_acomp), GFP_KERNEL);
+	if (!i915_acomp)
 		return -ENOMEM;
+	acomp = &i915_acomp->base;
 	bus->audio_component = acomp;
 	hdac_acomp = acomp;
 
@@ -421,13 +423,13 @@ EXPORT_SYMBOL_GPL(snd_hdac_i915_init);
 int snd_hdac_i915_exit(struct hdac_bus *bus)
 {
 	struct device *dev = bus->dev;
-	struct i915_audio_component *acomp = bus->audio_component;
+	struct drm_audio_component *acomp = bus->audio_component;
 
 	if (!acomp)
 		return 0;
 
-	WARN_ON(bus->i915_power_refcount);
-	if (bus->i915_power_refcount > 0 && acomp->ops)
+	WARN_ON(bus->drm_power_refcount);
+	if (bus->drm_power_refcount > 0 && acomp->ops)
 		acomp->ops->put_power(acomp->dev);
 
 	component_master_del(dev, &hdac_component_master_ops);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8a49415aebac..c0847017114c 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -177,7 +177,7 @@ struct hdmi_spec {
 
 	/* i915/powerwell (Haswell+/Valleyview+) specific */
 	bool use_acomp_notifier; /* use i915 eld_notify callback for hotplug */
-	struct i915_audio_component_audio_ops i915_audio_ops;
+	struct drm_audio_component_audio_ops drm_audio_ops;
 
 	struct hdac_chmap chmap;
 	hda_nid_t vendor_nid;
@@ -2511,14 +2511,14 @@ static void register_i915_notifier(struct hda_codec *codec)
 	struct hdmi_spec *spec = codec->spec;
 
 	spec->use_acomp_notifier = true;
-	spec->i915_audio_ops.audio_ptr = codec;
+	spec->drm_audio_ops.audio_ptr = codec;
 	/* intel_audio_codec_enable() or intel_audio_codec_disable()
 	 * will call pin_eld_notify with using audio_ptr pointer
 	 * We need make sure audio_ptr is really setup
 	 */
 	wmb();
-	spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
-	snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
+	spec->drm_audio_ops.pin_eld_notify = intel_pin_eld_notify;
+	snd_hdac_i915_register_notifier(&spec->drm_audio_ops);
 }
 
 /* setup_stream ops override for HSW+ */
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 3e3a2a9ef310..460075475f20 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1583,7 +1583,7 @@ static void hdac_hdmi_eld_notify_cb(void *aptr, int port, int pipe)
 
 }
 
-static struct i915_audio_component_audio_ops aops = {
+static struct drm_audio_component_audio_ops aops = {
 	.pin_eld_notify	= hdac_hdmi_eld_notify_cb,
 };
 
-- 
2.18.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] ALSA: hda/i915: Associate audio component with devres
  2018-07-16 13:48 [PATCH 0/3] Make the audio component binding more generic Takashi Iwai
  2018-07-16 13:48 ` [PATCH 1/3] drm/i915: Split audio component to a generic type Takashi Iwai
@ 2018-07-16 13:48 ` Takashi Iwai
  2018-07-16 13:48 ` [PATCH 3/3] ALSA: hda: Make audio component support more generic Takashi Iwai
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 13:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Jim Qu, intel-gfx, dri-devel

The HD-audio i915 binding code contains a single pointer, hdac_acomp,
for allowing the access to audio component from the master bind/unbind
callbacks.  This was needed because the callbacks pass only the device
pointer and we can't guarantee the object type assigned to the drvdata
(which is free for each controller driver implementation).
And this implementation will be a problem if we support multiple
components for different DRM drivers, not only i915.

As a solution, allocate the audio component object via devres and
associate it with the given device, so that the component callbacks
can refer to it via devres_find().

The removal of the object is still done half-manually via
devres_destroy() to make the code consistent (although it may work
without the explicit call).

Also, the snd_hda_i915_register_notifier() had the reference to
hdac_acomp as well.  In this patch, the corresponding code is removed
by passing hdac_bus object to the function, too.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/hda_i915.h     |  6 ++++--
 sound/hda/hdac_i915.c        | 34 +++++++++++++++++++++-------------
 sound/pci/hda/patch_hdmi.c   |  5 +++--
 sound/soc/codecs/hdac_hdmi.c |  2 +-
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
index f69ea84e7b65..648263791559 100644
--- a/include/sound/hda_i915.h
+++ b/include/sound/hda_i915.h
@@ -17,7 +17,8 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
 			   bool *audio_enabled, char *buffer, int max_bytes);
 int snd_hdac_i915_init(struct hdac_bus *bus);
 int snd_hdac_i915_exit(struct hdac_bus *bus);
-int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *);
+int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
+				    const struct drm_audio_component_audio_ops *ops);
 #else
 static inline int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
 {
@@ -49,7 +50,8 @@ static inline int snd_hdac_i915_exit(struct hdac_bus *bus)
 {
 	return 0;
 }
-static inline int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *ops)
+static inline int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
+						  const struct drm_audio_component_audio_ops *ops)
 {
 	return -ENODEV;
 }
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 1a88c1aaf9bb..861b77bbc7df 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -22,7 +22,14 @@
 #include <sound/hda_i915.h>
 #include <sound/hda_register.h>
 
-static struct drm_audio_component *hdac_acomp;
+static void hdac_acomp_release(struct device *dev, void *res)
+{
+}
+
+static struct drm_audio_component *hdac_get_acomp(struct device *dev)
+{
+	return devres_find(dev, hdac_acomp_release, NULL, NULL);
+}
 
 /**
  * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
@@ -262,7 +269,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
 
 static int hdac_component_master_bind(struct device *dev)
 {
-	struct drm_audio_component *acomp = hdac_acomp;
+	struct drm_audio_component *acomp = hdac_get_acomp(dev);
 	int ret;
 
 	ret = component_bind_all(dev, acomp);
@@ -294,7 +301,7 @@ static int hdac_component_master_bind(struct device *dev)
 
 static void hdac_component_master_unbind(struct device *dev)
 {
-	struct drm_audio_component *acomp = hdac_acomp;
+	struct drm_audio_component *acomp = hdac_get_acomp(dev);
 
 	module_put(acomp->ops->owner);
 	component_unbind_all(dev, acomp);
@@ -314,6 +321,7 @@ static int hdac_component_master_match(struct device *dev, void *data)
 
 /**
  * snd_hdac_i915_register_notifier - Register i915 audio component ops
+ * @bus: HDA core bus
  * @aops: i915 audio component ops
  *
  * This function is supposed to be used only by a HD-audio controller
@@ -323,12 +331,13 @@ static int hdac_component_master_match(struct device *dev, void *data)
  *
  * Returns zero for success or a negative error code.
  */
-int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *aops)
+int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
+				    const struct drm_audio_component_audio_ops *aops)
 {
-	if (!hdac_acomp)
+	if (!bus->audio_component)
 		return -ENODEV;
 
-	hdac_acomp->audio_ops = aops;
+	bus->audio_component->audio_ops = aops;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_i915_register_notifier);
@@ -365,18 +374,19 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
 	struct drm_audio_component *acomp;
 	int ret;
 
-	if (WARN_ON(hdac_acomp))
+	if (WARN_ON(hdac_get_acomp(dev)))
 		return -EBUSY;
 
 	if (!i915_gfx_present())
 		return -ENODEV;
 
-	i915_acomp = kzalloc(sizeof(*i915_acomp), GFP_KERNEL);
+	i915_acomp = devres_alloc(hdac_acomp_release, sizeof(*i915_acomp),
+				  GFP_KERNEL);
 	if (!i915_acomp)
 		return -ENOMEM;
 	acomp = &i915_acomp->base;
 	bus->audio_component = acomp;
-	hdac_acomp = acomp;
+	devres_add(dev, acomp);
 
 	component_match_add(dev, &match, hdac_component_master_match, bus);
 	ret = component_master_add_with_match(dev, &hdac_component_master_ops,
@@ -400,9 +410,8 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
 out_master_del:
 	component_master_del(dev, &hdac_component_master_ops);
 out_err:
-	kfree(acomp);
 	bus->audio_component = NULL;
-	hdac_acomp = NULL;
+	devres_destroy(dev, hdac_acomp_release, NULL, NULL);
 	dev_info(dev, "failed to add i915 component master (%d)\n", ret);
 
 	return ret;
@@ -434,9 +443,8 @@ int snd_hdac_i915_exit(struct hdac_bus *bus)
 
 	component_master_del(dev, &hdac_component_master_ops);
 
-	kfree(acomp);
 	bus->audio_component = NULL;
-	hdac_acomp = NULL;
+	devres_destroy(dev, hdac_acomp_release, NULL, NULL);
 
 	return 0;
 }
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index c0847017114c..bf174a013f2d 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2288,7 +2288,7 @@ static void generic_hdmi_free(struct hda_codec *codec)
 	int pin_idx, pcm_idx;
 
 	if (codec_has_acomp(codec))
-		snd_hdac_i915_register_notifier(NULL);
+		snd_hdac_i915_register_notifier(&codec->bus->core, NULL);
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
@@ -2518,7 +2518,8 @@ static void register_i915_notifier(struct hda_codec *codec)
 	 */
 	wmb();
 	spec->drm_audio_ops.pin_eld_notify = intel_pin_eld_notify;
-	snd_hdac_i915_register_notifier(&spec->drm_audio_ops);
+	snd_hdac_i915_register_notifier(&codec->bus->core,
+					&spec->drm_audio_ops);
 }
 
 /* setup_stream ops override for HSW+ */
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 460075475f20..2b7c33db4ded 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1812,7 +1812,7 @@ static int hdmi_codec_probe(struct snd_soc_component *component)
 		return ret;
 
 	aops.audio_ptr = hdev;
-	ret = snd_hdac_i915_register_notifier(&aops);
+	ret = snd_hdac_i915_register_notifier(hdev->bus, &aops);
 	if (ret < 0) {
 		dev_err(&hdev->dev, "notifier register failed: err: %d\n", ret);
 		return ret;
-- 
2.18.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] ALSA: hda: Make audio component support more generic
  2018-07-16 13:48 [PATCH 0/3] Make the audio component binding more generic Takashi Iwai
  2018-07-16 13:48 ` [PATCH 1/3] drm/i915: Split audio component to a generic type Takashi Iwai
  2018-07-16 13:48 ` [PATCH 2/3] ALSA: hda/i915: Associate audio component with devres Takashi Iwai
@ 2018-07-16 13:48 ` Takashi Iwai
  2018-07-16 20:33   ` Takashi Iwai
  2018-07-16 19:02 ` ✗ Fi.CI.BAT: failure for Make the audio component binding " Patchwork
  2018-07-17  6:34 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
  4 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 13:48 UTC (permalink / raw)
  To: alsa-devel; +Cc: Jim Qu, intel-gfx, dri-devel

This is the final step for more generic support of DRM audio
component.  The generic audio component code is now moved to its own
file, and the symbols are renamed from snd_hac_i915_* to
snd_hdac_acomp_*, respectively.  The generic code is enabled via the
new kconfig, CONFIG_SND_HDA_COMPONENT, while CONFIG_SND_HDA_I915 is
kept as the super-class.

Along with the split, three new callbacks are added to audio_ops:
pin2port is for providing the conversion between the pin number and
the widget id, and master_bind/master_unbin are called at binding /
unbinding the master component, respectively.  All these are optional,
but used in i915 implementation and also other later implementations.

A note about the new snd_hdac_acomp_init() function: there is a slight
difference between this and the old snd_hdac_i915_init().  The latter
(still) synchronizes with the master component binding, i.e. it
assures that the relevant DRM component gets bound when it returns, or
gives a negative error.  Meanwhile the new function doesn't
synchronize but just leaves as is.  It's the responsibility by the
caller's side to synchronize, or the caller may accept the
asynchronous binding on the fly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/Kconfig      |   1 +
 include/drm/drm_audio_component.h |  23 ++
 include/sound/hda_component.h     |  61 ++++++
 include/sound/hda_i915.h          |  39 +---
 sound/hda/Kconfig                 |   7 +-
 sound/hda/Makefile                |   1 +
 sound/hda/hdac_component.c        | 332 +++++++++++++++++++++++++++++
 sound/hda/hdac_i915.c             | 343 ++----------------------------
 sound/pci/hda/patch_hdmi.c        |  50 +++--
 sound/soc/codecs/hdac_hdmi.c      |   8 +-
 10 files changed, 483 insertions(+), 382 deletions(-)
 create mode 100644 include/sound/hda_component.h
 create mode 100644 sound/hda/hdac_component.c

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index dfd95889f4b7..5c607f2c707b 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -23,6 +23,7 @@ config DRM_I915
 	select SYNC_FILE
 	select IOSF_MBI
 	select CRC32
+	select SND_HDA_I915 if SND_HDA_CORE
 	help
 	  Choose this option if you have a system that has "Intel Graphics
 	  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h
index f310b28404e8..746a5afc3970 100644
--- a/include/drm/drm_audio_component.h
+++ b/include/drm/drm_audio_component.h
@@ -24,6 +24,8 @@
 #ifndef _DRM_AUDIO_COMPONENT_H_
 #define _DRM_AUDIO_COMPONENT_H_
 
+struct drm_audio_component;
+
 /**
  * struct drm_audio_component_ops - Ops implemented by DRM driver, called by hda driver
  */
@@ -92,6 +94,27 @@ struct drm_audio_component_audio_ops {
 	 * mode).
 	 */
 	void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
+	/**
+	 * @pin2port: Check and convert from pin node to port number
+	 *
+	 * Called by HDA driver to check and convert from the pin widget node
+	 * number to a port number in the graphics side.
+	 */
+	int (*pin2port)(void *audio_ptr, int pin);
+	/**
+	 * @master_bind: (Optional) component master bind callback
+	 *
+	 * Called at binding master component, for HDA codec-specific
+	 * handling of dynamic binding.
+	 */
+	int (*master_bind)(struct device *dev, struct drm_audio_component *);
+	/**
+	 * @master_unbind: (Optional) component master unbind callback
+	 *
+	 * Called at unbinding master component, for HDA codec-specific
+	 * handling of dynamic unbinding.
+	 */
+	void (*master_unbind)(struct device *dev, struct drm_audio_component *);
 };
 
 /**
diff --git a/include/sound/hda_component.h b/include/sound/hda_component.h
new file mode 100644
index 000000000000..78626cde7081
--- /dev/null
+++ b/include/sound/hda_component.h
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+// HD-Audio helpers to sync with DRM driver
+
+#ifndef __SOUND_HDA_COMPONENT_H
+#define __SOUND_HDA_COMPONENT_H
+
+#include <drm/drm_audio_component.h>
+
+#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_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,
+			   bool *audio_enabled, char *buffer, int max_bytes);
+int snd_hdac_acomp_init(struct hdac_bus *bus,
+			const struct drm_audio_component_audio_ops *aops,
+			int (*match_master)(struct device *, void *),
+			size_t extra_size);
+int snd_hdac_acomp_exit(struct hdac_bus *bus);
+int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,
+				    const struct drm_audio_component_audio_ops *ops);
+#else
+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)
+{
+	return 0;
+}
+static inline int snd_hdac_sync_audio_rate(struct hdac_device *codec,
+					   hda_nid_t nid, int dev_id, int rate)
+{
+	return 0;
+}
+static inline int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid,
+					 int dev_id, bool *audio_enabled,
+					 char *buffer, int max_bytes)
+{
+	return -ENODEV;
+}
+static inline int snd_hdac_acomp_init(struct hdac_bus *bus,
+				      const struct drm_audio_component_audio_ops *aops,
+				      int (*match_master)(struct device *, void *),
+				      size_t extra_size)
+{
+	return -ENODEV;
+}
+static inline int snd_hdac_acomp_exit(struct hdac_bus *bus)
+{
+	return 0;
+}
+static inline int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,
+						  const struct drm_audio_component_audio_ops *ops)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif /* __SOUND_HDA_COMPONENT_H */
diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
index 648263791559..6b79614a893b 100644
--- a/include/sound/hda_i915.h
+++ b/include/sound/hda_i915.h
@@ -5,56 +5,23 @@
 #ifndef __SOUND_HDA_I915_H
 #define __SOUND_HDA_I915_H
 
-#include <drm/drm_audio_component.h>
+#include "hda_component.h"
 
 #ifdef CONFIG_SND_HDA_I915
-int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable);
-int snd_hdac_display_power(struct hdac_bus *bus, bool enable);
 void snd_hdac_i915_set_bclk(struct hdac_bus *bus);
-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,
-			   bool *audio_enabled, char *buffer, int max_bytes);
 int snd_hdac_i915_init(struct hdac_bus *bus);
-int snd_hdac_i915_exit(struct hdac_bus *bus);
-int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
-				    const struct drm_audio_component_audio_ops *ops);
 #else
-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)
-{
-	return 0;
-}
 static inline void snd_hdac_i915_set_bclk(struct hdac_bus *bus)
 {
 }
-static inline int snd_hdac_sync_audio_rate(struct hdac_device *codec,
-					   hda_nid_t nid, int dev_id, int rate)
-{
-	return 0;
-}
-static inline int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid,
-					 int dev_id, bool *audio_enabled,
-					 char *buffer, int max_bytes)
-{
-	return -ENODEV;
-}
 static inline int snd_hdac_i915_init(struct hdac_bus *bus)
 {
 	return -ENODEV;
 }
+#endif
 static inline int snd_hdac_i915_exit(struct hdac_bus *bus)
 {
-	return 0;
+	return snd_hdac_acomp_exit(bus);
 }
-static inline int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
-						  const struct drm_audio_component_audio_ops *ops)
-{
-	return -ENODEV;
-}
-#endif
 
 #endif /* __SOUND_HDA_I915_H */
diff --git a/sound/hda/Kconfig b/sound/hda/Kconfig
index 3129546398d0..2d90e11b3eaa 100644
--- a/sound/hda/Kconfig
+++ b/sound/hda/Kconfig
@@ -5,11 +5,12 @@ config SND_HDA_CORE
 config SND_HDA_DSP_LOADER
 	bool
 
+config SND_HDA_COMPONENT
+	bool
+
 config SND_HDA_I915
 	bool
-	default y
-	depends on DRM_I915
-	depends on SND_HDA_CORE
+	select SND_HDA_COMPONENT
 
 config SND_HDA_EXT_CORE
        tristate
diff --git a/sound/hda/Makefile b/sound/hda/Makefile
index e4e726f2ce98..2160202e2dc1 100644
--- a/sound/hda/Makefile
+++ b/sound/hda/Makefile
@@ -6,6 +6,7 @@ snd-hda-core-objs += trace.o
 CFLAGS_trace.o := -I$(src)
 
 # for sync with i915 gfx driver
+snd-hda-core-$(CONFIG_SND_HDA_COMPONENT) += hdac_component.o
 snd-hda-core-$(CONFIG_SND_HDA_I915) += hdac_i915.o
 
 obj-$(CONFIG_SND_HDA_CORE) += snd-hda-core.o
diff --git a/sound/hda/hdac_component.c b/sound/hda/hdac_component.c
new file mode 100644
index 000000000000..153ba7f4b3c7
--- /dev/null
+++ b/sound/hda/hdac_component.c
@@ -0,0 +1,332 @@
+// SPDX-License-Identifier: GPL-2.0
+// hdac_component.c - routines for sync between HD-A core and DRM driver
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/component.h>
+#include <sound/core.h>
+#include <sound/hdaudio.h>
+#include <sound/hda_component.h>
+#include <sound/hda_register.h>
+
+static void hdac_acomp_release(struct device *dev, void *res)
+{
+}
+
+static struct drm_audio_component *hdac_get_acomp(struct device *dev)
+{
+	return devres_find(dev, hdac_acomp_release, NULL, NULL);
+}
+
+/**
+ * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
+ * @bus: HDA core bus
+ * @enable: enable or disable the wakeup
+ *
+ * This function is supposed to be used only by a HD-audio controller
+ * driver that needs the interaction with graphics driver.
+ *
+ * This function should be called during the chip reset, also called at
+ * resume for updating STATESTS register read.
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
+{
+	struct drm_audio_component *acomp = bus->audio_component;
+
+	if (!acomp || !acomp->ops)
+		return -ENODEV;
+
+	if (!acomp->ops->codec_wake_override)
+		return 0;
+
+	dev_dbg(bus->dev, "%s codec wakeup\n",
+		enable ? "enable" : "disable");
+
+	acomp->ops->codec_wake_override(acomp->dev, enable);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_set_codec_wakeup);
+
+/**
+ * snd_hdac_display_power - Power up / down the power refcount
+ * @bus: HDA core bus
+ * @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 manages a refcount 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)
+{
+	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) {
+		if (!bus->drm_power_refcount++) {
+			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);
+		}
+	} else {
+		WARN_ON(!bus->drm_power_refcount);
+		if (!--bus->drm_power_refcount)
+			if (acomp->ops->put_power)
+				acomp->ops->put_power(acomp->dev);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_display_power);
+
+/**
+ * snd_hdac_sync_audio_rate - Set N/CTS based on the sample rate
+ * @codec: HDA codec
+ * @nid: the pin widget NID
+ * @dev_id: device identifier
+ * @rate: the sample rate to set
+ *
+ * This function is supposed to be used only by a HD-audio controller
+ * driver that needs the interaction with graphics driver.
+ *
+ * This function sets N/CTS value based on the given sample rate.
+ * Returns zero for success, or a negative error code.
+ */
+int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
+			     int dev_id, int rate)
+{
+	struct hdac_bus *bus = codec->bus;
+	struct drm_audio_component *acomp = bus->audio_component;
+	int port, pipe;
+
+	if (!acomp || !acomp->ops || !acomp->ops->sync_audio_rate)
+		return -ENODEV;
+	port = nid;
+	if (acomp->audio_ops && acomp->audio_ops->pin2port) {
+		port = acomp->audio_ops->pin2port(codec, nid);
+		if (port < 0)
+			return -EINVAL;
+	}
+	pipe = dev_id;
+	return acomp->ops->sync_audio_rate(acomp->dev, port, pipe, rate);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_sync_audio_rate);
+
+/**
+ * snd_hdac_acomp_get_eld - Get the audio state and ELD via component
+ * @codec: HDA codec
+ * @nid: the pin widget NID
+ * @dev_id: device identifier
+ * @audio_enabled: the pointer to store the current audio state
+ * @buffer: the buffer pointer to store ELD bytes
+ * @max_bytes: the max bytes to be stored on @buffer
+ *
+ * This function is supposed to be used only by a HD-audio controller
+ * driver that needs the interaction with graphics driver.
+ *
+ * This function queries the current state of the audio on the given
+ * digital port and fetches the ELD bytes onto the given buffer.
+ * It returns the number of bytes for the total ELD data, zero for
+ * invalid ELD, or a negative error code.
+ *
+ * The return size is the total bytes required for the whole ELD bytes,
+ * thus it may be over @max_bytes.  If it's over @max_bytes, it implies
+ * that only a part of ELD bytes have been fetched.
+ */
+int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
+			   bool *audio_enabled, char *buffer, int max_bytes)
+{
+	struct hdac_bus *bus = codec->bus;
+	struct drm_audio_component *acomp = bus->audio_component;
+	int port, pipe;
+
+	if (!acomp || !acomp->ops || !acomp->ops->get_eld)
+		return -ENODEV;
+
+	port = nid;
+	if (acomp->audio_ops && acomp->audio_ops->pin2port) {
+		port = acomp->audio_ops->pin2port(codec, nid);
+		if (port < 0)
+			return -EINVAL;
+	}
+	pipe = dev_id;
+	return acomp->ops->get_eld(acomp->dev, port, pipe, audio_enabled,
+				   buffer, max_bytes);
+}
+EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
+
+static int hdac_component_master_bind(struct device *dev)
+{
+	struct drm_audio_component *acomp = hdac_get_acomp(dev);
+	int ret;
+
+	ret = component_bind_all(dev, acomp);
+	if (ret < 0)
+		return ret;
+
+	if (WARN_ON(!(acomp->dev && acomp->ops))) {
+		ret = -EINVAL;
+		goto out_unbind;
+	}
+
+	/* pin the module to avoid dynamic unbinding, but only if given */
+	if (!try_module_get(acomp->ops->owner)) {
+		ret = -ENODEV;
+		goto out_unbind;
+	}
+
+	if (acomp->audio_ops->master_bind) {
+		ret = acomp->audio_ops->master_bind(dev, acomp);
+		if (ret < 0)
+			goto module_put;
+	}
+
+	return 0;
+
+ module_put:
+	module_put(acomp->ops->owner);
+out_unbind:
+	component_unbind_all(dev, acomp);
+
+	return ret;
+}
+
+static void hdac_component_master_unbind(struct device *dev)
+{
+	struct drm_audio_component *acomp = hdac_get_acomp(dev);
+
+	if (acomp->audio_ops->master_unbind)
+		acomp->audio_ops->master_unbind(dev, acomp);
+	module_put(acomp->ops->owner);
+	component_unbind_all(dev, acomp);
+	WARN_ON(acomp->ops || acomp->dev);
+}
+
+static const struct component_master_ops hdac_component_master_ops = {
+	.bind = hdac_component_master_bind,
+	.unbind = hdac_component_master_unbind,
+};
+
+/**
+ * snd_hdac_acomp_register_notifier - Register audio component ops
+ * @bus: HDA core bus
+ * @aops: audio component ops
+ *
+ * This function is supposed to be used only by a HD-audio controller
+ * driver that needs the interaction with graphics driver.
+ *
+ * This function sets the given ops to be called by the graphics driver.
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,
+				    const struct drm_audio_component_audio_ops *aops)
+{
+	if (!bus->audio_component)
+		return -ENODEV;
+
+	bus->audio_component->audio_ops = aops;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_acomp_register_notifier);
+
+/**
+ * snd_hdac_acomp_init - Initialize audio component
+ * @bus: HDA core bus
+ * @match_master: match function for finding components
+ * @extra_size: Extra bytes to allocate
+ *
+ * This function is supposed to be used only by a HD-audio controller
+ * driver that needs the interaction with graphics driver.
+ *
+ * This function initializes and sets up the audio component to communicate
+ * with graphics driver.
+ *
+ * Unlike snd_hdac_i915_init(), this function doesn't synchronize with the
+ * binding with the DRM component.  Each caller needs to sync via master_bind
+ * audio_ops.
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_hdac_acomp_init(struct hdac_bus *bus,
+			const struct drm_audio_component_audio_ops *aops,
+			int (*match_master)(struct device *, void *),
+			size_t extra_size)
+{
+	struct component_match *match = NULL;
+	struct device *dev = bus->dev;
+	struct drm_audio_component *acomp;
+	int ret;
+
+	if (WARN_ON(hdac_get_acomp(dev)))
+		return -EBUSY;
+
+	acomp = devres_alloc(hdac_acomp_release, sizeof(*acomp) + extra_size,
+			     GFP_KERNEL);
+	if (!acomp)
+		return -ENOMEM;
+	acomp->audio_ops = aops;
+	bus->audio_component = acomp;
+	devres_add(dev, acomp);
+
+	component_match_add(dev, &match, match_master, bus);
+	ret = component_master_add_with_match(dev, &hdac_component_master_ops,
+					      match);
+	if (ret < 0)
+		goto out_err;
+
+	return 0;
+
+out_err:
+	bus->audio_component = NULL;
+	devres_destroy(dev, hdac_acomp_release, NULL, NULL);
+	dev_info(dev, "failed to add audio component master (%d)\n", ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_acomp_init);
+
+/**
+ * snd_hdac_acomp_exit - Finalize audio component
+ * @bus: HDA core bus
+ *
+ * This function is supposed to be used only by a HD-audio controller
+ * driver that needs the interaction with graphics driver.
+ *
+ * This function releases the audio component that has been used.
+ *
+ * Returns zero for success or a negative error code.
+ */
+int snd_hdac_acomp_exit(struct hdac_bus *bus)
+{
+	struct device *dev = bus->dev;
+	struct drm_audio_component *acomp = bus->audio_component;
+
+	if (!acomp)
+		return 0;
+
+	WARN_ON(bus->drm_power_refcount);
+	if (bus->drm_power_refcount > 0 && acomp->ops)
+		acomp->ops->put_power(acomp->dev);
+
+	component_master_del(dev, &hdac_component_master_ops);
+
+	bus->audio_component = NULL;
+	devres_destroy(dev, hdac_acomp_release, NULL, NULL);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_acomp_exit);
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 861b77bbc7df..8f2aa8bc1185 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -15,96 +15,11 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/pci.h>
-#include <linux/component.h>
-#include <drm/drm_audio_component.h>
 #include <sound/core.h>
 #include <sound/hdaudio.h>
 #include <sound/hda_i915.h>
 #include <sound/hda_register.h>
 
-static void hdac_acomp_release(struct device *dev, void *res)
-{
-}
-
-static struct drm_audio_component *hdac_get_acomp(struct device *dev)
-{
-	return devres_find(dev, hdac_acomp_release, NULL, NULL);
-}
-
-/**
- * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
- * @bus: HDA core bus
- * @enable: enable or disable the wakeup
- *
- * This function is supposed to be used only by a HD-audio controller
- * driver that needs the interaction with i915 graphics.
- *
- * This function should be called during the chip reset, also called at
- * resume for updating STATESTS register read.
- *
- * Returns zero for success or a negative error code.
- */
-int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
-{
-	struct drm_audio_component *acomp = bus->audio_component;
-
-	if (!acomp || !acomp->ops)
-		return -ENODEV;
-
-	if (!acomp->ops->codec_wake_override) {
-		dev_warn(bus->dev,
-			"Invalid codec wake callback\n");
-		return 0;
-	}
-
-	dev_dbg(bus->dev, "%s codec wakeup\n",
-		enable ? "enable" : "disable");
-
-	acomp->ops->codec_wake_override(acomp->dev, enable);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(snd_hdac_set_codec_wakeup);
-
-/**
- * snd_hdac_display_power - Power up / down the power refcount
- * @bus: HDA core bus
- * @enable: power up or down
- *
- * This function is supposed to be used only by a HD-audio controller
- * driver that needs the interaction with i915 graphics.
- *
- * This function manages a refcount and calls the i915 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)
-{
-	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) {
-		if (!bus->drm_power_refcount++) {
-			acomp->ops->get_power(acomp->dev);
-			snd_hdac_set_codec_wakeup(bus, true);
-			snd_hdac_set_codec_wakeup(bus, false);
-		}
-	} else {
-		WARN_ON(!bus->drm_power_refcount);
-		if (!--bus->drm_power_refcount)
-			acomp->ops->put_power(acomp->dev);
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(snd_hdac_display_power);
-
 #define CONTROLLER_IN_GPU(pci) (((pci)->device == 0x0a0c) || \
 				((pci)->device == 0x0c0c) || \
 				((pci)->device == 0x0d0c) || \
@@ -165,183 +80,11 @@ void snd_hdac_i915_set_bclk(struct hdac_bus *bus)
 }
 EXPORT_SYMBOL_GPL(snd_hdac_i915_set_bclk);
 
-/* There is a fixed mapping between audio pin node and display port.
- * on SNB, IVY, HSW, BSW, SKL, BXT, KBL:
- * Pin Widget 5 - PORT B (port = 1 in i915 driver)
- * Pin Widget 6 - PORT C (port = 2 in i915 driver)
- * Pin Widget 7 - PORT D (port = 3 in i915 driver)
- *
- * on VLV, ILK:
- * Pin Widget 4 - PORT B (port = 1 in i915 driver)
- * Pin Widget 5 - PORT C (port = 2 in i915 driver)
- * Pin Widget 6 - PORT D (port = 3 in i915 driver)
- */
-static int pin2port(struct hdac_device *codec, hda_nid_t pin_nid)
-{
-	int base_nid;
-
-	switch (codec->vendor_id) {
-	case 0x80860054: /* ILK */
-	case 0x80862804: /* ILK */
-	case 0x80862882: /* VLV */
-		base_nid = 3;
-		break;
-	default:
-		base_nid = 4;
-		break;
-	}
-
-	if (WARN_ON(pin_nid <= base_nid || pin_nid > base_nid + 3))
-		return -1;
-	return pin_nid - base_nid;
-}
-
-/**
- * snd_hdac_sync_audio_rate - Set N/CTS based on the sample rate
- * @codec: HDA codec
- * @nid: the pin widget NID
- * @dev_id: device identifier
- * @rate: the sample rate to set
- *
- * This function is supposed to be used only by a HD-audio controller
- * driver that needs the interaction with i915 graphics.
- *
- * This function sets N/CTS value based on the given sample rate.
- * Returns zero for success, or a negative error code.
- */
-int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
-			     int dev_id, int rate)
+static int i915_component_master_match(struct device *dev, void *data)
 {
-	struct hdac_bus *bus = codec->bus;
-	struct drm_audio_component *acomp = bus->audio_component;
-	int port, pipe;
-
-	if (!acomp || !acomp->ops || !acomp->ops->sync_audio_rate)
-		return -ENODEV;
-	port = pin2port(codec, nid);
-	if (port < 0)
-		return -EINVAL;
-	pipe = dev_id;
-	return acomp->ops->sync_audio_rate(acomp->dev, port, pipe, rate);
-}
-EXPORT_SYMBOL_GPL(snd_hdac_sync_audio_rate);
-
-/**
- * snd_hdac_acomp_get_eld - Get the audio state and ELD via component
- * @codec: HDA codec
- * @nid: the pin widget NID
- * @dev_id: device identifier
- * @audio_enabled: the pointer to store the current audio state
- * @buffer: the buffer pointer to store ELD bytes
- * @max_bytes: the max bytes to be stored on @buffer
- *
- * This function is supposed to be used only by a HD-audio controller
- * driver that needs the interaction with i915 graphics.
- *
- * This function queries the current state of the audio on the given
- * digital port and fetches the ELD bytes onto the given buffer.
- * It returns the number of bytes for the total ELD data, zero for
- * invalid ELD, or a negative error code.
- *
- * The return size is the total bytes required for the whole ELD bytes,
- * thus it may be over @max_bytes.  If it's over @max_bytes, it implies
- * that only a part of ELD bytes have been fetched.
- */
-int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
-			   bool *audio_enabled, char *buffer, int max_bytes)
-{
-	struct hdac_bus *bus = codec->bus;
-	struct drm_audio_component *acomp = bus->audio_component;
-	int port, pipe;
-
-	if (!acomp || !acomp->ops || !acomp->ops->get_eld)
-		return -ENODEV;
-
-	port = pin2port(codec, nid);
-	if (port < 0)
-		return -EINVAL;
-
-	pipe = dev_id;
-	return acomp->ops->get_eld(acomp->dev, port, pipe, audio_enabled,
-				   buffer, max_bytes);
-}
-EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
-
-static int hdac_component_master_bind(struct device *dev)
-{
-	struct drm_audio_component *acomp = hdac_get_acomp(dev);
-	int ret;
-
-	ret = component_bind_all(dev, acomp);
-	if (ret < 0)
-		return ret;
-
-	if (WARN_ON(!(acomp->dev && acomp->ops && acomp->ops->get_power &&
-		      acomp->ops->put_power && acomp->ops->get_cdclk_freq))) {
-		ret = -EINVAL;
-		goto out_unbind;
-	}
-
-	/*
-	 * Atm, we don't support dynamic unbinding initiated by the child
-	 * component, so pin its containing module until we unbind.
-	 */
-	if (!try_module_get(acomp->ops->owner)) {
-		ret = -ENODEV;
-		goto out_unbind;
-	}
-
-	return 0;
-
-out_unbind:
-	component_unbind_all(dev, acomp);
-
-	return ret;
-}
-
-static void hdac_component_master_unbind(struct device *dev)
-{
-	struct drm_audio_component *acomp = hdac_get_acomp(dev);
-
-	module_put(acomp->ops->owner);
-	component_unbind_all(dev, acomp);
-	WARN_ON(acomp->ops || acomp->dev);
-}
-
-static const struct component_master_ops hdac_component_master_ops = {
-	.bind = hdac_component_master_bind,
-	.unbind = hdac_component_master_unbind,
-};
-
-static int hdac_component_master_match(struct device *dev, void *data)
-{
-	/* i915 is the only supported component */
 	return !strcmp(dev->driver->name, "i915");
 }
 
-/**
- * snd_hdac_i915_register_notifier - Register i915 audio component ops
- * @bus: HDA core bus
- * @aops: i915 audio component ops
- *
- * This function is supposed to be used only by a HD-audio controller
- * driver that needs the interaction with i915 graphics.
- *
- * This function sets the given ops to be called by the i915 graphics driver.
- *
- * Returns zero for success or a negative error code.
- */
-int snd_hdac_i915_register_notifier(struct hdac_bus *bus,
-				    const struct drm_audio_component_audio_ops *aops)
-{
-	if (!bus->audio_component)
-		return -ENODEV;
-
-	bus->audio_component->audio_ops = aops;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(snd_hdac_i915_register_notifier);
-
 /* check whether intel graphics is present */
 static bool i915_gfx_present(void)
 {
@@ -368,84 +111,26 @@ static bool i915_gfx_present(void)
  */
 int snd_hdac_i915_init(struct hdac_bus *bus)
 {
-	struct component_match *match = NULL;
-	struct device *dev = bus->dev;
-	struct i915_audio_component *i915_acomp;
 	struct drm_audio_component *acomp;
-	int ret;
-
-	if (WARN_ON(hdac_get_acomp(dev)))
-		return -EBUSY;
+	int err;
 
 	if (!i915_gfx_present())
 		return -ENODEV;
 
-	i915_acomp = devres_alloc(hdac_acomp_release, sizeof(*i915_acomp),
-				  GFP_KERNEL);
-	if (!i915_acomp)
-		return -ENOMEM;
-	acomp = &i915_acomp->base;
-	bus->audio_component = acomp;
-	devres_add(dev, acomp);
-
-	component_match_add(dev, &match, hdac_component_master_match, bus);
-	ret = component_master_add_with_match(dev, &hdac_component_master_ops,
-					      match);
-	if (ret < 0)
-		goto out_err;
-
-	/*
-	 * Atm, we don't support deferring the component binding, so make sure
-	 * i915 is loaded and that the binding successfully completes.
-	 */
-	request_module("i915");
-
+	err = snd_hdac_acomp_init(bus, NULL,
+				  i915_component_master_match,
+				  sizeof(struct i915_audio_component) - sizeof(*acomp));
+	if (err < 0)
+		return err;
+	acomp = bus->audio_component;
+	if (!acomp)
+		return -ENODEV;
+	if (!acomp->ops)
+		request_module("i915");
 	if (!acomp->ops) {
-		ret = -ENODEV;
-		goto out_master_del;
+		snd_hdac_acomp_exit(bus);
+		return -ENODEV;
 	}
-	dev_dbg(dev, "bound to i915 component master\n");
-
 	return 0;
-out_master_del:
-	component_master_del(dev, &hdac_component_master_ops);
-out_err:
-	bus->audio_component = NULL;
-	devres_destroy(dev, hdac_acomp_release, NULL, NULL);
-	dev_info(dev, "failed to add i915 component master (%d)\n", ret);
-
-	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_i915_init);
-
-/**
- * snd_hdac_i915_exit - Finalize i915 audio component
- * @bus: HDA core bus
- *
- * This function is supposed to be used only by a HD-audio controller
- * driver that needs the interaction with i915 graphics.
- *
- * This function releases the i915 audio component that has been used.
- *
- * Returns zero for success or a negative error code.
- */
-int snd_hdac_i915_exit(struct hdac_bus *bus)
-{
-	struct device *dev = bus->dev;
-	struct drm_audio_component *acomp = bus->audio_component;
-
-	if (!acomp)
-		return 0;
-
-	WARN_ON(bus->drm_power_refcount);
-	if (bus->drm_power_refcount > 0 && acomp->ops)
-		acomp->ops->put_power(acomp->dev);
-
-	component_master_del(dev, &hdac_component_master_ops);
-
-	bus->audio_component = NULL;
-	devres_destroy(dev, hdac_acomp_release, NULL, NULL);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(snd_hdac_i915_exit);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index bf174a013f2d..1de5491fb9bf 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -183,7 +183,7 @@ struct hdmi_spec {
 	hda_nid_t vendor_nid;
 };
 
-#ifdef CONFIG_SND_HDA_I915
+#ifdef CONFIG_SND_HDA_COMPONENT
 static inline bool codec_has_acomp(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
@@ -2288,7 +2288,7 @@ static void generic_hdmi_free(struct hda_codec *codec)
 	int pin_idx, pcm_idx;
 
 	if (codec_has_acomp(codec))
-		snd_hdac_i915_register_notifier(&codec->bus->core, NULL);
+		snd_hdac_acomp_register_notifier(&codec->bus->core, NULL);
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
@@ -2471,6 +2471,38 @@ static void haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg,
 	snd_hda_codec_set_power_to_all(codec, fg, power_state);
 }
 
+/* There is a fixed mapping between audio pin node and display port.
+ * on SNB, IVY, HSW, BSW, SKL, BXT, KBL:
+ * Pin Widget 5 - PORT B (port = 1 in i915 driver)
+ * Pin Widget 6 - PORT C (port = 2 in i915 driver)
+ * Pin Widget 7 - PORT D (port = 3 in i915 driver)
+ *
+ * on VLV, ILK:
+ * Pin Widget 4 - PORT B (port = 1 in i915 driver)
+ * Pin Widget 5 - PORT C (port = 2 in i915 driver)
+ * Pin Widget 6 - PORT D (port = 3 in i915 driver)
+ */
+static int intel_base_nid(struct hda_codec *codec)
+{
+	switch (codec->core.vendor_id) {
+	case 0x80860054: /* ILK */
+	case 0x80862804: /* ILK */
+	case 0x80862882: /* VLV */
+		return 4;
+	default:
+		return 5;
+	}
+}
+
+static int intel_pin2port(void *audio_ptr, int pin_nid)
+{
+	int base_nid = intel_base_nid(audio_ptr);
+
+	if (WARN_ON(pin_nid < base_nid || pin_nid >= base_nid + 3))
+		return -1;
+	return pin_nid - base_nid + 1; /* intel port is 1-based */
+}
+
 static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe)
 {
 	struct hda_codec *codec = audio_ptr;
@@ -2481,16 +2513,7 @@ static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe)
 	if (port < 1 || port > 3)
 		return;
 
-	switch (codec->core.vendor_id) {
-	case 0x80860054: /* ILK */
-	case 0x80862804: /* ILK */
-	case 0x80862882: /* VLV */
-		pin_nid = port + 0x03;
-		break;
-	default:
-		pin_nid = port + 0x04;
-		break;
-	}
+	pin_nid = port + intel_base_nid(codec) - 1; /* intel port is 1-based */
 
 	/* skip notification during system suspend (but not in runtime PM);
 	 * the state will be updated at resume
@@ -2517,8 +2540,9 @@ static void register_i915_notifier(struct hda_codec *codec)
 	 * We need make sure audio_ptr is really setup
 	 */
 	wmb();
+	spec->drm_audio_ops.pin2port = intel_pin2port;
 	spec->drm_audio_ops.pin_eld_notify = intel_pin_eld_notify;
-	snd_hdac_i915_register_notifier(&codec->bus->core,
+	snd_hdac_acomp_register_notifier(&codec->bus->core,
 					&spec->drm_audio_ops);
 }
 
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 2b7c33db4ded..4748a9d5de3b 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1530,6 +1530,11 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_device *hdev,
 	return ret;
 }
 
+static int hdac_hdmi_pin2port(void *aptr, int pin)
+{
+	return pin - 4; /* map NID 0x05 -> port #1 */
+}
+
 static void hdac_hdmi_eld_notify_cb(void *aptr, int port, int pipe)
 {
 	struct hdac_device *hdev = aptr;
@@ -1584,6 +1589,7 @@ static void hdac_hdmi_eld_notify_cb(void *aptr, int port, int pipe)
 }
 
 static struct drm_audio_component_audio_ops aops = {
+	.pin2port	= hdac_hdmi_pin2port,
 	.pin_eld_notify	= hdac_hdmi_eld_notify_cb,
 };
 
@@ -1812,7 +1818,7 @@ static int hdmi_codec_probe(struct snd_soc_component *component)
 		return ret;
 
 	aops.audio_ptr = hdev;
-	ret = snd_hdac_i915_register_notifier(hdev->bus, &aops);
+	ret = snd_hdac_acomp_register_notifier(hdev->bus, &aops);
 	if (ret < 0) {
 		dev_err(&hdev->dev, "notifier register failed: err: %d\n", ret);
 		return ret;
-- 
2.18.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Split audio component to a generic type
  2018-07-16 13:48 ` [PATCH 1/3] drm/i915: Split audio component to a generic type Takashi Iwai
@ 2018-07-16 17:22   ` Rodrigo Vivi
  2018-07-16 17:50     ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2018-07-16 17:22 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jim Qu, alsa-devel, intel-gfx, dri-devel

On Mon, Jul 16, 2018 at 03:48:13PM +0200, Takashi Iwai wrote:
> For allowing other drivers to use the DRM audio component, rename the
> i915_audio_component_* with drm_audio_component_*, and split the
> generic part into drm_audio_component.h.  The i915 specific stuff
> remains in struct i915_audio_component, which contains
> drm_audio_component as the base.
> 
> This is a preliminary change for further development, and no
> functional changes by this patch itself, merely code-split and
> renames.

+1 on the idea....

> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/intel_audio.c |  22 +++---
>  include/drm/drm_audio_component.h  | 115 +++++++++++++++++++++++++++++
>  include/drm/i915_component.h       |  85 ++-------------------
>  include/sound/hda_i915.h           |   6 +-
>  include/sound/hdaudio.h            |   6 +-
>  sound/hda/hdac_i915.c              |  40 +++++-----
>  sound/pci/hda/patch_hdmi.c         |   8 +-
>  sound/soc/codecs/hdac_hdmi.c       |   2 +-
>  8 files changed, 164 insertions(+), 120 deletions(-)
>  create mode 100644 include/drm/drm_audio_component.h
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..7dd5605d94ae 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -639,11 +639,12 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
>  	dev_priv->av_enc_map[pipe] = encoder;
>  	mutex_unlock(&dev_priv->av_mutex);
>  
> -	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
> +	if (acomp && acomp->base.audio_ops &&
> +	    acomp->base.audio_ops->pin_eld_notify) {
>  		/* audio drivers expect pipe = -1 to indicate Non-MST cases */
>  		if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
>  			pipe = -1;
> -		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> +		acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
>  						 (int) port, (int) pipe);
>  	}
>  
> @@ -681,11 +682,12 @@ void intel_audio_codec_disable(struct intel_encoder *encoder,
>  	dev_priv->av_enc_map[pipe] = NULL;
>  	mutex_unlock(&dev_priv->av_mutex);
>  
> -	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
> +	if (acomp && acomp->base.audio_ops &&
> +	    acomp->base.audio_ops->pin_eld_notify) {
>  		/* audio drivers expect pipe = -1 to indicate Non-MST cases */
>  		if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
>  			pipe = -1;
> -		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> +		acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
>  						 (int) port, (int) pipe);
>  	}
>  
> @@ -880,7 +882,7 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
>  	return ret;
>  }
>  
> -static const struct i915_audio_component_ops i915_audio_component_ops = {
> +static const struct drm_audio_component_ops i915_audio_component_ops = {
>  	.owner		= THIS_MODULE,
>  	.get_power	= i915_audio_component_get_power,
>  	.put_power	= i915_audio_component_put_power,
> @@ -897,12 +899,12 @@ static int i915_audio_component_bind(struct device *i915_kdev,
>  	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>  	int i;
>  
> -	if (WARN_ON(acomp->ops || acomp->dev))
> +	if (WARN_ON(acomp->base.ops || acomp->base.dev))
>  		return -EEXIST;
>  
>  	drm_modeset_lock_all(&dev_priv->drm);
> -	acomp->ops = &i915_audio_component_ops;
> -	acomp->dev = i915_kdev;
> +	acomp->base.ops = &i915_audio_component_ops;
> +	acomp->base.dev = i915_kdev;
>  	BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
>  	for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
>  		acomp->aud_sample_rate[i] = 0;
> @@ -919,8 +921,8 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
>  	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>  
>  	drm_modeset_lock_all(&dev_priv->drm);
> -	acomp->ops = NULL;
> -	acomp->dev = NULL;
> +	acomp->base.ops = NULL;
> +	acomp->base.dev = NULL;
>  	dev_priv->audio_component = NULL;
>  	drm_modeset_unlock_all(&dev_priv->drm);
>  }
> diff --git a/include/drm/drm_audio_component.h b/include/drm/drm_audio_component.h
> new file mode 100644
> index 000000000000..f310b28404e8
> --- /dev/null
> +++ b/include/drm/drm_audio_component.h
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */

I think it is better to start new files with SPDX identifiers....

> +
> +#ifndef _DRM_AUDIO_COMPONENT_H_
> +#define _DRM_AUDIO_COMPONENT_H_
> +
> +/**
> + * struct drm_audio_component_ops - Ops implemented by DRM driver, called by hda driver
> + */
> +struct drm_audio_component_ops {
> +	/**
> +	 * @owner: i915 module

drm now?

> +	 */
> +	struct module *owner;
> +	/**
> +	 * @get_power: get the POWER_DOMAIN_AUDIO power well
> +	 *
> +	 * Request the power well to be turned on.
> +	 */
> +	void (*get_power)(struct device *);
> +	/**
> +	 * @put_power: put the POWER_DOMAIN_AUDIO power well
> +	 *
> +	 * Allow the power well to be turned off.
> +	 */
> +	void (*put_power)(struct device *);
> +	/**
> +	 * @codec_wake_override: Enable/disable codec wake signal
> +	 */
> +	void (*codec_wake_override)(struct device *, bool enable);
> +	/**
> +	 * @get_cdclk_freq: Get the Core Display Clock in kHz
> +	 */
> +	int (*get_cdclk_freq)(struct device *);
> +	/**
> +	 * @sync_audio_rate: set n/cts based on the sample rate
> +	 *
> +	 * Called from audio driver. After audio driver sets the
> +	 * sample rate, it will call this function to set n/cts
> +	 */
> +	int (*sync_audio_rate)(struct device *, int port, int pipe, int rate);
> +	/**
> +	 * @get_eld: fill the audio state and ELD bytes for the given port
> +	 *
> +	 * Called from audio driver to get the HDMI/DP audio state of the given
> +	 * digital port, and also fetch ELD bytes to the given pointer.
> +	 *
> +	 * It returns the byte size of the original ELD (not the actually
> +	 * copied size), zero for an invalid ELD, or a negative error code.
> +	 *
> +	 * Note that the returned size may be over @max_bytes.  Then it
> +	 * implies that only a part of ELD has been copied to the buffer.
> +	 */
> +	int (*get_eld)(struct device *, int port, int pipe, bool *enabled,
> +		       unsigned char *buf, int max_bytes);
> +};
> +
> +/**
> + * struct drm_audio_component_audio_ops - Ops implemented by hda driver, called by DRM driver
> + */
> +struct drm_audio_component_audio_ops {
> +	/**
> +	 * @audio_ptr: Pointer to be used in call to pin_eld_notify
> +	 */
> +	void *audio_ptr;
> +	/**
> +	 * @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD information has changed
> +	 *
> +	 * Called when the DRM driver has set up audio pipeline or has just
> +	 * begun to tear it down. This allows the HDA driver to update its
> +	 * status accordingly (even when the HDA controller is in power save
> +	 * mode).
> +	 */
> +	void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
> +};
> +
> +/**
> + * struct drm_audio_component - Used for direct communication between DRM and hda drivers
> + */
> +struct drm_audio_component {
> +	/**
> +	 * @dev: DRM device, used as parameter for ops
> +	 */
> +	struct device *dev;
> +	/**
> +	 * @ops: Ops implemented by DRM driver, called by hda driver
> +	 */
> +	const struct drm_audio_component_ops *ops;
> +	/**
> +	 * @audio_ops: Ops implemented by hda driver, called by DRM driver
> +	 */
> +	const struct drm_audio_component_audio_ops *audio_ops;
> +};
> +
> +#endif /* _DRM_AUDIO_COMPONENT_H_ */
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index 346b1f5cb180..fca22d463e1b 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -24,101 +24,26 @@
>  #ifndef _I915_COMPONENT_H_
>  #define _I915_COMPONENT_H_
>  
> +#include "drm_audio_component.h"
> +
>  /* MAX_PORT is the number of port
>   * It must be sync with I915_MAX_PORTS defined i915_drv.h
>   */
>  #define MAX_PORTS 6
>  
> -/**
> - * struct i915_audio_component_ops - Ops implemented by i915 driver, called by hda driver
> - */
> -struct i915_audio_component_ops {
> -	/**
> -	 * @owner: i915 module
> -	 */
> -	struct module *owner;
> -	/**
> -	 * @get_power: get the POWER_DOMAIN_AUDIO power well
> -	 *
> -	 * Request the power well to be turned on.
> -	 */
> -	void (*get_power)(struct device *);
> -	/**
> -	 * @put_power: put the POWER_DOMAIN_AUDIO power well
> -	 *
> -	 * Allow the power well to be turned off.
> -	 */
> -	void (*put_power)(struct device *);
> -	/**
> -	 * @codec_wake_override: Enable/disable codec wake signal
> -	 */
> -	void (*codec_wake_override)(struct device *, bool enable);
> -	/**
> -	 * @get_cdclk_freq: Get the Core Display Clock in kHz
> -	 */
> -	int (*get_cdclk_freq)(struct device *);
> -	/**
> -	 * @sync_audio_rate: set n/cts based on the sample rate
> -	 *
> -	 * Called from audio driver. After audio driver sets the
> -	 * sample rate, it will call this function to set n/cts
> -	 */
> -	int (*sync_audio_rate)(struct device *, int port, int pipe, int rate);
> -	/**
> -	 * @get_eld: fill the audio state and ELD bytes for the given port
> -	 *
> -	 * Called from audio driver to get the HDMI/DP audio state of the given
> -	 * digital port, and also fetch ELD bytes to the given pointer.
> -	 *
> -	 * It returns the byte size of the original ELD (not the actually
> -	 * copied size), zero for an invalid ELD, or a negative error code.
> -	 *
> -	 * Note that the returned size may be over @max_bytes.  Then it
> -	 * implies that only a part of ELD has been copied to the buffer.
> -	 */
> -	int (*get_eld)(struct device *, int port, int pipe, bool *enabled,
> -		       unsigned char *buf, int max_bytes);
> -};
> -
> -/**
> - * struct i915_audio_component_audio_ops - Ops implemented by hda driver, called by i915 driver
> - */
> -struct i915_audio_component_audio_ops {
> -	/**
> -	 * @audio_ptr: Pointer to be used in call to pin_eld_notify
> -	 */
> -	void *audio_ptr;
> -	/**
> -	 * @pin_eld_notify: Notify the HDA driver that pin sense and/or ELD information has changed
> -	 *
> -	 * Called when the i915 driver has set up audio pipeline or has just
> -	 * begun to tear it down. This allows the HDA driver to update its
> -	 * status accordingly (even when the HDA controller is in power save
> -	 * mode).
> -	 */
> -	void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
> -};
> -
>  /**
>   * struct i915_audio_component - Used for direct communication between i915 and hda drivers
>   */
>  struct i915_audio_component {
>  	/**
> -	 * @dev: i915 device, used as parameter for ops
> +	 * @base: the drm_audio_component base class
>  	 */
> -	struct device *dev;
> +	struct drm_audio_component	base;
> +
>  	/**
>  	 * @aud_sample_rate: the array of audio sample rate per port
>  	 */
>  	int aud_sample_rate[MAX_PORTS];
> -	/**
> -	 * @ops: Ops implemented by i915 driver, called by hda driver
> -	 */
> -	const struct i915_audio_component_ops *ops;
> -	/**
> -	 * @audio_ops: Ops implemented by hda driver, called by i915 driver
> -	 */
> -	const struct i915_audio_component_audio_ops *audio_ops;
>  };
>  
>  #endif /* _I915_COMPONENT_H_ */
> diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
> index a94f5b6f92ac..f69ea84e7b65 100644
> --- a/include/sound/hda_i915.h
> +++ b/include/sound/hda_i915.h
> @@ -5,7 +5,7 @@
>  #ifndef __SOUND_HDA_I915_H
>  #define __SOUND_HDA_I915_H
>  
> -#include <drm/i915_component.h>
> +#include <drm/drm_audio_component.h>
>  
>  #ifdef CONFIG_SND_HDA_I915
>  int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable);
> @@ -17,7 +17,7 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
>  			   bool *audio_enabled, char *buffer, int max_bytes);
>  int snd_hdac_i915_init(struct hdac_bus *bus);
>  int snd_hdac_i915_exit(struct hdac_bus *bus);
> -int snd_hdac_i915_register_notifier(const struct i915_audio_component_audio_ops *);
> +int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *);
>  #else
>  static inline int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
>  {
> @@ -49,7 +49,7 @@ static inline int snd_hdac_i915_exit(struct hdac_bus *bus)
>  {
>  	return 0;
>  }
> -static inline int snd_hdac_i915_register_notifier(const struct i915_audio_component_audio_ops *ops)
> +static inline int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *ops)
>  {
>  	return -ENODEV;
>  }
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index f1baaa88e766..ab5ee3ef2198 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -333,9 +333,9 @@ struct hdac_bus {
>  	spinlock_t reg_lock;
>  	struct mutex cmd_mutex;
>  
> -	/* i915 component interface */
> -	struct i915_audio_component *audio_component;
> -	int i915_power_refcount;
> +	/* DRM component interface */
> +	struct drm_audio_component *audio_component;
> +	int drm_power_refcount;
>  
>  	/* parameters required for enhanced capabilities */
>  	int num_streams;
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index cbe818eda336..1a88c1aaf9bb 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -16,13 +16,13 @@
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/component.h>
> -#include <drm/i915_component.h>
> +#include <drm/drm_audio_component.h>
>  #include <sound/core.h>
>  #include <sound/hdaudio.h>
>  #include <sound/hda_i915.h>
>  #include <sound/hda_register.h>
>  
> -static struct i915_audio_component *hdac_acomp;
> +static struct drm_audio_component *hdac_acomp;
>  
>  /**
>   * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
> @@ -39,7 +39,7 @@ static struct i915_audio_component *hdac_acomp;
>   */
>  int snd_hdac_set_codec_wakeup(struct hdac_bus *bus, bool enable)
>  {
> -	struct i915_audio_component *acomp = bus->audio_component;
> +	struct drm_audio_component *acomp = bus->audio_component;
>  
>  	if (!acomp || !acomp->ops)
>  		return -ENODEV;
> @@ -74,7 +74,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_set_codec_wakeup);
>   */
>  int snd_hdac_display_power(struct hdac_bus *bus, bool enable)
>  {
> -	struct i915_audio_component *acomp = bus->audio_component;
> +	struct drm_audio_component *acomp = bus->audio_component;
>  
>  	if (!acomp || !acomp->ops)
>  		return -ENODEV;
> @@ -83,14 +83,14 @@ int snd_hdac_display_power(struct hdac_bus *bus, bool enable)
>  		enable ? "enable" : "disable");
>  
>  	if (enable) {
> -		if (!bus->i915_power_refcount++) {
> +		if (!bus->drm_power_refcount++) {
>  			acomp->ops->get_power(acomp->dev);
>  			snd_hdac_set_codec_wakeup(bus, true);
>  			snd_hdac_set_codec_wakeup(bus, false);
>  		}
>  	} else {
> -		WARN_ON(!bus->i915_power_refcount);
> -		if (!--bus->i915_power_refcount)
> +		WARN_ON(!bus->drm_power_refcount);
> +		if (!--bus->drm_power_refcount)
>  			acomp->ops->put_power(acomp->dev);
>  	}
>  
> @@ -119,7 +119,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_display_power);
>   */
>  void snd_hdac_i915_set_bclk(struct hdac_bus *bus)
>  {
> -	struct i915_audio_component *acomp = bus->audio_component;
> +	struct drm_audio_component *acomp = bus->audio_component;
>  	struct pci_dev *pci = to_pci_dev(bus->dev);
>  	int cdclk_freq;
>  	unsigned int bclk_m, bclk_n;
> @@ -206,7 +206,7 @@ int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
>  			     int dev_id, int rate)
>  {
>  	struct hdac_bus *bus = codec->bus;
> -	struct i915_audio_component *acomp = bus->audio_component;
> +	struct drm_audio_component *acomp = bus->audio_component;
>  	int port, pipe;
>  
>  	if (!acomp || !acomp->ops || !acomp->ops->sync_audio_rate)
> @@ -244,7 +244,7 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int dev_id,
>  			   bool *audio_enabled, char *buffer, int max_bytes)
>  {
>  	struct hdac_bus *bus = codec->bus;
> -	struct i915_audio_component *acomp = bus->audio_component;
> +	struct drm_audio_component *acomp = bus->audio_component;
>  	int port, pipe;
>  
>  	if (!acomp || !acomp->ops || !acomp->ops->get_eld)
> @@ -262,7 +262,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
>  
>  static int hdac_component_master_bind(struct device *dev)
>  {
> -	struct i915_audio_component *acomp = hdac_acomp;
> +	struct drm_audio_component *acomp = hdac_acomp;
>  	int ret;
>  
>  	ret = component_bind_all(dev, acomp);
> @@ -294,7 +294,7 @@ static int hdac_component_master_bind(struct device *dev)
>  
>  static void hdac_component_master_unbind(struct device *dev)
>  {
> -	struct i915_audio_component *acomp = hdac_acomp;
> +	struct drm_audio_component *acomp = hdac_acomp;
>  
>  	module_put(acomp->ops->owner);
>  	component_unbind_all(dev, acomp);
> @@ -323,7 +323,7 @@ static int hdac_component_master_match(struct device *dev, void *data)
>   *
>   * Returns zero for success or a negative error code.
>   */
> -int snd_hdac_i915_register_notifier(const struct i915_audio_component_audio_ops *aops)
> +int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *aops)
>  {
>  	if (!hdac_acomp)
>  		return -ENODEV;
> @@ -361,7 +361,8 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
>  {
>  	struct component_match *match = NULL;
>  	struct device *dev = bus->dev;
> -	struct i915_audio_component *acomp;
> +	struct i915_audio_component *i915_acomp;
> +	struct drm_audio_component *acomp;
>  	int ret;
>  
>  	if (WARN_ON(hdac_acomp))
> @@ -370,9 +371,10 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
>  	if (!i915_gfx_present())
>  		return -ENODEV;
>  
> -	acomp = kzalloc(sizeof(*acomp), GFP_KERNEL);
> -	if (!acomp)
> +	i915_acomp = kzalloc(sizeof(*i915_acomp), GFP_KERNEL);
> +	if (!i915_acomp)
>  		return -ENOMEM;
> +	acomp = &i915_acomp->base;
>  	bus->audio_component = acomp;
>  	hdac_acomp = acomp;
>  
> @@ -421,13 +423,13 @@ EXPORT_SYMBOL_GPL(snd_hdac_i915_init);
>  int snd_hdac_i915_exit(struct hdac_bus *bus)
>  {
>  	struct device *dev = bus->dev;
> -	struct i915_audio_component *acomp = bus->audio_component;
> +	struct drm_audio_component *acomp = bus->audio_component;
>  
>  	if (!acomp)
>  		return 0;
>  
> -	WARN_ON(bus->i915_power_refcount);
> -	if (bus->i915_power_refcount > 0 && acomp->ops)
> +	WARN_ON(bus->drm_power_refcount);
> +	if (bus->drm_power_refcount > 0 && acomp->ops)
>  		acomp->ops->put_power(acomp->dev);
>  
>  	component_master_del(dev, &hdac_component_master_ops);
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 8a49415aebac..c0847017114c 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -177,7 +177,7 @@ struct hdmi_spec {
>  
>  	/* i915/powerwell (Haswell+/Valleyview+) specific */
>  	bool use_acomp_notifier; /* use i915 eld_notify callback for hotplug */
> -	struct i915_audio_component_audio_ops i915_audio_ops;
> +	struct drm_audio_component_audio_ops drm_audio_ops;
>  
>  	struct hdac_chmap chmap;
>  	hda_nid_t vendor_nid;
> @@ -2511,14 +2511,14 @@ static void register_i915_notifier(struct hda_codec *codec)
>  	struct hdmi_spec *spec = codec->spec;
>  
>  	spec->use_acomp_notifier = true;
> -	spec->i915_audio_ops.audio_ptr = codec;
> +	spec->drm_audio_ops.audio_ptr = codec;
>  	/* intel_audio_codec_enable() or intel_audio_codec_disable()
>  	 * will call pin_eld_notify with using audio_ptr pointer
>  	 * We need make sure audio_ptr is really setup
>  	 */
>  	wmb();
> -	spec->i915_audio_ops.pin_eld_notify = intel_pin_eld_notify;
> -	snd_hdac_i915_register_notifier(&spec->i915_audio_ops);
> +	spec->drm_audio_ops.pin_eld_notify = intel_pin_eld_notify;
> +	snd_hdac_i915_register_notifier(&spec->drm_audio_ops);
>  }
>  
>  /* setup_stream ops override for HSW+ */
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 3e3a2a9ef310..460075475f20 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -1583,7 +1583,7 @@ static void hdac_hdmi_eld_notify_cb(void *aptr, int port, int pipe)
>  
>  }
>  
> -static struct i915_audio_component_audio_ops aops = {
> +static struct drm_audio_component_audio_ops aops = {
>  	.pin_eld_notify	= hdac_hdmi_eld_notify_cb,
>  };
>  
> -- 
> 2.18.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Split audio component to a generic type
  2018-07-16 17:22   ` [Intel-gfx] " Rodrigo Vivi
@ 2018-07-16 17:50     ` Takashi Iwai
  2018-07-16 20:35       ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 17:50 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jim Qu, alsa-devel, intel-gfx, dri-devel

On Mon, 16 Jul 2018 19:22:17 +0200,
Rodrigo Vivi wrote:
> 
> > --- /dev/null
> > +++ b/include/drm/drm_audio_component.h
> > @@ -0,0 +1,115 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> 
> I think it is better to start new files with SPDX identifiers....

Yeah, it's a more sensible option.  Will rewrite in v2.

> > +
> > +#ifndef _DRM_AUDIO_COMPONENT_H_
> > +#define _DRM_AUDIO_COMPONENT_H_
> > +
> > +/**
> > + * struct drm_audio_component_ops - Ops implemented by DRM driver, called by hda driver
> > + */
> > +struct drm_audio_component_ops {
> > +	/**
> > +	 * @owner: i915 module
> 
> drm now?

Right.  Will be fixed in v2, too.


Thanks!

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.BAT: failure for Make the audio component binding more generic
  2018-07-16 13:48 [PATCH 0/3] Make the audio component binding more generic Takashi Iwai
                   ` (2 preceding siblings ...)
  2018-07-16 13:48 ` [PATCH 3/3] ALSA: hda: Make audio component support more generic Takashi Iwai
@ 2018-07-16 19:02 ` Patchwork
  2018-07-17  6:34 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-07-16 19:02 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: intel-gfx

== Series Details ==

Series: Make the audio component binding more generic
URL   : https://patchwork.freedesktop.org/series/46603/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4499 -> Patchwork_9679 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9679 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9679, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46603/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9679:

  === IGT changes ===

    ==== Possible regressions ====

    igt@debugfs_test@read_all_entries:
      fi-ilk-650:         PASS -> DMESG-WARN

    igt@drv_module_reload@basic-reload:
      fi-cfl-s3:          PASS -> INCOMPLETE

    igt@gem_exec_suspend@basic-s3:
      {fi-kbl-8809g}:     PASS -> INCOMPLETE
      fi-bdw-5557u:       PASS -> INCOMPLETE
      fi-snb-2600:        PASS -> INCOMPLETE
      fi-whl-u:           PASS -> INCOMPLETE
      fi-bsw-n3050:       PASS -> INCOMPLETE
      fi-hsw-4770:        PASS -> INCOMPLETE
      fi-ilk-650:         PASS -> INCOMPLETE
      fi-glk-dsi:         PASS -> INCOMPLETE
      fi-hsw-peppy:       PASS -> INCOMPLETE

    igt@gem_exec_suspend@basic-s4-devices:
      fi-hsw-4770r:       PASS -> INCOMPLETE
      fi-cfl-8700k:       PASS -> INCOMPLETE

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-skl-iommu}:     PASS -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in Patchwork_9679 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-guc:         PASS -> FAIL (fdo#103375)
      fi-kbl-r:           PASS -> FAIL (fdo#103375)
      fi-skl-6770hq:      PASS -> INCOMPLETE (fdo#104108)
      fi-byt-n2820:       PASS -> FAIL (fdo#103375)
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
      fi-skl-6260u:       PASS -> INCOMPLETE (fdo#104108)
      fi-kbl-7560u:       PASS -> FAIL (fdo#103375)
      fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)
      fi-skl-6700hq:      PASS -> INCOMPLETE (fdo#104108)
      fi-ivb-3520m:       PASS -> INCOMPLETE (fdo#106220)
      fi-cfl-guc:         PASS -> INCOMPLETE (fdo#106693)
      fi-kbl-7567u:       PASS -> FAIL (fdo#103375)
      fi-skl-guc:         PASS -> FAIL (fdo#103375)
      fi-glk-j4005:       PASS -> INCOMPLETE (fdo#103359, k.org#198133)
      fi-ivb-3770:        PASS -> INCOMPLETE (fdo#106220)
      fi-skl-6700k2:      PASS -> FAIL (fdo#103375)
      fi-snb-2520m:       PASS -> FAIL (fdo#103375)
      fi-cfl-8700k:       PASS -> FAIL (fdo#103375)
      fi-hsw-4770r:       PASS -> FAIL (fdo#103375)
      fi-skl-6600u:       PASS -> FAIL (fdo#103375)
      fi-kbl-7500u:       PASS -> INCOMPLETE (fdo#105524, k.org#199541)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-byt-n2820:       PASS -> INCOMPLETE (fdo#102657)
      fi-skl-6700k2:      PASS -> INCOMPLETE (fdo#104108, fdo#105524, k.org#199541)
      fi-skl-guc:         PASS -> INCOMPLETE (fdo#104108, fdo#106693)
      fi-kbl-7567u:       PASS -> INCOMPLETE (fdo#107139, fdo#103665)
      fi-kbl-guc:         PASS -> INCOMPLETE (fdo#107139, fdo#106693)
      fi-kbl-r:           PASS -> INCOMPLETE (fdo#107139)
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)
      fi-skl-6600u:       PASS -> INCOMPLETE (fdo#104108)
      fi-kbl-7560u:       PASS -> INCOMPLETE (fdo#107139)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105524 https://bugs.freedesktop.org/show_bug.cgi?id=105524
  fdo#106220 https://bugs.freedesktop.org/show_bug.cgi?id=106220
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
  k.org#199541 https://bugzilla.kernel.org/show_bug.cgi?id=199541


== Participating hosts (46 -> 41) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4499 -> Patchwork_9679

  CI_DRM_4499: 30c05928fc8cdb8bbbf052fec71f484654cf2a49 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4558: d8e97e1710b27a3931a1c53d1dd88c0e709c085b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9679: 0d7235cdb8c0240104412aa4a05fd3d4cae4df91 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0d7235cdb8c0 ALSA: hda: Make audio component support more generic
702f221ef481 ALSA: hda/i915: Associate audio component with devres
801f1d8533a3 drm/i915: Split audio component to a generic type

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9679/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] ALSA: hda: Make audio component support more generic
  2018-07-16 13:48 ` [PATCH 3/3] ALSA: hda: Make audio component support more generic Takashi Iwai
@ 2018-07-16 20:33   ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 20:33 UTC (permalink / raw)
  To: alsa-devel; +Cc: Jim Qu, intel-gfx, dri-devel

On Mon, 16 Jul 2018 15:48:15 +0200,
Takashi Iwai wrote:
> 
> +static int hdac_component_master_bind(struct device *dev)
> +{
> +	struct drm_audio_component *acomp = hdac_get_acomp(dev);
> +	int ret;
> +
> +	ret = component_bind_all(dev, acomp);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (WARN_ON(!(acomp->dev && acomp->ops))) {
> +		ret = -EINVAL;
> +		goto out_unbind;
> +	}
> +
> +	/* pin the module to avoid dynamic unbinding, but only if given */
> +	if (!try_module_get(acomp->ops->owner)) {
> +		ret = -ENODEV;
> +		goto out_unbind;
> +	}
> +
> +	if (acomp->audio_ops->master_bind) {

Here misses the NULL check of acomp->audio_ops, which is caught by the
intel CI.  I overlooked it since the upcoming patch will set audio_ops
initially for i915 as well.  Will fix it in v2.


thanks,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Split audio component to a generic type
  2018-07-16 17:50     ` Takashi Iwai
@ 2018-07-16 20:35       ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2018-07-16 20:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jim Qu, alsa-devel, intel-gfx, dri-devel

On Mon, 16 Jul 2018 19:50:48 +0200,
Takashi Iwai wrote:
> 
> On Mon, 16 Jul 2018 19:22:17 +0200,
> Rodrigo Vivi wrote:
> > 
> > > --- /dev/null
> > > +++ b/include/drm/drm_audio_component.h
> > > @@ -0,0 +1,115 @@
> > > +/*
> > > + * Copyright © 2014 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining a
> > > + * copy of this software and associated documentation files (the "Software"),
> > > + * to deal in the Software without restriction, including without limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the next
> > > + * paragraph) shall be included in all copies or substantial portions of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + */
> > 
> > I think it is better to start new files with SPDX identifiers....
> 
> Yeah, it's a more sensible option.  Will rewrite in v2.

So, now I changed to

  // SPDX-License-Identifier: MIT

as the original i915_component.h seems containing that license.
Let me know if it should be changed to anything else.


thanks,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Make the audio component binding more generic
  2018-07-16 13:48 [PATCH 0/3] Make the audio component binding more generic Takashi Iwai
                   ` (3 preceding siblings ...)
  2018-07-16 19:02 ` ✗ Fi.CI.BAT: failure for Make the audio component binding " Patchwork
@ 2018-07-17  6:34 ` Patchwork
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-07-17  6:34 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: intel-gfx

== Series Details ==

Series: Make the audio component binding more generic
URL   : https://patchwork.freedesktop.org/series/46603/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
801f1d8533a3 drm/i915: Split audio component to a generic type
-:89: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#89: 
new file mode 100644

-:94: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#94: FILE: include/drm/drm_audio_component.h:1:
+/*

-:133: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name
#133: FILE: include/drm/drm_audio_component.h:40:
+	void (*get_power)(struct device *);

-:139: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name
#139: FILE: include/drm/drm_audio_component.h:46:
+	void (*put_power)(struct device *);

-:143: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name
#143: FILE: include/drm/drm_audio_component.h:50:
+	void (*codec_wake_override)(struct device *, bool enable);

-:147: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name
#147: FILE: include/drm/drm_audio_component.h:54:
+	int (*get_cdclk_freq)(struct device *);

-:154: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name
#154: FILE: include/drm/drm_audio_component.h:61:
+	int (*sync_audio_rate)(struct device *, int port, int pipe, int rate);

-:167: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct device *' should also have an identifier name
#167: FILE: include/drm/drm_audio_component.h:74:
+	int (*get_eld)(struct device *, int port, int pipe, bool *enabled,

-:338: WARNING:FUNCTION_ARGUMENTS: function definition argument 'const struct drm_audio_component_audio_ops *' should also have an identifier name
#338: FILE: include/sound/hda_i915.h:20:
+int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *);

-:347: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#347: FILE: include/sound/hda_i915.h:52:
 }
+static inline int snd_hdac_i915_register_notifier(const struct drm_audio_component_audio_ops *ops)

total: 0 errors, 9 warnings, 1 checks, 484 lines checked
702f221ef481 ALSA: hda/i915: Associate audio component with devres
-:47: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#47: FILE: include/sound/hda_i915.h:53:
 }
+static inline int snd_hdac_i915_register_notifier(struct hdac_bus *bus,

total: 0 errors, 0 warnings, 1 checks, 139 lines checked
0d7235cdb8c0 ALSA: hda: Make audio component support more generic
-:72: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct drm_audio_component *' should also have an identifier name
#72: FILE: include/drm/drm_audio_component.h:110:
+	int (*master_bind)(struct device *dev, struct drm_audio_component *);

-:79: WARNING:FUNCTION_ARGUMENTS: function definition argument 'struct drm_audio_component *' should also have an identifier name
#79: FILE: include/drm/drm_audio_component.h:117:
+	void (*master_unbind)(struct device *dev, struct drm_audio_component *);

-:84: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#84: 
new file mode 100644

-:89: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#89: FILE: include/sound/hda_component.h:1:
+// SPDX-License-Identifier: GPL-2.0

-:110: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#110: FILE: include/sound/hda_component.h:22:
+int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,
+				    const struct drm_audio_component_audio_ops *ops);

-:116: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#116: FILE: include/sound/hda_component.h:28:
+}
+static inline int snd_hdac_display_power(struct hdac_bus *bus, bool enable)

-:120: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#120: FILE: include/sound/hda_component.h:32:
+}
+static inline int snd_hdac_sync_audio_rate(struct hdac_device *codec,

-:125: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#125: FILE: include/sound/hda_component.h:37:
+}
+static inline int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid,

-:131: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#131: FILE: include/sound/hda_component.h:43:
+}
+static inline int snd_hdac_acomp_init(struct hdac_bus *bus,

-:138: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#138: FILE: include/sound/hda_component.h:50:
+}
+static inline int snd_hdac_acomp_exit(struct hdac_bus *bus)

-:142: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#142: FILE: include/sound/hda_component.h:54:
+}
+static inline int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,

-:143: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#143: FILE: include/sound/hda_component.h:55:
+static inline int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,
+						  const struct drm_audio_component_audio_ops *ops)

-:487: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#487: FILE: sound/hda/hdac_component.c:236:
+int snd_hdac_acomp_register_notifier(struct hdac_bus *bus,
+				    const struct drm_audio_component_audio_ops *aops)

-:1055: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#1055: FILE: sound/pci/hda/patch_hdmi.c:2546:
+	snd_hdac_acomp_register_notifier(&codec->bus->core,
 					&spec->drm_audio_ops);

total: 0 errors, 4 warnings, 10 checks, 968 lines checked

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-07-17  6:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-16 13:48 [PATCH 0/3] Make the audio component binding more generic Takashi Iwai
2018-07-16 13:48 ` [PATCH 1/3] drm/i915: Split audio component to a generic type Takashi Iwai
2018-07-16 17:22   ` [Intel-gfx] " Rodrigo Vivi
2018-07-16 17:50     ` Takashi Iwai
2018-07-16 20:35       ` Takashi Iwai
2018-07-16 13:48 ` [PATCH 2/3] ALSA: hda/i915: Associate audio component with devres Takashi Iwai
2018-07-16 13:48 ` [PATCH 3/3] ALSA: hda: Make audio component support more generic Takashi Iwai
2018-07-16 20:33   ` Takashi Iwai
2018-07-16 19:02 ` ✗ Fi.CI.BAT: failure for Make the audio component binding " Patchwork
2018-07-17  6:34 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.