Re: [PATCH] media: aspeed: clear garbage interrupts
On 9/26/19 5:27 PM, Jae Hyun Yoo wrote: From: Jae Hyun Yoo CAPTURE_COMPLETE and FRAME_COMPLETE interrupts come even when these are disabled in the VE_INTERRUPT_CTRL register and eventually this behavior causes disabling irq itself like below: Thanks! Reviewed-by: Eddie James [10055.108784] irq 23: nobody cared (try booting with the "irqpoll" option) [10055.115525] CPU: 0 PID: 331 Comm: swampd Tainted: GW 5.3.0-4fde000-dirty-d683e2e #1 [10055.124565] Hardware name: Generic DT based system [10055.129355] Backtrace: [10055.131854] [<80107d7c>] (dump_backtrace) from [<80107fb0>] (show_stack+0x20/0x24) [10055.139431] r7:0017 r6:0001 r5: r4:9d51dc00 [10055.145120] [<80107f90>] (show_stack) from [<8074bf50>] (dump_stack+0x20/0x28) [10055.152361] [<8074bf30>] (dump_stack) from [<80150ffc>] (__report_bad_irq+0x40/0xc0) [10055.160109] [<80150fbc>] (__report_bad_irq) from [<80150f2c>] (note_interrupt+0x23c/0x294) [10055.168374] r9:015b6e60 r8: r7:0017 r6:0001 r5: r4:9d51dc00 [10055.176136] [<80150cf0>] (note_interrupt) from [<8014df1c>] (handle_irq_event_percpu+0x88/0x98) [10055.184835] r10:7eff7910 r9:015b6e60 r8: r7:9d417600 r6:0001 r5:0002 [10055.192657] r4:9d51dc00 r3: [10055.196248] [<8014de94>] (handle_irq_event_percpu) from [<8014df64>] (handle_irq_event+0x38/0x4c) [10055.205113] r5:80b56d50 r4:9d51dc00 [10055.208697] [<8014df2c>] (handle_irq_event) from [<80151f1c>] (handle_level_irq+0xbc/0x12c) [10055.217037] r5:80b56d50 r4:9d51dc00 [10055.220623] [<80151e60>] (handle_level_irq) from [<8014d4b8>] (generic_handle_irq+0x30/0x44) [10055.229052] r5:80b56d50 r4:0017 [10055.232648] [<8014d488>] (generic_handle_irq) from [<8014d524>] (__handle_domain_irq+0x58/0xb4) [10055.241356] [<8014d4cc>] (__handle_domain_irq) from [<801021e4>] (avic_handle_irq+0x68/0x70) [10055.249797] r9:015b6e60 r8:00c5387d r7:00c5387d r6: r5:9dd33fb0 r4:9d402380 [10055.257539] [<8010217c>] (avic_handle_irq) from [<80101e34>] (__irq_usr+0x54/0x80) [10055.265105] Exception stack(0x9dd33fb0 to 0x9dd33ff8) [10055.270152] 3fa0: 015d0530 015d0538 [10055.278328] 3fc0: 015d0530 015b6e60 0052c5d0 015b6e60 7eff7910 7eff7918 [10055.286496] 3fe0: 76ce5614 7eff7908 0050e2f4 76a3a08c 2010 [10055.293104] r5:2010 r4:76a3a08c [10055.296673] handlers: [10055.298967] [<79f218a5>] irq_default_primary_handler threaded [<1de88514>] aspeed_video_irq [10055.307344] Disabling IRQ #23 To fix this issue, this commit makes the interrupt handler clear these garbage interrupts. This driver enables and uses only COMP_COMPLETE interrupt instead for frame handling. Signed-off-by: Jae Hyun Yoo --- drivers/media/platform/aspeed-video.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index eb12f3793062..e842f99d20a9 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -606,6 +606,16 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg) aspeed_video_start_frame(video); } + /* +* CAPTURE_COMPLETE and FRAME_COMPLETE interrupts come even when these +* are disabled in the VE_INTERRUPT_CTRL register so clear them to +* prevent unnecessary interrupt calls. +*/ + if (sts & VE_INTERRUPT_CAPTURE_COMPLETE) + sts &= ~VE_INTERRUPT_CAPTURE_COMPLETE; + if (sts & VE_INTERRUPT_FRAME_COMPLETE) + sts &= ~VE_INTERRUPT_FRAME_COMPLETE; + return sts ? IRQ_NONE : IRQ_HANDLED; }
Re: [PATCH -next v2 2/2] media: aspeed: set hsync and vsync polarities to normal before starting mode detection
On 9/13/19 1:11 PM, Jae Hyun Yoo wrote: Sometimes it detects a weird resolution such as 1024x287 when the actual resolution is 1024x768. To resolve such an issue, this commit adds clearing for hsync and vsync polarity register bits at the beginning of the first mode detection. This is recommended in the datasheet. Reviewed-by: Eddie James Signed-off-by: Jae Hyun Yoo --- Changes since v1: None drivers/media/platform/aspeed-video.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index 8f77079da55a..929b3a5b8849 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -740,6 +740,8 @@ static void aspeed_video_get_resolution(struct aspeed_video *video) } set_bit(VIDEO_RES_DETECT, &video->flags); + aspeed_video_update(video, VE_CTRL, + VE_CTRL_VSYNC_POL | VE_CTRL_HSYNC_POL, 0); aspeed_video_enable_mode_detect(video); rc = wait_event_interruptible_timeout(video->wait,
Re: [PATCH -next v2 1/2] media: aspeed: refine hsync/vsync polarity setting logic
On 9/13/19 1:11 PM, Jae Hyun Yoo wrote: To prevent inaccurate detections of resolution, this commit enables clearing of hsync/vsync polarity bits based on probed sync state. Thanks Jae, looks fine. Reviewed-by: Eddie James Signed-off-by: Jae Hyun Yoo --- Changes since v1: * Updated commit message. drivers/media/platform/aspeed-video.c | 43 +-- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index eb12f3793062..8f77079da55a 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -614,7 +614,7 @@ static void aspeed_video_check_and_set_polarity(struct aspeed_video *video) int i; int hsync_counter = 0; int vsync_counter = 0; - u32 sts; + u32 sts, ctrl; for (i = 0; i < NUM_POLARITY_CHECKS; ++i) { sts = aspeed_video_read(video, VE_MODE_DETECT_STATUS); @@ -629,30 +629,29 @@ static void aspeed_video_check_and_set_polarity(struct aspeed_video *video) hsync_counter++; } - if (hsync_counter < 0 || vsync_counter < 0) { - u32 ctrl = 0; + ctrl = aspeed_video_read(video, VE_CTRL); - if (hsync_counter < 0) { - ctrl = VE_CTRL_HSYNC_POL; - video->detected_timings.polarities &= - ~V4L2_DV_HSYNC_POS_POL; - } else { - video->detected_timings.polarities |= - V4L2_DV_HSYNC_POS_POL; - } - - if (vsync_counter < 0) { - ctrl = VE_CTRL_VSYNC_POL; - video->detected_timings.polarities &= - ~V4L2_DV_VSYNC_POS_POL; - } else { - video->detected_timings.polarities |= - V4L2_DV_VSYNC_POS_POL; - } + if (hsync_counter < 0) { + ctrl |= VE_CTRL_HSYNC_POL; + video->detected_timings.polarities &= + ~V4L2_DV_HSYNC_POS_POL; + } else { + ctrl &= ~VE_CTRL_HSYNC_POL; + video->detected_timings.polarities |= + V4L2_DV_HSYNC_POS_POL; + } - if (ctrl) - aspeed_video_update(video, VE_CTRL, 0, ctrl); + if (vsync_counter < 0) { + ctrl |= VE_CTRL_VSYNC_POL; + video->detected_timings.polarities &= + ~V4L2_DV_VSYNC_POS_POL; + } else { + ctrl &= ~VE_CTRL_VSYNC_POL; + video->detected_timings.polarities |= + V4L2_DV_VSYNC_POS_POL; } + + aspeed_video_write(video, VE_CTRL, ctrl); } static bool aspeed_video_alloc_buf(struct aspeed_video *video,
Re: [PATCH 3/7] media: aspeed-video: address a protential usage of an unit var
On 8/22/19 2:39 PM, Mauro Carvalho Chehab wrote: While this might not occur in practice, if the device is doing the right thing, it would be teoretically be possible to have both hsync_counter and vsync_counter negatives. If this ever happen, ctrl will be undefined, but the driver will still call: aspeed_video_update(video, VE_CTRL, 0, ctrl); Change the code to prevent this to happen. This was warned by cppcheck: [drivers/media/platform/aspeed-video.c:653]: (error) Uninitialized variable: ctrl Thanks Mauro. Reviewed-by: Eddie James Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/aspeed-video.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index f899ac3b4a61..4ef37cfc8446 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -630,7 +630,7 @@ static void aspeed_video_check_and_set_polarity(struct aspeed_video *video) } if (hsync_counter < 0 || vsync_counter < 0) { - u32 ctrl; + u32 ctrl = 0; if (hsync_counter < 0) { ctrl = VE_CTRL_HSYNC_POL; @@ -650,7 +650,8 @@ static void aspeed_video_check_and_set_polarity(struct aspeed_video *video) V4L2_DV_VSYNC_POS_POL; } - aspeed_video_update(video, VE_CTRL, 0, ctrl); + if (ctrl) + aspeed_video_update(video, VE_CTRL, 0, ctrl); } }
Re: MyGica T230 dvb-t2 data corruption since commit 5fa8815
Hi Jan, I've been running for a couple of weeks now with your two patches applied: 1) dvbsky: add MyGica T230 2) remove t230 from cxusb Everything has been working perfectly - the checksum errors when scanning muxes have gone, and streaming has been working nicely. Tested-by: James Hutchinson Regards, James On Fri, 19 Jul 2019 at 19:35, Jan Pieter van Woerkom wrote: > > dvbsky: add MyGica T230. > Moved from cxusb driver as that driver can't handle FX2 FIFO issue. > > Signed-off-by: Jan Pieter van Woerkom > --- > diff -ru a/drivers/media/usb/dvb-usb-v2/dvbsky.c > b/drivers/media/usb/dvb-usb-v2/dvbsky.c > --- a/drivers/media/usb/dvb-usb-v2/dvbsky.c 2019-07-08 00:41:56.0 > +0200 > +++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c 2019-07-19 17:50:54.671341146 > +0200 > @@ -561,11 +561,18 @@ > > /* attach tuner */ > si2157_config.fe = adap->fe[0]; > - si2157_config.if_port = 0; > - > - state->i2c_client_tuner = dvb_module_probe("si2157", "si2141", > + if (le16_to_cpu(d->udev->descriptor.idProduct) == > USB_PID_MYGICA_T230) { > + si2157_config.if_port = 1; > + state->i2c_client_tuner = dvb_module_probe("si2157", NULL, > + i2c_adapter, > + 0x60, &si2157_config); > + } > + else { > + si2157_config.if_port = 0; > + state->i2c_client_tuner = dvb_module_probe("si2157", "si2141", >i2c_adapter, >0x60, &si2157_config); > + } > if (!state->i2c_client_tuner) { > dvb_module_release(state->i2c_client_demod); > return -ENODEV; > @@ -787,6 +794,9 @@ > { DVB_USB_DEVICE(USB_VID_TERRATEC, USB_PID_TERRATEC_CINERGY_S2_R4, > &dvbsky_s960_props, "Terratec Cinergy S2 Rev.4", > RC_MAP_DVBSKY) }, > + { DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230, > + &mygica_t230c_props, "MyGica Mini DVB-T2 USB Stick T230", > + RC_MAP_TOTAL_MEDIA_IN_HAND_02) }, > { DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230C, > &mygica_t230c_props, "MyGica Mini DVB-T2 USB Stick T230C", > RC_MAP_TOTAL_MEDIA_IN_HAND_02) },
Re: MyGica T230 dvb-t2 data corruption since commit 5fa8815
Hi Jan, I've been running for a couple of weeks now with your two patches applied: 1) dvbsky: add MyGica T230 2) remove t230 from cxusb Everything has been working perfectly - the checksum errors when scanning muxes have gone, and streaming has been working nicely. Tested-by: James Hutchinson Regards, James On Tue, 23 Jul 2019 at 17:02, James Hutchinson wrote: > > Hi Jan, > > My initial testing of your two patches to move the T230 to the dvbsky > module looks good so far. > > I'm testing on an 8th Gen NUC (8i3BEH) 16GB RAM, 2 x MyGica T230 T2 > USB tuners, and 2 x DVBSky S960 S2 tuners. > > This machine is running Debian stretch with the Debian 4.19.37 > backport kernel (incl. your two patches to move the T230 to the dvbsky > module). > > I have a test script that performs 200x consecutive tvheadend network scans... > This was executed against the UK Freeview network (6 muxes) =~1,200 > tuning requests. This took a couple of hours, but not a single > checksum error. > > I moved onto streaming various channels and didn't have any problems > so far. I only noted a couple of minor picture disturbances > (continuity counter errors) when i tried to record two HD channels > whilst watching a 3rd on the same mux. I'm not sure whether this was > pushing things too far, and would like to carry out similar tests on a > known working 4.9.x kernel. > > I'll do more testing over the next couple of weeks and report back. > > Also adding Thomas who has the same issue with the T230 tuner and > bisected to issue to the same problematic commit as me. > > Regards, > James. > > > > On Mon, 22 Jul 2019 at 17:22, Jan Pieter van Woerkom wrote: > > > > remove t230 from cxusb > > > > Signed-off-by: Jan Pieter van Woerkom > > --- > > diff -ru a/drivers/media/usb/dvb-usb/cxusb.c > > b/drivers/media/usb/dvb-usb/cxusb.c > > --- a/drivers/media/usb/dvb-usb/cxusb.c 2019-07-08 00:41:56.0 +0200 > > +++ b/drivers/media/usb/dvb-usb/cxusb.c 2019-07-22 17:34:51.550698820 +0200 > > @@ -369,26 +369,6 @@ > > return 0; > > } > > > > -static int cxusb_read_status(struct dvb_frontend *fe, > > - enum fe_status *status) > > -{ > > - struct dvb_usb_adapter *adap = (struct dvb_usb_adapter > > *)fe->dvb->priv; > > - struct cxusb_state *state = (struct cxusb_state *)adap->dev->priv; > > - int ret; > > - > > - ret = state->fe_read_status(fe, status); > > - > > - /* it need resync slave fifo when signal change from unlock to > > lock.*/ > > - if ((*status & FE_HAS_LOCK) && (!state->last_lock)) { > > - mutex_lock(&state->stream_mutex); > > - cxusb_streaming_ctrl(adap, 1); > > - mutex_unlock(&state->stream_mutex); > > - } > > - > > - state->last_lock = (*status & FE_HAS_LOCK) ? 1 : 0; > > - return ret; > > -} > > - > > static void cxusb_d680_dmb_drain_message(struct dvb_usb_device *d) > > { > > int ep = d->props.generic_bulk_ctrl_endpoint; > > @@ -1164,83 +1144,6 @@ > > return 0; > > } > > > > -static int cxusb_mygica_t230_frontend_attach(struct dvb_usb_adapter *adap) > > -{ > > - struct dvb_usb_device *d = adap->dev; > > - struct cxusb_state *st = d->priv; > > - struct i2c_adapter *adapter; > > - struct i2c_client *client_demod; > > - struct i2c_client *client_tuner; > > - struct i2c_board_info info; > > - struct si2168_config si2168_config; > > - struct si2157_config si2157_config; > > - > > - /* Select required USB configuration */ > > - if (usb_set_interface(d->udev, 0, 0) < 0) > > - err("set interface failed"); > > - > > - /* Unblock all USB pipes */ > > - usb_clear_halt(d->udev, > > - usb_sndbulkpipe(d->udev, > > d->props.generic_bulk_ctrl_endpoint)); > > - usb_clear_halt(d->udev, > > - usb_rcvbulkpipe(d->udev, > > d->props.generic_bulk_ctrl_endpoint)); > > - usb_clear_halt(d->udev, > > - usb_rcvbulkpipe(d->udev, > > d->props.adapter[0].fe[0].stream.endpoint)); > > - > > - /* attach frontend */ > > - si2168_config.i2c_adapter = &adapter; > > - si2168_config.fe = &adap->fe_adap[0].fe; > > - si2168_config.ts_mode = SI2168_TS_PARALLEL; > >
Re: MyGica T230 dvb-t2 data corruption since commit 5fa8815
Hi Jan, My initial testing of your two patches to move the T230 to the dvbsky module looks good so far. I'm testing on an 8th Gen NUC (8i3BEH) 16GB RAM, 2 x MyGica T230 T2 USB tuners, and 2 x DVBSky S960 S2 tuners. This machine is running Debian stretch with the Debian 4.19.37 backport kernel (incl. your two patches to move the T230 to the dvbsky module). I have a test script that performs 200x consecutive tvheadend network scans... This was executed against the UK Freeview network (6 muxes) =~1,200 tuning requests. This took a couple of hours, but not a single checksum error. I moved onto streaming various channels and didn't have any problems so far. I only noted a couple of minor picture disturbances (continuity counter errors) when i tried to record two HD channels whilst watching a 3rd on the same mux. I'm not sure whether this was pushing things too far, and would like to carry out similar tests on a known working 4.9.x kernel. I'll do more testing over the next couple of weeks and report back. Also adding Thomas who has the same issue with the T230 tuner and bisected to issue to the same problematic commit as me. Regards, James. On Mon, 22 Jul 2019 at 17:22, Jan Pieter van Woerkom wrote: > > remove t230 from cxusb > > Signed-off-by: Jan Pieter van Woerkom > --- > diff -ru a/drivers/media/usb/dvb-usb/cxusb.c > b/drivers/media/usb/dvb-usb/cxusb.c > --- a/drivers/media/usb/dvb-usb/cxusb.c 2019-07-08 00:41:56.0 +0200 > +++ b/drivers/media/usb/dvb-usb/cxusb.c 2019-07-22 17:34:51.550698820 +0200 > @@ -369,26 +369,6 @@ > return 0; > } > > -static int cxusb_read_status(struct dvb_frontend *fe, > - enum fe_status *status) > -{ > - struct dvb_usb_adapter *adap = (struct dvb_usb_adapter > *)fe->dvb->priv; > - struct cxusb_state *state = (struct cxusb_state *)adap->dev->priv; > - int ret; > - > - ret = state->fe_read_status(fe, status); > - > - /* it need resync slave fifo when signal change from unlock to lock.*/ > - if ((*status & FE_HAS_LOCK) && (!state->last_lock)) { > - mutex_lock(&state->stream_mutex); > - cxusb_streaming_ctrl(adap, 1); > - mutex_unlock(&state->stream_mutex); > - } > - > - state->last_lock = (*status & FE_HAS_LOCK) ? 1 : 0; > - return ret; > -} > - > static void cxusb_d680_dmb_drain_message(struct dvb_usb_device *d) > { > int ep = d->props.generic_bulk_ctrl_endpoint; > @@ -1164,83 +1144,6 @@ > return 0; > } > > -static int cxusb_mygica_t230_frontend_attach(struct dvb_usb_adapter *adap) > -{ > - struct dvb_usb_device *d = adap->dev; > - struct cxusb_state *st = d->priv; > - struct i2c_adapter *adapter; > - struct i2c_client *client_demod; > - struct i2c_client *client_tuner; > - struct i2c_board_info info; > - struct si2168_config si2168_config; > - struct si2157_config si2157_config; > - > - /* Select required USB configuration */ > - if (usb_set_interface(d->udev, 0, 0) < 0) > - err("set interface failed"); > - > - /* Unblock all USB pipes */ > - usb_clear_halt(d->udev, > - usb_sndbulkpipe(d->udev, > d->props.generic_bulk_ctrl_endpoint)); > - usb_clear_halt(d->udev, > - usb_rcvbulkpipe(d->udev, > d->props.generic_bulk_ctrl_endpoint)); > - usb_clear_halt(d->udev, > - usb_rcvbulkpipe(d->udev, > d->props.adapter[0].fe[0].stream.endpoint)); > - > - /* attach frontend */ > - si2168_config.i2c_adapter = &adapter; > - si2168_config.fe = &adap->fe_adap[0].fe; > - si2168_config.ts_mode = SI2168_TS_PARALLEL; > - si2168_config.ts_clock_inv = 1; > - memset(&info, 0, sizeof(struct i2c_board_info)); > - strscpy(info.type, "si2168", I2C_NAME_SIZE); > - info.addr = 0x64; > - info.platform_data = &si2168_config; > - request_module(info.type); > - client_demod = i2c_new_device(&d->i2c_adap, &info); > - if (client_demod == NULL || client_demod->dev.driver == NULL) > - return -ENODEV; > - > - if (!try_module_get(client_demod->dev.driver->owner)) { > - i2c_unregister_device(client_demod); > - return -ENODEV; > - } > - > - st->i2c_client_demod = client_demod; > - > - /* attach tuner */ > - memset(&si2157_config, 0, sizeof(si2157_config)); > - si2157_config.fe = adap->fe_adap[0].fe;
MyGica T230 dvb-t2 data corruption since commit 5fa8815
(trying again in plain text)... I'm encountering invalid checksum errors randomly in tvheadend with my MyGica T230 DVB-T2 tuner, straight after upgrading from Debian Stretch (kernel 4.9.168) to Buster (kernel 4.19.37). For example 2019-06-29 16:32:27.259 [ INFO]:subscription: 004B: "scan" subscribing to mux "618.167MHz", weight: 6, adapter: "Silicon Labs Si2168 #0 : DVB-T #0", network: "Freeview", service: "Raw PID Subscription" 2019-06-29 16:32:27.769 [WARNING]:tbl-eit: uk_freeview: 618.167MHz in Freeview: invalid checksum (len 1558, errors 1) 2019-06-29 16:32:27.770 [WARNING]:tbl-base: pat: 618.167MHz in Freeview: invalid checksum (len 136, errors 1) 2019-06-29 16:32:27.967 [WARNING]:tbl-base: sdt: 618.167MHz in Freeview: invalid checksum (len 971, errors 1) 2019-06-29 16:32:28.462 [WARNING]:tbl-base: pmt: 618.167MHz in Freeview: invalid checksum (len 60, errors 1) 2019-06-29 16:32:28.463 [WARNING]:tbl-base: pmt: 618.167MHz in Freeview: invalid checksum (len 46, errors 1) 2019-06-29 16:32:28.637 [WARNING]:tbl-base: pmt: 618.167MHz in Freeview: invalid checksum (len 38, errors 1) 2019-06-29 16:32:32.258 [ INFO]:mpegts: 618.167MHz in Freeview scan complete 2019-06-29 16:32:32.258 [ INFO]:subscription: 004B: "scan" unsubscribing 2019-06-29 16:32:32.259 [ INFO]:mpegts: 642MHz in Freeview - tuning on Silicon Labs Si2168 #0 : DVB-T #0 2019-06-29 16:32:32.259 [ INFO]:subscription: 004E: "scan" subscribing to mux "642MHz", weight: 6, adapter: "Silicon Labs Si2168 #0 : DVB-T #0", network: "Freeview", service: "Raw PID Subscription" 2019-06-29 16:32:41.179 [ INFO]:mpegts: 642MHz in Freeview scan complete 2019-06-29 16:32:41.179 [ INFO]:subscription: 004E: "scan" unsubscribing 2019-06-29 16:32:41.180 [ INFO]:mpegts: 602MHz in Freeview - tuning on Silicon Labs Si2168 #0 : DVB-T #0 2019-06-29 16:32:41.180 [ INFO]:subscription: 0052: "scan" subscribing to mux "602MHz", weight: 6, adapter: "Silicon Labs Si2168 #0 : DVB-T #0", network: "Freeview", service: "Raw PID Subscription" 2019-06-29 16:32:41.835 [WARNING]:tbl-eit: uk_freeview: 602MHz in Freeview: invalid checksum (len 320, errors 1) 2019-06-29 16:32:42.109 [WARNING]:tbl-base: sdt: 602MHz in Freeview: invalid checksum (len 322, errors 1) 2019-06-29 16:32:42.314 [WARNING]:tbl-base: pmt: 602MHz in Freeview: invalid checksum (len 84, errors 1) 2019-06-29 16:32:46.180 [ INFO]:mpegts: 602MHz in Freeview scan complete 2019-06-29 16:32:46.180 [ INFO]:subscription: 0052: "scan" unsubscribing The issue is not specific to particular muxes, and happens entirely at random. If I retune then the same mux will scan without issue. It also causes data corruption when streaming with continuity counter errors in the tvh log; again retuning often resolves the problem, but i never had any such issues with kernel v4.9.x. Heres the output of lsusb to confirm my hardware id: 0572:c688 Conexant Systems (Rockwell), Inc. Geniatech T230 DVB-T2 TV Stick I've been testing various kernel versions in-between 4.9->4.19 and quickly found that the issue was introduced somewhere between 4.9->4.10. I therefore performed a (lengthy) git bisect to find out which of the 14,248 commits during the v4.10 development cycle introduced this regression. I have tracked this regression down to the following commit: 5fa8815: [media] dvb-usb-cxusb: Geniatech T230 - resync TS FIFO after lock Indeed, if I revert that commit on an upto-date kernel, I now have a stable MyGica T230 tuner once more. Please note that i've tried the latest media_build drivers and have also the 5.2 kernel; both of which have the issue, and both of which can be fixed by reverting 5fa8815. Regards, James
[PATCH] [dtv-scan-tables] Improve Makefile to avoid "Argument list too long" error
I wouldn't expect a foreach loop to be prone to this issue but it's not the right way to write a Makefile anyway. Note that conversion failures are now fatal and the dvb-t/ke-Nairobi file is failing to convert at the moment. Signed-off-by: James Le Cuirot --- Makefile | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 901dc9d..7cdb5bc 100644 --- a/Makefile +++ b/Makefile @@ -17,6 +17,8 @@ DVBV3DIRS = atsc dvb-c dvb-s dvb-t DVBV5DIRS = $(DVBV3DIRS) isdb-t DVBV3CHANNELFILES = $(foreach dir,$(DVBV3DIRS),$(wildcard $(dir)/*)) +DVBV3OUTPUTFILES = $(patsubst %,$(DVBV3OUTPUTDIR)/%,$(DVBV3CHANNELFILES)) +DVBV5OUTPUTFILES = $(patsubst %,$(DVBV5OUTPUTDIR)/%,$(DVBV3CHANNELFILES)) DVBFORMATCONVERT_CHANNEL_DVBV5 = -ICHANNEL -ODVBV5 DVBFORMATCONVERT_CHANNEL_DVBV3 = -IDVBV5 -OCHANNEL @@ -42,14 +44,16 @@ ifeq ($(DVBV3DIR),) DVBV3DIR = dvbv3 endif -dvbv3: - @$(foreach var,$(DVBV3DIRS), $(MKDIR) $(DVBV3OUTPUTDIR)/$(var);) - @$(foreach var,$(DVBV3CHANNELFILES), $(DVBFORMATCONVERT) $(DVBFORMATCONVERT_CHANNEL_DVBV3) $(var) $(DVBV3OUTPUTDIR)/$(var);) +$(DVBV3OUTPUTFILES): $(DVBV3OUTPUTDIR)/%: % + @$(MKDIR) "$(dir $@)" + @$(DVBFORMATCONVERT) $(DVBFORMATCONVERT_CHANNEL_DVBV3) "$<" "$@" +$(DVBV5OUTPUTFILES): $(DVBV5OUTPUTDIR)/%: $(DVBV3OUTPUTDIR)/% + @$(MKDIR) "$(dir $@)" + @$(DVBFORMATCONVERT) $(DVBFORMATCONVERT_CHANNEL_DVBV5) "$<" "$@" -dvbv5: $(DVBV3OUTPUTDIR) - @$(foreach var,$(DVBV3DIRS), $(MKDIR) $(DVBV5OUTPUTDIR)/$(var);) - @$(foreach var,$(DVBV3CHANNELFILES), $(DVBFORMATCONVERT) $(DVBFORMATCONVERT_CHANNEL_DVBV5) $(DVBV3OUTPUTDIR)/$(var) $(DVBV5OUTPUTDIR)/$(var);) +dvbv3: $(DVBV3OUTPUTFILES) +dvbv5: $(DVBV5OUTPUTFILES) install: @mkdir -p $(DATADIR)/$(DVBV5DIR) -- 2.21.0
Re: [PATCH v3 10/10] media: aspeed: add a workaround to fix a silicon bug
On 6/12/19 2:17 AM, Hans Verkuil wrote: Eddie: ping! Regards, Hans On 6/6/19 2:53 AM, Jae Hyun Yoo wrote: Hi Eddie, All patches in this series were queued to linux/media tree except this one. Can you please review this patch? Thanks, Jae On 5/31/2019 3:15 PM, Jae Hyun Yoo wrote: AST2500 silicon revision A1 and A2 have a silicon bug which causes extremly long capturing time on specific resolutions (1680 width). To fix the bug, this commit adjusts the capturing window register setting to 1728 if detected width is 1680. The compression window register setting will be kept as the original width so output result will be the same. Sorry, missed your followup email Jae and assumed everything was merged. Looks fine to me. Reviewed-by: Eddie James Signed-off-by: Jae Hyun Yoo --- v2 -> v3: Added more detail comments why the value 1728 is picked. v1 -> v2: New. drivers/media/platform/aspeed-video.c | 28 --- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index ba093096a5a7..f899ac3b4a61 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -824,8 +824,29 @@ static void aspeed_video_set_resolution(struct aspeed_video *video) struct v4l2_bt_timings *act = &video->active_timings; unsigned int size = act->width * act->height; + /* Set capture/compression frame sizes */ aspeed_video_calc_compressed_size(video, size); + if (video->active_timings.width == 1680) { + /* +* This is a workaround to fix a silicon bug on A1 and A2 +* revisions. Since it doesn't break capturing operation of +* other revisions, use it for all revisions without checking +* the revision ID. It picked 1728 which is a very next +* 64-pixels aligned value to 1680 to minimize memory bandwidth +* and to get better access speed from video engine. +*/ + aspeed_video_write(video, VE_CAP_WINDOW, + 1728 << 16 | act->height); + size += (1728 - 1680) * video->active_timings.height; + } else { + aspeed_video_write(video, VE_CAP_WINDOW, + act->width << 16 | act->height); + } + aspeed_video_write(video, VE_COMP_WINDOW, + act->width << 16 | act->height); + aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4); + /* Don't use direct mode below 1024 x 768 (irqs don't fire) */ if (size < DIRECT_FETCH_THRESHOLD) { aspeed_video_write(video, VE_TGS_0, @@ -842,13 +863,6 @@ static void aspeed_video_set_resolution(struct aspeed_video *video) aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH); } - /* Set capture/compression frame sizes */ - aspeed_video_write(video, VE_CAP_WINDOW, - act->width << 16 | act->height); - aspeed_video_write(video, VE_COMP_WINDOW, - act->width << 16 | act->height); - aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4); - size *= 4; if (size != video->srcs[0].size) {
Re: [PATCH v2 08/11] media: aspeed: remove source buffer allocation before mode detection
On 5/24/19 6:17 PM, Jae Hyun Yoo wrote: Mode detection doesn't require source buffer allocation so this commit removes that. Reviewed-by: Eddie James Signed-off-by: Jae Hyun Yoo --- v1 -> v2: New. drivers/media/platform/aspeed-video.c | 21 - 1 file changed, 21 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index c0b889141b8f..4647ed2e9e63 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -731,27 +731,6 @@ static void aspeed_video_get_resolution(struct aspeed_video *video) det->height = MIN_HEIGHT; video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL; - /* -* Since we need max buffer size for detection, free the second source -* buffer first. -*/ - if (video->srcs[1].size) - aspeed_video_free_buf(video, &video->srcs[1]); - - if (video->srcs[0].size < VE_MAX_SRC_BUFFER_SIZE) { - if (video->srcs[0].size) - aspeed_video_free_buf(video, &video->srcs[0]); - - if (!aspeed_video_alloc_buf(video, &video->srcs[0], - VE_MAX_SRC_BUFFER_SIZE)) { - dev_err(video->dev, - "Failed to allocate source buffers\n"); - return; - } - } - - aspeed_video_write(video, VE_SRC0_ADDR, video->srcs[0].dma); - do { if (tries) { set_current_state(TASK_INTERRUPTIBLE);
Re: [PATCH v2 09/11] media: aspeed: use different delays for triggering VE H/W reset
On 5/24/19 6:17 PM, Jae Hyun Yoo wrote: In case of watchdog timeout detected while doing mode detection, it's better triggering video engine hardware reset immediately so this commit fixes code for the case. Other than the case, it will trigger video engine hardware reset after RESOLUTION_CHANGE_DELAY. Reviewed-by: Eddie James Signed-off-by: Jae Hyun Yoo --- v1 -> v2: New. drivers/media/platform/aspeed-video.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index 4647ed2e9e63..67f476bf0a03 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -522,7 +522,7 @@ static void aspeed_video_bufs_done(struct aspeed_video *video, spin_unlock_irqrestore(&video->lock, flags); } -static void aspeed_video_irq_res_change(struct aspeed_video *video) +static void aspeed_video_irq_res_change(struct aspeed_video *video, ulong delay) { dev_dbg(video->dev, "Resolution changed; resetting\n"); @@ -532,7 +532,7 @@ static void aspeed_video_irq_res_change(struct aspeed_video *video) aspeed_video_off(video); aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR); - schedule_delayed_work(&video->res_work, RESOLUTION_CHANGE_DELAY); + schedule_delayed_work(&video->res_work, delay); } static irqreturn_t aspeed_video_irq(int irq, void *arg) @@ -545,7 +545,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg) * re-initialize */ if (sts & VE_INTERRUPT_MODE_DETECT_WD) { - aspeed_video_irq_res_change(video); + aspeed_video_irq_res_change(video, 0); return IRQ_HANDLED; } @@ -563,7 +563,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg) * Signal acquired while NOT doing resolution * detection; reset the engine and re-initialize */ - aspeed_video_irq_res_change(video); + aspeed_video_irq_res_change(video, + RESOLUTION_CHANGE_DELAY); return IRQ_HANDLED; } }
Re: [PATCH v2 11/11] media: aspeed: add a workaround to fix a silicon bug
On 5/24/19 6:17 PM, Jae Hyun Yoo wrote: AST2500 silicon revision A1 and A2 have a silicon bug which causes extremly long capturing time on specific resolutions (1680 width). To fix the bug, this commit adjusts the capturing window register setting to 1728 if detected width is 1680. The compression window register setting will be kept as the original width so output result will be the same. This is a bit curious, why 1728 in particular? And what is the behavior of the VE when the capture window is larger than the actual source resolution? Thanks, Eddie Signed-off-by: Jae Hyun Yoo --- v1 -> v2: New. drivers/media/platform/aspeed-video.c | 26 +++--- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index b05b073b63bc..f93989f532d6 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -824,8 +824,27 @@ static void aspeed_video_set_resolution(struct aspeed_video *video) struct v4l2_bt_timings *act = &video->active_timings; unsigned int size = act->width * act->height; + /* Set capture/compression frame sizes */ aspeed_video_calc_compressed_size(video, size); + if (video->active_timings.width == 1680) { + /* +* This is a workaround to fix a silicon bug on A1 and A2 +* revisions. Since it doesn't break capturing operation on A0 +* revision, use it for all revisions without checking the +* revision ID. +*/ + aspeed_video_write(video, VE_CAP_WINDOW, + 1728 << 16 | act->height); + size += (1728 - 1680) * video->active_timings.height; + } else { + aspeed_video_write(video, VE_CAP_WINDOW, + act->width << 16 | act->height); + } + aspeed_video_write(video, VE_COMP_WINDOW, + act->width << 16 | act->height); + aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4); + /* Don't use direct mode below 1024 x 768 (irqs don't fire) */ if (size < DIRECT_FETCH_THRESHOLD) { aspeed_video_write(video, VE_TGS_0, @@ -842,13 +861,6 @@ static void aspeed_video_set_resolution(struct aspeed_video *video) aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH); } - /* Set capture/compression frame sizes */ - aspeed_video_write(video, VE_CAP_WINDOW, - act->width << 16 | act->height); - aspeed_video_write(video, VE_COMP_WINDOW, - act->width << 16 | act->height); - aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4); - size *= 4; if (size == video->srcs[0].size / 2) {
Re: [PATCH v2 10/11] media: aspeed: fix an incorrect timeout checking in mode detection
On 5/24/19 6:17 PM, Jae Hyun Yoo wrote: There is an incorrect timeout checking in mode detection logic so it misses resolution detecting chances. This commit fixes the bug. Signed-off-by: Jae Hyun Yoo --- v1 -> v2: New. drivers/media/platform/aspeed-video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index 67f476bf0a03..b05b073b63bc 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -735,7 +735,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video) do { if (tries) { set_current_state(TASK_INTERRUPTIBLE); - if (schedule_timeout(INVALID_RESOLUTION_DELAY)) + if (!schedule_timeout(INVALID_RESOLUTION_DELAY)) return; schedule_timeout returns 0 when the timer has expired otherwise the remaining time in jiffies will be returned. So if it was interrupted (timer did not expire and it returns non-zero) then we should return, rather than keep trying. So I think it was correct before. Thanks, Eddie }
Re: [PATCH 1/7] media: aspeed: fix a kernel warning on clk control
On 5/8/19 9:16 PM, Benjamin Herrenschmidt wrote: On Wed, 2019-05-08 at 18:19 -0700, Jae Hyun Yoo wrote: I changed that from a bool because the maintainer of this code, Eddie doesn't like adding of an additional flag. I'll change it back with codes in the first submit: https://www.spinics.net/lists/linux-media/msg148955.html Eddie, Please let me know if you have any objection on that. Thats fine with me. I merely thought it was more conventional to use flags rather than bools, but I may be mistaken. Thanks! Eddie Ok, so random flags ... ugh. Well, you can approach it either way. Have them all be bitops or all be bool. The tricky thing however is that if they are bitops you need to ensure that they are *all* manipulated under the same lock. If not you have to use the atomic bitops variants. The reason I don't like that is that experience shows that most uses of such atomic variants in drivers usually are failed attempts at papering over broken locking. If everything is covered by a lock, then using the non-atomic versions is more efficient, but so is using bool (optionally with :1 bitfield qualifiers to avoid wasting memory), which from a pure C language perspective I think is more expressive of what you are doing and more readable. Cheers, Ben.
Re: [PATCH 7/7] media: aspeed: refine interrupt handling logic
On 5/2/19 2:13 PM, Jae Hyun Yoo wrote: There are cases that interrupt bits are cleared by a 500ms delayed work which causes unnecessary irq calls. Also, the current interrupt handler returns IRQ_HANDLED always but it should return IRQ_NONE if there is any unhandled interrupt. So this commit refines the interrupt handling logic to fix these issues. Thanks Jae, looks good. Reviewed-by: Eddie James Signed-off-by: Jae Hyun Yoo Reviewed-by: Andrew Jeffery --- drivers/media/platform/aspeed-video.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index 8d0bb395e46d..98944a911998 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -488,6 +488,7 @@ static void aspeed_video_off(struct aspeed_video *video) /* Disable interrupts */ aspeed_video_write(video, VE_INTERRUPT_CTRL, 0); + aspeed_video_write(video, VE_INTERRUPT_STATUS, 0x); /* Turn off the relevant clocks */ clk_disable(video->vclk); @@ -556,7 +557,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg) VE_INTERRUPT_MODE_DETECT, 0); aspeed_video_write(video, VE_INTERRUPT_STATUS, VE_INTERRUPT_MODE_DETECT); - + sts &= ~VE_INTERRUPT_MODE_DETECT; set_bit(VIDEO_MODE_DETECT_DONE, &video->flags); wake_up_interruptible_all(&video->wait); } else { @@ -601,12 +602,12 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg) VE_INTERRUPT_COMP_COMPLETE, 0); aspeed_video_write(video, VE_INTERRUPT_STATUS, VE_INTERRUPT_COMP_COMPLETE); - + sts &= ~VE_INTERRUPT_COMP_COMPLETE; if (test_bit(VIDEO_STREAMING, &video->flags) && buf) aspeed_video_start_frame(video); } - return IRQ_HANDLED; + return sts ? IRQ_NONE : IRQ_HANDLED; } static void aspeed_video_check_and_set_polarity(struct aspeed_video *video)
Re: [PATCH v2] media: platform: fix a kernel warning on clk control
On 4/12/19 7:17 AM, Herrenschmidt, Benjamin wrote: On Fri, 2019-04-12 at 13:00 +0200, Hans Verkuil wrote: Eddie, can you review this? Yes, this looks good. Thanks, Reviewed-by: Eddie James Jae Hyun Yoo, for future reference: please add the driver name in the subject. I.e. something like this: [PATCH v2] media: aspeed: fix a kernel warning on clk control That helps maintainers figuring out who should look at which patch. test_bit/clear_bit/set_bit are atomic. The way they are used here is either a waste of cycles (there's already a lock) or racy. Not too sure Yes, it's conceivable that this could be done locklessly, but I'm fine with being cautious. I asked Jae to use the bit functions and flag for code cleanliness instead of an additional bool... are the additional cycles that bad here? :) Cheers, Ben. Regards, Hans On 3/29/19 10:27 PM, Jae Hyun Yoo wrote: video_on and video_off functions in the Aspeed video engine driver are being called from multiple contexts without any protection so video clocks can be disabled twice and eventually it causes a kernel warning with stack dump printing out like below: [ 120.034729] WARNING: CPU: 0 PID: 1334 at drivers/clk/clk.c:684 clk_core_unprepare+0x13c/0x170 [ 120.043252] eclk-gate already unprepared [ 120.047283] CPU: 0 PID: 1334 Comm: obmc-ikvm Tainted: GW 5.0.3-b94b74e8b52db91fe4e99e0bb481ec8bf2b5b47c #1 [ 120.058417] Hardware name: Generic DT based system [ 120.063219] Backtrace: [ 120.065787] [<80107cdc>] (dump_backtrace) from [<80107f10>] (show_stack+0x20/0x24) [ 120.073371] r7:803a4ff0 r6:0009 r5: r4:96197e1c [ 120.079152] [<80107ef0>] (show_stack) from [<8068f7d8>] (dump_stack+0x20/0x28) [ 120.086479] [<8068f7b8>] (dump_stack) from [<8011604c>] (__warn.part.3+0xb4/0xdc) [ 120.094068] [<80115f98>] (__warn.part.3) from [<801160e0>] (warn_slowpath_fmt+0x6c/0x90) [ 120.102164] r6:02ac r5:8080c0b8 r4:80a07008 [ 120.106893] [<80116078>] (warn_slowpath_fmt) from [<803a4ff0>] (clk_core_unprepare+0x13c/0x170) [ 120.115686] r3:8080cf8c r2:8080c17c [ 120.119276] r7:97d68e58 r6:9df23200 r5:9668c260 r4:96459260 [ 120.125046] [<803a4eb4>] (clk_core_unprepare) from [<803a707c>] (clk_unprepare+0x34/0x3c) [ 120.133226] r5:9668c260 r4:96459260 [ 120.136932] [<803a7048>] (clk_unprepare) from [<804f34bc>] (aspeed_video_off+0x44/0x48) [ 120.145031] r5:9668c260 r4:9668cbc0 [ 120.148647] [<804f3478>] (aspeed_video_off) from [<804f3fd0>] (aspeed_video_release+0x94/0x118) [ 120.157435] r5:966a0cb8 r4:966a0800 [ 120.161049] [<804f3f3c>] (aspeed_video_release) from [<804d2c58>] (v4l2_release+0xd4/0xe8) [ 120.169404] r7:97d68e58 r6:9d087810 r5:9df23200 r4:966a0b20 [ 120.175168] [<804d2b84>] (v4l2_release) from [<80236224>] (__fput+0x98/0x1c4) [ 120.182316] r5:96698e78 r4:9df23200 [ 120.185994] [<8023618c>] (__fput) from [<802363b8>] (fput+0x18/0x1c) [ 120.192712] r9:80a0700c r8:801011e4 r7: r6:80a64bbc r5:961dd560 r4:961dd89c [ 120.200562] [<802363a0>] (fput) from [<80131c08>] (task_work_run+0x7c/0xa4) [ 120.207994] [<80131b8c>] (task_work_run) from [<80106884>] (do_work_pending+0x4a8/0x578) [ 120.216163] r7:801011e4 r6:80a07008 r5:96197fb0 r4:e000 [ 120.221856] [<801063dc>] (do_work_pending) from [<8010106c>] (slow_work_pending+0xc/0x20) [ 120.230116] Exception stack(0x96197fb0 to 0x96197ff8) [ 120.235254] 7fa0: 76ccf094 [ 120.243438] 7fc0: 0008 00a11978 7eab3c30 0006 475b0fa4 [ 120.251692] 7fe0: 0002 7eab3a40 47720e38 8010 0008 [ 120.258396] r10: r9:96196000 r8:801011e4 r7:0006 r6:7eab3c30 r5:00a11978 [ 120.266291] r4:0008 To prevent this issue, this commit adds spinlock protection and clock status checking logic into the Aspeed video engine driver. Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver") Signed-off-by: Jae Hyun Yoo Cc: Eddie James Cc: Mauro Carvalho Chehab Cc: Joel Stanley Cc: Andrew Jeffery --- drivers/media/platform/aspeed-video.c | 32 --- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index 692e08ef38c0..744d22ecc620 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -188,6 +188,7 @@ enum { VIDEO_STREAMING, VIDEO_FRAME_INPRG, VIDEO_STOPPED, + VIDEO_CLOCKS_ON, }; struct aspeed_video_addr { @@ -495,20 +496,30 @@ static void aspeed_video_reset(struct aspeed_video *video) static void aspeed_video_off(struct aspeed_video *video) { + if (!test_bit(VIDEO_CLOCKS_ON, &
Re: [PATCH] media: platform: Fix a kernel warning on clk control
On 3/28/19 4:25 PM, Jae Hyun Yoo wrote: Video engine clock control functions in the Aspeed video engine driver are being called from multiple context without any protection so video clocks can be disabled twice and eventually it causes a kernel warning with stack dump printing out like below: [ 120.034729] WARNING: CPU: 0 PID: 1334 at drivers/clk/clk.c:684 clk_core_unprepare+0x13c/0x170 [ 120.043252] eclk-gate already unprepared [ 120.047283] CPU: 0 PID: 1334 Comm: obmc-ikvm Tainted: GW 5.0.3-b94b74e8b52db91fe4e99e0bb481ec8bf2b5b47c #1 [ 120.058417] Hardware name: Generic DT based system [ 120.063219] Backtrace: [ 120.065787] [<80107cdc>] (dump_backtrace) from [<80107f10>] (show_stack+0x20/0x24) [ 120.073371] r7:803a4ff0 r6:0009 r5: r4:96197e1c [ 120.079152] [<80107ef0>] (show_stack) from [<8068f7d8>] (dump_stack+0x20/0x28) [ 120.086479] [<8068f7b8>] (dump_stack) from [<8011604c>] (__warn.part.3+0xb4/0xdc) [ 120.094068] [<80115f98>] (__warn.part.3) from [<801160e0>] (warn_slowpath_fmt+0x6c/0x90) [ 120.102164] r6:02ac r5:8080c0b8 r4:80a07008 [ 120.106893] [<80116078>] (warn_slowpath_fmt) from [<803a4ff0>] (clk_core_unprepare+0x13c/0x170) [ 120.115686] r3:8080cf8c r2:8080c17c [ 120.119276] r7:97d68e58 r6:9df23200 r5:9668c260 r4:96459260 [ 120.125046] [<803a4eb4>] (clk_core_unprepare) from [<803a707c>] (clk_unprepare+0x34/0x3c) [ 120.133226] r5:9668c260 r4:96459260 [ 120.136932] [<803a7048>] (clk_unprepare) from [<804f34bc>] (aspeed_video_off+0x44/0x48) [ 120.145031] r5:9668c260 r4:9668cbc0 [ 120.148647] [<804f3478>] (aspeed_video_off) from [<804f3fd0>] (aspeed_video_release+0x94/0x118) [ 120.157435] r5:966a0cb8 r4:966a0800 [ 120.161049] [<804f3f3c>] (aspeed_video_release) from [<804d2c58>] (v4l2_release+0xd4/0xe8) [ 120.169404] r7:97d68e58 r6:9d087810 r5:9df23200 r4:966a0b20 [ 120.175168] [<804d2b84>] (v4l2_release) from [<80236224>] (__fput+0x98/0x1c4) [ 120.182316] r5:96698e78 r4:9df23200 [ 120.185994] [<8023618c>] (__fput) from [<802363b8>] (fput+0x18/0x1c) [ 120.192712] r9:80a0700c r8:801011e4 r7: r6:80a64bbc r5:961dd560 r4:961dd89c [ 120.200562] [<802363a0>] (fput) from [<80131c08>] (task_work_run+0x7c/0xa4) [ 120.207994] [<80131b8c>] (task_work_run) from [<80106884>] (do_work_pending+0x4a8/0x578) [ 120.216163] r7:801011e4 r6:80a07008 r5:96197fb0 r4:e000 [ 120.221856] [<801063dc>] (do_work_pending) from [<8010106c>] (slow_work_pending+0xc/0x20) [ 120.230116] Exception stack(0x96197fb0 to 0x96197ff8) [ 120.235254] 7fa0: 76ccf094 [ 120.243438] 7fc0: 0008 00a11978 7eab3c30 0006 475b0fa4 [ 120.251692] 7fe0: 0002 7eab3a40 47720e38 8010 0008 [ 120.258396] r10: r9:96196000 r8:801011e4 r7:0006 r6:7eab3c30 r5:00a11978 [ 120.266291] r4:0008 To prevent this issue, this commit adds spinlock protection and clock status checking logic into the Aspeed video engine driver. Thanks Jae. Do you have a reliable way to reproduce this? Haven't seen it myself. Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver") Signed-off-by: Jae Hyun Yoo Cc: Eddie James Cc: Mauro Carvalho Chehab Cc: Joel Stanley Cc: Andrew Jeffery --- drivers/media/platform/aspeed-video.c | 46 --- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c index 692e08ef38c0..9663ba4281a8 100644 --- a/drivers/media/platform/aspeed-video.c +++ b/drivers/media/platform/aspeed-video.c @@ -227,6 +227,7 @@ struct aspeed_video { struct list_head buffers; unsigned long flags; unsigned int sequence; + bool is_video_on; This can probably be a flag, like VIDEO_CLOCKS_ON, not a new boolean. unsigned int max_compressed_size; struct aspeed_video_addr srcs[2]; @@ -495,20 +496,28 @@ static void aspeed_video_reset(struct aspeed_video *video) static void aspeed_video_off(struct aspeed_video *video) { - aspeed_video_reset(video); + if (video->is_video_on) { + aspeed_video_reset(video); I'm working on a patch to remove the use of the reset line too. + + /* Turn off the relevant clocks */ + clk_disable_unprepare(video->vclk); + clk_disable_unprepare(video->eclk); - /* Turn off the relevant clocks */ - clk_disable_unprepare(video->vclk); - clk_disable_unprepare(video->eclk); + video->is_video_on = false; + } } static void aspeed_video_on(struct aspeed_video *video) { - /* Turn on the relevant clocks */ -
[PATCH zbar 1/1] v4l2: add fallback for systems without v4l2_query_ext_ctrl
From: James Hilliard Signed-off-by: James Hilliard --- zbar/video/v4l2.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/zbar/video/v4l2.c b/zbar/video/v4l2.c index 0d18094..1610b95 100644 --- a/zbar/video/v4l2.c +++ b/zbar/video/v4l2.c @@ -55,6 +55,28 @@ #define V4L2_FORMATS_MAX 64 #define V4L2_FORMATS_SIZE_MAX 256 +#ifndef VIDIOC_QUERY_EXT_CTRL +#define VIDIOC_QUERY_EXT_CTRL _IOWR('V', 103, struct v4l2_query_ext_ctrl) +#define V4L2_CTRL_MAX_DIMS(4) + +struct v4l2_query_ext_ctrl { +__u32id; +__u32type; +char name[32]; +__s64minimum; +__s64maximum; +__u64step; +__s64default_value; +__u32flags; +__u32elem_size; +__u32elems; +__u32nr_of_dims; +__u32dims[V4L2_CTRL_MAX_DIMS]; +__u32reserved[32]; +}; + +#endif + typedef struct video_controls_priv_s { struct video_controls_s s; -- 2.7.4
Re: [PATCH] media: m88ds3103: serialize reset messages in m88ds3103_set_frontend
On Sun, Jan 20, 2019 at 04:43:08PM +0200, Antti Palosaari wrote: > On 1/13/19 11:13 PM, James Hutchinson wrote: > > Ref: https://bugzilla.kernel.org/show_bug.cgi?id=199323 > > > > Users are experiencing problems with the DVBSky S960/S960C USB devices > > since the following commit: > > > > 9d659ae: ("locking/mutex: Add lock handoff to avoid starvation") > > > > The device malfunctions after running for an indeterminable period of > > time, and the problem can only be cleared by rebooting the machine. > > > > It is possible to encourage the problem to surface by blocking the > > signal to the LNB. > > > > Further debugging revealed the cause of the problem. > > > > In the following capture: > > - thread #1325 is running m88ds3103_set_frontend > > - thread #42 is running ts2020_stat_work > > > > a> [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 07 80 > > [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 08 > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 68 3f > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 08 ff > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 60 3d > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 ff > > b> [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 07 00 > > [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 07 > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 60 21 > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 ff > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 > > [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 60 66 > > [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 ff > > [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 > > [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 07 > > [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 60 02 10 0b > > [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 07 > > > > Two i2c messages are sent to perform a reset in m88ds3103_set_frontend: > > > >a. 0x07, 0x80 > >b. 0x07, 0x00 > > > > However, as shown in the capture, the regmap mutex is being handed over > > to another thread (ts2020_stat_work) in between these two messages. > > > > > From here, the device responds to every i2c message with an 07 message, > > and will only return to normal operation following a power cycle. > > > > Use regmap_multi_reg_write to group the two reset messages, ensuring > > both are processed before the regmap mutex is unlocked. > > I tried to reproduce that issue with pctv 461e, which has em28xx > usb-interface, but without success. Even when I added some sleep between > reset commands and increased tuner statistic polling interval such that it > polls all the time, it works correctly. Device has tuner is connected to > demod i2c bus, which I think is same for your device (it calls demod i2c mux > select for every tuner i2c access). > > Taking into account tests I made it is probably issue with usb-interface i2c > adapter instead - for some reason it stops working and starts returning 07 > error all the time. Did any other I2C command succeed after failure? I mean > is there any other i2c client on that bus you could test if it fails too on > error situation? > > All in all, fix should be done to usb-interface i2c adapter if possible > unless it has proven issue is somewhere else. You could try to add some > sleep or repeat to i2c adapter in order to see if it helps. > > regards > Antti > > -- > http://palosaari.fi/ Thanks for taking the time to review my patch. My device is the dvbsky usb s960 which is a pretty popular device and hasn't been working for several users since commit 9d659ae. I did some further investigation and can now see that the issue likely only affects adapters which use the m88ds3103_get_agc_pwm function to get the AGC from the demodulator as part of ts2020_stat_work. This is the 3f message in my original capture, which gets an ff response. [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 68 3f [42] usb 1-1: dvb_usb_v2_generic_io: <<< 08 ff The m88ds3103_get_agc_pwm function looks to be used by a subset of devices and their vari
[PATCH zbar v2 1/1] Add simple dbus IPC API to zbarcam.
From: James Hilliard This is useful for running zbarcam as a systemd service so that other applications can receive scan messages through dbus. Signed-off-by: James Hilliard --- .gitignore | 1 + Makefile.am| 6 +++ configure.ac | 30 ++ dbus/org.linuxtv.Zbar.conf | 19 + include/zbar.h | 8 zbar/Makefile.am.inc | 4 ++ zbar/processor.c | 99 ++ zbar/processor.h | 4 ++ zbarcam/zbarcam.c | 14 +++ zbarimg/zbarimg.c | 14 +++ 10 files changed, 199 insertions(+) create mode 100644 dbus/org.linuxtv.Zbar.conf diff --git a/.gitignore b/.gitignore index 0d716e3..f7a0090 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ gtk/zbarmarshal.c gtk/zbarmarshal.h include/config.h include/config.h.in +include/config.h.in~ include/stamp-h1 libtool pygtk/zbarpygtk.c diff --git a/Makefile.am b/Makefile.am index 624dcde..62e516a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -50,6 +50,12 @@ if HAVE_DOC include $(srcdir)/doc/Makefile.am.inc endif +if HAVE_DBUS +dbusconfdir = @DBUS_CONFDIR@ +dbusconf_DATA = $(srcdir)/dbus/org.linuxtv.Zbar.conf +EXTRA_DIST += $(dbusconf_DATA) +endif + EXTRA_DIST += zbar.ico zbar.nsi EXTRA_DIST += examples/barcode.png examples/upcrpc.py examples/upcrpc.pl \ diff --git a/configure.ac b/configure.ac index 2633b48..2398da5 100644 --- a/configure.ac +++ b/configure.ac @@ -291,6 +291,35 @@ specify XV_LIBS or configure --without-xv to disable the extension])], ]) AM_CONDITIONAL([HAVE_XV], [test "x$with_xv" = "xyes"]) +dnl dbus +AC_ARG_WITH([dbus], + [AS_HELP_STRING([--without-dbus], +[disable support for dbus])], + [], + [with_dbus="check"]) + +AS_IF([test "x$with_dbus" != "xno"], + [PKG_CHECK_MODULES(DBUS, dbus-1 >= 1.0, have_dbus=yes, have_dbus=no) + AS_IF([test "x$have_dbusx$with_dbus" = "xnoxyes"], +[AC_MSG_FAILURE([DBus development libraries not found])], +[with_dbus="$have_dbus"]) +]) +AM_CONDITIONAL([HAVE_DBUS], [test "x$with_dbus" = "xyes"]) + +AS_IF([test "x$with_dbus" = "xyes"], + [CPPFLAGS="$CPPFLAGS $DBUS_CFLAGS" + AC_ARG_VAR([DBUS_LIBS], [linker flags for building dbus]) + AC_DEFINE([HAVE_DBUS], [1], [Define to 1 to use dbus]) + AC_ARG_WITH(dbusconfdir, AC_HELP_STRING([--with-dbusconfdir=PATH], + [path to D-Bus config directory]), + [path_dbusconf=$withval], + [path_dbusconf="`$PKG_CONFIG --variable=sysconfdir dbus-1`"]) + AS_IF([test -z "$path_dbusconf"], +DBUS_CONFDIR="$sysconfdir/dbus-1/system.d", +DBUS_CONFDIR="$path_dbusconf/dbus-1/system.d") + AC_SUBST(DBUS_CONFDIR) +]) + dnl libjpeg AC_ARG_WITH([jpeg], [AS_HELP_STRING([--without-jpeg], @@ -617,6 +646,7 @@ AS_IF([test "x$enable_video" != "xyes"], [echo "=> zbarcam video scanner will *NOT* be built"]) AS_IF([test "x$have_libv4l" != "xyes"], [echo "=> libv4l will *NOT* be used"]) +echo "dbus --with-dbus=$with_dbus" echo "jpeg --with-jpeg=$with_jpeg" AS_IF([test "x$with_jpeg" != "xyes"], [echo "=> JPEG image conversions will *NOT* be supported"]) diff --git a/dbus/org.linuxtv.Zbar.conf b/dbus/org.linuxtv.Zbar.conf new file mode 100644 index 000..75304c4 --- /dev/null +++ b/dbus/org.linuxtv.Zbar.conf @@ -0,0 +1,19 @@ +http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd";> + + + + + + + + + + + + + diff --git a/include/zbar.h b/include/zbar.h index 7138a95..e999fa8 100644 --- a/include/zbar.h +++ b/include/zbar.h @@ -985,6 +985,14 @@ extern int zbar_process_one(zbar_processor_t *processor, extern int zbar_process_image(zbar_processor_t *processor, zbar_image_t *image); +/** enable dbus IPC API. + * @returns 0 succesful + */ +#ifdef HAVE_DBUS +int zbar_processor_request_dbus (zbar_processor_t *proc, + int req_dbus_enabled); +#endif + /** display detail for last processor error to stderr. * @returns a non-zero value suitable for passing to exit() */ diff --git a/zbar/Makefile.am.inc b/zbar/Makefile.am.inc index 7806295..85d05c2 100644 --- a/zbar/Makefile.am.inc +++ b/zbar/Makefile.am.inc @@ -106,5 +106,9 @@ zbar_libzbar_la_SOURCES += zbar/processor/null.c zbar/window/null.c endif endif +if HAVE_DBUS +zbar_libzbar_la_LDFLAGS += $(DBUS_LIBS) +endif + zbar_libzbar_la_LDFLAGS += $(AM_LDFLAGS) zbar_libzbar_la_LIBADD += $(AM_LIBADD) diff --git a/zbar/processor.c b/zbar/processor.c index 1c7a150..7938d54 100644 --- a/zbar/processor.c +++ b/zbar/processor.c @@ -25,6 +25,10 @@ #include "
Re: [PATCH zbar 1/1] v4l2: add fallback for systems without V4L2_CTRL_WHICH_CUR_VAL
On Wed, Jan 16, 2019 at 2:08 PM Mauro Carvalho Chehab wrote: > > Em Wed, 16 Jan 2019 13:34:29 -0700 > James Hilliard escreveu: > > > > See the enclosed patch. I tested it here with Kernel 4.20 and works > > > fine. > > I'll test it on the system I had which needed the fallback. > > Ok. Please let me know once you test it. Tested now, looks good to me. > > > Thanks, > Mauro
Re: [PATCH zbar 1/1] Add simple dbus IPC API to zbarcam.
On Wed, Jan 16, 2019 at 7:37 AM Mauro Carvalho Chehab wrote: > > Hi James, > > Em Wed, 16 Jan 2019 16:54:55 +0800 > james.hillia...@gmail.com escreveu: > > > From: James Hilliard > > > > This is useful for running zbarcam as a systemd service so that other > > applications can receive scan messages through dbus. > > Nice approach! > > Yet, I would try to write it on a different way, making sure that it > could also be using by zbarimg. > > I mean, if you add the dbus bindings inside the zbar core, it > shouldn't matter if the source image comes via a webcam or via a > scanned image. Both will be able to send the scancodes via dbus. Which function should I call send_dbus() from in that case? > > As a future approach, we may even think on making the interface > duplex in the future, e. g. allowing any camera application to > send an image via dbus and let zbar to decode it and return > the decoded bar codes. That could be useful, probably too difficult for me to implement myself though(I don't write a lot of c usually). > > > > > Signed-off-by: James Hilliard > > --- > > Makefile.am| 6 > > configure.ac | 30 > > dbus/org.linuxtv.Zbar.conf | 19 + > > zbarcam/Makefile.am.inc| 4 +++ > > zbarcam/zbarcam.c | 70 > > ++ > > 5 files changed, 129 insertions(+) > > create mode 100644 dbus/org.linuxtv.Zbar.conf > > > > diff --git a/Makefile.am b/Makefile.am > > index 624dcde..62e516a 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -50,6 +50,12 @@ if HAVE_DOC > > include $(srcdir)/doc/Makefile.am.inc > > endif > > > > +if HAVE_DBUS > > +dbusconfdir = @DBUS_CONFDIR@ > > +dbusconf_DATA = $(srcdir)/dbus/org.linuxtv.Zbar.conf > > +EXTRA_DIST += $(dbusconf_DATA) > > +endif > > + > > EXTRA_DIST += zbar.ico zbar.nsi > > > > EXTRA_DIST += examples/barcode.png examples/upcrpc.py examples/upcrpc.pl \ > > diff --git a/configure.ac b/configure.ac > > index 2633b48..2398da5 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -291,6 +291,35 @@ specify XV_LIBS or configure --without-xv to disable > > the extension])], > > ]) > > AM_CONDITIONAL([HAVE_XV], [test "x$with_xv" = "xyes"]) > > > > +dnl dbus > > +AC_ARG_WITH([dbus], > > + [AS_HELP_STRING([--without-dbus], > > +[disable support for dbus])], > > + [], > > + [with_dbus="check"]) > > + > > +AS_IF([test "x$with_dbus" != "xno"], > > + [PKG_CHECK_MODULES(DBUS, dbus-1 >= 1.0, have_dbus=yes, have_dbus=no) > > + AS_IF([test "x$have_dbusx$with_dbus" = "xnoxyes"], > > +[AC_MSG_FAILURE([DBus development libraries not found])], > > +[with_dbus="$have_dbus"]) > > +]) > > +AM_CONDITIONAL([HAVE_DBUS], [test "x$with_dbus" = "xyes"]) > > + > > +AS_IF([test "x$with_dbus" = "xyes"], > > + [CPPFLAGS="$CPPFLAGS $DBUS_CFLAGS" > > + AC_ARG_VAR([DBUS_LIBS], [linker flags for building dbus]) > > + AC_DEFINE([HAVE_DBUS], [1], [Define to 1 to use dbus]) > > + AC_ARG_WITH(dbusconfdir, AC_HELP_STRING([--with-dbusconfdir=PATH], > > + [path to D-Bus config directory]), > > + [path_dbusconf=$withval], > > + [path_dbusconf="`$PKG_CONFIG --variable=sysconfdir dbus-1`"]) > > + AS_IF([test -z "$path_dbusconf"], > > +DBUS_CONFDIR="$sysconfdir/dbus-1/system.d", > > +DBUS_CONFDIR="$path_dbusconf/dbus-1/system.d") > > + AC_SUBST(DBUS_CONFDIR) > > +]) > > + > > dnl libjpeg > > AC_ARG_WITH([jpeg], > >[AS_HELP_STRING([--without-jpeg], > > @@ -617,6 +646,7 @@ AS_IF([test "x$enable_video" != "xyes"], > >[echo "=> zbarcam video scanner will *NOT* be built"]) > > AS_IF([test "x$have_libv4l" != "xyes"], > >[echo "=> libv4l will *NOT* be used"]) > > +echo "dbus --with-dbus=$with_dbus" > > echo "jpeg --with-jpeg=$with_jpeg" > > AS_IF([test "x$with_jpeg" != "xyes"], > >[echo "=> JPEG image conversions will *NOT* be supported"]) > > diff --git a/dbus/org.linuxtv.Zbar.conf b/dbus/org.linuxtv.Zbar.conf > > new file mode 100644 > > index 000..e0a5ea5 > > --- /dev/null > > +++ b/dbus/org.linuxt
Re: [PATCH zbar 1/1] v4l2: add fallback for systems without V4L2_CTRL_WHICH_CUR_VAL
On Wed, Jan 16, 2019 at 7:24 AM Mauro Carvalho Chehab wrote: > > Em Wed, 16 Jan 2019 13:23:10 +0800 > james.hillia...@gmail.com escreveu: > > > From: James Hilliard > > > > Some older systems don't seem to have V4L2_CTRL_WHICH_CUR_VAL so add a > > fallback. > > Nice catch. > > > > > Signed-off-by: James Hilliard > > --- > > zbar/video/v4l2.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/zbar/video/v4l2.c b/zbar/video/v4l2.c > > index 0147cb1..b883ecc 100644 > > --- a/zbar/video/v4l2.c > > +++ b/zbar/video/v4l2.c > > @@ -866,7 +866,11 @@ static int v4l2_s_control(zbar_video_t *vdo, > > > > memset(&ctrls, 0, sizeof(ctrls)); > > ctrls.count = 1; > > +#ifdef V4L2_CTRL_WHICH_CUR_VAL > > ctrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > +#else > > +ctrls.ctrl_class = V4L2_CTRL_CLASS_USER; > > +#endif > > ctrls.controls = &c; > > > > memset(&c, 0, sizeof(c)); > > @@ -914,7 +918,11 @@ static int v4l2_g_control(zbar_video_t *vdo, > > > > memset(&ctrls, 0, sizeof(ctrls)); > > ctrls.count = 1; > > +#ifdef V4L2_CTRL_WHICH_CUR_VAL > > ctrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > +#else > > +ctrls.ctrl_class = V4L2_CTRL_CLASS_USER; > > +#endif > > ctrls.controls = &c; > > > > memset(&c, 0, sizeof(c)); > > Hmm... This won't work if the control doesn't belong to V4L2_CTRL_CLASS_USER. > Depending on the device, it may have some controls on different classes. Yeah, I wasn't sure my fix was correct. > > So, it would be better to get the control class from its ID. > > Also, there's still a risk that someone would build zbar against > a Kernel > 4.4, and run it with an older Kernel. > > So, IMHO, the best is to also fill ctrls.which from the control > ID. There is a macro for such purpose. > > As the Kernel keeps backward-compatibility, with this approach, > it should work with any Kernel, even if someone, for example, builds it > on 4.20 and tries to run on a 2.6.x Kernel. > > See the enclosed patch. I tested it here with Kernel 4.20 and works > fine. I'll test it on the system I had which needed the fallback. > > Thanks, > Mauro > > v4l2: add fallback for systems without v4l2_ext_controls which field > > The v4l2_ext_controls.which field was introduced on Kernel 4.4, > in order to solve some ambiguities and make easier to handle > controls. > > Yet, there are several systems running older Kernels. As the > newer Linux Kernels are backward-compatible with the old way, > we can change the logic in a way that would allow someone to > build it against a kernel > 4.4, while letting it to keep running > with legacy Kernels. I needed the fix for a 4.4.0 kernel(ubuntu 16.04). > > Reported-by: James Hilliard > Signed-off-by: Mauro Carvalho Chehab > > diff --git a/zbar/video/v4l2.c b/zbar/video/v4l2.c > index 0147cb18d499..0d180947945f 100644 > --- a/zbar/video/v4l2.c > +++ b/zbar/video/v4l2.c > @@ -866,7 +866,11 @@ static int v4l2_s_control(zbar_video_t *vdo, > > memset(&ctrls, 0, sizeof(ctrls)); > ctrls.count = 1; > -ctrls.which = V4L2_CTRL_WHICH_CUR_VAL; > +#ifdef V4L2_CTRL_ID2WHICH > +ctrls.which = V4L2_CTRL_ID2WHICH(p->id); > +#else > +ctrls.ctrl_class = V4L2_CTRL_ID2CLASS(p->id); > +#endif > ctrls.controls = &c; > > memset(&c, 0, sizeof(c)); > @@ -914,7 +918,11 @@ static int v4l2_g_control(zbar_video_t *vdo, > > memset(&ctrls, 0, sizeof(ctrls)); > ctrls.count = 1; > -ctrls.which = V4L2_CTRL_WHICH_CUR_VAL; > +#ifdef V4L2_CTRL_ID2WHICH > +ctrls.which = V4L2_CTRL_ID2WHICH(p->id); > +#else > +ctrls.ctrl_class = V4L2_CTRL_ID2CLASS(p->id); > +#endif > ctrls.controls = &c; > > memset(&c, 0, sizeof(c));
[PATCH zbar 1/1] Add simple dbus IPC API to zbarcam.
From: James Hilliard This is useful for running zbarcam as a systemd service so that other applications can receive scan messages through dbus. Signed-off-by: James Hilliard --- Makefile.am| 6 configure.ac | 30 dbus/org.linuxtv.Zbar.conf | 19 + zbarcam/Makefile.am.inc| 4 +++ zbarcam/zbarcam.c | 70 ++ 5 files changed, 129 insertions(+) create mode 100644 dbus/org.linuxtv.Zbar.conf diff --git a/Makefile.am b/Makefile.am index 624dcde..62e516a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -50,6 +50,12 @@ if HAVE_DOC include $(srcdir)/doc/Makefile.am.inc endif +if HAVE_DBUS +dbusconfdir = @DBUS_CONFDIR@ +dbusconf_DATA = $(srcdir)/dbus/org.linuxtv.Zbar.conf +EXTRA_DIST += $(dbusconf_DATA) +endif + EXTRA_DIST += zbar.ico zbar.nsi EXTRA_DIST += examples/barcode.png examples/upcrpc.py examples/upcrpc.pl \ diff --git a/configure.ac b/configure.ac index 2633b48..2398da5 100644 --- a/configure.ac +++ b/configure.ac @@ -291,6 +291,35 @@ specify XV_LIBS or configure --without-xv to disable the extension])], ]) AM_CONDITIONAL([HAVE_XV], [test "x$with_xv" = "xyes"]) +dnl dbus +AC_ARG_WITH([dbus], + [AS_HELP_STRING([--without-dbus], +[disable support for dbus])], + [], + [with_dbus="check"]) + +AS_IF([test "x$with_dbus" != "xno"], + [PKG_CHECK_MODULES(DBUS, dbus-1 >= 1.0, have_dbus=yes, have_dbus=no) + AS_IF([test "x$have_dbusx$with_dbus" = "xnoxyes"], +[AC_MSG_FAILURE([DBus development libraries not found])], +[with_dbus="$have_dbus"]) +]) +AM_CONDITIONAL([HAVE_DBUS], [test "x$with_dbus" = "xyes"]) + +AS_IF([test "x$with_dbus" = "xyes"], + [CPPFLAGS="$CPPFLAGS $DBUS_CFLAGS" + AC_ARG_VAR([DBUS_LIBS], [linker flags for building dbus]) + AC_DEFINE([HAVE_DBUS], [1], [Define to 1 to use dbus]) + AC_ARG_WITH(dbusconfdir, AC_HELP_STRING([--with-dbusconfdir=PATH], + [path to D-Bus config directory]), + [path_dbusconf=$withval], + [path_dbusconf="`$PKG_CONFIG --variable=sysconfdir dbus-1`"]) + AS_IF([test -z "$path_dbusconf"], +DBUS_CONFDIR="$sysconfdir/dbus-1/system.d", +DBUS_CONFDIR="$path_dbusconf/dbus-1/system.d") + AC_SUBST(DBUS_CONFDIR) +]) + dnl libjpeg AC_ARG_WITH([jpeg], [AS_HELP_STRING([--without-jpeg], @@ -617,6 +646,7 @@ AS_IF([test "x$enable_video" != "xyes"], [echo "=> zbarcam video scanner will *NOT* be built"]) AS_IF([test "x$have_libv4l" != "xyes"], [echo "=> libv4l will *NOT* be used"]) +echo "dbus --with-dbus=$with_dbus" echo "jpeg --with-jpeg=$with_jpeg" AS_IF([test "x$with_jpeg" != "xyes"], [echo "=> JPEG image conversions will *NOT* be supported"]) diff --git a/dbus/org.linuxtv.Zbar.conf b/dbus/org.linuxtv.Zbar.conf new file mode 100644 index 000..e0a5ea5 --- /dev/null +++ b/dbus/org.linuxtv.Zbar.conf @@ -0,0 +1,19 @@ +http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd";> + + + + + + + + + + + + + diff --git a/zbarcam/Makefile.am.inc b/zbarcam/Makefile.am.inc index 5aa788a..98199ee 100644 --- a/zbarcam/Makefile.am.inc +++ b/zbarcam/Makefile.am.inc @@ -4,6 +4,10 @@ zbarcam_zbarcam_LDADD = zbar/libzbar.la # automake bug in "monolithic mode"? CLEANFILES += zbarcam/.libs/zbarcam zbarcam/moc_zbarcam_qt.h +if HAVE_DBUS +zbarcam_zbarcam_LDADD += $(DBUS_LIBS) +endif + if HAVE_GTK bin_PROGRAMS += zbarcam/zbarcam-gtk check_PROGRAMS += zbarcam/zbarcam-gtk diff --git a/zbarcam/zbarcam.c b/zbarcam/zbarcam.c index e0f3622..18b4fd1 100644 --- a/zbarcam/zbarcam.c +++ b/zbarcam/zbarcam.c @@ -33,6 +33,10 @@ #include +#ifdef HAVE_DBUS +#include +#endif + #define BELL "\a" static const char *note_usage = @@ -92,6 +96,68 @@ static inline int parse_config (const char *cfgstr, int i, int n, char *arg) return(0); } +#ifdef HAVE_DBUS +void send_dbus(const char* sigvalue) +{ +DBusMessage* msg; +DBusMessageIter args; +DBusConnection* conn; +DBusError err; +int ret; +dbus_uint32_t serial = 0; + +// initialise the error value +dbus_error_init(&err); + +// connect to the DBUS system bus, and check for errors +conn = dbus_bus_get(DBUS_BUS_SYSTEM, &err); +if (dbus_error_is_set(&err)) { +fprintf(stderr, "Connection Error (%s)\n", err.message); +dbus_error_free(&err); +} +if (NULL == conn) { +exit(1); +} + +// register our name on the bus, and check for errors +ret = dbus_bus_request_name(conn, "org.linuxtv.Zbar", DBUS_NAME_FLAG_REPLACE_EXISTING , &a
[PATCH zbar 1/1] v4l2: add fallback for systems without V4L2_CTRL_WHICH_CUR_VAL
From: James Hilliard Some older systems don't seem to have V4L2_CTRL_WHICH_CUR_VAL so add a fallback. Signed-off-by: James Hilliard --- zbar/video/v4l2.c | 8 1 file changed, 8 insertions(+) diff --git a/zbar/video/v4l2.c b/zbar/video/v4l2.c index 0147cb1..b883ecc 100644 --- a/zbar/video/v4l2.c +++ b/zbar/video/v4l2.c @@ -866,7 +866,11 @@ static int v4l2_s_control(zbar_video_t *vdo, memset(&ctrls, 0, sizeof(ctrls)); ctrls.count = 1; +#ifdef V4L2_CTRL_WHICH_CUR_VAL ctrls.which = V4L2_CTRL_WHICH_CUR_VAL; +#else +ctrls.ctrl_class = V4L2_CTRL_CLASS_USER; +#endif ctrls.controls = &c; memset(&c, 0, sizeof(c)); @@ -914,7 +918,11 @@ static int v4l2_g_control(zbar_video_t *vdo, memset(&ctrls, 0, sizeof(ctrls)); ctrls.count = 1; +#ifdef V4L2_CTRL_WHICH_CUR_VAL ctrls.which = V4L2_CTRL_WHICH_CUR_VAL; +#else +ctrls.ctrl_class = V4L2_CTRL_CLASS_USER; +#endif ctrls.controls = &c; memset(&c, 0, sizeof(c)); -- 2.7.4
Re: [PATCH zbar 1/5] Fix autoreconf by reducing the warning/error checking
On Mon, Jan 14, 2019 at 4:00 AM Mauro Carvalho Chehab wrote: > > Hi James, > > Em Mon, 14 Jan 2019 07:38:25 +0800 > james.hillia...@gmail.com escreveu: > > > From: Thomas Petazzoni > > > > Please add description to the patches. It helps reviewing them, and, > if we need to revert it for whatever reason in the future, the git log > will help to take into account the rationale about why the change was > needed in the first place. > > > Signed-off-by: Thomas Petazzoni > > Signed-off-by: James Hilliard > > --- > > configure.ac | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index a03d10e..6476a20 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -5,7 +5,7 @@ m4_ifndef([AC_LANG_DEFINES_PROVIDED], > >[m4_define([AC_LANG_DEFINES_PROVIDED])]) > > AC_CONFIG_AUX_DIR(config) > > AC_CONFIG_MACRO_DIR(config) > > -AM_INIT_AUTOMAKE([1.13 -Werror foreign subdir-objects std-options > > dist-bzip2]) > > +AM_INIT_AUTOMAKE([1.13 foreign subdir-objects std-options dist-bzip2]) > > m4_pattern_allow([AM_PROG_AR]) > > AC_CONFIG_HEADERS([include/config.h]) > > AC_CONFIG_SRCDIR(zbar/scanner.c) > > I applied patches 2 to 5 of this series, but I would prefer to keep the > -Werror here, as it helps to identify and fix potential issues. > > Here (Fedora 29), everything builds fine, but I haven't test on other > distros that could have newer packages. > > Why is this patch needed? It was part of buildroot's zbar patches, not sure if it's needed though anymore. > > Regards, > Mauro > > Thanks, > Mauro
[PATCH zbar 4/5] Ignore ENOTTY errors when calling VIDIOC_S_CROP
From: James Hilliard Silences this error: ERROR: zbar video in v4l2_reset_crop(): system error: setting default crop window (VIDIOC_S_CROP): Inappropriate ioctl for device (25) Signed-off-by: James Hilliard --- zbar/video/v4l2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zbar/video/v4l2.c b/zbar/video/v4l2.c index 1d685e9..ad6adf4 100644 --- a/zbar/video/v4l2.c +++ b/zbar/video/v4l2.c @@ -569,7 +569,7 @@ static inline int v4l2_reset_crop (zbar_video_t *vdo) memset(&crop, 0, sizeof(crop)); crop.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; crop.c = ccap.defrect; -if(v4l2_ioctl(vdo->fd, VIDIOC_S_CROP, &crop) < 0 && errno != EINVAL) +if(v4l2_ioctl(vdo->fd, VIDIOC_S_CROP, &crop) < 0 && errno != EINVAL && errno != ENOTTY) return(err_capture(vdo, SEV_ERROR, ZBAR_ERR_SYSTEM, __func__, "setting default crop window (VIDIOC_S_CROP)")); return(0); -- 2.7.4
[PATCH zbar 3/5] Add --disable-doc configure option to disable building docs
From: James Hilliard Signed-off-by: James Hilliard --- Makefile.am | 2 ++ configure.ac | 10 ++ 2 files changed, 12 insertions(+) diff --git a/Makefile.am b/Makefile.am index df3d79d..b4cae8a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -45,7 +45,9 @@ if HAVE_NPAPI include $(srcdir)/plugin/Makefile.am.inc endif include $(srcdir)/test/Makefile.am.inc +if HAVE_DOC include $(srcdir)/doc/Makefile.am.inc +endif EXTRA_DIST += zbar.ico zbar.nsi diff --git a/configure.ac b/configure.ac index 6476a20..afccc3a 100644 --- a/configure.ac +++ b/configure.ac @@ -177,6 +177,15 @@ or configure --disable-pthread to skip threaded support.])]) AC_DEFINE([__USE_UNIX98], [1], [used only for pthread debug attributes]) ]) +dnl doc +AC_ARG_ENABLE([doc], + [AS_HELP_STRING([--disable-doc], +[disable building docs])], + [], + [enable_doc="yes"]) + +AM_CONDITIONAL([HAVE_DOC], [test "x$enable_doc" != "xno"]) + dnl video AC_ARG_ENABLE([video], [AS_HELP_STRING([--disable-video], @@ -602,6 +611,7 @@ echo "please verify that the detected configuration matches your expectations:" echo "" echo "X --with-x=$have_x" echo "pthreads --enable-pthread=$enable_pthread" +echo "doc --enable-doc=$enable_doc" echo "v4l --enable-video=$enable_video" AS_IF([test "x$enable_video" != "xyes"], [echo "=> zbarcam video scanner will *NOT* be built"]) -- 2.7.4
[PATCH zbar 5/5] release video buffers after probing and request them again when needed
From: James Hilliard Patch adapted from https://bugs.archlinux.org/task/44091 Signed-off-by: James Hilliard --- zbar/video/v4l2.c | 17 + 1 file changed, 17 insertions(+) diff --git a/zbar/video/v4l2.c b/zbar/video/v4l2.c index ad6adf4..ca52e4c 100644 --- a/zbar/video/v4l2.c +++ b/zbar/video/v4l2.c @@ -243,6 +243,21 @@ static int v4l2_mmap_buffers (zbar_video_t *vdo) return(0); } +static int v4l2_request_buffers (zbar_video_t *vdo) +{ +struct v4l2_requestbuffers rb; +memset(&rb, 0, sizeof(rb)); +rb.count = vdo->num_images; +rb.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; +rb.memory = V4L2_MEMORY_USERPTR; +if(v4l2_ioctl(vdo->fd, VIDIOC_REQBUFS, &rb) < 0) +return(err_capture(vdo, SEV_ERROR, ZBAR_ERR_SYSTEM, __func__, + "requesting video frame buffers (VIDIOC_REQBUFS)")); +if(rb.count) +vdo->num_images = rb.count; +return(0); +} + static int v4l2_set_format (zbar_video_t *vdo, uint32_t fmt) { @@ -334,6 +349,8 @@ static int v4l2_init (zbar_video_t *vdo, if(vdo->iomode == VIDEO_MMAP) return(v4l2_mmap_buffers(vdo)); +if(vdo->iomode == VIDEO_USERPTR) +return(v4l2_request_buffers(vdo)); return(0); } -- 2.7.4
[PATCH zbar 1/5] Fix autoreconf by reducing the warning/error checking
From: Thomas Petazzoni Signed-off-by: Thomas Petazzoni Signed-off-by: James Hilliard --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index a03d10e..6476a20 100644 --- a/configure.ac +++ b/configure.ac @@ -5,7 +5,7 @@ m4_ifndef([AC_LANG_DEFINES_PROVIDED], [m4_define([AC_LANG_DEFINES_PROVIDED])]) AC_CONFIG_AUX_DIR(config) AC_CONFIG_MACRO_DIR(config) -AM_INIT_AUTOMAKE([1.13 -Werror foreign subdir-objects std-options dist-bzip2]) +AM_INIT_AUTOMAKE([1.13 foreign subdir-objects std-options dist-bzip2]) m4_pattern_allow([AM_PROG_AR]) AC_CONFIG_HEADERS([include/config.h]) AC_CONFIG_SRCDIR(zbar/scanner.c) -- 2.7.4
[PATCH zbar 2/5] Fix function protoype to be compatible with recent libjpeg
From: Viacheslav Volkov Signed-off-by: Viacheslav Volkov Signed-off-by: Thomas Petazzoni Signed-off-by: James Hilliard --- zbar/jpeg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zbar/jpeg.c b/zbar/jpeg.c index 972bfea..fdd1619 100644 --- a/zbar/jpeg.c +++ b/zbar/jpeg.c @@ -68,7 +68,7 @@ void init_source (j_decompress_ptr cinfo) cinfo->src->bytes_in_buffer = img->datalen; } -int fill_input_buffer (j_decompress_ptr cinfo) +boolean fill_input_buffer (j_decompress_ptr cinfo) { /* buffer underrun error case */ cinfo->src->next_input_byte = fake_eoi; -- 2.7.4
[PATCH] media: m88ds3103: serialize reset messages in m88ds3103_set_frontend
Ref: https://bugzilla.kernel.org/show_bug.cgi?id=199323 Users are experiencing problems with the DVBSky S960/S960C USB devices since the following commit: 9d659ae: ("locking/mutex: Add lock handoff to avoid starvation") The device malfunctions after running for an indeterminable period of time, and the problem can only be cleared by rebooting the machine. It is possible to encourage the problem to surface by blocking the signal to the LNB. Further debugging revealed the cause of the problem. In the following capture: - thread #1325 is running m88ds3103_set_frontend - thread #42 is running ts2020_stat_work a> [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 07 80 [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 08 [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 68 3f [42] usb 1-1: dvb_usb_v2_generic_io: <<< 08 ff [42] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 60 3d [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 ff b> [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 07 00 [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 07 [42] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 60 21 [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 ff [42] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 [42] usb 1-1: dvb_usb_v2_generic_io: >>> 09 01 01 60 66 [42] usb 1-1: dvb_usb_v2_generic_io: <<< 07 ff [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 68 02 03 11 [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 07 [1325] usb 1-1: dvb_usb_v2_generic_io: >>> 08 60 02 10 0b [1325] usb 1-1: dvb_usb_v2_generic_io: <<< 07 Two i2c messages are sent to perform a reset in m88ds3103_set_frontend: a. 0x07, 0x80 b. 0x07, 0x00 However, as shown in the capture, the regmap mutex is being handed over to another thread (ts2020_stat_work) in between these two messages. >From here, the device responds to every i2c message with an 07 message, and will only return to normal operation following a power cycle. Use regmap_multi_reg_write to group the two reset messages, ensuring both are processed before the regmap mutex is unlocked. Signed-off-by: James Hutchinson --- drivers/media/dvb-frontends/m88ds3103.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c index 123f2a3..77fe3dc 100644 --- a/drivers/media/dvb-frontends/m88ds3103.c +++ b/drivers/media/dvb-frontends/m88ds3103.c @@ -309,6 +309,7 @@ static int m88ds3103_set_frontend(struct dvb_frontend *fe) u16 u16tmp; u32 tuner_frequency_khz, target_mclk; s32 s32tmp; + static const struct reg_sequence reset_buf[] = {{0x07, 0x80}, {0x07, 0x00}}; dev_dbg(&client->dev, "delivery_system=%d modulation=%d frequency=%u symbol_rate=%d inversion=%d pilot=%d rolloff=%d\n", @@ -321,11 +322,7 @@ static int m88ds3103_set_frontend(struct dvb_frontend *fe) } /* reset */ - ret = regmap_write(dev->regmap, 0x07, 0x80); - if (ret) - goto err; - - ret = regmap_write(dev->regmap, 0x07, 0x00); + ret = regmap_multi_reg_write(dev->regmap, reset_buf, 2); if (ret) goto err; -- 2.7.4
Re: [PATCH v8 2/2] media: platform: Add Aspeed Video Engine driver
On 12/13/2018 07:09 PM, Joel Stanley wrote: On Wed, 12 Dec 2018 at 04:09, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) We tend to use the linux-aspeed list for upstream kernel discussions. Up to you if you want to use the openbmc list though. source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. This might need updating in response to my questions below about ast2400 testing. +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1729 @@ +// SPDX-License-Identifier: GPL-2.0+ You need to put this there as well: // Copyright 2018 IBM Corp +static int aspeed_video_init(struct aspeed_video *video) +{ + int irq; + int rc; + struct device *dev = video->dev; + + irq = irq_of_parse_and_map(dev->of_node, 0); + if (!irq) { + dev_err(dev, "Unable to find IRQ\n"); + return -ENODEV; + } + + rc = devm_request_irq(dev, irq, aspeed_video_irq, IRQF_SHARED, The datasheet indicates this IRQ is for the video engline only, so I don't think you want IRQF_SHARED. + DEVICE_NAME, video); + if (rc < 0) { + dev_err(dev, "Unable to request IRQ %d\n", irq); + return rc; + } + + video->eclk = devm_clk_get(dev, "eclk"); + if (IS_ERR(video->eclk)) { + dev_err(dev, "Unable to get ECLK\n"); + return PTR_ERR(video->eclk); + } + + video->vclk = devm_clk_get(dev, "vclk"); + if (IS_ERR(video->vclk)) { + dev_err(dev, "Unable to get VCLK\n"); + return PTR_ERR(video->vclk); + } + + video->rst = devm_reset_control_get_exclusive(dev, NULL); + if (IS_ERR(video->rst)) { + dev_err(dev, "Unable to get VE reset\n"); + return PTR_ERR(video->rst); + } As discussed in the clock driver, this can go as you've already released the reset when enabling the eclk. I use the reset control by itself during a resolution change, so I would like to have it available. However, you're requesting the clock without enabling it. You need to do a clk_prepare_enable(). That's because the clock is only enabled when the device is opened. No reason to turn it on at boot time. + + rc = of_reserved_mem_device_init(dev); + if (rc) { + dev_err(dev, "Unable to reserve memory\n"); + return rc; + } + + rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); + if (rc) { + dev_err(dev, "Failed to set DMA mask\n"); + of_reserved_mem_device_release(dev); + return rc; + } + + if (!aspeed_video_alloc_buf(video, &video->jpeg, + VE_JPEG_HEADER_SIZE)) { + dev_err(dev, "Failed to allocate DMA for JPEG header\n"); + of_reserved_mem_device_release(dev); + return rc; + } + + aspeed_video_init_jpeg_table(video->jpeg.virt, video->yuv420); + + return 0; +} + +static const struct of_device_id aspeed_video_of_match[] = { + { .compatible = "aspeed,ast2400-video-engine" }, I noticed the clock driver did not have the changed required for the 2400. Have you tested this on the ast2400? The clocking setup is different on the ast2400. Xiuzhi on the openbmc list has been testing on the ast2400 successfully. Thanks, Eddie + { .compatible = "aspeed,ast2500-video-engine" }, + {} +}; +MODULE_DEVICE_TABLE(of, aspeed_video_of_match); + +static struct platform_driver aspeed_video_driver = { + .driver = { + .name = DEVICE_NAME, + .of_match_table = aspeed_video_of_match, + }, + .probe = aspeed_video_probe, + .remove = aspeed_video_remove, +};
[PATCH v8 2/2] media: platform: Add Aspeed Video Engine driver
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1729 + 4 files changed, 1747 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index 9ceb649..1fd5fbd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2427,6 +2427,14 @@ S: Maintained F: Documentation/hwmon/asc7621 F: drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M: Corentin Chary L: acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ea33063..a505e9f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index d347a55..e6deb25 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE)+= cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..dfec813 --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1729 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 +#define MIN_HEIGHT 480 +#define MIN_WIDTH 640 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(1000) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#define VE_SEQ_CTRL_EN_WATCHDOG_COMP BIT(14) +#define VE_SEQ_CTRL_TRIG_JPG BIT(15) +#defi
[PATCH v8 0/2] media: platform: Add Aspeed Video Engine driver
From: Eddie James The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting as a service processor, the Video Engine can capture the host processor graphics output. This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming interface by way of videobuf2. Each frame, the driver triggers the hardware to capture the host graphics output and compress it to JPEG format. v4l2-compliance SHA: d039b47a108596ca004b11e52989054882d45888, 32 bits Compliance test for device /dev/video0: Driver Info: Driver name : aspeed-video Card type: Aspeed Video Engine Bus info : platform:aspeed-video Driver version : 4.19.6 Capabilities : 0x8521 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x0521 Video Capture Read/Write Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 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 test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 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 test VIDIOC_DV_TIMINGS_CAP: OK test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls (Input 0): 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 warn: v4l2-test-controls.cpp(853): V4L2_CID_DV_RX_POWER_PRESENT not found for input 0 test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 3 Private Controls: 0 Format ioctls (Input 0): test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK 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: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls (Input 0): test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls (Input 0): test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK test blocking wait: OK test MMAP: OK test USERPTR: OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total: 48, Succeeded: 48, Failed: 0, Warnings: 1 I'm unable to run linux-next or the media tree as our system doesn't boot without quite a few patches that aren't yet upstreamed, so I can't grab the fix for that warning Hans mentioned. Changes since v7: - Fix the check for sending the source change event to include acquiring or losing a signal without changing resolution - Fix return value of query_timings to reflect signal status - Added and improved a few comments Changes since v6: - Refactor open/start process such that the file handle can be opened without a video signal present - Added standards and capabilities - Removed used of VB2_BUF_FLAG_LAST - Added a few comments - Removed a couple of functions that were only used once - Removed client counter Changes since v5: - Rework resolution change and v4l2 timings functions again with active and detected timings. - Renamed a few things. - Fixed polarities in the timings. Changes since v4: - Set real min and max resolution in enum_dv_timings - Add check for vb2_is_busy
[PATCH v8 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation
Document the bindings. Signed-off-by: Eddie James Reviewed-by: Rob Herring --- .../devicetree/bindings/media/aspeed-video.txt | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt new file mode 100644 index 000..78b464a --- /dev/null +++ b/Documentation/devicetree/bindings/media/aspeed-video.txt @@ -0,0 +1,26 @@ +* Device tree bindings for Aspeed Video Engine + +The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can +capture and compress video data from digital or analog sources. + +Required properties: + - compatible: "aspeed,ast2400-video-engine" or + "aspeed,ast2500-video-engine" + - reg:contains the offset and length of the VE memory region + - clocks: clock specifiers for the syscon clocks associated with + the VE (ordering must match the clock-names property) + - clock-names:"vclk" and "eclk" + - resets: reset specifier for the syscon reset associated with + the VE + - interrupts: the interrupt associated with the VE on this platform + +Example: + +video-engine@1e70 { +compatible = "aspeed,ast2500-video-engine"; +reg = <0x1e70 0x2>; +clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; +clock-names = "vclk", "eclk"; +resets = <&syscon ASPEED_RESET_VIDEO>; +interrupts = <7>; +}; -- 1.8.3.1
Re: [PATCH v7 2/2] media: platform: Add Aspeed Video Engine driver
On 12/11/2018 04:44 AM, Hans Verkuil wrote: On 12/10/18 11:26 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- +static void aspeed_video_irq_res_change(struct aspeed_video *video) +{ + dev_dbg(video->dev, "Resolution changed; resetting\n"); + + set_bit(VIDEO_RES_CHANGE, &video->flags); + clear_bit(VIDEO_FRAME_INPRG, &video->flags); + + aspeed_video_off(video); + aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR); + + schedule_delayed_work(&video->res_work, RESOLUTION_CHANGE_DELAY); +} + +static irqreturn_t aspeed_video_irq(int irq, void *arg) +{ + struct aspeed_video *video = arg; + u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS); + + /* Resolution changed; reset entire engine and reinitialize */ + if (sts & VE_INTERRUPT_MODE_DETECT_WD) { Does this only trigger when the resolution changed, or also when the signal disappears? For the purpose of this review I assume that it is also triggered when the signal goes away. The comment should be updated in that case that it is not just resolution changes, but also a dropped video signal. That's correct, signal lost or resolution changed, I'll update the comment. + aspeed_video_irq_res_change(video); + return IRQ_HANDLED; + } + + if (sts & VE_INTERRUPT_MODE_DETECT) { + if (test_bit(VIDEO_RES_DETECT, &video->flags)) { + aspeed_video_update(video, VE_INTERRUPT_CTRL, + VE_INTERRUPT_MODE_DETECT, 0); + aspeed_video_write(video, VE_INTERRUPT_STATUS, + VE_INTERRUPT_MODE_DETECT); + + set_bit(VIDEO_MODE_DETECT_DONE, &video->flags); + wake_up_interruptible_all(&video->wait); + } else { + aspeed_video_irq_res_change(video); + return IRQ_HANDLED; + } + } + + if ((sts & VE_INTERRUPT_COMP_COMPLETE) && + (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) { + struct aspeed_video_buffer *buf; + u32 frame_size = aspeed_video_read(video, + VE_OFFSET_COMP_STREAM); + + spin_lock(&video->lock); + clear_bit(VIDEO_FRAME_INPRG, &video->flags); + buf = list_first_entry_or_null(&video->buffers, + struct aspeed_video_buffer, + link); + if (buf) { + vb2_set_plane_payload(&buf->vb.vb2_buf, 0, frame_size); + + if (!list_is_last(&buf->link, &video->buffers)) { + buf->vb.vb2_buf.timestamp = ktime_get_ns(); + buf->vb.sequence = video->sequence++; + buf->vb.field = V4L2_FIELD_NONE; + vb2_buffer_done(&buf->vb.vb2_buf, + VB2_BUF_STATE_DONE); + list_del(&buf->link); + } + } + spin_unlock(&video->lock); + + aspeed_video_update(video, VE_SEQ_CTRL, + VE_SEQ_CTRL_TRIG_CAPTURE | + VE_SEQ_CTRL_FORCE_IDLE | + VE_SEQ_CTRL_TRIG_COMP, 0); + aspeed_video_update(video, VE_INTERRUPT_CTRL, + VE_INTERRUPT_COMP_COMPLETE | + VE_INTERRUPT_CAPTURE_COMPLETE, 0); + aspeed_video_write(video, VE_INTERRUPT_STATUS, + VE_INTERRUPT_COMP_COMPLETE | + VE_INTERRUPT_CAPTURE_COMPLETE); + + if (test_bit(VIDEO_STREAMING, &video->flags) && buf) + aspeed_video_start_frame(video); + } + + return IRQ_HANDLED; +} +static void aspeed_video_get_resolution(struct aspeed_video *video) +{ + bool invalid_resolution = true; + int rc; + int tries = 0; + u32 mds; + u32 src_lr_edge; + u32 src_tb_edge; + u32 sync; + struct v4l2_bt_timings *det = &video->detected_timings; + + det->width = MIN_
[PATCH v7 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation
Document the bindings. Signed-off-by: Eddie James Reviewed-by: Rob Herring --- .../devicetree/bindings/media/aspeed-video.txt | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt new file mode 100644 index 000..78b464a --- /dev/null +++ b/Documentation/devicetree/bindings/media/aspeed-video.txt @@ -0,0 +1,26 @@ +* Device tree bindings for Aspeed Video Engine + +The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can +capture and compress video data from digital or analog sources. + +Required properties: + - compatible: "aspeed,ast2400-video-engine" or + "aspeed,ast2500-video-engine" + - reg:contains the offset and length of the VE memory region + - clocks: clock specifiers for the syscon clocks associated with + the VE (ordering must match the clock-names property) + - clock-names:"vclk" and "eclk" + - resets: reset specifier for the syscon reset associated with + the VE + - interrupts: the interrupt associated with the VE on this platform + +Example: + +video-engine@1e70 { +compatible = "aspeed,ast2500-video-engine"; +reg = <0x1e70 0x2>; +clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; +clock-names = "vclk", "eclk"; +resets = <&syscon ASPEED_RESET_VIDEO>; +interrupts = <7>; +}; -- 1.8.3.1
[PATCH v7 2/2] media: platform: Add Aspeed Video Engine driver
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1717 + 4 files changed, 1735 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index 7f5c634..136ffb0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2426,6 +2426,14 @@ S: Maintained F: Documentation/hwmon/asc7621 F: drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M: Corentin Chary L: acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ea33063..a505e9f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index d347a55..e6deb25 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE)+= cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..c24acb9 --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1717 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 +#define MIN_HEIGHT 480 +#define MIN_WIDTH 640 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(1000) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#define VE_SEQ_CTRL_EN_WATCHDOG_COMP BIT(14) +#define VE_SEQ_CTRL_TRIG_JPG BIT(15) +#defi
[PATCH v7 0/2] media: platform: Add Aspeed Video Engine driver
From: Eddie James The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting as a service processor, the Video Engine can capture the host processor graphics output. This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming interface by way of videobuf2. Each frame, the driver triggers the hardware to capture the host graphics output and compress it to JPEG format. v4l2-compliance SHA: d039b47a108596ca004b11e52989054882d45888, 32 bits Compliance test for device /dev/video0: Driver Info: Driver name : aspeed-video Card type: Aspeed Video Engine Bus info : platform:aspeed-video Driver version : 4.19.6 Capabilities : 0x8521 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x0521 Video Capture Read/Write Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 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 test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 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 test VIDIOC_DV_TIMINGS_CAP: OK test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls (Input 0): 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 warn: v4l2-test-controls.cpp(853): V4L2_CID_DV_RX_POWER_PRESENT not found for input 0 test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 3 Private Controls: 0 Format ioctls (Input 0): test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK 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: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls (Input 0): test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls (Input 0): test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK test blocking wait: OK test MMAP: OK test USERPTR: OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total: 48, Succeeded: 48, Failed: 0, Warnings: 1 I'm unable to run linux-next or the media tree as our system doesn't boot without quite a few patches that aren't yet upstreamed, so I can't grab the fix for that warning Hans mentioned. Changes since v6: - Refactor open/start process such that the file handle can be opened without a video signal present - Added standards and capabilities - Removed used of VB2_BUF_FLAG_LAST - Added a few comments - Removed a couple of functions that were only used once - Removed client counter Changes since v5: - Rework resolution change and v4l2 timings functions again with active and detected timings. - Renamed a few things. - Fixed polarities in the timings. Changes since v4: - Set real min and max resolution in enum_dv_timings - Add check for vb2_is_busy before settings the timings - Set max frame rate to the actual max rather than max + 1 - Correct the input status to 0. - Rework resolution change to only set the relevant h/w regs during startup or when set_timings is called. Changes
Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver
On 12/03/2018 02:14 PM, Hans Verkuil wrote: On 12/03/2018 05:39 PM, Eddie James wrote: On 12/03/2018 05:04 AM, Hans Verkuil wrote: On 11/27/2018 08:37 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- +static void aspeed_video_bufs_done(struct aspeed_video *video, + enum vb2_buffer_state state) +{ + unsigned long flags; + struct aspeed_video_buffer *buf; + + spin_lock_irqsave(&video->lock, flags); + list_for_each_entry(buf, &video->buffers, link) { + if (list_is_last(&buf->link, &video->buffers)) + buf->vb.flags |= V4L2_BUF_FLAG_LAST; This really makes no sense. This flag is for codecs, not for receivers. You say in an earlier reply about this: "I mentioned before that dequeue calls hang in an error condition unless this flag is specified. For example if resolution change is detected and application is in the middle of trying to dequeue..." What error condition are you referring to? Isn't your application using the select() or poll() calls to wait for events or new buffers to dequeue? If you just call VIDIOC_DQBUF to wait in blocking mode for a new buffer, then it will indeed block in that call. No other video receiver needs this flag, so there is something else that is the cause. Probably no one else uses it in blocking mode, but the thing should still work. Why wouldn't it stop blocking if there is an error? Isn't that normal? As I said, the error condition I've tested this with is resolution change. All the buffers are placed in error state, but dequeue does not return. If VIDIOC_DQBUF is waiting for a buffer, and the driver calls vb2_buffer_done, then the ioctl will return. If not, then something else is wrong. Is your application just requeueing the dequeued buffers? Does it work when you use v4l2-ctl --stream-mmap? I will try some tests. I much prefer using blocking mode in applications because it reduces complexity. You say that the flag is for codecs, not receivers, but I don't see why that has to be the case. Because there is no concept of 'last' buffer for receivers. If the source comes back with the same timings, then receiver will just pick it up again (see also my other email on how video receivers behave when a source disappears). + vb2_buffer_done(&buf->vb.vb2_buf, state); + } + INIT_LIST_HEAD(&video->buffers); + spin_unlock_irqrestore(&video->lock, flags); +} + +static irqreturn_t aspeed_video_irq(int irq, void *arg) +{ + struct aspeed_video *video = arg; + u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS); + + if (atomic_read(&video->clients) == 0) { + dev_info(video->dev, "irq with no client; disabling irqs\n"); + + aspeed_video_write(video, VE_INTERRUPT_CTRL, 0); + aspeed_video_write(video, VE_INTERRUPT_STATUS, 0x); + return IRQ_HANDLED; + } + + /* Resolution changed; reset entire engine and reinitialize */ + if (sts & VE_INTERRUPT_MODE_DETECT_WD) { + dev_info(video->dev, "resolution changed; resetting\n"); + set_bit(VIDEO_RES_CHANGE, &video->flags); + clear_bit(VIDEO_FRAME_INPRG, &video->flags); + clear_bit(VIDEO_STREAMING, &video->flags); + + aspeed_video_off(video); + aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR); + + schedule_delayed_work(&video->res_work, + RESOLUTION_CHANGE_DELAY); + return IRQ_HANDLED; + } + + if (sts & VE_INTERRUPT_MODE_DETECT) { + aspeed_video_update(video, VE_INTERRUPT_CTRL, + VE_INTERRUPT_MODE_DETECT, 0); + aspeed_video_write(video, VE_INTERRUPT_STATUS, + VE_INTERRUPT_MODE_DETECT); + + set_bit(VIDEO_MODE_DETECT_DONE, &video->flags); + wake_up_interruptible_all(&video->wait); + } + + if ((sts & VE_INTERRUPT_COMP_COMPLETE) && + (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) { + struct aspeed_video_buffer *buf; + u32 frame_size = aspeed_video_read(video, + VE_OFFSET_COMP_STREAM); + + spin_lock(&video->lo
Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver
On 12/03/2018 05:04 AM, Hans Verkuil wrote: On 11/27/2018 08:37 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1719 + 4 files changed, 1737 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index 602142c..51f513f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2423,6 +2423,14 @@ S: Maintained F:Documentation/hwmon/asc7621 F:drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M:Corentin Chary L:acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ea33063..a505e9f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index d347a55..e6deb25 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE) += cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..200f4d82 --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1719 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 +#define MIN_HEIGHT 480 +#define MIN_WIDTH 640 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(250) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#defi
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 14:26 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 03:14:59PM -0700, Jonathan Corbet wrote: [...] > > Have you read Documentation/process/code-of-conduct- > > interpretation.rst? > > As has been pointed out, it contains a clear answer to how things > > should be interpreted here. > > Ugh, was not aware that there two documents. > > Yeah, definitely sheds light. Why the documents could not be merged > to single common sense code of conduct? The fact that we've arrived at essentially an original CoC reinterpreted to the point where it's effectively a new CoC has been the source of much debate and recrimination over the last few months ... you can read it in the ksummit-discuss archives, but I really think we don't want to reopen that can of worms. James
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 14:12 -0800, Jarkko Sakkinen wrote: [...] > I pasted this already to another response and this was probably the > part that ignited me to send the patch set (was a few days ago, so > had to revisit to find the exact paragraph): I replied in to the other thread. > "Maintainers have the right and responsibility to remove, edit, or > reject comments, commits, code, wiki edits, issues, and other > contributions that are not aligned to this Code of Conduct, or to ban > temporarily or permanently any contributor for other behaviors that > they deem inappropriate, threatening, offensive, or harmful." > > The whole patch set is neither a joke/troll nor something I would > necessarily want to be include myself. It does have the RFC tag. > > As a maintainer myself (and based on somewhat disturbed feedback from > other maintainers) I can only make the conclusion that nobody knows > what the responsibility part here means. > > I would interpret, if I read it like at lawyer at least, that even > for existing code you would need to do the changes postmorterm. That's wrong in the light of the interpretation document, yes. > Is this wrong interpretation? Should I conclude that I made a > mistake by reading the CoC and trying to understand what it > *actually* says? You can't read it in isolation, you need to read it along with the interpretation document. The latter was created precisely because there was a lot of push back on interpretation problems and ambiguities with the original CoC and it specifically covers this case (and a lot of others). James > After this discussion, I can say that I understand it less than > before. > > /Jarkko >
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 13:54 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 01:48:08PM -0800, David Miller wrote: > > From: Jarkko Sakkinen > > Date: Fri, 30 Nov 2018 13:44:05 -0800 > > > > > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > > > > No because use of what some people consider to be bad language > > > > isn't > > > > necessarily abusive, offensive or degrading. Our most heavily > > > > censored > > > > medium is TV and "fuck" is now considered acceptable in certain > > > > contexts on most channels in the UK and EU. > > > > > > This makes following the CoC extremely hard to a non-native > > > speaker as > > > it is not too explicit on what is OK and what is not. I did > > > through the > > > whole thing with an eye glass and this what I deduced from it. > > > > It would be helpful if you could explain what part of the language > > is unclear wrt. explaining how CoC does not apply to existing code. > > > > That part seems very explicit to me. > > Well, now that I re-read it, it was this part to be exact: > > "Maintainers have the right and responsibility to remove, edit, or > reject comments, commits, code, wiki edits, issues, and other > contributions that are not aligned to this Code of Conduct, or to ban > temporarily or permanently any contributor for other behaviors that > they deem inappropriate, threatening, offensive, or harmful." > > How this should be interpreted? Firstly, this is *only* about contributions going forward. The interpretation document says we don't have to look back into the repository and we definitely can't remove something from git that's already been committed. As a Maintainer, if you feel bad language is inappropriate for your subsystem then you say so and reject with that reason patches that come in containing it (suggesting alternative rewordings). However, your determination about what is or isn't acceptable in your subsystem isn't binding on other maintainers, who may have different standards ... this is identical to what we do with coding, like your insistence on one line per variable or other subsystem's insistence on reverse christmas tree for includes ... James > I have not really followed the previous CoC discussions as I try to > always use polite language so I'm sorry if this duplicate. > > /Jarkko >
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 13:44 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 01:01:02PM -0800, James Bottomley wrote: > > No because use of what some people consider to be bad language > > isn't necessarily abusive, offensive or degrading. Our most > > heavily censored medium is TV and "fuck" is now considered > > acceptable in certain contexts on most channels in the UK and EU. > > This makes following the CoC extremely hard to a non-native speaker > as it is not too explicit on what is OK and what is not. I did > through the whole thing with an eye glass and this what I deduced > from it. OK, so something that would simply be considered in some quarters as bad language isn't explicitly banned. The thing which differentiates simple bad language from "abusive, offensive or degrading language", which is called out by the CoC, is the context and the target. So when it's a simple expletive or the code of the author or even the hardware is the target, I'd say it's an easy determination it's not a CoC violation. If someone else's code is the target or the inventor of the hardware is targetted by name, I'd say it is. Even non-native English speakers should be able to determine target and context, because that's the essence of meaning. James
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, 2018-11-30 at 12:55 -0800, Jarkko Sakkinen wrote: > On Fri, Nov 30, 2018 at 11:56:52AM -0800, Davidlohr Bueso wrote: > > On Fri, 30 Nov 2018, Kees Cook wrote: > > > > > On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen > > > wrote: > > > > > > > > In order to comply with the CoC, replace with a hug. > > > > I hope this is some kind of joke. How would anyone get offended by > > reading technical comments? This is all beyond me... > > Well... Not a joke really but more like conversation starter :-) > > I had 10h flight from Amsterdam to Portland and one of the things > that I did was to read the new CoC properly. > > This a direct quote from the CoC: > > "Harassment includes the use of abusive, offensive or degrading > language, intimidation, stalking, harassing photography or recording, > inappropriate physical contact, sexual imagery and unwelcome sexual > advances or requests for sexual favors." > > Doesn't this fall into this category? No because use of what some people consider to be bad language isn't necessarily abusive, offensive or degrading. Our most heavily censored medium is TV and "fuck" is now considered acceptable in certain contexts on most channels in the UK and EU. > Your argument is not that great because you could say that from any > LKML discussion. If you don't like hugging, please propose something > else > :-) The interpretation document also says this: ontributions submitted for the kernel should use appropriate language. Content that already exists predating the Code of Conduct will not be addressed now as a violation. So that definitely means there should be no hunting down of existing comments in kernel code. James
[PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1719 + 4 files changed, 1737 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index 602142c..51f513f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2423,6 +2423,14 @@ S: Maintained F: Documentation/hwmon/asc7621 F: drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M: Corentin Chary L: acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ea33063..a505e9f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index d347a55..e6deb25 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE)+= cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..200f4d82 --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1719 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 +#define MIN_HEIGHT 480 +#define MIN_WIDTH 640 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(250) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#define VE_SEQ_CTRL_EN_WATCHDOG_COMP BIT(14) +#define VE_SEQ_CTRL_TRIG_JPG BIT(15) +#defi
[PATCH v6 0/2] media: platform: Add Aspeed Video Engine driver
nd only if size actually changes - Locked open and release Changes since v2: - Switch to streaming interface. This involved a lot of changes. - Rework memory allocation due to using videobuf2 buffers, but also only allocate the necessary size of source buffer rather than the max size Changes since v1: - Removed le32_to_cpu calls for JPEG header data - Reworked v4l2 ioctls to be compliant. - Added JPEG controls - Updated devicetree docs according to Rob's suggestions. - Added myself to MAINTAINERS Eddie James (2): dt-bindings: media: Add Aspeed Video Engine binding documentation media: platform: Add Aspeed Video Engine driver .../devicetree/bindings/media/aspeed-video.txt | 26 + MAINTAINERS|8 + drivers/media/platform/Kconfig |9 + drivers/media/platform/Makefile|1 + drivers/media/platform/aspeed-video.c | 1719 5 files changed, 1763 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt create mode 100644 drivers/media/platform/aspeed-video.c -- 1.8.3.1
[PATCH v6 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation
Document the bindings. Signed-off-by: Eddie James Reviewed-by: Rob Herring --- .../devicetree/bindings/media/aspeed-video.txt | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt new file mode 100644 index 000..78b464a --- /dev/null +++ b/Documentation/devicetree/bindings/media/aspeed-video.txt @@ -0,0 +1,26 @@ +* Device tree bindings for Aspeed Video Engine + +The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can +capture and compress video data from digital or analog sources. + +Required properties: + - compatible: "aspeed,ast2400-video-engine" or + "aspeed,ast2500-video-engine" + - reg:contains the offset and length of the VE memory region + - clocks: clock specifiers for the syscon clocks associated with + the VE (ordering must match the clock-names property) + - clock-names:"vclk" and "eclk" + - resets: reset specifier for the syscon reset associated with + the VE + - interrupts: the interrupt associated with the VE on this platform + +Example: + +video-engine@1e70 { +compatible = "aspeed,ast2500-video-engine"; +reg = <0x1e70 0x2>; +clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; +clock-names = "vclk", "eclk"; +resets = <&syscon ASPEED_RESET_VIDEO>; +interrupts = <7>; +}; -- 1.8.3.1
Re: [PATCH v5 0/2] media: platform: Add Aspeed Video Engine Driver
On 11/15/2018 03:20 AM, Hans Verkuil wrote: On 11/12/2018 10:00 PM, Eddie James wrote: From: Eddie James The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting as a service processor, the Video Engine can capture the host processor graphics output. This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming interface by way of videobuf2. Each frame, the driver triggers the hardware to capture the host graphics output and compress it to JPEG format. I reviewed the driver, and there is still confusion about active timings and detected timings. You are really close, but it is still not right. The timings returned by G_DV_TIMINGS should never change, unless by calling S_DV_TIMINGS. The format returned by G_FMT should never change, unless by calling S_DV_TIMINGS (which implicitly changes the format) or S_FMT. Timings changes from the video or calling QUERY_DV_TIMINGS should have NO effect on the timings/format returned by G_DV_TIMINGS/G_FMT. Obviously, if the video timings change, then streaming will halt and an event is issued so userspace can take action. Thanks, I have made some changes for v6. v4l-utils HEAD dd3ff81f58c4e1e6f33765dc61ad33c48ae6bb07 v4l2-compliance SHA: not available, 32 bits Control ioctls (Input 0): 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 fail: v4l2-test-controls.cpp(823): failed to find event for control 'Chroma Subsampling' That's weird. It's not clear to me why this fails. test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 3 Private Controls: 0 Format ioctls (Input 0): test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK 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: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls (Input 0): test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls (Input 0): fail: v4l2-test-buffers.cpp(422): dmabuf_valid ^ !!(caps & V4L2_BUF_CAP_SUPPORTS_DMABUF) I updated v4l2-compliance, as this test was a bit too strict. test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL test VIDIOC_EXPBUF: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK test blocking wait: OK test MMAP: OK test USERPTR: OK (Not Supported) fail: v4l2-test-buffers.cpp(1102): exp_q.reqbufs(expbuf_node, q.g_buffers()) fail: v4l2-test-buffers.cpp(1192): setupDmaBuf(expbuf_node, node, q, exp_q) You need to add: .vidioc_expbuf = vb2_ioctl_expbuf, to aspeed_video_ioctl_ops. test DMABUF: FAIL Total: 48, Succeeded: 45, Failed: 3, Warnings: 0 The apparent rate of change to the v4l2/vb2 API makes it difficult to pass these tests. None of the failing tests even ran last time I submitted my series. And V4L2_BUF_CAP_SUPPORTS_DMABUF is undefined in 4.19. When developing new drivers you should use the master branch of https://git.linuxtv.org/media_tree.git/ Our 4.18 tree has quite a few chip-specific patches that I need that are not upstreamed yet, so it's not possible for me to use that. Thanks, Eddie The v4l-utils repo is kept in sync with the latest code from that master branch. Regards, Hans Changes since v4: - Set real min and max resolution in enum_dv_timings - Add check for vb2_is_busy before settings the timings - Set max frame rate to the actual max rather than max + 1 - Correct the input status to 0. - Rework resolution change to only set the relevant h/w regs during startup or when set_timings is called. Changes since v3: - Switch update reg function to use u32 clear rather than unsigned long mask - Add timing information from host VGA signal - Fix binding documentation mispelling - Fix upper case hex values - Add wait_prepare and wait_finish - Set buffer state to queued (rather than error) if streaming fails to start - Switch engine busy print statement to error - Removed a couple unecessary type assignments in v4l2 ioctls - Added query_dv_timings, fixed get_dv_timings - Corrected source change event to fire if and only if size actually changes - Locked open and release Changes since v2: - Switch to streaming interface. This involved a lot of changes. - Rework memory allocation due to using vid
Re: [PATCH v5 2/2] media: platform: Add Aspeed Video Engine driver
On 11/15/2018 02:56 AM, Hans Verkuil wrote: On 11/12/2018 10:00 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1711 + 4 files changed, 1729 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index fa45ff3..f8f08a4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2414,6 +2414,14 @@ S: Maintained F:Documentation/hwmon/asc7621 F:drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M:Corentin Chary L:acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 70c4f6c..ba78dd5 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 6ab6200..2973953 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE) += cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..a2fd0bf --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1711 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 +#define MIN_HEIGHT 480 +#define MIN_WIDTH 640 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(250) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#defi
[PATCH v5 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation
Document the bindings. Signed-off-by: Eddie James Reviewed-by: Rob Herring --- .../devicetree/bindings/media/aspeed-video.txt | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt new file mode 100644 index 000..78b464a --- /dev/null +++ b/Documentation/devicetree/bindings/media/aspeed-video.txt @@ -0,0 +1,26 @@ +* Device tree bindings for Aspeed Video Engine + +The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can +capture and compress video data from digital or analog sources. + +Required properties: + - compatible: "aspeed,ast2400-video-engine" or + "aspeed,ast2500-video-engine" + - reg:contains the offset and length of the VE memory region + - clocks: clock specifiers for the syscon clocks associated with + the VE (ordering must match the clock-names property) + - clock-names:"vclk" and "eclk" + - resets: reset specifier for the syscon reset associated with + the VE + - interrupts: the interrupt associated with the VE on this platform + +Example: + +video-engine@1e70 { +compatible = "aspeed,ast2500-video-engine"; +reg = <0x1e70 0x2>; +clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; +clock-names = "vclk", "eclk"; +resets = <&syscon ASPEED_RESET_VIDEO>; +interrupts = <7>; +}; -- 1.8.3.1
[PATCH v5 2/2] media: platform: Add Aspeed Video Engine driver
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1711 + 4 files changed, 1729 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index fa45ff3..f8f08a4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2414,6 +2414,14 @@ S: Maintained F: Documentation/hwmon/asc7621 F: drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M: Corentin Chary L: acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 70c4f6c..ba78dd5 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 6ab6200..2973953 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE)+= cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..a2fd0bf --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1711 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 +#define MIN_HEIGHT 480 +#define MIN_WIDTH 640 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(250) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#define VE_SEQ_CTRL_EN_WATCHDOG_COMP BIT(14) +#define VE_SEQ_CTRL_TRIG_JPG BIT(15) +#defi
[PATCH v5 0/2] media: platform: Add Aspeed Video Engine Driver
From: Eddie James The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting as a service processor, the Video Engine can capture the host processor graphics output. This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming interface by way of videobuf2. Each frame, the driver triggers the hardware to capture the host graphics output and compress it to JPEG format. v4l-utils HEAD dd3ff81f58c4e1e6f33765dc61ad33c48ae6bb07 v4l2-compliance SHA: not available, 32 bits Compliance test for device /dev/video0: Driver Info: Driver name : aspeed-video Card type: Aspeed Video Engine Bus info : platform:aspeed-video Driver version : 4.18.12 Capabilities : 0x8521 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x0521 Video Capture Read/Write Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 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 test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 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 test VIDIOC_DV_TIMINGS_CAP: OK test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls (Input 0): 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 fail: v4l2-test-controls.cpp(823): failed to find event for control 'Chroma Subsampling' test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 3 Private Controls: 0 Format ioctls (Input 0): test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK 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: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls (Input 0): test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls (Input 0): fail: v4l2-test-buffers.cpp(422): dmabuf_valid ^ !!(caps & V4L2_BUF_CAP_SUPPORTS_DMABUF) test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL test VIDIOC_EXPBUF: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK test blocking wait: OK test MMAP: OK test USERPTR: OK (Not Supported) fail: v4l2-test-buffers.cpp(1102): exp_q.reqbufs(expbuf_node, q.g_buffers()) fail: v4l2-test-buffers.cpp(1192): setupDmaBuf(expbuf_node, node, q, exp_q) test DMABUF: FAIL Total: 48, Succeeded: 45, Failed: 3, Warnings: 0 The apparent rate of change to the v4l2/vb2 API makes it difficult to pass these tests. None of the failing tests even ran last time I submitted my series. And V4L2_BUF_CAP_SUPPORTS_DMABUF is undefined in 4.19. Changes since v4: - Set real min and max resolution in enum_dv_timings - Add check for vb2_is_busy before settings the timings - Set max frame rate to the actual max rather than max + 1 - Correct the input status to 0. - Rework resolution change to only set the relevant h/w regs during startup or when set_timings is called. Changes since v3: - Switch update reg function to use u32 clear rather than unsigned long mask - Add timing information from host VGA signal - Fix binding documentation mispelling - Fix upper case hex
Re: [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
On 10/19/2018 03:26 PM, Eddie James wrote: On 10/12/2018 07:22 AM, Hans Verkuil wrote: On 10/05/2018 09:57 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James ---  MAINTAINERS  |   8 +  drivers/media/platform/Kconfig   |   8 +  drivers/media/platform/Makefile  |   1 +  drivers/media/platform/aspeed-video.c | 1674 +  4 files changed, 1691 insertions(+)  create mode 100644 drivers/media/platform/aspeed-video.c +static int aspeed_video_enum_input(struct file *file, void *fh, +  struct v4l2_input *inp) +{ +   if (inp->index) +   return -EINVAL; + +   strscpy(inp->name, "Host VGA capture", sizeof(inp->name)); +   inp->type = V4L2_INPUT_TYPE_CAMERA; +   inp->capabilities = V4L2_IN_CAP_DV_TIMINGS; +   inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC; This can't be right. If there is a valid signal, then status should be 0. And ideally you can tell the difference between no signal and no sync as well. + +   return 0; +} + +static int aspeed_video_get_input(struct file *file, void *fh, unsigned int *i) +{ +   *i = 0; + +   return 0; +} + +static int aspeed_video_set_input(struct file *file, void *fh, unsigned int i) +{ +   if (i) +   return -EINVAL; + +   return 0; +} + +static int aspeed_video_get_parm(struct file *file, void *fh, + struct v4l2_streamparm *a) +{ +   struct aspeed_video *video = video_drvdata(file); + +   a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; +   a->parm.capture.readbuffers = 3; +   a->parm.capture.timeperframe.numerator = 1; +   if (!video->frame_rate) +   a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1; +   else +   a->parm.capture.timeperframe.denominator = video->frame_rate; + +   return 0; +} + +static int aspeed_video_set_parm(struct file *file, void *fh, + struct v4l2_streamparm *a) +{ +   unsigned int frame_rate = 0; +   struct aspeed_video *video = video_drvdata(file); + +   a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; +   a->parm.capture.readbuffers = 3; + +   if (a->parm.capture.timeperframe.numerator) +   frame_rate = a->parm.capture.timeperframe.denominator / +   a->parm.capture.timeperframe.numerator; + +   if (!frame_rate || frame_rate > MAX_FRAME_RATE) { +   frame_rate = 0; + +   /* + * Set to max + 1 to differentiate between max and 0, which + * means "don't care". But what does "don't care" mean in practice? It's still not clear to me how this is supposed to work. + */ +   a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1; And regardless of anything else this timeperframe is out of the range that aspeed_video_enum_frameintervals() returns. + a->parm.capture.timeperframe.numerator = 1; +   } + +   if (video->frame_rate != frame_rate) { +   video->frame_rate = frame_rate; +   aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, +   FIELD_PREP(VE_CTRL_FRC, frame_rate)); +   } + +   return 0; +} + +static int aspeed_video_enum_framesizes(struct file *file, void *fh, +   struct v4l2_frmsizeenum *fsize) +{ +   struct aspeed_video *video = video_drvdata(file); + +   if (fsize->index) +   return -EINVAL; + +   if (fsize->pixel_format != V4L2_PIX_FMT_JPEG) +   return -EINVAL; + +   fsize->discrete.width = video->pix_fmt.width; +   fsize->discrete.height = video->pix_fmt.height; +   fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE; + +   return 0; +} + +static int aspeed_video_enum_frameintervals(struct file *file, void *fh, +   struct v4l2_frmivalenum *fival) +{ +   struct aspeed_video *video = video_drvdata(file); + +   if (fival->index) +   return -EINVAL; + +   if (fival->width != video->width || fival->height != video->height) +   return -EINVAL; + +   if (fival->pixel_format != V4L2_PIX_FMT_JPEG) +   return -EINVAL; + +   fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS; + +   fival->stepwise.min.denominator = MAX_FRAME_RATE; +   fival->stepwise.min.numerator = 1; +   fival->stepwise.max.denominator = 1; +   fival->stepwise.max.numerator = 1; +   fival->stepwise.step = fival->stepwise.max; + +   return 0; +} + +static int aspeed_video_set_dv_timings(struct file *file, void *fh, +  struct v4l2_dv_tim
Re: [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
On 10/12/2018 07:22 AM, Hans Verkuil wrote: On 10/05/2018 09:57 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|8 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1674 + 4 files changed, 1691 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c +static int aspeed_video_enum_input(struct file *file, void *fh, + struct v4l2_input *inp) +{ + if (inp->index) + return -EINVAL; + + strscpy(inp->name, "Host VGA capture", sizeof(inp->name)); + inp->type = V4L2_INPUT_TYPE_CAMERA; + inp->capabilities = V4L2_IN_CAP_DV_TIMINGS; + inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC; This can't be right. If there is a valid signal, then status should be 0. And ideally you can tell the difference between no signal and no sync as well. + + return 0; +} + +static int aspeed_video_get_input(struct file *file, void *fh, unsigned int *i) +{ + *i = 0; + + return 0; +} + +static int aspeed_video_set_input(struct file *file, void *fh, unsigned int i) +{ + if (i) + return -EINVAL; + + return 0; +} + +static int aspeed_video_get_parm(struct file *file, void *fh, +struct v4l2_streamparm *a) +{ + struct aspeed_video *video = video_drvdata(file); + + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; + a->parm.capture.readbuffers = 3; + a->parm.capture.timeperframe.numerator = 1; + if (!video->frame_rate) + a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1; + else + a->parm.capture.timeperframe.denominator = video->frame_rate; + + return 0; +} + +static int aspeed_video_set_parm(struct file *file, void *fh, +struct v4l2_streamparm *a) +{ + unsigned int frame_rate = 0; + struct aspeed_video *video = video_drvdata(file); + + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; + a->parm.capture.readbuffers = 3; + + if (a->parm.capture.timeperframe.numerator) + frame_rate = a->parm.capture.timeperframe.denominator / + a->parm.capture.timeperframe.numerator; + + if (!frame_rate || frame_rate > MAX_FRAME_RATE) { + frame_rate = 0; + + /* +* Set to max + 1 to differentiate between max and 0, which +* means "don't care". But what does "don't care" mean in practice? It's still not clear to me how this is supposed to work. +*/ + a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1; And regardless of anything else this timeperframe is out of the range that aspeed_video_enum_frameintervals() returns. + a->parm.capture.timeperframe.numerator = 1; + } + + if (video->frame_rate != frame_rate) { + video->frame_rate = frame_rate; + aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, + FIELD_PREP(VE_CTRL_FRC, frame_rate)); + } + + return 0; +} + +static int aspeed_video_enum_framesizes(struct file *file, void *fh, + struct v4l2_frmsizeenum *fsize) +{ + struct aspeed_video *video = video_drvdata(file); + + if (fsize->index) + return -EINVAL; + + if (fsize->pixel_format != V4L2_PIX_FMT_JPEG) + return -EINVAL; + + fsize->discrete.width = video->pix_fmt.width; + fsize->discrete.height = video->pix_fmt.height; + fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE; + + return 0; +} + +static int aspeed_video_enum_frameintervals(struct file *file, void *fh, + struct v4l2_frmivalenum *fival) +{ + struct aspeed_video *video = video_drvdata(file); + + if (fival->index) + return -EINVAL; + + if (fival->width != video->width || fival->height != video->height) + return -EINVAL; + + if (fival->pixel_format != V4L2_PIX_FMT_JPEG) + return -EINVAL; + + fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS; + + fival->stepwise.min.denominator = MAX_FRAME_RATE; +
Re: [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
On 10/12/2018 07:22 AM, Hans Verkuil wrote: On 10/05/2018 09:57 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|8 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1674 + 4 files changed, 1691 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c +static int aspeed_video_enum_input(struct file *file, void *fh, + struct v4l2_input *inp) +{ + if (inp->index) + return -EINVAL; + + strscpy(inp->name, "Host VGA capture", sizeof(inp->name)); + inp->type = V4L2_INPUT_TYPE_CAMERA; + inp->capabilities = V4L2_IN_CAP_DV_TIMINGS; + inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC; This can't be right. If there is a valid signal, then status should be 0. And ideally you can tell the difference between no signal and no sync as well. Ah, yes I misunderstood the status, thanks. + + return 0; +} + +static int aspeed_video_get_input(struct file *file, void *fh, unsigned int *i) +{ + *i = 0; + + return 0; +} + +static int aspeed_video_set_input(struct file *file, void *fh, unsigned int i) +{ + if (i) + return -EINVAL; + + return 0; +} + +static int aspeed_video_get_parm(struct file *file, void *fh, +struct v4l2_streamparm *a) +{ + struct aspeed_video *video = video_drvdata(file); + + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; + a->parm.capture.readbuffers = 3; + a->parm.capture.timeperframe.numerator = 1; + if (!video->frame_rate) + a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1; + else + a->parm.capture.timeperframe.denominator = video->frame_rate; + + return 0; +} + +static int aspeed_video_set_parm(struct file *file, void *fh, +struct v4l2_streamparm *a) +{ + unsigned int frame_rate = 0; + struct aspeed_video *video = video_drvdata(file); + + a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; + a->parm.capture.readbuffers = 3; + + if (a->parm.capture.timeperframe.numerator) + frame_rate = a->parm.capture.timeperframe.denominator / + a->parm.capture.timeperframe.numerator; + + if (!frame_rate || frame_rate > MAX_FRAME_RATE) { + frame_rate = 0; + + /* +* Set to max + 1 to differentiate between max and 0, which +* means "don't care". But what does "don't care" mean in practice? It's still not clear to me how this is supposed to work. +*/ + a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1; And regardless of anything else this timeperframe is out of the range that aspeed_video_enum_frameintervals() returns. + a->parm.capture.timeperframe.numerator = 1; + } + + if (video->frame_rate != frame_rate) { + video->frame_rate = frame_rate; + aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC, + FIELD_PREP(VE_CTRL_FRC, frame_rate)); + } + + return 0; +} + +static int aspeed_video_enum_framesizes(struct file *file, void *fh, + struct v4l2_frmsizeenum *fsize) +{ + struct aspeed_video *video = video_drvdata(file); + + if (fsize->index) + return -EINVAL; + + if (fsize->pixel_format != V4L2_PIX_FMT_JPEG) + return -EINVAL; + + fsize->discrete.width = video->pix_fmt.width; + fsize->discrete.height = video->pix_fmt.height; + fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE; + + return 0; +} + +static int aspeed_video_enum_frameintervals(struct file *file, void *fh, + struct v4l2_frmivalenum *fival) +{ + struct aspeed_video *video = video_drvdata(file); + + if (fival->index) + return -EINVAL; + + if (fival->width != video->width || fival->height != video->height) + return -EINVAL; + + if (fival->pixel_format != V4L2_PIX_FMT_JPEG) + return -EINVAL; + + fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS; + + fival->stepwise
Re: [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
On 10/17/2018 05:41 AM, Hans Verkuil wrote: On 10/05/2018 09:57 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|8 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1674 + 4 files changed, 1691 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c +static void aspeed_video_check_polarity(struct aspeed_video *video) +{ + int i; + int hsync_counter = 0; + int vsync_counter = 0; + u32 sts; + + for (i = 0; i < NUM_POLARITY_CHECKS; ++i) { + sts = aspeed_video_read(video, VE_MODE_DETECT_STATUS); + if (sts & VE_MODE_DETECT_STATUS_VSYNC) + vsync_counter--; + else + vsync_counter++; + + if (sts & VE_MODE_DETECT_STATUS_HSYNC) + hsync_counter--; + else + hsync_counter++; + } + + if (hsync_counter < 0 || vsync_counter < 0) { + u32 ctrl; + + if (hsync_counter < 0) + ctrl = VE_CTRL_HSYNC_POL; + else + video->timings.polarities |= V4L2_DV_HSYNC_POS_POL; + + if (vsync_counter < 0) + ctrl = VE_CTRL_VSYNC_POL; + else + video->timings.polarities |= V4L2_DV_VSYNC_POS_POL; + + aspeed_video_update(video, VE_CTRL, 0, ctrl); + } +} + +static bool aspeed_video_alloc_buf(struct aspeed_video *video, + struct aspeed_video_addr *addr, + unsigned int size) +{ + addr->virt = dma_alloc_coherent(video->dev, size, &addr->dma, + GFP_KERNEL); + if (!addr->virt) + return false; + + addr->size = size; + return true; +} + +static void aspeed_video_free_buf(struct aspeed_video *video, + struct aspeed_video_addr *addr) +{ + dma_free_coherent(video->dev, addr->size, addr->virt, addr->dma); + addr->size = 0; + addr->dma = 0ULL; + addr->virt = NULL; +} + +/* + * Get the minimum HW-supported compression buffer size for the frame size. + * Assume worst-case JPEG compression size is 1/8 raw size. This should be + * plenty even for maximum quality; any worse and the engine will simply return + * incomplete JPEGs. + */ +static void aspeed_video_calc_compressed_size(struct aspeed_video *video) +{ + int i, j; + u32 compression_buffer_size_reg = 0; + unsigned int size; + const unsigned int num_compression_packets = 4; + const unsigned int compression_packet_size = 1024; + const unsigned int max_compressed_size = + video->width * video->height / 2; /* 4 Bpp / 8 */ + + video->max_compressed_size = UINT_MAX; + + for (i = 0; i < 6; ++i) { + for (j = 0; j < 8; ++j) { + size = (num_compression_packets << i) * + (compression_packet_size << j); + if (size < max_compressed_size) + continue; + + if (size < video->max_compressed_size) { + compression_buffer_size_reg = (i << 3) | j; + video->max_compressed_size = size; + } + } + } + + aspeed_video_write(video, VE_STREAM_BUF_SIZE, + compression_buffer_size_reg); + + dev_dbg(video->dev, "max compressed size: %x\n", + video->max_compressed_size); +} + +#define res_check(v) test_and_clear_bit(VIDEO_MODE_DETECT_DONE, &(v)->flags) + +static int aspeed_video_get_resolution(struct aspeed_video *video) +{ + bool invalid_resolution = true; + int rc; + int tries = 0; + unsigned int bottom; + unsigned int left; + unsigned int right; + unsigned int size; + unsigned int top; + u32 mds; + u32 src_lr_edge; + u32 src_tb_edge; + u32 sync; + struct aspeed_video_addr src; + + if (video->srcs[1].size) + aspeed_video_free_buf(video, &video->srcs[1]); + +
Re: [PATCH v3 2/2] media: platform: Add Aspeed Video Engine driver
On 10/04/2018 08:12 AM, Hans Verkuil wrote: On 10/03/18 22:43, Eddie James wrote: On 09/28/2018 06:30 AM, Hans Verkuil wrote: On 09/25/2018 09:27 PM, Eddie James wrote: The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James + } + + video->height = (bottom - top) + 1; + video->width = (right - left) + 1; + size = video->height * video->width; It looks like you can actually determine the blanking width/height and possibly even more detailed information that would be very useful to show with the DV_TIMINGS ioctls. Hmm. This information is related to the video signal captured from the host. That information has nothing to do with the buffer that is compressed and grabbed by the driver and ultimately provided to userspace. Isn't the timing information meaningless for JPEG frames? It helps in debugging. Basically you are implementing a receiver for a video signal. So if for some reason you cannot support the video timings that the host sends, then it is very useful to have QUERY_DV_TIMINGS report as much information about the signal as possible. BTW, out of curiosity, how are the host video signals connected to the aspeed? Is it still a VGA video signal? Looking at product briefs it appears that it is VGA. So I guess the aspeed 'sniffs' the VGA signals from the host and can capture the video that way. Is that correct? I believe it is a VGA signal from the host, but the Aspeed Video Engine somewhat abstracts that away; not all the signal information that the engine is receiving is available to the BMC interface. I did add the timing information I could access to the latest patch set. As you say, it could be useful for debugging if weird things are happening. Thanks! Eddie If so, then this driver is a VGA receiver and should act like that. The host can configure its VGA transmitter to invalid timings, or weird values, and you need to be able to handle that in your driver. Forgot to include this question in my previous reply, sorry for the additional mail. No problem! Happy to help. Regards, Hans
[PATCH v4 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation
Document the bindings. Signed-off-by: Eddie James Reviewed-by: Rob Herring --- .../devicetree/bindings/media/aspeed-video.txt | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt new file mode 100644 index 000..78b464a --- /dev/null +++ b/Documentation/devicetree/bindings/media/aspeed-video.txt @@ -0,0 +1,26 @@ +* Device tree bindings for Aspeed Video Engine + +The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can +capture and compress video data from digital or analog sources. + +Required properties: + - compatible: "aspeed,ast2400-video-engine" or + "aspeed,ast2500-video-engine" + - reg:contains the offset and length of the VE memory region + - clocks: clock specifiers for the syscon clocks associated with + the VE (ordering must match the clock-names property) + - clock-names:"vclk" and "eclk" + - resets: reset specifier for the syscon reset associated with + the VE + - interrupts: the interrupt associated with the VE on this platform + +Example: + +video-engine@1e70 { +compatible = "aspeed,ast2500-video-engine"; +reg = <0x1e70 0x2>; +clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; +clock-names = "vclk", "eclk"; +resets = <&syscon ASPEED_RESET_VIDEO>; +interrupts = <7>; +}; -- 1.8.3.1
[PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs can capture and compress video data from digital or analog sources. With the Aspeed chip acting a service processor, the Video Engine can capture the host processor graphics output. Add a V4L2 driver to capture video data and compress it to JPEG images. Make the video frames available through the V4L2 streaming interface. Signed-off-by: Eddie James --- MAINTAINERS |8 + drivers/media/platform/Kconfig|8 + drivers/media/platform/Makefile |1 + drivers/media/platform/aspeed-video.c | 1674 + 4 files changed, 1691 insertions(+) create mode 100644 drivers/media/platform/aspeed-video.c diff --git a/MAINTAINERS b/MAINTAINERS index 29c0810..9b53694 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2407,6 +2407,14 @@ S: Maintained F: Documentation/hwmon/asc7621 F: drivers/hwmon/asc7621.c +ASPEED VIDEO ENGINE DRIVER +M: Eddie James +L: linux-media@vger.kernel.org +L: open...@lists.ozlabs.org (moderated for non-subscribers) +S: Maintained +F: drivers/media/platform/aspeed-video.c +F: Documentation/devicetree/bindings/media/aspeed-video.txt + ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS M: Corentin Chary L: acpi4asus-u...@lists.sourceforge.net diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 936675d..f211253 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -32,6 +32,14 @@ source "drivers/media/platform/davinci/Kconfig" source "drivers/media/platform/omap/Kconfig" +config VIDEO_ASPEED + tristate "Aspeed AST2400 and AST2500 Video Engine driver" + depends on VIDEO_V4L2 + help + Support for the Aspeed Video Engine (VE) embedded in the Aspeed + AST2400 and AST2500 SOCs. The VE can capture and compress video data + from digital or analog sources. + config VIDEO_SH_VOU tristate "SuperH VOU video output driver" depends on MEDIA_CAMERA_SUPPORT diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 6ab6200..2973953 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -3,6 +3,7 @@ # Makefile for the video capture/playback device drivers. # +obj-$(CONFIG_VIDEO_ASPEED) += aspeed-video.o obj-$(CONFIG_VIDEO_CADENCE)+= cadence/ obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c new file mode 100644 index 000..836a657 --- /dev/null +++ b/drivers/media/platform/aspeed-video.c @@ -0,0 +1,1674 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME"aspeed-video" + +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12 +#define ASPEED_VIDEO_JPEG_HEADER_SIZE 10 +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116 +#define ASPEED_VIDEO_JPEG_DCT_SIZE 34 + +#define MAX_FRAME_RATE 60 +#define MAX_HEIGHT 1200 +#define MAX_WIDTH 1920 + +#define NUM_POLARITY_CHECKS10 +#define INVALID_RESOLUTION_RETRIES 2 +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250) +#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500) +#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500) +#define STOP_TIMEOUT msecs_to_jiffies(250) +#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */ + +#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */ +#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */ + +#define VE_PROTECTION_KEY 0x000 +#define VE_PROTECTION_KEY_UNLOCK 0x1a038aa8 + +#define VE_SEQ_CTRL0x004 +#define VE_SEQ_CTRL_TRIG_MODE_DET BIT(0) +#define VE_SEQ_CTRL_TRIG_CAPTURE BIT(1) +#define VE_SEQ_CTRL_FORCE_IDLEBIT(2) +#define VE_SEQ_CTRL_MULT_FRAMEBIT(3) +#define VE_SEQ_CTRL_TRIG_COMP BIT(4) +#define VE_SEQ_CTRL_AUTO_COMP BIT(5) +#define VE_SEQ_CTRL_EN_WATCHDOG BIT(7) +#define VE_SEQ_CTRL_YUV420BIT(10) +#define VE_SEQ_CTRL_COMP_FMT GENMASK(11, 10) +#define VE_SEQ_CTRL_HALT BIT(12) +#define VE_SEQ_CTRL_EN_WATCHDOG_COMP BIT(14) +#define VE_SEQ_CTRL_TRIG_JPG BIT(15) +#define VE_SEQ_CTRL_CAP_BUSY BIT(16) +#define VE_SEQ_CTRL_COMP_BUSY BIT(18) + +#ifdef CONFIG_MACH_ASPEED_G5 +#define
[PATCH v4 0/2] media: platform: Add Aspeed Video Engine Driver
Eddie James (2): dt-bindings: media: Add Aspeed Video Engine binding documentation media: platform: Add Aspeed Video Engine driver .../devicetree/bindings/media/aspeed-video.txt | 26 + MAINTAINERS|8 + drivers/media/platform/Kconfig |8 + drivers/media/platform/Makefile|1 + drivers/media/platform/aspeed-video.c | 1674 5 files changed, 1717 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt create mode 100644 drivers/media/platform/aspeed-video.c -- 1.8.3.1
do the editing
Want to follow up the email sent last week. Do you have needs for photo editing? We can edit 400 images within 24 hours. We are working on all kinds of ecommerce photos, jewelry photos, and the portrait images. We do cutting out and clipping path and others, and also we provide retouching for your photos, You can throw us a photo and we will do testing for you to check our quality. Thanks, Jason
for editing the photos
Want to follow up the email sent last week. Do you have needs for photo editing? We can edit 400 images within 24 hours. We are working on all kinds of ecommerce photos, jewelry photos, and the portrait images. We do cutting out and clipping path and others, and also we provide retouching for your photos, You can throw us a photo and we will do testing for you to check our quality. Thanks, Jason
for editing the photos
Want to follow up the email sent last week. Do you have needs for photo editing? We can edit 400 images within 24 hours. We are working on all kinds of ecommerce photos, jewelry photos, and the portrait images. We do cutting out and clipping path and others, and also we provide retouching for your photos, You can throw us a photo and we will do testing for you to check our quality. Thanks, Jason
Re: [PATCH 1/7] i2c: i2c-gpio: move header to platform_data
On Mon, May 14, 2018 at 11:37:20PM +0200, Wolfram Sang wrote: > > diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c > > index 4e79dbd54a33..fa75d75b5ba9 100644 > > --- a/arch/mips/alchemy/board-gpr.c > > +++ b/arch/mips/alchemy/board-gpr.c > > @@ -29,7 +29,7 @@ > > #include > > #include > > #include > > -#include > > +#include > > #include > > #include > > #include Acked-by: James Hogan Cheers James signature.asc Description: PGP signature
,Your urgent confirmation
Attn: Beneficiary, We have contacted the Federal Ministry of Finance on your Behalf and they have brought a solution to your problem by coordinating your payment in total (10,000,000.00) Ten Million Dollars in an atm card which you can use to withdraw money from any ATM MACHINE CENTER anywhere in the world with a maximum of 1 Dollars daily. You now have the lawful right to claim your fund in an atm card. Since the Federal Bureau of Investigation is involved in this transaction, you have to be rest assured for this is 100% risk free it is our duty to protect the American Citizens, European Citizens, Asian Citizen. All I want you to do is to contact the atm card CENTER Via email or call the office telephone number one of the Consultant will assist you for their requirements to proceed and procure your Approval Slip on your behalf. CONTACT INFORMATION NAME: James Williams EMAIL: paymasterofficed...@gmail.com Do contact us with your details: Full name// Address// Age// Telephone Numbers// Occupation// Your Country// Bank Details// So your files would be updated after which the Delivery of your atm card will be affected to your designated home Address without any further delay and the bank will transfer your funds in total (10,000,000.00) Ten Million Dollars to your Bank account. We will reply you with the secret code (1600 atm card). We advice you get back to the payment office after you have contacted the ATM SWIFT CARD CENTER and we do await your response so we can move on with our Investigation and make sure your ATM SWIFT CARD gets to you. Best Regards James Williams Paymaster General Federal Republic Of Nigeri
[GIT PULL] Remove metag architecture
Hi Arnd, On Fri, Feb 23, 2018 at 01:26:09PM +0100, Arnd Bergmann wrote: > On Fri, Feb 23, 2018 at 12:02 PM, James Hogan wrote: > > I'm happy to put v2 in linux-next now (only patch 4 has changed, I just > > sent an updated version), and send you a pull request early next week so > > you can take it from there. The patches can't be directly applied with > > git-am anyway thanks to the -D option to make them more concise. > > > > Sound okay? > > Yes, sounds good, thanks! As discussed, here is a tagged branch to remove arch/metag and dependent drivers. Its basically v2 with some acks added. Cheers James The following changes since commit 91ab883eb21325ad80f3473633f794c78ac87f51: Linux 4.16-rc2 (2018-02-18 17:29:42 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/metag.git tags/metag_remove for you to fetch changes up to ef9fb83815db7d7e03da9a0904b4ef352e633922: i2c: img-scb: Drop METAG dependency (2018-02-26 14:58:09 +) Remove metag architecture These patches remove the metag architecture and tightly dependent drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4 based metag toolchain we have been using is hitting compiler bugs, so now seems a good time to drop it altogether. ---- James Hogan (13): metag: Remove arch/metag/ docs: Remove metag docs docs: Remove remaining references to metag Drop a bunch of metag references irqchip: Remove metag irqchip drivers clocksource: Remove metag generic timer driver tty: Remove metag DA TTY and console driver MAINTAINERS/CREDITS: Drop METAG ARCHITECTURE pinctrl: Drop TZ1090 drivers gpio: Drop TZ1090 drivers watchdog: imgpdc: Drop METAG dependency media: img-ir: Drop METAG dependency i2c: img-scb: Drop METAG dependency CREDITS|5 + Documentation/00-INDEX |2 - Documentation/admin-guide/kernel-parameters.txt|4 - Documentation/dev-tools/kmemleak.rst |2 +- .../devicetree/bindings/gpio/gpio-tz1090-pdc.txt | 45 - .../devicetree/bindings/gpio/gpio-tz1090.txt | 88 - Documentation/devicetree/bindings/metag/meta.txt | 30 - .../bindings/pinctrl/img,tz1090-pdc-pinctrl.txt| 127 -- .../bindings/pinctrl/img,tz1090-pinctrl.txt| 227 --- .../features/core/BPF-JIT/arch-support.txt |1 - .../core/generic-idle-thread/arch-support.txt |1 - .../features/core/jump-labels/arch-support.txt |1 - .../features/core/tracehook/arch-support.txt |1 - .../features/debug/KASAN/arch-support.txt |1 - .../debug/gcov-profile-all/arch-support.txt|1 - Documentation/features/debug/kgdb/arch-support.txt |1 - .../debug/kprobes-on-ftrace/arch-support.txt |1 - .../features/debug/kprobes/arch-support.txt|1 - .../features/debug/kretprobes/arch-support.txt |1 - .../features/debug/optprobes/arch-support.txt |1 - .../features/debug/stackprotector/arch-support.txt |1 - .../features/debug/uprobes/arch-support.txt|1 - .../debug/user-ret-profiler/arch-support.txt |1 - .../features/io/dma-api-debug/arch-support.txt |1 - .../features/io/dma-contiguous/arch-support.txt|1 - .../features/io/sg-chain/arch-support.txt |1 - .../features/lib/strncasecmp/arch-support.txt |1 - .../locking/cmpxchg-local/arch-support.txt |1 - .../features/locking/lockdep/arch-support.txt |1 - .../locking/queued-rwlocks/arch-support.txt|1 - .../locking/queued-spinlocks/arch-support.txt |1 - .../locking/rwsem-optimized/arch-support.txt |1 - .../features/perf/kprobes-event/arch-support.txt |1 - .../features/perf/perf-regs/arch-support.txt |1 - .../features/perf/perf-stackdump/arch-support.txt |1 - .../sched/membarrier-sync-core/arch-support.txt|1 - .../features/sched/numa-balancing/arch-support.txt |1 - .../seccomp/seccomp-filter/arch-support.txt|1 - .../time/arch-tick-broadcast/arch-support.txt |1 - .../features/time/clockevents/arch-support.txt |1 - .../time/context-tracking/arch-support.txt |1 - .../features/time/irq-time-acct/arch-support.txt |1 - .../time/modern-timekeeping/arch-support.txt |1 - .../features/time/virt-cpuacct/arch-support.txt|1 - .../features/vm/ELF-ASLR/arch-support.txt |1 - .../features/vm/PG_uncached/arch-support.txt |1 - Documentation/features/vm/THP/arch-support.txt |1 - Documentation/features/vm/TLB/arch-support.txt |1 - .../features/vm/huge-
Re: [PATCH 00/13] Remove metag architecture
On Fri, Feb 23, 2018 at 11:26:58AM +0100, Arnd Bergmann wrote: > On Thu, Feb 22, 2018 at 12:38 AM, James Hogan wrote: > > So lets call it a day and drop the Meta architecture port from the > > kernel. RIP Meta. > > Since I brought up the architecture removal independently, I could > pick this up into a git tree that also has the removal of some of the > other architectures. > > I see your tree is part of linux-next, so you could also just put it > in there and send a pull request at the merge window if you prefer. > > The only real reason I see for a shared git tree would be to avoid > conflicts when we touch the same Kconfig files or #ifdefs in driver, > but Meta only appears in > > config FRAME_POINTER > bool "Compile the kernel with frame pointers" > depends on DEBUG_KERNEL && \ > (CRIS || M68K || FRV || UML || \ > SUPERH || BLACKFIN || MN10300 || METAG) || \ > ARCH_WANT_FRAME_POINTERS > > and > > include/trace/events/mmflags.h:#elif defined(CONFIG_PARISC) || > defined(CONFIG_METAG) || defined(CONFIG_IA64) > > so there is little risk. I'm happy to put v2 in linux-next now (only patch 4 has changed, I just sent an updated version), and send you a pull request early next week so you can take it from there. The patches can't be directly applied with git-am anyway thanks to the -D option to make them more concise. Sound okay? Thanks James signature.asc Description: Digital signature
Re: [PATCH 00/13] Remove metag architecture
On Thu, Feb 22, 2018 at 10:26:54AM +0100, Peter Zijlstra wrote: > On Wed, Feb 21, 2018 at 11:38:12PM +0000, James Hogan wrote: > > So lets call it a day and drop the Meta architecture port from the > > kernel. RIP Meta. > > So long, and thanks for all the fish! > > Nice cleanup though, most welcome :-) I thought you might like it ;-) > Acked-by: Peter Zijlstra (Intel) Thanks James signature.asc Description: Digital signature
[PATCH 00/13] Remove metag architecture
These patches remove the metag architecture and tightly dependent drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4 based metag toolchain we have been using is hitting compiler bugs, so now seems a good time to drop it altogether. Quoting from patch 1: The earliest Meta architecture port of Linux I have a record of was an import of a Meta port of Linux v2.4.1 in February 2004, which was worked on significantly over the next few years by Graham Whaley, Will Newton, Matt Fleming, myself and others. Eventually the port was merged into mainline in v3.9 in March 2013, not long after Imagination Technologies bought MIPS Technologies and shifted its CPU focus over to the MIPS architecture. As a result, though the port was maintained for a while, kept on life support for a while longer, and useful for testing a few specific drivers for which I don't have ready access to the equivalent MIPS hardware, it is now essentially dead with no users. It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which is no longer maintained, now struggles to build modern kernels due to toolchain bugs, and doesn't itself build with a modern GCC. The latest buildroot port is still using an old uClibc snapshot which is no longer served, and the latest uClibc doesn't build with GCC 4.2.4. So lets call it a day and drop the Meta architecture port from the kernel. RIP Meta. Cc: Guenter Roeck Cc: Jonathan Corbet Cc: Steven Rostedt Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Arnaldo Carvalho de Melo Cc: Alexander Shishkin Cc: Jiri Olsa Cc: Namhyung Kim Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier Cc: Daniel Lezcano Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Linus Walleij Cc: Wim Van Sebroeck Cc: Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab Cc: Wolfram Sang Cc: linux-me...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux...@kvack.org Cc: linux-g...@vger.kernel.org Cc: linux-watch...@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: linux-...@vger.kernel.org James Hogan (13): metag: Remove arch/metag/ docs: Remove metag docs docs: Remove remaining references to metag Drop a bunch of metag references irqchip: Remove metag irqchip drivers clocksource: Remove metag generic timer driver tty: Remove metag DA TTY and console driver MAINTAINERS/CREDITS: Drop METAG ARCHITECTURE pinctrl: Drop TZ1090 drivers gpio: Drop TZ1090 drivers watchdog: imgpdc: Drop METAG dependency media: img-ir: Drop METAG dependency i2c: img-scb: Drop METAG dependency CREDITS|5 + Documentation/00-INDEX |2 - Documentation/admin-guide/kernel-parameters.txt|4 - Documentation/dev-tools/kmemleak.rst |2 +- .../devicetree/bindings/gpio/gpio-tz1090-pdc.txt | 45 - .../devicetree/bindings/gpio/gpio-tz1090.txt | 88 - Documentation/devicetree/bindings/metag/meta.txt | 30 - .../bindings/pinctrl/img,tz1090-pdc-pinctrl.txt| 127 -- .../bindings/pinctrl/img,tz1090-pinctrl.txt| 227 --- .../features/core/BPF-JIT/arch-support.txt |1 - .../core/generic-idle-thread/arch-support.txt |1 - .../features/core/jump-labels/arch-support.txt |1 - .../features/core/tracehook/arch-support.txt |1 - .../features/debug/KASAN/arch-support.txt |1 - .../debug/gcov-profile-all/arch-support.txt|1 - Documentation/features/debug/kgdb/arch-support.txt |1 - .../debug/kprobes-on-ftrace/arch-support.txt |1 - .../features/debug/kprobes/arch-support.txt|1 - .../features/debug/kretprobes/arch-support.txt |1 - .../features/debug/optprobes/arch-support.txt |1 - .../features/debug/stackprotector/arch-support.txt |1 - .../features/debug/uprobes/arch-support.txt|1 - .../debug/user-ret-profiler/arch-support.txt |1 - .../features/io/dma-api-debug/arch-support.txt |1 - .../features/io/dma-contiguous/arch-support.txt|1 - .../features/io/sg-chain/arch-support.txt |1 - .../features/lib/strncasecmp/arch-support.txt |1 - .../locking/cmpxchg-local/arch-support.txt |1 - .../features/locking/lockdep/arch-support.txt |1 - .../locking/queued-rwlocks/arch-support.txt|1 - .../locking/queued-spinlocks/arch-support.txt |1 - .../locking/rwsem-optimized/arch-support.txt |1 - .../features/perf/kprobes-event/arch-support.txt |1 - .../features/perf/perf-regs/arch-support.txt |1 - .../features/perf/perf-stackdump/arch-support.txt |1 - .../sched/membarrier-sync-core/arch-support.txt|1 - .../features/sched/numa-balancing/arch-support.txt |1 - .../seccomp/seccomp-filter/arch-support.txt|1 - .../time/arch-tick-broadcast/arch-support.txt |1 - .../features/time/clockevents/arch-support.txt |
[PATCH 12/13] media: img-ir: Drop METAG dependency
Now that arch/metag/ has been removed, remove the METAG dependency from the IMG IR device driver. The hardware is also present on MIPS SoCs so the driver still has value. Signed-off-by: James Hogan Cc: Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org Cc: linux-me...@vger.kernel.org --- drivers/media/rc/img-ir/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig index a896d3c83a1c..d2c6617d468e 100644 --- a/drivers/media/rc/img-ir/Kconfig +++ b/drivers/media/rc/img-ir/Kconfig @@ -1,7 +1,7 @@ config IR_IMG tristate "ImgTec IR Decoder" depends on RC_CORE - depends on METAG || MIPS || COMPILE_TEST + depends on MIPS || COMPILE_TEST select IR_IMG_HW if !IR_IMG_RAW help Say Y or M here if you want to use the ImgTec infrared decoder -- 2.13.6
Re: [PATCH 04/12] media: img-ir-hw: fix one kernel-doc comment
On Wed, Nov 29, 2017 at 05:46:25AM -0500, Mauro Carvalho Chehab wrote: > Needed to suppress the following warnings: > drivers/media/rc/img-ir/img-ir-hw.c:351: warning: No description found > for parameter 'reg_timings' > drivers/media/rc/img-ir/img-ir-hw.c:351: warning: Excess function > parameter 'timings' description in 'img_ir_decoder_convert' > > Signed-off-by: Mauro Carvalho Chehab Very true. Acked-by: James Hogan Thanks James > --- > drivers/media/rc/img-ir/img-ir-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c > index f54bc5d23893..ec4ded84cd17 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -339,7 +339,7 @@ static void img_ir_decoder_preprocess(struct > img_ir_decoder *decoder) > /** > * img_ir_decoder_convert() - Generate internal timings in decoder. > * @decoder: Decoder to be converted to internal timings. > - * @timings: Timing register values. > + * @reg_timings: Timing register values. > * @clock_hz:IR clock rate in Hz. > * > * Fills out the repeat timings and timing register values for a specific > clock > -- > 2.14.3 > signature.asc Description: Digital signature
Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
On Tue, 2017-03-14 at 12:29 +, Reshetova, Elena wrote: > > Elena Reshetova writes: > > > > > refcount_t type and corresponding API should be > > > used instead of atomic_t when the variable is used as > > > a reference counter. This allows to avoid accidental > > > refcounter overflows that might lead to use-after-free > > > situations. > > > > > > Signed-off-by: Elena Reshetova > > > Signed-off-by: Hans Liljestrand > > > Signed-off-by: Kees Cook > > > Signed-off-by: David Windsor > > > --- > > > drivers/md/md.c | 6 +++--- > > > drivers/md/md.h | 3 ++- > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > When booting linux-next (specifically 5be4921c9958ec) I'm seeing > > the > > backtrace below. I suspect this patch is just exposing an existing > > issue? > > Yes, we have actually been following this issue in the another > thread. > It looks like the object is re-used somehow, but I can't quite > understand how just by reading the code. > This was what I put into the previous thread: > > "The log below indicates that you are using your refcounter in a bit > weird way in mddev_find(). > However, I can't find the place (just by reading the code) where you > would increment refcounter from zero (vs. setting it to one). > It looks like you either iterate over existing nodes (and increment > their counters, which should be >= 1 at the time of increment) or > create a new node, but then mddev_init() sets the counter to 1. " > > If you can help to understand what is going on with the object > creation/destruction, would be appreciated! > > Also Shaohua Li stopped this patch coming from his tree since the > issue was caught at that time, so we are not going to merge this > until we figure it out. Asking on the correct list (dm-devel) would have got you the easy answer: The refcount behind mddev->active is a genuine atomic. It has refcount properties but only if the array fails to initialise (in that case, final put kills it). Once it's added to the system as a gendisk, it cannot be freed until md_free(). Thus its ->active count can go to zero (when it becomes inactive; usually because of an unmount). On a simple allocation regardless of outcome, the last executed statement in md_alloc is mddev_put(): that destroys the device if we didn't manage to create it or returns 0 and adds an inactive device to the system which the user can get with mddev_find(). James
Re: [PATCH v5 02/18] [media] img-ir: use new wakeup_protocols sysfs mechanism
Hi Sean, On Tue, Dec 13, 2016 at 07:54:16AM +, Sean Young wrote: > So that leaves the question open of whether we want to guess the protocol > variant from the scancode for img-ir or if we can live with having to > select this using wakeup_protocols. Having to do this does solve the issue > of the driver guessing the wrong protocol if the higher bits happen to be > 0 in the scancode. I've received confirmation that pistachio doesn't yet support suspend in mainline, in which case there can never be any real users of the old semantics on current/old mainline kernel versions. So I'm fine with it changing. Cheers James signature.asc Description: Digital signature
Re: [PATCH v5 02/18] [media] img-ir: use new wakeup_protocols sysfs mechanism
Hi Sean (and Sifan), On Mon, Dec 12, 2016 at 09:13:43PM +, Sean Young wrote: > Rather than guessing what variant a scancode is from its length, > use the new wakeup_protocol. > > Signed-off-by: Sean Young > Cc: James Hogan > Cc: Sifan Naeem > --- > drivers/media/rc/img-ir/img-ir-hw.c| 2 +- > drivers/media/rc/img-ir/img-ir-hw.h| 2 +- > drivers/media/rc/img-ir/img-ir-jvc.c | 2 +- > drivers/media/rc/img-ir/img-ir-nec.c | 6 +++--- > drivers/media/rc/img-ir/img-ir-rc5.c | 2 +- > drivers/media/rc/img-ir/img-ir-rc6.c | 2 +- > drivers/media/rc/img-ir/img-ir-sanyo.c | 2 +- > drivers/media/rc/img-ir/img-ir-sharp.c | 2 +- > drivers/media/rc/img-ir/img-ir-sony.c | 11 +++ > 9 files changed, 13 insertions(+), 18 deletions(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c > index 1a0811d..841d9d7 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -488,7 +488,7 @@ static int img_ir_set_filter(struct rc_dev *dev, enum > rc_filter_type type, > /* convert scancode filter to raw filter */ > filter.minlen = 0; > filter.maxlen = ~0; > - ret = hw->decoder->filter(sc_filter, &filter, hw->enabled_protocols); > + ret = hw->decoder->filter(sc_filter, &filter, dev->wakeup_protocol); According to patch 1, wakeup_protocol can always be set to RC_TYPE_UNKNOWN using the protocol "none", but this function is used for the normal filter too. AFAICT that would make it impossible to set a normal filter without first setting the (new) wakeup protocol too. Technically when type == RC_FILTER_NORMAL, the protocol should be based on enabled_protocols, which should be set to a single protocol group. I'll also note that enforcing that a wakeup protocol is set before setting the wakeup filter (in patch 1 which I'm not Cc'd on) is an incompatible API change. The old API basically meant that a mask of 0 disabled the wakeup filter, and there was no wakeup_protocol to set. If wakeup filters can be changed to still be writable when wakeup protocol is not set, then I suppose this driver could do something like: if (type == RC_TYPE_NORMAL) { use hw->enabled_protocols; } else if (type == RC_TYPE_WAKEUP) { if (dev->wakeup_protocol == RC_TYPE_UNKNOWN) use hw->enabled_protocols; else use 1 << dev->wakeup_protocol; } Clearly allowing a wakeup filter with no protocol is not ideal though. It probably isn't a big deal from the img-ir point of view for those semantics to change slightly. The TZ1090 SoC I originally wrote the driver for is practically obsolete from my point of view, and the common clk and power management drivers to make this driver/feature functional was never merged into mainline. Sifan: Does the MIPS pistachio SoC support wake up using img-ir, and does it even support suspend to RAM in mainline yet? If not then its impossible to utilise the wake filters in current kernels and changing the semantics is probably fine. Cheers James > if (ret) > goto unlock; > dev_dbg(priv->dev, "IR raw %sfilter=%016llx & %016llx\n", > diff --git a/drivers/media/rc/img-ir/img-ir-hw.h > b/drivers/media/rc/img-ir/img-ir-hw.h > index 91a2977..e1959ddc 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.h > +++ b/drivers/media/rc/img-ir/img-ir-hw.h > @@ -179,7 +179,7 @@ struct img_ir_decoder { > int (*scancode)(int len, u64 raw, u64 enabled_protocols, > struct img_ir_scancode_req *request); > int (*filter)(const struct rc_scancode_filter *in, > - struct img_ir_filter *out, u64 protocols); > + struct img_ir_filter *out, enum rc_type protocol); > }; > > extern struct img_ir_decoder img_ir_nec; > diff --git a/drivers/media/rc/img-ir/img-ir-jvc.c > b/drivers/media/rc/img-ir/img-ir-jvc.c > index d3e2fc0..10b302c 100644 > --- a/drivers/media/rc/img-ir/img-ir-jvc.c > +++ b/drivers/media/rc/img-ir/img-ir-jvc.c > @@ -30,7 +30,7 @@ static int img_ir_jvc_scancode(int len, u64 raw, u64 > enabled_protocols, > > /* Convert JVC scancode to JVC data filter */ > static int img_ir_jvc_filter(const struct rc_scancode_filter *in, > - struct img_ir_filter *out, u64 protocols) > + struct img_ir_filter *out, enum rc_type protocol) > { > unsigned int cust, data; > unsigned int cust_m, data_m; > diff --git a/drivers/media/rc/img-ir/img-ir-nec.c > b/drivers/media/rc/img-ir/img-ir-nec.c > index 0931493..fff00d4 100644 > --- a/drivers/
Re: [Ksummit-discuss] Including images on Sphinx documents
On Mon, 2016-11-21 at 12:06 -0200, Mauro Carvalho Chehab wrote: > Em Mon, 21 Nov 2016 11:39:41 +0100 > Johannes Berg escreveu: > > On Sat, 2016-11-19 at 10:15 -0700, Jonathan Corbet wrote: > > > > > Rather than beating our heads against the wall trying to convert > > > between various image formats, maybe we need to take a step > > > back. We're trying to build better documentation, and there is > > > certainly a place for diagrams and such in that > > > documentation. Johannes was asking about it for the 802.11 docs, > > > and I know Paul has run into these issues with the RCU docs as > > > well. Might there be a tool or an extension out there that would > > > allow us to express these diagrams in a text-friendly, editable > > > form? > > > > > > With some effort, I bet we could get rid of a number of the > > > images, and perhaps end up with something that makes sense when > > > read in the .rst source files as an extra benefit. > > > > I tend to agree, and I think that having this readable in the text > > would be good. > > > > You had pointed me to this plugin before > > https://pythonhosted.org/sphinxcontrib-aafig/ > > > > but I don't think it can actually represent any of the pictures. > > No, but there are some ascii art images inside some txt/rst files > and inside some kernel-doc comments. We could either use the above > extension for them or to convert into some image. The ascii art > images I saw seem to be diagrams, so Graphviz would allow replacing > most of them, if not all. Please don't replace ASCII art that effectively conveys conceptual diagrams. If you do, we'll wind up in situations where someone hasn't built the docs and doesn't possess the tools to see a diagram that was previously shown by every text editor (or can't be bothered to dig out the now separate file). In the name of creating "prettier" diagrams (and final doc), we'll have damaged capacity to understand stuff by just reading the source if this diagram is in kernel doc comments. I think this is a good application of "if it ain't broke, don't fix it". James -- 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: [Ksummit-discuss] Including images on Sphinx documents
On Thu, 2016-11-17 at 13:16 -0200, Mauro Carvalho Chehab wrote: > Hi Ted, > > Em Thu, 17 Nov 2016 09:52:44 -0500 > Theodore Ts'o escreveu: > > > On Thu, Nov 17, 2016 at 12:07:15PM +0100, Arnd Bergmann wrote: > > > [adding Linus for clarification] > > > > > > I understood the concern as being about binary files that you > > > cannot > > > modify with classic 'patch', which is a separate issue. > > > > I think the other complaint is that the image files aren't "source" > > in > > the proper term, since they are *not* the preferred form for > > modification --- that's the svg files. Beyond the license > > compliance > > issues (which are satisified because the .svg files are included in > > the git tree), there is the SCM cleaniless argument of not > > including > > generated files in the distribution, since this increases the > > opportunites for the "real" source file and the generated source > > file > > to get out of sync. (As just one example, if the patch can't > > represent the change to binary file.) > > > > I do check in generated files on occasion --- usually because I > > don't > > trust autoconf to be a stable in terms of generating a correct > > configure file from a configure.in across different versions of > > autoconf and different macro libraries that might be installed on > > the > > system. So this isn't a hard and fast rule by any means (although > > Linus may be more strict than I on that issue). > > > > I don't understand why it's so terrible to have generate the image > > file from the .svg file in a Makefile rule, and then copy it > > somewhere > > else if Sphinx is too dumb to fetch it from the normal location? > > The images whose source are in .svg are now generated via Makefile > for the PDF output (after my patches, already applied to the docs > -next > tree). > > So, the problem that remains is for those images whose source > is a bitmap. If we want to stick with the Sphinx supported formats, > we have only two options for bitmaps: jpg or png. We could eventually > use uuencode or base64 to make sure that the patches won't use > git binary diff extension, or, as Arnd proposed, use a portable > bitmap format, in ascii, converting via Makefile, but losing > the alpha channel with makes the background transparent. If it can use svg, why not use that? SVG files can be a simple xml wrapper around a wide variety of graphic image formats which are embedded in the svg using the data-uri format, you know ... Anything that handles SVGs should be able to handle all the embeddable image formats, which should give you a way around image restrictions whatever it is would otherwise have. James -- 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 1/1] subsystem:linux-media CVE-2016-5400
This patch addresses CVE-2016-5400, a local DOS vulnerability caused by a memory leak in the airspy usb device driver. The vulnerability is triggered when more than 64 usb devices register with v4l2 of type VFL_TYPE_SDR or VFL_TYPE_SUBDEV.A badusb device can emulate 64 of these devices then through continual emulated connect/disconnect of the 65th device, cause the kernel to run out of RAM and crash the kernel. The vulnerability exists in kernel versions from 3.17 to current 4.7. The memory leak is caused by the probe function of the airspy driver mishandeling errors and not freeing the corresponding control structures when an error occours registering the device to v4l2 core. Signed-off-by: James Patrick-Evans --- drivers/media/usb/airspy/airspy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c index 87c1293..6c3ac8b 100644 --- a/drivers/media/usb/airspy/airspy.c +++ b/drivers/media/usb/airspy/airspy.c @@ -1072,7 +1072,7 @@ static int airspy_probe(struct usb_interface *intf, if (ret) { dev_err(s->dev, "Failed to register as video device (%d)\n", ret); - goto err_unregister_v4l2_dev; + goto err_free_controls; } dev_info(s->dev, "Registered as %s\n", video_device_node_name(&s->vdev)); -- 1.9.1 -- 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: IR remote stopped working in kernels 4.5 and 4.6
On Mon, 2016-07-04 at 23:08 +0200, Heiner Kallweit wrote: > Am 04.07.2016 um 22:36 schrieb James Bottomley: > > This looks to be a problem with the rc subsystem. The IR > > controller in question is part of a cx8800 atsc card. In the 4.4 > > kernel, where it works, this is what ir-keytable says: > > > > Found /sys/class/rc/rc0/ (/dev/input/event12) with: > > Driver cx88xx, table rc-hauppauge > > Supported protocols: other lirc rc-5 jvc sony nec sanyo mce-kbd > > rc-6 sharp xmp > > Enabled protocols: lirc nec > > Name: cx88 IR (pcHDTV HD3000 HDTV) > > bus: 1, vendor/product: 7063:3000, version: 0x0001 > > Repeat delay = 500 ms, repeat period = 125 ms > > > > And in 4.6, where it doesn't work: > > > > Found /sys/class/rc/rc0/ (/dev/input/event12) with: > > Driver cx88xx, table rc-hauppauge > > Supported protocols: lirc > > Enabled protocols: lirc > > Name: cx88 IR (pcHDTV HD3000 HDTV) > > bus: 1, vendor/product: 7063:3000, version: 0x0001 > > Repeat delay = 500 ms, repeat period = 125 ms > > > > The particular remote in question seems to require the nec protocol > > to work and the failure in 4.5 and 4.6 is having any supported > > protocols at all. I can get the remote to start working again by > > adding the nec protocol: > > > > echo nec > /sys/class/rc/rc0/protocols > > > > But it would be nice to have this happen by default rather than > > having to add yet another work around init script. > > > Meanwhile decoder modules are loaded on demand only. This can be done > automatically w/o the need for additional init scripts. > > If /etc/rc_maps.cfg includes a keymap with type NEC then the nec > decoder module is loaded automatically. > > My rc_maps.cfg looks like this (and causes the SONY decoder module to > be loaded automatically): > > #driver tablefile > * *sony-rm-sx800 > > And the keymap: > > # table sony-rm-sx800, type SONY > 0x110030KEY_PREVIOUS > 0x110031KEY_NEXT > 0x110033KEY_BACK > .. > .. Well, to work in the 4.4 kernel, the rc_maps.cfg names a table with the nec controller, so it seems that whatever is supposed to trigger autoloading isn't. Is ir-keytable supposed to do this? The current debian testing one is Package: ir-keytable Source: v4l-utils Version: 1.10.1-1 Which seems to be the most current one. James -- 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
IR remote stopped working in kernels 4.5 and 4.6
This looks to be a problem with the rc subsystem. The IR controller in question is part of a cx8800 atsc card. In the 4.4 kernel, where it works, this is what ir-keytable says: Found /sys/class/rc/rc0/ (/dev/input/event12) with: Driver cx88xx, table rc-hauppauge Supported protocols: other lirc rc-5 jvc sony nec sanyo mce-kbd rc-6 sharp xmp Enabled protocols: lirc nec Name: cx88 IR (pcHDTV HD3000 HDTV) bus: 1, vendor/product: 7063:3000, version: 0x0001 Repeat delay = 500 ms, repeat period = 125 ms And in 4.6, where it doesn't work: Found /sys/class/rc/rc0/ (/dev/input/event12) with: Driver cx88xx, table rc-hauppauge Supported protocols: lirc Enabled protocols: lirc Name: cx88 IR (pcHDTV HD3000 HDTV) bus: 1, vendor/product: 7063:3000, version: 0x0001 Repeat delay = 500 ms, repeat period = 125 ms The particular remote in question seems to require the nec protocol to work and the failure in 4.5 and 4.6 is having any supported protocols at all. I can get the remote to start working again by adding the nec protocol: echo nec > /sys/class/rc/rc0/protocols But it would be nice to have this happen by default rather than having to add yet another work around init script. James -- 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: Build regressions/improvements in v4.5-rc7
Hi, On Mon, Mar 07, 2016 at 10:01:09AM +0100, Geert Uytterhoeven wrote: > On Mon, Mar 7, 2016 at 9:55 AM, Geert Uytterhoeven > wrote: > > JFYI, when comparing v4.5-rc7[1] to v4.5-rc6[3], the summaries are: > > - build errors: +8/-7 > + error: debugfs.c: undefined reference to `clk_round_rate': => > .text+0x11b9e0) > > arm-randconfig > > While looking for more context, I noticed another regression that fell through > the cracks of my script: > > arch/arm/kernel/head.o: In function `stext': > (.head.text+0x40): undefined reference to `CONFIG_PHYS_OFFSET' > drivers/built-in.o: In function `v4l2_clk_set_rate': > debugfs.c:(.text+0x11b9e0): undefined reference to `clk_round_rate' > > + error: misc.c: undefined reference to `ftrace_likely_update': => > .text+0x714), .text+0x94c), .text+0x3b8), .text+0xc10) > > sh-randconfig > > arch/sh/boot/compressed/misc.o: In function `lzo1x_decompress_safe': > misc.c:(.text+0x3b8): undefined reference to `ftrace_likely_update' > misc.c:(.text+0x714): undefined reference to `ftrace_likely_update' > misc.c:(.text+0x94c): undefined reference to `ftrace_likely_update' > arch/sh/boot/compressed/misc.o: In function `unlzo.constprop.2': > misc.c:(.text+0xc10): undefined reference to `ftrace_likely_update' > > + /tmp/cc52LvuK.s: Error: can't resolve `_start' {*UND* section} - > `L0^A' {.text section}: => 41, 403 > + /tmp/ccHfoDA4.s: Error: can't resolve `_start' {*UND* section} - > `L0^A' {.text section}: => 43 > + /tmp/cch1r0UQ.s: Error: can't resolve `_start' {*UND* section} - > `L0^A' {.text section}: => 49, 378 > + /tmp/ccoHdFI8.s: Error: can't resolve `_start' {*UND* section} - > `L0^A' {.text section}: => 43 > > mips-allnoconfig > bigsur_defconfig > malta_defconfig > cavium_octeon_defconfig > > Not really new, but it would be great if the MIPS people could get this > fixed for the final release. This would appear to be related to the ld version check for VDSO failing. awk: /home/kisskb/slave/src/scripts/ld-version.sh: line 4: regular expression compile failed (missing '(') .*) /bin/sh: 1: [: -lt: unexpected operator I.e. this line: gsub(".*)", ""); appears to be trying to turn e.g. "GNU ld (Gentoo 2.25.1 p1.1) 2.25.1" into " 2.25.1", so perhaps the bracket should be escaped. What version of awk is it using? (GNU Awk 4.0.2 works for me). Can somebody experiencing this please try: ${CROSS_COMPILE}ld --version | ./scripts/ld-version.sh with the following patch, to see if it helps. Thanks James diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index 198580d245e0..1659b409ef10 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -1,7 +1,7 @@ #!/usr/bin/awk -f # extract linker version number from stdin and turn into single number { - gsub(".*)", ""); + gsub(".*\\)", ""); split($1,a, "."); print a[1]*1000 + a[2]*10 + a[3]*1 + a[4]*100 + a[5]; exit signature.asc Description: Digital signature
Re: [PATCH v5 0/3] RFC: Secure Memory Allocation Framework
On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > > The outcome of the previous RFC about how do secure data path was the need > of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551) > Have you addressed all the questions raised by Alan here: https://lkml.org/lkml/2015/5/8/629 Also, is there any application of this beyond DRM? - James -- James Morris -- 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 v5 1/3] create SMAF module
On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > Secure Memory Allocation Framework goal is to be able > to allocate memory that can be securing. > There is so much ways to allocate and securing memory that SMAF > doesn't do it by itself but need help of additional modules. > To be sure to use the correct allocation method SMAF implement > deferred allocation (i.e. allocate memory when only really needed) > > Allocation modules (smaf-alloctor.h): > SMAF could manage with multiple allocation modules at same time. > To select the good one SMAF call match() to be sure that a module > can allocate memory for a given list of devices. It is to the module > to check if the devices are compatible or not with it allocation > method. > > Securing module (smaf-secure.h): > The way of how securing memory it is done is platform specific. > Secure module is responsible of grant/revoke memory access. > This documentation is highly inadequate. What does "allocate memory that can be securing" mean? -- James Morris -- 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: uvc-1.5
On 09/30/15 19:13, Mauro Carvalho Chehab wrote: Em Tue, 29 Sep 2015 19:06:18 -0400 James escreveu: When is the Linux kernel going to support uvc-1.5? It was made a standard on June 6, 2012 When someone writes patches adding support for it, together with the code that adds support to at least one device that uses UVC 1.5 ;) I dunno if anyone is working on that ATM. Regards, Mauro Apparently there was a patch in 2013. https://www.mail-archive.com/linux-media@vger.kernel.org/msg66199.html If the patch is in the kernel, I don't think it works any more. If the patch never made it into the kernel then I don't know why (my guess is it has something to do with QP controls). -- 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
uvc-1.5
When is the Linux kernel going to support uvc-1.5? It was made a standard on June 6, 2012 -- 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 29/31] parisc: handle page-less SG entries
On Thu, 2015-08-13 at 20:30 -0700, Dan Williams wrote: > On Thu, Aug 13, 2015 at 7:31 AM, Christoph Hellwig wrote: > > On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote: > >> I'm assuming that anybody who wants to use the page-less > >> scatter-gather lists always does so on memory that isn't actually > >> virtually mapped at all, or only does so on sane architectures that > >> are cache coherent at a physical level, but I'd like that assumption > >> *documented* somewhere. > > > > It's temporarily mapped by kmap-like helpers. That code isn't in > > this series. The most recent version of it is here: > > > > https://git.kernel.org/cgit/linux/kernel/git/djbw/nvdimm.git/commit/?h=pfn&id=de8237c99fdb4352be2193f3a7610e902b9bb2f0 > > > > note that it's not doing the cache flushing it would have to do yet, but > > it's also only enabled for x86 at the moment. > > For virtually tagged caches I assume we would temporarily map with > kmap_atomic_pfn_t(), similar to how drm_clflush_pages() implements > powerpc support. However with DAX we could end up with multiple > virtual aliases for a page-less pfn. At least on some PA architectures, you have to be very careful. Improperly managed, multiple aliases will cause the system to crash (actually a machine check in the cache chequerboard). For the most temperamental systems, we need the cache line flushed and the alias mapping ejected from the TLB cache before we access the same page at an inequivalent alias. James -- 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: prepare for struct scatterlist entries without page backing
On Wed, 2015-08-12 at 09:05 +0200, Christoph Hellwig wrote: > Dan Williams started to look into addressing I/O to and from > Persistent Memory in his series from June: > > http://thread.gmane.org/gmane.linux.kernel.cross-arch/27944 > > I've started looking into DMA mapping of these SGLs specifically instead > of the map_pfn method in there. In addition to supporting NVDIMM backed > I/O I also suspect this would be highly useful for media drivers that > go through nasty hoops to be able to DMA from/to their ioremapped regions, > with vb2_dc_get_userptr in drivers/media/v4l2-core/videobuf2-dma-contig.c > being a prime example for the unsafe hacks currently used. > > It turns out most DMA mapping implementation can handle SGLs without > page structures with some fairly simple mechanical work. Most of it > is just about consistently using sg_phys. For implementations that > need to flush caches we need a new helper that skips these cache > flushes if a entry doesn't have a kernel virtual address. > > However the ccio (parisc) and sba_iommu (parisc & ia64) IOMMUs seem > to be operate mostly on virtual addresses. It's a fairly odd concept > that I don't fully grasp, so I'll need some help with those if we want > to bring this forward. I can explain that. I think this doesn't apply to ia64 because it's cache is PIPT, but on parisc, we have a VIPT cache. On normal physically indexed architectures, when the iommu sees a DMA transfer to/from physical memory, it also notifies the CPU to flush the internal CPU caches of those lines. This is usually an interlocking step of the transfer to make sure the page is coherent before transfer to/from the device (it's why the ia32 for instance is a coherent architecture). Because the system is physically indexed, there's no need to worry about aliases. On Virtually Indexed systems, like parisc, there is an aliasing problem. The CCIO iommu unit (and all other iommu systems on parisc) have what's called a local coherence index (LCI). You program it as part of the IOMMU page table and it tells the system which Virtual line in the cache to flush as part of the IO transaction, thus still ensuring cache coherence. That's why we have to know the virtual as well as physical addresses for the page. The problem we have in Linux is that we have two virtual addresses, which are often incoherent aliases: the user virtual address and a kernel virtual address but we can only make the page coherent with a single alias (only one LCI). The way I/O on Linux currently works is that get_user_pages actually flushes the user virtual address, so that's expected to be coherent, so the address we program into the VCI is the kernel virtual address. Usually nothing in the kernel has ever touched the page, so there's nothing to flush, but we do it just in case. In theory, for these non kernel page backed SG entries, we can make the process more efficient by not flushing in gup and instead programming the user virtual address into the local coherence index. However, simply zeroing the LCI will also work (except that poor VI zero line will get flushed repeatedly, so it's probably best to pick a known untouched line in the kernel). James -- 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: [Xen-devel] RIP MTRR - status update for upcoming v4.2
On Fri, 2015-06-12 at 16:15 -0700, Andy Lutomirski wrote: > On Jun 12, 2015 12:59 AM, "Jan Beulich" wrote: > > > > >>> On 12.06.15 at 01:23, wrote: > > > There are two usages on MTRRs: > > > 1) MTRR entries set by firmware > > > 2) MTRR entries set by OS drivers > > > > > > We can obsolete 2), but we have no control over 1). As UEFI firmwares > > > also set this up, this usage will continue to stay. So, we should not > > > get rid of the MTRR code that looks up the MTRR entries, while we have > > > no need to modify them. > > > > > > Such MTRR entries provide safe guard to /dev/mem, which allows > > > privileged user to access a range that may require UC mapping while > > > the /dev/mem driver blindly maps it with WB. MTRRs converts WB to UC in > > > such a case. > > > > But it wouldn't be impossible to simply read the MTRRs upon boot, > > store the information, disable MTRRs, and correctly use PAT to > > achieve the same effect (i.e. the "blindly maps" part of course > > would need fixing). > > This may crash and burn badly when we call a UEFI function or an SMI > happens. I think we should just leave the MTRRs alone. Wholeheartedly agree: PAT only works when the given memory map is in operation but MTRRs function everywhere. Anything that goes into real mode or installs its own memory map won't see the Linux page attributes. James -- 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] rc: img-ir: fix error in parameters passed to irq_free()
On Tue, Feb 10, 2015 at 10:41:56AM +, Sifan Naeem wrote: > img_ir_remove() passes a pointer to the ISR function as the 2nd > parameter to irq_free() instead of a pointer to the device data > structure. > This issue causes unloading img-ir module to fail with the below > warning after building and loading img-ir as a module. > > WARNING: CPU: 2 PID: 155 at ../kernel/irq/manage.c:1278 > __free_irq+0xb4/0x214() Trying to free already-free IRQ 58 > Modules linked in: img_ir(-) > CPU: 2 PID: 155 Comm: rmmod Not tainted 3.14.0 #55 ... > Call Trace: > ... > [<8048d420>] __free_irq+0xb4/0x214 > [<8048d6b4>] free_irq+0xac/0xf4 > [] img_ir_remove+0x54/0xd4 [img_ir] [<8073ded0>] > platform_drv_remove+0x30/0x54 ... > > Signed-off-by: Sifan Naeem > Fixes: 160a8f8aec4d ("[media] rc: img-ir: add base driver") > Cc: # 3.15+ Thanks for catching this Sifan. It appears to have been introduced while getting the driver ready for upstream (it used to use the devm_* API to request the IRQ, but I changed it to avoid the ISR racing with module removal). Acked-by: James Hogan Cheers James > --- > drivers/media/rc/img-ir/img-ir-core.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-core.c > b/drivers/media/rc/img-ir/img-ir-core.c > index 77c78de..7020659 100644 > --- a/drivers/media/rc/img-ir/img-ir-core.c > +++ b/drivers/media/rc/img-ir/img-ir-core.c > @@ -146,7 +146,7 @@ static int img_ir_remove(struct platform_device *pdev) > { > struct img_ir_priv *priv = platform_get_drvdata(pdev); > > - free_irq(priv->irq, img_ir_isr); > + free_irq(priv->irq, priv); > img_ir_remove_hw(priv); > img_ir_remove_raw(priv); > > -- > 1.7.9.5 > signature.asc Description: Digital signature
Re: [PATCH v2] rc: img-ir: Add and enable sys clock for img-ir
On 04/02/15 16:48, Sifan Naeem wrote: > Gets a handle to the system clock, already described in the binding > document, and calls the appropriate common clock framework functions > to mark it prepared/enabled, the common clock framework initially > enables the clock and doesn't disable it at least until the > device/driver is removed. > It's important the systen clock is enabled before register interface is > accessed by the driver. > The system clock to IR is needed for the driver to communicate with the > IR hardware via MMIO accesses on the system bus, so it must not be > disabled during use or the driver will malfunction. > > Signed-off-by: Sifan Naeem Thanks Sifan, looks good to me, doesn't break tz1090, and seems to still cope with no clocks provided in DT. Acked-by: James Hogan Cheers James > --- > Changes from v1: > System clock enabled in probe function before any hardware is accessed. > Error handling in probe function ensures ISR doesn't get called with > system clock disabled. > > drivers/media/rc/img-ir/img-ir-core.c | 29 + > drivers/media/rc/img-ir/img-ir.h |2 ++ > 2 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-core.c > b/drivers/media/rc/img-ir/img-ir-core.c > index 77c78de..a10d666 100644 > --- a/drivers/media/rc/img-ir/img-ir-core.c > +++ b/drivers/media/rc/img-ir/img-ir-core.c > @@ -110,16 +110,32 @@ static int img_ir_probe(struct platform_device *pdev) > priv->clk = devm_clk_get(&pdev->dev, "core"); > if (IS_ERR(priv->clk)) > dev_warn(&pdev->dev, "cannot get core clock resource\n"); > + > + /* Get sys clock */ > + priv->sys_clk = devm_clk_get(&pdev->dev, "sys"); > + if (IS_ERR(priv->sys_clk)) > + dev_warn(&pdev->dev, "cannot get sys clock resource\n"); > /* > - * The driver doesn't need to know about the system ("sys") or power > - * modulation ("mod") clocks yet > + * Enabling the system clock before the register interface is > + * accessed. ISR shouldn't get called with Sys Clock disabled, > + * hence exiting probe with an error. >*/ > + if (!IS_ERR(priv->sys_clk)) { > + error = clk_prepare_enable(priv->sys_clk); > + if (error) { > + dev_err(&pdev->dev, "cannot enable sys clock\n"); > + return error; > + } > + } > > /* Set up raw & hw decoder */ > error = img_ir_probe_raw(priv); > error2 = img_ir_probe_hw(priv); > - if (error && error2) > - return (error == -ENODEV) ? error2 : error; > + if (error && error2) { > + if (error == -ENODEV) > + error = error2; > + goto err_probe; > + } > > /* Get the IRQ */ > priv->irq = irq; > @@ -139,6 +155,9 @@ static int img_ir_probe(struct platform_device *pdev) > err_irq: > img_ir_remove_hw(priv); > img_ir_remove_raw(priv); > +err_probe: > + if (!IS_ERR(priv->sys_clk)) > + clk_disable_unprepare(priv->sys_clk); > return error; > } > > @@ -152,6 +171,8 @@ static int img_ir_remove(struct platform_device *pdev) > > if (!IS_ERR(priv->clk)) > clk_disable_unprepare(priv->clk); > + if (!IS_ERR(priv->sys_clk)) > + clk_disable_unprepare(priv->sys_clk); > return 0; > } > > diff --git a/drivers/media/rc/img-ir/img-ir.h > b/drivers/media/rc/img-ir/img-ir.h > index 2ddf560..f1387c0 100644 > --- a/drivers/media/rc/img-ir/img-ir.h > +++ b/drivers/media/rc/img-ir/img-ir.h > @@ -138,6 +138,7 @@ struct clk; > * @dev: Platform device. > * @irq: IRQ number. > * @clk: Input clock. > + * @sys_clk: System clock. > * @reg_base:Iomem base address of IR register block. > * @lock:Protects IR registers and variables in this struct. > * @raw: Driver data for raw decoder. > @@ -147,6 +148,7 @@ struct img_ir_priv { > struct device *dev; > int irq; > struct clk *clk; > + struct clk *sys_clk; > void __iomem*reg_base; > spinlock_t lock; > > signature.asc Description: OpenPGP digital signature
Re: [PATCH] rc: img-ir: Add and enable sys clock for IR
Hi Sifan, On 03/02/15 17:30, Sifan Naeem wrote: > Gets a handle to the system clock, already described in the binding > document, and calls the appropriate common clock > framework functions to mark it prepared/enabled, the common clock > framework initially enables the clock and doesn't disable it at least > until the device/driver is removed. > The system clock to IR is needed for the driver to communicate with the > IR hardware via MMIO accesses on the system bus, so it must not be > disabled during use or the driver will malfunction. > > Signed-off-by: Sifan Naeem > --- > drivers/media/rc/img-ir/img-ir-core.c | 13 + > drivers/media/rc/img-ir/img-ir.h |2 ++ > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-core.c > b/drivers/media/rc/img-ir/img-ir-core.c > index 77c78de..783dd21 100644 > --- a/drivers/media/rc/img-ir/img-ir-core.c > +++ b/drivers/media/rc/img-ir/img-ir-core.c > @@ -60,6 +60,8 @@ static void img_ir_setup(struct img_ir_priv *priv) > > if (!IS_ERR(priv->clk)) > clk_prepare_enable(priv->clk); > + if (!IS_ERR(priv->sys_clk)) > + clk_prepare_enable(priv->sys_clk); The patch mostly looks good, however I just realised this only enables the system clock after it has already used the register interface. To be safe it should really be enabled before any hardware accesses, so before img_ir_ident() is called (and certainly before the img_ir_setup_raw() and img_ir_setup_hw() functions are called since they set up the default timings / irq mask etc). Currently img_ir_probe_raw() and img_ir_probe_hw() don't appear to access the hardware, but I can imagine they may want to access hardware in future to read the revision register, so I think its worth doing it from img_ir_probe() before they get called too, which should also ensure the ISR doesn't get called with sysclock disabled (obviously error handling in probe function would need to take it into account). I suspect you would see this causing a problem when the driver is built as a module and loaded after unused clocks have been automatically disabled by the common clock framework at the end of the init sequence. Thanks James > } > > static void img_ir_ident(struct img_ir_priv *priv) > @@ -110,10 +112,11 @@ static int img_ir_probe(struct platform_device *pdev) > priv->clk = devm_clk_get(&pdev->dev, "core"); > if (IS_ERR(priv->clk)) > dev_warn(&pdev->dev, "cannot get core clock resource\n"); > - /* > - * The driver doesn't need to know about the system ("sys") or power > - * modulation ("mod") clocks yet > - */ > + > + /* Get sys clock */ > + priv->sys_clk = devm_clk_get(&pdev->dev, "sys"); > + if (IS_ERR(priv->sys_clk)) > + dev_warn(&pdev->dev, "cannot get sys clock resource\n"); > > /* Set up raw & hw decoder */ > error = img_ir_probe_raw(priv); > @@ -152,6 +155,8 @@ static int img_ir_remove(struct platform_device *pdev) > > if (!IS_ERR(priv->clk)) > clk_disable_unprepare(priv->clk); > + if (!IS_ERR(priv->sys_clk)) > + clk_disable_unprepare(priv->sys_clk); > return 0; > } > > diff --git a/drivers/media/rc/img-ir/img-ir.h > b/drivers/media/rc/img-ir/img-ir.h > index 2ddf560..f1387c0 100644 > --- a/drivers/media/rc/img-ir/img-ir.h > +++ b/drivers/media/rc/img-ir/img-ir.h > @@ -138,6 +138,7 @@ struct clk; > * @dev: Platform device. > * @irq: IRQ number. > * @clk: Input clock. > + * @sys_clk: System clock. > * @reg_base:Iomem base address of IR register block. > * @lock:Protects IR registers and variables in this struct. > * @raw: Driver data for raw decoder. > @@ -147,6 +148,7 @@ struct img_ir_priv { > struct device *dev; > int irq; > struct clk *clk; > + struct clk *sys_clk; > void __iomem*reg_base; > spinlock_t lock; > > signature.asc Description: OpenPGP digital signature
RE: [possible BUG, cx23885] Dual tuner TV card, works using one tuner only, doesn't work if both tuners are used
> > Hi, James. > > After searching for somebody posting some issues similar to mine, I think this > one you posted to the mailing list can be related: > > https://www.mail-archive.com/linux- > media%40vger.kernel.org/msg80078.html > > I'm having problems using both tuners in a dual tuner card (Terratec Cinergy T > PCIe Dual), also based on cx23885, but it uses different frontends/tuners > than yours. > I'm pretty sure mine was an actual hardware fault. At first it worked perfectly. Then after a bit the server would occasionally lock up hard or reboot, then more often, then every time. I spent ages thinking it was a driver problem and did all sorts of traces etc and found nothing. In the end I just got mythtv to not use the second tuner (or the first tuner - as long as only one was used it was fine). Then about a month ago it started locking up again occasionally, then more often, then every time, only using the one tuner. Strange though that if it booted up without locking up it would be okay. I bought a usb tuner and haven't used the cx23885 card since. I will be seeking a replacement under warranty though, as the signal quality is quite a bit better. Good luck in your quest though! James
Re: [PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
On 12/12/14 12:07, James Hogan wrote: > Hi Sifan, > > On 11/12/14 20:06, Sifan Naeem wrote: >> Biphase decoding in the current img-ir has got a quirk, where multiple >> Interrupts are generated when an incomplete IR code is received by the >> decoder. >> >> Patch adds a work around for the quirk and enables biphase decoding. >> >> Changes from v1: >> * rebased due to conflict with "img-ir/hw: Fix potential deadlock stopping >> timer" >> * spinlock taken in img_ir_suspend_timer >> * check for hw->stopping before handling quirks in img_ir_isr_hw >> * new memeber added to img_ir_priv_hw to save irq status over suspend > > For future reference, the list of changes between patchset versions is > usually put after a "---" so that it doesn't get included in the final > git commit message. You can also add any Acked-by/Reviewed-by tags > you've been given to new versions of patchset, assuming nothing > significant has changed in that patch (maintainers generally add > relevant tags for you, that are sent in response to the patches being > applied). > > Anyway, the whole patchset looks okay to me, aside from the one question > I just asked on patch 3 of v1, which I'm not so sure about. I'll let you > decide whether that needs changing since you have the hardware to verify it. > > So for the whole patchset feel free to add my: > Acked-by: James Hogan Mauro: Assuming no other changes are requested in this patchset, do you want these resent with the moving of changelogs out of the main commit messages? Cheers James signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
Hi Sifan, On 11/12/14 20:06, Sifan Naeem wrote: > Biphase decoding in the current img-ir has got a quirk, where multiple > Interrupts are generated when an incomplete IR code is received by the > decoder. > > Patch adds a work around for the quirk and enables biphase decoding. > > Changes from v1: > * rebased due to conflict with "img-ir/hw: Fix potential deadlock stopping > timer" > * spinlock taken in img_ir_suspend_timer > * check for hw->stopping before handling quirks in img_ir_isr_hw > * new memeber added to img_ir_priv_hw to save irq status over suspend For future reference, the list of changes between patchset versions is usually put after a "---" so that it doesn't get included in the final git commit message. You can also add any Acked-by/Reviewed-by tags you've been given to new versions of patchset, assuming nothing significant has changed in that patch (maintainers generally add relevant tags for you, that are sent in response to the patches being applied). Anyway, the whole patchset looks okay to me, aside from the one question I just asked on patch 3 of v1, which I'm not so sure about. I'll let you decide whether that needs changing since you have the hardware to verify it. So for the whole patchset feel free to add my: Acked-by: James Hogan Thanks James > > Signed-off-by: Sifan Naeem > --- > drivers/media/rc/img-ir/img-ir-hw.c | 60 > +-- > drivers/media/rc/img-ir/img-ir-hw.h |4 +++ > 2 files changed, 61 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c > index 9cecda7..5c32f05 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -52,6 +52,11 @@ static struct img_ir_decoder *img_ir_decoders[] = { > > #define IMG_IR_QUIRK_CODE_BROKEN 0x1 /* Decode is broken */ > #define IMG_IR_QUIRK_CODE_LEN_INCR 0x2 /* Bit length needs increment */ > +/* > + * The decoder generates rapid interrupts without actually having > + * received any new data after an incomplete IR code is decoded. > + */ > +#define IMG_IR_QUIRK_CODE_IRQ0x4 > > /* functions for preprocessing timings, ensuring max is set */ > > @@ -542,6 +547,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv, >*/ > spin_unlock_irq(&priv->lock); > del_timer_sync(&hw->end_timer); > + del_timer_sync(&hw->suspend_timer); > spin_lock_irq(&priv->lock); > > hw->stopping = false; > @@ -861,6 +867,29 @@ static void img_ir_end_timer(unsigned long arg) > spin_unlock_irq(&priv->lock); > } > > +/* > + * Timer function to re-enable the current protocol after it had been > + * cleared when invalid interrupts were generated due to a quirk in the > + * img-ir decoder. > + */ > +static void img_ir_suspend_timer(unsigned long arg) > +{ > + struct img_ir_priv *priv = (struct img_ir_priv *)arg; > + > + spin_lock_irq(&priv->lock); > + /* > + * Don't overwrite enabled valid/match IRQs if they have already been > + * changed by e.g. a filter change. > + */ > + if ((priv->hw.quirk_suspend_irq & IMG_IR_IRQ_EDGE) == > + img_ir_read(priv, IMG_IR_IRQ_ENABLE)) > + img_ir_write(priv, IMG_IR_IRQ_ENABLE, > + priv->hw.quirk_suspend_irq); > + /* enable */ > + img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl); > + spin_unlock_irq(&priv->lock); > +} > + > #ifdef CONFIG_COMMON_CLK > static void img_ir_change_frequency(struct img_ir_priv *priv, > struct clk_notifier_data *change) > @@ -926,15 +955,38 @@ void img_ir_isr_hw(struct img_ir_priv *priv, u32 > irq_status) > if (!hw->decoder) > return; > > + ct = hw->decoder->control.code_type; > + > ir_status = img_ir_read(priv, IMG_IR_STATUS); > - if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) > + if (!(ir_status & (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) { > + if (!(priv->hw.ct_quirks[ct] & IMG_IR_QUIRK_CODE_IRQ) || > + hw->stopping) > + return; > + /* > + * The below functionality is added as a work around to stop > + * multiple Interrupts generated when an incomplete IR code is > + * received by the decoder. > + * The decoder generates rapid interrupts without actually > + * having received any new data. After a si
Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
Hi Sifan, On 11/12/14 18:54, Sifan Naeem wrote: >>> +/* >>> + * Timer function to re-enable the current protocol after it had been >>> + * cleared when invalid interrupts were generated due to a quirk in >>> +the >>> + * img-ir decoder. >>> + */ >>> +static void img_ir_suspend_timer(unsigned long arg) { >>> + struct img_ir_priv *priv = (struct img_ir_priv *)arg; >>> + >>> + img_ir_write(priv, IMG_IR_IRQ_CLEAR, >>> + IMG_IR_IRQ_ALL & ~IMG_IR_IRQ_EDGE); >>> + >>> + /* Don't set IRQ if it has changed in a different context. */ >> >> Should you even be clearing IRQs in that case? Maybe safer to just treat that >> case as a "return immediately without touching anything" sort of situation. >> > don't have to clear it for this work around to work, so will remove. > >>> + if ((priv->hw.suspend_irqen & IMG_IR_IRQ_EDGE) == >>> + img_ir_read(priv, IMG_IR_IRQ_ENABLE)) >>> + img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv- >>> hw.suspend_irqen); >>> + /* enable */ >>> + img_ir_write(priv, IMG_IR_CONTROL, priv->hw.reg_timings.ctrl); } To clarify, I was only referring to the case where the irq mask has changed unexpectedly. If it hasn't changed then it would seem to make sense to clear pending interrupts (i.e. the ones we've been intentionally ignoring) before re-enabling them. When you say it works without, do you mean there never are pending interrupts (if you don't press any other buttons on the remote)? Cheers James signature.asc Description: OpenPGP digital signature
Re: [PATCH 5/5] rc: img-ir: add philips rc6 decoder module
On 04/12/14 15:38, Sifan Naeem wrote: > Add img-ir module for decoding Philips rc6 protocol. > > Signed-off-by: Sifan Naeem Aside from the "Philips" thing: Acked-by: James Hogan (It's unpleasant having unexplained timings for RC-6, but it's better than no RC-6 support, and hopefully in the future it can be improved). Cheers James > --- > drivers/media/rc/img-ir/Kconfig |8 +++ > drivers/media/rc/img-ir/Makefile |1 + > drivers/media/rc/img-ir/img-ir-hw.c |3 + > drivers/media/rc/img-ir/img-ir-hw.h |1 + > drivers/media/rc/img-ir/img-ir-rc6.c | 117 > ++ > 5 files changed, 130 insertions(+) > create mode 100644 drivers/media/rc/img-ir/img-ir-rc6.c > > diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig > index b5b114f..4d3fca9 100644 > --- a/drivers/media/rc/img-ir/Kconfig > +++ b/drivers/media/rc/img-ir/Kconfig > @@ -66,3 +66,11 @@ config IR_IMG_RC5 > help > Say Y here to enable support for the RC5 protocol in the ImgTec > infrared decoder block. > + > +config IR_IMG_RC6 > + bool "Phillips RC6 protocol support" > + depends on IR_IMG_HW > + help > +Say Y here to enable support for the RC6 protocol in the ImgTec > +infrared decoder block. > +Note: This version only supports mode 0. > diff --git a/drivers/media/rc/img-ir/Makefile > b/drivers/media/rc/img-ir/Makefile > index 898b1b8..8e6d458 100644 > --- a/drivers/media/rc/img-ir/Makefile > +++ b/drivers/media/rc/img-ir/Makefile > @@ -7,6 +7,7 @@ img-ir-$(CONFIG_IR_IMG_SONY) += img-ir-sony.o > img-ir-$(CONFIG_IR_IMG_SHARP)+= img-ir-sharp.o > img-ir-$(CONFIG_IR_IMG_SANYO)+= img-ir-sanyo.o > img-ir-$(CONFIG_IR_IMG_RC5) += img-ir-rc5.o > +img-ir-$(CONFIG_IR_IMG_RC6) += img-ir-rc6.o > img-ir-objs := $(img-ir-y) > > obj-$(CONFIG_IR_IMG) += img-ir.o > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c > index 322cdf8..3b70dc2 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -45,6 +45,9 @@ static struct img_ir_decoder *img_ir_decoders[] = { > #ifdef CONFIG_IR_IMG_RC5 > &img_ir_rc5, > #endif > +#ifdef CONFIG_IR_IMG_RC6 > + &img_ir_rc6, > +#endif > NULL > }; > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.h > b/drivers/media/rc/img-ir/img-ir-hw.h > index f124ec5..c7b6e1a 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.h > +++ b/drivers/media/rc/img-ir/img-ir-hw.h > @@ -188,6 +188,7 @@ extern struct img_ir_decoder img_ir_sony; > extern struct img_ir_decoder img_ir_sharp; > extern struct img_ir_decoder img_ir_sanyo; > extern struct img_ir_decoder img_ir_rc5; > +extern struct img_ir_decoder img_ir_rc6; > > /** > * struct img_ir_reg_timings - Reg values for decoder timings at clock rate. > diff --git a/drivers/media/rc/img-ir/img-ir-rc6.c > b/drivers/media/rc/img-ir/img-ir-rc6.c > new file mode 100644 > index 000..bcd0822 > --- /dev/null > +++ b/drivers/media/rc/img-ir/img-ir-rc6.c > @@ -0,0 +1,117 @@ > +/* > + * ImgTec IR Decoder setup for Phillips RC-6 protocol. > + * > + * Copyright 2012-2014 Imagination Technologies Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include "img-ir-hw.h" > + > +/* Convert RC6 data to a scancode */ > +static int img_ir_rc6_scancode(int len, u64 raw, u64 enabled_protocols, > + struct img_ir_scancode_req *request) > +{ > + unsigned int addr, cmd, mode, trl1, trl2; > + > + /* > + * Due to a side effect of the decoder handling the double length > + * Trailer bit, the header information is a bit scrambled, and the > + * raw data is shifted incorrectly. > + * This workaround effectively recovers the header bits. > + * > + * The Header field should look like this: > + * > + * StartBit ModeBit2 ModeBit1 ModeBit0 TrailerBit > + * > + * But what we get is: > + * > + * ModeBit2 ModeBit1 ModeBit0 TrailerBit1 TrailerBit2 > + * > + * The start bit is not important to recover the scancode. > + */ > + > + raw >>= 27; > + > + trl1= (raw >> 17) & 0x01; > + trl2= (raw >> 16) & 0x01; > + > + mode= (raw >> 18) &am
Re: [PATCH 4/5] rc: img-ir: add philips rc5 decoder module
On 04/12/14 15:38, Sifan Naeem wrote: > Add img-ir module for decoding Philips rc5 protocol. > > Signed-off-by: Sifan Naeem > --- > drivers/media/rc/img-ir/Kconfig |7 +++ > drivers/media/rc/img-ir/Makefile |1 + > drivers/media/rc/img-ir/img-ir-hw.c |3 ++ > drivers/media/rc/img-ir/img-ir-hw.h |1 + > drivers/media/rc/img-ir/img-ir-rc5.c | 88 > ++ > 5 files changed, 100 insertions(+) > create mode 100644 drivers/media/rc/img-ir/img-ir-rc5.c > > diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig > index 03ba9fc..b5b114f 100644 > --- a/drivers/media/rc/img-ir/Kconfig > +++ b/drivers/media/rc/img-ir/Kconfig > @@ -59,3 +59,10 @@ config IR_IMG_SANYO > help > Say Y here to enable support for the Sanyo protocol (used by Sanyo, > Aiwa, Chinon remotes) in the ImgTec infrared decoder block. > + > +config IR_IMG_RC5 > + bool "Phillips RC5 protocol support" I think that should be "Philips" (if wikipedia is anything to go by). Same elsewhere in this patch and patch 5. Other than that, Acked-by: James Hogan (Note, I don't have RC-5/RC-6 capable hardware yet so can't test this support) Thanks James > + depends on IR_IMG_HW > + help > +Say Y here to enable support for the RC5 protocol in the ImgTec > +infrared decoder block. > diff --git a/drivers/media/rc/img-ir/Makefile > b/drivers/media/rc/img-ir/Makefile > index 92a459d..898b1b8 100644 > --- a/drivers/media/rc/img-ir/Makefile > +++ b/drivers/media/rc/img-ir/Makefile > @@ -6,6 +6,7 @@ img-ir-$(CONFIG_IR_IMG_JVC) += img-ir-jvc.o > img-ir-$(CONFIG_IR_IMG_SONY) += img-ir-sony.o > img-ir-$(CONFIG_IR_IMG_SHARP)+= img-ir-sharp.o > img-ir-$(CONFIG_IR_IMG_SANYO)+= img-ir-sanyo.o > +img-ir-$(CONFIG_IR_IMG_RC5) += img-ir-rc5.o > img-ir-objs := $(img-ir-y) > > obj-$(CONFIG_IR_IMG) += img-ir.o > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c > index a977467..322cdf8 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -42,6 +42,9 @@ static struct img_ir_decoder *img_ir_decoders[] = { > #ifdef CONFIG_IR_IMG_SANYO > &img_ir_sanyo, > #endif > +#ifdef CONFIG_IR_IMG_RC5 > + &img_ir_rc5, > +#endif > NULL > }; > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.h > b/drivers/media/rc/img-ir/img-ir-hw.h > index 8578aa7..f124ec5 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.h > +++ b/drivers/media/rc/img-ir/img-ir-hw.h > @@ -187,6 +187,7 @@ extern struct img_ir_decoder img_ir_jvc; > extern struct img_ir_decoder img_ir_sony; > extern struct img_ir_decoder img_ir_sharp; > extern struct img_ir_decoder img_ir_sanyo; > +extern struct img_ir_decoder img_ir_rc5; > > /** > * struct img_ir_reg_timings - Reg values for decoder timings at clock rate. > diff --git a/drivers/media/rc/img-ir/img-ir-rc5.c > b/drivers/media/rc/img-ir/img-ir-rc5.c > new file mode 100644 > index 000..e1a0829 > --- /dev/null > +++ b/drivers/media/rc/img-ir/img-ir-rc5.c > @@ -0,0 +1,88 @@ > +/* > + * ImgTec IR Decoder setup for Phillips RC-5 protocol. > + * > + * Copyright 2012-2014 Imagination Technologies Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include "img-ir-hw.h" > + > +/* Convert RC5 data to a scancode */ > +static int img_ir_rc5_scancode(int len, u64 raw, u64 enabled_protocols, > + struct img_ir_scancode_req *request) > +{ > + unsigned int addr, cmd, tgl, start; > + > + /* Quirk in the decoder shifts everything by 2 to the left. */ > + raw >>= 2; > + > + start = (raw >> 13) & 0x01; > + tgl = (raw >> 11) & 0x01; > + addr= (raw >> 6) & 0x1f; > + cmd = raw & 0x3f; > + /* > + * 12th bit is used to extend the command in extended RC5 and has > + * no effect on standard RC5. > + */ > + cmd += ((raw >> 12) & 0x01) ? 0 : 0x40; > + > + if (!start) > + return -EINVAL; > + > + request->protocol = RC_TYPE_RC5; > + request->scancode = addr << 8 | cmd; > + request->toggle = tgl; > + return IMG_IR_SCANCODE; > +} > + > +/* Convert RC5 scanc