[PATCH v2 0/2] rcar-vin: always run in continues mode

[Niklas Söderlund]

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

[Niklas Söderlund]

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(>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 = >vb;
-   list_del_init(to_buf_list(vbuf));
-   vin->queue_buf[slot] = vbuf;
-
-   /* Setup DMA */
-   phys_addr_top = vb2_dma_contig_plane_dma_addr(>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(>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 = >vb;
+   list_del_init(to_buf_list(vbuf));
+   vin->queue_buf[slot] = vbuf;
+
+   /* Setup DMA */
+ 

[PATCH v2 1/2] rcar-vin: allocate a scratch buffer at stream start

[Niklas Söderlund]

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,
+ >scratch_phys, GFP_KERNEL);
+   if (!vin->scratch) {
+   spin_lock_irqsave(>qlock, flags);
+   return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
+   spin_unlock_irqrestore(>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(>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

[Kieran Bingham]

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

[Kieran Bingham]

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 

Re: [Qemu-devel] [RFC PATCH] i2c: device passthrough HACK(!) & evaluation

[no-reply]

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"

[Wolfram Sang]

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

[Wolfram Sang]

> > -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

[Simon Horman]

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: 

[RFC PATCH] i2c: device passthrough HACK(!) & evaluation

[Wolfram Sang]

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) 

[PATCH] pinctrl: sh-pfc: r8a77970: add EtherAVB pin groups

[Sergei Shtylyov]

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

[jacopo mondi]

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,
> +   >scratch_phys, GFP_KERNEL);
> + if (!vin->scratch) {
> + spin_lock_irqsave(>qlock, flags);
> + return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
> + spin_unlock_irqrestore(>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(>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

[Sergei Shtylyov]

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>;
 };
 
+ {
+   scif0_pins: scif0 {
+   groups = "scif0_data";
+   function = "scif0";
+   };
+};
+
  {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
status = "okay";
 };

Re: [PATCH] arm64: dts: renesas: r8a7795: Add missing SYS-DMAC2 dmas

[Geert Uytterhoeven]

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"

[Geert Uytterhoeven]

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"

[Simon Horman]

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

[Simon Horman]

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

[Hans Verkuil]

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(>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 = >vb;
> - list_del_init(to_buf_list(vbuf));
> - vin->queue_buf[slot] = vbuf;
> -
> - /* Setup DMA */
> - phys_addr_top = vb2_dma_contig_plane_dma_addr(>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(>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 

Re: [PATCH 0/3] Add R8A77980/Condor PFC support

[Simon Horman]

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

[Simon Horman]

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.

[PATCH v2 00/11] R-Car DU Interlaced support through VSP1

[Kieran Bingham]

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

[Kieran Bingham]

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

[Kieran Bingham]

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

[Kieran Bingham]

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, >chain, chain)
-   __vsp1_dl_list_put(dl_child);
+   list_for_each_entry(dl_next, >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(>chain));
 
-   list_for_each_entry(dl_child, >chain, chain) {
-   bool last = list_is_last(_child->chain, >chain);
+   list_for_each_entry(dl_next, >chain, chain) {
+   bool last = list_is_last(_next->chain, >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

[Kieran Bingham]

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

[Kieran Bingham]

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(>list, >free);
}
-- 
git-series 0.9.1

[PATCH v2 06/11] media: vsp1: Provide VSP1 feature helper macro

[Kieran Bingham]

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(>brs->entity.list_dev, >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(>bru->entity.list_dev, >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(>hst->entity.list_dev, >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)
  >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, 

[PATCH v2 07/11] media: vsp1: Use header display lists for all WPF outputs linked to the DU

[Kieran Bingham]

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(>refcnt);
 
list_add_tail(>list, >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(>chain, >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 

[PATCH v2 08/11] media: vsp1: Add support for extended display list headers

[Kieran Bingham]

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 

[PATCH v2 11/11] drm: rcar-du: Support interlaced video output through vsp1

[Kieran Bingham]

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.src) >> 16;
-- 
git-series 0.9.1

[PATCH v2 09/11] media: vsp1: Provide support for extended command pools

[Kieran Bingham]

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, >dma,
+GFP_KERNEL);
+   if (!pool->mem) {
+   kfree(pool->cmds);
+   kfree(pool);
+   return NULL;
+   }
+
+   spin_lock_init(>lock);
+   INIT_LIST_HEAD(>free);
+
+   for (i = 0; i < num_cmds; ++i) {
+   struct vsp1_dl_ext_cmd *cmd = >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 */
+   

[PATCH v2 10/11] media: vsp1: Support Interlaced display pipelines

[Kieran Bingham]

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(>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) |
 

Re: [PATCH 1/3] rcar-vin: remove duplicated check of state in irq handler

[Niklas Söderlund]

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

[kbuild test robot]

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(>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?

[kbuild test robot]

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

[Yoshihiro Kaneko]

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 *)_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 = >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

[Kieran Bingham]

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,
> +   >scratch_phys, GFP_KERNEL);
> + if (!vin->scratch) {
> + spin_lock_irqsave(>qlock, flags);
> + return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
> + spin_unlock_irqrestore(>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(>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

[Kieran Bingham]

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

[Yoshihiro Kaneko]

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

[Yoshihiro Kaneko]

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

[Geert Uytterhoeven]

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

[Jacopo Mondi]

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

[Geert Uytterhoeven]

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

[Jacopo Mondi]

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

[Jacopo Mondi]

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 = < 15 GPIO_ACTIVE_LOW>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   lvds_dec_in_0: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@2{
+   reg = <2>;
+
+   lvds_dec_out_2: endpoint {
+   remote-endpoint = <_in>;
+   };
+
+   };
+
+   };
+   };
-- 
2.7.4

[PATCH v3 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle

[Jacopo Mondi]

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 = <_out>;
+   };
+   };
+
+   port@2{
+   reg = <2>;
+
+   thc63lvd1024_out_2: endpoint {
+   remote-endpoint = <_in>;
+   };
+
+   };
+
+   };
+   };
 };
 
  {
@@ -98,7 +125,7 @@
port@0 {
reg = <0>;
adv7511_in: endpoint {
-   remote-endpoint = <_out>;
+   remote-endpoint = <_out_2>;
};
};
 
@@ -152,8 +179,8 @@
 
ports {
port@1 {
-   endpoint {
-   remote-endpoint = <_in>;
+   lvds0_out: endpoint {
+   remote-endpoint = <_in_0>;
};
};
};
-- 
2.7.4

[PATCH v3 2/3] drm: bridge: Add thc63lvd1024 LVDS decoder driver

[Jacopo Mondi]

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;
+  

Re: [PATCH 02/11] media: vsp1: use kernel __packed for structures

[Kieran Bingham]

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

[Geert Uytterhoeven]

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

[David Laight]

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

[Kieran Bingham]

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

[David Laight]

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

[Lorenzo Pieralisi]

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(>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"

[Philippe Mathieu-Daudé]

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

[Philippe Mathieu-Daudé]

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, _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"

[Philippe Mathieu-Daudé]

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

[Kieran Bingham]

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

[Yoshihiro Shimoda]

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 = _pwm->chip;
+
+   return >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, >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, >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 = _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

[Yoshihiro Shimoda]

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

[Yoshihiro Shimoda]

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

[Geert Uytterhoeven]

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 = _pwm->chip;
>> > +   struct pwm_device *pwm = >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

[Yoshihiro Shimoda]

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 = _pwm->chip;
> > +   struct pwm_device *pwm = >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