Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Thu, Oct 27, 2022 at 11:39:38AM +0200, Hans de Goede wrote: > The *only* behavior which actually is new in 6.1 is the native GPU > drivers now doing the equivalent of: > > if (acpi_video_get_backlight_type() != acpi_backlight_native) > return; > > In their backlight register paths (i), which is causing the native > backlight to disappear on your custom laptop setup and on Chromebooks > (with the Chromebooks case being already solved I hope.). It's causing the backlight control to vanish on any machine that isn't ((acpi_video || vendor interface) || !acpi). Most machines that fall into that are either weird or Chromebooks or old, but there are machines that fall into that. (I wrote https://mjg59.livejournal.com/127103.html over 12 years ago, so please do assume that I'm familiar with the complexities here :) ) > I agree this is a possible solution if this turns out to break more > systems and there is no other easy/clean way to fix those. But I would > greatly prefer to keep this change and stop the IMHO bad kernel behavior > of "registering multiple backlight-devices for a single panel and then > let userspace sort it out". If we're not able to make a correct policy decision in the kernel then punting it to userland seems like the right thing to do? The kernel absolutely *should* make the right decision where it has enough information to do so, but in this case the code that's making that decision doesn't have the full set of information.
Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Thu, Oct 27, 2022 at 10:51:45AM +0200, Hans de Goede wrote: > In their backlight register paths and this has been present since > circa 2015. > > So both before and after my 6.1 refactor vendor is only preferred > on devices which don't implement the ACPI video bus control method. Sorry, yes, that's the case I meant. > Just because a vendor interface is present does not mean that it will > work. Unfortunately for none of the 3 main native/acpi_video/vendor > backlight control methods the control method being present also guarantees > that it will work. Which completely sucks, but it is the reality we > have to deal with. But traditionally that's been logic that we've encoded into the vendor drivers, which can take other factors into account when determining whether the exposed interface works. You've now discarded that knowledge. The only way you can maintain the degree of functionality that 6.0 had is to move that determination into core code, or alternatively support dynamic reattachment of backlight interfaces based on vendor drivers loading later. An alternative would be to just revert all of this, and instead only use this logic for the output property interface (which would still result in different outcomes, but only for userland that's choosing to use the new interface, so that's a different problem).
Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Wed, Oct 26, 2022 at 11:59:28AM +0200, Hans de Goede wrote:
> Ok, so this is a local customization to what is already a custom BIOS
> for a custom motherboard. There is a lot of custom in that sentence and
> TBH at some point things might become too custom for them to be expected
> to work OOTB.
But it *did* work OOTB before. You broke it. I accept that I'm a
ludicrously weird corner case here, but there are going to be other
systems that are also affected by this.
> I'm afraid things are not that simple. I assume that with
> "if ACPI backlight control is expected to work" you mean don't
> use ACPI backlight control when (acpi_osi_is_win8() && native_available)
> evaluates to true because it is known to be broken on some of
> those systems because Windows 8 stopped using it ?
Correct.
> Unfortunately something similar applies to vendor interfaces,
> When Windows XP started using (and mandating for certification
> IIRC) ACPI backlight control, vendors still kept their own
> vendor specific EC/smbios/ACPI/WMI backlight interfaces around for
> a long long time, except they were often no longer tested.
The current situation (both before your patchset and with its current
implementation) is that vendor is preferred to native, so if the vendor
interface is present then we're already using it.
> > The
> > problem you're dealing with is that the knowledge of whether or not
> > there's a vendor interface isn't something the core kernel code knows
> > about. What you're proposing here is effectively for us to expose
> > additional information about whether or not there's a vendor interface
> > in the system firmware, but since we're talking in some cases about
> > hardware that's almost 20 years old, we're not realistically going to
> > get those old machines fixed.
>
> I don't understand why you keep talking about the old vendor interfaces,
> at least for the chromebook part of this thread the issue is that
> the i915 driver no longer registers the intel_backlight device which
> is a native device type, which is caused by the patch this email
> thread is about (and old vendor interfaces do not come into play
> at all here). So AFAICT this is a native vs acpi backlight control
> issue ?
I'm referring to your proposed patch that changed the default from
backlight_vendor to backlight_native, which would fix my machine and
Chromebooks but break anything that relies on the vendor interfaces.
> I really want to resolve your bug, but I still lack a lot of info,
> like what backlight interface you were actually using in 6.0 ?
Native.
> {
> .callback = video_detect_force_video,
> /* ThinkPad X201s */
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201s"),
> },
> },
>
> will trigger.
In this case you'd break anyone else running the system who isn't using
the hacked EC and different ACPI tables - obviously there's ways round
this, but realistically since I'm (as far as I know) the only person in
this situation it makes more sense for me to add a kernel parameter than
carry around an exceedingly niche DMI quirk. I'm fine with that. But the
point I'm trying to make is that the machines *are* telling you whether
they'd prefer vendor or native, and you're not taking that into account
in the video_detect code.
Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Wed, Oct 26, 2022 at 01:27:25AM +0200, Hans de Goede wrote:
> this code should actually set the ACPI_VIDEO_BACKLIGHT flag:
> drivers/acpi/scan.c:
>
> static acpi_status
> acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context,
> void **return_value)
> {
> long *cap = context;
>
> if (acpi_has_method(handle, "_BCM") &&
> acpi_has_method(handle, "_BCL")) {
> acpi_handle_debug(handle, "Found generic backlight
> support\n");
> *cap |= ACPI_VIDEO_BACKLIGHT;
> /* We have backlight support, no need to scan further */
> return AE_CTRL_TERMINATE;
> }
> return 0;
> }
Ah, yeah, my local tree no longer matches the upstream behaviour because
I've hacked the EC firmware to remove the backlight trigger because it
had an extremely poor brightness curve and also automatically changed it
on AC events - as a result I removed the backlight code from the DSDT
and just fell back to the native control. Like I said I'm a long way
from the normal setup, but this did previously work.
The "right" logic here seems pretty simple: if ACPI backlight control is
expected to work, use it. If it isn't, but there's a vendor interface,
use it. If there's no vendor interface, use the native interface. The
problem you're dealing with is that the knowledge of whether or not
there's a vendor interface isn't something the core kernel code knows
about. What you're proposing here is effectively for us to expose
additional information about whether or not there's a vendor interface
in the system firmware, but since we're talking in some cases about
hardware that's almost 20 years old, we're not realistically going to
get those old machines fixed. So, it feels like there's two choices:
1) Make a default policy decision, but then allow that decision to be
altered later on (eg, when a vendor-specific platform driver has been
loaded) - you've said this poses additional complexities.
2) Move the knowledge of whether or not there's a vendor interface into
the core code. Basically take every platform driver that exposes a
vendor interface, and move the detection code into the core.
I think any other approach is going to result in machines that
previously worked no longer working (and you can't just make the
vendor/native split dependent on the Coreboot DMI BIOS string, because
there are some Coreboot platforms that implement the vendor interface
for compatibility, and you also can't ask all Coreboot users to update
their firmware to fix things)
Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Tue, Oct 25, 2022 at 10:25:33PM +0200, Hans de Goede wrote: > Having the native driver come and then go and be replaced > with the vendor driver would also be quite inconvenient > for these planned changes. I understand that it would be inconvenient, but you've broken existing working setups. > Can you perhaps explain a bit in what way your laptop > is weird ? It's a Chinese replacement motherboard for a Thinkpad X201, running my own port of Coreboot. Its DMI strings look like an actual Thinkpad in order to ensure that thinkpad_acpi can bind for hotkey suport, so it's hard to quirk. It'll actually be fixed by your proposed patch to fall back to native rather than vendor, but that patch will break any older machines that offer a vendor interface and don't have the native control hooked up (pretty sure at least the Thinkpad X40 falls into that category).
Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Tue, Oct 25, 2022 at 08:50:54PM +0200, Hans de Goede wrote: > That is a valid point, but keep in mind that this is only used on ACPI > platforms and then only on devices with a builtin LCD panel and then > only by GPU drivers which actually call acpi_video_get_backlight_type(), > so e.g. not by all the ARM specific display drivers. > > So I believe that Chromebooks quite likely are the only devices with > this issue. My laptop is, uh, weird, but it falls into this category. > > I think for this to work correctly you need to have > > the infrastructure be aware of whether or not a vendor interface exists, > > which means having to handle cleanup if a vendor-specific module gets > > loaded later. > > Getting rid of the whole ping-ponging of which backlight drivers > get loaded during boot was actually one of the goals of the rework > which landed in 6.1 this actually allowed us to remove some quirks > because some hw/firmware did not like us changing our mind and > switching backlight interfaces after first poking another one. > So we definitely don't want to go back to the ping-pong thing. Defaulting to native but then having a vendor driver be able to disable native drivers seems easiest? It shouldn't be a regression over the previous state of affairs since both drivers were being loaded already.
Re: [Intel-gfx] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
On Tue, Sep 27, 2022 at 01:04:52PM +0200, Hans de Goede wrote: > So to fix this we need to make acpi_video_get_backlight_type() > return native on the Acer Chromebook Spin 713. Isn't the issue broader than that? Unless the platform is Windows 8 or later, we'll *always* (outside of some corner cases) return acpi_backlight_vendor if there's no ACPI video interface. This is broken for any platform that implements ACPI but relies on native video control, which is going to include a range of Coreboot platforms, not just Chromebooks. I think for this to work correctly you need to have the infrastructure be aware of whether or not a vendor interface exists, which means having to handle cleanup if a vendor-specific module gets loaded later.
Re: [Intel-gfx] git pull] drm for v4.1-rc1
On Thu, Apr 23, 2015 at 2:30 PM, Bjorn Helgaas bhelg...@google.com wrote:
Your i915 does not have a ROM BAR in hardware. If the default video
device has no ROM BAR, pci_fixup_video() sets IORESOURCE_ROM_SHADOW
even though the resource flags are zero because the BAR itself doesn't
exist.
If IORESOURCE_ROM_SHADOW is set, pci_map_rom() assumes there's a
shadow ROM image at 0xC. Is there a shadow image even if the
device itself doesn't have a ROM BAR?
On UEFI systems, there's no special reason to believe that there's
anything at 0xc - it depends on whether a CSM got loaded or not.
Everything is awful. So...
int pcibios_add_device(struct pci_dev *dev)
{
if (dev-is-default-vga-device) {
dev-rom = 0xC;
dev-romlen = 0x2;
}
I don't know what we want to do here. This is, at some level,
fundamentally wrong - however, it also wouldn't surprise me if this is
also the only copy of the video ROM we have on some UEFI systems,
especially since I believe that Windows 7 still required that there be
a legacy ROM it could use for bootloader modesetting on UEFI
platforms. So simply making this conditional on BIOS may break
existing machines. But if there *is* a ROM there then we should be
able to id it from the usual video ROM signature?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] git pull] drm for v4.1-rc1
On Thu, Apr 23, 2015 at 3:47 PM, Linus Torvalds torva...@linux-foundation.org wrote: I'm not sure why we want that IORESOURCE_ROM_SHADOW thing at all, but yes, if what this is all about is the magic video ROM at 0xc, then It's used in the PCI layer to say Read from 0xc rather than the ROM BAR and then ends up as a shorthand for Was this the boot video device in various places because we're bad at software. There is no way to see that from the PCI device state, because as mentioned, quite often the ROM is entirely fake, and is not just some shadowed copy of a real underlying hardware ROM, but is fundamentally just a RAM image decompressed from some other source and then marked read-only. If only - nvidias used to rewrite their image at runtime. -- Matthew Garrett | matthew.garr...@coreos.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Enable PSR by default.
I'm seeing the same behaviour with this patchset. After boot, X works fine but I get a rolling display on fbcon (the contents appear to be moving horizontally very quickly around the middle of the screen). If the screen is turned off and on again, X now only updates the screen once every second or so but fbcon works. If I suspend and resume, things go back to the working state until the next screen power cycle. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: PSR: Remove wrong LINK_DISABLE.
I've put this patchset on top of current Linus git. Switching to fbcon tends to result in rolling graphics, and turning the screen back on often gives me a static display or one that only updates every few seconds. This is with a Dell XPS 13 with Broadwell-U and a 3200x1800 display. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 2/2] drm/i915: respect the VBT minimum backlight brightness
On Wed, 2014-06-04 at 08:04 -0700, Stéphane Marchesin wrote: Well, for example ACPI backlight isn't the default on intel, instead i915 implements its own backlight. ACPI backlight doesn't support as many backlight levels as the native backlight for example. So I do think that ACPI backlight is inferior. ACPI backlight is the default on most pre-Windows 8 i915 systems. Userspace which assumes that backlight value 0 has any specific meaning is broken userspace. Adding an additional interface that allows disabling the backlight without powering down the panel seems like a completely reasonable thing to do, though. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/opregion: work around buggy firmware that provides 8+ output devices
On Wed, Feb 19, 2014 at 03:31:29PM +0800, Aaron Lu wrote: DID2 is in system memory region and has some assigned value like 0x400 when we read it. For this case it is easy since there is only one output device that is of type LVDS so we can match it to connector of type eDP or LVDS, suppose there is only one such connector. But for output devices' whose _ADR has the value of 0x301, 0x302, etc. I have no idea how to match them up to the connectors of that type as we can't be sure the probe order we have used in i915 driver is the same as BIOS'. Non-standard _ADR values are assigend by the GPU vendor, so Intel should be able to provide you with the correct interpretations. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On Tue, 2014-01-21 at 13:32 +0800, Aaron Lu wrote: On 01/21/2014 11:17 AM, Matthew Garrett wrote: We know that Windows 8 graphics drivers don't use the ACPI interface, and that systems change their behaviour as a result, in some cases with absolutely no way for the ACPI interface could possibly work. I haven't seen any cases where that's obviously true for any non-Windows 8 Perhaps I'm not clear, I didn't mean non-Windows 8 systems will all favor GPU's interface, I just meant for one specific win7 laptop I could re-use the existing code to make the GPU's interface as the only one left. And to achieve this, the Win8 OSI check in acpi_video_verify_backlight_support has to be gone. We could do that, but why do we think that's the correct fix? The plan is to remove the native list entirely and do this for all Windows 8 systems, so the Win8 OSI check is the right thing to do. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: 1 remove the win8 OSI check, I've seen win7 laptops that also needs to have only the GPU interface left and checking win8 doesn't make much sense now; Are we sure that those aren't simply some other bug? -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] ACPI / video: Add systems that should favor native backlight interface
On Tue, 2014-01-21 at 10:24 +0800, Aaron Lu wrote: On 01/20/2014 09:34 PM, Matthew Garrett wrote: On Mon, 2014-01-20 at 16:12 +0800, Aaron Lu wrote: 1 remove the win8 OSI check, I've seen win7 laptops that also needs to have only the GPU interface left and checking win8 doesn't make much sense now; Are we sure that those aren't simply some other bug? Well, the firmware on that laptop makes use of EC to do backlight control and the fact that the firmware interface doesn't work while the GPU's work seems to indicate that the backlight control circuit is not routed to EC. I think this is the same case as Win8 laptops. We know that Windows 8 graphics drivers don't use the ACPI interface, and that systems change their behaviour as a result, in some cases with absolutely no way for the ACPI interface could possibly work. I haven't seen any cases where that's obviously true for any non-Windows 8 systems. EC interfaces that don't work are often due to Linux leaving the hardware in a state other than the one expected by the firmware. We shouldn't assume that it's the same issue until we've investigated further. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] ACPI / video: Use native backlight control interface by default
We had problems with the Intel backlight driver the last time we tried this, so let's push this through their tree for testing first? Cc:ed intel-gfx and Daniel Vetter. On Thu, Dec 05, 2013 at 10:03:57AM +0800, AceLan Kao wrote: The native/vendor backlight control interface is hardly fail to control the brightness, and we had encountered many backlight issues result from the broken ACPI backlight interface, so I think we should promote the native backlight control interface and use it by default. Signed-off-by: AceLan Kao acelan@canonical.com --- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 995e91b..b3032f8 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -85,7 +85,7 @@ module_param(allow_duplicates, bool, 0644); * For Windows 8 systems: if set ture and the GPU driver has * registered a backlight interface, skip registering ACPI video's. */ -static bool use_native_backlight = false; +static bool use_native_backlight = true; module_param(use_native_backlight, bool, 0644); static int register_count; -- 1.8.3.2 -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 0/4] Fix Win8 backlight issue
On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote: Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default. I'd still really prefer not to add such a list, because it almost inevitably means that we'll never fix this problem properly. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? We have no evidence of Windows behaving differently based on the exposed firmware type. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-09-11 at 13:29 +0300, Jani Nikula wrote: On Wed, 11 Sep 2013, Matthew Garrett matthew.garr...@nebula.com wrote: On Wed, 2013-09-11 at 11:45 +0300, Jani Nikula wrote: Before plunging forward, have you observed any difference between the boot modes? We have reports [1] that the backlight behaviour is different with UEFI vs. UEFI+CSM or legacy boot. So I'm wondering if the acpi_gbl_osi_data = ACPI_OSI_WIN_8 check in patch 2/2 is the whole story. Further, if we tell the BIOS we're Windows 8 to use the tested BIOS code paths, what guarantees do we have of UEFI+CSM or legacy boots working? We have no evidence of Windows behaving differently based on the exposed firmware type. By behaving differently, do you mean internally adapting to the boot mode, or exhibiting different behaviour to the user? As far as backlight control goes, both. We have evidence of the firmware behaving differently (VBT, backlight) based on the boot mode, all else being equal. We don't adapt to that, and we fail. I don't know if we should adapt, or do things differently altogether. I don't even know if Windows 8 works on all boot modes on the machines in question. Sure, but Windows knows nothing about VBT or opregion-backed backlight control. That's up to the Intel driver. -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: I think the parameter Does the ACPI backlight interface work or not belongs to the ACPI video driver. That depends on how Windows 8 works. If Windows 8 policy is handled by the GPU drivers then it belongs in i915. If it's handled by the ACPI code then it belongs in the ACPI code. But I have no way of determining that, whereas you work for a company that produces a Windows 8 video driver… -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-09-10 at 17:21 +0300, Jani Nikula wrote: On Tue, 10 Sep 2013, Matthew Garrett matthew.garr...@nebula.com wrote: On Tue, 2013-09-10 at 16:53 +0300, Jani Nikula wrote: I think the parameter Does the ACPI backlight interface work or not belongs to the ACPI video driver. That depends on how Windows 8 works. If Windows 8 policy is handled by the GPU drivers then it belongs in i915. If it's handled by the ACPI code then it belongs in the ACPI code. I fail to see the logic. Windows 8 policy dictates whether we can use the AML code or not. IMHO, ACPI code is in the best position to figure this out and quirk as necessary. It's the part that knows about Windows 8, not i915. So if nvidia hardware uses the ACPI interface and Intel doesn't, we should still quirk it in the ACPI driver? -- Matthew Garrett matthew.garr...@nebula.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Backlight control only in the kernel?
On Wed, 2013-08-07 at 09:44 +0200, Borislav Petkov wrote: Background is that on my x230, I needed to connect the Fn-Fx backlight hotkey presses to a script to write to /sys/class/backlight/intel_backlight/brightness because Fluxbox doesn't do that (and maybe doesn't have to). So, without presuming any ACPI or backlight knowledge, can we make the backlight control work only in the kernel by connecting the hotkey presses to some backlight controlling interface which backlight-capable devices implement so that it works regardless of userspace environment? Even if the machine is not running X? Not really. The ACPI driver is able to do this because the events and the backlight are associated with the same device. We could potentially make something work with GPU backlight drivers since their PCI device should also be associated with the backlight device, but things get complicated quickly - once ddcci support is hooked up you'll also have kernel backlight devices for some external monitors, and now you need to make a policy decision about which display should be controlled in response to the keypress. One reasonable choice would be whichever display contains the currently focused window, which is obviously out of scope for the kernel. So if we're going to do this then we need a generalised mechanism in-kernel for connecting input devices to backlight devices, and we need a mechanism for disabling it when userspace knows better. This sounds like a lot of work for something that should really just be handled by userspace already. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Wed, 2013-07-31 at 02:01 +0200, Rafael J. Wysocki wrote: (3) Fix i915 backlight control issues for all systems known to have them (that may take a while) and flip the defailt for that option to set when we think we're ready. Unfortunately I don't have any systems that reproduce problems here, so I think Intel are going to have to take the lead on this one. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we no
longer see switch status of backlight.
Yeah, I can duplicate that. Rafael, we have to call
acpi_video_init_brightness() even if we're not going to initialise the
backlight - Thinkpads seem to use this as the trigger for enabling ACPI
notifications rather than handling it in firmware. This seems to do the
job:
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 01b1a25..71865f7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -900,6 +900,9 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
device-cap._DDC = 1;
}
+ if (acpi_video_init_brightness(device))
+ return;
+
if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
@@ -909,9 +912,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
static int count = 0;
char *name;
- result = acpi_video_init_brightness(device);
- if (result)
- return;
name = kasprintf(GFP_KERNEL, acpi_video%d, count);
if (!name)
return;
--
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Sat, Jun 22, 2013 at 11:46:39PM +0200, Yves-Alexis Perez wrote: Before Linux support for acpi_osi(Windows 2012) (and when booting with acpi_osi=!Windows 2012), brightness keys were handled by the kernel just fine, whether in console, in the display manager or in my desktop environment (Xfce). xfce4-power-manager just needs to be told that the brightness keys are already handled and it doesn't need to do anything. Right, the kernel has special-casing to hook the backlight keys up to the ACPI backlight control. This is an awful thing, because there's no way to detect this case other than parsing a single driver-specific module parameter. Could this functionality be duplicated across other backlight drivers? Not easily. The ACPI driver receives keypresses and performs backlight control. The i915 driver doesn't receive keypresses. We could easily tie certain keycodes into backlight events, but which backlight should they control? You're really starting to get into the kind of complex policy decision that's best left to userspace, which is where it should have been to begin with. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Tue, Jun 25, 2013 at 06:10:35PM +0200, Daniel Vetter wrote: Do we have any chances to amend this mistake by (gradually) phasing out support for the direct keypress forwarding in ACPI? Or at least some plans? We could make the default value of brightness_switch_enabled a config variable and encourage distributions to flip their behaviour. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Tue, Jun 25, 2013 at 10:43:57PM +0200, Yves-Alexis Perez wrote: On mar., 2013-06-25 at 17:08 +0100, Matthew Garrett wrote: Right, the kernel has special-casing to hook the backlight keys up to the ACPI backlight control. This is an awful thing, because there's no way to detect this case other than parsing a single driver-specific module parameter. I'm not sure what that means. To detect what case exactly? That the brightness is handled by video.ko? That the kernel will automatically handle backlight key presses. Could this functionality be duplicated across other backlight drivers? Not easily. The ACPI driver receives keypresses and performs backlight control. The i915 driver doesn't receive keypresses. We could easily tie certain keycodes into backlight events, but which backlight should they control? You're really starting to get into the kind of complex policy decision that's best left to userspace, which is where it should have been to begin with. Well, I get the reasoning, but I'm not sure I agree. That means userspace behavior is inconsistent depending on who does it (gnome-power-manager, gnome-setting-daemon, whatever), and it usually means there's nothing handling the brightness before those are running, not to mention people not running them because they don't run a full blown desktop environment (until someone starts thinking it's a good idea to handle brightness in systemd). The behaviour is already inconsistent. If you have an ACPI backlight interface, hitting the keys makes your brightness change without any userspace help. If you don't, it doesn't. And in the end, the user just want the brightness keys to correctly handle the brightness, full stop. Having multiple brightness daemons using different policies on different hardware is a nightmare for the end user, imho. From a user point of view, having it handled all in the kernel works really pretty fine and is completely transparent (I have to admit that from that point of view, it was even better when it was handled by the EC but those times seem long gone). I agree, we should standardise the behaviour. And the only way we can standardise the behaviour is to leave it up to userspace. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Tue, Jun 25, 2013 at 11:10:11PM +0200, Yves-Alexis Perez wrote: On mar., 2013-06-25 at 21:54 +0100, Matthew Garrett wrote: I agree, we should standardise the behaviour. And the only way we can standardise the behaviour is to leave it up to userspace. It's pretty clear we disagree on this and that my opinion won't really matter here. But letting userspace handle that just means broken functionality for those who have the chance (apparently) to have an ACPI backlight interface. Which, as we've already established, you don't - Lenovo broke it. Your Thinkpad claims to have 100 available levels, and most of them don't work. The kernel has no way of knowing which levels work and which don't, so leaving this up to the kernel won't actually fix your system either. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Tue, Jun 25, 2013 at 11:30:37PM +0200, Yves-Alexis Perez wrote: On mar., 2013-06-25 at 22:14 +0100, Matthew Garrett wrote: Which, as we've already established, you don't - Lenovo broke it. Your Thinkpad claims to have 100 available levels, and most of them don't work. The kernel has no way of knowing which levels work and which don't, so leaving this up to the kernel won't actually fix your system either. I was referring to “standardize the behaviour by leaving up to userspace”. A lot of thinkpads (for example) (all the pre-windows 8 ones) have a perfectly working ACPI backlight interface. And this patchset won't alter their behaviour. Also, if the kernel has no way of knowing which levels work, I fail to see how userspace can do better. It can't. That's why this patchset disables the ACPI interface on Windows 8 systems. I understand that switching to intel_backlight instead of acpi_video0 follows what Windows 8 recommends but for me it looks orthogonal to the fact ACPI methods now have some awkward (Lenovo) or broken (Dell). I mean, it's not the first time firmware people break some kernel behavior. I know it's usually not easy to contact them, but shouldn't those methods be fixed, instead of somehow blindly switching to graphic drivers? No. The correct answer to all firmware issues is Are we making the same firmware calls as the version of Windows that this hardware thinks it's running. If Windows 8 doesn't make these calls, we shouldn't make these calls. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Tue, Jun 25, 2013 at 11:46:01PM +0200, Yves-Alexis Perez wrote: But if that introduce regressions, shouldn't workarounds be found then? Sorry if I keep repeating that but brightness keys handling in-kernel is quite a useful feature and losing it (because of the “behave exactly like Windows 8 kernel” policy) is indeed a regression. Your firmware behaves differently depending on whether the OS claims to be Windows 8 or not. We can't make that invisible. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Fri, 2013-06-14 at 14:47 +0800, Aaron Lu wrote: What about a priority based solution? We can introduce a new field named priority to backlight_device and instead of calling another module's function like the unregister one here(which cause unnecessary module dependency), we only need to boost priority for its own interface. This field will be exported to sysfs, so user can change it during runtime too. And we can also introduce a new kernel command line as backlight.force_interface=raw/firmware/platform, to overcome the limited functionality provided by acpi_backlight=video/vendor, which does not involve GPU's interface. How would that work with existing userspace? And we can place the quirk code in backlight layer instead of individual backlight functionality provider module. Suppose we have a backlight manager there, for all win8 systems, we can boost the raw type's priority on its registration, so no need to add code in intel/amd/etc./'s GPU driver code. But we'd need to add code to every piece of userspace that currently uses the backlight, right? With priority based solution, all backlight control interfaces stay, the priority field is an indication given by kernel to user space. We shouldn't export interfaces if we don't expect them to work. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Sat, 2013-06-15 at 09:26 +0800, Aaron Lu wrote: On 06/15/2013 01:29 AM, Matthew Garrett wrote: How would that work with existing userspace? User space tool will need to be updated to use this as stated in the gist page, I've patches for gsd-backlight-helper and xorg-x11-drv-intel, for others we can add I think if the priority based solution is deemed useful. Right, that's not a great solution. We shouldn't export interfaces if we don't expect them to work. It's not easy to decide if they work or not sometimes, e.g. I came across a system that claims win8 in ACPI table and has an Intel GPU, while its ACPI video interface also works. With this patch, the working ACPI video interface is removed, while with the priority based solution, the GPU's interface priority gets higher, but the ACPI video interface still stays. Well, Windows 8 will only use the ACPI backlight interface if the GPU driver decides to, right? So the logic for deciding whether to remove the ACPI backlight control or not should be left up to the GPU. There's no harm in refusing to expose a working method if there's another working method, but there is harm in exposing a broken one and expecting userspace to know the difference. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote: On 06/15/2013 09:38 AM, Matthew Garrett wrote: Well, Windows 8 will only use the ACPI backlight interface if the GPU driver decides to, right? So the logic for deciding whether to remove the ACPI backlight control or not should be left up to the GPU. There's I don't know this. From the document I googled, Microsoft suggests under win8, backlight should be controlled by the graphics driver for smooth brightness level change, instead of ACPI or other methods. So it is possible that OEM will not test the ACPI interface well and thus the interface is likely broken. I don't see why GPU driver has any better knowledge on which systems the firmware interface is broken or not. The vendor will presumably have tested that backlight control works - if the GPU driver uses the ACPI interface and backlight control is broken, then the vendor would fix it. no harm in refusing to expose a working method if there's another working method, but there is harm in exposing a broken one and expecting userspace to know the difference. BTW, the proposed solution is not meant to solve win8 problems alone, it should make solving other problems easy and make individual backlight control interface provider module independent with each other such as the platform drivers will not need to check if ACPI video driver will control backlight or not and can always create backlight interface(its default priority is lower that ACPI video driver's so will not be taken by user space by default, showing the same behavior of the current code). Sure, but it still requires the replacement of existing userspace. We need to fix the problem with existing userspace, too. The current acpi_backlight=video/vendor kernel command line is pretty misleading, for laptops that do not have vendor backlight interface, adding acpi_backlight=vendor actually makes the system using the GPU's interface. Some laptops are using this switch to work around problems in ACPI video driver and users think they are using vendor interface. That's why I think we need a new command line as the backlight.force_interface=raw/firmware/platform. No, I think we need to fix the bugs that currently require users to pass options. Instead of letting individual driver to make decisions on which backlight interface this system should use(either in platform driver as we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), I think we need a better and clear way to handle such things. For example, suppose an acer laptop: vendor does not support backlight, ACPI video's backlight interface is broken and GPU's works, to make it work, user will need to select acer-wmi module while this module does not have anything to do with the functionality, but simply because it serves as the backlight manager for acer laptops. How do these machines work under Windows? Until we know the answer to that, we don't know what the correct way to handle the problem is under Linux. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] ACPICA: Add interface for getting latest OS version requested via _OSI
Drivers may need to make policy decisions based on the OS that the firmware
believes it's interacting with. ACPI firmware will make a series of _OSI
calls, starting from the oldest OS version they support and ending with the
most recent. Add a function to return the last successful call so that
drivers know what the firmware's expecting.
Based on a patch by Seth Forshee seth.fors...@canonical.com
Signed-off-by: Matthew Garrett matthew.garr...@nebula.com
Cc: Seth Forshee seth.fors...@canonical.com
---
drivers/acpi/acpica/aclocal.h | 13 -
drivers/acpi/acpica/utxface.c | 19 +++
include/acpi/acpixf.h | 22 ++
3 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index d5bfbd3..8a2f532 100644
--- a/drivers/acpi/acpica/aclocal.h
+++ b/drivers/acpi/acpica/aclocal.h
@@ -948,19 +948,6 @@ struct acpi_bit_register_info {
/* Structs and definitions for _OSI support and I/O port validation */
-#define ACPI_OSI_WIN_2000 0x01
-#define ACPI_OSI_WIN_XP 0x02
-#define ACPI_OSI_WIN_XP_SP1 0x03
-#define ACPI_OSI_WINSRV_20030x04
-#define ACPI_OSI_WIN_XP_SP2 0x05
-#define ACPI_OSI_WINSRV_2003_SP10x06
-#define ACPI_OSI_WIN_VISTA 0x07
-#define ACPI_OSI_WINSRV_20080x08
-#define ACPI_OSI_WIN_VISTA_SP1 0x09
-#define ACPI_OSI_WIN_VISTA_SP2 0x0A
-#define ACPI_OSI_WIN_7 0x0B
-#define ACPI_OSI_WIN_8 0x0C
-
#define ACPI_ALWAYS_ILLEGAL 0x00
struct acpi_interface_info {
diff --git a/drivers/acpi/acpica/utxface.c b/drivers/acpi/acpica/utxface.c
index 6505774..c1638c3 100644
--- a/drivers/acpi/acpica/utxface.c
+++ b/drivers/acpi/acpica/utxface.c
@@ -389,6 +389,25 @@ ACPI_EXPORT_SYMBOL(acpi_install_interface_handler)
/*
*
+ * FUNCTION:acpi_osi_version
+ *
+ * PARAMETERS: None
+ *
+ * RETURN: Last OS version requested via _OSI
+ *
+ * DESCRIPTION: Returns the argument to the most recent _OSI query performed
+ * by the firmware
+ *
+ /
+u8 acpi_osi_version(void)
+{
+ return acpi_gbl_osi_data;
+}
+
+ACPI_EXPORT_SYMBOL(acpi_osi_version)
+
+/*
+ *
* FUNCTION:acpi_check_address_range
*
* PARAMETERS: space_id- Address space ID
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 454881e..41d3ac1 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -82,6 +82,22 @@ extern u8 acpi_gbl_truncate_io_addresses;
extern u8 acpi_gbl_disable_auto_repair;
/*
+ * Values returned by acpi_osi_version()
+ */
+#define ACPI_OSI_WIN_2000 0x01
+#define ACPI_OSI_WIN_XP 0x02
+#define ACPI_OSI_WIN_XP_SP1 0x03
+#define ACPI_OSI_WINSRV_20030x04
+#define ACPI_OSI_WIN_XP_SP2 0x05
+#define ACPI_OSI_WINSRV_2003_SP10x06
+#define ACPI_OSI_WIN_VISTA 0x07
+#define ACPI_OSI_WINSRV_20080x08
+#define ACPI_OSI_WIN_VISTA_SP1 0x09
+#define ACPI_OSI_WIN_VISTA_SP2 0x0A
+#define ACPI_OSI_WIN_7 0x0B
+#define ACPI_OSI_WIN_8 0x0C
+
+/*
* Hardware-reduced prototypes. All interfaces that use these macros will
* be configured out of the ACPICA build if the ACPI_REDUCED_HARDWARE flag
* is set to TRUE.
@@ -307,6 +323,12 @@ acpi_status acpi_install_notify_handler(acpi_handle
device, u32 handler_type,
acpi_notify_handler handler,
void *context);
+#ifdef CONFIG_ACPI
+u8 acpi_osi_version(void);
+#else
+static inline u8 acpi_osi_version(void) { return 0; }
+#endif
+
acpi_status
acpi_remove_notify_handler(acpi_handle device,
u32 handler_type, acpi_notify_handler handler);
--
1.8.2.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems. Signed-off-by: Matthew Garrett matthew.garr...@nebula.com --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 3b315ba..23b6292 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1661,6 +1661,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Must be done after probing outputs */ intel_opregion_init(dev); acpi_video_register(); + /* Don't use ACPI backlight functions on Windows 8 platforms */ + if (acpi_osi_version() = ACPI_OSI_WIN_8) + acpi_video_backlight_unregister(); } if (IS_GEN5(dev)) -- 1.8.2.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] acpi: video: add function to support unregister backlight interface
From: Lee, Chun-Yi joeyli.ker...@gmail.com
There have some situation we unregister whole acpi/video driver by downstream
driver just want to remove backlight control interface of acpi/video. It caues
we lost other functions of acpi/video, e.g. transfer acpi event to input event.
So, this patch add a new function, find_video_unregister_backlight, it provide
the interface let downstream driver can tell acpi/video to unregister backlight
interface of all acpi video devices. Then we can keep functions of acpi/video
but only remove backlight support.
Reference: bko#35622
https://bugzilla.kernel.org/show_bug.cgi?id=35622
v2: Also unregister cooling devices.
Tested-by: Andrzej Krentosz endr...@gmail.com
Cc: Zhang Rui rui.zh...@intel.com
Cc: Len Brown l...@kernel.org
Cc: Rafael J. Wysocki r...@sisk.pl
Cc: Carlos Corbacho car...@strangeworlds.co.uk
Cc: Matthew Garrett m...@redhat.com
Cc: Dmitry Torokhov d...@mail.ru
Cc: Corentin Chary corenti...@iksaif.net
Cc: Aaron Lu aaron...@intel.com
Cc: Thomas Renninger tr...@suse.de
Signed-off-by: Lee, Chun-Yi j...@suse.com
---
drivers/acpi/video.c | 93
include/acpi/video.h | 2 ++
2 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 5b32e15..da08ff7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -43,6 +43,7 @@
#include acpi/acpi_drivers.h
#include linux/suspend.h
#include acpi/video.h
+#include linux/mutex.h
#define PREFIX ACPI:
@@ -86,6 +87,9 @@ module_param(allow_duplicates, bool, 0644);
static bool use_bios_initial_backlight = 1;
module_param(use_bios_initial_backlight, bool, 0644);
+static bool backlight_disable;
+module_param(backlight_disable, bool, 0644);
+
static int register_count = 0;
static int acpi_video_bus_add(struct acpi_device *device);
static int acpi_video_bus_remove(struct acpi_device *device);
@@ -214,6 +218,8 @@ static const char device_decode[][30] = {
UNKNOWN,
};
+static DEFINE_MUTEX(backlight_mutex);
+
static void acpi_video_device_notify(acpi_handle handle, u32 event, void
*data);
static void acpi_video_device_rebind(struct acpi_video_bus *video);
static void acpi_video_device_bind(struct acpi_video_bus *video,
@@ -882,7 +888,7 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
device-cap._DDC = 1;
}
- if (acpi_video_backlight_support()) {
+ if (acpi_video_backlight_support() || backlight_disable) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
@@ -1083,15 +1089,22 @@ acpi_video_bus_get_one_device(struct acpi_device
*device,
struct acpi_video_device *data;
struct acpi_video_device_attrib* attribute;
+ mutex_lock(backlight_mutex);
+
status =
acpi_evaluate_integer(device-handle, _ADR, NULL, device_id);
/* Some device omits _ADR, we skip them instead of fail */
- if (ACPI_FAILURE(status))
- return 0;
+ if (ACPI_FAILURE(status)) {
+ status = 0;
+ goto out;
+ }
data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
+ if (!data) {
+ status = -ENOMEM;
+ goto out;
+ }
+
strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS);
@@ -1156,6 +1169,8 @@ acpi_video_bus_get_one_device(struct acpi_device *device,
list_add_tail(data-entry, video-video_device_list);
mutex_unlock(video-device_list_lock);
+out:
+ mutex_unlock(backlight_mutex);
return status;
}
@@ -1336,7 +1351,7 @@ acpi_video_switch_brightness(struct acpi_video_device
*device, int event)
int result = -EINVAL;
/* no warning message if acpi_backlight=vendor is used */
- if (!acpi_video_backlight_support())
+ if (!acpi_video_backlight_support() || backlight_disable)
return 0;
if (!device-brightness)
@@ -1869,6 +1884,72 @@ static int __init intel_opregion_present(void)
return opregion;
}
+static acpi_status
+find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
+ void **rv)
+{
+ struct acpi_device *acpi_dev;
+ struct acpi_video_bus *video;
+ struct acpi_video_device *dev, *next;
+ acpi_status status = AE_OK;
+
+ mutex_lock(backlight_mutex);
+
+ if (acpi_bus_get_device(handle, acpi_dev))
+ goto out;
+
+ if (!acpi_match_device_ids(acpi_dev, video_device_ids)) {
+ video = acpi_driver_data(acpi_dev);
+
+ if (!video)
+ goto out;
+
+ acpi_video_bus_stop_devices(video);
+ mutex_lock(video-device_list_lock
[Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
Windows 8 introduced new policy for backlight control by pushing it out to graphics drivers. This appears to have coincided with a range of vendors adding Windows 8 checks to their backlight control code which trigger either awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest thing to do would be to just disable ACPI backlight control entirely if the firmware indicates Windows 8 support, but it's entirely possible that individual graphics drivers might still make use of the ACPI functionality in preference to native control. The first two patches in this series are picked from other patchesets aimed at solving similar problems. The last simply unregisters ACPI backlight control on Windows 8 systems when using an Intel GPU. Similar code could be added to other drivers, but I'm reluctant to do so without further investigation as to the behaviour of the vendor drivers under Windows. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Mon, 2013-06-10 at 13:59 +0200, Rafael J. Wysocki wrote: I happen to know that alternative solution to this problem is being worked on, so I'm going to wait until it is submitted and then we'll decide what to merge. Sure. I'm slightly concerned about unregistering ACPI backlight control for all systems declaring win8 support, even though it may actually work for them. Right, that's why I think it's correct to leave it up to the graphics drivers. It seems like they're the ones that make the policy determination under Windows now. The policy as implemented here may not be correct, but doing better would probably involve Intel letting us know how their Windows driver behaves. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
On Tue, Mar 26, 2013 at 12:48:44PM +0100, Danny Baumann wrote: This patch makes the behaviour of the intel_backlight backlight device consistent to e.g. acpi_videoX: When writing the value 0, the set brightness makes the panel content barely readable instead of turning the backlight off. This matches the expectations of user space (e.g. kde-workspace or the Intel X11 driver), which expects that it can use intel_backlight as a drop-in replacement for acpi_videoX. I'm not quite clear what you mean here. The behaviour of 0 isn't well defined for the ACPI backlight driver - it's perfectly reasonable for it to turn the backlight off entirely. Anything assuming that 0 is still visible is broken. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
On Wed, Mar 27, 2013 at 12:56:37PM +0100, Danny Baumann wrote: OK, I see. And there is user space depending on that behaviour? And again - how is user space supposed to know about the behavioral differences? Is it something like 'if type is raw, don't expect anything'? Do not rely upon 0 being visible. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups
Anyone have any further thoughts on this? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG] i915/intel-acpi.c: failed to get supported _DSM functions (was: [Dual-LVDS Acer Iconia laptop] i915/DRM issue: one screen stays off)
On Mon, Dec 05, 2011 at 09:56:47PM +0100, Baptiste Jonglez wrote: CC-ing intel-gfx@lists.freedesktop.org (see below) On Mon, Dec 05, 2011 at 11:00:41AM +0800, joeyli wrote: [drm:intel_dsm_pci_probe] *ERROR* failed to get supported _DSM functions (attached is the relevant part of dmesg [1]) Have no idea for this _DSM error, need help from drm and acpi experts. It definitely looks like an ACPI issue. That function is defined in `drivers/gpu/drm/i915/intel_acpi.c'. The whole file was added more than a year ago by commit 723bfd707a97 (see the relevant thread on intel-gfx@ [1]) to add _DSM support. One of the first comment is about Calpella, which is exactly the platform of my laptop (as shown by lshw) Ignore that - it's entirely harmless. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable
On Fri, Nov 18, 2011 at 10:41:29PM -0800, Keith Packard wrote: + /* + * Only enable RC6 if all dma remapping is disabled, or if + * there's no iommu present in the machine. + */ + if (INTEL_INFO(dev)-gen == 6) + return no_iommu || dmar_disabled; So the user has to choose between 5W of power saving or having dmar? And we default to giving them dmar? I think that's going to come as a surprise to people. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable
On Tue, Nov 22, 2011 at 06:46:21PM -0200, Eugeni Dodonov wrote: On Tue, Nov 22, 2011 at 18:15, Matthew Garrett m...@redhat.com wrote: On Fri, Nov 18, 2011 at 10:41:29PM -0800, Keith Packard wrote: + /* + * Only enable RC6 if all dma remapping is disabled, or if + * there's no iommu present in the machine. + */ + if (INTEL_INFO(dev)-gen == 6) + return no_iommu || dmar_disabled; So the user has to choose between 5W of power saving or having dmar? And we default to giving them dmar? From the latest investigations, it is either this, or random gpu hangs and crashes when both are enabled :(. So blacklist dmar on sandybridge. The power saving is going to be more important for most users. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] Reduce idle vblank wakeups
The drm core currently waits 5 seconds from userspace dropping a request for vblanks to vblanks actually being disabled. This appears to be a workaround for broken hardware, but results in a mostly idle desktop generating a huge number of wakeups that are entirely unnecessary but which consume measurable amounts of power. This patchset makes the vblank timeout per-device rather than global, making it easy for drivers to override the behaviour without compromising other graphics hardware in the system. It then removes the delay on Intel hardware. I've tested this successfully on Sandybridge without any evidence of spurious or missing irqs, but I don't know how well it works on older hardware. Feedback not only welcome, but positively desired. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 2/3] drm: Handle the vblank_offdelay=0 case properly
Right now if vblank_offdelay is 0, vblanks won't be disabled after the last user. Fix that case up. Signed-off-by: Matthew Garrett m...@redhat.com --- drivers/gpu/drm/drm_irq.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 8bcb6a4..94f9579 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -935,8 +935,7 @@ void drm_vblank_put(struct drm_device *dev, int crtc) BUG_ON(atomic_read(dev-vblank_refcount[crtc]) == 0); /* Last user schedules interrupt disable */ - if (atomic_dec_and_test(dev-vblank_refcount[crtc]) - (dev-vblank_offdelay 0)) + if (atomic_dec_and_test(dev-vblank_refcount[crtc])) mod_timer(dev-vblank_disable_timer, jiffies + ((dev-vblank_offdelay * DRM_HZ)/1000)); } -- 1.7.7.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 3/3] i915: Drop vblank_offdelay
Sandybridge, at least, seems to manage without any vblank offdelay. Dropping this reduces the number of wakeups on an otherwise idle system dramatically. Signed-off-by: Matthew Garrett m...@redhat.com --- drivers/gpu/drm/i915/i915_dma.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a9533c5..46e7172 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1917,6 +1917,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) goto free_priv; } + /* vblank is reliable */ + dev-vblank_offdelay = 0; + /* overlay on gen2 is broken and can't address above 1G */ if (IS_GEN2(dev)) dma_set_coherent_mask(dev-pdev-dev, DMA_BIT_MASK(30)); -- 1.7.7.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups
On Wed, Nov 16, 2011 at 06:03:15PM +0100, Michel Dänzer wrote: I thought the main reason for the delay wasn't broken hardware but to avoid constantly ping-ponging the vblank IRQ between on and off with apps which regularly neeed the vblank counter value, as that could make the counter unreliable. Maybe I'm misremembering though. If turning it on and off results in the counter value being wrong then surely that's a hardware problem? I've tested that turning it on and off between every IRQ still gives valid counter values on sandybridge. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Reduce idle vblank wakeups
On Wed, Nov 16, 2011 at 07:27:51PM +0100, Mario Kleiner wrote: It's not broken hardware, but fast ping-ponging it on and off can make the vblank counter and vblank timestamps unreliable for apps that need high timing precision, especially for the ones that use the OML_sync_control extensions for precise swap scheduling. My target application is vision science neuroscience, where (sub-)milliseconds often matter for visual stimulation. I'll admit that I'm struggling to understand the issue here. If the vblank counter is incremented at the time of vblank (which isn't the case for radeon, it seems, but as far as I can tell is the case for Intel) then how does ping-ponging the IRQ matter? vblank_disable_and_save() appears to handle this case. I think making the vblank off delay driver specific via these patches is a good idea. Lowering the timeout to something like a few refresh cycles, maybe somewhere between 50 msecs and 100 msecs would be also fine by me. I still would like to keep some drm config option to disable or override the vblank off delay by users. Does the timeout serve any purpose other than letting software effectively prevent vblanks from being disabled? The intel and radeon kms drivers implement everything that's needed to make it mostly work. Except for a small race between the cpu and gpu in the vblank_disable_and_save() function http://lxr.free- electrons.com/source/drivers/gpu/drm/drm_irq.c#L101 and drm_update_vblank_count(). It can cause an off-by-one error when reinitializing the drm vblank counter from the gpu's hardware counter if the enable/disable function is called at the wrong moment while the gpu's scanout is inside the vblank interval, see comments in the code. I have some sketchy idea for a patch that could detect when the race happens and retry hw counter queries to fix this. Without that patch, there's some chance between 0% and 4% of being off-by-one. For Radeon, I'd have thought you could handle this by scheduling an irq for the beginning of scanout (avivo has a register for that) and delaying the vblank disable until you hit it? On current nouveau kms, disabling vblank irqs guarantees you wrong vblank counts and wrong vblank timestamps after turning them on again, because the kms driver doesn't implement the hook for hardware vblank counter query correctly. The drm vblank counter misses all counts during the vblank irq off period. Other timing related hooks are missing as well. I have a couple of patches queued up and some more to come for the ddx and kms driver to mostly fix this. NVidia gpu's only have hardware vblank counters for NV-50 and later, fixing this for earlier gpu's would require some emulation of a hw vblank counter inside the kms driver. I've no problem with all of this work being case by case. Apps that rely on the vblank counter being totally reliable over long periods of time currently would be in a bad situation with a lowered vblank off delay, but that's probably generally not a good assumption. Toolkits like mine, which are more paranoid, currently can work fine as long as the off delay is at least a few video refresh cycles. I do the following for scheduling a reliably timed swap: 1. Query current vblank counter current_msc and vblank timestamp current_ust. 2. Calculate a target vblank count target_msc, based on current_msc, current_ust and some target time from usercode. 3. Schedule bufferswap for target_msc. As long as the vblank off delay is long enough so that vblanks don't get turned off between 1. and 3, everything is fine, otherwise bad things will happen. Keeping a way to override the default off delay would be good to allow poor scientists to work around potentially broken drivers or gpu's in the future. @Matthew: I'm appealing here to your ex- Drosophila biologist heritage ;-) If vblanks are disabled and then re-enabled between 1 and 3, what's the negative outcome? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drivers: i915: Default backlight PWM frequency
On Fri, Nov 11, 2011 at 02:17:20PM -0800, Olof Johansson wrote: On Fri, Nov 11, 2011 at 02:12:58PM -0800, Simon Que wrote: If the firmware did not initialize the backlight PWM registers, set up a default PWM frequency of 200 Hz. This is determined using the following formula: freq = refclk / (128 * pwm_max) The PWM register allows the max PWM value to be set. So we want to use the formula, where freq = 200: pwm_max = refclk / (128 * freq) This patch will, in the case of missing PWM register initialization values, look for the reference clock frequency. Based on that, it sets an appropriate max PWM value for a frequency of 200 Hz. If no refclk frequency is found, the max PWM will be zero, which results in no change to the PWM registers. Signed-off-by: Simon Que s...@chromium.org Acked-by: Olof Johansson o...@lixom.net Looks much better. I'm OK with this solution. Matthew? I'd still prefer this to come from the firmware in some way, but in the absence of the awesome let's go with the good. Acked-by: Matthew Garrett m...@redhat.com -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drivers: i915: Default max backlight brightness value
I feel like I'm missing something here. Where's the firmware getting its initial value from? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drivers: i915: Default max backlight brightness value
On Tue, Nov 08, 2011 at 02:05:14PM -0800, Simon Que wrote: On Tue, Nov 8, 2011 at 1:42 PM, Matthew Garrett mj...@srcf.ucam.org wrote: I feel like I'm missing something here. Where's the firmware getting its initial value from? My understanding is that normally, the firmware's VBIOS can program the value of the PWM register. But if the firmware doesn't have the VBIOS initialization code, it can still provide an initial value using this new param, as part of the kernel command line. Either way, it's up to the person writing or selecting the firmware to decide what initial value to provide. So the firmware doesn't set up graphics itself, it's entirely up to the kernel? What hardware is this for? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drivers: i915: Default max backlight brightness value
On Tue, Nov 08, 2011 at 02:27:04PM -0800, Simon Que wrote: This is for an x86-based Chromebook. Its firmware doesn't have the VBIOS support. Previously, we had our own backlight driver that also did a similar initialization. Now, we are trying to move away from that and use the upstream driver instead. However, we still don't have the firmware support, which is why we have this patch to provide the missing information. I'm still not clear on this. There is no video support in the firmware at all? What happens if the kernel fails to boot? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drivers: i915: Default max backlight brightness value
On Tue, Nov 08, 2011 at 02:41:56PM -0800, Simon Que wrote: There is a backup kernel partition that can be used for boot. Failing that, the system can boot from a recovery image over USB. So absolutely no video code in the firmware at all? Ok. What I'd really rather see here is some generic way for platform-specific code to pass the correct value to the driver. You're already exposing various things via the chromeos-acpi code, can you use that to set things up sensibly? It's obviously possible to use the command line to transfer this information, it's just considered pretty poor style. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drivers: i915: Default max backlight brightness value
On Tue, Nov 08, 2011 at 03:02:00PM -0800, Olof Johansson wrote: How about a DMI table check that overrides whatever is setup (or not setup) from the video bios? We know exactly what platforms need this so that table would be easy to specify. dmi's horribly unscalable. It's much better to have a communication channel that doesn't require new code for new models of the same platform. I'm not sure how well this would fit into our platform layer code, it would be pretty nasty to have to export the default backlight variable from the i915 driver and modify it from there as well, and I'm sure noone wants to see any kind of chromeos-specific code paths in the 915 driver (myself included). Well right now this path is (effectively) chromeos-specific. Refactoring the code so we just have the register readback as a single information source and allow the existing platform-specific code to hook in would be conceptually cleaner. But then maybe this is grotesque over-engineering and we should just hack this case. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drivers: i915: Default max backlight brightness value
Again, adding arbitrary constants without any explanation for why you're making this the default really isn't acceptable. We have no way to determine whether fixing one machine is worth making things worse for another. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drivers: i915: Default max backlight brightness value
On Fri, Oct 14, 2011 at 04:46:57PM -0700, Simon Que wrote:
Sorry, I thought I'd replied to this already. Sorry about that.
+static u32 intel_panel_get_default_backlight_period(struct drm_device *dev)
+{
+ /* The default number of clock cycles in one backlight PWM period. */
+ return 0x1000;
I'm uncomfortable with just hardcoding this, especially given that the
comment you're removing implies that it's possible to infer the correct
value from other GPU values. If this does vary between machines then
simply hardcoding it now isn't really any better than the existing error
path - it might make things better for some systems, but it has the
potential to break hardware that expects a different value and no longer
generates an error in that case.
--
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: do not setup intel_backlight twice
On Mon, Aug 22, 2011 at 12:39:10PM -0700, Kamal Mostafa wrote:
The commit Not all systems expose a firmware or platform mechanism for
changing the backlight intensity on i915, so add native driver support
adds calls to intel_panel_setup_backlight() from intel_{lvds,dp}_init
so do not call it again from intel_setup_outputs().
BugLink: http://bugs.launchpad.net/bugs/831542
Signed-off-by: Kamal Mostafa ka...@canonical.com
ACKed-by: Matthew Garrett m...@redhat.com
--
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] missing acpi backlight bisected to 9661e92c10
On Mon, Aug 22, 2011 at 10:25:24AM +0430, Ali Gholami Rudi wrote: Yet, /sys/devices/virtual/backlight/acpi_video0/brightness only appears after the revert. Seems something changes its behavior if new_bd-dev.parent is not NULL in backlight_device_register(). Well, yes, if it has a parent it won't be under /sys/devices/virtual. Does it appear under /sys/class/backlight ? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] missing acpi backlight bisected to 9661e92c10
Well, that all looks fine. And I also can't see any way that this commit could cause the backlight not to appear - all it does is set the parent if it's present. There's no new path that could cause it to return early. Does reverting this patch really make things work? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 native backlight never got merged
On Mon, Aug 08, 2011 at 02:27:46PM -0700, Keith Packard wrote: On Mon, 08 Aug 2011 11:54:22 -0700, Kamal Mostafa ka...@canonical.com wrote: So what happened to that patch? Did it get lost or is it stuck somewhere? I humbly ask that it be re-reviewed and pushed upstream. Afraid it was forgotten -- Matthew, is this patch still useful? Yup. There's a small set of systems that appear to provide no firmware control mechanism. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915: Fix opregion notifications
opregion-based platforms will send ACPI video event 0x80 for a range of
notification types for legacy compatibility. This is interpreted as a
display switch event, which may not be appropriate in the circumstances.
When we receive such an event we should make sure that the platform is
genuinely requesting a display switch before passing that event through
to userspace.
Signed-off-by: Matthew Garrett m...@redhat.com
Tested-by: Adam Jackson a...@redhat.com
---
drivers/acpi/video.c |7 ---
drivers/gpu/drm/i915/intel_opregion.c | 15 +++
include/acpi/video.h |2 ++
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index db39e9e..ada4b4d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -46,7 +46,6 @@
#define PREFIX ACPI:
-#define ACPI_VIDEO_CLASS video
#define ACPI_VIDEO_BUS_NAMEVideo Bus
#define ACPI_VIDEO_DEVICE_NAME Video Device
#define ACPI_VIDEO_NOTIFY_SWITCH 0x80
@@ -1445,7 +1444,8 @@ static void acpi_video_bus_notify(struct acpi_device
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch,
* most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
- keycode = KEY_SWITCHVIDEOMODE;
+ if (!acpi_notifier_call_chain(device, event, 0))
+ keycode = KEY_SWITCHVIDEOMODE;
break;
case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video
@@ -1475,7 +1475,8 @@ static void acpi_video_bus_notify(struct acpi_device
*device, u32 event)
break;
}
- acpi_notifier_call_chain(device, event, 0);
+ if (event != ACPI_VIDEO_NOTIFY_SWITCH)
+ acpi_notifier_call_chain(device, event, 0);
if (keycode) {
input_report_key(input, keycode, 1);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index d2c7104..3443fc1c 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -297,19 +297,26 @@ static int intel_opregion_video_event(struct
notifier_block *nb,
/* The only video events relevant to opregion are 0x80. These indicate
either a docking event, lid switch or display switch request. In
Linux, these are handled by the dock, button and video drivers.
- We might want to fix the video driver to be opregion-aware in
- future, but right now we just indicate to the firmware that the
- request has been handled */
+ */
struct opregion_acpi *acpi;
+ struct acpi_bus_event *event = data;
+ int ret = NOTIFY_OK;
+
+ if (strcmp(event-device_class, ACPI_VIDEO_CLASS))
+ return NOTIFY_DONE;
if (!system_opregion)
return NOTIFY_DONE;
acpi = system_opregion-acpi;
+
+ if (event-type == 0x80 !(acpi-cevt 0x1))
+ ret = NOTIFY_BAD;
+
acpi-csts = 0;
- return NOTIFY_OK;
+ return ret;
}
static struct notifier_block intel_opregion_notifier = {
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 0e98e67..61109f2 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -5,6 +5,8 @@
struct acpi_device;
+#define ACPI_VIDEO_CLASS video
+
#define ACPI_VIDEO_DISPLAY_CRT 1
#define ACPI_VIDEO_DISPLAY_TV 2
#define ACPI_VIDEO_DISPLAY_DVI 3
--
1.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V2] i915: Fix opregion notifications
opregion-based platforms will send ACPI video event 0x80 for a range of
notification types for legacy compatibility. This is interpreted as a
display switch event, which may not be appropriate in the circumstances.
When we receive such an event we should make sure that the platform is
genuinely requesting a display switch before passing that event through
to userspace.
Signed-off-by: Matthew Garrett m...@redhat.com
Tested-by: Adam Jackson a...@redhat.com
---
drivers/acpi/video.c |7 ---
drivers/gpu/drm/i915/intel_opregion.c | 15 +++
include/acpi/video.h |2 ++
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index db39e9e..ada4b4d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -46,7 +46,6 @@
#define PREFIX ACPI:
-#define ACPI_VIDEO_CLASS video
#define ACPI_VIDEO_BUS_NAMEVideo Bus
#define ACPI_VIDEO_DEVICE_NAME Video Device
#define ACPI_VIDEO_NOTIFY_SWITCH 0x80
@@ -1445,7 +1444,8 @@ static void acpi_video_bus_notify(struct acpi_device
*device, u32 event)
case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch,
* most likely via hotkey. */
acpi_bus_generate_proc_event(device, event, 0);
- keycode = KEY_SWITCHVIDEOMODE;
+ if (!acpi_notifier_call_chain(device, event, 0))
+ keycode = KEY_SWITCHVIDEOMODE;
break;
case ACPI_VIDEO_NOTIFY_PROBE: /* User plugged in or removed a video
@@ -1475,7 +1475,8 @@ static void acpi_video_bus_notify(struct acpi_device
*device, u32 event)
break;
}
- acpi_notifier_call_chain(device, event, 0);
+ if (event != ACPI_VIDEO_NOTIFY_SWITCH)
+ acpi_notifier_call_chain(device, event, 0);
if (keycode) {
input_report_key(input, keycode, 1);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index d2c7104..b7c5ddb 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -297,19 +297,26 @@ static int intel_opregion_video_event(struct
notifier_block *nb,
/* The only video events relevant to opregion are 0x80. These indicate
either a docking event, lid switch or display switch request. In
Linux, these are handled by the dock, button and video drivers.
- We might want to fix the video driver to be opregion-aware in
- future, but right now we just indicate to the firmware that the
- request has been handled */
+ */
struct opregion_acpi *acpi;
+ struct acpi_bus_event *event = data;
+ int ret = NOTIFY_OK;
+
+ if (strcmp(event-device_class, ACPI_VIDEO_CLASS) != 0)
+ return NOTIFY_DONE;
if (!system_opregion)
return NOTIFY_DONE;
acpi = system_opregion-acpi;
+
+ if (event-type == 0x80 !(acpi-cevt 0x1))
+ ret = NOTIFY_BAD;
+
acpi-csts = 0;
- return NOTIFY_OK;
+ return ret;
}
static struct notifier_block intel_opregion_notifier = {
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 0e98e67..61109f2 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -5,6 +5,8 @@
struct acpi_device;
+#define ACPI_VIDEO_CLASS video
+
#define ACPI_VIDEO_DISPLAY_CRT 1
#define ACPI_VIDEO_DISPLAY_TV 2
#define ACPI_VIDEO_DISPLAY_DVI 3
--
1.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: Fix opregion notifications
On Tue, Jul 12, 2011 at 03:20:23PM -0700, Keith Packard wrote: Seriously? It's some kind of magic non-switch switch event? And you can tell by checking magic bits within the event? The Intel drivers on Windows appear to have used 0x80 as a generic Something's chanegd notification, overloading it for docking, lid state change and display switch press. As a result of that right now we're sending a display switch event whenever one of these occurs. Less than ideal. Opregion gives us a magic field in shared OS/firmware memory that can be used to distinguish between these events. How can I test this and know if it works? Tested on a Thinkpad X200. Make sure suspend is disabled, then close the lid. Open it again. The ACPI video input device will send a display switch event. Add this patch and repeat and the event will vanish. The display switch itself will still generate an event. I'll send v2 now. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] ACPI/Intel: Rework Opregion support
On Tue, Mar 15, 2011 at 02:30:55PM +0100, Olivier Galibert wrote: On Tue, Mar 15, 2011 at 01:52:26AM +, Matthew Garrett wrote: Now that we've got multiple consumers it's probably not helpful to move the (potentially chip-specific) VBT handling to general code. We've got zero documentation on how GMA500 handles VBT, and not a great deal more for i915. I'm not sure what you mean by handles. If you mean find it, the gma500 driver code reads a 32-bit pointer to the opregion at offset 0xfc in the pci configuration space. If you mean parse it, I haven't seen anything that looked different to what the current kernel code parses. Opregion is one mechanism to provide VBT - it doesn't define it. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] ACPI/Intel: Rework Opregion support
On Tue, Mar 15, 2011 at 02:18:02AM +0100, Indan Zupancic wrote:
+
+ if (dev-set_backlight)
+ dev-set_backlight(dev-drm_dev, bclp * max / 255);
I would hide the max backlight from the opregion code and move this
calculation into set_brightness. Then change the interface to one
based on a scale from 0-255 or 0-100, which fits better with what
opregion actually does. On the other hand, it's the same code in
a different place, so it doesn't make much difference.
That would require an extra layer of indirection in every driver, so
keeping it here seems reasonable.
+void igd_opregion_enable_asle(struct opregion_dev *dev)
+{
+ struct opregion_asle *asle = dev-opregion.asle;
+
+ if (asle dev-enable_asle) {
+ dev-enable_asle(dev-drm_dev);
+
+ asle-tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
+ ASLE_PFMB_EN;
Shouldn't which flags are set depend on which callback functions are set?
Then you can remove all the function != NULL tests too. e.g:
Our experience is that we should give BIOSes the full set of support
flags even if we don't actually do anything with them. That way we avoid
cases where BIOS authors refuse to do anything with partial
implementations (even if we support everything they use) and we get
feedback when we actually find cases of advanced functionality in the
real world.
+ if (IS_MOBILE(dev)) {
+ dev_priv-opregion_dev.max_backlight =
intel_panel_get_max_backlight(dev);
+ dev_priv-opregion_dev.set_backlight =
intel_panel_set_backlight;
+ }
+ dev_priv-opregion_dev.enable_asle = intel_enable_asle;
+ dev_priv-opregion_dev.drm_dev = dev;
So we have a drm_device-drm_i915_private-opregion_dev.drm_device loop,
which is a bit messy, but consistent with other existing code.
Anyone who wants to clean that up is welcome to do so, but really I
optimised for ease of transition here.
- /* XXX Should this validation be moved to intel_opregion.c? */
- if (dev_priv-opregion.vbt) {
- struct vbt_header *vbt = dev_priv-opregion.vbt;
+ /* XXX Should this validation be moved to acpi_igd_opregion.c? */
Should it? Seems like a good moment to do so.
Now that we've got multiple consumers it's probably not helpful to move
the (potentially chip-specific) VBT handling to general code. We've got
zero documentation on how GMA500 handles VBT, and not a great deal more
for i915.
About the naming, as this is Intel-only intel_opregion seems clearer than
igd-opregion, which sounds like it could be anything.
It's Intel-only in implementation, not in specification. As much as I
dislike vendor-neutral specs that have been pushed and implemented by a
single vendor, I'd rather not make the naming Intel-specific if there's
a chance someone else can end up depending on it.
--
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] staging: Use generic IGD opregion code for gma500
The gma500 driver currently includes its own IGD opregion implementation.
Rework it to use the generic one instead. Note that this is missing a
certain level of functionality - the driver does nothing to enable
opregion interrupts right now, and so will simply poll for updates rather
than doing anything sensible.
Signed-off-by: Matthew Garrett m...@redhat.com
---
drivers/staging/gma500/Kconfig |1 +
drivers/staging/gma500/Makefile |1 -
drivers/staging/gma500/psb_drv.c| 28 --
drivers/staging/gma500/psb_drv.h| 21 +---
drivers/staging/gma500/psb_gfx.mod.c|4 +-
drivers/staging/gma500/psb_intel_drv.h |3 +-
drivers/staging/gma500/psb_intel_lvds.c |4 +-
drivers/staging/gma500/psb_intel_opregion.c | 78 ---
8 files changed, 31 insertions(+), 109 deletions(-)
delete mode 100644 drivers/staging/gma500/psb_intel_opregion.c
diff --git a/drivers/staging/gma500/Kconfig b/drivers/staging/gma500/Kconfig
index 5501eb9..f3ab6cb 100644
--- a/drivers/staging/gma500/Kconfig
+++ b/drivers/staging/gma500/Kconfig
@@ -1,6 +1,7 @@
config DRM_PSB
tristate Intel GMA500 KMS Framebuffer
depends on DRM PCI
+ depends on ACPI_IGD_OPREGION
select FB_CFB_COPYAREA
select FB_CFB_FILLRECT
select FB_CFB_IMAGEBLIT
diff --git a/drivers/staging/gma500/Makefile b/drivers/staging/gma500/Makefile
index 21381eb..dba41e3 100644
--- a/drivers/staging/gma500/Makefile
+++ b/drivers/staging/gma500/Makefile
@@ -8,7 +8,6 @@ psb_gfx-y += psb_bl.o \
psb_fb.o \
psb_gtt.o \
psb_intel_bios.o \
- psb_intel_opregion.o \
psb_intel_display.o \
psb_intel_i2c.o \
psb_intel_lvds.o \
diff --git a/drivers/staging/gma500/psb_drv.c b/drivers/staging/gma500/psb_drv.c
index 2fe09c8..f1775e1 100644
--- a/drivers/staging/gma500/psb_drv.c
+++ b/drivers/staging/gma500/psb_drv.c
@@ -29,6 +29,7 @@
#include psb_intel_bios.h
#include drm/drm_pciids.h
#include psb_powermgmt.h
+#include acpi/video.h
#include linux/cpu.h
#include linux/notifier.h
#include linux/spinlock.h
@@ -294,6 +295,18 @@ static struct drm_ioctl_desc psb_ioctls[] = {
DRM_AUTH),
};
+static void psb_lid_init(struct drm_device *dev)
+{
+ struct drm_psb_private *dev_priv = dev-dev_private;
+ struct igd_opregion *opregion = dev_priv-opregion_dev.opregion;
+
+ if (!opregion-acpi)
+ return;
+
+ dev_priv-lid_state = opregion-acpi-clid;
+ dev_priv-lid_last_state = *dev_priv-lid_state;
+}
+
static void psb_set_uopt(struct drm_psb_uopt *uopt)
{
return;
@@ -515,6 +528,8 @@ static int psb_driver_unload(struct drm_device *dev)
/* Kill vblank etc here */
+ acpi_video_unregister();
+
psb_backlight_exit(); /*writes minimum value to backlight HW reg */
if (drm_psb_no_fb == 0)
@@ -662,7 +677,10 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long chipset)
goto out_err;
psb_get_core_freq(dev);
- psb_intel_opregion_init(dev);
+ dev_priv-opregion_dev.drm_dev = dev;
+ dev_priv-opregion_dev.max_backlight =
psb_intel_lvds_get_max_backlight(dev);
+ dev_priv-opregion_dev.set_backlight = psb_intel_lvds_set_brightness;
+ igd_opregion_setup(dev_priv-opregion_dev);
psb_intel_init_bios(dev);
PSB_DEBUG_INIT(Init TTM fence and BO driver\n);
@@ -765,11 +783,9 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long chipset)
if (ret)
return ret;
- /**
-* Init lid switch timer.
-* NOTE: must do this after psb_intel_opregion_init
-* and psb_backlight_init
-*/
+ igd_opregion_init(dev_priv-opregion_dev);
+ acpi_video_register();
+ psb_lid_init(dev);
if (dev_priv-lid_state)
psb_lid_timer_init(dev_priv);
diff --git a/drivers/staging/gma500/psb_drv.h b/drivers/staging/gma500/psb_drv.h
index b75b9d8..30d161e 100644
--- a/drivers/staging/gma500/psb_drv.h
+++ b/drivers/staging/gma500/psb_drv.h
@@ -23,6 +23,7 @@
#include linux/version.h
#include drm/drmP.h
+#include acpi/acpi_igd_opregion.h
#include drm_global.h
#include psb_drm.h
#include psb_reg.h
@@ -253,19 +254,6 @@ enum {
#define MDFLD_PLANE_MAX_WIDTH 2048
#define MDFLD_PLANE_MAX_HEIGHT 2048
-struct opregion_header;
-struct opregion_acpi;
-struct opregion_swsci;
-struct opregion_asle;
-
-struct psb_intel_opregion {
- struct opregion_header *header;
- struct opregion_acpi *acpi;
- struct opregion_swsci *swsci;
- struct opregion_asle *asle;
- int enabled;
-};
-
/*
*User options.
*/
@@ -692,7 +680,7 @@ struct drm_psb_private {
*/
spinlock_t lid_lock;
struct timer_list lid_timer;
- struct psb_intel_opregion opregion;
+ struct
Re: [Intel-gfx] [PATCH 5/5] ACPI: Tie ACPI backlight devices to PCI devices if possible
On Sun, Feb 06, 2011 at 09:35:07PM +0100, Rafael J. Wysocki wrote:
+ acpi_get_parent(device-dev-handle, acpi_parent);
+
+ pdev = acpi_get_pci_dev(acpi_parent);
+ if (pdev) {
+ parent = pdev-dev;
+ pci_dev_put(pdev);
+ }
I'm afraid you can't do that or suspend problems will happen.
Ugh. Ok, how can we fix this?
--
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] ACPI: Tie ACPI backlight devices to PCI devices if possible
On Mon, Feb 07, 2011 at 12:01:25AM +0100, Rafael J. Wysocki wrote: Yes, it seems so, but I'm not sure what the short term consequences of that change will be. Perhaps there will be none. :-) Ok, I'll have a play with that. Maybe we should be fixing this up somehow in the acpi-pci glue code? It seems wrong to have acpi devices resumed before the PCI device they're associated with. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] i915: Add native backlight control
Well, that's odd. I'll look into it this week. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] i915: Add native backlight control
On Thu, Jan 20, 2011 at 03:13:49PM -0800, Andrew Morton wrote: On Fri, 21 Jan 2011 00:43:46 +0330 Ali Gholami Rudi aliqr...@gmail.com wrote: Ali Gholami Rudi aliqr...@gmail.com wrote: I tried to apply this patch but I get: drivers/gpu/drm/i915/intel_panel.c: In function 'intel_panel_setup_backlight': drivers/gpu/drm/i915/intel_panel.c:319: error: 'struct backlight_properties' has no member named 'type' drivers/gpu/drm/i915/intel_panel.c:319: error: 'BACKLIGHT_RAW' undeclared (first use in this function) drivers/gpu/drm/i915/intel_panel.c:319: error: (Each undeclared identifier is reported only once drivers/gpu/drm/i915/intel_panel.c:319: error: for each function it appears in.) After applying patch 1/5, the patch applied cleanly. Now I can change the brightness using /sys/class/backlight/intel_backlight/brightness. So it does fix my issue. So we have a patch ordering issue and the radeon-expose-backlight-class-device-for-legacy-lvds-encoder.patch build error. He applied 2/5, it didn't build, he applied 1/5 and it built? I don't think that's a patch ordering issue :) I think Michel's patch should fix the Radeon one. If not, can you drop that and keep the rest of the set? I'm travelling at the moment and won't have proper build access until the weekend. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] i915: Add native backlight control
On Thu, Jan 20, 2011 at 05:03:37PM -0800, Andrew Morton wrote: I updated the new drivers/video/backlight/adp5520_bl.c, but there's a decent chance that unconverted drivers will sneak in. I trust they will still work OK? They should fire a warning on registration but otherwise work. I'll send fixup patches for any I see. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] ACPI: Tie ACPI backlight devices to PCI devices if possible
Dual-GPU machines may provide more than one ACPI backlight interface. Tie
the backlight device to the GPU in order to allow userspace to identify
the correct interface.
Signed-off-by: Matthew Garrett m...@redhat.com
---
drivers/acpi/video.c | 15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index a9eec8c..a18e497 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -782,6 +782,9 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_video_backlight_support()) {
struct backlight_properties props;
+ struct pci_dev *pdev;
+ acpi_handle acpi_parent;
+ struct device *parent = NULL;
int result;
static int count = 0;
char *name;
@@ -794,10 +797,20 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
return;
count++;
+ acpi_get_parent(device-dev-handle, acpi_parent);
+
+ pdev = acpi_get_pci_dev(acpi_parent);
+ if (pdev) {
+ parent = pdev-dev;
+ pci_dev_put(pdev);
+ }
+
memset(props, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_FIRMWARE;
props.max_brightness = device-brightness-count - 3;
- device-backlight = backlight_device_register(name, NULL,
device,
+ device-backlight = backlight_device_register(name,
+ parent,
+ device,
acpi_backlight_ops,
props);
kfree(name);
--
1.7.3.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] radeon: Expose backlight class device for legacy LVDS encoder
From: Michel Dänzer mic...@daenzer.net
Allows e.g. power management daemons to control the backlight level. Inspired
by the corresponding code in radeonfb.
(Updated to add backlight type and make the connector the parent device - mjg)
Signed-off-by: Michel Dänzer daen...@vmware.com
Signed-off-by: Matthew Garrett m...@redhat.com
Cc: dri-de...@lists.freedesktop.org
---
drivers/gpu/drm/radeon/Kconfig |1 +
drivers/gpu/drm/radeon/radeon_connectors.c | 15 ++
drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 257 ++-
drivers/gpu/drm/radeon/radeon_mode.h| 10 +
4 files changed, 277 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 1c02d23..9746fee 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -1,6 +1,7 @@
config DRM_RADEON_KMS
bool Enable modesetting on radeon by default - NEW DRIVER
depends on DRM_RADEON
+ select BACKLIGHT_CLASS_DEVICE
help
Choose this option if you want kernel modesetting enabled by default.
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 22b7e3d..e842fb5 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -40,6 +40,10 @@ radeon_atombios_connected_scratch_regs(struct drm_connector
*connector,
struct drm_encoder *encoder,
bool connected);
+extern void
+radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
+struct drm_connector *drm_connector);
+
void radeon_connector_hotplug(struct drm_connector *connector)
{
struct drm_device *dev = connector-dev;
@@ -1517,6 +1521,17 @@ radeon_add_legacy_connector(struct drm_device *dev,
connector-polled = DRM_CONNECTOR_POLL_HPD;
connector-display_info.subpixel_order = subpixel_order;
drm_sysfs_connector_add(connector);
+ if (connector_type == DRM_MODE_CONNECTOR_LVDS) {
+ struct drm_encoder *drm_encoder;
+
+ list_for_each_entry(drm_encoder,
dev-mode_config.encoder_list, head) {
+ struct radeon_encoder *radeon_encoder;
+
+ radeon_encoder = to_radeon_encoder(drm_encoder);
+ if (radeon_encoder-encoder_id ==
ENCODER_OBJECT_ID_INTERNAL_LVDS)
+ radeon_legacy_backlight_init(radeon_encoder,
connector);
+ }
+ }
return;
failed:
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 59f834b..ba7dcc6 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -28,6 +28,10 @@
#include radeon_drm.h
#include radeon.h
#include atom.h
+#include linux/backlight.h
+#ifdef CONFIG_PMAC_BACKLIGHT
+#include asm/backlight.h
+#endif
static void radeon_legacy_encoder_disable(struct drm_encoder *encoder)
{
@@ -39,7 +43,7 @@ static void radeon_legacy_encoder_disable(struct drm_encoder
*encoder)
radeon_encoder-active_device = 0;
}
-static void radeon_legacy_lvds_dpms(struct drm_encoder *encoder, int mode)
+static void radeon_legacy_lvds_update(struct drm_encoder *encoder, int mode)
{
struct drm_device *dev = encoder-dev;
struct radeon_device *rdev = dev-dev_private;
@@ -47,15 +51,23 @@ static void radeon_legacy_lvds_dpms(struct drm_encoder
*encoder, int mode)
uint32_t lvds_gen_cntl, lvds_pll_cntl, pixclks_cntl, disp_pwr_man;
int panel_pwr_delay = 2000;
bool is_mac = false;
+ uint8_t backlight_level;
DRM_DEBUG_KMS(\n);
+ lvds_gen_cntl = RREG32(RADEON_LVDS_GEN_CNTL);
+ backlight_level = (lvds_gen_cntl RADEON_LVDS_BL_MOD_LEVEL_SHIFT)
0xff;
+
if (radeon_encoder-enc_priv) {
if (rdev-is_atom_bios) {
struct radeon_encoder_atom_dig *lvds =
radeon_encoder-enc_priv;
panel_pwr_delay = lvds-panel_pwr_delay;
+ if (lvds-bl_dev)
+ backlight_level = lvds-backlight_level;
} else {
struct radeon_encoder_lvds *lvds =
radeon_encoder-enc_priv;
panel_pwr_delay = lvds-panel_pwr_delay;
+ if (lvds-bl_dev)
+ backlight_level = lvds-backlight_level;
}
}
@@ -82,11 +94,13 @@ static void radeon_legacy_lvds_dpms(struct drm_encoder
*encoder, int mode)
lvds_pll_cntl = ~RADEON_LVDS_PLL_RESET;
WREG32(RADEON_LVDS_PLL_CNTL, lvds_pll_cntl);
- lvds_gen_cntl = RREG32(RADEON_LVDS_GEN_CNTL);
- lvds_gen_cntl |= (RADEON_LVDS_ON | RADEON_LVDS_EN
Re: [Intel-gfx] [PATCH 4/5] nouveau: Change the backlight parent device to the connector, not the PCI dev
On Fri, Jan 14, 2011 at 09:30:19PM +0200, Anca Emanuel wrote: Hi Matthew Garrett, I have problems with nouveau. Do you know ? Your best bet is to follow the instructions on http://nouveau.freedesktop.org/wiki/Bugs to report a bug. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] ACPI: Tie ACPI backlight devices to PCI devices if possible
Dual-GPU machines may provide more than one ACPI backlight interface. Tie
the backlight device to the GPU in order to allow userspace to identify
the correct interface.
Signed-off-by: Matthew Garrett m...@redhat.com
---
drivers/acpi/video.c | 15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 38165a8..0403647 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -843,6 +843,9 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
if (acpi_video_backlight_support()) {
struct backlight_properties props;
+ struct pci_dev *pdev;
+ acpi_handle acpi_parent;
+ struct device *parent = NULL;
int result;
static int count = 0;
char *name;
@@ -855,10 +858,20 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
return;
count++;
+ acpi_get_parent(device-dev-handle, acpi_parent);
+
+ pdev = acpi_get_pci_dev(acpi_parent);
+ if (pdev) {
+ parent = pdev-dev;
+ pci_dev_put(pdev);
+ }
+
memset(props, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_FIRMWARE;
props.max_brightness = device-brightness-count - 3;
- device-backlight = backlight_device_register(name, NULL,
device,
+ device-backlight = backlight_device_register(name,
+ parent,
+ device,
acpi_backlight_ops,
props);
kfree(name);
--
1.7.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] radeon: Expose backlight class device for legacy LVDS encoder
From: Michel Dänzer mic...@daenzer.net
Allows e.g. power management daemons to control the backlight level. Inspired
by the corresponding code in radeonfb.
(Updated to add backlight type and make the connector the parent device - mjg)
Signed-off-by: Michel Dänzer daen...@vmware.com
Signed-off-by: Matthew Garrett m...@redhat.com
Cc: dri-de...@lists.freedesktop.org
---
drivers/gpu/drm/radeon/Kconfig |1 +
drivers/gpu/drm/radeon/radeon_connectors.c | 15 ++
drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 257 ++-
drivers/gpu/drm/radeon/radeon_mode.h| 10 +
4 files changed, 277 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 1c02d23..9746fee 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -1,6 +1,7 @@
config DRM_RADEON_KMS
bool Enable modesetting on radeon by default - NEW DRIVER
depends on DRM_RADEON
+ select BACKLIGHT_CLASS_DEVICE
help
Choose this option if you want kernel modesetting enabled by default.
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index fe6c747..d20bc76 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -40,6 +40,10 @@ radeon_atombios_connected_scratch_regs(struct drm_connector
*connector,
struct drm_encoder *encoder,
bool connected);
+extern void
+radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
+struct drm_connector *drm_connector);
+
void radeon_connector_hotplug(struct drm_connector *connector)
{
struct drm_device *dev = connector-dev;
@@ -1462,6 +1466,17 @@ radeon_add_legacy_connector(struct drm_device *dev,
connector-polled = DRM_CONNECTOR_POLL_HPD;
connector-display_info.subpixel_order = subpixel_order;
drm_sysfs_connector_add(connector);
+ if (connector_type == DRM_MODE_CONNECTOR_LVDS) {
+ struct drm_encoder *drm_encoder;
+
+ list_for_each_entry(drm_encoder,
dev-mode_config.encoder_list, head) {
+ struct radeon_encoder *radeon_encoder;
+
+ radeon_encoder = to_radeon_encoder(drm_encoder);
+ if (radeon_encoder-encoder_id ==
ENCODER_OBJECT_ID_INTERNAL_LVDS)
+ radeon_legacy_backlight_init(radeon_encoder,
connector);
+ }
+ }
return;
failed:
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 0b83970..bdca317 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -28,6 +28,10 @@
#include radeon_drm.h
#include radeon.h
#include atom.h
+#include linux/backlight.h
+#ifdef CONFIG_PMAC_BACKLIGHT
+#include asm/backlight.h
+#endif
static void radeon_legacy_encoder_disable(struct drm_encoder *encoder)
{
@@ -39,7 +43,7 @@ static void radeon_legacy_encoder_disable(struct drm_encoder
*encoder)
radeon_encoder-active_device = 0;
}
-static void radeon_legacy_lvds_dpms(struct drm_encoder *encoder, int mode)
+static void radeon_legacy_lvds_update(struct drm_encoder *encoder, int mode)
{
struct drm_device *dev = encoder-dev;
struct radeon_device *rdev = dev-dev_private;
@@ -47,15 +51,23 @@ static void radeon_legacy_lvds_dpms(struct drm_encoder
*encoder, int mode)
uint32_t lvds_gen_cntl, lvds_pll_cntl, pixclks_cntl, disp_pwr_man;
int panel_pwr_delay = 2000;
bool is_mac = false;
+ uint8_t backlight_level;
DRM_DEBUG_KMS(\n);
+ lvds_gen_cntl = RREG32(RADEON_LVDS_GEN_CNTL);
+ backlight_level = (lvds_gen_cntl RADEON_LVDS_BL_MOD_LEVEL_SHIFT)
0xff;
+
if (radeon_encoder-enc_priv) {
if (rdev-is_atom_bios) {
struct radeon_encoder_atom_dig *lvds =
radeon_encoder-enc_priv;
panel_pwr_delay = lvds-panel_pwr_delay;
+ if (lvds-bl_dev)
+ backlight_level = lvds-backlight_level;
} else {
struct radeon_encoder_lvds *lvds =
radeon_encoder-enc_priv;
panel_pwr_delay = lvds-panel_pwr_delay;
+ if (lvds-bl_dev)
+ backlight_level = lvds-backlight_level;
}
}
@@ -82,11 +94,13 @@ static void radeon_legacy_lvds_dpms(struct drm_encoder
*encoder, int mode)
lvds_pll_cntl = ~RADEON_LVDS_PLL_RESET;
WREG32(RADEON_LVDS_PLL_CNTL, lvds_pll_cntl);
- lvds_gen_cntl = RREG32(RADEON_LVDS_GEN_CNTL);
- lvds_gen_cntl |= (RADEON_LVDS_ON | RADEON_LVDS_EN
[Intel-gfx] [PATCH 4/5] nouveau: Change the backlight parent device to the connector, not the PCI dev
We may eventually end up with per-connector backlights, especially with
ddcci devices. Make sure that the parent node for the backlight device is
the connector rather than the PCI device.
Signed-off-by: Matthew Garrett m...@redhat.com
---
drivers/gpu/drm/nouveau/nouveau_backlight.c | 21 -
drivers/gpu/drm/nouveau/nouveau_connector.c |9 +
drivers/gpu/drm/nouveau/nouveau_drv.h |4 ++--
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 9485af3..7e8a448 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -87,10 +87,11 @@ static struct backlight_ops nv50_bl_ops = {
.update_status = nv50_set_intensity,
};
-static int nouveau_nv40_backlight_init(struct drm_device *dev)
+static int nouveau_nv40_backlight_init(struct drm_connector *connector)
{
- struct backlight_properties props;
+ struct drm_device *dev = connector-dev;
struct drm_nouveau_private *dev_priv = dev-dev_private;
+ struct backlight_properties props;
struct backlight_device *bd;
if (!(nv_rd32(dev, NV40_PMC_BACKLIGHT) NV40_PMC_BACKLIGHT_MASK))
@@ -99,7 +100,7 @@ static int nouveau_nv40_backlight_init(struct drm_device
*dev)
memset(props, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 31;
- bd = backlight_device_register(nv_backlight, dev-pdev-dev, dev,
+ bd = backlight_device_register(nv_backlight, connector-kdev, dev,
nv40_bl_ops, props);
if (IS_ERR(bd))
return PTR_ERR(bd);
@@ -111,10 +112,11 @@ static int nouveau_nv40_backlight_init(struct drm_device
*dev)
return 0;
}
-static int nouveau_nv50_backlight_init(struct drm_device *dev)
+static int nouveau_nv50_backlight_init(struct drm_connector *connector)
{
- struct backlight_properties props;
+ struct drm_device *dev = connector-dev;
struct drm_nouveau_private *dev_priv = dev-dev_private;
+ struct backlight_properties props;
struct backlight_device *bd;
if (!nv_rd32(dev, NV50_PDISPLAY_SOR_BACKLIGHT))
@@ -123,7 +125,7 @@ static int nouveau_nv50_backlight_init(struct drm_device
*dev)
memset(props, 0, sizeof(struct backlight_properties));
props.type = BACKLIGHT_RAW;
props.max_brightness = 1025;
- bd = backlight_device_register(nv_backlight, dev-pdev-dev, dev,
+ bd = backlight_device_register(nv_backlight, connector-kdev, dev,
nv50_bl_ops, props);
if (IS_ERR(bd))
return PTR_ERR(bd);
@@ -134,15 +136,16 @@ static int nouveau_nv50_backlight_init(struct drm_device
*dev)
return 0;
}
-int nouveau_backlight_init(struct drm_device *dev)
+int nouveau_backlight_init(struct drm_connector *connector)
{
+ struct drm_device *dev = connector-dev;
struct drm_nouveau_private *dev_priv = dev-dev_private;
switch (dev_priv-card_type) {
case NV_40:
- return nouveau_nv40_backlight_init(dev);
+ return nouveau_nv40_backlight_init(connector);
case NV_50:
- return nouveau_nv50_backlight_init(dev);
+ return nouveau_nv50_backlight_init(connector);
default:
break;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 0871495..914058d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -106,6 +106,10 @@ nouveau_connector_destroy(struct drm_connector
*drm_connector)
dev = nv_connector-base.dev;
NV_DEBUG_KMS(dev, \n);
+ if (drm_connector-connector_type == DRM_MODE_CONNECTOR_LVDS ||
+ drm_connector-connector_type == DRM_MODE_CONNECTOR_eDP)
+ nouveau_backlight_exit(dev);
+
kfree(nv_connector-edid);
drm_sysfs_connector_remove(drm_connector);
drm_connector_cleanup(drm_connector);
@@ -894,6 +898,11 @@ nouveau_connector_create(struct drm_device *dev, int index)
nouveau_connector_set_polling(connector);
drm_sysfs_connector_add(connector);
+
+ if (connector-connector_type == DRM_MODE_CONNECTOR_LVDS ||
+ connector-connector_type == DRM_MODE_CONNECTOR_eDP)
+ nouveau_backlight_init(connector);
+
dcb-drm = connector;
return dcb-drm;
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 3a07e58..3c65b77 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -933,10 +933,10 @@ static inline int nouveau_acpi_edid(struct drm_device
*dev, struct drm_connector
/* nouveau_backlight.c */
#ifdef
Re: [Intel-gfx] [RFC PATCH 0/2] i915 brightness control
On Wed, Jun 02, 2010 at 03:11:40PM -0700, Kamal Mostafa wrote: In order to avoid the often-dysfunctional native acpi brightness control methods, a new acpi_brightness_hook interface is made available. The i915 driver uses the new hook to take over brightness control. Boot with i915.brightness=0 to disable. I've looked into this issue more closely and think I've worked out the underlying problem. The system in question appears to have two GPUs and exposes two ACPI backlight devices. Both of these are associated with existing PCI devices, so we don't ignore either of them because of that. Further, one of them (the AMD one) implements the spec properly and should work. We don't seem to perform a more fine-grained check to identify whether every ACPI backlight has all the required methods, and so as a result we provide both the working one and the non-working one. Having thought about this some more, I don't think this is the right approach. We should be ensuring that every backlight ahs all the required methods and then dropping the one that doesn't. This should be replaced with a native i915 backlight, and I sent patches to do that last week. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: Add native backlight control
On Thu, Sep 09, 2010 at 06:09:40PM +0100, Chris Wilson wrote: On Wed, 8 Sep 2010 12:32:18 -0400, Matthew Garrett m...@redhat.com wrote: Not all systems expose a firmware or platform mechanism for changing the backlight intensity on i915, so add native driver support. This will conflict with a similar patch I have in drm-intel-next to unify the various bits of code that were attempting to modify the BLC PWM, each in their own unique fashion. I'll respin this native interface on top of that. Do you want to add a native RAW interface all in one go, or to base it on the current backlight_device and then to add the RAW property with this patch set? I don't think providing a native interface in i915 is beneficial without providing the type information - userspace just doesn't have any way to identify the correct device otherwise. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915: Add native backlight control
Not all systems expose a firmware or platform mechanism for changing the
backlight intensity on i915, so add native driver support.
Signed-off-by: Matthew Garrett m...@redhat.com
Cc: intel-gfx intel-gfx@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_drv.h |3 +
drivers/gpu/drm/i915/i915_opregion.c | 60 +---
drivers/gpu/drm/i915/intel_drv.h |3 +
drivers/gpu/drm/i915/intel_lvds.c| 177 ++
4 files changed, 170 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index af4a263..36c4b407 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -34,6 +34,7 @@
#include intel_bios.h
#include intel_ringbuffer.h
#include linux/io-mapping.h
+#include linux/backlight.h
/* General customization:
*/
@@ -663,6 +664,8 @@ typedef struct drm_i915_private {
/* list of fbdev register on this device */
struct intel_fbdev *fbdev;
+
+ struct backlight_device *backlight;
} drm_i915_private_t;
/** driver private structure attached to each drm_gem_object */
diff --git a/drivers/gpu/drm/i915/i915_opregion.c
b/drivers/gpu/drm/i915/i915_opregion.c
index ea5d3fe..de199dd 100644
--- a/drivers/gpu/drm/i915/i915_opregion.c
+++ b/drivers/gpu/drm/i915/i915_opregion.c
@@ -31,9 +31,9 @@
#include drmP.h
#include i915_drm.h
#include i915_drv.h
+#include intel_drv.h
#define PCI_ASLE 0xe4
-#define PCI_LBPC 0xf4
#define PCI_ASLS 0xfc
#define OPREGION_SZ(8*1024)
@@ -147,8 +147,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct opregion_asle *asle = dev_priv-opregion.asle;
- u32 blc_pwm_ctl, blc_pwm_ctl2;
- u32 max_backlight, level, shift;
+ u32 max = intel_lvds_get_max_backlight(dev);
if (!(bclp ASLE_BCLP_VALID))
return ASLE_BACKLIGHT_FAILED;
@@ -157,27 +156,8 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
if (bclp 0 || bclp 255)
return ASLE_BACKLIGHT_FAILED;
- blc_pwm_ctl = I915_READ(BLC_PWM_CTL);
- blc_pwm_ctl2 = I915_READ(BLC_PWM_CTL2);
-
- if (IS_I965G(dev) (blc_pwm_ctl2 BLM_COMBINATION_MODE))
- pci_write_config_dword(dev-pdev, PCI_LBPC, bclp);
- else {
- if (IS_PINEVIEW(dev)) {
- blc_pwm_ctl = ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
- max_backlight = (blc_pwm_ctl
BACKLIGHT_MODULATION_FREQ_MASK)
- BACKLIGHT_MODULATION_FREQ_SHIFT;
- shift = BACKLIGHT_DUTY_CYCLE_SHIFT + 1;
- } else {
- blc_pwm_ctl = ~BACKLIGHT_DUTY_CYCLE_MASK;
- max_backlight = ((blc_pwm_ctl
BACKLIGHT_MODULATION_FREQ_MASK)
- BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
- shift = BACKLIGHT_DUTY_CYCLE_SHIFT;
- }
- level = (bclp * max_backlight) / 255;
- I915_WRITE(BLC_PWM_CTL, blc_pwm_ctl | (level shift));
- }
- asle-cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
+ asle-cblv = (bclp * 100 / 255) | ASLE_CBLV_VALID;
+ intel_lvds_set_backlight(dev, max * bclp / 255);
return 0;
}
@@ -243,36 +223,6 @@ void opregion_asle_intr(struct drm_device *dev)
asle-aslc = asle_stat;
}
-static u32 asle_set_backlight_ironlake(struct drm_device *dev, u32 bclp)
-{
- struct drm_i915_private *dev_priv = dev-dev_private;
- struct opregion_asle *asle = dev_priv-opregion.asle;
- u32 cpu_pwm_ctl, pch_pwm_ctl2;
- u32 max_backlight, level;
-
- if (!(bclp ASLE_BCLP_VALID))
- return ASLE_BACKLIGHT_FAILED;
-
- bclp = ASLE_BCLP_MSK;
- if (bclp 0 || bclp 255)
- return ASLE_BACKLIGHT_FAILED;
-
- cpu_pwm_ctl = I915_READ(BLC_PWM_CPU_CTL);
- pch_pwm_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
- /* get the max PWM frequency */
- max_backlight = (pch_pwm_ctl2 16) BACKLIGHT_DUTY_CYCLE_MASK;
- /* calculate the expected PMW frequency */
- level = (bclp * max_backlight) / 255;
- /* reserve the high 16 bits */
- cpu_pwm_ctl = ~(BACKLIGHT_DUTY_CYCLE_MASK);
- /* write the updated PWM frequency */
- I915_WRITE(BLC_PWM_CPU_CTL, cpu_pwm_ctl | level);
-
- asle-cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
-
- return 0;
-}
-
void ironlake_opregion_gse_intr(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -296,7 +246,7 @@ void ironlake_opregion_gse_intr(struct drm_device *dev)
}
if (asle_req ASLE_SET_BACKLIGHT)
- asle_stat |= asle_set_backlight_ironlake(dev, asle-bclp);
+ asle_stat |= asle_set_backlight(dev, asle-bclp);
if (asle_req ASLE_SET_PFIT
[Intel-gfx] [PATCH] Backlight: Add backlight type
There may be multiple ways of controlling the backlight on a given machine. Allow drivers to expose the type of interface they are providing, making it possible for userspace to make appropriate policy decisions. Signed-off-by: Matthew Garrett m...@redhat.com Cc: Richard Purdie rpur...@rpsys.net Cc: intel-gfx@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org --- Documentation/ABI/stable/sysfs-class-backlight | 20 drivers/acpi/video.c|1 + drivers/gpu/drm/nouveau/nouveau_backlight.c |2 ++ drivers/hid/hid-picolcd.c |1 + drivers/macintosh/via-pmu-backlight.c |1 + drivers/platform/x86/acer-wmi.c |1 + drivers/platform/x86/asus-laptop.c |1 + drivers/platform/x86/asus_acpi.c|1 + drivers/platform/x86/classmate-laptop.c |1 + drivers/platform/x86/compal-laptop.c|1 + drivers/platform/x86/dell-laptop.c |1 + drivers/platform/x86/eeepc-laptop.c |1 + drivers/platform/x86/fujitsu-laptop.c |1 + drivers/platform/x86/msi-laptop.c |1 + drivers/platform/x86/msi-wmi.c |1 + drivers/platform/x86/panasonic-laptop.c |1 + drivers/platform/x86/sony-laptop.c |1 + drivers/platform/x86/thinkpad_acpi.c|1 + drivers/platform/x86/toshiba_acpi.c |1 + drivers/staging/samsung-laptop/samsung-laptop.c |1 + drivers/usb/misc/appledisplay.c |1 + drivers/video/atmel_lcdfb.c |1 + drivers/video/aty/aty128fb.c|1 + drivers/video/aty/atyfb_base.c |1 + drivers/video/aty/radeon_backlight.c|1 + drivers/video/backlight/88pm860x_bl.c |1 + drivers/video/backlight/adp5520_bl.c|1 + drivers/video/backlight/adp8860_bl.c|1 + drivers/video/backlight/adx_bl.c|1 + drivers/video/backlight/atmel-pwm-bl.c |1 + drivers/video/backlight/backlight.c | 15 +++ drivers/video/backlight/corgi_lcd.c |1 + drivers/video/backlight/cr_bllcd.c |1 + drivers/video/backlight/da903x_bl.c |1 + drivers/video/backlight/ep93xx_bl.c |1 + drivers/video/backlight/generic_bl.c|1 + drivers/video/backlight/hp680_bl.c |1 + drivers/video/backlight/jornada720_bl.c |1 + drivers/video/backlight/kb3886_bl.c |1 + drivers/video/backlight/locomolcd.c |1 + drivers/video/backlight/max8925_bl.c|1 + drivers/video/backlight/mbp_nvidia_bl.c |1 + drivers/video/backlight/omap1_bl.c |1 + drivers/video/backlight/pcf50633-backlight.c|1 + drivers/video/backlight/progear_bl.c|1 + drivers/video/backlight/pwm_bl.c|1 + drivers/video/backlight/tosa_bl.c |1 + drivers/video/backlight/wm831x_bl.c |1 + drivers/video/bf54x-lq043fb.c |1 + drivers/video/bfin-t350mcqb-fb.c|1 + drivers/video/nvidia/nv_backlight.c |1 + drivers/video/omap2/displays/panel-acx565akm.c |1 + drivers/video/omap2/displays/panel-taal.c |2 ++ drivers/video/riva/fbdev.c |1 + include/linux/backlight.h |8 55 files changed, 97 insertions(+), 0 deletions(-) diff --git a/Documentation/ABI/stable/sysfs-class-backlight b/Documentation/ABI/stable/sysfs-class-backlight index 4d637e1..70302f3 100644 --- a/Documentation/ABI/stable/sysfs-class-backlight +++ b/Documentation/ABI/stable/sysfs-class-backlight @@ -34,3 +34,23 @@ Contact: Richard Purdie rpur...@rpsys.net Description: Maximum brightness for backlight. Users: HAL + +What: /sys/class/backlight/backlight/type +Date: September 2010 +KernelVersion: 2.6.37 +Contact: Matthew Garrett m...@redhat.com +Description: + The type of interface controlled by backlight. + firmware: The driver uses a standard firmware interface + platform: The driver uses a platform-specific interface + raw: The driver controls hardware registers directly + + In the general case, when multiple backlight + interfaces are available for a single device, firmware + control should be preferred to platform control should + be preferred to raw control. Using a firmware + interface reduces the probability of confusion with + the hardware and the OS independently updating the + backlight state. Platform interfaces are mostly a + holdover from
Re: [Intel-gfx] [PATCH] gpu/drm/i915: Don't disable panel for modesetting if pfit hasn't changed
On Tue, May 18, 2010 at 01:12:32PM -0700, Jesse Barnes wrote: On Tue, 18 May 2010 13:53:16 -0400 Matthew Garrett m...@redhat.com wrote: It seems to be possible to program a new mode without disabling the panel if the panel fitter setup doesn't change. Add support for that. Signed-off-by: Matthew Garrett m...@redhat.com --- drivers/gpu/drm/i915/intel_lvds.c | 22 -- 1 files changed, 20 insertions(+), 2 deletions(-) Looks good, we should get it upstream early in this cycle to get it some test coverage. If it doesn't work out it's easy to revert. It doesn't look like this got merged? -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/opregion: Use ASLE response codes defined in 0.1
On Thu, Aug 05, 2010 at 12:54:49PM +0100, Chris Wilson wrote: Within i915_opregion.c there are two blocks of semantically identical ASLE response codes defined. Only one of those matches the ACPI IGD OpRegion Specification 0.1, use those. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Matthew Garrett mj...@srcf.ucam.org Acked-by: Matthew Garrett m...@redhat.com -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 0/2] i915 brightness control
On Thu, Jun 03, 2010 at 12:20:39AM +0100, Pedro Ribeiro wrote: Are you sure this is a good idea to enable it by default just because of a problem with one manufacturer (Dell)? It should (a-ha ha) be entirely safe on systems using the IGD opregion spec. My Lenovo laptop has fine brightness control with a GM45 and this may break it. Might be safer only to enable it for laptops with known problems. You have a complete list of all laptops with problems in this area? The ongoing situation with Windows 7 compatibility breaking machines that previously worked suggests that there's something going on which we don't understand or implement. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] gpu/drm/i915: Don't disable panel for modesetting if pfit hasn't changed
It seems to be possible to program a new mode without disabling the panel
if the panel fitter setup doesn't change. Add support for that.
Signed-off-by: Matthew Garrett m...@redhat.com
---
drivers/gpu/drm/i915/intel_lvds.c | 22 --
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index b66806a..1da0030 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -84,6 +84,16 @@ static u32 intel_lvds_get_max_backlight(struct drm_device
*dev)
BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
}
+static void intel_lvds_lock_panel(struct drm_device *dev, bool lock)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (lock)
+ I915_WRITE(PP_CONTROL, I915_READ(PP_CONTROL) 0x3);
+ else
+ I915_WRITE(PP_CONTROL, I915_READ(PP_CONTROL) | (0xabcd 16));
+}
+
/**
* Sets the power state for the panel.
*/
@@ -555,6 +565,8 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
{
struct drm_device *dev = encoder-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_encoder *intel_encoder = enc_to_intel_encoder(encoder);
+ struct intel_lvds_priv *lvds_priv = intel_encoder-dev_priv;
u32 reg;
if (HAS_PCH_SPLIT(dev))
@@ -566,7 +578,10 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
dev_priv-backlight_duty_cycle = (dev_priv-saveBLC_PWM_CTL
BACKLIGHT_DUTY_CYCLE_MASK);
- intel_lvds_set_power(dev, false);
+ if (lvds_priv-pfit_control == I915_READ(PFIT_CONTROL))
+ intel_lvds_lock_panel(dev, false);
+ else
+ intel_lvds_set_power(dev, false);
}
static void intel_lvds_commit( struct drm_encoder *encoder)
@@ -578,7 +593,10 @@ static void intel_lvds_commit( struct drm_encoder *encoder)
dev_priv-backlight_duty_cycle =
intel_lvds_get_max_backlight(dev);
- intel_lvds_set_power(dev, true);
+ if ((I915_READ(PP_CONTROL) (0xabcd 16)) == (0xabcd 16))
+ intel_lvds_lock_panel(dev, true);
+ else
+ intel_lvds_set_power(dev, true);
}
static void intel_lvds_mode_set(struct drm_encoder *encoder,
--
1.6.5.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
