Linux-MIPS Archive on lore.kernel.org
 help / Atom feed
* fix DMA API abuse in various mips fbdev drivers
@ 2019-05-09 17:38 Christoph Hellwig
  2019-05-09 17:38 ` [PATCH 1/3] au1100fb: fix DMA API abuse Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-09 17:38 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Manuel Lauss
  Cc: linux-mips, linux-fbdev, linux-kernel

Hi all,

this series fixes up three mips-specific fbdev drivers to not poke
into the return values from the DMA memory allocators, as those
aren't guranteed to be pages backed (although on mips in practice
they are).  Two of them are also fixed up to use the proper DMA API
mmap helper.

Note that the first patch is required for pending mips DMA changes,
so if they are queued up for 5.3 we'll need a stable branch that
can be pulled into the dma-mapping or mips tree.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] au1100fb: fix DMA API abuse
  2019-05-09 17:38 fix DMA API abuse in various mips fbdev drivers Christoph Hellwig
@ 2019-05-09 17:38 ` Christoph Hellwig
  2019-05-09 17:38 ` [PATCH 2/3] au1200fb: " Christoph Hellwig
  2019-05-09 17:38 ` [PATCH 3/3] jz4740_fb: " Christoph Hellwig
  2 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-09 17:38 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Manuel Lauss
  Cc: linux-mips, linux-fbdev, linux-kernel

Virtual addresses return from dma(m)_alloc_coherent are opaque in what
backs then, and drivers must not poke into them.  Switch the driver
to use the generic DMA API mmap helper to avoid these problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/video/fbdev/au1100fb.c | 24 ++++--------------------
 drivers/video/fbdev/au1100fb.h |  1 +
 2 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index 0adf0683cf08..99941ae1f3a1 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -340,14 +340,12 @@ int au1100fb_fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi)
  */
 int au1100fb_fb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
 {
-	struct au1100fb_device *fbdev;
-
-	fbdev = to_au1100fb_device(fbi);
+	struct au1100fb_device *fbdev = to_au1100fb_device(fbi);
 
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
 	pgprot_val(vma->vm_page_prot) |= (6 << 9); //CCA=6
 
-	return vm_iomap_memory(vma, fbdev->fb_phys, fbdev->fb_len);
+	return dma_mmap_coherent(fbdev->dev, vma, fbdev->fb_mem, fbdev->fb_phys,
+			fbdev->fb_len);
 }
 
 static struct fb_ops au1100fb_ops =
@@ -412,7 +410,6 @@ static int au1100fb_drv_probe(struct platform_device *dev)
 {
 	struct au1100fb_device *fbdev;
 	struct resource *regs_res;
-	unsigned long page;
 	struct clk *c;
 
 	/* Allocate new device private */
@@ -424,6 +421,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
 		goto failed;
 
 	platform_set_drvdata(dev, (void *)fbdev);
+	fbdev->dev = &dev->dev;
 
 	/* Allocate region for our registers and map them */
 	regs_res = platform_get_resource(dev, IORESOURCE_MEM, 0);
@@ -472,20 +470,6 @@ static int au1100fb_drv_probe(struct platform_device *dev)
 	au1100fb_fix.smem_start = fbdev->fb_phys;
 	au1100fb_fix.smem_len = fbdev->fb_len;
 
-	/*
-	 * Set page reserved so that mmap will work. This is necessary
-	 * since we'll be remapping normal memory.
-	 */
-	for (page = (unsigned long)fbdev->fb_mem;
-	     page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len);
-	     page += PAGE_SIZE) {
-#ifdef CONFIG_DMA_NONCOHERENT
-		SetPageReserved(virt_to_page(CAC_ADDR((void *)page)));
-#else
-		SetPageReserved(virt_to_page(page));
-#endif
-	}
-
 	print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
 	print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
 
diff --git a/drivers/video/fbdev/au1100fb.h b/drivers/video/fbdev/au1100fb.h
index 9af19939a9c6..e7239bceefd3 100644
--- a/drivers/video/fbdev/au1100fb.h
+++ b/drivers/video/fbdev/au1100fb.h
@@ -110,6 +110,7 @@ struct au1100fb_device {
 	dma_addr_t    		fb_phys;
 	int			panel_idx;
 	struct clk		*lcdclk;
+	struct device		*dev;
 };
 
 /********************************************************************/
-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] au1200fb: fix DMA API abuse
  2019-05-09 17:38 fix DMA API abuse in various mips fbdev drivers Christoph Hellwig
  2019-05-09 17:38 ` [PATCH 1/3] au1100fb: fix DMA API abuse Christoph Hellwig
@ 2019-05-09 17:38 ` " Christoph Hellwig
  2019-05-15 11:10   ` Manuel Lauss
  2019-05-09 17:38 ` [PATCH 3/3] jz4740_fb: " Christoph Hellwig
  2 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-09 17:38 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Manuel Lauss
  Cc: linux-mips, linux-fbdev, linux-kernel

Virtual addresses return from dma(m)_alloc_attrs are opaque in what
backs then, and drivers must not poke into them.  Similarly caching
modes are not supposed to be directly set by the driver.  Switch the
driver to use the generic DMA API mmap helper to avoid these problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/au1200fb.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 3872ccef4cb2..26caffb02b7e 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -147,6 +147,7 @@ struct au1200_lcd_iodata_t {
 struct au1200fb_device {
 	struct fb_info *fb_info;		/* FB driver info record */
 	struct au1200fb_platdata *pd;
+	struct device *dev;
 
 	int					plane;
 	unsigned char* 		fb_mem;		/* FrameBuffer memory map */
@@ -1232,10 +1233,8 @@ static int au1200fb_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct au1200fb_device *fbdev = info->par;
 
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-	pgprot_val(vma->vm_page_prot) |= _CACHE_MASK; /* CCA=7 */
-
-	return vm_iomap_memory(vma, fbdev->fb_phys, fbdev->fb_len);
+	return dma_mmap_attrs(fbdev->dev, vma, fbdev->fb_mem, fbdev->fb_phys,
+			fbdev->fb_len, DMA_ATTR_NON_CONSISTENT);
 }
 
 static void set_global(u_int cmd, struct au1200_lcd_global_regs_t *pdata)
@@ -1647,7 +1646,6 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 	struct au1200fb_device *fbdev;
 	struct au1200fb_platdata *pd;
 	struct fb_info *fbi = NULL;
-	unsigned long page;
 	int bpp, plane, ret, irq;
 
 	print_info("" DRIVER_DESC "");
@@ -1685,6 +1683,7 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 		fbdev = fbi->par;
 		fbdev->fb_info = fbi;
 		fbdev->pd = pd;
+		fbdev->dev = &dev->dev;
 
 		fbdev->plane = plane;
 
@@ -1702,16 +1701,6 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 			goto failed;
 		}
 
-		/*
-		 * Set page reserved so that mmap will work. This is necessary
-		 * since we'll be remapping normal memory.
-		 */
-		for (page = (unsigned long)fbdev->fb_phys;
-		     page < PAGE_ALIGN((unsigned long)fbdev->fb_phys +
-			     fbdev->fb_len);
-		     page += PAGE_SIZE) {
-			SetPageReserved(pfn_to_page(page >> PAGE_SHIFT)); /* LCD DMA is NOT coherent on Au1200 */
-		}
 		print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
 		print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] jz4740_fb: fix DMA API abuse
  2019-05-09 17:38 fix DMA API abuse in various mips fbdev drivers Christoph Hellwig
  2019-05-09 17:38 ` [PATCH 1/3] au1100fb: fix DMA API abuse Christoph Hellwig
  2019-05-09 17:38 ` [PATCH 2/3] au1200fb: " Christoph Hellwig
@ 2019-05-09 17:38 ` " Christoph Hellwig
  2019-06-07 12:21   ` Bartlomiej Zolnierkiewicz
  2 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-09 17:38 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Manuel Lauss
  Cc: linux-mips, linux-fbdev, linux-kernel

Virtual addresses return from dma(m)_alloc_coherent are opaque in what
backs then, and drivers must not poke into them.  Switch the driver
to use the generic DMA API mmap helper to avoid these problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/jz4740_fb.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/video/fbdev/jz4740_fb.c b/drivers/video/fbdev/jz4740_fb.c
index b57df83fdbd3..b95cdfa9e0a1 100644
--- a/drivers/video/fbdev/jz4740_fb.c
+++ b/drivers/video/fbdev/jz4740_fb.c
@@ -466,7 +466,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
 {
 	int max_videosize = 0;
 	struct fb_videomode *mode = jzfb->pdata->modes;
-	void *page;
 	int i;
 
 	for (i = 0; i < jzfb->pdata->num_modes; ++mode, ++i) {
@@ -491,12 +490,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
 	if (!jzfb->vidmem)
 		goto err_free_framedesc;
 
-	for (page = jzfb->vidmem;
-		 page < jzfb->vidmem + PAGE_ALIGN(jzfb->vidmem_size);
-		 page += PAGE_SIZE) {
-		SetPageReserved(virt_to_page(page));
-	}
-
 	jzfb->framedesc->next = jzfb->framedesc_phys;
 	jzfb->framedesc->addr = jzfb->vidmem_phys;
 	jzfb->framedesc->id = 0xdeafbead;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] au1200fb: fix DMA API abuse
  2019-05-09 17:38 ` [PATCH 2/3] au1200fb: " Christoph Hellwig
@ 2019-05-15 11:10   ` Manuel Lauss
  2019-06-07 12:14     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 7+ messages in thread
From: Manuel Lauss @ 2019-05-15 11:10 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Bartlomiej Zolnierkiewicz, Linux-MIPS, linux-fbdev, LKML

Servus Christoph,

On Thu, May 9, 2019 at 7:39 PM Christoph Hellwig <hch@lst.de> wrote:
> Virtual addresses return from dma(m)_alloc_attrs are opaque in what
> backs then, and drivers must not poke into them.  Similarly caching
> modes are not supposed to be directly set by the driver.  Switch the
> driver to use the generic DMA API mmap helper to avoid these problems.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/video/fbdev/au1200fb.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)

Runs fine on my test system.

Tested-by: Manuel Lauss <manuel.lauss@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] au1200fb: fix DMA API abuse
  2019-05-15 11:10   ` Manuel Lauss
@ 2019-06-07 12:14     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-06-07 12:14 UTC (permalink / raw)
  To: Manuel Lauss, Christoph Hellwig; +Cc: Linux-MIPS, linux-fbdev, LKML


On 5/15/19 1:10 PM, Manuel Lauss wrote:
> Servus Christoph,
> 
> On Thu, May 9, 2019 at 7:39 PM Christoph Hellwig <hch@lst.de> wrote:
>> Virtual addresses return from dma(m)_alloc_attrs are opaque in what
>> backs then, and drivers must not poke into them.  Similarly caching
>> modes are not supposed to be directly set by the driver.  Switch the
>> driver to use the generic DMA API mmap helper to avoid these problems.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>  drivers/video/fbdev/au1200fb.c | 19 ++++---------------
>>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> Runs fine on my test system.
> 
> Tested-by: Manuel Lauss <manuel.lauss@gmail.com>

Patch queued for v5.3, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] jz4740_fb: fix DMA API abuse
  2019-05-09 17:38 ` [PATCH 3/3] jz4740_fb: " Christoph Hellwig
@ 2019-06-07 12:21   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-06-07 12:21 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Manuel Lauss, linux-mips, linux-fbdev, linux-kernel


On 5/9/19 7:38 PM, Christoph Hellwig wrote:
> Virtual addresses return from dma(m)_alloc_coherent are opaque in what
> backs then, and drivers must not poke into them.  Switch the driver
> to use the generic DMA API mmap helper to avoid these problems.
Change itself looks fine but the patch description doesn't match what
the patch is actually doing (there is no conversion to DMA API helper
because the driver is already using it). Please correct it. Thank you.

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/video/fbdev/jz4740_fb.c | 7 -------
>  1 file changed, 7 deletions(-)
> diff --git a/drivers/video/fbdev/jz4740_fb.c b/drivers/video/fbdev/jz4740_fb.c
> index b57df83fdbd3..b95cdfa9e0a1 100644
> --- a/drivers/video/fbdev/jz4740_fb.c
> +++ b/drivers/video/fbdev/jz4740_fb.c
> @@ -466,7 +466,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
>  {
>  	int max_videosize = 0;
>  	struct fb_videomode *mode = jzfb->pdata->modes;
> -	void *page;
>  	int i;
>  
>  	for (i = 0; i < jzfb->pdata->num_modes; ++mode, ++i) {
> @@ -491,12 +490,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
>  	if (!jzfb->vidmem)
>  		goto err_free_framedesc;
>  
> -	for (page = jzfb->vidmem;
> -		 page < jzfb->vidmem + PAGE_ALIGN(jzfb->vidmem_size);
> -		 page += PAGE_SIZE) {
> -		SetPageReserved(virt_to_page(page));
> -	}
> -
>  	jzfb->framedesc->next = jzfb->framedesc_phys;
>  	jzfb->framedesc->addr = jzfb->vidmem_phys;
>  	jzfb->framedesc->id = 0xdeafbead;

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-09 17:38 fix DMA API abuse in various mips fbdev drivers Christoph Hellwig
2019-05-09 17:38 ` [PATCH 1/3] au1100fb: fix DMA API abuse Christoph Hellwig
2019-05-09 17:38 ` [PATCH 2/3] au1200fb: " Christoph Hellwig
2019-05-15 11:10   ` Manuel Lauss
2019-06-07 12:14     ` Bartlomiej Zolnierkiewicz
2019-05-09 17:38 ` [PATCH 3/3] jz4740_fb: " Christoph Hellwig
2019-06-07 12:21   ` Bartlomiej Zolnierkiewicz

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org linux-mips@archiver.kernel.org
	public-inbox-index linux-mips


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/ public-inbox