[PATCH 00/19] drm/gma500: Unify most of the lvds code

[PATCH 00/19] drm/gma500: Unify most of the lvds code

[Patrik Jakobsson]

Much of the lvds code for Poulsbo, Oaktrail and Cedarview are almost
identical so there is no need to keep three copies of it. Unify as much
as possible into one implementation. There are still things like the
init code that can be unified but that requires unifying other parts of
the driver first.

Patrik Jakobsson (19):
  drm/gma500: Unify *_lvds_get_max_backlight()
  drm/gma500: Unify *_lvds_set_backlight()
  drm/gma500: Unify *_lvds_set_power()
  drm/gma500: Unify *_lvds_mode_valid()
  drm/gma500: Unify *_lvds_encoder_dpms()
  drm/gma500: Unify *_intel_lvds_save()
  drm/gma500: Unify struct *_intel_lvds_priv
  drm/gma500: Unify *_intel_lvds_restore()
  drm/gma500: Unify *_intel_lvds_mode_fixup()
  drm/gma500: Unify *_intel_lvds_prepare()
  drm/gma500: Unify *_intel_lvds_commit()
  drm/gma500: Unify *_intel_lvds_mode_set()
  drm/gma500: Unify struct *_intel_lvds_helper_funcs
  drm/gma500: Unify *_intel_lvds_get_modes()
  drm/gma500: Unify struct *_intel_lvds_connector_helper_funcs
  drm/gma500: Use i2c_bus in gma_encoder for PSB
  drm/gma500: Unify *_intel_lvds_destroy()
  drm/gma500: Unify *_intel_lvds_set_property()
  drm/gma500: Unify struct *_intel_lvds_connector_funcs

 drivers/gpu/drm/gma500/Makefile         |   1 +
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 390 +-----------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 462 +++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  38 ++
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 112 +-----
 drivers/gpu/drm/gma500/psb_drv.h        |   1 -
 drivers/gpu/drm/gma500/psb_intel_drv.h  |   2 -
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 507 +-----------------------
 8 files changed, 530 insertions(+), 983 deletions(-)
 create mode 100644 drivers/gpu/drm/gma500/gma_lvds.c
 create mode 100644 drivers/gpu/drm/gma500/gma_lvds.h

-- 
2.36.1

[PATCH 01/19] drm/gma500: Unify *_lvds_get_max_backlight()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them into one. All
unified lvds code will live in gma_lvds.c.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/Makefile         |  1 +
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 27 +++----------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 34 +++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       | 12 +++++++++
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 23 ++--------------
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 36 ++++---------------------
 6 files changed, 57 insertions(+), 76 deletions(-)
 create mode 100644 drivers/gpu/drm/gma500/gma_lvds.c
 create mode 100644 drivers/gpu/drm/gma500/gma_lvds.h

diff --git a/drivers/gpu/drm/gma500/Makefile b/drivers/gpu/drm/gma500/Makefile
index 63012bf2485a..6c707b5d29dc 100644
--- a/drivers/gpu/drm/gma500/Makefile
+++ b/drivers/gpu/drm/gma500/Makefile
@@ -15,6 +15,7 @@ gma500_gfx-y += \
 	  gem.o \
 	  gma_device.o \
 	  gma_display.o \
+	  gma_lvds.o \
 	  gtt.o \
 	  intel_bios.o \
 	  intel_gmbus.o \
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index be6efcaaa3b3..0c7c4a539e50 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -20,6 +20,7 @@
 #include "psb_drv.h"
 #include "psb_intel_drv.h"
 #include "psb_intel_reg.h"
+#include "gma_lvds.h"
 
 /*
  * LVDS I2C backlight control macros
@@ -52,32 +53,10 @@ struct cdv_intel_lvds_priv {
 	uint32_t saveBLC_PWM_CTL;
 };
 
-/*
- * Returns the maximum level of the backlight duty cycle field.
- */
-static u32 cdv_intel_lvds_get_max_backlight(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 retval;
-
-	if (gma_power_begin(dev, false)) {
-		retval = ((REG_READ(BLC_PWM_CTL) &
-			  BACKLIGHT_MODULATION_FREQ_MASK) >>
-			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
-
-		gma_power_end(dev);
-	} else
-		retval = ((dev_priv->regs.saveBLC_PWM_CTL &
-			  BACKLIGHT_MODULATION_FREQ_MASK) >>
-			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
-
-	return retval;
-}
-
 /*
  * Sets the backlight level.
  *
- * level backlight level, from 0 to cdv_intel_lvds_get_max_backlight().
+ * level backlight level, from 0 to gma_lvds_get_max_backlight().
  */
 static void cdv_intel_lvds_set_backlight(struct drm_device *dev, int level)
 {
@@ -250,7 +229,7 @@ static void cdv_intel_lvds_commit(struct drm_encoder *encoder)
 
 	if (mode_dev->backlight_duty_cycle == 0)
 		mode_dev->backlight_duty_cycle =
-		    cdv_intel_lvds_get_max_backlight(dev);
+		    gma_lvds_get_max_backlight(dev);
 
 	cdv_intel_lvds_set_power(dev, encoder, true);
 }
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
new file mode 100644
index 000000000000..0b646c7c7432
--- /dev/null
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright © 2006-2011 Intel Corporation
+ */
+
+#include "psb_drv.h"
+#include "psb_intel_drv.h"
+#include "power.h"
+#include "psb_intel_reg.h"
+
+/*
+ * Returns the maximum level of the backlight duty cycle field.
+ */
+u32 gma_lvds_get_max_backlight(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	u32 retval;
+
+	if (gma_power_begin(dev, false)) {
+		retval = ((REG_READ(BLC_PWM_CTL) &
+			  BACKLIGHT_MODULATION_FREQ_MASK) >>
+			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
+
+		gma_power_end(dev);
+	} else
+		retval = ((dev_priv->regs.saveBLC_PWM_CTL &
+			  BACKLIGHT_MODULATION_FREQ_MASK) >>
+			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
+
+	return retval;
+}
+
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
new file mode 100644
index 000000000000..2a9ce8ee3fa7
--- /dev/null
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/*
+ * Copyright © 2006-2011 Intel Corporation
+ */
+
+#ifndef _GMA_LVDS_H
+#define _GMA_LVDS_H
+
+u32 gma_lvds_get_max_backlight(struct drm_device *dev);
+
+#endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 9c9ebf8e29c4..4913baca7ae2 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -20,6 +20,7 @@
 #include "psb_drv.h"
 #include "psb_intel_drv.h"
 #include "psb_intel_reg.h"
+#include "gma_lvds.h"
 
 /* The max/min PWM frequency in BPCR[31:17] - */
 /* The smallest number is 1 (not 0) that can fit in the
@@ -170,25 +171,6 @@ static void oaktrail_lvds_prepare(struct drm_encoder *encoder)
 	gma_power_end(dev);
 }
 
-static u32 oaktrail_lvds_get_max_backlight(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 ret;
-
-	if (gma_power_begin(dev, false)) {
-		ret = ((REG_READ(BLC_PWM_CTL) &
-			  BACKLIGHT_MODULATION_FREQ_MASK) >>
-			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
-
-		gma_power_end(dev);
-	} else
-		ret = ((dev_priv->regs.saveBLC_PWM_CTL &
-			  BACKLIGHT_MODULATION_FREQ_MASK) >>
-			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
-
-	return ret;
-}
-
 static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
@@ -197,8 +179,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
 
 	if (mode_dev->backlight_duty_cycle == 0)
-		mode_dev->backlight_duty_cycle =
-					oaktrail_lvds_get_max_backlight(dev);
+		mode_dev->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
 	oaktrail_lvds_set_power(dev, gma_encoder, true);
 }
 
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 7ee6c8ce103b..371c202a15ce 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -18,6 +18,7 @@
 #include "psb_drv.h"
 #include "psb_intel_drv.h"
 #include "psb_intel_reg.h"
+#include "gma_lvds.h"
 
 /*
  * LVDS I2C backlight control macros
@@ -52,32 +53,6 @@ struct psb_intel_lvds_priv {
 	struct gma_i2c_chan *i2c_bus;
 };
 
-
-/*
- * Returns the maximum level of the backlight duty cycle field.
- */
-static u32 psb_intel_lvds_get_max_backlight(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 ret;
-
-	if (gma_power_begin(dev, false)) {
-		ret = REG_READ(BLC_PWM_CTL);
-		gma_power_end(dev);
-	} else /* Powered off, use the saved value */
-		ret = dev_priv->regs.saveBLC_PWM_CTL;
-
-	/* Top 15bits hold the frequency mask */
-	ret = (ret &  BACKLIGHT_MODULATION_FREQ_MASK) >>
-					BACKLIGHT_MODULATION_FREQ_SHIFT;
-
-        ret *= 2;	/* Return a 16bit range as needed for setting */
-        if (ret == 0)
-                dev_err(dev->dev, "BL bug: Reg %08x save %08X\n",
-                        REG_READ(BLC_PWM_CTL), dev_priv->regs.saveBLC_PWM_CTL);
-	return ret;
-}
-
 /*
  * Set LVDS backlight level by I2C command
  *
@@ -131,7 +106,7 @@ static int psb_lvds_pwm_set_brightness(struct drm_device *dev, int level)
 	u32 max_pwm_blc;
 	u32 blc_pwm_duty_cycle;
 
-	max_pwm_blc = psb_intel_lvds_get_max_backlight(dev);
+	max_pwm_blc = gma_lvds_get_max_backlight(dev);
 
 	/*BLC_PWM_CTL Should be initiated while backlight device init*/
 	BUG_ON(max_pwm_blc == 0);
@@ -176,7 +151,7 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 /*
  * Sets the backlight level.
  *
- * level: backlight level, from 0 to psb_intel_lvds_get_max_backlight().
+ * level: backlight level, from 0 to gma_lvds_get_max_backlight().
  */
 static void psb_intel_lvds_set_backlight(struct drm_device *dev, int level)
 {
@@ -275,8 +250,7 @@ static void psb_intel_lvds_save(struct drm_connector *connector)
 	 * just make it full brightness
 	 */
 	if (dev_priv->backlight_duty_cycle == 0)
-		dev_priv->backlight_duty_cycle =
-		psb_intel_lvds_get_max_backlight(dev);
+		dev_priv->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
 
 	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
 			lvds_priv->savePP_ON,
@@ -445,7 +419,7 @@ static void psb_intel_lvds_commit(struct drm_encoder *encoder)
 
 	if (mode_dev->backlight_duty_cycle == 0)
 		mode_dev->backlight_duty_cycle =
-		    psb_intel_lvds_get_max_backlight(dev);
+		    gma_lvds_get_max_backlight(dev);
 
 	psb_intel_lvds_set_power(dev, true);
 }
-- 
2.36.1

[PATCH 02/19] drm/gma500: Unify *_lvds_set_backlight()

[Patrik Jakobsson]

These functions do the same thing so unify them into one.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 29 ++--------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 26 ++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 32 ++-----------------------
 4 files changed, 31 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 0c7c4a539e50..615570de82b0 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -53,31 +53,6 @@ struct cdv_intel_lvds_priv {
 	uint32_t saveBLC_PWM_CTL;
 };
 
-/*
- * Sets the backlight level.
- *
- * level backlight level, from 0 to gma_lvds_get_max_backlight().
- */
-static void cdv_intel_lvds_set_backlight(struct drm_device *dev, int level)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 blc_pwm_ctl;
-
-	if (gma_power_begin(dev, false)) {
-		blc_pwm_ctl =
-			REG_READ(BLC_PWM_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
-		REG_WRITE(BLC_PWM_CTL,
-				(blc_pwm_ctl |
-				(level << BACKLIGHT_DUTY_CYCLE_SHIFT)));
-		gma_power_end(dev);
-	} else {
-		blc_pwm_ctl = dev_priv->regs.saveBLC_PWM_CTL &
-				~BACKLIGHT_DUTY_CYCLE_MASK;
-		dev_priv->regs.saveBLC_PWM_CTL = (blc_pwm_ctl |
-					(level << BACKLIGHT_DUTY_CYCLE_SHIFT));
-	}
-}
-
 /*
  * Sets the power state for the panel.
  */
@@ -97,10 +72,10 @@ static void cdv_intel_lvds_set_power(struct drm_device *dev,
 			pp_status = REG_READ(PP_STATUS);
 		} while ((pp_status & PP_ON) == 0);
 
-		cdv_intel_lvds_set_backlight(dev,
+		gma_lvds_set_backlight(dev,
 				dev_priv->mode_dev.backlight_duty_cycle);
 	} else {
-		cdv_intel_lvds_set_backlight(dev, 0);
+		gma_lvds_set_backlight(dev, 0);
 
 		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
 			  ~POWER_TARGET_ON);
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 0b646c7c7432..5b041fab82ba 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -31,4 +31,30 @@ u32 gma_lvds_get_max_backlight(struct drm_device *dev)
 	return retval;
 }
 
+/*
+ * Sets the backlight level.
+ *
+ * level: backlight level, from 0 to gma_lvds_get_max_backlight().
+ */
+void gma_lvds_set_backlight(struct drm_device *dev, int level)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	u32 blc_pwm_ctl;
+
+	if (gma_power_begin(dev, false)) {
+		blc_pwm_ctl = REG_READ(BLC_PWM_CTL);
+		blc_pwm_ctl &= ~BACKLIGHT_DUTY_CYCLE_MASK;
+		REG_WRITE(BLC_PWM_CTL,
+				(blc_pwm_ctl |
+				(level << BACKLIGHT_DUTY_CYCLE_SHIFT)));
+		dev_priv->regs.saveBLC_PWM_CTL = (blc_pwm_ctl |
+					(level << BACKLIGHT_DUTY_CYCLE_SHIFT));
+		gma_power_end(dev);
+	} else {
+		blc_pwm_ctl = dev_priv->regs.saveBLC_PWM_CTL &
+				~BACKLIGHT_DUTY_CYCLE_MASK;
+		dev_priv->regs.saveBLC_PWM_CTL = (blc_pwm_ctl |
+					(level << BACKLIGHT_DUTY_CYCLE_SHIFT));
+	}
+}
 
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 2a9ce8ee3fa7..f26cc69b6caa 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -8,5 +8,6 @@
 #define _GMA_LVDS_H
 
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
+void gma_lvds_set_backlight(struct drm_device *dev, int level);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 371c202a15ce..a304f840b127 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -148,33 +148,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-/*
- * Sets the backlight level.
- *
- * level: backlight level, from 0 to gma_lvds_get_max_backlight().
- */
-static void psb_intel_lvds_set_backlight(struct drm_device *dev, int level)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 blc_pwm_ctl;
-
-	if (gma_power_begin(dev, false)) {
-		blc_pwm_ctl = REG_READ(BLC_PWM_CTL);
-		blc_pwm_ctl &= ~BACKLIGHT_DUTY_CYCLE_MASK;
-		REG_WRITE(BLC_PWM_CTL,
-				(blc_pwm_ctl |
-				(level << BACKLIGHT_DUTY_CYCLE_SHIFT)));
-		dev_priv->regs.saveBLC_PWM_CTL = (blc_pwm_ctl |
-					(level << BACKLIGHT_DUTY_CYCLE_SHIFT));
-		gma_power_end(dev);
-	} else {
-		blc_pwm_ctl = dev_priv->regs.saveBLC_PWM_CTL &
-				~BACKLIGHT_DUTY_CYCLE_MASK;
-		dev_priv->regs.saveBLC_PWM_CTL = (blc_pwm_ctl |
-					(level << BACKLIGHT_DUTY_CYCLE_SHIFT));
-	}
-}
-
 /*
  * Sets the power state for the panel.
  */
@@ -196,10 +169,9 @@ static void psb_intel_lvds_set_power(struct drm_device *dev, bool on)
 			pp_status = REG_READ(PP_STATUS);
 		} while ((pp_status & PP_ON) == 0);
 
-		psb_intel_lvds_set_backlight(dev,
-					     mode_dev->backlight_duty_cycle);
+		gma_lvds_set_backlight(dev, mode_dev->backlight_duty_cycle);
 	} else {
-		psb_intel_lvds_set_backlight(dev, 0);
+		gma_lvds_set_backlight(dev, 0);
 
 		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
 			  ~POWER_TARGET_ON);
-- 
2.36.1

[PATCH 03/19] drm/gma500: Unify *_lvds_set_power()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them into one.
Oaktrail doesn't power on/off the backlight so don't touch that. Ignore
runtime-pm stuff since runtime-pm is broken anyways.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 41 +++------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 37 ++++++++++++++++++-
 drivers/gpu/drm/gma500/gma_lvds.h       |  2 +-
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 47 +++----------------------
 drivers/gpu/drm/gma500/psb_drv.h        |  1 -
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 43 +++-------------------
 6 files changed, 49 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 615570de82b0..7bf883bb8104 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -53,46 +53,13 @@ struct cdv_intel_lvds_priv {
 	uint32_t saveBLC_PWM_CTL;
 };
 
-/*
- * Sets the power state for the panel.
- */
-static void cdv_intel_lvds_set_power(struct drm_device *dev,
-				     struct drm_encoder *encoder, bool on)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 pp_status;
-
-	if (!gma_power_begin(dev, true))
-		return;
-
-	if (on) {
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) |
-			  POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while ((pp_status & PP_ON) == 0);
-
-		gma_lvds_set_backlight(dev,
-				dev_priv->mode_dev.backlight_duty_cycle);
-	} else {
-		gma_lvds_set_backlight(dev, 0);
-
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
-			  ~POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while (pp_status & PP_ON);
-	}
-	gma_power_end(dev);
-}
-
 static void cdv_intel_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct drm_device *dev = encoder->dev;
 	if (mode == DRM_MODE_DPMS_ON)
-		cdv_intel_lvds_set_power(dev, encoder, true);
+		gma_lvds_set_power(dev, true);
 	else
-		cdv_intel_lvds_set_power(dev, encoder, false);
+		gma_lvds_set_power(dev, false);
 	/* XXX: We never power down the LVDS pairs. */
 }
 
@@ -191,7 +158,7 @@ static void cdv_intel_lvds_prepare(struct drm_encoder *encoder)
 	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
 					  BACKLIGHT_DUTY_CYCLE_MASK);
 
-	cdv_intel_lvds_set_power(dev, encoder, false);
+	gma_lvds_set_power(dev, false);
 
 	gma_power_end(dev);
 }
@@ -206,7 +173,7 @@ static void cdv_intel_lvds_commit(struct drm_encoder *encoder)
 		mode_dev->backlight_duty_cycle =
 		    gma_lvds_get_max_backlight(dev);
 
-	cdv_intel_lvds_set_power(dev, encoder, true);
+	gma_lvds_set_power(dev, true);
 }
 
 static void cdv_intel_lvds_mode_set(struct drm_encoder *encoder,
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 5b041fab82ba..11efbb14b55c 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -36,7 +36,7 @@ u32 gma_lvds_get_max_backlight(struct drm_device *dev)
  *
  * level: backlight level, from 0 to gma_lvds_get_max_backlight().
  */
-void gma_lvds_set_backlight(struct drm_device *dev, int level)
+static void gma_lvds_set_backlight(struct drm_device *dev, int level)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	u32 blc_pwm_ctl;
@@ -58,3 +58,38 @@ void gma_lvds_set_backlight(struct drm_device *dev, int level)
 	}
 }
 
+/*
+ * Sets the power state for the panel.
+ */
+void gma_lvds_set_power(struct drm_device *dev, bool on)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	u32 pp_status;
+
+	if (!gma_power_begin(dev, true))
+		return;
+
+	if (on) {
+		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) |
+			  POWER_TARGET_ON);
+		do {
+			pp_status = REG_READ(PP_STATUS);
+		} while ((pp_status & PP_ON) == 0);
+
+		if (!IS_MRST(dev)) {
+			gma_lvds_set_backlight(dev,
+				dev_priv->mode_dev.backlight_duty_cycle);
+		}
+	} else {
+		if (!IS_MRST(dev))
+			gma_lvds_set_backlight(dev, 0);
+
+		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
+			  ~POWER_TARGET_ON);
+		do {
+			pp_status = REG_READ(PP_STATUS);
+		} while (pp_status & PP_ON);
+	}
+	gma_power_end(dev);
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index f26cc69b6caa..477d3b5005f7 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -8,6 +8,6 @@
 #define _GMA_LVDS_H
 
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
-void gma_lvds_set_backlight(struct drm_device *dev, int level);
+void gma_lvds_set_power(struct drm_device *dev, bool on);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 4913baca7ae2..9634807e4d8c 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -30,51 +30,14 @@
 #define MRST_BLC_MAX_PWM_REG_FREQ	    0xFFFF
 #define BRIGHTNESS_MAX_LEVEL 100
 
-/*
- * Sets the power state for the panel.
- */
-static void oaktrail_lvds_set_power(struct drm_device *dev,
-				struct gma_encoder *gma_encoder,
-				bool on)
-{
-	u32 pp_status;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-
-	if (!gma_power_begin(dev, true))
-		return;
-
-	if (on) {
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) |
-			  POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while ((pp_status & (PP_ON | PP_READY)) == PP_READY);
-		dev_priv->is_lvds_on = true;
-		if (dev_priv->ops->lvds_bl_power)
-			dev_priv->ops->lvds_bl_power(dev, true);
-	} else {
-		if (dev_priv->ops->lvds_bl_power)
-			dev_priv->ops->lvds_bl_power(dev, false);
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
-			  ~POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while (pp_status & PP_ON);
-		dev_priv->is_lvds_on = false;
-		pm_request_idle(dev->dev);
-	}
-	gma_power_end(dev);
-}
-
 static void oaktrail_lvds_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct drm_device *dev = encoder->dev;
-	struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
 
 	if (mode == DRM_MODE_DPMS_ON)
-		oaktrail_lvds_set_power(dev, gma_encoder, true);
+		gma_lvds_set_power(dev, true);
 	else
-		oaktrail_lvds_set_power(dev, gma_encoder, false);
+		gma_lvds_set_power(dev, false);
 
 	/* XXX: We never power down the LVDS pairs. */
 }
@@ -158,7 +121,6 @@ static void oaktrail_lvds_prepare(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
 	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
 
 	if (!gma_power_begin(dev, true))
@@ -167,7 +129,7 @@ static void oaktrail_lvds_prepare(struct drm_encoder *encoder)
 	mode_dev->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
 	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
 					  BACKLIGHT_DUTY_CYCLE_MASK);
-	oaktrail_lvds_set_power(dev, gma_encoder, false);
+	gma_lvds_set_power(dev, false);
 	gma_power_end(dev);
 }
 
@@ -175,12 +137,11 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
 	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
 
 	if (mode_dev->backlight_duty_cycle == 0)
 		mode_dev->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
-	oaktrail_lvds_set_power(dev, gma_encoder, true);
+	gma_lvds_set_power(dev, true);
 }
 
 static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index 0ea3d23575f3..2789ae9efe3c 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -601,7 +601,6 @@ struct psb_ops {
 	void (*update_wm)(struct drm_device *dev, struct drm_crtc *crtc);
 	void (*disable_sr)(struct drm_device *dev);
 
-	void (*lvds_bl_power)(struct drm_device *dev, bool on);
 #ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
 	/* Backlight */
 	int (*backlight_init)(struct drm_device *dev);
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index a304f840b127..06f1bd2250dd 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -148,49 +148,14 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-/*
- * Sets the power state for the panel.
- */
-static void psb_intel_lvds_set_power(struct drm_device *dev, bool on)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-	u32 pp_status;
-
-	if (!gma_power_begin(dev, true)) {
-	        dev_err(dev->dev, "set power, chip off!\n");
-		return;
-        }
-
-	if (on) {
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) |
-			  POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while ((pp_status & PP_ON) == 0);
-
-		gma_lvds_set_backlight(dev, mode_dev->backlight_duty_cycle);
-	} else {
-		gma_lvds_set_backlight(dev, 0);
-
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
-			  ~POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while (pp_status & PP_ON);
-	}
-
-	gma_power_end(dev);
-}
-
 static void psb_intel_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct drm_device *dev = encoder->dev;
 
 	if (mode == DRM_MODE_DPMS_ON)
-		psb_intel_lvds_set_power(dev, true);
+		gma_lvds_set_power(dev, true);
 	else
-		psb_intel_lvds_set_power(dev, false);
+		gma_lvds_set_power(dev, false);
 
 	/* XXX: We never power down the LVDS pairs. */
 }
@@ -378,7 +343,7 @@ static void psb_intel_lvds_prepare(struct drm_encoder *encoder)
 	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
 					  BACKLIGHT_DUTY_CYCLE_MASK);
 
-	psb_intel_lvds_set_power(dev, false);
+	gma_lvds_set_power(dev, false);
 
 	gma_power_end(dev);
 }
@@ -393,7 +358,7 @@ static void psb_intel_lvds_commit(struct drm_encoder *encoder)
 		mode_dev->backlight_duty_cycle =
 		    gma_lvds_get_max_backlight(dev);
 
-	psb_intel_lvds_set_power(dev, true);
+	gma_lvds_set_power(dev, true);
 }
 
 static void psb_intel_lvds_mode_set(struct drm_encoder *encoder,
-- 
2.36.1

[PATCH 04/19] drm/gma500: Unify *_lvds_mode_valid()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them into one. Skip
the INTEL_OUTPUT_MIPI2 code since we don't have that output type.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 27 +---------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 25 +++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  2 ++
 drivers/gpu/drm/gma500/psb_intel_drv.h  |  2 --
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 30 +------------------------
 5 files changed, 29 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 7bf883bb8104..968d627e23d1 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -71,31 +71,6 @@ static void cdv_intel_lvds_restore(struct drm_connector *connector)
 {
 }
 
-static enum drm_mode_status cdv_intel_lvds_mode_valid(struct drm_connector *connector,
-			      struct drm_display_mode *mode)
-{
-	struct drm_device *dev = connector->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct drm_display_mode *fixed_mode =
-					dev_priv->mode_dev.panel_fixed_mode;
-
-	/* just in case */
-	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
-		return MODE_NO_DBLESCAN;
-
-	/* just in case */
-	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-		return MODE_NO_INTERLACE;
-
-	if (fixed_mode) {
-		if (mode->hdisplay > fixed_mode->hdisplay)
-			return MODE_PANEL;
-		if (mode->vdisplay > fixed_mode->vdisplay)
-			return MODE_PANEL;
-	}
-	return MODE_OK;
-}
-
 static bool cdv_intel_lvds_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
@@ -321,7 +296,7 @@ static const struct drm_encoder_helper_funcs
 static const struct drm_connector_helper_funcs
 				cdv_intel_lvds_connector_helper_funcs = {
 	.get_modes = cdv_intel_lvds_get_modes,
-	.mode_valid = cdv_intel_lvds_mode_valid,
+	.mode_valid = gma_lvds_mode_valid,
 	.best_encoder = gma_best_encoder,
 };
 
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 11efbb14b55c..c36815493366 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -93,3 +93,28 @@ void gma_lvds_set_power(struct drm_device *dev, bool on)
 	gma_power_end(dev);
 }
 
+enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
+					 struct drm_display_mode *mode)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct drm_display_mode *fixed_mode =
+					dev_priv->mode_dev.panel_fixed_mode;
+
+	/* just in case */
+	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
+		return MODE_NO_DBLESCAN;
+
+	/* just in case */
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+		return MODE_NO_INTERLACE;
+
+	if (fixed_mode) {
+		if (mode->hdisplay > fixed_mode->hdisplay)
+			return MODE_PANEL;
+		if (mode->vdisplay > fixed_mode->vdisplay)
+			return MODE_PANEL;
+	}
+	return MODE_OK;
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 477d3b5005f7..6b4d8a024da1 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -9,5 +9,7 @@
 
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
 void gma_lvds_set_power(struct drm_device *dev, bool on);
+enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
+					 struct drm_display_mode *mode);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h b/drivers/gpu/drm/gma500/psb_intel_drv.h
index 8ccba116821b..db824aa6b589 100644
--- a/drivers/gpu/drm/gma500/psb_intel_drv.h
+++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
@@ -228,8 +228,6 @@ extern int intelfb_remove(struct drm_device *dev,
 extern bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
 				      const struct drm_display_mode *mode,
 				      struct drm_display_mode *adjusted_mode);
-extern enum drm_mode_status psb_intel_lvds_mode_valid(struct drm_connector *connector,
-				     struct drm_display_mode *mode);
 extern int psb_intel_lvds_set_property(struct drm_connector *connector,
 					struct drm_property *property,
 					uint64_t value);
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 06f1bd2250dd..c88697a805e0 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -239,34 +239,6 @@ static void psb_intel_lvds_restore(struct drm_connector *connector)
 	}
 }
 
-enum drm_mode_status psb_intel_lvds_mode_valid(struct drm_connector *connector,
-				 struct drm_display_mode *mode)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(connector->dev);
-	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
-	struct drm_display_mode *fixed_mode =
-					dev_priv->mode_dev.panel_fixed_mode;
-
-	if (gma_encoder->type == INTEL_OUTPUT_MIPI2)
-		fixed_mode = dev_priv->mode_dev.panel_fixed_mode2;
-
-	/* just in case */
-	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
-		return MODE_NO_DBLESCAN;
-
-	/* just in case */
-	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-		return MODE_NO_INTERLACE;
-
-	if (fixed_mode) {
-		if (mode->hdisplay > fixed_mode->hdisplay)
-			return MODE_PANEL;
-		if (mode->vdisplay > fixed_mode->vdisplay)
-			return MODE_PANEL;
-	}
-	return MODE_OK;
-}
-
 bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
@@ -509,7 +481,7 @@ static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
 const struct drm_connector_helper_funcs
 				psb_intel_lvds_connector_helper_funcs = {
 	.get_modes = psb_intel_lvds_get_modes,
-	.mode_valid = psb_intel_lvds_mode_valid,
+	.mode_valid = gma_lvds_mode_valid,
 	.best_encoder = gma_best_encoder,
 };
 
-- 
2.36.1

[PATCH 05/19] drm/gma500: Unify *_lvds_encoder_dpms()

[Patrik Jakobsson]

These functions are identical so unify them into one.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 12 +-----------
 drivers/gpu/drm/gma500/gma_lvds.c       | 11 +++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 14 +-------------
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 14 +-------------
 5 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 968d627e23d1..59d30fa0eb53 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -53,16 +53,6 @@ struct cdv_intel_lvds_priv {
 	uint32_t saveBLC_PWM_CTL;
 };
 
-static void cdv_intel_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct drm_device *dev = encoder->dev;
-	if (mode == DRM_MODE_DPMS_ON)
-		gma_lvds_set_power(dev, true);
-	else
-		gma_lvds_set_power(dev, false);
-	/* XXX: We never power down the LVDS pairs. */
-}
-
 static void cdv_intel_lvds_save(struct drm_connector *connector)
 {
 }
@@ -286,7 +276,7 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
 
 static const struct drm_encoder_helper_funcs
 					cdv_intel_lvds_helper_funcs = {
-	.dpms = cdv_intel_lvds_encoder_dpms,
+	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = cdv_intel_lvds_mode_fixup,
 	.prepare = cdv_intel_lvds_prepare,
 	.mode_set = cdv_intel_lvds_mode_set,
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index c36815493366..fb8f8bb599eb 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -118,3 +118,14 @@ enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
+void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
+{
+	struct drm_device *dev = encoder->dev;
+
+	if (mode == DRM_MODE_DPMS_ON)
+		gma_lvds_set_power(dev, true);
+	else
+		gma_lvds_set_power(dev, false);
+	/* XXX: We never power down the LVDS pairs. */
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 6b4d8a024da1..3babb522ee84 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -11,5 +11,6 @@ u32 gma_lvds_get_max_backlight(struct drm_device *dev);
 void gma_lvds_set_power(struct drm_device *dev, bool on);
 enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
 					 struct drm_display_mode *mode);
+void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 9634807e4d8c..00ec4fea4c12 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -30,18 +30,6 @@
 #define MRST_BLC_MAX_PWM_REG_FREQ	    0xFFFF
 #define BRIGHTNESS_MAX_LEVEL 100
 
-static void oaktrail_lvds_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct drm_device *dev = encoder->dev;
-
-	if (mode == DRM_MODE_DPMS_ON)
-		gma_lvds_set_power(dev, true);
-	else
-		gma_lvds_set_power(dev, false);
-
-	/* XXX: We never power down the LVDS pairs. */
-}
-
 static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
 			       struct drm_display_mode *mode,
 			       struct drm_display_mode *adjusted_mode)
@@ -145,7 +133,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 }
 
 static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
-	.dpms = oaktrail_lvds_dpms,
+	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = psb_intel_lvds_mode_fixup,
 	.prepare = oaktrail_lvds_prepare,
 	.mode_set = oaktrail_lvds_mode_set,
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index c88697a805e0..2470ab0e1e0e 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -148,18 +148,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-static void psb_intel_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct drm_device *dev = encoder->dev;
-
-	if (mode == DRM_MODE_DPMS_ON)
-		gma_lvds_set_power(dev, true);
-	else
-		gma_lvds_set_power(dev, false);
-
-	/* XXX: We never power down the LVDS pairs. */
-}
-
 static void psb_intel_lvds_save(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
@@ -471,7 +459,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
 }
 
 static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
-	.dpms = psb_intel_lvds_encoder_dpms,
+	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = psb_intel_lvds_mode_fixup,
 	.prepare = psb_intel_lvds_prepare,
 	.mode_set = psb_intel_lvds_mode_set,
-- 
2.36.1

[PATCH 06/19] drm/gma500: Unify *_intel_lvds_save()

[Patrik Jakobsson]

Cedarview never implemented this so use the Poulsbo version for both.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c |  6 +---
 drivers/gpu/drm/gma500/gma_lvds.c       | 38 +++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 40 +------------------------
 4 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 59d30fa0eb53..777eb7cf7d7f 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -53,10 +53,6 @@ struct cdv_intel_lvds_priv {
 	uint32_t saveBLC_PWM_CTL;
 };
 
-static void cdv_intel_lvds_save(struct drm_connector *connector)
-{
-}
-
 static void cdv_intel_lvds_restore(struct drm_connector *connector)
 {
 }
@@ -398,7 +394,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 	gma_encoder->dev_priv = lvds_priv;
 
 	connector = &gma_connector->base;
-	gma_connector->save = cdv_intel_lvds_save;
+	gma_connector->save = gma_lvds_save;
 	gma_connector->restore = cdv_intel_lvds_restore;
 	encoder = &gma_encoder->base;
 
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index fb8f8bb599eb..bd08ed049c5e 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -129,3 +129,41 @@ void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode)
 	/* XXX: We never power down the LVDS pairs. */
 }
 
+void gma_lvds_save(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
+	struct psb_intel_lvds_priv *lvds_priv =
+		(struct psb_intel_lvds_priv *)gma_encoder->dev_priv;
+
+	lvds_priv->savePP_ON = REG_READ(LVDSPP_ON);
+	lvds_priv->savePP_OFF = REG_READ(LVDSPP_OFF);
+	lvds_priv->saveLVDS = REG_READ(LVDS);
+	lvds_priv->savePP_CONTROL = REG_READ(PP_CONTROL);
+	lvds_priv->savePP_CYCLE = REG_READ(PP_CYCLE);
+	/*lvds_priv->savePP_DIVISOR = REG_READ(PP_DIVISOR);*/
+	lvds_priv->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
+	lvds_priv->savePFIT_CONTROL = REG_READ(PFIT_CONTROL);
+	lvds_priv->savePFIT_PGM_RATIOS = REG_READ(PFIT_PGM_RATIOS);
+
+	/*TODO: move backlight_duty_cycle to psb_intel_lvds_priv*/
+	dev_priv->backlight_duty_cycle = (dev_priv->regs.saveBLC_PWM_CTL &
+						BACKLIGHT_DUTY_CYCLE_MASK);
+
+	/*
+	 * If the light is off at server startup,
+	 * just make it full brightness
+	 */
+	if (dev_priv->backlight_duty_cycle == 0)
+		dev_priv->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
+
+	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
+			lvds_priv->savePP_ON,
+			lvds_priv->savePP_OFF,
+			lvds_priv->saveLVDS,
+			lvds_priv->savePP_CONTROL,
+			lvds_priv->savePP_CYCLE,
+			lvds_priv->saveBLC_PWM_CTL);
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 3babb522ee84..52422986be1e 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -12,5 +12,6 @@ void gma_lvds_set_power(struct drm_device *dev, bool on);
 enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
 					 struct drm_display_mode *mode);
 void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
+void gma_lvds_save(struct drm_connector *connector);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 2470ab0e1e0e..c00b00d70a30 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -148,44 +148,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-static void psb_intel_lvds_save(struct drm_connector *connector)
-{
-	struct drm_device *dev = connector->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
-	struct psb_intel_lvds_priv *lvds_priv =
-		(struct psb_intel_lvds_priv *)gma_encoder->dev_priv;
-
-	lvds_priv->savePP_ON = REG_READ(LVDSPP_ON);
-	lvds_priv->savePP_OFF = REG_READ(LVDSPP_OFF);
-	lvds_priv->saveLVDS = REG_READ(LVDS);
-	lvds_priv->savePP_CONTROL = REG_READ(PP_CONTROL);
-	lvds_priv->savePP_CYCLE = REG_READ(PP_CYCLE);
-	/*lvds_priv->savePP_DIVISOR = REG_READ(PP_DIVISOR);*/
-	lvds_priv->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
-	lvds_priv->savePFIT_CONTROL = REG_READ(PFIT_CONTROL);
-	lvds_priv->savePFIT_PGM_RATIOS = REG_READ(PFIT_PGM_RATIOS);
-
-	/*TODO: move backlight_duty_cycle to psb_intel_lvds_priv*/
-	dev_priv->backlight_duty_cycle = (dev_priv->regs.saveBLC_PWM_CTL &
-						BACKLIGHT_DUTY_CYCLE_MASK);
-
-	/*
-	 * If the light is off at server startup,
-	 * just make it full brightness
-	 */
-	if (dev_priv->backlight_duty_cycle == 0)
-		dev_priv->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
-
-	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
-			lvds_priv->savePP_ON,
-			lvds_priv->savePP_OFF,
-			lvds_priv->saveLVDS,
-			lvds_priv->savePP_CONTROL,
-			lvds_priv->savePP_CYCLE,
-			lvds_priv->saveBLC_PWM_CTL);
-}
-
 static void psb_intel_lvds_restore(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
@@ -526,7 +488,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 	gma_encoder->dev_priv = lvds_priv;
 
 	connector = &gma_connector->base;
-	gma_connector->save = psb_intel_lvds_save;
+	gma_connector->save = gma_lvds_save;
 	gma_connector->restore = psb_intel_lvds_restore;
 
 	/* Set up the DDC bus. */
-- 
2.36.1

[PATCH 07/19] drm/gma500: Unify struct *_intel_lvds_priv

[Patrik Jakobsson]

These structs are similar enough to be unified. This will allow unifying
the lvds functions that access the lvds encoder private.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 18 ++----------------
 drivers/gpu/drm/gma500/gma_lvds.c       |  5 +++--
 drivers/gpu/drm/gma500/gma_lvds.h       | 16 ++++++++++++++++
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 24 ++++--------------------
 4 files changed, 25 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 777eb7cf7d7f..b0e7b680f02b 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,20 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-struct cdv_intel_lvds_priv {
-	/**
-	 * Saved LVDO output states
-	 */
-	uint32_t savePP_ON;
-	uint32_t savePP_OFF;
-	uint32_t saveLVDS;
-	uint32_t savePP_CONTROL;
-	uint32_t savePP_CYCLE;
-	uint32_t savePFIT_CONTROL;
-	uint32_t savePFIT_PGM_RATIOS;
-	uint32_t saveBLC_PWM_CTL;
-};
-
 static void cdv_intel_lvds_restore(struct drm_connector *connector)
 {
 }
@@ -356,7 +342,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 {
 	struct gma_encoder *gma_encoder;
 	struct gma_connector *gma_connector;
-	struct cdv_intel_lvds_priv *lvds_priv;
+	struct gma_lvds_priv *lvds_priv;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_display_mode *scan;
@@ -387,7 +373,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 	if (!gma_connector)
 		goto err_free_encoder;
 
-	lvds_priv = kzalloc(sizeof(struct cdv_intel_lvds_priv), GFP_KERNEL);
+	lvds_priv = kzalloc(sizeof(*lvds_priv), GFP_KERNEL);
 	if (!lvds_priv)
 		goto err_free_connector;
 
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index bd08ed049c5e..7a81f44a40bd 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -8,6 +8,7 @@
 #include "psb_intel_drv.h"
 #include "power.h"
 #include "psb_intel_reg.h"
+#include "gma_lvds.h"
 
 /*
  * Returns the maximum level of the backlight duty cycle field.
@@ -134,8 +135,8 @@ void gma_lvds_save(struct drm_connector *connector)
 	struct drm_device *dev = connector->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
-	struct psb_intel_lvds_priv *lvds_priv =
-		(struct psb_intel_lvds_priv *)gma_encoder->dev_priv;
+	struct gma_lvds_priv *lvds_priv =
+		(struct gma_lvds_priv *)gma_encoder->dev_priv;
 
 	lvds_priv->savePP_ON = REG_READ(LVDSPP_ON);
 	lvds_priv->savePP_OFF = REG_READ(LVDSPP_OFF);
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 52422986be1e..98ad9bc878b7 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -7,6 +7,22 @@
 #ifndef _GMA_LVDS_H
 #define _GMA_LVDS_H
 
+struct gma_lvds_priv {
+	/*
+	 * Saved LVDO output states
+	 */
+	uint32_t savePP_ON;
+	uint32_t savePP_OFF;
+	uint32_t saveLVDS;
+	uint32_t savePP_CONTROL;
+	uint32_t savePP_CYCLE;
+	uint32_t savePFIT_CONTROL;
+	uint32_t savePFIT_PGM_RATIOS;
+	uint32_t saveBLC_PWM_CTL;
+
+	struct gma_i2c_chan *i2c_bus;
+};
+
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
 void gma_lvds_set_power(struct drm_device *dev, bool on);
 enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index c00b00d70a30..995e7aac53b9 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -37,22 +37,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-struct psb_intel_lvds_priv {
-	/*
-	 * Saved LVDO output states
-	 */
-	uint32_t savePP_ON;
-	uint32_t savePP_OFF;
-	uint32_t saveLVDS;
-	uint32_t savePP_CONTROL;
-	uint32_t savePP_CYCLE;
-	uint32_t savePFIT_CONTROL;
-	uint32_t savePFIT_PGM_RATIOS;
-	uint32_t saveBLC_PWM_CTL;
-
-	struct gma_i2c_chan *i2c_bus;
-};
-
 /*
  * Set LVDS backlight level by I2C command
  *
@@ -153,8 +137,8 @@ static void psb_intel_lvds_restore(struct drm_connector *connector)
 	struct drm_device *dev = connector->dev;
 	u32 pp_status;
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
-	struct psb_intel_lvds_priv *lvds_priv =
-		(struct psb_intel_lvds_priv *)gma_encoder->dev_priv;
+	struct gma_lvds_priv *lvds_priv =
+		(struct gma_lvds_priv *)gma_encoder->dev_priv;
 
 	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
 			lvds_priv->savePP_ON,
@@ -455,7 +439,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 {
 	struct gma_encoder *gma_encoder;
 	struct gma_connector *gma_connector;
-	struct psb_intel_lvds_priv *lvds_priv;
+	struct gma_lvds_priv *lvds_priv;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_display_mode *scan;	/* *modes, *bios_mode; */
@@ -479,7 +463,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 		goto err_free_encoder;
 	}
 
