On Tue, Mar 03, 2015 at 08:21:44AM -0300, Paulo Zanoni wrote: > Hi > > 2015-02-27 15:12 GMT-03:00 Matt Roper <[email protected]>: > > plane->fb is a legacy pointer that not always be up-to-date (or updated > > early enough). Make sure the watermark code uses plane->state->fb so > > that we're always doing our calculations based on the correct > > framebuffers. > > QA reported a regression caused by this patch: Kernel NULL pointer > dereference. > > https://bugs.freedesktop.org/show_bug.cgi?id=89388 >
Yeah, I just saw this. I'll have to look more closely later today, but I'm wondering whether I should have just done this replacement driver-wide instead of restricted to just the watermark code. I suspect killing off all of our uses of the old, legacy fields and using the new atomic state values instead will make the driver more internally consistent so we quit running into issues like this. Matt > > > > > This patch was generated by Coccinelle with the following semantic > > patch: > > > > @@ > > struct drm_plane *P; > > @@ > > - P->fb > > + P->state->fb > > > > v2: Rebase > > > > Signed-off-by: Matt Roper <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_wm.c | 22 +++++++++++----------- > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_wm.c > > b/drivers/gpu/drm/i915/intel_wm.c > > index 47a5175..e877e02 100644 > > --- a/drivers/gpu/drm/i915/intel_wm.c > > +++ b/drivers/gpu/drm/i915/intel_wm.c > > @@ -496,7 +496,7 @@ static void pineview_update_wm(struct drm_crtc > > *unused_crtc) > > crtc = single_enabled_crtc(dev); > > if (crtc) { > > const struct drm_display_mode *adjusted_mode; > > - int pixel_size = crtc->primary->fb->bits_per_pixel / 8; > > + int pixel_size = crtc->primary->state->fb->bits_per_pixel / > > 8; > > int clock; > > > > adjusted_mode = > > &to_intel_crtc(crtc)->config->base.adjusted_mode; > > @@ -572,7 +572,7 @@ static bool g4x_compute_wm0(struct drm_device *dev, > > clock = adjusted_mode->crtc_clock; > > htotal = adjusted_mode->crtc_htotal; > > hdisplay = to_intel_crtc(crtc)->config->pipe_src_w; > > - pixel_size = crtc->primary->fb->bits_per_pixel / 8; > > + pixel_size = crtc->primary->state->fb->bits_per_pixel / 8; > > > > /* Use the small buffer method to calculate plane watermark */ > > entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000; > > @@ -659,7 +659,7 @@ static bool g4x_compute_srwm(struct drm_device *dev, > > clock = adjusted_mode->crtc_clock; > > htotal = adjusted_mode->crtc_htotal; > > hdisplay = to_intel_crtc(crtc)->config->pipe_src_w; > > - pixel_size = crtc->primary->fb->bits_per_pixel / 8; > > + pixel_size = crtc->primary->state->fb->bits_per_pixel / 8; > > > > line_time_us = max(htotal * 1000 / clock, 1); > > line_count = (latency_ns / line_time_us + 1000) / 1000; > > @@ -742,7 +742,7 @@ static void vlv_update_drain_latency(struct drm_crtc > > *crtc) > > } > > > > /* Primary plane Drain Latency */ > > - pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */ > > + pixel_size = crtc->primary->state->fb->bits_per_pixel / 8; /* > > BPP */ > > if (vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, > > &drain_latency)) { > > plane_prec = (prec_mult == high_precision) ? > > DDL_PLANE_PRECISION_HIGH : > > @@ -1023,7 +1023,7 @@ static void i965_update_wm(struct drm_crtc > > *unused_crtc) > > int clock = adjusted_mode->crtc_clock; > > int htotal = adjusted_mode->crtc_htotal; > > int hdisplay = to_intel_crtc(crtc)->config->pipe_src_w; > > - int pixel_size = crtc->primary->fb->bits_per_pixel / 8; > > + int pixel_size = crtc->primary->state->fb->bits_per_pixel / > > 8; > > unsigned long line_time_us; > > int entries; > > > > @@ -1100,7 +1100,7 @@ static void i9xx_update_wm(struct drm_crtc > > *unused_crtc) > > crtc = intel_get_crtc_for_plane(dev, 0); > > if (intel_crtc_active(crtc)) { > > const struct drm_display_mode *adjusted_mode; > > - int cpp = crtc->primary->fb->bits_per_pixel / 8; > > + int cpp = crtc->primary->state->fb->bits_per_pixel / 8; > > if (IS_GEN2(dev)) > > cpp = 4; > > > > @@ -1122,7 +1122,7 @@ static void i9xx_update_wm(struct drm_crtc > > *unused_crtc) > > crtc = intel_get_crtc_for_plane(dev, 1); > > if (intel_crtc_active(crtc)) { > > const struct drm_display_mode *adjusted_mode; > > - int cpp = crtc->primary->fb->bits_per_pixel / 8; > > + int cpp = crtc->primary->state->fb->bits_per_pixel / 8; > > if (IS_GEN2(dev)) > > cpp = 4; > > > > @@ -1145,7 +1145,7 @@ static void i9xx_update_wm(struct drm_crtc > > *unused_crtc) > > if (IS_I915GM(dev) && enabled) { > > struct drm_i915_gem_object *obj; > > > > - obj = intel_fb_obj(enabled->primary->fb); > > + obj = intel_fb_obj(enabled->primary->state->fb); > > > > /* self-refresh seems busted with untiled */ > > if (obj->tiling_mode == I915_TILING_NONE) > > @@ -1169,7 +1169,7 @@ static void i9xx_update_wm(struct drm_crtc > > *unused_crtc) > > int clock = adjusted_mode->crtc_clock; > > int htotal = adjusted_mode->crtc_htotal; > > int hdisplay = to_intel_crtc(enabled)->config->pipe_src_w; > > - int pixel_size = enabled->primary->fb->bits_per_pixel / 8; > > + int pixel_size = > > enabled->primary->state->fb->bits_per_pixel / 8; > > unsigned long line_time_us; > > int entries; > > > > @@ -1874,7 +1874,7 @@ 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->fb->bits_per_pixel / 8; > > + p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / > > 8; > > p->cur.bytes_per_pixel = 4; > > p->pri.horiz_pixels = intel_crtc->config->pipe_src_w; > > p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w; > > @@ -2659,7 +2659,7 @@ static void skl_compute_wm_pipe_parameters(struct > > drm_crtc *crtc, > > */ > > p->plane[0].enabled = true; > > p->plane[0].bytes_per_pixel = > > - crtc->primary->fb->bits_per_pixel / 8; > > + crtc->primary->state->fb->bits_per_pixel / 8; > > p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w; > > p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h; > > p->plane[0].tiling = DRM_FORMAT_MOD_NONE; > > -- > > 1.8.5.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
