Skip to contentSkip to conversation

davinci: vpif_capture: raw camera support

PATCH New Kevin Hilman 5 participants 10 messages Jun 6, 2017 – Jun 9, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover davinci: vpif_capture: raw camera support

Same as v1, just rebased onto master branch of media tree, as requested by Hans.

This series fixes/updates the support for raw camera input to the VPIF.

Tested on da850-evm boards using the add-on UI board. Tested with both composite video input (on-board tvp514x) and raw camera input using the camera board from On-Semi based on the aptina,mt9v032 sensor[1], as this was the only camera board with the right connector for the da850-evm UI board.

Verified that composite video capture is still working well after these updates.

[1] http://www.mouser.com/search/ProductDetail.aspx?R=0virtualkey0virtualkeyMT9V032C12STCH-GEVB

Kevin Hilman (4):
[media] davinci: vpif_capture: drop compliance hack [media] davinci: vpif_capture: get subdevs from DT when available [media] davinci: vpif_capture: cleanup raw camera support [media] davinci: vpif: adaptions for DT support

 drivers/media/platform/davinci/vpif.c         |  49 +++++-
 drivers/media/platform/davinci/vpif_capture.c | 223 +++++++++++++++++++++++---
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h            |   9 +-
 4 files changed, 262 insertions(+), 24 deletions(-)
Signature · 1 line
2.9.3
#1 lore raw

PATCH v2 1/4 davinci: vpif_capture: drop compliance hack

Capture driver silently overrides pixel format with a hack (according to the comments) to pass v4l2 compliance tests. This isn't needed for normal functionality, and works for composite video and raw camera capture without.

In addition, the hack assumes that it only supports raw capture with a single format (SBGGR8) which isn't true. VPIF can also capture 10- and 12-bit raw formats as well. Forthcoming patches will enable VPIF input with raw-camera support and has been tested with 10-bit format from the aptina,mt9v032 sensor.

Any compliance failures should be fixed with a real fix.

---
 drivers/media/platform/davinci/vpif_capture.c | 15 ---------------
 1 file changed, 15 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +0 -15
index 128e92d1dd5a..fc5c7622660c 100644
@@ -936,21 +936,6 @@ static int vpif_try_fmt_vid_cap(struct file *file, void *priv,
 	struct channel_obj *ch = video_get_drvdata(vdev);
 	struct v4l2_pix_format *pixfmt = &fmt->fmt.pix;
 	struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
-	struct vpif_params *vpif_params = &ch->vpifparams;
-
-	/*
-	 * to supress v4l-compliance warnings silently correct
-	 * the pixelformat
-	 */
-	if (vpif_params->iface.if_type == VPIF_IF_RAW_BAYER) {
-		if (pixfmt->pixelformat != V4L2_PIX_FMT_SBGGR8)
-			pixfmt->pixelformat = V4L2_PIX_FMT_SBGGR8;
-	} else {
-		if (pixfmt->pixelformat != V4L2_PIX_FMT_NV16)
-			pixfmt->pixelformat = V4L2_PIX_FMT_NV16;
-	}
-
-	common->fmt.fmt.pix.pixelformat = pixfmt->pixelformat;
 
 	vpif_update_std_info(ch);
 
Signature · 1 line
2.9.3
#2 lore raw

PATCH v2 2/4 davinci: vpif_capture: get subdevs from DT when available

In reply to Kevin Hilman

Enable getting of subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from) am437x-vpfe.c

---
 drivers/media/platform/davinci/vpif_capture.c | 126 +++++++++++++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h            |   9 +-
 3 files changed, 134 insertions(+), 6 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +124 -2
index fc5c7622660c..b9d927d1e5a8 100644
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>
+#include <media/i2c/tvp514x.h>
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -655,7 +657,7 @@ static int vpif_input_to_subdev(
 	/* loop through the sub device list to get the sub device info */
 	for (i = 0; i < vpif_cfg->subdev_count; i++) {
 		subdev_info = &vpif_cfg->subdev_info[i];
-		if (!strcmp(subdev_info->name, subdev_name))
+		if (subdev_info && !strcmp(subdev_info->name, subdev_name))
 			return i;
 	}
 	return -1;
@@ -1308,6 +1310,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
 {
 	int i;
 
+	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
+		struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
+		const struct device_node *node = _asd->match.of.node;
+
+		if (node == subdev->of_node) {
+			vpif_obj.sd[i] = subdev;
+			vpif_obj.config->chan_config->inputs[i].subdev_name =
+				(char *)subdev->of_node->full_name;
+			vpif_dbg(2, debug,
+				 "%s: setting input %d subdev_name = %s\n",
+				 __func__, i, subdev->of_node->full_name);
+			return 0;
+		}
+	}
+
 	for (i = 0; i < vpif_obj.config->subdev_count; i++)
 		if (!strcmp(vpif_obj.config->subdev_info[i].name,
 			    subdev->name)) {
@@ -1403,6 +1420,105 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
 	return vpif_probe_complete();
 }
 
+static struct vpif_capture_config *
+vpif_capture_get_pdata(struct platform_device *pdev)
+{
+	struct device_node *endpoint = NULL;
+	struct v4l2_of_endpoint bus_cfg;
+	struct vpif_capture_config *pdata;
+	struct vpif_subdev_info *sdinfo;
+	struct vpif_capture_chan_config *chan;
+	unsigned int i;
+
+	/*
+	 * DT boot: OF node from parent device contains
+	 * video ports & endpoints data.
+	 */
+	if (pdev->dev.parent && pdev->dev.parent->of_node)
+		pdev->dev.of_node = pdev->dev.parent->of_node;
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+		return pdev->dev.platform_data;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+	pdata->subdev_info =
+		devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
+			     VPIF_CAPTURE_NUM_CHANNELS, GFP_KERNEL);
+
+	if (!pdata->subdev_info)
+		return NULL;
+
+	for (i = 0; i < VPIF_CAPTURE_NUM_CHANNELS; i++) {
+		struct device_node *rem;
+		unsigned int flags;
+		int err;
+
+		endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+						      endpoint);
+		if (!endpoint)
+			break;
+
+		sdinfo = &pdata->subdev_info[i];
+		chan = &pdata->chan_config[i];
+		chan->inputs = devm_kzalloc(&pdev->dev,
+					    sizeof(*chan->inputs) *
+					    VPIF_CAPTURE_NUM_CHANNELS,
+					    GFP_KERNEL);
+
+		chan->input_count++;
+		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
+		chan->inputs[i].input.std = V4L2_STD_ALL;
+		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
+
+		err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+		if (err) {
+			dev_err(&pdev->dev, "Could not parse the endpoint\n");
+			goto done;
+		}
+		dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
+			endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+		flags = bus_cfg.bus.parallel.flags;
+
+		if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+			chan->vpif_if.hd_pol = 1;
+
+		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+			chan->vpif_if.vd_pol = 1;
+
+		rem = of_graph_get_remote_port_parent(endpoint);
+		if (!rem) {
+			dev_dbg(&pdev->dev, "Remote device at %s not found\n",
+				endpoint->full_name);
+			goto done;
+		}
+
+		dev_dbg(&pdev->dev, "Remote device %s, %s found\n",
+			rem->name, rem->full_name);
+		sdinfo->name = rem->full_name;
+
+		pdata->asd[i] = devm_kzalloc(&pdev->dev,
+					     sizeof(struct v4l2_async_subdev),
+					     GFP_KERNEL);
+		if (!pdata->asd[i]) {
+			of_node_put(rem);
+			pdata = NULL;
+			goto done;
+		}
+
+		pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
+		pdata->asd[i]->match.of.node = rem;
+		of_node_put(rem);
+	}
+
+done:
+	pdata->asd_sizes[0] = i;
+	pdata->subdev_count = i;
+	pdata->card_name = "DA850/OMAP-L138 Video Capture";
+
+	return pdata;
+}
+
 /**
  * vpif_probe : This function probes the vpif capture driver
  * @pdev: platform device pointer
@@ -1419,6 +1535,12 @@ static __init int vpif_probe(struct platform_device *pdev)
 	int res_idx = 0;
 	int i, err;
 
+	pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	if (!pdev->dev.platform_data) {
 		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
 		return -EINVAL;
@@ -1459,7 +1581,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 		goto vpif_unregister;
 	}
 
-	if (!vpif_obj.config->asd_sizes) {
+	if (!vpif_obj.config->asd_sizes[0]) {
 		int i2c_id = vpif_obj.config->i2c_adapter_id;
 
 		i2c_adap = i2c_get_adapter(i2c_id);
drivers/media/platform/davinci/vpif_display.c +5 -0
index 7e5cf9923c8d..b5ac6ce626b3 100644
@@ -1250,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	if (!pdev->dev.platform_data) {
+		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
+		return -EINVAL;
+	}
+
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
include/media/davinci/vpif_types.h +5 -4
index 385597da20dc..eae23e4e9b93 100644
@@ -62,14 +62,14 @@ struct vpif_display_config {
 
 struct vpif_input {
 	struct v4l2_input input;
-	const char *subdev_name;
+	char *subdev_name;
 	u32 input_route;
 	u32 output_route;
 };
 
 struct vpif_capture_chan_config {
 	struct vpif_interface vpif_if;
-	const struct vpif_input *inputs;
+	struct vpif_input *inputs;
 	int input_count;
 };
 
@@ -81,7 +81,8 @@ struct vpif_capture_config {
 	int subdev_count;
 	int i2c_adapter_id;
 	const char *card_name;
-	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
-	int *asd_sizes;		/* 0-terminated array of asd group sizes */
+
+	struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
+	int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
 };
 #endif /* _VPIF_TYPES_H */
