Re: Media Controller patches

2015-12-13 Thread Mauro Carvalho Chehab
Em Fri, 11 Dec 2015 19:05:22 -0200
Mauro Carvalho Chehab  escreveu:

> Em Thu, 10 Dec 2015 18:34:11 -0200
> Mauro Carvalho Chehab  escreveu:
> 
> > Hi,
> > 
> > I've been working during this week to address the issues pointed during
> > the Media Controller really long review process. We should avoid taking
> > so long to review patches in the future, as it is really painful to
> > go back to the already done work 4/5/7 months after the patchsets
> > (yes, there are patches here written 7 months ago that were only
> > very recently reviewed!). Shame on us.
> > 
> > Anyway, The reviewed patches are now at the media-controller topic
> > branch, at the main tree.
> > 
> > I took the care of recompiling and automatically doing runtime tests
> > with KASAN enabled, patch by patch, in order to be sure that the
> > MC is in a sane state. I also ran kmemleak, and was unable to identify
> > any troubles associated with the MC next gen rework.
> > 
> > So, the media-controller topic branch looks sane to me. It should be
> > noticed that there are several items on a TODO list to be addressed
> > before being able to merge this branch back at the master branch.
> > 
> > Please notice that patch 22 was removed from this series:
> > Subject: [media] uapi/media.h: Declare interface types for ALSA
> > 
> > The idea is that this patch should be part of the patches that Shuah
> > will submit and that requires review from the ALSA community before
> > being merged.
> > 
> > Javier and me will start tomorrow on working on the pending items.
> > 
> > My goal is to have everything needed for Kernel 4.5 merge window
> > done up to the next week.
> > 
> > ---
> > 
> > The current TODO list, based on the per-patch review is:
> 
> As far as I checked, all issues at the TODO for Kernel 4.5 were
> already addressed, except for one item:
> 
> - Add documentation for the uAPI.

There are actually 3 other items that were not listed at the TODO:

- Merge of Sakari patches fixing media graph to work with entities
  with ID > 64;

- Use just one counter for the graph ID range. This patch depends on
  Sakari series;

- Merge of Javier patches that split media devnode register from the
  media_device internal register. Not actually a requirement for
  MC next gen, as it fixes an already existing race condition, but it
  will allow almost for free to have topology_version = 0 as the
  start version, with seems to be a good thing to drivers where the
  topology is always static;

I reviewed both Sakari and Javier series this weekend with a few
comments.

> 
> I'll address this last item tomorrow.

Item addressed. I also sent some patches fixing some kernel-doc left overs.
Now, there are only a few set of functions not documented at
media-entity.h:

- the ones that will be touched by Sakari patches;
- two ancillary functions that will be removed when we unify
  the object ID numberspace.

I'll review those remaining items after merging Sakari's series.

> 
> The patches that addressed the TODO list were sent already to the ML,
> on a few independent patch series.
> 
> They're all (including the Javier ones) applied on my experimental
> tree at branch media-controller-rc3:
>   git://linuxtv.org/mchehab/experimental.git media-controller-rc3
> 
> The userspace testing tool was also modified for the MC next gen,
> at the branch mc-next-gen-v2:
>   git://linuxtv.org/mchehab/experimental-v4l-utils.git mc-next-gen-v2
> 
> 
> 
> Please let me know if something else got missed ASAP, as I'll be 
> addressing any missing stuff during this weekend.
> 
> My goal is to merge those patches at the main development branch
> this Monday.
> 
> NOTE:
> 
> 
> The TODO list are hosted at: https://etherpad.fr
> 
>   The original one is on the above site, at: /p/mc-v2-todo
>   And we added a new version on the same site, at: /p/mc-v2-todo-v2
> 
> Things that got postponed to other Kernel versions:
> ===
> 
> 1) Sakari: Rethink about media-entity.h name;
> 2) Laurent: do a non-hacking version of the pad/subdev switch logic (waiting 
> for Laurent's comment on this one);
> 3) Should address on a later series the changes to remove 
> MEDIA_ENT_T_SUBDEV_UNKNOWN;
> 4) Laurent: All exported API functions need kerneldoc. (most are. There are a 
> few less used that needs documentation, like the __foo functions);

This will actually be addressed after applying Sakari's patch series.

> 5) Laurent: remove major/minor fields from entities
> 
> 6) Remove unused fields from media_entity (major, minor, num_links, 
> num_backlinks, num_pads)
> 7) dynamic entity/interface/link creation and removal;
> 8) SETUP_LINK_V2 with dynamic support;
> 9) dynamic pad creation and removal (needed?);
> 10) multiple function per entity support;
> 11) indirect interface links support;
> 12) MC properties API.
> 
> Userspace TODO:
> ==
> 
> 1) Create a library with 

[PATCH 3/4] [media] media-devnode: move kernel-doc documentation to the header

2015-12-13 Thread Mauro Carvalho Chehab
As we're using the headers file only for documentation, move the
two kernel-doc macros to the header, and fix it to avoid
warnings.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/media-devnode.c | 24 
 include/media/media-devnode.h | 27 +++
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index ebf9626e5ae5..cea35bf20011 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -217,20 +217,6 @@ static const struct file_operations media_devnode_fops = {
.llseek = no_llseek,
 };
 
-/**
- * media_devnode_register - register a media device node
- * @mdev: media device node structure we want to register
- *
- * The registration code assigns minor numbers and registers the new device 
node
- * with the kernel. An error is returned if no free minor number can be found,
- * or if the registration of the device node fails.
- *
- * Zero is returned on success.
- *
- * Note that if the media_devnode_register call fails, the release() callback 
of
- * the media_devnode structure is *not* called, so the caller is responsible 
for
- * freeing any data.
- */
 int __must_check media_devnode_register(struct media_devnode *mdev,
struct module *owner)
 {
@@ -285,16 +271,6 @@ error:
return ret;
 }
 
-/**
- * media_devnode_unregister - unregister a media device node
- * @mdev: the device node to unregister
- *
- * This unregisters the passed device. Future open calls will be met with
- * errors.
- *
- * This function can safely be called if the device node has never been
- * registered or has already been unregistered.
- */
 void media_devnode_unregister(struct media_devnode *mdev)
 {
/* Check if mdev was ever registered at all */
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index 17ddae32060d..77e9d3159be8 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -86,8 +86,35 @@ struct media_devnode {
 /* dev to media_devnode */
 #define to_media_devnode(cd) container_of(cd, struct media_devnode, dev)
 
+/**
+ * media_devnode_register - register a media device node
+ *
+ * @mdev: media device node structure we want to register
+ * @owner: should be filled with %THIS_MODULE
+ *
+ * The registration code assigns minor numbers and registers the new device 
node
+ * with the kernel. An error is returned if no free minor number can be found,
+ * or if the registration of the device node fails.
+ *
+ * Zero is returned on success.
+ *
+ * Note that if the media_devnode_register call fails, the release() callback 
of
+ * the media_devnode structure is *not* called, so the caller is responsible 
for
+ * freeing any data.
+ */
 int __must_check media_devnode_register(struct media_devnode *mdev,
struct module *owner);
+
+/**
+ * media_devnode_unregister - unregister a media device node
+ * @mdev: the device node to unregister
+ *
+ * This unregisters the passed device. Future open calls will be met with
+ * errors.
+ *
+ * This function can safely be called if the device node has never been
+ * registered or has already been unregistered.
+ */
 void media_devnode_unregister(struct media_devnode *mdev);
 
 static inline struct media_devnode *media_devnode_data(struct file *filp)
-- 
2.5.0

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


[PATCH 1/4] [media] media-entity.h: Document some ancillary functions

2015-12-13 Thread Mauro Carvalho Chehab
Add a basic documentation for most ancillary functions.

Signed-off-by: Mauro Carvalho Chehab 
---
 include/media/media-entity.h | 58 +++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index d073b205e6a6..81aca1f5a09a 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -238,11 +238,21 @@ struct media_intf_devnode {
u32 minor;
 };
 
+/**
+ * media_entity_id() - return the media entity graph object id
+ *
+ * @entity:pointer to entity
+ */
 static inline u32 media_entity_id(struct media_entity *entity)
 {
return entity->graph_obj.id;
 }
 
+/**
+ * media_type() - return the media object type
+ *
+ * @gobj:  pointer to the media graph object
+ */
 static inline enum media_gobj_type media_type(struct media_gobj *gobj)
 {
return gobj->id >> MEDIA_BITS_PER_LOCAL_ID;
@@ -263,6 +273,15 @@ static inline u32 media_gobj_gen_id(enum media_gobj_type 
type, u32 local_id)
return id;
 }
 
+/**
+ * is_media_entity_v4l2_io() - identify if the entity main function
+ *is a V4L2 I/O
+ *
+ * @entity:pointer to entity
+ *
+ * Return: true if the entity main function is one of the V4L2 I/O types
+ * (video, VBI or SDR radio); false otherwise.
+ */
 static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
 {
if (!entity)
@@ -278,6 +297,16 @@ static inline bool is_media_entity_v4l2_io(struct 
media_entity *entity)
}
 }
 
+/**
+ * is_media_entity_v4l2_subdev - return true if the entity main function is
+ *  associated with the V4L2 API subdev usage
+ *
+ * @entity:pointer to entity
+ *
+ * This is an ancillary function used by subdev-based V4L2 drivers.
+ * It checks if the entity function is one of functions used by a V4L2 subdev,
+ * e. g. camera-relatef functions, analog TV decoder, TV tuner, V4L2 DSPs.
+ */
 static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity)
 {
if (!entity)
@@ -652,16 +681,43 @@ struct media_link *
 __must_check media_create_intf_link(struct media_entity *entity,
struct media_interface *intf,
u32 flags);
+/**
+ * __media_remove_intf_link() - remove a single interface link
+ *
+ * @link:  pointer to _link.
+ *
+ * Note: this is an unlocked version of media_remove_intf_link()
+ */
 void __media_remove_intf_link(struct media_link *link);
+
+/**
+ * media_remove_intf_link() - remove a single interface link
+ *
+ * @link:  pointer to _link.
+ *
+ * Note: prefer to use this one, instead of __media_remove_intf_link()
+ */
 void media_remove_intf_link(struct media_link *link);
+
+/**
+ * __media_remove_intf_links() - remove all links associated with an interface
+ *
+ * @intf:  pointer to _interface
+ *
+ * Note: this is an unlocked version of media_remove_intf_links().
+ */
 void __media_remove_intf_links(struct media_interface *intf);
 /**
  * media_remove_intf_links() - remove all links associated with an interface
  *
  * @intf:  pointer to _interface
  *
- * Note: this is called automatically when an entity is unregistered via
+ * Notes:
+ *
+ * this is called automatically when an entity is unregistered via
  * media_device_register_entity() and by media_devnode_remove().
+ *
+ * Prefer to use this one, instead of __media_remove_intf_links().
  */
 void media_remove_intf_links(struct media_interface *intf);
 
-- 
2.5.0

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


[PATCH 4/4] [media] media-devnode.h: document the remaining struct/functions

2015-12-13 Thread Mauro Carvalho Chehab
There is one struct and two functions that were not documented.
Add the corresponding kernel-doc documentation for them.

Signed-off-by: Mauro Carvalho Chehab 
---
 include/media/media-devnode.h | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index 77e9d3159be8..fe42f08e72bd 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -40,6 +40,20 @@
  */
 #define MEDIA_FLAG_REGISTERED  0
 
+/**
+ * struct media_file_operations - Media device file operations
+ *
+ * @owner: should be filled with %THIS_MODULE
+ * @read: pointer to the function that implements read() syscall
+ * @write: pointer to the function that implements write() syscall
+ * @poll: pointer to the function that implements poll() syscall
+ * @ioctl: pointer to the function that implements ioctl() syscall
+ * @compat_ioctl: pointer to the function that will handle 32 bits userspace
+ * calls to the the ioctl() syscall on a Kernel compiled with 64 bits.
+ * @open: pointer to the function that implements open() syscall
+ * @release: pointer to the function that will release the resources allocated
+ * by the @open function.
+ */
 struct media_file_operations {
struct module *owner;
ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
@@ -53,7 +67,7 @@ struct media_file_operations {
 
 /**
  * struct media_devnode - Media device node
- * @fops:  pointer to struct media_file_operations with media device ops
+ * @fops:  pointer to struct _file_operations with media device ops
  * @dev:   struct device pointer for the media controller device
  * @cdev:  struct cdev pointer character device
  * @parent:parent device
@@ -117,11 +131,22 @@ int __must_check media_devnode_register(struct 
media_devnode *mdev,
  */
 void media_devnode_unregister(struct media_devnode *mdev);
 
+/**
+ * media_devnode_data - returns a pointer to the _devnode
+ *
+ * @filp: pointer to struct 
+ */
 static inline struct media_devnode *media_devnode_data(struct file *filp)
 {
return filp->private_data;
 }
 
+/**
+ * media_devnode_is_registered - returns true if _devnode is registered;
+ * false otherwise.
+ *
+ * @mdev: pointer to struct _devnode.
+ */
 static inline int media_devnode_is_registered(struct media_devnode *mdev)
 {
return test_bit(MEDIA_FLAG_REGISTERED, >flags);
-- 
2.5.0

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


[PATCH 2/4] [media] media-device.h: document the last functions

2015-12-13 Thread Mauro Carvalho Chehab
Add kernel-doc documentation for media_device_get_devres and
media_device_find_devres.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/media-device.c |  7 ---
 include/media/media-device.h | 22 ++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index c12481c753a0..ca16bd3091bd 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -689,10 +689,6 @@ static void media_device_release_devres(struct device 
*dev, void *res)
 {
 }
 
-/*
- * media_device_get_devres() - get media device as device resource
- * creates if one doesn't exist
-*/
 struct media_device *media_device_get_devres(struct device *dev)
 {
struct media_device *mdev;
@@ -709,9 +705,6 @@ struct media_device *media_device_get_devres(struct device 
*dev)
 }
 EXPORT_SYMBOL_GPL(media_device_get_devres);
 
-/*
- * media_device_find_devres() - find media device as device resource
-*/
 struct media_device *media_device_find_devres(struct device *dev)
 {
return devres_find(dev, media_device_release_devres, NULL, NULL);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index b0594be5d631..0c3611d1abea 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -449,7 +449,29 @@ int __must_check media_device_register_entity(struct 
media_device *mdev,
  * the driver if required.
  */
 void media_device_unregister_entity(struct media_entity *entity);
+
+/**
+ * media_device_get_devres() - get media device as device resource
+ * creates if one doesn't exist
+ *
+ * @dev: pointer to struct 
+ *
+ * Sometimes, the media controller _device needs to be shared by more
+ * than one driver. This function adds support for that, by dynamically
+ * allocating the _device and allowing it to be obtained from the
+ * struct  associated with the common device where all sub-device
+ * components belong. So, for example, on an USB device with multiple
+ * interfaces, each interface may be handled by a separate per-interface
+ * drivers. While each interface have its own , they all share a
+ * common  associated with the hole USB device.
+ */
 struct media_device *media_device_get_devres(struct device *dev);
+
+/**
+ * media_device_find_devres() - find media device as device resource
+ *
+ * @dev: pointer to struct 
+ */
 struct media_device *media_device_find_devres(struct device *dev);
 
 /* Iterate over all entities. */
-- 
2.5.0

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


Re: [PATCH v2 00/32] VSP: Add R-Car Gen3 support

2015-12-13 Thread Khiem Nguyen

On 12/14/2015 1:53 AM, Laurent Pinchart wrote:

Hello Khiem,

On Friday 11 December 2015 15:43:27 Khiem Nguyen wrote:

On 12/6/2015 5:54 AM, Laurent Pinchart wrote:

On Saturday 05 December 2015 11:57:49 Geert Uytterhoeven wrote:

On Sat, Dec 5, 2015 at 3:12 AM, Laurent Pinchart wrote:

This patch set adds support for the Renesas R-Car Gen3 SoC family to the
VSP1 driver. The large number of patches is caused by a change in the
display controller architecture that makes usage of the VSP mandatory as
the display controller has lost the ability to read data from memory.

Patch 01/32 to 27/32 prepare for the implementation of an API exported
to the DRM driver in patch 28/32. Patches 31/32 enables support for the
R-Car Gen3 family, and patch 32/32 finally enhances perfomances by
implementing support for display lists.

The major change compared to v1 is the usage of the IP version register
instead of DT properties to configure device parameters such as the
number of BRU inputs or the availability of the BRU.


Thanks for your series!

As http://git.linuxtv.org/pinchartl/media.git/tag/?id=vsp1-kms-20151112
is getting old, and has lots of conflicts with recent -next, do you plan
to publish this in a branch, and a separate branch for integration, to
ease integration in renesas-drivers?

Alternatively, I can just import the series you posted, but having the
broken-out integration part would be nice.


The issue I'm facing is that there's more than just two series. Beside the
base VSP patches from this series, I have a series of DRM patches that
depend on this one, a series of V4L2 core patches, another series of VSP
patches that I still need to finish and a bunch of integration patches.
As some of these have dependencies on H3 CCF support that hasn't landed
in Simon's tree yet, I have merged your topic/cpg-mssr-v6 and
topic/r8a7795-drivers-sh-v1 branches into my tree for development.

I could keep all series in separate branches and merge the two topic
branches last, but that's not very handy during development when I have
to continuously rebase my patches. Is there a way I could handle this
that would make your life easier while not making mine more difficult ?

In the meantime I've pushed vsp1-kms-20151206 to
git://linuxtv.org/pinchartl/media.git.


I failed to confirm DU (VGA port) with above branch.

To make DU (VGA port) work,
it seems I need to merge more branches like topic/cpg-mssr-v6
and topic/r8a7795-drivers-sh-v1 branches from renesas-drivers repo, is
it correct ?


The

git://linuxtv.org/pinchartl/media.git vsp1-kms-20151206

branch already contains topic/r8a7795-drivers-sh-v1 and topic/cpg-mssr-v6.
What is the failure you're seeing ?



Honestly, I tried some configurations but there's no output to VGA port 
(the display is blank).

1. The default 'defconfig' of this tag.
2. Enable DRM, CMA, RCAR_DU
3. Enable DRM, CMA, RCAR_DU, VSP1
 -> I got kernel build error in file 
drivers/media/platform/vsp1/vsp1_pipe.c


I also tried another tag vsp1-kms-request-20151206 which have more 
patches on top of vsp1-kms-20151206.

After enabling all DRM, CMA, RCAR_DU, VSP1, kernel build is OK.
Got a 'green color' display and no kernel log output.

With the same u-boot and IPL, it's able to see display output normally 
if using Gen3 BSP version.

Therefore, I guess it's no impact to DU operation.

Please let me know your procedure to test DU operation with your tag.

--
Best regards,
KHIEM Nguyen

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


af9015 not supported anymore ???

2015-12-13 Thread oddebian
Hello,

I have investigated a little further and it looks like the af9015 is not
working correctly with any linux kernels (32 or 64 bits) >= 3, although
it seems to be recognised (firmware uploaded).

Any idea about how to debug the thing ?

OD

 Message transféré 
> De: oddebian 
> À: linux-media@vger.kernel.org
> Objet: Problem with avermedia Volar Black HD (af9015) : recognised but
> not scanning
> Date: Wed, 02 Dec 2015 20:36:28 +0100
> 
> Hi,
> 
> I have an old avermedia Volar Black HD (af9015) that still works
> pretty well in windows 8 (it scans the whole dvb-t muxes in less than
> one minute and the image is perfect even in HD).
> When I try it on linux, it takes 12 minutes to scan with w_scan, and
> despite showing lines such as :
> updating transponder:
>(QAM_64   f = 4294967 kHz I999B8C999D0T8G32Y0) 0x405A
> to (QAM_64   f = 4294967 kHz I999B8C999D0T8G8Y0) 0x405A
> undefined coderate HP
> in the end, it says :
> 
> tune to: QAM_AUTO f = 482000 kHz I999B8C999D999T999G999Y999 
> (time: 12:13) --no signal--
> tune to: QAM_AUTO f = 482000 kHz I999B8C999D999T999G999Y999  (no
> signal)
> (time: 12:14) --no signal--
> tune to: QAM_64   f = 4294967 kHz I999B8C999D0T8G8Y0 
> (time: 12:16) skipped: (freq 4294967286 unsupported by driver)
> tune to: QAM_AUTO f = 482166 kHz I999B8C999D999T999G999Y999 
> (time: 12:16) --no signal--
> tune to: QAM_AUTO f = 482166 kHz I999B8C999D999T999G999Y999  (no
> signal)
> (time: 12:17) --no signal--
> 
> ERROR: Sorry - i couldn't get any working frequency/transponder
> Nothing to scan!!
> 
> 
> The problem is the same on my destop pc (debian 8, kernel
> 3.16.0-4-amd64) and on a Raspberry 1 (Linux osmc 4.2.3-3-osmc, or
> openelec).
> I tried also with tvheadend, but scan does not work either.
> 
> The firmware is correct and installed in /lib/firmware.
> Dmesg shows that the usb device is well detected, with no errors :
> [   13.846959] usb 1-5: dvb_usb_v2: found a 'AverMedia AVerTV Volar
> Black HD (A850)' in cold state
> [   13.847467] usb 1-5: firmware: direct-loading firmware
> dvb-usb-af9015.fw
> [   13.847474] usb 1-5: dvb_usb_v2: downloading firmware from file
> 'dvb-usb-af9015.fw'
> [   13.917176] usb 1-5: dvb_usb_v2: found a 'AverMedia AVerTV Volar
> Black HD (A850)' in warm state
> [   14.327175] usb 1-5: dvb_usb_v2: will pass the complete MPEG2
> transport stream to the software demuxer
> [   14.335086] usb 1-5: DVB: registering adapter 0 frontend 0 (Afatech
> AF9013)...
> [   14.345704] usb 1-5: dvb_usb_v2: 'AverMedia AVerTV Volar Black HD
> (A850)' successfully initialized and connected
> [   14.345795] usbcore: registered new interface driver dvb_usb_af9015
> 
> And lsusb :
> Bus 001 Device 003: ID 07ca:850a AVerMedia Technologies, Inc. AverTV
> Volar Black HD (A850)
> 
> I must say it is very frustrating to see a device still supported in
> windows 8, and working perfectly, but not working anymore in linux
> despite stated as supported in
> http://www.linuxtv.org/wiki/index.php/AVerTV_Volar_Black_HD_%28A850%29
> 
> Thanks in advance for any idea that could help !
> OD
> 


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


[PATCH] media: soc_camera: rcar_vin: Add R-Car Gen3 support

2015-12-13 Thread Yoshihiro Kaneko
From: Yoshihiko Mori 

Add chip identification for R-Car Gen3.

Signed-off-by: Yoshihiko Mori 
Signed-off-by: Yoshihiro Kaneko 
---

This patch is against master branch of linuxtv.org/media_tree.git.

 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
 drivers/media/platform/soc_camera/rcar_vin.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 9dafe6b..619193c 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -6,6 +6,7 @@ family of devices. The current blocks are always slaves and 
suppot one input
 channel which can be either RGB, YUYV or BT656.
 
  - compatible: Must be one of the following
+   - "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7794" for the R8A7794 device
- "renesas,vin-r8a7793" for the R8A7793 device
- "renesas,vin-r8a7791" for the R8A7791 device
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c 
b/drivers/media/platform/soc_camera/rcar_vin.c
index 5d90f39..29e7ca4 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -143,6 +143,7 @@
 #define RCAR_VIN_BT656 (1 << 3)
 
 enum chip_id {
+   RCAR_GEN3,
RCAR_GEN2,
RCAR_H1,
RCAR_M1,
@@ -1846,6 +1847,7 @@ static struct soc_camera_host_ops rcar_vin_host_ops = {
 
 #ifdef CONFIG_OF
 static const struct of_device_id rcar_vin_of_table[] = {
+   { .compatible = "renesas,vin-r8a7795", .data = (void *)RCAR_GEN3 },
{ .compatible = "renesas,vin-r8a7794", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,vin-r8a7793", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,vin-r8a7791", .data = (void *)RCAR_GEN2 },
-- 
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: [PATCH] media: soc_camera: rcar_vin: Add R-Car Gen3 support

2015-12-13 Thread Sergei Shtylyov

On 12/13/2015 06:27 PM, Yoshihiro Kaneko wrote:


From: Yoshihiko Mori 

Add chip identification for R-Car Gen3.

Signed-off-by: Yoshihiko Mori 
Signed-off-by: Yoshihiro Kaneko 

[...]

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c 
b/drivers/media/platform/soc_camera/rcar_vin.c
index 5d90f39..29e7ca4 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -143,6 +143,7 @@
  #define RCAR_VIN_BT656(1 << 3)

  enum chip_id {
+   RCAR_GEN3,
RCAR_GEN2,
RCAR_H1,
RCAR_M1,
@@ -1846,6 +1847,7 @@ static struct soc_camera_host_ops rcar_vin_host_ops = {

  #ifdef CONFIG_OF
  static const struct of_device_id rcar_vin_of_table[] = {
+   { .compatible = "renesas,vin-r8a7795", .data = (void *)RCAR_GEN3 },


   I don't see where this is checked in the driver. Shouldn't we just use gen2?

MBR, Sergei

--
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] adv7604: add direct interrupt handling

2015-12-13 Thread Laurent Pinchart
Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:28 Ulrich Hecht wrote:
> When probed from device tree, the i2c client driver can handle the
> interrupt on its own.
> 
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/media/i2c/adv7604.c | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index c2df7e8..129009f 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1971,6 +1972,16 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32
> status, bool *handled) return 0;
>  }
> 
> +static irqreturn_t adv76xx_irq_handler(int irq, void *devid)
> +{
> + struct adv76xx_state *state = devid;
> + bool handled;
> +
> + adv76xx_isr(>sd, 0, );
> +
> + return handled ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
>  static int adv76xx_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
> {
>   struct adv76xx_state *state = to_state(sd);
> @@ -2833,8 +2844,7 @@ static int adv76xx_parse_dt(struct adv76xx_state
> *state) state->pdata.op_656_range = 1;
>   }
> 
> - /* Disable the interrupt for now as no DT-based board uses it. */
> - state->pdata.int1_config = ADV76XX_INT1_CONFIG_DISABLED;
> + state->pdata.int1_config = ADV76XX_INT1_CONFIG_ACTIVE_LOW;
> 
>   /* Use the default I2C addresses. */
>   state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
> @@ -3224,6 +3234,16 @@ static int adv76xx_probe(struct i2c_client *client,
>   v4l2_info(sd, "%s found @ 0x%x (%s)\n", client->name,
>   client->addr << 1, client->adapter->name);
> 
> + if (client->irq) {
> + err = devm_request_threaded_irq(>dev,
> + client->irq,
> + NULL, adv76xx_irq_handler,
> + IRQF_TRIGGER_LOW|IRQF_ONESHOT,

While you hardcode the type to active low in int1_config, a board could have 
an inverter on the irq line, making it active high when it reaches the 
interrupt controller.

Unless I'm mistaken this will be handled properly if you set the correct 
interrupt level in DT, so you can just remove the IRQF_TRIGGER_LOW flag here.

Apart from that,

Reviewed-by: Laurent Pinchart 

> + dev_name(>dev), state);
> + if (err)
> + goto err_entity;
> + }
> +
>   err = v4l2_async_register_subdev(sd);
>   if (err)
>   goto err_entity;

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v2 0/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

2015-12-13 Thread Laurent Pinchart
Hi Marek,

Thank you for the patches.

On Wednesday 09 December 2015 14:58:15 Marek Szyprowski wrote:
> Hello,
> 
> This patchset finally perform cleanup of custom code in s5p-mfc codec
> driver. The first part is removal of custom, driver specific code for
> intializing and handling of reserved memory. Instead, a generic code for
> reserved memory regions is used.

Should you update the reserved memory bindings documentation 
(Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt) to 
document usage of the memory-region-names property ?

> Then, once it is done, the proper setup
> of DMA parameters (max segment size) is applied for all multimedia
> devices found on Exynos SoCs to let them properly handle shared buffers
> mapped into contiguous DMA address space. The last patch adds support
> for IOMMU to MFC driver. Some additional code is needed because of
> specific requirements of MFC device firmware (see patch 7 for more
> details). When no IOMMU is available, the code fallbacks to generic
> reserved memory regions.
> 
> After applying this patchset, MFC device works correctly when IOMMU is
> either enabled or disabled.
> 
> Patches have been tested on top of linux-next from 20151207. I would
> prefer to merge patches 1-2 via Samsung tree and patches 3-7 via media
> tree (there are no compile-time dependencies between patches 1-2 and
> 3-7). Patches have been tested on Odroid U3 (Exynos 4412 based) and
> Odroid XU3 (Exynos 5422 based) boards.
> 
> Best regards
> Marek Szyprowski
> Samsung R Institute Poland
> 
> 
> Changelog:
> v2:
> - reworked of_reserved_mem_init* functions on request from Rob Herring,
>   added separate index and name based selection of reserved region
> - adapted for of_reserved_mem_init* related changes
> 
> v1: https://www.mail-archive.com/linux-media@vger.kernel.org/msg94100.html
> - initial version of another approach for this problem, rewrote driver code
>   for new reserved memory bindings, which finally have been merged some
>   time ago
> 
> v0:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/189259.ht
> ml - old patchset solving the same problem, abandoned due to other tasks and
> long time of merging reserved memory bindings and support code for it
> 
> Patch summary:
> 
> Marek Szyprowski (7):
>   ARM: Exynos: convert MFC device to generic reserved memory bindings
>   ARM: dts: exynos4412-odroid*: enable MFC device
>   of: reserved_mem: add support for named reserved mem nodes
>   media: vb2-dma-contig: add helper for setting dma max seg size
>   media: set proper max seg size for devices on Exynos SoCs
>   media: s5p-mfc: replace custom reserved memory init code with generic
> one
>   media: s5p-mfc: add iommu support
> 
>  .../devicetree/bindings/media/s5p-mfc.txt  |  16 +--
>  arch/arm/boot/dts/exynos4210-origen.dts|  22 ++-
>  arch/arm/boot/dts/exynos4210-smdkv310.dts  |  22 ++-
>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi|  24 
>  arch/arm/boot/dts/exynos4412-origen.dts|  22 ++-
>  arch/arm/boot/dts/exynos4412-smdk4412.dts  |  22 ++-
>  arch/arm/boot/dts/exynos5250-arndale.dts   |  22 ++-
>  arch/arm/boot/dts/exynos5250-smdk5250.dts  |  22 ++-
>  arch/arm/boot/dts/exynos5250-spring.dts|  22 ++-
>  arch/arm/boot/dts/exynos5420-arndale-octa.dts  |  22 ++-
>  arch/arm/boot/dts/exynos5420-smdk5420.dts  |  22 ++-
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |  22 ++-
>  arch/arm/mach-exynos/Makefile  |   2 -
>  arch/arm/mach-exynos/exynos.c  |  19 ---
>  arch/arm/mach-exynos/mfc.h |  16 ---
>  arch/arm/mach-exynos/s5p-dev-mfc.c |  94 -
>  drivers/media/platform/exynos-gsc/gsc-core.c   |   1 +
>  drivers/media/platform/exynos4-is/fimc-core.c  |   1 +
>  drivers/media/platform/exynos4-is/fimc-is.c|   1 +
>  drivers/media/platform/exynos4-is/fimc-lite.c  |   1 +
>  drivers/media/platform/s5p-g2d/g2d.c   |   1 +
>  drivers/media/platform/s5p-jpeg/jpeg-core.c|   1 +
>  drivers/media/platform/s5p-mfc/s5p_mfc.c   | 153 ++
>  drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h |  79 +++
>  drivers/media/platform/s5p-tv/mixer_video.c|   1 +
>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  15 ++
>  drivers/of/of_reserved_mem.c   | 104 +++---
>  include/linux/of_reserved_mem.h|  31 -
>  include/media/videobuf2-dma-contig.h   |   1 +
>  29 files changed, 533 insertions(+), 248 deletions(-)
>  delete mode 100644 arch/arm/mach-exynos/mfc.h
>  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
>  create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to 

Re: [PATCH] libmediactl.c: add poor man's udev support

2015-12-13 Thread Laurent Pinchart
Hi Hans,

Thank you for the patch.

On Thursday 10 December 2015 11:44:43 Hans Verkuil wrote:
> If libudev is not available (android!),

It's time for Android to grow up and get rid of their LGPL hatred... That 
might actually happen as a result of the Chrome/Android merge that seems to be 
planned, as well as from the Android One project that will require something 
like libudev (although knowing the Android folks they might decide to 
reimplement it).

> then just try to find the device in /dev. It's a poor man's solution, but it
> is better than nothing.

Can't we assume that, on such systems, the device will always be named 
/dev/video* or /dev/v4l-subdev* ? If so we could lower the runtime impact by 
only checking the /sys/class/video4linux/video* and 
/sys/class/video4linux/v4l-subdev* entries to match the device minor and 
major, and use the index from the sysfs entry to create the /dev path.

My enumeration library (which I need to revive) implements that, feel free to 
use the code if it can be useful.

git://git.ideasonboard.org/media-enum.git

> Signed-off-by: Hans Verkuil 
> 
> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> index 4a82d24..1577783 100644
> --- a/utils/media-ctl/libmediactl.c
> +++ b/utils/media-ctl/libmediactl.c
> @@ -441,7 +441,10 @@ static int media_get_devname_udev(struct udev *udev,
>   return ret;
>  }
> 
> -#else/* HAVE_LIBUDEV */
> +#else
> +
> +#include 
> +#include 
> 
>  struct udev;
> 
> @@ -449,10 +452,36 @@ static inline int media_udev_open(struct udev **udev)
> { return 0; }
> 
>  static inline void media_udev_close(struct udev *udev) { }
> 
> -static inline int media_get_devname_udev(struct udev *udev,
> - struct media_entity *entity)
> +static int media_get_devname_udev(struct udev *udev,
> +   struct media_entity *entity)
>  {
> - return -ENOTSUP;
> + DIR *dp;
> + struct dirent *ep;
> + dev_t devnum;
> +
> + dp = opendir("/dev");
> + if (dp == NULL) {
> + media_dbg(entity->media, "couldn't open /dev\n");
> + return -ENODEV;
> + }
> + devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor);
> + while ((ep = readdir(dp))) {
> + struct stat st;
> + char fname[256];
> +
> + snprintf(fname, sizeof(fname) - 1, "/dev/%s", ep->d_name);
> + fname[sizeof(fname) - 1] = 0;
> + stat(fname, );
> + if ((st.st_mode & S_IFMT) != S_IFCHR)
> + continue;
> + if (st.st_rdev == devnum) {
> + strncpy(entity->devname, fname, 
> sizeof(entity->devname));
> +entity->devname[sizeof(entity->devname) - 1] =
> '\0';
> + return 0;
> + }
> + }
> + closedir(dp);
> + return -ENODEV;
>  }
> 
>  #endif   /* HAVE_LIBUDEV */

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v2 03/22] media: Add an API to manage entity enumerations

2015-12-13 Thread Sakari Ailus
Hi Mauro,

Thanks for the comments!

Mauro Carvalho Chehab wrote:
> Hi Sakari,
> 
> Em Sun, 29 Nov 2015 21:20:04 +0200
> Sakari Ailus  escreveu:
> 
>> From: Sakari Ailus 
>>
>> This is useful in e.g. knowing whether certain operations have already
>> been performed for an entity. The users include the framework itself (for
>> graph walking) and a number of drivers.
> 
> Please use better names on the vars. See below for details.
> 
>>
>> Signed-off-by: Sakari Ailus 
>> ---
>>  drivers/media/media-entity.c |  39 +
>>  include/media/media-device.h |  14 +
>>  include/media/media-entity.h | 136 
>> ---
>>  3 files changed, 181 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>> index d11f440..fceaf44 100644
>> --- a/drivers/media/media-entity.c
>> +++ b/drivers/media/media-entity.c
>> @@ -213,6 +213,45 @@ void media_gobj_remove(struct media_gobj *gobj)
>>  }
>>  
>>  /**
>> + * __media_entity_enum_init - Initialise an entity enumeration
>> + *
>> + * @e: Entity enumeration to be initialised
>> + * @idx_max: Maximum number of entities in the enumeration
>> + *
>> + * Returns zero on success or a negative error code.
>> + */
> 
> We're using kernel-doc macros only at the header files. Please
> move those macros to there.

Oops. Will fix.

> 
>> +int __media_entity_enum_init(struct media_entity_enum *e, int idx_max)
> 
> using "e" as a var is not nice, specially on a public header.
> 
> I would use "ent_enum" instead for media_entity_enum pointers. This
> applies everywhere on this patch series.

I'm fine with that suggestion, I'll change to use ent_enum instead
(where there's no more specific purpose for the variable / field).

> 
>> +{
>> +if (idx_max > MEDIA_ENTITY_ENUM_MAX_ID) {
>> +e->e = kcalloc(DIV_ROUND_UP(idx_max, BITS_PER_LONG),
>> +   sizeof(long), GFP_KERNEL);
>> +if (!e->e)
>> +return -ENOMEM;
>> +} else {
>> +e->e = e->__e;
> 
> This is crap! I can't tell what the above code is doing,
> as the var names don't give any clue!

In retrospect, the names could perhaps have been more descriptive.
There's no need to be angry at the letter "e" however, it's just doing
its job...

> 
>> +}
>> +
>> +bitmap_zero(e->e, idx_max);
> 
> Ok, here, there's a hint that one of the "e" is a bitmap, while
> the other is a struct... Weird, and very easy to do wrong things!
> 
> Please name the bitmap as "ent_enum_bmap" or something like that.

Fine with that.

> 
>> +e->idx_max = idx_max;
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__media_entity_enum_init);
>> +
>> +/**
>> + * media_entity_enum_cleanup - Release resources of an entity enumeration
>> + *
>> + * @e: Entity enumeration to be released
>> + */
>> +void media_entity_enum_cleanup(struct media_entity_enum *e)
>> +{
>> +if (e->e != e->__e)
>> +kfree(e->e);
>> +e->e = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(media_entity_enum_cleanup);
>> +
>> +/**
>>   * media_entity_init - Initialize a media entity
>>   *
>>   * @num_pads: Total number of sink and source pads.
>> diff --git a/include/media/media-device.h b/include/media/media-device.h
>> index c0e1764..2d46c66 100644
>> --- a/include/media/media-device.h
>> +++ b/include/media/media-device.h
>> @@ -110,6 +110,20 @@ struct media_device {
>>  /* media_devnode to media_device */
>>  #define to_media_device(node) container_of(node, struct media_device, 
>> devnode)
>>  
>> +/**
>> + * media_entity_enum_init - Initialise an entity enumeration
>> + *
>> + * @e: Entity enumeration to be initialised
>> + * @mdev: The related media device
>> + *
>> + * Returns zero on success or a negative error code.
>> + */
>> +static inline __must_check int media_entity_enum_init(
>> +struct media_entity_enum *e, struct media_device *mdev)
>> +{
>> +return __media_entity_enum_init(e, mdev->entity_internal_idx_max + 1);
>> +}
>> +
>>  void media_device_init(struct media_device *mdev);
>>  void media_device_cleanup(struct media_device *mdev);
>>  int __must_check __media_device_register(struct media_device *mdev,
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index d3d3a39..5a0339a 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -23,7 +23,7 @@
>>  #ifndef _MEDIA_ENTITY_H
>>  #define _MEDIA_ENTITY_H
>>  
>> -#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -71,6 +71,30 @@ struct media_gobj {
>>  struct list_headlist;
>>  };
>>  
>> +#define MEDIA_ENTITY_ENUM_MAX_DEPTH 16
>> +#define MEDIA_ENTITY_ENUM_MAX_ID64
>> +
>> +/*
>> + * The number of pads can't be bigger than the number of entities,
>> + * as the worse-case scenario is to have one entity linked up to
>> + * MEDIA_ENTITY_ENUM_MAX_ID - 1 entities.
>> 

Re: [PATCH v2 03/22] media: Add an API to manage entity enumerations

2015-12-13 Thread Mauro Carvalho Chehab
Em Mon, 14 Dec 2015 00:12:08 +0200
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> Thanks for the comments!
> 
> Mauro Carvalho Chehab wrote:
> > Hi Sakari,
> > 
> > Em Sun, 29 Nov 2015 21:20:04 +0200
> > Sakari Ailus  escreveu:
> > 
> >> From: Sakari Ailus 
> >>
> >> This is useful in e.g. knowing whether certain operations have already
> >> been performed for an entity. The users include the framework itself (for
> >> graph walking) and a number of drivers.
> > 
> > Please use better names on the vars. See below for details.
> > 
> >>
> >> Signed-off-by: Sakari Ailus 
> >> ---
> >>  drivers/media/media-entity.c |  39 +
> >>  include/media/media-device.h |  14 +
> >>  include/media/media-entity.h | 136 
> >> ---
> >>  3 files changed, 181 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> >> index d11f440..fceaf44 100644
> >> --- a/drivers/media/media-entity.c
> >> +++ b/drivers/media/media-entity.c
> >> @@ -213,6 +213,45 @@ void media_gobj_remove(struct media_gobj *gobj)
> >>  }
> >>  
> >>  /**
> >> + * __media_entity_enum_init - Initialise an entity enumeration
> >> + *
> >> + * @e: Entity enumeration to be initialised
> >> + * @idx_max: Maximum number of entities in the enumeration
> >> + *
> >> + * Returns zero on success or a negative error code.
> >> + */
> > 
> > We're using kernel-doc macros only at the header files. Please
> > move those macros to there.
> 
> Oops. Will fix.
> 
> > 
> >> +int __media_entity_enum_init(struct media_entity_enum *e, int idx_max)
> > 
> > using "e" as a var is not nice, specially on a public header.
> > 
> > I would use "ent_enum" instead for media_entity_enum pointers. This
> > applies everywhere on this patch series.
> 
> I'm fine with that suggestion, I'll change to use ent_enum instead
> (where there's no more specific purpose for the variable / field).

Thanks!
> 
> > 
> >> +{
> >> +  if (idx_max > MEDIA_ENTITY_ENUM_MAX_ID) {
> >> +  e->e = kcalloc(DIV_ROUND_UP(idx_max, BITS_PER_LONG),
> >> + sizeof(long), GFP_KERNEL);
> >> +  if (!e->e)
> >> +  return -ENOMEM;
> >> +  } else {
> >> +  e->e = e->__e;
> > 
> > This is crap! I can't tell what the above code is doing,
> > as the var names don't give any clue!
> 
> In retrospect, the names could perhaps have been more descriptive.
> There's no need to be angry at the letter "e" however, it's just doing
> its job...

I'm not angry with the letter "e" (or with you). Just the above code
seems too obscure for a poor reviewer ;)

> 
> > 
> >> +  }
> >> +
> >> +  bitmap_zero(e->e, idx_max);
> > 
> > Ok, here, there's a hint that one of the "e" is a bitmap, while
> > the other is a struct... Weird, and very easy to do wrong things!
> > 
> > Please name the bitmap as "ent_enum_bmap" or something like that.
> 
> Fine with that.
> 
> > 
> >> +  e->idx_max = idx_max;
> >> +
> >> +  return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(__media_entity_enum_init);
> >> +
> >> +/**
> >> + * media_entity_enum_cleanup - Release resources of an entity enumeration
> >> + *
> >> + * @e: Entity enumeration to be released
> >> + */
> >> +void media_entity_enum_cleanup(struct media_entity_enum *e)
> >> +{
> >> +  if (e->e != e->__e)
> >> +  kfree(e->e);
> >> +  e->e = NULL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(media_entity_enum_cleanup);
> >> +
> >> +/**
> >>   * media_entity_init - Initialize a media entity
> >>   *
> >>   * @num_pads: Total number of sink and source pads.
> >> diff --git a/include/media/media-device.h b/include/media/media-device.h
> >> index c0e1764..2d46c66 100644
> >> --- a/include/media/media-device.h
> >> +++ b/include/media/media-device.h
> >> @@ -110,6 +110,20 @@ struct media_device {
> >>  /* media_devnode to media_device */
> >>  #define to_media_device(node) container_of(node, struct media_device, 
> >> devnode)
> >>  
> >> +/**
> >> + * media_entity_enum_init - Initialise an entity enumeration
> >> + *
> >> + * @e: Entity enumeration to be initialised
> >> + * @mdev: The related media device
> >> + *
> >> + * Returns zero on success or a negative error code.
> >> + */
> >> +static inline __must_check int media_entity_enum_init(
> >> +  struct media_entity_enum *e, struct media_device *mdev)
> >> +{
> >> +  return __media_entity_enum_init(e, mdev->entity_internal_idx_max + 1);
> >> +}
> >> +
> >>  void media_device_init(struct media_device *mdev);
> >>  void media_device_cleanup(struct media_device *mdev);
> >>  int __must_check __media_device_register(struct media_device *mdev,
> >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> >> index d3d3a39..5a0339a 100644
> >> --- a/include/media/media-entity.h
> >> +++ b/include/media/media-entity.h
> >> @@ -23,7 +23,7 @@
> >>  #ifndef _MEDIA_ENTITY_H
> >>  #define 

Re: [PATCH 3/3] media: adv7604: update timings on change of input signal

2015-12-13 Thread Laurent Pinchart
Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:53 Ulrich Hecht wrote:
> Without this, g_crop will always return the boot-time state.
> 
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/media/i2c/adv7604.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1bfa9f3..d7d0bb7 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32
> status, bool *handled)
> 
>   v4l2_subdev_notify_event(sd, _ev_fmt);
> 
> + /* update timings */
> + if (adv76xx_query_dv_timings(sd, >timings)
> + == -ENOLINK) {

Nitpicking, I would write this as

ret = adv76xx_query_dv_timings(sd, >timings);
if (ret == -ENOLINK) {

to make it more explicit that the function has side effects. Functions called 
inside an if () statement are often assumed (at least by me) to perform checks 
only and not modify their parameters.

> + /* no signal, fall back to default timings */
> + const struct v4l2_dv_timings cea640x480 =
> + V4L2_DV_BT_CEA_640X480P59_94;
> + state->timings = cea640x480;

You can write this as

state->timings = (struct v4l2_dv_timings)
V4L2_DV_BT_CEA_640X480P59_94;

without using a local variable.

(And now that I mention that I wonder whether the definition of 
V4L2_DV_BT_CEA_640X480P59_94 should be updated to include the (struct 
v4l2_dv_timings))

> + }
> +
>   if (handled)
>   *handled = true;
>   }

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v2 00/32] VSP: Add R-Car Gen3 support

2015-12-13 Thread Laurent Pinchart
Hello Khiem,

On Friday 11 December 2015 15:43:27 Khiem Nguyen wrote:
> On 12/6/2015 5:54 AM, Laurent Pinchart wrote:
> > On Saturday 05 December 2015 11:57:49 Geert Uytterhoeven wrote:
> >> On Sat, Dec 5, 2015 at 3:12 AM, Laurent Pinchart wrote:
> >>> This patch set adds support for the Renesas R-Car Gen3 SoC family to the
> >>> VSP1 driver. The large number of patches is caused by a change in the
> >>> display controller architecture that makes usage of the VSP mandatory as
> >>> the display controller has lost the ability to read data from memory.
> >>> 
> >>> Patch 01/32 to 27/32 prepare for the implementation of an API exported
> >>> to the DRM driver in patch 28/32. Patches 31/32 enables support for the
> >>> R-Car Gen3 family, and patch 32/32 finally enhances perfomances by
> >>> implementing support for display lists.
> >>> 
> >>> The major change compared to v1 is the usage of the IP version register
> >>> instead of DT properties to configure device parameters such as the
> >>> number of BRU inputs or the availability of the BRU.
> >> 
> >> Thanks for your series!
> >> 
> >> As http://git.linuxtv.org/pinchartl/media.git/tag/?id=vsp1-kms-20151112
> >> is getting old, and has lots of conflicts with recent -next, do you plan
> >> to publish this in a branch, and a separate branch for integration, to
> >> ease integration in renesas-drivers?
> >> 
> >> Alternatively, I can just import the series you posted, but having the
> >> broken-out integration part would be nice.
> > 
> > The issue I'm facing is that there's more than just two series. Beside the
> > base VSP patches from this series, I have a series of DRM patches that
> > depend on this one, a series of V4L2 core patches, another series of VSP
> > patches that I still need to finish and a bunch of integration patches.
> > As some of these have dependencies on H3 CCF support that hasn't landed
> > in Simon's tree yet, I have merged your topic/cpg-mssr-v6 and
> > topic/r8a7795-drivers-sh-v1 branches into my tree for development.
> > 
> > I could keep all series in separate branches and merge the two topic
> > branches last, but that's not very handy during development when I have
> > to continuously rebase my patches. Is there a way I could handle this
> > that would make your life easier while not making mine more difficult ?
> > 
> > In the meantime I've pushed vsp1-kms-20151206 to
> > git://linuxtv.org/pinchartl/media.git.
> 
> I failed to confirm DU (VGA port) with above branch.
> 
> To make DU (VGA port) work,
> it seems I need to merge more branches like topic/cpg-mssr-v6
> and topic/r8a7795-drivers-sh-v1 branches from renesas-drivers repo, is
> it correct ?

The

git://linuxtv.org/pinchartl/media.git vsp1-kms-20151206

branch already contains topic/r8a7795-drivers-sh-v1 and topic/cpg-mssr-v6. 
What is the failure you're seeing ?

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH 1/3] media: adv7604: implement g_crop

2015-12-13 Thread Laurent Pinchart
Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:51 Ulrich Hecht wrote:
> The rcar_vin driver relies on this.
> 
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/media/i2c/adv7604.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 129009f..d30e7cc 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1885,6 +1885,17 @@ static int adv76xx_get_format(struct v4l2_subdev *sd,
> return 0;
>  }
> 
> +static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> +{
> + struct adv76xx_state *state = to_state(sd);
> +
> + a->c.height = state->timings.bt.height;
> + a->c.width  = state->timings.bt.width;

Shouldn't you set a->c.top and a->c.left to 0 ? There's no guarantee that the 
caller will always zero the structure before calling you.

> + a->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

The type field is an input parameter, you should just return -EINVAL if the 
value is not V4L2_BUF_TYPE_VIDEO_CAPTURE.

> +
> + return 0;
> +}
> +
>  static int adv76xx_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_pad_config *cfg,
> struct v4l2_subdev_format *format)
> @@ -2407,6 +2418,7 @@ static const struct v4l2_subdev_core_ops
> adv76xx_core_ops = {
> 
>  static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
>   .s_routing = adv76xx_s_routing,
> + .g_crop = adv76xx_g_crop,
>   .g_input_status = adv76xx_g_input_status,
>   .s_dv_timings = adv76xx_s_dv_timings,
>   .g_dv_timings = adv76xx_g_dv_timings,

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH 2/3] media: adv7604: implement cropcap

2015-12-13 Thread Laurent Pinchart
Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:52 Ulrich Hecht wrote:
> Used by the rcar_vin driver.
> 
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/media/i2c/adv7604.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index d30e7cc..1bfa9f3 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1896,6 +1896,22 @@ static int adv76xx_g_crop(struct v4l2_subdev *sd,
> struct v4l2_crop *a) return 0;
>  }
> 
> +static int adv76xx_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
> +{
> + struct adv76xx_state *state = to_state(sd);
> +
> + a->bounds.left   = 0;
> + a->bounds.top= 0;
> + a->bounds.width  = state->timings.bt.width;
> + a->bounds.height = state->timings.bt.height;
> + a->defrect   = a->bounds;
> + a->type  = V4L2_BUF_TYPE_VIDEO_CAPTURE;

As for patch 1/3 the type field is an input parameter.

> + a->pixelaspect.numerator   = 1;
> + a->pixelaspect.denominator = 1;

I'm curious, is this always true with HDMI ?

> +
> + return 0;
> +}
> +
>  static int adv76xx_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_pad_config *cfg,
> struct v4l2_subdev_format *format)
> @@ -2419,6 +2435,7 @@ static const struct v4l2_subdev_core_ops
> adv76xx_core_ops = { static const struct v4l2_subdev_video_ops
> adv76xx_video_ops = {
>   .s_routing = adv76xx_s_routing,
>   .g_crop = adv76xx_g_crop,
> + .cropcap = adv76xx_cropcap,
>   .g_input_status = adv76xx_g_input_status,
>   .s_dv_timings = adv76xx_s_dv_timings,
>   .g_dv_timings = adv76xx_g_dv_timings,

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

2015-12-13 Thread Laurent Pinchart
Hi Hans,

On Friday 11 December 2015 17:25:40 Hans Verkuil wrote:
> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
> > Hi!
> > 
> > The rcar_vin driver relies on these methods.  The third patch makes sure
> > that they return up-to-date data if the input signal has changed since
> > initialization.
> > 
> > CU
> > Uli
> > 
> > Ulrich Hecht (3):
> >   media: adv7604: implement g_crop
> >   media: adv7604: implement cropcap
> 
> I'm not keen on these changes. The reason is that these ops are deprecated
> and soc-camera is - almost - the last user. The g/s_selection ops should be
> used instead.
> 
> Now, I have a patch that changes soc-camera to g/s_selection. The reason it
> was never applied is that I had a hard time finding hardware to test it
> with.
> 
> Since you clearly have that hardware I think I'll rebase my (by now rather
> old) patch and post it again. If you can switch the adv7604 patch to
> g/s_selection and everything works with my patch, then I think I should
> just make a pull request for it.
> 
> I hope to be able to do this on Monday.
> 
> If switching soc-camera over to g/s_selection isn't possible, then at the
> very least your adv7604 changes should provide the g/s_selection
> implementation. I don't want to have to convert this driver later to
> g/s_selection.

I understand your concern and i agree with you. Our plan is to move the rcar-
vin driver away from soc-camera. Unfortunately that will take some time, and 
being able to use the adv7604 driver with rcar-vin would be very handy for 
testing on some of our boards.

Let's see how g/s_selection support in soc-camera works out and then decide on 
what to do.

> >   media: adv7604: update timings on change of input signal
> >  
> >  drivers/media/i2c/adv7604.c | 38 ++
> >  1 file changed, 38 insertions(+)

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH] v4l: Fix dma buf single plane compat handling

2015-12-13 Thread Laurent Pinchart
Hi Sakari,

On Wednesday 09 December 2015 13:07:40 Sakari Ailus wrote:
> On Wed, Dec 09, 2015 at 01:11:12AM +0200, Laurent Pinchart wrote:
> > On Tuesday 08 December 2015 17:29:16 Sakari Ailus wrote:
> > > On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
> > > > From: Gjorgji Rosikopulos 
> > > > 
> > > > Buffer length is needed for single plane as well, otherwise
> > > > is uninitialized and behaviour is undetermined.
> > > 
> > > How about:
> > > 
> > > The v4l2_buffer length field must be passed as well from user to kernel
> > > and back, otherwise uninitialised values will be used.
> > > 
> > > > Signed-off-by: Gjorgji Rosikopulos 
> > > > Signed-off-by: Laurent Pinchart 
> > > 
> > > Acked-by: Sakari Ailus 
> > > 
> > > Shouldn't this be submitted to stable as well?
> > 
> > I'll CC stable.
> > 
> > > > ---
> > > > 
> > > >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index
> > > > 8fd84a67478a..b0faa1f7e3a9 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > > @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer
> > > > *kp, struct v4l2_buffer32 __user
> > > > return -EFAULT;
> > > > break;
> > > > 
> > > > case V4L2_MEMORY_DMABUF:
> > > > -   if (get_user(kp->m.fd, >m.fd))
> > > > +   if (get_user(kp->m.fd, >m.fd) ||
> > > > +   get_user(kp->length, >length))
> > > > return -EFAULT;
> > > > +
> 
> Without the extra newline, please?

Sure, I'll fix that in the pull request.

-- 
Regards,

Laurent Pinchart

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


Re: [v4l-utils PATCH 1/1] v4l: libv4l2subdev: Precisely convert media bus string to code

2015-12-13 Thread Laurent Pinchart
Hi Sakari,

Thank you for the patch.

On Wednesday 09 December 2015 17:14:47 Sakari Ailus wrote:
> The length of the string was ignored, making it possible for the
> conversion to fail due to extra characters in the string.

I'm not sure to follow you there. Is the issue that passing a string such as 
"SBGGR10" would match "SBGGR10_DPCM8" if it was listed before "SBGGR10" ? If 
that's the case I'd write the commit message as

Any character beyond the fist `length' characters in the mbus_formats strings
are ignored, causing incorrect matches if the format entry starts with but 
isn't equal to the passed format.

> Signed-off-by: Sakari Ailus 
> ---
> This patch should be applied before the set "[v4l-utils PATCH v2 0/3] List
> supported formats in libv4l2subdev":
> 
> 
> 
>  utils/media-ctl/libv4l2subdev.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/utils/media-ctl/libv4l2subdev.c
> b/utils/media-ctl/libv4l2subdev.c index 33c1ee6..cce527d 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -769,14 +769,12 @@ enum v4l2_mbus_pixelcode
> v4l2_subdev_string_to_pixelcode(const char *string, unsigned int i;
> 
>   for (i = 0; i < ARRAY_SIZE(mbus_formats); ++i) {
> - if (strncmp(mbus_formats[i].name, string, length) == 0)
> - break;
> + if (strncmp(mbus_formats[i].name, string, length) == 0
> + && strlen(mbus_formats[i].name) == length)

How about mbus_formats[i].name[length] == '\0' instead ? That should be more 
efficient.

I also wonder whether we shouldn't just get rid of the length argument and 
force the passed format string to be zero-terminated.

> + return mbus_formats[i].code;
>   }
> 
> - if (i == ARRAY_SIZE(mbus_formats))
> - return (enum v4l2_mbus_pixelcode)-1;
> -
> - return mbus_formats[i].code;
> + return (enum v4l2_mbus_pixelcode)-1;
>  }
> 
>  static struct {

-- 
Regards,

Laurent Pinchart

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


Re: [v4l-utils PATCH v2 1/3] libv4l2subdev: Use generated format definitions in libv4l2subdev

2015-12-13 Thread Laurent Pinchart
Hi Sakari,

On Tuesday 08 December 2015 17:15:14 Sakari Ailus wrote:
> Instead of manually adding each and every new media bus pixel code to
> libv4l2subdev, generate the list automatically. The pre-existing formats
> that do not match the list are not modified so that existing users are
> unaffected by this change, with the exception of converting codes to
> strings, which will use the new definitions.
> 
> Signed-off-by: Sakari Ailus 

This will result in command line strings being a bit longer than I'd like, but 
I agree that adding them manually doesn't scale.

Acked-by: Laurent Pinchart 

> ---
>  utils/media-ctl/.gitignore  | 1 +
>  utils/media-ctl/Makefile.am | 8 
>  utils/media-ctl/libv4l2subdev.c | 1 +
>  3 files changed, 10 insertions(+)
> 
> diff --git a/utils/media-ctl/.gitignore b/utils/media-ctl/.gitignore
> index 95b6a57..8c7d576 100644
> --- a/utils/media-ctl/.gitignore
> +++ b/utils/media-ctl/.gitignore
> @@ -1 +1,2 @@
>  media-ctl
> +media-bus-formats.h
> diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
> index a3931fb..a1a9225 100644
> --- a/utils/media-ctl/Makefile.am
> +++ b/utils/media-ctl/Makefile.am
> @@ -4,6 +4,14 @@ libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h
>  libmediactl_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
>  libmediactl_la_LDFLAGS = -static $(LIBUDEV_LIBS)
> 
> +media-bus-formats.h: ../../include/linux/media-bus-format.h
> + sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*FMT_//; /FIXED/ d; s/\t.*//;
> s/.*/{ \"&\", MEDIA_BUS_FMT_& },/;' \ +   < $< > $@
> +
> +BUILT_SOURCES = media-bus-formats.h
> +CLEANFILES = media-bus-formats.h
> +
> +nodist_libv4l2subdev_la_SOURCES = media-bus-formats.h
>  libv4l2subdev_la_SOURCES = libv4l2subdev.c
>  libv4l2subdev_la_LIBADD = libmediactl.la
>  libv4l2subdev_la_CFLAGS = -static
> diff --git a/utils/media-ctl/libv4l2subdev.c
> b/utils/media-ctl/libv4l2subdev.c index 33c1ee6..5bcfe34 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -719,6 +719,7 @@ static struct {
>   const char *name;
>   enum v4l2_mbus_pixelcode code;
>  } mbus_formats[] = {
> +#include "media-bus-formats.h"
>   { "Y8", MEDIA_BUS_FMT_Y8_1X8},
>   { "Y10", MEDIA_BUS_FMT_Y10_1X10 },
>   { "Y12", MEDIA_BUS_FMT_Y12_1X12 },

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v2 4/7] media: vb2-dma-contig: add helper for setting dma max seg size

2015-12-13 Thread Laurent Pinchart
Hi Marek,

Thank you for the patch.

On Wednesday 09 December 2015 14:58:19 Marek Szyprowski wrote:
> Add a helper function for device drivers to set DMA's max_seg_size.
> Setting it to largest possible value lets DMA-mapping API always create
> contiguous mappings in DMA address space. This is essential for all
> devices, which use dma-contig videobuf2 memory allocator and shared
> buffers.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
>  include/media/videobuf2-dma-contig.h   |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index c331272..628518d
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -742,6 +742,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
>  }
>  EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
> 
> +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
> +{
> + if (!dev->dma_parms) {

When can this function be called with dev->dma_parms not NULL ?

> + dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
> +   GFP_KERNEL);
> + if (!dev->dma_parms)
> + return -ENOMEM;
> + }
> + if (dma_get_max_seg_size(dev) < size)
> + return dma_set_max_seg_size(dev, size);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
> +
>  MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
>  MODULE_AUTHOR("Pawel Osciak ");
>  MODULE_LICENSE("GPL");
> diff --git a/include/media/videobuf2-dma-contig.h
> b/include/media/videobuf2-dma-contig.h index c33dfa6..0e6ba64 100644
> --- a/include/media/videobuf2-dma-contig.h
> +++ b/include/media/videobuf2-dma-contig.h
> @@ -26,6 +26,7 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb,
> unsigned int plane_no)
> 
>  void *vb2_dma_contig_init_ctx(struct device *dev);
>  void vb2_dma_contig_cleanup_ctx(void *alloc_ctx);
> +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size);
> 
>  extern const struct vb2_mem_ops vb2_dma_contig_memops;

-- 
Regards,

Laurent Pinchart

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


Re: [v4l-utils PATCH v2 2/3] libv4l2subdev: Add a function to list library supported pixel codes

2015-12-13 Thread Laurent Pinchart
Hi Sakari,

Thank you for the patch.

On Tuesday 08 December 2015 17:15:15 Sakari Ailus wrote:
> Also mark which format definitions are compat definitions for the
> pre-existing codes. This way we don't end up listing the same formats
> twice.

Wouldn't it be easier to add a function to return the whole array (and 
terminate it with an empty entry to avoid having to return both the array and 
the length=) ?

> Signed-off-by: Sakari Ailus 
> ---
>  utils/media-ctl/libv4l2subdev.c | 69 +
>  utils/media-ctl/v4l2subdev.h| 11 +++
>  2 files changed, 50 insertions(+), 30 deletions(-)
> 
> diff --git a/utils/media-ctl/libv4l2subdev.c
> b/utils/media-ctl/libv4l2subdev.c index 5bcfe34..2cd8fd4 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -718,38 +718,39 @@ int v4l2_subdev_parse_setup_formats(struct
> media_device *media, const char *p) static struct {
>   const char *name;
>   enum v4l2_mbus_pixelcode code;
> + bool compat;
>  } mbus_formats[] = {
>  #include "media-bus-formats.h"
> - { "Y8", MEDIA_BUS_FMT_Y8_1X8},
> - { "Y10", MEDIA_BUS_FMT_Y10_1X10 },
> - { "Y12", MEDIA_BUS_FMT_Y12_1X12 },
> - { "YUYV", MEDIA_BUS_FMT_YUYV8_1X16 },
> - { "YUYV1_5X8", MEDIA_BUS_FMT_YUYV8_1_5X8 },
> - { "YUYV2X8", MEDIA_BUS_FMT_YUYV8_2X8 },
> - { "UYVY", MEDIA_BUS_FMT_UYVY8_1X16 },
> - { "UYVY1_5X8", MEDIA_BUS_FMT_UYVY8_1_5X8 },
> - { "UYVY2X8", MEDIA_BUS_FMT_UYVY8_2X8 },
> - { "VUY24", MEDIA_BUS_FMT_VUY8_1X24 },
> - { "SBGGR8", MEDIA_BUS_FMT_SBGGR8_1X8 },
> - { "SGBRG8", MEDIA_BUS_FMT_SGBRG8_1X8 },
> - { "SGRBG8", MEDIA_BUS_FMT_SGRBG8_1X8 },
> - { "SRGGB8", MEDIA_BUS_FMT_SRGGB8_1X8 },
> - { "SBGGR10", MEDIA_BUS_FMT_SBGGR10_1X10 },
> - { "SGBRG10", MEDIA_BUS_FMT_SGBRG10_1X10 },
> - { "SGRBG10", MEDIA_BUS_FMT_SGRBG10_1X10 },
> - { "SRGGB10", MEDIA_BUS_FMT_SRGGB10_1X10 },
> - { "SBGGR10_DPCM8", MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8 },
> - { "SGBRG10_DPCM8", MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8 },
> - { "SGRBG10_DPCM8", MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8 },
> - { "SRGGB10_DPCM8", MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8 },
> - { "SBGGR12", MEDIA_BUS_FMT_SBGGR12_1X12 },
> - { "SGBRG12", MEDIA_BUS_FMT_SGBRG12_1X12 },
> - { "SGRBG12", MEDIA_BUS_FMT_SGRBG12_1X12 },
> - { "SRGGB12", MEDIA_BUS_FMT_SRGGB12_1X12 },
> - { "AYUV32", MEDIA_BUS_FMT_AYUV8_1X32 },
> - { "RBG24", MEDIA_BUS_FMT_RBG888_1X24 },
> - { "RGB32", MEDIA_BUS_FMT_RGB888_1X32_PADHI },
> - { "ARGB32", MEDIA_BUS_FMT_ARGB_1X32 },
> + { "Y8", MEDIA_BUS_FMT_Y8_1X8, true },
> + { "Y10", MEDIA_BUS_FMT_Y10_1X10, true },
> + { "Y12", MEDIA_BUS_FMT_Y12_1X12, true },
> + { "YUYV", MEDIA_BUS_FMT_YUYV8_1X16, true },
> + { "YUYV1_5X8", MEDIA_BUS_FMT_YUYV8_1_5X8, true },
> + { "YUYV2X8", MEDIA_BUS_FMT_YUYV8_2X8, true },
> + { "UYVY", MEDIA_BUS_FMT_UYVY8_1X16, true },
> + { "UYVY1_5X8", MEDIA_BUS_FMT_UYVY8_1_5X8, true },
> + { "UYVY2X8", MEDIA_BUS_FMT_UYVY8_2X8, true },
> + { "VUY24", MEDIA_BUS_FMT_VUY8_1X24, true },
> + { "SBGGR8", MEDIA_BUS_FMT_SBGGR8_1X8, true },
> + { "SGBRG8", MEDIA_BUS_FMT_SGBRG8_1X8, true },
> + { "SGRBG8", MEDIA_BUS_FMT_SGRBG8_1X8, true },
> + { "SRGGB8", MEDIA_BUS_FMT_SRGGB8_1X8, true },
> + { "SBGGR10", MEDIA_BUS_FMT_SBGGR10_1X10, true },
> + { "SGBRG10", MEDIA_BUS_FMT_SGBRG10_1X10, true },
> + { "SGRBG10", MEDIA_BUS_FMT_SGRBG10_1X10, true },
> + { "SRGGB10", MEDIA_BUS_FMT_SRGGB10_1X10, true },
> + { "SBGGR10_DPCM8", MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, true },
> + { "SGBRG10_DPCM8", MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, true },
> + { "SGRBG10_DPCM8", MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, true },
> + { "SRGGB10_DPCM8", MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, true },
> + { "SBGGR12", MEDIA_BUS_FMT_SBGGR12_1X12, true },
> + { "SGBRG12", MEDIA_BUS_FMT_SGBRG12_1X12, true },
> + { "SGRBG12", MEDIA_BUS_FMT_SGRBG12_1X12, true },
> + { "SRGGB12", MEDIA_BUS_FMT_SRGGB12_1X12, true },
> + { "AYUV32", MEDIA_BUS_FMT_AYUV8_1X32, true },
> + { "RBG24", MEDIA_BUS_FMT_RBG888_1X24, true },
> + { "RGB32", MEDIA_BUS_FMT_RGB888_1X32_PADHI, true },
> + { "ARGB32", MEDIA_BUS_FMT_ARGB_1X32, true },
>  };
> 
>  const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code)
> @@ -823,3 +824,11 @@ enum v4l2_field v4l2_subdev_string_to_field(const char
> *string,
> 
>   return fields[i].field;
>  }
> +
> +enum v4l2_mbus_pixelcode v4l2_subdev_pixelcode_list(unsigned int i)
> +{
> + if (i >= ARRAY_SIZE(mbus_formats) || mbus_formats[i].compat)
> + return (enum v4l2_mbus_pixelcode)-1;
> +
> + return mbus_formats[i].code;
> +}
> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
> index 104e420..ef8ef95 100644
> --- a/utils/media-ctl/v4l2subdev.h
> +++ b/utils/media-ctl/v4l2subdev.h
> @@ -279,4 +279,15 @@ const 

cron job: media_tree daily build: WARNINGS

2015-12-13 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:   Mon Dec 14 04:00:22 CET 2015
git branch: test
git hash:   52d60eb7e6d6429a766ea1b8f67e01c3b2dcd3c5
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0-51-ga53cea2
smatch version: v0.5.0-3202-g618e15b
host hardware:  x86_64
host os:4.2.0-164

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.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-i686: OK
linux-4.3-i686: OK
linux-4.4-rc1-i686: 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-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.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


[yavta PATCH v2] Return proper error code if STREAMON fails

2015-12-13 Thread Tuukka Toivonen
Return the error code if video_enable() and VIDIOC_STREAMON
fails.

Signed-off-by: Tuukka Toivonen 
---
 yavta.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/yavta.c b/yavta.c
index b627725..3d80d3c 100644
--- a/yavta.c
+++ b/yavta.c
@@ -1708,7 +1708,9 @@ static int video_do_capture(struct device *dev, unsigned 
int nframes,
}
 
/* Stop streaming. */
-   video_enable(dev, 0);
+   ret = video_enable(dev, 0);
+   if (ret < 0)
+   return ret;
 
if (nframes == 0) {
printf("No frames captured.\n");
-- 
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: [yavta PATCH] Return proper error code if STREAMON fails

2015-12-13 Thread Tuukka Toivonen
Hi,

Thanks for your feedback.

On Saturday, December 12, 2015 17:40:07 Laurent Pinchart wrote:
> I wonder if there's really a point calling video_free_buffers() in the
> error case. The function will return an error causing the caller to
> close the device, which will free the buffers. There are other
> locations in yavta after buffers are allocated where the buffers are not
> freed in the error path. What would you think of just replacing the
> goto done statements by a return ret ?

I'm fine with that change. It will also simplify the patch.
I'll submit a new version of the patch.

- Tuukka

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