On Thu, Dec 15, 2016 at 10:18:13AM +0000, Anand, Jerome wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, December 14, 2016 5:21 PM To: Anand, Jerome jerome.anand@intel.com Cc: intel-gfx@lists.freedesktop.org; alsa-devel@alsa-project.org; ville.syrjala@linux.intel.com; broonie@kernel.org; pierre- louis.bossart@linux.intel.com; Ughreja, Rakesh A rakesh.a.ughreja@intel.com Subject: Re: [PATCH 2/7] drm/i915: Add support for audio driver notifications
On Mon, 12 Dec 2016 19:10:38 +0100, 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 | 46
++++++++++++++++++++++++++++++++++
include/drm/intel_lpe_audio.h | 1 + 5 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1317834..0dbe91a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3696,6 +3696,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_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 3bbc96c..77cd086 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> @@ -630,6 +631,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);}
/** @@ -663,6 +668,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);Does it still notify as connected?
OK - this will be changed
The entire 'connected' parameter seems superfluous.
Also why aren't we passing 'pipe' along here? How is the audio driver supposed to find the right thing to use?
}
/** diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 0bcfead..377584e1 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 e12e5f7..a141a9c 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -361,3 +361,49 @@ 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)) {
Just bail out here when !HAS_LPE_AUDIO(). Then the rest needs less indentation and will be more readable.
OK
struct intel_hdmi_lpe_audio_pdata *pdata =dev_get_platdata(
&(dev_priv->lpe_audio.platdev->dev));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)pdata->tmds_clock_speed =tmds_clk_speed;}If eld==NULL means that no ELD is found (or disconnected), it's better to clear pdata eld as well, so that we don't leak the previous ELD.
OK
pdata->hdmi_connected = connected;if (pdata->notify_audio_lpe)pdata->notify_audio_lpe((eld != NULL) ? &pdata->eld : NULL);elsepdata->notify_pending = true;spin_unlock_irqrestore(&pdata->lpe_audio_slock,irq_flags);- }
+} 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>
Why do we need to add a header at this point out of sudden?
It was for some warning removal. I'll try to remove this if not applicable now.
thanks,
Takashi
#define HDMI_MAX_ELD_BYTES 128
-- 2.9.3