| Message ID | c489bef11e080941295bc71ac0a4bbd9a211d83d.1744761756.git.soyer@irl.hu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | acpi: method: Fix _DDC test arguments | expand |
Thanks! Acked-by: Ivan Hu <ivan.hu@canonical.com> On 2025/4/17 02:25, Gergo Koteles wrote: > Call the method with 1, 2, 3, 4 instead of the desired length > as in the specification. > > https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device > > Signed-off-by: Gergo Koteles <soyer@irl.hu> > --- > src/acpi/method/method.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c > index 32aceaa9..2dcd1458 100644 > --- a/src/acpi/method/method.c > +++ b/src/acpi/method/method.c > @@ -4738,7 +4738,8 @@ static void method_test_DDC_return( > ACPI_OBJECT *obj, > void *private) > { > - uint32_t requested = *(uint32_t*)private; > + uint32_t arg = *(uint32_t*)private; > + uint32_t requested = arg * 128; > > FWTS_UNUSED(buf); > > @@ -4763,10 +4764,10 @@ static void method_test_DDC_return( > break; > case ACPI_TYPE_INTEGER: > fwts_passed(fw, > - "%s could not return a buffer of %d " > + "%s could not return a buffer of %" PRIu32 " " > "items and instead returned an error " > - "status.", > - name, obj->Buffer.Length); > + "status %" PRIu64 ".", > + name, requested, obj->Integer.Value); > break; > default: > fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_DDCBadReturnType", > @@ -4777,7 +4778,7 @@ static void method_test_DDC_return( > > static int method_test_DDC(fwts_framework *fw) > { > - static int values[] = { 128, 256, 384, 512 }; > + static uint32_t values[] = { 1, 2, 3, 4 }; > ACPI_OBJECT arg[1]; > uint32_t i; >
Hi, On Mon, 2025-04-21 at 14:57 +0800, ivanhu wrote: > Thanks! > > Acked-by: Ivan Hu <ivan.hu@canonical.com> > My _DDC test still passes, even though it shouldn't, because in reality it returns a buffer in package, instead of a buffer. This is because the value of one of the fields is not the same as in the system memory. Is it possible for tests to use the values of the fields from the system memory? Test results: Test 198 of 209: Test _DDC (Return the EDID for this Device). PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a buffer of 128 items and instead returned an error status 0. PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a buffer of 256 items and instead returned an error status 0. PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a buffer of 384 items and instead returned an error status 0. PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a buffer of 512 items and instead returned an error status 0 I have this PAID definition: Scope (\) { OperationRegion (LFCN, SystemMemory, 0x72EE7018, 0x0477) Field (LFCN, AnyAcc, Lock, Preserve) { PS2V, 8, KBID, 8, MCSZ, 8, OKRB, 8, HEAD, 64, MFID, 16, PAID, 16, ... With value 0x417A: # dd bs=1 skip="$((0x72ee7018+14))" count=2 if="/dev/mem" 2>/dev/null | od -An -x 417a And the _DDC method: Name (AUID, 0xA195) Name (IVID, 0x8C45) Name (BOID, 0x0B1B) Name (SAID, 0x417A) Name (SBID, 0x4193) ... Name (SUNG, Package (0x01) { Buffer (0x0100) { ... } }) ... Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current { If ((PAID == AUID)) { Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ } ElseIf ((PAID == IVID)) { Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ } ElseIf ((PAID == BOID)) { Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ } ElseIf ((PAID == SAID)) { Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ } ElseIf ((PAID == SBID)) { Return (SUMG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUMG */ } Return (Zero) } Thanks, Gergo
Hi Gergo, It seems that is another bug, could you help to file a bug on Launchpad, and provide the require files? Please refer to, https://wiki.ubuntu.com/FirmwareTestSuite/FirmwareTestSuiteReportBug Cheers, Ivan On 4/21/25 16:35, Gergo Koteles wrote: > Hi, > > On Mon, 2025-04-21 at 14:57 +0800, ivanhu wrote: >> Thanks! >> >> Acked-by: Ivan Hu <ivan.hu@canonical.com> >> > > My _DDC test still passes, even though it shouldn't, because in reality > it returns a buffer in package, instead of a buffer. > > This is because the value of one of the fields is not the same as in > the system memory. > > Is it possible for tests to use the values of the fields from the > system memory? > > Test results: > > Test 198 of 209: Test _DDC (Return the EDID for this Device). > PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a > buffer of 128 items and instead returned an error status 0. > PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a > buffer of 256 items and instead returned an error status 0. > PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a > buffer of 384 items and instead returned an error status 0. > PASSED: Test 198, \_SB_.PCI0.GP17.VGA_.LCD_._DDC could not return a > buffer of 512 items and instead returned an error status 0 > > I have this PAID definition: > > Scope (\) > { > OperationRegion (LFCN, SystemMemory, 0x72EE7018, 0x0477) > Field (LFCN, AnyAcc, Lock, Preserve) > { > PS2V, 8, > KBID, 8, > MCSZ, 8, > OKRB, 8, > HEAD, 64, > MFID, 16, > PAID, 16, > ... > > With value 0x417A: > > # dd bs=1 skip="$((0x72ee7018+14))" count=2 if="/dev/mem" 2>/dev/null | > od -An -x > 417a > > And the _DDC method: > > Name (AUID, 0xA195) > Name (IVID, 0x8C45) > Name (BOID, 0x0B1B) > Name (SAID, 0x417A) > Name (SBID, 0x4193) > ... > Name (SUNG, Package (0x01) > { > Buffer (0x0100) > { > ... > } > }) > ... > Method (_DDC, 1, NotSerialized) // _DDC: Display Data Current > { > If ((PAID == AUID)) > { > Return (AUOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.AUOP */ > } > ElseIf ((PAID == IVID)) > { > Return (IVOP) /* \_SB_.PCI0.GP17.VGA_.LCD_.IVOP */ > } > ElseIf ((PAID == BOID)) > { > Return (BOEP) /* \_SB_.PCI0.GP17.VGA_.LCD_.BOEP */ > } > ElseIf ((PAID == SAID)) > { > Return (SUNG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUNG */ > } > ElseIf ((PAID == SBID)) > { > Return (SUMG) /* \_SB_.PCI0.GP17.VGA_.LCD_.SUMG */ > } > > Return (Zero) > } > > Thanks, > Gergo >
diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c index 32aceaa9..2dcd1458 100644 --- a/src/acpi/method/method.c +++ b/src/acpi/method/method.c @@ -4738,7 +4738,8 @@ static void method_test_DDC_return( ACPI_OBJECT *obj, void *private) { - uint32_t requested = *(uint32_t*)private; + uint32_t arg = *(uint32_t*)private; + uint32_t requested = arg * 128; FWTS_UNUSED(buf); @@ -4763,10 +4764,10 @@ static void method_test_DDC_return( break; case ACPI_TYPE_INTEGER: fwts_passed(fw, - "%s could not return a buffer of %d " + "%s could not return a buffer of %" PRIu32 " " "items and instead returned an error " - "status.", - name, obj->Buffer.Length); + "status %" PRIu64 ".", + name, requested, obj->Integer.Value); break; default: fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_DDCBadReturnType", @@ -4777,7 +4778,7 @@ static void method_test_DDC_return( static int method_test_DDC(fwts_framework *fw) { - static int values[] = { 128, 256, 384, 512 }; + static uint32_t values[] = { 1, 2, 3, 4 }; ACPI_OBJECT arg[1]; uint32_t i;
Call the method with 1, 2, 3, 4 instead of the desired length as in the specification. https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device Signed-off-by: Gergo Koteles <soyer@irl.hu> --- src/acpi/method/method.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)