Signature · 1 line
2.9.3
#3 lore raw

PATCH v2 2/4 davinci: vpif_capture: get subdevs from DT when available

Quoted 17 lines · Kevin Hilman

Enable getting of subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from) am437x-vpfe.c

---
 drivers/media/platform/davinci/vpif_capture.c | 126 +++++++++++++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h            |   9 +-
 3 files changed, 134 insertions(+), 6 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +0 -0
index fc5c7622660c..b9d927d1e5a8 100644

drivers/media/platform/davinci/vpif_capture.c line 25

Quoted 4 lines
drivers/media/platform/davinci/vpif_capture.c +1 -0
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>

v4l2-of.h no longer exists, so this v2 is wrong. Unfortunately this patch has already been merged in our master. I'm not sure how this could have slipped past both my and Mauro's patch testing (and yours, for that matter).

Can you fix this and post a patch on top of the media master that makes this compile again?

Regards,

Hans

#4 lore raw

PATCH v2 2/4 davinci: vpif_capture: get subdevs from DT when available

Quoted 20 lines · Hans Verkuil
Quoted 19 lines · Kevin Hilman

Enable getting of subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from) am437x-vpfe.c

---
 drivers/media/platform/davinci/vpif_capture.c | 126 +++++++++++++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h            |   9 +-
 3 files changed, 134 insertions(+), 6 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +0 -0
index fc5c7622660c..b9d927d1e5a8 100644
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 
Quoted 6 lines

drivers/media/platform/davinci/vpif_capture.c line 25

Quoted 2 lines
drivers/media/platform/davinci/vpif_capture.c +1 -0
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>

v4l2-of.h no longer exists, so this v2 is wrong. Unfortunately this patch has already been merged in our master. I'm not sure how this could have slipped past both my and Mauro's patch testing (and yours, for that matter).

I have that file in the various trees I tested agains.

Quoted 2 lines

Can you fix this and post a patch on top of the media master that makes this compile again?

Sorry for the dumb question, but what tree are you referring to? I tried the master branch of both [1] and [2] and both seem to have that include.

Kevin

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git
[2] git://linuxtv.org/mchehab/media-next.git

maintainer
#5 lore raw

PATCH v2 2/4 davinci: vpif_capture: get subdevs from DT when available

Hi Kevin,

Thanks for the patch.

Quoted 11 lines · Kevin Hilman
Quoted 10 lines · Hans Verkuil
Quoted 9 lines · Kevin Hilman

Enable getting of subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from) am437x-vpfe.c

---
 drivers/media/platform/davinci/vpif_capture.c | 126 +++++++++++++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |   5 +

Can you make this as a separate patch.

Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
 include/media/davinci/vpif_types.h            |   9 +-
 3 files changed, 134 insertions(+), 6 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +0 -0
index fc5c7622660c..b9d927d1e5a8 100644
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 
Quoted 6 lines

drivers/media/platform/davinci/vpif_capture.c line 25

Quoted 2 lines
drivers/media/platform/davinci/vpif_capture.c +1 -0
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>

v4l2-of.h no longer exists, so this v2 is wrong. Unfortunately this patch has already been merged in our master. I'm not sure how this could have slipped past both my and Mauro's patch testing (and yours, for that matter).

I have that file in the various trees I tested agains.

Quoted 2 lines

Can you fix this and post a patch on top of the media master that makes this compile again?

Quoted 8 lines

Sorry for the dumb question, but what tree are you referring to? I tried the master branch of both [1] and [2] and both seem to have that include.

Kevin

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git
[2] git://linuxtv.org/mchehab/media-next.git

media master is https://git.linuxtv.org/media_tree.git/

Cheers,
--Prabhakar Lad

#6 lore raw

PATCH v2 2/4 davinci: vpif_capture: get subdevs from DT when available

In reply to Kevin Hilman

Hi Kevin,

Quoted 28 lines · Kevin Hilman
Quoted 20 lines · Hans Verkuil
Quoted 19 lines · Kevin Hilman

Enable getting of subdevs from DT ports and endpoints.

The _get_pdata() function was larely inspired by (i.e. stolen from) am437x-vpfe.c

---
 drivers/media/platform/davinci/vpif_capture.c | 126 +++++++++++++++++++++++++-
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h            |   9 +-
 3 files changed, 134 insertions(+), 6 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +0 -0
index fc5c7622660c..b9d927d1e5a8 100644
@@ -22,6 +22,8 @@
 #include <linux/slab.h>
 
Quoted 6 lines

drivers/media/platform/davinci/vpif_capture.c line 25

Quoted 2 lines
drivers/media/platform/davinci/vpif_capture.c +1 -0
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>

v4l2-of.h no longer exists, so this v2 is wrong. Unfortunately this patch has already been merged in our master. I'm not sure how this could have slipped past both my and Mauro's patch testing (and yours, for that matter).

Quoted 8 lines

I have that file in the various trees I tested agains.

Quoted 2 lines

Can you fix this and post a patch on top of the media master that makes this compile again?

Sorry for the dumb question, but what tree are you referring to? I tried the master branch of both [1] and [2] and both seem to have that include.

These patches are now merged as well as the changes requested:

<URL:https://git.linuxtv.org/media_tree.git/commit/?id=a2d17962c9ca7ac66a132bbbfc6054559856e14e>

Signature · 2 lines
Sakari Ailus
e-mail: sakari.ailus at iki.fi	XMPP: sailus at retiisi.org.uk
#7 lore raw

PATCH v2 3/4 davinci: vpif_capture: cleanup raw camera support

In reply to Kevin Hilman

The current driver has a handful of hard-coded assumptions based on its primary use for capture of video signals. Cleanup those assumptions, and also query the subdev for format information and use that if available.

Tested with 10-bit raw bayer input (SGRBG10) using the aptina,mt9v032 sensor, and also tested that composite video input still works from ti,tvp514x decoder. Both tests done on the da850-evm board with the add-on UI board.

NOTE: Will need further testing for other sensors with different bus formats.

---
 drivers/media/platform/davinci/vpif_capture.c | 82 ++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 2 deletions(-)
drivers/media/platform/davinci/vpif_capture.c +80 -2
index b9d927d1e5a8..67624dbf1272 100644
@@ -24,6 +24,9 @@
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-of.h>
 #include <media/i2c/tvp514x.h>
+#include <media/v4l2-mediabus.h>
+
+#include <linux/videodev2.h>
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -387,7 +390,8 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
 		common = &ch->common[i];
 		/* skip If streaming is not started in this channel */
 		/* Check the field format */
-		if (1 == ch->vpifparams.std_info.frm_fmt) {
+		if (1 == ch->vpifparams.std_info.frm_fmt ||
+		    common->fmt.fmt.pix.field == V4L2_FIELD_NONE) {
 			/* Progressive mode */
 			spin_lock(&common->irqlock);
 			if (list_empty(&common->dma_queue)) {
@@ -468,9 +472,38 @@ static int vpif_update_std_info(struct channel_obj *ch)
 	struct vpif_channel_config_params *std_info = &vpifparams->std_info;
 	struct video_obj *vid_ch = &ch->video;
 	int index;
+	struct v4l2_pix_format *pixfmt = &common->fmt.fmt.pix;
 
 	vpif_dbg(2, debug, "vpif_update_std_info\n");
 
+	/*
+	 * if called after try_fmt or g_fmt, there will already be a size
+	 * so use that by default.
+	 */
+	if (pixfmt->width && pixfmt->height) {
+		if (pixfmt->field == V4L2_FIELD_ANY ||
+		    pixfmt->field == V4L2_FIELD_NONE)
+			pixfmt->field = V4L2_FIELD_NONE;
+		
+		vpifparams->iface.if_type = VPIF_IF_BT656;
+		if (pixfmt->pixelformat == V4L2_PIX_FMT_SGRBG10 ||
+		    pixfmt->pixelformat == V4L2_PIX_FMT_SBGGR8)
+			vpifparams->iface.if_type = VPIF_IF_RAW_BAYER;
+
+		if (pixfmt->pixelformat == V4L2_PIX_FMT_SGRBG10)
+			vpifparams->params.data_sz = 1; /* 10 bits/pixel.  */
+
+		/* 
+		 * For raw formats from camera sensors, we don't need 
+		 * the std_info from table lookup, so nothing else to do here.
+		 */
+		if (vpifparams->iface.if_type == VPIF_IF_RAW_BAYER) {
+			memset(std_info, 0, sizeof(struct vpif_channel_config_params));
+			vpifparams->std_info.capture_format = 1; /* CCD/raw mode */
+			return 0;
+		}
+	}
+
 	for (index = 0; index < vpif_ch_params_count; index++) {
 		config = &vpif_ch_params[index];
 		if (config->hd_sd == 0) {
@@ -939,6 +972,7 @@ static int vpif_try_fmt_vid_cap(struct file *file, void *priv,
 	struct v4l2_pix_format *pixfmt = &fmt->fmt.pix;
 	struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
 
+	common->fmt = *fmt;
 	vpif_update_std_info(ch);
 
 	pixfmt->field = common->fmt.fmt.pix.field;
@@ -947,8 +981,17 @@ static int vpif_try_fmt_vid_cap(struct file *file, void *priv,
 	pixfmt->width = common->fmt.fmt.pix.width;
 	pixfmt->height = common->fmt.fmt.pix.height;
 	pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height * 2;
+	if (pixfmt->pixelformat == V4L2_PIX_FMT_SGRBG10) {
+		pixfmt->bytesperline = common->fmt.fmt.pix.width * 2;
+		pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
+	}
 	pixfmt->priv = 0;
 
+	dev_dbg(vpif_dev, "%s: %d x %d; pitch=%d pixelformat=0x%08x, field=%d, size=%d\n", __func__,
+		pixfmt->width, pixfmt->height,
+		pixfmt->bytesperline, pixfmt->pixelformat,
+		pixfmt->field, pixfmt->sizeimage);
+
 	return 0;
 }
 
@@ -965,13 +1008,47 @@ static int vpif_g_fmt_vid_cap(struct file *file, void *priv,
 	struct video_device *vdev = video_devdata(file);
 	struct channel_obj *ch = video_get_drvdata(vdev);
 	struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
+	struct v4l2_pix_format *pix_fmt = &fmt->fmt.pix;
+	struct v4l2_subdev_format format = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	struct v4l2_mbus_framefmt *mbus_fmt = &format.format;
+	int ret;
 
 	/* Check the validity of the buffer type */
 	if (common->fmt.type != fmt->type)
 		return -EINVAL;
 
-	/* Fill in the information about format */
+	/* By default, use currently set fmt */
 	*fmt = common->fmt;
+	
+	/* If subdev has get_fmt, use that to override */
+	ret = v4l2_subdev_call(ch->sd, pad, get_fmt, NULL, &format);
+	if (!ret && mbus_fmt->code) {
+		v4l2_fill_pix_format(pix_fmt, mbus_fmt);
+		pix_fmt->bytesperline = pix_fmt->width;
+		if (mbus_fmt->code == MEDIA_BUS_FMT_SGRBG10_1X10) {
+			/* e.g. mt9v032 */
+			pix_fmt->pixelformat = V4L2_PIX_FMT_SGRBG10;
+			pix_fmt->bytesperline = pix_fmt->width * 2;
+		} else if (mbus_fmt->code == MEDIA_BUS_FMT_UYVY8_2X8) {
+			/* e.g. tvp514x */
+			pix_fmt->pixelformat = V4L2_PIX_FMT_NV16;
+			pix_fmt->bytesperline = pix_fmt->width * 2;
+		} else {
+			dev_warn(vpif_dev, "%s: Unhandled media-bus format 0x%x\n",
+				 __func__, mbus_fmt->code);
+		}
+		pix_fmt->sizeimage = pix_fmt->bytesperline * pix_fmt->height;
+		dev_dbg(vpif_dev, "%s: %d x %d; pitch=%d, pixelformat=0x%08x, code=0x%x, field=%d, size=%d\n", __func__,
+			pix_fmt->width, pix_fmt->height,
+			pix_fmt->bytesperline, pix_fmt->pixelformat, 
+			mbus_fmt->code, pix_fmt->field, pix_fmt->sizeimage);
+
+		common->fmt = *fmt;
+		vpif_update_std_info(ch);
+	}
+
 	return 0;
 }
 
@@ -1358,6 +1435,7 @@ static int vpif_probe_complete(void)
 		/* set initial format */
 		ch->video.stdid = V4L2_STD_525_60;
 		memset(&ch->video.dv_timings, 0, sizeof(ch->video.dv_timings));
+		common->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 		vpif_update_std_info(ch);
 
 		/* Initialize vb2 queue */
Signature · 1 line
2.9.3
#8 lore raw

PATCH v2 4/4 davinci: vpif: adaptions for DT support

In reply to Kevin Hilman

The davinci VPIF is a single hardware block, but the existing driver is broken up into a common library (vpif.c), output (vpif_display.c) and intput (vpif_capture.c).

When migrating to DT, to better model the hardware, and because registers, interrupts, etc. are all common,it was decided to have a single VPIF hardware node[1].

Because davinci uses legacy, non-DT boot on several SoCs still, the platform_drivers need to remain. But they are also needed in DT boot. Since there are no DT nodes for the display/capture parts in DT boot (there is a single node for the parent/common device) we need to create platform_devices somewhere to instansiate the platform_drivers.

When VPIF display/capture are needed for a DT boot, the VPIF node will have endpoints defined for its subdevs. Therefore, vpif_probe() checks for the presence of endpoints, and if detected manually creates the platform_devices for the display and capture platform_drivers.

[1] Documentation/devicetree/bindings/media/ti,da850-vpif.txt

---
 drivers/media/platform/davinci/vpif.c | 49 ++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)
drivers/media/platform/davinci/vpif.c +48 -1
index 1b02a6363f77..502917abcb13 100644
@@ -26,6 +26,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/spinlock.h>
 #include <linux/v4l2-dv-timings.h>
+#include <linux/of_graph.h>
 
 #include "vpif.h"
 
@@ -423,7 +424,9 @@ EXPORT_SYMBOL(vpif_channel_getfid);
 
 static int vpif_probe(struct platform_device *pdev)
 {
-	static struct resource	*res;
+	static struct resource	*res, *res_irq;
+	struct platform_device *pdev_capture, *pdev_display;
+	struct device_node *endpoint = NULL;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	vpif_base = devm_ioremap_resource(&pdev->dev, res);
@@ -435,6 +438,50 @@ static int vpif_probe(struct platform_device *pdev)
 
 	spin_lock_init(&vpif_lock);
 	dev_info(&pdev->dev, "vpif probe success\n");
+
+	/*
+	 * If VPIF Node has endpoints, assume "new" DT support,
+	 * where capture and display drivers don't have DT nodes
+	 * so their devices need to be registered manually here
+	 * for their legacy platform_drivers to work.
+	 */
+	endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+					      endpoint);
+	if (!endpoint) 
+		return 0;
+
+	/*
+	 * For DT platforms, manually create platform_devices for
+	 * capture/display drivers.
+	 */
+	res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res_irq) {
+		dev_warn(&pdev->dev, "Missing IRQ resource.\n");
+		return -EINVAL;
+	}
+
+	pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
+				    GFP_KERNEL);
+	pdev_capture->name = "vpif_capture";
+	pdev_capture->id = -1;
+	pdev_capture->resource = res_irq;
+	pdev_capture->num_resources = 1;
+	pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
+	pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+	pdev_capture->dev.parent = &pdev->dev;
+	platform_device_register(pdev_capture);
+
+	pdev_display = devm_kzalloc(&pdev->dev, sizeof(*pdev_display),
+				    GFP_KERNEL);
+	pdev_display->name = "vpif_display";
+	pdev_display->id = -1;
+	pdev_display->resource = res_irq;
+	pdev_display->num_resources = 1;
+	pdev_display->dev.dma_mask = pdev->dev.dma_mask;
+	pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+	pdev_display->dev.parent = &pdev->dev;
+	platform_device_register(pdev_display);
+
 	return 0;
 }
 
Signature · 1 line
2.9.3
#9 lore raw

PATCH v2 4/4 davinci: vpif: adaptions for DT support

Em Tue, 6 Jun 2017 16:37:41 -0700
Kevin Hilman <[email protected]> escreveu:

Quoted 67 lines

The davinci VPIF is a single hardware block, but the existing driver is broken up into a common library (vpif.c), output (vpif_display.c) and intput (vpif_capture.c).

When migrating to DT, to better model the hardware, and because registers, interrupts, etc. are all common,it was decided to have a single VPIF hardware node[1].

Because davinci uses legacy, non-DT boot on several SoCs still, the platform_drivers need to remain. But they are also needed in DT boot. Since there are no DT nodes for the display/capture parts in DT boot (there is a single node for the parent/common device) we need to create platform_devices somewhere to instansiate the platform_drivers.

When VPIF display/capture are needed for a DT boot, the VPIF node will have endpoints defined for its subdevs. Therefore, vpif_probe() checks for the presence of endpoints, and if detected manually creates the platform_devices for the display and capture platform_drivers.

[1] Documentation/devicetree/bindings/media/ti,da850-vpif.txt

---
 drivers/media/platform/davinci/vpif.c | 49 ++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)
drivers/media/platform/davinci/vpif.c +18 -1
index 1b02a6363f77..502917abcb13 100644
@@ -26,6 +26,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/spinlock.h>
 #include <linux/v4l2-dv-timings.h>
+#include <linux/of_graph.h>
 
 #include "vpif.h"
 
@@ -423,7 +424,9 @@ EXPORT_SYMBOL(vpif_channel_getfid);
 
 static int vpif_probe(struct platform_device *pdev)
 {
-	static struct resource	*res;
+	static struct resource	*res, *res_irq;
+	struct platform_device *pdev_capture, *pdev_display;
+	struct device_node *endpoint = NULL;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	vpif_base = devm_ioremap_resource(&pdev->dev, res);
@@ -435,6 +438,50 @@ static int vpif_probe(struct platform_device *pdev)
 
 	spin_lock_init(&vpif_lock);
 	dev_info(&pdev->dev, "vpif probe success\n");
+
+	/*
+	 * If VPIF Node has endpoints, assume "new" DT support,
+	 * where capture and display drivers don't have DT nodes
+	 * so their devices need to be registered manually here
+	 * for their legacy platform_drivers to work.
+	 */
+	endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+					      endpoint);
+	if (!endpoint) 
+		return 0;
+
+	/*
+	 * For DT platforms, manually create platform_devices for

drivers/media/platform/davinci/vpif.c static int vpif_probe(struct platform_device *pdev) · line 464

Quoted 10 lines
drivers/media/platform/davinci/vpif.c +10 -0
+	 * capture/display drivers.
+	 */
+	res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res_irq) {
+		dev_warn(&pdev->dev, "Missing IRQ resource.\n");
+		return -EINVAL;
+	}
+
+	pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
+				    GFP_KERNEL);

You need to check if it won't return NULL here...

drivers/media/platform/davinci/vpif.c static int vpif_probe(struct platform_device *pdev) · line 475

Quoted 11 lines
drivers/media/platform/davinci/vpif.c +11 -0
+	pdev_capture->name = "vpif_capture";
+	pdev_capture->id = -1;
+	pdev_capture->resource = res_irq;
+	pdev_capture->num_resources = 1;
+	pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
+	pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+	pdev_capture->dev.parent = &pdev->dev;
+	platform_device_register(pdev_capture);
+
+	pdev_display = devm_kzalloc(&pdev->dev, sizeof(*pdev_display),
+				    GFP_KERNEL);

and here.

Ok, in thesis, if size is lower than page size, it shouldn't fail, but we still want to have an explicit check[1].

[1] https://lwn.net/Articles/723317/

Thanks,
Mauro