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

Submitted by Takashi Iwai on Dec. 4, 2015, 5:12 p.m.

Details

Message ID 1449249169-20602-2-git-send-email-tiwai@suse.de
State Accepted
Commit cae666ceb8c3f154351f7df29c522f7a7016bdc0
Headers show
Series "Add get_eld audio component for i915/HD-audio" ( rev: 5 4 ) in Intel GFX

Browsing this patch as part of:
"Add get_eld audio component for i915/HD-audio" rev 5 in Intel GFX
<< prev patch [1/3] next patch >>

Commit Message

Takashi Iwai Dec. 4, 2015, 5:12 p.m.
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(+)

Patch hide | download patch | download mbox

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;
+	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);
 };
 
 /**

Comments

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