-----Original Message----- From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] Sent: Thursday, November 24, 2016 7:03 PM To: Anand, Jerome jerome.anand@intel.com Cc: intel-gfx@lists.freedesktop.org; alsa-devel@alsa-project.org; broonie@kernel.org; tiwai@suse.de; pierre-louis.bossart@linux.intel.com; Ughreja, Rakesh A rakesh.a.ughreja@intel.com Subject: Re: [RFC PATCH v3 2/7] drm/i915: Add support for audio driver notifications
On Fri, Nov 25, 2016 at 05:25:43AM +0530, Jerome Anand wrote:
Notifiations like mode change, hot plug and edid to the audio driver are added. This is inturn used by the audio driver for its functionality.
A new interface file capturing the notifications needed by the audio driver is added
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Jerome Anand jerome.anand@intel.com
drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/intel_audio.c | 8 ++++++ drivers/gpu/drm/i915/intel_hdmi.c | 1 + drivers/gpu/drm/i915/intel_lpe_audio.c | 49
++++++++++++++++++++++++++++++++++
include/drm/intel_lpe_audio.h | 1 + 5 files changed, 62 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2a79048..33bc44c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3561,6 +3561,9 @@ int intel_lpe_audio_setup(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); bool intel_lpe_audio_detect(struct drm_i915_private *dev_priv); +void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
void *eld, int port, int tmds_clk_speed,bool connected);/* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 1c509f7..55a6831 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -24,6 +24,7 @@ #include <linux/kernel.h> #include <linux/component.h> #include <drm/i915_component.h> +#include <drm/intel_lpe_audio.h> #include "intel_drv.h"
#include <drm/drmP.h> @@ -627,6 +628,10 @@ void intel_audio_codec_enable(struct
intel_encoder *intel_encoder,
if (acomp && acomp->audio_ops && acomp->audio_ops- pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops- audio_ptr, (int) port, (int) pipe);
- if (HAS_LPE_AUDIO(dev_priv))
intel_lpe_audio_notify(dev_priv, connector->eld, port,crtc_state->port_clock, true);}
/** @@ -660,6 +665,9 @@ void intel_audio_codec_disable(struct
intel_encoder *intel_encoder)
if (acomp && acomp->audio_ops && acomp->audio_ops- pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops- audio_ptr, (int) port, (int) pipe);
- if (HAS_LPE_AUDIO(dev_priv))
intel_lpe_audio_notify(dev_priv, NULL, port, 0, true);}
/** diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index fb88e32..02d50e3 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -36,6 +36,7 @@ #include <drm/drm_edid.h> #include "intel_drv.h" #include <drm/i915_drm.h> +#include <drm/intel_lpe_audio.h> #include "i915_drv.h"
static struct drm_device *intel_hdmi_to_dev(struct intel_hdmi *intel_hdmi) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 5335fc6..93f83cb 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -367,3 +367,52 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); }
+/**
- intel_lpe_audio_notify() - notify lpe audio event
- audio driver and i915
- @dev_priv: the i915 drm device private data
- @eld : ELD data
- @port: port id
- @tmds_clk_speed: tmds clock frequency in Hz
- @connected: hdmi connected/disconnected
- Notify lpe audio driver of eld change.
- */
+void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
void *eld, int port, int tmds_clk_speed,bool connected)+{
- unsigned long irq_flags;
- if (HAS_LPE_AUDIO(dev_priv)) {
struct intel_hdmi_lpe_audio_pdata *pdata =dev_get_platdata(
&(dev_priv->lpe_audio.platdev->dev));if (pdata) {How could the !pdata case happen? If it can't we shouldn't cater for it.
It can happen if there is a notification before audio driver is initialized
spin_lock_irqsave(&pdata->lpe_audio_slock,irq_flags);if (eld != NULL) {memcpy(pdata->eld.eld_data, eld,HDMI_MAX_ELD_BYTES);pdata->eld.port_id = port;if (tmds_clk_speed)Why the if?
Tmds of zero is sent in codec_disable. Hence added, just to avoid unnecessary notification to audio driver
pdata->tmds_clock_speed =tmds_clk_speed;}pdata->hdmi_connected = connected;Also can't really see much need for this. Any one of the port/tmds clock/eld should be sufficient.
Eld and tmds are needed to cater to hotplug and display mode change notifications. Port can be avoided but kept for debug
if (pdata->notify_audio_lpe)pdata->notify_audio_lpe((eld != NULL) ? &pdata->eld : NULL);elsepdata->notify_pending = true;Still not sure why the "pending" thing is useful. Can't the audio driver just do its thing (whatever it is) unconditionally?
This is added to avoid race when audio driver loads late and the notification from display has already passed.
When disabling just clear the port to INVALID, eld to zero, and tmds clock to 0, and it should all be fine no?
Yes, that's what is being done.
spin_unlock_irqrestore(&pdata->lpe_audio_slock,irq_flags);} elseDRM_DEBUG_DRIVER("no audio notification\n");- }
+} diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index a64c449..952de05 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -25,6 +25,7 @@ #define _INTEL_LPE_AUDIO_H_
#include <linux/types.h> +#include <linux/spinlock_types.h>
#define HDMI_MAX_ELD_BYTES 128
-- 2.9.3
-- Ville Syrjälä Intel OTC