diff mbox

Legacy backlight control with KMS and BACKLIGHT property

Message ID 20090818145127.7d7f5ae4@jbarnes-g45 (mailing list archive)
State Not Applicable
Headers show

Commit Message

Jesse Barnes Aug. 18, 2009, 9:51 p.m. UTC
On Mon, 17 Aug 2009 01:41:03 +0100
Mike Lothian <mike@fireburn.co.uk> wrote:
> You are an absolute STAR Greg!!!
> 
> Finally I have brightness control for this laptop on Linux
> 
> I needed to set the DMI names to "R510/P510" and the pci device to
> 0x2a42 but apart from that it works!!!
> 
> Now I just have to get the keys working :D

Any chance I could get you to test this patch out?  It should add an
"i915-backlight" interface to sysfs instead, but hopefully will work
similarly otherwise...

Thanks,

Comments

Mike Lothian Aug. 19, 2009, 12:25 a.m. UTC | #1
2009/8/18 Jesse Barnes <jbarnes@virtuousgeek.org>:
> On Mon, 17 Aug 2009 01:41:03 +0100
> Mike Lothian <mike@fireburn.co.uk> wrote:
>> You are an absolute STAR Greg!!!
>>
>> Finally I have brightness control for this laptop on Linux
>>
>> I needed to set the DMI names to "R510/P510" and the pci device to
>> 0x2a42 but apart from that it works!!!
>>
>> Now I just have to get the keys working :D
>
> Any chance I could get you to test this patch out?  It should add an
> "i915-backlight" interface to sysfs instead, but hopefully will work
> similarly otherwise...
>
> Thanks,
> --
> Jesse Barnes, Intel Open Source Technology Center
>

Sorry I tried this on the latest tip from Linus and it has no effect

It might be because I do have acpi video (with or without the patch)
it just doesn't do anything when I change the values

Let me know if there are any logs you need, parameters I need to pass
or other patches to try

Thanks

Mike
Jesse Barnes Aug. 19, 2009, 1:08 a.m. UTC | #2
On Wed, 19 Aug 2009 01:25:57 +0100
Mike Lothian <mike@fireburn.co.uk> wrote:

> 2009/8/18 Jesse Barnes <jbarnes@virtuousgeek.org>:
> > On Mon, 17 Aug 2009 01:41:03 +0100
> > Mike Lothian <mike@fireburn.co.uk> wrote:
> >> You are an absolute STAR Greg!!!
> >>
> >> Finally I have brightness control for this laptop on Linux
> >>
> >> I needed to set the DMI names to "R510/P510" and the pci device to
> >> 0x2a42 but apart from that it works!!!
> >>
> >> Now I just have to get the keys working :D
> >
> > Any chance I could get you to test this patch out?  It should add an
> > "i915-backlight" interface to sysfs instead, but hopefully will work
> > similarly otherwise...
> >
> > Thanks,
> > --
> > Jesse Barnes, Intel Open Source Technology Center
> >
> 
> Sorry I tried this on the latest tip from Linus and it has no effect
> 
> It might be because I do have acpi video (with or without the patch)
> it just doesn't do anything when I change the values
> 
> Let me know if there are any logs you need, parameters I need to pass
> or other patches to try

Oh, you have a GM45, that should have OpRegion support afaik...  Just
for fun though, can you send me your VBIOS image?

# cd /sys/devices/pci0000\:00/0000\:00\:02.0/
# echo 1 > rom
# cat rom > /tmp/rom.bin
# echo 0 > rom

Thanks,
Matthew Garrett Aug. 19, 2009, 1:30 a.m. UTC | #3
On Wed, Aug 19, 2009 at 01:25:57AM +0100, Mike Lothian wrote:

> It might be because I do have acpi video (with or without the patch)
> it just doesn't do anything when I change the values

