| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When |
|---|---|---|
| mchehab | that will help to check if the new code won't break anything | [20:38] |
| larsc | tnt: you wrote that you don't need any hardware modifications with your latest core. But the core still needs separate inputs for data and for lp detection, how does that work? | [09:12] |
| ................. (idle for 1h23mn) | ||
| tnt | larsc: there is a 'lp bypass' config register.
larsc: so if you want the hw lp detection to be 'cleaner' you can. But if you don't have it, it can work without it. | [10:35] |
| larsc | tnt: ok
I'm going to try to get the core up and running on a zed board | [10:39] |
| tnt | larsc: nice. looking fwd to the results :)
larsc: I originally got it up using just awg30 soldered to the camera test point, connected to a pin header and then ribbon cable to a breakout board ... so it's pretty tolerant. | [10:42] |
| larsc: are you using a rpi cam too or some other CSI source ? | [10:49] | |
| larsc | a different one
adv7281m | [10:50] |
| tnt | I synthesized the core with constraints up to 250 MHz, but I never tried actually capturing data at more than 425 Mbps.
Oh, I'd also point out I only ever tested the 2-lane mode :p It "should" work when configured for 1-lane but ymmv :p | [10:52] |
| larsc | I only have one lane
so that shouldn't be a problem | [10:56] |
| .... (idle for 15mn) | ||
| tnt | yeah, that's what I meant. The rpi cam has 2 lanes so that's th eonly mode I tried. I wrote the core with a generic to selec the # of lanes but that's untested. | [11:11] |
| .... (idle for 18mn) | ||
| larsc: also, you might want to add 0x1E at line 183 of mipi_csi_pkt_parse.vhd (just copy the 0x1A case). This is to add YUV422 as a recognized packet type. | [11:29] | |
| larsc | k | [11:39] |
| mchehab | pinchartl: as requested, I sent an initial set of incremental patches
please review it as quick as possible, as I want to minimize rework I have a lot more code to backport as incremental patches | [11:41] |
| pinchartl | mchehab: thank you. I should have time over the weekend | [11:45] |
| mchehab | pinchartl: I want to have the entire patchset written this week...
if you postpone to weekend, you may have a really big pile of patches so, it would be really nice for both you and me if you could try to take a look on it earlier the changes so far are small and shouldn't break anything as it is just code additions, plus one small patch changing things at DVB side | [11:45] |
| pinchartl | I'm doing the best I can | [11:48] |
| mchehab | basically, calling media_entity_create() instead of kzalloc() and media_entity_remove() instead of kfree() | [11:49] |
| pinchartl | the 3 days of meeting last week got me late on other projects, I need to catch up | [11:49] |
| ............ (idle for 58mn) | ||
| larsc | I don't think kref'ing the entities makes too much sense, typically you hold a reference to the whole graph not just a single element | [12:47] |
| mchehab | larsc: my plan is to have krefs for all objects
this way, part of the graph can be removed on a safe way preventing freeing a memory that may be in usage assume, for example, a link between two entities if the entity is freed before the link, and some code tries to access the entity via the link on some race condition, we'll have a problem | [12:55] |
| larsc | but who would be holding the reference to the link? | [13:01] |
| mchehab | larsc: I'm adding routines to create/remove links too | [13:02] |
| larsc | yes, but who would be holding the reference to the link?
where do you get and put the link? | [13:02] |
| mchehab | larsc: all objects will be at a list of objects
it won't be inside the entity anymore that's my plan all objects will be inside the media_device struct | [13:04] |
| larsc | but when will you acquire and drop references to a object? | [13:05] |
| mchehab | when the object is created, it will acquire references
so, for example, when a link is set between two pads... it will increase pads refcounts when the link is destroyed, it will decrease on a real case: entity_a = media_entity_create() entity_b = media_entity_create() pad_a1 = media_pad_create() pad_b1 = media_pad_create() link_1 = media_link_create(pad_a1, pad_b1) something like the above so, if an entity or a pad is removed before the link, the actual data will still be there... until every object using it would decrease refcount | [13:05] |
| larsc | and if the link is removed before the pads? | [13:08] |
| mchehab | it will decrement its own refcount, and the two pads refcount...
but as pad_create() incremented the refcounts... the pads won't be removed until a media_pad_remove() would be called so you can dynamically remove a link and create another one | [13:09] |
| larsc | ok, I see your point, but I'm not convinced we really need this | [13:14] |
| mchehab | larsc: this is the easiest way to solve it, and also solves the problems with devm_*
plus, it uses a standard kernel way to defer memory free that is known to work we might use some other solution, but it would be non-standard and will likely take more time to develop and test and would make harder for Kernel janitors to work with with regards to the need of dynamically remove/create links, this is needed on devices that have multiple USB interfaces like em28xx and au0828 where ALSA will need to dynamically create links at the MC device created by the V4L2 driver... and will need to dynamically remove them, if the ALSA driver is unbound | [13:26] |
| larsc | I understand that | [13:32] |
| mchehab | (either by removing the driver or by using the sysfs node that unbinds it) | [13:32] |
| larsc | I'm just not conviced that it makes sense to keep a link around after the enity it refers to has been removed | [13:32] |
| mchehab | larsc: well, some code may be navigating at the link while some other part is removing it | [13:33] |
| larsc | I don't think we want to allow that anyway. It will lead to strange behavior. E.g. if a algorithm traverses the graph multiple times and the graph is changed in between. | [13:36] |
| mchehab | well, we can protect some cases with mutex...
but I'm not sure if we can lock all cases... as some code may be running non-interactive | [13:37] |
| larsc | I'd use a reader/writer lock for dynamic updates | [13:38] |
| mchehab | we may use it, but I doubt we'll be able to solve all cases of reading past free with just locks
specially if some code running at IRQ time would need to touch a graph element anyway, if we end by covering all cases with locks, we can drop kref in the future at least for now, I feel it is safer to have a kref there... this way, we can do all incremental changes we need, being sure that we won't read a freed memory and that devm_* won't kill us there are a lot of graph transversal algorithms coded inside the drivers that aren't using the core I guess we can only safely remove kref once we move those stuff into the core and being sure that they're doing the right thing with regards to properly locking resources that are being used there | [13:40] |
| larsc | if drivers want to traverse the graph they'd lock it before starting to traverse it and unlock it after they are done traversing the graph | [13:49] |
| mchehab | larsc: if something happens at IRQ time and needs to alter an entity property, it can't check the lock
as mutexes are not allowed at IRQ context (right now, MC uses mutex) a change like that won't affect graph traverse (I mean, a property that has changed) so, it doesn't need to be locked anyway... except that the memory used to store the affected element can't be freed while it is being used so, we actually need two types of locks: one for graph traversal and another one for object removal | [13:51] |
| .... (idle for 17mn) | ||
| pinchartl | I agree with Lars, we need a rwlock
the other option would be RCU but that's overkill | [14:11] |
| mchehab | pinchartl: I agree, for graph transvesal
*traversal but this is a separate issue we may still need kref at the end of the MC changes and, at least for now, kref is the best way to go for incremental changes | [14:14] |
| pinchartl | what does the kref bring you if you have a lock anyway ?
and if you don't take the lock, I don't think kref is enough you can't protect against races with graph traversal | [14:19] |
| mchehab | pinchartl: it should prevent entities to be deleted due to devm_*
kref won't prevent against races with graph traversal. It is not meant to prevent that but it will make a way easier when times come to partially remove things at a graph as the code doesn't need to traverse the graph to remove things all it needs to know is what part of the graph was created by a driver (on a complex scenario where several drivers using the same MC device create the graph) | [14:27] |
| pinchartl | how does it have anything to do with devm_kzalloc ? | [14:30] |
| mchehab | well, the graph won't be allocated by devm_kzalloc
as the code allocating the objects will be done inside MC code it can provide a function that will drop all entities/links/pads/ints/... at once that will just do a foreach on all elements of the object list without actually needing to traverse the graph | [14:31] |
| pinchartl | so you don't embed media_entity in v4l2_subdev anymore ? | [14:34] |
| mchehab | no
the next patch in the series is to change media_entity to pointers on both v4l2_subdev and v4l_dev (we'll latter change this one to interface or create both entities and interfaces there, depending on the device type) | [14:34] |
| ............ (idle for 59mn) | ||
| pinchartl: are there any good reason for it to be embed there? | [15:36] | |
| .............................................................. (idle for 5h8mn) | ||
| pinchartl | mchehab: I need to check the code. I'm not against dynamic allocation, but I need to check the implications in details. it's a significant departure from the current model | [20:44] |
| mchehab | pinchartl: indeed. I guess I'll find a way to avoid de-embedding media_entity from v4l2_subdev for now, postponing such change to happen latter at MC redesign
as current subdev-based devices don't need dynamic allocation | [20:51] |
| pinchartl | that's what I meant by a step by step approach :-)
with a first step that introduces media_interface without a full redesign | [21:01] |
| mchehab | pinchartl: introducing media_interface will require lots of previous change, in order to avoid breaking graph traversal
we need first to have an unique object ID and remove links/pads from media_entity | [21:10] |
| pinchartl | I'm not sure to understand why
why can't you just introduce media_interface without touching the rest ? | [21:15] |
| mchehab | because we don't want to duplicate the graph traversal code
it should work with both entities and interfaces by having a different structure for entities and interfaces, we'll need to change the links and pads | [21:15] |
| pinchartl | one step at a time
just introduce media_interface first I don't even think you'll need graph traversal code there | [21:16] |
| mchehab | no, this should be last step | [21:17] |
| pinchartl | *sigh*
that's not what we agreed on | [21:17] |
| mchehab | it should be possible to see what entities are controlled by an interface | [21:17] |
| pinchartl | that has nothing to do with graph traversal | [21:17] |
| mchehab | that's what we've agreed
my original proposal were a very simple change but you nacked | [21:18] |
| pinchartl | I'm too tired to argue about it now. and I have too much work to do anyway
I'll let Hans and Sakari comment on this | [21:18] |
| mchehab | I would very much prefer to do the approach I presented on the beginning of the first day, shown on the RFC patch I sent before
but that won't satisfy all the needs discussed at the MC workshop | [21:18] |
| pinchartl | I can't spend time on this this week | [21:19] |
| mchehab | pinchartl: do you have the patches for the virtual mc driver?
if so, please send to me... it would be easier to test if the changes are preserving the old behavior | [21:24] |
| pinchartl | the driver has been posted to the linux-media mailing list
and it's hosted on github let me find the link note that it's not my project I mentor Helen working on the vmc driver but it remains her project, not mine | [21:25] |
| mchehab | ok | [21:25] |
| pinchartl | https://github.com/helen-fornazier/opw-staging | [21:25] |
| mchehab | well, I'll merge it on my experimental tree
it would be useful for testing thanks! Merging it upstream is a separate matter, as I didn't review the patchsets | [21:25] |
| koike | Sorry, I wans't following the discussion. As I am still working on the branch, the patches may change | [21:27] |
| pinchartl | Hans had several comments, so at least a v2 is needed | [21:27] |
| mchehab | from what I understood, someone reviewed, and there are a few pending things
ok I'll merge on my experimental branch for test, but won't merge upstream from such branch | [21:27] |
| koike | I was just waiting for the answer to the last question I did to resend it | [21:28] |
| mchehab | and will wait for the usual review from hans/you/sakari on that
koike: not sure what you followed, but we had a MC workshop last week... I'm working on the internal changes at MC to fulfill the needs having a virtual driver at this moment will help a lot to test such changes so, my plan is to merge on an experimental branch but this shouldn't interfere on the usual procedure for its submission | [21:28] |
| koike | I see, ok | [21:30] |
| mchehab | all MC code merge upstream is actually in frozen state, until we fix MC API | [21:31] |
| koike | I am glad to know the vimc driver will be useful | [21:32] |
| mchehab | yet, if the driver is on a good shape, we may merge it earlier on some tentative tree for upstream submission
preventing the need of reworking on it after the internal API changes koike: yeah, it should be very helpful in this fase, provided that I can simulate there the MC of some real device, like omap3 or exynos | [21:32] |
| koike | I see, I'll try to implement more user-space controls then, I was focusing on the internal image processing
I still need to catch up what has being discussed in the workshop, the resume of what has being discussed was sent to the linux media mailing list? | [21:35] |
| mchehab | no, but you may find it at the media-workshop mailman archives
I'll be submitting a summary of it next week s/summary/report/ I probably won't time any time this week to finish, and I need to wait for someone that participated on a videoconf meeting to return from vacations in order to get the names of the participants on the remote side koike: with regards to userspace, I guess the best would be if we could get the output of a media-ctl --print (or --print-dot) and send to the virtual driver for it to create a pipeline like that this way, it would mimic the pipelines of a real device without requiring to have such hardware | [21:38] |
| koike | mchehab, I see, that is a good idea indeed, I'll work on that
the problem is if the driver has nodes that the virtual driver doesn't support | [21:43] |
| mchehab | I see
well, for now, my need is to be able to actually check the pipeline representation... so replacing an unsupported entity by a do-nothing entity would be fine | [21:44] |
| koike | ok
mchehab, for how long this MC fix fase is supposed to last? So I can organize myself | [21:46] |
| mchehab | koike: good question
we don't have an answer that will very much depend on the discussions of the patch series | [21:50] |
| koike | ok, it was just to check how doable is it, if it was "in a week" then my driver would be that useful yet | [21:51] |
| .......... (idle for 49mn) | ||
| Architect1 | hey
wonder where i can get help to install my usb capture card I already installed latests V4L-DVB Device Drivers but idk why when i plug my video capture card my linux doest load the driver for it tried to find something about to force something to use a specific driver but i cant find anything on gogle google* just to you guys know the driver i'm trying to use is this one http://linuxtv.org/wiki/index.php/USB_2828x if someone could give me a light i would be glad :) | [22:40] |
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |