Re: [PATCH 2/7] drm/i915: Add get_eld audio component

Ville Syrjälä
2015-11-30T15:24:45Z

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.

>
> 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 <tiwai@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