[PATCH 3/7] drm/i915: add dev_priv->mm.stolen_lock

Paulo Zanoni
2015-07-01T20:16:08Z

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Which should protect dev_priv->mm.stolen usage. This will allow us to
simplify the relationship between stolen memory, FBC and struct_mutex.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++-
drivers/gpu/drm/i915/i915_gem_stolen.c | 69 +++++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_fbc.c | 29 +++++++++++---
3 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c955037..0b908b1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1245,6 +1245,10 @@ struct intel_l3_parity {
struct i915_gem_mm {
/** Memory allocator for GTT stolen memory */
struct drm_mm stolen;
+ /** Protects the usage of the GTT stolen memory allocator. This is
+ * always the inner lock when overlapping with struct_mutex. */
+ struct mutex stolen_lock;
+
/** List of all objects in gtt_space. Used to restore gtt
* mappings on resume */
struct list_head bound_list;
@@ -3112,7 +3116,8 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
struct drm_mm_node *node, u64 size,
unsigned alignment);
-void i915_gem_stolen_remove_node(struct drm_mm_node *node);
+void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
+ struct drm_mm_node *node);
int i915_gem_init_stolen(struct drm_device *dev);
void i915_gem_cleanup_stolen(struct drm_device *dev);
struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 0619786..b432085 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -46,6 +46,8 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
struct drm_mm_node *node, u64 size,
unsigned alignment)
{
+ WARN_ON(!mutex_is_locked(&dev_priv->mm.stolen_lock));
+
if (!drm_mm_initialized(&dev_priv->mm.stolen))
return -ENODEV;

@@ -53,8 +55,11 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
DRM_MM_SEARCH_DEFAULT);
}

-void i915_gem_stolen_remove_node(struct drm_mm_node *node)
+void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
+ struct drm_mm_node *node)
{
+ WARN_ON(!mutex_is_locked(&dev_priv->mm.stolen_lock));
+
drm_mm_remove_node(node);
}

@@ -171,10 +176,15 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;

+ mutex_lock(&dev_priv->mm.stolen_lock);
+
if (!drm_mm_initialized(&dev_priv->mm.stolen))
- return;
+ goto out;

drm_mm_takedown(&dev_priv->mm.stolen);
+
+out:
+ mutex_unlock(&dev_priv->mm.stolen_lock);
}

int i915_gem_init_stolen(struct drm_device *dev)
@@ -183,6 +193,8 @@ int i915_gem_init_stolen(struct drm_device *dev)
u32 tmp;
int bios_reserved = 0;

+ mutex_init(&dev_priv->mm.stolen_lock);
+
#ifdef CONFIG_INTEL_IOMMU
if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
DRM_INFO("DMAR active, disabling use of stolen memory\n");
@@ -273,8 +285,10 @@ static void i915_gem_object_put_pages_stolen(struct drm_i915_gem_object *obj)
static void
i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
{
+ struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
+
if (obj->stolen) {
- i915_gem_stolen_remove_node(obj->stolen);
+ i915_gem_stolen_remove_node(dev_priv, obj->stolen);
kfree(obj->stolen);
obj->stolen = NULL;
}
@@ -325,29 +339,36 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
struct drm_mm_node *stolen;
int ret;

+ mutex_lock(&dev_priv->mm.stolen_lock);
+
if (!drm_mm_initialized(&dev_priv->mm.stolen))
- return NULL;
+ goto out_unlock;

DRM_DEBUG_KMS("creating stolen object: size=%x\n", size);
if (size == 0)
- return NULL;
+ goto out_unlock;

stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
- return NULL;
+ goto out_unlock;

ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096);
- if (ret) {
- kfree(stolen);
- return NULL;
- }
+ if (ret)
+ goto out_free;

obj = _i915_gem_object_create_stolen(dev, stolen);
- if (obj)
- return obj;
+ if (!obj)
+ goto out_node;

- i915_gem_stolen_remove_node(stolen);
+ mutex_unlock(&dev_priv->mm.stolen_lock);
+ return obj;
+
+out_node:
+ i915_gem_stolen_remove_node(dev_priv, stolen);
+out_free:
kfree(stolen);
+out_unlock:
+ mutex_unlock(&dev_priv->mm.stolen_lock);
return NULL;
}

@@ -364,8 +385,10 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
struct i915_vma *vma;
int ret;

+ mutex_lock(&dev_priv->mm.stolen_lock);
+
if (!drm_mm_initialized(&dev_priv->mm.stolen))
- return NULL;
+ goto err_unlock;

DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n",
stolen_offset, gtt_offset, size);
@@ -373,11 +396,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
/* KISS and expect everything to be page-aligned */
if (WARN_ON(size == 0) || WARN_ON(size & 4095) ||
WARN_ON(stolen_offset & 4095))
- return NULL;
+ goto err_unlock;

stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
- return NULL;
+ goto err_unlock;

stolen->start = stolen_offset;
stolen->size = size;
@@ -385,20 +408,20 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
if (ret) {
DRM_DEBUG_KMS("failed to allocate stolen space\n");
kfree(stolen);
- return NULL;
+ goto err_unlock;
}

obj = _i915_gem_object_create_stolen(dev, stolen);
if (obj == NULL) {
DRM_DEBUG_KMS("failed to allocate stolen object\n");
- i915_gem_stolen_remove_node(stolen);
+ i915_gem_stolen_remove_node(dev_priv, stolen);
kfree(stolen);
- return NULL;
+ goto err_unlock;
}

/* Some objects just need physical mem from stolen space */
if (gtt_offset == I915_GTT_OFFSET_NONE)
- return obj;
+ goto success;

vma = i915_gem_obj_lookup_or_create_vma(obj, ggtt);
if (IS_ERR(vma)) {
@@ -427,13 +450,17 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
list_add_tail(&vma->mm_list, &ggtt->inactive_list);
i915_gem_object_pin_pages(obj);

+success:
+ mutex_unlock(&dev_priv->mm.stolen_lock);
return obj;

err_vma:
i915_gem_vma_destroy(vma);
err_out:
- i915_gem_stolen_remove_node(stolen);
+ i915_gem_stolen_remove_node(dev_priv, stolen);
kfree(stolen);
drm_gem_object_unreference(&obj->base);
+err_unlock:
+ mutex_unlock(&dev_priv->mm.stolen_lock);
return NULL;
}
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index a91bf82..dcd83ab 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -601,40 +601,57 @@ static int intel_fbc_alloc_cfb(struct drm_device *dev, int size, int fb_cpp)

err_fb:
kfree(compressed_llb);
- i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
+ i915_gem_stolen_remove_node(dev_priv, &dev_priv->fbc.compressed_fb);
err_llb:
pr_info_once("drm: not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size);
return -ENOSPC;
}

-void intel_fbc_cleanup_cfb(struct drm_device *dev)
+static void __intel_fbc_cleanup_cfb(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;

if (dev_priv->fbc.uncompressed_size == 0)
return;

- i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
+ i915_gem_stolen_remove_node(dev_priv, &dev_priv->fbc.compressed_fb);

if (dev_priv->fbc.compressed_llb) {
- i915_gem_stolen_remove_node(dev_priv->fbc.compressed_llb);
+ i915_gem_stolen_remove_node(dev_priv,
+ dev_priv->fbc.compressed_llb);
kfree(dev_priv->fbc.compressed_llb);
}

dev_priv->fbc.uncompressed_size = 0;
}

+void intel_fbc_cleanup_cfb(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ mutex_lock(&dev_priv->mm.stolen_lock);
+ __intel_fbc_cleanup_cfb(dev);
+ mutex_unlock(&dev_priv->mm.stolen_lock);
+}
+
static int intel_fbc_setup_cfb(struct drm_device *dev, int size, int fb_cpp)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;

if (size <= dev_priv->fbc.uncompressed_size)
return 0;

+ mutex_lock(&dev_priv->mm.stolen_lock);
+
/* Release any current block */
- intel_fbc_cleanup_cfb(dev);
+ __intel_fbc_cleanup_cfb(dev);
+
+ ret = intel_fbc_alloc_cfb(dev, size, fb_cpp);
+
+ mutex_unlock(&dev_priv->mm.stolen_lock);

- return intel_fbc_alloc_cfb(dev, size, fb_cpp);
+ return ret;
}

/**
--
2.1.4