| Message ID | 20220411043423.37333-4-samuel@sholland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/sun4i: Allwinner D1 Display Engine 2.0 Support | expand |
Dne ponedeljek, 11. april 2022 ob 06:34:15 CEST je Samuel Holland napisal(a): > commit b4bdc4fbf8d0 ("soc: sunxi: Deal with the MBUS DMA offsets in a > central place") added a platform device notifier that sets the DMA > offset for all of the display engine frontend and backend devices. > > The code applying the offset to DMA buffer physical addresses was then > removed from the backend driver in commit 756668ba682e ("drm/sun4i: > backend: Remove the MBUS quirks"), but the code subtracting PHYS_OFFSET > was left in the frontend driver. > > As a result, the offset was applied twice in the frontend driver. This > likely went unnoticed because it only affects specific configurations > (scaling or certain pixel formats) where the frontend is used, on boards > with both one of these older SoCs and more than 1 GB of DRAM. > > In addition, the references to PHYS_OFFSET prevent compiling the driver > on architectures where PHYS_OFFSET is not defined. > > Fixes: b4bdc4fbf8d0 ("soc: sunxi: Deal with the MBUS DMA offsets in a central place") > Signed-off-by: Samuel Holland <samuel@sholland.org> Good catch! Actually, people complained about non-working display on Cubietruck IIRC, which has 2 GB of RAM. Did you test this on HW? Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej > --- > > drivers/gpu/drm/sun4i/sun4i_frontend.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/ sun4i_frontend.c > index 56ae38389db0..462fae73eae9 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_frontend.c > +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c > @@ -222,13 +222,11 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend, > > /* Set the physical address of the buffer in memory */ > paddr = drm_fb_cma_get_gem_addr(fb, state, 0); > - paddr -= PHYS_OFFSET; > DRM_DEBUG_DRIVER("Setting buffer #0 address to %pad\n", &paddr); > regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR0_REG, paddr); > > if (fb->format->num_planes > 1) { > paddr = drm_fb_cma_get_gem_addr(fb, state, swap ? 2 : 1); > - paddr -= PHYS_OFFSET; > DRM_DEBUG_DRIVER("Setting buffer #1 address to %pad\n", &paddr); > regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR1_REG, > paddr); > @@ -236,7 +234,6 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend, > > if (fb->format->num_planes > 2) { > paddr = drm_fb_cma_get_gem_addr(fb, state, swap ? 1 : 2); > - paddr -= PHYS_OFFSET; > DRM_DEBUG_DRIVER("Setting buffer #2 address to %pad\n", &paddr); > regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR2_REG, > paddr); > -- > 2.35.1 > >
On 4/11/22 10:37 AM, Jernej Škrabec wrote: > Dne ponedeljek, 11. april 2022 ob 06:34:15 CEST je Samuel Holland napisal(a): >> commit b4bdc4fbf8d0 ("soc: sunxi: Deal with the MBUS DMA offsets in a >> central place") added a platform device notifier that sets the DMA >> offset for all of the display engine frontend and backend devices. >> >> The code applying the offset to DMA buffer physical addresses was then >> removed from the backend driver in commit 756668ba682e ("drm/sun4i: >> backend: Remove the MBUS quirks"), but the code subtracting PHYS_OFFSET >> was left in the frontend driver. >> >> As a result, the offset was applied twice in the frontend driver. This >> likely went unnoticed because it only affects specific configurations >> (scaling or certain pixel formats) where the frontend is used, on boards >> with both one of these older SoCs and more than 1 GB of DRAM. >> >> In addition, the references to PHYS_OFFSET prevent compiling the driver >> on architectures where PHYS_OFFSET is not defined. >> >> Fixes: b4bdc4fbf8d0 ("soc: sunxi: Deal with the MBUS DMA offsets in a central > place") >> Signed-off-by: Samuel Holland <samuel@sholland.org> > > Good catch! Actually, people complained about non-working display on > Cubietruck IIRC, which has 2 GB of RAM. > > Did you test this on HW? The only DE1 board I have is an A33 tablet with 512 MB of DRAM. So while I boot-tested the patch, I am not able to verify if it has any real effect. The reason for sending this is that folks ran in to compilation errors on RISC-V. And I was surprised to still see references to PHYS_OFFSET, since sunxi_mbus.c includes the frontend compatibles. Regards, Samuel > Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > Best regards, > Jernej
diff --git a/drivers/gpu/drm/sun4i/sun4i_frontend.c b/drivers/gpu/drm/sun4i/sun4i_frontend.c index 56ae38389db0..462fae73eae9 100644 --- a/drivers/gpu/drm/sun4i/sun4i_frontend.c +++ b/drivers/gpu/drm/sun4i/sun4i_frontend.c @@ -222,13 +222,11 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend, /* Set the physical address of the buffer in memory */ paddr = drm_fb_cma_get_gem_addr(fb, state, 0); - paddr -= PHYS_OFFSET; DRM_DEBUG_DRIVER("Setting buffer #0 address to %pad\n", &paddr); regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR0_REG, paddr); if (fb->format->num_planes > 1) { paddr = drm_fb_cma_get_gem_addr(fb, state, swap ? 2 : 1); - paddr -= PHYS_OFFSET; DRM_DEBUG_DRIVER("Setting buffer #1 address to %pad\n", &paddr); regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR1_REG, paddr); @@ -236,7 +234,6 @@ void sun4i_frontend_update_buffer(struct sun4i_frontend *frontend, if (fb->format->num_planes > 2) { paddr = drm_fb_cma_get_gem_addr(fb, state, swap ? 1 : 2); - paddr -= PHYS_OFFSET; DRM_DEBUG_DRIVER("Setting buffer #2 address to %pad\n", &paddr); regmap_write(frontend->regs, SUN4I_FRONTEND_BUF_ADDR2_REG, paddr);
commit b4bdc4fbf8d0 ("soc: sunxi: Deal with the MBUS DMA offsets in a central place") added a platform device notifier that sets the DMA offset for all of the display engine frontend and backend devices. The code applying the offset to DMA buffer physical addresses was then removed from the backend driver in commit 756668ba682e ("drm/sun4i: backend: Remove the MBUS quirks"), but the code subtracting PHYS_OFFSET was left in the frontend driver. As a result, the offset was applied twice in the frontend driver. This likely went unnoticed because it only affects specific configurations (scaling or certain pixel formats) where the frontend is used, on boards with both one of these older SoCs and more than 1 GB of DRAM. In addition, the references to PHYS_OFFSET prevent compiling the driver on architectures where PHYS_OFFSET is not defined. Fixes: b4bdc4fbf8d0 ("soc: sunxi: Deal with the MBUS DMA offsets in a central place") Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/gpu/drm/sun4i/sun4i_frontend.c | 3 --- 1 file changed, 3 deletions(-)