From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f196.google.com (mail-yb1-f196.google.com [209.85.219.196]) by mx.groups.io with SMTP id smtpd.web11.317.1588114960093872944 for ; Tue, 28 Apr 2020 16:02:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=OX8L5OQ4; spf=pass (domain: gmail.com, ip: 209.85.219.196, mailfrom: andrey.warkentin@gmail.com) Received: by mail-yb1-f196.google.com with SMTP id t9so238593ybo.8 for ; Tue, 28 Apr 2020 16:02:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=DRPzt9O0XNXuFDPX3sn5dDfjMgdgiTSewgFjjpejfzw=; b=OX8L5OQ4OF5koB5jNVvRkalPOXTVEjJevc8USA+Ttozr3H2/Fkv0LKhy9JVALek5zF XhiLEpHjaP8VSF1eu7KuoVzDwlv4kltHUNIqlTObaT+CEBaUFjmF0ylZr/7o71UBO2VX mzJrmRbaqGrV7sY4RXKLKUo5XC0PVikk1zkT7WPdCbL2p1P5IEmYnaPaHltxG0Y9411r DGgcY7W268gimlLotmUVzyZH3xG8MtsJZRl5M6BrGNRAW3phRPMb9srohl83QT0wWzAd +6S+ucWO1k6FBw/txIqggUn2/gAb4p6fTfTurO5RhY2NNwF6Dpn4HWmdYoY3y3/m0DF9 e3qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=DRPzt9O0XNXuFDPX3sn5dDfjMgdgiTSewgFjjpejfzw=; b=kDrUjxml/gRYcIpd9wz4/t8r2ovJpE5dHUab6t55PrUecUkHcMVuCnbIQwnuIk5/Qm dqul/lUtRukB7cxCPOz4h5wG//wuKqjntZCylHWo/7Dpb+RHQIyHI+PfIkPlkybpVRJY IntOqftjMNOZYjAAzXlu0MWgGIrtNojG019h2KKYapfMSfJp/SWbjKSilfLuUo5aSol+ h8G6uXMLG8tmRrhc3GW91zh/fQYYpNmuLAr9Rx4gjgc4jWc3mvOEqe5Iau+aIRwtHB/s oos/KMkpc9tTsmiC7ss90kaKpC188uQloA/nXYogczxLNobn7KgcbAvJAf9vrxrDvarw 5M3Q== X-Gm-Message-State: AGi0Pub24pO179HvcmYbofmvAui3XKku6VeaK16gozMp4l6bwR5C/6mE h9T/CI3Pn6w2teI+18V2rOiY6n5rXSNQG9PbmrfGI6Ww X-Google-Smtp-Source: APiQypI6bnv1AIkuQ9w2EER90+/mvzRpvlkSgso5p40wZtrjW8vTzulDCS7TMdto6oZNOydwe0loG0utt5Hv2Rwa8rA= X-Received: by 2002:a25:86c6:: with SMTP id y6mr46678610ybm.311.1588114959165; Tue, 28 Apr 2020 16:02:39 -0700 (PDT) MIME-Version: 1.0 References: <20200428050940.350-1-driver1998@foxmail.com> In-Reply-To: <20200428050940.350-1-driver1998@foxmail.com> From: "Andrei Warkentin" Date: Tue, 28 Apr 2020 18:02:28 -0500 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms][PATCH] Platform/RaspberryPi: Fix VPU memory ranges in GPU device container To: devel@edk2.groups.io, driver1998@foxmail.com Content-Type: multipart/alternative; boundary="00000000000000ea6905a461d4a2" --00000000000000ea6905a461d4a2 Content-Type: text/plain; charset="UTF-8" Reviewed-by: Andrei Warkentin Note that this not normal - ACPI resources aren't supposed to encode bus and CPU resources in this fashion. This may mean the device will fail admission on some OSes, and ultimately may mean the PWM device definition will be hidden behind an "OS choice" UEFI setup option. I get that this is how the driver is consuming it, but consider that the driver ( https://github.com/ms-iot/rpi-iotcore/tree/master/drivers/pwm) is also open source and should be evolving as well. A On Tue, Apr 28, 2020 at 1:34 AM GH Cao wrote: > The PWM controller device specifies both CPU and VPU memory ranges. > CPU base addresses are provided as offsets to BCM2836_SOC_REGISTERS, > and VPU base addresses are constants. > > But in Dsdt.asl, both offsets and constant addresses are seen as offsets > by QWORDMEMORYSET macro, result in incorrect VPU memory ranges. > > This commits adds a new QWORDBUSMEMORYSET macro to handle VPU memory > ranges with constant base addresses. > > Signed-off-by: GH Cao > --- > Platform/RaspberryPi/AcpiTables/Dsdt.asl | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl > b/Platform/RaspberryPi/AcpiTables/Dsdt.asl > index 95766b0..ddaf1d4 100644 > --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl > +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl > @@ -31,7 +31,11 @@ > // The ASL compiler does not support argument arithmetic in functions > // like QWordMemory (). So we need to instantiate dummy qword regions > // that we can then update the Min, Max and Length attributes of. > -// The two macros below help accomplish this. > +// The three macros below help accomplish this. > +// > +// QWORDMEMORYSET specifies a CPU memory range (whose base address is > +// BCM2836_SOC_REGISTERS + Offset), and QWORDBUSMEMORYSET specifies > +// a VPU memory range (whose base address is provided directly). > // > #define QWORDMEMORYBUF(Index) \ > QWordMemory (ResourceProducer,, \ > @@ -46,6 +50,14 @@ > Add (BCM2836_SOC_REGISTERS, Offset, MI ## Index) \ > Add (MI ## Index, LE ## Index - 1, MA ## Index) > > +#define QWORDBUSMEMORYSET(Index, Base, Length) \ > + CreateQwordField (RBUF, RB ## Index._MIN, MI ## Index) \ > + CreateQwordField (RBUF, RB ## Index._MAX, MA ## Index) \ > + CreateQwordField (RBUF, RB ## Index._LEN, LE ## Index) \ > + Store (Base, MI ## Index) \ > + Store (Length, LE ## Index) \ > + Add (MI ## Index, LE ## Index - 1, MA ## Index) > + > DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2) > { > Scope (\_SB_) > @@ -173,8 +185,8 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", > "RPI", 2) > // PWM > QWORDMEMORYSET(17, BCM2836_PWM_DMA_OFFSET, BCM2836_PWM_DMA_LENGTH) > QWORDMEMORYSET(18, BCM2836_PWM_CTRL_OFFSET, > BCM2836_PWM_CTRL_LENGTH) > - QWORDMEMORYSET(19, BCM2836_PWM_BUS_BASE_ADDRESS, > BCM2836_PWM_BUS_LENGTH) > - QWORDMEMORYSET(20, BCM2836_PWM_CTRL_UNCACHED_BASE_ADDRESS, > BCM2836_PWM_CTRL_UNCACHED_LENGTH) > + QWORDBUSMEMORYSET(19, BCM2836_PWM_BUS_BASE_ADDRESS, > BCM2836_PWM_BUS_LENGTH) > + QWORDBUSMEMORYSET(20, BCM2836_PWM_CTRL_UNCACHED_BASE_ADDRESS, > BCM2836_PWM_CTRL_UNCACHED_LENGTH) > QWORDMEMORYSET(21, BCM2836_PWM_CLK_OFFSET, BCM2836_PWM_CLK_LENGTH) > > // UART > -- > 2.17.1 > > > > > -- A --00000000000000ea6905a461d4a2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Reviewed-by: Andrei Warkentin <andrey.warkentin@gmail.com>

=
Note that this not normal - ACPI resources aren't supposed to enco= de bus and CPU resources in this fashion. This may mean the device will fai= l admission on some OSes, and ultimately may mean the PWM device definition= will be hidden behind an "OS choice" UEFI setup option. I get th= at this is how the driver is consuming it, but consider that the driver (htt= ps://github.com/ms-iot/rpi-iotcore/tree/master/drivers/pwm) is also ope= n source and should be evolving as well.

A

On= Tue, Apr 28, 2020 at 1:34 AM GH Cao <driver1998@foxmail.com> wrote:
The PWM controller device specifies both CPU = and VPU memory ranges.
CPU base addresses are provided as offsets to BCM2836_SOC_REGISTERS,
and VPU base addresses are constants.

But in Dsdt.asl, both offsets and constant addresses are seen as offsets by QWORDMEMORYSET macro, result in incorrect VPU memory ranges.

This commits adds a new QWORDBUSMEMORYSET macro to handle VPU memory
ranges with constant base addresses.

Signed-off-by: GH Cao <driver1998@foxmail.com>
---
=C2=A0Platform/RaspberryPi/AcpiTables/Dsdt.asl | 18 +++++++++++++++---
=C2=A01 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/Raspberry= Pi/AcpiTables/Dsdt.asl
index 95766b0..ddaf1d4 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -31,7 +31,11 @@
=C2=A0// The ASL compiler does not support argument arithmetic in function= s
=C2=A0// like QWordMemory (). So we need to instantiate dummy qword region= s
=C2=A0// that we can then update the Min, Max and Length attributes of. -// The two macros below help accomplish this.
+// The three macros below help accomplish this.
+//
+// QWORDMEMORYSET specifies a CPU memory range (whose base address is
+// BCM2836_SOC_REGISTERS + Offset), and QWORDBUSMEMORYSET specifies
+// a VPU memory range (whose base address is provided directly).
=C2=A0//
=C2=A0#define QWORDMEMORYBUF(Index)=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = = =C2=A0 =C2=A0\
=C2=A0 =C2=A0QWordMemory (ResourceProducer,,=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0\
@@ -46,6 +50,14 @@
=C2=A0 =C2=A0Add (BCM2836_SOC_REGISTERS, Offset, MI ## Index)=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
=C2=A0 =C2=A0Add (MI ## Index, LE ## Index - 1, MA ## Index)

+#define QWORDBUSMEMORYSET(Index, Base, Length)=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 \
+=C2=A0 CreateQwordField (RBUF, RB ## Index._MIN, MI ## Index)=C2=A0 =C2= =A0 =C2=A0 =C2=A0 \
+=C2=A0 CreateQwordField (RBUF, RB ## Index._MAX, MA ## Index)=C2=A0 =C2= =A0 =C2=A0 =C2=A0 \
+=C2=A0 CreateQwordField (RBUF, RB ## Index._LEN, LE ## Index)=C2=A0 =C2= =A0 =C2=A0 =C2=A0 \
+=C2=A0 Store (Base, MI ## Index)=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0\
+=C2=A0 Store (Length, LE ## Index)=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = = =C2=A0 =C2=A0\
+=C2=A0 Add (MI ## Index, LE ## Index - 1, MA ## Index)
+
=C2=A0DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RP= IFDN", "RPI", 2)
=C2=A0{
=C2=A0 =C2=A0Scope (\_SB_)
@@ -173,8 +185,8 @@ DefinitionBlock ("Dsdt.aml", "DSDT"= ;, 5, "RPIFDN", "RPI", 2)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0// PWM
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QWORDMEMORYSET(17, BCM2836_PWM_DMA_OFFSE= T, BCM2836_PWM_DMA_LENGTH)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QWORDMEMORYSET(18, BCM2836_PWM_CTRL_OFFS= ET, BCM2836_PWM_CTRL_LENGTH)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 QWORDMEMORYSET(19, BCM2836_PWM_BUS_BASE_ADDRE= SS, BCM2836_PWM_BUS_LENGTH)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 QWORDMEMORYSET(20, BCM2836_PWM_CTRL_UNCACHED_= BASE_ADDRESS, BCM2836_PWM_CTRL_UNCACHED_LENGTH)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 QWORDBUSMEMORYSET(19, BCM2836_PWM_BUS_BASE_AD= DRESS, BCM2836_PWM_BUS_LENGTH)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 QWORDBUSMEMORYSET(20, BCM2836_PWM_CTRL_UNCACH= ED_BASE_ADDRESS, BCM2836_PWM_CTRL_UNCACHED_LENGTH)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QWORDMEMORYSET(21, BCM2836_PWM_CLK_OFFSE= T, BCM2836_PWM_CLK_LENGTH)

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0// UART
--
2.17.1






--
A
--00000000000000ea6905a461d4a2--