Skip to contentSkip to conversation

drm driver for VED in Intel GPU

RFC PATCH In review Yao Cheng 3 participants 10 messages Nov 21, 2014 – Dec 1, 2014

Subsystems (4)
lore
Share & tools
lore

Yao Cheng

author
#0 lore raw

v3 cover drm driver for VED in Intel GPU

drm/ipvr is a new GEM driver for VED (PowerVR's VPU integrated in Intel GPU), which extends video capability. A new Kconfig added for building ipvr driver:

CONFIG_DRM_IPVR: Build option for ipvr module

The driver name "ipvr" means the PowerVR's core wrapped by Intel. The PowerVR VPUs are also integrated by non-i915 platforms such as GMA500, so we keep ipvr driver and i915 driver separated and independent to each other. To achieve this we do the minimum change in i915: i915_ved.c added for setting up the bridge between VED and i915, kerneldoc also updated.

User mode drm helper "libdrm_ipvr.so" and simple ioctl/execute test is included.

one test script "drv_module_reload" in i-g-t also updated to support ipvr loading and unloading. Due to restriction in Linux platform device model, we have to manually unload ipvr before unloading i915.

Yao Cheng (4):
[1/4] drm/i915: add i915_ved.c to setup bridge for VED [2/4] drm/ipvr: drm driver for VED
[3/4] libdrm/ipvr: user mode helper for ipvr drm driver [4/4] i-g-t: tests/drv_module_reload: add ipvr support

Signature · 6 lines
2.1.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#1 lore raw

RFC PATCH v3 1/4 drm/i915: add i915_ved.c to setup bridge for VED

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.

---
 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 | 255 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 292 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_ved.c
Documentation/DocBook/drm.tmpl +5 -0
index 9d99772..b2d109f 100644
@@ -3839,6 +3839,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>
drivers/gpu/drm/i915/Makefile +3 -0
index 891e584..e2f2776 100644
@@ -83,6 +83,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)
drivers/gpu/drm/i915/i915_dma.c +11 -0
index 9a73533..fbf2144 100644
@@ -1791,6 +1791,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;
@@ -1833,6 +1840,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);
drivers/gpu/drm/i915/i915_drv.h +12 -0
index f830596..6cfabac 100644
@@ -1762,6 +1762,12 @@ struct drm_i915_private {
 
 	uint32_t bios_vgacntr;
 
+	/* necessary resource sharing with ved driver. */
+	struct {
+		struct platform_device *platdev;
+		int	irq;
+	} ved;
+
 	/* Old dri1 support infrastructure, beware the dragons ya fools entering
 	 * here! */
 	struct i915_dri1_state dri1;
@@ -2257,6 +2263,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
@@ -2851,6 +2858,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);
drivers/gpu/drm/i915/i915_irq.c +2 -0
index 5fff287..a3394de 100644
@@ -1865,6 +1865,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);
drivers/gpu/drm/i915/i915_reg.h +4 -0
index d43fa0e..1a46fe5 100644
@@ -1284,6 +1284,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 */
@@ -1477,6 +1480,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)
drivers/gpu/drm/i915/i915_ved.c new file +255 -0

255 lines not on this page · raw message

Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#2 lore raw

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

Quoted 218 lines · Yao Cheng

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.

---
  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 | 255 ++++++++++++++++++++++++++++++++++++++++
  7 files changed, 292 insertions(+)

create mode 100644 drivers/gpu/drm/i915/i915_ved.c

Documentation/DocBook/drm.tmpl +5 -0
index 9d99772..b2d109f 100644
@@ -3839,6 +3839,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>
drivers/gpu/drm/i915/Makefile +3 -0
index 891e584..e2f2776 100644
@@ -83,6 +83,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)
drivers/gpu/drm/i915/i915_dma.c +11 -0
index 9a73533..fbf2144 100644
@@ -1791,6 +1791,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;
@@ -1833,6 +1840,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);
drivers/gpu/drm/i915/i915_drv.h +12 -0
index f830596..6cfabac 100644
@@ -1762,6 +1762,12 @@ struct drm_i915_private {
 
  	uint32_t bios_vgacntr;
 
+	/* necessary resource sharing with ved driver. */
+	struct {
+		struct platform_device *platdev;
+		int	irq;
+	} ved;
+
  	/* Old dri1 support infrastructure, beware the dragons ya fools entering
  	 * here! */
  	struct i915_dri1_state dri1;
@@ -2257,6 +2263,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
@@ -2851,6 +2858,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);
drivers/gpu/drm/i915/i915_irq.c +2 -0
index 5fff287..a3394de 100644
@@ -1865,6 +1865,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);
drivers/gpu/drm/i915/i915_reg.h +4 -0
index d43fa0e..1a46fe5 100644
@@ -1284,6 +1284,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 */
@@ -1477,6 +1480,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)
drivers/gpu/drm/i915/i915_ved.c new file +39 -0
new file mode 100644
index 0000000..982e69e
@@ -0,0 +1,255 @@
+/*
+ * 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 <[email protected]>
+ *
+ */
+
+#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

