Re: [PATCH 1/2] vb2: add waiting_in_dqbuf flag
On 11/19/2018 06:27 AM, Tomasz Figa wrote: > On Fri, Nov 16, 2018 at 6:45 PM Hans Verkuil wrote: >> >> On 11/16/2018 09:43 AM, Tomasz Figa wrote: >>> Hi Hans, >>> >>> On Wed, Nov 14, 2018 at 12:08 AM Hans Verkuil wrote: Calling VIDIOC_DQBUF can release the core serialization lock pointed to by vb2_queue->lock if it has to wait for a new buffer to arrive. However, if userspace dup()ped the video device filehandle, then it is possible to read or call DQBUF from two filehandles at the same time. >>> >>> What side effects would reading have? >>> >>> As for another DQBUF in parallel, perhaps that's actually a valid >>> operation that should be handled? I can imagine that one could want to >>> have multiple threads dequeuing buffers as they become available, so >>> that no dispatch thread is needed. >> >> I think parallel DQBUFs can be done, but it has never been tested, nor >> has vb2 been designed with that in mind. I also don't see the use-case >> since if you have, say, two DQBUFs in parallel, then it will be random >> which DQBUF gets which frame. >> > > Any post processing that operates only on single frame data would be > able to benefit from multiple threads, with results ordered after the > processing, based on timestamps. > > Still, if that's not something we've ever claimed as supported and > couldn't work correctly with current code, it sounds fair to > completely forbid it for now. > >> If we ever see a need for this, then that needs to be designed and tested >> properly. >> >>> It is also possible to call REQBUFS from one filehandle while the other is waiting for a buffer. This will remove all the buffers and reallocate new ones. Removing all the buffers isn't the problem here (that's already handled correctly by DQBUF), but the reallocating part is: DQBUF isn't aware that the buffers have changed. This is fixed by setting a flag whenever the lock is released while waiting for a buffer to arrive. And checking the flag where needed so we can return -EBUSY. >>> >>> Maybe it would make more sense to actually handle those side effects? >>> Such waiting DQBUF would then just fail in the same way as if it >>> couldn't get a buffer (or if it's blocking, just retry until a correct >>> buffer becomes available?). >> >> That sounds like a good idea, but it isn't. >> >> With this patch you can't call REQBUFS to reallocate buffers while a thread >> is waiting for a buffer. >> >> If I allow this, then the problem moves to when the thread that called >> REQBUFS >> calls DQBUF next. Since we don't allow multiple DQBUFs this second DQBUF will >> mysteriously fail. If we DO allow multiple DQBUFs, then how does REQBUFS >> ensure >> that only the DQBUF that relied on the old buffers is stopped? >> >> It sounds nice, but the more I think about it, the more problems I see with >> it. >> >> I think it is perfectly reasonable to expect REQBUFS to return EBUSY if some >> thread is still waiting for a buffer. >> >> That said, I think one test is missing in vb2_core_create_bufs: there too it >> should check waiting_in_dqbuf if q->num_buffers == 0: it is possible to do >> REQBUFS(0) followed by CREATE_BUFS() while another thread is waiting for a >> buffer. CREATE_BUFS acts like REQBUFS(count >= 1) in that case. >> >> Admittedly, that would require some extremely unfortunate scheduling, but >> it is easy enough to check this. > > I thought a bit more about this and I agree with you. We should keep > things as simple as possible. > > Another thing that came to my mind is that the problematic scenario > described in the commit message can happen only if queue->lock == > dev->lock. I wonder how likely it would be to mandate queue->lock != > dev->lock? My plan is to switch vivid to that model. Expect patches for that today. One thing I noticed is that there is an issue with calling queue_setup in that case. I have a separate patch for that, so just read it when I post it. Regards, Hans
Re: [RFC PATCH v8 1/4] media: Media Device Allocator API
On Thu 2018-11-01 18:31:30, sh...@kernel.org wrote: > From: Shuah Khan > > Media Device Allocator API to allows multiple drivers share a media device. > Using this API, drivers can allocate a media device with the shared struct > device as the key. Once the media device is allocated by a driver, other > drivers can get a reference to it. The media device is released when all > the references are released. Sounds like a ... bad idea? That's what new "media control" framework is for, no? Why do you need this? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH 1/2] vb2: add waiting_in_dqbuf flag
On 11/19/2018 09:44 AM, Hans Verkuil wrote: > On 11/19/2018 06:27 AM, Tomasz Figa wrote: >> On Fri, Nov 16, 2018 at 6:45 PM Hans Verkuil wrote: >>> >>> On 11/16/2018 09:43 AM, Tomasz Figa wrote: Hi Hans, On Wed, Nov 14, 2018 at 12:08 AM Hans Verkuil wrote: > > Calling VIDIOC_DQBUF can release the core serialization lock pointed to > by vb2_queue->lock if it has to wait for a new buffer to arrive. > > However, if userspace dup()ped the video device filehandle, then it is > possible to read or call DQBUF from two filehandles at the same time. > What side effects would reading have? As for another DQBUF in parallel, perhaps that's actually a valid operation that should be handled? I can imagine that one could want to have multiple threads dequeuing buffers as they become available, so that no dispatch thread is needed. >>> >>> I think parallel DQBUFs can be done, but it has never been tested, nor >>> has vb2 been designed with that in mind. I also don't see the use-case >>> since if you have, say, two DQBUFs in parallel, then it will be random >>> which DQBUF gets which frame. >>> >> >> Any post processing that operates only on single frame data would be >> able to benefit from multiple threads, with results ordered after the >> processing, based on timestamps. >> >> Still, if that's not something we've ever claimed as supported and >> couldn't work correctly with current code, it sounds fair to >> completely forbid it for now. >> >>> If we ever see a need for this, then that needs to be designed and tested >>> properly. >>> > It is also possible to call REQBUFS from one filehandle while the other > is waiting for a buffer. This will remove all the buffers and reallocate > new ones. Removing all the buffers isn't the problem here (that's already > handled correctly by DQBUF), but the reallocating part is: DQBUF isn't > aware that the buffers have changed. > > This is fixed by setting a flag whenever the lock is released while > waiting > for a buffer to arrive. And checking the flag where needed so we can > return > -EBUSY. Maybe it would make more sense to actually handle those side effects? Such waiting DQBUF would then just fail in the same way as if it couldn't get a buffer (or if it's blocking, just retry until a correct buffer becomes available?). >>> >>> That sounds like a good idea, but it isn't. >>> >>> With this patch you can't call REQBUFS to reallocate buffers while a thread >>> is waiting for a buffer. >>> >>> If I allow this, then the problem moves to when the thread that called >>> REQBUFS >>> calls DQBUF next. Since we don't allow multiple DQBUFs this second DQBUF >>> will >>> mysteriously fail. If we DO allow multiple DQBUFs, then how does REQBUFS >>> ensure >>> that only the DQBUF that relied on the old buffers is stopped? >>> >>> It sounds nice, but the more I think about it, the more problems I see with >>> it. >>> >>> I think it is perfectly reasonable to expect REQBUFS to return EBUSY if some >>> thread is still waiting for a buffer. >>> >>> That said, I think one test is missing in vb2_core_create_bufs: there too it >>> should check waiting_in_dqbuf if q->num_buffers == 0: it is possible to do >>> REQBUFS(0) followed by CREATE_BUFS() while another thread is waiting for a >>> buffer. CREATE_BUFS acts like REQBUFS(count >= 1) in that case. >>> >>> Admittedly, that would require some extremely unfortunate scheduling, but >>> it is easy enough to check this. >> >> I thought a bit more about this and I agree with you. We should keep >> things as simple as possible. >> >> Another thing that came to my mind is that the problematic scenario >> described in the commit message can happen only if queue->lock == >> dev->lock. I wonder how likely it would be to mandate queue->lock != >> dev->lock? > > My plan is to switch vivid to that model. Expect patches for that today. > One thing I noticed is that there is an issue with calling queue_setup > in that case. I have a separate patch for that, so just read it when I > post it. Note that this specific scenario can happen regardless of whether queue->lock == dev->lock or not. Regards, Hans
Re: [PATCH 1/2] vb2: add waiting_in_dqbuf flag
On Mon, Nov 19, 2018 at 6:54 PM Hans Verkuil wrote: > > On 11/19/2018 09:44 AM, Hans Verkuil wrote: > > On 11/19/2018 06:27 AM, Tomasz Figa wrote: > >> On Fri, Nov 16, 2018 at 6:45 PM Hans Verkuil wrote: > >>> > >>> On 11/16/2018 09:43 AM, Tomasz Figa wrote: > Hi Hans, > > On Wed, Nov 14, 2018 at 12:08 AM Hans Verkuil wrote: > > > > Calling VIDIOC_DQBUF can release the core serialization lock pointed to > > by vb2_queue->lock if it has to wait for a new buffer to arrive. > > > > However, if userspace dup()ped the video device filehandle, then it is > > possible to read or call DQBUF from two filehandles at the same time. > > > > What side effects would reading have? > > As for another DQBUF in parallel, perhaps that's actually a valid > operation that should be handled? I can imagine that one could want to > have multiple threads dequeuing buffers as they become available, so > that no dispatch thread is needed. > >>> > >>> I think parallel DQBUFs can be done, but it has never been tested, nor > >>> has vb2 been designed with that in mind. I also don't see the use-case > >>> since if you have, say, two DQBUFs in parallel, then it will be random > >>> which DQBUF gets which frame. > >>> > >> > >> Any post processing that operates only on single frame data would be > >> able to benefit from multiple threads, with results ordered after the > >> processing, based on timestamps. > >> > >> Still, if that's not something we've ever claimed as supported and > >> couldn't work correctly with current code, it sounds fair to > >> completely forbid it for now. > >> > >>> If we ever see a need for this, then that needs to be designed and tested > >>> properly. > >>> > > > It is also possible to call REQBUFS from one filehandle while the other > > is waiting for a buffer. This will remove all the buffers and reallocate > > new ones. Removing all the buffers isn't the problem here (that's > > already > > handled correctly by DQBUF), but the reallocating part is: DQBUF isn't > > aware that the buffers have changed. > > > > This is fixed by setting a flag whenever the lock is released while > > waiting > > for a buffer to arrive. And checking the flag where needed so we can > > return > > -EBUSY. > > Maybe it would make more sense to actually handle those side effects? > Such waiting DQBUF would then just fail in the same way as if it > couldn't get a buffer (or if it's blocking, just retry until a correct > buffer becomes available?). > >>> > >>> That sounds like a good idea, but it isn't. > >>> > >>> With this patch you can't call REQBUFS to reallocate buffers while a > >>> thread > >>> is waiting for a buffer. > >>> > >>> If I allow this, then the problem moves to when the thread that called > >>> REQBUFS > >>> calls DQBUF next. Since we don't allow multiple DQBUFs this second DQBUF > >>> will > >>> mysteriously fail. If we DO allow multiple DQBUFs, then how does REQBUFS > >>> ensure > >>> that only the DQBUF that relied on the old buffers is stopped? > >>> > >>> It sounds nice, but the more I think about it, the more problems I see > >>> with it. > >>> > >>> I think it is perfectly reasonable to expect REQBUFS to return EBUSY if > >>> some > >>> thread is still waiting for a buffer. > >>> > >>> That said, I think one test is missing in vb2_core_create_bufs: there too > >>> it > >>> should check waiting_in_dqbuf if q->num_buffers == 0: it is possible to do > >>> REQBUFS(0) followed by CREATE_BUFS() while another thread is waiting for a > >>> buffer. CREATE_BUFS acts like REQBUFS(count >= 1) in that case. > >>> > >>> Admittedly, that would require some extremely unfortunate scheduling, but > >>> it is easy enough to check this. > >> > >> I thought a bit more about this and I agree with you. We should keep > >> things as simple as possible. > >> > >> Another thing that came to my mind is that the problematic scenario > >> described in the commit message can happen only if queue->lock == > >> dev->lock. I wonder how likely it would be to mandate queue->lock != > >> dev->lock? > > > > My plan is to switch vivid to that model. Expect patches for that today. > > One thing I noticed is that there is an issue with calling queue_setup > > in that case. I have a separate patch for that, so just read it when I > > post it. > > Note that this specific scenario can happen regardless of whether > queue->lock == dev->lock or not. Ah, good point. Somehow I assumed that only QBUF/DQBUF would use queue->lock, while anything else would use dev->lock, but that's not the case. Then I can't find any simpler and/or more general fix for now, so I'm okay with this. Another note, don't we need similar error in case of REQBUFS(0), while DQBUF() is waiting? Current patch seems to add one only for count != 0. Best regards, Tomasz
Re: [PATCH 1/2] vb2: add waiting_in_dqbuf flag
On 11/19/2018 11:32 AM, Tomasz Figa wrote: > On Mon, Nov 19, 2018 at 6:54 PM Hans Verkuil wrote: >> >> On 11/19/2018 09:44 AM, Hans Verkuil wrote: >>> On 11/19/2018 06:27 AM, Tomasz Figa wrote: On Fri, Nov 16, 2018 at 6:45 PM Hans Verkuil wrote: > > On 11/16/2018 09:43 AM, Tomasz Figa wrote: >> Hi Hans, >> >> On Wed, Nov 14, 2018 at 12:08 AM Hans Verkuil wrote: >>> >>> Calling VIDIOC_DQBUF can release the core serialization lock pointed to >>> by vb2_queue->lock if it has to wait for a new buffer to arrive. >>> >>> However, if userspace dup()ped the video device filehandle, then it is >>> possible to read or call DQBUF from two filehandles at the same time. >>> >> >> What side effects would reading have? >> >> As for another DQBUF in parallel, perhaps that's actually a valid >> operation that should be handled? I can imagine that one could want to >> have multiple threads dequeuing buffers as they become available, so >> that no dispatch thread is needed. > > I think parallel DQBUFs can be done, but it has never been tested, nor > has vb2 been designed with that in mind. I also don't see the use-case > since if you have, say, two DQBUFs in parallel, then it will be random > which DQBUF gets which frame. > Any post processing that operates only on single frame data would be able to benefit from multiple threads, with results ordered after the processing, based on timestamps. Still, if that's not something we've ever claimed as supported and couldn't work correctly with current code, it sounds fair to completely forbid it for now. > If we ever see a need for this, then that needs to be designed and tested > properly. > >> >>> It is also possible to call REQBUFS from one filehandle while the other >>> is waiting for a buffer. This will remove all the buffers and reallocate >>> new ones. Removing all the buffers isn't the problem here (that's >>> already >>> handled correctly by DQBUF), but the reallocating part is: DQBUF isn't >>> aware that the buffers have changed. >>> >>> This is fixed by setting a flag whenever the lock is released while >>> waiting >>> for a buffer to arrive. And checking the flag where needed so we can >>> return >>> -EBUSY. >> >> Maybe it would make more sense to actually handle those side effects? >> Such waiting DQBUF would then just fail in the same way as if it >> couldn't get a buffer (or if it's blocking, just retry until a correct >> buffer becomes available?). > > That sounds like a good idea, but it isn't. > > With this patch you can't call REQBUFS to reallocate buffers while a > thread > is waiting for a buffer. > > If I allow this, then the problem moves to when the thread that called > REQBUFS > calls DQBUF next. Since we don't allow multiple DQBUFs this second DQBUF > will > mysteriously fail. If we DO allow multiple DQBUFs, then how does REQBUFS > ensure > that only the DQBUF that relied on the old buffers is stopped? > > It sounds nice, but the more I think about it, the more problems I see > with it. > > I think it is perfectly reasonable to expect REQBUFS to return EBUSY if > some > thread is still waiting for a buffer. > > That said, I think one test is missing in vb2_core_create_bufs: there too > it > should check waiting_in_dqbuf if q->num_buffers == 0: it is possible to do > REQBUFS(0) followed by CREATE_BUFS() while another thread is waiting for a > buffer. CREATE_BUFS acts like REQBUFS(count >= 1) in that case. > > Admittedly, that would require some extremely unfortunate scheduling, but > it is easy enough to check this. I thought a bit more about this and I agree with you. We should keep things as simple as possible. Another thing that came to my mind is that the problematic scenario described in the commit message can happen only if queue->lock == dev->lock. I wonder how likely it would be to mandate queue->lock != dev->lock? >>> >>> My plan is to switch vivid to that model. Expect patches for that today. >>> One thing I noticed is that there is an issue with calling queue_setup >>> in that case. I have a separate patch for that, so just read it when I >>> post it. >> >> Note that this specific scenario can happen regardless of whether >> queue->lock == dev->lock or not. > > Ah, good point. Somehow I assumed that only QBUF/DQBUF would use > queue->lock, while anything else would use dev->lock, but that's not > the case. > > Then I can't find any simpler and/or more general fix for now, so I'm > okay with this. > > Another note, don't we need similar error in case of REQBUFS(0), while > DQBUF() is waiting? Current patch seems to add one only for count != > 0.
videobuf2-core.c: call to v4l_vb2q_enable_media_source()?
Hi Shuah, Mauro, I noticed that the function vb2_core_streamon() in videobuf2-core.c calls v4l_vb2q_enable_media_source(q). That function in turn assumes that q->owner is a v4l2_fh struct. But I don't think that is true for DVB devices. And since videobuf2-core.c is expected to be DVB/V4L independent, this seems wrong. It was introduced over 2 years ago in commit 77fa4e0729987: commit 77fa4e072998705883c4dc672963b4bf7483cea9 Author: Shuah Khan Date: Thu Feb 11 21:41:29 2016 -0200 [media] media: Change v4l-core to check if source is free Change s_input, s_fmt, s_tuner, s_frequency, querystd, s_hw_freq_seek, and vb2_core_streamon interfaces that alter the tuner configuration to check if it is free, by calling v4l_enable_media_source(). If source isn't free, return -EBUSY. v4l_disable_media_source() is called from v4l2_fh_exit() to release tuner (source). vb2_core_streamon() uses v4l_vb2q_enable_media_source(). Signed-off-by: Shuah Khan Signed-off-by: Mauro Carvalho Chehab Note that this precedes the DVB_MMAP config option, so at the time this likely worked fine. But I wonder why it works if that DVB_MMAP option is set? Pure luck? I think this call should be moved to videobuf2-v4l2.c (might require some refactoring). It really doesn't belong in videobuf2-core.c. Regards, Hans
[PATCHv2 1/4] vb2: add waiting_in_dqbuf flag
Calling VIDIOC_DQBUF can release the core serialization lock pointed to
by vb2_queue->lock if it has to wait for a new buffer to arrive.
However, if userspace dup()ped the video device filehandle, then it is
possible to read or call DQBUF from two filehandles at the same time.
It is also possible to call REQBUFS from one filehandle while the other
is waiting for a buffer. This will remove all the buffers and reallocate
new ones. Removing all the buffers isn't the problem here (that's already
handled correctly by DQBUF), but the reallocating part is: DQBUF isn't
aware that the buffers have changed.
This is fixed by setting a flag whenever the lock is released while waiting
for a buffer to arrive. And checking the flag where needed so we can return
-EBUSY.
Signed-off-by: Hans Verkuil
Reported-by: syzbot+4180ff9ca6810b06c...@syzkaller.appspotmail.com
---
.../media/common/videobuf2/videobuf2-core.c | 22 +++
include/media/videobuf2-core.h| 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index 975ff5669f72..f7e7e633bcd7 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -672,6 +672,11 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
return -EBUSY;
}
+ if (q->waiting_in_dqbuf && *count) {
+ dprintk(1, "another dup()ped fd is waiting for a buffer\n");
+ return -EBUSY;
+ }
+
if (*count == 0 || q->num_buffers != 0 ||
(q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) {
/*
@@ -809,6 +814,10 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum
vb2_memory memory,
}
if (!q->num_buffers) {
+ if (q->waiting_in_dqbuf && *count) {
+ dprintk(1, "another dup()ped fd is waiting for a
buffer\n");
+ return -EBUSY;
+ }
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
q->waiting_for_buffers = !q->is_output;
@@ -1621,6 +1630,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
int nonblocking)
for (;;) {
int ret;
+ if (q->waiting_in_dqbuf) {
+ dprintk(1, "another dup()ped fd is waiting for a
buffer\n");
+ return -EBUSY;
+ }
+
if (!q->streaming) {
dprintk(1, "streaming off, will not wait for
buffers\n");
return -EINVAL;
@@ -1648,6 +1662,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
int nonblocking)
return -EAGAIN;
}
+ q->waiting_in_dqbuf = 1;
/*
* We are streaming and blocking, wait for another buffer to
* become ready or for streamoff. Driver's lock is released to
@@ -1668,6 +1683,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
int nonblocking)
* the locks or return an error if one occurred.
*/
call_void_qop(q, wait_finish, q);
+ q->waiting_in_dqbuf = 0;
if (ret) {
dprintk(1, "sleep was interrupted\n");
return ret;
@@ -2539,6 +2555,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
if (!data)
return -EINVAL;
+ if (q->waiting_in_dqbuf) {
+ dprintk(3, "another dup()ped fd is %s\n",
+ read ? "reading" : "writing");
+ return -EBUSY;
+ }
+
/*
* Initialize emulator on first call.
*/
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e86981d615ae..613f22910174 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -584,6 +584,7 @@ struct vb2_queue {
unsigned intstart_streaming_called:1;
unsigned interror:1;
unsigned intwaiting_for_buffers:1;
+ unsigned intwaiting_in_dqbuf:1;
unsigned intis_multiplanar:1;
unsigned intis_output:1;
unsigned intcopy_timestamp:1;
--
2.19.1
[PATCHv2 3/4] vb2 core: add new queue_setup_lock/unlock ops
If queue->lock is different from the video_device lock, then
you need to serialize queue_setup with VIDIOC_S_FMT, and this
should be done by the driver.
Signed-off-by: Hans Verkuil
---
.../media/common/videobuf2/videobuf2-core.c | 51 +--
include/media/videobuf2-core.h| 19 +++
2 files changed, 55 insertions(+), 15 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index f7e7e633bcd7..269485920beb 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -465,7 +465,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
int buffers)
*/
if (q->num_buffers) {
bool unbalanced = q->cnt_start_streaming !=
q->cnt_stop_streaming ||
- q->cnt_wait_prepare != q->cnt_wait_finish;
+ q->cnt_wait_prepare != q->cnt_wait_finish ||
+ q->cnt_queue_setup_lock !=
q->cnt_queue_setup_unlock;
if (unbalanced || debug) {
pr_info("counters for queue %p:%s\n", q,
@@ -473,10 +474,14 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
int buffers)
pr_info(" setup: %u start_streaming: %u
stop_streaming: %u\n",
q->cnt_queue_setup, q->cnt_start_streaming,
q->cnt_stop_streaming);
+ pr_info(" queue_setup_lock: %u queue_setup_unlock:
%u\n",
+ q->cnt_queue_setup_lock,
q->cnt_queue_setup_unlock);
pr_info(" wait_prepare: %u wait_finish: %u\n",
q->cnt_wait_prepare, q->cnt_wait_finish);
}
q->cnt_queue_setup = 0;
+ q->cnt_queue_setup_lock = 0;
+ q->cnt_queue_setup_unlock = 0;
q->cnt_wait_prepare = 0;
q->cnt_wait_finish = 0;
q->cnt_start_streaming = 0;
@@ -717,6 +722,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME);
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
+ call_void_qop(q, queue_setup_lock, q);
/*
* Ask the driver how many buffers and planes per buffer it requires.
@@ -725,22 +731,27 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
plane_sizes, q->alloc_devs);
if (ret)
- return ret;
+ goto unlock;
/* Check that driver has set sane values */
- if (WARN_ON(!num_planes))
- return -EINVAL;
+ if (WARN_ON(!num_planes)) {
+ ret = -EINVAL;
+ goto unlock;
+ }
for (i = 0; i < num_planes; i++)
- if (WARN_ON(!plane_sizes[i]))
- return -EINVAL;
+ if (WARN_ON(!plane_sizes[i])) {
+ ret = -EINVAL;
+ goto unlock;
+ }
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes,
plane_sizes);
if (allocated_buffers == 0) {
dprintk(1, "memory allocation failed\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto unlock;
}
/*
@@ -775,19 +786,19 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
*/
}
- mutex_lock(&q->mmap_lock);
q->num_buffers = allocated_buffers;
+ call_void_qop(q, queue_setup_unlock, q);
if (ret < 0) {
/*
* Note: __vb2_queue_free() will subtract 'allocated_buffers'
* from q->num_buffers.
*/
+ mutex_lock(&q->mmap_lock);
__vb2_queue_free(q, allocated_buffers);
mutex_unlock(&q->mmap_lock);
return ret;
}
- mutex_unlock(&q->mmap_lock);
/*
* Return the number of successfully allocated buffers
@@ -795,8 +806,11 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
*/
*count = allocated_buffers;
q->waiting_for_buffers = !q->is_output;
-
return 0;
+
+unlock:
+ call_void_qop(q, queue_setup_unlock, q);
+ return ret;
}
EXPORT_SYMBOL_GPL(vb2_core_reqbufs);
@@ -813,10 +827,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum
vb2_memory memory,
return -ENOBUFS;
}
+ call_void_qop(q, queue_setup_lock, q);
if (!q->num_buffers) {
if (q->waiting_in_dqbuf && *count) {
[PATCHv2 0/4] vb2: fix syzkaller race conditions
These four patches fix syzkaller race conditions. The first patch fixes the case where VIDIOC_DQBUF (and indirectly via read()/write()) can release the core serialization mutex, thus allowing another thread to access the same vb2 queue through a dup()ped filehandle. If no new buffer is available the DQBUF ioctl will block and wait for a new buffer to arrive. Before it waits it releases the serialization lock, and afterwards it reacquires it. This is intentional, since you do not want to block other ioctls while waiting for a buffer. But this means that you need to flag that you are waiting for a buffer and check the flag in the appropriate places. Specifically, that has to happen for VIDIOC_REQBUFS and VIDIOC_CREATE_BUFS since those can free/reallocate all buffers. Obviously you should not do that while waiting for a new frame to arrive. The other place where the flag should be checked is in VIDIOC_DQBUF and read/write since it makes not sense to call those while another fd is already waiting for a new frame. The remaining three patches fix a problem with vivid: due to the way vivid was designed it had to release the dev->mutex lock when stop_streaming was called. However, that was the same lock that was assigned to queue->lock, so that caused a race condition as well. It really is a vivid bug, which I fixed by giving each queue its own lock instead of relying on dev->mutex. It is a good idea to have vivid do this, since, while vb2 has allowed this for a long time, no drivers were actually using that feature. But while analyzing the behavior of vivid and vb2 in this scenario I realized that doing this (i.e. separate mutexes per queue) caused another race between calling queue_setup and VIDIOC_S_FMT: if queue->lock and the ioctl serialization lock are the same, then those operations are nicely serialized. But if the locks are different, then it is possible that S_FMT changes the buffer size right after queue_setup returns. So queue_setup might report that each buffer is 1 MB, while the S_FMT changes it to 2 MB. So there is now a mismatch between what vb2 thinks the size should be and what the driver thinks. So to do this correctly the ioctl serialization lock (or whatever the driver uses for that) should be taken before calling queue_setup and released once q->num_buffers has been updated (so vb2_is_busy() will return true). The final two patches add support for that. Regards, Hans Hans Verkuil (4): vb2: add waiting_in_dqbuf flag vivid: use per-queue mutexes instead of one global mutex. vb2 core: add new queue_setup_lock/unlock ops vivid: add queue_setup_(un)lock ops .../media/common/videobuf2/videobuf2-core.c | 71 +++ drivers/media/platform/vivid/vivid-core.c | 29 ++-- drivers/media/platform/vivid/vivid-core.h | 8 +++ .../media/platform/vivid/vivid-kthread-cap.c | 2 - .../media/platform/vivid/vivid-kthread-out.c | 2 - drivers/media/platform/vivid/vivid-sdr-cap.c | 4 +- drivers/media/platform/vivid/vivid-vbi-cap.c | 2 + drivers/media/platform/vivid/vivid-vbi-out.c | 2 + drivers/media/platform/vivid/vivid-vid-cap.c | 2 + drivers/media/platform/vivid/vivid-vid-out.c | 2 + include/media/videobuf2-core.h| 20 ++ 11 files changed, 119 insertions(+), 25 deletions(-) -- 2.19.1
[PATCHv2 4/4] vivid: add queue_setup_(un)lock ops
Add these ops to serialize queue_setup with VIDIOC_S_FMT.
Signed-off-by: Hans Verkuil
---
drivers/media/platform/vivid/vivid-core.c| 14 ++
drivers/media/platform/vivid/vivid-core.h| 3 +++
drivers/media/platform/vivid/vivid-sdr-cap.c | 2 ++
drivers/media/platform/vivid/vivid-vbi-cap.c | 2 ++
drivers/media/platform/vivid/vivid-vbi-out.c | 2 ++
drivers/media/platform/vivid/vivid-vid-cap.c | 2 ++
drivers/media/platform/vivid/vivid-vid-out.c | 2 ++
7 files changed, 27 insertions(+)
diff --git a/drivers/media/platform/vivid/vivid-core.c
b/drivers/media/platform/vivid/vivid-core.c
index 38389af97b16..51b0a5c99365 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -475,6 +475,20 @@ static int vivid_fop_release(struct file *file)
return v4l2_fh_release(file);
}
+void vivid_queue_setup_lock(struct vb2_queue *q)
+{
+ struct vivid_dev *dev = vb2_get_drv_priv(q);
+
+ mutex_lock(&dev->mutex);
+}
+
+void vivid_queue_setup_unlock(struct vb2_queue *q)
+{
+ struct vivid_dev *dev = vb2_get_drv_priv(q);
+
+ mutex_unlock(&dev->mutex);
+}
+
static const struct v4l2_file_operations vivid_fops = {
.owner = THIS_MODULE,
.open = v4l2_fh_open,
diff --git a/drivers/media/platform/vivid/vivid-core.h
b/drivers/media/platform/vivid/vivid-core.h
index 337ccb563f9b..78c97c1dcd25 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -564,4 +564,7 @@ static inline bool vivid_is_hdmi_out(const struct vivid_dev
*dev)
return dev->output_type[dev->output] == HDMI;
}
+void vivid_queue_setup_lock(struct vb2_queue *q);
+void vivid_queue_setup_unlock(struct vb2_queue *q);
+
#endif
diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c
b/drivers/media/platform/vivid/vivid-sdr-cap.c
index 5dfb598af742..bac0dc47e24e 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -318,6 +318,8 @@ static void sdr_cap_buf_request_complete(struct vb2_buffer
*vb)
const struct vb2_ops vivid_sdr_cap_qops = {
.queue_setup= sdr_cap_queue_setup,
+ .queue_setup_lock = vivid_queue_setup_lock,
+ .queue_setup_unlock = vivid_queue_setup_unlock,
.buf_prepare= sdr_cap_buf_prepare,
.buf_queue = sdr_cap_buf_queue,
.start_streaming= sdr_cap_start_streaming,
diff --git a/drivers/media/platform/vivid/vivid-vbi-cap.c
b/drivers/media/platform/vivid/vivid-vbi-cap.c
index 903cebeb5ce5..b5c0ea8b848c 100644
--- a/drivers/media/platform/vivid/vivid-vbi-cap.c
+++ b/drivers/media/platform/vivid/vivid-vbi-cap.c
@@ -231,6 +231,8 @@ static void vbi_cap_buf_request_complete(struct vb2_buffer
*vb)
const struct vb2_ops vivid_vbi_cap_qops = {
.queue_setup= vbi_cap_queue_setup,
+ .queue_setup_lock = vivid_queue_setup_lock,
+ .queue_setup_unlock = vivid_queue_setup_unlock,
.buf_prepare= vbi_cap_buf_prepare,
.buf_queue = vbi_cap_buf_queue,
.start_streaming= vbi_cap_start_streaming,
diff --git a/drivers/media/platform/vivid/vivid-vbi-out.c
b/drivers/media/platform/vivid/vivid-vbi-out.c
index 9357c07e30d6..8f8ce00edaa2 100644
--- a/drivers/media/platform/vivid/vivid-vbi-out.c
+++ b/drivers/media/platform/vivid/vivid-vbi-out.c
@@ -126,6 +126,8 @@ static void vbi_out_buf_request_complete(struct vb2_buffer
*vb)
const struct vb2_ops vivid_vbi_out_qops = {
.queue_setup= vbi_out_queue_setup,
+ .queue_setup_lock = vivid_queue_setup_lock,
+ .queue_setup_unlock = vivid_queue_setup_unlock,
.buf_prepare= vbi_out_buf_prepare,
.buf_queue = vbi_out_buf_queue,
.start_streaming= vbi_out_start_streaming,
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c
b/drivers/media/platform/vivid/vivid-vid-cap.c
index 9c8e8be81ce3..f315f5f72616 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -271,6 +271,8 @@ static void vid_cap_buf_request_complete(struct vb2_buffer
*vb)
const struct vb2_ops vivid_vid_cap_qops = {
.queue_setup= vid_cap_queue_setup,
+ .queue_setup_lock = vivid_queue_setup_lock,
+ .queue_setup_unlock = vivid_queue_setup_unlock,
.buf_prepare= vid_cap_buf_prepare,
.buf_finish = vid_cap_buf_finish,
.buf_queue = vid_cap_buf_queue,
diff --git a/drivers/media/platform/vivid/vivid-vid-out.c
b/drivers/media/platform/vivid/vivid-vid-out.c
index aaf13f03d5d4..0fe7f449e416 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -190,6 +190,8 @@ static void vid_out_buf_request_complete(struct vb2_buffer
*
[PATCHv2 2/4] vivid: use per-queue mutexes instead of one global mutex.
This avoids having to unlock the queue lock in stop_streaming.
Signed-off-by: Hans Verkuil
Reported-by: syzbot+736c3aae4af7b50d9...@syzkaller.appspotmail.com
---
drivers/media/platform/vivid/vivid-core.c| 15 ++-
drivers/media/platform/vivid/vivid-core.h| 5 +
drivers/media/platform/vivid/vivid-kthread-cap.c | 2 --
drivers/media/platform/vivid/vivid-kthread-out.c | 2 --
drivers/media/platform/vivid/vivid-sdr-cap.c | 2 --
5 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-core.c
b/drivers/media/platform/vivid/vivid-core.c
index 626e2b24a403..38389af97b16 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -1075,7 +1075,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vid_cap_q_lock);
+ q->lock = &dev->vb_vid_cap_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1096,7 +1097,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vid_out_q_lock);
+ q->lock = &dev->vb_vid_out_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1117,7 +1119,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vbi_cap_q_lock);
+ q->lock = &dev->vb_vbi_cap_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1138,7 +1141,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vbi_out_q_lock);
+ q->lock = &dev->vb_vbi_out_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1158,7 +1162,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 8;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_sdr_cap_q_lock);
+ q->lock = &dev->vb_sdr_cap_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
diff --git a/drivers/media/platform/vivid/vivid-core.h
b/drivers/media/platform/vivid/vivid-core.h
index 1891254c8f0b..337ccb563f9b 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -385,8 +385,10 @@ struct vivid_dev {
struct v4l2_rectcompose_cap;
struct v4l2_rectcrop_bounds_cap;
struct vb2_queuevb_vid_cap_q;
+ struct mutexvb_vid_cap_q_lock;
struct list_headvid_cap_active;
struct vb2_queuevb_vbi_cap_q;
+ struct mutexvb_vbi_cap_q_lock;
struct list_headvbi_cap_active;
/* thread for generating video capture stream */
@@ -413,8 +415,10 @@ struct vivid_dev {
struct v4l2_rectcompose_out;
struct v4l2_rectcompose_bounds_out;
struct vb2_queuevb_vid_out_q;
+ struct mutexvb_vid_out_q_lock;
struct list_headvid_out_active;
struct vb2_queuevb_vbi_out_q;
+ struct mutexvb_vbi_out_q_lock;
struct list_headvbi_out_active;
/* video loop precalculated rectangles */
@@ -459,6 +463,7 @@ struct vivid_dev {
/* SDR capture */
struct vb2_queuevb_sdr_cap_q;
+ struct mutexvb_sdr_cap_q_lock;
struct list_headsdr_cap_active;
u32 sdr_pixelformat; /* v4l2 format id */
unsignedsdr_buffersize;
diff --git a/drivers/media/platform/vivid/vi
Re: [PATCHv2 0/9] vb2/cedrus: add tag support
On 11/14/2018 02:47 PM, Hans Verkuil wrote: > From: Hans Verkuil > > As was discussed here (among other places): > > https://lkml.org/lkml/2018/10/19/440 > > using capture queue buffer indices to refer to reference frames is > not a good idea. A better idea is to use a 'tag' where the > application can assign a u64 tag to an output buffer, which is then > copied to the capture buffer(s) derived from the output buffer. > > A u64 is chosen since this allows userspace to also use pointers to > internal structures as 'tag'. > > The first three patches add core tag support, the next patch document > the tag support, then a new helper function is added to v4l2-mem2mem.c > to easily copy data from a source to a destination buffer that drivers > can use. > > Next a new supports_tags vb2_queue flag is added to indicate that > the driver supports tags. Ideally this should not be necessary, but > that would require that all m2m drivers are converted to using the > new helper function introduced in the previous patch. That takes more > time then I have now since we want to get this in for 4.20. > > Finally the vim2m, vicodec and cedrus drivers are converted to support > tags. > > I also removed the 'pad' fields from the mpeg2 control structs (it > should never been added in the first place) and aligned the structs > to a u32 boundary (u64 for the tag values). > > Note that this might change further (Paul suggested using bitfields). > > Also note that the cedrus code doesn't set the sequence counter, that's > something that should still be added before this driver can be moved > out of staging. > > Note: if no buffer is found for a certain tag, then the dma address > is just set to 0. That happened before as well with invalid buffer > indices. This should be checked in the driver! > > The previous RFC series was tested successfully with the cedrus driver. > > Regards, > > Hans I'd like to get some Acked-by or Reviewed-by replies for this series. Or comments if you don't like something. I would really like to get this in for 4.20 so the cedrus API is stable (hopefully), since this is a last outstanding API item. Tomasz: you commented that the text still referred to the tag as a u64, but that was only in the cover letter, not the patches themselves. So I don't plan to post a v3 just for that. Regards, Hans > > Changes since v1: > > - changed to a u32 tag. Using a 64 bit tag was overly complicated due > to the bad layout of the v4l2_buffer struct, and there is no real > need for it by applications. > > Main changes since the RFC: > > - Added new buffer capability flag > - Added m2m helper to copy data between buffers > - Added documentation > - Added tag logging in v4l2-ioctl.c > > Hans Verkuil (9): > videodev2.h: add tag support > vb2: add tag support > v4l2-ioctl.c: log v4l2_buffer tag > buffer.rst: document the new buffer tag feature. > v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function > vb2: add new supports_tags queue flag > vim2m: add tag support > vicodec: add tag support > cedrus: add tag support > > Documentation/media/uapi/v4l/buffer.rst | 32 + > .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++ > .../media/common/videobuf2/videobuf2-v4l2.c | 45 --- > drivers/media/platform/vicodec/vicodec-core.c | 14 ++ > drivers/media/platform/vim2m.c| 14 ++ > drivers/media/v4l2-core/v4l2-ctrls.c | 9 > drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++-- > drivers/media/v4l2-core/v4l2-mem2mem.c| 23 ++ > drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++-- > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 - > .../staging/media/sunxi/cedrus/cedrus_video.c | 2 + > include/media/v4l2-mem2mem.h | 21 + > include/media/videobuf2-core.h| 2 + > include/media/videobuf2-v4l2.h| 21 - > include/uapi/linux/v4l2-controls.h| 14 +++--- > include/uapi/linux/videodev2.h| 9 +++- > 17 files changed, 178 insertions(+), 73 deletions(-) >
[PATCH v3] media: staging: tegra-vde: Replace debug messages with trace points
Trace points are much more efficient than debug messages for intensive
tracing and could be conveniently enabled / disabled dynamically, hence
let's replace debug messages with the trace points. This also makes
code a bit cleaner.
Signed-off-by: Dmitry Osipenko
---
Changelog:
v3: - Basically a RE-SEND of v2 with some very minor code reshuffling.
v2:
- Use __assign_str() for copying of HW sub-engine name during of
tracing. There is no functional changes since V1, that's just a
bit better variant of the patch that doesn't rely on stopping
tracing before releasing of managed resources (struct tegra_vde).
drivers/staging/media/tegra-vde/tegra-vde.c | 222 +++-
drivers/staging/media/tegra-vde/trace.h | 93
2 files changed, 220 insertions(+), 95 deletions(-)
create mode 100644 drivers/staging/media/tegra-vde/trace.h
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 66cf14212c14..aa6c6bba961e 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -35,14 +35,6 @@
#define BSE_ICMDQUE_EMPTY BIT(3)
#define BSE_DMA_BUSY BIT(23)
-#define VDE_WR(__data, __addr) \
-do { \
- dev_dbg(vde->miscdev.parent,\
- "%s: %d: 0x%08X => " #__addr ")\n", \
- __func__, __LINE__, (u32)(__data)); \
- writel_relaxed(__data, __addr); \
-} while (0)
-
struct video_frame {
struct dma_buf_attachment *y_dmabuf_attachment;
struct dma_buf_attachment *cb_dmabuf_attachment;
@@ -81,12 +73,66 @@ struct tegra_vde {
u32 *iram;
};
+static __maybe_unused char const *
+tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
+{
+ if (vde->sxe == base)
+ return "SXE";
+
+ if (vde->bsev == base)
+ return "BSEV";
+
+ if (vde->mbe == base)
+ return "MBE";
+
+ if (vde->ppe == base)
+ return "PPE";
+
+ if (vde->mce == base)
+ return "MCE";
+
+ if (vde->tfe == base)
+ return "TFE";
+
+ if (vde->ppb == base)
+ return "PPB";
+
+ if (vde->vdma == base)
+ return "VDMA";
+
+ if (vde->frameid == base)
+ return "FRAMEID";
+
+ return "???";
+}
+
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
+static void tegra_vde_writel(struct tegra_vde *vde,
+u32 value, void __iomem *base, u32 offset)
+{
+ trace_vde_writel(vde, base, offset, value);
+
+ writel_relaxed(value, base + offset);
+}
+
+static u32 tegra_vde_readl(struct tegra_vde *vde,
+ void __iomem *base, u32 offset)
+{
+ u32 value = readl_relaxed(base + offset);
+
+ trace_vde_readl(vde, base, offset, value);
+
+ return value;
+}
+
static void tegra_vde_set_bits(struct tegra_vde *vde,
- u32 mask, void __iomem *regs)
+ u32 mask, void __iomem *base, u32 offset)
{
- u32 value = readl_relaxed(regs);
+ u32 value = tegra_vde_readl(vde, base, offset);
- VDE_WR(value | mask, regs);
+ tegra_vde_writel(vde, value | mask, base, offset);
}
static int tegra_vde_wait_mbe(struct tegra_vde *vde)
@@ -107,8 +153,8 @@ static int tegra_vde_setup_mbe_frame_idx(struct tegra_vde
*vde,
unsigned int idx;
int err;
- VDE_WR(0xD000 | (0 << 23), vde->mbe + 0x80);
- VDE_WR(0xD020 | (0 << 23), vde->mbe + 0x80);
+ tegra_vde_writel(vde, 0xD000 | (0 << 23), vde->mbe, 0x80);
+ tegra_vde_writel(vde, 0xD020 | (0 << 23), vde->mbe, 0x80);
err = tegra_vde_wait_mbe(vde);
if (err)
@@ -118,8 +164,10 @@ static int tegra_vde_setup_mbe_frame_idx(struct tegra_vde
*vde,
return 0;
for (idx = 0, frame_idx = 1; idx < refs_nb; idx++, frame_idx++) {
- VDE_WR(0xD000 | (frame_idx << 23), vde->mbe + 0x80);
- VDE_WR(0xD020 | (frame_idx << 23), vde->mbe + 0x80);
+ tegra_vde_writel(vde, 0xD000 | (frame_idx << 23),
+vde->mbe, 0x80);
+ tegra_vde_writel(vde, 0xD020 | (frame_idx << 23),
+vde->mbe, 0x80);
frame_idx_enb_mask |= frame_idx << (6 * (idx % 4));
@@ -128,7 +176,7 @@ static int tegra_vde_setup_mbe_frame_idx(struct tegra_vde
*vde,
value |= (idx >> 2) << 24;
value |= frame_idx_enb_mask;
- VDE_WR(value, vde->mbe + 0x80);
+ tegra_vde_writel(vde, value, vde->mbe, 0x80);
err = tegra_vde_wait_mbe(vde);
if (err)
@@ -143,8 +191,
[GIT FIXES FOR v4.20] vicodec fix, add action to cedrus TODO
The new cedrus driver was missing an important action item in the TODO. Fix a nasty buffer overrun in vicodec. Regards, Hans The following changes since commit fbe57dde7126d1b2712ab5ea93fb9d15f89de708: media: ov7740: constify structures stored in fields of v4l2_subdev_ops structure (2018-11-06 07:17:02 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git tags/br-v4.20l for you to fetch changes up to e90e965c32b299fe89d23f490146963fbbf490dd: vicodec: fix memchr() kernel oops (2018-11-19 12:21:26 +0100) Tag branch Hans Verkuil (2): cedrus: add action item to the TODO vicodec: fix memchr() kernel oops drivers/media/platform/vicodec/vicodec-core.c | 3 ++- drivers/staging/media/sunxi/cedrus/TODO | 5 + 2 files changed, 7 insertions(+), 1 deletion(-)
Re: [PATCHv2 2/4] vivid: use per-queue mutexes instead of one global mutex.
Hi Hans,
On Mon, Nov 19, 2018 at 12:09:01PM +0100, Hans Verkuil wrote:
> This avoids having to unlock the queue lock in stop_streaming.
>
> Signed-off-by: Hans Verkuil
> Reported-by: syzbot+736c3aae4af7b50d9...@syzkaller.appspotmail.com
> ---
> drivers/media/platform/vivid/vivid-core.c| 15 ++-
> drivers/media/platform/vivid/vivid-core.h| 5 +
> drivers/media/platform/vivid/vivid-kthread-cap.c | 2 --
> drivers/media/platform/vivid/vivid-kthread-out.c | 2 --
> drivers/media/platform/vivid/vivid-sdr-cap.c | 2 --
> 5 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/vivid/vivid-core.c
> b/drivers/media/platform/vivid/vivid-core.c
> index 626e2b24a403..38389af97b16 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -1075,7 +1075,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vid_cap_q_lock);
Could you add the corresponding mutex_destroy()'s for these mutex_init()
calls?
> + q->lock = &dev->vb_vid_cap_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1096,7 +1097,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vid_out_q_lock);
> + q->lock = &dev->vb_vid_out_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1117,7 +1119,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vbi_cap_q_lock);
> + q->lock = &dev->vb_vbi_cap_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1138,7 +1141,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vbi_out_q_lock);
> + q->lock = &dev->vb_vbi_out_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1158,7 +1162,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 8;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_sdr_cap_q_lock);
> + q->lock = &dev->vb_sdr_cap_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> diff --git a/drivers/media/platform/vivid/vivid-core.h
> b/drivers/media/platform/vivid/vivid-core.h
> index 1891254c8f0b..337ccb563f9b 100644
> --- a/drivers/media/platform/vivid/vivid-core.h
> +++ b/drivers/media/platform/vivid/vivid-core.h
> @@ -385,8 +385,10 @@ struct vivid_dev {
> struct v4l2_rectcompose_cap;
> struct v4l2_rectcrop_bounds_cap;
> struct vb2_queuevb_vid_cap_q;
> + struct mutexvb_vid_cap_q_lock;
> struct list_headvid_cap_active;
> struct vb2_queuevb_vbi_cap_q;
> + struct mutexvb_vbi_cap_q_lock;
> struct list_headvbi_cap_active;
>
> /* thread for generating video capture stream */
> @@ -413,8 +415,10 @@ struct vivid_dev {
> struct v4l2_rectcompose_out;
> struct v4l2_rectcompose_bounds_out;
> struct vb2_queuevb_vid_out_q;
> + struct mutexvb_vid_out_q_lock;
> struct list_headvid_out_active;
> struct vb2_queuevb_vbi_out_q;
> + struct mutexvb_vbi_out_q_lock;
> struct list_headvbi_out_active;
>
> /* video loop precalculated rectangles */
> @@ -459,6 +463,7 @@ struct vivid_dev {
>
> /* SDR capture */
> struct vb2_queuevb_sdr_cap_q;
> + struct mutexvb_sdr_cap_q_
Re: [linux-sunxi] [PATCH v12 2/2] media: V3s: Add support for Allwinner CSI.
On Tue, Oct 30, 2018 at 1:49 PM Yong Deng wrote:
>
> Allwinner V3s SoC features a CSI module with parallel interface.
>
> This patch implement a v4l2 framework driver for it.
>
> Reviewed-by: Hans Verkuil
> Reviewed-by: Maxime Ripard
> Tested-by: Maxime Ripard
> Signed-off-by: Yong Deng
> ---
> MAINTAINERS| 8 +
> drivers/media/platform/Kconfig | 1 +
> drivers/media/platform/Makefile| 2 +
> drivers/media/platform/sunxi/sun6i-csi/Kconfig | 9 +
> drivers/media/platform/sunxi/sun6i-csi/Makefile| 3 +
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 915
> +
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 135 +++
> .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 196 +
> .../media/platform/sunxi/sun6i-csi/sun6i_video.c | 678 +++
> .../media/platform/sunxi/sun6i-csi/sun6i_video.h | 38 +
> 10 files changed, 1985 insertions(+)
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 23021e0df5d7..42d73b35ed3e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3900,6 +3900,14 @@ M: Jaya Kumar
> S: Maintained
> F: sound/pci/cs5535audio/
>
> +CSI DRIVERS FOR ALLWINNER V3s
> +M: Yong Deng
> +L: linux-media@vger.kernel.org
> +T: git git://linuxtv.org/media_tree.git
> +S: Maintained
> +F: drivers/media/platform/sunxi/sun6i-csi/
> +F: Documentation/devicetree/bindings/media/sun6i-csi.txt
> +
> CW1200 WLAN driver
> M: Solomon Peachy
> S: Maintained
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 0edacfb01f3a..be6626ed0ec8 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -138,6 +138,7 @@ source "drivers/media/platform/am437x/Kconfig"
> source "drivers/media/platform/xilinx/Kconfig"
> source "drivers/media/platform/rcar-vin/Kconfig"
> source "drivers/media/platform/atmel/Kconfig"
> +source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
[snip]
> +
> + return 0;
> +}
> +
> +static int sun6i_subdev_notify_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct sun6i_csi *csi = container_of(notifier, struct sun6i_csi,
> +notifier);
> + struct v4l2_device *v4l2_dev = &csi->v4l2_dev;
> + struct v4l2_subdev *sd;
> + int ret;
> +
> + dev_dbg(csi->dev, "notify complete, all subdevs registered\n");
> +
> + if (notifier->num_subdevs != 1)
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c: In function
‘sun6i_subdev_notify_complete’:
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:646:14: error:
‘struct v4l2_async_notifier’ has no member named ‘num_subdevs’
[PATCHv2.1 2/4] vivid: use per-queue mutexes instead of one global mutex.
>From d15ccd98e557a8cef1362cb591eb3011a6d8e1fd Mon Sep 17 00:00:00 2001
From: Hans Verkuil
Date: Fri, 16 Nov 2018 12:14:31 +0100
Subject: [PATCH 2/4] vivid: use per-queue mutexes instead of one global mutex.
This avoids having to unlock the queue lock in stop_streaming.
Signed-off-by: Hans Verkuil
Reported-by: syzbot+736c3aae4af7b50d9...@syzkaller.appspotmail.com
---
Changes since v2:
- add mutex_destroy()
---
drivers/media/platform/vivid/vivid-core.c | 26 +++
drivers/media/platform/vivid/vivid-core.h | 5
.../media/platform/vivid/vivid-kthread-cap.c | 2 --
.../media/platform/vivid/vivid-kthread-out.c | 2 --
drivers/media/platform/vivid/vivid-sdr-cap.c | 2 --
5 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-core.c
b/drivers/media/platform/vivid/vivid-core.c
index 626e2b24a403..9a548eea75cd 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -624,6 +624,17 @@ static void vivid_dev_release(struct v4l2_device *v4l2_dev)
vfree(dev->bitmap_out);
tpg_free(&dev->tpg);
kfree(dev->query_dv_timings_qmenu);
+ if (dev->has_vid_cap)
+ mutex_destroy(&dev->vb_vid_cap_q_lock);
+ if (dev->has_vid_out)
+ mutex_destroy(&dev->vb_vid_out_q_lock);
+ if (dev->has_vbi_cap)
+ mutex_destroy(&dev->vb_vbi_cap_q_lock);
+ if (dev->has_vbi_out)
+ mutex_destroy(&dev->vb_vbi_out_q_lock);
+ if (dev->has_sdr_cap)
+ mutex_destroy(&dev->vb_sdr_cap_q_lock);
+ mutex_destroy(&dev->mutex);
kfree(dev);
}
@@ -1075,7 +1086,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vid_cap_q_lock);
+ q->lock = &dev->vb_vid_cap_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1096,7 +1108,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vid_out_q_lock);
+ q->lock = &dev->vb_vid_out_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1117,7 +1130,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vbi_cap_q_lock);
+ q->lock = &dev->vb_vbi_cap_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1138,7 +1152,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_vbi_out_q_lock);
+ q->lock = &dev->vb_vbi_out_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
@@ -1158,7 +1173,8 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->mem_ops = vivid_mem_ops[allocator];
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 8;
- q->lock = &dev->mutex;
+ mutex_init(&dev->vb_sdr_cap_q_lock);
+ q->lock = &dev->vb_sdr_cap_q_lock;
q->dev = dev->v4l2_dev.dev;
q->supports_requests = true;
diff --git a/drivers/media/platform/vivid/vivid-core.h
b/drivers/media/platform/vivid/vivid-core.h
index 1891254c8f0b..337ccb563f9b 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -385,8 +385,10 @@ struct vivid_dev {
struct v4l2_rectcompose_cap;
struct v4l2_rectcrop_bounds_cap;
struct vb2_queuevb_vid_cap_q;
+ struct mutexvb_vid_cap_q_lock;
struct list_headvid_cap_active;
struct vb2_queuevb_vbi_cap_q;
+ struct mutexvb_vbi_cap_q_lock;
struct list_headvbi_cap_active;
/* thread for generating video capture stream */
@@ -413,8 +415,10 @@ struct vivid_dev {
Re: [PATCHv2.1 2/4] vivid: use per-queue mutexes instead of one global mutex.
On 11/19/2018 01:22 PM, Hans Verkuil wrote:
> From d15ccd98e557a8cef1362cb591eb3011a6d8e1fd Mon Sep 17 00:00:00 2001
> From: Hans Verkuil
> Date: Fri, 16 Nov 2018 12:14:31 +0100
> Subject: [PATCH 2/4] vivid: use per-queue mutexes instead of one global mutex.
Oops, forgot to delete these four lines above. Just ignore them.
Regards,
Hans
>
> This avoids having to unlock the queue lock in stop_streaming.
>
> Signed-off-by: Hans Verkuil
> Reported-by: syzbot+736c3aae4af7b50d9...@syzkaller.appspotmail.com
> ---
> Changes since v2:
> - add mutex_destroy()
> ---
> drivers/media/platform/vivid/vivid-core.c | 26 +++
> drivers/media/platform/vivid/vivid-core.h | 5
> .../media/platform/vivid/vivid-kthread-cap.c | 2 --
> .../media/platform/vivid/vivid-kthread-out.c | 2 --
> drivers/media/platform/vivid/vivid-sdr-cap.c | 2 --
> 5 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/vivid/vivid-core.c
> b/drivers/media/platform/vivid/vivid-core.c
> index 626e2b24a403..9a548eea75cd 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -624,6 +624,17 @@ static void vivid_dev_release(struct v4l2_device
> *v4l2_dev)
> vfree(dev->bitmap_out);
> tpg_free(&dev->tpg);
> kfree(dev->query_dv_timings_qmenu);
> + if (dev->has_vid_cap)
> + mutex_destroy(&dev->vb_vid_cap_q_lock);
> + if (dev->has_vid_out)
> + mutex_destroy(&dev->vb_vid_out_q_lock);
> + if (dev->has_vbi_cap)
> + mutex_destroy(&dev->vb_vbi_cap_q_lock);
> + if (dev->has_vbi_out)
> + mutex_destroy(&dev->vb_vbi_out_q_lock);
> + if (dev->has_sdr_cap)
> + mutex_destroy(&dev->vb_sdr_cap_q_lock);
> + mutex_destroy(&dev->mutex);
> kfree(dev);
> }
>
> @@ -1075,7 +1086,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vid_cap_q_lock);
> + q->lock = &dev->vb_vid_cap_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1096,7 +1108,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vid_out_q_lock);
> + q->lock = &dev->vb_vid_out_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1117,7 +1130,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vbi_cap_q_lock);
> + q->lock = &dev->vb_vbi_cap_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1138,7 +1152,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 2;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_vbi_out_q_lock);
> + q->lock = &dev->vb_vbi_out_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> @@ -1158,7 +1173,8 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> q->mem_ops = vivid_mem_ops[allocator];
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->min_buffers_needed = 8;
> - q->lock = &dev->mutex;
> + mutex_init(&dev->vb_sdr_cap_q_lock);
> + q->lock = &dev->vb_sdr_cap_q_lock;
> q->dev = dev->v4l2_dev.dev;
> q->supports_requests = true;
>
> diff --git a/drivers/media/platform/vivid/vivid-core.h
> b/drivers/media/platform/vivid/vivid-core.h
> index 1891254c8f0b..337ccb563f9b 100644
> --- a/drivers/media/platform/vivid/vivid-core.h
> +++ b/drivers/media/platform/vivid/vivid-core.h
> @@ -385,8 +385,10 @@ struct vivid_dev {
> struct v4l2_rectcompose_cap;
> struct v4l2_rectcrop_bounds_cap;
> struct vb2_queuevb_vid_cap_q;
> + struct mutexvb_vid_cap_q_lock;
> struct list_headvid_cap_active;
> struct vb2_queue
Re: [PATCH v2 4/9] phy: dphy: Add configuration helpers
Hi Maxime,
Apologies for the delayed review. Please see my comments below.
On Tue, Nov 06, 2018 at 03:54:16PM +0100, Maxime Ripard wrote:
> The MIPI D-PHY spec defines default values and boundaries for most of the
> parameters it defines. Introduce helpers to help drivers get meaningful
> values based on their current parameters, and validate the boundaries of
> these parameters if needed.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/phy/Kconfig | 8 ++-
> drivers/phy/Makefile | 1 +-
> drivers/phy/phy-core-mipi-dphy.c | 160 +++-
> include/linux/phy/phy-mipi-dphy.h | 6 +-
> 4 files changed, 175 insertions(+)
> create mode 100644 drivers/phy/phy-core-mipi-dphy.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 60f949e2a684..c87a7d49eaab 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,14 @@ config GENERIC_PHY
> phy users can obtain reference to the PHY. All the users of this
> framework should select this config.
>
> +config GENERIC_PHY_MIPI_DPHY
> + bool
> + help
> + Generic MIPI D-PHY support.
> +
> + Provides a number of helpers a core functions for MIPI D-PHY
> + drivers to us.
> +
> config PHY_LPC18XX_USB_OTG
> tristate "NXP LPC18xx/43xx SoC USB OTG PHY driver"
> depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 0301e25d07c1..baec59cebbab 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -4,6 +4,7 @@
> #
>
> obj-$(CONFIG_GENERIC_PHY)+= phy-core.o
> +obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY) += phy-core-mipi-dphy.o
> obj-$(CONFIG_PHY_LPC18XX_USB_OTG)+= phy-lpc18xx-usb-otg.o
> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
> obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
> diff --git a/drivers/phy/phy-core-mipi-dphy.c
> b/drivers/phy/phy-core-mipi-dphy.c
> new file mode 100644
> index ..127ca6960084
> --- /dev/null
> +++ b/drivers/phy/phy-core-mipi-dphy.c
> @@ -0,0 +1,160 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2013 NVIDIA Corporation
> + * Copyright (C) 2018 Cadence Design Systems Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +/*
> + * Minimum D-PHY timings based on MIPI D-PHY specification. Derived
> + * from the valid ranges specified in Section 6.9, Table 14, Page 41
> + * of the D-PHY specification (v2.1).
I assume these values are compliant with the earlier spec releases.
> + */
> +int phy_mipi_dphy_get_default_config(unsigned long pixel_clock,
How about using the bus frequency instead of the pixel clock? Chances are
that the caller already has that information, instead of calculating it
here?
> + unsigned int bpp,
> + unsigned int lanes,
> + struct phy_configure_opts_mipi_dphy *cfg)
> +{
> + unsigned long hs_clk_rate;
> + unsigned long ui;
> +
> + if (!cfg)
> + return -EINVAL;
> +
> + hs_clk_rate = pixel_clock * bpp / lanes;
> + ui = DIV_ROUND_UP(NSEC_PER_SEC, hs_clk_rate);
Nanoseconds may not be precise enough for practical computations on these
values. At 1 GHz, this ends up being precisely 1. At least Intel hardware
has some more precision, I presume others do, too. How about using
picoseconds instead?
> +
> + cfg->clk_miss = 0;
> + cfg->clk_post = 60 + 52 * ui;
> + cfg->clk_pre = 8;
> + cfg->clk_prepare = 38;
> + cfg->clk_settle = 95;
> + cfg->clk_term_en = 0;
> + cfg->clk_trail = 60;
> + cfg->clk_zero = 262;
> + cfg->d_term_en = 0;
> + cfg->eot = 0;
> + cfg->hs_exit = 100;
> + cfg->hs_prepare = 40 + 4 * ui;
> + cfg->hs_zero = 105 + 6 * ui;
> + cfg->hs_settle = 85 + 6 * ui;
> + cfg->hs_skip = 40;
> +
> + /*
> + * The MIPI D-PHY specification (Section 6.9, v1.2, Table 14, Page 40)
> + * contains this formula as:
> + *
> + * T_HS-TRAIL = max(n * 8 * ui, 60 + n * 4 * ui)
> + *
> + * where n = 1 for forward-direction HS mode and n = 4 for reverse-
> + * direction HS mode. There's only one setting and this function does
> + * not parameterize on anything other that ui, so this code will
> + * assumes that reverse-direction HS mode is supported and uses n = 4.
> + */
> + cfg->hs_trail = max(4 * 8 * ui, 60 + 4 * 4 * ui);
> +
> + cfg->init = 10;
> + cfg->lpx = 60;
> + cfg->ta_get = 5 * cfg->lpx;
> + cfg->ta_go = 4 * cfg->lpx;
> + cfg->ta_sure = 2 * cfg->lpx;
> + cfg->wakeup = 100;
> +
> + cfg->hs_clk_rate = hs_clk_rate;
How about the LP clock?
Frankly, I have worked with MIPI CSI-2 hardware soon a decade, and the very
few cases where software has needed to deal with these values has been in
form of d
Re: [PATCHv2 0/9] vb2/cedrus: add tag support
Hi, On Mon, 2018-11-19 at 12:18 +0100, Hans Verkuil wrote: > On 11/14/2018 02:47 PM, Hans Verkuil wrote: > > From: Hans Verkuil > > > > As was discussed here (among other places): > > > > https://lkml.org/lkml/2018/10/19/440 > > > > using capture queue buffer indices to refer to reference frames is > > not a good idea. A better idea is to use a 'tag' where the > > application can assign a u64 tag to an output buffer, which is then > > copied to the capture buffer(s) derived from the output buffer. > > > > A u64 is chosen since this allows userspace to also use pointers to > > internal structures as 'tag'. > > > > The first three patches add core tag support, the next patch document > > the tag support, then a new helper function is added to v4l2-mem2mem.c > > to easily copy data from a source to a destination buffer that drivers > > can use. > > > > Next a new supports_tags vb2_queue flag is added to indicate that > > the driver supports tags. Ideally this should not be necessary, but > > that would require that all m2m drivers are converted to using the > > new helper function introduced in the previous patch. That takes more > > time then I have now since we want to get this in for 4.20. > > > > Finally the vim2m, vicodec and cedrus drivers are converted to support > > tags. > > > > I also removed the 'pad' fields from the mpeg2 control structs (it > > should never been added in the first place) and aligned the structs > > to a u32 boundary (u64 for the tag values). > > > > Note that this might change further (Paul suggested using bitfields). > > > > Also note that the cedrus code doesn't set the sequence counter, that's > > something that should still be added before this driver can be moved > > out of staging. > > > > Note: if no buffer is found for a certain tag, then the dma address > > is just set to 0. That happened before as well with invalid buffer > > indices. This should be checked in the driver! > > > > The previous RFC series was tested successfully with the cedrus driver. > > > > Regards, > > > > Hans > > I'd like to get some Acked-by or Reviewed-by replies for this series. > Or comments if you don't like something. The series looks good to me, so consider it: Reviewed-by: Paul Kocialkowski Also, I'm glad you made the v4l2_m2m_buf_copy_data helper function, since all drivers will need to do these same operations anyway. > I would really like to get this in for 4.20 so the cedrus API is stable > (hopefully), since this is a last outstanding API item. I see a few more items that we need to tackle before we can consider the MPEG-2 stateless API as stable. * I mentionned using flags in the mpeg2 structures to solve the alignment issues, but I failed to provide a patch implementing it at this point. This would make the MPEG-2 controls more similar to the proposed H.264 ones and generally makes more sense than using a u8 for a binary value. * Some time ago, we also discussed adding extra fields to the structures for later use. Do you think that is still relevant? Adding a flags elements (with unused bits) would certainly help in this direction anyway. * During the discussions on the spec, the concensus was also to use one slice_params per-slice (instead of per-picture) which requires splitting the per-picture and the per-slice parameters. On this as well, I fell behind and didn't send a proposal yet. So my question is: does it need to be done for 4.20 or can we affoard going through another version cycle for these changes? It was my impression that we wanted to wait for another driver to use the API to declare it stable (and move the driver out of staging). Cheers, Paul > Tomasz: you commented that the text still referred to the tag as a u64, > but that was only in the cover letter, not the patches themselves. So > I don't plan to post a v3 just for that. > > Regards, > > Hans > > > Changes since v1: > > > > - changed to a u32 tag. Using a 64 bit tag was overly complicated due > > to the bad layout of the v4l2_buffer struct, and there is no real > > need for it by applications. > > > > Main changes since the RFC: > > > > - Added new buffer capability flag > > - Added m2m helper to copy data between buffers > > - Added documentation > > - Added tag logging in v4l2-ioctl.c > > > > Hans Verkuil (9): > > videodev2.h: add tag support > > vb2: add tag support > > v4l2-ioctl.c: log v4l2_buffer tag > > buffer.rst: document the new buffer tag feature. > > v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function > > vb2: add new supports_tags queue flag > > vim2m: add tag support > > vicodec: add tag support > > cedrus: add tag support > > > > Documentation/media/uapi/v4l/buffer.rst | 32 + > > .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++ > > .../media/common/videobuf2/videobuf2-v4l2.c | 45 --- > > drivers/media/platform/vicodec/vicodec-core.c | 14 ++ > > drivers/media/platform/vim
Re: [PATCH v2 3/9] phy: Add MIPI D-PHY configuration options
Hi Maxime,
On Tue, Nov 06, 2018 at 03:54:15PM +0100, Maxime Ripard wrote:
> Now that we have some infrastructure for it, allow the MIPI D-PHY phy's to
> be configured through the generic functions through a custom structure
> added to the generic union.
>
> The parameters added here are the ones defined in the MIPI D-PHY spec, plus
> the number of lanes in use. The current set of parameters should cover all
> the potential users.
>
> Signed-off-by: Maxime Ripard
> ---
> include/linux/phy/phy-mipi-dphy.h | 232 +++-
> include/linux/phy/phy.h | 6 +-
> 2 files changed, 238 insertions(+)
> create mode 100644 include/linux/phy/phy-mipi-dphy.h
>
> diff --git a/include/linux/phy/phy-mipi-dphy.h
> b/include/linux/phy/phy-mipi-dphy.h
> new file mode 100644
> index ..0b05932916af
> --- /dev/null
> +++ b/include/linux/phy/phy-mipi-dphy.h
> @@ -0,0 +1,232 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Cadence Design Systems Inc.
> + */
> +
> +#ifndef __PHY_MIPI_DPHY_H_
> +#define __PHY_MIPI_DPHY_H_
> +
> +#include
> +
> +/**
> + * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
> + *
> + * This structure is used to represent the configuration state of a
> + * MIPI D-PHY phy.
> + */
> +struct phy_configure_opts_mipi_dphy {
> + /**
> + * @clk_miss:
> + *
> + * Timeout, in nanoseconds, for receiver to detect absence of
> + * Clock transitions and disable the Clock Lane HS-RX.
> + */
> + unsigned intclk_miss;
> +
> + /**
> + * @clk_post:
> + *
> + * Time, in nanoseconds, that the transmitter continues to
> + * send HS clock after the last associated Data Lane has
> + * transitioned to LP Mode. Interval is defined as the period
> + * from the end of @hs_trail to the beginning of @clk_trail.
> + */
> + unsigned intclk_post;
> +
> + /**
> + * @clk_pre:
> + *
> + * Time, in nanoseconds, that the HS clock shall be driven by
> + * the transmitter prior to any associated Data Lane beginning
> + * the transition from LP to HS mode.
> + */
> + unsigned intclk_pre;
Is the unit of clk_pre intended to be UI or ns?
How about adding information on the limits of these values as well?
> +
> + /**
> + * @clk_prepare:
> + *
> + * Time, in nanoseconds, that the transmitter drives the Clock
> + * Lane LP-00 Line state immediately before the HS-0 Line
> + * state starting the HS transmission.
> + */
> + unsigned intclk_prepare;
> +
> + /**
> + * @clk_settle:
> + *
> + * Time interval, in nanoseconds, during which the HS receiver
> + * should ignore any Clock Lane HS transitions, starting from
> + * the beginning of @clk_prepare.
> + */
> + unsigned intclk_settle;
> +
> + /**
> + * @clk_term_en:
> + *
> + * Time, in nanoseconds, for the Clock Lane receiver to enable
> + * the HS line termination.
> + */
> + unsigned intclk_term_en;
> +
> + /**
> + * @clk_trail:
> + *
> + * Time, in nanoseconds, that the transmitter drives the HS-0
> + * state after the last payload clock bit of a HS transmission
> + * burst.
> + */
> + unsigned intclk_trail;
> +
> + /**
> + * @clk_zero:
> + *
> + * Time, in nanoseconds, that the transmitter drives the HS-0
> + * state prior to starting the Clock.
> + */
> + unsigned intclk_zero;
> +
> + /**
> + * @d_term_en:
> + *
> + * Time, in nanoseconds, for the Data Lane receiver to enable
> + * the HS line termination.
> + */
> + unsigned intd_term_en;
> +
> + /**
> + * @eot:
> + *
> + * Transmitted time interval, in nanoseconds, from the start
> + * of @hs_trail or @clk_trail, to the start of the LP- 11
> + * state following a HS burst.
> + */
> + unsigned inteot;
> +
> + /**
> + * @hs_exit:
> + *
> + * Time, in nanoseconds, that the transmitter drives LP-11
> + * following a HS burst.
> + */
> + unsigned inths_exit;
> +
> + /**
> + * @hs_prepare:
> + *
> + * Time, in nanoseconds, that the transmitter drives the Data
> + * Lane LP-00 Line state immediately before the HS-0 Line
> + * state starting the HS transmission
> + */
> + unsigned inths_prepare;
> +
> + /**
> + * @hs_settle:
> + *
> + * Time interval, in nanoseconds, during which the HS receiver
> + * shall ignore any Data Lane HS transitions, starting from
> + * the beginning of @hs_prepare.
> + */
> + unsigned inths_settle;
> +
> + /**
> + * @hs_skip:
> + *
> + * Time interval, in nanoseconds, during which the HS-R
Re: [PATCHv2 0/9] vb2/cedrus: add tag support
On 11/19/2018 02:53 PM, Paul Kocialkowski wrote: > Hi, > > On Mon, 2018-11-19 at 12:18 +0100, Hans Verkuil wrote: >> On 11/14/2018 02:47 PM, Hans Verkuil wrote: >>> From: Hans Verkuil >>> >>> As was discussed here (among other places): >>> >>> https://lkml.org/lkml/2018/10/19/440 >>> >>> using capture queue buffer indices to refer to reference frames is >>> not a good idea. A better idea is to use a 'tag' where the >>> application can assign a u64 tag to an output buffer, which is then >>> copied to the capture buffer(s) derived from the output buffer. >>> >>> A u64 is chosen since this allows userspace to also use pointers to >>> internal structures as 'tag'. >>> >>> The first three patches add core tag support, the next patch document >>> the tag support, then a new helper function is added to v4l2-mem2mem.c >>> to easily copy data from a source to a destination buffer that drivers >>> can use. >>> >>> Next a new supports_tags vb2_queue flag is added to indicate that >>> the driver supports tags. Ideally this should not be necessary, but >>> that would require that all m2m drivers are converted to using the >>> new helper function introduced in the previous patch. That takes more >>> time then I have now since we want to get this in for 4.20. >>> >>> Finally the vim2m, vicodec and cedrus drivers are converted to support >>> tags. >>> >>> I also removed the 'pad' fields from the mpeg2 control structs (it >>> should never been added in the first place) and aligned the structs >>> to a u32 boundary (u64 for the tag values). >>> >>> Note that this might change further (Paul suggested using bitfields). >>> >>> Also note that the cedrus code doesn't set the sequence counter, that's >>> something that should still be added before this driver can be moved >>> out of staging. >>> >>> Note: if no buffer is found for a certain tag, then the dma address >>> is just set to 0. That happened before as well with invalid buffer >>> indices. This should be checked in the driver! >>> >>> The previous RFC series was tested successfully with the cedrus driver. >>> >>> Regards, >>> >>> Hans >> >> I'd like to get some Acked-by or Reviewed-by replies for this series. >> Or comments if you don't like something. > > The series looks good to me, so consider it: > Reviewed-by: Paul Kocialkowski > > Also, I'm glad you made the v4l2_m2m_buf_copy_data helper function, > since all drivers will need to do these same operations anyway. > >> I would really like to get this in for 4.20 so the cedrus API is stable >> (hopefully), since this is a last outstanding API item. > > I see a few more items that we need to tackle before we can consider the > MPEG-2 stateless API as stable. > > * I mentionned using flags in the mpeg2 structures to solve the > alignment issues, but I failed to provide a patch implementing it at > this point. This would make the MPEG-2 controls more similar to the > proposed H.264 ones and generally makes more sense than using a u8 for a > binary value. I think it will be good to have this done. If you can make a patch that I can rebase my tag series on, then that would be best. > > * Some time ago, we also discussed adding extra fields to the structures > for later use. Do you think that is still relevant? Adding a flags > elements (with unused bits) would certainly help in this direction > anyway. If you switch to bitfields, then there might be enough room anyway for addition flags in the future. > > * During the discussions on the spec, the concensus was also to use one > slice_params per-slice (instead of per-picture) which requires splitting > the per-picture and the per-slice parameters. On this as well, I fell > behind and didn't send a proposal yet. > > So my question is: does it need to be done for 4.20 or can we affoard > going through another version cycle for these changes? > > It was my impression that we wanted to wait for another driver to use > the API to declare it stable (and move the driver out of staging). Good question, I would like to hear what Mauro says. I would prefer to postpone this to 4.21, since I don't feel comfortable with such large changes for an rcX (esp. splitting the per-picture/slice parameters). Regards, Hans > > Cheers, > > Paul > >> Tomasz: you commented that the text still referred to the tag as a u64, >> but that was only in the cover letter, not the patches themselves. So >> I don't plan to post a v3 just for that. >> >> Regards, >> >> Hans >> >>> Changes since v1: >>> >>> - changed to a u32 tag. Using a 64 bit tag was overly complicated due >>> to the bad layout of the v4l2_buffer struct, and there is no real >>> need for it by applications. >>> >>> Main changes since the RFC: >>> >>> - Added new buffer capability flag >>> - Added m2m helper to copy data between buffers >>> - Added documentation >>> - Added tag logging in v4l2-ioctl.c >>> >>> Hans Verkuil (9): >>> videodev2.h: add tag support >>> vb2: add tag support >>>
Re: [PATCH v2 0/2] media: cedrus: Add H264 decoding support
Hi Tomasz,
On Fri, Nov 16, 2018 at 04:04:40PM +0900, Tomasz Figa wrote:
> > I've been using the controls currently integrated into ChromeOS that
> > have a working version of this particular setup. However, these
> > controls have a number of shortcomings and inconsistencies with other
> > decoding API. I've worked with libva so far, but I've noticed already
> > that:
> > - The kernel UAPI expects to have the nal_ref_idc variable, while
> > libva only exposes whether that frame is a reference frame or
> > not. I've looked at the rockchip driver in the ChromeOS tree, and
> > our own driver, and they both need only the information about
> > whether the frame is a reference one or not, so maybe we should
> > change this?
>
> Since this is something that is actually present in the stream and the
> problem is that libva doesn't convey the information properly, I
> believe you can workaround it in the libva backend using this API by
> just setting it to 0 and some arbitrary non-zero value in a binary
> fashion.
That could work yes, thanks for the suggestion!
> > - The H264 bitstream exposes the picture default reference list (for
> > both list 0 and list 1), the slice reference list and an override
> > flag. The libva will only pass the reference list to be used (so
> > either the picture default's or the slice's) depending on the
> > override flag. The kernel UAPI wants the picture default reference
> > list and the slice reference list, but doesn't expose the override
> > flag, which prevents us from configuring properly the
> > hardware. Our video decoding engine needs the three information,
> > but we can easily adapt to having only one. However, having two
> > doesn't really work for us.
> >
>
> From what I can see in the H.264 Slice header, there are 3 related data:
> - num_ref_idx_active_override_flag - affects the number of reference
> indices for the slice,
> - ref_list_l{0,1}_modifications - modifications for the reference lists,
> - ref_pic_list_modification_flag_l{0,1} - selects whether the
> modifications are applied.
>
> The reference lists inside the v4l2_ctrl_h264_slice_param are expected
> to already take all the above into account and be the final reference
> lists to be used for the slice. For reference, the H.264 specification
> refers to those final reference lists as RefPicList0 and RefPicList1
> and so the names of the fields in the struct.
>
> There is some interesting background here, though. The Rockchip VPU
> parses the slice headers itself and handles the above data on its own.
> This means that it needs to be programmed with the unmodified
> reference lists, as in v4l2_ctrl_h264_decode_param.
>
> Given that, it sounds like we need to have both. Your driver would
> always use the lists in v4l2_ctrl_h264_slice_param, while the Rockchip
> VPU would ignore them, use the ones in v4l2_ctrl_h264_decode_param and
> perform the per-slice modifications on its own.
I guess that would work, yep
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v4 6/6] media: video-i2c: support runtime PM
On 10/20/2018 04:26 PM, Akinobu Mita wrote:
> AMG88xx has a register for setting operating mode. This adds support
> runtime PM by changing the operating mode.
>
> The instruction for changing sleep mode to normal mode is from the
> reference specifications.
>
> https://docid81hrs3j1.cloudfront.net/medialibrary/2017/11/PANA-S-A0002141979-1.pdf
>
> Cc: Matt Ranostay
> Cc: Sakari Ailus
> Cc: Hans Verkuil
> Cc: Mauro Carvalho Chehab
> Reviewed-by: Matt Ranostay
> Acked-by: Sakari Ailus
> Signed-off-by: Akinobu Mita
> ---
> * v4
> - Move set_power() call into release() callback
>
> drivers/media/i2c/video-i2c.c | 141
> +-
> 1 file changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> index 3334fc2..384a046 100644
> --- a/drivers/media/i2c/video-i2c.c
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -94,10 +95,23 @@ struct video_i2c_chip {
> /* xfer function */
> int (*xfer)(struct video_i2c_data *data, char *buf);
>
> + /* power control function */
> + int (*set_power)(struct video_i2c_data *data, bool on);
> +
> /* hwmon init function */
> int (*hwmon_init)(struct video_i2c_data *data);
> };
>
> +/* Power control register */
> +#define AMG88XX_REG_PCTL 0x00
> +#define AMG88XX_PCTL_NORMAL 0x00
> +#define AMG88XX_PCTL_SLEEP 0x10
> +
> +/* Reset register */
> +#define AMG88XX_REG_RST 0x01
> +#define AMG88XX_RST_FLAG 0x30
> +#define AMG88XX_RST_INIT 0x3f
> +
> /* Frame rate register */
> #define AMG88XX_REG_FPSC 0x02
> #define AMG88XX_FPSC_1FPSBIT(0)
> @@ -127,6 +141,59 @@ static int amg88xx_setup(struct video_i2c_data *data)
> return regmap_update_bits(data->regmap, AMG88XX_REG_FPSC, mask, val);
> }
>
> +static int amg88xx_set_power_on(struct video_i2c_data *data)
> +{
> + int ret;
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL, AMG88XX_PCTL_NORMAL);
> + if (ret)
> + return ret;
> +
> + msleep(50);
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_INIT);
> + if (ret)
> + return ret;
> +
> + msleep(2);
WARNING: msleep < 20ms can sleep for up to 20ms; see
Documentation/timers/timers-howto.txt
#55: FILE: drivers/media/i2c/video-i2c.c:158:
+ msleep(2);
Use usleep_range instead.
Regards,
Hans
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_FLAG);
> + if (ret)
> + return ret;
> +
> + /*
> + * Wait two frames before reading thermistor and temperature registers
> + */
> + msleep(200);
> +
> + return 0;
> +}
> +
> +static int amg88xx_set_power_off(struct video_i2c_data *data)
> +{
> + int ret;
> +
> + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL, AMG88XX_PCTL_SLEEP);
> + if (ret)
> + return ret;
> + /*
> + * Wait for a while to avoid resuming normal mode immediately after
> + * entering sleep mode, otherwise the device occasionally goes wrong
> + * (thermistor and temperature registers are not updated at all)
> + */
> + msleep(100);
> +
> + return 0;
> +}
> +
> +static int amg88xx_set_power(struct video_i2c_data *data, bool on)
> +{
> + if (on)
> + return amg88xx_set_power_on(data);
> +
> + return amg88xx_set_power_off(data);
> +}
> +
> #if IS_ENABLED(CONFIG_HWMON)
>
> static const u32 amg88xx_temp_config[] = {
> @@ -158,7 +225,15 @@ static int amg88xx_read(struct device *dev, enum
> hwmon_sensor_types type,
> __le16 buf;
> int tmp;
>
> + tmp = pm_runtime_get_sync(regmap_get_device(data->regmap));
> + if (tmp < 0) {
> + pm_runtime_put_noidle(regmap_get_device(data->regmap));
> + return tmp;
> + }
> +
> tmp = regmap_bulk_read(data->regmap, AMG88XX_REG_TTHL, &buf, 2);
> + pm_runtime_mark_last_busy(regmap_get_device(data->regmap));
> + pm_runtime_put_autosuspend(regmap_get_device(data->regmap));
> if (tmp)
> return tmp;
>
> @@ -217,6 +292,7 @@ static const struct video_i2c_chip video_i2c_chip[] = {
> .regmap_config = &amg88xx_regmap_config,
> .setup = &amg88xx_setup,
> .xfer = &amg88xx_xfer,
> + .set_power = amg88xx_set_power,
> .hwmon_init = amg88xx_hwmon_init,
> },
> };
> @@ -343,14 +419,21 @@ static void video_i2c_del_list(struct vb2_queue *vq,
> enum vb2_buffer_state state
> static int start_streaming(struct vb2_queue *vq, unsigned int count)
> {
> struct video_i2c_data *data = vb2_get_drv_priv(vq);
> + struct device *dev = regmap_get_device(data->regmap);
> int ret;
>
> if (data->kthread_vid_cap)
>
Re: [PATCH 2/2] media: video-i2c: add Melexis MLX90640 thermal camera support
On 11/01/2018 05:15 AM, Matt Ranostay wrote:
> Add initial support for MLX90640 thermal cameras which output an 32x24
> greyscale pixel image along with 2 rows of coefficent data.
>
> Because of this the data outputed is really 32x26 and needs the two rows
> removed after using the coefficent information to generate processed
> images in userspace.
>
> Signed-off-by: Matt Ranostay
> ---
> drivers/media/i2c/Kconfig | 1 +
> drivers/media/i2c/video-i2c.c | 110 +-
> 2 files changed, 110 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 704af210e270..4bfb2c66d192 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1085,6 +1085,7 @@ config VIDEO_I2C
> Enable the I2C transport video support which supports the
> following:
> * Panasonic AMG88xx Grid-Eye Sensors
> +* Melexis MLX90640 Thermal Cameras
>
> To compile this driver as a module, choose M here: the
> module will be called video-i2c
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> index 6d3b6df0b634..38ade8cb7656 100644
> --- a/drivers/media/i2c/video-i2c.c
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -6,6 +6,7 @@
> *
> * Supported:
> * - Panasonic AMG88xx Grid-Eye Sensors
> + * - Melexis MLX90640 Thermal Cameras
> */
>
> #include
> @@ -18,6 +19,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -66,12 +68,26 @@ static const struct v4l2_frmsize_discrete amg88xx_size = {
> .height = 8,
> };
>
> +static const struct v4l2_fmtdesc mlx90640_format = {
> + .pixelformat = V4L2_PIX_FMT_Y16_BE,
> +};
> +
> +static const struct v4l2_frmsize_discrete mlx90640_size = {
> + .width = 32,
> + .height = 26, /* 24 lines of pixel data + 2 lines of processing data */
> +};
> +
> static const struct regmap_config amg88xx_regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> .max_register = 0xff
> };
>
> +static const struct regmap_config mlx90640_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 16,
> +};
> +
> struct video_i2c_chip {
> /* video dimensions */
> const struct v4l2_fmtdesc *format;
> @@ -88,6 +104,7 @@ struct video_i2c_chip {
> unsigned int bpp;
>
> const struct regmap_config *regmap_config;
> + struct nvmem_config *nvmem_config;
>
> /* setup function */
> int (*setup)(struct video_i2c_data *data);
> @@ -102,6 +119,22 @@ struct video_i2c_chip {
> int (*hwmon_init)(struct video_i2c_data *data);
> };
>
> +static int mlx90640_nvram_read(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> +{
> + struct video_i2c_data *data = priv;
> +
> + return regmap_bulk_read(data->regmap, 0x2400 + offset, val, bytes);
> +}
> +
> +static struct nvmem_config mlx90640_nvram_config = {
> + .name = "mlx90640_nvram",
> + .word_size = 2,
> + .stride = 1,
> + .size = 1664,
> + .reg_read = mlx90640_nvram_read,
> +};
> +
> /* Power control register */
> #define AMG88XX_REG_PCTL 0x00
> #define AMG88XX_PCTL_NORMAL 0x00
> @@ -122,12 +155,23 @@ struct video_i2c_chip {
> /* Temperature register */
> #define AMG88XX_REG_T01L 0x80
>
> +/* Control register */
> +#define MLX90640_REG_CTL10x800d
> +#define MLX90640_REG_CTL1_MASK 0x0380
> +#define MLX90640_REG_CTL1_MASK_SHIFT 7
> +
> static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
> {
> return regmap_bulk_read(data->regmap, AMG88XX_REG_T01L, buf,
> data->chip->buffer_size);
> }
>
> +static int mlx90640_xfer(struct video_i2c_data *data, char *buf)
> +{
> + return regmap_bulk_read(data->regmap, 0x400, buf,
> + data->chip->buffer_size);
> +}
> +
> static int amg88xx_setup(struct video_i2c_data *data)
> {
> unsigned int mask = AMG88XX_FPSC_1FPS;
> @@ -141,6 +185,27 @@ static int amg88xx_setup(struct video_i2c_data *data)
> return regmap_update_bits(data->regmap, AMG88XX_REG_FPSC, mask, val);
> }
>
> +static int mlx90640_setup(struct video_i2c_data *data)
> +{
> + unsigned int n, idx;
> +
> + for (n = 0; n < data->chip->num_frame_intervals - 1; n++) {
> + if (data->frame_interval.numerator
> + != data->chip->frame_intervals[n].numerator)
> + continue;
> +
> + if (data->frame_interval.denominator
> + == data->chip->frame_intervals[n].denominator)
> + break;
> + }
> +
> + idx = data->chip->num_frame_intervals - n - 1;
> +
> + return regmap_update_bits(data->regmap, MLX90640_REG_CTL1,
> + MLX90640_REG_CTL1_MASK,
> + idx << MLX90640_REG_CTL1_MASK_SHIFT);
> +}
> +
> st
[GIT PULL FOR v4.21] Various fixes/improvements
The following changes since commit fbe57dde7126d1b2712ab5ea93fb9d15f89de708: media: ov7740: constify structures stored in fields of v4l2_subdev_ops structure (2018-11-06 07:17:02 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git tags/br-v4.21d for you to fetch changes up to b8e1d6a3dd646b57a7821a4b51796edee64787f4: media: vicodec: Add support for 4 planes formats (2018-11-19 15:47:22 +0100) Tag branch Akinobu Mita (5): media: video-i2c: avoid accessing released memory area when removing driver media: video-i2c: use i2c regmap media: v4l2-common: add V4L2_FRACT_COMPARE media: vivid: use V4L2_FRACT_COMPARE media: video-i2c: support changing frame interval Alexey Khoroshilov (1): media: mtk-vcodec: Release device nodes in mtk_vcodec_init_enc_pm() Dafna Hirschfeld (3): media: vicodec: prepare support for various number of planes media: vicodec: Add support of greyscale format media: vicodec: Add support for 4 planes formats Eric Biggers (1): media: v4l: constify v4l2_ioctls[] Hans Verkuil (2): vim2m/vicodec: set device_caps in video_device struct vidioc-enum-fmt.rst: update list of valid buftypes Julia Lawall (1): media: video-i2c: hwmon: constify vb2_ops structure Malathi Gottam (1): media: venus: change the default value of GOP size Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 8 ++- drivers/media/i2c/video-i2c.c | 153 +++-- drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 10 ++-- drivers/media/platform/qcom/venus/venc_ctrls.c| 2 +- drivers/media/platform/vicodec/codec-fwht.c | 84 +-- drivers/media/platform/vicodec/codec-fwht.h | 15 +++-- drivers/media/platform/vicodec/codec-v4l2-fwht.c | 123 +-- drivers/media/platform/vicodec/codec-v4l2-fwht.h | 3 +- drivers/media/platform/vicodec/vicodec-core.c | 43 ++ drivers/media/platform/vim2m.c| 3 +- drivers/media/platform/vivid/vivid-vid-cap.c | 9 +-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- include/media/v4l2-common.h | 5 ++ 13 files changed, 326 insertions(+), 134 deletions(-)
Re: [PATCH v4 6/6] media: video-i2c: support runtime PM
On 11/19/2018 03:26 PM, Hans Verkuil wrote: > On 10/20/2018 04:26 PM, Akinobu Mita wrote: >> AMG88xx has a register for setting operating mode. This adds support >> runtime PM by changing the operating mode. >> >> The instruction for changing sleep mode to normal mode is from the >> reference specifications. >> >> https://docid81hrs3j1.cloudfront.net/medialibrary/2017/11/PANA-S-A0002141979-1.pdf >> >> Cc: Matt Ranostay >> Cc: Sakari Ailus >> Cc: Hans Verkuil >> Cc: Mauro Carvalho Chehab >> Reviewed-by: Matt Ranostay >> Acked-by: Sakari Ailus >> Signed-off-by: Akinobu Mita For the record: I've accepted patches 1-5, so no need to repost the whole series, just this patch needs an update. Regards, Hans
Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate
On Wed, Nov 14, 2018 at 7:20 AM Adam Ford wrote:
>
> On Tue, Nov 13, 2018 at 7:04 AM 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
>
> Thanks for the patches! I tested the whole series. I am stil learning
> the v4l2 stuff, but I'm trying to test what and where I can.
> media-ctl shows the camera is talking at 60fps, but my imx6 is only
> capturing at 30, but I don't think it's the fault of the ov5640
> driver.
>
> Tested-by: Adam Ford #imx6dq
>
For what it's worth, I would I like to request that this series be
applied to 4.20 fixes (possibly 4.19 if appropriate) as it fixes some
issues where I am trying to capture JPEG images that previously came
up garbled, and now they are clear. The only change to my kernel is
this series.
adam
> adam
> > ---
> > drivers/media/i2c/ov5640.c | 366 -
> > 1 file changed, 365 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index eaefdb58653b..8476f85bb8e7 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,
> > +2800,
> > ov5640_setting_15fps_QCIF_176_144,
> > ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> > {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
> > 320, 1896, 240, 984,
> > +2800,
> > ov5640_setting_15fps_QVGA_320_240,
> > ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> > {OV5640_MODE_VGA_640_480, SUBSAMPLING,
> > 640, 1896, 480, 1080,
> > +2800,
> > ov5640_setting_15fps_VGA_640_480,
> > ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> > {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
> > 720, 1896, 480, 984,
> > +2800,
> > ov5640_setting_15fps_NTSC_720_480,
> > ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> > {OV5640_MODE_PAL_720_576, SUBSAMPLING,
> > 720, 1896, 576, 984,
> > +2800,
> > ov5640_setting_15fps_PAL_720_576,
> > ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> > {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
> > 1024, 1896, 768, 1080,
> > +2800,
> > ov5640_setting_15fps_XGA_1024_768,
> > ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> > {OV5640_MODE_720P_1280_720, SUBSAMPLING,
> > 1280, 1892, 720, 740,
> > +2100,
> > ov5640_setting_15fps_720P_1280_720,
> > ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> > {OV5640_MODE_1080P_1920_1080, SCALING,
> > 1920, 2500, 1080, 1120,
> > +4200,
> > ov5640_setting_15fps_1080P_1920_1080,
> > ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> > {OV5640_MODE_QSXGA_2592_1944, SCALING,
> > 2592, 2844, 1944, 1968,
> > +8400,
> > ov5640_setting_15fps_QSXGA
Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
Hi Mike, On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox wrote: > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote: > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport wrote: > > > > + * vm_insert_range - insert range of kernel pages into user vma > > > > + * @vma: user vma to map to > > > > + * @addr: target user address of this page > > > > + * @pages: pointer to array of source kernel pages > > > > + * @page_count: no. of pages need to insert into user vma > > > > + * > > > > + * This allows drivers to insert range of kernel pages they've > > > > allocated > > > > + * into a user vma. This is a generic function which drivers can use > > > > + * rather than using their own way of mapping range of kernel pages > > > > into > > > > + * user vma. > > > > > > Please add the return value and context descriptions. > > > > > > > Sure I will wait for some time to get additional review comments and > > add all of those requested changes in v2. > > You could send your proposed wording now which might remove the need > for a v3 if we end up arguing about the wording. Does this description looks good ? /** * vm_insert_range - insert range of kernel pages into user vma * @vma: user vma to map to * @addr: target user address of this page * @pages: pointer to array of source kernel pages * @page_count: number of pages need to insert into user vma * * This allows drivers to insert range of kernel pages they've allocated * into a user vma. This is a generic function which drivers can use * rather than using their own way of mapping range of kernel pages into * user vma. * * Context - Process context. Called by mmap handlers. * Return - int error value * 0- OK * -EINVAL - Invalid argument * -ENOMEM - No memory * -EFAULT - Bad address * -EBUSY - Device or resource busy */
Re: [PATCH v2 for v4.4 1/1] v4l: event: Add subscription to list before calling "add" operation
On Wed, Nov 14, 2018 at 11:37:46AM +0200, Sakari Ailus wrote: > [ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ] There is no such git commit id in Linus's tree :(
DVB-S PCI card regression on 4.19 / 4.20
Hi, we've got a regression report on openSUSE Bugzilla regarding DVB-S PCI card: https://bugzilla.opensuse.org/show_bug.cgi?id=1116374 According to the reporter (Stakanov, Cc'ed), the card worked fine on 4.18.15, but since 4.19, it doesn't give any channels, sound nor picture, but only EPG is received. The following errors might be relevant: [4.845180] b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded successfully [4.869703] b2c2-flexcop: MAC address = xx:xx:xx:xx:xx:xx [5.100318] b2c2-flexcop: found 'ST STV0299 DVB-S' . [5.100323] b2c2_flexcop_pci :06:06.0: DVB: registering adapter 0 frontend 0 (ST STV0299 DVB-S)... [5.100370] b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S rev 2.6' at the 'PCI' bus controlled by a 'FlexCopIIb' complete [ 117.513086] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 frequency 1549000 out of range (95..2150) [ 124.905222] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 frequency 188 out of range (95..2150) [ 127.337079] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 frequency 1353500 out of range (95..2150) The lspci shows: 06:06.0 Network controller: Techsan Electronics Co Ltd B2C2 FlexCopII DVB chip / Technisat SkyStar2 DVB card (rev 02) Subsystem: Techsan Electronics Co Ltd B2C2 FlexCopII DVB chip / Technisat SkyStar2 DVB card Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- SERR-
[PATCH] v4l2-compliance: Remove spurious error messages
Get rid of a couple confusing error messages, namely:
test VIDIOC_G_FMT: OK
fail: v4l2-test-formats.cpp(464): pix_mp.plane_fmt[0].reserved
not zeroed
fail: v4l2-test-formats.cpp(752): Video Output Multiplanar is
valid, but TRY_FMT failed to return a format
test VIDIOC_TRY_FMT: FAIL
fail: v4l2-test-formats.cpp(464): pix_mp.plane_fmt[0].reserved
not zeroed
fail: v4l2-test-formats.cpp(1017): Video Output Multiplanar is
valid, but no S_FMT was implemented
test VIDIOC_S_FMT: FAI
Where only the first message "pix_mp.plane_fmt[0].reserved not zeroed"
is accurate.
Signed-off-by: Ezequiel Garcia
---
utils/v4l2-compliance/v4l2-test-formats.cpp | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp
b/utils/v4l2-compliance/v4l2-test-formats.cpp
index 2fb811ad5eb4..006cc3222c65 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -748,8 +748,7 @@ int testTryFormats(struct node *node)
}
ret = testFormatsType(node, ret, type, fmt, true);
if (ret)
- return fail("%s is valid, but TRY_FMT failed to return
a format\n",
- buftype2s(type).c_str());
+ return ret;
}
memset(&fmt, 0, sizeof(fmt));
@@ -1013,8 +1012,7 @@ int testSetFormats(struct node *node)
}
ret = testFormatsType(node, ret, type, fmt_set, true);
if (ret)
- return fail("%s is valid, but no S_FMT was
implemented\n",
- buftype2s(type).c_str());
+ return ret;
fmt_set = fmt;
ret = doioctl(node, VIDIOC_S_FMT, &fmt_set);
--
2.19.1
[PATCH v8 1/3] ARM: dts: rockchip: add VPU device node for RK3288
Add the Video Processing Unit node for RK3288 SoC.
Fix the VPU IOMMU node, which was disabled and lacking
its power domain property.
Signed-off-by: Ezequiel Garcia
---
arch/arm/boot/dts/rk3288.dtsi | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 0840ffb3205c..40d203cdca09 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1223,6 +1223,18 @@
};
};
+ vpu: video-codec@ff9a {
+ compatible = "rockchip,rk3288-vpu";
+ reg = <0x0 0xff9a 0x0 0x800>;
+ interrupts = ,
+;
+ interrupt-names = "vepu", "vdpu";
+ clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
+ clock-names = "aclk", "hclk";
+ power-domains = <&power RK3288_PD_VIDEO>;
+ iommus = <&vpu_mmu>;
+ };
+
vpu_mmu: iommu@ff9a0800 {
compatible = "rockchip,iommu";
reg = <0x0 0xff9a0800 0x0 0x100>;
@@ -1230,8 +1242,8 @@
interrupt-names = "vpu_mmu";
clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
clock-names = "aclk", "iface";
+ power-domains = <&power RK3288_PD_VIDEO>;
#iommu-cells = <0>;
- status = "disabled";
};
hevc_mmu: iommu@ff9c0440 {
--
2.19.1
[PATCH v8 3/3] media: add Rockchip VPU JPEG encoder driver
Add a mem2mem driver for the VPU available on Rockchip SoCs. Currently only JPEG encoding is supported, for RK3399 and RK3288 platforms. Signed-off-by: Ezequiel Garcia --- MAINTAINERS | 7 + drivers/staging/media/Kconfig | 2 + drivers/staging/media/Makefile| 1 + drivers/staging/media/rockchip/vpu/Kconfig| 14 + drivers/staging/media/rockchip/vpu/Makefile | 10 + drivers/staging/media/rockchip/vpu/TODO | 9 + .../media/rockchip/vpu/rk3288_vpu_hw.c| 118 +++ .../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c | 133 .../media/rockchip/vpu/rk3288_vpu_regs.h | 442 +++ .../media/rockchip/vpu/rk3399_vpu_hw.c| 118 +++ .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c | 160 .../media/rockchip/vpu/rk3399_vpu_regs.h | 600 +++ .../staging/media/rockchip/vpu/rockchip_vpu.h | 237 ++ .../media/rockchip/vpu/rockchip_vpu_common.h | 29 + .../media/rockchip/vpu/rockchip_vpu_drv.c | 535 + .../media/rockchip/vpu/rockchip_vpu_enc.c | 701 ++ .../media/rockchip/vpu/rockchip_vpu_hw.h | 58 ++ .../media/rockchip/vpu/rockchip_vpu_jpeg.c| 289 .../media/rockchip/vpu/rockchip_vpu_jpeg.h| 12 + 19 files changed, 3475 insertions(+) create mode 100644 drivers/staging/media/rockchip/vpu/Kconfig create mode 100644 drivers/staging/media/rockchip/vpu/Makefile create mode 100644 drivers/staging/media/rockchip/vpu/TODO create mode 100644 drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c create mode 100644 drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c create mode 100644 drivers/staging/media/rockchip/vpu/rk3288_vpu_regs.h create mode 100644 drivers/staging/media/rockchip/vpu/rk3399_vpu_hw.c create mode 100644 drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c create mode 100644 drivers/staging/media/rockchip/vpu/rk3399_vpu_regs.h create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu.h create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_hw.h create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_jpeg.c create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_jpeg.h diff --git a/MAINTAINERS b/MAINTAINERS index a8588dedc683..e5a294453393 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12742,6 +12742,13 @@ S: Maintained F: drivers/media/platform/rockchip/rga/ F: Documentation/devicetree/bindings/media/rockchip-rga.txt +ROCKCHIP VPU CODEC DRIVER +M: Ezequiel Garcia +L: linux-media@vger.kernel.org +S: Maintained +F: drivers/staging/media/platform/rockchip/vpu/ +F: Documentation/devicetree/bindings/media/rockchip-vpu.txt + ROCKER DRIVER M: Jiri Pirko L: net...@vger.kernel.org diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig index b3620a8f2d9f..c6f3404dea43 100644 --- a/drivers/staging/media/Kconfig +++ b/drivers/staging/media/Kconfig @@ -31,6 +31,8 @@ source "drivers/staging/media/mt9t031/Kconfig" source "drivers/staging/media/omap4iss/Kconfig" +source "drivers/staging/media/rockchip/vpu/Kconfig" + source "drivers/staging/media/sunxi/Kconfig" source "drivers/staging/media/tegra-vde/Kconfig" diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile index 42948f805548..43c7bee1fc8c 100644 --- a/drivers/staging/media/Makefile +++ b/drivers/staging/media/Makefile @@ -8,3 +8,4 @@ obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ obj-$(CONFIG_VIDEO_SUNXI) += sunxi/ obj-$(CONFIG_TEGRA_VDE)+= tegra-vde/ obj-$(CONFIG_VIDEO_ZORAN) += zoran/ +obj-$(CONFIG_VIDEO_ROCKCHIP_VPU) += rockchip/vpu/ diff --git a/drivers/staging/media/rockchip/vpu/Kconfig b/drivers/staging/media/rockchip/vpu/Kconfig new file mode 100644 index ..fa65c03d65bf --- /dev/null +++ b/drivers/staging/media/rockchip/vpu/Kconfig @@ -0,0 +1,14 @@ +config VIDEO_ROCKCHIP_VPU + tristate "Rockchip VPU driver" + depends on ARCH_ROCKCHIP || COMPILE_TEST + depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER + select VIDEOBUF2_DMA_CONTIG + select VIDEOBUF2_VMALLOC + select V4L2_MEM2MEM_DEV + default n + help + Support for the Video Processing Unit present on Rockchip SoC, + which accelerates video and image encoding and decoding. + To compile this driver as a module, choose M here: the module + will be called rockchip-vpu. + diff --git a/drivers/staging/media/rockchip/vpu/Makefile b/drivers/staging/media/rockchip/vpu/Makefile new file mode 100644 index ..e9d733bb7632 --- /dev/null +++ b/drivers/staging/media/rockchip/vpu/Makefile @@ -0,0 +1,10 @@ +obj-$(CONFI
[PATCH v8 2/3] arm64: dts: rockchip: add VPU device node for RK3399
Add the Video Processing Unit node for the RK3399 SoC.
Also, fix the VPU IOMMU node, which was disabled and lacking
its power domain property.
Signed-off-by: Ezequiel Garcia
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 99e7f65c1779..040d3080565f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1226,6 +1226,18 @@
status = "disabled";
};
+ vpu: video-codec@ff65 {
+ compatible = "rockchip,rk3399-vpu";
+ reg = <0x0 0xff65 0x0 0x800>;
+ interrupts = ,
+;
+ interrupt-names = "vepu", "vdpu";
+ clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
+ clock-names = "aclk", "hclk";
+ power-domains = <&power RK3399_PD_VCODEC>;
+ iommus = <&vpu_mmu>;
+ };
+
vpu_mmu: iommu@ff650800 {
compatible = "rockchip,iommu";
reg = <0x0 0xff650800 0x0 0x40>;
@@ -1233,8 +1245,8 @@
interrupt-names = "vpu_mmu";
clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
clock-names = "aclk", "iface";
+ power-domains = <&power RK3399_PD_VCODEC>;
#iommu-cells = <0>;
- status = "disabled";
};
vdec_mmu: iommu@ff660480 {
--
2.19.1
[PATCH v8 0/3] Add Rockchip VPU JPEG encoder
Here's a new version of the Rockchip VPU JPEG encoder driver, for the RK3288 and RK3399 SoCs. This new version supports V4L2_PIX_FMT_JPEG capture format, which means standard userspace can be used, such as gstreamer: gst-launch-1.0 videotestsrc ! v4l2jpegenc extra-controls="c,compression_quality=95" ! ... The hardware produces a JPEG scan (i.e. without the start JPEG headers), so the driver has to add the needed headers. Note that the hardware produces a EOI marker at the end of the JPEG scan. Since the driver no longer needs a specific format, I've dropped the patch adding the V4L2_PIX_FMT_JPEG_RAW format. Also, since the driver adds the needed quantization tables, it's possible to drop the new controls and use V4L2_CID_JPEG_COMPRESSION_QUALITY. This is required for standard userspace to work, as the above example shows. Future work --- There's an opportunity for some factorization as default JPEG tables are specified by the JPEG standard and they are also currently defined by the CODA driver. Another factorization is the fill_fmt_mp function, which should be converted to a proper helper to be used by other drivers. I'd like to pospone such factorization until after this driver is merged, to avoid delaying this work any further. In addition to this, it would be interesting to add user controls to specify the quantization tables. It's not yet clear how this control would interact with the JPEG compression quality control, and so it needs some discussion. Compliance output - root@ficus:~# v4l2-compliance -d /dev/video3 -s v4l2-compliance SHA: d0f4ea7ddab6d0244c4fe1e960bb2aaeefb911b9, 64 bits Compliance test for device /dev/video3: Driver Info: Driver name : rockchip-vpu Card type: rockchip,rk3399-vpu-enc Bus info : platform: rockchip-vpu Driver version : 4.20.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Media Driver Info: Driver name : rockchip-vpu Model: rockchip-vpu Serial : Bus info : Media version: 4.20.0 Hardware revision: 0x (0) Driver version : 4.20.0 Interface Info: ID : 0x030c Type : V4L Video Entity Info: ID : 0x0001 (1) Name : rockchip,rk3399-vpu-enc-source Function : V4L2 I/O Pad 0x0102 : 0: Source Link 0x0208: to remote pad 0x105 of entity 'rockchip,rk3399-vpu-enc-proc': Data, Enabled, Immutable Required ioctls: test MC information (see 'Media Driver Info' above): OK test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video3 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 2 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing:
[PATCH] videobuf2-v4l2: drop WARN_ON in vb2_warn_zero_bytesused()
Userspace shouldn't set bytesused to 0 for output buffers.
vb2_warn_zero_bytesused() warns about this (only once!), but it also
calls WARN_ON(1), which is confusing since it is not immediately clear
that it warns about a 0 value for bytesused.
Just drop the WARN_ON as it serves no purpose.
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index a17033ab2c22..713326ef4e72 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -158,7 +158,6 @@ static void vb2_warn_zero_bytesused(struct vb2_buffer *vb)
return;
check_once = true;
- WARN_ON(1);
pr_warn("use of bytesused == 0 is deprecated and will be removed in the
future,\n");
if (vb->vb2_queue->allow_zero_bytesused)
Re: [PATCH] videobuf2-v4l2: drop WARN_ON in vb2_warn_zero_bytesused()
On Mon, 2018-11-19 at 16:33 +0100, Hans Verkuil wrote:
> Userspace shouldn't set bytesused to 0 for output buffers.
> vb2_warn_zero_bytesused() warns about this (only once!), but it also
> calls WARN_ON(1), which is confusing since it is not immediately clear
> that it warns about a 0 value for bytesused.
>
> Just drop the WARN_ON as it serves no purpose.
>
> Signed-off-by: Hans Verkuil
Acked-by: Ezequiel Garcia
> ---
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index a17033ab2c22..713326ef4e72 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -158,7 +158,6 @@ static void vb2_warn_zero_bytesused(struct vb2_buffer *vb)
> return;
>
> check_once = true;
> - WARN_ON(1);
>
> pr_warn("use of bytesused == 0 is deprecated and will be removed in the
> future,\n");
> if (vb->vb2_queue->allow_zero_bytesused)
Re: [ANN] Edinburgh Media Summit 2018 meeting report
Hello everyone, And thanks for the detailed report! On Mon, 2018-11-19 at 13:09 +0900, Tomasz Figa wrote: > On Sun, Nov 18, 2018 at 7:45 AM Sakari Ailus wrote: > > Hello everyone, > > > > > > Here's the report on the Media Summit held on 25th October in Edinburgh. > > The report is followed by the stateless codec discussion two days earlier. > > > > Note: this is bcc'd to the meeting attendees plus a few others. I didn't > > use cc as the list servers tend to reject messages with too many > > recipients in cc / to headers. > > > > Most presenters used slides some of which are already available here > > (expect more in the near future): > > > > https://www.linuxtv.org/downloads/presentations/media_summit_2018/> > > > > The original announcement for the meeting is here: > > > > https://www.spinics.net/lists/linux-media/msg141095.html> > > > > The raw notes can be found here: > > > > http://www.retiisi.org.uk/~sailus/v4l2/notes/osseu18-media.html> > > Thanks Sakari for editing the notes. Let me share my thoughts inline. > > [snip] > > Automated testing - Ezequiel Garcia > > --- > > > > Ideal Continuous Integration process consists of the following steps: > > > > 1. patch submission > > 2. review and approval > > 3. merge > > > > The core question is "what level of quality standards do we want to > > enforce". The maintenance process should be modelled around this question, > > and not the other way around. Automated testing can be a part of enforcing > > the quality standards. > > > > There are three steps: > > > > 1. Define the quality standard > > 2. Define how to quantify quality in respect to the standard > > 3. Define how to enforce the standards > > > > On the tooling side, an uAPI test tool exists. It's called v4l2-compliance, > > and new drivers are required to pass the v4l2-compliance test. > > It has quite a few favourable properties: > > > > - Complete in terms of the uAPI coverage > > - Quick and easy to run > > - Nice output format for humans & scripts > > > > There are some issues as well: > > > > - No codec support (stateful or stateless) > > - No SDR or touch support > > - Frequently updated (distribution shipped v4l2-compliance useless) > > - Only one contributor > > > > Ezequiel noted that some people think that v4l2-compliance is changing too > > often but Hans responded that this is a necessity. The API gets amended > > occasionally and the existing API gets new tests. Mauro proposed moving > > v4l2-compliance to the kernel source tree but Hans preferred keeping it > > separate. That way it's easier to develop it. > > > > To address the problem of only a single contributor, it was suggested that > > people implementing new APIs would need to provide the tests for > > v4l2-compliance as well. To achieve this, the v4l2-compliance codebase > > needs some cleanup to make it easier to contribute. The codebase is larger > > and there is no documentation. > > > > V4l2-compliance also covers MC, V4L2 and V4L2 sub-device uAPIs. > > > > DVB will require its own test tooling; it is not covered by > > v4l2-compliance. In order to facilitate automated testing, a virtual DVB > > driver would be useful as well. The task was added to the list of projects > > needing volunteers: > > > > https://linuxtv.org/wiki/index.php/Media_Open_Source_Projects:_Looking_for_Volunteers> > > > > There are some other test tools that could cover V4L2 but at the moment it > > seems somewhat far-fetched any of them would be used to test V4L2 in the > > near future: > > > > - kselftest > > - kunit > > - gst-validate > > - ktf (https://github.com/oracle/ktf, > > http://heim.ifi.uio.no/~knuto/ktf/) > > > > KernelCI is a test automation system that supports automated compile and > > boot testing. As a newly added feature, additional tests may be > > implemented. This is what Collabora has implemented, effectively the > > current demo system runs v4l2-compliance on virtual drivers in a virtual > > machines (LAVA slaves). > > > > A sample of the current test report is here: > > > > https://www.mail-archive.com/linux-media@vger.kernel.org/msg135787.html> > > > > The established way to run KernelCI tests is off the head of the branches of > > the stable and development kernel trees, including linux-next. This is not > > useful as such to support automated testing of patches for the media tree: > > the patches need to be tested before they are merged, not after merging. > > > > In the discusion that followed among a slightly smaller group of people, it > > was suggested that tests could be run from select developer kernel trees, > > from any branch. If a developer needs long-term storage, (s)he could have > > another tree which would not be subject automated test builds. > > Alternatively, the branch name could be used as a basis for triggering > > an automated build, but this could end up
Re: [PATCH v2 for v4.4 1/1] v4l: event: Add subscription to list before calling "add" operation
Hi Greg, On Mon, Nov 19, 2018 at 04:14:00PM +0100, Greg Kroah-Hartman wrote: > On Wed, Nov 14, 2018 at 11:37:46AM +0200, Sakari Ailus wrote: > > [ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ] > > There is no such git commit id in Linus's tree :( Right. At the moment it's in the media tree only. I expect it'll end up to Linus's tree once Mauro will send the next pull request from the media tree to Linus. -- Regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v2 for v4.4 1/1] v4l: event: Add subscription to list before calling "add" operation
On Mon, Nov 19, 2018 at 07:03:54PM +0200, Sakari Ailus wrote: > Hi Greg, > > On Mon, Nov 19, 2018 at 04:14:00PM +0100, Greg Kroah-Hartman wrote: > > On Wed, Nov 14, 2018 at 11:37:46AM +0200, Sakari Ailus wrote: > > > [ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ] > > > > There is no such git commit id in Linus's tree :( > > Right. At the moment it's in the media tree only. I expect it'll end up to > Linus's tree once Mauro will send the next pull request from the media tree > to Linus. Ok, please do not send requests for stable tree inclusion until _AFTER_ the patch is in Linus's tree, otherwise it just wastes the stable tree maintainer's time :( greg k-h
Re: [PATCH 2/2] media: video-i2c: add Melexis MLX90640 thermal camera support
On Mon, Nov 19, 2018 at 6:26 AM Hans Verkuil wrote:
>
> On 11/01/2018 05:15 AM, Matt Ranostay wrote:
> > Add initial support for MLX90640 thermal cameras which output an 32x24
> > greyscale pixel image along with 2 rows of coefficent data.
> >
> > Because of this the data outputed is really 32x26 and needs the two rows
> > removed after using the coefficent information to generate processed
> > images in userspace.
> >
> > Signed-off-by: Matt Ranostay
> > ---
> > drivers/media/i2c/Kconfig | 1 +
> > drivers/media/i2c/video-i2c.c | 110 +-
> > 2 files changed, 110 insertions(+), 1 deletion(-)
>
>
>
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 704af210e270..4bfb2c66d192 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -1085,6 +1085,7 @@ config VIDEO_I2C
> > Enable the I2C transport video support which supports the
> > following:
> > * Panasonic AMG88xx Grid-Eye Sensors
> > +* Melexis MLX90640 Thermal Cameras
> >
> > To compile this driver as a module, choose M here: the
> > module will be called video-i2c
> > diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> > index 6d3b6df0b634..38ade8cb7656 100644
> > --- a/drivers/media/i2c/video-i2c.c
> > +++ b/drivers/media/i2c/video-i2c.c
> > @@ -6,6 +6,7 @@
> > *
> > * Supported:
> > * - Panasonic AMG88xx Grid-Eye Sensors
> > + * - Melexis MLX90640 Thermal Cameras
> > */
> >
> > #include
> > @@ -18,6 +19,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -66,12 +68,26 @@ static const struct v4l2_frmsize_discrete amg88xx_size
> > = {
> > .height = 8,
> > };
> >
> > +static const struct v4l2_fmtdesc mlx90640_format = {
> > + .pixelformat = V4L2_PIX_FMT_Y16_BE,
> > +};
> > +
> > +static const struct v4l2_frmsize_discrete mlx90640_size = {
> > + .width = 32,
> > + .height = 26, /* 24 lines of pixel data + 2 lines of processing data
> > */
> > +};
> > +
> > static const struct regmap_config amg88xx_regmap_config = {
> > .reg_bits = 8,
> > .val_bits = 8,
> > .max_register = 0xff
> > };
> >
> > +static const struct regmap_config mlx90640_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 16,
> > +};
> > +
> > struct video_i2c_chip {
> > /* video dimensions */
> > const struct v4l2_fmtdesc *format;
> > @@ -88,6 +104,7 @@ struct video_i2c_chip {
> > unsigned int bpp;
> >
> > const struct regmap_config *regmap_config;
> > + struct nvmem_config *nvmem_config;
> >
> > /* setup function */
> > int (*setup)(struct video_i2c_data *data);
> > @@ -102,6 +119,22 @@ struct video_i2c_chip {
> > int (*hwmon_init)(struct video_i2c_data *data);
> > };
> >
> > +static int mlx90640_nvram_read(void *priv, unsigned int offset, void *val,
> > + size_t bytes)
> > +{
> > + struct video_i2c_data *data = priv;
> > +
> > + return regmap_bulk_read(data->regmap, 0x2400 + offset, val, bytes);
> > +}
> > +
> > +static struct nvmem_config mlx90640_nvram_config = {
> > + .name = "mlx90640_nvram",
> > + .word_size = 2,
> > + .stride = 1,
> > + .size = 1664,
> > + .reg_read = mlx90640_nvram_read,
> > +};
> > +
> > /* Power control register */
> > #define AMG88XX_REG_PCTL 0x00
> > #define AMG88XX_PCTL_NORMAL 0x00
> > @@ -122,12 +155,23 @@ struct video_i2c_chip {
> > /* Temperature register */
> > #define AMG88XX_REG_T01L 0x80
> >
> > +/* Control register */
> > +#define MLX90640_REG_CTL10x800d
> > +#define MLX90640_REG_CTL1_MASK 0x0380
> > +#define MLX90640_REG_CTL1_MASK_SHIFT 7
> > +
> > static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
> > {
> > return regmap_bulk_read(data->regmap, AMG88XX_REG_T01L, buf,
> > data->chip->buffer_size);
> > }
> >
> > +static int mlx90640_xfer(struct video_i2c_data *data, char *buf)
> > +{
> > + return regmap_bulk_read(data->regmap, 0x400, buf,
> > + data->chip->buffer_size);
> > +}
> > +
> > static int amg88xx_setup(struct video_i2c_data *data)
> > {
> > unsigned int mask = AMG88XX_FPSC_1FPS;
> > @@ -141,6 +185,27 @@ static int amg88xx_setup(struct video_i2c_data *data)
> > return regmap_update_bits(data->regmap, AMG88XX_REG_FPSC, mask, val);
> > }
> >
> > +static int mlx90640_setup(struct video_i2c_data *data)
> > +{
> > + unsigned int n, idx;
> > +
> > + for (n = 0; n < data->chip->num_frame_intervals - 1; n++) {
> > + if (data->frame_interval.numerator
> > + != data->chip->frame_intervals[n].numerator)
> > + continue;
> > +
> > + if (data->frame_interval.denominator
> > + == data->chip->frame_intervals[
MICROCHIP ISC DRIVER Bug: Possible NULL struct pointer dereference case
Hello,
I have discovered a bug in the Atmel/Microchip ISC driver while developing a
i2c v4l2 sub-device driver. This bug did not previously occur for me on an
older version of the driver (more details below), but I have confirmed this bug
on a platform based on the SAMA5D2 running a newer kernel v4.20 from the media
tree. I am running the Atmel ISC drivers as built-in and the module I am
developing as an LKM (though the problem will still occur if built-in as well).
Specifically I get a kernel oops when my driver is loaded and the ISC driver
attempts to initialize my sub-device.
The bug summary is as follows:
There is a case where a NULL pointer dereference can possibly occur in regards
to isc->raw_fmt
Details:
isc->raw_fmt is NULL by default. It is referenced in functions such as
isc_try_fmt()
i.e.
if (sensor_is_preferred(isc_fmt))
mbus_code = isc_fmt->mbus_code;
else
mbus_code = isc->raw_fmt->mbus_code;
and is only set in one place as far as I can see:
isc_formats_init()
if (fmt->flags & FMT_FLAG_RAW_FORMAT)
isc->raw_fmt = fmt;
These statements and the others are missing checks or handling for a possible
NULL pointer, so if they are hit this will cause a kernel oops. According to
git bisect, in my current use case, this does not occur until the following
commit:
commit f103ea11cd037943b511fa71c852ff14cc6de8ee
Author: Wenyou Yang
Date: Tue Oct 10 04:46:40 2017 +0200
media: atmel-isc: Rework the format list
To improve the readability of code, split the format array into two,
one for the format description, other for the register configuration.
Meanwhile, add the flag member to indicate the format can be achieved
from the sensor or be produced by the controller, and rename members
related to the register configuration.
Also add more formats support: GREY, ARGB444, ARGB555 and ARGB32.
Signed-off-by: Wenyou Yang
Signed-off-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
Specifically, this is caused by the introduction of new format flag statements
which possibly prevent isc-raw_fmt from being set:
while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
NULL, &mbus_code)) {
mbus_code.index++;
+
fmt = find_format_by_code(mbus_code.code, &i);
- if (!fmt)
+ if ((!fmt) || (!(fmt->flags & FMT_FLAG_FROM_SENSOR)))
continue;
fmt->sd_support = true;
- if (i <= RAW_FMT_IND_END) {
- for (j = ISC_FMT_IND_START; j <= ISC_FMT_IND_END; j++)
- isc_formats[j].isc_support = true;
-
+ if (fmt->flags & FMT_FLAG_RAW_FORMAT)
isc->raw_fmt = fmt;
- }
}
I am happy to provide any more details as needed as well as submit a patch if I
can understand a correct fix. A log of my kernel oops message follows:
Unable to handle kernel NULL pointer dereference at virtual address 0004
pgd = a9560d56
[0004] *pgd=232d7831, *pte=, *ppte=
Internal error: Oops: 17 [#1] ARM
Modules linked in: tw9990(FO+)
CPU: 0 PID: 519 Comm: insmod Tainted: GF O
4.20.0-rc1-00084-gd1a71a33591d-dirty #2
Hardware name: Atmel SAMA5
PC is at isc_try_fmt+0x20c/0x22c
LR is at isc_try_fmt+0x38/0x22c
pc : []lr : []psr: 200e0013
sp : c29a3a68 ip : 0008 fp : c0d0f3b8
r10: c0c03008 r9 : r8 :
r7 : c0d0f010 r6 : c0c03008 r5 : c29a3b38 r4 : c0c2ce88
r3 : 0280 r2 : r1 : 01e0 r0 : c29a3abc
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c53c7d Table: 22864059 DAC: 0051
Process insmod (pid: 519, stack limit = 0x5f911b4f)
Stack: (0xc29a3a68 to 0xc29a4000)
3a60: c0c0a8e0 c0c03008 c29a1c1c c29a3ad4 c07590c4
3a80: 0c00 400e0013 83edcaea c0c03008 c29a2000 c0c0fb80 c0c03008
3aa0: c0c0fb60 b5af c0c0fb80 c29a3ad4 c29a3ac4 c07594a0 400e0013
3ac0:
3ae0:
3b00: 83edcaea c0c03008 c0d0f010
3b20: 0008 0001 c0d0f010 c0d0fe74 c0c03008 c05268f4 0001 0280
3b40: 01e0 32315559 0001
3b60:
3b80:
3ba0:
3bc0:
3be0:
3c00: 83edcaea c0c2cf08 c0d0f060 0008 c0527fe0 0051 c0d4e040
3c20: 0035 0001 c29a3c24 83edcaea c0d4e040 0002
GUARANTEED AND SECURED LOAN!!!
Do you need urgent funding? Either business or personal? We can arrange quick guaranteed loans. Contact us today via email: yourfunding...@gmail.com
RE: media: ov8856: Add support for OV8856 sensor
Hi Sakari, > Hi Ben, > > On Thu, Nov 08, 2018 at 11:41:46AM +0800, Ben Kao wrote: > > This patch adds driver for Omnivision's ov8856 sensor, the driver > > supports following features: > > > > - manual exposure/gain(analog and digital) control support > > - two link frequencies > > - VBLANK/HBLANK support > > - test pattern support > > - media controller support > > - runtime pm support > > - supported resolutions > > + 3280x2464 at 30FPS > > + 1640x1232 at 30FPS > > > > Signed-off-by: Ben Kao > > I just realised the driver is missing the MAINTAINERS entry. Could you provide > one? Just the diff is fine, I'll then squash it to the patch. > > Thanks. > > -- > Regards, > > Sakari Ailus > sakari.ai...@linux.intel.com We are still waiting for Omnivision's approval. Once if get Omnivision's approval, I will update patch with MAINTAINERS entry. Thanks. Regards, Ben Kao
cron job: media_tree daily build: WARNINGS
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 Nov 20 05:00:12 CET 2018 media-tree git hash:fbe57dde7126d1b2712ab5ea93fb9d15f89de708 media_build git hash: a8aef9cea0a4a2f3ea86c0b37bd6a1378018c0c1 v4l-utils git hash: 044d5ab7b0d02683070d01a369c73d462d7a0cee 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.0-2-amd64 linux-git-arm-at91: WARNINGS linux-git-arm-davinci: WARNINGS linux-git-arm-multi: WARNINGS linux-git-arm-pxa: WARNINGS linux-git-arm-stm32: WARNINGS linux-git-arm64: OK linux-git-i686: WARNINGS linux-git-mips: OK linux-git-powerpc64: WARNINGS linux-git-sh: OK linux-git-x86_64: WARNINGS 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.1-i686: OK linux-4.19.1-x86_64: OK linux-4.20-rc1-i686: OK linux-4.20-rc1-x86_64: OK 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: MICROCHIP ISC DRIVER Bug: Possible NULL struct pointer dereference case
On 20.11.2018 01:06, Ken Sloat wrote:
> Hello,
>
> I have discovered a bug in the Atmel/Microchip ISC driver while developing a
> i2c v4l2 sub-device driver. This bug did not previously occur for me on an
> older version of the driver (more details below), but I have confirmed this
> bug on a platform based on the SAMA5D2 running a newer kernel v4.20 from the
> media tree. I am running the Atmel ISC drivers as built-in and the module I
> am developing as an LKM (though the problem will still occur if built-in as
> well). Specifically I get a kernel oops when my driver is loaded and the ISC
> driver attempts to initialize my sub-device.
>
> The bug summary is as follows:
> There is a case where a NULL pointer dereference can possibly occur in
> regards to isc->raw_fmt
Hello Ken,
Indeed this is a bug, I saw it before as well, but so far, this has not
appeared with the sensors we have connected. I have been trying to get
around to fix it, but it's not a simple fix, much rather a rework of the
driver part that handles the raw formats.
Feel free to submit patches if you find a good fix/rework and I will
have a look and test it for the sensors which I currently have.
Thanks again,
Eugen
>
> Details:
> isc->raw_fmt is NULL by default. It is referenced in functions such as
> isc_try_fmt()
>
> i.e.
> if (sensor_is_preferred(isc_fmt))
> mbus_code = isc_fmt->mbus_code;
> else
> mbus_code = isc->raw_fmt->mbus_code;
>
>
> and is only set in one place as far as I can see:
> isc_formats_init()
>
> if (fmt->flags & FMT_FLAG_RAW_FORMAT)
> isc->raw_fmt = fmt;
>
> These statements and the others are missing checks or handling for a possible
> NULL pointer, so if they are hit this will cause a kernel oops. According to
> git bisect, in my current use case, this does not occur until the following
> commit:
>
> commit f103ea11cd037943b511fa71c852ff14cc6de8ee
> Author: Wenyou Yang
> Date: Tue Oct 10 04:46:40 2017 +0200
>
> media: atmel-isc: Rework the format list
>
> To improve the readability of code, split the format array into two,
> one for the format description, other for the register configuration.
> Meanwhile, add the flag member to indicate the format can be achieved
> from the sensor or be produced by the controller, and rename members
> related to the register configuration.
>
> Also add more formats support: GREY, ARGB444, ARGB555 and ARGB32.
>
> Signed-off-by: Wenyou Yang
> Signed-off-by: Hans Verkuil
> Signed-off-by: Mauro Carvalho Chehab
>
>
> Specifically, this is caused by the introduction of new format flag
> statements which possibly prevent isc-raw_fmt from being set:
>
> while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
> NULL, &mbus_code)) {
> mbus_code.index++;
> +
> fmt = find_format_by_code(mbus_code.code, &i);
> - if (!fmt)
> + if ((!fmt) || (!(fmt->flags & FMT_FLAG_FROM_SENSOR)))
> continue;
>
> fmt->sd_support = true;
>
> - if (i <= RAW_FMT_IND_END) {
> - for (j = ISC_FMT_IND_START; j <= ISC_FMT_IND_END; j++)
> - isc_formats[j].isc_support = true;
> -
> + if (fmt->flags & FMT_FLAG_RAW_FORMAT)
> isc->raw_fmt = fmt;
> - }
> }
>
> I am happy to provide any more details as needed as well as submit a patch if
> I can understand a correct fix. A log of my kernel oops message follows:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0004
> pgd = a9560d56
> [0004] *pgd=232d7831, *pte=, *ppte=
> Internal error: Oops: 17 [#1] ARM
> Modules linked in: tw9990(FO+)
> CPU: 0 PID: 519 Comm: insmod Tainted: GF O
> 4.20.0-rc1-00084-gd1a71a33591d-dirty #2
> Hardware name: Atmel SAMA5
> PC is at isc_try_fmt+0x20c/0x22c
> LR is at isc_try_fmt+0x38/0x22c
> pc : []lr : []psr: 200e0013
> sp : c29a3a68 ip : 0008 fp : c0d0f3b8
> r10: c0c03008 r9 : r8 :
> r7 : c0d0f010 r6 : c0c03008 r5 : c29a3b38 r4 : c0c2ce88
> r3 : 0280 r2 : r1 : 01e0 r0 : c29a3abc
> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c53c7d Table: 22864059 DAC: 0051
> Process insmod (pid: 519, stack limit = 0x5f911b4f)
> Stack: (0xc29a3a68 to 0xc29a4000)
> 3a60: c0c0a8e0 c0c03008 c29a1c1c c29a3ad4 c07590c4
> 3a80: 0c00 400e0013 83edcaea c0c03008 c29a2000 c0c0fb80 c0c03008
> 3aa0: c0c0fb60 b5af c0c0fb80 c29a3ad4 c29a3ac4 c07594a0 400e0013
> 3ac0:
> 3ae0:
> 3b00: 83edcaea c0c03008
Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote: > Hi Mike, > > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox wrote: > > > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote: > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport wrote: > > > > > + * vm_insert_range - insert range of kernel pages into user vma > > > > > + * @vma: user vma to map to > > > > > + * @addr: target user address of this page > > > > > + * @pages: pointer to array of source kernel pages > > > > > + * @page_count: no. of pages need to insert into user vma > > > > > + * > > > > > + * This allows drivers to insert range of kernel pages they've > > > > > allocated > > > > > + * into a user vma. This is a generic function which drivers can use > > > > > + * rather than using their own way of mapping range of kernel pages > > > > > into > > > > > + * user vma. > > > > > > > > Please add the return value and context descriptions. > > > > > > > > > > Sure I will wait for some time to get additional review comments and > > > add all of those requested changes in v2. > > > > You could send your proposed wording now which might remove the need > > for a v3 if we end up arguing about the wording. > > Does this description looks good ? > > /** > * vm_insert_range - insert range of kernel pages into user vma > * @vma: user vma to map to > * @addr: target user address of this page > * @pages: pointer to array of source kernel pages > * @page_count: number of pages need to insert into user vma > * > * This allows drivers to insert range of kernel pages they've allocated > * into a user vma. This is a generic function which drivers can use > * rather than using their own way of mapping range of kernel pages into > * user vma. > * > * Context - Process context. Called by mmap handlers. Context: > * Return - int error value Return: > * 0- OK > * -EINVAL - Invalid argument > * -ENOMEM - No memory > * -EFAULT - Bad address > * -EBUSY - Device or resource busy I don't think that elaborate description of error values is needed, just "0 on success and error code otherwise" would be sufficient. > */ > -- Sincerely yours, Mike.
Re: [PATCH v2 6/9] phy: Move Allwinner A31 D-PHY driver to drivers/phy/
Hi Maxime, On Tue, Nov 06, 2018 at 03:54:18PM +0100, Maxime Ripard wrote: > Now that our MIPI D-PHY driver has been converted to the phy framework, > let's move it into the drivers/phy directory. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/sun4i/Kconfig | 10 +- > drivers/gpu/drm/sun4i/Makefile | 1 +- > drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 318 +- > drivers/phy/allwinner/Kconfig | 12 +- > drivers/phy/allwinner/Makefile | 1 +- > drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 +- > 6 files changed, 332 insertions(+), 328 deletions(-) > delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c > create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c Could you use git format-patch -M option on the next time, please? -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport wrote: > > On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote: > > Hi Mike, > > > > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox wrote: > > > > > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote: > > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport > > > > wrote: > > > > > > + * vm_insert_range - insert range of kernel pages into user vma > > > > > > + * @vma: user vma to map to > > > > > > + * @addr: target user address of this page > > > > > > + * @pages: pointer to array of source kernel pages > > > > > > + * @page_count: no. of pages need to insert into user vma > > > > > > + * > > > > > > + * This allows drivers to insert range of kernel pages they've > > > > > > allocated > > > > > > + * into a user vma. This is a generic function which drivers can > > > > > > use > > > > > > + * rather than using their own way of mapping range of kernel > > > > > > pages into > > > > > > + * user vma. > > > > > > > > > > Please add the return value and context descriptions. > > > > > > > > > > > > > Sure I will wait for some time to get additional review comments and > > > > add all of those requested changes in v2. > > > > > > You could send your proposed wording now which might remove the need > > > for a v3 if we end up arguing about the wording. > > > > Does this description looks good ? > > > > /** > > * vm_insert_range - insert range of kernel pages into user vma > > * @vma: user vma to map to > > * @addr: target user address of this page > > * @pages: pointer to array of source kernel pages > > * @page_count: number of pages need to insert into user vma > > * > > * This allows drivers to insert range of kernel pages they've allocated > > * into a user vma. This is a generic function which drivers can use > > * rather than using their own way of mapping range of kernel pages into > > * user vma. > > * > > * Context - Process context. Called by mmap handlers. > > Context: > > > * Return - int error value > > Return: > > > * 0- OK > > * -EINVAL - Invalid argument > > * -ENOMEM - No memory > > * -EFAULT - Bad address > > * -EBUSY - Device or resource busy > > I don't think that elaborate description of error values is needed, just "0 > on success and error code otherwise" would be sufficient. /** * vm_insert_range - insert range of kernel pages into user vma * @vma: user vma to map to * @addr: target user address of this page * @pages: pointer to array of source kernel pages * @page_count: number of pages need to insert into user vma * * This allows drivers to insert range of kernel pages they've allocated * into a user vma. This is a generic function which drivers can use * rather than using their own way of mapping range of kernel pages into * user vma. * * Context: Process context. Called by mmap handlers. * Return: 0 on success and error code otherwise */
Re: DVB-S PCI card regression on 4.19 / 4.20
Hi Takashi,
Em Mon, 19 Nov 2018 16:13:29 +0100
Takashi Iwai escreveu:
> Hi,
>
> we've got a regression report on openSUSE Bugzilla regarding DVB-S PCI
> card:
> https://bugzilla.opensuse.org/show_bug.cgi?id=1116374
>
> According to the reporter (Stakanov, Cc'ed), the card worked fine on
> 4.18.15, but since 4.19, it doesn't give any channels, sound nor
> picture, but only EPG is received.
Receiving just EPG is weird.
>
> The following errors might be relevant:
>
>
> [4.845180] b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver
> chip loaded successfully
> [4.869703] b2c2-flexcop: MAC address = xx:xx:xx:xx:xx:xx
> [5.100318] b2c2-flexcop: found 'ST STV0299 DVB-S' .
> [5.100323] b2c2_flexcop_pci :06:06.0: DVB: registering adapter 0
> frontend 0 (ST STV0299 DVB-S)...
> [5.100370] b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S rev
> 2.6' at the 'PCI' bus controlled by a 'FlexCopIIb' complete
> [ 117.513086] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0
> frequency 1549000 out of range (95..2150)
> [ 124.905222] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0
> frequency 188 out of range (95..2150)
> [ 127.337079] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0
> frequency 1353500 out of range (95..2150)
That indicates that it is trying to tune to an unsupported frequency. For
DVB-S, all frequencies above should be in kHz.
It is very weird that the low frequency is bigger than the higher one.
I suspect that this could be the root cause of the issue.
Yet, the entry at stv0299 (with apparently is the used frontend)
seems correct, as both min and max frequencies are in MHz:
static const struct dvb_frontend_ops stv0299_ops = {
.delsys = { SYS_DVBS },
.info = {
.name = "ST STV0299 DVB-S",
.frequency_min_hz = 950 * MHz,
.frequency_max_hz = 2150 * MHz,
.frequency_stepsize_hz = 125 * kHz,
.symbol_rate_min= 100,
.symbol_rate_max= 4500,
.symbol_rate_tolerance = 500, /* ppm */
.caps = FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 | FE_CAN_FEC_3_4 |
FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 |
FE_CAN_QPSK |
FE_CAN_FEC_AUTO
},
It could be some mistake at the tuner (if this driver loads a
separate tuner).
From the code, it seems that this specific board uses this tuner
from dvb-pll:
static const struct dvb_pll_desc dvb_pll_samsung_tbmu24112 = {
.name = "Samsung TBMU24112",
.min= 95,
.max= 215, /* guesses */
.iffreq = 0,
.count = 2,
.entries = {
{ 150, 125, 0x84, 0x18 },
{ 999, 125, 0x84, 0x08 },
}
};
Here, frequencies are in kHz. The code should be converting them to Hz
if the TV standard is satellite.
>
>
> The lspci shows:
>
> 06:06.0 Network controller: Techsan Electronics Co Ltd B2C2 FlexCopII DVB
> chip / Technisat SkyStar2 DVB card (rev 02)
> Subsystem: Techsan Electronics Co Ltd B2C2 FlexCopII DVB chip /
> Technisat SkyStar2 DVB card
> Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx-
> Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort-
> SERR- Latency: 32
> Interrupt: pin A routed to IRQ 20
> NUMA node: 0
> Region 0: Memory at fe50 (32-bit, non-prefetchable) [size=64K]
> Region 1: I/O ports at b040 [size=32]
> Kernel driver in use: b2c2_flexcop_pci
> Kernel modules: b2c2_flexcop_pci
>
>
> Other details can be found in the bugzilla entry above.
>
> If any fix is known, please let me know. I'll build a test kernel for
> openSUSE to confirm it.
>
>
> Thanks!
>
> Takashi
Thanks,
Mauro
--
Could you ask the user to apply the enclosed patch and provide us
the results of those prints?
diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
index 0f8b31874002..60874a1f3d89 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
@@ -1,4 +1,15 @@
-.. SPDX-License-Identifier: GPL-2.0 OR GFDL-1.1-or-later WITH
no-invariant-sections
+.. SPDX License for this file: GPL-2.0 OR GFDL-1.1-or-later
+..
+.. For GPL-2.0, see LICENSES/preferred/GPL-2.0
+..
+.. For GFDL-1.1-or-later, see:
+..
+.. Permission is granted to copy, distribute and/or modify this document
+.. under the terms of the GNU Free Documentation License, Version 1.1 or
+.. any later version published by the Free Software Foundation, with no
+.. Invariant Sections, no Front-Cover Texts and no Back
Re: DVB-S PCI card regression on 4.19 / 4.20
Hi On 19/11/2018 17:53, Mauro Carvalho Chehab wrote: > Hi Takashi, > > Em Mon, 19 Nov 2018 16:13:29 +0100 > Takashi Iwai escreveu: > >> Hi, >> >> we've got a regression report on openSUSE Bugzilla regarding DVB-S PCI >> card: >> https://bugzilla.opensuse.org/show_bug.cgi?id=1116374 >> >> According to the reporter (Stakanov, Cc'ed), the card worked fine on >> 4.18.15, but since 4.19, it doesn't give any channels, sound nor >> picture, but only EPG is received. > > Receiving just EPG is weird. > >> >> The following errors might be relevant: >> >> >> [4.845180] b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver >> chip loaded successfully >> [4.869703] b2c2-flexcop: MAC address = xx:xx:xx:xx:xx:xx >> [5.100318] b2c2-flexcop: found 'ST STV0299 DVB-S' . >> [5.100323] b2c2_flexcop_pci :06:06.0: DVB: registering adapter 0 >> frontend 0 (ST STV0299 DVB-S)... >> [5.100370] b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S rev >> 2.6' at the 'PCI' bus controlled by a 'FlexCopIIb' complete > > >> [ 117.513086] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 >> frequency 1549000 out of range (95..2150) >> [ 124.905222] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 >> frequency 188 out of range (95..2150) >> [ 127.337079] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 >> frequency 1353500 out of range (95..2150) > > That indicates that it is trying to tune to an unsupported frequency. For > DVB-S, all frequencies above should be in kHz. Kaffeine reports the wrong LNB frequency ranges for universal Europe... I just use Astra 1E settings instead which I am pretty sure is the same? I am using the stv0299 / dvb_pll(opera1) combi and not reporting any problems other than the streaming issue with dvb-usb-v2 which I don't think this uses? Regards Malcolm
Re: DVB-S PCI card regression on 4.19 / 4.20
On Mon, 19 Nov 2018 18:53:26 +0100, Mauro Carvalho Chehab wrote: > > Could you ask the user to apply the enclosed patch and provide us > the results of those prints? OK, I built a test kernel in OBS home:tiwai:bsc1116374 repo. Now it's available at https://download.opensuse.org/repositories/home:/tiwai:/bsc1116374/standard/ Stakanov, could you test it and give the kernel messages? Thanks! Takashi
Re: [PATCH v2 8/9] phy: Add Cadence D-PHY support
Hi Maxime,
On Tue, Nov 06, 2018 at 03:54:20PM +0100, Maxime Ripard wrote:
> Cadence has designed a D-PHY that can be used by the, currently in tree,
> DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
>
> Only the DSI driver has an ad-hoc driver for that phy at the moment, while
> the v4l2 drivers are completely missing any phy support. In order to make
> that phy support available to all these drivers, without having to
> duplicate that code three times, let's create a generic phy framework
> driver.
>
> Signed-off-by: Maxime Ripard
> ---
> Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +-
> Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +-
Coould you split out the DT binding changes into a separate patch?
> drivers/phy/cadence/Kconfig | 13 +-
> drivers/phy/cadence/Makefile | 1 +-
> drivers/phy/cadence/cdns-dphy.c | 459 +++-
> 5 files changed, 492 insertions(+), 22 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
> create mode 100644 drivers/phy/cadence/cdns-dphy.c
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> index f5725bb6c61c..525a4bfd8634 100644
> --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> @@ -31,28 +31,7 @@ Required subnodes:
> - one subnode per DSI device connected on the DSI bus. Each DSI device should
>contain a reg property encoding its virtual channel.
>
> -Cadence DPHY
> -
> -
> -Cadence DPHY block.
> -
> -Required properties:
> -- compatible: should be set to "cdns,dphy".
> -- reg: physical base address and length of the DPHY registers.
> -- clocks: DPHY reference clocks.
> -- clock-names: must contain "psm" and "pll_ref".
> -- #phy-cells: must be set to 0.
> -
> -
> Example:
> - dphy0: dphy@fd0e{
> - compatible = "cdns,dphy";
> - reg = <0x0 0xfd0e 0x0 0x1000>;
> - clocks = <&psm_clk>, <&pll_ref_clk>;
> - clock-names = "psm", "pll_ref";
> - #phy-cells = <0>;
> - };
> -
> dsi0: dsi@fd0c {
> compatible = "cdns,dsi";
> reg = <0x0 0xfd0c 0x0 0x1000>;
> diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt
> b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
> new file mode 100644
> index ..1095bc4e72d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
> @@ -0,0 +1,20 @@
> +Cadence DPHY
> +
> +
> +Cadence DPHY block.
> +
> +Required properties:
> +- compatible: should be set to "cdns,dphy".
> +- reg: physical base address and length of the DPHY registers.
> +- clocks: DPHY reference clocks.
> +- clock-names: must contain "psm" and "pll_ref".
> +- #phy-cells: must be set to 0.
> +
> +Example:
> + dphy0: dphy@fd0e{
> + compatible = "cdns,dphy";
> + reg = <0x0 0xfd0e 0x0 0x1000>;
> + clocks = <&psm_clk>, <&pll_ref_clk>;
> + clock-names = "psm", "pll_ref";
> + #phy-cells = <0>;
> + };
> diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
> index 57fff7de4031..240effa81046 100644
> --- a/drivers/phy/cadence/Kconfig
> +++ b/drivers/phy/cadence/Kconfig
> @@ -1,6 +1,7 @@
> #
> -# Phy driver for Cadence MHDP DisplayPort controller
> +# Phy drivers for Cadence IPs
> #
> +
> config PHY_CADENCE_DP
> tristate "Cadence MHDP DisplayPort PHY driver"
> depends on OF
> @@ -8,3 +9,13 @@ config PHY_CADENCE_DP
> select GENERIC_PHY
> help
> Support for Cadence MHDP DisplayPort PHY.
> +
> +config PHY_CADENCE_DPHY
> + tristate "Cadence D-PHY Support"
> + depends on HAS_IOMEM && OF
> + select GENERIC_PHY
> + select GENERIC_PHY_MIPI_DPHY
> + help
> + Choose this option if you have a Cadence D-PHY in your
> + system. If M is selected, the module will be called
> + cdns-csi.
> diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
> index e5b0a11cf28a..64cb9ea66ceb 100644
> --- a/drivers/phy/cadence/Makefile
> +++ b/drivers/phy/cadence/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
> +obj-$(CONFIG_PHY_CADENCE_DPHY) += cdns-dphy.o
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> new file mode 100644
> index ..a398b401e5d3
> --- /dev/null
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -0,0 +1,459 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright: 2017-2018 Cadence Design Systems, Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#define REG_W
Re: DVB-S PCI card regression on 4.19 / 4.20
In data lunedì 19 novembre 2018 20:47:19 CET, Takashi Iwai ha scritto: > On Mon, 19 Nov 2018 18:53:26 +0100, > > Mauro Carvalho Chehab wrote: > > Could you ask the user to apply the enclosed patch and provide us > > the results of those prints? > > OK, I built a test kernel in OBS home:tiwai:bsc1116374 repo. Now it's > available at > > https://download.opensuse.org/repositories/home:/tiwai:/bsc1116374/standard > / > > Stakanov, could you test it and give the kernel messages? > > > Thanks! > > Takashi Here we go, I did saw your mail only late, sorry. Result of proposed fix (rc3): card has still no function, does not sync, EPG works. No sound no picture. entropy@silversurfer:~> uname -a Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux output of entropy@silversurfer:~> sudo dmesg | grep -i b2c2 [4.831163] b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver chip loaded successfully [4.862648] b2c2-flexcop: MAC address = xx:xx:xx:xx:xx:xx [5.094173] b2c2-flexcop: found 'ST STV0299 DVB-S' . [5.094177] b2c2_flexcop_pci :06:06.0: DVB: registering adapter 0 frontend 0 (ST STV0299 DVB-S)... [5.094248] b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S rev 2.6' at the 'PCI' bus controlled by a 'FlexCopIIb' complete [ 121.789236] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 frequency 188 out of range (95..2150) [ 128.817325] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 frequency 1944750 out of range (95..2150) sudo lspci -vvv 06:06.0 Network controller: Techsan Electronics Co Ltd B2C2 FlexCopII DVB chip / Technisat SkyStar2 DVB card (rev 02) Subsystem: Techsan Electronics Co Ltd B2C2 FlexCopII DVB chip / Technisat SkyStar2 DVB card Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- SERR- https://www.eclipso.de
Re: DVB-S PCI card regression on 4.19 / 4.20
Em Mon, 19 Nov 2018 23:59:39 +0100 stakanov escreveu: > In data lunedì 19 novembre 2018 20:47:19 CET, Takashi Iwai ha scritto: > > On Mon, 19 Nov 2018 18:53:26 +0100, > > > > Mauro Carvalho Chehab wrote: > > > Could you ask the user to apply the enclosed patch and provide us > > > the results of those prints? > > > > OK, I built a test kernel in OBS home:tiwai:bsc1116374 repo. Now it's > > available at > > > > https://download.opensuse.org/repositories/home:/tiwai:/bsc1116374/standard > > / > > > > Stakanov, could you test it and give the kernel messages? > > > > > > Thanks! > > > > Takashi > Here we go, I did saw your mail only late, sorry. > > > Result of proposed fix (rc3): card has still no function, does not sync, EPG > works. No sound no picture. This is not a fix. It just adds some new messages that should help to explain why the frequency range seems wrong. > > entropy@silversurfer:~> uname -a > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux > > output of > entropy@silversurfer:~> sudo dmesg | grep -i b2c2 > [4.831163] b2c2-flexcop: B2C2 FlexcopII/II(b)/III digital TV receiver > chip > loaded successfully > [4.862648] b2c2-flexcop: MAC address = xx:xx:xx:xx:xx:xx > [5.094173] b2c2-flexcop: found 'ST STV0299 DVB-S' . > [5.094177] b2c2_flexcop_pci :06:06.0: DVB: registering adapter 0 > frontend 0 (ST STV0299 DVB-S)... > [5.094248] b2c2-flexcop: initialization of 'Sky2PC/SkyStar 2 DVB-S rev > 2.6' at the 'PCI' bus controlled by a 'FlexCopIIb' complete > [ 121.789236] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 > frequency 188 out of range (95..2150) > [ 128.817325] b2c2_flexcop_pci :06:06.0: DVB: adapter 0 frontend 0 > frequency 1944750 out of range (95..2150) Are you sure you booted the right Kernel? I'm not seeing the new messages here that should be printing the tuner and frontend frequency ranges. > > sudo lspci -vvv > 06:06.0 Network controller: Techsan Electronics Co Ltd B2C2 FlexCopII DVB > chip > / Technisat SkyStar2 DVB card (rev 02) > Subsystem: Techsan Electronics Co Ltd B2C2 FlexCopII DVB chip / > Technisat SkyStar2 DVB card > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR- FastB2B- DisINTx- > Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- > SERR- Latency: 32 > Interrupt: pin A routed to IRQ 20 > NUMA node: 0 > Region 0: Memory at fe50 (32-bit, non-prefetchable) [size=64K] > Region 1: I/O ports at b040 [size=32] > Kernel driver in use: b2c2_flexcop_pci > Kernel modules: b2c2_flexcop_pci > > > > > _ > > Ihre E-Mail-Postfächer sicher & zentral an einem Ort. Jetzt wechseln und alte > E-Mail-Adresse mitnehmen! https://www.eclipso.de > > Thanks, Mauro
Re: DVB-S PCI card regression on 4.19 / 4.20
In data martedì 20 novembre 2018 00:08:45 CET, Mauro Carvalho Chehab ha scritto: > uname -a > > > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux uname -a > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux from https://download.opensuse.org/repositories/home:/tiwai:/bsc1116374/ standard/x86_64/ So I booted this one, should be the right one. sudo dmesg | grep -i b2c2 should give these additional messages? If Takashi is still around, he could confirm. _ Ihre E-Mail-Postfächer sicher & zentral an einem Ort. Jetzt wechseln und alte E-Mail-Adresse mitnehmen! https://www.eclipso.de
Re: DVB-S PCI card regression on 4.19 / 4.20
Em Tue, 20 Nov 2018 00:19:54 +0100 stakanov escreveu: > In data martedì 20 novembre 2018 00:08:45 CET, Mauro Carvalho Chehab ha > scritto: > > uname -a > > > > > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > > > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux > > uname -a > > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux > > from https://download.opensuse.org/repositories/home:/tiwai:/bsc1116374/ > standard/x86_64/ > > So I booted this one, should be the right one. > sudo dmesg | grep -i b2c2 should give these additional messages? > > If Takashi is still around, he could confirm. Well, if the patch got applied, you should now be getting messages similar (but not identical) to: dvb_frontend_get_frequency_limits: frequencies: tuner: 915...215, frontend: 915...215 dvb_pll_attach: delsys: 5, frequency range: 915..215 > > > > _ > > Ihre E-Mail-Postfächer sicher & zentral an einem Ort. Jetzt wechseln und alte > E-Mail-Adresse mitnehmen! https://www.eclipso.de > > Thanks, Mauro
[PATCH v4 3/4] media: i2c: Add MAX9286 driver
Hi Luca,
My apologies for my travel induced delay in responding here,
On 14/11/2018 02:04, Luca Ceresoli wrote:
> Hi Kieran,
>
> On 14/11/18 01:46, Kieran Bingham wrote:
>> Hi Luca,
>>
>> Thank you for your review,
>>
>> On 13/11/2018 14:49, Luca Ceresoli wrote:
>>> Hi Kieran, All,
>>>
>>> below a few minor questions, and a big one at the bottom.
>>>
>>> On 02/11/18 16:47, Kieran Bingham wrote:
From: Kieran Bingham
The MAX9286 is a 4-channel GMSL deserializer with coax or STP input and
CSI-2 output. The device supports multicamera streaming applications,
and features the ability to synchronise the attached cameras.
CSI-2 output can be configured with 1 to 4 lanes, and a control channel
is supported over I2C, which implements an I2C mux to facilitate
communications with connected cameras across the reverse control
channel.
Signed-off-by: Jacopo Mondi
Signed-off-by: Kieran Bingham
Signed-off-by: Laurent Pinchart
Signed-off-by: Niklas Söderlund
>>>
>>> [...]
>>>
+struct max9286_device {
+ struct i2c_client *client;
+ struct v4l2_subdev sd;
+ struct media_pad pads[MAX9286_N_PADS];
+ struct regulator *regulator;
+ bool poc_enabled;
+ int streaming;
+
+ struct i2c_mux_core *mux;
+ unsigned int mux_channel;
+
+ struct v4l2_ctrl_handler ctrls;
+
+ struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
>>>
>>> 5 pads, 4 formats. Why does the source node have no fmt?
>>
>> The source pad is a CSI2 link - so a 'frame format' would be inappropriate.
>
> Ok, thanks for the clarification.
>
+static int max9286_probe(struct i2c_client *client,
+ const struct i2c_device_id *did)
+{
+ struct max9286_device *dev;
+ unsigned int i;
+ int ret;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->client = client;
+ i2c_set_clientdata(client, dev);
+
+ for (i = 0; i < MAX9286_N_SINKS; i++)
+ max9286_init_format(&dev->fmt[i]);
+
+ ret = max9286_parse_dt(dev);
+ if (ret)
+ return ret;
+
+ dev->regulator = regulator_get(&client->dev, "poc");
+ if (IS_ERR(dev->regulator)) {
+ if (PTR_ERR(dev->regulator) != -EPROBE_DEFER)
+ dev_err(&client->dev,
+ "Unable to get PoC regulator (%ld)\n",
+ PTR_ERR(dev->regulator));
+ ret = PTR_ERR(dev->regulator);
+ goto err_free;
+ }
+
+ /*
+ * We can have multiple MAX9286 instances on the same physical I2C
+ * bus, and I2C children behind ports of separate MAX9286 instances
+ * having the same I2C address. As the MAX9286 starts by default with
+ * all ports enabled, we need to disable all ports on all MAX9286
+ * instances before proceeding to further initialize the devices and
+ * instantiate children.
+ *
+ * Start by just disabling all channels on the current device. Then,
+ * if all other MAX9286 on the parent bus have been probed, proceed
+ * to initialize them all, including the current one.
+ */
+ max9286_i2c_mux_close(dev);
+
+ /*
+ * The MAX9286 initialises with auto-acknowledge enabled by default.
+ * This means that if multiple MAX9286 devices are connected to an I2C
+ * bus, another MAX9286 could ack I2C transfers meant for a device on
+ * the other side of the GMSL links for this MAX9286 (such as a
+ * MAX9271). To prevent that disable auto-acknowledge early on; it
+ * will be enabled later as needed.
+ */
+ max9286_configure_i2c(dev, false);
+
+ ret = device_for_each_child(client->dev.parent, &client->dev,
+ max9286_is_bound);
+ if (ret)
+ return 0;
+
+ dev_dbg(&client->dev,
+ "All max9286 probed: start initialization sequence\n");
+ ret = device_for_each_child(client->dev.parent, NULL,
+ max9286_init);
>>>
>>> I can't manage to like this initialization sequence, sorry. If at all
>>> possible, each max9286 should initialize itself independently from each
>>> other, like any normal driver.
>>
>> Yes, I think we're in agreement here, but unfortunately this section is
>> a workaround for the fact that our devices share a common address space.
>>
>> We (currently) *must* disable both devices before we start the
>> initialisation process for either on our platform currently...
>
> The model I proposed in my review to patch 1/4 (split remote physical
> address from local address pool) allows to avoid this workaround.
Having just talked this through with Jacopo I think I see that we have
two topics getting intertwined her
Re: [PATCH v2 8/9] phy: Add Cadence D-PHY support
Hi,
On 12/11/18 3:21 PM, Kishon Vijay Abraham I wrote:
Hi Maxime,
On 06/11/18 8:24 PM, Maxime Ripard wrote:
Cadence has designed a D-PHY that can be used by the, currently in tree,
DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
Only the DSI driver has an ad-hoc driver for that phy at the moment, while
the v4l2 drivers are completely missing any phy support. In order to make
that phy support available to all these drivers, without having to
duplicate that code three times, let's create a generic phy framework
driver.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +-
Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +-
drivers/phy/cadence/Kconfig | 13 +-
drivers/phy/cadence/Makefile | 1 +-
drivers/phy/cadence/cdns-dphy.c | 459 +++-
5 files changed, 492 insertions(+), 22 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
create mode 100644 drivers/phy/cadence/cdns-dphy.c
diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
index f5725bb6c61c..525a4bfd8634 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
@@ -31,28 +31,7 @@ Required subnodes:
- one subnode per DSI device connected on the DSI bus. Each DSI device should
contain a reg property encoding its virtual channel.
-Cadence DPHY
-
-
-Cadence DPHY block.
-
-Required properties:
-- compatible: should be set to "cdns,dphy".
-- reg: physical base address and length of the DPHY registers.
-- clocks: DPHY reference clocks.
-- clock-names: must contain "psm" and "pll_ref".
-- #phy-cells: must be set to 0.
-
-
Example:
- dphy0: dphy@fd0e{
- compatible = "cdns,dphy";
- reg = <0x0 0xfd0e 0x0 0x1000>;
- clocks = <&psm_clk>, <&pll_ref_clk>;
- clock-names = "psm", "pll_ref";
- #phy-cells = <0>;
- };
-
dsi0: dsi@fd0c {
compatible = "cdns,dsi";
reg = <0x0 0xfd0c 0x0 0x1000>;
diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt
b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
new file mode 100644
index ..1095bc4e72d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
@@ -0,0 +1,20 @@
+Cadence DPHY
+
+
+Cadence DPHY block.
+
+Required properties:
+- compatible: should be set to "cdns,dphy".
+- reg: physical base address and length of the DPHY registers.
+- clocks: DPHY reference clocks.
+- clock-names: must contain "psm" and "pll_ref".
+- #phy-cells: must be set to 0.
+
+Example:
+ dphy0: dphy@fd0e{
+ compatible = "cdns,dphy";
+ reg = <0x0 0xfd0e 0x0 0x1000>;
+ clocks = <&psm_clk>, <&pll_ref_clk>;
+ clock-names = "psm", "pll_ref";
+ #phy-cells = <0>;
+ };
diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
index 57fff7de4031..240effa81046 100644
--- a/drivers/phy/cadence/Kconfig
+++ b/drivers/phy/cadence/Kconfig
@@ -1,6 +1,7 @@
#
-# Phy driver for Cadence MHDP DisplayPort controller
+# Phy drivers for Cadence IPs
#
+
config PHY_CADENCE_DP
tristate "Cadence MHDP DisplayPort PHY driver"
depends on OF
@@ -8,3 +9,13 @@ config PHY_CADENCE_DP
select GENERIC_PHY
help
Support for Cadence MHDP DisplayPort PHY.
+
+config PHY_CADENCE_DPHY
+ tristate "Cadence D-PHY Support"
+ depends on HAS_IOMEM && OF
+ select GENERIC_PHY
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have a Cadence D-PHY in your
+ system. If M is selected, the module will be called
+ cdns-csi.
diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
index e5b0a11cf28a..64cb9ea66ceb 100644
--- a/drivers/phy/cadence/Makefile
+++ b/drivers/phy/cadence/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
+obj-$(CONFIG_PHY_CADENCE_DPHY) += cdns-dphy.o
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
new file mode 100644
index ..a398b401e5d3
--- /dev/null
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright: 2017-2018 Cadence Design Systems, Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#define REG_WAKEUP_TIME_NS 800
+#define DPHY_PLL_RATE_HZ 10800
+
+/* DPHY registers */
+#define DPHY_PMA_CMN(reg) (reg)
+#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
+#define DPHY_PMA_LDATA(lane, r
[PATCH v2] media: venus: amend buffer size for bitstream plane
Accept the buffer size requested by client and compare it
against driver calculated size and set the maximum to
bitstream plane.
Signed-off-by: Malathi Gottam
---
drivers/media/platform/qcom/venus/venc.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c
b/drivers/media/platform/qcom/venus/venc.c
index ce85962..ecfdbd6 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -303,6 +303,7 @@ static int venc_enum_fmt(struct file *file, void *fh,
struct v4l2_fmtdesc *f)
struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp;
struct v4l2_plane_pix_format *pfmt = pixmp->plane_fmt;
const struct venus_format *fmt;
+ __u32 sizeimage;
memset(pfmt[0].reserved, 0, sizeof(pfmt[0].reserved));
memset(pixmp->reserved, 0, sizeof(pixmp->reserved));
@@ -334,9 +335,10 @@ static int venc_enum_fmt(struct file *file, void *fh,
struct v4l2_fmtdesc *f)
pixmp->num_planes = fmt->num_planes;
pixmp->flags = 0;
- pfmt[0].sizeimage = venus_helper_get_framesz(pixmp->pixelformat,
-pixmp->width,
-pixmp->height);
+ sizeimage = venus_helper_get_framesz(pixmp->pixelformat,
+pixmp->width,
+pixmp->height);
+ pfmt[0].sizeimage = max(ALIGN(pfmt[0].sizeimage, SZ_4K), sizeimage);
if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
pfmt[0].bytesperline = ALIGN(pixmp->width, 128);
@@ -408,8 +410,10 @@ static int venc_s_fmt(struct file *file, void *fh, struct
v4l2_format *f)
if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
inst->fmt_out = fmt;
- else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+ else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
inst->fmt_cap = fmt;
+ inst->output_buf_size = pixmp->plane_fmt[0].sizeimage;
+ }
return 0;
}
@@ -908,6 +912,7 @@ static int venc_queue_setup(struct vb2_queue *q,
sizes[0] = venus_helper_get_framesz(inst->fmt_cap->pixfmt,
inst->width,
inst->height);
+ sizes[0] = max(sizes[0], inst->output_buf_size);
inst->output_buf_size = sizes[0];
break;
default:
--
1.9.1
Re: DVB-S PCI card regression on 4.19 / 4.20
On Tue, 20 Nov 2018 00:19:54 +0100, stakanov wrote: > > In data martedì 20 novembre 2018 00:08:45 CET, Mauro Carvalho Chehab ha > scritto: > > uname -a > > > > > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > > > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux > > uname -a > > Linux silversurfer 4.20.0-rc3-1.g7e16618-default #1 SMP PREEMPT Mon Nov 19 > > 18:54:15 UTC 2018 (7e16618) x86_64 x86_64 x86_64 GNU/Linux > > from https://download.opensuse.org/repositories/home:/tiwai:/bsc1116374/ > standard/x86_64/ The kernel must be correct (7e16618 contains the patch, I confirmed). > So I booted this one, should be the right one. > sudo dmesg | grep -i b2c2 should give these additional messages? Avoid grep but look around the whole lines. The added debug print is no dev_info() or such but just a plain printk(), hence it won't give any prefix. thanks, Takashi
