Share common logic between eDP and LVDS in panel

Share common logic between eDP and LVDS in panel

[Chris Wilson]

The eDP panel handling lacked a lot of the finese that we have developed
for LVDS, such as handling lid notifications and cacheing EDIDs. Also we
had a lot of connector specific data stuffed into the global
drm_i915_private which makes for a confusing read.

Let's try to reduce that confusion by sharing the common logic.
-Chris

[PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder

[Chris Wilson]

In preparation for introducing intel_lvds_connector to move some of the
LVDS specific storage away from drm_i915_private, first rename the
encoder to avoid potential confusion.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lvds.c |  227 +++++++++++++++++++------------------
 1 files changed, 114 insertions(+), 113 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ece5440..0882e4c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -41,7 +41,7 @@
 #include <linux/acpi.h>
 
 /* Private structure for the integrated LVDS support */
-struct intel_lvds {
+struct intel_lvds_encoder {
 	struct intel_encoder base;
 
 	struct edid *edid;
@@ -54,23 +54,23 @@ struct intel_lvds {
 	struct drm_display_mode *fixed_mode;
 };
 
-static struct intel_lvds *to_intel_lvds(struct drm_encoder *encoder)
+static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
 {
-	return container_of(encoder, struct intel_lvds, base.base);
+	return container_of(encoder, struct intel_lvds_encoder, base.base);
 }
 
-static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector)
+static struct intel_lvds_encoder *intel_attached_lvds(struct drm_connector *connector)
 {
 	return container_of(intel_attached_encoder(connector),
-			    struct intel_lvds, base);
+			    struct intel_lvds_encoder, base);
 }
 
 /**
  * Sets the power state for the panel.
  */
-static void intel_lvds_enable(struct intel_lvds *intel_lvds)
+static void intel_lvds_encoder_enable(struct intel_lvds_encoder *lvds_encoder)
 {
-	struct drm_device *dev = intel_lvds->base.base.dev;
+	struct drm_device *dev = lvds_encoder->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg;
 
@@ -84,7 +84,7 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 
 	I915_WRITE(lvds_reg, I915_READ(lvds_reg) | LVDS_PORT_EN);
 
-	if (intel_lvds->pfit_dirty) {
+	if (lvds_encoder->pfit_dirty) {
 		/*
 		 * Enable automatic panel scaling so that non-native modes
 		 * fill the screen.  The panel fitter should only be
@@ -92,14 +92,14 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 		 * register description and PRM.
 		 */
 		DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
-			      intel_lvds->pfit_control,
-			      intel_lvds->pfit_pgm_ratios);
+			      lvds_encoder->pfit_control,
+			      lvds_encoder->pfit_pgm_ratios);
 		if (wait_for((I915_READ(PP_STATUS) & PP_ON) == 0, 1000)) {
 			DRM_ERROR("timed out waiting for panel to power off\n");
 		} else {
-			I915_WRITE(PFIT_PGM_RATIOS, intel_lvds->pfit_pgm_ratios);
-			I915_WRITE(PFIT_CONTROL, intel_lvds->pfit_control);
-			intel_lvds->pfit_dirty = false;
+			I915_WRITE(PFIT_PGM_RATIOS, lvds_encoder->pfit_pgm_ratios);
+			I915_WRITE(PFIT_CONTROL, lvds_encoder->pfit_control);
+			lvds_encoder->pfit_dirty = false;
 		}
 	}
 
@@ -109,9 +109,9 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 	intel_panel_enable_backlight(dev);
 }
 
-static void intel_lvds_disable(struct intel_lvds *intel_lvds)
+static void intel_lvds_encoder_disable(struct intel_lvds_encoder *lvds_encoder)
 {
-	struct drm_device *dev = intel_lvds->base.base.dev;
+	struct drm_device *dev = lvds_encoder->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg;
 
@@ -127,35 +127,35 @@ static void intel_lvds_disable(struct intel_lvds *intel_lvds)
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
 
-	if (intel_lvds->pfit_control) {
+	if (lvds_encoder->pfit_control) {
 		if (wait_for((I915_READ(PP_STATUS) & PP_ON) == 0, 1000))
 			DRM_ERROR("timed out waiting for panel to power off\n");
 
 		I915_WRITE(PFIT_CONTROL, 0);
-		intel_lvds->pfit_dirty = true;
+		lvds_encoder->pfit_dirty = true;
 	}
 
 	I915_WRITE(lvds_reg, I915_READ(lvds_reg) & ~LVDS_PORT_EN);
 	POSTING_READ(lvds_reg);
 }
 
-static void intel_lvds_dpms(struct drm_encoder *encoder, int mode)
+static void intel_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
-	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
 
 	if (mode == DRM_MODE_DPMS_ON)
-		intel_lvds_enable(intel_lvds);
+		intel_lvds_encoder_enable(lvds_encoder);
 	else
-		intel_lvds_disable(intel_lvds);
+		intel_lvds_encoder_disable(lvds_encoder);
 
 	/* XXX: We never power down the LVDS pairs. */
 }
 
-static int intel_lvds_mode_valid(struct drm_connector *connector,
-				 struct drm_display_mode *mode)
+static int intel_lvds_connector_mode_valid(struct drm_connector *connector,
+					   struct drm_display_mode *mode)
 {
-	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
-	struct drm_display_mode *fixed_mode = intel_lvds->fixed_mode;
+	struct intel_lvds_encoder *lvds_encoder = intel_attached_lvds(connector);
+	struct drm_display_mode *fixed_mode = lvds_encoder->fixed_mode;
 
 	if (mode->hdisplay > fixed_mode->hdisplay)
 		return MODE_PANEL;
@@ -221,14 +221,14 @@ static inline u32 panel_fitter_scaling(u32 source, u32 target)
 	return (FACTOR * ratio + FACTOR/2) / FACTOR;
 }
 
-static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
-				  struct drm_display_mode *mode,
-				  struct drm_display_mode *adjusted_mode)
+static bool intel_lvds_encoder_mode_fixup(struct drm_encoder *encoder,
+					  struct drm_display_mode *mode,
+					  struct drm_display_mode *adjusted_mode)
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
-	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
 	struct drm_encoder *tmp_encoder;
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 	int pipe;
@@ -254,10 +254,10 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	 * with the panel scaling set up to source from the H/VDisplay
 	 * of the original mode.
 	 */
-	intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
+	intel_fixed_panel_mode(lvds_encoder->fixed_mode, adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
-		intel_pch_panel_fitting(dev, intel_lvds->fitting_mode,
+		intel_pch_panel_fitting(dev, lvds_encoder->fitting_mode,
 					mode, adjusted_mode);
 		return true;
 	}
@@ -281,7 +281,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	for_each_pipe(pipe)
 		I915_WRITE(BCLRPAT(pipe), 0);
 
-	switch (intel_lvds->fitting_mode) {
+	switch (lvds_encoder->fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
 		/*
 		 * For centered modes, we have to calculate border widths &
@@ -379,11 +379,11 @@ out:
 	if (INTEL_INFO(dev)->gen < 4 && dev_priv->lvds_dither)
 		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
 
-	if (pfit_control != intel_lvds->pfit_control ||
-	    pfit_pgm_ratios != intel_lvds->pfit_pgm_ratios) {
-		intel_lvds->pfit_control = pfit_control;
-		intel_lvds->pfit_pgm_ratios = pfit_pgm_ratios;
-		intel_lvds->pfit_dirty = true;
+	if (pfit_control != lvds_encoder->pfit_control ||
+	    pfit_pgm_ratios != lvds_encoder->pfit_pgm_ratios) {
+		lvds_encoder->pfit_control = pfit_control;
+		lvds_encoder->pfit_pgm_ratios = pfit_pgm_ratios;
+		lvds_encoder->pfit_dirty = true;
 	}
 	dev_priv->lvds_border_bits = border;
 
@@ -396,11 +396,11 @@ out:
 	return true;
 }
 
-static void intel_lvds_prepare(struct drm_encoder *encoder)
+static void intel_lvds_encoder_prepare(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
 
 	/* We try to do the minimum that is necessary in order to unlock
 	 * the registers for mode setting.
@@ -416,7 +416,7 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
 	if (HAS_PCH_SPLIT(dev)) {
 		I915_WRITE(PCH_PP_CONTROL,
 			   I915_READ(PCH_PP_CONTROL) | PANEL_UNLOCK_REGS);
-	} else if (intel_lvds->pfit_dirty) {
+	} else if (lvds_encoder->pfit_dirty) {
 		I915_WRITE(PP_CONTROL,
 			   (I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS)
 			   & ~POWER_TARGET_ON);
@@ -426,11 +426,11 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
 	}
 }
 
-static void intel_lvds_commit(struct drm_encoder *encoder)
+static void intel_lvds_encoder_commit(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
 
 	/* Undo any unlocking done in prepare to prevent accidental
 	 * adjustment of the registers.
@@ -448,12 +448,12 @@ static void intel_lvds_commit(struct drm_encoder *encoder)
 	/* Always do a full power on as we do not know what state
 	 * we were left in.
 	 */
-	intel_lvds_enable(intel_lvds);
+	intel_lvds_encoder_enable(lvds_encoder);
 }
 
-static void intel_lvds_mode_set(struct drm_encoder *encoder,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode)
+static void intel_lvds_encoder_mode_set(struct drm_encoder *encoder,
+					struct drm_display_mode *mode,
+					struct drm_display_mode *adjusted_mode)
 {
 	/*
 	 * The LVDS pin pair will already have been turned on in the
@@ -470,7 +470,7 @@ static void intel_lvds_mode_set(struct drm_encoder *encoder,
  * needed, using lid status notification from the input layer.
  */
 static enum drm_connector_status
-intel_lvds_detect(struct drm_connector *connector, bool force)
+intel_lvds_connector_detect(struct drm_connector *connector, bool force)
 {
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
@@ -485,16 +485,16 @@ intel_lvds_detect(struct drm_connector *connector, bool force)
 /**
  * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
  */
-static int intel_lvds_get_modes(struct drm_connector *connector)
+static int intel_lvds_connector_get_modes(struct drm_connector *connector)
 {
-	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
+	struct intel_lvds_encoder *lvds_encoder = intel_attached_lvds(connector);
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *mode;
 
-	if (intel_lvds->edid)
-		return drm_add_edid_modes(connector, intel_lvds->edid);
+	if (lvds_encoder->edid)
+		return drm_add_edid_modes(connector, lvds_encoder->edid);
 
-	mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
+	mode = drm_mode_duplicate(dev, lvds_encoder->fixed_mode);
 	if (mode == NULL)
 		return 0;
 
@@ -568,13 +568,13 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
 }
 
 /**
- * intel_lvds_destroy - unregister and free LVDS structures
+ * intel_lvds_encoder_destroy - unregister and free LVDS structures
  * @connector: connector to free
  *
  * Unregister the DDC bus for this connector then free the driver private
  * structure.
  */
-static void intel_lvds_destroy(struct drm_connector *connector)
+static void intel_lvds_connector_destroy(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -586,26 +586,26 @@ static void intel_lvds_destroy(struct drm_connector *connector)
 	kfree(connector);
 }
 
-static int intel_lvds_set_property(struct drm_connector *connector,
-				   struct drm_property *property,
-				   uint64_t value)
+static int intel_lvds_connector_set_property(struct drm_connector *connector,
+					     struct drm_property *property,
+					     uint64_t value)
 {
-	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
+	struct intel_lvds_encoder *lvds_encoder = intel_attached_lvds(connector);
 	struct drm_device *dev = connector->dev;
 
 	if (property == dev->mode_config.scaling_mode_property) {
-		struct drm_crtc *crtc = intel_lvds->base.base.crtc;
+		struct drm_crtc *crtc = lvds_encoder->base.base.crtc;
 
 		if (value == DRM_MODE_SCALE_NONE) {
 			DRM_DEBUG_KMS("no scaling not supported\n");
 			return -EINVAL;
 		}
 
-		if (intel_lvds->fitting_mode == value) {
+		if (lvds_encoder->fitting_mode == value) {
 			/* the LVDS scaling property is not changed */
 			return 0;
 		}
-		intel_lvds->fitting_mode = value;
+		lvds_encoder->fitting_mode = value;
 		if (crtc && crtc->enabled) {
 			/*
 			 * If the CRTC is enabled, the display will be changed
@@ -619,29 +619,29 @@ static int intel_lvds_set_property(struct drm_connector *connector,
 	return 0;
 }
 
-static const struct drm_encoder_helper_funcs intel_lvds_helper_funcs = {
-	.dpms = intel_lvds_dpms,
-	.mode_fixup = intel_lvds_mode_fixup,
-	.prepare = intel_lvds_prepare,
-	.mode_set = intel_lvds_mode_set,
-	.commit = intel_lvds_commit,
+static const struct drm_encoder_helper_funcs intel_lvds_encoder_helper_funcs = {
+	.dpms = intel_lvds_encoder_dpms,
+	.mode_fixup = intel_lvds_encoder_mode_fixup,
+	.prepare = intel_lvds_encoder_prepare,
+	.mode_set = intel_lvds_encoder_mode_set,
+	.commit = intel_lvds_encoder_commit,
 };
 
 static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
-	.get_modes = intel_lvds_get_modes,
-	.mode_valid = intel_lvds_mode_valid,
+	.get_modes = intel_lvds_connector_get_modes,
+	.mode_valid = intel_lvds_connector_mode_valid,
 	.best_encoder = intel_best_encoder,
 };
 
 static const struct drm_connector_funcs intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
-	.detect = intel_lvds_detect,
+	.detect = intel_lvds_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = intel_lvds_set_property,
-	.destroy = intel_lvds_destroy,
+	.set_property = intel_lvds_connector_set_property,
+	.destroy = intel_lvds_connector_destroy,
 };
 
-static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
+static const struct drm_encoder_funcs intel_lvds_encoder_enc_funcs = {
 	.destroy = intel_encoder_destroy,
 };
 
@@ -830,17 +830,16 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
 }
 
 /**
- * intel_lvds_init - setup LVDS connectors on this device
+ * intel_lvds_encoder_init - setup LVDS connectors on this device
  * @dev: drm device
  *
  * Create the connector, register the LVDS DDC bus, and try to figure out what
  * modes we can display on the LVDS panel (if present).
  */
-bool intel_lvds_init(struct drm_device *dev)
+bool intel_lvds_encoder_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_lvds *intel_lvds;
-	struct intel_encoder *intel_encoder;
+	struct intel_lvds_encoder *lvds_encoder;
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
@@ -869,38 +868,40 @@ bool intel_lvds_init(struct drm_device *dev)
 		}
 	}
 
-	intel_lvds = kzalloc(sizeof(struct intel_lvds), GFP_KERNEL);
-	if (!intel_lvds) {
+	lvds_encoder = kzalloc(sizeof(struct intel_lvds_encoder), GFP_KERNEL);
+	if (!lvds_encoder) {
 		return false;
 	}
 
 	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
 	if (!intel_connector) {
-		kfree(intel_lvds);
+		kfree(lvds_encoder);
 		return false;
 	}
 
 	if (!HAS_PCH_SPLIT(dev)) {
-		intel_lvds->pfit_control = I915_READ(PFIT_CONTROL);
+		lvds_encoder->pfit_control = I915_READ(PFIT_CONTROL);
 	}
 
-	intel_encoder = &intel_lvds->base;
-	encoder = &intel_encoder->base;
+	encoder = &lvds_encoder->base.base;
 	connector = &intel_connector->base;
-	drm_connector_init(dev, &intel_connector->base, &intel_lvds_connector_funcs,
+	drm_connector_init(dev,
+			   &intel_connector->base,
+			   &intel_lvds_connector_funcs,
 			   DRM_MODE_CONNECTOR_LVDS);
 
-	drm_encoder_init(dev, &intel_encoder->base, &intel_lvds_enc_funcs,
+	drm_encoder_init(dev, &lvds_encoder->base.base,
+			 &intel_lvds_encoder_enc_funcs,
 			 DRM_MODE_ENCODER_LVDS);
 
-	intel_connector_attach_encoder(intel_connector, intel_encoder);
-	intel_encoder->type = INTEL_OUTPUT_LVDS;
+	intel_connector_attach_encoder(intel_connector, &lvds_encoder->base);
+	lvds_encoder->base.type = INTEL_OUTPUT_LVDS;
 
-	intel_encoder->clone_mask = (1 << INTEL_LVDS_CLONE_BIT);
-	intel_encoder->crtc_mask = (1 << 1);
+	lvds_encoder->base.clone_mask = (1 << INTEL_LVDS_CLONE_BIT);
+	lvds_encoder->base.crtc_mask = (1 << 1);
 	if (INTEL_INFO(dev)->gen >= 5)
-		intel_encoder->crtc_mask |= (1 << 0);
-	drm_encoder_helper_add(encoder, &intel_lvds_helper_funcs);
+		lvds_encoder->base.crtc_mask |= (1 << 0);
+	drm_encoder_helper_add(encoder, &intel_lvds_encoder_helper_funcs);
 	drm_connector_helper_add(connector, &intel_lvds_connector_helper_funcs);
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
@@ -915,7 +916,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	drm_connector_attach_property(&intel_connector->base,
 				      dev->mode_config.scaling_mode_property,
 				      DRM_MODE_SCALE_ASPECT);
-	intel_lvds->fitting_mode = DRM_MODE_SCALE_ASPECT;
+	lvds_encoder->fitting_mode = DRM_MODE_SCALE_ASPECT;
 
 	/*
 	 * LVDS discovery:
@@ -937,19 +938,19 @@ bool intel_lvds_init(struct drm_device *dev)
 	 * Attempt to get the fixed panel mode from DDC.  Assume that the
 	 * preferred mode is the right one.
 	 */
-	intel_lvds->edid = drm_get_edid(connector,
-					&dev_priv->gmbus[pin].adapter);
-	if (intel_lvds->edid) {
+	lvds_encoder->edid = drm_get_edid(connector,
+					  &dev_priv->gmbus[pin].adapter);
+	if (lvds_encoder->edid) {
 		if (drm_add_edid_modes(connector,
-				       intel_lvds->edid)) {
+				       lvds_encoder->edid)) {
 			drm_mode_connector_update_edid_property(connector,
-								intel_lvds->edid);
+								lvds_encoder->edid);
 		} else {
-			kfree(intel_lvds->edid);
-			intel_lvds->edid = NULL;
+			kfree(lvds_encoder->edid);
+			lvds_encoder->edid = NULL;
 		}
 	}
-	if (!intel_lvds->edid) {
+	if (!lvds_encoder->edid) {
 		/* Didn't get an EDID, so
 		 * Set wide sync ranges so we get all modes
 		 * handed to valid_mode for checking
@@ -962,10 +963,10 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
-			intel_lvds->fixed_mode =
+			lvds_encoder->fixed_mode =
 				drm_mode_duplicate(dev, scan);
 			intel_find_lvds_downclock(dev,
-						  intel_lvds->fixed_mode,
+						  lvds_encoder->fixed_mode,
 						  connector);
 			goto out;
 		}
@@ -981,10 +982,10 @@ bool intel_lvds_init(struct drm_device *dev)
 		if (drm_mode_parse_command_line_for_connector(i915_lvds_fixed_mode,
 							      connector,
 							      &mode)) {
-			intel_lvds->fixed_mode =
+			lvds_encoder->fixed_mode =
 				drm_mode_create_from_cmdline_mode(dev, &mode);
-			if (intel_lvds->fixed_mode) {
-				intel_lvds->fixed_mode->type |=
+			if (lvds_encoder->fixed_mode) {
+				lvds_encoder->fixed_mode->type |=
 					DRM_MODE_TYPE_PREFERRED;
 				goto out;
 			}
@@ -993,10 +994,10 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	/* Failed to get EDID, no override, what about VBT? */
 	if (dev_priv->lfp_lvds_vbt_mode) {
-		intel_lvds->fixed_mode =
+		lvds_encoder->fixed_mode =
 			drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
-		if (intel_lvds->fixed_mode) {
-			intel_lvds->fixed_mode->type |=
+		if (lvds_encoder->fixed_mode) {
+			lvds_encoder->fixed_mode->type |=
 				DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
@@ -1017,16 +1018,16 @@ bool intel_lvds_init(struct drm_device *dev)
 	crtc = intel_get_crtc_for_pipe(dev, pipe);
 
 	if (crtc && (lvds & LVDS_PORT_EN)) {
-		intel_lvds->fixed_mode = intel_crtc_mode_get(dev, crtc);
-		if (intel_lvds->fixed_mode) {
-			intel_lvds->fixed_mode->type |=
+		lvds_encoder->fixed_mode = intel_crtc_mode_get(dev, crtc);
+		if (lvds_encoder->fixed_mode) {
+			lvds_encoder->fixed_mode->type |=
 				DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
 	}
 
 	/* If we still don't have a mode after all that, give up. */
-	if (!intel_lvds->fixed_mode)
+	if (!lvds_encoder->fixed_mode)
 		goto failed;
 
 out:
@@ -1063,7 +1064,7 @@ failed:
 	DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
 	drm_connector_cleanup(connector);
 	drm_encoder_cleanup(encoder);
-	kfree(intel_lvds);
+	kfree(lvds_encoder);
 	kfree(intel_connector);
 	return false;
 }
-- 
1.7.4.1

Re: [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:09 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> In preparation for introducing intel_lvds_connector to move some of the
> LVDS specific storage away from drm_i915_private, first rename the
> encoder to avoid potential confusion.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c |  227 +++++++++++++++++++------------------
>  1 files changed, 114 insertions(+), 113 deletions(-)
> 

Looks nice.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector

[Chris Wilson]

Introduce a local structure to move LVDS specific information away from
the drm_i915_private and onto the LVDS connector.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lvds.c |   31 +++++++++++++++++++++----------
 1 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 0882e4c..d82c8da 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -54,11 +54,20 @@ struct intel_lvds_encoder {
 	struct drm_display_mode *fixed_mode;
 };
 
+struct intel_lvds_connector {
+	struct intel_connector base;
+};
+
 static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
 {
 	return container_of(encoder, struct intel_lvds_encoder, base.base);
 }
 
+static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *connector)
+{
+	return container_of(connector, struct intel_lvds_connector, base.base);
+}
+
 static struct intel_lvds_encoder *intel_attached_lvds(struct drm_connector *connector)
 {
 	return container_of(intel_attached_encoder(connector),
@@ -836,11 +845,11 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
  * Create the connector, register the LVDS DDC bus, and try to figure out what
  * modes we can display on the LVDS panel (if present).
  */
-bool intel_lvds_encoder_init(struct drm_device *dev)
+bool intel_lvds_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds_encoder *lvds_encoder;
-	struct intel_connector *intel_connector;
+	struct intel_lvds_connector *lvds_connector;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_display_mode *scan; /* *modes, *bios_mode; */
@@ -873,8 +882,8 @@ bool intel_lvds_encoder_init(struct drm_device *dev)
 		return false;
 	}
 
-	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
-	if (!intel_connector) {
+	lvds_connector = kzalloc(sizeof(struct intel_lvds_connector), GFP_KERNEL);
+	if (!lvds_connector) {
 		kfree(lvds_encoder);
 		return false;
 	}
@@ -884,17 +893,19 @@ bool intel_lvds_encoder_init(struct drm_device *dev)
 	}
 
 	encoder = &lvds_encoder->base.base;
-	connector = &intel_connector->base;
+	connector = &lvds_connector->base.base;
 	drm_connector_init(dev,
-			   &intel_connector->base,
+			   &lvds_connector->base.base,
 			   &intel_lvds_connector_funcs,
 			   DRM_MODE_CONNECTOR_LVDS);
 
-	drm_encoder_init(dev, &lvds_encoder->base.base,
+	drm_encoder_init(dev,
+			 &lvds_encoder->base.base,
 			 &intel_lvds_encoder_enc_funcs,
 			 DRM_MODE_ENCODER_LVDS);
 
-	intel_connector_attach_encoder(intel_connector, &lvds_encoder->base);
+	intel_connector_attach_encoder(&lvds_connector->base,
+				       &lvds_encoder->base);
 	lvds_encoder->base.type = INTEL_OUTPUT_LVDS;
 
 	lvds_encoder->base.clone_mask = (1 << INTEL_LVDS_CLONE_BIT);
@@ -913,7 +924,7 @@ bool intel_lvds_encoder_init(struct drm_device *dev)
 	 * the initial panel fitting mode will be FULL_SCREEN.
 	 */
 
-	drm_connector_attach_property(&intel_connector->base,
+	drm_connector_attach_property(&lvds_connector->base.base,
 				      dev->mode_config.scaling_mode_property,
 				      DRM_MODE_SCALE_ASPECT);
 	lvds_encoder->fitting_mode = DRM_MODE_SCALE_ASPECT;
@@ -1065,6 +1076,6 @@ failed:
 	drm_connector_cleanup(connector);
 	drm_encoder_cleanup(encoder);
 	kfree(lvds_encoder);
-	kfree(intel_connector);
+	kfree(lvds_connector);
 	return false;
 }
-- 
1.7.4.1

Re: [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:10 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Introduce a local structure to move LVDS specific information away from
> the drm_i915_private and onto the LVDS connector.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Nice to split this out.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector

[Ben Widawsky]

On Fri, Apr 22, 2011 at 10:19:10AM +0100, Chris Wilson wrote:
> Introduce a local structure to move LVDS specific information away from
> the drm_i915_private and onto the LVDS connector.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c |   31 +++++++++++++++++++++----------
>  1 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 0882e4c..d82c8da 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -54,11 +54,20 @@ struct intel_lvds_encoder {
>  	struct drm_display_mode *fixed_mode;
>  };
>  
> +struct intel_lvds_connector {
> +	struct intel_connector base;
> +};

How about to save some eyestrain, using some macro helpers?

struct intel_lvds_connector {
	struct intel_connector base;
#define drm_base \
	base.base
#define drm_dev \
	drm_base.dev
#define drm_funcs \
	drm_base.funcs
};

Similarly with lvds_encoder, and intel_dp structures if you so desire.

Ben

Re: [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector

[Chris Wilson]

On Fri, 22 Apr 2011 10:50:47 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> How about to save some eyestrain, using some macro helpers?
> 
> struct intel_lvds_connector {
> 	struct intel_connector base;
> #define drm_base \
> 	base.base
> #define drm_dev \
> 	drm_base.dev
> #define drm_funcs \
> 	drm_base.funcs
> };
> 
> Similarly with lvds_encoder, and intel_dp structures if you so desire.

I'm always a little wary about macros to hide details of the structure,
they make the relationships less discoverable.

Fortunately, by the end of the series most of the eye sores have gone.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h   |    3 ---
 drivers/gpu/drm/i915/intel_lvds.c |   33 +++++++++++++++++----------------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b214709..4c96dfc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -364,8 +364,6 @@ typedef struct drm_i915_private {
 	} edp;
 	bool no_aux_handshake;
 
-	struct notifier_block lid_notifier;
-
 	int crt_ddc_pin;
 	struct drm_i915_fence_reg fence_regs[16]; /* assume 965 */
 	int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
@@ -682,7 +680,6 @@ typedef struct drm_i915_private {
 	u16 orig_clock;
 	int child_dev_num;
 	struct child_device_config *child_dev;
-	struct drm_connector *int_lvds_connector;
 
 	bool mchbar_need_disable;
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d82c8da..e0ada0c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -56,6 +56,8 @@ struct intel_lvds_encoder {
 
 struct intel_lvds_connector {
 	struct intel_connector base;
+
+	struct notifier_block lid_notifier;
 };
 
 static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
@@ -543,18 +545,18 @@ static const struct dmi_system_id intel_no_modeset_on_lid[] = {
 static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
 			    void *unused)
 {
-	struct drm_i915_private *dev_priv =
-		container_of(nb, struct drm_i915_private, lid_notifier);
-	struct drm_device *dev = dev_priv->dev;
-	struct drm_connector *connector = dev_priv->int_lvds_connector;
+	struct intel_lvds_connector *lvds_connector =
+		container_of(nb, struct intel_lvds_connector, lid_notifier);
+	struct drm_device *dev = lvds_connector->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/*
 	 * check and update the status of LVDS connector after receiving
 	 * the LID nofication event.
 	 */
-	if (connector)
-		connector->status = connector->funcs->detect(connector,
-							     false);
+	lvds_connector->base.base.status =
+		lvds_connector->base.base.funcs->detect(&lvds_connector->base.base,
+							false);
 
 	/* Don't force modeset on machines where it causes a GPU lockup */
 	if (dmi_check_system(intel_no_modeset_on_lid))
@@ -585,11 +587,12 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
  */
 static void intel_lvds_connector_destroy(struct drm_connector *connector)
 {
-	struct drm_device *dev = connector->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_lvds_connector *lvds_connector =
+		to_lvds_connector(connector);
+
+	if (lvds_connector->lid_notifier.notifier_call)
+		acpi_lid_notifier_unregister(&lvds_connector->lid_notifier);
 
-	if (dev_priv->lid_notifier.notifier_call)
-		acpi_lid_notifier_unregister(&dev_priv->lid_notifier);
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
@@ -1061,13 +1064,11 @@ out:
 		pwm |= PWM_PCH_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
 	}
-	dev_priv->lid_notifier.notifier_call = intel_lid_notify;
-	if (acpi_lid_notifier_register(&dev_priv->lid_notifier)) {
+	lvds_connector->lid_notifier.notifier_call = intel_lid_notify;
+	if (acpi_lid_notifier_register(&lvds_connector->lid_notifier)) {
 		DRM_DEBUG_KMS("lid notifier registration failed\n");
-		dev_priv->lid_notifier.notifier_call = NULL;
+		lvds_connector->lid_notifier.notifier_call = NULL;
 	}
-	/* keep the LVDS connector */
-	dev_priv->int_lvds_connector = connector;
 	drm_sysfs_connector_add(connector);
 	return true;
 
-- 
1.7.4.1

Re: [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:11 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |    3 ---
>  drivers/gpu/drm/i915/intel_lvds.c |   33 +++++++++++++++++----------------
>  2 files changed, 17 insertions(+), 19 deletions(-)
> 

Yeah, much better.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 04/10] drm/i915/lvds: Move some connector specific info across from the encoder

[Chris Wilson]

As there is 1:1 mapping between encoder and connector for the LVDS, the
goal is to simply reduce the amount of noise within the connector
functions, i.e. we split the encoder/connector for LVDS as best we can
and try to only operate on the LVDS connector from the connector funcs
and the LVDS encoder form the encoder funcs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lvds.c |  101 ++++++++++++++++++-------------------
 1 files changed, 50 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index e0ada0c..f87adaa 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -41,23 +41,24 @@
 #include <linux/acpi.h>
 
 /* Private structure for the integrated LVDS support */
-struct intel_lvds_encoder {
-	struct intel_encoder base;
+struct intel_lvds_connector {
+	struct intel_connector base;
 
+	struct notifier_block lid_notifier;
+	struct drm_display_mode *fixed_mode;
 	struct edid *edid;
 
 	int fitting_mode;
+};
+
+struct intel_lvds_encoder {
+	struct intel_encoder base;
+
 	u32 pfit_control;
 	u32 pfit_pgm_ratios;
 	bool pfit_dirty;
 
-	struct drm_display_mode *fixed_mode;
-};
-
-struct intel_lvds_connector {
-	struct intel_connector base;
-
-	struct notifier_block lid_notifier;
+	struct intel_lvds_connector *attached_connector;
 };
 
 static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
@@ -70,12 +71,6 @@ static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *conn
 	return container_of(connector, struct intel_lvds_connector, base.base);
 }
 
-static struct intel_lvds_encoder *intel_attached_lvds(struct drm_connector *connector)
-{
-	return container_of(intel_attached_encoder(connector),
-			    struct intel_lvds_encoder, base);
-}
-
 /**
  * Sets the power state for the panel.
  */
@@ -165,8 +160,8 @@ static void intel_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
 static int intel_lvds_connector_mode_valid(struct drm_connector *connector,
 					   struct drm_display_mode *mode)
 {
-	struct intel_lvds_encoder *lvds_encoder = intel_attached_lvds(connector);
-	struct drm_display_mode *fixed_mode = lvds_encoder->fixed_mode;
+	struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
+	struct drm_display_mode *fixed_mode = lvds_connector->fixed_mode;
 
 	if (mode->hdisplay > fixed_mode->hdisplay)
 		return MODE_PANEL;
@@ -240,6 +235,7 @@ static bool intel_lvds_encoder_mode_fixup(struct drm_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
+	struct intel_lvds_connector *lvds_connector = lvds_encoder->attached_connector;
 	struct drm_encoder *tmp_encoder;
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 	int pipe;
@@ -265,10 +261,10 @@ static bool intel_lvds_encoder_mode_fixup(struct drm_encoder *encoder,
 	 * with the panel scaling set up to source from the H/VDisplay
 	 * of the original mode.
 	 */
-	intel_fixed_panel_mode(lvds_encoder->fixed_mode, adjusted_mode);
+	intel_fixed_panel_mode(lvds_connector->fixed_mode, adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
-		intel_pch_panel_fitting(dev, lvds_encoder->fitting_mode,
+		intel_pch_panel_fitting(dev, lvds_connector->fitting_mode,
 					mode, adjusted_mode);
 		return true;
 	}
@@ -292,7 +288,7 @@ static bool intel_lvds_encoder_mode_fixup(struct drm_encoder *encoder,
 	for_each_pipe(pipe)
 		I915_WRITE(BCLRPAT(pipe), 0);
 
-	switch (lvds_encoder->fitting_mode) {
+	switch (lvds_connector->fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
 		/*
 		 * For centered modes, we have to calculate border widths &
@@ -498,14 +494,14 @@ intel_lvds_connector_detect(struct drm_connector *connector, bool force)
  */
 static int intel_lvds_connector_get_modes(struct drm_connector *connector)
 {
-	struct intel_lvds_encoder *lvds_encoder = intel_attached_lvds(connector);
+	struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
 	struct drm_device *dev = connector->dev;
 	struct drm_display_mode *mode;
 
-	if (lvds_encoder->edid)
-		return drm_add_edid_modes(connector, lvds_encoder->edid);
+	if (lvds_connector->edid)
+		return drm_add_edid_modes(connector, lvds_connector->edid);
 
-	mode = drm_mode_duplicate(dev, lvds_encoder->fixed_mode);
+	mode = drm_mode_duplicate(dev, lvds_connector->fixed_mode);
 	if (mode == NULL)
 		return 0;
 
@@ -602,29 +598,31 @@ static int intel_lvds_connector_set_property(struct drm_connector *connector,
 					     struct drm_property *property,
 					     uint64_t value)
 {
-	struct intel_lvds_encoder *lvds_encoder = intel_attached_lvds(connector);
+	struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
 	struct drm_device *dev = connector->dev;
 
 	if (property == dev->mode_config.scaling_mode_property) {
-		struct drm_crtc *crtc = lvds_encoder->base.base.crtc;
+		struct drm_crtc *crtc;
 
 		if (value == DRM_MODE_SCALE_NONE) {
 			DRM_DEBUG_KMS("no scaling not supported\n");
 			return -EINVAL;
 		}
 
-		if (lvds_encoder->fitting_mode == value) {
+		if (lvds_connector->fitting_mode == value) {
 			/* the LVDS scaling property is not changed */
 			return 0;
 		}
-		lvds_encoder->fitting_mode = value;
+		lvds_connector->fitting_mode = value;
+
+		crtc = intel_attached_encoder(connector)->base.crtc;
 		if (crtc && crtc->enabled) {
 			/*
 			 * If the CRTC is enabled, the display will be changed
 			 * according to the new panel fitting mode.
 			 */
 			drm_crtc_helper_set_mode(crtc, &crtc->mode,
-				crtc->x, crtc->y, crtc->fb);
+						 crtc->x, crtc->y, crtc->fb);
 		}
 	}
 
@@ -891,6 +889,8 @@ bool intel_lvds_init(struct drm_device *dev)
 		return false;
 	}
 
+	lvds_encoder->attached_connector = lvds_connector;
+
 	if (!HAS_PCH_SPLIT(dev)) {
 		lvds_encoder->pfit_control = I915_READ(PFIT_CONTROL);
 	}
@@ -930,7 +930,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	drm_connector_attach_property(&lvds_connector->base.base,
 				      dev->mode_config.scaling_mode_property,
 				      DRM_MODE_SCALE_ASPECT);
-	lvds_encoder->fitting_mode = DRM_MODE_SCALE_ASPECT;
+	lvds_connector->fitting_mode = DRM_MODE_SCALE_ASPECT;
 
 	/*
 	 * LVDS discovery:
@@ -952,19 +952,18 @@ bool intel_lvds_init(struct drm_device *dev)
 	 * Attempt to get the fixed panel mode from DDC.  Assume that the
 	 * preferred mode is the right one.
 	 */
-	lvds_encoder->edid = drm_get_edid(connector,
-					  &dev_priv->gmbus[pin].adapter);
-	if (lvds_encoder->edid) {
-		if (drm_add_edid_modes(connector,
-				       lvds_encoder->edid)) {
+	lvds_connector->edid = drm_get_edid(connector,
+					    &dev_priv->gmbus[pin].adapter);
+	if (lvds_connector->edid) {
+		if (drm_add_edid_modes(connector, lvds_connector->edid)) {
 			drm_mode_connector_update_edid_property(connector,
-								lvds_encoder->edid);
+								lvds_connector->edid);
 		} else {
-			kfree(lvds_encoder->edid);
-			lvds_encoder->edid = NULL;
+			kfree(lvds_connector->edid);
+			lvds_connector->edid = NULL;
 		}
 	}
-	if (!lvds_encoder->edid) {
+	if (!lvds_connector->edid) {
 		/* Didn't get an EDID, so
 		 * Set wide sync ranges so we get all modes
 		 * handed to valid_mode for checking
@@ -977,10 +976,10 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
-			lvds_encoder->fixed_mode =
+			lvds_connector->fixed_mode =
 				drm_mode_duplicate(dev, scan);
 			intel_find_lvds_downclock(dev,
-						  lvds_encoder->fixed_mode,
+						  lvds_connector->fixed_mode,
 						  connector);
 			goto out;
 		}
@@ -996,10 +995,10 @@ bool intel_lvds_init(struct drm_device *dev)
 		if (drm_mode_parse_command_line_for_connector(i915_lvds_fixed_mode,
 							      connector,
 							      &mode)) {
-			lvds_encoder->fixed_mode =
+			lvds_connector->fixed_mode =
 				drm_mode_create_from_cmdline_mode(dev, &mode);
-			if (lvds_encoder->fixed_mode) {
-				lvds_encoder->fixed_mode->type |=
+			if (lvds_connector->fixed_mode) {
+				lvds_connector->fixed_mode->type |=
 					DRM_MODE_TYPE_PREFERRED;
 				goto out;
 			}
@@ -1008,10 +1007,10 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	/* Failed to get EDID, no override, what about VBT? */
 	if (dev_priv->lfp_lvds_vbt_mode) {
-		lvds_encoder->fixed_mode =
+		lvds_connector->fixed_mode =
 			drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
-		if (lvds_encoder->fixed_mode) {
-			lvds_encoder->fixed_mode->type |=
+		if (lvds_connector->fixed_mode) {
+			lvds_connector->fixed_mode->type |=
 				DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
@@ -1032,16 +1031,16 @@ bool intel_lvds_init(struct drm_device *dev)
 	crtc = intel_get_crtc_for_pipe(dev, pipe);
 
 	if (crtc && (lvds & LVDS_PORT_EN)) {
-		lvds_encoder->fixed_mode = intel_crtc_mode_get(dev, crtc);
-		if (lvds_encoder->fixed_mode) {
-			lvds_encoder->fixed_mode->type |=
+		lvds_connector->fixed_mode = intel_crtc_mode_get(dev, crtc);
+		if (lvds_connector->fixed_mode) {
+			lvds_connector->fixed_mode->type |=
 				DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
 	}
 
 	/* If we still don't have a mode after all that, give up. */
-	if (!lvds_encoder->fixed_mode)
+	if (!lvds_connector->fixed_mode)
 		goto failed;
 
 out:
-- 
1.7.4.1

Re: [PATCH 04/10] drm/i915/lvds: Move some connector specific info across from the encoder

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:12 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> As there is 1:1 mapping between encoder and connector for the LVDS, the
> goal is to simply reduce the amount of noise within the connector
> functions, i.e. we split the encoder/connector for LVDS as best we can
> and try to only operate on the LVDS connector from the connector funcs
> and the LVDS encoder form the encoder funcs.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Nice cleanup.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 05/10] drm/i915: Move the ACPI lid notifier away from LVDS to the generic panel

[Chris Wilson]

... in preparation for enabling it from eDP as well.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_drv.h   |    8 +++
 drivers/gpu/drm/i915/intel_lvds.c  |   79 ++-----------------------------
 drivers/gpu/drm/i915/intel_panel.c |   92 ++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5b0d83..7ca9003 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -257,6 +257,14 @@ extern void intel_edp_link_config (struct intel_encoder *, int *, int *);
 extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
 
 /* intel_panel.c */
+struct intel_panel {
+	struct intel_connector *connector;
+	struct notifier_block lid_notifier;
+};
+extern int intel_panel_init(struct intel_panel *panel,
+			    struct intel_connector *connector);
+extern void intel_panel_fini(struct intel_panel *panel);
+
 extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 				   struct drm_display_mode *adjusted_mode);
 extern void intel_pch_panel_fitting(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index f87adaa..20d6235 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -27,7 +27,6 @@
  *      Jesse Barnes <jesse.barnes@intel.com>
  */
 
-#include <acpi/button.h>
 #include <linux/dmi.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
@@ -38,13 +37,12 @@
 #include "intel_drv.h"
 #include "i915_drm.h"
 #include "i915_drv.h"
-#include <linux/acpi.h>
 
 /* Private structure for the integrated LVDS support */
 struct intel_lvds_connector {
 	struct intel_connector base;
+	struct intel_panel panel;
 
-	struct notifier_block lid_notifier;
 	struct drm_display_mode *fixed_mode;
 	struct edid *edid;
 
@@ -509,71 +507,6 @@ static int intel_lvds_connector_get_modes(struct drm_connector *connector)
 	return 1;
 }
 
-static int intel_no_modeset_on_lid_dmi_callback(const struct dmi_system_id *id)
-{
-	DRM_DEBUG_KMS("Skipping forced modeset for %s\n", id->ident);
-	return 1;
-}
-
-/* The GPU hangs up on these systems if modeset is performed on LID open */
-static const struct dmi_system_id intel_no_modeset_on_lid[] = {
-	{
-		.callback = intel_no_modeset_on_lid_dmi_callback,
-		.ident = "Toshiba Tecra A11",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "TECRA A11"),
-		},
-	},
-
-	{ }	/* terminating entry */
-};
-
-/*
- * Lid events. Note the use of 'modeset_on_lid':
- *  - we set it on lid close, and reset it on open
- *  - we use it as a "only once" bit (ie we ignore
- *    duplicate events where it was already properly
- *    set/reset)
- *  - the suspend/resume paths will also set it to
- *    zero, since they restore the mode ("lid open").
- */
-static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
-			    void *unused)
-{
-	struct intel_lvds_connector *lvds_connector =
-		container_of(nb, struct intel_lvds_connector, lid_notifier);
-	struct drm_device *dev = lvds_connector->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	/*
-	 * check and update the status of LVDS connector after receiving
-	 * the LID nofication event.
-	 */
-	lvds_connector->base.base.status =
-		lvds_connector->base.base.funcs->detect(&lvds_connector->base.base,
-							false);
-
-	/* Don't force modeset on machines where it causes a GPU lockup */
-	if (dmi_check_system(intel_no_modeset_on_lid))
-		return NOTIFY_OK;
-	if (!acpi_lid_open()) {
-		dev_priv->modeset_on_lid = 1;
-		return NOTIFY_OK;
-	}
-
-	if (!dev_priv->modeset_on_lid)
-		return NOTIFY_OK;
-
-	dev_priv->modeset_on_lid = 0;
-
-	mutex_lock(&dev->mode_config.mutex);
-	drm_helper_resume_force_mode(dev);
-	mutex_unlock(&dev->mode_config.mutex);
-
-	return NOTIFY_OK;
-}
-
 /**
  * intel_lvds_encoder_destroy - unregister and free LVDS structures
  * @connector: connector to free
@@ -586,8 +519,7 @@ static void intel_lvds_connector_destroy(struct drm_connector *connector)
 	struct intel_lvds_connector *lvds_connector =
 		to_lvds_connector(connector);
 
-	if (lvds_connector->lid_notifier.notifier_call)
-		acpi_lid_notifier_unregister(&lvds_connector->lid_notifier);
+	intel_panel_fini(&lvds_connector->panel);
 
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
@@ -1063,11 +995,8 @@ out:
 		pwm |= PWM_PCH_ENABLE;
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
 	}
-	lvds_connector->lid_notifier.notifier_call = intel_lid_notify;
-	if (acpi_lid_notifier_register(&lvds_connector->lid_notifier)) {
-		DRM_DEBUG_KMS("lid notifier registration failed\n");
-		lvds_connector->lid_notifier.notifier_call = NULL;
-	}
+
+	intel_panel_init(&lvds_connector->panel, &lvds_connector->base);
 	drm_sysfs_connector_add(connector);
 	return true;
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a06ff07..d2dc066 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -28,6 +28,10 @@
  *      Chris Wilson <chris@chris-wilson.co.uk>
  */
 
+#include <linux/acpi.h>
+#include <linux/dmi.h>
+#include <acpi/button.h>
+
 #include "intel_drv.h"
 
 #define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
@@ -112,6 +116,94 @@ done:
 	dev_priv->pch_pf_size = (width << 16) | height;
 }
 
+/* Lid interactions */
+
+static int intel_no_modeset_on_lid_dmi_callback(const struct dmi_system_id *id)
+{
+	DRM_DEBUG_KMS("Skipping forced modeset for %s\n", id->ident);
+	return 1;
+}
+
+/* The GPU hangs up on these systems if modeset is performed on LID open */
+static const struct dmi_system_id intel_no_modeset_on_lid[] = {
+	{
+		.callback = intel_no_modeset_on_lid_dmi_callback,
+		.ident = "Toshiba Tecra A11",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TECRA A11"),
+		},
+	},
+
+	{ }	/* terminating entry */
+};
+
+/*
+ * Lid events. Note the use of 'modeset_on_lid':
+ *  - we set it on lid close, and reset it on open
+ *  - we use it as a "only once" bit (ie we ignore
+ *    duplicate events where it was already properly
+ *    set/reset)
+ *  - the suspend/resume paths will also set it to
+ *    zero, since they restore the mode ("lid open").
+ */
+static int intel_panel_lid_notify(struct notifier_block *nb, unsigned long val,
+				  void *unused)
+{
+	struct intel_panel *panel =
+		container_of(nb, struct intel_panel, lid_notifier);
+	struct intel_connector *connector = panel->connector;
+	struct drm_device *dev = connector->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/*
+	 * check and update the status of LVDS connector after receiving
+	 * the LID nofication event.
+	 */
+	connector->base.status =
+		connector->base.funcs->detect(&connector->base, false);
+
+	/* Don't force modeset on machines where it causes a GPU lockup */
+	if (dmi_check_system(intel_no_modeset_on_lid))
+		return NOTIFY_OK;
+	if (!acpi_lid_open()) {
+		dev_priv->modeset_on_lid = 1;
+		return NOTIFY_OK;
+	}
+
+	if (!dev_priv->modeset_on_lid)
+		return NOTIFY_OK;
+
+	dev_priv->modeset_on_lid = 0;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_helper_resume_force_mode(dev);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	return NOTIFY_OK;
+}
+
+int intel_panel_init(struct intel_panel *panel, struct intel_connector *connector)
+{
+	panel->connector = connector;
+
+	panel->lid_notifier.notifier_call = intel_panel_lid_notify;
+	if (acpi_lid_notifier_register(&panel->lid_notifier)) {
+		DRM_DEBUG_KMS("lid notifier registration failed\n");
+		panel->lid_notifier.notifier_call = NULL;
+	}
+
+	return 0;
+}
+
+void intel_panel_fini(struct intel_panel *panel)
+{
+	if (panel->lid_notifier.notifier_call)
+		acpi_lid_notifier_unregister(&panel->lid_notifier);
+}
+
+/* Panel backlight controls */
+
 static int is_backlight_combination_mode(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.7.4.1

Re: [PATCH 05/10] drm/i915: Move the ACPI lid notifier away from LVDS to the generic panel

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:13 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ... in preparation for enabling it from eDP as well.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |    8 +++
>  drivers/gpu/drm/i915/intel_lvds.c  |   79 ++-----------------------------
>  drivers/gpu/drm/i915/intel_panel.c |   92 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+), 75 deletions(-)

Not sure if this one or 6/10 are needed.  Originally we added lid
status support to detect LVDS presence and to reset modes if the BIOS
shut off planes/pipes at lid close time.

But neither of those was every very pretty, and the presence check was
a complete failure because every vendor has another, accurate way to
provide lid status; ACPI was only ever a red herring for us poor Linux
folks.

But if you have some good use case and we can get some really really
good test coverage, these patches look nice.

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 06/10] drm/i915/dp: Enable lid notifier for eDP

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0daefca..3ef944f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -60,6 +60,8 @@ struct intel_dp {
 	uint8_t	train_set[4];
 	uint8_t link_status[DP_LINK_STATUS_SIZE];
 
+	struct intel_panel panel; /* only for eDP! */
+
 	struct drm_property *force_audio_property;
 };
 
@@ -1743,6 +1745,8 @@ static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+	intel_panel_fini(&intel_dp->panel);
+
 	i2c_del_adapter(&intel_dp->adapter);
 	drm_encoder_cleanup(encoder);
 	kfree(intel_dp);
@@ -1976,6 +1980,8 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 					DRM_MODE_TYPE_PREFERRED;
 			}
 		}
+
+		intel_panel_init(&intel_dp->panel, intel_connector);
 	}
 
 	intel_dp_add_properties(intel_dp, connector);
-- 
1.7.4.1

[PATCH 07/10] drm/i915: Move the fixed_mode away from drm_i915_private and to the panel

[Chris Wilson]

... as matches usage.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h    |    1 -
 drivers/gpu/drm/i915/intel_dp.c    |   43 +++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h   |    4 ++-
 drivers/gpu/drm/i915/intel_lvds.c  |   45 ++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_panel.c |    9 ++++++-
 5 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4c96dfc..8abc98f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -340,7 +340,6 @@ typedef struct drm_i915_private {
 	/* LVDS info */
 	int backlight_level;  /* restore backlight to this value */
 	bool backlight_enabled;
-	struct drm_display_mode *panel_fixed_mode;
 	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
 	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3ef944f..384017c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -202,16 +202,14 @@ intel_dp_mode_valid(struct drm_connector *connector,
 		    struct drm_display_mode *mode)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct drm_device *dev = connector->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	int max_link_clock = intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
 	int max_lanes = intel_dp_max_lane_count(intel_dp);
 
-	if (is_edp(intel_dp) && dev_priv->panel_fixed_mode) {
-		if (mode->hdisplay > dev_priv->panel_fixed_mode->hdisplay)
+	if (is_edp(intel_dp) && intel_dp->panel.fixed_mode) {
+		if (mode->hdisplay > intel_dp->panel.fixed_mode->hdisplay)
 			return MODE_PANEL;
 
-		if (mode->vdisplay > dev_priv->panel_fixed_mode->vdisplay)
+		if (mode->vdisplay > intel_dp->panel.fixed_mode->vdisplay)
 			return MODE_PANEL;
 	}
 
@@ -592,22 +590,22 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		    struct drm_display_mode *adjusted_mode)
 {
 	struct drm_device *dev = encoder->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	int lane_count, clock;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);
 	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
 
-	if (is_edp(intel_dp) && dev_priv->panel_fixed_mode) {
-		intel_fixed_panel_mode(dev_priv->panel_fixed_mode, adjusted_mode);
+	if (is_edp(intel_dp) && intel_dp->panel.fixed_mode) {
+		intel_fixed_panel_mode(intel_dp->panel.fixed_mode,
+				       adjusted_mode);
 		intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
 					mode, adjusted_mode);
 		/*
 		 * the mode->clock is used to calculate the Data&Link M/N
 		 * of the pipe. For the eDP the fixed clock should be used.
 		 */
-		mode->clock = dev_priv->panel_fixed_mode->clock;
+		mode->clock = intel_dp->panel.fixed_mode->clock;
 	}
 
 	for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) {
@@ -1625,7 +1623,6 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct drm_device *dev = intel_dp->base.base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
 	/* We should parse the EDID data and find out if it has an audio sink
@@ -1633,12 +1630,12 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 
 	ret = intel_ddc_get_modes(connector, &intel_dp->adapter);
 	if (ret) {
-		if (is_edp(intel_dp) && !dev_priv->panel_fixed_mode) {
+		if (is_edp(intel_dp) && !intel_dp->panel.fixed_mode) {
 			struct drm_display_mode *newmode;
 			list_for_each_entry(newmode, &connector->probed_modes,
 					    head) {
 				if (newmode->type & DRM_MODE_TYPE_PREFERRED) {
-					dev_priv->panel_fixed_mode =
+					intel_dp->panel.fixed_mode =
 						drm_mode_duplicate(dev, newmode);
 					break;
 				}
@@ -1650,9 +1647,10 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 
 	/* if eDP has no EDID, try to use fixed panel mode from VBT */
 	if (is_edp(intel_dp)) {
-		if (dev_priv->panel_fixed_mode != NULL) {
+		if (intel_dp->panel.fixed_mode != NULL) {
 			struct drm_display_mode *mode;
-			mode = drm_mode_duplicate(dev, dev_priv->panel_fixed_mode);
+			mode = drm_mode_duplicate(dev,
+						  intel_dp->panel.fixed_mode);
 			drm_mode_probed_add(connector, mode);
 			return 1;
 		}
@@ -1971,17 +1969,20 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 	intel_encoder->hot_plug = intel_dp_hot_plug;
 
 	if (is_edp(intel_dp)) {
+		struct drm_display_mode *fixed_mode = NULL;
+
 		/* initialize panel mode from VBT if available for eDP */
 		if (dev_priv->lfp_lvds_vbt_mode) {
-			dev_priv->panel_fixed_mode =
-				drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
-			if (dev_priv->panel_fixed_mode) {
-				dev_priv->panel_fixed_mode->type |=
-					DRM_MODE_TYPE_PREFERRED;
-			}
+			fixed_mode =
+				drm_mode_duplicate(dev,
+						   dev_priv->lfp_lvds_vbt_mode);
+			if (fixed_mode)
+				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 		}
 
-		intel_panel_init(&intel_dp->panel, intel_connector);
+		intel_panel_init(&intel_dp->panel,
+				 intel_connector,
+				 fixed_mode);
 	}
 
 	intel_dp_add_properties(intel_dp, connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7ca9003..09bfc6b0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -259,10 +259,12 @@ extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
 /* intel_panel.c */
 struct intel_panel {
 	struct intel_connector *connector;
+	struct drm_display_mode *fixed_mode;
 	struct notifier_block lid_notifier;
 };
 extern int intel_panel_init(struct intel_panel *panel,
-			    struct intel_connector *connector);
+			    struct intel_connector *connector,
+			    struct drm_display_mode *fixed_mode);
 extern void intel_panel_fini(struct intel_panel *panel);
 
 extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 20d6235..68f03ea 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -43,7 +43,6 @@ struct intel_lvds_connector {
 	struct intel_connector base;
 	struct intel_panel panel;
 
-	struct drm_display_mode *fixed_mode;
 	struct edid *edid;
 
 	int fitting_mode;
@@ -159,7 +158,7 @@ static int intel_lvds_connector_mode_valid(struct drm_connector *connector,
 					   struct drm_display_mode *mode)
 {
 	struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
-	struct drm_display_mode *fixed_mode = lvds_connector->fixed_mode;
+	struct drm_display_mode *fixed_mode = lvds_connector->panel.fixed_mode;
 
 	if (mode->hdisplay > fixed_mode->hdisplay)
 		return MODE_PANEL;
@@ -259,7 +258,7 @@ static bool intel_lvds_encoder_mode_fixup(struct drm_encoder *encoder,
 	 * with the panel scaling set up to source from the H/VDisplay
 	 * of the original mode.
 	 */
-	intel_fixed_panel_mode(lvds_connector->fixed_mode, adjusted_mode);
+	intel_fixed_panel_mode(lvds_connector->panel.fixed_mode, adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
 		intel_pch_panel_fitting(dev, lvds_connector->fitting_mode,
@@ -499,7 +498,7 @@ static int intel_lvds_connector_get_modes(struct drm_connector *connector)
 	if (lvds_connector->edid)
 		return drm_add_edid_modes(connector, lvds_connector->edid);
 
-	mode = drm_mode_duplicate(dev, lvds_connector->fixed_mode);
+	mode = drm_mode_duplicate(dev, lvds_connector->panel.fixed_mode);
 	if (mode == NULL)
 		return 0;
 
@@ -786,6 +785,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_display_mode *scan; /* *modes, *bios_mode; */
+	struct drm_display_mode *fixed_mode = NULL;
 	struct drm_crtc *crtc;
 	u32 lvds;
 	int pipe;
@@ -908,11 +908,8 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
-			lvds_connector->fixed_mode =
-				drm_mode_duplicate(dev, scan);
-			intel_find_lvds_downclock(dev,
-						  lvds_connector->fixed_mode,
-						  connector);
+			fixed_mode = drm_mode_duplicate(dev, scan);
+			intel_find_lvds_downclock(dev, fixed_mode, connector);
 			goto out;
 		}
 	}
@@ -927,11 +924,10 @@ bool intel_lvds_init(struct drm_device *dev)
 		if (drm_mode_parse_command_line_for_connector(i915_lvds_fixed_mode,
 							      connector,
 							      &mode)) {
-			lvds_connector->fixed_mode =
+			fixed_mode =
 				drm_mode_create_from_cmdline_mode(dev, &mode);
-			if (lvds_connector->fixed_mode) {
-				lvds_connector->fixed_mode->type |=
-					DRM_MODE_TYPE_PREFERRED;
+			if (fixed_mode) {
+				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 				goto out;
 			}
 		}
@@ -939,11 +935,9 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	/* Failed to get EDID, no override, what about VBT? */
 	if (dev_priv->lfp_lvds_vbt_mode) {
-		lvds_connector->fixed_mode =
-			drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
-		if (lvds_connector->fixed_mode) {
-			lvds_connector->fixed_mode->type |=
-				DRM_MODE_TYPE_PREFERRED;
+		fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
+		if (fixed_mode) {
+			fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
 	}
@@ -963,16 +957,15 @@ bool intel_lvds_init(struct drm_device *dev)
 	crtc = intel_get_crtc_for_pipe(dev, pipe);
 
 	if (crtc && (lvds & LVDS_PORT_EN)) {
-		lvds_connector->fixed_mode = intel_crtc_mode_get(dev, crtc);
-		if (lvds_connector->fixed_mode) {
-			lvds_connector->fixed_mode->type |=
-				DRM_MODE_TYPE_PREFERRED;
+		fixed_mode = intel_crtc_mode_get(dev, crtc);
+		if (fixed_mode) {
+			fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
 	}
 
 	/* If we still don't have a mode after all that, give up. */
-	if (!lvds_connector->fixed_mode)
+	if (!fixed_mode)
 		goto failed;
 
 out:
@@ -996,7 +989,9 @@ out:
 		I915_WRITE(BLC_PWM_PCH_CTL1, pwm);
 	}
 
-	intel_panel_init(&lvds_connector->panel, &lvds_connector->base);
+	intel_panel_init(&lvds_connector->panel,
+			 &lvds_connector->base,
+			 fixed_mode);
 	drm_sysfs_connector_add(connector);
 	return true;
 
@@ -1004,6 +999,8 @@ failed:
 	DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
 	drm_connector_cleanup(connector);
 	drm_encoder_cleanup(encoder);
+	if (fixed_mode)
+		drm_mode_destroy(dev, fixed_mode);
 	kfree(lvds_encoder);
 	kfree(lvds_connector);
 	return false;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index d2dc066..b7329be 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -183,9 +183,12 @@ static int intel_panel_lid_notify(struct notifier_block *nb, unsigned long val,
 	return NOTIFY_OK;
 }
 
-int intel_panel_init(struct intel_panel *panel, struct intel_connector *connector)
+int intel_panel_init(struct intel_panel *panel,
+		     struct intel_connector *connector,
+		     struct drm_display_mode *fixed_mode)
 {
 	panel->connector = connector;
+	panel->fixed_mode = fixed_mode;
 
 	panel->lid_notifier.notifier_call = intel_panel_lid_notify;
 	if (acpi_lid_notifier_register(&panel->lid_notifier)) {
@@ -200,6 +203,10 @@ void intel_panel_fini(struct intel_panel *panel)
 {
 	if (panel->lid_notifier.notifier_call)
 		acpi_lid_notifier_unregister(&panel->lid_notifier);
+
+	if (panel->fixed_mode)
+		drm_mode_destroy(panel->connector->base.dev,
+				 panel->fixed_mode);
 }
 
 /* Panel backlight controls */
-- 
1.7.4.1

Re: [PATCH 07/10] drm/i915: Move the fixed_mode away from drm_i915_private and to the panel

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:15 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ... as matches usage.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    |    1 -
>  drivers/gpu/drm/i915/intel_dp.c    |   43 +++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h   |    4 ++-
>  drivers/gpu/drm/i915/intel_lvds.c  |   45 ++++++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_panel.c |    9 ++++++-
>  5 files changed, 54 insertions(+), 48 deletions(-)

Nice, dev_priv is getting way too bloated.

Acked-by: Jesse Barnes <jbarnes@virtuouseek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 08/10] drm/i915/dp: If the panel has a fixed_mode, it can only be eDP

[Chris Wilson]

... and so we can simplify some conditionals.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 384017c..4c42f39 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -205,7 +205,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
 	int max_link_clock = intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
 	int max_lanes = intel_dp_max_lane_count(intel_dp);
 
-	if (is_edp(intel_dp) && intel_dp->panel.fixed_mode) {
+	if (intel_dp->panel.fixed_mode) {
 		if (mode->hdisplay > intel_dp->panel.fixed_mode->hdisplay)
 			return MODE_PANEL;
 
@@ -596,7 +596,7 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
 
-	if (is_edp(intel_dp) && intel_dp->panel.fixed_mode) {
+	if (intel_dp->panel.fixed_mode) {
 		intel_fixed_panel_mode(intel_dp->panel.fixed_mode,
 				       adjusted_mode);
 		intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
@@ -1646,14 +1646,11 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 	}
 
 	/* if eDP has no EDID, try to use fixed panel mode from VBT */
-	if (is_edp(intel_dp)) {
-		if (intel_dp->panel.fixed_mode != NULL) {
-			struct drm_display_mode *mode;
-			mode = drm_mode_duplicate(dev,
-						  intel_dp->panel.fixed_mode);
-			drm_mode_probed_add(connector, mode);
-			return 1;
-		}
+	if (intel_dp->panel.fixed_mode != NULL) {
+		struct drm_display_mode *mode;
+		mode = drm_mode_duplicate(dev, intel_dp->panel.fixed_mode);
+		drm_mode_probed_add(connector, mode);
+		return 1;
 	}
 	return 0;
 }
-- 
1.7.4.1

Re: [PATCH 08/10] drm/i915/dp: If the panel has a fixed_mode, it can only be eDP

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:16 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ... and so we can simplify some conditionals.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   17 +++++++----------
>  1 files changed, 7 insertions(+), 10 deletions(-)

Only downside of this is that it makes the code a little less
readable.  The is_edp checks make it obvious that you're looking fro
eDP, while looking at the fixed mode is more implicit.

Maybe a new macro, is_panel() or something that does the check?

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 09/10] drm/i015: Share the EDID caching logic and mode query between LVDS and eDP

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c    |   54 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h   |    3 ++
 drivers/gpu/drm/i915/intel_lvds.c  |   33 ++++++---------------
 drivers/gpu/drm/i915/intel_panel.c |   19 ++++++++++++
 4 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4c42f39..31b0774 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1622,37 +1622,14 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 static int intel_dp_get_modes(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct drm_device *dev = intel_dp->base.base.dev;
-	int ret;
 
 	/* We should parse the EDID data and find out if it has an audio sink
 	 */
 
-	ret = intel_ddc_get_modes(connector, &intel_dp->adapter);
-	if (ret) {
-		if (is_edp(intel_dp) && !intel_dp->panel.fixed_mode) {
-			struct drm_display_mode *newmode;
-			list_for_each_entry(newmode, &connector->probed_modes,
-					    head) {
-				if (newmode->type & DRM_MODE_TYPE_PREFERRED) {
-					intel_dp->panel.fixed_mode =
-						drm_mode_duplicate(dev, newmode);
-					break;
-				}
-			}
-		}
+	if (intel_dp->panel.connector)
+		return intel_panel_get_modes(&intel_dp->panel);
 
-		return ret;
-	}
-
-	/* if eDP has no EDID, try to use fixed panel mode from VBT */
-	if (intel_dp->panel.fixed_mode != NULL) {
-		struct drm_display_mode *mode;
-		mode = drm_mode_duplicate(dev, intel_dp->panel.fixed_mode);
-		drm_mode_probed_add(connector, mode);
-		return 1;
-	}
-	return 0;
+	return intel_ddc_get_modes(connector, &intel_dp->adapter);
 }
 
 static bool
@@ -1967,9 +1944,30 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 
 	if (is_edp(intel_dp)) {
 		struct drm_display_mode *fixed_mode = NULL;
+		struct drm_display_mode *scan;
+		struct edid *edid;
+
+		edid = drm_get_edid(connector, &intel_dp->adapter);
+		if (edid) {
+			if (drm_add_edid_modes(connector, edid)) {
+				drm_mode_connector_update_edid_property(connector,
+									edid);
+			} else {
+				kfree(edid);
+				edid = NULL;
+			}
+		}
+
+		/* Downclock? */
+		list_for_each_entry(scan, &connector->probed_modes, head) {
+			if (scan->type & DRM_MODE_TYPE_PREFERRED) {
+				fixed_mode = drm_mode_duplicate(dev, scan);
+				break;
+			}
+		}
 
 		/* initialize panel mode from VBT if available for eDP */
-		if (dev_priv->lfp_lvds_vbt_mode) {
+		if (fixed_mode == NULL && dev_priv->lfp_lvds_vbt_mode) {
 			fixed_mode =
 				drm_mode_duplicate(dev,
 						   dev_priv->lfp_lvds_vbt_mode);
@@ -1979,7 +1977,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 
 		intel_panel_init(&intel_dp->panel,
 				 intel_connector,
-				 fixed_mode);
+				 edid, fixed_mode);
 	}
 
 	intel_dp_add_properties(intel_dp, connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 09bfc6b0..b4e9e54 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -259,13 +259,16 @@ extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
 /* intel_panel.c */
 struct intel_panel {
 	struct intel_connector *connector;
+	struct edid *edid;
 	struct drm_display_mode *fixed_mode;
 	struct notifier_block lid_notifier;
 };
 extern int intel_panel_init(struct intel_panel *panel,
 			    struct intel_connector *connector,
+			    struct edid *edid,
 			    struct drm_display_mode *fixed_mode);
 extern void intel_panel_fini(struct intel_panel *panel);
+extern int intel_panel_get_modes(struct intel_panel *panel);
 
 extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 				   struct drm_display_mode *adjusted_mode);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 68f03ea..98e75d1 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -43,8 +43,6 @@ struct intel_lvds_connector {
 	struct intel_connector base;
 	struct intel_panel panel;
 
-	struct edid *edid;
-
 	int fitting_mode;
 };
 
@@ -492,18 +490,7 @@ intel_lvds_connector_detect(struct drm_connector *connector, bool force)
 static int intel_lvds_connector_get_modes(struct drm_connector *connector)
 {
 	struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
-	struct drm_device *dev = connector->dev;
-	struct drm_display_mode *mode;
-
-	if (lvds_connector->edid)
-		return drm_add_edid_modes(connector, lvds_connector->edid);
-
-	mode = drm_mode_duplicate(dev, lvds_connector->panel.fixed_mode);
-	if (mode == NULL)
-		return 0;
-
-	drm_mode_probed_add(connector, mode);
-	return 1;
+	return intel_panel_get_modes(&lvds_connector->panel);
 }
 
 /**
@@ -787,6 +774,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	struct drm_display_mode *scan; /* *modes, *bios_mode; */
 	struct drm_display_mode *fixed_mode = NULL;
 	struct drm_crtc *crtc;
+	struct edid *edid;
 	u32 lvds;
 	int pipe;
 	u8 pin;
@@ -884,18 +872,17 @@ bool intel_lvds_init(struct drm_device *dev)
 	 * Attempt to get the fixed panel mode from DDC.  Assume that the
 	 * preferred mode is the right one.
 	 */
-	lvds_connector->edid = drm_get_edid(connector,
-					    &dev_priv->gmbus[pin].adapter);
-	if (lvds_connector->edid) {
-		if (drm_add_edid_modes(connector, lvds_connector->edid)) {
+	edid = drm_get_edid(connector, &dev_priv->gmbus[pin].adapter);
+	if (edid) {
+		if (drm_add_edid_modes(connector, edid)) {
 			drm_mode_connector_update_edid_property(connector,
-								lvds_connector->edid);
+								edid);
 		} else {
-			kfree(lvds_connector->edid);
-			lvds_connector->edid = NULL;
+			kfree(edid);
+			edid = NULL;
 		}
 	}
-	if (!lvds_connector->edid) {
+	if (!edid) {
 		/* Didn't get an EDID, so
 		 * Set wide sync ranges so we get all modes
 		 * handed to valid_mode for checking
@@ -991,7 +978,7 @@ out:
 
 	intel_panel_init(&lvds_connector->panel,
 			 &lvds_connector->base,
-			 fixed_mode);
+			 edid, fixed_mode);
 	drm_sysfs_connector_add(connector);
 	return true;
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index b7329be..b49aad7 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -185,9 +185,11 @@ static int intel_panel_lid_notify(struct notifier_block *nb, unsigned long val,
 
 int intel_panel_init(struct intel_panel *panel,
 		     struct intel_connector *connector,
+		     struct edid *edid,
 		     struct drm_display_mode *fixed_mode)
 {
 	panel->connector = connector;
+	panel->edid = edid;
 	panel->fixed_mode = fixed_mode;
 
 	panel->lid_notifier.notifier_call = intel_panel_lid_notify;
@@ -209,6 +211,23 @@ void intel_panel_fini(struct intel_panel *panel)
 				 panel->fixed_mode);
 }
 
+int intel_panel_get_modes(struct intel_panel *panel)
+{
+	struct drm_connector *connector = &panel->connector->base;
+	struct drm_device *dev = connector->dev;
+	struct drm_display_mode *mode;
+
+	if (panel->edid)
+		return drm_add_edid_modes(connector, panel->edid);
+
+	mode = drm_mode_duplicate(dev, panel->fixed_mode);
+	if (mode == NULL)
+		return 0;
+
+	drm_mode_probed_add(connector, mode);
+	return 1;
+}
+
 /* Panel backlight controls */
 
 static int is_backlight_combination_mode(struct drm_device *dev)
-- 
1.7.4.1

Re: [PATCH 09/10] drm/i015: Share the EDID caching logic and mode query between LVDS and eDP

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:17 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_dp.c    |   54 +++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h   |    3 ++
>  drivers/gpu/drm/i915/intel_lvds.c  |   33 ++++++---------------
>  drivers/gpu/drm/i915/intel_panel.c |   19 ++++++++++++
>  4 files changed, 58 insertions(+), 51 deletions(-)

Nice.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 10/10] drm/i915: Only force a modeset on a lid event if fbcon is active

[Chris Wilson]

... otherwise just left userspace handle the notification and change
modes as it desires.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_panel.c |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index b49aad7..e930397 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -138,6 +138,24 @@ static const struct dmi_system_id intel_no_modeset_on_lid[] = {
 	{ }	/* terminating entry */
 };
 
+/* See if any output is currently scanning from the fbcon */
+static bool fbdev_is_active(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_framebuffer *fb;
+	struct drm_crtc *crtc;
+
+	if (!dev_priv->fbdev)
+		return false;
+
+	fb = &dev_priv->fbdev->ifb.base;
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+		if (crtc->fb == fb)
+			return true;
+
+	return false;
+}
+
 /*
  * Lid events. Note the use of 'modeset_on_lid':
  *  - we set it on lid close, and reset it on open
@@ -177,7 +195,8 @@ static int intel_panel_lid_notify(struct notifier_block *nb, unsigned long val,
 	dev_priv->modeset_on_lid = 0;
 
 	mutex_lock(&dev->mode_config.mutex);
-	drm_helper_resume_force_mode(dev);
+	if (fbdev_is_active(dev))
+		drm_helper_resume_force_mode(dev);
 	mutex_unlock(&dev->mode_config.mutex);
 
 	return NOTIFY_OK;
-- 
1.7.4.1

Re: [PATCH 10/10] drm/i915: Only force a modeset on a lid event if fbcon is active

[Jesse Barnes]

On Fri, 22 Apr 2011 10:19:18 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ... otherwise just left userspace handle the notification and change
> modes as it desires.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Problem is that old userspace didn't do anything.  The change is a nice
one though; maybe a setparam is needed to flag the new behavior?

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring

[Jani Nikula]

Hi all, this is some LVDS/eDP panel refactoring and cleanup based on
Chris Wilson's earlier work and ideas last year [1]. I left out eDP lid
notifier based on Jesse's comments [2]; more of Jesse's old review is at
[3].

The main high level difference to Chris' patches is that intel_panel and
the cached EDID are placed in intel_connector rather than anywhere
else. This is because Daniel hinted that we might want to do some more
generic EDID caching (indeed, why keep reading the EDID if the display
has not been unplugged?). With the EDID and intel_panel in
intel_connector, we can add functions to query EDID and modes with
transparent caching and fallback to fixed mode when EDID is not
available.


BR,
Jani.


[1] http://cgit.freedesktop.org/~ickle/linux-2.6/log/?h=panel-refactor
[2] http://lists.freedesktop.org/archives/intel-gfx/2011-April/010419.html
[3] http://lists.freedesktop.org/archives/intel-gfx/2011-April/thread.html#10401

Jani Nikula (10):
  drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder
  drm/i915/lvds: Introduce intel_lvds_connector
  drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to
    the connector
  drm/i915: Backlight setup requires connector so pass it as parameter
  drm/i915/lvds: Move some connector specific info across from the
    encoder
  drm/i915/dp: Initialize eDP fixed mode in intel_dp_init
  drm/i915: Create generic intel_panel for LVDS and eDP
  drm/i915: Move the fixed mode to intel_panel
  drm/i915: Do not free the passed EDID in
    intel_connector_update_modes()
  drm/i915: Move cached EDID to intel_connector

 drivers/gpu/drm/i915/i915_drv.h    |    4 -
 drivers/gpu/drm/i915/intel_crt.c   |    6 +-
 drivers/gpu/drm/i915/intel_dp.c    |  144 +++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h   |   20 +++-
 drivers/gpu/drm/i915/intel_lvds.c  |  211 ++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_modes.c |    7 +-
 drivers/gpu/drm/i915/intel_panel.c |   32 ++++--
 7 files changed, 241 insertions(+), 183 deletions(-)

-- 
1.7.9.5

[PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector

[Jani Nikula]

Introduce a local structure to move LVDS specific information away from the
drm_i915_private and onto the LVDS connector.

Based on earlier work by Chris Wilson <chris@chris-wilson.co.uk>

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 61bdf4c..4a2f6fa 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -41,6 +41,10 @@
 #include <linux/acpi.h>
 
 /* Private structure for the integrated LVDS support */
+struct intel_lvds_connector {
+	struct intel_connector base;
+};
+
 struct intel_lvds_encoder {
 	struct intel_encoder base;
 
@@ -59,6 +63,11 @@ static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_lvds_encoder, base.base);
 }
 
+static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *connector)
+{
+	return container_of(connector, struct intel_lvds_connector, base.base);
+}
+
 static struct intel_lvds_encoder *intel_attached_lvds(struct drm_connector *connector)
 {
 	return container_of(intel_attached_encoder(connector),
@@ -907,6 +916,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds_encoder *lvds_encoder;
 	struct intel_encoder *intel_encoder;
+	struct intel_lvds_connector *lvds_connector;
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
@@ -942,8 +952,8 @@ bool intel_lvds_init(struct drm_device *dev)
 	if (!lvds_encoder)
 		return false;
 
-	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
-	if (!intel_connector) {
+	lvds_connector = kzalloc(sizeof(struct intel_lvds_connector), GFP_KERNEL);
+	if (!lvds_connector) {
 		kfree(lvds_encoder);
 		return false;
 	}
@@ -954,6 +964,7 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	intel_encoder = &lvds_encoder->base;
 	encoder = &intel_encoder->base;
+	intel_connector = &lvds_connector->base;
 	connector = &intel_connector->base;
 	drm_connector_init(dev, &intel_connector->base, &intel_lvds_connector_funcs,
 			   DRM_MODE_CONNECTOR_LVDS);
@@ -1104,6 +1115,6 @@ failed:
 	drm_connector_cleanup(connector);
 	drm_encoder_cleanup(encoder);
 	kfree(lvds_encoder);
-	kfree(intel_connector);
+	kfree(lvds_connector);
 	return false;
 }
-- 
1.7.9.5

Re: [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector

[Jesse Barnes]

On Fri, 19 Oct 2012 14:51:44 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> Introduce a local structure to move LVDS specific information away from the
> drm_i915_private and onto the LVDS connector.
> 
> Based on earlier work by Chris Wilson <chris@chris-wilson.co.uk>
> 
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 61bdf4c..4a2f6fa 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -41,6 +41,10 @@
>  #include <linux/acpi.h>
>  
>  /* Private structure for the integrated LVDS support */
> +struct intel_lvds_connector {
> +	struct intel_connector base;
> +};
> +
>  struct intel_lvds_encoder {
>  	struct intel_encoder base;
>  
> @@ -59,6 +63,11 @@ static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
>  	return container_of(encoder, struct intel_lvds_encoder, base.base);
>  }
>  
> +static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct intel_lvds_connector, base.base);
> +}
> +
>  static struct intel_lvds_encoder *intel_attached_lvds(struct drm_connector *connector)
>  {
>  	return container_of(intel_attached_encoder(connector),
> @@ -907,6 +916,7 @@ bool intel_lvds_init(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_lvds_encoder *lvds_encoder;
>  	struct intel_encoder *intel_encoder;
> +	struct intel_lvds_connector *lvds_connector;
>  	struct intel_connector *intel_connector;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> @@ -942,8 +952,8 @@ bool intel_lvds_init(struct drm_device *dev)
>  	if (!lvds_encoder)
>  		return false;
>  
> -	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
> -	if (!intel_connector) {
> +	lvds_connector = kzalloc(sizeof(struct intel_lvds_connector), GFP_KERNEL);
> +	if (!lvds_connector) {
>  		kfree(lvds_encoder);
>  		return false;
>  	}
> @@ -954,6 +964,7 @@ bool intel_lvds_init(struct drm_device *dev)
>  
>  	intel_encoder = &lvds_encoder->base;
>  	encoder = &intel_encoder->base;
> +	intel_connector = &lvds_connector->base;
>  	connector = &intel_connector->base;
>  	drm_connector_init(dev, &intel_connector->base, &intel_lvds_connector_funcs,
>  			   DRM_MODE_CONNECTOR_LVDS);
> @@ -1104,6 +1115,6 @@ failed:
>  	drm_connector_cleanup(connector);
>  	drm_encoder_cleanup(encoder);
>  	kfree(lvds_encoder);
> -	kfree(intel_connector);
> +	kfree(lvds_connector);
>  	return false;
>  }

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

-- 
Jesse Barnes, Intel Open Source Technology Center