All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring
@ 2012-10-19 11:51 Jani Nikula
  2012-10-19 11:51 ` [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder Jani Nikula
                   ` (10 more replies)
  0 siblings, 11 replies; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 16:46   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector Jani Nikula
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

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.

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 |  129 ++++++++++++++++++-------------------
 1 file changed, 61 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 40d72bd..61bdf4c 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,15 +54,15 @@ 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);
 }
 
 static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
@@ -97,7 +97,7 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 static void intel_enable_lvds(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
-	struct intel_lvds *intel_lvds = to_intel_lvds(&encoder->base);
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg, stat_reg;
@@ -114,7 +114,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
 
 	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
@@ -122,12 +122,12 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
 		 * 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);
 
-		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;
 	}
 
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
@@ -141,7 +141,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
 static void intel_disable_lvds(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
-	struct intel_lvds *intel_lvds = to_intel_lvds(&encoder->base);
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg, stat_reg;
 
@@ -161,9 +161,9 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
 		DRM_ERROR("timed out waiting for panel to power off\n");
 
-	if (intel_lvds->pfit_control) {
+	if (lvds_encoder->pfit_control) {
 		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);
@@ -173,8 +173,8 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 static int intel_lvds_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;
@@ -250,8 +250,8 @@ static bool intel_lvds_mode_fixup(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_crtc *intel_crtc = intel_lvds->base.new_crtc;
+	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
+	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 	int pipe;
 
@@ -261,7 +261,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 		return false;
 	}
 
-	if (intel_encoder_check_is_cloned(&intel_lvds->base))
+	if (intel_encoder_check_is_cloned(&lvds_encoder->base))
 		return false;
 
 	/*
@@ -270,10 +270,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;
 	}
@@ -299,7 +299,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 
 	drm_mode_set_crtcinfo(adjusted_mode, 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 &
@@ -397,11 +397,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;
 
@@ -450,14 +450,14 @@ intel_lvds_detect(struct drm_connector *connector, bool force)
  */
 static int intel_lvds_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;
 
@@ -558,22 +558,22 @@ static int intel_lvds_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
@@ -905,7 +905,7 @@ static bool intel_lvds_supported(struct drm_device *dev)
 bool intel_lvds_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_lvds *intel_lvds;
+	struct intel_lvds_encoder *lvds_encoder;
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
@@ -938,22 +938,21 @@ 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;
+	intel_encoder = &lvds_encoder->base;
 	encoder = &intel_encoder->base;
 	connector = &intel_connector->base;
 	drm_connector_init(dev, &intel_connector->base, &intel_lvds_connector_funcs,
@@ -993,7 +992,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:
 	 * 1) check for EDID on DDC
@@ -1008,20 +1007,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.
 	 */
-	intel_lvds->edid = drm_get_edid(connector,
-					intel_gmbus_get_adapter(dev_priv,
-								pin));
-	if (intel_lvds->edid) {
-		if (drm_add_edid_modes(connector,
-				       intel_lvds->edid)) {
+	lvds_encoder->edid = drm_get_edid(connector,
+					  intel_gmbus_get_adapter(dev_priv, pin));
+	if (lvds_encoder->edid) {
+		if (drm_add_edid_modes(connector, 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
@@ -1034,10 +1031,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) {
-			intel_lvds->fixed_mode =
-				drm_mode_duplicate(dev, scan);
-			intel_find_lvds_downclock(dev,
-						  intel_lvds->fixed_mode,
+			lvds_encoder->fixed_mode = drm_mode_duplicate(dev, scan);
+			intel_find_lvds_downclock(dev, lvds_encoder->fixed_mode,
 						  connector);
 			goto out;
 		}
@@ -1045,11 +1040,10 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	/* Failed to get EDID, 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 |=
-				DRM_MODE_TYPE_PREFERRED;
+		if (lvds_encoder->fixed_mode) {
+			lvds_encoder->fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
 	}
@@ -1069,16 +1063,15 @@ 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 |=
-				DRM_MODE_TYPE_PREFERRED;
+		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:
@@ -1110,7 +1103,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.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
  2012-10-19 11:51 ` [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 16:47   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector Jani Nikula
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: 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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
  2012-10-19 11:51 ` [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder Jani Nikula
  2012-10-19 11:51 ` [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 16:48   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 04/10] drm/i915: Backlight setup requires connector so pass it as parameter Jani Nikula
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

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/i915_drv.h   |    2 --
 drivers/gpu/drm/i915/intel_lvds.c |   32 +++++++++++++++++---------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4728d30..f63ae1c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -507,8 +507,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[I915_MAX_NUM_FENCES]; /* assume 965 */
 	int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 4a2f6fa..f3edbf6 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -43,6 +43,8 @@
 /* Private structure for the integrated LVDS support */
 struct intel_lvds_connector {
 	struct intel_connector base;
+
+	struct notifier_block lid_notifier;
 };
 
 struct intel_lvds_encoder {
@@ -506,10 +508,11 @@ 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_connector *connector = &lvds_connector->base.base;
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (dev->switch_power_state != DRM_SWITCH_POWER_ON)
 		return NOTIFY_OK;
@@ -518,9 +521,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
 	 * check and update the status of LVDS connector after receiving
 	 * the LID nofication event.
 	 */
-	if (connector)
-		connector->status = connector->funcs->detect(connector,
-							     false);
+	connector->status = connector->funcs->detect(connector, false);
 
 	/* Don't force modeset on machines where it causes a GPU lockup */
 	if (dmi_check_system(intel_no_modeset_on_lid))
@@ -551,13 +552,14 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
  */
 static void intel_lvds_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);
 
-	intel_panel_destroy_backlight(dev);
+	intel_panel_destroy_backlight(connector->dev);
 
-	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);
@@ -1097,10 +1099,10 @@ out:
 		I915_WRITE(PP_CONTROL,
 			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
 	}
-	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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 04/10] drm/i915: Backlight setup requires connector so pass it as parameter
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (2 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 16:54   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 05/10] drm/i915/lvds: Move some connector specific info across from the encoder Jani Nikula
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

Get rid of saved int_lvds_connector and int_edp_connector in
drm_i915_private.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    |    2 --
 drivers/gpu/drm/i915/intel_dp.c    |    6 ++----
 drivers/gpu/drm/i915/intel_drv.h   |    2 +-
 drivers/gpu/drm/i915/intel_lvds.c  |    4 +---
 drivers/gpu/drm/i915/intel_panel.c |   15 ++++-----------
 5 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f63ae1c..79dd42a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -826,8 +826,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;
-	struct drm_connector *int_edp_connector;
 
 	bool mchbar_need_disable;
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 697b176..09244f3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2836,10 +2836,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 	intel_encoder->hot_plug = intel_dp_hot_plug;
 
-	if (is_edp(intel_dp)) {
-		dev_priv->int_edp_connector = connector;
-		intel_panel_setup_backlight(dev);
-	}
+	if (is_edp(intel_dp))
+		intel_panel_setup_backlight(connector);
 
 	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 95cbd67..e5cdf3d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -444,7 +444,7 @@ extern void intel_pch_panel_fitting(struct drm_device *dev,
 				    struct drm_display_mode *adjusted_mode);
 extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
 extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
-extern int intel_panel_setup_backlight(struct drm_device *dev);
+extern int intel_panel_setup_backlight(struct drm_connector *connector);
 extern void intel_panel_enable_backlight(struct drm_device *dev,
 					 enum pipe pipe);
 extern void intel_panel_disable_backlight(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index f3edbf6..cdee7bf 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1104,11 +1104,9 @@ out:
 		DRM_DEBUG_KMS("lid notifier registration failed\n");
 		lvds_connector->lid_notifier.notifier_call = NULL;
 	}
-	/* keep the LVDS connector */
-	dev_priv->int_lvds_connector = connector;
 	drm_sysfs_connector_add(connector);
 
-	intel_panel_setup_backlight(dev);
+	intel_panel_setup_backlight(connector);
 
 	return true;
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index e019b23..d9752a3 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -416,21 +416,14 @@ static const struct backlight_ops intel_panel_bl_ops = {
 	.get_brightness = intel_panel_get_brightness,
 };
 
-int intel_panel_setup_backlight(struct drm_device *dev)
+int intel_panel_setup_backlight(struct drm_connector *connector)
 {
+	struct drm_device *dev = connector->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct backlight_properties props;
-	struct drm_connector *connector;
 
 	intel_panel_init_backlight(dev);
 
-	if (dev_priv->int_lvds_connector)
-		connector = dev_priv->int_lvds_connector;
-	else if (dev_priv->int_edp_connector)
-		connector = dev_priv->int_edp_connector;
-	else
-		return -ENODEV;
-
 	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = _intel_panel_get_max_backlight(dev);
@@ -460,9 +453,9 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
 		backlight_device_unregister(dev_priv->backlight);
 }
 #else
-int intel_panel_setup_backlight(struct drm_device *dev)
+int intel_panel_setup_backlight(struct drm_connector *connector)
 {
-	intel_panel_init_backlight(dev);
+	intel_panel_init_backlight(connector->dev);
 	return 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 05/10] drm/i915/lvds: Move some connector specific info across from the encoder
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (3 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 04/10] drm/i915: Backlight setup requires connector so pass it as parameter Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 16:55   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 06/10] drm/i915/dp: Initialize eDP fixed mode in intel_dp_init Jani Nikula
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

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.

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 |   83 +++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index cdee7bf..0ed2c3b 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -45,19 +45,19 @@ 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;
 
-	struct edid *edid;
-
-	int fitting_mode;
 	u32 pfit_control;
 	u32 pfit_pgm_ratios;
 	bool pfit_dirty;
 
-	struct drm_display_mode *fixed_mode;
+	struct intel_lvds_connector *attached_connector;
 };
 
 static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
@@ -70,12 +70,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);
-}
-
 static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 				    enum pipe *pipe)
 {
@@ -184,8 +178,8 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 static int intel_lvds_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;
@@ -262,6 +256,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
+	struct intel_lvds_connector *lvds_connector =
+		lvds_encoder->attached_connector;
 	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 	int pipe;
@@ -281,10 +277,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(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;
 	}
@@ -310,7 +306,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 
 	drm_mode_set_crtcinfo(adjusted_mode, 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 &
@@ -461,14 +457,14 @@ intel_lvds_detect(struct drm_connector *connector, bool force)
  */
 static int intel_lvds_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;
 
@@ -569,22 +565,24 @@ static int intel_lvds_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
@@ -960,6 +958,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);
 	}
