Hi Laurent,
On Tue, Apr 17, 2012 at 15:32:55, Laurent Pinchart wrote:
> Hi Manjunath,
>
> Thanks for the patch.
>
> 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.
> >
> > Signed-off-by: Manjunath Hadli <manjunath.hadli@xxxxxx>
> > ---
> > drivers/media/video/davinci/vpif_display.c | 65 ++++++++++++++++++++++++-
> > drivers/media/video/davinci/vpif_display.h | 1 +
> > 2 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/video/davinci/vpif_display.c
> > b/drivers/media/video/davinci/vpif_display.c index 355ad5c..27bc03d
> > 100644
> > --- a/drivers/media/video/davinci/vpif_display.c
> > +++ b/drivers/media/video/davinci/vpif_display.c
> > @@ -57,18 +57,24 @@ static u32 ch2_numbuffers = 3; static u32
> > ch3_numbuffers = 3; static u32 ch2_bufsize = 1920 * 1080 * 2; static
> > u32 ch3_bufsize = 720 * 576 * 2;
> > +static u32 cont_bufoffset;
> > +static u32 cont_bufsize;
> >
> > module_param(debug, int, 0644);
> > module_param(ch2_numbuffers, uint, S_IRUGO);
> > module_param(ch3_numbuffers, uint, S_IRUGO);
> > module_param(ch2_bufsize, uint, S_IRUGO); module_param(ch3_bufsize,
> > uint, S_IRUGO);
> > +module_param(cont_bufoffset, uint, S_IRUGO);
> > +module_param(cont_bufsize, uint, S_IRUGO);
> >
> > MODULE_PARM_DESC(debug, "Debug level 0-1");
> > MODULE_PARM_DESC(ch2_numbuffers, "Channel2 buffer count (default:3)");
> > MODULE_PARM_DESC(ch3_numbuffers, "Channel3 buffer count (default:3)");
> > MODULE_PARM_DESC(ch2_bufsize, "Channel2 buffer size (default:1920 x
> > 1080 x 2)"); MODULE_PARM_DESC(ch3_bufsize, "Channel3 buffer size
> > (default:720 x
> > 576 x 2)"); +MODULE_PARM_DESC(cont_bufoffset, "Display offset(default
> > 0)");
> > +MODULE_PARM_DESC(cont_bufsize, "Display buffer size(default 0)");
> >
> > static struct vpif_config_params config_params = {
> > .min_numbuffers = 3,
> > @@ -187,6 +193,24 @@ static int vpif_buffer_setup(struct
> > videobuf_queue *q, unsigned int *count, return 0;
> >
> > *size = config_params.channel_bufsize[ch->channel_id];
> > +
> > + /*
> > + * Checking if the buffer size exceeds the available buffer
> > + * ycmux_mode = 0 means 1 channel mode HD and
> > + * ycmux_mode = 1 means 2 channels mode SD
> > + */
> > + if (ch->vpifparams.std_info.ycmux_mode == 0) {
> > + if (config_params.video_limit[ch->channel_id])
> > + while (*size * *count > (config_params.video_limit[0]
> > + + config_params.video_limit[1]))
> > + (*count)--;
> > + } else {
> > + if (config_params.video_limit[ch->channel_id])
> > + while (*size * *count >
> > + config_params.video_limit[ch->channel_id])
> > + (*count)--;
> > + }
> > +
> > if (*count < config_params.min_numbuffers)
> > *count = config_params.min_numbuffers;
> >
> > @@ -830,7 +854,7 @@ static int vpif_reqbufs(struct file *file, void
> > *priv,
> >
> > common = &ch->common[index];
> >
> > - if (common->fmt.type != reqbuf->type)
> > + if (common->fmt.type != reqbuf->type || !vpif_dev)
> > return -EINVAL;
> >
> > if (0 != common->io_usrs)
> > @@ -847,7 +871,7 @@ static int vpif_reqbufs(struct file *file, void
> > *priv,
> >
> > /* Initialize videobuf queue as per the buffer type */
> > videobuf_queue_dma_contig_init(&common->buffer_queue,
> > - &video_qops, NULL,
> > + &video_qops, vpif_dev,
> > &common->irqlock,
> > reqbuf->type, field,
> > sizeof(struct videobuf_buffer), fh, @@ -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. Here the size is taken as part of module parameters, which
If I am right, cannot be moved to board file.
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
> > a/drivers/media/video/davinci/vpif_display.h
> > b/drivers/media/video/davinci/vpif_display.h index dd4887c..8a311f1
> > 100644
> > --- a/drivers/media/video/davinci/vpif_display.h
> > +++ b/drivers/media/video/davinci/vpif_display.h
> > @@ -158,6 +158,7 @@ struct vpif_config_params {
> > u32 min_bufsize[VPIF_DISPLAY_NUM_CHANNELS];
> > u32 channel_bufsize[VPIF_DISPLAY_NUM_CHANNELS];
> > u8 numbuffers[VPIF_DISPLAY_NUM_CHANNELS];
> > + u32 video_limit[VPIF_DISPLAY_NUM_CHANNELS];
> > u8 min_numbuffers;
> > };
>
> --
> 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
