Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Daniel Vetter]

On Fri, Jul 14, 2017 at 1:43 AM, Laurent Pinchart
 wrote:
>> Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
>> start to CRTC resume") changed the order of the plane commit and CRTC
>> enable operations to accommodate the runtime PM requirements. However,
>> this introduced corruption in the first displayed frame, as the CRTC is
>> now enabled without any plane configured. On Gen2 hardware the first
>> frame will be black and likely unnoticed, but on Gen3 hardware we end up
>> starting the display before the VSP compositor, which is more
>> noticeable.
>>
>> To fix this, revert the order of the commit operations back, and handle
>> runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
>> helper operation handlers.
>
> I believe that the "runtime PM" order is problematic in most drivers. The
> problem usually goes unnoticed as most monitors will not even display the
> first frame, and I assume many devices will just output it black, but it's an
> issue nonetheless.
>
> Note that my driver hasn't lost the "runtime PM" requirements, so I had to
> support them with the "standard" order. The best way I've found was to runtime
> resume in the one of .atomic_begin() and .enable() that is run first. Not very
> neat, as similar code would be needed in most drivers. I wonder whether it
> wouldn't be useful to add resume/suspend helper callbacks for the CRTC.

I discussed this with Laurent and explained that "first black frame"
is exactly what i915 does. I'd say given special customer requests we
don't yet have to bother with this in general ...

Wrt adding more hooks for rpm: I honestly don't like that, because
then someone else wants to add a hook for clocks, or for the sideband
and then we have a horror show of hooks where every driver uses just a
very small subset. The point of atomic helpers is to make them
modular, if one part doesn't fit, just redo in your driver. Goal
should be that shared parts are good for about 90% of
drivers/use-cases (maybe even less, there's so many special
cases).

tldr; I still think the _runtime_pm variant is the recommended way to do this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v2 00/14] Renesas R-Car VSP: Add H3 ES2.0 support

[Laurent Pinchart]

Hi Kieran,

On Thursday 13 Jul 2017 13:25:55 Kieran Bingham wrote:
> Hi Laurent,
> 
> Thank you for these patches, bringing life to the outputs of my ES2.0 target
> board.
> 
> I have tested them on my board, and including the VSP unit test suite, and
> kmscube utilities.
> 
> Feel free to add a Tested-by: Kieran Bingham
>  to all of the patches if you
> desire, and I'm working through the individual reviews.
> 
> Oh - except now I've just said that - I did some extra testing with both
> HDMI and VGA output connected and ran
>kmstest --flip --sync
> 
> This was soon followed by a kernel error trace [0] shown below.

:-/

> However replicating this is possibly more complicated than just running
> kmstest --flip --sync ... I've had to do various iterations of running
> with/without {flip, sync} but I have reproduced 'issues' about 3 times now.
> 
> I think the key thing is in the VGA connection or regarding trying to output
> to both.
> 
> Interestingly, I haven't been able to make this happen on the ES1.0 platform
> as yet... though --flip --sync is quicker to fail with 'Flip Commit failed:
> -16" there...
> 
> For reference this was tested on your
> pinchartl-media/drm/next/h3-es2/merged, branch which I don't believe has
> the recent work on not needing to wait for a final vblank on shutdown. So
> it is quite possible that the issue I am seeing is simply a symptom of that
> separately repaired issue.

That's possible, or it could also be related to the vblank race and the 
"[PATCH] drm: rcar-du: Wait for flip completion instead of vblank in commit 
tail" patch that I've just posted.

> On that basis I've left my comment regarding my Tested-by: tag above as I
> suspect that this issue I've hit could likely be separate and already
> resolved.
> 
> I'll try to add those patches to this tree to see if the issue resolves
> itself...

I've updated my drm/next/h3-es2/merged branch, could you please test it ? I've 
included "[PATCH] drm: rcar-du: Wait for flip completion instead of vblank in 
commit tail", you may want to try and revert it if it causes issues.

> Regardless of that, this series conflicts with my current developments,
> therefore I will likely rebase my work on top of this series. I don't need
> an immutable branch, but please do let me know if this series changes :-)

I will.

> [0] : Kernel log snippet posted below:
> 
> [  597.471369] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  607.711346] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  607.711354] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  607.749585] vsp1 fea2.vsp: failed to reset wpf.1
> [  617.951311] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  618.055358] vsp1 fea2.vsp: failed to reset wpf.1
> [  618.060498] vsp1 fea2.vsp: DRM pipeline stop timeout
> [  628.831762] vsp1 fea2.vsp: failed to reset wpf.1
> [  638.943315] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  649.183313] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  677.753465] vsp1 fea2.vsp: failed to reset wpf.1
> [  687.839322] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
> [CRTC:57:crtc-3] flip_done timed out
> [  687.935288] vsp1 fea2.vsp: failed to reset wpf.1
> [  687.940360] vsp1 fea2.vsp: DRM pipeline stop timeout
> [  687.945939] Unable to handle kernel NULL pointer dereference at virtual
> address 
> [  687.954206] pgd = ff8009a0a000
> [  687.957680] [] *pgd=00073fffe003, *pud=00073fffe003,
> *pmd=
> [  687.966068] Internal error: Oops: 9606 [#1] PREEMPT SMP
> [  687.971690] Modules linked in:
> [  687.974777] CPU: 0 PID: 10426 Comm: kmstest Not tainted
> 4.12.0-rc5-01343-g1bc824acf27c #5
> [  687.983019] Hardware name: Renesas Salvator-X 2nd version board based on
> r8a7795 ES2.0+ (DT)
> [  687.991525] task: ffc6f97b4080 task.stack: ffc6f51dc000
> [  687.997502] PC is at __media_pipeline_stop+0x24/0xd0
> [  688.002507] LR is at media_pipeline_stop+0x34/0x48
> [  688.007336] pc : [] lr : [] pstate:
> 6145 [  688.014790] sp : ffc6f51df780
> [  688.018129] x29: ffc6f51df780 x28: 
> [  688.023490] x27: 0038 x26: ff8008960088
> [  688.028850] x25: ffc6f98bcb10 x24: ffc6f98b8818
> [  688.034210] x23: 0001 x22: ffc6f9ff1998
> [  688.039570] x21: ffc6f98bc080 x20: 
> [  688.044929] x19: 0008 x18: 0010
> [  688.050288] x17: 007f9bc9e1c8 x16: ff8008165bb8
> [  688.055648] x15: ff80899bb63f x14: 0006
> [  688.061007] x13: ffc6faac6750 x12: ff80087bd078
> [  688.066366] x11:  x10: 

[PATCH v2.1 08/14] v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances

[Laurent Pinchart]

New Gen3 SoCs come with two new VSP2 variants names VSP2-BS and VSP2-DL,
as well as a new VSP2-D variant on V3M and V3H SoCs. Add new entries for
them in the VSP device info table.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
---
Changes since v2:

- Added VSP1_HAS_WPF_VFLIP to VSP2-BS
---
 drivers/media/platform/vsp1/vsp1_drv.c  | 24 
 drivers/media/platform/vsp1/vsp1_regs.h | 15 +--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index 6a9aeb71aedf..d1682a9719af 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -710,6 +710,14 @@ static const struct vsp1_device_info vsp1_device_infos[] = 
{
.num_bru_inputs = 5,
.uapi = true,
}, {
+   .version = VI6_IP_VERSION_MODEL_VSPBS_GEN3,
+   .model = "VSP2-BS",
+   .gen = 3,
+   .features = VSP1_HAS_BRS | VSP1_HAS_WPF_VFLIP,
+   .rpf_count = 2,
+   .wpf_count = 1,
+   .uapi = true,
+   }, {
.version = VI6_IP_VERSION_MODEL_VSPD_GEN3,
.model = "VSP2-D",
.gen = 3,
@@ -717,6 +725,22 @@ static const struct vsp1_device_info vsp1_device_infos[] = 
{
.rpf_count = 5,
.wpf_count = 2,
.num_bru_inputs = 5,
+   }, {
+   .version = VI6_IP_VERSION_MODEL_VSPD_V3,
+   .model = "VSP2-D",
+   .gen = 3,
+   .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,
+   .rpf_count = 5,
+   .wpf_count = 1,
+   .num_bru_inputs = 5,
+   }, {
+   .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
+   .model = "VSP2-DL",
+   .gen = 3,
+   .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,
+   .rpf_count = 5,
+   .wpf_count = 2,
+   .num_bru_inputs = 5,
},
 };
 
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h 
b/drivers/media/platform/vsp1/vsp1_regs.h
index 744217e020b9..ab439a60a100 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -699,9 +699,20 @@
 #define VI6_IP_VERSION_MODEL_VSPBD_GEN3(0x15 << 8)
 #define VI6_IP_VERSION_MODEL_VSPBC_GEN3(0x16 << 8)
 #define VI6_IP_VERSION_MODEL_VSPD_GEN3 (0x17 << 8)
+#define VI6_IP_VERSION_MODEL_VSPD_V3   (0x18 << 8)
+#define VI6_IP_VERSION_MODEL_VSPDL_GEN3(0x19 << 8)
+#define VI6_IP_VERSION_MODEL_VSPBS_GEN3(0x1a << 8)
 #define VI6_IP_VERSION_SOC_MASK(0xff << 0)
-#define VI6_IP_VERSION_SOC_H   (0x01 << 0)
-#define VI6_IP_VERSION_SOC_M   (0x02 << 0)
+#define VI6_IP_VERSION_SOC_H2  (0x01 << 0)
+#define VI6_IP_VERSION_SOC_V2H (0x01 << 0)
+#define VI6_IP_VERSION_SOC_V3M (0x01 << 0)
+#define VI6_IP_VERSION_SOC_M2  (0x02 << 0)
+#define VI6_IP_VERSION_SOC_M3W (0x02 << 0)
+#define VI6_IP_VERSION_SOC_V3H (0x02 << 0)
+#define VI6_IP_VERSION_SOC_H3  (0x03 << 0)
+#define VI6_IP_VERSION_SOC_D3  (0x04 << 0)
+#define VI6_IP_VERSION_SOC_M3N (0x04 << 0)
+#define VI6_IP_VERSION_SOC_E3  (0x04 << 0)
 
 /* 
-
  * RPF CLUT Registers
-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: Setup planes before enabling CRTC to avoid flicker

[Laurent Pinchart]

Hi Kieran,

On Wednesday 12 Jul 2017 17:35:53 Kieran Bingham wrote:
> On 28/06/17 19:50, Laurent Pinchart wrote:
> > Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
> > start to CRTC resume") changed the order of the plane commit and CRTC
> > enable operations to accommodate the runtime PM requirements. However,
> > this introduced corruption in the first displayed frame, as the CRTC is
> > now enabled without any plane configured. On Gen2 hardware the first
> > frame will be black and likely unnoticed, but on Gen3 hardware we end up
> > starting the display before the VSP compositor, which is more
> > noticeable.
> > 
> > To fix this, revert the order of the commit operations back, and handle
> > runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
> > helper operation handlers.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> I only have code reduction or comment suggestions below - so either with or
> without those changes, feel free to add my:
> 
> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 66 +
> >  drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  4 +--
> >  drivers/gpu/drm/rcar-du/rcar_du_kms.c  |  2 +-
> >  3 files changed, 43 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 6b5219ef0ad2..76cdb88b2b8e
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c

[snip]

> > @@ -467,6 +461,18 @@ static void rcar_du_crtc_start(struct rcar_du_crtc
> > *rcrtc)
> > /* Start with all planes disabled. */
> > rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
> > 
> > +   /* Enable the VSP compositor. */
> > +   if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
> > +   rcar_du_vsp_enable(rcrtc);
> > +
> > +   /* Turn vertical blanking interrupt reporting on. */
> > +   drm_crtc_vblank_on(>crtc);
> > +}
> > +
> > +static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
> > +{
> > +   bool interlaced;
> > +
> > /* Select master sync mode. This enables display operation in master
> 
> Are we close enough here to fix this multiline comment style ?
> (Not worth doing unless the patch is respun for other reasons ...)
> 
> Actually - there are a lot in this file, so it would be better to do them
> all in one hit/patch at a point of least conflicts ...

Done :-) I actually had such a patch in my tree before receiving your comment.

> >  * sync mode (with the HSYNC and VSYNC signals configured as outputs and
> >  * actively driven).

[snip]

> > @@ -546,12 +538,10 @@ void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc)
> > return;
> > 
> > rcar_du_crtc_get(rcrtc);
> > -   rcar_du_crtc_start(rcrtc);
> > +   rcar_du_crtc_setup(rcrtc);
> 
> Every call to _setup is immediately prefixed by a call to _get()
> 
> Could the _get() be done in the _setup() for code reduction?
> 
> I'm entirely open to that not happening here as it might be preferred to
> keep the _get() and _start() separate for style purposes.

Please see below.

> > /* Commit the planes state. */
> > -   if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
> > -   rcar_du_vsp_enable(rcrtc);
> > -   } else {
> > +   if (!rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
> > for (i = 0; i < rcrtc->group->num_planes; ++i) {
> > struct rcar_du_plane *plane = >group->planes[i];
> > 

[snip]

> > @@ -601,6 +602,19 @@ static void rcar_du_crtc_atomic_begin(struct drm_crtc
> > *crtc,
> >  {
> > struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> > 
> > +   WARN_ON(!crtc->state->enable);
> 
> Is this necessary if it's handled by the rcrtc->initialized flag? or is it
> so that we find out if it happens ?
> 
> (Or is this due to the re-ordering of the _commit_tail() function below?)

It's to find out whether it happens. Before this patch the plane update
occurred after enabling the CRTC (through
drm_atomic_helper_commit_modeset_enables()). With this patch the CRTC can be
disabled at the hardware level, but only if it will be enabled right after
plane update. The state->enable flag should thus always be true, and I added
a WARN_ON to ensure that. I'd like to keep it a bit, we can remove it after
running more tests.

> > +
> > +   /*
> > +* If a mode set is in progress we can be called with the CRTC disabled.
> > +* We then need to first setup the CRTC in order to configure planes.
> > +* The .atomic_enable() handler will notice and skip the CRTC setup.
> > +*/
> 
> I'm assuming this comment is the reason for the WARN_ON above ...
> 
> > +   if (!rcrtc->initialized) {
> > +   rcar_du_crtc_get(rcrtc);
> > +   rcar_du_crtc_setup(rcrtc);
> > +  

Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Thursday 13 Jul 2017 16:41:13 Maxime Ripard wrote:
> The current drm_atomic_helper_commit_tail helper works only if the CRTC is
> accessible, and documents an alternative implementation that is supposed to
> be used if that happens.
> 
> That implementation is then duplicated by some drivers. Instead of
> documenting it, let's implement an helper that all the relevant users can
> use directly.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c| 47 +++
>  drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-

I've submitted "[PATCH] drm: rcar-du: Setup planes before enabling CRTC to 
avoid flicker" that changes the rcar-du implementation to the standard 
disable/update planes/enable order, so I'd appreciate if you could drop the 
rcar-du part of this patch to avoid conflicts.

This being said, the reason why I switched back from the "runtime PM" to the 
"standard" order is probably of interest to you. Quoting the commit message,

> Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
> start to CRTC resume") changed the order of the plane commit and CRTC
> enable operations to accommodate the runtime PM requirements. However,
> this introduced corruption in the first displayed frame, as the CRTC is
> now enabled without any plane configured. On Gen2 hardware the first
> frame will be black and likely unnoticed, but on Gen3 hardware we end up
> starting the display before the VSP compositor, which is more
> noticeable.
> 
> To fix this, revert the order of the commit operations back, and handle
> runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
> helper operation handlers.

I believe that the "runtime PM" order is problematic in most drivers. The 
problem usually goes unnoticed as most monitors will not even display the 
first frame, and I assume many devices will just output it black, but it's an 
issue nonetheless.

Note that my driver hasn't lost the "runtime PM" requirements, so I had to 
support them with the "standard" order. The best way I've found was to runtime 
resume in the one of .atomic_begin() and .enable() that is run first. Not very 
neat, as similar code would be needed in most drivers. I wonder whether it 
wouldn't be useful to add resume/suspend helper callbacks for the CRTC.

>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +--
>  include/drm/drm_atomic_helper.h|  1 +-
>  5 files changed, 36 insertions(+), 78 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: Setup planes before enabling CRTC to avoid flicker

[Laurent Pinchart]

Hi Kieran,

On Thursday 13 Jul 2017 16:51:18 Kieran Bingham wrote:
> Hi Laurent,
> 
> I've just seen Maxime's latest series "[PATCH 0/4] drm/sun4i: Fix a register
> access bug" and it relates directly to a comment I had in this patch:
> On 12/07/17 17:35, Kieran Bingham wrote:
>
> > On 28/06/17 19:50, Laurent Pinchart wrote:
> >> Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
> >> start to CRTC resume") changed the order of the plane commit and CRTC
> >> enable operations to accommodate the runtime PM requirements. However,
> >> this introduced corruption in the first displayed frame, as the CRTC is
> >> now enabled without any plane configured. On Gen2 hardware the first
> >> frame will be black and likely unnoticed, but on Gen3 hardware we end up
> >> starting the display before the VSP compositor, which is more
> >> noticeable.
> >> 
> >> To fix this, revert the order of the commit operations back, and handle
> >> runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
> >> helper operation handlers.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> 
> > 
> > I only have code reduction or comment suggestions below - so either with
> > or without those changes, feel free to add my:
> > 
> > Reviewed-by: Kieran Bingham 
> > 
> >> ---
> >> 
> >>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 66 +---
> >>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  4 +--
> >>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  |  2 +-
> >>  3 files changed, 43 insertions(+), 29 deletions(-)

[snip]

> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> >> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 82b978a5dae6..c2f382feca07
> >> 100644
> >> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> >> @@ -255,9 +255,9 @@ static void rcar_du_atomic_commit_tail(struct
> >> drm_atomic_state *old_state)>> 
> >>/* Apply the atomic update. */
> >>drm_atomic_helper_commit_modeset_disables(dev, old_state);
> >> -  drm_atomic_helper_commit_modeset_enables(dev, old_state);
> >>drm_atomic_helper_commit_planes(dev, old_state,
> >>DRM_PLANE_COMMIT_ACTIVE_ONLY);
> > 
> > Except for DRM_PLANE_COMMIT_ACTIVE_ONLY, this function now looks very much
> > like the default drm_atomic_helper_commit_tail() code.
> > 
> > Reading around other uses /variants of commit_tail() style functions in
> > other drivers has left me confused as to how the ordering affects things
> > here.
> > 
> > Could be worth adding a comment at least to describe why we can't use the
> > default helper...
> 
> Or better still ... Use Maxime's new :
> 
> [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for
> runtime_pm users

Note that Maxime's patch implements the commit tail as

drm_atomic_helper_commit_modeset_disables(dev, old_state);
drm_atomic_helper_commit_modeset_enables(dev, old_state);
drm_atomic_helper_commit_planes(dev, old_state,
DRM_PLANE_COMMIT_ACTIVE_ONLY);

while this patches moves the drm_atomic_helper_commit_planes() back between 
drm_atomic_helper_commit_modeset_disables() and 
drm_atomic_helper_commit_modeset_enables().

> >> +  drm_atomic_helper_commit_modeset_enables(dev, old_state);
> >> 
> >>drm_atomic_helper_commit_hw_done(old_state);
> >>drm_atomic_helper_wait_for_vblanks(dev, old_state);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 08/14] v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances

[Laurent Pinchart]

Hi Kieran,

On Thursday 13 Jul 2017 18:49:19 Kieran Bingham wrote:
> On 26/06/17 19:12, Laurent Pinchart wrote:
> > New Gen3 SoCs come with two new VSP2 variants names VSP2-BS and VSP2-DL,
> > as well as a new VSP2-D variant on V3M and V3H SoCs. Add new entries for
> > them in the VSP device info table.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Code in the patch looks OK - but I can't see where the difference between
> the horizontal widths are supported between VSPD H3/VC
> 
> I see this in the datasheet: (32.1.1.6 in this particular part)
> 
> Direct connection to display module
> — Supporting 4096 pixels in horizontal direction [R-Car H3/R-Car M3-W/ R-Car
> M3-N]
> — Supporting 2048 pixels in horizontal direction [R-Car V3M/R-Car V3H/R-Car
> D3/R-Car E3]
> 
> Do we have this information encoded anywhere? or are they just talking about
> maximum performance capability there?

No, we don't. It's a limit that we should have. I think we should fix that in 
a separate patch, as the 4096 pixels limit isn't implemented either.

> Also some features that are implied as supported aren't mentioned - but
> that's not a blocker to adding in the initial devices at all.
> 
> Therefore:
> 
> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_drv.c  | 24 
> >  drivers/media/platform/vsp1/vsp1_regs.h | 15 +--
> >  2 files changed, 37 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> > b/drivers/media/platform/vsp1/vsp1_drv.c index 6a9aeb71aedf..c4f2ac61f7d2
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drv.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> > @@ -710,6 +710,14 @@ static const struct vsp1_device_info
> > vsp1_device_infos[] = {> 
> > .num_bru_inputs = 5,
> > .uapi = true,
> > }, {
> > +   .version = VI6_IP_VERSION_MODEL_VSPBS_GEN3,
> > +   .model = "VSP2-BS",
> > +   .gen = 3,
> > +   .features = VSP1_HAS_BRS,
> 
> 32.1.1.5 implies:
> | VSP1_HAS_WPF_VFLIP
> 
> But Figure 32.5 implies that it doesn't ...

The figures only tell whether the full combination of rotation and H/V flip is 
available. I think you're right, I'll add VSP1_HAS_WPF_VFLIP.

> Figure 32.5 also implies that | VSP1_HAS_CLU is there too on both RPF0, and
> RPF1

Note that CLUT != CLU. I know it's confusing :-)

> > +   .rpf_count = 2,
> > +   .wpf_count = 1,
> > +   .uapi = true,
> > +   }, {
> > .version = VI6_IP_VERSION_MODEL_VSPD_GEN3,
> > .model = "VSP2-D",
> > .gen = 3,
> > @@ -717,6 +725,22 @@ static const struct vsp1_device_info
> > vsp1_device_infos[] = {> 
> > .rpf_count = 5,
> > .wpf_count = 2,
> > .num_bru_inputs = 5,
> > +   }, {
> > +   .version = VI6_IP_VERSION_MODEL_VSPD_V3,
> > +   .model = "VSP2-D",
> > +   .gen = 3,
> > +   .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,
> > +   .rpf_count = 5,
> > +   .wpf_count = 1,
> > +   .num_bru_inputs = 5,
> > +   }, {
> > +   .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
> > +   .model = "VSP2-DL",
> > +   .gen = 3,
> > +   .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,
> 
> Hrm. 32.1.1.7 says:
> — Vertical flipping in case of output to memory.
> So thats some sort of a conditional : | VSP1_HAS_WPF_VFLIP
> 
> So looking at this and the settings of the existing models, I guess it looks
> like we don't support flip if we have an LIF output (as that would then be
> unsupported)

On Gen3 vertical flipping seems to always be supported, unlike on Gen2 where 
VSPD is specifically documented as not supporting vertical flipping. We could 
add the WFLIP on all VSP2-D* instances. This would create a corresponding 
control, which wouldn't do much harm as the VSPD instances on Gen3 are not 
exposed to userspace, but that would waste a bit of memory for no good purpose 
(beside correctness I suppose). I wonder if that's worth it, what do you think 
? If so, VSP2-D should be fixed too, so I'd prefer doing that in a separate 
patch.

> > +   .rpf_count = 5,
> > +   .wpf_count = 2,
> > +   .num_bru_inputs = 5,
> > },
> >  };
> > 

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: Fix comments to comply with the kernel coding style

[Laurent Pinchart]

Hi Daniel,

On Thursday 13 Jul 2017 14:38:15 Daniel Vetter wrote:
> On Wed, Jul 12, 2017 at 08:00:42PM +0300, Laurent Pinchart wrote:
> > To avoid mixing comment styles when new comments complying with the
> > kernel coding style are introduced, fix all multiline comments in one
> > go.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Afaik the entirety of davem's -net is commented like this, this is
> checkpatch being a bit over the top. And drm is often commented like this
> too.

I have a slight preference for the "-net" style, but given that both 
checkpatch and Linus complain about it, and that I receive patches with the 
"correct" coding style, I'd rather not mix the two in the driver.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 06/14] v4l: vsp1: Add pipe index argument to the VSP-DU API

[Laurent Pinchart]

Hi Kieran,

On Thursday 13 Jul 2017 14:16:03 Kieran Bingham wrote:
> On 26/06/17 19:12, Laurent Pinchart wrote:
> > In the H3 ES2.0 SoC the VSP2-DL instance has two connections to DU
> > channels that need to be configured independently. Extend the VSP-DU API
> > with a pipeline index to identify which pipeline the caller wants to
> > operate on.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> A bit of comment merge between this and the next patch but it's minor and
> not worth the effort to change that ... so I'll happily ignore it if you do
> :)
> 
> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 12 ++--
> >  drivers/media/platform/vsp1/vsp1_drm.c | 32 +++
> >  include/media/vsp1.h   | 10 ++
> >  3 files changed, 34 insertions(+), 20 deletions(-)

[snip]

> > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > b/drivers/media/platform/vsp1/vsp1_drm.c index c72d021ff820..daaafe7885fa
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > @@ -58,21 +58,26 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
> >  /**
> >   * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
> >   * @dev: the VSP device
> > + * @pipe_index: the DRM pipeline index
> >   * @cfg: the LIF configuration
> >   *
> >   * Configure the output part of VSP DRM pipeline for the given frame
> >   @cfg.width
> > - * and @cfg.height. This sets up formats on the BRU source pad, the WPF0
> > sink
> > - * and source pads, and the LIF sink pad.
> > + * and @cfg.height. This sets up formats on the blend unit (BRU or BRS)
> > source
> > + * pad, the WPF sink and source pads, and the LIF sink pad.
> >   *
> > - * As the media bus code on the BRU source pad is conditioned by the
> > - * configuration of the BRU sink 0 pad, we also set up the formats on all
> > BRU
> > + * The @pipe_index argument selects which DRM pipeline to setup. The
> > number of
> > + * available pipelines depend on the VSP instance.
> > + *
> > + * As the media bus code on the blend unit source pad is conditioned by
> > the
> > + * configuration of its sink 0 pad, we also set up the formats on all
> > blend unit
> >   * sinks, even if the configuration will be overwritten later by
> > - * vsp1_du_setup_rpf(). This ensures that the BRU configuration is set to
> > a well
> > - * defined state.
> > + * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is
> > set to
> > + * a well defined state.
> 
> I presume those comment updates for the BRU/ blend-unit configuration are
> actually for the next patch - but I don't think it matters here - and isn't
> worth the effort to move the hunks.

Too late, I've fixed it already :-) Thanks for pointing it out.

> It all reads OK.
> 
> >   *
> >   * Return 0 on success or a negative error code on failure.
> >   */
> > -int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config
> > *cfg)
> > +int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> > + const struct vsp1_du_lif_config *cfg)
> >  {
> > struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > struct vsp1_pipeline *pipe = >drm->pipe;

[snip]

-- 
Regards,

Laurent Pinchart

[PATCH] clk: renesas: rcar-gen3: Fix error return code in cpg_sd_clock_recalc_rate

[Wolfram Sang]

From: Takeshi Kihara 

In .recalc_rate of struct clk_ops, it is desirable to return 0 if an
error occurs, but -EINVAL is returned. This patch fixes it.

Fixes: 5b1defde7054 ("clk: renesas: cpg-mssr: Extract common R-Car Gen3 support 
code")
Signed-off-by: Takeshi Kihara 
Signed-off-by: Wolfram Sang 
---

Totally valid patch forwarded from BSP.

 drivers/clk/renesas/rcar-gen3-cpg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index 3dee900522b703..e7371a8c4eb701 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -147,7 +147,7 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw 
*hw,
break;
 
if (i >= clock->div_num)
-   return -EINVAL;
+   return 0;
 
return DIV_ROUND_CLOSEST(rate, clock->div_table[i].div);
 }
-- 
2.11.0

Re: [PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Daniel Vetter]

On Thu, Jul 13, 2017 at 04:41:13PM +0200, Maxime Ripard wrote:
> The current drm_atomic_helper_commit_tail helper works only if the CRTC is
> accessible, and documents an alternative implementation that is supposed to
> be used if that happens.
> 
> That implementation is then duplicated by some drivers. Instead of
> documenting it, let's implement an helper that all the relevant users can
> use directly.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c| 47 +++
>  drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +--
>  include/drm/drm_atomic_helper.h|  1 +-
>  5 files changed, 36 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 86d3093c6c9b..a288805078f9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1245,23 +1245,11 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>   * @old_state: atomic state object with old state structures
>   *
>   * This is the default implementation for the
> - * _mode_config_helper_funcs.atomic_commit_tail hook.
> + * _mode_config_helper_funcs.atomic_commit_tail hook, for drivers
> + * that do not support runtime_pm.
>   *
>   * Note that the default ordering of how the various stages are called is to
> - * match the legacy modeset helper library closest. One peculiarity of that 
> is
> - * that it doesn't mesh well with runtime PM at all.
> - *
> - * For drivers supporting runtime PM the recommended sequence is instead ::
> - *
> - * drm_atomic_helper_commit_modeset_disables(dev, old_state);
> - *
> - * drm_atomic_helper_commit_modeset_enables(dev, old_state);
> - *
> - * drm_atomic_helper_commit_planes(dev, old_state,
> - * DRM_PLANE_COMMIT_ACTIVE_ONLY);
> - *
> - * for committing the atomic update to hardware.  See the kerneldoc entries 
> for
> - * these three functions for more details.
> + * match the legacy modeset helper library closest.

Please sprinkle links into both functions (and everywhere the old one was
referenced) to make this more discoverable, and explain when to use the
other variant.

>   */
>  void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
>  {
> @@ -1281,6 +1269,35 @@ void drm_atomic_helper_commit_tail(struct 
> drm_atomic_state *old_state)
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit_tail);
>  
> +/**
> + * drm_atomic_helper_commit_tail_runtime_pm - commit atomic update to 
> hardware
> + * @old_state: new modeset state to be committed
> + *
> + * This is a variant of drm_atomic_helper_commit_tail suited for
> + * drivers that implement runtime_pm.
> + *
> + * Note that the default ordering of how the various stages are called is to
> + * match the legacy modeset helper library closest.
> + */
> +void drm_atomic_helper_commit_tail_runtime_pm(struct drm_atomic_state 
> *old_state)

Bikeshed: I'd go with _rpm since this thing is already super long. But fee
free to ignore that.

With the docs polished I think this looks good.
-Daniel

> +{
> + struct drm_device *dev = old_state->dev;
> +
> + drm_atomic_helper_commit_modeset_disables(dev, old_state);
> +
> + drm_atomic_helper_commit_modeset_enables(dev, old_state);
> +
> + drm_atomic_helper_commit_planes(dev, old_state,
> + DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +
> + drm_atomic_helper_commit_hw_done(old_state);
> +
> + drm_atomic_helper_wait_for_vblanks(dev, old_state);
> +
> + drm_atomic_helper_cleanup_planes(dev, old_state);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_commit_tail_runtime_pm);
> +
>  static void commit_tail(struct drm_atomic_state *old_state)
>  {
>   struct drm_device *dev = old_state->dev;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c 
> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index d48fd7c918f8..71f6873255f1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -187,33 +187,8 @@ dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer 
> *fb, int index)
>   return exynos_fb->dma_addr[index];
>  }
>  
> -static void exynos_drm_atomic_commit_tail(struct drm_atomic_state *state)
> -{
> - struct drm_device *dev = state->dev;
> -
> - drm_atomic_helper_commit_modeset_disables(dev, state);
> -
> - drm_atomic_helper_commit_modeset_enables(dev, state);
> -
> - /*
> -  * Exynos can't update planes with CRTCs and encoders disabled,
> -  * its updates routines, specially for FIMD, requires the clocks
> -  * to be enabled. So it is necessary to handle the modeset operations
> -  * *before* the commit_planes() step, this way it will always
> -  * have the relevant clocks enabled to perform the 

Re: [PATCH v2 09/14] v4l: vsp1: Add support for multiple LIF instances

[Kieran Bingham]

Hi Laurent,

On 26/06/17 19:12, Laurent Pinchart wrote:
> The VSP2-DL instance (present in the H3 ES2.0 and M3-N SoCs) has two LIF
> instances. Adapt the driver infrastructure to support multiple LIFs.
> Support for multiple display pipelines will be added separately.
> 
> The change to the entity routing table removes the ability to connect
> the LIF output to the HGO or HGT histogram generators. This feature is
> only available on Gen2 hardware, isn't supported by the rest of the
> driver, and has no known use case, so this isn't an issue.
> 
> Signed-off-by: Laurent Pinchart 
This looks good.

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1.h|  5 ++--
>  drivers/media/platform/vsp1/vsp1_drm.c|  8 ++---
>  drivers/media/platform/vsp1/vsp1_drv.c| 49 
> +++
>  drivers/media/platform/vsp1/vsp1_entity.c |  3 +-
>  drivers/media/platform/vsp1/vsp1_lif.c|  5 ++--
>  drivers/media/platform/vsp1/vsp1_lif.h|  2 +-
>  drivers/media/platform/vsp1/vsp1_regs.h   |  4 ++-
>  7 files changed, 46 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1.h 
> b/drivers/media/platform/vsp1/vsp1.h
> index 73858a0ed35c..78ef838416b3 100644
> --- a/drivers/media/platform/vsp1/vsp1.h
> +++ b/drivers/media/platform/vsp1/vsp1.h
> @@ -41,11 +41,11 @@ struct vsp1_rwpf;
>  struct vsp1_sru;
>  struct vsp1_uds;
>  
> +#define VSP1_MAX_LIF 2
>  #define VSP1_MAX_RPF 5
>  #define VSP1_MAX_UDS 3
>  #define VSP1_MAX_WPF 4
>  
> -#define VSP1_HAS_LIF (1 << 0)

I guess I can re-use this 'bit' for the Writeback prototype which used to be
BIT(9) (which is now the BRS)


>  #define VSP1_HAS_LUT (1 << 1)
>  #define VSP1_HAS_SRU (1 << 2)
>  #define VSP1_HAS_BRU (1 << 3)
> @@ -61,6 +61,7 @@ struct vsp1_device_info {
>   const char *model;
>   unsigned int gen;
>   unsigned int features;
> + unsigned int lif_count;
>   unsigned int rpf_count;
>   unsigned int uds_count;
>   unsigned int wpf_count;
> @@ -84,7 +85,7 @@ struct vsp1_device {
>   struct vsp1_hgt *hgt;
>   struct vsp1_hsit *hsi;
>   struct vsp1_hsit *hst;
> - struct vsp1_lif *lif;
> + struct vsp1_lif *lif[VSP1_MAX_LIF];
>   struct vsp1_lut *lut;
>   struct vsp1_rwpf *rpf[VSP1_MAX_RPF];
>   struct vsp1_sru *sru;
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index daaafe7885fa..4e1b893e8f51 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -181,7 +181,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
> pipe_index,
>   format.format.code);
>  
>   format.pad = LIF_PAD_SINK;
> - ret = v4l2_subdev_call(>lif->entity.subdev, pad, set_fmt, NULL,
> + ret = v4l2_subdev_call(>lif[0]->entity.subdev, pad, set_fmt, NULL,
>  );
>   if (ret < 0)
>   return ret;
> @@ -599,15 +599,15 @@ int vsp1_drm_init(struct vsp1_device *vsp1)
>  
>   vsp1->bru->entity.sink = >wpf[0]->entity;
>   vsp1->bru->entity.sink_pad = 0;
> - vsp1->wpf[0]->entity.sink = >lif->entity;
> + vsp1->wpf[0]->entity.sink = >lif[0]->entity;
>   vsp1->wpf[0]->entity.sink_pad = 0;
>  
>   list_add_tail(>bru->entity.list_pipe, >entities);
>   list_add_tail(>wpf[0]->entity.list_pipe, >entities);
> - list_add_tail(>lif->entity.list_pipe, >entities);
> + list_add_tail(>lif[0]->entity.list_pipe, >entities);
>  
>   pipe->bru = >bru->entity;
> - pipe->lif = >lif->entity;
> + pipe->lif = >lif[0]->entity;
>   pipe->output = vsp1->wpf[0];
>   pipe->output->pipe = pipe;
>   pipe->frame_end = vsp1_du_pipeline_frame_end;
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
> b/drivers/media/platform/vsp1/vsp1_drv.c
> index c4f2ac61f7d2..e875982f02ae 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -168,10 +168,13 @@ static int vsp1_uapi_create_links(struct vsp1_device 
> *vsp1)
>   return ret;
>   }
>  
> - if (vsp1->lif) {
> - ret = media_create_pad_link(>wpf[0]->entity.subdev.entity,
> + for (i = 0; i < vsp1->info->lif_count; ++i) {
> + if (!vsp1->lif[i])
> + continue;
> +
> + ret = media_create_pad_link(>wpf[i]->entity.subdev.entity,
>   RWPF_PAD_SOURCE,
> - >lif->entity.subdev.entity,
> + >lif[i]->entity.subdev.entity,
>   LIF_PAD_SINK, 0);
>   if (ret < 0)
>   return ret;
> @@ -334,18 +337,23 @@ static int vsp1_create_entities(struct vsp1_device 
> *vsp1)
>   }

Re: [PATCH v2 08/14] v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances

[Kieran Bingham]

Hi Laurent,

On 26/06/17 19:12, Laurent Pinchart wrote:
> New Gen3 SoCs come with two new VSP2 variants names VSP2-BS and VSP2-DL,
> as well as a new VSP2-D variant on V3M and V3H SoCs. Add new entries for
> them in the VSP device info table.
> 
> Signed-off-by: Laurent Pinchart 

Code in the patch looks OK - but I can't see where the difference between the
horizontal widths are supported between VSPD H3/VC

I see this in the datasheet: (32.1.1.6 in this particular part)

Direct connection to display module
— Supporting 4096 pixels in horizontal direction [R-Car H3/R-Car M3-W/ R-Car 
M3-N]
— Supporting 2048 pixels in horizontal direction [R-Car V3M/R-Car V3H/R-Car
D3/R-Car E3]

Do we have this information encoded anywhere? or are they just talking about
maximum performance capability there?

Also some features that are implied as supported aren't mentioned - but that's
not a blocker to adding in the initial devices at all.

Therefore:

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_drv.c  | 24 
>  drivers/media/platform/vsp1/vsp1_regs.h | 15 +--
>  2 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
> b/drivers/media/platform/vsp1/vsp1_drv.c
> index 6a9aeb71aedf..c4f2ac61f7d2 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -710,6 +710,14 @@ static const struct vsp1_device_info vsp1_device_infos[] 
> = {
>   .num_bru_inputs = 5,
>   .uapi = true,
>   }, {
> + .version = VI6_IP_VERSION_MODEL_VSPBS_GEN3,
> + .model = "VSP2-BS",
> + .gen = 3,
> + .features = VSP1_HAS_BRS,

32.1.1.5 implies:

| VSP1_HAS_WPF_VFLIP

But Figure 32.5 implies that it doesn't ...

Figure 32.5 also implies that | VSP1_HAS_CLU is there too on both RPF0, and RPF1

> + .rpf_count = 2,
> + .wpf_count = 1,
> + .uapi = true,
> + }, {
>   .version = VI6_IP_VERSION_MODEL_VSPD_GEN3,
>   .model = "VSP2-D",
>   .gen = 3,
> @@ -717,6 +725,22 @@ static const struct vsp1_device_info vsp1_device_infos[] 
> = {
>   .rpf_count = 5,
>   .wpf_count = 2,
>   .num_bru_inputs = 5,
> + }, {
> + .version = VI6_IP_VERSION_MODEL_VSPD_V3,
> + .model = "VSP2-D",
> + .gen = 3,
> + .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,
> + .rpf_count = 5,
> + .wpf_count = 1,
> + .num_bru_inputs = 5,
> + }, {
> + .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
> + .model = "VSP2-DL",
> + .gen = 3,
> + .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,

Hrm. 32.1.1.7 says:
— Vertical flipping in case of output to memory.
So thats some sort of a conditional : | VSP1_HAS_WPF_VFLIP

So looking at this and the settings of the existing models, I guess it looks
like we don't support flip if we have an LIF output (as that would then be
unsupported)

> + .rpf_count = 5,
> + .wpf_count = 2,
> + .num_bru_inputs = 5,
>   },
>  };
>  
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h 
> b/drivers/media/platform/vsp1/vsp1_regs.h
> index 744217e020b9..ab439a60a100 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -699,9 +699,20 @@
>  #define VI6_IP_VERSION_MODEL_VSPBD_GEN3  (0x15 << 8)
>  #define VI6_IP_VERSION_MODEL_VSPBC_GEN3  (0x16 << 8)
>  #define VI6_IP_VERSION_MODEL_VSPD_GEN3   (0x17 << 8)
> +#define VI6_IP_VERSION_MODEL_VSPD_V3 (0x18 << 8)
> +#define VI6_IP_VERSION_MODEL_VSPDL_GEN3  (0x19 << 8)
> +#define VI6_IP_VERSION_MODEL_VSPBS_GEN3  (0x1a << 8)
>  #define VI6_IP_VERSION_SOC_MASK  (0xff << 0)
> -#define VI6_IP_VERSION_SOC_H (0x01 << 0)
> -#define VI6_IP_VERSION_SOC_M (0x02 << 0)
> +#define VI6_IP_VERSION_SOC_H2(0x01 << 0)
> +#define VI6_IP_VERSION_SOC_V2H   (0x01 << 0)
> +#define VI6_IP_VERSION_SOC_V3M   (0x01 << 0)
> +#define VI6_IP_VERSION_SOC_M2(0x02 << 0)
> +#define VI6_IP_VERSION_SOC_M3W   (0x02 << 0)
> +#define VI6_IP_VERSION_SOC_V3H   (0x02 << 0)
> +#define VI6_IP_VERSION_SOC_H3(0x03 << 0)
> +#define VI6_IP_VERSION_SOC_D3(0x04 << 0)
> +#define VI6_IP_VERSION_SOC_M3N   (0x04 << 0)
> +#define VI6_IP_VERSION_SOC_E3(0x04 << 0)
>  
>  /* 
> -
>   * RPF CLUT Registers
> 

Re: [PATCH v2 02/14] v4l: vsp1: Don't recycle active list at display start

[Kieran Bingham]

Hi Laurent,

On 26/06/17 19:12, Laurent Pinchart wrote:
> When the display start interrupt occurs, we know that the hardware has
> finished loading the active display list. The driver then proceeds to
> recycle the list, assuming it won't be needed anymore.
> 
> This assumption holds true for headerless display lists, as the VSP
> doesn't reload the list for the next frame if it hasn't changed.
> However, this isn't true anymore for header display lists, as they are
> loaded at every frame start regardless of whether they have been
> updated.
> 
> To prepare for header display lists usage in display pipelines, we need
> to postpone recycling the list until it gets replaced by a new one
> through a page flip. The driver already does so in the frame end
> interrupt handler, so all we need is to skip list recycling in the
> display start interrupt handler.
> 
> While the active list can be recycled at display start for headerless
> display lists, there's no real harm in postponing that to the frame end
> interrupt handler in all cases. This simplifies interrupt handling as we
> don't need to process the display start interrupt anymore.
> 
> Signed-off-by: Laurent Pinchart 

Ok, I had skipped this one as I was concerned about its effects in relation to
11/14 but I see how that's working now.

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_dl.c  | 16 
>  drivers/media/platform/vsp1/vsp1_dl.h  |  1 -
>  drivers/media/platform/vsp1/vsp1_drm.c | 12 
>  drivers/media/platform/vsp1/vsp1_drm.h |  2 --
>  drivers/media/platform/vsp1/vsp1_drv.c |  8 
>  5 files changed, 4 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
> b/drivers/media/platform/vsp1/vsp1_dl.c
> index dc47e236c780..bb92be4fe0f0 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -547,22 +547,6 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>   * Display List Manager
>   */
>  
> -/* Interrupt Handling */
> -void vsp1_dlm_irq_display_start(struct vsp1_dl_manager *dlm)
> -{
> - spin_lock(>lock);
> -
> - /*
> -  * The display start interrupt signals the end of the display list
> -  * processing by the device. The active display list, if any, won't be
> -  * accessed anymore and can be reused.
> -  */
> - __vsp1_dl_list_put(dlm->active);
> - dlm->active = NULL;
> -
> - spin_unlock(>lock);
> -}
> -
>  /**
>   * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
>   * @dlm: the display list manager
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h 
> b/drivers/media/platform/vsp1/vsp1_dl.h
> index 6ec1380a10af..ee3508172f0a 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.h
> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
> @@ -27,7 +27,6 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device 
> *vsp1,
>   unsigned int prealloc);
>  void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm);
>  void vsp1_dlm_reset(struct vsp1_dl_manager *dlm);
> -void vsp1_dlm_irq_display_start(struct vsp1_dl_manager *dlm);
>  bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
>  
>  struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index 9377aafa8996..bc3fd9bc7126 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -32,11 +32,6 @@
>   * Interrupt Handling
>   */
>  
> -void vsp1_drm_display_start(struct vsp1_device *vsp1)
> -{
> - vsp1_dlm_irq_display_start(vsp1->drm->pipe.output->dlm);
> -}
> -
>  static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe)
>  {
>   struct vsp1_drm *drm = to_vsp1_drm(pipe);
> @@ -224,6 +219,10 @@ int vsp1_du_setup_lif(struct device *dev, const struct 
> vsp1_du_lif_config *cfg)
>   return ret;
>   }
>  
> + /* Disable the display interrupts. */
> + vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0);
> + vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
> +
>   dev_dbg(vsp1->dev, "%s: pipeline enabled\n", __func__);
>  
>   return 0;
> @@ -529,13 +528,10 @@ void vsp1_du_atomic_flush(struct device *dev)
>  
>   /* Start or stop the pipeline if needed. */
>   if (!vsp1->drm->num_inputs && pipe->num_inputs) {
> - vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0);
> - vsp1_write(vsp1, VI6_DISP_IRQ_ENB, VI6_DISP_IRQ_ENB_DSTE);
>   spin_lock_irqsave(>irqlock, flags);
>   vsp1_pipeline_run(pipe);
>   spin_unlock_irqrestore(>irqlock, flags);
>   } else if (vsp1->drm->num_inputs && !pipe->num_inputs) {
> - vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
>   vsp1_pipeline_stop(pipe);
>   }
>  }
> diff --git 

Re: [PATCH v2] ARM: dts: iwg20m: Add MMCIF0 support

[Geert Uytterhoeven]

On Thu, Jul 13, 2017 at 5:39 PM, Chris Paterson
 wrote:
> Define the iwg20m board dependent part of the MMCIF0 device node.
>
> Signed-off-by: Chris Paterson 

Reviewed-by: Geert Uytterhoeven 

> v1->v2:
>  - Corrected data pin group to mmc_data8_b - Thanks Geert.

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] drm: rcar-du: Setup planes before enabling CRTC to avoid flicker

[Kieran Bingham]

On 13/07/17 16:51, Kieran Bingham wrote:
> Hi Laurent,
> 
> I've just seen Maxime's latest series "[PATCH 0/4] drm/sun4i: Fix a register
> access bug" and it relates directly to a comment I had in this patch:
> 
> On 12/07/17 17:35, Kieran Bingham wrote:
>> Hi Laurent,
>>
>> On 28/06/17 19:50, Laurent Pinchart wrote:
>>> Commit 52055bafa1ff ("drm: rcar-du: Move plane commit code from CRTC
>>> start to CRTC resume") changed the order of the plane commit and CRTC
>>> enable operations to accommodate the runtime PM requirements. However,
>>> this introduced corruption in the first displayed frame, as the CRTC is
>>> now enabled without any plane configured. On Gen2 hardware the first
>>> frame will be black and likely unnoticed, but on Gen3 hardware we end up
>>> starting the display before the VSP compositor, which is more
>>> noticeable.
>>>
>>> To fix this, revert the order of the commit operations back, and handle
>>> runtime PM requirements in the CRTC .atomic_begin() and .atomic_enable()
>>> helper operation handlers.
>>>
>>> Signed-off-by: Laurent Pinchart 
>>
>> I only have code reduction or comment suggestions below - so either with or
>> without those changes, feel free to add my:
>>
>> Reviewed-by: Kieran Bingham 
>>
>>> ---
>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 66 
>>> --
>>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  4 +--
>>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  |  2 +-
>>>  3 files changed, 43 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
>>> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>>> index 6b5219ef0ad2..76cdb88b2b8e 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>>> @@ -448,14 +448,8 @@ static void rcar_du_crtc_wait_page_flip(struct 
>>> rcar_du_crtc *rcrtc)
>>>   * Start/Stop and Suspend/Resume
>>>   */
>>>  
>>> -static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
>>> +static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc)
>>>  {
>>> -   struct drm_crtc *crtc = >crtc;
>>> -   bool interlaced;
>>> -
>>> -   if (rcrtc->started)
>>> -   return;
>>> -
>>> /* Set display off and background to black */
>>> rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
>>> rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
>>> @@ -467,6 +461,18 @@ static void rcar_du_crtc_start(struct rcar_du_crtc 
>>> *rcrtc)
>>> /* Start with all planes disabled. */
>>> rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>>>  
>>> +   /* Enable the VSP compositor. */
>>> +   if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>>> +   rcar_du_vsp_enable(rcrtc);
>>> +
>>> +   /* Turn vertical blanking interrupt reporting on. */
>>> +   drm_crtc_vblank_on(>crtc);
>>> +}
>>> +
>>> +static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
>>> +{
>>> +   bool interlaced;
>>> +
>>> /* Select master sync mode. This enables display operation in master
>>
>> Are we close enough here to fix this multiline comment style ?
>> (Not worth doing unless the patch is respun for other reasons ...)
>>
>> Actually - there are a lot in this file, so it would be better to do them 
>> all in
>> one hit/patch at a point of least conflicts ...
>>
>>
>>>  * sync mode (with the HSYNC and VSYNC signals configured as outputs and
>>>  * actively driven).
>>> @@ -477,24 +483,12 @@ static void rcar_du_crtc_start(struct rcar_du_crtc 
>>> *rcrtc)
>>>  DSYSR_TVM_MASTER);
>>>  
>>> rcar_du_group_start_stop(rcrtc->group, true);
>>> -
>>> -   /* Enable the VSP compositor. */
>>> -   if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>>> -   rcar_du_vsp_enable(rcrtc);
>>> -
>>> -   /* Turn vertical blanking interrupt reporting back on. */
>>> -   drm_crtc_vblank_on(crtc);
>>> -
>>> -   rcrtc->started = true;
>>>  }
>>>  
>>>  static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>>>  {
>>> struct drm_crtc *crtc = >crtc;
>>>  
>>> -   if (!rcrtc->started)
>>> -   return;
>>> -
>>> /* Disable all planes and wait for the change to take effect. This is
>>>  * required as the DSnPR registers are updated on vblank, and no vblank
>>>  * will occur once the CRTC is stopped. Disabling planes when starting
>>> @@ -525,8 +519,6 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc 
>>> *rcrtc)
>>> rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH);
>>>  
>>> rcar_du_group_start_stop(rcrtc->group, false);
>>> -
>>> -   rcrtc->started = false;
>>>  }
>>>  
>>>  void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc)
>>> @@ -546,12 +538,10 @@ void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc)
>>> return;
>>>  
>>> rcar_du_crtc_get(rcrtc);
>>> -   rcar_du_crtc_start(rcrtc);
>>> +   rcar_du_crtc_setup(rcrtc);
>>
>> Every call to _setup is 

RE: [PATCH] pinctrl: sh-pfc: r8a7791: Add missing mmc_data8_b pin group

[Chris Paterson]

Hello Geert,

> From: linux-gpio-ow...@vger.kernel.org [mailto:linux-gpio-
> ow...@vger.kernel.org] On Behalf Of Geert Uytterhoeven
> Sent: 12 July 2017 12:59
> 
> Pins D6 and D7 of the MMC interface can be muxed to two different sets of
> pins, but currently only one set is supported.
> Add a pin group for the alternative set to fix this.
> 
> Signed-off-by: Geert Uytterhoeven 
Reviewed-by: Chris Paterson 

I've also tested this on the iwg20m board which uses these pins. All looks okay.
Tested-by: Chris Paterson 

Kind regards, Chris

[PATCH v2] ARM: dts: iwg20m: Add MMCIF0 support

[Chris Paterson]

Define the iwg20m board dependent part of the MMCIF0 device node.

Signed-off-by: Chris Paterson <chris.paters...@renesas.com>
---
This patch is based on:
   renesas-devel-20170713-v4.12
 + pinctrl: sh-pfc: r8a7791: Add R8A7743 support
 + pinctrl: sh-pfc: r8a7791: Add missing mmc_data8_b pin group

v1->v2:
 - Corrected data pin group to mmc_data8_b - Thanks Geert.


 arch/arm/boot/dts/r8a7743-iwg20m.dtsi | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi 
b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
index 001ca91..f78dbc5 100644
--- a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
+++ b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
@@ -22,8 +22,34 @@
device_type = "memory";
reg = <2 0x 0 0x2000>;
};
+
+   reg_3p3v: 3p3v {
+   compatible = "regulator-fixed";
+   regulator-name = "3P3V";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   regulator-always-on;
+   regulator-boot-on;
+   };
 };
 
 _clk {
clock-frequency = <2000>;
 };
+
+ {
+   mmcif0_pins: mmc {
+   groups = "mmc_data8_b", "mmc_ctrl";
+   function = "mmc";
+   };
+};
+
+ {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
+   vmmc-supply = <_3p3v>;
+   bus-width = <8>;
+   non-removable;
+   status = "okay";
+};
-- 
1.9.1

[PATCH 3/4] drm/sun4i: engine: Add commit_poll function

[Maxime Ripard]

On the earlier Allwinner chips, with the first iteration of the display
engine, the backend commit bit needs to be polled before making any
register access to the backend.

Add an operation for that, that will be called in atomic_begin in order to
be sure to have that bit cleared before we do any modifications.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun4i_crtc.c   |  2 ++
 drivers/gpu/drm/sun4i/sunxi_engine.h | 14 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c 
b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index f8c70439d1e2..2eba1d8639d8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -45,6 +45,8 @@ static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
spin_unlock_irqrestore(>event_lock, flags);
crtc->state->event = NULL;
 }
+
+   WARN_ON(sunxi_engine_commit_poll(engine));
 }
 
 static void sun4i_crtc_atomic_flush(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h 
b/drivers/gpu/drm/sun4i/sunxi_engine.h
index 4cb70ae65c79..6618e182613c 100644
--- a/drivers/gpu/drm/sun4i/sunxi_engine.h
+++ b/drivers/gpu/drm/sun4i/sunxi_engine.h
@@ -17,6 +17,7 @@ struct sunxi_engine;
 
 struct sunxi_engine_ops {
void (*commit)(struct sunxi_engine *engine);
+   int (*commit_poll)(struct sunxi_engine *engine);
struct drm_plane **(*layers_init)(struct drm_device *drm,
  struct sunxi_engine *engine);
 
@@ -55,6 +56,19 @@ sunxi_engine_commit(struct sunxi_engine *engine)
 }
 
 /**
+ * sunxi_engine_commit_poll() - wait for all changes to be committed
+ * @engine:pointer to the engine
+ */
+static inline int
+sunxi_engine_commit_poll(struct sunxi_engine *engine)
+{
+   if (engine->ops && engine->ops->commit_poll)
+   return engine->ops->commit_poll(engine);
+
+   return 0;
+}
+
+/**
  * sunxi_engine_layers_init() - Create planes (layers) for the engine
  * @drm:   pointer to the drm_device for which planes will be created
  * @engine:pointer to the engine
-- 
git-series 0.9.1

[PATCH 2/4] drm/sun4i: Use the runtime_pm commit_tail variant

[Maxime Ripard]

The backend (planes) commit can only happen when the TCON (CRTC) is
enabled, which is not guaranteed with the default commit_tail helper.

Let's use the runtime_pm version that is designed specifically to deal with
that case.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c 
b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
index 9872e0fc03b0..189048d33a1d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
@@ -10,6 +10,7 @@
  * the License, or (at your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -31,6 +32,10 @@ static const struct drm_mode_config_funcs 
sun4i_de_mode_config_funcs = {
.fb_create  = drm_fb_cma_create,
 };
 
+static struct drm_mode_config_helper_funcs sun4i_de_mode_config_helpers = {
+   .atomic_commit_tail = drm_atomic_helper_commit_tail_runtime_pm,
+};
+
 struct drm_fbdev_cma *sun4i_framebuffer_init(struct drm_device *drm)
 {
drm_mode_config_reset(drm);
@@ -39,6 +44,7 @@ struct drm_fbdev_cma *sun4i_framebuffer_init(struct 
drm_device *drm)
drm->mode_config.max_height = 8192;
 
drm->mode_config.funcs = _de_mode_config_funcs;
+   drm->mode_config.helper_private = _de_mode_config_helpers;
 
return drm_fbdev_cma_init(drm, 32, drm->mode_config.num_connector);
 }
-- 
git-series 0.9.1

[PATCH 4/4] drm/sun4i: make sure we don't have a commit pending

[Maxime Ripard]

In the earlier display engine designs, any register access while a commit
is pending is forbidden.

One of the symptoms is that reading a register will return another, random,
register value which can lead to register corruptions if we ever do a
read/modify/write cycle.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 14 ++
 drivers/gpu/drm/sun4i/sun4i_crtc.c|  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index cf480218daa5..ce1f40f7511e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -67,6 +67,19 @@ static void sun4i_backend_commit(struct sunxi_engine *engine)
 SUN4I_BACKEND_REGBUFFCTL_LOADCTL);
 }
 
+static int sun4i_backend_commit_poll(struct sunxi_engine *engine)
+{
+   u32 val;
+
+   DRM_DEBUG_DRIVER("Polling for the commit to end\n");
+
+   return regmap_read_poll_timeout(engine->regs,
+   SUN4I_BACKEND_REGBUFFCTL_REG,
+   val,
+   !(val & 
SUN4I_BACKEND_REGBUFFCTL_LOADCTL),
+   100, 5);
+}
+
 void sun4i_backend_layer_enable(struct sun4i_backend *backend,
int layer, bool enable)
 {
@@ -330,6 +343,7 @@ static int sun4i_backend_of_get_id(struct device_node *node)
 
 static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
.commit = sun4i_backend_commit,
+   .commit_poll= sun4i_backend_commit_poll,
.layers_init= sun4i_layers_init,
.apply_color_correction = sun4i_backend_apply_color_correction,
.disable_color_correction   = 
sun4i_backend_disable_color_correction,
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c 
b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index 2eba1d8639d8..31550493fa1d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -34,6 +34,7 @@ static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
 {
struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
+   struct sunxi_engine *engine = scrtc->engine;
struct drm_device *dev = crtc->dev;
unsigned long flags;
 
-- 
git-series 0.9.1

[PATCH 1/4] drm/atomic: implement drm_atomic_helper_commit_tail for runtime_pm users

[Maxime Ripard]

The current drm_atomic_helper_commit_tail helper works only if the CRTC is
accessible, and documents an alternative implementation that is supposed to
be used if that happens.

That implementation is then duplicated by some drivers. Instead of
documenting it, let's implement an helper that all the relevant users can
use directly.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_atomic_helper.c| 47 +++
 drivers/gpu/drm/exynos/exynos_drm_fb.c | 27 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 18 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 21 +--
 include/drm/drm_atomic_helper.h|  1 +-
 5 files changed, 36 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 86d3093c6c9b..a288805078f9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1245,23 +1245,11 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
  * @old_state: atomic state object with old state structures
  *
  * This is the default implementation for the
- * _mode_config_helper_funcs.atomic_commit_tail hook.
+ * _mode_config_helper_funcs.atomic_commit_tail hook, for drivers
+ * that do not support runtime_pm.
  *
  * Note that the default ordering of how the various stages are called is to
- * match the legacy modeset helper library closest. One peculiarity of that is
- * that it doesn't mesh well with runtime PM at all.
- *
- * For drivers supporting runtime PM the recommended sequence is instead ::
- *
- * drm_atomic_helper_commit_modeset_disables(dev, old_state);
- *
- * drm_atomic_helper_commit_modeset_enables(dev, old_state);
- *
- * drm_atomic_helper_commit_planes(dev, old_state,
- * DRM_PLANE_COMMIT_ACTIVE_ONLY);
- *
- * for committing the atomic update to hardware.  See the kerneldoc entries for
- * these three functions for more details.
+ * match the legacy modeset helper library closest.
  */
 void drm_atomic_helper_commit_tail(struct drm_atomic_state *old_state)
 {
@@ -1281,6 +1269,35 @@ void drm_atomic_helper_commit_tail(struct 
drm_atomic_state *old_state)
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_tail);
 
+/**
+ * drm_atomic_helper_commit_tail_runtime_pm - commit atomic update to hardware
+ * @old_state: new modeset state to be committed
+ *
+ * This is a variant of drm_atomic_helper_commit_tail suited for
+ * drivers that implement runtime_pm.
+ *
+ * Note that the default ordering of how the various stages are called is to
+ * match the legacy modeset helper library closest.
+ */
+void drm_atomic_helper_commit_tail_runtime_pm(struct drm_atomic_state 
*old_state)
+{
+   struct drm_device *dev = old_state->dev;
+
+   drm_atomic_helper_commit_modeset_disables(dev, old_state);
+
+   drm_atomic_helper_commit_modeset_enables(dev, old_state);
+
+   drm_atomic_helper_commit_planes(dev, old_state,
+   DRM_PLANE_COMMIT_ACTIVE_ONLY);
+
+   drm_atomic_helper_commit_hw_done(old_state);
+
+   drm_atomic_helper_wait_for_vblanks(dev, old_state);
+
+   drm_atomic_helper_cleanup_planes(dev, old_state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_tail_runtime_pm);
+
 static void commit_tail(struct drm_atomic_state *old_state)
 {
struct drm_device *dev = old_state->dev;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c 
b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index d48fd7c918f8..71f6873255f1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -187,33 +187,8 @@ dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer 
*fb, int index)
return exynos_fb->dma_addr[index];
 }
 
-static void exynos_drm_atomic_commit_tail(struct drm_atomic_state *state)
-{
-   struct drm_device *dev = state->dev;
-
-   drm_atomic_helper_commit_modeset_disables(dev, state);
-
-   drm_atomic_helper_commit_modeset_enables(dev, state);
-
-   /*
-* Exynos can't update planes with CRTCs and encoders disabled,
-* its updates routines, specially for FIMD, requires the clocks
-* to be enabled. So it is necessary to handle the modeset operations
-* *before* the commit_planes() step, this way it will always
-* have the relevant clocks enabled to perform the update.
-*/
-   drm_atomic_helper_commit_planes(dev, state,
-   DRM_PLANE_COMMIT_ACTIVE_ONLY);
-
-   drm_atomic_helper_commit_hw_done(state);
-
-   drm_atomic_helper_wait_for_vblanks(dev, state);
-
-   drm_atomic_helper_cleanup_planes(dev, state);
-}
-
 static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
-   .atomic_commit_tail = exynos_drm_atomic_commit_tail,
+   .atomic_commit_tail = drm_atomic_helper_commit_tail_runtime_pm,
 };
 
 static const struct drm_mode_config_funcs 

