[PATCH v5 2/2] media: ak7375: Add ak7375 lens voice coil driver
From: Bingbu Cao
Add a v4l2 sub-device driver for the ak7375 lens voice coil.
This is a voice coil module using the i2c bus to control the
focus position.
ak7375 can write multiple bytes of data at a time. If more
data is received instead of the stop condition after receiving
one byte of data, the address inside the chip is automatically
incremented and the data is written into the next address.
The ak7375 can control the position with 12 bits value and
consists of two 8 bit registers show as below:
register 0x00(AK7375_REG_POSITION):
+---+---+---+---+---+---+---+---+
|D11|D10|D09|D08|D07|D06|D05|D04|
+---+---+---+---+---+---+---+---+
register 0x01:
+---+---+---+---+---+---+---+---+
|D03|D02|D01|D00|---|---|---|---|
+---+---+---+---+---+---+---+---+
This driver support :
- set ak7375 to standby mode once suspend and
turn it back to active if resume
- set the position via V4L2_CID_FOCUS_ABSOLUTE ctrl
Signed-off-by: Tianshu Qiu
Signed-off-by: Bingbu Cao
Reviewed-by: Sakari Ailus
---
Changes from v1 -> v3:
- correct i2c write
- add media_entity_pads_init() into probe
- move the MAINTAINERs change into dt-binding change
- correct the compatible stringa
Changes since v3:
- add active flag to indicate the mode
- refine some coding style
This patch is based on Sakari's media-tree git:
https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.18-5
---
---
drivers/media/i2c/Kconfig | 10 ++
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/ak7375.c | 293 +
3 files changed, 304 insertions(+)
create mode 100644 drivers/media/i2c/ak7375.c
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452fe98df..ff3cb5afb0e1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -326,6 +326,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_AK7375
+ tristate "AK7375 lens voice coil support"
+ depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on VIDEO_V4L2_SUBDEV_API
+ help
+ This is a driver for the AK7375 camera lens voice coil.
+ AK7375 is a 12 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_DW9714
tristate "DW9714 lens voice coil support"
depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57cd3b3..05b97e319ea9 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,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_AK7375) += ak7375.o
obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
new file mode 100644
index ..9f039513d57f
--- /dev/null
+++ b/drivers/media/i2c/ak7375.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define AK7375_MAX_FOCUS_POS 4095
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define AK7375_FOCUS_STEPS 1
+/*
+ * 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 AK7375_CTRL_STEPS 64
+#define AK7375_CTRL_DELAY_US 1000
+
+#define AK7375_REG_POSITION0x0
+#define AK7375_REG_CONT0x2
+#define AK7375_MODE_ACTIVE 0x0
+#define AK7375_MODE_STANDBY0x40
+
+/* ak7375 device structure */
+struct ak7375_device {
+ struct v4l2_ctrl_handler ctrls_vcm;
+ struct v4l2_subdev sd;
+ struct v4l2_ctrl *focus;
+ /* active or standby mode */
+ bool active;
+};
+
+static inline struct ak7375_device *to_ak7375_vcm(struct v4l2_ctrl *ctrl)
+{
+ return container_of(ctrl->handler, struct ak7375_device, ctrls_vcm);
+}
+
+static inline struct ak7375_device *sd_to_ak7375_vcm(struct v4l2_subdev
*subdev)
+{
+ return container_of(subdev, struct ak7375_device, sd);
+}
+
+static int ak7375_i2c_write(struct ak7375_device *ak7375,
+ u8 addr, u16 data, u8 size)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(>sd);
+ u8 buf[3];
+ int ret;
+
+ if (size != 1 && size != 2)
+ return -EINVAL;
+ buf[0] = addr;
+ buf[size] = data & 0xff;
+ if
[PATCH v5 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens
From: Bingbu Cao Add device tree bindings for AKM ak7375 voice coil lens driver. This chip is used to drive a lens in a camera module. Signed-off-by: Tianshu Qiu Signed-off-by: Bingbu Cao Reviewed-by: Rob Herring --- Changes since v1: - add the MAINTAINERS change - correct the vendor prefix from akm to asahi-kasei This patch is based on Sakari's media-tree git: https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.18-5 --- --- Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 MAINTAINERS| 8 2 files changed, 16 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt b/Documentation/devicetree/bindings/media/i2c/ak7375.txt new file mode 100644 index ..aa3e24b41241 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ak7375.txt @@ -0,0 +1,8 @@ +Asahi Kasei Microdevices AK7375 voice coil lens driver + +AK7375 is a camera voice coil lens. + +Mandatory properties: + +- compatible: "asahi-kasei,ak7375" +- reg: I2C slave address diff --git a/MAINTAINERS b/MAINTAINERS index ea362219c4aa..ad68d75abc84 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2258,6 +2258,14 @@ L: linux-l...@vger.kernel.org S: Maintained F: drivers/leds/leds-as3645a.c +ASAHI KASEI AK7375 LENS VOICE COIL DRIVER +M: Tianshu Qiu +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/ak7375.c +F: Documentation/devicetree/bindings/media/i2c/ak7375.txt + ASAHI KASEI AK8974 DRIVER M: Linus Walleij L: linux-...@vger.kernel.org -- 1.9.1
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Jun 19 05:00:17 CEST 2018 media-tree git hash:f2809d20b9250c675fca8268a0f6274277cca7ff media_build git hash: 26d102795c91f8593a4f74f96b955f9a8b81dbc3 v4l-utils git hash: c3b46c2c53d7d815a53c902cfb2ddd96c3732c5b gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.42-i686: OK linux-4.14.42-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16.8-i686: OK linux-4.16.8-x86_64: OK linux-4.17.2-i686: OK linux-4.17.2-x86_64: OK linux-4.18-rc1-i686: ERRORS linux-4.18-rc1-x86_64: ERRORS apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [RFC PATCH v2] media: i2c: add SCCB helpers
> > I'd prefer this file to be in the i2c realm. Maybe
> > 'include/linux/i2c-sccb.h" or something. I will come back to this.
>
> And while at it, I think we also need a .c file, the functions (and
> especially
> sccb_read_byte()) should not be static inline.
Before we discuss this, we should make sure that the read-function is
complete and then we can decide further. I found some old notes based on
our previous discussions about SCCB and refactoring its access. You
mentioned there is HW supporting SCCB natively. Also, we found a device
where an SCCB device is connected to an SMBUS controller (yuck!). So,
given that, I'd think the read routine should look like this (in pseudo
code):
bool is_sccb_available(adap)
{
u32 needed_funcs = I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA;
/*
* sccb_xfer not needed yet, since there is no driver support currently.
* Just showing how it should be done if we ever need it.
*/
if (adap->algo->sccb_xfer)
return true;
if (i2c_get_functionality(adap) & needed_funcs == needed_funcs)
return true;
return false;
}
sccb_get_byte()
{
if (adap->algo->sccb_xfer)
adap->algo->sccb_xfer(...)
/*
* Prereq: __i2c_smbus_xfer needs to be created!
*/
i2c_lock_adapter(SEGMENT);
__i2c_smbus_xfer(..., SEND_BYTE, ...)
__i2c_smbus_xfer(..., GET_BYTE, ...)
i2c_unlock_adapter(SEGMENT)
}
sccb_write_byte()
{
return i2c_smbus_write_byte_data(...)
}
If I haven't overlooked something, this should make SCCB possible on I2C
controllers and SMBus controllers. We honor the requirement of not
having repeated start anywhere. As such, we might get even rid of the
I2C_M_STOP flag (for in-kernel users, but we sadly export it to
userspace).
About the IGNORE_NAK flag, I think this still should work where
supported as it is passed along indirectly with I2C_CLIENT_SCCB.
However, this flag handling with SCCB is really a mess and needs an
overhaul. This can be a second step, however. Most devices don't really
need it, do they?
Any further comments? Anything I missed?
Kind regards,
Wolfram
signature.asc
Description: PGP signature
RE: [PATCH v6 04/12] intel-ipu3: Implement DMA mapping functions
Hi, Tomasz,
Thanks for the review.
> -Original Message-
> From: Tomasz Figa [mailto:tf...@chromium.org]
> Sent: Monday, June 18, 2018 12:09 AM
> To: Zhi, Yong
> Cc: Linux Media Mailing List ; Sakari Ailus
> ; Mani, Rajmohan
> ; Toivonen, Tuukka
> ; Hu, Jerry W ; Zheng,
> Jian Xu
> Subject: Re: [PATCH v6 04/12] intel-ipu3: Implement DMA mapping
> functions
>
> On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
> >
> > From: Tomasz Figa
> >
> > This driver uses IOVA space for buffer mapping through IPU3 MMU to
> > transfer data between imaging pipelines and system DDR.
> >
> > Signed-off-by: Tomasz Figa
> > Signed-off-by: Yong Zhi
> > ---
> > drivers/media/pci/intel/ipu3/ipu3-css-pool.h | 36
> > drivers/media/pci/intel/ipu3/ipu3-dmamap.c | 280
> +++
> > drivers/media/pci/intel/ipu3/ipu3-dmamap.h | 22 +++
> > drivers/media/pci/intel/ipu3/ipu3.h | 151 +++
> > 4 files changed, 489 insertions(+)
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3.h
> >
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> > b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> > new file mode 100644
> > index ..4b22e0856232
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (C) 2018 Intel Corporation */
> > +
> > +#ifndef __IPU3_UTIL_H
> > +#define __IPU3_UTIL_H
> > +
> > +struct device;
> > +
> > +#define IPU3_CSS_POOL_SIZE 4
> > +
> > +struct ipu3_css_map {
> > + size_t size;
> > + void *vaddr;
> > + dma_addr_t daddr;
> > + struct vm_struct *vma;
> > +};
> > +
> > +struct ipu3_css_pool {
> > + struct {
> > + struct ipu3_css_map param;
> > + long framenum;
> > + } entry[IPU3_CSS_POOL_SIZE];
> > + unsigned int last; /* Latest entry */
>
> It's not clear what "Latest entry" means here. Since these structs are a part
> of the interface exposed by this header, could you write proper kerneldoc
> comments for all fields in both of them?
>
Sure.
> > +};
> > +
> > +int ipu3_css_dma_buffer_resize(struct device *dev, struct ipu3_css_map
> *map,
> > + size_t size); void
> > +ipu3_css_pool_cleanup(struct device *dev, struct ipu3_css_pool
> > +*pool); int ipu3_css_pool_init(struct device *dev, struct ipu3_css_pool
> *pool,
> > + size_t size);
> > +int ipu3_css_pool_get(struct ipu3_css_pool *pool, long framenum);
> > +void ipu3_css_pool_put(struct ipu3_css_pool *pool); const struct
> > +ipu3_css_map *ipu3_css_pool_last(struct ipu3_css_pool *pool,
> > + unsigned int last);
> > +
> > +#endif
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > new file mode 100644
> > index ..b2bc5d00debc
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > @@ -0,0 +1,280 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Intel Corporation
> > + * Copyright (C) 2018 Google, Inc.
>
> Would you mind changing as below?
>
> Copyright 2018 Google LLC.
>
Ack.
> > + *
> > + * Author: Tomasz Figa
> > + * Author: Yong Zhi */
> > +
> > +#include
> > +
> > +#include "ipu3.h"
> > +#include "ipu3-css-pool.h"
> > +#include "ipu3-mmu.h"
> > +
> > +/*
> > + * Free a buffer allocated by ipu3_dmamap_alloc_buffer() */ static
> > +void ipu3_dmamap_free_buffer(struct page **pages,
> > + size_t size) {
> > + int count = size >> PAGE_SHIFT;
> > +
> > + while (count--)
> > + __free_page(pages[count]);
> > + kvfree(pages);
> > +}
> > +
> > +/*
> > + * Based on the implementation of __iommu_dma_alloc_pages()
> > + * defined in drivers/iommu/dma-iommu.c */ static struct page
> > +**ipu3_dmamap_alloc_buffer(size_t size,
> > + unsigned long order_mask,
> > + gfp_t gfp) {
> > + struct page **pages;
> > + unsigned int i = 0, count = size >> PAGE_SHIFT;
> > + const gfp_t high_order_gfp = __GFP_NOWARN | __GFP_NORETRY;
> > +
> > + /* Allocate mem for array of page ptrs */
> > + pages = kvmalloc_array(count, sizeof(struct page *),
> > + GFP_KERNEL);
>
> sizeof(*pages) to ensure that the right type is used regardless of
> declaration.
>
Ack.
> > +
>
> No need for this blank line.
>
> > + if (!pages)
> > + return NULL;
> [snip]
> > +/**
> > + * ipu3_dmamap_alloc - allocate and map a buffer into KVA
> > + * @dev: struct device pointer
> > + * @map: struct to store mapping variables
RE: [PATCH v6 03/12] intel-ipu3: mmu: Implement driver
Hi, Tomasz,
Thanks for the code review.
> -Original Message-
> From: Tomasz Figa [mailto:tf...@chromium.org]
> Sent: Sunday, June 17, 2018 11:46 PM
> To: Zhi, Yong
> Cc: Linux Media Mailing List ; Sakari Ailus
> ; Mani, Rajmohan
> ; Toivonen, Tuukka
> ; Hu, Jerry W ; Zheng,
> Jian Xu
> Subject: Re: [PATCH v6 03/12] intel-ipu3: mmu: Implement driver
>
> On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
> >
> > From: Tomasz Figa
> >
> > This driver translates IO virtual address to physical address based on
> > two levels page tables.
> >
> > Signed-off-by: Tomasz Figa
> > Signed-off-by: Yong Zhi
> > ---
> > drivers/media/pci/intel/ipu3/ipu3-mmu.c | 560
> >
> > drivers/media/pci/intel/ipu3/ipu3-mmu.h | 28 ++
> > 2 files changed, 588 insertions(+)
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h
> >
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c
> > b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
> > new file mode 100644
> > index ..a4b3e1680bbb
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
> > @@ -0,0 +1,560 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Intel Corporation.
> > + * Copyright (C) 2018 Google, Inc.
>
> I followed wrong guide when adding this one. Could you fix it up to the
> following?
>
> Copyright 2018 Google LLC.
>
Sure, will do.
> [snip]
> > +/**
> > + * ipu3_mmu_exit() - clean up IPU3 MMU block
> > + * @mmu: IPU3 MMU private data
> > + */
> > +void ipu3_mmu_exit(struct ipu3_mmu_info *info) {
> > + struct ipu3_mmu *mmu = to_ipu3_mmu(info);
> > +
> > + /* We are going to free our page tables, no more memory access. */
> > + ipu3_mmu_set_halt(mmu, true);
> > + ipu3_mmu_tlb_invalidate(mmu);
> > +
> > + ipu3_mmu_free_page_table(mmu->l1pt);
> > + vfree(mmu->l2pts);
> > + ipu3_mmu_free_page_table(mmu->dummy_l2pt);
> > + kfree(mmu->dummy_page);
>
> Should be free_page(). (Might be already included in your tree as per
> https://chromium-
> review.googlesource.com/c/chromiumos/third_party/kernel/+/1084522)
>
Yes, will add above fix to next upstream version.
> > + kfree(mmu);
> > +}
> > +
> > +void ipu3_mmu_suspend(struct ipu3_mmu_info *info) {
> > + struct ipu3_mmu *mmu = to_ipu3_mmu(info);
> > +
> > + ipu3_mmu_set_halt(mmu, true);
> > +}
> > +
> > +void ipu3_mmu_resume(struct ipu3_mmu_info *info) {
> > + struct ipu3_mmu *mmu = to_ipu3_mmu(info);
> > + u32 pteval;
> > +
> > + ipu3_mmu_set_halt(mmu, true);
> > +
> > + pteval = IPU3_ADDR2PTE(virt_to_phys(mmu->l1pt));
> > + writel(pteval, mmu->base + REG_L1_PHYS);
> > +
> > + ipu3_mmu_tlb_invalidate(mmu);
> > + ipu3_mmu_set_halt(mmu, false); }
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.h
> > b/drivers/media/pci/intel/ipu3/ipu3-mmu.h
> > new file mode 100644
> > index ..4976187c18f6
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (C) 2018 Intel Corporation */
> > +/* Copyright (C) 2018 Google, Inc. */
> > +
> > +#ifndef __IPU3_MMU_H
> > +#define __IPU3_MMU_H
> > +
> > +struct ipu3_mmu_info {
> > + dma_addr_t aperture_start; /* First address that can be mapped
> */
> > + dma_addr_t aperture_end; /* Last address that can be mapped
> */
> > + unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
>
> If documenting the fields, why not use a kerneldoc comment above the
> struct instead?
>
Ack.
> Best regards,
> Tomasz
[PATCH v2 0/3] OV5640 hflip, vflip and module orientation support
This patch serie relates to flip and mirror at sensor level through registers "TIMING TC REG20/REG21". The first commit implements HFLIP and VFLIP V4L2 controls allowing V4L2 client to change horizontal and vertical flip before or during streaming. The second & third commit allows to inform driver of the physical orientation of the sensor module through devicetree "rotation" optional properties as defined by Sakari in media/video-interfaces.txt: https://www.mail-archive.com/linux-media@vger.kernel.org/msg132345.html === = history = === version 2: - Fix remarks from Rob Hering on devicetree part (new commit for binding and reference to video-interfaces.txt common bindings): https://www.spinics.net/lists/linux-media/msg136048.html Hugues Fruchet (3): media: ov5640: add HFLIP/VFLIP controls support dt-bindings: ov5640: Add "rotation" property media: ov5640: add support of module orientation .../devicetree/bindings/media/i2c/ov5640.txt | 5 + drivers/media/i2c/ov5640.c | 127 ++--- 2 files changed, 114 insertions(+), 18 deletions(-) -- 1.9.1
[PATCH v2 1/3] media: ov5640: add HFLIP/VFLIP controls support
Add HFLIP/VFLIP controls support by setting registers REG21/REG20.
Useless values in hardcoded mode sequences are removed and
remaining binning values are now set after mode sequence being set.
Note that due to BSI (Back Side Illuminated) technology, image capture
is physically mirrored, mirror logic is so inversed in REG21 register
to cancel this effect.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 103 +
1 file changed, 85 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f6e40cc..41039e5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -64,6 +64,7 @@
#define OV5640_REG_TIMING_DVPVO0x380a
#define OV5640_REG_TIMING_HTS 0x380c
#define OV5640_REG_TIMING_VTS 0x380e
+#define OV5640_REG_TIMING_TC_REG20 0x3820
#define OV5640_REG_TIMING_TC_REG21 0x3821
#define OV5640_REG_AEC_CTRL00 0x3a00
#define OV5640_REG_AEC_B50_STEP0x3a08
@@ -199,6 +200,8 @@ struct ov5640_ctrls {
struct v4l2_ctrl *contrast;
struct v4l2_ctrl *hue;
struct v4l2_ctrl *test_pattern;
+ struct v4l2_ctrl *hflip;
+ struct v4l2_ctrl *vflip;
};
struct ov5640_dev {
@@ -341,7 +344,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -360,7 +363,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -379,7 +382,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -399,7 +402,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -418,7 +421,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -437,7 +440,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803,
[PATCH v2 3/3] media: ov5640: add support of module orientation
Add support of module being physically mounted upside down.
In this case, mirror and flip are enabled to fix captured images
orientation.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 41039e5..5529b14 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -215,6 +215,7 @@ struct ov5640_dev {
struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
struct gpio_desc *reset_gpio;
struct gpio_desc *pwdn_gpio;
+ bool upside_down;
/* lock to protect all members below */
struct mutex lock;
@@ -,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev
*sensor, int value)
static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
{
/*
+* If sensor is mounted upside down, mirror logic is inversed.
+*
* Sensor is a BSI (Back Side Illuminated) one,
* so image captured is physically mirrored.
* This is why mirror logic is inversed in
@@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev
*sensor, int value)
*/
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
BIT(2) | BIT(1),
- (!value) ? (BIT(2) | BIT(1)) : 0);
+ (!(value ^ sensor->upside_down)) ?
+ (BIT(2) | BIT(1)) : 0);
}
static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
{
+ /* If sensor is mounted upside down, flip logic is inversed */
+
/*
* TIMING TC REG20:
* - [2]: ISP vflip
@@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev
*sensor, int value)
*/
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
BIT(2) | BIT(1),
- value ? (BIT(2) | BIT(1)) : 0);
+ (value ^ sensor->upside_down) ?
+ (BIT(2) | BIT(1)) : 0);
}
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
struct v4l2_mbus_framefmt *fmt;
+ u32 rotation;
int ret;
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
sensor->ae_target = 52;
+ /* optional indication of physical rotation of sensor */
+ ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
+ "rotation", );
+ if (!ret) {
+ switch (rotation) {
+ case 180:
+ sensor->upside_down = true;
+ /* fall through */
+ case 0:
+ break;
+ default:
+ dev_warn(dev, "%u degrees rotation is not supported,
ignoring...\n",
+rotation);
+ }
+ }
+
endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(>dev),
NULL);
if (!endpoint) {
--
1.9.1
[PATCH v2 2/3] dt-bindings: ov5640: Add "rotation" property
Add the rotation property to the list of optional properties
for the OV5640 camera sensor.
Signed-off-by: Hugues Fruchet
---
Documentation/devicetree/bindings/media/i2c/ov5640.txt | 5 +
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
index 8e36da0..c97c2f2 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
@@ -13,6 +13,10 @@ Optional Properties:
This is an active low signal to the OV5640.
- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
if any. This is an active high signal to the OV5640.
+- rotation: as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt,
+ valid values are 0 (sensor mounted upright) and 180 (sensor
+ mounted upside down).
The device node must contain one 'port' child node for its digital output
video port, in accordance with the video interface bindings defined in
@@ -51,6 +55,7 @@ Examples:
DVDD-supply = <_reg>; /* 1.5v */
powerdown-gpios = < 19 GPIO_ACTIVE_HIGH>;
reset-gpios = < 20 GPIO_ACTIVE_LOW>;
+ rotation = <180>;
port {
/* MIPI CSI-2 bus endpoint */
--
1.9.1
Re: [RFC 1/2] media: add helpers for memory-to-memory media controller
On 06/15/2018 06:22 PM, Ezequiel Garcia wrote:
> Hi Hans,
>
> Thanks for the review.
>
> On Fri, 2018-06-15 at 11:24 +0200, Hans Verkuil wrote:
>> On 12/06/18 12:48, Ezequiel Garcia wrote:
>>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>>> index 3aa3d58d1d58..ff6fbe8333e1 100644
>>> --- a/include/media/media-entity.h
>>> +++ b/include/media/media-entity.h
>>> @@ -206,6 +206,9 @@ struct media_entity_operations {
>>> * The entity is embedded in a struct video_device instance.
>>> * @MEDIA_ENTITY_TYPE_V4L2_SUBDEV:
>>> * The entity is embedded in a struct v4l2_subdev instance.
>>> + * @MEDIA_ENTITY_TYPE_V4L2_MEM2MEM:
>>> + * The entity is not embedded in any struct, but part of
>>> + * a memory-to-memory topology.
>>
>> I see no need for this. An M2M device is of type VIDEO_DEVICE, no need to
>> change that.
>>
>
> Well, the problem is that this type is used to cast the media_entity
> using container_of macro, by means of is_media_entity_v4l2_video_device
> and is_media_entity_v4l2_subdev.
>
> So, by using one these types we'd be breaking that assumption.
Good point. But in that case I would just use MEDIA_ENTITY_TYPE_V4L2_BASE.
That's fine here.
>
>>> *
>>> * Media entity objects are often not instantiated directly, but the media
>>> * entity structure is inherited by (through embedding) other
>>> subsystem-specific
>>> @@ -222,6 +225,7 @@ enum media_entity_type {
>>> MEDIA_ENTITY_TYPE_BASE,
>>> MEDIA_ENTITY_TYPE_VIDEO_DEVICE,
>>> MEDIA_ENTITY_TYPE_V4L2_SUBDEV,
>>> + MEDIA_ENTITY_TYPE_MEM2MEM,
>>> };
>>>
>>> /**
>>> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
>>> index 456ac13eca1d..a9df949bb9c3 100644
>>> --- a/include/media/v4l2-dev.h
>>> +++ b/include/media/v4l2-dev.h
>>> @@ -30,6 +30,7 @@
>>> * @VFL_TYPE_SUBDEV: for V4L2 subdevices
>>> * @VFL_TYPE_SDR: for Software Defined Radio tuners
>>> * @VFL_TYPE_TOUCH:for touch sensors
>>> + * @VFL_TYPE_MEM2MEM: for mem2mem devices
>>> * @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
>>> */
>>> enum vfl_devnode_type {
>>> @@ -39,6 +40,7 @@ enum vfl_devnode_type {
>>> VFL_TYPE_SUBDEV,
>>> VFL_TYPE_SDR,
>>> VFL_TYPE_TOUCH,
>>> + VFL_TYPE_MEM2MEM,
>>> VFL_TYPE_MAX /* Shall be the last one */
>>> };
>>>
>>> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
>>> index 3d07ba3a8262..9dfe9bd23f89 100644
>>> --- a/include/media/v4l2-mem2mem.h
>>> +++ b/include/media/v4l2-mem2mem.h
>>> @@ -53,6 +53,7 @@ struct v4l2_m2m_ops {
>>> void (*unlock)(void *priv);
>>> };
>>>
>>> +struct video_device;
>>> struct v4l2_m2m_dev;
>>>
>>> /**
>>> @@ -328,6 +329,10 @@ int v4l2_m2m_mmap(struct file *file, struct
>>> v4l2_m2m_ctx *m2m_ctx,
>>> */
>>> struct v4l2_m2m_dev *v4l2_m2m_init(const struct v4l2_m2m_ops *m2m_ops);
>>>
>>> +int v4l2_m2m_register_media_controller(struct v4l2_m2m_dev *m2m_dev,
>>> struct video_device *vdev);
>>> +
>>> +void v4l2_m2m_unregister_media_controller(struct v4l2_m2m_dev *m2m_dev);
>>> +
>>> /**
>>> * v4l2_m2m_release() - cleans up and frees a m2m_dev structure
>>> *
>>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>>> index c7e9a5cba24e..becb7db77f6a 100644
>>> --- a/include/uapi/linux/media.h
>>> +++ b/include/uapi/linux/media.h
>>> @@ -81,6 +81,7 @@ struct media_device_info {
>>> #define MEDIA_ENT_F_IO_DTV (MEDIA_ENT_F_BASE + 0x01001)
>>> #define MEDIA_ENT_F_IO_VBI (MEDIA_ENT_F_BASE + 0x01002)
>>> #define MEDIA_ENT_F_IO_SWRADIO (MEDIA_ENT_F_BASE +
>>> 0x01003)
>>> +#define MEDIA_ENT_F_IO_DMAENGINE (MEDIA_ENT_F_BASE + 0x01004)
>>
>> Drop this as well. Just stick to MEDIA_ENT_F_IO_V4L which is what we've
>> decided to
>> call such entities (for better or worse).
>>
>
> Will do.
>
>>>
>>> /*
>>> * Sensor functions
>>> @@ -132,6 +133,7 @@ struct media_device_info {
>>> #define MEDIA_ENT_F_PROC_VIDEO_LUT (MEDIA_ENT_F_BASE + 0x4004)
>>> #define MEDIA_ENT_F_PROC_VIDEO_SCALER (MEDIA_ENT_F_BASE +
>>> 0x4005)
>>> #define MEDIA_ENT_F_PROC_VIDEO_STATISTICS (MEDIA_ENT_F_BASE + 0x4006)
>>> +#define MEDIA_ENT_F_PROC_VIDEO_TRANSFORM (MEDIA_ENT_F_BASE + 0x4007)
>>
>> I think we need to be a bit more specific here:
>>
>> #define MEDIA_ENT_F_PROC_VIDEO_DECODER
>> #define MEDIA_ENT_F_PROC_VIDEO_ENCODER
>> #define MEDIA_ENT_F_PROC_VIDEO_DEINTERLACER
>> // others?
>>
>
> OK. And what about "composite" devices that can encode and perform
> other transforms, e.g. scale, rotation, etc.
The intention is to eventually allow for more than one function to be specified
for an entity. That should be done through 'properties', but that still hasn't
been implemented. For now we use 'function' to specify the primary function of
the device.
Regards,
Hans
Re: [ANN v2] Complex Camera Workshop - Tokyo - Jun, 19
Hi Tomasz, On June 18, 2018 6:00:47 PM GMT+09:00, Tomasz Figa wrote: >Hi Paul, > >On Mon, Jun 18, 2018 at 5:42 PM Paul Elder > wrote: >> >> >> >> Hello all, >> >> On June 4, 2018 10:33:03 PM GMT+09:00, Mauro Carvalho Chehab > wrote: >> >Hi all, >> > >> >I consolidated hopefully all comments I receive on the past >> >announcement >> >with regards to the complex camera workshop we're planning to happen >in >> >Tokyo, just before the Open Source Summit in Japan. >> > >> >The main focus of the workshop is to allow supporting devices with >> >MC-based >> >hardware connected to a camera. >> > >> >I'm enclosing a detailed description of the problem, in order to >> >allow the interested parties to be at the same page. >> > >> >We need to work towards an agenda for the meeting. >> > >> >From my side, I think we should have at least the following topics >at >> >the agenda: >> > >> >- a quick review about what's currently at libv4l2; >> >- a presentation about PipeWire solution; >> >- a discussion with the requirements for the new solution; >> >- a discussion about how we'll address - who will do what. >> > >> >Comments? Suggestions? >> > >> >Are there anyone else planning to either be there physically or via >> >Google Hangouts? >> > >> My name is Paul Elder. I am a university student studying computer >science, and I am interested in complex camera support in Linux. >> >> If it's not too late, could I join this meeting as well please, as I >am in Tokyo? > >Done. You should have received 3 further emails with necessary >invitations. Thank you. I have only received two: 来訪者事前登録のご案内, and the Google invitation. Paul -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
[PATCH] media/i2c: add missing entity functions
Several drivers in media/i2c do not set the entity function.
Correct this.
Signed-off-by: Hans Verkuil
---
drivers/media/i2c/adv7604.c | 1 +
drivers/media/i2c/adv7842.c | 1 +
drivers/media/i2c/et8ek8/et8ek8_driver.c | 1 +
drivers/media/i2c/mt9m032.c | 1 +
drivers/media/i2c/mt9p031.c | 1 +
drivers/media/i2c/mt9t001.c | 1 +
drivers/media/i2c/mt9v032.c | 1 +
7 files changed, 7 insertions(+)
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index cac2081e876e..f2df509e34f0 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3502,6 +3502,7 @@ static int adv76xx_probe(struct i2c_client *client,
for (i = 0; i < state->source_pad; ++i)
state->pads[i].flags = MEDIA_PAD_FL_SINK;
state->pads[state->source_pad].flags = MEDIA_PAD_FL_SOURCE;
+ sd->entity.function = MEDIA_ENT_F_DTV_DECODER;
err = media_entity_pads_init(>entity, state->source_pad + 1,
state->pads);
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index fddac32e5051..56da071f99cb 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -3541,6 +3541,7 @@ static int adv7842_probe(struct i2c_client *client,
INIT_DELAYED_WORK(>delayed_work_enable_hotplug,
adv7842_delayed_work_enable_hotplug);
+ sd->entity.function = MEDIA_ENT_F_DTV_DECODER;
state->pad.flags = MEDIA_PAD_FL_SOURCE;
err = media_entity_pads_init(>entity, 1, >pad);
if (err)
diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index e9eff9039ef5..37ef38947e01 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -1446,6 +1446,7 @@ static int et8ek8_probe(struct i2c_client *client,
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
sensor->subdev.internal_ops = _internal_ops;
+ sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0) {
diff --git a/drivers/media/i2c/mt9m032.c b/drivers/media/i2c/mt9m032.c
index 6a9e068462fd..b385f2b632ad 100644
--- a/drivers/media/i2c/mt9m032.c
+++ b/drivers/media/i2c/mt9m032.c
@@ -793,6 +793,7 @@ static int mt9m032_probe(struct i2c_client *client,
v4l2_ctrl_cluster(2, >hflip);
sensor->subdev.ctrl_handler = >ctrls;
+ sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 91d822fc4443..715be3632b01 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -,6 +,7 @@ static int mt9p031_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
mt9p031->subdev.internal_ops = _subdev_internal_ops;
+ mt9p031->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9p031->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0)
diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 9d981d9f5686..f683d2cb0486 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -943,6 +943,7 @@ static int mt9t001_probe(struct i2c_client *client,
mt9t001->subdev.internal_ops = _subdev_internal_ops;
mt9t001->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ mt9t001->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9t001->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index 4de63b2df334..f74730d24d8f 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -1162,6 +1162,7 @@ static int mt9v032_probe(struct i2c_client *client,
mt9v032->subdev.internal_ops = _subdev_internal_ops;
mt9v032->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ mt9v032->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9v032->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0)
--
2.17.0
[PATCH] adv7180/tvp514x/tvp7002: fix entity function
The entity function was ORed with the flags field instead of
assigned to the function field. Correct this.
Signed-off-by: Hans Verkuil
---
drivers/media/i2c/adv7180.c | 2 +-
drivers/media/i2c/tvp514x.c | 2 +-
drivers/media/i2c/tvp7002.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..a727d7f806a1 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1335,7 +1335,7 @@ static int adv7180_probe(struct i2c_client *client,
goto err_unregister_vpp_client;
state->pad.flags = MEDIA_PAD_FL_SOURCE;
- sd->entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+ sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>entity, 1, >pad);
if (ret)
goto err_free_ctrl;
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 6a9890531d01..675b9ae212ab 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -1084,7 +1084,7 @@ tvp514x_probe(struct i2c_client *client, const struct
i2c_device_id *id)
#if defined(CONFIG_MEDIA_CONTROLLER)
decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- decoder->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+ decoder->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(>sd.entity, 1, >pad);
if (ret < 0) {
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 4599b7e28a8d..4f5c627579c7 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -1010,7 +1010,7 @@ static int tvp7002_probe(struct i2c_client *c, const
struct i2c_device_id *id)
#if defined(CONFIG_MEDIA_CONTROLLER)
device->pad.flags = MEDIA_PAD_FL_SOURCE;
device->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- device->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+ device->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
error = media_entity_pads_init(>sd.entity, 1, >pad);
if (error < 0)
--
2.17.0
Re: [ANN v2] Complex Camera Workshop - Tokyo - Jun, 19
Hi Paul, On Mon, Jun 18, 2018 at 5:42 PM Paul Elder wrote: > > > > Hello all, > > On June 4, 2018 10:33:03 PM GMT+09:00, Mauro Carvalho Chehab > wrote: > >Hi all, > > > >I consolidated hopefully all comments I receive on the past > >announcement > >with regards to the complex camera workshop we're planning to happen in > >Tokyo, just before the Open Source Summit in Japan. > > > >The main focus of the workshop is to allow supporting devices with > >MC-based > >hardware connected to a camera. > > > >I'm enclosing a detailed description of the problem, in order to > >allow the interested parties to be at the same page. > > > >We need to work towards an agenda for the meeting. > > > >From my side, I think we should have at least the following topics at > >the agenda: > > > >- a quick review about what's currently at libv4l2; > >- a presentation about PipeWire solution; > >- a discussion with the requirements for the new solution; > >- a discussion about how we'll address - who will do what. > > > >Comments? Suggestions? > > > >Are there anyone else planning to either be there physically or via > >Google Hangouts? > > > My name is Paul Elder. I am a university student studying computer science, > and I am interested in complex camera support in Linux. > > If it's not too late, could I join this meeting as well please, as I am in > Tokyo? Done. You should have received 3 further emails with necessary invitations. Best regards, Tomasz
Re: [ANN v2] Complex Camera Workshop - Tokyo - Jun, 19
Hello, On Monday, 18 June 2018 11:42:37 EEST Paul Elder wrote: > On June 4, 2018 10:33:03 PM GMT+09:00, Mauro Carvalho Chehab wrote: > > Hi all, > > > > I consolidated hopefully all comments I receive on the past announcement > > with regards to the complex camera workshop we're planning to happen in > > Tokyo, just before the Open Source Summit in Japan. > > > > The main focus of the workshop is to allow supporting devices with > > MC-based hardware connected to a camera. > > > > I'm enclosing a detailed description of the problem, in order to > > allow the interested parties to be at the same page. > > > > We need to work towards an agenda for the meeting. > > > > From my side, I think we should have at least the following topics at > > the agenda: > > > > - a quick review about what's currently at libv4l2; > > - a presentation about PipeWire solution; > > - a discussion with the requirements for the new solution; > > - a discussion about how we'll address - who will do what. > > > > Comments? Suggestions? > > > > Are there anyone else planning to either be there physically or via > > Google Hangouts? > > My name is Paul Elder. I am a university student studying computer science, > and I am interested in complex camera support in Linux. > > If it's not too late, could I join this meeting as well please, as I am in > Tokyo? For the record, Paul is working with Kieran and me on V4L2 (and UVC in particular). -- Regards, Laurent Pinchart
Re: [PATCH v4 04/17] omap3isp: Add vb2_queue lock
On 06/15/2018 09:07 PM, Ezequiel Garcia wrote:
> vb2_queue locks is now mandatory. Add it, remove driver ad-hoc locks,
> and implement wait_{prepare, finish}.
You are also removing stream_lock, but that is not mentioned here.
This needs an improved commit log.
Regards,
Hans
>
> Signed-off-by: Ezequiel Garcia
> ---
> drivers/media/platform/omap3isp/ispvideo.c | 65 --
> drivers/media/platform/omap3isp/ispvideo.h | 1 -
> 2 files changed, 11 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c
> index 9d228eac24ea..f835aeb9ddc5 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -496,6 +496,8 @@ static const struct vb2_ops isp_video_queue_ops = {
> .buf_prepare = isp_video_buffer_prepare,
> .buf_queue = isp_video_buffer_queue,
> .start_streaming = isp_video_start_streaming,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> };
>
> /*
> @@ -628,11 +630,8 @@ void omap3isp_video_resume(struct isp_video *video, int
> continuous)
> {
> struct isp_buffer *buf = NULL;
>
> - if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> - mutex_lock(>queue_lock);
> + if (continuous && video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> vb2_discard_done(video->queue);
> - mutex_unlock(>queue_lock);
> - }
>
> if (!list_empty(>dmaqueue)) {
> buf = list_first_entry(>dmaqueue,
> @@ -909,13 +908,8 @@ isp_video_reqbufs(struct file *file, void *fh, struct
> v4l2_requestbuffers *rb)
> {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
> - int ret;
> -
> - mutex_lock(>queue_lock);
> - ret = vb2_reqbufs(>queue, rb);
> - mutex_unlock(>queue_lock);
>
> - return ret;
> + return vb2_reqbufs(>queue, rb);
> }
>
> static int
> @@ -923,13 +917,8 @@ isp_video_querybuf(struct file *file, void *fh, struct
> v4l2_buffer *b)
> {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
> - int ret;
>
> - mutex_lock(>queue_lock);
> - ret = vb2_querybuf(>queue, b);
> - mutex_unlock(>queue_lock);
> -
> - return ret;
> + return vb2_querybuf(>queue, b);
> }
>
> static int
> @@ -937,13 +926,8 @@ isp_video_qbuf(struct file *file, void *fh, struct
> v4l2_buffer *b)
> {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
> - int ret;
>
> - mutex_lock(>queue_lock);
> - ret = vb2_qbuf(>queue, b);
> - mutex_unlock(>queue_lock);
> -
> - return ret;
> + return vb2_qbuf(>queue, b);
> }
>
> static int
> @@ -951,13 +935,8 @@ isp_video_dqbuf(struct file *file, void *fh, struct
> v4l2_buffer *b)
> {
> struct isp_video_fh *vfh = to_isp_video_fh(fh);
> struct isp_video *video = video_drvdata(file);
> - int ret;
>
> - mutex_lock(>queue_lock);
> - ret = vb2_dqbuf(>queue, b, file->f_flags & O_NONBLOCK);
> - mutex_unlock(>queue_lock);
> -
> - return ret;
> + return vb2_dqbuf(>queue, b, file->f_flags & O_NONBLOCK);
> }
>
> static int isp_video_check_external_subdevs(struct isp_video *video,
> @@ -1096,8 +1075,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> if (type != video->type)
> return -EINVAL;
>
> - mutex_lock(>stream_lock);
> -
> /* Start streaming on the pipeline. No link touching an entity in the
>* pipeline can be activated or deactivated once streaming is started.
>*/
> @@ -1106,7 +1083,7 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
>
> ret = media_entity_enum_init(>ent_enum, >isp->media_dev);
> if (ret)
> - goto err_enum_init;
> + return ret;
>
> /* TODO: Implement PM QoS */
> pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
> @@ -1158,14 +1135,10 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> atomic_set(>frame_number, -1);
> pipe->field = vfh->format.fmt.pix.field;
>
> - mutex_lock(>queue_lock);
> ret = vb2_streamon(>queue, type);
> - mutex_unlock(>queue_lock);
> if (ret < 0)
> goto err_check_format;
>
> - mutex_unlock(>stream_lock);
> -
> return 0;
>
> err_check_format:
> @@ -1184,9 +1157,6 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
>
> media_entity_enum_cleanup(>ent_enum);
>
> -err_enum_init:
> - mutex_unlock(>stream_lock);
> -
> return ret;
> }
>
> @@ -1203,15 +1173,10 @@ isp_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type)
> if (type != video->type)
>
Re: [PATCH v6 04/12] intel-ipu3: Implement DMA mapping functions
On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
>
> From: Tomasz Figa
>
> This driver uses IOVA space for buffer mapping through IPU3 MMU
> to transfer data between imaging pipelines and system DDR.
>
> Signed-off-by: Tomasz Figa
> Signed-off-by: Yong Zhi
> ---
> drivers/media/pci/intel/ipu3/ipu3-css-pool.h | 36
> drivers/media/pci/intel/ipu3/ipu3-dmamap.c | 280
> +++
> drivers/media/pci/intel/ipu3/ipu3-dmamap.h | 22 +++
> drivers/media/pci/intel/ipu3/ipu3.h | 151 +++
> 4 files changed, 489 insertions(+)
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3.h
>
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> new file mode 100644
> index ..4b22e0856232
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2018 Intel Corporation */
> +
> +#ifndef __IPU3_UTIL_H
> +#define __IPU3_UTIL_H
> +
> +struct device;
> +
> +#define IPU3_CSS_POOL_SIZE 4
> +
> +struct ipu3_css_map {
> + size_t size;
> + void *vaddr;
> + dma_addr_t daddr;
> + struct vm_struct *vma;
> +};
> +
> +struct ipu3_css_pool {
> + struct {
> + struct ipu3_css_map param;
> + long framenum;
> + } entry[IPU3_CSS_POOL_SIZE];
> + unsigned int last; /* Latest entry */
It's not clear what "Latest entry" means here. Since these structs are
a part of the interface exposed by this header, could you write proper
kerneldoc comments for all fields in both of them?
> +};
> +
> +int ipu3_css_dma_buffer_resize(struct device *dev, struct ipu3_css_map *map,
> + size_t size);
> +void ipu3_css_pool_cleanup(struct device *dev, struct ipu3_css_pool *pool);
> +int ipu3_css_pool_init(struct device *dev, struct ipu3_css_pool *pool,
> + size_t size);
> +int ipu3_css_pool_get(struct ipu3_css_pool *pool, long framenum);
> +void ipu3_css_pool_put(struct ipu3_css_pool *pool);
> +const struct ipu3_css_map *ipu3_css_pool_last(struct ipu3_css_pool *pool,
> + unsigned int last);
> +
> +#endif
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> new file mode 100644
> index ..b2bc5d00debc
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> @@ -0,0 +1,280 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Intel Corporation
> + * Copyright (C) 2018 Google, Inc.
Would you mind changing as below?
Copyright 2018 Google LLC.
> + *
> + * Author: Tomasz Figa
> + * Author: Yong Zhi
> + */
> +
> +#include
> +
> +#include "ipu3.h"
> +#include "ipu3-css-pool.h"
> +#include "ipu3-mmu.h"
> +
> +/*
> + * Free a buffer allocated by ipu3_dmamap_alloc_buffer()
> + */
> +static void ipu3_dmamap_free_buffer(struct page **pages,
> + size_t size)
> +{
> + int count = size >> PAGE_SHIFT;
> +
> + while (count--)
> + __free_page(pages[count]);
> + kvfree(pages);
> +}
> +
> +/*
> + * Based on the implementation of __iommu_dma_alloc_pages()
> + * defined in drivers/iommu/dma-iommu.c
> + */
> +static struct page **ipu3_dmamap_alloc_buffer(size_t size,
> + unsigned long order_mask,
> + gfp_t gfp)
> +{
> + struct page **pages;
> + unsigned int i = 0, count = size >> PAGE_SHIFT;
> + const gfp_t high_order_gfp = __GFP_NOWARN | __GFP_NORETRY;
> +
> + /* Allocate mem for array of page ptrs */
> + pages = kvmalloc_array(count, sizeof(struct page *), GFP_KERNEL);
sizeof(*pages) to ensure that the right type is used regardless of declaration.
> +
No need for this blank line.
> + if (!pages)
> + return NULL;
[snip]
> +/**
> + * ipu3_dmamap_alloc - allocate and map a buffer into KVA
> + * @dev: struct device pointer
> + * @map: struct to store mapping variables
> + * @len: size required
> + *
> + * Return KVA on success or NULL on failure
> + */
> +void *ipu3_dmamap_alloc(struct device *dev, struct ipu3_css_map *map,
> + size_t len)
> +{
> + struct imgu_device *imgu = dev_get_drvdata(dev);
Wouldn't it make more sense to just pass struct imgu_device pointer to
all the functions in this file directly?
> + unsigned long shift = iova_shift(>iova_domain);
> + unsigned int alloc_sizes = imgu->mmu->pgsize_bitmap;
> + size_t size = PAGE_ALIGN(len);
> + struct page **pages;
> + dma_addr_t iovaddr;
> + struct iova *iova;
> + int i, rval;
> +
Re: [PATCH v6 03/12] intel-ipu3: mmu: Implement driver
On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
>
> From: Tomasz Figa
>
> This driver translates IO virtual address to physical
> address based on two levels page tables.
>
> Signed-off-by: Tomasz Figa
> Signed-off-by: Yong Zhi
> ---
> drivers/media/pci/intel/ipu3/ipu3-mmu.c | 560
>
> drivers/media/pci/intel/ipu3/ipu3-mmu.h | 28 ++
> 2 files changed, 588 insertions(+)
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h
>
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c
> b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
> new file mode 100644
> index ..a4b3e1680bbb
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
> @@ -0,0 +1,560 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Intel Corporation.
> + * Copyright (C) 2018 Google, Inc.
I followed wrong guide when adding this one. Could you fix it up to
the following?
Copyright 2018 Google LLC.
[snip]
> +/**
> + * ipu3_mmu_exit() - clean up IPU3 MMU block
> + * @mmu: IPU3 MMU private data
> + */
> +void ipu3_mmu_exit(struct ipu3_mmu_info *info)
> +{
> + struct ipu3_mmu *mmu = to_ipu3_mmu(info);
> +
> + /* We are going to free our page tables, no more memory access. */
> + ipu3_mmu_set_halt(mmu, true);
> + ipu3_mmu_tlb_invalidate(mmu);
> +
> + ipu3_mmu_free_page_table(mmu->l1pt);
> + vfree(mmu->l2pts);
> + ipu3_mmu_free_page_table(mmu->dummy_l2pt);
> + kfree(mmu->dummy_page);
Should be free_page(). (Might be already included in your tree as per
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1084522)
> + kfree(mmu);
> +}
> +
> +void ipu3_mmu_suspend(struct ipu3_mmu_info *info)
> +{
> + struct ipu3_mmu *mmu = to_ipu3_mmu(info);
> +
> + ipu3_mmu_set_halt(mmu, true);
> +}
> +
> +void ipu3_mmu_resume(struct ipu3_mmu_info *info)
> +{
> + struct ipu3_mmu *mmu = to_ipu3_mmu(info);
> + u32 pteval;
> +
> + ipu3_mmu_set_halt(mmu, true);
> +
> + pteval = IPU3_ADDR2PTE(virt_to_phys(mmu->l1pt));
> + writel(pteval, mmu->base + REG_L1_PHYS);
> +
> + ipu3_mmu_tlb_invalidate(mmu);
> + ipu3_mmu_set_halt(mmu, false);
> +}
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.h
> b/drivers/media/pci/intel/ipu3/ipu3-mmu.h
> new file mode 100644
> index ..4976187c18f6
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2018 Intel Corporation */
> +/* Copyright (C) 2018 Google, Inc. */
> +
> +#ifndef __IPU3_MMU_H
> +#define __IPU3_MMU_H
> +
> +struct ipu3_mmu_info {
> + dma_addr_t aperture_start; /* First address that can be mapped*/
> + dma_addr_t aperture_end; /* Last address that can be mapped */
> + unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
If documenting the fields, why not use a kerneldoc comment above the
struct instead?
Best regards,
Tomasz
Re: [PATCH v6 02/12] intel-ipu3: Add user space API definitions
Hi Yong,
On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
>
> Define the structures and macros to be used by public.
>
> Signed-off-by: Yong Zhi
> Signed-off-by: Rajmohan Mani
> ---
> include/uapi/linux/intel-ipu3.h | 1403
> +++
> 1 file changed, 1403 insertions(+)
> create mode 100644 include/uapi/linux/intel-ipu3.h
>
Since we'll need 1 more resend with latest fixes from Chromium tree
and recently posted documentation anyway, let me do some more
nitpicking inline, so we can end up with slightly cleaner code. :)
> diff --git a/include/uapi/linux/intel-ipu3.h b/include/uapi/linux/intel-ipu3.h
> new file mode 100644
> index ..694ef0c8d7a7
> --- /dev/null
> +++ b/include/uapi/linux/intel-ipu3.h
> @@ -0,0 +1,1403 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2018 Intel Corporation */
> +
> +#ifndef __IPU3_UAPI_H
> +#define __IPU3_UAPI_H
> +
> +#include
> +
> +#define IPU3_UAPI_ISP_VEC_ELEMS64
> +
> +#define IMGU_ABI_PAD __aligned(IPU3_UAPI_ISP_WORD_BYTES)
This seems unused.
> +#define IPU3_ALIGN __attribute__((aligned(IPU3_UAPI_ISP_WORD_BYTES)))
Any reason to mix both __aligned() and __attribute__((aligned()))?
> +
> +#define IPU3_UAPI_ISP_WORD_BYTES 32
It would make sense to define this above IPU3_ALIGN(), which references it.
> +#define IPU3_UAPI_MAX_STRIPES 2
> +
> +/*** ipu3_uapi_stats_3a ***/
> +
> +#define IPU3_UAPI_MAX_BUBBLE_SIZE 10
> +
> +#define IPU3_UAPI_AE_COLORS4
> +#define IPU3_UAPI_AE_BINS 256
> +
> +#define IPU3_UAPI_AWB_MD_ITEM_SIZE 8
> +#define IPU3_UAPI_AWB_MAX_SETS 60
> +#define IPU3_UAPI_AWB_SET_SIZE 0x500
Why not just decimal 1280?
> +#define IPU3_UAPI_AWB_SPARE_FOR_BUBBLES \
> + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> +IPU3_UAPI_AWB_MD_ITEM_SIZE)
> +#define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \
> + (IPU3_UAPI_AWB_MAX_SETS * \
> +(IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES))
> +
> +#define IPU3_UAPI_AF_MAX_SETS 24
> +#define IPU3_UAPI_AF_MD_ITEM_SIZE 4
> +#define IPU3_UAPI_AF_SPARE_FOR_BUBBLES \
> + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> +IPU3_UAPI_AF_MD_ITEM_SIZE)
> +#define IPU3_UAPI_AF_Y_TABLE_SET_SIZE 0x80
Why not just decimal 128?
> +#define IPU3_UAPI_AF_Y_TABLE_MAX_SIZE \
> + (IPU3_UAPI_AF_MAX_SETS * \
> +(IPU3_UAPI_AF_Y_TABLE_SET_SIZE + IPU3_UAPI_AF_SPARE_FOR_BUBBLES) * \
> +IPU3_UAPI_MAX_STRIPES)
> +
> +#define IPU3_UAPI_AWB_FR_MAX_SETS 24
> +#define IPU3_UAPI_AWB_FR_MD_ITEM_SIZE 8
> +#define IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE0x100
Why not just decimal 256?
> +#define IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES \
> + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
> +IPU3_UAPI_AWB_FR_MD_ITEM_SIZE)
> +#define IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE \
> + (IPU3_UAPI_AWB_FR_MAX_SETS * \
> + (IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE + \
> +IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES) * IPU3_UAPI_MAX_STRIPES)
[snip]
> +struct ipu3_uapi_af_filter_config {
> + struct {
> + __u8 a1;
> + __u8 a2;
> + __u8 a3;
> + __u8 a4;
> + } y1_coeff_0;
> + struct {
> + __u8 a5;
> + __u8 a6;
> + __u8 a7;
> + __u8 a8;
> + } y1_coeff_1;
> + struct {
> + __u8 a9;
> + __u8 a10;
> + __u8 a11;
> + __u8 a12;
> + } y1_coeff_2;
Why these aren't just __u8 y1_coeff[12]?
> +
> + __u32 y1_sign_vec;
> +
> + struct {
> + __u8 a1;
> + __u8 a2;
> + __u8 a3;
> + __u8 a4;
> + } y2_coeff_0;
> + struct {
> + __u8 a5;
> + __u8 a6;
> + __u8 a7;
> + __u8 a8;
> + } y2_coeff_1;
> + struct {
> + __u8 a9;
> + __u8 a10;
> + __u8 a11;
> + __u8 a12;
> + } y2_coeff_2;
Ditto.
> +
> + __u32 y2_sign_vec;
> +
> + struct {
> + __u8 y_gen_rate_gr; /* 6 bits */
> + __u8 y_gen_rate_r;
> + __u8 y_gen_rate_b;
> + __u8 y_gen_rate_gb;
> + } y_calc;
Why not just call the struct "y_gen_rate" and then fields "gr", "r",
"b", "gb"? It would make any code referencing them much shorter.
> +
> + struct {
> + __u32 __reserved0:8;
> + __u32 y1_nf:4;
> + __u32 __reserved1:4;
> + __u32 y2_nf:4;
> + __u32
