* use of dma_direct_set_offset in (allwinner) drivers @ 2020-11-03 9:55 Christoph Hellwig 2020-11-04 8:14 ` Maxime Ripard 0 siblings, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2020-11-03 9:55 UTC (permalink / raw) To: Maxime Ripard, Chen-Yu Tsai, Yong Deng, Paul Kocialkowski Cc: devel, linux-kernel, dri-devel, iommu, linux-arm-kernel, linux-media Hi all, Linux 5.10-rc1 switched from having a single dma offset in struct device to a set of DMA ranges, and introduced a new helper to set them, dma_direct_set_offset. This in fact surfaced that a bunch of drivers that violate our layering and set the offset from drivers, which meant we had to reluctantly export the symbol to set up the DMA range. The drivers are: drivers/gpu/drm/sun4i/sun4i_backend.c This just use dma_direct_set_offset as a fallback. Is there any good reason to not just kill off the fallback? drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c Same as above. drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c This driver unconditionally sets the offset. Why can't we do this in the device tree? drivers/staging/media/sunxi/cedrus/cedrus_hw.c Same as above. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: use of dma_direct_set_offset in (allwinner) drivers 2020-11-03 9:55 use of dma_direct_set_offset in (allwinner) drivers Christoph Hellwig @ 2020-11-04 8:14 ` Maxime Ripard 2020-11-04 10:15 ` Robin Murphy 0 siblings, 1 reply; 5+ messages in thread From: Maxime Ripard @ 2020-11-04 8:14 UTC (permalink / raw) To: Christoph Hellwig Cc: devel, linux-kernel, dri-devel, Paul Kocialkowski, Chen-Yu Tsai, iommu, Yong Deng, linux-arm-kernel, linux-media [-- Attachment #1.1: Type: text/plain, Size: 2280 bytes --] Hi Christoph, On Tue, Nov 03, 2020 at 10:55:38AM +0100, Christoph Hellwig wrote: > Linux 5.10-rc1 switched from having a single dma offset in struct device > to a set of DMA ranges, and introduced a new helper to set them, > dma_direct_set_offset. > > This in fact surfaced that a bunch of drivers that violate our layering > and set the offset from drivers, which meant we had to reluctantly > export the symbol to set up the DMA range. > > The drivers are: > > drivers/gpu/drm/sun4i/sun4i_backend.c > > This just use dma_direct_set_offset as a fallback. Is there any good > reason to not just kill off the fallback? > > drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c > > Same as above. So, the history of this is: - We initially introduced the support for those two controllers assuming that there was a direct mapping between the physical and DMA addresses. It turns out it didn't and the DMA accesses were going through a secondary, dedicated, bus that didn't have the same mapping of the RAM than the CPU. 4690803b09c6 ("drm/sun4i: backend: Offset layer buffer address by DRAM starting address") - This dedicated bus is undocumented and barely used in the vendor kernel so this was overlooked, and it's fairly hard to get infos on it for all the SoCs we support. We added the DT support for it though on some SoCs we had enough infos to do so: c43a4469402f ("dt-bindings: interconnect: Add a dma interconnect name") 22f88e311399 ("ARM: dts: sun5i: Add the MBUS controller") This explains the check on the interconnect property - However, due to the stable DT rule, we still need to operate without regressions on older DTs that wouldn't have that property (and for SoCs we haven't figured out). Hence the fallback. > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > > This driver unconditionally sets the offset. Why can't we do this > in the device tree? > > drivers/staging/media/sunxi/cedrus/cedrus_hw.c > > Same as above. > We should make those two match the previous ones, but we'll have the same issue here eventually. Most likely they were never ran on an SoC for which we have the MBUS figured out. Maxime [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: use of dma_direct_set_offset in (allwinner) drivers 2020-11-04 8:14 ` Maxime Ripard @ 2020-11-04 10:15 ` Robin Murphy 2020-11-04 10:29 ` Christoph Hellwig 2020-11-04 12:43 ` Maxime Ripard 0 siblings, 2 replies; 5+ messages in thread From: Robin Murphy @ 2020-11-04 10:15 UTC (permalink / raw) To: Maxime Ripard, Christoph Hellwig Cc: devel, iommu, dri-devel, linux-kernel, Paul Kocialkowski, Chen-Yu Tsai, Yong Deng, linux-arm-kernel, linux-media On 2020-11-04 08:14, Maxime Ripard wrote: > Hi Christoph, > > On Tue, Nov 03, 2020 at 10:55:38AM +0100, Christoph Hellwig wrote: >> Linux 5.10-rc1 switched from having a single dma offset in struct device >> to a set of DMA ranges, and introduced a new helper to set them, >> dma_direct_set_offset. >> >> This in fact surfaced that a bunch of drivers that violate our layering >> and set the offset from drivers, which meant we had to reluctantly >> export the symbol to set up the DMA range. >> >> The drivers are: >> >> drivers/gpu/drm/sun4i/sun4i_backend.c >> >> This just use dma_direct_set_offset as a fallback. Is there any good >> reason to not just kill off the fallback? >> >> drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c >> >> Same as above. > > So, the history of this is: > > - We initially introduced the support for those two controllers > assuming that there was a direct mapping between the physical and > DMA addresses. It turns out it didn't and the DMA accesses were > going through a secondary, dedicated, bus that didn't have the same > mapping of the RAM than the CPU. > > 4690803b09c6 ("drm/sun4i: backend: Offset layer buffer address by DRAM starting address") > > - This dedicated bus is undocumented and barely used in the vendor > kernel so this was overlooked, and it's fairly hard to get infos on > it for all the SoCs we support. We added the DT support for it > though on some SoCs we had enough infos to do so: > > c43a4469402f ("dt-bindings: interconnect: Add a dma interconnect name") > 22f88e311399 ("ARM: dts: sun5i: Add the MBUS controller") > > This explains the check on the interconnect property > > - However, due to the stable DT rule, we still need to operate without > regressions on older DTs that wouldn't have that property (and for > SoCs we haven't figured out). Hence the fallback. How about having something in the platform code that keys off the top-level SoC compatible and uses a bus notifier to create offsets for the relevant devices if an MBUS description is missing? At least that way the workaround could be confined to a single dedicated place and look somewhat similar to other special cases like sta2x11, rather than being duplicated all over the place. Robin. >> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c >> >> This driver unconditionally sets the offset. Why can't we do this >> in the device tree? >> >> drivers/staging/media/sunxi/cedrus/cedrus_hw.c >> >> Same as above. >> > > We should make those two match the previous ones, but we'll have the > same issue here eventually. Most likely they were never ran on an SoC > for which we have the MBUS figured out. > > Maxime > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: use of dma_direct_set_offset in (allwinner) drivers 2020-11-04 10:15 ` Robin Murphy @ 2020-11-04 10:29 ` Christoph Hellwig 2020-11-04 12:43 ` Maxime Ripard 1 sibling, 0 replies; 5+ messages in thread From: Christoph Hellwig @ 2020-11-04 10:29 UTC (permalink / raw) To: Robin Murphy Cc: devel, linux-kernel, dri-devel, Paul Kocialkowski, Chen-Yu Tsai, iommu, Maxime Ripard, Yong Deng, Christoph Hellwig, linux-arm-kernel, linux-media On Wed, Nov 04, 2020 at 10:15:49AM +0000, Robin Murphy wrote: > How about having something in the platform code that keys off the top-level > SoC compatible and uses a bus notifier to create offsets for the relevant > devices if an MBUS description is missing? At least that way the workaround > could be confined to a single dedicated place and look somewhat similar to > other special cases like sta2x11, rather than being duplicated all over the > place. Yes, that would be the right way to handle the issue. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: use of dma_direct_set_offset in (allwinner) drivers 2020-11-04 10:15 ` Robin Murphy 2020-11-04 10:29 ` Christoph Hellwig @ 2020-11-04 12:43 ` Maxime Ripard 1 sibling, 0 replies; 5+ messages in thread From: Maxime Ripard @ 2020-11-04 12:43 UTC (permalink / raw) To: Robin Murphy Cc: devel, linux-kernel, dri-devel, Paul Kocialkowski, Chen-Yu Tsai, iommu, Yong Deng, Christoph Hellwig, linux-arm-kernel, linux-media [-- Attachment #1.1: Type: text/plain, Size: 2623 bytes --] On Wed, Nov 04, 2020 at 10:15:49AM +0000, Robin Murphy wrote: > On 2020-11-04 08:14, Maxime Ripard wrote: > > Hi Christoph, > > > > On Tue, Nov 03, 2020 at 10:55:38AM +0100, Christoph Hellwig wrote: > > > Linux 5.10-rc1 switched from having a single dma offset in struct device > > > to a set of DMA ranges, and introduced a new helper to set them, > > > dma_direct_set_offset. > > > > > > This in fact surfaced that a bunch of drivers that violate our layering > > > and set the offset from drivers, which meant we had to reluctantly > > > export the symbol to set up the DMA range. > > > > > > The drivers are: > > > > > > drivers/gpu/drm/sun4i/sun4i_backend.c > > > > > > This just use dma_direct_set_offset as a fallback. Is there any good > > > reason to not just kill off the fallback? > > > > > > drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c > > > > > > Same as above. > > > > So, the history of this is: > > > > - We initially introduced the support for those two controllers > > assuming that there was a direct mapping between the physical and > > DMA addresses. It turns out it didn't and the DMA accesses were > > going through a secondary, dedicated, bus that didn't have the same > > mapping of the RAM than the CPU. > > > > 4690803b09c6 ("drm/sun4i: backend: Offset layer buffer address by DRAM starting address") > > > > - This dedicated bus is undocumented and barely used in the vendor > > kernel so this was overlooked, and it's fairly hard to get infos on > > it for all the SoCs we support. We added the DT support for it > > though on some SoCs we had enough infos to do so: > > > > c43a4469402f ("dt-bindings: interconnect: Add a dma interconnect name") > > 22f88e311399 ("ARM: dts: sun5i: Add the MBUS controller") > > > > This explains the check on the interconnect property > > > > - However, due to the stable DT rule, we still need to operate without > > regressions on older DTs that wouldn't have that property (and for > > SoCs we haven't figured out). Hence the fallback. > > How about having something in the platform code that keys off the top-level > SoC compatible and uses a bus notifier to create offsets for the relevant > devices if an MBUS description is missing? At least that way the workaround > could be confined to a single dedicated place and look somewhat similar to > other special cases like sta2x11, rather than being duplicated all over the > place. I'll give it a try, thanks for the suggestion :) Maxime [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-04 12:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-03 9:55 use of dma_direct_set_offset in (allwinner) drivers Christoph Hellwig 2020-11-04 8:14 ` Maxime Ripard 2020-11-04 10:15 ` Robin Murphy 2020-11-04 10:29 ` Christoph Hellwig 2020-11-04 12:43 ` Maxime Ripard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).