drm/i915: LVDS/eDP panel, EDID and fixed mode refactoring

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 <[email protected]>

CC Chris Wilson <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_lvds.c |  129 ++++++++++++++++++-------------------
 1 file changed, 61 insertions(+), 68 deletions(-)

drivers/gpu/drm/i915/intel_lvds.c +60 -67

index 40d72bd..61bdf4c 100644
@@ -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:

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 <[email protected]>

CC Chris Wilson <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_lvds.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/intel_lvds.c +13 -2

index 61bdf4c..4a2f6fa 100644
@@ -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);

Based on earlier work by Chris Wilson <[email protected]>

CC Chris Wilson <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h   |    2 --
 drivers/gpu/drm/i915/intel_lvds.c |   32 +++++++++++++++++---------------
 2 files changed, 17 insertions(+), 17 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +0 -2

index 4728d30..f63ae1c 100644
@@ -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 */

drivers/gpu/drm/i915/intel_lvds.c +14 -12

index 4a2f6fa..f3edbf6 100644
@@ -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);
 	}

Get rid of saved int_lvds_connector and int_edp_connector in drm_i915_private.

Signed-off-by Jani Nikula <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_drv.h +0 -2

index f63ae1c..79dd42a 100644
@@ -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;
 

drivers/gpu/drm/i915/intel_dp.c +2 -4

index 697b176..09244f3 100644
@@ -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);
 

drivers/gpu/drm/i915/intel_drv.h +1 -1

index 95cbd67..e5cdf3d 100644
@@ -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);

drivers/gpu/drm/i915/intel_lvds.c +1 -3

index f3edbf6..cdee7bf 100644
@@ -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;
 

drivers/gpu/drm/i915/intel_panel.c +2 -9

index e019b23..d9752a3 100644
@@ -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);

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 <[email protected]>

CC Chris Wilson <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_lvds.c |   83 +++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 41 deletions(-)

drivers/gpu/drm/i915/intel_lvds.c +41 -40

index cdee7bf..0ed2c3b 100644
@@ -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;

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 <[email protected]>
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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   53 ++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +18 -28

index 09244f3..52cbee7 100644
@@ -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 */

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 <[email protected]>

CC Chris Wilson <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/intel_dp.c +7 -2

index 52cbee7..abbdfed 100644
@@ -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);
 

drivers/gpu/drm/i915/intel_drv.h +9 -0

index e5cdf3d..df00f21 100644
@@ -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,

drivers/gpu/drm/i915/intel_lvds.c +2 -0

index 0ed2c3b..db1b1b7 100644
@@ -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;

drivers/gpu/drm/i915/intel_panel.c +0 -0

index d9752a3..4c64ebc 100644
@@ -464,3 +464,12 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
 	return;
 }

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

Based on earlier work by Chris Wilson <[email protected]>

CC Chris Wilson <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/intel_dp.c +18 -11

index abbdfed..8c72477 100644
@@ -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);
 	}
 

drivers/gpu/drm/i915/intel_drv.h +4 -2

index df00f21..ae0467a 100644
@@ -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,

drivers/gpu/drm/i915/intel_lvds.c +18 -18

index db1b1b7..ced6947 100644
@@ -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;

drivers/gpu/drm/i915/intel_panel.c +4 -1

index 4c64ebc..e91a0bb 100644
@@ -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;

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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_crt.c   |    6 +++++-
 drivers/gpu/drm/i915/intel_modes.c |    7 +++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/intel_crt.c +5 -1

index 53f3e87..35c92d5 100644
@@ -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)

drivers/gpu/drm/i915/intel_modes.c +1 -1

index 1773fb8..b57dbbe 100644
@@ -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)

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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/intel_dp.c +34 -27

index 8c72477..5b0e4cd 100644
@@ -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) {

drivers/gpu/drm/i915/intel_drv.h +3 -2

index ae0467a..5fa0af8 100644
@@ -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;
 };
 

drivers/gpu/drm/i915/intel_lvds.c +18 -10

index ced6947..efa62ac 100644
@@ -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);
 		}

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.

Quoted 5 lines · 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].

Quoted 8 lines

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 <[email protected]>