[Intel-gfx] [PATCH v2] drm/i915/dp: DP audio API changes for MST
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Aug 10 14:21:18 UTC 2016
On Tue, Aug 09, 2016 at 01:58:33PM -0700, Dhinakaran Pandiyan wrote:
> DP MST provides the capability to send multiple video and audio streams
> through a single port. This requires the API's between i915 and audio
> drivers to distinguish between multiple audio capable displays that can be
> connected to a port. Currently only the port identity is shared in the
> APIs. This patch adds support for MST with an additional parameter
> 'int pipe'. The existing parameter 'port' does not change it's meaning.
>
> pipe =
> MST : display pipe that the stream originates from
> Non-MST : -1
>
> Affected APIs:
> struct i915_audio_component_ops
> - int (*sync_audio_rate)(struct device *, int port, int rate);
> + int (*sync_audio_rate)(struct device *, int port, int pipe,
> + int rate);
>
> - int (*get_eld)(struct device *, int port, bool *enabled,
> - unsigned char *buf, int max_bytes);
> + int (*get_eld)(struct device *, int port, int pipe,
> + bool *enabled, unsigned char *buf, int max_bytes);
>
> struct i915_audio_component_audio_ops
> - void (*pin_eld_notify)(void *audio_ptr, int port);
> + void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
>
> This patch makes dummy changes in the audio drivers (Libin) for build to
> succeed. The audio side drivers will send the right 'pipe' values in
> patches that will follow.
>
> v2:
> Renamed the new API parameter from 'dev_id' to 'pipe'. (Jim, Ville)
> Included Asoc driver API compatibility changes from Jeeja.
> Added WARN_ON() for invalid pipe in get_saved_encoder(). (Takashi)
> Added comment for av_enc_map[] definition. (Takashi)
>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_audio.c | 92 ++++++++++++++++++++++++++------------
> include/drm/i915_component.h | 6 +--
> include/sound/hda_i915.h | 11 ++---
> sound/hda/hdac_i915.c | 9 ++--
> sound/pci/hda/patch_hdmi.c | 7 +--
> sound/soc/codecs/hdac_hdmi.c | 2 +-
> 7 files changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c36d176..8e4a88f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2036,7 +2036,8 @@ struct drm_i915_private {
> /* perform PHY state sanity checks? */
> bool chv_phy_assert[2];
>
> - struct intel_encoder *dig_port_map[I915_MAX_PORTS];
> + /* Used to save the pipe-to-encoder mapping for audio */
> + struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index ef20875..240dad2 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -500,6 +500,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> struct i915_audio_component *acomp = dev_priv->audio_component;
> struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> enum port port = intel_dig_port->port;
> + enum pipe pipe = -1;
>
> connector = drm_select_eld(encoder);
> if (!connector)
> @@ -524,12 +525,18 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>
> mutex_lock(&dev_priv->av_mutex);
> intel_encoder->audio_connector = connector;
> +
> /* referred in audio callbacks */
> - dev_priv->dig_port_map[port] = intel_encoder;
> + dev_priv->av_enc_map[pipe] = intel_encoder;
[-1] doesn't look right.
> mutex_unlock(&dev_priv->av_mutex);
>
> +
> + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> + pipe = crtc->pipe;
> +
> if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> - acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port);
> + acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> + (int) port, (int) pipe);
> }
>
> /**
> @@ -542,22 +549,28 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
> {
> struct drm_encoder *encoder = &intel_encoder->base;
> + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct i915_audio_component *acomp = dev_priv->audio_component;
> struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> enum port port = intel_dig_port->port;
> + enum pipe pipe = -1;
>
> if (dev_priv->display.audio_codec_disable)
> dev_priv->display.audio_codec_disable(intel_encoder);
>
> mutex_lock(&dev_priv->av_mutex);
> intel_encoder->audio_connector = NULL;
> - dev_priv->dig_port_map[port] = NULL;
> + dev_priv->av_enc_map[pipe] = NULL;
same
> mutex_unlock(&dev_priv->av_mutex);
>
> + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> + pipe = crtc->pipe;
> +
> if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> - acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port);
> + acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> + (int) port, (int) pipe);
> }
>
> /**
> @@ -632,15 +645,39 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
> return dev_priv->cdclk_freq;
> }
>
> -static int i915_audio_component_sync_audio_rate(struct device *dev,
> - int port, int rate)
> +static struct intel_encoder *get_saved_encoder(struct intel_encoder *av_enc_map[],
> + int port, int pipe)
> +{
> + struct drm_encoder *encoder;
> +
> + if (WARN_ON(pipe >= I915_MAX_PIPES))
> + return NULL;
> +
> + /* MST */
> + if (pipe != -1)
> + return av_enc_map[pipe];
> +
> + /* Non-MST */
> + for (pipe = PIPE_A; pipe < I915_MAX_PIPES; pipe++) {
> + if (!av_enc_map[pipe])
> + continue;
> +
> + encoder = &av_enc_map[pipe]->base;
> + if (port == enc_to_dig_port(encoder)->port)
> + return av_enc_map[pipe];
> + }
> +
> + return NULL;
> +}
> +
> +static int i915_audio_component_sync_audio_rate(struct device *dev, int port,
> + int pipe, int rate)
> {
> struct drm_i915_private *dev_priv = dev_to_i915(dev);
> struct intel_encoder *intel_encoder;
> struct intel_crtc *crtc;
> struct drm_display_mode *mode;
> struct i915_audio_component *acomp = dev_priv->audio_component;
> - enum pipe pipe = INVALID_PIPE;
> u32 tmp;
> int n;
> int err = 0;
> @@ -654,25 +691,20 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
>
> i915_audio_component_get_power(dev);
> mutex_lock(&dev_priv->av_mutex);
> +
> /* 1. get the pipe */
> - intel_encoder = dev_priv->dig_port_map[port];
> - /* intel_encoder might be NULL for DP MST */
> + intel_encoder = get_saved_encoder(dev_priv->av_enc_map, port, pipe);
> if (!intel_encoder || !intel_encoder->base.crtc ||
> intel_encoder->type != INTEL_OUTPUT_HDMI) {
> - DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
> + DRM_DEBUG_KMS("Not valid for port %c\n", port_name(port));
> err = -ENODEV;
> goto unlock;
> }
> +
> + /* pipe passed from the audio driver will be -1 for non-mst case */
> crtc = to_intel_crtc(intel_encoder->base.crtc);
> pipe = crtc->pipe;
> - if (pipe == INVALID_PIPE) {
> - DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
> - err = -ENODEV;
> - goto unlock;
> - }
>
> - DRM_DEBUG_KMS("pipe %c connects port %c\n",
> - pipe_name(pipe), port_name(port));
> mode = &crtc->config->base.adjusted_mode;
>
> /* port must be valid now, otherwise the pipe will be invalid */
> @@ -708,7 +740,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
> }
>
> static int i915_audio_component_get_eld(struct device *dev, int port,
> - bool *enabled,
> + int pipe, bool *enabled,
> unsigned char *buf, int max_bytes)
> {
> struct drm_i915_private *dev_priv = dev_to_i915(dev);
> @@ -717,16 +749,20 @@ static int i915_audio_component_get_eld(struct device *dev, int port,
> int ret = -EINVAL;
>
> mutex_lock(&dev_priv->av_mutex);
> - intel_encoder = dev_priv->dig_port_map[port];
> - /* intel_encoder might be NULL for DP MST */
> - if (intel_encoder) {
> - ret = 0;
> - *enabled = intel_encoder->audio_connector != NULL;
> - if (*enabled) {
> - eld = intel_encoder->audio_connector->eld;
> - ret = drm_eld_size(eld);
> - memcpy(buf, eld, min(max_bytes, ret));
> - }
> +
> + intel_encoder = get_saved_encoder(dev_priv->av_enc_map, port, pipe);
> + if (!intel_encoder) {
> + DRM_DEBUG_KMS("Not valid for port %c\n", port_name(port));
> + mutex_unlock(&dev_priv->av_mutex);
> + return ret;
> + }
> +
> + ret = 0;
> + *enabled = intel_encoder->audio_connector != NULL;
> + if (*enabled) {
> + eld = intel_encoder->audio_connector->eld;
> + ret = drm_eld_size(eld);
> + memcpy(buf, eld, min(max_bytes, ret));
> }
>
> mutex_unlock(&dev_priv->av_mutex);
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index b46fa0e..545c6e0 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -64,7 +64,7 @@ struct i915_audio_component_ops {
> * 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 rate);
> + 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
> *
> @@ -77,7 +77,7 @@ struct i915_audio_component_ops {
> * 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.
> */
Pleas update the docs to include what the pipe parameter means.
Looks like most of these docs could use a little beefing up to make
sure both sides follow the same contract. Right now there are a lot
of grey areas.
> - int (*get_eld)(struct device *, int port, bool *enabled,
> + int (*get_eld)(struct device *, int port, int pipe, bool *enabled,
> unsigned char *buf, int max_bytes);
> };
>
> @@ -97,7 +97,7 @@ struct i915_audio_component_audio_ops {
> * status accordingly (even when the HDA controller is in power save
> * mode).
> */
> - void (*pin_eld_notify)(void *audio_ptr, int port);
> + void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);
> };
>
> /**
> diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
> index 796cabf..07fd64e 100644
> --- a/include/sound/hda_i915.h
> +++ b/include/sound/hda_i915.h
> @@ -10,8 +10,9 @@
> 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 rate);
> -int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid,
> +int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
> + int pipe, int rate);
> +int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int pipe,
> 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);
> @@ -29,13 +30,13 @@ 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 rate)
> + hda_nid_t nid, int pipe, int rate)
> {
> return 0;
> }
> static inline int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid,
> - bool *audio_enabled, char *buffer,
> - int max_bytes)
> + int pipe, bool *audio_enabled,
> + char *buffer, int max_bytes)
> {
> return -ENODEV;
> }
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index c9af022..b99994b 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -201,7 +201,8 @@ static int pin2port(struct hdac_device *codec, hda_nid_t pin_nid)
> * 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 rate)
> +int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid,
> + int pipe, int rate)
> {
> struct hdac_bus *bus = codec->bus;
> struct i915_audio_component *acomp = bus->audio_component;
> @@ -212,7 +213,7 @@ int snd_hdac_sync_audio_rate(struct hdac_device *codec, hda_nid_t nid, int rate)
> port = pin2port(codec, nid);
> if (port < 0)
> return -EINVAL;
> - return acomp->ops->sync_audio_rate(acomp->dev, port, rate);
> + return acomp->ops->sync_audio_rate(acomp->dev, port, pipe, rate);
> }
> EXPORT_SYMBOL_GPL(snd_hdac_sync_audio_rate);
>
> @@ -236,7 +237,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_sync_audio_rate);
> * 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 snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid, int pipe,
> bool *audio_enabled, char *buffer, int max_bytes)
> {
> struct hdac_bus *bus = codec->bus;
> @@ -249,7 +250,7 @@ int snd_hdac_acomp_get_eld(struct hdac_device *codec, hda_nid_t nid,
> port = pin2port(codec, nid);
> if (port < 0)
> return -EINVAL;
> - return acomp->ops->get_eld(acomp->dev, port, audio_enabled,
> + return acomp->ops->get_eld(acomp->dev, port, pipe, audio_enabled,
> buffer, max_bytes);
> }
> EXPORT_SYMBOL_GPL(snd_hdac_acomp_get_eld);
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index d0d5ad8..67890df 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1485,7 +1485,7 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
>
> mutex_lock(&per_pin->lock);
> eld->monitor_present = false;
> - size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid,
> + size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid, -1,
> &eld->monitor_present, eld->eld_buffer,
> ELD_MAX_SIZE);
> if (size > 0) {
> @@ -1739,7 +1739,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> /* Call sync_audio_rate to set the N/CTS/M manually if necessary */
> /* Todo: add DP1.2 MST audio support later */
> if (codec_has_acomp(codec))
> - snd_hdac_sync_audio_rate(&codec->core, pin_nid, runtime->rate);
> + snd_hdac_sync_audio_rate(&codec->core, pin_nid, -1,
> + runtime->rate);
>
> non_pcm = check_non_pcm_per_cvt(codec, cvt_nid);
> mutex_lock(&per_pin->lock);
> @@ -2285,7 +2286,7 @@ 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);
> }
>
> -static void intel_pin_eld_notify(void *audio_ptr, int port)
> +static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe)
> {
> struct hda_codec *codec = audio_ptr;
> int pin_nid;
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 2abb742..cf57ab3 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -1366,7 +1366,7 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev,
> return hdac_hdmi_init_dai_map(edev);
> }
>
> -static void hdac_hdmi_eld_notify_cb(void *aptr, int port)
> +static void hdac_hdmi_eld_notify_cb(void *aptr, int port, int pipe)
> {
> struct hdac_ext_device *edev = aptr;
> struct hdac_hdmi_priv *hdmi = edev->private_data;
> --
> 2.5.0
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list