[PATCH] drm/i915: Make downclock deduction common for all panels

[PATCH] drm/i915: Make downclock deduction common for all panels

[Vandana Kannan]

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)
+{
+	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)
 {
-- 
1.7.9.5

Re: [PATCH] drm/i915: Make downclock deduction common for all panels

[Jani Nikula]

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

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] drm/i915: Make downclock deduction common for all panels

[Vandana Kannan]

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 |   57 +++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5dea389..9f8b465 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 struct drm_display_mode *intel_find_panel_downclock(
+				struct drm_device *dev,
+				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..364a47e 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);
+				intel_connector->panel.downclock_mode =
+					intel_find_panel_downclock(dev,
+					fixed_mode, connector);
+				if (intel_connector->panel.downclock_mode !=
+					NULL &&	i915_lvds_downclock) {
+					/* We found the downclock for LVDS. */
+					dev_priv->lvds_downclock_avail = true;
+					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..b0f6e6c 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1104,6 +1104,59 @@ 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
+ * @fixed_mode : panel native mode
+ * @connector: LVDS/eDP connector
+ *
+ * Return downclock_avail
+ * Find the reduced downclock for LVDS/eDP in EDID.
+ */
+struct drm_display_mode *
+intel_find_panel_downclock(struct drm_device *dev,
+			struct drm_display_mode *fixed_mode,
+			struct drm_connector *connector)
+{
+	struct drm_display_mode *scan, *tmp_mode;
+	int temp_downclock;
+
+	temp_downclock = fixed_mode->clock;
+	tmp_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->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;
+				tmp_mode = scan;
+			}
+		}
+	}
+
+	if (temp_downclock < fixed_mode->clock)
+		return drm_mode_duplicate(dev, tmp_mode);
+	else
+		return NULL;
+}
+
 /* Set up chip specific backlight functions */
 void intel_panel_init_backlight_funcs(struct drm_device *dev)
 {
@@ -1157,4 +1210,8 @@ void intel_panel_fini(struct intel_panel *panel)
 
 	if (panel->fixed_mode)
 		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
+
+	if (panel->downclock_mode)
+		drm_mode_destroy(intel_connector->base.dev,
+				panel->downclock_mode);
 }
-- 
1.7.9.5

Re: [PATCH] drm/i915: Make downclock deduction common for all panels

[Jani Nikula]

On Tue, 10 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.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> 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 |   57 +++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5dea389..9f8b465 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 struct drm_display_mode *intel_find_panel_downclock(
> +				struct drm_device *dev,
> +				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..364a47e 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);
> +				intel_connector->panel.downclock_mode =
> +					intel_find_panel_downclock(dev,
> +					fixed_mode, connector);
> +				if (intel_connector->panel.downclock_mode !=
> +					NULL &&	i915_lvds_downclock) {
> +					/* We found the downclock for LVDS. */
> +					dev_priv->lvds_downclock_avail = true;
> +					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..b0f6e6c 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1104,6 +1104,59 @@ 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
> + * @fixed_mode : panel native mode
> + * @connector: LVDS/eDP connector
> + *
> + * Return downclock_avail
> + * Find the reduced downclock for LVDS/eDP in EDID.
> + */
> +struct drm_display_mode *
> +intel_find_panel_downclock(struct drm_device *dev,
> +			struct drm_display_mode *fixed_mode,
> +			struct drm_connector *connector)
> +{
> +	struct drm_display_mode *scan, *tmp_mode;
> +	int temp_downclock;
> +
> +	temp_downclock = fixed_mode->clock;
> +	tmp_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->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;
> +				tmp_mode = scan;
> +			}
> +		}
> +	}
> +
> +	if (temp_downclock < fixed_mode->clock)
> +		return drm_mode_duplicate(dev, tmp_mode);
> +	else
> +		return NULL;
> +}
> +
>  /* Set up chip specific backlight functions */
>  void intel_panel_init_backlight_funcs(struct drm_device *dev)
>  {
> @@ -1157,4 +1210,8 @@ void intel_panel_fini(struct intel_panel *panel)
>  
>  	if (panel->fixed_mode)
>  		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
> +
> +	if (panel->downclock_mode)
> +		drm_mode_destroy(intel_connector->base.dev,
> +				panel->downclock_mode);
>  }
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915: Make downclock deduction common for all panels

[Daniel Vetter]

On Tue, Dec 10, 2013 at 12:36:34PM +0200, Jani Nikula wrote:
> On Tue, 10 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.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch. When resending reworked patches
please _always_ mention the version number of the patch somewhere and keep
an patch revision log in the commit message. In there mention please
mention all the changes compared to the last revision, and if applicable
on who's review request you've done these changes.

E.g. for this patch it'd be:

v2: Adjust calling convention for intel_find_panel_downclock as suggested
by Jani.

That way it's easier for reviewers to figure out what changed and they
don't need to re-read the entire patch again, which helps to speed up the
process a bit.

Aside: This is even more important if you disagree with a review comment
and opted for a different approach. Then you should explain this in a
reply to the review and probably also mention the discussion (if it's an
important topic and not just a misunderstanding) in the commit message,
too.

Thanks, Daniel
> 
> > 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 |   57 +++++++++++++++++++++++++++++
> >  3 files changed, 78 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 5dea389..9f8b465 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 struct drm_display_mode *intel_find_panel_downclock(
> > +				struct drm_device *dev,
> > +				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..364a47e 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);
> > +				intel_connector->panel.downclock_mode =
> > +					intel_find_panel_downclock(dev,
> > +					fixed_mode, connector);
> > +				if (intel_connector->panel.downclock_mode !=
> > +					NULL &&	i915_lvds_downclock) {
> > +					/* We found the downclock for LVDS. */
> > +					dev_priv->lvds_downclock_avail = true;
> > +					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..b0f6e6c 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1104,6 +1104,59 @@ 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
> > + * @fixed_mode : panel native mode
> > + * @connector: LVDS/eDP connector
> > + *
> > + * Return downclock_avail
> > + * Find the reduced downclock for LVDS/eDP in EDID.
> > + */
> > +struct drm_display_mode *
> > +intel_find_panel_downclock(struct drm_device *dev,
> > +			struct drm_display_mode *fixed_mode,
> > +			struct drm_connector *connector)
> > +{
> > +	struct drm_display_mode *scan, *tmp_mode;
> > +	int temp_downclock;
> > +
> > +	temp_downclock = fixed_mode->clock;
> > +	tmp_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->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;
> > +				tmp_mode = scan;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (temp_downclock < fixed_mode->clock)
> > +		return drm_mode_duplicate(dev, tmp_mode);
> > +	else
> > +		return NULL;
> > +}
> > +
> >  /* Set up chip specific backlight functions */
> >  void intel_panel_init_backlight_funcs(struct drm_device *dev)
> >  {
> > @@ -1157,4 +1210,8 @@ void intel_panel_fini(struct intel_panel *panel)
> >  
> >  	if (panel->fixed_mode)
> >  		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
> > +
> > +	if (panel->downclock_mode)
> > +		drm_mode_destroy(intel_connector->base.dev,
> > +				panel->downclock_mode);
> >  }
> > -- 
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch