Commit bbf47020 authored by Daniel Vetter's avatar Daniel Vetter Committed by Daniel Vetter

drm/i915: fix oops in primary_check_plane

On Sun, Jul 12, 2015 at 09:52:51AM -0700, Linus Torvalds wrote:
> On Sun, Jul 12, 2015 at 1:03 AM, Jörg Otte <jrg.otte@gmail.com> wrote:
> > BUG: unable to handle kernel NULL pointer dereference at 0000000000000009
> > IP: [<ffffffffbd3447bb>] 0xffffffffbd3447bb
>
> Ugh. Please enable KALLSYMS to get sane symbols.
>
> But yes, "crtc_state->base.active" is at offset 9 from "crtc_state",
> so it's pretty clearly just that change frm
>
> -       if (intel_crtc->active) {
> +       if (crtc_state->base.active) {
>
> and "crtc_state" is NULL.
>
> And the code very much knows that crtc_state can be NULL, since it's
> initialized with
>
>         crtc_state = state->base.state ?
>                 intel_atomic_get_crtc_state(state->base.state,
> intel_crtc) : NULL;
>
> Tssk. Daniel? Should I just revert that commit dec4f799
> ("drm/i915: Use crtc_state->active in primary check_plane func") for
> now, or is there a better fix? Like just checking crtc_state for NULL?

Indeed embarrassing. I've missed that we still have 1 caller left that's
using the transitional helpers, and those don't fill out
plane_state->state backpointers to the global atomic update since there is
no global atomic update for transitional helpers. Below diff should fix
this - we need to preferentially check crts_state->active and if that's
not set intel_crtc->active should yield the right result for the one
remaining caller (it's in the crtc_disable paths).

This fixes a regression introduced in

commit dec4f799
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Jul 7 11:15:47 2015 +0200

    drm/i915: Use crtc_state->active in primary check_plane func

which was quickly reverted in

commit 01e2d062
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun Jul 12 15:00:20 2015 -0700

    Revert "drm/i915: Use crtc_state->active in primary check_plane func"

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jörg Otte <jrg.otte@gmail.com>
Reported-and-tested-by: default avatarJörg Otte <jrg.otte@gmail.com>
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
parent 5ec5b516
...@@ -13276,7 +13276,7 @@ intel_check_primary_plane(struct drm_plane *plane, ...@@ -13276,7 +13276,7 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret) if (ret)
return ret; return ret;
if (intel_crtc->active) { if (crtc_state ? crtc_state->base.active : intel_crtc->active) {
struct intel_plane_state *old_state = struct intel_plane_state *old_state =
to_intel_plane_state(plane->state); to_intel_plane_state(plane->state);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment