Re: [PATCH 06/13] drm/i915: add pipe_config->has_pch_encoder

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 27 Mar 2013 00:44:55 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> This is used way too often in the enable/disable paths. And will
> be even more useful in the future.
> 
> Note that correct semantics of this change highly depend upon
> correct updating of intel_crtc->config: Like with all other
> modeset state, we need to call ->disable with the old config,
> but ->mode_set and ->enable with the new config.
> 
> v2: Do not yet use the flag in the ->disable callbacks - atm we don't
> yet have support for the information stored in the pipe_config in the
> hw state readout code, so this will be wrong at boot-up/resume.
> 
> v3: Rebased on top of the hdmi/dp ddi encoder merging.
> 
> v4: Fixup stupid rebase error which lead to a NULL vfunc deref.
> 
> v5: On haswell the VGA port is on the PCH!
> 
> v6: s/IS_HASWELL/HAS_DDI/, spotted by Paulo Zanoni. Also add a missing
> parameter name in a function declaration.
> 
> v7: Don't forget to git add ...

Looks like you got all the outputs covered.  But we always seem to get
this bit wrong, so we'll need to test on all the configs we know about
at least...

+	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && !is_cpu_edp(intel_dp))
+		pipe_config->has_pch_encoder = true;
+

This could just do
 if (intel_dp->is_pch_edp)
	pipe_config->has_pch_encoder = true;
right?  Since we cover the other cases in dp_init_connector?

But either way:

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center