This is on a Samsung? Could you install the pmtools (not pm-utils) 
package and run acpidump as root, then send me the output? If you have 
an acpi video device that's not working, then that's a bug that needs 
fixing.
Mike Lothian Aug. 19, 2009, 7:03 a.m. UTC | #4
2009/8/19 Jesse Barnes <jbarnes@virtuousgeek.org>:
> On Wed, 19 Aug 2009 01:25:57 +0100
> Mike Lothian <mike@fireburn.co.uk> wrote:
>
>> 2009/8/18 Jesse Barnes <jbarnes@virtuousgeek.org>:
>> > On Mon, 17 Aug 2009 01:41:03 +0100
>> > Mike Lothian <mike@fireburn.co.uk> wrote:
>> >> You are an absolute STAR Greg!!!
>> >>
>> >> Finally I have brightness control for this laptop on Linux
>> >>
>> >> I needed to set the DMI names to "R510/P510" and the pci device to
>> >> 0x2a42 but apart from that it works!!!
>> >>
>> >> Now I just have to get the keys working :D
>> >
>> > Any chance I could get you to test this patch out?  It should add an
>> > "i915-backlight" interface to sysfs instead, but hopefully will work
>> > similarly otherwise...
>> >
>> > Thanks,
>> > --
>> > Jesse Barnes, Intel Open Source Technology Center
>> >
>>
>> Sorry I tried this on the latest tip from Linus and it has no effect
>>
>> It might be because I do have acpi video (with or without the patch)
>> it just doesn't do anything when I change the values
>>
>> Let me know if there are any logs you need, parameters I need to pass
>> or other patches to try
>
> Oh, you have a GM45, that should have OpRegion support afaik...  Just
> for fun though, can you send me your VBIOS image?
>
> # cd /sys/devices/pci0000\:00/0000\:00\:02.0/
> # echo 1 > rom
> # cat rom > /tmp/rom.bin
> # echo 0 > rom
>
> Thanks,
> --
> Jesse Barnes, Intel Open Source Technology Center
>

That doesn't seem to work

tau 0000:00:02.0 #  echo 1 > rom
bash: rom: No such file or directory
Mike Lothian Aug. 19, 2009, 7:12 a.m. UTC | #5
2009/8/19 Matthew Garrett <mjg59@srcf.ucam.org>:
> On Wed, Aug 19, 2009 at 01:25:57AM +0100, Mike Lothian wrote:
>
>> It might be because I do have acpi video (with or without the patch)
>> it just doesn't do anything when I change the values
>
> This is on a Samsung? Could you install the pmtools (not pm-utils)
> package and run acpidump as root, then send me the output? If you have
> an acpi video device that's not working, then that's a bug that needs
> fixing.
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

I've already got several bugs open about this laptop on the kernel
bugzilla. There is also a problem running any 64bit OS. I have to run
linux with mem=4g to stop it from crashing when unplugging it or
plugging it into the A/C

I've attached the info you wanted

Cheers

Mike
Matthew Garrett Aug. 19, 2009, 11:49 a.m. UTC | #6
On Wed, Aug 19, 2009 at 08:12:36AM +0100, Mike Lothian wrote:

> I've already got several bugs open about this laptop on the kernel
> bugzilla. There is also a problem running any 64bit OS. I have to run
> linux with mem=4g to stop it from crashing when unplugging it or
> plugging it into the A/C

Brightness control appears to be SMI based, rather than opregion, so I'm 
at something of a loss as to why it's not working - your firmware should 
be handling it. On the other hand, your firmware is clearly buggy 
failure based on it crashing on various events - there's a patch that 
binds events to running purely on cpu0, which may help here.
Jesse Barnes Aug. 19, 2009, 4:14 p.m. UTC | #7
On Wed, 19 Aug 2009 08:03:06 +0100
Mike Lothian <mike@fireburn.co.uk> wrote:
> > # cd /sys/devices/pci0000\:00/0000\:00\:02.0/
> > # echo 1 > rom
> > # cat rom > /tmp/rom.bin
> > # echo 0 > rom
> >
> > Thanks,
> > --
> > Jesse Barnes, Intel Open Source Technology Center
> >
> 
> That doesn't seem to work
> 
> tau 0000:00:02.0 #  echo 1 > rom
> bash: rom: No such file or directory

?!  Hm, that could cause some problems...  Can I see your dmesg from a
fresh boot?  Maybe the kernel isn't allocating space for the ROM.
Mike Lothian Aug. 19, 2009, 6:02 p.m. UTC | #8
2009/8/19 Matthew Garrett <mjg59@srcf.ucam.org>:
> On Wed, Aug 19, 2009 at 08:12:36AM +0100, Mike Lothian wrote:
>
>> I've already got several bugs open about this laptop on the kernel
>> bugzilla. There is also a problem running any 64bit OS. I have to run
>> linux with mem=4g to stop it from crashing when unplugging it or
>> plugging it into the A/C
>
> Brightness control appears to be SMI based, rather than opregion, so I'm
> at something of a loss as to why it's not working - your firmware should
> be handling it. On the other hand, your firmware is clearly buggy
> failure based on it crashing on various events - there's a patch that
> binds events to running purely on cpu0, which may help here.
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
>

No I've tried that patch and there was no effect :-(

Damn their buggy BIOS
Mike Lothian Aug. 19, 2009, 6:04 p.m. UTC | #9
2009/8/19 Jesse Barnes <jbarnes@virtuousgeek.org>:
> On Wed, 19 Aug 2009 08:03:06 +0100
> Mike Lothian <mike@fireburn.co.uk> wrote:
>> > # cd /sys/devices/pci0000\:00/0000\:00\:02.0/
>> > # echo 1 > rom
>> > # cat rom > /tmp/rom.bin
>> > # echo 0 > rom
>> >
>> > Thanks,
>> > --
>> > Jesse Barnes, Intel Open Source Technology Center
>> >
>>
>> That doesn't seem to work
>>
>> tau 0000:00:02.0 #  echo 1 > rom
>> bash: rom: No such file or directory
>
> ?!  Hm, that could cause some problems...  Can I see your dmesg from a
> fresh boot?  Maybe the kernel isn't allocating space for the ROM.
>
> --
> Jesse Barnes, Intel Open Source Technology Center
>

Please find it attached

Cheers

Mike
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7537f57..96fe37c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -149,6 +149,11 @@  struct drm_i915_error_state {
 	struct timeval time;
 };
 
+struct blc_funcs {
+	int (*get)(struct drm_device *dev);
+	void (*set)(struct drm_device *dev, int level);
+};
+
 typedef struct drm_i915_private {
 	struct drm_device *dev;
 
@@ -212,6 +217,9 @@  typedef struct drm_i915_private {
 	struct drm_display_mode *panel_fixed_mode;
 	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
 	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
+	struct blc_struct blc_info;
+	struct blc_funcs backlight;
+	struct backlight_device *backlight_device; /* for sysfs */
 
 	/* Feature bits from the VBIOS */
 	unsigned int int_tv_support:1;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2955083..377f10e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -895,6 +895,7 @@ 
 /* Backlight control */
 #define BLC_PWM_CTL		0x61254
 #define   BACKLIGHT_MODULATION_FREQ_SHIFT		(17)
+#define   BACKLIGHT_MODULATION_FREQ_SHIFT2		(16)
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
 #define   BLM_COMBINATION_MODE (1 << 30)
 /*
@@ -904,6 +905,7 @@ 
  * The actual value is this field multiplied by two.
  */
 #define   BACKLIGHT_MODULATION_FREQ_MASK		(0x7fff << 17)
+#define   BACKLIGHT_MODULATION_FREQ_MASK2		(0xffff << 16)
 #define   BLM_LEGACY_MODE				(1 << 16)
 /*
  * This is the number of cycles out of the backlight modulation cycle for which
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 300aee3..afc042b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -315,6 +315,27 @@  parse_driver_features(struct drm_i915_private *dev_priv,
 		dev_priv->edp_support = 1;
 }
 
+static void
+parse_lvds_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
+{
+	struct bdb_lvds_options *lvds_options;
+	struct bdb_lvds_backlight *backlight;
+	int entry;
+
+	lvds_options = find_section(bdb, BDB_LVDS_OPTIONS);
+	backlight = find_section(bdb, BDB_LVDS_BACKLIGHT);
+	if (!lvds_options || !backlight)
+		return;
+
+	/* Make sure we have a compatible layout */
+	if (backlight->blcstruct_size != sizeof(struct blc_struct))
+		return;
+
+	entry = lvds_options->panel_type;
+	memcpy(&dev_priv->blc_info, &backlight->panels[entry],
+	       sizeof(struct blc_struct));
+}
+
 /**
  * intel_init_bios - initialize VBIOS settings & find VBT
  * @dev: DRM device
@@ -366,6 +387,7 @@  intel_init_bios(struct drm_device *dev)
 	parse_sdvo_panel_data(dev_priv, bdb);
 	parse_sdvo_device_mapping(dev_priv, bdb);
 	parse_driver_features(dev_priv, bdb);
+	parse_lvds_backlight(dev_priv, bdb);
 
 	pci_unmap_rom(pdev, bios);
 
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 0f8e5f6..5145151 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -380,6 +380,48 @@  struct bdb_sdvo_lvds_options {
 	u8 panel_misc_bits_4;
 } __attribute__((packed));
 
+#define BLC_INVERTER_TYPE_NONE 0
+#define BLC_INVERTER_TYPE_I2C 1
+#define BLC_INVERTER_TYPE_PWM 2
+
+#define BLC_GPIO_NONE 0
+#define BLC_GPIO_I2C 1
+#define BLC_GPIO_CRT_DDC 2
+#define BLC_GPIO_DVI_DDC 3
+#define BLC_GPIO_SDVO_I2C 5
+
+#define BLC_GMBUS_100KHZ 0
+#define BLC_GMBUS_50KHZ 1
+#define BLC_GMBUS_400KHZ 2
+#define BLC_GMBUS_1MHZ 3
+
+struct blc_struct {
+	u8 inverter_type:2;
+	u8 inverter_polarity:1; /* 1 means inverted (0 = max brightness) */
+	u8 gpio_pins:3;
+	u8 gmbus_speed:2;
+	u16 pwm_freq; /* in Hz */
+	u8 min_brightness; /* (0-255) */
+	u8 i2c_slave_addr;
+	u8 i2c_cmd;
+} __attribute__((packed));
+
+struct bdb_lvds_backlight {
+	u8 blcstruct_size;
+	struct blc_struct panels[16];
+} __attribute__((packed));
+
+struct bdb_lvds_power {
+	u8 dpst_enabled:1;
+	u8 pwr_prefs:3;
+	u8 rsvd1:3;
+	u8 als_enabled:1;
+	u16 als_backlight1;
+	u16 als_backlight2;
+	u16 als_backlight3;
+	u16 als_backlight4;
+	u16 als_backlight5;
+} __attribute__((packed));
 
 #define BDB_DRIVER_FEATURE_NO_LVDS		0
 #define BDB_DRIVER_FEATURE_INT_LVDS		1
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 3f445a8..4c1515b 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -27,6 +27,7 @@ 
  *      Jesse Barnes <jesse.barnes@intel.com>
  */
 
+#include <linux/backlight.h>
 #include <linux/dmi.h>
 #include <linux/i2c.h>
 #include "drmP.h"
@@ -56,11 +57,14 @@  struct intel_lvds_priv {
 };
 
 /**
- * Sets the backlight level.
+ * blc_pwm_set - set PWM backlight reg
+ * @dev: DRM device
+ * @level: brightness level (from 0-max)
  *
- * \param level backlight level, from 0 to intel_lvds_get_max_backlight().
+ * Set the backlight to @level, scaling as necessary based on hw maximum
+ * and inversion.
  */
-static void intel_lvds_set_backlight(struct drm_device *dev, int level)
+static void blc_pwm_set(struct drm_device *dev, int level)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 blc_pwm_ctl, reg;
@@ -70,9 +74,47 @@  static void intel_lvds_set_backlight(struct drm_device *dev, int level)
 	else
 		reg = BLC_PWM_CTL;
 
+	/* Adjust for min and invert if necessary */
+	level += dev_priv->blc_info.min_brightness;
+	if (dev_priv->blc_info.inverter_polarity)
+		level = dev_priv->backlight_device->props.max_brightness - level;
+	/* Adjust for combo mode?
+	 *  - LBB (if not 0xff) * pwm is brightness
+	 *  - so divide incoming value by 255 and feed into pwm
+	 *  - if it overflows, put remainder in lbb
+	 *  - if lbb is already 0, we need to increase it
+	 */
 	blc_pwm_ctl = I915_READ(reg) & ~BACKLIGHT_DUTY_CYCLE_MASK;
-	I915_WRITE(reg, (blc_pwm_ctl |
-				 (level << BACKLIGHT_DUTY_CYCLE_SHIFT)));
+	I915_WRITE(reg, (blc_pwm_ctl | (level << BACKLIGHT_DUTY_CYCLE_SHIFT)));
+}
+
+static int blc_pwm_get(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int level, reg;
+
+	if (IS_IGDNG(dev))
+		reg = BLC_PWM_CPU_CTL;
+	else
+		reg = BLC_PWM_CTL;
+
+	level = I915_READ(reg) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+
+	/* Adjust for min and invert if necessary */
+	if (dev_priv->blc_info.inverter_polarity)
+		level = dev_priv->backlight_device->props.max_brightness - level;
+	level -= dev_priv->blc_info.min_brightness;
+
+	return level;
+}
+
+static void blc_i2c_set(struct drm_device *dev, int level)
+{
+}
+
+static int blc_i2c_get(struct drm_device *dev)
+{
+	return 0;
 }
 
 /**
@@ -114,11 +156,7 @@  static void intel_lvds_set_power(struct drm_device *dev, bool on)
 		do {
 			pp_status = I915_READ(status_reg);
 		} while ((pp_status & PP_ON) == 0);
-
-		intel_lvds_set_backlight(dev, dev_priv->backlight_duty_cycle);
 	} else {
-		intel_lvds_set_backlight(dev, 0);
-
 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) &
 			   ~POWER_TARGET_ON);
 		do {
@@ -652,7 +690,11 @@  static int intel_lvds_get_modes(struct drm_connector *connector)
 static void intel_lvds_destroy(struct drm_connector *connector)
 {
 	struct intel_output *intel_output = to_intel_output(connector);
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	if (dev_priv->backlight_device)
+		backlight_device_unregister(dev_priv->backlight_device);
 	if (intel_output->ddc_bus)
 		intel_i2c_destroy(intel_output->ddc_bus);
 	drm_sysfs_connector_remove(connector);
@@ -856,6 +898,104 @@  static int intel_lid_present(void)
 }
 #endif
 
+static int intel_get_backlight_brightness(struct backlight_device *bd)
+{
+	struct drm_device *dev = bl_get_data(bd);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return dev_priv->backlight.get(dev);
+}
+
+static int intel_update_backlight_status(struct backlight_device *bd)
+{
+	struct drm_device *dev = bl_get_data(bd);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* FIXME: check for FB blanking */
+	dev_priv->backlight.set(dev, bd->props.brightness);
+	return 0;
+}
+
+static struct backlight_ops intel_backlight_ops = {
+	.get_brightness = intel_get_backlight_brightness,
+	.update_status = intel_update_backlight_status,
+};
+
+/**
+ * intel_lvds_backlight_init - find & initialize backlight devices
+ * @dev: DRM device
+ *
+ * If we find a backlight controller (i.e. the backlight controller type
+ * isn't BLC_INVERTER_TYPE_NONE) we set it up here.  This either means
+ * a PWM attached controller or an I2C attached controller.  Control of the
+ * backlight is exposed through sysfs, unless an ACPI or other platform
+ * device has already been registered.
+ */
+static void intel_lvds_backlight_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct backlight_device *backlight_device;
+	u32 pwm_ctl, pwm_ctl2;
+	int max;
+
+	/* FIXME: check whether /sys/class/backlight has a platform device */
+
+	/* ACPI already has one? */
+	if (acpi_video_backlight_support())
+		return;
+
+	switch (dev_priv->blc_info.inverter_type) {
+	case BLC_INVERTER_TYPE_I2C:
+		dev_priv->backlight.get = blc_i2c_get;
+		dev_priv->backlight.set = blc_i2c_set;
+		max = 255;
+		break;
+	case BLC_INVERTER_TYPE_PWM:
+		dev_priv->backlight.get = blc_pwm_get;
+		dev_priv->backlight.set = blc_pwm_set;
+		pwm_ctl = I915_READ(BLC_PWM_CTL);
+		if (IS_I965GM(dev) || IS_GM45(dev)) {
+			max = ((pwm_ctl & BACKLIGHT_MODULATION_FREQ_MASK2) >>
+			       BACKLIGHT_MODULATION_FREQ_SHIFT2);
+			pwm_ctl2 = I915_READ(BLC_PWM_CTL2);
+			if (pwm_ctl2 & BLM_COMBINATION_MODE)
+				max *= 255;
+		} else {
+			max = ((pwm_ctl & BACKLIGHT_MODULATION_FREQ_MASK) >>
+			       BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
+			if (pwm_ctl & BLM_LEGACY_MODE)
+				max *= 255;
+		}
+		break;
+	default:
+		DRM_DEBUG("backlight inverter not available\n");
+		goto out;
+	}
+
+	backlight_device = backlight_device_register("i915-backlight",
+						     &dev->pdev->dev, dev,
+						     &intel_backlight_ops);
+	if (IS_ERR(backlight_device)) {
+		DRM_ERROR("failed to register backlight device\n");
+		goto out;
+	}
+
+	dev_priv->backlight_device = backlight_device;
+
+	/* User exposed value always goes from 0-(max - min) */
+	if (dev_priv->blc_info.inverter_polarity)
+		backlight_device->props.max_brightness =
+			dev_priv->blc_info.min_brightness - max;
+	else
+		backlight_device->props.max_brightness =
+			max - dev_priv->blc_info.min_brightness;
+	backlight_device->props.brightness = dev_priv->backlight.get(dev);
+	backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(backlight_device);
+out:
+	return;
+}
+
 /**
  * intel_lvds_init - setup LVDS connectors on this device
  * @dev: drm device
@@ -1010,6 +1150,8 @@  void intel_lvds_init(struct drm_device *dev)
 		goto failed;
 
 out:
+	intel_lvds_backlight_init(dev);
+
 	if (IS_IGDNG(dev)) {
 		u32 pwm;
 		/* make sure PWM is enabled */