| Message ID | 20181119223500.29779-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [4.20,regression,fix] drm/i915: Revert "Fix assert_plane() warning on bootup with external display" | expand |
On Mon, Nov 19, 2018 at 11:35:00PM +0100, Hans de Goede wrote: > Starting with 4.20-rc1 I'm seeing the LCD screen briefly turn mostly purple > on devices with a DSI panel (seen on 2 different devices with a DSI panel). > > This happens both with and without fastboot=1. This is caused by > commit 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with > external display"). > > And a user commenting on fdo bug 108225 has reported a flicker caused by > the screen briefly turning blank when booting with fastboot=1, which goes > away when reverting this commit. > > I believe a revert of the offending commit is the best solution because > the commit introduces a drm_atomic_commit() call in the drivers' probe > path, causing writes to the hardware during probe. > I believe this goes agains the design of the driver which is to only > read-back state on probe and only write to the hardware on the first > commit from the fbcon or userspace. > > Instead the assert_plane() problem the commit tried to fix should be fixed > by fixing the state read-back code. > > Related: https://bugs.freedesktop.org/show_bug.cgi?id=108225 > Cc: Azhar Shaikh <azhar.shaikh@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Fixed with https://patchwork.freedesktop.org/series/52754/ > --- > drivers/gpu/drm/i915/intel_display.c | 61 +--------------------------- > 1 file changed, 2 insertions(+), 59 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2107de6da692..03dec1c05652 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15193,61 +15193,12 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv) > DRM_DEBUG_DRIVER("FDI PLL freq=%d\n", dev_priv->fdi_pll_freq); > } > > -static int intel_initial_commit(struct drm_device *dev) > -{ > - struct drm_atomic_state *state = NULL; > - struct drm_modeset_acquire_ctx ctx; > - struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > - int ret = 0; > - > - state = drm_atomic_state_alloc(dev); > - if (!state) > - return -ENOMEM; > - > - drm_modeset_acquire_init(&ctx, 0); > - > -retry: > - state->acquire_ctx = &ctx; > - > - drm_for_each_crtc(crtc, dev) { > - crtc_state = drm_atomic_get_crtc_state(state, crtc); > - if (IS_ERR(crtc_state)) { > - ret = PTR_ERR(crtc_state); > - goto out; > - } > - > - if (crtc_state->active) { > - ret = drm_atomic_add_affected_planes(state, crtc); > - if (ret) > - goto out; > - } > - } > - > - ret = drm_atomic_commit(state); > - > -out: > - if (ret == -EDEADLK) { > - drm_atomic_state_clear(state); > - drm_modeset_backoff(&ctx); > - goto retry; > - } > - > - drm_atomic_state_put(state); > - > - drm_modeset_drop_locks(&ctx); > - drm_modeset_acquire_fini(&ctx); > - > - return ret; > -} > - > int intel_modeset_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > struct i915_ggtt *ggtt = &dev_priv->ggtt; > enum pipe pipe; > struct intel_crtc *crtc; > - int ret; > > dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0); > > @@ -15319,6 +15270,8 @@ int intel_modeset_init(struct drm_device *dev) > INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : ""); > > for_each_pipe(dev_priv, pipe) { > + int ret; > + > ret = intel_crtc_init(dev_priv, pipe); > if (ret) { > drm_mode_config_cleanup(dev); > @@ -15374,16 +15327,6 @@ int intel_modeset_init(struct drm_device *dev) > if (!HAS_GMCH_DISPLAY(dev_priv)) > sanitize_watermarks(dev); > > - /* > - * Force all active planes to recompute their states. So that on > - * mode_setcrtc after probe, all the intel_plane_state variables > - * are already calculated and there is no assert_plane warnings > - * during bootup. > - */ > - ret = intel_initial_commit(dev); > - if (ret) > - DRM_DEBUG_KMS("Initial commit in probe failed.\n"); > - > return 0; > } > > -- > 2.19.1
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2107de6da692..03dec1c05652 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15193,61 +15193,12 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv) DRM_DEBUG_DRIVER("FDI PLL freq=%d\n", dev_priv->fdi_pll_freq); } -static int intel_initial_commit(struct drm_device *dev) -{ - struct drm_atomic_state *state = NULL; - struct drm_modeset_acquire_ctx ctx; - struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; - int ret = 0; - - state = drm_atomic_state_alloc(dev); - if (!state) - return -ENOMEM; - - drm_modeset_acquire_init(&ctx, 0); - -retry: - state->acquire_ctx = &ctx; - - drm_for_each_crtc(crtc, dev) { - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); - goto out; - } - - if (crtc_state->active) { - ret = drm_atomic_add_affected_planes(state, crtc); - if (ret) - goto out; - } - } - - ret = drm_atomic_commit(state); - -out: - if (ret == -EDEADLK) { - drm_atomic_state_clear(state); - drm_modeset_backoff(&ctx); - goto retry; - } - - drm_atomic_state_put(state); - - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - - return ret; -} - int intel_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); struct i915_ggtt *ggtt = &dev_priv->ggtt; enum pipe pipe; struct intel_crtc *crtc; - int ret; dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0); @@ -15319,6 +15270,8 @@ int intel_modeset_init(struct drm_device *dev) INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : ""); for_each_pipe(dev_priv, pipe) { + int ret; + ret = intel_crtc_init(dev_priv, pipe); if (ret) { drm_mode_config_cleanup(dev); @@ -15374,16 +15327,6 @@ int intel_modeset_init(struct drm_device *dev) if (!HAS_GMCH_DISPLAY(dev_priv)) sanitize_watermarks(dev); - /* - * Force all active planes to recompute their states. So that on - * mode_setcrtc after probe, all the intel_plane_state variables - * are already calculated and there is no assert_plane warnings - * during bootup. - */ - ret = intel_initial_commit(dev); - if (ret) - DRM_DEBUG_KMS("Initial commit in probe failed.\n"); - return 0; }
Starting with 4.20-rc1 I'm seeing the LCD screen briefly turn mostly purple on devices with a DSI panel (seen on 2 different devices with a DSI panel). This happens both with and without fastboot=1. This is caused by commit 516a49cc1946 ("drm/i915: Fix assert_plane() warning on bootup with external display"). And a user commenting on fdo bug 108225 has reported a flicker caused by the screen briefly turning blank when booting with fastboot=1, which goes away when reverting this commit. I believe a revert of the offending commit is the best solution because the commit introduces a drm_atomic_commit() call in the drivers' probe path, causing writes to the hardware during probe. I believe this goes agains the design of the driver which is to only read-back state on probe and only write to the hardware on the first commit from the fbcon or userspace. Instead the assert_plane() problem the commit tried to fix should be fixed by fixing the state read-back code. Related: https://bugs.freedesktop.org/show_bug.cgi?id=108225 Cc: Azhar Shaikh <azhar.shaikh@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/i915/intel_display.c | 61 +--------------------------- 1 file changed, 2 insertions(+), 59 deletions(-)