On Mon, Nov 30, 2015 at 05:24:41PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 30, 2015 at 02:37:46PM +0100, Takashi Iwai wrote:
> > Implement a new i915_audio_component_ops, get_eld(). It's called by
> > the audio driver to fetch the current ELD of the given HDMI/DP port.
> > It returns the size of ELD bytes if it's valid, or zero if the audio
> > is disabled or unplugged, or a negative error code.
>
> Why do we need this? Isn't it something the eld notify hook should
> pass from i915 to the audio driver?
>
> At least with the locking you have for this, the audio driver can not
> call this from the eld notify hook since it would deadlock.
Hmm. Actually the locking isn't perhaps quite like that atm. But I guess
the mode_config.mutex will make it so.
Apart from that it seesm to me that you should pull the av_mutex
lock/unlock from the .audio_code_eanble/disable hooks into
intel_audio_codec_enable/disable, so that it protects the audio_enabled
flag as well. Not sure if the eld_notify should be called while holding
that lock or not. If we need to avoid calling it from the eld_notify
anywya due to other locks then maybe it can be under av_mutex as well.
>
> >
> > For achieving this, a new field audio_enabled is added to struct
> > intel_digital_port. This is set/reset at each audio enable/disable
> > call in intel_audio.c.
> >
> > Signed-off-by: Takashi Iwai <ti...@suse.de>
> > ---
> > drivers/gpu/drm/i915/intel_audio.c | 40
> > ++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > include/drm/i915_component.h | 3 +++
> > 3 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 0c38cc6c82ae..6b318a8d5dc9 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -521,6 +521,7 @@ void intel_audio_codec_enable(struct intel_encoder
> > *intel_encoder)
> >
> > connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> >
> > + intel_dig_port->audio_enabled = true;
> > if (dev_priv->display.audio_codec_enable)
> > dev_priv->display.audio_codec_enable(connector, intel_encoder,
> > adjusted_mode);
> > @@ -545,6 +546,7 @@ void intel_audio_codec_disable(struct intel_encoder
> > *intel_encoder)
> > struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> > enum port port = intel_dig_port->port;
> >
> > + intel_dig_port->audio_enabled = false;
> > if (dev_priv->display.audio_codec_disable)
> > dev_priv->display.audio_codec_disable(intel_encoder);
> >
> > @@ -702,6 +704,43 @@ static int i915_audio_component_sync_audio_rate(struct
> > device *dev,
> > return 0;
> > }
> >
> > +static int i915_audio_component_get_eld(struct device *dev, int port,
> > + bool *enabled,
> > + unsigned char *buf, int max_bytes)
> > +{
> > + struct drm_i915_private *dev_priv = dev_to_i915(dev);
> > + struct drm_device *drm_dev = dev_priv->dev;
> > + struct intel_encoder *intel_encoder;
> > + struct intel_digital_port *intel_dig_port;
> > + struct drm_connector *connector;
> > + unsigned char *eld;
> > + int ret = -EINVAL;
> > +
> > + mutex_lock(&dev_priv->av_mutex);
> > + for_each_intel_encoder(drm_dev, intel_encoder) {
> > + if (intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT &&
> > + intel_encoder->type != INTEL_OUTPUT_HDMI)
> > + continue;
> > + intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> > + if (port == intel_dig_port->port) {
> > + ret = 0;
> > + *enabled = intel_dig_port->audio_enabled;
> > + if (!*enabled)
> > + break;
> > + connector = drm_select_eld(&intel_encoder->base);
> > + if (!connector)
> > + break;
> > + eld = connector->eld;
> > + ret = min(max_bytes, drm_eld_size(eld));
> > + memcpy(buf, eld, ret);
> > + break;
> > + }
> > + }
> > +
> > + mutex_unlock(&dev_priv->av_mutex);
> > + return ret;
> > +}
> > +
> > static const struct i915_audio_component_ops i915_audio_component_ops = {
> > .owner = THIS_MODULE,
> > .get_power = i915_audio_component_get_power,
> > @@ -709,6 +748,7 @@ static const struct i915_audio_component_ops
> > i915_audio_component_ops = {
> > .codec_wake_override = i915_audio_component_codec_wake_override,
> > .get_cdclk_freq = i915_audio_component_get_cdclk_freq,
> > .sync_audio_rate = i915_audio_component_sync_audio_rate,
> > + .get_eld = i915_audio_component_get_eld,
> > };
> >
> > static int i915_audio_component_bind(struct device *i915_dev,
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 0598932ce623..4afc7560be04 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -798,6 +798,7 @@ struct intel_digital_port {
> > u32 saved_port_bits;
> > struct intel_dp dp;
> > struct intel_hdmi hdmi;
> > + bool audio_enabled;
> > enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
> > bool release_cl2_override;
> > };
> > diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> > index 30d89e0da2c6..058d39e8d57f 100644
> > --- a/include/drm/i915_component.h
> > +++ b/include/drm/i915_component.h
> > @@ -38,6 +38,7 @@
> > * @codec_wake_override: Enable/Disable generating the codec wake signal
> > * @get_cdclk_freq: get the Core Display Clock in KHz
> > * @sync_audio_rate: set n/cts based on the sample rate
> > + * @get_eld: fill the audio state and ELD bytes for the given port
> > */
> > struct i915_audio_component_ops {
> > struct module *owner;
> > @@ -46,6 +47,8 @@ struct i915_audio_component_ops {
> > void (*codec_wake_override)(struct device *, bool enable);
> > int (*get_cdclk_freq)(struct device *);
> > int (*sync_audio_rate)(struct device *, int port, int rate);
> > + int (*get_eld)(struct device *, int port, bool *enabled,
> > + unsigned char *buf, int max_bytes);
> > };
> >
> > struct i915_audio_component_audio_ops {
> > --
> > 2.6.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
