RE: [PATCH v2 05/13] davinci: vpif display: declare contiguous region of memory handled by dma_alloc_coherent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On Mon, May 14, 2012 at 13:16:27, Laurent Pinchart wrote:
> Hi Manjunath,
> 
> On Friday 11 May 2012 05:30:43 Hadli, Manjunath wrote:
> > On Tue, Apr 17, 2012 at 15:32:55, Laurent Pinchart wrote:
> > > On Tuesday 17 April 2012 14:23:03 Manjunath Hadli wrote:
> > > > add support to declare contiguous region of memory to be handled when
> > > > requested by dma_alloc_coherent call. The user can specify the size of
> > > > the buffers with an offset from the kernel image using cont_bufsize
> > > > and cont_bufoffset module parameters respectively.
> 
> [snip]
> 
> > > > @@ -1686,12 +1710,14 @@ static __init int vpif_probe(struct
> > > > platform_device *pdev) struct vpif_subdev_info *subdevdata;
> > > > 
> > > >  	struct vpif_display_config *config;
> > > >  	int i, j = 0, k, q, m, err = 0;
> > > > 
> > > > +	unsigned long phys_end_kernel;
> > > > 
> > > >  	struct i2c_adapter *i2c_adap;
> > > >  	struct common_obj *common;
> > > >  	struct channel_obj *ch;
> > > >  	struct video_device *vfd;
> > > >  	struct resource *res;
> > > >  	int subdev_count;
> > > > 
> > > > +	size_t size;
> > > > 
> > > >  	vpif_dev = &pdev->dev;
> > > > 
> > > > @@ -1749,6 +1775,41 @@ static __init int vpif_probe(struct
> > > > platform_device
> > > > *pdev) ch->video_dev = vfd;
> > > > 
> > > >  	}
> > > > 
> > > > +	/* Initialising the memory from the input arguments file for
> > > > +	 * contiguous memory buffers and avoid defragmentation
> > > > +	 */
> > > > +	if (cont_bufsize) {
> > > > +		/* attempt to determine the end of Linux kernel memory */
> > > > +		phys_end_kernel = virt_to_phys((void *)PAGE_OFFSET) +
> > > > +			(num_physpages << PAGE_SHIFT);
> > > > +		phys_end_kernel += cont_bufoffset;
> > > > +		size = cont_bufsize;
> > > > +
> > > > +		err = dma_declare_coherent_memory(&pdev->dev, phys_end_kernel,
> > > > +				phys_end_kernel, size,
> > > > +				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
> > > 
> > > This is a pretty dangerous hack. You should compute the memory address and
> > > size in board code, and pass it to the driver through a device resource
> > > (don't forget to call request_mem_region on the resource). I think the
> > > dma_declare_coherent_memory() call should be moved to board code as well.
> >
> > I don't think it is a hack. Since the device does not support scatter gather
> > MMU, we need to make sure that the requirement of memory scucceeds for the
> > Driver buffers.
> 
> If two drivers attempt to do the same you're screwed. That's why this should 
> be moved to board code, where memory reservation for all devices that need it 
> can be coordinated. You should call dma_declare_coherent_memory() there, not 
> in the VPIF driver.
> 
  Ok I'll move dma_declare_coherent_memory() to board file.

> > Here the size is taken as part of module parameters, which If I am right,
> > cannot be moved to board file.
> 
> Depending on how early you need the information in board code you can use 
> early_param(), __setup() or normal module parameter macros in the board code.
> 
Ok I'll fix this.

Thx,
--Manju

> > > > +		if (!err) {
> > > > +			dev_err(&pdev->dev, "Unable to declare MMAP memory.\n");
> > > > +			err = -ENOMEM;
> > > > +			goto probe_out;
> > > > +		}
> > > > +
> > > > +		/* The resources are divided into two equal memory and when
> > > > +		 * we have HD output we can add them together
> > > > +		 */
> > > > +		for (j = 0; j < VPIF_DISPLAY_MAX_DEVICES; j++) {
> > > > +			ch = vpif_obj.dev[j];
> > > > +			ch->channel_id = j;
> > > > +
> > > > +			/* only enabled if second resource exists */
> > > > +			config_params.video_limit[ch->channel_id] = 0;
> > > > +			if (cont_bufsize)
> > > > +				config_params.video_limit[ch->channel_id] =
> > > > +									size/2;
> > > > +		}
> > > > +	}
> > > > +
> > > > 
> > > >  	for (j = 0; j < VPIF_DISPLAY_MAX_DEVICES; j++) {
> > > >  	
> > > >  		ch = vpif_obj.dev[j];
> > > >  		/* Initialize field of the channel objects */ diff --git
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux