Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
soc/amd/picasso/acpi: add UART DMA controller devices
The four Designware UARTs in the SoC each have a PL330 DMA controller attached that shares the IRQ of the UART and has its two DMA channels hard wired to the RX and TX channels of the UART. This patch exposes those DMA controllers as ACPI devices with the HID AMD0023. At 3Mbaud we need to use the DMA controller to avoid that some bytes got dropped occasionally.
The kernel-side patches are currently still work in progress.
BUG=b:160208269
Change-Id: I54c3f3d9b6806884366fdef131306c02d1142657 Signed-off-by: Julian Schroeder julian.schroeder@amd.corp-partner.google.com --- M src/soc/amd/picasso/acpi/sb_fch.asl 1 file changed, 127 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/45479/1
diff --git a/src/soc/amd/picasso/acpi/sb_fch.asl b/src/soc/amd/picasso/acpi/sb_fch.asl index 6cbfc5f..28ac988 100644 --- a/src/soc/amd/picasso/acpi/sb_fch.asl +++ b/src/soc/amd/picasso/acpi/sb_fch.asl @@ -92,6 +92,122 @@ } }
+Device (DMA0) { + Name (_HID, "AMD0023") + Name (_UID, 0x0) + Method (_CRS, 0) { + Local0 = ResourceTemplate() { + Interrupt ( + ResourceConsumer, + Edge, + ActiveHigh, + Shared, , , IRQR) + { 0 } + Memory32Fixed (ReadWrite, APU_DMAC0_BASE, 0x1000) + } + CreateDWordField (Local0, IRQR._INT, IRQN) + If (PMOD) { + IRQN = IUA0 + } Else { + IRQN = PUA0 + } + If (IRQN == 0x1f) { + Return (ResourceTemplate() { + Memory32Fixed (ReadWrite, APU_DMAC0_BASE, 0x1000) + }) + } Else { + Return (Local0) + } + } +} + +Device (DMA1) { + Name (_HID, "AMD0023") + Name (_UID, 0x0) + Method (_CRS, 0) { + Local0 = ResourceTemplate() { + Interrupt ( + ResourceConsumer, + Edge, + ActiveHigh, + Shared, , , IRQR) + { 0 } + Memory32Fixed (ReadWrite, APU_DMAC1_BASE, 0x1000) + } + CreateDWordField (Local0, IRQR._INT, IRQN) + If (PMOD) { + IRQN = IUA1 + } Else { + IRQN = PUA1 + } + If (IRQN == 0x1f) { + Return (ResourceTemplate() { + Memory32Fixed (ReadWrite, APU_DMAC1_BASE, 0x1000) + }) + } Else { + Return (Local0) + } + } +} + +Device (DMA2) { + Name (_HID, "AMD0023") + Name (_UID, 0x0) + Method (_CRS, 0) { + Local0 = ResourceTemplate() { + Interrupt ( + ResourceConsumer, + Edge, + ActiveHigh, + Shared, , , IRQR) + { 0 } + Memory32Fixed (ReadWrite, APU_DMAC2_BASE, 0x1000) + } + CreateDWordField (Local0, IRQR._INT, IRQN) + If (PMOD) { + IRQN = IUA2 + } Else { + IRQN = PUA2 + } + If (IRQN == 0x1f) { + Return (ResourceTemplate() { + Memory32Fixed (ReadWrite, APU_DMAC2_BASE, 0x1000) + }) + } Else { + Return (Local0) + } + } +} + +Device (DMA3) { + Name (_HID, "AMD0023") + Name (_UID, 0x0) + Method (_CRS, 0) { + Local0 = ResourceTemplate() { + Interrupt ( + ResourceConsumer, + Edge, + ActiveHigh, + Shared, , , IRQR) + { 0 } + Memory32Fixed (ReadWrite, APU_DMAC3_BASE, 0x1000) + } + CreateDWordField (Local0, IRQR._INT, IRQN) + If (PMOD) { + IRQN = IUA3 + } Else { + IRQN = PUA3 + } + If (IRQN == 0x1f) { + Return (ResourceTemplate() { + Memory32Fixed (ReadWrite, APU_DMAC3_BASE, 0x1000) + }) + } Else { + Return (Local0) + } + } +} + Device (FUR0) { Name (_HID, "AMD0020") @@ -105,7 +221,8 @@ Exclusive, , , IRQR) { 0 } Memory32Fixed (ReadWrite, APU_UART0_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC0_BASE, 0x1000) + FixedDMA (0x0, 0x0, Width32Bit, ) + FixedDMA (0x0, 0x1, Width32Bit, ) } CreateDWordField (Local0, IRQR._INT, IRQN) If (PMOD) { @@ -116,13 +233,11 @@ If (IRQN == 0x1f) { Return (ResourceTemplate() { Memory32Fixed (ReadWrite, APU_UART0_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC0_BASE, 0x1000) }) } Else { Return (Local0) } } - Name (_PR0, Package () { _SB.AOAC.FUR0 }) Name (_PR2, Package () { _SB.AOAC.FUR0 }) Name (_PR3, Package () { _SB.AOAC.FUR0 }) @@ -145,10 +260,11 @@ ResourceConsumer, Edge, ActiveHigh, - Exclusive, , , IRQR) + Shared, , , IRQR) { 0 } Memory32Fixed (ReadWrite, APU_UART1_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC1_BASE, 0x1000) + FixedDMA (0x1, 0x0, Width32Bit, ) + FixedDMA (0x1, 0x1, Width32Bit, ) } CreateDWordField (Local0, IRQR._INT, IRQN) If (PMOD) { @@ -159,13 +275,11 @@ If (IRQN == 0x1f) { Return (ResourceTemplate() { Memory32Fixed (ReadWrite, APU_UART1_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC1_BASE, 0x1000) }) } Else { Return (Local0) } } - Name (_PR0, Package () { _SB.AOAC.FUR1 }) Name (_PR2, Package () { _SB.AOAC.FUR1 }) Name (_PR3, Package () { _SB.AOAC.FUR1 }) @@ -188,10 +302,11 @@ ResourceConsumer, Edge, ActiveHigh, - Exclusive, , , IRQR) + Shared, , , IRQR) { 0 } Memory32Fixed (ReadWrite, APU_UART2_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC2_BASE, 0x1000) + FixedDMA (0x2, 0x0, Width32Bit, ) + FixedDMA (0x2, 0x1, Width32Bit, ) } CreateDWordField (Local0, IRQR._INT, IRQN) If (PMOD) { @@ -202,7 +317,6 @@ If (IRQN == 0x1f) { Return (ResourceTemplate() { Memory32Fixed (ReadWrite, APU_UART2_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC2_BASE, 0x1000) }) } Else { Return (Local0) @@ -231,10 +345,11 @@ ResourceConsumer, Edge, ActiveHigh, - Exclusive, , , IRQR) + Shared, , , IRQR) { 0 } Memory32Fixed (ReadWrite, APU_UART3_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC3_BASE, 0x1000) + FixedDMA (0x3, 0x0, Width32Bit, ) + FixedDMA (0x3, 0x1, Width32Bit, ) } CreateDWordField (Local0, IRQR._INT, IRQN) If (PMOD) { @@ -245,7 +360,6 @@ If (IRQN == 0x1f) { Return (ResourceTemplate() { Memory32Fixed (ReadWrite, APU_UART3_BASE, 0x1000) - Memory32Fixed (ReadWrite, APU_DMAC3_BASE, 0x1000) }) } Else { Return (Local0)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 182: Device (DMA3) { How does the driver associate these devices w/ the proper uart? Is that going to be open coded somehow to provide the binding? There doesn't appear to be any topology information encoded in the hierarchy because all these devices just float together.
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 221: Exclusive You changed FUR1, FUR2, and FUR3 to Shared.
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Marshall Dawson, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45479
to look at the new patch set (#2).
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
soc/amd/picasso/acpi: add UART DMA controller devices
The four Designware UARTs in the SoC each have a PL330 DMA controller attached that shares the IRQ of the UART and has its two DMA channels hard wired to the RX and TX channels of the UART. This patch exposes those DMA controllers as ACPI devices with the HID AMD0023. At 3Mbaud we need to use the DMA controller to avoid that some bytes got dropped occasionally.
The kernel-side patches are currently still work in progress.
BUG=b:160208269
Change-Id: I54c3f3d9b6806884366fdef131306c02d1142657 Signed-off-by: Julian Schroeder julian.schroeder@amd.corp-partner.google.com --- M src/soc/amd/picasso/acpi/sb_fch.asl 1 file changed, 130 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/45479/2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 182: Device (DMA3) {
How does the driver associate these devices w/ the proper uart? Is that going to be open coded someh […]
The mapping is in the first parameter of FixedDMA. I'm not sure how that works in detail though
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 221: Exclusive
You changed FUR1, FUR2, and FUR3 to Shared.
good catch
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 182: Device (DMA3) {
The mapping is in the first parameter of FixedDMA. […]
Ahh. I see. However, to make that binding the driver will have to open code those relative DmaRequestLine in some form. Ok. If you have a pointer to the kernel patch that'd be helpful.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/2/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/2/src/soc/amd/picasso/acpi/sb... PS2, Line 102: Edge Hrmm, is this still the case then? AFAIK we should not be sharing edge level interrupts.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 182: Device (DMA3) {
Ahh. I see. […]
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
* Accordingly to ACPI 5.0 Specification Table 6-170 "Fixed DMA Resource * Descriptor": * DMA Request Line bits is a platform-relative number uniquely * identifying the request line assigned. Request line-to-Controller * mapping is done in a controller-specific OS driver. * That's why we can safely adjust slave_id when the appropriate controller is * found.
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
I haven't found where the relative indexes are assigned.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/1/src/soc/amd/picasso/acpi/sb... PS1, Line 182: Device (DMA3) {
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th.... […]
as far as i know the linux-side code is still work in progress. I'm just upstreaming Julian's coreboot-side change for this
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45479/2//COMMIT_MSG@13 PS2, Line 13: got get
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Marshall Dawson, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45479
to look at the new patch set (#3).
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
soc/amd/picasso/acpi: add UART DMA controller devices
The four Designware UARTs in the SoC each have a PL330 DMA controller attached that shares the IRQ of the UART and has its two DMA channels hard wired to the RX and TX channels of the UART. This patch exposes those DMA controllers as ACPI devices with the HID AMD0023. At 3Mbaud we need to use the DMA controller to avoid that some bytes get dropped occasionally.
The kernel-side patches are currently still work in progress.
BUG=b:160208269
Change-Id: I54c3f3d9b6806884366fdef131306c02d1142657 Signed-off-by: Julian Schroeder julian.schroeder@amd.corp-partner.google.com --- M src/soc/amd/picasso/acpi/sb_fch.asl 1 file changed, 130 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/45479/3
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45479/2//COMMIT_MSG@13 PS2, Line 13: got
get
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 3: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/2/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/2/src/soc/amd/picasso/acpi/sb... PS2, Line 102: Edge
Hrmm, is this still the case then? AFAIK we should not be sharing edge level interrupts.
Felix, did you look into this?
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45479/2/src/soc/amd/picasso/acpi/sb... File src/soc/amd/picasso/acpi/sb_fch.asl:
https://review.coreboot.org/c/coreboot/+/45479/2/src/soc/amd/picasso/acpi/sb... PS2, Line 102: Edge
Felix, did you look into this?
I emailed Julian about this and he'll have look into it
Felix Held has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45479 )
Change subject: soc/amd/picasso/acpi: add UART DMA controller devices ......................................................................
Abandoned
i don't think that we got this working properly and it doesn't seem to be needed any more
-
Aaron Durbin (Code Review) -
Felix Held (Code Review)
-
Paul Menzel (Code Review)
-
Raul Rangel (Code Review)