Nouveau - May 2016 - [PATCH] gpu/nouveau/nouveau_acpi.c: Fix Type Mismatch ACPI warning

Hello Marcos,

I sent a serie a year ago to fix some of the ACPI handling in Nouveau and add
runtime pm support for laptops with an Apple GMUX (see [0], and especially [1]
and [2]). I was told that a more generic work for the runtime pm was in the
work, so I let the whole serie slip away. I was thinking of resubmitting the
serie without the runtime pm additions, to at least fix those warning/error
messages. I thought I had a patch to fix this issue in my serie, but going
through it again, it looks like I did not.
Note that even, though the spec says the 4th argument should be a package, some
old BIOSes expect a different type, like a buffer (see the comment above [5]; I
think there was also a comment about it in the spec, but I can’t find it
anymore).

I added some comments below. I’ll test this patch tonight, to see how it works
on my laptop as I think it expects a buffer as 4th argument.

Thank you!

Regards,
Pierre


[0]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083444.html
[1]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083448.html
[2]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083449.html

On 09:42 PM - May 18 2016, Marcos Paulo de Souza wrote:
> nouveau_optimus_dsm is using ACPI_TYPE_BUFFER, and this triggers warnings
on ACPI:
> [    7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [    7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (201509    30/nsarguments-95)
> 
> To fix it, change ACPI_TYPE_BUFFER to ACPI_TYPE_PACKAGE, as the warning
tells to do.
> 
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org at gmail.com>
> ---
> 
> After booting my Asus Laptop, and after a suspend/resume too, dmesg shows
warnings:
> [    1.633361] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [    1.633434] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [    7.730176] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [    7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [    7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   49.732059] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   49.732424] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [   49.732430] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   74.366300] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   74.366657] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [   74.366663] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [  140.357789] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [  140.358532] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [  140.358547] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type
mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
Out of curiosity, which GPU(s) do you have in your laptop? I guess there is at
least an NVIDIA one, but is it an Optimus configuration? And if so, does the
unused card go automatically to sleep after a few seconds?
> 
> I don't know if this is the right thing to do, I just looked at
intel_acpi.c to check how to use/check for ACPI Package.
> The patch below silenced the "type mismatch" warnings, and some
of the "evaluated _DSM" ones.
> 
> If this is not the right approach, please let me know how to fix it, I
don't have knowledge in ACPI, but I really want to help.
> 
>  drivers/gpu/drm/nouveau/nouveau_acpi.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> index cdf5227..f04aef3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> @@ -73,22 +73,10 @@ static const char nouveau_op_dsm_muid[] = {
>  
>  static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg,
uint32_t *result)
>  {
> -	int i;
>  	union acpi_object *obj;
> -	char args_buff[4];
> -	union acpi_object argv4 = {
> -		.buffer.type = ACPI_TYPE_BUFFER,
> -		.buffer.length = 4,
> -		.buffer.pointer = args_buff
> -	};
> -
> -	/* ACPI is little endian, AABBCCDD becomes {DD,CC,BB,AA} */
> -	for (i = 0; i < 4; i++)
> -		args_buff[i] = (arg >> i * 8) & 0xFF;
> -
>  	*result = 0;
>  	obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 0x00000100,
> -				      func, &argv4, ACPI_TYPE_BUFFER);
> +				      func, NULL, ACPI_TYPE_PACKAGE);
The last parameter you give to `acpi_evaluate_dsm_typed()` is the return type
you expect (see [3]), which will be a buffer if func is 0, and is
implementation dependent otherwise (see section 9.14.1 _DSM of [4]). So you
don’t want to change it to ACPI_TYPE_PACKAGE. If you look at the implementation
of `acpi_evaluate_dsm()` (which is called by `acpi_evaluate_dsm_typed()`), it
will automatically create a package for the 4th argument, if you pass it a NULL
pointer (see [5]).

[3]:
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/include/acpi/acpi_bus.h#L69
[4]: http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf
[5]:
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/drivers/acpi/utils.c#L628
>  	if (!obj) {
>  		acpi_handle_info(handle, "failed to evaluate _DSM\n");
>  		return AE_ERROR;
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20160519/8c17ee53/attachment.sig>

Hi Pierre,

Em qui, 19 de mai de 2016 às 06:49, Pierre Moreau <pierre.morrow at
free.fr>
escreveu:
> Hello Marcos,
>
> I sent a serie a year ago to fix some of the ACPI handling in Nouveau and
> add
> runtime pm support for laptops with an Apple GMUX (see [0], and especially
> [1]
> and [2]). I was told that a more generic work for the runtime pm was in the
> work, so I let the whole serie slip away. I was thinking of resubmitting
> the
> serie without the runtime pm additions, to at least fix those warning/error
> messages. I thought I had a patch to fix this issue in my serie, but going
> through it again, it looks like I did not.
> Note that even, though the spec says the 4th argument should be a package,
> some
> old BIOSes expect a different type, like a buffer (see the comment above
> [5]; I
> think there was also a comment about it in the spec, but I can’t find it
> anymore).
>
> I added some comments below. I’ll test this patch tonight, to see how it
> works
> on my laptop as I think it expects a buffer as 4th argument.
>
Good, I'm learning a lot here. I'm reading all links you pasted here.

>
> Thank you!
>
> Regards,
> Pierre
>
>
> [0]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083444.html
> [1]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083448.html
> [2]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083449.html
>
> On 09:42 PM - May 18 2016, Marcos Paulo de Souza wrote:
> > nouveau_optimus_dsm is using ACPI_TYPE_BUFFER, and this triggers
> warnings on ACPI:
> > [    7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> > [    7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package] (201509
> 30/nsarguments-95)
> >
> > To fix it, change ACPI_TYPE_BUFFER to ACPI_TYPE_PACKAGE, as the
warning
> tells to do.
> >
> > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org at
gmail.com>
> > ---
> >
> > After booting my Asus Laptop, and after a suspend/resume too, dmesg
> shows warnings:
> > [    1.633361] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [    1.633434] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [    7.730176] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [    7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> > [    7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [   49.732059] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [   49.732424] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> > [   49.732430] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [   74.366300] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [   74.366657] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> > [   74.366663] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [  140.357789] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
> > [  140.358532] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> > [  140.358547] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4
type
> mismatch - Found [Buffer], ACPI requires [Package]
(20150930/nsarguments-95)
>
> Out of curiosity, which GPU(s) do you have in your laptop? I guess there
> is at
> least an NVIDIA one, but is it an Optimus configuration? And if so, does
> the
> unused card go automatically to sleep after a few seconds?
>
I have an Asus laptop with a NVD7 (GF117) Fermi card, and a Haswell onboard
graphic card. About the automatic sleep, I really don't don't know,
I'm
just seeing these messages :)

>
> >
> > I don't know if this is the right thing to do, I just looked at
> intel_acpi.c to check how to use/check for ACPI Package.
> > The patch below silenced the "type mismatch" warnings, and
some of the
> "evaluated _DSM" ones.
> >
> > If this is not the right approach, please let me know how to fix it, I
> don't have knowledge in ACPI, but I really want to help.
> >
> >  drivers/gpu/drm/nouveau/nouveau_acpi.c | 14 +-------------
> >  1 file changed, 1 insertion(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > index cdf5227..f04aef3 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > @@ -73,22 +73,10 @@ static const char nouveau_op_dsm_muid[] = {
> >
> >  static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg,
> uint32_t *result)
> >  {
> > -     int i;
> >       union acpi_object *obj;
> > -     char args_buff[4];
> > -     union acpi_object argv4 = {
> > -             .buffer.type = ACPI_TYPE_BUFFER,
> > -             .buffer.length = 4,
> > -             .buffer.pointer = args_buff
> > -     };
> > -
> > -     /* ACPI is little endian, AABBCCDD becomes {DD,CC,BB,AA} */
> > -     for (i = 0; i < 4; i++)
> > -             args_buff[i] = (arg >> i * 8) & 0xFF;
> > -
> >       *result = 0;
> >       obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid,
> 0x00000100,
> > -                                   func, &argv4,
ACPI_TYPE_BUFFER);
> > +                                   func, NULL, ACPI_TYPE_PACKAGE);
>
> The last parameter you give to `acpi_evaluate_dsm_typed()` is the return
> type
> you expect (see [3]), which will be a buffer if func is 0, and is
> implementation dependent otherwise (see section 9.14.1 _DSM of [4]). So you
> don’t want to change it to ACPI_TYPE_PACKAGE. If you look at the
> implementation
> of `acpi_evaluate_dsm()` (which is called by `acpi_evaluate_dsm_typed()`),
> it
> will automatically create a package for the 4th argument, if you pass it a
> NULL
> pointer (see [5]).
>
> [3]:
>
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/include/acpi/acpi_bus.h#L69
> [4]: http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf
> [5]:
>
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/drivers/acpi/utils.c#L628

Thanks for all the links. I'll read the docs and send a new version of the
patch when it makes more sense instead of just replacing random things.

>
>
> >       if (!obj) {
> >               acpi_handle_info(handle, "failed to evaluate
_DSM\n");
> >               return AE_ERROR;
> > --
> > 2.5.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20160520/5416344d/attachment-0001.html>

On Fri, May 20, 2016 at 02:22:57AM +0000, Marcos Souza wrote:
[..]
> > > I don't know if this is the right thing to do, I just looked
at
> > intel_acpi.c to check how to use/check for ACPI Package.
> > > The patch below silenced the "type mismatch" warnings,
and some of the
> > "evaluated _DSM" ones.
> > >
> > > If this is not the right approach, please let me know how to fix
it, I
> > don't have knowledge in ACPI, but I really want to help.
> > >
> > >  drivers/gpu/drm/nouveau/nouveau_acpi.c | 14 +-------------
> > >  1 file changed, 1 insertion(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > > index cdf5227..f04aef3 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > > @@ -73,22 +73,10 @@ static const char nouveau_op_dsm_muid[] = {
> > >
> > >  static int nouveau_optimus_dsm(acpi_handle handle, int func, int
arg,
> > uint32_t *result)
> > >  {
> > > -     int i;
> > >       union acpi_object *obj;
> > > -     char args_buff[4];
> > > -     union acpi_object argv4 = {
> > > -             .buffer.type = ACPI_TYPE_BUFFER,
> > > -             .buffer.length = 4,
> > > -             .buffer.pointer = args_buff
> > > -     };
> > > -
> > > -     /* ACPI is little endian, AABBCCDD becomes {DD,CC,BB,AA} */
> > > -     for (i = 0; i < 4; i++)
> > > -             args_buff[i] = (arg >> i * 8) & 0xFF;
> > > -
> > >       *result = 0;
> > >       obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid,
> > 0x00000100,
> > > -                                   func, &argv4,
ACPI_TYPE_BUFFER);
> > > +                                   func, NULL,
ACPI_TYPE_PACKAGE);
This effectively removes the fourth parameter (actually, using a zero
as fourth argument), making the function useless.
> > The last parameter you give to `acpi_evaluate_dsm_typed()` is the
return
> > type
> > you expect (see [3]), which will be a buffer if func is 0, and is
> > implementation dependent otherwise (see section 9.14.1 _DSM of [4]).
So you
> > don’t want to change it to ACPI_TYPE_PACKAGE. If you look at the
> > implementation
> > of `acpi_evaluate_dsm()` (which is called by
`acpi_evaluate_dsm_typed()`),
> > it
> > will automatically create a package for the 4th argument, if you pass
it a
> > NULL
> > pointer (see [5]).
> >
> > [3]:
> >
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/include/acpi/acpi_bus.h#L69
> > [4]: http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf
> > [5]:
> >
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/drivers/acpi/utils.c#L628
> 
> 
> Thanks for all the links. I'll read the docs and send a new version of
the
> patch when it makes more sense instead of just replacing random things.
The warning is unavoidable, most firmware expect a Buffer for the fourth
argument (counting from 0, this is Arg3). Excerpt from the DSM method on
a recent Skylake laptop (Clevo P651, but this format is found on many
other models from various manufacturers too):

    If ((Arg2 == 0x1A))
    {
        CreateField (Arg3, 0x18, 0x02, OMPR)
        CreateField (Arg3, Zero, One, FLCH)
        CreateField (Arg3, One, One, DVSR)
        CreateField (Arg3, 0x02, One, DVSC)

(The sections below refer to the one in the ACPI 6.1 document that can
be found at http://uefi.org/specifications.)

The first parameter for CreateField is evaluated as buffer (sec 19.6.21).
According to 19.3.5.6 (Data Types and Type Conversions) an implicit
conversion to a Buffer is only possible from an Integer and String, a
Package does not belong to the possibilities.

Note that the return value may be an integer for unsupported revision
IDs or UUIDs (like 0x80000002). These should be compatible with Buffers
though as stated above and acpi_check_dsm() can handle that case, but
unfortunately sets a Package as fourth argument and can therefore not be
used in nouveau.
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl