Re: [PATCH] drm/i915: Stop calling intel_prepare_ddi during power well initialization.

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Sep 22, 2015 at 08:03:59AM -0700, Rodrigo Vivi wrote:
> The comment removed along with the calls explains why they shouldn't be here:
> 
> /* DDI buffer programming unnecessary during driver-load/resume
>  * as it's already done during modeset initialization then.
>  * It's also invalid here as encoder list is still uninitialized.
>  */
> 
> And the protection is not working well causing issues during the boot where
> power well initialization doesn't go as expected.
> 
> So this call during modeset initialization where the encoder list is yet
> unitilized causes a NULL dereference breaking i915.
> 
> While we don't find the right protection and we don't understand why this is
> actually needed I believe it is safe to just remove these calls.
> 
> Cc: Suketu Shah <suketu.j.shah@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

But if we shut down the power well the hw will forget these values. Where
are they programmed now? Which commit broke this, how does this blow up?
Please add more details, atm I have no idea what to do with this patch but
be confused about it ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 85c35fd..f7027ea 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -246,7 +246,6 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
>  	}
>  
>  	if (power_well->data == SKL_DISP_PW_1) {
> -		intel_prepare_ddi(dev);
>  		gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
>  	}
>  }
> @@ -632,13 +631,6 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  				power_well->data == SKL_DISP_PW_2) {
>  				if (SKL_ENABLE_DC6(dev)) {
>  					skl_disable_dc6(dev_priv);
> -					/*
> -					 * DDI buffer programming unnecessary during driver-load/resume
> -					 * as it's already done during modeset initialization then.
> -					 * It's also invalid here as encoder list is still uninitialized.
> -					 */
> -					if (!dev_priv->power_domains.initializing)
> -						intel_prepare_ddi(dev);
>  				} else {
>  					gen9_disable_dc5(dev_priv);
>  				}
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx