Re: [bug report] [media] davinci: vpif_capture: get subdevs from DT when available

[Kevin Hilman]

Dan Carpenter <dan.carpen...@oracle.com> writes:

> Hello Kevin Hilman,
>
> The patch 4a5f8ae50b66: "[media] davinci: vpif_capture: get subdevs
> from DT when available" from Jun 6, 2017, leads to the following
> static checker warning:
>
>   drivers/media/platform/davinci/vpif_capture.c:1596 
> vpif_capture_get_pdata()
>   error: potential NULL dereference 'pdata'.
>
> drivers/media/platform/davinci/vpif_capture.c
>   1576  
>   1577  dev_dbg(>dev, "Remote device %s, %s found\n",
>   1578  rem->name, rem->full_name);
>   1579  sdinfo->name = rem->full_name;
>   1580  
>   1581  pdata->asd[i] = devm_kzalloc(>dev,
>   1582   sizeof(struct 
> v4l2_async_subdev),
>   1583   GFP_KERNEL);
>   1584  if (!pdata->asd[i]) {
>   1585  of_node_put(rem);
>   1586  pdata = NULL;
> 
> Set to NULL
>
>   1587  goto done;
>   1588  }
>   1589  
>   1590  pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
>   1591  pdata->asd[i]->match.fwnode.fwnode = 
> of_fwnode_handle(rem);
>   1592  of_node_put(rem);
>   1593  }
>   1594  
>   1595  done:
>   1596  pdata->asd_sizes[0] = i;
> 
> Dereference.
>
>   1597  pdata->subdev_count = i;
>   1598  pdata->card_name = "DA850/OMAP-L138 Video Capture";
>   1599  
>   1600  return pdata;
>   1601  }
>

Thanks for the bug report.  Fix submitted:
https://patchwork.linuxtv.org/patch/42433/

Kevin

[PATCH] [media] davinci: vpif_capture: fix potential NULL deref

[Kevin Hilman]

Fix potential NULL pointer dereference in the error path of memory
allocation failure.

Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index d78580f9e431..2e36d8a9f5e3 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1593,10 +1593,12 @@ vpif_capture_get_pdata(struct platform_device *pdev)
}
 
 done:
-   pdata->asd_sizes[0] = i;
-   pdata->subdev_count = i;
-   pdata->card_name = "DA850/OMAP-L138 Video Capture";
-
+   if (pdata) {
+   pdata->asd_sizes[0] = i;
+   pdata->subdev_count = i;
+   pdata->card_name = "DA850/OMAP-L138 Video Capture";
+   }
+   
return pdata;
 }
 
-- 
2.9.3

Re: [PATCH v2] [media] davinci: vpif: adaptions for DT support

[Kevin Hilman]

Sakari Ailus <sakari.ai...@iki.fi> writes:

> Hi Kevin,
>
> On Fri, Jun 09, 2017 at 09:10:26AM -0700, Kevin Hilman wrote:
>> 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
>> 
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>> Changes since v1:
>> - added proper error checking to kzalloc calls
>> - rebased onto media/master
>> 
>>  drivers/media/platform/davinci/vpif.c | 57 
>> ++-
>>  1 file changed, 56 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/media/platform/davinci/vpif.c 
>> b/drivers/media/platform/davinci/vpif.c
>> index 1b02a6363f77..c2d214dfaa3e 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -26,6 +26,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #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(>dev, res);
>> @@ -435,6 +438,58 @@ static int vpif_probe(struct platform_device *pdev)
>>  
>>  spin_lock_init(_lock);
>>  dev_info(>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(>dev, "Missing IRQ resource.\n");
>> +return -EINVAL;
>> +}
>> +
>> +pdev_capture = devm_kzalloc(>dev, sizeof(*pdev_capture),
>> +GFP_KERNEL);
>> +if (pdev_capture) {
>> +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 = >dev;
>> +platform_device_register(pdev_capture);
>
> Don't both of these (vpif_capture and vpif_display) depend on platform data?
> Or do I miss something?

The driver can (continue to) work in legacy mode with platform_data.  In
that case, there is no VPIF DT node (or a node without endpoints).

However, with recent changes, it can also work in DT mode, where the
VPIF node and endpoints used for display/capture come from DT, in which
case these nodes are created an don't depend on platform_data at all.

Hope that clarifies things, and thanks for the review,

Kevin

Re: [PATCH v2] [media] davinci: vpif: adaptions for DT support

[Kevin Hilman]

Hi Hans, Mauro,

On Fri, Jun 9, 2017 at 9:10 AM, Kevin Hilman <khil...@baylibre.com> wrote:
> 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
>
> Signed-off-by: Kevin Hilman <khil...@baylibre.com>

Can this one make it for v4.13 along with the rest of the series that
it was initially sent with?

This one needed a respin for some error checking, but is otherwise
unchanged, and has been tested on top of media/next.

Thanks,

Kevin

[PATCH v2] [media] davinci: vpif: adaptions for DT support

[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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
Changes since v1:
- added proper error checking to kzalloc calls
- rebased onto media/master

 drivers/media/platform/davinci/vpif.c | 57 ++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 1b02a6363f77..c2d214dfaa3e 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #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(>dev, res);
@@ -435,6 +438,58 @@ static int vpif_probe(struct platform_device *pdev)
 
spin_lock_init(_lock);
dev_info(>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(>dev, "Missing IRQ resource.\n");
+   return -EINVAL;
+   }
+
+   pdev_capture = devm_kzalloc(>dev, sizeof(*pdev_capture),
+   GFP_KERNEL);
+   if (pdev_capture) {
+   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 = >dev;
+   platform_device_register(pdev_capture);
+   } else {
+   dev_warn(>dev, "Unable to allocate memory for 
pdev_capture.\n");
+   }
+
+   pdev_display = devm_kzalloc(>dev, sizeof(*pdev_display),
+   GFP_KERNEL);
+   if (pdev_display) {
+   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 = >dev;
+   platform_device_register(pdev_display);
+   } else {
+   dev_warn(>dev, "Unable to allocate memory for 
pdev_display.\n");
+   }
+
return 0;
 }
 
-- 
2.9.3

Re: [PATCH v2 2/4] [media] davinci: vpif_capture: get subdevs from DT when available

[Kevin Hilman]

On Wed, Jun 7, 2017 at 11:29 PM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> On 07/06/17 01:37, Kevin Hilman wrote:
>> Enable  getting of subdevs from DT ports and endpoints.
>>
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> am437x-vpfe.c
>>
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  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(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index fc5c7622660c..b9d927d1e5a8 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -22,6 +22,8 @@
>>  #include 
>>
>>  #include 
>> +#include 
>
> 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.

> 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

Re: [PATCH 0/4] [media] davinci: vpif_capture: raw camera support

[Kevin Hilman]

On Tue, Jun 6, 2017 at 1:34 PM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> Hi Kevin,
>
> On 02/06/17 23:34, Kevin Hilman wrote:
>> 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.
>
> Can you rebase this patch series against the latest media master branch?
>
> Mauro merged a lot of patches, in particular the one switching v4l2_of_ to
> v4l2_fwnode_. And that conflicts with patches 2 and 4.

Rebased and resent as v2.

Kevin

[PATCH v2 4/4] [media] davinci: vpif: adaptions for DT support

[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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 49 ++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 1b02a6363f77..502917abcb13 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #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(>dev, res);
@@ -435,6 +438,50 @@ static int vpif_probe(struct platform_device *pdev)
 
spin_lock_init(_lock);
dev_info(>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(>dev, "Missing IRQ resource.\n");
+   return -EINVAL;
+   }
+
+   pdev_capture = devm_kzalloc(>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 = >dev;
+   platform_device_register(pdev_capture);
+
+   pdev_display = devm_kzalloc(>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 = >dev;
+   platform_device_register(pdev_display);
+
return 0;
 }
 
-- 
2.9.3

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

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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 82 ++-
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index b9d927d1e5a8..67624dbf1272 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -24,6 +24,9 @@
 #include 
 #include 
 #include 
+#include 
+
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -387,7 +390,8 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
common = >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(>irqlock);
if (list_empty(>dma_queue)) {
@@ -468,9 +472,38 @@ static int vpif_update_std_info(struct channel_obj *ch)
struct vpif_channel_config_params *std_info = >std_info;
struct video_obj *vid_ch = >video;
int index;
+   struct v4l2_pix_format *pixfmt = >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 = _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.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 = >common[VPIF_VIDEO_INDEX];
+   struct v4l2_pix_format *pi

[PATCH v2 0/4] [media] davinci: vpif_capture: raw camera support

[Kevin Hilman]

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

-- 
2.9.3

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

[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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 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(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index fc5c7622660c..b9d927d1e5a8 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -22,6 +22,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #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 = _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(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>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 = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>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, _cfg);
+   if (err) {
+   dev_err(>dev, "Could not parse the endpoint\n");
+   goto done;
+   }
+   dev_dbg(>dev, "Endpoint %s, bus_width = %d\n",
+   endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+   flags = bus_cfg.bus.parallel.flags;
+
+   if (

[PATCH v2 1/4] [media] davinci: vpif_capture: drop compliance hack

[Kevin Hilman]

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.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 128e92d1dd5a..fc5c7622660c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -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.pix;
struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
-   struct vpif_params *vpif_params = >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);
 
-- 
2.9.3

[PATCH 0/4] [media] davinci: vpif_capture: raw camera support

[Kevin Hilman]

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 | 224 +++---
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h|   9 +-
 4 files changed, 263 insertions(+), 24 deletions(-)

-- 
2.9.3

[PATCH 4/4] [media] davinci: vpif: adaptions for DT support

[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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 49 ++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 1b02a6363f77..502917abcb13 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #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(>dev, res);
@@ -435,6 +438,50 @@ static int vpif_probe(struct platform_device *pdev)
 
spin_lock_init(_lock);
dev_info(>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(>dev, "Missing IRQ resource.\n");
+   return -EINVAL;
+   }
+
+   pdev_capture = devm_kzalloc(>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 = >dev;
+   platform_device_register(pdev_capture);
+
+   pdev_display = devm_kzalloc(>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 = >dev;
+   platform_device_register(pdev_display);
+
return 0;
 }
 
-- 
2.9.3

[PATCH 1/4] [media] davinci: vpif_capture: drop compliance hack

[Kevin Hilman]

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.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 128e92d1dd5a..fc5c7622660c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -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.pix;
struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
-   struct vpif_params *vpif_params = >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);
 
-- 
2.9.3

[PATCH 3/4] [media] davinci: vpif_capture: cleanup raw camera support

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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 82 ++-
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index b9d927d1e5a8..67624dbf1272 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -24,6 +24,9 @@
 #include 
 #include 
 #include 
+#include 
+
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -387,7 +390,8 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
common = >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(>irqlock);
if (list_empty(>dma_queue)) {
@@ -468,9 +472,38 @@ static int vpif_update_std_info(struct channel_obj *ch)
struct vpif_channel_config_params *std_info = >std_info;
struct video_obj *vid_ch = >video;
int index;
+   struct v4l2_pix_format *pixfmt = >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 = _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.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 = >common[VPIF_VIDEO_INDEX];
+   struct v4l2_pix_format *pi

[PATCH 2/4] [media] davinci: vpif_capture: get subdevs from DT when available

[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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 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(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index fc5c7622660c..b9d927d1e5a8 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -22,6 +22,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #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 = _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(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>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 = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>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, _cfg);
+   if (err) {
+   dev_err(>dev, "Could not parse the endpoint\n");
+   goto done;
+   }
+   dev_dbg(>dev, "Endpoint %s, bus_width = %d\n",
+   endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+   flags = bus_cfg.bus.parallel.flags;
+
+   if (

Re: [PATCH] davinci: vpif_capture: fix default pixel format for BT.656/BT.1120 video

[Kevin Hilman]

Sekhar Nori <nsek...@ti.com> writes:

> For both BT.656 and BT.1120 video, the pixel format
> used by VPIF is Y/CbCr 4:2:2 in semi-planar format
> (Luma in one plane and Chroma in another). This
> corresponds to NV16 pixel format.
>
> This is documented in section 36.2.3 of OMAP-L138
> Technical Reference Manual, SPRUH77A.
>
> The VPIF driver incorrectly sets the default format
> to V4L2_PIX_FMT_YUV422P. Fix it.
>
> Reported-by: Alejandro Hernandez <ajhernan...@ti.com>
> Signed-off-by: Sekhar Nori <nsek...@ti.com>

Acked-by: Kevin Hilman <khil...@baylibre.com>

Re: [PATCH] ARM64: defconfig: enable IR core, decoders and Meson IR device

[Kevin Hilman]

Neil Armstrong  writes:

> This patch enables the MEDIA Infrared RC Decoders and Meson Infrared
> decoder for ARM64 defconfig.
> These drivers are selected as modules by default.
>
> Signed-off-by: Neil Armstrong 
> ---
>  arch/arm64/configs/defconfig | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index c021aefa..59c400f 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -321,6 +321,11 @@ CONFIG_MEDIA_CAMERA_SUPPORT=y
>  CONFIG_MEDIA_ANALOG_TV_SUPPORT=y
>  CONFIG_MEDIA_DIGITAL_TV_SUPPORT=y
>  CONFIG_MEDIA_CONTROLLER=y
> +CONFIG_MEDIA_RC_SUPPORT=y
> +CONFIG_RC_CORE=y

This one should be a module too.

With that fixed, applied to v4.13/defconfig

Kevin

> +CONFIG_RC_DEVICES=y
> +CONFIG_RC_DECODERS=y
> +CONFIG_IR_MESON=m
>  CONFIG_VIDEO_V4L2_SUBDEV_API=y
>  # CONFIG_DVB_NET is not set
>  CONFIG_V4L_MEM2MEM_DRIVERS=y

Re: [PATCH 02/10] ARM: dts: da850-evm: fix whitespace errors

[Kevin Hilman]

Kevin Hilman <khil...@baylibre.com> writes:

> Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:
>
>> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
>
> I'll fold this one into the original since it's not yet merged.

Oops, Sekhar has already merged this one to his v4.11/dt branch, so he
can apply it (or fold it in.)

Kevin

Re: [PATCH 07/10] ARM: davinci: fix a whitespace error

[Kevin Hilman]

Kevin Hilman <khil...@baylibre.com> writes:

> Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:
>
>> There's a stray tab in da850_vpif_legacy_init(). Remove it.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
>
> Folding into the original,

Looks like the version in Sekhar's v4.11/soc branch already has this
fixed.

Kevin

Re: [PATCH 07/10] ARM: davinci: fix a whitespace error

[Kevin Hilman]

Bartosz Golaszewski  writes:

> There's a stray tab in da850_vpif_legacy_init(). Remove it.
>
> Signed-off-by: Bartosz Golaszewski 

Folding into the original,

Kevin

> ---
>  arch/arm/mach-davinci/pdata-quirks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/pdata-quirks.c 
> b/arch/arm/mach-davinci/pdata-quirks.c
> index a186513..94948c1 100644
> --- a/arch/arm/mach-davinci/pdata-quirks.c
> +++ b/arch/arm/mach-davinci/pdata-quirks.c
> @@ -111,7 +111,7 @@ static struct vpif_capture_config 
> da850_vpif_capture_config = {
>  static void __init da850_vpif_legacy_init(void)
>  {
>   int ret;
> - 
> +
>   /* LCDK doesn't have the 2nd TVP514x on CH1 */
>   if (of_machine_is_compatible("ti,da850-lcdk"))
>   da850_vpif_capture_config.subdev_count = 1;

Re: [PATCH 08/10] ARM: davinci: fix the DT boot on da850-evm

[Kevin Hilman]

Sekhar Nori <nsek...@ti.com> writes:

> On Tuesday 07 February 2017 11:51 PM, Kevin Hilman wrote:
>> Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:
>> 
>>> When we enable vpif capture on the da850-evm we hit a BUG_ON() because
>>> the i2c adapter can't be found. The board file boot uses i2c adapter 1
>>> but in the DT mode it's actually adapter 0. Drop the problematic lines.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
>>> ---
>>>  arch/arm/mach-davinci/pdata-quirks.c | 4 
>>>  1 file changed, 4 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-davinci/pdata-quirks.c 
>>> b/arch/arm/mach-davinci/pdata-quirks.c
>>> index 94948c1..09f62ac 100644
>>> --- a/arch/arm/mach-davinci/pdata-quirks.c
>>> +++ b/arch/arm/mach-davinci/pdata-quirks.c
>>> @@ -116,10 +116,6 @@ static void __init da850_vpif_legacy_init(void)
>>> if (of_machine_is_compatible("ti,da850-lcdk"))
>>> da850_vpif_capture_config.subdev_count = 1;
>>>  
>>> -   /* EVM (UI card) uses i2c adapter 1 (not default: zero) */
>>> -   if (of_machine_is_compatible("ti,da850-evm"))
>>> -   da850_vpif_capture_config.i2c_adapter_id = 1;
>>> -
>> 
>> oops, my bad.
>> 
>> Acked-by: Kevin Hilman <khil...@baylibre.com>
>
> The offending code is not in my master branch. Since its almost certain
> that VPIF platform support is going to wait for v4.12, can you or Kevin
> please update Kevin's original patches with these fixes rolled in?

I'm folding this into the original patch.

Kevin

Re: [PATCH 02/10] ARM: dts: da850-evm: fix whitespace errors

[Kevin Hilman]

Bartosz Golaszewski  writes:

> Signed-off-by: Bartosz Golaszewski 

I'll fold this one into the original since it's not yet merged.

Kevin

> ---
>  arch/arm/boot/dts/da850-evm.dts | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index c970b6e..94938a3 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -301,14 +301,14 @@
>   /* VPIF capture port */
>   port {
>   vpif_ch0: endpoint@0 {
> -   reg = <0>;
> -   bus-width = <8>;
> + reg = <0>;
> + bus-width = <8>;
>   };
>  
>   vpif_ch1: endpoint@1 {
> -   reg = <1>;
> -   bus-width = <8>;
> -   data-shift = <8>;
> + reg = <1>;
> + bus-width = <8>;
> + data-shift = <8>;
>   };
>   };
>  };

Re: [PATCH 02/10] ARM: dts: da850-evm: fix whitespace errors

[Kevin Hilman]

Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:

> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>

Acked-by: Kevin Hilman <khil...@baylibre.com>

> ---
>  arch/arm/boot/dts/da850-evm.dts | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index c970b6e..94938a3 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -301,14 +301,14 @@
>   /* VPIF capture port */
>   port {
>   vpif_ch0: endpoint@0 {
> -   reg = <0>;
> -   bus-width = <8>;
> + reg = <0>;
> + bus-width = <8>;
>   };
>  
>   vpif_ch1: endpoint@1 {
> -   reg = <1>;
> -   bus-width = <8>;
> -   data-shift = <8>;
> + reg = <1>;
> + bus-width = <8>;
> + data-shift = <8>;
>   };
>   };
>  };

Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:

> Extend the vpif node with an output port with a single channel.
>
> NOTE: this is still just hardware description - the actual driver
> is registered using pdata-quirks.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-evm.dts | 14 +++---
>  arch/arm/boot/dts/da850.dtsi|  8 +++-
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 94938a3..3d6dd66 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -299,16 +299,24 @@
>   status = "okay";
>  
>   /* VPIF capture port */
> - port {
> - vpif_ch0: endpoint@0 {
> + port@0 {
> + vpif_input_ch0: endpoint@0 {
>   reg = <0>;
>   bus-width = <8>;
>   };
>  
> - vpif_ch1: endpoint@1 {
> + vpif_input_ch1: endpoint@1 {
>   reg = <1>;
>   bus-width = <8>;
>   data-shift = <8>;
>   };
>   };
> +
> + /* VPIF display port */
> + port@1 {
> + vpif_output_ch0: endpoint@0 {
> + reg = <0>;
> + bus-width = <8>;
> + };
> + };
>  };
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 69ec5e7..768a58c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -494,7 +494,13 @@
>   status = "disabled";
>  
>   /* VPIF capture port */
> - port {
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + /* VPIF display port */
> + port@1 {
>   #address-cells = <1>;
>   #size-cells = <0>;
>   };

Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:

> There's a stray tab in da850_vpif_legacy_init(). Remove it.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
> ---
>  arch/arm/mach-davinci/pdata-quirks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/pdata-quirks.c 
> b/arch/arm/mach-davinci/pdata-quirks.c
> index a186513..94948c1 100644
> --- a/arch/arm/mach-davinci/pdata-quirks.c
> +++ b/arch/arm/mach-davinci/pdata-quirks.c
> @@ -111,7 +111,7 @@ static struct vpif_capture_config 
> da850_vpif_capture_config = {
>  static void __init da850_vpif_legacy_init(void)
>  {
>   int ret;
> - 
> +
>   /* LCDK doesn't have the 2nd TVP514x on CH1 */
>   if (of_machine_is_compatible("ti,da850-lcdk"))
>   da850_vpif_capture_config.subdev_count = 1;

Re: [PATCH 10/10] ARM: davinci: add pdata-quirks for da850-evm vpif display

[Kevin Hilman]

Hi Bartosz,

Bartosz Golaszewski  writes:

> Similarly to vpif capture: we need to register the vpif display driver
> and the corresponding adv7343 encoder in pdata-quirks as the DT
> support is not complete.
>
> Signed-off-by: Bartosz Golaszewski 
> ---
>  arch/arm/mach-davinci/pdata-quirks.c | 86 
> +++-
>  1 file changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/pdata-quirks.c 
> b/arch/arm/mach-davinci/pdata-quirks.c
> index 09f62ac..0a55546 100644
> --- a/arch/arm/mach-davinci/pdata-quirks.c
> +++ b/arch/arm/mach-davinci/pdata-quirks.c
> @@ -9,13 +9,17 @@
>   */
>  #include 
>  #include 
> +#include 
>  
>  #include 
> +#include 
>  
>  #include 
>  #include 
>  #include 
>  
> +#define DA850_EVM_UI_EXP_SEL_VPIF_DISP 5
> +
>  struct pdata_init {
>   const char *compatible;
>   void (*fn)(void);
> @@ -107,7 +111,78 @@ static struct vpif_capture_config 
> da850_vpif_capture_config = {
>   },
>   .card_name = "DA850/OMAP-L138 Video Capture",
>  };
> +#endif /* IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE) */
> +
> +#if defined(CONFIG_DA850_UI_SD_VIDEO_PORT)
> +static void vpif_evm_display_setup(void)
> +{
> + int gpio = DAVINCI_N_GPIO + DA850_EVM_UI_EXP_SEL_VPIF_DISP, ret;
> +
> + ret = gpio_request(gpio, "sel_c");
> + if (ret) {
> + pr_warn("Cannot open UI expander pin %d\n", gpio);
> + return;
> + }
> +
> + gpio_direction_output(gpio, 0);
> +}

I had a closer look at the UI board schematic, and it looks like the
SEL_C line of the GPIO exapander is actualy to select the VPIF *input*
source, not output, so I don't think it should be needed in this patch.

Can you test VPIF display works without calling this function?

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

Re: [PATCH 08/10] ARM: davinci: fix the DT boot on da850-evm

[Kevin Hilman]

Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:

> When we enable vpif capture on the da850-evm we hit a BUG_ON() because
> the i2c adapter can't be found. The board file boot uses i2c adapter 1
> but in the DT mode it's actually adapter 0. Drop the problematic lines.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
> ---
>  arch/arm/mach-davinci/pdata-quirks.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/pdata-quirks.c 
> b/arch/arm/mach-davinci/pdata-quirks.c
> index 94948c1..09f62ac 100644
> --- a/arch/arm/mach-davinci/pdata-quirks.c
> +++ b/arch/arm/mach-davinci/pdata-quirks.c
> @@ -116,10 +116,6 @@ static void __init da850_vpif_legacy_init(void)
>   if (of_machine_is_compatible("ti,da850-lcdk"))
>   da850_vpif_capture_config.subdev_count = 1;
>  
> - /* EVM (UI card) uses i2c adapter 1 (not default: zero) */
> - if (of_machine_is_compatible("ti,da850-evm"))
> -     da850_vpif_capture_config.i2c_adapter_id = 1;
> -

oops, my bad.

Acked-by: Kevin Hilman <khil...@baylibre.com>

>   ret = da850_register_vpif_capture(_vpif_capture_config);
>   if (ret)
>   pr_warn("%s: VPIF capture setup failed: %d\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] ARM: davinci: add pdata-quirks for da850-evm vpif display

[Kevin Hilman]

Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:

> Similarly to vpif capture: we need to register the vpif display driver
> and the corresponding adv7343 encoder in pdata-quirks as the DT
> support is not complete.

To add a bit more detail to the changelog:  DT support is not complete
since there isn't currently a way to define the output_routing in the
V4L2 drivers (c.f. s_routing) via DT.

> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>

minor nit below, otherwise

Reviewed-by: Kevin Hilman <khil...@baylibre.com>

> ---
>  arch/arm/mach-davinci/pdata-quirks.c | 86 
> +++-
>  1 file changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/pdata-quirks.c 
> b/arch/arm/mach-davinci/pdata-quirks.c
> index 09f62ac..0a55546 100644
> --- a/arch/arm/mach-davinci/pdata-quirks.c
> +++ b/arch/arm/mach-davinci/pdata-quirks.c
> @@ -9,13 +9,17 @@
>   */
>  #include 
>  #include 
> +#include 
>  
>  #include 
> +#include 
>  
>  #include 
>  #include 
>  #include 
>  
> +#define DA850_EVM_UI_EXP_SEL_VPIF_DISP 5
> +
>  struct pdata_init {
>   const char *compatible;
>   void (*fn)(void);
> @@ -107,7 +111,78 @@ static struct vpif_capture_config 
> da850_vpif_capture_config = {
>   },
>   .card_name = "DA850/OMAP-L138 Video Capture",
>  };
> +#endif /* IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE) */
> +
> +#if defined(CONFIG_DA850_UI_SD_VIDEO_PORT)

Why not IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE) also here?

> +static void vpif_evm_display_setup(void)
> +{
> + int gpio = DAVINCI_N_GPIO + DA850_EVM_UI_EXP_SEL_VPIF_DISP, ret;
> +
> + ret = gpio_request(gpio, "sel_c");
> + if (ret) {
> + pr_warn("Cannot open UI expander pin %d\n", gpio);
> + return;
> + }
> +
> + gpio_direction_output(gpio, 0);
> +}
> +
> +static struct adv7343_platform_data adv7343_pdata = {
> + .mode_config = {
> + .dac = { 1, 1, 1 },
> + },
> + .sd_config = {
> + .sd_dac_out = { 1 },
> + },
> +};
> +
> +static struct vpif_subdev_info da850_vpif_subdev[] = {
> + {
> + .name = "adv7343",
> + .board_info = {
> + I2C_BOARD_INFO("adv7343", 0x2a),
> + .platform_data = _pdata,
> + },
> + },
> +};
>  
> +static const struct vpif_output da850_ch0_outputs[] = {
> + {
> + .output = {
> + .index = 0,
> + .name = "Composite",
> + .type = V4L2_OUTPUT_TYPE_ANALOG,
> + .capabilities = V4L2_OUT_CAP_STD,
> + .std = V4L2_STD_ALL,
> + },
> + .subdev_name = "adv7343",
> + .output_route = ADV7343_COMPOSITE_ID,
> + },
> + {
> + .output = {
> + .index = 1,
> + .name = "S-Video",
> + .type = V4L2_OUTPUT_TYPE_ANALOG,
> + .capabilities = V4L2_OUT_CAP_STD,
> + .std = V4L2_STD_ALL,
> + },
> + .subdev_name = "adv7343",
> + .output_route = ADV7343_SVIDEO_ID,
> + },
> +};
> +
> +static struct vpif_display_config da850_vpif_display_config = {
> + .subdevinfo   = da850_vpif_subdev,
> + .subdev_count = ARRAY_SIZE(da850_vpif_subdev),
> + .chan_config[0] = {
> + .outputs = da850_ch0_outputs,
> + .output_count = ARRAY_SIZE(da850_ch0_outputs),
> + },
> + .card_name= "DA850/OMAP-L138 Video Display",
> +};
> +#endif /* defined(CONFIG_DA850_UI_SD_VIDEO_PORT) */
> +
> +#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE) || 
> defined(CONFIG_DA850_UI_SD_VIDEO_PORT)
>  static void __init da850_vpif_legacy_init(void)
>  {
>   int ret;
> @@ -120,8 +195,17 @@ static void __init da850_vpif_legacy_init(void)
>   if (ret)
>   pr_warn("%s: VPIF capture setup failed: %d\n",
>   __func__, ret);
> +
> + /* LCDK doesn't support VPIF display */
> + if (of_machine_is_compatible("ti,da850-evm")) {
> + vpif_evm_display_setup();
> + ret = da850_register_vpif_display(_vpif_display_config);
> + if (ret)
> + pr_warn("%s: VPIF display setup failed: %d\n",
> + __func__, ret);
> + }
>  }
> -#endif
> +#endif /* IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE) || 
> defined(CONFIG_DA850_UI_SD_VIDEO_PORT) */
>  
>  static void pdata_quirks_check(struct pdata_init *quirks)
>  {
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/10] media: vpif: use a configurable i2c_adapter_id for vpif display

[Kevin Hilman]

Bartosz Golaszewski <bgolaszew...@baylibre.com> writes:

> The vpif display driver uses a static i2c adapter ID of 1 but on the
> da850-evm board in DT boot mode the i2c adapter ID is actually 0.
>
> Make the adapter ID configurable like it already is for vpif capture.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>

Acked-by: Kevin Hilman <khil...@baylibre.com>

> ---
>  arch/arm/mach-davinci/board-da850-evm.c   | 1 +
>  drivers/media/platform/davinci/vpif_display.c | 2 +-
>  include/media/davinci/vpif_types.h| 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
> b/arch/arm/mach-davinci/board-da850-evm.c
> index e5d4ded..fe0bfa7 100644
> --- a/arch/arm/mach-davinci/board-da850-evm.c
> +++ b/arch/arm/mach-davinci/board-da850-evm.c
> @@ -1290,6 +1290,7 @@ static struct vpif_display_config 
> da850_vpif_display_config = {
>   .output_count = ARRAY_SIZE(da850_ch0_outputs),
>   },
>   .card_name= "DA850/OMAP-L138 Video Display",
> + .i2c_adapter_id = 1,
>  };
>  
>  static __init void da850_vpif_init(void)
> diff --git a/drivers/media/platform/davinci/vpif_display.c 
> b/drivers/media/platform/davinci/vpif_display.c
> index 50c3073..7e5cf99 100644
> --- a/drivers/media/platform/davinci/vpif_display.c
> +++ b/drivers/media/platform/davinci/vpif_display.c
> @@ -1287,7 +1287,7 @@ static __init int vpif_probe(struct platform_device 
> *pdev)
>   }
>  
>   if (!vpif_obj.config->asd_sizes) {
> - i2c_adap = i2c_get_adapter(1);
> + i2c_adap = i2c_get_adapter(vpif_obj.config->i2c_adapter_id);
>   for (i = 0; i < subdev_count; i++) {
>   vpif_obj.sd[i] =
>   v4l2_i2c_new_subdev_board(_obj.v4l2_dev,
> diff --git a/include/media/davinci/vpif_types.h 
> b/include/media/davinci/vpif_types.h
> index 4282a7d..0c72b46 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -57,6 +57,7 @@ struct vpif_display_config {
>   int (*set_clock)(int, int);
>   struct vpif_subdev_info *subdevinfo;
>   int subdev_count;
> + int i2c_adapter_id;
>   struct vpif_display_chan_config chan_config[VPIF_DISPLAY_MAX_CHANNELS];
>   const char *card_name;
>   struct v4l2_async_subdev **asd; /* Flat array, arranged in groups */
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/5] davinci: VPIF: add DT support

[Kevin Hilman]

On Fri, Dec 16, 2016 at 4:49 PM, Kevin Hilman <khil...@baylibre.com> wrote:
> Hans Verkuil <hverk...@xs4all.nl> writes:
>
>> On 07/12/16 19:30, Kevin Hilman wrote:
>>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>>> This series does some fixups/cleanups and then adds the DT binding and
>>> DT compatible string matching for DT probing.
>>>
>>> The controversial part from previous versions around async subdev
>>> parsing, and specifically hard-coding the input/output routing of
>>> subdevs, has been left out of this series.  That part can be done as a
>>> follow-on step after agreement has been reached on the path forward.
>>> With this version, platforms can still use the VPIF capture/display
>>> drivers, but must provide platform_data for the subdevs and subdev
>>> routing.
>>>
>>> Tested video capture to memory on da850-lcdk board using composite
>>> input.
>>
>> Other than the comment for the first patch this series looks good.
>>
>> So once that's addressed I'll queue it up for 4.11.
>
> I've fixed that issue, and sent an update for just that patch in reply
> to the original.
>
> Thanks for the review,

Gentle ping on this series.

I'm still not seeing this series yet in linux-next, so am worried it
might not make it for v4.11.

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

Re: [PATCH v6 0/5] davinci: VPIF: add DT support

[Kevin Hilman]

Hans Verkuil <hverk...@xs4all.nl> writes:

> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
>
> Other than the comment for the first patch this series looks good.
>
> So once that's addressed I'll queue it up for 4.11.

I've fixed that issue, and sent an update for just that patch in reply
to the original.

Thanks for the review,

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

[PATCH v6.1 1/5] [media] davinci: VPIF: fix module loading, init errors

[Kevin Hilman]

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
Minor tweaks since v6
- added ack from Sakari
- droped an extraneous change for NULL subdev_info

 drivers/media/platform/davinci/vpif.c |  5 -
 drivers/media/platform/davinci/vpif_capture.c | 13 +
 drivers/media/platform/davinci/vpif_display.c |  6 ++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME   "vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES 22
 #define VPIF_CH1_MAX_MODES 2
 #define VPIF_CH2_MAX_MODES 15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
.driver = {
-   .name   = "vpif",
+   .name   = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..892a26f3c5b4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
if (sd_index >= 0) {
sd = vpif_obj.sd[sd_index];
subdev_info = _cfg->subdev_info[sd_index];
+   } else {
+   /* no subdevice, no input to setup */
+   return 0;
}
 
/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c 
b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
err = initialize_vpif();
 
-- 
2.9.3

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

Re: [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors

[Kevin Hilman]

Hans Verkuil <hverk...@xs4all.nl> writes:

> On 07/12/16 19:30, Kevin Hilman wrote:
>> Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
>> fix various load-time errors cause by incorrect or not present
>> platform_data.
>>
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif.c |  5 -
>>  drivers/media/platform/davinci/vpif_capture.c | 15 ++-
>>  drivers/media/platform/davinci/vpif_display.c |  6 ++
>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif.c 
>> b/drivers/media/platform/davinci/vpif.c
>> index 0380cf2e5775..f50148dcba64 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -32,6 +32,9 @@
>>  MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>>  MODULE_LICENSE("GPL");
>>
>> +#define VPIF_DRIVER_NAME"vpif"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>> +
>>  #define VPIF_CH0_MAX_MODES  22
>>  #define VPIF_CH1_MAX_MODES  2
>>  #define VPIF_CH2_MAX_MODES  15
>> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>>
>>  static struct platform_driver vpif_driver = {
>>  .driver = {
>> -.name   = "vpif",
>> +.name   = VPIF_DRIVER_NAME,
>>  .pm = vpif_pm_ops,
>>  },
>>  .remove = vpif_remove,
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index 5104cc0ee40e..20c4344ed118 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>>  #define VPIF_DRIVER_NAME"vpif_capture"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>>
>>  /* global variables */
>>  static struct vpif_device vpif_obj = { {NULL} };
>> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>>
>>  vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>
>> +if (!chan_cfg)
>> +return -1;
>> +if (input_index >= chan_cfg->input_count)
>> +return -1;
>>  subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>  if (subdev_name == NULL)
>>  return -1;
>> @@ -654,7 +659,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 = _cfg->subdev_info[i];
>> -if (!strcmp(subdev_info->name, subdev_name))
>> +if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>
> Why this change? subdev_info can never be NULL.

A debugging leftover I guess.  Will remove and resend.

Thanks for the review,

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

Re: [PATCH v6 0/5] davinci: VPIF: add DT support

[Kevin Hilman]

Hi Javier,

Javier Martinez Canillas <jav...@dowhile0.org> writes:

> On Wed, Dec 7, 2016 at 3:30 PM, Kevin Hilman <khil...@baylibre.com> wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series.  That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>
> I had a similar need for another board (OMAP3 IGEPv2), that has a
> TVP5151 video decoder (that also supports 2 composite or 1 s-video
> signal) attached to the OMAP3 ISP.
>
> I posted some RFC patches [0] to define the input signals in the DT,
> and AFAICT Laurent and Hans were not against the approach but just had
> some comments on the DT binding.
>
> Basically they wanted the ports to be directly in the tvp5150 node
> instead of under a connectors sub-node [1] and to just be called just
> a (input / output) port instead of a connector [2].
>
> Unfortunately I was busy with other tasks so I couldn't res-pin the
> patches, but I think you could have something similar in the DT
> binding for your case and it shouldn't be hard to parse the ports /
> endpoints in the driver to get that information from DT and setup the
> input and output pins.

Thanks for pointing that out.  I did see this in Hans' reply to one of
my earlier versions.  Indeed I think this could be useful in solving my
problem.

>> With is version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>
> I guess DT backward compatibility isn't a big issue on this platform,
> since support for the platform is quite recently and after all someone
> who wants to use the vpif with current DT will need platform data and
> pdata-quirks anyways.

That's correct.

> So I agree with you that the input / output signals lookup from DT
> could be done as a follow-up.

Thanks. I'll happily add the input/output signals once they're agreed
upon.  In the mean time, at least we can have a usable video capture on
this platform, and it's at least a step in the right direction for DT
support.

Thanks for the review,

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

[PATCH v6 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id

[Kevin Hilman]

Remove hard-coded I2C adapter in favor of getting the
ID from platform_data.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 5 -
 include/media/davinci/vpif_types.h| 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 20c4344ed118..c24049acd40a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1486,7 +1486,10 @@ static __init int vpif_probe(struct platform_device 
*pdev)
}
 
if (!vpif_obj.config->asd_sizes) {
-   i2c_adap = i2c_get_adapter(1);
+   int i2c_id = vpif_obj.config->i2c_adapter_id;
+
+   i2c_adap = i2c_get_adapter(i2c_id);
+   WARN_ON(!i2c_adap);
for (i = 0; i < subdev_count; i++) {
subdevdata = _obj.config->subdev_info[i];
vpif_obj.sd[i] =
diff --git a/include/media/davinci/vpif_types.h 
b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4282a7db99d4 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -82,6 +82,7 @@ struct vpif_capture_config {
struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
struct vpif_subdev_info *subdev_info;
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 */
-- 
2.9.3

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

[PATCH v6 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

The IRQ-disabled locking is meant to protect the DMA queue list
throughout the rest of the driver, so update the locking in
[start|stop]_streaming to protect just this list, and update the irqlock
comment to reflect what it actually protects.

Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 6 +++---
 drivers/media/platform/davinci/vpif_capture.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index c24049acd40a..f72a719efb3d 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -179,8 +179,6 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
unsigned long addr, flags;
int ret;
 
-   spin_lock_irqsave(>irqlock, flags);
-
/* Initialize field_id */
ch->field_id = 0;
 
@@ -211,6 +209,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
vpif_config_addr(ch, ret);
 
/* Get the next frame from the buffer queue */
+   spin_lock_irqsave(>irqlock, flags);
common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
struct vpif_cap_buffer, list);
/* Remove buffer from the buffer queue */
@@ -244,6 +243,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
return 0;
 
 err:
+   spin_lock_irqsave(>irqlock, flags);
list_for_each_entry_safe(buf, tmp, >dma_queue, list) {
list_del(>list);
vb2_buffer_done(>vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -287,7 +287,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
vpif_dbg(1, debug, "stream off failed in subdev\n");
 
/* release all active buffers */
-   spin_lock_irqsave(>irqlock, flags);
if (common->cur_frm == common->next_frm) {
vb2_buffer_done(>cur_frm->vb.vb2_buf,
VB2_BUF_STATE_ERROR);
@@ -300,6 +299,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
VB2_BUF_STATE_ERROR);
}
 
+   spin_lock_irqsave(>irqlock, flags);
while (!list_empty(>dma_queue)) {
common->next_frm = list_entry(common->dma_queue.next,
struct vpif_cap_buffer, list);
diff --git a/drivers/media/platform/davinci/vpif_capture.h 
b/drivers/media/platform/davinci/vpif_capture.h
index 9e35b6771d22..1d2c052deedf 100644
--- a/drivers/media/platform/davinci/vpif_capture.h
+++ b/drivers/media/platform/davinci/vpif_capture.h
@@ -67,7 +67,7 @@ struct common_obj {
struct vb2_queue buffer_queue;
/* Queue of filled frames */
struct list_head dma_queue;
-   /* Used in video-buf */
+   /* Protects the dma_queue field */
spinlock_t irqlock;
/* lock used to access this structure */
struct mutex lock;
-- 
2.9.3

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

[PATCH v6 4/5] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Acked-by: Rob Herring <r...@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 .../devicetree/bindings/media/ti,da850-vpif.txt| 83 ++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index ..6d25d7f23d26
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,83 @@
+Texas Instruments VPIF
+--
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+Video Capture:
+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel.  It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+   vpif: vpif@217000 {
+   compatible = "ti,da850-vpif";
+   reg = <0x217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
+
+[ ... ]
+
+ {
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+};
+
+
+Alternatively, an example when the bus is configured as a single
+16-bit input (e.g. for raw-capture mode):
+
+   vpif: vpif@217000 {
+   compatible = "ti,da850-vpif";
+   reg = <0x217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint {
+ bus-width = <16>;
+   };
+   };
+   };
-- 
2.9.3

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

[PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors

[Kevin Hilman]

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c |  5 -
 drivers/media/platform/davinci/vpif_capture.c | 15 ++-
 drivers/media/platform/davinci/vpif_display.c |  6 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME   "vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES 22
 #define VPIF_CH1_MAX_MODES 2
 #define VPIF_CH2_MAX_MODES 15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
.driver = {
-   .name   = "vpif",
+   .name   = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..20c4344ed118 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -654,7 +659,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
if (sd_index >= 0) {
sd = vpif_obj.sd[sd_index];
subdev_info = _cfg->subdev_info[sd_index];
+   } else {
+   /* no subdevice, no input to setup */
+   return 0;
}
 
/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c 
b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
err = initialize_vpif();
 
-- 
2.9.3

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

[PATCH v6 0/5] davinci: VPIF: add DT support

[Kevin Hilman]

Prepare the groundwork for adding DT support for davinci VPIF drivers.
This series does some fixups/cleanups and then adds the DT binding and
DT compatible string matching for DT probing.

The controversial part from previous versions around async subdev
parsing, and specifically hard-coding the input/output routing of
subdevs, has been left out of this series.  That part can be done as a
follow-on step after agreement has been reached on the path forward.
With this version, platforms can still use the VPIF capture/display
drivers, but must provide platform_data for the subdevs and subdev
routing.

Tested video capture to memory on da850-lcdk board using composite
input.

Changes since v5:
- locking fix: updated comment around lock variable
- binding doc: added example for 
- added reviewed-by tags from Laurent (thanks!)

Changes since v4:
- dropped controversial async subdev parsing support.  That can be
  done as a follow-up step after the discussions have finalized on the
right approach.
- DT binding Acked by DT maintainer (Rob H.)
- reworked locking fix (suggested by Laurent)

Changes since v3:
- move to a single VPIF node, DT binding updated accordingly
- misc fixes/updates based on reviews from Sakari

Changes since v2:
- DT binding doc: fix example to use correct compatible

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream


Kevin Hilman (5):
  [media] davinci: VPIF: fix module loading, init errors
  [media] davinci: vpif_capture: remove hard-coded I2C adapter id
  [media] davinci: vpif_capture: fix start/stop streaming locking
  [media] dt-bindings: add TI VPIF documentation
  [media] davinci: VPIF: add basic support for DT init

 .../devicetree/bindings/media/ti,da850-vpif.txt| 83 ++
 drivers/media/platform/davinci/vpif.c  | 14 +++-
 drivers/media/platform/davinci/vpif_capture.c  | 26 +--
 drivers/media/platform/davinci/vpif_capture.h  |  2 +-
 drivers/media/platform/davinci/vpif_display.c  |  6 ++
 include/media/davinci/vpif_types.h |  1 +
 6 files changed, 125 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3

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

[PATCH v6 5/5] [media] davinci: VPIF: add basic support for DT init

[Kevin Hilman]

Add basic support for initialization via DT

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index f50148dcba64..1b02a6363f77 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -467,8 +467,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,da850-vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
-- 
2.9.3

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

Re: [PATCH v5 4/5] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Laurent Pinchart <laurent.pinch...@ideasonboard.com> writes:

> Hi Kevin,
>
> Thank you for the patch.
>
> On Tuesday 06 Dec 2016 21:08:25 Kevin Hilman wrote:
>> Acked-by: Rob Herring <r...@kernel.org>
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>> .../devicetree/bindings/media/ti,da850-vpif.txt| 67 +++
>> 1 file changed, 67 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file mode
>> 100644
>> index ..fa06dfdb6898
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> @@ -0,0 +1,67 @@
>> +Texas Instruments VPIF
>> +--
>> +
>> +The TI Video Port InterFace (VPIF) is the primary component for video
>> +capture and display on the DA850/AM18x family of TI DaVinci/Sitara
>> +SoCs.
>> +
>> +TI Document reference: SPRUH82C, Chapter 35
>> +http://www.ti.com/lit/pdf/spruh82
>> +
>> +Required properties:
>> +- compatible: must be "ti,da850-vpif"
>> +- reg: physical base address and length of the registers set for the
>> device;
>> +- interrupts: should contain IRQ line for the VPIF
>> +
>> +Video Capture:
>> +
>> +VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
>> +single 16-bit channel.  It should contain at least one port child node
>> +with child 'endpoint' node. Please refer to the bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> You might want to clarify how endpoints are use in the two cases. Apart from 
> that,

OK, I'll add another example for the 16-bit case.  Something like...

>> +Example using 2 8-bit input channels, one of which is connected to an
>> +I2C-connected TVP5147 decoder:
>> +
>> +vpif: vpif@217000 {
>> +compatible = "ti,da850-vpif";
>> +reg = <0x217000 0x1000>;
>> +interrupts = <92>;
>> +
>> +port {
>> +vpif_ch0: endpoint@0 {
>> +  reg = <0>;
>> +  bus-width = <8>;
>> +  remote-endpoint = <>;
>> +};
>> +
>> +vpif_ch1: endpoint@1 {
>> +  reg = <1>;
>> +  bus-width = <8>;
>> +  data-shift = <8>;
>> +};
>> +};
>> +};
>> +
>> +[ ... ]
>> +
>> + {
>> +
>> +tvp5147@5d {
>> +compatible = "ti,tvp5147";
>> +reg = <0x5d>;
>> +status = "okay";
>> +
>> +port {
>> +composite: endpoint {
>> +hsync-active = <1>;
>> +vsync-active = <1>;
>> +pclk-sample = <0>;
>> +
>> +/* VPIF channel 0 (lower 8-bits) */
>> +remote-endpoint = <_ch0>;
>> +bus-width = <8>;
>> +};
>> +};
>> +};
>> +};

...this, at the end of the binding doc:

Alternatively, an example when the bus is configured as a single
16-bit input (e.g. for raw-capture mode):

vpif: vpif@217000 {
compatible = "ti,da850-vpif";
reg = <0x217000 0x1000>;
interrupts = <92>;

port {
vpif_ch0: endpoint {
  bus-width = <16>;
};
};
};


Thanks for the review,

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

Re: [PATCH v5 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking

[Kevin Hilman]

Laurent Pinchart <laurent.pinch...@ideasonboard.com> writes:

> Hi Kevin,
>
> Thank you for the patch.
>
> On Tuesday 06 Dec 2016 21:08:24 Kevin Hilman wrote:
>> Video capture subdevs may be over I2C and may sleep during xfer, so we
>> cannot do IRQ-disabled locking when calling the subdev.
>> 
>> The IRQ-disabled locking is meant to protect the DMA queue list
>> throughout the rest of the driver, so update the locking in
>> [start|stop]_streaming to protect just this list.
>> 
>> Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>
> I would also add a comment to document the irqlock field as protecting the 
> dma_queue list only. Something like
>
> - /* Used in video-buf */
> + /* Protects the dma_queue field */
>   spinlock_t irqlock;
>
> With that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

OK, will update the comment.  Thanks for the review,

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

Re: [PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Laurent Pinchart <laurent.pinch...@ideasonboard.com> writes:

 > Hi Kevin,
>
> On Tuesday 06 Dec 2016 08:49:38 Kevin Hilman wrote:
>> Laurent Pinchart writes:
>> > On Tuesday 29 Nov 2016 15:57:09 Kevin Hilman wrote:
>> >> Video capture subdevs may be over I2C and may sleep during xfer, so we
>> >> cannot do IRQ-disabled locking when calling the subdev.
>> >> 
>> >> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> >> ---
>> >>  drivers/media/platform/davinci/vpif_capture.c | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >> 
>> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>> >> b/drivers/media/platform/davinci/vpif_capture.c index
>> >> 5104cc0ee40e..9f8f41c0f251 100644
>> >> --- a/drivers/media/platform/davinci/vpif_capture.c
>> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> >> @@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue
>> >> *vq, unsigned int count)
>> >>   }
>> >>   }
>> >> 
>> >> + spin_unlock_irqrestore(>irqlock, flags);
>> >>   ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
>> >> + spin_lock_irqsave(>irqlock, flags);
>> > 
>> > I always get anxious when I see a spinlock being released randomly with an
>> > operation in the middle of a protected section. Looking at the code it
>> > looks like the spinlock is abused here. irqlock should only protect the
>> > dma_queue and should thus only be taken around the following code:
>> > 
>> > spin_lock_irqsave(>irqlock, flags);
>> > /* Get the next frame from the buffer queue */
>> > common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
>> > struct vpif_cap_buffer, list);
>> > 
>> > /* Remove buffer from the buffer queue */
>> > list_del(>cur_frm->list);
>> > spin_unlock_irqrestore(>irqlock, flags);
>> 
>> Yes, that looks correct.  Will update.
>> 
>> > The code that is currently protected by the lock in the start and stop
>> > streaming functions should be protected by a mutex instead.
>> 
>> I tried taking the mutex here, but lockdep pointed out a deadlock.  I
>> may not be fully understanding the V4L2 internals here, but it seems
>> that the ioctl is already taking a mutex, so taking it again in
>> start/stop streaming is a deadlock.  Unless you think the locking should
>> be nested here, it seems to me that the mutex isn't needed.
>
> The V4L2 core can lock all ioctls using struct video_device::lock. For buffer-
> related ioctls, it can optionally use a separate lock from struct 
> vb2_queue::lock. See v4l2_ioctl_get_lock() in drivers/media/v4l2-core/v4l2-
> ioctl.c.
>
> The vpif-capture driver sets both the video_device and vb2_queue locks to the 
> same lock (which would have the same effect as leaving the vb2_queue lock 
> NULL). All ioctls are thus serialized. You would only need to handle locking 
> in start_streaming and stop_streaming manually if you didn't rely on the core 
> serializing the ioctls.

OK, thanks for clarifying how that works.

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

[PATCH v5 4/5] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Acked-by: Rob Herring <r...@kernel.org>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 .../devicetree/bindings/media/ti,da850-vpif.txt| 67 ++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index ..fa06dfdb6898
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,67 @@
+Texas Instruments VPIF
+--
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+Video Capture:
+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel.  It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+   vpif: vpif@217000 {
+   compatible = "ti,da850-vpif";
+   reg = <0x217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
+
+[ ... ]
+
+ {
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+};
-- 
2.9.3

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

[PATCH v5 5/5] [media] davinci: VPIF: add basic support for DT init

[Kevin Hilman]

Add basic support for initialization via DT

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index f50148dcba64..1b02a6363f77 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -467,8 +467,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,da850-vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
-- 
2.9.3

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

[PATCH v5 1/5] [media] davinci: VPIF: fix module loading, init errors

[Kevin Hilman]

Fix problems with automatic module loading by adding MODULE_ALIAS.  Also
fix various load-time errors cause by incorrect or not present
platform_data.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c |  5 -
 drivers/media/platform/davinci/vpif_capture.c | 15 ++-
 drivers/media/platform/davinci/vpif_display.c |  6 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
 MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
 MODULE_LICENSE("GPL");
 
+#define VPIF_DRIVER_NAME   "vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
 #define VPIF_CH0_MAX_MODES 22
 #define VPIF_CH1_MAX_MODES 2
 #define VPIF_CH2_MAX_MODES 15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
 
 static struct platform_driver vpif_driver = {
.driver = {
-   .name   = "vpif",
+   .name   = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..20c4344ed118 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -654,7 +659,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
if (sd_index >= 0) {
sd = vpif_obj.sd[sd_index];
subdev_info = _cfg->subdev_info[sd_index];
+   } else {
+   /* no subdevice, no input to setup */
+   return 0;
}
 
/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c 
b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
err = initialize_vpif();
 
-- 
2.9.3

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

[PATCH v5 0/5] davinci: VPIF: add DT support

[Kevin Hilman]

Prepare the groundwork for adding DT support for davinci VPIF drivers.
This series does some fixups/cleanups and then adds the DT binding and
DT compatible string matching for DT probing.

The controversial part from previous versions around async subdev
parsing, and specifically hard-coding the input/output routing of
subdevs, has been left out of this series.  That part can be done as a
follow-on step after agreement has been reached on the path forward.
With this version, platforms can still use the VPIF capture/display
drivers, but must provide platform_data for the subdevs and subdev
routing.

Tested video capture to memory on da850-lcdk board using composite
input.

Changes since v4:
- dropped controversial async subdev parsing support.  That can be
  done as a follow-up step after the discussions have finalized on the
  right approach.
- DT binding Acked by DT maintainer (Rob H.)
- reworked locking fix (suggested by Laurent)

Changes since v3:
- move to a single VPIF node, DT binding updated accordingly
- misc fixes/updates based on reviews from Sakari

Changes since v2:
- DT binding doc: fix example to use correct compatible

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream

Kevin Hilman (5):
  [media] davinci: VPIF: fix module loading, init errors
  [media] davinci: vpif_capture: remove hard-coded I2C adapter id
  [media] davinci: vpif_capture: fix start/stop streaming locking
  [media] dt-bindings: add TI VPIF documentation
  [media] davinci: VPIF: add basic support for DT init

 .../devicetree/bindings/media/ti,da850-vpif.txt| 67 ++
 drivers/media/platform/davinci/vpif.c  | 14 -
 drivers/media/platform/davinci/vpif_capture.c  | 26 +++--
 drivers/media/platform/davinci/vpif_display.c  |  6 ++
 include/media/davinci/vpif_types.h |  1 +
 5 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3

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

[PATCH v5 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

The IRQ-disabled locking is meant to protect the DMA queue list
throughout the rest of the driver, so update the locking in
[start|stop]_streaming to protect just this list.

Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index c24049acd40a..f72a719efb3d 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -179,8 +179,6 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
unsigned long addr, flags;
int ret;
 
-   spin_lock_irqsave(>irqlock, flags);
-
/* Initialize field_id */
ch->field_id = 0;
 
@@ -211,6 +209,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
vpif_config_addr(ch, ret);
 
/* Get the next frame from the buffer queue */
+   spin_lock_irqsave(>irqlock, flags);
common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
struct vpif_cap_buffer, list);
/* Remove buffer from the buffer queue */
@@ -244,6 +243,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
return 0;
 
 err:
+   spin_lock_irqsave(>irqlock, flags);
list_for_each_entry_safe(buf, tmp, >dma_queue, list) {
list_del(>list);
vb2_buffer_done(>vb.vb2_buf, VB2_BUF_STATE_QUEUED);
@@ -287,7 +287,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
vpif_dbg(1, debug, "stream off failed in subdev\n");
 
/* release all active buffers */
-   spin_lock_irqsave(>irqlock, flags);
if (common->cur_frm == common->next_frm) {
vb2_buffer_done(>cur_frm->vb.vb2_buf,
VB2_BUF_STATE_ERROR);
@@ -300,6 +299,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
VB2_BUF_STATE_ERROR);
}
 
+   spin_lock_irqsave(>irqlock, flags);
while (!list_empty(>dma_queue)) {
common->next_frm = list_entry(common->dma_queue.next,
struct vpif_cap_buffer, list);
-- 
2.9.3

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

[PATCH v5 2/5] [media] davinci: vpif_capture: remove hard-coded I2C adapter id

[Kevin Hilman]

Remove hard-coded I2C adapter in favor of getting the
ID from platform_data.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 5 -
 include/media/davinci/vpif_types.h| 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 20c4344ed118..c24049acd40a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1486,7 +1486,10 @@ static __init int vpif_probe(struct platform_device 
*pdev)
}
 
if (!vpif_obj.config->asd_sizes) {
-   i2c_adap = i2c_get_adapter(1);
+   int i2c_id = vpif_obj.config->i2c_adapter_id;
+
+   i2c_adap = i2c_get_adapter(i2c_id);
+   WARN_ON(!i2c_adap);
for (i = 0; i < subdev_count; i++) {
subdevdata = _obj.config->subdev_info[i];
vpif_obj.sd[i] =
diff --git a/include/media/davinci/vpif_types.h 
b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4282a7db99d4 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -82,6 +82,7 @@ struct vpif_capture_config {
struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
struct vpif_subdev_info *subdev_info;
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 */
-- 
2.9.3

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

Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

On Tue, Dec 6, 2016 at 9:40 AM, Kevin Hilman <khil...@baylibre.com> wrote:
> Hans Verkuil <hverk...@xs4all.nl> writes:
>
>> On 12/01/2016 10:16 AM, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> On Thursday 01 Dec 2016 09:57:31 Sakari Ailus wrote:
>>>> On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
>>>>> Sakari Ailus <sakari.ai...@iki.fi> writes:
>>>>>> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
>>>>>>> Sakari Ailus <sakari.ai...@iki.fi> writes:
>>>>>>>> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>>>>>>>>> Allow getting of subdevs from DT ports and endpoints.
>>>>>>>>>
>>>>>>>>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>>>>>>>>> am437x-vpfe.c
>>>>>>>>>
>>>>>>>>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>  drivers/media/platform/davinci/vpif_capture.c | 130 +++-
>>>>>>>>>  include/media/davinci/vpif_types.h
>>>>>>>>>|   9 +-
>>>>>>>>>  2 files changed, 133 insertions(+), 6 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>>>>>>>>> b/drivers/media/platform/davinci/vpif_capture.c index
>>>>>>>>> 94ee6cf03f02..47a4699157e7 100644
>>>>>>>>> --- a/drivers/media/platform/davinci/vpif_capture.c
>>>>>>>>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>>>>>>>>> @@ -26,6 +26,8 @@
>>>>>>>>>  #include 
>>>>>>>>>
>>>>>>>>>  #include 
>>>>>>>>> +#include 
>>>>>>>>> +#include 
>>>>>>>>
>>>>>>>> Do you need this header?
>>>>>>>
>>>>>>> Yes, based on discussion with Hans, since there is no DT binding for
>>>>>>> selecting the input pins of the TVP514x, I have to select it in the
>>>>>>> driver, so I need the defines from this header.  More on this below...
>>>
>>> That's really ugly :-( The problem should be fixed properly instead of 
>>> adding
>>> one more offender.
>>
>> Do you have time for that, Laurent? I don't. Until that time we just need to
>> make do with this workaround.
>>
>>>
>>>>>>>>>  #include "vpif.h"
>>>>>>>>>  #include "vpif_capture.h"
>>>>>>>>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>>>>>>>>>
>>>>>>>>>vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>>>>>>>>
>>>>>>>>> +  if (!chan_cfg)
>>>>>>>>> +  return -1;
>>>>>>>>> +  if (input_index >= chan_cfg->input_count)
>>>>>>>>> +  return -1;
>>>>>>>>>subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>>>>>>>>if (subdev_name == NULL)
>>>>>>>>>return -1;
>>>>>>>>> @@ -657,7 +663,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 = _cfg->subdev_info[i];
>>>>>>>>> -  if (!strcmp(subdev_info->name, subdev_name))
>>>>>>>>> +  if (subdev_info && !strcmp(subdev_info->name,
>>>>>>>>> subdev_name))
>>>>>>>>>return i;
>>>>>>>>>}
>>>>>>>>>return -1;
>>>>>>>>> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct
>>>>>>>>> v4l2_async_notifier *notifier,> >> >>
>>>>>>

Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Hans Verkuil <hverk...@xs4all.nl> writes:

> On 12/01/2016 10:16 AM, Laurent Pinchart wrote:
>> Hello,
>> 
>> On Thursday 01 Dec 2016 09:57:31 Sakari Ailus wrote:
>>> On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
>>>> Sakari Ailus <sakari.ai...@iki.fi> writes:
>>>>> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
>>>>>> Sakari Ailus <sakari.ai...@iki.fi> writes:
>>>>>>> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>>>>>>>> Allow getting of subdevs from DT ports and endpoints.
>>>>>>>>
>>>>>>>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>>>>>>>> am437x-vpfe.c
>>>>>>>>
>>>>>>>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>  drivers/media/platform/davinci/vpif_capture.c | 130 +++-
>>>>>>>>  include/media/davinci/vpif_types.h 
>>>>>>>>|   9 +-
>>>>>>>>  2 files changed, 133 insertions(+), 6 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>>>>>>>> b/drivers/media/platform/davinci/vpif_capture.c index
>>>>>>>> 94ee6cf03f02..47a4699157e7 100644
>>>>>>>> --- a/drivers/media/platform/davinci/vpif_capture.c
>>>>>>>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>>>>>>>> @@ -26,6 +26,8 @@
>>>>>>>>  #include 
>>>>>>>>
>>>>>>>>  #include 
>>>>>>>> +#include 
>>>>>>>> +#include 
>>>>>>>
>>>>>>> Do you need this header?
>>>>>>
>>>>>> Yes, based on discussion with Hans, since there is no DT binding for
>>>>>> selecting the input pins of the TVP514x, I have to select it in the
>>>>>> driver, so I need the defines from this header.  More on this below...
>> 
>> That's really ugly :-( The problem should be fixed properly instead of 
>> adding 
>> one more offender.
>
> Do you have time for that, Laurent? I don't. Until that time we just need to
> make do with this workaround.
>
>> 
>>>>>>>>  #include "vpif.h"
>>>>>>>>  #include "vpif_capture.h"
>>>>>>>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>>>>>>>>
>>>>>>>>vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>>>>>>>
>>>>>>>> +  if (!chan_cfg)
>>>>>>>> +  return -1;
>>>>>>>> +  if (input_index >= chan_cfg->input_count)
>>>>>>>> +  return -1;
>>>>>>>>subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>>>>>>>if (subdev_name == NULL)
>>>>>>>>return -1;
>>>>>>>> @@ -657,7 +663,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 = _cfg->subdev_info[i];
>>>>>>>> -  if (!strcmp(subdev_info->name, subdev_name))
>>>>>>>> +  if (subdev_info && !strcmp(subdev_info->name,
>>>>>>>> subdev_name))
>>>>>>>>return i;
>>>>>>>>}
>>>>>>>>return -1;
>>>>>>>> @@ -1327,6 +1333,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];
>>

Re: [PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Laurent Pinchart <laurent.pinch...@ideasonboard.com> writes:

> Hi Kevin,
>
> Thank you for the patch.
>
> On Tuesday 29 Nov 2016 15:57:09 Kevin Hilman wrote:
>> Video capture subdevs may be over I2C and may sleep during xfer, so we
>> cannot do IRQ-disabled locking when calling the subdev.
>> 
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif_capture.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>> b/drivers/media/platform/davinci/vpif_capture.c index
>> 5104cc0ee40e..9f8f41c0f251 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue *vq,
>> unsigned int count) }
>>  }
>> 
>> +spin_unlock_irqrestore(>irqlock, flags);
>>  ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
>> +spin_lock_irqsave(>irqlock, flags);
>
> I always get anxious when I see a spinlock being released randomly with an 
> operation in the middle of a protected section. Looking at the code it looks 
> like the spinlock is abused here. irqlock should only protect the dma_queue 
> and should thus only be taken around the following code:
>
> spin_lock_irqsave(>irqlock, flags);
> /* Get the next frame from the buffer queue */
> common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
> struct vpif_cap_buffer, list);
> /* Remove buffer from the buffer queue */
> list_del(>cur_frm->list);
> spin_unlock_irqrestore(>irqlock, flags);

Yes, that looks correct.  Will update.

> The code that is currently protected by the lock in the start and stop 
> streaming functions should be protected by a mutex instead.

I tried taking the mutex here, but lockdep pointed out a deadlock.  I
may not be fully understanding the V4L2 internals here, but it seems
that the ioctl is already taking a mutex, so taking it again in
start/stop streaming is a deadlock.  Unless you think the locking should
be nested here, it seems to me that the mutex isn't needed.

Kevin

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

Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Sakari Ailus <sakari.ai...@iki.fi> writes:

> Hi Kevin,
>
> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
>> Hi Sakari,
>> 
>> Sakari Ailus <sakari.ai...@iki.fi> writes:
>> 
>> > On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>> >> Allow getting of subdevs from DT ports and endpoints.
>> >> 
>> >> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> >
>> > vpif_capture_get_pdata and "largely"?
>> 
>> Yes, thanks.
>> 
>> >> am437x-vpfe.c
>> >> 
>> >> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> >> ---
>> >>  drivers/media/platform/davinci/vpif_capture.c | 130 
>> >> +-
>> >>  include/media/davinci/vpif_types.h|   9 +-
>> >>  2 files changed, 133 insertions(+), 6 deletions(-)
>> >> 
>> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> >> b/drivers/media/platform/davinci/vpif_capture.c
>> >> index 94ee6cf03f02..47a4699157e7 100644
>> >> --- a/drivers/media/platform/davinci/vpif_capture.c
>> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> >> @@ -26,6 +26,8 @@
>> >>  #include 
>> >>  
>> >>  #include 
>> >> +#include 
>> >> +#include 
>> >
>> > Do you need this header?
>> >
>> 
>> Yes, based on discussion with Hans, since there is no DT binding for
>> selecting the input pins of the TVP514x, I have to select it in the
>> driver, so I need the defines from this header.  More on this below...
>> 
>> >>  
>> >>  #include "vpif.h"
>> >>  #include "vpif_capture.h"
>> >> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>> >>  
>> >>   vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>> >>  
>> >> + if (!chan_cfg)
>> >> + return -1;
>> >> + if (input_index >= chan_cfg->input_count)
>> >> + return -1;
>> >>   subdev_name = chan_cfg->inputs[input_index].subdev_name;
>> >>   if (subdev_name == NULL)
>> >>   return -1;
>> >> @@ -657,7 +663,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 = _cfg->subdev_info[i];
>> >> - if (!strcmp(subdev_info->name, subdev_name))
>> >> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>> >>   return i;
>> >>   }
>> >>   return -1;
>> >> @@ -1327,6 +1333,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)) {
>> >> @@ -1422,6 +1443,110 @@ 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;
>

Re: [PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Sakari Ailus <sakari.ai...@iki.fi> writes:

> Hi Rob and Kevin,
>
> On Tue, Nov 29, 2016 at 08:41:44AM -0600, Rob Herring wrote:
>> On Mon, Nov 28, 2016 at 4:30 PM, Kevin Hilman <khil...@baylibre.com> wrote:
>> > Hi Rob,
>> >
>> > Rob Herring <r...@kernel.org> writes:
>> >
>> >> On Tue, Nov 22, 2016 at 07:52:44AM -0800, Kevin Hilman wrote:
>> >>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> >>> ---
>> >>>  .../bindings/media/ti,da850-vpif-capture.txt   | 65 
>> >>> ++
>> >>>  .../devicetree/bindings/media/ti,da850-vpif.txt|  8 +++
>> >>>  2 files changed, 73 insertions(+)
>> >>>  create mode 100644 
>> >>> Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> >>>  create mode 100644 
>> >>> Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> >>>
>> >>> diff --git 
>> >>> a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt 
>> >>> b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> >>> new file mode 100644
>> >>> index ..c447ac482c1d
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> >>> @@ -0,0 +1,65 @@
>> >>> +Texas Instruments VPIF Capture
>> >>> +--
>> >>> +
>> >>> +The TI Video Port InterFace (VPIF) capture component is the primary
>> >>> +component for video capture on the DA850 family of TI DaVinci SoCs.
>> >>> +
>> >>> +TI Document number reference: SPRUH82C
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: must be "ti,da850-vpif-capture"
>> >>> +- reg: physical base address and length of the registers set for the 
>> >>> device;
>> >>> +- interrupts: should contain IRQ line for the VPIF
>> >>> +
>> >>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
>> >>> +channels or a single 16-bit channel.  It should contain at least one
>> >>> +port child node with child 'endpoint' node. Please refer to the
>> >>> +bindings defined in
>> >>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> >>> +
>> >>> +Example using 2 8-bit input channels, one of which is connected to an
>> >>> +I2C-connected TVP5147 decoder:
>> >>> +
>> >>> +vpif_capture: video-capture@0x00217000 {
>> >>> +reg = <0x00217000 0x1000>;
>> >>> +interrupts = <92>;
>> >>> +
>> >>> +port {
>> >>> +vpif_ch0: endpoint@0 {
>> >>> +  reg = <0>;
>> >>> +  bus-width = <8>;
>> >>> +  remote-endpoint = <>;
>> >>> +};
>> >>> +
>> >>> +vpif_ch1: endpoint@1 {
>> >>
>> >> I think probably channels here should be ports rather than endpoints.
>> >> AIUI, having multiple endpoints is for cases like a mux or 1 to many
>> >> connections. There's only one data flow, but multiple sources or sinks.
>> >
>> > Looking at this closer... , I used an endpoint because it's bascially a
>> > 16-bit parallel bus, that can be configured as (up to) 2 8-bit
>> > "channels.  So, based on the video-interfaces.txt doc, I configured this
>> > as a single port, with (up to) 2 endpoints.  That also allows me to
>> > connect output of the decoder directly, using the remote-endpoint
>> > property.
>> >
>> > So I guess I'm not fully understanding your suggestion.
>> 
>> NM, looks like video-interfaces.txt actually spells out this case and
>> defines doing it as you did.
>
> It's actually the first time I read that portion (at least so that I could
> remember) of video-interfaces.txt. I'm not sure if anyone has implemented
> that previously, nor how we ended up with the text. The list archive could
> probably tell. Cc Guennadi who wrote it. :-) I couldn't immediately find DT
> source with this arrangement.
>
> In case of splitting the port into two parallel interfaces, how do you
> determine which wires belong to which endpoint? I guess they'd be particular
> sets of wires but as there's just a single port it isn't defined by the
> port.

Isn't that the point of data-shift?

e.g. it's a single 16-bit parallel bus, where the lower 8 bits are for
channel 0 and the upper 8 bits are for channel 1.  Alternately, the port
can also be configured as a single 16-bit channel (e.g. for raw
capture.)

If you want more details on this hardware, it's pretty well described in
Chapter 35 of http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf.

FWIW, I'm not really picky about how to do this.  I'm trying to learn
"the right way" and am happy to do that, but the feedback so far has
been confusing (at least for someone relatively new to the DT side of
the media framework.)

Kevin

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

[PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..9f8f41c0f251 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
}
 
+   spin_unlock_irqrestore(>irqlock, flags);
ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
+   spin_lock_irqsave(>irqlock, flags);
+
if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
vpif_dbg(1, debug, "stream on failed in subdev\n");
goto err;
-- 
2.9.3

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

[PATCH v4 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Allow getting of subdevs from DT ports and endpoints.

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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 138 +-
 include/media/davinci/vpif_types.h|   9 +-
 2 files changed, 141 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index a83df07e4051..4e363da2c21f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -651,6 +653,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -658,7 +664,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1328,6 +1334,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)) {
@@ -1423,6 +1444,118 @@ 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;
+
+   dev_dbg(>dev, "vpif_get_pdata\n");
+
+   /*
+* 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(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>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 = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>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;
+
+   /*
+* FIXME: need a new property for input/output ro

[PATCH v4 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 .../devicetree/bindings/media/ti,da850-vpif.txt| 67 ++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index ..fa06dfdb6898
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,67 @@
+Texas Instruments VPIF
+--
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+Video Capture:
+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel.  It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+   vpif: vpif@217000 {
+   compatible = "ti,da850-vpif";
+   reg = <0x217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
+
+[ ... ]
+
+ {
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+};
-- 
2.9.3

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

[PATCH v4 0/4] davinci: VPIF: add DT support

[Kevin Hilman]

Add DT support, including getting subdevs from DT ports/endpoints.

Tested video capture to memory on da850-lcdk board using composite
input.

Changes since v3:
- move to a single VPIF node, DT binding updated accordingly
- misc fixes/updates based on reviews from Sakari

Changes since v2:
- DT binding doc: fix example to use correct compatible

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream


Kevin Hilman (4):
  [media] davinci: vpif_capture: don't lock over s_stream
  [media] davinci: VPIF: add basic support for DT init
  [media] davinci: vpif_capture: get subdevs from DT
  [media] dt-bindings: add TI VPIF documentation

 .../devicetree/bindings/media/ti,da850-vpif.txt|  67 ++
 drivers/media/platform/davinci/vpif.c  |  48 ++-
 drivers/media/platform/davinci/vpif_capture.c  | 147 -
 drivers/media/platform/davinci/vpif_display.c  |   6 +
 include/media/davinci/vpif_types.h |   9 +-
 5 files changed, 270 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3

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

[PATCH v4 2/4] [media] davinci: VPIF: add basic support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

Because existing capture and display devices are implemented as separate
platform_devices, and in order to preserve that legacy support, during
DT boot, manually create capture and display platform devices to
initialize capture and display support.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c | 48 ++-
 drivers/media/platform/davinci/vpif_capture.c |  6 
 drivers/media/platform/davinci/vpif_display.c |  6 
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..528b30d52208 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -420,7 +420,8 @@ 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;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
vpif_base = devm_ioremap_resource(>dev, res);
@@ -432,6 +433,42 @@ static int vpif_probe(struct platform_device *pdev)
 
spin_lock_init(_lock);
dev_info(>dev, "vpif probe success\n");
+
+   if (!pdev->dev.of_node)
+   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(>dev, "Missing IRQ resource.\n");
+   return -EINVAL;
+   }
+
+   pdev_capture = devm_kzalloc(>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 = >dev;
+   platform_device_register(pdev_capture);
+
+   pdev_display = devm_kzalloc(>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 = >dev;
+   platform_device_register(pdev_display);
+
return 0;
 }
 
@@ -464,8 +501,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,da850-vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = "vpif",
.pm = vpif_pm_ops,
},
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 9f8f41c0f251..a83df07e4051 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* global variables */
 static struct vpif_device vpif_obj = { {NULL} };
@@ -1438,6 +1439,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c 
b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Debug level 0-1");
 
 #define VPIF_DRIVER_NAME   "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 
 /* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
 static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_

Re: [PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Hi Rob,

Rob Herring <r...@kernel.org> writes:

> On Tue, Nov 22, 2016 at 07:52:44AM -0800, Kevin Hilman wrote:
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  .../bindings/media/ti,da850-vpif-capture.txt   | 65 
>> ++
>>  .../devicetree/bindings/media/ti,da850-vpif.txt|  8 +++
>>  2 files changed, 73 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>>  create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt 
>> b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> new file mode 100644
>> index ..c447ac482c1d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> @@ -0,0 +1,65 @@
>> +Texas Instruments VPIF Capture
>> +--
>> +
>> +The TI Video Port InterFace (VPIF) capture component is the primary
>> +component for video capture on the DA850 family of TI DaVinci SoCs.
>> +
>> +TI Document number reference: SPRUH82C
>> +
>> +Required properties:
>> +- compatible: must be "ti,da850-vpif-capture"
>> +- reg: physical base address and length of the registers set for the device;
>> +- interrupts: should contain IRQ line for the VPIF
>> +
>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
>> +channels or a single 16-bit channel.  It should contain at least one
>> +port child node with child 'endpoint' node. Please refer to the
>> +bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example using 2 8-bit input channels, one of which is connected to an
>> +I2C-connected TVP5147 decoder:
>> +
>> +vpif_capture: video-capture@0x00217000 {
>> +reg = <0x00217000 0x1000>;
>> +interrupts = <92>;
>> +
>> +port {
>> +vpif_ch0: endpoint@0 {
>> +  reg = <0>;
>> +  bus-width = <8>;
>> +  remote-endpoint = <>;
>> +};
>> +
>> +vpif_ch1: endpoint@1 {
>
> I think probably channels here should be ports rather than endpoints. 
> AIUI, having multiple endpoints is for cases like a mux or 1 to many 
> connections. There's only one data flow, but multiple sources or sinks.

Looking at this closer... , I used an endpoint because it's bascially a
16-bit parallel bus, that can be configured as (up to) 2 8-bit
"channels.  So, based on the video-interfaces.txt doc, I configured this
as a single port, with (up to) 2 endpoints.  That also allows me to
connect output of the decoder directly, using the remote-endpoint
property.

So I guess I'm not fully understanding your suggestion.

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

Re: [PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Rob Herring <r...@kernel.org> writes:

> On Tue, Nov 22, 2016 at 07:52:44AM -0800, Kevin Hilman wrote:
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  .../bindings/media/ti,da850-vpif-capture.txt   | 65 
>> ++
>>  .../devicetree/bindings/media/ti,da850-vpif.txt|  8 +++
>>  2 files changed, 73 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>>  create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt 
>> b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> new file mode 100644
>> index ..c447ac482c1d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> @@ -0,0 +1,65 @@
>> +Texas Instruments VPIF Capture
>> +--
>> +
>> +The TI Video Port InterFace (VPIF) capture component is the primary
>> +component for video capture on the DA850 family of TI DaVinci SoCs.
>> +
>> +TI Document number reference: SPRUH82C
>> +
>> +Required properties:
>> +- compatible: must be "ti,da850-vpif-capture"
>> +- reg: physical base address and length of the registers set for the device;
>> +- interrupts: should contain IRQ line for the VPIF
>> +
>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
>> +channels or a single 16-bit channel.  It should contain at least one
>> +port child node with child 'endpoint' node. Please refer to the
>> +bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example using 2 8-bit input channels, one of which is connected to an
>> +I2C-connected TVP5147 decoder:
>> +
>> +vpif_capture: video-capture@0x00217000 {
>
> Drop the 0x00.
>
>> +compatible = "ti,da850-vpif-capture";
>> +reg = <0x00217000 0x1000>;
>> +interrupts = <92>;
>> +
>> +port {
>> +vpif_ch0: endpoint@0 {
>> +  reg = <0>;
>
> This is missing #size-cells and #addr-cells.
>

Yup.

>> +  bus-width = <8>;
>> +  remote-endpoint = <>;
>> +};
>> +
>> +vpif_ch1: endpoint@1 {
>
> I think probably channels here should be ports rather than endpoints. 
> AIUI, having multiple endpoints is for cases like a mux or 1 to many 
> connections. There's only one data flow, but multiple sources or sinks.

OK.

>> +  reg = <1>;
>> +  bus-width = <8>;
>> +  data-shift = <8>;
>> +};
>> +};
>> +};
>> +
>> +[ ... ]
>> +
>> + {
>> +
>> +tvp5147@5d {
>> +compatible = "ti,tvp5147";
>> +reg = <0x5d>;
>> +status = "okay";
>> +
>> +port {
>> +composite: endpoint {
>> +hsync-active = <1>;
>> +vsync-active = <1>;
>> +pclk-sample = <0>;
>> +
>> +/* VPIF channel 0 (lower 8-bits) */
>> +remote-endpoint = <_ch0>;
>> +bus-width = <8>;
>> +};
>> +};
>> +};
>> +
>> +};
>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
>> b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> new file mode 100644
>> index ..d004e600aabe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> @@ -0,0 +1,8 @@
>> +Texas Instruments VPIF
>> +--
>> +
>> +The Video Port InterFace (VPIF) is the core component for video output
>> +and capture on DA850 TI Davinci SoCs.
>> +
>> +- compatible: must be "ti,da850-vpif"
>> +- reg: physical base address and length of the registers set for the device;
>
> That's it? How does this block relate to the capture block?

I separated them because the current legacy drivers are separated into 3
different platform drivers.

However, after some discussions with Laurent, I'm going to just create a
single VPIF node with input (capture) and output (display) ports, and
then have to tweak the existing drivers a bit more than I had wanted to.

IOW, I was lazy.

Kevin

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

Re: [PATCH 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

On Fri, Nov 18, 2016 at 4:32 PM, Kevin Hilman <khil...@baylibre.com> wrote:
> Cc: Rob Herring <r...@kernel.org>
> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
> ---
>  .../devicetree/bindings/media/ti,vpif-capture.txt  | 65 
> ++
>  .../devicetree/bindings/media/ti,vpif.txt  |  8 +++
>  2 files changed, 73 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/ti,vpif-capture.txt
>  create mode 100644 Documentation/devicetree/bindings/media/ti,vpif.txt

@DT maintainers: this can be ignored, I'm reworking this after some
discussion with Laurent.

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

Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Hi Sakari,

Sakari Ailus <sakari.ai...@iki.fi> writes:

> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>> Allow getting of subdevs from DT ports and endpoints.
>> 
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>
> vpif_capture_get_pdata and "largely"?

Yes, thanks.

>> am437x-vpfe.c
>> 
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif_capture.c | 130 
>> +-
>>  include/media/davinci/vpif_types.h|   9 +-
>>  2 files changed, 133 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index 94ee6cf03f02..47a4699157e7 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -26,6 +26,8 @@
>>  #include 
>>  
>>  #include 
>> +#include 
>> +#include 
>
> Do you need this header?
>

Yes, based on discussion with Hans, since there is no DT binding for
selecting the input pins of the TVP514x, I have to select it in the
driver, so I need the defines from this header.  More on this below...

>>  
>>  #include "vpif.h"
>>  #include "vpif_capture.h"
>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>>  
>>  vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>  
>> +if (!chan_cfg)
>> +return -1;
>> +if (input_index >= chan_cfg->input_count)
>> +return -1;
>>  subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>  if (subdev_name == NULL)
>>  return -1;
>> @@ -657,7 +663,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 = _cfg->subdev_info[i];
>> -if (!strcmp(subdev_info->name, subdev_name))
>> +if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>>  return i;
>>  }
>>  return -1;
>> @@ -1327,6 +1333,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)) {
>> @@ -1422,6 +1443,110 @@ 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;
>> +
>> +dev_dbg(>dev, "vpif_get_pdata\n");
>> +
>> +if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>> +return pdev->dev.platform_data;
>> +
>> +pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
>> +if (!pdata)
>> +return NULL;
>> +pdata->subdev_info =
>> +devm_kzalloc(>dev, sizeof(*pdata->subdev_info) *
>> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
>> +
>> +if (!pdata->subdev_info)
>> +return NULL;
>> +dev_dbg(>dev, "%s\n", __func__);
>> +
>> +for (i = 0; ; i++) {
>> +struct device_node *rem;
>&

Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Hi Sakari,

Sakari Ailus <sakari.ai...@iki.fi> writes:

> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>> Allow getting of subdevs from DT ports and endpoints.
>> 
>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>
> vpif_capture_get_pdata and "largely"?

Yes, thanks.

>> am437x-vpfe.c
>> 
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif_capture.c | 130 
>> +-
>>  include/media/davinci/vpif_types.h|   9 +-
>>  2 files changed, 133 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index 94ee6cf03f02..47a4699157e7 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -26,6 +26,8 @@
>>  #include 
>>  
>>  #include 
>> +#include 
>> +#include 
>
> Do you need this header?
>

Yes, based on discussion with Hans, since there is no DT binding for
selecting the input pins of the TVP514x, I have to select it in the
driver, so I need the defines from this header.  More on this below...

>>  
>>  #include "vpif.h"
>>  #include "vpif_capture.h"
>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>>  
>>  vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>  
>> +if (!chan_cfg)
>> +return -1;
>> +if (input_index >= chan_cfg->input_count)
>> +return -1;
>>  subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>  if (subdev_name == NULL)
>>  return -1;
>> @@ -657,7 +663,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 = _cfg->subdev_info[i];
>> -if (!strcmp(subdev_info->name, subdev_name))
>> +if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>>  return i;
>>  }
>>  return -1;
>> @@ -1327,6 +1333,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)) {
>> @@ -1422,6 +1443,110 @@ 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;
>> +
>> +dev_dbg(>dev, "vpif_get_pdata\n");
>> +
>> +if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>> +return pdev->dev.platform_data;
>> +
>> +pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
>> +if (!pdata)
>> +return NULL;
>> +pdata->subdev_info =
>> +devm_kzalloc(>dev, sizeof(*pdata->subdev_info) *
>> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
>> +
>> +if (!pdata->subdev_info)
>> +return NULL;
>> +dev_dbg(>dev, "%s\n", __func__);
>> +
>> +for (i = 0; ; i++) {
>> +struct device_node *rem;
>&

[PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Allow getting of subdevs from DT ports and endpoints.

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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 130 +-
 include/media/davinci/vpif_types.h|   9 +-
 2 files changed, 133 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 94ee6cf03f02..47a4699157e7 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -657,7 +663,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1327,6 +1333,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)) {
@@ -1422,6 +1443,110 @@ 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;
+
+   dev_dbg(>dev, "vpif_get_pdata\n");
+
+   if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+   return pdev->dev.platform_data;
+
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>dev, sizeof(*pdata->subdev_info) *
+VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
+
+   if (!pdata->subdev_info)
+   return NULL;
+   dev_dbg(>dev, "%s\n", __func__);
+
+   for (i = 0; ; 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 = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>dev,
+   sizeof(*chan->inputs) *
+   VPIF_DISPLAY_MAX_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;
+
+   /* FIXME: need a new property? ch0:composite ch1: s-video */
+   if (i == 0)
+   chan->inputs[i].input_route = INPUT_CVBS_VI2B;
+   else
+   chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
+   chan->inputs[i].output_route = OUTPUT_10BIT_422_EMB

[PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 .../bindings/media/ti,da850-vpif-capture.txt   | 65 ++
 .../devicetree/bindings/media/ti,da850-vpif.txt|  8 +++
 2 files changed, 73 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
new file mode 100644
index ..c447ac482c1d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
@@ -0,0 +1,65 @@
+Texas Instruments VPIF Capture
+--
+
+The TI Video Port InterFace (VPIF) capture component is the primary
+component for video capture on the DA850 family of TI DaVinci SoCs.
+
+TI Document number reference: SPRUH82C
+
+Required properties:
+- compatible: must be "ti,da850-vpif-capture"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
+channels or a single 16-bit channel.  It should contain at least one
+port child node with child 'endpoint' node. Please refer to the
+bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+   vpif_capture: video-capture@0x00217000 {
+   compatible = "ti,da850-vpif-capture";
+   reg = <0x00217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
+
+[ ... ]
+
+ {
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+
+};
diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index ..d004e600aabe
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,8 @@
+Texas Instruments VPIF
+--
+
+The Video Port InterFace (VPIF) is the core component for video output
+and capture on DA850 TI Davinci SoCs.
+
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
-- 
2.9.3

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

[PATCH v3 1/4] [media] davinci: add support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c |  9 +
 drivers/media/platform/davinci/vpif_capture.c | 14 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..d4434f614141 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -464,8 +464,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,da850-vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = "vpif",
.pm = vpif_pm_ops,
},
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..87ee1e2c3864 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,6 +1435,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
@@ -1618,8 +1623,17 @@ static int vpif_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(vpif_pm_ops, vpif_suspend, vpif_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_capture_of_match[] = {
+   { .compatible = "ti,da850-vpif-capture", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_capture_of_match);
+#endif
+
 static __refdata struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_capture_of_match),
.name   = VPIF_DRIVER_NAME,
.pm = _pm_ops,
},
-- 
2.9.3

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

[PATCH v3 0/4] [media] davinci: VPIF: add DT support

[Kevin Hilman]

Add DT support, including getting subdevs from DT ports/endpoints.

Changes since v2:
- DT binding doc: fix example to use correct compatible

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream

Kevin Hilman (4):
  [media] davinci: add support for DT init
  [media] davinci: vpif_capture: don't lock over s_stream
  [media] davinci: vpif_capture: get subdevs from DT
  [media] dt-bindings: add TI VPIF documentation

 .../bindings/media/ti,da850-vpif-capture.txt   |  65 +
 .../devicetree/bindings/media/ti,da850-vpif.txt|   8 ++
 drivers/media/platform/davinci/vpif.c  |   9 ++
 drivers/media/platform/davinci/vpif_capture.c  | 147 -
 include/media/davinci/vpif_types.h |   9 +-
 5 files changed, 232 insertions(+), 6 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3

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

[PATCH v3 2/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 87ee1e2c3864..94ee6cf03f02 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
}
 
+   spin_unlock_irqrestore(>irqlock, flags);
ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
+   spin_lock_irqsave(>irqlock, flags);
+
if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
vpif_dbg(1, debug, "stream on failed in subdev\n");
goto err;
-- 
2.9.3

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

Re: [PATCH v2 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Hans Verkuil <hverk...@xs4all.nl> writes:

> On 22/11/16 02:44, Kevin Hilman wrote:
>> Cc: Rob Herring <r...@kernel.org>
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  .../bindings/media/ti,da850-vpif-capture.txt   | 65 
>> ++
>>  .../devicetree/bindings/media/ti,da850-vpif.txt|  8 +++
>>  2 files changed, 73 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>>  create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt 
>> b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> new file mode 100644
>> index ..bdd93267301f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> @@ -0,0 +1,65 @@
>> +Texas Instruments VPIF Capture
>> +--
>> +
>> +The TI Video Port InterFace (VPIF) capture component is the primary
>> +component for video capture on the DA850 family of TI DaVinci SoCs.
>> +
>> +TI Document number reference: SPRUH82C
>> +
>> +Required properties:
>> +- compatible: must be "ti,da850-vpif-capture"
>> +- reg: physical base address and length of the registers set for the device;
>> +- interrupts: should contain IRQ line for the VPIF
>> +
>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
>> +channels or a single 16-bit channel.  It should contain at least one
>> +port child node with child 'endpoint' node. Please refer to the
>> +bindings defined in
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example using 2 8-bit input channels, one of which is connected to an
>> +I2C-connected TVP5147 decoder:
>> +
>> +vpif_capture: video-capture@0x00217000 {
>> +compatible = "ti,vpif-capture";
>
> Did you forget to update the compatible string to ti,da850-vpif-capture?
>

Ugh, yup.   v3 coming right up.

Kevin


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

[PATCH v2 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Allow getting of subdevs from DT ports and endpoints.

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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 130 +-
 include/media/davinci/vpif_types.h|   9 +-
 2 files changed, 133 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 94ee6cf03f02..47a4699157e7 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -657,7 +663,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1327,6 +1333,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)) {
@@ -1422,6 +1443,110 @@ 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;
+
+   dev_dbg(>dev, "vpif_get_pdata\n");
+
+   if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+   return pdev->dev.platform_data;
+
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>dev, sizeof(*pdata->subdev_info) *
+VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
+
+   if (!pdata->subdev_info)
+   return NULL;
+   dev_dbg(>dev, "%s\n", __func__);
+
+   for (i = 0; ; 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 = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>dev,
+   sizeof(*chan->inputs) *
+   VPIF_DISPLAY_MAX_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;
+
+   /* FIXME: need a new property? ch0:composite ch1: s-video */
+   if (i == 0)
+   chan->inputs[i].input_route = INPUT_CVBS_VI2B;
+   else
+   chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
+   chan->inputs[i].output_route = OUTPUT_10BIT_422_EMB

[PATCH v2 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Cc: Rob Herring <r...@kernel.org>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 .../bindings/media/ti,da850-vpif-capture.txt   | 65 ++
 .../devicetree/bindings/media/ti,da850-vpif.txt|  8 +++
 2 files changed, 73 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
new file mode 100644
index ..bdd93267301f
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
@@ -0,0 +1,65 @@
+Texas Instruments VPIF Capture
+--
+
+The TI Video Port InterFace (VPIF) capture component is the primary
+component for video capture on the DA850 family of TI DaVinci SoCs.
+
+TI Document number reference: SPRUH82C
+
+Required properties:
+- compatible: must be "ti,da850-vpif-capture"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
+channels or a single 16-bit channel.  It should contain at least one
+port child node with child 'endpoint' node. Please refer to the
+bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+   vpif_capture: video-capture@0x00217000 {
+   compatible = "ti,vpif-capture";
+   reg = <0x00217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
+
+[ ... ]
+
+ {
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+
+};
diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt 
b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index ..d004e600aabe
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,8 @@
+Texas Instruments VPIF
+--
+
+The Video Port InterFace (VPIF) is the core component for video output
+and capture on DA850 TI Davinci SoCs.
+
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
-- 
2.9.3

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

[PATCH v2 1/4] [media] davinci: add support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c |  9 +
 drivers/media/platform/davinci/vpif_capture.c | 14 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..d4434f614141 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -464,8 +464,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,da850-vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = "vpif",
.pm = vpif_pm_ops,
},
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..87ee1e2c3864 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,6 +1435,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
@@ -1618,8 +1623,17 @@ static int vpif_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(vpif_pm_ops, vpif_suspend, vpif_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_capture_of_match[] = {
+   { .compatible = "ti,da850-vpif-capture", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_capture_of_match);
+#endif
+
 static __refdata struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_capture_of_match),
.name   = VPIF_DRIVER_NAME,
.pm = _pm_ops,
},
-- 
2.9.3

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

[PATCH v2 0/4] [media] davinci: VPIF: add DT support

[Kevin Hilman]

Add DT support, including getting subdevs from DT ports/endpoints.

Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream

Kevin Hilman (4):
  [media] davinci: add support for DT init
  [media] davinci: vpif_capture: don't lock over s_stream
  [media] davinci: vpif_capture: get subdevs from DT
  [media] dt-bindings: add TI VPIF documentation

 .../bindings/media/ti,da850-vpif-capture.txt   |  65 +
 .../devicetree/bindings/media/ti,da850-vpif.txt|   8 ++
 drivers/media/platform/davinci/vpif.c  |   9 ++
 drivers/media/platform/davinci/vpif_capture.c  | 147 -
 include/media/davinci/vpif_types.h |   9 +-
 5 files changed, 232 insertions(+), 6 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

-- 
2.9.3

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

[PATCH v2 2/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 87ee1e2c3864..94ee6cf03f02 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
}
 
+   spin_unlock_irqrestore(>irqlock, flags);
ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
+   spin_lock_irqsave(>irqlock, flags);
+
if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
vpif_dbg(1, debug, "stream on failed in subdev\n");
goto err;
-- 
2.9.3

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

Re: [PATCH 2/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Hans Verkuil <hverk...@xs4all.nl> writes:

> On 19/11/16 01:32, Kevin Hilman wrote:
>> Video capture subdevs may be over I2C and may sleep during xfer, so we
>> cannot do IRQ-disabled locking when calling the subdev.
>>
>> Signed-off-by: Kevin Hilman <khil...@baylibre.com>
>> ---
>>  drivers/media/platform/davinci/vpif_capture.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index 79cef74e164f..becc3e63b472 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
>> unsigned int count)
>>  }
>>  }
>>
>> +spin_unlock_irqrestore(>irqlock, flags);
>> +
>>  ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
>>  if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
>>  vpif_dbg(1, debug, "stream on failed in subdev\n");
>>  goto err;
>>  }
>>
>> +spin_lock_irqsave(>irqlock, flags);
>
> This needs to be moved to right after the v4l2_subdev_call, otherwise the
> goto err above will not have the spinlock.

Yes indeed.  Will respin.

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

Re: [PATCH 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Arnd Bergmann <a...@arndb.de> writes:

> On Friday, November 18, 2016 4:32:08 PM CET Kevin Hilman wrote:
>> +
>> +Required properties:
>> +- compatible: must be "ti,vpif-capture"
>> +- reg: physical base address and length of the registers set for the device;
>> +- interrupts: should contain IRQ line for the VPIF
>> +
>> 
>
> Shouldn't this have a SoC specific identifier or a version number
> in the compatible string? "vpif" seems rather generic, so it's
> likely that TI made more than one variant of it.

AFAICT, they used this for a single generation of davinci SoCs (dm6467,
da850) and then moved on to using something completely different.

But, that still proves your point because it's very SoC specific, so
I'll make the compatible specific.

Thanks for the review,

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

[PATCH 3/4] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

Allow getting of subdevs from DT ports and endpoints.

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

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 130 +-
 include/media/davinci/vpif_types.h|   9 +-
 2 files changed, 133 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index becc3e63b472..a0353d13eb92 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -651,6 +653,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -658,7 +664,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1328,6 +1334,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)) {
@@ -1423,6 +1444,110 @@ 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;
+
+   dev_dbg(>dev, "vpif_get_pdata\n");
+
+   if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+   return pdev->dev.platform_data;
+
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>dev, sizeof(*pdata->subdev_info) *
+VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
+
+   if (!pdata->subdev_info)
+   return NULL;
+   dev_dbg(>dev, "%s\n", __func__);
+
+   for (i = 0; ; 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 = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>dev,
+   sizeof(*chan->inputs) *
+   VPIF_DISPLAY_MAX_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;
+
+   /* FIXME: need a new property? ch0:composite ch1: s-video */
+   if (i == 0)
+   chan->inputs[i].input_route = INPUT_CVBS_VI2B;
+   else
+   chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
+   chan->inputs[i].output_route = OUTPUT_10BIT_422_EMB

[PATCH 4/4] [media] dt-bindings: add TI VPIF documentation

[Kevin Hilman]

Cc: Rob Herring <r...@kernel.org>
Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 .../devicetree/bindings/media/ti,vpif-capture.txt  | 65 ++
 .../devicetree/bindings/media/ti,vpif.txt  |  8 +++
 2 files changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,vpif-capture.txt
 create mode 100644 Documentation/devicetree/bindings/media/ti,vpif.txt

diff --git a/Documentation/devicetree/bindings/media/ti,vpif-capture.txt 
b/Documentation/devicetree/bindings/media/ti,vpif-capture.txt
new file mode 100644
index ..e46d3a5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,vpif-capture.txt
@@ -0,0 +1,65 @@
+Texas Instruments VPIF Capture
+--
+
+The TI Video Port InterFace (VPIF) capture component is the primary
+component for video capture on some TI DaVinci family SoCs.
+
+TI Document number reference: SPRUH82C
+
+Required properties:
+- compatible: must be "ti,vpif-capture"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
+channels or a single 16-bit channel.  It should contain at least one
+port child node with child 'endpoint' node. Please refer to the
+bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+   vpif_capture: video-capture@0x00217000 {
+   compatible = "ti,vpif-capture";
+   reg = <0x00217000 0x1000>;
+   interrupts = <92>;
+
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
+
+[ ... ]
+
+ {
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };
+   };
+
+};
diff --git a/Documentation/devicetree/bindings/media/ti,vpif.txt 
b/Documentation/devicetree/bindings/media/ti,vpif.txt
new file mode 100644
index ..0d5c16531c0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,vpif.txt
@@ -0,0 +1,8 @@
+Texas Instruments VPIF
+--
+
+The Video Port InterFace (VPIF) is the core component for video output
+and capture on TI Davinci family SoCs.
+
+- compatible: must be "ti,vpif"
+- reg: physical base address and length of the registers set for the device;
-- 
2.9.3

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

[PATCH 1/4] [media] davinci: add support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c |  9 +
 drivers/media/platform/davinci/vpif_capture.c | 14 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..077e328e0281 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -464,8 +464,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = "vpif",
.pm = vpif_pm_ops,
},
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..79cef74e164f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,6 +1435,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
@@ -1618,8 +1623,17 @@ static int vpif_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(vpif_pm_ops, vpif_suspend, vpif_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_capture_of_match[] = {
+   { .compatible = "ti,vpif-capture", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_capture_of_match);
+#endif
+
 static __refdata struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_capture_of_match),
.name   = VPIF_DRIVER_NAME,
.pm = _pm_ops,
},
-- 
2.9.3

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

[PATCH 2/4] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 79cef74e164f..becc3e63b472 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
}
 
+   spin_unlock_irqrestore(>irqlock, flags);
+
ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
vpif_dbg(1, debug, "stream on failed in subdev\n");
goto err;
}
 
+   spin_lock_irqsave(>irqlock, flags);
+
/* Call vpif_set_params function to set the parameters and addresses */
ret = vpif_set_video_params(vpif, ch->channel_id);
if (ret < 0) {
-- 
2.9.3

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

Re: [RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

Hans Verkuil <hverk...@xs4all.nl> writes:

> Hi Kevin,
>
> On 10/26/2016 01:55 AM, Kevin Hilman wrote:
>> This series attempts to add DT support to the davinci VPIF capture
>> driver.
>> 
>> I'm not sure I've completely grasped the proper use of the ports and
>> endpoints stuff, so this RFC is primarily to get input on whether I'm
>> on the right track.
>> 
>> The last patch is the one where all my questions are, the rest are
>> just prep work to ge there.
>> 
>> Tested on da850-lcdk and was able to do basic frame capture from the
>> composite input.
>> 
>> Series applies on v4.9-rc1
>> 
>> Kevin Hilman (6):
>>   [media] davinci: add support for DT init
>>   ARM: davinci: da8xx: VPIF: enable DT init
>>   ARM: dts: davinci: da850: add VPIF
>>   ARM: dts: davinci: da850-lcdk: enable VPIF capture
>>   [media] davinci: vpif_capture: don't lock over s_stream
>>   [media] davinci: vpif_capture: get subdevs from DT
>
> Looks good, but wouldn't it be better to do the dts changes last when all the
> supporting code is in?

I guess it doesn't really matter in this case, because the DT nodes will
be nops until the driver changes are in.

Either way, next week I'll repost a non-RFC version and separate out the
arch and DT patches, since those will go through Sekhar's davinci tree,
and then via arm-soc.

Thanks for the review,

Kevin

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

Re: [RFC PATCH 0/6] media: davinci: VPIF: add DT support

[Kevin Hilman]

Kevin Hilman <khil...@baylibre.com> writes:

> This series attempts to add DT support to the davinci VPIF capture
> driver.
>
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
>
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
>
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
>
> Series applies on v4.9-rc1

And FYI for anyone wanting to test, it needs a few config options
enabled[1] that are not (yet) part of davinci_all_defconfig.  I'll
update the defconfig when I'm ready to send non-RFC patches.

Kevin

[1]
CONFIG_MEDIA_SUPPORT=y
CONFIG_MEDIA_CAMERA_SUPPORT=y
CONFIG_VIDEO_DEV=y

CONFIG_VIDEO_V4L2=y
CONFIG_VIDEO_V4L2_SUBDEV_API=y

CONFIG_V4L_PLATFORM_DRIVERS=y
CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=y

# manually select codecs
CONFIG_MEDIA_SUBDRV_AUTOSELECT=n

# da850-lcdk
CONFIG_VIDEO_TVP514X=y
CONFIG_VIDEO_ADV7343=y
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 6/6] [media] davinci: vpif_capture: get subdevs from DT

[Kevin Hilman]

First pass at getting subdevs from DT ports and endpoints.

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

Questions:
- Legacy board file passes subdev input & output routes via pdata
  (e.g. tvp514x svideo or composite selection.)  How is this supposed
  to be done via DT?

Not-Yet-Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 132 +-
 include/media/davinci/vpif_types.h|   9 +-
 2 files changed, 134 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index becc3e63b472..df2af5cda37a 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
 #include 
 
 #include 
+#include 
+#include  /* FIXME: how to pass the INPUT_* OUTPUT* 
fields? */
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -651,6 +653,10 @@ static int vpif_input_to_subdev(
 
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
 
+   if (!chan_cfg)
+   return -1;
+   if (input_index >= chan_cfg->input_count)
+   return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -658,7 +664,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 = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1328,13 +1334,25 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
 {
int i;
 
+   for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
+   const struct device_node *node = 
vpif_obj.config->asd[i]->match.of.node;
+
+   if (node == subdev->of_node) {
+   vpif_obj.sd[i] = subdev;
+   vpif_obj.config->chan_config->inputs[i].subdev_name = 
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)) {
vpif_obj.sd[i] = subdev;
return 0;
}
-
+   
return -EINVAL;
 }
 
@@ -1423,6 +1441,113 @@ 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;
+
+   dev_dbg(>dev, "vpif_get_pdata\n");
+
+   if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+   return pdev->dev.platform_data;
+
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info = devm_kzalloc(>dev,
+ sizeof(*pdata->subdev_info) *
+ VPIF_CAPTURE_MAX_CHANNELS, 
GFP_KERNEL);
+   if (!pdata->subdev_info)
+   return NULL;
+   dev_dbg(>dev, "%s\n", __func__);
+
+   for (i = 0; ; i++) {
+   struct device_node *rem;
+   unsigned int flags;
+   int err;
+   
+   endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+ endpoint);
+   if (!endpoint)
+   break;
+
+   dev_dbg(>dev, "found endpoint %s, %s\n",
+   endpoint->name, endpoint->full_name);
+
+   sdinfo = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>dev,
+   sizeof(*chan->inputs) *
+   VPIF_DISPLAY_MAX_CHANNELS,
+   GFP_KERNEL);
+   
+   /* sdinfo->name = devm_kzalloc(>dev, 16, GFP_KERNEL); */
+   /* snprintf(sdinfo->name, 16

[RFC PATCH 4/6] ARM: dts: davinci: da850-lcdk: enable VPIF capture

[Kevin Hilman]

Enable video capture via the on-board TVP5147 decoder hooked up to ch0
one of the VPIF capture input.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21fed6c..ef3c2aa1b619 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -138,6 +138,24 @@
reg = <0x18>;
status = "okay";
};
+
+   tvp5147@5d {
+   compatible = "ti,tvp5147";
+   reg = <0x5d>;
+   status = "okay";
+
+   port {
+   composite: endpoint {
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <0>;
+
+   /* VPIF channel 0 (lower 8-bits) */
+   remote-endpoint = <_ch0>;
+   bus-width = <8>;
+   };
+   };  
+   };
 };
 
  {
@@ -219,3 +237,15 @@
};
};
 };
+
+ {
+   status = "okay";
+};
+
+_capture {
+   status = "okay";
+};
+
+_ch0 {
+   remote-endpoint = <>;
+};
-- 
2.9.3

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

[RFC PATCH 2/6] ARM: davinci: da8xx: VPIF: enable DT init

[Kevin Hilman]

Add basic support for DT initializaion of VPIF (capture) via DT.  Clocks
and mux still need to happen in this file until there are real clock and
pinctrl drivers, but the video nodes and subdevs can all come from DT.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c9f7e9274aa8..e1b7d72f9070 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -17,6 +17,7 @@
 #include 
 #include "cp_intc.h"
 #include 
+#include 
 
 static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
@@ -38,14 +39,30 @@ static struct of_dev_auxdata da850_auxdata_lookup[] 
__initdata = {
   NULL),
OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d0, "davinci-mcasp.0", 
NULL),
OF_DEV_AUXDATA("ti,da850-aemif", 0x6800, "ti-aemif", NULL),
+   OF_DEV_AUXDATA("ti,vpif", 0x01e17000, "vpif", NULL),
{}
 };
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
+#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE)
+static __init void da850_vpif_capture_init(void)
+{
+   int ret;
+   
+   ret = davinci_cfg_reg_list(da850_vpif_capture_pins);
+   if (ret)
+   pr_warn("da850_evm_init: VPIF capture mux setup failed: %d\n",
+   ret);
+}
+#else
+#define da850_vpif_capture_init()
+#endif
+
 static void __init da850_init_machine(void)
 {
of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
+   da850_vpif_capture_init();
 }
 
 static const char *const da850_boards_compat[] __initconst = {
-- 
2.9.3

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

[RFC PATCH 1/6] [media] davinci: add support for DT init

[Kevin Hilman]

Add basic support for initialization via DT.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif.c |  9 +
 drivers/media/platform/davinci/vpif_capture.c | 14 ++
 2 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..077e328e0281 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -464,8 +464,17 @@ static const struct dev_pm_ops vpif_pm = {
 #define vpif_pm_ops NULL
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+   { .compatible = "ti,vpif", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
 static struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_of_match),
.name   = "vpif",
.pm = vpif_pm_ops,
},
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..79cef74e164f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1435,6 +1435,11 @@ static __init int vpif_probe(struct platform_device 
*pdev)
int res_idx = 0;
int i, err;
 
+   if (!pdev->dev.platform_data) {
+   dev_warn(>dev, "Missing platform data.  Giving up.\n");
+   return -EINVAL;
+   }
+
vpif_dev = >dev;
 
err = initialize_vpif();
@@ -1618,8 +1623,17 @@ static int vpif_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(vpif_pm_ops, vpif_suspend, vpif_resume);
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_capture_of_match[] = {
+   { .compatible = "ti,vpif-capture", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_capture_of_match);
+#endif
+
 static __refdata struct platform_driver vpif_driver = {
.driver = {
+   .of_match_table = of_match_ptr(vpif_capture_of_match),
.name   = VPIF_DRIVER_NAME,
.pm = _pm_ops,
},
-- 
2.9.3

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

[RFC PATCH 5/6] [media] davinci: vpif_capture: don't lock over s_stream

[Kevin Hilman]

Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 drivers/media/platform/davinci/vpif_capture.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 79cef74e164f..becc3e63b472 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
}
 
+   spin_unlock_irqrestore(>irqlock, flags);
+
ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
vpif_dbg(1, debug, "stream on failed in subdev\n");
goto err;
}
 
+   spin_lock_irqsave(>irqlock, flags);
+
/* Call vpif_set_params function to set the parameters and addresses */
ret = vpif_set_video_params(vpif, ch->channel_id);
if (ret < 0) {
-- 
2.9.3

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

[RFC PATCH 3/6] ARM: dts: davinci: da850: add VPIF

[Kevin Hilman]

Add VPIF and VPIF capture nodes to da850.

Note that these are separate nodes because the current media drivers
have two separate drivers for vpif and vpif_capture.

Signed-off-by: Kevin Hilman <khil...@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 28 
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index f79e1b91c680..62c5b3e65071 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -399,7 +399,35 @@
< 0 1>;
dma-names = "tx", "rx";
};
+
+   vpif: video@0x00217000 {
+   compatible = "ti,vpif";
+   reg = <0x00217000 0x1000>;
+   status = "disabled";
+   };
+
+   vpif_capture: video-capture@0x00217000 {
+   compatible = "ti,vpif-capture";
+   reg = <0x00217000 0x1000>;
+   interrupts = <92>;
+   status = "disabled";
+
+   /* VPIF capture: input channels */
+   port {
+   vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+   };
+
+   vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+   };
+   };
+   };
};
+
aemif: aemif@6800 {
compatible = "ti,da850-aemif";
#address-cells = <2>;
-- 
2.9.3

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

Re: [PATCH 3/5] [media] rc: meson-ir: Fix module autoload

[Kevin Hilman]

On Mon, Oct 17, 2016 at 8:44 AM, Javier Martinez Canillas
<jav...@osg.samsung.com> wrote:
> If the driver is built as a module, autoload won't work because the module
> alias information is not filled. So user-space can't match the registered
> device with the corresponding module.
>
> Export the module alias information using the MODULE_DEVICE_TABLE() macro.
>
> Before this patch:
>
> $ modinfo drivers/media/rc/meson-ir.ko | grep alias
> $
>
> After this patch:
>
> $ modinfo drivers/media/rc/meson-ir.ko | grep alias
> alias:  of:N*T*Camlogic,meson-gxbb-irC*
> alias:  of:N*T*Camlogic,meson-gxbb-ir
> alias:  of:N*T*Camlogic,meson8b-irC*
> alias:  of:N*T*Camlogic,meson8b-ir
> alias:  of:N*T*Camlogic,meson6-irC*
> alias:  of:N*T*Camlogic,meson6-ir
>
> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

Acked-by: Kevin Hilman <khil...@baylibre.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/6] Add Meson 8b / GXBB support to the IR driver

[Kevin Hilman]

Martin Blumenstingl  writes:

> Newer Amlogic platforms (Meson 8b and GXBB) use a slightly different
> register layout for their Infrared Remoete Controller. The decoder mode
> is now configured in another register. Without the changes to the
> meson-ir driver we are simply getting incorrect "durations" reported
> from the hardware (because the hardware is not in time measurement aka
> software decode mode).
>
> This problem was also noticed by some people trying to use this on an
> ODROID-C1 and ODROID-C2 - the workaround there (probably because the
> datasheets were not publicy available yet at that time) was to switch
> to ir_raw_event_store_edge (which leaves it up to the kernel to measure
> the duration of a pulse). See [0] and [1] for the corresponding
> patches.
>
> Changes in v5:
> - changed pin function and group names to remote_input_ao so they match
>   with the datasheet
>
> Tested-by: Neil Armstrong 

Thanks for the respin.  I'll take the driver and DT/bindings through the
arm-soc tree and Linus has taken the pinctrl patch.

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

Re: [PATCH v4 4/6] media: rc: meson-ir: Add support for newer versions of the IR decoder

[Kevin Hilman]

Martin Blumenstingl <martin.blumensti...@googlemail.com> writes:

> From: Neil Armstrong <narmstr...@baylibre.com>
>
> Newer SoCs (Meson 8b and GXBB) are using REG2 (offset 0x20) instead of
> REG1 to configure the decoder mode. This makes it necessary to
> introduce new bindings so the driver knows which register has to be
> used.
>
> Signed-off-by: Neil Armstrong <narmstr...@baylibre.com>
> Signed-off-by: Martin Blumenstingl <martin.blumensti...@googlemail.com>

Acked-by: Kevin Hilman <khil...@baylibre.com>

Mauro, are you the one to pick up new media/rc drivers?  Or if you
prefer, with your ack, I'll take this along with the DT and submit via
arm-soc.

Thanks,

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

Re: [PATCH v4 1/6] pinctrl: amlogic: gxbb: add the IR remote pin

[Kevin Hilman]

Martin Blumenstingl <martin.blumensti...@googlemail.com> writes:

> This adds the IR remote receiver to the AO domain devices.

nit Re: Subject: should specify IR remote *input* pin.

> Signed-off-by: Martin Blumenstingl <martin.blumensti...@googlemail.com>
> ---
>  drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c 
> b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
> index cb4d6ad..8fffb31 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
> @@ -225,6 +225,8 @@ static const unsigned int i2c_sda_ao_pins[] = 
> {PIN(GPIOAO_5, 0) };
>  static const unsigned int i2c_slave_sck_ao_pins[] = {PIN(GPIOAO_4, 0) };
>  static const unsigned int i2c_slave_sda_ao_pins[] = {PIN(GPIOAO_5, 0) };
>  
> +static const unsigned int ir_in_ao_pins[] = {PIN(GPIOAO_7, 0) };
> +

I'm trying to keep the names here so they match the datasheet, which
calls this remote_input_ao.  Please update throughout the patch.

Otherwise looks good to me.  Feel free to add

Reviewed-by: Kevin Hilman <khil...@baylibre.com>

and Linus W can queue it up.

Thanks,

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

Re: [PATCH v3 0/4] Add Meson 8b / GXBB support to the IR driver

[Kevin Hilman]

Hi Martin,

On Tue, Jun 28, 2016 at 12:17 PM, Martin Blumenstingl
 wrote:
> Newer Amlogic platforms (Meson 8b and GXBB) use a slightly different
> register layout for their Infrared Remoete Controller. The decoder mode
> is now configured in another register. Without the changes to the
> meson-ir driver we are simply getting incorrect "durations" reported
> from the hardware (because the hardware is not in time measurement aka
> software decode mode).
>
> This problem was also noticed by some people trying to use this on an
> ODROID-C1 and ODROID-C2 - the workaround there (probably because the
> datasheets were not publicy available yet at that time) was to switch
> to ir_raw_event_store_edge (which leaves it up to the kernel to measure
> the duration of a pulse). See [0] and [1] for the corresponding
> patches.

I tried this on meson-gxbb-p200 and I'm not seeing any button press
events with evtest or ir-keytable when using the Amlogic remote that
came with the board.  Below is the register dump you requested on IRC:

[1.068347] Registered IR keymap rc-empty
[1.072422] input: meson-ir as
/devices/platform/soc/c810.aobus/c8100580.ir/rc/rc0/input0
[1.080814] rc rc0: meson-ir as
/devices/platform/soc/c810.aobus/c8100580.ir/rc/rc0
[1.088839] input: MCE IR Keyboard/Mouse (meson-ir) as
/devices/virtual/input/input1
[1.096519] rc rc0: lirc_dev: driver ir-lirc-codec (meson-ir)
registered at minor = 0
[1.104119] meson-ir c8100580.ir: receiver initialized
[1.109172] IR: reg 0x00 = 0x01d801ac
[1.112795] IR: reg 0x04 = 0x00f800ca
[1.116416] IR: reg 0x08 = 0x007a0066
[1.120037] IR: reg 0x0c = 0x0044002c
[1.123660] IR: reg 0x10 = 0x70fa0009
[1.127278] IR: reg 0x14 = 0x
[1.130907] IR: reg 0x18 = 0x08915c00
[1.134527] IR: reg 0x1c = 0x9f44
[1.138152] IR: reg 0x20 = 0x0002

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

Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration

[Kevin Hilman]

Hi Sakari,

Sakari Ailus sakari.ai...@iki.fi writes:

 The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to
 the actual CSI-2 receivers outside the ISP itself. Allow changing this
 configuration from the ISP driver.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi

These control module registers (CSIRXFE, CAMERA_PHY_CTRL) are in the
CORE powerdomain, so they will be lost during off-mode transitions.  So,
I suspect you'll also want to add them to the save/restore functions in
control.c in order for this to work across off-mode transitions.

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

Re: [PATCH v2 1/2] omap3: Provide means for changing CSI2 PHY configuration

[Kevin Hilman]

Sakari Ailus sakari.ai...@iki.fi writes:

 Hi Kevin,

 Thanks for the comments!

 On Tue, Oct 09, 2012 at 01:50:04PM -0700, Kevin Hilman wrote:
 Hi Sakari,
 
 Sakari Ailus sakari.ai...@iki.fi writes:
 
  The OMAP 3630 has configuration how the ISP CSI-2 PHY pins are connected to
  the actual CSI-2 receivers outside the ISP itself. Allow changing this
  configuration from the ISP driver.
 
  Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 
 These control module registers (CSIRXFE, CAMERA_PHY_CTRL) are in the
 CORE powerdomain, so they will be lost during off-mode transitions.  So,
 I suspect you'll also want to add them to the save/restore functions in
 control.c in order for this to work across off-mode transitions.

 I've got another patch that implements this in the ISP driver instead.

 URL:http://www.spinics.net/lists/linux-media/msg54781.html

Oops, sorry.  I should've looked at v3. 

 The ISP also can't wake up the MPU from the off mode, so I don't think
 losing the register contents is necessarily an issue. The registers will be
 written to a new value whenever streaming is started. Perhaps adding a note
 about that would be worthwhile.

Yes, I suggest mentioning that these register are not saved/restored and
why would be helpful.

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

Re: [PATCH v2 05/11] OMAP4: Add base addresses for ISS

[Kevin Hilman]

Sergio Aguirre saagui...@ti.com writes:

 NOTE: This isn't the whole list of features that the
 ISS supports, but the only ones supported at the moment.

 Signed-off-by: Sergio Aguirre saagui...@ti.com

[...]

 diff --git a/arch/arm/plat-omap/include/plat/omap44xx.h 
 b/arch/arm/plat-omap/include/plat/omap44xx.h
 index ea2b8a6..31432aa 100644
 --- a/arch/arm/plat-omap/include/plat/omap44xx.h
 +++ b/arch/arm/plat-omap/include/plat/omap44xx.h
 @@ -49,6 +49,15 @@
  #define OMAP44XX_MAILBOX_BASE(L4_44XX_BASE + 0xF4000)
  #define OMAP44XX_HSUSB_OTG_BASE  (L4_44XX_BASE + 0xAB000)
  
 +#define OMAP44XX_ISS_BASE0x5200
 +#define OMAP44XX_ISS_TOP_BASE(OMAP44XX_ISS_BASE + 
 0x0)
 +#define OMAP44XX_ISS_CSI2_A_REGS1_BASE   (OMAP44XX_ISS_BASE + 
 0x1000)
 +#define OMAP44XX_ISS_CAMERARX_CORE1_BASE (OMAP44XX_ISS_BASE + 0x1170)
 +
 +#define OMAP44XX_ISS_TOP_END (OMAP44XX_ISS_TOP_BASE + 256 - 
 1)
 +#define OMAP44XX_ISS_CSI2_A_REGS1_END
 (OMAP44XX_ISS_CSI2_A_REGS1_BASE + 368 - 1)
 +#define OMAP44XX_ISS_CAMERARX_CORE1_END  
 (OMAP44XX_ISS_CAMERARX_CORE1_BASE + 32 - 1)
 +
  #define OMAP4_MMU1_BASE  0x55082000
  #define OMAP4_MMU2_BASE  0x4A066000

Who are the users of thes address ranges?

IMO, we shouldn't ned to add anymore based address definitions.  These
should be done in the hwmod data, and drivers get base addresses using
the standard ways of getting resources (DT or platform_get_resource())

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

Re: [PATCH v2 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections

[Kevin Hilman]

+Benoit,

Aguirre, Sergio saagui...@ti.com writes:

 Hi Vaibhav,

 Thanks for the comments.

 On Thu, Dec 1, 2011 at 12:34 AM, Hiremath, Vaibhav hvaib...@ti.com wrote:

 -Original Message-
 From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
 ow...@vger.kernel.org] On Behalf Of Aguirre, Sergio
 Sent: Thursday, December 01, 2011 5:45 AM
 To: linux-media@vger.kernel.org
 Cc: linux-o...@vger.kernel.org; laurent.pinch...@ideasonboard.com;
 sakari.ai...@iki.fi; Aguirre, Sergio
 Subject: [PATCH v2 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory
 sections

 Signed-off-by: Sergio Aguirre saagui...@ti.com
 ---
  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   16 +---
  1 files changed, 13 insertions(+), 3 deletions(-)

 diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-
 omap2/omap_hwmod_44xx_data.c
 index 7695e5d..1b59e2f 100644
 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
 +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
 @@ -2623,8 +2623,18 @@ static struct omap_hwmod_ocp_if
 *omap44xx_iss_masters[] = {

  static struct omap_hwmod_addr_space omap44xx_iss_addrs[] = {
       {
 -             .pa_start       = 0x5200,
 -             .pa_end         = 0x52ff,
 +             .pa_start       = OMAP44XX_ISS_TOP_BASE,
 +             .pa_end         = OMAP44XX_ISS_TOP_END,
 +             .flags          = ADDR_TYPE_RT
 +     },
 +     {
 +             .pa_start       = OMAP44XX_ISS_CSI2_A_REGS1_BASE,
 +             .pa_end         = OMAP44XX_ISS_CSI2_A_REGS1_END,
 +             .flags          = ADDR_TYPE_RT
 +     },
 +     {
 +             .pa_start       = OMAP44XX_ISS_CAMERARX_CORE1_BASE,
 +             .pa_end         = OMAP44XX_ISS_CAMERARX_CORE1_END,
               .flags          = ADDR_TYPE_RT
       },
 This patch will result in build failure, because, the above base addresses
 are getting defined in the next patch

 [PATCH v2 05/11] OMAP4: Add base addresses for ISS

 Agreed. Will revisit git-bisectability of the patch series. Will fix.

To fix this, just drop the #defines from the header, and use raw
addresses directly.

Also, work with Benoit to make sure at the scripts that autogenerate
this data are updated to include these two regions.

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

Re: [PATCH v2 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections

[Kevin Hilman]

Aguirre, Sergio saagui...@ti.com writes:

[...]


 Also, work with Benoit to make sure at the scripts that autogenerate
 this data are updated to include these two regions.

 Ok.

 As a side note, I might need more addresses for the rest of the ISP
 components later on. I'll enable more subsystems once the CSI2-A only
 initial version is in an acceptable state.

That's fine, just work with Benoit to be sure that the autogen scripts
are updated for all the subsystems.

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

Re: [PATCH v3 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp

[Kevin Hilman]

Ramirez Luna, Omar omar.rami...@ti.com writes:

 Hi,

 On Fri, Nov 4, 2011 at 6:23 PM, Kevin Hilman khil...@ti.com wrote:
 +     .flags          = HWMOD_INIT_NO_RESET,

 Why is this needed?
 ...
 +     .flags          = HWMOD_INIT_NO_RESET,

 And this?

 I have this because the hwmod complains about a failure in hard reset,
 even though the reset deassert does complete after the clock is
 enabled. Later on, hwmod will warn again because of a wrong state when
 enabling, I believe because of the failure on _setup but didn't dig
 into it yet.

Please dig deeper into the reset failure.

We don't merge HWMOD_INIT_NO_RESET changes without a very good reason
(that is also well described in the changelog.)

Thanks,

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