lkml.org 
[lkml]   [2012]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v2] platform/x86: Add driver for Apple gmux device
    On Mon, Mar 12, 2012 at 03:07:17PM +0000, Matthew Garrett wrote:
    > The alternative is for apple_bl to export its unregister function and
    > have gmux call that - downside is obviously that that results in gmux
    > depending on apple_bl. Maybe some sort of notification list in the
    > backlight core?

    I've been think about this, and I'm not sure a notification in the
    backlight core makes sense; it seems likely to be limited in use to only
    this one use case. Other ideas might be to have an interface to disable
    specific backlight devices, or a way to score backlights which leaves
    only the "best" device of a given type enabled. Either of these might
    affect userspace ABI however.

    For now at least if something like this is needed maybe it makes the
    most sense to have apple_bl export a function. What about something like
    the patch at the end of this message (compile tested only)?

    > > We also have the problem of gmux_backlight versus acpi_video. On most
    > > machines with a gmux the acpi_video backlight interface is present but
    > > just doesn't work. This problem isn't just limited to Apples. I'm of the
    > > opinion that we need a more generalized solution for arbitrating between
    > > the backlight interfaces present on a given machine, but I haven't had a
    > > chance to really think about what that would look like.
    >
    > The ACPI code appears to be trapping into system management mode, so
    > figuring out what it's meant to do is going to be awkward. I think
    > having a hook in the ACPI video driver to deregister it in cases where
    > we know it doesn't work is legitimate, but since it presumably works
    > under Windows it'd be nice to figure out what's broken about it.

    I've been experimenting with a MBP 8,2. This actually has 2 ACPI
    backlights, one associated with the radeon GPU (acpi_video0) and the
    other with the Intel (acpi_video1).

    Turns out that acpi_video0 does work under Windows, and it also works
    under Linux depending on how the OS is booted. Doing a boot camp style
    boot gives us working acpi_video0 and apple_bl backlights. acpi_video1
    doesn't show up in a legacy boot situation, as evaluating _BCM gives an
    error.

    Under EFI boot, both acpi_video backlights show up but neither works.
    The readback verify in apple_bl's probe fails, so that one doesn't show
    up. Only gmux_backlight actually works.

    Using rEFIt is an oddball case. A BIOS-compatible boot gives non-working
    acpi_video and apple_bl backlights. I guess the Apple bootloader must
    enable some extra legacy support that rEFIt does not, but I haven't been
    able to find any way for us to enable it after Linux has started.

    Seth


    diff --git a/drivers/video/backlight/apple_bl.c b/drivers/video/backlight/apple_bl.c
    index be98d15..9b16d58 100644
    --- a/drivers/video/backlight/apple_bl.c
    +++ b/drivers/video/backlight/apple_bl.c
    @@ -27,6 +27,19 @@

    static struct backlight_device *apple_backlight_device;

    +/*
    + * apple_bl may be disabled by other modules. We need to track the state
    + * of the device to know how to respond to various events.
    + */
    +enum {
    + APPLE_BL_STATE_UNUSED,
    + APPLE_BL_STATE_IN_USE,
    + APPLE_BL_STATE_DISABLED,
    +};
    +
    +static int apple_bl_state = APPLE_BL_STATE_UNUSED;
    +DEFINE_MUTEX(apple_bl_mutex);
    +
    struct hw_data {
    /* I/O resource to allocate. */
    unsigned long iostart;
    @@ -139,17 +152,46 @@ static const struct hw_data nvidia_chipset_data = {
    .set_brightness = nvidia_chipset_set_brightness,
    };

    +static void apple_bl_unregister(void)
    +{
    + backlight_device_unregister(apple_backlight_device);
    + release_region(hw_data->iostart, hw_data->iolen);
    + hw_data = NULL;
    +}
    +
    +void apple_bl_disable(void)
    +{
    + mutex_lock(&apple_bl_mutex);
    + if (apple_bl_state == APPLE_BL_STATE_IN_USE)
    + apple_bl_unregister();
    + apple_bl_state = APPLE_BL_STATE_DISABLED;
    + mutex_unlock(&apple_bl_mutex);
    +}
    +EXPORT_SYMBOL_GPL(apple_bl_disable);
    +
    static int __devinit apple_bl_add(struct acpi_device *dev)
    {
    struct backlight_properties props;
    struct pci_dev *host;
    int intensity;
    + int ret = -ENODEV;
    +
    + mutex_lock(&apple_bl_mutex);
    +
    + switch (apple_bl_state) {
    + case APPLE_BL_STATE_IN_USE:
    + ret = -EBUSY;
    + goto out;
    + case APPLE_BL_STATE_DISABLED:
    + printk(KERN_ERR DRIVER "apple_bl disabled, ignoring device\n");
    + goto out;
    + }

    host = pci_get_bus_and_slot(0, 0);

    if (!host) {
    printk(KERN_ERR DRIVER "unable to find PCI host\n");
    - return -ENODEV;
    + goto out;
    }

    if (host->vendor == PCI_VENDOR_ID_INTEL)
    @@ -161,7 +203,7 @@ static int __devinit apple_bl_add(struct acpi_device *dev)

    if (!hw_data) {
    printk(KERN_ERR DRIVER "unknown hardware\n");
    - return -ENODEV;
    + goto out;
    }

    /* Check that the hardware responds - this may not work under EFI */
    @@ -171,14 +213,16 @@ static int __devinit apple_bl_add(struct acpi_device *dev)
    if (!intensity) {
    hw_data->set_brightness(1);
    if (!hw_data->backlight_ops.get_brightness(NULL))
    - return -ENODEV;
    + goto out;

    hw_data->set_brightness(0);
    }

    if (!request_region(hw_data->iostart, hw_data->iolen,
    - "Apple backlight"))
    - return -ENXIO;
    + "Apple backlight")) {
    + ret = -ENXIO;
    + goto out;
    + }

    memset(&props, 0, sizeof(struct backlight_properties));
    props.type = BACKLIGHT_PLATFORM;
    @@ -188,22 +232,30 @@ static int __devinit apple_bl_add(struct acpi_device *dev)

    if (IS_ERR(apple_backlight_device)) {
    release_region(hw_data->iostart, hw_data->iolen);
    - return PTR_ERR(apple_backlight_device);
    + ret = PTR_ERR(apple_backlight_device);
    + goto out;
    }

    apple_backlight_device->props.brightness =
    hw_data->backlight_ops.get_brightness(apple_backlight_device);
    backlight_update_status(apple_backlight_device);

    - return 0;
    + apple_bl_state = APPLE_BL_STATE_IN_USE;
    + ret = 0;
    +
    +out:
    + mutex_unlock(&apple_bl_mutex);
    + return ret;
    }

    static int __devexit apple_bl_remove(struct acpi_device *dev, int type)
    {
    - backlight_device_unregister(apple_backlight_device);
    -
    - release_region(hw_data->iostart, hw_data->iolen);
    - hw_data = NULL;
    + mutex_lock(&apple_bl_mutex);
    + if (apple_bl_state == APPLE_BL_STATE_IN_USE) {
    + apple_bl_unregister();
    + apple_bl_state = APPLE_BL_STATE_UNUSED;
    + }
    + mutex_unlock(&apple_bl_mutex);
    return 0;
    }

    diff --git a/include/linux/apple_bl.h b/include/linux/apple_bl.h
    new file mode 100644
    index 0000000..7a4b6bc
    --- /dev/null
    +++ b/include/linux/apple_bl.h
    @@ -0,0 +1,10 @@
    +/*
    + * apple_bl exported symbols
    + */
    +
    +#ifndef _LINUX_APPLE_BL_H
    +#define _LINUX_APPLE_BL_H
    +
    +extern void apple_bl_disable(void);
    +
    +#endif

    \
     
     \ /
      Last update: 2012-03-15 16:51    [W:0.032 / U:122.364 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site