Hello,
On 04/13/2013 01:52 AM, Scott Jiang wrote:
This is a bridge driver for blackfin diplay device.
It can work with ppi or eppi interface. DV timings
are supported.
Signed-off-by: Scott Jiang<scott.jiang.linux@xxxxxxxxx>
---
drivers/media/platform/blackfin/Kconfig | 15 +-
drivers/media/platform/blackfin/Makefile | 1 +
drivers/media/platform/blackfin/bfin_display.c | 1151 ++++++++++++++++++++++++
include/media/blackfin/bfin_display.h | 38 +
4 files changed, 1203 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/platform/blackfin/bfin_display.c
create mode 100644 include/media/blackfin/bfin_display.h
diff --git a/drivers/media/platform/blackfin/Kconfig b/drivers/media/platform/blackfin/Kconfig
index cc23997..8a8fd75 100644
--- a/drivers/media/platform/blackfin/Kconfig
+++ b/drivers/media/platform/blackfin/Kconfig
@@ -9,7 +9,18 @@ config VIDEO_BLACKFIN_CAPTURE
To compile this driver as a module, choose M here: the
module will be called bfin_capture.
+config VIDEO_BLACKFIN_DISPLAY
+ tristate "Blackfin Video Display Driver"
+ depends on VIDEO_V4L2&& BLACKFIN&& I2C
+ select VIDEOBUF2_DMA_CONTIG
+ help
+ V4L2 bridge driver for Blackfin video display device.
Shouldn't it just be "V4L2 output driver", why are you calling it "bridge" ?
+ Choose PPI or EPPI as its interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bfin_display.
+
config VIDEO_BLACKFIN_PPI
tristate
- depends on VIDEO_BLACKFIN_CAPTURE
- default VIDEO_BLACKFIN_CAPTURE
+ depends on VIDEO_BLACKFIN_CAPTURE || VIDEO_BLACKFIN_DISPLAY
+ default VIDEO_BLACKFIN_CAPTURE || VIDEO_BLACKFIN_DISPLAY
diff --git a/drivers/media/platform/blackfin/Makefile b/drivers/media/platform/blackfin/Makefile
index 30421bc..015c8f0 100644
--- a/drivers/media/platform/blackfin/Makefile
+++ b/drivers/media/platform/blackfin/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_VIDEO_BLACKFIN_CAPTURE) += bfin_capture.o
+obj-$(CONFIG_VIDEO_BLACKFIN_DISPLAY) += bfin_display.o
obj-$(CONFIG_VIDEO_BLACKFIN_PPI) += ppi.o
diff --git a/drivers/media/platform/blackfin/bfin_display.c b/drivers/media/platform/blackfin/bfin_display.c
new file mode 100644
index 0000000..d971d7b
--- /dev/null
+++ b/drivers/media/platform/blackfin/bfin_display.c
@@ -0,0 +1,1151 @@
+/*
+ * Analog Devices video display driver
Sounds a bit too generic.
+ *
+ * Copyright (c) 2011 Analog Devices Inc.
2011 - 2013 ?
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include<linux/completion.h>
+#include<linux/delay.h>
+#include<linux/errno.h>
+#include<linux/fs.h>
+#include<linux/i2c.h>
+#include<linux/init.h>
+#include<linux/interrupt.h>
+#include<linux/io.h>
+#include<linux/mm.h>
+#include<linux/module.h>
+#include<linux/platform_device.h>
+#include<linux/slab.h>
+#include<linux/time.h>
+#include<linux/types.h>
+
+#include<media/v4l2-chip-ident.h>
+#include<media/v4l2-common.h>
+#include<media/v4l2-ctrls.h>
+#include<media/v4l2-device.h>
+#include<media/v4l2-ioctl.h>
+#include<media/videobuf2-dma-contig.h>
+
+#include<asm/dma.h>
+
+#include<media/blackfin/bfin_display.h>
+#include<media/blackfin/ppi.h>
+
+#define DISPLAY_DRV_NAME "bfin_display"
+#define DISP_MIN_NUM_BUF 2
+
+struct disp_format {
+ char *desc;
+ u32 pixelformat;
+ enum v4l2_mbus_pixelcode mbus_code;
+ int bpp; /* bits per pixel */
+ int dlen; /* data length for ppi in bits */
+};
+
+struct disp_buffer {
+ struct vb2_buffer vb;
+ struct list_head list;
+};
+
+struct disp_device {
+ /* capture device instance */
Shouldn't it be "output device..." ?
+ struct v4l2_device v4l2_dev;
+ /* v4l2 control handler */
+ struct v4l2_ctrl_handler ctrl_handler;
This handler seems to be unused, I couldn't find any code adding controls
to it. Any initialization of this handler is a dead code now. You probably
want to move that bits to a patch actually adding any controls.
+ /* device node data */
+ struct video_device *video_dev;
+ /* sub device instance */
+ struct v4l2_subdev *sd;
+ /* capture config */
+ struct bfin_display_config *cfg;
+ /* ppi interface */
+ struct ppi_if *ppi;
+ /* current output */
+ unsigned int cur_output;
+ /* current selected standard */
+ v4l2_std_id std;
+ /* current selected dv_timings */
+ struct v4l2_dv_timings dv_timings;
+ /* used to store pixel format */
+ struct v4l2_pix_format fmt;
+ /* bits per pixel*/
+ int bpp;
+ /* data length for ppi in bits */
+ int dlen;
+ /* used to store encoder supported format */
+ struct disp_format *enc_formats;
+ /* number of encoder formats array */
+ int num_enc_formats;
+ /* pointing to current video buffer */
+ struct disp_buffer *cur_frm;
+ /* buffer queue used in videobuf2 */
+ struct vb2_queue buffer_queue;
+ /* allocator-specific contexts for each plane */
+ struct vb2_alloc_ctx *alloc_ctx;
+ /* queue of filled frames */
+ struct list_head dma_queue;
+ /* used in videobuf2 callback */
+ spinlock_t lock;
+ /* used to access display device */
+ struct mutex mutex;
+};
+
+struct disp_fh {
+ struct v4l2_fh fh;
+ /* indicates whether this file handle is doing IO */
+ bool io_allowed;
+};
This structure should not be needed when you use the vb2 helpers. Please
see
below for more details.
+static const struct disp_format disp_formats[] = {
+ {
+ .desc = "YCbCr 4:2:2 Interleaved UYVY 8bits",
+ .pixelformat = V4L2_PIX_FMT_UYVY,
+ .mbus_code = V4L2_MBUS_FMT_UYVY8_2X8,
+ .bpp = 16,
+ .dlen = 8,
+ },
+ {
+ .desc = "YCbCr 4:2:2 Interleaved YUYV 8bits",
+ .pixelformat = V4L2_PIX_FMT_YUYV,
+ .mbus_code = V4L2_MBUS_FMT_YUYV8_2X8,
+ .bpp = 16,
+ .dlen = 8,
+ },
+ {
+ .desc = "YCbCr 4:2:2 Interleaved UYVY 16bits",
+ .pixelformat = V4L2_PIX_FMT_UYVY,
+ .mbus_code = V4L2_MBUS_FMT_UYVY8_1X16,
+ .bpp = 16,
+ .dlen = 16,
+ },
+ {
+ .desc = "RGB 565",
+ .pixelformat = V4L2_PIX_FMT_RGB565,
+ .mbus_code = V4L2_MBUS_FMT_RGB565_2X8_LE,
+ .bpp = 16,
+ .dlen = 8,
+ },
+ {
+ .desc = "RGB 444",
+ .pixelformat = V4L2_PIX_FMT_RGB444,
+ .mbus_code = V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE,
+ .bpp = 16,
+ .dlen = 8,
+ },
+
+};
+#define DISP_MAX_FMTS ARRAY_SIZE(disp_formats)
+
+static irqreturn_t disp_isr(int irq, void *dev_id);
Couldn't the functions be reordered so this declaration can be avoided ?
+static int disp_open(struct file *file)
+{
+ struct disp_device *disp = video_drvdata(file);
+ struct video_device *vfd = disp->video_dev;
+ struct disp_fh *disp_fh;
+
+ if (!disp->sd) {
+ v4l2_err(&disp->v4l2_dev, "No sub device registered\n");
+ return -ENODEV;
+ }
+
+ disp_fh = kzalloc(sizeof(*disp_fh), GFP_KERNEL);
+ if (!disp_fh) {
+ v4l2_err(&disp->v4l2_dev,
+ "unable to allocate memory for file handle object\n");
k*alloc functions already log any errors, you could just return -ENOMEM,
without printing anything. There is similar occurrence in disp_probe.
Also it might be a good idea to make your function and data structure names
more specific to this device, e.g. s/disp_*/bfin_disp_*.
+ return -ENOMEM;
+ }
+
+ v4l2_fh_init(&disp_fh->fh, vfd);
+
+ /* store pointer to v4l2_fh in private_data member of file */
+ file->private_data =&disp_fh->fh;
+ v4l2_fh_add(&disp_fh->fh);
+ disp_fh->io_allowed = false;
+ return 0;
+}
+static int disp_reqbufs(struct file *file, void *priv,
+ struct v4l2_requestbuffers *req_buf)
+{
+ struct disp_device *disp = video_drvdata(file);
+ struct vb2_queue *vq =&disp->buffer_queue;
+ struct v4l2_fh *fh = file->private_data;
+ struct disp_fh *disp_fh = container_of(fh, struct disp_fh, fh);
+
+ if (vb2_is_busy(vq))
+ return -EBUSY;
+
+ disp_fh->io_allowed = true;
+
+ return vb2_reqbufs(vq, req_buf);
+}
+
+static int disp_querybuf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct disp_device *disp = video_drvdata(file);
+
+ return vb2_querybuf(&disp->buffer_queue, buf);
+}
+
+static int disp_qbuf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct disp_device *disp = video_drvdata(file);
+ struct v4l2_fh *fh = file->private_data;
+ struct disp_fh *disp_fh = container_of(fh, struct disp_fh, fh);
+
+ if (!disp_fh->io_allowed)
+ return -EBUSY;
+
+ return vb2_qbuf(&disp->buffer_queue, buf);
+}
+
+static int disp_dqbuf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct disp_device *disp = video_drvdata(file);
+ struct v4l2_fh *fh = file->private_data;
+ struct disp_fh *disp_fh = container_of(fh, struct disp_fh, fh);
+
+ if (!disp_fh->io_allowed)
+ return -EBUSY;
+
+ return vb2_dqbuf(&disp->buffer_queue,
+ buf, file->f_flags& O_NONBLOCK);
+}
I would suggest you have a look at the videobuf2 ioctl/fop helpers. Lots of
boilerplate code can be removed when you use them. For an example see:
http://git.linuxtv.org/snawrocki/samsung.git/commitdiff/38b7d67224965bf09eaa3ce147bbebc7fa089411
+static int disp_probe(struct platform_device *pdev)
+{
+ struct disp_device *disp;
+ struct video_device *vfd;
+ struct i2c_adapter *i2c_adap;
+ struct bfin_display_config *config;
+ struct vb2_queue *q;
+ struct disp_route *route;
+ int ret;
+
+ config = pdev->dev.platform_data;
+ if (!config) {
+ v4l2_err(pdev->dev.driver, "Unable to get board config\n");
+ return -ENODEV;
+ }
+
+ disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+ if (!disp) {
+ v4l2_err(pdev->dev.driver, "Unable to alloc disp\n");
+ return -ENOMEM;
+ }
+
+ disp->cfg = config;
+
+ disp->ppi = ppi_create_instance(config->ppi_info);
+ if (!disp->ppi) {
+ v4l2_err(pdev->dev.driver, "Unable to create ppi\n");
+ ret = -ENODEV;
+ goto err_free_dev;
+ }
+ disp->ppi->priv = disp;
+
+ disp->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
+ if (IS_ERR(disp->alloc_ctx)) {
+ ret = PTR_ERR(disp->alloc_ctx);
+ goto err_free_ppi;
+ }
+
+ vfd = video_device_alloc();
Instead of this allocation you could embed struct video_device instance
in struct disp_device...
+ if (!vfd) {
+ ret = -ENOMEM;
+ v4l2_err(pdev->dev.driver, "Unable to alloc video device\n");
+ goto err_cleanup_ctx;
+ }
+
+ /* initialize field of video device */
+ vfd->release = video_device_release;
..and make this
vfd->release = video_device_release_empty;
+ vfd->fops =&disp_fops;
+ vfd->ioctl_ops =&disp_ioctl_ops;
+ vfd->tvnorms = 0;
+ vfd->v4l2_dev =&disp->v4l2_dev;
+ vfd->vfl_dir = VFL_DIR_TX;
+ set_bit(V4L2_FL_USE_FH_PRIO,&vfd->flags);
+ strncpy(vfd->name, DISPLAY_DRV_NAME, sizeof(vfd->name));
+ disp->video_dev = vfd;
+
+ ret = v4l2_device_register(&pdev->dev,&disp->v4l2_dev);
+ if (ret) {
+ v4l2_err(pdev->dev.driver,
+ "Unable to register v4l2 device\n");
+ goto err_release_vdev;
+ }
+ v4l2_info(&disp->v4l2_dev, "v4l2 device registered\n");
+
+ disp->v4l2_dev.ctrl_handler =&disp->ctrl_handler;
+ ret = v4l2_ctrl_handler_init(&disp->ctrl_handler, 0);
+ if (ret) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to init control handler\n");
+ goto err_unreg_v4l2;
+ }
+
+ spin_lock_init(&disp->lock);
+ /* initialize queue */
+ q =&disp->buffer_queue;
+ q->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ q->io_modes = VB2_MMAP;
+ q->drv_priv = disp;
+ q->buf_struct_size = sizeof(struct disp_buffer);
+ q->ops =&disp_video_qops;
+ q->mem_ops =&vb2_dma_contig_memops;
+
+ ret = vb2_queue_init(q);
+ if (ret) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to init videobuf2 queue\n");
+ goto err_free_handler;
+ }
+
+ mutex_init(&disp->mutex);
+
+ /* init video dma queues */
+ INIT_LIST_HEAD(&disp->dma_queue);
+
+ vfd->lock =&disp->mutex;
+
+ /* register video device */
+ ret = video_register_device(disp->video_dev, VFL_TYPE_GRABBER, -1);
The video device should be registered as a last step, only when all
resources it uses are already initialized.
+ if (ret) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to register video device\n");
+ goto err_free_handler;
+ }
+ video_set_drvdata(disp->video_dev, disp);
+ v4l2_info(&disp->v4l2_dev, "video device registered as: %s\n",
+ video_device_node_name(vfd));
+
+ /* load up the subdevice */
+ i2c_adap = i2c_get_adapter(config->i2c_adapter_id);
+ if (!i2c_adap) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to find i2c adapter\n");
+ goto err_unreg_vdev;
+
+ }
+ disp->sd = v4l2_i2c_new_subdev_board(&disp->v4l2_dev,
+ i2c_adap,
+ &config->board_info,
+ NULL);
nit: I bit strange indentation, you could probably just fit it in 2 lines.
+ if (disp->sd) {
+ int i;
+ if (!config->num_outputs) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to work without output\n");
+ goto err_unreg_vdev;
+ }
+
+ /* update tvnorms from the sub devices */
+ for (i = 0; i< config->num_outputs; i++)
+ vfd->tvnorms |= config->outputs[i].std;
+ } else {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to register sub device\n");
+ goto err_unreg_vdev;
+ }
+
+ v4l2_info(&disp->v4l2_dev, "v4l2 sub device registered\n");
+
+ /*
+ * explicitly set output, otherwise some boards
+ * may not work at the state as we expected
+ */
+ route =&config->routes[0];
+ ret = v4l2_subdev_call(disp->sd, video, s_routing,
+ route->output, route->output, 0);
+ if ((ret< 0)&& (ret != -ENOIOCTLCMD)) {
+ v4l2_err(&disp->v4l2_dev, "Failed to set output\n");
+ goto err_unreg_vdev;
+ }
+ disp->cur_output = 0;
+ /* if this route has specific config, update ppi control */
+ if (route->ppi_control)
+ config->ppi_control = route->ppi_control;
+
+ /* now we can probe the default state */
+ if (config->outputs[0].capabilities& V4L2_IN_CAP_STD) {
+ v4l2_std_id std;
+ ret = v4l2_subdev_call(disp->sd, core, g_std,&std);
+ if (ret) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to get std\n");
+ goto err_unreg_vdev;
+ }
+ disp->std = std;
+ }
+ if (config->outputs[0].capabilities& V4L2_IN_CAP_CUSTOM_TIMINGS) {
+ struct v4l2_dv_timings dv_timings;
+ ret = v4l2_subdev_call(disp->sd, video,
+ g_dv_timings,&dv_timings);
+ if (ret) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to get dv timings\n");
+ goto err_unreg_vdev;
+ }
+ disp->dv_timings = dv_timings;
+ }
+ ret = disp_init_encoder_formats(disp);
+ if (ret) {
+ v4l2_err(&disp->v4l2_dev,
+ "Unable to create encoder formats table\n");
+ goto err_unreg_vdev;
+ }
+ return 0;
+err_unreg_vdev:
+ video_unregister_device(disp->video_dev);
+ disp->video_dev = NULL;
+err_free_handler:
+ v4l2_ctrl_handler_free(&disp->ctrl_handler);
+err_unreg_v4l2:
+ v4l2_device_unregister(&disp->v4l2_dev);
+err_release_vdev:
+ if (disp->video_dev)
+ video_device_release(disp->video_dev);
+err_cleanup_ctx:
+ vb2_dma_contig_cleanup_ctx(disp->alloc_ctx);
+err_free_ppi:
+ ppi_delete_instance(disp->ppi);
+err_free_dev:
+ kfree(disp);
+ return ret;
+}
Regards,
Sylwester
--
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
