Re: [PATCH 1/1] [media] radio-sf16fmr2: Remove redundant dev_set_drvdata

[Hans Verkuil]

On 09/20/2013 10:37 AM, Sachin Kamat wrote:
> Driver core sets driver data to NULL upon failure or remove.
> 
> Signed-off-by: Sachin Kamat 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/radio/radio-sf16fmr2.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/radio/radio-sf16fmr2.c 
> b/drivers/media/radio/radio-sf16fmr2.c
> index f1e3714..448cac9 100644
> --- a/drivers/media/radio/radio-sf16fmr2.c
> +++ b/drivers/media/radio/radio-sf16fmr2.c
> @@ -295,7 +295,6 @@ static void fmr2_remove(struct fmr2 *fmr2)
>  static int fmr2_isa_remove(struct device *pdev, unsigned int ndev)
>  {
>   fmr2_remove(dev_get_drvdata(pdev));
> - dev_set_drvdata(pdev, NULL);
>  
>   return 0;
>  }
> 

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

Re: [PATCH 1/3] [media] pci: cx88-alsa: Use module_pci_driver

[Hans Verkuil]

On 09/20/2013 10:32 AM, Sachin Kamat wrote:
> module_pci_driver removes some boilerplate and makes code simpler.
> 
> Signed-off-by: Sachin Kamat 

For this patch series:

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/pci/cx88/cx88-alsa.c |   25 +
>  1 file changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/drivers/media/pci/cx88/cx88-alsa.c 
> b/drivers/media/pci/cx88/cx88-alsa.c
> index 05428bf..11d0692 100644
> --- a/drivers/media/pci/cx88/cx88-alsa.c
> +++ b/drivers/media/pci/cx88/cx88-alsa.c
> @@ -949,27 +949,4 @@ static struct pci_driver cx88_audio_pci_driver = {
>   .remove   = cx88_audio_finidev,
>  };
>  
> -/
> - LINUX MODULE INIT
> - 
> /
> -
> -/*
> - * module init
> - */
> -static int __init cx88_audio_init(void)
> -{
> - printk(KERN_INFO "cx2388x alsa driver version %s loaded\n",
> -CX88_VERSION);
> - return pci_register_driver(&cx88_audio_pci_driver);
> -}
> -
> -/*
> - * module remove
> - */
> -static void __exit cx88_audio_fini(void)
> -{
> - pci_unregister_driver(&cx88_audio_pci_driver);
> -}
> -
> -module_init(cx88_audio_init);
> -module_exit(cx88_audio_fini);
> +module_pci_driver(cx88_audio_pci_driver);
> 

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

Re: [PATCH 1/9] [media] pci: flexcop: Remove redundant pci_set_drvdata

[Hans Verkuil]

On 09/20/2013 10:36 AM, Sachin Kamat wrote:
> Driver core sets driver data to NULL upon failure or remove.
> 
> Signed-off-by: Sachin Kamat 
> Cc: Patrick Boettcher 

For this patch series:

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/pci/b2c2/flexcop-pci.c |2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/media/pci/b2c2/flexcop-pci.c 
> b/drivers/media/pci/b2c2/flexcop-pci.c
> index 447afbd..8b5e0b3 100644
> --- a/drivers/media/pci/b2c2/flexcop-pci.c
> +++ b/drivers/media/pci/b2c2/flexcop-pci.c
> @@ -319,7 +319,6 @@ static int flexcop_pci_init(struct flexcop_pci *fc_pci)
>  
>  err_pci_iounmap:
>   pci_iounmap(fc_pci->pdev, fc_pci->io_mem);
> - pci_set_drvdata(fc_pci->pdev, NULL);
>  err_pci_release_regions:
>   pci_release_regions(fc_pci->pdev);
>  err_pci_disable_device:
> @@ -332,7 +331,6 @@ static void flexcop_pci_exit(struct flexcop_pci *fc_pci)
>   if (fc_pci->init_state & FC_PCI_INIT) {
>   free_irq(fc_pci->pdev->irq, fc_pci);
>   pci_iounmap(fc_pci->pdev, fc_pci->io_mem);
> - pci_set_drvdata(fc_pci->pdev, NULL);
>   pci_release_regions(fc_pci->pdev);
>   pci_disable_device(fc_pci->pdev);
>   }
> 

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

Re: [PATCH 8/9] [media] pci: bt878: Remove redundant pci_set_drvdata

[Manu Abraham]

On Fri, Sep 20, 2013 at 2:06 PM, Sachin Kamat  wrote:
> Driver core sets driver data to NULL upon failure or remove.
>
> Signed-off-by: Sachin Kamat 
Acked-by: Manu Abraham 
> ---
>  drivers/media/pci/bt8xx/bt878.c |1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/pci/bt8xx/bt878.c b/drivers/media/pci/bt8xx/bt878.c
> index 66eb0ba..2bd2483 100644
> --- a/drivers/media/pci/bt8xx/bt878.c
> +++ b/drivers/media/pci/bt8xx/bt878.c
> @@ -563,7 +563,6 @@ static void bt878_remove(struct pci_dev *pci_dev)
> bt->shutdown = 1;
> bt878_mem_free(bt);
>
> -   pci_set_drvdata(pci_dev, NULL);
> pci_disable_device(pci_dev);
> return;
>  }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] [media] pci: mantis: Remove redundant pci_set_drvdata

[Manu Abraham]

On Fri, Sep 20, 2013 at 2:06 PM, Sachin Kamat  wrote:
> Driver core sets driver data to NULL upon failure or remove.
>
> Signed-off-by: Sachin Kamat 
> Cc: Manu Abraham 
Acked-by: Manu Abraham 
> ---
>  drivers/media/pci/mantis/mantis_pci.c |2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/media/pci/mantis/mantis_pci.c 
> b/drivers/media/pci/mantis/mantis_pci.c
> index a846036..9e89e04 100644
> --- a/drivers/media/pci/mantis/mantis_pci.c
> +++ b/drivers/media/pci/mantis/mantis_pci.c
> @@ -143,7 +143,6 @@ fail1:
>
>  fail0:
> dprintk(MANTIS_ERROR, 1, "ERROR: <%d> exiting", ret);
> -   pci_set_drvdata(pdev, NULL);
> return ret;
>  }
>  EXPORT_SYMBOL_GPL(mantis_pci_init);
> @@ -161,7 +160,6 @@ void mantis_pci_exit(struct mantis_pci *mantis)
> }
>
> pci_disable_device(pdev);
> -   pci_set_drvdata(pdev, NULL);
>  }
>  EXPORT_SYMBOL_GPL(mantis_pci_exit);
>
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Hauppauge HVR-900 HD and HVR 930C-HD with si2165

[Matthias Schwarzott]

On 26.09.2013 16:54, Antti Palosaari wrote:

On 25.09.2013 07:50, Matthias Schwarzott wrote:

On 17.08.2013 13:30, Ulf wrote:

Hi,

I know the topic Hauppauge HVR-900 HD and HVR 930C-HD with si2165
demodulator was already discussed
http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/40982 


and
http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/46266. 



Just for me as a confirmation nobody plans to work on a driver for
si2165.
Is there any chance how to push the development?

Ulf

Hi!

I also bought one of these to find out it is not supported.
But my plan is to try to write a driver for this.
I want to get DVB-C working, but I also have DVB-T and analog reception
available.

My current status is I got it working in windows in qemu and did a usb
snoop.
I also have a second system to test it in windows vista directly on the
hardware.

Current status is documented here.
http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-HVR-930C-HD

Until now I only have a component list summarized from this list.

  * Conexant  CX231xx

  * Silicon Labs

 



si2165 
(Multi-Standard DVB-T and DVB-C Demodulator)
  * NXP TDA18271

(silicon tuner IC, most likely i2c-addr: 0x60)
  * eeprom (windows driver reads 1kb, i2c-addr: 0x50)


Is this correct?
Did anyone open his device and can show pictures?

I now need to know which component is at which i2c address.
Windows driver does upload file hcw10mlD.rom of 16kb to device 0x44.


I have opened it. There was similar sandwich PCB than used by rev1 
too. So you cannot see all the chip unless you use metal saw to 
separate PCBs.


PCB side A:
TDA18271HDC2
16.000 MHz

Si2165-GM
16.000 MHz


PCB side B:
24C02H

regards
Antti


Hi Antti,

thanks for that information.
The only real new information for me is the 16.000MHz xtal value.

Sad to know that the other chips are hidden.
I assigned more i2c addresses to functions, but not yet all (no idea if 
more addresses are real, or bad interpretations of snooped data).


I now try to check what already works:
- This is video via composite input.
- Next is to try video via analog input - see I see if the tuner in 
general works in this device.


In parallel I try to capture usb in different setups.
1. kvm+tcpdump (using usbmon)
2. usbsnoop on windows vista

Only setup 1 does provide a real list of usb packets.

Regards
Matthias

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

Re: [PATCH V5 1/5] ARM: dts: Add MIPI PHY node to exynos4.dtsi

[Kishon Vijay Abraham I]

Hi,


On Sunday 29 September 2013 12:57 AM, Sylwester Nawrocki wrote:
> Add PHY provider node for the MIPI CSIS and MIPI DSIM PHYs.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyungmin Park 
> Acked-by: Felipe Balbi 

Can this patch be taken through exynos dt tree?

Thanks
Kishon
> ---
>  arch/arm/boot/dts/exynos4.dtsi |   10 ++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index caadc02..a73eeb5 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -49,6 +49,12 @@
>   reg = <0x1000 0x100>;
>   };
>  
> + mipi_phy: video-phy@10020710 {
> + compatible = "samsung,s5pv210-mipi-video-phy";
> + reg = <0x10020710 8>;
> + #phy-cells = <1>;
> + };
> +
>   pd_mfc: mfc-power-domain@10023C40 {
>   compatible = "samsung,exynos4210-pd";
>   reg = <0x10023C40 0x20>;
> @@ -161,6 +167,8 @@
>   clock-names = "csis", "sclk_csis";
>   bus-width = <4>;
>   samsung,power-domain = <&pd_cam>;
> + phys = <&mipi_phy 0>;
> + phy-names = "csis";
>   status = "disabled";
>   #address-cells = <1>;
>   #size-cells = <0>;
> @@ -174,6 +182,8 @@
>   clock-names = "csis", "sclk_csis";
>   bus-width = <2>;
>   samsung,power-domain = <&pd_cam>;
> + phys = <&mipi_phy 2>;
> + phy-names = "csis";
>   status = "disabled";
>   #address-cells = <1>;
>   #size-cells = <0>;
> 

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

[no subject]

[Mr. Donald Hall]

Hello,


  Do you need a loan? Do you need money to pay off your debts? Do you need 
money to establish a
new business or to refinance an existing business? our loan company offers 
loans at 3% interest
rate with an understandable loan terms and condition. If you are interested in 
obtaining a loan
contact us today with your information/ Loan information to enable us proceed 
with offering you
your required loan. Contact us for a loan today via our email; 
apex.loanfi...@sify.com


 You could also apply by filling out our loan form on our website: 
www.apexloanfirms.com


We will be happy to have you as our client.


Regards


Mr. Donald Hall
Apex Loans/CEO

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

Re: [PATCH 00/10] [media] remove unnecessary pci_set_drvdata()

[Sachin Kamat]

Hi Hans,

On 30 September 2013 17:52, Hans Verkuil  wrote:
> On 09/23/2013 03:43 AM, Jingoo Han wrote:
>> Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
>> (device-core: Ensure drvdata = NULL when no driver is bound),
>> the driver core clears the driver data to NULL after device_release
>> or on probe failure. Thus, it is not needed to manually clear the
>> device driver data to NULL.
>
> For the whole patch series:
>
> Acked-by: Hans Verkuil 
>

I had sent a similar series (3 days) earlier to Jingoo's  [1]. I hope
to get your ack for that one instead.

[1] http://www.spinics.net/lists/linux-media/msg68164.html


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

cron job: media_tree daily build: WARNINGS

[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:   Tue Oct  1 04:00:14 CEST 2013
git branch: test
git hash:   ffee921033e64edf8579a3b21c7f15d1a6c3ef71
gcc version:i686-linux-gcc (GCC) 4.8.1
sparse version: 0.4.5-rc1
host hardware:  x86_64
host os:3.10.1

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin: 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.31.14-i686: OK
linux-2.6.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.10.1-i686: OK
linux-3.1.10-i686: OK
linux-3.11.1-i686: OK
linux-3.12-rc1-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-2.6.31.14-x86_64: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12-rc1-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse version: 0.4.5-rc1
sparse: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT

[Sakari Ailus]

Hi Laurent,

Thnanks for the comments. A few more below.

On Tue, Oct 01, 2013 at 01:21:58AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday 01 October 2013 02:08:47 Sakari Ailus wrote:
> > On Fri, Sep 20, 2013 at 11:08:47PM +0200, Laurent Pinchart wrote:
> > > On Thursday 19 September 2013 01:01:05 Sakari Ailus wrote:
> > > > Pads that set this flag must be connected by an active link for the
> > > > entity
> > > > to stream.
> > > > 
> > > > Signed-off-by: Sakari Ailus 
> > > > Acked-by: Sylwester Nawrocki 
> > > > ---
> > > > 
> > > >  Documentation/DocBook/media/v4l/media-ioc-enum-links.xml |8 +++
> > > >  include/uapi/linux/media.h   |1 +
> > > >  2 files changed, 9 insertions(+)
> > > > 
> > > > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > > > b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml index
> > > > 355df43..59b212a 100644
> > > > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > > > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > > > @@ -134,6 +134,14 @@
> > > > Output pad, relative to the entity. Output pads 
> > > > source
> > > > data and are origins of links.
> > > >   
> > > > + 
> > > > +   
> > > > MEDIA_PAD_FL_MUST_CONNECT
> > > > +   A pad must be connected with an enabled link for the
> > > 
> > > s/A pad/The pad/ ?
> > 
> > Fixed.
> > 
> > > > +   entity to be able to stream. There could be temporary 
> > > > reasons
> > > > +   (e.g. device configuration dependent) for the pad to need
> > > > +   connecting; the absence of the flag won't say there
> > > > +   may not be any.
> > > 
> > > I believe the description doesn't make it very explicit that a
> > > MUST_CONNECT pad with no existing link is valid, as opposed to existing
> > > links with no enabled link, which would be invalid. Do you think we should
> > > fix that ?
> > 
> > Yes. I propose to add this: "The flag has no effect on pads without
> > connected links."
> 
> What about
> 
> If the pad is linked to any other pad, at least one of the links must be
> enabled for the entity to be able to stream. There could be temporary reasons
> (e.g. device configuration dependent) for the pad to need enabled links; the 
> absence of the flag doesn't imply there is none. The flag has no effect on
> pads without connected links.

Thinking about this again, I'd add before the comma: "and this flag is set".

And if you put it like that then the last sentence is redundat --- I'd drop
it.

What do you think?

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT

[Laurent Pinchart]

Hi Sakari,

On Tuesday 01 October 2013 02:08:47 Sakari Ailus wrote:
> On Fri, Sep 20, 2013 at 11:08:47PM +0200, Laurent Pinchart wrote:
> > On Thursday 19 September 2013 01:01:05 Sakari Ailus wrote:
> > > Pads that set this flag must be connected by an active link for the
> > > entity
> > > to stream.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > Acked-by: Sylwester Nawrocki 
> > > ---
> > > 
> > >  Documentation/DocBook/media/v4l/media-ioc-enum-links.xml |8 +++
> > >  include/uapi/linux/media.h   |1 +
> > >  2 files changed, 9 insertions(+)
> > > 
> > > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > > b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml index
> > > 355df43..59b212a 100644
> > > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > > @@ -134,6 +134,14 @@
> > >   Output pad, relative to the entity. Output pads source
> > >   data and are origins of links.
> > > 
> > > +   
> > > + MEDIA_PAD_FL_MUST_CONNECT
> > > + A pad must be connected with an enabled link for the
> > 
> > s/A pad/The pad/ ?
> 
> Fixed.
> 
> > > + entity to be able to stream. There could be temporary reasons
> > > + (e.g. device configuration dependent) for the pad to need
> > > + connecting; the absence of the flag won't say there
> > > + may not be any.
> > 
> > I believe the description doesn't make it very explicit that a
> > MUST_CONNECT pad with no existing link is valid, as opposed to existing
> > links with no enabled link, which would be invalid. Do you think we should
> > fix that ?
> 
> Yes. I propose to add this: "The flag has no effect on pads without
> connected links."

What about

If the pad is linked to any other pad, at least one of the links must be
enabled for the entity to be able to stream. There could be temporary reasons
(e.g. device configuration dependent) for the pad to need enabled links; the 
absence of the flag doesn't imply there is none. The flag has no effect on
pads without connected links.

> > > +   
> > >   
> > >
> > >  

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT

[Sakari Ailus]

Hi Laurent,

On Fri, Sep 20, 2013 at 11:08:47PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.

Thanks for the review! :)

> On Thursday 19 September 2013 01:01:05 Sakari Ailus wrote:
> > Pads that set this flag must be connected by an active link for the entity
> > to stream.
> > 
> > Signed-off-by: Sakari Ailus 
> > Acked-by: Sylwester Nawrocki 
> > ---
> >  Documentation/DocBook/media/v4l/media-ioc-enum-links.xml |8 
> >  include/uapi/linux/media.h   |1 +
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml index
> > 355df43..59b212a 100644
> > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> > @@ -134,6 +134,14 @@
> > Output pad, relative to the entity. Output pads source data
> > and are origins of links.
> >   
> > + 
> > +   MEDIA_PAD_FL_MUST_CONNECT
> > +   A pad must be connected with an enabled link for the
> 
> s/A pad/The pad/ ?

Fixed.

> > +   entity to be able to stream. There could be temporary reasons
> > +   (e.g. device configuration dependent) for the pad to need
> > +   connecting; the absence of the flag won't say there
> > +   may not be any.
> 
> I believe the description doesn't make it very explicit that a MUST_CONNECT 
> pad with no existing link is valid, as opposed to existing links with no 
> enabled link, which would be invalid. Do you think we should fix that ?

Yes. I propose to add this: "The flag has no effect on pads without
connected links."

> > + 
> > 
> >
> >  
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index ed49574..d847c76 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -98,6 +98,7 @@ struct media_entity_desc {
> > 
> >  #define MEDIA_PAD_FL_SINK  (1 << 0)
> >  #define MEDIA_PAD_FL_SOURCE(1 << 1)
> > +#define MEDIA_PAD_FL_MUST_CONNECT  (1 << 2)
> > 
> >  struct media_pad_desc {
> > __u32 entity;   /* entity ID */

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [media] r820t: fix nint range check

[Mauro Carvalho Chehab]

Em Mon, 30 Sep 2013 12:46:52 -0400
Michael Krufky  escreveu:

> Do you have any comments on this, Mauro?
> 
> Assuming that Mauro is OK with this change, (since he is the author of
> this driver) then yes - please resubmit the patch with some
> explanation within comments inline or within the commit message.

Michael,

Please don't top post. My comments are below.

> 
> Best regards,
> 
> Mike Krufky
> 
> On Mon, Sep 30, 2013 at 12:42 PM, Jiří Pinkava  wrote:
> > Mike,
> >
> > unfortunately no documentation can be referenced except preliminary
> > version of
> > datasheet (1).This change is based on lucky guess and supported by lot of
> > testing on real hardware.
> >
> > This change add support for devices with Xtal frequency bellow 28.8MHz.

Hmm... I'm not seeing any test there checking if the frequency of the xtal
is below to 28.8 MHz. So, if this logic apples only in this case, the
patch needs to add a test for the xtal frequency.

Also, as the driver does some tests for other Raphael tuners, if this change
is known to work fine only on rt820t, please add a test there for the tuner
model too.

Except for that, I'm ok with the changes, provided that you add the
explanations about it.

> >
> > From Nint  are computed values of Ni and Si. For 28.8MHz crystal can
> > reach up to 12 / 3 (Ni / Si). Tuner supports crystals with frequencies
> > (1) 12, 16, 20, 20.48, 24, 27, 28.8, 32 MHz, but this kind of device is
> > rare to found.
> > Allowing Ni to go up to 15 instead of only 12 should be safe and for 15
> > / 3 (Ni / Si)
> > we can compute limit for Nint = max(Ni) * 4 + max(Si) + 13 = 76.
> >
> > If This is sufficient and acceptable explanation I can add some sort of
> > documentation into patch and resend it (both patches, I can prove I'm
> > right :)
> >
> > (1)
> > http://rtl-sdr.com/wp-content/uploads/2013/04/R820T_datasheet-Non_R-2030_unlocked.pdf
> >
> >> Jiří,
> >>
> >> Do you have any documentation that supports this value change?
> >> Changing this value affects the algorithm, and we'd be happier making
> >> this change if the patch included some better description and perhaps
> >> a reference explaining why the new value is correct.
> >>
> >> Regards,
> >>
> >> Mike Krufky
> >>
> >> On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava  wrote:
> >>>
> >>> Use full range of VCO parameters, fixes tunning for some frequencies.
> >>> ---
> >>>  drivers/media/tuners/r820t.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
> >>> index 1c23666..e25c720 100644
> >>> --- a/drivers/media/tuners/r820t.c
> >>> +++ b/drivers/media/tuners/r820t.c
> >>> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
> >>> enum v4l2_tuner_type type,
> >>> vco_fra = pll_ref * 129 / 128;
> >>> }
> >>>
> >>> -   if (nint > 63) {
> >>> +   if (nint > 76) {
> >>> tuner_info("No valid PLL values for %u kHz!\n", freq);
> >>> return -EINVAL;
> >>> }
> >>> --
> >>> 1.8.3.2
> >>>
> >>>
> >


-- 

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

Re: [PATCH 1/2] [media] r820t: fix nint range check

[Jiří Pinkava]

Mike,

unfortunately no documentation can be referenced except preliminary
version of
datasheet (1).This change is based on lucky guess and supported by lot of
testing on real hardware.

This change add support for devices with Xtal frequency bellow 28.8MHz.

>From Nint  are computed values of Ni and Si. For 28.8MHz crystal can
reach up to 12 / 3 (Ni / Si). Tuner supports crystals with frequencies
(1) 12, 16, 20, 20.48, 24, 27, 28.8, 32 MHz, but this kind of device is
rare to found.
Allowing Ni to go up to 15 instead of only 12 should be safe and for 15
/ 3 (Ni / Si)
we can compute limit for Nint = max(Ni) * 4 + max(Si) + 13 = 76.

If This is sufficient and acceptable explanation I can add some sort of
documentation into patch and resend it (both patches, I can prove I'm
right :)

(1)
http://rtl-sdr.com/wp-content/uploads/2013/04/R820T_datasheet-Non_R-2030_unlocked.pdf

> Jiří,
>
> Do you have any documentation that supports this value change?
> Changing this value affects the algorithm, and we'd be happier making
> this change if the patch included some better description and perhaps
> a reference explaining why the new value is correct.
>
> Regards,
>
> Mike Krufky
>
> On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava  wrote:
>>
>> Use full range of VCO parameters, fixes tunning for some frequencies.
>> ---
>>  drivers/media/tuners/r820t.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
>> index 1c23666..e25c720 100644
>> --- a/drivers/media/tuners/r820t.c
>> +++ b/drivers/media/tuners/r820t.c
>> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
>> enum v4l2_tuner_type type,
>> vco_fra = pll_ref * 129 / 128;
>> }
>>
>> -   if (nint > 63) {
>> +   if (nint > 76) {
>> tuner_info("No valid PLL values for %u kHz!\n", freq);
>> return -EINVAL;
>> }
>> --
>> 1.8.3.2
>>
>>

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

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mark Rutland]

On Fri, Sep 27, 2013 at 02:47:19PM +0100, Mauro Carvalho Chehab wrote:
> Em Fri, 27 Sep 2013 12:34:58 +0100
> Mark Rutland  escreveu:
> 
> > On Fri, Sep 27, 2013 at 10:33:11AM +0100, Srinivas KANDAGATLA wrote:
> > > From: Srinivas Kandagatla 
> > > 
> > > This patch attempts to collate generic bindings which can be used by
> > > the remote control hardwares. Currently the list is not long as there
> > > are only 2 drivers which are device tree'd.
> > > 
> > > Mainly this patch tries to document few bindings used by ST IRB driver
> > > which can be generic as well. This document also add fews common
> > > bindings used by most of the drivers like, interrupts, regs, clocks and
> > > pinctrls.
> > > 
> > > This document can also be holding place to describe generic bindings
> > > used in remote controls devices.
> > > 
> > > Signed-off-by: Srinivas Kandagatla 
> > > ---
> > > Hi All, 
> > > Following Stephen Warren's suggestions at 
> > > https://lkml.org/lkml/2013/9/24/452
> > > this patch is an attempt to document such generic bindings in a common
> > > document.
> > > 
> > > This document currently collates all the generic bindings used with
> > > remote-controls and act as holding place to describe generic bindings for
> > > remote controls.
> > > 
> > > Comments?
> > > 
> > > Thanks,
> > > srini
> > > 
> > >  .../devicetree/bindings/media/remote-control.txt   |   31 
> > > 
> > >  1 files changed, 31 insertions(+), 0 deletions(-)
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/media/remote-control.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/remote-control.txt 
> > > b/Documentation/devicetree/bindings/media/remote-control.txt
> > > new file mode 100644
> > > index 000..901ea56
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/remote-control.txt
> > > @@ -0,0 +1,31 @@
> > > +Generic device tree bindings for remote control.
> > > +
> > > +properties:
> > > + - compatible: Can contain any remote control driver compatible string.
> > > +   example: "st-comms-irb, "gpio-ir-receiver".
> > 
> > This is more generic than remote control, could this not just be left
> > for the specific binding to describe?
> > 
> > > + - reg: Base physical address of the controller and length of memory
> > > +   mapped region.
> > 
> > What if it's on a bus that isn't memory mapped (e.g. i2c, SPI)?
> > 
> > > + - interrupts: Interrupt-specifier for the sole interrupt generated by
> > > +   the device. The interrupt specifier format depends on the
> > > +   interrupt controller parent. Iff the device supports interrupts.
> > 
> > What if it has multiple interrupts, and has interrupts-names?
> > 
> > It might be better to only describe the properties that relate
> > specifically to remote controls, rather than listing all of the generic
> > properties that device tree bidnings may have. That would match the
> > style of the clock bindings.
> > 
> > > + - rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> > > +   the rx pins are wired up.
> > 
> > I'm unsure on this. What if the device has multiple receivers that can
> > be independently configured? 
> 
> Well, if a given remote controller hardware has more than one independent
> receiver (or transmitter), each one should have its own devnode.
> Likely, two entries at DT.

Why? If an IP block happens to have support for N connections, that
doesn't mean that each must be described individually. They likely share
a bank of registers, and depending on the device they might not even be
assigned consistently orgranised windows of that register bank.

> 
> > What if it supports something other than
> > "infrared" or "uhf"? What if a device can only be wired up as
> > "infrared"? 
> 
> I would say that a hardware that has both infrared and uhf has actually
> two different devices. So, it should be mapped as two separate devnodes.

I would say that it is still one device, one which happens to have two
outputs. Just because we want two dev nodes does not mean the dt has to
be structured as two devices.

> 
> > I'm not sure how generic these are, though we should certainly encourage
> > bindings that can be described this way to be described in the same way.
> > 
> > > + - tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> > > +   the tx pins are wired up.
> > 
> > I have similar concerns here to those for the rx-mode property.
> > 
> > > +
> > > +Optional properties:
> > > + - linux,rc-map-name: Linux specific remote control map name. Refer to
> > > +   include/media/rc-map.h for full list of maps.
> > 
> > We shouldn't refer to Linux specifics (i.e. headers) in general in
> > bindings. While it's possible to relax that a bit for linux,*
> > properties, I'd prefer to explicitly list elements in the binding. That
> > prevents the ABI from changing under our feet by someone altering what
> > looks like a completely internal header file.
> 
> Well, the remote controller

Re: [PATCH 1/2] [media] r820t: fix nint range check

[Michael Krufky]

Do you have any comments on this, Mauro?

Assuming that Mauro is OK with this change, (since he is the author of
this driver) then yes - please resubmit the patch with some
explanation within comments inline or within the commit message.

Best regards,

Mike Krufky

On Mon, Sep 30, 2013 at 12:42 PM, Jiří Pinkava  wrote:
> Mike,
>
> unfortunately no documentation can be referenced except preliminary
> version of
> datasheet (1).This change is based on lucky guess and supported by lot of
> testing on real hardware.
>
> This change add support for devices with Xtal frequency bellow 28.8MHz.
>
> From Nint  are computed values of Ni and Si. For 28.8MHz crystal can
> reach up to 12 / 3 (Ni / Si). Tuner supports crystals with frequencies
> (1) 12, 16, 20, 20.48, 24, 27, 28.8, 32 MHz, but this kind of device is
> rare to found.
> Allowing Ni to go up to 15 instead of only 12 should be safe and for 15
> / 3 (Ni / Si)
> we can compute limit for Nint = max(Ni) * 4 + max(Si) + 13 = 76.
>
> If This is sufficient and acceptable explanation I can add some sort of
> documentation into patch and resend it (both patches, I can prove I'm
> right :)
>
> (1)
> http://rtl-sdr.com/wp-content/uploads/2013/04/R820T_datasheet-Non_R-2030_unlocked.pdf
>
>> Jiří,
>>
>> Do you have any documentation that supports this value change?
>> Changing this value affects the algorithm, and we'd be happier making
>> this change if the patch included some better description and perhaps
>> a reference explaining why the new value is correct.
>>
>> Regards,
>>
>> Mike Krufky
>>
>> On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava  wrote:
>>>
>>> Use full range of VCO parameters, fixes tunning for some frequencies.
>>> ---
>>>  drivers/media/tuners/r820t.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
>>> index 1c23666..e25c720 100644
>>> --- a/drivers/media/tuners/r820t.c
>>> +++ b/drivers/media/tuners/r820t.c
>>> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
>>> enum v4l2_tuner_type type,
>>> vco_fra = pll_ref * 129 / 128;
>>> }
>>>
>>> -   if (nint > 63) {
>>> +   if (nint > 76) {
>>> tuner_info("No valid PLL values for %u kHz!\n", freq);
>>> return -EINVAL;
>>> }
>>> --
>>> 1.8.3.2
>>>
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dvb: fix potential format string leak

[Michael Krufky]

On Mon, Sep 16, 2013 at 7:48 PM, Joe Perches  wrote:
> On Mon, 2013-09-16 at 16:37 -0700, Kees Cook wrote:
>> Make sure that a format string cannot accidentally leak into the printk
>> buffer.
> []
>> diff --git a/drivers/media/dvb-frontends/dib9000.c 
>> b/drivers/media/dvb-frontends/dib9000.c
> []
>> @@ -649,7 +649,7 @@ static int dib9000_risc_debug_buf(struct dib9000_state 
>> *state, u16 * data, u8 si
>>   b[2 * (size - 2) - 1] = '\0';   /* Bullet proof the buffer */
>>   if (*b == '~') {
>>   b++;
>> - dprintk(b);
>> + dprintk("%s", b);
>>   } else
>>   dprintk("RISC%d: %d.%04d %s", state->fe_id, ts / 1, ts % 
>> 1, *b ? b : "");
>>   return 1;
>
> This looks odd.
>
> Perhaps this should be:
>
> if (*b == '~')
> b++;
> dprintk("etc...);
>
> It'd be nice to fix the  typo too.

This *does* look odd, I agree.  Meanwhile, I do believe this patch
leaves things better than before.  I'm going to merge Kees' patch for
now, but it would be nice to see a better cleanup for that code block.

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

Re: Subject: [PATCH 2/2] [media] r820t: simplify divisor calculation

[Michael Krufky]

Jiří,

It's difficult to understand a patch like this, let alone merging it,
without any kind of explanation.  At a quick glance, the patch looks
wrong.  If you believe the patch is correct, then please resubmit with
some sort of description and explanation for the change.

Best regards,

Mike Krufky

On Sun, Sep 29, 2013 at 6:46 AM, Jiří Pinkava  wrote:
>
> ---
>  drivers/media/tuners/r820t.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
> index e25c720..36dc63e 100644
> --- a/drivers/media/tuners/r820t.c
> +++ b/drivers/media/tuners/r820t.c
> @@ -596,13 +596,9 @@ static int r820t_set_pll(struct r820t_priv *priv,
> enum v4l2_tuner_type type,
> while (mix_div <= 64) {
> if (((freq * mix_div) >= vco_min) &&
>((freq * mix_div) < vco_max)) {
> -   div_buf = mix_div;
> -   while (div_buf > 2) {
> -   div_buf = div_buf >> 1;
> -   div_num++;
> -   }
> break;
> }
> +   ++div_num;
> mix_div = mix_div << 1;
> }
>
> --
> 1.8.3.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [media] r820t: fix nint range check

[Michael Krufky]

Jiří,

Do you have any documentation that supports this value change?
Changing this value affects the algorithm, and we'd be happier making
this change if the patch included some better description and perhaps
a reference explaining why the new value is correct.

Regards,

Mike Krufky

On Sun, Sep 29, 2013 at 6:45 AM, Jiří Pinkava  wrote:
>
>
> Use full range of VCO parameters, fixes tunning for some frequencies.
> ---
>  drivers/media/tuners/r820t.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c
> index 1c23666..e25c720 100644
> --- a/drivers/media/tuners/r820t.c
> +++ b/drivers/media/tuners/r820t.c
> @@ -637,7 +637,7 @@ static int r820t_set_pll(struct r820t_priv *priv,
> enum v4l2_tuner_type type,
> vco_fra = pll_ref * 129 / 128;
> }
>
> -   if (nint > 63) {
> +   if (nint > 76) {
> tuner_info("No valid PLL values for %u kHz!\n", freq);
> return -EINVAL;
> }
> --
> 1.8.3.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Revert "V4L/DVB: uvc: Enable USB autosuspend by default on uvcvideo"

[Laurent Pinchart]

Hi,

On Tuesday 24 September 2013 08:55:19 VDR User wrote:
> On Tue, Sep 24, 2013 at 4:34 AM, Laurent Pinchart wrote:
> > I've discussed this issue during LPC last week, and I still believe we
> > should enable auto-suspend. The feature really saves power, without it my
> > C910 Logitech webcam gets hot even when unused.
> > 
> > If we disable auto-suspend by default and enable it from userspace only a
> > handful of devices will get auto-suspended. Unless we can get distros to
> > automatically test auto-suspend on unknown webcam models and report the
> > results to update a central data base (which would grow much bigger than a
> > quirks list in the driver in my opinion), disabling auto-suspend would be
> > a serious regression.
> 
> Setting defaults which knowingly cause problems is a horrible idea. Just
> because it works for you and your setup is no justification to force it upon
> everyone. This is certainly a feature that, if wanted, can be enabled by the
> user.

It's not just my setup, auto-suspend works for the vast majority of webcams. 
It has been enabled three years ago, with a report that Fedora had enabled it 
by carrying a kernel patch for a while, without any user complaint.

> I don't see any reasonable argument against letting the user enable it if
> he/she wants it.

USB autosuspend is an important power saving feature. I would be fine with 
enabling it in userspace if we could find a reasonable, cross-distro way to 
create, maintain and distribute the list of devices that support USB 
autosuspend properly.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] media: i2c: adv7343: fix the DT binding properties

[Laurent Pinchart]

Hi Prabhakar,

On Monday 30 September 2013 18:57:01 Prabhakar Lad wrote:
> On Mon, Sep 23, 2013 at 8:18 AM, Prabhakar Lad wrote:
> > On Fri, Sep 20, 2013 at 3:22 PM, Sylwester Nawrocki wrote:
> >> On 09/20/2013 10:11 AM, Prabhakar Lad wrote:
> >>> OK I will, just send out a fix up patch which fixes the mismatch between
> >>> names for the rc-cycle, and later send out a patch which removes the
> >>> platform data usage for next release with proper DT bindings.
> >> 
> >> I think the binding need to be fully corrected now, I just meant to not
> >> touch the board file, i.e. leave non-dt support unchanged.
> > 
> > Ok
> > 
> >>> I'm OK with making regulator properties as optional, But still it would
> >>> change the meaning of what DT is, we know that the VDD/VDD_IO .. etc
> >>> pins are required properties (but still making them as optional) :-(
> >>> 
> >>> I think there might several devices where this situation may arise so
> >>> just thinking of a alternative solution.
> >>> 
> >>> say we have property 'software-regulator' which takes true/false(0/1)
> >>> If set to true we make the regulators as required property or else we
> >>> assume it is handled and ignore it ?
> >> 
> >> I don't think this is a good idea. You would have to add a similar
> >> platform data flag for non-dt, it doesn't sound right. I can see two
> >> options here:
> >> 
> >> 1. Make the regulator properties mandatory and, e.g. define a fixed
> >>voltage GPIO regulator in DT with an empty 'gpio' property. Then
> >>pass a phandle to that regulator in the adv7343 *-supply properties.
> >>For non-dt similarly a fixed voltage regulator(s) and voltage
> >>supplies  would need to be defined in the board files.
> >> 
> >> 2. Make the properties optional and use (devm_)regulator_get_optional()
> >>calls in the driver (a recently added function). I must admit I don't
> >>fully understand description of this function, it currently looks
> >>pretty much same as (devm_)regulator_get(). Thus the driver would
> >>need to be handling regulator supplies only when non ERR_PTR() is
> >>returned from regulator_get_optional() and otherwise assume a non
> >>critical error. There is already quite a few example occurrences of
> >>regulator_get_optional() usage.
> 
> The same question arises in case of the clock, The adv7343 encoder has two
> input clocks CLKIN_A and CLKIN_B. I case of da850 EVM the clock source to
> adv7343 encoder is fixed source which is enabled by default so none of the
> bridge nor the adv7343 driver cares of the clock to enable/disable. So in
> this case should I be registering  (v4l2_clk_register() /
> v4l2_clk_unregister()) a dummy clock in the bridge driver or in the board
> file ?

The fixed clock (which is a real clock, not a dummy clock) should be 
registered in board file, preferably using the common clock framework if 
that's available on your platform.

-- 
Regards,

Laurent Pinchart

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

RE: [PATCH 2/6] si4713 : Modified i2c driver to handle cases where interrupts are not used

[Dinesh Ram]

Hi Hans,

I probably should have done it by now.
I will send you the changes this weekend and you can test those.
I have been quite busy on my thesis lately.

Regards,
Dinesh

From: Hans Verkuil [hverk...@xs4all.nl]
Sent: 30 September 2013 15:07
To: Dinesh Ram
Cc: edubez...@gmail.com; Linux-Media
Subject: Re: [PATCH 2/6] si4713 : Modified i2c driver to handle cases where 
interrupts are not used

On 09/02/2013 12:29 PM, Dinesh Ram wrote:
> Hi Hans and Eduardo,
>
> Sorry for my radio silence. I was infact travelling and didn't have much 
> opportunity to check my mails.
> I will go through the list of comments in the thread and try to fix / justify 
> them in the next few days.
> Hans, probably at the end you might have to test it as I don't have the 
> hardware anymore.
>
> Regards,
> Dinesh

Dinesh,

Do you plan on finalizing this, or should I take over?

Regards,

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

[PATCH] [media] platform: Kconfig: Select SRAM for VIDEO_CODA

[Fabio Estevam]

Running the coda driver without CONFIG_SRAM selected leads to the following 
probe error:

coda 63ff4000.vpu: iram pool not available
coda: probe of 63ff4000.vpu failed with error -12

In order to avoid it, select CONFIG_SRAM inside VIDEO_CODA.

Signed-off-by: Fabio Estevam 
---
 drivers/media/platform/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index c7caf94..8b2467d 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -143,6 +143,7 @@ if V4L_MEM2MEM_DRIVERS
 config VIDEO_CODA
tristate "Chips&Media Coda multi-standard codec IP"
depends on VIDEO_DEV && VIDEO_V4L2 && ARCH_MXC
+   select SRAM
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
---help---
-- 
1.8.1.2


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

[PATCH v2 02/10] [media] coda: only set buffered input queue for decoder

[Philipp Zabel]

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 2805538..945c539 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -1928,8 +1928,9 @@ static int coda_start_streaming(struct vb2_queue *q, 
unsigned int count)
if (!(ctx->streamon_out & ctx->streamon_cap))
return 0;
 
-   /* Allow device_run with no buffers queued and after streamoff */
-   v4l2_m2m_set_src_buffered(ctx->m2m_ctx, true);
+   /* Allow decoder device_run with no new buffers queued */
+   if (ctx->inst_type == CODA_INST_DECODER)
+   v4l2_m2m_set_src_buffered(ctx->m2m_ctx, true);
 
ctx->gopcounter = ctx->params.gop_size - 1;
buf = v4l2_m2m_next_dst_buf(ctx->m2m_ctx);
-- 
1.8.4.rc3

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

[PATCH v2 04/10] [media] coda: fix FMO value setting for CodaDx6

[Philipp Zabel]

The register is only written on CodaDx6, so the temporary variable
to be written only needs to be initialized on CodaDx6. Also, drop
two no-op lines.

Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Removed no-op lines leaving FMOPARAM fields of CMD_ENC_SEQ_FMO
   at zero values on CodaDx6.
---
 drivers/media/platform/coda.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 53539c1..5fe947c 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -2074,10 +2074,8 @@ static int coda_start_streaming(struct vb2_queue *q, 
unsigned int count)
coda_setup_iram(ctx);
 
if (dst_fourcc == V4L2_PIX_FMT_H264) {
-   value  = (FMO_SLICE_SAVE_BUF_SIZE << 7);
-   value |= (0 & CODA_FMOPARAM_TYPE_MASK) << 
CODA_FMOPARAM_TYPE_OFFSET;
-   value |=  0 & CODA_FMOPARAM_SLICENUM_MASK;
if (dev->devtype->product == CODA_DX6) {
+   value = FMO_SLICE_SAVE_BUF_SIZE << 7;
coda_write(dev, value, CODADX6_CMD_ENC_SEQ_FMO);
} else {
coda_write(dev, ctx->iram_info.search_ram_paddr,
-- 
1.8.4.rc3

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

[PATCH v2 10/10] [media] coda: v4l2-compliance fix: zero pixel format priv field

[Philipp Zabel]

If unused, the pixel format priv field has to be cleared by the driver
in try_fmt.

Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Clear pixel format priv in try_fmt only
---
 drivers/media/platform/coda.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 2bf8175..0bd5211 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -613,6 +613,8 @@ static int coda_try_fmt(struct coda_ctx *ctx, struct 
coda_codec *codec,
BUG();
}
 
+   f->fmt.pix.priv = 0;
+
return 0;
 }
 
-- 
1.8.4.rc3

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

[PATCH v2 0/10] CODA driver fixes

[Philipp Zabel]

This series contains a few fixes for the CODA driver to allow more than
four simultaneous instances on i.MX53, and to make v4l2-compliance a
bit happier.

Changes since v1:
 - Removed no-op lines leaving FMOPARAM fields of CMD_ENC_SEQ_FMO
   at zero values on CodaDx6.
 - Use coda_ instead of coda_vidioc_ prefix for v4l2_ioctl_ops
 - Clear pixel format priv in try_fmt only

regards
Philipp

---
 drivers/media/platform/coda.c | 278 ++
 1 file changed, 172 insertions(+), 106 deletions(-)

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

[PATCH v2 05/10] [media] coda: move coda_product_name above vidioc_querycap

[Philipp Zabel]

Use the product name (currently CodaDx6 or CODA7541)
to fill the v4l2_capabilities.name field.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 5fe947c..e70a272 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -394,14 +394,32 @@ static struct coda_codec *coda_find_codec(struct coda_dev 
*dev, int src_fourcc,
return &codecs[k];
 }
 
+static char *coda_product_name(int product)
+{
+   static char buf[9];
+
+   switch (product) {
+   case CODA_DX6:
+   return "CodaDx6";
+   case CODA_7541:
+   return "CODA7541";
+   default:
+   snprintf(buf, sizeof(buf), "(0x%04x)", product);
+   return buf;
+   }
+}
+
 /*
  * V4L2 ioctl() operations.
  */
 static int vidioc_querycap(struct file *file, void *priv,
   struct v4l2_capability *cap)
 {
+   struct coda_ctx *ctx = fh_to_ctx(priv);
+
strlcpy(cap->driver, CODA_NAME, sizeof(cap->driver));
-   strlcpy(cap->card, CODA_NAME, sizeof(cap->card));
+   strlcpy(cap->card, coda_product_name(ctx->dev->devtype->product),
+   sizeof(cap->card));
strlcpy(cap->bus_info, "platform:" CODA_NAME, sizeof(cap->bus_info));
/*
 * This is only a mem-to-mem video device. The capture and output
@@ -2868,21 +2886,6 @@ static bool coda_firmware_supported(u32 vernum)
return false;
 }
 
-static char *coda_product_name(int product)
-{
-   static char buf[9];
-
-   switch (product) {
-   case CODA_DX6:
-   return "CodaDx6";
-   case CODA_7541:
-   return "CODA7541";
-   default:
-   snprintf(buf, sizeof(buf), "(0x%04x)", product);
-   return buf;
-   }
-}
-
 static int coda_hw_init(struct coda_dev *dev)
 {
u16 product, major, minor, release;
-- 
1.8.4.rc3

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

[PATCH v2 08/10] [media] coda: v4l2-compliance fix: overwrite invalid pixel formats with the current setting

[Philipp Zabel]

This patch fixes the v4l2-compliance "TRY_FMT(G_FMT) != G_FMT" issue.

The driver now overwrites invalid formats with the current setting, using
coda_get_max_dimensions to find device specific max width/height.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 72 +--
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 6286133..1572929 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -54,8 +54,6 @@
 
 #define CODA_MAX_FRAMEBUFFERS  8
 
-#define MAX_W  8192
-#define MAX_H  8192
 #define CODA_MAX_FRAME_SIZE0x10
 #define FMO_SLICE_SAVE_BUF_SIZE (32)
 #define CODA_DEFAULT_GAMMA 4096
@@ -394,6 +392,31 @@ static struct coda_codec *coda_find_codec(struct coda_dev 
*dev, int src_fourcc,
return &codecs[k];
 }
 
+static void coda_get_max_dimensions(struct coda_dev *dev,
+   struct coda_codec *codec,
+   int *max_w, int *max_h)
+{
+   struct coda_codec *codecs = dev->devtype->codecs;
+   int num_codecs = dev->devtype->num_codecs;
+   unsigned int w, h;
+   int k;
+
+   if (codec) {
+   w = codec->max_w;
+   h = codec->max_h;
+   } else {
+   for (k = 0, w = 0, h = 0; k < num_codecs; k++) {
+   w = max(w, codecs[k].max_w);
+   h = max(h, codecs[k].max_h);
+   }
+   }
+
+   if (max_w)
+   *max_w = w;
+   if (max_h)
+   *max_h = h;
+}
+
 static char *coda_product_name(int product)
 {
static char buf[9];
@@ -537,8 +560,11 @@ static int coda_g_fmt(struct file *file, void *priv,
return 0;
 }
 
-static int coda_try_fmt(struct coda_codec *codec, struct v4l2_format *f)
+static int coda_try_fmt(struct coda_ctx *ctx, struct coda_codec *codec,
+   struct v4l2_format *f)
 {
+   struct coda_dev *dev = ctx->dev;
+   struct coda_q_data *q_data;
unsigned int max_w, max_h;
enum v4l2_field field;
 
@@ -552,25 +578,39 @@ static int coda_try_fmt(struct coda_codec *codec, struct 
v4l2_format *f)
 * if any of the dimensions is unsupported */
f->fmt.pix.field = field;
 
-   if (codec) {
-   max_w = codec->max_w;
-   max_h = codec->max_h;
-   } else {
-   max_w = MAX_W;
-   max_h = MAX_H;
+   coda_get_max_dimensions(dev, codec, &max_w, &max_h);
+   v4l_bound_align_image(&f->fmt.pix.width, MIN_W, max_w, W_ALIGN,
+ &f->fmt.pix.height, MIN_H, max_h, H_ALIGN,
+ S_ALIGN);
+
+   switch (f->fmt.pix.pixelformat) {
+   case V4L2_PIX_FMT_YUV420:
+   case V4L2_PIX_FMT_YVU420:
+   case V4L2_PIX_FMT_H264:
+   case V4L2_PIX_FMT_MPEG4:
+   case V4L2_PIX_FMT_JPEG:
+   break;
+   default:
+   q_data = get_q_data(ctx, f->type);
+   f->fmt.pix.pixelformat = q_data->fourcc;
}
-   v4l_bound_align_image(&f->fmt.pix.width, MIN_W, max_w,
- W_ALIGN, &f->fmt.pix.height,
- MIN_H, max_h, H_ALIGN, S_ALIGN);
 
-   if (coda_format_is_yuv(f->fmt.pix.pixelformat)) {
+   switch (f->fmt.pix.pixelformat) {
+   case V4L2_PIX_FMT_YUV420:
+   case V4L2_PIX_FMT_YVU420:
/* Frame stride must be multiple of 8 */
f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 8);
f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
f->fmt.pix.height * 3 / 2;
-   } else { /*encoded formats h.264/mpeg4 */
+   break;
+   case V4L2_PIX_FMT_H264:
+   case V4L2_PIX_FMT_MPEG4:
+   case V4L2_PIX_FMT_JPEG:
f->fmt.pix.bytesperline = 0;
f->fmt.pix.sizeimage = CODA_MAX_FRAME_SIZE;
+   break;
+   default:
+   BUG();
}
 
return 0;
@@ -605,7 +645,7 @@ static int coda_try_fmt_vid_cap(struct file *file, void 
*priv,
 
f->fmt.pix.colorspace = ctx->colorspace;
 
-   ret = coda_try_fmt(codec, f);
+   ret = coda_try_fmt(ctx, codec, f);
if (ret < 0)
return ret;
 
@@ -634,7 +674,7 @@ static int coda_try_fmt_vid_out(struct file *file, void 
*priv,
if (!f->fmt.pix.colorspace)
f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
 
-   return coda_try_fmt(codec, f);
+   return coda_try_fmt(ctx, codec, f);
 }
 
 static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
-- 
1.8.4.rc3

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

[PATCH v2 07/10] [media] coda: prefix v4l2_ioctl_ops with coda_ instead of vidioc_

[Philipp Zabel]

Moving the ioctl handler callbacks into the coda namespace helps
tremendously to make sense of backtraces.

Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Use coda_ instead of coda_vidioc_ prefix
---
 drivers/media/platform/coda.c | 123 +-
 1 file changed, 63 insertions(+), 60 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 82049f8..6286133 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -412,8 +412,8 @@ static char *coda_product_name(int product)
 /*
  * V4L2 ioctl() operations.
  */
-static int vidioc_querycap(struct file *file, void *priv,
-  struct v4l2_capability *cap)
+static int coda_querycap(struct file *file, void *priv,
+struct v4l2_capability *cap)
 {
struct coda_ctx *ctx = fh_to_ctx(priv);
 
@@ -484,8 +484,8 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
return -EINVAL;
 }
 
-static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
-  struct v4l2_fmtdesc *f)
+static int coda_enum_fmt_vid_cap(struct file *file, void *priv,
+struct v4l2_fmtdesc *f)
 {
struct coda_ctx *ctx = fh_to_ctx(priv);
struct vb2_queue *src_vq;
@@ -503,13 +503,14 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, 
void *priv,
return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0);
 }
 
-static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
-  struct v4l2_fmtdesc *f)
+static int coda_enum_fmt_vid_out(struct file *file, void *priv,
+struct v4l2_fmtdesc *f)
 {
return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_OUTPUT, 0);
 }
 
-static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
+static int coda_g_fmt(struct file *file, void *priv,
+ struct v4l2_format *f)
 {
struct vb2_queue *vq;
struct coda_q_data *q_data;
@@ -536,7 +537,7 @@ static int vidioc_g_fmt(struct file *file, void *priv, 
struct v4l2_format *f)
return 0;
 }
 
-static int vidioc_try_fmt(struct coda_codec *codec, struct v4l2_format *f)
+static int coda_try_fmt(struct coda_codec *codec, struct v4l2_format *f)
 {
unsigned int max_w, max_h;
enum v4l2_field field;
@@ -575,8 +576,8 @@ static int vidioc_try_fmt(struct coda_codec *codec, struct 
v4l2_format *f)
return 0;
 }
 
-static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
- struct v4l2_format *f)
+static int coda_try_fmt_vid_cap(struct file *file, void *priv,
+   struct v4l2_format *f)
 {
struct coda_ctx *ctx = fh_to_ctx(priv);
struct coda_codec *codec;
@@ -604,7 +605,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void 
*priv,
 
f->fmt.pix.colorspace = ctx->colorspace;
 
-   ret = vidioc_try_fmt(codec, f);
+   ret = coda_try_fmt(codec, f);
if (ret < 0)
return ret;
 
@@ -620,8 +621,8 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void 
*priv,
return 0;
 }
 
-static int vidioc_try_fmt_vid_out(struct file *file, void *priv,
- struct v4l2_format *f)
+static int coda_try_fmt_vid_out(struct file *file, void *priv,
+   struct v4l2_format *f)
 {
struct coda_ctx *ctx = fh_to_ctx(priv);
struct coda_codec *codec;
@@ -633,10 +634,10 @@ static int vidioc_try_fmt_vid_out(struct file *file, void 
*priv,
if (!f->fmt.pix.colorspace)
f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
 
-   return vidioc_try_fmt(codec, f);
+   return coda_try_fmt(codec, f);
 }
 
-static int vidioc_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
+static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
 {
struct coda_q_data *q_data;
struct vb2_queue *vq;
@@ -666,61 +667,62 @@ static int vidioc_s_fmt(struct coda_ctx *ctx, struct 
v4l2_format *f)
return 0;
 }
 
-static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
-   struct v4l2_format *f)
+static int coda_s_fmt_vid_cap(struct file *file, void *priv,
+ struct v4l2_format *f)
 {
struct coda_ctx *ctx = fh_to_ctx(priv);
int ret;
 
-   ret = vidioc_try_fmt_vid_cap(file, priv, f);
+   ret = coda_try_fmt_vid_cap(file, priv, f);
if (ret)
return ret;
 
-   return vidioc_s_fmt(ctx, f);
+   return coda_s_fmt(ctx, f);
 }
 
-static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
-   struct v4l2_format *f)
+static int coda_s_fmt_vid_out(struct file *file, void *priv,
+ struct v4l2_format *f)
 {
struct coda_ctx *ctx = fh_to_ctx(priv);
int ret;
 
-   ret

[PATCH v2 09/10] [media] coda: v4l2-compliance fix: implement try_decoder_cmd

[Philipp Zabel]

Implement try_decoder_cmd to let userspace determine available commands
and flags.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 1572929..2bf8175 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -835,23 +835,34 @@ static int coda_streamoff(struct file *file, void *priv,
return ret;
 }
 
-static int coda_decoder_cmd(struct file *file, void *fh,
-  struct v4l2_decoder_cmd *dc)
+static int coda_try_decoder_cmd(struct file *file, void *fh,
+   struct v4l2_decoder_cmd *dc)
 {
-   struct coda_ctx *ctx = fh_to_ctx(fh);
-
if (dc->cmd != V4L2_DEC_CMD_STOP)
return -EINVAL;
 
-   if ((dc->flags & V4L2_DEC_CMD_STOP_TO_BLACK) ||
-   (dc->flags & V4L2_DEC_CMD_STOP_IMMEDIATELY))
+   if (dc->flags & V4L2_DEC_CMD_STOP_TO_BLACK)
return -EINVAL;
 
-   if (dc->stop.pts != 0)
+   if (!(dc->flags & V4L2_DEC_CMD_STOP_IMMEDIATELY) && (dc->stop.pts != 0))
return -EINVAL;
 
+   return 0;
+}
+
+static int coda_decoder_cmd(struct file *file, void *fh,
+   struct v4l2_decoder_cmd *dc)
+{
+   struct coda_ctx *ctx = fh_to_ctx(fh);
+   int ret;
+
+   ret = coda_try_decoder_cmd(file, fh, dc);
+   if (ret < 0)
+   return ret;
+
+   /* Ignore decoder stop command silently in encoder context */
if (ctx->inst_type != CODA_INST_DECODER)
-   return -EINVAL;
+   return 0;
 
/* Set the strem-end flag on this context */
ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
@@ -894,6 +905,7 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
.vidioc_streamon= coda_streamon,
.vidioc_streamoff   = coda_streamoff,
 
+   .vidioc_try_decoder_cmd = coda_try_decoder_cmd,
.vidioc_decoder_cmd = coda_decoder_cmd,
 
.vidioc_subscribe_event = coda_subscribe_event,
-- 
1.8.4.rc3

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

[PATCH v2 03/10] [media] coda: add compressed flag to format enumeration output

[Philipp Zabel]

Correctly flag compressed formats in the ENUM_FMT ioctl output.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 945c539..53539c1 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -457,6 +457,8 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
fmt = &formats[i];
strlcpy(f->description, fmt->name, sizeof(f->description));
f->pixelformat = fmt->fourcc;
+   if (!coda_format_is_yuv(fmt->fourcc))
+   f->flags |= V4L2_FMT_FLAG_COMPRESSED;
return 0;
}
 
-- 
1.8.4.rc3

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

[PATCH v2 06/10] [media] coda: use picture type returned from hardware

[Philipp Zabel]

Instead of copying v4l2_buf.flags from the source buffer, set
the destination buffer flags as reported by the hardware codec.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index e70a272..82049f8 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -2744,7 +2744,6 @@ static void coda_finish_encode(struct coda_ctx *ctx)
dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
 
/* Get results from the coda */
-   coda_read(dev, CODA_RET_ENC_PIC_TYPE);
start_ptr = coda_read(dev, CODA_CMD_ENC_PIC_BB_START);
wr_ptr = coda_read(dev, CODA_REG_BIT_WR_PTR(ctx->reg_idx));
 
@@ -2764,7 +2763,7 @@ static void coda_finish_encode(struct coda_ctx *ctx)
coda_read(dev, CODA_RET_ENC_PIC_SLICE_NUM);
coda_read(dev, CODA_RET_ENC_PIC_FLAG);
 
-   if (src_buf->v4l2_buf.flags & V4L2_BUF_FLAG_KEYFRAME) {
+   if (coda_read(dev, CODA_RET_ENC_PIC_TYPE) == 0) {
dst_buf->v4l2_buf.flags |= V4L2_BUF_FLAG_KEYFRAME;
dst_buf->v4l2_buf.flags &= ~V4L2_BUF_FLAG_PFRAME;
} else {
-- 
1.8.4.rc3

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

[PATCH v2 01/10] [media] coda: allow more than four instances on CODA7541

[Philipp Zabel]

With the new firmware, there are not anymore four register sets,
but a single register set, which the driver has to conserve across
context switches. This allows to handle more than four instances
at the same time.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 449d2fe..2805538 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -39,7 +39,7 @@
 
 #define CODA_NAME  "coda"
 
-#define CODA_MAX_INSTANCES 4
+#define CODADX6_MAX_INSTANCES  4
 
 #define CODA_FMO_BUF_SIZE  32
 #define CODADX6_WORK_BUF_SIZE  (288 * 1024 + CODA_FMO_BUF_SIZE * 8 * 1024)
@@ -2371,7 +2371,13 @@ static int coda_queue_init(void *priv, struct vb2_queue 
*src_vq,
 
 static int coda_next_free_instance(struct coda_dev *dev)
 {
-   return ffz(dev->instance_mask);
+   int idx = ffz(dev->instance_mask);
+
+   if ((idx < 0) ||
+   (dev->devtype->product == CODA_DX6 && idx > CODADX6_MAX_INSTANCES))
+   return -EBUSY;
+
+   return idx;
 }
 
 static int coda_open(struct file *file)
@@ -2386,8 +2392,8 @@ static int coda_open(struct file *file)
return -ENOMEM;
 
idx = coda_next_free_instance(dev);
-   if (idx >= CODA_MAX_INSTANCES) {
-   ret = -EBUSY;
+   if (idx < 0) {
+   ret = idx;
goto err_coda_max;
}
set_bit(idx, &dev->instance_mask);
-- 
1.8.4.rc3

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

Re: [PATCH] media: i2c: adv7343: fix the DT binding properties

[Prabhakar Lad]

Hi All,

On Mon, Sep 23, 2013 at 8:18 AM, Prabhakar Lad
 wrote:
> Hi Sylwester,
>
> On Fri, Sep 20, 2013 at 3:22 PM, Sylwester Nawrocki
>  wrote:
>> Hi Prabhakar,
>>
>> On 09/20/2013 10:11 AM, Prabhakar Lad wrote:
>>> OK I will, just send out a fix up patch which fixes the mismatch between
>>> names for the rc-cycle, and later send out a patch which removes the
>>> platform data usage for next release with proper DT bindings.
>>
>> I think the binding need to be fully corrected now, I just meant to not
>> touch the board file, i.e. leave non-dt support unchanged.
>>
> Ok
>
>>> I'm OK with making regulator properties as optional, But still it would
>>> change the meaning of what DT is, we know that the VDD/VDD_IO .. etc
>>> pins are required properties (but still making them as optional) :-(
>>>
>>> I think there might several devices where this situation may arise so
>>> just thinking of a alternative solution.
>>>
>>> say we have property 'software-regulator' which takes true/false(0/1)
>>> If set to true we make the regulators as required property or else we
>>> assume it is handled and ignore it ?
>>
>> I don't think this is a good idea. You would have to add a similar platform
>> data flag for non-dt, it doesn't sound right. I can see two options here:
>>
>> 1. Make the regulator properties mandatory and, e.g. define a fixed
>>voltage GPIO regulator in DT with an empty 'gpio' property. Then
>>pass a phandle to that regulator in the adv7343 *-supply properties.
>>For non-dt similarly a fixed voltage regulator(s) and voltage
>>supplies  would need to be defined in the board files.
>>
>> 2. Make the properties optional and use (devm_)regulator_get_optional()
>>calls in the driver (a recently added function). I must admit I don't
>>fully understand description of this function, it currently looks
>>pretty much same as (devm_)regulator_get(). Thus the driver would
>>need to be handling regulator supplies only when non ERR_PTR() is
>>returned from regulator_get_optional() and otherwise assume a non
>>critical error. There is already quite a few example occurrences of
>>regulator_get_optional() usage.
>>
The same question arises in case of the clock, The adv7343 encoder has
two input clocks CLKIN_A and CLKIN_B. I case of da850 EVM the
clock source to adv7343 encoder is fixed source which is enabled
by default so none of the bridge nor the adv7343 driver cares of the
clock to enable/disable.
So in this case should I be registering  (v4l2_clk_register() /
v4l2_clk_unregister())
a dummy clock in the bridge driver or in the board file ?

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

Re: [PATCH 07/10] [media] coda: prefix v4l2_ioctl_ops with coda_

[Philipp Zabel]

Am Montag, den 30.09.2013, 13:50 +0200 schrieb Hans Verkuil:
> On 09/19/2013 11:13 AM, Philipp Zabel wrote:
> > Moving the ioctl handler callbacks into the coda namespace helps
> > tremendously to make sense of backtraces.
> 
> I like the idea, but I would just use the coda_ prefix, not coda_vidioc_. In 
> general
> the prefix is either vidioc_ or the name of the driver, not both.

Thank you, I'll change this patch (and the following patches)
accordingly.

regards
Philipp

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

Re: [PATCH 04/10] [media] coda: fix FMO value setting for CodaDx6

[Philipp Zabel]

Am Montag, den 30.09.2013, 13:48 +0200 schrieb Hans Verkuil:
> On 09/19/2013 11:13 AM, Philipp Zabel wrote:
> > The register is only written on CodaDx6, so the temporary variable
> > to be written only needs to be initialized on CodaDx6.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >  drivers/media/platform/coda.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> > index 53539c1..e8acff3 100644
> > --- a/drivers/media/platform/coda.c
> > +++ b/drivers/media/platform/coda.c
> > @@ -2074,10 +2074,10 @@ static int coda_start_streaming(struct vb2_queue 
> > *q, unsigned int count)
> > coda_setup_iram(ctx);
> >  
> > if (dst_fourcc == V4L2_PIX_FMT_H264) {
> > -   value  = (FMO_SLICE_SAVE_BUF_SIZE << 7);
> > -   value |= (0 & CODA_FMOPARAM_TYPE_MASK) << 
> > CODA_FMOPARAM_TYPE_OFFSET;
> > -   value |=  0 & CODA_FMOPARAM_SLICENUM_MASK;
> > if (dev->devtype->product == CODA_DX6) {
> > +   value  = (FMO_SLICE_SAVE_BUF_SIZE << 7);
> > +   value |= (0 & CODA_FMOPARAM_TYPE_MASK) << 
> > CODA_FMOPARAM_TYPE_OFFSET;
> > +   value |=  0 & CODA_FMOPARAM_SLICENUM_MASK;
> 
> 0 & CODA_FMOPARAM_SLICENUM_MASK?
> 
> These last two lines evaluate to a nop, so that looks very weird. Is this a 
> bug?

I assume Javier added those for documentation purposes. The newer CODA
cores don't have the FMO configuration anymore. I'll remove the no-op
lines for now if he doesn't mind.

regards
Philipp

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

Re: [PATCH 2/6] si4713 : Modified i2c driver to handle cases where interrupts are not used

[Hans Verkuil]

On 09/02/2013 12:29 PM, Dinesh Ram wrote:
> Hi Hans and Eduardo,
> 
> Sorry for my radio silence. I was infact travelling and didn't have much 
> opportunity to check my mails.
> I will go through the list of comments in the thread and try to fix / justify 
> them in the next few days. 
> Hans, probably at the end you might have to test it as I don't have the 
> hardware anymore.
> 
> Regards,
> Dinesh

Dinesh,

Do you plan on finalizing this, or should I take over?

Regards,

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

Re: [PATCH 3/8] [media] core: Don't use i2c_client->driver

[Hans Verkuil]

On 09/29/2013 10:51 AM, Lars-Peter Clausen wrote:
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. The results of the expressions 'client->driver.driver->field' and
> 'client->dev.driver->field' are identical, so replace all occurrences of the
> former with the later.
> 
> Signed-off-by: Lars-Peter Clausen 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/v4l2-core/tuner-core.c  |  6 +++---
>  drivers/media/v4l2-core/v4l2-common.c | 10 +-
>  include/media/v4l2-common.h   |  2 +-
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/tuner-core.c 
> b/drivers/media/v4l2-core/tuner-core.c
> index ddc9379..4133af0 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -43,7 +43,7 @@
>  
>  #define UNSET (-1U)
>  
> -#define PREFIX (t->i2c->driver->driver.name)
> +#define PREFIX (t->i2c->dev.driver->name)
>  
>  /*
>   * Driver modprobe parameters
> @@ -452,7 +452,7 @@ static void set_type(struct i2c_client *c, unsigned int 
> type,
>   }
>  
>   tuner_dbg("%s %s I2C addr 0x%02x with type %d used for 0x%02x\n",
> -   c->adapter->name, c->driver->driver.name, c->addr << 1, type,
> +   c->adapter->name, c->dev.driver->name, c->addr << 1, type,
> t->mode_mask);
>   return;
>  
> @@ -556,7 +556,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
>   int mode_mask;
>  
>   if (pos->i2c->adapter != adap ||
> - strcmp(pos->i2c->driver->driver.name, "tuner"))
> + strcmp(pos->i2c->dev.driver->name, "tuner"))
>   continue;
>  
>   mode_mask = pos->mode_mask;
> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
> b/drivers/media/v4l2-core/v4l2-common.c
> index 037d7a5..433d6d7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -236,14 +236,14 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, 
> struct i2c_client *client,
>   v4l2_subdev_init(sd, ops);
>   sd->flags |= V4L2_SUBDEV_FL_IS_I2C;
>   /* the owner is the same as the i2c_client's driver owner */
> - sd->owner = client->driver->driver.owner;
> + sd->owner = client->dev.driver->owner;
>   sd->dev = &client->dev;
>   /* i2c_client and v4l2_subdev point to one another */
>   v4l2_set_subdevdata(sd, client);
>   i2c_set_clientdata(client, sd);
>   /* initialize name */
>   snprintf(sd->name, sizeof(sd->name), "%s %d-%04x",
> - client->driver->driver.name, i2c_adapter_id(client->adapter),
> + client->dev.driver->name, i2c_adapter_id(client->adapter),
>   client->addr);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
> @@ -274,11 +274,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct 
> v4l2_device *v4l2_dev,
>  loaded. This delay-load mechanism doesn't work if other drivers
>  want to use the i2c device, so explicitly loading the module
>  is the best alternative. */
> - if (client == NULL || client->driver == NULL)
> + if (client == NULL || client->dev.driver == NULL)
>   goto error;
>  
>   /* Lock the module so we can safely get the v4l2_subdev pointer */
> - if (!try_module_get(client->driver->driver.owner))
> + if (!try_module_get(client->dev.driver->owner))
>   goto error;
>   sd = i2c_get_clientdata(client);
>  
> @@ -287,7 +287,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct 
> v4l2_device *v4l2_dev,
>   if (v4l2_device_register_subdev(v4l2_dev, sd))
>   sd = NULL;
>   /* Decrease the module use count to match the first try_module_get. */
> - module_put(client->driver->driver.owner);
> + module_put(client->dev.driver->owner);
>  
>  error:
>   /* If we have a client but no subdev, then something went wrong and
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 16550c4..a707529 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -35,7 +35,7 @@
>   printk(level "%s %d-%04x: " fmt, name, i2c_adapter_id(adapter), addr , 
> ## arg)
>  
>  #define v4l_client_printk(level, client, fmt, arg...)
> \
> - v4l_printk(level, (client)->driver->driver.name, (client)->adapter, \
> + v4l_printk(level, (client)->dev.driver->name, (client)->adapter, \
>  (client)->addr, fmt , ## arg)
>  
>  #define v4l_err(client, fmt, arg...) \
> 

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

Re: [PATCH v9 09/13] [media] exynos5-fimc-is: Add the hardware pipeline control

[Hans Verkuil]

On 09/27/2013 12:59 PM, Arun Kumar K wrote:
> This patch adds the crucial hardware pipeline control for the
> fimc-is driver. All the subdev nodes will call this pipeline
> interfaces to reach the hardware. Responsibilities of this module
> involves configuring and maintaining the hardware pipeline involving
> multiple sub-ips like ISP, DRC, Scalers, ODC, 3DNR, FD etc.
> 
> Signed-off-by: Arun Kumar K 
> Signed-off-by: Kilyeon Im 
> Reviewed-by: Sylwester Nawrocki 
> ---
>  .../media/platform/exynos5-is/fimc-is-pipeline.c   | 1708 
> 
>  .../media/platform/exynos5-is/fimc-is-pipeline.h   |  129 ++
>  2 files changed, 1837 insertions(+)
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-pipeline.c
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-pipeline.h
> 
> diff --git a/drivers/media/platform/exynos5-is/fimc-is-pipeline.c 
> b/drivers/media/platform/exynos5-is/fimc-is-pipeline.c
> new file mode 100644
> index 000..a73d952
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/fimc-is-pipeline.c



> +int fimc_is_pipeline_open(struct fimc_is_pipeline *pipeline,
> + struct fimc_is_sensor *sensor)
> +{
> + struct fimc_is *is = pipeline->is;
> + struct is_region *region;
> + unsigned long index[2] = {0};
> + int ret;
> +
> + if (!sensor)
> + return -EINVAL;
> +
> + mutex_lock(&pipeline->pipe_lock);
> +
> + if (test_bit(PIPELINE_OPEN, &pipeline->state)) {
> + dev_err(pipeline->dev, "Pipeline already open\n");
> + ret = -EINVAL;
> + goto err_exit;
> + }
> +
> + pipeline->fcount = 0;
> + pipeline->sensor = sensor;
> +
> + if (is->num_pipelines == 0) {
> + /* Init memory */
> + ret = fimc_is_pipeline_initmem(pipeline);
> + if (ret) {
> + dev_err(pipeline->dev, "Pipeline memory init failed\n");
> + goto err_exit;
> + }
> +
> + /* Load firmware */
> + ret = fimc_is_pipeline_load_firmware(pipeline);
> + if (ret) {
> + dev_err(pipeline->dev, "Firmware load failed\n");
> + goto err_fw;
> + }
> +
> + /* Power ON */
> + ret = fimc_is_pipeline_power(pipeline, 1);
> + if (ret) {
> + dev_err(pipeline->dev, "A5 power on failed\n");
> + goto err_fw;
> + }
> +
> + /* Wait for FW Init to complete */
> + ret = fimc_is_itf_wait_init_state(&is->interface);
> + if (ret) {
> + dev_err(pipeline->dev, "FW init failed\n");
> + goto err_fw;
> + }
> + }
> +
> + /* Open Sensor */
> + region = pipeline->is_region;
> + ret = fimc_is_itf_open_sensor(&is->interface,
> + pipeline->instance,
> + sensor->drvdata->id,
> + sensor->i2c_bus,
> + pipeline->minfo->shared.paddr);
> + if (ret) {
> + dev_err(pipeline->dev, "Open sensor failed\n");
> + goto err_exit;
> + }
> +
> + /* Load setfile */
> + ret = fimc_is_pipeline_setfile(pipeline);
> + if (ret)
> + goto err_exit;
> +
> + /* Stream off */
> + ret = fimc_is_itf_stream_off(&is->interface, pipeline->instance);
> + if (ret)
> + goto err_exit;
> +
> + /* Process off */
> + ret = fimc_is_itf_process_off(&is->interface, pipeline->instance);
> + if (ret)
> + goto err_exit;
> +
> + if (is->num_pipelines == 0) {
> + /* Copy init params to FW region */
> + memset(®ion->parameter, 0x0, sizeof(struct is_param_region));
> +
> + memcpy(®ion->parameter.sensor, &init_sensor_param,
> + sizeof(struct sensor_param));

How about:

region->parameter.sensor = init_sensor_param;

Shorter and type-safe.

Ditto for the memcpy's below.

> + memcpy(®ion->parameter.isp, &init_isp_param,
> + sizeof(struct isp_param));
> + memcpy(®ion->parameter.drc, &init_drc_param,
> + sizeof(struct drc_param));
> + memcpy(®ion->parameter.scalerc, &init_scalerc_param,
> + sizeof(struct scalerc_param));
> + memcpy(®ion->parameter.odc, &init_odc_param,
> + sizeof(struct odc_param));
> + memcpy(®ion->parameter.dis, &init_dis_param,
> + sizeof(struct dis_param));
> + memcpy(®ion->parameter.tdnr, &init_tdnr_param,
> + sizeof(struct tdnr_param));
> + memcpy(®ion->parameter.scalerp, &init_scalerp_param,
> + sizeof(struct scalerp_param));
> + memcpy(®ion->parameter.fd, 

Re: [PATCH] [media] stk1135: fix two warnings added by changeset 76e0598

[Hans Verkuil]

On 09/26/2013 12:42 PM, Mauro Carvalho Chehab wrote:
> drivers/media/usb/gspca/stk1135.c:615:6: warning: no previous prototype for 
> 'stk1135_try_fmt' [-Wmissing-prototypes]
>  void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format *fmt)
>   ^
> drivers/media/usb/gspca/stk1135.c:627:5: warning: no previous prototype for 
> 'stk1135_enum_framesizes' [-Wmissing-prototypes]
>  int stk1135_enum_framesizes(struct gspca_dev *gspca_dev,
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/usb/gspca/stk1135.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/gspca/stk1135.c 
> b/drivers/media/usb/gspca/stk1135.c
> index 8add2f7..1fc80af 100644
> --- a/drivers/media/usb/gspca/stk1135.c
> +++ b/drivers/media/usb/gspca/stk1135.c
> @@ -612,7 +612,7 @@ static int sd_init_controls(struct gspca_dev *gspca_dev)
>   return 0;
>  }
>  
> -void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format *fmt)
> +static void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format 
> *fmt)
>  {
>   fmt->fmt.pix.width = clamp(fmt->fmt.pix.width, 32U, 1280U);
>   fmt->fmt.pix.height = clamp(fmt->fmt.pix.height, 32U, 1024U);
> @@ -624,7 +624,7 @@ void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct 
> v4l2_format *fmt)
>   fmt->fmt.pix.sizeimage = fmt->fmt.pix.width * fmt->fmt.pix.height;
>  }
>  
> -int stk1135_enum_framesizes(struct gspca_dev *gspca_dev,
> +static int stk1135_enum_framesizes(struct gspca_dev *gspca_dev,
>   struct v4l2_frmsizeenum *fsize)
>  {
>   if (fsize->index != 0 || fsize->pixel_format != V4L2_PIX_FMT_SBGGR8)
> 

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

Re: [PATCH 00/10] [media] remove unnecessary pci_set_drvdata()

[Hans Verkuil]

On 09/23/2013 03:43 AM, Jingoo Han wrote:
> Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
> (device-core: Ensure drvdata = NULL when no driver is bound),
> the driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not needed to manually clear the
> device driver data to NULL.

For the whole patch series:

Acked-by: Hans Verkuil 

Regards,

Hans

> 
> ---
>  drivers/media/common/saa7146/saa7146_core.c |2 --
>  drivers/media/pci/b2c2/flexcop-pci.c|2 --
>  drivers/media/pci/bt8xx/bt878.c |1 -
>  drivers/media/pci/cx88/cx88-alsa.c  |2 --
>  drivers/media/pci/cx88/cx88-mpeg.c  |1 -
>  drivers/media/pci/cx88/cx88-video.c |1 -
>  drivers/media/pci/dm1105/dm1105.c   |2 --
>  drivers/media/pci/mantis/mantis_pci.c   |2 --
>  drivers/media/pci/ngene/ngene-core.c|2 --
>  drivers/media/pci/pluto2/pluto2.c   |2 --
>  drivers/media/pci/pt1/pt1.c |2 --
>  drivers/media/pci/saa7164/saa7164-core.c|1 -
>  12 files changed, 20 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

Re: [PATCH] saa7134: Fix crash when device is closed before streamoff

[Hans Verkuil]

On 09/20/2013 07:15 PM, Simon Farnsworth wrote:
> pm_qos_remove_request was not called on video_release, resulting in the PM
> core's list of requests being corrupted when the file handle was freed.
> 
> This has no immediate symptoms, but later in operation, the kernel will
> panic as the PM core dereferences a dangling pointer.
> 
> Signed-off-by: Simon Farnsworth 
> Cc: sta...@vger.kernel.org

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
> 
> I didn't notice this when I first implemented the pm_qos_request as the
> userspace I was using always called streamoff before closing the
> device. I've since changed userspace components, and hit the kernel panic.
> 
>  drivers/media/pci/saa7134/saa7134-video.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c 
> b/drivers/media/pci/saa7134/saa7134-video.c
> index e12bbd8..fb60da8 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -1455,6 +1455,7 @@ static int video_release(struct file *file)
>  
>   /* stop video capture */
>   if (res_check(fh, RESOURCE_VIDEO)) {
> + pm_qos_remove_request(&dev->qos_request);
>   videobuf_streamoff(&fh->cap);
>   res_free(dev,fh,RESOURCE_VIDEO);
>   }
> 

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

Re: [PATCH v3 3/4] [media] exynos-scaler: Add m2m functionality for the SCALER driver

[Hans Verkuil]

On 09/30/2013 12:48 PM, Shaik Ameer Basha wrote:
> Hi Hans,
> 
> 
> On Mon, Sep 30, 2013 at 4:02 PM, Hans Verkuil  wrote:
>> On 09/30/2013 11:32 AM, Shaik Ameer Basha wrote:
>>> Hi Hans,
>>>
>>> Thanks for pointing it out.
>>>
>>>
>>> On Mon, Sep 30, 2013 at 1:38 PM, Hans Verkuil  wrote:
 Hi Shaik,

 I have a few questions regarding the selection part...

 On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
> This patch adds the Makefile and memory to memory (m2m) interface
> functionality for the SCALER driver.
>
> Signed-off-by: Shaik Ameer Basha 
> ---
>  drivers/media/platform/Kconfig|8 +
>  drivers/media/platform/Makefile   |1 +
>  drivers/media/platform/exynos-scaler/Makefile |3 +
>  drivers/media/platform/exynos-scaler/scaler-m2m.c |  781 
> +
>  4 files changed, 793 insertions(+)
>  create mode 100644 drivers/media/platform/exynos-scaler/Makefile
>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c
>

>>>
>>>
>>> [...]
>>>
>>>
> +
> +static int scaler_m2m_s_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct scaler_frame *frame;
> + struct scaler_ctx *ctx = fh_to_ctx(fh);
> + struct v4l2_crop cr;
> + struct scaler_variant *variant = ctx->scaler_dev->variant;
> + int ret;
> +
> + cr.type = s->type;
> + cr.c = s->r;
> +
> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
> + return -EINVAL;
> +
> + ret = scaler_try_crop(ctx, &cr);
> + if (ret < 0)
> + return ret;
> +
> + if (s->flags & V4L2_SEL_FLAG_LE &&
> + !is_rectangle_enclosed(&cr.c, &s->r))
> + return -ERANGE;
> +
> + if (s->flags & V4L2_SEL_FLAG_GE &&
> + !is_rectangle_enclosed(&s->r, &cr.c))
> + return -ERANGE;
> +
> + s->r = cr.c;
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE:
> + frame = &ctx->s_frame;
> + break;
> +
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + frame = &ctx->d_frame;
> + break;

 Similar problems as with g_selection above. Tomasz mentioned to me that 
 the selection
 API is not implemented correctly in m2m Samsung drivers. It looks like 
 this code is
 copied-and-pasted from other drivers, so it seems he was right.
>>>
>>> Sorry, after going through the documentation, I have to agree with you...
>>> As you mentioned, this part of the code was copied while implementing
>>> the G-Scaler driver :)
>>>
>>> I will change the above implementation for M2M devices (GScaler and
>>> SCALER) as below,
>>> I will only allow all V4L2_SEL_TGT_COMPOSE_* target requests if
>>> 's->type' is equal to "V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE".
>>> and all V4L2_SEL_TGT_CROP_* target requests if 's->type' is equal to
>>> "V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE".
>>>
>>> I hope with the above two checkings taken in to care, there should not
>>> be any issues with using selection APIs here.
>>
>> Well, that depends on what the hardware does.
>>
>> Using compose with a capture buffer means that the frame as delivered by
>> the hardware is composed into a larger buffer. E.g. the hardware gives
>> you 1280x720 which is composed into a buffer of size 1920x1080.
>>
>> Using crop with an output buffer means that the hardware gets a cropped
>> part of a larger frame. E.g. you give a 1280x720 crop from a larger 1920x1080
>> buffer.
>>
>> I suspect however, that in this case the hardware does the opposite for
>> capture: you really want to crop with a capture buffer (e.g. the hardware
>> delivers a 1280x720 frame which is cropped before DMA to 640x360).
>>
>> I'm not sure what you want to do with an output buffer: cropping or 
>> composing.
>>
>> Tomasz mentioned that the M2M + selection API was screwy, and this seems to
>> be to be the case indeed.
>>
>> Which is also why I would like to know exactly what this hardware does.
> 
> This hardware is just a M2M device.
> It accepts one source buffer at a time and does some operations on
> that and saves to the destination buffer.
> Operations like Rotation, Cropping, Scaling, Color Space Conversion
> etc are possible.
> 
> Here when I provide the Output buffer (source buffer), I can apply all
> V4L2_SEL_TGT_CROP_* targets on it.
> That means I can select the whole buffer for processing or apply some
> crop and select that area for further processing.
> 
> similarly, On the capture buffer (output buffer

Re: [PATCH] v4l2: Support for multiple selections

[Hans Verkuil]

On 09/30/2013 01:17 PM, Ricardo Ribalda Delgado wrote:
> Hello Hans
> 
> As allways thank you very much for your comments.
> 
> On Mon, Sep 30, 2013 at 12:21 PM, Hans Verkuil  wrote:
>> Hi Ricardo,
>>
>> Sorry for the delayed review, but I'm finally catching up with my backlog.
>>
>> I've got a number of comments regarding this patch. I'm ignoring the platform
>> driver patches for now until the core support is correct.
>>
>> On 09/16/2013 02:54 PM, Ricardo Ribalda Delgado wrote:
>>> From: Ricardo Ribalda 
>>>
>>> Extend v4l2 selection API to support multiple selection areas, this way
>>> sensors that support multiple readout areas can work with multiple areas
>>> of insterest.
>>>
>>> The struct v4l2_selection and v4l2_subdev_selection has been extented
>>> with a new field rectangles. If it is value is different than zero the
>>> pr array is used instead of the r field.
>>>
>>> A new structure v4l2_ext_rect has been defined, containing 4 reserved
>>> fields for future improvements, as suggested by Hans.
>>>
>>> A new function in v4l2-comon (v4l2_selection_flat_struct) is in charge
>>> of converting a pr pointer with one item to a flatten struct. This
>>> function is used in all the old drivers that dont support multiple
>>> selections.
>>>
>>> Signed-off-by: Ricardo Ribalda Delgado 
>>> ---
>>>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |  6 +++
>>>  drivers/media/platform/exynos4-is/fimc-capture.c |  6 +++
>>>  drivers/media/platform/exynos4-is/fimc-lite.c|  6 +++
>>>  drivers/media/platform/s3c-camif/camif-capture.c |  6 +++
>>>  drivers/media/platform/s5p-jpeg/jpeg-core.c  |  3 ++
>>>  drivers/media/platform/s5p-tv/mixer_video.c  |  6 +++
>>>  drivers/media/platform/soc_camera/soc_camera.c   |  6 +++
>>>  drivers/media/v4l2-core/v4l2-common.c| 13 ++
>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 54 
>>> +---
>>>  include/media/v4l2-common.h  |  2 +
>>>  include/uapi/linux/v4l2-subdev.h | 10 -
>>>  include/uapi/linux/videodev2.h   | 15 ++-
>>>  12 files changed, 122 insertions(+), 11 deletions(-)
>>>
>>
>> 
>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
>>> b/drivers/media/v4l2-core/v4l2-common.c
>>> index a95e5e2..cd20567 100644
>>> --- a/drivers/media/v4l2-core/v4l2-common.c
>>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>>> @@ -886,3 +886,16 @@ void v4l2_get_timestamp(struct timeval *tv)
>>>   tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>>  }
>>>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
>>> +
>>> +int v4l2_selection_flat_struct(struct v4l2_selection *s)
>>> +{
>>> + if (s->rectangles == 0)
>>> + return 0;
>>> +
>>> + if (s->rectangles != 1)
>>> + return -EINVAL;
>>> +
>>> + s->r = s->pr[0].r;
>>
>> This would overwrite the pr pointer. Not a good idea.
> 
> That was exactly the point. The helper function convert the
> multi_selection, ext_rect to the legacy struct. This way the drivers
> needed almost no modification, just a call to the helper at the
> beginning of the handler.

That doesn't work: G and S_SELECTION are IOWR, so the driver can modify the
rectangles and those will have to be passed back to userspace. So you cannot
just change the contents of struct v4l2_selection.

> 
> Otherwise we need your get_rect helper, and then a set_rect helper at
> every exit.
> 
> If you think this is the way, then lets do it. Right now there are not
> too many drivers that supports selection, so it is right time to make
> such a decisions.
> 
>>
>> I would make a helper function like this:
>>
>> int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect 
>> *r)
>> {
>> if (s->rectangles > 1)
>> return -EINVAL;
>> if (s->rectangles == 1) {
>> *r = s->pr[0];
>> return 0;
>> }
>> if (s->r.width < 0 || s->r.height < 0)
>> return -EINVAL;
>> r->left = s->r.left;
>> r->top = s->r.top;
>> r->width = s->r.width;
>> r->height = s->r.height;
>> memset(r->reserved, 0, sizeof(r->reserved));
>> return 0;
>> }
>>
>> See also my proposed v4l2_ext_rect definition below.
>>
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_selection_flat_struct);
>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
>>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> index 68e6b5e..fe92f6b 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool 
>>> write_only)
>>>  static void v4l_print_selection(const void *arg, bool write_only)
>>>  {
>>>   const struct v4l2_selection *p = arg;
>>> + int i;
>>>
>>> - pr_cont("type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n",
>>> - prt_names(p->type, v4l2_type_names),
>>> - p->target, p->flags,
>>> - p->r

Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper

[Hans Verkuil]

On 09/19/2013 11:44 PM, Russell King wrote:
> Replace the following sequence:
> 
>   dma_set_mask(dev, mask);
>   dma_set_coherent_mask(dev, mask);
> 
> with a call to the new helper dma_set_mask_and_coherent().
> 
> Signed-off-by: Russell King 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/staging/media/dt3155v4l/dt3155v4l.c |5 +
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c 
> b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> index 90d6ac4..081407b 100644
> --- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
> +++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> @@ -901,10 +901,7 @@ dt3155_probe(struct pci_dev *pdev, const struct 
> pci_device_id *id)
>   int err;
>   struct dt3155_priv *pd;
>  
> - err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> - if (err)
> - return -ENODEV;
> - err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>   if (err)
>   return -ENODEV;
>   pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> 

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

Re: [PATCH 10/10] [media] coda: v4l2-compliance fix: zero pixel format priv field

[Hans Verkuil]

On 09/19/2013 11:13 AM, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/platform/coda.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index d15238a..6dec34d 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -556,6 +556,7 @@ static int coda_vidioc_g_fmt(struct file *file, void 
> *priv,
>  
>   f->fmt.pix.sizeimage= q_data->sizeimage;
>   f->fmt.pix.colorspace   = ctx->colorspace;
> + f->fmt.pix.priv = 0;

This is cleared by the core code, so this isn't necessary.

>  
>   return 0;
>  }
> @@ -613,6 +614,8 @@ static int coda_vidioc_try_fmt(struct coda_ctx *ctx, 
> struct coda_codec *codec,
>   BUG();
>   }
>  
> + f->fmt.pix.priv = 0;
> +

This is the only one that needs to be cleared manually, so this is fine.

Regards,

Hans

>   return 0;
>  }
>  
> 

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

Re: [PATCH 07/10] [media] coda: prefix v4l2_ioctl_ops with coda_

[Hans Verkuil]

On 09/19/2013 11:13 AM, Philipp Zabel wrote:
> Moving the ioctl handler callbacks into the coda namespace helps
> tremendously to make sense of backtraces.

I like the idea, but I would just use the coda_ prefix, not coda_vidioc_. In 
general
the prefix is either vidioc_ or the name of the driver, not both.

Regards,

Hans

> 
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/platform/coda.c | 123 
> +-
>  1 file changed, 63 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 4f038c2..9091664 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -412,8 +412,8 @@ static char *coda_product_name(int product)
>  /*
>   * V4L2 ioctl() operations.
>   */
> -static int vidioc_querycap(struct file *file, void *priv,
> -struct v4l2_capability *cap)
> +static int coda_vidioc_querycap(struct file *file, void *priv,
> + struct v4l2_capability *cap)
>  {
>   struct coda_ctx *ctx = fh_to_ctx(priv);
>  
> @@ -484,8 +484,8 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
>   return -EINVAL;
>  }
>  
> -static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
> -struct v4l2_fmtdesc *f)
> +static int coda_vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
> + struct v4l2_fmtdesc *f)
>  {
>   struct coda_ctx *ctx = fh_to_ctx(priv);
>   struct vb2_queue *src_vq;
> @@ -503,13 +503,14 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, 
> void *priv,
>   return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0);
>  }
>  
> -static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
> -struct v4l2_fmtdesc *f)
> +static int coda_vidioc_enum_fmt_vid_out(struct file *file, void *priv,
> + struct v4l2_fmtdesc *f)
>  {
>   return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_OUTPUT, 0);
>  }
>  
> -static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
> +static int coda_vidioc_g_fmt(struct file *file, void *priv,
> +  struct v4l2_format *f)
>  {
>   struct vb2_queue *vq;
>   struct coda_q_data *q_data;
> @@ -536,7 +537,7 @@ static int vidioc_g_fmt(struct file *file, void *priv, 
> struct v4l2_format *f)
>   return 0;
>  }
>  
> -static int vidioc_try_fmt(struct coda_codec *codec, struct v4l2_format *f)
> +static int coda_vidioc_try_fmt(struct coda_codec *codec, struct v4l2_format 
> *f)
>  {
>   unsigned int max_w, max_h;
>   enum v4l2_field field;
> @@ -575,8 +576,8 @@ static int vidioc_try_fmt(struct coda_codec *codec, 
> struct v4l2_format *f)
>   return 0;
>  }
>  
> -static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
> -   struct v4l2_format *f)
> +static int coda_vidioc_try_fmt_vid_cap(struct file *file, void *priv,
> +struct v4l2_format *f)
>  {
>   struct coda_ctx *ctx = fh_to_ctx(priv);
>   struct coda_codec *codec;
> @@ -604,7 +605,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void 
> *priv,
>  
>   f->fmt.pix.colorspace = ctx->colorspace;
>  
> - ret = vidioc_try_fmt(codec, f);
> + ret = coda_vidioc_try_fmt(codec, f);
>   if (ret < 0)
>   return ret;
>  
> @@ -620,8 +621,8 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> -static int vidioc_try_fmt_vid_out(struct file *file, void *priv,
> -   struct v4l2_format *f)
> +static int coda_vidioc_try_fmt_vid_out(struct file *file, void *priv,
> +struct v4l2_format *f)
>  {
>   struct coda_ctx *ctx = fh_to_ctx(priv);
>   struct coda_codec *codec;
> @@ -633,10 +634,10 @@ static int vidioc_try_fmt_vid_out(struct file *file, 
> void *priv,
>   if (!f->fmt.pix.colorspace)
>   f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
>  
> - return vidioc_try_fmt(codec, f);
> + return coda_vidioc_try_fmt(codec, f);
>  }
>  
> -static int vidioc_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
> +static int coda_vidioc_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
>  {
>   struct coda_q_data *q_data;
>   struct vb2_queue *vq;
> @@ -666,61 +667,62 @@ static int vidioc_s_fmt(struct coda_ctx *ctx, struct 
> v4l2_format *f)
>   return 0;
>  }
>  
> -static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
> - struct v4l2_format *f)
> +static int coda_vidioc_s_fmt_vid_cap(struct file *file, void *priv,
> +  struct v4l2_format *f)
>  {
>   struct coda_ctx *ctx = fh_to_ctx(priv);
>   int ret;
>  
> - ret = vidioc_try_fmt_vid_cap(file, priv, f);
> + ret = coda_vidioc_try_fmt_vid_cap

Re: [PATCH 04/10] [media] coda: fix FMO value setting for CodaDx6

[Hans Verkuil]

On 09/19/2013 11:13 AM, Philipp Zabel wrote:
> The register is only written on CodaDx6, so the temporary variable
> to be written only needs to be initialized on CodaDx6.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/platform/coda.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 53539c1..e8acff3 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -2074,10 +2074,10 @@ static int coda_start_streaming(struct vb2_queue *q, 
> unsigned int count)
>   coda_setup_iram(ctx);
>  
>   if (dst_fourcc == V4L2_PIX_FMT_H264) {
> - value  = (FMO_SLICE_SAVE_BUF_SIZE << 7);
> - value |= (0 & CODA_FMOPARAM_TYPE_MASK) << 
> CODA_FMOPARAM_TYPE_OFFSET;
> - value |=  0 & CODA_FMOPARAM_SLICENUM_MASK;
>   if (dev->devtype->product == CODA_DX6) {
> + value  = (FMO_SLICE_SAVE_BUF_SIZE << 7);
> + value |= (0 & CODA_FMOPARAM_TYPE_MASK) << 
> CODA_FMOPARAM_TYPE_OFFSET;
> + value |=  0 & CODA_FMOPARAM_SLICENUM_MASK;

0 & CODA_FMOPARAM_SLICENUM_MASK?

These last two lines evaluate to a nop, so that looks very weird. Is this a bug?

Regards,

Hans

>   coda_write(dev, value, CODADX6_CMD_ENC_SEQ_FMO);
>   } else {
>   coda_write(dev, ctx->iram_info.search_ram_paddr,
> 

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

Re: [PATCH] [media] videobuf2-core: call __setup_offsets only for mmap memory type

[Hans Verkuil]

On 09/19/2013 09:37 AM, Philipp Zabel wrote:
> __setup_offsets fills the v4l2_planes' mem_offset fields, which is only valid
> for V4L2_MEMORY_MMAP type buffers. For V4L2_MEMORY_DMABUF and _USERPTR 
> buffers,
> this incorrectly overwrites the fd and userptr fields.
> 
> Reported-by: Michael Olbrich 
> Signed-off-by: Philipp Zabel 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 9c865da..95a798e 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -241,7 +241,8 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
> v4l2_memory memory,
>   q->bufs[q->num_buffers + buffer] = vb;
>   }
>  
> - __setup_offsets(q, buffer);
> + if (memory == V4L2_MEMORY_MMAP)
> + __setup_offsets(q, buffer);
>  
>   dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
>   buffer, num_planes);
> 

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

Re: [RFC 1/1] v4l: return POLLERR on V4L2 sub-devices if no events are subscribed

[Sakari Ailus]

Hi Hans,

Thanks for the review.

Hans Verkuil wrote:

Hi Teemu, Sakari,

I've been thinking about this change and I think it is a reasonable change.
I had some doubts earlier, but after thinking it through I agree with this.

But it needs a bit more work, see below.

On 09/17/2013 08:07 PM, Sakari Ailus wrote:

From: Teemu Tuominen 

Add check and return POLLERR from subdev_poll() in case of no events
subscribed and wakeup once the last event subscription is removed.

This change is essentially done to add possibility to wakeup polling
with concurrent unsubscribe.

Signed-off-by: Teemu Tuominen 

Move the check after calling poll_wait(). Otherwise it's possible that we go
to sleep without getting notified if the subscription went away between the
two.

Signed-off-by: Sakari Ailus 
Tested-by: Teemu Tuominen 
---
Hi all,

Poll for events will sleep forever if there are no events subscribed.
Calling poll from an application that has not subscribed any event indeed
sounds silly, but un multi-threaded applications this is not quite as
straightforward.

Assume the following: an application has two threads where one handles
event subscription and the other handles the events. The first thread
unsubscribes the events and the latter will sleep forever. And do this while
the program intends to quit without an intention to subscribe for any
further events, and there's a deadlock.

Alternative solutions to handle this are signals (rather a nuisance if the
application happens to be a library instead) or a pipe (2) between the
threads. Pipe is workable, but instead of being a proper solution to the
problem still looks like a workaround instead.

This patch fixes the issue on kernel side by waking up the processes
sleeping in poll and returning POLLERR when the last subscribed event is
gone. The behaviour mirrors that of videobuf2 which will return POLLERR if
either streaming is disabled or no buffers are queued.

Just "waking up the sleeping threads once" is not an option: threads are not
visible to the kernel at this level and just "waking them up" isn't either
since poll will go back to sleep before returning the control back to user
space as long as it would return zero.

(Thinking about it --- similar change should probably be made to videobuf2
event poll handling as well.)


Yes, this should be added here as well. Can you implement that as well?


Sure. I thought I'd do that once I get the idea accepted. :-) This will 
take some time but I'll do that.




Kind regards,
Sakari

  drivers/media/v4l2-core/v4l2-event.c  | 15 +++
  drivers/media/v4l2-core/v4l2-subdev.c |  3 +++
  include/media/v4l2-event.h|  1 +
  3 files changed, 19 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-event.c 
b/drivers/media/v4l2-core/v4l2-event.c
index 86dcb54..b53897e 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -107,6 +107,19 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
return NULL;
  }

+bool v4l2_event_has_subscribed(struct v4l2_fh *fh)
+{
+   unsigned long flags;
+   bool rval;
+
+   spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+   rval = !list_empty(&fh->subscribed);
+   spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+   return rval;
+}
+EXPORT_SYMBOL(v4l2_event_has_subscribed);
+
  static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event 
*ev,
const struct timespec *ts)
  {
@@ -299,6 +312,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
fh->navailable--;
}
list_del(&sev->list);
+   if (list_empty(&fh->subscribed))
+   wake_up_all(&fh->wait);
}

spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index 996c248..f2aa00f 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -382,6 +382,9 @@ static unsigned int subdev_poll(struct file *file, 
poll_table *wait)
if (v4l2_event_pending(fh))
return POLLPRI;

+   if (!v4l2_event_has_subscribed(fh))
+   return POLLERR;
+


This needs a bit more work: you should also check that poll() actually is 
waiting
for exceptions. If not, then also return POLLERR. This has never been checked 
before,
but it is needed.


Good point. I'll fix that.

--
Kind regards,

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

Re: ivtv 1.4.2/1.4.3 broken in recent kernels?

[Hans Verkuil]

On 09/17/2013 10:49 PM, Rajil Saraswat wrote:
> Hi,
> 
>  I have a couple of PVR-500's which have additional tuners connected
> to them (using daughter cards). The audio is not usable on either
> 1.4.2 or 1.4.3 ivtv drivers. The issue is described at
> http://ivtvdriver.org/pipermail/ivtv-users/2013-September/010462.html
> 
> Is there anything i can do to make kernel 3.10.7 (ivtv 1.4.3) play
> nice with my card?

Andy, is this something you can look at?

Regards,

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

Re: [RFC 1/1] v4l: return POLLERR on V4L2 sub-devices if no events are subscribed

[Hans Verkuil]

Hi Teemu, Sakari,

I've been thinking about this change and I think it is a reasonable change.
I had some doubts earlier, but after thinking it through I agree with this.

But it needs a bit more work, see below.

On 09/17/2013 08:07 PM, Sakari Ailus wrote:
> From: Teemu Tuominen 
> 
> Add check and return POLLERR from subdev_poll() in case of no events
> subscribed and wakeup once the last event subscription is removed.
> 
> This change is essentially done to add possibility to wakeup polling
> with concurrent unsubscribe.
> 
> Signed-off-by: Teemu Tuominen 
> 
> Move the check after calling poll_wait(). Otherwise it's possible that we go
> to sleep without getting notified if the subscription went away between the
> two.
> 
> Signed-off-by: Sakari Ailus 
> Tested-by: Teemu Tuominen 
> ---
> Hi all,
> 
> Poll for events will sleep forever if there are no events subscribed.
> Calling poll from an application that has not subscribed any event indeed
> sounds silly, but un multi-threaded applications this is not quite as
> straightforward.
> 
> Assume the following: an application has two threads where one handles
> event subscription and the other handles the events. The first thread
> unsubscribes the events and the latter will sleep forever. And do this while
> the program intends to quit without an intention to subscribe for any
> further events, and there's a deadlock.
> 
> Alternative solutions to handle this are signals (rather a nuisance if the
> application happens to be a library instead) or a pipe (2) between the
> threads. Pipe is workable, but instead of being a proper solution to the
> problem still looks like a workaround instead.
> 
> This patch fixes the issue on kernel side by waking up the processes
> sleeping in poll and returning POLLERR when the last subscribed event is
> gone. The behaviour mirrors that of videobuf2 which will return POLLERR if
> either streaming is disabled or no buffers are queued.
> 
> Just "waking up the sleeping threads once" is not an option: threads are not
> visible to the kernel at this level and just "waking them up" isn't either
> since poll will go back to sleep before returning the control back to user
> space as long as it would return zero.
> 
> (Thinking about it --- similar change should probably be made to videobuf2
> event poll handling as well.)

Yes, this should be added here as well. Can you implement that as well?

> 
> Kind regards,
> Sakari
> 
>  drivers/media/v4l2-core/v4l2-event.c  | 15 +++
>  drivers/media/v4l2-core/v4l2-subdev.c |  3 +++
>  include/media/v4l2-event.h|  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-event.c 
> b/drivers/media/v4l2-core/v4l2-event.c
> index 86dcb54..b53897e 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -107,6 +107,19 @@ static struct v4l2_subscribed_event 
> *v4l2_event_subscribed(
>   return NULL;
>  }
>  
> +bool v4l2_event_has_subscribed(struct v4l2_fh *fh)
> +{
> + unsigned long flags;
> + bool rval;
> +
> + spin_lock_irqsave(&fh->vdev->fh_lock, flags);
> + rval = !list_empty(&fh->subscribed);
> + spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> +
> + return rval;
> +}
> +EXPORT_SYMBOL(v4l2_event_has_subscribed);
> +
>  static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct 
> v4l2_event *ev,
>   const struct timespec *ts)
>  {
> @@ -299,6 +312,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
>   fh->navailable--;
>   }
>   list_del(&sev->list);
> + if (list_empty(&fh->subscribed))
> + wake_up_all(&fh->wait);
>   }
>  
>   spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
> b/drivers/media/v4l2-core/v4l2-subdev.c
> index 996c248..f2aa00f 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -382,6 +382,9 @@ static unsigned int subdev_poll(struct file *file, 
> poll_table *wait)
>   if (v4l2_event_pending(fh))
>   return POLLPRI;
>  
> + if (!v4l2_event_has_subscribed(fh))
> + return POLLERR;
> +

This needs a bit more work: you should also check that poll() actually is 
waiting
for exceptions. If not, then also return POLLERR. This has never been checked 
before,
but it is needed.

>   return 0;
>  }
>  
> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
> index be05d01..a9ca2b5 100644
> --- a/include/media/v4l2-event.h
> +++ b/include/media/v4l2-event.h
> @@ -121,6 +121,7 @@ struct v4l2_subscribed_event {
>  
>  int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
>  int nonblocking);
> +bool v4l2_event_has_subscribed(struct v4l2_fh *fh);
>  void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event 
> *ev);
>  void v4l2_event_queue_fh(st

Re: [PATCH] v4l2: Support for multiple selections

[Ricardo Ribalda Delgado]

Hello Hans

As allways thank you very much for your comments.

On Mon, Sep 30, 2013 at 12:21 PM, Hans Verkuil  wrote:
> Hi Ricardo,
>
> Sorry for the delayed review, but I'm finally catching up with my backlog.
>
> I've got a number of comments regarding this patch. I'm ignoring the platform
> driver patches for now until the core support is correct.
>
> On 09/16/2013 02:54 PM, Ricardo Ribalda Delgado wrote:
>> From: Ricardo Ribalda 
>>
>> Extend v4l2 selection API to support multiple selection areas, this way
>> sensors that support multiple readout areas can work with multiple areas
>> of insterest.
>>
>> The struct v4l2_selection and v4l2_subdev_selection has been extented
>> with a new field rectangles. If it is value is different than zero the
>> pr array is used instead of the r field.
>>
>> A new structure v4l2_ext_rect has been defined, containing 4 reserved
>> fields for future improvements, as suggested by Hans.
>>
>> A new function in v4l2-comon (v4l2_selection_flat_struct) is in charge
>> of converting a pr pointer with one item to a flatten struct. This
>> function is used in all the old drivers that dont support multiple
>> selections.
>>
>> Signed-off-by: Ricardo Ribalda Delgado 
>> ---
>>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |  6 +++
>>  drivers/media/platform/exynos4-is/fimc-capture.c |  6 +++
>>  drivers/media/platform/exynos4-is/fimc-lite.c|  6 +++
>>  drivers/media/platform/s3c-camif/camif-capture.c |  6 +++
>>  drivers/media/platform/s5p-jpeg/jpeg-core.c  |  3 ++
>>  drivers/media/platform/s5p-tv/mixer_video.c  |  6 +++
>>  drivers/media/platform/soc_camera/soc_camera.c   |  6 +++
>>  drivers/media/v4l2-core/v4l2-common.c| 13 ++
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 54 
>> +---
>>  include/media/v4l2-common.h  |  2 +
>>  include/uapi/linux/v4l2-subdev.h | 10 -
>>  include/uapi/linux/videodev2.h   | 15 ++-
>>  12 files changed, 122 insertions(+), 11 deletions(-)
>>
>
> 
>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
>> b/drivers/media/v4l2-core/v4l2-common.c
>> index a95e5e2..cd20567 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -886,3 +886,16 @@ void v4l2_get_timestamp(struct timeval *tv)
>>   tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>  }
>>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
>> +
>> +int v4l2_selection_flat_struct(struct v4l2_selection *s)
>> +{
>> + if (s->rectangles == 0)
>> + return 0;
>> +
>> + if (s->rectangles != 1)
>> + return -EINVAL;
>> +
>> + s->r = s->pr[0].r;
>
> This would overwrite the pr pointer. Not a good idea.

That was exactly the point. The helper function convert the
multi_selection, ext_rect to the legacy struct. This way the drivers
needed almost no modification, just a call to the helper at the
beginning of the handler.

Otherwise we need your get_rect helper, and then a set_rect helper at
every exit.

If you think this is the way, then lets do it. Right now there are not
too many drivers that supports selection, so it is right time to make
such a decisions.

>
> I would make a helper function like this:
>
> int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect *r)
> {
> if (s->rectangles > 1)
> return -EINVAL;
> if (s->rectangles == 1) {
> *r = s->pr[0];
> return 0;
> }
> if (s->r.width < 0 || s->r.height < 0)
> return -EINVAL;
> r->left = s->r.left;
> r->top = s->r.top;
> r->width = s->r.width;
> r->height = s->r.height;
> memset(r->reserved, 0, sizeof(r->reserved));
> return 0;
> }
>
> See also my proposed v4l2_ext_rect definition below.
>
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_selection_flat_struct);
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 68e6b5e..fe92f6b 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool 
>> write_only)
>>  static void v4l_print_selection(const void *arg, bool write_only)
>>  {
>>   const struct v4l2_selection *p = arg;
>> + int i;
>>
>> - pr_cont("type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n",
>> - prt_names(p->type, v4l2_type_names),
>> - p->target, p->flags,
>> - p->r.width, p->r.height, p->r.left, p->r.top);
>> + if (p->rectangles == 0)
>> + pr_cont("type=%s, target=%d, flags=0x%x, wxh=%dx%d, 
>> x,y=%d,%d\n",
>> + prt_names(p->type, v4l2_type_names),
>> + p->target, p->flags,
>> + p->r.width, p->r.height, p->r.left, p->r.top);
>> + else{
>> + pr_cont("type=%s, target=%d, flags=

Re: [PATCH v3 3/4] [media] exynos-scaler: Add m2m functionality for the SCALER driver

[Shaik Ameer Basha]

On Mon, Sep 30, 2013 at 4:18 PM, Shaik Ameer Basha
 wrote:
> Hi Hans,
>
>
> On Mon, Sep 30, 2013 at 4:02 PM, Hans Verkuil  wrote:
>> On 09/30/2013 11:32 AM, Shaik Ameer Basha wrote:
>>> Hi Hans,
>>>
>>> Thanks for pointing it out.
>>>
>>>
>>> On Mon, Sep 30, 2013 at 1:38 PM, Hans Verkuil  wrote:
 Hi Shaik,

 I have a few questions regarding the selection part...

 On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
> This patch adds the Makefile and memory to memory (m2m) interface
> functionality for the SCALER driver.
>
> Signed-off-by: Shaik Ameer Basha 
> ---
>  drivers/media/platform/Kconfig|8 +
>  drivers/media/platform/Makefile   |1 +
>  drivers/media/platform/exynos-scaler/Makefile |3 +
>  drivers/media/platform/exynos-scaler/scaler-m2m.c |  781 
> +
>  4 files changed, 793 insertions(+)
>  create mode 100644 drivers/media/platform/exynos-scaler/Makefile
>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c
>

>>>
>>>
>>> [...]
>>>
>>>
> +
> +static int scaler_m2m_s_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct scaler_frame *frame;
> + struct scaler_ctx *ctx = fh_to_ctx(fh);
> + struct v4l2_crop cr;
> + struct scaler_variant *variant = ctx->scaler_dev->variant;
> + int ret;
> +
> + cr.type = s->type;
> + cr.c = s->r;
> +
> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
> + return -EINVAL;
> +
> + ret = scaler_try_crop(ctx, &cr);
> + if (ret < 0)
> + return ret;
> +
> + if (s->flags & V4L2_SEL_FLAG_LE &&
> + !is_rectangle_enclosed(&cr.c, &s->r))
> + return -ERANGE;
> +
> + if (s->flags & V4L2_SEL_FLAG_GE &&
> + !is_rectangle_enclosed(&s->r, &cr.c))
> + return -ERANGE;
> +
> + s->r = cr.c;
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE:
> + frame = &ctx->s_frame;
> + break;
> +
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + frame = &ctx->d_frame;
> + break;

 Similar problems as with g_selection above. Tomasz mentioned to me that 
 the selection
 API is not implemented correctly in m2m Samsung drivers. It looks like 
 this code is
 copied-and-pasted from other drivers, so it seems he was right.
>>>
>>> Sorry, after going through the documentation, I have to agree with you...
>>> As you mentioned, this part of the code was copied while implementing
>>> the G-Scaler driver :)
>>>
>>> I will change the above implementation for M2M devices (GScaler and
>>> SCALER) as below,
>>> I will only allow all V4L2_SEL_TGT_COMPOSE_* target requests if
>>> 's->type' is equal to "V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE".
>>> and all V4L2_SEL_TGT_CROP_* target requests if 's->type' is equal to
>>> "V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE".
>>>
>>> I hope with the above two checkings taken in to care, there should not
>>> be any issues with using selection APIs here.
>>
>> Well, that depends on what the hardware does.
>>
>> Using compose with a capture buffer means that the frame as delivered by
>> the hardware is composed into a larger buffer. E.g. the hardware gives
>> you 1280x720 which is composed into a buffer of size 1920x1080.
>>
>> Using crop with an output buffer means that the hardware gets a cropped
>> part of a larger frame. E.g. you give a 1280x720 crop from a larger 1920x1080
>> buffer.
>>
>> I suspect however, that in this case the hardware does the opposite for
>> capture: you really want to crop with a capture buffer (e.g. the hardware
>> delivers a 1280x720 frame which is cropped before DMA to 640x360).
>>
>> I'm not sure what you want to do with an output buffer: cropping or 
>> composing.
>>
>> Tomasz mentioned that the M2M + selection API was screwy, and this seems to
>> be to be the case indeed.
>>
>> Which is also why I would like to know exactly what this hardware does.
>
> This hardware is just a M2M device.
> It accepts one source buffer at a time and does some operations on
> that and saves to the destination buffer.
> Operations like Rotation, Cropping, Scaling, Color Space Conversion
> etc are possible.
>
> Here when I provide the Output buffer (source buffer), I can apply all
> V4L2_SEL_TGT_CROP_* targets on it.
> That means I can select the whole buffer for processing or apply some
> crop and select that area for further processing.
>
> similarly, On the capture buffer (output b

Re: [PATCH v3 3/4] [media] exynos-scaler: Add m2m functionality for the SCALER driver

[Shaik Ameer Basha]

Hi Hans,


On Mon, Sep 30, 2013 at 4:02 PM, Hans Verkuil  wrote:
> On 09/30/2013 11:32 AM, Shaik Ameer Basha wrote:
>> Hi Hans,
>>
>> Thanks for pointing it out.
>>
>>
>> On Mon, Sep 30, 2013 at 1:38 PM, Hans Verkuil  wrote:
>>> Hi Shaik,
>>>
>>> I have a few questions regarding the selection part...
>>>
>>> On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
 This patch adds the Makefile and memory to memory (m2m) interface
 functionality for the SCALER driver.

 Signed-off-by: Shaik Ameer Basha 
 ---
  drivers/media/platform/Kconfig|8 +
  drivers/media/platform/Makefile   |1 +
  drivers/media/platform/exynos-scaler/Makefile |3 +
  drivers/media/platform/exynos-scaler/scaler-m2m.c |  781 
 +
  4 files changed, 793 insertions(+)
  create mode 100644 drivers/media/platform/exynos-scaler/Makefile
  create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c

>>>
>>
>>
>> [...]
>>
>>
 +
 +static int scaler_m2m_s_selection(struct file *file, void *fh,
 + struct v4l2_selection *s)
 +{
 + struct scaler_frame *frame;
 + struct scaler_ctx *ctx = fh_to_ctx(fh);
 + struct v4l2_crop cr;
 + struct scaler_variant *variant = ctx->scaler_dev->variant;
 + int ret;
 +
 + cr.type = s->type;
 + cr.c = s->r;
 +
 + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
 + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
 + return -EINVAL;
 +
 + ret = scaler_try_crop(ctx, &cr);
 + if (ret < 0)
 + return ret;
 +
 + if (s->flags & V4L2_SEL_FLAG_LE &&
 + !is_rectangle_enclosed(&cr.c, &s->r))
 + return -ERANGE;
 +
 + if (s->flags & V4L2_SEL_FLAG_GE &&
 + !is_rectangle_enclosed(&s->r, &cr.c))
 + return -ERANGE;
 +
 + s->r = cr.c;
 +
 + switch (s->target) {
 + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
 + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
 + case V4L2_SEL_TGT_COMPOSE:
 + frame = &ctx->s_frame;
 + break;
 +
 + case V4L2_SEL_TGT_CROP_BOUNDS:
 + case V4L2_SEL_TGT_CROP:
 + case V4L2_SEL_TGT_CROP_DEFAULT:
 + frame = &ctx->d_frame;
 + break;
>>>
>>> Similar problems as with g_selection above. Tomasz mentioned to me that the 
>>> selection
>>> API is not implemented correctly in m2m Samsung drivers. It looks like this 
>>> code is
>>> copied-and-pasted from other drivers, so it seems he was right.
>>
>> Sorry, after going through the documentation, I have to agree with you...
>> As you mentioned, this part of the code was copied while implementing
>> the G-Scaler driver :)
>>
>> I will change the above implementation for M2M devices (GScaler and
>> SCALER) as below,
>> I will only allow all V4L2_SEL_TGT_COMPOSE_* target requests if
>> 's->type' is equal to "V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE".
>> and all V4L2_SEL_TGT_CROP_* target requests if 's->type' is equal to
>> "V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE".
>>
>> I hope with the above two checkings taken in to care, there should not
>> be any issues with using selection APIs here.
>
> Well, that depends on what the hardware does.
>
> Using compose with a capture buffer means that the frame as delivered by
> the hardware is composed into a larger buffer. E.g. the hardware gives
> you 1280x720 which is composed into a buffer of size 1920x1080.
>
> Using crop with an output buffer means that the hardware gets a cropped
> part of a larger frame. E.g. you give a 1280x720 crop from a larger 1920x1080
> buffer.
>
> I suspect however, that in this case the hardware does the opposite for
> capture: you really want to crop with a capture buffer (e.g. the hardware
> delivers a 1280x720 frame which is cropped before DMA to 640x360).
>
> I'm not sure what you want to do with an output buffer: cropping or composing.
>
> Tomasz mentioned that the M2M + selection API was screwy, and this seems to
> be to be the case indeed.
>
> Which is also why I would like to know exactly what this hardware does.

This hardware is just a M2M device.
It accepts one source buffer at a time and does some operations on
that and saves to the destination buffer.
Operations like Rotation, Cropping, Scaling, Color Space Conversion
etc are possible.

Here when I provide the Output buffer (source buffer), I can apply all
V4L2_SEL_TGT_CROP_* targets on it.
That means I can select the whole buffer for processing or apply some
crop and select that area for further processing.

similarly, On the capture buffer (output buffer), I can apply
V4L2_SEL_TGT_COMPOSE_* targets.
That means I can compose the final output to the complete capture
frame (dst frame), or I can choose some part of the destination frame.

Regards,
Shaik Ame

Re: [PATCH v3 3/4] [media] exynos-scaler: Add m2m functionality for the SCALER driver

[Hans Verkuil]

On 09/30/2013 11:32 AM, Shaik Ameer Basha wrote:
> Hi Hans,
> 
> Thanks for pointing it out.
> 
> 
> On Mon, Sep 30, 2013 at 1:38 PM, Hans Verkuil  wrote:
>> Hi Shaik,
>>
>> I have a few questions regarding the selection part...
>>
>> On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
>>> This patch adds the Makefile and memory to memory (m2m) interface
>>> functionality for the SCALER driver.
>>>
>>> Signed-off-by: Shaik Ameer Basha 
>>> ---
>>>  drivers/media/platform/Kconfig|8 +
>>>  drivers/media/platform/Makefile   |1 +
>>>  drivers/media/platform/exynos-scaler/Makefile |3 +
>>>  drivers/media/platform/exynos-scaler/scaler-m2m.c |  781 
>>> +
>>>  4 files changed, 793 insertions(+)
>>>  create mode 100644 drivers/media/platform/exynos-scaler/Makefile
>>>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c
>>>
>>
> 
> 
> [...]
> 
> 
>>> +
>>> +static int scaler_m2m_s_selection(struct file *file, void *fh,
>>> + struct v4l2_selection *s)
>>> +{
>>> + struct scaler_frame *frame;
>>> + struct scaler_ctx *ctx = fh_to_ctx(fh);
>>> + struct v4l2_crop cr;
>>> + struct scaler_variant *variant = ctx->scaler_dev->variant;
>>> + int ret;
>>> +
>>> + cr.type = s->type;
>>> + cr.c = s->r;
>>> +
>>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
>>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
>>> + return -EINVAL;
>>> +
>>> + ret = scaler_try_crop(ctx, &cr);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (s->flags & V4L2_SEL_FLAG_LE &&
>>> + !is_rectangle_enclosed(&cr.c, &s->r))
>>> + return -ERANGE;
>>> +
>>> + if (s->flags & V4L2_SEL_FLAG_GE &&
>>> + !is_rectangle_enclosed(&s->r, &cr.c))
>>> + return -ERANGE;
>>> +
>>> + s->r = cr.c;
>>> +
>>> + switch (s->target) {
>>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>>> + case V4L2_SEL_TGT_COMPOSE:
>>> + frame = &ctx->s_frame;
>>> + break;
>>> +
>>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>>> + case V4L2_SEL_TGT_CROP:
>>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>>> + frame = &ctx->d_frame;
>>> + break;
>>
>> Similar problems as with g_selection above. Tomasz mentioned to me that the 
>> selection
>> API is not implemented correctly in m2m Samsung drivers. It looks like this 
>> code is
>> copied-and-pasted from other drivers, so it seems he was right.
> 
> Sorry, after going through the documentation, I have to agree with you...
> As you mentioned, this part of the code was copied while implementing
> the G-Scaler driver :)
> 
> I will change the above implementation for M2M devices (GScaler and
> SCALER) as below,
> I will only allow all V4L2_SEL_TGT_COMPOSE_* target requests if
> 's->type' is equal to "V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE".
> and all V4L2_SEL_TGT_CROP_* target requests if 's->type' is equal to
> "V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE".
> 
> I hope with the above two checkings taken in to care, there should not
> be any issues with using selection APIs here.

Well, that depends on what the hardware does.

Using compose with a capture buffer means that the frame as delivered by
the hardware is composed into a larger buffer. E.g. the hardware gives
you 1280x720 which is composed into a buffer of size 1920x1080.

Using crop with an output buffer means that the hardware gets a cropped
part of a larger frame. E.g. you give a 1280x720 crop from a larger 1920x1080
buffer.

I suspect however, that in this case the hardware does the opposite for
capture: you really want to crop with a capture buffer (e.g. the hardware
delivers a 1280x720 frame which is cropped before DMA to 640x360).

I'm not sure what you want to do with an output buffer: cropping or composing.

Tomasz mentioned that the M2M + selection API was screwy, and this seems to
be to be the case indeed.

Which is also why I would like to know exactly what this hardware does.

Regards,

Hans

> 
> Thanks,
> Shaik Ameer Basha
> 
>>
>> The selection API for m2m devices will be discussed during the upcoming V4L2 
>> mini-summit
>> since the API may actually need some adjustments to have it work the way it 
>> should.
>>
>> As requested above, if you can explain the exact functionality you are 
>> trying to
>> implement here, then I can look over this code carefully and see how it 
>> should be done.
>>
>> Thanks!
>>
>> Hans
>>
> [...]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

Re: [PATCH] v4l2: Support for multiple selections

[Hans Verkuil]

Hi Ricardo,

Sorry for the delayed review, but I'm finally catching up with my backlog.

I've got a number of comments regarding this patch. I'm ignoring the platform
driver patches for now until the core support is correct.

On 09/16/2013 02:54 PM, Ricardo Ribalda Delgado wrote:
> From: Ricardo Ribalda 
> 
> Extend v4l2 selection API to support multiple selection areas, this way
> sensors that support multiple readout areas can work with multiple areas
> of insterest.
> 
> The struct v4l2_selection and v4l2_subdev_selection has been extented
> with a new field rectangles. If it is value is different than zero the
> pr array is used instead of the r field.
> 
> A new structure v4l2_ext_rect has been defined, containing 4 reserved
> fields for future improvements, as suggested by Hans.
> 
> A new function in v4l2-comon (v4l2_selection_flat_struct) is in charge
> of converting a pr pointer with one item to a flatten struct. This
> function is used in all the old drivers that dont support multiple
> selections.
> 
> Signed-off-by: Ricardo Ribalda Delgado 
> ---
>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |  6 +++
>  drivers/media/platform/exynos4-is/fimc-capture.c |  6 +++
>  drivers/media/platform/exynos4-is/fimc-lite.c|  6 +++
>  drivers/media/platform/s3c-camif/camif-capture.c |  6 +++
>  drivers/media/platform/s5p-jpeg/jpeg-core.c  |  3 ++
>  drivers/media/platform/s5p-tv/mixer_video.c  |  6 +++
>  drivers/media/platform/soc_camera/soc_camera.c   |  6 +++
>  drivers/media/v4l2-core/v4l2-common.c| 13 ++
>  drivers/media/v4l2-core/v4l2-ioctl.c | 54 
> +---
>  include/media/v4l2-common.h  |  2 +
>  include/uapi/linux/v4l2-subdev.h | 10 -
>  include/uapi/linux/videodev2.h   | 15 ++-
>  12 files changed, 122 insertions(+), 11 deletions(-)
> 



> diff --git a/drivers/media/v4l2-core/v4l2-common.c 
> b/drivers/media/v4l2-core/v4l2-common.c
> index a95e5e2..cd20567 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -886,3 +886,16 @@ void v4l2_get_timestamp(struct timeval *tv)
>   tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
> +
> +int v4l2_selection_flat_struct(struct v4l2_selection *s)
> +{
> + if (s->rectangles == 0)
> + return 0;
> +
> + if (s->rectangles != 1)
> + return -EINVAL;
> +
> + s->r = s->pr[0].r;

This would overwrite the pr pointer. Not a good idea.

I would make a helper function like this:

int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect *r)
{
if (s->rectangles > 1)
return -EINVAL;
if (s->rectangles == 1) {
*r = s->pr[0];
return 0;
}
if (s->r.width < 0 || s->r.height < 0)
return -EINVAL;
r->left = s->r.left;
r->top = s->r.top;
r->width = s->r.width;
r->height = s->r.height;
memset(r->reserved, 0, sizeof(r->reserved));
return 0;
}

See also my proposed v4l2_ext_rect definition below.

> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_selection_flat_struct);
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 68e6b5e..fe92f6b 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool 
> write_only)
>  static void v4l_print_selection(const void *arg, bool write_only)
>  {
>   const struct v4l2_selection *p = arg;
> + int i;
>  
> - pr_cont("type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n",
> - prt_names(p->type, v4l2_type_names),
> - p->target, p->flags,
> - p->r.width, p->r.height, p->r.left, p->r.top);
> + if (p->rectangles == 0)
> + pr_cont("type=%s, target=%d, flags=0x%x, wxh=%dx%d, 
> x,y=%d,%d\n",
> + prt_names(p->type, v4l2_type_names),
> + p->target, p->flags,
> + p->r.width, p->r.height, p->r.left, p->r.top);
> + else{
> + pr_cont("type=%s, target=%d, flags=0x%x rectangles=%d\n",
> + prt_names(p->type, v4l2_type_names),
> + p->target, p->flags, p->rectangles);
> + for (i = 0; i < p->rectangles; i++)
> + pr_cont("rectangle %d: wxh=%dx%d, x,y=%d,%d\n",
> + i, p->r.width, p->r.height,
> + p->r.left, p->r.top);
> + }
>  }
>  
>  static void v4l_print_jpegcompression(const void *arg, bool write_only)
> @@ -1645,6 +1656,7 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>   struct v4l2_crop *p = arg;
>   struct v4l2_selection s = {
>   .type = p->type,
> + .rectangles = 0,

No need for this. In i

Re: [PATCH RFC 6/7] exynos-gsc: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> TODO: Add setting of default initial format.

So this patch can't be applied yet.

Other than that it looks good, but I won't ack it since it introduces a 
regression
as long as the TODO isn't addressed.

Regards,

Hans

> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 
> ---
>  drivers/media/platform/exynos-gsc/gsc-core.h |   12 ---
>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |  109 
> --
>  2 files changed, 16 insertions(+), 105 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h 
> b/drivers/media/platform/exynos-gsc/gsc-core.h
> index cc19bba..1afad32 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
> @@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, 
> int irq)
>   writel(cfg, dev->regs + GSC_IRQ);
>  }
>  
> -static inline void gsc_lock(struct vb2_queue *vq)
> -{
> - struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
> - mutex_lock(&ctx->gsc_dev->lock);
> -}
> -
> -static inline void gsc_unlock(struct vb2_queue *vq)
> -{
> - struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
> - mutex_unlock(&ctx->gsc_dev->lock);
> -}
> -
>  static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx)
>  {
>   unsigned long flags;
> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c 
> b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> index 40a73f7..4f5d6cb 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> @@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = {
>   .queue_setup = gsc_m2m_queue_setup,
>   .buf_prepare = gsc_m2m_buf_prepare,
>   .buf_queue   = gsc_m2m_buf_queue,
> - .wait_prepare= gsc_unlock,
> - .wait_finish = gsc_lock,
> + .wait_prepare= vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
>   .stop_streaming  = gsc_m2m_stop_streaming,
>   .start_streaming = gsc_m2m_start_streaming,
>  };
> @@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh,
>   return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
>  }
>  
> -static int gsc_m2m_expbuf(struct file *file, void *fh,
> - struct v4l2_exportbuffer *eb)
> -{
> - struct gsc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
> -}
> -
> -static int gsc_m2m_querybuf(struct file *file, void *fh,
> - struct v4l2_buffer *buf)
> -{
> - struct gsc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_qbuf(struct file *file, void *fh,
> -   struct v4l2_buffer *buf)
> -{
> - struct gsc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_dqbuf(struct file *file, void *fh,
> -struct v4l2_buffer *buf)
> -{
> - struct gsc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_streamon(struct file *file, void *fh,
> -enum v4l2_buf_type type)
> -{
> - struct gsc_ctx *ctx = fh_to_ctx(fh);
> -
> - /* The source and target color format need to be set */
> - if (V4L2_TYPE_IS_OUTPUT(type)) {
> - if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx))
> - return -EINVAL;
> - } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) {
> - return -EINVAL;
> - }
> -
> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int gsc_m2m_streamoff(struct file *file, void *fh,
> - enum v4l2_buf_type type)
> -{
> - struct gsc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */
>  static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b)
>  {
> @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = {
>   .vidioc_try_fmt_vid_out_mplane  = gsc_m2m_try_fmt_mplane,
>   .vidioc_s_fmt_vid_cap_mplane= gsc_m2m_s_fmt_mplane,
>   .vidioc_s_fmt_vid_out_mplane= gsc_m2m_s_fmt_mplane,
> - .vidioc_reqbufs = gsc_m2m_reqbufs,
> - .vidioc_expbuf  = gsc_m2m_expbuf,
> - .vidioc_querybuf= gsc_m2m_querybuf,
> - .vidioc_qbuf= gsc_m2m_qbuf,
> - .vidioc_dqbuf   = gsc_m2m_dqbuf,
> - .vidioc_streamon= gsc_m2m_streamon,
> - .vidioc_streamoff   = gsc_m2m_streamoff,
> +
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_querybuf= v4l2_m2m

Re: [PATCH RFC 7/7] s5p-g2d: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/s5p-g2d/g2d.c |  103 
> --
>  1 file changed, 11 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-g2d/g2d.c 
> b/drivers/media/platform/s5p-g2d/g2d.c
> index 553d87e..d59d546 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> @@ -139,7 +139,6 @@ static void g2d_buf_queue(struct vb2_buffer *vb)
>   v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -
>  static struct vb2_ops g2d_qops = {
>   .queue_setup= g2d_queue_setup,
>   .buf_prepare= g2d_buf_prepare,
> @@ -159,6 +158,7 @@ static int queue_init(void *priv, struct vb2_queue 
> *src_vq,
>   src_vq->mem_ops = &vb2_dma_contig_memops;
>   src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>   src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + src_vq->lock = &ctx->dev->mutex;
>  
>   ret = vb2_queue_init(src_vq);
>   if (ret)
> @@ -171,6 +171,7 @@ static int queue_init(void *priv, struct vb2_queue 
> *src_vq,
>   dst_vq->mem_ops = &vb2_dma_contig_memops;
>   dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>   dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + dst_vq->lock = &ctx->dev->mutex;
>  
>   return vb2_queue_init(dst_vq);
>  }
> @@ -263,6 +264,7 @@ static int g2d_open(struct file *file)
>   v4l2_fh_init(&ctx->fh, video_devdata(file));
>   file->private_data = &ctx->fh;
>   v4l2_fh_add(&ctx->fh);
> + ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>   g2d_setup_ctrls(ctx);
>  
> @@ -410,72 +412,6 @@ static int vidioc_s_fmt(struct file *file, void *prv, 
> struct v4l2_format *f)
>   return 0;
>  }
>  
> -static unsigned int g2d_poll(struct file *file, struct poll_table_struct 
> *wait)
> -{
> - struct g2d_ctx *ctx = fh2ctx(file->private_data);
> - struct g2d_dev *dev = ctx->dev;
> - unsigned int res;
> -
> - mutex_lock(&dev->mutex);
> - res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> - mutex_unlock(&dev->mutex);
> - return res;
> -}
> -
> -static int g2d_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> - struct g2d_ctx *ctx = fh2ctx(file->private_data);
> - struct g2d_dev *dev = ctx->dev;
> - int ret;
> -
> - if (mutex_lock_interruptible(&dev->mutex))
> - return -ERESTARTSYS;
> - ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> - mutex_unlock(&dev->mutex);
> - return ret;
> -}
> -
> -static int vidioc_reqbufs(struct file *file, void *priv,
> - struct v4l2_requestbuffers *reqbufs)
> -{
> - struct g2d_ctx *ctx = priv;
> - return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> - struct v4l2_buffer *buf)
> -{
> - struct g2d_ctx *ctx = priv;
> - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct g2d_ctx *ctx = priv;
> - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct g2d_ctx *ctx = priv;
> - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> - enum v4l2_buf_type type)
> -{
> - struct g2d_ctx *ctx = priv;
> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> - enum v4l2_buf_type type)
> -{
> - struct g2d_ctx *ctx = priv;
> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int vidioc_cropcap(struct file *file, void *priv,
>   struct v4l2_cropcap *cr)
>  {
> @@ -551,20 +487,6 @@ static int vidioc_s_crop(struct file *file, void *prv, 
> const struct v4l2_crop *c
>   return 0;
>  }
>  
> -static void g2d_lock(void *prv)
> -{
> - struct g2d_ctx *ctx = prv;
> - struct g2d_dev *dev = ctx->dev;
> - mutex_lock(&dev->mutex);
> -}
> -
> -static void g2d_unlock(void *prv)
> -{
> - struct g2d_ctx *ctx = prv;
> - struct g2d_dev *dev = ctx->dev;
> - mutex_unlock(&dev->mutex);
> -}
> -
>  static void job_abort(void *prv)
>  {
>   struct g2d_ctx *ctx = prv;
> @@ -653,9 +575,9 @@ static const struct v4l2_file_operations g2d_fops = {
>   .owner  = THIS_MODULE,
>   .open   = g2d_open,
>   .release= g2d_release,
> - .poll   = g2d_poll,
> + .poll   = v4l2_m2m_fop_poll,
>   .unlocked_ioctl = vi

Re: [PATCH RFC 5/7] mx2-emmaprp: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 
> ---
>  drivers/media/platform/mx2_emmaprp.c |  108 
> --
>  1 file changed, 11 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/media/platform/mx2_emmaprp.c 
> b/drivers/media/platform/mx2_emmaprp.c
> index c690435..a531862 100644
> --- a/drivers/media/platform/mx2_emmaprp.c
> +++ b/drivers/media/platform/mx2_emmaprp.c
> @@ -253,20 +253,6 @@ static void emmaprp_job_abort(void *priv)
>   v4l2_m2m_job_finish(pcdev->m2m_dev, ctx->m2m_ctx);
>  }
>  
> -static void emmaprp_lock(void *priv)
> -{
> - struct emmaprp_ctx *ctx = priv;
> - struct emmaprp_dev *pcdev = ctx->dev;
> - mutex_lock(&pcdev->dev_mutex);
> -}
> -
> -static void emmaprp_unlock(void *priv)
> -{
> - struct emmaprp_ctx *ctx = priv;
> - struct emmaprp_dev *pcdev = ctx->dev;
> - mutex_unlock(&pcdev->dev_mutex);
> -}
> -
>  static inline void emmaprp_dump_regs(struct emmaprp_dev *pcdev)
>  {
>   dprintk(pcdev,
> @@ -617,52 +603,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void 
> *priv,
>   return vidioc_s_fmt(priv, f);
>  }
>  
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -   struct v4l2_requestbuffers *reqbufs)
> -{
> - struct emmaprp_ctx *ctx = priv;
> -
> - return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -struct v4l2_buffer *buf)
> -{
> - struct emmaprp_ctx *ctx = priv;
> -
> - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct emmaprp_ctx *ctx = priv;
> -
> - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct emmaprp_ctx *ctx = priv;
> -
> - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> -enum v4l2_buf_type type)
> -{
> - struct emmaprp_ctx *ctx = priv;
> -
> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> - enum v4l2_buf_type type)
> -{
> - struct emmaprp_ctx *ctx = priv;
> -
> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static const struct v4l2_ioctl_ops emmaprp_ioctl_ops = {
>   .vidioc_querycap= vidioc_querycap,
>  
> @@ -676,14 +616,13 @@ static const struct v4l2_ioctl_ops emmaprp_ioctl_ops = {
>   .vidioc_try_fmt_vid_out = vidioc_try_fmt_vid_out,
>   .vidioc_s_fmt_vid_out   = vidioc_s_fmt_vid_out,
>  
> - .vidioc_reqbufs = vidioc_reqbufs,
> - .vidioc_querybuf= vidioc_querybuf,
> -
> - .vidioc_qbuf= vidioc_qbuf,
> - .vidioc_dqbuf   = vidioc_dqbuf,
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_querybuf= v4l2_m2m_ioctl_querybuf,
> + .vidioc_qbuf= v4l2_m2m_ioctl_qbuf,
> + .vidioc_dqbuf   = v4l2_m2m_ioctl_dqbuf,
>  
> - .vidioc_streamon= vidioc_streamon,
> - .vidioc_streamoff   = vidioc_streamoff,
> + .vidioc_streamon= v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff   = v4l2_m2m_ioctl_streamoff,
>  };
>  
>  
> @@ -767,6 +706,7 @@ static int queue_init(void *priv, struct vb2_queue 
> *src_vq,
>   src_vq->ops = &emmaprp_qops;
>   src_vq->mem_ops = &vb2_dma_contig_memops;
>   src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + src_vq->lock = &ctx->dev->dev_mutex;
>  
>   ret = vb2_queue_init(src_vq);
>   if (ret)
> @@ -779,6 +719,7 @@ static int queue_init(void *priv, struct vb2_queue 
> *src_vq,
>   dst_vq->ops = &emmaprp_qops;
>   dst_vq->mem_ops = &vb2_dma_contig_memops;
>   dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + dst_vq->lock = &ctx->dev->dev_mutex;
>  
>   return vb2_queue_init(dst_vq);
>  }
> @@ -812,6 +753,7 @@ static int emmaprp_open(struct file *file)
>   kfree(ctx);
>   return ret;
>   }
> + /* TODO: Assign fh->m2m_ctx, needs conversion to struct v4l2_fh first */

What's the point of adding this patch if it won't work yet?

Regards,

Hans

>  
>   clk_prepare_enable(pcdev->clk_emma_ipg);
>   clk_prepare_enable(pcdev->clk_emma_ahb);
> @@ -841,39 +783,13 @@ static int emmaprp_release(struct file *file)
>   return 0;
>  }
>  
> -static unsigned int emmaprp_poll(struct file *file,
> -  struct poll_table_struct *wait)
> -{
> - struct emmaprp_dev *pcdev = video_drvdata(file);
> - struct emmaprp_ctx *ctx = file->private_data;
> - u

Re: [PATCH RFC 4/7] s5p-jpeg: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 


Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c |  111 
> ---
>  1 file changed, 13 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 88c5beb..66f7519 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -322,6 +322,7 @@ static int s5p_jpeg_open(struct file *file)
>   ret = PTR_ERR(ctx->m2m_ctx);
>   goto error;
>   }
> + ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>   ctx->out_q.fmt = out_fmt;
>   ctx->cap_q.fmt = s5p_jpeg_find_format(ctx->mode, V4L2_PIX_FMT_YUYV);
> @@ -353,39 +354,13 @@ static int s5p_jpeg_release(struct file *file)
>   return 0;
>  }
>  
> -static unsigned int s5p_jpeg_poll(struct file *file,
> -  struct poll_table_struct *wait)
> -{
> - struct s5p_jpeg *jpeg = video_drvdata(file);
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
> - unsigned int res;
> -
> - mutex_lock(&jpeg->lock);
> - res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> - mutex_unlock(&jpeg->lock);
> - return res;
> -}
> -
> -static int s5p_jpeg_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> - struct s5p_jpeg *jpeg = video_drvdata(file);
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
> - int ret;
> -
> - if (mutex_lock_interruptible(&jpeg->lock))
> - return -ERESTARTSYS;
> - ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> - mutex_unlock(&jpeg->lock);
> - return ret;
> -}
> -
>  static const struct v4l2_file_operations s5p_jpeg_fops = {
>   .owner  = THIS_MODULE,
>   .open   = s5p_jpeg_open,
>   .release= s5p_jpeg_release,
> - .poll   = s5p_jpeg_poll,
> + .poll   = v4l2_m2m_fop_poll,
>   .unlocked_ioctl = video_ioctl2,
> - .mmap   = s5p_jpeg_mmap,
> + .mmap   = v4l2_m2m_fop_mmap,
>  };
>  
>  /*
> @@ -793,53 +768,6 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, 
> void *priv,
>   return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
>  }
>  
> -static int s5p_jpeg_reqbufs(struct file *file, void *priv,
> -   struct v4l2_requestbuffers *reqbufs)
> -{
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> - return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int s5p_jpeg_querybuf(struct file *file, void *priv,
> -struct v4l2_buffer *buf)
> -{
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int s5p_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int s5p_jpeg_dqbuf(struct file *file, void *priv,
> -   struct v4l2_buffer *buf)
> -{
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int s5p_jpeg_streamon(struct file *file, void *priv,
> -enum v4l2_buf_type type)
> -{
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int s5p_jpeg_streamoff(struct file *file, void *priv,
> - enum v4l2_buf_type type)
> -{
> - struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int s5p_jpeg_g_selection(struct file *file, void *priv,
>struct v4l2_selection *s)
>  {
> @@ -973,14 +901,13 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = 
> {
>   .vidioc_s_fmt_vid_cap   = s5p_jpeg_s_fmt_vid_cap,
>   .vidioc_s_fmt_vid_out   = s5p_jpeg_s_fmt_vid_out,
>  
> - .vidioc_reqbufs = s5p_jpeg_reqbufs,
> - .vidioc_querybuf= s5p_jpeg_querybuf,
> -
> - .vidioc_qbuf= s5p_jpeg_qbuf,
> - .vidioc_dqbuf   = s5p_jpeg_dqbuf,
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_querybuf= v4l2_m2m_ioctl_querybuf,
> + .vidioc_qbuf= v4l2_m2m_ioctl_qbuf,
> + .vidioc_dqbuf   = v4l2_m2m_ioctl_dqbuf,
>  
> - .vidioc_streamon= s5p_jpeg_streamon,
> - .vidioc_streamoff   = s5p_jpeg_streamoff,
> + .vidioc_streamon= v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff   = v4l2_m2m_ioctl_streamoff,
>  
> 

Re: [PATCH RFC 3/7] exynos4-is: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the FIMC mem-to-mem driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/exynos4-is/fimc-m2m.c |  119 
> +++---
>  1 file changed, 14 insertions(+), 105 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c 
> b/drivers/media/platform/exynos4-is/fimc-m2m.c
> index 8d33b68..71ee871 100644
> --- a/drivers/media/platform/exynos4-is/fimc-m2m.c
> +++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
> @@ -226,24 +226,12 @@ static void fimc_buf_queue(struct vb2_buffer *vb)
>   v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -static void fimc_lock(struct vb2_queue *vq)
> -{
> - struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
> - mutex_lock(&ctx->fimc_dev->lock);
> -}
> -
> -static void fimc_unlock(struct vb2_queue *vq)
> -{
> - struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
> - mutex_unlock(&ctx->fimc_dev->lock);
> -}
> -
>  static struct vb2_ops fimc_qops = {
>   .queue_setup = fimc_queue_setup,
>   .buf_prepare = fimc_buf_prepare,
>   .buf_queue   = fimc_buf_queue,
> - .wait_prepare= fimc_unlock,
> - .wait_finish = fimc_lock,
> + .wait_prepare= vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
>   .stop_streaming  = stop_streaming,
>   .start_streaming = start_streaming,
>  };
> @@ -410,56 +398,6 @@ static int fimc_m2m_s_fmt_mplane(struct file *file, void 
> *fh,
>   return 0;
>  }
>  
> -static int fimc_m2m_reqbufs(struct file *file, void *fh,
> - struct v4l2_requestbuffers *reqbufs)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int fimc_m2m_querybuf(struct file *file, void *fh,
> -  struct v4l2_buffer *buf)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int fimc_m2m_qbuf(struct file *file, void *fh,
> -  struct v4l2_buffer *buf)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int fimc_m2m_dqbuf(struct file *file, void *fh,
> -   struct v4l2_buffer *buf)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int fimc_m2m_expbuf(struct file *file, void *fh,
> - struct v4l2_exportbuffer *eb)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
> -}
> -
> -
> -static int fimc_m2m_streamon(struct file *file, void *fh,
> -  enum v4l2_buf_type type)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int fimc_m2m_streamoff(struct file *file, void *fh,
> - enum v4l2_buf_type type)
> -{
> - struct fimc_ctx *ctx = fh_to_ctx(fh);
> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int fimc_m2m_cropcap(struct file *file, void *fh,
>   struct v4l2_cropcap *cr)
>  {
> @@ -598,13 +536,13 @@ static const struct v4l2_ioctl_ops fimc_m2m_ioctl_ops = 
> {
>   .vidioc_try_fmt_vid_out_mplane  = fimc_m2m_try_fmt_mplane,
>   .vidioc_s_fmt_vid_cap_mplane= fimc_m2m_s_fmt_mplane,
>   .vidioc_s_fmt_vid_out_mplane= fimc_m2m_s_fmt_mplane,
> - .vidioc_reqbufs = fimc_m2m_reqbufs,
> - .vidioc_querybuf= fimc_m2m_querybuf,
> - .vidioc_qbuf= fimc_m2m_qbuf,
> - .vidioc_dqbuf   = fimc_m2m_dqbuf,
> - .vidioc_expbuf  = fimc_m2m_expbuf,
> - .vidioc_streamon= fimc_m2m_streamon,
> - .vidioc_streamoff   = fimc_m2m_streamoff,
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_querybuf= v4l2_m2m_ioctl_querybuf,
> + .vidioc_qbuf= v4l2_m2m_ioctl_qbuf,
> + .vidioc_dqbuf   = v4l2_m2m_ioctl_dqbuf,
> + .vidioc_expbuf  = v4l2_m2m_ioctl_expbuf,
> + .vidioc_streamon= v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff   = v4l2_m2m_ioctl_streamoff,
>   .vidioc_g_crop  = fimc_m2m_g_crop,
>   .vidioc_s_crop  = fimc_m2m_s_crop,
>   .vidioc_cropcap = fimc_m2m_cropcap
> @@ -624,6 +562,7 @@ static int queue_init(void *priv, struct vb2_queue 
> *src_vq,
>   src_vq->mem_ops = &vb2_dma_contig_memops;
>   src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>   src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTA

Re: [PATCH RFC 2/7] mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/mem2mem_testdev.c |   94 
> --
>  1 file changed, 11 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/media/platform/mem2mem_testdev.c 
> b/drivers/media/platform/mem2mem_testdev.c
> index 6a17676..73df44f 100644
> --- a/drivers/media/platform/mem2mem_testdev.c
> +++ b/drivers/media/platform/mem2mem_testdev.c
> @@ -359,21 +359,6 @@ static void job_abort(void *priv)
>   ctx->aborting = 1;
>  }
>  
> -static void m2mtest_lock(void *priv)
> -{
> - struct m2mtest_ctx *ctx = priv;
> - struct m2mtest_dev *dev = ctx->dev;
> - mutex_lock(&dev->dev_mutex);
> -}
> -
> -static void m2mtest_unlock(void *priv)
> -{
> - struct m2mtest_ctx *ctx = priv;
> - struct m2mtest_dev *dev = ctx->dev;
> - mutex_unlock(&dev->dev_mutex);
> -}
> -
> -
>  /* device_run() - prepares and starts the device
>   *
>   * This simulates all the immediate preparations required before starting
> @@ -648,52 +633,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void 
> *priv,
>   return ret;
>  }
>  
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -   struct v4l2_requestbuffers *reqbufs)
> -{
> - struct m2mtest_ctx *ctx = file2ctx(file);
> -
> - return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -struct v4l2_buffer *buf)
> -{
> - struct m2mtest_ctx *ctx = file2ctx(file);
> -
> - return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct m2mtest_ctx *ctx = file2ctx(file);
> -
> - return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer 
> *buf)
> -{
> - struct m2mtest_ctx *ctx = file2ctx(file);
> -
> - return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> -enum v4l2_buf_type type)
> -{
> - struct m2mtest_ctx *ctx = file2ctx(file);
> -
> - return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> - enum v4l2_buf_type type)
> -{
> - struct m2mtest_ctx *ctx = file2ctx(file);
> -
> - return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int m2mtest_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>   struct m2mtest_ctx *ctx =
> @@ -748,14 +687,14 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = {
>   .vidioc_try_fmt_vid_out = vidioc_try_fmt_vid_out,
>   .vidioc_s_fmt_vid_out   = vidioc_s_fmt_vid_out,
>  
> - .vidioc_reqbufs = vidioc_reqbufs,
> - .vidioc_querybuf= vidioc_querybuf,
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_querybuf= v4l2_m2m_ioctl_querybuf,
> + .vidioc_qbuf= v4l2_m2m_ioctl_qbuf,
> + .vidioc_dqbuf   = v4l2_m2m_ioctl_dqbuf,
>  
> - .vidioc_qbuf= vidioc_qbuf,
> - .vidioc_dqbuf   = vidioc_dqbuf,
> + .vidioc_streamon= v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff   = v4l2_m2m_ioctl_streamoff,
>  
> - .vidioc_streamon= vidioc_streamon,
> - .vidioc_streamoff   = vidioc_streamoff,
>   .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>   .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>  };
> @@ -821,24 +760,12 @@ static void m2mtest_buf_queue(struct vb2_buffer *vb)
>   v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -static void m2mtest_wait_prepare(struct vb2_queue *q)
> -{
> - struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
> - m2mtest_unlock(ctx);
> -}
> -
> -static void m2mtest_wait_finish(struct vb2_queue *q)
> -{
> - struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
> - m2mtest_lock(ctx);
> -}
> -
>  static struct vb2_ops m2mtest_qops = {
>   .queue_setup = m2mtest_queue_setup,
>   .buf_prepare = m2mtest_buf_prepare,
>   .buf_queue   = m2mtest_buf_queue,
> - .wait_prepare= m2mtest_wait_prepare,
> - .wait_finish = m2mtest_wait_finish,
> + .wait_prepare= vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
>  };
>  
>  static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue 
> *dst_vq)
> @@ -853,6 +780,7 @@ static int queue_init(void *priv, struct vb2_queue 
> *src_vq, struct vb2_queue *ds
>   src_vq->ops = &m2mtest_qops;
>   src_vq->mem_ops = &vb2_vmalloc_memops;
>   src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + src_vq->lock = &ctx->dev->dev_mutex

Re: [PATCH RFC 1/7] V4L: Add mem2mem ioctl and file operation helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> This patch adds ioctl helpers to the V4L2 mem-to-mem API, so we
> can avoid several ioctl handlers in the mem-to-mem video node
> drivers that are simply a pass-through to the v4l2_m2m_* calls.
> These helpers will only be useful for drivers that use same mutex
> for both OUTPUT and CAPTURE queue, which is the case for all
> currently in tree v4l2 m2m drivers.
> In order to use the helpers the driver are required to use
> struct v4l2_fh.

Looks good! I have one small comment below that you might want to address,
although it isn't blocking.

Acked-by: Hans Verkuil 

Regards,

Hans

> 
> Signed-off-by: Sylwester Nawrocki 
> Signed-off-by: Kyugmin Park 
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c |  110 
> 
>  include/media/v4l2-fh.h|4 ++
>  include/media/v4l2-mem2mem.h   |   22 +++
>  3 files changed, 136 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 7c43712..dddad5b 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -544,6 +544,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct 
> v4l2_m2m_ctx *m2m_ctx,
>  
>   if (m2m_ctx->m2m_dev->m2m_ops->unlock)
>   m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
> + else if (m2m_ctx->q_lock)
> + mutex_unlock(m2m_ctx->q_lock);
>  
>   if (list_empty(&src_q->done_list))
>   poll_wait(file, &src_q->done_wq, wait);
> @@ -552,6 +554,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct 
> v4l2_m2m_ctx *m2m_ctx,
>  
>   if (m2m_ctx->m2m_dev->m2m_ops->lock)
>   m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
> + else if (m2m_ctx->q_lock)
> + mutex_lock(m2m_ctx->q_lock);
>  
>   spin_lock_irqsave(&src_q->done_lock, flags);
>   if (!list_empty(&src_q->done_list))
> @@ -679,6 +683,13 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct 
> v4l2_m2m_dev *m2m_dev,
>  
>   if (ret)
>   goto err;
> + /*
> +  * If both queues use same mutex assign it as the common buffer
> +  * queues lock to the m2m context. This lock is used in the
> +  * v4l2_m2m_ioctl_* helpers.
> +  */
> + if (out_q_ctx->q.lock == cap_q_ctx->q.lock)
> + m2m_ctx->q_lock = out_q_ctx->q.lock;
>  
>   return m2m_ctx;
>  err:
> @@ -726,3 +737,102 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, 
> struct vb2_buffer *vb)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
>  
> +/* Videobuf2 ioctl helpers */
> +
> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
> + struct v4l2_requestbuffers *rb)
> +{
> + struct v4l2_fh *fh = file->private_data;

I prefer an empty line after the variable declaration. Ditto below.

> + return v4l2_m2m_reqbufs(file, fh->m2m_ctx, rb);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_reqbufs);
> +
> +int v4l2_m2m_ioctl_querybuf(struct file *file, void *priv,
> + struct v4l2_buffer *buf)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + return v4l2_m2m_querybuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_querybuf);
> +
> +int v4l2_m2m_ioctl_qbuf(struct file *file, void *priv,
> + struct v4l2_buffer *buf)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_qbuf);
> +
> +int v4l2_m2m_ioctl_dqbuf(struct file *file, void *priv,
> + struct v4l2_buffer *buf)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + return v4l2_m2m_dqbuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_dqbuf);
> +
> +int v4l2_m2m_ioctl_expbuf(struct file *file, void *priv,
> + struct v4l2_exportbuffer *eb)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + return v4l2_m2m_expbuf(file, fh->m2m_ctx, eb);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_expbuf);
> +
> +int v4l2_m2m_ioctl_streamon(struct file *file, void *priv,
> + enum v4l2_buf_type type)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + return v4l2_m2m_streamon(file, fh->m2m_ctx, type);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamon);
> +
> +int v4l2_m2m_ioctl_streamoff(struct file *file, void *priv,
> + enum v4l2_buf_type type)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + return v4l2_m2m_streamoff(file, fh->m2m_ctx, type);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamoff);
> +
> +/*
> + * v4l2_file_operations helpers. It is assumed here same lock is used
> + * for the output and the capture buffer queue.
> + */
> +
> +int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct v4l2_fh *fh = file->private_data;
> + struc

