Re: [PATCH v7 6/7] media: videodev2: add a flag for MC-centric devices

2017-09-28 Thread Sakari Ailus
On Wed, Sep 27, 2017 at 07:23:48PM -0300, Mauro Carvalho Chehab wrote:
> As both vdev-centric and MC-centric devices may implement the
> same APIs, we need a flag to allow userspace to distinguish
> between them.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v7 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

2017-09-28 Thread Sakari Ailus
On Wed, Sep 27, 2017 at 07:23:49PM -0300, Mauro Carvalho Chehab wrote:
> The documentation doesn't mention if vdev-centric hardware
> control would have subdev API or not.
> 
> Add a notice about that, reflecting the current status, where
> three drivers use it, in order to support some subdev-specific
> controls.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Thanks!

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


cron job: media_tree daily build: ERRORS

2017-09-28 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Fri Sep 29 05:00:17 CEST 2017
media-tree git hash:d5426f4c2ebac8cf05de43988c3fccddbee13d28
media_build git hash:   b829b621b4c2e6c5cbedbd1ce62b4e958f7d13a4
v4l-utils git hash: 8be65674f9a57e4bc35858f86bb5489f0afd22c1
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: [PATCH v3 3/5] media: atmel-isc: Enable the clocks during probe

2017-09-28 Thread Sakari Ailus
Hi Wenyou,

On Thu, Sep 28, 2017 at 04:18:26PM +0800, Wenyou Yang wrote:
> To meet the relationship, enable the HCLOCK and ispck during the
> device probe, "isc_pck frequency is less than or equal to isc_ispck,
> and isc_ispck is greater than or equal to HCLOCK."
> Meanwhile, call the pm_runtime_enable() in the right place.
> 
> Signed-off-by: Wenyou Yang 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/media/platform/atmel/atmel-isc.c | 31 +--
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/atmel/atmel-isc.c 
> b/drivers/media/platform/atmel/atmel-isc.c
> index 0b15dc1a3a0b..f092c95587c1 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -1594,6 +1594,7 @@ static int isc_async_complete(struct 
> v4l2_async_notifier *notifier)
>   struct isc_subdev_entity *sd_entity;
>   struct video_device *vdev = &isc->video_dev;
>   struct vb2_queue *q = &isc->vb2_vidq;
> + struct device *dev = isc->dev;
>   int ret;
>  
>   ret = v4l2_device_register_subdev_nodes(&isc->v4l2_dev);
> @@ -1677,6 +1678,10 @@ static int isc_async_complete(struct 
> v4l2_async_notifier *notifier)
>   return ret;
>   }
>  
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_request_idle(dev);

Remember that the driver's async complete function could never get called.

What would be the reason to move it here?

> +
>   return 0;
>  }
>  
> @@ -1856,25 +1861,37 @@ static int atmel_isc_probe(struct platform_device 
> *pdev)
>   return ret;
>   }
>  
> + ret = clk_prepare_enable(isc->hclock);
> + if (ret) {
> + dev_err(dev, "failed to enable hclock: %d\n", ret);
> + return ret;
> + }
> +
>   ret = isc_clk_init(isc);
>   if (ret) {
>   dev_err(dev, "failed to init isc clock: %d\n", ret);
> - goto clean_isc_clk;
> + goto unprepare_hclk;
>   }
>  
>   isc->ispck = isc->isc_clks[ISC_ISPCK].clk;
>  
> + ret = clk_prepare_enable(isc->ispck);
> + if (ret) {
> + dev_err(dev, "failed to enable ispck: %d\n", ret);
> + goto unprepare_hclk;
> + }
> +
>   /* ispck should be greater or equal to hclock */
>   ret = clk_set_rate(isc->ispck, clk_get_rate(isc->hclock));
>   if (ret) {
>   dev_err(dev, "failed to set ispck rate: %d\n", ret);
> - goto clean_isc_clk;
> + goto unprepare_clk;
>   }
>  
>   ret = v4l2_device_register(dev, &isc->v4l2_dev);
>   if (ret) {
>   dev_err(dev, "unable to register v4l2 device.\n");
> - goto clean_isc_clk;
> + goto unprepare_clk;
>   }
>  
>   ret = isc_parse_dt(dev, isc);
> @@ -1907,8 +1924,6 @@ static int atmel_isc_probe(struct platform_device *pdev)
>   break;
>   }
>  
> - pm_runtime_enable(dev);
> -
>   return 0;
>  
>  cleanup_subdev:
> @@ -1917,7 +1932,11 @@ static int atmel_isc_probe(struct platform_device 
> *pdev)
>  unregister_v4l2_device:
>   v4l2_device_unregister(&isc->v4l2_dev);
>  
> -clean_isc_clk:
> +unprepare_clk:
> + clk_disable_unprepare(isc->ispck);
> +unprepare_hclk:
> + clk_disable_unprepare(isc->hclock);

I think you're missing clk_disable_unprepare() in the driver's remove
callback.

> +
>   isc_clk_cleanup(isc);
>  
>   return ret;

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v10 20/24] dt: bindings: smiapp: Document lens-focus and flash properties

2017-09-28 Thread Sakari Ailus
Hi Rob,

On Tue, Sep 19, 2017 at 03:00:11PM -0500, Rob Herring wrote:
> On Mon, Sep 18, 2017 at 4:56 PM, Sakari Ailus
>  wrote:
> > Hi Rob,
> >
> >
> > Rob Herring wrote:
> >>
> >> On Mon, Sep 11, 2017 at 11:00:04AM +0300, Sakari Ailus wrote:
> >>>
> >>> Document optional lens-focus and flash properties for the smiapp driver.
> >>>
> >>> Signed-off-by: Sakari Ailus 
> >>> ---
> >>>  Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>
> >>
> >> Acked-by: Rob Herring 
> >
> >
> > Thanks for the ack. There have been since a few iterations of the set, and
> > the corresponding patch in v13 has minor changes to this:
> 
> My review script can't deal with subject changes...
> 
> > http://www.spinics.net/lists/linux-media/msg121929.html>
> >
> > Essentially "flash" was renamed to "flash-leds" as the current flash devices
> > we have are all LEDs and the referencing assumes LED framework's ways to
> > describe LEDs. The same change is present in the patch adding the property
> 
> So we're kind of creating a binding that mirrors the gpio bindings
> (*-gpios) which is a bit of an oddball as all other bindings have gone
> with a fixed property name and then a *-names property to name them.

It could be that "flash-leds" will remain the only one. Depending on
whether anyone would ever want to support a Xenon flash in which case we
could add a property for "flash-xenon" or such. Quite possibly not; LEDs
have improved in luminosity a lot over the recent years and aren't that far
from tiny Xenon flash devices. I don't remember seeing a mobile phone less
than five years or so with a Xenon flash.

> The main downside to this form is a prefixed property name is harder
> to parse and validate. So perhaps we should follow the more common
> pattern, but we're not really describing a h/w connection just an
> association. And now we also have the trigger source binding to
> associate LEDs with device nodes, so perhaps that should be used here.
> We shouldn't really have 2 ways to associate things in DT even if how
> that gets handled in the kernel is different.

trigger-sources is not really the same thing: it's present in the LED node,
not in the sensor to start with. The LED driver has no knowledge of the
Media device --- the sensor driver gains this information through the
endpoints. The sensor driver would need to find the node which contains a
phandle back to the sensor node.

Having this property in the sensor gives the association information
between the sensor and the LED. It could be present elsewhere, e.g. the
master device with DMA engines if there's no association with a sensor.

Some sensors do support strobing the flash, too. The flash type (Xenon or
LED) needs to be known to the strobe source (sensor).

The strobe wiring may not be there: the module vendors often omit that. Or
it could be omitted on the board. That's the case with LED flashes; they
can mostly be triggered using I²C as well albeit less precisely.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v2 12/13] scripts: kernel-doc: handle nested struct function arguments

2017-09-28 Thread Markus Heiser
Hi Mauro,

this 'else' addition seems a bit spooky to me. As I commented in patch 09/13
may it helps when you look at 

  https://github.com/return42/linuxdoc/blob/master/linuxdoc/kernel_doc.py#L2499

which is IMO a bit more clear.

-- Markus --

> Am 27.09.2017 um 23:10 schrieb Mauro Carvalho Chehab 
> :
> 
> Function arguments are different than usual ones. So, an
> special logic is needed in order to handle such arguments
> on nested structs.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
> scripts/kernel-doc | 38 ++
> 1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 713608046d3a..376365d41718 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1015,18 +1015,32 @@ sub dump_struct($$) {
>   $id =~ s/^\*+//;
>   foreach my $arg (split /;/, $content) {
>   next if ($arg =~ m/^\s*$/);
> - my $type = $arg;
> - my $name = $arg;
> - $type =~ s/\s\S+$//;
> - $name =~ s/.*\s//;
> - $name =~ s/[:\[].*//;
> - $name =~ s/^\*+//;
> - next if (($name =~ m/^\s*$/));
> - if ($id =~ m/^\s*$/) {
> - # anonymous struct/union
> - $newmember .= "$type $name;";
> + if ($arg =~ 
> m/^([^\(]+\(\*?\s*)([\w\.]*)(\s*\).*)/) {
> + # pointer-to-function
> + my $type = $1;
> + my $name = $2;
> + my $extra = $3;
> + next if (!$name);
> + if ($id =~ m/^\s*$/) {
> + # anonymous struct/union
> + $newmember .= 
> "$type$name$extra;";
> + } else {
> + $newmember .= 
> "$type$id.$name$extra;";
> + }
>   } else {
> - $newmember .= "$type $id.$name;";
> + my $type = $arg;
> + my $name = $arg;
> + $type =~ s/\s\S+$//;
> + $name =~ s/.*\s+//;
> + $name =~ s/[:\[].*//;
> + $name =~ s/^\*+//;
> + next if (($name =~ m/^\s*$/));
> + if ($id =~ m/^\s*$/) {
> + # anonymous struct/union
> + $newmember .= "$type $name;";
> + } else {
> + $newmember .= "$type 
> $id.$name;";
> + }
>   }
>   }
>   $members =~ 
> s/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/$newmember/;
> @@ -1215,7 +1229,7 @@ sub create_parameterlist() {
>   } elsif ($arg =~ m/\(.+\)\s*\(/) {
>   # pointer-to-function
>   $arg =~ tr/#/,/;
> - $arg =~ m/[^\(]+\(\*?\s*(\w*)\s*\)/;
> + $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
>   $param = $1;
>   $type = $arg;
>   $type =~ s/([^\(]+\(\*?)\s*$param/$1/;
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-doc" 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 09/13] scripts: kernel-doc: parse next structs/unions

2017-09-28 Thread Markus Heiser
Hi Mauro,

> Am 27.09.2017 um 23:10 schrieb Mauro Carvalho Chehab 
> :
> 
> 
> 
> diff --git a/Documentation/doc-guide/kernel-doc.rst 
> b/Documentation/doc-guide/kernel-doc.rst
> index 96012f9e314d..9e63a18cceea 100644
> --- a/Documentation/doc-guide/kernel-doc.rst
> +++ b/Documentation/doc-guide/kernel-doc.rst
> @@ -281,6 +281,52 @@ comment block.
> The kernel-doc data structure comments describe each member of the structure,
> in order, with the member descriptions.
> 
> +Nested structs/unions
> +~
> +
> +It is possible to document nested structs unions, like::
> +
> +  /**
> +   * struct nested_foobar - a struct with nested unions and structs
> +   * @arg1: - first argument of anonymous union/anonymous struct
> +   * @arg2: - second argument of anonymous union/anonymous struct
> +   * @arg3: - third argument of anonymous union/anonymous struct
> +   * @arg4: - fourth argument of anonymous union/anonymous struct
> +   * @bar.st1.arg1 - first argument of struct st1 on union bar
> +   * @bar.st1.arg2 - second argument of struct st1 on union bar
> +   * @bar.st2.arg1 - first argument of struct st2 on union bar
> +   * @bar.st2.arg2 - second argument of struct st2 on union bar

Sorry, this example is totally broken --> below I attached a more
elaborate example. I also untabified the example since tabs in reST are
a nightmare, especially in code blocks ... tabulators are the source
of all evil [1] ...

  Please, never use tabs in markups or programming languages
  where indentation is a part of the markup respectively the 
  language!!

> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index b6f3f6962897..63aa9f85d635 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -210,7 +210,7 @@ my $anon_struct_union = 0;
> my $type_constant = '\b``([^\`]+)``\b';
> my $type_constant2 = '\%([-_\w]+)';
> my $type_func = '(\w+)\(\)';
> -my $type_param = '\@(\w+(\.\.\.)?)';
> +my $type_param = '\@(\w*(\.\w+)*(\.\.\.)?)';
> my $type_fp_param = '\@(\w+)\(\)';  # Special RST handling for func ptr params
> my $type_env = '(\$\w+)';
> my $type_enum = '\&(enum\s*([_\w]+))';
> @@ -663,32 +663,12 @@ sub output_struct_man(%) {
>print ".SH NAME\n";
>print $args{'type'} . " " . $args{'struct'} . " \\- " . $args{'purpose'} . 
> "\n";
> 
> +my $declaration = $args{'definition'};
> +$declaration =~ s/\t/  /g;
> +$declaration =~ s/\n/"\n.br\n.BI \"/g;
>print ".SH SYNOPSIS\n";
>print $args{'type'} . " " . $args{'struct'} . " {\n.br\n";
> -
> -foreach my $parameter (@{$args{'parameterlist'}}) {
> - if ($parameter =~ /^#/) {
> - print ".BI \"$parameter\"\n.br\n";
> - next;
> - }
> - my $parameter_name = $parameter;
> - $parameter_name =~ s/\[.*//;
> -
> - ($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
> - $type = $args{'parametertypes'}{$parameter};
> - if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
> - # pointer-to-function
> - print ".BI \"" . $1 . "\" " . $parameter . " \") (" . $2 . ")" 
> . "\"\n;\n";
> - } elsif ($type =~ m/^(.*?)\s*(:.*)/) {
> - # bitfield
> - print ".BI \"" . $1 . "\ \" " . $parameter . $2 . " \"" . 
> "\"\n;\n";
> - } else {
> - $type =~ s/([^\*])$/$1 /;
> - print ".BI \"" . $type . "\" " . $parameter . " \"" . "\"\n;\n";
> - }
> - print "\n.br\n";
> -}
> -print "};\n.br\n";
> +print ".BI \"$declaration\n};\n.br\n\n";
> 
>print ".SH Members\n";
>foreach $parameter (@{$args{'parameterlist'}}) {
> @@ -926,29 +906,9 @@ sub output_struct_rst(%) {
> 
>print "**Definition**\n\n";
>print "::\n\n";
> -print "  " . $args{'type'} . " " . $args{'struct'} . " {\n";
> -foreach $parameter (@{$args{'parameterlist'}}) {
> - if ($parameter =~ /^#/) {
> - print "  " . "$parameter\n";
> - next;
> - }
> -
> - my $parameter_name = $parameter;
> - $parameter_name =~ s/\[.*//;
> -
> - ($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
> - $type = $args{'parametertypes'}{$parameter};
> - if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
> - # pointer-to-function
> - print "$1 $parameter) ($2);\n";
> - } elsif ($type =~ m/^(.*?)\s*(:.*)/) {
> - # bitfield
> - print "$1 $parameter$2;\n";
> - } else {
> - print "" . $type . " " . $parameter . ";\n";
> - }
> -}
> -print "  };\n\n";
> +my $declaration = $args{'definition'};
> +$declaration =~ s/\t/  /g;
> +print "  " . $args{'type'} . " " . $args{'struct'} . " {\n$declaration  
> };\n\n";
> 
>print "**Members**\n\n";
>$lineprefix = "  ";
> @@ -1022,20 +982,15 @@ sub dump_struct($$) {
>my $nested;
> 
>if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
> - #my $decl_type = $1;
> + my $decl_type = $1;
>   $declaration_name = $2;
>   m

[PATCH v2 1/2] v4l: Add support for V4L2_BUF_TYPE_META_OUTPUT

2017-09-28 Thread Sakari Ailus
The V4L2_BUF_TYPE_META_OUTPUT mirrors the V4L2_BUF_TYPE_META_CAPTURE with
the exception that it is an OUTPUT type. The use case for this is to pass
buffers to the device that are not image data but metadata. The formats,
just as the metadata capture formats, are typically device specific and
highly structured.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
Reviewed-by: Tomasz Figa 
Tested-by: Tian Shu Qiu 
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  2 ++
 drivers/media/v4l2-core/v4l2-dev.c| 12 
 drivers/media/v4l2-core/v4l2-ioctl.c  | 25 +
 drivers/media/v4l2-core/videobuf2-v4l2.c  |  1 +
 include/media/v4l2-ioctl.h| 17 +
 include/uapi/linux/videodev2.h|  2 ++
 6 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 821f2aa..9f5c7d4 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -235,6 +235,7 @@ static int __get_v4l2_format32(struct v4l2_format *kp, 
struct v4l2_format32 __us
case V4L2_BUF_TYPE_SDR_OUTPUT:
return get_v4l2_sdr_format(&kp->fmt.sdr, &up->fmt.sdr);
case V4L2_BUF_TYPE_META_CAPTURE:
+   case V4L2_BUF_TYPE_META_OUTPUT:
return get_v4l2_meta_format(&kp->fmt.meta, &up->fmt.meta);
default:
pr_info("compat_ioctl32: unexpected VIDIOC_FMT type %d\n",
@@ -284,6 +285,7 @@ static int __put_v4l2_format32(struct v4l2_format *kp, 
struct v4l2_format32 __us
case V4L2_BUF_TYPE_SDR_OUTPUT:
return put_v4l2_sdr_format(&kp->fmt.sdr, &up->fmt.sdr);
case V4L2_BUF_TYPE_META_CAPTURE:
+   case V4L2_BUF_TYPE_META_OUTPUT:
return put_v4l2_meta_format(&kp->fmt.meta, &up->fmt.meta);
default:
pr_info("compat_ioctl32: unexpected VIDIOC_FMT type %d\n",
diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index c647ba6..3660cb8 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -581,7 +581,8 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
   ops->vidioc_enum_fmt_vid_overlay ||
   ops->vidioc_enum_fmt_meta_cap)) ||
(is_tx && (ops->vidioc_enum_fmt_vid_out ||
-  ops->vidioc_enum_fmt_vid_out_mplane)))
+  ops->vidioc_enum_fmt_vid_out_mplane ||
+  ops->vidioc_enum_fmt_meta_out)))
set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
   ops->vidioc_g_fmt_vid_cap_mplane ||
@@ -589,7 +590,8 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
   ops->vidioc_g_fmt_meta_cap)) ||
(is_tx && (ops->vidioc_g_fmt_vid_out ||
   ops->vidioc_g_fmt_vid_out_mplane ||
-  ops->vidioc_g_fmt_vid_out_overlay)))
+  ops->vidioc_g_fmt_vid_out_overlay ||
+  ops->vidioc_g_fmt_meta_out)))
 set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
   ops->vidioc_s_fmt_vid_cap_mplane ||
@@ -597,7 +599,8 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
   ops->vidioc_s_fmt_meta_cap)) ||
(is_tx && (ops->vidioc_s_fmt_vid_out ||
   ops->vidioc_s_fmt_vid_out_mplane ||
-  ops->vidioc_s_fmt_vid_out_overlay)))
+  ops->vidioc_s_fmt_vid_out_overlay ||
+  ops->vidioc_s_fmt_meta_out)))
 set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
   ops->vidioc_try_fmt_vid_cap_mplane ||
@@ -605,7 +608,8 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
   ops->vidioc_try_fmt_meta_cap)) ||
(is_tx && (ops->vidioc_try_fmt_vid_out ||
   ops->vidioc_try_fmt_vid_out_mplane ||
-  ops->vidioc_try_fmt_vid_out_overlay)))
+  ops->vidioc_try_fmt_vid_out_overlay ||
+  ops->vidioc_try_fmt_meta_out)))
 set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2

[PATCH v2 0/2] Add V4L2_BUF_TYPE_META_OUTPUT buffer type

2017-09-28 Thread Sakari Ailus
Hi folks,

Here's a second non-RFC version of the META_OUTPUT buffer type patches.

The V4L2_BUF_TYPE_META_OUTPUT buffer type complements the metadata buffer
types support for OUTPUT buffers, capture being already supported. This is
intended for similar cases than V4L2_BUF_TYPE_META_CAPTURE but for output
buffers, e.g. device parameters that may be complex and highly
hierarchical data structure. Statistics are a current use case for
metadata capture buffers.

Yong: could you take these to your IPU3 ImgU patchset, please? As that
would be the first user, the patches would be merged with the driver
itself.

since v1:

- Correctly determine valid IOCTLs for META_OUTPUT type in
  determine_valid_ioctls().

since RFC:

- Fix make htmldocs build.

- Fix CAPTURE -> OUTPUT in buffer.rst.

- Added " for specifying how the device processes images" in the
  documentation.

Sakari Ailus (2):
  v4l: Add support for V4L2_BUF_TYPE_META_OUTPUT
  docs-rst: v4l: Document V4L2_BUF_TYPE_META_OUTPUT interface

 Documentation/media/uapi/v4l/buffer.rst  |  3 +++
 Documentation/media/uapi/v4l/dev-meta.rst| 33 ++--
 Documentation/media/uapi/v4l/vidioc-querycap.rst |  3 +++
 Documentation/media/videodev2.h.rst.exceptions   |  2 ++
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c|  2 ++
 drivers/media/v4l2-core/v4l2-dev.c   | 12 ++---
 drivers/media/v4l2-core/v4l2-ioctl.c | 25 ++
 drivers/media/v4l2-core/videobuf2-v4l2.c |  1 +
 include/media/v4l2-ioctl.h   | 17 
 include/uapi/linux/videodev2.h   |  2 ++
 10 files changed, 83 insertions(+), 17 deletions(-)

-- 
2.7.4



[PATCH v2 2/2] docs-rst: v4l: Document V4L2_BUF_TYPE_META_OUTPUT interface

2017-09-28 Thread Sakari Ailus
Document the interface for metadata output, including
V4L2_BUF_TYPE_META_OUTPUT buffer type and V4L2_CAP_META_OUTPUT capability
bits.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
Reviewed-by: Tomasz Figa 
Tested-by: Tian Shu Qiu 
---
 Documentation/media/uapi/v4l/buffer.rst  |  3 +++
 Documentation/media/uapi/v4l/dev-meta.rst| 33 ++--
 Documentation/media/uapi/v4l/vidioc-querycap.rst |  3 +++
 Documentation/media/videodev2.h.rst.exceptions   |  2 ++
 4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index ae6ee73..33b932e 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -452,6 +452,9 @@ enum v4l2_buf_type
 * - ``V4L2_BUF_TYPE_META_CAPTURE``
   - 13
   - Buffer for metadata capture, see :ref:`metadata`.
+* - ``V4L2_BUF_TYPE_META_OUTPUT``
+  - 14
+  - Buffer for metadata output, see :ref:`metadata`.
 
 
 
diff --git a/Documentation/media/uapi/v4l/dev-meta.rst 
b/Documentation/media/uapi/v4l/dev-meta.rst
index f7ac8d0..93269d2 100644
--- a/Documentation/media/uapi/v4l/dev-meta.rst
+++ b/Documentation/media/uapi/v4l/dev-meta.rst
@@ -7,21 +7,27 @@ Metadata Interface
 **
 
 Metadata refers to any non-image data that supplements video frames with
-additional information. This may include statistics computed over the image
-or frame capture parameters supplied by the image source. This interface is
-intended for transfer of metadata to userspace and control of that operation.
+additional information. This may include statistics computed over the image,
+frame capture parameters supplied by the image source or device specific
+parameters for specifying how the device processes images. This interface is
+intended for transfer of metadata between the userspace and the hardware and
+control of that operation.
 
-The metadata interface is implemented on video capture device nodes. The device
-can be dedicated to metadata or can implement both video and metadata capture
-as specified in its reported capabilities.
+The metadata interface is implemented on video device nodes. The device can be
+dedicated to metadata or can support both video and metadata as specified in 
its
+reported capabilities.
 
 Querying Capabilities
 =
 
-Device nodes supporting the metadata interface set the 
``V4L2_CAP_META_CAPTURE``
-flag in the ``device_caps`` field of the
+Device nodes supporting the metadata capture interface set the
+``V4L2_CAP_META_CAPTURE`` flag in the ``device_caps`` field of the
 :c:type:`v4l2_capability` structure returned by the :c:func:`VIDIOC_QUERYCAP`
-ioctl. That flag means the device can capture metadata to memory.
+ioctl. That flag means the device can capture metadata to memory. Similarly,
+device nodes supporting metadata output interface set the
+``V4L2_CAP_META_OUTPUT`` flag in the ``device_caps`` field of
+:c:type:`v4l2_capability` structure. That flag means the device can read
+metadata from memory.
 
 At least one of the read/write or streaming I/O methods must be supported.
 
@@ -35,10 +41,11 @@ to the basic :ref:`format` ioctls, the 
:c:func:`VIDIOC_ENUM_FMT` ioctl must be
 supported as well.
 
 To use the :ref:`format` ioctls applications set the ``type`` field of the
-:c:type:`v4l2_format` structure to ``V4L2_BUF_TYPE_META_CAPTURE`` and use the
-:c:type:`v4l2_meta_format` ``meta`` member of the ``fmt`` union as needed per
-the desired operation. Both drivers and applications must set the remainder of
-the :c:type:`v4l2_format` structure to 0.
+:c:type:`v4l2_format` structure to ``V4L2_BUF_TYPE_META_CAPTURE`` or to
+``V4L2_BUF_TYPE_META_OUTPUT`` and use the :c:type:`v4l2_meta_format` ``meta``
+member of the ``fmt`` union as needed per the desired operation. Both drivers
+and applications must set the remainder of the :c:type:`v4l2_format` structure
+to 0.
 
 .. _v4l2-meta-format:
 
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst 
b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 66fb1b3..31c3f33 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -251,6 +251,9 @@ specification the ioctl returns an ``EINVAL`` error code.
 * - ``V4L2_CAP_STREAMING``
   - 0x0400
   - The device supports the :ref:`streaming ` I/O method.
