Re: [PATCH 14/15] ARM: pxa: change SSP devices allocation
On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik wrote: > > +static struct pxa_ssp_info pxa_ssp_infos[] = { > + { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", }, > + { .dma_chan_rx_name = "ssp1_rx", .dma_chan_tx_name = "ssp1_tx", }, > + { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", }, > + { .dma_chan_rx_name = "ssp2_rx", .dma_chan_tx_name = "ssp2_tx", }, > + { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", }, > + { .dma_chan_rx_name = "ssp3_rx", .dma_chan_tx_name = "ssp3_tx", }, > + { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", }, > + { .dma_chan_rx_name = "ssp4_rx", .dma_chan_tx_name = "ssp4_tx", }, > +}; This part looks odd to me, you're adding an extra level of indirection to do two stages of lookups in some form of platform data. Why can't you just always use "rx" and "tx" as the names here? (also, I don't see why each line is duplicated, but I'm sure there's an easy answer for that). Arnd
Re: [PATCH 12/15] dmaengine: pxa: make the filter function internal
On Mon, Apr 2, 2018 at 6:35 PM, kbuild test robot wrote: > >drivers/mtd/nand/marvell_nand.c:2621:17: sparse: undefined identifier > 'pxad_filter_fn' >>> drivers/mtd/nand/marvell_nand.c:2621:17: sparse: call with no type! >In file included from drivers/mtd/nand/marvell_nand.c:21:0: >drivers/mtd/nand/marvell_nand.c: In function 'marvell_nfc_init_dma': >drivers/mtd/nand/marvell_nand.c:2621:42: error: 'pxad_filter_fn' > undeclared (first use in this function); did you mean 'dma_filter_fn'? > dma_request_slave_channel_compat(mask, pxad_filter_fn, > ^ >include/linux/dmaengine.h:1408:46: note: in definition of macro > 'dma_request_slave_channel_compat' > __dma_request_slave_channel_compat(&(mask), x, y, dev, name) > ^ >drivers/mtd/nand/marvell_nand.c:2621:42: note: each undeclared identifier > is reported only once for each function it appears in > dma_request_slave_channel_compat(mask, pxad_filter_fn, > ^ >include/linux/dmaengine.h:1408:46: note: in definition of macro > 'dma_request_slave_channel_compat' > __dma_request_slave_channel_compat(&(mask), x, y, dev, name) The driver is a replacement for the pxa3xx nand driver, so it now has to get changed as well. Arnd
Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps
On Mon, Apr 2, 2018 at 4:26 PM, Robert Jarzmik wrote: > Hi, > > This serie is aimed at removing the dmaengine slave compat use, and transfer > knowledge of the DMA requestors into architecture code. > > This was discussed/advised by Arnd a couple of years back, it's almost time. > > The serie is divided in 3 phasees : > - phase 1 : patch 1/15 and patch 2/15 >=> this is the preparation work > - phase 2 : patches 3/15 .. 10/15 >=> this is the switch of all the drivers >=> this one will require either an Ack of the maintainers or be taken by > them > once phase 1 is merged > - phase 3 : patches 11/15 >=> this is the last part, cleanup and removal of export of the DMA filter > function > > As this looks like a patch bomb, each maintainer expressing for his tree > either > an Ack or "I want to take through my tree" will be spared in the next > iterations > of this serie. > > Several of these changes have been tested on actual hardware, including : > - pxamci > - pxa_camera > - smc* > - ASoC and SSP > > Happy review. This looks really great overall, thanks for cleaning this up! The SSP part is still a bit weird, as I commented, but I'm sure we can figure something out there. Arnd
Re: V4l2 Sensor driver and V4l2 ctrls
On 03/04/18 08:32, asadpt iqroot wrote: > Hi Hans, > > Thanks for the reply. > > In board, we have the HDMI connectors. Is it mandatory to use this ctrl > V4L2_CID_DV_RX_POWER_PRESENT. Based on this v4l2 ctrl, what v4l2 framework > will do? If I do not set any value to this ctrl, what will happen to > video streaming? This control isn't for the kernel, it's to help userspace. Userspace can subscribe to this control and receive an event whenever it changes value (i.e. whenever the driver detects a change in the 5V HDMI line). It then knows that it looks like something has been connected to the receiver and it can start waiting for a stable signal, inform the user or just keep track of it internally for debug purposes. What HDMI receivers do you use? Analog Devices? NXP? SoC-based? > > For example, if I did not add this control in driver, what will > happen? Whether this will > affect the functionality of the video streaming? or not. No, it is just to inform userspace that something was connected or disconnected. And to aid in debugging problems. > Why do we have the standard v4l2 ctrl like this? See above. Note: if you plan on upstreaming your HDMI receiver driver, then this control must be present. Regards, Hans > > Thanks for the inputs. > > On 30 March 2018 at 12:58, Hans Verkuil wrote: >> On 30/03/18 09:23, asadpt iqroot wrote: >>> Hi Hans, >>> >>> Thanks for the reply. >>> >>> In HDMI receivers, when we need to use this control. What scenario? >> >> https://www.linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/extended-controls.html#digital-video-control-reference >> >> "Detects whether the receiver receives power from the source (e.g. HDMI >> carries 5V on one of the pins)." >> >> Regards, >> >> Hans >> >>> >>> -Thanks. >>> >>> >>> On 30 March 2018 at 12:13, Hans Verkuil wrote: On 30/03/18 08:16, asadpt iqroot wrote: > Hi All, > > In reference sensor drivers, they used the > V4L2_CID_DV_RX_POWER_PRESENT v4l2 ctrl. > It is a standard ctrl and created using v4l2_ctrl_new_std(). > > The doubts are: > > 1. Whether in our sensor driver, we need to create this Control Id or > not. How to take the decision on this. Since this is the standard > ctrl. When we need to use these standard ctrls?? No. This control is for HDMI receivers, not for sensors. Regards, Hans > > 2. In Sensor driver, the ctrls creation is anything depends on the > bridge driver. > Based on bridge driver, whether we need to create any ctrls in Sensor > driver. > > This question belongs to design of the sensor driver. > > > > Thanks & Regards > >>
Re: [PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property
On 27/03/18 00:25, Rob Herring wrote: > On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote: >> From: Hans Verkuil >> >> Some boards have both a DesignWare and their own CEC controller. >> The CEC pin is only hooked up to their own CEC controller and not >> to the DW controller. >> >> Add the cec-disable property to disable the DW CEC controller. >> >> This particular situation happens on Amlogic boards that have their >> own meson CEC controller. > > Seems like we could avoid this by describing how the CEC line is hooked > up which could be needed for other reasons. So there are three situations: 1) The cec pin is connected to the DW HDMI TX. That's already supported. 2) The cec pin is not connected at all, but the CEC IP is instantiated. We need the cec-disable property for that. This simply states that the CEC pin is not connected. 3) The cec pin is connected to an HDMI RX. We do not support this at the moment. If we want to support this, then we need a 'hdmi-rx' phandle that points to the HDMI receiver that the CEC pin is associated with. This will be similar to the already existing 'hdmi-phandle' property used to associate a CEC driver with an HDMI transmitter. In hindsight it would have been better if 'hdmi-phandle' was named 'hdmi-tx' :-( I can make a binding proposal for 3, but I have no hardware to test it, so I think it is better to add this only when someone has hardware. It will require quite a few changes to the driver and likely also the CEC core. Regards, Hans > >> >> Signed-off-by: Hans Verkuil >> Acked-by: Neil Armstrong >> --- >> Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++ >> 1 file changed, 3 insertions(+)
Re: [PATCH 4/8] dma-buf: add peer2peer flag
On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote: > Am 29.03.2018 um 08:57 schrieb Daniel Vetter: > > On Sun, Mar 25, 2018 at 12:59:56PM +0200, Christian König wrote: > > > Add a peer2peer flag noting that the importer can deal with device > > > resources which are not backed by pages. > > > > > > Signed-off-by: Christian König > > Um strictly speaking they all should, but ttm never bothered to use the > > real interfaces but just hacked around the provided sg list, grabbing the > > underlying struct pages, then rebuilding&remapping the sg list again. > > Actually that isn't correct. TTM converts them to a dma address array > because drivers need it like this (at least nouveau, radeon and amdgpu). > > I've fixed radeon and amdgpu to be able to deal without it and mailed with > Ben about nouveau, but the outcome is they don't really know. > > TTM itself doesn't have any need for the pages on imported BOs (you can't > mmap them anyway), the real underlying problem is that sg tables doesn't > provide what drivers need. > > I think we could rather easily fix sg tables, but that is a totally separate > task. Looking at patch 8, the sg table seems perfectly sufficient to convey the right dma addresses to the importer. Ofcourse the exporter has to set up the right kind of iommu mappings to make this work. > > The entire point of using sg lists was exactly to allow this use case of > > peer2peer dma (or well in general have special exporters which managed > > memory/IO ranges not backed by struct page). So essentially you're having > > a "I'm totally not broken flag" here. > > No, independent of needed struct page pointers we need to note if the > exporter can handle peer2peer stuff from the hardware side in general. > > So what I've did is just to set peer2peer allowed on the importer because of > the driver needs and clear it in the exporter if the hardware can't handle > that. The only thing the importer seems to do is call the pci_peer_traffic_supported, which the exporter could call too. What am I missing (since the sturct_page stuff sounds like it's fixed already by you)? -Daniel > > I think a better approach would be if we add a requires_struct_page or so, > > and annotate the current importers accordingly. Or we just fix them up (it > > is all in shared ttm code after all, I think everyone else got this > > right). > > I would rather not bed on that. > > Christian. > > > -Daniel > > > > > --- > > > drivers/dma-buf/dma-buf.c | 1 + > > > include/linux/dma-buf.h | 4 > > > 2 files changed, 5 insertions(+) > > > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > > > index ffaa2f9a9c2c..f420225f93c6 100644 > > > --- a/drivers/dma-buf/dma-buf.c > > > +++ b/drivers/dma-buf/dma-buf.c > > > @@ -565,6 +565,7 @@ struct dma_buf_attachment *dma_buf_attach(const > > > struct dma_buf_attach_info *info > > > attach->dev = info->dev; > > > attach->dmabuf = dmabuf; > > > + attach->peer2peer = info->peer2peer; > > > attach->priv = info->priv; > > > attach->invalidate = info->invalidate; > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > > > index 15dd8598bff1..1ef50bd9bc5b 100644 > > > --- a/include/linux/dma-buf.h > > > +++ b/include/linux/dma-buf.h > > > @@ -313,6 +313,7 @@ struct dma_buf { > > >* @dmabuf: buffer for this attachment. > > >* @dev: device attached to the buffer. > > >* @node: list of dma_buf_attachment. > > > + * @peer2peer: true if the importer can handle peer resources without > > > pages. > > >* @priv: exporter specific attachment data. > > >* > > >* This structure holds the attachment information between the dma_buf > > > buffer > > > @@ -328,6 +329,7 @@ struct dma_buf_attachment { > > > struct dma_buf *dmabuf; > > > struct device *dev; > > > struct list_head node; > > > + bool peer2peer; > > > void *priv; > > > /** > > > @@ -392,6 +394,7 @@ struct dma_buf_export_info { > > >* @dmabuf: the exported dma_buf > > >* @dev:the device which wants to import the attachment > > >* @priv: private data of importer to this attachment > > > + * @peer2peer: true if the importer can handle peer resources without > > > pages > > >* @invalidate: callback to use for invalidating mappings > > >* > > >* This structure holds the information required to attach to a buffer. > > > Used > > > @@ -401,6 +404,7 @@ struct dma_buf_attach_info { > > > struct dma_buf *dmabuf; > > > struct device *dev; > > > void *priv; > > > + bool peer2peer; > > > void (*invalidate)(struct dma_buf_attachment *attach); > > > }; > > > -- > > > 2.14.1 > > > > > > ___ > > > dri-devel mailing list > > > dri-de...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > _
Re: [PATCH] media: coda: do not try to propagate format if capture queue busy
Hi Philipp, On Thu, Mar 29, 2018 at 2:12 AM Philipp Zabel wrote: > The driver helpfully resets the capture queue format and selection > rectangle whenever output format is changed. This only works while > the capture queue is not busy. Is the code in question used only for decoder case? For encoder, CAPTURE queue determines the codec and so things should work the other way around, i.e. setting CAPTURE format should reset OUTPUT format and it should be allowed only if OUTPUT queue is not busy. Best regards, Tomasz
[GIT PULL for v4.17-rc1] media updates
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v4.17-1 For: - New CEC pin injection code for testing purposes; - DVB frontend cxd2099 promoted from staging; - New platform driver for Sony cxd2880 DVB devices; - New sensor drivers: mt9t112, ov2685, ov5695, ov772x, tda1997x, tw9910.c - Removal of unused cx18 and ivtv alsa mixers; - The reneseas-ceu driver doesn't depend on soc_camera anymore and moved from staging; - Removed the mantis_vp3028 driver, unused since 2009; - s5p-mfc: add support for version 10 of the MSP; - Added a decoder for imon protocol; - atomisp: lots of cleanups; - imx074 and mt9t031: don't depend on soc_camera anymore, being promoted from staging; - added helper functions to better support DVB I2C binding; - lots of driver improvements and cleanups. The following changes since commit 661e50bc853209e41a5c14a290ca4decc43cbfd1: Linux 4.16-rc4 (2018-03-04 14:54:11 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v4.17-1 for you to fetch changes up to f8a695c4b43d02c89b8bba9ba6058fd5db1bc71d: media: v4l2-ioctl: rename a temp var that stores _IOC_SIZE(cmd) (2018-03-26 06:58:47 -0400) media updates for v4.17-rc1 A Sun (1): media: mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short-range receiver) Akinobu Mita (3): media: MAINTAINERS: add entry for ov9650 driver media: ov9650: add device tree binding media: ov9650: support device tree probing Alexandre Courbot (3): media: v4l: vidioc-prepare-buf.rst: fix link to VIDIOC_QBUF media: v4l2_fh.h: add missing kconfig.h include media: media-types.rst: fix typo Alexey Khoroshilov (1): media: rc: ir-hix5hd2: fix error handling of clk_prepare_enable() Alona Solntseva (1): media: drivers: staging: media: atomisp: pci: atomisp2: css2400: fix misspellings Andi Kleen (1): media: rc: don't mark IR decoders default y Antonio Cardace (2): media: em28xx: use %*phC to print small buffers media: gspca: dtcs033: use %*ph to print small buffer Antti Palosaari (18): media: af9013: change lock detection slightly media: af9013: dvbv5 signal strength media: af9013: dvbv5 cnr media: af9013: dvbv5 ber and per media: af9013: wrap dvbv3 statistics via dvbv5 media: af9015: fix logging media: af9013: convert inittabs suitable for regmap_update_bits media: af9013: add i2c mux adapter for tuner bus media: af9015: attach demod using i2c binding media: af9013: remove all legacy media attach releated stuff media: af9013: add pid filter support media: af9015: use af9013 demod pid filters media: af9015: refactor firmware download media: af9015: refactor copy firmware to slave demod media: af9015: enhance streaming config media: dvb-usb-v2: add probe/disconnect callbacks media: af9015: convert to regmap api media: af9015: correct some coding style issues Arnd Bergmann (10): media: au0828: fix VIDEO_V4L2 dependency media: i2c: TDA1997x: add CONFIG_SND dependency media: ov5695: mark PM functions as __maybe_unused media: ov2685: mark PM functions as __maybe_unused media: em28xx: split up em28xx_dvb_init to reduce stack size media: s3c-camif: fix out-of-bounds array access media: renesas-ceu: mark PM functions as __maybe_unused media: staging: media: atomisp: remove pointless string copy media: v4l: omap_vout: vrfb: remove an unused variable media: ngene: avoid unused variable warning Arushi Singhal (4): media: staging: media: Remove unnecessary semicolon media: staging: media: Replace "be be" with "be" media: staging: media: Replace "dont" with "don't" media: staging: media: Replace "cant" with "can't" Benjamin Gaignard (1): media: platform: stm32: Adopt SPDX identifier Brad Love (37): media: em28xx: Hauppauge DualHD second tuner functionality media: em28xx: Bulk transfer implementation fix media: em28xx: USB bulk packet size fix media: em28xx: Increase max em28xx boards to max dvb adapters media: em28xx: Add Hauppauge SoloHD/DualHD bulk models media: em28xx: Enable Hauppauge SoloHD rebranded 292e SE media: lgdt3306a: Set fe ops.release to NULL if probed media: lgdt3306a: QAM streaming improvement media: lgdt3306a: Add QAM AUTO support media: lgdt3306a: Fix module count mismatch on usb unplug media: lgdt3306a: Fix a double kfree on i2c device remove media: cx23885: Enable new Hauppauge PCIe ImpactVCBe variant media: cx23885: Add support for Hauppauge PCIe HVR1265 K4 media
[PATCH] media: gspca: fix Kconfig help info
Documentation/video4linux/gspca.txt is missing. It has moved to Documentation/media/v4l-drivers/gspca-cardlist.rst Signed-off-by: winton.liu <18502523...@163.com> --- drivers/media/usb/gspca/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/gspca/Kconfig b/drivers/media/usb/gspca/Kconfig index d214a21..bc9a439 100644 --- a/drivers/media/usb/gspca/Kconfig +++ b/drivers/media/usb/gspca/Kconfig @@ -7,7 +7,7 @@ menuconfig USB_GSPCA Say Y here if you want to enable selecting webcams based on the GSPCA framework. - See for more info. + See for more info. This driver uses the Video For Linux API. You must say Y or M to "Video For Linux" to use this driver. -- 2.7.4
Re: [PATCH v9 2/2] media: V3s: Add support for Allwinner CSI.
On 06/03/18 03:16, Yong Deng wrote: > Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > interface and CSI1 is used for parallel interface. This is not > documented in datasheet but by test and guess. > > This patch implement a v4l2 framework driver for it. > > Currently, the driver only support the parallel interface. MIPI-CSI2, > ISP's support are not included in this patch. > > Reviewed-by: Maxime Ripard > Tested-by: Maxime Ripard > Signed-off-by: Yong Deng Reviewed-by: Hans Verkuil Thanks! Hans
I am waiting to hear from you soon,
From The international Investigation Financial Crime Unit Attention Dear!! We need your urgent information . I am writing to inform you that I am Staff In the international investigation financial crime unit and I have discovered through our network E-mail system This Week that your Inheritance Claim Fund $6.5m usd have been trapped by Some Bank Officers and who specialized in collecting money from You. There fore I would appreciate to inform you that there is hope for you to recover all what you have lost . Your Inheritance Fund is $6.5m usd Have Been Removed From That Bank And Deposited To Security Financier Company This Morning In Burkina Faso. You should send to us your complete information to enable us Submit Them on your behalf To The Security And Finance Company To Enable The Finance Company Proceed Transferring Your Inheritance Fund $6.5m usd to your country! This will be completed within the next few days . 1.Your complete name 2.Your Mobile phone number 3. Your nationality I am waiting to hear from you soon, Thank you . Mrs Raymond Mabel
Re: [PATCH v13 00/33] rcar-vin: Add Gen3 with media controller
On 26/03/18 23:44, Niklas Söderlund wrote: > Hi, > > This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795, > r8a7796 and r8a77970. It is based on the media-tree and depends on > Fabrizio Castro patches as they touches the order of the compatible > strings in the documentation to reduce merge conflicts. The dependencies > are included in this series. Laurent, Kieran, Unless there are any objections I want to make a pull request for this series once 4.17-rc1 has been merged back into our master tree. It all looks good to me, and it will be nice to get this in (finally!). I don't want to postpone the pull request for small improvements, they can be applied later. But if there are more serious concerns, then let us know. Regards, Hans > > The driver is tested on Renesas H3 (r8a7795, ES2.0), > M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and > not yet upstream) and the Salvator-X onboard ADV7482. It is also tested > on the V3M (r8a77970) on the Eagle board together with its expansion > board with a ADV7482 and out of tree patches for GMSL capture using the > max9286 and rdacm20 drivers. > > It is possible to capture both CVBS and HDMI video streams, > v4l2-compliance passes with no errors and media-ctl can be used to > change the routing and formats for the different entities in the media > graph. > > Gen2 compatibility is verified on Koelsch and no problems where found, > video can be captured just like before and v4l2-compliance passes > without errors or warnings just like before this series. > > For convenience the series can be fetched from: > > git://git.ragnatech.se/linux rcar/vin/mc-v13 > > I have started on a very basic test suite for the VIN driver at: > > https://git.ragnatech.se/vin-tests > > And as before the state of the driver and information about how to test > it can be found on the elinux wiki: > > http://elinux.org/R-Car/Tests:rcar-vin > > * Changes from v12 > - Rebase to latest media-tree/master changed a 'return ret' to a 'goto > out' in rvin_start_streaming() to take recent changes to the VIN > driver into account. > - Moved field != V4L2_FIELD_ANY in 'rcar-vin: set a default field to > fallback on' check from a later commit 'rcar-vin: simplify how formats > are set and reset' in the series. This is to avoid ignoring the field > returned from the sensor if FIELD_ANY was requested by the user. This > was only a problem between this change and a few patches later, but > better to fix it now. Reported by Hans, thanks for spotting this. > - Fix spelling. > - Add review tags from Hans. > > * Changes since v11 > - Rewrote commit message for '[PATCH v11 22/32] rcar-vin: force default > colorspace for media centric mode'. Also set fixed values for > xfer_func, quantization and ycbcr_enc. > - Reorderd filed order in struct rvin_group_route. > - Renamed chan to channel in struct rvin_group_route. > - Rework 'rcar-vin: read subdevice format for crop only when > needed' into 'rcar-vin: simplify how formats are set and reset'. > - Keep caching the source dimensions and drop all changes to > rvin_g_selection() and rvin_s_selection(). > - Inline rvin_get_vin_format_from_source() into rvin_reset_format() > which now is the only user left. > - Add patch to cache the video standard instead of reading it at stream > on. > - Fix error labels in rvin_mc_open(). > - Fixed spelling in commit messages and comment, thanks Laurent! > - Added reviewed tags from Laurent, Thanks! > > * Changes since v10 > - Corrected spelling in comments and commit messages. > - Reworked 'rcar-vin: read subdevice format for crop only when needed' > to only get the source format once per operation. > - Moved some patches around to make it easier to review, moved: > - rcar-vin: set a default field to fallback on > - rcar-vin: fix handling of single field frames (top, bottom and > alternate fields) > - rcar-vin: update bytesperline and sizeimage calculation > - rcar-vin: break out format alignment and checking > - rcar-vin: update pixelformat check for M1 > Before: > - rcar-vin: read subdevice format for crop only when needed > - Rename variable 'code' to 'mbus_code' in struct rvin_dev. > - Add comment describing no locking is needed in > rvin_set_channel_routing(). > - Check return value of pm_runtime_get_sync() in > rvin_set_channel_routing(). > - Rework 'rcar-vin: add check for colorspace' to not try to check the > format, instead force a default format. This should be revisited once > either v4l2-compliance or v4l2 framework changes are worked out to > allow for MC centric drivers to validate user supplied colorspace. > - Add error checking for pm_runtime_get_sync() and > v4l2_pipeline_pm_use(). > - Change mutex_lock() to mutex_lock_interruptible() in rvin_mc_open(). > - Rewrote documentation for struct rvin_group_route. > - Rename rvin_mc_parse_v4l2() to rvin_mc_parse_of_endpoint(). >
Re: [PATCH v13 00/33] rcar-vin: Add Gen3 with media controller
Hi Hans, On 03/04/18 13:30, Hans Verkuil wrote: > On 26/03/18 23:44, Niklas Söderlund wrote: >> Hi, >> >> This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795, >> r8a7796 and r8a77970. It is based on the media-tree and depends on >> Fabrizio Castro patches as they touches the order of the compatible >> strings in the documentation to reduce merge conflicts. The dependencies >> are included in this series. > > Laurent, Kieran, > > Unless there are any objections I want to make a pull request for this > series once 4.17-rc1 has been merged back into our master tree. It all > looks good to me, and it will be nice to get this in (finally!). > > I don't want to postpone the pull request for small improvements, they > can be applied later. But if there are more serious concerns, then let > us know. Certainly sounds good, and no objections from me. Looking forward to getting this series in. Thanks Kieran > Regards, > > Hans > >> >> The driver is tested on Renesas H3 (r8a7795, ES2.0), >> M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and >> not yet upstream) and the Salvator-X onboard ADV7482. It is also tested >> on the V3M (r8a77970) on the Eagle board together with its expansion >> board with a ADV7482 and out of tree patches for GMSL capture using the >> max9286 and rdacm20 drivers. >> >> It is possible to capture both CVBS and HDMI video streams, >> v4l2-compliance passes with no errors and media-ctl can be used to >> change the routing and formats for the different entities in the media >> graph. >> >> Gen2 compatibility is verified on Koelsch and no problems where found, >> video can be captured just like before and v4l2-compliance passes >> without errors or warnings just like before this series. >> >> For convenience the series can be fetched from: >> >> git://git.ragnatech.se/linux rcar/vin/mc-v13 >> >> I have started on a very basic test suite for the VIN driver at: >> >> https://git.ragnatech.se/vin-tests >> >> And as before the state of the driver and information about how to test >> it can be found on the elinux wiki: >> >> http://elinux.org/R-Car/Tests:rcar-vin >> >> * Changes from v12 >> - Rebase to latest media-tree/master changed a 'return ret' to a 'goto >> out' in rvin_start_streaming() to take recent changes to the VIN >> driver into account. >> - Moved field != V4L2_FIELD_ANY in 'rcar-vin: set a default field to >> fallback on' check from a later commit 'rcar-vin: simplify how formats >> are set and reset' in the series. This is to avoid ignoring the field >> returned from the sensor if FIELD_ANY was requested by the user. This >> was only a problem between this change and a few patches later, but >> better to fix it now. Reported by Hans, thanks for spotting this. >> - Fix spelling. >> - Add review tags from Hans. >> >> * Changes since v11 >> - Rewrote commit message for '[PATCH v11 22/32] rcar-vin: force default >> colorspace for media centric mode'. Also set fixed values for >> xfer_func, quantization and ycbcr_enc. >> - Reorderd filed order in struct rvin_group_route. >> - Renamed chan to channel in struct rvin_group_route. >> - Rework 'rcar-vin: read subdevice format for crop only when >> needed' into 'rcar-vin: simplify how formats are set and reset'. >> - Keep caching the source dimensions and drop all changes to >> rvin_g_selection() and rvin_s_selection(). >> - Inline rvin_get_vin_format_from_source() into rvin_reset_format() >> which now is the only user left. >> - Add patch to cache the video standard instead of reading it at stream >> on. >> - Fix error labels in rvin_mc_open(). >> - Fixed spelling in commit messages and comment, thanks Laurent! >> - Added reviewed tags from Laurent, Thanks! >> >> * Changes since v10 >> - Corrected spelling in comments and commit messages. >> - Reworked 'rcar-vin: read subdevice format for crop only when needed' >> to only get the source format once per operation. >> - Moved some patches around to make it easier to review, moved: >> - rcar-vin: set a default field to fallback on >> - rcar-vin: fix handling of single field frames (top, bottom and >> alternate fields) >> - rcar-vin: update bytesperline and sizeimage calculation >> - rcar-vin: break out format alignment and checking >> - rcar-vin: update pixelformat check for M1 >> Before: >> - rcar-vin: read subdevice format for crop only when needed >> - Rename variable 'code' to 'mbus_code' in struct rvin_dev. >> - Add comment describing no locking is needed in >> rvin_set_channel_routing(). >> - Check return value of pm_runtime_get_sync() in >> rvin_set_channel_routing(). >> - Rework 'rcar-vin: add check for colorspace' to not try to check the >> format, instead force a default format. This should be revisited once >> either v4l2-compliance or v4l2 framework changes are worked out to >> allow for MC centric drivers to validate user supplied colorspace. >> - Add
Re: [PATCH v7 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver
Hi Niklas, On Thu, Mar 29, 2018 at 02:35:34PM +0200, Niklas Söderlund wrote: > > + /* > > +* Create a static mapping between the CSI virtual channels > > +* and the input streams. > > +* > > +* This should be enhanced, but v4l2 lacks the support for > > +* changing that mapping dynamically at the moment. > > +* > > +* We're protected from the userspace setting up links at the > > +* same time by the upper layer having called > > +* media_pipeline_start(). > > +*/ > > + list_for_each_entry(link, &entity->links, list) { > > I wonder is this list_for_each_entry() really needed? Can't you simply > iterate over all sink pads as with the loop bellow but drop the pad == > link->sink check? Maybe I'm missing something. This was a review made by Sakari here: https://patchwork.linuxtv.org/patch/44422/ The idea is that we need to know if the pad is enabled, and as far as I know this information is only stored at the link level, not at the pad level. > Apart from this and the small nit-picks (one more bellow) I think this > patch is fine. Once I understand this I be happy to add my tag to this > change, great work! Is this a reviewed by? :) > I also think you shall consider to add a MAINTAINERS record for the RX > and TX drivers. Maybe one entry for both drivers as they live in the > same directory but I think one of the two should add it :-) Right, I'll do it, thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v13 00/33] rcar-vin: Add Gen3 with media controller
Hi Hans, On Tuesday, 3 April 2018 15:30:40 EEST Hans Verkuil wrote: > On 26/03/18 23:44, Niklas Söderlund wrote: > > Hi, > > > > This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795, > > r8a7796 and r8a77970. It is based on the media-tree and depends on > > Fabrizio Castro patches as they touches the order of the compatible > > strings in the documentation to reduce merge conflicts. The dependencies > > are included in this series. > > Laurent, Kieran, > > Unless there are any objections I want to make a pull request for this > series once 4.17-rc1 has been merged back into our master tree. It all > looks good to me, and it will be nice to get this in (finally!). > > I don't want to postpone the pull request for small improvements, they > can be applied later. But if there are more serious concerns, then let > us know. There's just a handful of patchs I haven't acked (but I have reviewed previous version) and I don't think there's any blocker there, small problems can always be fixed later. I'll still try to review them, and if I notice any issue I'll ask Niklas to submit follow-up patches. v13 can be merged upstream from my point of view. Thank you for handling this. > > The driver is tested on Renesas H3 (r8a7795, ES2.0), > > M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and > > not yet upstream) and the Salvator-X onboard ADV7482. It is also tested > > on the V3M (r8a77970) on the Eagle board together with its expansion > > board with a ADV7482 and out of tree patches for GMSL capture using the > > max9286 and rdacm20 drivers. > > > > It is possible to capture both CVBS and HDMI video streams, > > v4l2-compliance passes with no errors and media-ctl can be used to > > change the routing and formats for the different entities in the media > > graph. > > > > Gen2 compatibility is verified on Koelsch and no problems where found, > > video can be captured just like before and v4l2-compliance passes > > without errors or warnings just like before this series. > > > > For convenience the series can be fetched from: > > git://git.ragnatech.se/linux rcar/vin/mc-v13 > > > > I have started on a very basic test suite for the VIN driver at: > > https://git.ragnatech.se/vin-tests > > > > And as before the state of the driver and information about how to test > > > > it can be found on the elinux wiki: > > http://elinux.org/R-Car/Tests:rcar-vin > > > > * Changes from v12 > > - Rebase to latest media-tree/master changed a 'return ret' to a 'goto > > > > out' in rvin_start_streaming() to take recent changes to the VIN > > driver into account. > > > > - Moved field != V4L2_FIELD_ANY in 'rcar-vin: set a default field to > > > > fallback on' check from a later commit 'rcar-vin: simplify how formats > > are set and reset' in the series. This is to avoid ignoring the field > > returned from the sensor if FIELD_ANY was requested by the user. This > > was only a problem between this change and a few patches later, but > > better to fix it now. Reported by Hans, thanks for spotting this. > > > > - Fix spelling. > > - Add review tags from Hans. > > > > * Changes since v11 > > - Rewrote commit message for '[PATCH v11 22/32] rcar-vin: force default > > > > colorspace for media centric mode'. Also set fixed values for > > xfer_func, quantization and ycbcr_enc. > > > > - Reorderd filed order in struct rvin_group_route. > > - Renamed chan to channel in struct rvin_group_route. > > - Rework 'rcar-vin: read subdevice format for crop only when > > > > needed' into 'rcar-vin: simplify how formats are set and reset'. > > > > - Keep caching the source dimensions and drop all changes to > > > > rvin_g_selection() and rvin_s_selection(). > > > > - Inline rvin_get_vin_format_from_source() into rvin_reset_format() > > > > which now is the only user left. > > > > - Add patch to cache the video standard instead of reading it at stream > > > > on. > > > > - Fix error labels in rvin_mc_open(). > > - Fixed spelling in commit messages and comment, thanks Laurent! > > - Added reviewed tags from Laurent, Thanks! > > > > * Changes since v10 > > - Corrected spelling in comments and commit messages. > > - Reworked 'rcar-vin: read subdevice format for crop only when needed' > > > > to only get the source format once per operation. > > > > - Moved some patches around to make it easier to review, moved: > > - rcar-vin: set a default field to fallback on > > - rcar-vin: fix handling of single field frames (top, bottom and > > alternate fields) - rcar-vin: update bytesperline and sizeimage > > calculation > > - rcar-vin: break out format alignment and checking > > - rcar-vin: update pixelformat check for M1 > > > > Before: > > - rcar-vin: read subdevice format for crop only when needed > > > > - Rename variable 'code' to 'mbus_code' in struct rvin_dev. > > - Add comment describing no locking is needed in > > > >
Re: [PATCH v8 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver
Hi Niklas, Thanks for your review, On Thu, Mar 29, 2018 at 03:32:55PM +0200, Niklas Söderlund wrote: > > diff --git a/drivers/media/platform/cadence/Kconfig > > b/drivers/media/platform/cadence/Kconfig > > new file mode 100644 > > index ..18f061e5cbd1 > > --- /dev/null > > +++ b/drivers/media/platform/cadence/Kconfig > > @@ -0,0 +1,17 @@ > > +config VIDEO_CADENCE > > + bool "Cadence Video Devices" > > I'm no expert on Kconfig best practices so if nothing else I might learn > something. There is no need to add a description to this option as it > only groups the Cadence drivers? You don't strictly need it, but you're right and one should be better, I've added it. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH v4 1/2] media: ov2680: dt: Add bindings for OV2680
Add device tree binding documentation for the OV2680 camera sensor. Reviewed-by: Rob Herring CC: devicet...@vger.kernel.org Signed-off-by: Rui Miguel Silva --- .../devicetree/bindings/media/i2c/ov2680.txt | 40 ++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt new file mode 100644 index ..0e29f1a113c0 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt @@ -0,0 +1,40 @@ +* Omnivision OV2680 MIPI CSI-2 sensor + +Required Properties: +- compatible: should be "ovti,ov2680". +- clocks: reference to the xvclk input clock. +- clock-names: should be "xvclk". + +Optional Properties: +- powerdown-gpios: reference to the GPIO connected to the powerdown pin, +if any. This is an active high signal to the OV2680. + +The device node must contain one 'port' child node for its digital output +video port, and this port must have a single endpoint in accordance with + the video interface bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. + +Endpoint node required properties for CSI-2 connection are: +- remote-endpoint: a phandle to the bus receiver's endpoint node. +- clock-lanes: should be set to <0> (clock lane on hardware lane 0). +- data-lanes: should be set to <1> (one CSI-2 lane supported). + +Example: + +&i2c2 { + ov2680: camera-sensor@36 { + compatible = "ovti,ov2680"; + reg = <0x36>; + clocks = <&osc>; + clock-names = "xvclk"; + powerdown-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>; + + port { + ov2680_mipi_ep: endpoint { + remote-endpoint = <&mipi_sensor_ep>; + clock-lanes = <0>; + data-lanes = <1>; + }; + }; + }; +}; -- 2.16.3
[PATCH v4 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
This patch adds V4L2 sub-device driver for OV2680 image sensor. The OV2680 is a 1/5" CMOS color sensor from Omnivision. Supports output format: 10-bit Raw RGB. The OV2680 has a single lane MIPI interface. The driver exposes following V4L2 controls: - auto/manual exposure, - exposure, - auto/manual gain, - gain, - horizontal/vertical flip, - test pattern menu. Supported resolution are only: QUXGA, 720P, UXGA. Signed-off-by: Rui Miguel Silva --- drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov2680.c | 3 files changed, 1124 insertions(+) create mode 100644 drivers/media/i2c/ov2680.c diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9f18cd296841..39dc9f236ffa 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -586,6 +586,18 @@ config VIDEO_OV2659 To compile this driver as a module, choose M here: the module will be called ov2659. +config VIDEO_OV2680 + tristate "OmniVision OV2680 sensor support" + depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API + depends on MEDIA_CAMERA_SUPPORT + select V4L2_FWNODE + ---help--- + This is a Video4Linux2 sensor-level driver for the OmniVision + OV2680 camera sensor with a MIPI CSI-2 interface. + + To compile this driver as a module, choose M here: the + module will be called ov2680. + config VIDEO_OV5640 tristate "OmniVision OV5640 sensor support" depends on OF diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index c0f94cd8d56d..d0aba4d37b8d 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o obj-$(CONFIG_VIDEO_OV2640) += ov2640.o +obj-$(CONFIG_VIDEO_OV2680) += ov2680.o obj-$(CONFIG_VIDEO_OV5640) += ov5640.o obj-$(CONFIG_VIDEO_OV5645) += ov5645.o obj-$(CONFIG_VIDEO_OV5647) += ov5647.o diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c new file mode 100644 index ..1af6175e09d5 --- /dev/null +++ b/drivers/media/i2c/ov2680.c @@ -0,0 +1, @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Omnivision OV2680 CMOS Image Sensor driver + * + * Copyright (C) 2018 Linaro Ltd + * + * Based on OV5640 Sensor Driver + * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved. + * Copyright (C) 2014-2017 Mentor Graphics Inc. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#define OV2680_XVCLK_VALUE 2400 + +#define OV2680_CHIP_ID 0x2680 + +#define OV2680_REG_STREAM_CTRL 0x0100 +#define OV2680_REG_SOFT_RESET 0x0103 + +#define OV2680_REG_CHIP_ID_HIGH0x300a +#define OV2680_REG_CHIP_ID_LOW 0x300b + +#define OV2680_REG_R_MANUAL0x3503 +#define OV2680_REG_GAIN_PK 0x350a +#define OV2680_REG_EXPOSURE_PK_HIGH0x3500 +#define OV2680_REG_TIMING_HTS 0x380c +#define OV2680_REG_TIMING_VTS 0x380e +#define OV2680_REG_FORMAT1 0x3820 +#define OV2680_REG_FORMAT2 0x3821 + +#define OV2680_REG_ISP_CTRL00 0x5080 + +#define OV2680_FRAME_RATE 30 + +#define OV2680_REG_VALUE_8BIT 1 +#define OV2680_REG_VALUE_16BIT 2 +#define OV2680_REG_VALUE_24BIT 3 + +enum ov2680_mode_id { + OV2680_MODE_QUXGA_800_600, + OV2680_MODE_720P_1280_720, + OV2680_MODE_UXGA_1600_1200, + OV2680_MODE_MAX, +}; + +struct reg_value { + u16 reg_addr; + u8 val; +}; + +struct ov2680_mode_info { + const char *name; + enum ov2680_mode_id id; + u32 width; + u32 height; + const struct reg_value *reg_data; + u32 reg_data_size; +}; + +struct ov2680_ctrls { + struct v4l2_ctrl_handler handler; + struct { + struct v4l2_ctrl *auto_exp; + struct v4l2_ctrl *exposure; + }; + struct { + struct v4l2_ctrl *auto_gain; + struct v4l2_ctrl *gain; + }; + + struct v4l2_ctrl *hflip; + struct v4l2_ctrl *vflip; + struct v4l2_ctrl *test_pattern; +}; + +struct ov2680_dev { + struct i2c_client *i2c_client; + struct v4l2_subdev sd; + + struct media_padpad; + struct clk *xvclk; + u32 xvclk_freq; + + struct gpio_desc*pwdn_gpio; + struct mutexlock; /* protect members */ + + boolmode_pending_changes; + boolis_enabled; + boolis_streaming; + + struct ov2680_ctrls
[PATCH v4 0/2] media: Introduce Omnivision OV2680 driver
Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has a single MIPI lane interface and output format of 10-bit Raw RGB. Features supported are described in PATCH 2/2. v3->v4: Sakari Ailus: - remove auto_{exposure|gain}_enable and direct call the set functions - add separe control sets to gain and exposure - fix number of controls allocated - check the exact frequency that it is supported v2->v3: Rob Herring: - add Reviewed-by tag to dts PATCH 1/1 Sakari Ailus: - align register values with bracket - redone the {write|read}_reg i2c functions - add bayer order handling with flip and mirror controls - fix error path in probe release resources - remove i2c_device_id and use probe_new Myself: - remove ; at the end of macros v1->v2: Fabio Estevam: - s/OV5640/OV2680 in PATCH 1/2 changelog Sakari Ailus: - add description on endpoint properties in bindings - add single endpoint in bindings - drop OF dependency - cleanup includes - fix case in Color Bars - remove frame rate selection - 8/16/24 bit register access in the same transaction - merge _reset and _soft_reset to _enable and rename it to power_on - _gain_set use only the gain value (drop & 0x7ff) - _gain_get remove the (0x377) - single write/read at _exposure_set/get use write_reg24/read_reg24 - move mode_set_direct to _mode_set - _mode_set set auto exposure/gain based on ctrl value - s_frame_interval equal to g_frame_interval - use closest match from: v4l: common: Add a function to obtain best size from a list - check v4l2_ctrl_new_std return in _init - fix gain manual value in auto_cluster Cheers, Rui Rui Miguel Silva (2): media: ov2680: dt: Add bindings for OV2680 media: ov2680: Add Omnivision OV2680 sensor driver .../devicetree/bindings/media/i2c/ov2680.txt | 40 + drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov2680.c | 4 files changed, 1164 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt create mode 100644 drivers/media/i2c/ov2680.c -- 2.16.3
Re: [PATCH v7 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver
Hi Maxime, On 2018-04-03 15:48:59 +0200, Maxime Ripard wrote: > Hi Niklas, > > On Thu, Mar 29, 2018 at 02:35:34PM +0200, Niklas Söderlund wrote: > > > + /* > > > + * Create a static mapping between the CSI virtual channels > > > + * and the input streams. > > > + * > > > + * This should be enhanced, but v4l2 lacks the support for > > > + * changing that mapping dynamically at the moment. > > > + * > > > + * We're protected from the userspace setting up links at the > > > + * same time by the upper layer having called > > > + * media_pipeline_start(). > > > + */ > > > + list_for_each_entry(link, &entity->links, list) { > > > > I wonder is this list_for_each_entry() really needed? Can't you simply > > iterate over all sink pads as with the loop bellow but drop the pad == > > link->sink check? Maybe I'm missing something. > > This was a review made by Sakari here: > https://patchwork.linuxtv.org/patch/44422/ > > The idea is that we need to know if the pad is enabled, and as far as > I know this information is only stored at the link level, not at the > pad level. Ahh I see, you are correct. > > > Apart from this and the small nit-picks (one more bellow) I think this > > patch is fine. Once I understand this I be happy to add my tag to this > > change, great work! > > Is this a reviewed by? :) Yes, I now understand what I did not before :-) Feel free to add my Reviewed-by: Niklas Söderlund > > > I also think you shall consider to add a MAINTAINERS record for the RX > > and TX drivers. Maybe one entry for both drivers as they live in the > > same directory but I think one of the two should add it :-) > > Right, I'll do it, thanks! > Maxime > > -- > Maxime Ripard, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com -- Regards, Niklas Söderlund
Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps
On 2 April 2018 at 16:26, Robert Jarzmik wrote: > Hi, > > This serie is aimed at removing the dmaengine slave compat use, and transfer > knowledge of the DMA requestors into architecture code. > > This was discussed/advised by Arnd a couple of years back, it's almost time. > > The serie is divided in 3 phasees : > - phase 1 : patch 1/15 and patch 2/15 >=> this is the preparation work > - phase 2 : patches 3/15 .. 10/15 >=> this is the switch of all the drivers >=> this one will require either an Ack of the maintainers or be taken by > them > once phase 1 is merged > - phase 3 : patches 11/15 >=> this is the last part, cleanup and removal of export of the DMA filter > function > > As this looks like a patch bomb, each maintainer expressing for his tree > either > an Ack or "I want to take through my tree" will be spared in the next > iterations > of this serie. Perhaps an option is to send this hole series as PR for 3.17 rc1, that would removed some churns and make this faster/easier? Well, if you receive the needed acks of course. For the mmc change: Acked-by: Ulf Hansson Kind regards Uffe > > Several of these changes have been tested on actual hardware, including : > - pxamci > - pxa_camera > - smc* > - ASoC and SSP > > Happy review. > > Robert Jarzmik (15): > dmaengine: pxa: use a dma slave map > ARM: pxa: add dma slave map > mmc: pxamci: remove the dmaengine compat need > media: pxa_camera: remove the dmaengine compat need > mtd: nand: pxa3xx: remove the dmaengine compat need > net: smc911x: remove the dmaengine compat need > net: smc91x: remove the dmaengine compat need > ASoC: pxa: remove the dmaengine compat need > net: irda: pxaficp_ir: remove the dmaengine compat need > ata: pata_pxa: remove the dmaengine compat need > dmaengine: pxa: document pxad_param > dmaengine: pxa: make the filter function internal > ARM: pxa: remove the DMA IO resources > ARM: pxa: change SSP devices allocation > ARM: pxa: change SSP DMA channels allocation > > arch/arm/mach-pxa/devices.c | 269 > ++ > arch/arm/mach-pxa/devices.h | 14 +- > arch/arm/mach-pxa/include/mach/audio.h| 12 ++ > arch/arm/mach-pxa/pxa25x.c| 4 +- > arch/arm/mach-pxa/pxa27x.c| 4 +- > arch/arm/mach-pxa/pxa3xx.c| 5 +- > arch/arm/plat-pxa/ssp.c | 50 +- > drivers/ata/pata_pxa.c| 10 +- > drivers/dma/pxa_dma.c | 13 +- > drivers/media/platform/pxa_camera.c | 22 +-- > drivers/mmc/host/pxamci.c | 29 +--- > drivers/mtd/nand/pxa3xx_nand.c| 10 +- > drivers/net/ethernet/smsc/smc911x.c | 16 +- > drivers/net/ethernet/smsc/smc91x.c| 12 +- > drivers/net/ethernet/smsc/smc91x.h| 1 - > drivers/staging/irda/drivers/pxaficp_ir.c | 14 +- > include/linux/dma/pxa-dma.h | 20 +-- > include/linux/platform_data/mmp_dma.h | 4 + > include/linux/pxa2xx_ssp.h| 4 +- > sound/arm/pxa2xx-ac97.c | 14 +- > sound/arm/pxa2xx-pcm-lib.c| 6 +- > sound/soc/pxa/pxa-ssp.c | 5 +- > sound/soc/pxa/pxa2xx-ac97.c | 32 +--- > 23 files changed, 196 insertions(+), 374 deletions(-) > > -- > 2.11.0 >
Re: [PATCH 02/15] ARM: pxa: add dma slave map
Arnd Bergmann writes: >> + { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) }, >> + { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) }, >> + { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) }, > > This one is interesting, as you are dealing with an off-chip device, > and the channel number is '-'1. How does this even work? Does it > mean This relies on pxa_dma, in which the "-1" for a requestor line means "no requestor" or said in another way "always requesting". As a consequence, as soon as the DMA descriptors are queued, the transfer begins, and it is supposed implicitely that the FIFO output availability is at least as quick as the system bus and the DMA size is perfectly fit for the FIFO available bytes. This is what has been the underlying of DMA transfers of smc91x(x) on the PXA platforms, where the smc91x(s) are directly wired on the system bus (the same bus having DRAM, SRAM, IO-mapped devices). > >> + /* PXA25x specific map */ >> + { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) }, >> + { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) }, >> + { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) }, >> + { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) }, >> + { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) }, >> + { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) }, >> + { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) }, >> + { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) }, >> + { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) }, >> + { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) }, >> + >> + /* PXA27x specific map */ >> + { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) }, >> + { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) }, >> + { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) }, >> + { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) }, >> + { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) }, >> + >> + /* PXA3xx specific map */ >> + { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) }, >> + { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) }, >> + { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) }, >> + { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) }, >> + { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) }, >> + { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) }, >> + { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) }, >> +}; > > Since more than half the entries in here are chip specific, maybe it would be > better to split that table into three and have a copy for each one in > arch/arm/mach-pxa/pxa{25x.27x.3xx}.c? Mmmh, today the split is : - 16 common entries - 10 pxa25x specific entries - 5 pxa27x specific entries - 7 pxa3xx specific entries => total of 38 lines After the split we'll have : - 26 pxa25x specific entries - 21 pxa27x specific entries - 23 pxa3xx specific entries => total of 70 lines That doubles the number of lines, not counting the declarations, and amending of pxa2xx_set_dmac_info(). If you think it's worth it, what is the driving benefit behind ? > Does that mean it's actually a memory-to-memory transfer with a device being > on the external SRAM interface? I'm taking this is the follow up to the "-1" question :0 Cheers. -- Robert
Re: [PATCH 02/15] ARM: pxa: add dma slave map
On Tue, Apr 3, 2018 at 5:18 PM, Robert Jarzmik wrote: > Arnd Bergmann writes: > >>> + { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) }, >>> + { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) }, >>> + { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) }, >> >> This one is interesting, as you are dealing with an off-chip device, >> and the channel number is '-'1. How does this even work? Does it >> mean > > This relies on pxa_dma, in which the "-1" for a requestor line means "no > requestor" or said in another way "always requesting". As a consequence, as > soon > as the DMA descriptors are queued, the transfer begins, and it is supposed > implicitely that the FIFO output availability is at least as quick as the > system > bus and the DMA size is perfectly fit for the FIFO available bytes. > > This is what has been the underlying of DMA transfers of smc91x(x) on the PXA > platforms, where the smc91x(s) are directly wired on the system bus (the same > bus having DRAM, SRAM, IO-mapped devices). Ok, I looked at the driver in more detail now and found the scary parts. So it's using the async DMA interface to do synchronous DMA in interrupt context in order to transfer the rx data faster than an readsl() would, correct? It still feels odd to me that there is an entry in the slave map for a device that does not have a request line. However, it also seems that the entire code in those two drivers that deals with DMA is specific to PXA anyway, so maybe it can be done differently: instead of calling dma_request_slave_channel_compat() or dma_request_chan() with a fake request line, how about calling dma_request_channel() with an NULL filter function and data, and have the driver handle the empty data case the same way as the rq=-1 case today? >>> + /* PXA25x specific map */ >>> + { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) }, >>> + { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) }, >>> + { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) }, >>> + { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) }, >>> + { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) }, >>> + { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) }, >>> + { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) }, >>> + { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) }, >>> + { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) }, >>> + { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) }, >>> + >>> + /* PXA27x specific map */ >>> + { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) }, >>> + { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) }, >>> + { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) }, >>> + { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) }, >>> + { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) }, >>> + >>> + /* PXA3xx specific map */ >>> + { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) }, >>> + { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) }, >>> + { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) }, >>> + { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) }, >>> + { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) }, >>> + { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) }, >>> + { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) }, >>> +}; >> >> Since more than half the entries in here are chip specific, maybe it would be >> better to split that table into three and have a copy for each one in >> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c? > Mmmh, today the split is : > - 16 common entries > - 10 pxa25x specific entries > - 5 pxa27x specific entries > - 7 pxa3xx specific entries > => total of 38 lines > > After the split we'll have : > - 26 pxa25x specific entries > - 21 pxa27x specific entries > - 23 pxa3xx specific entries > => total of 70 lines > > That doubles the number of lines, not counting the declarations, and amending > of > pxa2xx_set_dmac_info(). > > If you think it's worth it, what is the driving benefit behind ? It seems a bit cleaner to only register the tables for the dma lines that are actually present on a given chip. >> Does that mean it's actually a memory-to-memory transfer with a device being >> on the external SRAM interface? > I'm taking this is the follow up to the "-1" question :0 Right. Arnd
Re: [PATCH 4/8] dma-buf: add peer2peer flag
On Tue, Apr 03, 2018 at 11:09:09AM +0200, Daniel Vetter wrote: > On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote: > > Am 29.03.2018 um 08:57 schrieb Daniel Vetter: > > > On Sun, Mar 25, 2018 at 12:59:56PM +0200, Christian König wrote: > > > > Add a peer2peer flag noting that the importer can deal with device > > > > resources which are not backed by pages. > > > > > > > > Signed-off-by: Christian König > > > Um strictly speaking they all should, but ttm never bothered to use the > > > real interfaces but just hacked around the provided sg list, grabbing the > > > underlying struct pages, then rebuilding&remapping the sg list again. > > > > Actually that isn't correct. TTM converts them to a dma address array > > because drivers need it like this (at least nouveau, radeon and amdgpu). > > > > I've fixed radeon and amdgpu to be able to deal without it and mailed with > > Ben about nouveau, but the outcome is they don't really know. > > > > TTM itself doesn't have any need for the pages on imported BOs (you can't > > mmap them anyway), the real underlying problem is that sg tables doesn't > > provide what drivers need. > > > > I think we could rather easily fix sg tables, but that is a totally separate > > task. > > Looking at patch 8, the sg table seems perfectly sufficient to convey the > right dma addresses to the importer. Ofcourse the exporter has to set up > the right kind of iommu mappings to make this work. > > > > The entire point of using sg lists was exactly to allow this use case of > > > peer2peer dma (or well in general have special exporters which managed > > > memory/IO ranges not backed by struct page). So essentially you're having > > > a "I'm totally not broken flag" here. > > > > No, independent of needed struct page pointers we need to note if the > > exporter can handle peer2peer stuff from the hardware side in general. > > > > So what I've did is just to set peer2peer allowed on the importer because of > > the driver needs and clear it in the exporter if the hardware can't handle > > that. > > The only thing the importer seems to do is call the > pci_peer_traffic_supported, which the exporter could call too. What am I > missing (since the sturct_page stuff sounds like it's fixed already by > you)? > -Daniel AFAIK Logan patchset require to register and initialize struct page for the device memory you want to map (export from exporter point of view). With GPU this isn't something we want, struct page is >~= 2^6 so for 4GB GPU = 2^6*2^32/2^12 = 2^26 = 64MB of RAM 8GB GPU = 2^6*2^33/2^12 = 2^27 = 128MB of RAM 16GB GPU = 2^6*2^34/2^12 = 2^28 = 256MB of RAM 32GB GPU = 2^6*2^34/2^12 = 2^29 = 512MB of RAM All this is mostly wasted as only a small sub-set (that can not be constraint to specific range) will ever be exported at any point in time. For GPU work load this is hardly justifiable, even for HMM i do not plan to register all those pages. Hence why i argue that dma_map_resource() like use by Christian is good enough for us. People that care about SG can fix that but i rather not have to depend on that and waste system memory. Cheers, Jérôme
Re: [PATCH 4/8] dma-buf: add peer2peer flag
On Tue, Apr 03, 2018 at 01:06:45PM -0400, Jerome Glisse wrote: > On Tue, Apr 03, 2018 at 11:09:09AM +0200, Daniel Vetter wrote: > > On Thu, Mar 29, 2018 at 01:34:24PM +0200, Christian König wrote: > > > Am 29.03.2018 um 08:57 schrieb Daniel Vetter: > > > > On Sun, Mar 25, 2018 at 12:59:56PM +0200, Christian König wrote: > > > > > Add a peer2peer flag noting that the importer can deal with device > > > > > resources which are not backed by pages. > > > > > > > > > > Signed-off-by: Christian König > > > > Um strictly speaking they all should, but ttm never bothered to use the > > > > real interfaces but just hacked around the provided sg list, grabbing > > > > the > > > > underlying struct pages, then rebuilding&remapping the sg list again. > > > > > > Actually that isn't correct. TTM converts them to a dma address array > > > because drivers need it like this (at least nouveau, radeon and amdgpu). > > > > > > I've fixed radeon and amdgpu to be able to deal without it and mailed with > > > Ben about nouveau, but the outcome is they don't really know. > > > > > > TTM itself doesn't have any need for the pages on imported BOs (you can't > > > mmap them anyway), the real underlying problem is that sg tables doesn't > > > provide what drivers need. > > > > > > I think we could rather easily fix sg tables, but that is a totally > > > separate > > > task. > > > > Looking at patch 8, the sg table seems perfectly sufficient to convey the > > right dma addresses to the importer. Ofcourse the exporter has to set up > > the right kind of iommu mappings to make this work. > > > > > > The entire point of using sg lists was exactly to allow this use case of > > > > peer2peer dma (or well in general have special exporters which managed > > > > memory/IO ranges not backed by struct page). So essentially you're > > > > having > > > > a "I'm totally not broken flag" here. > > > > > > No, independent of needed struct page pointers we need to note if the > > > exporter can handle peer2peer stuff from the hardware side in general. > > > > > > So what I've did is just to set peer2peer allowed on the importer because > > > of > > > the driver needs and clear it in the exporter if the hardware can't handle > > > that. > > > > The only thing the importer seems to do is call the > > pci_peer_traffic_supported, which the exporter could call too. What am I > > missing (since the sturct_page stuff sounds like it's fixed already by > > you)? > > -Daniel > > AFAIK Logan patchset require to register and initialize struct page > for the device memory you want to map (export from exporter point of > view). > > With GPU this isn't something we want, struct page is >~= 2^6 so for > 4GB GPU = 2^6*2^32/2^12 = 2^26 = 64MB of RAM > 8GB GPU = 2^6*2^33/2^12 = 2^27 = 128MB of RAM > 16GB GPU = 2^6*2^34/2^12 = 2^28 = 256MB of RAM > 32GB GPU = 2^6*2^34/2^12 = 2^29 = 512MB of RAM > > All this is mostly wasted as only a small sub-set (that can not be > constraint to specific range) will ever be exported at any point in > time. For GPU work load this is hardly justifiable, even for HMM i > do not plan to register all those pages. > > Hence why i argue that dma_map_resource() like use by Christian is > good enough for us. People that care about SG can fix that but i > rather not have to depend on that and waste system memory. I did not mean you should dma_map_sg/page. I just meant that using dma_map_resource to fill only the dma address part of the sg table seems perfectly sufficient. And that's exactly why the importer gets an already mapped sg table, so that it doesn't have to call dma_map_sg on something that dma_map_sg can't handle. Assuming you get an sg table that's been mapping by calling dma_map_sg was always a bit a case of bending the abstraction to avoid typing code. The only thing an importer ever should have done is look at the dma addresses in that sg table, nothing else. And p2p seems to perfectly fit into this (surprise, it was meant to). That's why I suggested we annotate the broken importers who assume the sg table is mapped using dma_map_sg or has a struct_page backing the memory (but there doesn't seem to be any left it seems) instead of annotating the ones that aren't broken with a flag that's confusing - you could also have a dma-buf sgt that points at some other memory that doesn't have struct pages backing it. Aside: At least internally in i915 we've been using this forever for our own private/stolen memory. Unfortunately no other device can access that range of memory, which is why we don't allow it to be imported to anything but i915 itself. But if that hw restriction doesn't exist, it'd would work. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] media: platform: video-mux: propagate format from sink to source
Propagate the v4l2_mbus_framefmt to the source pad when either a sink pad is activated or when the format of the active sink pad changes. Signed-off-by: Chris Lesiak --- drivers/media/platform/video-mux.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c index ee89ad76bee2..1fb887293337 100644 --- a/drivers/media/platform/video-mux.c +++ b/drivers/media/platform/video-mux.c @@ -45,6 +45,7 @@ static int video_mux_link_setup(struct media_entity *entity, { struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); struct video_mux *vmux = v4l2_subdev_to_video_mux(sd); + u16 source_pad = entity->num_pads - 1; int ret = 0; /* @@ -74,6 +75,9 @@ static int video_mux_link_setup(struct media_entity *entity, if (ret < 0) goto out; vmux->active = local->index; + + /* Propagate the active format to the source */ + vmux->format_mbus[source_pad] = vmux->format_mbus[vmux->active]; } else { if (vmux->active != local->index) goto out; @@ -162,14 +166,20 @@ static int video_mux_set_format(struct v4l2_subdev *sd, struct v4l2_subdev_format *sdformat) { struct video_mux *vmux = v4l2_subdev_to_video_mux(sd); - struct v4l2_mbus_framefmt *mbusformat; + struct v4l2_mbus_framefmt *mbusformat, *source_mbusformat; struct media_pad *pad = &vmux->pads[sdformat->pad]; + u16 source_pad = sd->entity.num_pads - 1; mbusformat = __video_mux_get_pad_format(sd, cfg, sdformat->pad, sdformat->which); if (!mbusformat) return -EINVAL; + source_mbusformat = __video_mux_get_pad_format(sd, cfg, source_pad, + sdformat->which); + if (!source_mbusformat) + return -EINVAL; + mutex_lock(&vmux->lock); /* Source pad mirrors active sink pad, no limitations on sink pads */ @@ -178,6 +188,10 @@ static int video_mux_set_format(struct v4l2_subdev *sd, *mbusformat = sdformat->format; + /* Propagate the format from an active sink to source */ + if ((pad->flags & MEDIA_PAD_FL_SINK) && (pad->index == vmux->active)) + *source_mbusformat = sdformat->format; + mutex_unlock(&vmux->lock); return 0; -- 2.14.3
Re: [PATCH v4 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi Rui, I love your patch! Yet something to improve: [auto build test ERROR on v4.16] [cannot apply to linuxtv-media/master next-20180403] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180404-032903 config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sh All errors (new ones prefixed by >>): drivers/media//i2c/ov2680.c: In function 'ov2680_set_fmt': >> drivers/media//i2c/ov2680.c:692:9: error: implicit declaration of function >> 'v4l2_find_nearest_size'; did you mean 'v4l2_find_nearest_format'? >> [-Werror=implicit-function-declaration] mode = v4l2_find_nearest_size(ov2680_mode_data, ^~ v4l2_find_nearest_format >> drivers/media//i2c/ov2680.c:693:41: error: 'width' undeclared (first use in >> this function) ARRAY_SIZE(ov2680_mode_data), width, ^ drivers/media//i2c/ov2680.c:693:41: note: each undeclared identifier is reported only once for each function it appears in >> drivers/media//i2c/ov2680.c:694:11: error: 'height' undeclared (first use in >> this function); did you mean 'hweight8'? height, fmt->width, fmt->height); ^~ hweight8 cc1: some warnings being treated as errors vim +692 drivers/media//i2c/ov2680.c 672 673 static int ov2680_set_fmt(struct v4l2_subdev *sd, 674struct v4l2_subdev_pad_config *cfg, 675struct v4l2_subdev_format *format) 676 { 677 struct ov2680_dev *sensor = to_ov2680_dev(sd); 678 struct v4l2_mbus_framefmt *fmt = &format->format; 679 const struct ov2680_mode_info *mode; 680 int ret = 0; 681 682 if (format->pad != 0) 683 return -EINVAL; 684 685 mutex_lock(&sensor->lock); 686 687 if (sensor->is_streaming) { 688 ret = -EBUSY; 689 goto unlock; 690 } 691 > 692 mode = v4l2_find_nearest_size(ov2680_mode_data, > 693ARRAY_SIZE(ov2680_mode_data), > width, > 694height, fmt->width, fmt->height); 695 if (!mode) { 696 ret = -EINVAL; 697 goto unlock; 698 } 699 700 if (format->which == V4L2_SUBDEV_FORMAT_TRY) { 701 fmt = v4l2_subdev_get_try_format(sd, cfg, 0); 702 703 *fmt = format->format; 704 goto unlock; 705 } 706 707 fmt->width = mode->width; 708 fmt->height = mode->height; 709 fmt->code = sensor->fmt.code; 710 fmt->colorspace = sensor->fmt.colorspace; 711 712 sensor->current_mode = mode; 713 sensor->fmt = format->format; 714 sensor->mode_pending_changes = true; 715 716 unlock: 717 mutex_unlock(&sensor->lock); 718 719 return ret; 720 } 721 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH 5/5] media: docs: selection: fix misleading sentence about the CROP API
The API limitation described here is about the CROP API, not about the entire V4L2. Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst index ba1064a244a0..e7455fb1e572 100644 --- a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst +++ b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst @@ -15,7 +15,7 @@ because the described operation is actually the composing. The selection API makes a clear distinction between composing and cropping operations by setting the appropriate targets. -The V4L2 API lacks any support for composing to and cropping from an +The CROP API lacks any support for composing to and cropping from an image inside a memory buffer. The application could configure a capture device to fill only a part of an image by abusing V4L2 API. Cropping a smaller image from a larger one is achieved by setting -- 2.7.4
[PATCH 4/5] media: docs: selection: improve formatting
Split section "Comparison with old cropping API" in paragraphs for easier reading and improve visible links text. Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- .../media/uapi/v4l/selection-api-vs-crop-api.rst | 55 -- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst index 2ad30a49184f..ba1064a244a0 100644 --- a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst +++ b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst @@ -6,31 +6,34 @@ Comparison with old cropping API -The selection API was introduced to cope with deficiencies of previous -:ref:`API `, that was designed to control simple capture -devices. Later the cropping API was adopted by video output drivers. The -ioctls are used to select a part of the display were the video signal is -inserted. It should be considered as an API abuse because the described -operation is actually the composing. The selection API makes a clear -distinction between composing and cropping operations by setting the -appropriate targets. The V4L2 API lacks any support for composing to and -cropping from an image inside a memory buffer. The application could -configure a capture device to fill only a part of an image by abusing -V4L2 API. Cropping a smaller image from a larger one is achieved by -setting the field ``bytesperline`` at struct -:c:type:`v4l2_pix_format`. -Introducing an image offsets could be done by modifying field ``m_userptr`` -at struct -:c:type:`v4l2_buffer` before calling -:ref:`VIDIOC_QBUF`. Those operations should be avoided because they are not -portable (endianness), and do not work for macroblock and Bayer formats -and mmap buffers. The selection API deals with configuration of buffer +The selection API was introduced to cope with deficiencies of the +older :ref:`CROP API `, that was designed to control simple +capture devices. Later the cropping API was adopted by video output +drivers. The ioctls are used to select a part of the display were the +video signal is inserted. It should be considered as an API abuse +because the described operation is actually the composing. The +selection API makes a clear distinction between composing and cropping +operations by setting the appropriate targets. + +The V4L2 API lacks any support for composing to and cropping from an +image inside a memory buffer. The application could configure a +capture device to fill only a part of an image by abusing V4L2 +API. Cropping a smaller image from a larger one is achieved by setting +the field ``bytesperline`` at struct :c:type:`v4l2_pix_format`. +Introducing an image offsets could be done by modifying field +``m_userptr`` at struct :c:type:`v4l2_buffer` before calling +:ref:`VIDIOC_QBUF `. Those operations should be avoided +because they are not portable (endianness), and do not work for +macroblock and Bayer formats and mmap buffers. + +The selection API deals with configuration of buffer cropping/composing in a clear, intuitive and portable way. Next, with the selection API the concepts of the padded target and constraints -flags are introduced. Finally, struct :c:type:`v4l2_crop` -and struct :c:type:`v4l2_cropcap` have no reserved -fields. Therefore there is no way to extend their functionality. The new -struct :c:type:`v4l2_selection` provides a lot of place -for future extensions. Driver developers are encouraged to implement -only selection API. The former cropping API would be simulated using the -new one. +flags are introduced. Finally, struct :c:type:`v4l2_crop` and struct +:c:type:`v4l2_cropcap` have no reserved fields. Therefore there is no +way to extend their functionality. The new struct +:c:type:`v4l2_selection` provides a lot of place for future +extensions. + +Driver developers are encouraged to implement only selection API. The +former cropping API would be simulated using the new one. -- 2.7.4
[PATCH 1/5] media: docs: selection: fix typos
Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- Documentation/media/uapi/v4l/selection-api-004.rst | 2 +- Documentation/media/uapi/v4l/selection.svg | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/media/uapi/v4l/selection-api-004.rst b/Documentation/media/uapi/v4l/selection-api-004.rst index d782cd5b2117..0a4ddc2d71db 100644 --- a/Documentation/media/uapi/v4l/selection-api-004.rst +++ b/Documentation/media/uapi/v4l/selection-api-004.rst @@ -41,7 +41,7 @@ The driver may further adjust the requested size and/or position according to hardware limitations. Each capture device has a default source rectangle, given by the -``V4L2_SEL_TGT_CROP_DEFAULT`` target. This rectangle shall over what the +``V4L2_SEL_TGT_CROP_DEFAULT`` target. This rectangle shall cover what the driver writer considers the complete picture. Drivers shall set the active crop rectangle to the default when the driver is first loaded, but not later. diff --git a/Documentation/media/uapi/v4l/selection.svg b/Documentation/media/uapi/v4l/selection.svg index a93e3b59786d..911062bd2844 100644 --- a/Documentation/media/uapi/v4l/selection.svg +++ b/Documentation/media/uapi/v4l/selection.svg @@ -1128,11 +1128,11 @@ - COMPOSE_BONDS + COMPOSE_BOUNDS -CROP_BONDS +CROP_BOUNDS overscan area -- 2.7.4
[PATCH 3/5] media: docs: selection: rename files to something meaningful
These files have an automatically-generated numbering. Replaname them to something that suggests their meaning. Reported-by: Hans Verkuil Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- .../{selection-api-004.rst => selection-api-configuration.rst} | 0 .../v4l/{selection-api-006.rst => selection-api-examples.rst} | 0 .../v4l/{selection-api-002.rst => selection-api-intro.rst} | 0 .../v4l/{selection-api-003.rst => selection-api-targets.rst} | 0 .../{selection-api-005.rst => selection-api-vs-crop-api.rst} | 0 Documentation/media/uapi/v4l/selection-api.rst | 10 +- 6 files changed, 5 insertions(+), 5 deletions(-) rename Documentation/media/uapi/v4l/{selection-api-004.rst => selection-api-configuration.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-006.rst => selection-api-examples.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-002.rst => selection-api-intro.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-003.rst => selection-api-targets.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-005.rst => selection-api-vs-crop-api.rst} (100%) diff --git a/Documentation/media/uapi/v4l/selection-api-004.rst b/Documentation/media/uapi/v4l/selection-api-configuration.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-004.rst rename to Documentation/media/uapi/v4l/selection-api-configuration.rst diff --git a/Documentation/media/uapi/v4l/selection-api-006.rst b/Documentation/media/uapi/v4l/selection-api-examples.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-006.rst rename to Documentation/media/uapi/v4l/selection-api-examples.rst diff --git a/Documentation/media/uapi/v4l/selection-api-002.rst b/Documentation/media/uapi/v4l/selection-api-intro.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-002.rst rename to Documentation/media/uapi/v4l/selection-api-intro.rst diff --git a/Documentation/media/uapi/v4l/selection-api-003.rst b/Documentation/media/uapi/v4l/selection-api-targets.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-003.rst rename to Documentation/media/uapi/v4l/selection-api-targets.rst diff --git a/Documentation/media/uapi/v4l/selection-api-005.rst b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-005.rst rename to Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst diff --git a/Documentation/media/uapi/v4l/selection-api.rst b/Documentation/media/uapi/v4l/selection-api.rst index e4e623824b30..390233f704a3 100644 --- a/Documentation/media/uapi/v4l/selection-api.rst +++ b/Documentation/media/uapi/v4l/selection-api.rst @@ -9,8 +9,8 @@ Cropping, composing and scaling -- the SELECTION API .. toctree:: :maxdepth: 1 -selection-api-002 -selection-api-003 -selection-api-004 -selection-api-005 -selection-api-006 +selection-api-intro.rst +selection-api-targets.rst +selection-api-configuration.rst +selection-api-vs-crop-api.rst +selection-api-examples.rst -- 2.7.4
[PATCH 2/5] media: docs: clarify relationship between crop and selection APIs
Having two somewhat similar and largely overlapping APIs is confusing, especially since the older one appears in the docs before the newer and most featureful counterpart. Clarify all of this in several ways: - swap the two sections - give a name to the two APIs in the section names - add a note at the beginning of the CROP API section Also remove a note that is incorrect (correct wording is in vidioc-cropcap.rst). Signed-off-by: Luca Ceresoli Based on info from: Hans Verkuil Cc: Hans Verkuil --- Documentation/media/uapi/v4l/common.rst| 2 +- Documentation/media/uapi/v4l/crop.rst | 21 - Documentation/media/uapi/v4l/selection-api-005.rst | 2 ++ Documentation/media/uapi/v4l/selection-api.rst | 4 ++-- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Documentation/media/uapi/v4l/common.rst b/Documentation/media/uapi/v4l/common.rst index 13f2ed3fc5a6..5f93e71122ef 100644 --- a/Documentation/media/uapi/v4l/common.rst +++ b/Documentation/media/uapi/v4l/common.rst @@ -41,6 +41,6 @@ applicable to all devices. extended-controls format planar-apis -crop selection-api +crop streaming-par diff --git a/Documentation/media/uapi/v4l/crop.rst b/Documentation/media/uapi/v4l/crop.rst index 182565b9ace4..83fa16eb347e 100644 --- a/Documentation/media/uapi/v4l/crop.rst +++ b/Documentation/media/uapi/v4l/crop.rst @@ -2,9 +2,18 @@ .. _crop: -* -Image Cropping, Insertion and Scaling -* +* +Image Cropping, Insertion and Scaling -- the CROP API +* + +.. note:: + + The CROP API is mostly superseded by the newer :ref:`SELECTION API + `. The new API should be preferred in most cases, + with the exception of pixel aspect ratio detection, which is + implemented by :ref:`VIDIOC_CROPCAP ` and has no + equivalent in the SELECTION API. See :ref:`selection-vs-crop` for a + comparison of the two APIs. Some video capture devices can sample a subsection of the picture and shrink or enlarge it to an image of arbitrary size. We call these @@ -40,12 +49,6 @@ support scaling or the :ref:`VIDIOC_G_CROP ` and :ref:`VIDIOC_S_CROP ` ioctls. Their size (and position where applicable) will be fixed in this case. -.. note:: - - All capture and output devices must support the - :ref:`VIDIOC_CROPCAP ` ioctl such that applications - can determine if scaling takes place. - Cropping Structures === diff --git a/Documentation/media/uapi/v4l/selection-api-005.rst b/Documentation/media/uapi/v4l/selection-api-005.rst index 5b47a28ac6d7..2ad30a49184f 100644 --- a/Documentation/media/uapi/v4l/selection-api-005.rst +++ b/Documentation/media/uapi/v4l/selection-api-005.rst @@ -1,5 +1,7 @@ .. -*- coding: utf-8; mode: rst -*- +.. _selection-vs-crop: + Comparison with old cropping API diff --git a/Documentation/media/uapi/v4l/selection-api.rst b/Documentation/media/uapi/v4l/selection-api.rst index 81ea52d785b9..e4e623824b30 100644 --- a/Documentation/media/uapi/v4l/selection-api.rst +++ b/Documentation/media/uapi/v4l/selection-api.rst @@ -2,8 +2,8 @@ .. _selection-api: -API for cropping, composing and scaling -=== +Cropping, composing and scaling -- the SELECTION API + .. toctree:: -- 2.7.4
Re: [PATCH v13 29/33] rcar-vin: add link notify for Gen3
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:52 EEST Niklas Söderlund wrote: > Add the ability to process media device link change requests. Link > enabling is a bit complicated on Gen3, whether or not it's possible to > enable a link depends on what other links already are enabled. On Gen3 > the 8 VINs are split into two subgroup's (VIN0-3 and VIN4-7) and from a > routing perspective these two groups are independent of each other. > Each subgroup's routing is controlled by the subgroup VIN master > instance (VIN0 and VIN4). > > There are a limited number of possible route setups available for each > subgroup and the configuration of each setup is dictated by the > hardware. On H3 for example there are 6 possible route setups for each > subgroup to choose from. > > This leads to the media device link notification code being rather large > since it will find the best routing configuration to try and accommodate > as many links as possible. When it's not possible to enable a new link > due to hardware constrains the link_notifier callback will return > -EMLINK. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil Reviewed-by: Laurent Pinchart > > --- > > * Changes since v11 > - Fixed spelling > - Updated comment to clarify the intent that no link can be enabled if > any video node is open. > - Use container_of() instead of a loop to find struct vin_dev from the > video device. > --- > drivers/media/platform/rcar-vin/rcar-core.c | 147 > 1 file changed, 147 insertions(+) > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c > b/drivers/media/platform/rcar-vin/rcar-core.c index > 99f6301a778046df..0cc76d73115e9277 100644 > --- a/drivers/media/platform/rcar-vin/rcar-core.c > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > @@ -24,6 +24,7 @@ > > #include > #include > +#include > > #include "rcar-vin.h" > > @@ -44,6 +45,151 @@ > */ > #define rvin_group_id_to_master(vin) ((vin) < 4 ? 0 : 4) > > +/* > --- > -- + * Media Controller link notification > + */ > + > +/* group lock should be held when calling this function. */ > +static int rvin_group_entity_to_csi_id(struct rvin_group *group, > +struct media_entity *entity) > +{ > + struct v4l2_subdev *sd; > + unsigned int i; > + > + sd = media_entity_to_v4l2_subdev(entity); > + > + for (i = 0; i < RVIN_CSI_MAX; i++) > + if (group->csi[i].subdev == sd) > + return i; > + > + return -ENODEV; > +} > + > +static unsigned int rvin_group_get_mask(struct rvin_dev *vin, > + enum rvin_csi_id csi_id, > + unsigned char channel) > +{ > + const struct rvin_group_route *route; > + unsigned int mask = 0; > + > + for (route = vin->info->routes; route->mask; route++) { > + if (route->vin == vin->id && > + route->csi == csi_id && > + route->channel == channel) { > + vin_dbg(vin, > + "Adding route: vin: %d csi: %d channel: %d\n", > + route->vin, route->csi, route->channel); > + mask |= route->mask; > + } > + } > + > + return mask; > +} > + > +/* > + * Link setup for the links between a VIN and a CSI-2 receiver is a bit > + * complex. The reason for this is that the register controlling routing > + * is not present in each VIN instance. There are special VINs which > + * control routing for themselves and other VINs. There are not many > + * different possible links combinations that can be enabled at the same > + * time, therefor all already enabled links which are controlled by a > + * master VIN need to be taken into account when making the decision > + * if a new link can be enabled or not. > + * > + * 1. Find out which VIN the link the user tries to enable is connected to. > + * 2. Lookup which master VIN controls the links for this VIN. > + * 3. Start with a bitmask with all bits set. > + * 4. For each previously enabled link from the master VIN bitwise AND its > + *route mask (see documentation for mask in struct rvin_group_route) > + *with the bitmask. > + * 5. Bitwise AND the mask for the link the user tries to enable to the > bitmask. + * 6. If the bitmask is not empty at this point the new link can > be enabled + *while keeping all previous links enabled. Update the > CHSEL value of the + *master VIN and inform the user that the link > could be enabled. + * > + * Please note that no link can be enabled if any VIN in the group is > + * currently open. > + */ > +static int rvin_group_link_notify(struct media_link *link, u32 flags, > + unsigned int notification) > +{ > + struct rvin_group *group = container_of(link->graph_obj.mdev, > +
Re: [PATCH v13 27/33] rcar-vin: add chsel information to rvin_info
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:50 EEST Niklas Söderlund wrote: > Each Gen3 SoC has a limited set of predefined routing possibilities for > which CSI-2 device and channel can be routed to which VIN instance. > Prepare to store this information in the struct rvin_info. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil Reviewed-by: Laurent Pinchart > --- > > * Changes since v11 > - Fixed spelling. > - Reorderd filed order in struct rvin_group_route. > - Renamed chan to channel in struct rvin_group_route. > --- > drivers/media/platform/rcar-vin/rcar-vin.h | 42 +++ > 1 file changed, 42 insertions(+) > > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h > b/drivers/media/platform/rcar-vin/rcar-vin.h index > cf5c467d45e10847..93eb40856b866117 100644 > --- a/drivers/media/platform/rcar-vin/rcar-vin.h > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h > @@ -43,6 +43,14 @@ enum model_id { > RCAR_GEN3, > }; > > +enum rvin_csi_id { > + RVIN_CSI20, > + RVIN_CSI21, > + RVIN_CSI40, > + RVIN_CSI41, > + RVIN_CSI_MAX, > +}; > + > /** > * STOPPED - No operation in progress > * RUNNING - Operation in progress have buffers > @@ -79,12 +87,45 @@ struct rvin_graph_entity { > unsigned int sink_pad; > }; > > +/** > + * struct rvin_group_route - describes a route from a channel of a > + * CSI-2 receiver to a VIN > + * > + * @csi: CSI-2 receiver ID. > + * @channel: Output channel of the CSI-2 receiver. > + * @vin: VIN ID. > + * @mask:Bitmask of the different CHSEL register values that > + * allow for a route from @csi + @chan to @vin. > + * > + * .. note:: > + * Each R-Car CSI-2 receiver has four output channels facing the VIN > + * devices, each channel can carry one CSI-2 Virtual Channel (VC). > + * There is no correlation between channel number and CSI-2 VC. It's > + * up to the CSI-2 receiver driver to configure which VC is output > + * on which channel, the VIN devices only care about output channels. > + * > + * There are in some cases multiple CHSEL register settings which would > + * allow for the same route from @csi + @channel to @vin. For example > + * on R-Car H3 both the CHSEL values 0 and 3 allow for a route from > + * CSI40/VC0 to VIN0. All possible CHSEL values for a route need to be > + * recorded as a bitmask in @mask, in this example bit 0 and 3 should > + * be set. > + */ > +struct rvin_group_route { > + enum rvin_csi_id csi; > + unsigned int channel; > + unsigned int vin; > + unsigned int mask; > +}; > + > /** > * struct rvin_info - Information about the particular VIN implementation > * @model: VIN model > * @use_mc: use media controller instead of controlling subdevice > * @max_width: max input width the VIN supports > * @max_height: max input height the VIN supports > + * @routes: list of possible routes from the CSI-2 recivers to > + * all VINs. The list mush be NULL terminated. > */ > struct rvin_info { > enum model_id model; > @@ -92,6 +133,7 @@ struct rvin_info { > > unsigned int max_width; > unsigned int max_height; > + const struct rvin_group_route *routes; > }; > > /** -- Regards, Laurent Pinchart
Re: [PATCH v13 23/33] rcar-vin: force default colorspace for media centric mode
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:46 EEST Niklas Söderlund wrote: > The V4L2 specification clearly documents the colorspace fields as being > set by drivers for capture devices. Using the values supplied by > userspace thus wouldn't comply with the API. Until the API is updated to > allow for userspace to set these Hans wants the fields to be set by the > driver to fixed values. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil Reviewed-by: Laurent Pinchart > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 21 +++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > 2280535ca981993f..ea0759a645e49490 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -664,12 +664,29 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = { > * V4L2 Media Controller > */ > > +static int rvin_mc_try_format(struct rvin_dev *vin, struct v4l2_pix_format > *pix) +{ > + /* > + * The V4L2 specification clearly documents the colorspace fields > + * as being set by drivers for capture devices. Using the values > + * supplied by userspace thus wouldn't comply with the API. Until > + * the API is updated force fixed vaules. > + */ > + pix->colorspace = RVIN_DEFAULT_COLORSPACE; > + pix->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(pix->colorspace); > + pix->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(pix->colorspace); > + pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace, > + pix->ycbcr_enc); > + > + return rvin_format_align(vin, pix); > +} > + > static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv, > struct v4l2_format *f) > { > struct rvin_dev *vin = video_drvdata(file); > > - return rvin_format_align(vin, &f->fmt.pix); > + return rvin_mc_try_format(vin, &f->fmt.pix); > } > > static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv, > @@ -681,7 +698,7 @@ static int rvin_mc_s_fmt_vid_cap(struct file *file, void > *priv, if (vb2_is_busy(&vin->queue)) > return -EBUSY; > > - ret = rvin_format_align(vin, &f->fmt.pix); > + ret = rvin_mc_try_format(vin, &f->fmt.pix); > if (ret) > return ret; -- Regards, Laurent Pinchart
Re: [PATCH v13 22/33] rcar-vin: use different v4l2 operations in media controller mode
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:45 EEST Niklas Söderlund wrote: > When the driver runs in media controller mode it should not directly > control the subdevice instead userspace will be responsible for > configuring the pipeline. To be able to run in this mode a different set > of v4l2 operations needs to be used. > > Add a new set of v4l2 operations to support operation without directly > interacting with the source subdevice. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil Reviewed-by: Laurent Pinchart > > --- > > * Changes since v11 > - Fixed error labels name in rvin_mc_open(). > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 2 +- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 161 > +++- 2 files changed, 159 insertions(+), 4 > deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c index > 1809f5c0190eafb6..a93772c10baaa003 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -627,7 +627,7 @@ static int rvin_setup(struct rvin_dev *vin) > /* Default to TB */ > vnmc = VNMC_IM_FULL; > /* Use BT if video standard can be read and is 60 Hz format */ > - if (vin->std & V4L2_STD_525_60) > + if (!vin->info->use_mc && vin->std & V4L2_STD_525_60) > vnmc = VNMC_IM_FULL | VNMC_FOC; > break; > case V4L2_FIELD_INTERLACED_TB: > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > dd835be0f9cbcc05..2280535ca981993f 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -18,12 +18,16 @@ > > #include > #include > +#include > #include > > #include "rcar-vin.h" > > #define RVIN_DEFAULT_FORMAT V4L2_PIX_FMT_YUYV > +#define RVIN_DEFAULT_WIDTH 800 > +#define RVIN_DEFAULT_HEIGHT 600 > #define RVIN_DEFAULT_FIELD V4L2_FIELD_NONE > +#define RVIN_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB > > /* > --- > -- * Format Conversions > @@ -656,6 +660,74 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = { > .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > }; > > +/* > --- > -- + * V4L2 Media Controller > + */ > + > +static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv, > +struct v4l2_format *f) > +{ > + struct rvin_dev *vin = video_drvdata(file); > + > + return rvin_format_align(vin, &f->fmt.pix); > +} > + > +static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv, > + struct v4l2_format *f) > +{ > + struct rvin_dev *vin = video_drvdata(file); > + int ret; > + > + if (vb2_is_busy(&vin->queue)) > + return -EBUSY; > + > + ret = rvin_format_align(vin, &f->fmt.pix); > + if (ret) > + return ret; > + > + vin->format = f->fmt.pix; > + > + return 0; > +} > + > +static int rvin_mc_enum_input(struct file *file, void *priv, > + struct v4l2_input *i) > +{ > + if (i->index != 0) > + return -EINVAL; > + > + i->type = V4L2_INPUT_TYPE_CAMERA; > + strlcpy(i->name, "Camera", sizeof(i->name)); > + > + return 0; > +} > + > +static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = { > + .vidioc_querycap= rvin_querycap, > + .vidioc_try_fmt_vid_cap = rvin_mc_try_fmt_vid_cap, > + .vidioc_g_fmt_vid_cap = rvin_g_fmt_vid_cap, > + .vidioc_s_fmt_vid_cap = rvin_mc_s_fmt_vid_cap, > + .vidioc_enum_fmt_vid_cap= rvin_enum_fmt_vid_cap, > + > + .vidioc_enum_input = rvin_mc_enum_input, > + .vidioc_g_input = rvin_g_input, > + .vidioc_s_input = rvin_s_input, > + > + .vidioc_reqbufs = vb2_ioctl_reqbufs, > + .vidioc_create_bufs = vb2_ioctl_create_bufs, > + .vidioc_querybuf= vb2_ioctl_querybuf, > + .vidioc_qbuf= vb2_ioctl_qbuf, > + .vidioc_dqbuf = vb2_ioctl_dqbuf, > + .vidioc_expbuf = vb2_ioctl_expbuf, > + .vidioc_prepare_buf = vb2_ioctl_prepare_buf, > + .vidioc_streamon= vb2_ioctl_streamon, > + .vidioc_streamoff = vb2_ioctl_streamoff, > + > + .vidioc_log_status = v4l2_ctrl_log_status, > + .vidioc_subscribe_event = rvin_subscribe_event, > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > +}; > + > /* > --- > -- * File Operations > */ > @@ -799,6
Re: [PATCH v13 15/33] rcar-vin: break out format alignment and checking
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:38 EEST Niklas Söderlund wrote: > Part of the format alignment and checking can be shared with the Gen3 > format handling. Break that part out to a separate function. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 85 +++-- > 1 file changed, 48 insertions(+), 37 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > 8dbd764883976ad1..c39891386576afb8 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -87,6 +87,53 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format > *pix) return pix->bytesperline * pix->height; > } > > +static int rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format > *pix) > +{ > + u32 walign; > + > + if (!rvin_format_from_pixel(pix->pixelformat) || > + (vin->info->model == RCAR_M1 && > + pix->pixelformat == V4L2_PIX_FMT_XBGR32)) > + pix->pixelformat = RVIN_DEFAULT_FORMAT; > + > + switch (pix->field) { > + case V4L2_FIELD_TOP: > + case V4L2_FIELD_BOTTOM: > + case V4L2_FIELD_NONE: > + case V4L2_FIELD_INTERLACED_TB: > + case V4L2_FIELD_INTERLACED_BT: > + case V4L2_FIELD_INTERLACED: > + break; > + case V4L2_FIELD_ALTERNATE: > + /* > + * Driver dose not (yet) support outputting ALTERNATE to a s/dose/does/ Apart from that, Reviewed-by: Laurent Pinchart > + * userspace. It does support outputting INTERLACED so use > + * the VIN hardware to combine the two fields. > + */ > + pix->field = V4L2_FIELD_INTERLACED; > + pix->height *= 2; > + break; > + default: > + pix->field = RVIN_DEFAULT_FIELD; > + break; > + } > + > + /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */ > + walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1; > + > + /* Limit to VIN capabilities */ > + v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign, > + &pix->height, 4, vin->info->max_height, 2, 0); > + > + pix->bytesperline = rvin_format_bytesperline(pix); > + pix->sizeimage = rvin_format_sizeimage(pix); > + > + vin_dbg(vin, "Format %ux%u bpl: %u size: %u\n", > + pix->width, pix->height, pix->bytesperline, pix->sizeimage); > + > + return 0; > +} > + > /* > --- > -- * V4L2 > */ > @@ -186,7 +233,6 @@ static int __rvin_try_format(struct rvin_dev *vin, >struct v4l2_pix_format *pix, >struct rvin_source_fmt *source) > { > - u32 walign; > int ret; > > if (!rvin_format_from_pixel(pix->pixelformat) || > @@ -199,42 +245,7 @@ static int __rvin_try_format(struct rvin_dev *vin, > if (ret) > return ret; > > - switch (pix->field) { > - case V4L2_FIELD_TOP: > - case V4L2_FIELD_BOTTOM: > - case V4L2_FIELD_NONE: > - case V4L2_FIELD_INTERLACED_TB: > - case V4L2_FIELD_INTERLACED_BT: > - case V4L2_FIELD_INTERLACED: > - break; > - case V4L2_FIELD_ALTERNATE: > - /* > - * Driver dose not (yet) support outputting ALTERNATE to a > - * userspace. It does support outputting INTERLACED so use > - * the VIN hardware to combine the two fields. > - */ > - pix->field = V4L2_FIELD_INTERLACED; > - pix->height *= 2; > - break; > - default: > - pix->field = RVIN_DEFAULT_FIELD; > - break; > - } > - > - /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */ > - walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1; > - > - /* Limit to VIN capabilities */ > - v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign, > - &pix->height, 4, vin->info->max_height, 2, 0); > - > - pix->bytesperline = rvin_format_bytesperline(pix); > - pix->sizeimage = rvin_format_sizeimage(pix); > - > - vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n", > - pix->width, pix->height, pix->bytesperline, pix->sizeimage); > - > - return 0; > + return rvin_format_align(vin, pix); > } > > static int rvin_querycap(struct file *file, void *priv, -- Regards, Laurent Pinchart
Re: [PATCH v13 12/33] rcar-vin: fix handling of single field frames (top, bottom and alternate fields)
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:35 EEST Niklas Söderlund wrote: > There was never proper support in the VIN driver to deliver ALTERNATING > field format to user-space, remove this field option. The problem is > that ALTERNATING field order requires the sequence numbers of buffers > returned to userspace to reflect if fields were dropped or not, > something which is not possible with the VIN drivers capture logic. > > The VIN driver can still capture from a video source which delivers > frames in ALTERNATING field order, but needs to combine them using the > VIN hardware into INTERLACED field order. Before this change if a source > was delivering fields using ALTERNATE the driver would default to > combining them using this hardware feature. Only if the user explicitly > requested ALTERNATE field order would incorrect frames be delivered. > > The height should not be cut in half for the format for TOP or BOTTOM > fields settings. This was a mistake and it was made visible by the > scaling refactoring. Correct behavior is that the user should request a > frame size that fits the half height frame reflected in the field > setting. If not the VIN will do its best to scale the top or bottom to > the requested format and cropping and scaling do not work as expected. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil > > --- > > * Changes since v12 > - Spelling where -> were. > - Add review tag from Hans. > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 15 +-- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 40 +++--- > 2 files changed, 10 insertions(+), 45 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c index > 4f48575f2008fe34..9233924e5b52de5f 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -617,7 +617,6 @@ static int rvin_setup(struct rvin_dev *vin) > case V4L2_FIELD_INTERLACED_BT: > vnmc = VNMC_IM_FULL | VNMC_FOC; > break; > - case V4L2_FIELD_ALTERNATE: > case V4L2_FIELD_NONE: > vnmc = VNMC_IM_ODD_EVEN; > progressive = true; > @@ -745,18 +744,6 @@ static bool rvin_capture_active(struct rvin_dev *vin) > return rvin_read(vin, VNMS_REG) & VNMS_CA; > } > > -static enum v4l2_field rvin_get_active_field(struct rvin_dev *vin, u32 > vnms) > -{ > - if (vin->format.field == V4L2_FIELD_ALTERNATE) { > - /* If FS is set it's a Even field */ > - if (vnms & VNMS_FS) > - return V4L2_FIELD_BOTTOM; > - return V4L2_FIELD_TOP; > - } > - > - return vin->format.field; > -} > - > static void rvin_set_slot_addr(struct rvin_dev *vin, int slot, dma_addr_t > addr) { > const struct rvin_video_format *fmt; > @@ -892,7 +879,7 @@ static irqreturn_t rvin_irq(int irq, void *data) > > /* Capture frame */ > if (vin->queue_buf[slot]) { > - vin->queue_buf[slot]->field = rvin_get_active_field(vin, vnms); > + vin->queue_buf[slot]->field = vin->format.field; > vin->queue_buf[slot]->sequence = vin->sequence; > vin->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns(); > vb2_buffer_done(&vin->queue_buf[slot]->vb2_buf, > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > 16e895657c3f51c5..e956a385cc13f5f3 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -121,33 +121,6 @@ static int rvin_reset_format(struct rvin_dev *vin) > vin->format.colorspace = mf->colorspace; > vin->format.field = mf->field; > > - /* > - * If the subdevice uses ALTERNATE field mode and G_STD is > - * implemented use the VIN HW to combine the two fields to > - * one INTERLACED frame. The ALTERNATE field mode can still > - * be requested in S_FMT and be respected, this is just the > - * default which is applied at probing or when S_STD is called. > - */ > - if (vin->format.field == V4L2_FIELD_ALTERNATE && > - v4l2_subdev_has_op(vin_to_source(vin), video, g_std)) > - vin->format.field = V4L2_FIELD_INTERLACED; > - > - switch (vin->format.field) { > - case V4L2_FIELD_TOP: > - case V4L2_FIELD_BOTTOM: > - case V4L2_FIELD_ALTERNATE: > - vin->format.height /= 2; > - break; > - case V4L2_FIELD_NONE: > - case V4L2_FIELD_INTERLACED_TB: > - case V4L2_FIELD_INTERLACED_BT: > - case V4L2_FIELD_INTERLACED: > - break; > - default: > - vin->format.field = RVIN_DEFAULT_FIELD; > - break; > - } > - > rvin_reset_crop_compose(vin); > > vin->format.bytesperline = rvin_format_bytesperline(&vin->format); > @@ -235,15 +208,20 @@ static i
Re: [PATCH v13 16/33] rcar-vin: simplify how formats are set and reset
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:39 EEST Niklas Söderlund wrote: > With the recent cleanup of the format code to prepare for Gen3 it's > possible to simplify the Gen2 format code path as well. Clean up the > process by defining two functions to handle the set format and reset of > format when the standard is changed. > > While at it replace the driver local struct rvin_source_fmt with a > struct v4l2_rect as all it's used for is keep track of the source > dimensions. I wonder whether we should introduce v4l2_size (or ) when all we need is width and height, as v4l2_rect stores the top and left offsets too. This doesn't have to be fixed here though. > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil > > --- > > * Changes since v11 > - This patch where 'rcar-vin: read subdevice format for crop only when > needed' > - Keep caching the source dimensions and drop all changes to > rvin_g_selection() and rvin_s_selection(). > - Inline rvin_get_vin_format_from_source() into rvin_reset_format() > which now is the only user left. > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 116 +++-- > drivers/media/platform/rcar-vin/rcar-vin.h | 14 +--- > 2 files changed, 52 insertions(+), 78 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > c39891386576afb8..c4be0bcb8b16f941 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -138,67 +138,60 @@ static int rvin_format_align(struct rvin_dev *vin, > struct v4l2_pix_format *pix) * V4L2 > */ > > -static void rvin_reset_crop_compose(struct rvin_dev *vin) > -{ > - vin->crop.top = vin->crop.left = 0; > - vin->crop.width = vin->source.width; > - vin->crop.height = vin->source.height; > - > - vin->compose.top = vin->compose.left = 0; > - vin->compose.width = vin->format.width; > - vin->compose.height = vin->format.height; > -} > - > static int rvin_reset_format(struct rvin_dev *vin) > { > struct v4l2_subdev_format fmt = { > .which = V4L2_SUBDEV_FORMAT_ACTIVE, > + .pad = vin->digital->source_pad, > }; > - struct v4l2_mbus_framefmt *mf = &fmt.format; > int ret; > > - fmt.pad = vin->digital->source_pad; > - > ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt); > if (ret) > return ret; > > - vin->format.width = mf->width; > - vin->format.height = mf->height; > - vin->format.colorspace = mf->colorspace; > - vin->format.field = mf->field; > + v4l2_fill_pix_format(&vin->format, &fmt.format); > > - rvin_reset_crop_compose(vin); > + ret = rvin_format_align(vin, &vin->format); > + if (ret) > + return ret; rvin_format_align() always returns 0 so you can remove the error check. You can actually turn the function into a void function. > - vin->format.bytesperline = rvin_format_bytesperline(&vin->format); > - vin->format.sizeimage = rvin_format_sizeimage(&vin->format); > + vin->source.top = vin->crop.top = 0; > + vin->source.left = vin->crop.left = 0; > + vin->source.width = vin->crop.width = vin->format.width; > + vin->source.height = vin->crop.height = vin->format.height; I find multiple assignations on the same line hard to read. How about vin->source.top = 0; vin->source.left = 0; vin->source.width = vin->format.width; vin->source.height = vin->format.height; vin->crop = vin->source; vin->compose = vin->source; > + vin->compose.top = vin->compose.left = 0; > + vin->compose.width = vin->format.width; > + vin->compose.height = vin->format.height; > > return 0; > } I like the new rvin_reset_format(), it's much simpler. > -static int __rvin_try_format_source(struct rvin_dev *vin, > - u32 which, > - struct v4l2_pix_format *pix, > - struct rvin_source_fmt *source) > +static int rvin_try_format(struct rvin_dev *vin, u32 which, > +struct v4l2_pix_format *pix, > +struct v4l2_rect *crop, struct v4l2_rect *compose) > { > - struct v4l2_subdev *sd; > + struct v4l2_subdev *sd = vin_to_source(vin); > struct v4l2_subdev_pad_config *pad_cfg; > struct v4l2_subdev_format format = { > .which = which, > + .pad = vin->digital->source_pad, > }; > enum v4l2_field field; > u32 width, height; > int ret; > > - sd = vin_to_source(vin); > - > - v4l2_fill_mbus_format(&format.format, pix, vin->digital->code); > - > pad_cfg = v4l2_subdev_alloc_pad_config(sd); > if (pad_cfg == NULL) > return -ENOMEM; > > - format.pad = vin->digital->source_pad; > + if (!rvin_forma
Re: [PATCH v13 17/33] rcar-vin: cache video standard
Hi Niklas, Thank you for the patch. On Tuesday, 27 March 2018 00:44:40 EEST Niklas Söderlund wrote: > At stream on time the driver should not query the subdevice for which > standard are used. Instead it should be cached when userspace sets the > standard and used at stream on time. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil Reviewed-by: Laurent Pinchart > --- > drivers/media/platform/rcar-vin/rcar-core.c | 6 ++ > drivers/media/platform/rcar-vin/rcar-dma.c | 7 ++- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 10 -- > drivers/media/platform/rcar-vin/rcar-vin.h | 2 ++ > 4 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c > b/drivers/media/platform/rcar-vin/rcar-core.c index > be49d8968f0a0cef..8c251687e81b345b 100644 > --- a/drivers/media/platform/rcar-vin/rcar-core.c > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > @@ -96,6 +96,12 @@ static int rvin_digital_subdevice_attach(struct rvin_dev > *vin, if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) > return ret; > > + /* Read standard */ > + vin->std = V4L2_STD_UNKNOWN; > + ret = v4l2_subdev_call(subdev, video, g_std, &vin->std); > + if (ret < 0 && ret != -ENOIOCTLCMD) > + return ret; > + > /* Add the controls */ > ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 16); > if (ret < 0) > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c index > 9233924e5b52de5f..79f4074b931b5aeb 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -592,7 +592,6 @@ void rvin_crop_scale_comp(struct rvin_dev *vin) > static int rvin_setup(struct rvin_dev *vin) > { > u32 vnmc, dmr, dmr2, interrupts; > - v4l2_std_id std; > bool progressive = false, output_is_yuv = false, input_is_yuv = false; > > switch (vin->format.field) { > @@ -606,10 +605,8 @@ static int rvin_setup(struct rvin_dev *vin) > /* Default to TB */ > vnmc = VNMC_IM_FULL; > /* Use BT if video standard can be read and is 60 Hz format */ > - if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) { > - if (std & V4L2_STD_525_60) > - vnmc = VNMC_IM_FULL | VNMC_FOC; > - } > + if (vin->std & V4L2_STD_525_60) > + vnmc = VNMC_IM_FULL | VNMC_FOC; > break; > case V4L2_FIELD_INTERLACED_TB: > vnmc = VNMC_IM_FULL; > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > c4be0bcb8b16f941..43370c57d4b6239a 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -477,6 +477,8 @@ static int rvin_s_std(struct file *file, void *priv, > v4l2_std_id a) if (ret < 0) > return ret; > > + vin->std = a; > + > /* Changing the standard will change the width/height */ > return rvin_reset_format(vin); > } > @@ -484,9 +486,13 @@ static int rvin_s_std(struct file *file, void *priv, > v4l2_std_id a) static int rvin_g_std(struct file *file, void *priv, > v4l2_std_id *a) { > struct rvin_dev *vin = video_drvdata(file); > - struct v4l2_subdev *sd = vin_to_source(vin); > > - return v4l2_subdev_call(sd, video, g_std, a); > + if (v4l2_subdev_has_op(vin_to_source(vin), pad, dv_timings_cap)) > + return -ENOIOCTLCMD; > + > + *a = vin->std; > + > + return 0; > } > > static int rvin_subscribe_event(struct v4l2_fh *fh, > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h > b/drivers/media/platform/rcar-vin/rcar-vin.h index > e940366d7e8d0e76..06cec4f8e5ffaf2b 100644 > --- a/drivers/media/platform/rcar-vin/rcar-vin.h > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h > @@ -118,6 +118,7 @@ struct rvin_info { > * @crop:active cropping > * @compose: active composing > * @source: active size of the video source > + * @std: active video standard of the video source > */ > struct rvin_dev { > struct device *dev; > @@ -146,6 +147,7 @@ struct rvin_dev { > struct v4l2_rect crop; > struct v4l2_rect compose; > struct v4l2_rect source; > + v4l2_std_id std; > }; > > #define vin_to_source(vin) ((vin)->digital->subdev) -- Regards, Laurent Pinchart
[RFC PATCH]: intel-ipu3: Add uAPI documentation
This is a preliminary effort to add documentation for the following BNR(bayer noise reduction) structs: ipu3_uapi_bnr_static_config_wb_gains_config ipu3_uapi_bnr_static_config_wb_gains_thr_config ipu3_uapi_bnr_static_config_thr_coeffs_config ipu3_uapi_bnr_static_config_thr_ctrl_shd_config ipu3_uapi_bnr_static_config_opt_center_sqr_config ipu3_uapi_bnr_static_config_bp_ctrl_config ipu3_uapi_bnr_static_config_dn_detect_ctrl_config ipu3_uapi_bnr_static_config_opt_center_sqr_config ipu3_uapi_bnr_static_config_green_disparity The feedback on this patch will be used towards the documentation of reset of the uAPI in intel-ipu3.h. Link to v6 IPU3 ImgU patchset: https://patchwork.kernel.org/patch/10316725/> Signed-off-by: Yong Zhi Signed-off-by: Chao C Li --- Documentation/media/media_uapi.rst |1 + Documentation/media/uapi/intel-ipu3.rst |8 + include/uapi/linux/intel-ipu3.h | 1520 +++ 3 files changed, 1529 insertions(+) create mode 100644 Documentation/media/uapi/intel-ipu3.rst create mode 100644 include/uapi/linux/intel-ipu3.h diff --git a/Documentation/media/media_uapi.rst b/Documentation/media/media_uapi.rst index 28eb35a1f965..edfa674b49c3 100644 --- a/Documentation/media/media_uapi.rst +++ b/Documentation/media/media_uapi.rst @@ -31,3 +31,4 @@ License". uapi/cec/cec-api uapi/gen-errors uapi/fdl-appendix +uapi/intel-ipu3 diff --git a/Documentation/media/uapi/intel-ipu3.rst b/Documentation/media/uapi/intel-ipu3.rst new file mode 100644 index ..d4d9b2806fe9 --- /dev/null +++ b/Documentation/media/uapi/intel-ipu3.rst @@ -0,0 +1,8 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _intel-ipu3: + +Intel IPU3 ImgU uAPI headers + + +.. kernel-doc:: include/uapi/linux/intel-ipu3.h diff --git a/include/uapi/linux/intel-ipu3.h b/include/uapi/linux/intel-ipu3.h new file mode 100644 index ..34b071524beb --- /dev/null +++ b/include/uapi/linux/intel-ipu3.h @@ -0,0 +1,1520 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2018 Intel Corporation */ + +#ifndef __IPU3_UAPI_H +#define __IPU3_UAPI_H + +#include + +#define IPU3_UAPI_ISP_VEC_ELEMS64 + +#define IMGU_ABI_PAD __aligned(IPU3_UAPI_ISP_WORD_BYTES) +#define IPU3_ALIGN __attribute__((aligned(IPU3_UAPI_ISP_WORD_BYTES))) + +#define IPU3_UAPI_ISP_WORD_BYTES 32 +#define IPU3_UAPI_MAX_STRIPES 2 + +/*** ipu3_uapi_stats_3a ***/ + +#define IPU3_UAPI_MAX_BUBBLE_SIZE 10 + +#define IPU3_UAPI_AE_COLORS4 +#define IPU3_UAPI_AE_BINS 256 + +#define IPU3_UAPI_AWB_MD_ITEM_SIZE 8 +#define IPU3_UAPI_AWB_MAX_SETS 60 +#define IPU3_UAPI_AWB_SET_SIZE 0x500 +#define IPU3_UAPI_AWB_SPARE_FOR_BUBBLES \ + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \ +IPU3_UAPI_AWB_MD_ITEM_SIZE) +#define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \ + (IPU3_UAPI_AWB_MAX_SETS * \ +(IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) + +#define IPU3_UAPI_AF_MAX_SETS 24 +#define IPU3_UAPI_AF_MD_ITEM_SIZE 4 +#define IPU3_UAPI_AF_SPARE_FOR_BUBBLES \ + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \ +IPU3_UAPI_AF_MD_ITEM_SIZE) +#define IPU3_UAPI_AF_Y_TABLE_SET_SIZE 0x80 +#define IPU3_UAPI_AF_Y_TABLE_MAX_SIZE \ + (IPU3_UAPI_AF_MAX_SETS * \ +(IPU3_UAPI_AF_Y_TABLE_SET_SIZE + IPU3_UAPI_AF_SPARE_FOR_BUBBLES) * \ +IPU3_UAPI_MAX_STRIPES) + +#define IPU3_UAPI_AWB_FR_MAX_SETS 24 +#define IPU3_UAPI_AWB_FR_MD_ITEM_SIZE 8 +#define IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE0x100 +#define IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES \ + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \ +IPU3_UAPI_AWB_FR_MD_ITEM_SIZE) +#define IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE \ + (IPU3_UAPI_AWB_FR_MAX_SETS * \ + (IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE + \ +IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES) * IPU3_UAPI_MAX_STRIPES) + +struct ipu3_uapi_grid_config { + __u8 width; /* 6 or 7 (rgbs_grd_cfg) bits */ + __u8 height; + __u16 block_width_log2:3; + __u16 block_height_log2:3; + __u16 height_per_slice:8; /* default value 1 */ + __u16 x_start; /* 12 bits */ + __u16 y_start; +#define IPU3_UAPI_GRID_START_MASK ((1 << 12) - 1) +#define IPU3_UAPI_GRID_Y_START_EN (1 << 15) + __u16 x_end;/* 12 bits */ + __u16 y_end; +} __packed; + +struct ipu3_uapi_awb_meta_data { + __u8 meta_data_buffer[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]; +} __packed; + +struct ipu3_uapi_awb_raw_buffer { +
RE: [PATCH v4 00/12] Intel IPU3 ImgU patchset
Hi Mauro and all, > > Subject: RE: [PATCH v4 00/12] Intel IPU3 ImgU patchset > > > > Hi Mauro, > > > > > > > Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset > > > > > > > > > > Hi, > > > > > > > > > > Em Fri, 17 Nov 2017 02:58:56 + "Mani, Rajmohan" > > > > > escreveu: > > > > > > > > > > > Here is an update on the IPU3 documentation that we are > > > > > > currently working > > > > > on. > > > > > > > > > > > > Image processing in IPU3 relies on the following. > > > > > > > > > > > > 1) HW configuration to enable ISP and > > > > > > 2) setting customer specific 3A Tuning / Algorithm Parameters > > > > > > to achieve > > > > > desired image quality. > > > > > > > > > > > > We intend to provide documentation on ImgU driver programming > > > > > > interface > > > > > to help users of this driver to configure and enable ISP HW to > > > > > meet their needs. This documentation will include details on > > > > > complete > > > > > V4L2 Kernel driver interface and IO-Control parameters, except > > > > > for the ISP internal algorithm and its parameters (which is > > > > > Intel proprietary > > IP). > > > > > > > > > > Sakari asked me to take a look on this thread, specifically on > > > > > this email. I took a look on the other e-mails from this thread > > > > > that are discussing about this IP block. > > > > > > > > > > I understand that Intel wants to keep their internal 3A > > > > > algorithm protected, just like other vendors protect their own > > > > > algos. It was never a requirement to open whatever algorithm are > > > > > used inside a hardware (or firmware). The only requirement is > > > > > that firmwares should be licensed with redistribution > > > > > permission, ideally merged at linux-firmware > > > > git tree. > > > > > > > > > > Yet, what I don't understand is why Intel also wants to also > > > > > protect the interface for such 3A hardware/firmware algorithm. > > > > > The parameters that are passed from an userspace application to > > > > > Intel ISP logic doesn't contain the algorithm itself. What's the > > > > > issue of documenting the meaning of each parameter? > > > > > > > > > > > > > Thanks for looking into this. > > > > > > > > To achieve improved image quality using IPU3, 3A (Auto White > > > > balance, Auto Focus and Auto Exposure) Tuning parameters specific > > > > to a given camera sensor module, are converted to Intel ISP > > > > algorithm parameters in user space camera HAL using AIC (Automatic > > > > ISP > > Configuration) library. > > > > > > > > As a unique design of Intel ISP, it exposes very detailed > > > > algorithm parameters (~ 1 parameters) to configure ISP's image > > > > processing algorithm per each image fame in runtime. Typical > > > > Camera SW developers (including those at > > > > Intel) are not expected to fully understand and directly set these > > > > parameters to configure the ISP algorithm blocks. Due to the > > > > above, a user space AIC library (in binary form) is provided to > > > > generate ISP Algorithm specific parameters, for a given set of 3A > > > > tuning parameters. It significantly reduces the efforts of SW > > > > development in ISP HW > > > configuration. > > > > > > > > On the other hand, the ISP algorithm details could be deduced > > > > readily through these detailed parameters by other ISP experts > > > > outside > > Intel. > > > > This is the reason that we want to keep these parameter > > > > definitions as Intel > > > proprietary IP. > > > > > > > > We are fully aware of your concerns on how to enable open source > > > > developers to use Intel ISP through up-streamed Kernel Driver. > > > > Internally, we are working on the license for this AIC library > > > > release now (as Hans said NDA license is not acceptable). We > > > > believe this will be more efficient way to help open source developers. > > > > > > > > This AIC library release would be a binary-only release. This AIC > > > > library does not use any kernel uAPIs directly. The user space > > > > Camera HAL that uses kernel uAPIs is available at > > > > https://chromium.googlesource.com/chromiumos/platform/arc- > > > > camera/+/master > > > > > > > > The AIC library (in binary form) is available here. > > https://storage.googleapis.com/chromeos-localmirror/distfiles/intel-3a > > -libs- > > bin-2017.09.27.tbz2 > > > > Licensing information can be found in ./LICENSE.intel_3a_library file > > after unzipping the tar file. > > > > > > > > Just pinging to know your thoughts on this. > > > To follow up further on this, we have posted the preliminary documentation on bnr (bayer noise reduction) parameters as below. https://www.mail-archive.com/linux-media@vger.kernel.org/msg128683.html We would like hear your early feedback and apply the same towards the documentation of remaining IPU3 parameters. Thanks Raj
cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Apr 4 05:00:17 CEST 2018 media-tree git hash:6ccd228e0cfce2a4f44558422d25c60fcb1a6710 media_build git hash: 541653bb52fcf7c34b8b83a4c8cc66b09c68ac37 v4l-utils git hash: 65d4f9242cede9f6cdc0ee86dabae31cee552dee gcc version:i686-linux-gcc (GCC) 7.3.0 sparse version: v0.5.0-3994-g45eb2282 smatch version: v0.5.0-3994-g45eb2282 host hardware: x86_64 host os:4.14.0-3-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.14.31-i686: OK linux-4.14.31-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html