> On 26 May 2015, at 07:17, Ilia Mirkin <imirkin@xxxxxxxxxxxx> wrote: > > On Tue, May 26, 2015 at 1:10 AM, Pierre Moreau <pierre.morrow@xxxxxxx> wrote: >>> On 26 May 2015, at 00:39, Ilia Mirkin <imirkin@xxxxxxxxxxxx> wrote: >>> >>>> On Mon, May 25, 2015 at 6:22 PM, Pierre Moreau <pierre.morrow@xxxxxxx> wrote: >>>> Most _DSM will return an integer value of 0x80000002 when given an unknown >>>> UUID, revision ID or function ID. Checking locally allows us to differentiate >>>> that case from other ACPI errors, and to not report a "failed to evaluate _DSM" >>>> if 0x80000002 is returned which was confusing. >>>> >>>> Signed-off-by: Pierre Moreau <pierre.morrow@xxxxxxx> >>>> --- >>>> drm/nouveau/nouveau_acpi.c | 15 ++++++++++++--- >>>> 1 file changed, 12 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drm/nouveau/nouveau_acpi.c b/drm/nouveau/nouveau_acpi.c >>>> index 073f7d7..7aeaf7d 100644 >>>> --- a/drm/nouveau/nouveau_acpi.c >>>> +++ b/drm/nouveau/nouveau_acpi.c >>>> @@ -88,12 +88,12 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int arg, u >>>> for (i = 0; i < 4; i++) >>>> args_buff[i] = (arg >> i * 8) & 0xFF; >>>> >>>> - obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, nouveau_op_dsm_rid, >>>> - func, &argv4, ACPI_TYPE_BUFFER); >>>> + obj = acpi_evaluate_dsm(handle, nouveau_op_dsm_muid, nouveau_op_dsm_rid, >>>> + func, &argv4); >>>> if (!obj) { >>>> acpi_handle_info(handle, "failed to evaluate _DSM\n"); >>>> return AE_ERROR; >>>> - } else { >>>> + } else if (obj->type == ACPI_TYPE_BUFFER) { >>>> if (!result && obj->buffer.length == 4) { >>>> *result = obj->buffer.pointer[0]; >>>> *result |= (obj->buffer.pointer[1] << 8); >>>> @@ -101,6 +101,15 @@ static int nouveau_evaluate_optimus_dsm(acpi_handle handle, int func, int arg, u >>>> *result |= (obj->buffer.pointer[3] << 24); >>>> } >>>> ACPI_FREE(obj); >>>> + } else if (obj->type == ACPI_TYPE_INTEGER && >>>> + obj->integer.value == 0x80000002) { >>>> + acpi_handle_debug(handle, "failed to query Optimus _DSM\n"); >>>> + ACPI_FREE(obj); >>>> + return -ENODEV; >>> >>> should this be AE_ERROR? >> >> I would say no, because ACPI was parsed correctly, just that we didn't it give the correct arguments, or rather, the _DSM we tested isn't an Optimus one, but it could a mux or gmux. And I used ENODEV as it is the value returned by nouveau_evaluate_mux_dsm in the same context. > > Hm ok. It just seemed odd to be returning AE_* in one context, and > -ENODEV in another context -- they're different types of errors. > However if the caller handles it, I guess it's OK... I haven't looked > at the API in depth. The caller doesn’t care about the returned error and just check wether it’s non-zero (and sometimes it doesn’t even check). > >> >>> >>>> + } else { >>>> + acpi_handle_err(handle, "unexpected returned value by Optimus _DSM\n"); >>>> + ACPI_FREE(obj); >>>> + return AE_ERROR; >>>> } >>>> >>>> return 0; >>>> -- >>>> 2.4.1 >>>> >>>> _______________________________________________ >>>> Nouveau mailing list >>>> Nouveau@xxxxxxxxxxxxxxxxxxxxx >>>> http://lists.freedesktop.org/mailman/listinfo/nouveau _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel