[02:18] <posciak> stormer: I don't think that broke
[02:18] <posciak> stormer: Kamil would probably know more
[02:18] <posciak> stormer: I was mostly referring to setting bytesused = length if bytesused == 0
[07:39] <posciak> hverkuil: ping
[07:47] <hverkuil> posciak: ping again in 45 minutes when I'm in the office (about to leave for work)
[07:47] <posciak> hverkuil: thanks
[08:31] <hverkuil> posciak: pong
[08:33] <posciak> hverkuil: I was looking at https://patchwork.linuxtv.org/patch/24861/. It seems to me that __verify_length doesn't need to adjust bytesused to lenght, since __fill_vb2_buffer does that later. Is there something I'm missing?
[08:35] <posciak> sorry need to afk for a bit, but please feel free to leave a resp.
[08:36] <hverkuil> ??? __verify_length doesn't set bytesused.
[08:36] <posciak> ohhh
[08:36] <posciak> heh
[08:36] <posciak> total misread
[08:36] <posciak> it's a local var
[08:36] <posciak> sorry
[08:36] <posciak> sorry sorry sorry
[08:36] <posciak> :)
[10:38] <mchehab> pinchartl: did you have time to review the MC patches?
[10:39] <pinchartl> mchehab: I'd love to say yes
[10:40] <pinchartl> but I have to say no
[10:40] <mchehab> :(
[10:40] <pinchartl> I was away last week
[10:40] <pinchartl> and my e-mail backlog is quite large
[10:43] <mchehab> when do you think you could look on it?
[10:45] <pinchartl> possibly tomorrow morning, or Friday afternoon
[10:46] <hverkuil> mchehab: that reminds me: can you mark your dvb patch series as accepted in patchwork? That would clean it up.
[10:47] <mchehab> pinchartl: ok
[10:47] <mchehab> hverkuil: I will
[10:47] <mchehab> hverkuil: from your review, your only big issue was the ALSA changes, right?
[10:48] <hverkuil> Yes. And the fact that you committed it without giving time for review as you may have noticed :-)
[10:49] <mchehab> hverkuil: I gave 1,5 months of time to review. This sounds reasonable for me
[10:50] <mchehab> the changes i made on the last version were cosmetics
[10:50] <hverkuil> let's just agree to disagree on that.
[10:50] <mchehab> :)
[10:51] <mchehab> I do cosmetic changes when applying patches, even when I pull from sub-maintainers
[10:51] <mchehab> when those changes are needed
[10:52] <mchehab> (patch rebasing, comment fixes, coding style fixes, warning fixes, and I even did a few DocBook changes - although those are rare)
[10:52] <mchehab> such procedure is needed in order to avoid delaying merge patches due to just silly things
[10:54] <mchehab> when those changes require a separate patch, I send to the ML to warn people, but I just commit and move on generally without waiting for acks, as the hole reason for those quick fixes is to fast track a pull request
[11:00] <hverkuil> pinchartl: entity names have to be unique, right? Or was that just the entity id? I know we discussed this before, but I forgot the details and the spec is not explicit about this
[11:09] <pinchartl> hverkuil: I don't think we have an explicit requirements for unique entity names
[11:09] <pinchartl> the media-ctl userspace tool expects that though
[11:09] <pinchartl> as it allows specifying entities by name
[11:14] <pinchartl> hverkuil: why?
[11:14] <mchehab> right now, we're not assuring that the device names on the media controller patches are unique
[11:14] <mchehab> s/device names/names/
[11:15] <mchehab> at the DVB patches
[11:15] <hverkuil> pinchartl: http://www.spinics.net/lists/linux-media/msg86508.html
[11:16] <mchehab> on usual cases, they'll be unique, but more complex devices may have duplicated names
[11:17] <mchehab> hverkuil: I'm a way less concern with tuner names than I'm concerned with the names registered by the DVB core
[11:17] <mchehab> one device may have multiple "dvb-ca" modules, for example
[11:18] <mchehab> at the DVB side, there are several boards that are dual
[11:19] <mchehab> the typical case is that the same device has internally two independent devices
[11:19] <mchehab> but there are more complex devices that allows sharing entities among the two devices
[11:20] <mchehab> for the typical case, we may map them using two media controller devices...
[11:20] <mchehab> but for the case where the sharing is possible, we should map them as just one media controller device
[11:21] <mchehab> See for example this device:
[11:21] <mchehab> http://linuxtv.org/wiki/index.php/File:Netup_dual_dvb_universal_scheme_hw.png
[11:21] <pinchartl> can't we append an index to the entity name ?
[11:21] <mchehab> It has two CI slots (DVB CA)
[11:22] <mchehab> in thesis, one for adapter 0 and another one for adapter 1, but it is (likely) be possible to use both for adapter 0 or both for adapter 1
[11:23] <mchehab> pinchartl: yes, but the index would need to be actually two numbers
[11:23] <mchehab> as the DVB API namespace is:
[11:24] <mchehab> adapter\d/frontend\d, adapter\d/demux\d, ...
[11:24] <mchehab> so, if we append the number, we'll need to add both adapter# and frontend/demux/dvr/ca/net#
[12:21] <pinchartl> larsc: I'm having a problem with the adv7511 driver than the adv7511_wait_for_interrupt() implementation
[12:22] <pinchartl> if an IRQ is declared, the driver calls wait_event_interruptible_timeout() to wait for the interrupt
[12:22] <pinchartl> the wait condition is implemented in the adv7511_is_interrupt_pending() function
[12:22] <pinchartl> trouble is, wait_event_interruptible_timeout() will set the task state to TASK_INTERRUPTIBLE
[12:23] <pinchartl> adv7511_is_interrupt_pending() will thus be called with the task state set to TASK_INTERRUPTIBLE as well
[12:23] <pinchartl> the function then proceeds to read I2C registers
[12:23] <pinchartl> which calls __might_sleep
[12:23] <pinchartl> resulting in a WARN_ON(current->state != TASK_RUNNING)
[12:23] <pinchartl> the WARN_ON got introduced in v3.19-rc1
[12:24] <pinchartl> have you seen that ?
[12:25] <pinchartl> I'm not sure whether wait_event_interruptible_timeout() is supposed to allow its condition to sleep
[12:29] <larsc> pinchartl: this I guess http://lwn.net/Articles/628628/
[12:36] <pinchartl> larsc: exactly :-)
[12:36] <pinchartl> thanks
[12:39] <pinchartl> I don't think the adv7511 driver really requires nested sleeps
[12:39] <pinchartl> you don't have a fix in your tree, do you ? :-)
[12:44] <larsc> no
[12:44] <pinchartl> ok, I'll fix it
[13:00] <pinchartl> there's surprisingly many bugs in that code
[13:19] <larsc> no surprises, its a vendor driver after all
[13:20] <pinchartl> :-)
[13:29] <pinchartl> larsc: you have a fix in your mailbox
[13:42] <larsc> thanks
[13:42] <pinchartl> but now I sometimes get invalid EDID data :-/
[13:42] <pinchartl> the first byte is missing
[13:42] <pinchartl> I rememeber having that problem earlier, but can't remember how I've fixed it
[13:43] <pinchartl> does it ring a bell ?
[13:47] <pinchartl> I remember it was something really stupid :-)
[13:47] <pinchartl> or maybe it was an issue with the adv7611 ?
[13:50] <larsc> only the first byte?
[13:50] <pinchartl> yes
[13:51] <pinchartl> the EDID block is shifted by one byte
[13:51] <larsc> that's strange
[13:51] <pinchartl> it occurs randomly
[13:51] <pinchartl> I know I've seen that before
[13:51] <larsc> using the xilinx ps7 i2c controller?
[13:51] <pinchartl> no, on a Renesas board
[13:52] <pinchartl> I'm pretty sure to have discussed this issue in this channel :-)
[13:52] <pinchartl> but I can't remember if it was in relation with Xilinx or Renesas, and whether it was the adv7511 or the adv7611
[13:55] <larsc> let me try the patch
[13:56] <larsc> can you send me the non-diff version?
[13:58] <pinchartl> non-diff version ?
[13:58] <pinchartl> do you mean adv7511.c ?
[13:58] <larsc> yes
[13:59] <larsc> I can't get the patch to apply
[13:59] <pinchartl> done
[14:00] <pinchartl> the patch should apply on Dave's drm-next branch
[14:06] <larsc> "axi-hdmi 70e00000.axi_hdmi: HDMI-A-1: EDID block 0 invalid.
[14:07] <larsc> but it's more than 1 byte
[14:07] <pinchartl> the driver should print the raw EDID in the kernel log
[14:07] <larsc> yes, the whole header is missing
[14:07] <pinchartl> when it failed this is what I had
[14:07] <pinchartl> https://paste.kde.org/pplyihety
[14:10] <pinchartl> is the adv7511 irq wired up on your system, or are you using the polling method ?
[14:12] <larsc> polling atm
[14:13] <pinchartl> I haven't tested that, I probably should
[14:25] <larsc> for some reason current_edid_segment is 0 when I try to read it the first time
[14:28] <larsc> ah, the data parameter for get_edid_block changed
[14:31] <larsc> And now it crashes because the wq is not intialized
[14:32] <pinchartl> ouch
[14:32] <pinchartl> sorry about that :-/
[14:32] <pinchartl> I'll fix it
[14:33] <pinchartl> and test the polling case
[14:38] <larsc> tried a few reboots, no problems
[14:38] <larsc> I don't think we actually need to change the polled path
[14:43] <pinchartl> we don't
[14:43] <pinchartl> except to fix the bug due to DDC_ERROR being checked in irq0
[14:44] <pinchartl> but I like using the same code path for both irq and poll
[15:30] <mchehab> hverkuil: sent the fix patches for the media controller stuff via ML
[15:30] <mchehab> still pending the changes at the name for them to be unique
[15:30] <mchehab> this will require to convert some static struct into some dynamic ones
[15:30] <mchehab> at the dvb side
[15:38] <hverkuil> reviewing patches
[15:46] <hverkuil> finished reviewing
[15:46] <mchehab> thanks!
[15:46] <mchehab> bbl
[21:19] <henr_k> both mchehab and hansverk is on the top-10 linux contributor list since 2.6.11, that's pretty amazing. Good work guys! :)
[21:24] <headless> henr_k: the site I'm looking at has hverkuil at 12th place
[21:24] <headless> http://remword.com/kps_result/all_petop.html
[21:24] <henr_k> headless: yes, I just realized that myself (I read the pdf from lf and got fooled by the table)
[21:25] <henr_k> even so, 2 amongst top-12 is pretty well done imho
[21:25] <headless> sure
[21:27] <headless> I was surprised someone could displace Al Viro :-)
[21:32] <larsc> ALSA still beats V4L ;)