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: Fri Oct 19 05:00:11 CEST 2018 media-tree git hash:490d84f6d73c12f4204241cff8651eed60aae914 media_build git hash: 9f419c414672676f63e85a61ea99df0ddcd6e9a7 v4l-utils git hash: 8d2093a8a4bc43622cd68aeb85fb2b817de55993 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.11-marune 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-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.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.123-i686: OK linux-3.18.123-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.52-i686: OK linux-4.1.52-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.159-i686: OK linux-4.4.159-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.131-i686: OK linux-4.9.131-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.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19-rc6-i686: OK linux-4.19-rc6-x86_64: OK apps: ERRORS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v3 01/16] media: imx: add mem2mem device
On Tue, Sep 18, 2018 at 2:34 AM Philipp Zabel wrote: > > Add a single imx-media mem2mem video device that uses the IPU IC PP > (image converter post processing) task for scaling and colorspace > conversion. > On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used. > > The hardware only supports writing to destination buffers up to > 1024x1024 pixels in a single pass, arbitrary sizes can be achieved > by rendering multiple tiles per frame. > > Signed-off-by: Philipp Zabel > [steve_longerb...@mentor.com: use ipu_image_convert_adjust(), fix > device_run() error handling] > Signed-off-by: Steve Longerbeam > --- > Changes since v2: > - Rely on ipu_image_convert_adjust() in mem2mem_try_fmt() for format >adjustments. This makes the mem2mem driver mostly a V4L2 mem2mem API >wrapper around the IPU image converter, and independent of the >internal image converter implementation. > - Remove the source and destination buffers on error in device_run(). >Otherwise the conversion is re-attempted apparently over and over >again (with WARN() backtraces). > - Allow subscribing to control changes. > --- > drivers/staging/media/imx/Kconfig | 1 + > drivers/staging/media/imx/Makefile| 1 + > drivers/staging/media/imx/imx-media-dev.c | 11 + > drivers/staging/media/imx/imx-media-mem2mem.c | 873 ++ > drivers/staging/media/imx/imx-media.h | 10 + > 5 files changed, 896 insertions(+) > create mode 100644 drivers/staging/media/imx/imx-media-mem2mem.c > Philipp, Thanks for submitting this! I'm hoping this lets us use non-IMX capture devices along with the IMX media controller entities to so we can use hardware CSC,scaling,pixel-format-conversions and ultimately coda based encode. I've built this on top of linux-media and see that it registers as /dev/video8 but I'm not clear how to use it? I don't see it within the media controller graph. Regards, Tim
[PATCH] media: rc: imon_raw: use fls rather than loop per bit
Previously, the code would loop for each of the 40 bits. Now it will
branch for each edge in the IR, which will be much less.
Signed-off-by: Sean Young
---
drivers/media/rc/imon_raw.c | 47 ++---
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/drivers/media/rc/imon_raw.c b/drivers/media/rc/imon_raw.c
index 7796098d9c30..25e56c5b13c0 100644
--- a/drivers/media/rc/imon_raw.c
+++ b/drivers/media/rc/imon_raw.c
@@ -14,51 +14,50 @@ struct imon {
struct device *dev;
struct urb *ir_urb;
struct rc_dev *rcdev;
- u8 ir_buf[8];
+ u8 ir_buf[8] __aligned(__alignof__(u64));
char phys[64];
};
/*
- * ffs/find_next_bit() searches in the wrong direction, so open-code our own.
+ * The first 5 bytes of data represent IR pulse or space. Each bit, starting
+ * from highest bit in the first byte, represents 250µs of data. It is 1
+ * for space and 0 for pulse.
+ *
+ * The station sends 10 packets, and the 7th byte will be number 1 to 10, so
+ * when we receive 10 we assume all the data has arrived.
*/
-static inline int is_bit_set(const u8 *buf, int bit)
-{
- return buf[bit / 8] & (0x80 >> (bit & 7));
-}
-
static void imon_ir_data(struct imon *imon)
{
struct ir_raw_event rawir = {};
- int offset = 0, size = 5 * 8;
+ u64 d = be64_to_cpup((__be64 *)imon->ir_buf) >> 24;
+ int offset = 40;
int bit;
dev_dbg(imon->dev, "data: %*ph", 8, imon->ir_buf);
- while (offset < size) {
- bit = offset;
- while (!is_bit_set(imon->ir_buf, bit) && bit < size)
- bit++;
- dev_dbg(imon->dev, "pulse: %d bits", bit - offset);
- if (bit > offset) {
+ do {
+ bit = fls64(d & (BIT_ULL(offset) - 1));
+ if (bit < offset) {
+ dev_dbg(imon->dev, "pulse: %d bits", offset - bit);
rawir.pulse = true;
- rawir.duration = (bit - offset) * BIT_DURATION;
+ rawir.duration = (offset - bit) * BIT_DURATION;
ir_raw_event_store_with_filter(imon->rcdev, );
- }
- if (bit >= size)
- break;
+ if (bit == 0)
+ break;
- offset = bit;
- while (is_bit_set(imon->ir_buf, bit) && bit < size)
- bit++;
- dev_dbg(imon->dev, "space: %d bits", bit - offset);
+ offset = bit;
+ }
+
+ bit = fls64(~d & (BIT_ULL(offset) - 1));
+ dev_dbg(imon->dev, "space: %d bits", offset - bit);
rawir.pulse = false;
- rawir.duration = (bit - offset) * BIT_DURATION;
+ rawir.duration = (offset - bit) * BIT_DURATION;
ir_raw_event_store_with_filter(imon->rcdev, );
offset = bit;
- }
+ } while (offset > 0);
if (imon->ir_buf[7] == 0x0a) {
ir_raw_event_set_idle(imon->rcdev, true);
--
2.17.2
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
> On Oct 18, 2018, at 3:03 AM, jacopo mondi wrote: > >> On Thu, Oct 18, 2018 at 11:31:52AM +0200, Maxime Ripard wrote: >>> On Wed, Oct 17, 2018 at 09:51:43PM +0200, jacopo mondi wrote: >>> Hello Sam and Maxime (and other ov5640-ers :) >>> On Wed, Oct 17, 2018 at 10:54:01AM -0700, Sam Bobrowicz wrote: Hello Maxime and Jacopo (and other ov5640-ers), I just submitted my version of this patch to the mailing list as RFC. It is working on my MIPI platform. Please try it if you have time. Hopefully we can merge these two into a single patch that doesn't break any platforms. >>> >>> Thanks, I have seen your patch but it seems to contain a lot of things >>> already part of Maxime's series. Was this intentional? >>> >>> Now the un-pleaseant part: I have just sent out my re-implementation >>> of the MIPI clock tree configuration, based on top of Maxime's series. >>> Both you and me have spent a looot of time on this I'm sure, and now >>> we have two competing implementations. >>> >>> I had a quick look at yours, and for sure there are things I am not >>> taking care of (I'm thinking about the 0x4837 register that seems to >>> be important for your platform), so I think both our implementations >>> can benefits from a comparison. What is important to me is that both >>> you and me don't feel like our work has been wasted, so let's try to >>> find out a way to get the better of the two put together, and possibly >>> applied on top of Maxime's series, so that a v5 of this will work for >>> both MIPI and DVP interfaces. How to do that I'm not sure atm, I think >>> other reviewers might help in that if they want to have a look at both >>> our series :) >> >> IIRC, Sam's system has never worked with the ov5640 driver, and his >> patches now make it work. >> >> Your patches on the other hand make sure that the current series >> doesn't break existing users. So I guess we could merge your current >> patches into the v5 of my rework, and have Sam send his work on top of >> that. >> >> Does that make sense? > > It does for me, but it puts the burden on Sam to re-apply his work > on top of [yours+mine] (which is something he would have had to do > anyhow to have his patches accepted, as he would have had to rebase on > top of your series). > Don’t worry about it :) > I hope to find some more time to look into his series and find out how > hard it would be to add his changes on top of mine, and hopefully help > with this. > Also, testing my patches with DVP would be nice (it should not be > affected at all, but still...) > > Thanks > j > >> >> Maxime >> >> -- >> Maxime Ripard, Bootlin >> Embedded Linux and Kernel engineering >> https://bootlin.com > > I’m fine with this approach, but it takes my ability to easily test your changes on my MIPI platform off the table. I will be around to run some manual tests on your algorithms and answer tech details about my experiments with the sensor, but it will fall on Jacobi to ensure that whatever patch you land on doesn’t introduce a regression for MIPI platforms. I can then submit a PCLK period patch on top of what you end up with, which will then put my platform in the game. Sam
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
Hey Jacobi,
Not a lot of time to respond right now, these days I’m bouncing around between
a couple jobs. I’ll be trying your and Maximes patches and responding to tech
details this weekend.
Just want to say that as long as we get the driver working I’m happy :) The
VAST majority of time put into this has been the reverse engineering effort to
understand how the part works. Iterating through various versions takes time
as we get on the same page, but will be necessary to make sure we don’t
introduce any regressions on any platforms. I don’t think it is time wasted.
I don’t think my patch is competing, submitting it was just the best way to
communicate what works on my platform. I don’t expect the final patch we land
on to be designated as “authored by” me, especially since it was copied mostly
from Maximes patch. Also, FYI, I’ve put my other patch series about the ov5640
on hold until we figure this out.
Sam
Sent from my iPhone
> On Oct 17, 2018, at 12:51 PM, jacopo mondi wrote:
>
> Hello Sam and Maxime (and other ov5640-ers :)
>
>> On Wed, Oct 17, 2018 at 10:54:01AM -0700, Sam Bobrowicz wrote:
>> Hello Maxime and Jacopo (and other ov5640-ers),
>>
>> I just submitted my version of this patch to the mailing list as RFC.
>> It is working on my MIPI platform. Please try it if you have time.
>> Hopefully we can merge these two into a single patch that doesn't
>> break any platforms.
>
> Thanks, I have seen your patch but it seems to contain a lot of things
> already part of Maxime's series. Was this intentional?
>
> Now the un-pleaseant part: I have just sent out my re-implementation
> of the MIPI clock tree configuration, based on top of Maxime's series.
> Both you and me have spent a looot of time on this I'm sure, and now
> we have two competing implementations.
>
> I had a quick look at yours, and for sure there are things I am not
> taking care of (I'm thinking about the 0x4837 register that seems to
> be important for your platform), so I think both our implementations
> can benefits from a comparison. What is important to me is that both
> you and me don't feel like our work has been wasted, so let's try to
> find out a way to get the better of the two put together, and possibly
> applied on top of Maxime's series, so that a v5 of this will work for
> both MIPI and DVP interfaces. How to do that I'm not sure atm, I think
> other reviewers might help in that if they want to have a look at both
> our series :)
>
> Thanks everyone for the hard work on this sensor for now!
>
> Thanks
> j
>
>>
>> Thanks,
>> Sam
>>
>> Additional notes below.
>>
>>> On Tue, Oct 16, 2018 at 9:54 AM jacopo mondi wrote:
>>>
>>> Hello Maxime,
>>> a few comments I have collected while testing the series.
>>>
>>> Please see below.
>>>
On Thu, Oct 11, 2018 at 11:20:56AM +0200, Maxime Ripard wrote:
The clock structure for the PCLK is quite obscure in the documentation, and
was hardcoded through the bytes array of each and every mode.
This is troublesome, since we cannot adjust it at runtime based on other
parameters (such as the number of bytes per pixel), and we can't support
either framerates that have not been used by the various vendors, since we
don't have the needed initialization sequence.
We can however understand how the clock tree works, and then implement some
functions to derive the various parameters from a given rate. And now that
those parameters are calculated at runtime, we can remove them from the
initialization sequence.
The modes also gained a new parameter which is the clock that they are
running at, from the register writes they were doing, so for now the switch
to the new algorithm should be transparent.
Signed-off-by: Maxime Ripard
---
drivers/media/i2c/ov5640.c | 289 -
1 file changed, 288 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 30b15e91d8be..88fb16341466 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -175,6 +175,7 @@ struct ov5640_mode_info {
u32 htot;
u32 vact;
u32 vtot;
+ u32 pixel_clock;
const struct reg_value *reg_data;
u32 reg_data_size;
};
@@ -700,6 +701,7 @@ static const struct reg_value
ov5640_setting_15fps_QSXGA_2592_1944[] = {
/* power-on sensor init reg table */
static const struct ov5640_mode_info ov5640_mode_init_data = {
0, SUBSAMPLING, 640, 1896, 480, 984,
+ 5600,
ov5640_init_setting_30fps_VGA,
ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
};
@@ -709,74 +711,91 @@
ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
{
{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
176, 1896, 144, 984,
+
RFC: kernelCI media subsystem pilot (Test results for gtucker/kernelci-media - gtucker-kernelci-media-001-6-g1b2c6e5844d8)
Hi everyone, In Collabora, and as part of our kernelci work, we are doing research on kernel functional testing with kernelci. For those new to kernelci, see https://github.com/kernelci/kernelci-doc/wiki/KernelCI and https://kernelci.org/. The goal is to lay down the infrastructure required to make automated test coverage an integral part of our feature and bugfix development process. So, as a first attempt, we've decided to extend kernelci test v4l2 plan support, leading the way to extending other subsystems' test plans. Currently, kernelci looks for a list of branches every hour and see if anything changed. For any branch that has changed, it triggers builds, boots, tests and reports for each branch that had some changes since last time it ran. For this pilot, we've decided to target just a few devices: qemu with vivid, rk3399-gru-kevin and rk3288-veyron-jaq with uvc. We'd like to get some early feedback on this, so we are sending an example of how a kernelci report would look like, to trigger some discussion around the direction this should take. Thanks! === Test results for: Tree:gtucker Branch: kernelci-media Kernel: gtucker-kernelci-media-002-2-gaa27eb0392c7 URL: https://gitlab.collabora.com/gtucker/linux.git Commit: aa27eb0392c70adec713e911a9b5267a1d853624 Test plans: v4l2 Summary --- 3 test groups results 1 | v4l2 | rk3399-gru-kevin | arm64 | 49 total: 17 PASS 4 FAIL 28 SKIP 2 | v4l2 | rk3288-veyron-jaq | arm | 49 total: 17 PASS 4 FAIL 28 SKIP 3 | v4l2 | qemu | arm64 | 168 total: 102 PASS 0 FAIL 66 SKIP Tests - 1 | v4l2 | rk3399-gru-kevin | arm64 | 49 total: 17 PASS 4 FAIL 28 SKIP Config: defconfig Lab Name:lab-collabora-dev Date:2018-10-18 18:48:52.426000 TXT log: http://staging-storage.kernelci.org/gtucker/kernelci-media/gtucker-kernelci-media-002-2-gaa27eb0392c7/arm64/defconfig/lab-collabora-dev /v4l2-rk3399-gru-kevin.txt HTML log: http://staging-storage.kernelci.org/gtucker/kernelci-media/gtucker-kernelci-media-002-2-gaa27eb0392c7/arm64/defconfig/lab-collabora-dev /v4l2-rk3399-gru-kevin.html Rootfs: http://staging-storage.kernelci.org/images/rootfs/debian/stretchv4l2/20181017.1//arm64/rootfs.cpio.gz Test Git:git://linuxtv.org/v4l-utils.git Test Commit: e889b0d56757b9a74eb8c4c8cc2ebd5b18e228b2 Test cases: * MMAP: FAIL * blocking-wait: PASS * read/write: SKIP * read/write: SKIP * read/write: SKIP * VIDIOC_EXPBUF: PASS * VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: PASS * VIDIOC_TRY_DECODER_CMD: SKIP * VIDIOC_G_ENC_INDEX: SKIP * VIDIOC_TRY_ENCODER_CMD: SKIP * Scaling: SKIP * Composing: SKIP * Cropping: SKIP * VIDIOC_G_SLICED_VBI_CAP: SKIP * VIDIOC_S_FMT: PASS * VIDIOC_TRY_FMT: PASS * VIDIOC_G_FMT: PASS * VIDIOC_G_FBUF: SKIP * VIDIOC_G/S_PARM: FAIL * VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: PASS * VIDIOC_G/S_JPEGCOMP: SKIP * VIDIOC_UNSUBSCRIBE_EVENT/DQEVENT: PASS * VIDIOC_G/S/TRY_EXT_CTRLS: FAIL * VIDIOC_G/S_CTRL: PASS * VIDIOC_QUERYCTRL: PASS * VIDIOC_QUERY_EXT_CTRL/QUERYMENU: FAIL * VIDIOC_G/S_EDID: SKIP * VIDIOC_DV_TIMINGS_CAP: SKIP * VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: SKIP * VIDIOC_ENUM/G/S/QUERY_STD: SKIP * VIDIOC_G/S_AUDOUT: SKIP * VIDIOC_G/S/ENUMOUTPUT: SKIP * VIDIOC_ENUMAUDOUT: SKIP * VIDIOC_G/S_FREQUENCY: SKIP * VIDIOC_G/S_MODULATOR: SKIP * VIDIOC_G/S_AUDIO: SKIP * VIDIOC_G/S/ENUMINPUT: PASS * VIDIOC_ENUMAUDIO: SKIP * VIDIOC_S_HW_FREQ_SEEK: SKIP * VIDIOC_G/S_FREQUENCY: SKIP * VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: SKIP * VIDIOC_LOG_STATUS: SKIP * VIDIOC_DBG_G/S_REGISTER: SKIP * for-unlimited-opens: PASS * VIDIOC_G/S_PRIORITY: PASS * VIDIOC_QUERYCAP: PASS * second-/dev/video0-open: PASS * VIDIOC_QUERYCAP: PASS * MC-information-see-Media-Driver-Info-above: PASS 2 | v4l2 | rk3288-veyron-jaq | arm | 49 total: 17 PASS 4 FAIL 28 SKIP Config: multi_v7_defconfig Lab Name:lab-collabora-dev Date:2018-10-18 17:00:41.724000 TXT log: http://staging-storage.kernelci.org/gtucker/kernelci-media/gtucker-kernelci-media-002-2-gaa27eb0392c7/arm/multi_v7_defconfig/lab-collab ora-dev/v4l2-rk3288-veyron-jaq.txt HTML log: http://staging-storage.kernelci.org/gtucker/kernelci-media/gtucker-kernelci-media-002-2-gaa27eb0392c7/arm/multi_v7_defconfig/lab-collab ora-dev/v4l2-rk3288-veyron-jaq.html Rootfs: http://staging-storage.kernelci.org/images/rootfs/debian/stretchv4l2/20181017.1//armhf/rootfs.cpio.gz Test Git:git://linuxtv.org/v4l-utils.git Test Commit: e889b0d56757b9a74eb8c4c8cc2ebd5b18e228b2 Test cases: * MMAP: FAIL * blocking-wait: PASS * read/write: SKIP * read/write: SKIP * read/write: SKIP * VIDIOC_EXPBUF: PASS *
[PATCH] media: Rename vb2_m2m_request_queue -> v4l2_m2m_request_queue
To be consistent with the rest of the mem2mem helpers,
rename vb2_m2m_request_queue to v4l2_m2m_request_queue.
This is just a cosmetic change.
Signed-off-by: Ezequiel Garcia
---
drivers/media/platform/vim2m.c | 2 +-
drivers/media/v4l2-core/v4l2-mem2mem.c | 4 ++--
drivers/staging/media/sunxi/cedrus/cedrus.c | 2 +-
include/media/v4l2-mem2mem.h| 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index af150a0395df..d82db738f174 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -1009,7 +1009,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
static const struct media_device_ops m2m_media_ops = {
.req_validate = vb2_request_validate,
- .req_queue = vb2_m2m_request_queue,
+ .req_queue = v4l2_m2m_request_queue,
};
static int vim2m_probe(struct platform_device *pdev)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index d7806db222d8..1ed2465972ac 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -953,7 +953,7 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx,
}
EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
-void vb2_m2m_request_queue(struct media_request *req)
+void v4l2_m2m_request_queue(struct media_request *req)
{
struct media_request_object *obj, *obj_safe;
struct v4l2_m2m_ctx *m2m_ctx = NULL;
@@ -997,7 +997,7 @@ void vb2_m2m_request_queue(struct media_request *req)
if (m2m_ctx)
v4l2_m2m_try_schedule(m2m_ctx);
}
-EXPORT_SYMBOL_GPL(vb2_m2m_request_queue);
+EXPORT_SYMBOL_GPL(v4l2_m2m_request_queue);
/* Videobuf2 ioctl helpers */
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 82558455384a..dd121f66fa2d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -253,7 +253,7 @@ static const struct v4l2_m2m_ops cedrus_m2m_ops = {
static const struct media_device_ops cedrus_m2m_media_ops = {
.req_validate = cedrus_request_validate,
- .req_queue = vb2_m2m_request_queue,
+ .req_queue = v4l2_m2m_request_queue,
};
static int cedrus_probe(struct platform_device *pdev)
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 58c1ecf3d648..5467264771ec 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -624,7 +624,7 @@ v4l2_m2m_dst_buf_remove_by_idx(struct v4l2_m2m_ctx
*m2m_ctx, unsigned int idx)
/* v4l2 request helper */
-void vb2_m2m_request_queue(struct media_request *req);
+void v4l2_m2m_request_queue(struct media_request *req);
/* v4l2 ioctl helpers */
--
2.19.1
[PATCH v5 1/5] mem2mem: Require capture and output mutexes to match
Currently, all the mem2mem driver either use a single mutex
to lock the capture and output videobuf2 queues, or don't
set any mutex.
This means the mutexes match, and so the mem2mem framework
is able to set the m2m context lock.
Enforce this by making it mandatory for drivers to set
the same capture and output mutex, or not set any mutex at all.
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index d7806db222d8..bc72243eb91d 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -908,12 +908,14 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct
v4l2_m2m_dev *m2m_dev,
if (ret)
goto err;
/*
-* If both queues use same mutex assign it as the common buffer
-* queues lock to the m2m context. This lock is used in the
-* v4l2_m2m_ioctl_* helpers.
+* Both queues should use same the mutex to lock the m2m context.
+* This lock is used in some v4l2_m2m_* helpers.
*/
- if (out_q_ctx->q.lock == cap_q_ctx->q.lock)
- m2m_ctx->q_lock = out_q_ctx->q.lock;
+ if (WARN_ON(out_q_ctx->q.lock != cap_q_ctx->q.lock)) {
+ ret = -EINVAL;
+ goto err;
+ }
+ m2m_ctx->q_lock = out_q_ctx->q.lock;
return m2m_ctx;
err:
--
2.19.1
[PATCH v5 5/5] media: cedrus: Get rid of interrupt bottom-half
Now that the mem2mem framework guarantees that .device_run
won't be called from interrupt context, it is safe to call
v4l2_m2m_job_finish directly in the top-half.
So this means the bottom-half is no longer needed and we
can get rid of it.
Signed-off-by: Ezequiel Garcia
---
.../staging/media/sunxi/cedrus/cedrus_hw.c| 26 ---
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index 32adbcbe6175..493e65b17b30 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -98,23 +98,6 @@ void cedrus_dst_format_set(struct cedrus_dev *dev,
}
}
-static irqreturn_t cedrus_bh(int irq, void *data)
-{
- struct cedrus_dev *dev = data;
- struct cedrus_ctx *ctx;
-
- ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
- if (!ctx) {
- v4l2_err(>v4l2_dev,
-"Instance released before the end of transaction\n");
- return IRQ_HANDLED;
- }
-
- v4l2_m2m_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx);
-
- return IRQ_HANDLED;
-}
-
static irqreturn_t cedrus_irq(int irq, void *data)
{
struct cedrus_dev *dev = data;
@@ -165,7 +148,9 @@ static irqreturn_t cedrus_irq(int irq, void *data)
spin_unlock_irqrestore(>irq_lock, flags);
- return IRQ_WAKE_THREAD;
+ v4l2_m2m_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx);
+
+ return IRQ_HANDLED;
}
int cedrus_hw_probe(struct cedrus_dev *dev)
@@ -187,9 +172,8 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
return irq_dec;
}
- ret = devm_request_threaded_irq(dev->dev, irq_dec, cedrus_irq,
- cedrus_bh, 0, dev_name(dev->dev),
- dev);
+ ret = devm_request_irq(dev->dev, irq_dec, cedrus_irq,
+ 0, dev_name(dev->dev), dev);
if (ret) {
v4l2_err(>v4l2_dev, "Failed to request IRQ\n");
--
2.19.1
[PATCH v5 3/5] v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule
From: Sakari Ailus
The __v4l2_m2m_try_schedule function acquires and releases multiple
spinlocks. Simplify unlocking the job lock by adding labels to unlock
the lock and exit the function.
Signed-off-by: Sakari Ailus
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 29 --
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index bc72243eb91d..0665a97ed89e 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -297,51 +297,48 @@ static void __v4l2_m2m_try_queue(struct v4l2_m2m_dev
*m2m_dev,
/* If the context is aborted then don't schedule it */
if (m2m_ctx->job_flags & TRANS_ABORT) {
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("Aborted context\n");
- return;
+ goto job_unlock;
}
if (m2m_ctx->job_flags & TRANS_QUEUED) {
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("On job queue already\n");
- return;
+ goto job_unlock;
}
spin_lock_irqsave(_ctx->out_q_ctx.rdy_spinlock, flags_out);
if (list_empty(_ctx->out_q_ctx.rdy_queue)
&& !m2m_ctx->out_q_ctx.buffered) {
- spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock,
- flags_out);
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("No input buffers available\n");
- return;
+ goto out_unlock;
}
spin_lock_irqsave(_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
if (list_empty(_ctx->cap_q_ctx.rdy_queue)
&& !m2m_ctx->cap_q_ctx.buffered) {
- spin_unlock_irqrestore(_ctx->cap_q_ctx.rdy_spinlock,
- flags_cap);
- spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock,
- flags_out);
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("No output buffers available\n");
- return;
+ goto cap_unlock;
}
spin_unlock_irqrestore(_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock, flags_out);
if (m2m_dev->m2m_ops->job_ready
&& (!m2m_dev->m2m_ops->job_ready(m2m_ctx->priv))) {
- spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
dprintk("Driver not ready\n");
- return;
+ goto job_unlock;
}
list_add_tail(_ctx->queue, _dev->job_queue);
m2m_ctx->job_flags |= TRANS_QUEUED;
spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
+ return;
+
+cap_unlock:
+ spin_unlock_irqrestore(_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
+out_unlock:
+ spin_unlock_irqrestore(_ctx->out_q_ctx.rdy_spinlock, flags_out);
+job_unlock:
+ spin_unlock_irqrestore(_dev->job_spinlock, flags_job);
}
/**
--
2.19.1
[PATCH v5 2/5] v4l2-ioctl.c: Simplify locking for m2m devices
Now that the mutexes for output and capture vb2 queues match,
it is possible to refer to the context q_lock as the
m2m lock for a given m2m context.
Remove the output/capture lock selection.
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-ioctl.c | 47 ++--
1 file changed, 2 insertions(+), 45 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index c63746968fa3..54919c227bc1 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2679,45 +2679,6 @@ static bool v4l2_is_known_ioctl(unsigned int cmd)
return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd;
}
-#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
-static bool v4l2_ioctl_m2m_queue_is_output(unsigned int cmd, void *arg)
-{
- switch (cmd) {
- case VIDIOC_CREATE_BUFS: {
- struct v4l2_create_buffers *cbufs = arg;
-
- return V4L2_TYPE_IS_OUTPUT(cbufs->format.type);
- }
- case VIDIOC_REQBUFS: {
- struct v4l2_requestbuffers *rbufs = arg;
-
- return V4L2_TYPE_IS_OUTPUT(rbufs->type);
- }
- case VIDIOC_QBUF:
- case VIDIOC_DQBUF:
- case VIDIOC_QUERYBUF:
- case VIDIOC_PREPARE_BUF: {
- struct v4l2_buffer *buf = arg;
-
- return V4L2_TYPE_IS_OUTPUT(buf->type);
- }
- case VIDIOC_EXPBUF: {
- struct v4l2_exportbuffer *expbuf = arg;
-
- return V4L2_TYPE_IS_OUTPUT(expbuf->type);
- }
- case VIDIOC_STREAMON:
- case VIDIOC_STREAMOFF: {
- int *type = arg;
-
- return V4L2_TYPE_IS_OUTPUT(*type);
- }
- default:
- return false;
- }
-}
-#endif
-
static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev,
struct v4l2_fh *vfh, unsigned int cmd,
void *arg)
@@ -2727,12 +2688,8 @@ static struct mutex *v4l2_ioctl_get_lock(struct
video_device *vdev,
#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
if (vfh && vfh->m2m_ctx &&
(v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE)) {
- bool is_output = v4l2_ioctl_m2m_queue_is_output(cmd, arg);
- struct v4l2_m2m_queue_ctx *ctx = is_output ?
- >m2m_ctx->out_q_ctx : >m2m_ctx->cap_q_ctx;
-
- if (ctx->q.lock)
- return ctx->q.lock;
+ if (vfh->m2m_ctx->q_lock)
+ return vfh->m2m_ctx->q_lock;
}
#endif
if (vdev->queue && vdev->queue->lock &&
--
2.19.1
[PATCH v5 4/5] v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish
v4l2_m2m_job_finish() is typically called when
DMA operations complete, in interrupt handlers or DMA
completion callbacks. Calling .device_run from v4l2_m2m_job_finish
creates a nasty re-entrancy path into the driver.
Moreover, some implementation of .device_run might need to sleep,
as is the case for drivers supporting the Request API,
where controls are applied via v4l2_ctrl_request_setup,
which takes the ctrl handler mutex.
This commit adds a deferred context that calls v4l2_m2m_try_run,
and gets scheduled by v4l2_m2m_job_finish().
Before this change, device_run would be called from these
paths:
vb2_m2m_request_queue, or
v4l2_m2m_streamon, or
v4l2_m2m_qbuf
v4l2_m2m_try_schedule
v4l2_m2m_try_run
.device_run
v4l2_m2m_job_finish
v4l2_m2m_try_run
.device_run
After this change, the latter is now gone and instead:
v4l2_m2m_device_run_work
v4l2_m2m_try_run
.device_run
Signed-off-by: Ezequiel Garcia
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 0665a97ed89e..2b250fd10531 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -87,6 +87,7 @@ static const char * const m2m_entity_name[] = {
* @curr_ctx: currently running instance
* @job_queue: instances queued to run
* @job_spinlock: protects job_queue
+ * @job_work: worker to run queued jobs.
* @m2m_ops: driver callbacks
*/
struct v4l2_m2m_dev {
@@ -103,6 +104,7 @@ struct v4l2_m2m_dev {
struct list_headjob_queue;
spinlock_t job_spinlock;
+ struct work_struct job_work;
const struct v4l2_m2m_ops *m2m_ops;
};
@@ -244,6 +246,9 @@ EXPORT_SYMBOL(v4l2_m2m_get_curr_priv);
* @m2m_dev: per-device context
*
* Get next transaction (if present) from the waiting jobs list and run it.
+ *
+ * Note that this function can run on a given v4l2_m2m_ctx context,
+ * but call .device_run for another context.
*/
static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
{
@@ -362,6 +367,18 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
}
EXPORT_SYMBOL_GPL(v4l2_m2m_try_schedule);
+/**
+ * v4l2_m2m_device_run_work() - run pending jobs for the context
+ * @work: Work structure used for scheduling the execution of this function.
+ */
+static void v4l2_m2m_device_run_work(struct work_struct *work)
+{
+ struct v4l2_m2m_dev *m2m_dev =
+ container_of(work, struct v4l2_m2m_dev, job_work);
+
+ v4l2_m2m_try_run(m2m_dev);
+}
+
/**
* v4l2_m2m_cancel_job() - cancel pending jobs for the context
* @m2m_ctx: m2m context with jobs to be canceled
@@ -421,7 +438,12 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
/* This instance might have more buffers ready, but since we do not
* allow more than one job on the job_queue per instance, each has
* to be scheduled separately after the previous one finishes. */
- v4l2_m2m_try_schedule(m2m_ctx);
+ __v4l2_m2m_try_queue(m2m_dev, m2m_ctx);
+
+ /* We might be running in atomic context,
+* but the job must be run in non-atomic context.
+*/
+ schedule_work(_dev->job_work);
}
EXPORT_SYMBOL(v4l2_m2m_job_finish);
@@ -863,6 +885,7 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct
v4l2_m2m_ops *m2m_ops)
m2m_dev->m2m_ops = m2m_ops;
INIT_LIST_HEAD(_dev->job_queue);
spin_lock_init(_dev->job_spinlock);
+ INIT_WORK(_dev->job_work, v4l2_m2m_device_run_work);
return m2m_dev;
}
--
2.19.1
[PATCH v5 0/5] Make sure .device_run is always called in non-atomic context
This series goal is to avoid drivers from having ad-hoc code to call .device_run in non-atomic context. Currently, .device_run can be called via v4l2_m2m_job_finish(), not only running in interrupt context, but also creating a nasty re-entrant path into mem2mem drivers. The proposed solution is to add a per-device worker that is scheduled by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt or similar. This change allows v4l2_m2m_job_finish() to be called in interrupt context, separating .device_run and v4l2_m2m_job_finish() contexts. It's worth mentioning that v4l2_m2m_cancel_job() doesn't need to flush or cancel the new worker, because the job_spinlock synchronizes both and also because the core prevents simultaneous jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the worker will be unable to run a new job. Patches apply on top of Request API and the Cedrus VPU driver. Tested with cedrus driver using v4l2-request-test and vicodec driver using gstreamer. Ezequiel Garcia (4): mem2mem: Require capture and output mutexes to match v4l2-ioctl.c: Simplify locking for m2m devices v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish media: cedrus: Get rid of interrupt bottom-half Sakari Ailus (1): v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule drivers/media/v4l2-core/v4l2-ioctl.c | 47 + drivers/media/v4l2-core/v4l2-mem2mem.c| 66 --- .../staging/media/sunxi/cedrus/cedrus_hw.c| 26 ++-- 3 files changed, 51 insertions(+), 88 deletions(-) -- 2.19.1
Re: i.MX6 IPU CSI analog video input on Ventana
On Wed, Oct 17, 2018 at 4:37 PM Steve Longerbeam wrote: > > > On 10/17/18 4:05 PM, Tim Harvey wrote: > > On Wed, Oct 17, 2018 at 2:33 PM Steve Longerbeam > > wrote: > >> Hi Tim, > >> > >> On 10/17/18 1:38 PM, Tim Harvey wrote: > >> > >> On Mon, Jun 4, 2018 at 1:58 AM Krzysztof Hałasa wrote: > >> > >> I've just tested the PAL setup: in currect situation (v4.17 + Steve's > >> fix-csi-interlaced.2 + "media: adv7180: fix field type" + a small cheap > >> PAL camera) the following produces bottom-first interlaced frames: > >> > >> media-ctl -r -l '"adv7180 2-0020":0->"ipu2_csi1_mux":1[1], > >> "ipu2_csi1_mux":2->"ipu2_csi1":0[1], > >> "ipu2_csi1":2->"ipu2_csi1 capture":0[1]' > >> > >> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x576 field:alternate]" > >> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x576]" > >> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x576 field:interlaced]" > >> > >> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1_mux":1 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1_mux":2 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1":0 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1":2 [fmt:AYUV32/720x576 field:interlaced] > >> > >> I think it would be great if these changes make their way upstream. > >> The details could be refined then. > >> > >> Krzysztof / Steve / Philipp, > >> > >> I jumped back onto IMX6 video capture from the adv7180 the other day > >> trying to help out a customer that's using mainline and found things > >> are still not working right. Where is all of this at these days? > >> > >> If I use v4.19 with Steves 'imx-media: Fixes for interlaced capture' > >> v3 series (https://patchwork.kernel.org/cover/10626499/) I > >> rolling/split (un-synchronized) video using: > >> > >> # Setup links > >> media-ctl -r > >> media-ctl -l '"adv7180 2-0020":0 -> "ipu2_csi1_mux":1[1]' > >> media-ctl -l '"ipu2_csi1_mux":2 -> "ipu2_csi1":0[1]' > >> media-ctl -l '"ipu2_csi1":1 -> "ipu2_ic_prp":0[1]' > >> media-ctl -l '"ipu2_ic_prp":2 -> "ipu2_ic_prpvf":0[1]' > >> media-ctl -l '"ipu2_ic_prpvf":1 -> "ipu2_ic_prpvf capture":0[1]' > >> # Configure pads > >> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480]" > >> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]" > >> media-ctl -V "'ipu2_csi1':1 [fmt:UYVY2X8/720x480 field:interlaced]" > >> media-ctl -V "'ipu2_ic_prp':2 [fmt:UYVY2X8/720x480 field:interlaced]" > >> media-ctl -V "'ipu2_ic_prpvf':1 [fmt:UYVY2X8/720x480 field:none]" > >> # stream JPEG/RTP/UDP > >> gst-launch-1.0 v4l2src device=/dev/video3 ! video/x-raw,format=UYVY ! > >> jpegenc ! rtpjpegpay ! udpsink host=$SERVER port=$PORT > >> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Device > >> '/dev/video3' does not support progressive interlacing > >> > >> I'm doing the above on a Gateworks GW5404 IMXQ which has a tda1997x > >> HDMI receiver sensor and an adv7180 Analog CVBS sensor - media graph > >> is here: http://dev.gateworks.com/docs/linux/media/imx6q-gw54xx-media.png > >> > >> Are there other patches I need or different field formats above with > >> 4.19? Do any of the other kernels work without patchsets that you know > >> of between 4.16 and 4.19? > >> > >> > >> First, the v3 series is out of date. Please apply the latest v5 posting > >> of that series. See the imx.rst doc regarding field type negotiation, > >> all pads starting at ipu2_csi1:1 should be 'seq-bt' or 'seq-tb' until the > >> capture device, which should be set to 'interlaced' to enable IDMAC > >> interweave. The ADV7180 now correctly sets its field type to alternate, > >> which imx-media-csi.c translates to seq-tb or seq-bt at its output pad. > >> > >> See the SabreAuto examples in the doc. > >> > >> For the rolling/split image problem, try the attached somewhat hackish > >> patch. > >> There used to be code in imx-media-csi.c that searched for the backend > >> sensor > >> and queries via .g_skip_frames whether the sensor produces bad frames at > >> first > >> stream-on. But there was push-back on that, so the attached is another > >> approach that doesn't require searching for a backend sensor. > > Steve, > > > > Thanks - I hadn't noticed the updated series. I've built it on top of > > linux-media/master and tested with: > > > > - Testing linux-media/master + your v5 now: > > > > # Use simple interweaving > > media-ctl -r > > # Setup links > > media-ctl -l '"adv7180 2-0020":0 -> "ipu2_csi1_mux":1[1]' > > media-ctl -l '"ipu2_csi1_mux":2 -> "ipu2_csi1":0[1]' > > media-ctl -l '"ipu2_csi1":2 -> "ipu2_csi1 capture":0[1]' > > # Configure pads > > media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]" > > media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]" > > media-ctl -V "'ipu2_csi1':1 [fmt:AYUV32/720x480]" > > # Configure ipu_csi capture interface (/dev/video7) > > v4l2-ctl -d7 --set-fmt-video=field=interlaced_bt > > # Stream JPEG/RTP/UDP > > gst-launch-1.0 v4l2src
Images 34
We are an image team who can process 400+ images each day. If you need any image editing service, please let us know. Image cut out and clipping path, masking. Such as for ecommerce photos, jewelry photos retouching, beauty and skin images and wedding photos. We give test editing for your photos if you send us some. Thanks, Nancy
Re: [RFC PATCH] media: ov5640: calculate PLL settings for modes
Hi Sam,
some comments and questions.
I still hope we find a way to merge our changes, but I need to
understand something before.
On Wed, Oct 17, 2018 at 10:31:48AM -0700, Sam Bobrowicz wrote:
> Remove the PLL settings from the register blobs and
> calculate them based on required clocks. This allows
> more mode and input clock (xclk) configurations.
>
> Also ensure that PCLK PERIOD register 0x4837 is set
> so that MIPI receivers are not broken by this patch.
>
> Last, a change to the init register blob that helps
> ensure the following DPHY spec requirement is met:
>
> MIN HS_ZERO + MIN HS_PREPARE > 145 + t_UI * 10
>
> Signed-off-by: Sam Bobrowicz
> ---
>
> This is a modified version of Maxime's patch that works on my platform.
> My platform is a dual-lane MIPI CSI2 module with xclk=24MHz connected
> to a Xilinx Zynq Ultrascale+.
>
> One issue I am currently experiencing with this patch is that some
> 15Hz framerates are not working. This seems to be due to the slower
> clocks which are generated, and may be caused by the large ADC clock
> to SCLK ratio. I will be exploring some fixes this weekend. Thoughts on
> this would be appreciated.
>
> I am submitting this so that it can be compared to Maxime's, which has
> been reported to not be functional on MIPI platforms. I do a number of
> things differently, and I hope that those which are useful will be
> integrated into his patch.
>
> I think this patch (or the modified version of Maxime's patch) should
> be tested under the following conditions:
>
> 1) MIPI mode, xclk=24MHz
> 2) MIPI mode, xclk!=24MHz
> 3) DVP mode
> 4) JPEG format
>
> I'm setup to test the first two, but don't have the hardware/software
> to test 3 and 4.
I could test with 1) and 2) with xvclk at 26MHz
>
> This patch is based on the current master of media_linux
> "media: ov5640: fix framerate update".
>
> drivers/media/i2c/ov5640.c | 332
> ++---
> 1 file changed, 281 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..c076955 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -85,6 +85,7 @@
> #define OV5640_REG_POLARITY_CTRL00 0x4740
> #define OV5640_REG_MIPI_CTRL00 0x4800
> #define OV5640_REG_DEBUG_MODE0x4814
> +#define OV5640_REG_PCLK_PERIOD 0x4837
> #define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
> #define OV5640_REG_PRE_ISP_TEST_SET1 0x503d
> #define OV5640_REG_SDE_CTRL0 0x5580
> @@ -94,9 +95,6 @@
> #define OV5640_REG_SDE_CTRL5 0x5585
> #define OV5640_REG_AVG_READOUT 0x56a1
>
> -#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT 1
> -#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT 2
> -
> enum ov5640_mode_id {
> OV5640_MODE_QCIF_176_144 = 0,
> OV5640_MODE_QVGA_320_240,
> @@ -171,6 +169,7 @@ struct reg_value {
> struct ov5640_mode_info {
> enum ov5640_mode_id id;
> enum ov5640_downsize_mode dn_mode;
> + bool scaler; /* Mode uses ISP scaler (reg 0x5001,BIT(5)=='1') */
> u32 hact;
> u32 htot;
> u32 vact;
> @@ -291,7 +290,7 @@ static const struct reg_value
> ov5640_init_setting_30fps_VGA[] = {
> {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> {0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
> {0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
> - {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0},
> + {0x3824, 0x02, 0, 0}, {0x482a, 0x06, 0, 0},
> {0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
> {0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
> {0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
> @@ -345,7 +344,7 @@ static const struct reg_value
> ov5640_init_setting_30fps_VGA[] = {
> };
>
> static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
> - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
> + {0x3c07, 0x08, 0, 0},
> {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> {0x3814, 0x31, 0, 0},
> {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> @@ -364,7 +363,7 @@ static const struct reg_value
> ov5640_setting_30fps_VGA_640_480[] = {
> };
>
> static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
> - {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
> + {0x3c07, 0x08, 0, 0},
> {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> {0x3814, 0x31, 0, 0},
> {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> @@ -383,7 +382,7 @@ static const struct reg_value
> ov5640_setting_15fps_VGA_640_480[] = {
> };
>
> static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
> - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
> + {0x3c07, 0x08, 0, 0},
> {0x3c09,
Photos 39
We are an image team who can process 400+ images each day. If you need any image editing service, please let us know. Image cut out and clipping path, masking. Such as for ecommerce photos, jewelry photos retouching, beauty and skin images and wedding photos. We give test editing for your photos if you send us some. Thanks, Nancy
[PATCH 0/2] vicodec: a couple fixes towards spec compliancy
Given the stateful codec specification is still a moving target, it doesn't make any sense to try to comply fully with it. On the other side, we can still comply with some basic userspace expectations, with just a couple small changes. This series implements proper resolution changes propagation, and fixes the CMD_STOP so it actually works. The intention of this series is to be able to test this driver using already existing userspace, gstreamer in particular. With this changes, it's possible to construct variations of this pipeline: gst-launch-1.0 videotestsrc ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink Ezequiel Garcia (2): vicodec: Have decoder propagate changes to the CAPTURE queue vicodec: Implement spec-compliant stop command drivers/media/platform/vicodec/vicodec-core.c | 95 --- 1 file changed, 59 insertions(+), 36 deletions(-) -- 2.19.1
[PATCH 1/2] vicodec: Have decoder propagate changes to the CAPTURE queue
The decoder interface (not yet merged) specifies that
width and height values set on the OUTPUT queue, must
be propagated to the CAPTURE queue.
This is not enough to comply with the specification,
which would require to properly support stream resolution
changes detection and notification.
However, it's a relatively small change, which fixes behavior
required by some applications such as gstreamer.
With this change, it's possible to run a simple T(T⁻¹) pipeline:
gst-launch-1.0 videotestsrc ! v4l2fwhtenc ! v4l2fwhtdec ! fakevideosink
Signed-off-by: Ezequiel Garcia
---
drivers/media/platform/vicodec/vicodec-core.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/media/platform/vicodec/vicodec-core.c
b/drivers/media/platform/vicodec/vicodec-core.c
index 1eb9132bfc85..a2c487b4b80d 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -673,6 +673,13 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct
v4l2_format *f)
q_data->width = pix->width;
q_data->height = pix->height;
q_data->sizeimage = pix->sizeimage;
+
+ /* Propagate changes to CAPTURE queue */
+ if (!ctx->is_enc && V4L2_TYPE_IS_OUTPUT(f->type)) {
+ ctx->q_data[V4L2_M2M_DST].width = pix->width;
+ ctx->q_data[V4L2_M2M_DST].height = pix->height;
+ ctx->q_data[V4L2_M2M_DST].sizeimage = pix->sizeimage;
+ }
break;
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
@@ -693,6 +700,14 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct
v4l2_format *f)
q_data->width = pix_mp->width;
q_data->height = pix_mp->height;
q_data->sizeimage = pix_mp->plane_fmt[0].sizeimage;
+
+ /* Propagate changes to CAPTURE queue */
+ if (!ctx->is_enc && V4L2_TYPE_IS_OUTPUT(f->type)) {
+ ctx->q_data[V4L2_M2M_DST].width = pix_mp->width;
+ ctx->q_data[V4L2_M2M_DST].height = pix_mp->height;
+ ctx->q_data[V4L2_M2M_DST].sizeimage =
+ pix_mp->plane_fmt[0].sizeimage;
+ }
break;
default:
return -EINVAL;
--
2.19.1
[PATCH 2/2] vicodec: Implement spec-compliant stop command
Set up a statically-allocated, dummy buffer to
be used as flush buffer, which signals
a encoding (or decoding) stop.
When the flush buffer is queued to the OUTPUT queue,
the driver will send an V4L2_EVENT_EOS event, and
mark the CAPTURE buffer with V4L2_BUF_FLAG_LAST.
With this change, it's possible to run a pipeline to completion:
gst-launch-1.0 videotestsrc num-buffers=10 ! v4l2fwhtenc ! v4l2fwhtdec !
fakevideosink
Signed-off-by: Ezequiel Garcia
---
drivers/media/platform/vicodec/vicodec-core.c | 80 ++-
1 file changed, 44 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/vicodec/vicodec-core.c
b/drivers/media/platform/vicodec/vicodec-core.c
index a2c487b4b80d..4ed4dae10e30 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -113,7 +113,7 @@ struct vicodec_ctx {
struct v4l2_ctrl_handler hdl;
struct vb2_v4l2_buffer *last_src_buf;
- struct vb2_v4l2_buffer *last_dst_buf;
+ struct vb2_v4l2_buffer flush_buf;
/* Source and destination queue data */
struct vicodec_q_data q_data[2];
@@ -220,6 +220,7 @@ static void device_run(void *priv)
struct vicodec_dev *dev = ctx->dev;
struct vb2_v4l2_buffer *src_buf, *dst_buf;
struct vicodec_q_data *q_out;
+ bool flushing;
u32 state;
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
@@ -227,26 +228,36 @@ static void device_run(void *priv)
q_out = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
state = VB2_BUF_STATE_DONE;
- if (device_process(ctx, src_buf, dst_buf))
+
+ flushing = (src_buf == >flush_buf);
+ if (!flushing && device_process(ctx, src_buf, dst_buf))
state = VB2_BUF_STATE_ERROR;
- ctx->last_dst_buf = dst_buf;
spin_lock(ctx->lock);
- if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
+ if (!flushing) {
+ if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
+ dst_buf->flags |= V4L2_BUF_FLAG_LAST;
+ v4l2_event_queue_fh(>fh, _event);
+ }
+
+ if (ctx->is_enc) {
+ src_buf->sequence = q_out->sequence++;
+ src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+ v4l2_m2m_buf_done(src_buf, state);
+ } else if (vb2_get_plane_payload(_buf->vb2_buf, 0)
+ == ctx->cur_buf_offset) {
+ src_buf->sequence = q_out->sequence++;
+ src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+ v4l2_m2m_buf_done(src_buf, state);
+ ctx->cur_buf_offset = 0;
+ ctx->comp_has_next_frame = false;
+ }
+ } else {
+ src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+ vb2_set_plane_payload(_buf->vb2_buf, 0, 0);
dst_buf->flags |= V4L2_BUF_FLAG_LAST;
v4l2_event_queue_fh(>fh, _event);
}
- if (ctx->is_enc) {
- src_buf->sequence = q_out->sequence++;
- src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
- v4l2_m2m_buf_done(src_buf, state);
- } else if (vb2_get_plane_payload(_buf->vb2_buf, 0) ==
ctx->cur_buf_offset) {
- src_buf->sequence = q_out->sequence++;
- src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
- v4l2_m2m_buf_done(src_buf, state);
- ctx->cur_buf_offset = 0;
- ctx->comp_has_next_frame = false;
- }
v4l2_m2m_buf_done(dst_buf, state);
ctx->comp_size = 0;
ctx->comp_magic_cnt = 0;
@@ -293,6 +304,8 @@ static int job_ready(void *priv)
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
if (!src_buf)
return 0;
+ if (src_buf == >flush_buf)
+ return 1;
p_out = vb2_plane_vaddr(_buf->vb2_buf, 0);
sz = vb2_get_plane_payload(_buf->vb2_buf, 0);
p = p_out + ctx->cur_buf_offset;
@@ -770,21 +783,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void
*priv,
return ret;
}
-static void vicodec_mark_last_buf(struct vicodec_ctx *ctx)
-{
- static const struct v4l2_event eos_event = {
- .type = V4L2_EVENT_EOS
- };
-
- spin_lock(ctx->lock);
- ctx->last_src_buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
- if (!ctx->last_src_buf && ctx->last_dst_buf) {
- ctx->last_dst_buf->flags |= V4L2_BUF_FLAG_LAST;
- v4l2_event_queue_fh(>fh, _event);
- }
- spin_unlock(ctx->lock);
-}
-
static int vicodec_try_encoder_cmd(struct file *file, void *fh,
struct v4l2_encoder_cmd *ec)
{
@@ -806,8 +804,8 @@ static int vicodec_encoder_cmd(struct file *file, void *fh,
ret =
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
Hello Maxime,
On Thu, Oct 18, 2018 at 11:29:12AM +0200, Maxime Ripard wrote:
> Hi Jacopo,
>
> Thanks for reviewing this patch
>
> On Tue, Oct 16, 2018 at 06:54:50PM +0200, jacopo mondi wrote:
> > > +static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
> > > + u8 pll_prediv, u8 pll_mult,
> > > + u8 sysdiv)
> > > +{
> > > + unsigned long rate = clk_get_rate(sensor->xclk);
> >
> > The clock rate is stored in sensor->xclk at probe time, no need to
> > query it every iteration.
>
> From a clk API point of view though, there's nothing that guarantees
> that the clock rate hasn't changed between the probe and the time
> where this function is called.
Correct, bell, it can be queried in the caller and re-used here :)
>
> I appreciate that we're probably connected to an oscillator, but even
> then, on the Allwinner SoCs we've had the issue recently that one
> oscillator feeding the BT chip was actually had a muxer, with each
> option having a slightly different rate, which was bad enough for the
> BT chip to be non-functional.
>
> I can definitely imagine the same case happening here for some
> SoCs. Plus, the clock framework will cache the rate as well when
> possible, so we're not losing anything here.
I see, so please ignore this comment :)
>
> > > +
> > > + return rate / pll_prediv * pll_mult / sysdiv;
> > > +}
> > > +
> > > +static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
> > > + unsigned long rate,
> > > + u8 *pll_prediv, u8 *pll_mult,
> > > + u8 *sysdiv)
> > > +{
> > > + unsigned long best = ~0;
> > > + u8 best_sysdiv = 1, best_mult = 1;
> > > + u8 _sysdiv, _pll_mult;
> > > +
> > > + for (_sysdiv = OV5640_SYSDIV_MIN;
> > > + _sysdiv <= OV5640_SYSDIV_MAX;
> > > + _sysdiv++) {
> > > + for (_pll_mult = OV5640_PLL_MULT_MIN;
> > > + _pll_mult <= OV5640_PLL_MULT_MAX;
> > > + _pll_mult++) {
> > > + unsigned long _rate;
> > > +
> > > + /*
> > > + * The PLL multiplier cannot be odd if above
> > > + * 127.
> > > + */
> > > + if (_pll_mult > 127 && (_pll_mult % 2))
> > > + continue;
> > > +
> > > + _rate = ov5640_compute_sys_clk(sensor,
> > > +OV5640_PLL_PREDIV,
> > > +_pll_mult, _sysdiv);
> >
> > I'm under the impression a system clock slower than the requested one, even
> > if more accurate is not good.
> >
> > I'm still working on understanding how all CSI-2 related timing
> > parameters play together, but since the system clock is calculated
> > from the pixel clock (which comes from the frame dimensions, bpp, and
> > rate), and it is then used to calculate the MIPI BIT clock frequency,
> > I think it would be better to be a little faster than a bit slower,
> > otherwise the serial lane clock wouldn't be fast enough to output
> > frames generated by the sensor core (or maybe it would just decrease
> > the frame rate and that's it, but I don't think it is just this).
> >
> > What do you think of adding the following here:
> >
> > if (_rate < rate)
> > continue
>
> I really don't know MIPI-CSI2 enough to be able to comment on your
> concerns, but when reaching the end of the operating limit of the
> clock, it would prevent us from having any rate at all, which seems
> bad too.
Are you referring to the 1GHz limit of the (xvlkc / pre_div * mult)
output here? If that's your concern we should adjust the requested
SYSCLK rate then (and I added a check for that in my patches on top of
yours, but it could be improved to be honest, as it just refuses the
current rate, while it should increment the pre_divider instead, now
that I think better about that).
>
> > > + if (abs(rate - _rate) < abs(rate - best)) {
> > > + best = _rate;
> > > + best_sysdiv = _sysdiv;
> > > + best_mult = _pll_mult;
> > > + }
> > > +
> > > + if (_rate == rate)
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > +out:
> > > + *sysdiv = best_sysdiv;
> > > + *pll_prediv = OV5640_PLL_PREDIV;
> > > + *pll_mult = best_mult;
> > > + return best;
> > > +}
> >
> > These function gets called at s_stream time, and cycle for a while,
> > and I'm under the impression the MIPI state machine doesn't like
> > delays too much, as I see timeouts on the receiver side.
> >
> > I have tried to move this function at set_fmt() time, every time a new
> > mode is selected, sysdiv, pll_prediv and pll_mult gets recalculated
> > (and stored in the ov5640_dev structure). I now have other timeouts on
> >
Re: [PATCH] media: rc: XBox DVD Remote uses 12 bits scancodes
Tested-by: Benjamin Valentin
On Thu, 18 Oct 2018 12:27:17 +0100
Sean Young wrote:
> The xbox dvd remote sends 24 bits, the first 12 bits are repeated
> and inverted so only 12 bits are used. The upper 4 bits can be read
> at offset 3. Ensure we pass this to rc-core and update the keymap
> accordingly.
>
> Signed-off-by: Sean Young
> ---
> drivers/media/rc/keymaps/rc-xbox-dvd.c | 58
> +- drivers/media/rc/xbox_remote.c |
> 7 ++-- 2 files changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/rc/keymaps/rc-xbox-dvd.c
> b/drivers/media/rc/keymaps/rc-xbox-dvd.c index
> 61da6706715c..af387244636b 100644 ---
> a/drivers/media/rc/keymaps/rc-xbox-dvd.c +++
> b/drivers/media/rc/keymaps/rc-xbox-dvd.c @@ -7,35 +7,35 @@
>
> /* based on lircd.conf.xbox */
> static struct rc_map_table xbox_dvd[] = {
> - {0x0b, KEY_OK},
> - {0xa6, KEY_UP},
> - {0xa7, KEY_DOWN},
> - {0xa8, KEY_RIGHT},
> - {0xa9, KEY_LEFT},
> - {0xc3, KEY_INFO},
> -
> - {0xc6, KEY_9},
> - {0xc7, KEY_8},
> - {0xc8, KEY_7},
> - {0xc9, KEY_6},
> - {0xca, KEY_5},
> - {0xcb, KEY_4},
> - {0xcc, KEY_3},
> - {0xcd, KEY_2},
> - {0xce, KEY_1},
> - {0xcf, KEY_0},
> -
> - {0xd5, KEY_ANGLE},
> - {0xd8, KEY_BACK},
> - {0xdd, KEY_PREVIOUSSONG},
> - {0xdf, KEY_NEXTSONG},
> - {0xe0, KEY_STOP},
> - {0xe2, KEY_REWIND},
> - {0xe3, KEY_FASTFORWARD},
> - {0xe5, KEY_TITLE},
> - {0xe6, KEY_PAUSE},
> - {0xea, KEY_PLAY},
> - {0xf7, KEY_MENU},
> + {0xa0b, KEY_OK},
> + {0xaa6, KEY_UP},
> + {0xaa7, KEY_DOWN},
> + {0xaa8, KEY_RIGHT},
> + {0xaa9, KEY_LEFT},
> + {0xac3, KEY_INFO},
> +
> + {0xac6, KEY_9},
> + {0xac7, KEY_8},
> + {0xac8, KEY_7},
> + {0xac9, KEY_6},
> + {0xaca, KEY_5},
> + {0xacb, KEY_4},
> + {0xacc, KEY_3},
> + {0xacd, KEY_2},
> + {0xace, KEY_1},
> + {0xacf, KEY_0},
> +
> + {0xad5, KEY_ANGLE},
> + {0xad8, KEY_BACK},
> + {0xadd, KEY_PREVIOUSSONG},
> + {0xadf, KEY_NEXTSONG},
> + {0xae0, KEY_STOP},
> + {0xae2, KEY_REWIND},
> + {0xae3, KEY_FASTFORWARD},
> + {0xae5, KEY_TITLE},
> + {0xae6, KEY_PAUSE},
> + {0xaea, KEY_PLAY},
> + {0xaf7, KEY_MENU},
> };
>
> static struct rc_map_list xbox_dvd_map = {
> diff --git a/drivers/media/rc/xbox_remote.c
> b/drivers/media/rc/xbox_remote.c index 141ef9253018..4d41e31369d2
> 100644 --- a/drivers/media/rc/xbox_remote.c
> +++ b/drivers/media/rc/xbox_remote.c
> @@ -55,7 +55,7 @@ struct xbox_remote {
> struct usb_interface *interface;
>
> struct urb *irq_urb;
> - unsigned char inbuf[DATA_BUFSIZE];
> + unsigned char inbuf[DATA_BUFSIZE] __aligned(sizeof(u16));
>
> char rc_name[NAME_BUFSIZE];
> char rc_phys[NAME_BUFSIZE];
> @@ -95,7 +95,7 @@ static void xbox_remote_input_report(struct urb
> *urb)
>* data[0] = 0x00
>* data[1] = length - always 0x06
>* data[2] = the key code
> - * data[3] = high part of key code? - always 0x0a
> + * data[3] = high part of key code
>* data[4] = last_press_ms (low)
>* data[5] = last_press_ms (high)
>*/
> @@ -107,7 +107,8 @@ static void xbox_remote_input_report(struct urb
> *urb) return;
> }
>
> - rc_keydown(xbox_remote->rdev, RC_PROTO_UNKNOWN, data[2], 0);
> + rc_keydown(xbox_remote->rdev, RC_PROTO_UNKNOWN,
> +le16_to_cpup((__le16*)(data + 2)), 0);
> }
>
> /*
Re: [PATCH 1/2] media: ov5640: Add check for PLL1 output max frequency
Hi Maxime,
On Thu, Oct 18, 2018 at 11:15:50AM +0200, Maxime Ripard wrote:
> On Wed, Oct 17, 2018 at 09:37:17PM +0200, Jacopo Mondi wrote:
> > Check that the PLL1 output frequency does not exceed the maximum allowed
> > 1GHz
> > frequency.
> >
> > Signed-off-by: Jacopo Mondi
> > ---
> > drivers/media/i2c/ov5640.c | 23 +++
> > 1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index e098435..1f2e72d 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -770,7 +770,7 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor,
> > u16 reg,
> > * always set to either 1 or 2 in the vendor kernels.
> > */
> > #define OV5640_SYSDIV_MIN 1
> > -#define OV5640_SYSDIV_MAX 2
> > +#define OV5640_SYSDIV_MAX 16
> >
> > /*
> > * This is supposed to be ranging from 1 to 16, but the value is always
> > @@ -806,15 +806,20 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor,
> > u16 reg,
> > * This is supposed to be ranging from 1 to 8, but the value is always
> > * set to 1 in the vendor kernels.
> > */
> > -#define OV5640_PCLK_ROOT_DIV 1
> > +#define OV5640_PCLK_ROOT_DIV 1
> > +#define OV5640_PLL_SYS_ROOT_DIVIDER_BYPASS 0x00
> >
> > static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
> > u8 pll_prediv, u8 pll_mult,
> > u8 sysdiv)
> > {
> > - unsigned long rate = clk_get_rate(sensor->xclk);
> > + unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
> >
> > - return rate / pll_prediv * pll_mult / sysdiv;
> > + /* PLL1 output cannot exceed 1GHz. */
> > + if (sysclk / 100 > 1000)
> > + return 0;
> > +
> > + return sysclk / sysdiv;
> > }
> >
> > static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
> > @@ -844,6 +849,16 @@ static unsigned long ov5640_calc_sys_clk(struct
> > ov5640_dev *sensor,
> > _rate = ov5640_compute_sys_clk(sensor,
> >OV5640_PLL_PREDIV,
> >_pll_mult, _sysdiv);
> > +
> > + /*
> > +* We have reached the maximum allowed PLL1 output,
> > +* increase sysdiv.
> > +*/
> > + if (rate == 0) {
> > + _pll_mult = OV5640_PLL_MULT_MAX + 1;
> > + continue;
> > + }
> > +
>
> Both your patches look sane to me. However, I guess here you're
> setting _pll_mult at this value so that you won't reach the for
> condition on the next iteration?
>
> Wouldn't it be cleaner to just use a break statement here?
Yes, it's much cleaner indeed. Not sure why I thought this was a good
idea tbh.
Would you like me to send a v2, or can you take care of this when
re-sending v5?
Thanks
j
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
signature.asc
Description: PGP signature
[PATCH] media: rc: XBox DVD Remote uses 12 bits scancodes
The xbox dvd remote sends 24 bits, the first 12 bits are repeated
and inverted so only 12 bits are used. The upper 4 bits can be read
at offset 3. Ensure we pass this to rc-core and update the keymap
accordingly.
Signed-off-by: Sean Young
---
drivers/media/rc/keymaps/rc-xbox-dvd.c | 58 +-
drivers/media/rc/xbox_remote.c | 7 ++--
2 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/drivers/media/rc/keymaps/rc-xbox-dvd.c
b/drivers/media/rc/keymaps/rc-xbox-dvd.c
index 61da6706715c..af387244636b 100644
--- a/drivers/media/rc/keymaps/rc-xbox-dvd.c
+++ b/drivers/media/rc/keymaps/rc-xbox-dvd.c
@@ -7,35 +7,35 @@
/* based on lircd.conf.xbox */
static struct rc_map_table xbox_dvd[] = {
- {0x0b, KEY_OK},
- {0xa6, KEY_UP},
- {0xa7, KEY_DOWN},
- {0xa8, KEY_RIGHT},
- {0xa9, KEY_LEFT},
- {0xc3, KEY_INFO},
-
- {0xc6, KEY_9},
- {0xc7, KEY_8},
- {0xc8, KEY_7},
- {0xc9, KEY_6},
- {0xca, KEY_5},
- {0xcb, KEY_4},
- {0xcc, KEY_3},
- {0xcd, KEY_2},
- {0xce, KEY_1},
- {0xcf, KEY_0},
-
- {0xd5, KEY_ANGLE},
- {0xd8, KEY_BACK},
- {0xdd, KEY_PREVIOUSSONG},
- {0xdf, KEY_NEXTSONG},
- {0xe0, KEY_STOP},
- {0xe2, KEY_REWIND},
- {0xe3, KEY_FASTFORWARD},
- {0xe5, KEY_TITLE},
- {0xe6, KEY_PAUSE},
- {0xea, KEY_PLAY},
- {0xf7, KEY_MENU},
+ {0xa0b, KEY_OK},
+ {0xaa6, KEY_UP},
+ {0xaa7, KEY_DOWN},
+ {0xaa8, KEY_RIGHT},
+ {0xaa9, KEY_LEFT},
+ {0xac3, KEY_INFO},
+
+ {0xac6, KEY_9},
+ {0xac7, KEY_8},
+ {0xac8, KEY_7},
+ {0xac9, KEY_6},
+ {0xaca, KEY_5},
+ {0xacb, KEY_4},
+ {0xacc, KEY_3},
+ {0xacd, KEY_2},
+ {0xace, KEY_1},
+ {0xacf, KEY_0},
+
+ {0xad5, KEY_ANGLE},
+ {0xad8, KEY_BACK},
+ {0xadd, KEY_PREVIOUSSONG},
+ {0xadf, KEY_NEXTSONG},
+ {0xae0, KEY_STOP},
+ {0xae2, KEY_REWIND},
+ {0xae3, KEY_FASTFORWARD},
+ {0xae5, KEY_TITLE},
+ {0xae6, KEY_PAUSE},
+ {0xaea, KEY_PLAY},
+ {0xaf7, KEY_MENU},
};
static struct rc_map_list xbox_dvd_map = {
diff --git a/drivers/media/rc/xbox_remote.c b/drivers/media/rc/xbox_remote.c
index 141ef9253018..4d41e31369d2 100644
--- a/drivers/media/rc/xbox_remote.c
+++ b/drivers/media/rc/xbox_remote.c
@@ -55,7 +55,7 @@ struct xbox_remote {
struct usb_interface *interface;
struct urb *irq_urb;
- unsigned char inbuf[DATA_BUFSIZE];
+ unsigned char inbuf[DATA_BUFSIZE] __aligned(sizeof(u16));
char rc_name[NAME_BUFSIZE];
char rc_phys[NAME_BUFSIZE];
@@ -95,7 +95,7 @@ static void xbox_remote_input_report(struct urb *urb)
* data[0] = 0x00
* data[1] = length - always 0x06
* data[2] = the key code
-* data[3] = high part of key code? - always 0x0a
+* data[3] = high part of key code
* data[4] = last_press_ms (low)
* data[5] = last_press_ms (high)
*/
@@ -107,7 +107,8 @@ static void xbox_remote_input_report(struct urb *urb)
return;
}
- rc_keydown(xbox_remote->rdev, RC_PROTO_UNKNOWN, data[2], 0);
+ rc_keydown(xbox_remote->rdev, RC_PROTO_UNKNOWN,
+ le16_to_cpup((__le16*)(data + 2)), 0);
}
/*
--
2.17.2
[PATCH v4l-utils] keytable: bpf decoder and keymap for XBox DVD Remote
This uses a modified nec protocol, where 24 bits are sent and the first
12 bits are inverted.
Signed-off-by: Sean Young
---
utils/keytable/bpf_protocols/Makefile.am | 2 +-
utils/keytable/bpf_protocols/xbox.c | 129 ++
.../rc_keymaps_userspace/xbox_dvd.toml| 31 +
3 files changed, 161 insertions(+), 1 deletion(-)
create mode 100644 utils/keytable/bpf_protocols/xbox.c
create mode 100644 utils/keytable/rc_keymaps_userspace/xbox_dvd.toml
diff --git a/utils/keytable/bpf_protocols/Makefile.am
b/utils/keytable/bpf_protocols/Makefile.am
index 8887b897..2d005b1f 100644
--- a/utils/keytable/bpf_protocols/Makefile.am
+++ b/utils/keytable/bpf_protocols/Makefile.am
@@ -10,7 +10,7 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - &1 \
%.o: %.c bpf_helpers.h
$(CLANG) $(CLANG_SYS_INCLUDES) -I$(top_srcdir)/include -target bpf -O2
-c $<
-PROTOCOLS = grundig.o pulse_distance.o pulse_length.o rc_mm.o manchester.o
+PROTOCOLS = grundig.o pulse_distance.o pulse_length.o rc_mm.o manchester.o
xbox.o
all: $(PROTOCOLS)
diff --git a/utils/keytable/bpf_protocols/xbox.c
b/utils/keytable/bpf_protocols/xbox.c
new file mode 100644
index ..e48e0a79
--- /dev/null
+++ b/utils/keytable/bpf_protocols/xbox.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2018 Sean Young
+
+#include
+#include
+
+#include "bpf_helpers.h"
+
+enum state {
+ STATE_INACTIVE,
+ STATE_HEADER_SPACE,
+ STATE_BITS_SPACE,
+ STATE_BITS_PULSE,
+ STATE_TRAILER,
+};
+
+struct decoder_state {
+ unsigned long bits;
+ enum state state;
+ unsigned int count;
+};
+
+struct bpf_map_def SEC("maps") decoder_state_map = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(unsigned int),
+ .value_size = sizeof(struct decoder_state),
+ .max_entries = 1,
+};
+
+// These values can be overridden in the rc_keymap toml
+//
+// We abuse elf relocations. We cast the address of these variables to
+// an int, so that the compiler emits a mov immediate for the address
+// but uses it as an int. The bpf loader replaces the relocation with the
+// actual value (either overridden or taken from the data segment).
+int margin = 200;
+int header_pulse = 4000;
+int header_space = 3900;
+int bit_pulse = 550;
+int bit_0_space = 900;
+int bit_1_space = 1900;
+int trailer_pulse = 550;
+int bits = 24;
+int rc_protocol = 68;
+
+#define BPF_PARAM(x) (int)(&(x))
+
+static inline int eq_margin(unsigned d1, unsigned d2)
+{
+ return ((d1 > (d2 - BPF_PARAM(margin))) && (d1 < (d2 +
BPF_PARAM(margin;
+}
+
+SEC("xbox")
+int bpf_decoder(unsigned int *sample)
+{
+ unsigned int key = 0;
+ struct decoder_state *s = bpf_map_lookup_elem(_state_map, );
+
+ if (!s)
+ return 0;
+
+ switch (*sample & LIRC_MODE2_MASK) {
+ case LIRC_MODE2_SPACE:
+ case LIRC_MODE2_PULSE:
+ case LIRC_MODE2_TIMEOUT:
+ break;
+ default:
+ // not a timing events
+ return 0;
+ }
+
+ int duration = LIRC_VALUE(*sample);
+ int pulse = LIRC_IS_PULSE(*sample);
+
+ switch (s->state) {
+ case STATE_HEADER_SPACE:
+ if (!pulse && eq_margin(BPF_PARAM(header_space), duration))
+ s->state = STATE_BITS_PULSE;
+ else
+ s->state = STATE_INACTIVE;
+ break;
+ case STATE_INACTIVE:
+ if (pulse && eq_margin(BPF_PARAM(header_pulse), duration)) {
+ s->bits = 0;
+ s->state = STATE_HEADER_SPACE;
+ s->count = 0;
+ }
+ break;
+ case STATE_BITS_PULSE:
+ if (pulse && eq_margin(BPF_PARAM(bit_pulse), duration))
+ s->state = STATE_BITS_SPACE;
+ else
+ s->state = STATE_INACTIVE;
+ break;
+ case STATE_BITS_SPACE:
+ if (pulse) {
+ s->state = STATE_INACTIVE;
+ break;
+ }
+
+ s->bits <<= 1;
+
+ if (eq_margin(BPF_PARAM(bit_1_space), duration))
+ s->bits |= 1;
+ else if (!eq_margin(BPF_PARAM(bit_0_space), duration)) {
+ s->state = STATE_INACTIVE;
+ break;
+ }
+
+ s->count++;
+ if (s->count == BPF_PARAM(bits))
+ s->state = STATE_TRAILER;
+ else
+ s->state = STATE_BITS_PULSE;
+ break;
+ case STATE_TRAILER:
+ if (pulse && eq_margin(BPF_PARAM(trailer_pulse), duration)) {
+ if (((s->bits >> 12) ^ (s->bits & 0xfff)) == 0xfff)
+ bpf_rc_keydown(sample, BPF_PARAM(rc_protocol),
s->bits & 0xfff, 0);
+ }
+
+
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
On Thu, Oct 18, 2018 at 11:31:52AM +0200, Maxime Ripard wrote: > On Wed, Oct 17, 2018 at 09:51:43PM +0200, jacopo mondi wrote: > > Hello Sam and Maxime (and other ov5640-ers :) > > > > On Wed, Oct 17, 2018 at 10:54:01AM -0700, Sam Bobrowicz wrote: > > > Hello Maxime and Jacopo (and other ov5640-ers), > > > > > > I just submitted my version of this patch to the mailing list as RFC. > > > It is working on my MIPI platform. Please try it if you have time. > > > Hopefully we can merge these two into a single patch that doesn't > > > break any platforms. > > > > Thanks, I have seen your patch but it seems to contain a lot of things > > already part of Maxime's series. Was this intentional? > > > > Now the un-pleaseant part: I have just sent out my re-implementation > > of the MIPI clock tree configuration, based on top of Maxime's series. > > Both you and me have spent a looot of time on this I'm sure, and now > > we have two competing implementations. > > > > I had a quick look at yours, and for sure there are things I am not > > taking care of (I'm thinking about the 0x4837 register that seems to > > be important for your platform), so I think both our implementations > > can benefits from a comparison. What is important to me is that both > > you and me don't feel like our work has been wasted, so let's try to > > find out a way to get the better of the two put together, and possibly > > applied on top of Maxime's series, so that a v5 of this will work for > > both MIPI and DVP interfaces. How to do that I'm not sure atm, I think > > other reviewers might help in that if they want to have a look at both > > our series :) > > IIRC, Sam's system has never worked with the ov5640 driver, and his > patches now make it work. > > Your patches on the other hand make sure that the current series > doesn't break existing users. So I guess we could merge your current > patches into the v5 of my rework, and have Sam send his work on top of > that. > > Does that make sense? It does for me, but it puts the burden on Sam to re-apply his work on top of [yours+mine] (which is something he would have had to do anyhow to have his patches accepted, as he would have had to rebase on top of your series). I hope to find some more time to look into his series and find out how hard it would be to add his changes on top of mine, and hopefully help with this. Also, testing my patches with DVP would be nice (it should not be affected at all, but still...) Thanks j > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
On Wed, Oct 17, 2018 at 09:51:43PM +0200, jacopo mondi wrote: > Hello Sam and Maxime (and other ov5640-ers :) > > On Wed, Oct 17, 2018 at 10:54:01AM -0700, Sam Bobrowicz wrote: > > Hello Maxime and Jacopo (and other ov5640-ers), > > > > I just submitted my version of this patch to the mailing list as RFC. > > It is working on my MIPI platform. Please try it if you have time. > > Hopefully we can merge these two into a single patch that doesn't > > break any platforms. > > Thanks, I have seen your patch but it seems to contain a lot of things > already part of Maxime's series. Was this intentional? > > Now the un-pleaseant part: I have just sent out my re-implementation > of the MIPI clock tree configuration, based on top of Maxime's series. > Both you and me have spent a looot of time on this I'm sure, and now > we have two competing implementations. > > I had a quick look at yours, and for sure there are things I am not > taking care of (I'm thinking about the 0x4837 register that seems to > be important for your platform), so I think both our implementations > can benefits from a comparison. What is important to me is that both > you and me don't feel like our work has been wasted, so let's try to > find out a way to get the better of the two put together, and possibly > applied on top of Maxime's series, so that a v5 of this will work for > both MIPI and DVP interfaces. How to do that I'm not sure atm, I think > other reviewers might help in that if they want to have a look at both > our series :) IIRC, Sam's system has never worked with the ov5640 driver, and his patches now make it work. Your patches on the other hand make sure that the current series doesn't break existing users. So I guess we could merge your current patches into the v5 of my rework, and have Sam send his work on top of that. Does that make sense? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
Hi Jacopo,
Thanks for reviewing this patch
On Tue, Oct 16, 2018 at 06:54:50PM +0200, jacopo mondi wrote:
> > +static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
> > + u8 pll_prediv, u8 pll_mult,
> > + u8 sysdiv)
> > +{
> > + unsigned long rate = clk_get_rate(sensor->xclk);
>
> The clock rate is stored in sensor->xclk at probe time, no need to
> query it every iteration.
From a clk API point of view though, there's nothing that guarantees
that the clock rate hasn't changed between the probe and the time
where this function is called.
I appreciate that we're probably connected to an oscillator, but even
then, on the Allwinner SoCs we've had the issue recently that one
oscillator feeding the BT chip was actually had a muxer, with each
option having a slightly different rate, which was bad enough for the
BT chip to be non-functional.
I can definitely imagine the same case happening here for some
SoCs. Plus, the clock framework will cache the rate as well when
possible, so we're not losing anything here.
> > +
> > + return rate / pll_prediv * pll_mult / sysdiv;
> > +}
> > +
> > +static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
> > +unsigned long rate,
> > +u8 *pll_prediv, u8 *pll_mult,
> > +u8 *sysdiv)
> > +{
> > + unsigned long best = ~0;
> > + u8 best_sysdiv = 1, best_mult = 1;
> > + u8 _sysdiv, _pll_mult;
> > +
> > + for (_sysdiv = OV5640_SYSDIV_MIN;
> > +_sysdiv <= OV5640_SYSDIV_MAX;
> > +_sysdiv++) {
> > + for (_pll_mult = OV5640_PLL_MULT_MIN;
> > +_pll_mult <= OV5640_PLL_MULT_MAX;
> > +_pll_mult++) {
> > + unsigned long _rate;
> > +
> > + /*
> > +* The PLL multiplier cannot be odd if above
> > +* 127.
> > +*/
> > + if (_pll_mult > 127 && (_pll_mult % 2))
> > + continue;
> > +
> > + _rate = ov5640_compute_sys_clk(sensor,
> > + OV5640_PLL_PREDIV,
> > + _pll_mult, _sysdiv);
>
> I'm under the impression a system clock slower than the requested one, even
> if more accurate is not good.
>
> I'm still working on understanding how all CSI-2 related timing
> parameters play together, but since the system clock is calculated
> from the pixel clock (which comes from the frame dimensions, bpp, and
> rate), and it is then used to calculate the MIPI BIT clock frequency,
> I think it would be better to be a little faster than a bit slower,
> otherwise the serial lane clock wouldn't be fast enough to output
> frames generated by the sensor core (or maybe it would just decrease
> the frame rate and that's it, but I don't think it is just this).
>
> What do you think of adding the following here:
>
> if (_rate < rate)
> continue
I really don't know MIPI-CSI2 enough to be able to comment on your
concerns, but when reaching the end of the operating limit of the
clock, it would prevent us from having any rate at all, which seems
bad too.
> > + if (abs(rate - _rate) < abs(rate - best)) {
> > + best = _rate;
> > + best_sysdiv = _sysdiv;
> > + best_mult = _pll_mult;
> > + }
> > +
> > + if (_rate == rate)
> > + goto out;
> > + }
> > + }
> > +
> > +out:
> > + *sysdiv = best_sysdiv;
> > + *pll_prediv = OV5640_PLL_PREDIV;
> > + *pll_mult = best_mult;
> > + return best;
> > +}
>
> These function gets called at s_stream time, and cycle for a while,
> and I'm under the impression the MIPI state machine doesn't like
> delays too much, as I see timeouts on the receiver side.
>
> I have tried to move this function at set_fmt() time, every time a new
> mode is selected, sysdiv, pll_prediv and pll_mult gets recalculated
> (and stored in the ov5640_dev structure). I now have other timeouts on
> missing EOF, but not anymore at startup time it seems.
I have no objection caching the values if it solves issues with CSI :)
Can you send that patch?
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH 1/2] media: ov5640: Add check for PLL1 output max frequency
On Wed, Oct 17, 2018 at 09:37:17PM +0200, Jacopo Mondi wrote:
> Check that the PLL1 output frequency does not exceed the maximum allowed 1GHz
> frequency.
>
> Signed-off-by: Jacopo Mondi
> ---
> drivers/media/i2c/ov5640.c | 23 +++
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index e098435..1f2e72d 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -770,7 +770,7 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor, u16
> reg,
> * always set to either 1 or 2 in the vendor kernels.
> */
> #define OV5640_SYSDIV_MIN1
> -#define OV5640_SYSDIV_MAX2
> +#define OV5640_SYSDIV_MAX16
>
> /*
> * This is supposed to be ranging from 1 to 16, but the value is always
> @@ -806,15 +806,20 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor,
> u16 reg,
> * This is supposed to be ranging from 1 to 8, but the value is always
> * set to 1 in the vendor kernels.
> */
> -#define OV5640_PCLK_ROOT_DIV 1
> +#define OV5640_PCLK_ROOT_DIV 1
> +#define OV5640_PLL_SYS_ROOT_DIVIDER_BYPASS 0x00
>
> static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
> u8 pll_prediv, u8 pll_mult,
> u8 sysdiv)
> {
> - unsigned long rate = clk_get_rate(sensor->xclk);
> + unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
>
> - return rate / pll_prediv * pll_mult / sysdiv;
> + /* PLL1 output cannot exceed 1GHz. */
> + if (sysclk / 100 > 1000)
> + return 0;
> +
> + return sysclk / sysdiv;
> }
>
> static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
> @@ -844,6 +849,16 @@ static unsigned long ov5640_calc_sys_clk(struct
> ov5640_dev *sensor,
> _rate = ov5640_compute_sys_clk(sensor,
> OV5640_PLL_PREDIV,
> _pll_mult, _sysdiv);
> +
> + /*
> + * We have reached the maximum allowed PLL1 output,
> + * increase sysdiv.
> + */
> + if (rate == 0) {
> + _pll_mult = OV5640_PLL_MULT_MAX + 1;
> + continue;
> + }
> +
Both your patches look sane to me. However, I guess here you're
setting _pll_mult at this value so that you won't reach the for
condition on the next iteration?
Wouldn't it be cleaner to just use a break statement here?
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
