[PATCH] video: Fix broken bcm2835 fb driver

[PATCH] video: Fix broken bcm2835 fb driver

[Daniel Brát]

The bcm2835 framebuffer driver was broken, because the address of video
buffer allocated for us by the GPU and returned through mailbox was
used without converting it back to ARM address space. That unconverted
address was also in the range of peripheral addresses, which caused
other issues later on due to it being filled with garbage data.
The offset by which to convert the address back can vary by device,
so the value is read from devicetree 'dma-ranges' for somewhat portable
operation.
This fix was tested on Raspberry PI B+ and Raspberry PI 3B+.

Signed-off-by: Daniel Brát <danek.brat@gmail.com>
---
 drivers/video/bcm2835.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index d808bc5c9..2ebe912d0 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -14,6 +14,7 @@
 #include <malloc.h>
 #include <xfuncs.h>
 
+#include <of_address.h>
 #include <mach/mbox.h>
 
 struct bcm2835fb_info {
@@ -58,9 +59,24 @@ static int bcm2835fb_probe(struct device_d *dev)
 	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_query, msg_query);
 	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_setup, msg_setup);
 	struct bcm2835fb_info *info;
+	struct device_node *soc;
 	u32 w, h;
+	u64 dma_addr, cpu_addr, _region_size;
+	phys_addr_t buffer_addr;
 	int ret;
 
+	soc = of_find_node_by_path("/soc");
+	if (!soc) {
+		dev_err(dev, "could not find required of node /soc\n");
+		return -ENODEV;
+	}
+
+	ret = of_dma_get_range(soc, &dma_addr, &cpu_addr, &_region_size);
+	if (ret) {
+		dev_err(dev, "of node /soc has no dma-ranges\n");
+		return ret;
+	}
+
 	BCM2835_MBOX_INIT_HDR(msg_query);
 	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_query->physical_w_h,
 					GET_PHYSICAL_W_H);
@@ -99,10 +115,11 @@ static int bcm2835fb_probe(struct device_d *dev)
 		return ret;
 	}
 
+	buffer_addr = (msg_setup->allocate_buffer.body.resp.fb_address & ~dma_addr) + cpu_addr;
+
 	info = xzalloc(sizeof *info);
 	info->fbi.fbops = &bcm2835fb_ops;
-	info->fbi.screen_base =
-	   (void *)msg_setup->allocate_buffer.body.resp.fb_address;
+	info->fbi.screen_base = phys_to_virt(buffer_addr);
 	info->fbi.xres = msg_setup->physical_w_h.body.resp.width;
 	info->fbi.yres = msg_setup->physical_w_h.body.resp.height;
 	info->fbi.bits_per_pixel = 16;
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] video: Fix broken bcm2835 fb driver

[Roland Hieber]

On Tue, Sep 07, 2021 at 09:29:15AM +0200, Daniel Brát wrote:
> The bcm2835 framebuffer driver was broken, because the address of video
> buffer allocated for us by the GPU and returned through mailbox was
> used without converting it back to ARM address space. That unconverted
> address was also in the range of peripheral addresses, which caused
> other issues later on due to it being filled with garbage data.
> The offset by which to convert the address back can vary by device,
> so the value is read from devicetree 'dma-ranges' for somewhat portable
> operation.
> This fix was tested on Raspberry PI B+ and Raspberry PI 3B+.
> 
> Signed-off-by: Daniel Brát <danek.brat@gmail.com>
> ---
>  drivers/video/bcm2835.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
> index d808bc5c9..2ebe912d0 100644
> --- a/drivers/video/bcm2835.c
> +++ b/drivers/video/bcm2835.c
> @@ -14,6 +14,7 @@
>  #include <malloc.h>
>  #include <xfuncs.h>
>  
> +#include <of_address.h>
>  #include <mach/mbox.h>
>  
>  struct bcm2835fb_info {
> @@ -58,9 +59,24 @@ static int bcm2835fb_probe(struct device_d *dev)
>  	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_query, msg_query);
>  	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_setup, msg_setup);
>  	struct bcm2835fb_info *info;
> +	struct device_node *soc;
>  	u32 w, h;
> +	u64 dma_addr, cpu_addr, _region_size;
> +	phys_addr_t buffer_addr;
>  	int ret;
>  
> +	soc = of_find_node_by_path("/soc");
> +	if (!soc) {
> +		dev_err(dev, "could not find required of node /soc\n");

Nit: I'd use "OF" in capitals to ease human parsing and prevent
confusing with the English preposition.

> +		return -ENODEV;
> +	}
> +
> +	ret = of_dma_get_range(soc, &dma_addr, &cpu_addr, &_region_size);
> +	if (ret) {
> +		dev_err(dev, "of node /soc has no dma-ranges\n");

Here too.

 - Roland

> +		return ret;
> +	}
> +
>  	BCM2835_MBOX_INIT_HDR(msg_query);
>  	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_query->physical_w_h,
>  					GET_PHYSICAL_W_H);
> @@ -99,10 +115,11 @@ static int bcm2835fb_probe(struct device_d *dev)
>  		return ret;
>  	}
>  
> +	buffer_addr = (msg_setup->allocate_buffer.body.resp.fb_address & ~dma_addr) + cpu_addr;
> +
>  	info = xzalloc(sizeof *info);
>  	info->fbi.fbops = &bcm2835fb_ops;
> -	info->fbi.screen_base =
> -	   (void *)msg_setup->allocate_buffer.body.resp.fb_address;
> +	info->fbi.screen_base = phys_to_virt(buffer_addr);
>  	info->fbi.xres = msg_setup->physical_w_h.body.resp.width;
>  	info->fbi.yres = msg_setup->physical_w_h.body.resp.height;
>  	info->fbi.bits_per_pixel = 16;
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] video: Fix broken bcm2835 fb driver

[Sascha Hauer]

On Tue, Sep 07, 2021 at 09:29:15AM +0200, Daniel Brát wrote:
> The bcm2835 framebuffer driver was broken, because the address of video
> buffer allocated for us by the GPU and returned through mailbox was
> used without converting it back to ARM address space. That unconverted
> address was also in the range of peripheral addresses, which caused
> other issues later on due to it being filled with garbage data.
> The offset by which to convert the address back can vary by device,
> so the value is read from devicetree 'dma-ranges' for somewhat portable
> operation.
> This fix was tested on Raspberry PI B+ and Raspberry PI 3B+.
> 
> Signed-off-by: Daniel Brát <danek.brat@gmail.com>
> ---
>  drivers/video/bcm2835.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)

Applied with Rolands remarks, thanks

Sascha

> 
> diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
> index d808bc5c9..2ebe912d0 100644
> --- a/drivers/video/bcm2835.c
> +++ b/drivers/video/bcm2835.c
> @@ -14,6 +14,7 @@
>  #include <malloc.h>
>  #include <xfuncs.h>
>  
> +#include <of_address.h>
>  #include <mach/mbox.h>
>  
>  struct bcm2835fb_info {
> @@ -58,9 +59,24 @@ static int bcm2835fb_probe(struct device_d *dev)
>  	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_query, msg_query);
>  	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_setup, msg_setup);
>  	struct bcm2835fb_info *info;
> +	struct device_node *soc;
>  	u32 w, h;
> +	u64 dma_addr, cpu_addr, _region_size;
> +	phys_addr_t buffer_addr;
>  	int ret;
>  
> +	soc = of_find_node_by_path("/soc");
> +	if (!soc) {
> +		dev_err(dev, "could not find required of node /soc\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = of_dma_get_range(soc, &dma_addr, &cpu_addr, &_region_size);
> +	if (ret) {
> +		dev_err(dev, "of node /soc has no dma-ranges\n");
> +		return ret;
> +	}
> +
>  	BCM2835_MBOX_INIT_HDR(msg_query);
>  	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_query->physical_w_h,
>  					GET_PHYSICAL_W_H);
> @@ -99,10 +115,11 @@ static int bcm2835fb_probe(struct device_d *dev)
>  		return ret;
>  	}
>  
> +	buffer_addr = (msg_setup->allocate_buffer.body.resp.fb_address & ~dma_addr) + cpu_addr;
> +
>  	info = xzalloc(sizeof *info);
>  	info->fbi.fbops = &bcm2835fb_ops;
> -	info->fbi.screen_base =
> -	   (void *)msg_setup->allocate_buffer.body.resp.fb_address;
> +	info->fbi.screen_base = phys_to_virt(buffer_addr);
>  	info->fbi.xres = msg_setup->physical_w_h.body.resp.width;
>  	info->fbi.yres = msg_setup->physical_w_h.body.resp.height;
>  	info->fbi.bits_per_pixel = 16;
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox