On Fri, Nov 25, 2016 at 05:42:38AM +0000, Anand, Jerome wrote: > > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > > Sent: Thursday, November 24, 2016 7:02 PM > > To: Anand, Jerome <jerome.anand@xxxxxxxxx> > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; > > broonie@xxxxxxxxxx; tiwai@xxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; > > Ughreja, Rakesh A <rakesh.a.ughreja@xxxxxxxxx> > > Subject: Re: [RFC PATCH v3 1/7] drm/i915: setup bridge for HDMI LPE audio > > driver > > > > On Fri, Nov 25, 2016 at 05:25:42AM +0530, Jerome Anand wrote: > > > Enable support for HDMI LPE audio mode on Baytrail and Cherrytrail > > > when HDaudio controller is not detected > > > > > > Setup minimum required resources during i915_driver_load: > > > 1. Create a platform device to share MMIO/IRQ resources 2. Make the > > > platform device child of i915 device for runtime PM. > > > 3. Create IRQ chip to forward HDMI LPE audio irqs. > > > > > > HDMI LPE audio driver (a standalone sound driver) probes the LPE audio > > > device and creates a new sound card. > > > > > > Signed-off-by: Pierre-Louis Bossart > > > <pierre-louis.bossart@xxxxxxxxxxxxxxx> > > > Signed-off-by: Jerome Anand <jerome.anand@xxxxxxxxx> > > > --- > > > Documentation/gpu/i915.rst | 9 + > > > drivers/gpu/drm/i915/Makefile | 3 + > > > drivers/gpu/drm/i915/i915_drv.c | 8 +- > > > drivers/gpu/drm/i915/i915_drv.h | 15 ++ > > > drivers/gpu/drm/i915/i915_irq.c | 29 +++ > > > drivers/gpu/drm/i915/i915_reg.h | 3 + > > > drivers/gpu/drm/i915/intel_lpe_audio.c | 369 > > +++++++++++++++++++++++++++++++++ > > > include/drm/intel_lpe_audio.h | 45 ++++ > > > 8 files changed, 479 insertions(+), 2 deletions(-) create mode > > > 100644 drivers/gpu/drm/i915/intel_lpe_audio.c > > > create mode 100644 include/drm/intel_lpe_audio.h > > > > > > diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst > > > index 117d2ab..a671eee 100644 > > > --- a/Documentation/gpu/i915.rst > > > +++ b/Documentation/gpu/i915.rst > > > @@ -213,6 +213,15 @@ Video BIOS Table (VBT) .. kernel-doc:: > > > drivers/gpu/drm/i915/intel_vbt_defs.h > > > :internal: > > > > > > +intel hdmi lpe audio support > > > +---------------------------- > > > + > > > +.. kernel-doc:: drivers/gpu/drm/i915/intel_lpe_audio.c > > > + :doc: LPE Audio integration for HDMI or DP playback > > > + > > > +.. kernel-doc:: drivers/gpu/drm/i915/intel_lpe_audio.c > > > + :internal: > > > + > > > Memory Management and Command Submission > > > ======================================== > > > > > > diff --git a/drivers/gpu/drm/i915/Makefile > > > b/drivers/gpu/drm/i915/Makefile index 580602d..3a2369c 100644 > > > --- a/drivers/gpu/drm/i915/Makefile > > > +++ b/drivers/gpu/drm/i915/Makefile > > > @@ -126,6 +126,9 @@ i915-y += intel_gvt.o include $(src)/gvt/Makefile > > > endif > > > > > > +# LPE Audio for VLV and CHT > > > +i915-y += intel_lpe_audio.o > > > + > > > obj-$(CONFIG_DRM_I915) += i915.o > > > > > > CFLAGS_i915_trace_points.o := -I$(src) diff --git > > > a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > index b893e67..93811ed 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1144,7 +1144,8 @@ static void i915_driver_register(struct > > drm_i915_private *dev_priv) > > > if (IS_GEN5(dev_priv)) > > > intel_gpu_ips_init(dev_priv); > > > > > > - i915_audio_component_init(dev_priv); > > > + if (intel_lpe_audio_init(dev_priv) < 0) > > > + i915_audio_component_init(dev_priv); > > > > > > /* > > > * Some ports require correctly set-up hpd registers for detection > > > to @@ -1162,7 +1163,10 @@ static void i915_driver_register(struct > > drm_i915_private *dev_priv) > > > */ > > > static void i915_driver_unregister(struct drm_i915_private *dev_priv) > > > { > > > - i915_audio_component_cleanup(dev_priv); > > > + if (HAS_LPE_AUDIO(dev_priv)) > > > + intel_lpe_audio_teardown(dev_priv); > > > + else > > > + i915_audio_component_cleanup(dev_priv); > > > > > > intel_gpu_ips_teardown(); > > > acpi_video_unregister(); > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h index 970e50b..2a79048 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -2284,6 +2284,12 @@ struct drm_i915_private { > > > /* Used to save the pipe-to-encoder mapping for audio */ > > > struct intel_encoder *av_enc_map[I915_MAX_PIPES]; > > > > > > + /* necessary resource sharing with HDMI LPE audio driver. */ > > > + struct { > > > + struct platform_device *platdev; > > > + int irq; > > > + } lpe_audio; > > > + > > > /* > > > * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your > > patch > > > * will be rejected. Instead look for a better place. > > > @@ -2773,6 +2779,8 @@ intel_info(const struct drm_i915_private > > > *dev_priv) > > > > > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu) > > > > > > +#define HAS_LPE_AUDIO(dev_priv) ((dev_priv)->lpe_audio.platdev != > > > +NULL) > > > + > > > #define INTEL_PCH_DEVICE_ID_MASK 0xff00 > > > #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 > > > #define INTEL_PCH_CPT_DEVICE_ID_TYPE 0x1c00 > > > @@ -3547,6 +3555,13 @@ extern int i915_restore_state(struct drm_device > > > *dev); void i915_setup_sysfs(struct drm_i915_private *dev_priv); > > > void i915_teardown_sysfs(struct drm_i915_private *dev_priv); > > > > > > +/* i915_lpe_audio.c */ > > > +int intel_lpe_audio_init(struct drm_i915_private *dev_priv); 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); > > > + > > > /* intel_i2c.c */ > > > extern int intel_setup_gmbus(struct drm_device *dev); extern void > > > intel_teardown_gmbus(struct drm_device *dev); diff --git > > > a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > > index 07ca71c..6c98b28 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -1894,6 +1894,15 @@ static irqreturn_t valleyview_irq_handler(int irq, > > void *arg) > > > valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats); > > > > > > /* > > > + * LPE audio interrupts are only enabled on Baytrail and > > > + * CherryView platforms without HDaudio > > > + */ > > > + if (iir & (I915_LPE_PIPE_A_INTERRUPT | > > > + I915_LPE_PIPE_B_INTERRUPT | > > > + I915_LPE_PIPE_C_INTERRUPT)) > > > > No pipe C on vlv. > > > > OK > > > > + intel_lpe_audio_irq_handler(dev_priv); > > > + > > > + /* > > > * VLV_IIR is single buffered, and reflects the level > > > * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last. > > > */ > > > @@ -1974,6 +1983,15 @@ static irqreturn_t cherryview_irq_handler(int irq, > > void *arg) > > > valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats); > > > > > > /* > > > + * LPE audio interrupts are only enabled on Baytrail and > > > + * CherryView platforms without HDaudio > > > + */ > > > + if (iir & (I915_LPE_PIPE_A_INTERRUPT | > > > + I915_LPE_PIPE_B_INTERRUPT | > > > + I915_LPE_PIPE_C_INTERRUPT)) > > > + intel_lpe_audio_irq_handler(dev_priv); > > > + > > > + /* > > > * VLV_IIR is single buffered, and reflects the level > > > * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last. > > > */ > > > @@ -2930,6 +2948,17 @@ static void vlv_display_irq_postinstall(struct > > > drm_i915_private *dev_priv) > > > > > > WARN_ON(dev_priv->irq_mask != ~0); > > > > > > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > > > > This is vlv/chv specific code, so no need to check for it. > > > > OK > > > > + /* add interrupt masks unconditially here, the actual unmask > > > + * will take place only if the LPE_AUDIO mode is detected > > > + */ > > > + u32 val = (I915_LPE_PIPE_A_INTERRUPT | > > > + I915_LPE_PIPE_B_INTERRUPT | > > > + I915_LPE_PIPE_C_INTERRUPT); > > > + > > > + enable_mask |= val; > > > + } > > > + > > > dev_priv->irq_mask = ~enable_mask; > > > > > > GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask); diff --git > > > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index fcad8fa..6b16ed0 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -2395,6 +2395,9 @@ enum skl_disp_power_wells { > > > #define I915_ASLE_INTERRUPT (1<<0) > > > #define I915_BSD_USER_INTERRUPT > > (1<<25) > > > > > > +#define I915_HDMI_LPE_AUDIO_BASE (VLV_DISPLAY_BASE + > > 0x65000) > > > +#define I915_HDMI_LPE_AUDIO_SIZE 0x1000 > > > + > > > #define GEN6_BSD_RNCID _MMIO(0x12198) > > > > > > #define GEN7_FF_THREAD_MODE _MMIO(0x20a0) > > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c > > > b/drivers/gpu/drm/i915/intel_lpe_audio.c > > > new file mode 100644 > > > index 0000000..5335fc6 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c > > > @@ -0,0 +1,369 @@ > > > +/* > > > + * Copyright © 2016 Intel Corporation > > > + * > > > + * Permission is hereby granted, free of charge, to any person > > > +obtaining a > > > + * copy of this software and associated documentation files (the > > > +"Software"), > > > + * to deal in the Software without restriction, including without > > > +limitation > > > + * the rights to use, copy, modify, merge, publish, distribute, > > > +sublicense, > > > + * and/or sell copies of the Software, and to permit persons to whom > > > +the > > > + * Software is furnished to do so, subject to the following conditions: > > > + * > > > + * The above copyright notice and this permission notice (including > > > +the next > > > + * paragraph) shall be included in all copies or substantial portions > > > +of the > > > + * Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > > KIND, > > > +EXPRESS OR > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > > +MERCHANTABILITY, > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN > > NO EVENT > > > +SHALL > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > > DAMAGES > > > +OR OTHER > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > > OTHERWISE, > > > +ARISING > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE > > OR > > > +OTHER DEALINGS > > > + * IN THE SOFTWARE. > > > + * > > > + * Authors: > > > + * Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > > > + * Jerome Anand <jerome.anand@xxxxxxxxx> > > > + * based on VED patches > > > + * > > > + */ > > > + > > > +/** > > > + * DOC: LPE Audio integration for HDMI or DP playback > > > + * > > > + * Motivation: > > > + * Atom platforms (e.g. valleyview and cherryTrail) integrates a > > > +DMA-based > > > + * interface as an alternative to the traditional HDaudio path. While > > > +this > > > + * mode is unrelated to the LPE aka SST audio engine, the > > > +documentation refers > > > + * to this mode as LPE so we keep this notation for the sake of > > consistency. > > > + * > > > + * The interface is handled by a separate standalone driver > > > +maintained in the > > > + * ALSA subsystem for simplicity. To minimize the interaction between > > > +the two > > > + * subsystems, a bridge is setup between the hdmi-lpe-audio and i915: > > > + * 1. Create a platform device to share MMIO/IRQ resources > > > + * 2. Make the platform device child of i915 device for runtime PM. > > > + * 3. Create IRQ chip to forward the LPE audio irqs. > > > + * the hdmi-lpe-audio driver probes the lpe audio device and creates > > > +a new > > > + * sound card > > > + * > > > + * Threats: > > > + * Due to the restriction in Linux platform device model, user need > > > +manually > > > + * uninstall the hdmi-lpe-audio driver before uninstalling i915 > > > +module, > > > + * otherwise we might run into use-after-free issues after i915 > > > +removes the > > > + * platform device: even though hdmi-lpe-audio driver is released, > > > +the modules > > > + * is still in "installed" status. > > > + * > > > + * Implementation: > > > + * The MMIO/REG platform resources are created according to the > > > +registers > > > + * specification. > > > + * When forwarding LPE audio irqs, the flow control handler selection > > > +depends > > > + * on the platform, for example on valleyview handle_simple_irq is > > enough. > > > + * > > > + */ > > > + > > > +#include <linux/acpi.h> > > > +#include <linux/device.h> > > > +#include <linux/pci.h> > > > + > > > +#include "i915_drv.h" > > > +#include <linux/delay.h> > > > +#include <drm/intel_lpe_audio.h> > > > + > > > +static struct platform_device* > > > +lpe_audio_platdev_create(struct drm_i915_private *dev_priv) { > > > + struct drm_device *dev = &dev_priv->drm; > > > + int ret; > > > + struct resource rsc[2] = {}; > > > + struct platform_device *platdev; > > > + u64 *dma_mask; > > > + struct intel_hdmi_lpe_audio_pdata *pdata; > > > + > > > + if (dev_priv->lpe_audio.irq < 0) > > > + return ERR_PTR(-EINVAL); > > > + > > > + platdev = platform_device_alloc("hdmi-lpe-audio", -1); > > > + if (!platdev) { > > > + ret = -ENOMEM; > > > + DRM_ERROR("Failed to allocate LPE audio platform > > device\n"); > > > + goto err; > > > + } > > > + > > > + /* to work-around check_addr in nommu_map_sg() */ > > > + dma_mask = kmalloc(sizeof(*platdev->dev.dma_mask), > > GFP_KERNEL); > > > + if (!dma_mask) { > > > + ret = -ENOMEM; > > > + DRM_ERROR("Failed to allocate dma_mask\n"); > > > + goto err_put_dev; > > > + } > > > + *dma_mask = DMA_BIT_MASK(32); > > > + platdev->dev.dma_mask = dma_mask; > > > + platdev->dev.coherent_dma_mask = *dma_mask; > > > + > > > + rsc[0].start = rsc[0].end = dev_priv->lpe_audio.irq; > > > + rsc[0].flags = IORESOURCE_IRQ; > > > + rsc[0].name = "hdmi-lpe-audio-irq"; > > > + > > > + rsc[1].start = pci_resource_start(dev->pdev, 0) + > > > + I915_HDMI_LPE_AUDIO_BASE; > > > + rsc[1].end = pci_resource_start(dev->pdev, 0) + > > > + I915_HDMI_LPE_AUDIO_BASE + > > I915_HDMI_LPE_AUDIO_SIZE - 1; > > > + rsc[1].flags = IORESOURCE_MEM; > > > + rsc[1].name = "hdmi-lpe-audio-mmio"; > > > + > > > + ret = platform_device_add_resources(platdev, rsc, 2); > > > + if (ret) { > > > + DRM_ERROR("Failed to add resource for platform device: > > %d\n", > > > + ret); > > > + goto err_put_dev; > > > + } > > > + > > > + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); > > > + > > > + if (pdata == NULL) { > > > + ret = -ENOMEM; > > > + goto err_put_dev; > > > + } > > > + platdev->dev.platform_data = pdata; > > > + spin_lock_init(&pdata->lpe_audio_slock); > > > + > > > + /* for LPE audio driver's runtime-PM */ > > > + platdev->dev.parent = dev->dev; > > > + ret = platform_device_add(platdev); > > > + if (ret) { > > > + DRM_ERROR("Failed to add LPE audio platform device: > > %d\n", > > > + ret); > > > + goto err_put_dev; > > > + } > > > + > > > + return platdev; > > > +err_put_dev: > > > + platform_device_put(platdev); > > > + kfree(dma_mask); > > > +err: > > > + return ERR_PTR(ret); > > > +} > > > + > > > +static void lpe_audio_platdev_destroy(struct drm_i915_private > > > +*dev_priv) { > > > + if (!HAS_LPE_AUDIO(dev_priv)) > > > + return; > > > + platform_device_unregister(dev_priv->lpe_audio.platdev); > > > + kfree(dev_priv->lpe_audio.platdev->dev.dma_mask); > > > +} > > > + > > > +static void lpe_audio_irq_unmask(struct irq_data *d) { > > > + struct drm_device *dev = d->chip_data; > > > + struct drm_i915_private *dev_priv = to_i915(dev); > > > + unsigned long irqflags; > > > + u32 val = (I915_LPE_PIPE_A_INTERRUPT | > > > + I915_LPE_PIPE_B_INTERRUPT | > > > + I915_LPE_PIPE_C_INTERRUPT); > > > > No pipe C on vlv. > > > > OK > > > > + > > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > > + > > > + /* > > > + * VLV_IER is already set in the vlv_display_postinstall(), > > > + * we only change VLV_IIR and VLV_IMR > > > + */ > > > + dev_priv->irq_mask &= ~val; > > > + I915_WRITE(VLV_IIR, val); > > > + I915_WRITE(VLV_IIR, val); > > > + I915_WRITE(VLV_IMR, dev_priv->irq_mask); > > > + POSTING_READ(VLV_IMR); > > > + > > > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } > > > + > > > +static void lpe_audio_irq_mask(struct irq_data *d) { > > > + struct drm_device *dev = d->chip_data; > > > + struct drm_i915_private *dev_priv = to_i915(dev); > > > + unsigned long irqflags; > > > + u32 val = (I915_LPE_PIPE_A_INTERRUPT | > > > + I915_LPE_PIPE_B_INTERRUPT | > > > + I915_LPE_PIPE_C_INTERRUPT); > > > > ditto > > > > OK > > > > + > > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > > + > > > + /* > > > + * VLV_IER is already set in the vlv_display_postinstall(), > > > + * we only change VLV_IIR and VLV_IMR > > > + */ > > > + dev_priv->irq_mask |= val; > > > + I915_WRITE(VLV_IMR, dev_priv->irq_mask); > > > + I915_WRITE(VLV_IIR, val); > > > + I915_WRITE(VLV_IIR, val); > > > + POSTING_READ(VLV_IIR); > > > + > > > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } > > > + > > > +static struct irq_chip lpe_audio_irqchip = { > > > + .name = "hdmi_lpe_audio_irqchip", > > > + .irq_mask = lpe_audio_irq_mask, > > > + .irq_unmask = lpe_audio_irq_unmask, > > > +}; > > > + > > > +static int lpe_audio_irq_init(struct drm_i915_private *dev_priv) { > > > + int irq = dev_priv->lpe_audio.irq; > > > + > > > + WARN_ON(!intel_irqs_enabled(dev_priv)); > > > + irq_set_chip_and_handler_name(irq, > > > + &lpe_audio_irqchip, > > > + handle_simple_irq, > > > + "hdmi_lpe_audio_irq_handler"); > > > + > > > + return irq_set_chip_data(irq, &dev_priv->drm); > > > > Just pass dev_priv as the chip data. > > > > OK > > > > +} > > > + > > > +/** > > > + * intel_lpe_audio_irq_handler() - forwards the LPE audio irq > > > + * @dev_priv: the i915 drm device private data > > > + * > > > + * the LPE Audio irq is forwarded to the irq handler registered by > > > +LPE audio > > > + * driver. > > > + */ > > > +void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv) { > > > + int ret; > > > + > > > + ret = generic_handle_irq(dev_priv->lpe_audio.irq); > > > + if (ret) > > > + DRM_ERROR_RATELIMITED("error handling LPE audio irq: > > %d\n", > > > + ret); > > > +} > > > + > > > +/** > > > + * intel_lpe_audio_detect() - check & setup lpe audio if present > > > + * @dev_priv: the i915 drm device private data > > > + * > > > + * Detect if lpe audio is present > > > + * > > > + * Return: true if lpe audio present else Return = false */ bool > > > +intel_lpe_audio_detect(struct drm_i915_private *dev_priv) { > > > + int lpe_present = false; > > > + > > > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > > > > Could avoid indenting the whole thig by reversing this test and doing an early > > return. > > > > Will prefer the existing method > > > > + static const struct pci_device_id atom_hdaudio_ids[] = { > > > + /* Baytrail */ > > > + {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0f04)}, > > > + /* Braswell */ > > > + {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x2284)}, > > > + {} > > > + }; > > > > IIRC I asked if we could use a HDA class match instead. But I forget what the > > answer was. No presumably? > > > > The previous reply was " we are not aware of any such class that exists at present" Hmm. 0x0403 would be audio device. I guess we can't really use that unless we combine it with the devfn (assuming the hda controller is alwauys device 3. So yeah, specific device IDs seem better. > > > > + > > > + if (!pci_dev_present(atom_hdaudio_ids)) { > > > + DRM_INFO("%s%s\n", "HDaudio controller not > > detected,", > > > + "using LPE audio instead\n"); > > > + lpe_present = true; > > > + } > > > + } > > > + return lpe_present; > > > +} > > > + > > > +/** > > > + * intel_lpe_audio_setup() - setup the bridge between HDMI LPE Audio > > > + * driver and i915 > > > + * @dev_priv: the i915 drm device private data > > > + * > > > + * set up the minimum required resources for the bridge: irq chip, > > > + * platform resource and platform device. i915 device is set as > > > +parent > > > + * of the new platform device. > > > + * > > > + * Return: 0 if successful. non-zero if allocation/initialization > > > +fails */ int intel_lpe_audio_setup(struct drm_i915_private > > > +*dev_priv) { > > > + int ret; > > > + > > > + dev_priv->lpe_audio.irq = irq_alloc_desc(0); > > > + if (dev_priv->lpe_audio.irq < 0) { > > > + DRM_ERROR("Failed to allocate IRQ desc: %d\n", > > > + dev_priv->lpe_audio.irq); > > > + ret = dev_priv->lpe_audio.irq; > > > + goto err; > > > + } > > > + > > > + DRM_DEBUG("irq = %d\n", dev_priv->lpe_audio.irq); > > > + > > > + ret = lpe_audio_irq_init(dev_priv); > > > + > > > + if (ret) { > > > + DRM_ERROR("Failed to initialize irqchip for lpe audio: %d\n", > > > + ret); > > > + goto err_free_irq; > > > + } > > > + > > > + dev_priv->lpe_audio.platdev = lpe_audio_platdev_create(dev_priv); > > > + > > > + if (IS_ERR(dev_priv->lpe_audio.platdev)) { > > > + ret = PTR_ERR(dev_priv->lpe_audio.platdev); > > > + DRM_ERROR("Failed to create lpe audio platform device: > > %d\n", > > > + ret); > > > + goto err_free_irq; > > > + } > > > + > > > + return 0; > > > +err_free_irq: > > > + irq_free_desc(dev_priv->lpe_audio.irq); > > > +err: > > > + dev_priv->lpe_audio.irq = -1; > > > + dev_priv->lpe_audio.platdev = NULL; > > > + return ret; > > > +} > > > + > > > + > > > +/** > > > + * intel_lpe_audio_init() - detect and setup the bridge between HDMI > > > +LPE Audio > > > + * driver and i915 > > > + * @dev_priv: the i915 drm device private data > > > + * > > > + * Return: 0 if successful. non-zero if detection or > > > + * llocation/initialization fails > > > + */ > > > +int intel_lpe_audio_init(struct drm_i915_private *dev_priv) { > > > + int ret = -ENODEV; > > > + > > > + if (intel_lpe_audio_detect(dev_priv)) { > > > + ret = intel_lpe_audio_setup(dev_priv); > > > + if (ret < 0) > > > + DRM_ERROR("failed to setup LPE Audio bridge\n"); > > > + } > > > + return ret; > > > +} > > > + > > > +/** > > > + * intel_lpe_audio_teardown() - destroy the bridge between HDMI LPE > > > + * audio driver and i915 > > > + * @dev_priv: the i915 drm device private data > > > + * > > > + * release all the resources for LPE audio <-> i915 bridge. > > > + */ > > > +void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv) { > > > + unsigned long irqflags; > > > + struct irq_desc *desc; > > > + > > > + desc = (dev_priv->lpe_audio.irq >= 0) ? > > > + irq_to_desc(dev_priv->lpe_audio.irq) : NULL; > > > > Why not just "if (lpe not initialized) return;" ? > > > > Need to perform cleaning in spite of it Why? Partial initialization doesn't sounds good. > > > > + > > > + /** > > > + * mask LPE audio irq before destroying > > > + */ > > > + if (desc) > > > + lpe_audio_irq_mask(&desc->irq_data); > > > > This seems racy. Should unregister the platdev first I think, > > > > OK > > > > + > > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > > + > > > + lpe_audio_platdev_destroy(dev_priv); > > > + > > > + if (desc) > > > + irq_free_desc(dev_priv->lpe_audio.irq); > > > + > > > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } > > > diff --git a/include/drm/intel_lpe_audio.h > > > b/include/drm/intel_lpe_audio.h new file mode 100644 index > > > 0000000..a64c449 > > > --- /dev/null > > > +++ b/include/drm/intel_lpe_audio.h > > > @@ -0,0 +1,45 @@ > > > +/* > > > + * Copyright © 2016 Intel Corporation > > > + * > > > + * Permission is hereby granted, free of charge, to any person > > > +obtaining a > > > + * copy of this software and associated documentation files (the > > > +"Software"), > > > + * to deal in the Software without restriction, including without > > > +limitation > > > + * the rights to use, copy, modify, merge, publish, distribute, > > > +sublicense, > > > + * and/or sell copies of the Software, and to permit persons to whom > > > +the > > > + * Software is furnished to do so, subject to the following conditions: > > > + * > > > + * The above copyright notice and this permission notice (including > > > +the next > > > + * paragraph) shall be included in all copies or substantial portions > > > +of the > > > + * Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > > KIND, > > > +EXPRESS OR > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > > +MERCHANTABILITY, > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN > > NO EVENT > > > +SHALL > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > > DAMAGES > > > +OR OTHER > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > > OTHERWISE, > > > +ARISING > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE > > OR > > > +OTHER DEALINGS > > > + * IN THE SOFTWARE. > > > + */ > > > + > > > +#ifndef _INTEL_LPE_AUDIO_H_ > > > +#define _INTEL_LPE_AUDIO_H_ > > > + > > > +#include <linux/types.h> > > > + > > > +#define HDMI_MAX_ELD_BYTES 128 > > > + > > > +struct intel_hdmi_lpe_audio_eld { > > > + int port_id; > > > + unsigned char eld_data[HDMI_MAX_ELD_BYTES]; }; > > > + > > > +struct intel_hdmi_lpe_audio_pdata { > > > + bool notify_pending; > > > + int tmds_clock_speed; > > > + bool hdmi_connected; > > > + struct intel_hdmi_lpe_audio_eld eld; > > > + void (*notify_audio_lpe)(void *audio_ptr); > > > + spinlock_t lpe_audio_slock; > > > +}; > > > + > > > +#endif /* _I915_LPE_AUDIO_H_ */ > > > -- > > > 2.9.3 > > > > -- > > Ville Syrjälä > > Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
