On 21 June 2010 16:39, Alex Deucher <alexdeucher at gmail.com> wrote: > On Mon, Jun 21, 2010 at 10:21 AM, Alberto Milone > <alberto.milone at canonical.com> wrote: >> Hi all, >> >> Thanks to the help of Alex Deucher and Matthew Garrett, I've added >> support for calling the ATIF ACPI method to the radeon driver. This >> makes the video switch hotkey work properly, as we get an ACPI event >> when the key is pressed. >> >> Note: I guess it depends on the bios I'm working with, but I also need >> to pass acpi_osi=\"!Windows 2009\" on boot in order to get the ACPI >> event. > > This is due to windows 7 wanting win-p for display switch events: > http://mjg59.livejournal.com/121851.html > > Couple of minor nits: > > +/* radeon_acpi.c */ > +#if defined(CONFIG_ACPI) > +extern int radeon_acpi_init(struct radeon_device *rdev); > +#else > +static inline int radeon_acpi_init(struct radeon_device *rdev) { return 1; } > +#endif > + > > We probably want to return 0 in the non CONFIG_ACPI case? ?On systems > like ppc without acpi, there's no need for the acpi stuff so the > warning would be needless. ?On systems with acpi that are compiled > without it, well, they will probably have issues anyway... > > + > + ? ? ? /* No need to proceed if we're sure that ATIF is not supported */ > + ? ? ? if (!ASIC_IS_AVIVO(rdev) || !rdev->bios) > + ? ? ? ? ? ? ? return 0; > + > > Move this check to radeon_acpi_init() for now and there's no need to > pass rdev to radeon_atif_call() anymore. > > With those changes: > > Reviewed-by: Alex Deucher <alexdeucher at gmail.com> > > Alex >
You made some really good points. Thanks for reviewing the patch. Here's the attached patch with the changes that you requested. Regards, -- Alberto Milone Sustaining Engineer (system) Foundations Team Canonical OEM Services -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Add-support-for-the-ATIF-ACPI-method-to-the-radeon-d.patch Type: text/x-patch Size: 4709 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20100621/0c7714ed/attachment-0001.bin>