@@ -1005,7 +1005,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);
-	lvds_encoder->fitting_mode = DRM_MODE_SCALE_ASPECT;
+	lvds_connector->fitting_mode = DRM_MODE_SCALE_ASPECT;
 	/*
 	 * LVDS discovery:
 	 * 1) check for EDID on DDC
@@ -1020,18 +1020,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,
-					  intel_gmbus_get_adapter(dev_priv, pin));
-	if (lvds_encoder->edid) {
-		if (drm_add_edid_modes(connector, lvds_encoder->edid)) {
+	lvds_connector->edid = drm_get_edid(connector,
+					    intel_gmbus_get_adapter(dev_priv, pin));
+	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
@@ -1044,8 +1044,9 @@ 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 = drm_mode_duplicate(dev, scan);
-			intel_find_lvds_downclock(dev, lvds_encoder->fixed_mode,
+			lvds_connector->fixed_mode = drm_mode_duplicate(dev, scan);
+			intel_find_lvds_downclock(dev,
+						  lvds_connector->fixed_mode,
 						  connector);
 			goto out;
 		}
@@ -1053,10 +1054,10 @@ bool intel_lvds_init(struct drm_device *dev)
 
 	/* Failed to get EDID, 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 |= DRM_MODE_TYPE_PREFERRED;
+		if (lvds_connector->fixed_mode) {
+			lvds_connector->fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 			goto out;
 		}
 	}
@@ -1076,15 +1077,15 @@ 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 |= DRM_MODE_TYPE_PREFERRED;
+		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.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 06/10] drm/i915/dp: Initialize eDP fixed mode in intel_dp_init
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (4 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 05/10] drm/i915/lvds: Move some connector specific info across from the encoder Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 17:00   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 07/10] drm/i915: Create generic intel_panel for LVDS and eDP Jani Nikula
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

Since we do EDID caching in intel_dp_init, we can do the fixed mode
initialization there too. This should not change the functionality apart
from initializing fixed mode earlier. Particularly retain the behaviour of
only falling back to VBT if EDID is not available to not regress

commit 47f0eb2234a2a1c790825393bbaccfadf82463d3
Author: Keith Packard <keithp@keithp.com>
Date:   Mon Sep 19 14:33:26 2011 -0700

    drm/i915: Only use VBT panel mode on eDP if no EDID is found

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   53 ++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 09244f3..52cbee7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2407,42 +2407,20 @@ 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
 	 */
 
 	ret = intel_dp_get_edid_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 (ret)
 		return ret;
-	}
 
-	/* if eDP has no EDID, try to use fixed panel mode from VBT */
-	if (is_edp(intel_dp)) {
-		/* initialize panel mode from VBT if available for eDP */
-		if (intel_dp->panel_fixed_mode == NULL && dev_priv->lfp_lvds_vbt_mode != NULL) {
-			intel_dp->panel_fixed_mode =
-				drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
-			if (intel_dp->panel_fixed_mode) {
-				intel_dp->panel_fixed_mode->type |=
-					DRM_MODE_TYPE_PREFERRED;
-			}
-		}
-		if (intel_dp->panel_fixed_mode) {
-			struct drm_display_mode *mode;
-			mode = drm_mode_duplicate(dev, intel_dp->panel_fixed_mode);
+	/* if eDP has no EDID, fall back to fixed mode */
+	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
+		struct drm_display_mode *mode;
+		mode = drm_mode_duplicate(dev, intel_dp->panel_fixed_mode);
+		if (mode) {
 			drm_mode_probed_add(connector, mode);
 			return 1;
 		}
@@ -2638,6 +2616,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	struct intel_dp *intel_dp;
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
+	struct drm_display_mode *fixed_mode = NULL;
 	const char *name = NULL;
 	int type;
 
@@ -2802,6 +2781,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 	if (is_edp(intel_dp)) {
 		bool ret;
+		struct drm_display_mode *scan;
 		struct edid *edid;
 
 		ironlake_edp_panel_vdd_on(intel_dp);
@@ -2831,6 +2811,23 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 			drm_edid_to_eld(connector, edid);
 			intel_dp->edid = edid;
 		}
+
+		/* prefer fixed mode from EDID if available */
+		list_for_each_entry(scan, &connector->probed_modes, head) {
+			if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
+				fixed_mode = drm_mode_duplicate(dev, scan);
+				break;
+			}
+		}
+
+		/* fallback to VBT if available for eDP */
+		if (!fixed_mode && dev_priv->lfp_lvds_vbt_mode) {
+			fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
+			if (fixed_mode)
+				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
+		}
+		intel_dp->panel_fixed_mode = fixed_mode;
+
 		ironlake_edp_panel_vdd_off(intel_dp, false);
 	}
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 07/10] drm/i915: Create generic intel_panel for LVDS and eDP
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (5 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 06/10] drm/i915/dp: Initialize eDP fixed mode in intel_dp_init Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 17:04   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 08/10] drm/i915: Move the fixed mode to intel_panel Jani Nikula
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

Create a generic struct intel_panel for sharing a data structure and code
between eDP and LVDS panels. Add the new struct to intel_connector so that
later on we can have generic EDID and mode reading functions with EDID
caching that transparently fallback to fixed mode when EDID is not
available.

Add intel_panel as a dummy first, and move data (such as the mentioned
fixed mode) to it in later patches.

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_dp.c    |    9 +++++++--
 drivers/gpu/drm/i915/intel_drv.h   |    9 +++++++++
 drivers/gpu/drm/i915/intel_lvds.c  |    2 ++
 drivers/gpu/drm/i915/intel_panel.c |    9 +++++++++
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 52cbee7..abbdfed 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2502,9 +2502,12 @@ static void
 intel_dp_destroy(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
 
-	if (intel_dpd_is_edp(dev))
+	if (intel_dpd_is_edp(dev)) {
 		intel_panel_destroy_backlight(dev);
+		intel_panel_fini(&intel_connector->panel);
+	}
 
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
@@ -2833,8 +2836,10 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 	intel_encoder->hot_plug = intel_dp_hot_plug;
 
-	if (is_edp(intel_dp))
+	if (is_edp(intel_dp)) {
+		intel_panel_init(&intel_connector->panel);
 		intel_panel_setup_backlight(connector);
+	}
 
 	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 e5cdf3d..df00f21 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -163,6 +163,9 @@ struct intel_encoder {
 	int crtc_mask;
 };
 
+struct intel_panel {
+};
+
 struct intel_connector {
 	struct drm_connector base;
 	/*
@@ -179,6 +182,9 @@ struct intel_connector {
 	/* Reads out the current hw, returning true if the connector is enabled
 	 * and active (i.e. dpms ON state). */
 	bool (*get_hw_state)(struct intel_connector *);
+
+	/* Panel info for eDP and LVDS */
+	struct intel_panel panel;
 };
 
 struct intel_crtc {
@@ -436,6 +442,9 @@ extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
 				      enum plane plane);
 
 /* intel_panel.c */
+extern int intel_panel_init(struct intel_panel *panel);
+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 0ed2c3b..db1b1b7 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -555,6 +555,7 @@ static void intel_lvds_destroy(struct drm_connector *connector)
 		acpi_lid_notifier_unregister(&lvds_connector->lid_notifier);
 
 	intel_panel_destroy_backlight(connector->dev);
+	intel_panel_fini(&lvds_connector->base.panel);
 
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
@@ -1107,6 +1108,7 @@ out:
 	}
 	drm_sysfs_connector_add(connector);
 
+	intel_panel_init(&intel_connector->panel);
 	intel_panel_setup_backlight(connector);
 
 	return true;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index d9752a3..4c64ebc 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -464,3 +464,12 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
 	return;
 }
 #endif
+
+int intel_panel_init(struct intel_panel *panel)
+{
+	return 0;
+}
+
+void intel_panel_fini(struct intel_panel *panel)
+{
+}
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 08/10] drm/i915: Move the fixed mode to intel_panel
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (6 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 07/10] drm/i915: Create generic intel_panel for LVDS and eDP Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 17:09   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 09/10] drm/i915: Do not free the passed EDID in intel_connector_update_modes() Jani Nikula
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

Pave the way for sharing some logic between eDP and LVDS.

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_dp.c    |   29 ++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h   |    6 ++++--
 drivers/gpu/drm/i915/intel_lvds.c  |   36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_panel.c |   10 +++++++++-
 4 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index abbdfed..8c72477 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -122,9 +122,10 @@ intel_edp_target_clock(struct intel_encoder *intel_encoder,
 		       struct drm_display_mode *mode)
 {
 	struct intel_dp *intel_dp = container_of(intel_encoder, struct intel_dp, base);
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
 
-	if (intel_dp->panel_fixed_mode)
-		return intel_dp->panel_fixed_mode->clock;
+	if (intel_connector->panel.fixed_mode)
+		return intel_connector->panel.fixed_mode->clock;
 	else
 		return mode->clock;
 }
@@ -228,12 +229,14 @@ intel_dp_mode_valid(struct drm_connector *connector,
 		    struct drm_display_mode *mode)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
 
-	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
-		if (mode->hdisplay > intel_dp->panel_fixed_mode->hdisplay)
+	if (is_edp(intel_dp) && fixed_mode) {
+		if (mode->hdisplay > fixed_mode->hdisplay)
 			return MODE_PANEL;
 
-		if (mode->vdisplay > intel_dp->panel_fixed_mode->vdisplay)
+		if (mode->vdisplay > fixed_mode->vdisplay)
 			return MODE_PANEL;
 	}
 
@@ -700,14 +703,16 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
 {
 	struct drm_device *dev = encoder->dev;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	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;
 	int bpp, mode_rate;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
 
-	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
-		intel_fixed_panel_mode(intel_dp->panel_fixed_mode, adjusted_mode);
+	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
+		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
+				       adjusted_mode);
 		intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
 					mode, adjusted_mode);
 	}
@@ -2406,6 +2411,7 @@ 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 intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_device *dev = intel_dp->base.base.dev;
 	int ret;
 
@@ -2417,9 +2423,10 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 		return ret;
 
 	/* if eDP has no EDID, fall back to fixed mode */
-	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
+	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
 		struct drm_display_mode *mode;
-		mode = drm_mode_duplicate(dev, intel_dp->panel_fixed_mode);
+		mode = drm_mode_duplicate(dev,
+					  intel_connector->panel.fixed_mode);
 		if (mode) {
 			drm_mode_probed_add(connector, mode);
 			return 1;
@@ -2638,6 +2645,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 		return;
 	}
 	intel_encoder = &intel_dp->base;
+	intel_dp->attached_connector = intel_connector;
 
 	if (HAS_PCH_SPLIT(dev) && output_reg == PCH_DP_D)
 		if (intel_dpd_is_edp(dev))
@@ -2829,7 +2837,6 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 			if (fixed_mode)
 				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 		}
-		intel_dp->panel_fixed_mode = fixed_mode;
 
 		ironlake_edp_panel_vdd_off(intel_dp, false);
 	}
@@ -2837,7 +2844,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->hot_plug = intel_dp_hot_plug;
 
 	if (is_edp(intel_dp)) {
-		intel_panel_init(&intel_connector->panel);
+		intel_panel_init(&intel_connector->panel, fixed_mode);
 		intel_panel_setup_backlight(connector);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index df00f21..ae0467a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -164,6 +164,7 @@ struct intel_encoder {
 };
 
 struct intel_panel {
+	struct drm_display_mode *fixed_mode;
 };
 
 struct intel_connector {
@@ -364,11 +365,11 @@ struct intel_dp {
 	int panel_power_cycle_delay;
 	int backlight_on_delay;
 	int backlight_off_delay;
-	struct drm_display_mode *panel_fixed_mode;  /* for eDP */
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
 	struct edid *edid; /* cached EDID for eDP */
 	int edid_mode_count;
+	struct intel_connector *attached_connector;
 };
 
 static inline struct drm_crtc *
@@ -442,7 +443,8 @@ extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
 				      enum plane plane);
 
 /* intel_panel.c */
-extern int intel_panel_init(struct intel_panel *panel);
+extern int intel_panel_init(struct intel_panel *panel,
+			    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 db1b1b7..ced6947 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -45,7 +45,6 @@ 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;
 };
@@ -178,8 +177,8 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 static int intel_lvds_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 intel_connector *intel_connector = to_intel_connector(connector);
+	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
 
 	if (mode->hdisplay > fixed_mode->hdisplay)
 		return MODE_PANEL;
@@ -277,7 +276,8 @@ 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(lvds_connector->fixed_mode, adjusted_mode);
+	intel_fixed_panel_mode(lvds_connector->base.panel.fixed_mode,
+			       adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
 		intel_pch_panel_fitting(dev, lvds_connector->fitting_mode,
@@ -464,7 +464,7 @@ static int intel_lvds_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->base.panel.fixed_mode);
 	if (mode == NULL)
 		return 0;
 
@@ -922,6 +922,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;
@@ -1045,20 +1046,17 @@ 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;
 		}
 	}
 
 	/* Failed to get EDID, 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;
 		}
 	}
@@ -1078,15 +1076,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:
@@ -1108,7 +1106,7 @@ out:
 	}
 	drm_sysfs_connector_add(connector);
 
-	intel_panel_init(&intel_connector->panel);
+	intel_panel_init(&intel_connector->panel, fixed_mode);
 	intel_panel_setup_backlight(connector);
 
 	return true;
@@ -1117,6 +1115,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 4c64ebc..e91a0bb 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -465,11 +465,19 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
 }
 #endif
 
-int intel_panel_init(struct intel_panel *panel)
+int intel_panel_init(struct intel_panel *panel,
+		     struct drm_display_mode *fixed_mode)
 {
+	panel->fixed_mode = fixed_mode;
+
 	return 0;
 }
 
 void intel_panel_fini(struct intel_panel *panel)
 {
+	struct intel_connector *intel_connector =
+		container_of(panel, struct intel_connector, panel);
+
+	if (panel->fixed_mode)
+		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 09/10] drm/i915: Do not free the passed EDID in intel_connector_update_modes()
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (7 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 08/10] drm/i915: Move the fixed mode to intel_panel Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 17:10   ` Jesse Barnes
  2012-10-19 11:51 ` [PATCH 10/10] drm/i915: Move cached EDID to intel_connector Jani Nikula
  2012-10-19 17:58 ` [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Chris Wilson
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

The caller, not intel_connector_update_modes(), should free the edid. This
improves the reusability of intel_connector_update_modes().

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c   |    6 +++++-
 drivers/gpu/drm/i915/intel_modes.c |    7 +++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 53f3e87..35c92d5 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -419,12 +419,16 @@ static int intel_crt_ddc_get_modes(struct drm_connector *connector,
 				struct i2c_adapter *adapter)
 {
 	struct edid *edid;
+	int ret;
 
 	edid = intel_crt_get_edid(connector, adapter);
 	if (!edid)
 		return 0;
 
-	return intel_connector_update_modes(connector, edid);
+	ret = intel_connector_update_modes(connector, edid);
+	kfree(edid);
+
+	return ret;
 }
 
 static bool intel_crt_detect_ddc(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 1773fb8..b57dbbe 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -44,7 +44,6 @@ int intel_connector_update_modes(struct drm_connector *connector,
 	drm_mode_connector_update_edid_property(connector, edid);
 	ret = drm_add_edid_modes(connector, edid);
 	drm_edid_to_eld(connector, edid);
-	kfree(edid);
 
 	return ret;
 }
@@ -60,12 +59,16 @@ int intel_ddc_get_modes(struct drm_connector *connector,
 			struct i2c_adapter *adapter)
 {
 	struct edid *edid;
+	int ret;
 
 	edid = drm_get_edid(connector, adapter);
 	if (!edid)
 		return 0;
 
-	return intel_connector_update_modes(connector, edid);
+	ret = intel_connector_update_modes(connector, edid);
+	kfree(edid);
+
+	return ret;
 }
 
 static const struct drm_prop_enum_list force_audio_names[] = {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 10/10] drm/i915: Move cached EDID to intel_connector
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (8 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 09/10] drm/i915: Do not free the passed EDID in intel_connector_update_modes() Jani Nikula
@ 2012-10-19 11:51 ` Jani Nikula
  2012-10-19 17:13   ` Jesse Barnes
  2012-10-19 17:58 ` [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Chris Wilson
  10 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2012-10-19 11:51 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

Move the cached EDID from intel_dp and intel_lvds_connector to
intel_connector. Unify cached EDID handling for LVDS and eDP, in
preparation for adding more generic EDID caching later.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |   61 +++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h  |    5 +--
 drivers/gpu/drm/i915/intel_lvds.c |   34 ++++++++++++++-------
 3 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8c72477..5b0e4cd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2323,44 +2323,45 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 static struct edid *
 intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 {
-	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct edid	*edid;
-	int size;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
 
-	if (is_edp(intel_dp)) {
-		if (!intel_dp->edid)
+	/* use cached edid if we have one */
+	if (intel_connector->edid) {
+		struct edid *edid;
+		int size;
+
+		/* invalid edid */
+		if (IS_ERR(intel_connector->edid))
 			return NULL;
 
-		size = (intel_dp->edid->extensions + 1) * EDID_LENGTH;
+		size = (intel_connector->edid->extensions + 1) * EDID_LENGTH;
 		edid = kmalloc(size, GFP_KERNEL);
 		if (!edid)
 			return NULL;
 
-		memcpy(edid, intel_dp->edid, size);
+		memcpy(edid, intel_connector->edid, size);
 		return edid;
 	}
 
-	edid = drm_get_edid(connector, adapter);
-	return edid;
+	return drm_get_edid(connector, adapter);
 }
 
 static int
 intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter)
 {
-	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	int	ret;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
 
-	if (is_edp(intel_dp)) {
-		drm_mode_connector_update_edid_property(connector,
-							intel_dp->edid);
-		ret = drm_add_edid_modes(connector, intel_dp->edid);
-		drm_edid_to_eld(connector,
-				intel_dp->edid);
-		return intel_dp->edid_mode_count;
+	/* use cached edid if we have one */
+	if (intel_connector->edid) {
+		/* invalid edid */
+		if (IS_ERR(intel_connector->edid))
+			return 0;
+
+		return intel_connector_update_modes(connector,
+						    intel_connector->edid);
 	}
 
-	ret = intel_ddc_get_modes(connector, adapter);
-	return ret;
+	return intel_ddc_get_modes(connector, adapter);
 }
 
 
@@ -2511,6 +2512,9 @@ intel_dp_destroy(struct drm_connector *connector)
 	struct drm_device *dev = connector->dev;
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 
+	if (!IS_ERR_OR_NULL(intel_connector->edid))
+		kfree(intel_connector->edid);
+
 	if (intel_dpd_is_edp(dev)) {
 		intel_panel_destroy_backlight(dev);
 		intel_panel_fini(&intel_connector->panel);
@@ -2528,7 +2532,6 @@ static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 	i2c_del_adapter(&intel_dp->adapter);
 	drm_encoder_cleanup(encoder);
 	if (is_edp(intel_dp)) {
-		kfree(intel_dp->edid);
 		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
 		ironlake_panel_vdd_off_sync(intel_dp);
 	}
@@ -2815,13 +2818,17 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 		ironlake_edp_panel_vdd_on(intel_dp);
 		edid = drm_get_edid(connector, &intel_dp->adapter);
 		if (edid) {
-			drm_mode_connector_update_edid_property(connector,
-								edid);
-			intel_dp->edid_mode_count =
-				drm_add_edid_modes(connector, edid);
-			drm_edid_to_eld(connector, edid);
-			intel_dp->edid = edid;
+			if (drm_add_edid_modes(connector, edid)) {
+				drm_mode_connector_update_edid_property(connector, edid);
+				drm_edid_to_eld(connector, edid);
+			} else {
+				kfree(edid);
+				edid = ERR_PTR(-EINVAL);
+			}
+		} else {
+			edid = ERR_PTR(-ENOENT);
 		}
+		intel_connector->edid = edid;
 
 		/* prefer fixed mode from EDID if available */
 		list_for_each_entry(scan, &connector->probed_modes, head) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ae0467a..5fa0af8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -186,6 +186,9 @@ struct intel_connector {
 
 	/* Panel info for eDP and LVDS */
 	struct intel_panel panel;
+
+	/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
+	struct edid *edid;
 };
 
 struct intel_crtc {
@@ -367,8 +370,6 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	struct edid *edid; /* cached EDID for eDP */
-	int edid_mode_count;
 	struct intel_connector *attached_connector;
 };
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ced6947..efa62ac 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -45,7 +45,6 @@ struct intel_lvds_connector {
 	struct intel_connector base;
 
 	struct notifier_block lid_notifier;
-	struct edid *edid;
 	int fitting_mode;
 };
 
@@ -461,8 +460,14 @@ static int intel_lvds_get_modes(struct drm_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);
+	/* use cached edid if we have one */
+	if (lvds_connector->base.edid) {
+		/* invalid edid */
+		if (IS_ERR(lvds_connector->base.edid))
+			return 0;
+
+		return drm_add_edid_modes(connector, lvds_connector->base.edid);
+	}
 
 	mode = drm_mode_duplicate(dev, lvds_connector->base.panel.fixed_mode);
 	if (mode == NULL)
@@ -554,6 +559,9 @@ static void intel_lvds_destroy(struct drm_connector *connector)
 	if (lvds_connector->lid_notifier.notifier_call)
 		acpi_lid_notifier_unregister(&lvds_connector->lid_notifier);
 
+	if (!IS_ERR_OR_NULL(lvds_connector->base.edid))
+		kfree(lvds_connector->base.edid);
+
 	intel_panel_destroy_backlight(connector->dev);
 	intel_panel_fini(&lvds_connector->base.panel);
 
@@ -923,6 +931,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	struct drm_encoder *encoder;
 	struct drm_display_mode *scan; /* *modes, *bios_mode; */
 	struct drm_display_mode *fixed_mode = NULL;
+	struct edid *edid;
 	struct drm_crtc *crtc;
 	u32 lvds;
 	int pipe;
@@ -1022,18 +1031,21 @@ 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,
-					    intel_gmbus_get_adapter(dev_priv, pin));
-	if (lvds_connector->edid) {
-		if (drm_add_edid_modes(connector, lvds_connector->edid)) {
+	edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, pin));
+	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 = ERR_PTR(-EINVAL);
 		}
+	} else {
+		edid = ERR_PTR(-ENOENT);
 	}
-	if (!lvds_connector->edid) {
+	lvds_connector->base.edid = edid;
+
+	if (IS_ERR_OR_NULL(edid)) {
 		/* Didn't get an EDID, so
 		 * Set wide sync ranges so we get all modes
 		 * handed to valid_mode for checking
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder
  2012-10-19 11:51 ` [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder Jani Nikula
@ 2012-10-19 16:46   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 16:46 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, 19 Oct 2012 14:51:43 +0300
Jani Nikula <jani.nikula@intel.com> 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.
> 
> 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 |  129 ++++++++++++++++++-------------------
>  1 file changed, 61 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 40d72bd..61bdf4c 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,15 +54,15 @@ 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);
>  }
>  
>  static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
> @@ -97,7 +97,7 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
>  static void intel_enable_lvds(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> -	struct intel_lvds *intel_lvds = to_intel_lvds(&encoder->base);
> +	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 ctl_reg, lvds_reg, stat_reg;
> @@ -114,7 +114,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>  
>  	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
> @@ -122,12 +122,12 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>  		 * 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);
>  
> -		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;
>  	}
>  
>  	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
> @@ -141,7 +141,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>  static void intel_disable_lvds(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> -	struct intel_lvds *intel_lvds = to_intel_lvds(&encoder->base);
> +	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 ctl_reg, lvds_reg, stat_reg;
>  
> @@ -161,9 +161,9 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>  	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
>  		DRM_ERROR("timed out waiting for panel to power off\n");
>  
> -	if (intel_lvds->pfit_control) {
> +	if (lvds_encoder->pfit_control) {
>  		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);
> @@ -173,8 +173,8 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>  static int intel_lvds_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;
> @@ -250,8 +250,8 @@ static bool intel_lvds_mode_fixup(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_crtc *intel_crtc = intel_lvds->base.new_crtc;
> +	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
> +	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
>  	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
>  	int pipe;
>  
> @@ -261,7 +261,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
>  		return false;
>  	}
>  
> -	if (intel_encoder_check_is_cloned(&intel_lvds->base))
> +	if (intel_encoder_check_is_cloned(&lvds_encoder->base))
>  		return false;
>  
>  	/*
> @@ -270,10 +270,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;
>  	}
> @@ -299,7 +299,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
>  
>  	drm_mode_set_crtcinfo(adjusted_mode, 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 &
> @@ -397,11 +397,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;
>  
> @@ -450,14 +450,14 @@ intel_lvds_detect(struct drm_connector *connector, bool force)
>   */
>  static int intel_lvds_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;
>  
> @@ -558,22 +558,22 @@ static int intel_lvds_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
> @@ -905,7 +905,7 @@ static bool intel_lvds_supported(struct drm_device *dev)
>  bool intel_lvds_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_lvds *intel_lvds;
> +	struct intel_lvds_encoder *lvds_encoder;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_connector *intel_connector;
>  	struct drm_connector *connector;
> @@ -938,22 +938,21 @@ 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;
> +	intel_encoder = &lvds_encoder->base;
>  	encoder = &intel_encoder->base;
>  	connector = &intel_connector->base;
>  	drm_connector_init(dev, &intel_connector->base, &intel_lvds_connector_funcs,
> @@ -993,7 +992,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:
>  	 * 1) check for EDID on DDC
> @@ -1008,20 +1007,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.
>  	 */
> -	intel_lvds->edid = drm_get_edid(connector,
> -					intel_gmbus_get_adapter(dev_priv,
> -								pin));
> -	if (intel_lvds->edid) {
> -		if (drm_add_edid_modes(connector,
> -				       intel_lvds->edid)) {
> +	lvds_encoder->edid = drm_get_edid(connector,
> +					  intel_gmbus_get_adapter(dev_priv, pin));
> +	if (lvds_encoder->edid) {
> +		if (drm_add_edid_modes(connector, 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
> @@ -1034,10 +1031,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) {
> -			intel_lvds->fixed_mode =
> -				drm_mode_duplicate(dev, scan);
> -			intel_find_lvds_downclock(dev,
> -						  intel_lvds->fixed_mode,
> +			lvds_encoder->fixed_mode = drm_mode_duplicate(dev, scan);
> +			intel_find_lvds_downclock(dev, lvds_encoder->fixed_mode,
>  						  connector);
>  			goto out;
>  		}
> @@ -1045,11 +1040,10 @@ bool intel_lvds_init(struct drm_device *dev)
>  
>  	/* Failed to get EDID, 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 |=
> -				DRM_MODE_TYPE_PREFERRED;
> +		if (lvds_encoder->fixed_mode) {
> +			lvds_encoder->fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
>  			goto out;
>  		}
>  	}
> @@ -1069,16 +1063,15 @@ 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 |=
> -				DRM_MODE_TYPE_PREFERRED;
> +		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:
> @@ -1110,7 +1103,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;
>  }

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

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector
  2012-10-19 11:51 ` [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector Jani Nikula
@ 2012-10-19 16:47   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 16:47 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector
  2012-10-19 11:51 ` [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector Jani Nikula
@ 2012-10-19 16:48   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 16:48 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> 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/i915_drv.h   |    2 --
>  drivers/gpu/drm/i915/intel_lvds.c |   32 +++++++++++++++++---------------
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4728d30..f63ae1c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -507,8 +507,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[I915_MAX_NUM_FENCES]; /* assume 965 */
>  	int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 4a2f6fa..f3edbf6 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -43,6 +43,8 @@
>  /* Private structure for the integrated LVDS support */
>  struct intel_lvds_connector {
>  	struct intel_connector base;
> +
> +	struct notifier_block lid_notifier;
>  };
>  
>  struct intel_lvds_encoder {
> @@ -506,10 +508,11 @@ 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_connector *connector = &lvds_connector->base.base;
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	if (dev->switch_power_state != DRM_SWITCH_POWER_ON)
>  		return NOTIFY_OK;
> @@ -518,9 +521,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
>  	 * check and update the status of LVDS connector after receiving
>  	 * the LID nofication event.
>  	 */
> -	if (connector)
> -		connector->status = connector->funcs->detect(connector,
> -							     false);
> +	connector->status = connector->funcs->detect(connector, false);
>  
>  	/* Don't force modeset on machines where it causes a GPU lockup */
>  	if (dmi_check_system(intel_no_modeset_on_lid))
> @@ -551,13 +552,14 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
>   */
>  static void intel_lvds_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);
>  
> -	intel_panel_destroy_backlight(dev);
> +	intel_panel_destroy_backlight(connector->dev);
>  
> -	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);
> @@ -1097,10 +1099,10 @@ out:
>  		I915_WRITE(PP_CONTROL,
>  			   I915_READ(PP_CONTROL) | PANEL_UNLOCK_REGS);
>  	}
> -	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;

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

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 04/10] drm/i915: Backlight setup requires connector so pass it as parameter
  2012-10-19 11:51 ` [PATCH 04/10] drm/i915: Backlight setup requires connector so pass it as parameter Jani Nikula
@ 2012-10-19 16:54   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 16:54 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> Get rid of saved int_lvds_connector and int_edp_connector in
> drm_i915_private.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    |    2 --
>  drivers/gpu/drm/i915/intel_dp.c    |    6 ++----
>  drivers/gpu/drm/i915/intel_drv.h   |    2 +-
>  drivers/gpu/drm/i915/intel_lvds.c  |    4 +---
>  drivers/gpu/drm/i915/intel_panel.c |   15 ++++-----------
>  5 files changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f63ae1c..79dd42a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -826,8 +826,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;
> -	struct drm_connector *int_edp_connector;
>  
>  	bool mchbar_need_disable;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 697b176..09244f3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2836,10 +2836,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  
>  	intel_encoder->hot_plug = intel_dp_hot_plug;
>  
> -	if (is_edp(intel_dp)) {
> -		dev_priv->int_edp_connector = connector;
> -		intel_panel_setup_backlight(dev);
> -	}
> +	if (is_edp(intel_dp))
> +		intel_panel_setup_backlight(connector);
>  
>  	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 95cbd67..e5cdf3d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -444,7 +444,7 @@ extern void intel_pch_panel_fitting(struct drm_device *dev,
>  				    struct drm_display_mode *adjusted_mode);
>  extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
>  extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
> -extern int intel_panel_setup_backlight(struct drm_device *dev);
> +extern int intel_panel_setup_backlight(struct drm_connector *connector);
>  extern void intel_panel_enable_backlight(struct drm_device *dev,
>  					 enum pipe pipe);
>  extern void intel_panel_disable_backlight(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index f3edbf6..cdee7bf 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1104,11 +1104,9 @@ out:
>  		DRM_DEBUG_KMS("lid notifier registration failed\n");
>  		lvds_connector->lid_notifier.notifier_call = NULL;
>  	}
> -	/* keep the LVDS connector */
> -	dev_priv->int_lvds_connector = connector;
>  	drm_sysfs_connector_add(connector);
>  
> -	intel_panel_setup_backlight(dev);
> +	intel_panel_setup_backlight(connector);
>  
>  	return true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e019b23..d9752a3 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -416,21 +416,14 @@ static const struct backlight_ops intel_panel_bl_ops = {
>  	.get_brightness = intel_panel_get_brightness,
>  };
>  
> -int intel_panel_setup_backlight(struct drm_device *dev)
> +int intel_panel_setup_backlight(struct drm_connector *connector)
>  {
> +	struct drm_device *dev = connector->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct backlight_properties props;
> -	struct drm_connector *connector;
>  
>  	intel_panel_init_backlight(dev);
>  
> -	if (dev_priv->int_lvds_connector)
> -		connector = dev_priv->int_lvds_connector;
> -	else if (dev_priv->int_edp_connector)
> -		connector = dev_priv->int_edp_connector;
> -	else
> -		return -ENODEV;
> -
>  	memset(&props, 0, sizeof(props));
>  	props.type = BACKLIGHT_RAW;
>  	props.max_brightness = _intel_panel_get_max_backlight(dev);
> @@ -460,9 +453,9 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
>  		backlight_device_unregister(dev_priv->backlight);
>  }
>  #else
> -int intel_panel_setup_backlight(struct drm_device *dev)
> +int intel_panel_setup_backlight(struct drm_connector *connector)
>  {
> -	intel_panel_init_backlight(dev);
> +	intel_panel_init_backlight(connector->dev);
>  	return 0;
>  }
>  

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 05/10] drm/i915/lvds: Move some connector specific info across from the encoder
  2012-10-19 11:51 ` [PATCH 05/10] drm/i915/lvds: Move some connector specific info across from the encoder Jani Nikula
@ 2012-10-19 16:55   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 16:55 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, 19 Oct 2012 14:51:47 +0300
Jani Nikula <jani.nikula@intel.com> 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.
> 
> 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 |   83 +++++++++++++++++++------------------
>  1 file changed, 42 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index cdee7bf..0ed2c3b 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -45,19 +45,19 @@ 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;
>  
> -	struct edid *edid;
> -
> -	int fitting_mode;
>  	u32 pfit_control;
>  	u32 pfit_pgm_ratios;
>  	bool pfit_dirty;
>  
> -	struct drm_display_mode *fixed_mode;
> +	struct intel_lvds_connector *attached_connector;
>  };
>  
>  static struct intel_lvds_encoder *to_lvds_encoder(struct drm_encoder *encoder)
> @@ -70,12 +70,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);
> -}
> -
>  static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
>  				    enum pipe *pipe)
>  {
> @@ -184,8 +178,8 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>  static int intel_lvds_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;
> @@ -262,6 +256,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(encoder);
> +	struct intel_lvds_connector *lvds_connector =
> +		lvds_encoder->attached_connector;
>  	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
>  	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
>  	int pipe;
> @@ -281,10 +277,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(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;
>  	}
> @@ -310,7 +306,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
>  
>  	drm_mode_set_crtcinfo(adjusted_mode, 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 &
> @@ -461,14 +457,14 @@ intel_lvds_detect(struct drm_connector *connector, bool force)
>   */
>  static int intel_lvds_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;
>  
> @@ -569,22 +565,24 @@ static int intel_lvds_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
> @@ -960,6 +958,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);
>  	}
> @@ -1005,7 +1005,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);
> -	lvds_encoder->fitting_mode = DRM_MODE_SCALE_ASPECT;
> +	lvds_connector->fitting_mode = DRM_MODE_SCALE_ASPECT;
>  	/*
>  	 * LVDS discovery:
>  	 * 1) check for EDID on DDC
> @@ -1020,18 +1020,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,
> -					  intel_gmbus_get_adapter(dev_priv, pin));
> -	if (lvds_encoder->edid) {
> -		if (drm_add_edid_modes(connector, lvds_encoder->edid)) {
> +	lvds_connector->edid = drm_get_edid(connector,
> +					    intel_gmbus_get_adapter(dev_priv, pin));
> +	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
> @@ -1044,8 +1044,9 @@ 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 = drm_mode_duplicate(dev, scan);
> -			intel_find_lvds_downclock(dev, lvds_encoder->fixed_mode,
> +			lvds_connector->fixed_mode = drm_mode_duplicate(dev, scan);
> +			intel_find_lvds_downclock(dev,
> +						  lvds_connector->fixed_mode,
>  						  connector);
>  			goto out;
>  		}
> @@ -1053,10 +1054,10 @@ bool intel_lvds_init(struct drm_device *dev)
>  
>  	/* Failed to get EDID, 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 |= DRM_MODE_TYPE_PREFERRED;
> +		if (lvds_connector->fixed_mode) {
> +			lvds_connector->fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
>  			goto out;
>  		}
>  	}
> @@ -1076,15 +1077,15 @@ 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 |= DRM_MODE_TYPE_PREFERRED;
> +		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:

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

IMO these first 5 should go into -queued asap.  They're nice cleanups
by themselves and make the code structure a lot better and more
readable.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 06/10] drm/i915/dp: Initialize eDP fixed mode in intel_dp_init
  2012-10-19 11:51 ` [PATCH 06/10] drm/i915/dp: Initialize eDP fixed mode in intel_dp_init Jani Nikula
@ 2012-10-19 17:00   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 17:00 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> Since we do EDID caching in intel_dp_init, we can do the fixed mode
> initialization there too. This should not change the functionality apart
> from initializing fixed mode earlier. Particularly retain the behaviour of
> only falling back to VBT if EDID is not available to not regress
> 
> commit 47f0eb2234a2a1c790825393bbaccfadf82463d3
> Author: Keith Packard <keithp@keithp.com>
> Date:   Mon Sep 19 14:33:26 2011 -0700
> 
>     drm/i915: Only use VBT panel mode on eDP if no EDID is found
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   53 ++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 09244f3..52cbee7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2407,42 +2407,20 @@ 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
>  	 */
>  
>  	ret = intel_dp_get_edid_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 (ret)
>  		return ret;
> -	}
>  
> -	/* if eDP has no EDID, try to use fixed panel mode from VBT */
> -	if (is_edp(intel_dp)) {
> -		/* initialize panel mode from VBT if available for eDP */
> -		if (intel_dp->panel_fixed_mode == NULL && dev_priv->lfp_lvds_vbt_mode != NULL) {
> -			intel_dp->panel_fixed_mode =
> -				drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
> -			if (intel_dp->panel_fixed_mode) {
> -				intel_dp->panel_fixed_mode->type |=
> -					DRM_MODE_TYPE_PREFERRED;
> -			}
> -		}
> -		if (intel_dp->panel_fixed_mode) {
> -			struct drm_display_mode *mode;
> -			mode = drm_mode_duplicate(dev, intel_dp->panel_fixed_mode);
> +	/* if eDP has no EDID, fall back to fixed mode */
> +	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
> +		struct drm_display_mode *mode;
> +		mode = drm_mode_duplicate(dev, intel_dp->panel_fixed_mode);
> +		if (mode) {
>  			drm_mode_probed_add(connector, mode);
>  			return 1;
>  		}
> @@ -2638,6 +2616,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	struct intel_dp *intel_dp;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_connector *intel_connector;
> +	struct drm_display_mode *fixed_mode = NULL;
>  	const char *name = NULL;
>  	int type;
>  
> @@ -2802,6 +2781,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  
>  	if (is_edp(intel_dp)) {
>  		bool ret;
> +		struct drm_display_mode *scan;
>  		struct edid *edid;
>  
>  		ironlake_edp_panel_vdd_on(intel_dp);
> @@ -2831,6 +2811,23 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  			drm_edid_to_eld(connector, edid);
>  			intel_dp->edid = edid;
>  		}
> +
> +		/* prefer fixed mode from EDID if available */
> +		list_for_each_entry(scan, &connector->probed_modes, head) {
> +			if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
> +				fixed_mode = drm_mode_duplicate(dev, scan);
> +				break;
> +			}
> +		}
> +
> +		/* fallback to VBT if available for eDP */
> +		if (!fixed_mode && dev_priv->lfp_lvds_vbt_mode) {
> +			fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
> +			if (fixed_mode)
> +				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> +		}
> +		intel_dp->panel_fixed_mode = fixed_mode;
> +
>  		ironlake_edp_panel_vdd_off(intel_dp, false);
>  	}
>  

Might be nice to factor out some of this fixed mode/eDP stuff into a
separate function, but that's just a cleanup and can happen on top.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 07/10] drm/i915: Create generic intel_panel for LVDS and eDP
  2012-10-19 11:51 ` [PATCH 07/10] drm/i915: Create generic intel_panel for LVDS and eDP Jani Nikula
@ 2012-10-19 17:04   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 17:04 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> Create a generic struct intel_panel for sharing a data structure and code
> between eDP and LVDS panels. Add the new struct to intel_connector so that
> later on we can have generic EDID and mode reading functions with EDID
> caching that transparently fallback to fixed mode when EDID is not
> available.
> 
> Add intel_panel as a dummy first, and move data (such as the mentioned
> fixed mode) to it in later patches.
> 
> 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_dp.c    |    9 +++++++--
>  drivers/gpu/drm/i915/intel_drv.h   |    9 +++++++++
>  drivers/gpu/drm/i915/intel_lvds.c  |    2 ++
>  drivers/gpu/drm/i915/intel_panel.c |    9 +++++++++
>  4 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 52cbee7..abbdfed 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2502,9 +2502,12 @@ static void
>  intel_dp_destroy(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
>  
> -	if (intel_dpd_is_edp(dev))
> +	if (intel_dpd_is_edp(dev)) {
>  		intel_panel_destroy_backlight(dev);
> +		intel_panel_fini(&intel_connector->panel);
> +	}
>  
>  	drm_sysfs_connector_remove(connector);
>  	drm_connector_cleanup(connector);
> @@ -2833,8 +2836,10 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  
>  	intel_encoder->hot_plug = intel_dp_hot_plug;
>  
> -	if (is_edp(intel_dp))
> +	if (is_edp(intel_dp)) {
> +		intel_panel_init(&intel_connector->panel);
>  		intel_panel_setup_backlight(connector);
> +	}
>  
>  	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 e5cdf3d..df00f21 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -163,6 +163,9 @@ struct intel_encoder {
>  	int crtc_mask;
>  };
>  
> +struct intel_panel {
> +};
> +
>  struct intel_connector {
>  	struct drm_connector base;
>  	/*
> @@ -179,6 +182,9 @@ struct intel_connector {
>  	/* Reads out the current hw, returning true if the connector is enabled
>  	 * and active (i.e. dpms ON state). */
>  	bool (*get_hw_state)(struct intel_connector *);
> +
> +	/* Panel info for eDP and LVDS */
> +	struct intel_panel panel;
>  };
>  
>  struct intel_crtc {
> @@ -436,6 +442,9 @@ extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
>  				      enum plane plane);
>  
>  /* intel_panel.c */
> +extern int intel_panel_init(struct intel_panel *panel);
> +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 0ed2c3b..db1b1b7 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -555,6 +555,7 @@ static void intel_lvds_destroy(struct drm_connector *connector)
>  		acpi_lid_notifier_unregister(&lvds_connector->lid_notifier);
>  
>  	intel_panel_destroy_backlight(connector->dev);
> +	intel_panel_fini(&lvds_connector->base.panel);
>  
>  	drm_sysfs_connector_remove(connector);
>  	drm_connector_cleanup(connector);
> @@ -1107,6 +1108,7 @@ out:
>  	}
>  	drm_sysfs_connector_add(connector);
>  
> +	intel_panel_init(&intel_connector->panel);
>  	intel_panel_setup_backlight(connector);
>  
>  	return true;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index d9752a3..4c64ebc 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -464,3 +464,12 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
>  	return;
>  }
>  #endif
> +
> +int intel_panel_init(struct intel_panel *panel)
> +{
> +	return 0;
> +}
> +
> +void intel_panel_fini(struct intel_panel *panel)
> +{
> +}

Looks good.  My only nitpick is that this could be any type of fixed
display, so panel may not be the best name.  But practically speaking I
guess MIPI, eDP, LVDS, or even fixed HDMI or DVI displays will be
panels so no biggie.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 08/10] drm/i915: Move the fixed mode to intel_panel
  2012-10-19 11:51 ` [PATCH 08/10] drm/i915: Move the fixed mode to intel_panel Jani Nikula
@ 2012-10-19 17:09   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 17:09 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> Pave the way for sharing some logic between eDP and LVDS.
> 
> 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_dp.c    |   29 ++++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_drv.h   |    6 ++++--
>  drivers/gpu/drm/i915/intel_lvds.c  |   36 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_panel.c |   10 +++++++++-
>  4 files changed, 49 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index abbdfed..8c72477 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -122,9 +122,10 @@ intel_edp_target_clock(struct intel_encoder *intel_encoder,
>  		       struct drm_display_mode *mode)
>  {
>  	struct intel_dp *intel_dp = container_of(intel_encoder, struct intel_dp, base);
> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
>  
> -	if (intel_dp->panel_fixed_mode)
> -		return intel_dp->panel_fixed_mode->clock;
> +	if (intel_connector->panel.fixed_mode)
> +		return intel_connector->panel.fixed_mode->clock;
>  	else
>  		return mode->clock;
>  }
> @@ -228,12 +229,14 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  		    struct drm_display_mode *mode)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>  
> -	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
> -		if (mode->hdisplay > intel_dp->panel_fixed_mode->hdisplay)
> +	if (is_edp(intel_dp) && fixed_mode) {
> +		if (mode->hdisplay > fixed_mode->hdisplay)
>  			return MODE_PANEL;
>  
> -		if (mode->vdisplay > intel_dp->panel_fixed_mode->vdisplay)
> +		if (mode->vdisplay > fixed_mode->vdisplay)
>  			return MODE_PANEL;
>  	}
>  
> @@ -700,14 +703,16 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
>  {
>  	struct drm_device *dev = encoder->dev;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
>  	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;
>  	int bpp, mode_rate;
>  	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
>  
> -	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
> -		intel_fixed_panel_mode(intel_dp->panel_fixed_mode, adjusted_mode);
> +	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> +		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> +				       adjusted_mode);
>  		intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
>  					mode, adjusted_mode);
>  	}
> @@ -2406,6 +2411,7 @@ 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 intel_connector *intel_connector = to_intel_connector(connector);
>  	struct drm_device *dev = intel_dp->base.base.dev;
>  	int ret;
>  
> @@ -2417,9 +2423,10 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  		return ret;
>  
>  	/* if eDP has no EDID, fall back to fixed mode */
> -	if (is_edp(intel_dp) && intel_dp->panel_fixed_mode) {
> +	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>  		struct drm_display_mode *mode;
> -		mode = drm_mode_duplicate(dev, intel_dp->panel_fixed_mode);
> +		mode = drm_mode_duplicate(dev,
> +					  intel_connector->panel.fixed_mode);
>  		if (mode) {
>  			drm_mode_probed_add(connector, mode);
>  			return 1;
> @@ -2638,6 +2645,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  		return;
>  	}
>  	intel_encoder = &intel_dp->base;
> +	intel_dp->attached_connector = intel_connector;
>  
>  	if (HAS_PCH_SPLIT(dev) && output_reg == PCH_DP_D)
>  		if (intel_dpd_is_edp(dev))
> @@ -2829,7 +2837,6 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  			if (fixed_mode)
>  				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
>  		}
> -		intel_dp->panel_fixed_mode = fixed_mode;
>  
>  		ironlake_edp_panel_vdd_off(intel_dp, false);
>  	}
> @@ -2837,7 +2844,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	intel_encoder->hot_plug = intel_dp_hot_plug;
>  
>  	if (is_edp(intel_dp)) {
> -		intel_panel_init(&intel_connector->panel);
> +		intel_panel_init(&intel_connector->panel, fixed_mode);
>  		intel_panel_setup_backlight(connector);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index df00f21..ae0467a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -164,6 +164,7 @@ struct intel_encoder {
>  };
>  
>  struct intel_panel {
> +	struct drm_display_mode *fixed_mode;
>  };
>  
>  struct intel_connector {
> @@ -364,11 +365,11 @@ struct intel_dp {
>  	int panel_power_cycle_delay;
>  	int backlight_on_delay;
>  	int backlight_off_delay;
> -	struct drm_display_mode *panel_fixed_mode;  /* for eDP */
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
>  	struct edid *edid; /* cached EDID for eDP */
>  	int edid_mode_count;
> +	struct intel_connector *attached_connector;
>  };
>  
>  static inline struct drm_crtc *
> @@ -442,7 +443,8 @@ extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
>  				      enum plane plane);
>  
>  /* intel_panel.c */
> -extern int intel_panel_init(struct intel_panel *panel);
> +extern int intel_panel_init(struct intel_panel *panel,
> +			    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 db1b1b7..ced6947 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -45,7 +45,6 @@ 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;
>  };
> @@ -178,8 +177,8 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>  static int intel_lvds_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 intel_connector *intel_connector = to_intel_connector(connector);
> +	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>  
>  	if (mode->hdisplay > fixed_mode->hdisplay)
>  		return MODE_PANEL;
> @@ -277,7 +276,8 @@ 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(lvds_connector->fixed_mode, adjusted_mode);
> +	intel_fixed_panel_mode(lvds_connector->base.panel.fixed_mode,
> +			       adjusted_mode);
>  
>  	if (HAS_PCH_SPLIT(dev)) {
>  		intel_pch_panel_fitting(dev, lvds_connector->fitting_mode,
> @@ -464,7 +464,7 @@ static int intel_lvds_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->base.panel.fixed_mode);
>  	if (mode == NULL)
>  		return 0;
>  
> @@ -922,6 +922,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;
> @@ -1045,20 +1046,17 @@ 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;
>  		}
>  	}
>  
>  	/* Failed to get EDID, 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;
>  		}
>  	}
> @@ -1078,15 +1076,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:
> @@ -1108,7 +1106,7 @@ out:
>  	}
>  	drm_sysfs_connector_add(connector);
>  
> -	intel_panel_init(&intel_connector->panel);
> +	intel_panel_init(&intel_connector->panel, fixed_mode);
>  	intel_panel_setup_backlight(connector);
>  
>  	return true;
> @@ -1117,6 +1115,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 4c64ebc..e91a0bb 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -465,11 +465,19 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
>  }
>  #endif
>  
> -int intel_panel_init(struct intel_panel *panel)
> +int intel_panel_init(struct intel_panel *panel,
> +		     struct drm_display_mode *fixed_mode)
>  {
> +	panel->fixed_mode = fixed_mode;
> +
>  	return 0;
>  }
>  
>  void intel_panel_fini(struct intel_panel *panel)
>  {
> +	struct intel_connector *intel_connector =
> +		container_of(panel, struct intel_connector, panel);
> +
> +	if (panel->fixed_mode)
> +		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
>  }

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


-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 09/10] drm/i915: Do not free the passed EDID in intel_connector_update_modes()
  2012-10-19 11:51 ` [PATCH 09/10] drm/i915: Do not free the passed EDID in intel_connector_update_modes() Jani Nikula
@ 2012-10-19 17:10   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 17:10 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> The caller, not intel_connector_update_modes(), should free the edid. This
> improves the reusability of intel_connector_update_modes().
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c   |    6 +++++-
>  drivers/gpu/drm/i915/intel_modes.c |    7 +++++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 53f3e87..35c92d5 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -419,12 +419,16 @@ static int intel_crt_ddc_get_modes(struct drm_connector *connector,
>  				struct i2c_adapter *adapter)
>  {
>  	struct edid *edid;
> +	int ret;
>  
>  	edid = intel_crt_get_edid(connector, adapter);
>  	if (!edid)
>  		return 0;
>  
> -	return intel_connector_update_modes(connector, edid);
> +	ret = intel_connector_update_modes(connector, edid);
> +	kfree(edid);
> +
> +	return ret;
>  }
>  
>  static bool intel_crt_detect_ddc(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 1773fb8..b57dbbe 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -44,7 +44,6 @@ int intel_connector_update_modes(struct drm_connector *connector,
>  	drm_mode_connector_update_edid_property(connector, edid);
>  	ret = drm_add_edid_modes(connector, edid);
>  	drm_edid_to_eld(connector, edid);
> -	kfree(edid);
>  
>  	return ret;
>  }
> @@ -60,12 +59,16 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>  			struct i2c_adapter *adapter)
>  {
>  	struct edid *edid;
> +	int ret;
>  
>  	edid = drm_get_edid(connector, adapter);
>  	if (!edid)
>  		return 0;
>  
> -	return intel_connector_update_modes(connector, edid);
> +	ret = intel_connector_update_modes(connector, edid);
> +	kfree(edid);
> +
> +	return ret;
>  }
>  
>  static const struct drm_prop_enum_list force_audio_names[] = {

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

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 10/10] drm/i915: Move cached EDID to intel_connector
  2012-10-19 11:51 ` [PATCH 10/10] drm/i915: Move cached EDID to intel_connector Jani Nikula
@ 2012-10-19 17:13   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2012-10-19 17:13 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

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

> Move the cached EDID from intel_dp and intel_lvds_connector to
> intel_connector. Unify cached EDID handling for LVDS and eDP, in
> preparation for adding more generic EDID caching later.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c   |   61 +++++++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h  |    5 +--
>  drivers/gpu/drm/i915/intel_lvds.c |   34 ++++++++++++++-------
>  3 files changed, 60 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8c72477..5b0e4cd 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2323,44 +2323,45 @@ g4x_dp_detect(struct intel_dp *intel_dp)
>  static struct edid *
>  intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  {
> -	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct edid	*edid;
> -	int size;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
>  
> -	if (is_edp(intel_dp)) {
> -		if (!intel_dp->edid)
> +	/* use cached edid if we have one */
> +	if (intel_connector->edid) {
> +		struct edid *edid;
> +		int size;
> +
> +		/* invalid edid */
> +		if (IS_ERR(intel_connector->edid))
>  			return NULL;
>  
> -		size = (intel_dp->edid->extensions + 1) * EDID_LENGTH;
> +		size = (intel_connector->edid->extensions + 1) * EDID_LENGTH;
>  		edid = kmalloc(size, GFP_KERNEL);
>  		if (!edid)
>  			return NULL;
>  
> -		memcpy(edid, intel_dp->edid, size);
> +		memcpy(edid, intel_connector->edid, size);
>  		return edid;
>  	}
>  
> -	edid = drm_get_edid(connector, adapter);
> -	return edid;
> +	return drm_get_edid(connector, adapter);
>  }
>  
>  static int
>  intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter)
>  {
> -	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	int	ret;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
>  
> -	if (is_edp(intel_dp)) {
> -		drm_mode_connector_update_edid_property(connector,
> -							intel_dp->edid);
> -		ret = drm_add_edid_modes(connector, intel_dp->edid);
> -		drm_edid_to_eld(connector,
> -				intel_dp->edid);
> -		return intel_dp->edid_mode_count;
> +	/* use cached edid if we have one */
> +	if (intel_connector->edid) {
> +		/* invalid edid */
> +		if (IS_ERR(intel_connector->edid))
> +			return 0;
> +
> +		return intel_connector_update_modes(connector,
> +						    intel_connector->edid);
>  	}
>  
> -	ret = intel_ddc_get_modes(connector, adapter);
> -	return ret;
> +	return intel_ddc_get_modes(connector, adapter);
>  }
>  
>  
> @@ -2511,6 +2512,9 @@ intel_dp_destroy(struct drm_connector *connector)
>  	struct drm_device *dev = connector->dev;
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  
> +	if (!IS_ERR_OR_NULL(intel_connector->edid))
> +		kfree(intel_connector->edid);
> +
>  	if (intel_dpd_is_edp(dev)) {
>  		intel_panel_destroy_backlight(dev);
>  		intel_panel_fini(&intel_connector->panel);
> @@ -2528,7 +2532,6 @@ static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  	i2c_del_adapter(&intel_dp->adapter);
>  	drm_encoder_cleanup(encoder);
>  	if (is_edp(intel_dp)) {
> -		kfree(intel_dp->edid);
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
>  		ironlake_panel_vdd_off_sync(intel_dp);
>  	}
> @@ -2815,13 +2818,17 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  		ironlake_edp_panel_vdd_on(intel_dp);
>  		edid = drm_get_edid(connector, &intel_dp->adapter);
>  		if (edid) {
> -			drm_mode_connector_update_edid_property(connector,
> -								edid);
> -			intel_dp->edid_mode_count =
> -				drm_add_edid_modes(connector, edid);
> -			drm_edid_to_eld(connector, edid);
> -			intel_dp->edid = edid;
> +			if (drm_add_edid_modes(connector, edid)) {
> +				drm_mode_connector_update_edid_property(connector, edid);
> +				drm_edid_to_eld(connector, edid);
> +			} else {
> +				kfree(edid);
> +				edid = ERR_PTR(-EINVAL);
> +			}
> +		} else {
> +			edid = ERR_PTR(-ENOENT);
>  		}
> +		intel_connector->edid = edid;
>  
>  		/* prefer fixed mode from EDID if available */
>  		list_for_each_entry(scan, &connector->probed_modes, head) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ae0467a..5fa0af8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -186,6 +186,9 @@ struct intel_connector {
>  
>  	/* Panel info for eDP and LVDS */
>  	struct intel_panel panel;
> +
> +	/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
> +	struct edid *edid;
>  };
>  
>  struct intel_crtc {
> @@ -367,8 +370,6 @@ struct intel_dp {
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> -	struct edid *edid; /* cached EDID for eDP */
> -	int edid_mode_count;
>  	struct intel_connector *attached_connector;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index ced6947..efa62ac 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -45,7 +45,6 @@ struct intel_lvds_connector {
>  	struct intel_connector base;
>  
>  	struct notifier_block lid_notifier;
> -	struct edid *edid;
>  	int fitting_mode;
>  };
>  
> @@ -461,8 +460,14 @@ static int intel_lvds_get_modes(struct drm_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);
> +	/* use cached edid if we have one */
> +	if (lvds_connector->base.edid) {
> +		/* invalid edid */
> +		if (IS_ERR(lvds_connector->base.edid))
> +			return 0;
> +
> +		return drm_add_edid_modes(connector, lvds_connector->base.edid);
> +	}
>  
>  	mode = drm_mode_duplicate(dev, lvds_connector->base.panel.fixed_mode);
>  	if (mode == NULL)
> @@ -554,6 +559,9 @@ static void intel_lvds_destroy(struct drm_connector *connector)
>  	if (lvds_connector->lid_notifier.notifier_call)
>  		acpi_lid_notifier_unregister(&lvds_connector->lid_notifier);
>  
> +	if (!IS_ERR_OR_NULL(lvds_connector->base.edid))
> +		kfree(lvds_connector->base.edid);
> +
>  	intel_panel_destroy_backlight(connector->dev);
>  	intel_panel_fini(&lvds_connector->base.panel);
>  
> @@ -923,6 +931,7 @@ bool intel_lvds_init(struct drm_device *dev)
>  	struct drm_encoder *encoder;
>  	struct drm_display_mode *scan; /* *modes, *bios_mode; */
>  	struct drm_display_mode *fixed_mode = NULL;
> +	struct edid *edid;
>  	struct drm_crtc *crtc;
>  	u32 lvds;
>  	int pipe;
> @@ -1022,18 +1031,21 @@ 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,
> -					    intel_gmbus_get_adapter(dev_priv, pin));
> -	if (lvds_connector->edid) {
> -		if (drm_add_edid_modes(connector, lvds_connector->edid)) {
> +	edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, pin));
> +	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 = ERR_PTR(-EINVAL);
>  		}
> +	} else {
> +		edid = ERR_PTR(-ENOENT);
>  	}
> -	if (!lvds_connector->edid) {
> +	lvds_connector->base.edid = edid;
> +
> +	if (IS_ERR_OR_NULL(edid)) {
>  		/* Didn't get an EDID, so
>  		 * Set wide sync ranges so we get all modes
>  		 * handed to valid_mode for checking

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

Looks like 6-10 should also just go upstream, looks like good stuff.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring
  2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
                   ` (9 preceding siblings ...)
  2012-10-19 11:51 ` [PATCH 10/10] drm/i915: Move cached EDID to intel_connector Jani Nikula
@ 2012-10-19 17:58 ` Chris Wilson
  2012-10-22 17:37   ` Daniel Vetter
  10 siblings, 1 reply; 25+ messages in thread
From: Chris Wilson @ 2012-10-19 17:58 UTC (permalink / raw)
  To: intel-gfx, daniel; +Cc: jani.nikula

On Fri, 19 Oct 2012 14:51:42 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
> 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.

Patches look really good, I haven't checked yet to see if there is
anything missing, but I already like the improvement to the code....

So for the series,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring
  2012-10-19 17:58 ` [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Chris Wilson
@ 2012-10-22 17:37   ` Daniel Vetter
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2012-10-22 17:37 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Jani Nikula, intel-gfx

On Fri, Oct 19, 2012 at 06:58:56PM +0100, Chris Wilson wrote:
> On Fri, 19 Oct 2012 14:51:42 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
> > 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.
> 
> Patches look really good, I haven't checked yet to see if there is
> anything missing, but I already like the improvement to the code....
> 
> So for the series,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Slurped in the entire series, thanks a lot for the patches and review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector
  2011-04-22  9:19 ` [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector Chris Wilson
@ 2011-04-22 16:19   ` Jesse Barnes
  0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2011-04-22 16:19 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector
  2011-04-22  9:19 Share common logic between eDP and LVDS in panel Chris Wilson
@ 2011-04-22  9:19 ` Chris Wilson
  2011-04-22 16:19   ` Jesse Barnes
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Wilson @ 2011-04-22  9:19 UTC (permalink / raw)
  To: intel-gfx

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2012-10-22 17:36 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-19 11:51 [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Jani Nikula
2012-10-19 11:51 ` [PATCH 01/10] drm/i915/lvds: Rename intel_lvds to intel_lvds_encoder Jani Nikula
2012-10-19 16:46   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 02/10] drm/i915/lvds: Introduce intel_lvds_connector Jani Nikula
2012-10-19 16:47   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector Jani Nikula
2012-10-19 16:48   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 04/10] drm/i915: Backlight setup requires connector so pass it as parameter Jani Nikula
2012-10-19 16:54   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 05/10] drm/i915/lvds: Move some connector specific info across from the encoder Jani Nikula
2012-10-19 16:55   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 06/10] drm/i915/dp: Initialize eDP fixed mode in intel_dp_init Jani Nikula
2012-10-19 17:00   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 07/10] drm/i915: Create generic intel_panel for LVDS and eDP Jani Nikula
2012-10-19 17:04   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 08/10] drm/i915: Move the fixed mode to intel_panel Jani Nikula
2012-10-19 17:09   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 09/10] drm/i915: Do not free the passed EDID in intel_connector_update_modes() Jani Nikula
2012-10-19 17:10   ` Jesse Barnes
2012-10-19 11:51 ` [PATCH 10/10] drm/i915: Move cached EDID to intel_connector Jani Nikula
2012-10-19 17:13   ` Jesse Barnes
2012-10-19 17:58 ` [PATCH 00/10] drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring Chris Wilson
2012-10-22 17:37   ` Daniel Vetter
  -- strict thread matches above, loose matches on Subject: below --
2011-04-22  9:19 Share common logic between eDP and LVDS in panel Chris Wilson
2011-04-22  9:19 ` [PATCH 03/10] drm/i915/lvds: Move the acpi_lid_notifier from drm_i915_private to the connector Chris Wilson
2011-04-22 16:19   ` Jesse Barnes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.