[FWD] PROBLEM: there exists a wrong return value of function mantis_dma_init()

2015-08-10 Thread Linus Torvalds
It does seem like the error handling for mantis_dma_init() is insane..

   Linus

On Sun, Aug 9, 2015 at 5:12 PM, RUC_Soft_Sec zy900...@163.com wrote:
 Summary:
 there exists a wrong return value of function mantis_dma_init().It's a
 theoretical problem. we use static analysis method to detect this bug.
 Bug Description:

In function mantis_dma_init() at
 drivers/media/pci/mantis/mantis_dma.c:131, the call to
 mantis_alloc_buffers() in line 136 may return a negative error code, and
 thus function mantis_dma_init() will return the value of variable err. And,
 the function mantis_dma_init() will return 0 at last when it runs well.
 However, when the call to mantis_alloc_buffers() in line 136 return a
 negative error code, the value of err is 0. So the function
 mantis_dma_init() will return 0 to its caller functions when it runs error
 because of the failing call to mantis_alloc_buffers(), leading to a wrong
 return value of function mantis_dma_init().
 The related code snippets in mantis_dma_init() is as following.
 mantis_dma_init @@ drivers/media/pci/mantis/mantis_dma.c:131
  131int mantis_dma_init(struct mantis_pci *mantis)
  132{
  133int err = 0;
  134
  135dprintk(MANTIS_DEBUG, 1, Mantis DMA init);
  136if (mantis_alloc_buffers(mantis)  0) {
  137dprintk(MANTIS_ERROR, 1, Error allocating DMA buffer);
  138
  139/* Stop RISC Engine */
  140mmwrite(0, MANTIS_DMA_CTL);
  141
  142goto err;
  143}
  144
  145return 0;
  146err:
  147return err;
  148}

 Moreover, in the caller function of mantis_dma_init() the return value will
 be checked if it is a negative number. Now, the return value of
 mantis_dma_init() is always 0 and the check is useless.
 The related code snippets in mantis_core_init() is as following.
  137int mantis_core_init(struct mantis_pci *mantis)
  138{
 ...
  163err = mantis_dma_init(mantis);
  164if (err  0) {
  165dprintk(verbose, MANTIS_ERROR, 1, Mantis DMA init
 failed);
  166return err;
  167}
 ...
  179return 0;
  180}

 Kernel version:
 3.19.1




--
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 RFC] DT support for omap4-iss

2015-08-10 Thread Michael Allwright
Hi All,

The following PRELIMINARY patch adds DT support to the OMAP4 ISS. It
also fixes some problems a have found along the way. It is tightly
modelled after the omap3-isp media platform driver. This patch is a
work in progress as I would like feedback. It contains debugging
messages that need to be removed, as well as disgusting abuses of the
C language as required (i.e. clk_core_fake and clk_fake).

I'm working in the latest stable mainline which as far as this patch
is concerned is compatible with media tree master. I have had this
omap4-iss working on my hardware in the 3.17 kernel, however I'm
currently having the following issue in 4.1.4 stable when I start to
stream:

[  141.612609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!

Any feedback regarding this issue would really be appreciated. After
resolving this issue, we still need to do a proper implementation
using syscon and also to find a solution regarding where to put the
iss_set_constraints function. I have to give up for the next couple of
weeks as I need to submit a conference paper, which I'm going to use
my 3.17 implementation for. The following is an example of how the ISS
would be instantiated in the top level device tree:

iss_csi21_pins: pinmux_iss_csi21_pins {
pinctrl-single,pins = 
OMAP4_IOPAD(0x0a0, PIN_INPUT | MUX_MODE0)/*
csi21_dx0.csi21_dx0 */
OMAP4_IOPAD(0x0a2, PIN_INPUT | MUX_MODE0)/*
csi21_dy0.csi21_dy0 */
OMAP4_IOPAD(0x0a4, PIN_INPUT | MUX_MODE0)/*
csi21_dx1.csi21_dx1 */
OMAP4_IOPAD(0x0a6, PIN_INPUT | MUX_MODE0)/*
csi21_dy1.csi21_dy1 */
OMAP4_IOPAD(0x0a8, PIN_INPUT | MUX_MODE0)/*
csi21_dx2.csi21_dx2 */
OMAP4_IOPAD(0x0aa, PIN_INPUT | MUX_MODE0)/*
csi21_dy2.csi21_dy2 */
;
};

iss {
status = ok;

pinctrl-names = default;
pinctrl-0 = iss_csi21_pins;

ports {
port@0 {
reg = 0;
csi2a_ep: endpoint {
remote-endpoint = ov5640_1_cam_ep;
clock-lanes = 1;
data-lanes = 2;
crc = 0;
lane-polarities = 0 0;
};
};
};
};

and for the connected camera:

ov5640_1_camera: ov5640@3c {
compatible = omnivision,ov5640;
status = ok;
reg = 0x3c;

pwdn-gpios = ov5640_1_gpio 5 GPIO_ACTIVE_HIGH;
reset-gpios = ov5640_1_gpio 6 GPIO_ACTIVE_LOW;

avdd-supply = switch_ov5640_1_avdd;
dvdd-supply = switch_ov5640_1_dvdd;

clocks = ov5640_1_camera_clk;

port {
ov5640_1_cam_ep: endpoint {
clock-lanes = 0;
data-lanes = 1;
remote-endpoint = csi2a_ep;
};
};
};

From 919995491fb34cf7e2bd8a331c47e45cad677ce6 Mon Sep 17 00:00:00 2001
From: Michael Allwright allse...@gmail.com
Date: Mon, 10 Aug 2015 16:55:57 +0200
Subject: [PATCH] omap4-iss: Add device support (WIP)

---
 arch/arm/boot/dts/omap4.dtsi|  33 +++
 drivers/staging/media/omap4iss/iss.c| 419 +---
 drivers/staging/media/omap4iss/iss.h|  11 +
 drivers/staging/media/omap4iss/iss_csi2.c   |   4 +-
 drivers/staging/media/omap4iss/iss_csiphy.c |  16 +-
 drivers/staging/media/omap4iss/iss_video.c  |   6 +-
 include/media/omap4iss.h|  18 +-
 7 files changed, 393 insertions(+), 114 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index f884d6a..bd37437 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -923,6 +923,39 @@
 status = disabled;
 };

+iss: iss@5200 {
+compatible = ti,omap4-iss;
+reg = 0x5200 0x100, /* top */
+  0x52001000 0x170, /* csi2_a_regs1 */
+  0x52001170 0x020, /* camerarx_core1 */
+  0x52001400 0x170, /* csi2_b_regs1 */
+  0x52001570 0x020, /* camerarx_core2 */
+  0x52002000 0x200, /* bte */
+  0x5201 0x0a0, /* isp_sys1 */
+  0x52010400 0x400, /* isp_resizer */
+  0x52010800 0x800, /* isp_ipipe */
+  0x52011000 0x200, /* isp_isif */
+  0x52011200 0x080; /* isp_ipipeif */
+reg-names = top,
+csi2_a_regs1,
+camerarx_core1,
+csi2_b_regs1,
+camerarx_core2,
+bte,
+isp_sys1,
+isp_resizer,
+isp_ipipe,
+isp_isif,
+isp_ipipeif;
+status = ok;
+ti,hwmods = iss;
+interrupts = GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH;
+clocks = ducati_clk_mux_ck, iss_ctrlclk;
+clock-names = iss_fck, iss_ctrlclk;
+dmas = sdma 9, sdma 10, sdma 12, sdma 13;
+dma-names = 1, 2, 3, 4;
+};
+
 

[PATCH] [media] mantis: Fix error handling in mantis_dma_init()

2015-08-10 Thread Fabio Estevam
Current code assigns 0 to variable 'err', which makes mantis_dma_init()
to return success even if mantis_alloc_buffers() fails.

Fix it by checking the return value from mantis_alloc_buffers() and
propagating it in the case of error. 

Reported-by: RUC_Soft_Sec zy900...@163.com
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
 drivers/media/pci/mantis/mantis_dma.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/mantis/mantis_dma.c 
