From patchwork Mon Aug 17 23:57:43 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesse Barnes X-Patchwork-Id: 42215 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n7HNvpBs022118 for ; Mon, 17 Aug 2009 23:57:51 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 531269E8BE; Mon, 17 Aug 2009 16:57:50 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from outbound-mail-306.bluehost.com (outbound-mail-306.bluehost.com [67.222.53.252]) by gabe.freedesktop.org (Postfix) with SMTP id C31AA9E75A for ; Mon, 17 Aug 2009 16:57:47 -0700 (PDT) Received: (qmail 22216 invoked by uid 0); 17 Aug 2009 23:57:47 -0000 Received: from unknown (HELO box514.bluehost.com) (74.220.219.114) by outboundproxy6.bluehost.com with SMTP; 17 Aug 2009 23:57:47 -0000 Received: from [75.111.28.251] (helo=jbarnes-g45) by box514.bluehost.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69) (envelope-from ) id 1MdC5C-0005PC-OA; Mon, 17 Aug 2009 17:57:46 -0600 Date: Mon, 17 Aug 2009 16:57:43 -0700 From: Jesse Barnes To: Matthew Garrett Message-ID: <20090817165743.28ac3e40@jbarnes-g45> In-Reply-To: <20090817203245.GA10264@srcf.ucam.org> References: <20090817140745.GA32497@suse.de> <20090817161702.GE30130@kroah.com> <20090817165237.GA20948@suse.de> <20090817170909.GB20519@kroah.com> <20090817203245.GA10264@srcf.ucam.org> X-Mailer: Claws Mail 3.7.2 (GTK+ 2.17.5; i486-pc-linux-gnu) Mime-Version: 1.0 X-Identified-User: {10642:box514.bluehost.com:virtuous:virtuousgeek.org} {sentby:smtp auth 75.111.28.251 authed with jbarnes@virtuousgeek.org} Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] BACKLIGHT property for KMS case X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.9 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org On Mon, 17 Aug 2009 21:32:45 +0100 Matthew Garrett wrote: > On Mon, Aug 17, 2009 at 10:09:09AM -0700, Greg KH wrote: > > On Mon, Aug 17, 2009 at 06:52:37PM +0200, Matthias Hopf wrote: > > > So you want 2 different drivers controlling the same hardware? > > That's a recipe for disaster :) > > samsung-laptop (in its current form) is controlling the same hardware > that i915 is already. Implementing it properly involves parsing the > BIOS tables to tell you the minimum acceptable value (driving the > backlight controller outside its specced range probably isn't a good > idea) and potentially also touching the MMIO backlight registers. I'd > really recommend not merging this as a separate driver. It's slightly > more work to do it in the drm driver, but it's also the only way to > do it properly. Yeah, I mentioned that in another thread. This patch (untested) illustrates the sort of thing I'd rather see. It integrates the backlight detection and sysfs support into the i915 driver (as part of the existing LVDS code), which should make it more reliable in the face of the various machine types. It still needs to detect whether a backlight device has been registered already, and the i2c code still needs to be written (should be easy, but I'm not sure if I have a test platform for it). diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7537f57..2adad14 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/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..75237c1 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -27,6 +27,7 @@ * Jesse Barnes */ +#include #include #include #include "drmP.h" @@ -60,7 +61,7 @@ struct intel_lvds_priv { * * \param level backlight level, from 0 to intel_lvds_get_max_backlight(). */ -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; @@ -75,6 +76,30 @@ static void intel_lvds_set_backlight(struct drm_device *dev, int level) (level << BACKLIGHT_DUTY_CYCLE_SHIFT))); } +static int blc_pwm_get(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + u32 blc_pwm_ctl, reg; + + if (IS_IGDNG(dev)) + reg = BLC_PWM_CPU_CTL; + else + reg = BLC_PWM_CTL; + + blc_pwm_ctl = I915_READ(reg) & ~BACKLIGHT_DUTY_CYCLE_MASK; + + return blc_pwm_ctl; +} + +static void blc_i2c_set(struct drm_device *dev, int level) +{ +} + +static int blc_i2c_get(struct drm_device *dev) +{ + return 0; +} + /** * Returns the maximum level of the backlight duty cycle field. */ @@ -114,11 +139,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 +673,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 +881,75 @@ static int intel_lid_present(void) } #endif +static int intel_get_backlight_brightness(struct backlight_device *bd) +{ + return bd->props.brightness; +} + +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; + + /* FIXME: check whether /sys/class/backlight is populated or not */ + + 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; + break; + case BLC_INVERTER_TYPE_PWM: + dev_priv->backlight->get = blc_pwm_get; + dev_priv->backlight->set = blc_pwm_set; + break; + default: + DRM_DEBUG("backlight inverter not available\n"); + goto out; + } + + backlight_device = backlight_device_register("i915", &dev->pdev->dev, dev, + &intel_backlight_ops); + if (IS_ERR(backlight_device)) { + DRM_ERROR("failed to register backlight device\n"); + goto out; + } + + backlight_device->props.max_brightness = + dev_priv->blc_info.inverter_polarity ? 0 : 255; + backlight_device->props.brightness = dev_priv->backlight->get(dev); + backlight_device->props.power = FB_BLANK_UNBLANK; + backlight_update_status(backlight_device); + + dev_priv->backlight_device = backlight_device; +out: + return; +} + /** * intel_lvds_init - setup LVDS connectors on this device * @dev: drm device @@ -1009,6 +1103,8 @@ void intel_lvds_init(struct drm_device *dev) if (!dev_priv->panel_fixed_mode) goto failed; + intel_lvds_backlight_init(dev); + out: if (IS_IGDNG(dev)) { u32 pwm;