Re: [Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles

2014-05-21 Thread Daniel Vetter
On Wed, May 21, 2014 at 12:42:56PM +0100, Chris Wilson wrote:
> A single object may be referenced by multiple registers fundamentally
> breaking the static allotment of ids in the current design. When the
> object is used the second time, the physical address of the first
> assignment is relinquished and a second one granted. However, the
> hardware is still reading (and possibly writing) to the old physical
> address now returned to the system. Eventually hilarity will ensue, but
> in the short term, it just means that cursors are broken when using more
> than one pipe.
> 
> v2: Fix up leak of pci handle when handling an error during attachment,
> and avoid a double kmap/kunmap. (Ville)
> Rebase against -fixes.
> 
> v3: And fix the error handling added in v2 (Ville)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
> Signed-off-by: Chris Wilson 
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> Cc: sta...@vger.kernel.org
> Reviewed-by: Ville Syrjälä 

Picked up for -fixes, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_dma.c  |   1 -
>  drivers/gpu/drm/i915/i915_drv.h  |  24 +--
>  drivers/gpu/drm/i915/i915_gem.c  | 319 
> ++-
>  drivers/gpu/drm/i915/intel_display.c |  11 +-
>  drivers/gpu/drm/i915/intel_overlay.c |  12 +-
>  5 files changed, 136 insertions(+), 231 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 96177eec0a0e..eedb023af27d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1833,7 +1833,6 @@ int i915_driver_unload(struct drm_device *dev)
>   flush_workqueue(dev_priv->wq);
>  
>   mutex_lock(&dev->struct_mutex);
> - i915_gem_free_all_phys_object(dev);
>   i915_gem_cleanup_ringbuffer(dev);
>   i915_gem_context_fini(dev);
>   WARN_ON(dev_priv->mm.aliasing_ppgtt);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 108e1ec2fa4b..ec5f6fb42ab3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -242,18 +242,6 @@ struct intel_ddi_plls {
>  #define WATCH_LISTS  0
>  #define WATCH_GTT0
>  
> -#define I915_GEM_PHYS_CURSOR_0 1
> -#define I915_GEM_PHYS_CURSOR_1 2
> -#define I915_GEM_PHYS_OVERLAY_REGS 3
> -#define I915_MAX_PHYS_OBJECT (I915_GEM_PHYS_OVERLAY_REGS)
> -
> -struct drm_i915_gem_phys_object {
> - int id;
> - struct page **page_list;
> - drm_dma_handle_t *handle;
> - struct drm_i915_gem_object *cur_obj;
> -};
> -
>  struct opregion_header;
>  struct opregion_acpi;
>  struct opregion_swsci;
> @@ -1187,9 +1175,6 @@ struct i915_gem_mm {
>   /** Bit 6 swizzling required for Y tiling */
>   uint32_t bit_6_swizzle_y;
>  
> - /* storage for physical objects */
> - struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
> -
>   /* accounting, useful for userland debugging */
>   spinlock_t object_stat_lock;
>   size_t object_memory;
> @@ -1769,7 +1754,7 @@ struct drm_i915_gem_object {
>   struct drm_file *pin_filp;
>  
>   /** for phy allocated objects */
> - struct drm_i915_gem_phys_object *phys_obj;
> + drm_dma_handle_t *phys_handle;
>  };
>  
>  #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> @@ -2334,13 +2319,8 @@ i915_gem_object_pin_to_display_plane(struct 
> drm_i915_gem_object *obj,
>u32 alignment,
>struct intel_ring_buffer *pipelined);
>  void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object 
> *obj);
> -int i915_gem_attach_phys_object(struct drm_device *dev,
> - struct drm_i915_gem_object *obj,
> - int id,
> +int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>   int align);
> -void i915_gem_detach_phys_object(struct drm_device *dev,
> -  struct drm_i915_gem_object *obj);
> -void i915_gem_free_all_phys_object(struct drm_device *dev);
>  int i915_gem_open(struct drm_device *dev, struct drm_file *file);
>  void i915_gem_release(struct drm_device *dev, struct drm_file *file);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2871ce75f438..b391f30f9985 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -43,10 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct 
> drm_i915_gem_object *o
>  static __must_check int
>  i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
>  bool readonly);
> -static int i915_gem_phys_pwrite(struct drm_device *dev,
> - struct drm_i915_gem_object *obj,
> - struct drm_i915_gem_pwrite *args,
> - struct drm_file *file);
>  
>  st

[Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles

2014-05-21 Thread Chris Wilson
A single object may be referenced by multiple registers fundamentally
breaking the static allotment of ids in the current design. When the
object is used the second time, the physical address of the first
assignment is relinquished and a second one granted. However, the
hardware is still reading (and possibly writing) to the old physical
address now returned to the system. Eventually hilarity will ensue, but
in the short term, it just means that cursors are broken when using more
than one pipe.

v2: Fix up leak of pci handle when handling an error during attachment,
and avoid a double kmap/kunmap. (Ville)
Rebase against -fixes.

v3: And fix the error handling added in v2 (Ville)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Jani Nikula 
Cc: sta...@vger.kernel.org
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_dma.c  |   1 -
 drivers/gpu/drm/i915/i915_drv.h  |  24 +--
 drivers/gpu/drm/i915/i915_gem.c  | 319 ++-
 drivers/gpu/drm/i915/intel_display.c |  11 +-
 drivers/gpu/drm/i915/intel_overlay.c |  12 +-
 5 files changed, 136 insertions(+), 231 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 96177eec0a0e..eedb023af27d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1833,7 +1833,6 @@ int i915_driver_unload(struct drm_device *dev)
flush_workqueue(dev_priv->wq);
 
mutex_lock(&dev->struct_mutex);
-   i915_gem_free_all_phys_object(dev);
i915_gem_cleanup_ringbuffer(dev);
i915_gem_context_fini(dev);
WARN_ON(dev_priv->mm.aliasing_ppgtt);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 108e1ec2fa4b..ec5f6fb42ab3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -242,18 +242,6 @@ struct intel_ddi_plls {
 #define WATCH_LISTS0
 #define WATCH_GTT  0
 
-#define I915_GEM_PHYS_CURSOR_0 1
-#define I915_GEM_PHYS_CURSOR_1 2
-#define I915_GEM_PHYS_OVERLAY_REGS 3
-#define I915_MAX_PHYS_OBJECT (I915_GEM_PHYS_OVERLAY_REGS)
-
-struct drm_i915_gem_phys_object {
-   int id;
-   struct page **page_list;
-   drm_dma_handle_t *handle;
-   struct drm_i915_gem_object *cur_obj;
-};
-
 struct opregion_header;
 struct opregion_acpi;
 struct opregion_swsci;
@@ -1187,9 +1175,6 @@ struct i915_gem_mm {
/** Bit 6 swizzling required for Y tiling */
uint32_t bit_6_swizzle_y;
 
-   /* storage for physical objects */
-   struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
-
/* accounting, useful for userland debugging */
spinlock_t object_stat_lock;
size_t object_memory;
@@ -1769,7 +1754,7 @@ struct drm_i915_gem_object {
struct drm_file *pin_filp;
 
/** for phy allocated objects */
-   struct drm_i915_gem_phys_object *phys_obj;
+   drm_dma_handle_t *phys_handle;
 };
 
 #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
@@ -2334,13 +2319,8 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
 u32 alignment,
 struct intel_ring_buffer *pipelined);
 void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj);
-int i915_gem_attach_phys_object(struct drm_device *dev,
-   struct drm_i915_gem_object *obj,
-   int id,
+int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align);
-void i915_gem_detach_phys_object(struct drm_device *dev,
-struct drm_i915_gem_object *obj);
-void i915_gem_free_all_phys_object(struct drm_device *dev);
 int i915_gem_open(struct drm_device *dev, struct drm_file *file);
 void i915_gem_release(struct drm_device *dev, struct drm_file *file);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2871ce75f438..b391f30f9985 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -43,10 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct 
drm_i915_gem_object *o
 static __must_check int
 i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
   bool readonly);
-static int i915_gem_phys_pwrite(struct drm_device *dev,
-   struct drm_i915_gem_object *obj,
-   struct drm_i915_gem_pwrite *args,
-   struct drm_file *file);
 
 static void i915_gem_write_fence(struct drm_device *dev, int reg,
 struct drm_i915_gem_object *obj);
@@ -209,6 +205,128 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void 
*data,
return 0;
 }
 
+static void i915_gem_object_detach_phys(st

Re: [Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles

2014-05-15 Thread Ville Syrjälä
On Wed, May 14, 2014 at 01:53:17PM +0100, Chris Wilson wrote:
> A single object may be referenced by multiple registers fundamentally
> breaking the static allotment of ids in the current design. When the
> object is used the second time, the physical address of the first
> assignment is relinquished and a second one granted. However, the
> hardware is still reading (and possibly writing) to the old physical
> address now returned to the system. Eventually hilarity will ensue, but
> in the short term, it just means that cursors are broken when using more
> than one pipe.
> 
> v2: Fix up leak of pci handle when handling an error during attachment,
> and avoid a double kmap/kunmap. (Ville)
> Rebase against -fixes.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
> Signed-off-by: Chris Wilson 
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/i915_dma.c  |   1 -
>  drivers/gpu/drm/i915/i915_drv.h  |  24 +--
>  drivers/gpu/drm/i915/i915_gem.c  | 319 
> ++-
>  drivers/gpu/drm/i915/intel_display.c |  11 +-
>  drivers/gpu/drm/i915/intel_overlay.c |  12 +-
>  5 files changed, 136 insertions(+), 231 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 96177eec0a0e..eedb023af27d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1833,7 +1833,6 @@ int i915_driver_unload(struct drm_device *dev)
>   flush_workqueue(dev_priv->wq);
>  
>   mutex_lock(&dev->struct_mutex);
> - i915_gem_free_all_phys_object(dev);
>   i915_gem_cleanup_ringbuffer(dev);
>   i915_gem_context_fini(dev);
>   WARN_ON(dev_priv->mm.aliasing_ppgtt);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 108e1ec2fa4b..ec5f6fb42ab3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -242,18 +242,6 @@ struct intel_ddi_plls {
>  #define WATCH_LISTS  0
>  #define WATCH_GTT0
>  
> -#define I915_GEM_PHYS_CURSOR_0 1
> -#define I915_GEM_PHYS_CURSOR_1 2
> -#define I915_GEM_PHYS_OVERLAY_REGS 3
> -#define I915_MAX_PHYS_OBJECT (I915_GEM_PHYS_OVERLAY_REGS)
> -
> -struct drm_i915_gem_phys_object {
> - int id;
> - struct page **page_list;
> - drm_dma_handle_t *handle;
> - struct drm_i915_gem_object *cur_obj;
> -};
> -
>  struct opregion_header;
>  struct opregion_acpi;
>  struct opregion_swsci;
> @@ -1187,9 +1175,6 @@ struct i915_gem_mm {
>   /** Bit 6 swizzling required for Y tiling */
>   uint32_t bit_6_swizzle_y;
>  
> - /* storage for physical objects */
> - struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
> -
>   /* accounting, useful for userland debugging */
>   spinlock_t object_stat_lock;
>   size_t object_memory;
> @@ -1769,7 +1754,7 @@ struct drm_i915_gem_object {
>   struct drm_file *pin_filp;
>  
>   /** for phy allocated objects */
> - struct drm_i915_gem_phys_object *phys_obj;
> + drm_dma_handle_t *phys_handle;
>  };
>  
>  #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> @@ -2334,13 +2319,8 @@ i915_gem_object_pin_to_display_plane(struct 
> drm_i915_gem_object *obj,
>u32 alignment,
>struct intel_ring_buffer *pipelined);
>  void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object 
> *obj);
> -int i915_gem_attach_phys_object(struct drm_device *dev,
> - struct drm_i915_gem_object *obj,
> - int id,
> +int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>   int align);
> -void i915_gem_detach_phys_object(struct drm_device *dev,
> -  struct drm_i915_gem_object *obj);
> -void i915_gem_free_all_phys_object(struct drm_device *dev);
>  int i915_gem_open(struct drm_device *dev, struct drm_file *file);
>  void i915_gem_release(struct drm_device *dev, struct drm_file *file);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2871ce75f438..0b947fd97d1f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -43,10 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct 
> drm_i915_gem_object *o
>  static __must_check int
>  i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
>  bool readonly);
> -static int i915_gem_phys_pwrite(struct drm_device *dev,
> - struct drm_i915_gem_object *obj,
> - struct drm_i915_gem_pwrite *args,
> - struct drm_file *file);
>  
>  static void i915_gem_write_fence(struct drm_device *dev, int reg,
>struct drm_i915_gem_object *obj);
> @@ -209

Re: [Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles

2014-05-15 Thread Chris Wilson
On Thu, May 15, 2014 at 12:56:48PM +0300, Jani Nikula wrote:
> On Wed, 14 May 2014, Chris Wilson  wrote:
> > A single object may be referenced by multiple registers fundamentally
> > breaking the static allotment of ids in the current design. When the
> > object is used the second time, the physical address of the first
> > assignment is relinquished and a second one granted. However, the
> > hardware is still reading (and possibly writing) to the old physical
> > address now returned to the system. Eventually hilarity will ensue, but
> > in the short term, it just means that cursors are broken when using more
> > than one pipe.
> >
> > v2: Fix up leak of pci handle when handling an error during attachment,
> > and avoid a double kmap/kunmap. (Ville)
> > Rebase against -fixes.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
> > Signed-off-by: Chris Wilson 
> > Cc: Ville Syrjälä 
> > Cc: Jani Nikula 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c  |   1 -
> >  drivers/gpu/drm/i915/i915_drv.h  |  24 +--
> >  drivers/gpu/drm/i915/i915_gem.c  | 319 
> > ++-
> >  drivers/gpu/drm/i915/intel_display.c |  11 +-
> >  drivers/gpu/drm/i915/intel_overlay.c |  12 +-
> >  5 files changed, 136 insertions(+), 231 deletions(-)
> 
> With this diffstat, this late in rc, bug referring to 855GM (is this
> required on others?!), I want r-b/t-b on this.

It affects nearly all gen2/gen3 machines. You can punt it to Daniel, but
keep cc: stable, as I want to build upon this patch to improve phys
objs, so having it heading upstream is essential.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles

2014-05-15 Thread Jani Nikula
On Wed, 14 May 2014, Chris Wilson  wrote:
> A single object may be referenced by multiple registers fundamentally
> breaking the static allotment of ids in the current design. When the
> object is used the second time, the physical address of the first
> assignment is relinquished and a second one granted. However, the
> hardware is still reading (and possibly writing) to the old physical
> address now returned to the system. Eventually hilarity will ensue, but
> in the short term, it just means that cursors are broken when using more
> than one pipe.
>
> v2: Fix up leak of pci handle when handling an error during attachment,
> and avoid a double kmap/kunmap. (Ville)
> Rebase against -fixes.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
> Signed-off-by: Chris Wilson 
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/i915_dma.c  |   1 -
>  drivers/gpu/drm/i915/i915_drv.h  |  24 +--
>  drivers/gpu/drm/i915/i915_gem.c  | 319 
> ++-
>  drivers/gpu/drm/i915/intel_display.c |  11 +-
>  drivers/gpu/drm/i915/intel_overlay.c |  12 +-
>  5 files changed, 136 insertions(+), 231 deletions(-)

With this diffstat, this late in rc, bug referring to 855GM (is this
required on others?!), I want r-b/t-b on this.

The diffstat also fails the stable rules.

BR,
Jani.


>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 96177eec0a0e..eedb023af27d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1833,7 +1833,6 @@ int i915_driver_unload(struct drm_device *dev)
>   flush_workqueue(dev_priv->wq);
>  
>   mutex_lock(&dev->struct_mutex);
> - i915_gem_free_all_phys_object(dev);
>   i915_gem_cleanup_ringbuffer(dev);
>   i915_gem_context_fini(dev);
>   WARN_ON(dev_priv->mm.aliasing_ppgtt);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 108e1ec2fa4b..ec5f6fb42ab3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -242,18 +242,6 @@ struct intel_ddi_plls {
>  #define WATCH_LISTS  0
>  #define WATCH_GTT0
>  
> -#define I915_GEM_PHYS_CURSOR_0 1
> -#define I915_GEM_PHYS_CURSOR_1 2
> -#define I915_GEM_PHYS_OVERLAY_REGS 3
> -#define I915_MAX_PHYS_OBJECT (I915_GEM_PHYS_OVERLAY_REGS)
> -
> -struct drm_i915_gem_phys_object {
> - int id;
> - struct page **page_list;
> - drm_dma_handle_t *handle;
> - struct drm_i915_gem_object *cur_obj;
> -};
> -
>  struct opregion_header;
>  struct opregion_acpi;
>  struct opregion_swsci;
> @@ -1187,9 +1175,6 @@ struct i915_gem_mm {
>   /** Bit 6 swizzling required for Y tiling */
>   uint32_t bit_6_swizzle_y;
>  
> - /* storage for physical objects */
> - struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
> -
>   /* accounting, useful for userland debugging */
>   spinlock_t object_stat_lock;
>   size_t object_memory;
> @@ -1769,7 +1754,7 @@ struct drm_i915_gem_object {
>   struct drm_file *pin_filp;
>  
>   /** for phy allocated objects */
> - struct drm_i915_gem_phys_object *phys_obj;
> + drm_dma_handle_t *phys_handle;
>  };
>  
>  #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> @@ -2334,13 +2319,8 @@ i915_gem_object_pin_to_display_plane(struct 
> drm_i915_gem_object *obj,
>u32 alignment,
>struct intel_ring_buffer *pipelined);
>  void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object 
> *obj);
> -int i915_gem_attach_phys_object(struct drm_device *dev,
> - struct drm_i915_gem_object *obj,
> - int id,
> +int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>   int align);
> -void i915_gem_detach_phys_object(struct drm_device *dev,
> -  struct drm_i915_gem_object *obj);
> -void i915_gem_free_all_phys_object(struct drm_device *dev);
>  int i915_gem_open(struct drm_device *dev, struct drm_file *file);
>  void i915_gem_release(struct drm_device *dev, struct drm_file *file);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2871ce75f438..0b947fd97d1f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -43,10 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct 
> drm_i915_gem_object *o
>  static __must_check int
>  i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
>  bool readonly);
> -static int i915_gem_phys_pwrite(struct drm_device *dev,
> - struct drm_i915_gem_object *obj,
> - struct drm_i915_gem_pwrite *args,
> - struct drm_file *file);

[Intel-gfx] [PATCH] drm/i915: Fix dynamic allocation of physical handles

2014-05-14 Thread Chris Wilson
A single object may be referenced by multiple registers fundamentally
breaking the static allotment of ids in the current design. When the
object is used the second time, the physical address of the first
assignment is relinquished and a second one granted. However, the
hardware is still reading (and possibly writing) to the old physical
address now returned to the system. Eventually hilarity will ensue, but
in the short term, it just means that cursors are broken when using more
than one pipe.

v2: Fix up leak of pci handle when handling an error during attachment,
and avoid a double kmap/kunmap. (Ville)
Rebase against -fixes.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77351
Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Jani Nikula 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_dma.c  |   1 -
 drivers/gpu/drm/i915/i915_drv.h  |  24 +--
 drivers/gpu/drm/i915/i915_gem.c  | 319 ++-
 drivers/gpu/drm/i915/intel_display.c |  11 +-
 drivers/gpu/drm/i915/intel_overlay.c |  12 +-
 5 files changed, 136 insertions(+), 231 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 96177eec0a0e..eedb023af27d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1833,7 +1833,6 @@ int i915_driver_unload(struct drm_device *dev)
flush_workqueue(dev_priv->wq);
 
mutex_lock(&dev->struct_mutex);
-   i915_gem_free_all_phys_object(dev);
i915_gem_cleanup_ringbuffer(dev);
i915_gem_context_fini(dev);
WARN_ON(dev_priv->mm.aliasing_ppgtt);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 108e1ec2fa4b..ec5f6fb42ab3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -242,18 +242,6 @@ struct intel_ddi_plls {
 #define WATCH_LISTS0
 #define WATCH_GTT  0
 
-#define I915_GEM_PHYS_CURSOR_0 1
-#define I915_GEM_PHYS_CURSOR_1 2
-#define I915_GEM_PHYS_OVERLAY_REGS 3
-#define I915_MAX_PHYS_OBJECT (I915_GEM_PHYS_OVERLAY_REGS)
-
-struct drm_i915_gem_phys_object {
-   int id;
-   struct page **page_list;
-   drm_dma_handle_t *handle;
-   struct drm_i915_gem_object *cur_obj;
-};
-
 struct opregion_header;
 struct opregion_acpi;
 struct opregion_swsci;
@@ -1187,9 +1175,6 @@ struct i915_gem_mm {
/** Bit 6 swizzling required for Y tiling */
uint32_t bit_6_swizzle_y;
 
-   /* storage for physical objects */
-   struct drm_i915_gem_phys_object *phys_objs[I915_MAX_PHYS_OBJECT];
-
/* accounting, useful for userland debugging */
spinlock_t object_stat_lock;
size_t object_memory;
@@ -1769,7 +1754,7 @@ struct drm_i915_gem_object {
struct drm_file *pin_filp;
 
/** for phy allocated objects */
-   struct drm_i915_gem_phys_object *phys_obj;
+   drm_dma_handle_t *phys_handle;
 };
 
 #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
@@ -2334,13 +2319,8 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
 u32 alignment,
 struct intel_ring_buffer *pipelined);
 void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj);
-int i915_gem_attach_phys_object(struct drm_device *dev,
-   struct drm_i915_gem_object *obj,
-   int id,
+int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align);
-void i915_gem_detach_phys_object(struct drm_device *dev,
-struct drm_i915_gem_object *obj);
-void i915_gem_free_all_phys_object(struct drm_device *dev);
 int i915_gem_open(struct drm_device *dev, struct drm_file *file);
 void i915_gem_release(struct drm_device *dev, struct drm_file *file);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2871ce75f438..0b947fd97d1f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -43,10 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct 
drm_i915_gem_object *o
 static __must_check int
 i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
   bool readonly);
-static int i915_gem_phys_pwrite(struct drm_device *dev,
-   struct drm_i915_gem_object *obj,
-   struct drm_i915_gem_pwrite *args,
-   struct drm_file *file);
 
 static void i915_gem_write_fence(struct drm_device *dev, int reg,
 struct drm_i915_gem_object *obj);
@@ -209,6 +205,128 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void 
*data,
return 0;
 }
 
+static void i915_gem_object_detach_phys(struct drm_i915_gem_object *obj)
+{
+   drm_dma_handle_t *phys = obj->phys_han