Re: [PATCH] drm/i915: Reset dpll_hw_state when selecting a new pll on hsw

From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Reset dpll_hw_state when selecting a new pll on hsw
Date: Tue, 13 Oct 2015 15:58:30 +0200	[thread overview]
Message-ID: <20151013135830.GF26718@phenom.ffwll.local> (raw)
In-Reply-To: <561D0A80.7050503@linux.intel.com>

On Tue, Oct 13, 2015 at 03:43:28PM +0200, Maarten Lankhorst wrote:
> Op 13-10-15 om 15:35 schreef Daniel Vetter:
> > On Tue, Oct 13, 2015 at 03:18:16PM +0200, Maarten Lankhorst wrote:
> >> Op 23-09-15 om 17:34 schreef Gabriel Feceoru:
> >>> Using 2 connectors (DVI and VGA) will cause wrpll to be set for
> >>> INTEL_OUTPUT_HDMI but never reset if switching to INTEL_OUTPUT_VGA
> >>>
> >>> Supresses errors like these:
> >>> [drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in dpll_hw_state.wrpll
> >>>
> >> Looks like a good idea to always zero it.
> > Except that we still have a bunch of cases where we recompute clock state
> > but only partially. Can we just move them all up into a common place
> > please? That would also catch cases where we simply forget to fill this
> > out at all.
> >
> > One case I noticed is edp in skl_ddi_pll_select, but there's probably
> > more.
> >
> Something like below, with all the memsets for dpll_hw_state removed?

I think this will blow up since we recompute clock state only when
needs_modeset is true. So needs a bit more intelligence in deciding when
to clear it I think.
-Daniel
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 633da693fed8..956b7ffab32f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11911,7 +11911,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_crtc_state tmp_state;
>  	struct intel_crtc_scaler_state scaler_state;
> -	struct intel_dpll_hw_state dpll_hw_state;
>  	enum intel_dpll_id shared_dpll;
>  	uint32_t ddi_pll_sel;
>  	bool force_thru;
> @@ -11924,7 +11923,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
>  	tmp_state = crtc_state->base;
>  	scaler_state = crtc_state->scaler_state;
>  	shared_dpll = crtc_state->shared_dpll;
> -	dpll_hw_state = crtc_state->dpll_hw_state;
>  	ddi_pll_sel = crtc_state->ddi_pll_sel;
>  	force_thru = crtc_state->pch_pfit.force_thru;
>  
> 

-- 
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