b/drivers/media/pci/mantis/mantis_dma.c
index 1d59c7e..87990ec 100644
--- a/drivers/media/pci/mantis/mantis_dma.c
+++ b/drivers/media/pci/mantis/mantis_dma.c
@@ -130,10 +130,11 @@ err:
 
 int mantis_dma_init(struct mantis_pci *mantis)
 {
-   int err = 0;
+   int err;
 
dprintk(MANTIS_DEBUG, 1, Mantis DMA init);
-   if (mantis_alloc_buffers(mantis)  0) {
+   err = mantis_alloc_buffers(mantis);
+   if (err  0) {
dprintk(MANTIS_ERROR, 1, Error allocating DMA buffer);
 
/* Stop RISC Engine */
-- 
1.9.1

--
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: VIMC: API proposal, configuring the topology through user space

2015-08-10 Thread Helen Fornazier
Hi, thanks for your reviews

On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Helen!

 On 08/08/2015 03:55 AM, Helen Fornazier wrote:
 Hi!

 I've made a first sketch about the API of the vimc driver (virtual
 media controller) to configure the topology through user space.
 As first suggested by Laurent Pinchart, it is based on configfs.

 I would like to know you opinion about it, if you have any suggestion
 to improve it, otherwise I'll start its implementation as soon as
 possible.
 This API may change with the MC changes and I may see other possible
 configurations as I implementing it but here is the first idea of how
 the API will look like.

 vimc project link: https://github.com/helen-fornazier/opw-staging/
 For more information: http://kernelnewbies.org/LaurentPinchart

 /***
 The API:
 /

 In short, a topology like this one: http://goo.gl/Y7eUfu
 Would look like this filesystem tree: https://goo.gl/tCZPTg
 Txt version of the filesystem tree: https://goo.gl/42KX8Y

 * The /configfs/vimc subsystem

 I haven't checked the latest vimc driver, but doesn't it support
 multiple instances, just like vivid? I would certainly recommend that.

Yes, it does support


 And if there are multiple instances, then each instance gets its own
 entry in configfs: /configs/vimc0, /configs/vimc1, etc.

You are right, I'll add those


 The vimc driver registers a subsystem in the configfs with the
 following contents:
  ls /configfs/vimc
 build_topology status
 The build_topology attribute file will be used to tell the driver the
 configuration is done and it can build the topology internally
  echo -n anything here  /configfs/vimc/build_topology
 Reading from the status attribute can have 3 different classes of outputs
 1) deployed: the current configured tree is built
 2) undeployed: no errors, the user has modified the configfs tree thus
 the topology was undeployed
 3) error error_message: the topology configuration is wrong

 I don't see the status file in the filesystem tree links above.

Sorry, I forgot to add


 I actually wonder if we can't use build_topology for that: reading it
 will just return the status.

Yes we can, I was just wondering what should be the name of the file,
as getting the status from reading the build_topology file doesn't
seem much intuitive


 What happens if it is deployed and you want to 'undeploy' it? Instead of
 writing anything to build_topology it might be useful to write a real
 command to it. E.g. 'deploy', 'destroy'.

 What happens when you make changes while a topology is deployed? Should
 such changes be rejected until the usecount of the driver goes to 0, or
 will it only be rejected when you try to deploy the new configuration?

I was thinking that if the user try changing the topology, or it will
fail (because the device instance is in use) or it will undeploy the
old topology (if the device is not in use). Then a 'destroy' command
won't be useful, the user can just unload the driver when it won't be
used anymore,


 * Creating an entity:
 Use mkdir in the /configfs/vimc to create an entity representation, e.g.:
  mkdir /configfs/vimc/sensor_a
 The attribute files will be created by the driver through configfs:
  ls /configfs/vimc/sensor_a
 name role
 Configure the name that will appear to the /dev/media0 and what this
 node do (debayer, scaler, capture, input, generic)
  echo -n Sensor A  /configfs/vimc/sensor_a/name
  echo -n sensor  /configfs/vimc/sensor_a/role

 * Creating a pad:
 Use mkdir inside an entity's folder, the attribute called direction
 will be automatically created in the process, for example:
  mkdir /configfs/vimc/sensor_a/pad_0
  ls /configfs/vimc/sensor_a/pad_0
 direction
  echo -n source  /configfs/vimc/sensor_a/pad_0/direction
 The direction attribute can be source or sink

 Hmm. Aren't the pads+direction dictated by the entity role? E.g. a sensor
 will only have one pad which is always a source. I think setting the role
 is what creates the pad directories + direction files.

I thought that we could add as many pads that we want, having a scaler
with two or more sink pads (for example) in the same format that
scales the frames coming from any sink pad and send it to its source
pads, no?
Maybe it is better if we don't let this choice.
I need to check if I can modify the configfs dynamically, I mean, if
the user writes debayer to the role file, I need to create at least
one folder (or file) to allow the user to configure the link flag
related to its source pad, but if in the future we have another entity
role (lets say new_entity) that has more then one source pad, and
the user writes debayer in the role and then new_entity, we will
need to erase the folder created by the debayer to create two new
folders, I am still not sure if I can do that.



 * Creating a link between pads in two 

WARNING: CPU: 1 PID: 813 at kernel/module.c:291 module_assert_mutex_or_preempt+0x49/0x90()

2015-08-10 Thread poma

[ cut here ]
WARNING: CPU: 1 PID: 813 at kernel/module.c:291 
module_assert_mutex_or_preempt+0x49/0x90()
Modules linked in: mxl5007t af9013 ... dvb_usb_af9015(+) ... dvb_usb_v2 
dvb_core rc_core ...
CPU: 1 PID: 813 Comm: systemd-udevd Not tainted 
4.2.0-0.rc6.git0.1.fc24.x86_64+debug #1
...
Call Trace:
 [81868d8e] dump_stack+0x4c/0x65
 [810ab406] warn_slowpath_common+0x86/0xc0
 [a057d0b0] ? af9013_read_ucblocks+0x20/0x20 [af9013]
 [a057d0b0] ? af9013_read_ucblocks+0x20/0x20 [af9013]
 [810ab53a] warn_slowpath_null+0x1a/0x20
 [81150529] module_assert_mutex_or_preempt+0x49/0x90
 [81150822] __module_address+0x32/0x150
 [a057d0b0] ? af9013_read_ucblocks+0x20/0x20 [af9013]
 [a057d0b0] ? af9013_read_ucblocks+0x20/0x20 [af9013]
 [81150956] __module_text_address+0x16/0x70
 [a057d0b0] ? af9013_read_ucblocks+0x20/0x20 [af9013]
 [a057d0b0] ? af9013_read_ucblocks+0x20/0x20 [af9013]
 [81150f19] symbol_put_addr+0x29/0x40
 [a04b77ad] dvb_frontend_detach+0x7d/0x90 [dvb_core]
 [a04cdfd5] dvb_usbv2_probe+0xc85/0x11a0 [dvb_usb_v2]
 [a05607c4] af9015_probe+0x84/0xf0 [dvb_usb_af9015]
 [8161c03b] usb_probe_interface+0x1bb/0x2e0
 [81579f26] driver_probe_device+0x1f6/0x450
 [8157a214] __driver_attach+0x94/0xa0
 [8157a180] ? driver_probe_device+0x450/0x450
 [815778f3] bus_for_each_dev+0x73/0xc0
 [815796fe] driver_attach+0x1e/0x20
 [8157922e] bus_add_driver+0x1ee/0x280
 [8157b0a0] driver_register+0x60/0xe0
 [8161a87d] usb_register_driver+0xad/0x160
 [a0567000] ? 0xa0567000
 [a056701e] af9015_usb_driver_init+0x1e/0x1000 [dvb_usb_af9015]
 [81002123] do_one_initcall+0xb3/0x200
 [8124ac65] ? kmem_cache_alloc_trace+0x355/0x380
 [81867c37] ? do_init_module+0x28/0x1e9
 [81867c6f] do_init_module+0x60/0x1e9
 [81154167] load_module+0x21f7/0x28d0
 [8114f600] ? m_show+0x1b0/0x1b0
 [81026d79] ? sched_clock+0x9/0x10
 [810e6ddc] ? local_clock+0x1c/0x20
 [811549b8] SyS_init_module+0x178/0x1c0
 [8187282e] entry_SYSCALL_64_fastpath+0x12/0x76
---[ end trace 31a9dd90d4f559f5 ]---


Ref.
https://bugzilla.redhat.com/show_bug.cgi?id=1252167
https://bugzilla.kernel.org/show_bug.cgi?id=102631

--
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: dvb_usb_af9015: command failed=1 _ kernel = 4.1.x

2015-08-10 Thread poma
On 31.07.2015 17:55, Olli Salonen wrote:
 Hi Poma,
 
 Might I suggest reading points 9, 10 and 11 from the following document:
 https://www.kernel.org/doc/Documentation/SubmittingPatches
 
 Especially point 11 needs to be taken care of in order for your patch to be
 merged.
 
 Cheers,
 -olli
 

This is why top posting is so bad:

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
  

Furthermore, to fix this issue - AF9015 DVB-T USB2.0 stick brokenness - is the 
responsibility of developers.
I am here only proven tester.

I hope we understand each other, and this problem will be resolved in good 
faith.


--
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] Staging: media: davinci_vpfe: Fix over 80 characters coding style issue

2015-08-10 Thread Junsu Shin
This is a patch to the dm365_ipipe.c that fixes over 80 characters warning 
detected.

Signed-off-by: Junsu Shin jjun...@gmail.com
---
 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 1bbb90c..a474adf 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1536,8 +1536,9 @@ ipipe_get_format(struct v4l2_subdev *sd, struct 
v4l2_subdev_pad_config *cfg,
  * @fse: pointer to v4l2_subdev_frame_size_enum structure.
  */
 static int
-ipipe_enum_frame_size(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config 
*cfg,
- struct v4l2_subdev_frame_size_enum *fse)
+ipipe_enum_frame_size(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_frame_size_enum *fse)
 {
struct vpfe_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
struct v4l2_mbus_framefmt format;
-- 
1.9.1

--
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: dvb_usb_af9015: command failed=1 _ kernel = 4.1.x

2015-08-10 Thread Antti Palosaari

On 08/11/2015 12:46 AM, poma wrote:

Furthermore, to fix this issue - AF9015 DVB-T USB2.0 stick brokenness - is the 
responsibility of developers.
I am here only proven tester.

I hope we understand each other, and this problem will be resolved in good 
faith.


Your patches are implemented wrong.

When I added mxl5007t support to that driver it was DigitalNow TinyTwin 
v2 I had. The rest mxl5007t dual devices using reference design IDs went 
to same due to reason driver detects used tuner. My device is still 
working fine, which means your device has different wiring. As 2nd tuner 
attach fails it means there is communication loss to tuner. Which means 
tuner is most likely hold in a reset attach time or there is some I2C 
gating which prevents communication.


Patches you sent will introduce another issue. For dual tuner 
configuration there could be antenna wired from tuner chip to another. 
After that patch you will lose antenna signal from 2nd tuner on cases 
where tuner antenna wire is loop through master tuner to slave.


So fix it correctly. Find out reason there is communication loss to 2nd 
tuner on attach time. I cannot do much as I simply don't have such 
hardware. And I really do not care to take any responsibility when that 
kind of issues happens - it is not my job to bough every single device 
from the market in able to test and fix every hardware combination.



Antti

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


Re: [RFC PATCH v2 4/5] media: videobuf2: Define vb2_buf_type and vb2_memory

2015-08-10 Thread Junghak Sung


Hi Hans,

On 08/10/2015 05:22 PM, Hans Verkuil wrote:

On 07/31/2015 10:44 AM, Junghak Sung wrote:

Define enum vb2_buf_type and enum vb2_memory for videobuf2-core. This
change requires translation functions that could covert v4l2-core stuffs
to videobuf2-core stuffs in videobuf2-v4l2.c file.
The v4l2-specific member variables(e.g. type, memory) remains in
struct vb2_queue for backward compatibility and performance of type translation.

Signed-off-by: Junghak Sung jh1009.s...@samsung.com
Signed-off-by: Geunyoung Kim nenggun@samsung.com
Acked-by: Seung-Woo Kim sw0312@samsung.com
Acked-by: Inki Dae inki@samsung.com
---
  drivers/media/v4l2-core/videobuf2-core.c |  139 +++-
  drivers/media/v4l2-core/videobuf2-v4l2.c |  209 --
  include/media/videobuf2-core.h   |   99 +++---
  include/media/videobuf2-v4l2.h   |   12 +-
  4 files changed, 299 insertions(+), 160 deletions(-)



snip


diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 85527e9..22dd19c 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -30,8 +30,46 @@
  #include media/v4l2-common.h
  #include media/videobuf2-v4l2.h

+#define CREATE_TRACE_POINTS
  #include trace/events/v4l2.h

+static const enum vb2_buf_type _tbl_buf_type[] = {
+   [V4L2_BUF_TYPE_VIDEO_CAPTURE]   = VB2_BUF_TYPE_VIDEO_CAPTURE,
+   [V4L2_BUF_TYPE_VIDEO_OUTPUT]= VB2_BUF_TYPE_VIDEO_OUTPUT,
+   [V4L2_BUF_TYPE_VIDEO_OVERLAY]   = VB2_BUF_TYPE_VIDEO_OVERLAY,
+   [V4L2_BUF_TYPE_VBI_CAPTURE] = VB2_BUF_TYPE_VBI_CAPTURE,
+   [V4L2_BUF_TYPE_VBI_OUTPUT]  = VB2_BUF_TYPE_VBI_OUTPUT,
+   [V4L2_BUF_TYPE_SLICED_VBI_CAPTURE]  = 
VB2_BUF_TYPE_SLICED_VBI_CAPTURE,
+   [V4L2_BUF_TYPE_SLICED_VBI_OUTPUT]   = 
VB2_BUF_TYPE_SLICED_VBI_OUTPUT,
+   [V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY]= 
VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY,
+   [V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE]= 
VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+   [V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE] = 
VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+   [V4L2_BUF_TYPE_SDR_CAPTURE] = VB2_BUF_TYPE_SDR_CAPTURE,
+   [V4L2_BUF_TYPE_PRIVATE] = VB2_BUF_TYPE_PRIVATE,
+};
+
+static const enum vb2_memory _tbl_memory[] = {
+   [V4L2_MEMORY_MMAP]  = VB2_MEMORY_MMAP,
+   [V4L2_MEMORY_USERPTR]   = VB2_MEMORY_USERPTR,
+   [V4L2_MEMORY_DMABUF]= VB2_MEMORY_DMABUF,
+};
+
+#define to_vb2_buf_type(type)  \
+({ \
+   enum vb2_buf_type ret = 0;  \
+   if( type  0  type  ARRAY_SIZE(_tbl_buf_type) )\
+   ret = (_tbl_buf_type[type]);\
+   ret;\
+})
+
+#define to_vb2_memory(memory)  \
+({ \
+   enum vb2_memory ret = 0;\
+   if( memory  0  memory  ARRAY_SIZE(_tbl_memory) )  \
+   ret = (_tbl_memory[memory]);\
+   ret;\
+})
+


snip


diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index dc405da..871fcc6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -15,9 +15,47 @@
  #include linux/mm_types.h
  #include linux/mutex.h
  #include linux/poll.h
-#include linux/videodev2.h
  #include linux/dma-buf.h

+#define VB2_MAX_FRAME   32
+#define VB2_MAX_PLANES   8
+
+enum vb2_buf_type {
+   VB2_BUF_TYPE_UNKNOWN= 0,
+   VB2_BUF_TYPE_VIDEO_CAPTURE  = 1,
+   VB2_BUF_TYPE_VIDEO_OUTPUT   = 2,
+   VB2_BUF_TYPE_VIDEO_OVERLAY  = 3,
+   VB2_BUF_TYPE_VBI_CAPTURE= 4,
+   VB2_BUF_TYPE_VBI_OUTPUT = 5,
+   VB2_BUF_TYPE_SLICED_VBI_CAPTURE = 6,
+   VB2_BUF_TYPE_SLICED_VBI_OUTPUT  = 7,
+   VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY   = 8,
+   VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE   = 9,
+   VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE= 10,
+   VB2_BUF_TYPE_SDR_CAPTURE= 11,
+   VB2_BUF_TYPE_DVB_CAPTURE= 12,
+   VB2_BUF_TYPE_PRIVATE= 0x80,
+};
+
+enum vb2_memory {
+   VB2_MEMORY_UNKNOWN  = 0,
+   VB2_MEMORY_MMAP = 1,
+   VB2_MEMORY_USERPTR  = 2,
+   VB2_MEMORY_DMABUF   = 4,
+};
+
+#define VB2_TYPE_IS_MULTIPLANAR(type)  \
+   ((type) == VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\
+|| (type) == VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+
+#define VB2_TYPE_IS_OUTPUT(type)   \
+   ((type) == 

cron job: media_tree daily build: OK

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

Results of the daily build of media_tree:

date:   Tue Aug 11 04:00:25 CEST 2015
git branch: test
git hash:   4dc102b2f53d63207fa12a6ad49c7b6448bc3301
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0-51-ga53cea2
smatch version: 0.4.1-3153-g7d56ab3
host hardware:  x86_64
host os:4.0.0-3.slh.1-amd64

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-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.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.1.10-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-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-rc1-i686: 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.1.10-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
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-rc1-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: 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: [PATCHv3 12/13] DocBook: fix S_FREQUENCY = G_FREQUENCY

2015-08-10 Thread Antti Palosaari

On 08/10/2015 12:41 PM, Hans Verkuil wrote:

On 07/31/2015 04:10 AM, Antti Palosaari wrote:

It is VIDIOC_G_FREQUENCY which does not use type to identify tuner,
not VIDIOC_S_FREQUENCY. VIDIOC_S_FREQUENCY uses both tuner and type
fields. One of these V4L API weirdness...


Actually, that's not what this is about. It's about whether g/s_frequency 
gets/sets
the frequency for the tuner or the modulator. That has nothing to do with the 
tuner
and type fields. The problem described here in the spec is a problem for both G 
and
S_FREQUENCY.


ah, now I think I see it. There is no tuner type for FM radio modulator 
- but instead it is capability of tuner type V4L2_TUNER_RADIO. As both 
radio receiver and transmitter has same tuner type it is not possible to 
identify it using tuner type...


regards
Antti

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


Re: [RFC PATCH v2 0/5] Refactoring Videobuf2 for common use

2015-08-10 Thread Junghak Sung


Hi, Hans and Mauro.

First of all, thank you for your reviews.

On 08/10/2015 08:44 PM, Hans Verkuil wrote:

On 08/10/2015 12:49 PM, Mauro Carvalho Chehab wrote:

Em Mon, 10 Aug 2015 12:11:39 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:


On 08/10/2015 11:32 AM, Mauro Carvalho Chehab wrote:

Em Mon, 10 Aug 2015 10:31:03 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:


Hi Jungsak,

On 07/31/2015 10:44 AM, Junghak Sung wrote:

Hello everybody,

This is the 2nd round for refactoring Videobuf2(a.k.a VB2).
The purpose of this patch series is to separate existing VB2 framework
into core part and V4L2 specific part. So that not only V4L2 but also other
frameworks can use them to manage buffer and utilize queue.

Why do we try to make the VB2 framework to be common?

As you may know, current DVB framework uses ringbuffer mechanism to demux
MPEG-2 TS data and pass it to userspace. However, this mechanism requires
extra memory copy because DVB framework provides only read() system call for
application - read() system call copies the kernel data to user-space buffer.
So if we can use VB2 framework which supports streaming I/O and buffer
sharing mechanism, then we could enhance existing DVB framework by removing
the extra memory copy - with VB2 framework, application can access the kernel
data directly through mmap system call.

We have a plan for this work as follows:
1. Separate existing VB2 framework into three parts - VB2 common, VB2-v4l2.
Of course, this change will not affect other v4l2-based
device drivers. This patch series corresponds to this step.

2. Add and implement new APIs for DVB streaming I/O.
We can remove unnecessary memory copy between kernel-space and user-space
by using these new APIs. However, we leaves legacy interfaces as-is
for backward compatibility.

This patch series is the first step for it.
The previous version of this patch series can be found at [1].

[1] RFC PATCH v1 - http://www.spinics.net/lists/linux-media/msg90688.html


This v2 looks much better, but, as per my comment to patch 1/5, it needs a bit
more work before I can do a really good review. I think things will be much
clearer once patch 3 shows the code moving from core.c/h to v4l2.c/h instead
of the other way around. That shouldn't be too difficult.


Hans,

I suggested Junkhak to do that. On his previous patchset, he did what
you're suggestiong, e. g moving things from vb2-core into vb2-v4l2, and
that resulted on patches big enough to not be catched by vger.


Actually, that wasn't the reason why the patches became so big. I just
reorganized the patch series as I suggested above (pretty easy to do) and
the size of patch 3 went down.


Considering only size of the patch, it is better to move from vb2-v4l2 
to vb2-core as this patch. Because the file size of videobuf2-v4l2.c 
(56k) is bigger that videobuf2-core.c (50k). :P




Ah, ok.


Also, IMHO, it is cleared this way, as we can see what parts of VB2 will
actually be shared, as there are lots of things that won't be shared:
overlay, userptr, multiplanar.


That's why I prefer to see what moves *out* from the core.

To be honest, it depends on what your preference is.


Yeah. I actually prefer to see what will be shared, because the
non-shared code won't have changes (except for minor kABI things),
while the shared code will be more affected :)



As Mauro said, I focus on what code can be shared. In other words,
what code is NOT dependent on V4L2 and what code is really necessary for 
vb2-core even though it is depend on V4l2. And then, those codes

are moved from vb2-v4l2 to vb2-core.


Junghak, just leave the patch as-is. However, for v3 you should run
checkpatch.pl over the diff since it complained about various things.


There are two things here:

1) on patches that just move things around, we should not
run checkpatch, as otherwise it would be a nightmare for
review. Ok, as we're doing a remanufacturing, it is a good
idea to run checkpatch at the end and see what should be fixed
(or do it before), but this is out of the scope of the manufacturing.
I can do that myself when applying the series.


It was actually complaining about new code.

Hans



2) For all other patches that are adding/changing the code, then
Junghak should run checkpatch and fix (most) stuff complained there.


Ok. for v3, I will run checkpatch for added/changed code.



Regards,
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


--
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] Media graph flow for an hybrid device as discussed at the media workshop

2015-08-10 Thread Hans Verkuil
Hi Mauro,

On 08/08/2015 01:33 PM, Mauro Carvalho Chehab wrote:
 During the discussions at the Media Workshop, we came with some dot files that
 would describe a hybrid PC-consumer TV stick with radio, analog video, analog
 TV and digital TV on it.
 
 I consolidated all the dot files we've worked there, and added the
 connectors for RF, S-Video and Composite.
 
 The dot file and the corresponding picture is at:
   
 http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.dot
   
 http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.png
 
 As my plan is to start working on some real driver to produce such graph,
 please validate if the entities, interfaces, data links and interface links
 are correct, and if the namespace nomenclature is ok, or if I miss something.

This looks OK to me, except for one small detail: I wouldn't use the name
Source entities for connectors. Instead use Connector entities since
such entities correspond to actual real connectors on a backplane. A proper
source entity would be a sensor or test pattern generator. Which actually
can occur with the em28xx since it's used in webcams as well.

And a really, really small detail: in the legend the 'interface link' is an
arrow, but it should be a line, since there is no direction. The graph itself
is fine.

As you mentioned on irc, the v4l-subdevX nodes won't be created for this device
since all the configuration happens via the standard interfaces.

But if they were to be created, then they would appear where they are in this
example.

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 PATCH v2 3/5] media: videobuf2: Divide videobuf2-core into 2 parts

2015-08-10 Thread Mauro Carvalho Chehab
Em Mon, 10 Aug 2015 14:07:21 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:

 Hi Junghak,
 
 I'm reviewing the header changes since I think there are several improvements
 that can be done that will make things more logical and will simplify the 
 code.
 
 My comments below are a mix of suggestions for improvement and brainstorming.
 
 Feel free to ask for clarification if something is not clear.
 
 On 07/31/2015 10:44 AM, Junghak Sung wrote:
  Divide videobuf2-core into core part and v4l2-specific part
   - core part: videobuf2 core related with buffer management  memory 
  allocation
   - v4l2-specific part: v4l2-specific stuff
  
  Signed-off-by: Junghak Sung jh1009.s...@samsung.com
  Signed-off-by: Geunyoung Kim nenggun@samsung.com
  Acked-by: Seung-Woo Kim sw0312@samsung.com
  Acked-by: Inki Dae inki@samsung.com
  ---
 
 snip

...

 I noticed that __qbuf_mmap/userptr/dmabuf are all in -v4l2.c. That's a bad 
 sign:
 those are some of the most complex vb2 functions and they really belong in the
 core since you'll need it for DVB as well. As suggested above, by moving the 
 index,
 length and offset/userptr/fd data to the core structs these functions can all 
 be
 moved back into core.c as far as I can see.

Well, that will depend on how the DVB implementation will actually be.

Currently, VB2 has lot of V4L2-dependent code on it, with lots of V4L2
structs from videodev2.h that are there.

Well, if we want the core to be re-used, it should not include videodev2.h
anymore. Also, it should not assume that all non-V4L2 cores would use
exactly the same logic for the userspace API.

In the DVB case, it makes no sense to have anything similar to OVERLAY
there. I also can't see any usage for USERPTR at DVB neither, as either
MMAP or DMABUF should fulfill all userspace needs I'm aware of.

Also, the meta data for the DVB upcoming ioctls for MMAP/DMABUF aren't
yet proposed. They can be very different than the ones inside the V4L2
ioctls.

So, I guess it is better for now to keep those API-dependent stuff at 
VB2-v4l2 and, once the DVB code (and the corresponding API bits) are
written, revisit it and then move the common code to the VB2 core.

 It is good to remember that today the v4l2_buffer struct is used in the vb2
 core because vb2 is only used with v4l2, so why duplicate v4l2_buffer fields
 in the vb2 core structs? 

We should not have any v4l2_* struct inside VB2 core, as the DVB core
should not be dependent on the V4L2 structs. So, everything that it is
V4L2-specific should be inside the VB2-v4l2. The reverse is also true:
we should not pollute the VB2 core with DVB-specific data structures.
So, all VB2-specific struct should be at VB2-dvb.

 But if we want to reuse it for other subsystems, then
 the vb2 core structs should contain all the core buffer information. This 
 avoids
 the need for a lot of the ops that you added and makes it possible to keep the
 __qbuf_mmap/userptr/dmabuf in the core code as well.
 
 Adding these fields to the vb2 core structs is something that can be done 
 first,
 before splitting up core.c into core.c and v4l2.c.

I'm afraid that we'll lose the big picture if we try to put the
API-dependent parts at the core before adding a non-V4L2 usage on VB2.

We can always simplify the code latter, but IMHO we should focus first
on adding the new functionality (support for DVB). Afterwards, we'll have
a better view on what API-dependent code could be shared.

 
  +
  +/**
  + * struct vb2_queue - a videobuf queue
  + *
  + * @type:  queue type (see V4L2_BUF_TYPE_* in linux/videodev2.h
  + * @io_modes:  supported io methods (see vb2_io_modes enum)
  + * @fileio_read_once:  report EOF after reading the first 
  buffer
  + * @fileio_write_immediately:  queue buffer after each write() call
  + * @allow_zero_bytesused:  allow bytesused == 0 to be passed to the driver
  + * @lock:  pointer to a mutex that protects the vb2_queue struct. The
  + * driver can set this to a mutex to let the vb2 core serialize
  + * the queuing ioctls. If the driver wants to handle locking
  + * itself, then this should be set to NULL. This lock is not used
  + * by the videobuf2 core API.
  + * @owner: The filehandle that 'owns' the buffers, i.e. the filehandle
  + * that called reqbufs, create_buffers or started fileio.
  + * This field is not used by the videobuf2 core API, but it allows
  + * drivers to easily associate an owner filehandle with the queue.
  + * @ops:   driver-specific callbacks
  + * @mem_ops:   memory allocator specific callbacks
  + * @drv_priv:  driver private data
  + * @buf_struct_size: size of the driver-specific buffer structure;
  + * 0 indicates the driver doesn't want to use a custom buffer
  + * structure type, so, sizeof(struct vb2_v4l2_buffer) will is used
  + * in case of v4l2.
  + * @timestamp_flags: Timestamp flags; V4L2_BUF_FLAG_TIMESTAMP_* and
  + *

Re: [media-workshop] [RFC] Media graph flow for an hybrid device as discussed at the media workshop

2015-08-10 Thread Mauro Carvalho Chehab
Em Mon, 10 Aug 2015 14:43:50 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:

 Hi Mauro,

Thanks for the review!

 
 On 08/08/2015 01:33 PM, Mauro Carvalho Chehab wrote:
  During the discussions at the Media Workshop, we came with some dot files 
  that
  would describe a hybrid PC-consumer TV stick with radio, analog video, 
  analog
  TV and digital TV on it.
  
  I consolidated all the dot files we've worked there, and added the
  connectors for RF, S-Video and Composite.
  
  The dot file and the corresponding picture is at:
  
  http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.dot
  
  http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.png
  
  As my plan is to start working on some real driver to produce such graph,
  please validate if the entities, interfaces, data links and interface links
  are correct, and if the namespace nomenclature is ok, or if I miss 
  something.
 
 This looks OK to me, except for one small detail: I wouldn't use the name
 Source entities for connectors. Instead use Connector entities since
 such entities correspond to actual real connectors on a backplane. 

Yeah. Well, they're actually Source connector entities ;) But I see
your point. All connectors should be marked with a different type at
the media_graph_obj.

 A proper
 source entity would be a sensor or test pattern generator. Which actually
 can occur with the em28xx since it's used in webcams as well.

Ah, true. I'll add that in the graph and use a different color to
distinguish between source and connector entities.

 
 And a really, really small detail: in the legend the 'interface link' is an
 arrow, but it should be a line, since there is no direction. The graph itself
 is fine.

Well, I didn't find a way to put a line there. The legend is produced by
an html code. I would need to have a line character, or to add an image.

Perhaps I should look deeper to find a bold horizontal line at the UTF-8
charset. #8212; and #8213; are too thin. Do you know any char that would
look better there?

 As you mentioned on irc, the v4l-subdevX nodes won't be created for this 
 device
 since all the configuration happens via the standard interfaces.
 
 But if they were to be created, then they would appear where they are in this
 example.

Thanks!
Mauro

 
 Regards,
 
   Hans
 
 ___
 media-workshop mailing list
 media-works...@linuxtv.org
 http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v2 0/5] Refactoring Videobuf2 for common use

2015-08-10 Thread Hans Verkuil
Hi Jungsak,

On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Hello everybody,
 
 This is the 2nd round for refactoring Videobuf2(a.k.a VB2).
 The purpose of this patch series is to separate existing VB2 framework
 into core part and V4L2 specific part. So that not only V4L2 but also other
 frameworks can use them to manage buffer and utilize queue.
 
 Why do we try to make the VB2 framework to be common?
 
 As you may know, current DVB framework uses ringbuffer mechanism to demux
 MPEG-2 TS data and pass it to userspace. However, this mechanism requires
 extra memory copy because DVB framework provides only read() system call for
 application - read() system call copies the kernel data to user-space buffer.
 So if we can use VB2 framework which supports streaming I/O and buffer
 sharing mechanism, then we could enhance existing DVB framework by removing
 the extra memory copy - with VB2 framework, application can access the kernel
 data directly through mmap system call.
 
 We have a plan for this work as follows:
 1. Separate existing VB2 framework into three parts - VB2 common, VB2-v4l2.
Of course, this change will not affect other v4l2-based
device drivers. This patch series corresponds to this step.
 
 2. Add and implement new APIs for DVB streaming I/O.
We can remove unnecessary memory copy between kernel-space and user-space
by using these new APIs. However, we leaves legacy interfaces as-is
for backward compatibility.
 
 This patch series is the first step for it.
 The previous version of this patch series can be found at [1].
 
 [1] RFC PATCH v1 - http://www.spinics.net/lists/linux-media/msg90688.html

This v2 looks much better, but, as per my comment to patch 1/5, it needs a bit
more work before I can do a really good review. I think things will be much
clearer once patch 3 shows the code moving from core.c/h to v4l2.c/h instead
of the other way around. That shouldn't be too difficult.

Regards,

Hans

 
 Changes since v1:
 1. Divide patch set into more pieces
 v1 was not reviewed normally because the 2/3 patch is failed to send to 
 mailing
 list with size problem - over 300kb. So I have divided the patch set into five
 pieces and refined them neatly, which was pointed by Hans.
 
 2. Add shell scripts for renaming patch
 In case of renaming patch, shell scripts are included inside the body of the
 patches by Mauro's advice. 1/5 and 5/5 patches include these scripts, which 
 can
 be used by reviewers or maintainers to regenerate big patch file if something
 goes wrong during patch apply.
 
 3. Remove dependency on v4l2 from videobuf2
 In previous patch set, videobuf2-core uses v4l2-specific stuff as it is.
 e.g. enum v4l2_buf_type and enum v4l2_memory. That prevented other frameworks
 from using videobuf2 independently and made them forced to include
 v4l2-specific stuff.
 In this version, these dependent stuffs are replaced with VB2 own stuffs.
 e.g. enum vb2_buf_type and enum vb2_memory. So, v4l2-specific header file 
 isn't
 required to use videobuf2 in other modules. Please, note that videobuf2 stuffs
 will be translated to v4l2-specific stuffs in videobuf2-v4l2.c file for
 backward compatibility.
 
 4. Unify duplicated definitions
 VB2_DEBUG() is newly defined in videobuf2-core header file in order to unify
 duplicated macro functions that invoke callback functions implemented in vb2
 backends - i.e., videobuf2-vmalloc and videobuf2-dma-sg - and queue relevant
 callbacks of device drivers.
 In previous patch set, these macro functions were defined
 in both videobuf2-core.c and videobuf2-v4l2.c.
 
 This patch series is base on media_tree.git [2] by Mauro  Hans's request.
 And I applied this patches to my own git [3] which can be helpful to review.
 My test boards are ubuntu PC(Intel i7-3770) and odroid-xu3(exynos5422). And
 basic oprerations, e.g. reqbuf, querybuf, qbuf, dqbuf, are tested with
 v4l-utils. But, more tests for the all ioctls will be required on many other
 targets.
 
 [2] media_tree.git - http://git.linuxtv.org/cgit.cgi/media_tree.git/
 [3] jsung/dvb-vb2.git - http://git.linuxtv.org/cgit.cgi/jsung/dvb-vb2.git/
 
 Any suggestions and comments are welcome.
 
 Regards,
 Junghak
 
 Junghak Sung (5):
   media: videobuf2: Rename videobuf2-core to videobuf2-v4l2
   media: videobuf2: Restructurng struct vb2_buffer for common use.
   media: videobuf2: Divide videobuf2-core into 2 parts
   media: videobuf2: Define vb2_buf_type and vb2_memory
   media: videobuf2: Modify prefix for VB2 functions
 
  drivers/input/touchscreen/sur40.c  |   23 +-
  drivers/media/dvb-frontends/rtl2832_sdr.c  |   19 +-
  drivers/media/pci/cobalt/cobalt-alsa-pcm.c |4 +-
  drivers/media/pci/cobalt/cobalt-v4l2.c |8 +-
  drivers/media/pci/cx23885/cx23885-417.c|   15 +-
  drivers/media/pci/cx23885/cx23885-core.c   |   10 +-
  drivers/media/pci/cx23885/cx23885-dvb.c|   13 +-
  

Re: [PATCHv3 02/13] v4l2: add RF gain control

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 Add new RF tuner gain control named RF Gain. That is aimed for first
 amplifier chip right after antenna connector.
 There is existing LNA Gain control, which is quite same, but it is
 aimed for cases amplifier is integrated to tuner chip. Some designs
 have both, as almost all recent tuner silicons has integrated LNA/RF
 amplifier in any case.
 
 Cc: Hans Verkuil hverk...@xs4all.nl

Acked-by: Hans Verkuil hans.verk...@cisco.com

 Signed-off-by: Antti Palosaari cr...@iki.fi
 ---
  drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++
  include/uapi/linux/v4l2-controls.h   | 1 +
  2 files changed, 3 insertions(+)
 
 diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
 b/drivers/media/v4l2-core/v4l2-ctrls.c
 index b6b7dcc..d18462c 100644
 --- a/drivers/media/v4l2-core/v4l2-ctrls.c
 +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
 @@ -888,6 +888,7 @@ const char *v4l2_ctrl_get_name(u32 id)
   case V4L2_CID_TUNE_DEEMPHASIS:  return De-Emphasis;
   case V4L2_CID_RDS_RECEPTION:return RDS Reception;
   case V4L2_CID_RF_TUNER_CLASS:   return RF Tuner Controls;
 + case V4L2_CID_RF_TUNER_RF_GAIN: return RF Gain;
   case V4L2_CID_RF_TUNER_LNA_GAIN_AUTO:   return LNA Gain, Auto;
   case V4L2_CID_RF_TUNER_LNA_GAIN:return LNA Gain;
   case V4L2_CID_RF_TUNER_MIXER_GAIN_AUTO: return Mixer Gain, Auto;
 @@ -1161,6 +1162,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
 v4l2_ctrl_type *type,
   case V4L2_CID_PILOT_TONE_FREQUENCY:
   case V4L2_CID_TUNE_POWER_LEVEL:
   case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
 + case V4L2_CID_RF_TUNER_RF_GAIN:
   case V4L2_CID_RF_TUNER_LNA_GAIN:
   case V4L2_CID_RF_TUNER_MIXER_GAIN:
   case V4L2_CID_RF_TUNER_IF_GAIN:
 diff --git a/include/uapi/linux/v4l2-controls.h 
 b/include/uapi/linux/v4l2-controls.h
 index d448c53..1bdce50 100644
 --- a/include/uapi/linux/v4l2-controls.h
 +++ b/include/uapi/linux/v4l2-controls.h
 @@ -936,6 +936,7 @@ enum v4l2_deemphasis {
  
  #define V4L2_CID_RF_TUNER_BANDWIDTH_AUTO (V4L2_CID_RF_TUNER_CLASS_BASE + 
 11)
  #define V4L2_CID_RF_TUNER_BANDWIDTH  (V4L2_CID_RF_TUNER_CLASS_BASE + 
 12)
 +#define V4L2_CID_RF_TUNER_RF_GAIN(V4L2_CID_RF_TUNER_CLASS_BASE + 
 32)
  #define V4L2_CID_RF_TUNER_LNA_GAIN_AUTO  
 (V4L2_CID_RF_TUNER_CLASS_BASE + 41)
  #define V4L2_CID_RF_TUNER_LNA_GAIN   (V4L2_CID_RF_TUNER_CLASS_BASE + 
 42)
  #define V4L2_CID_RF_TUNER_MIXER_GAIN_AUTO(V4L2_CID_RF_TUNER_CLASS_BASE + 
 51)
 

--
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: [PATCHv3 03/13] DocBook: document tuner RF gain control

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 Add brief description for tuner RF gain control.
 
 Cc: Hans Verkuil hverk...@xs4all.nl

Acked-by: Hans Verkuil hans.verk...@cisco.com

 Signed-off-by: Antti Palosaari cr...@iki.fi
 ---
  Documentation/DocBook/media/v4l/compat.xml   |  4 
  Documentation/DocBook/media/v4l/controls.xml | 14 ++
  Documentation/DocBook/media/v4l/v4l2.xml |  1 +
  3 files changed, 19 insertions(+)
 
 diff --git a/Documentation/DocBook/media/v4l/compat.xml 
 b/Documentation/DocBook/media/v4l/compat.xml
 index f56faf5..eb091c7 100644
 --- a/Documentation/DocBook/media/v4l/compat.xml
 +++ b/Documentation/DocBook/media/v4l/compat.xml
 @@ -2600,6 +2600,10 @@ and v4l2-mbus-framefmt;.
  constantV4L2_TUNER_ADC/constant is deprecated now.
 /para
   /listitem
 + listitem
 +   paraAdded constantV4L2_CID_RF_TUNER_RF_GAIN/constant
 +RF Tuner control./para
 + /listitem
/orderedlist
  /section
  
 diff --git a/Documentation/DocBook/media/v4l/controls.xml 
 b/Documentation/DocBook/media/v4l/controls.xml
 index 6e1667b..7cae933 100644
 --- a/Documentation/DocBook/media/v4l/controls.xml
 +++ b/Documentation/DocBook/media/v4l/controls.xml
 @@ -5418,6 +5418,18 @@ set. Unit is in Hz. The range and step are 
 driver-specific./entry
entry spanname=descrEnables/disables IF automatic gain 
 control (AGC)/entry
  /row
  row
 +  entry 
 spanname=idconstantV4L2_CID_RF_TUNER_RF_GAIN/constantnbsp;/entry
 +  entryinteger/entry
 +/row
 +row
 +  entry spanname=descrThe RF amplifier is the very first
 +amplifier on the receiver signal path, just right after the antenna input.
 +The difference between the LNA gain and the RF gain in this document is that
 +the LNA gain is integrated in the tuner chip while the RF gain is a separate
 +chip. There may be both RF and LNA gain controls in the same device.
 +The range and step are driver-specific./entry
 +/row
 +row
entry 
 spanname=idconstantV4L2_CID_RF_TUNER_LNA_GAIN/constantnbsp;/entry
entryinteger/entry
  /row
 @@ -5425,6 +5437,8 @@ set. Unit is in Hz. The range and step are 
 driver-specific./entry
entry spanname=descrLNA (low noise amplifier) gain is first
  gain stage on the RF tuner signal path. It is located very close to tuner
  antenna input. Used when 
 constantV4L2_CID_RF_TUNER_LNA_GAIN_AUTO/constant is not set.
 +See constantV4L2_CID_RF_TUNER_RF_GAIN/constant to understand how RF gain
 +and LNA gain differs from the each others.
  The range and step are driver-specific./entry
  /row
  row
 diff --git a/Documentation/DocBook/media/v4l/v4l2.xml 
 b/Documentation/DocBook/media/v4l/v4l2.xml
 index c9eedc1..ab9fca4 100644
 --- a/Documentation/DocBook/media/v4l/v4l2.xml
 +++ b/Documentation/DocBook/media/v4l/v4l2.xml
 @@ -156,6 +156,7 @@ applications. --
   date2015-05-26/date
   authorinitialsap/authorinitials
   revremarkRenamed V4L2_TUNER_ADC to V4L2_TUNER_SDR.
 +Added V4L2_CID_RF_TUNER_RF_GAIN control.
   /revremark
/revision
  
 

--
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: [PATCHv3 06/13] v4l: add type field to v4l2_modulator struct

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 Add type field to that struct like it counterpart v4l2_tuner
 already has. We need type field to distinguish different tuner
 types from each others for transmitter too.
 
 Cc: Hans Verkuil hverk...@xs4all.nl
 Signed-off-by: Antti Palosaari cr...@iki.fi

Acked-by: Hans Verkuil hans.verk...@cisco.com

 ---
  drivers/media/v4l2-core/v4l2-ioctl.c | 18 +-
  include/uapi/linux/videodev2.h   |  3 ++-
  2 files changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
 b/drivers/media/v4l2-core/v4l2-ioctl.c
 index 21e9598..85f80cb 100644
 --- a/drivers/media/v4l2-core/v4l2-ioctl.c
 +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
 @@ -1646,15 +1646,31 @@ static int v4l_s_tuner(const struct v4l2_ioctl_ops 
 *ops,
  static int v4l_g_modulator(const struct v4l2_ioctl_ops *ops,
   struct file *file, void *fh, void *arg)
  {
 + struct video_device *vfd = video_devdata(file);
   struct v4l2_modulator *p = arg;
   int err;
  
 + if (vfd-vfl_type == VFL_TYPE_RADIO)
 + p-type = V4L2_TUNER_RADIO;
 +
   err = ops-vidioc_g_modulator(file, fh, p);
   if (!err)
   p-capability |= V4L2_TUNER_CAP_FREQ_BANDS;
   return err;
  }
  
 +static int v4l_s_modulator(const struct v4l2_ioctl_ops *ops,
 + struct file *file, void *fh, void *arg)
 +{
 + struct video_device *vfd = video_devdata(file);
 + struct v4l2_modulator *p = arg;
 +
 + if (vfd-vfl_type == VFL_TYPE_RADIO)
 + p-type = V4L2_TUNER_RADIO;
 +
 + return ops-vidioc_s_modulator(file, fh, p);
 +}
 +
  static int v4l_g_frequency(const struct v4l2_ioctl_ops *ops,
   struct file *file, void *fh, void *arg)
  {
 @@ -2441,7 +2457,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
   IOCTL_INFO_STD(VIDIOC_G_AUDOUT, vidioc_g_audout, v4l_print_audioout, 0),
   IOCTL_INFO_STD(VIDIOC_S_AUDOUT, vidioc_s_audout, v4l_print_audioout, 
 INFO_FL_PRIO),
   IOCTL_INFO_FNC(VIDIOC_G_MODULATOR, v4l_g_modulator, 
 v4l_print_modulator, INFO_FL_CLEAR(v4l2_modulator, index)),
 - IOCTL_INFO_STD(VIDIOC_S_MODULATOR, vidioc_s_modulator, 
 v4l_print_modulator, INFO_FL_PRIO),
 + IOCTL_INFO_FNC(VIDIOC_S_MODULATOR, v4l_s_modulator, 
 v4l_print_modulator, INFO_FL_PRIO),
   IOCTL_INFO_FNC(VIDIOC_G_FREQUENCY, v4l_g_frequency, 
 v4l_print_frequency, INFO_FL_CLEAR(v4l2_frequency, tuner)),
   IOCTL_INFO_FNC(VIDIOC_S_FREQUENCY, v4l_s_frequency, 
 v4l_print_frequency, INFO_FL_PRIO),
   IOCTL_INFO_FNC(VIDIOC_CROPCAP, v4l_cropcap, v4l_print_cropcap, 
 INFO_FL_CLEAR(v4l2_cropcap, type)),
 diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
 index 70f06c9..3a8eb8e 100644
 --- a/include/uapi/linux/videodev2.h
 +++ b/include/uapi/linux/videodev2.h
 @@ -1584,7 +1584,8 @@ struct v4l2_modulator {
   __u32   rangelow;
   __u32   rangehigh;
   __u32   txsubchans;
 - __u32   reserved[4];
 + __u32   type;   /* enum v4l2_tuner_type */
 + __u32   reserved[3];
  };
  
  /*  Flags for the 'capability' field */
 

--
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: [PATCHv3 12/13] DocBook: fix S_FREQUENCY = G_FREQUENCY

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 It is VIDIOC_G_FREQUENCY which does not use type to identify tuner,
 not VIDIOC_S_FREQUENCY. VIDIOC_S_FREQUENCY uses both tuner and type
 fields. One of these V4L API weirdness...

Actually, that's not what this is about. It's about whether g/s_frequency 
gets/sets
the frequency for the tuner or the modulator. That has nothing to do with the 
tuner
and type fields. The problem described here in the spec is a problem for both G 
and
S_FREQUENCY.

Regards,

Hans

 
 Cc: Hans Verkuil hverk...@xs4all.nl
 Signed-off-by: Antti Palosaari cr...@iki.fi
 ---
  Documentation/DocBook/media/v4l/common.xml | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/Documentation/DocBook/media/v4l/common.xml 
 b/Documentation/DocBook/media/v4l/common.xml
 index 8b5e014..f7008ea 100644
 --- a/Documentation/DocBook/media/v4l/common.xml
 +++ b/Documentation/DocBook/media/v4l/common.xml
 @@ -428,7 +428,7 @@ zero, no video outputs./para
  modulator. Two separate device nodes will have to be used for such
  hardware, one that supports the tuner functionality and one that supports
  the modulator functionality. The reason is a limitation with the
 -VIDIOC-S-FREQUENCY; ioctl where you cannot specify whether the frequency
 +VIDIOC-G-FREQUENCY; ioctl where you cannot specify whether the frequency
  is for a tuner or a modulator./para
  
paraTo query and change modulator properties applications use
 

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


Re: [RFC PATCH v2 4/5] media: videobuf2: Define vb2_buf_type and vb2_memory

2015-08-10 Thread Hans Verkuil
On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Define enum vb2_buf_type and enum vb2_memory for videobuf2-core. This
 change requires translation functions that could covert v4l2-core stuffs
 to videobuf2-core stuffs in videobuf2-v4l2.c file.
 The v4l2-specific member variables(e.g. type, memory) remains in
 struct vb2_queue for backward compatibility and performance of type 
 translation.
 
 Signed-off-by: Junghak Sung jh1009.s...@samsung.com
 Signed-off-by: Geunyoung Kim nenggun@samsung.com
 Acked-by: Seung-Woo Kim sw0312@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 ---
  drivers/media/v4l2-core/videobuf2-core.c |  139 +++-
  drivers/media/v4l2-core/videobuf2-v4l2.c |  209 
 --
  include/media/videobuf2-core.h   |   99 +++---
  include/media/videobuf2-v4l2.h   |   12 +-
  4 files changed, 299 insertions(+), 160 deletions(-)
 

snip

 diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
 b/drivers/media/v4l2-core/videobuf2-v4l2.c
 index 85527e9..22dd19c 100644
 --- a/drivers/media/v4l2-core/videobuf2-v4l2.c
 +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
 @@ -30,8 +30,46 @@
  #include media/v4l2-common.h
  #include media/videobuf2-v4l2.h
  
 +#define CREATE_TRACE_POINTS
  #include trace/events/v4l2.h
  
 +static const enum vb2_buf_type _tbl_buf_type[] = {
 + [V4L2_BUF_TYPE_VIDEO_CAPTURE]   = VB2_BUF_TYPE_VIDEO_CAPTURE,
 + [V4L2_BUF_TYPE_VIDEO_OUTPUT]= VB2_BUF_TYPE_VIDEO_OUTPUT,
 + [V4L2_BUF_TYPE_VIDEO_OVERLAY]   = VB2_BUF_TYPE_VIDEO_OVERLAY,
 + [V4L2_BUF_TYPE_VBI_CAPTURE] = VB2_BUF_TYPE_VBI_CAPTURE,
 + [V4L2_BUF_TYPE_VBI_OUTPUT]  = VB2_BUF_TYPE_VBI_OUTPUT,
 + [V4L2_BUF_TYPE_SLICED_VBI_CAPTURE]  = 
 VB2_BUF_TYPE_SLICED_VBI_CAPTURE,
 + [V4L2_BUF_TYPE_SLICED_VBI_OUTPUT]   = 
 VB2_BUF_TYPE_SLICED_VBI_OUTPUT,
 + [V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY]= 
 VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY,
 + [V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE]= 
 VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
 + [V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE] = 
 VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 + [V4L2_BUF_TYPE_SDR_CAPTURE] = VB2_BUF_TYPE_SDR_CAPTURE,
 + [V4L2_BUF_TYPE_PRIVATE] = VB2_BUF_TYPE_PRIVATE,
 +};
 +
 +static const enum vb2_memory _tbl_memory[] = {
 + [V4L2_MEMORY_MMAP]  = VB2_MEMORY_MMAP,
 + [V4L2_MEMORY_USERPTR]   = VB2_MEMORY_USERPTR,
 + [V4L2_MEMORY_DMABUF]= VB2_MEMORY_DMABUF,
 +};
 +
 +#define to_vb2_buf_type(type)\
 +({   \
 + enum vb2_buf_type ret = 0;  \
 + if( type  0  type  ARRAY_SIZE(_tbl_buf_type) )  \
 + ret = (_tbl_buf_type[type]);\
 + ret;\
 +})
 +
 +#define to_vb2_memory(memory)\
 +({   \
 + enum vb2_memory ret = 0;\
 + if( memory  0  memory  ARRAY_SIZE(_tbl_memory) )\
 + ret = (_tbl_memory[memory]);\
 + ret;\
 +})
 +

snip

 diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
 index dc405da..871fcc6 100644
 --- a/include/media/videobuf2-core.h
 +++ b/include/media/videobuf2-core.h
 @@ -15,9 +15,47 @@
  #include linux/mm_types.h
  #include linux/mutex.h
  #include linux/poll.h
 -#include linux/videodev2.h
  #include linux/dma-buf.h
  
 +#define VB2_MAX_FRAME   32
 +#define VB2_MAX_PLANES   8
 +
 +enum vb2_buf_type {
 + VB2_BUF_TYPE_UNKNOWN= 0,
 + VB2_BUF_TYPE_VIDEO_CAPTURE  = 1,
 + VB2_BUF_TYPE_VIDEO_OUTPUT   = 2,
 + VB2_BUF_TYPE_VIDEO_OVERLAY  = 3,
 + VB2_BUF_TYPE_VBI_CAPTURE= 4,
 + VB2_BUF_TYPE_VBI_OUTPUT = 5,
 + VB2_BUF_TYPE_SLICED_VBI_CAPTURE = 6,
 + VB2_BUF_TYPE_SLICED_VBI_OUTPUT  = 7,
 + VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY   = 8,
 + VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE   = 9,
 + VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE= 10,
 + VB2_BUF_TYPE_SDR_CAPTURE= 11,
 + VB2_BUF_TYPE_DVB_CAPTURE= 12,
 + VB2_BUF_TYPE_PRIVATE= 0x80,
 +};
 +
 +enum vb2_memory {
 + VB2_MEMORY_UNKNOWN  = 0,
 + VB2_MEMORY_MMAP = 1,
 + VB2_MEMORY_USERPTR  = 2,
 + VB2_MEMORY_DMABUF   = 4,
 +};
 +
 +#define VB2_TYPE_IS_MULTIPLANAR(type)\
 + ((type) == VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\
 +  || (type) == VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 +
 +#define VB2_TYPE_IS_OUTPUT(type) \
 + ((type) == VB2_BUF_TYPE_VIDEO_OUTPUT   

Re: [PATCHv3 08/13] hackrf: add control for RF amplifier

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 There is Avago MGA-81563 amplifier just right after antenna connector.
 It could be turned on/off and its gain is around 12dB.
 
 Signed-off-by: Antti Palosaari cr...@iki.fi

Acked-by: Hans Verkuil hans.verk...@cisco.com

 ---
  drivers/media/usb/hackrf/hackrf.c | 26 +-
  1 file changed, 25 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/media/usb/hackrf/hackrf.c 
 b/drivers/media/usb/hackrf/hackrf.c
 index fd1fa41..136de9a 100644
 --- a/drivers/media/usb/hackrf/hackrf.c
 +++ b/drivers/media/usb/hackrf/hackrf.c
 @@ -31,6 +31,7 @@ enum {
   CMD_BOARD_ID_READ  = 0x0e,
   CMD_VERSION_STRING_READ= 0x0f,
   CMD_SET_FREQ   = 0x10,
 + CMD_AMP_ENABLE = 0x11,
   CMD_SET_LNA_GAIN   = 0x13,
   CMD_SET_VGA_GAIN   = 0x14,
  };
 @@ -133,6 +134,7 @@ struct hackrf_dev {
   struct v4l2_ctrl_handler hdl;
   struct v4l2_ctrl *bandwidth_auto;
   struct v4l2_ctrl *bandwidth;
 + struct v4l2_ctrl *rf_gain;
   struct v4l2_ctrl *lna_gain;
   struct v4l2_ctrl *if_gain;
  
 @@ -164,6 +166,7 @@ static int hackrf_ctrl_msg(struct hackrf_dev *dev, u8 
 request, u16 value,
   switch (request) {
   case CMD_SET_TRANSCEIVER_MODE:
   case CMD_SET_FREQ:
 + case CMD_AMP_ENABLE:
   case CMD_SAMPLE_RATE_SET:
   case CMD_BASEBAND_FILTER_BANDWIDTH_SET:
   pipe = usb_sndctrlpipe(dev-udev, 0);
 @@ -949,6 +952,22 @@ static int hackrf_set_bandwidth(struct hackrf_dev *dev)
   return ret;
  }
  
 +static int hackrf_set_rf_gain(struct hackrf_dev *dev)
 +{
 + int ret;
 + u8 u8tmp;
 +
 + dev_dbg(dev-dev, rf val=%d-%d\n,
 + dev-rf_gain-cur.val, dev-rf_gain-val);
 +
 + u8tmp = (dev-rf_gain-val) ? 1 : 0;
 + ret = hackrf_ctrl_msg(dev, CMD_AMP_ENABLE, u8tmp, 0, NULL, 0);
 + if (ret)
 + dev_dbg(dev-dev, failed=%d\n, ret);
 +
 + return ret;
 +}
 +
  static int hackrf_set_lna_gain(struct hackrf_dev *dev)
  {
   int ret;
 @@ -992,6 +1011,9 @@ static int hackrf_s_ctrl(struct v4l2_ctrl *ctrl)
   case V4L2_CID_RF_TUNER_BANDWIDTH:
   ret = hackrf_set_bandwidth(dev);
   break;
 + case  V4L2_CID_RF_TUNER_RF_GAIN:
 + ret = hackrf_set_rf_gain(dev);
 + break;
   case  V4L2_CID_RF_TUNER_LNA_GAIN:
   ret = hackrf_set_lna_gain(dev);
   break;
 @@ -1077,13 +1099,15 @@ static int hackrf_probe(struct usb_interface *intf,
   }
  
   /* Register controls */
 - v4l2_ctrl_handler_init(dev-hdl, 4);
 + v4l2_ctrl_handler_init(dev-hdl, 5);
   dev-bandwidth_auto = v4l2_ctrl_new_std(dev-hdl, hackrf_ctrl_ops,
   V4L2_CID_RF_TUNER_BANDWIDTH_AUTO, 0, 1, 1, 1);
   dev-bandwidth = v4l2_ctrl_new_std(dev-hdl, hackrf_ctrl_ops,
   V4L2_CID_RF_TUNER_BANDWIDTH,
   175, 2800, 5, 175);
   v4l2_ctrl_auto_cluster(2, dev-bandwidth_auto, 0, false);
 + dev-rf_gain = v4l2_ctrl_new_std(dev-hdl, hackrf_ctrl_ops,
 + V4L2_CID_RF_TUNER_RF_GAIN, 0, 12, 12, 0);
   dev-lna_gain = v4l2_ctrl_new_std(dev-hdl, hackrf_ctrl_ops,
   V4L2_CID_RF_TUNER_LNA_GAIN, 0, 40, 8, 0);
   dev-if_gain = v4l2_ctrl_new_std(dev-hdl, hackrf_ctrl_ops,
 

--
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: [PATCHv3 07/13] DocBook: add modulator type field

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 Add new modulator type field to documentation.
 
 Cc: Hans Verkuil hverk...@xs4all.nl
 Signed-off-by: Antti Palosaari cr...@iki.fi
 ---
  Documentation/DocBook/media/v4l/vidioc-g-modulator.xml | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml 
 b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
 index 7068b59..fe4230c 100644
 --- a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
 +++ b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
 @@ -140,7 +140,13 @@ indicator, for example a stereo pilot tone./entry
 /row
 row
   entry__u32/entry
 - entrystructfieldreserved/structfield[4]/entry
 + entrystructfieldtype/structfield/entry
 + entry spanname=hspanType of the tuner, see xref

s/tuner/modulator/

Regards,

Hans

 + linkend=v4l2-tuner-type /./entry
 +   /row
 +   row
 + entry__u32/entry
 + entrystructfieldreserved/structfield[3]/entry
   entryReserved for future extensions. Drivers and
  applications must set the array to zero./entry
 /row
 

--
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: [PATCHv3 10/13] hackrf: add support for transmitter

2015-08-10 Thread Hans Verkuil
Some comments below:

On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 HackRF SDR device has both receiver and transmitter. There is limitation
 that receiver and transmitter cannot be used at the same time
 (half-duplex operation). That patch implements transmitter support to
 existing receiver only driver.
 
 Cc: Hans Verkuil hverk...@xs4all.nl
 Signed-off-by: Antti Palosaari cr...@iki.fi
 ---
  drivers/media/usb/hackrf/hackrf.c | 894 
 ++
  1 file changed, 627 insertions(+), 267 deletions(-)
 
 diff --git a/drivers/media/usb/hackrf/hackrf.c 
 b/drivers/media/usb/hackrf/hackrf.c
 index 5bd291b..f4b5606 100644
 --- a/drivers/media/usb/hackrf/hackrf.c
 +++ b/drivers/media/usb/hackrf/hackrf.c

snip

 @@ -748,15 +925,11 @@ static int hackrf_s_fmt_sdr_cap(struct file *file, void 
 *priv,
   struct v4l2_format *f)
  {
   struct hackrf_dev *dev = video_drvdata(file);
 - struct vb2_queue *q = dev-vb_queue;
   int i;
  
   dev_dbg(dev-dev, pixelformat fourcc %4.4s\n,
   (char *)f-fmt.sdr.pixelformat);
  
 - if (vb2_is_busy(q))
 - return -EBUSY;
 -

Why is this removed?

   memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved));
   for (i = 0; i  NUM_FORMATS; i++) {
   if (f-fmt.sdr.pixelformat == formats[i].pixelformat) {
 @@ -856,17 +1029,64 @@ static int hackrf_g_tuner(struct file *file, void 
 *priv, struct v4l2_tuner *v)
  
   if (v-index == 0) {
   strlcpy(v-name, HackRF ADC, sizeof(v-name));
 - v-type = V4L2_TUNER_ADC;
 + v-type = V4L2_TUNER_SDR;
   v-capability = V4L2_TUNER_CAP_1HZ | V4L2_TUNER_CAP_FREQ_BANDS;
 - v-rangelow  = bands_adc[0].rangelow;
 - v-rangehigh = bands_adc[0].rangehigh;
 + v-rangelow  = bands_adc_dac[0].rangelow;
 + v-rangehigh = bands_adc_dac[0].rangehigh;
   ret = 0;
   } else if (v-index == 1) {
 - strlcpy(v-name, HackRF RF, sizeof(v-name));
 + strlcpy(v-name, HackRF RF RX, sizeof(v-name));

This seems unnecessary. You're calling g_tuner, so it is obvious that it is RX.
I'd keep the name as HackRF RF.

   v-type = V4L2_TUNER_RF;
   v-capability = V4L2_TUNER_CAP_1HZ | V4L2_TUNER_CAP_FREQ_BANDS;
 - v-rangelow  = bands_rf[0].rangelow;
 - v-rangehigh = bands_rf[0].rangehigh;
 + v-rangelow  = bands_rx_tx[0].rangelow;
 + v-rangehigh = bands_rx_tx[0].rangehigh;
 + ret = 0;
 + } else {
 + ret = -EINVAL;
 + }
 +
 + return ret;
 +}
 +
 +static int hackrf_s_modulator(struct file *file, void *fh,
 +const struct v4l2_modulator *a)
 +{
 + struct hackrf_dev *dev = video_drvdata(file);
 + int ret;
 +
 + dev_dbg(dev-dev, index=%d\n, a-index);
 +
 + if (a-index == 0)
 + ret = 0;
 + else if (a-index == 1)
 + ret = 0;
 + else
 + ret = -EINVAL;
 +
 + return ret;

I'd just write:

return a-index  1 ? -EINVAL : 0;

More to the point, why implement this at all? At the moment s_modulator is 
meaningless
since there is nothing to set.

 +}
 +
 +static int hackrf_g_modulator(struct file *file, void *fh,
 +   struct v4l2_modulator *a)
 +{
 + struct hackrf_dev *dev = video_drvdata(file);
 + int ret;
 +
 + dev_dbg(dev-dev, index=%d\n, a-index);
 +
 + if (a-index == 0) {
 + strlcpy(a-name, HackRF DAC, sizeof(a-name));
 + a-type = V4L2_TUNER_SDR;
 + a-capability = V4L2_TUNER_CAP_1HZ | V4L2_TUNER_CAP_FREQ_BANDS;
 + a-rangelow  = bands_adc_dac[0].rangelow;
 + a-rangehigh = bands_adc_dac[0].rangehigh;
 + ret = 0;
 + } else if (a-index == 1) {
 + strlcpy(a-name, HackRF RF TX, sizeof(a-name));

Same as with g_tuner: I'd drop the TX part in the name.

 + a-type = V4L2_TUNER_RF;
 + a-capability = V4L2_TUNER_CAP_1HZ | V4L2_TUNER_CAP_FREQ_BANDS;
 + a-rangelow  = bands_rx_tx[0].rangelow;
 + a-rangehigh = bands_rx_tx[0].rangehigh;
   ret = 0;
   } else {
   ret = -EINVAL;

snip

 @@ -969,6 +1213,11 @@ static const struct v4l2_ioctl_ops hackrf_ioctl_ops = {
   .vidioc_enum_fmt_sdr_cap  = hackrf_enum_fmt_sdr_cap,
   .vidioc_try_fmt_sdr_cap   = hackrf_try_fmt_sdr_cap,
  
 + .vidioc_s_fmt_sdr_out = hackrf_s_fmt_sdr_cap,
 + .vidioc_g_fmt_sdr_out = hackrf_g_fmt_sdr_cap,
 + .vidioc_enum_fmt_sdr_out  = hackrf_enum_fmt_sdr_cap,
 + .vidioc_try_fmt_sdr_out   = hackrf_try_fmt_sdr_cap,
 +

Since hackrf_*_fmt_sdr_cap is used for both capture and output I suggest that
those functions are renamed to hackrf_*_fmt_sdr(), dropping the _cap part.

   .vidioc_reqbufs   = vb2_ioctl_reqbufs,
   .vidioc_create_bufs   = vb2_ioctl_create_bufs,
   

Re: [RFC PATCH v2 0/5] Refactoring Videobuf2 for common use

2015-08-10 Thread Mauro Carvalho Chehab
Em Mon, 10 Aug 2015 10:31:03 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:

 Hi Jungsak,
 
 On 07/31/2015 10:44 AM, Junghak Sung wrote:
  Hello everybody,
  
  This is the 2nd round for refactoring Videobuf2(a.k.a VB2).
  The purpose of this patch series is to separate existing VB2 framework
  into core part and V4L2 specific part. So that not only V4L2 but also other
  frameworks can use them to manage buffer and utilize queue.
  
  Why do we try to make the VB2 framework to be common?
  
  As you may know, current DVB framework uses ringbuffer mechanism to demux
  MPEG-2 TS data and pass it to userspace. However, this mechanism requires
  extra memory copy because DVB framework provides only read() system call for
  application - read() system call copies the kernel data to user-space 
  buffer.
  So if we can use VB2 framework which supports streaming I/O and buffer
  sharing mechanism, then we could enhance existing DVB framework by removing
  the extra memory copy - with VB2 framework, application can access the 
  kernel
  data directly through mmap system call.
  
  We have a plan for this work as follows:
  1. Separate existing VB2 framework into three parts - VB2 common, VB2-v4l2.
 Of course, this change will not affect other v4l2-based
 device drivers. This patch series corresponds to this step.
  
  2. Add and implement new APIs for DVB streaming I/O.
 We can remove unnecessary memory copy between kernel-space and user-space
 by using these new APIs. However, we leaves legacy interfaces as-is
 for backward compatibility.
  
  This patch series is the first step for it.
  The previous version of this patch series can be found at [1].
  
  [1] RFC PATCH v1 - http://www.spinics.net/lists/linux-media/msg90688.html
 
 This v2 looks much better, but, as per my comment to patch 1/5, it needs a bit
 more work before I can do a really good review. I think things will be much
 clearer once patch 3 shows the code moving from core.c/h to v4l2.c/h instead
 of the other way around. That shouldn't be too difficult.

Hans,

I suggested Junkhak to do that. On his previous patchset, he did what
you're suggestiong, e. g moving things from vb2-core into vb2-v4l2, and
that resulted on patches big enough to not be catched by vger.

Also, IMHO, it is cleared this way, as we can see what parts of VB2 will
actually be shared, as there are lots of things that won't be shared:
overlay, userptr, multiplanar.

Regards,
Mauro

 
 Regards,
 
   Hans
 
  
  Changes since v1:
  1. Divide patch set into more pieces
  v1 was not reviewed normally because the 2/3 patch is failed to send to 
  mailing
  list with size problem - over 300kb. So I have divided the patch set into 
  five
  pieces and refined them neatly, which was pointed by Hans.
  
  2. Add shell scripts for renaming patch
  In case of renaming patch, shell scripts are included inside the body of the
  patches by Mauro's advice. 1/5 and 5/5 patches include these scripts, which 
  can
  be used by reviewers or maintainers to regenerate big patch file if 
  something
  goes wrong during patch apply.
  
  3. Remove dependency on v4l2 from videobuf2
  In previous patch set, videobuf2-core uses v4l2-specific stuff as it is.
  e.g. enum v4l2_buf_type and enum v4l2_memory. That prevented other 
  frameworks
  from using videobuf2 independently and made them forced to include
  v4l2-specific stuff.
  In this version, these dependent stuffs are replaced with VB2 own stuffs.
  e.g. enum vb2_buf_type and enum vb2_memory. So, v4l2-specific header file 
  isn't
  required to use videobuf2 in other modules. Please, note that videobuf2 
  stuffs
  will be translated to v4l2-specific stuffs in videobuf2-v4l2.c file for
  backward compatibility.
  
  4. Unify duplicated definitions
  VB2_DEBUG() is newly defined in videobuf2-core header file in order to unify
  duplicated macro functions that invoke callback functions implemented in vb2
  backends - i.e., videobuf2-vmalloc and videobuf2-dma-sg - and queue relevant
  callbacks of device drivers.
  In previous patch set, these macro functions were defined
  in both videobuf2-core.c and videobuf2-v4l2.c.
  
  This patch series is base on media_tree.git [2] by Mauro  Hans's request.
  And I applied this patches to my own git [3] which can be helpful to review.
  My test boards are ubuntu PC(Intel i7-3770) and odroid-xu3(exynos5422). And
  basic oprerations, e.g. reqbuf, querybuf, qbuf, dqbuf, are tested with
  v4l-utils. But, more tests for the all ioctls will be required on many other
  targets.
  
  [2] media_tree.git - http://git.linuxtv.org/cgit.cgi/media_tree.git/
  [3] jsung/dvb-vb2.git - http://git.linuxtv.org/cgit.cgi/jsung/dvb-vb2.git/
  
  Any suggestions and comments are welcome.
  
  Regards,
  Junghak
  
  Junghak Sung (5):
media: videobuf2: Rename videobuf2-core to videobuf2-v4l2
media: videobuf2: Restructurng struct vb2_buffer for common use.
media: videobuf2: Divide videobuf2-core 

Re: [RFC PATCH v2 1/5] media: videobuf2: Rename videobuf2-core to videobuf2-v4l2

2015-08-10 Thread Hans Verkuil
Hi Junghak,

On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Rename file name - from videobuf2-core.[ch] to videobuf2-v4l2.[ch]
 This renaming patch should be accompanied by the modifications for all device
 drivers that include this header file. It can be done with just running this
 shell script.
 
 replace()
 {
 str1=$1
 str2=$2
 dir=$3
 for file in $(find $dir -name *.h -o -name *.c -o -name Makefile)
 do
 echo $file
 sed s/$str1/$str2/g $file  $file.out
 mv $file.out $file
 done
 }
 replace videobuf2-core videobuf2-v4l2 include/media/
 replace videobuf2-core videobuf2-v4l2 drivers/media/
 
 Signed-off-by: Junghak Sung jh1009.s...@samsung.com
 Signed-off-by: Geunyoung Kim nenggun@samsung.com
 Acked-by: Seung-Woo Kim sw0312@samsung.com
 Acked-by: Inki Dae inki@samsung.com

Rather than moving videobuf2-core.c to videobuf2-v4l2.c I would recommend to 
just
create a videobuf2-v4l2.h header that includes videobuf2-core.h and nothing 
else.

So this patch will move all drivers over to include the new videobuf2-v4l2.h 
header,
leaving everything else unchanged.

Then in patch 3 you can move code from videobuf2-core.c/h to 
videobuf2-v4l2.c/h. This
will make patch 3 easier to read for me since I can clearly see in the diff what
has been moved from -core.c/h to -v4l2.c/h.

Right now patch 3 shows what moved from -v4l2.c/h to -core.c/h, which is the 
wrong
way around. And makes patch 3 much larger than it needs to be since 
(unsurprisingly)
most code from v4l2.c/h moves back to core.c/h.

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: [PATCHv3 09/13] hackrf: switch to single function which configures everything

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 Implement single funtion, hackrf_set_params(), which handles all
 needed settings. Controls and other IOCTLs are just wrappers to that
 function. That way we can get easily better control what we could do
 on different device states - sleeping, receiving, transmitting.
 
 Signed-off-by: Antti Palosaari cr...@iki.fi

Acked-by: Hans Verkuil hans.verk...@cisco.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: [PATCHv3 13/13] DocBook: add tuner types SDR and RF for G_TUNER / S_TUNER

2015-08-10 Thread Hans Verkuil
On 07/31/2015 04:10 AM, Antti Palosaari wrote:
 Add V4L2_TUNER_SDR and V4L2_TUNER_RF to supported tuner types to
 table.
 
 Cc: Hans Verkuil hverk...@xs4all.nl
 Signed-off-by: Antti Palosaari cr...@iki.fi
 ---
  Documentation/DocBook/media/v4l/vidioc-g-tuner.xml | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml 
 b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
 index b0d8659..10737a1 100644
 --- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
 +++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
 @@ -261,6 +261,16 @@ applications must set the array to zero./entry
   entry2/entry
   entry/entry
 /row
 +   row
 + entryconstantV4L2_TUNER_SDR/constant/entry
 + entry4/entry
 + entry/entry
 +   /row
 +   row
 + entryconstantV4L2_TUNER_RF/constant/entry
 + entry5/entry
 + entry/entry
 +   /row
   /tbody
/tgroup
  /table
 

The description should also be filled in here. It was never filled in for the
RADIO and ANALOG_TV since that was obvious, but for these two new types it
should be set. For consistency just set the description of the RADIO type to
Radio and for ANALOG_TV to Analog TV.

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 PATCH v2 0/5] Refactoring Videobuf2 for common use

2015-08-10 Thread Hans Verkuil
On 08/10/2015 11:32 AM, Mauro Carvalho Chehab wrote:
 Em Mon, 10 Aug 2015 10:31:03 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:
 
 Hi Jungsak,

 On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Hello everybody,

 This is the 2nd round for refactoring Videobuf2(a.k.a VB2).
 The purpose of this patch series is to separate existing VB2 framework
 into core part and V4L2 specific part. So that not only V4L2 but also other
 frameworks can use them to manage buffer and utilize queue.

 Why do we try to make the VB2 framework to be common?

 As you may know, current DVB framework uses ringbuffer mechanism to demux
 MPEG-2 TS data and pass it to userspace. However, this mechanism requires
 extra memory copy because DVB framework provides only read() system call for
 application - read() system call copies the kernel data to user-space 
 buffer.
 So if we can use VB2 framework which supports streaming I/O and buffer
 sharing mechanism, then we could enhance existing DVB framework by removing
 the extra memory copy - with VB2 framework, application can access the 
 kernel
 data directly through mmap system call.

 We have a plan for this work as follows:
 1. Separate existing VB2 framework into three parts - VB2 common, VB2-v4l2.
Of course, this change will not affect other v4l2-based
device drivers. This patch series corresponds to this step.

 2. Add and implement new APIs for DVB streaming I/O.
We can remove unnecessary memory copy between kernel-space and user-space
by using these new APIs. However, we leaves legacy interfaces as-is
for backward compatibility.

 This patch series is the first step for it.
 The previous version of this patch series can be found at [1].

 [1] RFC PATCH v1 - http://www.spinics.net/lists/linux-media/msg90688.html

 This v2 looks much better, but, as per my comment to patch 1/5, it needs a 
 bit
 more work before I can do a really good review. I think things will be much
 clearer once patch 3 shows the code moving from core.c/h to v4l2.c/h instead
 of the other way around. That shouldn't be too difficult.
 
 Hans,
 
 I suggested Junkhak to do that. On his previous patchset, he did what
 you're suggestiong, e. g moving things from vb2-core into vb2-v4l2, and
 that resulted on patches big enough to not be catched by vger.

Actually, that wasn't the reason why the patches became so big. I just
reorganized the patch series as I suggested above (pretty easy to do) and
the size of patch 3 went down.

 Also, IMHO, it is cleared this way, as we can see what parts of VB2 will
 actually be shared, as there are lots of things that won't be shared:
 overlay, userptr, multiplanar.

That's why I prefer to see what moves *out* from the core.

To be honest, it depends on what your preference is.

Junghak, just leave the patch as-is. However, for v3 you should run
checkpatch.pl over the diff since it complained about various things.

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 PATCH v2 0/5] Refactoring Videobuf2 for common use

2015-08-10 Thread Mauro Carvalho Chehab
Em Mon, 10 Aug 2015 12:11:39 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:

 On 08/10/2015 11:32 AM, Mauro Carvalho Chehab wrote:
  Em Mon, 10 Aug 2015 10:31:03 +0200
  Hans Verkuil hverk...@xs4all.nl escreveu:
  
  Hi Jungsak,
 
  On 07/31/2015 10:44 AM, Junghak Sung wrote:
  Hello everybody,
 
  This is the 2nd round for refactoring Videobuf2(a.k.a VB2).
  The purpose of this patch series is to separate existing VB2 framework
  into core part and V4L2 specific part. So that not only V4L2 but also 
  other
  frameworks can use them to manage buffer and utilize queue.
 
  Why do we try to make the VB2 framework to be common?
 
  As you may know, current DVB framework uses ringbuffer mechanism to demux
  MPEG-2 TS data and pass it to userspace. However, this mechanism requires
  extra memory copy because DVB framework provides only read() system call 
  for
  application - read() system call copies the kernel data to user-space 
  buffer.
  So if we can use VB2 framework which supports streaming I/O and buffer
  sharing mechanism, then we could enhance existing DVB framework by 
  removing
  the extra memory copy - with VB2 framework, application can access the 
  kernel
  data directly through mmap system call.
 
  We have a plan for this work as follows:
  1. Separate existing VB2 framework into three parts - VB2 common, 
  VB2-v4l2.
 Of course, this change will not affect other v4l2-based
 device drivers. This patch series corresponds to this step.
 
  2. Add and implement new APIs for DVB streaming I/O.
 We can remove unnecessary memory copy between kernel-space and 
  user-space
 by using these new APIs. However, we leaves legacy interfaces as-is
 for backward compatibility.
 
  This patch series is the first step for it.
  The previous version of this patch series can be found at [1].
 
  [1] RFC PATCH v1 - http://www.spinics.net/lists/linux-media/msg90688.html
 
  This v2 looks much better, but, as per my comment to patch 1/5, it needs a 
  bit
  more work before I can do a really good review. I think things will be much
  clearer once patch 3 shows the code moving from core.c/h to v4l2.c/h 
  instead
  of the other way around. That shouldn't be too difficult.
  
  Hans,
  
  I suggested Junkhak to do that. On his previous patchset, he did what
  you're suggestiong, e. g moving things from vb2-core into vb2-v4l2, and
  that resulted on patches big enough to not be catched by vger.
 
 Actually, that wasn't the reason why the patches became so big. I just
 reorganized the patch series as I suggested above (pretty easy to do) and
 the size of patch 3 went down.

Ah, ok.

  Also, IMHO, it is cleared this way, as we can see what parts of VB2 will
  actually be shared, as there are lots of things that won't be shared:
  overlay, userptr, multiplanar.
 
 That's why I prefer to see what moves *out* from the core.
 
 To be honest, it depends on what your preference is.

Yeah. I actually prefer to see what will be shared, because the
non-shared code won't have changes (except for minor kABI things),
while the shared code will be more affected :)

 Junghak, just leave the patch as-is. However, for v3 you should run
 checkpatch.pl over the diff since it complained about various things.

There are two things here:

1) on patches that just move things around, we should not
run checkpatch, as otherwise it would be a nightmare for
review. Ok, as we're doing a remanufacturing, it is a good
idea to run checkpatch at the end and see what should be fixed
(or do it before), but this is out of the scope of the manufacturing.
I can do that myself when applying the series.

2) For all other patches that are adding/changing the code, then
Junghak should run checkpatch and fix (most) stuff complained there.

Regards,
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: VIMC: API proposal, configuring the topology through user space

2015-08-10 Thread Hans Verkuil
Hi Helen!

On 08/08/2015 03:55 AM, Helen Fornazier wrote:
 Hi!
 
 I've made a first sketch about the API of the vimc driver (virtual
 media controller) to configure the topology through user space.
 As first suggested by Laurent Pinchart, it is based on configfs.
 
 I would like to know you opinion about it, if you have any suggestion
 to improve it, otherwise I'll start its implementation as soon as
 possible.
 This API may change with the MC changes and I may see other possible
 configurations as I implementing it but here is the first idea of how
 the API will look like.
 
 vimc project link: https://github.com/helen-fornazier/opw-staging/
 For more information: http://kernelnewbies.org/LaurentPinchart
 
 /***
 The API:
 /
 
 In short, a topology like this one: http://goo.gl/Y7eUfu
 Would look like this filesystem tree: https://goo.gl/tCZPTg
 Txt version of the filesystem tree: https://goo.gl/42KX8Y
 
 * The /configfs/vimc subsystem

I haven't checked the latest vimc driver, but doesn't it support
multiple instances, just like vivid? I would certainly recommend that.

And if there are multiple instances, then each instance gets its own
entry in configfs: /configs/vimc0, /configs/vimc1, etc.

 The vimc driver registers a subsystem in the configfs with the
 following contents:
  ls /configfs/vimc
 build_topology status
 The build_topology attribute file will be used to tell the driver the
 configuration is done and it can build the topology internally
  echo -n anything here  /configfs/vimc/build_topology
 Reading from the status attribute can have 3 different classes of outputs
 1) deployed: the current configured tree is built
 2) undeployed: no errors, the user has modified the configfs tree thus
 the topology was undeployed
 3) error error_message: the topology configuration is wrong

I don't see the status file in the filesystem tree links above.

I actually wonder if we can't use build_topology for that: reading it
will just return the status.

What happens if it is deployed and you want to 'undeploy' it? Instead of
writing anything to build_topology it might be useful to write a real
command to it. E.g. 'deploy', 'destroy'.

What happens when you make changes while a topology is deployed? Should
such changes be rejected until the usecount of the driver goes to 0, or
will it only be rejected when you try to deploy the new configuration?

 * Creating an entity:
 Use mkdir in the /configfs/vimc to create an entity representation, e.g.:
  mkdir /configfs/vimc/sensor_a
 The attribute files will be created by the driver through configfs:
  ls /configfs/vimc/sensor_a
 name role
 Configure the name that will appear to the /dev/media0 and what this
 node do (debayer, scaler, capture, input, generic)
  echo -n Sensor A  /configfs/vimc/sensor_a/name
  echo -n sensor  /configfs/vimc/sensor_a/role
 
 * Creating a pad:
 Use mkdir inside an entity's folder, the attribute called direction
 will be automatically created in the process, for example:
  mkdir /configfs/vimc/sensor_a/pad_0
  ls /configfs/vimc/sensor_a/pad_0
 direction
  echo -n source  /configfs/vimc/sensor_a/pad_0/direction
 The direction attribute can be source or sink

Hmm. Aren't the pads+direction dictated by the entity role? E.g. a sensor
will only have one pad which is always a source. I think setting the role
is what creates the pad directories + direction files.

 
 * Creating a link between pads in two steps:
 Step 1)
 Create a folder inside the source pad folder, the attribute called
 flag will be automatically created in the process, for example:
  mkdir /configfs/vimc/sensor_a/pad_0/link_to_raw_capture_0/
  ls /configfs/vimc/sensor_a/pad_0/link_to_raw_capture_0/
 flags
  echo -n enabled,immutable 
 /configfs/vimc/sensor_a/pad_0/link_to_raw_capture_0/flags
 In the flags attribute we can have all the links attributes (enabled,
 immutable and dynamic) separated by comma
 
 Step 2)
 Add a symlink between the previous folder we just created in the
 source pad and the sink pad folder we want to connect. Lets say we
 want to connect with the pad on the raw_capture_0 entity pad 0
  ln -s /configfs/vimc/sensor_a/pad_0/link_to_raw_capture_0/
 /configfs/vimc/raw_capture_0/pad_0/

Can't this be created automatically? Or possibly not at all, since it is
implicit in step 1.

BTW, the direction is the wrong way around for pads 0 and 1 of the debayer
and scaler entities: pad_0 is a sink since it gets its data from a sensor
or debayer source pad.

 
 * Build the topology.
 After configuring it, tell the driver we finished:
  echo -n anything here  /configfs/vimc/build_topology
  cat /configfs/vimc/status
 
 NOTE 1: The entity's numbering, as read from /dev/media0, will be the
 order of the creation, same about the pads. Pad 0 will be the first
 pad created in an 

Re: [media-workshop] [RFC] Media graph flow for an hybrid device as discussed at the media workshop

2015-08-10 Thread Mauro Carvalho Chehab
Em Mon, 10 Aug 2015 10:05:24 -0300
Mauro Carvalho Chehab mche...@osg.samsung.com escreveu:

 Em Mon, 10 Aug 2015 14:43:50 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:
 
  Hi Mauro,
 
 Thanks for the review!
 
  
  On 08/08/2015 01:33 PM, Mauro Carvalho Chehab wrote:
   During the discussions at the Media Workshop, we came with some dot files 
   that
   would describe a hybrid PC-consumer TV stick with radio, analog video, 
   analog
   TV and digital TV on it.
   
   I consolidated all the dot files we've worked there, and added the
   connectors for RF, S-Video and Composite.
   
   The dot file and the corresponding picture is at:
 
   http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.dot
 
   http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.png
   
   As my plan is to start working on some real driver to produce such graph,
   please validate if the entities, interfaces, data links and interface 
   links
   are correct, and if the namespace nomenclature is ok, or if I miss 
   something.
  
  This looks OK to me, except for one small detail: I wouldn't use the name
  Source entities for connectors. Instead use Connector entities since
  such entities correspond to actual real connectors on a backplane. 
 
 Yeah. Well, they're actually Source connector entities ;) But I see
 your point. All connectors should be marked with a different type at
 the media_graph_obj.
 
  A proper
  source entity would be a sensor or test pattern generator. Which actually
  can occur with the em28xx since it's used in webcams as well.
 
 Ah, true. I'll add that in the graph and use a different color to
 distinguish between source and connector entities.
 
  
  And a really, really small detail: in the legend the 'interface link' is an
  arrow, but it should be a line, since there is no direction. The graph 
  itself
  is fine.
 
 Well, I didn't find a way to put a line there. The legend is produced by
 an html code. I would need to have a line character, or to add an image.
 
 Perhaps I should look deeper to find a bold horizontal line at the UTF-8
 charset. #8212; and #8213; are too thin. Do you know any char that would
 look better there?

Found one character ;)

I also added a webcam sensor and fixed the legend. See below:

http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v3.png
http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v3.dot

 
  As you mentioned on irc, the v4l-subdevX nodes won't be created for this 
  device
  since all the configuration happens via the standard interfaces.
  
  But if they were to be created, then they would appear where they are in 
  this
  example.
 
 Thanks!
 Mauro
 
  
  Regards,
  
  Hans
  
  ___
  media-workshop mailing list
  media-works...@linuxtv.org
  http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v2 3/5] media: videobuf2: Divide videobuf2-core into 2 parts

2015-08-10 Thread Hans Verkuil
On 08/10/2015 02:55 PM, Mauro Carvalho Chehab wrote:
 Em Mon, 10 Aug 2015 14:07:21 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:
 
 Hi Junghak,

 I'm reviewing the header changes since I think there are several improvements
 that can be done that will make things more logical and will simplify the 
 code.

 My comments below are a mix of suggestions for improvement and brainstorming.

 Feel free to ask for clarification if something is not clear.

 On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Divide videobuf2-core into core part and v4l2-specific part
  - core part: videobuf2 core related with buffer management  memory 
 allocation
  - v4l2-specific part: v4l2-specific stuff

 Signed-off-by: Junghak Sung jh1009.s...@samsung.com
 Signed-off-by: Geunyoung Kim nenggun@samsung.com
 Acked-by: Seung-Woo Kim sw0312@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 ---

 snip
 
 ...
 
 I noticed that __qbuf_mmap/userptr/dmabuf are all in -v4l2.c. That's a bad 
 sign:
 those are some of the most complex vb2 functions and they really belong in 
 the
 core since you'll need it for DVB as well. As suggested above, by moving the 
 index,
 length and offset/userptr/fd data to the core structs these functions can 
 all be
 moved back into core.c as far as I can see.
 
 Well, that will depend on how the DVB implementation will actually be.
 
 Currently, VB2 has lot of V4L2-dependent code on it, with lots of V4L2
 structs from videodev2.h that are there.
 
 Well, if we want the core to be re-used, it should not include videodev2.h
 anymore. Also, it should not assume that all non-V4L2 cores would use
 exactly the same logic for the userspace API.

Agreed.

 In the DVB case, it makes no sense to have anything similar to OVERLAY
 there.

VB2 doesn't support overlay at all, so that's no problem.

 I also can't see any usage for USERPTR at DVB neither, as either
 MMAP or DMABUF should fulfill all userspace needs I'm aware of.

While USERPTR isn't needed for DVB, the actual handling of such buffers
is completely independent of the API. I think it is from an architecture
point-of-view a really bad idea if anyone other than the vb2 core would
call the memops. So yes, the core would have code that is not needed by
DVB, but it still is something that belongs to the core in my view. Anything
else is very ugly.

 
 Also, the meta data for the DVB upcoming ioctls for MMAP/DMABUF aren't
 yet proposed. They can be very different than the ones inside the V4L2
 ioctls.

Well, it's pretty much constrained by mmap() and the dma-buf API. I.e. an
offset for for mmap and a fd for dmabuf. You don't have a choice here.

 
 So, I guess it is better for now to keep those API-dependent stuff at 
 VB2-v4l2 and, once the DVB code (and the corresponding API bits) are
 written, revisit it and then move the common code to the VB2 core.

I strongly disagree with that. Having API-dependent code calling memops
defeats the purpose. There is nothing wrong with having a vb2 core that
supports e.g. USERPTR and dma-buf as long as that core is API-independent.

But there is a lot wrong if the API-dependent code is bypassing the vb2 core
code to call low-level memops.

 It is good to remember that today the v4l2_buffer struct is used in the vb2
 core because vb2 is only used with v4l2, so why duplicate v4l2_buffer fields
 in the vb2 core structs? 
 
 We should not have any v4l2_* struct inside VB2 core, as the DVB core
 should not be dependent on the V4L2 structs. So, everything that it is
 V4L2-specific should be inside the VB2-v4l2. The reverse is also true:
 we should not pollute the VB2 core with DVB-specific data structures.

I never said anything else. I was talking about what's used today and why
it is the way it is now.

 So, all VB2-specific struct should be at VB2-dvb.
 
 But if we want to reuse it for other subsystems, then
 the vb2 core structs should contain all the core buffer information. This 
 avoids
 the need for a lot of the ops that you added and makes it possible to keep 
 the
 __qbuf_mmap/userptr/dmabuf in the core code as well.

 Adding these fields to the vb2 core structs is something that can be done 
 first,
 before splitting up core.c into core.c and v4l2.c.
 
 I'm afraid that we'll lose the big picture if we try to put the
 API-dependent parts at the core before adding a non-V4L2 usage on VB2.
 
 We can always simplify the code latter, but IMHO we should focus first
 on adding the new functionality (support for DVB). Afterwards, we'll have
 a better view on what API-dependent code could be shared.

This is core code. I must be able to follow all the changes and right now
I can't. So simplifications like this should be done first before the split.
It will makes things much easier to follow.

The whole goal of a vb2 core is that it:

1) does not use any v4l2/dvb/whatever subsystem specific structs or includes.
2) deals with *all* the buffer memory handling. This is the most complex part, 
and
   that's what really belongs 

Re: [media-workshop] [RFC] Media graph flow for an hybrid device as discussed at the media workshop

2015-08-10 Thread Hans Verkuil
On 08/10/2015 03:19 PM, Mauro Carvalho Chehab wrote:
 Em Mon, 10 Aug 2015 10:05:24 -0300
 Mauro Carvalho Chehab mche...@osg.samsung.com escreveu:
 
 Em Mon, 10 Aug 2015 14:43:50 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:

 Hi Mauro,

 Thanks for the review!


 On 08/08/2015 01:33 PM, Mauro Carvalho Chehab wrote:
 During the discussions at the Media Workshop, we came with some dot files 
 that
 would describe a hybrid PC-consumer TV stick with radio, analog video, 
 analog
 TV and digital TV on it.

 I consolidated all the dot files we've worked there, and added the
 connectors for RF, S-Video and Composite.

 The dot file and the corresponding picture is at:

 http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.dot

 http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v2.png

 As my plan is to start working on some real driver to produce such graph,
 please validate if the entities, interfaces, data links and interface links
 are correct, and if the namespace nomenclature is ok, or if I miss 
 something.

 This looks OK to me, except for one small detail: I wouldn't use the name
 Source entities for connectors. Instead use Connector entities since
 such entities correspond to actual real connectors on a backplane. 

 Yeah. Well, they're actually Source connector entities ;) But I see
 your point. All connectors should be marked with a different type at
 the media_graph_obj.

 A proper
 source entity would be a sensor or test pattern generator. Which actually
 can occur with the em28xx since it's used in webcams as well.

 Ah, true. I'll add that in the graph and use a different color to
 distinguish between source and connector entities.


 And a really, really small detail: in the legend the 'interface link' is an
 arrow, but it should be a line, since there is no direction. The graph 
 itself
 is fine.

 Well, I didn't find a way to put a line there. The legend is produced by
 an html code. I would need to have a line character, or to add an image.

 Perhaps I should look deeper to find a bold horizontal line at the UTF-8
 charset. #8212; and #8213; are too thin. Do you know any char that would
 look better there?
 
 Found one character ;)
 
 I also added a webcam sensor and fixed the legend. See below:
 
 http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v3.png
 http://linuxtv.org/downloads/presentations/mc_ws_2015/dvb-pipeline-v3.dot

Looks good. But if you have a sensor, then there should also be a v4l-subdev2
interface for the sensor entity, and it is also controlled by video0, so that
interface-to-entity link is missing.

And the saa7115 output pads need to be renumbered to 4-6 (there are two pads '3'
at the moment and the mixer is linked to the input pad 3).

Regards,

Hans

 

 As you mentioned on irc, the v4l-subdevX nodes won't be created for this 
 device
 since all the configuration happens via the standard interfaces.

 But if they were to be created, then they would appear where they are in 
 this
 example.

 Thanks!
 Mauro


 Regards,

 Hans

 ___
 media-workshop mailing list
 media-works...@linuxtv.org
 http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop
 --
 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: [RFC PATCH v2 0/5] Refactoring Videobuf2 for common use

2015-08-10 Thread Hans Verkuil
On 08/10/2015 12:49 PM, Mauro Carvalho Chehab wrote:
 Em Mon, 10 Aug 2015 12:11:39 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:
 
 On 08/10/2015 11:32 AM, Mauro Carvalho Chehab wrote:
 Em Mon, 10 Aug 2015 10:31:03 +0200
 Hans Verkuil hverk...@xs4all.nl escreveu:

 Hi Jungsak,

 On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Hello everybody,

 This is the 2nd round for refactoring Videobuf2(a.k.a VB2).
 The purpose of this patch series is to separate existing VB2 framework
 into core part and V4L2 specific part. So that not only V4L2 but also 
 other
 frameworks can use them to manage buffer and utilize queue.

 Why do we try to make the VB2 framework to be common?

 As you may know, current DVB framework uses ringbuffer mechanism to demux
 MPEG-2 TS data and pass it to userspace. However, this mechanism requires
 extra memory copy because DVB framework provides only read() system call 
 for
 application - read() system call copies the kernel data to user-space 
 buffer.
 So if we can use VB2 framework which supports streaming I/O and buffer
 sharing mechanism, then we could enhance existing DVB framework by 
 removing
 the extra memory copy - with VB2 framework, application can access the 
 kernel
 data directly through mmap system call.

 We have a plan for this work as follows:
 1. Separate existing VB2 framework into three parts - VB2 common, 
 VB2-v4l2.
Of course, this change will not affect other v4l2-based
device drivers. This patch series corresponds to this step.

 2. Add and implement new APIs for DVB streaming I/O.
We can remove unnecessary memory copy between kernel-space and 
 user-space
by using these new APIs. However, we leaves legacy interfaces as-is
for backward compatibility.

 This patch series is the first step for it.
 The previous version of this patch series can be found at [1].

 [1] RFC PATCH v1 - http://www.spinics.net/lists/linux-media/msg90688.html

 This v2 looks much better, but, as per my comment to patch 1/5, it needs a 
 bit
 more work before I can do a really good review. I think things will be much
 clearer once patch 3 shows the code moving from core.c/h to v4l2.c/h 
 instead
 of the other way around. That shouldn't be too difficult.

 Hans,

 I suggested Junkhak to do that. On his previous patchset, he did what
 you're suggestiong, e. g moving things from vb2-core into vb2-v4l2, and
 that resulted on patches big enough to not be catched by vger.

 Actually, that wasn't the reason why the patches became so big. I just
 reorganized the patch series as I suggested above (pretty easy to do) and
 the size of patch 3 went down.
 
 Ah, ok.
 
 Also, IMHO, it is cleared this way, as we can see what parts of VB2 will
 actually be shared, as there are lots of things that won't be shared:
 overlay, userptr, multiplanar.

 That's why I prefer to see what moves *out* from the core.

 To be honest, it depends on what your preference is.
 
 Yeah. I actually prefer to see what will be shared, because the
 non-shared code won't have changes (except for minor kABI things),
 while the shared code will be more affected :)
 
 Junghak, just leave the patch as-is. However, for v3 you should run
 checkpatch.pl over the diff since it complained about various things.
 
 There are two things here:
 
 1) on patches that just move things around, we should not
 run checkpatch, as otherwise it would be a nightmare for
 review. Ok, as we're doing a remanufacturing, it is a good
 idea to run checkpatch at the end and see what should be fixed
 (or do it before), but this is out of the scope of the manufacturing.
 I can do that myself when applying the series.

It was actually complaining about new code.

Hans

 
 2) For all other patches that are adding/changing the code, then
 Junghak should run checkpatch and fix (most) stuff complained there.
 
 Regards,
 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: [RFC PATCH v2 3/5] media: videobuf2: Divide videobuf2-core into 2 parts

2015-08-10 Thread Hans Verkuil
Hi Junghak,

I'm reviewing the header changes since I think there are several improvements
that can be done that will make things more logical and will simplify the code.

My comments below are a mix of suggestions for improvement and brainstorming.

Feel free to ask for clarification if something is not clear.

On 07/31/2015 10:44 AM, Junghak Sung wrote:
 Divide videobuf2-core into core part and v4l2-specific part
  - core part: videobuf2 core related with buffer management  memory 
 allocation
  - v4l2-specific part: v4l2-specific stuff
 
 Signed-off-by: Junghak Sung jh1009.s...@samsung.com
 Signed-off-by: Geunyoung Kim nenggun@samsung.com
 Acked-by: Seung-Woo Kim sw0312@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 ---

snip

 diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
 new file mode 100644
 index 000..dc405da
 --- /dev/null
 +++ b/include/media/videobuf2-core.h
 @@ -0,0 +1,724 @@
 +/*
 + * videobuf2-core.h - Video Buffer 2 Core Framework
 + *
 + * Copyright (C) 2010 Samsung Electronics
 + *
 + * Author: Pawel Osciak pa...@osciak.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation.
 + */
 +#ifndef _MEDIA_VIDEOBUF2_CORE_H
 +#define _MEDIA_VIDEOBUF2_CORE_H
 +
 +#include linux/mm_types.h
 +#include linux/mutex.h
 +#include linux/poll.h
 +#include linux/videodev2.h
 +#include linux/dma-buf.h
 +
 +struct vb2_alloc_ctx;
 +struct vb2_fileio_data;
 +struct vb2_threadio_data;
 +
 +/**
 + * struct vb2_mem_ops - memory handling/memory allocator operations
 + * @alloc:   allocate video memory and, optionally, allocator private data,
 + *   return NULL on failure or a pointer to allocator private,
 + *   per-buffer data on success; the returned private structure
 + *   will then be passed as buf_priv argument to other ops in this
 + *   structure. Additional gfp_flags to use when allocating the
 + *   are also passed to this operation. These flags are from the
 + *   gfp_flags field of vb2_queue.
 + * @put: inform the allocator that the buffer will no longer be used;
 + *   usually will result in the allocator freeing the buffer (if
 + *   no other users of this buffer are present); the buf_priv
 + *   argument is the allocator private per-buffer structure
 + *   previously returned from the alloc callback.
 + * @get_userptr: acquire userspace memory for a hardware operation; used for
 + *USERPTR memory types; vaddr is the address passed to the
 + *videobuf layer when queuing a video buffer of USERPTR type;
 + *should return an allocator private per-buffer structure
 + *associated with the buffer on success, NULL on failure;
 + *the returned private structure will then be passed as buf_priv
 + *argument to other ops in this structure.
 + * @put_userptr: inform the allocator that a USERPTR buffer will no longer
 + *be used.
 + * @attach_dmabuf: attach a shared struct dma_buf for a hardware operation;
 + *  used for DMABUF memory types; alloc_ctx is the alloc context
 + *  dbuf is the shared dma_buf; returns NULL on failure;
 + *  allocator private per-buffer structure on success;
 + *  this needs to be used for further accesses to the buffer.
 + * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF
 + *  buffer is no longer used; the buf_priv argument is the
 + *  allocator private per-buffer structure previously returned
 + *  from the attach_dmabuf callback.
 + * @map_dmabuf: request for access to the dmabuf from allocator; the 
 allocator
 + *   of dmabuf is informed that this driver is going to use the
 + *   dmabuf.
 + * @unmap_dmabuf: releases access control to the dmabuf - allocator is 
 notified
 + * that this driver is done using the dmabuf for now.
 + * @prepare: called every time the buffer is passed from userspace to the
 + *   driver, useful for cache synchronisation, optional.
 + * @finish:  called every time the buffer is passed back from the driver
 + *   to the userspace, also optional.
 + * @vaddr:   return a kernel virtual address to a given memory buffer
 + *   associated with the passed private structure or NULL if no
 + *   such mapping exists.
 + * @cookie:  return allocator specific cookie for a given memory buffer
 + *   associated with the passed private structure or NULL if not
 + *   available.
 + * @num_users:   return the current number of users of a memory buffer;
 + *   return 1 if the videobuf layer (or actually the driver using
 + *   it) is the only user.
 + * @mmap:setup a userspace mapping for a given memory buffer under
 + 

Re: [RFC v3 02/19] media/v4l2-core: add new ioctl VIDIOC_G_DEF_EXT_CTRLS

2015-08-10 Thread Ricardo Ribalda Delgado
Hello Hans, Sakari and Laurent:

I am back from holidays :). Shall I prepare a new patchset with the
suggestion from Hans?

Thanks!

On Mon, Jul 20, 2015 at 4:31 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 07/20/2015 03:52 PM, Ricardo Ribalda Delgado wrote:
 Hello

 I have no preference over the two implementations, but I see an issue
 with this suggestion.


 What happens to out out tree drivers, or drivers that don't support
 this functionality?

 With the ioctl, the user receives a -ENOTTY. So he knows there is
 something wrong with the driver.

 With this class, the driver might interpret this a simple G_VAL and
 return he current value with no way for the user to know what is going
 on.

 Drivers that implement the current API correctly will return an error
 if V4L2_CTRL_WHICH_DEF_VAL was specified. Such drivers will interpret
 the value as a control class, and no control classes in that range exist.
 See also class_check() in v4l2-ctrls.c.

 The exception here is uvc which doesn't have this class check and it will
 just return the current value :-(

 I don't see a way around this, unfortunately.

 Out-of-tree drivers that use the control framework are fine, and I don't
 really care about drivers (out-of-tree or otherwise) that do not use the
 control framework.

 Regarding the new implementation I can make some code next week,
 this week I am 120% busy :)

 Wait until there is a decision first :-)

 It's not a lot of work, I think.

 Regards,

 Hans

 What do you think?
 --
 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





-- 
Ricardo Ribalda
--
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] v4l2-subdev: return -EPIPE instead of -EINVAL in link validate default

2015-08-10 Thread Hans Verkuil
On 07/14/2015 04:32 PM, Laurent Pinchart wrote:
 Hi Hans,
 
 On Tuesday 14 July 2015 16:19:50 Hans Verkuil wrote:
 On 07/13/15 10:03, Sakari Ailus wrote:
 Helen Fornazier wrote:
 On Tue, Jun 30, 2015 at 4:26 PM, Helen Fornazier wrote:
 On Tue, Jun 30, 2015 at 6:19 AM, Sakari Ailus wrote:
 Laurent Pinchart wrote:
 On Monday 29 June 2015 10:23:34 Sakari Ailus wrote:
 Helen Fornazier wrote:
 According to the V4L2 API, the VIDIOC_STREAMON ioctl should return

 EPIPE

 when the pipeline configuration is invalid.

 As the .vidioc_streamon in the v4l2_ioctl_ops usually forwards the

 error

 caused by the v4l2_subdev_link_validate_default (if it is in use),
 it
 should return -EPIPE if it detects a format mismatch in the pipeline
 configuration

 Only link configuration errors have yielded -EPIPE so far, sub-device
 format configuration error has returned -INVAL instead as you
 noticed.

 It should also be noted that while v4l2_subdev_link_validate() will

 return -

 EINVAL in case of error, the only driver that performs custom link

 validation

 (omap3isp/ispccdc.c) will return -EPIPE.

 Good point. That has escaped me until now.

 There are not many sources of -EINVAL while enabling streaming and
 all others are directly caused by the application; I lean towards
 thinking the code is good as it was. The documentation could be
 improved though. It may not be clear which error codes could be
 caused by different conditions.

 The debug level messages from media module
 (drivers/media/media-entity.c) do provide more information if needed,
 albeit this certainly is not an application interface.

 I wonder what others think.

 There's a discrepancy between the implementation and the
 documentation, so at least one of them need to be fixed. -EPIPE would
 be coherent with the documentation and seems appropriately named, but
 another error code would allow userspace to tell link configuration
 and format configuration problems apart.

 That was the original intent, I think.

 Do you think -EINVAL is the most appropriate error code for format
 configuration ? It's already used to indicate that the stream type is
 invalid or that not enough buffers have been allocated, and is also
 used by drivers directly for various purposes.

 That's true, it's been used also for that purpose. At the time this
 certainly was not the primary concern. If you can think of a better
 error code for the purpose (than EINVAL) I'm certainly fine with using
 one. I still think that -EPIPE is worse for telling about incorrect
 format configuration than -EINVAL since it's relatively easy to avoid -
 EINVAL for the documented reasons.

 I'd like just to point out where in the docs EPIPE for format mismatch
 is specified, as it is not described in the streamon page as I thought
 it would, but it is in the subdev page in case anyone is looking for
 it (as I took some time to find it too):

 http://linuxtv.org/downloads/v4l-dvb-apis/subdev.html
 Applications are responsible for configuring coherent parameters on
 the whole pipeline and making sure that connected pads have compatible
 formats. The pipeline is checked for formats mismatch at
 VIDIOC_STREAMON time, and an EPIPE error code is then returned if the
 configuration is invalid

 So maybe the doc should be improved as you already stated.

 I would like to revive this subject.

 Should we change the docs? Change the -EINVAL to -EPIPE, or create
 another error code? What are your opinion?

 I read in the docs of dev-kmsg that EPIPE is returned when messages get
 overwritten, and in other parts of the code EPIPE is returned when there
 is an error in the pipeline communication level while trying to send
 information through the pipe or a pipe broken error.

 But in the error-codes.txt files, the EPIPE error is defined as:
 *EPIPE The pipe type specified in the URB doesn't match the endpoint's
 actual type*

 This exact definition sound USB specific to me.

 Then, if EPIPE is used when types don't match between two endpoints, it
 seems reasonable to me to use EPIPE when formats don't match either. Or
 do types in this context have a specific definition? I don't know much
 about URB, you may be able to judge this better.

 A short recap of the current situation as far as I understand it:

 - MC link validation failure yields EPIPE to the user space,

 - V4L2 sub-device format validation failure generally results in EINVAL,
 except that

 I think that returning EINVAL here is wrong. EINVAL is returned when you
 set e.g. a format and the format is invalid. In this case the format for
 each subdev is perfectly fine, it's just that the sink and source formats
 do not match.

 Rather than returning EINVAL I think ENOLINK would work well here as you
 can't setup a link between two entities. And EPIPE can still be used
 for other higher-level pipeline errors.

 - omap3isp CCDC driver returns EPIPE instead and

 That seems definitely the wrong thing to do.

I think I was ambiguous here. What is wrong