Re: [RFC PATCH V4 1/4] drm/i915: add i915_ved.c to setup bridge for VED

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+ commenters of v1~v3

Thanks,
Yao

> -----Original Message-----
> From: Sean V Kelley [mailto:seanvk@xxxxxxxxx]
> Sent: Thursday, January 8, 2015 8:35
> To: Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; Cheng, Yao; Sean V Kelley
> Subject: [RFC PATCH V4 1/4] drm/i915: add i915_ved.c to setup bridge for
> VED
> 
> From: Yao Cheng <yao.cheng@xxxxxxxxx>
> 
> 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.
> 
> v2:
> take Daniel & Jani's comments
> 	- extract change to new file i915_ved.c
> 	- add kerneldoc
> 	- change 'ipvr-ved' to 'ipvr-vlv-ved' for extensibility
> 	- unregister platdev before irq_free_desc
> 	- add WARN_ON(!intel_irqs_enabled) in irq init code
> 	- remove unnecessary trace point
> 	- remove unnecessary BUG_ON
> 
> v3:
> take Ville's comments and VED PRIME support
> 	- add HAS_VED() check
> 	- add ved struct to make code neat
> 	- no need to check platform in vlv_irq_handler
> 	- i915_reg.h update
> 	- no need to kmalloc for small amount of resource
> 	- remove unnecessary REG resource
> 	- follow vlv_display_irqs_install() to implement VED mask/unmask
> 	- workaround nommu_map_sg issue by set dma_mask to support
> VED PRIME.
> 
> v4:
> take Bob's comments
> 	- add more detail on the use-after-free issue description
> 	- mask VED irq before removing the child device
> 
> Signed-off-by: Yao Cheng <yao.cheng@xxxxxxxxx>
> Signed-off-by: Sean V Kelley <seanvk@xxxxxxxxx>
> ---
>  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
> |  12 ++
>  drivers/gpu/drm/i915/i915_irq.c |   2 +
>  drivers/gpu/drm/i915/i915_reg.h |   4 +
>  drivers/gpu/drm/i915/i915_ved.c | 270
> ++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 307 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/i915_ved.c
> 
> diff --git a/Documentation/DocBook/drm.tmpl
> b/Documentation/DocBook/drm.tmpl index 56e2a9b..9db989c 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -3867,6 +3867,11 @@ int
> num_ioctls;</synopsis>  !Fdrivers/gpu/drm/i915/i915_irq.c
> intel_runtime_pm_disable_interrupts
>  !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts
>        </sect2>
> +      <sect2>
> +        <title>VED video core integration</title>
> +!Pdrivers/gpu/drm/i915/i915_ved.c VED video core integration
> +!Idrivers/gpu/drm/i915/i915_ved.c
> +      </sect2>
>      </sect1>
>      <sect1>
>        <title>Display Hardware Handling</title> diff --git
> a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index
> e4083e4..7d0bbfa 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -84,6 +84,9 @@ i915-y += dvo_ch7017.o \  i915-y += i915_dma.o \
>  	  i915_ums.o
> 
> +# VED for VLV
> +i915-y += i915_ved.o
> +
>  obj-$(CONFIG_DRM_I915)  += i915.o
> 
>  CFLAGS_i915_trace_points.o := -I$(src)
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c index 887d88f..cd96618 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -828,6 +828,13 @@ int i915_driver_load(struct drm_device *dev,
> unsigned long flags)
>  	if (IS_GEN5(dev))
>  		intel_gpu_ips_init(dev_priv);
> 
> +	if (HAS_VED(dev)) {
> +		ret = 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;
> @@ -870,6 +877,10 @@ int i915_driver_unload(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
> 
> +	if (HAS_VED(dev)) {
> +		vlv_teardown_ved(dev);
> +	}
> +
>  	ret = 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 502a01b..aa39d47 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1773,6 +1773,12 @@ struct drm_i915_private {
> 
>  	uint32_t bios_vgacntr;
> 
> +	/* necessary resource sharing with ved driver. */
> +	struct {
> +		struct platform_device *platdev;
> +		int	irq;
> +	} ved;
> +
>  	/* Abstract the submission mechanism (legacy ringbuffer or execlists)
> away */
>  	struct {
>  		int (*do_execbuf)(struct drm_device *dev, struct drm_file
> *file, @@ -2305,6 +2311,7 @@ struct drm_i915_cmd_table {
>  				 IS_BROADWELL(dev) ||
> IS_VALLEYVIEW(dev))
>  #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
>  #define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 ||
> IS_IVYBRIDGE(dev))
> +#define HAS_VED(dev)		(INTEL_INFO(dev)->is_valleyview &&
> IS_GEN7(dev))
> 
>  #define INTEL_PCH_DEVICE_ID_MASK		0xff00
>  #define INTEL_PCH_IBX_DEVICE_ID_TYPE		0x3b00
> @@ -2894,6 +2901,11 @@ void i915_restore_display_reg(struct drm_device
> *dev);  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
> 7913a72..9837a71 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1892,6 +1892,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 (iir & VLV_VED_BLOCK_INTERRUPT)
> +			vlv_ved_irq_handler(dev);
>  		/* 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 869e5ae..79d8764 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1301,6 +1301,9 @@ enum punit_power_well {  #define
> VLV_DISPLAY_BASE 0x180000  #define VLV_MIPI_BASE VLV_DISPLAY_BASE
> 
> +#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 */
> @@ -1494,6 +1497,7 @@ enum punit_power_well {
>  #define ILK_BSD_USER_INTERRUPT				(1<<5)
> 
>  #define I915_PM_INTERRUPT				(1<<31)
> +#define VLV_VED_BLOCK_INTERRUPT			(1<<23)
>  #define I915_ISP_INTERRUPT				(1<<22)
>  #define I915_LPE_PIPE_B_INTERRUPT			(1<<21)
>  #define I915_LPE_PIPE_A_INTERRUPT			(1<<20)
> diff --git a/drivers/gpu/drm/i915/i915_ved.c
> b/drivers/gpu/drm/i915/i915_ved.c new file mode 100644 index
> 0000000..f8ea7be
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_ved.c
> @@ -0,0 +1,270 @@
> +/*
> + * Copyright © 2014 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:
> + *    Yao Cheng <yao.cheng@xxxxxxxxx>
> + *
> + */
> +
> +#include "i915_drv.h"
> +
> +/**
> + * DOC: VED video core integration
> + *
> + * Motivation:
> + * Some platforms (e.g. valleyview) integrates a VED inside GPU to
> +extend the
> + * video decoding capability.
> + * The VED is driven by the standalone drm driver "ipvr" which covers
> +PowerVR
> + * VPUs. Since the PowerVR VPUs are also integrated by non-i915
> +platforms 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 install.
> + *
> + * Threats:
> + * Due to the restriction in Linux platform device model, user need
> +manually
> + * uninstall ipvr driver before uninstalling i915 module, otherwise we
> +might
> + * run into use-after-free issues after i915 removes the platform
> +device: even
> + * though ipvr 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 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[2] = { {0}, {0} };
> +	struct platform_device *platdev;
> +	u64 *dma_mask = NULL;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (dev_priv->ved.irq < 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	platdev = platform_device_alloc("ipvr-ved-vlv", -1);
> +	if (!platdev) {
> +		ret = -ENOMEM;
> +		DRM_ERROR("Failed to allocate VED 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(31);
> +	platdev->dev.dma_mask = dma_mask;
> +	platdev->dev.coherent_dma_mask = *dma_mask;
> +
> +	rsc[0].start    = rsc[0].end = dev_priv->ved.irq;
> +	rsc[0].flags    = IORESOURCE_IRQ;
> +	rsc[0].name     = "ipvr-ved-vlv-irq";
> +
> +	rsc[1].start    = pci_resource_start(dev->pdev, 0) + VLV_VED_BASE;
> +	rsc[1].end      = pci_resource_start(dev->pdev, 0) + VLV_VED_BASE +
> VLV_VED_SIZE;
> +	rsc[1].flags    = IORESOURCE_MEM;
> +	rsc[1].name     = "ipvr-ved-vlv-mmio";
> +
> +	ret = platform_device_add_resources(platdev, rsc, 2);
> +	if (ret) {
> +		DRM_ERROR("Failed to add resource for VED platform
> device: %d\n", ret);
> +		goto err_put_dev;
> +	}
> +
> +	platdev->dev.parent = dev->dev; /* for VED driver's runtime-PM */
> +	ret = platform_device_add(platdev);
> +	if (ret) {
> +		DRM_ERROR("Failed to add VED platform device: %d\n", ret);
> +		goto err_put_dev;
> +	}
> +
> +	return platdev;
> +err_put_dev:
> +	platform_device_put(platdev);
> +err:
> +	if (dma_mask)
> +		kfree(dma_mask);
> +	return ERR_PTR(ret);
> +}
> +
> +static void vlv_ved_platdev_destroy(struct drm_device *dev) {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	if (dev_priv->ved.platdev) {
> +		kfree(dev_priv->ved.platdev->dev.dma_mask);
> +		platform_device_unregister(dev_priv->ved.platdev);
> +	}
> +}
> +
> +static void vlv_ved_irq_unmask(struct irq_data *d) {
> +	struct drm_device *dev = d->chip_data;
> +	struct drm_i915_private *dev_priv = (struct drm_i915_private *)
> dev->dev_private;
> +	unsigned long irqflags;
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> +
> +	dev_priv->irq_mask &= ~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 = d->chip_data;
> +	struct drm_i915_private *dev_priv = (struct drm_i915_private *)
> dev->dev_private;
> +	unsigned long irqflags;
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> +
> +	dev_priv->irq_mask |= VLV_VED_BLOCK_INTERRUPT;
> +	I915_WRITE(VLV_IER, ~dev_priv->irq_mask);
> +	I915_WRITE(VLV_IMR, dev_priv->irq_mask);
> +	I915_WRITE(VLV_IIR, VLV_VED_BLOCK_INTERRUPT);
> +	I915_WRITE(VLV_IIR, VLV_VED_BLOCK_INTERRUPT);
> +	POSTING_READ(VLV_IIR);
> +
> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); }
> +
> +static struct irq_chip vlv_ved_irqchip = {
> +	.name = "ipvr_ved_irqchip",
> +	.irq_mask = vlv_ved_irq_mask,
> +	.irq_unmask = vlv_ved_irq_unmask,
> +};
> +
> +static int vlv_ved_irq_init(struct drm_device *dev, int irq) {
> +	struct drm_i915_private *dev_priv = (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);
> +}
> +
> +/**
> + * vlv_ved_irq_handler() - forwards the VED irq
> + * @dev: the i915 drm device
> + *
> + * the VED irq is forwarded to the irq handler registered by VED driver.
> + */
> +void vlv_ved_irq_handler(struct drm_device *dev) {
> +	int ret;
> +	struct drm_i915_private *dev_priv = 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 = 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,
> +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 vlv_setup_ved(struct drm_device *dev) {
> +	int ret;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	dev_priv->ved.irq = irq_alloc_descs(-1, 0, 1, 0);
> +	if (dev_priv->ved.irq < 0) {
> +		DRM_ERROR("Failed to allocate IRQ desc: %d\n", dev_priv-
> >ved.irq);
> +		ret = dev_priv->ved.irq;
> +		goto err;
> +	}
> +
> +	ret = vlv_ved_irq_init(dev, dev_priv->ved.irq);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize irqchip for vlv-ved: %d\n",
> ret);
> +		goto err_free_irq;
> +	}
> +
> +	dev_priv->ved.platdev = vlv_ved_platdev_create(dev);
> +	if (IS_ERR(dev_priv->ved.platdev)) {
> +		ret = PTR_ERR(dev_priv->ved.platdev);
> +		DRM_ERROR("Failed to create platform device for vlv-
> ved: %d\n", ret);
> +		goto err_free_irq;
> +	}
> +
> +	return 0;
> +err_free_irq:
> +	irq_free_desc(dev_priv->ved.irq);
> +err:
> +	dev_priv->ved.irq = -1;
> +	dev_priv->ved.platdev = NULL;
> +	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 = dev->dev_private;
> +	unsigned long irqflags;
> +
> +	/**
> +	 * mask VED irq before destroying
> +	 */
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> +	dev_priv->irq_mask |= VLV_VED_BLOCK_INTERRUPT;
> +	I915_WRITE(VLV_IER, ~dev_priv->irq_mask);
> +	I915_WRITE(VLV_IMR, dev_priv->irq_mask);
> +	I915_WRITE(VLV_IIR, VLV_VED_BLOCK_INTERRUPT);
> +	I915_WRITE(VLV_IIR, VLV_VED_BLOCK_INTERRUPT);
> +	POSTING_READ(VLV_IIR);
> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +
> +	vlv_ved_platdev_destroy(dev);
> +	if (dev_priv->ved.irq >= 0)
> +		irq_free_desc(dev_priv->ved.irq);
> +}
> --
> 2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux