2014-07-22 04:36:16

by Hugh Dickins

[permalink] [raw]
Subject: [PATCH] drm/i915: fix freeze with blank screen booting highmem

x86_64 boots and displays fine, but booting x86_32 with CONFIG_HIGHMEM
has frozen with a blank screen throughout 3.16-rc on this ThinkPad T420s,
with i915 generation 6 graphics.

Fix 9d0a6fa6c5e6 ("drm/i915: add render state initialization"): kunmap()
takes struct page * argument, not virtual address. Which the compiler
kindly points out, if you use the appropriate u32 *batch, instead of
silencing it with a void *.

Why did bisection lead decisively to nearby 229b0489aa75 ("drm/i915:
add null render states for gen6, gen7 and gen8")? Because the u32
deposited at that virtual address by the previous stub failed the
PageHighMem test, and so did no harm.

Signed-off-by: Hugh Dickins <[email protected]>
---

drivers/gpu/drm/i915/i915_gem_render_state.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

--- 3.16-rc/drivers/gpu/drm/i915/i915_gem_render_state.c 2014-06-16 00:28:52.384076465 -0700
+++ linux/drivers/gpu/drm/i915/i915_gem_render_state.c 2014-07-21 20:10:03.824481521 -0700
@@ -31,7 +31,7 @@
struct i915_render_state {
struct drm_i915_gem_object *obj;
unsigned long ggtt_offset;
- void *batch;
+ u32 *batch;
u32 size;
u32 len;
};
@@ -80,7 +80,7 @@ free:

static void render_state_free(struct i915_render_state *so)
{
- kunmap(so->batch);
+ kunmap(kmap_to_page(so->batch));
i915_gem_object_ggtt_unpin(so->obj);
drm_gem_object_unreference(&so->obj->base);
kfree(so);


2014-07-22 06:11:29

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/i915: fix freeze with blank screen booting highmem

On Mon, Jul 21, 2014 at 09:34:30PM -0700, Hugh Dickins wrote:
> x86_64 boots and displays fine, but booting x86_32 with CONFIG_HIGHMEM
> has frozen with a blank screen throughout 3.16-rc on this ThinkPad T420s,
> with i915 generation 6 graphics.
>
> Fix 9d0a6fa6c5e6 ("drm/i915: add render state initialization"): kunmap()
> takes struct page * argument, not virtual address. Which the compiler
> kindly points out, if you use the appropriate u32 *batch, instead of
> silencing it with a void *.
>
> Why did bisection lead decisively to nearby 229b0489aa75 ("drm/i915:
> add null render states for gen6, gen7 and gen8")? Because the u32
> deposited at that virtual address by the previous stub failed the
> PageHighMem test, and so did no harm.
>
> Signed-off-by: Hugh Dickins <[email protected]>

Oops. Thanks for tracking this down, patch merged to my -fixes queue.
-Daniel

> ---
>
> drivers/gpu/drm/i915/i915_gem_render_state.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- 3.16-rc/drivers/gpu/drm/i915/i915_gem_render_state.c 2014-06-16 00:28:52.384076465 -0700
> +++ linux/drivers/gpu/drm/i915/i915_gem_render_state.c 2014-07-21 20:10:03.824481521 -0700
> @@ -31,7 +31,7 @@
> struct i915_render_state {
> struct drm_i915_gem_object *obj;
> unsigned long ggtt_offset;
> - void *batch;
> + u32 *batch;
> u32 size;
> u32 len;
> };
> @@ -80,7 +80,7 @@ free:
>
> static void render_state_free(struct i915_render_state *so)
> {
> - kunmap(so->batch);
> + kunmap(kmap_to_page(so->batch));
> i915_gem_object_ggtt_unpin(so->obj);
> drm_gem_object_unreference(&so->obj->base);
> kfree(so);

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch