diff mbox

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

Message ID 1364341502-1184-7-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter March 26, 2013, 11:44 p.m. UTC 
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 ...

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_crt.c     | 12 +++++++----
 drivers/gpu/drm/i915/intel_ddi.c     | 16 +++++++--------
 drivers/gpu/drm/i915/intel_display.c | 40 ++++++++----------------------------
 drivers/gpu/drm/i915/intel_dp.c      | 16 +++++++++------
 drivers/gpu/drm/i915/intel_drv.h     | 13 ++++++------
 drivers/gpu/drm/i915/intel_hdmi.c    | 14 ++++++++-----
 drivers/gpu/drm/i915/intel_lvds.c    |  2 ++
 drivers/gpu/drm/i915/intel_sdvo.c    |  3 +++
 8 files changed, 54 insertions(+), 62 deletions(-)

Comments

Jesse Barnes March 27, 2013, 5:06 p.m. UTC | #1 
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>
Daniel Vetter March 27, 2013, 5:11 p.m. UTC | #2 
On Wed, Mar 27, 2013 at 6:06 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> +       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?

That would give you two wrong case currently:
- hsw port D eDP would be marked as pch port
- any pch non-eDP DP ports would not be marked as pch ports

The ugly thing with this patch here is that this property is actually
fixed to the encoder, but I dynamically compute it in compute_config.
We have a few other such cases (e.g. the cpu transcoder for edp on
hsw). But I've figured there's no point in adding something clever,
which then updates the pipe_config according to connected encoders
with data structures ...
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 0e9e9e5..1d8d63a 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -199,10 +199,14 @@  static int intel_crt_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static bool intel_crt_mode_fixup(struct drm_encoder *encoder,
-				 const struct drm_display_mode *mode,
-				 struct drm_display_mode *adjusted_mode)
+static bool intel_crt_compute_config(struct intel_encoder *encoder,
+				     struct intel_crtc_config *pipe_config)
 {
+	struct drm_device *dev = encoder->base.dev;
+
+	if (HAS_PCH_SPLIT(dev))
+		pipe_config->has_pch_encoder = true;
+
 	return true;
 }
 
@@ -676,7 +680,6 @@  static void intel_crt_reset(struct drm_connector *connector)
  */
 
 static const struct drm_encoder_helper_funcs crt_encoder_funcs = {
-	.mode_fixup = intel_crt_mode_fixup,
 	.mode_set = intel_crt_mode_set,
 };
 
@@ -768,6 +771,7 @@  void intel_crt_init(struct drm_device *dev)
 	else
 		crt->adpa_reg = ADPA;
 
+	crt->base.compute_config = intel_crt_compute_config;
 	crt->base.disable = intel_disable_crt;
 	crt->base.enable = intel_enable_crt;
 	if (I915_HAS_HOTPLUG(dev))
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 258e38e..baeb470 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1476,19 +1476,17 @@  static void intel_ddi_destroy(struct drm_encoder *encoder)
 	intel_dp_encoder_destroy(encoder);
 }
 
-static bool intel_ddi_mode_fixup(struct drm_encoder *encoder,
-				 const struct drm_display_mode *mode,
-				 struct drm_display_mode *adjusted_mode)
+static bool intel_ddi_compute_config(struct intel_encoder *encoder,
+				     struct intel_crtc_config *pipe_config)
 {
-	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
-	int type = intel_encoder->type;
+	int type = encoder->type;
 
-	WARN(type == INTEL_OUTPUT_UNKNOWN, "mode_fixup() on unknown output!\n");
+	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
 
 	if (type == INTEL_OUTPUT_HDMI)
-		return intel_hdmi_mode_fixup(encoder, mode, adjusted_mode);
+		return intel_hdmi_compute_config(encoder, pipe_config);
 	else
-		return intel_dp_mode_fixup(encoder, mode, adjusted_mode);
+		return intel_dp_compute_config(encoder, pipe_config);
 }
 
 static const struct drm_encoder_funcs intel_ddi_funcs = {
@@ -1496,7 +1494,6 @@  static const struct drm_encoder_funcs intel_ddi_funcs = {
 };
 
 static const struct drm_encoder_helper_funcs intel_ddi_helper_funcs = {
-	.mode_fixup = intel_ddi_mode_fixup,
 	.mode_set = intel_ddi_mode_set,
 };
 
@@ -1536,6 +1533,7 @@  void intel_ddi_init(struct drm_device *dev, enum port port)
 			 DRM_MODE_ENCODER_TMDS);
 	drm_encoder_helper_add(encoder, &intel_ddi_helper_funcs);
 
+	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3672b8d..fda0754 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2960,27 +2960,6 @@  static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 	mutex_unlock(&dev->struct_mutex);
 }
 
-static bool ironlake_crtc_driving_pch(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct intel_encoder *intel_encoder;
-
-	/*
-	 * If there's a non-PCH eDP on this crtc, it must be DP_A, and that
-	 * must be driven by its own crtc; no sharing is possible.
-	 */
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
-		switch (intel_encoder->type) {
-		case INTEL_OUTPUT_EDP:
-			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
-				return false;
-			continue;
-		}
-	}
-
-	return true;
-}
-
 static bool haswell_crtc_driving_pch(struct drm_crtc *crtc)
 {
 	return intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG);
@@ -3321,7 +3300,6 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 	u32 temp;
-	bool is_pch_port;
 
 	WARN_ON(!crtc->enabled);
 
@@ -3337,9 +3315,8 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 			I915_WRITE(PCH_LVDS, temp | LVDS_PORT_EN);
 	}
 
-	is_pch_port = ironlake_crtc_driving_pch(crtc);
 
-	if (is_pch_port) {
+	if (intel_crtc->config.has_pch_encoder) {
 		/* Note: FDI PLL enabling _must_ be done before we enable the
 		 * cpu pipes, hence this is separate from all the other fdi/pch
 		 * enabling. */
@@ -3376,10 +3353,11 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	 */
 	intel_crtc_load_lut(crtc);
 
-	intel_enable_pipe(dev_priv, pipe, is_pch_port);
+	intel_enable_pipe(dev_priv, pipe,
+			  intel_crtc->config.has_pch_encoder);
 	intel_enable_plane(dev_priv, plane, pipe);
 
-	if (is_pch_port)
+	if (intel_crtc->config.has_pch_encoder)
 		ironlake_pch_enable(crtc);
 
 	mutex_lock(&dev->struct_mutex);
@@ -3413,7 +3391,6 @@  static void haswell_crtc_enable(struct drm_crtc *crtc)
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
-	bool is_pch_port;
 
 	WARN_ON(!crtc->enabled);
 
@@ -3423,9 +3400,7 @@  static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc->active = true;
 	intel_update_watermarks(dev);
 
-	is_pch_port = haswell_crtc_driving_pch(crtc);
-
-	if (is_pch_port)
+	if (intel_crtc->config.has_pch_encoder)
 		dev_priv->display.fdi_link_train(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -3456,10 +3431,11 @@  static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_ddi_set_pipe_settings(crtc);
 	intel_ddi_enable_transcoder_func(crtc);
 
-	intel_enable_pipe(dev_priv, pipe, is_pch_port);
+	intel_enable_pipe(dev_priv, pipe,
+			  intel_crtc->config.has_pch_encoder);
 	intel_enable_plane(dev_priv, plane, pipe);
 
-	if (is_pch_port)
+	if (intel_crtc->config.has_pch_encoder)
 		lpt_pch_enable(crtc);
 
 	mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 88d4eec..bc73e5e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -689,12 +689,13 @@  intel_dp_i2c_init(struct intel_dp *intel_dp,
 }
 
 bool
-intel_dp_mode_fixup(struct drm_encoder *encoder,
-		    const struct drm_display_mode *mode,
-		    struct drm_display_mode *adjusted_mode)
+intel_dp_compute_config(struct intel_encoder *encoder,
+			struct intel_crtc_config *pipe_config)
 {
-	struct drm_device *dev = encoder->dev;
-	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	struct drm_display_mode *mode = &pipe_config->requested_mode;
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int lane_count, clock;
 	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
@@ -702,6 +703,9 @@  intel_dp_mode_fixup(struct drm_encoder *encoder,
 	int bpp, mode_rate;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
 
+	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && !is_cpu_edp(intel_dp))
+		pipe_config->has_pch_encoder = true;
+
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
 				       adjusted_mode);
@@ -2540,7 +2544,6 @@  void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 }
 
 static const struct drm_encoder_helper_funcs intel_dp_helper_funcs = {
-	.mode_fixup = intel_dp_mode_fixup,
 	.mode_set = intel_dp_mode_set,
 };
 
@@ -2960,6 +2963,7 @@  intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 			 DRM_MODE_ENCODER_TMDS);
 	drm_encoder_helper_add(&intel_encoder->base, &intel_dp_helper_funcs);
 
+	intel_encoder->compute_config = intel_dp_compute_config;
 	intel_encoder->enable = intel_enable_dp;
 	intel_encoder->pre_enable = intel_pre_enable_dp;
 	intel_encoder->disable = intel_disable_dp;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f0e5462..8de1855 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -190,6 +190,9 @@  struct intel_crtc_config {
 	 * changes the crtc timings in the mode to prevent the crtc fixup from
 	 * overwriting them.  Currently only lvds needs that. */
 	bool timings_set;
+	/* Whether to set up the PCH/FDI. Note that we never allow sharing
+	 * between pch encoders and cpu encoders. */
+	bool has_pch_encoder;
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
 };
@@ -449,9 +452,8 @@  extern void intel_hdmi_init(struct drm_device *dev,
 extern void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 				      struct intel_connector *intel_connector);
 extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
-extern bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
-				  const struct drm_display_mode *mode,
-				  struct drm_display_mode *adjusted_mode);
+extern bool intel_hdmi_compute_config(struct intel_encoder *encoder,
+				      struct intel_crtc_config *pipe_config);
 extern void intel_dip_infoframe_csum(struct dip_infoframe *avi_if);
 extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
 			    bool is_sdvob);
@@ -475,9 +477,8 @@  extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);
 extern void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 extern void intel_dp_encoder_destroy(struct drm_encoder *encoder);
 extern void intel_dp_check_link_status(struct intel_dp *intel_dp);
-extern bool intel_dp_mode_fixup(struct drm_encoder *encoder,
-				const struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode);
+extern bool intel_dp_compute_config(struct intel_encoder *encoder,
+				    struct intel_crtc_config *pipe_config);
 extern bool intel_dpd_is_edp(struct drm_device *dev);
 extern void ironlake_edp_backlight_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_backlight_off(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b9a83d7..b588e6c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -768,11 +768,12 @@  static int intel_hdmi_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
-			   const struct drm_display_mode *mode,
-			   struct drm_display_mode *adjusted_mode)
+bool intel_hdmi_compute_config(struct intel_encoder *encoder,
+			       struct intel_crtc_config *pipe_config)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 
 	if (intel_hdmi->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -786,6 +787,9 @@  bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
 	if (intel_hdmi->color_range)
 		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
 
+	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev))
+		pipe_config->has_pch_encoder = true;
+
 	return true;
 }
 
@@ -937,7 +941,6 @@  static void intel_hdmi_destroy(struct drm_connector *connector)
 }
 
 static const struct drm_encoder_helper_funcs intel_hdmi_helper_funcs = {
-	.mode_fixup = intel_hdmi_mode_fixup,
 	.mode_set = intel_hdmi_mode_set,
 };
 
@@ -1066,6 +1069,7 @@  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 			 DRM_MODE_ENCODER_TMDS);
 	drm_encoder_helper_add(&intel_encoder->base, &intel_hdmi_helper_funcs);
 
+	intel_encoder->compute_config = intel_hdmi_compute_config;
 	intel_encoder->enable = intel_enable_hdmi;
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index a2c516c..9d6ed91 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -331,6 +331,8 @@  static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 			       adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
+		pipe_config->has_pch_encoder = true;
+
 		intel_pch_panel_fitting(dev,
 					intel_connector->panel.fitting_mode,
 					mode, adjusted_mode);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 6912742..5f3f9e9 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1047,6 +1047,9 @@  static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 	struct drm_display_mode *mode = &pipe_config->requested_mode;
 
+	if (HAS_PCH_SPLIT(encoder->base.dev))
+		pipe_config->has_pch_encoder = true;
+
 	/* We need to construct preferred input timings based on our
 	 * output timings.  To do that, we have to set the output
 	 * timings, even though this isn't really the right place in