> -----Original Message-----
> From: Pandiyan, Dhinakaran
> Sent: Friday, August 5, 2016 2:42 PM
> To: Yang, Libin <[email protected]>
> Cc: [email protected]; [email protected]; alsa-devel@alsa-
> project.org; [email protected]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: DP audio API changes for MST
>
> On Fri, 2016-08-05 at 06:21 +0000, Yang, Libin wrote:
> > > -----Original Message-----
> > > From: Pandiyan, Dhinakaran
> > > Sent: Friday, August 5, 2016 1:57 PM
> > > To: Yang, Libin <[email protected]>
> > > Cc: [email protected]; [email protected]; alsa-devel@alsa-
> > > project.org; [email protected]
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: DP audio API changes
> > > for MST
> > >
> > > On Fri, 2016-08-05 at 02:35 +0000, Yang, Libin wrote:
> > > > > -----Original Message-----
> > > > > From: Intel-gfx [mailto:[email protected]]
> > > > > On Behalf Of Dhinakaran Pandiyan
> > > > > Sent: Wednesday, August 3, 2016 10:15 AM
> > > > > To: [email protected]
> > > > > Cc: [email protected]; [email protected];
> > > > > [email protected]; Pandiyan, Dhinakaran
> > > > > <[email protected]>
> > > > > Subject: [Intel-gfx] [PATCH] drm/i915/dp: DP audio API changes
> > > > > for MST
> > > > >
> > > > > DP MST provides the capability to send multiple video and audio
> > > > > streams via one single port. This requires the API's between
> > > > > i915 and audio drivers to distinguish between audio capable
> > > > > displays connected to a port. This patch adds this support via
> > > > > an additional parameter 'int dev_id'. The existing parameter
> > > > > 'port' does not change it's
> > > meaning.
> > > > >
> > > > > dev_id =
> > > > > MST : 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 dev_id,
> > > > > + 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 dev_id,
> > > > > + 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
> > > > > + dev_id);
> > > > >
> > > > > This patch makes dummy changes in the audio drivers for build to
> succeed.
> > > > >
> > > > > Signed-off-by: Dhinakaran Pandiyan
> > > > > <[email protected]>
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > > > > drivers/gpu/drm/i915/intel_audio.c | 82
> > > > > +++++++++++++++++++++++++++++--
> > > > > -------
> > > > > 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 ++--
> > > > > 6 files changed, 82 insertions(+), 35 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > > b/drivers/gpu/drm/i915/i915_drv.h index 65ada5d..8e4c8c8 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -2054,7 +2054,7 @@ struct drm_i915_private {
> > > > > /* perform PHY state sanity checks? */
> > > > > bool chv_phy_assert[2];
> > > > >
> > > > > - struct intel_encoder *dig_port_map[I915_MAX_PORTS];
> > > > > + struct intel_encoder *av_enc_map[I915_MAX_PIPES];
> > > >
> > > > We may still need save the port number info.
> > > > In non-MST mode, device entry means nothing. This means
> > > > av_enc_map[pipe] may not get the right intel_encoder
> > > >
> > > > Regards,
> > > > Libin
> > > >
> > > We find the encoder using port in get_saved_encoder()
> >
> > It seems it still can't get the encoder for SST/HDMI as pipe may
> > always be -1 or 0 for SST/HDMI?
> >
> > Regards,
> > Libin
> >
>
> The audio driver sends dev_id=-1, but the correct "port" for SST. So, we loop
> through all the saved encoders and choose the one that is tied to "port"
>
> 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];
> }
Get your mean. Thanks.
>
>
>
> > > > >
> > > > > /*
> > > > > * 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 8c493de..cbe44c8 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > > > @@ -500,6 +500,9 @@ 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 = crtc->pipe;
> > > > > + int dev_id = -1;
> > > > > +
> > > > >
> > > > > connector = drm_select_eld(encoder);
> > > > > if (!connector)
> > > > > @@ -522,14 +525,19 @@ void intel_audio_codec_enable(struct
> > > > > intel_encoder
> > > > > *intel_encoder)
> > > > > dev_priv->display.audio_codec_enable(connector,
> > > > > intel_encoder,
> > > > > adjusted_mode);
> > > > >
> > > > > + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> > > > > + dev_id = pipe;
> > > > > +
> > > > > 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;
> > > > > mutex_unlock(&dev_priv->av_mutex);
> > > > >
> > > > > 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, dev_id);
> > > > > }
> > > > >
> > > > > /**
> > > > > @@ -542,22 +550,29 @@ 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 = crtc->pipe;
> > > > > + int dev_id = -1;
> > > > > +
> > > > > + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> > > > > + dev_id = pipe;
> > > > >
> > > > > 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;
> > > > > mutex_unlock(&dev_priv->av_mutex);
> > > > >
> > > > > 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, dev_id);
> > > > > }
> > > > >
> > > > > /**
> > > > > @@ -628,8 +643,32 @@ 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 dev_id) {
> > > > > +
> > > > > + enum pipe pipe;
> > > > > + struct drm_encoder *encoder;
> > > > > +
> > > > > + if (dev_id >= 0 && dev_id < I915_MAX_PIPES)
> > > > > + return av_enc_map[dev_id];
> > > > > +
> > > > > + 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 dev_id, int rate)
> > > > > {
> > > > > struct drm_i915_private *dev_priv = dev_to_i915(dev);
> > > > > struct intel_encoder *intel_encoder; @@ -649,15 +688,16 @@
> > > > > static int i915_audio_component_sync_audio_rate(struct device *dev,
> > > > > return 0;
> > > > >
> > > > > 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,
> > > > > dev_id);
> > > > > if (!intel_encoder || !intel_encoder->base.crtc ||
> > > > > intel_encoder->type != INTEL_OUTPUT_HDMI) {
> > > > > DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
> > > > > err = -ENODEV;
> > > > > goto unlock;
> > > > > }
> > > > > +
> > > > > crtc = to_intel_crtc(intel_encoder->base.crtc);
> > > > > pipe = crtc->pipe;
> > > > > if (pipe == INVALID_PIPE) {
> > > > > @@ -702,7 +742,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 dev_id, bool *enabled,
> > > > > unsigned char *buf, int
> > > > > max_bytes) {
> > > > > struct drm_i915_private *dev_priv = dev_to_i915(dev); @@
> > > > > -710,17
> > > > > +750,21 @@ static int i915_audio_component_get_eld(struct device
> > > > > +*dev, int
> > > > > port,
> > > > > const u8 *eld;
> > > > > 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,
> > > > > dev_id);
> > > > > + if (!intel_encoder) {
> > > > > + 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..1419c98 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 dev_id,
> > > > > +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.
> > > > > */
> > > > > - int (*get_eld)(struct device *, int port, bool *enabled,
> > > > > + int (*get_eld)(struct device *, int port, int dev_id, 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 dev_id);
> > > > > };
> > > > >
> > > > > /**
> > > > > diff --git a/include/sound/hda_i915.h b/include/sound/hda_i915.h
> > > > > index 796cabf..5ab972e 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 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); @@ -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 dev_id,
> 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 dev_id, 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..6ea63ac 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 dev_id, 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,
> dev_id,
> > > > > +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 dev_id,
> > > > > 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, dev_id,
> > > > > 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..077d48a 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
> > > > > +dev_id)
> > > > > {
> > > > > struct hda_codec *codec = audio_ptr;
> > > > > int pin_nid;
> > > > > --
> > > > > 2.5.0
> > > > >
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > [email protected]
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > [email protected]
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
