| Message ID | 20210503102422.1384502-1-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/etnaviv: rework linear window offset calculation | expand |
Am Mo., 3. Mai 2021 um 12:24 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > The current calculation based on the required_dma mask can be significantly > off, so that the linear window only overlaps a small part of the DRAM > address space. This can lead to the command buffer being unmappable, which > is obviously bad. > > Rework the linear window offset calculation to be based on the command buffer > physical address, making sure that the command buffer is always mappable. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com> > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 52 +++++++++++++-------------- > 1 file changed, 26 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index c6404b8d067f..a454b13e8106 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -27,10 +27,6 @@ > #include "state_hi.xml.h" > #include "cmdstream.xml.h" > > -#ifndef PHYS_OFFSET > -#define PHYS_OFFSET 0 > -#endif > - > static const struct platform_device_id gpu_ids[] = { > { .name = "etnaviv-gpu,2d" }, > { }, > @@ -724,6 +720,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu) > int etnaviv_gpu_init(struct etnaviv_gpu *gpu) > { > struct etnaviv_drm_private *priv = gpu->drm->dev_private; > + dma_addr_t cmdbuf_paddr; > int ret, i; > > ret = pm_runtime_get_sync(gpu->dev); > @@ -766,28 +763,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) > if (ret) > goto fail; > > - /* > - * Set the GPU linear window to be at the end of the DMA window, where > - * the CMA area is likely to reside. This ensures that we are able to > - * map the command buffers while having the linear window overlap as > - * much RAM as possible, so we can optimize mappings for other buffers. > - * > - * For 3D cores only do this if MC2.0 is present, as with MC1.0 it leads > - * to different views of the memory on the individual engines. > - */ > - if (!(gpu->identity.features & chipFeatures_PIPE_3D) || > - (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) { > - u32 dma_mask = (u32)dma_get_required_mask(gpu->dev); > - if (dma_mask < PHYS_OFFSET + SZ_2G) > - priv->mmu_global->memory_base = PHYS_OFFSET; > - else > - priv->mmu_global->memory_base = dma_mask - SZ_2G + 1; > - } else if (PHYS_OFFSET >= SZ_2G) { > - dev_info(gpu->dev, "Need to move linear window on MC1.0, disabling TS\n"); > - priv->mmu_global->memory_base = PHYS_OFFSET; > - gpu->identity.features &= ~chipFeatures_FAST_CLEAR; > - } > - > /* > * If the GPU is part of a system with DMA addressing limitations, > * request pages for our SHM backend buffers from the DMA32 zone to > @@ -804,6 +779,31 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) > goto fail; > } > > + /* > + * Set the GPU linear window to cover the cmdbuf region, as the GPU > + * won't be able to start execution otherwise. The alignment to 128M is > + * chosen arbitrarily but helps in debugging, as the MMU offset > + * calculations are much more straight forward this way. > + * > + * On MC1.0 cores the linear window offset is ignored by the TS engine, > + * leading to inconsistent memory views. Avoid using the offset on those > + * cores if possible, otherwise disable the TS feature. > + */ > + cmdbuf_paddr = ALIGN_DOWN(etnaviv_cmdbuf_get_pa(&gpu->buffer), SZ_128M); > + > + if (!(gpu->identity.features & chipFeatures_PIPE_3D) || > + (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) { > + if (cmdbuf_paddr >= SZ_2G) > + priv->mmu_global->memory_base = SZ_2G; > + else > + priv->mmu_global->memory_base = cmdbuf_paddr; > + } else if (cmdbuf_paddr + SZ_128M >= SZ_2G) { > + dev_info(gpu->dev, > + "Need to move linear window on MC1.0, disabling TS\n"); > + gpu->identity.features &= ~chipFeatures_FAST_CLEAR; > + priv->mmu_global->memory_base = SZ_2G; > + } > + > /* Setup event management */ > spin_lock_init(&gpu->event_spinlock); > init_completion(&gpu->event_free); > -- > 2.29.2 >
Hi Lucas,
we tested your patch on PHYTEC i.MX6Q phyCORE with 1GiB and 2GiB RAM variants.
I can happily report that the previous issues with "command buffer outside
valid memory window" are gone when using kernel's cmdline parameter cma=256M
for example!
If you want you can add:
Tested-by: Primoz Fiser <primoz.fiser@norik.com>
Otherwise, thank you for prompt response and implemented fix.
BR,
Primoz
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index c6404b8d067f..a454b13e8106 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -27,10 +27,6 @@ #include "state_hi.xml.h" #include "cmdstream.xml.h" -#ifndef PHYS_OFFSET -#define PHYS_OFFSET 0 -#endif - static const struct platform_device_id gpu_ids[] = { { .name = "etnaviv-gpu,2d" }, { }, @@ -724,6 +720,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu) int etnaviv_gpu_init(struct etnaviv_gpu *gpu) { struct etnaviv_drm_private *priv = gpu->drm->dev_private; + dma_addr_t cmdbuf_paddr; int ret, i; ret = pm_runtime_get_sync(gpu->dev); @@ -766,28 +763,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) if (ret) goto fail; - /* - * Set the GPU linear window to be at the end of the DMA window, where - * the CMA area is likely to reside. This ensures that we are able to - * map the command buffers while having the linear window overlap as - * much RAM as possible, so we can optimize mappings for other buffers. - * - * For 3D cores only do this if MC2.0 is present, as with MC1.0 it leads - * to different views of the memory on the individual engines. - */ - if (!(gpu->identity.features & chipFeatures_PIPE_3D) || - (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) { - u32 dma_mask = (u32)dma_get_required_mask(gpu->dev); - if (dma_mask < PHYS_OFFSET + SZ_2G) - priv->mmu_global->memory_base = PHYS_OFFSET; - else - priv->mmu_global->memory_base = dma_mask - SZ_2G + 1; - } else if (PHYS_OFFSET >= SZ_2G) { - dev_info(gpu->dev, "Need to move linear window on MC1.0, disabling TS\n"); - priv->mmu_global->memory_base = PHYS_OFFSET; - gpu->identity.features &= ~chipFeatures_FAST_CLEAR; - } - /* * If the GPU is part of a system with DMA addressing limitations, * request pages for our SHM backend buffers from the DMA32 zone to @@ -804,6 +779,31 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) goto fail; } + /* + * Set the GPU linear window to cover the cmdbuf region, as the GPU + * won't be able to start execution otherwise. The alignment to 128M is + * chosen arbitrarily but helps in debugging, as the MMU offset + * calculations are much more straight forward this way. + * + * On MC1.0 cores the linear window offset is ignored by the TS engine, + * leading to inconsistent memory views. Avoid using the offset on those + * cores if possible, otherwise disable the TS feature. + */ + cmdbuf_paddr = ALIGN_DOWN(etnaviv_cmdbuf_get_pa(&gpu->buffer), SZ_128M); + + if (!(gpu->identity.features & chipFeatures_PIPE_3D) || + (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) { + if (cmdbuf_paddr >= SZ_2G) + priv->mmu_global->memory_base = SZ_2G; + else + priv->mmu_global->memory_base = cmdbuf_paddr; + } else if (cmdbuf_paddr + SZ_128M >= SZ_2G) { + dev_info(gpu->dev, + "Need to move linear window on MC1.0, disabling TS\n"); + gpu->identity.features &= ~chipFeatures_FAST_CLEAR; + priv->mmu_global->memory_base = SZ_2G; + } + /* Setup event management */ spin_lock_init(&gpu->event_spinlock); init_completion(&gpu->event_free);
The current calculation based on the required_dma mask can be significantly off, so that the linear window only overlaps a small part of the DRAM address space. This can lead to the command buffer being unmappable, which is obviously bad. Rework the linear window offset calculation to be based on the command buffer physical address, making sure that the command buffer is always mappable. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 52 +++++++++++++-------------- 1 file changed, 26 insertions(+), 26 deletions(-)