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

[Peter Wu]

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