[PATCH v2 0/2] rcar-vin: always run in continues mode
Hi, This series reworks the R-Car VIN driver to only run using its continues capture mode. This improves performance a lot when userspace struggles to keep up and queue buffers as fast as the VIN driver consumes them. The solution to always be able to run in continues is to introduce a scratch buffer inside the VIN driver which it can pad the hardware capture buffer ring with if it have no buffer from userspace. Using this scratch buffer allows the driver to not need to stop capturing when it run out of buffers and then restart it once it have more buffers. Patch 1/2 adds the allocation of the scratch buffer. And patch 2/2 drops the single capture mode in favor of always running in continues capture mode and the scratch buffer. The series is based on top of the latest media-tree master branch and can be fetched from. git://git.ragnatech.se/linux v4l2/next/vin/mode-v2 It is tested on R-Car Koelsch Gen2 board using the onboard HDMI and CVBS inputs. The test application v4l2-compliance pass for both inputs without issues or warnings. A slight adaption of these patches to the pending VIN Gen3 patches have been tested with great improvement in capture speed for buffer strained situations and no regressions in the vin-tests suite. * Changes since v1 - Dropped first patch in series as it removed a correct check due to my poor reading skills. - Corrected spelling in commit messages and comments. - Added review tags from Jacopo and Kieran, thanks! Niklas Söderlund (2): rcar-vin: allocate a scratch buffer at stream start rcar-vin: use scratch buffer and always run in continuous mode drivers/media/platform/rcar-vin/rcar-dma.c | 206 ++--- drivers/media/platform/rcar-vin/rcar-vin.h | 10 +- 2 files changed, 75 insertions(+), 141 deletions(-) -- 2.16.2
[PATCH v2 2/2] rcar-vin: use scratch buffer and always run in continuous mode
Instead of switching capture mode depending on how many buffers are available use a scratch buffer and always run in continuous mode. By using a scratch buffer the responsiveness of the capture loop is increased as it can keep running even if there are no buffers available from userspace. As soon as a userspace queues a buffer it is inserted into the capture loop and returned as soon as it is filled. This is a improvement on the previous logic where the whole capture loop was stopped and switched to single capture mode if userspace did not feed the VIN driver buffers at the same time it consumed them. To make matters worse it was difficult for the driver to reenter continuous mode if it entered single mode even if userspace started to queue buffers faster. This resulted in suboptimal performance where if userspace where delayed for a short period the ongoing capture would be slowed down and run in single mode until the capturing process where restarted. An additional effect of this change is that the capture logic can be made much simple as we know that continuous mode will always be used. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- drivers/media/platform/rcar-vin/rcar-dma.c | 187 - drivers/media/platform/rcar-vin/rcar-vin.h | 6 +- 2 files changed, 52 insertions(+), 141 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index 1f91b056188ebdb3..4a40e6ad1be7ed95 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -168,12 +168,8 @@ static int rvin_setup(struct rvin_dev *vin) break; case V4L2_FIELD_ALTERNATE: case V4L2_FIELD_NONE: - if (vin->continuous) { - vnmc = VNMC_IM_ODD_EVEN; - progressive = true; - } else { - vnmc = VNMC_IM_ODD; - } + vnmc = VNMC_IM_ODD_EVEN; + progressive = true; break; default: vnmc = VNMC_IM_ODD; @@ -298,14 +294,6 @@ static bool rvin_capture_active(struct rvin_dev *vin) return rvin_read(vin, VNMS_REG) & VNMS_CA; } -static int rvin_get_active_slot(struct rvin_dev *vin, u32 vnms) -{ - if (vin->continuous) - return (vnms & VNMS_FBS_MASK) >> VNMS_FBS_SHIFT; - - return 0; -} - static enum v4l2_field rvin_get_active_field(struct rvin_dev *vin, u32 vnms) { if (vin->format.field == V4L2_FIELD_ALTERNATE) { @@ -344,76 +332,47 @@ static void rvin_set_slot_addr(struct rvin_dev *vin, int slot, dma_addr_t addr) rvin_write(vin, offset, VNMB_REG(slot)); } -/* Moves a buffer from the queue to the HW slots */ -static bool rvin_fill_hw_slot(struct rvin_dev *vin, int slot) +/* + * Moves a buffer from the queue to the HW slot. If no buffer is + * available use the scratch buffer. The scratch buffer is never + * returned to userspace, its only function is to enable the capture + * loop to keep running. + */ +static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot) { struct rvin_buffer *buf; struct vb2_v4l2_buffer *vbuf; - dma_addr_t phys_addr_top; - - if (vin->queue_buf[slot] != NULL) - return true; + dma_addr_t phys_addr; - if (list_empty(&vin->buf_list)) - return false; + /* A already populated slot shall never be overwritten. */ + if (WARN_ON(vin->queue_buf[slot] != NULL)) + return; vin_dbg(vin, "Filling HW slot: %d\n", slot); - /* Keep track of buffer we give to HW */ - buf = list_entry(vin->buf_list.next, struct rvin_buffer, list); - vbuf = &buf->vb; - list_del_init(to_buf_list(vbuf)); - vin->queue_buf[slot] = vbuf; - - /* Setup DMA */ - phys_addr_top = vb2_dma_contig_plane_dma_addr(&vbuf->vb2_buf, 0); - rvin_set_slot_addr(vin, slot, phys_addr_top); - - return true; -} - -static bool rvin_fill_hw(struct rvin_dev *vin) -{ - int slot, limit; - - limit = vin->continuous ? HW_BUFFER_NUM : 1; - - for (slot = 0; slot < limit; slot++) - if (!rvin_fill_hw_slot(vin, slot)) - return false; - return true; -} - -static void rvin_capture_on(struct rvin_dev *vin) -{ - vin_dbg(vin, "Capture on in %s mode\n", - vin->continuous ? "continuous" : "single"); + if (list_empty(&vin->buf_list)) { + vin->queue_buf[slot] = NULL; + phys_addr = vin->scratch_phys; + } else { + /* Keep track of buffer we give to HW */ + buf = list_entry(vin->buf_list.next, struct rvin_buffer, list); + vbuf = &buf->vb; + list_del_init(to_buf_list(vbuf)); + vin->queue_buf[slot] = vbuf; + + /* Setup DMA */ + phys_addr = vb2_dma_contig
[PATCH v2 1/2] rcar-vin: allocate a scratch buffer at stream start
Before starting a capture, allocate a scratch buffer which can be used by the driver to give to the hardware if no buffers are available from userspace. The buffer is not used in this patch but prepares for future refactoring where the scratch buffer can be used to avoid the need to fallback on single capture mode if userspace can't queue buffers as fast as the VIN driver consumes them. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Kieran Bingham --- drivers/media/platform/rcar-vin/rcar-dma.c | 19 +++ drivers/media/platform/rcar-vin/rcar-vin.h | 4 2 files changed, 23 insertions(+) diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index 23fdff7a7370842e..1f91b056188ebdb3 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -1076,6 +1076,17 @@ static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count) unsigned long flags; int ret; + /* Allocate scratch buffer. */ + vin->scratch = dma_alloc_coherent(vin->dev, vin->format.sizeimage, + &vin->scratch_phys, GFP_KERNEL); + if (!vin->scratch) { + spin_lock_irqsave(&vin->qlock, flags); + return_all_buffers(vin, VB2_BUF_STATE_QUEUED); + spin_unlock_irqrestore(&vin->qlock, flags); + vin_err(vin, "Failed to allocate scratch buffer\n"); + return -ENOMEM; + } + sd = vin_to_source(vin); v4l2_subdev_call(sd, video, s_stream, 1); @@ -1091,6 +1102,10 @@ static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count) spin_unlock_irqrestore(&vin->qlock, flags); + if (ret) + dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, + vin->scratch_phys); + return ret; } @@ -1141,6 +1156,10 @@ static void rvin_stop_streaming(struct vb2_queue *vq) /* disable interrupts */ rvin_disable_interrupts(vin); + + /* Free scratch buffer. */ + dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, + vin->scratch_phys); } static const struct vb2_ops rvin_qops = { diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h index 5382078143fb3869..00b405f78d0912c9 100644 --- a/drivers/media/platform/rcar-vin/rcar-vin.h +++ b/drivers/media/platform/rcar-vin/rcar-vin.h @@ -102,6 +102,8 @@ struct rvin_graph_entity { * * @lock: protects @queue * @queue: vb2 buffers queue + * @scratch: cpu address for scratch buffer + * @scratch_phys: physical address of the scratch buffer * * @qlock: protects @queue_buf, @buf_list, @continuous, @sequence * @state @@ -130,6 +132,8 @@ struct rvin_dev { struct mutex lock; struct vb2_queue queue; + void *scratch; + dma_addr_t scratch_phys; spinlock_t qlock; struct vb2_v4l2_buffer *queue_buf[HW_BUFFER_NUM]; -- 2.16.2
Re: [PATCH 02/11] media: vsp1: use kernel __packed for structures
Hi David, Just for reference here, I've posted a v2 of this patch now titled: [PATCH v2 02/11] media: vsp1: Remove packed attributes from aligned structures which removes the attributes instead of modifying them. Thanks for the pointers! Regards Kieran On 13/03/18 15:03, Kieran Bingham wrote: > Hi David, > > On 13/03/18 13:38, David Laight wrote: >> From: Kieran Bingham [mailto:kieran.bingham+rene...@ideasonboard.com] >>> On 13/03/18 11:20, David Laight wrote: From: Kieran Bingham > Sent: 09 March 2018 22:04 > The kernel provides a __packed definition to abstract away from the > compiler specific attributes tag. > > Convert all packed structures in VSP1 to use it. > > Signed-off-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_dl.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index 37e2c984fbf3..382e45c2054e 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -29,19 +29,19 @@ > struct vsp1_dl_header_list { > u32 num_bytes; > u32 addr; > -} __attribute__((__packed__)); > +} __packed; > > struct vsp1_dl_header { > u32 num_lists; > struct vsp1_dl_header_list lists[8]; > u32 next_header; > u32 flags; > -} __attribute__((__packed__)); > +} __packed; > > struct vsp1_dl_entry { > u32 addr; > u32 data; > -} __attribute__((__packed__)); > +} __packed; Do these structures ever actually appear in misaligned memory? If they don't then they shouldn't be marked 'packed'. >>> >>> I believe the declaration is to ensure that the struct definition is not >>> altered >>> by the compiler as these structures specifically define the layout of how >>> the >>> memory is read by the VSP1 hardware. >> >> The C language and ABI define structure layouts. >> >>> Certainly 2 u32's sequentially stored in a struct are unlikely to be moved >>> or >>> rearranged by the compiler (though I believe it would be free to do so if it >>> chose without this attribute), but I think the declaration shows the intent >>> of >>> the memory structure. >> >> The language requires the fields be in order and the ABI stops the compiler >> adding 'random' padding. >> >>> Isn't this a common approach throughout the kernel when dealing with >>> hardware >>> defined memory structures ? >> >> Absolutely not. >> __packed tells the compiler that the structure might be on any address >> boundary. >> On many architectures this means the compiler must use byte accesses with >> shifts >> and ors for every access. >> The most a hardware defined structure will have is a compile-time assert >> that it is the correct size (to avoid silly errors from changes). > > > > Ok - interesting, I see what you're saying - and certainly don't want the > compiler to be performing byte accesses on the structures unnecessarily. > > I'm trying to distinguish the difference here. Is the single point that > __packed > > causes byte-access, where as > > __attribute__((__packed__)); > > does not? > > Looking at the GCC docs [0]: I see that __attribute__((__packed__)) tells the > compiler that the "structure or union is placed to minimize the memory > required". > > However, the keil compiler docs[1] do certainly declare that __packed causes > byte alignment. > > I was confused/thrown off here by the Kernel defining __packed as > __attribute__((packed)) at [2]. > > Do __attribute__((packed)) and __attribute__((__packed__)) differ ? > > In which case, from what I've read so far I wish "__packed" was > "__unaligned"... > > > [0] > https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-packed-type-attribute > > [1] http://www.keil.com/support/man/docs/armcc/armcc_chr1359124230195.htm > > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/compiler-gcc.h?h=v4.16-rc5#n92 > > > Regards > > Kieran > > >> David >>
Re: [PATCH v13 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
Hi Niklas Thank you for this patch ... I know it has been a lot of work getting this and the VIN together! On 13/02/18 00:01, Niklas Söderlund wrote: > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver > supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are > connected between the video sources and the video grabbers (VIN). > > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP. > > Signed-off-by: Niklas Söderlund > Reviewed-by: Hans Verkuil I don't think there's actually anything major in the below - so with any comments addressed, or specifically ignored you can add my: Reviewed-by: Kieran Bingham tag if you like. > --- > drivers/media/platform/rcar-vin/Kconfig | 12 + > drivers/media/platform/rcar-vin/Makefile| 1 + > drivers/media/platform/rcar-vin/rcar-csi2.c | 884 > > 3 files changed, 897 insertions(+) > create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c > > diff --git a/drivers/media/platform/rcar-vin/Kconfig > b/drivers/media/platform/rcar-vin/Kconfig > index af4c98b44d2e22cb..6875f30c1ae42631 100644 > --- a/drivers/media/platform/rcar-vin/Kconfig > +++ b/drivers/media/platform/rcar-vin/Kconfig > @@ -1,3 +1,15 @@ > +config VIDEO_RCAR_CSI2 > + tristate "R-Car MIPI CSI-2 Receiver" > + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF I think I recall recently seeing that depending upon OF is redundant and not necessary (though I think that's minor in this instance) > + depends on ARCH_RENESAS || COMPILE_TEST > + select V4L2_FWNODE > + ---help--- > + Support for Renesas R-Car MIPI CSI-2 receiver. > + Supports R-Car Gen3 SoCs. > + > + To compile this driver as a module, choose M here: the > + module will be called rcar-csi2. > + > config VIDEO_RCAR_VIN > tristate "R-Car Video Input (VIN) Driver" > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && > MEDIA_CONTROLLER > diff --git a/drivers/media/platform/rcar-vin/Makefile > b/drivers/media/platform/rcar-vin/Makefile > index 48c5632c21dc060b..5ab803d3e7c1aa57 100644 > --- a/drivers/media/platform/rcar-vin/Makefile > +++ b/drivers/media/platform/rcar-vin/Makefile > @@ -1,3 +1,4 @@ > rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o > > +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o > obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > b/drivers/media/platform/rcar-vin/rcar-csi2.c > new file mode 100644 > index ..c0c2a763151bc928 > --- /dev/null > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -0,0 +1,884 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Driver for Renesas R-Car MIPI CSI-2 Receiver > + * > + * Copyright (C) 2018 Renesas Electronics Corp. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +/* Register offsets and bits */ > + > +/* Control Timing Select */ > +#define TREF_REG 0x00 > +#define TREF_TREFBIT(0) > + > +/* Software Reset */ > +#define SRST_REG 0x04 > +#define SRST_SRSTBIT(0) > + > +/* PHY Operation Control */ > +#define PHYCNT_REG 0x08 > +#define PHYCNT_SHUTDOWNZ BIT(17) > +#define PHYCNT_RSTZ BIT(16) > +#define PHYCNT_ENABLECLK BIT(4) > +#define PHYCNT_ENABLE_3 BIT(3) > +#define PHYCNT_ENABLE_2 BIT(2) > +#define PHYCNT_ENABLE_1 BIT(1) > +#define PHYCNT_ENABLE_0 BIT(0) > + > +/* Checksum Control */ > +#define CHKSUM_REG 0x0c > +#define CHKSUM_ECC_ENBIT(1) > +#define CHKSUM_CRC_ENBIT(0) > + > +/* > + * Channel Data Type Select > + * VCDT[0-15]: Channel 1 VCDT[16-31]: Channel 2 > + * VCDT2[0-15]: Channel 3 VCDT2[16-31]: Channel 4 > + */ > +#define VCDT_REG 0x10 > +#define VCDT2_REG0x14 > +#define VCDT_VCDTN_ENBIT(15) > +#define VCDT_SEL_VC(n) (((n) & 0x3) << 8) > +#define VCDT_SEL_DTN_ON BIT(6) > +#define VCDT_SEL_DT(n) (((n) & 0x3f) << 0) > + > +/* Frame Data Type Select */ > +#define FRDT_REG 0x18 > + > +/* Field Detection Control */ > +#define FLD_REG 0x1c > +#define FLD_FLD_NUM(n) (((n) & 0xff) << 16) > +#define FLD_FLD_EN4 BIT(3) > +#define FLD_FLD_EN3 BIT(2) > +#define FLD_FLD_EN2 BIT(1) > +#define FLD_FLD_EN BIT(0) > + > +/* Automatic Standby Control */ > +#define ASTBY_REG0x20 > + > +/* Long Data Type Setting 0 */ >
Re: [Qemu-devel] [RFC PATCH] i2c: device passthrough HACK(!) & evaluation
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180313200245.2350-1-wsa+rene...@sang-engineering.com Subject: [Qemu-devel] [RFC PATCH] i2c: device passthrough HACK(!) & evaluation === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 7a638ffbc1 i2c: device passthrough HACK(!) & evaluation === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-nkgm8s_c/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora make[1]: Entering directory '/var/tmp/patchew-tester-tmp-nkgm8s_c/src' GEN /var/tmp/patchew-tester-tmp-nkgm8s_c/src/docker-src.2018-03-13-16.06.36.10128/qemu.tar Cloning into '/var/tmp/patchew-tester-tmp-nkgm8s_c/src/docker-src.2018-03-13-16.06.36.10128/qemu.tar.vroot'... done. Your branch is up-to-date with 'origin/test'. Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-nkgm8s_c/src/docker-src.2018-03-13-16.06.36.10128/qemu.tar.vroot/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-nkgm8s_c/src/docker-src.2018-03-13-16.06.36.10128/qemu.tar.vroot/ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' COPYRUNNER RUN test-mingw in qemu:fedora Packages installed: PyYAML-3.12-5.fc27.x86_64 SDL-devel-1.2.15-29.fc27.x86_64 bc-1.07.1-3.fc27.x86_64 bison-3.0.4-8.fc27.x86_64 bzip2-1.0.6-24.fc27.x86_64 ccache-3.3.6-1.fc27.x86_64 clang-5.0.1-3.fc27.x86_64 findutils-4.6.0-16.fc27.x86_64 flex-2.6.1-5.fc27.x86_64 gcc-7.3.1-5.fc27.x86_64 gcc-c++-7.3.1-5.fc27.x86_64 gettext-0.19.8.1-12.fc27.x86_64 git-2.14.3-3.fc27.x86_64 glib2-devel-2.54.3-2.fc27.x86_64 hostname-3.18-4.fc27.x86_64 libaio-devel-0.3.110-9.fc27.x86_64 libasan-7.3.1-5.fc27.x86_64 libfdt-devel-1.4.6-1.fc27.x86_64 libubsan-7.3.1-5.fc27.x86_64 llvm-5.0.1-3.fc27.x86_64 make-4.2.1-4.fc27.x86_64 mingw32-SDL-1.2.15-9.fc27.noarch mingw32-bzip2-1.0.6-9.fc27.noarch mingw32-curl-7.54.1-2.fc27.noarch mingw32-glib2-2.54.1-1.fc27.noarch mingw32-gmp-6.1.2-2.fc27.noarch mingw32-gnutls-3.5.13-2.fc27.noarch mingw32-gtk2-2.24.31-4.fc27.noarch mingw32-gtk3-3.22.16-1.fc27.noarch mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch mingw32-libpng-1.6.29-2.fc27.noarch mingw32-libssh2-1.8.0-3.fc27.noarch mingw32-libtasn1-4.13-1.fc27.noarch mingw32-nettle-3.3-3.fc27.noarch mingw32-pixman-0.34.0-3.fc27.noarch mingw32-pkg-config-0.28-9.fc27.x86_64 mingw64-SDL-1.2.15-9.fc27.noarch mingw64-bzip2-1.0.6-9.fc27.noarch mingw64-curl-7.54.1-2.fc27.noarch mingw64-glib2-2.54.1-1.fc27.noarch mingw64-gmp-6.1.2-2.fc27.noarch mingw64-gnutls-3.5.13-2.fc27.noarch mingw64-gtk2-2.24.31-4.fc27.noarch mingw64-gtk3-3.22.16-1.fc27.noarch mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch mingw64-libpng-1.6.29-2.fc27.noarch mingw64-libssh2-1.8.0-3.fc27.noarch mingw64-libtasn1-4.13-1.fc27.noarch mingw64-nettle-3.3-3.fc27.noarch mingw64-pixman-0.34.0-3.fc27.noarch mingw64-pkg-config-0.28-9.fc27.x86_64 nettle-devel-3.4-1.fc27.x86_64 perl-5.26.1-403.fc27.x86_64 pixman-devel-0.34.0-4.fc27.x86_64 python3-3.6.2-13.fc27.x86_64 sparse-0.5.1-2.fc27.x86_64 tar-1.29-7.fc27.x86_64 which-2.21-4.fc27.x86_64 zlib-devel-1.2.11-4.fc27.x86_64 Environment variables: TARGET_LIST= PACKAGES=ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel gcc gcc-c++ llvm clang make perl which bc findutils libaio-devel nettle-devel libasan libubsan mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 mingw32-bzip2 mingw64-pixman mingw64-glib2 mingw64-gmp mingw64-SDL mingw64-pkg-config mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 mingw64-bzip2 J=8 V= HOSTNAME=8acbc79569cc DEBUG= SHOW_ENV=1 PWD=/ HOME=/root CCACHE_DIR=/var/tmp/ccache DISTTAG=f27container QEMU_CONFIGURE_OPTS=--python=/usr/bin/python3 FGC=f27 TEST_DIR=/tmp/qemu-test SHLVL=1 FEATURES=mingw clang pyyaml asan dtc PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin MAKEFLAGS= -j8 EXTRA_CONFIGURE_OPTS= _=/usr/bin/env Configure
Re: [Qemu-devel] [PATCH 3/3] nvram: at24c: use a sane default for "rom-size"
Hi Philippe, > > static Property at24c_eeprom_props[] = { > > -DEFINE_PROP_UINT32("rom-size", EEPROMState, rsize, 0), > > +DEFINE_PROP_UINT32("rom-size", EEPROMState, rsize, 128), > > This patch should goes before your 2/3 in your series. I don't mind much, but why? My reasoning was "let's first fix the cause and then the symptom"? > Can you add a #define for this value? Such AT24C_ROMSIZE_MIN. Can do, of course. But won't that give room for regressions because people are already using it with lower values? Ideally, we would have a "model" variable. The model type would define the size of the memory. The "rom-size" variable could then be kept as is (except for the 0 bugfix) or deprecated? Thanks for the review, Wolfram signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 1/3] nvram: at24c: remove doubled prefix for ERR
> > -ERR(TYPE_AT24C_EE > > -" : failed to write backing file\n"); > > +ERR("failed to write backing file\n"); > > printf/fprintf are deprecated, since you are modifying this file can you > use a newer API, "qemu/error-report.h" for example. Sure, I'll have a look. > > } > > DPRINTK("Wrote to backing file\n"); > > DPRINTK() can be converted to tracepoint. I'd leave that for another incremental change. signature.asc Description: PGP signature
Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1
On Thu, Mar 01, 2018 at 11:20:11AM +0100, Geert Uytterhoeven wrote: > Hi Simon, > > On Fri, Feb 23, 2018 at 9:14 AM, Simon Horman wrote: > > On Thu, Feb 22, 2018 at 09:38:39AM +0100, Geert Uytterhoeven wrote: > >> On Wed, Feb 21, 2018 at 7:32 PM, Simon Horman wrote: > >> > On Wed, Feb 21, 2018 at 05:30:12PM +0100, Geert Uytterhoeven wrote: > >> >> On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman > >> >> wrote: > >> >> > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote: > >> >> >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro > >> >> >> wrote: > >> >> >> > this series has been around for some time as RFC, and it has > >> >> >> > collected > >> >> >> > useful comments from the community along the way. > >> >> >> > The solution proposed by this patch set works for most R-Car Gen2 > >> >> >> > and > >> >> >> > RZ/G1 devices, but not all of them. We now know that for some R-Car > >> >> >> > Gen2 early revisions there is no proper software fix. Anyway, no > >> >> >> > product has been built around early revisions, but development > >> >> >> > boards > >> >> >> > mounting early revisions (basically prototypes) are still out > >> >> >> > there. > >> >> >> > As a result, this series isn't enabling the internal watchdog on > >> >> >> > R-Car > >> >> >> > Gen2 boards, developers may enable it in board specific device > >> >> >> > trees > >> >> >> > if needed. > >> >> >> > This series has been tested by me on the iwg20d, iwg22d, Lager, > >> >> >> > Alt, > >> >> >> > and Koelsch boards. > >> >> >> > > >> >> >> > The problem > >> >> >> > === > >> >> >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector > >> >> >> > to ICRAM1 and we program the [S]BAR registers so that when we turn > >> >> >> > ON > >> >> >> > the non-boot CPUs they are redirected to the reset vector > >> >> >> > installed by > >> >> >> > Linux in ICRAM1, and eventually they continue the execution to RAM, > >> >> >> > where the SMP bring-up code will take care of the rest. > >> >> >> > The content of the [S]BAR registers survives a watchdog triggered > >> >> >> > reset, > >> >> >> > and as such after the watchdog fires the boot core will try and > >> >> >> > execute > >> >> >> > the SMP bring-up code instead of jumping to the bootrom code. > >> >> >> > > >> >> >> > The fix > >> >> >> > === > >> >> >> > The main strategy for the solution is to let the reset vector > >> >> >> > decide > >> >> >> > if it needs to jump to shmobile_boot_fn or to the bootrom code. > >> >> >> > In a watchdog triggered reset scenario, since the [S]BAR registers > >> >> >> > keep > >> >> >> > their values, the boot CPU will jump into the newly designed reset > >> >> >> > vector, the assembly routine will eventually test WOVF (a bit in > >> >> >> > register > >> >> >> > RWTCSRA that indicates if the watchdog counter has overflown, the > >> >> >> > value > >> >> >> > of this bit gets retained in this scenario), and jump to the > >> >> >> > bootrom code > >> >> >> > which will in turn load up the bootloader, etc. > >> >> >> > When bringing up SMP or using CPU hotplug, the reset vector will > >> >> >> > jump > >> >> >> > to shmobile_boot_fn instead. > >> >> >> > > >> >> >> > Thank you All for your help. > >> >> >> > > >> >> >> > Best regards, > >> >> >> > > >> >> >> > Fabrizio Castro (26): > >> >> >> > ARM: shmobile: Add watchdog support > >> >> >> > ARM: dts: r8a7743: Adjust SMP routine size > >> >> >> > ARM: dts: r8a7745: Adjust SMP routine size > >> >> >> > ARM: dts: r8a7790: Adjust SMP routine size > >> >> >> > ARM: dts: r8a7791: Adjust SMP routine size > >> >> >> > ARM: dts: r8a7792: Adjust SMP routine size > >> >> >> > ARM: dts: r8a7793: Adjust SMP routine size > >> >> >> > ARM: dts: r8a7794: Adjust SMP routine size > >> >> >> > soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2 > >> >> >> > ARM: shmobile: rcar-gen2: Add watchdog support > >> >> >> > dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support > >> >> >> > watchdog: renesas_wdt: Add R-Car Gen2 support > >> >> >> > watchdog: renesas_wdt: Add restart handler > >> >> >> > ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN > >> >> >> > clk: renesas: r8a7743: Add rwdt clock > >> >> >> > clk: renesas: r8a7745: Add rwdt clock > >> >> >> > clk: renesas: r8a7790: Add rwdt clock > >> >> >> > clk: renesas: r8a7791/r8a7793: Add rwdt clock > >> >> >> > clk: renesas: r8a7794: Add rwdt clock > >> >> >> > ARM: dts: r8a7743: Add watchdog support to SoC dtsi > >> >> >> > ARM: dts: r8a7745: Add watchdog support to SoC dtsi > >> >> >> > ARM: dts: r8a7790: Add watchdog support to SoC dtsi > >> >> >> > ARM: dts: r8a7791: Add watchdog support to SoC dtsi > >> >> >> > ARM: dts: r8a7794: Add watchdog support to SoC dtsi > >> >> >> > ARM: dts: iwg20m: Add watchdog support to SoM dtsi > >> >> >> > ARM: dts: iwg22m: Add watchdog support to SoM dtsi > >> >> >> > > >> >> >> > .../devicetree/bindings/watchdo
[RFC PATCH] i2c: device passthrough HACK(!) & evaluation
I was asked to investigate I2C device passthrough possibilities for QEMU on Linux. The idea was to expose only a single device, not the whole bus. There was no specific use case explained, so some decisions are still to be made. E.g. I think the host-device should get its own virtualized bus later, but I can't really tell yet. This is my first contact with QEMU, so the patch is more like getting my feet wet. I wouldn't even call it a proof-of-concept. However, while working on this (which gets at least something done), I already gained experiences which I would like to share here: Full virtualization --- Would be nice to have because it would work with all virtual I2C adapters instantly, however there come problems with it. For that to work, we would need to be able to transmit the QEMU I2C primitives from userspace to the kernel. There is currently no such interface for that. As of today, there is only the i2c-dev interface which allows to send a complete transfer (which may consist of multiple, combined messages) or SMBus commands. There is no way to send more primitive commands like "send {start|stop|acknowledge}-bit". Even if there was, most hardware I know of wouldn't work well with this. We often need a-priori knowledge like length of the message to program the controller. Such information is not available when working with such primitives. On top of that, that approach would cause quite some overhead, so performance regressions for drivers which use other devices on the same bus are likely. virtio? --- >From what I understood so far, virtio could help here. Yes, we would need separate drivers, yet data transfer could be super simple. If we had a simple virtio-PCI I2C master device, it could have a really simple kernel driver. It basically takes the I2C transfer it gets, does some sanity checking so no other devices are accessed, and then passes it to the kernel. I have not fully understood yet, if the virtio transportation mechanism is better/required here, or if we can/should still use the existing i2c-dev character interface. Other problems found Here is a list of other problems I discovered which need addressing in one way or the other: * exclusive access to I2C devices We currently don't have a way to mark devices as being "in use, don't touch" from userspace. This could be added but we need to decide on the transportation layer first (i2c-dev vs. virtio). * no generic I2C master (at least for x86) Unless I overlooked something, we currently can't simply add a new I2C bus on x86 because there is no virtual hardware encoded just doing that. I found patches for a USB-to-I2C bridge floating around. USB is nicely generic, so probably worth evaluating those again if this task is to be continued. * re-definition of I2C_SLAVE QEMU defines I2C_SLAVE as well as . So, if that interface is going to be used, we need something to fix this. * likely improvements to the QEMU I2C core >From visual review, I am quite sure QEMU I2C core does not support 'repeated start' with the following message having a different I2C destination address than the previous one. This is legal, but up to now very rarely seen in practice. However, with devices becoming more complex and those devices maybe being passed-through as well, more improvements to the QEMU I2C core might be needed as well. Conclusion -- These are my finding regarding I2C device passthrough with QEMU. The below patch is a very first step in the "full virtualization" direction because it transports every read byte access directly to the host (totally missing proper I2C start/stop/ack generation). Write byte access is discarded here for security reasons. As described above, I would not recommend this approach any further. My staring point now would be a simplified virtio or virtio-alike device where the transfer is passed-through as such to the host, and not split up into primitives. So the patch itself is somehow already obsolete, but it served well for gaining experience. For the record, the description of my test procedure is here: https://elinux.org/R-Car/I2C-Virtualization I will still be around for further discussion. I can't really tell, though, if there will be a follow-up task for me to continue this. Signed-off-by: Wolfram Sang --- hw/i2c/Makefile.objs | 2 +- hw/i2c/host-i2cdev.c | 110 +++ 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 hw/i2c/host-i2cdev.c diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs index 37cacde978..d0cc08dd10 100644 --- a/hw/i2c/Makefile.objs +++ b/hw/i2c/Makefile.objs @@ -1,4 +1,4 @@ -common-obj-$(CONFIG_I2C) += core.o smbus.o smbus_eeprom.o +common-obj-$(CONFIG_I2C) += core.o smbus.o smbus_eeprom.o host-i2cdev.o common-obj-$(CONFIG_DDC) += i2c-ddc.o common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o common-obj-$(CONFIG_ACPI_X86) += smbus_ich9.o diff --git a/hw/i2c
[PATCH] pinctrl: sh-pfc: r8a77970: add EtherAVB pin groups
Add the EtherAVB pin groups to the R8A77970 PFC driver. Signed-off-by: Sergei Shtylyov --- The patch is against the 'sh-pfc' branch of Geert's 'renesas-drivers.git' repo. drivers/pinctrl/sh-pfc/pfc-r8a77970.c | 98 ++ 1 file changed, 98 insertions(+) Index: renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77970.c === --- renesas-drivers.orig/drivers/pinctrl/sh-pfc/pfc-r8a77970.c +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77970.c @@ -728,6 +728,82 @@ static const struct sh_pfc_pin pinmux_pi PINMUX_GPIO_GP_ALL(), }; +/* - AVB0 --- */ +static const unsigned int avb0_link_pins[] = { + /* AVB0_LINK */ + RCAR_GP_PIN(1, 18), +}; +static const unsigned int avb0_link_mux[] = { + AVB0_LINK_MARK, +}; +static const unsigned int avb0_magic_pins[] = { + /* AVB0_MAGIC */ + RCAR_GP_PIN(1, 16), +}; +static const unsigned int avb0_magic_mux[] = { + AVB0_MAGIC_MARK, +}; +static const unsigned int avb0_phy_int_pins[] = { + /* AVB0_PHY_INT */ + RCAR_GP_PIN(1, 17), +}; +static const unsigned int avb0_phy_int_mux[] = { + AVB0_PHY_INT_MARK, +}; +static const unsigned int avb0_mdio_pins[] = { + /* AVB0_MDC, AVB0_MDIO */ + RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14), +}; +static const unsigned int avb0_mdio_mux[] = { + AVB0_MDC_MARK, AVB0_MDIO_MARK, +}; +static const unsigned int avb0_rgmii_pins[] = { + /* +* AVB0_TX_CTL, AVB0_TXC, AVB0_TD0, AVB0_TD1, AVB0_TD2, AVB0_TD3, +* AVB0_RX_CTL, AVB0_RXC, AVB0_RD0, AVB0_RD1, AVB0_RD2, AVB0_RD3 +*/ + RCAR_GP_PIN(1, 7), RCAR_GP_PIN(1, 8), + RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 10), + RCAR_GP_PIN(1, 11), RCAR_GP_PIN(1, 12), + RCAR_GP_PIN(1, 1), RCAR_GP_PIN(1, 2), + RCAR_GP_PIN(1, 3), RCAR_GP_PIN(1, 4), + RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6), +}; +static const unsigned int avb0_rgmii_mux[] = { + AVB0_TX_CTL_MARK, AVB0_TXC_MARK, + AVB0_TD0_MARK, AVB0_TD1_MARK, AVB0_TD2_MARK, AVB0_TD3_MARK, + AVB0_RX_CTL_MARK, AVB0_RXC_MARK, + AVB0_RD0_MARK, AVB0_RD1_MARK, AVB0_RD2_MARK, AVB0_RD3_MARK, +}; +static const unsigned int avb0_txcrefclk_pins[] = { + /* AVB0_TXCREFCLK */ + RCAR_GP_PIN(1, 13), +}; +static const unsigned int avb0_txcrefclk_mux[] = { + AVB0_TXCREFCLK_MARK, +}; +static const unsigned int avb0_avtp_pps_pins[] = { + /* AVB0_AVTP_PPS */ + RCAR_GP_PIN(2, 6), +}; +static const unsigned int avb0_avtp_pps_mux[] = { + AVB0_AVTP_PPS_MARK, +}; +static const unsigned int avb0_avtp_capture_pins[] = { + /* AVB0_AVTP_CAPTURE */ + RCAR_GP_PIN(1, 20), +}; +static const unsigned int avb0_avtp_capture_mux[] = { + AVB0_AVTP_CAPTURE_MARK, +}; +static const unsigned int avb0_avtp_match_pins[] = { + /* AVB0_AVTP_MATCH */ + RCAR_GP_PIN(1, 19), +}; +static const unsigned int avb0_avtp_match_mux[] = { + AVB0_AVTP_MATCH_MARK, +}; + /* - CANFD Clock */ static const unsigned int canfd_clk_a_pins[] = { /* CANFD_CLK */ @@ -1599,6 +1675,15 @@ static const unsigned int vin1_clk_mux[] }; static const struct sh_pfc_pin_group pinmux_groups[] = { + SH_PFC_PIN_GROUP(avb0_link), + SH_PFC_PIN_GROUP(avb0_magic), + SH_PFC_PIN_GROUP(avb0_phy_int), + SH_PFC_PIN_GROUP(avb0_mdio), + SH_PFC_PIN_GROUP(avb0_rgmii), + SH_PFC_PIN_GROUP(avb0_txcrefclk), + SH_PFC_PIN_GROUP(avb0_avtp_pps), + SH_PFC_PIN_GROUP(avb0_avtp_capture), + SH_PFC_PIN_GROUP(avb0_avtp_match), SH_PFC_PIN_GROUP(canfd_clk_a), SH_PFC_PIN_GROUP(canfd_clk_b), SH_PFC_PIN_GROUP(canfd0_data_a), @@ -1709,6 +1794,18 @@ static const struct sh_pfc_pin_group pin SH_PFC_PIN_GROUP(vin1_clk), }; +static const char * const avb0_groups[] = { + "avb0_link", + "avb0_magic", + "avb0_phy_int", + "avb0_mdio", + "avb0_rgmii", + "avb0_txcrefclk", + "avb0_avtp_pps", + "avb0_avtp_capture", + "avb0_avtp_match", +}; + static const char * const canfd_clk_groups[] = { "canfd_clk_a", "canfd_clk_b", @@ -1914,6 +2011,7 @@ static const char * const vin1_groups[] }; static const struct sh_pfc_function pinmux_functions[] = { + SH_PFC_FUNCTION(avb0), SH_PFC_FUNCTION(canfd_clk), SH_PFC_FUNCTION(canfd0), SH_PFC_FUNCTION(canfd1),
Re: [PATCH 2/3] rcar-vin: allocate a scratch buffer at stream start
Hi Niklas, On Sat, Mar 10, 2018 at 01:09:52AM +0100, Niklas Söderlund wrote: > Before starting capturing allocate a scratch buffer which can be used by > the driver to give to the hardware if no buffers are available from > userspace. The buffer is not used in this patch but prepares for future > refactoring where the scratch buffer can be used to avoid the need to > fallback on single capture mode if userspace don't queue buffers as fast > as the VIN driver consumes them. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 19 +++ > drivers/media/platform/rcar-vin/rcar-vin.h | 4 > 2 files changed, 23 insertions(+) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index b4be75d5009080f7..8ea73cdc9a720abe 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -1070,6 +1070,17 @@ static int rvin_start_streaming(struct vb2_queue *vq, > unsigned int count) > unsigned long flags; > int ret; > > + /* Allocate scratch buffer. */ > + vin->scratch = dma_alloc_coherent(vin->dev, vin->format.sizeimage, > + &vin->scratch_phys, GFP_KERNEL); > + if (!vin->scratch) { > + spin_lock_irqsave(&vin->qlock, flags); > + return_all_buffers(vin, VB2_BUF_STATE_QUEUED); > + spin_unlock_irqrestore(&vin->qlock, flags); > + vin_err(vin, "Failed to allocate scratch buffer\n"); > + return -ENOMEM; > + } > + > sd = vin_to_source(vin); > v4l2_subdev_call(sd, video, s_stream, 1); > > @@ -1085,6 +1096,10 @@ static int rvin_start_streaming(struct vb2_queue *vq, > unsigned int count) > > spin_unlock_irqrestore(&vin->qlock, flags); > > + if (ret) > + dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, > + vin->scratch_phys); > + > return ret; > } > > @@ -1135,6 +1150,10 @@ static void rvin_stop_streaming(struct vb2_queue *vq) > > /* disable interrupts */ > rvin_disable_interrupts(vin); > + > + /* Free scratch buffer. */ > + dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, > + vin->scratch_phys); > } > > static const struct vb2_ops rvin_qops = { > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h > b/drivers/media/platform/rcar-vin/rcar-vin.h > index 5382078143fb3869..11a981d707c7ca47 100644 > --- a/drivers/media/platform/rcar-vin/rcar-vin.h > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h > @@ -102,6 +102,8 @@ struct rvin_graph_entity { > * > * @lock:protects @queue > * @queue: vb2 buffers queue > + * @scratch: cpu address for scratch buffer > + * @scratch_phys:pysical address of the scratch buffer Nitpicking: physical Thanks j > * > * @qlock: protects @queue_buf, @buf_list, @continuous, @sequence > * @state > @@ -130,6 +132,8 @@ struct rvin_dev { > > struct mutex lock; > struct vb2_queue queue; > + void *scratch; > + dma_addr_t scratch_phys; > > spinlock_t qlock; > struct vb2_v4l2_buffer *queue_buf[HW_BUFFER_NUM]; > -- > 2.16.2 > signature.asc Description: PGP signature
[PATCH] arm64: dts: renesas: v3msk: add SCIF0 pins
Add the (previously omitted) SCIF0 pin data to the V3M Starter Kit board's device tree. Signed-off-by: Sergei Shtylyov --- The patch is against the 'renesas-devel-20180308-v4.16-rc4' tag of Simon's 'renesas.git repo... arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts | 10 ++ 1 file changed, 10 insertions(+) Index: renesas/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts === --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts +++ renesas/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts @@ -51,6 +51,16 @@ clock-frequency = <32768>; }; +&pfc { + scif0_pins: scif0 { + groups = "scif0_data"; + function = "scif0"; + }; +}; + &scif0 { + pinctrl-0 = <&scif0_pins>; + pinctrl-names = "default"; + status = "okay"; };
Re: [PATCH] arm64: dts: renesas: r8a7795: Add missing SYS-DMAC2 dmas
Hi Simon, On Tue, Mar 13, 2018 at 7:43 PM, Simon Horman wrote: > On Thu, Mar 08, 2018 at 03:10:05PM +0100, Geert Uytterhoeven wrote: >> On R-Car H3, on-chip peripheral modules that can make use of DMA are >> wired to either SYS-DMAC0 only, or to both SYS-DMAC1 and SYS-DMAC2. >> >> Add the missing DMA properties pointing to SYS-DMAC2 for HSCIF[0-2], >> SCIF[015], and I2C[0-2]. These were initially left out because early >> firmware versions prohibited using SYS-DMAC2. This restriction has been >> lifted in IPL and Secure Monitor Rev1.0.6 (released on Feb 25, 2016). >> >> Signed-off-by: Geert Uytterhoeven > > Thanks, applied. This one, or v2? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 0/3] arm64: dts: renesas: Rename EtherAVB "mdc" pin group to "mdio"
Hi Simon, On Tue, Mar 13, 2018 at 7:49 PM, Simon Horman wrote: > On Mon, Mar 12, 2018 at 04:11:57PM +0100, Geert Uytterhoeven wrote: >> On Renesas R-Car Gen2 SoCs, the pin group for the MDIO bus is named >> "mdio". >> >> When initial support was added for R-Car H3, the MDIO pin was forgotten, >> and the MDC pin got its own group named "mdc". During the addition of >> support for R-Car M3-W, this mistake was noticed. But as R-Car H3 and >> M3-W are pin compatible, and can be mounted on the same boards, the >> decision was made to just add the MDIO pin to the existing "mdc" group. >> Later this was extended to R-Car H3 ES2.0, and M3-N, because of pin >> compatibility, and to R-Car D3, in the name of consistency among R-Car >> Gen3 SoCs. >> >> However, this decision keeps on being questioned when adding new SoC >> support. Hence bite the bullet and admit our mistake, and rename the >> pin group from "mdc" to "mdio", like on R-Car Gen2 SoCs. >> >> This series is the DTS part, and depends on the series '[PATCH 0/6] >> pinctrl: sh-pfc: rcar-gen3: Rename EtherAVB "mdc" pin group to "mdio"'. > > could you comment on the forwards/backwards compatibility considerations > of this series? Old and new kernels work with old DT (tested on Salvator-X(S). New DT requires a new kernel, hence the dependency of the DTS part on the driver part. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 0/3] arm64: dts: renesas: Rename EtherAVB "mdc" pin group to "mdio"
On Mon, Mar 12, 2018 at 04:11:57PM +0100, Geert Uytterhoeven wrote: > Hi Simon, Magnus, > > On Renesas R-Car Gen2 SoCs, the pin group for the MDIO bus is named > "mdio". > > When initial support was added for R-Car H3, the MDIO pin was forgotten, > and the MDC pin got its own group named "mdc". During the addition of > support for R-Car M3-W, this mistake was noticed. But as R-Car H3 and > M3-W are pin compatible, and can be mounted on the same boards, the > decision was made to just add the MDIO pin to the existing "mdc" group. > Later this was extended to R-Car H3 ES2.0, and M3-N, because of pin > compatibility, and to R-Car D3, in the name of consistency among R-Car > Gen3 SoCs. > > However, this decision keeps on being questioned when adding new SoC > support. Hence bite the bullet and admit our mistake, and rename the > pin group from "mdc" to "mdio", like on R-Car Gen2 SoCs. > > This series is the DTS part, and depends on the series '[PATCH 0/6] > pinctrl: sh-pfc: rcar-gen3: Rename EtherAVB "mdc" pin group to "mdio"'. Hi Geert, could you comment on the forwards/backwards compatibility considerations of this series?
Re: [PATCH v3 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle
On Tue, Mar 13, 2018 at 03:30:25PM +0100, Jacopo Mondi wrote: > The R-Car V3M Eagle board includes a transparent THC63LVD1024 LVDS > decoder, connected to the on-chip LVDS encoder output on one side > and to HDMI encoder ADV7511w on the other one. > > As the decoder does not need any configuration it has been so-far > omitted from DTS. Now that a driver is available, describe it in DT > as well. > > Signed-off-by: Jacopo Mondi I have marked this as deferred pending acceptance of the new binding. Please repost or ping me once that has happened.
Re: [PATCH 3/3] rcar-vin: use scratch buffer and always run in continuous mode
On 03/09/2018 04:09 PM, Niklas Söderlund wrote: > Instead of switching capture mode depending on how many buffers are > available use a scratch buffer and always run in continuous mode. By > using a scratch buffer the responsiveness of the capture loop is > increased as it can keep running even if there are no buffers available > from userspace. > > As soon as a userspace queues a buffer it is inserted into the capture > loop and returned as soon as it is filled. This is a improvement on the > previous logic where the whole capture loop was stopped and switched to > single capture mode if userspace did not feed the VIN driver buffers at > the same time it consumed them. To make matters worse it was difficult > for the driver to reenter continues mode if it entered single mode even continues -> continuous > if userspace started to queue buffers faster. This resulted in > suboptimal performance where if userspace where delayed for a short > period the ongoing capture would be slowed down and run in single mode > until the capturing process where restarted. > > An additional effect of this change is that the capture logic can be > made much simple as we know that continues mode will always be used. ditto > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 187 > - > drivers/media/platform/rcar-vin/rcar-vin.h | 6 +- > 2 files changed, 52 insertions(+), 141 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index 8ea73cdc9a720abe..208cf8a0ea77002d 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -168,12 +168,8 @@ static int rvin_setup(struct rvin_dev *vin) > break; > case V4L2_FIELD_ALTERNATE: > case V4L2_FIELD_NONE: > - if (vin->continuous) { > - vnmc = VNMC_IM_ODD_EVEN; > - progressive = true; > - } else { > - vnmc = VNMC_IM_ODD; > - } > + vnmc = VNMC_IM_ODD_EVEN; > + progressive = true; > break; > default: > vnmc = VNMC_IM_ODD; > @@ -298,14 +294,6 @@ static bool rvin_capture_active(struct rvin_dev *vin) > return rvin_read(vin, VNMS_REG) & VNMS_CA; > } > > -static int rvin_get_active_slot(struct rvin_dev *vin, u32 vnms) > -{ > - if (vin->continuous) > - return (vnms & VNMS_FBS_MASK) >> VNMS_FBS_SHIFT; > - > - return 0; > -} > - > static enum v4l2_field rvin_get_active_field(struct rvin_dev *vin, u32 vnms) > { > if (vin->format.field == V4L2_FIELD_ALTERNATE) { > @@ -344,76 +332,47 @@ static void rvin_set_slot_addr(struct rvin_dev *vin, > int slot, dma_addr_t addr) > rvin_write(vin, offset, VNMB_REG(slot)); > } > > -/* Moves a buffer from the queue to the HW slots */ > -static bool rvin_fill_hw_slot(struct rvin_dev *vin, int slot) > +/* > + * Moves a buffer from the queue to the HW slot. If no buffer is > + * available use the scratch buffer. The scratch buffer is never > + * returned to userspace, its only function is to enable the capture > + * loop to keep running. > + */ > +static void rvin_fill_hw_slot(struct rvin_dev *vin, int slot) > { > struct rvin_buffer *buf; > struct vb2_v4l2_buffer *vbuf; > - dma_addr_t phys_addr_top; > - > - if (vin->queue_buf[slot] != NULL) > - return true; > + dma_addr_t phys_addr; > > - if (list_empty(&vin->buf_list)) > - return false; > + /* A already populated slot shall never be overwritten. */ > + if (WARN_ON(vin->queue_buf[slot] != NULL)) > + return; > > vin_dbg(vin, "Filling HW slot: %d\n", slot); > > - /* Keep track of buffer we give to HW */ > - buf = list_entry(vin->buf_list.next, struct rvin_buffer, list); > - vbuf = &buf->vb; > - list_del_init(to_buf_list(vbuf)); > - vin->queue_buf[slot] = vbuf; > - > - /* Setup DMA */ > - phys_addr_top = vb2_dma_contig_plane_dma_addr(&vbuf->vb2_buf, 0); > - rvin_set_slot_addr(vin, slot, phys_addr_top); > - > - return true; > -} > - > -static bool rvin_fill_hw(struct rvin_dev *vin) > -{ > - int slot, limit; > - > - limit = vin->continuous ? HW_BUFFER_NUM : 1; > - > - for (slot = 0; slot < limit; slot++) > - if (!rvin_fill_hw_slot(vin, slot)) > - return false; > - return true; > -} > - > -static void rvin_capture_on(struct rvin_dev *vin) > -{ > - vin_dbg(vin, "Capture on in %s mode\n", > - vin->continuous ? "continuous" : "single"); > + if (list_empty(&vin->buf_list)) { > + vin->queue_buf[slot] = NULL; > + phys_addr = vin->scratch_phys; > + } else { > + /* Keep track of buffer we give to HW */ > + buf = list_entry(vin->buf_list.next, struct rvin_buffer, list); > +
Re: [PATCH 0/3] Add R8A77980/Condor PFC support
On Fri, Mar 09, 2018 at 03:04:56PM +0300, Sergei Shtylyov wrote: > Hello! > > Here's the set of 3 patches against Simon Horman's 'renesas.git' repo's > 'renesas-devel-20180308-v4.16-rc4' tag. We're adding the R8A77980 PFC node > and then describing the pins for SCIF0 and EtherAVB devices declared earlier. > These patches depend on the R8A77980 PFC support in order to work properly. I have marked these patches as Deferred. Please repost or ping me once the dependencies are in an RC release. If looser dependency handling is appropriate please let me know.
Re: [PATCH] arm64: dts: renesas: r8a7795: Add missing SYS-DMAC2 dmas
On Thu, Mar 08, 2018 at 03:10:05PM +0100, Geert Uytterhoeven wrote: > On R-Car H3, on-chip peripheral modules that can make use of DMA are > wired to either SYS-DMAC0 only, or to both SYS-DMAC1 and SYS-DMAC2. > > Add the missing DMA properties pointing to SYS-DMAC2 for HSCIF[0-2], > SCIF[015], and I2C[0-2]. These were initially left out because early > firmware versions prohibited using SYS-DMAC2. This restriction has been > lifted in IPL and Secure Monitor Rev1.0.6 (released on Feb 25, 2016). > > Signed-off-by: Geert Uytterhoeven Thanks, applied.
Re: [PATCH 1/3] rcar-vin: remove duplicated check of state in irq handler
On 03/09/2018 04:09 PM, Niklas Söderlund wrote: > This is an error from when the driver where converted from soc-camera. where -> was > There is absolutely no gain to check the state variable two times to be > extra sure if the hardware is stopped. I'll wait for v2 before applying this. Regards, Hans > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index 23fdff7a7370842e..b4be75d5009080f7 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -916,12 +916,6 @@ static irqreturn_t rvin_irq(int irq, void *data) > rvin_ack_interrupt(vin); > handled = 1; > > - /* Nothing to do if capture status is 'STOPPED' */ > - if (vin->state == STOPPED) { > - vin_dbg(vin, "IRQ while state stopped\n"); > - goto done; > - } > - > /* Nothing to do if capture status is 'STOPPING' */ > if (vin->state == STOPPING) { > vin_dbg(vin, "IRQ while state stopping\n"); >
[PATCH v2 00/11] R-Car DU Interlaced support through VSP1
The Gen3 R-Car DU devices make use of the VSP to handle frame processing. In this series we implement support for handling interlaced pipelines by using the auto-fld feature of the VSP hardware. The implementation is preceded by some cleanup work and refactoring, through patches 1 to 6. These are trivial and could be collected earlier and independently if this series requires further revisions. Patch 7 makes a key distinctive change to remove all existing support for headerless display lists throughout the VSP1 driver, and ensures that all pipelines use the same code path. This simplifies the code and reduces opportunity for untested code paths to exist. Patches 8, 9 and 10 implement the relevant support in the VSP1 driver, before patch 11 finally enables the feature through the drm R-Car DU driver. This series is based upon my previous TLB optimise and body rework (v7), and is available from the following URL: git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git tags/vsp1/du/interlaced/v2 ChangeLog: v2: - media: vsp1: use kernel __packed for structures became: media: vsp1: Remove packed attributes from aligned structures - media: vsp1: Add support for extended display list headers - No longer declares structs __packed - media: vsp1: Provide support for extended command pools - Fix spelling typo in commit message - constify, and staticify the instantiation of vsp1_extended_commands - s/autfld_cmds/autofld_cmds/ - staticify cmd pool functions (Thanks kbuild-bot) - media: vsp1: Support Interlaced display pipelines - fix erroneous BIT value which enabled interlaced - fix field handling at frame_end interrupt Kieran Bingham (11): media: vsp1: drm: Fix minor grammar error media: vsp1: Remove packed attributes from aligned structures media: vsp1: Rename dl_child to dl_next media: vsp1: Remove unused display list structure field media: vsp1: Clean up DLM objects on error media: vsp1: Provide VSP1 feature helper macro media: vsp1: Use header display lists for all WPF outputs linked to the DU media: vsp1: Add support for extended display list headers media: vsp1: Provide support for extended command pools media: vsp1: Support Interlaced display pipelines drm: rcar-du: Support interlaced video output through vsp1 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +- drivers/media/platform/vsp1/vsp1.h | 3 +- drivers/media/platform/vsp1/vsp1_dl.c | 406 +++-- drivers/media/platform/vsp1/vsp1_dl.h | 32 +- drivers/media/platform/vsp1/vsp1_drm.c | 13 +- drivers/media/platform/vsp1/vsp1_drv.c | 23 +- drivers/media/platform/vsp1/vsp1_regs.h | 6 +- drivers/media/platform/vsp1/vsp1_rpf.c | 72 +++- drivers/media/platform/vsp1/vsp1_rwpf.h | 1 +- drivers/media/platform/vsp1/vsp1_wpf.c | 6 +- include/media/vsp1.h| 1 +- 12 files changed, 455 insertions(+), 112 deletions(-) base-commit: 397eb3811ec096d0ceefa1dbea2d0ae68feb0587 -- git-series 0.9.1
[PATCH v2 01/11] media: vsp1: drm: Fix minor grammar error
The pixel format is 'unsupported'. Fix the small debug message which incorrectly declares this. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 3c8b1952799d..0459b970e9da 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -363,7 +363,7 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index, */ fmtinfo = vsp1_get_format_info(vsp1, cfg->pixelformat); if (!fmtinfo) { - dev_dbg(vsp1->dev, "Unsupport pixel format %08x for RPF\n", + dev_dbg(vsp1->dev, "Unsupported pixel format %08x for RPF\n", cfg->pixelformat); return -EINVAL; } -- git-series 0.9.1
[PATCH v2 04/11] media: vsp1: Remove unused display list structure field
The vsp1 reference in the vsp1_dl_body structure is not used. Remove it. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_dl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 8162f4ce66eb..310ce81cd724 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -48,7 +48,6 @@ struct vsp1_dl_entry { * @list: entry in the display list list of bodies * @free: entry in the pool free body list * @pool: pool to which this body belongs - * @vsp1: the VSP1 device * @entries: array of entries * @dma: DMA address of the entries * @size: size of the DMA memory in bytes @@ -62,7 +61,6 @@ struct vsp1_dl_body { refcount_t refcnt; struct vsp1_dl_body_pool *pool; - struct vsp1_device *vsp1; struct vsp1_dl_entry *entries; dma_addr_t dma; -- git-series 0.9.1
[PATCH v2 03/11] media: vsp1: Rename dl_child to dl_next
Both vsp1_dl_list_commit() and __vsp1_dl_list_put() walk the display list chain referencing the nodes as children, when in reality they are siblings. Update the terminology to 'dl_next' to be consistent with the vsp1_video_pipeline_run() usage. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_dl.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 85795b55a357..8162f4ce66eb 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -400,7 +400,7 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm) /* This function must be called with the display list manager lock held.*/ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl) { - struct vsp1_dl_list *dl_child; + struct vsp1_dl_list *dl_next; if (!dl) return; @@ -410,8 +410,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl) * hardware operation. */ if (dl->has_chain) { - list_for_each_entry(dl_child, &dl->chain, chain) - __vsp1_dl_list_put(dl_child); + list_for_each_entry(dl_next, &dl->chain, chain) + __vsp1_dl_list_put(dl_next); } dl->has_chain = false; @@ -667,17 +667,17 @@ static void vsp1_dl_list_commit_singleshot(struct vsp1_dl_list *dl) void vsp1_dl_list_commit(struct vsp1_dl_list *dl) { struct vsp1_dl_manager *dlm = dl->dlm; - struct vsp1_dl_list *dl_child; + struct vsp1_dl_list *dl_next; unsigned long flags; if (dlm->mode == VSP1_DL_MODE_HEADER) { /* Fill the header for the head and chained display lists. */ vsp1_dl_list_fill_header(dl, list_empty(&dl->chain)); - list_for_each_entry(dl_child, &dl->chain, chain) { - bool last = list_is_last(&dl_child->chain, &dl->chain); + list_for_each_entry(dl_next, &dl->chain, chain) { + bool last = list_is_last(&dl_next->chain, &dl->chain); - vsp1_dl_list_fill_header(dl_child, last); + vsp1_dl_list_fill_header(dl_next, last); } } -- git-series 0.9.1
[PATCH v2 02/11] media: vsp1: Remove packed attributes from aligned structures
The use of the packed attribute can cause a performance penalty for all accesses to the struct members, as the compiler will assume that the structure has the potential to have an unaligned base. These structures are all correctly aligned and contain no holes, thus the attribute is redundant and negatively impacts performance, so we remove the attributes entirely. Signed-off-by: Kieran Bingham --- v2 - Remove attributes entirely drivers/media/platform/vsp1/vsp1_dl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 37e2c984fbf3..85795b55a357 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -29,19 +29,19 @@ struct vsp1_dl_header_list { u32 num_bytes; u32 addr; -} __attribute__((__packed__)); +}; struct vsp1_dl_header { u32 num_lists; struct vsp1_dl_header_list lists[8]; u32 next_header; u32 flags; -} __attribute__((__packed__)); +}; struct vsp1_dl_entry { u32 addr; u32 data; -} __attribute__((__packed__)); +}; /** * struct vsp1_dl_body - Display list body -- git-series 0.9.1
[PATCH v2 05/11] media: vsp1: Clean up DLM objects on error
If there is an error allocating a display list within a DLM object the existing display lists are not free'd, and neither is the DL body pool. Use the existing vsp1_dlm_destroy() function to clean up on error. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_dl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 310ce81cd724..680eedb6fc9f 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -831,8 +831,10 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1, struct vsp1_dl_list *dl; dl = vsp1_dl_list_alloc(dlm, dlm->pool); - if (!dl) + if (!dl) { + vsp1_dlm_destroy(dlm); return NULL; + } list_add_tail(&dl->list, &dlm->free); } -- git-series 0.9.1
[PATCH v2 06/11] media: vsp1: Provide VSP1 feature helper macro
The VSP1 devices define their specific capabilities through features marked in their device info structure. Various parts of the code read this info structure to infer if the features are available. Wrap this into a more readable vsp1_feature(vsp1, f) macro to ensure that usage is consistent throughout the driver. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1.h | 2 ++ drivers/media/platform/vsp1/vsp1_drv.c | 16 drivers/media/platform/vsp1/vsp1_wpf.c | 6 +++--- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h index 78ef838416b3..1c080538c993 100644 --- a/drivers/media/platform/vsp1/vsp1.h +++ b/drivers/media/platform/vsp1/vsp1.h @@ -69,6 +69,8 @@ struct vsp1_device_info { bool uapi; }; +#define vsp1_feature(vsp1, f) ((vsp1)->info->features & (f)) + struct vsp1_device { struct device *dev; const struct vsp1_device_info *info; diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c index eed9516e25e1..6fa0019ffc6e 100644 --- a/drivers/media/platform/vsp1/vsp1_drv.c +++ b/drivers/media/platform/vsp1/vsp1_drv.c @@ -268,7 +268,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) } /* Instantiate all the entities. */ - if (vsp1->info->features & VSP1_HAS_BRS) { + if (vsp1_feature(vsp1, VSP1_HAS_BRS)) { vsp1->brs = vsp1_bru_create(vsp1, VSP1_ENTITY_BRS); if (IS_ERR(vsp1->brs)) { ret = PTR_ERR(vsp1->brs); @@ -278,7 +278,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) list_add_tail(&vsp1->brs->entity.list_dev, &vsp1->entities); } - if (vsp1->info->features & VSP1_HAS_BRU) { + if (vsp1_feature(vsp1, VSP1_HAS_BRU)) { vsp1->bru = vsp1_bru_create(vsp1, VSP1_ENTITY_BRU); if (IS_ERR(vsp1->bru)) { ret = PTR_ERR(vsp1->bru); @@ -288,7 +288,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) list_add_tail(&vsp1->bru->entity.list_dev, &vsp1->entities); } - if (vsp1->info->features & VSP1_HAS_CLU) { + if (vsp1_feature(vsp1, VSP1_HAS_CLU)) { vsp1->clu = vsp1_clu_create(vsp1); if (IS_ERR(vsp1->clu)) { ret = PTR_ERR(vsp1->clu); @@ -314,7 +314,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) list_add_tail(&vsp1->hst->entity.list_dev, &vsp1->entities); - if (vsp1->info->features & VSP1_HAS_HGO && vsp1->info->uapi) { + if (vsp1_feature(vsp1, VSP1_HAS_HGO) && vsp1->info->uapi) { vsp1->hgo = vsp1_hgo_create(vsp1); if (IS_ERR(vsp1->hgo)) { ret = PTR_ERR(vsp1->hgo); @@ -325,7 +325,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) &vsp1->entities); } - if (vsp1->info->features & VSP1_HAS_HGT && vsp1->info->uapi) { + if (vsp1_feature(vsp1, VSP1_HAS_HGT) && vsp1->info->uapi) { vsp1->hgt = vsp1_hgt_create(vsp1); if (IS_ERR(vsp1->hgt)) { ret = PTR_ERR(vsp1->hgt); @@ -356,7 +356,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) } } - if (vsp1->info->features & VSP1_HAS_LUT) { + if (vsp1_feature(vsp1, VSP1_HAS_LUT)) { vsp1->lut = vsp1_lut_create(vsp1); if (IS_ERR(vsp1->lut)) { ret = PTR_ERR(vsp1->lut); @@ -390,7 +390,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) } } - if (vsp1->info->features & VSP1_HAS_SRU) { + if (vsp1_feature(vsp1, VSP1_HAS_SRU)) { vsp1->sru = vsp1_sru_create(vsp1); if (IS_ERR(vsp1->sru)) { ret = PTR_ERR(vsp1->sru); @@ -524,7 +524,7 @@ static int vsp1_device_init(struct vsp1_device *vsp1) vsp1_write(vsp1, VI6_DPR_HSI_ROUTE, VI6_DPR_NODE_UNUSED); vsp1_write(vsp1, VI6_DPR_BRU_ROUTE, VI6_DPR_NODE_UNUSED); - if (vsp1->info->features & VSP1_HAS_BRS) + if (vsp1_feature(vsp1, VSP1_HAS_BRS)) vsp1_write(vsp1, VI6_DPR_ILV_BRS_ROUTE, VI6_DPR_NODE_UNUSED); vsp1_write(vsp1, VI6_DPR_HGO_SMPPT, (7 << VI6_DPR_SMPPT_TGW_SHIFT) | diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c index 68218625549e..f90e474cf2cc 100644 --- a/drivers/media/platform/vsp1/vsp1_wpf.c +++ b/drivers/media/platform/vsp1/vsp1_wpf.c @@ -146,13 +146,13 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf) if (wpf->entity.index != 0) { /* Only WPF0 supports flipping. */ num_flip_ctrls = 0; - } else if (vsp1->info->features & VSP1_HAS_WPF_HFLIP) { + } else if (vsp1_feature(vsp1, VSP1_HAS_WPF
[PATCH v2 07/11] media: vsp1: Use header display lists for all WPF outputs linked to the DU
Header mode display lists are now supported on all WPF outputs. To support extended headers and auto-fld capabilities for interlaced mode handling only header mode display lists can be used. Disable the headerless display list configuration, and remove the dead code. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_dl.c | 107 ++- 1 file changed, 27 insertions(+), 80 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 680eedb6fc9f..5f5706f8a84c 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -98,7 +98,7 @@ struct vsp1_dl_body_pool { * struct vsp1_dl_list - Display list * @list: entry in the display list manager lists * @dlm: the display list manager - * @header: display list header, NULL for headerless lists + * @header: display list header * @dma: DMA address for the header * @body0: first display list body * @bodies: list of extra display list bodies @@ -119,15 +119,9 @@ struct vsp1_dl_list { struct list_head chain; }; -enum vsp1_dl_mode { - VSP1_DL_MODE_HEADER, - VSP1_DL_MODE_HEADERLESS, -}; - /** * struct vsp1_dl_manager - Display List manager * @index: index of the related WPF - * @mode: display list operation mode (header or headerless) * @singleshot: execute the display list in single-shot mode * @vsp1: the VSP1 device * @lock: protects the free, active, queued, pending and gc_bodies lists @@ -139,7 +133,6 @@ enum vsp1_dl_mode { */ struct vsp1_dl_manager { unsigned int index; - enum vsp1_dl_mode mode; bool singleshot; struct vsp1_device *vsp1; @@ -320,6 +313,7 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm, struct vsp1_dl_body_pool *pool) { struct vsp1_dl_list *dl; + size_t header_offset; dl = kzalloc(sizeof(*dl), GFP_KERNEL); if (!dl) @@ -332,16 +326,15 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm, dl->body0 = vsp1_dl_body_get(pool); if (!dl->body0) return NULL; - if (dlm->mode == VSP1_DL_MODE_HEADER) { - size_t header_offset = dl->body0->max_entries -* sizeof(*dl->body0->entries); - dl->header = ((void *)dl->body0->entries) + header_offset; - dl->dma = dl->body0->dma + header_offset; + header_offset = dl->body0->max_entries +* sizeof(*dl->body0->entries); - memset(dl->header, 0, sizeof(*dl->header)); - dl->header->lists[0].addr = dl->body0->dma; - } + dl->header = ((void *)dl->body0->entries) + header_offset; + dl->dma = dl->body0->dma + header_offset; + + memset(dl->header, 0, sizeof(*dl->header)); + dl->header->lists[0].addr = dl->body0->dma; return dl; } @@ -473,16 +466,9 @@ struct vsp1_dl_body *vsp1_dl_list_get_body0(struct vsp1_dl_list *dl) * * The reference must be explicitly released by a call to vsp1_dl_body_put() * when the body isn't needed anymore. - * - * Additional bodies are only usable for display lists in header mode. - * Attempting to add a body to a header-less display list will return an error. */ int vsp1_dl_list_add_body(struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb) { - /* Multi-body lists are only available in header mode. */ - if (dl->dlm->mode != VSP1_DL_MODE_HEADER) - return -EINVAL; - refcount_inc(&dlb->refcnt); list_add_tail(&dlb->list, &dl->bodies); @@ -503,17 +489,10 @@ int vsp1_dl_list_add_body(struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb) * Adding a display list to a chain passes ownership of the display list to * the head display list item. The chain is released when the head dl item is * put back with __vsp1_dl_list_put(). - * - * Chained display lists are only usable in header mode. Attempts to add a - * display list to a chain in header-less mode will return an error. */ int vsp1_dl_list_add_chain(struct vsp1_dl_list *head, struct vsp1_dl_list *dl) { - /* Chained lists are only available in header mode. */ - if (head->dlm->mode != VSP1_DL_MODE_HEADER) - return -EINVAL; - head->has_chain = true; list_add_tail(&dl->chain, &head->chain); return 0; @@ -581,17 +560,10 @@ static bool vsp1_dl_list_hw_update_pending(struct vsp1_dl_manager *dlm) return false; /* -* Check whether the VSP1 has taken the update. In headerless mode the -* hardware indicates this by clearing the UPD bit in the DL_BODY_SIZE -* register, and in header mode by clearing the UPDHDR bit in the CMD -* register. +* Check whether the VSP1 has taken the update. In header mode by +* cle
[PATCH v2 08/11] media: vsp1: Add support for extended display list headers
Extended display list headers allow pre and post command lists to be executed by the VSP pipeline. This provides the base support for features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for supporting continuous camera preview pipelines. Signed-off-by: Kieran Bingham --- v2: - remove __packed attributes drivers/media/platform/vsp1/vsp1.h | 1 +- drivers/media/platform/vsp1/vsp1_dl.c | 83 +- drivers/media/platform/vsp1/vsp1_dl.h | 29 - drivers/media/platform/vsp1/vsp1_drv.c | 7 +- drivers/media/platform/vsp1/vsp1_regs.h | 5 +- 5 files changed, 116 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h index 1c080538c993..bb3b32795206 100644 --- a/drivers/media/platform/vsp1/vsp1.h +++ b/drivers/media/platform/vsp1/vsp1.h @@ -55,6 +55,7 @@ struct vsp1_uds; #define VSP1_HAS_HGO (1 << 7) #define VSP1_HAS_HGT (1 << 8) #define VSP1_HAS_BRS (1 << 9) +#define VSP1_HAS_EXT_DL(1 << 10) struct vsp1_device_info { u32 version; diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 5f5706f8a84c..cd91b50deed1 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -26,6 +26,9 @@ #define VSP1_DLH_INT_ENABLE(1 << 1) #define VSP1_DLH_AUTO_START(1 << 0) +#define VSP1_DLH_EXT_PRE_CMD_EXEC (1 << 9) +#define VSP1_DLH_EXT_POST_CMD_EXEC (1 << 8) + struct vsp1_dl_header_list { u32 num_bytes; u32 addr; @@ -38,11 +41,34 @@ struct vsp1_dl_header { u32 flags; }; +struct vsp1_dl_ext_header { + u32 reserved0; /* alignment padding */ + + u16 pre_ext_cmd_qty; + u16 flags; + u32 pre_ext_cmd_plist; + + u32 post_ext_cmd_qty; + u32 post_ext_cmd_plist; +}; + +struct vsp1_dl_header_extended { + struct vsp1_dl_header header; + struct vsp1_dl_ext_header ext; +}; + struct vsp1_dl_entry { u32 addr; u32 data; }; +struct vsp1_dl_ext_cmd_header { + u32 cmd; + u32 flags; + u32 data; + u32 reserved; +}; + /** * struct vsp1_dl_body - Display list body * @list: entry in the display list list of bodies @@ -99,9 +125,12 @@ struct vsp1_dl_body_pool { * @list: entry in the display list manager lists * @dlm: the display list manager * @header: display list header + * @extended: extended display list header. NULL for normal lists * @dma: DMA address for the header * @body0: first display list body * @bodies: list of extra display list bodies + * @pre_cmd: pre cmd to be issued through extended dl header + * @post_cmd: post cmd to be issued through extended dl header * @has_chain: if true, indicates that there's a partition chain * @chain: entry in the display list partition chain */ @@ -110,11 +139,15 @@ struct vsp1_dl_list { struct vsp1_dl_manager *dlm; struct vsp1_dl_header *header; + struct vsp1_dl_ext_header *extended; dma_addr_t dma; struct vsp1_dl_body *body0; struct list_head bodies; + struct vsp1_dl_ext_cmd *pre_cmd; + struct vsp1_dl_ext_cmd *post_cmd; + bool has_chain; struct list_head chain; }; @@ -498,6 +531,14 @@ int vsp1_dl_list_add_chain(struct vsp1_dl_list *head, return 0; } +static void vsp1_dl_ext_cmd_fill_header(struct vsp1_dl_ext_cmd *cmd) +{ + cmd->cmds[0].cmd = cmd->cmd_opcode; + cmd->cmds[0].flags = cmd->flags; + cmd->cmds[0].data = cmd->data_dma; + cmd->cmds[0].reserved = 0; +} + static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last) { struct vsp1_dl_manager *dlm = dl->dlm; @@ -550,6 +591,27 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last) */ dl->header->flags = VSP1_DLH_INT_ENABLE; } + + if (!dl->extended) + return; + + dl->extended->flags = 0; + + if (dl->pre_cmd) { + dl->extended->pre_ext_cmd_plist = dl->pre_cmd->cmd_dma; + dl->extended->pre_ext_cmd_qty = dl->pre_cmd->num_cmds; + dl->extended->flags |= VSP1_DLH_EXT_PRE_CMD_EXEC; + + vsp1_dl_ext_cmd_fill_header(dl->pre_cmd); + } + + if (dl->post_cmd) { + dl->extended->pre_ext_cmd_plist = dl->post_cmd->cmd_dma; + dl->extended->pre_ext_cmd_qty = dl->post_cmd->num_cmds; + dl->extended->flags |= VSP1_DLH_EXT_POST_CMD_EXEC; + + vsp1_dl_ext_cmd_fill_header(dl->pre_cmd); + } } static bool vsp1_dl_list_hw_update_pending(struct vsp1_dl_manager *dlm) @@ -715,14 +777,20 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) } /* Hardware Setup */ -void vsp1_dlm_setup(struct vsp1_device *vsp1) +void vsp1_dlm_setup(struct vsp1_device *vsp1, unsigned int ind
[PATCH v2 11/11] drm: rcar-du: Support interlaced video output through vsp1
Use the newly exposed VSP1 interface to enable interlaced frame support through the VSP1 lif pipelines. Signed-off-by: Kieran Bingham --- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 + drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 5685d5af6998..9854d9deb944 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -248,6 +248,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) /* Signal polarities */ value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0) | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0) + | ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 0) | DSMR_DIPM_DISP | DSMR_CSPM; rcar_du_crtc_write(rcrtc, DSMR, value); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c260c33840b..5e47daef8bd2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -178,6 +178,9 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) }; unsigned int i; + cfg.interlaced = !!(plane->plane.state->crtc->mode.flags + & DRM_MODE_FLAG_INTERLACE); + cfg.src.left = state->state.src.x1 >> 16; cfg.src.top = state->state.src.y1 >> 16; cfg.src.width = drm_rect_width(&state->state.src) >> 16; -- git-series 0.9.1
[PATCH v2 09/11] media: vsp1: Provide support for extended command pools
VSPD and VSP-DL devices can provide extended display lists supporting extended command display list objects. These extended commands require their own dma memory areas for a header and body specific to the command type. Implement a command pool to allocate all necessary memory in a single DMA allocation to reduce pressure on the TLB, and provide convenient re-usable command objects for the entities to utilise. Signed-off-by: Kieran Bingham --- v2: - Fix spelling typo in commit message - constify, and staticify the instantiation of vsp1_extended_commands - s/autfld_cmds/autofld_cmds/ - staticify cmd pool functions (Thanks kbuild-bot) drivers/media/platform/vsp1/vsp1_dl.c | 190 +++- drivers/media/platform/vsp1/vsp1_dl.h | 3 +- 2 files changed, 193 insertions(+) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index cd91b50deed1..d8392bd866f1 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -121,6 +121,30 @@ struct vsp1_dl_body_pool { }; /** + * struct vsp1_cmd_pool - display list body pool + * @dma: DMA address of the entries + * @size: size of the full DMA memory pool in bytes + * @mem: CPU memory pointer for the pool + * @bodies: Array of DLB structures for the pool + * @free: List of free DLB entries + * @lock: Protects the pool and free list + * @vsp1: the VSP1 device + */ +struct vsp1_dl_cmd_pool { + /* DMA allocation */ + dma_addr_t dma; + size_t size; + void *mem; + + struct vsp1_dl_ext_cmd *cmds; + struct list_head free; + + spinlock_t lock; + + struct vsp1_device *vsp1; +}; + +/** * struct vsp1_dl_list - Display list * @list: entry in the display list manager lists * @dlm: the display list manager @@ -163,6 +187,7 @@ struct vsp1_dl_list { * @queued: list queued to the hardware (written to the DL registers) * @pending: list waiting to be queued to the hardware * @pool: body pool for the display list bodies + * @autofld_cmds: command pool to support auto-fld interlaced mode */ struct vsp1_dl_manager { unsigned int index; @@ -176,6 +201,7 @@ struct vsp1_dl_manager { struct vsp1_dl_list *pending; struct vsp1_dl_body_pool *pool; + struct vsp1_dl_cmd_pool *autofld_cmds; }; /* - @@ -339,6 +365,139 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data) } /* - + * Display List Extended Command Management + */ + +enum vsp1_extcmd_type { + VSP1_EXTCMD_AUTODISP, + VSP1_EXTCMD_AUTOFLD, +}; + +struct vsp1_extended_command_info { + u16 opcode; + size_t body_size; +} static const vsp1_extended_commands[] = { + [VSP1_EXTCMD_AUTODISP] = { 0x02, 96 }, + [VSP1_EXTCMD_AUTOFLD] = { 0x03, 160 }, +}; + +/** + * vsp1_dl_cmd_pool_create - Create a pool of commands from a single allocation + * @vsp1: The VSP1 device + * @type: The command pool type + * @num_commands: The quantity of commands to allocate + * + * Allocate a pool of commands each with enough memory to contain the private + * data of each command. The allocation sizes are dependent upon the command + * type. + * + * Return a pointer to a pool on success or NULL if memory can't be allocated. + */ +static struct vsp1_dl_cmd_pool * +vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum vsp1_extcmd_type type, + unsigned int num_cmds) +{ + struct vsp1_dl_cmd_pool *pool; + unsigned int i; + size_t cmd_size; + + pool = kzalloc(sizeof(*pool), GFP_KERNEL); + if (!pool) + return NULL; + + pool->cmds = kcalloc(num_cmds, sizeof(*pool->cmds), GFP_KERNEL); + if (!pool->cmds) { + kfree(pool); + return NULL; + } + + cmd_size = sizeof(struct vsp1_dl_ext_cmd_header) + + vsp1_extended_commands[type].body_size; + cmd_size = ALIGN(cmd_size, 16); + + pool->size = cmd_size * num_cmds; + pool->mem = dma_alloc_wc(vsp1->bus_master, pool->size, &pool->dma, +GFP_KERNEL); + if (!pool->mem) { + kfree(pool->cmds); + kfree(pool); + return NULL; + } + + spin_lock_init(&pool->lock); + INIT_LIST_HEAD(&pool->free); + + for (i = 0; i < num_cmds; ++i) { + struct vsp1_dl_ext_cmd *cmd = &pool->cmds[i]; + size_t cmd_offset = i * cmd_size; + size_t data_offset = sizeof(struct vsp1_dl_ext_cmd_header) + +cmd_offset; + + cmd->pool = pool; + cmd->cmd_opcode = vsp1_extended_commands[type].opcode; + + /* TODO: Auto-disp can utilise more than one command per cmd */ + cmd->num_cmds = 1;
[PATCH v2 10/11] media: vsp1: Support Interlaced display pipelines
Calculate the top and bottom fields for the interlaced frames and utilise the extended display list command feature to implement the auto-field operations. This allows the DU to update the VSP2 registers dynamically based upon the currently processing field. Signed-off-by: Kieran Bingham --- v2: - fix erroneous BIT value which enabled interlaced - fix field handling at frame_end interrupt drivers/media/platform/vsp1/vsp1_dl.c | 10 - drivers/media/platform/vsp1/vsp1_drm.c | 11 - drivers/media/platform/vsp1/vsp1_regs.h | 1 +- drivers/media/platform/vsp1/vsp1_rpf.c | 72 -- drivers/media/platform/vsp1/vsp1_rwpf.h | 1 +- include/media/vsp1.h| 1 +- 6 files changed, 93 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index d8392bd866f1..08715ce6db1e 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -889,7 +889,9 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) */ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) { + struct vsp1_device *vsp1 = dlm->vsp1; bool completed = false; + u32 status = vsp1_read(vsp1, VI6_STATUS); spin_lock(&dlm->lock); @@ -914,6 +916,14 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) goto done; /* +* Progressive streams report only TOP fields. If we have a BOTTOM +* field, we are interlaced, and expect the frame to complete on the +* next frame end interrupt. +*/ + if (status & VI6_STATUS_FLD_STD(dlm->index)) + goto done; + + /* * The device starts processing the queued display list right after the * frame end interrupt. The display list thus becomes active. */ diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 0459b970e9da..a714c53601b6 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -368,6 +368,17 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index, return -EINVAL; } + if (!(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) && cfg->interlaced) { + /* +* Interlaced support requires extended display lists to +* provide the auto-fld feature with the DU. +*/ + dev_dbg(vsp1->dev, "Interlaced unsupported on this output\n"); + return -EINVAL; + } + + rpf->interlaced = cfg->interlaced; + rpf->fmtinfo = fmtinfo; rpf->format.num_planes = fmtinfo->planes; rpf->format.plane_fmt[0].bytesperline = cfg->pitch; diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h index 43ad68ff3167..e2dffbe82809 100644 --- a/drivers/media/platform/vsp1/vsp1_regs.h +++ b/drivers/media/platform/vsp1/vsp1_regs.h @@ -31,6 +31,7 @@ #define VI6_SRESET_SRTS(n) (1 << (n)) #define VI6_STATUS 0x0038 +#define VI6_STATUS_FLD_STD(n) (1 << ((n) + 28)) #define VI6_STATUS_SYS_ACT(n) (1 << ((n) + 8)) #define VI6_WPF_IRQ_ENB(n) (0x0048 + (n) * 12) diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c index 67f2fb3e0611..86ac8f488b7a 100644 --- a/drivers/media/platform/vsp1/vsp1_rpf.c +++ b/drivers/media/platform/vsp1/vsp1_rpf.c @@ -24,6 +24,20 @@ #define RPF_MAX_WIDTH 8190 #define RPF_MAX_HEIGHT 8190 +/* Pre extended display list command data structure */ +struct vsp1_extcmd_auto_fld_body { + u32 top_y0; + u32 bottom_y0; + u32 top_c0; + u32 bottom_c0; + u32 top_c1; + u32 bottom_c1; + u32 reserved0; + u32 reserved1; +} __packed; + +#define VSP1_DL_EXT_AUTOFLD_INTBIT(0) + /* - * Device Access */ @@ -68,6 +82,14 @@ static void rpf_configure_stream(struct vsp1_entity *entity, pstride |= format->plane_fmt[1].bytesperline << VI6_RPF_SRCM_PSTRIDE_C_SHIFT; + /* +* pstride has both STRIDE_Y and STRIDE_C, but multiplying the whole +* of pstride by 2 is conveniently OK here as we are multiplying both +* values. +*/ + if (rpf->interlaced) + pstride *= 2; + vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_PSTRIDE, pstride); /* Format */ @@ -104,6 +126,9 @@ static void rpf_configure_stream(struct vsp1_entity *entity, top = compose->top; } + if (rpf->interlaced) + top /= 2; + vsp1_rpf_write(rpf, dlb, VI6_RPF_LOC, (left << VI6_RPF_LOC_HCOORD_SHIFT) | (top << VI6_RPF_LOC_VCOORD_SHIFT))
Re: [PATCH 1/3] rcar-vin: remove duplicated check of state in irq handler
Hi Kieran, Thanks for your feedback. On 2018-03-13 17:42:25 +0100, Kieran Bingham wrote: > Hi Niklas, > > Thanks for the patch series :) - I've been looking forward to seeing this one > ! > > On 10/03/18 01:09, Niklas Söderlund wrote: > > This is an error from when the driver where converted from soc-camera. > > There is absolutely no gain to check the state variable two times to be > > extra sure if the hardware is stopped. > > I'll not say this isn't a redundant check ... but isn't the check against two > different states, and thus the remaining check doesn't actually catch the case > now where state == STOPPED ? Thanks for noticing this, you are correct. I think I need to refresh my glasses subscription after missing this :-) > > (Perhaps state != RUNNING would be better ?, but I haven't checked the rest of > the code) I will respin this in a v2 and either use state != RUNNING or at least combine the two checks to prevent future embarrassing mistakes like this. > > -- > Kieran > > > > > > Signed-off-by: Niklas Söderlund > > --- > > drivers/media/platform/rcar-vin/rcar-dma.c | 6 -- > > 1 file changed, 6 deletions(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > > b/drivers/media/platform/rcar-vin/rcar-dma.c > > index 23fdff7a7370842e..b4be75d5009080f7 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > > @@ -916,12 +916,6 @@ static irqreturn_t rvin_irq(int irq, void *data) > > rvin_ack_interrupt(vin); > > handled = 1; > > > > - /* Nothing to do if capture status is 'STOPPED' */ > > - if (vin->state == STOPPED) { > > - vin_dbg(vin, "IRQ while state stopped\n"); > > - goto done; > > - } > > - > > /* Nothing to do if capture status is 'STOPPING' */ > > if (vin->state == STOPPING) { > > vin_dbg(vin, "IRQ while state stopping\n"); > > -- Regards, Niklas Söderlund
[RFC PATCH renesas-drivers] media: vsp1: vsp1_dl_cmd_pool_create can be static
Fixes: 02dcefdd58c7 ("media: vsp1: Provide support for extended command pools") Signed-off-by: Fengguang Wu --- vsp1_dl.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 6d17b8b..7b99d47 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -392,7 +392,7 @@ struct vsp1_extended_command_info { * * Return a pointer to a pool on success or NULL if memory can't be allocated. */ -struct vsp1_dl_cmd_pool * +static struct vsp1_dl_cmd_pool * vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum vsp1_extcmd_type type, unsigned int num_cmds) { @@ -450,7 +450,7 @@ vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum vsp1_extcmd_type type, return pool; } -struct vsp1_dl_ext_cmd *vsp1_dl_ext_cmd_get(struct vsp1_dl_cmd_pool *pool) +static struct vsp1_dl_ext_cmd *vsp1_dl_ext_cmd_get(struct vsp1_dl_cmd_pool *pool) { struct vsp1_dl_ext_cmd *cmd = NULL; unsigned long flags; @@ -468,7 +468,7 @@ struct vsp1_dl_ext_cmd *vsp1_dl_ext_cmd_get(struct vsp1_dl_cmd_pool *pool) return cmd; } -void vsp1_dl_ext_cmd_put(struct vsp1_dl_ext_cmd *cmd) +static void vsp1_dl_ext_cmd_put(struct vsp1_dl_ext_cmd *cmd) { unsigned long flags; @@ -483,7 +483,7 @@ void vsp1_dl_ext_cmd_put(struct vsp1_dl_ext_cmd *cmd) spin_unlock_irqrestore(&cmd->pool->lock, flags); } -void vsp1_dl_ext_cmd_pool_destroy(struct vsp1_dl_cmd_pool *pool) +static void vsp1_dl_ext_cmd_pool_destroy(struct vsp1_dl_cmd_pool *pool) { if (!pool) return;
[renesas-drivers:master 4329/4669] drivers/media/platform/vsp1/vsp1_dl.c:378:3: sparse: symbol 'vsp1_extended_commands' was not declared. Should it be static?
tree: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git master head: b67ea3b553661cb76397afbf73701c3ea14b19de commit: 02dcefdd58c734623b9caf2513316380feb9f993 [4329/4669] media: vsp1: Provide support for extended command pools reproduce: # apt-get install sparse git checkout 02dcefdd58c734623b9caf2513316380feb9f993 make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/media/platform/vsp1/vsp1_dl.c:378:3: sparse: symbol >> 'vsp1_extended_commands' was not declared. Should it be static? >> drivers/media/platform/vsp1/vsp1_dl.c:395:25: sparse: symbol >> 'vsp1_dl_cmd_pool_create' was not declared. Should it be static? >> drivers/media/platform/vsp1/vsp1_dl.c:453:24: sparse: symbol >> 'vsp1_dl_ext_cmd_get' was not declared. Should it be static? >> drivers/media/platform/vsp1/vsp1_dl.c:471:6: sparse: symbol >> 'vsp1_dl_ext_cmd_put' was not declared. Should it be static? >> drivers/media/platform/vsp1/vsp1_dl.c:486:6: sparse: symbol >> 'vsp1_dl_ext_cmd_pool_destroy' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH/RFT 1/3] thermal: rcar_thermal: add r8a77995 support
Hi Geert-san, Thanks for your review. 2018-03-12 20:02 GMT+09:00 Geert Uytterhoeven : > Hi Kaneko-san, > > On Sun, Mar 11, 2018 at 9:26 PM, Yoshihiro Kaneko > wrote: >> Add support for R-Car D3 (r8a77995) thermal sensor. >> >> Signed-off-by: Yoshihiro Kaneko > > Thanks for your patch! > >> --- a/drivers/thermal/rcar_thermal.c >> +++ b/drivers/thermal/rcar_thermal.c > >> @@ -77,13 +101,23 @@ struct rcar_thermal_priv { >> #define rcar_priv_to_dev(priv) ((priv)->common->dev) >> #define rcar_has_irq_support(priv) ((priv)->common->base) >> #define rcar_id_to_shift(priv) ((priv)->id * 8) >> -#define rcar_of_data(dev) ((unsigned >> long)of_device_get_match_data(dev)) >> -#define rcar_use_of_thermal(dev) (rcar_of_data(dev) == USE_OF_THERMAL) >> +#define rcar_of_data(dev) \ >> + ((struct rcar_thermal_chip *)of_device_get_match_data(dev)) > > I think it would be better to get rid of this macro, as > of_device_get_match_data() > walks the rcar_thermal_dt_ids[] over and over again. > > Instead, you can store a pointer to struct rcar_thermal_chip in struct > rcar_thermal_priv in rcar_thermal_probe(). I understand. I will work on this improvement. > >> +#define rcar_use_of_thermal(dev) (rcar_of_data(dev)->use_of_thermal) >> -#define USE_OF_THERMAL 1 >> static const struct of_device_id rcar_thermal_dt_ids[] = { >> - { .compatible = "renesas,rcar-thermal", }, >> - { .compatible = "renesas,rcar-gen2-thermal", .data = (void >> *)USE_OF_THERMAL }, >> + { >> + .compatible = "renesas,rcar-thermal", >> + .data = (void *)&rcar_thermal, > > This cast not needed. Yes. > >> @@ -438,6 +473,7 @@ static int rcar_thermal_probe(struct platform_device >> *pdev) >> struct rcar_thermal_priv *priv; >> struct device *dev = &pdev->dev; >> struct resource *res, *irq; >> + int nirq = rcar_of_data(dev)->type == RCAR_GEN3_THERMAL ? 2 : 1; > > Why 2? Your DT patch has 3 interrupts, which matches the datasheet. This driver uses INTDT0 and INTDT1. INTDT2 is not used. Thanks, Kaneko > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH 2/3] rcar-vin: allocate a scratch buffer at stream start
Hi Niklas, On 10/03/18 01:09, Niklas Söderlund wrote: > Before starting capturing allocate a scratch buffer which can be used by "Before starting a capture, allocate a..." (two 'ings' together doesn't sound right) > the driver to give to the hardware if no buffers are available from > userspace. The buffer is not used in this patch but prepares for future > refactoring where the scratch buffer can be used to avoid the need to > fallback on single capture mode if userspace don't queue buffers as fast s/don't/doesn't/ or alternatively s/don't/can't/ > as the VIN driver consumes them. > > Signed-off-by: Niklas Söderlund With minor comments attended to: Reviewed-by: Kieran Bingham > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 19 +++ > drivers/media/platform/rcar-vin/rcar-vin.h | 4 > 2 files changed, 23 insertions(+) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index b4be75d5009080f7..8ea73cdc9a720abe 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -1070,6 +1070,17 @@ static int rvin_start_streaming(struct vb2_queue *vq, > unsigned int count) > unsigned long flags; > int ret; > > + /* Allocate scratch buffer. */ > + vin->scratch = dma_alloc_coherent(vin->dev, vin->format.sizeimage, > + &vin->scratch_phys, GFP_KERNEL); > + if (!vin->scratch) { > + spin_lock_irqsave(&vin->qlock, flags); > + return_all_buffers(vin, VB2_BUF_STATE_QUEUED); > + spin_unlock_irqrestore(&vin->qlock, flags); > + vin_err(vin, "Failed to allocate scratch buffer\n"); > + return -ENOMEM; > + } > + > sd = vin_to_source(vin); > v4l2_subdev_call(sd, video, s_stream, 1); > > @@ -1085,6 +1096,10 @@ static int rvin_start_streaming(struct vb2_queue *vq, > unsigned int count) > > spin_unlock_irqrestore(&vin->qlock, flags); > > + if (ret) > + dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, > + vin->scratch_phys); > + > return ret; > } > > @@ -1135,6 +1150,10 @@ static void rvin_stop_streaming(struct vb2_queue *vq) > > /* disable interrupts */ > rvin_disable_interrupts(vin); > + > + /* Free scratch buffer. */ > + dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, > + vin->scratch_phys); > } > > static const struct vb2_ops rvin_qops = { > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h > b/drivers/media/platform/rcar-vin/rcar-vin.h > index 5382078143fb3869..11a981d707c7ca47 100644 > --- a/drivers/media/platform/rcar-vin/rcar-vin.h > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h > @@ -102,6 +102,8 @@ struct rvin_graph_entity { > * > * @lock:protects @queue > * @queue: vb2 buffers queue > + * @scratch: cpu address for scratch buffer > + * @scratch_phys:pysical address of the scratch buffer s/pysical/physical/ > * > * @qlock: protects @queue_buf, @buf_list, @continuous, @sequence > * @state > @@ -130,6 +132,8 @@ struct rvin_dev { > > struct mutex lock; > struct vb2_queue queue; > + void *scratch; > + dma_addr_t scratch_phys; > > spinlock_t qlock; > struct vb2_v4l2_buffer *queue_buf[HW_BUFFER_NUM]; >
Re: [PATCH 1/3] rcar-vin: remove duplicated check of state in irq handler
Hi Niklas, Thanks for the patch series :) - I've been looking forward to seeing this one ! On 10/03/18 01:09, Niklas Söderlund wrote: > This is an error from when the driver where converted from soc-camera. > There is absolutely no gain to check the state variable two times to be > extra sure if the hardware is stopped. I'll not say this isn't a redundant check ... but isn't the check against two different states, and thus the remaining check doesn't actually catch the case now where state == STOPPED ? (Perhaps state != RUNNING would be better ?, but I haven't checked the rest of the code) -- Kieran > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index 23fdff7a7370842e..b4be75d5009080f7 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -916,12 +916,6 @@ static irqreturn_t rvin_irq(int irq, void *data) > rvin_ack_interrupt(vin); > handled = 1; > > - /* Nothing to do if capture status is 'STOPPED' */ > - if (vin->state == STOPPED) { > - vin_dbg(vin, "IRQ while state stopped\n"); > - goto done; > - } > - > /* Nothing to do if capture status is 'STOPPING' */ > if (vin->state == STOPPING) { > vin_dbg(vin, "IRQ while state stopping\n"); >
Re: [PATCH 3/3] arm64: dts: renesas: r8a77995: add thermal device support
Hi Geert-san, Thanks for your review. 2018-03-12 19:56 GMT+09:00 Geert Uytterhoeven : > Hi Kaneko-san, > > On Sun, Mar 11, 2018 at 9:26 PM, Yoshihiro Kaneko > wrote: >> Signed-off-by: Yoshihiro Kaneko > > Thanks for your patch! > >> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> @@ -402,5 +402,36 @@ >> #phy-cells = <0>; >> status = "disabled"; >> }; >> + >> + thermal: thermal@e61f { > > According to the Hard User Manual rev. 0.80, the base address is e619? You are correct. I will update this patch to fix it. > >> + compatible = "renesas,thermal-r8a77995", >> +"renesas,rcar-thermal"; > > I would drop the fallback property, cfr. my comments on the DT binding > patch. Yes. I will do it. > >> + reg = <0 0xe61f 0 0x10>, <0 0xe61f0100 0 0x38>; > > 0xe619... (twice) I will fix it. Thanks, Kaneko > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH 2/3] dt-bindings: thermal: rcar-thermal: add R8A77995 support
Hi Geert-san, Thank you for your review! 2018-03-12 19:13 GMT+09:00 Geert Uytterhoeven : > Hi Kaneko-san, > > On Sun, Mar 11, 2018 at 9:26 PM, Yoshihiro Kaneko > wrote: >> Signed-off-by: Yoshihiro Kaneko > > Thanks for your patch! > >> --- a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt >> +++ b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt >> @@ -12,6 +12,7 @@ Required properties: >> - "renesas,thermal-r8a7791" (R-Car M2-W) >> - "renesas,thermal-r8a7792" (R-Car V2H) >> - "renesas,thermal-r8a7793" (R-Car M2-N) >> + - "renesas,thermal-r8a77995" (R-Car D3) > > OK for this change. > > I think the paragraph above needs some clarification, too: > > Required properties: > - compatible: "renesas,thermal-", >"renesas,rcar-gen2-thermal" (with > thermal-zone) or >"renesas,rcar-thermal" (without > thermal-zone) as fallback. > > Your DT update for D3 contains the thermal-zone, but it is not R-Car Gen2, > while you declare it compatible with "renesas,rcar-thermal". > > Probably we can just drop the fallback for D3 (and V3M later)? It sounds good. I will update DT to drop the fallback for D3. > > And the paragraph about interrupts need an update, too, as D3 has more > than one interrupt: > > Option properties: > - interrupts: use interrupt I will update this paragraph. Thanks, Kaneko > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH] clk: renesas: cpg-mssr: r8a77965: Replace DU2 clock
Hi Jacopo, On Tue, Mar 13, 2018 at 4:18 PM, Jacopo Mondi wrote: > R-Car M3-N does not have DU2 unit but DU3 instead. Fix the module clocks > definition to reflect that. > > Reported-by: Yoshihiro Shimoda > Signed-off-by: Jacopo Mondi Thank you! Fixes: 7ce36da900c0a2ff ("clk: renesas: cpg-mssr: Add support for R-Car M3-N") Queued in clk-renesas-for-v4.17. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
[PATCH] clk: renesas: cpg-mssr: r8a77965: Replace DU2 clock
R-Car M3-N does not have DU2 unit but DU3 instead. Fix the module clocks definition to reflect that. Reported-by: Yoshihiro Shimoda Signed-off-by: Jacopo Mondi --- drivers/clk/renesas/r8a77965-cpg-mssr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/renesas/r8a77965-cpg-mssr.c b/drivers/clk/renesas/r8a77965-cpg-mssr.c index 41e506a..b1acfb6 100644 --- a/drivers/clk/renesas/r8a77965-cpg-mssr.c +++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c @@ -173,7 +173,7 @@ static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = { DEF_MOD("hsusb",704,R8A77965_CLK_S3D4), DEF_MOD("csi20",714,R8A77965_CLK_CSI0), DEF_MOD("csi40",716,R8A77965_CLK_CSI0), - DEF_MOD("du2", 722,R8A77965_CLK_S2D1), + DEF_MOD("du3", 721,R8A77965_CLK_S2D1), DEF_MOD("du1", 723,R8A77965_CLK_S2D1), DEF_MOD("du0", 724,R8A77965_CLK_S2D1), DEF_MOD("lvds", 727,R8A77965_CLK_S2D1), -- 2.7.4
renesas-drivers-2018-03-13-v4.16-rc5
I have pushed renesas-drivers-2018-03-13-v4.16-rc5 to https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git This tree is meant to ease development of platform support and drivers for Renesas ARM SoCs. It is created by merging (a) the for-next branches of various subsystem trees and (b) branches with driver code submitted or planned for submission to maintainers into the development branch of Simon Horman's renesas.git tree. Today's version is based on renesas-devel-20180308-v4.16-rc4. Included branches with driver code: - clk-renesas - sh-pfc - topic/rcar-genpd-wakeup-v4 - topic/rcar-gen2-wdt-v6 - topic/bd9571-ddr-backup-mode-driver-v2 - topic/bd9571-ddr-backup-mode-dt-v2 - git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git#topic/cpufreq-automatic - git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git#topic/ipmmu-driver-v6 - git://linuxtv.org/pinchartl/media.git#drm-du-iommu-v1-20171115 - git://linuxtv.org/pinchartl/media.git#tags/vsp1-discom-v1-20171215 - git://linuxtv.org/pinchartl/media.git#tags/drm-next-colorkey-v1-20171215 - git://git.ragnatech.se/linux#for-renesas-drivers - git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#tags/vsp1/tlb-optimise/v7 - git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#d3/i2c-conflict/drm - git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#d3/i2c-conflict/media - git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#tags/vsp1/du/interlaced/v1 Included fixes: - [LOCAL] arm64: defconfig: Update renesas_defconfig Included subsystem trees: - git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git#linux-next - git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git#clk-next - git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git#for-next - git://git.infradead.org/users/dedekind/l2-mtd-2.6.git#master - git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git#master - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git#tty-next - git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#i2c/for-next - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git#master - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git#usb-next - git://people.freedesktop.org/~airlied/linux#drm-next - git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git#next - git://linuxtv.org/mchehab/media-next.git#master - git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git#next - git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git#for-next - git://git.linaro.org/people/daniel.lezcano/linux.git#clockevents/next - git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git#testing/next - git://git.infradead.org/users/vkoul/slave-dma.git#next - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git#staging-next - git://git.armlinux.org.uk/~rmk/linux-arm.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git#next - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git#for-next - git://git.infradead.org/users/jcooper/linux.git#irqchip/for-next - git://github.com/bzolnier/linux.git#fbdev-for-next - git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git#for-next - git://www.linux-watchdog.org/linux-watchdog-next.git#master - git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git#for-next - git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git#for-next/core - git://anongit.freedesktop.org/drm/drm-misc#for-linux-next - git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next - git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git#next - git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git#next - git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git#for-mfd-next - git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git#for-next Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
[PATCH v3 0/3] drm: Add Thine THC63LVD1024 LVDS decoder bridge
Hello, opposed to v2, this version drops the "transparent LVDS decoder" and provides support for the Thine THC63LVD1024 chip only. I removed all references to "lvds-decoder" and made driver and bindings specific for the above mentioned chip. Andrzej: Bindings now describe 2 available inputs (Port@0 mandatory, Port@1 optional) and 2 possible outputs (Port@2 mandatory, Port@3 optional). The driver still takes only Port@0 and Port@2 into account. This leaves out discussions on how support bridges with multiple input streams within the DRM framework from this series. Simon: Please drop the previous bindings proposal you marked as "deferred" as that one is superseded by this new one. I still plan to use this series to propose an API to propagate formats through bridges, which is not possible at the moment, if I got things right. Thanks j v2 -> v3: - Drop support for "lvds-decoder" and make the driver THC63LVD1024 specific -- Rework bindings to describe multiple input/output ports -- Rename driver and remove "lvds-decoder" references -- Rework Eagle DTS to use new bindings v1 -> v2: - Drop support for THC63LVD1024 Jacopo Mondi (3): dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder drm: bridge: Add thc63lvd1024 LVDS decoder driver arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle .../bindings/display/bridge/thine,thc63lvd1024.txt | 63 ++ arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 31 ++- drivers/gpu/drm/bridge/Kconfig | 10 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 239 + 5 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c -- 2.7.4
[PATCH v3 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder
Document Thine THC63LVD1024 LVDS decoder device tree bindings. Signed-off-by: Jacopo Mondi --- .../bindings/display/bridge/thine,thc63lvd1024.txt | 63 ++ 1 file changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt new file mode 100644 index 000..5b5afcd --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -0,0 +1,63 @@ +Thine Electronics THC63LVD1024 LVDS decoder +--- + +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS streams +to parallel data outputs. The chip supports single/dual input/output modes, +handling up to two two input LVDS stream and up to two digital CMOS/TTL outputs. + +Required properties: +- compatible: Shall be "thine,thc63lvd1024", + +Optional properties: +- vcc-supply: Power supply for TTL output and digital circuitry +- cvcc-supply: Power supply for TTL CLOCKOUT signal +- lvcc-supply: Power supply for LVDS inputs +- pvcc-supply: Power supply for PLL circuitry +- pwnd-gpios: Power down GPIO signal. Active low. +- oe-gpios: Output enable GPIO signal. Active high. + +The THC63LVD1024 video port connections are modeled according +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt + +Required video port nodes: +- Port@0: First LVDS input port +- Port@2: First digital CMOS/TTL parallel output + +Optional video port nodes: +- Port@1: Second LVDS input port +- Port@3: Second digital CMOS/TTL parallel output + +Example: + + + thc63lvd1024: lvds-decoder { + compatible = "thine,thc63lvd1024"; + + vcc-supply = <®_lvds_vcc>; + lvcc-supply = <®_lvds_lvcc>; + + pwdn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + lvds_dec_in_0: endpoint { + remote-endpoint = <&lvds_out>; + }; + }; + + port@2{ + reg = <2>; + + lvds_dec_out_2: endpoint { + remote-endpoint = <&adv7511_in>; + }; + + }; + + }; + }; -- 2.7.4
[PATCH v3 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle
The R-Car V3M Eagle board includes a transparent THC63LVD1024 LVDS decoder, connected to the on-chip LVDS encoder output on one side and to HDMI encoder ADV7511w on the other one. As the decoder does not need any configuration it has been so-far omitted from DTS. Now that a driver is available, describe it in DT as well. Signed-off-by: Jacopo Mondi --- arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 33 +++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index c0fd144..69f43b8 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts @@ -42,6 +42,33 @@ }; }; }; + + thc63lvd1024: lvds-decoder { + compatible = "thine,thc63lvd1024"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + thc63lvd1024_in_0: endpoint { + remote-endpoint = <&lvds0_out>; + }; + }; + + port@2{ + reg = <2>; + + thc63lvd1024_out_2: endpoint { + remote-endpoint = <&adv7511_in>; + }; + + }; + + }; + }; }; &avb { @@ -98,7 +125,7 @@ port@0 { reg = <0>; adv7511_in: endpoint { - remote-endpoint = <&lvds0_out>; + remote-endpoint = <&thc63lvd1024_out_2>; }; }; @@ -152,8 +179,8 @@ ports { port@1 { - endpoint { - remote-endpoint = <&adv7511_in>; + lvds0_out: endpoint { + remote-endpoint = <&thc63lvd1024_in_0>; }; }; }; -- 2.7.4
[PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver
Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel output decoder. Signed-off-by: Jacopo Mondi --- drivers/gpu/drm/bridge/Kconfig| 7 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 241 ++ 3 files changed, 249 insertions(+) create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3b99d5a..facf6bd 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -92,6 +92,13 @@ config DRM_SII9234 It is an I2C driver, that detects connection of MHL bridge and starts encapsulation of HDMI signal. +config DRM_THINE_THC63LVD1024 + tristate "Thine THC63LVD1024 LVDS decoder bridge" + depends on OF + select DRM_PANEL_BRIDGE + ---help--- + Thine THC63LVD1024 LVDS decoder bridge driver. + config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 373eb28..fd90b16 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o obj-$(CONFIG_DRM_SII902X) += sii902x.o obj-$(CONFIG_DRM_SII9234) += sii9234.o +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c new file mode 100644 index 000..4b059c0 --- /dev/null +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -0,0 +1,241 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * THC63LVD1024 LVDS to parallel data DRM bridge driver. + * + * Copyright (C) 2018 Jacopo Mondi + */ + +#include +#include +#include + +#include +#include +#include + +static const char * const thc63_reg_names[] = { + "vcc", "lvcc", "pvcc", "cvcc", }; + +struct thc63_dev { + struct device *dev; + + struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)]; + + struct gpio_desc *pwdn; + struct gpio_desc *oe; + + struct drm_bridge bridge; + struct drm_bridge *next; +}; + +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) +{ + return container_of(bridge, struct thc63_dev, bridge); +} + +static int thc63_attach(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + + return drm_bridge_attach(bridge->encoder, thc63->next, bridge); +} + +static void thc63_enable(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + struct regulator *vcc; + unsigned int i; + int ret; + + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) { + vcc = thc63->vccs[i]; + if (vcc) { + ret = regulator_enable(vcc); + if (ret) + goto error_vcc_enable; + } + } + + if (thc63->pwdn) + gpiod_set_value(thc63->pwdn, 0); + + if (thc63->oe) + gpiod_set_value(thc63->oe, 1); + + return; + +error_vcc_enable: + dev_err(thc63->dev, "Failed to enable regulator %u\n", i); +} + +static void thc63_disable(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + struct regulator *vcc; + unsigned int i; + int ret; + + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) { + vcc = thc63->vccs[i]; + if (vcc) { + ret = regulator_disable(vcc); + if (ret) + goto error_vcc_disable; + } + } + + if (thc63->pwdn) + gpiod_set_value(thc63->pwdn, 1); + + if (thc63->oe) + gpiod_set_value(thc63->oe, 0); + + return; + +error_vcc_disable: + dev_err(thc63->dev, "Failed to disable regulator %u\n", i); +} + +struct drm_bridge_funcs thc63_bridge_func = { + .attach = thc63_attach, + .enable = thc63_enable, + .disable = thc63_disable, +}; + +static int thc63_parse_dt(struct thc63_dev *thc63) +{ + struct device_node *thc63_out; + struct device_node *remote; + int ret = 0; + + thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node, 2, -1); + if (!thc63_out) { + dev_err(thc63->dev, "Missing endpoint in Port@2\n"); + return -ENODEV; + } + + remote = of_graph_get_remote_port_parent(thc63_out); + if (!remote) { + dev_err(thc63->dev, "Endpoint in Port@2 unconnected\n"); + ret = -ENODEV; + goto error_put_out_node; + } + + if (!of_device_is_available(remote)) { +
Re: [PATCH 02/11] media: vsp1: use kernel __packed for structures
Hi David, On 13/03/18 13:38, David Laight wrote: > From: Kieran Bingham [mailto:kieran.bingham+rene...@ideasonboard.com] >> On 13/03/18 11:20, David Laight wrote: >>> From: Kieran Bingham Sent: 09 March 2018 22:04 The kernel provides a __packed definition to abstract away from the compiler specific attributes tag. Convert all packed structures in VSP1 to use it. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_dl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 37e2c984fbf3..382e45c2054e 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -29,19 +29,19 @@ struct vsp1_dl_header_list { u32 num_bytes; u32 addr; -} __attribute__((__packed__)); +} __packed; struct vsp1_dl_header { u32 num_lists; struct vsp1_dl_header_list lists[8]; u32 next_header; u32 flags; -} __attribute__((__packed__)); +} __packed; struct vsp1_dl_entry { u32 addr; u32 data; -} __attribute__((__packed__)); +} __packed; >>> >>> Do these structures ever actually appear in misaligned memory? >>> If they don't then they shouldn't be marked 'packed'. >> >> I believe the declaration is to ensure that the struct definition is not >> altered >> by the compiler as these structures specifically define the layout of how the >> memory is read by the VSP1 hardware. > > The C language and ABI define structure layouts. > >> Certainly 2 u32's sequentially stored in a struct are unlikely to be moved or >> rearranged by the compiler (though I believe it would be free to do so if it >> chose without this attribute), but I think the declaration shows the intent >> of >> the memory structure. > > The language requires the fields be in order and the ABI stops the compiler > adding 'random' padding. > >> Isn't this a common approach throughout the kernel when dealing with hardware >> defined memory structures ? > > Absolutely not. > __packed tells the compiler that the structure might be on any address > boundary. > On many architectures this means the compiler must use byte accesses with > shifts > and ors for every access. > The most a hardware defined structure will have is a compile-time assert > that it is the correct size (to avoid silly errors from changes). Ok - interesting, I see what you're saying - and certainly don't want the compiler to be performing byte accesses on the structures unnecessarily. I'm trying to distinguish the difference here. Is the single point that __packed causes byte-access, where as __attribute__((__packed__)); does not? Looking at the GCC docs [0]: I see that __attribute__((__packed__)) tells the compiler that the "structure or union is placed to minimize the memory required". However, the keil compiler docs[1] do certainly declare that __packed causes byte alignment. I was confused/thrown off here by the Kernel defining __packed as __attribute__((packed)) at [2]. Do __attribute__((packed)) and __attribute__((__packed__)) differ ? In which case, from what I've read so far I wish "__packed" was "__unaligned"... [0] https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-packed-type-attribute [1] http://www.keil.com/support/man/docs/armcc/armcc_chr1359124230195.htm [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/compiler-gcc.h?h=v4.16-rc5#n92 Regards Kieran > David >
Re: [PATCH v6 01/26] ARM: shmobile: Add watchdog support
Hi Fabrizio, On Wed, Feb 28, 2018 at 6:40 PM, Fabrizio Castro wrote: > On R-Car Gen2 and RZ/G1 platforms, we use the SBAR registers to make non > boot CPUs run a routine designed to bring up SMP and deal with hot plug. > The value contained in the SBAR registers is not initialized by a WDT > triggered reset, which means that after a WDT triggered reset we jump > to the SMP bring up routine, preventing the system from executing the > bootrom code. > > The purpose of this patch is to jump to the bootrom code in case of a > WDT triggered reset, and keep the SMP functionality untouched. > In order to tell if the code had been called due to the WDT overflowing > we are testing WOVF from register RWTCSRA. > > The new function shmobile_boot_vector_gen2 isn't replacing > shmobile_boot_vector for backward compatibility reasons. The kernel > will install the best option (either shmobile_boot_vector or > shmobile_boot_vector_gen2) to ICRAM1 after parsing the device tree, > according to the amount of memory available. > > Since shmobile_boot_vector has become bigger, "reg" property of nodes > compatible with "renesas,smp-sram" now need to be set to a value > greater or equal to "<0 0x60>". > > Signed-off-by: Fabrizio Castro > Signed-off-by: Ramesh Shanmugasundaram > > Reviewed-by: Geert Uytterhoeven > --- > v5->v6: > * taken ifdefs out as per Geert's suggestion. My intention was to remove the #ifdefs from the header file only, not from arch/arm/mach-shmobile/headsmp.S, as the latter is used for other SoC families, too. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
RE: [PATCH 02/11] media: vsp1: use kernel __packed for structures
From: Kieran Bingham [mailto:kieran.bingham+rene...@ideasonboard.com] > On 13/03/18 11:20, David Laight wrote: > > From: Kieran Bingham > >> Sent: 09 March 2018 22:04 > >> The kernel provides a __packed definition to abstract away from the > >> compiler specific attributes tag. > >> > >> Convert all packed structures in VSP1 to use it. > >> > >> Signed-off-by: Kieran Bingham > >> --- > >> drivers/media/platform/vsp1/vsp1_dl.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > >> b/drivers/media/platform/vsp1/vsp1_dl.c > >> index 37e2c984fbf3..382e45c2054e 100644 > >> --- a/drivers/media/platform/vsp1/vsp1_dl.c > >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c > >> @@ -29,19 +29,19 @@ > >> struct vsp1_dl_header_list { > >>u32 num_bytes; > >>u32 addr; > >> -} __attribute__((__packed__)); > >> +} __packed; > >> > >> struct vsp1_dl_header { > >>u32 num_lists; > >>struct vsp1_dl_header_list lists[8]; > >>u32 next_header; > >>u32 flags; > >> -} __attribute__((__packed__)); > >> +} __packed; > >> > >> struct vsp1_dl_entry { > >>u32 addr; > >>u32 data; > >> -} __attribute__((__packed__)); > >> +} __packed; > > > > Do these structures ever actually appear in misaligned memory? > > If they don't then they shouldn't be marked 'packed'. > > I believe the declaration is to ensure that the struct definition is not > altered > by the compiler as these structures specifically define the layout of how the > memory is read by the VSP1 hardware. The C language and ABI define structure layouts. > Certainly 2 u32's sequentially stored in a struct are unlikely to be moved or > rearranged by the compiler (though I believe it would be free to do so if it > chose without this attribute), but I think the declaration shows the intent of > the memory structure. The language requires the fields be in order and the ABI stops the compiler adding 'random' padding. > Isn't this a common approach throughout the kernel when dealing with hardware > defined memory structures ? Absolutely not. __packed tells the compiler that the structure might be on any address boundary. On many architectures this means the compiler must use byte accesses with shifts and ors for every access. The most a hardware defined structure will have is a compile-time assert that it is the correct size (to avoid silly errors from changes). David
Re: [PATCH 02/11] media: vsp1: use kernel __packed for structures
Hi David, On 13/03/18 11:20, David Laight wrote: > From: Kieran Bingham >> Sent: 09 March 2018 22:04 >> The kernel provides a __packed definition to abstract away from the >> compiler specific attributes tag. >> >> Convert all packed structures in VSP1 to use it. >> >> Signed-off-by: Kieran Bingham >> --- >> drivers/media/platform/vsp1/vsp1_dl.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c >> b/drivers/media/platform/vsp1/vsp1_dl.c >> index 37e2c984fbf3..382e45c2054e 100644 >> --- a/drivers/media/platform/vsp1/vsp1_dl.c >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c >> @@ -29,19 +29,19 @@ >> struct vsp1_dl_header_list { >> u32 num_bytes; >> u32 addr; >> -} __attribute__((__packed__)); >> +} __packed; >> >> struct vsp1_dl_header { >> u32 num_lists; >> struct vsp1_dl_header_list lists[8]; >> u32 next_header; >> u32 flags; >> -} __attribute__((__packed__)); >> +} __packed; >> >> struct vsp1_dl_entry { >> u32 addr; >> u32 data; >> -} __attribute__((__packed__)); >> +} __packed; > > Do these structures ever actually appear in misaligned memory? > If they don't then they shouldn't be marked 'packed'. I believe the declaration is to ensure that the struct definition is not altered by the compiler as these structures specifically define the layout of how the memory is read by the VSP1 hardware. Certainly 2 u32's sequentially stored in a struct are unlikely to be moved or rearranged by the compiler (though I believe it would be free to do so if it chose without this attribute), but I think the declaration shows the intent of the memory structure. Isn't this a common approach throughout the kernel when dealing with hardware defined memory structures ? Regards -- Kieran > > David >
RE: [PATCH 02/11] media: vsp1: use kernel __packed for structures
From: Kieran Bingham > Sent: 09 March 2018 22:04 > The kernel provides a __packed definition to abstract away from the > compiler specific attributes tag. > > Convert all packed structures in VSP1 to use it. > > Signed-off-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_dl.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index 37e2c984fbf3..382e45c2054e 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -29,19 +29,19 @@ > struct vsp1_dl_header_list { > u32 num_bytes; > u32 addr; > -} __attribute__((__packed__)); > +} __packed; > > struct vsp1_dl_header { > u32 num_lists; > struct vsp1_dl_header_list lists[8]; > u32 next_header; > u32 flags; > -} __attribute__((__packed__)); > +} __packed; > > struct vsp1_dl_entry { > u32 addr; > u32 data; > -} __attribute__((__packed__)); > +} __packed; Do these structures ever actually appear in misaligned memory? If they don't then they shouldn't be marked 'packed'. David
Re: [PATCH V3] PCI: rcar: Use runtime PM to control controller clock
Hi Marek, On Fri, Nov 10, 2017 at 10:54:11PM +0100, Marek Vasut wrote: > From: Dien Pham > > The controller clock can be switched off during suspend/resume, > let runtime PM take care of that. > > Signed-off-by: Dien Pham > Signed-off-by: Hien Dang > Signed-off-by: Marek Vasut > Cc: Geert Uytterhoeven > Cc: Phil Edworthy > Cc: Simon Horman > Cc: Wolfram Sang > Cc: linux-renesas-soc@vger.kernel.org > To: linux-...@vger.kernel.org > --- > V2: - Reorder the fail path in rcar_pcie_probe() to cater for the > reordering of function calls in probe > - Dispose of fail_clk in rcar_pcie_get_resources() > V3: - Fix up the failpath in probe function > --- > drivers/pci/host/pcie-rcar.c | 40 > 1 file changed, 12 insertions(+), 28 deletions(-) I would like to ask you if this patch is still applicable and if so whether you want me to apply it stand-alone (ie does it depend on other patches ?), please let me know. Thanks, Lorenzo > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 12796eccb2be..e00f865952d5 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -145,7 +145,6 @@ struct rcar_pcie { > void __iomem*base; > struct list_headresources; > int root_bus_nr; > - struct clk *clk; > struct clk *bus_clk; > struct rcar_msi msi; > }; > @@ -917,24 +916,14 @@ static int rcar_pcie_get_resources(struct rcar_pcie > *pcie) > if (IS_ERR(pcie->base)) > return PTR_ERR(pcie->base); > > - pcie->clk = devm_clk_get(dev, "pcie"); > - if (IS_ERR(pcie->clk)) { > - dev_err(dev, "cannot get platform clock\n"); > - return PTR_ERR(pcie->clk); > - } > - err = clk_prepare_enable(pcie->clk); > - if (err) > - return err; > - > pcie->bus_clk = devm_clk_get(dev, "pcie_bus"); > if (IS_ERR(pcie->bus_clk)) { > dev_err(dev, "cannot get pcie bus clock\n"); > - err = PTR_ERR(pcie->bus_clk); > - goto fail_clk; > + return PTR_ERR(pcie->bus_clk); > } > err = clk_prepare_enable(pcie->bus_clk); > if (err) > - goto fail_clk; > + return err; > > i = irq_of_parse_and_map(dev->of_node, 0); > if (!i) { > @@ -956,8 +945,6 @@ static int rcar_pcie_get_resources(struct rcar_pcie *pcie) > > err_map_reg: > clk_disable_unprepare(pcie->bus_clk); > -fail_clk: > - clk_disable_unprepare(pcie->clk); > > return err; > } > @@ -1125,22 +1112,22 @@ static int rcar_pcie_probe(struct platform_device > *pdev) > > rcar_pcie_parse_request_of_pci_ranges(pcie); > > + pm_runtime_enable(pcie->dev); > + err = pm_runtime_get_sync(pcie->dev); > + if (err < 0) { > + dev_err(pcie->dev, "pm_runtime_get_sync failed\n"); > + goto err_pm_disable; > + } > + > err = rcar_pcie_get_resources(pcie); > if (err < 0) { > dev_err(dev, "failed to request resources: %d\n", err); > - goto err_free_bridge; > + goto err_pm_put; > } > > err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node); > if (err) > - goto err_free_bridge; > - > - pm_runtime_enable(dev); > - err = pm_runtime_get_sync(dev); > - if (err < 0) { > - dev_err(dev, "pm_runtime_get_sync failed\n"); > - goto err_pm_disable; > - } > + goto err_pm_put; > > /* Failure to get a link might just be that no cards are inserted */ > hw_init_fn = of_device_get_match_data(dev); > @@ -1172,13 +1159,10 @@ static int rcar_pcie_probe(struct platform_device > *pdev) > > err_pm_put: > pm_runtime_put(dev); > - > err_pm_disable: > pm_runtime_disable(dev); > - > -err_free_bridge: > - pci_free_host_bridge(bridge); > pci_free_resource_list(&pcie->resources); > + pci_free_host_bridge(bridge); > > return err; > } > -- > 2.11.0 >
Re: [Qemu-devel] [PATCH 2/3] nvram: at24c: prevent segfault by checking "rom-size"
On 03/12/2018 10:42 PM, Wolfram Sang wrote: > The value for "rom-size" is used as a divisor, so it must not be 0 or it > will segfault. A size of 0 wouldn't make sense as well. > > Signed-off-by: Wolfram Sang > --- > hw/nvram/eeprom_at24c.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c > index 8507516b7e..d82710e1df 100644 > --- a/hw/nvram/eeprom_at24c.c > +++ b/hw/nvram/eeprom_at24c.c > @@ -120,6 +120,11 @@ int at24c_eeprom_init(I2CSlave *i2c) > { > EEPROMState *ee = AT24C_EE(i2c); > > +if (!ee->rsize) { > +ERR("rom-size not allowed to be 0\n"); This might be more useful: error_report("Minimum rom-size is %u", AT24C_ROMSIZE_MIN); > +exit(1); > +} > + > ee->mem = g_malloc0(ee->rsize); > > if (ee->blk) { >
Re: [Qemu-devel] [PATCH 1/3] nvram: at24c: remove doubled prefix for ERR
Hi Wolfram, On 03/12/2018 10:42 PM, Wolfram Sang wrote: > The ERR macro already has the TYPE_AT24C_EE prefix, no need to repeat in > the error message. > > Signed-off-by: Wolfram Sang > --- > hw/nvram/eeprom_at24c.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c > index 5494351976..8507516b7e 100644 > --- a/hw/nvram/eeprom_at24c.c > +++ b/hw/nvram/eeprom_at24c.c > @@ -60,8 +60,7 @@ int at24c_eeprom_event(I2CSlave *s, enum i2c_event event) > if (ee->blk && ee->changed) { > int len = blk_pwrite(ee->blk, 0, ee->mem, ee->rsize, 0); > if (len != ee->rsize) { > -ERR(TYPE_AT24C_EE > -" : failed to write backing file\n"); > +ERR("failed to write backing file\n"); printf/fprintf are deprecated, since you are modifying this file can you use a newer API, "qemu/error-report.h" for example. > } > DPRINTK("Wrote to backing file\n"); DPRINTK() can be converted to tracepoint. > } > @@ -127,7 +126,7 @@ int at24c_eeprom_init(I2CSlave *i2c) > int64_t len = blk_getlength(ee->blk); > > if (len != ee->rsize) { > -ERR(TYPE_AT24C_EE " : Backing file size %lu != %u\n", > +ERR("Backing file size %lu != %u\n", > (unsigned long)len, (unsigned)ee->rsize); > exit(1); > } > @@ -135,8 +134,7 @@ int at24c_eeprom_init(I2CSlave *i2c) > if (blk_set_perm(ee->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, > BLK_PERM_ALL, &error_fatal) < 0) > { > -ERR(TYPE_AT24C_EE > -" : Backing file incorrect permission\n"); > +ERR("Backing file incorrect permission\n"); > exit(1); > } > } > @@ -158,8 +156,7 @@ void at24c_eeprom_reset(DeviceState *state) > int len = blk_pread(ee->blk, 0, ee->mem, ee->rsize); > > if (len != ee->rsize) { > -ERR(TYPE_AT24C_EE > -" : Failed initial sync with backing file\n"); > +ERR("Failed initial sync with backing file\n"); > } > DPRINTK("Reset read backing file\n"); > } >
Re: [Qemu-devel] [PATCH 3/3] nvram: at24c: use a sane default for "rom-size"
Hi Wolfram, On 03/12/2018 10:42 PM, Wolfram Sang wrote: > 0 as "rom-size" doesn't make much sense, let's use the smallest 24cXX > which has 128 byte. > > Signed-off-by: Wolfram Sang > --- > hw/nvram/eeprom_at24c.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c > index d82710e1df..de988f8d07 100644 > --- a/hw/nvram/eeprom_at24c.c > +++ b/hw/nvram/eeprom_at24c.c > @@ -168,7 +168,7 @@ void at24c_eeprom_reset(DeviceState *state) > } > > static Property at24c_eeprom_props[] = { > -DEFINE_PROP_UINT32("rom-size", EEPROMState, rsize, 0), > +DEFINE_PROP_UINT32("rom-size", EEPROMState, rsize, 128), This patch should goes before your 2/3 in your series. Can you add a #define for this value? Such AT24C_ROMSIZE_MIN. With a #define you can add: Reviewed-by: Philippe Mathieu-Daudé > DEFINE_PROP_BOOL("writable", EEPROMState, writable, true), > DEFINE_PROP_DRIVE("drive", EEPROMState, blk), > DEFINE_PROP_END_OF_LIST() >
Re: [PATCH 09/11] media: vsp1: Provide support for extended command pools
Hi Jacopo, On 12/03/18 16:30, jacopo mondi wrote: > Hi Kieran, > just one small thing I noticed below... > > On Fri, Mar 09, 2018 at 10:04:07PM +, Kieran Bingham wrote: >> VSPD and VSP-DL devices can provide extended display lists supporting >> extended command display list objects. >> >> These extended commands require their own dma memory areas for a header >> and body specific to the command type. >> >> Implement a command pool to allocate all necessary memory in a single >> DMA allocation to reduce pressure on the TLB, and provide convenvient >> re-usable command objects for the entities to utilise. >> >> Signed-off-by: Kieran Bingham >> --- >> drivers/media/platform/vsp1/vsp1_dl.c | 189 +++- >> drivers/media/platform/vsp1/vsp1_dl.h | 3 +- >> 2 files changed, 192 insertions(+) >> >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c >> b/drivers/media/platform/vsp1/vsp1_dl.c >> index 36440a2a2c8b..6d17b8bfa21c 100644 >> --- a/drivers/media/platform/vsp1/vsp1_dl.c >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c >> @@ -121,6 +121,30 @@ struct vsp1_dl_body_pool { >> }; >> >> /** >> + * struct vsp1_cmd_pool - display list body pool >> + * @dma: DMA address of the entries >> + * @size: size of the full DMA memory pool in bytes >> + * @mem: CPU memory pointer for the pool >> + * @bodies: Array of DLB structures for the pool >> + * @free: List of free DLB entries >> + * @lock: Protects the pool and free list >> + * @vsp1: the VSP1 device >> + */ >> +struct vsp1_dl_cmd_pool { >> +/* DMA allocation */ >> +dma_addr_t dma; >> +size_t size; >> +void *mem; >> + >> +struct vsp1_dl_ext_cmd *cmds; >> +struct list_head free; >> + >> +spinlock_t lock; >> + >> +struct vsp1_device *vsp1; >> +}; >> + >> +/** >> * struct vsp1_dl_list - Display list >> * @list: entry in the display list manager lists >> * @dlm: the display list manager >> @@ -176,6 +200,7 @@ struct vsp1_dl_manager { >> struct vsp1_dl_list *pending; >> >> struct vsp1_dl_body_pool *pool; >> +struct vsp1_dl_cmd_pool *autfld_cmds; >> }; >> >> /* >> - >> @@ -339,6 +364,139 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 >> reg, u32 data) >> } >> >> /* >> - >> + * Display List Extended Command Management >> + */ >> + >> +enum vsp1_extcmd_type { >> +VSP1_EXTCMD_AUTODISP, >> +VSP1_EXTCMD_AUTOFLD, >> +}; >> + >> +struct vsp1_extended_command_info { >> +u16 opcode; >> +size_t body_size; >> +} vsp1_extended_commands[] = { >> +[VSP1_EXTCMD_AUTODISP] = { 0x02, 96 }, >> +[VSP1_EXTCMD_AUTOFLD] = { 0x03, 160 }, >> +}; > > How about making this one static and const, since it does not get > modified? Good spot. Certainly. This is just a static descriptor table of the extended command parameter sizes, so it should not change. (but could be added to in later hardware operations I presume). Cheers Kieran > > Thanks >j >
[PATCH v2 2/2] pwm: rcar: add suspend/resume support
This patch adds suspend/resume support for Renesas PWM driver. Signed-off-by: Yoshihiro Shimoda --- drivers/pwm/pwm-rcar.c | 42 ++ 1 file changed, 42 insertions(+) diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c index b942010..34069ae 100644 --- a/drivers/pwm/pwm-rcar.c +++ b/drivers/pwm/pwm-rcar.c @@ -254,11 +254,53 @@ static int rcar_pwm_remove(struct platform_device *pdev) }; MODULE_DEVICE_TABLE(of, rcar_pwm_of_table); +#ifdef CONFIG_PM_SLEEP +static struct pwm_device *rcar_pwm_dev_to_pwm_dev(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev); + struct pwm_chip *chip = &rcar_pwm->chip; + + return &chip->pwms[0]; +} + +static int rcar_pwm_suspend(struct device *dev) +{ + struct pwm_device *pwm = rcar_pwm_dev_to_pwm_dev(dev); + + if (!test_bit(PWMF_REQUESTED, &pwm->flags)) + return 0; + + pm_runtime_put(dev); + + return 0; +} + +static int rcar_pwm_resume(struct device *dev) +{ + struct pwm_device *pwm = rcar_pwm_dev_to_pwm_dev(dev); + + if (!test_bit(PWMF_REQUESTED, &pwm->flags)) + return 0; + + pm_runtime_get_sync(dev); + + rcar_pwm_config(pwm->chip, pwm, pwm->state.duty_cycle, + pwm->state.period); + if (pwm_is_enabled(pwm)) + rcar_pwm_enable(pwm->chip, pwm); + + return 0; +} +#endif /* CONFIG_PM_SLEEP */ +static SIMPLE_DEV_PM_OPS(rcar_pwm_pm_ops, rcar_pwm_suspend, rcar_pwm_resume); + static struct platform_driver rcar_pwm_driver = { .probe = rcar_pwm_probe, .remove = rcar_pwm_remove, .driver = { .name = "pwm-rcar", + .pm = &rcar_pwm_pm_ops, .of_match_table = of_match_ptr(rcar_pwm_of_table), } }; -- 1.9.1
[PATCH v2 1/2] pwm: rcar: Use PM Runtime to control module clock
From: Hien Dang Runtime PM API (pm_runtime_get_sync/pm_runtime_put) should be used to control module clock instead of clk_prepare_enable and clk_disable_unprepare. Signed-off-by: Hien Dang Signed-off-by: Yoshihiro Shimoda Reviewed-by: Geert Uytterhoeven --- drivers/pwm/pwm-rcar.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c index 1c85ecc..b942010 100644 --- a/drivers/pwm/pwm-rcar.c +++ b/drivers/pwm/pwm-rcar.c @@ -134,16 +134,12 @@ static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns, static int rcar_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) { - struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip); - - return clk_prepare_enable(rp->clk); + return pm_runtime_get_sync(chip->dev); } static void rcar_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) { - struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip); - - clk_disable_unprepare(rp->clk); + pm_runtime_put(chip->dev); } static int rcar_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, -- 1.9.1
[PATCH v2 0/2] pwm: rcar: Add suspend/resume support and cleanup for runtime_pm
This patch set improves power management for Renesas PWM driver. Changes from v1: - Add Reviewed-by Geert-san in patch 1 (Thanks!). - Check a condition in suspend/resume if requested or not in patch 2. Hien Dang (1): pwm: rcar: Use PM Runtime to control module clock Yoshihiro Shimoda (1): pwm: rcar: add suspend/resume support drivers/pwm/pwm-rcar.c | 50 -- 1 file changed, 44 insertions(+), 6 deletions(-) -- 1.9.1
Re: [PATCH 2/2] pwm: rcar: add suspend/resume support
Hi Shimoda-san, On Tue, Mar 13, 2018 at 8:04 AM, Yoshihiro Shimoda wrote: >> From: Geert Uytterhoeven, Sent: Friday, March 9, 2018 9:53 PM >> On Fri, Mar 9, 2018 at 12:54 PM, Yoshihiro Shimoda >> wrote: >> > This patch adds suspend/resume support for Renesas PWM driver. >> > >> > Signed-off-by: Yoshihiro Shimoda >> >> Thanks for your patch! > > Thank you for your review! > >> > --- a/drivers/pwm/pwm-rcar.c >> > +++ b/drivers/pwm/pwm-rcar.c >> > @@ -254,11 +254,38 @@ static int rcar_pwm_remove(struct platform_device >> > *pdev) >> > }; >> > MODULE_DEVICE_TABLE(of, rcar_pwm_of_table); >> > >> > +#ifdef CONFIG_PM_SLEEP >> > +static int rcar_pwm_suspend(struct device *dev) >> > +{ >> > + pm_runtime_put(dev); >> > + >> > + return 0; >> > +} >> > + >> > +static int rcar_pwm_resume(struct device *dev) >> > +{ >> > + struct platform_device *pdev = to_platform_device(dev); >> > + struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev); >> > + struct pwm_chip *chip = &rcar_pwm->chip; >> > + struct pwm_device *pwm = &chip->pwms[0]; >> > + >> > + pm_runtime_get_sync(dev); >> >> This enables the module clock unconditionally, so you can restore the >> register values below... >> >> > + rcar_pwm_config(chip, pwm, pwm->state.duty_cycle, >> > pwm->state.period); >> > + if (pwm_is_enabled(pwm)) >> > + rcar_pwm_enable(chip, pwm); >> >> ... but shouldn't you disable the clock again if the PWM is not in use, >> like below? >> >> else >> pm_runtime_put(dev); > > Thank you for the pointing out. > I realize that this suspend/resume function should check PWMF_REQUESTED > condition > because this driver enables the clock after the .request(rcar_pwm_request) > was called. > So, I will fix this patch in v2. OK. > Also, reducing power point of view, this driver should call not > pm_runtime_get_sync/put() > in _request/_free(). But, I would like to implement such code as a next step > :) That makes sense. I was already wondering whether those are the best locations to call the pm_runtime_*() functions, but I'm not sufficiently familiar with the PWM subsystem. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
RE: [PATCH 2/2] pwm: rcar: add suspend/resume support
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Friday, March 9, 2018 9:53 PM > > Hi Shimoda-san, > > On Fri, Mar 9, 2018 at 12:54 PM, Yoshihiro Shimoda > wrote: > > This patch adds suspend/resume support for Renesas PWM driver. > > > > Signed-off-by: Yoshihiro Shimoda > > Thanks for your patch! Thank you for your review! > > --- a/drivers/pwm/pwm-rcar.c > > +++ b/drivers/pwm/pwm-rcar.c > > @@ -254,11 +254,38 @@ static int rcar_pwm_remove(struct platform_device > > *pdev) > > }; > > MODULE_DEVICE_TABLE(of, rcar_pwm_of_table); > > > > +#ifdef CONFIG_PM_SLEEP > > +static int rcar_pwm_suspend(struct device *dev) > > +{ > > + pm_runtime_put(dev); > > + > > + return 0; > > +} > > + > > +static int rcar_pwm_resume(struct device *dev) > > +{ > > + struct platform_device *pdev = to_platform_device(dev); > > + struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev); > > + struct pwm_chip *chip = &rcar_pwm->chip; > > + struct pwm_device *pwm = &chip->pwms[0]; > > + > > + pm_runtime_get_sync(dev); > > This enables the module clock unconditionally, so you can restore the > register values below... > > > + rcar_pwm_config(chip, pwm, pwm->state.duty_cycle, > > pwm->state.period); > > + if (pwm_is_enabled(pwm)) > > + rcar_pwm_enable(chip, pwm); > > ... but shouldn't you disable the clock again if the PWM is not in use, > like below? > > else > pm_runtime_put(dev); Thank you for the pointing out. I realize that this suspend/resume function should check PWMF_REQUESTED condition because this driver enables the clock after the .request(rcar_pwm_request) was called. So, I will fix this patch in v2. Also, reducing power point of view, this driver should call not pm_runtime_get_sync/put() in _request/_free(). But, I would like to implement such code as a next step :) Best regards, Yoshihiro Shimoda > Likewise, I think the call to "pm_runtime_put(dev)" in rcar_pwm_suspend() > should be protected by a pwm_is_enabled(pwm) check. > > > + return 0; > > +} > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds