Re: [PATCH v3 1/4] drm/i915: Add get_eld audio component

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Dec 04, 2015 at 06:12: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 audio status and ELD of the
> given HDMI/DP port.  It returns the size of expected ELD bytes if it's
> valid, zero if no valid ELD is found, or a negative error code.  The
> current state of audio on/off is stored in the given pointer, too.
> 
> Note that the returned size isn't limited to the given max bytes.  If
> the size is greater than the max bytes, it means that only a part of
> ELD has been copied back.
> 
> For achieving this implementation, a new field audio_connector is
> added to struct intel_digital_port.  It points to the connector
> assigned to the given digital port.  It's set/reset at each audio
> enable/disable call in intel_audio.c, and protected with av_mutex.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> v2->v3:
> * Track drm_connector for easier ELD retrieval, remove superfluous
>   audio_enabled flag instead
> * Back to av_mutex
> * Rebase to drm-next, update get_eld documentation for new kernel doc
> 
>  drivers/gpu/drm/i915/intel_audio.c | 42 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h   |  2 ++
>  include/drm/i915_component.h       | 14 +++++++++++++
>  3 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 9aa83e71b792..eeac9f763110 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -521,6 +521,10 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>  		dev_priv->display.audio_codec_enable(connector, intel_encoder,
>  						     adjusted_mode);
>  
> +	mutex_lock(&dev_priv->av_mutex);
> +	intel_dig_port->audio_connector = connector;

This still has the problem that the eld might get ovewritten while we're
only holding av_mutex below. But I think that can/should be solved as part
of my probe vs. modeset locking rework.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +	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);
>  }
> @@ -544,6 +548,10 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
>  	if (dev_priv->display.audio_codec_disable)
>  		dev_priv->display.audio_codec_disable(intel_encoder);
>  
> +	mutex_lock(&dev_priv->av_mutex);
> +	intel_dig_port->audio_connector = 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);
>  }
> @@ -703,6 +711,39 @@ 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;
> +	const u8 *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_connector != NULL;
> +			if (!*enabled)
> +				break;
> +			eld = intel_dig_port->audio_connector->eld;
> +			ret = drm_eld_size(eld);
> +			memcpy(buf, eld, min(max_bytes, 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,
> @@ -710,6 +751,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 ab5c147fa9e9..fe58a5722b16 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -814,6 +814,8 @@ struct intel_digital_port {
>  	struct intel_hdmi hdmi;
>  	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
>  	bool release_cl2_override;
> +	/* for communication with audio component; protected by av_mutex */
> +	const struct drm_connector *audio_connector;
>  };
>  
>  struct intel_dp_mst_encoder {
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index fab13851f95a..b46fa0ef3005 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -65,6 +65,20 @@ struct i915_audio_component_ops {
>  	 * sample rate, it will call this function to set n/cts
>  	 */
>  	int (*sync_audio_rate)(struct device *, int port, 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, bool *enabled,
> +		       unsigned char *buf, int max_bytes);
>  };
>  
>  /**
> -- 
> 2.6.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx