[Intel-gfx] [PATCH 2/8] Review comments.

[Jesse Barnes]

On Thu, 18 Apr 2013 10:46:09 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:

> Imo extracting vlv_enable_pll should be done now, the other stuff
> is imo ok as follow-up (you could keep the comments as FIXME sections).
> ---
>  drivers/gpu/drm/i915/intel_display.c |   28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 26ef98d..c124dba 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3701,6 +3701,14 @@ static void vlv_pll_pre_enable(struct drm_crtc *crtc)
>  	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
>  
>  	/* Assume eDP on port C and HDMI/DP on port B */
> +
> +	/*
> +	 * Presuming the port macro argument below is indeed the same port the
> +	 * above comment claims, the code does not match up with reality.
> +	 *
> +	 * Second, if you move this into encoder->pre_pll_enable you'll have
> +	 * access to the port number directly and can dtrt.
> +	 */
>  	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI)) {

Ok I see you're talking about the DP (not eDP) case here.  There's
definitely some mixing going on there; DP probably won't work right
now.  Some of the eDP values apply to DP, but on port B.

I can test that and fix it up today incrementally.

I'll drop the cruft that got pulled in and post again.

AAAAAAAAAAAAAAAAAAAAAAAAAARRRRRRRRRRRRRRRRGGGGGGGGGG

Ok I feel a little better and am consoled by the fact that my VLV
display actually works pretty well now.  panel fitting patches are on
their way too.

-- 
Jesse Barnes, Intel Open Source Technology Center