All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
@ 2019-09-15 18:45 Chris Wilson
  2019-09-15 18:45 ` [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Chris Wilson @ 2019-09-15 18:45 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.

See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/dma-buf/dma-fence.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..587727089134 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -285,19 +285,18 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
 {
 	unsigned long flags;
 
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return;
+
+	spin_lock_irqsave(fence->lock, flags);
 	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
 			      &fence->flags) &&
-	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
 	    fence->ops->enable_signaling) {
 		trace_dma_fence_enable_signal(fence);
-
-		spin_lock_irqsave(fence->lock, flags);
-
 		if (!fence->ops->enable_signaling(fence))
 			dma_fence_signal_locked(fence);
-
-		spin_unlock_irqrestore(fence->lock, flags);
 	}
+	spin_unlock_irqrestore(fence->lock, flags);
 }
 EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
 
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield
  2019-09-15 18:45 [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
@ 2019-09-15 18:45 ` Chris Wilson
  2019-09-16 19:45   ` Ruhl, Michael J
  2019-10-03 13:42   ` Tvrtko Ursulin
  2019-09-15 19:12 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2019-09-15 18:45 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

The ulterior motive to switching the booleans over to bitops is to
allow use of the allocated flag as a bitlock.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_mm.c                      | 36 +++++++++++--------
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++--
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  2 +-
 drivers/gpu/drm/i915/i915_gem.c               | 16 ++++-----
 drivers/gpu/drm/i915/i915_gem_evict.c         |  2 +-
 drivers/gpu/drm/i915/i915_vma.c               |  2 +-
 drivers/gpu/drm/i915/i915_vma.h               |  4 +--
 drivers/gpu/drm/selftests/test-drm_mm.c       | 14 ++++----
 drivers/gpu/drm/vc4/vc4_crtc.c                |  2 +-
 drivers/gpu/drm/vc4/vc4_hvs.c                 |  2 +-
 drivers/gpu/drm/vc4/vc4_plane.c               |  4 +--
 include/drm/drm_mm.h                          |  7 ++--
 12 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 4581c5387372..211967006cec 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -174,7 +174,7 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
 
 	node->__subtree_last = LAST(node);
 
-	if (hole_node->allocated) {
+	if (drm_mm_node_allocated(hole_node)) {
 		rb = &hole_node->rb;
 		while (rb) {
 			parent = rb_entry(rb, struct drm_mm_node, rb);
@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
 
 	node->mm = mm;
 
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 	list_add(&node->node_list, &hole->node_list);
 	drm_mm_interval_tree_add_node(hole, node);
-	node->allocated = true;
 	node->hole_size = 0;
 
 	rm_hole(hole);
@@ -543,9 +543,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
 		node->color = color;
 		node->hole_size = 0;
 
+		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 		list_add(&node->node_list, &hole->node_list);
 		drm_mm_interval_tree_add_node(hole, node);
-		node->allocated = true;
 
 		rm_hole(hole);
 		if (adj_start > hole_start)
@@ -561,6 +561,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
 }
 EXPORT_SYMBOL(drm_mm_insert_node_in_range);
 
+static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)
+{
+	return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
+}
+
 /**
  * drm_mm_remove_node - Remove a memory node from the allocator.
  * @node: drm_mm_node to remove
@@ -574,8 +579,8 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 	struct drm_mm *mm = node->mm;
 	struct drm_mm_node *prev_node;
 
-	DRM_MM_BUG_ON(!node->allocated);
-	DRM_MM_BUG_ON(node->scanned_block);
+	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
+	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
 
 	prev_node = list_prev_entry(node, node_list);
 
@@ -584,11 +589,12 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 
 	drm_mm_interval_tree_remove(node, &mm->interval_tree);
 	list_del(&node->node_list);
-	node->allocated = false;
 
 	if (drm_mm_hole_follows(prev_node))
 		rm_hole(prev_node);
 	add_hole(prev_node);
+
+	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 }
 EXPORT_SYMBOL(drm_mm_remove_node);
 
@@ -605,7 +611,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
 {
 	struct drm_mm *mm = old->mm;
 
-	DRM_MM_BUG_ON(!old->allocated);
+	DRM_MM_BUG_ON(!drm_mm_node_allocated(old));
 
 	*new = *old;
 
@@ -622,8 +628,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
 				&mm->holes_addr);
 	}
 
-	old->allocated = false;
-	new->allocated = true;
+	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
 }
 EXPORT_SYMBOL(drm_mm_replace_node);
 
@@ -731,9 +736,9 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
 	u64 adj_start, adj_end;
 
 	DRM_MM_BUG_ON(node->mm != mm);
-	DRM_MM_BUG_ON(!node->allocated);
-	DRM_MM_BUG_ON(node->scanned_block);
-	node->scanned_block = true;
+	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
+	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
+	__set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
 	mm->scan_active++;
 
 	/* Remove this block from the node_list so that we enlarge the hole
@@ -818,8 +823,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
 	struct drm_mm_node *prev_node;
 
 	DRM_MM_BUG_ON(node->mm != scan->mm);
-	DRM_MM_BUG_ON(!node->scanned_block);
-	node->scanned_block = false;
+	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
 
 	DRM_MM_BUG_ON(!node->mm->scan_active);
 	node->mm->scan_active--;
@@ -837,6 +841,8 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
 		      list_next_entry(node, node_list));
 	list_add(&node->node_list, &prev_node->node_list);
 
+	__clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
+
 	return (node->start + node->size > scan->hit_start &&
 		node->start < scan->hit_end);
 }
@@ -917,7 +923,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
 
 	/* Clever trick to avoid a special case in the free hole tracking. */
 	INIT_LIST_HEAD(&mm->head_node.node_list);
-	mm->head_node.allocated = false;
+	mm->head_node.flags = 0;
 	mm->head_node.mm = mm;
 	mm->head_node.start = start + size;
 	mm->head_node.size = -size;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 27dbcb508055..c049199a1df5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache,
 	cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
 	cache->has_fence = cache->gen < 4;
 	cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
-	cache->node.allocated = false;
+	cache->node.flags = 0;
 	cache->ce = NULL;
 	cache->rq = NULL;
 	cache->rq_size = 0;
@@ -968,7 +968,7 @@ static void reloc_cache_reset(struct reloc_cache *cache)
 		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 		io_mapping_unmap_atomic((void __iomem *)vaddr);
 
-		if (cache->node.allocated) {
+		if (drm_mm_node_allocated(&cache->node)) {
 			ggtt->vm.clear_range(&ggtt->vm,
 					     cache->node.start,
 					     cache->node.size);
@@ -1061,7 +1061,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 	}
 
 	offset = cache->node.start;
-	if (cache->node.allocated) {
+	if (drm_mm_node_allocated(&cache->node)) {
 		ggtt->vm.insert_page(&ggtt->vm,
 				     i915_gem_object_get_dma_address(obj, page),
 				     offset, I915_CACHE_NONE, 0);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 296a82603be0..07fc6f28abcd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -401,7 +401,7 @@ static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
 {
 	struct drm_mm_node *node = &ggtt->uc_fw;
 
-	GEM_BUG_ON(!node->allocated);
+	GEM_BUG_ON(!drm_mm_node_allocated(node));
 	GEM_BUG_ON(upper_32_bits(node->start));
 	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2da9544fa9a4..b8f7fe311b0a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -351,12 +351,12 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 					       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
-		node.allocated = false;
+		node.flags = 0;
 	} else {
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
 			goto out_unlock;
-		GEM_BUG_ON(!node.allocated);
+		GEM_BUG_ON(!drm_mm_node_allocated(&node));
 	}
 
 	mutex_unlock(&i915->drm.struct_mutex);
@@ -393,7 +393,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 		unsigned page_offset = offset_in_page(offset);
 		unsigned page_length = PAGE_SIZE - page_offset;
 		page_length = remain < page_length ? remain : page_length;
-		if (node.allocated) {
+		if (drm_mm_node_allocated(&node)) {
 			ggtt->vm.insert_page(&ggtt->vm,
 					     i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
 					     node.start, I915_CACHE_NONE, 0);
@@ -415,7 +415,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 	i915_gem_object_unlock_fence(obj, fence);
 out_unpin:
 	mutex_lock(&i915->drm.struct_mutex);
-	if (node.allocated) {
+	if (drm_mm_node_allocated(&node)) {
 		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
 		remove_mappable_node(&node);
 	} else {
@@ -561,12 +561,12 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 					       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
-		node.allocated = false;
+		node.flags = 0;
 	} else {
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
 			goto out_rpm;
-		GEM_BUG_ON(!node.allocated);
+		GEM_BUG_ON(!drm_mm_node_allocated(&node));
 	}
 
 	mutex_unlock(&i915->drm.struct_mutex);
@@ -604,7 +604,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 		unsigned int page_offset = offset_in_page(offset);
 		unsigned int page_length = PAGE_SIZE - page_offset;
 		page_length = remain < page_length ? remain : page_length;
-		if (node.allocated) {
+		if (drm_mm_node_allocated(&node)) {
 			/* flush the write before we modify the GGTT */
 			intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 			ggtt->vm.insert_page(&ggtt->vm,
@@ -636,7 +636,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 out_unpin:
 	mutex_lock(&i915->drm.struct_mutex);
 	intel_gt_flush_ggtt_writes(ggtt->vm.gt);
-	if (node.allocated) {
+	if (drm_mm_node_allocated(&node)) {
 		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
 		remove_mappable_node(&node);
 	} else {
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index e76c9da9992d..8c1e04f402bc 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -299,7 +299,7 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 			break;
 		}
 
-		GEM_BUG_ON(!node->allocated);
+		GEM_BUG_ON(!drm_mm_node_allocated(node));
 		vma = container_of(node, typeof(*vma), node);
 
 		/* If we are using coloring to insert guard pages between
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 411047d6a909..d10614091bb9 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -795,7 +795,7 @@ void i915_vma_reopen(struct i915_vma *vma)
 
 static void __i915_vma_destroy(struct i915_vma *vma)
 {
-	GEM_BUG_ON(vma->node.allocated);
+	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(vma->fence);
 
 	mutex_lock(&vma->vm->mutex);
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 8bcb5812c446..e49b199f7de7 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -228,7 +228,7 @@ static inline bool i915_vma_is_closed(const struct i915_vma *vma)
 static inline u32 i915_ggtt_offset(const struct i915_vma *vma)
 {
 	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
-	GEM_BUG_ON(!vma->node.allocated);
+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(upper_32_bits(vma->node.start));
 	GEM_BUG_ON(upper_32_bits(vma->node.start + vma->node.size - 1));
 	return lower_32_bits(vma->node.start);
@@ -390,7 +390,7 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
 static inline bool i915_node_color_differs(const struct drm_mm_node *node,
 					   unsigned long color)
 {
-	return node->allocated && node->color != color;
+	return drm_mm_node_allocated(node) && node->color != color;
 }
 
 /**
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c
index 388f9844f4ba..9aabe82dcd3a 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -854,7 +854,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
 
 	if (start > 0) {
 		node = __drm_mm_interval_first(mm, 0, start - 1);
-		if (node->allocated) {
+		if (drm_mm_node_allocated(node)) {
 			pr_err("node before start: node=%llx+%llu, start=%llx\n",
 			       node->start, node->size, start);
 			return false;
@@ -863,7 +863,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
 
 	if (end < U64_MAX) {
 		node = __drm_mm_interval_first(mm, end, U64_MAX);
-		if (node->allocated) {
+		if (drm_mm_node_allocated(node)) {
 			pr_err("node after end: node=%llx+%llu, end=%llx\n",
 			       node->start, node->size, end);
 			return false;
@@ -1156,12 +1156,12 @@ static void show_holes(const struct drm_mm *mm, int count)
 		struct drm_mm_node *next = list_next_entry(hole, node_list);
 		const char *node1 = NULL, *node2 = NULL;
 
-		if (hole->allocated)
+		if (drm_mm_node_allocated(hole))
 			node1 = kasprintf(GFP_KERNEL,
 					  "[%llx + %lld, color=%ld], ",
 					  hole->start, hole->size, hole->color);
 
-		if (next->allocated)
+		if (drm_mm_node_allocated(next))
 			node2 = kasprintf(GFP_KERNEL,
 					  ", [%llx + %lld, color=%ld]",
 					  next->start, next->size, next->color);
@@ -1900,18 +1900,18 @@ static void separate_adjacent_colors(const struct drm_mm_node *node,
 				     u64 *start,
 				     u64 *end)
 {
-	if (node->allocated && node->color != color)
+	if (drm_mm_node_allocated(node) && node->color != color)
 		++*start;
 
 	node = list_next_entry(node, node_list);
-	if (node->allocated && node->color != color)
+	if (drm_mm_node_allocated(node) && node->color != color)
 		--*end;
 }
 
 static bool colors_abutt(const struct drm_mm_node *node)
 {
 	if (!drm_mm_hole_follows(node) &&
-	    list_next_entry(node, node_list)->allocated) {
+	    drm_mm_node_allocated(list_next_entry(node, node_list))) {
 		pr_err("colors abutt; %ld [%llx + %llx] is next to %ld [%llx + %llx]!\n",
 		       node->color, node->start, node->size,
 		       list_next_entry(node, node_list)->color,
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f1f0a7c87771..b00e20f5ce05 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
 	struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
 	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
 
-	if (vc4_state->mm.allocated) {
+	if (drm_mm_node_allocated(&vc4_state->mm)) {
 		unsigned long flags;
 
 		spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 9936b15d0bf1..5a43659da319 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -315,7 +315,7 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master,
 	struct drm_device *drm = dev_get_drvdata(master);
 	struct vc4_dev *vc4 = drm->dev_private;
 
-	if (vc4->hvs->mitchell_netravali_filter.allocated)
+	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
 		drm_mm_remove_node(&vc4->hvs->mitchell_netravali_filter);
 
 	drm_mm_takedown(&vc4->hvs->dlist_mm);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 5e5f90810aca..4934127f0d76 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -178,7 +178,7 @@ static void vc4_plane_destroy_state(struct drm_plane *plane,
 	struct vc4_dev *vc4 = to_vc4_dev(plane->dev);
 	struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
 
-	if (vc4_state->lbm.allocated) {
+	if (drm_mm_node_allocated(&vc4_state->lbm)) {
 		unsigned long irqflags;
 
 		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
@@ -557,7 +557,7 @@ static int vc4_plane_allocate_lbm(struct drm_plane_state *state)
 	/* Allocate the LBM memory that the HVS will use for temporary
 	 * storage due to our scaling/format conversion.
 	 */
-	if (!vc4_state->lbm.allocated) {
+	if (!drm_mm_node_allocated(&vc4_state->lbm)) {
 		int ret;
 
 		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 2c3bbb43c7d1..d7939c054259 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -168,8 +168,9 @@ struct drm_mm_node {
 	struct rb_node rb_hole_addr;
 	u64 __subtree_last;
 	u64 hole_size;
-	bool allocated : 1;
-	bool scanned_block : 1;
+	unsigned long flags;
+#define DRM_MM_NODE_ALLOCATED_BIT	0
+#define DRM_MM_NODE_SCANNED_BIT		1
 #ifdef CONFIG_DRM_DEBUG_MM
 	depot_stack_handle_t stack;
 #endif
@@ -253,7 +254,7 @@ struct drm_mm_scan {
  */
 static inline bool drm_mm_node_allocated(const struct drm_mm_node *node)
 {
-	return node->allocated;
+	return test_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 }
 
 /**
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-09-15 18:45 [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
  2019-09-15 18:45 ` [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
@ 2019-09-15 19:12 ` Patchwork
  2019-09-15 19:34 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-09-15 19:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
URL   : https://patchwork.freedesktop.org/series/66726/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f54627385b19 dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
-:14: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal")'
#14: 
See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked

total: 1 errors, 0 warnings, 0 checks, 24 lines checked
2e450b7867f5 drm/mm: Pack allocated/scanned boolean into a bitfield

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-09-15 18:45 [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
  2019-09-15 18:45 ` [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
  2019-09-15 19:12 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Patchwork
@ 2019-09-15 19:34 ` Patchwork
  2019-09-16  8:51 ` ✓ Fi.CI.IGT: " Patchwork
  2019-10-03 13:16 ` [PATCH 1/2] " Tvrtko Ursulin
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-09-15 19:34 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
URL   : https://patchwork.freedesktop.org/series/66726/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6896 -> Patchwork_14415
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/

Known issues
------------

  Here are the changes found in Patchwork_14415 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@read_all_entries:
    - fi-ilk-650:         [PASS][1] -> [DMESG-WARN][2] ([fdo#106387])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-ilk-650/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-ilk-650/igt@debugfs_test@read_all_entries.html

  * igt@gem_mmap_gtt@basic-write-no-prefault:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-icl-u3/igt@gem_mmap_gtt@basic-write-no-prefault.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-icl-u3/igt@gem_mmap_gtt@basic-write-no-prefault.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-skl-6700k2:      [PASS][5] -> [FAIL][6] ([fdo#103375]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-icl-u2:          [PASS][7] -> [FAIL][8] ([fdo#109483] / [fdo#109635 ])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-kbl-7500u:       [PASS][9] -> [FAIL][10] ([fdo#109483])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][11] -> [FAIL][12] ([fdo#111407])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_exec_fence@nb-await-default:
    - {fi-tgl-u2}:        [FAIL][13] ([fdo#111562] / [fdo#111597]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-tgl-u2/igt@gem_exec_fence@nb-await-default.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-tgl-u2/igt@gem_exec_fence@nb-await-default.html

  * igt@kms_chamelium@hdmi-edid-read:
    - {fi-icl-u4}:        [FAIL][15] ([fdo#111045]) -> [PASS][16] +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-u3:          [DMESG-WARN][17] ([fdo#107724]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/fi-icl-u3/igt@prime_vgem@basic-fence-flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/fi-icl-u3/igt@prime_vgem@basic-fence-flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111562]: https://bugs.freedesktop.org/show_bug.cgi?id=111562
  [fdo#111597]: https://bugs.freedesktop.org/show_bug.cgi?id=111597


Participating hosts (52 -> 43)
------------------------------

  Missing    (9): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6896 -> Patchwork_14415

  CI-20190529: 20190529
  CI_DRM_6896: 666925d1b342cca839902dd418eea383e69f373e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5183: 6ddc1a143495baa68dbc909f2a8819ec03c31c8e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14415: 2e450b7867f545f949504678dfa49abf844c2512 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2e450b7867f5 drm/mm: Pack allocated/scanned boolean into a bitfield
f54627385b19 dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-09-15 18:45 [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
                   ` (2 preceding siblings ...)
  2019-09-15 19:34 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-16  8:51 ` Patchwork
  2019-10-03 13:16 ` [PATCH 1/2] " Tvrtko Ursulin
  4 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-09-16  8:51 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
URL   : https://patchwork.freedesktop.org/series/66726/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6896_full -> Patchwork_14415_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_14415_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#111325]) +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb3/igt@gem_exec_schedule@in-order-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@preempt-other-bsd1:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +15 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb1/igt@gem_exec_schedule@preempt-other-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb8/igt@gem_exec_schedule@preempt-other-bsd1.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-a-ctm-green-to-red:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([fdo#107201])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl10/igt@kms_color@pipe-a-ctm-green-to-red.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl9/igt@kms_color@pipe-a-ctm-green-to-red.html

  * igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#103232])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl10/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html

  * igt@kms_cursor_legacy@pipe-a-forked-bo:
    - shard-apl:          [PASS][11] -> [INCOMPLETE][12] ([fdo#103927]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-apl2/igt@kms_cursor_legacy@pipe-a-forked-bo.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-apl1/igt@kms_cursor_legacy@pipe-a-forked-bo.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#105363])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167]) +4 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103166])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109642] / [fdo#111068])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb5/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#109441]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb1/igt@kms_psr@psr2_cursor_plane_onoff.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][27] ([fdo#110841]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb7/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_big@single:
    - shard-apl:          [INCOMPLETE][29] ([fdo#103927]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-apl5/igt@gem_exec_big@single.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-apl3/igt@gem_exec_big@single.html

  * igt@gem_exec_schedule@deep-bsd:
    - shard-iclb:         [SKIP][31] ([fdo#111325]) -> [PASS][32] +7 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb2/igt@gem_exec_schedule@deep-bsd.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb5/igt@gem_exec_schedule@deep-bsd.html

  * igt@gem_exec_schedule@independent-bsd1:
    - shard-iclb:         [SKIP][33] ([fdo#109276]) -> [PASS][34] +21 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb3/igt@gem_exec_schedule@independent-bsd1.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb2/igt@gem_exec_schedule@independent-bsd1.html

  * igt@i915_selftest@live_gem_contexts:
    - shard-hsw:          [DMESG-FAIL][35] -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-hsw6/igt@i915_selftest@live_gem_contexts.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-hsw5/igt@i915_selftest@live_gem_contexts.html

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-apl6/igt@i915_suspend@debugfs-reader.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-apl2/igt@i915_suspend@debugfs-reader.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][39] ([fdo#105363]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         [FAIL][41] ([fdo#103167]) -> [PASS][42] +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-iclb:         [INCOMPLETE][43] ([fdo#106978] / [fdo#107713]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][45] ([fdo#108145]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_mmap_cpu:
    - shard-iclb:         [SKIP][47] ([fdo#109441]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_cpu.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_cpu.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][49] ([fdo#110728]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-skl9/igt@perf@blocking.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-skl4/igt@perf@blocking.html

  * igt@perf_pmu@busy-no-semaphores-bcs0:
    - shard-hsw:          [DMESG-WARN][51] -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-hsw6/igt@perf_pmu@busy-no-semaphores-bcs0.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-hsw8/igt@perf_pmu@busy-no-semaphores-bcs0.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-isolation-bsd2:
    - shard-iclb:         [FAIL][53] ([fdo#111330]) -> [SKIP][54] ([fdo#109276])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb2/igt@gem_mocs_settings@mocs-isolation-bsd2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb5/igt@gem_mocs_settings@mocs-isolation-bsd2.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [SKIP][55] ([fdo#109276]) -> [FAIL][56] ([fdo#111330]) +2 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6896/shard-iclb3/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330


Participating hosts (9 -> 9)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6896 -> Patchwork_14415

  CI-20190529: 20190529
  CI_DRM_6896: 666925d1b342cca839902dd418eea383e69f373e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5183: 6ddc1a143495baa68dbc909f2a8819ec03c31c8e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14415: 2e450b7867f545f949504678dfa49abf844c2512 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14415/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield
  2019-09-15 18:45 ` [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
@ 2019-09-16 19:45   ` Ruhl, Michael J
  2019-10-03  7:07     ` Chris Wilson
  2019-10-03 13:42   ` Tvrtko Ursulin
  1 sibling, 1 reply; 10+ messages in thread
From: Ruhl, Michael J @ 2019-09-16 19:45 UTC (permalink / raw)
  To: Chris Wilson, dri-devel; +Cc: intel-gfx

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf
>Of Chris Wilson
>Sent: Sunday, September 15, 2019 2:46 PM
>To: dri-devel@lists.freedesktop.org
>Cc: intel-gfx@lists.freedesktop.org
>Subject: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield
>
>The ulterior motive to switching the booleans over to bitops is to
>allow use of the allocated flag as a bitlock.
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> drivers/gpu/drm/drm_mm.c                      | 36 +++++++++++--------
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++--
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  2 +-
> drivers/gpu/drm/i915/i915_gem.c               | 16 ++++-----
> drivers/gpu/drm/i915/i915_gem_evict.c         |  2 +-
> drivers/gpu/drm/i915/i915_vma.c               |  2 +-
> drivers/gpu/drm/i915/i915_vma.h               |  4 +--
> drivers/gpu/drm/selftests/test-drm_mm.c       | 14 ++++----
> drivers/gpu/drm/vc4/vc4_crtc.c                |  2 +-
> drivers/gpu/drm/vc4/vc4_hvs.c                 |  2 +-
> drivers/gpu/drm/vc4/vc4_plane.c               |  4 +--
> include/drm/drm_mm.h                          |  7 ++--
> 12 files changed, 52 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
>index 4581c5387372..211967006cec 100644
>--- a/drivers/gpu/drm/drm_mm.c
>+++ b/drivers/gpu/drm/drm_mm.c
>@@ -174,7 +174,7 @@ static void drm_mm_interval_tree_add_node(struct
>drm_mm_node *hole_node,
>
> 	node->__subtree_last = LAST(node);
>
>-	if (hole_node->allocated) {
>+	if (drm_mm_node_allocated(hole_node)) {
> 		rb = &hole_node->rb;
> 		while (rb) {
> 			parent = rb_entry(rb, struct drm_mm_node, rb);
>@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm,
>struct drm_mm_node *node)
>
> 	node->mm = mm;
>
>+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);

Maybe a silly question(s), but you appear to be mixing atomic bit ops
(clear_ and test_) with non-atomic bit ops __xxx_bit().

Should that be a concern?   Should that be mention in the commit?

Mike


> 	list_add(&node->node_list, &hole->node_list);
> 	drm_mm_interval_tree_add_node(hole, node);
>-	node->allocated = true;
> 	node->hole_size = 0;
>
> 	rm_hole(hole);
>@@ -543,9 +543,9 @@ int drm_mm_insert_node_in_range(struct drm_mm *
>const mm,
> 		node->color = color;
> 		node->hole_size = 0;
>
>+		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
> 		list_add(&node->node_list, &hole->node_list);
> 		drm_mm_interval_tree_add_node(hole, node);
>-		node->allocated = true;
>
> 		rm_hole(hole);
> 		if (adj_start > hole_start)
>@@ -561,6 +561,11 @@ int drm_mm_insert_node_in_range(struct drm_mm
>* const mm,
> }
> EXPORT_SYMBOL(drm_mm_insert_node_in_range);
>
>+static inline bool drm_mm_node_scanned_block(const struct
>drm_mm_node *node)
>+{
>+	return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
>+}
>+
> /**
>  * drm_mm_remove_node - Remove a memory node from the allocator.
>  * @node: drm_mm_node to remove
>@@ -574,8 +579,8 @@ void drm_mm_remove_node(struct drm_mm_node
>*node)
> 	struct drm_mm *mm = node->mm;
> 	struct drm_mm_node *prev_node;
>
>-	DRM_MM_BUG_ON(!node->allocated);
>-	DRM_MM_BUG_ON(node->scanned_block);
>+	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
>+	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
>
> 	prev_node = list_prev_entry(node, node_list);
>
>@@ -584,11 +589,12 @@ void drm_mm_remove_node(struct
>drm_mm_node *node)
>
> 	drm_mm_interval_tree_remove(node, &mm->interval_tree);
> 	list_del(&node->node_list);
>-	node->allocated = false;
>
> 	if (drm_mm_hole_follows(prev_node))
> 		rm_hole(prev_node);
> 	add_hole(prev_node);
>+
>+	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
> }
> EXPORT_SYMBOL(drm_mm_remove_node);
>
>@@ -605,7 +611,7 @@ void drm_mm_replace_node(struct drm_mm_node
>*old, struct drm_mm_node *new)
> {
> 	struct drm_mm *mm = old->mm;
>
>-	DRM_MM_BUG_ON(!old->allocated);
>+	DRM_MM_BUG_ON(!drm_mm_node_allocated(old));
>
> 	*new = *old;
>
>@@ -622,8 +628,7 @@ void drm_mm_replace_node(struct drm_mm_node
>*old, struct drm_mm_node *new)
> 				&mm->holes_addr);
> 	}
>
>-	old->allocated = false;
>-	new->allocated = true;
>+	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
> }
> EXPORT_SYMBOL(drm_mm_replace_node);
>
>@@ -731,9 +736,9 @@ bool drm_mm_scan_add_block(struct drm_mm_scan
>*scan,
> 	u64 adj_start, adj_end;
>
> 	DRM_MM_BUG_ON(node->mm != mm);
>-	DRM_MM_BUG_ON(!node->allocated);
>-	DRM_MM_BUG_ON(node->scanned_block);
>-	node->scanned_block = true;
>+	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
>+	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
>+	__set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
> 	mm->scan_active++;
>
> 	/* Remove this block from the node_list so that we enlarge the hole
>@@ -818,8 +823,7 @@ bool drm_mm_scan_remove_block(struct
>drm_mm_scan *scan,
> 	struct drm_mm_node *prev_node;
>
> 	DRM_MM_BUG_ON(node->mm != scan->mm);
>-	DRM_MM_BUG_ON(!node->scanned_block);
>-	node->scanned_block = false;
>+	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
>
> 	DRM_MM_BUG_ON(!node->mm->scan_active);
> 	node->mm->scan_active--;
>@@ -837,6 +841,8 @@ bool drm_mm_scan_remove_block(struct
>drm_mm_scan *scan,
> 		      list_next_entry(node, node_list));
> 	list_add(&node->node_list, &prev_node->node_list);
>
>+	__clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
>+
> 	return (node->start + node->size > scan->hit_start &&
> 		node->start < scan->hit_end);
> }
>@@ -917,7 +923,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start,
>u64 size)
>
> 	/* Clever trick to avoid a special case in the free hole tracking. */
> 	INIT_LIST_HEAD(&mm->head_node.node_list);
>-	mm->head_node.allocated = false;
>+	mm->head_node.flags = 0;
> 	mm->head_node.mm = mm;
> 	mm->head_node.start = start + size;
> 	mm->head_node.size = -size;
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>index 27dbcb508055..c049199a1df5 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>@@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache,
> 	cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
> 	cache->has_fence = cache->gen < 4;
> 	cache->needs_unfenced = INTEL_INFO(i915)-
>>unfenced_needs_alignment;
>-	cache->node.allocated = false;
>+	cache->node.flags = 0;
> 	cache->ce = NULL;
> 	cache->rq = NULL;
> 	cache->rq_size = 0;
>@@ -968,7 +968,7 @@ static void reloc_cache_reset(struct reloc_cache
>*cache)
> 		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
> 		io_mapping_unmap_atomic((void __iomem *)vaddr);
>
>-		if (cache->node.allocated) {
>+		if (drm_mm_node_allocated(&cache->node)) {
> 			ggtt->vm.clear_range(&ggtt->vm,
> 					     cache->node.start,
> 					     cache->node.size);
>@@ -1061,7 +1061,7 @@ static void *reloc_iomap(struct
>drm_i915_gem_object *obj,
> 	}
>
> 	offset = cache->node.start;
>-	if (cache->node.allocated) {
>+	if (drm_mm_node_allocated(&cache->node)) {
> 		ggtt->vm.insert_page(&ggtt->vm,
> 				     i915_gem_object_get_dma_address(obj,
>page),
> 				     offset, I915_CACHE_NONE, 0);
>diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>index 296a82603be0..07fc6f28abcd 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>@@ -401,7 +401,7 @@ static u32 uc_fw_ggtt_offset(struct intel_uc_fw
>*uc_fw, struct i915_ggtt *ggtt)
> {
> 	struct drm_mm_node *node = &ggtt->uc_fw;
>
>-	GEM_BUG_ON(!node->allocated);
>+	GEM_BUG_ON(!drm_mm_node_allocated(node));
> 	GEM_BUG_ON(upper_32_bits(node->start));
> 	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
>
>diff --git a/drivers/gpu/drm/i915/i915_gem.c
>b/drivers/gpu/drm/i915/i915_gem.c
>index 2da9544fa9a4..b8f7fe311b0a 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -351,12 +351,12 @@ i915_gem_gtt_pread(struct drm_i915_gem_object
>*obj,
> 					       PIN_NOEVICT);
> 	if (!IS_ERR(vma)) {
> 		node.start = i915_ggtt_offset(vma);
>-		node.allocated = false;
>+		node.flags = 0;
> 	} else {
> 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
> 		if (ret)
> 			goto out_unlock;
>-		GEM_BUG_ON(!node.allocated);
>+		GEM_BUG_ON(!drm_mm_node_allocated(&node));
> 	}
>
> 	mutex_unlock(&i915->drm.struct_mutex);
>@@ -393,7 +393,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object
>*obj,
> 		unsigned page_offset = offset_in_page(offset);
> 		unsigned page_length = PAGE_SIZE - page_offset;
> 		page_length = remain < page_length ? remain : page_length;
>-		if (node.allocated) {
>+		if (drm_mm_node_allocated(&node)) {
> 			ggtt->vm.insert_page(&ggtt->vm,
>
>i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
> 					     node.start, I915_CACHE_NONE, 0);
>@@ -415,7 +415,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object
>*obj,
> 	i915_gem_object_unlock_fence(obj, fence);
> out_unpin:
> 	mutex_lock(&i915->drm.struct_mutex);
>-	if (node.allocated) {
>+	if (drm_mm_node_allocated(&node)) {
> 		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
> 		remove_mappable_node(&node);
> 	} else {
>@@ -561,12 +561,12 @@ i915_gem_gtt_pwrite_fast(struct
>drm_i915_gem_object *obj,
> 					       PIN_NOEVICT);
> 	if (!IS_ERR(vma)) {
> 		node.start = i915_ggtt_offset(vma);
>-		node.allocated = false;
>+		node.flags = 0;
> 	} else {
> 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
> 		if (ret)
> 			goto out_rpm;
>-		GEM_BUG_ON(!node.allocated);
>+		GEM_BUG_ON(!drm_mm_node_allocated(&node));
> 	}
>
> 	mutex_unlock(&i915->drm.struct_mutex);
>@@ -604,7 +604,7 @@ i915_gem_gtt_pwrite_fast(struct
>drm_i915_gem_object *obj,
> 		unsigned int page_offset = offset_in_page(offset);
> 		unsigned int page_length = PAGE_SIZE - page_offset;
> 		page_length = remain < page_length ? remain : page_length;
>-		if (node.allocated) {
>+		if (drm_mm_node_allocated(&node)) {
> 			/* flush the write before we modify the GGTT */
> 			intel_gt_flush_ggtt_writes(ggtt->vm.gt);
> 			ggtt->vm.insert_page(&ggtt->vm,
>@@ -636,7 +636,7 @@ i915_gem_gtt_pwrite_fast(struct
>drm_i915_gem_object *obj,
> out_unpin:
> 	mutex_lock(&i915->drm.struct_mutex);
> 	intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>-	if (node.allocated) {
>+	if (drm_mm_node_allocated(&node)) {
> 		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
> 		remove_mappable_node(&node);
> 	} else {
>diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c
>b/drivers/gpu/drm/i915/i915_gem_evict.c
>index e76c9da9992d..8c1e04f402bc 100644
>--- a/drivers/gpu/drm/i915/i915_gem_evict.c
>+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>@@ -299,7 +299,7 @@ int i915_gem_evict_for_node(struct
>i915_address_space *vm,
> 			break;
> 		}
>
>-		GEM_BUG_ON(!node->allocated);
>+		GEM_BUG_ON(!drm_mm_node_allocated(node));
> 		vma = container_of(node, typeof(*vma), node);
>
> 		/* If we are using coloring to insert guard pages between
>diff --git a/drivers/gpu/drm/i915/i915_vma.c
>b/drivers/gpu/drm/i915/i915_vma.c
>index 411047d6a909..d10614091bb9 100644
>--- a/drivers/gpu/drm/i915/i915_vma.c
>+++ b/drivers/gpu/drm/i915/i915_vma.c
>@@ -795,7 +795,7 @@ void i915_vma_reopen(struct i915_vma *vma)
>
> static void __i915_vma_destroy(struct i915_vma *vma)
> {
>-	GEM_BUG_ON(vma->node.allocated);
>+	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
> 	GEM_BUG_ON(vma->fence);
>
> 	mutex_lock(&vma->vm->mutex);
>diff --git a/drivers/gpu/drm/i915/i915_vma.h
>b/drivers/gpu/drm/i915/i915_vma.h
>index 8bcb5812c446..e49b199f7de7 100644
>--- a/drivers/gpu/drm/i915/i915_vma.h
>+++ b/drivers/gpu/drm/i915/i915_vma.h
>@@ -228,7 +228,7 @@ static inline bool i915_vma_is_closed(const struct
>i915_vma *vma)
> static inline u32 i915_ggtt_offset(const struct i915_vma *vma)
> {
> 	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
>-	GEM_BUG_ON(!vma->node.allocated);
>+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
> 	GEM_BUG_ON(upper_32_bits(vma->node.start));
> 	GEM_BUG_ON(upper_32_bits(vma->node.start + vma->node.size -
>1));
> 	return lower_32_bits(vma->node.start);
>@@ -390,7 +390,7 @@ static inline bool i915_vma_is_bound(const struct
>i915_vma *vma,
> static inline bool i915_node_color_differs(const struct drm_mm_node
>*node,
> 					   unsigned long color)
> {
>-	return node->allocated && node->color != color;
>+	return drm_mm_node_allocated(node) && node->color != color;
> }
>
> /**
>diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c
>b/drivers/gpu/drm/selftests/test-drm_mm.c
>index 388f9844f4ba..9aabe82dcd3a 100644
>--- a/drivers/gpu/drm/selftests/test-drm_mm.c
>+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
>@@ -854,7 +854,7 @@ static bool assert_contiguous_in_range(struct
>drm_mm *mm,
>
> 	if (start > 0) {
> 		node = __drm_mm_interval_first(mm, 0, start - 1);
>-		if (node->allocated) {
>+		if (drm_mm_node_allocated(node)) {
> 			pr_err("node before start: node=%llx+%llu,
>start=%llx\n",
> 			       node->start, node->size, start);
> 			return false;
>@@ -863,7 +863,7 @@ static bool assert_contiguous_in_range(struct
>drm_mm *mm,
>
> 	if (end < U64_MAX) {
> 		node = __drm_mm_interval_first(mm, end, U64_MAX);
>-		if (node->allocated) {
>+		if (drm_mm_node_allocated(node)) {
> 			pr_err("node after end: node=%llx+%llu,
>end=%llx\n",
> 			       node->start, node->size, end);
> 			return false;
>@@ -1156,12 +1156,12 @@ static void show_holes(const struct drm_mm
>*mm, int count)
> 		struct drm_mm_node *next = list_next_entry(hole,
>node_list);
> 		const char *node1 = NULL, *node2 = NULL;
>
>-		if (hole->allocated)
>+		if (drm_mm_node_allocated(hole))
> 			node1 = kasprintf(GFP_KERNEL,
> 					  "[%llx + %lld, color=%ld], ",
> 					  hole->start, hole->size, hole->color);
>
>-		if (next->allocated)
>+		if (drm_mm_node_allocated(next))
> 			node2 = kasprintf(GFP_KERNEL,
> 					  ", [%llx + %lld, color=%ld]",
> 					  next->start, next->size, next->color);
>@@ -1900,18 +1900,18 @@ static void separate_adjacent_colors(const struct
>drm_mm_node *node,
> 				     u64 *start,
> 				     u64 *end)
> {
>-	if (node->allocated && node->color != color)
>+	if (drm_mm_node_allocated(node) && node->color != color)
> 		++*start;
>
> 	node = list_next_entry(node, node_list);
>-	if (node->allocated && node->color != color)
>+	if (drm_mm_node_allocated(node) && node->color != color)
> 		--*end;
> }
>
> static bool colors_abutt(const struct drm_mm_node *node)
> {
> 	if (!drm_mm_hole_follows(node) &&
>-	    list_next_entry(node, node_list)->allocated) {
>+	    drm_mm_node_allocated(list_next_entry(node, node_list))) {
> 		pr_err("colors abutt; %ld [%llx + %llx] is next to %ld [%llx +
>%llx]!\n",
> 		       node->color, node->start, node->size,
> 		       list_next_entry(node, node_list)->color,
>diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
>index f1f0a7c87771..b00e20f5ce05 100644
>--- a/drivers/gpu/drm/vc4/vc4_crtc.c
>+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
>@@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc
>*crtc,
> 	struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
> 	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
>
>-	if (vc4_state->mm.allocated) {
>+	if (drm_mm_node_allocated(&vc4_state->mm)) {
> 		unsigned long flags;
>
> 		spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
>diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
>index 9936b15d0bf1..5a43659da319 100644
>--- a/drivers/gpu/drm/vc4/vc4_hvs.c
>+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
>@@ -315,7 +315,7 @@ static void vc4_hvs_unbind(struct device *dev, struct
>device *master,
> 	struct drm_device *drm = dev_get_drvdata(master);
> 	struct vc4_dev *vc4 = drm->dev_private;
>
>-	if (vc4->hvs->mitchell_netravali_filter.allocated)
>+	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
> 		drm_mm_remove_node(&vc4->hvs-
>>mitchell_netravali_filter);
>
> 	drm_mm_takedown(&vc4->hvs->dlist_mm);
>diff --git a/drivers/gpu/drm/vc4/vc4_plane.c
>b/drivers/gpu/drm/vc4/vc4_plane.c
>index 5e5f90810aca..4934127f0d76 100644
>--- a/drivers/gpu/drm/vc4/vc4_plane.c
>+++ b/drivers/gpu/drm/vc4/vc4_plane.c
>@@ -178,7 +178,7 @@ static void vc4_plane_destroy_state(struct drm_plane
>*plane,
> 	struct vc4_dev *vc4 = to_vc4_dev(plane->dev);
> 	struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
>
>-	if (vc4_state->lbm.allocated) {
>+	if (drm_mm_node_allocated(&vc4_state->lbm)) {
> 		unsigned long irqflags;
>
> 		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
>@@ -557,7 +557,7 @@ static int vc4_plane_allocate_lbm(struct
>drm_plane_state *state)
> 	/* Allocate the LBM memory that the HVS will use for temporary
> 	 * storage due to our scaling/format conversion.
> 	 */
>-	if (!vc4_state->lbm.allocated) {
>+	if (!drm_mm_node_allocated(&vc4_state->lbm)) {
> 		int ret;
>
> 		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
>diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
>index 2c3bbb43c7d1..d7939c054259 100644
>--- a/include/drm/drm_mm.h
>+++ b/include/drm/drm_mm.h
>@@ -168,8 +168,9 @@ struct drm_mm_node {
> 	struct rb_node rb_hole_addr;
> 	u64 __subtree_last;
> 	u64 hole_size;
>-	bool allocated : 1;
>-	bool scanned_block : 1;
>+	unsigned long flags;
>+#define DRM_MM_NODE_ALLOCATED_BIT	0
>+#define DRM_MM_NODE_SCANNED_BIT		1
> #ifdef CONFIG_DRM_DEBUG_MM
> 	depot_stack_handle_t stack;
> #endif
>@@ -253,7 +254,7 @@ struct drm_mm_scan {
>  */
> static inline bool drm_mm_node_allocated(const struct drm_mm_node
>*node)
> {
>-	return node->allocated;
>+	return test_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
> }
>
> /**
>--
>2.23.0
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield
  2019-09-16 19:45   ` Ruhl, Michael J
@ 2019-10-03  7:07     ` Chris Wilson
  2019-10-03 14:02       ` Ruhl, Michael J
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2019-10-03  7:07 UTC (permalink / raw)
  To: Ruhl, Michael J, dri-devel; +Cc: intel-gfx

Quoting Ruhl, Michael J (2019-09-16 20:45:14)
> >-----Original Message-----
> >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf
> >Of Chris Wilson
> >Sent: Sunday, September 15, 2019 2:46 PM
> >@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm,
> >struct drm_mm_node *node)
> >
> >       node->mm = mm;
> >
> >+      __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
> 
> Maybe a silly question(s), but you appear to be mixing atomic bit ops
> (clear_ and test_) with non-atomic bit ops __xxx_bit().
> 
> Should that be a concern?   Should that be mention in the commit?

Generally yes, but this is inside an allocation function so the new node
cannot be accessed concurrently yet (and manipulation of the drm_mm
itself requires external serialisation).

The concern is with blocks like

> >diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> >index f1f0a7c87771..b00e20f5ce05 100644
> >--- a/drivers/gpu/drm/vc4/vc4_crtc.c
> >+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> >@@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc
> >*crtc,
> >       struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
> >       struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
> >
> >-      if (vc4_state->mm.allocated) {
> >+      if (drm_mm_node_allocated(&vc4_state->mm)) {
> >               unsigned long flags;
> >
> >               spin_lock_irqsave(&vc4->hvs->mm_lock, flags);

where we are testing the bit prior to taking the lock to serialise
removal before free. To avoid the cost of serialising here we have to be
sure that any other thread has completely stopped using the drm_mm_node
when it is marked as released.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-09-15 18:45 [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
                   ` (3 preceding siblings ...)
  2019-09-16  8:51 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-10-03 13:16 ` Tvrtko Ursulin
  4 siblings, 0 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2019-10-03 13:16 UTC (permalink / raw)
  To: Chris Wilson, dri-devel; +Cc: intel-gfx


On 15/09/2019 19:45, Chris Wilson wrote:
> Make dma_fence_enable_sw_signaling() behave like its
> dma_fence_add_callback() and dma_fence_default_wait() counterparts and
> perform the test to enable signaling under the fence->lock, along with
> the action to do so. This ensure that should an implementation be trying
> to flush the cb_list (by signaling) on retirement before freeing the
> fence, it can do so in a race-free manner.
> 
> See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
> with dma_fence_signal").
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/dma-buf/dma-fence.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 2c136aee3e79..587727089134 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -285,19 +285,18 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>   {
>   	unsigned long flags;
>   
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +		return;
> +
> +	spin_lock_irqsave(fence->lock, flags);
>   	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

I dare not to ask if this couldn't be the non-atomic version, but I have 
empirically proven to myself things are not that straightforward around 
here.

>   			      &fence->flags) &&
> -	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
>   	    fence->ops->enable_signaling) {
>   		trace_dma_fence_enable_signal(fence);
> -
> -		spin_lock_irqsave(fence->lock, flags);
> -
>   		if (!fence->ops->enable_signaling(fence))
>   			dma_fence_signal_locked(fence);
> -
> -		spin_unlock_irqrestore(fence->lock, flags);
>   	}
> +	spin_unlock_irqrestore(fence->lock, flags);
>   }
>   EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield
  2019-09-15 18:45 ` [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
  2019-09-16 19:45   ` Ruhl, Michael J
@ 2019-10-03 13:42   ` Tvrtko Ursulin
  1 sibling, 0 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2019-10-03 13:42 UTC (permalink / raw)
  To: Chris Wilson, dri-devel; +Cc: intel-gfx


On 15/09/2019 19:45, Chris Wilson wrote:
> The ulterior motive to switching the booleans over to bitops is to
> allow use of the allocated flag as a bitlock.

Locked bit usage applies only to DRM_MM_NODE_ALLOCATED_BIT?

Any value in extracting the conversion to calling drm_mm_node_allocated 
ahead of the main patch? Maybe introducing drm_mm_node_scanned_block 
helper in a separate patch as well. Then it's really easy to focus on 
the real change in a last patch which just switches the implementation 
over. But it's not too critical to do this. I don't think patch is that 
big or that it has any potential for regression on it's own to be 
important it is split into smaller chunks. So I think this is "would be 
nice" category of suggestions.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/drm_mm.c                      | 36 +++++++++++--------
>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  6 ++--
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  2 +-
>   drivers/gpu/drm/i915/i915_gem.c               | 16 ++++-----
>   drivers/gpu/drm/i915/i915_gem_evict.c         |  2 +-
>   drivers/gpu/drm/i915/i915_vma.c               |  2 +-
>   drivers/gpu/drm/i915/i915_vma.h               |  4 +--
>   drivers/gpu/drm/selftests/test-drm_mm.c       | 14 ++++----
>   drivers/gpu/drm/vc4/vc4_crtc.c                |  2 +-
>   drivers/gpu/drm/vc4/vc4_hvs.c                 |  2 +-
>   drivers/gpu/drm/vc4/vc4_plane.c               |  4 +--
>   include/drm/drm_mm.h                          |  7 ++--
>   12 files changed, 52 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 4581c5387372..211967006cec 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -174,7 +174,7 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
>   
>   	node->__subtree_last = LAST(node);
>   
> -	if (hole_node->allocated) {
> +	if (drm_mm_node_allocated(hole_node)) {
>   		rb = &hole_node->rb;
>   		while (rb) {
>   			parent = rb_entry(rb, struct drm_mm_node, rb);
> @@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
>   
>   	node->mm = mm;
>   
> +	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   	list_add(&node->node_list, &hole->node_list);
>   	drm_mm_interval_tree_add_node(hole, node);
> -	node->allocated = true;
>   	node->hole_size = 0;
>   
>   	rm_hole(hole);
> @@ -543,9 +543,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
>   		node->color = color;
>   		node->hole_size = 0;
>   
> +		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   		list_add(&node->node_list, &hole->node_list);
>   		drm_mm_interval_tree_add_node(hole, node);
> -		node->allocated = true;
>   
>   		rm_hole(hole);
>   		if (adj_start > hole_start)
> @@ -561,6 +561,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
>   }
>   EXPORT_SYMBOL(drm_mm_insert_node_in_range);
>   
> +static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)
> +{
> +	return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
> +}
> +
>   /**
>    * drm_mm_remove_node - Remove a memory node from the allocator.
>    * @node: drm_mm_node to remove
> @@ -574,8 +579,8 @@ void drm_mm_remove_node(struct drm_mm_node *node)
>   	struct drm_mm *mm = node->mm;
>   	struct drm_mm_node *prev_node;
>   
> -	DRM_MM_BUG_ON(!node->allocated);
> -	DRM_MM_BUG_ON(node->scanned_block);
> +	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
> +	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
>   
>   	prev_node = list_prev_entry(node, node_list);
>   
> @@ -584,11 +589,12 @@ void drm_mm_remove_node(struct drm_mm_node *node)
>   
>   	drm_mm_interval_tree_remove(node, &mm->interval_tree);
>   	list_del(&node->node_list);
> -	node->allocated = false;
>   
>   	if (drm_mm_hole_follows(prev_node))
>   		rm_hole(prev_node);
>   	add_hole(prev_node);
> +
> +	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   }
>   EXPORT_SYMBOL(drm_mm_remove_node);
>   
> @@ -605,7 +611,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
>   {
>   	struct drm_mm *mm = old->mm;
>   
> -	DRM_MM_BUG_ON(!old->allocated);
> +	DRM_MM_BUG_ON(!drm_mm_node_allocated(old));
>   
>   	*new = *old;
>   
> @@ -622,8 +628,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
>   				&mm->holes_addr);
>   	}
>   
> -	old->allocated = false;
> -	new->allocated = true;
> +	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
>   }
>   EXPORT_SYMBOL(drm_mm_replace_node);
>   
> @@ -731,9 +736,9 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
>   	u64 adj_start, adj_end;
>   
>   	DRM_MM_BUG_ON(node->mm != mm);
> -	DRM_MM_BUG_ON(!node->allocated);
> -	DRM_MM_BUG_ON(node->scanned_block);
> -	node->scanned_block = true;
> +	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
> +	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
> +	__set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
>   	mm->scan_active++;
>   
>   	/* Remove this block from the node_list so that we enlarge the hole
> @@ -818,8 +823,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
>   	struct drm_mm_node *prev_node;
>   
>   	DRM_MM_BUG_ON(node->mm != scan->mm);
> -	DRM_MM_BUG_ON(!node->scanned_block);
> -	node->scanned_block = false;
> +	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
>   
>   	DRM_MM_BUG_ON(!node->mm->scan_active);
>   	node->mm->scan_active--;
> @@ -837,6 +841,8 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
>   		      list_next_entry(node, node_list));
>   	list_add(&node->node_list, &prev_node->node_list);
>   
> +	__clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
> +
>   	return (node->start + node->size > scan->hit_start &&
>   		node->start < scan->hit_end);
>   }
> @@ -917,7 +923,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
>   
>   	/* Clever trick to avoid a special case in the free hole tracking. */
>   	INIT_LIST_HEAD(&mm->head_node.node_list);
> -	mm->head_node.allocated = false;
> +	mm->head_node.flags = 0;
>   	mm->head_node.mm = mm;
>   	mm->head_node.start = start + size;
>   	mm->head_node.size = -size;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 27dbcb508055..c049199a1df5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache,
>   	cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
>   	cache->has_fence = cache->gen < 4;
>   	cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> -	cache->node.allocated = false;
> +	cache->node.flags = 0;
>   	cache->ce = NULL;
>   	cache->rq = NULL;
>   	cache->rq_size = 0;
> @@ -968,7 +968,7 @@ static void reloc_cache_reset(struct reloc_cache *cache)
>   		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>   		io_mapping_unmap_atomic((void __iomem *)vaddr);
>   
> -		if (cache->node.allocated) {
> +		if (drm_mm_node_allocated(&cache->node)) {
>   			ggtt->vm.clear_range(&ggtt->vm,
>   					     cache->node.start,
>   					     cache->node.size);
> @@ -1061,7 +1061,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
>   	}
>   
>   	offset = cache->node.start;
> -	if (cache->node.allocated) {
> +	if (drm_mm_node_allocated(&cache->node)) {
>   		ggtt->vm.insert_page(&ggtt->vm,
>   				     i915_gem_object_get_dma_address(obj, page),
>   				     offset, I915_CACHE_NONE, 0);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 296a82603be0..07fc6f28abcd 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -401,7 +401,7 @@ static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
>   {
>   	struct drm_mm_node *node = &ggtt->uc_fw;
>   
> -	GEM_BUG_ON(!node->allocated);
> +	GEM_BUG_ON(!drm_mm_node_allocated(node));
>   	GEM_BUG_ON(upper_32_bits(node->start));
>   	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
>   
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2da9544fa9a4..b8f7fe311b0a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -351,12 +351,12 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   					       PIN_NOEVICT);
>   	if (!IS_ERR(vma)) {
>   		node.start = i915_ggtt_offset(vma);
> -		node.allocated = false;
> +		node.flags = 0;
>   	} else {
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>   		if (ret)
>   			goto out_unlock;
> -		GEM_BUG_ON(!node.allocated);
> +		GEM_BUG_ON(!drm_mm_node_allocated(&node));
>   	}
>   
>   	mutex_unlock(&i915->drm.struct_mutex);
> @@ -393,7 +393,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   		unsigned page_offset = offset_in_page(offset);
>   		unsigned page_length = PAGE_SIZE - page_offset;
>   		page_length = remain < page_length ? remain : page_length;
> -		if (node.allocated) {
> +		if (drm_mm_node_allocated(&node)) {
>   			ggtt->vm.insert_page(&ggtt->vm,
>   					     i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
>   					     node.start, I915_CACHE_NONE, 0);
> @@ -415,7 +415,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   	i915_gem_object_unlock_fence(obj, fence);
>   out_unpin:
>   	mutex_lock(&i915->drm.struct_mutex);
> -	if (node.allocated) {
> +	if (drm_mm_node_allocated(&node)) {
>   		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
>   		remove_mappable_node(&node);
>   	} else {
> @@ -561,12 +561,12 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   					       PIN_NOEVICT);
>   	if (!IS_ERR(vma)) {
>   		node.start = i915_ggtt_offset(vma);
> -		node.allocated = false;
> +		node.flags = 0;
>   	} else {
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>   		if (ret)
>   			goto out_rpm;
> -		GEM_BUG_ON(!node.allocated);
> +		GEM_BUG_ON(!drm_mm_node_allocated(&node));
>   	}
>   
>   	mutex_unlock(&i915->drm.struct_mutex);
> @@ -604,7 +604,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   		unsigned int page_offset = offset_in_page(offset);
>   		unsigned int page_length = PAGE_SIZE - page_offset;
>   		page_length = remain < page_length ? remain : page_length;
> -		if (node.allocated) {
> +		if (drm_mm_node_allocated(&node)) {
>   			/* flush the write before we modify the GGTT */
>   			intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>   			ggtt->vm.insert_page(&ggtt->vm,
> @@ -636,7 +636,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   out_unpin:
>   	mutex_lock(&i915->drm.struct_mutex);
>   	intel_gt_flush_ggtt_writes(ggtt->vm.gt);
> -	if (node.allocated) {
> +	if (drm_mm_node_allocated(&node)) {
>   		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
>   		remove_mappable_node(&node);
>   	} else {
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index e76c9da9992d..8c1e04f402bc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -299,7 +299,7 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>   			break;
>   		}
>   
> -		GEM_BUG_ON(!node->allocated);
> +		GEM_BUG_ON(!drm_mm_node_allocated(node));
>   		vma = container_of(node, typeof(*vma), node);
>   
>   		/* If we are using coloring to insert guard pages between
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 411047d6a909..d10614091bb9 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -795,7 +795,7 @@ void i915_vma_reopen(struct i915_vma *vma)
>   
>   static void __i915_vma_destroy(struct i915_vma *vma)
>   {
> -	GEM_BUG_ON(vma->node.allocated);
> +	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
>   	GEM_BUG_ON(vma->fence);
>   
>   	mutex_lock(&vma->vm->mutex);
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 8bcb5812c446..e49b199f7de7 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -228,7 +228,7 @@ static inline bool i915_vma_is_closed(const struct i915_vma *vma)
>   static inline u32 i915_ggtt_offset(const struct i915_vma *vma)
>   {
>   	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
> -	GEM_BUG_ON(!vma->node.allocated);
> +	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
>   	GEM_BUG_ON(upper_32_bits(vma->node.start));
>   	GEM_BUG_ON(upper_32_bits(vma->node.start + vma->node.size - 1));
>   	return lower_32_bits(vma->node.start);
> @@ -390,7 +390,7 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
>   static inline bool i915_node_color_differs(const struct drm_mm_node *node,
>   					   unsigned long color)
>   {
> -	return node->allocated && node->color != color;
> +	return drm_mm_node_allocated(node) && node->color != color;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c
> index 388f9844f4ba..9aabe82dcd3a 100644
> --- a/drivers/gpu/drm/selftests/test-drm_mm.c
> +++ b/drivers/gpu/drm/selftests/test-drm_mm.c
> @@ -854,7 +854,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
>   
>   	if (start > 0) {
>   		node = __drm_mm_interval_first(mm, 0, start - 1);
> -		if (node->allocated) {
> +		if (drm_mm_node_allocated(node)) {
>   			pr_err("node before start: node=%llx+%llu, start=%llx\n",
>   			       node->start, node->size, start);
>   			return false;
> @@ -863,7 +863,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
>   
>   	if (end < U64_MAX) {
>   		node = __drm_mm_interval_first(mm, end, U64_MAX);
> -		if (node->allocated) {
> +		if (drm_mm_node_allocated(node)) {
>   			pr_err("node after end: node=%llx+%llu, end=%llx\n",
>   			       node->start, node->size, end);
>   			return false;
> @@ -1156,12 +1156,12 @@ static void show_holes(const struct drm_mm *mm, int count)
>   		struct drm_mm_node *next = list_next_entry(hole, node_list);
>   		const char *node1 = NULL, *node2 = NULL;
>   
> -		if (hole->allocated)
> +		if (drm_mm_node_allocated(hole))
>   			node1 = kasprintf(GFP_KERNEL,
>   					  "[%llx + %lld, color=%ld], ",
>   					  hole->start, hole->size, hole->color);
>   
> -		if (next->allocated)
> +		if (drm_mm_node_allocated(next))
>   			node2 = kasprintf(GFP_KERNEL,
>   					  ", [%llx + %lld, color=%ld]",
>   					  next->start, next->size, next->color);
> @@ -1900,18 +1900,18 @@ static void separate_adjacent_colors(const struct drm_mm_node *node,
>   				     u64 *start,
>   				     u64 *end)
>   {
> -	if (node->allocated && node->color != color)
> +	if (drm_mm_node_allocated(node) && node->color != color)
>   		++*start;
>   
>   	node = list_next_entry(node, node_list);
> -	if (node->allocated && node->color != color)
> +	if (drm_mm_node_allocated(node) && node->color != color)
>   		--*end;
>   }
>   
>   static bool colors_abutt(const struct drm_mm_node *node)
>   {
>   	if (!drm_mm_hole_follows(node) &&
> -	    list_next_entry(node, node_list)->allocated) {
> +	    drm_mm_node_allocated(list_next_entry(node, node_list))) {
>   		pr_err("colors abutt; %ld [%llx + %llx] is next to %ld [%llx + %llx]!\n",
>   		       node->color, node->start, node->size,
>   		       list_next_entry(node, node_list)->color,
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index f1f0a7c87771..b00e20f5ce05 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>   	struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
>   	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
>   
> -	if (vc4_state->mm.allocated) {
> +	if (drm_mm_node_allocated(&vc4_state->mm)) {
>   		unsigned long flags;
>   
>   		spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 9936b15d0bf1..5a43659da319 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -315,7 +315,7 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master,
>   	struct drm_device *drm = dev_get_drvdata(master);
>   	struct vc4_dev *vc4 = drm->dev_private;
>   
> -	if (vc4->hvs->mitchell_netravali_filter.allocated)
> +	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
>   		drm_mm_remove_node(&vc4->hvs->mitchell_netravali_filter);
>   
>   	drm_mm_takedown(&vc4->hvs->dlist_mm);
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 5e5f90810aca..4934127f0d76 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -178,7 +178,7 @@ static void vc4_plane_destroy_state(struct drm_plane *plane,
>   	struct vc4_dev *vc4 = to_vc4_dev(plane->dev);
>   	struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
>   
> -	if (vc4_state->lbm.allocated) {
> +	if (drm_mm_node_allocated(&vc4_state->lbm)) {
>   		unsigned long irqflags;
>   
>   		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
> @@ -557,7 +557,7 @@ static int vc4_plane_allocate_lbm(struct drm_plane_state *state)
>   	/* Allocate the LBM memory that the HVS will use for temporary
>   	 * storage due to our scaling/format conversion.
>   	 */
> -	if (!vc4_state->lbm.allocated) {
> +	if (!drm_mm_node_allocated(&vc4_state->lbm)) {
>   		int ret;
>   
>   		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
> index 2c3bbb43c7d1..d7939c054259 100644
> --- a/include/drm/drm_mm.h
> +++ b/include/drm/drm_mm.h
> @@ -168,8 +168,9 @@ struct drm_mm_node {
>   	struct rb_node rb_hole_addr;
>   	u64 __subtree_last;
>   	u64 hole_size;
> -	bool allocated : 1;
> -	bool scanned_block : 1;
> +	unsigned long flags;
> +#define DRM_MM_NODE_ALLOCATED_BIT	0

Document here the rules for manipulating this bit would be good.

> +#define DRM_MM_NODE_SCANNED_BIT		1
>   #ifdef CONFIG_DRM_DEBUG_MM
>   	depot_stack_handle_t stack;
>   #endif
> @@ -253,7 +254,7 @@ struct drm_mm_scan {
>    */
>   static inline bool drm_mm_node_allocated(const struct drm_mm_node *node)
>   {
> -	return node->allocated;
> +	return test_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   }
>   
>   /**
> 

Regards,

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield
  2019-10-03  7:07     ` Chris Wilson
@ 2019-10-03 14:02       ` Ruhl, Michael J
  0 siblings, 0 replies; 10+ messages in thread
From: Ruhl, Michael J @ 2019-10-03 14:02 UTC (permalink / raw)
  To: Chris Wilson, dri-devel; +Cc: intel-gfx

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf
>Of Chris Wilson
>Sent: Thursday, October 3, 2019 3:08 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>; dri-
>devel@lists.freedesktop.org
>Cc: intel-gfx@lists.freedesktop.org
>Subject: RE: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a
>bitfield
>
>Quoting Ruhl, Michael J (2019-09-16 20:45:14)
>> >-----Original Message-----
>> >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On
>Behalf
>> >Of Chris Wilson
>> >Sent: Sunday, September 15, 2019 2:46 PM
>> >@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm,
>> >struct drm_mm_node *node)
>> >
>> >       node->mm = mm;
>> >
>> >+      __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>>
>> Maybe a silly question(s), but you appear to be mixing atomic bit ops
>> (clear_ and test_) with non-atomic bit ops __xxx_bit().
>>
>> Should that be a concern?   Should that be mention in the commit?
>
>Generally yes, but this is inside an allocation function so the new node
>cannot be accessed concurrently yet (and manipulation of the drm_mm
>itself requires external serialisation).

Got it. 

Thanks for the clarification.

Mike


>The concern is with blocks like
>
>> >diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c
>b/drivers/gpu/drm/vc4/vc4_crtc.c
>> >index f1f0a7c87771..b00e20f5ce05 100644
>> >--- a/drivers/gpu/drm/vc4/vc4_crtc.c
>> >+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
>> >@@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc
>> >*crtc,
>> >       struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
>> >       struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
>> >
>> >-      if (vc4_state->mm.allocated) {
>> >+      if (drm_mm_node_allocated(&vc4_state->mm)) {
>> >               unsigned long flags;
>> >
>> >               spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
>
>where we are testing the bit prior to taking the lock to serialise
>removal before free. To avoid the cost of serialising here we have to be
>sure that any other thread has completely stopped using the drm_mm_node
>when it is marked as released.
>-Chris
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-10-03 14:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-15 18:45 [PATCH 1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
2019-09-15 18:45 ` [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a bitfield Chris Wilson
2019-09-16 19:45   ` Ruhl, Michael J
2019-10-03  7:07     ` Chris Wilson
2019-10-03 14:02       ` Ruhl, Michael J
2019-10-03 13:42   ` Tvrtko Ursulin
2019-09-15 19:12 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Patchwork
2019-09-15 19:34 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-16  8:51 ` ✓ Fi.CI.IGT: " Patchwork
2019-10-03 13:16 ` [PATCH 1/2] " Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.