Re: [Intel-gfx] [git pull] drm fixes

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Mar 25, 2015 at 12:42:46PM -0400, Josh Boyer wrote:
> On Wed, Mar 25, 2015 at 11:53 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> > On Wed, Mar 25, 2015 at 11:50 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> On Wed, Mar 25, 2015 at 11:37:35AM -0400, Josh Boyer wrote:
> >>> On Wed, Mar 25, 2015 at 10:00 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >>> > On Wed, Mar 25, 2015 at 09:11:17AM -0400, Josh Boyer wrote:
> >>> >> On Wed, Mar 25, 2015 at 4:54 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >>> >> >> >> commit f55548b5af87ebfc586ca75748947f1c1b1a4a52
> >>> >> >> >> Author: Damien Lespiau <damien.lespiau@intel.com>
> >>> >> >> >> Date:   Thu Feb 5 18:30:20 2015 +0000
> >>> >> >> >>
> >>> >> >> >>     drm/i915: Don't try to reference the fb in get_initial_plane_config()
> >>> >> >> >>
> >>> >> >> >> From linux-next?
> >>> >> >> >
> >>> >> >> > Yes, building now.  Will let you know as soon as I test it on both machines.
> >>> >> >>
> >>> >> >> OK, with that commit applied I no longer get the kref.h splat and the
> >>> >> >> NUC machine boots headless.  I still see the backtrace below on both
> >>> >> >> the NUC and the macbook.  I have a copy of it with drm.debug=0xff from
> >>> >> >> the NUC here:
> >>> >> >>
> >>> >> >> https://jwboyer.fedorapeople.org/pub/nuc-drm-debug-ff-with-fixes.txt
> >>> >> >>
> >>> >> >> Getting better at least :).
> >>> >> >
> >>> >> > On top of what you currently have please also cherry-pick
> >>> >> >
> >>> >> > commit fb9981aa675eb7b398849915364916fd98833cfa
> >>> >> > Author: Damien Lespiau <damien.lespiau@intel.com>
> >>> >> > Date:   Thu Feb 5 19:24:25 2015 +0000
> >>> >> >
> >>> >> >     drm/i915: Fix atomic state when reusing the firmware fb
> >>> >> >
> >>> >> > from -next. Let's hope this terminates eventually ;-)
> >>> >>
> >>> >> Hm.  That one doesn't apply cleanly.  I think because it needs:
> >>> >>
> >>> >> From 2d14030b1a9d0e89cfdca6f16851e2eac8cb4de0 Mon Sep 17 00:00:00 2001
> >>> >> From: Damien Lespiau <damien.lespiau@intel.com>
> >>> >> Date: Thu, 5 Feb 2015 17:22:18 +0000
> >>> >> Subject: drm/i915: Store the initial framebuffer in initial_plane_config
> >>> >>
> >>> >> first.  Do you want me to grab both, or should I try and figure out
> >>> >> how to backport fb9981aa67 without it?
> >>> >
> >>> > Oops missed that. The active ingredient is setting crtc->primary->state->crtc like this:
> >>> > -Daniel
> >>> >
> >>> >
> >>> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> > index 1c12262029fb..bfc14a6046ea 100644
> >>> > --- a/drivers/gpu/drm/i915/intel_display.c
> >>> > +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> > @@ -2439,6 +2439,7 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
> >>> >                 return;
> >>> >
> >>> >         if (intel_alloc_plane_obj(intel_crtc, plane_config)) {
> >>> > +               intel_crtc->base.primary->state->crtc = &intel_crtc->base;
> >>> >                 update_state_fb(intel_crtc->base.primary);
> >>> >                 return;
> >>> >         }
> >>> > @@ -2469,6 +2470,7 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
> >>> >
> >>> >                         drm_framebuffer_reference(c->primary->fb);
> >>> >                         intel_crtc->base.primary->fb = c->primary->fb;
> >>> > +                       intel_crtc->base.primary->state->crtc = &intel_crtc->base;
> >>> >                         obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
> >>> >                         break;
> >>> >                 }
> >>>
> >>> Hm.  So I used your patch above.  The macbook boots fine and all the
> >>> oops/WARNS are gone except the audio one that was unrelated and
> >>> present before all of this.
> >>>
> >>> However, the NUC is back to not booting without HDMI plugged in.  I
> >>> did the drm.debug=0xff+blacklist/insmod trick again and put the
> >>> results up here:
> >>>
> >>> https://jwboyer.fedorapeople.org/pub/vetters.txt
> >>>
> >>> The frontbuffer splat is back now.
> >>>
> >>> I confirmed multiple times that the NUC boots fine with the kernel
> >>> that doesn't include the above patch but has the other two included
> >>> (albeit with the drm_atomic WARN still).
> >>>
> >>> Not sure what to make of this one.
> >>
> >> Yeah that fail looks like we're freeing an fb that's still in use.
> >> Hilarity happens and since that happens under console_lock at boot-up your
> >> machine dies.
> >>
> >> Does that machine die the same way in drm-intel-nightly/linux-next?
> >
> > I'll try that a bit later today.  Out of sheer curiosity, I folded
> > commit5ba76c41e55c (drm/i915: Put update_state_fb() next to the fb
> > update) into the patch above and kicked off a build.  The theory is
> > that we're picking up a bunch of other changes right in that range of
> > commits, why not try one more.  I'll let you know if that fixes
> > anything.  Otherwise, I'll try building drm-intel-nightly and/or
> > linux-next after that.
> 
> The drm-intel-nightly build finished first.  It boots without HDMI
> plugged in, but it has pretty much the same splats as the previous
> kernel.  Confused.  Full log here:
> 
> https://jwboyer.fedorapeople.org/pub/intel-nightly.txt
> 
> I don't have much hope for my other build.

Yeah that's at least good news for the theory I've been cooking meanwhile.
Can you try the below diff (on top of next/nightly)? For the current
cherry-pick pile on top of 4.0-rc you'd need to prepend intel_crtc->base.
to primary->...

Thanks, Daniel

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ceb2e61b4c91..cb508542c6ab 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2594,6 +2594,7 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
 
 		primary->fb = &plane_config->fb->base;
 		primary->state->crtc = &intel_crtc->base;
+		primary->crtc = &intel_crtc->base;
 		update_state_fb(primary);
 
 		return;
@@ -2627,6 +2628,7 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
 			drm_framebuffer_reference(c->primary->fb);
 			primary->fb = c->primary->fb;
 			primary->state->crtc = &intel_crtc->base;
+			primary->crtc = &intel_crtc->base;
 			update_state_fb(intel_crtc->base.primary);
 			obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
 			break;
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch