LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Arvind Yadav <arvind.yadav.cs@gmail.com>,
	mjpeg-users@lists.sourceforge.net, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: zoran: move to dma-mapping interface
Date: Tue, 24 Apr 2018 23:15:37 -0700	[thread overview]
Message-ID: <20180425061537.GA23383@infradead.org> (raw)
In-Reply-To: <20180424204158.2764095-1-arnd@arndb.de>

On Tue, Apr 24, 2018 at 10:40:45PM +0200, Arnd Bergmann wrote:
> No drivers should use virt_to_bus() any more. This converts
> one of the few remaining ones to the DMA mapping interface.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/pci/zoran/Kconfig        |  2 +-
>  drivers/media/pci/zoran/zoran.h        | 10 +++++--
>  drivers/media/pci/zoran/zoran_card.c   | 10 +++++--
>  drivers/media/pci/zoran/zoran_device.c | 16 +++++-----
>  drivers/media/pci/zoran/zoran_driver.c | 54 +++++++++++++++++++++++++---------
>  5 files changed, 63 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig
> index 39ec35bd21a5..26f40e124a32 100644
> --- a/drivers/media/pci/zoran/Kconfig
> +++ b/drivers/media/pci/zoran/Kconfig
> @@ -1,6 +1,6 @@
>  config VIDEO_ZORAN
>  	tristate "Zoran ZR36057/36067 Video For Linux"
> -	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS
> +	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
>  	depends on !ALPHA
>  	help
>  	  Say Y for support for MJPEG capture cards based on the Zoran
> diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
> index 9bb3c21aa275..9ff3a9acb60a 100644
> --- a/drivers/media/pci/zoran/zoran.h
> +++ b/drivers/media/pci/zoran/zoran.h
> @@ -183,13 +183,14 @@ struct zoran_buffer {
>  	struct zoran_sync bs;		/* DONE: info to return to application */
>  	union {
>  		struct {
> -			__le32 *frag_tab;	/* addresses of frag table */
> -			u32 frag_tab_bus;	/* same value cached to save time in ISR */
> +			__le32 *frag_tab;	/* DMA addresses of frag table */
> +			void **frag_virt_tab;	/* virtual addresses of frag table */
> +			dma_addr_t frag_tab_dma;/* same value cached to save time in ISR */
>  		} jpg;
>  		struct {
>  			char *fbuffer;		/* virtual address of frame buffer */
>  			unsigned long fbuffer_phys;/* physical address of frame buffer */
> -			unsigned long fbuffer_bus;/* bus address of frame buffer */
> +			dma_addr_t fbuffer_dma;/* bus address of frame buffer */
>  		} v4l;
>  	};
>  };
> @@ -221,6 +222,7 @@ struct zoran_fh {
>  
>  	struct zoran_overlay_settings overlay_settings;
>  	u32 *overlay_mask;			/* overlay mask */
> +	dma_addr_t overlay_mask_dma;
>  	enum zoran_lock_activity overlay_active;/* feature currently in use? */
>  
>  	struct zoran_buffer_col buffers;	/* buffers' info */
> @@ -307,6 +309,7 @@ struct zoran {
>  
>  	struct zoran_overlay_settings overlay_settings;
>  	u32 *overlay_mask;	/* overlay mask */
> +	dma_addr_t overlay_mask_dma;
>  	enum zoran_lock_activity overlay_active;	/* feature currently in use? */
>  
>  	wait_queue_head_t v4l_capq;
> @@ -346,6 +349,7 @@ struct zoran {
>  
>  	/* zr36057's code buffer table */
>  	__le32 *stat_com;		/* stat_com[i] is indexed by dma_head/tail & BUZ_MASK_STAT_COM */
> +	dma_addr_t stat_com_dma;
>  
>  	/* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
>  	int jpg_pend[BUZ_MAX_FRAME];
> diff --git a/drivers/media/pci/zoran/zoran_card.c b/drivers/media/pci/zoran/zoran_card.c
> index a6b9ebd20263..dabd8bf77472 100644
> --- a/drivers/media/pci/zoran/zoran_card.c
> +++ b/drivers/media/pci/zoran/zoran_card.c
> @@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
>  	/* User must explicitly set a window */
>  	zr->overlay_settings.is_set = 0;
>  	zr->overlay_mask = NULL;
> +	zr->overlay_mask_dma = 0;

I don't think this zeroing is required, and given that 0 is a valid
dma address on some platforms is also is rather confusing.:w

>  
>  		mask_line_size = (BUZ_MAX_WIDTH + 31) / 32;
> -		reg = virt_to_bus(zr->overlay_mask);
> +		reg = zr->overlay_mask_dma;
>  		btwrite(reg, ZR36057_MMTR);
> -		reg = virt_to_bus(zr->overlay_mask + mask_line_size);
> +		reg = zr->overlay_mask_dma + mask_line_size;

I think this would be easier to read if the reg assignments got
removed, e.g.

	btwrite(zr->overlay_mask_dma, ZR36057_MMTR);
	btwrite(zr->overlay_mask_dma + mask_line_size, ZR36057_MMBR);

Same in a few other places.

> +		virt_tab = (void *)get_zeroed_page(GFP_KERNEL);
> +		if (!mem || !virt_tab) {
>  			dprintk(1,
>  				KERN_ERR
>  				"%s: %s - get_zeroed_page (frag_tab) failed for buffer %d\n",
>  				ZR_DEVNAME(zr), __func__, i);
> +			kfree(mem);
> +			kfree(virt_tab);
>  			jpg_fbuffer_free(fh);
>  			return -ENOBUFS;
>  		}
>  		fh->buffers.buffer[i].jpg.frag_tab = (__le32 *)mem;
> -		fh->buffers.buffer[i].jpg.frag_tab_bus = virt_to_bus(mem);
> +		fh->buffers.buffer[i].jpg.frag_virt_tab = virt_tab;

>From a quick look it seems like frag_tab should be a dma_alloc_coherent
allocation, or you would need a lot of cache sync operations.

That probably also means it can use dma_mmap_coherent instead of the
handcrafted remap_pfn_range loop and the PageReserved abuse.

  reply	other threads:[~2018-04-25  6:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-24 20:40 Arnd Bergmann
2018-04-25  6:15 ` Christoph Hellwig [this message]
2018-04-25  7:08   ` Arnd Bergmann
2018-04-25  7:21     ` Christoph Hellwig
2018-04-25 11:15       ` Arnd Bergmann
2018-04-25 15:26         ` Christoph Hellwig
2018-04-25 15:58           ` Arnd Bergmann
2018-04-25 17:22             ` Mauro Carvalho Chehab
2018-04-25 17:43               ` Trent Piepho
2018-05-07  9:05               ` Hans Verkuil
2018-05-07 21:17                 ` Arnd Bergmann
2018-04-25 17:27       ` [Mjpeg-users] " Bernhard Praschinger
2018-04-26 17:49 ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180425061537.GA23383@infradead.org \
    --to=hch@infradead.org \
    --cc=arnd@arndb.de \
    --cc=arvind.yadav.cs@gmail.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mjpeg-users@lists.sourceforge.net \
    --subject='Re: [PATCH] media: zoran: move to dma-mapping interface' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).