| Message ID | 57644915.3010006@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Steve, Jack, All, On Fri, Jun 17, 2016 at 12:01:41PM -0700, Steve Longerbeam wrote: > > On 06/17/2016 08:18 AM, Gary Bisson wrote: > > Steve, All, > > > > On Thu, Jun 16, 2016 at 10:32:31AM +0200, Gary Bisson wrote: > > > Steve, All, > > > > > > On Tue, Jun 14, 2016 at 03:49:15PM -0700, Steve Longerbeam wrote: > > > > Defines the host video capture device node and an OV5642 camera sensor > > > > node on i2c2. The host capture device connects to the OV5642 via the > > > > parallel-bus mux input on the ipu1_csi0_mux. > > > > > > > > Note there is a pin conflict with GPIO6. This pin functions as a power > > > > input pin to the OV5642, but ENET requires it to wake-up the ARM cores > > > > on normal RX and TX packet done events (see 6261c4c8). So by default, > > > > capture is disabled, enable by uncommenting __OV5642_CAPTURE__ macro. > > > > Ethernet will still work just not quite as well. > > > Actually the following patch fixes this issue and has already been > > > applied on Shawn's tree: > > > https://patchwork.kernel.org/patch/9153523/ > > > > > > Also, this follow-up patch declared the HW workaround for SabreLite: > > > https://patchwork.kernel.org/patch/9153525/ > > > > > > So ideally, once those two patches land on your base tree, you could get > > > rid of the #define and remove the HW workaround declaration. > > > > > > Finally, I'll test the series on Sabre-Lite this week. > > I've applied this series on top of Shawn tree (for-next branch) in order > > not to worry about the GPIO6 workaround. > > > > Although the camera seems to get enumerated properly, I can't seem to > > get anything from it. See log: > > http://pastebin.com/xnw1ujUq > > Hi Gary, the driver does not implement vidioc_cropcap, it has > switched to the new selection APIs and v4l2src should be using > vidioc_g_selection instead of vidioc_cropcap. I confirm this was the issue, your patch fixes it. > > In your cover letter, you said that you have not run through > > v4l2-compliance. How have you tested the capture? > > I use v4l2-ctl, and have used v4l2src in the past, but that was before > switching to the selection APIs. Try the attached hack that adds > vidioc_cropcap back in, and see how far you get on SabreLite with > v4l2src. I tried the following on SabreAuto: > > gst-launch-1.0 v4l2src io_mode=4 ! > "video/x-raw,format=RGB16,width=640,height=480" ! fbdevsink I confirm that works with OV5642 on SabreLite: Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> > > Also, why isn't the OV5640 MIPI camera declared on the SabreLite device > > tree? > > See Jack Mitchell's patch at http://ix.io/TTg. Thanks Jack! I will work on > incorporating it. I've tried that patch have a some comments: - When applied, no capture shows up any more, instead I have two m2m v4l2 devices [1]. - OV5640 Mipi is assigned the same address as OV5642, therefore both can't work at the same time right now. There's a register in the camera that allows to modify its I2C address, see this patch [2]. - How is the mclk working in this patch? It should be using the PWM3 output to generate a ~22MHz clock. I would expect the use of a pwm-clock node [3]. Also another remark on both OV5642 and OV5640 patches, is it recommended to use 0x80000000 pin muxing value? This leaves it to the bootloader to properly setup the I/O. It sounds safer to properly set them up in the device tree in my opinion in order not to be dependent on the bootloader. All the above remarks said, thanks for this work on i.MX camera support, that feature will be highly appreciated. Regards, Gary [1] http://pastebin.com/i5MrhB1h [2] https://github.com/boundarydevices/linux-imx6/commit/f915806d [3] http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/clock/pwm-clock.txt -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/06/16 10:33, Gary Bisson wrote: > Steve, Jack, All, > > On Fri, Jun 17, 2016 at 12:01:41PM -0700, Steve Longerbeam wrote: >> >> On 06/17/2016 08:18 AM, Gary Bisson wrote: >>> Steve, All, >>> >>> On Thu, Jun 16, 2016 at 10:32:31AM +0200, Gary Bisson wrote: >>>> Steve, All, >>>> >>>> On Tue, Jun 14, 2016 at 03:49:15PM -0700, Steve Longerbeam wrote: >>>>> Defines the host video capture device node and an OV5642 camera sensor >>>>> node on i2c2. The host capture device connects to the OV5642 via the >>>>> parallel-bus mux input on the ipu1_csi0_mux. >>>>> >>>>> Note there is a pin conflict with GPIO6. This pin functions as a power >>>>> input pin to the OV5642, but ENET requires it to wake-up the ARM cores >>>>> on normal RX and TX packet done events (see 6261c4c8). So by default, >>>>> capture is disabled, enable by uncommenting __OV5642_CAPTURE__ macro. >>>>> Ethernet will still work just not quite as well. >>>> Actually the following patch fixes this issue and has already been >>>> applied on Shawn's tree: >>>> https://patchwork.kernel.org/patch/9153523/ >>>> >>>> Also, this follow-up patch declared the HW workaround for SabreLite: >>>> https://patchwork.kernel.org/patch/9153525/ >>>> >>>> So ideally, once those two patches land on your base tree, you could get >>>> rid of the #define and remove the HW workaround declaration. >>>> >>>> Finally, I'll test the series on Sabre-Lite this week. >>> I've applied this series on top of Shawn tree (for-next branch) in order >>> not to worry about the GPIO6 workaround. >>> >>> Although the camera seems to get enumerated properly, I can't seem to >>> get anything from it. See log: >>> http://pastebin.com/xnw1ujUq >> >> Hi Gary, the driver does not implement vidioc_cropcap, it has >> switched to the new selection APIs and v4l2src should be using >> vidioc_g_selection instead of vidioc_cropcap. > > I confirm this was the issue, your patch fixes it. > >>> In your cover letter, you said that you have not run through >>> v4l2-compliance. How have you tested the capture? >> >> I use v4l2-ctl, and have used v4l2src in the past, but that was before >> switching to the selection APIs. Try the attached hack that adds >> vidioc_cropcap back in, and see how far you get on SabreLite with >> v4l2src. I tried the following on SabreAuto: >> >> gst-launch-1.0 v4l2src io_mode=4 ! >> "video/x-raw,format=RGB16,width=640,height=480" ! fbdevsink > > I confirm that works with OV5642 on SabreLite: > Tested-by: Gary Bisson <gary.bisson@boundarydevices.com> > >>> Also, why isn't the OV5640 MIPI camera declared on the SabreLite device >>> tree? >> >> See Jack Mitchell's patch at http://ix.io/TTg. Thanks Jack! I will work on >> incorporating it. Hi Gary, > > I've tried that patch have a some comments: > - When applied, no capture shows up any more, instead I have two m2m > v4l2 devices [1]. > - OV5640 Mipi is assigned the same address as OV5642, therefore both Yes, I only have one device attached in my scenario. > can't work at the same time right now. There's a register in the > camera that allows to modify its I2C address, see this patch [2]. > - How is the mclk working in this patch? It should be using the PWM3 As mentioned I have an eCon sensor board [1] which generates it's own clock on the board and as such I don't need the PWM signal, just the two GPIOs. > output to generate a ~22MHz clock. I would expect the use of a > pwm-clock node [3]. > > Also another remark on both OV5642 and OV5640 patches, is it recommended > to use 0x80000000 pin muxing value? This leaves it to the bootloader to I also wondered about this, but didn't know if the pinmux driver did this based on the define name? I tried it both ways and it worked so I just left it as it was. Cheers, Jack. [1] https://www.e-consystems.com/iMX6-MIPI-Camera-Board.asp > properly setup the I/O. It sounds safer to properly set them up in the > device tree in my opinion in order not to be dependent on the bootloader. > > All the above remarks said, thanks for this work on i.MX camera support, > that feature will be highly appreciated. > > Regards, > Gary > > [1] http://pastebin.com/i5MrhB1h > [2] https://github.com/boundarydevices/linux-imx6/commit/f915806d > [3] http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/clock/pwm-clock.txt > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jack, All, On Mon, Jun 20, 2016 at 10:44:44AM +0100, Jack Mitchell wrote: > <snip> > > I've tried that patch have a some comments: > > - When applied, no capture shows up any more, instead I have two m2m > > v4l2 devices [1]. > > - OV5640 Mipi is assigned the same address as OV5642, therefore both > > Yes, I only have one device attached in my scenario. Thanks for confirming. > > can't work at the same time right now. There's a register in the > > camera that allows to modify its I2C address, see this patch [2]. > > - How is the mclk working in this patch? It should be using the PWM3 > > As mentioned I have an eCon sensor board [1] which generates it's own clock > on the board and as such I don't need the PWM signal, just the two GPIOs. Oh ok, thanks I didn't this sensor board was different than ours [1]. But in your patch, you specifically disable pwm3, what's the reason for it? > > output to generate a ~22MHz clock. I would expect the use of a > > pwm-clock node [3]. > > > > Also another remark on both OV5642 and OV5640 patches, is it recommended > > to use 0x80000000 pin muxing value? This leaves it to the bootloader to > > I also wondered about this, but didn't know if the pinmux driver did this > based on the define name? I tried it both ways and it worked so I just left > it as it was. Actually my phrasing is wrong, the muxing is ok. Yes depending on the name a pin will be muxed to one function or another. The problem is the pad configuration (pull-up, pull-down etc..). I am not surprised that it works, because the bootloader should properly set those. But it would be safer IMO not to rely on it. Regards, Gary [1] https://boundarydevices.com/product/nit6x_5mp_mipi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/06/16 11:16, Gary Bisson wrote: > Jack, All, > > On Mon, Jun 20, 2016 at 10:44:44AM +0100, Jack Mitchell wrote: >> <snip> >>> I've tried that patch have a some comments: >>> - When applied, no capture shows up any more, instead I have two m2m >>> v4l2 devices [1]. >>> - OV5640 Mipi is assigned the same address as OV5642, therefore both >> >> Yes, I only have one device attached in my scenario. > > Thanks for confirming. > >>> can't work at the same time right now. There's a register in the >>> camera that allows to modify its I2C address, see this patch [2]. >>> - How is the mclk working in this patch? It should be using the PWM3 >> >> As mentioned I have an eCon sensor board [1] which generates it's own clock >> on the board and as such I don't need the PWM signal, just the two GPIOs. > > Oh ok, thanks I didn't this sensor board was different than ours [1]. > > But in your patch, you specifically disable pwm3, what's the reason for > it? Yes, it uses the GPIO on the PWM3 pin (beats me why...) so I had to specifically disable it to stop the pin muxing clash. > >>> output to generate a ~22MHz clock. I would expect the use of a >>> pwm-clock node [3]. >>> >>> Also another remark on both OV5642 and OV5640 patches, is it recommended >>> to use 0x80000000 pin muxing value? This leaves it to the bootloader to >> >> I also wondered about this, but didn't know if the pinmux driver did this >> based on the define name? I tried it both ways and it worked so I just left >> it as it was. > > Actually my phrasing is wrong, the muxing is ok. Yes depending on the > name a pin will be muxed to one function or another. The problem is the > pad configuration (pull-up, pull-down etc..). I am not surprised that it > works, because the bootloader should properly set those. But it would be > safer IMO not to rely on it. Ah ok, makes sense. > > Regards, > Gary > > [1] https://boundarydevices.com/product/nit6x_5mp_mipi/ > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/20/2016 03:44 AM, Jack Mitchell wrote: > > > On 20/06/16 11:16, Gary Bisson wrote: >> Jack, All, >> >> On Mon, Jun 20, 2016 at 10:44:44AM +0100, Jack Mitchell wrote: >>> <snip> >>>> I've tried that patch have a some comments: >>>> - When applied, no capture shows up any more, instead I have two m2m >>>> v4l2 devices [1]. >>>> - OV5640 Mipi is assigned the same address as OV5642, therefore both >>> >>> Yes, I only have one device attached in my scenario. >> >> Thanks for confirming. >> >>>> can't work at the same time right now. There's a register in the >>>> camera that allows to modify its I2C address, see this patch [2]. >>>> - How is the mclk working in this patch? It should be using the PWM3 >>> >>> As mentioned I have an eCon sensor board [1] which generates it's own clock >>> on the board and as such I don't need the PWM signal, just the two GPIOs. >> >> Oh ok, thanks I didn't this sensor board was different than ours [1]. >> >> But in your patch, you specifically disable pwm3, what's the reason for >> it? > > Yes, it uses the GPIO on the PWM3 pin (beats me why...) so I had to specifically disable it to stop the pin muxing clash. > Hi Jack, in your patch, the PWM3 pin (which is SD1_DAT1 operating as GPIO1_IO17) is being used as the power-down pin for your OV5640 board. Anyway, I didn't realize this was a different camera from the boundary-devices module (https://boundarydevices.com/product/nit6x_5mp_mipi/). I would like to add support in the DT for the BD module, but I am unable to test/debug as I don't have one. I'm wondering if someone could lend me one, along with the schematics. I have the OV5642 parallel interface module for the SabreLite, so I'd love to get my hands on the OV5640 mipi module as that would allow testing of multiple camera capture which I've never been able to do before. In the meantime I am going to omit support for this module in the sabrelite DT (there's also the problem of the i2c bus address conflict on the same i2c2 bus with the ov5642). Or if someone can add support for the BD module later that would be great. Steve >> >>>> output to generate a ~22MHz clock. I would expect the use of a >>>> pwm-clock node [3]. >>>> >>>> Also another remark on both OV5642 and OV5640 patches, is it recommended >>>> to use 0x80000000 pin muxing value? This leaves it to the bootloader to >>> >>> I also wondered about this, but didn't know if the pinmux driver did this >>> based on the define name? I tried it both ways and it worked so I just left >>> it as it was. >> >> Actually my phrasing is wrong, the muxing is ok. Yes depending on the >> name a pin will be muxed to one function or another. The problem is the >> pad configuration (pull-up, pull-down etc..). I am not surprised that it >> works, because the bootloader should properly set those. But it would be >> safer IMO not to rely on it. > > Ah ok, makes sense. > >> >> Regards, >> Gary >> >> [1] https://boundarydevices.com/product/nit6x_5mp_mipi/ >> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/staging/media/imx/capture/imx-camif.c b/drivers/staging/media/imx/capture/imx-camif.c index 9c247e0..2c51bc7 100644 --- a/drivers/staging/media/imx/capture/imx-camif.c +++ b/drivers/staging/media/imx/capture/imx-camif.c @@ -1561,6 +1561,23 @@ static int vidioc_s_parm(struct file *file, void *fh, return v4l2_subdev_call(dev->sensor->sd, video, s_parm, a); } +static int vidioc_cropcap(struct file *file, void *priv, + struct v4l2_cropcap *cropcap) +{ + struct imxcam_ctx *ctx = file2ctx(file); + struct imxcam_dev *dev = ctx->dev; + + if (cropcap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE && + cropcap->type != V4L2_BUF_TYPE_VIDEO_OVERLAY) + return -EINVAL; + + cropcap->bounds = dev->crop_bounds; + cropcap->defrect = dev->crop_defrect; + cropcap->pixelaspect.numerator = 1; + cropcap->pixelaspect.denominator = 1; + return 0; +} + static int vidioc_g_selection(struct file *file, void *priv, struct v4l2_selection *sel) { @@ -1794,6 +1811,7 @@ static const struct v4l2_ioctl_ops imxcam_ioctl_ops = { .vidioc_g_parm = vidioc_g_parm, .vidioc_s_parm = vidioc_s_parm, + .vidioc_cropcap = vidioc_cropcap, .vidioc_g_selection = vidioc_g_selection, .vidioc_s_selection = vidioc_s_selection,