Various places in the atomic plane code obtain the CRTC via plane_state->crtc. But plane_state->crtc is NULL when disabling the plane, so the code will fall back to looking at the old CRTC value in plane->crtc in that case. This isn't going to work when we start supporting non-blocking flips (since the legacy plane->crtc pointer will already be updated to its new value before the asynchronous workqueue task runs the plane commit function). The code is also fragile in general (we have to be careful when doing stuff like updating properties on a disabled plane). Since Intel hardware has a fixed plane to pipe mapping, let's just lookup the CRTC via that fixed mapping and avoid future mistakes. Cc: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx> Reported-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> --- drivers/gpu/drm/i915/intel_atomic_plane.c | 12 ++++++------ drivers/gpu/drm/i915/intel_display.c | 8 ++++---- drivers/gpu/drm/i915/intel_drv.h | 7 +++++++ drivers/gpu/drm/i915/intel_sprite.c | 4 ++-- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 714ee24..21c808d 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -115,16 +115,16 @@ static int intel_plane_atomic_check(struct drm_plane *plane, struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_plane_state *intel_state = to_intel_plane_state(state); - crtc = crtc ? crtc : plane->crtc; + crtc = intel_get_crtc_for_drm_plane(plane); intel_crtc = to_intel_crtc(crtc); /* - * Both crtc and plane->crtc could be NULL if we're updating a - * property while the plane is disabled. We don't actually have - * anything driver-specific we need to test in that case, so - * just return success. + * Both state->crtc and plane->state->crtc could be NULL if we're + * updating a property while the plane is disabled. We don't actually + * have anything driver-specific we need to test in that case, so just + * return success. */ - if (!crtc) + if (!state->crtc && !plane->state->crtc) return 0; crtc_state = intel_crtc_state_for_plane(intel_state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1a7d2a9..f4398a7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13092,7 +13092,7 @@ intel_check_primary_plane(struct drm_plane *plane, int min_scale = DRM_PLANE_HELPER_NO_SCALING; int ret; - crtc = crtc ? crtc : plane->crtc; + crtc = intel_get_crtc_for_drm_plane(plane); intel_crtc = to_intel_crtc(crtc); crtc_state = intel_crtc_state_for_plane(state); @@ -13174,7 +13174,7 @@ intel_commit_primary_plane(struct drm_plane *plane, struct intel_crtc *intel_crtc; struct drm_rect *src = &state->src; - crtc = crtc ? crtc : plane->crtc; + crtc = intel_get_crtc_for_drm_plane(plane); intel_crtc = to_intel_crtc(crtc); plane->fb = fb; @@ -13409,7 +13409,7 @@ intel_check_cursor_plane(struct drm_plane *plane, unsigned stride; int ret; - crtc = crtc ? crtc : plane->crtc; + crtc = intel_get_crtc_for_drm_plane(plane); intel_crtc = to_intel_crtc(crtc); ret = drm_plane_helper_check_update(plane, crtc, fb, @@ -13482,7 +13482,7 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); uint32_t addr; - crtc = crtc ? crtc : plane->crtc; + crtc = intel_get_crtc_for_drm_plane(plane); intel_crtc = to_intel_crtc(crtc); plane->fb = state->base.fb; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1e22ffe..ea67093 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -803,6 +803,13 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane) return dev_priv->plane_to_crtc_mapping[plane]; } +static inline struct drm_crtc * +intel_get_crtc_for_drm_plane(struct drm_plane *plane) +{ + struct drm_i915_private *dev_priv = to_i915(plane->dev); + return dev_priv->pipe_to_crtc_mapping[to_intel_plane(plane)->pipe]; +} + struct intel_unpin_work { struct work_struct work; struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index bfe9213..2164e63 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -752,7 +752,7 @@ intel_check_sprite_plane(struct drm_plane *plane, int pixel_size; int ret; - intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc); + intel_crtc = to_intel_crtc(intel_get_crtc_for_drm_plane(plane)); crtc_state = intel_crtc_state_for_plane(state); if (!fb) { @@ -942,7 +942,7 @@ intel_commit_sprite_plane(struct drm_plane *plane, unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; - crtc = crtc ? crtc : plane->crtc; + crtc = intel_get_crtc_for_drm_plane(plane); intel_crtc = to_intel_crtc(crtc); plane->fb = fb; -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx