[RFC] drm/i915: check FDI config for 3 pipe

[RFC] drm/i915: check FDI config for 3 pipe

[Jesse Barnes]

Three pipe can only be enabled in some cases and updated docs indicate
a bit to control FDI B+C sharing.

This patch adds a check to make sure we can support a given config with
the existing FDI lane configuration, and tries to set the bit as
appropriate.

We may want to go further though and just totally break the link
between our pipes, planes, PLLs, FDI lanes, transcoders, etc and the
CRTCs we expose to userspace.  We could theoretically just allocate new
ones every mode set.  That wouldn't help a whole lot though since we
don't get full configuration changes from userspace, just a small
window of CRTC data with no idea of what might happen next...

-- 
Jesse Barnes, Intel Open Source Technology Center


diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3cf5770..74c2f8c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3570,6 +3570,7 @@
 #define SOUTH_CHICKEN1		0xc2000
 #define  FDIA_PHASE_SYNC_SHIFT_OVR	19
 #define  FDIA_PHASE_SYNC_SHIFT_EN	18
+#define  FDIBC_LANE_SHARE_EN		(1<<12)
 #define FDI_PHASE_SYNC_OVR(pipe) (1<<(FDIA_PHASE_SYNC_SHIFT_OVR - ((pipe) * 2)))
 #define FDI_PHASE_SYNC_EN(pipe) (1<<(FDIA_PHASE_SYNC_SHIFT_EN - ((pipe) * 2)))
 #define SOUTH_CHICKEN2		0xc2004
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1e33093..03e0579 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2557,6 +2557,15 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 	I915_READ(reg);
 	udelay(150);
 
+	if (intel_crtc->pipe > 1) {
+		if (intel_crtc->fdi_lanes > 2)
+			I915_WRITE(SOUTH_CHICKEN1, I915_READ(SOUTH_CHICKEN1) &
+				   ~FDIBC_LANE_SHARE_EN);
+		else
+			I915_WRITE(SOUTH_CHICKEN1, I915_READ(SOUTH_CHICKEN1) |
+				   FDIBC_LANE_SHARE_EN);
+	}
+
 	/* enable CPU FDI TX and PCH FDI RX */
 	reg = FDI_TX_CTL(pipe);
 	temp = I915_READ(reg);
@@ -6043,6 +6052,42 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
 	return 120000;
 }
 
+/*
+ * On IVB, we can have 3 pipe configs subject to certain limits.  One
+ * limit is the FDI lane width.  FDI interfaces B and C share lanes, so
+ * if both are enabled each gets 2 lanes max.  Check for that here.
+ */
+static int ivb_validate_fdi(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct drm_crtc *tmp;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *pipeb_crtc = NULL;
+
+	if (!IS_IVYBRIDGE(dev))
+		return 0;
+
+	if (intel_crtc->pipe == 2 && intel_crtc->fdi_lanes > 2) {
+		DRM_DEBUG_KMS("FDI C limited to 2 lanes\n");
+		return -EINVAL;
+	}
+
+	list_for_each_entry(tmp, &mode_config->crtc_list, head) {
+		pipeb_crtc = to_intel_crtc(tmp);
+
+		if (pipeb_crtc->pipe == 1)
+			break;
+	}
+
+	if (pipeb_crtc->fdi_lanes > 2) {
+		DRM_DEBUG_KMS("FDI B taking more than 2 lanes\n");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 				  struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode,
@@ -6224,6 +6269,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_crtc->fdi_lanes = lane;
 
+	ret = ivb_validate_fdi(crtc);
+	if (ret)
+		return ret;
+
 	if (pixel_multiplier > 1)
 		link_bw *= pixel_multiplier;
 	ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,

Re: [RFC] drm/i915: check FDI config for 3 pipe

[Jesse Barnes]

On Wed, 18 Apr 2012 14:25:36 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Three pipe can only be enabled in some cases and updated docs indicate
> a bit to control FDI B+C sharing.
> 
> This patch adds a check to make sure we can support a given config with
> the existing FDI lane configuration, and tries to set the bit as
> appropriate.
> 
> We may want to go further though and just totally break the link
> between our pipes, planes, PLLs, FDI lanes, transcoders, etc and the
> CRTCs we expose to userspace.  We could theoretically just allocate new
> ones every mode set.  That wouldn't help a whole lot though since we
> don't get full configuration changes from userspace, just a small
> window of CRTC data with no idea of what might happen next...

Ah just found a bug in reviewing my own patch here.

+	if (intel_crtc->pipe > 1) {
+		if (intel_crtc->fdi_lanes > 2)
+			I915_WRITE(SOUTH_CHICKEN1, I915_READ(SOUTH_CHICKEN1) &
+				   ~FDIBC_LANE_SHARE_EN);
+		else
+			I915_WRITE(SOUTH_CHICKEN1, I915_READ(SOUTH_CHICKEN1) |
+				   FDIBC_LANE_SHARE_EN);
+	}

Should be if (intel_crtc->pipe > 0) since we need to apply this when
either FDI C *or* B changes config.

I'll try, maybe it'll help with the funky 3 pipe config mode set
problem I saw.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm/i915: check FDI config for 3 pipe

[Jesse Barnes]

On Wed, 18 Apr 2012 15:27:44 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Wed, 18 Apr 2012 14:25:36 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Three pipe can only be enabled in some cases and updated docs indicate
> > a bit to control FDI B+C sharing.
> > 
> > This patch adds a check to make sure we can support a given config with
> > the existing FDI lane configuration, and tries to set the bit as
> > appropriate.
> > 
> > We may want to go further though and just totally break the link
> > between our pipes, planes, PLLs, FDI lanes, transcoders, etc and the
> > CRTCs we expose to userspace.  We could theoretically just allocate new
> > ones every mode set.  That wouldn't help a whole lot though since we
> > don't get full configuration changes from userspace, just a small
> > window of CRTC data with no idea of what might happen next...
> 
> Ah just found a bug in reviewing my own patch here.
> 
> +	if (intel_crtc->pipe > 1) {
> +		if (intel_crtc->fdi_lanes > 2)
> +			I915_WRITE(SOUTH_CHICKEN1, I915_READ(SOUTH_CHICKEN1) &
> +				   ~FDIBC_LANE_SHARE_EN);
> +		else
> +			I915_WRITE(SOUTH_CHICKEN1, I915_READ(SOUTH_CHICKEN1) |
> +				   FDIBC_LANE_SHARE_EN);
> +	}
> 
> Should be if (intel_crtc->pipe > 0) since we need to apply this when
> either FDI C *or* B changes config.
> 
> I'll try, maybe it'll help with the funky 3 pipe config mode set
> problem I saw.

Oh and this hunk belongs in the IVB function!

-- 
Jesse Barnes, Intel Open Source Technology Center