Re: [PATCH v2 07/14] v4l: vsp1: Add support for the BRS entity

[Kieran Bingham]

On 26/06/17 19:12, Laurent Pinchart wrote:
> The Blend/ROP Sub Unit (BRS) is a stripped-down version of the BRU found
> in several VSP2 instances. Compared to a regular BRU, it supports two
> inputs only, and thus has no ROP unit.
> 
> Add support for the BRS by modeling it as a new entity type, but reuse

s/modeling/modelling/


> the vsp1_bru object underneath. Chaining the BRU and BRS entities seems
> to be supported by the hardware but isn't implemented yet as it isn't
> the primary use case for the BRS.
> 
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1.h|  2 +
>  drivers/media/platform/vsp1/vsp1_bru.c| 45 ++
>  drivers/media/platform/vsp1/vsp1_bru.h|  4 +-
>  drivers/media/platform/vsp1/vsp1_drv.c| 19 +-
>  drivers/media/platform/vsp1/vsp1_entity.c | 13 ++-
>  drivers/media/platform/vsp1/vsp1_entity.h |  1 +
>  drivers/media/platform/vsp1/vsp1_pipe.c   |  7 ++--
>  drivers/media/platform/vsp1/vsp1_regs.h   | 26 +
>  drivers/media/platform/vsp1/vsp1_video.c  | 63 
> ---
>  drivers/media/platform/vsp1/vsp1_wpf.c|  4 +-
>  10 files changed, 130 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1.h 
> b/drivers/media/platform/vsp1/vsp1.h
> index 847963b6e9eb..73858a0ed35c 100644
> --- a/drivers/media/platform/vsp1/vsp1.h
> +++ b/drivers/media/platform/vsp1/vsp1.h
> @@ -54,6 +54,7 @@ struct vsp1_uds;
>  #define VSP1_HAS_WPF_HFLIP   (1 << 6)
>  #define VSP1_HAS_HGO (1 << 7)
>  #define VSP1_HAS_HGT (1 << 8)
> +#define VSP1_HAS_BRS (1 << 9)
>  
>  struct vsp1_device_info {
>   u32 version;
> @@ -76,6 +77,7 @@ struct vsp1_device {
>   struct rcar_fcp_device *fcp;
>   struct device *bus_master;
>  
> + struct vsp1_bru *brs;
>   struct vsp1_bru *bru;
>   struct vsp1_clu *clu;
>   struct vsp1_hgo *hgo;
> diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
> b/drivers/media/platform/vsp1/vsp1_bru.c
> index 85362c5ef57a..e8fd2ae3b3eb 100644
> --- a/drivers/media/platform/vsp1/vsp1_bru.c
> +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> @@ -33,7 +33,7 @@
>  static inline void vsp1_bru_write(struct vsp1_bru *bru, struct vsp1_dl_list 
> *dl,
> u32 reg, u32 data)
>  {
> - vsp1_dl_list_write(dl, reg, data);
> + vsp1_dl_list_write(dl, bru->base + reg, data);
>  }
>  
>  /* 
> -
> @@ -332,11 +332,14 @@ static void bru_configure(struct vsp1_entity *entity,
>   /*
>* Route BRU input 1 as SRC input to the ROP unit and configure the ROP
>* unit with a NOP operation to make BRU input 1 available as the
> -  * Blend/ROP unit B SRC input.
> +  * Blend/ROP unit B SRC input. Only needed for BRU, the BRS has no ROP
> +  * unit.
>*/
> - vsp1_bru_write(bru, dl, VI6_BRU_ROP, VI6_BRU_ROP_DSTSEL_BRUIN(1) |
> -VI6_BRU_ROP_CROP(VI6_ROP_NOP) |
> -VI6_BRU_ROP_AROP(VI6_ROP_NOP));
> + if (entity->type == VSP1_ENTITY_BRU)
> + vsp1_bru_write(bru, dl, VI6_BRU_ROP,
> +VI6_BRU_ROP_DSTSEL_BRUIN(1) |
> +VI6_BRU_ROP_CROP(VI6_ROP_NOP) |
> +VI6_BRU_ROP_AROP(VI6_ROP_NOP));
>  
>   for (i = 0; i < bru->entity.source_pad; ++i) {
>   bool premultiplied = false;
> @@ -366,12 +369,13 @@ static void bru_configure(struct vsp1_entity *entity,
>   ctrl |= VI6_BRU_CTRL_DSTSEL_VRPF;
>  
>   /*
> -  * Route BRU inputs 0 to 3 as SRC inputs to Blend/ROP units A to
> -  * D in that order. The Blend/ROP unit B SRC is hardwired to the
> -  * ROP unit output, the corresponding register bits must be set
> -  * to 0.
> +  * Route inputs 0 to 3 as SRC inputs to Blend/ROP units A to D
> +  * in that order. In the BRU the Blend/ROP unit B SRC is
> +  * hardwired to the ROP unit output, the corresponding register
> +  * bits must be set to 0. The BRS has no ROP unit and doesn't
> +  * need any special processing.
>*/
> - if (i != 1)
> + if (!(entity->type == VSP1_ENTITY_BRU && i == 1))

If we're using this module for both BRU and BRS, would an is_bru(entity) and
is_brs(entity) be cleaner here ?

Not required - just thinking outloud...

Actaully - it's only this line that would be affected so not really needed.  I
thought there would be more uses/differences.

>   ctrl |= VI6_BRU_CTRL_SRCSEL_BRUIN(i);
>  
>   vsp1_bru_write(bru, dl, VI6_BRU_CTRL(i), ctrl);
> @@ -407,20 +411,31 @@ static const struct vsp1_entity_operations 
> bru_entity_ops = 

Re: [PATCH v2 06/14] v4l: vsp1: Add pipe index argument to the VSP-DU API

[Kieran Bingham]

On 26/06/17 19:12, Laurent Pinchart wrote:
> In the H3 ES2.0 SoC the VSP2-DL instance has two connections to DU
> channels that need to be configured independently. Extend the VSP-DU API
> with a pipeline index to identify which pipeline the caller wants to
> operate on.
> 
> Signed-off-by: Laurent Pinchart 

A bit of comment merge between this and the next patch but it's minor and not
worth the effort to change that ... so I'll happily ignore it if you do :)

Reviewed-by: Kieran Bingham 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 12 ++--
>  drivers/media/platform/vsp1/vsp1_drm.c | 32 ++--
>  include/media/vsp1.h   | 10 ++
>  3 files changed, 34 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index f870445ebc8d..d46dce054442 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -81,22 +81,22 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
>*/
>   crtc->group->need_restart = true;
>  
> - vsp1_du_setup_lif(crtc->vsp->vsp, );
> + vsp1_du_setup_lif(crtc->vsp->vsp, 0, );
>  }
>  
>  void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
>  {
> - vsp1_du_setup_lif(crtc->vsp->vsp, NULL);
> + vsp1_du_setup_lif(crtc->vsp->vsp, 0, NULL);
>  }
>  
>  void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
>  {
> - vsp1_du_atomic_begin(crtc->vsp->vsp);
> + vsp1_du_atomic_begin(crtc->vsp->vsp, 0);
>  }
>  
>  void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
>  {
> - vsp1_du_atomic_flush(crtc->vsp->vsp);
> + vsp1_du_atomic_flush(crtc->vsp->vsp, 0);
>  }
>  
>  /* Keep the two tables in sync. */
> @@ -192,7 +192,7 @@ static void rcar_du_vsp_plane_setup(struct 
> rcar_du_vsp_plane *plane)
>   }
>   }
>  
> - vsp1_du_atomic_update(plane->vsp->vsp, plane->index, );
> + vsp1_du_atomic_update(plane->vsp->vsp, 0, plane->index, );
>  }
>  
>  static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
> @@ -292,7 +292,7 @@ static void rcar_du_vsp_plane_atomic_update(struct 
> drm_plane *plane,
>   if (plane->state->crtc)
>   rcar_du_vsp_plane_setup(rplane);
>   else
> - vsp1_du_atomic_update(rplane->vsp->vsp, rplane->index, NULL);
> + vsp1_du_atomic_update(rplane->vsp->vsp, 0, rplane->index, NULL);
>  }
>  
>  static const struct drm_plane_helper_funcs rcar_du_vsp_plane_helper_funcs = {
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index c72d021ff820..daaafe7885fa 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -58,21 +58,26 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
>  /**
>   * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
>   * @dev: the VSP device
> + * @pipe_index: the DRM pipeline index
>   * @cfg: the LIF configuration
>   *
>   * Configure the output part of VSP DRM pipeline for the given frame 
> @cfg.width
> - * and @cfg.height. This sets up formats on the BRU source pad, the WPF0 sink
> - * and source pads, and the LIF sink pad.
> + * and @cfg.height. This sets up formats on the blend unit (BRU or BRS) 
> source
> + * pad, the WPF sink and source pads, and the LIF sink pad.
>   *
> - * As the media bus code on the BRU source pad is conditioned by the
> - * configuration of the BRU sink 0 pad, we also set up the formats on all BRU
> + * The @pipe_index argument selects which DRM pipeline to setup. The number 
> of
> + * available pipelines depend on the VSP instance.
> + *
> + * As the media bus code on the blend unit source pad is conditioned by the
> + * configuration of its sink 0 pad, we also set up the formats on all blend 
> unit
>   * sinks, even if the configuration will be overwritten later by
> - * vsp1_du_setup_rpf(). This ensures that the BRU configuration is set to a 
> well
> - * defined state.
> + * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is 
> set to
> + * a well defined state.

I presume those comment updates for the BRU/ blend-unit configuration are
actually for the next patch - but I don't think it matters here - and isn't
worth the effort to move the hunks.

It all reads OK.


>   *
>   * Return 0 on success or a negative error code on failure.
>   */
> -int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config 
> *cfg)
> +int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> +   const struct vsp1_du_lif_config *cfg)
>  {
>   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
>   struct vsp1_pipeline *pipe = >drm->pipe;
> @@ -81,6 +86,9 @@ int vsp1_du_setup_lif(struct device *dev, const struct 
> vsp1_du_lif_config *cfg)
>   unsigned int i;
>   int ret;
>  
> + if (pipe_index > 

Re: [PATCH v2 03/14] v4l: vsp1: Don't set WPF sink pointer

[Kieran Bingham]

On 26/06/17 19:12, Laurent Pinchart wrote:
> The sink pointer is used to configure routing inside the VSP, and as
> such must point to the next VSP entity in the pipeline. The WPF being a
> pipeline terminal sink, its output route can't be configured. The
> routing configuration code already handles this correctly without
> referring to the sink pointer, which thus doesn't need to be set.
> 
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_drv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
> b/drivers/media/platform/vsp1/vsp1_drv.c
> index 6b35e043b554..35087d5573ce 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -412,7 +412,6 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
>   }
>  
>   list_add_tail(>list, >videos);
> - wpf->entity.sink = >video.entity;
>   }
>   }
>  
> 

Re: [PATCH v2 01/14] v4l: vsp1: Fill display list headers without holding dlm spinlock

[Kieran Bingham]

Hi Laurent,

Starts easy ... (I haven't gone through these in numerical order of course :D)

On 26/06/17 19:12, Laurent Pinchart wrote:
> The display list headers are filled using information from the display
> list only. Lower the display list manager spinlock contention by filling
> the headers without holding the lock.
> 
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/media/platform/vsp1/vsp1_dl.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
> b/drivers/media/platform/vsp1/vsp1_dl.c
> index aaf17b13fd78..dc47e236c780 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -483,8 +483,6 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>   unsigned long flags;
>   bool update;
>  
> - spin_lock_irqsave(>lock, flags);
> -
>   if (dl->dlm->mode == VSP1_DL_MODE_HEADER) {
>   struct vsp1_dl_list *dl_child;
>  
> @@ -501,7 +499,11 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>  
>   vsp1_dl_list_fill_header(dl_child, last);
>   }
> + }
>  
> + spin_lock_irqsave(>lock, flags);
> +
> + if (dl->dlm->mode == VSP1_DL_MODE_HEADER) {
>   /*
>* Commit the head display list to hardware. Chained headers
>* will auto-start.
> 

Re: [PATCH] drm: rcar-du: Fix comments to comply with the kernel coding style

[Daniel Vetter]

On Wed, Jul 12, 2017 at 08:00:42PM +0300, Laurent Pinchart wrote:
> To avoid mixing comment styles when new comments complying with the
> kernel coding style are introduced, fix all multiline comments in one
> go.
> 
> Signed-off-by: Laurent Pinchart 

Afaik the entirety of davem's -net is commented like this, this is
checkpatch being a bit over the top. And drm is often commented like this
too.

Just fyi.
-Daniel
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 24 --
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_group.c   | 18 -
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 18 -
>  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 12 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   | 42 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |  3 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c |  6 +++--
>  8 files changed, 96 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index f131fc68cc46..a04802f7b2f1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -168,7 +168,8 @@ static void rcar_du_crtc_set_display_timing(struct 
> rcar_du_crtc *rcrtc)
>   u32 escr;
>   u32 div;
>  
> - /* Compute the clock divisor and select the internal or external dot
> + /*
> +  * Compute the clock divisor and select the internal or external dot
>* clock based on the requested frequency.
>*/
>   clk = clk_get_rate(rcrtc->clock);
> @@ -261,12 +262,14 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc,
>   struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
>   struct rcar_du_device *rcdu = rcrtc->group->dev;
>  
> - /* Store the route from the CRTC output to the DU output. The DU will be
> + /*
> +  * Store the route from the CRTC output to the DU output. The DU will be
>* configured when starting the CRTC.
>*/
>   rcrtc->outputs |= BIT(output);
>  
> - /* Store RGB routing to DPAD0, the hardware will be configured when
> + /*
> +  * Store RGB routing to DPAD0, the hardware will be configured when
>* starting the CRTC.
>*/
>   if (output == RCAR_DU_OUTPUT_DPAD0)
> @@ -342,7 +345,8 @@ static void rcar_du_crtc_update_planes(struct 
> rcar_du_crtc *rcrtc)
>   }
>   }
>  
> - /* Update the planes to display timing and dot clock generator
> + /*
> +  * Update the planes to display timing and dot clock generator
>* associations.
>*
>* Updating the DPTSR register requires restarting the CRTC group,
> @@ -450,7 +454,8 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
>   /* Start with all planes disabled. */
>   rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>  
> - /* Select master sync mode. This enables display operation in master
> + /*
> +  * Select master sync mode. This enables display operation in master
>* sync mode (with the HSYNC and VSYNC signals configured as outputs and
>* actively driven).
>*/
> @@ -478,7 +483,8 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   if (!rcrtc->started)
>   return;
>  
> - /* Disable all planes and wait for the change to take effect. This is
> + /*
> +  * Disable all planes and wait for the change to take effect. This is
>* required as the DSnPR registers are updated on vblank, and no vblank
>* will occur once the CRTC is stopped. Disabling planes when starting
>* the CRTC thus wouldn't be enough as it would start scanning out
> @@ -491,7 +497,8 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>   drm_crtc_wait_one_vblank(crtc);
>  
> - /* Disable vertical blanking interrupt reporting. We first need to wait
> + /*
> +  * Disable vertical blanking interrupt reporting. We first need to wait
>* for page flip completion before stopping the CRTC as userspace
>* expects page flips to eventually complete.
>*/
> @@ -502,7 +509,8 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>   rcar_du_vsp_disable(rcrtc);
>  
> - /* Select switch sync mode. This stops display operation and configures
> + /*
> +  * Select switch sync mode. This stops display operation and configures
>* the HSYNC and VSYNC signals as inputs.
>*/
>   rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index d6a0255181cc..b95437bccb3d 100644
> --- 

Re: [PATCH v2 00/14] Renesas R-Car VSP: Add H3 ES2.0 support

[Kieran Bingham]

Hi Laurent,

Thankyou for these patches, bringing life to the outputs of my ES2.0 target 
board.

I have tested them on my board, and including the VSP unit test suite, and
kmscube utilities.

Feel free to add a Tested-by: Kieran Bingham
 to all of the patches if you desire,
and I'm working through the individual reviews.

Oh - except now I've just said that - I did some extra testing with both HDMI
and VGA output connected and ran
   kmstest --flip --sync

This was soon followed by a kernel error trace [0] shown below.

However replicating this is possibly more complicated than just running kmstest
--flip --sync ... I've had to do various iterations of running with/without
{flip, sync} but I have reproduced 'issues' about 3 times now.

I think the key thing is in the VGA connection or regarding trying to output to
both.

Interestingly, I haven't been able to make this happen on the ES1.0 platform as
yet... though --flip --sync is quicker to fail with 'Flip Commit failed: -16"
there...

For reference this was tested on your pinchartl-media/drm/next/h3-es2/merged,
branch which I don't believe has the recent work on not needing to wait for a
final vblank on shutdown. So it is quite possible that the issue I am seeing is
simply a symptom of that separately repaired issue.

On that basis I've left my comment regarding my Tested-by: tag above as I
suspect that this issue I've hit could likely be separate and already resolved.

I'll try to add those patches to this tree to see if the issue resolves 
itself...

Regardless of that, this series conflicts with my current developments,
therefore I will likely rebase my work on top of this series. I don't need an
immutable branch, but please do let me know if this series changes :-)

--
Regards

Kieran.






[0] : Kernel log snippet posted below:

[  597.471369] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  607.711346] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  607.711354] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  607.749585] vsp1 fea2.vsp: failed to reset wpf.1
[  617.951311] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  618.055358] vsp1 fea2.vsp: failed to reset wpf.1
[  618.060498] vsp1 fea2.vsp: DRM pipeline stop timeout
[  628.831762] vsp1 fea2.vsp: failed to reset wpf.1
[  638.943315] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  649.183313] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  677.753465] vsp1 fea2.vsp: failed to reset wpf.1
[  687.839322] [drm:drm_atomic_helper_commit_cleanup_done] *ERROR*
[CRTC:57:crtc-3] flip_done timed out
[  687.935288] vsp1 fea2.vsp: failed to reset wpf.1
[  687.940360] vsp1 fea2.vsp: DRM pipeline stop timeout
[  687.945939] Unable to handle kernel NULL pointer dereference at virtual
address 
[  687.954206] pgd = ff8009a0a000
[  687.957680] [] *pgd=00073fffe003, *pud=00073fffe003,
*pmd=
[  687.966068] Internal error: Oops: 9606 [#1] PREEMPT SMP
[  687.971690] Modules linked in:
[  687.974777] CPU: 0 PID: 10426 Comm: kmstest Not tainted
4.12.0-rc5-01343-g1bc824acf27c #5
[  687.983019] Hardware name: Renesas Salvator-X 2nd version board based on
r8a7795 ES2.0+ (DT)
[  687.991525] task: ffc6f97b4080 task.stack: ffc6f51dc000
[  687.997502] PC is at __media_pipeline_stop+0x24/0xd0
[  688.002507] LR is at media_pipeline_stop+0x34/0x48
[  688.007336] pc : [] lr : [] pstate: 
6145
[  688.014790] sp : ffc6f51df780
[  688.018129] x29: ffc6f51df780 x28: 
[  688.023490] x27: 0038 x26: ff8008960088
[  688.028850] x25: ffc6f98bcb10 x24: ffc6f98b8818
[  688.034210] x23: 0001 x22: ffc6f9ff1998
[  688.039570] x21: ffc6f98bc080 x20: 
[  688.044929] x19: 0008 x18: 0010
[  688.050288] x17: 007f9bc9e1c8 x16: ff8008165bb8
[  688.055648] x15: ff80899bb63f x14: 0006
[  688.061007] x13: ffc6faac6750 x12: ff80087bd078
[  688.066366] x11:  x10: ff80097bb000
[  688.071725] x9 : ff8008afd000 x8 : 
[  688.077085] x7 : ff8008583c1c x6 : ff8008da75c8
[  688.082443] x5 : ff8009585d00 x4 : 2d28ca88
[  688.087803] x3 : 89277f76 x2 : 
[  688.093162] x1 : ffc6f97b4080 x0 : ff8008583c24
[  688.098523] Process kmstest (pid: 10426, stack limit = 0xffc6f51dc000)
[  688.105453] Stack: (0xffc6f51df780 to 0xffc6f51e)
[  688.111245] f780: ffc6f51df7b0 ff8008583c24 ffc6f98b8ae8
ffc6f98bc080
[  688.119138] f7a0: ffc6f98bc818 ff8009d18000 ffc6f51df7e0
ff80085aa688
[  688.127031] f7c0: 

[PATCH] arm64: dts: renesas: salvator-common: Remove extra LVDS port label

[Laurent Pinchart]

The DU LVDS output is on port 3 on R8A7795 but on port 2 on R8A7796. The
lvds_connector label thus can't be defined in salvator-common.dtsi,
common to the two SoCs.

The lvds_connector label is meant for convience to be referenced from
panel device tree files, such as r8a77xx-aa104xd12-panel.dtsi or
r8a77xx-aa121td01-panel.dtsi.  As those files are not included in any
device tree source, and the label never used elsewhere, we can simply
remove it. Out-of-tree patches that include panel device tree files can
then add a

#define lvds_connector du_out_lvds0

before including the panel device tree file.

Signed-off-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi 
b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index aef35e0b685a..7d46a6c0f74b 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -268,10 +268,6 @@
remote-endpoint = <_in>;
};
};
-   port@3 {
-   lvds_connector: endpoint {
-   };
-   };
};
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH v2] arm64: dts: renesas: r8a7796: Add FDP1 instance

[Laurent Pinchart]

The r8a7796 has a single FDP1 instance.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Kieran Bingham 
Reviewed-by: Geert Uytterhoeven 
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 10 ++
 1 file changed, 10 insertions(+)

Changes since v1:

- Added resets property

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index ef1120f4e561..898a8dbb31de 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -1562,6 +1562,16 @@
/* placeholder */
};
 
+   fdp1@fe94 {
+   compatible = "renesas,fdp1";
+   reg = <0 0xfe94 0 0x2400>;
+   interrupts = ;
+   clocks = < CPG_MOD 119>;
+   power-domains = < R8A7796_PD_A3VC>;
+   resets = < 119>;
+   renesas,fcp = <>;
+   };
+
fcpf0: fcp@fe95 {
compatible = "renesas,fcpf";
reg = <0 0xfe95 0 0x200>;
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] arm64: dts: r8a7795: salvator-xs: Connect DU dot clocks 0 and 3

[Geert Uytterhoeven]

On Thu, Jul 13, 2017 at 1:09 PM, Laurent Pinchart
 wrote:
> The DU dot clocks 0 and 3 are provided by the programmable VC6 clock
> generator. Connect them to the clock source node.
>
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Geert Uytterhoeven 

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] drm: rcar-du: Fix comments to comply with the kernel coding style

[Kieran Bingham]

Hi Laurent,

Thanks for this - I think that's a better way forward in this instance now that
a few of the locations have been touched, rather than leaving the driver in a
mixed state.

On 12/07/17 18:00, Laurent Pinchart wrote:
> To avoid mixing comment styles when new comments complying with the
> kernel coding style are introduced, fix all multiline comments in one
> go.
> 
> Signed-off-by: Laurent Pinchart 
Acked-by: Kieran Bingham 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 24 --
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_group.c   | 18 -
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 18 -
>  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 12 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   | 42 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.h   |  3 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c |  6 +++--
>  8 files changed, 96 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index f131fc68cc46..a04802f7b2f1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -168,7 +168,8 @@ static void rcar_du_crtc_set_display_timing(struct 
> rcar_du_crtc *rcrtc)
>   u32 escr;
>   u32 div;
>  
> - /* Compute the clock divisor and select the internal or external dot
> + /*
> +  * Compute the clock divisor and select the internal or external dot
>* clock based on the requested frequency.
>*/
>   clk = clk_get_rate(rcrtc->clock);
> @@ -261,12 +262,14 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc,
>   struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
>   struct rcar_du_device *rcdu = rcrtc->group->dev;
>  
> - /* Store the route from the CRTC output to the DU output. The DU will be
> + /*
> +  * Store the route from the CRTC output to the DU output. The DU will be
>* configured when starting the CRTC.
>*/
>   rcrtc->outputs |= BIT(output);
>  
> - /* Store RGB routing to DPAD0, the hardware will be configured when
> + /*
> +  * Store RGB routing to DPAD0, the hardware will be configured when
>* starting the CRTC.
>*/
>   if (output == RCAR_DU_OUTPUT_DPAD0)
> @@ -342,7 +345,8 @@ static void rcar_du_crtc_update_planes(struct 
> rcar_du_crtc *rcrtc)
>   }
>   }
>  
> - /* Update the planes to display timing and dot clock generator
> + /*
> +  * Update the planes to display timing and dot clock generator
>* associations.
>*
>* Updating the DPTSR register requires restarting the CRTC group,
> @@ -450,7 +454,8 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
>   /* Start with all planes disabled. */
>   rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>  
> - /* Select master sync mode. This enables display operation in master
> + /*
> +  * Select master sync mode. This enables display operation in master
>* sync mode (with the HSYNC and VSYNC signals configured as outputs and
>* actively driven).
>*/
> @@ -478,7 +483,8 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   if (!rcrtc->started)
>   return;
>  
> - /* Disable all planes and wait for the change to take effect. This is
> + /*
> +  * Disable all planes and wait for the change to take effect. This is
>* required as the DSnPR registers are updated on vblank, and no vblank
>* will occur once the CRTC is stopped. Disabling planes when starting
>* the CRTC thus wouldn't be enough as it would start scanning out
> @@ -491,7 +497,8 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
>   drm_crtc_wait_one_vblank(crtc);
>  
> - /* Disable vertical blanking interrupt reporting. We first need to wait
> + /*
> +  * Disable vertical blanking interrupt reporting. We first need to wait
>* for page flip completion before stopping the CRTC as userspace
>* expects page flips to eventually complete.
>*/
> @@ -502,7 +509,8 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   if (rcar_du_has(rcrtc->group->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>   rcar_du_vsp_disable(rcrtc);
>  
> - /* Select switch sync mode. This stops display operation and configures
> + /*
> +  * Select switch sync mode. This stops display operation and configures
>* the HSYNC and VSYNC signals as inputs.
>*/
>   rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
> 

Re: [PATCH] arm64: dts: renesas: salvator-xs: Add VC6 clock generator

[Geert Uytterhoeven]

On Thu, Jul 13, 2017 at 1:06 PM, Laurent Pinchart
 wrote:
> The VC6 is an I2C-controlled programmable clock generator, used on the
> board to provide a display dot clock. Add it to DT.
>
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Geert Uytterhoeven 

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 1/2] arm64: dts: renesas: r8a7796: Add resets property to FCP devices

[Geert Uytterhoeven]

On Thu, Jul 13, 2017 at 12:58 PM, Laurent Pinchart
 wrote:
> The FCP nodes are missing the resets property. Add it.
>
> Fixes: 7153c69fb8e1 ("arm64: dts: renesas: r8a7796: Add FCPF and FCPV 
> instances")
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Geert Uytterhoeven 

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] arm64: dts: renesas: r8a7796: Add resets property to VSP devices

[Geert Uytterhoeven]

On Thu, Jul 13, 2017 at 12:58 PM, Laurent Pinchart
 wrote:
> The VSP nodes are missing the resets property. Add it.
>
> Fixes: 5a89c826745f ("arm64: dts: renesas: r8a7796: Add VSP instances")
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Geert Uytterhoeven 

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 2/2] arm64: dts: r8a7795: salvator-xs: Connect DU dot clocks 0 and 3

[Laurent Pinchart]

The DU dot clocks 0 and 3 are provided by the programmable VC6 clock
generator. Connect them to the clock source node.

Signed-off-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts 
b/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts
index 6a7d1b22d0fe..7675de5d4f2c 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts
@@ -44,10 +44,12 @@
 < CPG_MOD 722>,
 < CPG_MOD 721>,
 < CPG_MOD 727>,
+< 1>,
 <_clk>,
-<_clk>;
+<_clk>,
+< 2>;
clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0",
- "dclkin.1", "dclkin.2";
+ "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
 };
 
  {
-- 
Regards,

Laurent Pinchart

[PATCH] arm64: dts: renesas: salvator-xs: Add VC6 clock generator

[Laurent Pinchart]

The VC6 is an I2C-controlled programmable clock generator, used on the
board to provide a display dot clock. Add it to DT.

Signed-off-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/salvator-xs.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-xs.dtsi 
b/arch/arm64/boot/dts/renesas/salvator-xs.dtsi
index 81227e3c2c6f..bf4d200fb546 100644
--- a/arch/arm64/boot/dts/renesas/salvator-xs.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-xs.dtsi
@@ -18,3 +18,13 @@
 _clk {
clock-frequency = <1664>;
 };
+
+ {
+   versaclock6: clock-generator@6a {
+   compatible = "idt,5p49v6901";
+   reg = <0x6a>;
+   #clock-cells = <1>;
+   clocks = <_clk>;
+   clock-names = "xin";
+   };
+};
-- 
Regards,

Laurent Pinchart

[PATCH 1/2] arm64: dts: renesas: r8a7796: Add resets property to FCP devices

[Laurent Pinchart]

The FCP nodes are missing the resets property. Add it.

Fixes: 7153c69fb8e1 ("arm64: dts: renesas: r8a7796: Add FCPF and FCPV 
instances")
Signed-off-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 6d0048d7394a..1eef59f6fb63 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -1567,6 +1567,7 @@
reg = <0 0xfe95 0 0x200>;
clocks = < CPG_MOD 615>;
power-domains = < R8A7796_PD_A3VC>;
+   resets = < 615>;
};
 
vspb: vsp@fe96 {
@@ -1584,6 +1585,7 @@
reg = <0 0xfe96f000 0 0x200>;
clocks = < CPG_MOD 607>;
power-domains = < R8A7796_PD_A3VC>;
+   resets = < 607>;
};
 
vspi0: vsp@fe9a {
@@ -1601,6 +1603,7 @@
reg = <0 0xfe9af000 0 0x200>;
clocks = < CPG_MOD 611>;
power-domains = < R8A7796_PD_A3VC>;
+   resets = < 611>;
};
 
vspd0: vsp@fea2 {
@@ -1618,6 +1621,7 @@
reg = <0 0xfea27000 0 0x200>;
clocks = < CPG_MOD 603>;
power-domains = < R8A7796_PD_ALWAYS_ON>;
+   resets = < 603>;
};
 
vspd1: vsp@fea28000 {
@@ -1635,6 +1639,7 @@
reg = <0 0xfea2f000 0 0x200>;
clocks = < CPG_MOD 602>;
power-domains = < R8A7796_PD_ALWAYS_ON>;
+   resets = < 602>;
};
 
vspd2: vsp@fea3 {
@@ -1652,6 +1657,7 @@
reg = <0 0xfea37000 0 0x200>;
clocks = < CPG_MOD 601>;
power-domains = < R8A7796_PD_ALWAYS_ON>;
+   resets = < 601>;
};
 
hdmi0: hdmi@fead {
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] dt-bindings: mmc: sh_mmcif: Document r8a7743 DT bindings

[Ulf Hansson]

On 12 July 2017 at 12:03, Chris Paterson  wrote:
> Signed-off-by: Chris Paterson 
>
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt 
> b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> index c32dc5a..703e18c 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -11,6 +11,7 @@ Required properties:
> - "renesas,mmcif-r7s72100" for the MMCIF found in r7s72100 SoCs
> - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs
> - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
> +   - "renesas,mmcif-r8a7743" for the MMCIF found in r8a7743 SoCs
> - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs
> - "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
> - "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
> @@ -21,7 +22,7 @@ Required properties:
>  - interrupts: Some SoCs have only 1 shared interrupt, while others have 
> either
>2 or 3 individual interrupts (error, int, card detect). Below is the number
>of interrupts for each SoC:
> -1: r8a73a4, r8a7778, r8a7790, r8a7791, r8a7793, r8a7794
> +1: r8a73a4, r8a7743, r8a7778, r8a7790, r8a7791, r8a7793, r8a7794
>  2: r8a7740, sh73a0
>  3: r7s72100
>
> --
> 1.9.1
>

Thanks, applied for next!

Kind regards
Uffe

Re: [PATCH 14/14] pinctrl: sh-pfc: r8a7796: Rename CS1# pin function definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch renames the pin function macro definitions of the GPSR1 and
> IPSR4 registers value for the CS1# pin.
>
> This is a correction because GPSR and IPSR register specification for
> R8A7796 SoC was changed in R-Car Gen3 Hardware User's Manual Rev.0.54E.
>
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Fixes: f9aece7344bd81ce ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 v3] mmc: tmio-mmc: fix bad pointer math

[Ulf Hansson]

On 12 July 2017 at 17:40, Chris Brandt  wrote:
> The existing code gives an incorrect pointer value.
> The buffer pointer 'buf' was of type unsigned short *, and 'count' was a
> number in bytes. A cast of buf should have been used.
>
> However, instead of casting, just change the code to use u32 pointers.
>
> Reported-by: Dan Carpenter 
> Fixes: 8185e51f358a: ("mmc: tmio-mmc: add support for 32bit data port")
> Signed-off-by: Chris Brandt 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for fixes and added a stable tag.

Kind regards
Uffe

> ---
> v3:
>  * Merged lines
>  * Added Reviewed-by
> v2:
>  * Use u32 pointers instead of casting
> ---
>  drivers/mmc/host/tmio_mmc_core.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 77e7b56a9099..db779732fd2e 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -415,30 +415,29 @@ static void tmio_mmc_transfer_data(struct tmio_mmc_host 
> *host,
>  * Transfer the data
>  */
> if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {
> -   u8 data[4] = { };
> +   u32 data = 0;
> +   u32 *buf32 = (u32 *)buf;
>
> if (is_read)
> -   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
> +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, buf32,
>count >> 2);
> else
> -   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32 
> *)buf,
> +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, buf32,
> count >> 2);
>
> /* if count was multiple of 4 */
> if (!(count & 0x3))
> return;
>
> -   buf8 = (u8 *)(buf + (count >> 2));
> +   buf32 += count >> 2;
> count %= 4;
>
> if (is_read) {
> -   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,
> -  (u32 *)data, 1);
> -   memcpy(buf8, data, count);
> +   sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, , 1);
> +   memcpy(buf32, , count);
> } else {
> -   memcpy(data, buf8, count);
> -   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
> -   (u32 *)data, 1);
> +   memcpy(, buf32, count);
> +   sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, , 1);
> }
>
> return;
> --
> 2.13.0
>
>

Re: [PATCH 13/14] pinctrl: sh-pfc: r8a7796: Fix IPSR and MOD_SEL register pin assignment for FSO pins group

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes IPSR{12,17,18} and MOD_SEL0 pin assignment for FSO pins
> group.
>
> This is a correction because GPSR and IPSR register specification for
> R8A7796 SoC was changed in R-Car Gen3 Hardware User's Manual Rev.0.54E.
>
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 12/14] pinctrl: sh-pfc: r8a7796: Fix to delete MOD_SEL0 bit2 register definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the macro definitions of MOD_SEL0 bit2 register deleted.
>
> This is a correction because MOD_SEL register specification for R8A7796
> SoC was changed in R-Car Gen3 Hardware User's Manual Rev.0.53E.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 11/14] pinctrl: sh-pfc: r8a7796: Fix to delete SATA_DEVSLP_B pins function definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the macro definitions of SATA_DEVSLP_B pins function
> deleted.
>
> This is a correction to the incorrect implementation of IPSR register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 10/14] pinctrl: sh-pfc: r8a7796: Fix to delete FSCLKST pin and IPSR7 bit[15:12] register definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the macro definitions of FSCLKST pins function and IPSR7
> bit[15:12] register deleted.
>
> This is a correction because IPSR register specification for R8A7796 SoC
> was changed in R-Car Gen3 Hardware User's Manual Rev.0.53E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 09/14] pinctrl: sh-pfc: r8a7796: Fix MOD_SEL register pin assignment for TCLK{1,2}_{A,B} pins group

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes to set MOD_SEL2 bit19 when using TCLK2_A pin function is
> selected for IPSR16 bit[23:20] or using TCLK2_B pin function is selected
> for IPSR17 bit[27:24].
>
> This is a correction to the incorrect implementation of MOD_SEL register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 08/14] pinctrl: sh-pfc: r8a7796: Fix MSIOF3_{SS1,SS2}_E pins function definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the implementation incorrect of IPSR register value
> definitions for MSIOF3_{SS1,SS2}_E pins function.
>
> This is a correction to the incorrect implementation of IPSR register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Identical to "[PATCH 3/5] pinctrl: sh-pfc: r8a7796: Fix MSIOF3_{SS1,SS2}_E pin
 function definitions" I sent yesterday.

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 07/14] pinctrl: sh-pfc: r8a7796: Fix NFDATA{0..13} and NF{ALE,CLE,WE_N,RE_N} pin function definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the implementation incorrect of IPSR register value
> definitions for NFDATA{0..13} and NF{ALE,CLE,WE_N,RE_N} pins function.
>
> This is a correction to the incorrect implementation of IPSR register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 06/14] pinctrl: sh-pfc: r8a7796: Fix FMCLK{_C,_D} and FMIN{_C,_D} pin function definitions

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the implementation incorrect of IPSR register value
> definitions for FMCLK{_C,_D} and FMIN{_C,_D} pins function.
>
> This is a correction to the incorrect implementation of IPSR register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 3/3] ARM: dts: iwg20m: Add MMCIF0 support

[Chris Paterson]

> From: Simon Horman [mailto:ho...@verge.net.au]
> Sent: 13 July 2017 09:12
> 
> On Thu, Jul 13, 2017 at 10:02:29AM +0200, Simon Horman wrote:
> > On Wed, Jul 12, 2017 at 01:52:49PM +0200, Geert Uytterhoeven wrote:
> > > Hi Chris,
> > >
> > > On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
> > >  wrote:
> > > > Define the iwg20m board dependent part of the MMCIF0 device node.
> > > >
> > > > Signed-off-by: Chris Paterson 
> > > >
> > > > diff --git a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > > > b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > > > index 001ca91..ffce1b6 100644
> > > > --- a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > > > +++ b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > >
> > > > + {
> > > > +   mmcif0_pins: mmc {
> > > > +   groups = "mmc_data8", "mmc_ctrl";
> > >
> > > "mmc_data8" is not correct, as D6/D7 of the eMMC are not connected
> > > to GP6_28 resp. GP6_29, but to GP6_6 resp. GP6_7.
> > > So it should be "mmc_data8_b".
> > >
> > > Unfortunately the latter pin group isn't supported by the PFC driver yet.
> > > Cooking a patch...

Thanks for your review Geert. Sorry I missed this.

> >
> > The above notwithstanding I have applied this patch for v4.14.
> 
> ...
> 
> Sorry, I think I need more coffee.

No worries!

> 
> I misread Geert's comment above. I have dropped this patch.
> Please post a corrected v2 of this patch.

Will do.

Re: [PATCH 05/14] pinctrl: sh-pfc: r8a7796: Fix SCIF_CLK_{A,B} pin's MOD_SEL assignment to MOD_SEL1 bit10

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes SCIF_CLK_{A,B} pin's MOD_SEL assignment from MOD_SEL1
> bit11 to MOD_SEL1 bit10.
>
> This is a correction to the incorrect implementation of IPSR register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 04/14] pinctrl: sh-pfc: r8a7796: Fix MOD_SEL register pin assignment for SSI pins group

[Geert Uytterhoeven]

Hi Kaneko-san, Kihara-san,

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes MOD_SEL1 bit20 and MOD_SEL2 bit20, bit21 pin assignment
> for SSI pins group.
>
> This is a correction to the incorrect implementation of MOD_SEL register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7796.c | 38 
> ++--
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
> index 4d070c2..18c9c61 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c

> @@ -1277,7 +1277,7 @@ enum {
> PINMUX_IPSR_MSEL(IP14_3_0,  RX5_A,  SEL_SCIF5_0),
> PINMUX_IPSR_MSEL(IP14_3_0,  NFWP_N_A,   SEL_NDF_0),
> PINMUX_IPSR_MSEL(IP14_3_0,  AUDIO_CLKA_C,   SEL_ADG_A_2),
> -   PINMUX_IPSR_MSEL(IP14_3_0,  SSI_SCK2_A, SEL_SSI_0),
> +   PINMUX_IPSR_MSEL(IP14_3_0,  SSI_SCK2_A, SEL_SSI2_0),

The SSI_SCK2_A part seems to have been removed in Rev.052E and later?
However, I can't find that in the errata for Rev.051E.

> PINMUX_IPSR_MSEL(IP14_3_0,  STP_IVCXO27_0_C,SEL_SSP1_0_2),
> PINMUX_IPSR_GPSR(IP14_3_0,  AUDIO_CLKOUT3_A),
> PINMUX_IPSR_MSEL(IP14_3_0,  TCLK1_B,
> SEL_TIMER_TMU_1),

The rest looks good to me.

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: [kbuild-all] [PATCH 3/3] ARM: dts: iwg20m: Add MMCIF0 support

[Ye Xiaolong]

On 07/13, Simon Horman wrote:
>On Thu, Jul 13, 2017 at 08:25:56AM +0800, kbuild test robot wrote:
>> Hi Chris,
>> 
>> [auto build test ERROR on renesas/next]
>> [also build test ERROR on next-20170712]
>> [cannot apply to v4.12]
>> [if your patch is applied to the wrong git tree, please drop us a note to 
>> help improve the system]
>> 
>> url:
>> https://github.com/0day-ci/linux/commits/Chris-Paterson/Add-MMCIF0-support-for-r8a7743-iwg20m/20170713-042814
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git 
>> next
>> config: arm-at91_dt_defconfig (attached as .config)
>> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
>> reproduce:
>> wget 
>> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
>> ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # save the attached .config to linux build tree
>> make.cross ARCH=arm 
>> 
>> All errors (new ones prefixed by >>):
>> 
>> >> Error: arch/arm/boot/dts/r8a7743-iwg20m.dtsi:40.1-5 Label or path pfc not 
>> >> found
>>FATAL ERROR: Syntax error parsing input tree
>
>This seems to be a false positive. The node in question is present in
>the devel branch which is the base of these changes.

Thanks for the feedback, we'll update 0day's configuration accordingly.

Thanks,
Xiaolong
>
>
>___
>kbuild-all mailing list
>kbuild-...@lists.01.org
>https://lists.01.org/mailman/listinfo/kbuild-all

Re: [PATCH 03/14] pinctrl: sh-pfc: r8a7796: Fix MOD_SEL2 bit26 to 0x0 when using SCK5_A

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the implementation incorrect of MOD_SEL2 bit26 value
> when SCK5_A pin function is selected for IPSR16 bit[31:28].
>
> This is a correction to the incorrect implementation of MOD_SEL register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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] arm64: dts: r8a7796: Add missing second pair of DMA names to MSIOF nodes

[Simon Horman]

On Thu, Jul 13, 2017 at 10:23:43AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Jul 13, 2017 at 10:18 AM, Simon Horman  wrote:
> > On Wed, Jul 12, 2017 at 12:35:53PM +0200, Geert Uytterhoeven wrote:
> >> MSIOF0 and MSIOF1 are tied to two DMA controllers through two pairs of
> >> DMA specifiers.  However, the second pair of corresponding DMA names was
> >> missing.
> >>
> >> Fixes: 80fab06e258da762 ("arm64: dts: r8a7796: Add all MSIOF nodes")
> >> Signed-off-by: Geert Uytterhoeven 
> >> ---
> >>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Thanks, applied for v4.14.
> >
> > Let me know if you would prefer it handled as a fix for v4.13.
> > I see the commit it fixes was part of v4.11.
> 
> Thanks!
> 
> V4.14 is fine.  It's not enabled in any board DTS, and DMA will stll work as
> long as there are free channels on DMAC1.

Thanks, for clarifying. The second point above was the main part of my
reasoning that it is v4.14 material.

Re: [PATCH 02/14] pinctrl: sh-pfc: r8a7796: Fix IPSR register setting when MSIOF3_SS1_E pin was selected

[Geert Uytterhoeven]

Hi Kaneko-san,

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes to set IPSR register when using MSIOF3_SS1_E pin function
> is selected.
>
> This is a correction to the incorrect implementation of IPSR register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Identical to "[PATCH 4/5] pinctrl: sh-pfc: r8a7796: Fix IPSR setting for
MSIOF3_SS1_E pin" I sent yesterday.

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] arm64: dts: r8a7796: Add missing second pair of DMA names to MSIOF nodes

[Geert Uytterhoeven]

Hi Simon,

On Thu, Jul 13, 2017 at 10:18 AM, Simon Horman  wrote:
> On Wed, Jul 12, 2017 at 12:35:53PM +0200, Geert Uytterhoeven wrote:
>> MSIOF0 and MSIOF1 are tied to two DMA controllers through two pairs of
>> DMA specifiers.  However, the second pair of corresponding DMA names was
>> missing.
>>
>> Fixes: 80fab06e258da762 ("arm64: dts: r8a7796: Add all MSIOF nodes")
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Thanks, applied for v4.14.
>
> Let me know if you would prefer it handled as a fix for v4.13.
> I see the commit it fixes was part of v4.11.

Thanks!

V4.14 is fine.  It's not enabled in any board DTS, and DMA will stll work as
long as there are free channels on DMAC1.

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 v2] ARM: shmobile: rcar-gen2: Add support for CPG/MSSR bindings

[Simon Horman]

On Wed, Jul 12, 2017 at 12:39:32PM +0200, Geert Uytterhoeven wrote:
> When using the new CPG/MSSR bindings, there is no longer a
> "renesas,rcar-gen2-cpg-clocks" node, and the code to obtain the external
> clock crystal frequency falls back to a default of 20 MHz.
> While this is correct for all upstream R-Car Gen2 and RZ/G1 boards, this
> is not necessarily the case for out-of-tree third party boards.
> 
> Add support for finding the external clock crystal oscillator on RZ/G1M,
> and on R-Car H2, M2-W, and M2-N using the new CPG/MSSR bindings, through
> the corresponding "renesas,r8a77xx-cpg-mssr" nodes.
> 
> Note that this is not needed on R-Car V2H and E2, and on RZ/G1E, as on
> those SoCs the arch_timer and generic counter clock is derived from the
> ZS clock instead.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
> v2:
>   - Drop RZ/G1E,
>   - Add support for R-Car H2, M2-W, and M2-N.

Thanks, applied for v4.14.

Re: [PATCH] arm64: dts: r8a7796: Add missing second pair of DMA names to MSIOF nodes

[Simon Horman]

On Wed, Jul 12, 2017 at 12:35:53PM +0200, Geert Uytterhoeven wrote:
> MSIOF0 and MSIOF1 are tied to two DMA controllers through two pairs of
> DMA specifiers.  However, the second pair of corresponding DMA names was
> missing.
> 
> Fixes: 80fab06e258da762 ("arm64: dts: r8a7796: Add all MSIOF nodes")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks, applied for v4.14.

Let me know if you would prefer it handled as a fix for v4.13.
I see the commit it fixes was part of v4.11.

Re: [PATCH v3] arm64: dts: r8a7795: Add all MSIOF nodes

[Simon Horman]

On Wed, Jul 12, 2017 at 12:34:21PM +0200, Geert Uytterhoeven wrote:
> Add the device nodes for all MSIOF SPI controllers, incl. clocks, power
> domain, dma, and reset properties.
> 
> Due to a hardware erratum on R-Car H3 ES1.x, using MSIOF for SPI is only
> supported on ES2.0 and later.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
> Tested on r8a7795/salvator-xs with MSIOF3(A) connected to a HD44780
> character LCD through a pair of 74HCT595 shift registers.

Thanks, applied for v4.14.

> 
> v3:
>   - Add resets properties,
>   - Add family-specific compatible values,
>   - Add link to second DMA controller for MSIOF[01],
> 
> v2:
>   - Rebased,
>   - Switch to final CPG/MSSR bindings,
>   - Change one-line summary prefix to match current arm-soc practices.
> ---
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 62 
> 
>  1 file changed, 62 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index 6f5130766a212d6d..507d8ad07aaf9007 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -899,6 +899,68 @@
>   status = "disabled";
>   };
>  
> + msiof0: spi@e6e9 {
> + compatible = "renesas,msiof-r8a7795",

I take it there is also a patch to add the above compat string
to the bindings documentation.

> +  "renesas,rcar-gen3-msiof";
> + reg = <0 0xe6e9 0 0x0064>;
> + interrupts = ;
> + clocks = < CPG_MOD 211>;
> + dmas = < 0x41>, < 0x40>,
> +< 0x41>, < 0x40>;
> + dma-names = "tx", "rx", "tx", "rx";
> + power-domains = < R8A7795_PD_ALWAYS_ON>;
> + resets = < 211>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };

...

Re: [PATCH 3/3] ARM: dts: iwg20m: Add MMCIF0 support

[Simon Horman]

On Thu, Jul 13, 2017 at 10:02:29AM +0200, Simon Horman wrote:
> On Wed, Jul 12, 2017 at 01:52:49PM +0200, Geert Uytterhoeven wrote:
> > Hi Chris,
> > 
> > On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
> >  wrote:
> > > Define the iwg20m board dependent part of the MMCIF0 device node.
> > >
> > > Signed-off-by: Chris Paterson 
> > >
> > > diff --git a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi 
> > > b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > > index 001ca91..ffce1b6 100644
> > > --- a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > > +++ b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > 
> > > + {
> > > +   mmcif0_pins: mmc {
> > > +   groups = "mmc_data8", "mmc_ctrl";
> > 
> > "mmc_data8" is not correct, as D6/D7 of the eMMC are not connected to GP6_28
> > resp. GP6_29, but to GP6_6 resp. GP6_7.
> > So it should be "mmc_data8_b".
> > 
> > Unfortunately the latter pin group isn't supported by the PFC driver yet.
> > Cooking a patch...
> 
> The above notwithstanding I have applied this patch for v4.14.

...

Sorry, I think I need more coffee.

I misread Geert's comment above. I have dropped this patch.
Please post a corrected v2 of this patch.

Re: [PATCH 0/3] ARM: dts: r8a779[014]: Use R-Car Gen 2 fallback binding for vin nodes

[Simon Horman]

On Thu, Jul 13, 2017 at 07:07:44AM +0200, Niklas Söderlund wrote:
> Hi Simon,
> 
> Thanks for your work.
> 
> On 2017-07-11 14:56:46 +0200, Simon Horman wrote:
> > Use R-Car Gen 2 fallback binding for vind nodes in DT for r8a779[014] SoCs.
> > 
> > This has no run-time effect for the current driver as the initialisation
> > sequence is the same for the SoC-specific binding for r8a779[014] and the
> > fallback binding for R-Car Gen 2
> > 
> > This is consistent with existing compat string usage in vin nodes for the
> > r8a779[23] SoCs.
> 
> For whole serries:
> 
> Acked-by: Niklas Söderlund 

Thanks, applied for v4.14.

Re: [PATCH 01/14] pinctrl: sh-pfc: r8a7796: Fix MOD_SEL1 bit[25:24] to 0x3 when using STP_ISEN_1_D

[Geert Uytterhoeven]

On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko  wrote:
> From: Takeshi Kihara 
>
> This patch fixes the implementation incorrect of MOD_SEL1 bit[25:24]
> value when STP_ISEN_1_D pin function is selected for IPSR17 bit[27:24].
>
> This is a correction to the incorrect implementation of MOD_SEL register
> pin assignment for R8A7796 SoC specification of R-Car Gen3 Hardware
> User's Manual Rev.0.51E or later.
>
> Fixes: f9aece7344bd ("pinctrl: sh-pfc: Initial R8A7796 PFC support")
> Signed-off-by: Takeshi Kihara 
> Signed-off-by: Yoshihiro Kaneko 

Reviewed-by: Geert Uytterhoeven 

i.e. will queue in sh-pfc-for-v4.14.

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 1/3] dt-bindings: mmc: sh_mmcif: Document r8a7743 DT bindings

[Simon Horman]

On Thu, Jul 13, 2017 at 10:00:11AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Jul 13, 2017 at 9:54 AM, Simon Horman  wrote:
> > On Wed, Jul 12, 2017 at 01:20:07PM +0200, Geert Uytterhoeven wrote:
> >> On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
> >>  wrote:
> >> > Signed-off-by: Chris Paterson 
> >>
> >> Reviewed-by: Geert Uytterhoeven 
> >
> > Thanks, applied for v4.14.
> 
> Shouldn't this be applied by Ulf to the MMC tree?

Sorry, my bad. I have dropped this from the renesas tree.

Ulf, please consider this patch for the mmc tree.

Re: [PATCH 3/3] ARM: dts: iwg20m: Add MMCIF0 support

[Simon Horman]

On Wed, Jul 12, 2017 at 01:52:49PM +0200, Geert Uytterhoeven wrote:
> Hi Chris,
> 
> On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
>  wrote:
> > Define the iwg20m board dependent part of the MMCIF0 device node.
> >
> > Signed-off-by: Chris Paterson 
> >
> > diff --git a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi 
> > b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > index 001ca91..ffce1b6 100644
> > --- a/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> > +++ b/arch/arm/boot/dts/r8a7743-iwg20m.dtsi
> 
> > + {
> > +   mmcif0_pins: mmc {
> > +   groups = "mmc_data8", "mmc_ctrl";
> 
> "mmc_data8" is not correct, as D6/D7 of the eMMC are not connected to GP6_28
> resp. GP6_29, but to GP6_6 resp. GP6_7.
> So it should be "mmc_data8_b".
> 
> Unfortunately the latter pin group isn't supported by the PFC driver yet.
> Cooking a patch...

The above notwithstanding I have applied this patch for v4.14.

This is under the assumption that:
a) the relevant (pfc) driver changes will appear in v4.14 and;
b) there is no regression introduced in having this change present
   without the driver changes

An implication of the above is that the new feature will not work
until both the driver and DTS changes are in the same tree. But that the
trees with each of those changes continue to work at least as well as they
did before.

Please test the devel branch that I push later today to make sure there
are no regressions.

Re: [PATCH 1/3] dt-bindings: mmc: sh_mmcif: Document r8a7743 DT bindings

[Geert Uytterhoeven]

Hi Simon,

On Thu, Jul 13, 2017 at 9:54 AM, Simon Horman  wrote:
> On Wed, Jul 12, 2017 at 01:20:07PM +0200, Geert Uytterhoeven wrote:
>> On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
>>  wrote:
>> > Signed-off-by: Chris Paterson 
>>
>> Reviewed-by: Geert Uytterhoeven 
>
> Thanks, applied for v4.14.

Shouldn't this be applied by Ulf to the MMC tree?

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] ARM: dts: r8a7743: Add MMCIF0 support

[Simon Horman]

On Wed, Jul 12, 2017 at 01:21:20PM +0200, Geert Uytterhoeven wrote:
> On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
>  wrote:
> > Add the MMCIF0 device to the r8a7743 device tree.
> >
> > Signed-off-by: Chris Paterson 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.14.

Re: [PATCH 1/3] dt-bindings: mmc: sh_mmcif: Document r8a7743 DT bindings

[Simon Horman]

On Wed, Jul 12, 2017 at 01:20:07PM +0200, Geert Uytterhoeven wrote:
> On Wed, Jul 12, 2017 at 12:03 PM, Chris Paterson
>  wrote:
> > Signed-off-by: Chris Paterson 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.14.

Re: [PATCH 3/3] ARM: dts: iwg20m: Add MMCIF0 support

[Simon Horman]

On Thu, Jul 13, 2017 at 08:25:56AM +0800, kbuild test robot wrote:
> Hi Chris,
> 
> [auto build test ERROR on renesas/next]
> [also build test ERROR on next-20170712]
> [cannot apply to v4.12]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Chris-Paterson/Add-MMCIF0-support-for-r8a7743-iwg20m/20170713-042814
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git next
> config: arm-at91_dt_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget 
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
> >> Error: arch/arm/boot/dts/r8a7743-iwg20m.dtsi:40.1-5 Label or path pfc not 
> >> found
>FATAL ERROR: Syntax error parsing input tree

This seems to be a false positive. The node in question is present in
the devel branch which is the base of these changes.

Re: [PATCH v2 2/7] arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances

[Simon Horman]

On Wed, Jul 12, 2017 at 11:45:41AM +0300, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Wednesday 12 Jul 2017 09:22:50 Geert Uytterhoeven wrote:
> > On Wed, Jun 21, 2017 at 11:31 AM, Laurent Pinchart wrote:
> > > The FCPs handle the interface between various IP cores and memory. Add
> > > the instances related to the FDPs and VSP2s.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > > Reviewed-by: Geert Uytterhoeven 
> > 
> > Ah, the pitfalls of reposting patches 8 months later...
> > 
> > >  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 42
> > >   1 file changed, 42 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > > b/arch/arm64/boot/dts/renesas/r8a7796.dtsi index
> > > 1f6710912045..28b0e2127021 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > > @@ -1440,6 +1440,48 @@
> > > /* placeholder */
> > > };
> > > 
> > > +   fcpf0: fcp@fe95 {
> > > +   compatible = "renesas,fcpf";
> > > +   reg = <0 0xfe95 0 0x200>;
> > > +   clocks = < CPG_MOD 615>;
> > > +   power-domains = < R8A7796_PD_A3VC>;
> > 
> > ... missing resets properties.
> 
> Oops :-/
> 
> Simon, can I send an incremental patch ?

Yes, please do.

Re: [PATCH v2 3/7] arm64: dts: renesas: r8a7796: Add VSP instances

[Simon Horman]

On Wed, Jul 12, 2017 at 09:24:22AM +0200, Geert Uytterhoeven wrote:
> Hi Laurent, Simon,
> 
> On Wed, Jun 21, 2017 at 11:31 AM, Laurent Pinchart
>  wrote:
> > The r8a7796 has 5 VSP instances.
> >
> > Signed-off-by: Laurent Pinchart 
> > Reviewed-by: Geert Uytterhoeven 
> 
> Ah, the pitfalls of reposting patches 8 months later...
> 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 50 
> > 
> >  1 file changed, 50 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
> > b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > index 28b0e2127021..ad9cd1c3199f 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> > @@ -1447,6 +1447,16 @@
> > power-domains = < R8A7796_PD_A3VC>;
> > };
> >
> > +   vspb: vsp@fe96 {
> > +   compatible = "renesas,vsp2";
> > +   reg = <0 0xfe96 0 0x8000>;
> > +   interrupts = ;
> > +   clocks = < CPG_MOD 626>;
> > +   power-domains = < R8A7796_PD_A3VC>;
> 
> ... missing resets properties.

Sorry for letting that slip through.
Laurent, please send an incremental patch.

Re: [RESEND] [PATCH v2.1 1/2] dt-bindings: display: rcar-du: Add a VSP channel index to the vsps DT property

[Simon Horman]

On Thu, Jul 13, 2017 at 09:46:19AM +0200, Simon Horman wrote:
> On Wed, Jul 12, 2017 at 11:43:36AM +0300, Laurent Pinchart wrote:
> > On some R-Car SoCs a single VSP can serve multiple DU channels through
> > multiple LIF instances in the VSP. The current DT bindings don't support
> > specifying that kind of SoC integration scheme. Extend them with a VSP
> > channel index.
> > 
> > Backward compatibility can be ensured in drivers by checking the length
> > of the vsps property and setting the channel to 0 when the property
> > doesn't contain channel indices.
> > 
> > Signed-off-by: Laurent Pinchart 
> > Reviewed-by: Geert Uytterhoeven 
> > Acked-by: Rob Herring 
> 
> Thanks, applied for v2.1.

s/v2.1/v4.14/

Re: [PATCH v2 0/2] R-Car H3 ES2.0 Salvator-X: Enable DU support in DT

[Simon Horman]

On Wed, Jul 12, 2017 at 11:44:41AM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Wednesday 12 Jul 2017 07:56:15 Simon Horman wrote:
> > On Wed, Jul 12, 2017 at 02:20:43AM +0300, Laurent Pinchart wrote:
> > > On Tuesday 11 Jul 2017 11:16:17 Simon Horman wrote:
> > >> On Mon, Jul 10, 2017 at 04:31:38PM +0300, Laurent Pinchart wrote:
> > >>> On Monday 26 Jun 2017 19:29:28 Laurent Pinchart wrote:
> >  Hello,
> >  
> >  This patch series enable DU support in DT for the R-Car H3 ES2.0
> >  Salvator-X board. Patch 1/2 extends the DT bindings as needed, and
> >  patch
> >  2/2 then enables DU in the SoC DT.
> >  
> >  The patches are based on top of Simon's arm64-dt-for-v4.14 branch.
> >  
> >  Compared to v1
> >  
> >  - the VSP DT bindings have been extended in patch 1/2
> >  - patch 2/2 has been updated accordingly
> >  - patch "arm64: dts: r8a7795: salvator-x: Unify DU node between ES1.x
> >  
> >    and ES2.0" has been dropped
> >  
> >  Laurent Pinchart (2):
> >    drm: rcar-du: Add a VSP channel index to the vsps DT property
> >    arm64: dts: r8a7795: Add support for the DU
> >   
> >   .../devicetree/bindings/display/renesas,du.txt | 51 +++--
> >   arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi   |  1 -
> >   arch/arm64/boot/dts/renesas/r8a7795.dtsi   |  2 +
> >   3 files changed, 32 insertions(+), 22 deletions(-)
> > >>> 
> > >>> Hi Simon,
> > >>> 
> > >>> Could you please pick this series for v4.14 ? I believe it has
> > >>> received the necessary acks.
> > >> 
> > >> Hi Laurent,
> > >> 
> > >> sure, both patches applied for v4.14.
> > > 
> > > It looks like you've picked v2 instead of v2.1 for patch 2/2 :-/ Is it
> > > possible to fix that ?
> > 
> > Sure, but I don't see v2.1. Could you repost it?
> 
> Strange. I've reposted the patch and CC'ed you.

Thanks, I think the change of subject confused me.
In any case I have applied the repost of v2.1 and will push it later today.

Re: [RESEND] [PATCH v2.1 1/2] dt-bindings: display: rcar-du: Add a VSP channel index to the vsps DT property

[Simon Horman]

On Wed, Jul 12, 2017 at 11:43:36AM +0300, Laurent Pinchart wrote:
> On some R-Car SoCs a single VSP can serve multiple DU channels through
> multiple LIF instances in the VSP. The current DT bindings don't support
> specifying that kind of SoC integration scheme. Extend them with a VSP
> channel index.
> 
> Backward compatibility can be ensured in drivers by checking the length
> of the vsps property and setting the channel to 0 when the property
> doesn't contain channel indices.
> 
> Signed-off-by: Laurent Pinchart 
> Reviewed-by: Geert Uytterhoeven 
> Acked-by: Rob Herring 

Thanks, applied for v2.1.