Re: [PATCH] drm/nouveau/acpi: de-dup use of DSM methods

[Peter Wu <lekensteyn@gmail.com>]

On Friday 02 August 2013 15:58:38 Dave Airlie wrote:
> On Fri, Aug 2, 2013 at 12:41 AM, Peter Wu <lekensteyn@gmail.com> wrote:
> > Observe that nouveau_optimus_dsm and nouveau_dsm are equal except for
> > the parameters handling (UUID, revision ID and function arguments). The
> > function arguments are passed as Buffer in the "optimus dsm" and Integer
> > in "nvidia dsm". As buffers are implicitly converted to integers, merge
> > both functions.
> > 
> > The ACPI spec defines the fourth parameter (Arg3 a.k.a. "function
> > arguments") as Package, but many BIOSes expect a Buffer instead. For
> > instance, for the "nvidia DSM", the Lenovo T410s uses CreateByteField on
> > Arg3 which does not work with a package. The Clevo B7130 does something
> > similar for the "Optimus DSM". Unfortunately, this means that the
> > following ACPI warning (introduced with 29a241c) cannot be fixed (when
> 
> > toggling power or muxing):
> By cannot be fixed, why is it there then? maybe ask the ACPI folks to
> drop this error, since as far as I can see all optimus dsm want a
> buffer here.

It helps tracking violations of the ACPI spec which can be used as debugging 
tool, as a result there is also a patch for i915 (which was based on this 
nouveau code). Of course, if real world only uses Buffers, then this warning 
should be dropped/changed, but there are more users of _DSM besides the 
graphics subsystem.

Regards,
Peter

> >     ACPI Warning: \_SB_.PCI0.GFX0._DSM: Argument #4 type mismatch - Found
> >     [Buffer], ACPI requires [Package] (20130517/nsarguments-95) ACPI
> >     Warning: \_SB_.PCI0.GFX0._DSM: Argument #4 type mismatch - Found
> >     [Buffer], ACPI requires [Package] (20130517/nsarguments-95) ACPI
> >     Warning: \_SB_.PCI0.P0P2.PEGP._DSM: Argument #4 type mismatch - Found
> >     [Buffer], ACPI requires [Package] (20130517/nsarguments-95) ACPI
> >     Warning: \_SB_.PCI0.P0P2.PEGP._DSM: Argument #4 type mismatch - Found
> >     [Buffer], ACPI requires [Package] (20130517/nsarguments-95)> 
> > Signed-off-by: Peter Wu <lekensteyn@gmail.com>
> > ---
> > 
> >  drivers/gpu/drm/nouveau/nouveau_acpi.c | 67
> >  ++++++++++------------------------ 1 file changed, 20 insertions(+), 47
> >  deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > b/drivers/gpu/drm/nouveau/nouveau_acpi.c index d97f200..a75684f 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> > @@ -46,6 +46,9 @@ bool nouveau_is_v1_dsm(void) {
> > 
> >  #define NOUVEAU_DSM_HAS_MUX 0x1
> >  #define NOUVEAU_DSM_HAS_OPT 0x2
> > 
> > +#define NOUVEAU_DSM_REVID_NVIDIA 0x102
> > +#define NOUVEAU_DSM_REVID_OPTIMUS 0x100
> > +
> > 
> >  static const char nouveau_dsm_muid[] = {
> >  
> >         0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
> >         0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
> > 
> > @@ -56,7 +59,8 @@ static const char nouveau_op_dsm_muid[] = {
> > 
> >         0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
> >  
> >  };
> > 
> > -static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg,
> > uint32_t *result) +static int nouveau_call_dsm(acpi_handle handle, const
> > char *uuid, int revid, +       int func, int arg, uint32_t *result)
> > 
> >  {
> >  
> >         struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> >         struct acpi_object_list input;
> > 
> > @@ -68,12 +72,15 @@ static int nouveau_optimus_dsm(acpi_handle handle, int
> > func, int arg, uint32_t *> 
> >         input.count = 4;
> >         input.pointer = params;
> >         params[0].type = ACPI_TYPE_BUFFER;
> > 
> > -       params[0].buffer.length = sizeof(nouveau_op_dsm_muid);
> > -       params[0].buffer.pointer = (char *)nouveau_op_dsm_muid;
> > +       params[0].buffer.length = 16;
> > +       params[0].buffer.pointer = (char *)uuid;
> > 
> >         params[1].type = ACPI_TYPE_INTEGER;
> > 
> > -       params[1].integer.value = 0x00000100;
> > +       params[1].integer.value = revid;
> > 
> >         params[2].type = ACPI_TYPE_INTEGER;
> >         params[2].integer.value = func;
> > 
> > +       /* Although the ACPI spec defines Arg3 as a Package, in practise
> > +        * implementations expect a Buffer (CreateWordField and Index
> > functions +        * are applied to it). */
> > 
> >         params[3].type = ACPI_TYPE_BUFFER;
> >         params[3].buffer.length = 4;
> >         /* ACPI is little endian, AABBCCDD becomes {DD,CC,BB,AA} */
> > 
> > @@ -108,50 +115,16 @@ static int nouveau_optimus_dsm(acpi_handle handle,
> > int func, int arg, uint32_t *> 
> >         return 0;
> >  
> >  }
> > 
> > -static int nouveau_dsm(acpi_handle handle, int func, int arg, uint32_t
> > *result) -{
> > -       struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> > -       struct acpi_object_list input;
> > -       union acpi_object params[4];
> > -       union acpi_object *obj;
> > -       int err;
> > -
> > -       input.count = 4;
> > -       input.pointer = params;
> > -       params[0].type = ACPI_TYPE_BUFFER;
> > -       params[0].buffer.length = sizeof(nouveau_dsm_muid);
> > -       params[0].buffer.pointer = (char *)nouveau_dsm_muid;
> > -       params[1].type = ACPI_TYPE_INTEGER;
> > -       params[1].integer.value = 0x00000102;
> > -       params[2].type = ACPI_TYPE_INTEGER;
> > -       params[2].integer.value = func;
> > -       params[3].type = ACPI_TYPE_INTEGER;
> > -       params[3].integer.value = arg;
> > -
> > -       err = acpi_evaluate_object(handle, "_DSM", &input, &output);
> > -       if (err) {
> > -               printk(KERN_INFO "failed to evaluate _DSM: %d\n", err);
> > -               return err;
> > -       }
> > -
> > -       obj = (union acpi_object *)output.pointer;
> > -
> > -       if (obj->type == ACPI_TYPE_INTEGER)
> > -               if (obj->integer.value == 0x80000002)
> > -                       return -ENODEV;
> > -
> > -       if (obj->type == ACPI_TYPE_BUFFER) {
> > -               if (obj->buffer.length == 4 && result) {
> > -                       *result = 0;
> > -                       *result |= obj->buffer.pointer[0];
> > -                       *result |= (obj->buffer.pointer[1] << 8);
> > -                       *result |= (obj->buffer.pointer[2] << 16);
> > -                       *result |= (obj->buffer.pointer[3] << 24);
> > -               }
> > -       }
> > +static int nouveau_optimus_dsm(acpi_handle handle, int func,
> > +       int arg, uint32_t *result) {
> > +       return nouveau_call_dsm(handle, nouveau_op_dsm_muid,
> > +                       NOUVEAU_DSM_REVID_OPTIMUS, func, arg, result);
> > +}
> > 
> > -       kfree(output.pointer);
> > -       return 0;
> > +static int nouveau_dsm(acpi_handle handle, int func,
> > +       int arg, uint32_t *result) {
> > +       return nouveau_call_dsm(handle, nouveau_dsm_muid,
> > +                       NOUVEAU_DSM_REVID_NVIDIA, func, arg, result);
> > 
> >  }
> >  
> >  /* Returns 1 if a DSM function is usable and 0 otherwise */
> > 
> > --
> > 1.8.3.4
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel