| Message ID | 20190507155935.12672-1-christoph.muellner@theobroma-systems.com |
|---|---|
| State | Deferred |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | [U-Boot,v2,1/3] board_f: Add mach specific DMA address check function. | expand |
Hi Christoph, On Tue, 7 May 2019 at 09:59, Christoph Muellner <christoph.muellner@theobroma-systems.com> wrote: > > From: Christoph Müllner <christoph.muellner@theobroma-systems.com> > > Some machines have limited DMA engines, which cannot deal > with arbitrary addresses. This patch introduces a function > to model these restrictions on a machine level. > > Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com> > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > --- > > Changes in v2: None > > common/board_f.c | 5 +++++ > include/init.h | 2 ++ > 2 files changed, 7 insertions(+) > Can we handle this with driver model somehow? How does the kernel handle it? Is there a device-tree binding for the DMA node that could provide this information. Also, where is this function called from? Regards, Simon
Hi Simon, Am Samstag, 18. Mai 2019, 18:08:58 CEST schrieb Simon Glass: > On Tue, 7 May 2019 at 09:59, Christoph Muellner > <christoph.muellner@theobroma-systems.com> wrote: > > > > From: Christoph Müllner <christoph.muellner@theobroma-systems.com> > > > > Some machines have limited DMA engines, which cannot deal > > with arbitrary addresses. This patch introduces a function > > to model these restrictions on a machine level. > > > > Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com> > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > --- > > > > Changes in v2: None > > > > common/board_f.c | 5 +++++ > > include/init.h | 2 ++ > > 2 files changed, 7 insertions(+) > > > > Can we handle this with driver model somehow? How does the kernel > handle it? The problem Christoph is trying to solve here is doing a mmc transfer from mmc to the sram (not main memory), which cannot use dma. So this exact problem doesn't happen in the kernel itself. But back in veyron times we had a similar dma issue with anything accessing that area as dma hung the system, but the solution was just reserving the memblock: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b21bcfc9fda56bac573367d18ce3e4dbf3cdedf9 As the commit describes, other options would've been soc-settings also going around the kernel's driver model or limiting the dma-able memory even more (to 2GB), so we opted to just reserve the upper 16MB completely. > Is there a device-tree binding for the DMA node that could > provide this information. I don't think so. At least in the kernel affecting the dma-mask is a setting for the using component (mmc, gmac, whatever), so would mean adapting every device doing dma ... and even then there wasn't a dt-binding for something like that, which in turn would've required to adapt every driver to set a specific per-soc dma-mask for Rockchip compatibles - hence the "simple" reserve above was the least intrusive option. > Also, where is this function called from? In the theobroma u-boot it gets called from the bouncebuffer right now https://git.theobroma-systems.com/puma-u-boot.git/commit/?id=d68222d45b4e7f55f500f5e28722cb4304ecde96 to check if the mmc drivers can dma directly or needs to use the bouncebuffer for reaching something like the sram. Heiko
Hi Heiko, On Wed, 22 May 2019 at 05:29, Heiko Stuebner <heiko@sntech.de> wrote: > > Hi Simon, > > Am Samstag, 18. Mai 2019, 18:08:58 CEST schrieb Simon Glass: > > On Tue, 7 May 2019 at 09:59, Christoph Muellner > > <christoph.muellner@theobroma-systems.com> wrote: > > > > > > From: Christoph Müllner <christoph.muellner@theobroma-systems.com> > > > > > > Some machines have limited DMA engines, which cannot deal > > > with arbitrary addresses. This patch introduces a function > > > to model these restrictions on a machine level. > > > > > > Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com> > > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > > --- > > > > > > Changes in v2: None > > > > > > common/board_f.c | 5 +++++ > > > include/init.h | 2 ++ > > > 2 files changed, 7 insertions(+) > > > > > > > Can we handle this with driver model somehow? How does the kernel > > handle it? > > The problem Christoph is trying to solve here is doing a mmc transfer > from mmc to the sram (not main memory), which cannot use dma. > So this exact problem doesn't happen in the kernel itself. > > But back in veyron times we had a similar dma issue with anything accessing > that area as dma hung the system, but the solution was just reserving the > memblock: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b21bcfc9fda56bac573367d18ce3e4dbf3cdedf9 > > As the commit describes, other options would've been soc-settings > also going around the kernel's driver model or limiting the dma-able > memory even more (to 2GB), so we opted to just reserve the upper 16MB > completely. OK I see. > > > > Is there a device-tree binding for the DMA node that could > > provide this information. > > I don't think so. At least in the kernel affecting the dma-mask is a > setting for the using component (mmc, gmac, whatever), so would > mean adapting every device doing dma ... and even then there wasn't > a dt-binding for something like that, which in turn would've required > to adapt every driver to set a specific per-soc dma-mask for Rockchip > compatibles - hence the "simple" reserve above was the least intrusive > option. That's odd. Are you saying that some devices can DMA from SRAM and some cannot? If the DMA is not allowed, could the DMA driver return -EPERM or similar when the attempt is made, and then the bounce buffer can be used if needed? > > > > Also, where is this function called from? > > In the theobroma u-boot it gets called from the bouncebuffer right now > https://git.theobroma-systems.com/puma-u-boot.git/commit/?id=d68222d45b4e7f55f500f5e28722cb4304ecde96 > to check if the mmc drivers can dma directly or needs to use the > bouncebuffer for reaching something like the sram. OK ta. > > > Heiko > > Regards, Simon
On 23.05.19 00:57, Simon Glass wrote: > Hi Heiko, > > On Wed, 22 May 2019 at 05:29, Heiko Stuebner <heiko@sntech.de> wrote: >> >> Hi Simon, >> >> Am Samstag, 18. Mai 2019, 18:08:58 CEST schrieb Simon Glass: >>> On Tue, 7 May 2019 at 09:59, Christoph Muellner >>> <christoph.muellner@theobroma-systems.com> wrote: >>>> >>>> From: Christoph Müllner <christoph.muellner@theobroma-systems.com> >>>> >>>> Some machines have limited DMA engines, which cannot deal >>>> with arbitrary addresses. This patch introduces a function >>>> to model these restrictions on a machine level. >>>> >>>> Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com> >>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> >>>> --- >>>> >>>> Changes in v2: None >>>> >>>> common/board_f.c | 5 +++++ >>>> include/init.h | 2 ++ >>>> 2 files changed, 7 insertions(+) >>>> >>> >>> Can we handle this with driver model somehow? How does the kernel >>> handle it? >> >> The problem Christoph is trying to solve here is doing a mmc transfer >> from mmc to the sram (not main memory), which cannot use dma. >> So this exact problem doesn't happen in the kernel itself. >> >> But back in veyron times we had a similar dma issue with anything accessing >> that area as dma hung the system, but the solution was just reserving the >> memblock: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b21bcfc9fda56bac573367d18ce3e4dbf3cdedf9 >> >> As the commit describes, other options would've been soc-settings >> also going around the kernel's driver model or limiting the dma-able >> memory even more (to 2GB), so we opted to just reserve the upper 16MB >> completely. > > OK I see. > >> >> >>> Is there a device-tree binding for the DMA node that could >>> provide this information. >> >> I don't think so. At least in the kernel affecting the dma-mask is a >> setting for the using component (mmc, gmac, whatever), so would >> mean adapting every device doing dma ... and even then there wasn't >> a dt-binding for something like that, which in turn would've required >> to adapt every driver to set a specific per-soc dma-mask for Rockchip >> compatibles - hence the "simple" reserve above was the least intrusive >> option. > > That's odd. Are you saying that some devices can DMA from SRAM and some cannot? Yes, that is also confirmed by Kever (see commit message of [1]). My patch series assumes, that only the CPU can see the non-DDR memory areas (including SRAM). I would really like to get feedback here from Kever. Kever has posted a first patch to address this end of March [2]. After waiting a few weeks, I've prepared a reworked version (because I needed it for mainline ATF testing on our RK3399-Q7). A few hours after I sent them to the list Kever also pushed another patch for this [1]. From my perspective mainline ATF cannot be used with mainline U-Boot on RK3399 when booting from SD card or eMMC at the moment. [1] https://patchwork.ozlabs.org/patch/1096366/ [2] https://patchwork.ozlabs.org/patch/1069730/ > > If the DMA is not allowed, could the DMA driver return -EPERM or > similar when the attempt is made, and then the bounce buffer can be > used if needed? > >> >> >>> Also, where is this function called from? >> >> In the theobroma u-boot it gets called from the bouncebuffer right now >> https://git.theobroma-systems.com/puma-u-boot.git/commit/?id=d68222d45b4e7f55f500f5e28722cb4304ecde96 >> to check if the mmc drivers can dma directly or needs to use the >> bouncebuffer for reaching something like the sram. > > OK ta. > >> >> >> Heiko >> >> > > Regards, > Simon >
diff --git a/common/board_f.c b/common/board_f.c index 7ef20f2042..92dc34d2d8 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -1053,3 +1053,8 @@ void board_init_f_r(void) hang(); } #endif /* CONFIG_X86 */ + +__weak int mach_addr_is_dmaable(void __iomem *ptr) +{ + return 1; +} diff --git a/include/init.h b/include/init.h index afc953d51e..1653d5086f 100644 --- a/include/init.h +++ b/include/init.h @@ -125,6 +125,8 @@ int misc_init_f(void); int embedded_dtb_select(void); #endif +int mach_addr_is_dmaable(void __iomem *ptr); + /* common/init/board_init.c */ extern ulong monitor_flash_len;