drivers/gpu/drm/i915/i915_ved.c line 49

Quoted 10 lines
drivers/gpu/drm/i915/i915_ved.c new file +10 -0
+ * 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 he might
+ * run into use-after-free issues after i915 removes the platform device.

Can you go in to more detail on what you consider to be the restriction in the platform device model?

When removing the device via platform_device_unregister, it will call the remove callback of any drivers handling this device (via the bus remove function). It is then up to the driver to ensure no further usage of the device from which it is being removed. This usually involves removing all user input vectors, disabling interrupts involved and flushing/canceling any delayed work. This should prevent any further use of the device by the driver.

The driver's remove function is called in a direct call chain from platform_device_unregister, so by the time it returns, there should be no further chance of accesses.

Quoted 192 lines
drivers/gpu/drm/i915/i915_ved.c new file +192 -0
+ *
+ * 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;
+}

drivers/gpu/drm/i915/i915_ved.c line 251

Quoted 10 lines
drivers/gpu/drm/i915/i915_ved.c new file +10 -0
+
+/**
+ * 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;

you may want to mask the i915 interrupt for the VED block before removing the device.

drivers/gpu/drm/i915/i915_ved.c line 255

Quoted 5 lines
drivers/gpu/drm/i915/i915_ved.c new file +4 -0
+	vlv_ved_platdev_destroy(dev);
+	if (dev_priv->ved.irq >= 0)
+		irq_free_desc(dev_priv->ved.irq);
+}

Generally it is a nicely interfaced child device.

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#3 lore raw

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

Quoted 27 lines

-----Original Message-----
From: Beckett, Robert
Sent: Saturday, November 29, 2014 0:59
To: Cheng, Yao; [email protected]; dri- [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: [email protected]; Jiang, Fei; [email protected]; [email protected]
Subject: Re: [Intel-gfx] [RFC PATCH v3 1/4] drm/i915: add i915_ved.c to setup bridge for VED

Review

Quoted 6 lines

+ * Threats:
+ * Due to the restriction in Linux platform device model, user need

(quoted diff) +4 -0
+manually
+ * uninstall ipvr driver before uninstalling i915 module, otherwise
+he might
+ * run into use-after-free issues after i915 removes the platform device.

Can you go in to more detail on what you consider to be the restriction in the platform device model?

When removing the device via platform_device_unregister, it will call the remove callback of any drivers handling this device (via the bus remove function). It is then up to the driver to ensure no further usage of the device from which it is being removed. This usually involves removing all user input vectors, disabling interrupts involved and flushing/canceling any delayed work. This should prevent any further use of the device by the driver.

Quoted 4 lines

The driver's remove function is called in a direct call chain from platform_device_unregister, so by the time it returns, there should be no further chance of accesses.

Bob, thx for your review comments.
The symptom is, after "rmmod i915", though drm_drv_release() is also called on the child device "ipvr", I still see the module exist in the system (check it by "lsmod"). This causes issue when I modprobe i915 and ipvr again later. Actually I don't understand why this restriction exists, but accroding to Daniel, grabbing a module refcount for the platform device doesn't work (it would pin the module forever).

Quoted 6 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+void vlv_teardown_ved(struct drm_device *dev) {
+	struct drm_i915_private *dev_priv = dev->dev_private;

you may want to mask the i915 interrupt for the VED block before removing the device.

Thx, will add it.

Quoted 9 lines

drivers/gpu/drm/i915/i915_ved.c line 255

Quoted 5 lines
drivers/gpu/drm/i915/i915_ved.c new file +4 -0
+	vlv_ved_platdev_destroy(dev);
+	if (dev_priv->ved.irq >= 0)
+		irq_free_desc(dev_priv->ved.irq);
+}

Generally it is a nicely interfaced child device.

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

#4 lore raw

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

Quoted 32 lines · Yao
Quoted 27 lines

-----Original Message-----
From: Beckett, Robert
Sent: Saturday, November 29, 2014 0:59
To: Cheng, Yao; [email protected]; dri- [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: [email protected]; Jiang, Fei; [email protected]; [email protected]
Subject: Re: [Intel-gfx] [RFC PATCH v3 1/4] drm/i915: add i915_ved.c to setup bridge for VED

Review

Quoted 6 lines

+ * Threats:
+ * Due to the restriction in Linux platform device model, user need

(quoted diff) +4 -0
+manually
+ * uninstall ipvr driver before uninstalling i915 module, otherwise
+he might
+ * run into use-after-free issues after i915 removes the platform device.

Can you go in to more detail on what you consider to be the restriction in the platform device model?

When removing the device via platform_device_unregister, it will call the remove callback of any drivers handling this device (via the bus remove function). It is then up to the driver to ensure no further usage of the device from which it is being removed. This usually involves removing all user input vectors, disabling interrupts involved and flushing/canceling any delayed work. This should prevent any further use of the device by the driver.

Quoted 4 lines

The driver's remove function is called in a direct call chain from platform_device_unregister, so by the time it returns, there should be no further chance of accesses.

Quoted 7 lines

Bob, thx for your review comments.
The symptom is, after "rmmod i915", though drm_drv_release() is also called on the child device "ipvr", I still see the module exist in the system (check it by "lsmod"). This causes issue when I modprobe i915 and ipvr again later. Actually I don't understand why this restriction exists, but accroding to Daniel, grabbing a module refcount for the platform device doesn't work (it would pin the module forever).

Btw this entire story with platform drivers make unload a mess was just a best guess from my side why module unloading doesn't work well or could blow up. I've thought platform drivers registered from modules are inheritely racy, but I'll be very happy to learn that this is a misunderstanding on my side ;-)

Wrt the testcase I'm fairly pragmatic though: Whatever makes it work reliably is good enough for me, even if it's a gross hack (we already need to manually kick fbcon anyway).
Cheers, Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#5 lore raw

RFC PATCH v3 2/4 drm/ipvr: drm driver for VED

In reply to Yao Cheng

Probes VED and creates a new drm device for hardware accelerated video decoding.
Currently support VP8 decoding on valleyview.

v2:
take David's comments
- add mmap support and remove mmap_ioctl
- remove postclose since it's deprecated
- NULL set_busid

v3:
take David, Daniel and Jesse's comments and massive refine
- use drm_dev_alloc+drm_dev_register to replace drm_platform_init
- same as above in exit side
- remove fd based explit fence
- refine ipvr_drm.h, use __u32 series and refine paddings
- add doc to describe ipvr/ved terminology
- ioctl refine: remove unused code
- use uintptr_t series for address/number conversion
- runtime pm refine: guarantee get/put paring
- add PRIME feature and remove USERPTR ioctl
- implement relocation fixup in EXEC ioctl
- call drm_gem_get_pages to replace my own implementation
- code cleanup: remove unused code

---
 Documentation/DocBook/drm.tmpl    |   39 ++
 drivers/gpu/drm/Kconfig           |    2 +
 drivers/gpu/drm/Makefile          |    1 +
 drivers/gpu/drm/ipvr/Kconfig      |    9 +
 drivers/gpu/drm/ipvr/Makefile     |   18 +
 drivers/gpu/drm/ipvr/ipvr_bo.c    |  546 +++++++++++++++++
 drivers/gpu/drm/ipvr/ipvr_bo.h    |   80 +++
 drivers/gpu/drm/ipvr/ipvr_debug.c |  347 +++++++++++
 drivers/gpu/drm/ipvr/ipvr_debug.h |   76 +++
 drivers/gpu/drm/ipvr/ipvr_drm.h   |  278 +++++++++
 drivers/gpu/drm/ipvr/ipvr_drv.c   |  614 +++++++++++++++++++
 drivers/gpu/drm/ipvr/ipvr_drv.h   |  294 +++++++++
 drivers/gpu/drm/ipvr/ipvr_exec.c  |  623 +++++++++++++++++++
 drivers/gpu/drm/ipvr/ipvr_exec.h  |   57 ++
 drivers/gpu/drm/ipvr/ipvr_fence.c |  490 +++++++++++++++
 drivers/gpu/drm/ipvr/ipvr_fence.h |   74 +++
 drivers/gpu/drm/ipvr/ipvr_gem.c   |  341 +++++++++++
 drivers/gpu/drm/ipvr/ipvr_gem.h   |   48 ++
 drivers/gpu/drm/ipvr/ipvr_mmu.c   |  752 +++++++++++++++++++++++
 drivers/gpu/drm/ipvr/ipvr_mmu.h   |  111 ++++
 drivers/gpu/drm/ipvr/ipvr_trace.c |   11 +
 drivers/gpu/drm/ipvr/ipvr_trace.h |  326 ++++++++++
 drivers/gpu/drm/ipvr/ved_cmd.c    |  882 +++++++++++++++++++++++++++
 drivers/gpu/drm/ipvr/ved_cmd.h    |   71 +++
 drivers/gpu/drm/ipvr/ved_fw.c     | 1199 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/ipvr/ved_fw.h     |   81 +++
 drivers/gpu/drm/ipvr/ved_msg.h    |  256 ++++++++
 drivers/gpu/drm/ipvr/ved_pm.c     |  332 ++++++++++
 drivers/gpu/drm/ipvr/ved_pm.h     |   36 ++
 drivers/gpu/drm/ipvr/ved_reg.h    |  561 +++++++++++++++++
 30 files changed, 8555 insertions(+)
 create mode 100644 drivers/gpu/drm/ipvr/Kconfig
 create mode 100644 drivers/gpu/drm/ipvr/Makefile
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_bo.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_bo.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_debug.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_debug.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_drm.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_drv.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_drv.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_exec.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_exec.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_fence.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_fence.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_gem.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_gem.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_mmu.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_mmu.h
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_trace.c
 create mode 100644 drivers/gpu/drm/ipvr/ipvr_trace.h
 create mode 100644 drivers/gpu/drm/ipvr/ved_cmd.c
 create mode 100644 drivers/gpu/drm/ipvr/ved_cmd.h
 create mode 100644 drivers/gpu/drm/ipvr/ved_fw.c
 create mode 100644 drivers/gpu/drm/ipvr/ved_fw.h
 create mode 100644 drivers/gpu/drm/ipvr/ved_msg.h
 create mode 100644 drivers/gpu/drm/ipvr/ved_pm.c
 create mode 100644 drivers/gpu/drm/ipvr/ved_pm.h
 create mode 100644 drivers/gpu/drm/ipvr/ved_reg.h
Documentation/DocBook/drm.tmpl +39 -0
index b2d109f..07b5b19 100644
@@ -4008,5 +4008,44 @@ int num_ioctls;</synopsis>
     </sect1>
   </chapter>
 !Cdrivers/gpu/drm/i915/i915_irq.c
+  <chapter id="drmIpvr">
+    <title>drm/ipvr Intel's driver for PowerVR video core</title>
+    <para>
+      The drm/ipvr driver intends to support the PowerVR video cores
+	  integrated on Intel's platforms. The video cores can be categorized
+	  into 3 types:
+        <variablelist>
+          <varlistentry>
+            <term>VED</term>
+            <listitem>
+              <para>
+                multi-format video decoder, various video codecs are supported,
+				e.g. H.264, VP8, etc..
+              </para>
+            </listitem>
+          </varlistentry>
+          <varlistentry>
+            <term>VEC</term>
+            <listitem>
+              <para>
+                multi-format video encoder, various video codecs support like
+				VED.
+              </para>
+            </listitem>
+          </varlistentry>
+          <varlistentry>
+            <term>VSP</term>
+            <listitem>
+              <para>
+		        multi-function video processing. supports various features such
+				as color-space-conversion, sharpening, deblocking, etc..
+              </para>
+            </listitem>
+          </varlistentry>
+        </variablelist>
+	  Now ipvr only supports VED on Valleyview platform. The names "VEC"
+	  and "VSP" are reserved for possible future support.
+    </para>
+  </chapter>
 </part>
 </book>
drivers/gpu/drm/Kconfig +2 -0
index e3b4b0f..ad7585d 100644
@@ -165,6 +165,8 @@ config DRM_SAVAGE
 	  Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister
 	  chipset. If M is selected the module will be called savage.
 
+source "drivers/gpu/drm/ipvr/Kconfig"
+
 source "drivers/gpu/drm/exynos/Kconfig"
 
 source "drivers/gpu/drm/vmwgfx/Kconfig"
drivers/gpu/drm/Makefile +1 -0
index c3cf64c..f877208 100644
@@ -41,6 +41,7 @@ obj-$(CONFIG_DRM_RADEON)+= radeon/
 obj-$(CONFIG_DRM_MGA)	+= mga/
 obj-$(CONFIG_DRM_I810)	+= i810/
 obj-$(CONFIG_DRM_I915)  += i915/
+obj-$(CONFIG_DRM_IPVR)  += ipvr/
 obj-$(CONFIG_DRM_MGAG200) += mgag200/
 obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
 obj-$(CONFIG_DRM_SIS)   += sis/
drivers/gpu/drm/ipvr/Kconfig new file +9 -0
new file mode 100644
index 0000000..869bad4
@@ -0,0 +1,9 @@
+config DRM_IPVR
+       tristate "IPVR video decode driver"
+       depends on DRM
+       select SHMEM
+       select TMPFS
+       default m
+       help
+         Choose this option if you want to enable accelerated video decode with VED hardware.
+		 Currently support VP8 decoding on valleyview.
drivers/gpu/drm/ipvr/Makefile new file +18 -0
new file mode 100644
index 0000000..280647e
@@ -0,0 +1,18 @@
+ccflags-y := -Iinclude/drm
+
+ipvr-y := \
+        ipvr_drv.o \
+        ipvr_bo.o \
+        ipvr_exec.o \
+        ipvr_fence.o \
+        ipvr_gem.o \
+        ipvr_mmu.o \
+        ipvr_debug.o \
+        ipvr_trace.o \
+        ved_pm.o \
+        ved_cmd.o \
+        ved_fw.o
+
+obj-$(CONFIG_DRM_IPVR) += ipvr.o
+
+CFLAGS_ipvr_trace.o := -I$(src)
drivers/gpu/drm/ipvr/ipvr_bo.c new file +546 -0

546 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_bo.h new file +80 -0

80 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_debug.c new file +347 -0

347 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_debug.h new file +76 -0

76 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_drm.h new file +278 -0

278 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_drv.c new file +614 -0

614 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_drv.h new file +294 -0

294 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_exec.c new file +623 -0

623 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_exec.h new file +57 -0

56 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_fence.c new file +490 -0

490 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_fence.h new file +74 -0

74 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_gem.c new file +341 -0

341 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_gem.h new file +48 -0

48 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_mmu.c new file +752 -0

752 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_mmu.h new file +111 -0

111 lines not on this page · raw message

drivers/gpu/drm/ipvr/ipvr_trace.c new file +11 -0
new file mode 100644
index 0000000..91c0bda

Lines of this file are not shown. View the raw message.

drivers/gpu/drm/ipvr/ipvr_trace.h new file +326 -0

326 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_cmd.c new file +882 -0

882 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_cmd.h new file +71 -0

71 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_fw.c new file +1199 -0

1,199 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_fw.h new file +81 -0

81 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_msg.h new file +256 -0

256 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_pm.c new file +332 -0

332 lines not on this page · raw message

drivers/gpu/drm/ipvr/ved_pm.h new file +36 -0
new file mode 100644
index 0000000..4ed1485

Lines of this file are not shown. View the raw message.

drivers/gpu/drm/ipvr/ved_reg.h new file +561 -0

561 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Part of the diff is not shown here. View the raw message.

author
#6 lore raw

RE: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

Quoted 22 lines

-----Original Message-----
From: Cheng, Yao
Sent: Saturday, November 22, 2014 3:07
To: [email protected]; [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: Jiang, Fei; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Cheng, Yao
Subject: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

drivers/gpu/drm/ipvr/ipvr_gem.c new file +9 -0
+
+int ipvr_gem_mmap_offset_ioctl(struct drm_device *dev,
+				void *data, struct drm_file *file_priv)
+{
+	int ret;
+	struct drm_ipvr_gem_mmap_offset *args = data;
+	struct drm_ipvr_gem_object *obj;
+
+	IPVR_DEBUG_ENTRY("getting mmap offset for BO %u.\n", args-
Quoted 1 line

handle);

(quoted diff) +1 -0
+	obj = to_ipvr_bo(drm_gem_object_lookup(dev, file_priv, args-
Quoted 1 line

handle));

drivers/gpu/drm/ipvr/ipvr_gem.c line 341

Quoted 10 lines
drivers/gpu/drm/ipvr/ipvr_gem.c new file +10 -0
+
+	/* create map offset */
+	ret = drm_gem_create_mmap_offset(&obj->base);
+	if (ret) {
+		IPVR_ERROR("could not allocate mmap offset: %d\n", ret);
+		return ret;
+	}
+	args->offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+	return 0;
+}

I missed an unreferenced here.
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

author
#7 lore raw

RE: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

In reply to Yao Cheng
Quoted 19 lines

-----Original Message-----
From: Cheng, Yao
Sent: Saturday, November 22, 2014 3:07
To: [email protected]; [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: Jiang, Fei; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Cheng, Yao
Subject: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

drivers/gpu/drm/ipvr/ipvr_drv.c new file +9 -0
+static void
+ipvr_drm_preclose(struct drm_device *dev, struct drm_file *file_priv)
+{
+	/* if user didn't destory ctx explicitly, remove ctx here */
+	struct drm_ipvr_private *dev_priv;
+	struct drm_ipvr_file_private *ipvr_fpriv;
+	struct ved_private *ved_priv;
+	struct ipvr_context *ipvr_ctx  = NULL;
+	unsigned long irq_flags;

drivers/gpu/drm/ipvr/ipvr_drv.c line 390

Quoted 10 lines
drivers/gpu/drm/ipvr/ipvr_drv.c new file +10 -0
+
+	IPVR_DEBUG_ENTRY("enter\n");
+	dev_priv = dev->dev_private;
+	ipvr_fpriv = file_priv->driver_priv;
+	ved_priv = dev_priv->ved_private;
+
+	if (ipvr_fpriv->ctx_id == IPVR_CONTEXT_INVALID_ID)
+		return;
+	ipvr_ctx = (struct ipvr_context *)
+			idr_find(&dev_priv->ipvr_ctx_idr, ipvr_fpriv->ctx_id);

Need protection on ipvr_ctx_idr. Same to the other idr related code.

Quoted 22 lines
(quoted diff) +2 -0
+	if (!ipvr_ctx  || (ipvr_ctx->ipvr_fpriv != ipvr_fpriv)) {
+		IPVR_DEBUG_GENERAL("ctx for id %d has already

destroyed\n",

drivers/gpu/drm/ipvr/ipvr_drv.c new file +13 -0
+				ipvr_fpriv->ctx_id);
+		return;
+	}
+
+	/**
+	 * fixme: remove this work-around (WA the issue that calling
+	 * close() with queued cmd might cause state machine issue).
+	 * we should wait for only the cmds sent from contexts in this file
+	 * instead of all cmds
+	 */
+	ipvr_fence_wait_empty_locked(dev_priv);
+
+	IPVR_DEBUG_PM("Video:remove context type 0x%x\n", ipvr_ctx-
Quoted 1 line

ctx_type);

drivers/gpu/drm/ipvr/ipvr_drv.c new file +5 -0
+	mutex_lock(&ved_priv->ved_mutex);
+	if (ved_priv->ipvr_ctx == ipvr_ctx )
+		ved_priv->ipvr_ctx = NULL;
+	mutex_unlock(&ved_priv->ved_mutex);
+

drivers/gpu/drm/ipvr/ipvr_drv.c line 420

Quoted 10 lines
drivers/gpu/drm/ipvr/ipvr_drv.c new file +10 -0
+	spin_lock_irqsave(&dev_priv->ipvr_ctx_lock, irq_flags);
+	list_del(&ipvr_ctx->head);
+	ipvr_fpriv->ctx_id = IPVR_CONTEXT_INVALID_ID;
+	spin_unlock_irqrestore(&dev_priv->ipvr_ctx_lock, irq_flags);
+
+	idr_remove(&dev_priv->ipvr_ctx_idr, ipvr_ctx->ctx_id);
+
+	kfree(ipvr_ctx );
+	kfree(ipvr_fpriv);
+}

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

author
#8 lore raw

Re: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

In reply to Yao Cheng
Quoted 9 lines

-----Original Message-----
From: Cheng, Yao
Sent: Saturday, November 22, 2014 3:07
To: [email protected]; [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: Jiang, Fei; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Cheng, Yao
Subject: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

drivers/gpu/drm/ipvr/ipvr_drv.h line 200

Quoted 6 lines
drivers/gpu/drm/ipvr/ipvr_drv.h new file +6 -0
+typedef struct drm_ipvr_private {
+	struct drm_device *dev;
+	struct pci_dev *pci_root;
+
+	/* IMG video context */
+	struct list_head ipvr_ctx_list;

The current design leads to ctx leak. There's no ctx_list for each file struct, so each create_context_ioctl causes one ctx leak. Need to move the ctx_list from dev_private to file_private.

Quoted 23 lines
drivers/gpu/drm/ipvr/ipvr_drv.h new file +23 -0
+	spinlock_t ipvr_ctx_lock;
+	struct idr ipvr_ctx_idr;
+	struct ipvr_context default_ctx;
+
+	/* PM related */
+	atomic_t pending_events;
+
+	/* exec related */
+	struct ipvr_validate_context validate_ctx;
+
+	/* IMG MMU specific */
+	struct ipvr_mmu_driver *mmu;
+	/*struct ipvr_mmu_pd *pf_pd;*/
+	atomic_t ipvr_mmu_invaldc;
+
+	/* GEM mm related */
+	struct ipvr_gem_stat ipvr_stat;
+	struct kmem_cache *ipvr_bo_slab;
+	struct ipvr_address_space addr_space;
+
+	/* fence related */
+	u32 last_seq;
+	wait_queue_head_t fence_queue;

drivers/gpu/drm/ipvr/ipvr_drv.h line 233

Quoted 10 lines
drivers/gpu/drm/ipvr/ipvr_drv.h new file +10 -0
+	struct ipvr_fence_driver fence_drv;
+
+	/* MMIO window shared from parent device */
+	u8 __iomem* reg_base;
+
+	/*
+	 * VED specific
+	 */
+	struct ved_private *ved_private;
+}drm_ipvr_private_t;

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#9 lore raw

Re: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

Add Jon/Bob/Raf for detail review.

Quoted 58 lines

-----Original Message-----
From: dri-devel [mailto:[email protected]] On Behalf Of Cheng, Yao
Sent: Thursday, November 27, 2014 19:49
To: [email protected]; [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: [email protected]; Jiang, Fei
Subject: RE: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

Quoted 9 lines

-----Original Message-----
From: Cheng, Yao
Sent: Saturday, November 22, 2014 3:07
To: [email protected]; [email protected]; [email protected]; Kelley, Sean V; Chehab, John
Cc: Jiang, Fei; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Cheng, Yao
Subject: [RFC PATCH v3 2/4] drm/ipvr: drm driver for VED

drivers/gpu/drm/ipvr/ipvr_drv.h line 200

Quoted 6 lines
drivers/gpu/drm/ipvr/ipvr_drv.h new file +6 -0
+typedef struct drm_ipvr_private {
+	struct drm_device *dev;
+	struct pci_dev *pci_root;
+
+	/* IMG video context */
+	struct list_head ipvr_ctx_list;

The current design leads to ctx leak. There's no ctx_list for each file struct, so each create_context_ioctl causes one ctx leak.
Need to move the ctx_list from dev_private to file_private.

Quoted 29 lines
drivers/gpu/drm/ipvr/ipvr_drv.h new file +29 -0
+	spinlock_t ipvr_ctx_lock;
+	struct idr ipvr_ctx_idr;
+	struct ipvr_context default_ctx;
+
+	/* PM related */
+	atomic_t pending_events;
+
+	/* exec related */
+	struct ipvr_validate_context validate_ctx;
+
+	/* IMG MMU specific */
+	struct ipvr_mmu_driver *mmu;
+	/*struct ipvr_mmu_pd *pf_pd;*/
+	atomic_t ipvr_mmu_invaldc;
+
+	/* GEM mm related */
+	struct ipvr_gem_stat ipvr_stat;
+	struct kmem_cache *ipvr_bo_slab;
+	struct ipvr_address_space addr_space;
+
+	/* fence related */
+	u32 last_seq;
+	wait_queue_head_t fence_queue;
+	struct ipvr_fence_driver fence_drv;
+
+	/* MMIO window shared from parent device */
+	u8 __iomem* reg_base;
+
+	/*
Quoted 8 lines

drivers/gpu/drm/ipvr/ipvr_drv.h line 233

Quoted 4 lines
drivers/gpu/drm/ipvr/ipvr_drv.h new file +4 -0
+	 * VED specific
+	 */
+	struct ved_private *ved_private;
+}drm_ipvr_private_t;

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx