diff mbox

drm/i915: Make downclock deduction common for all panels

Message ID 1386577535-4245-1-git-send-email-vandana.kannan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

vandana.kannan@intel.com Dec. 9, 2013, 8:25 a.m. UTC
If one mode of a internal panel has more than one refresh rate, then a reduced
clock is found for the LFP (LVDS/eDP). This enables switching between low
and high frequency dynamically. Moving downclock calculation to intel_panel
so that it is common for LVDS and eDP.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h   |    6 +++-
 drivers/gpu/drm/i915/intel_lvds.c  |   69 +++++++++---------------------------
 drivers/gpu/drm/i915/intel_panel.c |   56 +++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 54 deletions(-)

Comments

Jani Nikula Dec. 9, 2013, 9:31 a.m. UTC | #1
On Mon, 09 Dec 2013, Vandana Kannan <vandana.kannan@intel.com> wrote:
> If one mode of a internal panel has more than one refresh rate, then a reduced
> clock is found for the LFP (LVDS/eDP). This enables switching between low
> and high frequency dynamically. Moving downclock calculation to intel_panel
> so that it is common for LVDS and eDP.
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |    6 +++-
>  drivers/gpu/drm/i915/intel_lvds.c  |   69 +++++++++---------------------------
>  drivers/gpu/drm/i915/intel_panel.c |   56 +++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5dea389..17e8964 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -155,6 +155,7 @@ struct intel_encoder {
>  
>  struct intel_panel {
>  	struct drm_display_mode *fixed_mode;
> +	struct drm_display_mode *downclock_mode;
>  	int fitting_mode;
>  
>  	/* backlight */
> @@ -823,7 +824,10 @@ void intel_panel_disable_backlight(struct intel_connector *connector);
>  void intel_panel_destroy_backlight(struct drm_connector *connector);
>  void intel_panel_init_backlight_funcs(struct drm_device *dev);
>  enum drm_connector_status intel_panel_detect(struct drm_device *dev);
> -
> +extern bool intel_find_panel_downclock(struct drm_device *dev,
> +			      struct intel_panel *panel,
> +			      struct drm_display_mode *fixed_mode,
> +			      struct drm_connector *connector);
>  
>  /* intel_pm.c */
>  void intel_init_clock_gating(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 3deb58e..a589814 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -745,57 +745,6 @@ static const struct dmi_system_id intel_no_lvds[] = {
>  	{ }	/* terminating entry */
>  };
>  
> -/**
> - * intel_find_lvds_downclock - find the reduced downclock for LVDS in EDID
> - * @dev: drm device
> - * @connector: LVDS connector
> - *
> - * Find the reduced downclock for LVDS in EDID.
> - */
> -static void intel_find_lvds_downclock(struct drm_device *dev,
> -				      struct drm_display_mode *fixed_mode,
> -				      struct drm_connector *connector)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_display_mode *scan;
> -	int temp_downclock;
> -
> -	temp_downclock = fixed_mode->clock;
> -	list_for_each_entry(scan, &connector->probed_modes, head) {
> -		/*
> -		 * If one mode has the same resolution with the fixed_panel
> -		 * mode while they have the different refresh rate, it means
> -		 * that the reduced downclock is found for the LVDS. In such
> -		 * case we can set the different FPx0/1 to dynamically select
> -		 * between low and high frequency.
> -		 */
> -		if (scan->hdisplay == fixed_mode->hdisplay &&
> -		    scan->hsync_start == fixed_mode->hsync_start &&
> -		    scan->hsync_end == fixed_mode->hsync_end &&
> -		    scan->htotal == fixed_mode->htotal &&
> -		    scan->vdisplay == fixed_mode->vdisplay &&
> -		    scan->vsync_start == fixed_mode->vsync_start &&
> -		    scan->vsync_end == fixed_mode->vsync_end &&
> -		    scan->vtotal == fixed_mode->vtotal) {
> -			if (scan->clock < temp_downclock) {
> -				/*
> -				 * The downclock is already found. But we
> -				 * expect to find the lower downclock.
> -				 */
> -				temp_downclock = scan->clock;
> -			}
> -		}
> -	}
> -	if (temp_downclock < fixed_mode->clock && i915_lvds_downclock) {
> -		/* We found the downclock for LVDS. */
> -		dev_priv->lvds_downclock_avail = 1;
> -		dev_priv->lvds_downclock = temp_downclock;
> -		DRM_DEBUG_KMS("LVDS downclock is found in EDID. "
> -			      "Normal clock %dKhz, downclock %dKhz\n",
> -			      fixed_mode->clock, temp_downclock);
> -	}
> -}
> -
>  /*
>   * Enumerate the child dev array parsed from VBT to check whether
>   * the LVDS is present.
> @@ -1073,8 +1022,22 @@ void intel_lvds_init(struct drm_device *dev)
>  
>  			fixed_mode = drm_mode_duplicate(dev, scan);
>  			if (fixed_mode) {
> -				intel_find_lvds_downclock(dev, fixed_mode,
> -							  connector);
> +				dev_priv->lvds_downclock_avail =
> +					intel_find_panel_downclock(dev,
> +					&intel_connector->panel,
> +					fixed_mode, connector);
> +				if (dev_priv->lvds_downclock_avail &&
> +					i915_lvds_downclock) {
> +					/* We found the downclock for LVDS. */
> +					dev_priv->lvds_downclock =
> +						intel_connector->panel.
> +						downclock_mode->clock;
> +					DRM_DEBUG_KMS("LVDS downclock is found"
> +					" in EDID. Normal clock %dKhz, "
> +					"downclock %dKhz\n",
> +					fixed_mode->clock,
> +					dev_priv->lvds_downclock);
> +				}
>  				goto out;
>  			}
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e480cf4..f49a136 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1104,6 +1104,62 @@ void intel_panel_destroy_backlight(struct drm_connector *connector)
>  	intel_backlight_device_unregister(intel_connector);
>  }
>  
> +/**
> + * intel_find_panel_downclock - find the reduced downclock for LVDS in EDID
> + * @dev: drm device
> + * @panel: intel panel
> + * @fixed_mode : panel native mode
> + * @connector: LVDS/eDP connector
> + *
> + * Return downclock_avail
> + * Find the reduced downclock for LVDS/eDP in EDID.
> + */
> +bool intel_find_panel_downclock(struct drm_device *dev,
> +			      struct intel_panel *panel,
> +			      struct drm_display_mode *fixed_mode,
> +			      struct drm_connector *connector)

I think I'd make this

struct drm_display_mode *
intel_panel_find_downclock(struct drm_connector *connector,
			   struct drm_display_mode *fixed_mode)

which would return a drm_mode_duplicate'd downclocked mode if one was
found, NULL otherwise. Then whoever calls the function would set
panel->downclock_mode explicitly instead of it happening as a side
effect here. (A "find" function that sets stuff is surprising! I know it
was a little like this already, but let's be a little more careful with
non-static functions.)

> +{
> +	struct drm_display_mode *scan, *tmp_mode;
> +	int temp_downclock;
> +	bool ret = false;
> +
> +	temp_downclock = fixed_mode->clock;
> +	tmp_mode = NULL;
> +	panel->downclock_mode = NULL;
> +
> +	list_for_each_entry(scan, &connector->probed_modes, head) {
> +		/*
> +		 * If one mode has the same resolution with the fixed_panel
> +		 * mode while they have the different refresh rate, it means
> +		 * that the reduced downclock is found. In such
> +		 * case we can set the different FPx0/1 to dynamically select
> +		 * between low and high frequency.
> +		 */
> +		if (scan->hdisplay == fixed_mode->hdisplay &&
> +		    scan->hsync_start == fixed_mode->hsync_start &&
> +		    scan->hsync_end == fixed_mode->hsync_end &&
> +		    scan->vdisplay == fixed_mode->vdisplay &&
> +		    scan->vsync_start == fixed_mode->vsync_start &&
> +		    scan->vsync_end == fixed_mode->vsync_end) {

You drop htotal and vtotal from the check. Is this intentional? It
*must* be a separate patch with a commit message that justifies the
removal.

> +			if (scan->clock < temp_downclock) {
> +				/*
> +				 * The downclock is already found. But we
> +				 * expect to find the lower downclock.
> +				 */
> +				temp_downclock = scan->clock;
> +				tmp_mode = scan;
> +			}
> +		}
> +	}
> +
> +	if (temp_downclock < fixed_mode->clock) {
> +		panel->downclock_mode = drm_mode_duplicate(dev, tmp_mode);

You need to drm_mode_destroy() the downclock_mode in intel_panel_fini().

BR,
Jani.

> +		ret = true;
> +	}
> +
> +	return ret;
> +}
> +
>  /* Set up chip specific backlight functions */
>  void intel_panel_init_backlight_funcs(struct drm_device *dev)
>  {
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5dea389..17e8964 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -155,6 +155,7 @@  struct intel_encoder {
 
 struct intel_panel {
 	struct drm_display_mode *fixed_mode;
+	struct drm_display_mode *downclock_mode;
 	int fitting_mode;
 
 	/* backlight */
@@ -823,7 +824,10 @@  void intel_panel_disable_backlight(struct intel_connector *connector);
 void intel_panel_destroy_backlight(struct drm_connector *connector);
 void intel_panel_init_backlight_funcs(struct drm_device *dev);
 enum drm_connector_status intel_panel_detect(struct drm_device *dev);
-
+extern bool intel_find_panel_downclock(struct drm_device *dev,
+			      struct intel_panel *panel,
+			      struct drm_display_mode *fixed_mode,
+			      struct drm_connector *connector);
 
 /* intel_pm.c */
 void intel_init_clock_gating(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 3deb58e..a589814 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -745,57 +745,6 @@  static const struct dmi_system_id intel_no_lvds[] = {
 	{ }	/* terminating entry */
 };
 
-/**
- * intel_find_lvds_downclock - find the reduced downclock for LVDS in EDID
- * @dev: drm device
- * @connector: LVDS connector
- *
- * Find the reduced downclock for LVDS in EDID.
- */
-static void intel_find_lvds_downclock(struct drm_device *dev,
-				      struct drm_display_mode *fixed_mode,
-				      struct drm_connector *connector)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_display_mode *scan;
-	int temp_downclock;
-
-	temp_downclock = fixed_mode->clock;
-	list_for_each_entry(scan, &connector->probed_modes, head) {
-		/*
-		 * If one mode has the same resolution with the fixed_panel
-		 * mode while they have the different refresh rate, it means
-		 * that the reduced downclock is found for the LVDS. In such
-		 * case we can set the different FPx0/1 to dynamically select
-		 * between low and high frequency.
-		 */
-		if (scan->hdisplay == fixed_mode->hdisplay &&
-		    scan->hsync_start == fixed_mode->hsync_start &&
-		    scan->hsync_end == fixed_mode->hsync_end &&
-		    scan->htotal == fixed_mode->htotal &&
-		    scan->vdisplay == fixed_mode->vdisplay &&
-		    scan->vsync_start == fixed_mode->vsync_start &&
-		    scan->vsync_end == fixed_mode->vsync_end &&
-		    scan->vtotal == fixed_mode->vtotal) {
-			if (scan->clock < temp_downclock) {
-				/*
-				 * The downclock is already found. But we
-				 * expect to find the lower downclock.
-				 */
-				temp_downclock = scan->clock;
-			}
-		}
-	}
-	if (temp_downclock < fixed_mode->clock && i915_lvds_downclock) {
-		/* We found the downclock for LVDS. */
-		dev_priv->lvds_downclock_avail = 1;
-		dev_priv->lvds_downclock = temp_downclock;
-		DRM_DEBUG_KMS("LVDS downclock is found in EDID. "
-			      "Normal clock %dKhz, downclock %dKhz\n",
-			      fixed_mode->clock, temp_downclock);
-	}
-}
-
 /*
  * Enumerate the child dev array parsed from VBT to check whether
  * the LVDS is present.
@@ -1073,8 +1022,22 @@  void intel_lvds_init(struct drm_device *dev)
 
 			fixed_mode = drm_mode_duplicate(dev, scan);
 			if (fixed_mode) {
-				intel_find_lvds_downclock(dev, fixed_mode,
-							  connector);
+				dev_priv->lvds_downclock_avail =
+					intel_find_panel_downclock(dev,
+					&intel_connector->panel,
+					fixed_mode, connector);
+				if (dev_priv->lvds_downclock_avail &&
+					i915_lvds_downclock) {
+					/* We found the downclock for LVDS. */
+					dev_priv->lvds_downclock =
+						intel_connector->panel.
+						downclock_mode->clock;
+					DRM_DEBUG_KMS("LVDS downclock is found"
+					" in EDID. Normal clock %dKhz, "
+					"downclock %dKhz\n",
+					fixed_mode->clock,
+					dev_priv->lvds_downclock);
+				}
 				goto out;
 			}
 		}
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index e480cf4..f49a136 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1104,6 +1104,62 @@  void intel_panel_destroy_backlight(struct drm_connector *connector)
 	intel_backlight_device_unregister(intel_connector);
 }
 
+/**
+ * intel_find_panel_downclock - find the reduced downclock for LVDS in EDID
+ * @dev: drm device
+ * @panel: intel panel
+ * @fixed_mode : panel native mode
+ * @connector: LVDS/eDP connector
+ *
+ * Return downclock_avail
+ * Find the reduced downclock for LVDS/eDP in EDID.
+ */
+bool intel_find_panel_downclock(struct drm_device *dev,
+			      struct intel_panel *panel,
+			      struct drm_display_mode *fixed_mode,
+			      struct drm_connector *connector)
+{
+	struct drm_display_mode *scan, *tmp_mode;
+	int temp_downclock;
+	bool ret = false;
+
+	temp_downclock = fixed_mode->clock;
+	tmp_mode = NULL;
+	panel->downclock_mode = NULL;
+
+	list_for_each_entry(scan, &connector->probed_modes, head) {
+		/*
+		 * If one mode has the same resolution with the fixed_panel
+		 * mode while they have the different refresh rate, it means
+		 * that the reduced downclock is found. In such
+		 * case we can set the different FPx0/1 to dynamically select
+		 * between low and high frequency.
+		 */
+		if (scan->hdisplay == fixed_mode->hdisplay &&
+		    scan->hsync_start == fixed_mode->hsync_start &&
+		    scan->hsync_end == fixed_mode->hsync_end &&
+		    scan->vdisplay == fixed_mode->vdisplay &&
+		    scan->vsync_start == fixed_mode->vsync_start &&
+		    scan->vsync_end == fixed_mode->vsync_end) {
+			if (scan->clock < temp_downclock) {
+				/*
+				 * The downclock is already found. But we
+				 * expect to find the lower downclock.
+				 */
+				temp_downclock = scan->clock;
+				tmp_mode = scan;
+			}
+		}
+	}
+
+	if (temp_downclock < fixed_mode->clock) {
+		panel->downclock_mode = drm_mode_duplicate(dev, tmp_mode);
+		ret = true;
+	}
+
+	return ret;
+}
+
 /* Set up chip specific backlight functions */
 void intel_panel_init_backlight_funcs(struct drm_device *dev)
 {