| Message ID | 1423156939-15705-4-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 05, 2015 at 05:22:17PM +0000, Damien Lespiau wrote: > Tvrtko noticed a new warning on boot: > > WARNING: CPU: 1 PID: 353 at include/linux/kref.h:47 drm_framebuffer_reference+0x6c/0x80 [drm]() > Call Trace: > [<ffffffff8161f10c>] dump_stack+0x4f/0x7b > [<ffffffff81052caa>] warn_slowpath_common+0xaa/0xd0 > [<ffffffff81052d8a>] warn_slowpath_null+0x1a/0x20 > [<ffffffffa00d035c>] drm_framebuffer_reference+0x6c/0x80 [drm] > [<ffffffffa01c0df7>] update_state_fb.isra.54+0x47/0x50 [i915] > [<ffffffffa01ccd5c>] skylake_get_initial_plane_config+0x93c/0x950 [i915] > [<ffffffffa01e8721>] intel_modeset_init+0x1551/0x17c0 [i915] > [<ffffffffa02476e0>] i915_driver_load+0xed0/0x11e0 [i915] > [<ffffffff81627aa1>] ? _raw_spin_unlock_irqrestore+0x51/0x70 > [<ffffffffa00ca8b7>] drm_dev_register+0x77/0x110 [drm] > [<ffffffffa00cda3b>] drm_get_pci_dev+0x11b/0x1f0 [drm] > [<ffffffff81098e3d>] ? trace_hardirqs_on+0xd/0x10 > [<ffffffff81627aa1>] ? _raw_spin_unlock_irqrestore+0x51/0x70 > [<ffffffffa0145276>] i915_pci_probe+0x56/0x60 [i915] > [<ffffffff813ad59c>] pci_device_probe+0x7c/0x100 > [<ffffffff81466aad>] driver_probe_device+0x16d/0x380 > > We cannot take a reference at this point, not before > intel_framebuffer_init() and the underlying drm_framebuffer_init(). > > Introduced in: > > commit 706dc7b549175e47f23e913b7f1e52874a7d0f56 > Author: Matt Roper <matthew.d.roper@intel.com> > Date: Tue Feb 3 13:10:04 2015 -0800 > > drm/i915: Ensure plane->state->fb stays in sync with plane->fb > > Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++------------------ > 1 file changed, 17 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3301b61..e1e89d0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2364,6 +2364,20 @@ static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha) > } > } > > +/* Update plane->state->fb to match plane->fb after driver-internal updates */ > +static void > +update_state_fb(struct drm_plane *plane) > +{ > + if (plane->fb == plane->state->fb) > + return; > + > + if (plane->state->fb) > + drm_framebuffer_unreference(plane->state->fb); > + plane->state->fb = plane->fb; > + if (plane->state->fb) > + drm_framebuffer_reference(plane->state->fb); > +} > + Was moving this function without changes intentional? It seems to be unrelated to the other changes here. Anyway, #1-4 are Reviewed-by: Matt Roper <matthew.d.roper@intel.com> I left a couple comments on #5. Matt > static bool > intel_alloc_plane_obj(struct intel_crtc *crtc, > struct intel_initial_plane_config *plane_config) > @@ -2411,20 +2425,6 @@ out_unref_obj: > return false; > } > > -/* Update plane->state->fb to match plane->fb after driver-internal updates */ > -static void > -update_state_fb(struct drm_plane *plane) > -{ > - if (plane->fb == plane->state->fb) > - return; > - > - if (plane->state->fb) > - drm_framebuffer_unreference(plane->state->fb); > - plane->state->fb = plane->fb; > - if (plane->state->fb) > - drm_framebuffer_reference(plane->state->fb); > -} > - > static void > intel_find_plane_obj(struct intel_crtc *intel_crtc, > struct intel_initial_plane_config *plane_config) > @@ -2438,8 +2438,10 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc, > if (!intel_crtc->base.primary->fb) > return; > > - if (intel_alloc_plane_obj(intel_crtc, plane_config)) > + if (intel_alloc_plane_obj(intel_crtc, plane_config)) { > + update_state_fb(intel_crtc->base.primary); > return; > + } > > kfree(intel_crtc->base.primary->fb); > intel_crtc->base.primary->fb = NULL; > @@ -6652,7 +6654,6 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, > plane_config->size); > > crtc->base.primary->fb = fb; > - update_state_fb(crtc->base.primary); > } > > static void chv_crtc_clock_get(struct intel_crtc *crtc, > @@ -7690,7 +7691,6 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, > plane_config->size); > > crtc->base.primary->fb = fb; > - update_state_fb(crtc->base.primary); > return; > > error: > @@ -7782,7 +7782,6 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, > plane_config->size); > > crtc->base.primary->fb = fb; > - update_state_fb(crtc->base.primary); > } > > static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > -- > 1.8.3.1 >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3301b61..e1e89d0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2364,6 +2364,20 @@ static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha) } } +/* Update plane->state->fb to match plane->fb after driver-internal updates */ +static void +update_state_fb(struct drm_plane *plane) +{ + if (plane->fb == plane->state->fb) + return; + + if (plane->state->fb) + drm_framebuffer_unreference(plane->state->fb); + plane->state->fb = plane->fb; + if (plane->state->fb) + drm_framebuffer_reference(plane->state->fb); +} + static bool intel_alloc_plane_obj(struct intel_crtc *crtc, struct intel_initial_plane_config *plane_config) @@ -2411,20 +2425,6 @@ out_unref_obj: return false; } -/* Update plane->state->fb to match plane->fb after driver-internal updates */ -static void -update_state_fb(struct drm_plane *plane) -{ - if (plane->fb == plane->state->fb) - return; - - if (plane->state->fb) - drm_framebuffer_unreference(plane->state->fb); - plane->state->fb = plane->fb; - if (plane->state->fb) - drm_framebuffer_reference(plane->state->fb); -} - static void intel_find_plane_obj(struct intel_crtc *intel_crtc, struct intel_initial_plane_config *plane_config) @@ -2438,8 +2438,10 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc, if (!intel_crtc->base.primary->fb) return; - if (intel_alloc_plane_obj(intel_crtc, plane_config)) + if (intel_alloc_plane_obj(intel_crtc, plane_config)) { + update_state_fb(intel_crtc->base.primary); return; + } kfree(intel_crtc->base.primary->fb); intel_crtc->base.primary->fb = NULL; @@ -6652,7 +6654,6 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, plane_config->size); crtc->base.primary->fb = fb; - update_state_fb(crtc->base.primary); } static void chv_crtc_clock_get(struct intel_crtc *crtc, @@ -7690,7 +7691,6 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, plane_config->size); crtc->base.primary->fb = fb; - update_state_fb(crtc->base.primary); return; error: @@ -7782,7 +7782,6 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, plane_config->size); crtc->base.primary->fb = fb; - update_state_fb(crtc->base.primary); } static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
Tvrtko noticed a new warning on boot: WARNING: CPU: 1 PID: 353 at include/linux/kref.h:47 drm_framebuffer_reference+0x6c/0x80 [drm]() Call Trace: [<ffffffff8161f10c>] dump_stack+0x4f/0x7b [<ffffffff81052caa>] warn_slowpath_common+0xaa/0xd0 [<ffffffff81052d8a>] warn_slowpath_null+0x1a/0x20 [<ffffffffa00d035c>] drm_framebuffer_reference+0x6c/0x80 [drm] [<ffffffffa01c0df7>] update_state_fb.isra.54+0x47/0x50 [i915] [<ffffffffa01ccd5c>] skylake_get_initial_plane_config+0x93c/0x950 [i915] [<ffffffffa01e8721>] intel_modeset_init+0x1551/0x17c0 [i915] [<ffffffffa02476e0>] i915_driver_load+0xed0/0x11e0 [i915] [<ffffffff81627aa1>] ? _raw_spin_unlock_irqrestore+0x51/0x70 [<ffffffffa00ca8b7>] drm_dev_register+0x77/0x110 [drm] [<ffffffffa00cda3b>] drm_get_pci_dev+0x11b/0x1f0 [drm] [<ffffffff81098e3d>] ? trace_hardirqs_on+0xd/0x10 [<ffffffff81627aa1>] ? _raw_spin_unlock_irqrestore+0x51/0x70 [<ffffffffa0145276>] i915_pci_probe+0x56/0x60 [i915] [<ffffffff813ad59c>] pci_device_probe+0x7c/0x100 [<ffffffff81466aad>] driver_probe_device+0x16d/0x380 We cannot take a reference at this point, not before intel_framebuffer_init() and the underlying drm_framebuffer_init(). Introduced in: commit 706dc7b549175e47f23e913b7f1e52874a7d0f56 Author: Matt Roper <matthew.d.roper@intel.com> Date: Tue Feb 3 13:10:04 2015 -0800 drm/i915: Ensure plane->state->fb stays in sync with plane->fb Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-)