On Mon, Jun 21, 2010 at 12:08 PM, Alberto Milone <alberto.milone@xxxxxxxxxxxxx> wrote: > On 21 June 2010 16:39, Alex Deucher <alexdeucher@xxxxxxxxx> wrote: >> On Mon, Jun 21, 2010 at 10:21 AM, Alberto Milone >> <alberto.milone@xxxxxxxxxxxxx> 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@xxxxxxxxx> >> >> Alex >> > > You made some really good points. > > Thanks for reviewing the patch. Here's the attached patch with the > changes that you requested. Looks good. Thanks! Reviewed-by: Alex Deucher <alexdeucher@xxxxxxxxx> Alex _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel