Re: media_build: fails to install

2017-06-02 Thread Vincent McIntyre
On Wed, May 31, 2017 at 03:57:04AM +0300, Olli Salonen wrote:
> It seems that I'm able to build the media_build correctly on Ubuntu
> 16.04.2 with kernel 4.8, but make install fails:
> 
> ~/src/media_build$ sudo make install
> make -C /home/olli/src/media_build/v4l install
> make[1]: Entering directory '/home/olli/src/media_build/v4l'
> make[1]: *** No rule to make target 'media-install', needed by 'install'.  
> Stop.
> make[1]: Leaving directory '/home/olli/src/media_build/v4l'
> Makefile:15: recipe for target 'install' failed
> make: *** [install] Error 2
> 

I can confirm this issue.

The reason is that scripts/make_makefile.pl aborts

make[1]: Entering directory '/home/me/git/clones/media_build/v4l'^M
scripts/make_makefile.pl^M  
Can't handle includes! In 
../linux/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile at 
scripts/  make_makefile.pl line 109,  line 4.^M

because that css2400/Makefile includes another:

$ cat ../linux/drivers/staging/media/atomisp/pci/atomisp2/css2400/Makefile

ccflags-y += -DISP2400B0
ISP2400B0 := y

include $(srctree)/$(src)/../Makefile.common

The abort of scripts/make_makefile.pl means that the v4l/Makefile
does not get completely written out, in particular the rules for
making the 'media-install' target.

I am not sure how to fix this. The make_makefile.pl deliberately
falls over when given an include to deal with, so there must be
some other mechanism in the media_build framework that handles
this kind of thing. But I am not aware of it. Hans, help pretty please?

Vince



cron job: media_tree daily build: WARNINGS

2017-06-02 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:   Sat Jun  3 05:00:21 CEST 2017
media-tree git hash:36bcba973ad478042d1ffc6e89afd92e8bd17030
media_build git hash:   0d8b3274e29b597780719e7ce1b3b460241a5395
v4l-utils git hash: ef074cf5500b7dd62e6eb3527ec47a914c7189ca
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: WARNINGS
linux-git-arm-davinci: WARNINGS
linux-git-arm-multi: WARNINGS
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: WARNINGS
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
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: WARNINGS
linux-3.11.1-i686: OK
linux-3.12.67-i686: OK
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12-rc1-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12-rc1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [media] vimc: API proposal, configuring the topology from user space

2017-06-02 Thread Helen Koike

ping

On 2017-04-10 07:53 PM, Helen Koike wrote:


Hi,

Continuing the discussion about the API of the vimc driver, I made some
changes
based on the previous comments, please see below and let me know your
opinion about it.

Helen

/***
Configfs considerations:
/
Informal definitions:
subsystem: the root driver folder in user space (/configfs/vimc)
item: aka a folder in user space
attributes: aka files in the folder
group: aka a folder that can contain subfolders (parent and child
relation)
default group: aka a subfolder that is created automatically when
the "parent" folder is created
it is not considered a child in terms of rmdir

* Performing rmdir in a group will fail if it contain children that are
not default groups, i.e, if the
folder contain subfolders that are default group, then it can be removed
with rmdir, if the
subfolders were created with mkdir, then rmdir in the parent will fail.

* Configfs has the notion of committable item but it is not implemented
yet. A committable item is an item
that can be in one of two parent folders called: live and pending. The
idea is to create and modify the item
in the pending directory and then to move the item through a rename to
the live directory where
it can't be modified. This seems to be a nice feature for vimc, but as
it is not available yet the
proposal below won't be based on this.

* Groups can be dynamically created/destroyed by the driver whenever it
wants. Afaik attributes can only
be created when the group or item is created and symlinks can only be
create from user space, i.e, the
driver don't know how to create/destroy attributes or symlinks in anytime.

/***
The API:
/

In short, a topology like this one: http://goo.gl/Y7eUfu
Would look like this filesystem tree: https://goo.gl/mEOmOf

v3 core changes:
- I removed the use of symlinks as I wans't able to see how to do it
nicely.
- I use the names of the folders created by user space to retrieve
information at mkdir time
- hotplug file in each entity
- hotplug file in each device
- reset file in each device

* The /configfs/vimc subsystem
empty when the driver is loaded

* Create a device
Userspace can create a new vimc device with:

$ mkdir /configfs/vimc/any_name
Example:
$ mkdir /configfs/vimc/vimc0
$ ls -l /configfs/vimc/vimc0
hotplug
reset
entities/
links/

entities/ and links/ folder are default groups, thus they don't prevent
rmdir vimc0/, but
rmdir will fail if it has any child inside entities/ or links/.
hotplug is used to plug and unplug the device, it can read "plugged" or
"unplugged" and user can
write "plug" or "unplug" to change its state.
Changing hotplug state will never fail as the configfs tree will always
be in a valid state.
reset is used to easily destroy all the topology without the need to
walk through all the children
to perform rmdir, writing 1 to reset file will set hotplug to
"unplugged" and erase all folders
under entities/ and links/.

* Create an entity
Userspace can create a new entity with:

$ mkdir /configfs/vimc/vimc0/entities/:
Example:
$ mkdir /configfs/vimc/vimc0/entities/sensor:SensorA
$ ls -l /configfs/vimc/vimc0/entities/sensor:SensorA
hotplug
pad:source:0/

The name of the folder needs to be in the format : or it
will be rejected, this allows the
creation of the right pads according to its role at mkdir time,
eliminating the previously proposed role
and name files.
hotplug is used to plug and unplug the hw block, it can read "plugged"
or "unplugged" and user can
write "plug" or "unplug" to change its state. As we don't support this
yet in the media core, changing it
will only be allowed if /configfs/vimc/vimc0/hotplug is "unplugged".
hotplug file is "unplugged" by default.
Pads will be created as default groups with the name in the format
pad:: and it
will be an empty folder.
If the hw block supports different number of pads, we could expose two
files:
sinks
sources
where the user space can write the desired number of sink and source
pads and the driver will dynamically
create the folders pad::

* Create a link
User space can create a link between two entities with:

$ mkdir
/configfs/vimc/vimc0/links/:->:

Example:
$ mkdir /configfs/vimc/vimc0/links/DebayerA:1->Scaler:0
$ ls -l /configfs/vimc/vimc0/links/DebayerA:1->Scaler:0
flags

mkdir will be rejected if folder is not on the format
:->:.
mkdir will be rejected if either  or 
are not found in /configfs/vimc/vimc0/entities/
The link will only be created if both entities are in "plugged" state.
When an entity is removed from /configfs/vimc/vimc0/entities/ with
rmdir, its corresponding link folders at
/configfs/vimc/vimc0/links will be automatically removed.
If one of the entities changes from "plugged" to "unplugged", the link
is only removed from the media
representation, the link folder won't be removed.
flags 

[RFC PATCH v3 04/11] [media] vimc: common: Add vimc_pipeline_s_stream helper

2017-06-02 Thread Helen Koike
Move the vimc_cap_pipeline_s_stream from the vimc-cap.c to vimc-common.c
as this core will be reused by other subdevices to activate the stream
in their directly connected nodes

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: Add vimc_pipeline_s_stream in the core
- add it in vimc-common instead of vimc-core
- rename commit with "common" tag

Changes in v2:
[media] vimc: Add vimc_pipeline_s_stream in the core
- Use is_media_entity_v4l2_subdev instead of comparing with the old
entity->type
- Fix comments style
- add kernel-docs
- call s_stream across all sink pads


---
 drivers/media/platform/vimc/vimc-capture.c | 29 ++-
 drivers/media/platform/vimc/vimc-common.c  | 32 ++
 drivers/media/platform/vimc/vimc-common.h  | 11 ++
 3 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index 9adb06d..93f6a09 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -132,31 +132,6 @@ static void vimc_cap_return_all_buffers(struct 
vimc_cap_device *vcap,
spin_unlock(>qlock);
 }
 
-static int vimc_cap_pipeline_s_stream(struct vimc_cap_device *vcap, int enable)
-{
-   struct v4l2_subdev *sd;
-   struct media_pad *pad;
-   int ret;
-
-   /* Start the stream in the subdevice direct connected */
-   pad = media_entity_remote_pad(>vdev.entity.pads[0]);
-
-   /*
-* if it is a raw node from vimc-core, there is nothing to activate
-* TODO: remove this when there are no more raw nodes in the
-* core and return error instead
-*/
-   if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_BASE)
-   return 0;
-
-   sd = media_entity_to_v4l2_subdev(pad->entity);
-   ret = v4l2_subdev_call(sd, video, s_stream, enable);
-   if (ret && ret != -ENOIOCTLCMD)
-   return ret;
-
-   return 0;
-}
-
 static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
@@ -173,7 +148,7 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
 
/* Enable streaming from the pipe */
-   ret = vimc_cap_pipeline_s_stream(vcap, 1);
+   ret = vimc_pipeline_s_stream(>vdev.entity, 1);
if (ret) {
media_pipeline_stop(entity);
vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
@@ -192,7 +167,7 @@ static void vimc_cap_stop_streaming(struct vb2_queue *vq)
struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
 
/* Disable streaming from the pipe */
-   vimc_cap_pipeline_s_stream(vcap, 0);
+   vimc_pipeline_s_stream(>vdev.entity, 0);
 
/* Stop the media pipeline */
media_pipeline_stop(>vdev.entity);
diff --git a/drivers/media/platform/vimc/vimc-common.c 
b/drivers/media/platform/vimc/vimc-common.c
index 3afbabd..f809a9d 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -220,6 +220,38 @@ struct media_pad *vimc_pads_init(u16 num_pads, const 
unsigned long *pads_flag)
return pads;
 }
 
+int vimc_pipeline_s_stream(struct media_entity *ent, int enable)
+{
+   struct v4l2_subdev *sd;
+   struct media_pad *pad;
+   unsigned int i;
+   int ret;
+
+   for (i = 0; i < ent->num_pads; i++) {
+   if (ent->pads[i].flags & MEDIA_PAD_FL_SOURCE)
+   continue;
+
+   /* Start the stream in the subdevice direct connected */
+   pad = media_entity_remote_pad(>pads[i]);
+
+   /*
+* if this is a raw node from vimc-core, then there is
+* nothing to activate
+* TODO: remove this when there are no more raw nodes in the
+* core and return error instead
+*/
+   if (pad->entity->obj_type == MEDIA_ENTITY_TYPE_BASE)
+   continue;
+
+   sd = media_entity_to_v4l2_subdev(pad->entity);
+   ret = v4l2_subdev_call(sd, video, s_stream, enable);
+   if (ret && ret != -ENOIOCTLCMD)
+   return ret;
+   }
+
+   return 0;
+}
+
 static const struct media_entity_operations vimc_ent_sd_mops = {
.link_validate = v4l2_subdev_link_validate,
 };
diff --git a/drivers/media/platform/vimc/vimc-common.h 
b/drivers/media/platform/vimc/vimc-common.h
index 9ec361c..73e7e94 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -97,6 +97,17 @@ static inline void vimc_pads_cleanup(struct media_pad *pads)
 }
 
 /**
+ * vimc_pipeline_s_stream - start stream through the pipeline
+ *
+ * @ent:   the pointer to struct 

[RFC PATCH v3 03/11] [media] vimc: common: Add vimc_ent_sd_* helper

2017-06-02 Thread Helen Koike
As all the subdevices in the topology will be initialized in the same
way, to avoid code repetition the vimc_ent_sd_{register, unregister}
helper functions were created

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: Add vimc_ent_sd_* helper functions
- add it in vimc-common.c instead in vimc-core.c
- fix vimc_ent_sd_register, use function parameter to assign
sd->entity.function instead of using a fixed value
- rename commit to add the "common" tag

Changes in v2:
[media] vimc: Add vimc_ent_sd_* helper functions
- Comments in vimc_ent_sd_init
- Update vimc_ent_sd_init with upstream code as media_entity_pads_init
(instead of media_entity_init), entity->function intead of entity->type
- Add missing vimc_pads_cleanup in vimc_ent_sd_cleanup
- remove subdevice v4l2_dev and dev fields
- change unregister order in vimc_ent_sd_cleanup
- rename vimc_ent_sd_{init,cleanup} to vimc_ent_sd_{register,unregister}
- remove struct vimc_ent_subdevice, use ved and sd directly
- don't impose struct vimc_sen_device to declare ved and sd struct first
- add kernel docs


---
 drivers/media/platform/vimc/vimc-common.c | 66 +++
 drivers/media/platform/vimc/vimc-common.h | 39 ++
 drivers/media/platform/vimc/vimc-sensor.c | 58 +--
 3 files changed, 114 insertions(+), 49 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-common.c 
b/drivers/media/platform/vimc/vimc-common.c
index 42f779a..3afbabd 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -219,3 +219,69 @@ struct media_pad *vimc_pads_init(u16 num_pads, const 
unsigned long *pads_flag)
 
return pads;
 }
+
+static const struct media_entity_operations vimc_ent_sd_mops = {
+   .link_validate = v4l2_subdev_link_validate,
+};
+
+int vimc_ent_sd_register(struct vimc_ent_device *ved,
+struct v4l2_subdev *sd,
+struct v4l2_device *v4l2_dev,
+const char *const name,
+u32 function,
+u16 num_pads,
+const unsigned long *pads_flag,
+const struct v4l2_subdev_ops *sd_ops,
+void (*sd_destroy)(struct vimc_ent_device *))
+{
+   int ret;
+
+   /* Allocate the pads */
+   ved->pads = vimc_pads_init(num_pads, pads_flag);
+   if (IS_ERR(ved->pads))
+   return PTR_ERR(ved->pads);
+
+   /* Fill the vimc_ent_device struct */
+   ved->destroy = sd_destroy;
+   ved->ent = >entity;
+
+   /* Initialize the subdev */
+   v4l2_subdev_init(sd, sd_ops);
+   sd->entity.function = function;
+   sd->entity.ops = _ent_sd_mops;
+   sd->owner = THIS_MODULE;
+   strlcpy(sd->name, name, sizeof(sd->name));
+   v4l2_set_subdevdata(sd, ved);
+
+   /* Expose this subdev to user space */
+   sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+   /* Initialize the media entity */
+   ret = media_entity_pads_init(>entity, num_pads, ved->pads);
+   if (ret)
+   goto err_clean_pads;
+
+   /* Register the subdev with the v4l2 and the media framework */
+   ret = v4l2_device_register_subdev(v4l2_dev, sd);
+   if (ret) {
+   dev_err(v4l2_dev->dev,
+   "%s: subdev register failed (err=%d)\n",
+   name, ret);
+   goto err_clean_m_ent;
+   }
+
+   return 0;
+
+err_clean_m_ent:
+   media_entity_cleanup(>entity);
+err_clean_pads:
+   vimc_pads_cleanup(ved->pads);
+   return ret;
+}
+
+void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev 
*sd)
+{
+   v4l2_device_unregister_subdev(sd);
+   media_entity_cleanup(ved->ent);
+   vimc_pads_cleanup(ved->pads);
+}
diff --git a/drivers/media/platform/vimc/vimc-common.h 
b/drivers/media/platform/vimc/vimc-common.h
index 00d3da4..9ec361c 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -110,4 +110,43 @@ const struct vimc_pix_map *vimc_pix_map_by_code(u32 code);
  */
 const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
 
+/**
+ * vimc_ent_sd_register - initialize and register a subdev node
+ *
+ * @ved:   the vimc_ent_device struct to be initialize
+ * @sd:the v4l2_subdev struct to be initialize and registered
+ * @v4l2_dev:  the v4l2 device to register the v4l2_subdev
+ * @name:  name of the sub-device. Please notice that the name must be
+ * unique.
+ * @function:  media entity function defined by MEDIA_ENT_F_* macros
+ * @num_pads:  number of pads to initialize
+ * @pads_flag: flags to use in each pad
+ * @sd_ops:pointer to  v4l2_subdev_ops.
+ * @sd_destroy:callback to 

[RFC PATCH v3 01/11] [media] vimc: sen: Integrate the tpg on the sensor

2017-06-02 Thread Helen Koike
Initialize the test pattern generator on the sensor
Generate a colored bar image instead of a grey one

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: sen: Integrate the tpg on the sensor
- Declare frame_size as a local variable
- Set tpg frame format before starting kthread
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: propagate error from kthread_stop
- coding style when calling tpg_s_bytesperline
- s/vimc_thread_sen/vimc_sen_tpg_thread
- fix multiline comment

Changes in v2:
[media] vimc: sen: Integrate the tpg on the sensor
- Fix include location
- Select V4L2_TPG in Kconfig
- configure tpg on streamon only
- rm BUG_ON
- coding style
- remove V4L2_FIELD_ALTERNATE from tpg_s_field
- remove V4L2_STD_PAL from tpg_fill_plane_buffer


---
 drivers/media/platform/vimc/Kconfig   |  1 +
 drivers/media/platform/vimc/vimc-sensor.c | 64 ---
 2 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/vimc/Kconfig 
b/drivers/media/platform/vimc/Kconfig
index a18f635..71c9fe7 100644
--- a/drivers/media/platform/vimc/Kconfig
+++ b/drivers/media/platform/vimc/Kconfig
@@ -2,6 +2,7 @@ config VIDEO_VIMC
tristate "Virtual Media Controller Driver (VIMC)"
depends on VIDEO_DEV && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
select VIDEOBUF2_VMALLOC
+   select VIDEO_V4L2_TPG
default n
---help---
  Skeleton driver for Virtual Media Controller
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
index 591f6a4..2e83487 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -20,17 +20,20 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vimc-sensor.h"
 
+#define VIMC_SEN_FRAME_MAX_WIDTH 4096
+
 struct vimc_sen_device {
struct vimc_ent_device ved;
struct v4l2_subdev sd;
+   struct tpg_data tpg;
struct task_struct *kthread_sen;
u8 *frame;
/* The active format */
struct v4l2_mbus_framefmt mbus_format;
-   int frame_size;
 };
 
 static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
@@ -84,6 +87,24 @@ static int vimc_sen_get_fmt(struct v4l2_subdev *sd,
return 0;
 }
 
+static void vimc_sen_tpg_s_format(struct vimc_sen_device *vsen)
+{
+   const struct vimc_pix_map *vpix =
+   vimc_pix_map_by_code(vsen->mbus_format.code);
+
+   tpg_reset_source(>tpg, vsen->mbus_format.width,
+vsen->mbus_format.height, vsen->mbus_format.field);
+   tpg_s_bytesperline(>tpg, 0, vsen->mbus_format.width * vpix->bpp);
+   tpg_s_buf_height(>tpg, vsen->mbus_format.height);
+   tpg_s_fourcc(>tpg, vpix->pixelformat);
+   /* TODO: add support for V4L2_FIELD_ALTERNATE */
+   tpg_s_field(>tpg, vsen->mbus_format.field, false);
+   tpg_s_colorspace(>tpg, vsen->mbus_format.colorspace);
+   tpg_s_ycbcr_enc(>tpg, vsen->mbus_format.ycbcr_enc);
+   tpg_s_quantization(>tpg, vsen->mbus_format.quantization);
+   tpg_s_xfer_func(>tpg, vsen->mbus_format.xfer_func);
+}
+
 static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
.enum_mbus_code = vimc_sen_enum_mbus_code,
.enum_frame_size= vimc_sen_enum_frame_size,
@@ -97,7 +118,7 @@ static const struct media_entity_operations vimc_sen_mops = {
.link_validate = v4l2_subdev_link_validate,
 };
 
-static int vimc_thread_sen(void *data)
+static int vimc_sen_tpg_thread(void *data)
 {
struct vimc_sen_device *vsen = data;
unsigned int i;
@@ -110,7 +131,7 @@ static int vimc_thread_sen(void *data)
if (kthread_should_stop())
break;
 
-   memset(vsen->frame, 100, vsen->frame_size);
+   tpg_fill_plane_buffer(>tpg, 0, 0, vsen->frame);
 
/* Send the frame to all source pads */
for (i = 0; i < vsen->sd.entity.num_pads; i++)
@@ -132,26 +153,31 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int 
enable)
 
if (enable) {
const struct vimc_pix_map *vpix;
+   unsigned int frame_size;
 
if (vsen->kthread_sen)
-   return -EINVAL;
+   /* tpg is already executing */
+   return 0;
 
/* Calculate the frame size */
vpix = vimc_pix_map_by_code(vsen->mbus_format.code);
-   vsen->frame_size = vsen->mbus_format.width * vpix->bpp *
-  vsen->mbus_format.height;
+   frame_size = vsen->mbus_format.width * vpix->bpp *
+vsen->mbus_format.height;
 
/*
   

[RFC PATCH v3 02/11] [media] vimc: Move common code from the core

2017-06-02 Thread Helen Koike
Remove helper functions from vimc-core and add it in vimc-common to
clean up the core.

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: Move common code from the core
- This is a new patch in the series

Changes in v2: None


---
 drivers/media/platform/vimc/Makefile   |   2 +-
 drivers/media/platform/vimc/vimc-capture.h |   2 +-
 drivers/media/platform/vimc/vimc-common.c  | 221 +
 .../platform/vimc/{vimc-core.h => vimc-common.h}   |   7 +-
 drivers/media/platform/vimc/vimc-core.c| 205 +--
 drivers/media/platform/vimc/vimc-sensor.h  |   2 +-
 6 files changed, 229 insertions(+), 210 deletions(-)
 create mode 100644 drivers/media/platform/vimc/vimc-common.c
 rename drivers/media/platform/vimc/{vimc-core.h => vimc-common.h} (96%)

diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
index c45195e..6b6ddf4 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,3 +1,3 @@
-vimc-objs := vimc-core.o vimc-capture.o vimc-sensor.o
+vimc-objs := vimc-core.o vimc-capture.o vimc-common.o vimc-sensor.o
 
 obj-$(CONFIG_VIDEO_VIMC) += vimc.o
diff --git a/drivers/media/platform/vimc/vimc-capture.h 
b/drivers/media/platform/vimc/vimc-capture.h
index 581a813..7e5c707 100644
--- a/drivers/media/platform/vimc/vimc-capture.h
+++ b/drivers/media/platform/vimc/vimc-capture.h
@@ -18,7 +18,7 @@
 #ifndef _VIMC_CAPTURE_H_
 #define _VIMC_CAPTURE_H_
 
-#include "vimc-core.h"
+#include "vimc-common.h"
 
 struct vimc_ent_device *vimc_cap_create(struct v4l2_device *v4l2_dev,
const char *const name,
diff --git a/drivers/media/platform/vimc/vimc-common.c 
b/drivers/media/platform/vimc/vimc-common.c
new file mode 100644
index 000..42f779a
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -0,0 +1,221 @@
+/*
+ * vimc-common.c Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015-2017 Helen Koike 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include "vimc-common.h"
+
+static const struct vimc_pix_map vimc_pix_map_list[] = {
+   /* TODO: add all missing formats */
+
+   /* RGB formats */
+   {
+   .code = MEDIA_BUS_FMT_BGR888_1X24,
+   .pixelformat = V4L2_PIX_FMT_BGR24,
+   .bpp = 3,
+   },
+   {
+   .code = MEDIA_BUS_FMT_RGB888_1X24,
+   .pixelformat = V4L2_PIX_FMT_RGB24,
+   .bpp = 3,
+   },
+   {
+   .code = MEDIA_BUS_FMT_ARGB_1X32,
+   .pixelformat = V4L2_PIX_FMT_ARGB32,
+   .bpp = 4,
+   },
+
+   /* Bayer formats */
+   {
+   .code = MEDIA_BUS_FMT_SBGGR8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SBGGR8,
+   .bpp = 1,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SGBRG8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SGBRG8,
+   .bpp = 1,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SGRBG8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SGRBG8,
+   .bpp = 1,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SRGGB8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SRGGB8,
+   .bpp = 1,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SBGGR10_1X10,
+   .pixelformat = V4L2_PIX_FMT_SBGGR10,
+   .bpp = 2,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SGBRG10_1X10,
+   .pixelformat = V4L2_PIX_FMT_SGBRG10,
+   .bpp = 2,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SGRBG10_1X10,
+   .pixelformat = V4L2_PIX_FMT_SGRBG10,
+   .bpp = 2,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SRGGB10_1X10,
+   .pixelformat = V4L2_PIX_FMT_SRGGB10,
+   .bpp = 2,
+   },
+
+   /* 10bit raw bayer a-law compressed to 8 bits */
+   {
+   .code = MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SBGGR10ALAW8,
+   .bpp = 1,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SGBRG10ALAW8,
+   .bpp = 1,
+   },
+   {
+   .code = MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8,
+   .pixelformat = V4L2_PIX_FMT_SGRBG10ALAW8,
+   .bpp = 1,
+  

[RFC PATCH v3 06/11] [media] vimc: sen: Support several image formats

2017-06-02 Thread Helen Koike
Allow user space to change the image format as the frame size, the
media bus pixel format, colorspace, quantization, field YCbCr encoding
and the transfer function

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: sen: Support several image formats
- remove support for V4L2_FIELD_ALTERNATE (left as TODO for now)
- clamp image size to an even dimension for height and width
- set default values for colorimetry using _DEFAULT macro
- reset all values of colorimetry to _DEFAULT if user tries to
set an invalid colorspace

Changes in v2:
[media] vimc: sen: Support several image formats
- this is a new commit in the serie (the old one was splitted in two)
- add init_cfg to initialize try_fmt
- reorder code in vimc_sen_set_fmt
- allow user space to change all fields from struct v4l2_mbus_framefmt
  (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
- merge with patch for the enum_mbus_code and enum_frame_size
- change commit message
- add vimc_pix_map_by_index
- rename MIN/MAX macros
- check set_fmt default parameters for quantization, colorspace 
...media] vimc: sen: Support several image formats


---
 drivers/media/platform/vimc/vimc-common.c |   8 ++
 drivers/media/platform/vimc/vimc-common.h |  12 +++
 drivers/media/platform/vimc/vimc-sensor.c | 145 --
 3 files changed, 136 insertions(+), 29 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-common.c 
b/drivers/media/platform/vimc/vimc-common.c
index 83d4251..ff59e09 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -144,6 +144,14 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
},
 };
 
+const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i)
+{
+   if (i >= ARRAY_SIZE(vimc_pix_map_list))
+   return NULL;
+
+   return _pix_map_list[i];
+}
+
 const struct vimc_pix_map *vimc_pix_map_by_code(u32 code)
 {
unsigned int i;
diff --git a/drivers/media/platform/vimc/vimc-common.h 
b/drivers/media/platform/vimc/vimc-common.h
index 60ebde2..2189fd6 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -22,6 +22,11 @@
 #include 
 #include 
 
+#define VIMC_FRAME_MAX_WIDTH 4096
+#define VIMC_FRAME_MAX_HEIGHT 2160
+#define VIMC_FRAME_MIN_WIDTH 16
+#define VIMC_FRAME_MIN_HEIGHT 16
+
 /**
  * struct vimc_pix_map - maps media bus code with v4l2 pixel format
  *
@@ -113,6 +118,13 @@ static inline void vimc_pads_cleanup(struct media_pad 
*pads)
 int vimc_pipeline_s_stream(struct media_entity *ent, int enable);
 
 /**
+ * vimc_pix_map_by_index - get vimc_pix_map struct by its index
+ *
+ * @i: index of the vimc_pix_map struct in vimc_pix_map_list
+ */
+const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i);
+
+/**
  * vimc_pix_map_by_code - get vimc_pix_map struct by media bus code
  *
  * @code:  media bus format code defined by MEDIA_BUS_FMT_* macros
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
index 6386ac1..90c41c6 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -24,8 +24,6 @@
 
 #include "vimc-sensor.h"
 
-#define VIMC_SEN_FRAME_MAX_WIDTH 4096
-
 struct vimc_sen_device {
struct vimc_ent_device ved;
struct v4l2_subdev sd;
@@ -36,18 +34,39 @@ struct vimc_sen_device {
struct v4l2_mbus_framefmt mbus_format;
 };
 
+static const struct v4l2_mbus_framefmt fmt_default = {
+   .width = 640,
+   .height = 480,
+   .code = MEDIA_BUS_FMT_RGB888_1X24,
+   .field = V4L2_FIELD_NONE,
+   .colorspace = V4L2_COLORSPACE_SRGB,
+};
+
+static int vimc_sen_init_cfg(struct v4l2_subdev *sd,
+struct v4l2_subdev_pad_config *cfg)
+{
+   unsigned int i;
+
+   for (i = 0; i < sd->entity.num_pads; i++) {
+   struct v4l2_mbus_framefmt *mf;
+
+   mf = v4l2_subdev_get_try_format(sd, cfg, i);
+   *mf = fmt_default;
+   }
+
+   return 0;
+}
+
 static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_mbus_code_enum *code)
 {
-   struct vimc_sen_device *vsen =
-   container_of(sd, struct vimc_sen_device, sd);
+   const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
 
-   /* TODO: Add support for other codes */
-   if (code->index)
+   if (!vpix)
return -EINVAL;
 
-   code->code = vsen->mbus_format.code;
+   code->code = vpix->code;
 
return 0;
 }
@@ -56,33 +75,34 @@ static int vimc_sen_enum_frame_size(struct v4l2_subdev *sd,
 

[RFC PATCH v3 07/11] [media] vimc: cap: Support several image formats

2017-06-02 Thread Helen Koike
Allow user space to change the image format as the frame size, the
pixel format, colorspace, quantization, field YCbCr encoding
and the transfer function

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: cap: Support several image formats
- use *_DEFAULT macros for colorimetry in the default format
- clamp height and width of the image by an even value
- is user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- remove V4L2_FMT_FLAG_COMPRESSED from vimc_cap_enum_fmt_vid_cap
- remove V4L2_BUF_TYPE_VIDEO_CAPTURE from vimc_cap_enum_fmt_vid_cap
- increase step_width and step_height to 2 instead of 1
- remove link validate function, use the one in vimc-common.c

Changes in v2:
[media] vimc: cap: Support several image formats
- this is a new commit in the serie (the old one was splitted in two)
- allow user space to change all fields from struct v4l2_pix_format
  (e.g. colospace, quantization, field, xfer_func, ycbcr_enc)
- link_validate and try_fmt: also checks colospace, quantization, 
field, xfer_func, ycbcr_enc
- add struct v4l2_pix_format fmt_default
- add enum_framesizes
- enum_fmt_vid_cap: enumerate all formats from vimc_pix_map_table
- add mode dev_dbg


---
 drivers/media/platform/vimc/vimc-capture.c | 131 +
 1 file changed, 115 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index 5bdecd1..e943267 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -40,6 +40,14 @@ struct vimc_cap_device {
struct media_pipeline pipe;
 };
 
+static const struct v4l2_pix_format fmt_default = {
+   .width = 640,
+   .height = 480,
+   .pixelformat = V4L2_PIX_FMT_RGB24,
+   .field = V4L2_FIELD_NONE,
+   .colorspace = V4L2_COLORSPACE_SRGB,
+};
+
 struct vimc_cap_buffer {
/*
 * struct vb2_v4l2_buffer must be the first element
@@ -73,7 +81,7 @@ static void vimc_cap_get_format(struct vimc_ent_device *ved,
*fmt = vcap->format;
 }
 
-static int vimc_cap_fmt_vid_cap(struct file *file, void *priv,
+static int vimc_cap_g_fmt_vid_cap(struct file *file, void *priv,
  struct v4l2_format *f)
 {
struct vimc_cap_device *vcap = video_drvdata(file);
@@ -83,16 +91,112 @@ static int vimc_cap_fmt_vid_cap(struct file *file, void 
*priv,
return 0;
 }
 
+static int vimc_cap_try_fmt_vid_cap(struct file *file, void *priv,
+   struct v4l2_format *f)
+{
+   struct v4l2_pix_format *format = >fmt.pix;
+   const struct vimc_pix_map *vpix;
+
+   format->width = clamp_t(u32, format->width, VIMC_FRAME_MIN_WIDTH,
+   VIMC_FRAME_MAX_WIDTH) & ~1;
+   format->height = clamp_t(u32, format->height, VIMC_FRAME_MIN_HEIGHT,
+VIMC_FRAME_MAX_HEIGHT) & ~1;
+
+   /* Don't accept a pixelformat that is not on the table */
+   vpix = vimc_pix_map_by_pixelformat(format->pixelformat);
+   if (!vpix) {
+   format->pixelformat = fmt_default.pixelformat;
+   vpix = vimc_pix_map_by_pixelformat(format->pixelformat);
+   }
+   /* TODO: Add support for custom bytesperline values */
+   format->bytesperline = format->width * vpix->bpp;
+   format->sizeimage = format->bytesperline * format->height;
+
+   if (format->field == V4L2_FIELD_ANY)
+   format->field = fmt_default.field;
+
+   if (format->colorspace == V4L2_COLORSPACE_DEFAULT)
+   format->colorspace = fmt_default.colorspace;
+
+   /* Check if values are out of range */
+   if (format->colorspace > V4L2_COLORSPACE_DCI_P3) {
+   format->colorspace = fmt_default.colorspace;
+   format->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+   format->quantization = V4L2_QUANTIZATION_DEFAULT;
+   format->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+   }
+   if (format->ycbcr_enc > V4L2_YCBCR_ENC_SMPTE240M)
+   format->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+   if (format->quantization > V4L2_QUANTIZATION_LIM_RANGE)
+   format->quantization = V4L2_QUANTIZATION_DEFAULT;
+   if (format->xfer_func > V4L2_XFER_FUNC_SMPTE2084)
+   format->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+   return 0;
+}
+
+static int vimc_cap_s_fmt_vid_cap(struct file *file, void *priv,
+ struct v4l2_format *f)
+{
+   struct vimc_cap_device *vcap = video_drvdata(file);
+
+   /* Do not change the format while stream is on */
+   if (vb2_is_busy(>queue))
+   return -EBUSY;
+
+   vimc_cap_try_fmt_vid_cap(file, priv, f);
+
+   dev_dbg(vcap->vdev.v4l2_dev->dev, "%s: 

[RFC PATCH v3 08/11] [media] vimc: Optimize frame generation through the pipe

2017-06-02 Thread Helen Koike
Add a parameter called vsen_tpg, if true then vimc will work as before:
frames will be generated in the sensor nodes then propagated through the
pipe and processed by each node until a capture node is reached.
If vsen_tpg is false, then the frame is not generated by the sensor, but
directly from the capture node, thus saving intermediate memory buffers
and process time, allowing a higher frame rate.

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: Optimize frame generation through the pipe
- This is a new patch in the series

Changes in v2: None


---
 drivers/media/platform/vimc/vimc-capture.c | 178 +
 drivers/media/platform/vimc/vimc-common.h  |   2 +
 drivers/media/platform/vimc/vimc-sensor.c  |  30 -
 3 files changed, 156 insertions(+), 54 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index e943267..b5da0ea 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -15,7 +15,10 @@
  *
  */
 
+#include 
+#include 
 #include 
+#include 
 #include 
 #include 
 
@@ -38,6 +41,8 @@ struct vimc_cap_device {
struct mutex lock;
u32 sequence;
struct media_pipeline pipe;
+   struct tpg_data tpg;
+   struct task_struct *kthread_cap;
 };
 
 static const struct v4l2_pix_format fmt_default = {
@@ -246,6 +251,91 @@ static void vimc_cap_return_all_buffers(struct 
vimc_cap_device *vcap,
spin_unlock(>qlock);
 }
 
+static void vimc_cap_process_frame(struct vimc_ent_device *ved,
+  struct media_pad *sink, const void *frame)
+{
+   struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
+   ved);
+   struct vimc_cap_buffer *vimc_buf;
+   void *vbuf;
+
+   spin_lock(>qlock);
+
+   /* Get the first entry of the list */
+   vimc_buf = list_first_entry_or_null(>buf_list,
+   typeof(*vimc_buf), list);
+   if (!vimc_buf) {
+   spin_unlock(>qlock);
+   return;
+   }
+
+   /* Remove this entry from the list */
+   list_del(_buf->list);
+
+   spin_unlock(>qlock);
+
+   /* Fill the buffer */
+   vimc_buf->vb2.vb2_buf.timestamp = ktime_get_ns();
+   vimc_buf->vb2.sequence = vcap->sequence++;
+   vimc_buf->vb2.field = vcap->format.field;
+
+   vbuf = vb2_plane_vaddr(_buf->vb2.vb2_buf, 0);
+
+   if (sink)
+   memcpy(vbuf, frame, vcap->format.sizeimage);
+   else
+   tpg_fill_plane_buffer(>tpg, V4L2_STD_PAL, 0, vbuf);
+
+   /* Set it as ready */
+   vb2_set_plane_payload(_buf->vb2.vb2_buf, 0,
+ vcap->format.sizeimage);
+   vb2_buffer_done(_buf->vb2.vb2_buf, VB2_BUF_STATE_DONE);
+}
+
+
+static int vimc_cap_tpg_thread(void *data)
+{
+   struct vimc_cap_device *vcap = data;
+
+   set_freezable();
+   set_current_state(TASK_UNINTERRUPTIBLE);
+
+   for (;;) {
+   try_to_freeze();
+   if (kthread_should_stop())
+   break;
+
+   vimc_cap_process_frame(>ved, NULL, NULL);
+
+   /* 60 frames per second */
+   schedule_timeout(HZ/60);
+   }
+
+   return 0;
+}
+
+static void vimc_cap_tpg_s_format(struct vimc_cap_device *vcap)
+{
+   const struct vimc_pix_map *vpix =
+   vimc_pix_map_by_pixelformat(vcap->format.pixelformat);
+
+   tpg_reset_source(>tpg, vcap->format.width, vcap->format.height,
+vcap->format.field);
+   tpg_s_bytesperline(>tpg, 0, vcap->format.width * vpix->bpp);
+   tpg_s_buf_height(>tpg, vcap->format.height);
+   tpg_s_fourcc(>tpg, vpix->pixelformat);
+   /*
+* TODO: check why the tpg_s_field need this third argument if
+* it is already receiving the field
+*/
+   tpg_s_field(>tpg, vcap->format.field,
+   vcap->format.field == V4L2_FIELD_ALTERNATE);
+   tpg_s_colorspace(>tpg, vcap->format.colorspace);
+   tpg_s_ycbcr_enc(>tpg, vcap->format.ycbcr_enc);
+   tpg_s_quantization(>tpg, vcap->format.quantization);
+   tpg_s_xfer_func(>tpg, vcap->format.xfer_func);
+}
+
 static int vimc_cap_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
struct vimc_cap_device *vcap = vb2_get_drv_priv(vq);
@@ -256,20 +346,43 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 
/* Start the media pipeline */
ret = media_pipeline_start(entity, >pipe);
-   if (ret) {
-   vimc_cap_return_all_buffers(vcap, VB2_BUF_STATE_QUEUED);
-   return ret;
-   }
+   if (ret)
+   goto err_ret_all_buffs;
 
/* Enable streaming from the pipe */
ret = vimc_pipeline_s_stream(>vdev.entity, 1);
- 

[RFC PATCH v3 10/11] [media] vimc: deb: Add debayer filter

2017-06-02 Thread Helen Koike
Implement the debayer filter and integrate it with the core

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: deb: Add debayer filter
- Declare frame_size as a local variable
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: add ret variable to propagate return errors
- structure code to be a module, use platform_driver and component 
system
- fix multiline comment
- s/thought/through
- s/RGB/RGB888
- clamp height and width of the image by an even value
- if user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- uset _DEFAULT for colorimetry in the default format

Changes in v2:
[media] vimc: deb: Add debayer filter
- Using MEDIA_ENT_F_ATV_DECODER in function
- remove v4l2_dev and dev from vimc_deb_device struct
- src fmt propagates from the sink
- coding style
- remove redundant else if statements
- check end of enum and remove BUG_ON
- enum frame size with min and max values
- set/try fmt
- remove unecessary include freezer.h
- check pad types on create
- return EBUSY when trying to set the format while stream is on
- remove vsd struct
- add IS_SRC and IS_SINK macros
- add deb_mean_win_size as a parameter of the module
- check set_fmt default parameters for quantization, colorspace ...
- add more dev_dbg


---
 drivers/media/platform/vimc/Makefile   |   4 +-
 drivers/media/platform/vimc/vimc-common.h  |   2 +
 drivers/media/platform/vimc/vimc-debayer.c | 615 +
 3 files changed, 620 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/platform/vimc/vimc-debayer.c

diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
index 0e5d5ce..4fba8ef 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,6 +1,8 @@
 vimc-objs := vimc-core.o
 vimc_capture-objs := vimc-capture.o
 vimc_common-objs := vimc-common.o
+vimc_debayer-objs := vimc-debayer.o
 vimc_sensor-objs := vimc-sensor.o
 
-obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc_capture.o vimc_common.o vimc_sensor.o
+obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc_capture.o vimc_common.o vimc-debayer.o 
\
+   vimc_sensor.o
diff --git a/drivers/media/platform/vimc/vimc-common.h 
b/drivers/media/platform/vimc/vimc-common.h
index 27c9b8c..c63d51f 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -29,6 +29,8 @@
 
 #define VIMC_PIPE_OPT 1
 
+#define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp)
+
 /**
  * struct vimc_pix_map - maps media bus code with v4l2 pixel format
  *
diff --git a/drivers/media/platform/vimc/vimc-debayer.c 
b/drivers/media/platform/vimc/vimc-debayer.c
new file mode 100644
index 000..9f9604c
--- /dev/null
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -0,0 +1,615 @@
+/*
+ * vimc-debayer.c Virtual Media Controller Driver
+ *
+ * Copyright (C) 2015-2017 Helen Koike 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "vimc-common.h"
+
+#define VIMC_DEB_DRV_NAME "vimc-debayer"
+
+static unsigned int deb_mean_win_size = 3;
+module_param(deb_mean_win_size, uint, );
+MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n"
+   "NOTE: the window size need to be an odd number, as the main pixel "
+   "stays in the center of the window, otherwise the next odd number "
+   "is considered");
+
+#define IS_SINK(pad) (!pad)
+#define IS_SRC(pad)  (pad)
+
+enum vimc_deb_rgb_colors {
+   VIMC_DEB_RED = 0,
+   VIMC_DEB_GREEN = 1,
+   VIMC_DEB_BLUE = 2,
+};
+
+struct vimc_deb_pix_map {
+   u32 code;
+   enum vimc_deb_rgb_colors order[2][2];
+};
+
+struct vimc_deb_device {
+   struct vimc_ent_device ved;
+   struct v4l2_subdev sd;
+   struct device *dev;
+   /* The active format */
+   struct v4l2_mbus_framefmt sink_fmt;
+   u32 src_code;
+   void (*set_rgb_src)(struct vimc_deb_device *vdeb, unsigned int lin,
+   unsigned int col, unsigned int rgb[3]);
+   /* Values calculated when the stream 

[RFC PATCH v3 09/11] [media] vimc: Subdevices as modules

2017-06-02 Thread Helen Koike
Change the core structure for adding subdevices in the topology.
Instead of calling the specific create function for each subdevice,
inject a child platform_device with the driver's name.
Each type of node in the topology (sensor, capture, debayer, scaler)
will register a platform_driver with the corresponding name through the
component subsystem.
Implementing a new subdevice type doesn't require vimc-core to be altered.

This facilitates future implementation of dynamic entities, where
hotpluging an entity in the topology is just a matter of
registering/unregistering a platform_device in the system.
It also facilitates other implementations of different nodes without
touching the core code and remove the need of a header file for each
type of node.

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: Subdevices as modules
- This is a new patch in the series

Changes in v2: None


---
 drivers/media/platform/vimc/Makefile   |   7 +-
 drivers/media/platform/vimc/vimc-capture.c |  96 ---
 drivers/media/platform/vimc/vimc-capture.h |  28 --
 drivers/media/platform/vimc/vimc-common.c  |  37 ++-
 drivers/media/platform/vimc/vimc-common.h  |  14 +-
 drivers/media/platform/vimc/vimc-core.c| 406 +++--
 drivers/media/platform/vimc/vimc-sensor.c  |  91 +--
 drivers/media/platform/vimc/vimc-sensor.h  |  28 --
 8 files changed, 320 insertions(+), 387 deletions(-)
 delete mode 100644 drivers/media/platform/vimc/vimc-capture.h
 delete mode 100644 drivers/media/platform/vimc/vimc-sensor.h

diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
index 6b6ddf4..0e5d5ce 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -1,3 +1,6 @@
-vimc-objs := vimc-core.o vimc-capture.o vimc-common.o vimc-sensor.o
+vimc-objs := vimc-core.o
+vimc_capture-objs := vimc-capture.o
+vimc_common-objs := vimc-common.o
+vimc_sensor-objs := vimc-sensor.o
 
-obj-$(CONFIG_VIDEO_VIMC) += vimc.o
+obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc_capture.o vimc_common.o vimc_sensor.o
diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index b5da0ea..633d99a 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -15,18 +15,24 @@
  *
  */
 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
 #include 
 
-#include "vimc-capture.h"
+#include "vimc-common.h"
+
+#define VIMC_CAP_DRV_NAME "vimc-capture"
 
 struct vimc_cap_device {
struct vimc_ent_device ved;
struct video_device vdev;
+   struct device *dev;
struct v4l2_pix_format format;
struct vb2_queue queue;
struct list_head buf_list;
@@ -150,7 +156,7 @@ static int vimc_cap_s_fmt_vid_cap(struct file *file, void 
*priv,
 
vimc_cap_try_fmt_vid_cap(file, priv, f);
 
-   dev_dbg(vcap->vdev.v4l2_dev->dev, "%s: format update: "
+   dev_dbg(vcap->dev, "%s: format update: "
"old:%dx%d (0x%x, %d, %d, %d, %d) "
"new:%dx%d (0x%x, %d, %d, %d, %d)\n", vcap->vdev.name,
/* old */
@@ -365,8 +371,7 @@ static int vimc_cap_start_streaming(struct vb2_queue *vq, 
unsigned int count)
vcap->kthread_cap = kthread_run(vimc_cap_tpg_thread, vcap,
"%s-cap", vcap->vdev.v4l2_dev->name);
if (IS_ERR(vcap->kthread_cap)) {
-   dev_err(vcap->vdev.v4l2_dev->dev,
-   "%s: kernel_thread() failed\n",
+   dev_err(vcap->dev, "%s: kernel_thread() failed\n",
vcap->vdev.name);
ret = PTR_ERR(vcap->kthread_cap);
goto err_tpg_free;
@@ -443,8 +448,7 @@ static int vimc_cap_buffer_prepare(struct vb2_buffer *vb)
unsigned long size = vcap->format.sizeimage;
 
if (vb2_plane_size(vb, 0) < size) {
-   dev_err(vcap->vdev.v4l2_dev->dev,
-   "%s: buffer too small (%lu < %lu)\n",
+   dev_err(vcap->dev, "%s: buffer too small (%lu < %lu)\n",
vcap->vdev.name, vb2_plane_size(vb, 0), size);
return -EINVAL;
}
@@ -469,8 +473,10 @@ static const struct media_entity_operations vimc_cap_mops 
= {
.link_validate  = vimc_link_validate,
 };
 
-static void vimc_cap_destroy(struct vimc_ent_device *ved)
+static void vimc_cap_comp_unbind(struct device *comp, struct device *master,
+void *master_data)
 {
+   struct vimc_ent_device *ved = dev_get_drvdata(comp);
struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
ved);
 
@@ -481,32 +487,25 @@ static void vimc_cap_destroy(struct vimc_ent_device *ved)
kfree(vcap);
 }
 
-struct 

[RFC PATCH v3 11/11] [media] vimc: sca: Add scaler

2017-06-02 Thread Helen Koike
Implement scaler and integrated with the core

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: sca: Add scaler
- Declare frame_size as a local variable
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: add ret variable to propagate return errors
- structure code to be a module, use platform_driver and component 
system
- s/thought/through
- clamp height and width of the image by an even value
- if user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- uset _DEFAULT for colorimetry in the default format

Changes in v2:
[media] vimc: sca: Add scaler
- Add function MEDIA_ENT_F_IO_V4L
- remove v4l2_dev and dev
- s/sink_mbus_fmt/sink_fmt
- remove BUG_ON, remove redundant if else, rewrite TODO, check end of 
enum
- rm src_width/height, enum fsize with min and max values
- set/try fmt
- remove unecessary include freezer.h
- core: add bayer boolean in pixel table
- coding style
- fix bug in enum frame size
- check pad types on create
- return EBUSY when trying to set the format while stream is on
- remove vsd struct
- add IS_SRC and IS_SINK macros
- add sca_mult as a parameter of the module
- check set_fmt default parameters for quantization, colorspace ...
- add more dev_dbg


---
 drivers/media/platform/vimc/Makefile  |   3 +-
 drivers/media/platform/vimc/vimc-common.c |  27 ++
 drivers/media/platform/vimc/vimc-common.h |   1 +
 drivers/media/platform/vimc/vimc-scaler.c | 469 ++
 4 files changed, 499 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/platform/vimc/vimc-scaler.c

diff --git a/drivers/media/platform/vimc/Makefile 
b/drivers/media/platform/vimc/Makefile
index 4fba8ef..68c5d98 100644
--- a/drivers/media/platform/vimc/Makefile
+++ b/drivers/media/platform/vimc/Makefile
@@ -2,7 +2,8 @@ vimc-objs := vimc-core.o
 vimc_capture-objs := vimc-capture.o
 vimc_common-objs := vimc-common.o
 vimc_debayer-objs := vimc-debayer.o
+vimc_scaler-objs := vimc-scaler.o
 vimc_sensor-objs := vimc-sensor.o
 
 obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc_capture.o vimc_common.o vimc-debayer.o 
\
-   vimc_sensor.o
+   vimc_scaler.o vimc_sensor.o
diff --git a/drivers/media/platform/vimc/vimc-common.c 
b/drivers/media/platform/vimc/vimc-common.c
index d5ed387..68887d7 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -20,6 +20,10 @@
 
 #include "vimc-common.h"
 
+/*
+ * NOTE: non-bayer formats need to come first (necessary for enum_mbus_code
+ * in the scaler)
+ */
 static const struct vimc_pix_map vimc_pix_map_list[] = {
/* TODO: add all missing formats */
 
@@ -28,16 +32,19 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
.code = MEDIA_BUS_FMT_BGR888_1X24,
.pixelformat = V4L2_PIX_FMT_BGR24,
.bpp = 3,
+   .bayer = false,
},
{
.code = MEDIA_BUS_FMT_RGB888_1X24,
.pixelformat = V4L2_PIX_FMT_RGB24,
.bpp = 3,
+   .bayer = false,
},
{
.code = MEDIA_BUS_FMT_ARGB_1X32,
.pixelformat = V4L2_PIX_FMT_ARGB32,
.bpp = 4,
+   .bayer = false,
},
 
/* Bayer formats */
@@ -45,41 +52,49 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
.code = MEDIA_BUS_FMT_SBGGR8_1X8,
.pixelformat = V4L2_PIX_FMT_SBGGR8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGBRG8_1X8,
.pixelformat = V4L2_PIX_FMT_SGBRG8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGRBG8_1X8,
.pixelformat = V4L2_PIX_FMT_SGRBG8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SRGGB8_1X8,
.pixelformat = V4L2_PIX_FMT_SRGGB8,
.bpp = 1,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SBGGR10_1X10,
.pixelformat = V4L2_PIX_FMT_SBGGR10,
.bpp = 2,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGBRG10_1X10,
.pixelformat = V4L2_PIX_FMT_SGBRG10,
.bpp = 2,
+   .bayer = true,
},
{
.code = MEDIA_BUS_FMT_SGRBG10_1X10,
.pixelformat = V4L2_PIX_FMT_SGRBG10,
.bpp = 2,
+   .bayer = true,

[RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link_validate

2017-06-02 Thread Helen Koike
All links will be checked in the same way. Adding a helper function for
that

Signed-off-by: Helen Koike 

---

Changes in v3:
[media] vimc: common: Add vimc_link_validate
- this is a new patch in the series

Changes in v2: None


---
 drivers/media/platform/vimc/vimc-capture.c |  78 +++---
 drivers/media/platform/vimc/vimc-common.c  | 124 -
 drivers/media/platform/vimc/vimc-common.h  |  14 
 3 files changed, 148 insertions(+), 68 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c 
b/drivers/media/platform/vimc/vimc-capture.c
index 93f6a09..5bdecd1 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -64,6 +64,15 @@ static int vimc_cap_querycap(struct file *file, void *priv,
return 0;
 }
 
+static void vimc_cap_get_format(struct vimc_ent_device *ved,
+   struct v4l2_pix_format *fmt)
+{
+   struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device,
+   ved);
+
+   *fmt = vcap->format;
+}
+
 static int vimc_cap_fmt_vid_cap(struct file *file, void *priv,
  struct v4l2_format *f)
 {
@@ -231,74 +240,8 @@ static const struct vb2_ops vimc_cap_qops = {
.wait_finish= vb2_ops_wait_finish,
 };
 
-/*
- * NOTE: this function is a copy of v4l2_subdev_link_validate_get_format
- * maybe the v4l2 function should be public
- */
-static int vimc_cap_v4l2_subdev_link_validate_get_format(struct media_pad *pad,
-   struct v4l2_subdev_format *fmt)
-{
-   struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(pad->entity);
-
-   fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
-   fmt->pad = pad->index;
-
-   return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
-}
-
-static int vimc_cap_link_validate(struct media_link *link)
-{
-   struct v4l2_subdev_format source_fmt;
-   const struct vimc_pix_map *vpix;
-   struct vimc_cap_device *vcap = container_of(link->sink->entity,
-   struct vimc_cap_device,
-   vdev.entity);
-   struct v4l2_pix_format *sink_fmt = >format;
-   int ret;
-
-   /*
-* if it is a raw node from vimc-core, ignore the link for now
-* TODO: remove this when there are no more raw nodes in the
-* core and return error instead
-*/
-   if (link->source->entity->obj_type == MEDIA_ENTITY_TYPE_BASE)
-   return 0;
-
-   /* Get the the format of the subdev */
-   ret = vimc_cap_v4l2_subdev_link_validate_get_format(link->source,
-   _fmt);
-   if (ret)
-   return ret;
-
-   dev_dbg(vcap->vdev.v4l2_dev->dev,
-   "%s: link validate formats src:%dx%d %d sink:%dx%d %d\n",
-   vcap->vdev.name,
-   source_fmt.format.width, source_fmt.format.height,
-   source_fmt.format.code,
-   sink_fmt->width, sink_fmt->height,
-   sink_fmt->pixelformat);
-
-   /* The width, height and code must match. */
-   vpix = vimc_pix_map_by_pixelformat(sink_fmt->pixelformat);
-   if (source_fmt.format.width != sink_fmt->width
-   || source_fmt.format.height != sink_fmt->height
-   || vpix->code != source_fmt.format.code)
-   return -EPIPE;
-
-   /*
-* The field order must match, or the sink field order must be NONE
-* to support interlaced hardware connected to bridges that support
-* progressive formats only.
-*/
-   if (source_fmt.format.field != sink_fmt->field &&
-   sink_fmt->field != V4L2_FIELD_NONE)
-   return -EPIPE;
-
-   return 0;
-}
-
 static const struct media_entity_operations vimc_cap_mops = {
-   .link_validate  = vimc_cap_link_validate,
+   .link_validate  = vimc_link_validate,
 };
 
 static void vimc_cap_destroy(struct vimc_ent_device *ved)
@@ -434,6 +377,7 @@ struct vimc_ent_device *vimc_cap_create(struct v4l2_device 
*v4l2_dev,
vcap->ved.destroy = vimc_cap_destroy;
vcap->ved.ent = >vdev.entity;
vcap->ved.process_frame = vimc_cap_process_frame;
+   vcap->ved.vdev_get_format = vimc_cap_get_format;
 
/* Initialize the video_device struct */
vdev = >vdev;
diff --git a/drivers/media/platform/vimc/vimc-common.c 
b/drivers/media/platform/vimc/vimc-common.c
index f809a9d..83d4251 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -252,8 +252,130 @@ int vimc_pipeline_s_stream(struct media_entity *ent, int 
enable)
return 0;
 }
 
+static void vimc_fmt_pix_to_mbus(struct v4l2_mbus_framefmt *mfmt,
+struct 

[RFC PATCH v3 00/11] [media]: vimc: Virtual Media Control VPU's

2017-06-02 Thread Helen Koike
This patch series improves the current video processing units in vimc
(by adding more controls to the sensor and capture node, allowing the
user to configure different frame formats) and also adds a debayer
and a scaler node.
The debayer transforms the bayer format image received in its sink pad
to a bayer format by averaging the pixels within a mean window.
The scaler only scales up the image for now.

In this version I added an optimization where the image can be generated
direct in the capture device instead of being generated in the sensor
and processed by each node in the topology.
I also changed the approach to implement each node of the topology as a
submodule to make the code component oriented, where new components
won't need to touch vimc-core and won't need a header file.
Please, let me know your view regarding this new approach.

Changes in v3:
[media] vimc: sen: Integrate the tpg on the sensor
- Declare frame_size as a local variable
- Set tpg frame format before starting kthread
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: propagate error from kthread_stop
- coding style when calling tpg_s_bytesperline
- s/vimc_thread_sen/vimc_sen_tpg_thread
- fix multiline comment
[media] vimc: Move common code from the core
- This is a new patch in the series
[media] vimc: Add vimc_ent_sd_* helper functions
- add it in vimc-common.c instead in vimc-core.c
- fix vimc_ent_sd_register, use function parameter to assign
sd->entity.function instead of using a fixed value
- rename commit to add the "common" tag
[media] vimc: Add vimc_pipeline_s_stream in the core
- add it in vimc-common instead of vimc-core
- rename commit with "common" tag
[media] vimc: common: Add vimc_link_validate
- this is a new patch in the series
[media] vimc: sen: Support several image formats
- remove support for V4L2_FIELD_ALTERNATE (left as TODO for now)
- clamp image size to an even dimension for height and width
- set default values for colorimetry using _DEFAULT macro
- reset all values of colorimetry to _DEFAULT if user tries to
set an invalid colorspace
[media] vimc: cap: Support several image formats
- use *_DEFAULT macros for colorimetry in the default format
- clamp height and width of the image by an even value
- is user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- remove V4L2_FMT_FLAG_COMPRESSED from vimc_cap_enum_fmt_vid_cap
- remove V4L2_BUF_TYPE_VIDEO_CAPTURE from vimc_cap_enum_fmt_vid_cap
- increase step_width and step_height to 2 instead of 1
- remove link validate function, use the one in vimc-common.c
[media] vimc: Optimize frame generation through the pipe
- This is a new patch in the series
[media] vimc: Subdevices as modules
- This is a new patch in the series
[media] vimc: deb: Add debayer filter
- Declare frame_size as a local variable
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: add ret variable to propagate return errors
- structure code to be a module, use platform_driver and component 
system
- fix multiline comment
- s/thought/through
- s/RGB/RGB888
- clamp height and width of the image by an even value
- if user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- uset _DEFAULT for colorimetry in the default format
[media] vimc: sca: Add scaler
- Declare frame_size as a local variable
- s_stream(sd, 1): return 0 if stream is already enabled
- s_stream(sd, 0): return 0 if stream is already disabled
- s_stream: add ret variable to propagate return errors
- structure code to be a module, use platform_driver and component 
system
- s/thought/through
- clamp height and width of the image by an even value
- if user try to set colorspace to an invalid format, set all
colorimetry parameters to _DEFAULT
- uset _DEFAULT for colorimetry in the default format

Changes in v2:
[media] vimc: sen: Integrate the tpg on the sensor
- Fix include location
- Select V4L2_TPG in Kconfig
- configure tpg on streamon only
- rm BUG_ON
- coding style
- remove V4L2_FIELD_ALTERNATE from tpg_s_field
- remove V4L2_STD_PAL from tpg_fill_plane_buffer
[media] vimc: Add vimc_ent_sd_* helper functions
- Comments in vimc_ent_sd_init
- Update vimc_ent_sd_init with upstream code as media_entity_pads_init
(instead of media_entity_init), entity->function intead of entity->type
- Add missing 

Re: [PATCH 5/9] [media] s5p-jpeg: Add IOMMU support

2017-06-02 Thread Shuah Khan
On Fri, Jun 2, 2017 at 10:02 AM, Thierry Escande
 wrote:
> From: Tony K Nadackal 
>
> This patch adds support for IOMMU s5p-jpeg driver if the Exynos IOMMU
> and ARM DMA IOMMU configurations are supported. The address space is
> created with size limited to 256M and base address set to 0x2000.
>
> Signed-off-by: Tony K Nadackal 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 77 
> +
>  1 file changed, 77 insertions(+)
>
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 770a709..5569b99 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -28,6 +28,14 @@
>  #include 
>  #include 
>  #include 
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
>
>  #include "jpeg-core.h"
>  #include "jpeg-hw-s5p.h"
> @@ -35,6 +43,10 @@
>  #include "jpeg-hw-exynos3250.h"
>  #include "jpeg-regs.h"
>
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +static struct dma_iommu_mapping *mapping;
> +#endif
> +
>  static struct s5p_jpeg_fmt sjpeg_formats[] = {
> {
> .name   = "JPEG JFIF",
> @@ -956,6 +968,60 @@ static void exynos4_jpeg_parse_q_tbl(struct s5p_jpeg_ctx 
> *ctx)
> }
>  }
>
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +static int jpeg_iommu_init(struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +   int err;
> +
> +   mapping = arm_iommu_create_mapping(_bus_type, 0x2000,
> +  SZ_512M);

Change log says 256M??

What happens when another driver uses the same start point?
exynos drm uses the same  looks like

EXYNOS_DEV_ADDR_START   0x2000

> +   if (IS_ERR(mapping)) {
> +   dev_err(dev, "IOMMU mapping failed\n");
> +   return PTR_ERR(mapping);
> +   }
> +
> +   dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms), 
> GFP_KERNEL);
> +   if (!dev->dma_parms) {
> +   err = -ENOMEM;
> +   goto error_alloc;
> +   }
> +
> +   err = dma_set_max_seg_size(dev, 0xu);

You could use DMA_BIT_MASK(32) instead of 0xu

> +   if (err)
> +   goto error;
> +
> +   err = arm_iommu_attach_device(dev, mapping);
> +   if (err)
> +   goto error;
> +
> +   return 0;
> +
> +error:
> +   devm_kfree(dev, dev->dma_parms);
> +   dev->dma_parms = NULL;
> +
> +error_alloc:
> +   arm_iommu_release_mapping(mapping);
> +   mapping = NULL;
> +
> +   return err;
> +}
> +
> +static void jpeg_iommu_deinit(struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +
> +   if (mapping) {
> +   arm_iommu_detach_device(dev);
> +   devm_kfree(dev, dev->dma_parms);
> +   dev->dma_parms = NULL;
> +   arm_iommu_release_mapping(mapping);
> +   mapping = NULL;
> +   }
> +}
> +#endif
> +
>  /*
>   * 
> 
>   * Device file operations
> @@ -2816,6 +2882,13 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
> spin_lock_init(>slock);
> jpeg->dev = >dev;
>
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +   ret = jpeg_iommu_init(pdev);
> +   if (ret) {
> +   dev_err(>dev, "IOMMU Initialization failed\n");
> +   return ret;
> +   }
> +#endif

You might be able to avoid use of ifdefs if you define stubs for !defines case.

> /* memory-mapped registers */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> @@ -2962,6 +3035,10 @@ static int s5p_jpeg_remove(struct platform_device 
> *pdev)
> clk_disable_unprepare(jpeg->clocks[i]);
> }
>
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +   jpeg_iommu_deinit(pdev);
> +#endif
> +
> return 0;
>  }
>
> --
> 2.7.4
>


Re: [PATCH 9/9] [media] s5p-jpeg: Add support for multi-planar APIs

2017-06-02 Thread Jacek Anaszewski
Hi Thierry,

What is the gain of introducing multiplanar API for this hardware?
AFAIR all the HW implementations store the data in a single contiguous
memory region and use suitable padding between planes.

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: Ricky Liang 
> 
> This patch adds multi-planar APIs to the s5p-jpeg driver. The multi-planar
> APIs are identical to the exisiting single-planar APIs except the plane
> format info is stored in the v4l2_pixel_format_mplan struct instead
> of the v4l2_pixel_format struct.
> 
> Signed-off-by: Ricky Liang 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 152 
> +---
>  drivers/media/platform/s5p-jpeg/jpeg-core.h |   2 +
>  2 files changed, 139 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index db56135..a8fd7ed 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -1371,6 +1371,15 @@ static int s5p_jpeg_querycap(struct file *file, void 
> *priv,
>dev_name(ctx->jpeg->dev));
>   cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M;
>   cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> + /*
> +  * Advertise multi-planar capabilities. The driver supports only
> +  * single-planar pixel format at this moment so all the buffers will
> +  * have only one plane.
> +  */
> + cap->capabilities |= V4L2_CAP_VIDEO_M2M_MPLANE |
> +  V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> +  V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> +
>   return 0;
>  }
>  
> @@ -1430,12 +1439,10 @@ static int s5p_jpeg_enum_fmt_vid_out(struct file 
> *file, void *priv,
>  static struct s5p_jpeg_q_data *get_q_data(struct s5p_jpeg_ctx *ctx,
> enum v4l2_buf_type type)
>  {
> - if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> + if (V4L2_TYPE_IS_OUTPUT(type))
>   return >out_q;
> - if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> - return >cap_q;
>  
> - return NULL;
> + return >cap_q;
>  }
>  
>  static int s5p_jpeg_g_fmt(struct file *file, void *priv, struct v4l2_format 
> *f)
> @@ -1449,16 +1456,14 @@ static int s5p_jpeg_g_fmt(struct file *file, void 
> *priv, struct v4l2_format *f)
>   if (!vq)
>   return -EINVAL;
>  
> - if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + if (!V4L2_TYPE_IS_OUTPUT(f->type) &&
>   ct->mode == S5P_JPEG_DECODE && !ct->hdr_parsed)
>   return -EINVAL;
>   q_data = get_q_data(ct, f->type);
>   BUG_ON(q_data == NULL);
>  
> - if ((f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> -  ct->mode == S5P_JPEG_ENCODE) ||
> - (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
> -  ct->mode == S5P_JPEG_DECODE)) {
> + if ((!V4L2_TYPE_IS_OUTPUT(f->type) && ct->mode == S5P_JPEG_ENCODE) ||
> + (V4L2_TYPE_IS_OUTPUT(f->type) && ct->mode == S5P_JPEG_DECODE)) {
>   pix->width = 0;
>   pix->height = 0;
>   } else {
> @@ -1715,6 +1720,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, 
> struct v4l2_format *f)
>  
>   q_data = get_q_data(ct, f->type);
>   BUG_ON(q_data == NULL);
> + vq->type = f->type;
> + q_data->type = f->type;
>  
>   if (vb2_is_busy(vq)) {
>   v4l2_err(>jpeg->v4l2_dev, "%s queue busy\n", __func__);
> @@ -1919,7 +1926,9 @@ static int s5p_jpeg_g_selection(struct file *file, void 
> *priv,
>   struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
>  
>   if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
> - s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
> + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>   return -EINVAL;
>  
>   /* For JPEG blob active == default == bounds */
> @@ -1957,7 +1966,8 @@ static int s5p_jpeg_s_selection(struct file *file, void 
> *fh,
>   struct v4l2_rect *rect = >r;
>   int ret = -EINVAL;
>  
> - if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>   return -EINVAL;
>  
>   if (s->target == V4L2_SEL_TGT_COMPOSE) {
> @@ -2118,6 +2128,107 @@ static int s5p_jpeg_controls_create(struct 
> s5p_jpeg_ctx *ctx)
>   return ret;
>  }
>  
> +static void v4l2_format_pixmp_to_pix(struct v4l2_format *fmt_pix_mp,
> +  struct v4l2_format *fmt_pix) {
> + struct v4l2_pix_format *pix = _pix->fmt.pix;
> + struct v4l2_pix_format_mplane *pix_mp = _pix_mp->fmt.pix_mp;
> +
> + fmt_pix->type = fmt_pix_mp->type;
> + pix->width = pix_mp->width;
> + 

Re: [PATCH 7/9] [media] s5p-jpeg: Change sclk_jpeg to 166MHz for Exynos5250

2017-06-02 Thread Jacek Anaszewski
Cc Marek and Sylwester.

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: henryhsu 
> 
> The default clock parent of jpeg on Exynos5250 is fin_pll, which is
> 24MHz. We have to change the clock parent to CPLL, which is 333MHz,
> and set sclk_jpeg to 166MHz.
> 
> Signed-off-by: Heng-Ruey Hsu 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 47 
> +
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 7a7acbc..430e925 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -969,6 +969,44 @@ static void exynos4_jpeg_parse_q_tbl(struct s5p_jpeg_ctx 
> *ctx)
>   }
>  }
>  
> +static int exynos4_jpeg_set_sclk_rate(struct s5p_jpeg *jpeg, struct clk 
> *sclk)

Why here exynos4 and in the subject Exynos5250?

> +{
> + struct clk *mout_jpeg;
> + struct clk *sclk_cpll;
> + int ret;
> +
> + mout_jpeg = clk_get(jpeg->dev, "mout_jpeg");
> + if (IS_ERR(mout_jpeg)) {
> + dev_err(jpeg->dev, "mout_jpeg clock not available: %ld\n",
> + PTR_ERR(mout_jpeg));
> + return PTR_ERR(mout_jpeg);
> + }
> +
> + sclk_cpll = clk_get(jpeg->dev, "sclk_cpll");
> + if (IS_ERR(sclk_cpll)) {
> + dev_err(jpeg->dev, "sclk_cpll clock not available: %ld\n",
> + PTR_ERR(sclk_cpll));
> + clk_put(mout_jpeg);
> + return PTR_ERR(sclk_cpll);
> + }
> +
> + ret = clk_set_parent(mout_jpeg, sclk_cpll);
> + clk_put(sclk_cpll);
> + clk_put(mout_jpeg);
> + if (ret) {
> + dev_err(jpeg->dev, "clk_set_parent failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_set_rate(sclk, 166500 * 1000);
> + if (ret) {
> + dev_err(jpeg->dev, "clk_set_rate failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
>  #if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
>  static int jpeg_iommu_init(struct platform_device *pdev)
>  {
> @@ -2974,6 +3012,15 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
>   jpeg->variant->clk_names[i]);
>   return PTR_ERR(jpeg->clocks[i]);
>   }
> +
> + if (jpeg->variant->version == SJPEG_EXYNOS4 &&
> + !strncmp(jpeg->variant->clk_names[i],
> +  "sclk", strlen("sclk"))) {
> + ret = exynos4_jpeg_set_sclk_rate(jpeg,
> +  jpeg->clocks[i]);
> + if (ret)
> + return ret;
> + }
>   }
>  
>   /* v4l2 device */
> 

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 6/9] [media] s5p-jpeg: Add support for resolution change event

2017-06-02 Thread Jacek Anaszewski
Hi Thierry,

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: henryhsu 
> 
> This patch adds support for resolution change event to notify clients so
> they can prepare correct output buffer. When resolution change happened,
> G_FMT for CAPTURE should return old resolution and format before CAPTURE
> queues streamoff.

Do you have a use case for that?

> 
> Signed-off-by: Henry-Ruey Hsu 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 121 
> 
>  drivers/media/platform/s5p-jpeg/jpeg-core.h |   7 ++
>  2 files changed, 95 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 5569b99..7a7acbc 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1416,8 +1417,17 @@ static int s5p_jpeg_g_fmt(struct file *file, void 
> *priv, struct v4l2_format *f)
>   q_data = get_q_data(ct, f->type);
>   BUG_ON(q_data == NULL);
>  
> - pix->width = q_data->w;
> - pix->height = q_data->h;
> + if ((f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> +  ct->mode == S5P_JPEG_ENCODE) ||
> + (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
> +  ct->mode == S5P_JPEG_DECODE)) {
> + pix->width = 0;
> + pix->height = 0;
> + } else {
> + pix->width = q_data->w;
> + pix->height = q_data->h;
> + }
> +

Is this change related to the patch subject?

>   pix->field = V4L2_FIELD_NONE;
>   pix->pixelformat = q_data->fmt->fourcc;
>   pix->bytesperline = 0;
> @@ -1677,8 +1687,6 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, 
> struct v4l2_format *f)
>   FMT_TYPE_OUTPUT : FMT_TYPE_CAPTURE;
>  
>   q_data->fmt = s5p_jpeg_find_format(ct, pix->pixelformat, f_type);
> - q_data->w = pix->width;
> - q_data->h = pix->height;
>   if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG) {
>   /*
>* During encoding Exynos4x12 SoCs access wider memory area
> @@ -1686,6 +1694,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, 
> struct v4l2_format *f)
>* the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
>* page fault calculate proper buffer size in such a case.
>*/
> + q_data->w = pix->width;
> + q_data->h = pix->height;
>   if (ct->jpeg->variant->hw_ex4_compat &&
>   f_type == FMT_TYPE_OUTPUT && ct->mode == S5P_JPEG_ENCODE)
>   q_data->size = exynos4_jpeg_get_output_buffer_size(ct,
> @@ -1761,6 +1771,15 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, 
> void *priv,
>   return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
>  }
>  
> +static int s5p_jpeg_subscribe_event(struct v4l2_fh *fh,
> + const struct v4l2_event_subscription *sub)
> +{
> + if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
> + return v4l2_src_change_event_subscribe(fh, sub);
> +
> + return -EINVAL;
> +}
> +
>  static int exynos3250_jpeg_try_downscale(struct s5p_jpeg_ctx *ctx,
>  struct v4l2_rect *r)
>  {
> @@ -2086,6 +2105,9 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = 
> {
>  
>   .vidioc_g_selection = s5p_jpeg_g_selection,
>   .vidioc_s_selection = s5p_jpeg_s_selection,
> +
> + .vidioc_subscribe_event = s5p_jpeg_subscribe_event,
> + .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
>  };
>  
>  /*
> @@ -2478,8 +2500,17 @@ static int s5p_jpeg_job_ready(void *priv)
>  {
>   struct s5p_jpeg_ctx *ctx = priv;
>  
> - if (ctx->mode == S5P_JPEG_DECODE)
> + if (ctx->mode == S5P_JPEG_DECODE) {
> + /*
> +  * We have only one input buffer and one output buffer. If there
> +  * is a resolution change event, no need to continue decoding.
> +  */
> + if (ctx->state == JPEGCTX_RESOLUTION_CHANGE)
> + return 0;
> +
>   return ctx->hdr_parsed;
> + }
> +
>   return 1;
>  }
>  
> @@ -2558,6 +2589,21 @@ static int s5p_jpeg_buf_prepare(struct vb2_buffer *vb)
>   return 0;
>  }
>  
> +static void s5p_jpeg_set_capture_queue_data(struct s5p_jpeg_ctx *ctx)
> +{
> + struct s5p_jpeg_q_data *q_data = >cap_q;
> +
> + q_data->w = ctx->out_q.w;
> + q_data->h = ctx->out_q.h;
> +
> + jpeg_bound_align_image(ctx, _data->w, S5P_JPEG_MIN_WIDTH,
> +S5P_JPEG_MAX_WIDTH, q_data->fmt->h_align,
> +_data->h, S5P_JPEG_MIN_HEIGHT,
> +S5P_JPEG_MAX_HEIGHT, q_data->fmt->v_align);
> 

Re: [PATCH 5/9] [media] s5p-jpeg: Add IOMMU support

2017-06-02 Thread Jacek Anaszewski
Cc Marek Szyprowski.

Marek, could you share your opinion about this patch?

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: Tony K Nadackal 
> 
> This patch adds support for IOMMU s5p-jpeg driver if the Exynos IOMMU
> and ARM DMA IOMMU configurations are supported. The address space is
> created with size limited to 256M and base address set to 0x2000.
> 
> Signed-off-by: Tony K Nadackal 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 77 
> +
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 770a709..5569b99 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -28,6 +28,14 @@
>  #include 
>  #include 
>  #include 
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
>  
>  #include "jpeg-core.h"
>  #include "jpeg-hw-s5p.h"
> @@ -35,6 +43,10 @@
>  #include "jpeg-hw-exynos3250.h"
>  #include "jpeg-regs.h"
>  
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +static struct dma_iommu_mapping *mapping;
> +#endif
> +
>  static struct s5p_jpeg_fmt sjpeg_formats[] = {
>   {
>   .name   = "JPEG JFIF",
> @@ -956,6 +968,60 @@ static void exynos4_jpeg_parse_q_tbl(struct s5p_jpeg_ctx 
> *ctx)
>   }
>  }
>  
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> +static int jpeg_iommu_init(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + int err;
> +
> + mapping = arm_iommu_create_mapping(_bus_type, 0x2000,
> +SZ_512M);
> + if (IS_ERR(mapping)) {
> + dev_err(dev, "IOMMU mapping failed\n");
> + return PTR_ERR(mapping);
> + }
> +
> + dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL);
> + if (!dev->dma_parms) {
> + err = -ENOMEM;
> + goto error_alloc;
> + }
> +
> + err = dma_set_max_seg_size(dev, 0xu);
> + if (err)
> + goto error;
> +
> + err = arm_iommu_attach_device(dev, mapping);
> + if (err)
> + goto error;
> +
> + return 0;
> +
> +error:
> + devm_kfree(dev, dev->dma_parms);
> + dev->dma_parms = NULL;
> +
> +error_alloc:
> + arm_iommu_release_mapping(mapping);
> + mapping = NULL;
> +
> + return err;
> +}
> +
> +static void jpeg_iommu_deinit(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> +
> + if (mapping) {
> + arm_iommu_detach_device(dev);
> + devm_kfree(dev, dev->dma_parms);
> + dev->dma_parms = NULL;
> + arm_iommu_release_mapping(mapping);
> + mapping = NULL;
> + }
> +}
> +#endif
> +
>  /*
>   * 
> 
>   * Device file operations
> @@ -2816,6 +2882,13 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
>   spin_lock_init(>slock);
>   jpeg->dev = >dev;
>  
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> + ret = jpeg_iommu_init(pdev);
> + if (ret) {
> + dev_err(>dev, "IOMMU Initialization failed\n");
> + return ret;
> + }
> +#endif
>   /* memory-mapped registers */
>   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  
> @@ -2962,6 +3035,10 @@ static int s5p_jpeg_remove(struct platform_device 
> *pdev)
>   clk_disable_unprepare(jpeg->clocks[i]);
>   }
>  
> +#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
> + jpeg_iommu_deinit(pdev);
> +#endif
> +
>   return 0;
>  }
>  
> 

-- 
Best regards,
Jacek Anaszewski


Re: [PATCH 4/9] [media] s5p-jpeg: Decode 4:1:1 chroma subsampling format

2017-06-02 Thread Jacek Anaszewski
Hi Thierry,

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: Tony K Nadackal 
> 
> This patch adds support for decoding 4:1:1 chroma subsampling in the
> jpeg header parsing function.
> 
> Signed-off-by: Tony K Nadackal 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 0d83948..770a709 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -1236,6 +1236,9 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data 
> *result,
>   case 0x33:
>   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
>   break;
> + case 0x41:
> + ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_411;
> + break;
>   default:
>   return false;
>   }
> 

Acked-by: Jacek Anaszewski 

-- 
Best regards,
Jacek Anaszewski


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

2017-06-02 Thread Kevin Hilman
This series fixes/updates the support for raw camera input to the VPIF.

Tested on da850-evm boards using the add-on UI board.  Tested with
both composite video input (on-board tvp514x) and raw camera input
using the camera board from On-Semi based on the aptina,mt9v032
sensor[1], as this was the only camera board with the right connector
for the da850-evm UI board.

Verified that composite video capture is still working well after these
updates.

[1] 
http://www.mouser.com/search/ProductDetail.aspx?R=0virtualkey0virtualkeyMT9V032C12STCH-GEVB

Kevin Hilman (4):
  [media] davinci: vpif_capture: drop compliance hack
  [media] davinci: vpif_capture: get subdevs from DT when available
  [media] davinci: vpif_capture: cleanup raw camera support
  [media] davinci: vpif: adaptions for DT support

 drivers/media/platform/davinci/vpif.c |  49 +-
 drivers/media/platform/davinci/vpif_capture.c | 224 +++---
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h|   9 +-
 4 files changed, 263 insertions(+), 24 deletions(-)

-- 
2.9.3



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

2017-06-02 Thread Kevin Hilman
The davinci VPIF is a single hardware block, but the existing driver
is broken up into a common library (vpif.c), output (vpif_display.c) and
intput (vpif_capture.c).

When migrating to DT, to better model the hardware, and because
registers, interrupts, etc. are all common,it was decided to
have a single VPIF hardware node[1].

Because davinci uses legacy, non-DT boot on several SoCs still, the
platform_drivers need to remain.  But they are also needed in DT boot.
Since there are no DT nodes for the display/capture parts in DT
boot (there is a single node for the parent/common device) we need to
create platform_devices somewhere to instansiate the platform_drivers.

When VPIF display/capture are needed for a DT boot, the VPIF node
will have endpoints defined for its subdevs.  Therefore, vpif_probe()
checks for the presence of endpoints, and if detected manually creates
the platform_devices for the display and capture platform_drivers.

[1] Documentation/devicetree/bindings/media/ti,da850-vpif.txt

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

diff --git a/drivers/media/platform/davinci/vpif.c 
b/drivers/media/platform/davinci/vpif.c
index 1b02a6363f77..502917abcb13 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vpif.h"
 
@@ -423,7 +424,9 @@ EXPORT_SYMBOL(vpif_channel_getfid);
 
 static int vpif_probe(struct platform_device *pdev)
 {
-   static struct resource  *res;
+   static struct resource  *res, *res_irq;
+   struct platform_device *pdev_capture, *pdev_display;
+   struct device_node *endpoint = NULL;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
vpif_base = devm_ioremap_resource(>dev, res);
@@ -435,6 +438,50 @@ static int vpif_probe(struct platform_device *pdev)
 
spin_lock_init(_lock);
dev_info(>dev, "vpif probe success\n");
+
+   /*
+* If VPIF Node has endpoints, assume "new" DT support,
+* where capture and display drivers don't have DT nodes
+* so their devices need to be registered manually here
+* for their legacy platform_drivers to work.
+*/
+   endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+ endpoint);
+   if (!endpoint) 
+   return 0;
+
+   /*
+* For DT platforms, manually create platform_devices for
+* capture/display drivers.
+*/
+   res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+   if (!res_irq) {
+   dev_warn(>dev, "Missing IRQ resource.\n");
+   return -EINVAL;
+   }
+
+   pdev_capture = devm_kzalloc(>dev, sizeof(*pdev_capture),
+   GFP_KERNEL);
+   pdev_capture->name = "vpif_capture";
+   pdev_capture->id = -1;
+   pdev_capture->resource = res_irq;
+   pdev_capture->num_resources = 1;
+   pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
+   pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+   pdev_capture->dev.parent = >dev;
+   platform_device_register(pdev_capture);
+
+   pdev_display = devm_kzalloc(>dev, sizeof(*pdev_display),
+   GFP_KERNEL);
+   pdev_display->name = "vpif_display";
+   pdev_display->id = -1;
+   pdev_display->resource = res_irq;
+   pdev_display->num_resources = 1;
+   pdev_display->dev.dma_mask = pdev->dev.dma_mask;
+   pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+   pdev_display->dev.parent = >dev;
+   platform_device_register(pdev_display);
+
return 0;
 }
 
-- 
2.9.3



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

2017-06-02 Thread Kevin Hilman
Capture driver silently overrides pixel format with a hack (according to
the comments) to pass v4l2 compliance tests.  This isn't needed for
normal functionality, and works for composite video and raw camera capture
without.

In addition, the hack assumes that it only supports raw capture with a
single format (SBGGR8) which isn't true.  VPIF can also capture 10- and
12-bit raw formats as well.  Forthcoming patches will enable VPIF
input with raw-camera support and has been tested with 10-bit format
from the aptina,mt9v032 sensor.

Any compliance failures should be fixed with a real fix.

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

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index 128e92d1dd5a..fc5c7622660c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -936,21 +936,6 @@ static int vpif_try_fmt_vid_cap(struct file *file, void 
*priv,
struct channel_obj *ch = video_get_drvdata(vdev);
struct v4l2_pix_format *pixfmt = >fmt.pix;
struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
-   struct vpif_params *vpif_params = >vpifparams;
-
-   /*
-* to supress v4l-compliance warnings silently correct
-* the pixelformat
-*/
-   if (vpif_params->iface.if_type == VPIF_IF_RAW_BAYER) {
-   if (pixfmt->pixelformat != V4L2_PIX_FMT_SBGGR8)
-   pixfmt->pixelformat = V4L2_PIX_FMT_SBGGR8;
-   } else {
-   if (pixfmt->pixelformat != V4L2_PIX_FMT_NV16)
-   pixfmt->pixelformat = V4L2_PIX_FMT_NV16;
-   }
-
-   common->fmt.fmt.pix.pixelformat = pixfmt->pixelformat;
 
vpif_update_std_info(ch);
 
-- 
2.9.3



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

2017-06-02 Thread Kevin Hilman
The current driver has a handful of hard-coded assumptions based on its
primary use for capture of video signals.  Cleanup those assumptions,
and also query the subdev for format information and use that if
available.

Tested with 10-bit raw bayer input (SGRBG10) using the aptina,mt9v032
sensor, and also tested that composite video input still works from
ti,tvp514x decoder.  Both tests done on the da850-evm board with the
add-on UI board.

NOTE: Will need further testing for other sensors with different bus
formats.

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

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index b9d927d1e5a8..67624dbf1272 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -24,6 +24,9 @@
 #include 
 #include 
 #include 
+#include 
+
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -387,7 +390,8 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
common = >common[i];
/* skip If streaming is not started in this channel */
/* Check the field format */
-   if (1 == ch->vpifparams.std_info.frm_fmt) {
+   if (1 == ch->vpifparams.std_info.frm_fmt ||
+   common->fmt.fmt.pix.field == V4L2_FIELD_NONE) {
/* Progressive mode */
spin_lock(>irqlock);
if (list_empty(>dma_queue)) {
@@ -468,9 +472,38 @@ static int vpif_update_std_info(struct channel_obj *ch)
struct vpif_channel_config_params *std_info = >std_info;
struct video_obj *vid_ch = >video;
int index;
+   struct v4l2_pix_format *pixfmt = >fmt.fmt.pix;
 
vpif_dbg(2, debug, "vpif_update_std_info\n");
 
+   /*
+* if called after try_fmt or g_fmt, there will already be a size
+* so use that by default.
+*/
+   if (pixfmt->width && pixfmt->height) {
+   if (pixfmt->field == V4L2_FIELD_ANY ||
+   pixfmt->field == V4L2_FIELD_NONE)
+   pixfmt->field = V4L2_FIELD_NONE;
+   
+   vpifparams->iface.if_type = VPIF_IF_BT656;
+   if (pixfmt->pixelformat == V4L2_PIX_FMT_SGRBG10 ||
+   pixfmt->pixelformat == V4L2_PIX_FMT_SBGGR8)
+   vpifparams->iface.if_type = VPIF_IF_RAW_BAYER;
+
+   if (pixfmt->pixelformat == V4L2_PIX_FMT_SGRBG10)
+   vpifparams->params.data_sz = 1; /* 10 bits/pixel.  */
+
+   /* 
+* For raw formats from camera sensors, we don't need 
+* the std_info from table lookup, so nothing else to do here.
+*/
+   if (vpifparams->iface.if_type == VPIF_IF_RAW_BAYER) {
+   memset(std_info, 0, sizeof(struct 
vpif_channel_config_params));
+   vpifparams->std_info.capture_format = 1; /* CCD/raw 
mode */
+   return 0;
+   }
+   }
+
for (index = 0; index < vpif_ch_params_count; index++) {
config = _ch_params[index];
if (config->hd_sd == 0) {
@@ -939,6 +972,7 @@ static int vpif_try_fmt_vid_cap(struct file *file, void 
*priv,
struct v4l2_pix_format *pixfmt = >fmt.pix;
struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
 
+   common->fmt = *fmt;
vpif_update_std_info(ch);
 
pixfmt->field = common->fmt.fmt.pix.field;
@@ -947,8 +981,17 @@ static int vpif_try_fmt_vid_cap(struct file *file, void 
*priv,
pixfmt->width = common->fmt.fmt.pix.width;
pixfmt->height = common->fmt.fmt.pix.height;
pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height * 2;
+   if (pixfmt->pixelformat == V4L2_PIX_FMT_SGRBG10) {
+   pixfmt->bytesperline = common->fmt.fmt.pix.width * 2;
+   pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
+   }
pixfmt->priv = 0;
 
+   dev_dbg(vpif_dev, "%s: %d x %d; pitch=%d pixelformat=0x%08x, field=%d, 
size=%d\n", __func__,
+   pixfmt->width, pixfmt->height,
+   pixfmt->bytesperline, pixfmt->pixelformat,
+   pixfmt->field, pixfmt->sizeimage);
+
return 0;
 }
 
@@ -965,13 +1008,47 @@ static int vpif_g_fmt_vid_cap(struct file *file, void 
*priv,
struct video_device *vdev = video_devdata(file);
struct channel_obj *ch = video_get_drvdata(vdev);
struct common_obj *common = >common[VPIF_VIDEO_INDEX];
+   struct v4l2_pix_format *pix_fmt = >fmt.pix;
+   struct v4l2_subdev_format format = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   };
+   struct v4l2_mbus_framefmt *mbus_fmt = 
+   int ret;
 
/* Check 

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

2017-06-02 Thread Kevin Hilman
Enable  getting of subdevs from DT ports and endpoints.

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

Signed-off-by: Kevin Hilman 
---
 drivers/media/platform/davinci/vpif_capture.c | 126 +-
 drivers/media/platform/davinci/vpif_display.c |   5 +
 include/media/davinci/vpif_types.h|   9 +-
 3 files changed, 134 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index fc5c7622660c..b9d927d1e5a8 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -22,6 +22,8 @@
 #include 
 
 #include 
+#include 
+#include 
 
 #include "vpif.h"
 #include "vpif_capture.h"
@@ -655,7 +657,7 @@ static int vpif_input_to_subdev(
/* loop through the sub device list to get the sub device info */
for (i = 0; i < vpif_cfg->subdev_count; i++) {
subdev_info = _cfg->subdev_info[i];
-   if (!strcmp(subdev_info->name, subdev_name))
+   if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1308,6 +1310,21 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
 {
int i;
 
+   for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
+   struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
+   const struct device_node *node = _asd->match.of.node;
+
+   if (node == subdev->of_node) {
+   vpif_obj.sd[i] = subdev;
+   vpif_obj.config->chan_config->inputs[i].subdev_name =
+   (char *)subdev->of_node->full_name;
+   vpif_dbg(2, debug,
+"%s: setting input %d subdev_name = %s\n",
+__func__, i, subdev->of_node->full_name);
+   return 0;
+   }
+   }
+
for (i = 0; i < vpif_obj.config->subdev_count; i++)
if (!strcmp(vpif_obj.config->subdev_info[i].name,
subdev->name)) {
@@ -1403,6 +1420,105 @@ static int vpif_async_complete(struct 
v4l2_async_notifier *notifier)
return vpif_probe_complete();
 }
 
+static struct vpif_capture_config *
+vpif_capture_get_pdata(struct platform_device *pdev)
+{
+   struct device_node *endpoint = NULL;
+   struct v4l2_of_endpoint bus_cfg;
+   struct vpif_capture_config *pdata;
+   struct vpif_subdev_info *sdinfo;
+   struct vpif_capture_chan_config *chan;
+   unsigned int i;
+
+   /*
+* DT boot: OF node from parent device contains
+* video ports & endpoints data.
+*/
+   if (pdev->dev.parent && pdev->dev.parent->of_node)
+   pdev->dev.of_node = pdev->dev.parent->of_node;
+   if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+   return pdev->dev.platform_data;
+
+   pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+   pdata->subdev_info =
+   devm_kzalloc(>dev, sizeof(*pdata->subdev_info) *
+VPIF_CAPTURE_NUM_CHANNELS, GFP_KERNEL);
+
+   if (!pdata->subdev_info)
+   return NULL;
+
+   for (i = 0; i < VPIF_CAPTURE_NUM_CHANNELS; i++) {
+   struct device_node *rem;
+   unsigned int flags;
+   int err;
+
+   endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+ endpoint);
+   if (!endpoint)
+   break;
+
+   sdinfo = >subdev_info[i];
+   chan = >chan_config[i];
+   chan->inputs = devm_kzalloc(>dev,
+   sizeof(*chan->inputs) *
+   VPIF_CAPTURE_NUM_CHANNELS,
+   GFP_KERNEL);
+
+   chan->input_count++;
+   chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
+   chan->inputs[i].input.std = V4L2_STD_ALL;
+   chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
+
+   err = v4l2_of_parse_endpoint(endpoint, _cfg);
+   if (err) {
+   dev_err(>dev, "Could not parse the endpoint\n");
+   goto done;
+   }
+   dev_dbg(>dev, "Endpoint %s, bus_width = %d\n",
+   endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+   flags = bus_cfg.bus.parallel.flags;
+
+   if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+   chan->vpif_if.hd_pol = 1;
+
+   if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+   chan->vpif_if.vd_pol = 1;
+
+   rem = 

Re: [PATCH 2/9] [media] s5p-jpeg: Call jpeg_bound_align_image after qbuf

2017-06-02 Thread Jacek Anaszewski
Hi Thierry,

Thanks for the patch.

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: Tony K Nadackal 
> 
> When queuing an OUTPUT buffer for decoder, s5p_jpeg_parse_hdr()
> function parses the input jpeg file and takes the width and height
> parameters from its header. These new width/height values will be used
> for the calculation of stride. HX_JPEG Hardware needs the width and
> height values aligned on a 16 bits boundary. This width/height alignment
> is handled in the s5p_jpeg_s_fmt_vid_cap() function during the S_FMT
> ioctl call.
> 
> But if user space calls the QBUF of OUTPUT buffer after the S_FMT of
> CAPTURE buffer, these aligned values will be replaced by the values in
> jpeg header.

I assume that you may want to avoid re-setting the capture buf format
when decoding a stream of JPEGs and you are certain that all of them
have the same subsampling. Nonetheless, please keep in mind that in case
of Exynos4x12 SoCs there is a risk of permanent decoder hangup if you'd
try to decode to a YUV with lower subsampling than the one of input
JPEG. s5p_jpeg_try_fmt_vid_cap() does a suitable adjustment to avoid the
problem.

I'd add a comment over this call to jpeg_bound_align_image() that
resigning from executing S_FMT on capture buf for each JPEG image
can result in a hardware hangup if forbidden decoding will be enforced.

> If the width/height values of jpeg are not aligned, the
> decoder output will be corrupted. So in this patch we call
> jpeg_bound_align_image() to align the width/height values of Capture
> buffer in s5p_jpeg_buf_queue().
> 
> Signed-off-by: Tony K Nadackal 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 52dc794..6fb1ab4 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -2523,6 +2523,13 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
>   q_data = >cap_q;
>   q_data->w = tmp.w;
>   q_data->h = tmp.h;
> +
> + jpeg_bound_align_image(ctx, _data->w, S5P_JPEG_MIN_WIDTH,
> +S5P_JPEG_MAX_WIDTH, q_data->fmt->h_align,
> +_data->h, S5P_JPEG_MIN_HEIGHT,
> +S5P_JPEG_MAX_HEIGHT, q_data->fmt->v_align
> +   );
> + q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
>   }
>  
>   v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
> 

-- 
Best regards,
Jacek Anaszewski


Re: Firmware for staging atomisp driver

2017-06-02 Thread Hans de Goede

Hi,

On 05/28/2017 02:30 PM, Hans de Goede wrote:

Hi All,

I've been trying to get the atomisp driver from staging to work
on a couple of devices I have.

I started with an Asus T100TA after fixing 2 oopses in the sensor
driver there I found out that the BIOS does not allow to put the
ISP in PCI mode and that there is no code to drive it in ACPI
enumerated mode.

So I moved to a generic Insyde T701 tablet which does allow
this. After fixing some more sensor driver issues there I was
ready to actually load the atomisp driver, but I could not
find the exact firmware required, I did find a version which
is close: "irci_stable_candrpv_0415_20150423_1753"
and tried that but that causes the atomisp driver to explode
in a backtrace which contains atomisp_load_firmware() so that
one seems no good.


Ok, so it turns out that the explosion was not a probem with
a wrong firmware version, but rather another atomisp code
bug. According to this patch:

https://github.com/01org/ProductionKernelQuilts/blob/master/uefi/cht-m1stable/patches/cam-0418-atomisp2-css2401-and-2401_legacy-irci_stable_candrpv.patch

The irci_stable_candrpv_0415_20150423_1753 version I
have and the irci_stable_candrpv_0415_20150521_0458
version expected are fully compatible.

So I'e focussed on fixing the crash and that was easy, see
the patch I just send.

Then I hit another bunch of crashes which all turn out to
be due to races with udev opening /dev/video# nodes for probing
before the driver is ready to handle this because the driver
registers the v4l2 devices too soon. I've hacked around this
for now:

https://github.com/jwrdegoede/linux-sunxi/commit/88c9c248e6e0f86d547ea8441e16b0e8b4c951c8

And with this hack the driver loads without issues, giving
me 10 /dev/video# devices. So I tried this app:

https://github.com/jfwells/linux-asus-t100ta/tree/master/webcam/atomisp_testapp

On a number of the v4l devices which leads to yet more oopses.

So I'm starting to wonder, does anyone have this driver working
(in its current form in 4.12-rc3 drivers/staging) at all ?

I'm asking  because that is hard to believe given e.g. the recursion bug
I've just fixed.

Can someone provide step-by-step instructions for how to get this
driver working?

I'm not expecting all userspace apps to just work, but at least a userspace 
example
given a picture from the camera would be very helpful in further developing
this driver.

Regards,

Hans


[PATCH] staging: atomisp: Fix endless recursion in hmm_init

2017-06-02 Thread Hans de Goede
hmm_init calls hmm_alloc to set dummy_ptr, hmm_alloc calls
hmm_init when dummy_ptr is not yet set, which is the case in
the call from hmm_init, so it calls hmm_init again, this continues
until we have a stack overflow due to the recursion.

This commit fixes this by adding a separate flag for tracking if
hmm_init has been called. Not pretty, but it gets the job done,
eventually we should be able to remove the hmm_init call from
hmm_alloc.

Signed-off-by: Hans de Goede 
---
 drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c 
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
index 5729539..e79ca3c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
@@ -43,6 +43,7 @@ struct hmm_bo_device bo_device;
 struct hmm_pooldynamic_pool;
 struct hmm_poolreserved_pool;
 static ia_css_ptr dummy_ptr;
+static bool hmm_initialized;
 struct _hmm_mem_stat hmm_mem_stat;
 
 /* p: private
@@ -186,6 +187,8 @@ int hmm_init(void)
if (ret)
dev_err(atomisp_dev, "hmm_bo_device_init failed.\n");
 
+   hmm_initialized = true;
+
/*
 * As hmm use NULL to indicate invalid ISP virtual address,
 * and ISP_VM_START is defined to 0 too, so we allocate
@@ -217,6 +220,7 @@ void hmm_cleanup(void)
dummy_ptr = 0;
 
hmm_bo_device_exit(_device);
+   hmm_initialized = false;
 }
 
 ia_css_ptr hmm_alloc(size_t bytes, enum hmm_bo_type type,
@@ -229,7 +233,7 @@ ia_css_ptr hmm_alloc(size_t bytes, enum hmm_bo_type type,
/* Check if we are initialized. In the ideal world we wouldn't need
   this but we can tackle it once the driver is a lot cleaner */
 
-   if (!dummy_ptr)
+   if (!hmm_initialized)
hmm_init();
/*Get page number from size*/
pgnr = size_to_pgnr_ceil(bytes);
-- 
2.9.4



Re: [PATCH 1/9] [media] s5p-jpeg: Reset the Codec before doing a soft reset

2017-06-02 Thread Jacek Anaszewski
Hi Thierry,

On 06/02/2017 06:02 PM, Thierry Escande wrote:
> From: Abhilash Kesavan 
> 
> This patch resets the encoding and decoding register bits before doing a
> soft reset.
> 
> Signed-off-by: Tony K Nadackal 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
> index a1d823a..9ad8f6d 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
> @@ -21,6 +21,10 @@ void exynos4_jpeg_sw_reset(void __iomem *base)
>   unsigned int reg;
>  
>   reg = readl(base + EXYNOS4_JPEG_CNTL_REG);
> + writel(reg & ~(EXYNOS4_DEC_MODE | EXYNOS4_ENC_MODE),
> +base + EXYNOS4_JPEG_CNTL_REG);

Why is it required? It would be nice if commit message explained that.

> + reg = readl(base + EXYNOS4_JPEG_CNTL_REG);
>   writel(reg & ~EXYNOS4_SOFT_RESET_HI, base + EXYNOS4_JPEG_CNTL_REG);
>  
>   udelay(100);
> 

-- 
Best regards,
Jacek Anaszewski


RE: [PATCH v7 1/1] [media] i2c: add support for OV13858 sensor

2017-06-02 Thread Yang, Hyungwoo

Hi Sakari,

I have fixed runtime PM calls in .probe() and .remove(). I'll submit v8

Thanks,
Hyungwoo

-Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@iki.fi] 
> Sent: Friday, June 2, 2017 12:54 AM
> To: Yang, Hyungwoo 
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; Zheng, Jian Xu 
> ; tf...@chromium.org; Hsu, Cedric 
> 
> Subject: Re: [PATCH v7 1/1] [media] i2c: add support for OV13858 sensor
> 
> Hi Hyungwoo,
> 
> On Thu, Jun 01, 2017 at 03:45:16PM -0700, Hyungwoo Yang wrote:
> ...
> > +static int ov13858_probe(struct i2c_client *client,
> > +const struct i2c_device_id *devid) {
> > +   struct ov13858 *ov13858;
> > +   int ret;
> > +
> > +   ov13858 = devm_kzalloc(>dev, sizeof(*ov13858), GFP_KERNEL);
> > +   if (!ov13858)
> > +   return -ENOMEM;
> > +
> > +   /* Initialize subdev */
> > +   v4l2_i2c_subdev_init(>sd, client, _subdev_ops);
> > +
> > +   /*
> > +* Enable runtime PM.
> > +* The sensor is already powered on ACPI domain PM
> > +*/
> > +   pm_runtime_get_noresume(>dev);
> > +   pm_runtime_set_active(>dev);
> > +   pm_runtime_enable(>dev);
> 
> As you already have in comments, the device is already powered on in an ACPI 
> based system. pm_runtime_get_noresume() prevents powering the device off 
> during probe after runtime PM is enabled.
> 
> It'd be better to also move the calls to pm_runtime_set_active() and
> pm_runtime_enable() just before returning 0. This way you can get rid of 
> extra error handling you'd otherwise need to do: once you call 
> pm_runtime_get_noresume(), you need to call pm_runtime_put() or one of its 
> variants as well.
> 

Ack. I agree I have lazy impelementation in .probe() and .remove(). Like you 
said, it's better to enable runtime PM just before returning 0.
Yes, I'm calling pm_runtme_put as you can see to turn off the device.

> > +
> > +   /* Check module identity */
> > +   ret = ov13858_identify_module(ov13858);
> > +   if (ret) {
> > +   dev_err(>dev, "failed to find sensor: %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   /* Set default mode to max resolution */
> > +   ov13858->cur_mode = _modes[0];
> > +
> > +   ret = ov13858_init_controls(ov13858);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /* Initialize subdev */
> > +   ov13858->sd.internal_ops = _internal_ops;
> > +   ov13858->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +   ov13858->sd.entity.ops = _subdev_entity_ops;
> > +   ov13858->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > +   /* Initialize source pad */
> > +   ov13858->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +   ret = media_entity_pads_init(>sd.entity, 1, >pad);
> > +   if (ret) {
> > +   dev_err(>dev, "%s failed:%d\n", __func__, ret);
> > +   goto error_handler_free;
> > +   }
> > +
> > +   ret = v4l2_async_register_subdev(>sd);
> > +   if (ret < 0)
> > +   goto error_media_entity;
> > +
> > +   /* Turn off */
> > +   pm_runtime_put(>dev);
> > +
> > +   return 0;
> > +
> > +error_media_entity:
> > +   media_entity_cleanup(>sd.entity);
> > +
> > +error_handler_free:
> > +   ov13858_free_controls(ov13858);
> > +   dev_err(>dev, "%s failed:%d\n", __func__, ret);
> > +
> > +   return ret;
> > +}
> 
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi   XMPP: sai...@retiisi.org.uk
>


[PATCH v8 1/1] [media] i2c: add support for OV13858 sensor

2017-06-02 Thread Hyungwoo Yang
This patch adds driver for Omnivision's ov13858
sensor, the driver supports following features:

- manual exposure/analog gain
- two link frequencies
- VBLANK support
- test pattern support
- media controller support
- runtime pm support
- supported resolutions
  + 4224x3136 at 30FPS
  + 2112x1568 at 30FPS(default) and 60FPS
  + 2112x1188 at 30FPS(default) and 60FPS
  + 1056x784 at 30FPS(default) and 60FPS

Signed-off-by: Hyungwoo Yang 
---
 drivers/media/i2c/Kconfig   |8 +
 drivers/media/i2c/Makefile  |1 +
 drivers/media/i2c/ov13858.c | 1752 +++
 3 files changed, 1761 insertions(+)
 create mode 100644 drivers/media/i2c/ov13858.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fd181c9..f8c5cca 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -589,6 +589,14 @@ config VIDEO_OV9650
  This is a V4L2 sensor-level driver for the Omnivision
  OV9650 and OV9652 camera sensors.
 
+config VIDEO_OV13858
+   tristate "OmniVision OV13858 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV13858 camera.
+
 config VIDEO_VS6624
tristate "ST VS6624 sensor support"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 62323ec..3f4dc02 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
+obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
 obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
 obj-$(CONFIG_VIDEO_MT9M111) += mt9m111.o
 obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o
diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
new file mode 100644
index 000..17127b5
--- /dev/null
+++ b/drivers/media/i2c/ov13858.c
@@ -0,0 +1,1752 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define OV13858_REG_VALUE_08BIT1
+#define OV13858_REG_VALUE_16BIT2
+#define OV13858_REG_VALUE_24BIT3
+
+#define OV13858_REG_MODE_SELECT0x0100
+#define OV13858_MODE_STANDBY   0x00
+#define OV13858_MODE_STREAMING 0x01
+
+#define OV13858_REG_SOFTWARE_RST   0x0103
+#define OV13858_SOFTWARE_RST   0x01
+
+/* PLL1 generates PCLK and MIPI_PHY_CLK */
+#define OV13858_REG_PLL1_CTRL_00x0300
+#define OV13858_REG_PLL1_CTRL_10x0301
+#define OV13858_REG_PLL1_CTRL_20x0302
+#define OV13858_REG_PLL1_CTRL_30x0303
+#define OV13858_REG_PLL1_CTRL_40x0304
+#define OV13858_REG_PLL1_CTRL_50x0305
+
+/* PLL2 generates DAC_CLK, SCLK and SRAM_CLK */
+#define OV13858_REG_PLL2_CTRL_B0x030b
+#define OV13858_REG_PLL2_CTRL_C0x030c
+#define OV13858_REG_PLL2_CTRL_D0x030d
+#define OV13858_REG_PLL2_CTRL_E0x030e
+#define OV13858_REG_PLL2_CTRL_F0x030f
+#define OV13858_REG_PLL2_CTRL_12   0x0312
+#define OV13858_REG_MIPI_SC_CTRL0  0x3016
+#define OV13858_REG_MIPI_SC_CTRL1  0x3022
+
+/* Chip ID */
+#define OV13858_REG_CHIP_ID0x300a
+#define OV13858_CHIP_ID0x00d855
+
+/* V_TIMING internal */
+#define OV13858_REG_VTS0x380e
+#define OV13858_VTS_30FPS  0x0c8e /* 30 fps */
+#define OV13858_VTS_60FPS  0x0648 /* 60 fps */
+#define OV13858_VTS_MAX0x7fff
+#define OV13858_VBLANK_MIN 56
+
+/* Exposure control */
+#define OV13858_REG_EXPOSURE   0x3500
+#define OV13858_EXPOSURE_MIN   4
+#define OV13858_EXPOSURE_MAX   (OV13858_VTS_MAX - 8)
+#define OV13858_EXPOSURE_STEP  1
+#define OV13858_EXPOSURE_DEFAULT   0x640
+
+/* Analog gain control */
+#define OV13858_REG_ANALOG_GAIN0x3508
+#define OV13858_ANA_GAIN_MIN   0
+#define OV13858_ANA_GAIN_MAX   0x1fff
+#define OV13858_ANA_GAIN_STEP  1
+#define OV13858_ANA_GAIN_DEFAULT   0x80
+
+/* Test Pattern Control */
+#define OV13858_REG_TEST_PATTERN   0x4503
+#define OV13858_TEST_PATTERN_ENABLEBIT(7)
+#define 

Re: [PATCH v2 14/27] ASoC: blackfin: Convert to the new PCM ops

2017-06-02 Thread Mark Brown
On Thu, Jun 01, 2017 at 10:58:37PM +0200, Takashi Iwai wrote:
> Replace the copy and the silence ops with the new PCM ops.
> In AC97 and I2S-TDM mode, we need to convert back to frames, but
> otherwise the conversion is pretty straightforward.

Acked-by: Mark Brown 


signature.asc
Description: PGP signature


Re: [PATCH v2 02/27] ALSA: pcm: Introduce copy_user, copy_kernel and fill_silence ops

2017-06-02 Thread Mark Brown
On Thu, Jun 01, 2017 at 10:58:25PM +0200, Takashi Iwai wrote:
> For supporting the explicit in-kernel copy of PCM buffer data, and
> also for further code refactoring, three new PCM ops, copy_user,
> copy_kernel and fill_silence, are introduced.  The old copy and
> silence ops will be deprecated and removed later once when all callers
> are converted.

Acked-by: Mark Brown 


signature.asc
Description: PGP signature


Re: regmap for i2c pages

2017-06-02 Thread Mark Brown
On Thu, Jun 01, 2017 at 11:37:43PM -0700, Tim Harvey wrote:

> I believe this is a very common i2c register mechanism but I'm not
> clear what the best way to use i2c regmap for this is. I'm reading
> that regmap 'handles register pages' but I'm not clear if that's the
> same thing I'm looking for. If so, are there any examples of this? I
> see several i2c drivers that reference pages but it looks to me that
> each page is a different i2c slave address which is something
> different.

You're looking for regmap_range (search for window in regmap.h).


signature.asc
Description: PGP signature


[PATCH 4/9] [media] s5p-jpeg: Decode 4:1:1 chroma subsampling format

2017-06-02 Thread Thierry Escande
From: Tony K Nadackal 

This patch adds support for decoding 4:1:1 chroma subsampling in the
jpeg header parsing function.

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 0d83948..770a709 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1236,6 +1236,9 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data 
*result,
case 0x33:
ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
break;
+   case 0x41:
+   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_411;
+   break;
default:
return false;
}
-- 
2.7.4



[PATCH 0/9] [media] s5p-jpeg: Various fixes and improvements

2017-06-02 Thread Thierry Escande
Hi,

This series contains various fixes and improvements for the Samsung
s5p-jpeg driver. All these patches come from the Chromium v3.8 kernel
tree.

Regards,
 Thierry

Abhilash Kesavan (1):
  [media] s5p-jpeg: Reset the Codec before doing a soft reset

Ricky Liang (1):
  [media] s5p-jpeg: Add support for multi-planar APIs

Tony K Nadackal (4):
  [media] s5p-jpeg: Call jpeg_bound_align_image after qbuf
  [media] s5p-jpeg: Correct WARN_ON statement for checking subsampling
  [media] s5p-jpeg: Decode 4:1:1 chroma subsampling format
  [media] s5p-jpeg: Add IOMMU support

henryhsu (3):
  [media] s5p-jpeg: Add support for resolution change event
  [media] s5p-jpeg: Change sclk_jpeg to 166MHz for Exynos5250
  [media] s5p-jpeg: Add stream error handling for Exynos5420

 drivers/media/platform/s5p-jpeg/jpeg-core.c   | 387 --
 drivers/media/platform/s5p-jpeg/jpeg-core.h   |   9 +
 drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c |   4 +
 3 files changed, 368 insertions(+), 32 deletions(-)

-- 
2.7.4



[PATCH 1/9] [media] s5p-jpeg: Reset the Codec before doing a soft reset

2017-06-02 Thread Thierry Escande
From: Abhilash Kesavan 

This patch resets the encoding and decoding register bits before doing a
soft reset.

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c 
b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
index a1d823a..9ad8f6d 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c
@@ -21,6 +21,10 @@ void exynos4_jpeg_sw_reset(void __iomem *base)
unsigned int reg;
 
reg = readl(base + EXYNOS4_JPEG_CNTL_REG);
+   writel(reg & ~(EXYNOS4_DEC_MODE | EXYNOS4_ENC_MODE),
+  base + EXYNOS4_JPEG_CNTL_REG);
+
+   reg = readl(base + EXYNOS4_JPEG_CNTL_REG);
writel(reg & ~EXYNOS4_SOFT_RESET_HI, base + EXYNOS4_JPEG_CNTL_REG);
 
udelay(100);
-- 
2.7.4



[PATCH 2/9] [media] s5p-jpeg: Call jpeg_bound_align_image after qbuf

2017-06-02 Thread Thierry Escande
From: Tony K Nadackal 

When queuing an OUTPUT buffer for decoder, s5p_jpeg_parse_hdr()
function parses the input jpeg file and takes the width and height
parameters from its header. These new width/height values will be used
for the calculation of stride. HX_JPEG Hardware needs the width and
height values aligned on a 16 bits boundary. This width/height alignment
is handled in the s5p_jpeg_s_fmt_vid_cap() function during the S_FMT
ioctl call.

But if user space calls the QBUF of OUTPUT buffer after the S_FMT of
CAPTURE buffer, these aligned values will be replaced by the values in
jpeg header. If the width/height values of jpeg are not aligned, the
decoder output will be corrupted. So in this patch we call
jpeg_bound_align_image() to align the width/height values of Capture
buffer in s5p_jpeg_buf_queue().

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 52dc794..6fb1ab4 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2523,6 +2523,13 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
q_data = >cap_q;
q_data->w = tmp.w;
q_data->h = tmp.h;
+
+   jpeg_bound_align_image(ctx, _data->w, S5P_JPEG_MIN_WIDTH,
+  S5P_JPEG_MAX_WIDTH, q_data->fmt->h_align,
+  _data->h, S5P_JPEG_MIN_HEIGHT,
+  S5P_JPEG_MAX_HEIGHT, q_data->fmt->v_align
+ );
+   q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
}
 
v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
-- 
2.7.4



[PATCH 5/9] [media] s5p-jpeg: Add IOMMU support

2017-06-02 Thread Thierry Escande
From: Tony K Nadackal 

This patch adds support for IOMMU s5p-jpeg driver if the Exynos IOMMU
and ARM DMA IOMMU configurations are supported. The address space is
created with size limited to 256M and base address set to 0x2000.

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 77 +
 1 file changed, 77 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 770a709..5569b99 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -28,6 +28,14 @@
 #include 
 #include 
 #include 
+#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
 
 #include "jpeg-core.h"
 #include "jpeg-hw-s5p.h"
@@ -35,6 +43,10 @@
 #include "jpeg-hw-exynos3250.h"
 #include "jpeg-regs.h"
 
+#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
+static struct dma_iommu_mapping *mapping;
+#endif
+
 static struct s5p_jpeg_fmt sjpeg_formats[] = {
{
.name   = "JPEG JFIF",
@@ -956,6 +968,60 @@ static void exynos4_jpeg_parse_q_tbl(struct s5p_jpeg_ctx 
*ctx)
}
 }
 
+#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
+static int jpeg_iommu_init(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   int err;
+
+   mapping = arm_iommu_create_mapping(_bus_type, 0x2000,
+  SZ_512M);
+   if (IS_ERR(mapping)) {
+   dev_err(dev, "IOMMU mapping failed\n");
+   return PTR_ERR(mapping);
+   }
+
+   dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL);
+   if (!dev->dma_parms) {
+   err = -ENOMEM;
+   goto error_alloc;
+   }
+
+   err = dma_set_max_seg_size(dev, 0xu);
+   if (err)
+   goto error;
+
+   err = arm_iommu_attach_device(dev, mapping);
+   if (err)
+   goto error;
+
+   return 0;
+
+error:
+   devm_kfree(dev, dev->dma_parms);
+   dev->dma_parms = NULL;
+
+error_alloc:
+   arm_iommu_release_mapping(mapping);
+   mapping = NULL;
+
+   return err;
+}
+
+static void jpeg_iommu_deinit(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+
+   if (mapping) {
+   arm_iommu_detach_device(dev);
+   devm_kfree(dev, dev->dma_parms);
+   dev->dma_parms = NULL;
+   arm_iommu_release_mapping(mapping);
+   mapping = NULL;
+   }
+}
+#endif
+
 /*
  * 
  * Device file operations
@@ -2816,6 +2882,13 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
spin_lock_init(>slock);
jpeg->dev = >dev;
 
+#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
+   ret = jpeg_iommu_init(pdev);
+   if (ret) {
+   dev_err(>dev, "IOMMU Initialization failed\n");
+   return ret;
+   }
+#endif
/* memory-mapped registers */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
@@ -2962,6 +3035,10 @@ static int s5p_jpeg_remove(struct platform_device *pdev)
clk_disable_unprepare(jpeg->clocks[i]);
}
 
+#if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
+   jpeg_iommu_deinit(pdev);
+#endif
+
return 0;
 }
 
-- 
2.7.4



[PATCH 6/9] [media] s5p-jpeg: Add support for resolution change event

2017-06-02 Thread Thierry Escande
From: henryhsu 

This patch adds support for resolution change event to notify clients so
they can prepare correct output buffer. When resolution change happened,
G_FMT for CAPTURE should return old resolution and format before CAPTURE
queues streamoff.

Signed-off-by: Henry-Ruey Hsu 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 121 
 drivers/media/platform/s5p-jpeg/jpeg-core.h |   7 ++
 2 files changed, 95 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 5569b99..7a7acbc 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1416,8 +1417,17 @@ static int s5p_jpeg_g_fmt(struct file *file, void *priv, 
struct v4l2_format *f)
q_data = get_q_data(ct, f->type);
BUG_ON(q_data == NULL);
 
-   pix->width = q_data->w;
-   pix->height = q_data->h;
+   if ((f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+ct->mode == S5P_JPEG_ENCODE) ||
+   (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
+ct->mode == S5P_JPEG_DECODE)) {
+   pix->width = 0;
+   pix->height = 0;
+   } else {
+   pix->width = q_data->w;
+   pix->height = q_data->h;
+   }
+
pix->field = V4L2_FIELD_NONE;
pix->pixelformat = q_data->fmt->fourcc;
pix->bytesperline = 0;
@@ -1677,8 +1687,6 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
FMT_TYPE_OUTPUT : FMT_TYPE_CAPTURE;
 
q_data->fmt = s5p_jpeg_find_format(ct, pix->pixelformat, f_type);
-   q_data->w = pix->width;
-   q_data->h = pix->height;
if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG) {
/*
 * During encoding Exynos4x12 SoCs access wider memory area
@@ -1686,6 +1694,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
 * the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
 * page fault calculate proper buffer size in such a case.
 */
+   q_data->w = pix->width;
+   q_data->h = pix->height;
if (ct->jpeg->variant->hw_ex4_compat &&
f_type == FMT_TYPE_OUTPUT && ct->mode == S5P_JPEG_ENCODE)
q_data->size = exynos4_jpeg_get_output_buffer_size(ct,
@@ -1761,6 +1771,15 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, 
void *priv,
return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
 }
 
+static int s5p_jpeg_subscribe_event(struct v4l2_fh *fh,
+   const struct v4l2_event_subscription *sub)
+{
+   if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
+   return v4l2_src_change_event_subscribe(fh, sub);
+
+   return -EINVAL;
+}
+
 static int exynos3250_jpeg_try_downscale(struct s5p_jpeg_ctx *ctx,
   struct v4l2_rect *r)
 {
@@ -2086,6 +2105,9 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = {
 
.vidioc_g_selection = s5p_jpeg_g_selection,
.vidioc_s_selection = s5p_jpeg_s_selection,
+
+   .vidioc_subscribe_event = s5p_jpeg_subscribe_event,
+   .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
 };
 
 /*
@@ -2478,8 +2500,17 @@ static int s5p_jpeg_job_ready(void *priv)
 {
struct s5p_jpeg_ctx *ctx = priv;
 
-   if (ctx->mode == S5P_JPEG_DECODE)
+   if (ctx->mode == S5P_JPEG_DECODE) {
+   /*
+* We have only one input buffer and one output buffer. If there
+* is a resolution change event, no need to continue decoding.
+*/
+   if (ctx->state == JPEGCTX_RESOLUTION_CHANGE)
+   return 0;
+
return ctx->hdr_parsed;
+   }
+
return 1;
 }
 
@@ -2558,6 +2589,21 @@ static int s5p_jpeg_buf_prepare(struct vb2_buffer *vb)
return 0;
 }
 
+static void s5p_jpeg_set_capture_queue_data(struct s5p_jpeg_ctx *ctx)
+{
+   struct s5p_jpeg_q_data *q_data = >cap_q;
+
+   q_data->w = ctx->out_q.w;
+   q_data->h = ctx->out_q.h;
+
+   jpeg_bound_align_image(ctx, _data->w, S5P_JPEG_MIN_WIDTH,
+  S5P_JPEG_MAX_WIDTH, q_data->fmt->h_align,
+  _data->h, S5P_JPEG_MIN_HEIGHT,
+  S5P_JPEG_MAX_HEIGHT, q_data->fmt->v_align);
+
+   q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
+}
+
 static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
 {
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -2565,9 +2611,20 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
 
  

[PATCH 8/9] [media] s5p-jpeg: Add stream error handling for Exynos5420

2017-06-02 Thread Thierry Escande
From: henryhsu 

On Exynos5420, the STREAM_STAT bit raised on the JPGINTST register means
there is a syntax error or an unrecoverable error on compressed file
when ERR_INT_EN is set to 1.

Fix this case and report BUF_STATE_ERROR to videobuf2.

Signed-off-by: Henry-Ruey Hsu 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 430e925..db56135 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2894,6 +2894,7 @@ static irqreturn_t exynos3250_jpeg_irq(int irq, void 
*dev_id)
unsigned long payload_size = 0;
enum vb2_buffer_state state = VB2_BUF_STATE_DONE;
bool interrupt_timeout = false;
+   bool stream_error = false;
u32 irq_status;
 
spin_lock(>slock);
@@ -2910,6 +2911,11 @@ static irqreturn_t exynos3250_jpeg_irq(int irq, void 
*dev_id)
 
jpeg->irq_status |= irq_status;
 
+   if (irq_status & EXYNOS3250_STREAM_STAT) {
+   stream_error = true;
+   dev_err(jpeg->dev, "Syntax error or unrecoverable error 
occurred.\n");
+   }
+
curr_ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
 
if (!curr_ctx)
@@ -2926,7 +2932,7 @@ static irqreturn_t exynos3250_jpeg_irq(int irq, void 
*dev_id)
EXYNOS3250_RDMA_DONE |
EXYNOS3250_RESULT_STAT))
payload_size = exynos3250_jpeg_compressed_size(jpeg->regs);
-   else if (interrupt_timeout)
+   else if (interrupt_timeout || stream_error)
state = VB2_BUF_STATE_ERROR;
else
goto exit_unlock;
-- 
2.7.4



[PATCH 7/9] [media] s5p-jpeg: Change sclk_jpeg to 166MHz for Exynos5250

2017-06-02 Thread Thierry Escande
From: henryhsu 

The default clock parent of jpeg on Exynos5250 is fin_pll, which is
24MHz. We have to change the clock parent to CPLL, which is 333MHz,
and set sclk_jpeg to 166MHz.

Signed-off-by: Heng-Ruey Hsu 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 47 +
 1 file changed, 47 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 7a7acbc..430e925 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -969,6 +969,44 @@ static void exynos4_jpeg_parse_q_tbl(struct s5p_jpeg_ctx 
*ctx)
}
 }
 
+static int exynos4_jpeg_set_sclk_rate(struct s5p_jpeg *jpeg, struct clk *sclk)
+{
+   struct clk *mout_jpeg;
+   struct clk *sclk_cpll;
+   int ret;
+
+   mout_jpeg = clk_get(jpeg->dev, "mout_jpeg");
+   if (IS_ERR(mout_jpeg)) {
+   dev_err(jpeg->dev, "mout_jpeg clock not available: %ld\n",
+   PTR_ERR(mout_jpeg));
+   return PTR_ERR(mout_jpeg);
+   }
+
+   sclk_cpll = clk_get(jpeg->dev, "sclk_cpll");
+   if (IS_ERR(sclk_cpll)) {
+   dev_err(jpeg->dev, "sclk_cpll clock not available: %ld\n",
+   PTR_ERR(sclk_cpll));
+   clk_put(mout_jpeg);
+   return PTR_ERR(sclk_cpll);
+   }
+
+   ret = clk_set_parent(mout_jpeg, sclk_cpll);
+   clk_put(sclk_cpll);
+   clk_put(mout_jpeg);
+   if (ret) {
+   dev_err(jpeg->dev, "clk_set_parent failed: %d\n", ret);
+   return ret;
+   }
+
+   ret = clk_set_rate(sclk, 166500 * 1000);
+   if (ret) {
+   dev_err(jpeg->dev, "clk_set_rate failed: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
 #if defined(CONFIG_EXYNOS_IOMMU) && defined(CONFIG_ARM_DMA_USE_IOMMU)
 static int jpeg_iommu_init(struct platform_device *pdev)
 {
@@ -2974,6 +3012,15 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
jpeg->variant->clk_names[i]);
return PTR_ERR(jpeg->clocks[i]);
}
+
+   if (jpeg->variant->version == SJPEG_EXYNOS4 &&
+   !strncmp(jpeg->variant->clk_names[i],
+"sclk", strlen("sclk"))) {
+   ret = exynos4_jpeg_set_sclk_rate(jpeg,
+jpeg->clocks[i]);
+   if (ret)
+   return ret;
+   }
}
 
/* v4l2 device */
-- 
2.7.4



[PATCH 9/9] [media] s5p-jpeg: Add support for multi-planar APIs

2017-06-02 Thread Thierry Escande
From: Ricky Liang 

This patch adds multi-planar APIs to the s5p-jpeg driver. The multi-planar
APIs are identical to the exisiting single-planar APIs except the plane
format info is stored in the v4l2_pixel_format_mplan struct instead
of the v4l2_pixel_format struct.

Signed-off-by: Ricky Liang 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 152 +---
 drivers/media/platform/s5p-jpeg/jpeg-core.h |   2 +
 2 files changed, 139 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index db56135..a8fd7ed 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1371,6 +1371,15 @@ static int s5p_jpeg_querycap(struct file *file, void 
*priv,
 dev_name(ctx->jpeg->dev));
cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M;
cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
+   /*
+* Advertise multi-planar capabilities. The driver supports only
+* single-planar pixel format at this moment so all the buffers will
+* have only one plane.
+*/
+   cap->capabilities |= V4L2_CAP_VIDEO_M2M_MPLANE |
+V4L2_CAP_VIDEO_CAPTURE_MPLANE |
+V4L2_CAP_VIDEO_OUTPUT_MPLANE;
+
return 0;
 }
 
@@ -1430,12 +1439,10 @@ static int s5p_jpeg_enum_fmt_vid_out(struct file *file, 
void *priv,
 static struct s5p_jpeg_q_data *get_q_data(struct s5p_jpeg_ctx *ctx,
  enum v4l2_buf_type type)
 {
-   if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+   if (V4L2_TYPE_IS_OUTPUT(type))
return >out_q;
-   if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-   return >cap_q;
 
-   return NULL;
+   return >cap_q;
 }
 
 static int s5p_jpeg_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
@@ -1449,16 +1456,14 @@ static int s5p_jpeg_g_fmt(struct file *file, void 
*priv, struct v4l2_format *f)
if (!vq)
return -EINVAL;
 
-   if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   if (!V4L2_TYPE_IS_OUTPUT(f->type) &&
ct->mode == S5P_JPEG_DECODE && !ct->hdr_parsed)
return -EINVAL;
q_data = get_q_data(ct, f->type);
BUG_ON(q_data == NULL);
 
-   if ((f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-ct->mode == S5P_JPEG_ENCODE) ||
-   (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
-ct->mode == S5P_JPEG_DECODE)) {
+   if ((!V4L2_TYPE_IS_OUTPUT(f->type) && ct->mode == S5P_JPEG_ENCODE) ||
+   (V4L2_TYPE_IS_OUTPUT(f->type) && ct->mode == S5P_JPEG_DECODE)) {
pix->width = 0;
pix->height = 0;
} else {
@@ -1715,6 +1720,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
 
q_data = get_q_data(ct, f->type);
BUG_ON(q_data == NULL);
+   vq->type = f->type;
+   q_data->type = f->type;
 
if (vb2_is_busy(vq)) {
v4l2_err(>jpeg->v4l2_dev, "%s queue busy\n", __func__);
@@ -1919,7 +1926,9 @@ static int s5p_jpeg_g_selection(struct file *file, void 
*priv,
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
 
if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
-   s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
+   s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
return -EINVAL;
 
/* For JPEG blob active == default == bounds */
@@ -1957,7 +1966,8 @@ static int s5p_jpeg_s_selection(struct file *file, void 
*fh,
struct v4l2_rect *rect = >r;
int ret = -EINVAL;
 
-   if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+   s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
return -EINVAL;
 
if (s->target == V4L2_SEL_TGT_COMPOSE) {
@@ -2118,6 +2128,107 @@ static int s5p_jpeg_controls_create(struct s5p_jpeg_ctx 
*ctx)
return ret;
 }
 
+static void v4l2_format_pixmp_to_pix(struct v4l2_format *fmt_pix_mp,
+struct v4l2_format *fmt_pix) {
+   struct v4l2_pix_format *pix = _pix->fmt.pix;
+   struct v4l2_pix_format_mplane *pix_mp = _pix_mp->fmt.pix_mp;
+
+   fmt_pix->type = fmt_pix_mp->type;
+   pix->width = pix_mp->width;
+   pix->height = pix_mp->height;
+   pix->pixelformat = pix_mp->pixelformat;
+   pix->field = pix_mp->field;
+   pix->colorspace = pix_mp->colorspace;
+   pix->bytesperline = pix_mp->plane_fmt[0].bytesperline;
+   pix->sizeimage = pix_mp->plane_fmt[0].sizeimage;
+}
+
+static void v4l2_format_pixmp_from_pix(struct v4l2_format *fmt_pix_mp,
+   

[PATCH 3/9] [media] s5p-jpeg: Correct WARN_ON statement for checking subsampling

2017-06-02 Thread Thierry Escande
From: Tony K Nadackal 

Corrects the WARN_ON statement for subsampling based on the
JPEG Hardware version.

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 6fb1ab4..0d83948 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -614,24 +614,26 @@ static inline struct s5p_jpeg_ctx *fh_to_ctx(struct 
v4l2_fh *fh)
 
 static int s5p_jpeg_to_user_subsampling(struct s5p_jpeg_ctx *ctx)
 {
-   WARN_ON(ctx->subsampling > 3);
-
switch (ctx->jpeg->variant->version) {
case SJPEG_S5P:
+   WARN_ON(ctx->subsampling > 3);
if (ctx->subsampling > 2)
return V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
return ctx->subsampling;
case SJPEG_EXYNOS3250:
case SJPEG_EXYNOS5420:
+   WARN_ON(ctx->subsampling > 6);
if (ctx->subsampling > 3)
return V4L2_JPEG_CHROMA_SUBSAMPLING_411;
return exynos3250_decoded_subsampling[ctx->subsampling];
case SJPEG_EXYNOS4:
case SJPEG_EXYNOS5433:
+   WARN_ON(ctx->subsampling > 3);
if (ctx->subsampling > 2)
return V4L2_JPEG_CHROMA_SUBSAMPLING_420;
return exynos4x12_decoded_subsampling[ctx->subsampling];
default:
+   WARN_ON(ctx->subsampling > 3);
return V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
}
 }
-- 
2.7.4



Re: [PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Philipp Zabel
Hi Dave,

On Fri, 2017-06-02 at 15:36 +0100, Dave Stevenson wrote:
[...]
> >> > Are you aware of the HDMI modes that the TC358743 driver has been used 
> >> > with?
> >> > The comments mention 720P60 at 594MHz, but I have had to modify the
> >> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
> >> > value came out of Toshiba's spreadsheet for computing register
> >> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
> >> > so not a huge change.
> >> > Is it worth going to the effort of dynamically computing the delay, or
> >> > is increasing the default acceptable?
> >>
> >> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
> >> I have CC-ed him as I am not sure where those values came from.
> >
> > I've just chosen a small delay that worked reliably. For 4-lane 1080p60
> > and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet
> > believes that it is ok to decrease the FIFO delay all the way down to 0
> > (it is not). I think it should be fine to delay transmission for a few
> > microseconds unconditionally, I'll test this next week.
> 
> Thanks Philipp. Were 1080p60 and 720p60 the only modes you were really 
> testing?

Yes. Since 720p60 needs half the bandwidth of 1080p60, it was possible
to just use the 1080p60 timings by switching from 4 to 2 lanes.

> Did the 594Mbps lane speed come from a specific requirement somewhere?

It is what the spreadsheed suggests for 4-lane 1080p60, I assume because
it is nice and even to generate from the 27 MHz reference clock, and it
fits the 547.28 Mbps/lane requirement (for YCbCr 4:2:2 CSI output) with
a bit of headroom.

> The standard Pi only has 2 CSI2 lanes exposed, and 1080P30 RGB3 just
> tips over into needing 3 lanes with the current link frequency.
>  When I can find a bit more time I was thinking that an alternate link
> frequency would allow us to squeeze it in to 2 lanes. Obviously the
> timing values need to be checked carefully, but it should all work and
> allow support of multiple link frequencies.

See the FIXME comment in tc358743_probe_of, you could calculate and add
the correct pdata for the raised link-frequency.

> (My calcs say that 1080p50 UYVY can fit down 2 lanes, but that
> requires more extensive register mods).

With a lane rate of 911.25 Mbps or higher that should be possible, with
all the CSI timings are relaxed a bit.

regards
Philipp




Re: [PATCH] ARM: dts: exynos: Add HDMI CEC device to Exynos5 SoC family

2017-06-02 Thread Krzysztof Kozlowski
On Thu, Jun 01, 2017 at 08:19:23AM +0200, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> On 2017-05-31 21:55, Krzysztof Kozlowski wrote:
> > On Wed, May 31, 2017 at 01:00:17PM +0200, Marek Szyprowski wrote:
> > > Exynos5250 and Exynos542x SoCs have the same CEC hardware module as
> > > Exynos4 SoC series, so enable support for it using the same compatible
> > > string.
> > > 
> > > Tested on Odroid XU3 (Exynos5422) and Google Snow (Exynos5250) boards.
> > > 
> > > Signed-off-by: Marek Szyprowski 
> > > ---
> > >   arch/arm/boot/dts/exynos5250-pinctrl.dtsi  |  7 +++
> > >   arch/arm/boot/dts/exynos5250-snow-common.dtsi  |  4 
> > >   arch/arm/boot/dts/exynos5250.dtsi  | 13 +
> > >   arch/arm/boot/dts/exynos5420-pinctrl.dtsi  |  7 +++
> > >   arch/arm/boot/dts/exynos5420.dtsi  | 13 +
> > >   arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |  4 
> > >   6 files changed, 48 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi 
> > > b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> > > index 2f6ab32b5954..1fd122db18e6 100644
> > > --- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> > > +++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> > > @@ -589,6 +589,13 @@
> > >   samsung,pin-pud = ;
> > >   samsung,pin-drv = ;
> > >   };
> > > +
> > > + hdmi_cec: hdmi-cec {
> > > + samsung,pins = "gpx3-6";
> > > + samsung,pin-function = ;
> > > + samsung,pin-pud = ;
> > > + samsung,pin-drv = ;
> > > + };
> > >   };
> > >   _1 {
> > > diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi 
> > > b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
> > > index 8f3a80430748..e1d293dbbe5d 100644
> > > --- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
> > > +++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
> > > @@ -272,6 +272,10 @@
> > >   vdd_pll-supply = <_reg>;
> > >   };
> > > + {
> > > + status = "okay";
> > > +};
> > > +
> > >   _0 {
> > >   status = "okay";
> > >   samsung,i2c-sda-delay = <100>;
> > > diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
> > > b/arch/arm/boot/dts/exynos5250.dtsi
> > > index 79c9c885613a..fbdc1d53a2ce 100644
> > > --- a/arch/arm/boot/dts/exynos5250.dtsi
> > > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> > > @@ -689,6 +689,19 @@
> > >   samsung,syscon-phandle = 
> > > <_system_controller>;
> > >   };
> > > + hdmicec: cec@101B {
> > > + compatible = "samsung,s5p-cec";
> > > + reg = <0x101B 0x200>;
> > > + interrupts = ;
> > > + clocks = < CLK_HDMI_CEC>;
> > > + clock-names = "hdmicec";
> > > + samsung,syscon-phandle = <_system_controller>;
> > > + hdmi-phandle = <>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <_cec>;
> > > + status = "disabled";
> > > + };
> > What about Exynos5410? Is it applicable there as well? If yes, then this
> > could be added to exynos5.dtsi... although then clocks and pinctrl
> > should remain in SoC-specific DTSI. We're following such pattern in many
> > places but I am not sure if this more readable.
> 
> Exynos5410 has the same HW module, but as for now, it doesn't have support
> for
> HDMI due to missing a few pieces (mainly clocks definitions). I'm not sure
> if it makes sense to add only HDMICEC without HDMI itself. Maybe later, when
> multimedia support is added to Exynos5410, this can be integrated to
> exynos5.dtsi.

Sounds fair, applied. Thanks!

Best regards,
Krzysztof



Re: [PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Dave Stevenson
On 2 June 2017 at 15:13, Philipp Zabel  wrote:
> On Fri, 2017-06-02 at 15:27 +0200, Hans Verkuil wrote:
>> On 06/02/17 15:03, Dave Stevenson wrote:
>> > Hi Hans.
>> >
>> > On 2 June 2017 at 13:35, Hans Verkuil  wrote:
>> >> On 06/02/17 14:18, Dave Stevenson wrote:
>> >>> These 3 patches for TC358743 came out of trying to use the
>> >>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
>> >>
>> >> Nice! Doing that has been on my todo list for ages but I never got
>> >> around to it. I have one of these and using the Raspberry Pi with
>> >> the tc358743 would allow me to add a CEC driver as well.
>> >
>> > It's been on my list for a while too! It's working, but just the final
>> > clean ups needed.
>> > I've got 1 v4l2-compliance failure still outstanding that needs
>> > digging into (subscribe_event), rebasing on top of the fwnode tree,
>> > and a couple of config things to tidy up. RFC hopefully next week.
>> > I'm testing with a demo board designed here at Pi Towers, but there
>> > are others successfully testing it using the auvidea.com B101 board.
>> >
>> > Are you aware of the HDMI modes that the TC358743 driver has been used 
>> > with?
>> > The comments mention 720P60 at 594MHz, but I have had to modify the
>> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
>> > value came out of Toshiba's spreadsheet for computing register
>> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
>> > so not a huge change.
>> > Is it worth going to the effort of dynamically computing the delay, or
>> > is increasing the default acceptable?
>>
>> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
>> I have CC-ed him as I am not sure where those values came from.
>
> I've just chosen a small delay that worked reliably. For 4-lane 1080p60
> and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet
> believes that it is ok to decrease the FIFO delay all the way down to 0
> (it is not). I think it should be fine to delay transmission for a few
> microseconds unconditionally, I'll test this next week.

Thanks Philipp. Were 1080p60 and 720p60 the only modes you were really testing?

Did the 594Mbps lane speed come from a specific requirement somewhere?
The standard Pi only has 2 CSI2 lanes exposed, and 1080P30 RGB3 just
tips over into needing 3 lanes with the current link frequency. When I
can find a bit more time I was thinking that an alternate link
frequency would allow us to squeeze it in to 2 lanes. Obviously the
timing values need to be checked carefully, but it should all work and
allow support of multiple link frequencies.
(My calcs say that 1080p50 UYVY can fit down 2 lanes, but that
requires more extensive register mods).

>> This driver is also used in a Cisco product, but we use platform_data for 
>> that.
>> Here are our settings that we use for reference:
>>
>> static struct tc358743_platform_data tc358743_pdata = {
>> .refclk_hz = 2700,
>> .ddc5v_delay = DDC5V_DELAY_100_MS,
>> .fifo_level = 300,
>> .pll_prd = 4,
>> .pll_fbd = 122,
>> /* CSI */
>> .lineinitcnt = 0x1770,
>> .lptxtimecnt = 0x0005,
>> .tclk_headercnt = 0x1d04,
>> .ths_headercnt = 0x0505,
>> .twakeup = 0x4650,
>> .ths_trailcnt = 0x0004,
>> .hstxvregcnt = 0x0005,
>> /* HDMI PHY */
>> .hdmi_phy_auto_reset_tmds_detected = true,
>> .hdmi_phy_auto_reset_tmds_in_range = true,
>> .hdmi_phy_auto_reset_tmds_valid = true,
>> .hdmi_phy_auto_reset_hsync_out_of_range = true,
>> .hdmi_phy_auto_reset_vsync_out_of_range = true,
>> .hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
>> };
>>
>> I believe these are all calculated from the Toshiba spreadsheet.
>>
>> Frankly, I have no idea what they mean :-)
>>
>> I am fine with increasing the default if Philipp is OK as well. Since
>> Cisco uses a value of 300 I would expect that 16 is indeed too low.
>>
>> Regards,
>>
>>   Hans
>>
>> >
>> >>> A couple of the subdevice API calls were not implemented or
>> >>> otherwise gave odd results. Those are fixed.
>> >>>
>> >>> The TC358743 interface board being used didn't have the IRQ
>> >>> line wired up to the SoC. "interrupts" is listed as being
>> >>> optional in the DT binding, but the driver didn't actually
>> >>> function if it wasn't provided.
>> >>>
>> >>> Dave Stevenson (3):
>> >>>   [media] tc358743: Add enum_mbus_code
>> >>>   [media] tc358743: Setup default mbus_fmt before registering
>> >>>   [media] tc358743: Add support for platforms without IRQ line
>> >>
>> >> All looks good, I'll take this for 4.12.
>> >
>> > Thanks.
>> >
>> >> Regards,
>> >>
>> >> Hans
>> >>

Re: [PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Philipp Zabel
On Fri, 2017-06-02 at 15:27 +0200, Hans Verkuil wrote:
> On 06/02/17 15:03, Dave Stevenson wrote:
> > Hi Hans.
> > 
> > On 2 June 2017 at 13:35, Hans Verkuil  wrote:
> >> On 06/02/17 14:18, Dave Stevenson wrote:
> >>> These 3 patches for TC358743 came out of trying to use the
> >>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
> >>
> >> Nice! Doing that has been on my todo list for ages but I never got
> >> around to it. I have one of these and using the Raspberry Pi with
> >> the tc358743 would allow me to add a CEC driver as well.
> > 
> > It's been on my list for a while too! It's working, but just the final
> > clean ups needed.
> > I've got 1 v4l2-compliance failure still outstanding that needs
> > digging into (subscribe_event), rebasing on top of the fwnode tree,
> > and a couple of config things to tidy up. RFC hopefully next week.
> > I'm testing with a demo board designed here at Pi Towers, but there
> > are others successfully testing it using the auvidea.com B101 board.
> > 
> > Are you aware of the HDMI modes that the TC358743 driver has been used with?
> > The comments mention 720P60 at 594MHz, but I have had to modify the
> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
> > value came out of Toshiba's spreadsheet for computing register
> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
> > so not a huge change.
> > Is it worth going to the effort of dynamically computing the delay, or
> > is increasing the default acceptable?
> 
> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
> I have CC-ed him as I am not sure where those values came from.

I've just chosen a small delay that worked reliably. For 4-lane 1080p60
and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet
believes that it is ok to decrease the FIFO delay all the way down to 0
(it is not). I think it should be fine to delay transmission for a few
microseconds unconditionally, I'll test this next week.

> This driver is also used in a Cisco product, but we use platform_data for 
> that.
> Here are our settings that we use for reference:
>
> static struct tc358743_platform_data tc358743_pdata = {
> .refclk_hz = 2700,
> .ddc5v_delay = DDC5V_DELAY_100_MS,
> .fifo_level = 300,
> .pll_prd = 4,
> .pll_fbd = 122,
> /* CSI */
> .lineinitcnt = 0x1770,
> .lptxtimecnt = 0x0005,
> .tclk_headercnt = 0x1d04,
> .ths_headercnt = 0x0505,
> .twakeup = 0x4650,
> .ths_trailcnt = 0x0004,
> .hstxvregcnt = 0x0005,
> /* HDMI PHY */
> .hdmi_phy_auto_reset_tmds_detected = true,
> .hdmi_phy_auto_reset_tmds_in_range = true,
> .hdmi_phy_auto_reset_tmds_valid = true,
> .hdmi_phy_auto_reset_hsync_out_of_range = true,
> .hdmi_phy_auto_reset_vsync_out_of_range = true,
> .hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
> };
> 
> I believe these are all calculated from the Toshiba spreadsheet.
> 
> Frankly, I have no idea what they mean :-)
> 
> I am fine with increasing the default if Philipp is OK as well. Since
> Cisco uses a value of 300 I would expect that 16 is indeed too low.
> 
> Regards,
> 
>   Hans
> 
> > 
> >>> A couple of the subdevice API calls were not implemented or
> >>> otherwise gave odd results. Those are fixed.
> >>>
> >>> The TC358743 interface board being used didn't have the IRQ
> >>> line wired up to the SoC. "interrupts" is listed as being
> >>> optional in the DT binding, but the driver didn't actually
> >>> function if it wasn't provided.
> >>>
> >>> Dave Stevenson (3):
> >>>   [media] tc358743: Add enum_mbus_code
> >>>   [media] tc358743: Setup default mbus_fmt before registering
> >>>   [media] tc358743: Add support for platforms without IRQ line
> >>
> >> All looks good, I'll take this for 4.12.
> > 
> > Thanks.
> > 
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>>  drivers/media/i2c/tc358743.c | 59 
> >>> +++-
> >>>  1 file changed, 58 insertions(+), 1 deletion(-)
> >>>
> >>

regards
Philipp



Re: [PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Hans Verkuil
On 06/02/17 15:03, Dave Stevenson wrote:
> Hi Hans.
> 
> On 2 June 2017 at 13:35, Hans Verkuil  wrote:
>> On 06/02/17 14:18, Dave Stevenson wrote:
>>> These 3 patches for TC358743 came out of trying to use the
>>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
>>
>> Nice! Doing that has been on my todo list for ages but I never got
>> around to it. I have one of these and using the Raspberry Pi with
>> the tc358743 would allow me to add a CEC driver as well.
> 
> It's been on my list for a while too! It's working, but just the final
> clean ups needed.
> I've got 1 v4l2-compliance failure still outstanding that needs
> digging into (subscribe_event), rebasing on top of the fwnode tree,
> and a couple of config things to tidy up. RFC hopefully next week.
> I'm testing with a demo board designed here at Pi Towers, but there
> are others successfully testing it using the auvidea.com B101 board.
> 
> Are you aware of the HDMI modes that the TC358743 driver has been used with?
> The comments mention 720P60 at 594MHz, but I have had to modify the
> fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
> value came out of Toshiba's spreadsheet for computing register
> settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
> so not a huge change.
> Is it worth going to the effort of dynamically computing the delay, or
> is increasing the default acceptable?

I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
I have CC-ed him as I am not sure where those values came from. This driver
is also used in a Cisco product, but we use platform_data for that. Here are
our settings that we use for reference:

static struct tc358743_platform_data tc358743_pdata = {
.refclk_hz = 2700,
.ddc5v_delay = DDC5V_DELAY_100_MS,
.fifo_level = 300,
.pll_prd = 4,
.pll_fbd = 122,
/* CSI */
.lineinitcnt = 0x1770,
.lptxtimecnt = 0x0005,
.tclk_headercnt = 0x1d04,
.ths_headercnt = 0x0505,
.twakeup = 0x4650,
.ths_trailcnt = 0x0004,
.hstxvregcnt = 0x0005,
/* HDMI PHY */
.hdmi_phy_auto_reset_tmds_detected = true,
.hdmi_phy_auto_reset_tmds_in_range = true,
.hdmi_phy_auto_reset_tmds_valid = true,
.hdmi_phy_auto_reset_hsync_out_of_range = true,
.hdmi_phy_auto_reset_vsync_out_of_range = true,
.hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
};

I believe these are all calculated from the Toshiba spreadsheet.

Frankly, I have no idea what they mean :-)

I am fine with increasing the default if Philipp is OK as well. Since
Cisco uses a value of 300 I would expect that 16 is indeed too low.

Regards,

Hans

> 
>>> A couple of the subdevice API calls were not implemented or
>>> otherwise gave odd results. Those are fixed.
>>>
>>> The TC358743 interface board being used didn't have the IRQ
>>> line wired up to the SoC. "interrupts" is listed as being
>>> optional in the DT binding, but the driver didn't actually
>>> function if it wasn't provided.
>>>
>>> Dave Stevenson (3):
>>>   [media] tc358743: Add enum_mbus_code
>>>   [media] tc358743: Setup default mbus_fmt before registering
>>>   [media] tc358743: Add support for platforms without IRQ line
>>
>> All looks good, I'll take this for 4.12.
> 
> Thanks.
> 
>> Regards,
>>
>> Hans
>>
>>>
>>>  drivers/media/i2c/tc358743.c | 59 
>>> +++-
>>>  1 file changed, 58 insertions(+), 1 deletion(-)
>>>
>>



Support for RGB/YUV 10, 12 BPC(bits per color/component) image data formats in kernel

2017-06-02 Thread Ajay kumar
Hi all,

I have tried searching for RGB/YUV 10, 12 BPC formats in videodev2.h,
media-bus-format.h and drm_fourcc.h
I could only find RGB 10BPC support in drm_fourcc.h.
I guess not much support is present for formats with (BPC > 8) in the kernel.

Are there any plans to add fourcc defines for such formats?
Also, I wanted to how to define fourcc code for those formats?

Thanks,
Ajay Kumar


Re: [PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Dave Stevenson
Hi Hans.

On 2 June 2017 at 13:35, Hans Verkuil  wrote:
> On 06/02/17 14:18, Dave Stevenson wrote:
>> These 3 patches for TC358743 came out of trying to use the
>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
>
> Nice! Doing that has been on my todo list for ages but I never got
> around to it. I have one of these and using the Raspberry Pi with
> the tc358743 would allow me to add a CEC driver as well.

It's been on my list for a while too! It's working, but just the final
clean ups needed.
I've got 1 v4l2-compliance failure still outstanding that needs
digging into (subscribe_event), rebasing on top of the fwnode tree,
and a couple of config things to tidy up. RFC hopefully next week.
I'm testing with a demo board designed here at Pi Towers, but there
are others successfully testing it using the auvidea.com B101 board.

Are you aware of the HDMI modes that the TC358743 driver has been used with?
The comments mention 720P60 at 594MHz, but I have had to modify the
fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
value came out of Toshiba's spreadsheet for computing register
settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
so not a huge change.
Is it worth going to the effort of dynamically computing the delay, or
is increasing the default acceptable?

>> A couple of the subdevice API calls were not implemented or
>> otherwise gave odd results. Those are fixed.
>>
>> The TC358743 interface board being used didn't have the IRQ
>> line wired up to the SoC. "interrupts" is listed as being
>> optional in the DT binding, but the driver didn't actually
>> function if it wasn't provided.
>>
>> Dave Stevenson (3):
>>   [media] tc358743: Add enum_mbus_code
>>   [media] tc358743: Setup default mbus_fmt before registering
>>   [media] tc358743: Add support for platforms without IRQ line
>
> All looks good, I'll take this for 4.12.

Thanks.

> Regards,
>
> Hans
>
>>
>>  drivers/media/i2c/tc358743.c | 59 
>> +++-
>>  1 file changed, 58 insertions(+), 1 deletion(-)
>>
>


Re: [PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Hans Verkuil
On 06/02/17 14:18, Dave Stevenson wrote:
> These 3 patches for TC358743 came out of trying to use the
> existing driver with a new Raspberry Pi CSI-2 receiver driver.

Nice! Doing that has been on my todo list for ages but I never got
around to it. I have one of these and using the Raspberry Pi with
the tc358743 would allow me to add a CEC driver as well.

> A couple of the subdevice API calls were not implemented or
> otherwise gave odd results. Those are fixed.
> 
> The TC358743 interface board being used didn't have the IRQ
> line wired up to the SoC. "interrupts" is listed as being
> optional in the DT binding, but the driver didn't actually
> function if it wasn't provided.
> 
> Dave Stevenson (3):
>   [media] tc358743: Add enum_mbus_code
>   [media] tc358743: Setup default mbus_fmt before registering
>   [media] tc358743: Add support for platforms without IRQ line

All looks good, I'll take this for 4.12.

Regards,

Hans

> 
>  drivers/media/i2c/tc358743.c | 59 
> +++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 



[PATCH 2/3] [media] tc358743: Setup default mbus_fmt before registering

2017-06-02 Thread Dave Stevenson
Previously the mbus_fmt_code was set after the device was
registered. If a connected sub-device called tc358743_get_fmt
prior to that point it would get an invalid code back.

Signed-off-by: Dave Stevenson 
---
 drivers/media/i2c/tc358743.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 06bfdca..2f5763d 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1905,6 +1905,8 @@ static int tc358743_probe(struct i2c_client *client,
if (err < 0)
goto err_hdl;
 
+   state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24;
+
sd->dev = >dev;
err = v4l2_async_register_subdev(sd);
if (err < 0)
@@ -1919,7 +1921,6 @@ static int tc358743_probe(struct i2c_client *client,
 
tc358743_s_dv_timings(sd, _timing);
 
-   state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24;
tc358743_set_csi_color_space(sd);
 
tc358743_init_interrupts(sd);
-- 
2.7.4



[PATCH 1/3] [media] tc358743: Add enum_mbus_code

2017-06-02 Thread Dave Stevenson
There was no way to query the supported mbus formats from this
driver. enum_mbus_code is the function to expose that, so
implement it.

Signed-off-by: Dave Stevenson 
---
 drivers/media/i2c/tc358743.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 3251cba..06bfdca 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1473,6 +1473,23 @@ static int tc358743_s_stream(struct v4l2_subdev *sd, int 
enable)
 
 /* --- PAD OPS --- */
 
+static int tc358743_enum_mbus_code(struct v4l2_subdev *sd,
+   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_mbus_code_enum *code)
+{
+   switch (code->index) {
+   case 0:
+   code->code = MEDIA_BUS_FMT_RGB888_1X24;
+   break;
+   case 1:
+   code->code = MEDIA_BUS_FMT_UYVY8_1X16;
+   break;
+   default:
+   return -EINVAL;
+   }
+   return 0;
+}
+
 static int tc358743_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
@@ -1642,6 +1659,7 @@ static const struct v4l2_subdev_video_ops 
tc358743_video_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops tc358743_pad_ops = {
+   .enum_mbus_code = tc358743_enum_mbus_code,
.set_fmt = tc358743_set_fmt,
.get_fmt = tc358743_get_fmt,
.get_edid = tc358743_g_edid,
-- 
2.7.4



[PATCH 3/3] [media] tc358743: Add support for platforms without IRQ line

2017-06-02 Thread Dave Stevenson
interrupts is listed as an optional property in the DT
binding, but in reality the driver didn't work without it.
The existing driver relied on having the interrupt line
connected to the SoC to trigger handling various events.

Add the option to poll the interrupt status register via
a timer if no interrupt source is defined.

Signed-off-by: Dave Stevenson 
---
 drivers/media/i2c/tc358743.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 2f5763d..654aac2 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -61,6 +62,8 @@ MODULE_LICENSE("GPL");
 
 #define I2C_MAX_XFER_SIZE  (EDID_BLOCK_SIZE + 2)
 
+#define POLL_INTERVAL_MS   1000
+
 static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
@@ -91,6 +94,9 @@ struct tc358743_state {
 
struct delayed_work delayed_work_enable_hotplug;
 
+   struct timer_list timer;
+   struct work_struct work_i2c_poll;
+
/* edid  */
u8 edid_blocks_written;
 
@@ -1319,6 +1325,24 @@ static irqreturn_t tc358743_irq_handler(int irq, void 
*dev_id)
return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
+static void tc358743_irq_poll_timer(unsigned long arg)
+{
+   struct tc358743_state *state = (struct tc358743_state *)arg;
+
+   schedule_work(>work_i2c_poll);
+
+   mod_timer(>timer, jiffies + msecs_to_jiffies(POLL_INTERVAL_MS));
+}
+
+static void tc358743_work_i2c_poll(struct work_struct *work)
+{
+   struct tc358743_state *state = container_of(work,
+   struct tc358743_state, work_i2c_poll);
+   bool handled;
+
+   tc358743_isr(>sd, 0, );
+}
+
 static int tc358743_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
struct v4l2_event_subscription *sub)
 {
@@ -1933,6 +1957,14 @@ static int tc358743_probe(struct i2c_client *client,
"tc358743", state);
if (err)
goto err_work_queues;
+   } else {
+   INIT_WORK(>work_i2c_poll,
+ tc358743_work_i2c_poll);
+   state->timer.data = (unsigned long)state;
+   state->timer.function = tc358743_irq_poll_timer;
+   state->timer.expires = jiffies +
+  msecs_to_jiffies(POLL_INTERVAL_MS);
+   add_timer(>timer);
}
 
tc358743_enable_interrupts(sd, tx_5v_power_present(sd));
@@ -1948,6 +1980,8 @@ static int tc358743_probe(struct i2c_client *client,
return 0;
 
 err_work_queues:
+   if (!state->i2c_client->irq)
+   flush_work(>work_i2c_poll);
cancel_delayed_work(>delayed_work_enable_hotplug);
mutex_destroy(>confctl_mutex);
 err_hdl:
@@ -1961,6 +1995,10 @@ static int tc358743_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct tc358743_state *state = to_state(sd);
 
+   if (!state->i2c_client->irq) {
+   del_timer_sync(>timer);
+   flush_work(>work_i2c_poll);
+   }
cancel_delayed_work(>delayed_work_enable_hotplug);
v4l2_async_unregister_subdev(sd);
v4l2_device_unregister_subdev(sd);
-- 
2.7.4



[PATCH 0/3] tc358743: minor driver fixes

2017-06-02 Thread Dave Stevenson
These 3 patches for TC358743 came out of trying to use the
existing driver with a new Raspberry Pi CSI-2 receiver driver.

A couple of the subdevice API calls were not implemented or
otherwise gave odd results. Those are fixed.

The TC358743 interface board being used didn't have the IRQ
line wired up to the SoC. "interrupts" is listed as being
optional in the DT binding, but the driver didn't actually
function if it wasn't provided.

Dave Stevenson (3):
  [media] tc358743: Add enum_mbus_code
  [media] tc358743: Setup default mbus_fmt before registering
  [media] tc358743: Add support for platforms without IRQ line

 drivers/media/i2c/tc358743.c | 59 +++-
 1 file changed, 58 insertions(+), 1 deletion(-)

-- 
2.7.4



Re: [PATCH 4.4 058/103] [media] ttusb2: limit messages to buffer size

2017-06-02 Thread Ben Hutchings
[Dropped cc to stable and LKML.]

On Tue, 2017-05-23 at 22:09 +0200, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Alyssa Milburn 
> 
> commit a12b8ab8c5ff7ccd7b107a564743507c850a441d upstream.
> 
> Otherwise ttusb2_i2c_xfer can read or write beyond the end of static and
> heap buffers.

This function has another problem: it uses per-device mutexes to guard
access to static buffers.  This only works as long as there's a single
device.  It should be using per-device buffers (or a static mutex, but
that's less good).

Ben.

> Signed-off-by: Alyssa Milburn 
> Signed-off-by: Mauro Carvalho Chehab 
> Signed-off-by: Greg Kroah-Hartman 
> 
> ---
>  drivers/media/usb/dvb-usb/ttusb2.c |   19 +++
>  1 file changed, 19 insertions(+)
> 
> --- a/drivers/media/usb/dvb-usb/ttusb2.c
> +++ b/drivers/media/usb/dvb-usb/ttusb2.c
> @@ -78,6 +78,9 @@ static int ttusb2_msg(struct dvb_usb_dev
>   u8 *s, *r = NULL;
>   int ret = 0;
>  
> + if (4 + rlen > 64)
> + return -EIO;
> +
>   s = kzalloc(wlen+4, GFP_KERNEL);
>   if (!s)
>   return -ENOMEM;
> @@ -381,6 +384,22 @@ static int ttusb2_i2c_xfer(struct i2c_ad
>   write_read = i+1 < num && (msg[i+1].flags & I2C_M_RD);
>   read = msg[i].flags & I2C_M_RD;
>  
> + if (3 + msg[i].len > sizeof(obuf)) {
> + err("i2c wr len=%d too high", msg[i].len);
> + break;
> + }
> + if (write_read) {
> + if (3 + msg[i+1].len > sizeof(ibuf)) {
> + err("i2c rd len=%d too high", msg[i+1].len);
> + break;
> + }
> + } else if (read) {
> + if (3 + msg[i].len > sizeof(ibuf)) {
> + err("i2c rd len=%d too high", msg[i].len);
> + break;
> + }
> + }
> +
>   obuf[0] = (msg[i].addr << 1) | (write_read | read);
>   if (read)
>   obuf[1] = 0;
> 
> 
> 

-- 
Ben Hutchings
Software Developer, Codethink Ltd.




Re: [PATCH] ALSA: hda - Fix applying MSI dual-codec mobo quirk

2017-06-02 Thread Sergei Shtylyov

Hello!

On 6/1/2017 11:58 PM, Takashi Iwai wrote:


The previous commit [63691587f7b0: ALSA: hda - Apply dual-codec quirk
for MSI Z270-Gaming mobo] attempted to apply the existing dual-codec


   The standard way of citing a commit is: commit 63691587f7b0 ("ALSA: hda - 
Apply dual-codec quirk for MSI Z270-Gaming mobo"), just like in the Fixes: tag.



quirk for a MSI mobo.  But it turned out that this isn't applied
properly due to the MSI-vendor quirk before this entry.  I overlooked
such two MSI entries just because they were put in the wrong position,
although we have a list ordered by PCI SSID numbers.

This patch fixes it by rearranging the unordered entries.

Fixes: 63691587f7b0 ("ALSA: hda - Apply dual-codec quirk for MSI Z270-Gaming 
mobo")
Reported-by: Rudolf Schmidt 
Signed-off-by: Takashi Iwai 

[...]

MBR, Sergei



[PATCH][media] atomisp: make repool_pgnr and punit_ddr_dvfs_enable static

2017-06-02 Thread Colin King
From: Colin Ian King 

integer repool_pgnr and function punit_ddr_dvfs_enable can be made
static as they do not need to be in global scope.

Cleans up sparse warnings:
 "symbol 'repool_pgnr' was not declared. Should it be static?"
 "symbol 'punit_ddr_dvfs_enable' was not declared. Should it be static?"

Signed-off-by: Colin Ian King 
---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index a0478077a012..a543def739fc 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -56,7 +56,7 @@ module_param(skip_fwload, uint, 0644);
 MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load");
 
 /* set reserved memory pool size in page */
-unsigned int repool_pgnr;
+static unsigned int repool_pgnr;
 module_param(repool_pgnr, uint, 0644);
 MODULE_PARM_DESC(repool_pgnr,
"Set the reserved memory pool size in page (default:0)");
@@ -384,7 +384,7 @@ static int atomisp_mrfld_pre_power_down(struct 
atomisp_device *isp)
  * WA for DDR DVFS enable/disable
  * By default, ISP will force DDR DVFS 1600MHz before disable DVFS
  */
-void punit_ddr_dvfs_enable(bool enable)
+static void punit_ddr_dvfs_enable(bool enable)
 {
int reg = intel_mid_msgbus_read32(PUNIT_PORT, MRFLD_ISPSSDVFS);
int door_bell = 1 << 8;
-- 
2.11.0



Re: [PATCH v7] dw9714: Initial driver for dw9714 VCM

2017-06-02 Thread Sakari Ailus
On Thu, Jun 01, 2017 at 11:06:51PM -0700, Rajmohan Mani wrote:
> +static int dw9714_probe(struct i2c_client *client,
> + const struct i2c_device_id *devid)
> +{
> + struct dw9714_device *dw9714_dev;
> + int rval;
> +
> + dw9714_dev = devm_kzalloc(>dev, sizeof(*dw9714_dev),
> +   GFP_KERNEL);
> + if (dw9714_dev == NULL)
> + return -ENOMEM;
> +
> + dw9714_dev->client = client;
> +
> + v4l2_i2c_subdev_init(_dev->sd, client, _ops);
> + dw9714_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + dw9714_dev->sd.internal_ops = _int_ops;
> +
> + rval = dw9714_init_controls(dw9714_dev);
> + if (rval)
> + goto err_cleanup;
> +
> + rval = media_entity_pads_init(_dev->sd.entity, 0, NULL);
> + if (rval < 0)
> + goto err_cleanup;
> +
> + dw9714_dev->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> + rval = v4l2_async_register_subdev(_dev->sd);
> + if (rval < 0)
> + goto err_cleanup;
> +

You need pm_runtime_set_active() here.

> + pm_runtime_enable(>dev);
> +
> + return 0;
> +
> +err_cleanup:
> + dw9714_subdev_cleanup(dw9714_dev);
> + dev_err(>dev, "Probe failed: %d\n", rval);
> + return rval;
> +}

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


Re: [PATCH v7 1/1] [media] i2c: add support for OV13858 sensor

2017-06-02 Thread Sakari Ailus
Hi Hyungwoo,

On Thu, Jun 01, 2017 at 03:45:16PM -0700, Hyungwoo Yang wrote:
...
> +static int ov13858_probe(struct i2c_client *client,
> +  const struct i2c_device_id *devid)
> +{
> + struct ov13858 *ov13858;
> + int ret;
> +
> + ov13858 = devm_kzalloc(>dev, sizeof(*ov13858), GFP_KERNEL);
> + if (!ov13858)
> + return -ENOMEM;
> +
> + /* Initialize subdev */
> + v4l2_i2c_subdev_init(>sd, client, _subdev_ops);
> +
> + /*
> +  * Enable runtime PM.
> +  * The sensor is already powered on ACPI domain PM
> +  */
> + pm_runtime_get_noresume(>dev);
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);

As you already have in comments, the device is already powered on in an ACPI
based system. pm_runtime_get_noresume() prevents powering the device off
during probe after runtime PM is enabled.

It'd be better to also move the calls to pm_runtime_set_active() and
pm_runtime_enable() just before returning 0. This way you can get rid of
extra error handling you'd otherwise need to do: once you call
pm_runtime_get_noresume(), you need to call pm_runtime_put() or one of its
variants as well.

> +
> + /* Check module identity */
> + ret = ov13858_identify_module(ov13858);
> + if (ret) {
> + dev_err(>dev, "failed to find sensor: %d\n", ret);
> + return ret;
> + }
> +
> + /* Set default mode to max resolution */
> + ov13858->cur_mode = _modes[0];
> +
> + ret = ov13858_init_controls(ov13858);
> + if (ret)
> + return ret;
> +
> + /* Initialize subdev */
> + ov13858->sd.internal_ops = _internal_ops;
> + ov13858->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + ov13858->sd.entity.ops = _subdev_entity_ops;
> + ov13858->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> + /* Initialize source pad */
> + ov13858->pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(>sd.entity, 1, >pad);
> + if (ret) {
> + dev_err(>dev, "%s failed:%d\n", __func__, ret);
> + goto error_handler_free;
> + }
> +
> + ret = v4l2_async_register_subdev(>sd);
> + if (ret < 0)
> + goto error_media_entity;
> +
> + /* Turn off */
> + pm_runtime_put(>dev);
> +
> + return 0;
> +
> +error_media_entity:
> + media_entity_cleanup(>sd.entity);
> +
> +error_handler_free:
> + ov13858_free_controls(ov13858);
> + dev_err(>dev, "%s failed:%d\n", __func__, ret);
> +
> + return ret;
> +}

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


[PATCH] cec: add cec_transmit_attempt_done helper function

2017-06-02 Thread Hans Verkuil

A simpler variant of cec_transmit_done to be used where the HW does
just a single attempt at a transmit. So if the status indicates an
error, then the corresponding error count will always be 1 and this
function figures that out based on the status argument.

Signed-off-by: Hans Verkuil 
---
Russell, this should simplify things for you.
---
 Documentation/media/kapi/cec-core.rst | 10 ++
 drivers/media/cec/cec-adap.c  | 26 ++
 include/media/cec.h   |  6 ++
 3 files changed, 42 insertions(+)

diff --git a/Documentation/media/kapi/cec-core.rst 
b/Documentation/media/kapi/cec-core.rst
index 7a04c5386dc8..25728545e4ec 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -194,6 +194,11 @@ When a transmit finished (successfully or otherwise):
void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 
arb_lost_cnt,
   u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);

+or:
+
+.. c:function::
+   void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status);
+
 The status can be one of:

 CEC_TX_STATUS_OK:
@@ -231,6 +236,11 @@ to 1, if the hardware does support retry then either set 
these counters to
 0 if the hardware provides no feedback of which errors occurred and how many
 times, or fill in the correct values as reported by the hardware.

+The cec_transmit_attempt_done() function is a helper for cases where the
+hardware never retries, so the transmit was always for just a single
+attempt. It will call cec_transmit_done() in turn, filling in 1 for the
+count argument corresponding to the status. Or all 0 if the status was OK.
+
 When a CEC message was received:

 .. c:function::
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index f5fe01c9da8a..0f4621cd8748 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -544,6 +544,32 @@ void cec_transmit_done(struct cec_adapter *adap, u8 
status, u8 arb_lost_cnt,
 }
 EXPORT_SYMBOL_GPL(cec_transmit_done);

+void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status)
+{
+   switch (status) {
+   case CEC_TX_STATUS_OK:
+   cec_transmit_done(adap, status, 0, 0, 0, 0);
+   return;
+   case CEC_TX_STATUS_ARB_LOST:
+   cec_transmit_done(adap, status, 1, 0, 0, 0);
+   return;
+   case CEC_TX_STATUS_NACK:
+   cec_transmit_done(adap, status, 0, 1, 0, 0);
+   return;
+   case CEC_TX_STATUS_LOW_DRIVE:
+   cec_transmit_done(adap, status, 0, 0, 1, 0);
+   return;
+   case CEC_TX_STATUS_ERROR:
+   cec_transmit_done(adap, status, 0, 0, 0, 1);
+   return;
+   default:
+   /* Should never happen */
+   WARN(1, "cec-%s: invalid status 0x%02x\n", adap->name, status);
+   return;
+   }
+}
+EXPORT_SYMBOL_GPL(cec_transmit_attempt_done);
+
 /*
  * Called when waiting for a reply times out.
  */
diff --git a/include/media/cec.h b/include/media/cec.h
index b8eb895731d5..5582e1cac1b9 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -223,6 +223,12 @@ int cec_transmit_msg(struct cec_adapter *adap, struct 
cec_msg *msg,
 /* Called by the adapter */
 void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
   u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
+/*
+ * Simplified version of cec_transmit_done for hardware that doesn't retry
+ * failed transmits. So this is was always just one attempt in which case
+ * the status is sufficient.
+ */
+void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status);
 void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);

 /**
--
2.11.0



Re: [PATCH v2 00/27] Revised full patchset for PCM in-kernel copy support

2017-06-02 Thread Takashi Sakamoto

On Jul 02 2017 05:58, Takashi Iwai wrote:

Hi,

this is a full patchset of what I sent previously, containing the all
changes instead of the snippet.  The main purpose of this patchset is
to eliminate the remaining usages of set_fs().  They are basically
used for in-kernel PCM data transfer, and this patch provides the new
API functions and replaces the hackish set_fs() calls with them.

Unlike the first patchset with the unified copy_silence ops, this adds
a new copy_kernel ops instead.  At the same time, copy/silence are
changed to receive the position and size in bytes instead of frames.
This allows us to simplify the PCM core code.  As a result, a good
amount of code could be removed from pcm_lib.c.

The difference from the previous patchset is that this is a full
patchset, i.e. all relevant drivers have been covered, and also some
small issues have been addressed, in addition, the documentation
update is provided, too.

I'm Cc'ing the media and the USB people since it touches solo6x10 and
usb-gadget drivers.

The previous ACK was dropped as each patch was rewritten again.  Sorry
for the doubly patch-review labours.


thanks,

Takashi

===

Takashi Iwai (26):
   ALSA: pcm: Introduce copy_user, copy_kernel and fill_silence ops


Below commits look good to me.


   ALSA: dummy: Convert to new PCM copy ops
   ALSA: es1938: Convert to the new PCM copy ops
   ALSA: nm256: Convert to new PCM copy ops
   ALSA: korg1212: Convert to the new PCM ops
   ALSA: rme32: Convert to the new PCM copy ops
   ALSA: rme96: Convert to the new PCM ops
   ALSA: rme9652: Convert to the new PCM ops
   ALSA: hdsp: Convert to the new PCM ops
   ALSA: gus: Convert to the new PCM ops
   ALSA: sb: Convert to the new PCM ops
   ALSA: sh: Convert to the new PCM ops
   ASoC: blackfin: Convert to the new PCM ops
   [media] solo6x10: Convert to the new PCM ops
   ALSA: pcm: Drop the old copy and silence ops
   ALSA: pcm: Check PCM state by a common helper function
   ALSA: pcm: Shuffle codes
   ALSA: pcm: Call directly the common read/write helpers
   ALSA: pcm: More unification of PCM transfer codes
   ALSA: pcm: Unify read/write loop
   ALSA: pcm: Simplify snd_pcm_playback_silence()
   ALSA: pcm: Direct in-kernel read/write support
   usb: gadget: u_uac1: Kill set_fs() usage
   ALSA: pcm: Kill set_fs() in PCM OSS layer
   ALSA: pcm: Build OSS writev/readv helpers conditionally
   ALSA: doc: Update copy_user, copy_kernel and fill_silence PCM ops


Reviewed-by: Takashi Sakamoto 

I did easy test with snd-hda-intel/snd-fireworks in below conditions. 
Things work well:

1. ALSA application (aplay/arecord) for '__user *' <-> '__kernel *' copying.
2. loaded snd-oss-pcm and an Open Sound System application 
(ossplay/ossrecord), for '__kernel *' <-> '__kernel *' copying.


I have no devices for which drivers have the .copy_user, .copy_kernel 
and .fill_silence, and all of my attemps to work with OTG chip for v4.12 
fails (sigh...). My test is not comprehensive at all, however the 
patchset is programmed with handler-oriented ways and in this point I 
think snd-pcm works as expected.


I note that patch 19 brings merge conflict to current HEAD 
ee6f4cde4f74("Merge branch 'for-linus'"), due to my patch, 
2c4842d3b6b3("ALSA: pcm: add local header file for snd-pcm module"). I 
should have postponed it.. For the above test, I handy modifies the 
history with little affections for my reviewing.



  .../sound/kernel-api/writing-an-alsa-driver.rst| 111 ++--
  drivers/media/pci/solo6x10/solo6x10-g723.c |  32 +-
  drivers/usb/gadget/function/u_uac1.c   |   7 +-
  include/sound/pcm.h|  80 ++-
  sound/core/oss/io.c|   4 +-
  sound/core/oss/pcm_oss.c   |  81 +--
  sound/core/oss/pcm_plugin.h|   6 +-
  sound/core/pcm_lib.c   | 564 -
  sound/drivers/dummy.c  |  20 +-
  sound/isa/gus/gus_pcm.c|  97 ++--
  sound/isa/sb/emu8000_pcm.c | 190 ---
  sound/pci/es1938.c |  33 +-
  sound/pci/korg1212/korg1212.c  | 112 ++--
  sound/pci/nm256/nm256.c|  57 ++-
  sound/pci/rme32.c  |  65 ++-
  sound/pci/rme96.c  |  70 ++-
  sound/pci/rme9652/hdsp.c   |  67 ++-
  sound/pci/rme9652/rme9652.c|  71 ++-
  sound/sh/sh_dac_audio.c|  54 +-
  sound/soc/blackfin/bf5xx-ac97-pcm.c|  27 +-
  sound/soc/blackfin/bf5xx-i2s-pcm.c |  36 +-
  sound/soc/soc-pcm.c|   5 +-
  22 files changed, 977 insertions(+), 812 deletions(-)



Regards

Takashi Sakamoto


regmap for i2c pages

2017-06-02 Thread Tim Harvey
I'm working on a driver for an i2c based media controller device that
uses pages. By that I mean there are several pages of 8bit registers
and a page-select register that holds the current page. Multiple
accesses to the same page can occur without writing to this page
register but if you want to access another page you need to update it.

I believe this is a very common i2c register mechanism but I'm not
clear what the best way to use i2c regmap for this is. I'm reading
that regmap 'handles register pages' but I'm not clear if that's the
same thing I'm looking for. If so, are there any examples of this? I
see several i2c drivers that reference pages but it looks to me that
each page is a different i2c slave address which is something
different.

Regards,

Tim


[PATCH v7] dw9714: Initial driver for dw9714 VCM

2017-06-02 Thread Rajmohan Mani
DW9714 is a 10 bit DAC, designed for linear
control of voice coil motor.

This driver creates a V4L2 subdevice and
provides control to set the desired focus.

Signed-off-by: Rajmohan Mani 
---
Changes in v7:
- Removed DW9714 ACPI hwid from ACPI match table, until
the correct ACPI id is available
- Added details in an error message
Changes in v6:
- Addressed review comments from Sakari on v5 patch
Changes in v5:
- Addressed review comments from Tomasz, Sakari and Sylwester on v4
of this patch
Changes in v4:
- Addressed review comments from Tomasz
Changes in v3:
- Addressed most of the review comments from Sakari
  on v1 of this patch
Changes in v2:
- Addressed review comments from Hans Verkuil
- Fixed a debug message typo
---
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9714.c | 290 +
 3 files changed, 301 insertions(+)
 create mode 100644 drivers/media/i2c/dw9714.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fd181c9..188ab15 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -300,6 +300,16 @@ config VIDEO_AD5820
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).
 
+config VIDEO_DW9714
+   tristate "DW9714 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a driver for the DW9714 camera lens voice coil.
+ DW9714 is a 10 bit DAC with 120mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_SAA7110
tristate "Philips SAA7110 video decoder"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 62323ec..987bd1f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
new file mode 100644
index 000..8f19776
--- /dev/null
+++ b/drivers/media/i2c/dw9714.c
@@ -0,0 +1,290 @@
+/*
+ * Copyright (c) 2015--2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DW9714_NAME"dw9714"
+#define DW9714_MAX_FOCUS_POS   1023
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9714_CTRL_STEPS  16
+#define DW9714_CTRL_DELAY_US   1000
+/*
+ * S[3:2] = 0x00, codes per step for "Linear Slope Control"
+ * S[1:0] = 0x00, step period
+ */
+#define DW9714_DEFAULT_S 0x0
+#define DW9714_VAL(data, s) ((data) << 4 | (s))
+
+/* dw9714 device structure */
+struct dw9714_device {
+   struct i2c_client *client;
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   u16 current_val;
+};
+
+static inline struct dw9714_device *to_dw9714_vcm(struct v4l2_ctrl *ctrl)
+{
+   return container_of(ctrl->handler, struct dw9714_device, ctrls_vcm);
+}
+
+static inline struct dw9714_device *sd_to_dw9714_vcm(struct v4l2_subdev 
*subdev)
+{
+   return container_of(subdev, struct dw9714_device, sd);
+}
+
+static int dw9714_i2c_write(struct i2c_client *client, u16 data)
+{
+   int ret;
+   u16 val = cpu_to_be16(data);
+
+   ret = i2c_master_send(client, (const char *), sizeof(val));
+   if (ret != sizeof(val)) {
+   dev_err(>dev, "I2C write fail\n");
+   return -EIO;
+   }
+   return 0;
+}
+
+static int dw9714_t_focus_vcm(struct dw9714_device *dw9714_dev, u16 val)
+{
+   struct i2c_client *client = dw9714_dev->client;
+
+   dw9714_dev->current_val = val;
+
+   return dw9714_i2c_write(client, DW9714_VAL(val, DW9714_DEFAULT_S));
+}
+
+static int dw9714_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct dw9714_device