Re: [PATCH v3 3/4] [media] exynos-scaler: Add m2m functionality for the SCALER driver

[Shaik Ameer Basha]

Hi Hans,

Thanks for pointing it out.


On Mon, Sep 30, 2013 at 1:38 PM, Hans Verkuil  wrote:
> Hi Shaik,
>
> I have a few questions regarding the selection part...
>
> On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
>> This patch adds the Makefile and memory to memory (m2m) interface
>> functionality for the SCALER driver.
>>
>> Signed-off-by: Shaik Ameer Basha 
>> ---
>>  drivers/media/platform/Kconfig|8 +
>>  drivers/media/platform/Makefile   |1 +
>>  drivers/media/platform/exynos-scaler/Makefile |3 +
>>  drivers/media/platform/exynos-scaler/scaler-m2m.c |  781 
>> +
>>  4 files changed, 793 insertions(+)
>>  create mode 100644 drivers/media/platform/exynos-scaler/Makefile
>>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c
>>
>


[...]


>> +
>> +static int scaler_m2m_s_selection(struct file *file, void *fh,
>> + struct v4l2_selection *s)
>> +{
>> + struct scaler_frame *frame;
>> + struct scaler_ctx *ctx = fh_to_ctx(fh);
>> + struct v4l2_crop cr;
>> + struct scaler_variant *variant = ctx->scaler_dev->variant;
>> + int ret;
>> +
>> + cr.type = s->type;
>> + cr.c = s->r;
>> +
>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
>> + return -EINVAL;
>> +
>> + ret = scaler_try_crop(ctx, &cr);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (s->flags & V4L2_SEL_FLAG_LE &&
>> + !is_rectangle_enclosed(&cr.c, &s->r))
>> + return -ERANGE;
>> +
>> + if (s->flags & V4L2_SEL_FLAG_GE &&
>> + !is_rectangle_enclosed(&s->r, &cr.c))
>> + return -ERANGE;
>> +
>> + s->r = cr.c;
>> +
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> + case V4L2_SEL_TGT_COMPOSE:
>> + frame = &ctx->s_frame;
>> + break;
>> +
>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>> + case V4L2_SEL_TGT_CROP:
>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>> + frame = &ctx->d_frame;
>> + break;
>
> Similar problems as with g_selection above. Tomasz mentioned to me that the 
> selection
> API is not implemented correctly in m2m Samsung drivers. It looks like this 
> code is
> copied-and-pasted from other drivers, so it seems he was right.

Sorry, after going through the documentation, I have to agree with you...
As you mentioned, this part of the code was copied while implementing
the G-Scaler driver :)

I will change the above implementation for M2M devices (GScaler and
SCALER) as below,
I will only allow all V4L2_SEL_TGT_COMPOSE_* target requests if
's->type' is equal to "V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE".
and all V4L2_SEL_TGT_CROP_* target requests if 's->type' is equal to
"V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE".

I hope with the above two checkings taken in to care, there should not
be any issues with using selection APIs here.

Thanks,
Shaik Ameer Basha

>
> The selection API for m2m devices will be discussed during the upcoming V4L2 
> mini-summit
> since the API may actually need some adjustments to have it work the way it 
> should.
>
> As requested above, if you can explain the exact functionality you are trying 
> to
> implement here, then I can look over this code carefully and see how it 
> should be done.
>
> Thanks!
>
> Hans
>
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Initial scan files for PT

[Oliver Schinagl]

Hello Nuno,

On 28-09-13 13:07, Nuno Gonçalves wrote:

Here goes the initial scan files for Portugal DVB-T. Please commit them.
Thank you for these files, I decided to merge them all together into a 
pt-All. See github/git.linuxtv.com for details.




Portugal is schedule to migrate from a SFN to a MFN, but for now this
is how it is.
Whenever this change happens, please feel free to submit these changes 
to us and we will commit them. Do mind formatting next time (whitespaces).


Files in attach and also the hg diff:
Actually, dtv-scan-files are now stored in git, you can use git 
format-patch/send-email or use github's pull request.


Thank you,

Oliver



diff -r 3ee111da5b3a util/scan/dvb-t/pt-Azores-Faial
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-Azores-Faial Sat Sep 28 11:49:03 2013 +0100
@@ -0,0 +1,5 @@
+# PT, Azores, Faial
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 69800 8MHz  2/3 NONEQAM64   8k  1/4 NONE
+
diff -r 3ee111da5b3a util/scan/dvb-t/pt-Azores-Pico
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-Azores-Pico Sat Sep 28 11:49:03 2013 +0100
@@ -0,0 +1,5 @@
+# PT, Azores, Pico
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 75400 8MHz  2/3 NONEQAM64   8k  1/4 NONE
+
diff -r 3ee111da5b3a util/scan/dvb-t/pt-Azores-SaoJorge
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-Azores-SaoJorge Sat Sep 28 11:49:03 2013 +0100
@@ -0,0 +1,5 @@
+# PT, Azores, Sao Jorge
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 68200 8MHz  2/3 NONEQAM64   8k  1/4 NONE
+
diff -r 3ee111da5b3a util/scan/dvb-t/pt-Azores-SaoMiguel-Graciosa
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-Azores-SaoMiguel-Graciosa Sat Sep 28 11:49:03
2013 +0100
@@ -0,0 +1,5 @@
+# PT, Azores, Sao Miguel and Graciosa
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 69000 8MHz  2/3 NONEQAM64   8k  1/4 NONE
+
diff -r 3ee111da5b3a util/scan/dvb-t/pt-Azores-Terceira-SMaria-Flores-Corvo
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-Azores-Terceira-SMaria-Flores-Corvo Sat Sep
28 11:49:03 2013 +0100
@@ -0,0 +1,5 @@
+# PT, Azores, Terceira and S. Maria and Graciosa
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 73800 8MHz  2/3 NONEQAM64   8k  1/4 NONE
+
diff -r 3ee111da5b3a util/scan/dvb-t/pt-Madeira
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-Madeira Sat Sep 28 11:49:03 2013 +0100
@@ -0,0 +1,5 @@
+# PT, Madeira
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 73800 8MHz  2/3 NONEQAM64   8k  1/4 NONE
+
diff -r 3ee111da5b3a util/scan/dvb-t/pt-mainland
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/util/scan/dvb-t/pt-mainland Sat Sep 28 11:49:03 2013 +0100
@@ -0,0 +1,7 @@
+# PT, mainland
+# Generated from http://tdt-portugal.blogspot.pt/
+# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
+T 64200 8MHz  2/3 NONEQAM64   8k  1/4 NONE # Monte da Virgem
+T 67400 8MHz  2/3 NONEQAM64   8k  1/4 NONE # Lousa (Trevim)
+T 69800 8MHz  2/3 NONEQAM64   8k  1/4 NONE # Montejunto
+T 75400 8MHz  2/3 NONEQAM64   8k  1/4 NONE # Mainland SFN