+* - ``V4L2_CAP_META_OUTPUT``
+  - 0x0800
+  - The device supports the :ref:`metadata` output interface.
 * - ``V4L2_CAP_TOUCH``
   - 0x1000
   - This is a touch device.
diff --git a/Documentation/media/videodev2.h.rst.exceptions 
b/Documentation/media/videodev2.h.rst.exceptions
index a5cb0a8..32172db 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -28,6 +28,7 @@ replace symbol V4L2_FIELD_TOP :c:type:`v4l2_field`
 
 # Documented enum v4l2

Re: [PATCH 09/10] scripts: kernel-doc: parse next structs/unions

2017-09-28 Thread Jonathan Corbet
On Tue, 26 Sep 2017 17:29:47 -0300
Mauro Carvalho Chehab  wrote:

> This patch actually need a fixup, in order to handle pointer,
> array and bitmask IDs.

Can you send a new series with the fixed patch?

I sure do like the shrinking of kernel-doc that comes with this series!  

Thanks,

jon


Re: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN

2017-09-28 Thread Arnd Bergmann
On Thu, Sep 28, 2017 at 6:09 AM, Andrey Ryabinin
 wrote:
> On 09/27/2017 04:26 PM, Arnd Bergmann wrote:
>> On Tue, Sep 26, 2017 at 9:49 AM, Andrey Ryabinin
>>  wrote:

>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -227,7 +227,7 @@ static inline const char *kbasename(const char *path)
>>  #define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
>>  #define __RENAME(x) __asm__(#x)
>>
>> -void fortify_panic(const char *name) __noreturn __cold;
>> +void fortify_panic(const char *name) __cold;
>>  void __read_overflow(void) __compiletime_error("detected read beyond
>> size of object passed as 1st parameter");
>>  void __read_overflow2(void) __compiletime_error("detected read beyond
>> size of object passed as 2nd parameter");
>>  void __read_overflow3(void) __compiletime_error("detected read beyond
>> size of object passed as 3rd parameter");
>>
>> I don't immediately see why the __noreturn changes the behavior here, any 
>> idea?
>>
>
>
> At first I thought that this somehow might be related to 
> __asan_handle_no_return(). GCC calls it
> before noreturn function. So I made patch to remove generation of these calls 
> (we don't need them in the kernel anyway)
> but it didn't help. It must be something else than.

I made a reduced test case yesterday (see http://paste.ubuntu.com/25628030/),
and it shows the same behavior with and without the sanitizer, it uses 128
bytes without the noreturn attribute and 480 bytes when its added, the sanitizer
adds a factor of 1.5x on top. It's possible that I did something wrong while
reducing, since the original driver file uses very little stack (a few hundred
bytes) without -fsanitize=kernel-address, but finding out what happens in
the reduced case may still help understand the other one.

Arnd


Re: [RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sakari Ailus
Hi Mauro,

On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.

The idea is that in the union, there's a struct which is specific to the
match_type field. I wouldn't call it senseless.

In the two cases there's just a single field in the containing struct. You
could remove the struct in that case as you do in this patch, and just use
the field. But I think the result is less clean and so I wouldn't make this
change.

The confusion comes possibly from the fact that the struct is named the
same as the field in the struct. These used to be called of and node, but
with the fwnode property framework the references to the fwnode are, well,
typically similarly called "fwnode". There's no underlying firmware
interface with that name, fwnode property API is just an API.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN

2017-09-28 Thread Andrey Ryabinin
On 09/27/2017 04:26 PM, Arnd Bergmann wrote:
> On Tue, Sep 26, 2017 at 9:49 AM, Andrey Ryabinin
>  wrote:
>>
>>
>> On 09/26/2017 09:47 AM, Arnd Bergmann wrote:
>>> On Mon, Sep 25, 2017 at 11:32 PM, Arnd Bergmann  wrote:
> 
>>> +   ret = __builtin_strlen(q);
>>
>>
>> I think this is not correct. Fortified strlen called here on purpose. If 
>> sizeof q is known at compile time
>> and 'q' contains not-null fortified strlen() will panic.
> 
> Ok, got it.
> 
>>> if (size) {
>>> size_t len = (ret >= size) ? size - 1 : ret;
>>> if (__builtin_constant_p(len) && len >= p_size)
>>>
>>> The problem is apparently that the fortified strlcpy calls the fortified 
>>> strlen,
>>> which in turn calls strnlen and that ends up calling the extern 
>>> '__real_strnlen'
>>> that gcc cannot reduce to a constant expression for a constant input.
>>
>>
>> Per my observation, it's the code like this:
>> if ()
>> fortify_panic(__func__);
>>
>>
>> somehow prevent gcc to merge several "struct i2c_board_info info;" into one 
>> stack slot.
>> With the hack bellow, stack usage reduced to ~1,6K:
> 
> 1.6k is also what I see with my patch, or any other approach I tried
> that changes
> string.h. With the split up em28xx_dvb_init() function (and without
> changes to string.h),
> I got down to a few hundred bytes for the largest handler.
> 
>> ---
>>  include/linux/string.h | 4 
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 54d21783e18d..9a96ff3ebf94 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -261,8 +261,6 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
>> if (p_size == (size_t)-1)
>> return __builtin_strlen(p);
>> ret = strnlen(p, p_size);
>> -   if (p_size <= ret)
>> -   fortify_panic(__func__);
>> return ret;
>>  }
>>
>> @@ -271,8 +269,6 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, 
>> __kernel_size_t maxlen)
>>  {
>> size_t p_size = __builtin_object_size(p, 0);
>> __kernel_size_t ret = __real_strnlen(p, maxlen < p_size ? maxlen : 
>> p_size);
>> -   if (p_size <= ret && maxlen != ret)
>> -   fortify_panic(__func__);
>> return ret;
> 
> I've reduced it further to this change:
> 
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -227,7 +227,7 @@ static inline const char *kbasename(const char *path)
>  #define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
>  #define __RENAME(x) __asm__(#x)
> 
> -void fortify_panic(const char *name) __noreturn __cold;
> +void fortify_panic(const char *name) __cold;
>  void __read_overflow(void) __compiletime_error("detected read beyond
> size of object passed as 1st parameter");
>  void __read_overflow2(void) __compiletime_error("detected read beyond
> size of object passed as 2nd parameter");
>  void __read_overflow3(void) __compiletime_error("detected read beyond
> size of object passed as 3rd parameter");
> 
> I don't immediately see why the __noreturn changes the behavior here, any 
> idea?
> 


At first I thought that this somehow might be related to 
__asan_handle_no_return(). GCC calls it
before noreturn function. So I made patch to remove generation of these calls 
(we don't need them in the kernel anyway)
but it didn't help. It must be something else than.


>>> Not sure if that change is the best fix, but it seems to address the 
>>> problem in
>>> this driver and probably leads to better code in other places as well.
>>>
>>
>> Probably it would be better to solve this on the strlcpy side, but I haven't 
>> found the way to do this right.
>> Alternative solutions:
>>
>>  - use memcpy() instead of strlcpy(). All source strings are smaller than 
>> I2C_NAME_SIZE, so we could
>>do something like this - memcpy(info.type, "si2168", sizeof("si2168"));
>>Also this should be faster.
> 
> This would be very similar to the patch I posted at the start of this
> thread to use strncpy(), right?

Sure.

> I was hoping that changing strlcpy() here could also improve other
> users that might run into
> the same situation, but stay below the 2048-byte stack frame limit.
> 
>>  - Move code under different "case:" in the switch(dev->model) to the 
>> separate function should help as well.
>>But it might be harder to backport into stables.
> 
> Agreed, I posted this in earlier versions of the patch series, see
> https://patchwork.kernel.org/patch/9601025/
> 
> The new patch was a result of me trying to come up with a less
> invasive version to
> make it easier to backport, since I would like to backport the last
> patch in the series
> that depends on all the earlier ones.
> 
>  Arnd
> 


[PATCH] media: tc358743: remove an unneeded condition

2017-09-28 Thread Dan Carpenter
We can remove the check for if "state->cec_adap" is NULL.  The
cec_allocate_adapter() function never returns NULL and also we verified
that "state->cec_adap" is an error pointer.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e1d8eef7055e..6bbe112be267 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -2122,7 +2122,7 @@ static int tc358743_probe(struct i2c_client *client,
state, dev_name(&client->dev),
CEC_CAP_DEFAULTS | CEC_CAP_MONITOR_ALL, CEC_MAX_LOG_ADDRS);
if (IS_ERR(state->cec_adap)) {
-   err = state->cec_adap ? PTR_ERR(state->cec_adap) : -ENOMEM;
+   err = PTR_ERR(state->cec_adap);
goto err_hdl;
}
irq_mask |= MASK_CEC_RMSK | MASK_CEC_TMSK;


Re: [RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sylwester Nawrocki
On 09/28/2017 02:53 PM, Mauro Carvalho Chehab wrote:
> (Resending for Mauro, while dropping the non-list recipients. The original
> likely had too many recipients.)
> 
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.
> 
> At drivers, this makes even clearer about the match criteria.
> 
> Signed-off-by: Mauro Carvalho Chehab 


Acked-by: Sylwester Nawrocki 


Re: [RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sakari Ailus
Hi Mauro,

On Wed, Sep 27, 2017 at 06:46:56PM -0300, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.

The idea is that in the union, there's a struct which is specific to the
match_type field. I wouldn't call it senseless.

In the two cases there's just a single field in the containing struct. You
could remove the struct in that case as you do in this patch, and just use
the field. But I think the result is less clean and so I wouldn't make this
change.

The confusion comes possibly from the fact that the struct is named the
same as the field in the struct. These used to be called of and node, but
with the fwnode property framework the references to the fwnode are, well,
typically similarly called "fwnode". There's no underlying firmware
interface with that name, fwnode property API is just an API.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


[RESEND PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Mauro Carvalho Chehab
(Resending for Mauro, while dropping the non-list recipients. The original
likely had too many recipients.)

The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
match criteria requires just a device name.

So, it doesn't make sense to enclose those into structs,
as the criteria can go directly into the union.

That makes easier to document it, as we don't need to document
weird senseless structs.

At drivers, this makes even clearer about the match criteria.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/am437x/am437x-vpfe.c| 6 +++---
 drivers/media/platform/atmel/atmel-isc.c   | 2 +-
 drivers/media/platform/atmel/atmel-isi.c   | 2 +-
 drivers/media/platform/davinci/vpif_capture.c  | 4 ++--
 drivers/media/platform/exynos4-is/media-dev.c  | 4 ++--
 drivers/media/platform/omap3isp/isp.c  | 4 ++--
 drivers/media/platform/pxa_camera.c| 2 +-
 drivers/media/platform/qcom/camss-8x16/camss.c | 2 +-
 drivers/media/platform/rcar-vin/rcar-core.c| 8 
 drivers/media/platform/rcar_drif.c | 4 ++--
 drivers/media/platform/soc_camera/soc_camera.c | 2 +-
 drivers/media/platform/stm32/stm32-dcmi.c  | 2 +-
 drivers/media/platform/ti-vpe/cal.c| 2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c| 2 +-
 drivers/media/v4l2-core/v4l2-async.c   | 4 ++--
 drivers/staging/media/imx/imx-media-dev.c  | 8 
 include/media/v4l2-async.h | 8 ++--
 17 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
b/drivers/media/platform/am437x/am437x-vpfe.c
index dfcc484cab89..f87e8f9467e9 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2304,8 +2304,8 @@ vpfe_async_bound(struct v4l2_async_notifier *notifier,
vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
 
for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
-   if (vpfe->cfg->asd[i]->match.fwnode.fwnode ==
-   asd[i].match.fwnode.fwnode) {
+   if (vpfe->cfg->asd[i]->match.fwnode ==
+   asd[i].match.fwnode) {
sdinfo = &vpfe->cfg->sub_devs[i];
vpfe->sd[i] = subdev;
vpfe->sd[i]->grp_id = sdinfo->grp_id;
@@ -2505,7 +2505,7 @@ vpfe_get_pdata(struct platform_device *pdev)
}
 
pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_FWNODE;
-   pdata->asd[i]->match.fwnode.fwnode = of_fwnode_handle(rem);
+   pdata->asd[i]->match.fwnode = of_fwnode_handle(rem);
of_node_put(rem);
}
 
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d7103c5f92c3..c04d9a4dbfac 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1742,7 +1742,7 @@ static int isc_parse_dt(struct device *dev, struct 
isc_device *isc)
subdev_entity->pfe_cfg0 |= ISC_PFE_CFG0_PPOL_LOW;
 
subdev_entity->asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-   subdev_entity->asd->match.fwnode.fwnode =
+   subdev_entity->asd->match.fwnode =
of_fwnode_handle(rem);
list_add_tail(&subdev_entity->list, &isc->subdev_entities);
}
diff --git a/drivers/media/platform/atmel/atmel-isi.c 
b/drivers/media/platform/atmel/atmel-isi.c
index 891fa2505efa..8c52f9f5f2db 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1124,7 +1124,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct 
device_node *node)
/* Remote node to connect */
isi->entity.node = remote;
isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
-   isi->entity.asd.match.fwnode.fwnode = of_fwnode_handle(remote);
+   isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
return 0;
}
 }
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 0ef36cec21d1..0cf141635cbc 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1390,7 +1390,7 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
 
for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
-   const struct fwnode_handle *fwnode = _asd->match.fwnode.fwnode;
+   const struct fwnode_handle *fwnode = _asd->match.fwnode;
 
if (fwnode == subdev->fwnode) {
vpif_obj.sd[i] = subdev;
@@ -1588,7 +1588,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
}
 
  

Re: [PATCH v2 00/17] V4L cleanups and documentation improvements

2017-09-28 Thread Sakari Ailus
Hi Mauro,

On Wed, Sep 27, 2017 at 06:46:43PM -0300, Mauro Carvalho Chehab wrote:
> This patch series is meant to improve V4L documentation. It touches
> some files at the tree doing some cleanup, in order to simplify
> the source code.
> 
> Mauro Carvalho Chehab (17):
>   media: tuner-types: add kernel-doc markups for struct tunertype
>   media: v4l2-common: get rid of v4l2_routing dead struct
>   media: v4l2-common: get rid of struct v4l2_discrete_probe
>   media: v4l2-common.h: document ancillary functions
>   media: v4l2-device.h: document ancillary macros
>   media: v4l2-dv-timings.h: convert comment into kernel-doc markup
>   media: v4l2-event.rst: improve events description
>   media: v4l2-ioctl.h: convert debug macros into enum and document
>   media: cec-pin.h: convert comments for cec_pin_state into kernel-doc
>   media: rc-core.rst: add an introduction for RC core
>   media: rc-core.h: minor adjustments at rc_driver_type doc
>   media: v4l2-fwnode.h: better describe bus union at fwnode endpoint
> struct
>   media: v4l2-async: simplify v4l2_async_subdev structure
>   media: v4l2-async: better describe match union at async match struct
>   media: v4l2-ctrls: document nested members of structs
>   media: videobuf2-core: improve kernel-doc markups
>   media: media-entity.h: add kernel-doc markups for nested structs

For patches apart form 7 and 13 (see my comments to them):

Acked-by: Sakari Ailus 

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v2 07/17] media: v4l2-event.rst: improve events description

2017-09-28 Thread Sakari Ailus
Hi Mauro,

On Wed, Sep 27, 2017 at 06:46:50PM -0300, Mauro Carvalho Chehab wrote:
> Both v4l2-event.rst and v4l2-event.h have an overview of
> events, but there are some inconsistencies there:
> 
> - at v4l2-event, the event's ringbuffer is called kevent. Its

s/ringbuffer/ring buffer/g

With that,

Acked-by: Sakari Ailus 

>   name is, instead, v4l2_kevent;
> 
> - Some things are mentioned on both places (with different words),
>   others are either on one of the files.
> 
> In order to cleanup this mess, put everything at v4l2-event.rst
> and improve it to be a little more coherent and to have cross
> references.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  Documentation/media/kapi/v4l2-event.rst | 64 
> ++---
>  include/media/v4l2-event.h  | 34 --
>  2 files changed, 52 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/media/kapi/v4l2-event.rst 
> b/Documentation/media/kapi/v4l2-event.rst
> index 9938d21ef4d1..7831a503e2aa 100644
> --- a/Documentation/media/kapi/v4l2-event.rst
> +++ b/Documentation/media/kapi/v4l2-event.rst
> @@ -5,27 +5,67 @@ V4L2 events
>  The V4L2 events provide a generic way to pass events to user space.
>  The driver must use :c:type:`v4l2_fh` to be able to support V4L2 events.
>  
> -Events are defined by a type and an optional ID. The ID may refer to a V4L2
> -object such as a control ID. If unused, then the ID is 0.
> +Events are subscribed per-filehandle. An event specification consists of a
> +``type`` and is optionally associated with an object identified through the
> +``id`` field. If unused, then the ``id`` is 0. So an event is uniquely
> +identified by the ``(type, id)`` tuple.
>  
> -When the user subscribes to an event the driver will allocate a number of
> -kevent structs for that event. So every (type, ID) event tuple will have
> -its own set of kevent structs. This guarantees that if a driver is generating
> -lots of events of one type in a short time, then that will not overwrite
> -events of another type.
> +The :c:type:`v4l2_fh` struct has a list of subscribed events on its
> +``subscribed`` field.
>  
> -But if you get more events of one type than the number of kevents that were
> -reserved, then the oldest event will be dropped and the new one added.
> +When the user subscribes to an event, a :c:type:`v4l2_subscribed_event`
> +struct is added to :c:type:`v4l2_fh`\ ``.subscribed``, one for every
> +subscribed event.
> +
> +Each :c:type:`v4l2_subscribed_event` struct ends with a
> +:c:type:`v4l2_kevent` ringbuffer, with the size given by the caller
> +of :c:func:`v4l2_event_subscribe`. Such ringbuffer is used to store any 
> events
> +raised by the driver.
> +
> +So every ``(type, ID)`` event tuple will have its own set of
> +:c:type:`v4l2_kevent` ringbuffer. This guarantees that if a driver is
> +generating lots of events of one type in a short time, then that will
> +not overwrite events of another type.
> +
> +But if you get more events of one type than the size of the
> +:c:type:`v4l2_kevent` ringbuffer, then the oldest event will be dropped
> +and the new one added.
> +
> +The :c:type:`v4l2_kevent` struct links into the ``available``
> +list of the :c:type:`v4l2_fh` struct so :ref:`VIDIOC_DQEVENT` will
> +know which event to dequeue first.
> +
> +Finally, if the event subscription is associated with a particular object
> +such as a V4L2 control, then that object needs to know about that as well
> +so that an event can be raised by that object. So the ``node`` field can
> +be used to link the :c:type:`v4l2_subscribed_event` struct into a list of
> +such object.
> +
> +So to summarize:
> +
> +- struct :c:type:`v4l2_fh` has two lists: one of the ``subscribed`` events,
> +  and one of the ``available`` events.
> +
> +- struct :c:type:`v4l2_subscribed_event` has a ringbuffer of raised
> +  (pending) events of that particular type.
> +
> +- If struct :c:type:`v4l2_subscribed_event` is associated with a specific
> +  object, then that object will have an internal list of
> +  struct :c:type:`v4l2_subscribed_event` so it knows who subscribed an
> +  event to that object.
>  
>  Furthermore, the internal struct :c:type:`v4l2_subscribed_event` has
>  ``merge()`` and ``replace()`` callbacks which drivers can set. These
>  callbacks are called when a new event is raised and there is no more room.
> +
>  The ``replace()`` callback allows you to replace the payload of the old event
>  with that of the new event, merging any relevant data from the old payload
>  into the new payload that replaces it. It is called when this event type has
> -only one kevent struct allocated. The ``merge()`` callback allows you to 
> merge
> -the oldest event payload into that of the second-oldest event payload. It is
> -called when there are two or more kevent structs allocated.
> +only one :c:type:`v4l2_kevent` struct allocated.
> +
> +The ``merge()`` callback allows you to merge the oldest event payload into
> +tha

Re: [PATCH v2 12/17] media: v4l2-fwnode.h: better describe bus union at fwnode endpoint struct

2017-09-28 Thread Sakari Ailus
Hi Mauro,

Thanks for the patch.

On Wed, Sep 27, 2017 at 06:46:55PM -0300, Mauro Carvalho Chehab wrote:
> Now that kernel-doc handles nested unions, better document the
> bus union at struct v4l2_fwnode_endpoint.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  include/media/v4l2-fwnode.h | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 7adec9851d9e..5f4716f967d0 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -79,7 +79,18 @@ struct v4l2_fwnode_bus_mipi_csi1 {
>   * struct v4l2_fwnode_endpoint - the endpoint data structure
>   * @base: fwnode endpoint of the v4l2_fwnode
>   * @bus_type: bus type
> - * @bus: bus configuration data structure
> + * @bus: union with bus configuration data structure
> + * @bus.parallel: pointer for &struct v4l2_fwnode_bus_parallel.
> + * Used if the bus is parallel.
> + * @bus.mipi_csi1: pointer for &struct v4l2_fwnode_bus_mipi_csi1.
> + *  Used if the bus is Mobile Industry Processor

s/Mobile.*Interface/MIPI Alliance/

Same below.

Please see:

https://mipi.org/content/what-does-mipi-stand>

With that,

Acked-by: Sakari Ailus 

> + *  Interface's Camera Serial Interface version 1
> + *  (MIPI CSI1) or Standard Mobile Imaging Architecture's
> + *  Compact Camera Port 2 (SMIA CCP2).
> + * @bus.mipi_csi2: pointer for &struct v4l2_fwnode_bus_mipi_csi2.
> + *  Used if the bus is Mobile Industry Processor
> + *  Interface's Camera Serial Interface version 2
> + *  (MIPI CSI2).
>   * @link_frequencies: array of supported link frequencies
>   * @nr_of_link_frequencies: number of elements in link_frequenccies array
>   */

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi


[RFC PATCH 3/9] [media] videobuf2: add support for jobs API

2017-09-28 Thread Alexandre Courbot
Add generic support for jobs in videobuf2. When the jobs API is active,
the passing of buffers to the driver is delayed until their job is
submitted. Drivers need to call the vb2_queue_active_job_buffers()
function in order to receive the buffers corresponding to the active
job.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/videobuf2-core.c | 33 
 include/media/videobuf2-core.h   | 16 
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 14f83cecfa92..2ac880ebe192 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -26,6 +26,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -1304,6 +1305,22 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
int index, void *pb)
 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+void vb2_queue_active_job_buffers(struct vb2_queue *q)
+{
+   const struct v4l2_job_state_handler *hdl = q->state_handler;
+   struct vb2_buffer *vb;
+
+   if (!q->start_streaming_called)
+   return;
+
+   list_for_each_entry(vb, &q->queued_list, queued_entry) {
+   if (!hdl || hdl->active_state == vb->job)
+   __enqueue_in_driver(vb);
+   }
+}
+EXPORT_SYMBOL_GPL(vb2_queue_active_job_buffers);
+
+
 /**
  * vb2_start_streaming() - Attempt to start streaming.
  * @q: videobuf2 queue
@@ -1320,15 +1337,15 @@ static int vb2_start_streaming(struct vb2_queue *q)
struct vb2_buffer *vb;
int ret;
 
+   q->start_streaming_called = 1;
+
/*
 * If any buffers were queued before streamon,
 * we can now pass them to driver for processing.
 */
-   list_for_each_entry(vb, &q->queued_list, queued_entry)
-   __enqueue_in_driver(vb);
+   vb2_queue_active_job_buffers(q);
 
/* Tell the driver to start streaming */
-   q->start_streaming_called = 1;
ret = call_qop(q, start_streaming, q,
   atomic_read(&q->owned_by_drv_count));
if (!ret)
@@ -1398,6 +1415,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb)
q->queued_count++;
q->waiting_for_buffers = false;
vb->state = VB2_BUF_STATE_QUEUED;
+   vb->job = q->state_handler ? q->state_handler->current_state : NULL;
 
if (pb)
call_void_bufop(q, copy_timestamp, vb, pb);
@@ -1407,8 +1425,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb)
/*
 * If already streaming, give the buffer to driver for processing.
 * If not, the buffer will be given to driver on next streamon.
+*
+* If using the jobs API, we will give the buffer to the driver when
+* its job becomes active.
 */
-   if (q->start_streaming_called)
+   if (q->start_streaming_called && !vb->job)
__enqueue_in_driver(vb);
 
/* Fill buffer information for the userspace */
@@ -1422,6 +1443,8 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb)
 * then we can finally call start_streaming().
 */
if (q->streaming && !q->start_streaming_called &&
+   /* TODO potential issue: what if we have less than min_buffers_needed
+* in the next job? */
q->queued_count >= q->min_buffers_needed) {
ret = vb2_start_streaming(q);
if (ret)
@@ -1728,6 +1751,8 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int 
type)
 * Tell driver to start streaming provided sufficient buffers
 * are available.
 */
+   /* TODO potential issue: what if we have less than min_buffers_needed
+* in the next job? */
if (q->queued_count >= q->min_buffers_needed) {
ret = v4l_vb2q_enable_media_source(q);
if (ret)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index cb97c224be73..9e172168e011 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -246,6 +246,7 @@ struct vb2_buffer {
unsigned intnum_planes;
struct vb2_planeplanes[VB2_MAX_PLANES];
u64 timestamp;
+   struct v4l2_job_state   *job;
 
/* private: internal use only
 *
@@ -506,6 +507,7 @@ struct vb2_queue {
const struct vb2_ops*ops;
const struct vb2_mem_ops*mem_ops;
const struct vb2_buf_ops*buf_ops;
+   const struct v4l2_job_state_handler *state_handler;
 
void*drv_priv;
unsigned intbuf_struct_size;
@@ -625,6 +627,20 @@ void vb2_discard_done(struct vb2_queue *q);
  */
 int vb2_wait_for_all_buffers(struct vb2_queue *q);
 
+/**
+ * vb2_queue_active_job_buffers() - Pass all 

[RFC PATCH 2/9] [media] v4l2-core: add core jobs API support

2017-09-28 Thread Alexandre Courbot
Add core support code for jobs API. This manages the life cycle of jobs
and creation of a jobs queue, as well as the interface for job states.

It also exposes the user-space jobs API.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/Makefile|   3 +-
 drivers/media/v4l2-core/v4l2-dev.c  |   6 +
 drivers/media/v4l2-core/v4l2-jobqueue-dev.c | 173 +++
 drivers/media/v4l2-core/v4l2-jobqueue.c | 764 
 include/media/v4l2-dev.h|   4 +
 include/media/v4l2-fh.h |   4 +
 include/media/v4l2-job-state.h  |  75 +++
 include/media/v4l2-jobqueue-dev.h   |  24 +
 include/media/v4l2-jobqueue.h   |  54 ++
 include/uapi/linux/v4l2-jobs.h  |  40 ++
 include/uapi/linux/videodev2.h  |   2 +
 11 files changed, 1148 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
 create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
 create mode 100644 include/media/v4l2-job-state.h
 create mode 100644 include/media/v4l2-jobqueue-dev.h
 create mode 100644 include/media/v4l2-jobqueue.h
 create mode 100644 include/uapi/linux/v4l2-jobs.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 098ad5fd5231..a717bb8f1a25 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -6,7 +6,8 @@ tuner-objs  :=  tuner-core.o
 
 videodev-objs  :=  v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
-   v4l2-async.o
+   v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o
+
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 5a7063886c93..fb229b671b9d 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define VIDEO_NUM_DEVICES  256
 #define VIDEO_NAME  "video4linux"
@@ -1058,6 +1059,10 @@ static int __init videodev_init(void)
return -EIO;
}
 
+   ret = v4l2_jobqueue_device_init();
+   if (ret < 0)
+   printk(KERN_WARNING "video_dev: channel initialization 
failed\n");
+
return 0;
 }
 
@@ -1065,6 +1070,7 @@ static void __exit videodev_exit(void)
 {
dev_t dev = MKDEV(VIDEO_MAJOR, 0);
 
+   v4l2_jobqueue_device_exit();
class_unregister(&video_class);
unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
 }
diff --git a/drivers/media/v4l2-core/v4l2-jobqueue-dev.c 
b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
new file mode 100644
index ..688c4ba275a6
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
@@ -0,0 +1,173 @@
+/*
+V4L2 job queue device
+
+Copyright (C) 2017  The Chromium project
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CLASS_NAME "v4l2_jobqueue"
+#define DEVICE_NAME "v4l2_jobqueue"
+
+static int major;
+static struct class *jobqueue_class;
+static struct device *jobqueue_device;
+
+static int v4l2_jobqueue_device_open(struct inode *inode, struct file *filp)
+{
+   struct v4l2_jobqueue *jq;
+
+   jq = v4l2_jobqueue_new();
+   if (IS_ERR(jq))
+   return PTR_ERR(jq);
+
+   filp->private_data = jq;
+
+   return 0;
+}
+
+static int v4l2_jobqueue_device_release(struct inode *inode, struct file *filp)
+{
+   struct v4l2_jobqueue *jq = filp->private_data;
+
+   return v4l2_jobqueue_del(jq);
+}
+
+static long v4l2_jobqueue_ioctl_init(struct file *filp, void *arg)
+{
+   struct v4l2_jobqueue *jq = filp->private_data;
+   struct v4l2_jobqueue_init *cinit = arg;
+
+   return v4l2_jobqueue_init(jq, cinit);
+}
+
+static long v4l2_jobqueue_device_ioctl_qjob(struct file *filp, void *arg)
+{
+   struct v4l2_jobqueue *jq = filp->private_data;
+
+   return v4l2_jobqueue_qjob(jq);
+}
+
+static long v4l2_jobqueue_device_ioctl_dqjob(struct file *filp, void *arg)
+{
+   struct v4l2_jobqueue *jq = filp->private_data;
+
+   return v4l2_jobqueue_dqjob(jq);
+}
+
+static long v4l2_jobqueue_device_ioctl_export_job(struct file *filp, void *arg)
+{
+   struct v4l2_jobqueue *jq = filp->private_data;
+   struct v4l2_jobqueue_jo

[RFC PATCH 5/9] [media] v4l2-job: add generic jobs ops

2017-09-28 Thread Alexandre Courbot
Add a generic state handler that records controls to be set for a given
job and applies them once the job is scheduled, while also allowing said
controls to be read back once the job is dequeued.

This implementation can be used as-is for most drivers, with only drivers
with specific needs needing to provide an alternative implementation.

Note: this is still very early, but should do the job to demonstrate the
jobs API feasibility. Amongst the current limitations:

- We use v4l2_ext_control to store controls, which expects user-space
pointers. As a consequence only integer controls are supported at the
moment.
- No support for try_ctrl yet.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/Makefile   |   3 +-
 drivers/media/v4l2-core/v4l2-job-generic.c | 394 +
 include/media/v4l2-job-generic.h   |  47 
 3 files changed, 443 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-job-generic.c
 create mode 100644 include/media/v4l2-job-generic.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index a717bb8f1a25..ee09e1f29129 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -6,7 +6,8 @@ tuner-objs  :=  tuner-core.o
 
 videodev-objs  :=  v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
-   v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o
+   v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o \
+   v4l2-job-generic.o
 
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
diff --git a/drivers/media/v4l2-core/v4l2-job-generic.c 
b/drivers/media/v4l2-core/v4l2-job-generic.c
new file mode 100644
index ..ded6464e723a
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-job-generic.c
@@ -0,0 +1,394 @@
+/*
+V4L2 generic jobs implementation
+
+Copyright (C) 2017  The Chromium project
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define to_generic_state_handler(hdl) \
+   container_of(hdl, struct v4l2_generic_state_handler, base)
+
+struct v4l2_generic_job_state {
+   struct v4l2_job_state base;
+   struct list_head node;
+
+   int nr_ctrls;
+   struct v4l2_ext_control ctrls[0];
+};
+#define to_generic_job_state(job) \
+   container_of(job, struct v4l2_generic_job_state, base)
+
+/* TODO this is O(n). Find a better way to store/lookup controls */
+static struct v4l2_ext_control *
+v4l2_generic_job_find_control(struct v4l2_generic_job_state *job, u32 id)
+{
+   int i;
+
+   for (i = 0; i < job->nr_ctrls; i++)
+   if (job->ctrls[i].id == id)
+   return &job->ctrls[i];
+
+   return NULL;
+}
+
+static struct v4l2_job_state *
+v4l2_job_generic_job_new(struct v4l2_job_state_handler *_hdl)
+{
+   struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+   struct v4l2_generic_job_state *ret;
+
+   ret = kzalloc(sizeof(*ret) + sizeof(ret->ctrls[0]) * hdl->nr_ctrls,
+ GFP_KERNEL);
+   if (ret == NULL)
+   return ERR_PTR(-ENOMEM);
+
+   return &ret->base;
+}
+
+static void v4l2_job_generic_job_free(struct v4l2_job_state_handler *hdl,
+   struct v4l2_job_state *_job)
+{
+   struct v4l2_generic_job_state *job = to_generic_job_state(_job);
+
+   kfree(job);
+}
+
+static int v4l2_job_generic_job_apply(struct v4l2_job_state_handler *_hdl)
+{
+   struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+   struct v4l2_generic_job_state *job;
+   struct v4l2_ext_controls ctrls;
+   int ret;
+
+   job = to_generic_job_state(_hdl->active_state);
+
+   if (job->nr_ctrls == 0)
+   return 0;
+
+   ctrls.which = V4L2_CTRL_WHICH_CUR_VAL;
+   ctrls.count = job->nr_ctrls;
+   ctrls.controls = job->ctrls;
+
+   ret = v4l2_s_ext_ctrls(hdl->fh, hdl->ctrl_hdl, &ctrls);
+   if (ret) {
+   pr_err("Cannot set job controls: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int v4l2_job_generic_s_ctrl(struct v4l2_job_state_handler *_hdl,
+  struct v4l2_ext_control *c)
+{
+   struct v4l2_generic_state_handler *hdl = to_g

[RFC PATCH 4/9] [media] v4l2-ctrls: add support for jobs API

2017-09-28 Thread Alexandre Courbot
Add generic support for jobs in the control framework by handling the
new V4L2_CTRL_WHICH_*JOB_VAL which values. This calls the state handler
callbacks in such a case and takes care of control management for
drivers with jobs API support.

Note: this is a very simple and naive way to manage controls in jobs.
Doing this properly will probably require more changes to the control
framework, but the current form is enough to demonstrate the general
ideas.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 50 
 include/media/v4l2-ctrls.h   |  6 +
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index dd1db678718c..fcc644a83cf0 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define has_op(master, op) \
(master->ops && master->ops->op)
@@ -1603,8 +1604,14 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 ch_flags)
 
/* has_changed is set by cluster_changed */
changed = ctrl->has_changed;
-   if (changed)
+   if (changed) {
+   struct v4l2_job_state_handler *state = 
ctrl->handler->state_handler;
+
+   if (state && state->ops->ctrl_changed)
+   state->ops->ctrl_changed(state, ctrl);
+
ptr_to_ptr(ctrl, ctrl->p_new, ctrl->p_cur);
+   }
 
if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) {
/* Note: CH_FLAGS is only set for auto clusters. */
@@ -2738,6 +2745,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler 
*hdl,
 
if (cs->which &&
cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
+   cs->which != V4L2_CTRL_WHICH_CURJOB_VAL &&
+   cs->which != V4L2_CTRL_WHICH_DEQJOB_VAL &&
V4L2_CTRL_ID2WHICH(id) != cs->which)
return -EINVAL;
 
@@ -2817,7 +2826,9 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler 
*hdl,
whether there are any controls at all. */
 static int class_check(struct v4l2_ctrl_handler *hdl, u32 which)
 {
-   if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL)
+   if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL ||
+   which == V4L2_CTRL_WHICH_CURJOB_VAL ||
+   which == V4L2_CTRL_WHICH_DEQJOB_VAL)
return list_empty(&hdl->ctrl_refs) ? -EINVAL : 0;
return find_ref_lock(hdl, which | 1) ? 0 : -EINVAL;
 }
@@ -2829,11 +2840,14 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, 
struct v4l2_ext_controls *cs
 {
struct v4l2_ctrl_helper helper[4];
struct v4l2_ctrl_helper *helpers = helper;
+   struct v4l2_job_state_handler *state;
int ret;
int i, j;
-   bool def_value;
+   bool def_value, job_value;
 
def_value = (cs->which == V4L2_CTRL_WHICH_DEF_VAL);
+   job_value = (cs->which == V4L2_CTRL_WHICH_DEQJOB_VAL ||
+cs->which == V4L2_CTRL_WHICH_CURJOB_VAL);
 
cs->error_idx = cs->count;
cs->which = V4L2_CTRL_ID2WHICH(cs->which);
@@ -2841,6 +2855,10 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, 
struct v4l2_ext_controls *cs
if (hdl == NULL)
return -EINVAL;
 
+   state = hdl->state_handler;
+   if (!state && job_value)
+   return -EINVAL;
+
if (cs->count == 0)
return class_check(hdl, cs->which);
 
@@ -2874,18 +2892,20 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, 
struct v4l2_ext_controls *cs
v4l2_ctrl_lock(master);
 
/* g_volatile_ctrl will update the new control values */
-   if (!def_value &&
+   if (!def_value && !job_value &&
((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
(master->has_volatiles && !is_cur_manual(master {
for (j = 0; j < master->ncontrols; j++)
cur_to_new(master->cluster[j]);
ret = call_op(master, g_volatile_ctrl);
ctrl_to_user = new_to_user;
+   } else if (job_value) {
+   ret = state->ops->g_ctrl(state, master->id, 
cs->controls + i, cs->which);
}
/* If OK, then copy the current (for non-volatile controls)
   or the new (for volatile controls) control values to the
   caller */
-   if (!ret) {
+   if (!ret && !job_value) {
u32 idx = i;
 
do {
@@ -3008,6 +3028,7 @@ static int try_or_set_cluster(struct v4l2_fh *fh, struct 
v4l2_ctrl *master,
/* Don't set if there is no change */
if (ret || !set || !cluster_changed(master))
return ret;
+
ret = call_op

[RFC PATCH 7/9] [media] vim2m: add jobs API support

2017-09-28 Thread Alexandre Courbot
Add support for jobs in vim2m, using the generic state handler.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/platform/vim2m.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 970b9b6dab25..aff21d53d898 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 
+#include 
+
 MODULE_DESCRIPTION("Virtual device for mem2mem framework testing");
 MODULE_AUTHOR("Pawel Osciak, ");
 MODULE_LICENSE("GPL");
@@ -155,6 +157,7 @@ struct vim2m_ctx {
struct vim2m_dev*dev;
 
struct v4l2_ctrl_handler hdl;
+   struct v4l2_generic_state_handler state;
 
/* Processed buffers in this transaction */
u8  num_processed;
@@ -877,6 +880,15 @@ static const struct v4l2_ctrl_config 
vim2m_ctrl_trans_num_bufs = {
.step = 1,
 };
 
+static void vim2m_process_active_job(struct v4l2_job_state_handler *hdl)
+{
+   struct vim2m_ctx *ctx = container_of(hdl, struct vim2m_ctx, state.base);
+
+   vb2_queue_active_job_buffers(&ctx->fh.m2m_ctx->cap_q_ctx.q);
+   vb2_queue_active_job_buffers(&ctx->fh.m2m_ctx->out_q_ctx.q);
+   v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
+}
+
 /*
  * File operations
  */
@@ -886,6 +898,7 @@ static int vim2m_open(struct file *file)
struct vim2m_ctx *ctx = NULL;
struct v4l2_ctrl_handler *hdl;
int rc = 0;
+   int ret;
 
if (mutex_lock_interruptible(&dev->dev_mutex))
return -ERESTARTSYS;
@@ -913,6 +926,15 @@ static int vim2m_open(struct file *file)
ctx->fh.ctrl_handler = hdl;
v4l2_ctrl_handler_setup(hdl);
 
+   ret = v4l2_job_generic_init(&ctx->state, vim2m_process_active_job,
+   &ctx->fh, NULL);
+   if (ret) {
+   v4l2_ctrl_handler_free(hdl);
+   v4l2_fh_exit(&ctx->fh);
+   kfree(ctx);
+   goto open_unlock;
+   }
+
ctx->q_data[V4L2_M2M_SRC].fmt = &formats[0];
ctx->q_data[V4L2_M2M_SRC].width = 640;
ctx->q_data[V4L2_M2M_SRC].height = 480;
@@ -934,6 +956,8 @@ static int vim2m_open(struct file *file)
goto open_unlock;
}
 
+   v4l2_mem_ctx_job_init(ctx->fh.m2m_ctx, &ctx->state.base);
+
v4l2_fh_add(&ctx->fh);
atomic_inc(&dev->num_inst);
 
-- 
2.14.2.822.g60be5d43e6-goog



[RFC PATCH 9/9] [media] document jobs API

2017-09-28 Thread Alexandre Courbot
Still a work-in-progress, but hopefully conveys the general idea.

Signed-off-by: Alexandre Courbot 
---
 Documentation/media/intro.rst  |   2 +
 Documentation/media/media_uapi.rst |   1 +
 Documentation/media/uapi/jobs/jobs-api.rst |  23 +++
 Documentation/media/uapi/jobs/jobs-example.rst |  69 
 Documentation/media/uapi/jobs/jobs-intro.rst   |  61 +++
 Documentation/media/uapi/jobs/jobs-queue.rst   |  73 
 Documentation/media/uapi/jobs/jobs-queue.svg   | 192 +
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst  |   6 +
 8 files changed, 427 insertions(+)
 create mode 100644 Documentation/media/uapi/jobs/jobs-api.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-example.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-intro.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-queue.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-queue.svg

diff --git a/Documentation/media/intro.rst b/Documentation/media/intro.rst
index 9ce2e23a0236..e39a9dd3444a 100644
--- a/Documentation/media/intro.rst
+++ b/Documentation/media/intro.rst
@@ -38,6 +38,8 @@ divided into five parts.
 
 5. The :ref:`fifth part ` covers the CEC (Consumer Electronics Control) 
API.
 
+6. The :ref:`sixth part ` covers the jobs API.
+
 It should also be noted that a media device may also have audio components, 
like
 mixers, PCM capture, PCM playback, etc, which are controlled via ALSA API.  For
 additional information and for the latest development code, see:
diff --git a/Documentation/media/media_uapi.rst 
b/Documentation/media/media_uapi.rst
index fd8ebe002cd2..254e3d085abc 100644
--- a/Documentation/media/media_uapi.rst
+++ b/Documentation/media/media_uapi.rst
@@ -27,5 +27,6 @@ License".
 uapi/rc/remote_controllers
 uapi/mediactl/media-controller
 uapi/cec/cec-api
+uapi/jobs/jobs-api
 uapi/gen-errors
 uapi/fdl-appendix
diff --git a/Documentation/media/uapi/jobs/jobs-api.rst 
b/Documentation/media/uapi/jobs/jobs-api.rst
new file mode 100644
index ..3a7aa8568e93
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-api.rst
@@ -0,0 +1,23 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. include:: 
+
+.. _jobsapi:
+
+##
+Part VI - Jobs API
+##
+
+This part describes the V4L2 Jobs API.
+
+.. class:: toc-title
+
+Table of Contents
+
+.. toctree::
+:maxdepth: 5
+:numbered:
+
+jobs-intro
+jobs-queue
+jobs-example
diff --git a/Documentation/media/uapi/jobs/jobs-example.rst 
b/Documentation/media/uapi/jobs/jobs-example.rst
new file mode 100644
index ..0b725dfb58bd
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-example.rst
@@ -0,0 +1,69 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _jobs-example:
+
+===
+Example: Using the Jobs API for HDR capture
+===
+
+HDR capturing involves taking two shots of the same image with different
+exposure settings. When using the V4L2 API, one must wait for the first shot to
+be completed before updating the exposure and taking the second one, 
introducing
+synchronization with user-space and potential delays for the second image.
+
+The Jobs API allows us to simply submit two jobs with different exposure
+parameters, and to dequeue their buffers to get the result.
+
+The following example shows how to do this on a capture device with the 
exposure
+control. It assumes that the capture device has already been opened and its
+format set. To keep the code simple this code does not handle errors.
+
+.. code-block:: c
+
+struct v4l2_ext_control ctrl[1];
+struct v4l2_ext_controls ctrls;
+struct v4l2_jobqueue_init jq_init;
+struct v4l2_buffer buf[2];
+int jq_fd;
+
+/* FD for the jobs queue */
+jq_fd = open("/dev/v4l2_jobqueue");
+
+/* Initialize the jobs queue */
+jq_init.nb_devs = 1;
+jq_init.fd[0] = capture_fd;
+ioctl(jq_fd, VIDIOC_JOBQUEUE_INIT, &jq_init);
+
+/* Prepare and submit the first capture job, low exposure */
+ctrl[0].id = V4L2_CID_EXPOSURE;
+ctrl[0].value = 32;
+ctrls.which = V4L2_CTRL_WHICH_CURJOB_VAL;
+ctrls.count = 1;
+ctrsl.controls = ctrl;
+ioctl(capture_fd, VICIOC_S_EXT_CTRLS, &ctrls);
+ioctl(capture_fd, VIDIOC_QBUF, &buf[0]);
+ioctl(jq_fd, VIDIOC_JOBQUEUE_QJOB, NULL);
+
+/* Prepare and submit the second capture job, high exposure */
+ctrl[0].id = V4L2_CID_EXPOSURE;
+ctrl[0].value = 192;
+ctrls.which = V4L2_CTRL_WHICH_CURJOB_VAL;
+ctrls.count = 1;
+ctrsl.controls = ctrl;
+ioctl(capture_fd, VICIOC_S_EXT_CTRLS, &ctrls);
+ioctl(capture_fd, VIDIOC_QBUF, &buf[1]);
+ioctl(jq_fd, VIDIOC_JOBQUEUE_QJOB, NULL);
+
+/* Dequeue buffers with the captured data */
+ioctl(capture_fd, VIDIOC_DQBUF, &buf[0]);
+ioctl(capture_fd, VIDIOC_DQBUF, &buf[1]);
+
+/* Dequeue jobs an

[RFC PATCH 6/9] [media] m2m: add generic support for jobs API

2017-09-28 Thread Alexandre Courbot
Add a v4l2_mem_ctx_job_init() function that drivers using m2m can call
at init time in order to set the state handler.

Also make sure to call v4l2_jobqueue_job_finish() when the jobs API is
used and all buffers for the current job have been processed.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 19 +++
 include/media/v4l2-mem2mem.h   | 11 +++
 2 files changed, 30 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index f62e68aa04c4..dc4d8b16c0e2 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -22,6 +22,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 MODULE_DESCRIPTION("Mem to mem device framework for videobuf");
 MODULE_AUTHOR("Pawel Osciak, ");
@@ -333,6 +335,13 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
 
spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
 
+   /* Jobs mode: if we have no more buffers for the current job,
+* consider it completed */
+   if (m2m_ctx->state && m2m_ctx->state->active_state &&
+   v4l2_m2m_next_src_buf(m2m_ctx) == NULL &&
+   v4l2_m2m_next_dst_buf(m2m_ctx) == NULL)
+   v4l2_jobqueue_job_finish(m2m_ctx->state);
+
/* This instance might have more buffers ready, but since we do not
 * allow more than one job on the job_queue per instance, each has
 * to be scheduled separately after the previous one finishes. */
@@ -665,6 +674,16 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev 
*m2m_dev,
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_ctx_init);
 
+void v4l2_mem_ctx_job_init(struct v4l2_m2m_ctx *ctx,
+  struct v4l2_job_state_handler *hdl)
+{
+   ctx->state = hdl;
+
+   ctx->out_q_ctx.q.state_handler = hdl;
+   ctx->cap_q_ctx.q.state_handler = hdl;
+}
+EXPORT_SYMBOL_GPL(v4l2_mem_ctx_job_init);
+
 void v4l2_m2m_ctx_release(struct v4l2_m2m_ctx *m2m_ctx)
 {
/* wait until the current context is dequeued from job_queue */
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index e157d5c9b224..4ba18a32c9b0 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -83,6 +83,7 @@ struct v4l2_m2m_queue_ctx {
  *
  * @q_lock: struct &mutex lock
  * @m2m_dev: opaque pointer to the internal data to handle M2M context
+ * @state: Pointer to state handler for channel support
  * @cap_q_ctx: Capture (output to memory) queue context
  * @out_q_ctx: Output (input from memory) queue context
  * @queue: List of memory to memory contexts
@@ -100,6 +101,7 @@ struct v4l2_m2m_ctx {
 
/* internal use only */
struct v4l2_m2m_dev *m2m_dev;
+   struct v4l2_job_state_handler   *state;
 
struct v4l2_m2m_queue_ctx   cap_q_ctx;
 
@@ -351,6 +353,15 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev 
*m2m_dev,
void *drv_priv,
int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct 
vb2_queue *dst_vq));
 
+/**
+ * v4l2_mem_ctx_channel_init() - enable a context to be used in a channel
+ *
+ * @ctx: context to potentially use within a channel
+ * @hal: state handler that will be used with this context
+ */
+void v4l2_mem_ctx_job_init(struct v4l2_m2m_ctx *ctx,
+  struct v4l2_job_state_handler *hdl);
+
 static inline void v4l2_m2m_set_src_buffered(struct v4l2_m2m_ctx *m2m_ctx,
 bool buffered)
 {
-- 
2.14.2.822.g60be5d43e6-goog



[RFC PATCH 1/9] [media] v4l2-core: add v4l2_is_v4l2_file function

2017-09-28 Thread Alexandre Courbot
Add a function that checks whether a given open file is a v4l2 device
instance. This will be useful for job queue creation as we are passed a
set of FDs and we need to make this check.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-dev.c | 6 ++
 include/media/v4l2-dev.h   | 9 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index c647ba648805..5a7063886c93 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -470,6 +470,12 @@ static const struct file_operations v4l2_fops = {
.llseek = no_llseek,
 };
 
+bool v4l2_is_v4l2_file(struct file *filp)
+{
+   return filp->f_op == &v4l2_fops;
+}
+EXPORT_SYMBOL(v4l2_is_v4l2_file);
+
 /**
  * get_index - assign stream index number based on v4l2_dev
  * @vdev: video_device to assign index number to, vdev->v4l2_dev should be 
assigned
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index e657614521e3..b73d646980da 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -395,6 +395,15 @@ void video_device_release(struct video_device *vdev);
  */
 void video_device_release_empty(struct video_device *vdev);
 
+/**
+ * v4l2_is_v4l2_file - Check whether a file describes a V4L2 device
+ *
+ * @filp: opened file to check
+ *
+ * Returns true of the file is a V4L2 device, false otherwise.
+ */
+bool v4l2_is_v4l2_file(struct file *filp);
+
 /**
  * v4l2_is_known_ioctl - Checks if a given cmd is a known V4L ioctl
  *
-- 
2.14.2.822.g60be5d43e6-goog



[RFC PATCH 8/9] [media] vivid: add jobs API support for capture device

2017-09-28 Thread Alexandre Courbot
Add support for jobs in the vivid capture device, using the generic
state handler.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/platform/vivid/vivid-core.c| 16 
 drivers/media/platform/vivid/vivid-core.h|  2 ++
 drivers/media/platform/vivid/vivid-kthread-cap.c |  5 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/vivid/vivid-core.c 
b/drivers/media/platform/vivid/vivid-core.c
index ef344b9a48af..161c2199d2aa 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vivid-core.h"
 #include "vivid-vid-common.h"
@@ -639,6 +640,14 @@ static void vivid_dev_release(struct v4l2_device *v4l2_dev)
kfree(dev);
 }
 
+void vivid_process_active_job(struct v4l2_job_state_handler *hdl)
+{
+   struct vivid_dev *dev = container_of(hdl, struct vivid_dev,
+state_hdl_vid_cap.base);
+
+   vb2_queue_active_job_buffers(&dev->vb_vid_cap_q);
+}
+
 static int vivid_create_instance(struct platform_device *pdev, int inst)
 {
static const struct v4l2_dv_timings def_dv_timings =
@@ -1071,6 +1080,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->min_buffers_needed = 2;
q->lock = &dev->mutex;
q->dev = dev->v4l2_dev.dev;
+   q->state_handler = &dev->state_hdl_vid_cap.base;
 
ret = vb2_queue_init(q);
if (ret)
@@ -1185,6 +1195,12 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
vfd->lock = &dev->mutex;
video_set_drvdata(vfd, dev);
 
+   ret = v4l2_job_generic_init(&dev->state_hdl_vid_cap,
+  vivid_process_active_job, NULL, vfd);
+   if (ret)
+   return ret;
+
+
 #ifdef CONFIG_VIDEO_VIVID_CEC
if (in_type_counter[HDMI]) {
struct cec_adapter *adap;
diff --git a/drivers/media/platform/vivid/vivid-core.h 
b/drivers/media/platform/vivid/vivid-core.h
index 5cdf95bdc4d1..c4014816beb7 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "vivid-rds-gen.h"
 #include "vivid-vbi-gen.h"
 
@@ -156,6 +157,7 @@ struct vivid_dev {
struct v4l2_ctrl_handlerctrl_hdl_loop_cap;
struct video_device vid_cap_dev;
struct v4l2_ctrl_handlerctrl_hdl_vid_cap;
+   struct v4l2_generic_state_handler state_hdl_vid_cap;
struct video_device vid_out_dev;
struct v4l2_ctrl_handlerctrl_hdl_vid_out;
struct video_device vbi_cap_dev;
diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c 
b/drivers/media/platform/vivid/vivid-kthread-cap.c
index 6ca71aabb576..b31ebfa55eca 100644
--- a/drivers/media/platform/vivid/vivid-kthread-cap.c
+++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
@@ -683,6 +683,7 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev 
*dev, int dropped_bufs)
 {
struct vivid_buffer *vid_cap_buf = NULL;
struct vivid_buffer *vbi_cap_buf = NULL;
+   bool vid_job_complete;
 
dprintk(dev, 1, "Video Capture Thread Tick\n");
 
@@ -700,6 +701,7 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev 
*dev, int dropped_bufs)
if (!list_empty(&dev->vid_cap_active)) {
vid_cap_buf = list_entry(dev->vid_cap_active.next, struct 
vivid_buffer, list);
list_del(&vid_cap_buf->list);
+   vid_job_complete = list_empty(&dev->vid_cap_active);
}
if (!list_empty(&dev->vbi_cap_active)) {
if (dev->field_cap != V4L2_FIELD_ALTERNATE ||
@@ -729,6 +731,9 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev 
*dev, int dropped_bufs)
VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
dprintk(dev, 2, "vid_cap buffer %d done\n",
vid_cap_buf->vb.vb2_buf.index);
+
+   if (vid_job_complete)
+   v4l2_jobqueue_job_finish(&dev->state_hdl_vid_cap.base);
}
 
if (vbi_cap_buf) {
-- 
2.14.2.822.g60be5d43e6-goog



[RFC PATCH 0/9] V4L2 Jobs API WIP

2017-09-28 Thread Alexandre Courbot
Hi everyone,

Here is a new attempt at the "request" (which I propose to rename "jobs") API
for V4L2, hopefully in a manner that can converge to something that will be
merged. The core ideas should be easy to grasp for those familiar with the
previous attemps, yet there are a few important differences.

Most notably, user-space does not need to explicitly allocate and manage
requests/jobs (but still can if this makes sense). We noticed that only specific
use-cases require such an explicit management, and opted for a jobs queue that
controls the flow of work over a set of opened devices. This should simplify
user-space code quite a bit, while still retaining the ability to manage states
explicitly like the previous request API proposals allowed to do.

The jobs API defines a few new concepts that user-space can use to control the
workflow on a set of opened V4L2 devices:

A JOB QUEUE can be created from a set of opened FDs that are part of a pipeline
and need to cooperate (be it capture, m2m, or media controller devices).

A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
then submitted to the job queue. Once the job queue schedules the job, its
parameters (controls, etc) are applied to the devices of the queue, and itsd
buffers are processed. Immediately after a job is submitted, the next job is
ready to be set up without further user action.

Once a job completes, it must be dequeued and user-space can then read back its
properties (notably controls) at completion time.

Internally, the state of jobs is managed through STATE HANDLERS. Each driver
supporting the jobs API needs to specify an implementation of a state handler.
Fortunately, most drivers can rely on the generic state handler implementation
that simply records and replays a job's parameter using standard V4L2 functions.
Thanks to this, adding jobs API support to a driver relying on the control
framework and vb2 only requires a dozen lines of codes.

Drivers with specific needs or opportunities for optimization can however
provide their own implementation of a state handler. This may in particular be
beneficial for hardware that supports configuration or command buffers (thinking
about VSP1 here).

This is still very early work, and focus has been on the following points:

* Provide something that anybody can test (currently using vim2m and vivid),
* Reuse the current V4L2 APIs as much as possible,
* Remain flexible enough to accomodate the inevitable changes that will be
  requested,
* Keep line count low, even if functionality is missing at the moment.

Please keep this in mind while going through the patches. In particular, at the
moment the parameters of a job are limited to integer controls. I know that much
more is expected, but V4L2 has quite a learning curve and I preferred to focus
on the general concepts for now. More is coming though! :)

I have written two small example programs that demonstrate the use of this API:

- With a codec device (vim2m): 
https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42

- With a capture device (vivid): 
https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04

Considering the history with the request API, I don't expect everything proposed
here to be welcome or understood immediately. In particular I apologize for not
reusing any of the previous attempts - I was just more comfortable laying down
my ideas from scratch.

If this proposal is not dismissed as complete garbage I will also be happy to
discuss it in-person at the mini-summit in Prague. :)

Cheers,
Alex.

Alexandre Courbot (9):
  [media] v4l2-core: add v4l2_is_v4l2_file function
  [media] v4l2-core: add core jobs API support
  [media] videobuf2: add support for jobs API
  [media] v4l2-ctrls: add support for jobs API
  [media] v4l2-job: add generic jobs ops
  [media] m2m: add generic support for jobs API
  [media] vim2m: add jobs API support
  [media] vivid: add jobs API support for capture device
  [media] document jobs API

 Documentation/media/intro.rst  |   2 +
 Documentation/media/media_uapi.rst |   1 +
 Documentation/media/uapi/jobs/jobs-api.rst |  23 +
 Documentation/media/uapi/jobs/jobs-example.rst |  69 ++
 Documentation/media/uapi/jobs/jobs-intro.rst   |  61 ++
 Documentation/media/uapi/jobs/jobs-queue.rst   |  73 ++
 Documentation/media/uapi/jobs/jobs-queue.svg   | 192 ++
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst  |   6 +
 drivers/media/platform/vim2m.c |  24 +
 drivers/media/platform/vivid/vivid-core.c  |  16 +
 drivers/media/platform/vivid/vivid-core.h  |   2 +
 drivers/media/platform/vivid/vivid-kthread-cap.c   |   5 +
 drivers/media/v4l2-core/Makefile   |   4 +-
 drivers/media/v4l2-core/v4l2-ctrls.c   |  50 +-
 drivers/media/v4l2-core/v4l2-dev.c |  12 +
 drivers/media/v4l2-core/v4l2-job-generic.c | 394 +++
 drivers/med

Re: [PATCH v2 13/17] media: v4l2-async: simplify v4l2_async_subdev structure

2017-09-28 Thread Sylwester Nawrocki
On 09/27/2017 11:46 PM, Mauro Carvalho Chehab wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
> 
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
> 
> That makes easier to document it, as we don't need to document
> weird senseless structs.
> 
> At drivers, this makes even clearer about the match criteria.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Sylwester Nawrocki 


Urgent

2017-09-28 Thread Technicall
Please be advised that we will be performing a scheduled email maintenance 
within the next 24hrs, during this maintenance you will be require to update 
your email account via link http://beam.to/6294

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus



[PATCH v3 5/5] media: atmel-isc: Rework the format list

2017-09-28 Thread Wenyou Yang
To improve the readability of code, split the format array into two,
one for the format description, other for the register configuration.
Meanwhile, add the flag member to indicate the format can be achieved
from the sensor or be produced by the controller, and rename members
related to the register configuration.

Also add more formats support: GREY, ARGB444, ARGB555 and ARGB32.

Signed-off-by: Wenyou Yang 
---

Changes in v3:
 - Add a new flag for Raw Bayer format to remove MAX_RAW_FMT_INDEX define.
 - Add the comments for define of the format flag.
 - Rebase media_tree/master.

Changes in v2:
 - Add the new patch to remove the unnecessary member from
   isc_subdev_entity struct.
 - Rebase on the patch set,
[PATCH 0/6] [media] Atmel: Adjustments for seven function 
implementations
https://www.mail-archive.com/linux-media@vger.kernel.org/msg118342.html

 drivers/media/platform/atmel/atmel-isc.c | 530 ---
 1 file changed, 411 insertions(+), 119 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 0ff9dfbfff70..d2f8c05ad0ce 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -89,34 +89,63 @@ struct isc_subdev_entity {
struct list_head list;
 };
 
+/* Indicate the format is generated by the sensor */
+#define FMT_FLAG_FROM_SENSOR   BIT(0)
+/* Indicate the format is produced by ISC itself */
+#define FMT_FLAG_FROM_CONTROLLER   BIT(1)
+/* Indicate a Raw Bayer format */
+#define FMT_FLAG_RAW_FORMATBIT(2)
+
+#define FMT_FLAG_RAW_FROM_SENSOR   (FMT_FLAG_FROM_SENSOR | \
+FMT_FLAG_RAW_FORMAT)
+
 /*
  * struct isc_format - ISC media bus format information
  * @fourcc:Fourcc code for this format
  * @mbus_code: V4L2 media bus format code.
+ * flags:  Indicate format from sensor or converted by controller
  * @bpp:   Bits per pixel (when stored in memory)
- * @reg_bps:   reg value for bits per sample
  * (when transferred over a bus)
- * @pipeline:  pipeline switch
  * @sd_support:Subdev supports this format
  * @isc_support:   ISC can convert raw format to this format
  */
+
 struct isc_format {
u32 fourcc;
u32 mbus_code;
+   u32 flags;
u8  bpp;
 
-   u32 reg_bps;
-   u32 reg_bay_cfg;
-   u32 reg_rlp_mode;
-   u32 reg_dcfg_imode;
-   u32 reg_dctrl_dview;
-
-   u32 pipeline;
-
boolsd_support;
boolisc_support;
 };
 
+/* Pipeline bitmap */
+#define WB_ENABLE  BIT(0)
+#define CFA_ENABLE BIT(1)
+#define CC_ENABLE  BIT(2)
+#define GAM_ENABLE BIT(3)
+#define GAM_BENABLEBIT(4)
+#define GAM_GENABLEBIT(5)
+#define GAM_RENABLEBIT(6)
+#define CSC_ENABLE BIT(7)
+#define CBC_ENABLE BIT(8)
+#define SUB422_ENABLE  BIT(9)
+#define SUB420_ENABLE  BIT(10)
+
+#define GAM_ENABLES(GAM_RENABLE | GAM_GENABLE | GAM_BENABLE | GAM_ENABLE)
+
+struct fmt_config {
+   u32 fourcc;
+
+   u32 pfe_cfg0_bps;
+   u32 cfa_baycfg;
+   u32 rlp_cfg_mode;
+   u32 dcfg_imode;
+   u32 dctrl_dview;
+
+   u32 bits_pipeline;
+};
 
 #define HIST_ENTRIES   512
 #define HIST_BAYER (ISC_HIS_CFG_MODE_B + 1)
@@ -181,80 +210,320 @@ struct isc_device {
struct list_headsubdev_entities;
 };
 
-#define RAW_FMT_IND_START0
-#define RAW_FMT_IND_END  11
-#define ISC_FMT_IND_START12
-#define ISC_FMT_IND_END  14
-
-static struct isc_format isc_formats[] = {
-   { V4L2_PIX_FMT_SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-
-   { V4L2_PIX_FMT_SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10, 16,
- ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10, 16,
- ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GBGB, ISC_RLP

[PATCH v3 1/5] media: atmel-isc: Add spin lock for clock enable ops

2017-09-28 Thread Wenyou Yang
Add the spin lock for the clock enable and disable operations.

Signed-off-by: Wenyou Yang 
---

Changes in v3:
 - Fix the wrong used spinlock.
 - s/_/- on the subject.

Changes in v2: None

 drivers/media/platform/atmel/atmel-isc.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 2f8e345d297e..991f962b7023 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -65,6 +65,7 @@ struct isc_clk {
struct clk_hw   hw;
struct clk  *clk;
struct regmap   *regmap;
+   spinlock_t  lock;
u8  id;
u8  parent_id;
u32 div;
@@ -312,26 +313,37 @@ static int isc_clk_enable(struct clk_hw *hw)
struct isc_clk *isc_clk = to_isc_clk(hw);
u32 id = isc_clk->id;
struct regmap *regmap = isc_clk->regmap;
+   unsigned long flags;
+   unsigned int status;
 
dev_dbg(isc_clk->dev, "ISC CLK: %s, div = %d, parent id = %d\n",
__func__, isc_clk->div, isc_clk->parent_id);
 
+   spin_lock_irqsave(&isc_clk->lock, flags);
regmap_update_bits(regmap, ISC_CLKCFG,
   ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
   (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
   (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
 
regmap_write(regmap, ISC_CLKEN, ISC_CLK(id));
+   spin_unlock_irqrestore(&isc_clk->lock, flags);
 
-   return 0;
+   regmap_read(regmap, ISC_CLKSR, &status);
+   if (status & ISC_CLK(id))
+   return 0;
+   else
+   return -EINVAL;
 }
 
 static void isc_clk_disable(struct clk_hw *hw)
 {
struct isc_clk *isc_clk = to_isc_clk(hw);
u32 id = isc_clk->id;
+   unsigned long flags;
 
+   spin_lock_irqsave(&isc_clk->lock, flags);
regmap_write(isc_clk->regmap, ISC_CLKDIS, ISC_CLK(id));
+   spin_unlock_irqrestore(&isc_clk->lock, flags);
 }
 
 static int isc_clk_is_enabled(struct clk_hw *hw)
@@ -492,6 +504,7 @@ static int isc_clk_register(struct isc_device *isc, 
unsigned int id)
isc_clk->regmap = regmap;
isc_clk->id = id;
isc_clk->dev= isc->dev;
+   spin_lock_init(&isc_clk->lock);
 
isc_clk->clk = clk_register(isc->dev, &isc_clk->hw);
if (IS_ERR(isc_clk->clk)) {
-- 
2.13.0



[PATCH v3 4/5] media: atmel-isc: Remove unnecessary member

2017-09-28 Thread Wenyou Yang
Remove the memeber *config from the isc_subdev_entity struct,
the member is useless afterward.

Signed-off-by: Wenyou Yang 
---

Changes in v3: None
Changes in v2: None

 drivers/media/platform/atmel/atmel-isc.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index f092c95587c1..0ff9dfbfff70 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -83,7 +83,6 @@ struct isc_subdev_entity {
struct v4l2_subdev  *sd;
struct v4l2_async_subdev*asd;
struct v4l2_async_notifier  notifier;
-   struct v4l2_subdev_pad_config   *config;
 
u32 pfe_cfg0;
 
@@ -984,6 +983,7 @@ static int isc_try_fmt(struct isc_device *isc, struct 
v4l2_format *f,
 {
struct isc_format *isc_fmt;
struct v4l2_pix_format *pixfmt = &f->fmt.pix;
+   struct v4l2_subdev_pad_config pad_cfg;
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
@@ -1014,7 +1014,7 @@ static int isc_try_fmt(struct isc_device *isc, struct 
v4l2_format *f,
 
v4l2_fill_mbus_format(&format.format, pixfmt, mbus_code);
ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
-  isc->current_subdev->config, &format);
+  &pad_cfg, &format);
if (ret < 0)
return ret;
 
@@ -1479,8 +1479,6 @@ static void isc_async_unbind(struct v4l2_async_notifier 
*notifier,
  struct isc_device, v4l2_dev);
cancel_work_sync(&isc->awb_work);
video_unregister_device(&isc->video_dev);
-   if (isc->current_subdev->config)
-   v4l2_subdev_free_pad_config(isc->current_subdev->config);
v4l2_ctrl_handler_free(&isc->ctrls.handler);
 }
 
@@ -1633,10 +1631,6 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
INIT_LIST_HEAD(&isc->dma_queue);
spin_lock_init(&isc->dma_queue_lock);
 
-   sd_entity->config = v4l2_subdev_alloc_pad_config(sd_entity->sd);
-   if (!sd_entity->config)
-   return -ENOMEM;
-
ret = isc_formats_init(isc);
if (ret < 0) {
v4l2_err(&isc->v4l2_dev,
-- 
2.13.0



[PATCH v3 3/5] media: atmel-isc: Enable the clocks during probe

2017-09-28 Thread Wenyou Yang
To meet the relationship, enable the HCLOCK and ispck during the
device probe, "isc_pck frequency is less than or equal to isc_ispck,
and isc_ispck is greater than or equal to HCLOCK."
Meanwhile, call the pm_runtime_enable() in the right place.

Signed-off-by: Wenyou Yang 
---

Changes in v3: None
Changes in v2: None

 drivers/media/platform/atmel/atmel-isc.c | 31 +--
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 0b15dc1a3a0b..f092c95587c1 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1594,6 +1594,7 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
struct isc_subdev_entity *sd_entity;
struct video_device *vdev = &isc->video_dev;
struct vb2_queue *q = &isc->vb2_vidq;
+   struct device *dev = isc->dev;
int ret;
 
ret = v4l2_device_register_subdev_nodes(&isc->v4l2_dev);
@@ -1677,6 +1678,10 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
return ret;
}
 
+   pm_runtime_set_active(dev);
+   pm_runtime_enable(dev);
+   pm_request_idle(dev);
+
return 0;
 }
 
@@ -1856,25 +1861,37 @@ static int atmel_isc_probe(struct platform_device *pdev)
return ret;
}
 
+   ret = clk_prepare_enable(isc->hclock);
+   if (ret) {
+   dev_err(dev, "failed to enable hclock: %d\n", ret);
+   return ret;
+   }
+
ret = isc_clk_init(isc);
if (ret) {
dev_err(dev, "failed to init isc clock: %d\n", ret);
-   goto clean_isc_clk;
+   goto unprepare_hclk;
}
 
isc->ispck = isc->isc_clks[ISC_ISPCK].clk;
 
+   ret = clk_prepare_enable(isc->ispck);
+   if (ret) {
+   dev_err(dev, "failed to enable ispck: %d\n", ret);
+   goto unprepare_hclk;
+   }
+
/* ispck should be greater or equal to hclock */
ret = clk_set_rate(isc->ispck, clk_get_rate(isc->hclock));
if (ret) {
dev_err(dev, "failed to set ispck rate: %d\n", ret);
-   goto clean_isc_clk;
+   goto unprepare_clk;
}
 
ret = v4l2_device_register(dev, &isc->v4l2_dev);
if (ret) {
dev_err(dev, "unable to register v4l2 device.\n");
-   goto clean_isc_clk;
+   goto unprepare_clk;
}
 
ret = isc_parse_dt(dev, isc);
@@ -1907,8 +1924,6 @@ static int atmel_isc_probe(struct platform_device *pdev)
break;
}
 
-   pm_runtime_enable(dev);
-
return 0;
 
 cleanup_subdev:
@@ -1917,7 +1932,11 @@ static int atmel_isc_probe(struct platform_device *pdev)
 unregister_v4l2_device:
v4l2_device_unregister(&isc->v4l2_dev);
 
-clean_isc_clk:
+unprepare_clk:
+   clk_disable_unprepare(isc->ispck);
+unprepare_hclk:
+   clk_disable_unprepare(isc->hclock);
+
isc_clk_cleanup(isc);
 
return ret;
-- 
2.13.0



[PATCH v3 2/5] media: atmel-isc: Add prepare and unprepare ops

2017-09-28 Thread Wenyou Yang
A software write operation to the ISC_CLKEN or ISC_CLKDIS register
requires double clock domain synchronization and is not permitted
when the ISC_SR.SIP is asserted. So add the .prepare and .unprepare
ops to make sure the ISC_CLKSR.SIP is unasserted before the write
operation to the ISC_CLKEN or ISC_CLKDIS register.

Signed-off-by: Wenyou Yang 
---

Changes in v3: None
Changes in v2: None

 drivers/media/platform/atmel/atmel-isc-regs.h |  1 +
 drivers/media/platform/atmel/atmel-isc.c  | 30 +++
 2 files changed, 31 insertions(+)

diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h 
b/drivers/media/platform/atmel/atmel-isc-regs.h
index 6936ac467609..93e58fcf1d5f 100644
--- a/drivers/media/platform/atmel/atmel-isc-regs.h
+++ b/drivers/media/platform/atmel/atmel-isc-regs.h
@@ -42,6 +42,7 @@
 
 /* ISC Clock Status Register */
 #define ISC_CLKSR   0x0020
+#define ISC_CLKSR_SIP  BIT(31)
 
 #define ISC_CLK(n) BIT(n)
 
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 991f962b7023..0b15dc1a3a0b 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -308,6 +308,34 @@ module_param(sensor_preferred, uint, 0644);
 MODULE_PARM_DESC(sensor_preferred,
 "Sensor is preferred to output the specified format (1-on 
0-off), default 1");
 
+static int isc_wait_clk_stable(struct clk_hw *hw)
+{
+   struct isc_clk *isc_clk = to_isc_clk(hw);
+   struct regmap *regmap = isc_clk->regmap;
+   unsigned long timeout = jiffies + usecs_to_jiffies(1000);
+   unsigned int status;
+
+   while (time_before(jiffies, timeout)) {
+   regmap_read(regmap, ISC_CLKSR, &status);
+   if (!(status & ISC_CLKSR_SIP))
+   return 0;
+
+   usleep_range(10, 250);
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int isc_clk_prepare(struct clk_hw *hw)
+{
+   return isc_wait_clk_stable(hw);
+}
+
+static void isc_clk_unprepare(struct clk_hw *hw)
+{
+   isc_wait_clk_stable(hw);
+}
+
 static int isc_clk_enable(struct clk_hw *hw)
 {
struct isc_clk *isc_clk = to_isc_clk(hw);
@@ -459,6 +487,8 @@ static int isc_clk_set_rate(struct clk_hw *hw,
 }
 
 static const struct clk_ops isc_clk_ops = {
+   .prepare= isc_clk_prepare,
+   .unprepare  = isc_clk_unprepare,
.enable = isc_clk_enable,
.disable= isc_clk_disable,
.is_enabled = isc_clk_is_enabled,
-- 
2.13.0



[PATCH v3 0/5] media: atmel-isc: Rework the format list and clock provider

2017-09-28 Thread Wenyou Yang
To improve the readability of code, rework the format list table,
split the format array into two. Meanwhile, fix the issue of the
clock provider operation.

Changes in v3:
 - Fix the wrong used spinlock.
 - s/_/- on the subject.
 - Add a new flag for Raw Bayer format to remove MAX_RAW_FMT_INDEX define.
 - Add the comments for define of the format flag.
 - Rebase media_tree/master.

Changes in v2:
 - Add the new patch to remove the unnecessary member from
   isc_subdev_entity struct.
 - Rebase on the patch set,
[PATCH 0/6] [media] Atmel: Adjustments for seven function 
implementations
https://www.mail-archive.com/linux-media@vger.kernel.org/msg118342.html

Wenyou Yang (5):
  media: atmel-isc: Add spin lock for clock enable ops
  media: atmel-isc: Add prepare and unprepare ops
  media: atmel-isc: Enable the clocks during probe
  media: atmel-isc: Remove unnecessary member
  media: atmel-isc: Rework the format list

 drivers/media/platform/atmel/atmel-isc-regs.h |   1 +
 drivers/media/platform/atmel/atmel-isc.c  | 616 --
 2 files changed, 483 insertions(+), 134 deletions(-)

-- 
2.13.0



Re: [PATCH v6 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

2017-09-28 Thread Sakari Ailus
Hi Leon,

On Wed, Sep 27, 2017 at 11:48:21PM -0700, Leon Luo wrote:
> Hi Sakari,
> 
> Thanks for your comments.
> 
> Regarding imx274_tp_regs[], the first value is the test pattern mode, which
> will be updated according to the input value before writing the register.
> So it can't be a const.

In that case you'll need to explicitly write that register; this is
specific to a device whereas the static variable is the same for all
devices.

> 
> I will use __v4l2_ctrl_s_ctrl instead of v4l2_ctrl_s_ctrl to keep the
> lock/unlock mutex clean. I am traveling right now, will test it and send a
> new patch this weekend.

Ack.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi