Re: [PATCH 4.20 regression fix] drm/i915: Revert "Fix assert_plane() warning on bootup with external display"

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx