From 319c1d420a0b62d9dbb88104afebaabc968cdbfa Mon Sep 17 00:00:00 2001 From: Xi Ruoyao Date: Thu, 12 Mar 2015 20:16:32 +0800 Subject: drm/i915: Ensure plane->state->fb stays in sync with plane->fb plane->state->fb and plane->fb should always reference the same FB so that atomic and legacy codepaths have the same view of display state. However, there are some places in kernel code that directly set plane->fb and neglect to update plane->state->fb. If we never do a successful update through the atomic pipeline, the RmFB cleanup code will look at the plane->state->fb pointer, which has never actually been set to a legitimate value, and try to clean it up, leading to BUG's. Add a quick helper function to synchronize plane->state->fb with plane->fb and call it everywhere the driver tries to manually set plane->fb outside of the atomic pipeline. In this function, use drm_atomic_set_fb_for_plane instead of writing plane->state->fb directly to keep the reference count right. This is modified from Matt Roper's patch to drm-intel-nightly with commit id commit afd65eb4cc0578a9c07d621acdb8a570e2782bf7 Author: Matt Roper Date: Tue Feb 3 13:10:04 2015 -0800 drm/i915: Ensure plane->state->fb stays in sync with plane->fb However this bug exists in mainline kernel too, so I created this to fix it in mainline kernel. A minor change is to use drm_atomic_set_fb_for_plane instead of update reference count manually. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88909 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=93711 Signed-off-by: Xi Ruoyao Reviewed-by: Matt Roper [Jani: included the patch notes in the commit message] Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9943c20a741d..2de362be885a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -37,6 +37,7 @@ #include #include "i915_drv.h" #include "i915_trace.h" +#include #include #include #include @@ -2416,6 +2417,14 @@ 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) + drm_atomic_set_fb_for_plane(plane->state, plane->fb); +} + static void intel_find_plane_obj(struct intel_crtc *intel_crtc, struct intel_initial_plane_config *plane_config) @@ -2462,6 +2471,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc, break; } } + + update_state_fb(intel_crtc->base.primary); } static void i9xx_update_primary_plane(struct drm_crtc *crtc, @@ -6650,6 +6661,7 @@ 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, @@ -7687,6 +7699,7 @@ 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: @@ -7778,6 +7791,7 @@ 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, @@ -9816,6 +9830,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, drm_gem_object_reference(&obj->base); crtc->primary->fb = fb; + update_state_fb(crtc->primary); work->pending_flip_obj = obj; @@ -9884,6 +9899,7 @@ cleanup_unpin: cleanup_pending: atomic_dec(&intel_crtc->unpin_work_count); crtc->primary->fb = old_fb; + update_state_fb(crtc->primary); drm_gem_object_unreference(&work->old_fb_obj->base); drm_gem_object_unreference(&obj->base); mutex_unlock(&dev->struct_mutex); @@ -13718,6 +13734,7 @@ void intel_modeset_gem_init(struct drm_device *dev) to_intel_crtc(c)->pipe); drm_framebuffer_unreference(c->primary->fb); c->primary->fb = NULL; + update_state_fb(c->primary); } } mutex_unlock(&dev->struct_mutex); -- cgit v1.2.3 From 7f0801e566cc78315e5dc383bf3c3b5b5b436048 Mon Sep 17 00:00:00 2001 From: Damien Lespiau Date: Thu, 5 Feb 2015 19:35:13 +0000 Subject: drm/i915: Make sure the primary plane is enabled before reading out the fb state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't want to end up in a state where we track that the pipe has its primary plane enabled when primary plane registers are programmed with values that look possible but the plane actually disabled. Refuse to read out the fb state when the primary plane isn't enabled. Suggested-by: Ville Syrjälä Suggested-by: Matt Roper Signed-off-by: Damien Lespiau Reviewed-by: Matt Roper Signed-off-by: Daniel Vetter Reported-by: Andrey Skvortsov Reported-by: Steven Rostedt Reference: http://mid.gmane.org/20150203191507.GA2374@crion86 Tested-by: Steven Rostedt Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2de362be885a..6d22128d97b1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6613,6 +6613,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, struct drm_framebuffer *fb; struct intel_framebuffer *intel_fb; + val = I915_READ(DSPCNTR(plane)); + if (!(val & DISPLAY_PLANE_ENABLE)) + return; + intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); if (!intel_fb) { DRM_DEBUG_KMS("failed to alloc fb\n"); @@ -6621,8 +6625,6 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, fb = &intel_fb->base; - val = I915_READ(DSPCNTR(plane)); - if (INTEL_INFO(dev)->gen >= 4) if (val & DISPPLANE_TILED) plane_config->tiling = I915_TILING_X; @@ -7655,6 +7657,9 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, fb = &intel_fb->base; val = I915_READ(PLANE_CTL(pipe, 0)); + if (!(val & PLANE_CTL_ENABLE)) + goto error; + if (val & PLANE_CTL_TILED_MASK) plane_config->tiling = I915_TILING_X; @@ -7743,6 +7748,10 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, struct drm_framebuffer *fb; struct intel_framebuffer *intel_fb; + val = I915_READ(DSPCNTR(pipe)); + if (!(val & DISPLAY_PLANE_ENABLE)) + return; + intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); if (!intel_fb) { DRM_DEBUG_KMS("failed to alloc fb\n"); @@ -7751,8 +7760,6 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, fb = &intel_fb->base; - val = I915_READ(DSPCNTR(pipe)); - if (INTEL_INFO(dev)->gen >= 4) if (val & DISPPLANE_TILED) plane_config->tiling = I915_TILING_X; -- cgit v1.2.3