[alsa-devel] [PATCH v2 1/3] drm/i915: Split audio component to a generic type
Rodrigo Vivi
rodrigo.vivi at intel.com
Tue Jul 17 22:06:30 CEST 2018
On Tue, Jul 17, 2018 at 11:26:50AM +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.
>
> The license of drm_audio_component.h is kept to MIT as same as the the
> original i915_component.h.
>
> This is a preliminary change for further development, and no
> functional changes by this patch itself, merely code-split and
> renames.
>
> v1->v2: Use SPDX for drm_audio_component.h, fix remaining i915
> argument in drm_audio_component.h
>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 22 +++----
> include/drm/drm_audio_component.h | 95 ++++++++++++++++++++++++++++++
> 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, 144 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..e85689f212c2
> --- /dev/null
> +++ b/include/drm/drm_audio_component.h
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: MIT
> +// Copyright © 2014 Intel Corporation
> +
> +#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: drm module to pin down
> + */
> + 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
>
More information about the Alsa-devel
mailing list