On Wed, 04 Mar 2015, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote: > Current ILK-style watermark code assumes the primary plane and cursor > plane are always enabled. This assumption, along with the combination > of two independent commits that got merged at the same time, results in > a NULL dereference. The offending commits are: > > commit fd2d61341bf39d1054256c07d6eddd624ebc4241 > Author: Matt Roper <matthew.d.roper@xxxxxxxxx> > Date: Fri Feb 27 10:12:01 2015 -0800 > > drm/i915: Use plane->state->fb in watermark code (v2) > > and > > commit 0fda65680e92545caea5be7805a7f0a617fb6c20 > Author: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Date: Fri Feb 27 15:12:35 2015 +0000 > > drm/i915/skl: Update watermarks for Y tiling > > The first commit causes us to use the FB from plane->state->fb rather > than the legacy plane->fb, which is updated a bit later in the process. > > The second commit includes a change that now triggers watermark > reprogramming on primary plane enable/disable where we didn't have one > before (which wasn't really correct, but we had been getting lucky > because we always calculated as if the primary plane was on). > > Together, these two commits cause the watermark calculation to > (properly) see plane->state->fb = NULL when we're in the process of > disabling the primary plane. However the existing watermark code > assumes there's always a primary fb and tries to dereference it to find > out pixel format / bpp information. > > The fix is to make ILK-style watermark calculation actually check the > true status of primary & cursor planes and adjust our watermark logic > accordingly. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Cc: Michael Leuchtenburg <michael@xxxxxxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89388 > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> Tested-by: lu hua <huax.lu@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_pm.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index e710b43..93fd15f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1924,13 +1924,25 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, > p->active = true; > p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal; > p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc); > - p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8; > - p->cur.bytes_per_pixel = 4; > + > + if (crtc->primary->state->fb) { > + p->pri.enabled = true; > + p->pri.bytes_per_pixel = > + crtc->primary->state->fb->bits_per_pixel / 8; > + } else { > + p->pri.enabled = false; > + p->pri.bytes_per_pixel = 0; > + } > + > + if (crtc->cursor->state->fb) { > + p->cur.enabled = true; > + p->cur.bytes_per_pixel = 4; > + } else { > + p->cur.enabled = false; > + p->cur.bytes_per_pixel = 0; > + } > p->pri.horiz_pixels = intel_crtc->config->pipe_src_w; > p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w; > - /* TODO: for now, assume primary and cursor planes are always enabled. */ > - p->pri.enabled = true; > - p->cur.enabled = true; > > drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) { > struct intel_plane *intel_plane = to_intel_plane(plane); > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