Regards,
Nuno Goncalves



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

Re: [PATCH 6/8] ALSA: ppc: keywest: Don't use i2c_client->driver

[Takashi Iwai]

At Sun, 29 Sep 2013 10:51:04 +0200,
Lars-Peter Clausen wrote:
> 
> The 'driver' field of the i2c_client struct is redundant and is going to be
> removed. Use 'to_i2c_driver(client->dev.driver)' instead to get direct
> access to the i2c_driver struct.
> 
> Signed-off-by: Lars-Peter Clausen 

Acked-by: Takashi Iwai 


thanks,

Takashi

> ---
>  sound/ppc/keywest.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
> index 01aecc2..0d1c27e 100644
> --- a/sound/ppc/keywest.c
> +++ b/sound/ppc/keywest.c
> @@ -65,7 +65,7 @@ static int keywest_attach_adapter(struct i2c_adapter 
> *adapter)
>* already bound. If not it means binding failed, and then there
>* is no point in keeping the device instantiated.
>*/
> - if (!keywest_ctx->client->driver) {
> + if (!keywest_ctx->client->dev.driver) {
>   i2c_unregister_device(keywest_ctx->client);
>   keywest_ctx->client = NULL;
>   return -ENODEV;
> @@ -76,7 +76,7 @@ static int keywest_attach_adapter(struct i2c_adapter 
> *adapter)
>* This is safe because i2c-core holds the core_lock mutex for us.
>*/
>   list_add_tail(&keywest_ctx->client->detected,
> -   &keywest_ctx->client->driver->clients);
> +   &to_i2c_driver(keywest_ctx->client->dev.driver)->clients);
>   return 0;
>  }
>  
> -- 
> 1.8.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iram pool not available for MX27

[Chris Ruehl]

On Monday, September 30, 2013 05:08 PM, Chris Ruehl wrote:

Hi Philipp,

On Monday, September 30, 2013 04:30 PM, Philipp Zabel wrote:

Hi Chris,

Am Montag, den 30.09.2013, 13:40 +0800 schrieb Chris Ruehl:

Hi Phillipp,

hope things doing OK.

I recently update to the 3.12-rc kernel and hit this problem below.

[ 3.377790] coda coda-imx27.0: iram pool not available
[ 3.383363] coda: probe of coda-imx27.0 failed with error -12

I read your comments of the patch-set using platform data rather then
hard coded addresses to get
the ocram from a SoC.

I checked the imx27.dtsi for the iram (coda: coda@..) definition and
compare with the former hard coded address and size it matches.

My .config also has the CONFIG_OF set.

Any Idea what's go wrong?

do you have the mmio-sram driver enabled (CONFIG_SRAM=y)?

regards
Philipp



No, I didn't,  and I found out that my device is not yet ported to use 
"Device Tree Support"


for the moment I will quick add the CONFIG_SRAM  and see what happen,
but on the long term I move my code (clone of mach-mx27ads.c)
to DTS which makes absolute sense when I see how nice that code works.

Thanks for the reply!
Chris

CONFIG_SRAM not solve my problem, I must port the code to Device Tree 
Support and call the of_ functions to make the iram config available.


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

Re: iram pool not available for MX27

[Chris Ruehl]

Hi Philipp,

On Monday, September 30, 2013 04:30 PM, Philipp Zabel wrote:

Hi Chris,

Am Montag, den 30.09.2013, 13:40 +0800 schrieb Chris Ruehl:

Hi Phillipp,

hope things doing OK.

I recently update to the 3.12-rc kernel and hit this problem below.

[ 3.377790] coda coda-imx27.0: iram pool not available
[ 3.383363] coda: probe of coda-imx27.0 failed with error -12

I read your comments of the patch-set using platform data rather then
hard coded addresses to get
the ocram from a SoC.

I checked the imx27.dtsi for the iram (coda: coda@..) definition and
compare with the former hard coded address and size it matches.

My .config also has the CONFIG_OF set.

Any Idea what's go wrong?

do you have the mmio-sram driver enabled (CONFIG_SRAM=y)?

regards
Philipp



No, I didn't,  and I found out that my device is not yet ported to use 
"Device Tree Support"


for the moment I will quick add the CONFIG_SRAM  and see what happen,
but on the long term I move my code (clone of mach-mx27ads.c)
to DTS which makes absolute sense when I see how nice that code works.

Thanks for the reply!
Chris

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

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Srinivas KANDAGATLA]

On 27/09/13 14:57, Mauro Carvalho Chehab wrote:
> Em Fri, 27 Sep 2013 14:26:12 +0100
> Srinivas KANDAGATLA  escreveu:
> 
>> On 27/09/13 12:34, Mark Rutland wrote:
>>
> + - rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> +   the rx pins are wired up.
>>> I'm unsure on this. What if the device has multiple receivers that can
>>> be independently configured? What if it supports something other than
>>> "infrared" or "uhf"? What if a device can only be wired up as
>>> "infrared"? 
>>>
>>> I'm not sure how generic these are, though we should certainly encourage
>>> bindings that can be described this way to be described in the same way.
>>>
> + - tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> +   the tx pins are wired up.
>>> I have similar concerns here to those for the rx-mode property.
>>>
>> Initially rx-mode and tx-mode sounded like more generic properties
>> that's the reason I ended up in this route. But after this discussion it
>> looks like its not really generic enough to cater all the use cases.
>>
>> It make sense for me to perfix "st," for these properties in the st-rc
>> driver rather than considering them as generic properties.
> 
> Well, for sure the direction (TX, RX, both) is a generic property.
> 
> I'd say that the level 1 protocol (IR, UHF, Bluetooth, ...) is also a
> generic property. Most remotes are IR, but there are some that are
> bluetooth, and your hardware is using UHF.
Yes these are generic.

> 
> Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via
> the RC subsystem. So, another L1 protocol would be "hdmi-cec".
> 
Ok.
> Yet, it seems unlikely that the very same remote controller IP would use
> a different protocol for RX and TX, while sharing the same registers.

ST IRB block has one IR processor which has both TX and RX support and
one UHF Processor which has RX support only. However the register map
for all these support is in single IRB IP block.

So the driver can configure the IP as TX in "infrared" and RX in "uhf".
This is supported in ST IRB IP.

This case can not be represented in a single device tree node with
l1-protocol and direction properties.

IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol
properties will give more flexibility.

What do you think?

> 
> So, for example, a hardware with "hdmi-cec" and "infrared" will actually
> have two remote controller devices. Eventually, the "infrared" being
> just RX, while "hdmi-cec" being bi-directional.
> 
> So, IMHO, this could be mapped as "l1_protocol" ("infrared", "uhf", ...)
> and another one "direction" ("rx", "tx", "bi-directional").
> 

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

Re: iram pool not available for MX27

[Philipp Zabel]

Hi Chris,

Am Montag, den 30.09.2013, 13:40 +0800 schrieb Chris Ruehl:
> Hi Phillipp,
> 
> hope things doing OK.
> 
> I recently update to the 3.12-rc kernel and hit this problem below.
> 
> [ 3.377790] coda coda-imx27.0: iram pool not available
> [ 3.383363] coda: probe of coda-imx27.0 failed with error -12
> 
> I read your comments of the patch-set using platform data rather then 
> hard coded addresses to get
> the ocram from a SoC.
> 
> I checked the imx27.dtsi for the iram (coda: coda@..) definition and 
> compare with the former hard coded address and size it matches.
> 
> My .config also has the CONFIG_OF set.
> 
> Any Idea what's go wrong?

do you have the mmio-sram driver enabled (CONFIG_SRAM=y)?

regards
Philipp

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

Re: [PATCH v8] s5k5baf: add camera sensor driver

[Mark Rutland]

> >> diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt 
> >> b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
> >> new file mode 100644
> >> index 000..7704a1e
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt
> >> @@ -0,0 +1,58 @@
> >> +Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor with embedded SoC ISP
> >> +
> >> +
> >> +Required properties:
> >> +
> >> +- compatible : "samsung,s5k5baf";
> >> +- reg: I2C slave address of the sensor;
> >> +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V);
> >> +- vddreg-supply  : regulator input power supply 1.8V (1.7V to 
> >> 1.9V)
> >> +   or 2.8V (2.6V to 3.0);
> >> +- vddio-supply   : I/O power supply 1.8V (1.65V to 1.95V)
> >> +   or 2.8V (2.5V to 3.1V);
> >> +- stbyn-gpios: GPIO connected to STDBYN pin;
> >> +- rstn-gpios : GPIO connected to RSTN pin;
> >> +- clocks : the sensor's master clock specifier (from the common
> >> +   clock bindings);
> >> +- clock-names: must be "mclk";
> > I'd reword this slightly:
> >
> > - clocks: clock-specifiers (per the common clock bindings) for the
> >   clocks described in clock-names
> OK
> > - clock-names: should include "mclk" for the sensor's master clock
> IMHO it suggests there could be more than one clock, is it OK?

In general I'd prefer things to be described in this way so as to not
describe any requirements on the order of clocks in the binding (and to
suggest that clocks should be requested by name in code).

Future hardware variants could have multiple clocks that we need to add
to bindings, or it's possible we miss some clocks in the initial version
of a binding. In either case it's easier to extend the binding and code
if clocks are described by name and requested by name.

> >
> >> +
> >> +Optional properties:
> >> +
> >> +- clock-frequency : the frequency at which the "mclk" clock should be
> >> +   configured to operate, in Hz; if this property is not
> >> +   specified default 24 MHz value will be used.
> >> +
> >> +The device node should contain one 'port' child node with one child 
> >> 'endpoint'
> >> +node, according to the bindings defined in 
> >> Documentation/devicetree/bindings/
> >> +media/video-interfaces.txt. The following are properties specific to those
> >> +nodes.
> >> +
> >> +endpoint node
> >> +-
> >> +
> >> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
> >> +  video-interfaces.txt. This sensor doesn't support data lane remapping
> >> +  and physical lane indexes in subsequent elements of the array should
> >> +  have consecutive values.
> > Do these need to start at 1, or may they have any initial value?
> After re-checking I have found some inconsistency in the specs,
> regarding lanes.
> Final conclusion is that sensor supports only one lane without re-mapping.
> I would then change description to:
> 
> - data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
>   video-interfaces.txt. If present it should be <1> - the device supports 
> only one data lane
>   without re-mapping.

OK. That sounds reasonable to me.

> 
> 
> >> +
> >> +Example:
> >> +
> >> +s5k5bafx@2d {
> >> +   compatible = "samsung,s5k5baf";
> >> +   reg = <0x2d>;
> >> +   vdda-supply = <&cam_io_en_reg>;
> >> +   vddreg-supply = <&vt_core_15v_reg>;
> >> +   vddio-supply = <&vtcam_reg>;
> >> +   stbyn-gpios = <&gpl2 0 1>;
> >> +   rstn-gpios = <&gpl2 1 1>;
> >> +   clock-names = "mclk";
> >> +   clocks = <&clock_cam 0>;
> >> +   clock-frequency = <2400>;
> >> +
> >> +   port {
> >> +   s5k5bafx_ep: endpoint {
> >> +   remote-endpoint = <&csis1_ep>;
> >> +   data-lanes = <1>;
> >> +   };
> >> +   };
> >> +};
> > Otherwise, I think the binding looks fine.
> >
> > I took a quick skim over the driver and I have a few other comments.
> >
> > [...]
> >
> >> +enum s5k5baf_gpio_id {
> >> +   STBY,
> >> +   RST,
> >> +   GPIO_NUM,
> > I'd expect this to be NUM_GPIOS or MAX_GPIOS, as GPIO_NUM sounds like
> > the index for a GPIO, rather than how many GPIOs there are.
> OK
> >
> >> +};
> >> +
> >> +#define PAD_CIS 0
> >> +#define PAD_OUT 1
> >> +#define CIS_PAD_NUM 1
> >> +#define ISP_PAD_NUM 2
> > Similarly here, I think NUM_*S or MAX_*S is preferable.
> OK
> >
> > [...]
> >
> >> +static void s5k5baf_hw_patch(struct s5k5baf *state)
> >> +{
> >> +   static const u16 nseq_patch[] = {
> >> +   NSEQ(0x1668,
> >> +   0xb5fe, 0x0007, 0x683c, 0x687e, 0x1da5, 0x88a0, 0x2800, 
> >> 0xd00b,
> >> +   0x88a8, 0x2800, 0xd008, 0x8820, 0x8829, 0x4288, 0xd301, 
> >> 0x1a40,
> >> +   0xe000, 0x1a08, 0x9001, 0xe001, 0x2019, 0x9001, 0x4916, 
>

Re: mcam-core.c:undefined reference to `vb2_dma_sg_memops'

[Fengguang Wu]

On Mon, Sep 30, 2013 at 09:09:13AM +0200, Geert Uytterhoeven wrote:
> Hi Fengguang,
> 
> On Mon, Sep 30, 2013 at 5:05 AM, Fengguang Wu  wrote:
> > FYI, kernel build failed on
> >
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> > master
> > head:   15c03dd4859ab16f9212238f29dd315654aa94f6
> > commit: 866f321339988293a5bb3ec6634c2c9d8396bf54 Revert "staging/solo6x10: 
> > depend on CONFIG_FONTS"
> > date:   3 months ago
> > config: x86_64-randconfig-c5-0930 (attached as .config)
> >
> > All error/warnings:
> >
> >drivers/built-in.o: In function `mcam_v4l_open':
> >>> mcam-core.c:(.text+0x3bf73a): undefined reference to `vb2_dma_sg_memops'
> 
> The referenced commit above is completely unrelated to this failure, as
> both CONFIG_SOLO6X10=m and CONFIG_VIDEOBUF2_DMA_SG=m,
> while this is about a missing symbol in builtin code.

You are probably right.. However I tried manually reproduce this error
and find that 866f3213 is the first bad commit (for whatever reason),
so I decided to email the report out.

> However, there's something wrong with the VIDEO_CAFE_CCIC dependencies.
> Untested gmail-white-space-damaged patch below (so your trick of emailing 
> random
> people to obtain a solution worked ;-)

Yeah, indeed! :)

Thanks,
Fengguang

> >From 8a53ff3c33cfaa8641c9ba3e16bc5b0a35c74842 Mon Sep 17 00:00:00 2001
> From: Geert Uytterhoeven 
> Date: Mon, 30 Sep 2013 09:03:20 +0200
> Subject: [PATCH] [media] VIDEO_CAFE_CCIC should select VIDEOBUF2_DMA_SG
> 
> If VIDEO_CAFE_CCIC=y, but VIDEOBUF2_DMA_SG=m:
> 
> drivers/built-in.o: In function `mcam_v4l_open':
> >> mcam-core.c:(.text+0x3bf73a): undefined reference to `vb2_dma_sg_memops'
> 
> Reported-by: Fengguang Wu 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/media/platform/marvell-ccic/Kconfig |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/marvell-ccic/Kconfig
> b/drivers/media/platform/marvell-ccic/Kconfig
> index bf739e3..ec4c771 100644
> --- a/drivers/media/platform/marvell-ccic/Kconfig
> +++ b/drivers/media/platform/marvell-ccic/Kconfig
> @@ -4,6 +4,7 @@ config VIDEO_CAFE_CCIC
>   select VIDEO_OV7670
>   select VIDEOBUF2_VMALLOC
>   select VIDEOBUF2_DMA_CONTIG
> + select VIDEOBUF2_DMA_SG
>   ---help---
>This is a video4linux2 driver for the Marvell 88ALP01 integrated
>CMOS camera controller.  This is the controller found on first-
> -- 
> 1.7.9.5
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/4] [media] exynos-scaler: Add m2m functionality for the SCALER driver

[Hans Verkuil]

Hi Shaik,

I have a few questions regarding the selection part...

On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
> This patch adds the Makefile and memory to memory (m2m) interface
> functionality for the SCALER driver.
> 
> Signed-off-by: Shaik Ameer Basha 
> ---
>  drivers/media/platform/Kconfig|8 +
>  drivers/media/platform/Makefile   |1 +
>  drivers/media/platform/exynos-scaler/Makefile |3 +
>  drivers/media/platform/exynos-scaler/scaler-m2m.c |  781 
> +
>  4 files changed, 793 insertions(+)
>  create mode 100644 drivers/media/platform/exynos-scaler/Makefile
>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c
> 

...

> +
> +static int scaler_m2m_g_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct scaler_frame *frame;
> + struct scaler_ctx *ctx = fh_to_ctx(fh);
> +
> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
> + return -EINVAL;
> +
> + frame = ctx_get_frame(ctx, s->type);
> + if (IS_ERR(frame))
> + return PTR_ERR(frame);
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_CROP_DEFAULT:

This can't be right: depending on s->type you either support compose or crop,
I'm pretty sure you are not supporting both composing and cropping for both
capture and output directions.

What exactly is the functionality you attempt to implement here?

I'm CC-ing Tomasz as well as I discussed the selection API for m2m devices with
him during the LPC.

> + s->r.left = 0;
> + s->r.top = 0;
> + s->r.width = frame->f_width;
> + s->r.height = frame->f_height;
> + return 0;
> +
> + case V4L2_SEL_TGT_COMPOSE:
> + case V4L2_SEL_TGT_CROP:

Ditto.

> + s->r.left = frame->crop.left;
> + s->r.top = frame->crop.top;
> + s->r.width = frame->crop.width;
> + s->r.height = frame->crop.height;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int scaler_m2m_s_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct scaler_frame *frame;
> + struct scaler_ctx *ctx = fh_to_ctx(fh);
> + struct v4l2_crop cr;
> + struct scaler_variant *variant = ctx->scaler_dev->variant;
> + int ret;
> +
> + cr.type = s->type;
> + cr.c = s->r;
> +
> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
> + return -EINVAL;
> +
> + ret = scaler_try_crop(ctx, &cr);
> + if (ret < 0)
> + return ret;
> +
> + if (s->flags & V4L2_SEL_FLAG_LE &&
> + !is_rectangle_enclosed(&cr.c, &s->r))
> + return -ERANGE;
> +
> + if (s->flags & V4L2_SEL_FLAG_GE &&
> + !is_rectangle_enclosed(&s->r, &cr.c))
> + return -ERANGE;
> +
> + s->r = cr.c;
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE:
> + frame = &ctx->s_frame;
> + break;
> +
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + frame = &ctx->d_frame;
> + break;

Similar problems as with g_selection above. Tomasz mentioned to me that the 
selection
API is not implemented correctly in m2m Samsung drivers. It looks like this 
code is
copied-and-pasted from other drivers, so it seems he was right.

The selection API for m2m devices will be discussed during the upcoming V4L2 
mini-summit
since the API may actually need some adjustments to have it work the way it 
should.

As requested above, if you can explain the exact functionality you are trying to
implement here, then I can look over this code carefully and see how it should 
be done.

Thanks!

Hans

> +
> + default:
> + return -EINVAL;
> + }
> +
> + /* Check to see if scaling ratio is within supported range */
> + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> + ret = scaler_check_scaler_ratio(variant, cr.c.width,
> + cr.c.height, ctx->d_frame.crop.width,
> + ctx->d_frame.crop.height,
> + ctx->ctrls_scaler.rotate->val);
> + } else {
> + ret = scaler_check_scaler_ratio(variant,
> + ctx->s_frame.crop.width,
> + ctx->s_frame.crop.height, cr.c.width,
> + cr.c.height, ctx->ctrls_scaler.rotate->val);
> + }
> +
> + if (ret < 0) {
> + scaler_dbg(ctx->scaler_dev, "Out of scaler range");
> + 

Re: [PATCH] SOLO6x10: don't do DMA from stack in solo_dma_vin_region().

[Hans Verkuil]

Ismael,

Can you Ack these four patches?

Regards,

Hans

On 09/12/2013 02:25 PM, Krzysztof Hałasa wrote:
> Signed-off-by: Krzysztof Hałasa 
> 
> diff --git a/drivers/staging/media/solo6x10/solo6x10-disp.c 
> b/drivers/staging/media/solo6x10/solo6x10-disp.c
> index 32d9953..884512e 100644
> --- a/drivers/staging/media/solo6x10/solo6x10-disp.c
> +++ b/drivers/staging/media/solo6x10/solo6x10-disp.c
> @@ -176,18 +176,26 @@ static void solo_vout_config(struct solo_dev *solo_dev)
>  static int solo_dma_vin_region(struct solo_dev *solo_dev, u32 off,
>  u16 val, int reg_size)
>  {
> - u16 buf[64];
> - int i;
> - int ret = 0;
> + u16 *buf;
> + const int n = 64, size = n * sizeof(*buf);
> + int i, ret = 0;
> +
> + if (!(buf = kmalloc(size, GFP_KERNEL)))
> + return -ENOMEM;
>  
> - for (i = 0; i < sizeof(buf) >> 1; i++)
> + for (i = 0; i < n; i++)
>   buf[i] = cpu_to_le16(val);
>  
> - for (i = 0; i < reg_size; i += sizeof(buf))
> - ret |= solo_p2m_dma(solo_dev, 1, buf,
> - SOLO_MOTION_EXT_ADDR(solo_dev) + off + i,
> - sizeof(buf), 0, 0);
> + for (i = 0; i < reg_size; i += size) {
> + ret = solo_p2m_dma(solo_dev, 1, buf,
> +SOLO_MOTION_EXT_ADDR(solo_dev) + off + i,
> +size, 0, 0);
> +
> + if (ret)
> + break;
> + }
>  
> + kfree(buf);
>   return ret;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

RE: [PATCH 1/5] ARM: Kconfig: Move if ARCH_S3C64XX statement to mach-s3c64xx/Kconfig

[Kukjin Kim]

Tomasz Figa wrote:
> 
> All other platforms have this condition checked inside their own Kconfig
> files, so for consistency this patch makes it this way for mach-s3c64xx
> as well.
> 
> Signed-off-by: Tomasz Figa 
> ---
>  arch/arm/Kconfig  | 2 --
>  arch/arm/mach-s3c64xx/Kconfig | 4 
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b766dad..dc51f8a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -995,9 +995,7 @@ source "arch/arm/mach-sti/Kconfig"
> 
>  source "arch/arm/mach-s3c24xx/Kconfig"
> 
> -if ARCH_S3C64XX
>  source "arch/arm/mach-s3c64xx/Kconfig"
> -endif
> 
>  source "arch/arm/mach-s5p64x0/Kconfig"
> 
> diff --git a/arch/arm/mach-s3c64xx/Kconfig b/arch/arm/mach-s3c64xx/Kconfig
> index bd14e3a..0e23910 100644
> --- a/arch/arm/mach-s3c64xx/Kconfig
> +++ b/arch/arm/mach-s3c64xx/Kconfig
> @@ -3,6 +3,8 @@
>  #
>  # Licensed under GPLv2
> 
> +if ARCH_S3C64XX
> +
>  # temporary until we can eliminate all drivers using it.
>  config PLAT_S3C64XX
>   bool
> @@ -322,3 +324,5 @@ config MACH_S3C64XX_DT
> board.
> Note: This is under development and not all peripherals can be
> supported with this machine file.
> +
> +endif
> --
> 1.8.3.2

Looks good to me, applied 1 to 5 patches into cleanup.

Thanks,
Kukjin

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

Re: mcam-core.c:undefined reference to `vb2_dma_sg_memops'

[Geert Uytterhoeven]

Hi Fengguang,

On Mon, Sep 30, 2013 at 5:05 AM, Fengguang Wu  wrote:
> FYI, kernel build failed on
>
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> master
> head:   15c03dd4859ab16f9212238f29dd315654aa94f6
> commit: 866f321339988293a5bb3ec6634c2c9d8396bf54 Revert "staging/solo6x10: 
> depend on CONFIG_FONTS"
> date:   3 months ago
> config: x86_64-randconfig-c5-0930 (attached as .config)
>
> All error/warnings:
>
>drivers/built-in.o: In function `mcam_v4l_open':
>>> mcam-core.c:(.text+0x3bf73a): undefined reference to `vb2_dma_sg_memops'

The referenced commit above is completely unrelated to this failure, as
both CONFIG_SOLO6X10=m and CONFIG_VIDEOBUF2_DMA_SG=m,
while this is about a missing symbol in builtin code.

However, there's something wrong with the VIDEO_CAFE_CCIC dependencies.
Untested gmail-white-space-damaged patch below (so your trick of emailing random
people to obtain a solution worked ;-)

>From 8a53ff3c33cfaa8641c9ba3e16bc5b0a35c74842 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Mon, 30 Sep 2013 09:03:20 +0200
Subject: [PATCH] [media] VIDEO_CAFE_CCIC should select VIDEOBUF2_DMA_SG

If VIDEO_CAFE_CCIC=y, but VIDEOBUF2_DMA_SG=m:

drivers/built-in.o: In function `mcam_v4l_open':
>> mcam-core.c:(.text+0x3bf73a): undefined reference to `vb2_dma_sg_memops'

Reported-by: Fengguang Wu 
Signed-off-by: Geert Uytterhoeven 
---
 drivers/media/platform/marvell-ccic/Kconfig |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/marvell-ccic/Kconfig
b/drivers/media/platform/marvell-ccic/Kconfig
index bf739e3..ec4c771 100644
--- a/drivers/media/platform/marvell-ccic/Kconfig
+++ b/drivers/media/platform/marvell-ccic/Kconfig
@@ -4,6 +4,7 @@ config VIDEO_CAFE_CCIC
  select VIDEO_OV7670
  select VIDEOBUF2_VMALLOC
  select VIDEOBUF2_DMA_CONTIG
+ select VIDEOBUF2_DMA_SG
  ---help---
   This is a video4linux2 driver for the Marvell 88ALP01 integrated
   CMOS camera controller.  This is the controller found on first-
-- 
1.7.9.5

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html