Re: [PATCH 09/25] drm/i915: Remove debugfs/i915_ppgtt_info

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 02/11/2018 16:12, Chris Wilson wrote:
> The information presented here is not relevant to current development.
> We can either use the context information, but more often we want to
> inspect the active gpu state.
> 
> The ulterior motive is to eradicate dev->filelist.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 119 ------------------------
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 134 ----------------------------
>   drivers/gpu/drm/i915/i915_gem_gtt.h |   2 -
>   3 files changed, 255 deletions(-)

I gave r-b for this one back in September.

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

Regards,

Tvrtko

> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e33483687e12..49c1c94cd4a4 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2034,124 +2034,6 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
>   	return 0;
>   }
>   
> -static int per_file_ctx(int id, void *ptr, void *data)
> -{
> -	struct i915_gem_context *ctx = ptr;
> -	struct seq_file *m = data;
> -	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
> -
> -	if (!ppgtt) {
> -		seq_printf(m, "  no ppgtt for context %d\n",
> -			   ctx->user_handle);
> -		return 0;
> -	}
> -
> -	if (i915_gem_context_is_default(ctx))
> -		seq_puts(m, "  default context:\n");
> -	else
> -		seq_printf(m, "  context %d:\n", ctx->user_handle);
> -	ppgtt->debug_dump(ppgtt, m);
> -
> -	return 0;
> -}
> -
> -static void gen8_ppgtt_info(struct seq_file *m,
> -			    struct drm_i915_private *dev_priv)
> -{
> -	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -	int i;
> -
> -	if (!ppgtt)
> -		return;
> -
> -	for_each_engine(engine, dev_priv, id) {
> -		seq_printf(m, "%s\n", engine->name);
> -		for (i = 0; i < 4; i++) {
> -			u64 pdp = I915_READ(GEN8_RING_PDP_UDW(engine, i));
> -			pdp <<= 32;
> -			pdp |= I915_READ(GEN8_RING_PDP_LDW(engine, i));
> -			seq_printf(m, "\tPDP%d 0x%016llx\n", i, pdp);
> -		}
> -	}
> -}
> -
> -static void gen6_ppgtt_info(struct seq_file *m,
> -			    struct drm_i915_private *dev_priv)
> -{
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -
> -	if (IS_GEN6(dev_priv))
> -		seq_printf(m, "GFX_MODE: 0x%08x\n", I915_READ(GFX_MODE));
> -
> -	for_each_engine(engine, dev_priv, id) {
> -		seq_printf(m, "%s\n", engine->name);
> -		if (IS_GEN7(dev_priv))
> -			seq_printf(m, "GFX_MODE: 0x%08x\n",
> -				   I915_READ(RING_MODE_GEN7(engine)));
> -		seq_printf(m, "PP_DIR_BASE: 0x%08x\n",
> -			   I915_READ(RING_PP_DIR_BASE(engine)));
> -		seq_printf(m, "PP_DIR_BASE_READ: 0x%08x\n",
> -			   I915_READ(RING_PP_DIR_BASE_READ(engine)));
> -		seq_printf(m, "PP_DIR_DCLV: 0x%08x\n",
> -			   I915_READ(RING_PP_DIR_DCLV(engine)));
> -	}
> -	if (dev_priv->mm.aliasing_ppgtt) {
> -		struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> -
> -		seq_puts(m, "aliasing PPGTT:\n");
> -		seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd.base.ggtt_offset);
> -
> -		ppgtt->debug_dump(ppgtt, m);
> -	}
> -
> -	seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
> -}
> -
> -static int i915_ppgtt_info(struct seq_file *m, void *data)
> -{
> -	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	struct drm_device *dev = &dev_priv->drm;
> -	struct drm_file *file;
> -	int ret;
> -
> -	mutex_lock(&dev->filelist_mutex);
> -	ret = mutex_lock_interruptible(&dev->struct_mutex);
> -	if (ret)
> -		goto out_unlock;
> -
> -	intel_runtime_pm_get(dev_priv);
> -
> -	if (INTEL_GEN(dev_priv) >= 8)
> -		gen8_ppgtt_info(m, dev_priv);
> -	else if (INTEL_GEN(dev_priv) >= 6)
> -		gen6_ppgtt_info(m, dev_priv);
> -
> -	list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> -		struct drm_i915_file_private *file_priv = file->driver_priv;
> -		struct task_struct *task;
> -
> -		task = get_pid_task(file->pid, PIDTYPE_PID);
> -		if (!task) {
> -			ret = -ESRCH;
> -			goto out_rpm;
> -		}
> -		seq_printf(m, "\nproc: %s\n", task->comm);
> -		put_task_struct(task);
> -		idr_for_each(&file_priv->context_idr, per_file_ctx,
> -			     (void *)(unsigned long)m);
> -	}
> -
> -out_rpm:
> -	intel_runtime_pm_put(dev_priv);
> -	mutex_unlock(&dev->struct_mutex);
> -out_unlock:
> -	mutex_unlock(&dev->filelist_mutex);
> -	return ret;
> -}
> -
>   static int count_irq_waiters(struct drm_i915_private *i915)
>   {
>   	struct intel_engine_cs *engine;
> @@ -4787,7 +4669,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
>   	{"i915_context_status", i915_context_status, 0},
>   	{"i915_forcewake_domains", i915_forcewake_domains, 0},
>   	{"i915_swizzle_info", i915_swizzle_info, 0},
> -	{"i915_ppgtt_info", i915_ppgtt_info, 0},
>   	{"i915_llc", i915_llc, 0},
>   	{"i915_edp_psr_status", i915_edp_psr_status, 0},
>   	{"i915_energy_uJ", i915_energy_uJ, 0},
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b7120bf03c31..db375cda6203 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1490,84 +1490,6 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
>   	return -ENOMEM;
>   }
>   
> -static void gen8_dump_pdp(struct i915_hw_ppgtt *ppgtt,
> -			  struct i915_page_directory_pointer *pdp,
> -			  u64 start, u64 length,
> -			  gen8_pte_t scratch_pte,
> -			  struct seq_file *m)
> -{
> -	struct i915_address_space *vm = &ppgtt->vm;
> -	struct i915_page_directory *pd;
> -	u32 pdpe;
> -
> -	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
> -		struct i915_page_table *pt;
> -		u64 pd_len = length;
> -		u64 pd_start = start;
> -		u32 pde;
> -
> -		if (pdp->page_directory[pdpe] == ppgtt->vm.scratch_pd)
> -			continue;
> -
> -		seq_printf(m, "\tPDPE #%d\n", pdpe);
> -		gen8_for_each_pde(pt, pd, pd_start, pd_len, pde) {
> -			u32 pte;
> -			gen8_pte_t *pt_vaddr;
> -
> -			if (pd->page_table[pde] == ppgtt->vm.scratch_pt)
> -				continue;
> -
> -			pt_vaddr = kmap_atomic_px(pt);
> -			for (pte = 0; pte < GEN8_PTES; pte += 4) {
> -				u64 va = (pdpe << GEN8_PDPE_SHIFT |
> -					  pde << GEN8_PDE_SHIFT |
> -					  pte << GEN8_PTE_SHIFT);
> -				int i;
> -				bool found = false;
> -
> -				for (i = 0; i < 4; i++)
> -					if (pt_vaddr[pte + i] != scratch_pte)
> -						found = true;
> -				if (!found)
> -					continue;
> -
> -				seq_printf(m, "\t\t0x%llx [%03d,%03d,%04d]: =", va, pdpe, pde, pte);
> -				for (i = 0; i < 4; i++) {
> -					if (pt_vaddr[pte + i] != scratch_pte)
> -						seq_printf(m, " %llx", pt_vaddr[pte + i]);
> -					else
> -						seq_puts(m, "  SCRATCH ");
> -				}
> -				seq_puts(m, "\n");
> -			}
> -			kunmap_atomic(pt_vaddr);
> -		}
> -	}
> -}
> -
> -static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
> -{
> -	struct i915_address_space *vm = &ppgtt->vm;
> -	const gen8_pte_t scratch_pte = vm->scratch_pte;
> -	u64 start = 0, length = ppgtt->vm.total;
> -
> -	if (use_4lvl(vm)) {
> -		u64 pml4e;
> -		struct i915_pml4 *pml4 = &ppgtt->pml4;
> -		struct i915_page_directory_pointer *pdp;
> -
> -		gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) {
> -			if (pml4->pdps[pml4e] == ppgtt->vm.scratch_pdp)
> -				continue;
> -
> -			seq_printf(m, "    PML4E #%llu\n", pml4e);
> -			gen8_dump_pdp(ppgtt, pdp, start, length, scratch_pte, m);
> -		}
> -	} else {
> -		gen8_dump_pdp(ppgtt, &ppgtt->pdp, start, length, scratch_pte, m);
> -	}
> -}
> -
>   static int gen8_preallocate_top_level_pdp(struct i915_hw_ppgtt *ppgtt)
>   {
>   	struct i915_address_space *vm = &ppgtt->vm;
> @@ -1672,7 +1594,6 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
>   		gen8_ppgtt_notify_vgt(ppgtt, true);
>   
>   	ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
> -	ppgtt->debug_dump = gen8_dump_ppgtt;
>   
>   	ppgtt->vm.vma_ops.bind_vma    = ppgtt_bind_vma;
>   	ppgtt->vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
> @@ -1688,60 +1609,6 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
>   	return ERR_PTR(err);
>   }
>   
> -static void gen6_dump_ppgtt(struct i915_hw_ppgtt *base, struct seq_file *m)
> -{
> -	struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(base);
> -	const gen6_pte_t scratch_pte = base->vm.scratch_pte;
> -	struct i915_page_table *pt;
> -	u32 pte, pde;
> -
> -	gen6_for_all_pdes(pt, &base->pd, pde) {
> -		gen6_pte_t *vaddr;
> -
> -		if (pt == base->vm.scratch_pt)
> -			continue;
> -
> -		if (i915_vma_is_bound(ppgtt->vma, I915_VMA_GLOBAL_BIND)) {
> -			u32 expected =
> -				GEN6_PDE_ADDR_ENCODE(px_dma(pt)) |
> -				GEN6_PDE_VALID;
> -			u32 pd_entry = readl(ppgtt->pd_addr + pde);
> -
> -			if (pd_entry != expected)
> -				seq_printf(m,
> -					   "\tPDE #%d mismatch: Actual PDE: %x Expected PDE: %x\n",
> -					   pde,
> -					   pd_entry,
> -					   expected);
> -
> -			seq_printf(m, "\tPDE: %x\n", pd_entry);
> -		}
> -
> -		vaddr = kmap_atomic_px(base->pd.page_table[pde]);
> -		for (pte = 0; pte < GEN6_PTES; pte += 4) {
> -			int i;
> -
> -			for (i = 0; i < 4; i++)
> -				if (vaddr[pte + i] != scratch_pte)
> -					break;
> -			if (i == 4)
> -				continue;
> -
> -			seq_printf(m, "\t\t(%03d, %04d) %08llx: ",
> -				   pde, pte,
> -				   (pde * GEN6_PTES + pte) * I915_GTT_PAGE_SIZE);
> -			for (i = 0; i < 4; i++) {
> -				if (vaddr[pte + i] != scratch_pte)
> -					seq_printf(m, " %08x", vaddr[pte + i]);
> -				else
> -					seq_puts(m, "  SCRATCH");
> -			}
> -			seq_puts(m, "\n");
> -		}
> -		kunmap_atomic(vaddr);
> -	}
> -}
> -
>   /* Write pde (index) from the page directory @pd to the page table @pt */
>   static inline void gen6_write_pde(const struct gen6_hw_ppgtt *ppgtt,
>   				  const unsigned int pde,
> @@ -2129,7 +1996,6 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
>   	ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range;
>   	ppgtt->base.vm.insert_entries = gen6_ppgtt_insert_entries;
>   	ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup;
> -	ppgtt->base.debug_dump = gen6_dump_ppgtt;
>   
>   	ppgtt->base.vm.vma_ops.bind_vma    = ppgtt_bind_vma;
>   	ppgtt->base.vm.vma_ops.unbind_vma  = ppgtt_unbind_vma;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 4874da09a3c4..e2360f16427a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -413,8 +413,6 @@ struct i915_hw_ppgtt {
>   		struct i915_page_directory_pointer pdp;	/* GEN8+ */
>   		struct i915_page_directory pd;		/* GEN6-7 */
>   	};
> -
> -	void (*debug_dump)(struct i915_hw_ppgtt *ppgtt, struct seq_file *m);
>   };
>   
>   struct gen6_hw_ppgtt {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx