diff mbox

Need help debugging Sony Vaio VPCY2 backlight control problem

Message ID 1309769864.1896.113.camel@rui (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang Rui July 4, 2011, 8:57 a.m. UTC 
Hi, Michael,

please check if the two patches attached help.

thanks,
rui

On Mon, 2011-07-04 at 16:48 +0800, Michel Alexandre Salim wrote:
> On 07/04/2011 10:46 AM, Michel Alexandre Salim wrote:
> > Here are the ectool dumps, before setting any brightness (backlight=0),
> > after setting it to 5, and after setting it to 3, plus the typescript
> > log of the commands issued and the sysfs readings.
> > 
> > I then set brightness back to 0 (ec-after3.txt) -- didn't log the
> > keyboard interactions because it's the same (value for brightness
> > concurs with what is set, but actual_brightness is still -1)
> > 
> > Let me know if you need anything else. Once we can at least pinpoint the
> > problem to the right subsystem, I'll add these attachments to the
> > correct bug report.
> > 
> > Thanks,
> > 
> And here's the last dump file, forgot to attach it earlier.
> 
> Thanks,
>

Comments

Michel Alexandre Salim July 4, 2011, 1:34 p.m. UTC | #1 
On 07/04/2011 01:31 PM, Michel Alexandre Salim wrote:
> Hi Rui,
> 
> They apply cleanly on top of 3.0-rc5, but I'm getting a
> warning-treated-as-failure. Which kernel tree am I supposed to be
> applying this to -- linus' 3.0-rc5, a post-rc5 commit, drm-intel-fixes
> or drm-intel-next?
> 
> Thanks -- Michel
> 
>   CC [M]  drivers/gpu/drm/i915/i915_drv.o
>   CC [M]  drivers/gpu/drm/i915/i915_dma.o
>   CC [M]  drivers/gpu/drm/i915/i915_irq.o
> drivers/gpu/drm/i915/i915_irq.c: In function ‘ivybridge_irq_handler’:
> drivers/gpu/drm/i915/i915_irq.c:505:3: error: implicit declaration of
> function ‘intel_opregion_gse_intr’ [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
> 
> make[4]: *** [drivers/gpu/drm/i915/i915_irq.o] Error 1
> make[3]: *** [drivers/gpu/drm/i915] Error 2
> make[2]: *** [drivers/gpu/drm] Error 2
> make[1]: *** [drivers/gpu] Error 2
> make: *** [drivers] Error 2
> 
And if I comment out those two lines, compile finishes but fails because
of undefined references -- seems like there's an Ivybridge-specific
references from another patch that I don't have.

Kernel: arch/x86/boot/bzImage is ready  (#3)
  Building modules, stage 2.
  MODPOST 2369 modules
ERROR: "igd_opregion_init" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "igd_opregion_intr" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "igd_opregion_enable_asle" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "igd_opregion_setup" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "igd_opregion_fini" [drivers/gpu/drm/i915/i915.ko] undefined!
WARNING: modpost: Found 3 section mismatch(es).
To see full details build your kernel with:
'make CONFIG_DEBUG_SECTION_MISMATCH=y'
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

> On 07/04/2011 10:57 AM, Zhang Rui wrote:
>> Hi, Michael,
>>
>> please check if the two patches attached help.
>>
>> thanks,
>> rui
>>
>> On Mon, 2011-07-04 at 16:48 +0800, Michel Alexandre Salim wrote:
>>> On 07/04/2011 10:46 AM, Michel Alexandre Salim wrote:
>>>> Here are the ectool dumps, before setting any brightness (backlight=0),
>>>> after setting it to 5, and after setting it to 3, plus the typescript
>>>> log of the commands issued and the sysfs readings.
>>>>
>>>> I then set brightness back to 0 (ec-after3.txt) -- didn't log the
>>>> keyboard interactions because it's the same (value for brightness
>>>> concurs with what is set, but actual_brightness is still -1)
>>>>
>>>> Let me know if you need anything else. Once we can at least pinpoint the
>>>> problem to the right subsystem, I'll add these attachments to the
>>>> correct bug report.
>>>>
>>>> Thanks,
>>>>
>>> And here's the last dump file, forgot to attach it earlier.
>>>
>>> Thanks,
>>>
> 
>
Michel Alexandre Salim July 13, 2011, 12:51 p.m. UTC | #2 
The patches don't apply, but looks like the native backlight control
patch is now in Linus' 3.0 tree, and I could cherry-pick Matthew's last
iteration of the i915 patch (from July 7th) from linux-next onto 3.0-rc7.

Going to update my bug report to reflect this. Now the only problem is I
have both acpi_video0 and intel_backlight, and GNOME Power Manager uses
the former rather than the latter. Manually setting the brightness
through the intel_backlight interface works.

Thanks, everyone.
diff mbox

Patch

>From d957b0a6ca265797cce4296dfc3cfda625dfd96f Mon Sep 17 00:00:00 2001
From: Matthew Garrett <mjg@redhat.com>
Date: Wed, 18 May 2011 17:15:48 -0400
Subject: [PATCH 2/2] i915: Add GPU backlight interface

Not all systems provide a firmware or platform interface to backlight
control. We should expose a gpu-level backlight interface to handle that
case.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 +
 drivers/gpu/drm/i915/intel_display.c |    2 -
 drivers/gpu/drm/i915/intel_dp.c      |    6 +++
 drivers/gpu/drm/i915/intel_drv.h     |    3 +-
 drivers/gpu/drm/i915/intel_lvds.c    |    6 +++
 drivers/gpu/drm/i915/intel_panel.c   |   78 ++++++++++++++++++++++++++++++----
 6 files changed, 86 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9b2b94a..68e8869 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -33,6 +33,7 @@ 
 #include "i915_reg.h"
 #include "intel_bios.h"
 #include "intel_ringbuffer.h"
+#include <linux/backlight.h>
 #include <linux/io-mapping.h>
 #include <linux/i2c.h>
 #include <drm/intel-gtt.h>
@@ -667,6 +668,7 @@  typedef struct drm_i915_private {
 	int child_dev_num;
 	struct child_device_config *child_dev;
 	struct drm_connector *int_lvds_connector;
+	struct backlight_device *backlight;
 
 	bool mchbar_need_disable;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 03726a5..7ffb851 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6752,8 +6752,6 @@  static void intel_setup_outputs(struct drm_device *dev)
 			intel_encoder_clones(dev, encoder->clone_mask);
 	}
 
-	intel_panel_setup_backlight(dev);
-
 	/* disable all the possible outputs/crtcs before entering KMS mode */
 	drm_helper_disable_unused_functions(dev);
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a4d8031..c262378 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1747,6 +1747,11 @@  done:
 static void
 intel_dp_destroy (struct drm_connector *connector)
 {
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+
+	if (is_edp(intel_dp))
+		intel_panel_destroy_backlight(connector);
+
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
@@ -1989,6 +1994,7 @@  intel_dp_init(struct drm_device *dev, int output_reg)
 					DRM_MODE_TYPE_PREFERRED;
 			}
 		}
+		intel_panel_setup_backlight(connector);
 	}
 
 	intel_dp_add_properties(intel_dp, connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 831d7a4..c2b5c11 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -265,7 +265,8 @@  extern void intel_pch_panel_fitting(struct drm_device *dev,
 extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
 extern u32 intel_panel_get_backlight(struct drm_device *dev);
 extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
-extern void intel_panel_setup_backlight(struct drm_device *dev);
+extern int intel_panel_setup_backlight(struct drm_connector *dev);
+extern void intel_panel_destroy_backlight(struct drm_connector *dev);
 extern void intel_panel_enable_backlight(struct drm_device *dev);
 extern void intel_panel_disable_backlight(struct drm_device *dev);
 extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 3b90e68..38047b6 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -584,6 +584,9 @@  static void intel_lvds_destroy(struct drm_connector *connector)
 
 	if (dev_priv->lid_notifier.notifier_call)
 		acpi_lid_notifier_unregister(&dev_priv->lid_notifier);
+
+	intel_panel_destroy_backlight(connector);
+
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
@@ -1033,6 +1036,9 @@  out:
 	/* keep the LVDS connector */
 	dev_priv->int_lvds_connector = connector;
 	drm_sysfs_connector_add(connector);
+
+	intel_panel_setup_backlight(connector);
+
 	return true;
 
 failed:
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index f893ff0..1be17b5 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -250,6 +250,76 @@  void intel_panel_set_backlight(struct drm_device *dev, u32 level)
 	I915_WRITE(BLC_PWM_CTL, tmp | level);
 }
 
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
+static int intel_panel_update_brightness(struct backlight_device *bd)
+{
+	struct drm_device *dev = bl_get_data(bd);
+
+	intel_panel_set_backlight(dev, bd->props.brightness);
+	return 0;
+}
+
+static int intel_panel_get_brightness(struct backlight_device *bd)
+{
+	struct drm_device *dev = bl_get_data(bd);
+
+	return intel_panel_get_backlight(dev);
+}
+
+static const struct backlight_ops intel_panel_bl_ops = {
+	.update_status = intel_panel_update_brightness,
+	.get_brightness = intel_panel_get_brightness,
+};
+
+int intel_panel_setup_backlight(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct backlight_properties props;
+
+	dev_priv->backlight_level = intel_panel_get_backlight(dev);
+	dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
+
+	props.max_brightness = intel_panel_get_max_backlight(dev);
+	props.type = BACKLIGHT_RAW;
+	dev_priv->backlight = backlight_device_register("intel_backlight",
+			   &connector->kdev, dev, &intel_panel_bl_ops, &props);
+
+	if (IS_ERR(dev_priv->backlight)) {
+		DRM_ERROR("Failed to register backlight: %ld\n",
+			  PTR_ERR(dev_priv->backlight));
+		dev_priv->backlight = NULL;
+		return -ENODEV;
+	}
+
+	dev_priv->backlight->props.brightness = intel_panel_get_backlight(dev);
+	return 0;
+}
+
+void intel_panel_destroy_backlight(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (dev_priv->backlight)
+		backlight_device_unregister(dev_priv->backlight);
+}
+#else
+int intel_panel_setup_backlight(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->backlight_level = intel_panel_get_backlight(dev);
+	dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
+}
+
+void intel_panel_destroy_backlight(struct drm_connector *connector)
+{
+	return;
+}
+#endif
+
 void intel_panel_disable_backlight(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -273,14 +343,6 @@  void intel_panel_enable_backlight(struct drm_device *dev)
 	dev_priv->backlight_enabled = true;
 }
 
-void intel_panel_setup_backlight(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	dev_priv->backlight_level = intel_panel_get_backlight(dev);
-	dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
-}
-
 enum drm_connector_status
 intel_panel_detect(struct drm_device *dev)
 {
-- 
1.7.5