Re: [PATCH v2 7/9] drm/i915: Add eDP support for Valleyview

From: Daniel Vetter <daniel@ffwll.ch>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Intel Graphics <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2 7/9] drm/i915: Add eDP support for Valleyview
Date: Fri, 28 Sep 2012 17:08:27 +0200	[thread overview]
Message-ID: <20120928150827.GN2098@bremse> (raw)
In-Reply-To: <20120927082446.464a8fc6@jbarnes-desktop>

On Thu, Sep 27, 2012 at 08:24:46AM -0700, Jesse Barnes wrote:
> On Thu, 27 Sep 2012 19:13:07 +0530
> Vijay Purushothaman <vijay.a.purushothaman@intel.com> wrote:
> 
> > From: Gajanan Bhat <gajanan.bhat@intel.com>
> > 
> > Eventhough Valleyview display block is derived from Cantiga, VLV
> > supports eDP. So, added eDP checks in i9xx_crtc_mode_set path.
> > 
> > v2: use different DPIO_DIVISOR values for VGA, DP and eDP
> > v3: fix DPIO value calculation to use same values for all display
> > interfaces
> > v4: removed unconditional enabling of 6bpc dithering based on comments
> > from Daniel & Jani Nikula. Also changed the display enabling order to
> > force eDP detection first.
> > 
> > Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> > Signed-off-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com>

Ok, I've slurped in the patches with the exception of the vga detect quirk
(maybe that'll go away with the hpd rework anyway) and the panel stuff
(since no one volunteered yet to smash an r-b onto it).

For the edp stuff here I think we need to add a bit more abstraction
eventually - maybe add an enum of the different kinds of DP connectors
(cpu, fdi, gmch) or add some vtable interfaces for the things which are
different on different platforms/ports ...

Thanks, Daniel

> > ---
> >  drivers/gpu/drm/i915/intel_display.c |   15 ++++++++++++---
> >  drivers/gpu/drm/i915/intel_dp.c      |   17 ++++++++++++-----
> >  2 files changed, 24 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ed749c4..0362c80 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4413,6 +4413,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  		}
> >  	}
> >  
> > +	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
> > +		if (adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC) {
> > +			pipeconf |= PIPECONF_BPP_6 |
> > +					PIPECONF_ENABLE |
> > +					I965_PIPECONF_ACTIVE;
> > +		}
> > +	}
> > +
> >  	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
> >  	drm_mode_debug_printmodeline(mode);
> >  
> > @@ -7623,6 +7631,10 @@ static void intel_setup_outputs(struct drm_device *dev)
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >  		int found;
> >  
> > +		/* Check for built-in panel first. Shares lanes with HDMI on SDVOC */
> > +		if (I915_READ(DP_C) & DP_DETECTED)
> > +			intel_dp_init(dev, DP_C, PORT_C);
> > +
> >  		if (I915_READ(SDVOB) & PORT_DETECTED) {
> >  			/* SDVOB multiplex with HDMIB */
> >  			found = intel_sdvo_init(dev, SDVOB, true);
> > @@ -7635,9 +7647,6 @@ static void intel_setup_outputs(struct drm_device *dev)
> >  		if (I915_READ(SDVOC) & PORT_DETECTED)
> >  			intel_hdmi_init(dev, SDVOC, PORT_C);
> >  
> > -		/* Shares lanes with HDMI on SDVOC */
> > -		if (I915_READ(DP_C) & DP_DETECTED)
> > -			intel_dp_init(dev, DP_C, PORT_C);
> >  	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
> >  		bool found = false;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index c111c3f..867c568 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -885,7 +885,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
> >  
> >  	/* Split out the IBX/CPU vs CPT settings */
> >  
> > -	if (is_cpu_edp(intel_dp) && IS_GEN7(dev)) {
> > +	if (is_cpu_edp(intel_dp) && IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) {
> >  		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
> >  			intel_dp->DP |= DP_SYNC_HS_HIGH;
> >  		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> > @@ -1474,7 +1474,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
> >  {
> >  	struct drm_device *dev = intel_dp->base.base.dev;
> >  
> > -	if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
> > +	if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
> >  		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> >  		case DP_TRAIN_VOLTAGE_SWING_400:
> >  			return DP_TRAIN_PRE_EMPHASIS_6;
> > @@ -1773,7 +1773,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> >  		uint32_t    signal_levels;
> >  
> >  
> > -		if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
> > +		if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
> >  			signal_levels = intel_gen7_edp_signal_levels(intel_dp->train_set[0]);
> >  			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_IVB) | signal_levels;
> >  		} else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
> > @@ -1859,7 +1859,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
> >  			break;
> >  		}
> >  
> > -		if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
> > +		if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
> >  			signal_levels = intel_gen7_edp_signal_levels(intel_dp->train_set[0]);
> >  			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_IVB) | signal_levels;
> >  		} else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
> > @@ -2471,7 +2471,14 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> >  		if (intel_dpd_is_edp(dev))
> >  			intel_dp->is_pch_edp = true;
> >  
> > -	if (output_reg == DP_A || is_pch_edp(intel_dp)) {
> > +	/*
> > +	 * FIXME : We need to initialize built-in panels before external panels.
> > +	 * For X0, DP_C is fixed as eDP. Revisit this as part of VLV eDP cleanup
> > +	 */
> > +	if (IS_VALLEYVIEW(dev) && output_reg == DP_C) {
> > +		type = DRM_MODE_CONNECTOR_eDP;
> > +		intel_encoder->type = INTEL_OUTPUT_EDP;
> > +	} else if (output_reg == DP_A || is_pch_edp(intel_dp)) {
> >  		type = DRM_MODE_CONNECTOR_eDP;
> >  		intel_encoder->type = INTEL_OUTPUT_EDP;
> >  	} else {
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch