Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code

From: Luca Tettamanti <kronos.it@gmail.com>
To: joeyli <jlee@suse.com>
Cc: Alex Deucher <alexander.deucher@amd.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/radeon: add new AMD ACPI header and update relevant code
Date: Mon, 30 Jul 2012 16:16:36 +0200	[thread overview]
Message-ID: <20120730141636.GA18550@growl> (raw)
In-Reply-To: <1343637167.6341.76.camel@linux-s257.site>

On Mon, Jul 30, 2012 at 04:32:47PM +0800, joeyli wrote:
> 於 日，2012-07-29 於 15:10 +0200，Luca Tettamanti 提到：
> > On Sun, Jul 29, 2012 at 11:51:48AM +0800, joeyli wrote:
> > > Hi Luca, 
> > > 
> > > 於 六，2012-07-28 於 16:56 +0200，Luca Tettamanti 提到：
> > > > I just found the first problem (probably a BIOS bug):
> > > > ATIF_FUNCTION_GET_SYSTEM_PARAMETERS is implemented in the DSDT, but the
> > > > corresponding bit ATIF_GET_SYSTEM_PARAMETERS_SUPPORTED is not set :(
> > > > I intended to use the method to set up the notification handler but now
> > > > my BIOS says that it's not there even if it is...
> > > > Can I assume some default values (e.g. notifications are enabled and will
> > > > use 0x81 unless ATIF_FUNCTION_GET_SYSTEM_PARAMETERS says something
> > > > different)?
> > > > 
> > > 
> > > Did you check your DSDT for there have some "Notify (VGA, 0x81)"
> > > statement in AFN0..AFN15?
> > > If YES, I think that means your machine in case the 0x81 is for ATI used
> > > by default.
> > 
> > Yes, my point is that the nofication is there, but since
> > GET_SYSTEM_PARAMETERS is not announced I have not way to check it.
> > IOW, what is implemented in the DSDT does not match what is announced by
> > VERIFY_INTERFACE.
> > For reference this is the DSDT: http://pastebin.com/KKS7ZsTt
> > 
> > Luca
> > 
> 
> Yes, saw the problem in your DSDT:
> 
>         Method (AF00, 0, NotSerialized)
>         {
>             CreateWordField (ATIB, Zero, SSZE)
> 		...
>             Store (0x80, NMSK)
>             Store (0x02, SFUN)		<=== 10b, bit 0 is 0
>             Return (ATIB)
>         }
> 
> But, AF01 still supported in ATIF on this machine, maybe we should still
> try GET_SYSTEM_PARAMETERS even the function vector set to 0?
> No idea...

That's what I'm doing right now... if SBIOS_REQUESTS is supported I try
and call GET_SYSTEM_PARAMETERS even if it's not announced.

> On the other hand, 
> My patch to avoid 0x81 event conflict with acpi/video driver like below.
> This patch woks on my notebook. Your patches do much more things then
> mine, so I think my patch just for reference.  

I ignored the event handling for now... I'd like to hear something back
from ACPI camp before committing to this solution.

> There have a problem is:
> If we want use acpi_notifier_call_chain to check does event consume by
> any notifier in acpi's blocking notifier chain, then we need return
> NOTIFY_BAD in radeon_acpi but not NOTIFY_OK.
> 
> So, I suggest radeon_acpi should register to acpi notifier chain by
> itself but not append on radeon_acpi_event in radeon_pm. 

It shouldn't matter, once I change radeon_atif_handler to return
NOTIFY_BAD the call chain will be stopped anyway.

> And,
> suggest also check the device class is ACPI_VIDEO_CLASS like following:
> 
> +static int radeon_acpi_video_event(struct notifier_block *nb,
> ...
> +	if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0)
> +		return NOTIFY_DONE;
> +

Will do. I'll use the package structs in the next iteration.

Luca
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel