Re: [PATCH 2/2] [media] m2m: fix bad unlock balance
On Fri, Aug 14, 2015 at 01:57:51PM +0200, Hans Verkuil wrote:
> On 08/12/2015 05:50 PM, Kamil Debski wrote:
> > Hi,
> >
> > On 12 August 2015 at 13:42, Marek Szyprowski
> > wrote:
> >> Hello Hans,
> >>
> >> I'm sorry for a delay. Once again I've been busy with some other internal
> >> stuff.
> >>
> >> On 2015-07-28 11:02, Hans Verkuil wrote:
> >>>
> >>> Kamil, Marek,
> >>>
> >>> Why does v4l2_m2m_poll unlock and lock in that function?
> >>
> >>
> >> I've checked the code and indeed the poll_wait() function doesn't do
> >> anything that
> >> should not be done with queue mutex being taken. I don't remember if it was
> >> always
> >> like that. You are right that the unlock&lock code should be removed.
> >>
> >>> Zahari is right that the locking is unbalanced, but I don't see the reason
> >>> for the unlock/lock sequence in the first place. I'm wondering if that
> >>> shouldn't just be removed.
> >>>
> >>> Am I missing something?
> >>>
> >>> Instead, I would expect to see a spin_lock_irqsave(&src/dst_q->done_lock,
> >>> flags)
> >>> around the list_empty(&src/dst_q->done_list) calls.
> >>
> >>
> >> Indeed, that's another thing that should be fixed in this function. I looks
> >> that
> >> commit c16218402a000bb25c1277c43ae98c11bcb59bd1 ("[media] videobuf2: return
> >> -EPIPE
> >> from DQBUF after the last buffer") is the root cause of both issues
> >> (unballanced
> >> locking and lack of spinlock protection), while the unnecessary queue
> >> unlock/lock
> >> sequence was there from the beginning.
> >>
> >
> > I am all with Marek on this. Unlock/lock was there from the beginning,
> > it is not necessary. I agree also that spin_lock/unlock should be
> > added for the list_empty call.
>
> Zahari, will you make a new version of this patch with the suggested changes?
yes I will do it.
Regards
Zahari
>
> Regards,
>
> Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] [media] m2m: fix bad unlock balance
This patch removes unnecessary mutex queue unlock/lock sequence causing bad
unlock balance in v4l2_m2m_poll when the last buffer on the destination
queue has been dequeued and adds spin lock protection for the done list
list_empty calls.
[ 144.990873] =
[ 144.995584] [ BUG: bad unlock balance detected! ]
[ 145.000301] 4.1.0-00137-ga105070 #98 Tainted: GW
[ 145.006140] -
[ 145.010851] demux:sink/487 is trying to release lock (&dev->dev_mutex) at:
[ 145.017785] [<808cc578>] mutex_unlock+0x18/0x1c
[ 145.022322] but there are no more locks to release!
[ 145.027205]
[ 145.027205] other info that might help us debug this:
[ 145.033741] no locks held by demux:sink/487.
[ 145.038015]
[ 145.038015] stack backtrace:
[ 145.042385] CPU: 2 PID: 487 Comm: demux:sink Tainted: GW
4.1.0-00137-ga105070 #98
[ 145.051089] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[ 145.057622] Backtrace:
[ 145.060102] [<80014a4c>] (dump_backtrace) from [<80014cc4>]
(show_stack+0x20/0x24)
[ 145.067679] r6:80cedf78 r5: r4: r3:
[ 145.073421] [<80014ca4>] (show_stack) from [<808c61e0>]
(dump_stack+0x8c/0xa4)
[ 145.080661] [<808c6154>] (dump_stack) from [<80072b64>]
(print_unlock_imbalance_bug+0xb8/0xe8)
[ 145.089277] r6:808cc578 r5:ac6cd050 r4:ac38e400 r3:0001
[ 145.095020] [<80072aac>] (print_unlock_imbalance_bug) from [<80077db4>]
(lock_release+0x1a4/0x250)
[ 145.103983] r6:808cc578 r5:ac6cd050 r4:ac38e400 r3:
[ 145.109728] [<80077c10>] (lock_release) from [<808cc470>]
(__mutex_unlock_slowpath+0xc4/0x1b4)
[ 145.118344] r9:acb27a41 r8: r7:81553814 r6:808cc578 r5:60030013
r4:ac6cd01c
[ 145.126190] [<808cc3ac>] (__mutex_unlock_slowpath) from [<808cc578>]
(mutex_unlock+0x18/0x1c)
[ 145.134720] r7: r6:aced7cd4 r5:0041 r4:acb87800
[ 145.140468] [<808cc560>] (mutex_unlock) from [<805a98b8>]
(v4l2_m2m_fop_poll+0x5c/0x64)
[ 145.148494] [<805a985c>] (v4l2_m2m_fop_poll) from [<805955a0>]
(v4l2_poll+0x6c/0xa0)
[ 145.156243] r6:aced7bec r5: r4:ac6cc380 r3:805a985c
[ 145.161991] [<80595534>] (v4l2_poll) from [<80156edc>]
(do_sys_poll+0x230/0x4c0)
[ 145.169391] r5: r4:aced7be4
[ 145.173013] [<80156cac>] (do_sys_poll) from [<801574a8>]
(SyS_ppoll+0x1d4/0x1fc)
[ 145.180414] r10: r9:aced6000 r8: r7: r6:75c04538
r5:0002
[ 145.188338] r4:
[ 145.190906] [<801572d4>] (SyS_ppoll) from [<800108c0>]
(ret_fast_syscall+0x0/0x54)
[ 145.198481] r8:80010aa4 r7:0150 r6:75c04538 r5:0002 r4:0008
Signed-off-by: Zahari Doychev
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index ec3ad4e..2f7291c 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -583,32 +583,25 @@ unsigned int v4l2_m2m_poll(struct file *file, struct
v4l2_m2m_ctx *m2m_ctx,
goto end;
}
- if (m2m_ctx->m2m_dev->m2m_ops->unlock)
- m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
- else if (m2m_ctx->q_lock)
- mutex_unlock(m2m_ctx->q_lock);
-
+ spin_lock_irqsave(&src_q->done_lock, flags);
if (list_empty(&src_q->done_list))
poll_wait(file, &src_q->done_wq, wait);
+ spin_unlock_irqrestore(&src_q->done_lock, flags);
+
+ spin_lock_irqsave(&dst_q->done_lock, flags);
if (list_empty(&dst_q->done_list)) {
/*
* If the last buffer was dequeued from the capture queue,
* return immediately. DQBUF will return -EPIPE.
*/
- if (dst_q->last_buffer_dequeued)
+ if (dst_q->last_buffer_dequeued) {
+ spin_unlock_irqrestore(&dst_q->done_lock, flags);
return rc | POLLIN | POLLRDNORM;
+ }
poll_wait(file, &dst_q->done_wq, wait);
}
-
- if (m2m_ctx->m2m_dev->m2m_ops->lock)
- m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
- else if (m2m_ctx->q_lock) {
- if (mutex_lock_interruptible(m2m_ctx->q_lock)) {
- rc |= POLLERR;
- goto end;
- }
- }
+ spin_unlock_irqrestore(&dst_q->done_lock, flags);
spin_lock_irqsave(&src_q->done_lock, flags);
if (!list_empty(&src_q->done_list))
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Media Controller, the next generation
On 08/16/2015 03:37 PM, Mauro Carvalho Chehab wrote: > Em Sat, 15 Aug 2015 21:25:40 +0300 > Laurent Pinchart escreveu: > >> Hi Hans, >> >> Thank you for the RFC. I believe it's quite good (at least to my eyes). >> Everything is of course not perfect yet so please see below for some >> comments. > > (added c/c linux-...@vger.kernel.org) > >> >> On Monday 03 August 2015 16:07:48 Hans Verkuil wrote: >>> Hi all, >>> >>> During last week's brainstorm meeting in Espoo, Finland, we discussed >>> how to proceed with the MC API. Trying to apply the existing API to DVB >>> devices caused a lot of controversy and this meeting was an attempt to >>> resolve these issues. >>> >>> This RFC is the proposal for the public API (NOT the internal API!) we >>> came up with. >>> >>> The main change is that interfaces (such as devices in /dev) get their own >>> object: struct media_interface. The struct media_entity as is used today >>> will not refer to interfaces anymore. >>> >>> Since interfaces control entities we also want to tell userspace which >>> entities are controlled by which interfaces. We want to keep things simple >>> and avoid having to create a new link structure just for this, > > I agree with the Hans proposal. Having a single object to represent graph > links makes easier for both Kernelspace and Userspace to handle them. Right. >> During the meeting we discussed the possibility of creating a >> media_interface_link structure (as drafted in >> http://linuxtv.org/downloads/presentations/mc_ws_2015/media.h). What would >> be >> the advantage of reusing media_link ? > > I would reverse the question: what's the disadvantage? > > By using the same type, it should be easier to change userspace programs > to support both interface and data links. > > With a single struct, if/when apps need to do graph traversal, they can > use a single algorithm. It also means a single code to allocate and > fetch the structs that come from the Kernel. Also, since all graph objects (interfaces, entities, pads) all use the same base struct (media_graph_obj) it makes sense that we use the same media_link struct for linking two graph objects. >>> so instead a struct media_link just provides two object IDs and a flags >>> field. The object IDs refer to pads (for data links) or entity/interface IDs >>> (for associating interfaces with entities). >>> >>> To make this work we need unique pad IDs. >>> >>> We will need to represent connectors as well. The media_entity struct is >>> used for that, but it will be marked as a connector since connectors work >>> slightly different from an entity: if an entity has both input and output >>> pads, then that means that the entity processes the input data in some way >>> before passing it on to the output pads. But for a connector the input and >>> output pads are just hooked up to input and output pins or buses and not >>> to one another. >>> >>> Finally we would like to get all this information atomically in userspace. >>> Atomicity is desired since some of this information well be dynamic. >>> Currently the only dynamic thing is enabling/disabling links, but that is >>> too limiting for devices like FPGAs where the entities can change on the >>> fly as well. >>> >>> Being able to get all the information with a single ioctl will simplify >>> implementing this atomic behavior, and it is also more efficient than >>> calling lots of ENUM ioctls. >>> >>> We decided to keep the existing structs and ioctls and introduce new >>> versions of these structs redesigned to cope with the new insights. The >>> sizes of the reserved fields are suggestions only. >>> >>> We had a discussion about the object IDs. Currently we have only entity IDs, >>> but in the redesign we'll have interface, connector and pad IDs as well. >> >> As connectors are entities I don't think they deserve a special mention >> here. > > Hans proposal is to use a different type for connectors. So, I guess it > makes sense to keep the mention. There is a difference in how connector entities behave (e.g. input pads are not connected to output pads, instead they represent pins on the connector) and how connector entities are used (V4L2 drivers might want to enumerate connector entities to implement VIDIOC_ENUMINPUT). So giving them their own distinct type makes sense to me. >> Regarding pad IDs, is there a reason to give pads IDs in the global ID space >> other than reusing media_link ? > > I actually have another usage for that ;) > > I'll comment that below. > >> >>> The idea from the meeting was to use the least significant X bits of the ID >>> to tell the difference of entity, interface and pad types and to use a flag >>> for marking connector entities. After thinking some more I would suggest >>> this scheme instead: >>> >>> #define MEDIA_ID_T_ENTITY 0 >>> #define MEDIA_ID_T_CONNECTOR1 >>> #define MEDIA_ID_T_INTERFACE2 >>> #define MEDIA_ID_T_PAD 3 >
[PATCH] [media] DocBook media: Fix typo "the the" in xml files
This patch fix spelling typo "the the" found in controls.xml and vidioc-g-param.xml. These xml files are generated from NOT any files, so I have to fix these xml files. Signed-off-by: Masanari Iida --- Documentation/DocBook/media/v4l/controls.xml | 2 +- Documentation/DocBook/media/v4l/vidioc-g-parm.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 6e1667b..33aece5 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -3414,7 +3414,7 @@ giving priority to the center of the metered area. V4L2_EXPOSURE_METERING_MATRIX A multi-zone metering. The light intensity is measured -in several points of the frame and the the results are combined. The +in several points of the frame and the results are combined. The algorithm of the zones selection and their significance in calculating the final value is device dependent. diff --git a/Documentation/DocBook/media/v4l/vidioc-g-parm.xml b/Documentation/DocBook/media/v4l/vidioc-g-parm.xml index f4e28e7..7217287 100644 --- a/Documentation/DocBook/media/v4l/vidioc-g-parm.xml +++ b/Documentation/DocBook/media/v4l/vidioc-g-parm.xml @@ -267,7 +267,7 @@ is intended for still imaging applications. The idea is to get the best possible image quality that the hardware can deliver. It is not defined how the driver writer may achieve that; it will depend on the hardware and the ingenuity of the driver writer. High quality mode is -a different mode from the the regular motion video capture modes. In +a different mode from the regular motion video capture modes. In high quality mode: The driver may be able to capture higher -- 2.5.0.234.gefc8a62 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] [media] m2m: fix bad unlock balance
Hello,
On 2015-08-17 12:13, Zahari Doychev wrote:
This patch removes unnecessary mutex queue unlock/lock sequence causing bad
unlock balance in v4l2_m2m_poll when the last buffer on the destination
queue has been dequeued and adds spin lock protection for the done list
list_empty calls.
[ 144.990873] =
[ 144.995584] [ BUG: bad unlock balance detected! ]
[ 145.000301] 4.1.0-00137-ga105070 #98 Tainted: GW
[ 145.006140] -
[ 145.010851] demux:sink/487 is trying to release lock (&dev->dev_mutex) at:
[ 145.017785] [<808cc578>] mutex_unlock+0x18/0x1c
[ 145.022322] but there are no more locks to release!
[ 145.027205]
[ 145.027205] other info that might help us debug this:
[ 145.033741] no locks held by demux:sink/487.
[ 145.038015]
[ 145.038015] stack backtrace:
[ 145.042385] CPU: 2 PID: 487 Comm: demux:sink Tainted: GW
4.1.0-00137-ga105070 #98
[ 145.051089] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[ 145.057622] Backtrace:
[ 145.060102] [<80014a4c>] (dump_backtrace) from [<80014cc4>]
(show_stack+0x20/0x24)
[ 145.067679] r6:80cedf78 r5: r4: r3:
[ 145.073421] [<80014ca4>] (show_stack) from [<808c61e0>]
(dump_stack+0x8c/0xa4)
[ 145.080661] [<808c6154>] (dump_stack) from [<80072b64>]
(print_unlock_imbalance_bug+0xb8/0xe8)
[ 145.089277] r6:808cc578 r5:ac6cd050 r4:ac38e400 r3:0001
[ 145.095020] [<80072aac>] (print_unlock_imbalance_bug) from [<80077db4>]
(lock_release+0x1a4/0x250)
[ 145.103983] r6:808cc578 r5:ac6cd050 r4:ac38e400 r3:
[ 145.109728] [<80077c10>] (lock_release) from [<808cc470>]
(__mutex_unlock_slowpath+0xc4/0x1b4)
[ 145.118344] r9:acb27a41 r8: r7:81553814 r6:808cc578 r5:60030013
r4:ac6cd01c
[ 145.126190] [<808cc3ac>] (__mutex_unlock_slowpath) from [<808cc578>]
(mutex_unlock+0x18/0x1c)
[ 145.134720] r7: r6:aced7cd4 r5:0041 r4:acb87800
[ 145.140468] [<808cc560>] (mutex_unlock) from [<805a98b8>]
(v4l2_m2m_fop_poll+0x5c/0x64)
[ 145.148494] [<805a985c>] (v4l2_m2m_fop_poll) from [<805955a0>]
(v4l2_poll+0x6c/0xa0)
[ 145.156243] r6:aced7bec r5: r4:ac6cc380 r3:805a985c
[ 145.161991] [<80595534>] (v4l2_poll) from [<80156edc>]
(do_sys_poll+0x230/0x4c0)
[ 145.169391] r5: r4:aced7be4
[ 145.173013] [<80156cac>] (do_sys_poll) from [<801574a8>]
(SyS_ppoll+0x1d4/0x1fc)
[ 145.180414] r10: r9:aced6000 r8: r7: r6:75c04538
r5:0002
[ 145.188338] r4:
[ 145.190906] [<801572d4>] (SyS_ppoll) from [<800108c0>]
(ret_fast_syscall+0x0/0x54)
[ 145.198481] r8:80010aa4 r7:0150 r6:75c04538 r5:0002 r4:0008
Signed-off-by: Zahari Doychev
Acked-by: Marek Szyprowski
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index ec3ad4e..2f7291c 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -583,32 +583,25 @@ unsigned int v4l2_m2m_poll(struct file *file, struct
v4l2_m2m_ctx *m2m_ctx,
goto end;
}
- if (m2m_ctx->m2m_dev->m2m_ops->unlock)
- m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
- else if (m2m_ctx->q_lock)
- mutex_unlock(m2m_ctx->q_lock);
-
+ spin_lock_irqsave(&src_q->done_lock, flags);
if (list_empty(&src_q->done_list))
poll_wait(file, &src_q->done_wq, wait);
+ spin_unlock_irqrestore(&src_q->done_lock, flags);
+
+ spin_lock_irqsave(&dst_q->done_lock, flags);
if (list_empty(&dst_q->done_list)) {
/*
* If the last buffer was dequeued from the capture queue,
* return immediately. DQBUF will return -EPIPE.
*/
- if (dst_q->last_buffer_dequeued)
+ if (dst_q->last_buffer_dequeued) {
+ spin_unlock_irqrestore(&dst_q->done_lock, flags);
return rc | POLLIN | POLLRDNORM;
+ }
poll_wait(file, &dst_q->done_wq, wait);
}
-
- if (m2m_ctx->m2m_dev->m2m_ops->lock)
- m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
- else if (m2m_ctx->q_lock) {
- if (mutex_lock_interruptible(m2m_ctx->q_lock)) {
- rc |= POLLERR;
- goto end;
- }
- }
+ spin_unlock_irqrestore(&dst_q->done_lock, flags);
spin_lock_irqsave(&src_q->done_lock, flags);
if (!list_empty(&src_q->done_list))
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Media Controller, the next generation
Em Mon, 17 Aug 2015 12:33:30 +0200 Hans Verkuil escreveu: > On 08/16/2015 03:37 PM, Mauro Carvalho Chehab wrote: > > Em Sat, 15 Aug 2015 21:25:40 +0300 > > Laurent Pinchart escreveu: > > > >> Hi Hans, > >> > >> Thank you for the RFC. I believe it's quite good (at least to my eyes). > >> Everything is of course not perfect yet so please see below for some > >> comments. > > > > (added c/c linux-...@vger.kernel.org) > > > >> > >> On Monday 03 August 2015 16:07:48 Hans Verkuil wrote: > >>> Hi all, > >>> > >>> During last week's brainstorm meeting in Espoo, Finland, we discussed > >>> how to proceed with the MC API. Trying to apply the existing API to DVB > >>> devices caused a lot of controversy and this meeting was an attempt to > >>> resolve these issues. > >>> > >>> This RFC is the proposal for the public API (NOT the internal API!) we > >>> came up with. > >>> > >>> The main change is that interfaces (such as devices in /dev) get their own > >>> object: struct media_interface. The struct media_entity as is used today > >>> will not refer to interfaces anymore. > >>> > >>> Since interfaces control entities we also want to tell userspace which > >>> entities are controlled by which interfaces. We want to keep things simple > >>> and avoid having to create a new link structure just for this, > > > > I agree with the Hans proposal. Having a single object to represent graph > > links makes easier for both Kernelspace and Userspace to handle them. > > Right. > > >> During the meeting we discussed the possibility of creating a > >> media_interface_link structure (as drafted in > >> http://linuxtv.org/downloads/presentations/mc_ws_2015/media.h). What would > >> be > >> the advantage of reusing media_link ? > > > > I would reverse the question: what's the disadvantage? > > > > By using the same type, it should be easier to change userspace programs > > to support both interface and data links. > > > > With a single struct, if/when apps need to do graph traversal, they can > > use a single algorithm. It also means a single code to allocate and > > fetch the structs that come from the Kernel. > > Also, since all graph objects (interfaces, entities, pads) all use the same > base struct (media_graph_obj) it makes sense that we use the same media_link > struct for linking two graph objects. > > >>> so instead a struct media_link just provides two object IDs and a flags > >>> field. The object IDs refer to pads (for data links) or entity/interface > >>> IDs > >>> (for associating interfaces with entities). > >>> > >>> To make this work we need unique pad IDs. > >>> > >>> We will need to represent connectors as well. The media_entity struct is > >>> used for that, but it will be marked as a connector since connectors work > >>> slightly different from an entity: if an entity has both input and output > >>> pads, then that means that the entity processes the input data in some way > >>> before passing it on to the output pads. But for a connector the input and > >>> output pads are just hooked up to input and output pins or buses and not > >>> to one another. > >>> > >>> Finally we would like to get all this information atomically in userspace. > >>> Atomicity is desired since some of this information well be dynamic. > >>> Currently the only dynamic thing is enabling/disabling links, but that is > >>> too limiting for devices like FPGAs where the entities can change on the > >>> fly as well. > >>> > >>> Being able to get all the information with a single ioctl will simplify > >>> implementing this atomic behavior, and it is also more efficient than > >>> calling lots of ENUM ioctls. > >>> > >>> We decided to keep the existing structs and ioctls and introduce new > >>> versions of these structs redesigned to cope with the new insights. The > >>> sizes of the reserved fields are suggestions only. > >>> > >>> We had a discussion about the object IDs. Currently we have only entity > >>> IDs, > >>> but in the redesign we'll have interface, connector and pad IDs as well. > >> > >> As connectors are entities I don't think they deserve a special mention > >> here. > > > > Hans proposal is to use a different type for connectors. So, I guess it > > makes sense to keep the mention. > > There is a difference in how connector entities behave (e.g. input pads are > not connected to output pads, instead they represent pins on the connector) > and > how connector entities are used (V4L2 drivers might want to enumerate > connector > entities to implement VIDIOC_ENUMINPUT). > > So giving them their own distinct type makes sense to me. > > >> Regarding pad IDs, is there a reason to give pads IDs in the global ID > >> space > >> other than reusing media_link ? > > > > I actually have another usage for that ;) > > > > I'll comment that below. > > > >> > >>> The idea from the meeting was to use the least significant X bits of the > >>> ID > >>> to tell the difference of entity, interface and pad types and t
Re: [RFC] Media Controller, the next generation
On 08/17/2015 02:39 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 17 Aug 2015 12:33:30 +0200
> Hans Verkuil escreveu:
>> That's not what I meant. I meant using the mc_ prefix instead of the media_
>> prefix for internal structs. So media_entity in the public API would map to
>> mc_entity in the internal API. Ditto for media_interface/mc_interface,
>> media_pad/mc_pad, etc.
>
> Ah! that could work. Yet, I guess we decided to not use mc_ internally
> in the first place several years ago because there was already some
> namespace conflict.
>
> Ah, yes:
>
> $ git grep -e \\bmc_
> ...
> arch/powerpc/kernel/mce.c: struct machine_check_event *mc_evt;
> arch/powerpc/kernel/mce.c: mc_evt =
> this_cpu_ptr(&mce_event[index]);
> arch/powerpc/kernel/mce.c: *mce = *mc_evt;
> ...
> arch/um/drivers/line.h: struct mc_device mc;
> ...
> arch/um/drivers/net_kern.c:static struct mc_device net_mc = {
> ...
>
> A total of 302 references... Among them, some arch-dependent stuff,
> including x86 and powerPC error hanlding stuff (MCE, APEI).
>
> I would avoid it, as the risk of namespace collisions are high.
OK.
>
>> Using graph_ instead of mc_ is something I would be OK with as well.
What about graph_ as the internal prefix? Or mctl_?
>> I am a bit worried about this. To my knowledge applications that use the MC
>> today are expected to know which pad of an entity does what, and it
>> identifies
>> the pad by index.
>>
>> The new public API should still provide applications with this information in
>> one way or another. The pad ID won't work, certainly not in the dynamic case,
>> the PAD_TYPE as suggested here will only work as long as there is only one
>> pad per type. Suppose there is an entity with two output pads, both for
>> video?
>> One might be SDTV, one HDTV. How to tell the difference?
>
> The first one will have "SDTV" as type, the other one "HDTV". So, no problem
> with that.
So now we have SDTV and HDTV, the next entity will have YUV_420 and YUV_422
pads,
or whatever. The number of types will quickly escalate. It's similar to the
'entity type' problem (e.g. sensor, flash, video encoder, scaler, etc.).
>
> There are, however, some usages where an index is enough. For example, the
> hardware demux filter outputs can fully be identified by just an index, and
> each index could (ideally) be mapped into a different DMA.
>
> So, I guess we need to offer the flexibility to keep using indexes where
> it makes sense.
>
> That's why I proposed a find_pad function that would allow to use an
> index:
> media_pad *find_pad(struct media_entity entity, enum pad_usage usage,
> int idx);
> or
> media_pad *find_pad(struct media_entity entity, char *pad_name);
>
>> One option might be to have a pad_type or data_type for basic type
>> information
>> to have generic code that just wants to find VBI/VIDEO/AUDIO streaming pads,
>> and a name[32] field that is uniquely identifying the pad and that can be
>> used
>> in userspace applications (or drivers for that matter by adding a
>> find_pad_name()).
>
> just a name could be enough:
> vbi_pad = find_pad(entity, "vbi");
>
> However, that would require to have a strict namespace if we want the core
> to rely on it.
I think names would work: we can make a number of 'core' names that should be
used where appropriate, and leave it up to the driver otherwise.
So if there is only one video source pad, then call the pad "video". If there
are multiple video source pads, then zero or one pads can be called "video",
the others should get different names "video-sdtv").
To be honest, I'm not terribly keen on this. I think a data/pad type + name
would work better.
>>> };
>>>
>>> And have the structs with (some) properties embed on it, like:
>>>
>>> struct media_graph_entity {
>>> struct media_graph_object obj;
>>
>> This doesn't make sense: struct media_graph_topology would fill in an
>> array of struct media_graph_object would point to media objects that
>> in turn contain a struct media_graph_object.
>
> Sorry, I didn't get your idea. Could you please give a C code example on
> what you're thinking?
>
>>
>> I wouldn't embed a struct media_graph_object in these structs, there
>> is no point to that. Thinking about this some more you don't need the
>> length field either, only an id (this gives you the type, so you know
>> whether the object_data pointer is for a media_entity or a media_pad
>> or whatever) and the pointer. Strictly speaking you would only need
>> the 'type' part of the ID, but I see no reason not to fill in the full
>> ID.
>>
>>> u32 flags;
>>> /* ... */
>>> char name[64];
>>> };
>>>
>>> Please notice that we don't need to add reserved fields at the structs,
>>> as we're now putting the struct length at the media_graph_object.
>>>
>>> So, if we need to, for example, add a new "foo" inside the
>>> media_graph_entity:
>>>
>>> struct media_graph_entity {
>>> struct media_graph_objec
Re: [PATCH v2 05/11] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.
Em Thu, 30 Jul 2015 18:08:55 +0100
Peter Griffin escreveu:
> This patch adds in the required DT node for the c8sectpfe
> Linux DVB demux driver which allows the tsin channels
> to be used on an upstream kernel.
>
> Signed-off-by: Peter Griffin
> ---
> arch/arm/boot/dts/stihxxx-b2120.dtsi | 38
>
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> index 62994ae..1bc018e 100644
> --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> @@ -6,6 +6,10 @@
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> */
> +
> +#include
> +#include
> +
> / {
> soc {
> sbc_serial0: serial@953 {
> @@ -85,5 +89,39 @@
> status = "okay";
> };
>
> + c8sectpfe@08a2 {
> + compatible = "st,stih407-c8sectpfe";
> + status = "okay";
> + reg = <0x08a2 0x1>, <0x08a0 0x4000>;
> + reg-names = "c8sectpfe", "c8sectpfe-ram";
> +
> + interrupts = <0 34 0>, <0 35 0>;
> + interrupt-names = "c8sectpfe-error-irq",
> + "c8sectpfe-idle-irq";
> +
> + pinctrl-names = "tsin0-serial", "tsin0-parallel",
> + "tsin3-serial", "tsin4-serial",
> + "tsin5-serial";
> +
> + pinctrl-0 = <&pinctrl_tsin0_serial>;
> + pinctrl-1 = <&pinctrl_tsin0_parallel>;
> + pinctrl-2 = <&pinctrl_tsin3_serial>;
> + pinctrl-3 = <&pinctrl_tsin4_serial_alt3>;
> + pinctrl-4 = <&pinctrl_tsin5_serial_alt1>;
> +
> + clocks = <&clk_s_c0_flexgen CLK_PROC_STFE>;
> + clock-names = "c8sectpfe";
> +
> + /* tsin0 is TSA on NIMA */
> + tsin0: port@0 {
> +
> + tsin-num = <0>;
> + serial-not-parallel;
> + i2c-bus = <&ssc2>;
There's no ssc2 defined at the device tree.
I'll revert this patch and mark the driver as broken until this gets
fixed.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Media Controller, the next generation
Em Mon, 17 Aug 2015 15:21:22 +0200
Hans Verkuil escreveu:
> On 08/17/2015 02:39 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 17 Aug 2015 12:33:30 +0200
> > Hans Verkuil escreveu:
> >> That's not what I meant. I meant using the mc_ prefix instead of the media_
> >> prefix for internal structs. So media_entity in the public API would map to
> >> mc_entity in the internal API. Ditto for media_interface/mc_interface,
> >> media_pad/mc_pad, etc.
> >
> > Ah! that could work. Yet, I guess we decided to not use mc_ internally
> > in the first place several years ago because there was already some
> > namespace conflict.
> >
> > Ah, yes:
> >
> > $ git grep -e \\bmc_
> > ...
> > arch/powerpc/kernel/mce.c: struct machine_check_event *mc_evt;
> > arch/powerpc/kernel/mce.c: mc_evt =
> > this_cpu_ptr(&mce_event[index]);
> > arch/powerpc/kernel/mce.c: *mce = *mc_evt;
> > ...
> > arch/um/drivers/line.h: struct mc_device mc;
> > ...
> > arch/um/drivers/net_kern.c:static struct mc_device net_mc = {
> > ...
> >
> > A total of 302 references... Among them, some arch-dependent stuff,
> > including x86 and powerPC error hanlding stuff (MCE, APEI).
> >
> > I would avoid it, as the risk of namespace collisions are high.
>
> OK.
>
> >
> >> Using graph_ instead of mc_ is something I would be OK with as well.
>
> What about graph_ as the internal prefix? Or mctl_?
>
> >> I am a bit worried about this. To my knowledge applications that use the MC
> >> today are expected to know which pad of an entity does what, and it
> >> identifies
> >> the pad by index.
> >>
> >> The new public API should still provide applications with this information
> >> in
> >> one way or another. The pad ID won't work, certainly not in the dynamic
> >> case,
> >> the PAD_TYPE as suggested here will only work as long as there is only one
> >> pad per type. Suppose there is an entity with two output pads, both for
> >> video?
> >> One might be SDTV, one HDTV. How to tell the difference?
> >
> > The first one will have "SDTV" as type, the other one "HDTV". So, no problem
> > with that.
>
> So now we have SDTV and HDTV, the next entity will have YUV_420 and YUV_422
> pads,
> or whatever. The number of types will quickly escalate. It's similar to the
> 'entity type' problem (e.g. sensor, flash, video encoder, scaler, etc.).
well, as new pads will require new Kernel patches, nothing prevents to add
a new #define. A string would also work.
> >
> > There are, however, some usages where an index is enough. For example, the
> > hardware demux filter outputs can fully be identified by just an index, and
> > each index could (ideally) be mapped into a different DMA.
> >
> > So, I guess we need to offer the flexibility to keep using indexes where
> > it makes sense.
> >
> > That's why I proposed a find_pad function that would allow to use an
> > index:
> > media_pad *find_pad(struct media_entity entity, enum pad_usage usage,
> > int idx);
> > or
> > media_pad *find_pad(struct media_entity entity, char *pad_name);
> >
> >> One option might be to have a pad_type or data_type for basic type
> >> information
> >> to have generic code that just wants to find VBI/VIDEO/AUDIO streaming
> >> pads,
> >> and a name[32] field that is uniquely identifying the pad and that can be
> >> used
> >> in userspace applications (or drivers for that matter by adding a
> >> find_pad_name()).
> >
> > just a name could be enough:
> > vbi_pad = find_pad(entity, "vbi");
> >
> > However, that would require to have a strict namespace if we want the core
> > to rely on it.
>
> I think names would work: we can make a number of 'core' names that should be
> used where appropriate, and leave it up to the driver otherwise.
OK.
> So if there is only one video source pad, then call the pad "video". If there
> are multiple video source pads, then zero or one pads can be called "video",
> the others should get different names "video-sdtv").
>
> To be honest, I'm not terribly keen on this. I think a data/pad type + name
> would work better.
Me too.
> >>> };
> >>>
> >>> And have the structs with (some) properties embed on it, like:
> >>>
> >>> struct media_graph_entity {
> >>> struct media_graph_object obj;
> >>
> >> This doesn't make sense: struct media_graph_topology would fill in an
> >> array of struct media_graph_object would point to media objects that
> >> in turn contain a struct media_graph_object.
> >
> > Sorry, I didn't get your idea. Could you please give a C code example on
> > what you're thinking?
> >
> >>
> >> I wouldn't embed a struct media_graph_object in these structs, there
> >> is no point to that. Thinking about this some more you don't need the
> >> length field either, only an id (this gives you the type, so you know
> >> whether the object_data pointer is for a media_entity or a media_pad
> >> or whatever) and the pointer. Strictly speaking you would only need
> >> the 'type' part of the ID,
[ANNOUNCE] Report for the Media Controller Workshop - Espoo - Aug, 17 2015
Report of the Media Controller workshop in Espoo, Finland – July, 29-31 2015 PS.: For those that prefer, a web version is at: http://linuxtv.org/news.php?entry=2015-08-17.mchehab This is the first workshop dedicated to the Linux Media Controller. We had a v4l summit back in 2010, also in Finland, that established the current foundation for the media controller, andto properly satisfy the needs of properly reporting the pipelines on the smartphone System on a Chip (SoC). The focus of this year's workshop was to clarify the kernel→userspace interfaces and extend the Media Controller to be used on other subsystems that need to represent graphs, like Digital Video Broadcasting (DVB), Advanced Linux Sound Architecture (ALSA), and Industrial I/O (IIO). Main event attendees: Antti Laakso Hans Verkuil Lars-Peter Clausen Laurent Pinchart Mauro Chehab Sakari Ailus Shuah Khan Tuukka Toivonen MC videoconf attendees: Dong-Joo Kim Geunyoung Kim In-Ki Dae Junghak Sung Minsong Kim Ohin Kwon Seung-Woo Kim EDITOR'S NOTE: Usually, we produce the summit reports using a cronologic approach. This time, I opted to move the content of the second day to the beginning of the report, as they're basically several presentations we had covering the Media Controller needs. I opted to put the presentations first, and then put the discussions we had on July 29 and 31 together. 1) Presentations made on July, 30 1.1. Videobuf2 redesign to allow it to support DVB Representatives from Samsung offered a presentation via video conference about the scheduled changes that are planned in order to split the videobuf2 core from the V4L2 specific part. This will allow the videobuf2 core to also be for DVB. 1.2. DVB pipelines Samsung representatives via video conference presented about the pipelines. Essencially, the pipelines on real use-cases for embedded devices would look like: tuner 1 -> -> demux-> Video tuner 2 -> cam 1 -> demux-> Audio tuner 3 -> cam 2 -> demux-> Data -> V4L2 -> ALSA 1.3. ASoC http://linuxtv.org/downloads/presentations/mc_ws_2015/sound_media_controller.pdf This presentation covered the following topics: Generic DT machine driver can support around half of DT based devices Not all required is data available in ACPI, meaning ACPI based devices typically have their own drivers. DAPM handles power management in ASoC much like the current omap3isp driver implementation does. Walks the graph ALSA supports large numbers of entities and cyclical graphs ALSA contains ASoC dependencies, but the core of the algorithm is generic A stack based, iterative version is in the works MC has generic graph walking code as well Iterative Less concerned with performance omap3isp driver has PM code, which could be more generic Ideally we should have single implementation for graph walking and for power management 1.4 Updates to ALSA and au0828 to share resources: http://linuxtv.org/downloads/presentations/mc_ws_2015/alsa_media_controller.pdf 1. The au8522 pad that sends data to the ALSA entity is currently represented as state->pads[AU8522_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SINK. It should be, instead MEDIA_PAD_FL_SOURCE, as it provides data to the ALSA PCM capture device node. 2. ALSA PCM operates in atomic context by default holding a spinlock. This atomic locking conflicts with graph_mutex hold from Media Controller API from ALSA and au0828 drivers. However, there are concerns about I2C drivers breaking if graph_mutex is changed to a spinlock. Lars suggested, instead, to use ALSA in non-atomic context: Not a good idea to change graph_mutex to spinlock at MC as this will badly impact calls from I2C drivers, causing them to break. Using ALSA in non-atomic is simple: just initialize pcm->substream->nonatomic = 1 IRQ safe start/stop pipeline will not be necessary anymore. This will reduce the work and patch series v6 becomes smaller. 3. An enable_source() function is necessary at media_device level, but find a better name 4. A Write helper function to hide decoder and tuner details from ALSA should be written 5. Support for remove links are needed - Shuah can do that as part of ALSA work, after adding support for it at the core 6. Managed Media Controller API uses devres - change it to get reference and put reference (kref), as it isn't safe to delete media_device device resource. Shuah will create a media_device_get() and media_device_put() set of functions. In addition, deleting media device Managed or otherwise is a problem. This problem isn't just a Managed Media Device issue. 1.5 IIO - Industrial I/O ht
Re: terratec HTC XS HD USB
Robert N gmail.com> writes: > > I will reply my own question. it seems that w_scan is able to find the > muxes/services if I push the antenna cable only half way into the > tuner stick. I assume my problem is related to RF signal strength. is > there a range of valid strengths? > > On Fri, Nov 21, 2014 at 5:13 PM, Robert N gmail.com> wrote: > > Hi, > > > > I'm trying to get my USB tuner stick working on an openwrt, but > > getting some errors. > > > > using w_scan to scan the available channels, gives: > > > > 113000: sr6900 (time: 00:11) (time: 00:12) signal ok: > > QAM_64 f = 113000 kHz S6900C999 > > start_filter:1410: ERROR: ioctl DMX_SET_FILTER failed: 97 Message too long > > Info: NIT(actual) filter timeout > > > > I know the 113Mhz is a valid MUX, because tuner works well under windows. > > > > Any hints what could be the reason of the error messages? > > > > Thanks. > Hi Robert, I am trying to do achieve the exact same thing as you. My hardware: TP-Link Archer C5 with OpenWRT 14.07 Barrier Breaker Terratec HTC XS HD USB (http://www.terratec.net/details.php?artnr=Cinergy+HTC+USB+XS+HD) I built OpenWRT from source which worked fine. But now after inserting the kernel modules for the TV stick and the firmware, I can't get tvheadend to perform a scan and w_scan to find anything. First it threw the same error as you encountered (that's how I found you). Thanks to your trick the error is gone, but still no signal, it recognizes a valid mux at 121Mhz, but no channels. My dmesg looks good, just as on the Ubuntu machine where the stick works. I don't know the specifications (RF signal strength) of the stick, but it seems that it is a little bit vulnerable to signal errors. On the Ubuntu machine it sometimes has stuttering video for me. Have you found a solution to make the stick work? If you could share it that would be really great. Best Regards, BadTenMan -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Aug 18 04:00:18 CEST 2015 git branch: test git hash: 3a6b0605c73d1d695f6d4e49289deaa3fa3e73ee gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-51-ga53cea2 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-rc1-i686: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-rc1-x86_64: OK apps: OK spec-git: OK sparse: ERRORS smatch: ERRORS 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/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: VIMC: API proposal, configuring the topology through user space
Hi Hans, On Friday 14 August 2015 12:54:44 Hans Verkuil wrote: > On 08/13/2015 07:29 PM, Laurent Pinchart wrote: > > On Tuesday 11 August 2015 17:07:04 Helen Fornazier wrote: > >> On Tue, Aug 11, 2015 at 6:36 AM, Mauro Carvalho Chehab wrote: > >>> Hans Verkuil escreveu: > On 08/10/15 19:21, Helen Fornazier wrote: > > On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil wrote: > >> On 08/08/2015 03:55 AM, Helen Fornazier wrote: > >>> Hi! > >>> > >>> I've made a first sketch about the API of the vimc driver (virtual > >>> media controller) to configure the topology through user space. > >>> As first suggested by Laurent Pinchart, it is based on configfs. > >>> > >>> I would like to know you opinion about it, if you have any > >>> suggestion to improve it, otherwise I'll start its implementation as > >>> soon as possible. This API may change with the MC changes and I may > >>> see other possible configurations as I implementing it but here is > >>> the first idea of how the API will look like. > >>> > >>> vimc project link: https://github.com/helen-fornazier/opw-staging/ > >>> For more information: http://kernelnewbies.org/LaurentPinchart > >>> > >>> /*** > >>> The API: > >>> / > >>> > >>> In short, a topology like this one: http://goo.gl/Y7eUfu > >>> Would look like this filesystem tree: https://goo.gl/tCZPTg > >>> Txt version of the filesystem tree: https://goo.gl/42KX8Y > >>> > >>> * The /configfs/vimc subsystem > >> > >> I haven't checked the latest vimc driver, but doesn't it support > >> multiple instances, just like vivid? I would certainly recommend > >> that. > > > > Yes, it does support > > > >> And if there are multiple instances, then each instance gets its own > >> entry in configfs: /configs/vimc0, /configs/vimc1, etc. > > > > You are right, I'll add those > > > >>> The vimc driver registers a subsystem in the configfs with the > >>> following contents: > >>> > ls /configfs/vimc > >>> build_topology status > >>> > >>> The build_topology attribute file will be used to tell the driver > >>> the configuration is done and it can build the topology internally > >>> > >>> > echo -n "anything here" > /configfs/vimc/build_topology > >>> > >>> Reading from the status attribute can have 3 different classes of > >>> outputs > >>> 1) deployed: the current configured tree is built > >>> 2) undeployed: no errors, the user has modified the configfs tree > >>> thus the topology was undeployed > >>> 3) error error_message: the topology configuration is wrong > >> > >> I don't see the status file in the filesystem tree links above. > > > > Sorry, I forgot to add > > > >> I actually wonder if we can't use build_topology for that: reading it > >> will just return the status. > > > > Yes we can, I was just wondering what should be the name of the file, > > as getting the status from reading the build_topology file doesn't > > seem much intuitive > > I'm not opposed to a status file, but it would be good to know what > Laurent thinks. > > >> What happens if it is deployed and you want to 'undeploy' it? Instead > >> of writing anything to build_topology it might be useful to write a > >> real command to it. E.g. 'deploy', 'destroy'. > >> > >> What happens when you make changes while a topology is deployed? > >> Should such changes be rejected until the usecount of the driver goes > >> to 0, or will it only be rejected when you try to deploy the new > >> configuration? > > > > I was thinking that if the user try changing the topology, or it will > > fail (because the device instance is in use) or it will undeploy the > > old topology (if the device is not in use). Then a 'destroy' command > > won't be useful, the user can just unload the driver when it won't be > > used anymore, > >>> > >>> Well, we're planning to add support for dynamic addition/removal of > >>> entities, interfaces, pads and links. So, instead of undeploy the > >>> > >>> old topology, one could just do: > >>> rm -rf > >> > >> I think I misunderstood the word dynamic here. Do you mean that > >> entities/interfaces/pads/link could be created/removed while their > >> file handlers are opened? While an instance (lets say vimc2) is being > >> used? > > > > Let's keep in mind that what can appear or disappear at runtime in an > > uncontrolled manner are hardware blocks. The associated media_entity > > instances will of course need to be created and destroyed, but we have > > control over that in the sense that the kernel controls the lifetime of > > those structures. > > > > For the vimc driver hardware addition and removal
