Re: [Intel-gfx] [PATCH] igt: Correct the return value for drm short_buffer read
On Tue, Dec 23, 2014 at 03:52:11PM +0800, Xiong Zhang wrote: After i915 commit: commit bd008e5b2953186fc0c6633a885ade95e7043800 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Tue Oct 7 14:13:51 2014 +0100 drm: Implement O_NONBLOCK support on /dev/dri/cardN the return value for drm short_buffer read is -1 and errno is EAGAIN. No, it is not. -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 2/5] drm/i915: Move vblank evasion to commit (v3)
On 12/16/2014 02:23 AM, Matt Roper wrote:
Move the vblank evasion up from the low-level, hw-specific
update_plane() handlers to the general plane commit operation.
Everything inside commit should now be non-sleeping, so this brings us
closer to how vblank evasion will behave once we move over to atomic.
v2:
- Restore lost intel_crtc-active check on vblank evasion
v3:
- Replace assert_pipe_enabled() in intel_disable_primary_hw_plane()
with an intel_crtc-active test; it turns out assert_pipe_enabled()
grabs some mutexes and can sleep, which we can't do with interrupts
disabled.
Sounds like this should have gone in the previous patch, or a separate one.
Ander
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 13 ++-
drivers/gpu/drm/i915/intel_drv.h | 4
drivers/gpu/drm/i915/intel_sprite.c | 42
3 files changed, 16 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5d90114..ce552d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2165,7 +2165,8 @@ static void intel_disable_primary_hw_plane(struct
drm_plane *plane,
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- assert_pipe_enabled(dev_priv, intel_crtc-pipe);
+ if (WARN_ON(!intel_crtc-active))
+ return;
if (!intel_crtc-primary_enabled)
return;
@@ -11861,6 +11862,12 @@ static void intel_begin_crtc_commit(struct drm_crtc
*crtc)
if (intel_crtc-atomic.update_wm)
intel_update_watermarks(crtc);
+
+ /* Perform vblank evasion around commit operation */
+ if (intel_crtc-active)
+ intel_crtc-atomic.evade =
+ intel_pipe_update_start(intel_crtc,
+
intel_crtc-atomic.start_vbl_count);
}
static void intel_finish_crtc_commit(struct drm_crtc *crtc)
@@ -11869,6 +11876,10 @@ static void intel_finish_crtc_commit(struct drm_crtc
*crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *p;
+ if (intel_crtc-atomic.evade)
+ intel_pipe_update_end(intel_crtc,
+ intel_crtc-atomic.start_vbl_count);
+
if (intel_crtc-atomic.wait_vblank)
intel_wait_for_vblank(dev, intel_crtc-pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a03bd72..1934156 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -428,6 +428,10 @@ struct skl_pipe_wm {
* and thus can't be run with interrupts disabled.
*/
struct intel_crtc_atomic_commit {
+ /* vblank evasion */
+ bool evade;
+ unsigned start_vbl_count;
+
/* Sleepable operations to perform before commit */
bool wait_for_flips;
bool disable_fbc;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index ff7d6a1..2520748 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -412,8 +412,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
- u32 start_vbl_count;
- bool atomic_update;
sprctl = I915_READ(SPCNTR(pipe, plane));
@@ -502,8 +500,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
linear_offset += src_h * fb-pitches[0] + src_w * pixel_size;
}
- atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
-
intel_update_primary_plane(intel_crtc);
if (IS_CHERRYVIEW(dev) pipe == PIPE_B)
@@ -525,9 +521,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
sprsurf_offset);
intel_flush_primary_plane(dev_priv, intel_crtc-plane);
-
- if (atomic_update)
- intel_pipe_update_end(intel_crtc, start_vbl_count);
}
static void
@@ -539,10 +532,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct
drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_plane-pipe;
int plane = intel_plane-plane;
- u32 start_vbl_count;
- bool atomic_update;
-
- atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
intel_update_primary_plane(intel_crtc);
@@ -553,9 +542,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc
*crtc)
intel_flush_primary_plane(dev_priv, intel_crtc-plane);
- if (atomic_update)
- intel_pipe_update_end(intel_crtc, start_vbl_count);
-
intel_update_sprite_watermarks(dplane, crtc, 0,
Re: [Intel-gfx] [PATCH 3/5] drm/i915: Clarify sprite plane function names (v3)
On 12/16/2014 02:23 AM, Matt Roper wrote:
A few of the sprite-related function names in i915 are very similar
(e.g., intel_enable_planes() vs intel_crtc_enable_planes()) and don't
make it clear whether they only operate on sprite planes, or whether
they also apply to all universal plane types. Rename a few functions to
be more consistent with our function naming for primary/cursor planes or
to clarify that they apply specifically to sprite planes:
- s/intel_disable_planes/intel_disable_sprite_planes/
- s/intel_enable_planes/intel_enable_sprite_planes/
Also, drop the sprite-specific intel_destroy_plane() and just use
the type-agnostic intel_plane_destroy() function. The extra 'disable'
call that intel_destroy_plane() did is unnecessary since the plane will
already be disabled due to framebuffer destruction by the point it gets
called.
v2: Earlier consolidation patches have reduced the number of functions
we need to rename here.
v3: Also rename intel_plane_funcs vtable to intel_sprite_plane_funcs
for consistency with primary/cursor. (Ander)
Reviewed-by(v1): Bob Paauwe bob.j.paa...@intel.com
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 10 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 14 +++---
3 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index ce552d1..030cf93 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4037,7 +4037,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
}
}
-static void intel_enable_planes(struct drm_crtc *crtc)
+static void intel_enable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
enum pipe pipe = to_intel_crtc(crtc)-pipe;
@@ -4051,7 +4051,7 @@ static void intel_enable_planes(struct drm_crtc *crtc)
}
}
-static void intel_disable_planes(struct drm_crtc *crtc)
+static void intel_disable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
enum pipe pipe = to_intel_crtc(crtc)-pipe;
@@ -4195,7 +4195,7 @@ static void intel_crtc_enable_planes(struct drm_crtc
*crtc)
int pipe = intel_crtc-pipe;
intel_enable_primary_hw_plane(crtc-primary, crtc);
- intel_enable_planes(crtc);
+ intel_enable_sprite_planes(crtc);
intel_crtc_update_cursor(crtc, true);
intel_crtc_dpms_overlay(intel_crtc, true);
@@ -4230,7 +4230,7 @@ static void intel_crtc_disable_planes(struct drm_crtc
*crtc)
intel_crtc_dpms_overlay(intel_crtc, false);
intel_crtc_update_cursor(crtc, false);
- intel_disable_planes(crtc);
+ intel_disable_sprite_planes(crtc);
intel_disable_primary_hw_plane(crtc-primary, crtc);
/*
@@ -12008,7 +12008,7 @@ intel_disable_plane(struct drm_plane *plane)
}
/* Common destruction function for both primary and cursor planes */
-static void intel_plane_destroy(struct drm_plane *plane)
+void intel_plane_destroy(struct drm_plane *plane)
Do we need kernel doc for this?
Ander
{
struct intel_plane *intel_plane = to_intel_plane(plane);
drm_plane_cleanup(plane);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1934156..2523315 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1053,6 +1053,7 @@ int intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
int intel_disable_plane(struct drm_plane *plane);
+void intel_plane_destroy(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int
conn_id);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 2520748..a689c73 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1286,14 +1286,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static void intel_destroy_plane(struct drm_plane *plane)
-{
- struct intel_plane *intel_plane = to_intel_plane(plane);
- intel_disable_plane(plane);
- drm_plane_cleanup(plane);
- kfree(intel_plane);
-}
-
int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -1393,10 +1385,10 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane-src_w, intel_plane-src_h);
}
-static const struct drm_plane_funcs intel_plane_funcs = {
+static const struct drm_plane_funcs intel_sprite_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
- .destroy = intel_destroy_plane,
+
Re: [Intel-gfx] [PATCH] igt: Correct the return value for drm short_buffer read
-Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, December 23, 2014 5:53 PM To: Zhang, Xiong Y Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] igt: Correct the return value for drm short_buffer read On Tue, Dec 23, 2014 at 03:52:11PM +0800, Xiong Zhang wrote: After i915 commit: commit bd008e5b2953186fc0c6633a885ade95e7043800 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Tue Oct 7 14:13:51 2014 +0100 drm: Implement O_NONBLOCK support on /dev/dri/cardN the return value for drm short_buffer read is -1 and errno is EAGAIN. No, it is not. -Chris Without this patch, system fail in short-buffer-block and short-buffer-nonblock subtest. With this patch, these two subtest could pass. -- 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] igt: Correct the return value for drm short_buffer read
On Tue, Dec 23, 2014 at 10:14:15AM +, Zhang, Xiong Y wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, December 23, 2014 5:53 PM To: Zhang, Xiong Y Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] igt: Correct the return value for drm short_buffer read On Tue, Dec 23, 2014 at 03:52:11PM +0800, Xiong Zhang wrote: After i915 commit: commit bd008e5b2953186fc0c6633a885ade95e7043800 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Tue Oct 7 14:13:51 2014 +0100 drm: Implement O_NONBLOCK support on /dev/dri/cardN the return value for drm short_buffer read is -1 and errno is EAGAIN. No, it is not. -Chris Without this patch, system fail in short-buffer-block and short-buffer-nonblock subtest. With this patch, these two subtest could pass. That's the point of the test, the kernel behaviour is wrong. There is a patch to fix the kernel. -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
[Intel-gfx] [PATCH 2/9] drm/i915: fix the FBC CFB size tracking
From: Paulo Zanoni paulo.r.zan...@intel.com
We have dev_priv-fbc.size which is supposed to contain the compressed
FB size, but it is not: at find_compression_threshold() we try to
overallocate the CFB, but we don't consider this when we assign a
value to dev_priv-fbc.size. Since the correct CFB size should already
be stored at dev_priv-fbc.compressed_fb.size, just kill
dev_priv-fbc.size and use the correct value isntead.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h| 1 -
drivers/gpu/drm/i915/i915_gem_stolen.c | 13 +
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3752040..f0419c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -734,7 +734,6 @@ enum fb_op_origin {
};
struct i915_fbc {
- unsigned long size;
unsigned threshold;
unsigned int fb_id;
unsigned int possible_framebuffer_bits;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 4797138..d02c102 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -231,10 +231,8 @@ static int i915_setup_compression(struct drm_device *dev,
int size, int fb_cpp)
dev_priv-mm.stolen_base + compressed_llb-start);
}
- dev_priv-fbc.size = size / dev_priv-fbc.threshold;
-
- DRM_DEBUG_KMS(reserved %d bytes of contiguous stolen space for FBC\n,
- size);
+ DRM_DEBUG_KMS(reserved %lu bytes of contiguous stolen space for FBC\n,
+ dev_priv-fbc.compressed_fb.size);
return 0;
@@ -253,7 +251,8 @@ int i915_gem_stolen_setup_compression(struct drm_device
*dev, int size, int fb_c
if (!drm_mm_initialized(dev_priv-mm.stolen))
return -ENODEV;
- if (size = dev_priv-fbc.size)
+ if (dev_priv-fbc.compressed_fb.allocated
+ size = dev_priv-fbc.compressed_fb.size)
return 0;
/* Release any current block */
@@ -266,7 +265,7 @@ void i915_gem_stolen_cleanup_compression(struct drm_device
*dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
- if (dev_priv-fbc.size == 0)
+ if (dev_priv-fbc.compressed_fb.allocated == 0)
return;
drm_mm_remove_node(dev_priv-fbc.compressed_fb);
@@ -275,8 +274,6 @@ void i915_gem_stolen_cleanup_compression(struct drm_device
*dev)
drm_mm_remove_node(dev_priv-fbc.compressed_llb);
kfree(dev_priv-fbc.compressed_llb);
}
-
- dev_priv-fbc.size = 0;
}
void i915_gem_cleanup_stolen(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index d3ff2c1..5270dc4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -78,7 +78,7 @@ static void i8xx_fbc_enable(struct drm_crtc *crtc)
dev_priv-fbc.enabled = true;
- cfb_pitch = dev_priv-fbc.size / FBC_LL_SIZE;
+ cfb_pitch = dev_priv-fbc.compressed_fb.size / FBC_LL_SIZE;
if (fb-pitches[0] cfb_pitch)
cfb_pitch = fb-pitches[0];
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/9] More simple FBC fixes
From: Paulo Zanoni paulo.r.zan...@intel.com Hi Since the previous FBC series is already entirely reviewed, it's time to submit a few more FBC patches to the mailing list. These patches just fix some simple bugs and annoyances: nothing really major. I tested them on my BDW and they pass the tests I have. In theory we could even think about maybe enabling FBC on HSW+ since I can't think of any bugs remaining on these platforms. The biggest reason to prevent that is that I still didn't upstream the FBC tests I wrote since I still didn't integrate them to kms_fbc_crc - which I'm also running. This doesn't mean FBC's TODO list is empty: there are still a lot of small improvements to do, and one of them is to _not_ disable FBC during page flips. I also didn't really do any power or performance tests yet: the focus is completely on bugs. Regarding the previous platforms, I think there could be some hope to support FBC on ILK+, but I'm really only focused on HSW+, so I don't know if we're missing some checks and restrictions. For the older platforms, I think that, given all the conditions, the risk of enabling FBC and breaking these platforms even more is probably not worth it. Thanks, Paulo Paulo Zanoni (9): drm/i915: don't reallocate the compressed FB at every frame drm/i915: fix the FBC CFB size tracking drm/i915: don't increment the FBC threshold at fbc_enable drm/i915: don't free the CFB while FBC is enabled drm/i915: don't set the FBC plane select bits on HSW+ drm/i915: add the FBC mutex drm/i915: don't alloc/free fbc_work at every update drm/i915: print FBC compression status on debugfs drm/i915: FBC only supports 16bpp and 32bpp drivers/gpu/drm/i915/i915_debugfs.c| 5 + drivers/gpu/drm/i915/i915_drv.h| 5 +- drivers/gpu/drm/i915/i915_gem_stolen.c | 16 ++-- drivers/gpu/drm/i915/i915_reg.h| 3 + drivers/gpu/drm/i915/intel_fbc.c | 161 + 5 files changed, 123 insertions(+), 67 deletions(-) -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/9] drm/i915: don't free the CFB while FBC is enabled
From: Paulo Zanoni paulo.r.zan...@intel.com
Because that is probably not very a good idea: if we used the stolen
memory for more things, there could be a risk that someone would
allocate the memory that the HW is still using as the CFB while FBC
was still enabled.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 3 +++
drivers/gpu/drm/i915/intel_fbc.c | 14 +++---
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index d02c102..f84c5f3 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -198,6 +198,8 @@ static int i915_setup_compression(struct drm_device *dev,
int size, int fb_cpp)
struct drm_mm_node *uninitialized_var(compressed_llb);
int ret;
+ WARN_ON(dev_priv-fbc.enabled);
+
ret = find_compression_threshold(dev, dev_priv-fbc.compressed_fb,
size, fb_cpp);
if (!ret)
@@ -268,6 +270,7 @@ void i915_gem_stolen_cleanup_compression(struct drm_device
*dev)
if (dev_priv-fbc.compressed_fb.allocated == 0)
return;
+ WARN_ON(dev_priv-fbc.enabled);
drm_mm_remove_node(dev_priv-fbc.compressed_fb);
if (dev_priv-fbc.compressed_llb) {
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1b10b06..83d3c8a 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -593,13 +593,6 @@ void intel_fbc_update(struct drm_device *dev)
if (in_dbg_master())
goto out_disable;
- if (i915_gem_stolen_setup_compression(dev, obj-base.size,
-
drm_format_plane_cpp(fb-pixel_format, 0))) {
- if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
- DRM_DEBUG_KMS(framebuffer too large, disabling
compression\n);
- goto out_disable;
- }
-
/* If the scanout has not changed, don't modify the FBC settings.
* Note that we make the fundamental assumption that the fb-obj
* cannot be unpinned (and have its GTT offset and fence revoked)
@@ -638,6 +631,13 @@ void intel_fbc_update(struct drm_device *dev)
intel_fbc_disable(dev);
}
+ if (i915_gem_stolen_setup_compression(dev, obj-base.size,
+
drm_format_plane_cpp(fb-pixel_format, 0))) {
+ if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
+ DRM_DEBUG_KMS(framebuffer too large, disabling
compression\n);
+ return;
+ }
+
intel_fbc_enable(crtc);
dev_priv-fbc.no_fbc_reason = FBC_OK;
return;
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/9] drm/i915: print FBC compression status on debugfs
From: Paulo Zanoni paulo.r.zan...@intel.com
We already had a few bugs in the past where FBC was compressing
nothing when it was enabled, which makes the feature quite useless.
Add this information on debugfs so the test suites can check for
regressions in this piece of the code.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 +
drivers/gpu/drm/i915/i915_reg.h | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index e515aad..e34e021 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1510,6 +1510,11 @@ static int i915_fbc_status(struct seq_file *m, void
*unused)
seq_putc(m, '\n');
}
+ if (INTEL_INFO(dev_priv)-gen = 7 !IS_VALLEYVIEW(dev_priv))
+ seq_printf(m, Compressing: %s\n,
+ yesno(I915_READ(FBC_STATUS2)
+FBC_COMPRESSION_MASK));
+
intel_runtime_pm_put(dev_priv);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 40ca873..b6db966 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1600,6 +1600,9 @@ enum punit_power_well {
#define FBC_FENCE_OFF 0x03218 /* BSpec typo has 321Bh */
#define FBC_TAG0x03300
+#define FBC_STATUS20x43214
+#define FBC_COMPRESSION_MASK 0x7ff
+
#define FBC_LL_SIZE(1536)
/* Framebuffer compression for GM45+ */
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/9] drm/i915: don't increment the FBC threshold at fbc_enable
From: Paulo Zanoni paulo.r.zan...@intel.com
We first set the threshold value when we're allocating the CFB, and
then later at {ilk,gen7}_fbc_enable() we increment it in case we're
using 16bpp. While that is correct, it is dangerous: if we rework the
code a little bit in a way that allows us to call intel_fbc_enable()
without necessarily calling i915_gem_stolen_setup_compression() first,
we might end up incrementing threshold more than once. To prevent
that, increment a temporary variable instead.
We're going to need this patch for a later rework.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_fbc.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 5270dc4..1b10b06 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -187,14 +187,15 @@ static void ilk_fbc_enable(struct drm_crtc *crtc)
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
u32 dpfc_ctl;
+ int threshold = dev_priv-fbc.threshold;
dev_priv-fbc.enabled = true;
dpfc_ctl = DPFC_CTL_PLANE(intel_crtc-plane);
if (drm_format_plane_cpp(fb-pixel_format, 0) == 2)
- dev_priv-fbc.threshold++;
+ threshold++;
- switch (dev_priv-fbc.threshold) {
+ switch (threshold) {
case 4:
case 3:
dpfc_ctl |= DPFC_CTL_LIMIT_4X;
@@ -258,14 +259,15 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
u32 dpfc_ctl;
+ int threshold = dev_priv-fbc.threshold;
dev_priv-fbc.enabled = true;
dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc-plane);
if (drm_format_plane_cpp(fb-pixel_format, 0) == 2)
- dev_priv-fbc.threshold++;
+ threshold++;
- switch (dev_priv-fbc.threshold) {
+ switch (threshold) {
case 4:
case 3:
dpfc_ctl |= DPFC_CTL_LIMIT_4X;
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915: don't alloc/free fbc_work at every update
From: Paulo Zanoni paulo.r.zan...@intel.com
Because there's no need for it. Just use a static structure with a
bool field to tell if it's in use or not. The big advantage here is
not saving kzalloc/kfree calls, it's cutting the ugly failed to
allocate FBC work structure code path: in this path we call
enable_fbc() directly but we don't update fbc.crtc, fbc.fb_id and
fbc.y - they are updated in intel_fbc_work_fn(), which we're not
calling. And since testing out-of-memory cases like this is really
hard, getting rid of the code path is a major relief.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_fbc.c | 41 +++-
2 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18fcce4..40bc864 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -751,10 +751,11 @@ struct i915_fbc {
bool enabled;
struct intel_fbc_work {
+ bool scheduled;
struct delayed_work work;
struct drm_crtc *crtc;
struct drm_framebuffer *fb;
- } *fbc_work;
+ } work;
enum no_fbc_reason {
FBC_OK, /* FBC is enabled */
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 6611266..80bdbde 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -336,7 +336,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
mutex_lock(dev-struct_mutex);
mutex_lock(dev_priv-fbc.lock);
- if (work == dev_priv-fbc.fbc_work) {
+ if (dev_priv-fbc.work.scheduled) {
/* Double check that we haven't switched fb without cancelling
* the prior work.
*/
@@ -348,42 +348,37 @@ static void intel_fbc_work_fn(struct work_struct *__work)
dev_priv-fbc.y = work-crtc-y;
}
- dev_priv-fbc.fbc_work = NULL;
+ dev_priv-fbc.work.scheduled = false;
}
mutex_unlock(dev_priv-fbc.lock);
mutex_unlock(dev-struct_mutex);
-
- kfree(work);
}
static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv)
{
lockdep_assert_held(dev_priv-fbc.lock);
- if (dev_priv-fbc.fbc_work == NULL)
+ if (!dev_priv-fbc.work.scheduled)
return;
DRM_DEBUG_KMS(cancelling pending FBC enable\n);
- /* Synchronisation is provided by struct_mutex and checking of
-* dev_priv-fbc.fbc_work, so we can perform the cancellation
+ /* Synchronisation is provided by fbc.lock and checking of
+* dev_priv-fbc.work.scheduled, so we can perform the cancellation
* entirely asynchronously.
*/
- if (cancel_delayed_work(dev_priv-fbc.fbc_work-work))
- /* tasklet was killed before being run, clean up */
- kfree(dev_priv-fbc.fbc_work);
+ cancel_delayed_work(dev_priv-fbc.work.work);
/* Mark the work as no longer wanted so that if it does
* wake-up (because the work was already running and waiting
* for our mutex), it will discover that is no longer
* necessary to run.
*/
- dev_priv-fbc.fbc_work = NULL;
+ dev_priv-fbc.work.scheduled = false;
}
static void intel_fbc_enable(struct drm_crtc *crtc)
{
- struct intel_fbc_work *work;
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -394,18 +389,10 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
intel_fbc_cancel_work(dev_priv);
- work = kzalloc(sizeof(*work), GFP_KERNEL);
- if (work == NULL) {
- DRM_ERROR(Failed to allocate FBC work structure\n);
- dev_priv-display.enable_fbc(crtc);
- return;
- }
+ dev_priv-fbc.work.crtc = crtc;
+ dev_priv-fbc.work.fb = crtc-primary-fb;
- work-crtc = crtc;
- work-fb = crtc-primary-fb;
- INIT_DELAYED_WORK(work-work, intel_fbc_work_fn);
-
- dev_priv-fbc.fbc_work = work;
+ dev_priv-fbc.work.scheduled = true;
/* Delay the actual enabling to let pageflipping cease and the
* display to settle before starting the compression. Note that
@@ -420,7 +407,7 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
*
* WaFbcWaitForVBlankBeforeEnable:ilk,snb
*/
- schedule_delayed_work(work-work, msecs_to_jiffies(50));
+ schedule_delayed_work(dev_priv-fbc.work.work, msecs_to_jiffies(50));
}
static void __intel_fbc_disable(struct drm_device *dev)
@@ -702,9 +689,9 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
if (dev_priv-fbc.enabled)
fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv-fbc.crtc-pipe);
- else
[Intel-gfx] [PATCH 9/9] drm/i915: FBC only supports 16bpp and 32bpp
From: Paulo Zanoni paulo.r.zan...@intel.com
So check for this.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_fbc.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 80bdbde..8c6fb1d 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -515,6 +515,7 @@ static void __intel_fbc_update(struct drm_device *dev)
struct drm_i915_gem_object *obj;
const struct drm_display_mode *adjusted_mode;
unsigned int max_width, max_height;
+ int fb_cpp;
lockdep_assert_held(dev_priv-fbc.lock);
@@ -598,6 +599,13 @@ static void __intel_fbc_update(struct drm_device *dev)
goto out_disable;
}
+ fb_cpp = drm_format_plane_cpp(fb-pixel_format, 0);
+ if (!(fb_cpp == 2 || fb_cpp == 4)) {
+ if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE));
+ DRM_DEBUG_KMS(FBC only supports 16bpp and 32bpp\n);
+ goto out_disable;
+ }
+
/* If the kernel debugger is active, always disable compression */
if (in_dbg_master())
goto out_disable;
@@ -640,8 +648,7 @@ static void __intel_fbc_update(struct drm_device *dev)
__intel_fbc_disable(dev);
}
- if (i915_gem_stolen_setup_compression(dev, obj-base.size,
-
drm_format_plane_cpp(fb-pixel_format, 0))) {
+ if (i915_gem_stolen_setup_compression(dev, obj-base.size, fb_cpp)) {
if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
DRM_DEBUG_KMS(framebuffer too large, disabling
compression\n);
return;
--
2.1.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/9] drm/i915: don't reallocate the compressed FB at every frame
From: Paulo Zanoni paulo.r.zan...@intel.com With the current code we just reallocate the compressed FB at every FBC update: we have X in one frame, then in the other frame we need X again, but we check needed have instead of needed = have. There are still other problems with this code: we don't take the threshold into consideration and we also have cases where we reallocate the CFB while FBC is enabled, but let's leave these for later patches. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index a204584..4797138 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -253,7 +253,7 @@ int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_c if (!drm_mm_initialized(dev_priv-mm.stolen)) return -ENODEV; - if (size dev_priv-fbc.size) + if (size = dev_priv-fbc.size) return 0; /* Release any current block */ -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: add the FBC mutex
From: Paulo Zanoni paulo.r.zan...@intel.com
Make sure we're not gonna have weird races in really weird cases where
a lot of different CRTCs are doing rendering and modesets at the same
time.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_fbc.c | 80 +++-
2 files changed, 64 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f0419c8..18fcce4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -734,6 +734,7 @@ enum fb_op_origin {
};
struct i915_fbc {
+ struct mutex lock;
unsigned threshold;
unsigned int fb_id;
unsigned int possible_framebuffer_bits;
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index c6e688c..6611266 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -335,6 +335,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
struct drm_i915_private *dev_priv = dev-dev_private;
mutex_lock(dev-struct_mutex);
+ mutex_lock(dev_priv-fbc.lock);
if (work == dev_priv-fbc.fbc_work) {
/* Double check that we haven't switched fb without cancelling
* the prior work.
@@ -349,6 +350,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
dev_priv-fbc.fbc_work = NULL;
}
+ mutex_unlock(dev_priv-fbc.lock);
mutex_unlock(dev-struct_mutex);
kfree(work);
@@ -356,6 +358,8 @@ static void intel_fbc_work_fn(struct work_struct *__work)
static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv)
{
+ lockdep_assert_held(dev_priv-fbc.lock);
+
if (dev_priv-fbc.fbc_work == NULL)
return;
@@ -383,6 +387,8 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+ lockdep_assert_held(dev_priv-fbc.lock);
+
if (!dev_priv-display.enable_fbc)
return;
@@ -417,16 +423,12 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
schedule_delayed_work(work-work, msecs_to_jiffies(50));
}
-/**
- * intel_fbc_disable - disable FBC
- * @dev: the drm_device
- *
- * This function disables FBC.
- */
-void intel_fbc_disable(struct drm_device *dev)
+static void __intel_fbc_disable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ lockdep_assert_held(dev_priv-fbc.lock);
+
intel_fbc_cancel_work(dev_priv);
if (!dev_priv-display.disable_fbc)
@@ -436,6 +438,21 @@ void intel_fbc_disable(struct drm_device *dev)
dev_priv-fbc.crtc = NULL;
}
+/**
+ * intel_fbc_disable - disable FBC
+ * @dev: the drm_device
+ *
+ * This function disables FBC.
+ */
+void intel_fbc_disable(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ mutex_lock(dev_priv-fbc.lock);
+ __intel_fbc_disable(dev);
+ mutex_unlock(dev_priv-fbc.lock);
+}
+
static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
enum no_fbc_reason reason)
{
@@ -484,7 +501,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct
drm_i915_private *dev_priv)
}
/**
- * intel_fbc_update - enable/disable FBC as needed
+ * __intel_fbc_update - enable/disable FBC as needed, unlocked
* @dev: the drm_device
*
* Set up the framebuffer compression hardware at mode set time. We
@@ -502,7 +519,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct
drm_i915_private *dev_priv)
*
* We need to enable/disable FBC on a global basis.
*/
-void intel_fbc_update(struct drm_device *dev)
+static void __intel_fbc_update(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc = NULL;
@@ -512,6 +529,8 @@ void intel_fbc_update(struct drm_device *dev)
const struct drm_display_mode *adjusted_mode;
unsigned int max_width, max_height;
+ lockdep_assert_held(dev_priv-fbc.lock);
+
if (!HAS_FBC(dev))
return;
@@ -631,7 +650,7 @@ void intel_fbc_update(struct drm_device *dev)
* some point. And we wait before enabling FBC anyway.
*/
DRM_DEBUG_KMS(disabling active FBC for update\n);
- intel_fbc_disable(dev);
+ __intel_fbc_disable(dev);
}
if (i915_gem_stolen_setup_compression(dev, obj-base.size,
@@ -649,11 +668,26 @@ out_disable:
/* Multiple disables should be harmless */
if (intel_fbc_enabled(dev)) {
DRM_DEBUG_KMS(unsupported config, disabling FBC\n);
- intel_fbc_disable(dev);
+ __intel_fbc_disable(dev);
}
i915_gem_stolen_cleanup_compression(dev);
}
+/*
+
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Move to atomic plane helpers (v8)
On 12/16/2014 02:23 AM, Matt Roper wrote: Switch plane handling to use the atomic plane helpers. This means that rather than provide our own implementations of .update_plane() and .disable_plane(), we expose the lower-level check/prepare/commit/cleanup entrypoints and let the DRM core implement update/disable for us using those entrypoints. The other main change that falls out of this patch is that our drm_plane's will now always have a valid plane-state that contains the relevant plane state (initial state is allocated at plane creation). The base drm_plane_state pointed to holds the requested source/dest coordinates, and the subclassed intel_plane_state holds the adjusted values that our driver actually uses. v2: - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel) - Fix a copy/paste comment mistake (Bob) v3: - Use prepare/cleanup functions that we've already factored out - Use newly refactored pre_commit/commit/post_commit to avoid sleeping during vblank evasion v4: - Rebase to latest di-nightly requires adding an 'old_state' parameter to atomic_update; v5: - Must have botched a rebase somewhere and lost some work. Restore state 'dirty' flag to let begin/end code know which planes to run the pre_commit/post_commit hooks for. This would have actually shown up as broken in the next commit rather than this one. v6: - Squash kerneldoc patch into this one. - Previous patches have now already taken care of most of the infrastructure that used to be in this patch. All we're adding here now is some thin wrappers. v7: - Check return of intel_plane_duplicate_state() for allocation failures. v8: - Drop unused drm_plane_state - intel_plane_state cast. (Ander) - Squash in actual transition to plane helpers. Significant refactoring earlier in the patchset has made the combined prep+transition much easier to swallow than it was in earlier iterations. (Ander) Patch looks good overall. Just a little bike shedding below. Testcase: igt/kms_plane Testcase: igt/kms_universal_plane Testcase: igt/kms_cursor_crc Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- Documentation/DocBook/drm.tmpl| 5 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 150 ++ drivers/gpu/drm/i915/intel_display.c | 250 -- drivers/gpu/drm/i915/intel_drv.h | 10 +- drivers/gpu/drm/i915/intel_sprite.c | 50 -- 6 files changed, 301 insertions(+), 165 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 995dfb2..c033a0d 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3932,6 +3932,11 @@ int num_ioctls;/synopsis /para /sect2 sect2 +titleAtomic Plane Helpers/title +!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers +!Idrivers/gpu/drm/i915/intel_atomic_plane.c + /sect2 + sect2 titleOutput Probing/title para This section covers output probing and related infrastructure like the diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 1849ffa..16e3dc3 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -66,6 +66,7 @@ i915-y += dvo_ch7017.o \ dvo_ns2501.o \ dvo_sil164.o \ dvo_tfp410.o \ + intel_atomic_plane.o \ intel_crt.o \ intel_ddi.o \ intel_dp.o \ diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c new file mode 100644 index 000..286fec8 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -0,0 +1,150 @@ +/* + * 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
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Drop unused position fields (v2)
On 12/16/2014 02:23 AM, Matt Roper wrote:
The userspace-requested plane coordinates are now always available via
plane-state.base (and the i915-adjusted values are stored in
plane-state), so we no longer use the coordinate fields in intel_plane
and can drop them.
Also, note that the error case for pageflip calls update_plane() to
program the values from plane-state; it's simpler to just call
intel_plane_restore() which does the same thing.
v2: Replace manual update_plane() with intel_plane_restore() in pageflip
error handler.
Reviewed-by(v1): Bob Paauwe bob.j.paa...@intel.com
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
Reviewed-by: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 27 +--
drivers/gpu/drm/i915/intel_drv.h | 4
drivers/gpu/drm/i915/intel_sprite.c | 19 ---
3 files changed, 5 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 934e6a8..d664104 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9616,7 +9616,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *primary = crtc-primary;
- struct intel_plane *intel_plane = to_intel_plane(primary);
enum pipe pipe = intel_crtc-pipe;
struct intel_unpin_work *work;
struct intel_engine_cs *ring;
@@ -9775,15 +9774,7 @@ free_work:
if (ret == -EIO) {
out_hang:
- ret = primary-funcs-update_plane(primary, crtc, fb,
-intel_plane-crtc_x,
-intel_plane-crtc_y,
-intel_plane-crtc_h,
-intel_plane-crtc_w,
-intel_plane-src_x,
-intel_plane-src_y,
-intel_plane-src_h,
-intel_plane-src_w);
+ ret = intel_plane_restore(primary);
if (ret == 0 event) {
spin_lock_irq(dev-event_lock);
drm_send_vblank_event(dev, pipe, event);
@@ -11827,14 +11818,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
crtc-x = src-x1 16;
crtc-y = src-y1 16;
- intel_plane-crtc_x = state-base.crtc_x;
- intel_plane-crtc_y = state-base.crtc_y;
- intel_plane-crtc_w = state-base.crtc_w;
- intel_plane-crtc_h = state-base.crtc_h;
- intel_plane-src_x = state-base.src_x;
- intel_plane-src_y = state-base.src_y;
- intel_plane-src_w = state-base.src_w;
- intel_plane-src_h = state-base.src_h;
intel_plane-obj = obj;
if (intel_crtc-active) {
@@ -12099,14 +12082,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
crtc-cursor_x = state-base.crtc_x;
crtc-cursor_y = state-base.crtc_y;
- intel_plane-crtc_x = state-base.crtc_x;
- intel_plane-crtc_y = state-base.crtc_y;
- intel_plane-crtc_w = state-base.crtc_w;
- intel_plane-crtc_h = state-base.crtc_h;
- intel_plane-src_x = state-base.src_x;
- intel_plane-src_y = state-base.src_y;
- intel_plane-src_w = state-base.src_w;
- intel_plane-src_h = state-base.src_h;
intel_plane-obj = obj;
if (intel_crtc-cursor_bo == obj)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index ab23190..dbf04dc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -517,10 +517,6 @@ struct intel_plane {
struct drm_i915_gem_object *obj;
bool can_scale;
int max_downscale;
- int crtc_x, crtc_y;
- unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y;
- uint32_t src_w, src_h;
unsigned int rotation;
/* Since we need to change the watermarks before/after
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index f8efcfd..d874ce0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1257,15 +1257,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
- intel_plane-crtc_x = state-base.crtc_x;
- intel_plane-crtc_y = state-base.crtc_y;
- intel_plane-crtc_w = state-base.crtc_w;
- intel_plane-crtc_h = state-base.crtc_h;
- intel_plane-src_x = state-base.src_x;
- intel_plane-src_y = state-base.src_y;
- intel_plane-src_w = state-base.src_w;
- intel_plane-src_h = state-base.src_h;
-
[Intel-gfx] [PATCH v2 03/24] drm/i915: Rename to GEN8_LEGACY_PDPES
From: Ben Widawsky benjamin.widaw...@intel.com
In gen8, 32b PPGTT has always had one pdp (it doesn't actually have
one, but it resembles having one). The #define was confusing as is, and
using PDPE is a much better description.
sed -i 's/GEN8_LEGACY_PDPS/GEN8_LEGACY_PDPES/' drivers/gpu/drm/i915/*.[ch]
It also matches the x86 pagetable terminology:
PTE = Page Table Entry - pagetable level 1 page
PDE = Page Directory Entry - pagetable level 2 page
PDPE = Page Directory Pointer Entry - pagetable level 3 page
And in the near future (for 48b addressing):
PML4E = Page Map Level 4 Entry
v2: Expanded information about Page Directory/Table nomenclature.
Cc: Daniel Vetter dan...@ffwll.ch
CC: Dave Gordon david.s.gor...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2)
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 6 +++---
2 files 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 746f77f..58d54bd 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -375,7 +375,7 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
pt_vaddr = NULL;
for_each_sg_page(pages-sgl, sg_iter, pages-nents, 0) {
- if (WARN_ON(pdpe = GEN8_LEGACY_PDPS))
+ if (WARN_ON(pdpe = GEN8_LEGACY_PDPES))
break;
if (pt_vaddr == NULL)
@@ -486,7 +486,7 @@ bail:
static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt,
const int max_pdp)
{
- struct page **pt_pages[GEN8_LEGACY_PDPS];
+ struct page **pt_pages[GEN8_LEGACY_PDPES];
int i, ret;
for (i = 0; i max_pdp; i++) {
@@ -537,7 +537,7 @@ static int gen8_ppgtt_allocate_page_directories(struct
i915_hw_ppgtt *ppgtt,
return -ENOMEM;
ppgtt-num_pd_pages = 1 get_order(max_pdp PAGE_SHIFT);
- BUG_ON(ppgtt-num_pd_pages GEN8_LEGACY_PDPS);
+ BUG_ON(ppgtt-num_pd_pages GEN8_LEGACY_PDPES);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index e377c7d..9d998ec 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -88,7 +88,7 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t;
#define GEN8_PDE_MASK 0x1ff
#define GEN8_PTE_SHIFT 12
#define GEN8_PTE_MASK 0x1ff
-#define GEN8_LEGACY_PDPS 4
+#define GEN8_LEGACY_PDPES 4
#define GEN8_PTES_PER_PAGE (PAGE_SIZE / sizeof(gen8_gtt_pte_t))
#define GEN8_PDES_PER_PAGE (PAGE_SIZE / sizeof(gen8_ppgtt_pde_t))
@@ -273,12 +273,12 @@ struct i915_hw_ppgtt {
unsigned num_pd_pages; /* gen8+ */
union {
struct page **pt_pages;
- struct page **gen8_pt_pages[GEN8_LEGACY_PDPS];
+ struct page **gen8_pt_pages[GEN8_LEGACY_PDPES];
};
struct page *pd_pages;
union {
uint32_t pd_offset;
- dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS];
+ dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPES];
};
union {
dma_addr_t *pt_dma_addr;
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 02/24] drm/i915/trace: Fix offsets for 64b
From: Ben Widawsky benjamin.widaw...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_trace.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 6058a01..f004d3d 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -115,7 +115,7 @@ TRACE_EVENT(i915_vma_bind, TP_STRUCT__entry( __field(struct drm_i915_gem_object *, obj) __field(struct i915_address_space *, vm) -__field(u32, offset) +__field(u64, offset) __field(u32, size) __field(unsigned, flags) ), @@ -128,7 +128,7 @@ TRACE_EVENT(i915_vma_bind, __entry-flags = flags; ), - TP_printk(obj=%p, offset=%08x size=%x%s vm=%p, + TP_printk(obj=%p, offset=%016llx size=%x%s vm=%p, __entry-obj, __entry-offset, __entry-size, __entry-flags PIN_MAPPABLE ? , mappable : , __entry-vm) @@ -141,7 +141,7 @@ TRACE_EVENT(i915_vma_unbind, TP_STRUCT__entry( __field(struct drm_i915_gem_object *, obj) __field(struct i915_address_space *, vm) -__field(u32, offset) +__field(u64, offset) __field(u32, size) ), @@ -152,7 +152,7 @@ TRACE_EVENT(i915_vma_unbind, __entry-size = vma-node.size; ), - TP_printk(obj=%p, offset=%08x size=%x vm=%p, + TP_printk(obj=%p, offset=%016llx size=%x vm=%p, __entry-obj, __entry-offset, __entry-size, __entry-vm) ); -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 00/24] PPGTT dynamic page allocations
Addressing comments from v1. For GEN8, it has also been extended to work in logical ring submission (lrc) mode, as it will be the preferred mode of operation. I also tried to update the lrc code at the same time the ppgtt refactoring occurred, leaving only one patch that is exclusively for lrc. This list can be seen in 3 parts: [01-10] Include code rework for PPGTT (all GENs). [11-14] Adds page table allocation for GEN6/GEN7 [15-24] Enables dynamic allocation in GEN8. It is enabled for both legacy and execlist submission modes. Ben Widawsky (23): drm/i915: Add some extra guards in evict_vm drm/i915/trace: Fix offsets for 64b drm/i915: Rename to GEN8_LEGACY_PDPES drm/i915: Setup less PPGTT on failed pagedir drm/i915/gen8: Un-hardcode number of page directories drm/i915: Range clearing is PPGTT agnostic drm/i915: page table abstractions drm/i915: Complete page table structures drm/i915: Create page table allocators drm/i915: Track GEN6 page table usage drm/i915: Extract context switch skip and pd load logic drm/i915: Track page table reload need drm/i915: Initialize all contexts drm/i915: Finish gen6/7 dynamic page table allocation drm/i915/bdw: Use dynamic allocation idioms on free drm/i915/bdw: pagedirs rework allocation drm/i915/bdw: pagetable allocation rework drm/i915/bdw: Update pdp switch and point unused PDPs to scratch page drm/i915: num_pd_pages/num_pd_entries isn't useful drm/i915: Extract PPGTT param from pagedir alloc drm/i915/bdw: Split out mappings drm/i915/bdw: begin bitmap tracking drm/i915/bdw: Dynamic page table allocations Michel Thierry (1): drm/i915/bdw: Dynamic page table allocations in lrc mode drivers/gpu/drm/i915/i915_debugfs.c|7 +- drivers/gpu/drm/i915/i915_gem.c| 11 + drivers/gpu/drm/i915/i915_gem_context.c| 62 +- drivers/gpu/drm/i915/i915_gem_evict.c |3 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 + drivers/gpu/drm/i915/i915_gem_gtt.c| 1200 drivers/gpu/drm/i915/i915_gem_gtt.h| 250 +- drivers/gpu/drm/i915/i915_trace.h | 123 ++- drivers/gpu/drm/i915/intel_lrc.c | 80 +- 9 files changed, 1360 insertions(+), 387 deletions(-) -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 04/24] drm/i915: Setup less PPGTT on failed pagedir
From: Ben Widawsky benjamin.widaw...@intel.com 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 b...@bwidawsk.net Signed-off-by: Michel Thierry michel.thie...@intel.com --- 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 58d54bd..b48b586 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1032,11 +1032,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) -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 07/24] drm/i915: page table abstractions
From: Ben Widawsky benjamin.widaw...@intel.com
When we move to dynamic page allocation, keeping pagedir and pagetabs as
separate structures will help to break actions into simpler tasks.
To help transition the code nicely there is some wasted space in gen6/7.
This will be ameliorated shortly.
v2: fixed mismatches after clean-up/rebase.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2)
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 177 ++--
drivers/gpu/drm/i915/i915_gem_gtt.h | 23 -
2 files changed, 107 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0f6a196..6902462 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -334,7 +334,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_PAGE)
@@ -378,8 +379,12 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
if (WARN_ON(pdpe = GEN8_LEGACY_PDPES))
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),
@@ -403,29 +408,33 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
}
}
-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 GEN8_PDES_PER_PAGE; 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_ppgtt_free(const struct i915_hw_ppgtt *ppgtt)
+static void gen8_free_page_directories(struct i915_pagedir *pd)
+{
+ kfree(pd-page_tables);
+ __free_page(pd-page);
+}
+
+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_unmap_pages(struct i915_hw_ppgtt *ppgtt)
@@ -460,86 +469,75 @@ 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(GEN8_PDES_PER_PAGE, sizeof(struct page *),
GFP_KERNEL);
- if (!pt_pages)
- return ERR_PTR(-ENOMEM);
-
- for (i = 0; i GEN8_PDES_PER_PAGE; i++) {
- pt_pages[i] = alloc_page(GFP_KERNEL);
- if (!pt_pages[i])
- goto bail;
+ for (i = 0; i ppgtt-num_pd_pages; i++) {
+ ppgtt-gen8_pt_dma_addr[i] = kcalloc(GEN8_PDES_PER_PAGE,
+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_PDPES];
- 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])) {
- ret = PTR_ERR(pt_pages[i]);
-
[Intel-gfx] [PATCH v2 06/24] drm/i915: Range clearing is PPGTT agnostic
From: Ben Widawsky benjamin.widaw...@intel.com 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 b...@bwidawsk.net Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index b48b586..0f6a196 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -672,8 +672,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, @@ -1146,8 +1144,6 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) ppgtt-pd_offset = ppgtt-node.start / PAGE_SIZE * sizeof(gen6_gtt_pte_t); - 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); @@ -1181,6 +1177,8 @@ int i915_ppgtt_init(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); } -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 09/24] drm/i915: Create page table allocators
From: Ben Widawsky benjamin.widaw...@intel.com
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.
This patch exists for a few reasons:
1. Splitting out the functions allows easily combining GEN6 and GEN8
code. Page tables have no difference based on GEN8. As we'll see in a
future patch when we add the DMA mappings to the allocations, it
requires only one small change to make work, and error handling should
just fall into place.
2. Unless we always want to allocate all page tables under a given PDE,
we'll have to eventually break this up into an array of pointers (or
pointer to pointer).
3. Having the discrete functions is easier to review, and understand.
All allocations and frees now take place in just a couple of locations.
Reviewing, and catching leaks should be easy.
4. Less important: the GFP flags are confined to one location, which
makes playing around with such things trivial.
v2: Updated commit message to explain why this patch exists
v3: For lrc, s/pdp.pagedir[i].daddr/pdp.pagedir[i]-daddr/
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v3)
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 228 +++-
drivers/gpu/drm/i915/i915_gem_gtt.h | 4 +-
drivers/gpu/drm/i915/intel_lrc.c| 16 +--
3 files changed, 155 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a26c18c..52bdde7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -275,6 +275,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 GEN6_PPGTT_PD_ENTRIES);
+
+ 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
[Intel-gfx] [PATCH v2 14/24] drm/i915: Finish gen6/7 dynamic page table allocation
From: Ben Widawsky benjamin.widaw...@intel.com
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.
v2: Make the page table bitmap declared inside the function (Chris)
Simplify the way scratching address space works.
Move the alloc/teardown tracepoints up a level in the call stack so that
both all implementations get the trace.
v3: Updated trace event to spit out a name
v4: Aliasing ppgtt is now initialized differently (in setup global gtt)
v5: Rebase to latest code. Also removed unnecessary aliasing ppgtt check for
trace, as it is no longer possible after the PPGTT cleanup patch series
of a couple of months ago (Daniel).
Cc: Daniel Vetter dan...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v4+)
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +
drivers/gpu/drm/i915/i915_gem_gtt.c | 128
drivers/gpu/drm/i915/i915_trace.h | 115
4 files changed, 236 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 60f91bc..0f63076 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2149,6 +2149,8 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
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;
@@ -2165,7 +2167,6 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
get_pid_task(file-pid, PIDTYPE_PID)-comm);
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)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5d52990..1649fb2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3599,6 +3599,8 @@ search_free:
/* allocate before insert / bind */
if (vma-vm-allocate_va_range) {
+ trace_i915_va_alloc(vma-vm, vma-node.start, vma-node.size,
+ VM_TO_TRACE_NAME(vma-vm));
ret = vma-vm-allocate_va_range(vma-vm,
vma-node.start,
vma-node.size);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 54c7ca7..32a355a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1138,10 +1138,47 @@ static void gen6_ppgtt_unmap_pages(struct i915_hw_ppgtt
*ppgtt)
static int gen6_alloc_va_range(struct i915_address_space *vm,
uint64_t start, uint64_t length)
{
+ DECLARE_BITMAP(new_page_tables, GEN6_PPGTT_PD_ENTRIES);
+ 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
[Intel-gfx] [PATCH v2 08/24] drm/i915: Complete page table structures
From: Ben Widawsky benjamin.widaw...@intel.com
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.
v2: In lrc: s/ppgtt-pd_dma_addr[i]/ppgtt-pdp.pagedir[i].daddr/
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2)
---
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 | 14 +++---
drivers/gpu/drm/i915/intel_lrc.c| 16 +++
4 files changed, 45 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index e515aad..60f91bc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2153,7 +2153,7 @@ static void gen6_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
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);
+ seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd.pd_offset);
ppgtt-debug_dump(ppgtt, m);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6902462..a26c18c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -307,7 +307,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
int used_pd = ppgtt-num_pd_entries / GEN8_PDES_PER_PAGE;
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);
if (ret)
return ret;
@@ -433,7 +433,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]);
}
}
@@ -445,14 +444,14 @@ static void gen8_ppgtt_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 GEN8_PDES_PER_PAGE; 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);
@@ -469,32 +468,19 @@ 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(GEN8_PDES_PER_PAGE,
-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 GEN8_PDES_PER_PAGE; 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;
+
}
}
@@ -555,9 +541,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
ppgtt-num_pd_entries = max_pdp * GEN8_PDES_PER_PAGE;
- ret = gen8_ppgtt_allocate_dma(ppgtt);
- if (!ret)
- return ret;
+ return 0;
/* TODO: Check this for all cases */
err_out:
@@ -579,7 +563,7 @@ static int gen8_ppgtt_setup_page_directories(struct
i915_hw_ppgtt *ppgtt,
if (ret)
return ret;
- ppgtt-pd_dma_addr[pd] = pd_addr;
+ ppgtt-pdp.pagedir[pd].daddr =
[Intel-gfx] [PATCH v2 13/24] drm/i915: Initialize all contexts
From: Ben Widawsky benjamin.widaw...@intel.com
The problem is we're going to switch to a new context, which could be
the default context. The plan was to use restore inhibit, which would be
fine, except if we are using dynamic page tables (which we will). If we
use dynamic page tables and we don't load new page tables, the previous
page tables might go away, and future operations will fault.
CTXA runs.
switch to default, restore inhibit
CTXA dies and has its address space taken away.
Run CTXB, tries to save using the context A's address space - this
fails.
The general solution is to make sure every context has it's own state,
and its own address space. For cases when we must restore inhibit, first
thing we do is load a valid address space. I thought this would be
enough, but apparently there are references within the context itself
which will refer to the old address space - therefore, we also must
reinitialize.
It was tricky to track this down as we don't have much insight into what
happens in a context save.
This is required for the next patch which enables dynamic page tables.
v2: to-ppgtt is only valid in full ppgtt.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2)
---
drivers/gpu/drm/i915/i915_gem_context.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index fa9d4a1..b1f3d50 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -592,13 +592,6 @@ needs_pd_load_pre(struct intel_engine_cs *ring, struct
intel_context *to)
(ring != dev_priv-ring[RCS])) to-ppgtt;
}
-static bool
-needs_pd_load_post(struct intel_engine_cs *ring, struct intel_context *to)
-{
- return IS_GEN8(ring-dev)
- (to-ppgtt || to-ppgtt-base.pd_reload_mask);
-}
-
static int do_switch(struct intel_engine_cs *ring,
struct intel_context *to)
{
@@ -679,20 +672,24 @@ static int do_switch(struct intel_engine_cs *ring,
/* GEN8 does *not* require an explicit reload if the PDPs have been
* setup, and we do not wish to move them.
-*
-* XXX: If we implemented page directory eviction code, this
-* optimization needs to be removed.
*/
- if (!to-legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+ if (!to-legacy_hw_ctx.initialized) {
hw_flags |= MI_RESTORE_INHIBIT;
- else if (to-ppgtt test_and_clear_bit(ring-id,
to-ppgtt-base.pd_reload_mask))
+ /* NB: If we inhibit the restore, the context is not allowed to
+* die because future work may end up depending on valid address
+* space. This means we must enforce that a page table load
+* occur when this occurs. */
+ } else if (to-ppgtt test_and_clear_bit(ring-id,
to-ppgtt-base.pd_reload_mask))
hw_flags |= MI_FORCE_RESTORE;
ret = mi_set_context(ring, to, hw_flags);
if (ret)
goto unpin_out;
- if (needs_pd_load_post(ring, to)) {
+ if (IS_GEN8(ring-dev) to-ppgtt (hw_flags MI_RESTORE_INHIBIT))
{
+ /* We have a valid page directory (scratch) to switch to. This
+* allows the old VM to be freed. Note that if anything occurs
+* between the set context, and here, we are f*cked */
ret = to-ppgtt-switch_mm(to-ppgtt, ring);
/* The hardware context switch is emitted, but we haven't
* actually changed the state - so it's probably safe to bail
@@ -742,7 +739,7 @@ static int do_switch(struct intel_engine_cs *ring,
i915_gem_context_unreference(from);
}
- uninitialized = !to-legacy_hw_ctx.initialized from == NULL;
+ uninitialized = !to-legacy_hw_ctx.initialized;
to-legacy_hw_ctx.initialized = true;
done:
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 11/24] drm/i915: Extract context switch skip and pd load logic
From: Ben Widawsky benjamin.widaw...@intel.com
We have some fanciness coming up. This patch just breaks out the logic
of context switch skip, pd load pre, and pd load post.
v2: Use new functions to replace the logic right away (Daniel)
Cc: Daniel Vetter dan...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2)
---
drivers/gpu/drm/i915/i915_gem_context.c | 40 +
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index b67d269..7b20bd4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -563,6 +563,33 @@ mi_set_context(struct intel_engine_cs *ring,
return ret;
}
+static inline bool should_skip_switch(struct intel_engine_cs *ring,
+ struct intel_context *from,
+ struct intel_context *to)
+{
+ if (from == to !to-remap_slice)
+ return true;
+
+ return false;
+}
+
+static bool
+needs_pd_load_pre(struct intel_engine_cs *ring, struct intel_context *to)
+{
+ struct drm_i915_private *dev_priv = ring-dev-dev_private;
+
+ return ((INTEL_INFO(ring-dev)-gen 8) ||
+ (ring != dev_priv-ring[RCS])) to-ppgtt;
+}
+
+static bool
+needs_pd_load_post(struct intel_engine_cs *ring, struct intel_context *to)
+{
+ return (!to-legacy_hw_ctx.initialized ||
+ i915_gem_context_is_default(to))
+ to-ppgtt IS_GEN8(ring-dev);
+}
+
static int do_switch(struct intel_engine_cs *ring,
struct intel_context *to)
{
@@ -571,9 +598,6 @@ static int do_switch(struct intel_engine_cs *ring,
u32 hw_flags = 0;
bool uninitialized = false;
struct i915_vma *vma;
- bool needs_pd_load_pre = ((INTEL_INFO(ring-dev)-gen 8) ||
- (ring != dev_priv-ring[RCS])) to-ppgtt;
- bool needs_pd_load_post = false;
int ret, i;
if (from != NULL ring == dev_priv-ring[RCS]) {
@@ -581,7 +605,7 @@ static int do_switch(struct intel_engine_cs *ring,
BUG_ON(!i915_gem_obj_is_pinned(from-legacy_hw_ctx.rcs_state));
}
- if (from == to !to-remap_slice)
+ if (should_skip_switch(ring, from, to))
return 0;
/* Trying to pin first makes error handling easier. */
@@ -599,7 +623,7 @@ static int do_switch(struct intel_engine_cs *ring,
*/
from = ring-last_context;
- if (needs_pd_load_pre) {
+ if (needs_pd_load_pre(ring, to)) {
/* Older GENs and non render rings still want the load first,
* PP_DCLV followed by PP_DIR_BASE register through Load
* Register Immediate commands in Ring Buffer before submitting
@@ -644,16 +668,14 @@ static int do_switch(struct intel_engine_cs *ring,
* XXX: If we implemented page directory eviction code, this
* optimization needs to be removed.
*/
- if (!to-legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) {
+ if (!to-legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
hw_flags |= MI_RESTORE_INHIBIT;
- needs_pd_load_post = to-ppgtt IS_GEN8(ring-dev);
- }
ret = mi_set_context(ring, to, hw_flags);
if (ret)
goto unpin_out;
- if (needs_pd_load_post) {
+ if (needs_pd_load_post(ring, to)) {
ret = to-ppgtt-switch_mm(to-ppgtt, ring);
/* The hardware context switch is emitted, but we haven't
* actually changed the state - so it's probably safe to bail
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 21/24] drm/i915/bdw: Split out mappings
From: Ben Widawsky benjamin.widaw...@intel.com
When we do dynamic page table allocations for gen8, we'll need to have
more control over how and when we map page tables, similar to gen6.
In particular, DMA mappings for page directories/tables occur at allocation
time.
This patch adds the functionality and calls it at init, which should
have no functional change.
The PDPEs are still a special case for now. We'll need a function for
that in the future as well.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 186 ++--
1 file changed, 72 insertions(+), 114 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6d67660..ff3aac5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -415,21 +415,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)
@@ -441,6 +443,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;
}
@@ -580,6 +589,36 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
}
}
+static void __gen8_do_map_pt(gen8_ppgtt_pde_t *pde,
+struct i915_pagetab *pt,
+struct drm_device *dev)
+{
+ gen8_ppgtt_pde_t entry =
+ gen8_pde_encode(dev, pt-daddr, I915_CACHE_LLC);
+ *pde = entry;
+}
+
+/* It's likely we'll map more than one pagetable at a time. This function will
+ * save us unnecessary kmap calls, but do no more functionally than multiple
+ * calls to map_pt. */
+static void gen8_map_pagetable_range(struct i915_pagedir *pd,
+uint64_t start,
+uint64_t length,
+struct drm_device *dev)
+{
+ gen8_ppgtt_pde_t *pagedir = kmap_atomic(pd-page);
+ struct i915_pagetab *pt;
+ uint64_t temp, pde;
+
+ gen8_for_each_pde(pt, pd, start, length, temp, pde)
+ __gen8_do_map_pt(pagedir + pde, pt, dev);
+
+ if (!HAS_LLC(dev))
+ drm_clflush_virt_range(pagedir, PAGE_SIZE);
+
+ kunmap_atomic(pagedir);
+}
+
static void gen8_teardown_va_range(struct i915_address_space *vm,
uint64_t start, uint64_t length)
{
@@ -597,7 +636,7 @@ static void gen8_teardown_va_range(struct
i915_address_space *vm,
free_pt_single(pt, vm-dev);
pd-page_tables[pde] = NULL;
}
- free_pd_single(pd);
+ free_pd_single(pd, vm-dev);
ppgtt-pdp.pagedir[pdpe] = NULL;
}
}
@@ -629,9 +668,6 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt
*ppgtt)
static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
{
- trace_i915_va_teardown(ppgtt-base,
- ppgtt-base.start, ppgtt-base.total,
- VM_TO_TRACE_NAME(ppgtt-base));
gen8_teardown_va_range(ppgtt-base,
ppgtt-base.start, ppgtt-base.total);
}
@@ -672,7 +708,8 @@ unwind_out:
static int gen8_ppgtt_alloc_pagedirs(struct i915_pagedirpo *pdp,
uint64_t start,
-uint64_t length)
+uint64_t length,
+struct drm_device *dev)
{
struct i915_pagedir *unused;
uint64_t temp;
@@ -683,7 +720,7 @@ static int gen8_ppgtt_alloc_pagedirs(struct i915_pagedirpo
*pdp,
gen8_for_each_pdpe(unused, pdp, start, length, temp, pdpe) {
BUG_ON(unused);
- pdp-pagedir[pdpe] = alloc_pd_single();
+ pdp-pagedir[pdpe] = alloc_pd_single(dev);
if (IS_ERR(pdp-pagedir[pdpe]))
goto unwind_out;
}
@@ -692,21 +729,25 @@ static int gen8_ppgtt_alloc_pagedirs(struct
i915_pagedirpo *pdp,
unwind_out:
while (pdpe--)
-
[Intel-gfx] [PATCH v2 22/24] drm/i915/bdw: begin bitmap tracking
From: Ben Widawsky benjamin.widaw...@intel.com
Like with gen6/7, we can enable bitmap tracking with all the
preallocations to make sure things actually don't blow up.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 121 +---
drivers/gpu/drm/i915/i915_gem_gtt.h | 24 +++
2 files changed, 108 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ff3aac5..6254677 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -417,8 +417,12 @@ err_out:
static void __free_pd_single(struct i915_pagedir *pd, struct drm_device *dev)
{
+ WARN(!bitmap_empty(pd-used_pdes, GEN8_PDES_PER_PAGE),
+Free page directory with %d used pages\n,
+bitmap_weight(pd-used_pdes, GEN8_PDES_PER_PAGE));
i915_dma_unmap_single(pd, dev);
__free_page(pd-page);
+ kfree(pd-used_pdes);
kfree(pd);
}
@@ -431,26 +435,35 @@ static void __free_pd_single(struct i915_pagedir *pd,
struct drm_device *dev)
static struct i915_pagedir *alloc_pd_single(struct drm_device *dev)
{
struct i915_pagedir *pd;
- int ret;
+ int ret = -ENOMEM;
pd = kzalloc(sizeof(*pd), GFP_KERNEL);
if (!pd)
return ERR_PTR(-ENOMEM);
+ pd-used_pdes = kcalloc(BITS_TO_LONGS(GEN8_PDES_PER_PAGE),
+ sizeof(*pd-used_pdes), GFP_KERNEL);
+ if (!pd-used_pdes)
+ goto free_pd;
+
pd-page = alloc_page(GFP_KERNEL | __GFP_ZERO);
- if (!pd-page) {
- kfree(pd);
- return ERR_PTR(-ENOMEM);
- }
+ if (!pd-page)
+ goto free_bitmap;
ret = i915_dma_map_px_single(pd, dev);
- if (ret) {
- __free_page(pd-page);
- kfree(pd);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto free_page;
return pd;
+
+free_page:
+ __free_page(pd-page);
+free_bitmap:
+ kfree(pd-used_pdes);
+free_pd:
+ kfree(pd);
+
+ return ERR_PTR(ret);
}
/* Broadwell Page Directory Pointer Descriptors */
@@ -632,36 +645,47 @@ static void gen8_teardown_va_range(struct
i915_address_space *vm,
gen8_for_each_pdpe(pd, ppgtt-pdp, start, length, temp, pdpe) {
uint64_t pd_len = gen8_clamp_pd(start, length);
uint64_t pd_start = start;
- gen8_for_each_pde(pt, pd, pd_start, pd_len, temp, pde) {
- free_pt_single(pt, vm-dev);
- pd-page_tables[pde] = NULL;
- }
- free_pd_single(pd, vm-dev);
- ppgtt-pdp.pagedir[pdpe] = NULL;
- }
-}
-static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
-{
- struct pci_dev *hwdev = ppgtt-base.dev-pdev;
- int i, j;
-
- for (i = 0; i GEN8_PDES_PER_PAGE; i++) {
- /* TODO: In the future we'll support sparse mappings, so this
-* will have to change. */
- if (!ppgtt-pdp.pagedir[i]-daddr)
+ /* Page directories might not be present since the macro rounds
+* down, and up.
+*/
+ if (!pd) {
+ WARN(test_bit(pdpe, ppgtt-pdp.used_pdpes),
+PDPE %d is not allocated, but is reserved (%p)\n,
+pdpe, vm);
continue;
+ } else {
+ WARN(!test_bit(pdpe, ppgtt-pdp.used_pdpes),
+PDPE %d not reserved, but is allocated (%p),
+pdpe, vm);
+ }
- pci_unmap_page(hwdev, ppgtt-pdp.pagedir[i]-daddr, PAGE_SIZE,
- PCI_DMA_BIDIRECTIONAL);
+ gen8_for_each_pde(pt, pd, pd_start, pd_len, temp, pde) {
+ if (!pt) {
+ WARN(test_bit(pde, pd-used_pdes),
+PDE %d is not allocated, but is reserved
(%p)\n,
+pde, vm);
+ continue;
+ } else
+ WARN(!test_bit(pde, pd-used_pdes),
+PDE %d not reserved, but is allocated
(%p),
+pde, vm);
+
+ bitmap_clear(pt-used_ptes,
+gen8_pte_index(pd_start),
+gen8_pte_count(pd_start, pd_len));
+
+ if (bitmap_empty(pt-used_ptes, GEN8_PTES_PER_PAGE)) {
+ free_pt_single(pt, vm-dev);
+ pd-page_tables[pde] = NULL;
+ WARN_ON(!test_and_clear_bit(pde,
[Intel-gfx] [PATCH v2 16/24] drm/i915/bdw: pagedirs rework allocation
From: Ben Widawsky benjamin.widaw...@intel.com
Start using gen8_for_each_pdpe macro to allocate the page directories.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 43 ++---
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 971c05b..e759a03 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -594,8 +594,10 @@ static void gen8_teardown_va_range(struct
i915_address_space *vm,
uint64_t pd_start = start;
gen8_for_each_pde(pt, pd, pd_start, pd_len, temp, pde) {
free_pt_single(pt, vm-dev);
+ pd-page_tables[pde] = NULL;
}
free_pd_single(pd);
+ ppgtt-pdp.pagedir[pdpe] = NULL;
}
}
@@ -670,25 +672,39 @@ unwind_out:
return -ENOMEM;
}
-static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
- const int max_pdp)
+static int gen8_ppgtt_alloc_pagedirs(struct i915_pagedirpo *pdp,
+uint64_t start,
+uint64_t length)
{
- int i;
+ struct i915_hw_ppgtt *ppgtt =
+ container_of(pdp, struct i915_hw_ppgtt, pdp);
+ struct i915_pagedir *unused;
+ uint64_t temp;
+ uint32_t pdpe;
- for (i = 0; i max_pdp; i++) {
- ppgtt-pdp.pagedir[i] = alloc_pd_single();
- if (IS_ERR(ppgtt-pdp.pagedir[i]))
+ /* FIXME: PPGTT container_of won't work for 64b */
+ BUG_ON((start + length) 0x8ULL);
+
+ gen8_for_each_pdpe(unused, pdp, start, length, temp, pdpe) {
+ BUG_ON(unused);
+ pdp-pagedir[pdpe] = alloc_pd_single();
+ if (IS_ERR(ppgtt-pdp.pagedir[pdpe]))
goto unwind_out;
+
+ ppgtt-num_pd_pages++;
}
- ppgtt-num_pd_pages = max_pdp;
BUG_ON(ppgtt-num_pd_pages GEN8_LEGACY_PDPES);
return 0;
unwind_out:
- while (i--)
- free_pd_single(ppgtt-pdp.pagedir[i]);
+ while (pdpe--) {
+ free_pd_single(ppgtt-pdp.pagedir[pdpe]);
+ ppgtt-num_pd_pages--;
+ }
+
+ WARN_ON(ppgtt-num_pd_pages);
return -ENOMEM;
}
@@ -698,7 +714,8 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
{
int ret;
- ret = gen8_ppgtt_allocate_page_directories(ppgtt, max_pdp);
+ ret = gen8_ppgtt_alloc_pagedirs(ppgtt-pdp, ppgtt-base.start,
+ ppgtt-base.total);
if (ret)
return ret;
@@ -775,6 +792,10 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
if (size % (130))
DRM_INFO(Pages will be wasted unless GTT size (%llu) is
divisible by 1GB\n, size);
+ ppgtt-base.start = 0;
+ ppgtt-base.total = size;
+ BUG_ON(ppgtt-base.total == 0);
+
/* 1. Do all our allocations for page directories and page tables. */
ret = gen8_ppgtt_alloc(ppgtt, max_pdp);
if (ret)
@@ -822,8 +843,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
ppgtt-base.clear_range = gen8_ppgtt_clear_range;
ppgtt-base.insert_entries = gen8_ppgtt_insert_entries;
ppgtt-base.cleanup = gen8_ppgtt_cleanup;
- ppgtt-base.start = 0;
- ppgtt-base.total = ppgtt-num_pd_entries * GEN8_PTES_PER_PAGE *
PAGE_SIZE;
DRM_DEBUG_DRIVER(Allocated %d pages for page directories (%d
wasted)\n,
ppgtt-num_pd_pages, ppgtt-num_pd_pages - max_pdp);
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 18/24] drm/i915/bdw: Update pdp switch and point unused PDPs to scratch page
From: Ben Widawsky benjamin.widaw...@intel.com
One important part of this patch is we now write a scratch page
directory into any unused PDP descriptors. This matters for 2 reasons,
first, we're not allowed to just use 0, or an invalid pointer, and second,
we must wipe out any previous contents from the last context.
The latter point only matters with full PPGTT. The former point only
effect platforms with less than 4GB memory.
v2: Updated commit message to point that we must set unused PDPs to the
scratch page.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2)
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 29 ++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 5 -
2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f928c10..bd6cb2f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -445,8 +445,9 @@ static struct i915_pagedir *alloc_pd_single(void)
}
/* Broadwell Page Directory Pointer Descriptors */
-static int gen8_write_pdp(struct intel_engine_cs *ring, unsigned entry,
- uint64_t val)
+static int gen8_write_pdp(struct intel_engine_cs *ring,
+ unsigned entry,
+ dma_addr_t addr)
{
int ret;
@@ -458,10 +459,10 @@ static int gen8_write_pdp(struct intel_engine_cs *ring,
unsigned entry,
intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, GEN8_RING_PDP_UDW(ring, entry));
- intel_ring_emit(ring, (u32)(val 32));
+ intel_ring_emit(ring, upper_32_bits(addr));
intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, GEN8_RING_PDP_LDW(ring, entry));
- intel_ring_emit(ring, (u32)(val));
+ intel_ring_emit(ring, lower_32_bits(addr));
intel_ring_advance(ring);
return 0;
@@ -472,12 +473,12 @@ 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;
-
- for (i = used_pd - 1; i = 0; i--) {
- dma_addr_t addr = ppgtt-pdp.pagedir[i]-daddr;
- ret = gen8_write_pdp(ring, i, addr);
+ for (i = GEN8_LEGACY_PDPES - 1; i = 0; i--) {
+ struct i915_pagedir *pd = ppgtt-pdp.pagedir[i];
+ dma_addr_t pd_daddr = pd ? pd-daddr : ppgtt-scratch_pd-daddr;
+ /* The page directory might be NULL, but we need to clear out
+* whatever the previous context might have used. */
+ ret = gen8_write_pdp(ring, i, pd_daddr);
if (ret)
return ret;
}
@@ -800,10 +801,16 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
ppgtt-base.start = 0;
ppgtt-base.total = size;
+ ppgtt-scratch_pd = alloc_pt_scratch(ppgtt-base.dev);
+ if (IS_ERR(ppgtt-scratch_pd))
+ return PTR_ERR(ppgtt-scratch_pd);
+
/* 1. Do all our allocations for page directories and page tables. */
ret = gen8_ppgtt_alloc(ppgtt, ppgtt-base.start, ppgtt-base.total);
- if (ret)
+ if (ret) {
+ free_pt_scratch(ppgtt-scratch_pd, ppgtt-base.dev);
return ret;
+ }
/*
* 2. Create DMA mappings for the page directories and page tables.
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 74837a3..0cf4f6d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -283,7 +283,10 @@ struct i915_hw_ppgtt {
struct i915_pagedir pd;
};
- struct i915_pagetab *scratch_pt;
+ union {
+ struct i915_pagetab *scratch_pt;
+ struct i915_pagetab *scratch_pd; /* Just need the daddr */
+ };
struct drm_i915_file_private *file_priv;
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 24/24] drm/i915/bdw: Dynamic page table allocations in lrc mode
Logic ring contexts need to know the PDPs when they are populated. With
dynamic page table allocations, these PDPs may not exist yet.
Check if PDPs have been allocated and use the scratch page if they do
not exist yet.
Before submission, update the PDPs in the logic ring context as PDPs
have been allocated.
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/intel_lrc.c | 80 +++-
1 file changed, 70 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 546884b..6abe4bc 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -358,6 +358,7 @@ static void execlists_elsp_write(struct intel_engine_cs
*ring,
static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,
struct drm_i915_gem_object *ring_obj,
+ struct i915_hw_ppgtt *ppgtt,
u32 tail)
{
struct page *page;
@@ -369,6 +370,40 @@ static int execlists_update_context(struct
drm_i915_gem_object *ctx_obj,
reg_state[CTX_RING_TAIL+1] = tail;
reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(ring_obj);
+ /* True PPGTT with dynamic page allocation: update PDP registers and
+* point the unallocated PDPs to the scratch page
+*/
+ if (ppgtt) {
+ if (test_bit(3, ppgtt-pdp.used_pdpes)) {
+ reg_state[CTX_PDP3_UDW+1] =
upper_32_bits(ppgtt-pdp.pagedir[3]-daddr);
+ reg_state[CTX_PDP3_LDW+1] =
lower_32_bits(ppgtt-pdp.pagedir[3]-daddr);
+ } else {
+ reg_state[CTX_PDP3_UDW+1] =
upper_32_bits(ppgtt-scratch_pd-daddr);
+ reg_state[CTX_PDP3_LDW+1] =
lower_32_bits(ppgtt-scratch_pd-daddr);
+ }
+ if (test_bit(2, ppgtt-pdp.used_pdpes)) {
+ reg_state[CTX_PDP2_UDW+1] =
upper_32_bits(ppgtt-pdp.pagedir[2]-daddr);
+ reg_state[CTX_PDP2_LDW+1] =
lower_32_bits(ppgtt-pdp.pagedir[2]-daddr);
+ } else {
+ reg_state[CTX_PDP2_UDW+1] =
upper_32_bits(ppgtt-scratch_pd-daddr);
+ reg_state[CTX_PDP2_LDW+1] =
lower_32_bits(ppgtt-scratch_pd-daddr);
+ }
+ if (test_bit(1, ppgtt-pdp.used_pdpes)) {
+ reg_state[CTX_PDP1_UDW+1] =
upper_32_bits(ppgtt-pdp.pagedir[1]-daddr);
+ reg_state[CTX_PDP1_LDW+1] =
lower_32_bits(ppgtt-pdp.pagedir[1]-daddr);
+ } else {
+ reg_state[CTX_PDP1_UDW+1] =
upper_32_bits(ppgtt-scratch_pd-daddr);
+ reg_state[CTX_PDP1_LDW+1] =
lower_32_bits(ppgtt-scratch_pd-daddr);
+ }
+ if (test_bit(0, ppgtt-pdp.used_pdpes)) {
+ reg_state[CTX_PDP0_UDW+1] =
upper_32_bits(ppgtt-pdp.pagedir[0]-daddr);
+ reg_state[CTX_PDP0_LDW+1] =
lower_32_bits(ppgtt-pdp.pagedir[0]-daddr);
+ } else {
+ reg_state[CTX_PDP0_UDW+1] =
upper_32_bits(ppgtt-scratch_pd-daddr);
+ reg_state[CTX_PDP0_LDW+1] =
lower_32_bits(ppgtt-scratch_pd-daddr);
+ }
+ }
+
kunmap_atomic(reg_state);
return 0;
@@ -387,7 +422,7 @@ static void execlists_submit_contexts(struct
intel_engine_cs *ring,
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
WARN_ON(!i915_gem_obj_is_pinned(ringbuf0-obj));
- execlists_update_context(ctx_obj0, ringbuf0-obj, tail0);
+ execlists_update_context(ctx_obj0, ringbuf0-obj, to0-ppgtt, tail0);
if (to1) {
ringbuf1 = to1-engine[ring-id].ringbuf;
@@ -396,7 +431,7 @@ static void execlists_submit_contexts(struct
intel_engine_cs *ring,
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
WARN_ON(!i915_gem_obj_is_pinned(ringbuf1-obj));
- execlists_update_context(ctx_obj1, ringbuf1-obj, tail1);
+ execlists_update_context(ctx_obj1, ringbuf1-obj, to1-ppgtt,
tail1);
}
execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
@@ -1731,14 +1766,39 @@ populate_lr_context(struct intel_context *ctx, struct
drm_i915_gem_object *ctx_o
reg_state[CTX_PDP1_LDW] = GEN8_RING_PDP_LDW(ring, 1);
reg_state[CTX_PDP0_UDW] = GEN8_RING_PDP_UDW(ring, 0);
reg_state[CTX_PDP0_LDW] = GEN8_RING_PDP_LDW(ring, 0);
- reg_state[CTX_PDP3_UDW+1] = upper_32_bits(ppgtt-pdp.pagedir[3]-daddr);
- reg_state[CTX_PDP3_LDW+1] = lower_32_bits(ppgtt-pdp.pagedir[3]-daddr);
- reg_state[CTX_PDP2_UDW+1] = upper_32_bits(ppgtt-pdp.pagedir[2]-daddr);
- reg_state[CTX_PDP2_LDW+1] = lower_32_bits(ppgtt-pdp.pagedir[2]-daddr);
- reg_state[CTX_PDP1_UDW+1] = upper_32_bits(ppgtt-pdp.pagedir[1]-daddr);
-
[Intel-gfx] [PATCH v2 19/24] drm/i915: num_pd_pages/num_pd_entries isn't useful
From: Ben Widawsky benjamin.widaw...@intel.com
These values are never quite useful for dynamic allocations of the page
tables. Getting rid of them will help prevent later confusion.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 --
drivers/gpu/drm/i915/i915_gem_gtt.c | 68 -
drivers/gpu/drm/i915/i915_gem_gtt.h | 7 ++--
3 files changed, 27 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0f63076..b00760b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2117,8 +2117,6 @@ static void gen8_ppgtt_info(struct seq_file *m, struct
drm_device *dev)
if (!ppgtt)
return;
- seq_printf(m, Page directories: %d\n, ppgtt-num_pd_pages);
- seq_printf(m, Page tables: %d\n, ppgtt-num_pd_entries);
for_each_ring(ring, dev_priv, unused) {
seq_printf(m, %s\n, ring-name);
for (i = 0; i 4; i++) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bd6cb2f..c40db0e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -607,7 +607,7 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt
*ppgtt)
struct pci_dev *hwdev = ppgtt-base.dev-pdev;
int i, j;
- for (i = 0; i ppgtt-num_pd_pages; i++) {
+ for (i = 0; i GEN8_PDES_PER_PAGE; i++) {
/* TODO: In the future we'll support sparse mappings, so this
* will have to change. */
if (!ppgtt-pdp.pagedir[i]-daddr)
@@ -688,21 +688,13 @@ static int gen8_ppgtt_alloc_pagedirs(struct
i915_pagedirpo *pdp,
pdp-pagedir[pdpe] = alloc_pd_single();
if (IS_ERR(ppgtt-pdp.pagedir[pdpe]))
goto unwind_out;
-
- ppgtt-num_pd_pages++;
}
- BUG_ON(ppgtt-num_pd_pages GEN8_LEGACY_PDPES);
-
return 0;
unwind_out:
- while (pdpe--) {
+ while (pdpe--)
free_pd_single(ppgtt-pdp.pagedir[pdpe]);
- ppgtt-num_pd_pages--;
- }
-
- WARN_ON(ppgtt-num_pd_pages);
return -ENOMEM;
}
@@ -725,12 +717,8 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
ppgtt-base.dev);
if (ret)
goto err_out;
-
- ppgtt-num_pd_entries += GEN8_PDES_PER_PAGE;
}
- BUG_ON(pdpe ppgtt-num_pd_pages);
-
return 0;
/* TODO: Check this for all cases */
@@ -792,7 +780,6 @@ static int gen8_ppgtt_setup_page_tables(struct
i915_hw_ppgtt *ppgtt,
static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
{
const int max_pdp = DIV_ROUND_UP(size, 1 30);
- const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
int i, j, ret;
if (size % (130))
@@ -855,11 +842,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
ppgtt-base.insert_entries = gen8_ppgtt_insert_entries;
ppgtt-base.cleanup = gen8_ppgtt_cleanup;
- 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,
-ppgtt-num_pd_entries,
-(ppgtt-num_pd_entries - min_pt_pages) + size %
(130));
return 0;
bail:
@@ -870,26 +852,20 @@ 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;
+ struct i915_pagetab *unused;
gen6_gtt_pte_t scratch_pte;
uint32_t pd_entry;
- int pte, pde;
+ uint32_t pte, pde, temp;
+ uint32_t start = ppgtt-base.start, length = ppgtt-base.total;
scratch_pte = vm-pte_encode(vm-scratch.addr, I915_CACHE_LLC, true, 0);
- 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);
- for (pde = 0; pde ppgtt-num_pd_entries; pde++) {
+ gen6_for_each_pde(unused, ppgtt-pd, start, length, temp, pde) {
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);
[Intel-gfx] [PATCH v2 20/24] drm/i915: Extract PPGTT param from pagedir alloc
From: Ben Widawsky benjamin.widaw...@intel.com
Now that we don't need to trace num_pd_pages, we may as well kill all
need for the PPGTT structure in the alloc_pagedirs. This is very useful
for when we move to 48b addressing, and the PDP isn't the root of the
page table structure.
The param is replaced with drm_device, which is an unavoidable wart
throughout the series. (in other words, not extra flagrant).
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c40db0e..6d67660 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -674,8 +674,6 @@ static int gen8_ppgtt_alloc_pagedirs(struct i915_pagedirpo
*pdp,
uint64_t start,
uint64_t length)
{
- struct i915_hw_ppgtt *ppgtt =
- container_of(pdp, struct i915_hw_ppgtt, pdp);
struct i915_pagedir *unused;
uint64_t temp;
uint32_t pdpe;
@@ -686,7 +684,7 @@ static int gen8_ppgtt_alloc_pagedirs(struct i915_pagedirpo
*pdp,
gen8_for_each_pdpe(unused, pdp, start, length, temp, pdpe) {
BUG_ON(unused);
pdp-pagedir[pdpe] = alloc_pd_single();
- if (IS_ERR(ppgtt-pdp.pagedir[pdpe]))
+ if (IS_ERR(pdp-pagedir[pdpe]))
goto unwind_out;
}
@@ -694,7 +692,7 @@ static int gen8_ppgtt_alloc_pagedirs(struct i915_pagedirpo
*pdp,
unwind_out:
while (pdpe--)
- free_pd_single(ppgtt-pdp.pagedir[pdpe]);
+ free_pd_single(pdp-pagedir[pdpe]);
return -ENOMEM;
}
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 01/24] drm/i915: Add some extra guards in evict_vm
From: Ben Widawsky benjamin.widaw...@intel.com
v2: Use WARN_ONs (Daniel)
Cc: Daniel Vetter dan...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_evict.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c
b/drivers/gpu/drm/i915/i915_gem_evict.c
index 886ff2e..3dc7b37 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -214,6 +214,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool
do_idle)
struct i915_vma *vma, *next;
int ret;
+ WARN_ON(!mutex_is_locked(vm-dev-struct_mutex));
trace_i915_gem_evict_vm(vm);
if (do_idle) {
@@ -222,6 +223,8 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool
do_idle)
return ret;
i915_gem_retire_requests(vm-dev);
+
+ WARN_ON(!list_empty(vm-active_list));
}
list_for_each_entry_safe(vma, next, vm-inactive_list, mm_list)
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 15/24] drm/i915/bdw: Use dynamic allocation idioms on free
From: Ben Widawsky benjamin.widaw...@intel.com
The page directory freer is left here for now as it's still useful given
that GEN8 still preallocates. Once the allocation functions are broken
up into more discrete chunks, we'll follow suit and destroy this
leftover piece.
v2: Match trace_i915_va_teardown params
v3: Multiple rebases.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2, v3)
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 54 +++--
drivers/gpu/drm/i915/i915_gem_gtt.h | 46 +++
2 files changed, 80 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 32a355a..971c05b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -579,27 +579,32 @@ static void gen8_ppgtt_insert_entries(struct
i915_address_space *vm,
}
}
-static void gen8_free_page_tables(struct i915_pagedir *pd, struct drm_device
*dev)
+static void gen8_teardown_va_range(struct i915_address_space *vm,
+ uint64_t start, uint64_t length)
{
- int i;
-
- if (!pd-page)
- return;
-
- for (i = 0; i GEN8_PDES_PER_PAGE; i++) {
- free_pt_single(pd-page_tables[i], dev);
- pd-page_tables[i] = NULL;
+ struct i915_hw_ppgtt *ppgtt =
+ container_of(vm, struct i915_hw_ppgtt, base);
+ struct i915_pagedir *pd;
+ struct i915_pagetab *pt;
+ uint64_t temp;
+ uint32_t pdpe, pde;
+
+ gen8_for_each_pdpe(pd, ppgtt-pdp, start, length, temp, pdpe) {
+ uint64_t pd_len = gen8_clamp_pd(start, length);
+ uint64_t pd_start = start;
+ gen8_for_each_pde(pt, pd, pd_start, pd_len, temp, pde) {
+ free_pt_single(pt, vm-dev);
+ }
+ free_pd_single(pd);
}
}
-static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
+/* This function will die soon */
+static void gen8_free_full_pagedir(struct i915_hw_ppgtt *ppgtt, int i)
{
- int i;
-
- 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]);
- }
+ gen8_teardown_va_range(ppgtt-base,
+ i GEN8_PDPE_SHIFT,
+ (1 GEN8_PDPE_SHIFT));
}
static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
@@ -614,19 +619,28 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt
*ppgtt)
continue;
pci_unmap_page(hwdev, ppgtt-pdp.pagedir[i]-daddr, PAGE_SIZE,
- PCI_DMA_BIDIRECTIONAL);
+ PCI_DMA_BIDIRECTIONAL);
for (j = 0; j GEN8_PDES_PER_PAGE; j++) {
struct i915_pagedir *pd = ppgtt-pdp.pagedir[i];
- struct i915_pagetab *pt = pd-page_tables[j];
+ 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);
+ PCI_DMA_BIDIRECTIONAL);
}
}
}
+static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
+{
+ trace_i915_va_teardown(ppgtt-base,
+ ppgtt-base.start, ppgtt-base.total,
+ VM_TO_TRACE_NAME(ppgtt-base));
+ gen8_teardown_va_range(ppgtt-base,
+ ppgtt-base.start, ppgtt-base.total);
+}
+
static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
{
struct i915_hw_ppgtt *ppgtt =
@@ -651,7 +665,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], ppgtt-base.dev);
+ gen8_free_full_pagedir(ppgtt, i);
return -ENOMEM;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index dc71cae..96209c2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -387,6 +387,52 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
return i915_pde_index(addr, GEN6_PDE_SHIFT);
}
+#define gen8_for_each_pde(pt, pd, start, length, temp, iter) \
+ for (iter = gen8_pde_index(start), pt = (pd)-page_tables[iter]; \
+length 0 iter GEN8_PDES_PER_PAGE; \
+pt = (pd)-page_tables[++iter],\
+temp = ALIGN(start+1, 1 GEN8_PDE_SHIFT) - start,\
+temp = min(temp, length),
[Intel-gfx] [PATCH v2 05/24] drm/i915/gen8: Un-hardcode number of page directories
From: Ben Widawsky benjamin.widaw...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 9d998ec..8f76990 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -282,7 +282,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_PDPES];
};
struct drm_i915_file_private *file_priv;
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 12/24] drm/i915: Track page table reload need
From: Ben Widawsky benjamin.widaw...@intel.com
This patch was formerly known as, Force pd restore when PDEs change,
gen6-7. I had to change the name because it is needed for GEN8 too.
The real issue this is trying to solve is when a new object is mapped
into the current address space. The GPU does not snoop the new mapping
so we must do the gen specific action to reload the page tables.
GEN8 and GEN7 do differ in the way they load page tables for the RCS.
GEN8 does so with the context restore, while GEN7 requires the proper
load commands in the command streamer. Non-render is similar for both.
Caveat for GEN7
The docs say you cannot change the PDEs of a currently running context.
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.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
squash! drm/i915: Force pd restore when PDEs change, gen6-7
It's not just for gen8. If the current context has mappings change, we
need a context reload to switch
v2: Rebased after ppgtt clean up patches. Split the warning for aliasing
and true ppgtt options. And do not break aliasing ppgtt, where to-ppgtt
is always null.
v3: Invalidate PPGTT TLBs inside alloc_va_range and teardown_va_range.
Signed-off-by: Michel Thierry michel.thie...@intel.com (v2+)
---
drivers/gpu/drm/i915/i915_gem_context.c| 27 ++-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++
drivers/gpu/drm/i915/i915_gem_gtt.c| 12
drivers/gpu/drm/i915/i915_gem_gtt.h| 2 ++
4 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 7b20bd4..fa9d4a1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -567,8 +567,18 @@ static inline bool should_skip_switch(struct
intel_engine_cs *ring,
struct intel_context *from,
struct intel_context *to)
{
- if (from == to !to-remap_slice)
- return true;
+ struct drm_i915_private *dev_priv = ring-dev-dev_private;
+
+ if (to-remap_slice)
+ return false;
+
+ if (to-ppgtt) {
+ if (from == to !test_bit(ring-id,
to-ppgtt-base.pd_reload_mask))
+ return true;
+ } else {
+ if (from == to !test_bit(ring-id,
dev_priv-mm.aliasing_ppgtt-base.pd_reload_mask))
+ return true;
+ }
return false;
}
@@ -585,9 +595,8 @@ needs_pd_load_pre(struct intel_engine_cs *ring, struct
intel_context *to)
static bool
needs_pd_load_post(struct intel_engine_cs *ring, struct intel_context *to)
{
- return (!to-legacy_hw_ctx.initialized ||
- i915_gem_context_is_default(to))
- to-ppgtt IS_GEN8(ring-dev);
+ return IS_GEN8(ring-dev)
+ (to-ppgtt || to-ppgtt-base.pd_reload_mask);
}
static int do_switch(struct intel_engine_cs *ring,
@@ -632,6 +641,12 @@ static int do_switch(struct intel_engine_cs *ring,
ret = to-ppgtt-switch_mm(to-ppgtt, ring);
if (ret)
goto unpin_out;
+
+ /* Doing a PD load always reloads the page dirs */
+ if (to-ppgtt)
+ clear_bit(ring-id, to-ppgtt-base.pd_reload_mask);
+ else
+ clear_bit(ring-id,
dev_priv-mm.aliasing_ppgtt-base.pd_reload_mask);
}
if (ring != dev_priv-ring[RCS]) {
@@ -670,6 +685,8 @@ static int do_switch(struct intel_engine_cs *ring,
*/
if (!to-legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
hw_flags |= MI_RESTORE_INHIBIT;
+ else if (to-ppgtt test_and_clear_bit(ring-id,
to-ppgtt-base.pd_reload_mask))
+ hw_flags |= MI_FORCE_RESTORE;
ret = mi_set_context(ring, to, hw_flags);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8330660..09d864f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1199,6 +1199,13 @@ i915_gem_ringbuffer_submission(struct drm_device *dev,
struct drm_file *file,
if (ret)
goto error;
+ if
[Intel-gfx] [PATCH v2 10/24] drm/i915: Track GEN6 page table usage
From: Ben Widawsky benjamin.widaw...@intel.com
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.
One important change introduced here is that DMA mappings are
created/destroyed at the same page directories/tables are
allocated/deallocated.
Notice that aliasing PPGTT is not managed here. The patch which actually
begins dynamic allocation/teardown explains the reasoning for this.
v2: s/pdp.pagedir/pdp.pagedirs
Make a scratch page allocation helper
v3: Rebase and expand commit message.
v4: Allocate required pagetables only when it is needed, _bind_to_vm
instead of bind_vma (Daniel).
Cc: Daniel Vetter dan...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com (v3+)
---
drivers/gpu/drm/i915/i915_gem.c | 9 ++
drivers/gpu/drm/i915/i915_gem_gtt.c | 277 ++--
drivers/gpu/drm/i915/i915_gem_gtt.h | 149 ++-
3 files changed, 322 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2b6ecfd..5d52990 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3597,6 +3597,15 @@ search_free:
if (ret)
goto err_remove_node;
+ /* allocate before insert / bind */
+ if (vma-vm-allocate_va_range) {
+ ret = vma-vm-allocate_va_range(vma-vm,
+ vma-node.start,
+ vma-node.size);
+ if (ret)
+ goto err_remove_node;
+ }
+
trace_i915_vma_bind(vma, flags);
ret = i915_vma_bind(vma, obj-cache_level,
flags PIN_GLOBAL ? GLOBAL_BIND : 0);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 52bdde7..313432e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -138,10 +138,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev,
int enable_ppgtt)
return has_aliasing_ppgtt ? 1 : 0;
}
-
static void ppgtt_bind_vma(struct i915_vma *vma,
- enum i915_cache_level cache_level,
- u32 flags);
+ enum i915_cache_level cache_level,
+ u32 flags);
static void ppgtt_unbind_vma(struct i915_vma *vma);
static inline gen8_gtt_pte_t gen8_pte_encode(dma_addr_t addr,
@@ -275,27 +274,99 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
return pte;
}
-static void free_pt_single(struct i915_pagetab *pt)
-{
+#define i915_dma_unmap_single(px, dev) do { \
+ pci_unmap_page((dev)-pdev, (px)-daddr, 4096, PCI_DMA_BIDIRECTIONAL); \
+} while (0);
+
+/**
+ * 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. If we keep the structs
+ * symmetric here, the simple macro covers us for every page table type.
+ *
+ * Return: 0 if success.
+ */
+#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,
+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_PAGE : I915_PPGTT_PT_ENTRIES;
+ 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);
}
-static struct i915_pagetab *alloc_pt_single(void)
+#define free_pt_single(pt, dev) \
+
[Intel-gfx] [PATCH v2 17/24] drm/i915/bdw: pagetable allocation rework
From: Ben Widawsky benjamin.widaw...@intel.com
Start using gen8_for_each_pde macro to allocate page tables.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Michel Thierry michel.thie...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 54 -
drivers/gpu/drm/i915/i915_gem_gtt.h | 10 +++
2 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e759a03..f928c10 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -601,14 +601,6 @@ static void gen8_teardown_va_range(struct
i915_address_space *vm,
}
}
-/* This function will die soon */
-static void gen8_free_full_pagedir(struct i915_hw_ppgtt *ppgtt, int i)
-{
- gen8_teardown_va_range(ppgtt-base,
- i GEN8_PDPE_SHIFT,
- (1 GEN8_PDPE_SHIFT));
-}
-
static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
{
struct pci_dev *hwdev = ppgtt-base.dev-pdev;
@@ -652,22 +644,27 @@ static void gen8_ppgtt_cleanup(struct i915_address_space
*vm)
gen8_ppgtt_free(ppgtt);
}
-static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt)
+static int gen8_ppgtt_alloc_pagetabs(struct i915_pagedir *pd,
+uint64_t start,
+uint64_t length,
+struct drm_device *dev)
{
- int i, ret;
+ struct i915_pagetab *unused;
+ uint64_t temp;
+ uint32_t pde;
- for (i = 0; i ppgtt-num_pd_pages; i++) {
- ret = alloc_pt_range(ppgtt-pdp.pagedir[i],
-0, GEN8_PDES_PER_PAGE, ppgtt-base.dev);
- if (ret)
+ gen8_for_each_pde(unused, pd, start, length, temp, pde) {
+ BUG_ON(unused);
+ pd-page_tables[pde] = alloc_pt_single(dev);
+ if (IS_ERR(pd-page_tables[pde]))
goto unwind_out;
}
return 0;
unwind_out:
- while (i--)
- gen8_free_full_pagedir(ppgtt, i);
+ while (pde--)
+ free_pt_single(pd-page_tables[pde], dev);
return -ENOMEM;
}
@@ -710,20 +707,28 @@ unwind_out:
}
static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
- const int max_pdp)
+ uint64_t start,
+ uint64_t length)
{
+ struct i915_pagedir *pd;
+ uint64_t temp;
+ uint32_t pdpe;
int ret;
- ret = gen8_ppgtt_alloc_pagedirs(ppgtt-pdp, ppgtt-base.start,
- ppgtt-base.total);
+ ret = gen8_ppgtt_alloc_pagedirs(ppgtt-pdp, start, length);
if (ret)
return ret;
- ret = gen8_ppgtt_allocate_page_tables(ppgtt);
- if (ret)
- goto err_out;
+ gen8_for_each_pdpe(pd, ppgtt-pdp, start, length, temp, pdpe) {
+ ret = gen8_ppgtt_alloc_pagetabs(pd, start, length,
+ ppgtt-base.dev);
+ if (ret)
+ goto err_out;
+
+ ppgtt-num_pd_entries += GEN8_PDES_PER_PAGE;
+ }
- ppgtt-num_pd_entries = max_pdp * GEN8_PDES_PER_PAGE;
+ BUG_ON(pdpe ppgtt-num_pd_pages);
return 0;
@@ -794,10 +799,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
ppgtt-base.start = 0;
ppgtt-base.total = size;
- BUG_ON(ppgtt-base.total == 0);
/* 1. Do all our allocations for page directories and page tables. */
- ret = gen8_ppgtt_alloc(ppgtt, max_pdp);
+ ret = gen8_ppgtt_alloc(ppgtt, ppgtt-base.start, ppgtt-base.total);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 96209c2..74837a3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -403,6 +403,16 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
temp = min(temp, length), \
start += temp, length -= temp)
+/* Clamp length to the next pagetab boundary */
+static inline uint64_t gen8_clamp_pt(uint64_t start, uint64_t length)
+{
+ uint64_t next_pt = ALIGN(start + 1, 1 GEN8_PDE_SHIFT);
+ if (next_pt (start + length))
+ return length;
+
+ return next_pt - start;
+}
+
/* Clamp length to the next pagedir boundary */
static inline uint64_t gen8_clamp_pd(uint64_t start, uint64_t length)
{
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] i915 atomic plane helper conversion (v5)
Updated based on review feedback from Ander. Previous series is available here: http://lists.freedesktop.org/archives/intel-gfx/2014-December/057598.html Matt Roper (5): drm/i915: Refactor work that can sleep out of commit (v6) drm/i915: Move vblank evasion to commit (v4) drm/i915: Clarify sprite plane function names (v4) drm/i915: Move to atomic plane helpers (v9) drm/i915: Drop unused position fields (v2) Documentation/DocBook/drm.tmpl| 5 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 151 drivers/gpu/drm/i915/intel_display.c | 377 +++--- drivers/gpu/drm/i915/intel_drv.h | 50 +++- drivers/gpu/drm/i915/intel_sprite.c | 203 +++- 6 files changed, 473 insertions(+), 314 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: Drop unused position fields (v2)
The userspace-requested plane coordinates are now always available via
plane-state.base (and the i915-adjusted values are stored in
plane-state), so we no longer use the coordinate fields in intel_plane
and can drop them.
Also, note that the error case for pageflip calls update_plane() to
program the values from plane-state; it's simpler to just call
intel_plane_restore() which does the same thing.
v2: Replace manual update_plane() with intel_plane_restore() in pageflip
error handler.
Reviewed-by(v1): Bob Paauwe bob.j.paa...@intel.com
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
Reviewed-by: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 27 +--
drivers/gpu/drm/i915/intel_drv.h | 4
drivers/gpu/drm/i915/intel_sprite.c | 19 ---
3 files changed, 5 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e7b51db..75a514d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9616,7 +9616,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *primary = crtc-primary;
- struct intel_plane *intel_plane = to_intel_plane(primary);
enum pipe pipe = intel_crtc-pipe;
struct intel_unpin_work *work;
struct intel_engine_cs *ring;
@@ -9775,15 +9774,7 @@ free_work:
if (ret == -EIO) {
out_hang:
- ret = primary-funcs-update_plane(primary, crtc, fb,
- intel_plane-crtc_x,
- intel_plane-crtc_y,
- intel_plane-crtc_h,
- intel_plane-crtc_w,
- intel_plane-src_x,
- intel_plane-src_y,
- intel_plane-src_h,
- intel_plane-src_w);
+ ret = intel_plane_restore(primary);
if (ret == 0 event) {
spin_lock_irq(dev-event_lock);
drm_send_vblank_event(dev, pipe, event);
@@ -11823,14 +11814,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
crtc-x = src-x1 16;
crtc-y = src-y1 16;
- intel_plane-crtc_x = state-base.crtc_x;
- intel_plane-crtc_y = state-base.crtc_y;
- intel_plane-crtc_w = state-base.crtc_w;
- intel_plane-crtc_h = state-base.crtc_h;
- intel_plane-src_x = state-base.src_x;
- intel_plane-src_y = state-base.src_y;
- intel_plane-src_w = state-base.src_w;
- intel_plane-src_h = state-base.src_h;
intel_plane-obj = obj;
if (intel_crtc-active) {
@@ -12111,14 +12094,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
crtc-cursor_x = state-base.crtc_x;
crtc-cursor_y = state-base.crtc_y;
- intel_plane-crtc_x = state-base.crtc_x;
- intel_plane-crtc_y = state-base.crtc_y;
- intel_plane-crtc_w = state-base.crtc_w;
- intel_plane-crtc_h = state-base.crtc_h;
- intel_plane-src_x = state-base.src_x;
- intel_plane-src_y = state-base.src_y;
- intel_plane-src_w = state-base.src_w;
- intel_plane-src_h = state-base.src_h;
intel_plane-obj = obj;
if (intel_crtc-cursor_bo == obj)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 604bd22..db3fde3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -517,10 +517,6 @@ struct intel_plane {
struct drm_i915_gem_object *obj;
bool can_scale;
int max_downscale;
- int crtc_x, crtc_y;
- unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y;
- uint32_t src_w, src_h;
unsigned int rotation;
/* Since we need to change the watermarks before/after
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 16e7c96..37079f1 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1277,15 +1277,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
- intel_plane-crtc_x = state-base.crtc_x;
- intel_plane-crtc_y = state-base.crtc_y;
- intel_plane-crtc_w = state-base.crtc_w;
- intel_plane-crtc_h = state-base.crtc_h;
- intel_plane-src_x = state-base.src_x;
- intel_plane-src_y = state-base.src_y;
- intel_plane-src_w = state-base.src_w;
- intel_plane-src_h = state-base.src_h;
-
crtc = crtc ? crtc : plane-crtc;
[Intel-gfx] [PATCH 4/5] drm/i915: Move to atomic plane helpers (v9)
Switch plane handling to use the atomic plane helpers. This means that rather than provide our own implementations of .update_plane() and .disable_plane(), we expose the lower-level check/prepare/commit/cleanup entrypoints and let the DRM core implement update/disable for us using those entrypoints. The other main change that falls out of this patch is that our drm_plane's will now always have a valid plane-state that contains the relevant plane state (initial state is allocated at plane creation). The base drm_plane_state pointed to holds the requested source/dest coordinates, and the subclassed intel_plane_state holds the adjusted values that our driver actually uses. v2: - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel) - Fix a copy/paste comment mistake (Bob) v3: - Use prepare/cleanup functions that we've already factored out - Use newly refactored pre_commit/commit/post_commit to avoid sleeping during vblank evasion v4: - Rebase to latest di-nightly requires adding an 'old_state' parameter to atomic_update; v5: - Must have botched a rebase somewhere and lost some work. Restore state 'dirty' flag to let begin/end code know which planes to run the pre_commit/post_commit hooks for. This would have actually shown up as broken in the next commit rather than this one. v6: - Squash kerneldoc patch into this one. - Previous patches have now already taken care of most of the infrastructure that used to be in this patch. All we're adding here now is some thin wrappers. v7: - Check return of intel_plane_duplicate_state() for allocation failures. v8: - Drop unused drm_plane_state - intel_plane_state cast. (Ander) - Squash in actual transition to plane helpers. Significant refactoring earlier in the patchset has made the combined prep+transition much easier to swallow than it was in earlier iterations. (Ander) v9: - s/track_fbs/disabled_planes/ in the atomic crtc flags. The only fb's we need to update frontbuffer tracking for are those on a plane about to be disabled (since the atomic helpers never call prepare_fb() when disabling a plane), so the new name more accurately describes what we're actually tracking. Testcase: igt/kms_plane Testcase: igt/kms_universal_plane Testcase: igt/kms_cursor_crc Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- Documentation/DocBook/drm.tmpl| 5 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 151 ++ drivers/gpu/drm/i915/intel_display.c | 251 +- drivers/gpu/drm/i915/intel_drv.h | 10 +- drivers/gpu/drm/i915/intel_sprite.c | 50 -- 6 files changed, 303 insertions(+), 165 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 3b2571e..169d205 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4018,6 +4018,11 @@ int num_ioctls;/synopsis /para /sect2 sect2 +titleAtomic Plane Helpers/title +!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers +!Idrivers/gpu/drm/i915/intel_atomic_plane.c + /sect2 + sect2 titleOutput Probing/title para This section covers output probing and related infrastructure like the diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 1849ffa..16e3dc3 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -66,6 +66,7 @@ i915-y += dvo_ch7017.o \ dvo_ns2501.o \ dvo_sil164.o \ dvo_tfp410.o \ + intel_atomic_plane.o \ intel_crt.o \ intel_ddi.o \ intel_dp.o \ diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c new file mode 100644 index 000..e7425d6c --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -0,0 +1,151 @@ +/* + * 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
[Intel-gfx] [PATCH 3/5] drm/i915: Clarify sprite plane function names (v4)
A few of the sprite-related function names in i915 are very similar
(e.g., intel_enable_planes() vs intel_crtc_enable_planes()) and don't
make it clear whether they only operate on sprite planes, or whether
they also apply to all universal plane types. Rename a few functions to
be more consistent with our function naming for primary/cursor planes or
to clarify that they apply specifically to sprite planes:
- s/intel_disable_planes/intel_disable_sprite_planes/
- s/intel_enable_planes/intel_enable_sprite_planes/
Also, drop the sprite-specific intel_destroy_plane() and just use
the type-agnostic intel_plane_destroy() function. The extra 'disable'
call that intel_destroy_plane() did is unnecessary since the plane will
already be disabled due to framebuffer destruction by the point it gets
called.
v2: Earlier consolidation patches have reduced the number of functions
we need to rename here.
v3: Also rename intel_plane_funcs vtable to intel_sprite_plane_funcs
for consistency with primary/cursor. (Ander)
v4: Convert comment for intel_plane_destroy() to kerneldoc now that it
is no longer a static function. (Ander)
Reviewed-by(v1): Bob Paauwe bob.j.paa...@intel.com
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 --
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 14 +++---
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8d6e710..4bb1a37 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4037,7 +4037,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
}
}
-static void intel_enable_planes(struct drm_crtc *crtc)
+static void intel_enable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
enum pipe pipe = to_intel_crtc(crtc)-pipe;
@@ -4051,7 +4051,7 @@ static void intel_enable_planes(struct drm_crtc *crtc)
}
}
-static void intel_disable_planes(struct drm_crtc *crtc)
+static void intel_disable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
enum pipe pipe = to_intel_crtc(crtc)-pipe;
@@ -4195,7 +4195,7 @@ static void intel_crtc_enable_planes(struct drm_crtc
*crtc)
int pipe = intel_crtc-pipe;
intel_enable_primary_hw_plane(crtc-primary, crtc);
- intel_enable_planes(crtc);
+ intel_enable_sprite_planes(crtc);
intel_crtc_update_cursor(crtc, true);
intel_crtc_dpms_overlay(intel_crtc, true);
@@ -4230,7 +4230,7 @@ static void intel_crtc_disable_planes(struct drm_crtc
*crtc)
intel_crtc_dpms_overlay(intel_crtc, false);
intel_crtc_update_cursor(crtc, false);
- intel_disable_planes(crtc);
+ intel_disable_sprite_planes(crtc);
intel_disable_primary_hw_plane(crtc-primary, crtc);
/*
@@ -12012,8 +12012,14 @@ intel_disable_plane(struct drm_plane *plane)
0, 0, 0, 0, 0, 0, 0, 0);
}
-/* Common destruction function for both primary and cursor planes */
-static void intel_plane_destroy(struct drm_plane *plane)
+/**
+ * intel_plane_destroy - destroy a plane
+ * @plane: plane to destroy
+ *
+ * Common destruction function for all types of planes (primary, cursor,
+ * sprite).
+ */
+void intel_plane_destroy(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
drm_plane_cleanup(plane);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1934156..2523315 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1053,6 +1053,7 @@ int intel_update_plane(struct drm_plane *plane, struct
drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
int intel_disable_plane(struct drm_plane *plane);
+void intel_plane_destroy(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int
conn_id);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 18be827..537fff25 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1306,14 +1306,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static void intel_destroy_plane(struct drm_plane *plane)
-{
- struct intel_plane *intel_plane = to_intel_plane(plane);
- intel_disable_plane(plane);
- drm_plane_cleanup(plane);
- kfree(intel_plane);
-}
-
int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -1413,10 +1405,10 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane-src_w, intel_plane-src_h);
}
[Intel-gfx] [PATCH 2/5] drm/i915: Move vblank evasion to commit (v4)
Move the vblank evasion up from the low-level, hw-specific
update_plane() handlers to the general plane commit operation.
Everything inside commit should now be non-sleeping, so this brings us
closer to how vblank evasion will behave once we move over to atomic.
v2:
- Restore lost intel_crtc-active check on vblank evasion
v3:
- Replace assert_pipe_enabled() in intel_disable_primary_hw_plane()
with an intel_crtc-active test; it turns out assert_pipe_enabled()
grabs some mutexes and can sleep, which we can't do with interrupts
disabled.
v4:
- Equivalent to v2; v3 change is now squashed into an earlier patch
of the series. (Ander).
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 10 +
drivers/gpu/drm/i915/intel_drv.h | 4
drivers/gpu/drm/i915/intel_sprite.c | 42
3 files changed, 14 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6bd44f3..8d6e710 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11864,6 +11864,12 @@ static void intel_begin_crtc_commit(struct drm_crtc
*crtc)
intel_update_watermarks(crtc);
intel_runtime_pm_get(dev_priv);
+
+ /* Perform vblank evasion around commit operation */
+ if (intel_crtc-active)
+ intel_crtc-atomic.evade =
+ intel_pipe_update_start(intel_crtc,
+
intel_crtc-atomic.start_vbl_count);
}
static void intel_finish_crtc_commit(struct drm_crtc *crtc)
@@ -11873,6 +11879,10 @@ static void intel_finish_crtc_commit(struct drm_crtc
*crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *p;
+ if (intel_crtc-atomic.evade)
+ intel_pipe_update_end(intel_crtc,
+ intel_crtc-atomic.start_vbl_count);
+
intel_runtime_pm_put(dev_priv);
if (intel_crtc-atomic.wait_vblank)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a03bd72..1934156 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -428,6 +428,10 @@ struct skl_pipe_wm {
* and thus can't be run with interrupts disabled.
*/
struct intel_crtc_atomic_commit {
+ /* vblank evasion */
+ bool evade;
+ unsigned start_vbl_count;
+
/* Sleepable operations to perform before commit */
bool wait_for_flips;
bool disable_fbc;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 6e649de..18be827 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -412,8 +412,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
- u32 start_vbl_count;
- bool atomic_update;
sprctl = I915_READ(SPCNTR(pipe, plane));
@@ -502,8 +500,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
linear_offset += src_h * fb-pitches[0] + src_w * pixel_size;
}
- atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
-
intel_update_primary_plane(intel_crtc);
if (IS_CHERRYVIEW(dev) pipe == PIPE_B)
@@ -525,9 +521,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
sprsurf_offset);
intel_flush_primary_plane(dev_priv, intel_crtc-plane);
-
- if (atomic_update)
- intel_pipe_update_end(intel_crtc, start_vbl_count);
}
static void
@@ -539,10 +532,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct
drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_plane-pipe;
int plane = intel_plane-plane;
- u32 start_vbl_count;
- bool atomic_update;
-
- atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
intel_update_primary_plane(intel_crtc);
@@ -553,9 +542,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc
*crtc)
intel_flush_primary_plane(dev_priv, intel_crtc-plane);
- if (atomic_update)
- intel_pipe_update_end(intel_crtc, start_vbl_count);
-
intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
}
@@ -626,8 +612,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
u32 sprctl, sprscale = 0;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
- u32 start_vbl_count;
- bool atomic_update;
sprctl = I915_READ(SPRCTL(pipe));
@@ -711,8 +695,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
[Intel-gfx] [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v6)
Once we integrate our work into the atomic pipeline, plane commit
operations will need to happen with interrupts disabled, due to vblank
evasion. Our commit functions today include sleepable work, so those
operations need to be split out and run either before or after the
atomic register programming.
The solution here calculates which of those operations will need to be
performed during the 'check' phase and sets flags in an intel_crtc
sub-struct. New intel_begin_crtc_commit() and
intel_finish_crtc_commit() functions are added before and after the
actual register programming; these will eventually be called from the
atomic plane helper's .atomic_begin() and .atomic_end() entrypoints.
v2: Fix broken sprite code split
v3: Make the pre/post commit work crtc-based to match how we eventually
want this to be called from the atomic plane helpers.
v4: Some platforms that haven't had their watermark code reworked were
waiting for vblank, then calling update_sprite_watermarks in their
platform-specific disable code. These also need to be flagged out
of the critical section.
v5: Sprite plane test for primary show/hide should just set the flag to
wait for pending flips, not actually perform the wait. (Ander)
v6:
- Rebase onto latest di-nightly; picks up an important runtime PM fix.
- Handle 'wait_for_flips' flag in intel_begin_crtc_commit(). (Ander)
- Use wait_for_flips flag for primary plane update rather than
performing the wait in the check routine.
- Added kerneldoc to pre_disable/post_enable functions that are no
longer static. (Ander)
- Replace assert_pipe_enabled() in intel_disable_primary_hw_plane()
with an intel_crtc-active test; it turns out assert_pipe_enabled()
grabs some mutexes and can sleep, which we can't do with interrupts
disabled.
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 199 ++-
drivers/gpu/drm/i915/intel_drv.h | 31 ++
drivers/gpu/drm/i915/intel_sprite.c | 98 -
3 files changed, 209 insertions(+), 119 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a03955d..6bd44f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2165,7 +2165,8 @@ static void intel_disable_primary_hw_plane(struct
drm_plane *plane,
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- assert_pipe_enabled(dev_priv, intel_crtc-pipe);
+ if (WARN_ON(!intel_crtc-active))
+ return;
if (!intel_crtc-primary_enabled)
return;
@@ -11737,7 +11738,11 @@ static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
+ struct drm_device *dev = plane-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc = state-base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = state-base.fb;
struct drm_rect *dest = state-dst;
struct drm_rect *src = state-src;
@@ -11752,10 +11757,40 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
- intel_crtc_wait_for_pending_flips(crtc);
- if (intel_crtc_has_pending_flip(crtc)) {
- DRM_ERROR(pipe is still busy with an old pageflip\n);
- return -EBUSY;
+ if (intel_crtc-active) {
+ intel_crtc-atomic.wait_for_flips = true;
+
+ /*
+* FBC does not work on some platforms for rotated
+* planes, so disable it when rotation is not 0 and
+* update it when rotation is set back to 0.
+*
+* FIXME: This is redundant with the fbc update done in
+* the primary plane enable function except that that
+* one is done too late. We eventually need to unify
+* this.
+*/
+ if (intel_crtc-primary_enabled
+ INTEL_INFO(dev)-gen = 4 !IS_G4X(dev)
+ dev_priv-fbc.plane == intel_crtc-plane
+ intel_plane-rotation != BIT(DRM_ROTATE_0)) {
+ intel_crtc-atomic.disable_fbc = true;
+ }
+
+ if (state-visible) {
+ /*
+* BDW signals flip done immediately if the plane
+* is disabled, even if the plane enable is already
+* armed to occur at the next vblank :(
+*/
+ if (IS_BROADWELL(dev) !intel_crtc-primary_enabled)
+ intel_crtc-atomic.wait_vblank = true;
+ }
+
+
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Drop unused position fields (v2)
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) -Summary- Platform Delta drm-intel-nightly Series Applied PNV 363/364 363/364 ILK -1 364/366 363/366 SNB -2 447/450 445/450 IVB -2 496/498 494/498 BYT 288/289 288/289 HSW -1 562/564 561/564 BDW 415/417 415/417 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt_gem_reloc_vs_gpu_forked-faulting-reloc-thrash-inactive PASS(2, M37) DMESG_WARN(1, M37) *SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M35) DMESG_WARN(1, M35) *SNB igt_kms_rotation_crc_sprite-rotation PASS(2, M35) DMESG_WARN(1, M35) *IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M34) DMESG_WARN(1, M34) *IVB igt_kms_rotation_crc_sprite-rotation PASS(2, M34) DMESG_WARN(1, M34) *HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M40) DMESG_WARN(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] new behaviors: ?2.99.917?
4 series onboard video rev 03 (G41) using DVI to HDMI cable on DVI motherboard connector, which Xorg reports as HDMI1 Display is 1920x1080 LCD TV, using HDMI connector #4 Rawhide kernel 3.18.1 Xorg 1.16.3RC1 xorg-x11-drv-intel-2.99.917 1-connected and unconnected absent from Xorg.0.log 2-EDID preferred absent from Xorg.0.log 3-EDID preferred is not used by Xorg 4-video= on cmdline if present and valid is used by Xorg instead of EDID preferred Are each or any of these now expected? None occur in F20, kernel 3.17.7, server 1.14.3, driver 2.21.15. http://fm.no-ip.com/Tmp/Linux/Xorg/xorg.0.log-f22-i299917drv-1024x768-brokenAutoMagic -- The wise are known for their understanding, and pleasant words are persuasive. Proverbs 16:21 (New Living Translation) Team OS/2 ** Reg. Linux User #211409 ** a11y rocks! Felix Miata *** http://fm.no-ip.com/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] new behaviors: ?2.99.917?
On Wed, Dec 24, 2014 at 12:22:38AM -0500, Felix Miata wrote: 4 series onboard video rev 03 (G41) using DVI to HDMI cable on DVI motherboard connector, which Xorg reports as HDMI1 Display is 1920x1080 LCD TV, using HDMI connector #4 Rawhide kernel 3.18.1 Xorg 1.16.3RC1 xorg-x11-drv-intel-2.99.917 1-connected and unconnected absent from Xorg.0.log That's not in any -intel. You are misinterpreting verbiage in the logfile if you think that is what 2.21.15 says. 2-EDID preferred absent from Xorg.0.log That's not used by any -intel. 3-EDID preferred is not used by Xorg Indeed. 4-video= on cmdline if present and valid is used by Xorg instead of EDID preferred Xorg inherits the current mode. -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
