[Intel-gfx] [PATCH 23/26] drm/i915: Force pd restore when PDEs change, gen6-7
The docs say you cannot change the PDEs of a currently running context. If you
are changing the PDEs of the currently running context then. We never
map new PDEs of a running context, and expect them to be present - so I
think this is okay. (We can unmap, but this should also be okay since we
only unmap unreferenced objects that the GPU shouldn't be tryingto
va->pa xlate.) The MI_SET_CONTEXT command does have a flag to signal
that even if the context is the same, force a reload. It's unclear
exactly what this does, but I have a hunch it's the right thing to do.
The logic assumes that we always emit a context switch after mapping new
PDEs, and before we submit a batch. This is the case today, and has been
the case since the inception of hardware contexts. A note in the comment
let's the user know.
NOTE: I have no evidence to suggest this is actually needed other than a
few tidbits which lead me to believe there are some corner cases that
will require it. I'm mostly depending on the reload of DCLV to
invalidate the old TLBs. We can try to remove this patch and see what
happens.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_context.c| 15 ---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +
drivers/gpu/drm/i915/i915_gem_gtt.c| 17 -
drivers/gpu/drm/i915/i915_gem_gtt.h| 2 ++
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index a899e11..6ad5380 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -636,9 +636,18 @@ mi_set_context(struct intel_ring_buffer *ring,
static inline bool should_skip_switch(struct intel_ring_buffer *ring,
struct i915_hw_context *from,
- struct i915_hw_context *to)
+ struct i915_hw_context *to,
+ u32 *flags)
{
- if (from == to && from->last_ring == ring && !to->remap_slice)
+ if (test_and_clear_bit(ring->id, &to->vm->pd_reload_mask)) {
+ *flags |= MI_FORCE_RESTORE;
+ return false;
+ }
+
+ if (to->remap_slice)
+ return false;
+
+ if (from == to && from->last_ring == ring)
return true;
return false;
@@ -658,7 +667,7 @@ static int do_switch(struct intel_ring_buffer *ring,
BUG_ON(!i915_gem_obj_is_pinned(from->obj));
}
- if (should_skip_switch(ring, from, to))
+ if (should_skip_switch(ring, from, to, &hw_flags))
return 0;
/* Trying to pin first makes error handling easier. */
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 856fa9d..bb901e8 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1162,6 +1162,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if (ret)
goto err;
+ /* XXX: Reserve has possibly change PDEs which means we must do a
+* context switch before we can coherently read some of the reserved
+* VMAs. */
+
/* The objects are in their final locations, apply the relocations. */
if (need_relocs)
ret = i915_gem_execbuffer_relocate(eb);
@@ -1263,6 +1267,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
goto err;
}
} else {
+ WARN_ON(vm->pd_reload_mask & (1dispatch_execbuffer(ring,
exec_start, exec_len,
flags);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d3c77d1..6d904c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1228,6 +1228,16 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
return 0;
}
+/* PDE TLBs are a pain invalidate pre GEN8. It requires a context reload. If we
+ * are switching between contexts with the same LRCA, we also must do a force
+ * restore.
+ */
+#define ppgtt_invalidate_tlbs(vm) do {\
+ if (INTEL_INFO(vm->dev)->gen < 8) { \
+ vm->pd_reload_mask = INTEL_INFO(vm->dev)->ring_mask; \
+ } \
+} while(0)
+
static int
ppgtt_bind_vma(struct i915_vma *vma,
enum i915_cache_level cache_level,
@@ -1242,10 +1252,13 @@ ppgtt_bind_vma(struct i915_vma *vma,
vma->node.size);
if (ret)
return ret;
+
+ ppgtt_invalidate_tlbs(vma->vm);
}
vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start,
cache
[Intel-gfx] [PATCH 24/26] drm/i915: Finish gen6/7 dynamic page table allocation
This patch continues on the idea from the previous patch. From here on,
in the steady state, PDEs are all pointing to the scratch page table (as
recommended in the spec). When an object is allocated in the VA range,
the code will determine if we need to allocate a page for the page
table. Similarly when the object is destroyed, we will remove, and free
the page table pointing the PDE back to the scratch page.
Following patches will work to unify the code a bit as we bring in GEN8
support. GEN6 and GEN8 are different enough that I had a hard time to
get to this point with as much common code as I do.
The aliasing PPGTT must pre-allocate all of the page tables. There are a
few reasons for this. Two trivial ones: aliasing ppgtt goes through the
ggtt paths, so it's hard to maintain, we currently do not restore the
default context (assuming the previous force reload is indeed
necessary). Most importantly though, the only way (it seems from
empirical evidence) to invalidate the CS TLBs on non-render ring is to
either use ring sync (which requires actually stopping the rings in
order to synchronize when the sync completes vs. where you are in
execution), or to reload DCLV. Since without full PPGTT we do not ever
reload the DCLV register, there is no good way to achieve this. The
simplest solution is just to not support dynamic page table
creation/destruction in the aliasing PPGTT.
We could always reload DCLV, but this seems like quite a bit of excess
overhead only to save at most 2MB-4k of memory for the aliasing PPGTT
page tables.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 123 +---
drivers/gpu/drm/i915/i915_trace.h | 108
4 files changed, 224 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b19442c..eeef032 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2373,7 +2373,7 @@ static inline void i915_gem_chipset_flush(struct
drm_device *dev)
if (INTEL_INFO(dev)->gen < 6)
intel_gtt_chipset_flush();
}
-int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
+int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt,
bool aliasing);
bool intel_enable_ppgtt(struct drm_device *dev, bool full);
/* i915_gem_stolen.c */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 6ad5380..185c926 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -209,7 +209,7 @@ create_vm_for_ctx(struct drm_device *dev, struct
i915_hw_context *ctx)
if (!ppgtt)
return ERR_PTR(-ENOMEM);
- ret = i915_gem_init_ppgtt(dev, ppgtt);
+ ret = i915_gem_init_ppgtt(dev, ppgtt, ctx->file_priv == NULL);
if (ret) {
kfree(ppgtt);
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6d904c9..846a5b5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1016,13 +1016,54 @@ static void gen6_ppgtt_insert_entries(struct
i915_address_space *vm,
kunmap_atomic(pt_vaddr);
}
+static DECLARE_BITMAP(new_page_tables, I915_PDES_PER_PD);
static int gen6_alloc_va_range(struct i915_address_space *vm,
uint64_t start, uint64_t length)
{
+ struct drm_device *dev = vm->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
struct i915_pagetab *pt;
+ const uint32_t start_save = start, length_save = length;
uint32_t pde, temp;
+ int ret;
+
+ BUG_ON(upper_32_bits(start));
+
+ bitmap_zero(new_page_tables, I915_PDES_PER_PD);
+
+ trace_i915_va_alloc(vm, start, length);
+
+ /* The allocation is done in two stages so that we can bail out with
+* minimal amount of pain. The first stage finds new page tables that
+* need allocation. The second stage marks use ptes within the page
+* tables.
+*/
+ gen6_for_each_pde(pt, &ppgtt->pd, start, length, temp, pde) {
+ if (pt != ppgtt->scratch_pt) {
+ WARN_ON(bitmap_empty(pt->used_ptes, GEN6_PTES_PER_PT));
+ continue;
+ }
+
+ /* We've already allocated a page table */
+ WARN_ON(!bitmap_empty(pt->used_ptes, GEN6_PTES_PER_PT));
+
+ pt = alloc_pt_single(dev);
+ if (IS_ERR(pt)) {
+ ret = PTR_ERR(pt);
+ goto unwind_out;
+ }
+
+ ppgtt->pd.page_tables[p
[Intel-gfx] [PATCH 16/26] drm/i915: Generalize GEN6 mapping
Having a more general way of doing mappings will allow the ability to
easy map and unmap a specific page table. Specifically in this case, we
pass down the page directory + entry, and the page table to map. This
works similarly to the x86 code.
The same work will need to happen for GEN8. At that point I will try to
combine functionality.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 61 +++--
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
2 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5c08cf9..35acccb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -663,18 +663,13 @@ bail:
static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
{
- struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
struct i915_address_space *vm = &ppgtt->base;
- gen6_gtt_pte_t __iomem *pd_addr;
gen6_gtt_pte_t scratch_pte;
uint32_t pd_entry;
int pte, pde;
scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true);
- pd_addr = (gen6_gtt_pte_t __iomem *)dev_priv->gtt.gsm +
- ppgtt->pd.pd_offset / sizeof(gen6_gtt_pte_t);
-
seq_printf(m, " VM %p (pd_offset %x-%x):\n", vm,
ppgtt->pd.pd_offset,
ppgtt->pd.pd_offset + ppgtt->num_pd_entries);
@@ -682,7 +677,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt,
struct seq_file *m)
u32 expected;
gen6_gtt_pte_t *pt_vaddr;
dma_addr_t pt_addr = ppgtt->pd.page_tables[pde]->daddr;
- pd_entry = readl(pd_addr + pde);
+ pd_entry = readl(ppgtt->pd_addr + pde);
expected = (GEN6_PDE_ADDR_ENCODE(pt_addr) | GEN6_PDE_VALID);
if (pd_entry != expected)
@@ -718,39 +713,43 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt,
struct seq_file *m)
}
}
-static void gen6_map_single(struct i915_hw_ppgtt *ppgtt,
- const unsigned pde_index,
- dma_addr_t daddr)
+/* Map pde (index) from the page directory @pd to the page table @pt */
+static void gen6_map_single(struct i915_pagedir *pd,
+ const int pde, struct i915_pagetab *pt)
{
- struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
- uint32_t pd_entry;
- gen6_gtt_pte_t __iomem *pd_addr = (gen6_gtt_pte_t
__iomem*)dev_priv->gtt.gsm;
- pd_addr += ppgtt->pd.pd_offset / sizeof(gen6_gtt_pte_t);
+ struct i915_hw_ppgtt *ppgtt =
+ container_of(pd, struct i915_hw_ppgtt, pd);
+ u32 pd_entry;
- pd_entry = GEN6_PDE_ADDR_ENCODE(daddr);
+ pd_entry = GEN6_PDE_ADDR_ENCODE(pt->daddr);
pd_entry |= GEN6_PDE_VALID;
- writel(pd_entry, pd_addr + pde_index);
+ writel(pd_entry, ppgtt->pd_addr + pde);
+
+ /* XXX: Caller needs to make sure the write completes if necessary */
}
/* Map all the page tables found in the ppgtt structure to incrementing page
* directories. */
-static void gen6_map_page_tables(struct i915_hw_ppgtt *ppgtt)
+static void gen6_map_page_range(struct drm_i915_private *dev_priv,
+ struct i915_pagedir *pd, unsigned pde, size_t n)
{
- struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
- int i;
+ if (WARN_ON(pde + n > I915_PDES_PER_PD))
+ n = I915_PDES_PER_PD - pde;
- WARN_ON(ppgtt->pd.pd_offset & 0x3f);
- for (i = 0; i < ppgtt->num_pd_entries; i++)
- gen6_map_single(ppgtt, i, ppgtt->pd.page_tables[i]->daddr);
+ n += pde;
+
+ for (; pde < n; pde++)
+ gen6_map_single(pd, pde, pd->page_tables[pde]);
+ /* Make sure write is complete before other code can use this page
+* table. Also require for WC mapped PTEs */
readl(dev_priv->gtt.gsm);
}
static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
{
BUG_ON(ppgtt->pd.pd_offset & 0x3f);
-
return (ppgtt->pd.pd_offset / 64) << 16;
}
@@ -1184,7 +1183,10 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt->pd.pd_offset =
ppgtt->node.start / PAGE_SIZE * sizeof(gen6_gtt_pte_t);
- gen6_map_page_tables(ppgtt);
+ ppgtt->pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm +
+ ppgtt->pd.pd_offset / sizeof(gen6_gtt_pte_t);
+
+ gen6_map_page_range(dev_priv, &ppgtt->pd, 0, ppgtt->num_pd_entries);
DRM_DEBUG_DRIVER("Allocated pde space (%ldM) at GTT entry: %lx\n",
ppgtt->node.size >> 20,
@@ -1355,13 +1357,14 @@ void i915_gem_restore_gtt_mappings(struct drm_device
*dev)
list_for_each_entry(vm, &dev_priv->vm_list, global_link) {
/* TODO: Perhaps it shouldn't b
[Intel-gfx] [PATCH 22/26] drm/i915: Extract context switch skip logic
We have some fanciness coming up. This patch just breaks out the logic.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_context.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f918f2c..a899e11 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -634,6 +634,16 @@ mi_set_context(struct intel_ring_buffer *ring,
return ret;
}
+static inline bool should_skip_switch(struct intel_ring_buffer *ring,
+ struct i915_hw_context *from,
+ struct i915_hw_context *to)
+{
+ if (from == to && from->last_ring == ring && !to->remap_slice)
+ return true;
+
+ return false;
+}
+
static int do_switch(struct intel_ring_buffer *ring,
struct i915_hw_context *to)
{
@@ -648,7 +658,7 @@ static int do_switch(struct intel_ring_buffer *ring,
BUG_ON(!i915_gem_obj_is_pinned(from->obj));
}
- if (from == to && from->last_ring == ring && !to->remap_slice)
+ if (should_skip_switch(ring, from, to))
return 0;
/* Trying to pin first makes error handling easier. */
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 26/26] FOR REFERENCE ONLY
Start using size/length through the GEN8 code. The same approach was
taken for gen7. The difference with gen8 to this point is we need to
take care to the do the page directory allocations, as well as the page
tables.
This patch is meant to show how things will look (more or less) if I
keep up in the same direction.
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 104 +++-
drivers/gpu/drm/i915/i915_gem_gtt.h | 37 +
2 files changed, 115 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 846a5b5..1348d48 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -488,29 +488,50 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
kunmap_atomic(pt_vaddr);
}
-static void gen8_free_page_tables(struct i915_pagedir *pd, struct drm_device
*dev)
+static void gen8_free_page_tables(struct i915_pagedir *pd,
+ uint64_t start, uint64_t length,
+ struct drm_device *dev)
{
int i;
if (!pd->page)
return;
- for (i = 0; i < I915_PDES_PER_PD; i++) {
+ for (i = gen8_pte_index(start);
+length && i < GEN8_PTES_PER_PT; i++, length -= PAGE_SIZE) {
+ if (!pd->page_tables[i])
+ continue;
+
free_pt_single(pd->page_tables[i], dev);
pd->page_tables[i] = NULL;
}
}
-static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
+static void gen8_teardown_va_range(struct i915_hw_ppgtt *ppgtt,
+ uint64_t start, uint64_t length)
{
- int i;
+ struct drm_device *dev = ppgtt->base.dev;
+ struct i915_pagedir *pd;
+ struct i915_pagetab *pt;
+ uint64_t temp, temp2;
+ uint32_t pdpe, pde;
+
+ gen8_for_each_pdpe(pd, &ppgtt->pdp, start, length, temp, pdpe) {
+ uint64_t pd_start = start;
+ uint64_t pd_len = gen8_bound_pt(start, length);
+ gen8_for_each_pde(pt, pd, pd_start, pd_len, temp2, pde) {
+ gen8_free_page_tables(pd, pd_start, pd_len, dev);
+ }
- for (i = 0; i < ppgtt->num_pd_pages; i++) {
- gen8_free_page_tables(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
- free_pd_single(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
+ free_pd_single(pd, dev);
}
}
+static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
+{
+ gen8_teardown_va_range(ppgtt, ppgtt->base.start, ppgtt->base.total);
+}
+
static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
{
struct i915_hw_ppgtt *ppgtt =
@@ -537,41 +558,75 @@ static int gen8_ppgtt_allocate_page_tables(struct
i915_hw_ppgtt *ppgtt)
unwind_out:
while (i--)
- gen8_free_page_tables(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
+ gen8_free_page_tables(ppgtt->pdp.pagedir[i],
+ i * I915_PDES_PER_PD * GEN8_PTES_PER_PT,
+ (i + 1)* I915_PDES_PER_PD *
GEN8_PTES_PER_PT,
+ ppgtt->base.dev);
return -ENOMEM;
}
static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
- const int max_pdp)
+ uint64_t start, uint64_t length)
{
- int i;
+ struct i915_pagedir *unused;
+ uint64_t temp;
+ uint32_t pdpe;
- for (i = 0; i < max_pdp; i++) {
- ppgtt->pdp.pagedir[i] = alloc_pd_single(ppgtt->base.dev);
- if (IS_ERR(ppgtt->pdp.pagedir[i]))
- goto unwind_out;
+ gen8_for_each_pdpe(unused, &ppgtt->pdp, start, length, temp, pdpe) {
+ struct i915_pagedir *pd;
+
+ BUG_ON(unused);
+ pd = alloc_pd_single(ppgtt->base.dev);
+ if (!pd)
+ goto pd_fail;
+
+ ppgtt->pdp.pagedir[pdpe] = pd;
+ ppgtt->num_pd_pages++;
}
- ppgtt->num_pd_pages = max_pdp;
BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS);
return 0;
-unwind_out:
- while (i--)
- free_pd_single(ppgtt->pdp.pagedir[i],
- ppgtt->base.dev);
+pd_fail:
+ while (pdpe--)
+ free_pd_single(ppgtt->pdp.pagedir[pdpe], ppgtt->base.dev);
return -ENOMEM;
}
+static void gen8_alloc_va_range(struct i915_hw_ppgtt *ppgtt,
+ uint64_t start, uint64_t length)
+{
+ struct i915_pagedir *pd;
+ struct i915_pagetab *pt;
+ uint64_t temp, temp2;
+ uint32_t pdpe, pde;
+
+ gen8_for_each_pdpe(pd, &ppgtt->pdp, start, length, temp, pdpe) {
+ uint64_t pd_start = start;
+ uint64_
[Intel-gfx] [PATCH 14/26] drm/i915: Complete page table structures
Move the remaining members over to the new page table structures.
This can be squashed with the previous commit if desire. The reasoning
is the same as that patch. I simply felt it is easier to review if split.
Signed-off-by: Ben Widawsky
Conflicts:
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem_gtt.c
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 85 +--
drivers/gpu/drm/i915/i915_gem_gtt.h | 15 +++
drivers/gpu/drm/i915/i915_gpu_error.c | 1 -
4 files changed, 38 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index b226788..5f3666a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1788,7 +1788,7 @@ static void gen8_ppgtt_info(struct seq_file *m, struct
drm_device *dev, int verb
static void print_ppgtt(struct seq_file *m, struct i915_hw_ppgtt *ppgtt, const
char *name)
{
seq_printf(m, "%s:\n", name);
- seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
+ seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd.pd_offset);
}
static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool
verbose)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5b283f2..d91a545 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -223,7 +223,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
int used_pd = ppgtt->num_pd_entries / I915_PDES_PER_PD;
for (i = used_pd - 1; i >= 0; i--) {
- dma_addr_t addr = ppgtt->pd_dma_addr[i];
+ dma_addr_t addr = ppgtt->pdp.pagedir[i].daddr;
ret = gen8_write_pdp(ring, i, addr, synchronous);
if (ret)
return ret;
@@ -341,7 +341,6 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
for (i = 0; i < ppgtt->num_pd_pages; i++) {
gen8_free_page_tables(&ppgtt->pdp.pagedir[i]);
gen8_free_page_directories(&ppgtt->pdp.pagedir[i]);
- kfree(ppgtt->gen8_pt_dma_addr[i]);
}
}
@@ -353,14 +352,14 @@ static void gen8_ppgtt_dma_unmap_pages(struct
i915_hw_ppgtt *ppgtt)
for (i = 0; i < ppgtt->num_pd_pages; i++) {
/* TODO: In the future we'll support sparse mappings, so this
* will have to change. */
- if (!ppgtt->pd_dma_addr[i])
+ if (!ppgtt->pdp.pagedir[i].daddr)
continue;
- pci_unmap_page(hwdev, ppgtt->pd_dma_addr[i], PAGE_SIZE,
+ pci_unmap_page(hwdev, ppgtt->pdp.pagedir[i].daddr, PAGE_SIZE,
PCI_DMA_BIDIRECTIONAL);
for (j = 0; j < I915_PDES_PER_PD; j++) {
- dma_addr_t addr = ppgtt->gen8_pt_dma_addr[i][j];
+ dma_addr_t addr =
ppgtt->pdp.pagedir[i].page_tables[j].daddr;
if (addr)
pci_unmap_page(hwdev, addr, PAGE_SIZE,
PCI_DMA_BIDIRECTIONAL);
@@ -380,31 +379,18 @@ static void gen8_ppgtt_cleanup(struct i915_address_space
*vm)
gen8_ppgtt_free(ppgtt);
}
-static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
-{
- int i;
-
- for (i = 0; i < ppgtt->num_pd_pages; i++) {
- ppgtt->gen8_pt_dma_addr[i] = kcalloc(I915_PDES_PER_PD,
-sizeof(dma_addr_t),
-GFP_KERNEL);
- if (!ppgtt->gen8_pt_dma_addr[i])
- return -ENOMEM;
- }
-
- return 0;
-}
-
static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt)
{
int i, j;
for (i = 0; i < ppgtt->num_pd_pages; i++) {
+ struct i915_pagedir *pd = &ppgtt->pdp.pagedir[i];
for (j = 0; j < I915_PDES_PER_PD; j++) {
- struct i915_pagetab *pt =
&ppgtt->pdp.pagedir[i].page_tables[j];
+ struct i915_pagetab *pt = &pd->page_tables[j];
pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO);
if (!pt->page)
goto unwind_out;
+
}
}
@@ -464,9 +450,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
ppgtt->num_pd_entries = max_pdp * I915_PDES_PER_PD;
- ret = gen8_ppgtt_allocate_dma(ppgtt);
- if (!ret)
- return ret;
+ return 0;
/* TODO: Check this for all cases */
err_out:
@@ -488,7 +472,7 @@ static int gen8_ppgtt_setup_page_directories(struct
i915_hw_ppgtt *ppgtt,
if (ret)
return ret;
- ppgtt->pd_dma_addr[pdpe] = pd_addr;
+ ppgtt->pdp.pagedir[pdpe].daddr =
[Intel-gfx] [PATCH 18/26] drm/i915: Always dma map page table allocations
There is never a case where we don't want to do it. Since we've broken
up the allocations into nice clean helper functions, it's both easy and
obvious to do the dma mapping at the same time.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 78 -
1 file changed, 17 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 92e03dd..9630109 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -187,20 +187,6 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
pci_unmap_page((dev)->pdev, (pt)->daddr, 4096, PCI_DMA_BIDIRECTIONAL); \
} while (0);
-
-static void dma_unmap_pt_range(struct i915_pagedir *pd,
- unsigned pde, size_t n,
- struct drm_device *dev)
-{
- if (WARN_ON(pde + n > I915_PDES_PER_PD))
- n = I915_PDES_PER_PD - pde;
-
- n += pde;
-
- for (; pde < n; pde++)
- dma_unmap_pt_single(pd->page_tables[pde], dev);
-}
-
/**
* dma_map_pt_single() - Create a dma mapping for a page table
* @pt:Page table to get a DMA map for
@@ -230,33 +216,12 @@ static int dma_map_pt_single(struct i915_pagetab *pt,
struct drm_device *dev)
return 0;
}
-static int dma_map_pt_range(struct i915_pagedir *pd,
- unsigned pde, size_t n,
- struct drm_device *dev)
-{
- const int first = pde;
-
- if (WARN_ON(pde + n > I915_PDES_PER_PD))
- n = I915_PDES_PER_PD - pde;
-
- n += pde;
-
- for (; pde < n; pde++) {
- int ret;
- ret = dma_map_pt_single(pd->page_tables[pde], dev);
- if (ret) {
- dma_unmap_pt_range(pd, first, pde, dev);
- return ret;
- }
- }
-
- return 0;
-}
-
-static void free_pt_single(struct i915_pagetab *pt)
+static void free_pt_single(struct i915_pagetab *pt, struct drm_device *dev)
{
if (WARN_ON(!pt->page))
return;
+
+ dma_unmap_pt_single(pt, dev);
__free_page(pt->page);
kfree(pt);
}
@@ -264,6 +229,7 @@ static void free_pt_single(struct i915_pagetab *pt)
static struct i915_pagetab *alloc_pt_single(struct drm_device *dev)
{
struct i915_pagetab *pt;
+ int ret;
pt = kzalloc(sizeof(*pt), GFP_KERNEL);
if (!pt)
@@ -275,6 +241,13 @@ static struct i915_pagetab *alloc_pt_single(struct
drm_device *dev)
return ERR_PTR(-ENOMEM);
}
+ ret = dma_map_pt_single(pt, dev);
+ if (ret) {
+ __free_page(pt->page);
+ kfree(pt);
+ return ERR_PTR(ret);
+ }
+
return pt;
}
@@ -318,7 +291,7 @@ static int alloc_pt_range(struct i915_pagedir *pd, uint16_t
pde, size_t count,
err_out:
while (i--)
- free_pt_single(pd->page_tables[i]);
+ free_pt_single(pd->page_tables[i], dev);
return ret;
}
@@ -486,7 +459,7 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
kunmap_atomic(pt_vaddr);
}
-static void gen8_free_page_tables(struct i915_pagedir *pd)
+static void gen8_free_page_tables(struct i915_pagedir *pd, struct drm_device
*dev)
{
int i;
@@ -494,7 +467,7 @@ static void gen8_free_page_tables(struct i915_pagedir *pd)
return;
for (i = 0; i < I915_PDES_PER_PD; i++) {
- free_pt_single(pd->page_tables[i]);
+ free_pt_single(pd->page_tables[i], dev);
pd->page_tables[i] = NULL;
}
}
@@ -504,7 +477,7 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
int i;
for (i = 0; i < ppgtt->num_pd_pages; i++) {
- gen8_free_page_tables(ppgtt->pdp.pagedir[i]);
+ gen8_free_page_tables(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
free_pd_single(ppgtt->pdp.pagedir[i]);
}
}
@@ -561,7 +534,7 @@ static int gen8_ppgtt_allocate_page_tables(struct
i915_hw_ppgtt *ppgtt)
unwind_out:
while (i--)
- gen8_free_page_tables(ppgtt->pdp.pagedir[i]);
+ gen8_free_page_tables(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
return -ENOMEM;
}
@@ -659,18 +632,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
* 2. Create DMA mappings for the page directories and page tables.
*/
for (i = 0; i < max_pdp; i++) {
- struct i915_pagedir *pd;
ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
if (ret)
goto bail;
-
- pd = ppgtt->pdp.pagedir[i];
-
- for (j = 0; j < I915_PDES_PER_PD; j++) {
- ret = dma_map_pt_single(pd->page_tables[j],
ppgtt->base.dev);
-
[Intel-gfx] [PATCH 21/26] drm/i915: Track GEN6 page table usage
Instead of implementing the full tracking + dynamic allocation, this
patch does a bit less than half of the work, by tracking and warning on
unexpected conditions. The tracking itself follows which PTEs within a
page table are currently being used for objects. The next patch will
modify this to actually allocate the page tables only when necessary.
With the current patch there isn't much in the way of making a gen
agnostic range allocation function. However, in the next patch we'll add
more specificity which makes having separate functions a bit easier to
manage.
Notice that aliasing PPGTT is not managed here. The patch which actually
begins dynamic allocation/teardown explains the reasoning forthis.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 170 +---
drivers/gpu/drm/i915/i915_gem_gtt.h | 117 +++--
2 files changed, 212 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ad2f2c5..d3c77d1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -53,9 +53,9 @@ bool intel_enable_ppgtt(struct drm_device *dev, bool full)
return HAS_ALIASING_PPGTT(dev);
}
-static void ppgtt_bind_vma(struct i915_vma *vma,
- enum i915_cache_level cache_level,
- u32 flags);
+static int ppgtt_bind_vma(struct i915_vma *vma,
+ enum i915_cache_level cache_level,
+ u32 flags);
static void ppgtt_unbind_vma(struct i915_vma *vma);
static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt);
@@ -204,39 +204,71 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
(px)->page, 0, 4096, \
PCI_DMA_BIDIRECTIONAL))
-static void free_pt_single(struct i915_pagetab *pt, struct drm_device *dev)
+static void __free_pt_single(struct i915_pagetab *pt, struct drm_device *dev,
+int scratch)
{
+ if (WARN(scratch ^ pt->scratch,
+"Tried to free scratch = %d. Is scratch = %d\n",
+scratch, pt->scratch))
+ return;
+
if (WARN_ON(!pt->page))
return;
+ if (!scratch) {
+ const size_t count = INTEL_INFO(dev)->gen >= 8 ?
+ GEN8_PTES_PER_PT : GEN6_PTES_PER_PT;
+ WARN(!bitmap_empty(pt->used_ptes, count),
+"Free page table with %d used pages\n",
+bitmap_weight(pt->used_ptes, count));
+ }
+
i915_dma_unmap_single(pt, dev);
__free_page(pt->page);
+ kfree(pt->used_ptes);
kfree(pt);
}
+#define free_pt_single(pt, dev) \
+ __free_pt_single(pt, dev, false)
+#define free_pt_scratch(pt, dev) \
+ __free_pt_single(pt, dev, true)
+
static struct i915_pagetab *alloc_pt_single(struct drm_device *dev)
{
struct i915_pagetab *pt;
- int ret;
+ const size_t count = INTEL_INFO(dev)->gen >= 8 ?
+ GEN8_PTES_PER_PT : GEN6_PTES_PER_PT;
+ int ret = -ENOMEM;
pt = kzalloc(sizeof(*pt), GFP_KERNEL);
if (!pt)
return ERR_PTR(-ENOMEM);
+ pt->used_ptes = kcalloc(BITS_TO_LONGS(count), sizeof(*pt->used_ptes),
+ GFP_KERNEL);
+
+ if (!pt->used_ptes)
+ goto fail_bitmap;
+
pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO);
- if (!pt->page) {
- kfree(pt);
- return ERR_PTR(-ENOMEM);
- }
+ if (!pt->page)
+ goto fail_page;
ret = i915_dma_map_px_single(pt, dev);
- if (ret) {
- __free_page(pt->page);
- kfree(pt);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto fail_dma;
return pt;
+
+fail_dma:
+ __free_page(pt->page);
+fail_page:
+ kfree(pt->used_ptes);
+fail_bitmap:
+ kfree(pt);
+
+ return ERR_PTR(ret);
}
/**
@@ -689,15 +721,13 @@ static void gen6_map_single(struct i915_pagedir *pd,
/* Map all the page tables found in the ppgtt structure to incrementing page
* directories. */
static void gen6_map_page_range(struct drm_i915_private *dev_priv,
- struct i915_pagedir *pd, unsigned pde, size_t n)
+ struct i915_pagedir *pd, uint32_t start,
uint32_t length)
{
- if (WARN_ON(pde + n > I915_PDES_PER_PD))
- n = I915_PDES_PER_PD - pde;
-
- n += pde;
+ struct i915_pagetab *pt;
+ uint32_t pde, temp;
- for (; pde < n; pde++)
- gen6_map_single(pd, pde, pd->page_tables[pde]);
+ gen6_for_each_pde(pt, pd, start, length, temp, pde)
+ gen6_map_single(pd, pde, pt);
/* Make sure write is complete
[Intel-gfx] [PATCH 17/26] drm/i915: Clean up pagetable DMA map & unmap
Map and unmap are common operations across all generations for
pagetables. With a simple helper, we can get a nice net code reduction
as well as simplified complexity.
There is some room for optimization here, for instance with the multiple
page mapping, that can be done in one pci_map operation. In that case
however, the max value we'll ever see there is 512, and so I believe the
simpler code makes this a worthwhile trade-off. Also, the range mapping
functions are place holders to help transition the code. Eventually,
mapping will only occur during a page allocation which will always be a
discrete operation.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 147 +---
1 file changed, 85 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 35acccb..92e03dd 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -183,6 +183,76 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
return pte;
}
+#define dma_unmap_pt_single(pt, dev) do { \
+ pci_unmap_page((dev)->pdev, (pt)->daddr, 4096, PCI_DMA_BIDIRECTIONAL); \
+} while (0);
+
+
+static void dma_unmap_pt_range(struct i915_pagedir *pd,
+ unsigned pde, size_t n,
+ struct drm_device *dev)
+{
+ if (WARN_ON(pde + n > I915_PDES_PER_PD))
+ n = I915_PDES_PER_PD - pde;
+
+ n += pde;
+
+ for (; pde < n; pde++)
+ dma_unmap_pt_single(pd->page_tables[pde], dev);
+}
+
+/**
+ * dma_map_pt_single() - Create a dma mapping for a page table
+ * @pt:Page table to get a DMA map for
+ * @dev: drm device
+ *
+ * Page table allocations are unified across all gens. They always require a
+ * single 4k allocation, as well as a DMA mapping.
+ *
+ * Return: 0 if success.
+ */
+static int dma_map_pt_single(struct i915_pagetab *pt, struct drm_device *dev)
+{
+ struct page *page;
+ dma_addr_t pt_addr;
+ int ret;
+
+ page = pt->page;
+ pt_addr = pci_map_page(dev->pdev, page, 0, 4096,
+ PCI_DMA_BIDIRECTIONAL);
+
+ ret = pci_dma_mapping_error(dev->pdev, pt_addr);
+ if (ret)
+ return ret;
+
+ pt->daddr = pt_addr;
+
+ return 0;
+}
+
+static int dma_map_pt_range(struct i915_pagedir *pd,
+ unsigned pde, size_t n,
+ struct drm_device *dev)
+{
+ const int first = pde;
+
+ if (WARN_ON(pde + n > I915_PDES_PER_PD))
+ n = I915_PDES_PER_PD - pde;
+
+ n += pde;
+
+ for (; pde < n; pde++) {
+ int ret;
+ ret = dma_map_pt_single(pd->page_tables[pde], dev);
+ if (ret) {
+ dma_unmap_pt_range(pd, first, pde, dev);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static void free_pt_single(struct i915_pagetab *pt)
{
if (WARN_ON(!pt->page))
@@ -191,7 +261,7 @@ static void free_pt_single(struct i915_pagetab *pt)
kfree(pt);
}
-static struct i915_pagetab *alloc_pt_single(void)
+static struct i915_pagetab *alloc_pt_single(struct drm_device *dev)
{
struct i915_pagetab *pt;
@@ -214,6 +284,7 @@ static struct i915_pagetab *alloc_pt_single(void)
* available to point to the allocated page tables.
* @pde: First page directory entry for which we are allocating.
* @count: Number of pages to allocate.
+ * @devDRM device used for DMA mapping.
*
* Allocates multiple page table pages and sets the appropriate entries in the
* page table structure within the page directory. Function cleans up after
@@ -221,7 +292,8 @@ static struct i915_pagetab *alloc_pt_single(void)
*
* Return: 0 if allocation succeeded.
*/
-static int alloc_pt_range(struct i915_pagedir *pd, uint16_t pde, size_t count)
+static int alloc_pt_range(struct i915_pagedir *pd, uint16_t pde, size_t count,
+ struct drm_device *dev)
{
int i, ret;
@@ -231,7 +303,7 @@ static int alloc_pt_range(struct i915_pagedir *pd, uint16_t
pde, size_t count)
BUG_ON(pde + count > I915_PDES_PER_PD);
for (i = pde; i < pde + count; i++) {
- struct i915_pagetab *pt = alloc_pt_single();
+ struct i915_pagetab *pt = alloc_pt_single(dev);
if (IS_ERR(pt)) {
ret = PTR_ERR(pt);
goto err_out;
@@ -480,7 +552,7 @@ static int gen8_ppgtt_allocate_page_tables(struct
i915_hw_ppgtt *ppgtt)
for (i = 0; i < ppgtt->num_pd_pages; i++) {
ret = alloc_pt_range(ppgtt->pdp.pagedir[i],
-0, I915_PDES_PER_PD);
+0, I915_PDES_PER_PD, ppgtt->base.dev);
if (ret)
g
[Intel-gfx] [PATCH 25/26] drm/i915: Print used ppgtt pages for gen6 in debugfs
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 5f3666a..04d40fa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1785,10 +1785,26 @@ static void gen8_ppgtt_info(struct seq_file *m, struct
drm_device *dev, int verb
}
}
+static size_t gen6_ppgtt_count_pt_pages(struct i915_hw_ppgtt *ppgtt)
+{
+ struct i915_pagedir *pd = &ppgtt->pd;
+ struct i915_pagetab **pt = &pd->page_tables[0];
+ size_t cnt = 0;
+ int i;
+
+ for (i = 0; i < ppgtt->num_pd_entries; i++) {
+ if (pt[i] != ppgtt->scratch_pt)
+ cnt++;
+ }
+
+ return cnt;
+}
+
static void print_ppgtt(struct seq_file *m, struct i915_hw_ppgtt *ppgtt, const
char *name)
{
seq_printf(m, "%s:\n", name);
seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd.pd_offset);
+ seq_printf(m, "\tpd pages: %zu\n", gen6_ppgtt_count_pt_pages(ppgtt));
}
static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool
verbose)
@@ -1809,6 +1825,8 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev, bool ver
seq_printf(m, "PP_DIR_BASE_READ: 0x%08x\n",
I915_READ(RING_PP_DIR_BASE_READ(ring)));
seq_printf(m, "PP_DIR_DCLV: 0x%08x\n",
I915_READ(RING_PP_DIR_DCLV(ring)));
}
+ seq_printf(m, "ECOCHK: 0x%08x\n\n", I915_READ(GAM_ECOCHK));
+
if (dev_priv->mm.aliasing_ppgtt) {
struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
@@ -1829,7 +1847,6 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev, bool ver
if (verbose)
idr_for_each(&file_priv->context_idr, per_file_ctx, m);
}
- seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
}
static int i915_ppgtt_info(struct seq_file *m, void *data)
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/26] drm/i915: Consolidate dma mappings
With a little bit of macro magic, and the fact that every page
table/dir/etc. we wish to map will have a page, and daddr member, we can
greatly simplify and reduce code.
The patch introduces an i915_dma_map/unmap which has the same semantics
as pci_map_page, but is 1 line, and doesn't require newlines, or local
variables to make it fit cleanly.
Notice that even the page allocation shares this same attribute. For
now, I am leaving that code untouched because the macro version would be
a bit on the big side - but it's a nice cleanup as well (IMO)
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 56 -
1 file changed, 18 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9630109..abef33dd 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -183,45 +183,33 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
return pte;
}
-#define dma_unmap_pt_single(pt, dev) do { \
- pci_unmap_page((dev)->pdev, (pt)->daddr, 4096, PCI_DMA_BIDIRECTIONAL); \
+#define i915_dma_unmap_single(px, dev) do { \
+ pci_unmap_page((dev)->pdev, (px)->daddr, 4096, PCI_DMA_BIDIRECTIONAL); \
} while (0);
/**
- * dma_map_pt_single() - Create a dma mapping for a page table
- * @pt:Page table to get a DMA map for
+ * i915_dma_map_px_single() - Create a dma mapping for a page table/dir/etc.
+ * @px:Page table/dir/etc to get a DMA map for
* @dev: drm device
*
* Page table allocations are unified across all gens. They always require a
- * single 4k allocation, as well as a DMA mapping.
+ * single 4k allocation, as well as a DMA mapping. If we keep the structs
+ * symmetric here, the simple macro covers us for every page table type.
*
* Return: 0 if success.
*/
-static int dma_map_pt_single(struct i915_pagetab *pt, struct drm_device *dev)
-{
- struct page *page;
- dma_addr_t pt_addr;
- int ret;
-
- page = pt->page;
- pt_addr = pci_map_page(dev->pdev, page, 0, 4096,
- PCI_DMA_BIDIRECTIONAL);
-
- ret = pci_dma_mapping_error(dev->pdev, pt_addr);
- if (ret)
- return ret;
-
- pt->daddr = pt_addr;
-
- return 0;
-}
+#define i915_dma_map_px_single(px, dev) \
+ pci_dma_mapping_error((dev)->pdev, \
+ (px)->daddr = pci_map_page((dev)->pdev, \
+(px)->page, 0, 4096, \
+PCI_DMA_BIDIRECTIONAL))
static void free_pt_single(struct i915_pagetab *pt, struct drm_device *dev)
{
if (WARN_ON(!pt->page))
return;
- dma_unmap_pt_single(pt, dev);
+ i915_dma_unmap_single(pt, dev);
__free_page(pt->page);
kfree(pt);
}
@@ -241,7 +229,7 @@ static struct i915_pagetab *alloc_pt_single(struct
drm_device *dev)
return ERR_PTR(-ENOMEM);
}
- ret = dma_map_pt_single(pt, dev);
+ ret = i915_dma_map_px_single(pt, dev);
if (ret) {
__free_page(pt->page);
kfree(pt);
@@ -484,7 +472,7 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
static void gen8_ppgtt_dma_unmap_pages(struct i915_hw_ppgtt *ppgtt)
{
- struct pci_dev *hwdev = ppgtt->base.dev->pdev;
+ struct drm_device *dev = ppgtt->base.dev;
int i, j;
for (i = 0; i < ppgtt->num_pd_pages; i++) {
@@ -493,16 +481,14 @@ static void gen8_ppgtt_dma_unmap_pages(struct
i915_hw_ppgtt *ppgtt)
if (!ppgtt->pdp.pagedir[i]->daddr)
continue;
- pci_unmap_page(hwdev, ppgtt->pdp.pagedir[i]->daddr, PAGE_SIZE,
- PCI_DMA_BIDIRECTIONAL);
+ i915_dma_unmap_single(ppgtt->pdp.pagedir[i], dev);
for (j = 0; j < I915_PDES_PER_PD; j++) {
struct i915_pagedir *pd = ppgtt->pdp.pagedir[i];
struct i915_pagetab *pt = pd->page_tables[j];
dma_addr_t addr = pt->daddr;
if (addr)
- pci_unmap_page(hwdev, addr, PAGE_SIZE,
- PCI_DMA_BIDIRECTIONAL);
+ i915_dma_unmap_single(pt, dev);
}
}
}
@@ -588,19 +574,13 @@ err_out:
static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
const int pdpe)
{
- dma_addr_t pd_addr;
int ret;
- pd_addr = pci_map_page(ppgtt->base.dev->pdev,
- ppgtt->pdp.pagedir[pdpe]->page, 0,
- PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
-
- ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr);
+ ret = i915_dma_map
[Intel-gfx] [PATCH 05/26] drm/i915: Setup less PPGTT on failed pagedir
The current code will both potentially print a WARN, and setup part of
the PPGTT structure. Neither of these harm the current code, it is
simply for clarity, and to perhaps prevent later bugs, or weird
debug messages.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 08a1e1c..09556d1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1085,11 +1085,14 @@ alloc:
goto alloc;
}
+ if (ret)
+ return ret;
+
if (ppgtt->node.start < dev_priv->gtt.mappable_end)
DRM_DEBUG("Forced to use aperture for PDEs\n");
ppgtt->num_pd_entries = GEN6_PPGTT_PD_ENTRIES;
- return ret;
+ return 0;
}
static int gen6_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt)
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/26] drm/i915: Split out gtt specific header file
TODO: Do header files need a copyright?
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 162 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 57 -
drivers/gpu/drm/i915/i915_gem_gtt.h | 225
3 files changed, 227 insertions(+), 217 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_gem_gtt.h
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 084e82f..b19442c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -44,6 +44,8 @@
#include
#include
+#include "i915_gem_gtt.h"
+
/* General customization:
*/
@@ -572,166 +574,6 @@ enum i915_cache_level {
I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
};
-typedef uint32_t gen6_gtt_pte_t;
-
-/**
- * A VMA represents a GEM BO that is bound into an address space. Therefore, a
- * VMA's presence cannot be guaranteed before binding, or after unbinding the
- * object into/from the address space.
- *
- * To make things as simple as possible (ie. no refcounting), a VMA's lifetime
- * will always be <= an objects lifetime. So object refcounting should cover
us.
- */
-struct i915_vma {
- struct drm_mm_node node;
- struct drm_i915_gem_object *obj;
- struct i915_address_space *vm;
-
- /** This object's place on the active/inactive lists */
- struct list_head mm_list;
-
- struct list_head vma_link; /* Link in the object's VMA list */
-
- /** This vma's place in the batchbuffer or on the eviction list */
- struct list_head exec_list;
-
- /**
-* Used for performing relocations during execbuffer insertion.
-*/
- struct hlist_node exec_node;
- unsigned long exec_handle;
- struct drm_i915_gem_exec_object2 *exec_entry;
-
- /**
-* How many users have pinned this object in GTT space. The following
-* users can each hold at most one reference: pwrite/pread, pin_ioctl
-* (via user_pin_count), execbuffer (objects are not allowed multiple
-* times for the same batchbuffer), and the framebuffer code. When
-* switching/pageflipping, the framebuffer code has at most two buffers
-* pinned per crtc.
-*
-* In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3
-* bits with absolutely no headroom. So use 4 bits. */
- unsigned int pin_count:4;
-#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf
-
- /** Unmap an object from an address space. This usually consists of
-* setting the valid PTE entries to a reserved scratch page. */
- void (*unbind_vma)(struct i915_vma *vma);
- /* Map an object into an address space with the given cache flags. */
-#define GLOBAL_BIND (1<<0)
- void (*bind_vma)(struct i915_vma *vma,
-enum i915_cache_level cache_level,
-u32 flags);
-};
-
-struct i915_address_space {
- struct drm_mm mm;
- struct drm_device *dev;
- struct list_head global_link;
- unsigned long start;/* Start offset always 0 for dri2 */
- size_t total; /* size addr space maps (ex. 2GB for ggtt) */
-
- struct {
- dma_addr_t addr;
- struct page *page;
- } scratch;
-
- /**
-* List of objects currently involved in rendering.
-*
-* Includes buffers having the contents of their GPU caches
-* flushed, not necessarily primitives. last_rendering_seqno
-* represents when the rendering involved will be completed.
-*
-* A reference is held on the buffer while on this list.
-*/
- struct list_head active_list;
-
- /**
-* LRU list of objects which are not in the ringbuffer and
-* are ready to unbind, but are still in the GTT.
-*
-* last_rendering_seqno is 0 while an object is in this list.
-*
-* A reference is not held on the buffer while on this list,
-* as merely being GTT-bound shouldn't prevent its being
-* freed, and we'll pull it off the list in the free path.
-*/
- struct list_head inactive_list;
-
- /* FIXME: Need a more generic return type */
- gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
-enum i915_cache_level level,
-bool valid); /* Create a valid PTE */
- void (*clear_range)(struct i915_address_space *vm,
- uint64_t start,
- uint64_t length,
- bool use_scratch);
- void (*insert_entries)(struct i915_address_space *vm,
- struct sg_table *st,
- uint64_t start,
- enum i915_cache_level cache_level);
- void (*cleanup)(struct i915_address_space *vm);
-};
-
-/* Th
[Intel-gfx] [PATCH 10/26] drm/i915: Make gen6_write_pdes gen6_map_page_tables
Split out single mappings which will help with upcoming work. Also while
here, rename the function because it is a better description - but this
function is going away soon.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 39 ++---
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a239196..d89054d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -655,26 +655,33 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt,
struct seq_file *m)
}
}
-static void gen6_write_pdes(struct i915_hw_ppgtt *ppgtt)
+static void gen6_map_single(struct i915_hw_ppgtt *ppgtt,
+ const unsigned pde_index,
+ dma_addr_t daddr)
{
struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
- gen6_gtt_pte_t __iomem *pd_addr;
uint32_t pd_entry;
+ gen6_gtt_pte_t __iomem *pd_addr =
+ (gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm + ppgtt->pd_offset /
sizeof(gen6_gtt_pte_t);
+
+ pd_entry = GEN6_PDE_ADDR_ENCODE(daddr);
+ pd_entry |= GEN6_PDE_VALID;
+
+ writel(pd_entry, pd_addr + pde_index);
+}
+
+/* Map all the page tables found in the ppgtt structure to incrementing page
+ * directories. */
+static void gen6_map_page_tables(struct i915_hw_ppgtt *ppgtt)
+{
+ struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
int i;
WARN_ON(ppgtt->pd_offset & 0x3f);
- pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv->gtt.gsm +
- ppgtt->pd_offset / sizeof(gen6_gtt_pte_t);
- for (i = 0; i < ppgtt->num_pd_entries; i++) {
- dma_addr_t pt_addr;
-
- pt_addr = ppgtt->pt_dma_addr[i];
- pd_entry = GEN6_PDE_ADDR_ENCODE(pt_addr);
- pd_entry |= GEN6_PDE_VALID;
+ for (i = 0; i < ppgtt->num_pd_entries; i++)
+ gen6_map_single(ppgtt, i, ppgtt->pt_dma_addr[i]);
- writel(pd_entry, pd_addr + i);
- }
- readl(pd_addr);
+ readl(dev_priv->gtt.gsm);
}
static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt)
@@ -1145,7 +1152,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt->pd_offset =
ppgtt->node.start / PAGE_SIZE * sizeof(gen6_gtt_pte_t);
- gen6_write_pdes(ppgtt);
+ gen6_map_page_tables(ppgtt);
ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
@@ -1319,11 +1326,11 @@ void i915_gem_restore_gtt_mappings(struct drm_device
*dev)
/* TODO: Perhaps it shouldn't be gen6 specific */
if (i915_is_ggtt(vm)) {
if (dev_priv->mm.aliasing_ppgtt)
- gen6_write_pdes(dev_priv->mm.aliasing_ppgtt);
+
gen6_map_page_tables(dev_priv->mm.aliasing_ppgtt);
continue;
}
- gen6_write_pdes(container_of(vm, struct i915_hw_ppgtt, base));
+ gen6_map_page_tables(container_of(vm, struct i915_hw_ppgtt,
base));
}
i915_gem_chipset_flush(dev);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/26] drm/i915: rename map/unmap to dma_map/unmap
Upcoming patches will use the terms map and unmap in references to the
page table entries. Having this distinction will really help with code
clarity at that point.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b26b186..08a1e1c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -394,7 +394,7 @@ static void gen8_ppgtt_free(const struct i915_hw_ppgtt
*ppgtt)
__free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages <<
PAGE_SHIFT));
}
-static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
+static void gen8_ppgtt_dma_unmap_pages(struct i915_hw_ppgtt *ppgtt)
{
struct pci_dev *hwdev = ppgtt->base.dev->pdev;
int i, j;
@@ -425,7 +425,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space
*vm)
list_del(&vm->global_link);
drm_mm_takedown(&vm->mm);
- gen8_ppgtt_unmap_pages(ppgtt);
+ gen8_ppgtt_dma_unmap_pages(ppgtt);
gen8_ppgtt_free(ppgtt);
}
@@ -651,7 +651,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
return 0;
bail:
- gen8_ppgtt_unmap_pages(ppgtt);
+ gen8_ppgtt_dma_unmap_pages(ppgtt);
gen8_ppgtt_free(ppgtt);
return ret;
}
@@ -1019,7 +1019,7 @@ static void gen6_ppgtt_insert_entries(struct
i915_address_space *vm,
kunmap_atomic(pt_vaddr);
}
-static void gen6_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
+static void gen6_ppgtt_dma_unmap_pages(struct i915_hw_ppgtt *ppgtt)
{
int i;
@@ -1050,7 +1050,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space
*vm)
drm_mm_takedown(&ppgtt->base.mm);
drm_mm_remove_node(&ppgtt->node);
- gen6_ppgtt_unmap_pages(ppgtt);
+ gen6_ppgtt_dma_unmap_pages(ppgtt);
gen6_ppgtt_free(ppgtt);
}
@@ -1150,7 +1150,7 @@ static int gen6_ppgtt_setup_page_tables(struct
i915_hw_ppgtt *ppgtt)
PCI_DMA_BIDIRECTIONAL);
if (pci_dma_mapping_error(dev->pdev, pt_addr)) {
- gen6_ppgtt_unmap_pages(ppgtt);
+ gen6_ppgtt_dma_unmap_pages(ppgtt);
return -EIO;
}
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/26] drm/i915: s/pd/pdpe, s/pt/pde
The actual correct way to think about this with the new style of page
table data structures is as the actual entry that is being indexed into
the array. "pd", and "pt" aren't representative of what the operation is
doing.
The clarity here will improve the readability of future patches.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bd016e2..b26b186 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -537,40 +537,40 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
}
static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
-const int pd)
+const int pdpe)
{
dma_addr_t pd_addr;
int ret;
pd_addr = pci_map_page(ppgtt->base.dev->pdev,
- &ppgtt->pd_pages[pd], 0,
+ &ppgtt->pd_pages[pdpe], 0,
PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr);
if (ret)
return ret;
- ppgtt->pd_dma_addr[pd] = pd_addr;
+ ppgtt->pd_dma_addr[pdpe] = pd_addr;
return 0;
}
static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
- const int pd,
- const int pt)
+ const int pdpe,
+ const int pde)
{
dma_addr_t pt_addr;
struct page *p;
int ret;
- p = ppgtt->gen8_pt_pages[pd][pt];
+ p = ppgtt->gen8_pt_pages[pdpe][pde];
pt_addr = pci_map_page(ppgtt->base.dev->pdev,
p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
if (ret)
return ret;
- ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
+ ppgtt->gen8_pt_dma_addr[pdpe][pde] = pt_addr;
return 0;
}
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 20/26] drm/i915: Always dma map page directory allocations
Similar to the patch a few back in the series, we can always map and
unmap page directories when we do their allocation and teardown. Page
directory pages only exist on gen8+, so this should only effect behavior
on those platforms.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 79 +
1 file changed, 19 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index abef33dd..ad2f2c5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -283,21 +283,23 @@ err_out:
return ret;
}
-static void __free_pd_single(struct i915_pagedir *pd)
+static void __free_pd_single(struct i915_pagedir *pd, struct drm_device *dev)
{
+ i915_dma_unmap_single(pd, dev);
__free_page(pd->page);
kfree(pd);
}
-#define free_pd_single(pd) do { \
+#define free_pd_single(pd, dev) do { \
if ((pd)->page) { \
- __free_pd_single(pd); \
+ __free_pd_single(pd, dev); \
} \
} while (0)
-static struct i915_pagedir *alloc_pd_single(void)
+static struct i915_pagedir *alloc_pd_single(struct drm_device *dev)
{
struct i915_pagedir *pd;
+ int ret;
pd = kzalloc(sizeof(*pd), GFP_KERNEL);
if (!pd)
@@ -309,6 +311,13 @@ static struct i915_pagedir *alloc_pd_single(void)
return ERR_PTR(-ENOMEM);
}
+ ret = i915_dma_map_px_single(pd, dev);
+ if (ret) {
+ __free_page(pd->page);
+ kfree(pd);
+ return ERR_PTR(ret);
+ }
+
return pd;
}
@@ -466,30 +475,7 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
for (i = 0; i < ppgtt->num_pd_pages; i++) {
gen8_free_page_tables(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
- free_pd_single(ppgtt->pdp.pagedir[i]);
- }
-}
-
-static void gen8_ppgtt_dma_unmap_pages(struct i915_hw_ppgtt *ppgtt)
-{
- struct drm_device *dev = ppgtt->base.dev;
- int i, j;
-
- for (i = 0; i < ppgtt->num_pd_pages; i++) {
- /* TODO: In the future we'll support sparse mappings, so this
-* will have to change. */
- if (!ppgtt->pdp.pagedir[i]->daddr)
- continue;
-
- i915_dma_unmap_single(ppgtt->pdp.pagedir[i], dev);
-
- for (j = 0; j < I915_PDES_PER_PD; j++) {
- struct i915_pagedir *pd = ppgtt->pdp.pagedir[i];
- struct i915_pagetab *pt = pd->page_tables[j];
- dma_addr_t addr = pt->daddr;
- if (addr)
- i915_dma_unmap_single(pt, dev);
- }
+ free_pd_single(ppgtt->pdp.pagedir[i], ppgtt->base.dev);
}
}
@@ -501,7 +487,6 @@ static void gen8_ppgtt_cleanup(struct i915_address_space
*vm)
list_del(&vm->global_link);
drm_mm_takedown(&vm->mm);
- gen8_ppgtt_dma_unmap_pages(ppgtt);
gen8_ppgtt_free(ppgtt);
}
@@ -531,7 +516,7 @@ static int gen8_ppgtt_allocate_page_directories(struct
i915_hw_ppgtt *ppgtt,
int i;
for (i = 0; i < max_pdp; i++) {
- ppgtt->pdp.pagedir[i] = alloc_pd_single();
+ ppgtt->pdp.pagedir[i] = alloc_pd_single(ppgtt->base.dev);
if (IS_ERR(ppgtt->pdp.pagedir[i]))
goto unwind_out;
}
@@ -543,7 +528,8 @@ static int gen8_ppgtt_allocate_page_directories(struct
i915_hw_ppgtt *ppgtt,
unwind_out:
while (i--)
- free_pd_single(ppgtt->pdp.pagedir[i]);
+ free_pd_single(ppgtt->pdp.pagedir[i],
+ ppgtt->base.dev);
return -ENOMEM;
}
@@ -571,19 +557,6 @@ err_out:
return ret;
}
-static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
-const int pdpe)
-{
- int ret;
-
- ret = i915_dma_map_px_single(ppgtt->pdp.pagedir[pdpe],
-ppgtt->base.dev);
- if (ret)
- return ret;
-
- return 0;
-}
-
/**
* GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers
* with a net effect resembling a 2-level page table in normal x86 terms. Each
@@ -609,16 +582,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
return ret;
/*
-* 2. Create DMA mappings for the page directories and page tables.
-*/
- for (i = 0; i < max_pdp; i++) {
- ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
- if (ret)
- goto bail;
- }
-
- /*
-* 3. Map all the page directory entires to point to the page tables
+* 2. Map all the page directory entires to point to the page tables
* we've allocated.
*
[Intel-gfx] [PATCH 00/26] [RFCish] GEN7 dynamic page tables
These patches live here, based on my temporary Broadwell branch: http://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=dynamic_pt_alloc First, and most importantly, this work should have no impact on current drm-intel code because PPGTT is currently shut off there. To actually test this patch series, one must re-enable PPGTT. On a single run of IGT on IVB, it seem this doesn't introduce any regressions, but y'know, it's PPGTT, so there's some instability, and it's hard to claim for certain this doesn't break anything on top. Also, as stated below, the gen8 work is only partially done. Before I go too much further with this, I wanted to get eyes on it. I am really open to any feedback. Before you do request a change though, please realize that I've gone through several iterations of the functions/interfaces. So please, spare me some pain and try to think through what your request is before rattling it off. Daniel has expressed to me already that he is unwilling to merge certain things until PPGTT problems are fixed, and that can be enabled by default. That's okay. In my opinion, many of the patches don't really have any major behavioral changes, and only make the code so much more readable and easy to deal with, that I believe merging it would only improve PPGTT debugging in the future. There are several cleanups in the series which could also go in relatively harmlessly. Okay, so what does this do? The patch series /dynamicizes/ page table allocation and teardown for GEN7. It also starts to introduce GEN8, but the tricky stuff is still not done. Up until now, all our page tables are pre-allocated when the address space is created. That's actually okay for current GENs since we don't use many address spaces, and the page tables occupy only 2MB each. However, on GEN8 we can use a deeper page table, and to preallocate such an address space would be very costly. This work was done for GEN7 first because this is the most well tested with full PPGTT, and stable platforms are readily available. In this patch series, I've demonstrated how we will manage tracking used page tables (bitmaps), and broken things out into much more discrete functions. I'm hoping I'll get feedback on the way I've implemented things (primarily if it seems fundamentally flawed in any way). The real goal was to prove out the dynamic allocation so we can begin to enable GEN8 in the same way. I'll emphasize now that I put in a lot of effort limit risk with each patch, and this does result in some excess churn. My next step is bring GEN8 up to par with GEN7. Once GEN8 is working, and clean we can find where GEN7, and GEN8 overlap, and then recombine where I haven't done so already. It's possible this plan will not work out, and the above 2 steps will end up as one. After that, I plan to merge the VA range allocation, and teardown into the insert/clear entries (currently it's two steps). I think both of those steps should be distinct. On x86 code overlap: I spent more time that I would have liked trying to conjoin our pagetable management with x86 code. In the end I decided not to depend on any of the x86 definitions (other than PAGE_SIZE) because I found the maze of conditional compiles and defines a bit too cumbersome. I also didn't feel the abstract pagetable topology used in x86 code was worthwhile given that with about 6 #defines, we achieve the same thing. We just don't support nearly as many configurations, and our page table format differs in too many places. One thing I had really considered, and toyed around with was not having data structures to track the page tables we've allocated and simply use the one that's in memory (which is what x86 does). I was not able to make this work because of IOMMU. The address we write into our page tables is an IOMMU address. This means we need to know, or be able to easily derive both the physical address (or pfn, or struct page), and the DMA address. I failed to accomplish this. I think using the bitmaps should be a fast way than having to kmap the pagetables to determine their status anyway. And, one thing to keep in mind is currently we don't have any GPU faulting capability. This will greatly limit the ability to map things sparsely, which also will greatly limit the effective virtual address space we can use. Ben Widawsky (26): drm/i915: Split out verbose PPGTT dumping drm/i915: Extract switch to default context drm/i915: s/pd/pdpe, s/pt/pde drm/i915: rename map/unmap to dma_map/unmap drm/i915: Setup less PPGTT on failed pagedir drm/i915: Wrap VMA binding drm/i915: clean up PPGTT init error path drm/i915: Un-hardcode number of page directories drm/i915: Split out gtt specific header file drm/i915: Make gen6_write_pdes gen6_map_page_tables drm/i915: Range clearing is PPGTT agnostic drm/i915: Page table helpers, and define renames drm/i915: construct page table abstractions drm/i915: Complete page table structures drm/i915: Create page table allocators dr
[Intel-gfx] [PATCH 07/26] drm/i915: clean up PPGTT init error path
The old code (I'm having trouble finding the commit) had a reason for
doing things when there was an error, and would continue on, thus the
!ret. For the newer code however, this looks completely silly.
Follow the normal idiom of if (ret) return ret.
Also, put the pde wiring in the gen specific init, now that GEN8 exists.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1620211..5f73284 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1202,6 +1202,8 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt->pd_offset =
ppgtt->node.start / PAGE_SIZE * sizeof(gen6_gtt_pte_t);
+ gen6_write_pdes(ppgtt);
+
ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
DRM_DEBUG_DRIVER("Allocated pde space (%ldM) at GTT entry: %lx\n",
@@ -1226,20 +1228,14 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
else
BUG();
- if (!ret) {
- struct drm_i915_private *dev_priv = dev->dev_private;
- kref_init(&ppgtt->ref);
- drm_mm_init(&ppgtt->base.mm, ppgtt->base.start,
- ppgtt->base.total);
- i915_init_vm(dev_priv, &ppgtt->base);
- if (INTEL_INFO(dev)->gen < 8) {
- gen6_write_pdes(ppgtt);
- DRM_DEBUG("Adding PPGTT at offset %x\n",
- ppgtt->pd_offset << 10);
- }
- }
+ if (ret)
+ return ret;
- return ret;
+ kref_init(&ppgtt->ref);
+ drm_mm_init(&ppgtt->base.mm, ppgtt->base.start, ppgtt->base.total);
+ i915_init_vm(dev_priv, &ppgtt->base);
+
+ return 0;
}
static void
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/26] drm/i915: Create page table allocators
As we move toward dynamic page table allocation, it becomes much easier
to manage our data structures if break do things less coarsely by
breaking up all of our actions into individual tasks. This makes the
code easier to write, read, and verify.
Aside from the dissection of the allocation functions, the patch
statically allocates the page table structures without a page directory.
This remains the same for all platforms,
The patch itself should not have much functional difference. The primary
noticeable difference is the fact that page tables are no longer
allocated, but rather statically declared as part of the page directory.
This has non-zero overhead, but things gain non-trivial complexity as a
result.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 226 +++-
drivers/gpu/drm/i915/i915_gem_gtt.h | 4 +-
2 files changed, 147 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d91a545..5c08cf9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -183,6 +183,102 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
return pte;
}
+static void free_pt_single(struct i915_pagetab *pt)
+{
+ if (WARN_ON(!pt->page))
+ return;
+ __free_page(pt->page);
+ kfree(pt);
+}
+
+static struct i915_pagetab *alloc_pt_single(void)
+{
+ struct i915_pagetab *pt;
+
+ pt = kzalloc(sizeof(*pt), GFP_KERNEL);
+ if (!pt)
+ return ERR_PTR(-ENOMEM);
+
+ pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!pt->page) {
+ kfree(pt);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return pt;
+}
+
+/**
+ * alloc_pt_range() - Allocate a multiple page tables
+ * @pd:The page directory which will have at least @count
entries
+ * available to point to the allocated page tables.
+ * @pde: First page directory entry for which we are allocating.
+ * @count: Number of pages to allocate.
+ *
+ * Allocates multiple page table pages and sets the appropriate entries in the
+ * page table structure within the page directory. Function cleans up after
+ * itself on any failures.
+ *
+ * Return: 0 if allocation succeeded.
+ */
+static int alloc_pt_range(struct i915_pagedir *pd, uint16_t pde, size_t count)
+{
+ int i, ret;
+
+ /* 512 is the max page tables per pagedir on any platform.
+* TODO: make WARN after patch series is done
+*/
+ BUG_ON(pde + count > I915_PDES_PER_PD);
+
+ for (i = pde; i < pde + count; i++) {
+ struct i915_pagetab *pt = alloc_pt_single();
+ if (IS_ERR(pt)) {
+ ret = PTR_ERR(pt);
+ goto err_out;
+ }
+ WARN(pd->page_tables[i],
+"Leaking page directory entry %d (%pa)\n",
+i, pd->page_tables[i]);
+ pd->page_tables[i] = pt;
+ }
+
+ return 0;
+
+err_out:
+ while (i--)
+ free_pt_single(pd->page_tables[i]);
+ return ret;
+}
+
+static void __free_pd_single(struct i915_pagedir *pd)
+{
+ __free_page(pd->page);
+ kfree(pd);
+}
+
+#define free_pd_single(pd) do { \
+ if ((pd)->page) { \
+ __free_pd_single(pd); \
+ } \
+} while (0)
+
+static struct i915_pagedir *alloc_pd_single(void)
+{
+ struct i915_pagedir *pd;
+
+ pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+ if (!pd)
+ return ERR_PTR(-ENOMEM);
+
+ pd->page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!pd->page) {
+ kfree(pd);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return pd;
+}
+
/* Broadwell Page Directory Pointer Descriptors */
static int gen8_write_pdp(struct intel_ring_buffer *ring, unsigned entry,
uint64_t val, bool synchronous)
@@ -223,7 +319,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
int used_pd = ppgtt->num_pd_entries / I915_PDES_PER_PD;
for (i = used_pd - 1; i >= 0; i--) {
- dma_addr_t addr = ppgtt->pdp.pagedir[i].daddr;
+ dma_addr_t addr = ppgtt->pdp.pagedir[i]->daddr;
ret = gen8_write_pdp(ring, i, addr, synchronous);
if (ret)
return ret;
@@ -250,8 +346,9 @@ static void gen8_ppgtt_clear_range(struct
i915_address_space *vm,
I915_CACHE_LLC, use_scratch);
while (num_entries) {
- struct i915_pagedir *pd = &ppgtt->pdp.pagedir[pdpe];
- struct page *page_table = pd->page_tables[pde].page;
+ struct i915_pagedir *pd = ppgtt->pdp.pagedir[pdpe];
+ struct i915_pagetab *pt = pd->page_tables[pde];
+ struct page *page_table = pt->page;
last_pte = pte
[Intel-gfx] [PATCH 12/26] drm/i915: Page table helpers, and define renames
These page table helpers make the code much cleaner. There is some
room to use the arch/x86 header files. The reason I've opted not to is
in several cases, the definitions are dictated by the CONFIG_ options
which do not always indicate the restrictions in the GPU. While here,
clean up the defines to have more concise names, and consolidate between
gen6 and gen8 where appropriate.
I've made a lot of tiny errors in these helpers. Often I'd correct an
error only to introduce another one. While IGT was capable of catching
them, the tests often took a while to catch, and where hard/slow to
debug in the kernel. As a result, to test this, I compiled
i915_gem_gtt.h in userspace, and ran tests from userspace. What follows
isn't by any means complete, but it was able to catch lot of bugs. Gen8
is also untested, but since the current code is almost identical, I feel
pretty comfortable with that.
void test_pte(uint32_t base) {
uint32_t ret;
assert_pte_index((base + 0), 0);
assert_pte_index((base + 1), 0);
assert_pte_index((base + 0x1000), 1);
assert_pte_index((base + (1<<22)), 0);
assert_pte_index((base + ((1<<22) - 1)), 1023);
assert_pte_index((base + (1<<21)), 512);
assert_pte_count(base + 0, 0, 0);
assert_pte_count(base + 0, 1, 1);
assert_pte_count(base + 0, 0x1000, 1);
assert_pte_count(base + 0, 0x1001, 2);
assert_pte_count(base + 0, 1<<21, 512);
assert_pte_count(base + 0, 1<<22, 1024);
assert_pte_count(base + 0, (1<<22) - 1, 1024);
assert_pte_count(base + (1<<21), 1<<22, 512);
assert_pte_count(base + (1<<21), (1<<22)+1, 512);
assert_pte_count(base + (1<<21), 10<<22, 512);
}
void test_pde(uint32_t base) {
assert(gen6_pde_index(base + 0) == 0);
assert(gen6_pde_index(base + 1) == 0);
assert(gen6_pde_index(base + (1<<21)) == 0);
assert(gen6_pde_index(base + (1<<22)) == 1);
assert(gen6_pde_index(base + ((256<<22)))== 256);
assert(gen6_pde_index(base + ((512<<22))) == 0);
assert(gen6_pde_index(base + ((513<<22))) == 1); /* This is
actually not possible on gen6 */
assert(gen6_pde_count(base + 0, 0) == 0);
assert(gen6_pde_count(base + 0, 1) == 1);
assert(gen6_pde_count(base + 0, 1<<21) == 1);
assert(gen6_pde_count(base + 0, 1<<22) == 1);
assert(gen6_pde_count(base + 0, (1<<22) + 0x1000) == 2);
assert(gen6_pde_count(base + 0x1000, 1<<22) == 2);
assert(gen6_pde_count(base + 0, 511<<22) == 511);
assert(gen6_pde_count(base + 0, 512<<22) == 512);
assert(gen6_pde_count(base + 0x1000, 512<<22) == 512);
assert(gen6_pde_count(base + (1<<22), 512<<22) == 511);
}
int main()
{
test_pde(0);
while (1)
test_pte(rand() & ~((1<<22) - 1));
return 0;
}
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 90 +-
drivers/gpu/drm/i915/i915_gem_gtt.h | 125 ++--
2 files changed, 162 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 77556ac..7afa5f4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -220,7 +220,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
int i, ret;
/* bit of a hack to find the actual last used pd */
- int used_pd = ppgtt->num_pd_entries / GEN8_PDES_PER_PAGE;
+ int used_pd = ppgtt->num_pd_entries / I915_PDES_PER_PD;
for (i = used_pd - 1; i >= 0; i--) {
dma_addr_t addr = ppgtt->pd_dma_addr[i];
@@ -240,9 +240,9 @@ static void gen8_ppgtt_clear_range(struct
i915_address_space *vm,
struct i915_hw_ppgtt *ppgtt =
container_of(vm, struct i915_hw_ppgtt, base);
gen8_gtt_pte_t *pt_vaddr, scratch_pte;
- unsigned pdpe = start >> GEN8_PDPE_SHIFT & GEN8_PDPE_MASK;
- unsigned pde = start >> GEN8_PDE_SHIFT & GEN8_PDE_MASK;
- unsigned pte = start >> GEN8_PTE_SHIFT & GEN8_PTE_MASK;
+ unsigned pdpe = gen8_pdpe_index(start);
+ unsigned pde = gen8_pde_index(start);
+ unsigned pte = gen8_pte_index(start);
unsigned num_entries = length >> PAGE_SHIFT;
unsigned last_pte, i;
@@ -253,8 +253,8 @@ static void gen8_ppgtt_clear_range(struct
i915_address_space *vm,
struct page *page_table = ppgtt->gen8_pt_pages[pdpe][pde];
last_pte = pte + num_entries;
- if (last_pte > GEN8_PTES_PER_PAGE)
- last_pte = GEN8_PTES_PER_PAGE;
+ if (last_pte > GEN8_PTES_PER_PT)
+ last_pte = GEN8_PTES_PER_PT;
pt_vaddr = kmap_atomic(page_table);
@@ -266,7 +266,7 @@ static void gen8_ppgtt_clear_range(struct
i915_address_space *vm,
kunmap_atomic(pt_vaddr);
pte = 0;
[Intel-gfx] [PATCH 13/26] drm/i915: construct page table abstractions
Thus far we've opted to make complex code requiring difficult review. In
the future, the code is only going to become more complex, and as such
we'll take the hit now and start to encapsulate things.
To help transition the code nicely there is some wasted space in gen6/7.
This will be ameliorated shortly.
NOTE: The pun in the subject was intentional.
Signed-off-by: Ben Widawsky
Conflicts:
drivers/gpu/drm/i915/i915_drv.h
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 175 ++--
drivers/gpu/drm/i915/i915_gem_gtt.h | 24 +++--
2 files changed, 104 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7afa5f4..5b283f2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -250,7 +250,8 @@ static void gen8_ppgtt_clear_range(struct
i915_address_space *vm,
I915_CACHE_LLC, use_scratch);
while (num_entries) {
- struct page *page_table = ppgtt->gen8_pt_pages[pdpe][pde];
+ struct i915_pagedir *pd = &ppgtt->pdp.pagedir[pdpe];
+ struct page *page_table = pd->page_tables[pde].page;
last_pte = pte + num_entries;
if (last_pte > GEN8_PTES_PER_PT)
@@ -292,8 +293,11 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
if (WARN_ON(pdpe >= GEN8_LEGACY_PDPS))
break;
- if (pt_vaddr == NULL)
- pt_vaddr = kmap_atomic(ppgtt->gen8_pt_pages[pdpe][pde]);
+ if (pt_vaddr == NULL) {
+ struct i915_pagedir *pd = &ppgtt->pdp.pagedir[pdpe];
+ struct page *page_table = pd->page_tables[pde].page;
+ pt_vaddr = kmap_atomic(page_table);
+ }
pt_vaddr[pte] =
gen8_pte_encode(sg_page_iter_dma_address(&sg_iter),
@@ -312,29 +316,33 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
kunmap_atomic(pt_vaddr);
}
-static void gen8_free_page_tables(struct page **pt_pages)
+static void gen8_free_page_tables(struct i915_pagedir *pd)
{
int i;
- if (pt_pages == NULL)
+ if (pd->page_tables == NULL)
return;
for (i = 0; i < I915_PDES_PER_PD; i++)
- if (pt_pages[i])
- __free_pages(pt_pages[i], 0);
+ if (pd->page_tables[i].page)
+ __free_page(pd->page_tables[i].page);
+}
+
+static void gen8_free_page_directories(struct i915_pagedir *pd)
+{
+ kfree(pd->page_tables);
+ __free_page(pd->page);
}
-static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt)
+static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
{
int i;
for (i = 0; i < ppgtt->num_pd_pages; i++) {
- gen8_free_page_tables(ppgtt->gen8_pt_pages[i]);
- kfree(ppgtt->gen8_pt_pages[i]);
+ gen8_free_page_tables(&ppgtt->pdp.pagedir[i]);
+ gen8_free_page_directories(&ppgtt->pdp.pagedir[i]);
kfree(ppgtt->gen8_pt_dma_addr[i]);
}
-
- __free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages <<
PAGE_SHIFT));
}
static void gen8_ppgtt_dma_unmap_pages(struct i915_hw_ppgtt *ppgtt)
@@ -372,87 +380,73 @@ static void gen8_ppgtt_cleanup(struct i915_address_space
*vm)
gen8_ppgtt_free(ppgtt);
}
-static struct page **__gen8_alloc_page_tables(void)
+static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
{
- struct page **pt_pages;
int i;
- pt_pages = kcalloc(I915_PDES_PER_PD, sizeof(struct page *), GFP_KERNEL);
- if (!pt_pages)
- return ERR_PTR(-ENOMEM);
-
- for (i = 0; i < I915_PDES_PER_PD; i++) {
- pt_pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
- if (!pt_pages[i])
- goto bail;
+ for (i = 0; i < ppgtt->num_pd_pages; i++) {
+ ppgtt->gen8_pt_dma_addr[i] = kcalloc(I915_PDES_PER_PD,
+sizeof(dma_addr_t),
+GFP_KERNEL);
+ if (!ppgtt->gen8_pt_dma_addr[i])
+ return -ENOMEM;
}
- return pt_pages;
-
-bail:
- gen8_free_page_tables(pt_pages);
- kfree(pt_pages);
- return ERR_PTR(-ENOMEM);
+ return 0;
}
-static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt,
- const int max_pdp)
+static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt)
{
- struct page **pt_pages[GEN8_LEGACY_PDPS];
- int i, ret;
+ int i, j;
- for (i = 0; i < max_pdp; i++) {
- pt_pages[i] = __gen8_alloc_page_tables();
- if (IS_ERR(pt_pages[i])) {
[Intel-gfx] [PATCH 06/26] drm/i915: Wrap VMA binding
This will be useful for some upcoming patches which do more platform
specific work. Having it in one central place just makes things a bit
cleaner and easier.
There is a small functional change here. There are more calls to the
tracepoints.
NOTE: I didn't actually end up using this patch for the intended purpose, but I
thought it was a nice patch to keep around.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h| 3 +++
drivers/gpu/drm/i915/i915_gem.c| 8
drivers/gpu/drm/i915/i915_gem_context.c| 2 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +++--
drivers/gpu/drm/i915/i915_gem_gtt.c| 16 ++--
5 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c59b707..b3e31fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2408,6 +2408,9 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
struct i915_address_space *vm);
unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
struct i915_address_space *vm);
+void i915_gem_bind_vma(struct i915_vma *vma, enum i915_cache_level,
+ unsigned flags);
+void i915_gem_unbind_vma(struct i915_vma *vma);
struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
struct i915_address_space *vm);
struct i915_vma *
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ed09dda..0a3f4ac 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2765,7 +2765,7 @@ int i915_vma_unbind(struct i915_vma *vma)
trace_i915_vma_unbind(vma);
- vma->unbind_vma(vma);
+ i915_gem_unbind_vma(vma);
i915_gem_gtt_finish_object(obj);
@@ -3514,8 +3514,8 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
list_for_each_entry(vma, &obj->vma_list, vma_link)
if (drm_mm_node_allocated(&vma->node))
- vma->bind_vma(vma, cache_level,
- obj->has_global_gtt_mapping ?
GLOBAL_BIND : 0);
+ i915_gem_bind_vma(vma, cache_level,
+ obj->has_global_gtt_mapping ?
GLOBAL_BIND : 0);
}
list_for_each_entry(vma, &obj->vma_list, vma_link)
@@ -3878,7 +3878,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
}
if (flags & PIN_GLOBAL && !obj->has_global_gtt_mapping)
- vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND);
+ i915_gem_bind_vma(vma, obj->cache_level, GLOBAL_BIND);
vma->pin_count++;
if (flags & PIN_MAPPABLE)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 7dfdc02..f918f2c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -693,7 +693,7 @@ static int do_switch(struct intel_ring_buffer *ring,
if (!to->obj->has_global_gtt_mapping) {
struct i915_vma *vma = i915_gem_obj_to_vma(to->obj,
&dev_priv->gtt.base);
- vma->bind_vma(vma, to->obj->cache_level, GLOBAL_BIND);
+ i915_gem_bind_vma(vma, to->obj->cache_level, GLOBAL_BIND);
}
if (!to->is_initialized || i915_gem_context_is_default(to))
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3851a1b..856fa9d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -369,7 +369,8 @@ i915_gem_execbuffer_relocate_entry(struct
drm_i915_gem_object *obj,
struct i915_vma *vma =
list_first_entry(&target_i915_obj->vma_list,
typeof(*vma), vma_link);
- vma->bind_vma(vma, target_i915_obj->cache_level, GLOBAL_BIND);
+ i915_gem_bind_vma(vma, target_i915_obj->cache_level,
+ GLOBAL_BIND);
}
/* Validate that the target is in a valid r/w GPU domain */
@@ -1209,7 +1210,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
* allocate space first */
struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
BUG_ON(!vma);
- vma->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND);
+ i915_gem_bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND);
}
if (flags & I915_DISPATCH_SECURE)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09556d1..1620211 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1370,10 +1
[Intel-gfx] [PATCH 02/26] drm/i915: Extract switch to default context
This patch existed for another reason which no longer exists. I liked
it, so I kept it in the series. It can skipped if undesirable.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 35f9a37..c59b707 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2476,6 +2476,8 @@ int i915_gem_context_enable(struct drm_i915_private
*dev_priv);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct intel_ring_buffer *ring,
struct drm_file *file, struct i915_hw_context *to);
+#define i915_switch_to_default(ring) \
+ i915_switch_context(ring, NULL, ring->default_context)
struct i915_hw_context *
i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
void i915_gem_context_free(struct kref *ctx_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b2565d2..ed09dda 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2799,7 +2799,7 @@ int i915_gpu_idle(struct drm_device *dev)
/* Flush everything onto the inactive list. */
for_each_ring(ring, dev_priv, i) {
- ret = i915_switch_context(ring, NULL, ring->default_context);
+ ret = i915_switch_to_default(ring);
if (ret)
return ret;
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/26] drm/i915: Un-hardcode number of page directories
trivial.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b3e31fd..084e82f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -722,7 +722,7 @@ struct i915_hw_ppgtt {
};
union {
dma_addr_t *pt_dma_addr;
- dma_addr_t *gen8_pt_dma_addr[4];
+ dma_addr_t *gen8_pt_dma_addr[GEN8_LEGACY_PDPS];
};
int (*enable)(struct i915_hw_ppgtt *ppgtt);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/26] drm/i915: Split out verbose PPGTT dumping
There often is not enough memory to dump the full contents of the PPGTT.
As a temporary bandage, to continue getting valuable basic PPGTT info,
wrap the dangerous, memory hungry part inside of a new verbose version
of the debugfs file.
Also while here we can split out the ppgtt print function so it's more
reusable.
I'd really like to get ppgtt info into our error state, but I found it too
difficult to make work in the limited time I have. Maybe Mika can find a way.
Cc: Mika Kuoppala
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_debugfs.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 1031c43..b226788 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1760,7 +1760,7 @@ static int per_file_ctx(int id, void *ptr, void *data)
return 0;
}
-static void gen8_ppgtt_info(struct seq_file *m, struct drm_device *dev)
+static void gen8_ppgtt_info(struct seq_file *m, struct drm_device *dev, int
verbose)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_ring_buffer *ring;
@@ -1785,7 +1785,13 @@ static void gen8_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
}
}
-static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
+static void print_ppgtt(struct seq_file *m, struct i915_hw_ppgtt *ppgtt, const
char *name)
+{
+ seq_printf(m, "%s:\n", name);
+ seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
+}
+
+static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool
verbose)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_ring_buffer *ring;
@@ -1806,10 +1812,9 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
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_offset);
-
- ppgtt->debug_dump(ppgtt, m);
+ print_ppgtt(m, ppgtt, "Aliasing PPGTT");
+ if (verbose)
+ ppgtt->debug_dump(ppgtt, m);
} else
return;
@@ -1820,8 +1825,9 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
pvt_ppgtt = ctx_to_ppgtt(file_priv->private_default_ctx);
seq_printf(m, "proc: %s\n",
get_pid_task(file->pid, PIDTYPE_PID)->comm);
- seq_puts(m, " default context:\n");
- idr_for_each(&file_priv->context_idr, per_file_ctx, m);
+ print_ppgtt(m, pvt_ppgtt, "Default context");
+ if (verbose)
+ idr_for_each(&file_priv->context_idr, per_file_ctx, m);
}
seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
}
@@ -1831,6 +1837,7 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ bool verbose = node->info_ent->data ? true : false;
int ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
@@ -1838,9 +1845,9 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
intel_runtime_pm_get(dev_priv);
if (INTEL_INFO(dev)->gen >= 8)
- gen8_ppgtt_info(m, dev);
+ gen8_ppgtt_info(m, dev, verbose);
else if (INTEL_INFO(dev)->gen >= 6)
- gen6_ppgtt_info(m, dev);
+ gen6_ppgtt_info(m, dev, verbose);
intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
@@ -3826,6 +3833,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0},
{"i915_swizzle_info", i915_swizzle_info, 0},
{"i915_ppgtt_info", i915_ppgtt_info, 0},
+ {"i915_ppgtt_verbose_info", i915_ppgtt_info, 0, (void *)1},
{"i915_dpio", i915_dpio_info, 0},
{"i915_llc", i915_llc, 0},
{"i915_edp_psr_status", i915_edp_psr_status, 0},
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/26] drm/i915: Range clearing is PPGTT agnostic
Therefore we can do it from our general init function. Eventually, I
hope to have a lot more commonality like this. It won't arrive yet, but
this was a nice easy one.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d89054d..77556ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -584,8 +584,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
ppgtt->base.start = 0;
ppgtt->base.total = ppgtt->num_pd_entries * GEN8_PTES_PER_PAGE *
PAGE_SIZE;
- ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
-
DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d
wasted)\n",
ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp);
DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld wasted)\n",
@@ -1154,8 +1152,6 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
gen6_map_page_tables(ppgtt);
- ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
-
DRM_DEBUG_DRIVER("Allocated pde space (%ldM) at GTT entry: %lx\n",
ppgtt->node.size >> 20,
ppgtt->node.start / PAGE_SIZE);
@@ -1183,6 +1179,7 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
kref_init(&ppgtt->ref);
drm_mm_init(&ppgtt->base.mm, ppgtt->base.start, ppgtt->base.total);
+ ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
i915_init_vm(dev_priv, &ppgtt->base);
return 0;
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] prime_mmap: Add new test for calling mmap() on dma-buf fds
From: Rob Bradford
This test has the following subtests:
- test_correct for correctness of the data
- test_map_unmap checks for mapping idempotency
- test_reprime checks for dma-buf creation idempotency
- test_forked checks for multiprocess access
- test_refcounting checks for buffer reference counting
- test_dup chats that dup()ing the fd works
- test_errors checks the error return values for failures
- test_aperture_limit tests multiple buffer creation at the gtt aperture
limit
Signed-off-by: Rob Bradford
---
tests/Makefile.sources | 1 +
tests/prime_mmap.c | 384 +
2 files changed, 385 insertions(+)
create mode 100644 tests/prime_mmap.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 88866ac..d666381 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -65,6 +65,7 @@ TESTS_progs_M = \
pm_lpsp \
pm_pc8 \
pm_rps \
+ prime_mmap \
prime_self_import \
template \
$(NULL)
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
new file mode 100644
index 000..94ba191
--- /dev/null
+++ b/tests/prime_mmap.c
@@ -0,0 +1,384 @@
+
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Rob Bradford
+ *
+ */
+
+/*
+ * Testcase: Check whether mmap()ing dma-buf works
+ */
+#define _GNU_SOURCE
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "drm.h"
+#include "i915_drm.h"
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "ioctl_wrappers.h"
+
+#define BO_SIZE (16*1024)
+
+static int fd;
+
+char pattern[] = {0xff, 0x00, 0x00, 0x00,
+ 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x00, 0xff};
+
+static void
+fill_bo(uint32_t handle, size_t size)
+{
+ off_t i;
+ for (i = 0; i < size; i+=sizeof(pattern))
+ {
+ gem_write(fd, handle, i, pattern, sizeof(pattern));
+ }
+}
+
+static int
+pattern_check(char *ptr, size_t size)
+{
+ off_t i;
+ for (i = 0; i < size; i+=sizeof(pattern))
+ {
+ if (memcmp(ptr, pattern, sizeof(pattern)) != 0)
+ return 1;
+ }
+
+ return 0;
+}
+
+static void
+test_correct(void)
+{
+ int dma_buf_fd;
+ char *ptr1, *ptr2;
+ uint32_t handle;
+
+ handle = gem_create(fd, BO_SIZE);
+ fill_bo(handle, BO_SIZE);
+
+ dma_buf_fd = prime_handle_to_fd(fd, handle);
+ igt_assert(errno == 0);
+
+ /* Check correctness vs GEM_MMAP_GTT */
+ ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ | PROT_WRITE);
+ ptr2 = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+ igt_assert(ptr1 != MAP_FAILED);
+ igt_assert(ptr2 != MAP_FAILED);
+ igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
+
+ /* Check pattern correctness */
+ igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+
+ munmap(ptr1, BO_SIZE);
+ munmap(ptr2, BO_SIZE);
+ close(dma_buf_fd);
+ gem_close(fd, handle);
+}
+
+static void
+test_map_unmap(void)
+{
+ int dma_buf_fd;
+ char *ptr;
+ uint32_t handle;
+
+ handle = gem_create(fd, BO_SIZE);
+ fill_bo(handle, BO_SIZE);
+
+ dma_buf_fd = prime_handle_to_fd(fd, handle);
+ igt_assert(errno == 0);
+
+ ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+ igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+ /* Unmap and remap */
+ munmap(ptr, BO_SIZE);
+ ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+ igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+ munmap(ptr, BO_SIZE);
+ close(dma_buf_fd);
+ gem_close(fd, h
[Intel-gfx] [PATCH] drm/i915: Rename intel_setup_wm_latency() to ilk_setup_wm_latency()
This function is only used on ILK+, so rename it accordingly.
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6dbaf66..47bf433 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2085,7 +2085,7 @@ static void intel_print_wm_latency(struct drm_device *dev,
}
}
-static void intel_setup_wm_latency(struct drm_device *dev)
+static void ilk_setup_wm_latency(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5990,7 +5990,7 @@ void intel_init_pm(struct drm_device *dev)
/* For FIFO watermark updates */
if (HAS_PCH_SPLIT(dev)) {
- intel_setup_wm_latency(dev);
+ ilk_setup_wm_latency(dev);
if ((IS_GEN5(dev) && dev_priv->wm.pri_latency[1] &&
dev_priv->wm.spr_latency[1] &&
dev_priv->wm.cur_latency[1]) ||
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Remove spurious '()' in WARN macros
No need of any here.
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_display.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d34add5..c6743f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1166,7 +1166,7 @@ static void assert_planes_disabled(struct
drm_i915_private *dev_priv,
if (INTEL_INFO(dev)->gen >= 4) {
reg = DSPCNTR(pipe);
val = I915_READ(reg);
- WARN((val & DISPLAY_PLANE_ENABLE),
+ WARN(val & DISPLAY_PLANE_ENABLE,
"plane %c assertion failure, should be disabled but not\n",
plane_name(pipe));
return;
@@ -1195,20 +1195,20 @@ static void assert_sprites_disabled(struct
drm_i915_private *dev_priv,
for_each_sprite(pipe, sprite) {
reg = SPCNTR(pipe, sprite);
val = I915_READ(reg);
- WARN((val & SP_ENABLE),
+ WARN(val & SP_ENABLE,
"sprite %c assertion failure, should be off on
pipe %c but is still active\n",
sprite_name(pipe, sprite), pipe_name(pipe));
}
} else if (INTEL_INFO(dev)->gen >= 7) {
reg = SPRCTL(pipe);
val = I915_READ(reg);
- WARN((val & SPRITE_ENABLE),
+ WARN(val & SPRITE_ENABLE,
"sprite %c assertion failure, should be off on pipe %c but
is still active\n",
plane_name(pipe), pipe_name(pipe));
} else if (INTEL_INFO(dev)->gen >= 5) {
reg = DVSCNTR(pipe);
val = I915_READ(reg);
- WARN((val & DVS_ENABLE),
+ WARN(val & DVS_ENABLE,
"sprite %c assertion failure, should be off on pipe %c but
is still active\n",
plane_name(pipe), pipe_name(pipe));
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Give the sprite width to the WM computation
In the future, we'll need the height of the fb we're fetching from
memory for WM computation.
At some point in the future, it'd be nice to give a pointer to a
mystical plane_config structure instead of chaplet of parameters.
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_drv.h| 5 -
drivers/gpu/drm/i915/intel_pm.c | 17 +++--
drivers/gpu/drm/i915/intel_sprite.c | 15 +--
4 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 70fbe90..057873e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -438,8 +438,8 @@ struct drm_i915_display_funcs {
void (*update_wm)(struct drm_crtc *crtc);
void (*update_sprite_wm)(struct drm_plane *plane,
struct drm_crtc *crtc,
-uint32_t sprite_width, int pixel_size,
-bool enable, bool scaled);
+uint32_t sprite_width, uint32_t sprite_height,
+int pixel_size, bool enable, bool scaled);
void (*modeset_global_resources)(struct drm_device *dev);
/* Returns the active state of the crtc, and if the crtc is active,
* fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e0064a1..8e6f971 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -392,6 +392,7 @@ struct intel_crtc {
struct intel_plane_wm_parameters {
uint32_t horiz_pixels;
+ uint32_t vert_pixels;
uint8_t bytes_per_pixel;
bool enabled;
bool scaled;
@@ -873,7 +874,9 @@ void intel_suspend_hw(struct drm_device *dev);
void intel_update_watermarks(struct drm_crtc *crtc);
void intel_update_sprite_watermarks(struct drm_plane *plane,
struct drm_crtc *crtc,
- uint32_t sprite_width, int pixel_size,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
bool enabled, bool scaled);
void intel_init_pm(struct drm_device *dev);
void intel_pm_setup(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ad58ce3..6dbaf66 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2579,10 +2579,11 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, &results);
}
-static void ilk_update_sprite_wm(struct drm_plane *plane,
-struct drm_crtc *crtc,
-uint32_t sprite_width, int pixel_size,
-bool enabled, bool scaled)
+static void
+ilk_update_sprite_wm(struct drm_plane *plane,
+struct drm_crtc *crtc,
+uint32_t sprite_width, uint32_t sprite_height,
+int pixel_size, bool enabled, bool scaled)
{
struct drm_device *dev = plane->dev;
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -2590,6 +2591,7 @@ static void ilk_update_sprite_wm(struct drm_plane *plane,
intel_plane->wm.enabled = enabled;
intel_plane->wm.scaled = scaled;
intel_plane->wm.horiz_pixels = sprite_width;
+ intel_plane->wm.vert_pixels = sprite_width;
intel_plane->wm.bytes_per_pixel = pixel_size;
/*
@@ -2720,13 +2722,16 @@ void intel_update_watermarks(struct drm_crtc *crtc)
void intel_update_sprite_watermarks(struct drm_plane *plane,
struct drm_crtc *crtc,
- uint32_t sprite_width, int pixel_size,
+ uint32_t sprite_width,
+ uint32_t sprite_height,
+ int pixel_size,
bool enabled, bool scaled)
{
struct drm_i915_private *dev_priv = plane->dev->dev_private;
if (dev_priv->display.update_sprite_wm)
- dev_priv->display.update_sprite_wm(plane, crtc, sprite_width,
+ dev_priv->display.update_sprite_wm(plane, crtc,
+ sprite_width, sprite_height,
pixel_size, enabled, scaled);
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..844bccd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -115,7 +115,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
sprctl |= SP_ENABLE;
- intel_upda
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
Jani Nikula writes: > Here's two patches to try, separately, *without* the quirk: [...] Okay, I've tried both and neither works. However, I've noticed that while the backlight level never changes even though I'm changing it with Fn+F4/Fn+F5, it *does* get registered somewhere because if I bring it all the day down and reboot, the brightness is reduced in the next BIOS POST (loading i915 gets it back to 100%). Thanks, -r ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests/gem_reset_stats: run non hw context tests also on older gens
To gain more coverage on interface, default context and banning.
As there is no proper reset support for gen <= 3, we only
do limited interface testing on those.
Signed-off-by: Mika Kuoppala
---
tests/gem_reset_stats.c | 91 ++-
1 file changed, 75 insertions(+), 16 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 3719f40..2bb4681 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -51,6 +51,9 @@
#define RS_BATCH_PENDING (1 << 1)
#define RS_UNKNOWN (1 << 2)
+static uint32_t devid;
+static bool hw_contexts;
+
struct local_drm_i915_reset_stats {
__u32 ctx_id;
__u32 flags;
@@ -101,6 +104,9 @@ static const struct target_ring {
static bool has_context(const struct target_ring *ring)
{
+ if (!hw_contexts)
+ return false;
+
if(ring->exec == I915_EXEC_RENDER)
return true;
@@ -277,7 +283,7 @@ static int inject_hang_ring(int fd, int ctx, int ring)
srandom(time(NULL));
- if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+ if (intel_gen(devid) >= 8)
cmd_len = 3;
buf = malloc(BUFSIZE);
@@ -960,7 +966,7 @@ static int _test_params(int fd, int ctx, uint32_t flags,
uint32_t pad)
typedef enum { root = 0, user } cap_t;
-static void test_param_ctx(const int fd, const int ctx, const cap_t cap)
+static void _check_param_ctx(const int fd, const int ctx, const cap_t cap)
{
const uint32_t bad = rand() + 1;
@@ -981,8 +987,7 @@ static void check_params(const int fd, const int ctx, cap_t
cap)
igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
- test_param_ctx(fd, 0, cap);
- test_param_ctx(fd, ctx, cap);
+ _check_param_ctx(fd, ctx, cap);
}
static void _test_param(const int fd, const int ctx)
@@ -1002,7 +1007,7 @@ static void _test_param(const int fd, const int ctx)
igt_waitchildren();
}
-static void test_params(void)
+static void test_params_ctx(void)
{
int fd, ctx;
@@ -1015,29 +1020,76 @@ static void test_params(void)
close(fd);
}
-#define RING_HAS_CONTEXTS current_ring->contexts(current_ring)
-#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false);
__VA_ARGS__; } while (0)
+static void test_params(void)
+{
+ int fd;
-int fd;
+ fd = drm_open_any();
+ igt_assert(fd >= 0);
-igt_main
+ _test_param(fd, 0);
+
+ close(fd);
+
+}
+
+static bool gem_has_hw_contexts(int fd)
{
struct local_drm_i915_gem_context_create create;
- uint32_t devid;
int ret;
+ memset(&create, 0, sizeof(create));
+ ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
+
+ if (ret == 0) {
+ drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &create);
+ return true;
+ }
+
+ return false;
+}
+
+static bool gem_has_reset_stats(int fd)
+{
+ struct local_drm_i915_reset_stats rs;
+ int ret;
+
+ /* Carefully set flags and pad to zero, otherwise
+ we get -EINVAL
+ */
+ memset(&rs, 0, sizeof(rs));
+
+ ret = drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs);
+ if (ret == 0)
+ return true;
+
+ /* If we get EPERM, we have support but did not
+ have CAP_SYSADM */
+ if (ret == -1 && errno == EPERM)
+ return true;
+
+ return false;
+}
+
+#define RING_HAS_CONTEXTS (current_ring->contexts(current_ring))
+#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false);
__VA_ARGS__; } while (0)
+
+static int fd;
+
+igt_main
+{
igt_skip_on_simulation();
igt_fixture {
+ bool has_reset_stats;
fd = drm_open_any();
devid = intel_get_drm_devid(fd);
- igt_require_f(intel_gen(devid) >= 4,
- "Architecture %d too old\n", intel_gen(devid));
- ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
- igt_skip_on_f(ret != 0 && (errno == ENODEV || errno == EINVAL),
- "Kernel is too old, or contexts not supported:
%s\n",
- strerror(errno));
+ hw_contexts = gem_has_hw_contexts(fd);
+ has_reset_stats = gem_has_reset_stats(fd);
+
+ igt_require_f(has_reset_stats,
+ "No reset stats ioctl support. Too old
kernel?\n");
}
igt_subtest("params")
@@ -1052,6 +1104,13 @@ igt_main
igt_fixture
gem_require_ring(fd, current_ring->exec);
+ igt_fixture
+ igt_require_f(intel_gen(devid) >= 4,
+ "gen %d doesn't support reset\n",
intel_gen(devid));
+
+ igt_subtest_f("params-ctx-%s", name)
+ RUN_CTX_TEST(test_params_ctx());
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
Jani Nikula writes: > Please file a new bug on DRM/Intel component at [1]. Please attach > intel_reg_dumper output (tool from intel-gpu-tools [2]) with and without > the revert. Please also attach dmesg with drm.debug=0xe from early > boot. Done, https://bugs.freedesktop.org/show_bug.cgi?id=76276 Let me know if you need more info. > Do you have the snb or ivb model? IVB. > The register dump from *before* i915 gets loaded is also interesting. That's more difficult to procure, is it really necessary? Thanks, -r ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
Hi Daniel, Daniel Vetter writes: > Please never just send a mail to maintainers, always cc mailing lists. Yes, sorry, I sent this privately because it wasn't a fully-formed bug report; I just wanted Jani to confirm that it wasn't intentionally broken as of -rc7. > Have you tested with the patch reverted to confirm that it's indeed > this commit and not something else (e.g. in the acpi code)? I hadn't, but I've now reverted the revert and things work again as in v3.13. So as far as I can tell the quirk is still definitely needed. Thanks! -r ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: finish off reverting eDP VDD changes
On Mon, Mar 17, 2014 at 04:43:36PM +0200, Jani Nikula wrote:
> This is a small follow-up fix to the series of eDP VDD back and forth
> we've had recently. This is effectively a combined revert of three
> commits:
>
> commit 2c2894f698fffd8ff53e1e1d3834f9e1035b1f39
> Author: Paulo Zanoni
> Date: Fri Mar 7 20:05:20 2014 -0300
>
> drm/i915: properly disable the VDD when disabling the panel
>
> commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
> Author: Patrik Jakobsson
> Date: Tue Mar 4 00:42:44 2014 +0100
>
> drm/i915: Don't just say it, actually force edp vdd
>
> commit dff392dbd258381a6c3164f38420593f2d291e3b
> Author: Paulo Zanoni
> Date: Fri Dec 6 17:32:41 2013 -0200
>
> drm/i915: don't touch the VDD when disabling the panel
>
> which shows that we're pretty close back to where we started
> already. The first two were basically reverting the last, but missing
> the WARN. Add that back. We also OCD the intel_ prefix back to
> intel_edp_panel_vdd_on() which was lost somewhere in between. The circle
> closes.
>
> For future reference, "drm/i915: don't touch the VDD when disabling the
> panel" failed to take into account
>
> commit 6cb49835da0426f69a2931bc2a0a8156344b0e41
> Author: Daniel Vetter
> Date: Sun May 20 17:14:50 2012 +0200
>
> drm/i915: enable vdd when switching off the eDP panel
>
> and
>
> commit 35a38556d900b9cb5dfa2529c93944b847f8a8a4
> Author: Daniel Vetter
> Date: Sun Aug 12 22:17:14 2012 +0200
>
> drm/i915: reorder edp disabling to fix ivb MacBook Air
>
> Cc: Patrik Jakobsson
> Cc: Paulo Zanoni
> Signed-off-by: Jani Nikula
5 commit citations and we still need more. I think that's a new highscore
;-)
Thanks for doing this detailed revert, queued for -next, thanks for the patch.
And an ack on the relevant revert for -fixes ofc.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_ddi.c |2 +-
> drivers/gpu/drm/i915/intel_dp.c | 14 --
> drivers/gpu/drm/i915/intel_drv.h |2 +-
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index fe1f5f012c4f..070bf2e78d61 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1340,7 +1340,7 @@ static void intel_ddi_post_disable(struct intel_encoder
> *intel_encoder)
> if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> - edp_panel_vdd_on(intel_dp);
> + intel_edp_panel_vdd_on(intel_dp);
> intel_edp_panel_off(intel_dp);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7854cdbb462a..071d44fe2fc8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -676,7 +676,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> int reply_bytes;
> int ret;
>
> - edp_panel_vdd_on(intel_dp);
> + intel_edp_panel_vdd_on(intel_dp);
> intel_dp_check_edp(intel_dp);
> /* Set up the command byte */
> if (mode & MODE_I2C_READ)
> @@ -1161,7 +1161,7 @@ static u32 ironlake_get_pp_control(struct intel_dp
> *intel_dp)
> return control;
> }
>
> -void edp_panel_vdd_on(struct intel_dp *intel_dp)
> +void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1329,6 +1329,8 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
>
> edp_wait_backlight_off(intel_dp);
>
> + WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
> +
> pp = ironlake_get_pp_control(intel_dp);
> /* We need to switch off panel power _and_ force vdd, for otherwise some
>* panels get very unhappy and cease to work. */
> @@ -1880,7 +1882,7 @@ static void intel_disable_dp(struct intel_encoder
> *encoder)
>
> /* Make sure the panel is off before trying to change the mode. But also
>* ensure that we have vdd while we switch off the panel. */
> - edp_panel_vdd_on(intel_dp);
> + intel_edp_panel_vdd_on(intel_dp);
> intel_edp_backlight_off(intel_dp);
> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> intel_edp_panel_off(intel_dp);
> @@ -1913,7 +1915,7 @@ static void intel_enable_dp(struct intel_encoder
> *encoder)
> if (WARN_ON(dp_reg & DP_PORT_EN))
> return;
>
> - edp_panel_vdd_on(intel_dp);
> + intel_edp_panel_vdd_on(intel_dp);
> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> intel_dp_start_link_train(intel_dp);
> intel_edp_panel_on(intel_dp);
> @@ -2951,7 +2953,7 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
> if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
> return;
>
> -
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Fix GEN8 GTT size calculation
On Mon, Mar 17, 2014 at 5:17 PM, Woodhouse, David wrote: > I think I have found this problem on the IOMMU side. We usually assume > that RMRRs are for boot-time only, such as USB controllers for the > legacy keyboard/mouse emulation. And a patch sneaked in which > effectively *unmaps* the RMRR regions when you do the first "real" > mapping for the driver. Having fixed that, I think I should no longer > see these DMA faults. Which commit is this? I've a patch+bug report that stolen + dmar is busted. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
On Mon, 17 Mar 2014, Romain Francoise wrote: > Hi Daniel, > > Daniel Vetter writes: > >> Please never just send a mail to maintainers, always cc mailing lists. > > Yes, sorry, I sent this privately because it wasn't a fully-formed bug > report; I just wanted Jani to confirm that it wasn't intentionally > broken as of -rc7. > >> Have you tested with the patch reverted to confirm that it's indeed >> this commit and not something else (e.g. in the acpi code)? > > I hadn't, but I've now reverted the revert and things work again as in > v3.13. So as far as I can tell the quirk is still definitely needed. Here's two patches to try, separately, *without* the quirk: Patch #1: diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index cb058408c70e..ef26115dbe68 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -659,7 +659,7 @@ static void pch_enable_backlight(struct intel_connector *connector) /* This won't stick until the above enable. */ intel_panel_actually_set_backlight(connector, panel->backlight.level); - pch_ctl2 = panel->backlight.max << 16; + pch_ctl2 = panel->backlight.max << 16 | panel->backlight.level; I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2); pch_ctl1 = 0; Patch #2: diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index cb058408c70e..10849104fa1e 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -669,6 +669,8 @@ static void pch_enable_backlight(struct intel_connector *connector) I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1); POSTING_READ(BLC_PWM_PCH_CTL1); I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE); + + intel_panel_actually_set_backlight(connector, panel->backlight.level); } static void i9xx_enable_backlight(struct intel_connector *connector) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] uxa: Support BLT ring flushes on Broadwell, but not render ring flushes.
Several places (such as intel_cache_expire) call intel_emit_batch_flush,
so it needs to work on Broadwell. Sometimes the batch is empty, in
which case current_batch may not yet be BLT_RING.
The PIPE_CONTROL code has not been ported to work on Broadwell, so
trying to do a render ring flush will hang the GPU. It also doesn't
make any sense to do a render ring flush, given that we never use the
render ring for UXA on Broadwell.
Signed-off-by: Kenneth Graunke
---
src/uxa/intel_batchbuffer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
This is an alternative to my old "uxa: Don't emit PIPE_CONTROLs in an
empty batch." patch, which Chris pointed out was broken.
Chris,
In the future, if you're going to rewrite significant portions of my
patches, could you please at least put your Signed-off-by or something
on it? In the version of "uxa: Enable BLT acceleration on Broadwell.",
you committed, at least half the patch was not actually written by me,
and the resulting code either hit assertion failures or GPU hangs if
run at all.
It's pretty disconcerting to see code committed under my name, with my
Signed-off-by, that doesn't work and which I've never even seen before.
diff --git a/src/uxa/intel_batchbuffer.c b/src/uxa/intel_batchbuffer.c
index 4aabe48..ec71ce2 100644
--- a/src/uxa/intel_batchbuffer.c
+++ b/src/uxa/intel_batchbuffer.c
@@ -183,11 +183,11 @@ void intel_batch_emit_flush(ScrnInfoPtr scrn)
int flags;
assert (!intel->in_batch_atomic);
- assert (INTEL_INFO(intel)->gen < 0100);
/* Big hammer, look to the pipelined flushes in future. */
if ((INTEL_INFO(intel)->gen >= 060)) {
- if (intel->current_batch == BLT_BATCH) {
+ if (intel->current_batch == BLT_BATCH ||
+ INTEL_INFO(intel)->gen >= 0100) {
BEGIN_BATCH_BLT(4);
OUT_BATCH(MI_FLUSH_DW | 2);
OUT_BATCH(0);
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Fix GEN8 GTT size calculation
On Mon, 2014-03-17 at 16:17 +, David Woodhouse wrote: > > Can't tell though, because my machine is still dying in an endless > stream of > [ 199.647850] [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* > Interrupt arrived before CRTCs were setup up Except when the failure mode is different... [ 24.143180] [ cut here ] [ 24.150433] WARNING: CPU: 1 PID: 161 at kernel/watchdog.c:245 watchdog_overflow_callback+0x9c/0xd0() [ 24.162945] Watchdog detected hard LOCKUP on cpu 1 [ 24.168206] Modules linked in: i915(+) i2c_algo_bit drm_kms_helper drm i2c_core video [ 24.181743] CPU: 1 PID: 161 Comm: systemd-udevd Not tainted 3.14.0-rc5+ #54 [ 24.191833] Hardware name: Intel Corporation Broadwell Client platform/WhiteTip Mountain 1, BIOS BDW-E2R1.86C.0053.R02.131213 [ 24.208364] 0009 88014e446c20 816b5c48 88014e446c68 [ 24.219026] 88014e446c58 81085e9d 880147cf [ 24.229676] 88014e446d88 88014e446ef8 88014e446cb8 [ 24.240298] Call Trace: [ 24.245251][] dump_stack+0x45/0x56 [ 24.254028] [] warn_slowpath_common+0x7d/0xa0 [ 24.263071] [] warn_slowpath_fmt+0x4c/0x50 [ 24.271802] [] ? restart_watchdog_hrtimer+0x50/0x50 [ 24.281352] [] watchdog_overflow_callback+0x9c/0xd0 [ 24.290979] [] __perf_event_overflow+0x8e/0x240 [ 24.300211] [] ? x86_perf_event_set_period+0xe8/0x150 [ 24.310053] [] perf_event_overflow+0x14/0x20 [ 24.319014] [] intel_pmu_handle_irq+0x1cd/0x3c0 [ 24.328264] [] perf_event_nmi_handler+0x2b/0x50 [ 24.337519] [] nmi_handle.isra.3+0x88/0x180 [ 24.346369] [] do_nmi+0x169/0x310 [ 24.354215] [] end_repeat_nmi+0x1e/0x2e [ 24.362660] [] ? alloc_vmap_area+0x6f/0x310 [ 24.371525] [] ? __gen6_gt_force_wake_mt_get+0x7b/0x150 [i915] [ 24.382215] [] ? __gen6_gt_force_wake_mt_get+0x7b/0x150 [i915] [ 24.392935] [] ? __gen6_gt_force_wake_mt_get+0x7b/0x150 [i915] [ 24.403590] <> [] gen8_write32+0x10c/0x120 [i915] [ 24.413655] [] gen8_gmch_probe+0xe7/0x190 [i915] [ 24.422984] [] i915_gem_gtt_init+0x62/0x200 [i915] [ 24.432504] [] i915_driver_load+0x43d/0xe10 [i915] [ 24.442000] [] ? drm_get_minor+0x1a5/0x200 [drm] [ 24.451322] [] drm_dev_register+0x7b/0x160 [drm] [ 24.460620] [] drm_get_pci_dev+0xa0/0x220 [drm] [ 24.469827] [] i915_pci_probe+0x3b/0x60 [i915] [ 24.478917] [] local_pci_probe+0x45/0xa0 [ 24.487382] [] pci_device_probe+0xd1/0x130 [ 24.496072] [] driver_probe_device+0x125/0x3a0 [ 24.505141] [] __driver_attach+0x93/0xa0 [ 24.513631] [] ? __device_attach+0x40/0x40 [ 24.522301] [] bus_for_each_dev+0x63/0xa0 [ 24.530871] [] driver_attach+0x1e/0x20 [ 24.539114] [] bus_add_driver+0x180/0x250 [ 24.547625] [] ? 0xa0146fff [ 24.54] [] driver_register+0x64/0xf0 [ 24.563824] [] ? 0xa0146fff [ 24.571716] [] __pci_register_driver+0x4b/0x50 [ 24.580701] [] drm_pci_init+0x11a/0x130 [drm] [ 24.589447] [] ? 0xa0146fff [ 24.597330] [] i915_init+0x6a/0x6c [i915] [ 24.605806] [] do_one_initcall+0xd2/0x180 [ 24.614278] [] ? set_memory_nx+0x43/0x50 [ 24.622651] [] load_module+0x1e0b/0x25a0 [ 24.631014] [] ? store_uevent+0x40/0x40 [ 24.639285] [] ? copy_module_from_fd.isra.47+0x12a/0x190 [ 24.649242] [] SyS_finit_module+0x86/0xb0 [ 24.657721] [] system_call_fastpath+0x16/0x1b [ 24.666590] ---[ end trace d4228967fcb7ae4a ]--- -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Fix GEN8 GTT size calculation
On Fri, 2014-03-14 at 23:10 +0100, Daniel Vetter wrote: > > I've asked you on private irc whether this range matches/overlaps with > stolen - we know of things blowing up at least on earlier generations > in combination with dmar. Please boot with drm.debug=0xe and scan for > the stolen mem reporting: Surely it *has* to be stolen? That's the whole *point* in the RMRR that the BIOS provides, telling us that the gfx unit is expecting to do DMA to this range of memory. If it isn't stolen, it's just being wantonly "borrowed". Have you *ever* known an RMRR point at memory other than the stolen range? I think I have found this problem on the IOMMU side. We usually assume that RMRRs are for boot-time only, such as USB controllers for the legacy keyboard/mouse emulation. And a patch sneaked in which effectively *unmaps* the RMRR regions when you do the first "real" mapping for the driver. Having fixed that, I think I should no longer see these DMA faults. Can't tell though, because my machine is still dying in an endless stream of [ 199.647850] [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* Interrupt arrived before CRTCs were setup up -- Sent with MeeGo's ActiveSync support. David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
On Mon, 17 Mar 2014, Jani Nikula wrote: > On Mon, 17 Mar 2014, Romain Francoise wrote: >> Hi Daniel, >> >> Daniel Vetter writes: >> >>> Please never just send a mail to maintainers, always cc mailing lists. >> >> Yes, sorry, I sent this privately because it wasn't a fully-formed bug >> report; I just wanted Jani to confirm that it wasn't intentionally >> broken as of -rc7. >> >>> Have you tested with the patch reverted to confirm that it's indeed >>> this commit and not something else (e.g. in the acpi code)? >> >> I hadn't, but I've now reverted the revert and things work again as in >> v3.13. So as far as I can tell the quirk is still definitely needed. > > I still don't think having the quirk is the way to go. Let's fix this > properly, shall we? > > Please file a new bug on DRM/Intel component at [1]. Please attach > intel_reg_dumper output (tool from intel-gpu-tools [2]) with and without > the revert. Please also attach dmesg with drm.debug=0xe from early boot. The register dump from *before* i915 gets loaded is also interesting. BR, Jani. > > Do you have the snb or ivb model? > > BR, > Jani. > > > [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI > [2] http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/ > >> >> Thanks! >> -r > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
On Mon, 17 Mar 2014, Romain Francoise wrote: > Hi Daniel, > > Daniel Vetter writes: > >> Please never just send a mail to maintainers, always cc mailing lists. > > Yes, sorry, I sent this privately because it wasn't a fully-formed bug > report; I just wanted Jani to confirm that it wasn't intentionally > broken as of -rc7. > >> Have you tested with the patch reverted to confirm that it's indeed >> this commit and not something else (e.g. in the acpi code)? > > I hadn't, but I've now reverted the revert and things work again as in > v3.13. So as far as I can tell the quirk is still definitely needed. I still don't think having the quirk is the way to go. Let's fix this properly, shall we? Please file a new bug on DRM/Intel component at [1]. Please attach intel_reg_dumper output (tool from intel-gpu-tools [2]) with and without the revert. Please also attach dmesg with drm.debug=0xe from early boot. Do you have the snb or ivb model? BR, Jani. [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI [2] http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/ > > Thanks! > -r -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux
On Mon, 17 Mar 2014, Rodrigo Vivi wrote: > On Mon, Mar 17, 2014 at 11:08 AM, Rodrigo Vivi wrote: >> On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote: >>> - if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1)) >>> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0); > > wrong ";" here.. Yikes, good catch. I've been rebasing this way too many times... Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux
On Mon, Mar 17, 2014 at 11:08 AM, Rodrigo Vivi wrote:
> On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
>> Functionality remains largely the same as before.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 257
>> +-
>> drivers/gpu/drm/i915/intel_drv.h |1 +
>> 2 files changed, 116 insertions(+), 142 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 22134edc03dd..24cbf4bd36c4 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -575,97 +575,77 @@ out:
>> return ret;
>> }
>>
>> -/* Write data to the aux channel in native mode */
>> -static int
>> -intel_dp_aux_native_write(struct intel_dp *intel_dp,
>> - uint16_t address, uint8_t *send, int send_bytes)
>> +#define HEADER_SIZE4
>> +static ssize_t
>> +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>> {
>> + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
>> + uint8_t txbuf[20], rxbuf[20];
>> + size_t txsize, rxsize;
>> int ret;
>> - uint8_t msg[20];
>> - int msg_bytes;
>> - uint8_t ack;
>> - int retry;
>>
>> - if (WARN_ON(send_bytes > 16))
>> - return -E2BIG;
>> + txbuf[0] = msg->request << 4;
>> + txbuf[1] = msg->address >> 8;
>> + txbuf[2] = msg->address & 0xff;
>> + txbuf[3] = msg->size - 1;
>>
>> - intel_dp_check_edp(intel_dp);
>> - msg[0] = DP_AUX_NATIVE_WRITE << 4;
>> - msg[1] = address >> 8;
>> - msg[2] = address & 0xff;
>> - msg[3] = send_bytes - 1;
>> - memcpy(&msg[4], send, send_bytes);
>> - msg_bytes = send_bytes + 4;
>> - for (retry = 0; retry < 7; retry++) {
>
> we were retrying 7 times always, but now we are now retrying only 3
> times or even none.
> Couldn't it trigger some new bug or regression?
>
>> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
>> - if (ret < 0)
>> - return ret;
>> - ack >>= 4;
>> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) ==
>> DP_AUX_NATIVE_REPLY_ACK)
>> - return send_bytes;
>> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) ==
>> DP_AUX_NATIVE_REPLY_DEFER)
>> - usleep_range(400, 500);
>
> we are also not checking this ack x defer (here nor in native_write)
> replies anymore.
> Don't we need it anymore? why?
>
>> - else
>> - return -EIO;
>> - }
>> + switch (msg->request & ~DP_AUX_I2C_MOT) {
>> + case DP_AUX_NATIVE_WRITE:
>> + case DP_AUX_I2C_WRITE:
>> + txsize = HEADER_SIZE + msg->size;
>> + rxsize = 1;
>>
>> - DRM_ERROR("too many retries, giving up\n");
>> - return -EIO;
>> -}
>> + if (WARN_ON(txsize > 20))
>> + return -E2BIG;
>>
>> -/* Write a single byte to the aux channel in native mode */
>> -static int
>> -intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
>> - uint16_t address, uint8_t byte)
>> -{
>> - return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
>> -}
>> + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>>
>> -/* read bytes from a native aux channel */
>> -static int
>> -intel_dp_aux_native_read(struct intel_dp *intel_dp,
>> -uint16_t address, uint8_t *recv, int recv_bytes)
>> -{
>> - uint8_t msg[4];
>> - int msg_bytes;
>> - uint8_t reply[20];
>> - int reply_bytes;
>> - uint8_t ack;
>> - int ret;
>> - int retry;
>> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf,
>> rxsize);
>> + if (ret > 0) {
>> + msg->reply = rxbuf[0] >> 4;
>>
>> - if (WARN_ON(recv_bytes > 19))
>> - return -E2BIG;
>> + /* Return payload size. */
>> + ret = msg->size;
>> + }
>> + break;
>>
>> - intel_dp_check_edp(intel_dp);
>> - msg[0] = DP_AUX_NATIVE_READ << 4;
>> - msg[1] = address >> 8;
>> - msg[2] = address & 0xff;
>> - msg[3] = recv_bytes - 1;
>> + case DP_AUX_NATIVE_READ:
>> + case DP_AUX_I2C_READ:
>> + txsize = HEADER_SIZE;
>> + rxsize = msg->size + 1;
>>
>> - msg_bytes = 4;
>> - reply_bytes = recv_bytes + 1;
>> + if (WARN_ON(rxsize > 20))
>> + return -E2BIG;
>>
>> - for (retry = 0; retry < 7; retry++) {
>> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
>> - reply, reply_bytes);
>> - if (ret == 0)
>> - return -EPROTO;
>> - if (ret < 0)
>> -
Re: [Intel-gfx] [PATCH 5/6] drm/i915/dp: move dp aux ch register init to aux init
On Mon, 17 Mar 2014, Rodrigo Vivi wrote:
> I would prefer an else instead of unecessary comparisons and
> assignment that will be override..
In general I agree, but see next patch. ;)
BR,
Jani.
>
> But anyway it works, so feel free to use:
> Reviewed-by: Rodrigo Vivi
>
> On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
>> Do a slight rearrangement of the switch to prep for follow-up.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 42
>> ---
>> 1 file changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 24cbf4bd36c4..71a76d00d634 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -643,6 +643,28 @@ static void
>> intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector
>> *connector)
>> {
>> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> + enum port port = intel_dig_port->port;
>> +
>> + switch (port) {
>> + case PORT_A:
>> + intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
>> + break;
>> + case PORT_B:
>> + intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
>> + break;
>> + case PORT_C:
>> + intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
>> + break;
>> + case PORT_D:
>> + intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
>> + break;
>> + default:
>> + BUG();
>> + }
>> +
>> + if (!HAS_DDI(dev))
>> + intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
>>
>> intel_dp->aux.dev = dev->dev;
>> intel_dp->aux.transfer = intel_dp_aux_transfer;
>> @@ -3849,26 +3871,6 @@ intel_dp_init_connector(struct intel_digital_port
>> *intel_dig_port,
>> intel_connector->get_hw_state = intel_connector_get_hw_state;
>> intel_connector->unregister = intel_dp_connector_unregister;
>>
>> - intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
>> - if (HAS_DDI(dev)) {
>> - switch (intel_dig_port->port) {
>> - case PORT_A:
>> - intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
>> - break;
>> - case PORT_B:
>> - intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
>> - break;
>> - case PORT_C:
>> - intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
>> - break;
>> - case PORT_D:
>> - intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
>> - break;
>> - default:
>> - BUG();
>> - }
>> - }
>> -
>> /* Set up the DDC bus. */
>> switch (port) {
>> case PORT_A:
>> --
>> 1.7.9.5
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp: split edp_panel_vdd_on() for reuse
forgot to say that this patch needs a rebase on -nightly
On Mon, Mar 17, 2014 at 11:09 AM, Rodrigo Vivi wrote:
> Reviewed-by: Rodrigo Vivi
>
> On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
>> Introduce _edp_panel_vdd_on() that returns true if the call enabled vdd,
>> and a matching disable is needed. Keep edp_panel_vdd_on() as a helper
>> for when it is expected the vdd is off.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 22 --
>> 1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index bb66f9301cd9..94e7577da484 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -91,6 +91,7 @@ static struct intel_dp *intel_attached_dp(struct
>> drm_connector *connector)
>> }
>>
>> static void intel_dp_link_down(struct intel_dp *intel_dp);
>> +static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
>> static void edp_panel_vdd_on(struct intel_dp *intel_dp);
>> static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
>>
>> @@ -1162,23 +1163,21 @@ static u32 ironlake_get_pp_control(struct intel_dp
>> *intel_dp)
>> return control;
>> }
>>
>> -static void edp_panel_vdd_on(struct intel_dp *intel_dp)
>> +static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
>> {
>> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> u32 pp;
>> u32 pp_stat_reg, pp_ctrl_reg;
>> + bool need_to_disable = !intel_dp->want_panel_vdd;
>>
>> if (!is_edp(intel_dp))
>> - return;
>> -
>> - WARN(intel_dp->want_panel_vdd,
>> -"eDP VDD already requested on\n");
>> + return false;
>>
>> intel_dp->want_panel_vdd = true;
>>
>> if (edp_have_panel_vdd(intel_dp))
>> - return;
>> + return need_to_disable;
>>
>> intel_runtime_pm_get(dev_priv);
>>
>> @@ -1204,6 +1203,17 @@ static void edp_panel_vdd_on(struct intel_dp
>> *intel_dp)
>> DRM_DEBUG_KMS("eDP was not running\n");
>> msleep(intel_dp->panel_power_up_delay);
>> }
>> +
>> + return need_to_disable;
>> +}
>> +
>> +static void edp_panel_vdd_on(struct intel_dp *intel_dp)
>> +{
>> + if (is_edp(intel_dp)) {
>> + bool vdd = _edp_panel_vdd_on(intel_dp);
>> +
>> + WARN(!vdd, "eDP VDD already requested on\n");
>> + }
>> }
>>
>> static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>> --
>> 1.7.9.5
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/dp: use the new drm helpers for dp i2c-over-aux
On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
> The functionality remains largerly the same. The main difference is that
> i2c-over-aux defer timeouts are increased to be safe for all use cases
> instead of depending on DP device type and properties.
I was going to ask about different timeouts but you had already answered here ;)
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dp.c | 197
> ++
> drivers/gpu/drm/i915/intel_drv.h |2 -
> 2 files changed, 30 insertions(+), 169 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 71a76d00d634..208fdf075140 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -645,19 +645,25 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct
> intel_connector *connector)
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> enum port port = intel_dig_port->port;
> + const char *name = NULL;
> + int ret;
>
> switch (port) {
> case PORT_A:
> intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
> + name = "DPDDC-A";
> break;
> case PORT_B:
> intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
> + name = "DPDDC-B";
> break;
> case PORT_C:
> intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
> + name = "DPDDC-C";
> break;
> case PORT_D:
> intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
> + name = "DPDDC-D";
> break;
> default:
> BUG();
> @@ -666,128 +672,27 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct
> intel_connector *connector)
> if (!HAS_DDI(dev))
> intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
>
> + intel_dp->aux.name = name;
> intel_dp->aux.dev = dev->dev;
> intel_dp->aux.transfer = intel_dp_aux_transfer;
> -}
>
> -static int
> -intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> - uint8_t write_byte, uint8_t *read_byte)
> -{
> - struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
> - struct intel_dp *intel_dp = container_of(adapter,
> - struct intel_dp,
> - adapter);
> - uint16_t address = algo_data->address;
> - uint8_t msg[5];
> - uint8_t reply[2];
> - unsigned retry;
> - int msg_bytes;
> - int reply_bytes;
> - int ret;
> -
> - /* Set up the command byte */
> - if (mode & MODE_I2C_READ)
> - msg[0] = DP_AUX_I2C_READ << 4;
> - else
> - msg[0] = DP_AUX_I2C_WRITE << 4;
> + DRM_DEBUG_KMS("registering %s bus for %s\n", name,
> + connector->base.kdev->kobj.name);
>
> - if (!(mode & MODE_I2C_STOP))
> - msg[0] |= DP_AUX_I2C_MOT << 4;
> -
> - msg[1] = address >> 8;
> - msg[2] = address;
> -
> - switch (mode) {
> - case MODE_I2C_WRITE:
> - msg[3] = 0;
> - msg[4] = write_byte;
> - msg_bytes = 5;
> - reply_bytes = 1;
> - break;
> - case MODE_I2C_READ:
> - msg[3] = 0;
> - msg_bytes = 4;
> - reply_bytes = 2;
> - break;
> - default:
> - msg_bytes = 3;
> - reply_bytes = 1;
> - break;
> + ret = drm_dp_aux_register_i2c_bus(&intel_dp->aux);
> + if (ret < 0) {
> + DRM_ERROR("drm_dp_aux_register_i2c_bus() for %s failed
> (%d)\n",
> + name, ret);
> + return;
> }
>
> - /*
> -* DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device
> is
> -* required to retry at least seven times upon receiving AUX_DEFER
> -* before giving up the AUX transaction.
> -*/
> - for (retry = 0; retry < 7; retry++) {
> - ret = intel_dp_aux_ch(intel_dp,
> - msg, msg_bytes,
> - reply, reply_bytes);
> - if (ret < 0) {
> - DRM_DEBUG_KMS("aux_ch failed %d\n", ret);
> - goto out;
> - }
> -
> - switch ((reply[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK) {
> - case DP_AUX_NATIVE_REPLY_ACK:
> - /* I2C-over-AUX Reply field is only valid
> -* when paired with AUX ACK.
> -*/
> - break;
> - case DP_AUX_NATIVE_REPLY_NACK:
> - DRM_DEBUG_KMS("aux_ch native nack\n");
> -
[Intel-gfx] [PATCH] drm/i915: finish off reverting eDP VDD changes
This is a small follow-up fix to the series of eDP VDD back and forth
we've had recently. This is effectively a combined revert of three
commits:
commit 2c2894f698fffd8ff53e1e1d3834f9e1035b1f39
Author: Paulo Zanoni
Date: Fri Mar 7 20:05:20 2014 -0300
drm/i915: properly disable the VDD when disabling the panel
commit b3064154dfd37deb386b1e459c54e1ca2460b3d5
Author: Patrik Jakobsson
Date: Tue Mar 4 00:42:44 2014 +0100
drm/i915: Don't just say it, actually force edp vdd
commit dff392dbd258381a6c3164f38420593f2d291e3b
Author: Paulo Zanoni
Date: Fri Dec 6 17:32:41 2013 -0200
drm/i915: don't touch the VDD when disabling the panel
which shows that we're pretty close back to where we started
already. The first two were basically reverting the last, but missing
the WARN. Add that back. We also OCD the intel_ prefix back to
intel_edp_panel_vdd_on() which was lost somewhere in between. The circle
closes.
For future reference, "drm/i915: don't touch the VDD when disabling the
panel" failed to take into account
commit 6cb49835da0426f69a2931bc2a0a8156344b0e41
Author: Daniel Vetter
Date: Sun May 20 17:14:50 2012 +0200
drm/i915: enable vdd when switching off the eDP panel
and
commit 35a38556d900b9cb5dfa2529c93944b847f8a8a4
Author: Daniel Vetter
Date: Sun Aug 12 22:17:14 2012 +0200
drm/i915: reorder edp disabling to fix ivb MacBook Air
Cc: Patrik Jakobsson
Cc: Paulo Zanoni
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_ddi.c |2 +-
drivers/gpu/drm/i915/intel_dp.c | 14 --
drivers/gpu/drm/i915/intel_drv.h |2 +-
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fe1f5f012c4f..070bf2e78d61 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1340,7 +1340,7 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
- edp_panel_vdd_on(intel_dp);
+ intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7854cdbb462a..071d44fe2fc8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -676,7 +676,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
int reply_bytes;
int ret;
- edp_panel_vdd_on(intel_dp);
+ intel_edp_panel_vdd_on(intel_dp);
intel_dp_check_edp(intel_dp);
/* Set up the command byte */
if (mode & MODE_I2C_READ)
@@ -1161,7 +1161,7 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
return control;
}
-void edp_panel_vdd_on(struct intel_dp *intel_dp)
+void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1329,6 +1329,8 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
edp_wait_backlight_off(intel_dp);
+ WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
+
pp = ironlake_get_pp_control(intel_dp);
/* We need to switch off panel power _and_ force vdd, for otherwise some
* panels get very unhappy and cease to work. */
@@ -1880,7 +1882,7 @@ static void intel_disable_dp(struct intel_encoder
*encoder)
/* Make sure the panel is off before trying to change the mode. But also
* ensure that we have vdd while we switch off the panel. */
- edp_panel_vdd_on(intel_dp);
+ intel_edp_panel_vdd_on(intel_dp);
intel_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_off(intel_dp);
@@ -1913,7 +1915,7 @@ static void intel_enable_dp(struct intel_encoder *encoder)
if (WARN_ON(dp_reg & DP_PORT_EN))
return;
- edp_panel_vdd_on(intel_dp);
+ intel_edp_panel_vdd_on(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
intel_edp_panel_on(intel_dp);
@@ -2951,7 +2953,7 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
return;
- edp_panel_vdd_on(intel_dp);
+ intel_edp_panel_vdd_on(intel_dp);
if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3))
DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
@@ -3748,7 +3750,7 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
return true;
/* Cache DPCD and EDID for edp. */
- edp_panel_vdd_on(intel_dp);
+ intel_edp_panel_vdd_on(intel_dp);
[Intel-gfx] [PATCH for -fixes] Revert "drm/i915: don't touch the VDD when disabling the panel"
This reverts
commit dff392dbd258381a6c3164f38420593f2d291e3b
Author: Paulo Zanoni
Date: Fri Dec 6 17:32:41 2013 -0200
drm/i915: don't touch the VDD when disabling the panel
which didn't take into account
commit 6cb49835da0426f69a2931bc2a0a8156344b0e41
Author: Daniel Vetter
Date: Sun May 20 17:14:50 2012 +0200
drm/i915: enable vdd when switching off the eDP panel
and
commit 35a38556d900b9cb5dfa2529c93944b847f8a8a4
Author: Daniel Vetter
Date: Sun Aug 12 22:17:14 2012 +0200
drm/i915: reorder edp disabling to fix ivb MacBook Air
Unsurprisingly, various MacBooks failed.
Effectively the same has already been done in drm-intel-next-queued.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74628
Cc: Patrik Jakobsson
Cc: Paulo Zanoni
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_ddi.c |1 +
drivers/gpu/drm/i915/intel_dp.c | 10 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e06b9e017d6b..234ac5f7bc5a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1244,6 +1244,7 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+ ironlake_edp_panel_vdd_on(intel_dp);
ironlake_edp_panel_off(intel_dp);
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41bdac44dfc1..2688f6d64bb9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1249,17 +1249,24 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
DRM_DEBUG_KMS("Turn eDP power off\n");
+ WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
+
pp = ironlake_get_pp_control(intel_dp);
/* We need to switch off panel power _and_ force vdd, for otherwise some
* panels get very unhappy and cease to work. */
- pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);
+ pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET |
EDP_BLC_ENABLE);
pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
+ intel_dp->want_panel_vdd = false;
+
ironlake_wait_panel_off(intel_dp);
+
+ /* We got a reference when we enabled the VDD. */
+ intel_runtime_pm_put(dev_priv);
}
void ironlake_edp_backlight_on(struct intel_dp *intel_dp)
@@ -1784,6 +1791,7 @@ static void intel_disable_dp(struct intel_encoder
*encoder)
/* Make sure the panel is off before trying to change the mode. But also
* ensure that we have vdd while we switch off the panel. */
+ ironlake_edp_panel_vdd_on(intel_dp);
ironlake_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
ironlake_edp_panel_off(intel_dp);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915/dp: move dp aux ch register init to aux init
I would prefer an else instead of unecessary comparisons and
assignment that will be override..
But anyway it works, so feel free to use:
Reviewed-by: Rodrigo Vivi
On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
> Do a slight rearrangement of the switch to prep for follow-up.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dp.c | 42
> ---
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 24cbf4bd36c4..71a76d00d634 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -643,6 +643,28 @@ static void
> intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector
> *connector)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + enum port port = intel_dig_port->port;
> +
> + switch (port) {
> + case PORT_A:
> + intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
> + break;
> + case PORT_B:
> + intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
> + break;
> + case PORT_C:
> + intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
> + break;
> + case PORT_D:
> + intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
> + break;
> + default:
> + BUG();
> + }
> +
> + if (!HAS_DDI(dev))
> + intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
>
> intel_dp->aux.dev = dev->dev;
> intel_dp->aux.transfer = intel_dp_aux_transfer;
> @@ -3849,26 +3871,6 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> intel_connector->get_hw_state = intel_connector_get_hw_state;
> intel_connector->unregister = intel_dp_connector_unregister;
>
> - intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
> - if (HAS_DDI(dev)) {
> - switch (intel_dig_port->port) {
> - case PORT_A:
> - intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
> - break;
> - case PORT_B:
> - intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
> - break;
> - case PORT_C:
> - intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
> - break;
> - case PORT_D:
> - intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
> - break;
> - default:
> - BUG();
> - }
> - }
> -
> /* Set up the DDC bus. */
> switch (port) {
> case PORT_A:
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915/dp: split edp_panel_vdd_on() for reuse
Reviewed-by: Rodrigo Vivi
On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
> Introduce _edp_panel_vdd_on() that returns true if the call enabled vdd,
> and a matching disable is needed. Keep edp_panel_vdd_on() as a helper
> for when it is expected the vdd is off.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dp.c | 22 --
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bb66f9301cd9..94e7577da484 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -91,6 +91,7 @@ static struct intel_dp *intel_attached_dp(struct
> drm_connector *connector)
> }
>
> static void intel_dp_link_down(struct intel_dp *intel_dp);
> +static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
> static void edp_panel_vdd_on(struct intel_dp *intel_dp);
> static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
>
> @@ -1162,23 +1163,21 @@ static u32 ironlake_get_pp_control(struct intel_dp
> *intel_dp)
> return control;
> }
>
> -static void edp_panel_vdd_on(struct intel_dp *intel_dp)
> +static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 pp;
> u32 pp_stat_reg, pp_ctrl_reg;
> + bool need_to_disable = !intel_dp->want_panel_vdd;
>
> if (!is_edp(intel_dp))
> - return;
> -
> - WARN(intel_dp->want_panel_vdd,
> -"eDP VDD already requested on\n");
> + return false;
>
> intel_dp->want_panel_vdd = true;
>
> if (edp_have_panel_vdd(intel_dp))
> - return;
> + return need_to_disable;
>
> intel_runtime_pm_get(dev_priv);
>
> @@ -1204,6 +1203,17 @@ static void edp_panel_vdd_on(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("eDP was not running\n");
> msleep(intel_dp->panel_power_up_delay);
> }
> +
> + return need_to_disable;
> +}
> +
> +static void edp_panel_vdd_on(struct intel_dp *intel_dp)
> +{
> + if (is_edp(intel_dp)) {
> + bool vdd = _edp_panel_vdd_on(intel_dp);
> +
> + WARN(!vdd, "eDP VDD already requested on\n");
> + }
> }
>
> static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915/dp: move edp vdd enable/disable at a lower level in i2c-over-aux
Reviewed-by: Rodrigo Vivi On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote: > This is prep work for conversion to generic drm i2c-over-aux helpers > where we won't have the function to do this at. > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_dp.c |9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 94e7577da484..22134edc03dd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -461,6 +461,9 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > uint32_t status; > int try, clock = 0; > bool has_aux_irq = HAS_AUX_IRQ(dev); > + bool vdd; > + > + vdd = _edp_panel_vdd_on(intel_dp); > > /* dp aux is extremely sensitive to irq latency, hence request the > * lowest possible wakeup latency and so prevent the cpu from going > into > @@ -566,6 +569,9 @@ out: > pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE); > intel_aux_display_runtime_put(dev_priv); > > + if (vdd) > + edp_panel_vdd_off(intel_dp, false); > + > return ret; > } > > @@ -678,8 +684,6 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode, > int reply_bytes; > int ret; > > - edp_panel_vdd_on(intel_dp); > - intel_dp_check_edp(intel_dp); > /* Set up the command byte */ > if (mode & MODE_I2C_READ) > msg[0] = DP_AUX_I2C_READ << 4; > @@ -781,7 +785,6 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode, > ret = -EREMOTEIO; > > out: > - edp_panel_vdd_off(intel_dp, false); > return ret; > } > > -- > 1.7.9.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/dp: let drivers specify the name of the I2C-over-AUX adapter
Reviewed-by: Rodrigo Vivi
//Although I know it has been already reviewed ;)
On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
> Let the drivers specify the name of the I2C-over-AUX adapter to maintain
> backwards compatibility in the sysfs when converting to the new
> I2C-over-AUX helper infrastructure.
>
> The i915 driver currently uses DPDDC-A to DPDDC-D as names for the DP
> i2c adapters. These names show up in the i2c sysfs name attribute. We'd
> like to be able to maintain that when switching over to the new helpers.
>
> Due to i2c device and connector cleanup ordering issues we also recently
> made the drm device (instead of connector) the parent of the i2c
> adapters:
>
> commit 80f65de3c9b8101c1613fa82df500ba6a099a11c
> Author: Imre Deak
> Date: Tue Feb 11 17:12:49 2014 +0200
>
> drm/i915: dp: fix order of dp aux i2c device cleanup
>
> With the name picked up from the adapter parent using dev_name(), it
> would be the same for all i2c adapters with the current I2C-over-AUX
> helpers.
>
> Signed-off-by: Jani Nikula
> Reviewed-by: Thierry Reding
> ---
> drivers/gpu/drm/drm_dp_helper.c |3 ++-
> include/drm/drm_dp_helper.h |4
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 35251af3b14e..17832d048147 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -726,7 +726,8 @@ int drm_dp_aux_register_i2c_bus(struct drm_dp_aux *aux)
> aux->ddc.dev.parent = aux->dev;
> aux->ddc.dev.of_node = aux->dev->of_node;
>
> - strncpy(aux->ddc.name, dev_name(aux->dev), sizeof(aux->ddc.name));
> + strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
> + sizeof(aux->ddc.name));
>
> return i2c_add_adapter(&aux->ddc);
> }
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 42947566e755..b4f58914bf7d 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -438,6 +438,9 @@ struct drm_dp_aux_msg {
> * The .dev field should be set to a pointer to the device that implements
> * the AUX channel.
> *
> + * The .name field may be used to specify the name of the I2C adapter. If
> set to
> + * NULL, dev_name() of .dev will be used.
> + *
> * Drivers provide a hardware-specific implementation of how transactions
> * are executed via the .transfer() function. A pointer to a drm_dp_aux_msg
> * structure describing the transaction is passed into this function. Upon
> @@ -455,6 +458,7 @@ struct drm_dp_aux_msg {
> * should call drm_dp_aux_unregister_i2c_bus() to remove the I2C adapter.
> */
> struct drm_dp_aux {
> + const char *name;
> struct i2c_adapter ddc;
> struct device *dev;
>
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux
On Fri, Mar 14, 2014 at 11:51 AM, Jani Nikula wrote:
> Functionality remains largely the same as before.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dp.c | 257
> +-
> drivers/gpu/drm/i915/intel_drv.h |1 +
> 2 files changed, 116 insertions(+), 142 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 22134edc03dd..24cbf4bd36c4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -575,97 +575,77 @@ out:
> return ret;
> }
>
> -/* Write data to the aux channel in native mode */
> -static int
> -intel_dp_aux_native_write(struct intel_dp *intel_dp,
> - uint16_t address, uint8_t *send, int send_bytes)
> +#define HEADER_SIZE4
> +static ssize_t
> +intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> {
> + struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
> + uint8_t txbuf[20], rxbuf[20];
> + size_t txsize, rxsize;
> int ret;
> - uint8_t msg[20];
> - int msg_bytes;
> - uint8_t ack;
> - int retry;
>
> - if (WARN_ON(send_bytes > 16))
> - return -E2BIG;
> + txbuf[0] = msg->request << 4;
> + txbuf[1] = msg->address >> 8;
> + txbuf[2] = msg->address & 0xff;
> + txbuf[3] = msg->size - 1;
>
> - intel_dp_check_edp(intel_dp);
> - msg[0] = DP_AUX_NATIVE_WRITE << 4;
> - msg[1] = address >> 8;
> - msg[2] = address & 0xff;
> - msg[3] = send_bytes - 1;
> - memcpy(&msg[4], send, send_bytes);
> - msg_bytes = send_bytes + 4;
> - for (retry = 0; retry < 7; retry++) {
we were retrying 7 times always, but now we are now retrying only 3
times or even none.
Couldn't it trigger some new bug or regression?
> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
> - if (ret < 0)
> - return ret;
> - ack >>= 4;
> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) ==
> DP_AUX_NATIVE_REPLY_ACK)
> - return send_bytes;
> - else if ((ack & DP_AUX_NATIVE_REPLY_MASK) ==
> DP_AUX_NATIVE_REPLY_DEFER)
> - usleep_range(400, 500);
we are also not checking this ack x defer (here nor in native_write)
replies anymore.
Don't we need it anymore? why?
> - else
> - return -EIO;
> - }
> + switch (msg->request & ~DP_AUX_I2C_MOT) {
> + case DP_AUX_NATIVE_WRITE:
> + case DP_AUX_I2C_WRITE:
> + txsize = HEADER_SIZE + msg->size;
> + rxsize = 1;
>
> - DRM_ERROR("too many retries, giving up\n");
> - return -EIO;
> -}
> + if (WARN_ON(txsize > 20))
> + return -E2BIG;
>
> -/* Write a single byte to the aux channel in native mode */
> -static int
> -intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
> - uint16_t address, uint8_t byte)
> -{
> - return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
> -}
> + memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>
> -/* read bytes from a native aux channel */
> -static int
> -intel_dp_aux_native_read(struct intel_dp *intel_dp,
> -uint16_t address, uint8_t *recv, int recv_bytes)
> -{
> - uint8_t msg[4];
> - int msg_bytes;
> - uint8_t reply[20];
> - int reply_bytes;
> - uint8_t ack;
> - int ret;
> - int retry;
> + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
> + if (ret > 0) {
> + msg->reply = rxbuf[0] >> 4;
>
> - if (WARN_ON(recv_bytes > 19))
> - return -E2BIG;
> + /* Return payload size. */
> + ret = msg->size;
> + }
> + break;
>
> - intel_dp_check_edp(intel_dp);
> - msg[0] = DP_AUX_NATIVE_READ << 4;
> - msg[1] = address >> 8;
> - msg[2] = address & 0xff;
> - msg[3] = recv_bytes - 1;
> + case DP_AUX_NATIVE_READ:
> + case DP_AUX_I2C_READ:
> + txsize = HEADER_SIZE;
> + rxsize = msg->size + 1;
>
> - msg_bytes = 4;
> - reply_bytes = recv_bytes + 1;
> + if (WARN_ON(rxsize > 20))
> + return -E2BIG;
>
> - for (retry = 0; retry < 7; retry++) {
> - ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
> - reply, reply_bytes);
> - if (ret == 0)
> - return -EPROTO;
> - if (ret < 0)
> - return ret;
> - ack = reply[0] >> 4;
> - if ((ack & DP_AUX_NATIVE_REPLY_MASK) ==
> DP_AUX_NATIVE_REPLY_ACK) {
> - memcpy(recv, reply + 1, ret - 1);
> -
Re: [Intel-gfx] [PATCH] drm/i915: Don't del_timer_sync uninitialized timer
On Mon, Mar 17, 2014 at 01:17:32PM +, Chris Wilson wrote:
> On Sat, Mar 15, 2014 at 11:30:18AM -0700, Ben Widawsky wrote:
> > On Sat, Mar 15, 2014 at 03:20:23PM +, Chris Wilson wrote:
> > > On Sat, Mar 15, 2014 at 12:47:22PM +0100, Daniel Vetter wrote:
> > > > On Fri, Mar 14, 2014 at 05:21:36PM -0700, Ben Widawsky wrote:
> > > > > Broken by:
> > > > > commit 0294ae7b44bba7ab0d4cef9a8736287f38bdb4fd
> > > > > Author: Chris Wilson
> > > > > Date: Thu Mar 13 12:00:29 2014 +
> > > > >
> > > > > drm/i915: Consolidate forcewake resetting to a single function
> > > > >
> > > > > Cc: Chris Wilson
> > > > > Cc: Mika Kuoppala
> > > > > Signed-off-by: Ben Widawsky
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_uncore.c | 6 +++---
> > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > > > > b/drivers/gpu/drm/i915/intel_uncore.c
> > > > > index e6bb421..7e55ceb 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > > > @@ -362,6 +362,9 @@ void intel_uncore_early_sanitize(struct
> > > > > drm_device *dev)
> > > > > {
> > > > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > > >
> > > > > + setup_timer(&dev_priv->uncore.force_wake_timer,
> > > > > + gen6_force_wake_timer, (unsigned long)dev_priv);
> > > >
> > > > We call early_sanitize also from our resume code, so this will now
> > > > re-setup the timer again. We generally don't do that since if we ever
> > > > leak
> > > > the timer to here in an enabled state it causes havoc.
> > >
> > > Gah, really? intel_uncore_early_init()! There must be a clean way to
> > > break this up.
> > > -Chris
> >
> > At least in the code base I was looking at, we currently do this also,
> > so I didn't think this was any worse.
> >
> > With lockdep turned on, the module will not even load, so please either
> > revert the original, or merge this.
>
> I think we can just:
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index a62ff21..c9fcc20 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -317,9 +317,10 @@ static void intel_uncore_forcewake_reset(struct
> drm_device *dev, bool restore)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> unsigned long irqflags;
> - int pending;
> + int pending = 0;
>
> - pending = del_timer_sync(&dev_priv->uncore.force_wake_timer);
> + if (dev_priv->uncore.force_wake_timer.function)
> + pending = del_timer_sync(&dev_priv->uncore.force_wake_timer);
>
> /* Hold uncore.lock across reset to prevent any register access
> * with forcewake not set correctly
>
> i.e.
>
> - del_timer_sync(&dev_priv->uncore.force_wake_timer);
> + if (dev_priv->uncore.force_wake_timer.function)
> + del_timer_sync(&dev_priv->uncore.force_wake_timer);
I've thought of this:
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e4d2b9f15ae2..9faee49f210d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1608,8 +1608,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto put_bridge;
}
- intel_uncore_early_sanitize(dev);
-
/* This must be called before any calls to HAS_PCH_* */
intel_detect_pch(dev);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index e2e328d86aff..c3832d9270a6 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -736,6 +736,8 @@ void intel_uncore_init(struct drm_device *dev)
setup_timer(&dev_priv->uncore.force_wake_timer,
gen6_force_wake_timer, (unsigned long)dev_priv);
+ intel_uncore_early_sanitize(dev);
+
if (IS_VALLEYVIEW(dev)) {
dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
dev_priv->uncore.funcs.force_wake_put = __vlv_force_wake_put;
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] v3.14-rc7: Backlight control broken on Dell XPS13
Please never just send a mail to maintainers, always cc mailing lists. On Mon, Mar 17, 2014 at 2:33 PM, Romain Francoise wrote: > Moving from v3.13 to v3.14-rc7 breaks backlight control on my Dell > XPS13, which I assume is because of this change: > > http://git.kernel.org/linus/bc0bb9fd1c7810407ab810d204bbaecb255fddde > > Is this right? Is backlight supposed to work correctly without the quirk > now? Yeah, it should work without the quirk. Have you tested with the patch reverted to confirm that it's indeed this commit and not something else (e.g. in the acpi code)? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't del_timer_sync uninitialized timer
On Sat, Mar 15, 2014 at 11:30:18AM -0700, Ben Widawsky wrote:
> On Sat, Mar 15, 2014 at 03:20:23PM +, Chris Wilson wrote:
> > On Sat, Mar 15, 2014 at 12:47:22PM +0100, Daniel Vetter wrote:
> > > On Fri, Mar 14, 2014 at 05:21:36PM -0700, Ben Widawsky wrote:
> > > > Broken by:
> > > > commit 0294ae7b44bba7ab0d4cef9a8736287f38bdb4fd
> > > > Author: Chris Wilson
> > > > Date: Thu Mar 13 12:00:29 2014 +
> > > >
> > > > drm/i915: Consolidate forcewake resetting to a single function
> > > >
> > > > Cc: Chris Wilson
> > > > Cc: Mika Kuoppala
> > > > Signed-off-by: Ben Widawsky
> > > > ---
> > > > drivers/gpu/drm/i915/intel_uncore.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > > > b/drivers/gpu/drm/i915/intel_uncore.c
> > > > index e6bb421..7e55ceb 100644
> > > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > > @@ -362,6 +362,9 @@ void intel_uncore_early_sanitize(struct drm_device
> > > > *dev)
> > > > {
> > > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > >
> > > > + setup_timer(&dev_priv->uncore.force_wake_timer,
> > > > + gen6_force_wake_timer, (unsigned long)dev_priv);
> > >
> > > We call early_sanitize also from our resume code, so this will now
> > > re-setup the timer again. We generally don't do that since if we ever leak
> > > the timer to here in an enabled state it causes havoc.
> >
> > Gah, really? intel_uncore_early_init()! There must be a clean way to
> > break this up.
> > -Chris
>
> At least in the code base I was looking at, we currently do this also,
> so I didn't think this was any worse.
>
> With lockdep turned on, the module will not even load, so please either
> revert the original, or merge this.
I think we can just:
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index a62ff21..c9fcc20 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -317,9 +317,10 @@ static void intel_uncore_forcewake_reset(struct drm_device
*dev, bool restore)
{
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long irqflags;
- int pending;
+ int pending = 0;
- pending = del_timer_sync(&dev_priv->uncore.force_wake_timer);
+ if (dev_priv->uncore.force_wake_timer.function)
+ pending = del_timer_sync(&dev_priv->uncore.force_wake_timer);
/* Hold uncore.lock across reset to prevent any register access
* with forcewake not set correctly
i.e.
- del_timer_sync(&dev_priv->uncore.force_wake_timer);
+ if (dev_priv->uncore.force_wake_timer.function)
+ del_timer_sync(&dev_priv->uncore.force_wake_timer);
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] lib: Export interval_tree
On Mon, 2014-03-17 at 12:21 +, Chris Wilson wrote: > +EXPORT_SYMBOL(interval_tree_insert); > +EXPORT_SYMBOL(interval_tree_remove); > +EXPORT_SYMBOL(interval_tree_iter_first); > +EXPORT_SYMBOL(interval_tree_iter_next); I'd prefer to see EXPORT_SYMBOL_GPL for this. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] lib: Export interval_tree
lib/interval_tree.c provides a simple interface for an interval-tree
(an augmented red-black tree) but is only built when testing the generic
macros for building interval-trees. For drivers with modest needs,
export the simple interval-tree library as is.
v2: Lots of help from Michel Lespinasse to only compile the code
as required:
- make INTERVAL_TREE a config option
- make INTERVAL_TREE_TEST select the library functions
and sanitize the filenames & Makefile
- prepare interval_tree for being built as a module if required
Signed-off-by: Chris Wilson
Cc: Michel Lespinasse
Cc: Rik van Riel
Cc: Peter Zijlstra
Cc: Andrea Arcangeli
Cc: David Woodhouse
Cc: Andrew Morton
Reviewed-by: Michel Lespinasse
[Acked for inclusion via drm/i915 by Andrew Morton.]
---
lib/Kconfig | 14 ++
lib/Kconfig.debug | 1 +
lib/Makefile | 3 +-
lib/interval_tree.c | 6 +++
lib/interval_tree_test.c | 106 ++
lib/interval_tree_test_main.c | 106 --
6 files changed, 128 insertions(+), 108 deletions(-)
create mode 100644 lib/interval_tree_test.c
delete mode 100644 lib/interval_tree_test_main.c
diff --git a/lib/Kconfig b/lib/Kconfig
index 991c98bc4a3f..04270aae4b60 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -322,6 +322,20 @@ config TEXTSEARCH_FSM
config BTREE
boolean
+config INTERVAL_TREE
+ boolean
+ help
+ Simple, embeddable, interval-tree. Can find the start of an
+ overlapping range in log(n) time and then iterate over all
+ overlapping nodes. The algorithm is implemented as an
+ augmented rbtree.
+
+ See:
+
+ Documentation/rbtree.txt
+
+ for more information.
+
config ASSOCIATIVE_ARRAY
bool
help
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a48abeac753f..135a6a0588c9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1487,6 +1487,7 @@ config RBTREE_TEST
config INTERVAL_TREE_TEST
tristate "Interval tree test"
depends on m && DEBUG_KERNEL
+ select INTERVAL_TREE
help
A benchmark measuring the performance of the interval tree library
diff --git a/lib/Makefile b/lib/Makefile
index 48140e3ba73f..0969494f20d1 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -50,6 +50,7 @@ CFLAGS_hweight.o = $(subst
$(quote),,$(CONFIG_ARCH_HWEIGHT_CFLAGS))
obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o
obj-$(CONFIG_BTREE) += btree.o
+obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o
obj-$(CONFIG_ASSOCIATIVE_ARRAY) += assoc_array.o
obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
obj-$(CONFIG_DEBUG_LIST) += list_debug.o
@@ -155,8 +156,6 @@ lib-$(CONFIG_LIBFDT) += $(libfdt_files)
obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o
obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o
-interval_tree_test-objs := interval_tree_test_main.o interval_tree.o
-
obj-$(CONFIG_PERCPU_TEST) += percpu_test.o
obj-$(CONFIG_ASN1) += asn1_decoder.o
diff --git a/lib/interval_tree.c b/lib/interval_tree.c
index e6eb406f2d65..e4109f624e51 100644
--- a/lib/interval_tree.c
+++ b/lib/interval_tree.c
@@ -1,6 +1,7 @@
#include
#include
#include
+#include
#define START(node) ((node)->start)
#define LAST(node) ((node)->last)
@@ -8,3 +9,8 @@
INTERVAL_TREE_DEFINE(struct interval_tree_node, rb,
unsigned long, __subtree_last,
START, LAST,, interval_tree)
+
+EXPORT_SYMBOL(interval_tree_insert);
+EXPORT_SYMBOL(interval_tree_remove);
+EXPORT_SYMBOL(interval_tree_iter_first);
+EXPORT_SYMBOL(interval_tree_iter_next);
diff --git a/lib/interval_tree_test.c b/lib/interval_tree_test.c
new file mode 100644
index ..245900b98c8e
--- /dev/null
+++ b/lib/interval_tree_test.c
@@ -0,0 +1,106 @@
+#include
+#include
+#include
+#include
+
+#define NODES100
+#define PERF_LOOPS 10
+#define SEARCHES 100
+#define SEARCH_LOOPS 1
+
+static struct rb_root root = RB_ROOT;
+static struct interval_tree_node nodes[NODES];
+static u32 queries[SEARCHES];
+
+static struct rnd_state rnd;
+
+static inline unsigned long
+search(unsigned long query, struct rb_root *root)
+{
+ struct interval_tree_node *node;
+ unsigned long results = 0;
+
+ for (node = interval_tree_iter_first(root, query, query); node;
+node = interval_tree_iter_next(node, query, query))
+ results++;
+ return results;
+}
+
+static void init(void)
+{
+ int i;
+ for (i = 0; i < NODES; i++) {
+ u32 a = prandom_u32_state(&rnd);
+ u32 b = prandom_u32_state(&rnd);
+ if (a <= b) {
+ nodes[i].start = a;
+ nodes[i].last = b;
+ } else {
+ nodes[i].start = b;
+ nodes[i].last = a;
+ }
+ }
+ for (i = 0;
[Intel-gfx] [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering
A common issue we have is that retiring requests causes recursion
through GTT manipulation or page table manipulation which we can only
handle at very specific points. However, to maintain internal
consistency (enforced through our sanity checks on write_domain at
various points in the GEM object lifecycle) we do need to retire the
object prior to marking it with a new write_domain, and also clear the
write_domain for the implicit flush following a batch.
Note that this then allows the unbound objects to still be on the active
lists, and so care must be taken when removing objects from unbound lists
(similar to the caveats we face processing the bound lists).
v2: Fix i915_gem_shrink_all() to handle updated object lifetime rules,
by refactoring it to call into __i915_gem_shrink().
v3: Missed an object-retire prior to changing cache domains in
i915_gem_object_set_cache_leve()
v4: Rebase
Signed-off-by: Chris Wilson
Tested-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_gem.c| 112 -
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +
2 files changed, 66 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 58704ce62e3e..5cf4d80de867 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,9 @@ static void i915_gem_object_flush_cpu_write_domain(struct
drm_i915_gem_object *o
static __must_check int
i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
bool readonly);
+static void
+i915_gem_object_retire(struct drm_i915_gem_object *obj);
+
static int i915_gem_phys_pwrite(struct drm_device *dev,
struct drm_i915_gem_object *obj,
struct drm_i915_gem_pwrite *args,
@@ -502,6 +505,8 @@ int i915_gem_obj_prepare_shmem_read(struct
drm_i915_gem_object *obj,
ret = i915_gem_object_wait_rendering(obj, true);
if (ret)
return ret;
+
+ i915_gem_object_retire(obj);
}
ret = i915_gem_object_get_pages(obj);
@@ -917,6 +922,8 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
ret = i915_gem_object_wait_rendering(obj, false);
if (ret)
return ret;
+
+ i915_gem_object_retire(obj);
}
/* Same trick applies to invalidate partially written cachelines read
* before writing. */
@@ -1304,7 +1311,8 @@ static int
i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj,
struct intel_ring_buffer *ring)
{
- i915_gem_retire_requests_ring(ring);
+ if (!obj->active)
+ return 0;
/* Manually manage the write flush as we may have not yet
* retired the buffer.
@@ -1314,7 +1322,6 @@ i915_gem_object_wait_rendering__tail(struct
drm_i915_gem_object *obj,
* we know we have passed the last write.
*/
obj->last_write_seqno = 0;
- obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
return 0;
}
@@ -1949,58 +1956,58 @@ static unsigned long
__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
bool purgeable_only)
{
- struct list_head still_bound_list;
- struct drm_i915_gem_object *obj, *next;
+ struct list_head still_in_list;
+ struct drm_i915_gem_object *obj;
unsigned long count = 0;
- list_for_each_entry_safe(obj, next,
-&dev_priv->mm.unbound_list,
-global_list) {
- if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) &&
- i915_gem_object_put_pages(obj) == 0) {
- count += obj->base.size >> PAGE_SHIFT;
- if (count >= target)
- return count;
- }
- }
-
/*
-* As we may completely rewrite the bound list whilst unbinding
+* As we may completely rewrite the (un)bound list whilst unbinding
* (due to retiring requests) we have to strictly process only
* one element of the list at the time, and recheck the list
* on every iteration.
+*
+* In particular, we must hold a reference whilst removing the
+* object as we may end up waiting for and/or retiring the objects.
+* This might release the final reference (held by the active list)
+* and result in the object being freed from under us. This is
+* similar to the precautions the eviction code must take whilst
+* removing objects.
+*
+* Also note that although these lists do not hold a reference to
+* the object we can safely grab one here: The final object
+* unreferencing and the bound_list are both protected by the
+* dev->struct_mutex and so we won'
[Intel-gfx] [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl
By exporting the ability to map user address and inserting PTEs
representing their backing pages into the GTT, we can exploit UMA in order
to utilize normal application data as a texture source or even as a
render target (depending upon the capabilities of the chipset). This has
a number of uses, with zero-copy downloads to the GPU and efficient
readback making the intermixed streaming of CPU and GPU operations
fairly efficient. This ability has many widespread implications from
faster rendering of client-side software rasterisers (chromium),
mitigation of stalls due to read back (firefox) and to faster pipelining
of texture data (such as pixel buffer objects in GL or data blobs in CL).
v2: Compile with CONFIG_MMU_NOTIFIER
v3: We can sleep while performing invalidate-range, which we can utilise
to drop our page references prior to the kernel manipulating the vma
(for either discard or cloning) and so protect normal users.
v4: Only run the invalidate notifier if the range intercepts the bo.
v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers
v6: Recheck after reacquire mutex for lost mmu.
v7: Fix implicit padding of ioctl struct by rounding to next 64bit boundary.
v8: Fix rebasing error after forwarding porting the back port.
v9: Limit the userptr to page aligned entries. We now expect userspace
to handle all the offset-in-page adjustments itself.
v10: Prevent vma from being copied across fork to avoid issues with cow.
v11: Drop vma behaviour changes -- locking is nigh on impossible.
Use a worker to load user pages to avoid lock inversions.
v12: Use get_task_mm()/mmput() for correct refcounting of mm.
v13: Use a worker to release the mmu_notifier to avoid lock inversion
v14: Decouple mmu_notifier from struct_mutex using a custom mmu_notifer
with its own locking and tree of objects for each mm/mmu_notifier.
v15: Prevent overlapping userptr objects, and invalidate all objects
within the mmu_notifier range
v16: Fix a typo for iterating over multiple objects in the range and
rearrange error path to destroy the mmu_notifier locklessly.
Also close a race between invalidate_range and the get_pages_worker.
v17: Close a race between get_pages_worker/invalidate_range and fresh
allocations of the same userptr range - and notice that
struct_mutex was presumed to be held when during creation it wasn't.
v18: Sigh. Fix the refactor of st_set_pages() to allocate enough memory
for the struct sg_table and to clear it before reporting an error.
v19: Always error out on read-only userptr requests as we don't have the
hardware infrastructure to support them at the moment.
v20: Refuse to implement read-only support until we have the required
infrastructure - but reserve the bit in flags for future use.
v21: use_mm() is not required for get_user_pages(). It is only meant to
be used to fix up the kernel thread's current->mm for use with
copy_user().
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: "Gong, Zhipeng"
Cc: Akash Goel
Cc: "Volkin, Bradley D"
Reviewed-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/Kconfig| 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_dma.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 24 +-
drivers/gpu/drm/i915/i915_gem.c | 4 +
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 5 +
drivers/gpu/drm/i915/i915_gem_userptr.c | 679
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +
include/uapi/drm/i915_drm.h | 16 +
9 files changed, 732 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 73ed59eff139..9940baee10c2 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -5,6 +5,7 @@ config DRM_I915
depends on (AGP || AGP=n)
select INTEL_GTT
select AGP_INTEL if AGP
+ select INTERVAL_TREE
# we need shmfs for the swappable backing store, and in particular
# the shmem_readpage() which depends upon tmpfs
select SHMEM
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 2e02137a0d6e..1856494439e9 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -27,6 +27,7 @@ i915-y += i915_cmd_parser.o \
i915_gem.o \
i915_gem_stolen.o \
i915_gem_tiling.o \
+ i915_gem_userptr.o \
i915_gpu_error.o \
i915_irq.o \
i915_trace_points.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a98d66fb0302..4f7b93e2cb4f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1979,6 +1979,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(
[Intel-gfx] [PULL] drm-intel-next
Hi Dave, drm-intel-next-2014-03-07: - fine-grained display power domains for byt (Imre) - runtime pm prep patches for !hsw from Paulo - WiZ hashing flag updates from Ville - ppgtt setup cleanup and enabling of full 4G range on bdw (Ben) - fixes from Jesse for the inherited intial config code - gpu reset code improvements from Mika - per-pipe num_planes refactoring from Damien - stability fixes around bdw forcewake handling and other bdw w/a from Mika and Ken - and as usual a pile of smaller fixes all over The big thing here is the disabling of the full ppgtt code. I've figured I'll need to pull the plug a bit earlier to have enough time to test thing throughroughly before the 3.15 merge window opens. Cheers, Daniel The following changes since commit 4c0e552882114d1edb588242d45035246ab078a0: drm/i915: fix NULL deref in the load detect code (2014-02-14 17:23:12 +0100) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2014-03-07 for you to fetch changes up to 2fae6a860ca9adb0c881f6dcd633df775c2520e9: drm/i915: Go OCD on the Makefile (2014-03-07 22:37:00 +0100) - fine-grained display power domains for byt (Imre) - runtime pm prep patches for !hsw from Paulo - WiZ hashing flag updates from Ville - ppgtt setup cleanup and enabling of full 4G range on bdw (Ben) - fixes from Jesse for the inherited intial config code - gpu reset code improvements from Mika - per-pipe num_planes refactoring from Damien - stability fixes around bdw forcewake handling and other bdw w/a from Mika and Ken - and as usual a pile of smaller fixes all over Ben Widawsky (12): drm/i915: Move ppgtt_release out of the header drm/i915/bdw: Free PPGTT struct drm/i915/bdw: Reorganize PPGTT init drm/i915/bdw: Split ppgtt initialization up drm/i915: Make clear/insert vfuncs args absolute drm/i915/bdw: Reorganize PT allocations Revert "drm/i915/bdw: Limit GTT to 2GB" drm/i915: Update i915_gem_gtt.c copyright drm/i915: Split GEN6 PPGTT cleanup drm/i915: Split GEN6 PPGTT initialization up drm/i915/bdw: Kill ppgtt->num_pt_pages drm/i915/bdw: Add FBC support Brad Volkin (2): drm/i915: Refactor shmem pread setup drm/i915: Implement command buffer parsing logic Chris Wilson (9): drm/i915: Revert workaround for disabling L3 cache aging on IVB Revert "drm/i915: enable HiZ Raw Stall Optimization on IVB" drm/i915: Reject changes of fb base when we have a flip pending drm/i915: Accurately track when we mark the hardware as idle/busy drm/i915: Convert the forcewake worker into a timer func drm/i915: Perform pageflip using mmio if the GPU is terminally wedged drm/i915: Reset vma->mm_list after unbinding drm/i915: Rely on accurate request tracking for finding hung batches drm/i915: Record pid/comm of hanging task Damien Lespiau (8): drm/i915: Use a pipe variable to cycle through the pipes drm/i915: Don't declare unnecessary shadowing variable drm/i915: Replace a few for_each_pipe(i) by for_each_pipe(pipe) drm/i915: Add a for_each_sprite() macro drm/i915: Make num_sprites a per-pipe value drm/i915: Make i915_gem_retire_requests_ring() static drm/i915: Remove unused to_gem_object() macro drm/i915: Fix i915_switch_context() argument name in kerneldoc Daniel Vetter (7): drm/i915: tune down user-triggerable dmesg noise in the cursor/overlay code drm/i915: sprinkle static drm/i915: s/any_enabled/!fallback/ in fbdev_initial_config drm/i915: ignore bios output config if not all outputs are on drm/i915: reverse dp link param selection, prefer fast over wide again drm/i915: Disable full ppgtt by default drm/i915: Go OCD on the Makefile Imre Deak (21): drm/i915: use drm_i915_private everywhere in the power domain api drm/i915: switch order of power domain init wrt. irq install drm/i915: use power domain api to check vga power state drm/i915: move hsw power domain comment to its right place drm/i915: fold in __intel_power_well_get/put functions drm/i915: move modeset_update_power_wells earlier drm/i915: move power domain macros to intel_pm.c drm/i915: add init power domain to always-on power wells drm/i915: split power well 'set' handler to separate enable/disable/sync_hw drm/i915: add noop power well handlers instead of NULL checking them drm/i915: add port power domains drm/i915: get port power domain in connector detect handlers drm/i915: check port power domain when reading the encoder hw state drm/i915: check pipe power domain when reading its hw state drm/i915: vlv: keep first level vblank IRQs masked drm/i915: sanitize PUNIT register macro defin
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, Two 3.14 specific fixes, two cc: stable. I'm afraid I've accumulated a slight backlog of fixes. This pull is to get the tested stuff out of the way first. I'll be pushing more patches soon, and will send another pull once our QA has had some time to run them. BR, Jani. The following changes since commit 6375b768a9850b6154478993e5fb566fa4614a9c: drm/i915: Reject >165MHz modes w/ DVI monitors (2014-03-03 19:08:08 +0200) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-03-17 for you to fetch changes up to 5c673b60a9b3b23486f4eda75c72e91d31d26a2b: drm/i915: Don't enable display error interrupts from the start (2014-03-12 17:20:34 +0200) Ben Widawsky (1): drm/i915: Fix PSR programming Daniel Vetter (1): drm/i915: Don't enable display error interrupts from the start Ville Syrjälä (2): drm/i915: Add a workaround for HSW scanline counter weirdness drm/i915: Fix scanline counter fixup on BDW drivers/gpu/drm/i915/i915_irq.c | 71 ++- drivers/gpu/drm/i915/intel_dp.c |2 +- 2 files changed, 42 insertions(+), 31 deletions(-) -- Jani Nikula, Intel Open Source Technology Center pgpHlgXBjRz8n.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't del_timer_sync uninitialized timer
On Mon, Mar 17, 2014 at 11:14:25AM +0200, Jani Nikula wrote: > On Mon, 17 Mar 2014, Jani Nikula wrote: > > On Sat, 15 Mar 2014, Ben Widawsky wrote: > >> Broken by: > >> commit 0294ae7b44bba7ab0d4cef9a8736287f38bdb4fd > >> Author: Chris Wilson > >> Date: Thu Mar 13 12:00:29 2014 + > >> > >> drm/i915: Consolidate forcewake resetting to a single function > >> > > > > Does this result in https://bugs.freedesktop.org/show_bug.cgi?id=76242 ? > > QA confirms this patch fixes the bug. Also the commit message seriously lacks details. Apparantly this only blows up with lockdep (since most my machines here are still happy), which is a rather crucial detail. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Added a new 'I915_CPU_MAP_NOT_NEEDED' flag to gem_create ioctl.
On Mon, 2014-03-10 at 22:07 +, 'Chris Wilson' wrote: > On Mon, Mar 10, 2014 at 04:12:23PM +, Gupta, Sourab wrote: > > > > Hi Chris, > > > > For the issue mentioned by you ( regarding botching up ioctls), we > > understand that this is related to the > > compatibility between the older/newer versions of driver/userspace. > > In our old implementation, the 'pad' field was replaced with 'flags' in the > > ioctl structure. This would have > > led to the erroneous behavior when the new userspace is communicating with > > old driver. > > You missed the important point that there is no guarrantee that current > userspace is not stuffing garbage into the pad field. It is. > > You have to go with a new ioctl. So you may as well design one that > tries to fulfil today's varied needs for a constructor. For example, > here's one I prepared earlier, > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=create2&id=401fa740adcaf252d0149cdd63d5fdf5e3969907 > -Chris > Hi Chris, We went through this patch and this fulfills the requirements from the stolen mem perspective, and this looks like a replacement of the first patch we have submitted. This patch seems to be in your private branch. When do you plan to merge this patch with drm-intel branch? Have you also prepared the corresponding libdrm side patches? If so, can you share those also. Actually, the igt patch which we submitted earlier would also need to be modified according to the libdrm changes. Also, When do you plan to merge the second and third patch in the series (truncation of stolen mem objs) as they seem to be fine to us. Regards, Sourab ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't del_timer_sync uninitialized timer
On Mon, 17 Mar 2014, Jani Nikula wrote: > On Sat, 15 Mar 2014, Ben Widawsky wrote: >> Broken by: >> commit 0294ae7b44bba7ab0d4cef9a8736287f38bdb4fd >> Author: Chris Wilson >> Date: Thu Mar 13 12:00:29 2014 + >> >> drm/i915: Consolidate forcewake resetting to a single function >> > > Does this result in https://bugs.freedesktop.org/show_bug.cgi?id=76242 ? QA confirms this patch fixes the bug. Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] yet another modesetting backtrace.
On Mon, Mar 17, 2014 at 12:51:24PM +1000, Dave Airlie wrote: > Booted drm-next merged into Linus 3.14-rc7 this morning, > > got splatted. > > Ivybridge, one DP one DVI monitor. Oh, the infamous. We have a report of these for pretty much all our our hw platforms up to ivb (I think hsw is fine actually). Played around with piles of things to no avail thus far. For 3.15 there's two patches from Imre which might improve this mess (since in general if the pipe stops running when we don't expect it to our code will hang all over the place since we don't have a hangcheck for the display hardware): 2e82a7203182d0883d0f9450d40ad6e1c6578ad9 drm/i915: don't disable DP port after a failed link training 5d6a1116c6475404e6505b708320f9579ae19acd drm/i915: don't disable the DP port if the link is lost Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/1] kms_cursor_crc: Enabling this test for 128x128 and 256x256 cursors
On Sat, Mar 8, 2014 at 7:49 PM, wrote:
> From: Sagar Kamble
>
> v1: Added 128x128 and 256x256 cursor size support.
>
> v2: Refined the test to use igt_subtest_f and automate enumeration.
>
> Signed-off-by: Sagar Kamble
Hm, do you have an update of this patch to use the drm ioctl to query
the max cursor size and correctly skip 128x128 and higher?
Also adding the additional modes into an enum looks a bit strange. I'm
it would be simpler to explicit pass the cursor size and have an outer
loop over all of them, i.e.
for (cursor_size = 64 ; cursor_size <= 256; cursor_size *= 2) {
igt_require(cusror_max >= cursor_size);
/* lists of all the existing subtests with a
igt_subtest_f("subtest-%s", cursor_size) */
}
Adding all the indirections with the enum switches makes the code imo
harder to read, and for testcases we want to aim for simple
straightforward code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't del_timer_sync uninitialized timer
On Sat, 15 Mar 2014, Ben Widawsky wrote:
> Broken by:
> commit 0294ae7b44bba7ab0d4cef9a8736287f38bdb4fd
> Author: Chris Wilson
> Date: Thu Mar 13 12:00:29 2014 +
>
> drm/i915: Consolidate forcewake resetting to a single function
>
Does this result in https://bugs.freedesktop.org/show_bug.cgi?id=76242 ?
Backtrace in the commit message would help a lot in pattern matching...
BR,
Jani.
> Cc: Chris Wilson
> Cc: Mika Kuoppala
> Signed-off-by: Ben Widawsky
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index e6bb421..7e55ceb 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -362,6 +362,9 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> + setup_timer(&dev_priv->uncore.force_wake_timer,
> + gen6_force_wake_timer, (unsigned long)dev_priv);
> +
> if (HAS_FPGA_DBG_UNCLAIMED(dev))
> __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
>
> @@ -724,9 +727,6 @@ void intel_uncore_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - setup_timer(&dev_priv->uncore.force_wake_timer,
> - gen6_force_wake_timer, (unsigned long)dev_priv);
> -
> if (IS_VALLEYVIEW(dev)) {
> dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
> dev_priv->uncore.funcs.force_wake_put = __vlv_force_wake_put;
> --
> 1.9.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
