From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup bridge for VED Date: Tue, 21 Oct 2014 13:29:31 +0300 Message-ID: <20141021102931.GZ4284@intel.com> References: <1413873402-4331-1-git-send-email-yao.cheng@intel.com> <1413873402-4331-2-git-send-email-yao.cheng@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <1413873402-4331-2-git-send-email-yao.cheng@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Yao Cheng Cc: ram.r.rao@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, fei.jiang@intel.com, michael.j.abel@intel.com, daniel.vetter@intel.com List-Id: intel-gfx@lists.freedesktop.org On Tue, Oct 21, 2014 at 02:36:41PM +0800, Yao Cheng wrote: > 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 the VED irqs. > VED driver (a standalone drm driver) probes the VED device and > creates a new dri card on install. > = > Currently only supports VED on valleyview. > Kerneldoc is updated for i915_ved.c. > = > Signed-off-by: Yao Cheng > --- > Documentation/DocBook/drm.tmpl | 5 + > drivers/gpu/drm/i915/Makefile | 3 + > drivers/gpu/drm/i915/i915_dma.c | 11 ++ > drivers/gpu/drm/i915/i915_drv.h | 9 ++ > drivers/gpu/drm/i915/i915_irq.c | 2 + > drivers/gpu/drm/i915/i915_reg.h | 5 + > drivers/gpu/drm/i915/i915_ved.c | 264 ++++++++++++++++++++++++++++++++++= ++++++ > 7 files changed, 299 insertions(+) > create mode 100644 drivers/gpu/drm/i915/i915_ved.c > = > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.t= mpl > index d7cfc98..f1787b4 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -3806,6 +3806,11 @@ int num_ioctls; > !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_disable_interrupts > !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts > > + > + VED video core integration > +!Pdrivers/gpu/drm/i915/i915_ved.c VED video core integration > +!Idrivers/gpu/drm/i915/i915_ved.c > + > > > Display Hardware Handling > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 3a6bce0..a4b9252 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -80,6 +80,9 @@ i915-y +=3D dvo_ch7017.o \ > i915-y +=3D i915_dma.o \ > i915_ums.o > = > +# VED for VLV > +i915-y +=3D i915_ved.o > + > obj-$(CONFIG_DRM_I915) +=3D i915.o > = > CFLAGS_i915_trace_points.o :=3D -I$(src) > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_= dma.c > index 85d14e1..47714e1 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1791,6 +1791,13 @@ int i915_driver_load(struct drm_device *dev, unsig= ned long flags) > if (IS_GEN5(dev)) > intel_gpu_ips_init(dev_priv); > = > + if (IS_VALLEYVIEW(dev)) { These must be (IS_VLV && !IS_CHV), or maybe define some HAS_VED() macro to hide that. > + ret =3D vlv_setup_ved(dev); > + if (ret < 0) { > + DRM_ERROR("failed to setup VED bridge: %d\n", ret); > + } > + } > + > intel_runtime_pm_enable(dev_priv); > = > return 0; > @@ -1833,6 +1840,10 @@ int i915_driver_unload(struct drm_device *dev) > struct drm_i915_private *dev_priv =3D dev->dev_private; > int ret; > = > + if (IS_VALLEYVIEW(dev)) { > + vlv_teardown_ved(dev); > + } > + > ret =3D i915_gem_suspend(dev); > if (ret) { > DRM_ERROR("failed to idle hardware: %d\n", ret); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index 821ba26..952df34 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1709,6 +1709,10 @@ struct drm_i915_private { > = > uint32_t bios_vgacntr; > = > + /* used for setup VED bridge */ > + struct platform_device *ved_platdev; > + int ved_irq; > + Could be neater to wrap this in a struct: struct { struct platform_device *platdev; int irq; } ved; > /* Old dri1 support infrastructure, beware the dragons ya fools entering > * here! */ > struct i915_dri1_state dri1; > @@ -2785,6 +2789,11 @@ void i915_restore_display_reg(struct drm_device *d= ev); > void i915_setup_sysfs(struct drm_device *dev_priv); > void i915_teardown_sysfs(struct drm_device *dev_priv); > = > +/* i915_ved.c */ > +int vlv_setup_ved(struct drm_device *dev); > +void vlv_teardown_ved(struct drm_device *dev); > +void vlv_ved_irq_handler(struct drm_device *dev); > + > /* 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 737b239..68c2977 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2177,6 +2177,8 @@ static irqreturn_t valleyview_irq_handler(int irq, = void *arg) > snb_gt_irq_handler(dev, dev_priv, gt_iir); > if (pm_iir) > gen6_rps_irq_handler(dev_priv, pm_iir); > + if (IS_VALLEYVIEW(dev) && (iir & VLV_VED_BLOCK_INTERRUPT)) > + vlv_ved_irq_handler(dev); This is the vlv irq handler so no need to check the platform type. > /* Call regardless, as some status bits might not be > * signalled in iir */ > valleyview_pipestat_irq_handler(dev, iir); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_= reg.h > index 2ed02c3..95421ef 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1284,6 +1284,11 @@ enum punit_power_well { > #define VLV_DISPLAY_BASE 0x180000 > #define VLV_MIPI_BASE VLV_DISPLAY_BASE > = > +/* forwarding VED irq and sharing MMIO to the VED driver */ > +#define VLV_VED_BLOCK_INTERRUPT (1 << 23) This define should be alongside the other IxR bits. > +#define VLV_VED_BASE 0x170000 > +#define VLV_VED_SIZE 0x010000 > + > #define VLV_GU_CTL0 (VLV_DISPLAY_BASE + 0x2030) > #define VLV_GU_CTL1 (VLV_DISPLAY_BASE + 0x2034) > #define SCPD0 0x0209c /* 915+ only */ > diff --git a/drivers/gpu/drm/i915/i915_ved.c b/drivers/gpu/drm/i915/i915_= ved.c > new file mode 100644 > index 0000000..5dfe4af > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_ved.c > @@ -0,0 +1,264 @@ > +/* > + * Copyright =A9 2014 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sublicen= se, > + * 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, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR = OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISI= NG > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER D= EALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Yao Cheng > + * > + */ > + > +#include "i915_drv.h" > + > +/** > + * DOC: VED video core integration > + * > + * Motivation: > + * Some platforms (e.g. valleyview) integrates a VED inside GPU to exten= d the > + * video decoding capability. > + * The VED is driven by the standalone drm driver "ipvr" which covers Po= werVR > + * VPUs. Since the PowerVR VPUs are also integrated by non-i915 platform= s such > + * as GMA500, we'd like to keep ipvr driver and i915 driver separated and > + * independent to each other. To achieve this we do the minimum work in = i915 > + * to setup a bridge between ipvr 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 VED irqs. > + * ipvr driver probes the VED device and creates a new dri card on insta= ll. > + * > + * Threats: > + * Due to the restriction in Linux platform device model, user need manu= ally > + * uninstall ipvr driver before uninstalling i915 module, otherwise he m= ight > + * run into use-after-free issues after i915 removes the platform device. > + * > + * Implementation: > + * The MMIO/REG platform resources are created according to the registers > + * specification. > + * When forwarding VED irqs, the flow control handler selection depends = on the > + * platform, for example on valleyview handle_simple_irq is enough. > + * > + */ > + > +static struct platform_device* vlv_ved_platdev_create(struct drm_device = *dev) > +{ > + int ret; > + struct resource *rsc =3D NULL; > + struct platform_device *platdev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + > + platdev =3D platform_device_alloc("ipvr-ved-vlv", -1); > + if (!platdev) { > + DRM_ERROR("Failed to allocate VED platform device\n"); > + ret =3D -ENOMEM; > + goto err; > + } > + > + rsc =3D kzalloc(sizeof(*rsc) * 3, GFP_KERNEL); struct resource seems small enough that this could be on the stack. > + if (!rsc) { > + DRM_ERROR("Failed to allocate resource for VED platform device\n"); > + ret =3D -ENOMEM; > + goto err; > + } > + > + WARN_ON(dev_priv->ved_irq < 0); > + rsc[0].start =3D rsc[0].end =3D dev_priv->ved_irq; > + rsc[0].flags =3D IORESOURCE_IRQ; > + rsc[0].name =3D "ipvr-ved-vlv-irq"; > + > + /* MMIO/REG for child's use */ > + rsc[1].start =3D pci_resource_start(dev->pdev, 0); > + rsc[1].end =3D pci_resource_start(dev->pdev, 0) + 2*1024*1024; /* = gen7 */ > + rsc[1].flags =3D IORESOURCE_MEM; > + rsc[1].name =3D "ipvr-ved-vlv-mmio"; > + > + rsc[2].start =3D VLV_VED_BASE; > + rsc[2].end =3D VLV_VED_BASE + VLV_VED_SIZE; > + rsc[2].flags =3D IORESOURCE_REG; > + rsc[2].name =3D "ipvr-ved-vlv-reg"; I don't see why you need both MEM and REG resources. Just the MEM itself should suffice: rsc[1].start =3D pci_resource_start(dev->pdev, 0) + VLV_VED_BASE; rsc[1].end =3D pci_resource_start(dev->pdev, 0) + VLV_VED_BASE + VLV_V= ED_SIZE; rsc[1].flags =3D IORESOURCE_MEM; rsc[1].name =3D "ipvr-ved-vlv-mmio"; Also in the ved driver you're mapping it with ioremap_wc() which isn't generally safe to do with registers. I'm not sure the kernel would even allow it since i915 has already mapped it as UC. > + > + ret =3D platform_device_add_resources(platdev, rsc, 3); > + if (ret) { > + DRM_ERROR("Failed to add resource for VED platform device: %d\n", ret); > + goto err; > + } > + > + /* Runtime-PM hook */ > + platdev->dev.parent =3D dev->dev; > + ret =3D platform_device_add(platdev); > + if (ret) { > + DRM_ERROR("Failed to add VED platform device: %d\n", ret); > + goto err; > + } > + > + kfree(rsc); > + return platdev; > +err: > + if (rsc) > + kfree(rsc); > + if (platdev) > + platform_device_unregister(platdev); > + return ERR_PTR(ret); > +} > + > +static void vlv_ved_platdev_destroy(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + if (dev_priv->ved_platdev) > + platform_device_unregister(dev_priv->ved_platdev); > +} > + > +static void vlv_ved_irq_unmask(struct irq_data *d) > +{ > + struct drm_device *dev =3D d->chip_data; > + struct drm_i915_private *dev_priv =3D (struct drm_i915_private *) dev->= dev_private; > + unsigned long irqflags; > + u32 imr, ier; > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > + > + ier =3D I915_READ(VLV_IER); > + ier |=3D VLV_VED_BLOCK_INTERRUPT; > + I915_WRITE(VLV_IER, ier); > + POSTING_READ(VLV_IER); > + > + imr =3D I915_READ(VLV_IMR); > + imr &=3D ~VLV_VED_BLOCK_INTERRUPT; > + dev_priv->irq_mask =3D imr; > + I915_WRITE(VLV_IMR, imr); > + POSTING_READ(VLV_IMR); I think you could just follow the same procedure that valleyview_display_irqs_install() uses: dev_priv->irq_mask &=3D ~VLV_VED_BLOCK_INTERRUPT; I915_WRITE(VLV_IIR, VLV_VED_BLOCK_INTERRUPT); I915_WRITE(VLV_IIR, VLV_VED_BLOCK_INTERRUPT); I915_WRITE(VLV_IMR, dev_priv->irq_mask); I915_WRITE(VLV_IER, ~dev_priv->irq_mask); POSTING_READ(VLV_IER); > + > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > +} > + > +static void vlv_ved_irq_mask(struct irq_data *d) > +{ > + struct drm_device *dev =3D d->chip_data; > + struct drm_i915_private *dev_priv =3D (struct drm_i915_private *) dev->= dev_private; > + unsigned long irqflags; > + u32 imr, ier; > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > + > + ier =3D I915_READ(VLV_IER); > + ier &=3D ~VLV_VED_BLOCK_INTERRUPT; > + I915_WRITE(VLV_IER, ier); > + POSTING_READ(VLV_IER); > + > + imr =3D I915_READ(VLV_IMR); > + imr |=3D VLV_VED_BLOCK_INTERRUPT; > + dev_priv->irq_mask =3D imr; > + I915_WRITE(VLV_IMR, imr); > + POSTING_READ(VLV_IMR); Same here. > + > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > +} > + > +static struct irq_chip vlv_ved_irqchip =3D { > + .name =3D "ipvr_ved_irqchip", > + .irq_mask =3D vlv_ved_irq_mask, > + .irq_unmask =3D vlv_ved_irq_unmask, > +}; > + > +int vlv_ved_irq_init(struct drm_device *dev, int irq) > +{ > + struct drm_i915_private *dev_priv =3D (struct drm_i915_private *) dev->= dev_private; > + WARN_ON(!intel_irqs_enabled(dev_priv)); > + irq_set_chip_and_handler_name(irq, > + &vlv_ved_irqchip, > + handle_simple_irq, > + "ipvr_ved_vlv_irq_handler"); > + return irq_set_chip_data(irq, dev); > +} > + > +void vlv_ved_irq_handler(struct drm_device *dev) > +{ > + int ret; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + if (dev_priv->ved_irq < 0 && printk_ratelimit()) { > + DRM_ERROR("invalid ved irq number: %d\n", dev_priv->ved_irq); > + return; > + } > + ret =3D generic_handle_irq(dev_priv->ved_irq); > + if (ret && printk_ratelimit()) { > + DRM_ERROR("error handling vlv ved irq: %d\n", ret); > + } > +} > + > +/** > + * vlv_setup_ved() - setup the bridge between VED driver and i915 > + * @dev: the i915 drm device > + * > + * set up the minimum required resources for the bridge: irq chip, platf= orm > + * 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 vlv_setup_ved(struct drm_device *dev) > +{ > + int irq; > + int ret; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + struct platform_device *platdev; > + dev_priv->ved_irq =3D -1; > + dev_priv->ved_platdev =3D NULL; > + > + irq =3D irq_alloc_descs(-1, 0, 1, 0); > + if (irq < 0) { > + DRM_ERROR("Failed to allocate IRQ desc: %d\n", irq); > + ret =3D -ENOMEM; > + goto err; > + } > + > + ret =3D vlv_ved_irq_init(dev, irq); > + if (ret) { > + DRM_ERROR("Failed to initialize irqchip for vlv-ved: %d\n", ret); > + goto err; > + } > + dev_priv->ved_irq =3D irq; > + > + platdev =3D vlv_ved_platdev_create(dev); > + if (IS_ERR(platdev)) { > + DRM_ERROR("Failed to create platform device for vlv-ved: %ld\n", PTR_E= RR(platdev)); > + ret =3D -EINVAL; > + goto err; > + } > + > + dev_priv->ved_platdev =3D platdev; > + return 0; > +err: > + vlv_teardown_ved(dev); > + return ret; > +} > + > +/** > + * vlv_teardown_ved() - destroy the bridge between VED driver and i915 > + * @dev: the i915 drm device > + * > + * release all the resources for VED <-> i915 bridge. > + */ > +void vlv_teardown_ved(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + vlv_ved_platdev_destroy(dev); > + dev_priv->ved_platdev =3D NULL; > + if (dev_priv->ved_irq >=3D 0) { > + irq_free_desc(dev_priv->ved_irq); > + } > + dev_priv->ved_irq =3D -1; > +} > -- = > 1.9.1 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC