[00:16] *** gtucker has quit IRC (Ping timeout: 246 seconds)
[03:42] *** stdint has quit IRC (Read error: Connection reset by peer)
[08:23] *** prabhakarlad has left 
[10:11] <ezequielg> hverkuil: hope it's not too late to fix the num_planes issue.
[10:13] <hverkuil> ezequielg: did you post a patch?
[10:14] <ezequielg> not yet.
[10:18] <ezequielg> just finishing some tests.
[10:36] <hverkuil> sailus: can you review https://patchwork.linuxtv.org/patch/54532/ as soon as possible? This is a fix for 5.0 so if I get a quick Reviewed-by, then I can make a pull request for it.
[11:28] <sailus> hverkuil: The number of planes is currently limited because of IOCTL argument size.
[11:28] <sailus> How likely it is that the IOCTL argument is otherwise sane if the number of planes is invalid?
[11:29] <sailus> Note that this signals an intended size of an array, not an individual parameter.
[11:30] <sailus> It is in alignment with the documentation though, but that documentation was written before support for multi-plane buffers was added.
[11:31] <hverkuil> It's independent of num_planes. S_FMT passed without error before this patch was applied, now it returns an error. That's a sudden change in behavior.
[11:31] <sailus> videobuf2 returns an error if the number of planes exceed VB2_MAX_PLANES.
[11:32] <hverkuil> I think you are confusing v4l2_buffer with v4l2_format.
[11:32] <hverkuil> This patch is for setting/trying formats.
[11:32] <sailus> hverkuil: Yes, that's a valid point as well. But with buffers it still works differently.
[11:32] <hverkuil> Yes, but we
[11:32] <hverkuil> Yes, but we're not talking about buffers :-)
[11:32] <sailus> :-)
[11:33] <ezequielg> i think the main point here is that it's a uabi regression.
[11:33] <sailus> And you're expected to set to the buffer just as many planes as you had on a format, but for formats you don't know that in advance.
[11:33] <hverkuil> Right. Clamping to the max number of planes makes a lot of sense.
[11:34] <sailus> A moment.
[11:34] <hverkuil> It keeps the original behavior w.r.t. error returns and drivers get safe values.
[11:35] <hverkuil> (BTW, this is a typical change that would be caught with a proper regression test. Which is almost ready for use)
[11:38] <sailus> I'll reply to the patch.
[11:39] <sailus> Done.
[11:39] <hverkuil> Thanks!
[11:40] <hverkuil> mchehab: ping
[12:20] <mchehab> hverkuil: pong
[12:26] <hverkuil> mchehab: https://patchwork.linuxtv.org/patch/54532/
[12:27] <hverkuil> this is a fix for 5.0, but it relies on earlier fixes that have been merged in 5.0 mainline, but not yet merged back in our master.
[12:27] <hverkuil> Is this something you can handle? Alternatively, I can make a pull request on top of the mainline kernel instead of our master.
[12:28] <hverkuil> I'm not sure what is easiest for you.
[13:06] <mchehab> hverkuil: it sounds better to rebase it over 5.0
[13:06] <mchehab> and then handle the conflicts on 5.1
[13:07] <mchehab> I need to go out for a while to do some errands. Will take a look after my return
[13:08] <mchehab> (hopefully... internet is too unstable here today)
[13:08] <hverkuil> ok, just let me know what you want me to do.
[15:02] <mchehab> hans, X-Patchwork-Id: 54532 applies cleanly against my 5.0 branch
[15:03] <mchehab>  hverkuil ^
[15:04] <mchehab> ezequielg: ^ did you test https://patchwork.linuxtv.org/patch/54532/ against 5.0-rc ?
[15:04] <mchehab> or does it need something else?
[15:05] <hverkuil> mchehab: which branch? fixes or master?
[15:06] <mchehab> fixes
[15:06] <mchehab> it is already at master
[15:06] <hverkuil> It's fine for fixes, not for master.
[15:07] <hverkuil> I'm confused.
[15:07] <mchehab> I am so confused...
[15:07] <mchehab> if I understood well, you said it should go to 5.0, but it would need some rebase for it
[15:08] <mchehab> let's restart the thread...
[15:08] <hverkuil> This is a fix for a patch that went in for rc2 or rc3.
[15:08] <mchehab> what do you expect me to do with https://patchwork.linuxtv.org/patch/54532/?
[15:08] <hverkuil> But our master is still at rc1, so it doesn't contain those fixes.
[15:09] <hverkuil> That patch is a fix for a 5.0 fix.
[15:09] <mchehab> ok...
[15:09] <hverkuil> Perhaps it is easiest if you merge mainline into our master so that we're up to date?
[15:09] <mchehab> so, if I'm understaning it well, you want me to add it at -fixes (as is)...
[15:09] <hverkuil> right.
[15:09] <mchehab> then pull from, let's say, -rc6
[15:09] <mchehab> and apply it too at master, right?
[15:10] <hverkuil> yes
[15:10] <mchehab> ok
[15:11] <hverkuil> I think in the future when fixes are merged in mainline, we should pull them back in our master.
[15:11] <mchehab> I usually do that
[15:12] <mchehab> probably forgot this time
[15:13] <mchehab> what I do is that I wait for the next -rc to be released, and pull back from it
[15:13] <hverkuil> OK
[15:13] <mchehab> merged on both branches
[15:18] <mchehab> hverkuil: btw, there were a regression on cx231xx - I guess b-rad did some patches, and someone posted another fix
[15:18] <mchehab> it would be great if such regression fix could be on one of the pending pull requests
[15:18] * mchehab is starting to handle them now
[15:19] <hverkuil> Is it this series? [PATCH 0/2] Media Controller "taint" fixes
[15:19] <hverkuil> (I'm preparing a last pull request, so I can add these to that PR)
[15:19] <mchehab> I guess this could contain one of the fixes
[15:20] <mchehab> see Subject: Regression in 4.20 - still present in 5.0-rc6
[15:22] <mchehab> sailus: I'm starting to be really pissed with the ipu3 warnings. please fix it
[15:23] <hverkuil> mchehab: I'll do some tests with cx231xx first.
[15:23] <mchehab> gah, I solved a conflict at the wrong way:
[15:23] <mchehab> drivers/media/platform/vim2m.c:908:29: error: 'dev' undeclared (first use in this function); did you mean 'cdev'?
[15:23] <mchehab>   if (v4l2_m2m_get_curr_priv(dev->m2m_dev) == ctx)
[15:23] <mchehab>                              ^~~
[15:23] <mchehab>                              cdev
[15:26] <sailus> mchehab: I know... I've started looking into the issues.
[15:27] <sailus> hverkuil pinged me about the same today.
[15:34] <mchehab> ok
[15:34] <mchehab> hverkuil: please check this: https://patchwork.linuxtv.org/patch/54534/
[15:35] <mchehab> forgot to add this:
[15:35] <mchehab> Fixes: 240809ef6630 ("media: vim2m: only cancel work if it is for right context")
[15:36] <mchehab> in summary, this patch caused a conflict with some patches that got merged at fixes
[15:38] <mchehab> that's the patch it conflicts with:
[15:38] <mchehab> Fixes: b3e64e5b0778 ("media: vim2m: use per-file handler work queue")
[15:45] <hverkuil> mchehab: reviewed, needs a bit more work. Non-trivial conflict.
[15:46] <mchehab> yeah, that's what I was afraid
[15:47] <mchehab> we could just remove the if, but not sure if this is what it would be expected
[15:47] <mchehab> ah, you suggested the same thing :-)
[15:48] <hverkuil> I had to think about this one a bit :-)
[15:52] <mchehab> v2 sent
[15:53] <mchehab> I suspect that you fixed the bug I was noticing before migrating the work_run to struct vim2m_cts
[15:53] <mchehab>  struct vim2m_ctx
[15:53] <mchehab> :-)
[15:56] <mchehab> ok, fix merged
[15:56] <mchehab> I almost rebased the tree and folded it with the merge
[15:57] <mchehab> but opted to merge in a separate patch
[15:57] <mchehab> to avoid rebase
[16:04] *** benjiG has left 
[16:06] <hverkuil> mchehab: not sure how this patch https://patchwork.kernel.org/patch/10763655/ relates to Brad's patch series.
[16:06] <hverkuil> Brad's series doesn't work, it's still failing.
[16:06] <hverkuil> I've no idea what Brad did.
[16:07] <mchehab> lt's switch to #linuxtv channel... he's probably there
[16:07] <mchehab> hmm... you're not there :-)
[16:15] <ezequielg> hey
[16:15] <ezequielg> i was on a call, sorry.
[16:17] <ezequielg> but i see you guys solved it :-)
[16:22] <mchehab> ezequielg: yeah, already solved. thanks!
[16:22] <ezequielg> good!
[18:43] <shuah> hverkuil: Thanks for sharing you test. I got the sharing working now. I have one last thing to fix before I send the patch series out
[20:38] <pinchartl> mchehab: thanks for pulling the uvc + vsp1 changes
[20:38] <mchehab> anytime
[20:42] <ezequielg> hverkuil: sailus: please review the v4l2-ctrls debugging series
[20:42] <ezequielg> I've added mostly arbitrary debug messages, but the goal is the cover all possible failures, or at least the 99% most common.