-	lvds_priv = kzalloc(sizeof(struct psb_intel_lvds_priv), GFP_KERNEL);
+	lvds_priv = kzalloc(sizeof(*lvds_priv), GFP_KERNEL);
 	if (!lvds_priv) {
 		dev_err(dev->dev, "LVDS private allocation error\n");
 		goto err_free_connector;
-- 
2.36.1

[PATCH 08/19] drm/gma500: Unify *_intel_lvds_restore()

[Patrik Jakobsson]

Cedarview never implemented this so use the Poulsbo version for both.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c |  6 +---
 drivers/gpu/drm/gma500/gma_lvds.c       | 41 +++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 43 +------------------------
 4 files changed, 44 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index b0e7b680f02b..61dc65964e21 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,10 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static void cdv_intel_lvds_restore(struct drm_connector *connector)
-{
-}
-
 static bool cdv_intel_lvds_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
@@ -381,7 +377,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 
 	connector = &gma_connector->base;
 	gma_connector->save = gma_lvds_save;
-	gma_connector->restore = cdv_intel_lvds_restore;
+	gma_connector->restore = gma_lvds_restore;
 	encoder = &gma_encoder->base;
 
 	/* Set up the DDC bus. */
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 7a81f44a40bd..19679ee36062 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -168,3 +168,44 @@ void gma_lvds_save(struct drm_connector *connector)
 			lvds_priv->saveBLC_PWM_CTL);
 }
 
+void gma_lvds_restore(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	u32 pp_status;
+	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
+	struct gma_lvds_priv *lvds_priv =
+		(struct gma_lvds_priv *)gma_encoder->dev_priv;
+
+	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
+			lvds_priv->savePP_ON,
+			lvds_priv->savePP_OFF,
+			lvds_priv->saveLVDS,
+			lvds_priv->savePP_CONTROL,
+			lvds_priv->savePP_CYCLE,
+			lvds_priv->saveBLC_PWM_CTL);
+
+	REG_WRITE(BLC_PWM_CTL, lvds_priv->saveBLC_PWM_CTL);
+	REG_WRITE(PFIT_CONTROL, lvds_priv->savePFIT_CONTROL);
+	REG_WRITE(PFIT_PGM_RATIOS, lvds_priv->savePFIT_PGM_RATIOS);
+	REG_WRITE(LVDSPP_ON, lvds_priv->savePP_ON);
+	REG_WRITE(LVDSPP_OFF, lvds_priv->savePP_OFF);
+	/*REG_WRITE(PP_DIVISOR, lvds_priv->savePP_DIVISOR);*/
+	REG_WRITE(PP_CYCLE, lvds_priv->savePP_CYCLE);
+	REG_WRITE(PP_CONTROL, lvds_priv->savePP_CONTROL);
+	REG_WRITE(LVDS, lvds_priv->saveLVDS);
+
+	if (lvds_priv->savePP_CONTROL & POWER_TARGET_ON) {
+		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) |
+			POWER_TARGET_ON);
+		do {
+			pp_status = REG_READ(PP_STATUS);
+		} while ((pp_status & PP_ON) == 0);
+	} else {
+		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
+			~POWER_TARGET_ON);
+		do {
+			pp_status = REG_READ(PP_STATUS);
+		} while (pp_status & PP_ON);
+	}
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 98ad9bc878b7..eee0da00a0cf 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -29,5 +29,6 @@ enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
 					 struct drm_display_mode *mode);
 void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
 void gma_lvds_save(struct drm_connector *connector);
+void gma_lvds_restore(struct drm_connector *connector);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 995e7aac53b9..6e5abb670bff 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,47 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-static void psb_intel_lvds_restore(struct drm_connector *connector)
-{
-	struct drm_device *dev = connector->dev;
-	u32 pp_status;
-	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
-	struct gma_lvds_priv *lvds_priv =
-		(struct gma_lvds_priv *)gma_encoder->dev_priv;
-
-	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
-			lvds_priv->savePP_ON,
-			lvds_priv->savePP_OFF,
-			lvds_priv->saveLVDS,
-			lvds_priv->savePP_CONTROL,
-			lvds_priv->savePP_CYCLE,
-			lvds_priv->saveBLC_PWM_CTL);
-
-	REG_WRITE(BLC_PWM_CTL, lvds_priv->saveBLC_PWM_CTL);
-	REG_WRITE(PFIT_CONTROL, lvds_priv->savePFIT_CONTROL);
-	REG_WRITE(PFIT_PGM_RATIOS, lvds_priv->savePFIT_PGM_RATIOS);
-	REG_WRITE(LVDSPP_ON, lvds_priv->savePP_ON);
-	REG_WRITE(LVDSPP_OFF, lvds_priv->savePP_OFF);
-	/*REG_WRITE(PP_DIVISOR, lvds_priv->savePP_DIVISOR);*/
-	REG_WRITE(PP_CYCLE, lvds_priv->savePP_CYCLE);
-	REG_WRITE(PP_CONTROL, lvds_priv->savePP_CONTROL);
-	REG_WRITE(LVDS, lvds_priv->saveLVDS);
-
-	if (lvds_priv->savePP_CONTROL & POWER_TARGET_ON) {
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) |
-			POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while ((pp_status & PP_ON) == 0);
-	} else {
-		REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) &
-			~POWER_TARGET_ON);
-		do {
-			pp_status = REG_READ(PP_STATUS);
-		} while (pp_status & PP_ON);
-	}
-}
-
 bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
@@ -473,7 +432,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 
 	connector = &gma_connector->base;
 	gma_connector->save = gma_lvds_save;
-	gma_connector->restore = psb_intel_lvds_restore;
+	gma_connector->restore = gma_lvds_restore;
 
 	/* Set up the DDC bus. */
 	ddc_bus = gma_i2c_create(dev, GPIOC, "LVDSDDC_C");
-- 
2.36.1

[PATCH 09/19] drm/gma500: Unify *_intel_lvds_mode_fixup()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them. Change a check
of !IS_MRST() to IS_PSB() to not change the behaviour for CDV.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 51 +------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 59 ++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  3 ++
 drivers/gpu/drm/gma500/oaktrail_lvds.c  |  2 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 65 +------------------------
 5 files changed, 65 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 61dc65964e21..65532308f1e7 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,55 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static bool cdv_intel_lvds_mode_fixup(struct drm_encoder *encoder,
-				  const struct drm_display_mode *mode,
-				  struct drm_display_mode *adjusted_mode)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-	struct drm_encoder *tmp_encoder;
-	struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
-
-	/* Should never happen!! */
-	list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
-			    head) {
-		if (tmp_encoder != encoder
-		    && tmp_encoder->crtc == encoder->crtc) {
-			pr_err("Can't enable LVDS and another encoder on the same pipe\n");
-			return false;
-		}
-	}
-
-	/*
-	 * If we have timings from the BIOS for the panel, put them in
-	 * to the adjusted mode.  The CRTC will be set up for this mode,
-	 * with the panel scaling set up to source from the H/VDisplay
-	 * of the original mode.
-	 */
-	if (panel_fixed_mode != NULL) {
-		adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
-		adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
-		adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
-		adjusted_mode->htotal = panel_fixed_mode->htotal;
-		adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
-		adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
-		adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
-		adjusted_mode->vtotal = panel_fixed_mode->vtotal;
-		adjusted_mode->clock = panel_fixed_mode->clock;
-		drm_mode_set_crtcinfo(adjusted_mode,
-				      CRTC_INTERLACE_HALVE_V);
-	}
-
-	/*
-	 * XXX: It would be nice to support lower refresh rates on the
-	 * panels to reduce power consumption, and perhaps match the
-	 * user's requested refresh rate.
-	 */
-
-	return true;
-}
-
 static void cdv_intel_lvds_prepare(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
@@ -255,7 +206,7 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
 static const struct drm_encoder_helper_funcs
 					cdv_intel_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
-	.mode_fixup = cdv_intel_lvds_mode_fixup,
+	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = cdv_intel_lvds_prepare,
 	.mode_set = cdv_intel_lvds_mode_set,
 	.commit = cdv_intel_lvds_commit,
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 19679ee36062..32ecf5835828 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -209,3 +209,62 @@ void gma_lvds_restore(struct drm_connector *connector)
 	}
 }
 
+bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
+			 const struct drm_display_mode *mode,
+			 struct drm_display_mode *adjusted_mode)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
+	struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
+	struct drm_encoder *tmp_encoder;
+	struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
+
+	/* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
+	if (IS_PSB(dev) && gma_crtc->pipe == 0) {
+		pr_err("Can't support LVDS on pipe A\n");
+		return false;
+	}
+	if (IS_MRST(dev) && gma_crtc->pipe != 0) {
+		pr_err("Must use PIPE A\n");
+		return false;
+	}
+	/* Should never happen!! */
+	list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
+			    head) {
+		if (tmp_encoder != encoder
+		    && tmp_encoder->crtc == encoder->crtc) {
+			pr_err("Can't enable LVDS and another encoder on the same pipe\n");
+			return false;
+		}
+	}
+
+	/*
+	 * If we have timings from the BIOS for the panel, put them in
+	 * to the adjusted mode.  The CRTC will be set up for this mode,
+	 * with the panel scaling set up to source from the H/VDisplay
+	 * of the original mode.
+	 */
+	if (panel_fixed_mode != NULL) {
+		adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
+		adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
+		adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
+		adjusted_mode->htotal = panel_fixed_mode->htotal;
+		adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
+		adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
+		adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
+		adjusted_mode->vtotal = panel_fixed_mode->vtotal;
+		adjusted_mode->clock = panel_fixed_mode->clock;
+		drm_mode_set_crtcinfo(adjusted_mode,
+				      CRTC_INTERLACE_HALVE_V);
+	}
+
+	/*
+	 * XXX: It would be nice to support lower refresh rates on the
+	 * panels to reduce power consumption, and perhaps match the
+	 * user's requested refresh rate.
+	 */
+
+	return true;
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index eee0da00a0cf..950ab7b88ad6 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -30,5 +30,8 @@ enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
 void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
 void gma_lvds_save(struct drm_connector *connector);
 void gma_lvds_restore(struct drm_connector *connector);
+bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
+			 const struct drm_display_mode *mode,
+			 struct drm_display_mode *adjusted_mode);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 00ec4fea4c12..16699b0fbbc9 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -134,7 +134,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 
 static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
-	.mode_fixup = psb_intel_lvds_mode_fixup,
+	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = oaktrail_lvds_prepare,
 	.mode_set = oaktrail_lvds_mode_set,
 	.commit = oaktrail_lvds_commit,
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 6e5abb670bff..317bd1fcfa2a 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,69 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
-				  const struct drm_display_mode *mode,
-				  struct drm_display_mode *adjusted_mode)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-	struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
-	struct drm_encoder *tmp_encoder;
-	struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
-	struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
-
-	if (gma_encoder->type == INTEL_OUTPUT_MIPI2)
-		panel_fixed_mode = mode_dev->panel_fixed_mode2;
-
-	/* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
-	if (!IS_MRST(dev) && gma_crtc->pipe == 0) {
-		pr_err("Can't support LVDS on pipe A\n");
-		return false;
-	}
-	if (IS_MRST(dev) && gma_crtc->pipe != 0) {
-		pr_err("Must use PIPE A\n");
-		return false;
-	}
-	/* Should never happen!! */
-	list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
-			    head) {
-		if (tmp_encoder != encoder
-		    && tmp_encoder->crtc == encoder->crtc) {
-			pr_err("Can't enable LVDS and another encoder on the same pipe\n");
-			return false;
-		}
-	}
-
-	/*
-	 * If we have timings from the BIOS for the panel, put them in
-	 * to the adjusted mode.  The CRTC will be set up for this mode,
-	 * with the panel scaling set up to source from the H/VDisplay
-	 * of the original mode.
-	 */
-	if (panel_fixed_mode != NULL) {
-		adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
-		adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
-		adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
-		adjusted_mode->htotal = panel_fixed_mode->htotal;
-		adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
-		adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
-		adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
-		adjusted_mode->vtotal = panel_fixed_mode->vtotal;
-		adjusted_mode->clock = panel_fixed_mode->clock;
-		drm_mode_set_crtcinfo(adjusted_mode,
-				      CRTC_INTERLACE_HALVE_V);
-	}
-
-	/*
-	 * XXX: It would be nice to support lower refresh rates on the
-	 * panels to reduce power consumption, and perhaps match the
-	 * user's requested refresh rate.
-	 */
-
-	return true;
-}
-
 static void psb_intel_lvds_prepare(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
@@ -365,7 +302,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
 
 static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
-	.mode_fixup = psb_intel_lvds_mode_fixup,
+	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = psb_intel_lvds_prepare,
 	.mode_set = psb_intel_lvds_mode_set,
 	.commit = psb_intel_lvds_commit,
-- 
2.36.1

[PATCH 10/19] drm/gma500: Unify *_intel_lvds_prepare()

[Patrik Jakobsson]

The functions are identical so unify them.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 20 +-------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 18 ++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 18 +-----------------
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 20 +-------------------
 5 files changed, 22 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 65532308f1e7..4f0754b8c0b0 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,24 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static void cdv_intel_lvds_prepare(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-
-	if (!gma_power_begin(dev, true))
-		return;
-
-	mode_dev->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
-	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
-					  BACKLIGHT_DUTY_CYCLE_MASK);
-
-	gma_lvds_set_power(dev, false);
-
-	gma_power_end(dev);
-}
-
 static void cdv_intel_lvds_commit(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
@@ -207,7 +189,7 @@ static const struct drm_encoder_helper_funcs
 					cdv_intel_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = gma_lvds_mode_fixup,
-	.prepare = cdv_intel_lvds_prepare,
+	.prepare = gma_lvds_prepare,
 	.mode_set = cdv_intel_lvds_mode_set,
 	.commit = cdv_intel_lvds_commit,
 };
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 32ecf5835828..d0a97eeac38b 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -268,3 +268,21 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 	return true;
 }
 
+void gma_lvds_prepare(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
+
+	if (!gma_power_begin(dev, true))
+		return;
+
+	mode_dev->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
+	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
+					  BACKLIGHT_DUTY_CYCLE_MASK);
+
+	gma_lvds_set_power(dev, false);
+
+	gma_power_end(dev);
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 950ab7b88ad6..c1a7b3611d27 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -33,5 +33,6 @@ void gma_lvds_restore(struct drm_connector *connector);
 bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 const struct drm_display_mode *mode,
 			 struct drm_display_mode *adjusted_mode);
+void gma_lvds_prepare(struct drm_encoder *encoder);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 16699b0fbbc9..dba0ca4b73a0 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -105,22 +105,6 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
 	gma_power_end(dev);
 }
 
-static void oaktrail_lvds_prepare(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-
-	if (!gma_power_begin(dev, true))
-		return;
-
-	mode_dev->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
-	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
-					  BACKLIGHT_DUTY_CYCLE_MASK);
-	gma_lvds_set_power(dev, false);
-	gma_power_end(dev);
-}
-
 static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
@@ -135,7 +119,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
 static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = gma_lvds_mode_fixup,
-	.prepare = oaktrail_lvds_prepare,
+	.prepare = gma_lvds_prepare,
 	.mode_set = oaktrail_lvds_mode_set,
 	.commit = oaktrail_lvds_commit,
 };
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 317bd1fcfa2a..e4258205a262 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,24 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-static void psb_intel_lvds_prepare(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-
-	if (!gma_power_begin(dev, true))
-		return;
-
-	mode_dev->saveBLC_PWM_CTL = REG_READ(BLC_PWM_CTL);
-	mode_dev->backlight_duty_cycle = (mode_dev->saveBLC_PWM_CTL &
-					  BACKLIGHT_DUTY_CYCLE_MASK);
-
-	gma_lvds_set_power(dev, false);
-
-	gma_power_end(dev);
-}
-
 static void psb_intel_lvds_commit(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
@@ -303,7 +285,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
 static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = gma_lvds_mode_fixup,
-	.prepare = psb_intel_lvds_prepare,
+	.prepare = gma_lvds_prepare,
 	.mode_set = psb_intel_lvds_mode_set,
 	.commit = psb_intel_lvds_commit,
 };
-- 
2.36.1

[PATCH 11/19] drm/gma500: Unify *_intel_lvds_commit()

[Patrik Jakobsson]

These functions are identical so unify them

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 15 +--------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 15 ++++++++++++++-
 drivers/gpu/drm/gma500/gma_lvds.h       |  2 +-
 drivers/gpu/drm/gma500/oaktrail_lvds.c  | 13 +------------
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 15 +--------------
 5 files changed, 18 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 4f0754b8c0b0..77a5d167c508 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,19 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static void cdv_intel_lvds_commit(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-
-	if (mode_dev->backlight_duty_cycle == 0)
-		mode_dev->backlight_duty_cycle =
-		    gma_lvds_get_max_backlight(dev);
-
-	gma_lvds_set_power(dev, true);
-}
-
 static void cdv_intel_lvds_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -191,7 +178,7 @@ static const struct drm_encoder_helper_funcs
 	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = gma_lvds_prepare,
 	.mode_set = cdv_intel_lvds_mode_set,
-	.commit = cdv_intel_lvds_commit,
+	.commit = gma_lvds_commit,
 };
 
 static const struct drm_connector_helper_funcs
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index d0a97eeac38b..6364d3aef064 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -62,7 +62,7 @@ static void gma_lvds_set_backlight(struct drm_device *dev, int level)
 /*
  * Sets the power state for the panel.
  */
-void gma_lvds_set_power(struct drm_device *dev, bool on)
+static void gma_lvds_set_power(struct drm_device *dev, bool on)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	u32 pp_status;
@@ -286,3 +286,16 @@ void gma_lvds_prepare(struct drm_encoder *encoder)
 	gma_power_end(dev);
 }
 
+void gma_lvds_commit(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
+
+	if (mode_dev->backlight_duty_cycle == 0)
+		mode_dev->backlight_duty_cycle =
+		    gma_lvds_get_max_backlight(dev);
+
+	gma_lvds_set_power(dev, true);
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index c1a7b3611d27..c2c8fe5b5aac 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -24,7 +24,6 @@ struct gma_lvds_priv {
 };
 
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
-void gma_lvds_set_power(struct drm_device *dev, bool on);
 enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
 					 struct drm_display_mode *mode);
 void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
@@ -34,5 +33,6 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 const struct drm_display_mode *mode,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
+void gma_lvds_commit(struct drm_encoder *encoder);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index dba0ca4b73a0..85cab0f7028a 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -105,23 +105,12 @@ static void oaktrail_lvds_mode_set(struct drm_encoder *encoder,
 	gma_power_end(dev);
 }
 
-static void oaktrail_lvds_commit(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-
-	if (mode_dev->backlight_duty_cycle == 0)
-		mode_dev->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
-	gma_lvds_set_power(dev, true);
-}
-
 static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = gma_lvds_prepare,
 	.mode_set = oaktrail_lvds_mode_set,
-	.commit = oaktrail_lvds_commit,
+	.commit = gma_lvds_commit,
 };
 
 /* Returns the panel fixed mode from configuration. */
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index e4258205a262..fbb72be6e017 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,19 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-static void psb_intel_lvds_commit(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-
-	if (mode_dev->backlight_duty_cycle == 0)
-		mode_dev->backlight_duty_cycle =
-		    gma_lvds_get_max_backlight(dev);
-
-	gma_lvds_set_power(dev, true);
-}
-
 static void psb_intel_lvds_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -287,7 +274,7 @@ static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
 	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = gma_lvds_prepare,
 	.mode_set = psb_intel_lvds_mode_set,
-	.commit = psb_intel_lvds_commit,
+	.commit = gma_lvds_commit,
 };
 
 const struct drm_connector_helper_funcs
-- 
2.36.1

[PATCH 12/19] drm/gma500: Unify *_intel_lvds_mode_set()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them. CDV include the
pipe select bit in the pfit control register but we can do this on PSB
as well since LVDS is always on the same pipe there. Oaktrail lvds
modeset sequence is slightly different so is not unified here.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 38 +------------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 35 +++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  3 ++
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 35 +----------------------
 4 files changed, 40 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 77a5d167c508..ddfb976b6059 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,42 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static void cdv_intel_lvds_mode_set(struct drm_encoder *encoder,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
-	u32 pfit_control;
-
-	/*
-	 * The LVDS pin pair will already have been turned on in the
-	 * cdv_intel_crtc_mode_set since it has a large impact on the DPLL
-	 * settings.
-	 */
-
-	/*
-	 * Enable automatic panel scaling so that non-native modes fill the
-	 * screen.  Should be enabled before the pipe is enabled, according to
-	 * register description and PRM.
-	 */
-	if (mode->hdisplay != adjusted_mode->hdisplay ||
-	    mode->vdisplay != adjusted_mode->vdisplay)
-		pfit_control = (PFIT_ENABLE | VERT_AUTO_SCALE |
-				HORIZ_AUTO_SCALE | VERT_INTERP_BILINEAR |
-				HORIZ_INTERP_BILINEAR);
-	else
-		pfit_control = 0;
-
-	pfit_control |= gma_crtc->pipe << PFIT_PIPE_SHIFT;
-
-	if (dev_priv->lvds_dither)
-		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
-
-	REG_WRITE(PFIT_CONTROL, pfit_control);
-}
-
 /*
  * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
  */
@@ -177,7 +141,7 @@ static const struct drm_encoder_helper_funcs
 	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = gma_lvds_prepare,
-	.mode_set = cdv_intel_lvds_mode_set,
+	.mode_set = gma_lvds_mode_set,
 	.commit = gma_lvds_commit,
 };
 
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 6364d3aef064..215bf8f7d41f 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -299,3 +299,38 @@ void gma_lvds_commit(struct drm_encoder *encoder)
 	gma_lvds_set_power(dev, true);
 }
 
+void gma_lvds_mode_set(struct drm_encoder *encoder,
+		       struct drm_display_mode *mode,
+		       struct drm_display_mode *adjusted_mode)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
+	u32 pfit_control;
+
+	/*
+	 * The LVDS pin pair will already have been turned on in the
+	 * *_crtc_mode_set since it has a large impact on the DPLL * settings.
+	 */
+
+	/*
+	 * Enable automatic panel scaling so that non-native modes fill the
+	 * screen.  Should be enabled before the pipe is enabled, according to
+	 * register description and PRM.
+	 */
+	if (mode->hdisplay != adjusted_mode->hdisplay ||
+	    mode->vdisplay != adjusted_mode->vdisplay)
+		pfit_control = (PFIT_ENABLE | VERT_AUTO_SCALE |
+				HORIZ_AUTO_SCALE | VERT_INTERP_BILINEAR |
+				HORIZ_INTERP_BILINEAR);
+	else
+		pfit_control = 0;
+
+	pfit_control |= gma_crtc->pipe << PFIT_PIPE_SHIFT;
+
+	if (dev_priv->lvds_dither)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	REG_WRITE(PFIT_CONTROL, pfit_control);
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index c2c8fe5b5aac..ebba869a25b7 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -34,5 +34,8 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
+void gma_lvds_mode_set(struct drm_encoder *encoder,
+		       struct drm_display_mode *mode,
+		       struct drm_display_mode *adjusted_mode);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index fbb72be6e017..553f6cb5f322 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,39 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-static void psb_intel_lvds_mode_set(struct drm_encoder *encoder,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode)
-{
-	struct drm_device *dev = encoder->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 pfit_control;
-
-	/*
-	 * The LVDS pin pair will already have been turned on in the
-	 * psb_intel_crtc_mode_set since it has a large impact on the DPLL
-	 * settings.
-	 */
-
-	/*
-	 * Enable automatic panel scaling so that non-native modes fill the
-	 * screen.  Should be enabled before the pipe is enabled, according to
-	 * register description and PRM.
-	 */
-	if (mode->hdisplay != adjusted_mode->hdisplay ||
-	    mode->vdisplay != adjusted_mode->vdisplay)
-		pfit_control = (PFIT_ENABLE | VERT_AUTO_SCALE |
-				HORIZ_AUTO_SCALE | VERT_INTERP_BILINEAR |
-				HORIZ_INTERP_BILINEAR);
-	else
-		pfit_control = 0;
-
-	if (dev_priv->lvds_dither)
-		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
-
-	REG_WRITE(PFIT_CONTROL, pfit_control);
-}
-
 /*
  * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
  */
@@ -273,7 +240,7 @@ static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
 	.dpms = gma_lvds_encoder_dpms,
 	.mode_fixup = gma_lvds_mode_fixup,
 	.prepare = gma_lvds_prepare,
-	.mode_set = psb_intel_lvds_mode_set,
+	.mode_set = gma_lvds_mode_set,
 	.commit = gma_lvds_commit,
 };
 
-- 
2.36.1

[PATCH 13/19] drm/gma500: Unify struct *_intel_lvds_helper_funcs

[Patrik Jakobsson]

These now only contains generic gma functions so create
gma_lvds_helper_funcs that both PSB and CDV can use. Oaktrail still
needs the modeset callback refactored to align with PSB and CDV.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 11 +----------
 drivers/gpu/drm/gma500/gma_lvds.c       | 14 +++++++++++---
 drivers/gpu/drm/gma500/gma_lvds.h       |  5 ++---
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +---------
 4 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index ddfb976b6059..80ccc00c47e5 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -136,15 +136,6 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
 	return 0;
 }
 
-static const struct drm_encoder_helper_funcs
-					cdv_intel_lvds_helper_funcs = {
-	.dpms = gma_lvds_encoder_dpms,
-	.mode_fixup = gma_lvds_mode_fixup,
-	.prepare = gma_lvds_prepare,
-	.mode_set = gma_lvds_mode_set,
-	.commit = gma_lvds_commit,
-};
-
 static const struct drm_connector_helper_funcs
 				cdv_intel_lvds_connector_helper_funcs = {
 	.get_modes = cdv_intel_lvds_get_modes,
@@ -286,7 +277,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 	gma_connector_attach_encoder(gma_connector, gma_encoder);
 	gma_encoder->type = INTEL_OUTPUT_LVDS;
 
-	drm_encoder_helper_add(encoder, &cdv_intel_lvds_helper_funcs);
+	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
 	drm_connector_helper_add(connector,
 				 &cdv_intel_lvds_connector_helper_funcs);
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 215bf8f7d41f..bf9fa5ebdbd3 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -299,9 +299,9 @@ void gma_lvds_commit(struct drm_encoder *encoder)
 	gma_lvds_set_power(dev, true);
 }
 
-void gma_lvds_mode_set(struct drm_encoder *encoder,
-		       struct drm_display_mode *mode,
-		       struct drm_display_mode *adjusted_mode)
+static void gma_lvds_mode_set(struct drm_encoder *encoder,
+			      struct drm_display_mode *mode,
+			      struct drm_display_mode *adjusted_mode)
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -334,3 +334,11 @@ void gma_lvds_mode_set(struct drm_encoder *encoder,
 	REG_WRITE(PFIT_CONTROL, pfit_control);
 }
 
+const struct drm_encoder_helper_funcs gma_lvds_helper_funcs = {
+	.dpms = gma_lvds_encoder_dpms,
+	.mode_fixup = gma_lvds_mode_fixup,
+	.prepare = gma_lvds_prepare,
+	.mode_set = gma_lvds_mode_set,
+	.commit = gma_lvds_commit,
+};
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index ebba869a25b7..3c47bea859ad 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -34,8 +34,7 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
-void gma_lvds_mode_set(struct drm_encoder *encoder,
-		       struct drm_display_mode *mode,
-		       struct drm_display_mode *adjusted_mode);
+
+extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
 
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 553f6cb5f322..29a9b4ea5803 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -236,14 +236,6 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
 	return -1;
 }
 
-static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
-	.dpms = gma_lvds_encoder_dpms,
-	.mode_fixup = gma_lvds_mode_fixup,
-	.prepare = gma_lvds_prepare,
-	.mode_set = gma_lvds_mode_set,
-	.commit = gma_lvds_commit,
-};
-
 const struct drm_connector_helper_funcs
 				psb_intel_lvds_connector_helper_funcs = {
 	.get_modes = psb_intel_lvds_get_modes,
@@ -329,7 +321,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 	gma_connector_attach_encoder(gma_connector, gma_encoder);
 	gma_encoder->type = INTEL_OUTPUT_LVDS;
 
-	drm_encoder_helper_add(encoder, &psb_intel_lvds_helper_funcs);
+	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
 	drm_connector_helper_add(connector,
 				 &psb_intel_lvds_connector_helper_funcs);
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
-- 
2.36.1

[PATCH 14/19] drm/gma500: Unify *_intel_lvds_get_modes()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them. Oaktrail already
uses the PSB connector helpers so it is already handled.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 27 +-----------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 26 +++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 28 +------------------------
 4 files changed, 29 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 80ccc00c47e5..a6f94572baaf 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,31 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-/*
- * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
- */
-static int cdv_intel_lvds_get_modes(struct drm_connector *connector)
-{
-	struct drm_device *dev = connector->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-	int ret;
-
-	ret = psb_intel_ddc_get_modes(connector, connector->ddc);
-
-	if (ret)
-		return ret;
-
-	if (mode_dev->panel_fixed_mode != NULL) {
-		struct drm_display_mode *mode =
-		    drm_mode_duplicate(dev, mode_dev->panel_fixed_mode);
-		drm_mode_probed_add(connector, mode);
-		return 1;
-	}
-
-	return 0;
-}
-
 static void cdv_intel_lvds_destroy(struct drm_connector *connector)
 {
 	struct gma_connector *gma_connector = to_gma_connector(connector);
@@ -138,7 +113,7 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
 
 static const struct drm_connector_helper_funcs
 				cdv_intel_lvds_connector_helper_funcs = {
-	.get_modes = cdv_intel_lvds_get_modes,
+	.get_modes = gma_lvds_get_modes,
 	.mode_valid = gma_lvds_mode_valid,
 	.best_encoder = gma_best_encoder,
 };
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index bf9fa5ebdbd3..49c11b5337cb 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -342,3 +342,29 @@ const struct drm_encoder_helper_funcs gma_lvds_helper_funcs = {
 	.commit = gma_lvds_commit,
 };
 
+/*
+ * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
+ */
+int gma_lvds_get_modes(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
+	int ret = 0;
+
+	if (!IS_MRST(dev))
+		ret = psb_intel_ddc_get_modes(connector, connector->ddc);
+
+	if (ret)
+		return ret;
+
+	if (mode_dev->panel_fixed_mode != NULL) {
+		struct drm_display_mode *mode =
+		    drm_mode_duplicate(dev, mode_dev->panel_fixed_mode);
+		drm_mode_probed_add(connector, mode);
+		return 1;
+	}
+
+	return 0;
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 3c47bea859ad..70c416d8b7a7 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -34,6 +34,7 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
+int gma_lvds_get_modes(struct drm_connector *connector);
 
 extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
 
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 29a9b4ea5803..4ef0150c6a03 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,32 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-/*
- * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
- */
-static int psb_intel_lvds_get_modes(struct drm_connector *connector)
-{
-	struct drm_device *dev = connector->dev;
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
-	int ret = 0;
-
-	if (!IS_MRST(dev))
-		ret = psb_intel_ddc_get_modes(connector, connector->ddc);
-
-	if (ret)
-		return ret;
-
-	if (mode_dev->panel_fixed_mode != NULL) {
-		struct drm_display_mode *mode =
-		    drm_mode_duplicate(dev, mode_dev->panel_fixed_mode);
-		drm_mode_probed_add(connector, mode);
-		return 1;
-	}
-
-	return 0;
-}
-
 void psb_intel_lvds_destroy(struct drm_connector *connector)
 {
 	struct gma_connector *gma_connector = to_gma_connector(connector);
@@ -238,7 +212,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
 
 const struct drm_connector_helper_funcs
 				psb_intel_lvds_connector_helper_funcs = {
-	.get_modes = psb_intel_lvds_get_modes,
+	.get_modes = gma_lvds_get_modes,
 	.mode_valid = gma_lvds_mode_valid,
 	.best_encoder = gma_best_encoder,
 };
-- 
2.36.1

[PATCH 15/19] drm/gma500: Unify struct *_intel_lvds_connector_helper_funcs

[Patrik Jakobsson]

These now only contains generic gma functions so create
gma_lvds_connector_helper_funcs that all chips can use.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 10 +---------
 drivers/gpu/drm/gma500/gma_lvds.c       | 12 +++++++++---
 drivers/gpu/drm/gma500/gma_lvds.h       |  4 +---
 drivers/gpu/drm/gma500/oaktrail_lvds.c  |  3 +--
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +---------
 5 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index a6f94572baaf..2ba635513401 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -111,13 +111,6 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
 	return 0;
 }
 
-static const struct drm_connector_helper_funcs
-				cdv_intel_lvds_connector_helper_funcs = {
-	.get_modes = gma_lvds_get_modes,
-	.mode_valid = gma_lvds_mode_valid,
-	.best_encoder = gma_best_encoder,
-};
-
 static const struct drm_connector_funcs cdv_intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
@@ -253,8 +246,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 	gma_encoder->type = INTEL_OUTPUT_LVDS;
 
 	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
-	drm_connector_helper_add(connector,
-				 &cdv_intel_lvds_connector_helper_funcs);
+	drm_connector_helper_add(connector, &gma_lvds_connector_helper_funcs);
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 49c11b5337cb..3b48999105d1 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -94,8 +94,8 @@ static void gma_lvds_set_power(struct drm_device *dev, bool on)
 	gma_power_end(dev);
 }
 
-enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
-					 struct drm_display_mode *mode)
+static enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
+						struct drm_display_mode *mode)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -345,7 +345,7 @@ const struct drm_encoder_helper_funcs gma_lvds_helper_funcs = {
 /*
  * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
  */
-int gma_lvds_get_modes(struct drm_connector *connector)
+static int gma_lvds_get_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -368,3 +368,9 @@ int gma_lvds_get_modes(struct drm_connector *connector)
 	return 0;
 }
 
+const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs = {
+	.get_modes = gma_lvds_get_modes,
+	.mode_valid = gma_lvds_mode_valid,
+	.best_encoder = gma_best_encoder,
+};
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 70c416d8b7a7..5c7da22400fb 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -24,8 +24,6 @@ struct gma_lvds_priv {
 };
 
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
-enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
-					 struct drm_display_mode *mode);
 void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
 void gma_lvds_save(struct drm_connector *connector);
 void gma_lvds_restore(struct drm_connector *connector);
@@ -34,8 +32,8 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
-int gma_lvds_get_modes(struct drm_connector *connector);
 
 extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
+extern const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs;
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 85cab0f7028a..7724ebd71aa8 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -230,8 +230,7 @@ void oaktrail_lvds_init(struct drm_device *dev,
 	gma_encoder->type = INTEL_OUTPUT_LVDS;
 
 	drm_encoder_helper_add(encoder, &oaktrail_lvds_helper_funcs);
-	drm_connector_helper_add(connector,
-				 &psb_intel_lvds_connector_helper_funcs);
+	drm_connector_helper_add(connector, &gma_lvds_connector_helper_funcs);
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 4ef0150c6a03..f129e53f0233 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -210,13 +210,6 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
 	return -1;
 }
 
-const struct drm_connector_helper_funcs
-				psb_intel_lvds_connector_helper_funcs = {
-	.get_modes = gma_lvds_get_modes,
-	.mode_valid = gma_lvds_mode_valid,
-	.best_encoder = gma_best_encoder,
-};
-
 const struct drm_connector_funcs psb_intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
@@ -296,8 +289,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 	gma_encoder->type = INTEL_OUTPUT_LVDS;
 
 	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
-	drm_connector_helper_add(connector,
-				 &psb_intel_lvds_connector_helper_funcs);
+	drm_connector_helper_add(connector, &gma_lvds_connector_helper_funcs);
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
-- 
2.36.1

[PATCH 16/19] drm/gma500: Use i2c_bus in gma_encoder for PSB

[Patrik Jakobsson]

PSB stores the backlight i2c adapter in lvds_priv. CDV stores it in
gma_encoder. Neither place is perfect but lets pick gma_encoder to make
life simple.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gma_lvds.h       |  2 --
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +++++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 5c7da22400fb..dcba810dc470 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -19,8 +19,6 @@ struct gma_lvds_priv {
 	uint32_t savePFIT_CONTROL;
 	uint32_t savePFIT_PGM_RATIOS;
 	uint32_t saveBLC_PWM_CTL;
-
-	struct gma_i2c_chan *i2c_bus;
 };
 
 u32 gma_lvds_get_max_backlight(struct drm_device *dev);
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index f129e53f0233..ea5f2f078a7f 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -306,14 +306,14 @@ void psb_intel_lvds_init(struct drm_device *dev,
 	 * Set up I2C bus
 	 * FIXME: distroy i2c_bus when exit
 	 */
-	lvds_priv->i2c_bus = gma_i2c_create(dev, GPIOB, "LVDSBLC_B");
-	if (!lvds_priv->i2c_bus) {
+	gma_encoder->i2c_bus = gma_i2c_create(dev, GPIOB, "LVDSBLC_B");
+	if (!gma_encoder->i2c_bus) {
 		dev_printk(KERN_ERR,
 			dev->dev, "I2C bus registration failed.\n");
 		goto err_encoder_cleanup;
 	}
-	lvds_priv->i2c_bus->slave_addr = 0x2C;
-	dev_priv->lvds_i2c_bus =  lvds_priv->i2c_bus;
+	gma_encoder->i2c_bus->slave_addr = 0x2C;
+	dev_priv->lvds_i2c_bus = gma_encoder->i2c_bus;
 
 	/*
 	 * LVDS discovery:
@@ -390,7 +390,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 
 err_unlock:
 	mutex_unlock(&dev->mode_config.mutex);
-	gma_i2c_destroy(lvds_priv->i2c_bus);
+	gma_i2c_destroy(gma_encoder->i2c_bus);
 err_encoder_cleanup:
 	drm_encoder_cleanup(encoder);
 err_connector_cleanup:
-- 
2.36.1

[PATCH 17/19] drm/gma500: Unify *_intel_lvds_destroy()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them into one.
Only destroy i2c adapters that have actually been created.
gma_i2c_destroy() is now also called for PSB.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 14 +-------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 15 +++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  1 +
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 13 +------------
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index 2ba635513401..c492f1b3c8ea 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,17 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static void cdv_intel_lvds_destroy(struct drm_connector *connector)
-{
-	struct gma_connector *gma_connector = to_gma_connector(connector);
-	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
-
-	gma_i2c_destroy(to_gma_i2c_chan(connector->ddc));
-	gma_i2c_destroy(gma_encoder->i2c_bus);
-	drm_connector_cleanup(connector);
-	kfree(gma_connector);
-}
-
 static int cdv_intel_lvds_set_property(struct drm_connector *connector,
 				       struct drm_property *property,
 				       uint64_t value)
@@ -115,7 +104,7 @@ static const struct drm_connector_funcs cdv_intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = cdv_intel_lvds_set_property,
-	.destroy = cdv_intel_lvds_destroy,
+	.destroy = gma_lvds_destroy,
 };
 
 /*
@@ -261,7 +250,6 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 
 	/**
 	 * Set up I2C bus
-	 * FIXME: distroy i2c_bus when exit
 	 */
 	gma_encoder->i2c_bus = gma_i2c_create(dev, GPIOB, "LVDSBLC_B");
 	if (!gma_encoder->i2c_bus) {
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 3b48999105d1..9238cad5fac8 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -374,3 +374,18 @@ const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs = {
 	.best_encoder = gma_best_encoder,
 };
 
+void gma_lvds_destroy(struct drm_connector *connector)
+{
+	struct gma_connector *gma_connector = to_gma_connector(connector);
+	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
+
+	if (connector->ddc)
+		gma_i2c_destroy(to_gma_i2c_chan(connector->ddc));
+
+	if (gma_encoder->i2c_bus)
+		gma_i2c_destroy(gma_encoder->i2c_bus);
+
+	drm_connector_cleanup(connector);
+	kfree(gma_connector);
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index dcba810dc470..835305c900d8 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -30,6 +30,7 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
+void gma_lvds_destroy(struct drm_connector *connector);
 
 extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
 extern const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs;
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index ea5f2f078a7f..76742925b760 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,16 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-void psb_intel_lvds_destroy(struct drm_connector *connector)
-{
-	struct gma_connector *gma_connector = to_gma_connector(connector);
-	struct gma_i2c_chan *ddc_bus = to_gma_i2c_chan(connector->ddc);
-
-	gma_i2c_destroy(ddc_bus);
-	drm_connector_cleanup(connector);
-	kfree(gma_connector);
-}
-
 int psb_intel_lvds_set_property(struct drm_connector *connector,
 				       struct drm_property *property,
 				       uint64_t value)
@@ -214,7 +204,7 @@ const struct drm_connector_funcs psb_intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = psb_intel_lvds_set_property,
-	.destroy = psb_intel_lvds_destroy,
+	.destroy = gma_lvds_destroy,
 };
 
 /**
@@ -304,7 +294,6 @@ void psb_intel_lvds_init(struct drm_device *dev,
 
 	/*
 	 * Set up I2C bus
-	 * FIXME: distroy i2c_bus when exit
 	 */
 	gma_encoder->i2c_bus = gma_i2c_create(dev, GPIOB, "LVDSBLC_B");
 	if (!gma_encoder->i2c_bus) {
-- 
2.36.1

[PATCH 18/19] drm/gma500: Unify *_intel_lvds_set_property()

[Patrik Jakobsson]

These functions mostly do the same thing so unify them into one.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 63 +---------------------
 drivers/gpu/drm/gma500/gma_lvds.c       | 64 ++++++++++++++++++++++
 drivers/gpu/drm/gma500/gma_lvds.h       |  2 +
 drivers/gpu/drm/gma500/psb_intel_lvds.c | 70 +------------------------
 4 files changed, 68 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index c492f1b3c8ea..ccb489d6795b 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,71 +39,10 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static int cdv_intel_lvds_set_property(struct drm_connector *connector,
-				       struct drm_property *property,
-				       uint64_t value)
-{
-	struct drm_encoder *encoder = connector->encoder;
-
-	if (!strcmp(property->name, "scaling mode") && encoder) {
-		struct gma_crtc *crtc = to_gma_crtc(encoder->crtc);
-		uint64_t curValue;
-
-		if (!crtc)
-			return -1;
-
-		switch (value) {
-		case DRM_MODE_SCALE_FULLSCREEN:
-			break;
-		case DRM_MODE_SCALE_NO_SCALE:
-			break;
-		case DRM_MODE_SCALE_ASPECT:
-			break;
-		default:
-			return -1;
-		}
-
-		if (drm_object_property_get_value(&connector->base,
-						     property,
-						     &curValue))
-			return -1;
-
-		if (curValue == value)
-			return 0;
-
-		if (drm_object_property_set_value(&connector->base,
-							property,
-							value))
-			return -1;
-
-		if (crtc->saved_mode.hdisplay != 0 &&
-		    crtc->saved_mode.vdisplay != 0) {
-			if (!drm_crtc_helper_set_mode(encoder->crtc,
-						      &crtc->saved_mode,
-						      encoder->crtc->x,
-						      encoder->crtc->y,
-						      encoder->crtc->primary->fb))
-				return -1;
-		}
-	} else if (!strcmp(property->name, "backlight") && encoder) {
-		if (drm_object_property_set_value(&connector->base,
-							property,
-							value))
-			return -1;
-		else
-                        gma_backlight_set(encoder->dev, value);
-	} else if (!strcmp(property->name, "DPMS") && encoder) {
-		const struct drm_encoder_helper_funcs *helpers =
-					encoder->helper_private;
-		helpers->dpms(encoder, value);
-	}
-	return 0;
-}
-
 static const struct drm_connector_funcs cdv_intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = cdv_intel_lvds_set_property,
+	.set_property = gma_lvds_set_property,
 	.destroy = gma_lvds_destroy,
 };
 
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index 9238cad5fac8..e4278d26f811 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -389,3 +389,67 @@ void gma_lvds_destroy(struct drm_connector *connector)
 	kfree(gma_connector);
 }
 
+int gma_lvds_set_property(struct drm_connector *connector,
+			  struct drm_property *property,
+			  uint64_t value)
+{
+	struct drm_encoder *encoder = connector->encoder;
+
+	if (!encoder)
+		return -1;
+
+	if (!strcmp(property->name, "scaling mode") && encoder) {
+		struct gma_crtc *crtc = to_gma_crtc(encoder->crtc);
+		uint64_t curValue;
+
+		if (!crtc)
+			return -1;
+
+		switch (value) {
+		case DRM_MODE_SCALE_FULLSCREEN:
+			break;
+		case DRM_MODE_SCALE_NO_SCALE:
+			break;
+		case DRM_MODE_SCALE_ASPECT:
+			break;
+		default:
+			return -1;
+		}
+
+		if (drm_object_property_get_value(&connector->base,
+						     property,
+						     &curValue))
+			return -1;
+
+		if (curValue == value)
+			return 0;
+
+		if (drm_object_property_set_value(&connector->base,
+							property,
+							value))
+			return -1;
+
+		if (crtc->saved_mode.hdisplay != 0 &&
+		    crtc->saved_mode.vdisplay != 0) {
+			if (!drm_crtc_helper_set_mode(encoder->crtc,
+						      &crtc->saved_mode,
+						      encoder->crtc->x,
+						      encoder->crtc->y,
+						      encoder->crtc->primary->fb))
+				return -1;
+		}
+	} else if (!strcmp(property->name, "backlight") && encoder) {
+		if (drm_object_property_set_value(&connector->base,
+							property,
+							value))
+			return -1;
+		else
+                        gma_backlight_set(encoder->dev, value);
+	} else if (!strcmp(property->name, "DPMS") && encoder) {
+		const struct drm_encoder_helper_funcs *helpers =
+					encoder->helper_private;
+		helpers->dpms(encoder, value);
+	}
+	return 0;
+}
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index 835305c900d8..ea261a60e9ff 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -31,6 +31,8 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
 void gma_lvds_destroy(struct drm_connector *connector);
+int gma_lvds_set_property(struct drm_connector *connector,
+			  struct drm_property *property, uint64_t value);
 
 extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
 extern const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs;
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index 76742925b760..df2e770c660a 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,78 +132,10 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-int psb_intel_lvds_set_property(struct drm_connector *connector,
-				       struct drm_property *property,
-				       uint64_t value)
-{
-	struct drm_encoder *encoder = connector->encoder;
-
-	if (!encoder)
-		return -1;
-
-	if (!strcmp(property->name, "scaling mode")) {
-		struct gma_crtc *crtc = to_gma_crtc(encoder->crtc);
-		uint64_t curval;
-
-		if (!crtc)
-			goto set_prop_error;
-
-		switch (value) {
-		case DRM_MODE_SCALE_FULLSCREEN:
-			break;
-		case DRM_MODE_SCALE_NO_SCALE:
-			break;
-		case DRM_MODE_SCALE_ASPECT:
-			break;
-		default:
-			goto set_prop_error;
-		}
-
-		if (drm_object_property_get_value(&connector->base,
-						     property,
-						     &curval))
-			goto set_prop_error;
-
-		if (curval == value)
-			goto set_prop_done;
-
-		if (drm_object_property_set_value(&connector->base,
-							property,
-							value))
-			goto set_prop_error;
-
-		if (crtc->saved_mode.hdisplay != 0 &&
-		    crtc->saved_mode.vdisplay != 0) {
-			if (!drm_crtc_helper_set_mode(encoder->crtc,
-						      &crtc->saved_mode,
-						      encoder->crtc->x,
-						      encoder->crtc->y,
-						      encoder->crtc->primary->fb))
-				goto set_prop_error;
-		}
-	} else if (!strcmp(property->name, "backlight")) {
-		if (drm_object_property_set_value(&connector->base,
-							property,
-							value))
-			goto set_prop_error;
-		else
-                        gma_backlight_set(encoder->dev, value);
-	} else if (!strcmp(property->name, "DPMS")) {
-		const struct drm_encoder_helper_funcs *hfuncs
-						= encoder->helper_private;
-		hfuncs->dpms(encoder, value);
-	}
-
-set_prop_done:
-	return 0;
-set_prop_error:
-	return -1;
-}
-
 const struct drm_connector_funcs psb_intel_lvds_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = psb_intel_lvds_set_property,
+	.set_property = gma_lvds_set_property,
 	.destroy = gma_lvds_destroy,
 };
 
-- 
2.36.1

[PATCH 19/19] drm/gma500: Unify struct *_intel_lvds_connector_funcs

[Patrik Jakobsson]

These now only contains generic drm/gma functions so create
gma_lvds_connector_funcs that all chips can use.

Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c |  9 +--------
 drivers/gpu/drm/gma500/gma_lvds.c       | 15 +++++++++++----
 drivers/gpu/drm/gma500/gma_lvds.h       |  4 +---
 drivers/gpu/drm/gma500/oaktrail_lvds.c  |  2 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c |  9 +--------
 5 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index ccb489d6795b..0b8f818ca9b5 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -39,13 +39,6 @@
 #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
 #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
 
-static const struct drm_connector_funcs cdv_intel_lvds_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = gma_lvds_set_property,
-	.destroy = gma_lvds_destroy,
-};
-
 /*
  * Enumerate the child dev array parsed from VBT to check whether
  * the LVDS is present.
@@ -160,7 +153,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
 	}
 
 	ret = drm_connector_init_with_ddc(dev, connector,
-					  &cdv_intel_lvds_connector_funcs,
+					  &gma_lvds_connector_funcs,
 					  DRM_MODE_CONNECTOR_LVDS,
 					  &ddc_bus->base);
 	if (ret)
diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
index e4278d26f811..612013baf7ad 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.c
+++ b/drivers/gpu/drm/gma500/gma_lvds.c
@@ -374,7 +374,7 @@ const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs = {
 	.best_encoder = gma_best_encoder,
 };
 
-void gma_lvds_destroy(struct drm_connector *connector)
+static void gma_lvds_destroy(struct drm_connector *connector)
 {
 	struct gma_connector *gma_connector = to_gma_connector(connector);
 	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
@@ -389,9 +389,9 @@ void gma_lvds_destroy(struct drm_connector *connector)
 	kfree(gma_connector);
 }
 
-int gma_lvds_set_property(struct drm_connector *connector,
-			  struct drm_property *property,
-			  uint64_t value)
+static int gma_lvds_set_property(struct drm_connector *connector,
+				 struct drm_property *property,
+				 uint64_t value)
 {
 	struct drm_encoder *encoder = connector->encoder;
 
@@ -453,3 +453,10 @@ int gma_lvds_set_property(struct drm_connector *connector,
 	return 0;
 }
 
+const struct drm_connector_funcs gma_lvds_connector_funcs = {
+	.dpms = drm_helper_connector_dpms,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.set_property = gma_lvds_set_property,
+	.destroy = gma_lvds_destroy,
+};
+
diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
index ea261a60e9ff..bcb2efa7a1b5 100644
--- a/drivers/gpu/drm/gma500/gma_lvds.h
+++ b/drivers/gpu/drm/gma500/gma_lvds.h
@@ -30,11 +30,9 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
 			 struct drm_display_mode *adjusted_mode);
 void gma_lvds_prepare(struct drm_encoder *encoder);
 void gma_lvds_commit(struct drm_encoder *encoder);
-void gma_lvds_destroy(struct drm_connector *connector);
-int gma_lvds_set_property(struct drm_connector *connector,
-			  struct drm_property *property, uint64_t value);
 
 extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
 extern const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs;
+extern const struct drm_connector_funcs gma_lvds_connector_funcs;
 
 #endif
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 7724ebd71aa8..ea2745116f92 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -217,7 +217,7 @@ void oaktrail_lvds_init(struct drm_device *dev,
 	encoder = &gma_encoder->base;
 	dev_priv->is_lvds_on = true;
 	ret = drm_connector_init(dev, connector,
-				 &psb_intel_lvds_connector_funcs,
+				 &gma_lvds_connector_funcs,
 				 DRM_MODE_CONNECTOR_LVDS);
 	if (ret)
 		goto err_free_connector;
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
index df2e770c660a..4d9b9b45525f 100644
--- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
@@ -132,13 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
 		psb_lvds_pwm_set_brightness(dev, level);
 }
 
-const struct drm_connector_funcs psb_intel_lvds_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.set_property = gma_lvds_set_property,
-	.destroy = gma_lvds_destroy,
-};
-
 /**
  * psb_intel_lvds_init - setup LVDS connectors on this device
  * @dev: drm device
@@ -197,7 +190,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
 	}
 
 	ret = drm_connector_init_with_ddc(dev, connector,
-					  &psb_intel_lvds_connector_funcs,
+					  &gma_lvds_connector_funcs,
 					  DRM_MODE_CONNECTOR_LVDS,
 					  &ddc_bus->base);
 	if (ret)
-- 
2.36.1

Re: [PATCH 01/19] drm/gma500: Unify *_lvds_get_max_backlight()

[Sam Ravnborg]

Hi Patrick.

On Mon, Jun 13, 2022 at 02:34:18PM +0200, Patrik Jakobsson wrote:
> These functions mostly do the same thing so unify them into one. All
> unified lvds code will live in gma_lvds.c.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
> +/*
> + * Returns the maximum level of the backlight duty cycle field.
> + */

I find this function quite un-readable.

> +u32 gma_lvds_get_max_backlight(struct drm_device *dev)
> +{
> +	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +	u32 retval;
> +
> +	if (gma_power_begin(dev, false)) {
> +		retval = ((REG_READ(BLC_PWM_CTL) &
> +			  BACKLIGHT_MODULATION_FREQ_MASK) >>
> +			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> +
> +		gma_power_end(dev);
> +	} else
> +		retval = ((dev_priv->regs.saveBLC_PWM_CTL &
> +			  BACKLIGHT_MODULATION_FREQ_MASK) >>
> +			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> +
> +	return retval;
> +}

Maybe like this - which should be the same functionality:

u32 gma_lvds_get_max_backlight(struct drm_device *dev)
{
	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
	u32 pwmctl;

	if (gma_power_begin(dev, false)) {
		pwmctl = REG_READ(BLC_PWM_CTL);
		gma_power_end(dev);
	} else {
		pwmctl = dev_priv->regs.saveBLC_PWM_CTL;
	}
	
	pwmctl = pwmctl & BACKLIGHT_MODULATION_FREQ_MASK;
	return  (pwmctl >> BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
}

this is more or less the same as in psb_intel_lvds_get_max_backlight().

With or without this change the patch is:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

>> +
> +
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> new file mode 100644
> index 000000000000..2a9ce8ee3fa7
> --- /dev/null
> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +/*
> + * Copyright © 2006-2011 Intel Corporation
> + */
> +
> +#ifndef _GMA_LVDS_H
> +#define _GMA_LVDS_H
> +
> +u32 gma_lvds_get_max_backlight(struct drm_device *dev);
> +
> +#endif
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index 9c9ebf8e29c4..4913baca7ae2 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -20,6 +20,7 @@
>  #include "psb_drv.h"
>  #include "psb_intel_drv.h"
>  #include "psb_intel_reg.h"
> +#include "gma_lvds.h"
>  
>  /* The max/min PWM frequency in BPCR[31:17] - */
>  /* The smallest number is 1 (not 0) that can fit in the
> @@ -170,25 +171,6 @@ static void oaktrail_lvds_prepare(struct drm_encoder *encoder)
>  	gma_power_end(dev);
>  }
>  
> -static u32 oaktrail_lvds_get_max_backlight(struct drm_device *dev)
> -{
> -	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -	u32 ret;
> -
> -	if (gma_power_begin(dev, false)) {
> -		ret = ((REG_READ(BLC_PWM_CTL) &
> -			  BACKLIGHT_MODULATION_FREQ_MASK) >>
> -			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> -
> -		gma_power_end(dev);
> -	} else
> -		ret = ((dev_priv->regs.saveBLC_PWM_CTL &
> -			  BACKLIGHT_MODULATION_FREQ_MASK) >>
> -			  BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> -
> -	return ret;
> -}
> -
>  static void oaktrail_lvds_commit(struct drm_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->dev;
> @@ -197,8 +179,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
>  	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
>  
>  	if (mode_dev->backlight_duty_cycle == 0)
> -		mode_dev->backlight_duty_cycle =
> -					oaktrail_lvds_get_max_backlight(dev);
> +		mode_dev->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
>  	oaktrail_lvds_set_power(dev, gma_encoder, true);
>  }
>  
> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> index 7ee6c8ce103b..371c202a15ce 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> @@ -18,6 +18,7 @@
>  #include "psb_drv.h"
>  #include "psb_intel_drv.h"
>  #include "psb_intel_reg.h"
> +#include "gma_lvds.h"
>  
>  /*
>   * LVDS I2C backlight control macros
> @@ -52,32 +53,6 @@ struct psb_intel_lvds_priv {
>  	struct gma_i2c_chan *i2c_bus;
>  };
>  
> -
> -/*
> - * Returns the maximum level of the backlight duty cycle field.
> - */
> -static u32 psb_intel_lvds_get_max_backlight(struct drm_device *dev)
> -{
> -	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -	u32 ret;
> -
> -	if (gma_power_begin(dev, false)) {
> -		ret = REG_READ(BLC_PWM_CTL);
> -		gma_power_end(dev);
> -	} else /* Powered off, use the saved value */
> -		ret = dev_priv->regs.saveBLC_PWM_CTL;
> -
> -	/* Top 15bits hold the frequency mask */
> -	ret = (ret &  BACKLIGHT_MODULATION_FREQ_MASK) >>
> -					BACKLIGHT_MODULATION_FREQ_SHIFT;
> -
> -        ret *= 2;	/* Return a 16bit range as needed for setting */
> -        if (ret == 0)
> -                dev_err(dev->dev, "BL bug: Reg %08x save %08X\n",
> -                        REG_READ(BLC_PWM_CTL), dev_priv->regs.saveBLC_PWM_CTL);
> -	return ret;
> -}
> -
>  /*
>   * Set LVDS backlight level by I2C command
>   *
> @@ -131,7 +106,7 @@ static int psb_lvds_pwm_set_brightness(struct drm_device *dev, int level)
>  	u32 max_pwm_blc;
>  	u32 blc_pwm_duty_cycle;
>  
> -	max_pwm_blc = psb_intel_lvds_get_max_backlight(dev);
> +	max_pwm_blc = gma_lvds_get_max_backlight(dev);
>  
>  	/*BLC_PWM_CTL Should be initiated while backlight device init*/
>  	BUG_ON(max_pwm_blc == 0);
> @@ -176,7 +151,7 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
>  /*
>   * Sets the backlight level.
>   *
> - * level: backlight level, from 0 to psb_intel_lvds_get_max_backlight().
> + * level: backlight level, from 0 to gma_lvds_get_max_backlight().
>   */
>  static void psb_intel_lvds_set_backlight(struct drm_device *dev, int level)
>  {
> @@ -275,8 +250,7 @@ static void psb_intel_lvds_save(struct drm_connector *connector)
>  	 * just make it full brightness
>  	 */
>  	if (dev_priv->backlight_duty_cycle == 0)
> -		dev_priv->backlight_duty_cycle =
> -		psb_intel_lvds_get_max_backlight(dev);
> +		dev_priv->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
>  
>  	dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
>  			lvds_priv->savePP_ON,
> @@ -445,7 +419,7 @@ static void psb_intel_lvds_commit(struct drm_encoder *encoder)
>  
>  	if (mode_dev->backlight_duty_cycle == 0)
>  		mode_dev->backlight_duty_cycle =
> -		    psb_intel_lvds_get_max_backlight(dev);
> +		    gma_lvds_get_max_backlight(dev);
>  
>  	psb_intel_lvds_set_power(dev, true);
>  }
> -- 
> 2.36.1

Re: [PATCH 01/19] drm/gma500: Unify *_lvds_get_max_backlight()

[Patrik Jakobsson]

On Mon, Jun 13, 2022 at 7:08 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Patrick.
>
> On Mon, Jun 13, 2022 at 02:34:18PM +0200, Patrik Jakobsson wrote:
> > These functions mostly do the same thing so unify them into one. All
> > unified lvds code will live in gma_lvds.c.
> >
> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> > ---
> > +/*
> > + * Returns the maximum level of the backlight duty cycle field.
> > + */
>
> I find this function quite un-readable.
>
> > +u32 gma_lvds_get_max_backlight(struct drm_device *dev)
> > +{
> > +     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > +     u32 retval;
> > +
> > +     if (gma_power_begin(dev, false)) {
> > +             retval = ((REG_READ(BLC_PWM_CTL) &
> > +                       BACKLIGHT_MODULATION_FREQ_MASK) >>
> > +                       BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> > +
> > +             gma_power_end(dev);
> > +     } else
> > +             retval = ((dev_priv->regs.saveBLC_PWM_CTL &
> > +                       BACKLIGHT_MODULATION_FREQ_MASK) >>
> > +                       BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> > +
> > +     return retval;
> > +}
>
> Maybe like this - which should be the same functionality:
>
> u32 gma_lvds_get_max_backlight(struct drm_device *dev)
> {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>         u32 pwmctl;
>
>         if (gma_power_begin(dev, false)) {
>                 pwmctl = REG_READ(BLC_PWM_CTL);
>                 gma_power_end(dev);
>         } else {
>                 pwmctl = dev_priv->regs.saveBLC_PWM_CTL;
>         }
>
>         pwmctl = pwmctl & BACKLIGHT_MODULATION_FREQ_MASK;
>         return  (pwmctl >> BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> }
>
> this is more or less the same as in psb_intel_lvds_get_max_backlight().
>
> With or without this change the patch is:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Hi Sam,
Thanks for having a look.

I've intentionally tried to change as little as possible from the
version I copied so that any functional change is easy to spot and the
series becomes easy to review. Would it be ok if I do cleanups as a
followup series?

-Patrik

>
> >> +
> > +
> > diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> > new file mode 100644
> > index 000000000000..2a9ce8ee3fa7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +/*
> > + * Copyright © 2006-2011 Intel Corporation
> > + */
> > +
> > +#ifndef _GMA_LVDS_H
> > +#define _GMA_LVDS_H
> > +
> > +u32 gma_lvds_get_max_backlight(struct drm_device *dev);
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > index 9c9ebf8e29c4..4913baca7ae2 100644
> > --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > @@ -20,6 +20,7 @@
> >  #include "psb_drv.h"
> >  #include "psb_intel_drv.h"
> >  #include "psb_intel_reg.h"
> > +#include "gma_lvds.h"
> >
> >  /* The max/min PWM frequency in BPCR[31:17] - */
> >  /* The smallest number is 1 (not 0) that can fit in the
> > @@ -170,25 +171,6 @@ static void oaktrail_lvds_prepare(struct drm_encoder *encoder)
> >       gma_power_end(dev);
> >  }
> >
> > -static u32 oaktrail_lvds_get_max_backlight(struct drm_device *dev)
> > -{
> > -     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > -     u32 ret;
> > -
> > -     if (gma_power_begin(dev, false)) {
> > -             ret = ((REG_READ(BLC_PWM_CTL) &
> > -                       BACKLIGHT_MODULATION_FREQ_MASK) >>
> > -                       BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> > -
> > -             gma_power_end(dev);
> > -     } else
> > -             ret = ((dev_priv->regs.saveBLC_PWM_CTL &
> > -                       BACKLIGHT_MODULATION_FREQ_MASK) >>
> > -                       BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
> > -
> > -     return ret;
> > -}
> > -
> >  static void oaktrail_lvds_commit(struct drm_encoder *encoder)
> >  {
> >       struct drm_device *dev = encoder->dev;
> > @@ -197,8 +179,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
> >       struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> >
> >       if (mode_dev->backlight_duty_cycle == 0)
> > -             mode_dev->backlight_duty_cycle =
> > -                                     oaktrail_lvds_get_max_backlight(dev);
> > +             mode_dev->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
> >       oaktrail_lvds_set_power(dev, gma_encoder, true);
> >  }
> >
> > diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> > index 7ee6c8ce103b..371c202a15ce 100644
> > --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> > +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> > @@ -18,6 +18,7 @@
> >  #include "psb_drv.h"
> >  #include "psb_intel_drv.h"
> >  #include "psb_intel_reg.h"
> > +#include "gma_lvds.h"
> >
> >  /*
> >   * LVDS I2C backlight control macros
> > @@ -52,32 +53,6 @@ struct psb_intel_lvds_priv {
> >       struct gma_i2c_chan *i2c_bus;
> >  };
> >
> > -
> > -/*
> > - * Returns the maximum level of the backlight duty cycle field.
> > - */
> > -static u32 psb_intel_lvds_get_max_backlight(struct drm_device *dev)
> > -{
> > -     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > -     u32 ret;
> > -
> > -     if (gma_power_begin(dev, false)) {
> > -             ret = REG_READ(BLC_PWM_CTL);
> > -             gma_power_end(dev);
> > -     } else /* Powered off, use the saved value */
> > -             ret = dev_priv->regs.saveBLC_PWM_CTL;
> > -
> > -     /* Top 15bits hold the frequency mask */
> > -     ret = (ret &  BACKLIGHT_MODULATION_FREQ_MASK) >>
> > -                                     BACKLIGHT_MODULATION_FREQ_SHIFT;
> > -
> > -        ret *= 2;    /* Return a 16bit range as needed for setting */
> > -        if (ret == 0)
> > -                dev_err(dev->dev, "BL bug: Reg %08x save %08X\n",
> > -                        REG_READ(BLC_PWM_CTL), dev_priv->regs.saveBLC_PWM_CTL);
> > -     return ret;
> > -}
> > -
> >  /*
> >   * Set LVDS backlight level by I2C command
> >   *
> > @@ -131,7 +106,7 @@ static int psb_lvds_pwm_set_brightness(struct drm_device *dev, int level)
> >       u32 max_pwm_blc;
> >       u32 blc_pwm_duty_cycle;
> >
> > -     max_pwm_blc = psb_intel_lvds_get_max_backlight(dev);
> > +     max_pwm_blc = gma_lvds_get_max_backlight(dev);
> >
> >       /*BLC_PWM_CTL Should be initiated while backlight device init*/
> >       BUG_ON(max_pwm_blc == 0);
> > @@ -176,7 +151,7 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
> >  /*
> >   * Sets the backlight level.
> >   *
> > - * level: backlight level, from 0 to psb_intel_lvds_get_max_backlight().
> > + * level: backlight level, from 0 to gma_lvds_get_max_backlight().
> >   */
> >  static void psb_intel_lvds_set_backlight(struct drm_device *dev, int level)
> >  {
> > @@ -275,8 +250,7 @@ static void psb_intel_lvds_save(struct drm_connector *connector)
> >        * just make it full brightness
> >        */
> >       if (dev_priv->backlight_duty_cycle == 0)
> > -             dev_priv->backlight_duty_cycle =
> > -             psb_intel_lvds_get_max_backlight(dev);
> > +             dev_priv->backlight_duty_cycle = gma_lvds_get_max_backlight(dev);
> >
> >       dev_dbg(dev->dev, "(0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x)\n",
> >                       lvds_priv->savePP_ON,
> > @@ -445,7 +419,7 @@ static void psb_intel_lvds_commit(struct drm_encoder *encoder)
> >
> >       if (mode_dev->backlight_duty_cycle == 0)
> >               mode_dev->backlight_duty_cycle =
> > -                 psb_intel_lvds_get_max_backlight(dev);
> > +                 gma_lvds_get_max_backlight(dev);
> >
> >       psb_intel_lvds_set_power(dev, true);
> >  }
> > --
> > 2.36.1

Re: [PATCH 09/19] drm/gma500: Unify *_intel_lvds_mode_fixup()

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 11421 bytes --]

Hi

Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> These functions mostly do the same thing so unify them. Change a check
> of !IS_MRST() to IS_PSB() to not change the behaviour for CDV.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
>   drivers/gpu/drm/gma500/cdv_intel_lvds.c | 51 +------------------
>   drivers/gpu/drm/gma500/gma_lvds.c       | 59 ++++++++++++++++++++++
>   drivers/gpu/drm/gma500/gma_lvds.h       |  3 ++
>   drivers/gpu/drm/gma500/oaktrail_lvds.c  |  2 +-
>   drivers/gpu/drm/gma500/psb_intel_lvds.c | 65 +------------------------
>   5 files changed, 65 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index 61dc65964e21..65532308f1e7 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -39,55 +39,6 @@
>   #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
>   #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
>   
> -static bool cdv_intel_lvds_mode_fixup(struct drm_encoder *encoder,
> -				  const struct drm_display_mode *mode,
> -				  struct drm_display_mode *adjusted_mode)
> -{
> -	struct drm_device *dev = encoder->dev;
> -	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> -	struct drm_encoder *tmp_encoder;
> -	struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
> -
> -	/* Should never happen!! */
> -	list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
> -			    head) {
> -		if (tmp_encoder != encoder
> -		    && tmp_encoder->crtc == encoder->crtc) {
> -			pr_err("Can't enable LVDS and another encoder on the same pipe\n");
> -			return false;
> -		}
> -	}
> -
> -	/*
> -	 * If we have timings from the BIOS for the panel, put them in
> -	 * to the adjusted mode.  The CRTC will be set up for this mode,
> -	 * with the panel scaling set up to source from the H/VDisplay
> -	 * of the original mode.
> -	 */
> -	if (panel_fixed_mode != NULL) {
> -		adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
> -		adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
> -		adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
> -		adjusted_mode->htotal = panel_fixed_mode->htotal;
> -		adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
> -		adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
> -		adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
> -		adjusted_mode->vtotal = panel_fixed_mode->vtotal;
> -		adjusted_mode->clock = panel_fixed_mode->clock;
> -		drm_mode_set_crtcinfo(adjusted_mode,
> -				      CRTC_INTERLACE_HALVE_V);
> -	}
> -
> -	/*
> -	 * XXX: It would be nice to support lower refresh rates on the
> -	 * panels to reduce power consumption, and perhaps match the
> -	 * user's requested refresh rate.
> -	 */
> -
> -	return true;
> -}
> -
>   static void cdv_intel_lvds_prepare(struct drm_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->dev;
> @@ -255,7 +206,7 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
>   static const struct drm_encoder_helper_funcs
>   					cdv_intel_lvds_helper_funcs = {
>   	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = cdv_intel_lvds_mode_fixup,
> +	.mode_fixup = gma_lvds_mode_fixup,
>   	.prepare = cdv_intel_lvds_prepare,
>   	.mode_set = cdv_intel_lvds_mode_set,
>   	.commit = cdv_intel_lvds_commit,
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
> index 19679ee36062..32ecf5835828 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.c
> +++ b/drivers/gpu/drm/gma500/gma_lvds.c
> @@ -209,3 +209,62 @@ void gma_lvds_restore(struct drm_connector *connector)
>   	}
>   }
>   
> +bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
> +			 const struct drm_display_mode *mode,
> +			 struct drm_display_mode *adjusted_mode)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> +	struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
> +	struct drm_encoder *tmp_encoder;
> +	struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
> +
> +	/* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
> +	if (IS_PSB(dev) && gma_crtc->pipe == 0) {
> +		pr_err("Can't support LVDS on pipe A\n");
> +		return false;
> +	}
> +	if (IS_MRST(dev) && gma_crtc->pipe != 0) {
> +		pr_err("Must use PIPE A\n");
> +		return false;
> +	}

In my experience, per-model branching is horrible for maintenance.

I suggest to keep a _lvds_mode_fixup function for each model, each wit 
the rsp model. The rest of gma_lvds_mode_fixup() can than be a shared 
helper for all those model-specific functions.

> +	/* Should never happen!! */
> +	list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
> +			    head) {
> +		if (tmp_encoder != encoder
> +		    && tmp_encoder->crtc == encoder->crtc) {
> +			pr_err("Can't enable LVDS and another encoder on the same pipe\n");
> +			return false;
> +		}
> +	}
> +
> +	/*
> +	 * If we have timings from the BIOS for the panel, put them in
> +	 * to the adjusted mode.  The CRTC will be set up for this mode,
> +	 * with the panel scaling set up to source from the H/VDisplay
> +	 * of the original mode.
> +	 */
> +	if (panel_fixed_mode != NULL) {
> +		adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
> +		adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
> +		adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
> +		adjusted_mode->htotal = panel_fixed_mode->htotal;
> +		adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
> +		adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
> +		adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
> +		adjusted_mode->vtotal = panel_fixed_mode->vtotal;
> +		adjusted_mode->clock = panel_fixed_mode->clock;
> +		drm_mode_set_crtcinfo(adjusted_mode,
> +				      CRTC_INTERLACE_HALVE_V);
> +	}
> +
> +	/*
> +	 * XXX: It would be nice to support lower refresh rates on the
> +	 * panels to reduce power consumption, and perhaps match the
> +	 * user's requested refresh rate.
> +	 */
> +
> +	return true;
> +}
> +
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> index eee0da00a0cf..950ab7b88ad6 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.h
> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> @@ -30,5 +30,8 @@ enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
>   void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
>   void gma_lvds_save(struct drm_connector *connector);
>   void gma_lvds_restore(struct drm_connector *connector);
> +bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
> +			 const struct drm_display_mode *mode,
> +			 struct drm_display_mode *adjusted_mode);
>   
>   #endif
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index 00ec4fea4c12..16699b0fbbc9 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -134,7 +134,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
>   
>   static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
>   	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = psb_intel_lvds_mode_fixup,
> +	.mode_fixup = gma_lvds_mode_fixup,
>   	.prepare = oaktrail_lvds_prepare,
>   	.mode_set = oaktrail_lvds_mode_set,
>   	.commit = oaktrail_lvds_commit,
> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> index 6e5abb670bff..317bd1fcfa2a 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> @@ -132,69 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
>   		psb_lvds_pwm_set_brightness(dev, level);
>   }
>   
> -bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
> -				  const struct drm_display_mode *mode,
> -				  struct drm_display_mode *adjusted_mode)
> -{
> -	struct drm_device *dev = encoder->dev;
> -	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -	struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> -	struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
> -	struct drm_encoder *tmp_encoder;
> -	struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
> -	struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
> -
> -	if (gma_encoder->type == INTEL_OUTPUT_MIPI2)
> -		panel_fixed_mode = mode_dev->panel_fixed_mode2;

What happened to this test? I cannot see it in the new helper.

Best regards
Thomas

> -
> -	/* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
> -	if (!IS_MRST(dev) && gma_crtc->pipe == 0) {
> -		pr_err("Can't support LVDS on pipe A\n");
> -		return false;
> -	}
> -	if (IS_MRST(dev) && gma_crtc->pipe != 0) {
> -		pr_err("Must use PIPE A\n");
> -		return false;
> -	}
> -	/* Should never happen!! */
> -	list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
> -			    head) {
> -		if (tmp_encoder != encoder
> -		    && tmp_encoder->crtc == encoder->crtc) {
> -			pr_err("Can't enable LVDS and another encoder on the same pipe\n");
> -			return false;
> -		}
> -	}
> -
> -	/*
> -	 * If we have timings from the BIOS for the panel, put them in
> -	 * to the adjusted mode.  The CRTC will be set up for this mode,
> -	 * with the panel scaling set up to source from the H/VDisplay
> -	 * of the original mode.
> -	 */
> -	if (panel_fixed_mode != NULL) {
> -		adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
> -		adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
> -		adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
> -		adjusted_mode->htotal = panel_fixed_mode->htotal;
> -		adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
> -		adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
> -		adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
> -		adjusted_mode->vtotal = panel_fixed_mode->vtotal;
> -		adjusted_mode->clock = panel_fixed_mode->clock;
> -		drm_mode_set_crtcinfo(adjusted_mode,
> -				      CRTC_INTERLACE_HALVE_V);
> -	}
> -
> -	/*
> -	 * XXX: It would be nice to support lower refresh rates on the
> -	 * panels to reduce power consumption, and perhaps match the
> -	 * user's requested refresh rate.
> -	 */
> -
> -	return true;
> -}
> -
>   static void psb_intel_lvds_prepare(struct drm_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->dev;
> @@ -365,7 +302,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
>   
>   static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
>   	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = psb_intel_lvds_mode_fixup,
> +	.mode_fixup = gma_lvds_mode_fixup,
>   	.prepare = psb_intel_lvds_prepare,
>   	.mode_set = psb_intel_lvds_mode_set,
>   	.commit = psb_intel_lvds_commit,

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 13/19] drm/gma500: Unify struct *_intel_lvds_helper_funcs

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 5547 bytes --]

Hi

Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> These now only contains generic gma functions so create
> gma_lvds_helper_funcs that both PSB and CDV can use. Oaktrail still
> needs the modeset callback refactored to align with PSB and CDV.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
>   drivers/gpu/drm/gma500/cdv_intel_lvds.c | 11 +----------
>   drivers/gpu/drm/gma500/gma_lvds.c       | 14 +++++++++++---
>   drivers/gpu/drm/gma500/gma_lvds.h       |  5 ++---
>   drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +---------
>   4 files changed, 15 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index ddfb976b6059..80ccc00c47e5 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -136,15 +136,6 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
>   	return 0;
>   }
>   
> -static const struct drm_encoder_helper_funcs
> -					cdv_intel_lvds_helper_funcs = {
> -	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = gma_lvds_mode_fixup,
> -	.prepare = gma_lvds_prepare,
> -	.mode_set = gma_lvds_mode_set,
> -	.commit = gma_lvds_commit,
> -};
> -
>   static const struct drm_connector_helper_funcs
>   				cdv_intel_lvds_connector_helper_funcs = {
>   	.get_modes = cdv_intel_lvds_get_modes,
> @@ -286,7 +277,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
>   	gma_connector_attach_encoder(gma_connector, gma_encoder);
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
> -	drm_encoder_helper_add(encoder, &cdv_intel_lvds_helper_funcs);
> +	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
>   	drm_connector_helper_add(connector,
>   				 &cdv_intel_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
> index 215bf8f7d41f..bf9fa5ebdbd3 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.c
> +++ b/drivers/gpu/drm/gma500/gma_lvds.c
> @@ -299,9 +299,9 @@ void gma_lvds_commit(struct drm_encoder *encoder)
>   	gma_lvds_set_power(dev, true);
>   }
>   
> -void gma_lvds_mode_set(struct drm_encoder *encoder,
> -		       struct drm_display_mode *mode,
> -		       struct drm_display_mode *adjusted_mode)
> +static void gma_lvds_mode_set(struct drm_encoder *encoder,
> +			      struct drm_display_mode *mode,
> +			      struct drm_display_mode *adjusted_mode)
>   {
>   	struct drm_device *dev = encoder->dev;
>   	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> @@ -334,3 +334,11 @@ void gma_lvds_mode_set(struct drm_encoder *encoder,
>   	REG_WRITE(PFIT_CONTROL, pfit_control);
>   }
>   
> +const struct drm_encoder_helper_funcs gma_lvds_helper_funcs = {
> +	.dpms = gma_lvds_encoder_dpms,
> +	.mode_fixup = gma_lvds_mode_fixup,
> +	.prepare = gma_lvds_prepare,
> +	.mode_set = gma_lvds_mode_set,
> +	.commit = gma_lvds_commit,
> +};
> +

Alternatively, you could use an initializer macro, such as

#define GMA_LVDS_HELPER_FUNCS \
   .dpms = ..
   .mode_fixup = ...
   ...

and use it to initialize the per-model model instances. This would allow 
to keep each instance as 'static const'.  The linker should be able to 
sort out duplicates.

Best regards
Thomas

> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> index ebba869a25b7..3c47bea859ad 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.h
> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> @@ -34,8 +34,7 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
>   			 struct drm_display_mode *adjusted_mode);
>   void gma_lvds_prepare(struct drm_encoder *encoder);
>   void gma_lvds_commit(struct drm_encoder *encoder);
> -void gma_lvds_mode_set(struct drm_encoder *encoder,
> -		       struct drm_display_mode *mode,
> -		       struct drm_display_mode *adjusted_mode);
> +
> +extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
>   
>   #endif
> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> index 553f6cb5f322..29a9b4ea5803 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> @@ -236,14 +236,6 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
>   	return -1;
>   }
>   
> -static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
> -	.dpms = gma_lvds_encoder_dpms,
> -	.mode_fixup = gma_lvds_mode_fixup,
> -	.prepare = gma_lvds_prepare,
> -	.mode_set = gma_lvds_mode_set,
> -	.commit = gma_lvds_commit,
> -};
> -
>   const struct drm_connector_helper_funcs
>   				psb_intel_lvds_connector_helper_funcs = {
>   	.get_modes = psb_intel_lvds_get_modes,
> @@ -329,7 +321,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
>   	gma_connector_attach_encoder(gma_connector, gma_encoder);
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
> -	drm_encoder_helper_add(encoder, &psb_intel_lvds_helper_funcs);
> +	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
>   	drm_connector_helper_add(connector,
>   				 &psb_intel_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 15/19] drm/gma500: Unify struct *_intel_lvds_connector_helper_funcs

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 6750 bytes --]

Hi Patrik

Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> These now only contains generic gma functions so create
> gma_lvds_connector_helper_funcs that all chips can use.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
>   drivers/gpu/drm/gma500/cdv_intel_lvds.c | 10 +---------
>   drivers/gpu/drm/gma500/gma_lvds.c       | 12 +++++++++---
>   drivers/gpu/drm/gma500/gma_lvds.h       |  4 +---
>   drivers/gpu/drm/gma500/oaktrail_lvds.c  |  3 +--
>   drivers/gpu/drm/gma500/psb_intel_lvds.c | 10 +---------
>   5 files changed, 13 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index a6f94572baaf..2ba635513401 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -111,13 +111,6 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
>   	return 0;
>   }
>   
> -static const struct drm_connector_helper_funcs
> -				cdv_intel_lvds_connector_helper_funcs = {
> -	.get_modes = gma_lvds_get_modes,
> -	.mode_valid = gma_lvds_mode_valid,
> -	.best_encoder = gma_best_encoder,
> -};
> -
>   static const struct drm_connector_funcs cdv_intel_lvds_connector_funcs = {
>   	.dpms = drm_helper_connector_dpms,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> @@ -253,8 +246,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
>   	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
> -	drm_connector_helper_add(connector,
> -				 &cdv_intel_lvds_connector_helper_funcs);
> +	drm_connector_helper_add(connector, &gma_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
>   	connector->interlace_allowed = false;
>   	connector->doublescan_allowed = false;
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
> index 49c11b5337cb..3b48999105d1 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.c
> +++ b/drivers/gpu/drm/gma500/gma_lvds.c
> @@ -94,8 +94,8 @@ static void gma_lvds_set_power(struct drm_device *dev, bool on)
>   	gma_power_end(dev);
>   }
>   
> -enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
> -					 struct drm_display_mode *mode)
> +static enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
> +						struct drm_display_mode *mode)
>   {
>   	struct drm_device *dev = connector->dev;
>   	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> @@ -345,7 +345,7 @@ const struct drm_encoder_helper_funcs gma_lvds_helper_funcs = {
>   /*
>    * Return the list of DDC modes if available, or the BIOS fixed mode otherwise.
>    */
> -int gma_lvds_get_modes(struct drm_connector *connector)
> +static int gma_lvds_get_modes(struct drm_connector *connector)
>   {
>   	struct drm_device *dev = connector->dev;
>   	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> @@ -368,3 +368,9 @@ int gma_lvds_get_modes(struct drm_connector *connector)
>   	return 0;
>   }
>   
> +const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs = {
> +	.get_modes = gma_lvds_get_modes,
> +	.mode_valid = gma_lvds_mode_valid,
> +	.best_encoder = gma_best_encoder,
> +};

Same suggestion as for pathc 13.

Best regards
Thomas

> +
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> index 70c416d8b7a7..5c7da22400fb 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.h
> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> @@ -24,8 +24,6 @@ struct gma_lvds_priv {
>   };
>   
>   u32 gma_lvds_get_max_backlight(struct drm_device *dev);
> -enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
> -					 struct drm_display_mode *mode);
>   void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
>   void gma_lvds_save(struct drm_connector *connector);
>   void gma_lvds_restore(struct drm_connector *connector);
> @@ -34,8 +32,8 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
>   			 struct drm_display_mode *adjusted_mode);
>   void gma_lvds_prepare(struct drm_encoder *encoder);
>   void gma_lvds_commit(struct drm_encoder *encoder);
> -int gma_lvds_get_modes(struct drm_connector *connector);
>   
>   extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
> +extern const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs;
>   
>   #endif
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index 85cab0f7028a..7724ebd71aa8 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -230,8 +230,7 @@ void oaktrail_lvds_init(struct drm_device *dev,
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
>   	drm_encoder_helper_add(encoder, &oaktrail_lvds_helper_funcs);
> -	drm_connector_helper_add(connector,
> -				 &psb_intel_lvds_connector_helper_funcs);
> +	drm_connector_helper_add(connector, &gma_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
>   	connector->interlace_allowed = false;
>   	connector->doublescan_allowed = false;
> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> index 4ef0150c6a03..f129e53f0233 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> @@ -210,13 +210,6 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
>   	return -1;
>   }
>   
> -const struct drm_connector_helper_funcs
> -				psb_intel_lvds_connector_helper_funcs = {
> -	.get_modes = gma_lvds_get_modes,
> -	.mode_valid = gma_lvds_mode_valid,
> -	.best_encoder = gma_best_encoder,
> -};
> -
>   const struct drm_connector_funcs psb_intel_lvds_connector_funcs = {
>   	.dpms = drm_helper_connector_dpms,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
> @@ -296,8 +289,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
>   	gma_encoder->type = INTEL_OUTPUT_LVDS;
>   
>   	drm_encoder_helper_add(encoder, &gma_lvds_helper_funcs);
> -	drm_connector_helper_add(connector,
> -				 &psb_intel_lvds_connector_helper_funcs);
> +	drm_connector_helper_add(connector, &gma_lvds_connector_helper_funcs);
>   	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
>   	connector->interlace_allowed = false;
>   	connector->doublescan_allowed = false;

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 19/19] drm/gma500: Unify struct *_intel_lvds_connector_funcs

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 5886 bytes --]

Hi

Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> These now only contains generic drm/gma functions so create
> gma_lvds_connector_funcs that all chips can use.
> 
> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
>   drivers/gpu/drm/gma500/cdv_intel_lvds.c |  9 +--------
>   drivers/gpu/drm/gma500/gma_lvds.c       | 15 +++++++++++----
>   drivers/gpu/drm/gma500/gma_lvds.h       |  4 +---
>   drivers/gpu/drm/gma500/oaktrail_lvds.c  |  2 +-
>   drivers/gpu/drm/gma500/psb_intel_lvds.c |  9 +--------
>   5 files changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index ccb489d6795b..0b8f818ca9b5 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -39,13 +39,6 @@
>   #define PSB_BACKLIGHT_PWM_CTL_SHIFT	(16)
>   #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
>   
> -static const struct drm_connector_funcs cdv_intel_lvds_connector_funcs = {
> -	.dpms = drm_helper_connector_dpms,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.set_property = gma_lvds_set_property,
> -	.destroy = gma_lvds_destroy,
> -};
> -
>   /*
>    * Enumerate the child dev array parsed from VBT to check whether
>    * the LVDS is present.
> @@ -160,7 +153,7 @@ void cdv_intel_lvds_init(struct drm_device *dev,
>   	}
>   
>   	ret = drm_connector_init_with_ddc(dev, connector,
> -					  &cdv_intel_lvds_connector_funcs,
> +					  &gma_lvds_connector_funcs,
>   					  DRM_MODE_CONNECTOR_LVDS,
>   					  &ddc_bus->base);
>   	if (ret)
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
> index e4278d26f811..612013baf7ad 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.c
> +++ b/drivers/gpu/drm/gma500/gma_lvds.c
> @@ -374,7 +374,7 @@ const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs = {
>   	.best_encoder = gma_best_encoder,
>   };
>   
> -void gma_lvds_destroy(struct drm_connector *connector)
> +static void gma_lvds_destroy(struct drm_connector *connector)
>   {
>   	struct gma_connector *gma_connector = to_gma_connector(connector);
>   	struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
> @@ -389,9 +389,9 @@ void gma_lvds_destroy(struct drm_connector *connector)
>   	kfree(gma_connector);
>   }
>   
> -int gma_lvds_set_property(struct drm_connector *connector,
> -			  struct drm_property *property,
> -			  uint64_t value)
> +static int gma_lvds_set_property(struct drm_connector *connector,
> +				 struct drm_property *property,
> +				 uint64_t value)
>   {
>   	struct drm_encoder *encoder = connector->encoder;
>   
> @@ -453,3 +453,10 @@ int gma_lvds_set_property(struct drm_connector *connector,
>   	return 0;
>   }
>   
> +const struct drm_connector_funcs gma_lvds_connector_funcs = {
> +	.dpms = drm_helper_connector_dpms,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.set_property = gma_lvds_set_property,
> +	.destroy = gma_lvds_destroy,
> +};

Again, same suggestion as for patch 13.

> +
> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> index ea261a60e9ff..bcb2efa7a1b5 100644
> --- a/drivers/gpu/drm/gma500/gma_lvds.h
> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> @@ -30,11 +30,9 @@ bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
>   			 struct drm_display_mode *adjusted_mode);
>   void gma_lvds_prepare(struct drm_encoder *encoder);
>   void gma_lvds_commit(struct drm_encoder *encoder);
> -void gma_lvds_destroy(struct drm_connector *connector);
> -int gma_lvds_set_property(struct drm_connector *connector,
> -			  struct drm_property *property, uint64_t value);
>   
>   extern const struct drm_encoder_helper_funcs gma_lvds_helper_funcs;
>   extern const struct drm_connector_helper_funcs gma_lvds_connector_helper_funcs;
> +extern const struct drm_connector_funcs gma_lvds_connector_funcs;
>   
>   #endif
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index 7724ebd71aa8..ea2745116f92 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -217,7 +217,7 @@ void oaktrail_lvds_init(struct drm_device *dev,
>   	encoder = &gma_encoder->base;
>   	dev_priv->is_lvds_on = true;
>   	ret = drm_connector_init(dev, connector,
> -				 &psb_intel_lvds_connector_funcs,
> +				 &gma_lvds_connector_funcs,
>   				 DRM_MODE_CONNECTOR_LVDS);
>   	if (ret)
>   		goto err_free_connector;
> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> index df2e770c660a..4d9b9b45525f 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> @@ -132,13 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
>   		psb_lvds_pwm_set_brightness(dev, level);
>   }
>   
> -const struct drm_connector_funcs psb_intel_lvds_connector_funcs = {
> -	.dpms = drm_helper_connector_dpms,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.set_property = gma_lvds_set_property,
> -	.destroy = gma_lvds_destroy,
> -};
> -
>   /**
>    * psb_intel_lvds_init - setup LVDS connectors on this device
>    * @dev: drm device
> @@ -197,7 +190,7 @@ void psb_intel_lvds_init(struct drm_device *dev,
>   	}
>   
>   	ret = drm_connector_init_with_ddc(dev, connector,
> -					  &psb_intel_lvds_connector_funcs,
> +					  &gma_lvds_connector_funcs,
>   					  DRM_MODE_CONNECTOR_LVDS,
>   					  &ddc_bus->base);
>   	if (ret)

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 00/19] drm/gma500: Unify most of the lvds code

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2367 bytes --]

Hi Patrik,

I've send comments on a few patches. For the other patches, you can add

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> Much of the lvds code for Poulsbo, Oaktrail and Cedarview are almost
> identical so there is no need to keep three copies of it. Unify as much
> as possible into one implementation. There are still things like the
> init code that can be unified but that requires unifying other parts of
> the driver first.
> 
> Patrik Jakobsson (19):
>    drm/gma500: Unify *_lvds_get_max_backlight()
>    drm/gma500: Unify *_lvds_set_backlight()
>    drm/gma500: Unify *_lvds_set_power()
>    drm/gma500: Unify *_lvds_mode_valid()
>    drm/gma500: Unify *_lvds_encoder_dpms()
>    drm/gma500: Unify *_intel_lvds_save()
>    drm/gma500: Unify struct *_intel_lvds_priv
>    drm/gma500: Unify *_intel_lvds_restore()
>    drm/gma500: Unify *_intel_lvds_mode_fixup()
>    drm/gma500: Unify *_intel_lvds_prepare()
>    drm/gma500: Unify *_intel_lvds_commit()
>    drm/gma500: Unify *_intel_lvds_mode_set()
>    drm/gma500: Unify struct *_intel_lvds_helper_funcs
>    drm/gma500: Unify *_intel_lvds_get_modes()
>    drm/gma500: Unify struct *_intel_lvds_connector_helper_funcs
>    drm/gma500: Use i2c_bus in gma_encoder for PSB
>    drm/gma500: Unify *_intel_lvds_destroy()
>    drm/gma500: Unify *_intel_lvds_set_property()
>    drm/gma500: Unify struct *_intel_lvds_connector_funcs
> 
>   drivers/gpu/drm/gma500/Makefile         |   1 +
>   drivers/gpu/drm/gma500/cdv_intel_lvds.c | 390 +-----------------
>   drivers/gpu/drm/gma500/gma_lvds.c       | 462 +++++++++++++++++++++
>   drivers/gpu/drm/gma500/gma_lvds.h       |  38 ++
>   drivers/gpu/drm/gma500/oaktrail_lvds.c  | 112 +-----
>   drivers/gpu/drm/gma500/psb_drv.h        |   1 -
>   drivers/gpu/drm/gma500/psb_intel_drv.h  |   2 -
>   drivers/gpu/drm/gma500/psb_intel_lvds.c | 507 +-----------------------
>   8 files changed, 530 insertions(+), 983 deletions(-)
>   create mode 100644 drivers/gpu/drm/gma500/gma_lvds.c
>   create mode 100644 drivers/gpu/drm/gma500/gma_lvds.h
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 09/19] drm/gma500: Unify *_intel_lvds_mode_fixup()

[Patrik Jakobsson]

On Tue, Jun 14, 2022 at 9:23 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
> > These functions mostly do the same thing so unify them. Change a check
> > of !IS_MRST() to IS_PSB() to not change the behaviour for CDV.
> >
> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> > ---
> >   drivers/gpu/drm/gma500/cdv_intel_lvds.c | 51 +------------------
> >   drivers/gpu/drm/gma500/gma_lvds.c       | 59 ++++++++++++++++++++++
> >   drivers/gpu/drm/gma500/gma_lvds.h       |  3 ++
> >   drivers/gpu/drm/gma500/oaktrail_lvds.c  |  2 +-
> >   drivers/gpu/drm/gma500/psb_intel_lvds.c | 65 +------------------------
> >   5 files changed, 65 insertions(+), 115 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> > index 61dc65964e21..65532308f1e7 100644
> > --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> > +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> > @@ -39,55 +39,6 @@
> >   #define PSB_BACKLIGHT_PWM_CTL_SHIFT (16)
> >   #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
> >
> > -static bool cdv_intel_lvds_mode_fixup(struct drm_encoder *encoder,
> > -                               const struct drm_display_mode *mode,
> > -                               struct drm_display_mode *adjusted_mode)
> > -{
> > -     struct drm_device *dev = encoder->dev;
> > -     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > -     struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> > -     struct drm_encoder *tmp_encoder;
> > -     struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
> > -
> > -     /* Should never happen!! */
> > -     list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
> > -                         head) {
> > -             if (tmp_encoder != encoder
> > -                 && tmp_encoder->crtc == encoder->crtc) {
> > -                     pr_err("Can't enable LVDS and another encoder on the same pipe\n");
> > -                     return false;
> > -             }
> > -     }
> > -
> > -     /*
> > -      * If we have timings from the BIOS for the panel, put them in
> > -      * to the adjusted mode.  The CRTC will be set up for this mode,
> > -      * with the panel scaling set up to source from the H/VDisplay
> > -      * of the original mode.
> > -      */
> > -     if (panel_fixed_mode != NULL) {
> > -             adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
> > -             adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
> > -             adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
> > -             adjusted_mode->htotal = panel_fixed_mode->htotal;
> > -             adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
> > -             adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
> > -             adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
> > -             adjusted_mode->vtotal = panel_fixed_mode->vtotal;
> > -             adjusted_mode->clock = panel_fixed_mode->clock;
> > -             drm_mode_set_crtcinfo(adjusted_mode,
> > -                                   CRTC_INTERLACE_HALVE_V);
> > -     }
> > -
> > -     /*
> > -      * XXX: It would be nice to support lower refresh rates on the
> > -      * panels to reduce power consumption, and perhaps match the
> > -      * user's requested refresh rate.
> > -      */
> > -
> > -     return true;
> > -}
> > -
> >   static void cdv_intel_lvds_prepare(struct drm_encoder *encoder)
> >   {
> >       struct drm_device *dev = encoder->dev;
> > @@ -255,7 +206,7 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
> >   static const struct drm_encoder_helper_funcs
> >                                       cdv_intel_lvds_helper_funcs = {
> >       .dpms = gma_lvds_encoder_dpms,
> > -     .mode_fixup = cdv_intel_lvds_mode_fixup,
> > +     .mode_fixup = gma_lvds_mode_fixup,
> >       .prepare = cdv_intel_lvds_prepare,
> >       .mode_set = cdv_intel_lvds_mode_set,
> >       .commit = cdv_intel_lvds_commit,
> > diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
> > index 19679ee36062..32ecf5835828 100644
> > --- a/drivers/gpu/drm/gma500/gma_lvds.c
> > +++ b/drivers/gpu/drm/gma500/gma_lvds.c
> > @@ -209,3 +209,62 @@ void gma_lvds_restore(struct drm_connector *connector)
> >       }
> >   }
> >
> > +bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
> > +                      const struct drm_display_mode *mode,
> > +                      struct drm_display_mode *adjusted_mode)
> > +{
> > +     struct drm_device *dev = encoder->dev;
> > +     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > +     struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> > +     struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
> > +     struct drm_encoder *tmp_encoder;
> > +     struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
> > +
> > +     /* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
> > +     if (IS_PSB(dev) && gma_crtc->pipe == 0) {
> > +             pr_err("Can't support LVDS on pipe A\n");
> > +             return false;
> > +     }
> > +     if (IS_MRST(dev) && gma_crtc->pipe != 0) {
> > +             pr_err("Must use PIPE A\n");
> > +             return false;
> > +     }
>
> In my experience, per-model branching is horrible for maintenance.
>
> I suggest to keep a _lvds_mode_fixup function for each model, each wit
> the rsp model. The rest of gma_lvds_mode_fixup() can than be a shared
> helper for all those model-specific functions.

Hi Thomas, thanks for having a look.

I wasn't sure if I wanted to refactor the code in this series or not
so I ended up just doing the unification. I fully agree that the
IS_*() checks are horrible and need fixing in most cases. And as Sam
mentioned in 1/19 there are ways to clean up the code as well.

For the above code the even better way to do it is to use the device's
lvds_mask. That way the code can be device-independent and all chips
can use it.

I can respin this series with refactoring/cleanup included or I can
send out a v2 with the initialized macro and send the
refactoring/cleanup as a separate series.

Which do you prefer?

-Patrik

>
> > +     /* Should never happen!! */
> > +     list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
> > +                         head) {
> > +             if (tmp_encoder != encoder
> > +                 && tmp_encoder->crtc == encoder->crtc) {
> > +                     pr_err("Can't enable LVDS and another encoder on the same pipe\n");
> > +                     return false;
> > +             }
> > +     }
> > +
> > +     /*
> > +      * If we have timings from the BIOS for the panel, put them in
> > +      * to the adjusted mode.  The CRTC will be set up for this mode,
> > +      * with the panel scaling set up to source from the H/VDisplay
> > +      * of the original mode.
> > +      */
> > +     if (panel_fixed_mode != NULL) {
> > +             adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
> > +             adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
> > +             adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
> > +             adjusted_mode->htotal = panel_fixed_mode->htotal;
> > +             adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
> > +             adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
> > +             adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
> > +             adjusted_mode->vtotal = panel_fixed_mode->vtotal;
> > +             adjusted_mode->clock = panel_fixed_mode->clock;
> > +             drm_mode_set_crtcinfo(adjusted_mode,
> > +                                   CRTC_INTERLACE_HALVE_V);
> > +     }
> > +
> > +     /*
> > +      * XXX: It would be nice to support lower refresh rates on the
> > +      * panels to reduce power consumption, and perhaps match the
> > +      * user's requested refresh rate.
> > +      */
> > +
> > +     return true;
> > +}
> > +
> > diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
> > index eee0da00a0cf..950ab7b88ad6 100644
> > --- a/drivers/gpu/drm/gma500/gma_lvds.h
> > +++ b/drivers/gpu/drm/gma500/gma_lvds.h
> > @@ -30,5 +30,8 @@ enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
> >   void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
> >   void gma_lvds_save(struct drm_connector *connector);
> >   void gma_lvds_restore(struct drm_connector *connector);
> > +bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
> > +                      const struct drm_display_mode *mode,
> > +                      struct drm_display_mode *adjusted_mode);
> >
> >   #endif
> > diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > index 00ec4fea4c12..16699b0fbbc9 100644
> > --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> > @@ -134,7 +134,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
> >
> >   static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
> >       .dpms = gma_lvds_encoder_dpms,
> > -     .mode_fixup = psb_intel_lvds_mode_fixup,
> > +     .mode_fixup = gma_lvds_mode_fixup,
> >       .prepare = oaktrail_lvds_prepare,
> >       .mode_set = oaktrail_lvds_mode_set,
> >       .commit = oaktrail_lvds_commit,
> > diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> > index 6e5abb670bff..317bd1fcfa2a 100644
> > --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
> > +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
> > @@ -132,69 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
> >               psb_lvds_pwm_set_brightness(dev, level);
> >   }
> >
> > -bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
> > -                               const struct drm_display_mode *mode,
> > -                               struct drm_display_mode *adjusted_mode)
> > -{
> > -     struct drm_device *dev = encoder->dev;
> > -     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> > -     struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
> > -     struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
> > -     struct drm_encoder *tmp_encoder;
> > -     struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
> > -     struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
> > -
> > -     if (gma_encoder->type == INTEL_OUTPUT_MIPI2)
> > -             panel_fixed_mode = mode_dev->panel_fixed_mode2;
>
> What happened to this test? I cannot see it in the new helper.

We don't have MIPI support so I removed the test. I forgot to mention
it in this patch. Other patches should have a note about it when I
remove MIPI code.





>
> Best regards
> Thomas
>
> > -
> > -     /* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
> > -     if (!IS_MRST(dev) && gma_crtc->pipe == 0) {
> > -             pr_err("Can't support LVDS on pipe A\n");
> > -             return false;
> > -     }
> > -     if (IS_MRST(dev) && gma_crtc->pipe != 0) {
> > -             pr_err("Must use PIPE A\n");
> > -             return false;
> > -     }
> > -     /* Should never happen!! */
> > -     list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
> > -                         head) {
> > -             if (tmp_encoder != encoder
> > -                 && tmp_encoder->crtc == encoder->crtc) {
> > -                     pr_err("Can't enable LVDS and another encoder on the same pipe\n");
> > -                     return false;
> > -             }
> > -     }
> > -
> > -     /*
> > -      * If we have timings from the BIOS for the panel, put them in
> > -      * to the adjusted mode.  The CRTC will be set up for this mode,
> > -      * with the panel scaling set up to source from the H/VDisplay
> > -      * of the original mode.
> > -      */
> > -     if (panel_fixed_mode != NULL) {
> > -             adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
> > -             adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
> > -             adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
> > -             adjusted_mode->htotal = panel_fixed_mode->htotal;
> > -             adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
> > -             adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
> > -             adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
> > -             adjusted_mode->vtotal = panel_fixed_mode->vtotal;
> > -             adjusted_mode->clock = panel_fixed_mode->clock;
> > -             drm_mode_set_crtcinfo(adjusted_mode,
> > -                                   CRTC_INTERLACE_HALVE_V);
> > -     }
> > -
> > -     /*
> > -      * XXX: It would be nice to support lower refresh rates on the
> > -      * panels to reduce power consumption, and perhaps match the
> > -      * user's requested refresh rate.
> > -      */
> > -
> > -     return true;
> > -}
> > -
> >   static void psb_intel_lvds_prepare(struct drm_encoder *encoder)
> >   {
> >       struct drm_device *dev = encoder->dev;
> > @@ -365,7 +302,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
> >
> >   static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
> >       .dpms = gma_lvds_encoder_dpms,
> > -     .mode_fixup = psb_intel_lvds_mode_fixup,
> > +     .mode_fixup = gma_lvds_mode_fixup,
> >       .prepare = psb_intel_lvds_prepare,
> >       .mode_set = psb_intel_lvds_mode_set,
> >       .commit = psb_intel_lvds_commit,
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev

Re: [PATCH 09/19] drm/gma500: Unify *_intel_lvds_mode_fixup()

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 14713 bytes --]

Hi

Am 14.06.22 um 14:42 schrieb Patrik Jakobsson:
> On Tue, Jun 14, 2022 at 9:23 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 13.06.22 um 14:34 schrieb Patrik Jakobsson:
>>> These functions mostly do the same thing so unify them. Change a check
>>> of !IS_MRST() to IS_PSB() to not change the behaviour for CDV.
>>>
>>> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>>> ---
>>>    drivers/gpu/drm/gma500/cdv_intel_lvds.c | 51 +------------------
>>>    drivers/gpu/drm/gma500/gma_lvds.c       | 59 ++++++++++++++++++++++
>>>    drivers/gpu/drm/gma500/gma_lvds.h       |  3 ++
>>>    drivers/gpu/drm/gma500/oaktrail_lvds.c  |  2 +-
>>>    drivers/gpu/drm/gma500/psb_intel_lvds.c | 65 +------------------------
>>>    5 files changed, 65 insertions(+), 115 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
>>> index 61dc65964e21..65532308f1e7 100644
>>> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
>>> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
>>> @@ -39,55 +39,6 @@
>>>    #define PSB_BACKLIGHT_PWM_CTL_SHIFT (16)
>>>    #define PSB_BACKLIGHT_PWM_POLARITY_BIT_CLEAR (0xFFFE)
>>>
>>> -static bool cdv_intel_lvds_mode_fixup(struct drm_encoder *encoder,
>>> -                               const struct drm_display_mode *mode,
>>> -                               struct drm_display_mode *adjusted_mode)
>>> -{
>>> -     struct drm_device *dev = encoder->dev;
>>> -     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>> -     struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
>>> -     struct drm_encoder *tmp_encoder;
>>> -     struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
>>> -
>>> -     /* Should never happen!! */
>>> -     list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
>>> -                         head) {
>>> -             if (tmp_encoder != encoder
>>> -                 && tmp_encoder->crtc == encoder->crtc) {
>>> -                     pr_err("Can't enable LVDS and another encoder on the same pipe\n");
>>> -                     return false;
>>> -             }
>>> -     }
>>> -
>>> -     /*
>>> -      * If we have timings from the BIOS for the panel, put them in
>>> -      * to the adjusted mode.  The CRTC will be set up for this mode,
>>> -      * with the panel scaling set up to source from the H/VDisplay
>>> -      * of the original mode.
>>> -      */
>>> -     if (panel_fixed_mode != NULL) {
>>> -             adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
>>> -             adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
>>> -             adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
>>> -             adjusted_mode->htotal = panel_fixed_mode->htotal;
>>> -             adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
>>> -             adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
>>> -             adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
>>> -             adjusted_mode->vtotal = panel_fixed_mode->vtotal;
>>> -             adjusted_mode->clock = panel_fixed_mode->clock;
>>> -             drm_mode_set_crtcinfo(adjusted_mode,
>>> -                                   CRTC_INTERLACE_HALVE_V);
>>> -     }
>>> -
>>> -     /*
>>> -      * XXX: It would be nice to support lower refresh rates on the
>>> -      * panels to reduce power consumption, and perhaps match the
>>> -      * user's requested refresh rate.
>>> -      */
>>> -
>>> -     return true;
>>> -}
>>> -
>>>    static void cdv_intel_lvds_prepare(struct drm_encoder *encoder)
>>>    {
>>>        struct drm_device *dev = encoder->dev;
>>> @@ -255,7 +206,7 @@ static int cdv_intel_lvds_set_property(struct drm_connector *connector,
>>>    static const struct drm_encoder_helper_funcs
>>>                                        cdv_intel_lvds_helper_funcs = {
>>>        .dpms = gma_lvds_encoder_dpms,
>>> -     .mode_fixup = cdv_intel_lvds_mode_fixup,
>>> +     .mode_fixup = gma_lvds_mode_fixup,
>>>        .prepare = cdv_intel_lvds_prepare,
>>>        .mode_set = cdv_intel_lvds_mode_set,
>>>        .commit = cdv_intel_lvds_commit,
>>> diff --git a/drivers/gpu/drm/gma500/gma_lvds.c b/drivers/gpu/drm/gma500/gma_lvds.c
>>> index 19679ee36062..32ecf5835828 100644
>>> --- a/drivers/gpu/drm/gma500/gma_lvds.c
>>> +++ b/drivers/gpu/drm/gma500/gma_lvds.c
>>> @@ -209,3 +209,62 @@ void gma_lvds_restore(struct drm_connector *connector)
>>>        }
>>>    }
>>>
>>> +bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
>>> +                      const struct drm_display_mode *mode,
>>> +                      struct drm_display_mode *adjusted_mode)
>>> +{
>>> +     struct drm_device *dev = encoder->dev;
>>> +     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>> +     struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
>>> +     struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
>>> +     struct drm_encoder *tmp_encoder;
>>> +     struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
>>> +
>>> +     /* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
>>> +     if (IS_PSB(dev) && gma_crtc->pipe == 0) {
>>> +             pr_err("Can't support LVDS on pipe A\n");
>>> +             return false;
>>> +     }
>>> +     if (IS_MRST(dev) && gma_crtc->pipe != 0) {
>>> +             pr_err("Must use PIPE A\n");
>>> +             return false;
>>> +     }
>>
>> In my experience, per-model branching is horrible for maintenance.
>>
>> I suggest to keep a _lvds_mode_fixup function for each model, each wit
>> the rsp model. The rest of gma_lvds_mode_fixup() can than be a shared
>> helper for all those model-specific functions.
> 
> Hi Thomas, thanks for having a look.
> 
> I wasn't sure if I wanted to refactor the code in this series or not
> so I ended up just doing the unification. I fully agree that the
> IS_*() checks are horrible and need fixing in most cases. And as Sam
> mentioned in 1/19 there are ways to clean up the code as well.
> 
> For the above code the even better way to do it is to use the device's
> lvds_mask. That way the code can be device-independent and all chips
> can use it.
> 
> I can respin this series with refactoring/cleanup included or I can
> send out a v2 with the initialized macro and send the
> refactoring/cleanup as a separate series.
> 
> Which do you prefer?

I think it would be better to do the refactoring in a separate series.

Best regards
Thomas

> 
> -Patrik
> 
>>
>>> +     /* Should never happen!! */
>>> +     list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
>>> +                         head) {
>>> +             if (tmp_encoder != encoder
>>> +                 && tmp_encoder->crtc == encoder->crtc) {
>>> +                     pr_err("Can't enable LVDS and another encoder on the same pipe\n");
>>> +                     return false;
>>> +             }
>>> +     }
>>> +
>>> +     /*
>>> +      * If we have timings from the BIOS for the panel, put them in
>>> +      * to the adjusted mode.  The CRTC will be set up for this mode,
>>> +      * with the panel scaling set up to source from the H/VDisplay
>>> +      * of the original mode.
>>> +      */
>>> +     if (panel_fixed_mode != NULL) {
>>> +             adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
>>> +             adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
>>> +             adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
>>> +             adjusted_mode->htotal = panel_fixed_mode->htotal;
>>> +             adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
>>> +             adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
>>> +             adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
>>> +             adjusted_mode->vtotal = panel_fixed_mode->vtotal;
>>> +             adjusted_mode->clock = panel_fixed_mode->clock;
>>> +             drm_mode_set_crtcinfo(adjusted_mode,
>>> +                                   CRTC_INTERLACE_HALVE_V);
>>> +     }
>>> +
>>> +     /*
>>> +      * XXX: It would be nice to support lower refresh rates on the
>>> +      * panels to reduce power consumption, and perhaps match the
>>> +      * user's requested refresh rate.
>>> +      */
>>> +
>>> +     return true;
>>> +}
>>> +
>>> diff --git a/drivers/gpu/drm/gma500/gma_lvds.h b/drivers/gpu/drm/gma500/gma_lvds.h
>>> index eee0da00a0cf..950ab7b88ad6 100644
>>> --- a/drivers/gpu/drm/gma500/gma_lvds.h
>>> +++ b/drivers/gpu/drm/gma500/gma_lvds.h
>>> @@ -30,5 +30,8 @@ enum drm_mode_status gma_lvds_mode_valid(struct drm_connector *connector,
>>>    void gma_lvds_encoder_dpms(struct drm_encoder *encoder, int mode);
>>>    void gma_lvds_save(struct drm_connector *connector);
>>>    void gma_lvds_restore(struct drm_connector *connector);
>>> +bool gma_lvds_mode_fixup(struct drm_encoder *encoder,
>>> +                      const struct drm_display_mode *mode,
>>> +                      struct drm_display_mode *adjusted_mode);
>>>
>>>    #endif
>>> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c
>>> index 00ec4fea4c12..16699b0fbbc9 100644
>>> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
>>> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
>>> @@ -134,7 +134,7 @@ static void oaktrail_lvds_commit(struct drm_encoder *encoder)
>>>
>>>    static const struct drm_encoder_helper_funcs oaktrail_lvds_helper_funcs = {
>>>        .dpms = gma_lvds_encoder_dpms,
>>> -     .mode_fixup = psb_intel_lvds_mode_fixup,
>>> +     .mode_fixup = gma_lvds_mode_fixup,
>>>        .prepare = oaktrail_lvds_prepare,
>>>        .mode_set = oaktrail_lvds_mode_set,
>>>        .commit = oaktrail_lvds_commit,
>>> diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c
>>> index 6e5abb670bff..317bd1fcfa2a 100644
>>> --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c
>>> +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c
>>> @@ -132,69 +132,6 @@ void psb_intel_lvds_set_brightness(struct drm_device *dev, int level)
>>>                psb_lvds_pwm_set_brightness(dev, level);
>>>    }
>>>
>>> -bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
>>> -                               const struct drm_display_mode *mode,
>>> -                               struct drm_display_mode *adjusted_mode)
>>> -{
>>> -     struct drm_device *dev = encoder->dev;
>>> -     struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>> -     struct psb_intel_mode_device *mode_dev = &dev_priv->mode_dev;
>>> -     struct gma_crtc *gma_crtc = to_gma_crtc(encoder->crtc);
>>> -     struct drm_encoder *tmp_encoder;
>>> -     struct drm_display_mode *panel_fixed_mode = mode_dev->panel_fixed_mode;
>>> -     struct gma_encoder *gma_encoder = to_gma_encoder(encoder);
>>> -
>>> -     if (gma_encoder->type == INTEL_OUTPUT_MIPI2)
>>> -             panel_fixed_mode = mode_dev->panel_fixed_mode2;
>>
>> What happened to this test? I cannot see it in the new helper.
> 
> We don't have MIPI support so I removed the test. I forgot to mention
> it in this patch. Other patches should have a note about it when I
> remove MIPI code.
> 
> 
> 
> 
> 
>>
>> Best regards
>> Thomas
>>
>>> -
>>> -     /* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
>>> -     if (!IS_MRST(dev) && gma_crtc->pipe == 0) {
>>> -             pr_err("Can't support LVDS on pipe A\n");
>>> -             return false;
>>> -     }
>>> -     if (IS_MRST(dev) && gma_crtc->pipe != 0) {
>>> -             pr_err("Must use PIPE A\n");
>>> -             return false;
>>> -     }
>>> -     /* Should never happen!! */
>>> -     list_for_each_entry(tmp_encoder, &dev->mode_config.encoder_list,
>>> -                         head) {
>>> -             if (tmp_encoder != encoder
>>> -                 && tmp_encoder->crtc == encoder->crtc) {
>>> -                     pr_err("Can't enable LVDS and another encoder on the same pipe\n");
>>> -                     return false;
>>> -             }
>>> -     }
>>> -
>>> -     /*
>>> -      * If we have timings from the BIOS for the panel, put them in
>>> -      * to the adjusted mode.  The CRTC will be set up for this mode,
>>> -      * with the panel scaling set up to source from the H/VDisplay
>>> -      * of the original mode.
>>> -      */
>>> -     if (panel_fixed_mode != NULL) {
>>> -             adjusted_mode->hdisplay = panel_fixed_mode->hdisplay;
>>> -             adjusted_mode->hsync_start = panel_fixed_mode->hsync_start;
>>> -             adjusted_mode->hsync_end = panel_fixed_mode->hsync_end;
>>> -             adjusted_mode->htotal = panel_fixed_mode->htotal;
>>> -             adjusted_mode->vdisplay = panel_fixed_mode->vdisplay;
>>> -             adjusted_mode->vsync_start = panel_fixed_mode->vsync_start;
>>> -             adjusted_mode->vsync_end = panel_fixed_mode->vsync_end;
>>> -             adjusted_mode->vtotal = panel_fixed_mode->vtotal;
>>> -             adjusted_mode->clock = panel_fixed_mode->clock;
>>> -             drm_mode_set_crtcinfo(adjusted_mode,
>>> -                                   CRTC_INTERLACE_HALVE_V);
>>> -     }
>>> -
>>> -     /*
>>> -      * XXX: It would be nice to support lower refresh rates on the
>>> -      * panels to reduce power consumption, and perhaps match the
>>> -      * user's requested refresh rate.
>>> -      */
>>> -
>>> -     return true;
>>> -}
>>> -
>>>    static void psb_intel_lvds_prepare(struct drm_encoder *encoder)
>>>    {
>>>        struct drm_device *dev = encoder->dev;
>>> @@ -365,7 +302,7 @@ int psb_intel_lvds_set_property(struct drm_connector *connector,
>>>
>>>    static const struct drm_encoder_helper_funcs psb_intel_lvds_helper_funcs = {
>>>        .dpms = gma_lvds_encoder_dpms,
>>> -     .mode_fixup = psb_intel_lvds_mode_fixup,
>>> +     .mode_fixup = gma_lvds_mode_fixup,
>>>        .prepare = psb_intel_lvds_prepare,
>>>        .mode_set = psb_intel_lvds_mode_set,
>>>        .commit = psb_intel_lvds_commit,
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 01/19] drm/gma500: Unify *_lvds_get_max_backlight()

[Sam Ravnborg]

Hi Patrik,

> >
> > With or without this change the patch is:
> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> Hi Sam,
> Thanks for having a look.
> 
> I've intentionally tried to change as little as possible from the
> version I copied so that any functional change is easy to spot and the
> series becomes easy to review. Would it be ok if I do cleanups as a
> followup series?
That would be perfect!

	Sam