| Message ID | 1347627426-3813-1-git-send-email-grant.likely@secretlab.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 14, 2012 at 2:12 PM, David Woodhouse <dwmw2@infradead.org> wrote: > On Fri, 2012-09-14 at 14:09 +0100, Grant Likely wrote: >> On Fri, Sep 14, 2012 at 2:01 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> > On Fri, 14 Sep 2012 13:57:06 +0100, Grant Likely <grant.likely@secretlab.ca> wrote: >> >> Some platforms (for instance MacbookPros) have custom backlight drivers >> >> and don't use the integrated i915 backlight control. This patch adds a >> >> quirk to disable registering the intel backlight when unused on a >> >> platform. >> >> >> >> Tested on MacbookPro8,3. Without this patch both the intel_backlight and >> >> gmux_backlight devices get registered and userspace doesn't know which >> >> it should use. >> > >> > Userspace is informed throught the backlight/type property. >> >> Perhaps, but userspace (Ubuntu) isn't doing anything with it, and it >> still remains that it makes no sense whatsoever to register a >> backlight device that doesn't exist. > > Indeed. Userspace (well, gnome-settings-daemon) will use the backlight > provided by X, in preference to anything it finds > in /sys/class/backlight. So if the Intel one is present (and thus > exposed via X) then userspace will never bother with comparing types and > choosing the sanest backlight to use. > > See https://bugzilla.redhat.com/show_bug.cgi?id=752595 In that bug you mention that the intel backlight sets a bogus max of '1' when a backlight isn't present. I saw that too here. Here's the offending code: u32 intel_panel_get_max_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; u32 max; max = i915_read_blc_pwm_ctl(dev_priv); if (max == 0) { /* XXX add code here to query mode clock or hardware clock * and program max PWM appropriately. */ pr_warn_once("fixme: max PWM is zero\n"); return 1; } I used a quirk in my patch, but I could instead change the driver to bail here instead of trying to limp along. g.
On Fri, 14 Sep 2012, Grant Likely <grant.likely@secretlab.ca> wrote: > On Fri, Sep 14, 2012 at 2:12 PM, David Woodhouse <dwmw2@infradead.org> wrote: >> See https://bugzilla.redhat.com/show_bug.cgi?id=752595 > > In that bug you mention that the intel backlight sets a bogus max of > '1' when a backlight isn't present. I saw that too here. Here's the > offending code: > > u32 intel_panel_get_max_backlight(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > u32 max; > > max = i915_read_blc_pwm_ctl(dev_priv); > if (max == 0) { > /* XXX add code here to query mode clock or hardware clock > * and program max PWM appropriately. > */ > pr_warn_once("fixme: max PWM is zero\n"); > return 1; > } > > I used a quirk in my patch, but I could instead change the driver to > bail here instead of trying to limp along. Hi Grant, please try v3.6-rc6 that does exactly that with: commit 28dcc2d60cb570d9f549c329b2f51400553412a1 Author: Jani Nikula <jani.nikula@intel.com> Date: Mon Sep 3 16:25:12 2012 +0300 drm/i915: do not expose a dysfunctional backlight interface to userspace Does that fix it for you? BR, Jani.
On Mon, Sep 17, 2012 at 9:03 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > Hi Grant, please try v3.6-rc6 that does exactly that with: > > commit 28dcc2d60cb570d9f549c329b2f51400553412a1 > Author: Jani Nikula <jani.nikula@intel.com> > Date: Mon Sep 3 16:25:12 2012 +0300 > > drm/i915: do not expose a dysfunctional backlight interface to userspace > > Does that fix it for you? Yes, thanks. g.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 627fe35..48860a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -349,6 +349,7 @@ enum intel_pch { #define QUIRK_PIPEA_FORCE (1<<0) #define QUIRK_LVDS_SSC_DISABLE (1<<1) #define QUIRK_INVERT_BRIGHTNESS (1<<2) +#define QUIRK_NO_BACKLIGHT (2<<2) struct intel_fbdev; struct intel_fbc_work; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2dfa6cf..c8153cd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7088,6 +7088,13 @@ static void quirk_invert_brightness(struct drm_device *dev) DRM_INFO("applying inverted panel brightness quirk\n"); } +static void quirk_no_backlight(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + dev_priv->quirks |= QUIRK_NO_BACKLIGHT; + DRM_INFO("applying no backlight quirk\n"); +} + struct intel_quirk { int device; int subsystem_vendor; @@ -7123,6 +7130,9 @@ static struct intel_quirk intel_quirks[] = { /* Acer Aspire 5734Z must invert backlight brightness */ { 0x2a42, 0x1025, 0x0459, quirk_invert_brightness }, + + /* Apple MacbookPro8,3 doesn't have a backlight */ + { 0x0126, 0x106b, 0x00de, quirk_no_backlight }, }; static void intel_init_quirks(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 3df4f5f..f116e2a 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -413,6 +413,9 @@ int intel_panel_setup_backlight(struct drm_device *dev) struct backlight_properties props; struct drm_connector *connector; + if (dev_priv->quirks & QUIRK_NO_BACKLIGHT) + return 0; + intel_panel_init_backlight(dev); if (dev_priv->int_lvds_connector)
Some platforms (for instance MacbookPros) have custom backlight drivers and don't use the integrated i915 backlight control. This patch adds a quirk to disable registering the intel backlight when unused on a platform. Tested on MacbookPro8,3. Without this patch both the intel_backlight and gmux_backlight devices get registered and userspace doesn't know which it should use. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: David Airlie <airlied@linux.ie> Cc: Matthew Garrett <mjg@redhat.com> Cc: David Woodhouse <dwmw2@infradead.org> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ drivers/gpu/drm/i915/intel_panel.c | 3 +++ 3 files changed, 14 insertions(+)