Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 15:46, Frank Rowand wrote:
> On 01/15/18 12:29, Laurent Pinchart wrote:
>> Hi Frank,
>>
>> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>>> On 01/15/18 11:22, Laurent Pinchart wrote:
 On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> On 01/15/18 09:09, Rob Herring wrote:
>> +Frank
>>
>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>>> The internal LVDS encoders now have their own DT bindings. Before
>>> switching the driver infrastructure to those new bindings, implement
>>> backward-compatibility through live DT patching.
>>
>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>>
>>> Let me first answer the question that you ask later.  You ask "Can we work
>>> on this together to find a solution that would suit us both ?"
>>>
>>> My answer to that is emphatically YES.  I will definitely work with you to
>>> try to find a good solution.
>>
>> \o/
>>
> Please no.  What we just got rid of was making it difficult for me to
> make changes to the overlay infrastructure.  There are issues with
> overlays that need to be fixed before overlays become really usable.
> I am about to propose the next change, which involves removing a
> chunk of code that I will not be able to remove if this patch is
> accepted (the proposal is awaiting me collecting some data about
> the impact of the change, which I expect to complete this week).
>>>
>>> I should have thought just a little bit more before I hit send.  The
>>> situation is even worse than I wrote.  One of the next steps (in
>>> addition to what I wrote about above) is to change the overlay apply
>>> function to accept a flattened device tree (FDT), not an expanded
>>> device tree.  In this changed model, the unflattened overlay is
>>> not available to be modified before it is applied.
>>
>> That makes sense if we consider overlays to be immutable objects that we 
>> apply 
>> without offering a way to modify them. I won't challenge that API decision, 
>> as 
>> my use of an overlay here is a bit of a hack indeed.
>>
>>> It is important for the devicetree infrastructure to have ownership
>>> of the FDT that is used to create the unflattened tree.  (Notice
>>> that in the proposed patch, rcar_du_of_get_overlay() follows the
>>> TI example of doing a kmemdup of the blob (FDT), then uses the
>>> copy for the unflatten.  The kmemdup() in this case is to create
>>> a persistent copy of the FDT.)  The driver having ownership of
>>> this copy, and having the ability to free it is one of the many
>>> problems with the current overlay implementation.
>>
>> Yes, that's something I've identified as well. Lots of work has been done to 
>> clean up the OF core and we're clearly not done yet. I'm happy to see all 
>> the 
>> improvements you're working on.
>>
> Can you please handle both the old and new bindings through driver
> code instead?

 I could, but it would be pointless. The point here is to allow cleanups in
 the driver. The LVDS encoder handling code is very intrusive in its
 current form and I need to get rid of it. There would be zero point in
 moving to the new infrastructure, as the main point is to get rid of the
 old code which prevents moving forward. As a consequence that would block
 new boards from receiving proper upstream support. An easy option is to
 break backward compatibility. I'm personally fine with that, but I assume
 other people would complain :-)

 I can, on the other hand, work with you to see how live DT patching could
 be implemented in this driver without blocking your code. When developing
 this patch series I start by patching the device tree manually without
 relying on overlays at all, but got blocked by the fact that I need to
 allocate phandles for new nodes I create. If there was an API to allocate
 an unused phandle I could avoid using the overlay infrastructure at all.
 Or there could be other
>>>
>>> It seems reasonable to provide a way to autogenerate a phandle (if
>>> requested) by the devicetree code that creates a new node.  Were you using
>>> a function from drivers/of/dynamic.c to create the node?
>>
>> Not to allocate the node, no. I allocated the device_node structure manually 
>> with kzalloc(), and inserted it in the device tree with of_attach_node(). Is 
>> that the right approach ? I haven't been able to test the code as I stopped 
>> when I realized I couldn't allocate phandles.
>>
 options I'm not thinking of as I don't know what the changes you're
 working on are. Can we work on this together to find a solution that
 would suit us both ?
>>>
>>> Again, yes, I would be glad to work with you on this.
>>
>> How would you like to proceed ? I can try the manual approach again but need 
>> information about how I could cleanly implement phandle allocation. I will 
>> likely then run into other 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 12:29, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>> On 01/15/18 11:22, Laurent Pinchart wrote:
>>> On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
 On 01/15/18 09:09, Rob Herring wrote:
> +Frank
>
> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>> The internal LVDS encoders now have their own DT bindings. Before
>> switching the driver infrastructure to those new bindings, implement
>> backward-compatibility through live DT patching.
>
> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>
>> Let me first answer the question that you ask later.  You ask "Can we work
>> on this together to find a solution that would suit us both ?"
>>
>> My answer to that is emphatically YES.  I will definitely work with you to
>> try to find a good solution.
> 
> \o/
> 
 Please no.  What we just got rid of was making it difficult for me to
 make changes to the overlay infrastructure.  There are issues with
 overlays that need to be fixed before overlays become really usable.
 I am about to propose the next change, which involves removing a
 chunk of code that I will not be able to remove if this patch is
 accepted (the proposal is awaiting me collecting some data about
 the impact of the change, which I expect to complete this week).
>>
>> I should have thought just a little bit more before I hit send.  The
>> situation is even worse than I wrote.  One of the next steps (in
>> addition to what I wrote about above) is to change the overlay apply
>> function to accept a flattened device tree (FDT), not an expanded
>> device tree.  In this changed model, the unflattened overlay is
>> not available to be modified before it is applied.
> 
> That makes sense if we consider overlays to be immutable objects that we 
> apply 
> without offering a way to modify them. I won't challenge that API decision, 
> as 
> my use of an overlay here is a bit of a hack indeed.
> 
>> It is important for the devicetree infrastructure to have ownership
>> of the FDT that is used to create the unflattened tree.  (Notice
>> that in the proposed patch, rcar_du_of_get_overlay() follows the
>> TI example of doing a kmemdup of the blob (FDT), then uses the
>> copy for the unflatten.  The kmemdup() in this case is to create
>> a persistent copy of the FDT.)  The driver having ownership of
>> this copy, and having the ability to free it is one of the many
>> problems with the current overlay implementation.
> 
> Yes, that's something I've identified as well. Lots of work has been done to 
> clean up the OF core and we're clearly not done yet. I'm happy to see all the 
> improvements you're working on.
> 
 Can you please handle both the old and new bindings through driver
 code instead?
>>>
>>> I could, but it would be pointless. The point here is to allow cleanups in
>>> the driver. The LVDS encoder handling code is very intrusive in its
>>> current form and I need to get rid of it. There would be zero point in
>>> moving to the new infrastructure, as the main point is to get rid of the
>>> old code which prevents moving forward. As a consequence that would block
>>> new boards from receiving proper upstream support. An easy option is to
>>> break backward compatibility. I'm personally fine with that, but I assume
>>> other people would complain :-)
>>>
>>> I can, on the other hand, work with you to see how live DT patching could
>>> be implemented in this driver without blocking your code. When developing
>>> this patch series I start by patching the device tree manually without
>>> relying on overlays at all, but got blocked by the fact that I need to
>>> allocate phandles for new nodes I create. If there was an API to allocate
>>> an unused phandle I could avoid using the overlay infrastructure at all.
>>> Or there could be other
>>
>> It seems reasonable to provide a way to autogenerate a phandle (if
>> requested) by the devicetree code that creates a new node.  Were you using
>> a function from drivers/of/dynamic.c to create the node?
> 
> Not to allocate the node, no. I allocated the device_node structure manually 
> with kzalloc(), and inserted it in the device tree with of_attach_node(). Is 
> that the right approach ? I haven't been able to test the code as I stopped 
> when I realized I couldn't allocate phandles.
> 
>>> options I'm not thinking of as I don't know what the changes you're
>>> working on are. Can we work on this together to find a solution that
>>> would suit us both ?
>>
>> Again, yes, I would be glad to work with you on this.
> 
> How would you like to proceed ? I can try the manual approach again but need 
> information about how I could cleanly implement phandle allocation. I will 
> likely then run into other issues for which I might need help.

Here are my first thoughts, based on 4.15-rc7:

Question to Rob and Frank: should of_attach_node() 

Re: [PATCH v2 0/6] i2c: send STOP after recovery; use it for i2c-rcar

[Wolfram Sang]

On Tue, Jan 09, 2018 at 02:58:53PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang 
> 
> When implementing bus recovery for the i2c-rcar driver, two problems were
> encountered: 1) When reading the SDA bit, not the SDA status was returned but
> the internal state of the "bus_is_busy" logic. 2) This logic needs a STOP to
> consider the bus free again. SCL/SDA high is not enough, and there is no other
> way known to reset the internal logic otherwise.
> 
> The obvious solution to just send STOP after recovery makes sense for the
> generic case, too, IMO. If we made a device release SDA again, and are about
> start a new transfer using START, then we should terminate the previous state
> properly with STOP. This may help with some devices and shouldn't create any
> drawback AFAICS.
> 
> For this, we need to introduce a 'set_sda' callback to the recovery
> infrastructure. The first five patches may be interesting for anyone, so input
> is greatly appreciated. Also, testing the new features with GPIO based 
> recovery
> would be awesome to have. Thanks to Phil for testing already!
> 
> This was tested on a Renesas Lager board (r8a7790/R-Car H2). My test procedure
> is documented here:
> 
> https://elinux.org/Tests:I2C-bus-recovery
> 
> A branch is available here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/topic/rcar-i2c-recovery
> 
> Please let me know what you think.
> 
> Changes since V1:
> 
> * add more identifiers in patch 2
> * don't use GPIOF_* flags but hardocded '0'
> * added Phil's Tested-by
> 
> 
> 
> Wolfram Sang (6):
>   i2c: make kerneldoc about bus recovery more precise
>   i2c: add identifier in declarations for i2c_bus_recovery
>   i2c: add 'set_sda' to bus_recovery_info
>   i2c: ensure SDA is released in recovery if SDA is controllable
>   i2c: send STOP after successful bus recovery
>   i2c: rcar: implement bus recovery
> 
>  drivers/i2c/busses/i2c-rcar.c | 54 
> +--
>  drivers/i2c/i2c-core-base.c   | 25 +++-
>  include/linux/i2c.h   | 26 -
>  3 files changed, 91 insertions(+), 14 deletions(-)

Applied to for-next, thanks!



signature.asc
Description: PGP signature

Re: [PATCH v2 04/12] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Sergei Shtylyov]

On 01/15/2018 11:32 PM, Laurent Pinchart wrote:


The LVDS encoders used to be described in DT as part of the DU. They now
have their own DT node, linked to the DU using the OF graph bindings.
This allows moving internal LVDS encoder support to a separate driver
modelled as a DRM bridge. Backward compatibility is retained as legacy
DT is patched live to move to the new bindings.

Signed-off-by: Laurent Pinchart



[...]


diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 6e02c762a557..06a3fbdd728a
100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c


[...]


   /* 
@@ -74,7 +75,6 @@ static const struct rcar_du_device_info
rzg1_du_r8a7745_info = {
.port = 1,

  >  },

},
-   .num_lvds = 0,
   };
   
   static const struct rcar_du_device_info rcar_du_r8a7779_info = {

@@ -95,14 +95,13 @@ static const struct rcar_du_device_info
rcar_du_r8a7779_info = {
.port = 1,
},
},
-   .num_lvds = 0,
   };
   
   static const struct rcar_du_device_info rcar_du_r8a7790_info = {

.gen = 2,
.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-   .quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
+   .quirks = RCAR_DU_QUIRK_ALIGN_128B,
.num_crtcs = 3,
.routes = {
/*
@@ -164,7 +163,6 @@ static const struct rcar_du_device_info
rcar_du_r8a7792_info = {
.port = 1,
},
},
-   .num_lvds = 0,
   };
   
   static const struct rcar_du_device_info rcar_du_r8a7794_info = {

@@ -186,7 +184,6 @@ static const struct rcar_du_device_info
rcar_du_r8a7794_info = {
.port = 1,
},
},
-   .num_lvds = 0,


I think you can remove *all* such initializers and the field itself with
them -- otherwise it looks like you're doing a not-quite-related drive-by
clean up...


The OF compatibility code uses the .num_lvds field, that's why I haven't
removed it.


   Ah, I haven't yet reviewed that patch! But then I would leave the 
initializers alone...


MBR, Sergei

Re: [PATCH v2] vsp1: fix video output on R8A77970

[Laurent Pinchart]

Hi Sergei,

On Monday, 15 January 2018 18:06:48 EET Sergei Shtylyov wrote:
> On 01/15/2018 03:51 PM, Laurent Pinchart wrote:
> > On Tuesday, 26 December 2017 23:14:12 EET Sergei Shtylyov wrote:
> >> Laurent has added support for the VSP2-D found on R-Car V3M (R8A77970)
> >> but
> > 
> > I'm not sure there's a need to state my name in the commit message.
> 
> You were the author of the patch this one has in the Fixes: tag, so I
> thought that was appropriate. I can remove that if you don't want you name
> mentioned...

It's just that I thought it was more important to refer to the patch than to 
my name, developers reading the log will be more interested in the technical 
details than in my person :-)

> >> the video  output that VSP2-D sends to DU has a greenish garbage-like
> >> line
> > 
> > Why does the text in your patches (commit message, comments, ...) sometime
> > have double spaces between words ?
> 
> The text looks more pleasant (at least to me). Can remove them if you
> want...

I don't think we try to do typesetting in kernel code or commit messages, so 
I'd stick to single spaces if you don't mind.

> >> repeated every 8 or so screen rows.
> > 
> > Is it every "8 or so" rows, or exactly every 8 rows ?
> 
> You really want me to count the pixels?

I'd like to know a bit more about the systems, whether the green lines appear 
at the exact same interval, whether they bounce up and down or are always at 
the same place, ...

> >> It turns out that V3M has a teeny LIF register (at least it's
> >> documented!) that you need to set to some kind of a  magic value for the
> >> LIF to work correctly...
> >> 
> >> Based on the original (and large) patch by Daisuke Matsushita
> >> .
> > 
> > What else is in the big patch ? Is it available somewhere ?
> 
> Assorted changes gathered together only because they all bring the
> support for R8A77970. If you're really curious, here you are:
> 
> https://github.com/CogentEmbedded/meta-rcar/blob/v2.12.0/meta-rcar-gen3/reci
> pes-kernel/linux/linux-renesas/0030-arm64-renesas-r8a7797-Add-Renesas-R8A779
> 7-SoC-suppor.patch

Thank you.

> >> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
> >> VSP2-D instances")
> >> Signed-off-by: Sergei Shtylyov 
> >> 
> >> ---
> >> This patch is against the 'media_tree.git' repo's 'master' branch.
> >> 
> >> Changes in version 2:
> >> - added a  comment before the V3M SoC check;
> >> - fixed indetation in that check;
> >> - reformatted  the patch description.
> >> 
> >>   drivers/media/platform/vsp1/vsp1_lif.c  |   12 
> >>   drivers/media/platform/vsp1/vsp1_regs.h |5 +
> >>   2 files changed, 17 insertions(+)
> >> 
> >> Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> >> ===
> >> --- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
> >> +++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> >> @@ -155,6 +155,18 @@ static void lif_configure(struct vsp1_en
> >>(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
> >>(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
> >>VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
> >> +
> >> +  /*
> >> +   * R-Car V3M has the buffer attribute register you absolutely need
> >> +   * to write kinda magic value to  for the LIF to work correctly...
> >> +   */
> > 
> > I'm not sure about the "kinda" magic value. 1536 is very likely a buffer
> > size.
> 
> Well, that's only guessing. The manual doesn't say anything about what the
> number is.

Yes it's just an educated guess.

> > How about the following text ?
> > 
> > /*
> > 
> >  * On V3M the LBA register has to be set to a non-default value to
> 
> I'd then spell its full name, LIF0 Buffer Attribute register.

Sounds good to me.

> >  * guarantee proper operation (otherwise artifacts may appear on the
> >  * output). The value required by the datasheet is not documented but
> >  * is likely a buffer size or threshold.
> >  */
> 
> OK,  can change that (modulo the name).
> 
> > The commit message should also be updated to feel a bit less magic.
> 
> I'll see about it.
> 
> >> +  if ((entity->vsp1->version &
> >> +   (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
> >> +  (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
> >> +  vsp1_lif_write(lif, dl, VI6_LIF_LBA,
> >> + VI6_LIF_LBA_LBA0 |
> >> + (1536 << VI6_LIF_LBA_LBA1_SHIFT));
> >> +  }
> >> 
> >>   }
> > 
> > The datasheet documents the register as being present on both V3M and M3-W
> > (and the test I've just run on H3 shows that the register is present there
> > as well). Should we program it on M3-W or leave it to the default value
> > that should be what is recommended by the datasheet for that SoC ?
> 
> If the default value matches what's 

Re: [PATCH v2 04/12] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Laurent Pinchart]

Hi Sergei,

On Monday, 15 January 2018 22:25:16 EET Sergei Shtylyov wrote:
> On 01/13/2018 02:14 AM, Laurent Pinchart wrote:
> > The LVDS encoders used to be described in DT as part of the DU. They now
> > have their own DT node, linked to the DU using the OF graph bindings.
> > This allows moving internal LVDS encoder support to a separate driver
> > modelled as a DRM bridge. Backward compatibility is retained as legacy
> > DT is patched live to move to the new bindings.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> [...]
> 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 6e02c762a557..06a3fbdd728a
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> 
> [...]
> 
> >   /* 
> > @@ -74,7 +75,6 @@ static const struct rcar_du_device_info
> > rzg1_du_r8a7745_info = {
> > .port = 1,
 >  },
> > },
> > -   .num_lvds = 0,
> >   };
> >   
> >   static const struct rcar_du_device_info rcar_du_r8a7779_info = {
> > @@ -95,14 +95,13 @@ static const struct rcar_du_device_info
> > rcar_du_r8a7779_info = {
> > .port = 1,
> > },
> > },
> > -   .num_lvds = 0,
> >   };
> >   
> >   static const struct rcar_du_device_info rcar_du_r8a7790_info = {
> > .gen = 2,
> > .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> >   | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> > -   .quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
> > +   .quirks = RCAR_DU_QUIRK_ALIGN_128B,
> > .num_crtcs = 3,
> > .routes = {
> > /*
> > @@ -164,7 +163,6 @@ static const struct rcar_du_device_info
> > rcar_du_r8a7792_info = {
> > .port = 1,
> > },
> > },
> > -   .num_lvds = 0,
> >   };
> >   
> >   static const struct rcar_du_device_info rcar_du_r8a7794_info = {
> > @@ -186,7 +184,6 @@ static const struct rcar_du_device_info
> > rcar_du_r8a7794_info = {
> > .port = 1,
> > },
> > },
> > -   .num_lvds = 0,
> 
> I think you can remove *all* such initializers and the field itself with
> them -- otherwise it looks like you're doing a not-quite-related drive-by
> clean up...

The OF compatibility code uses the .num_lvds field, that's why I haven't 
removed it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Frank,

On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
> On 01/15/18 11:22, Laurent Pinchart wrote:
> > On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> >> On 01/15/18 09:09, Rob Herring wrote:
> >>> +Frank
> >>> 
> >>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>  The internal LVDS encoders now have their own DT bindings. Before
>  switching the driver infrastructure to those new bindings, implement
>  backward-compatibility through live DT patching.
> >>> 
> >>> Uhh, we just got rid of TI's patching and now adding this one. I guess
> 
> Let me first answer the question that you ask later.  You ask "Can we work
> on this together to find a solution that would suit us both ?"
> 
> My answer to that is emphatically YES.  I will definitely work with you to
> try to find a good solution.

\o/

> >> Please no.  What we just got rid of was making it difficult for me to
> >> make changes to the overlay infrastructure.  There are issues with
> >> overlays that need to be fixed before overlays become really usable.
> >> I am about to propose the next change, which involves removing a
> >> chunk of code that I will not be able to remove if this patch is
> >> accepted (the proposal is awaiting me collecting some data about
> >> the impact of the change, which I expect to complete this week).
> 
> I should have thought just a little bit more before I hit send.  The
> situation is even worse than I wrote.  One of the next steps (in
> addition to what I wrote about above) is to change the overlay apply
> function to accept a flattened device tree (FDT), not an expanded
> device tree.  In this changed model, the unflattened overlay is
> not available to be modified before it is applied.

That makes sense if we consider overlays to be immutable objects that we apply 
without offering a way to modify them. I won't challenge that API decision, as 
my use of an overlay here is a bit of a hack indeed.

> It is important for the devicetree infrastructure to have ownership
> of the FDT that is used to create the unflattened tree.  (Notice
> that in the proposed patch, rcar_du_of_get_overlay() follows the
> TI example of doing a kmemdup of the blob (FDT), then uses the
> copy for the unflatten.  The kmemdup() in this case is to create
> a persistent copy of the FDT.)  The driver having ownership of
> this copy, and having the ability to free it is one of the many
> problems with the current overlay implementation.

Yes, that's something I've identified as well. Lots of work has been done to 
clean up the OF core and we're clearly not done yet. I'm happy to see all the 
improvements you're working on.

> >> Can you please handle both the old and new bindings through driver
> >> code instead?
> > 
> > I could, but it would be pointless. The point here is to allow cleanups in
> > the driver. The LVDS encoder handling code is very intrusive in its
> > current form and I need to get rid of it. There would be zero point in
> > moving to the new infrastructure, as the main point is to get rid of the
> > old code which prevents moving forward. As a consequence that would block
> > new boards from receiving proper upstream support. An easy option is to
> > break backward compatibility. I'm personally fine with that, but I assume
> > other people would complain :-)
> > 
> > I can, on the other hand, work with you to see how live DT patching could
> > be implemented in this driver without blocking your code. When developing
> > this patch series I start by patching the device tree manually without
> > relying on overlays at all, but got blocked by the fact that I need to
> > allocate phandles for new nodes I create. If there was an API to allocate
> > an unused phandle I could avoid using the overlay infrastructure at all.
> > Or there could be other
> 
> It seems reasonable to provide a way to autogenerate a phandle (if
> requested) by the devicetree code that creates a new node.  Were you using
> a function from drivers/of/dynamic.c to create the node?

Not to allocate the node, no. I allocated the device_node structure manually 
with kzalloc(), and inserted it in the device tree with of_attach_node(). Is 
that the right approach ? I haven't been able to test the code as I stopped 
when I realized I couldn't allocate phandles.

> > options I'm not thinking of as I don't know what the changes you're
> > working on are. Can we work on this together to find a solution that
> > would suit us both ?
> 
> Again, yes, I would be glad to work with you on this.

How would you like to proceed ? I can try the manual approach again but need 
information about how I could cleanly implement phandle allocation. I will 
likely then run into other issues for which I might need help.

> >>> it's necessary, but I'd like to know how long we need to keep this?
> >>> 
> >>> How many overlays would you need if everything is static (i.e.
> >>> removing all your fixup code)?
> >>> 
>  Patching is 

Re: [PATCH v2 04/12] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Sergei Shtylyov]

Hello!

On 01/13/2018 02:14 AM, Laurent Pinchart wrote:


The LVDS encoders used to be described in DT as part of the DU. They now
have their own DT node, linked to the DU using the OF graph bindings.
This allows moving internal LVDS encoder support to a separate driver
modelled as a DRM bridge. Backward compatibility is retained as legacy
DT is patched live to move to the new bindings.

Signed-off-by: Laurent Pinchart 

[...]

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 6e02c762a557..06a3fbdd728a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c

[...]

  /* 
-
@@ -74,7 +75,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info 
= {
.port = 1,
},
},
-   .num_lvds = 0,
  };
  
  static const struct rcar_du_device_info rcar_du_r8a7779_info = {

@@ -95,14 +95,13 @@ static const struct rcar_du_device_info 
rcar_du_r8a7779_info = {
.port = 1,
},
},
-   .num_lvds = 0,
  };
  
  static const struct rcar_du_device_info rcar_du_r8a7790_info = {

.gen = 2,
.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-   .quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
+   .quirks = RCAR_DU_QUIRK_ALIGN_128B,
.num_crtcs = 3,
.routes = {
/*
@@ -164,7 +163,6 @@ static const struct rcar_du_device_info 
rcar_du_r8a7792_info = {
.port = 1,
},
},
-   .num_lvds = 0,
  };
  
  static const struct rcar_du_device_info rcar_du_r8a7794_info = {

@@ -186,7 +184,6 @@ static const struct rcar_du_device_info 
rcar_du_r8a7794_info = {
.port = 1,
},
},
-   .num_lvds = 0,


   I think you can remove *all* such initializers and the field itself with 
them -- otherwise it looks like you're doing a not-quite-related drive-by 
clean up...


[...]

MBR, Sergei

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 11:22, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
>> On 01/15/18 09:09, Rob Herring wrote:
>>> +Frank
>>>
>>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
 The internal LVDS encoders now have their own DT bindings. Before
 switching the driver infrastructure to those new bindings, implement
 backward-compatibility through live DT patching.
>>>
>>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>

Let me first answer the question that you ask later.  You ask "Can we work on
this together to find a solution that would suit us both ?"

My answer to that is emphatically YES.  I will definitely work with you to
try to find a good solution.


>> Please no.  What we just got rid of was making it difficult for me to
>> make changes to the overlay infrastructure.  There are issues with
>> overlays that need to be fixed before overlays become really usable.
>> I am about to propose the next change, which involves removing a
>> chunk of code that I will not be able to remove if this patch is
>> accepted (the proposal is awaiting me collecting some data about
>> the impact of the change, which I expect to complete this week).

I should have thought just a little bit more before I hit send.  The
situation is even worse than I wrote.  One of the next steps (in
addition to what I wrote about above) is to change the overlay apply
function to accept a flattened device tree (FDT), not an expanded
device tree.  In this changed model, the unflattened overlay is
not available to be modified before it is applied.

It is important for the devicetree infrastructure to have ownership
of the FDT that is used to create the unflattened tree.  (Notice
that in the proposed patch, rcar_du_of_get_overlay() follows the
TI example of doing a kmemdup of the blob (FDT), then uses the
copy for the unflatten.  The kmemdup() in this case is to create
a persistent copy of the FDT.)  The driver having ownership of
this copy, and having the ability to free it is one of the many
problems with the current overlay implementation.

>>
>> Can you please handle both the old and new bindings through driver
>> code instead?
> 
> I could, but it would be pointless. The point here is to allow cleanups in 
> the 
> driver. The LVDS encoder handling code is very intrusive in its current form 
> and I need to get rid of it. There would be zero point in moving to the new 
> infrastructure, as the main point is to get rid of the old code which 
> prevents 
> moving forward. As a consequence that would block new boards from receiving 
> proper upstream support. An easy option is to break backward compatibility. 
> I'm personally fine with that, but I assume other people would complain :-)
> 
> I can, on the other hand, work with you to see how live DT patching could be 
> implemented in this driver without blocking your code. When developing this 
> patch series I start by patching the device tree manually without relying on 
> overlays at all, but got blocked by the fact that I need to allocate phandles 
> for new nodes I create. If there was an API to allocate an unused phandle I 
> could avoid using the overlay infrastructure at all. Or there could be other 

It seems reasonable to provide a way to autogenerate a phandle (if requested)
by the devicetree code that creates a new node.  Were you using a function
from drivers/of/dynamic.c to create the node?


> options I'm not thinking of as I don't know what the changes you're working 
> on 
> are. Can we work on this together to find a solution that would suit us both ?

Again, yes, I would be glad to work with you on this.

-Frank

> 
>>> it's necessary, but I'd like to know how long we need to keep this?
>>>
>>> How many overlays would you need if everything is static (i.e.
>>> removing all your fixup code)?
>>>
 Patching is disabled and will be enabled along with support for the new
 DT bindings in the DU driver.

 Signed-off-by: Laurent Pinchart
 
 ---
 Changes since v1:

 - Select OF_FLATTREE
 - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
 - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
 - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
 - Pass void begin and end pointers to rcar_du_of_get_overlay()
 - Use of_get_parent() instead of accessing the parent pointer directly
 - Find the LVDS endpoints nodes based on the LVDS node instead of the
   root of the overlay
 - Update to the -lvds compatible string format
 ---

  drivers/gpu/drm/rcar-du/Kconfig |   2 +
  drivers/gpu/drm/rcar-du/Makefile|   3 +-
  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 ++
  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +

Re: [PATCH 0/2] sh_eth: simplify TSU initialization

[David Miller]

From: Sergei Shtylyov 
Date: Sun, 14 Jan 2018 20:47:42 +0300

> Here's a set of 2 patches against DaveM's 'net-next.git' repo. With those,
> I'm somewhat simplifying the TSU init code in the driver probe() method...
> 
> [1/2] sh_eth: gather all TSU init code in one place
> [2/2] sh_eth: get Ether port # only when needed

Series applied, thanks Sergei.

Re: [PATCH] sh_eth: dix dumping ARSTR

[David Miller]

From: Sergei Shtylyov 
Date: Sat, 13 Jan 2018 20:22:01 +0300

> ARSTR  is always located at the start of the TSU register region, thus
> using add_reg()  instead of add_tsu_reg() in __sh_eth_get_regs() to dump it
> causes EDMR or EDSR (depending on the register layout) to be dumped instead
> of ARSTR.  Use the correct condition/macro there...
> 
> Fixes: 6b4b4fead342 ("sh_eth: Implement ethtool register dump operations")
> Signed-off-by: Sergei Shtylyov 

Applied to 'net' with subj. typo fixed :)

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Frank,

On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> On 01/15/18 09:09, Rob Herring wrote:
> > +Frank
> > 
> > On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> >> The internal LVDS encoders now have their own DT bindings. Before
> >> switching the driver infrastructure to those new bindings, implement
> >> backward-compatibility through live DT patching.
> > 
> > Uhh, we just got rid of TI's patching and now adding this one. I guess
> 
> Please no.  What we just got rid of was making it difficult for me to
> make changes to the overlay infrastructure.  There are issues with
> overlays that need to be fixed before overlays become really usable.
> I am about to propose the next change, which involves removing a
> chunk of code that I will not be able to remove if this patch is
> accepted (the proposal is awaiting me collecting some data about
> the impact of the change, which I expect to complete this week).
> 
> Can you please handle both the old and new bindings through driver
> code instead?

I could, but it would be pointless. The point here is to allow cleanups in the 
driver. The LVDS encoder handling code is very intrusive in its current form 
and I need to get rid of it. There would be zero point in moving to the new 
infrastructure, as the main point is to get rid of the old code which prevents 
moving forward. As a consequence that would block new boards from receiving 
proper upstream support. An easy option is to break backward compatibility. 
I'm personally fine with that, but I assume other people would complain :-)

I can, on the other hand, work with you to see how live DT patching could be 
implemented in this driver without blocking your code. When developing this 
patch series I start by patching the device tree manually without relying on 
overlays at all, but got blocked by the fact that I need to allocate phandles 
for new nodes I create. If there was an API to allocate an unused phandle I 
could avoid using the overlay infrastructure at all. Or there could be other 
options I'm not thinking of as I don't know what the changes you're working on 
are. Can we work on this together to find a solution that would suit us both ?

> > it's necessary, but I'd like to know how long we need to keep this?
> > 
> > How many overlays would you need if everything is static (i.e.
> > removing all your fixup code)?
> > 
> >> Patching is disabled and will be enabled along with support for the new
> >> DT bindings in the DU driver.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> 
> >> ---
> >> Changes since v1:
> >> 
> >> - Select OF_FLATTREE
> >> - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
> >> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> >> - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
> >> - Pass void begin and end pointers to rcar_du_of_get_overlay()
> >> - Use of_get_parent() instead of accessing the parent pointer directly
> >> - Find the LVDS endpoints nodes based on the LVDS node instead of the
> >>   root of the overlay
> >> - Update to the -lvds compatible string format
> >> ---
> >> 
> >>  drivers/gpu/drm/rcar-du/Kconfig |   2 +
> >>  drivers/gpu/drm/rcar-du/Makefile|   3 +-
> >>  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 ++
> >>  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
> >>  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
> >>  5 files changed, 553 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
> >>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
> >>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 09:09, Rob Herring wrote:
> +Frank
> 
> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart
>  wrote:
>> The internal LVDS encoders now have their own DT bindings. Before
>> switching the driver infrastructure to those new bindings, implement
>> backward-compatibility through live DT patching.
> 
> Uhh, we just got rid of TI's patching and now adding this one. I guess

Please no.  What we just got rid of was making it difficult for me to
make changes to the overlay infrastructure.  There are issues with
overlays that need to be fixed before overlays become really usable.
I am about to propose the next change, which involves removing a
chunk of code that I will not be able to remove if this patch is
accepted (the proposal is awaiting me collecting some data about
the impact of the change, which I expect to complete this week).

Can you please handle both the old and new bindings through driver
code instead?

Thanks,

Frank


> it's necessary, but I'd like to know how long we need to keep this?
> 
> How many overlays would you need if everything is static (i.e.
> removing all your fixup code)?
> 
>> Patching is disabled and will be enabled along with support for the new
>> DT bindings in the DU driver.
>>
>> Signed-off-by: Laurent Pinchart 
>> ---
>> Changes since v1:
>>
>> - Select OF_FLATTREE
>> - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
>> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
>> - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
>> - Pass void begin and end pointers to rcar_du_of_get_overlay()
>> - Use of_get_parent() instead of accessing the parent pointer directly
>> - Find the LVDS endpoints nodes based on the LVDS node instead of the
>>   root of the overlay
>> - Update to the -lvds compatible string format
>> ---
>>  drivers/gpu/drm/rcar-du/Kconfig |   2 +
>>  drivers/gpu/drm/rcar-du/Makefile|   3 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 
>> 
>>  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
>>  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
>>  5 files changed, 553 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts
>>
>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig 
>> b/drivers/gpu/drm/rcar-du/Kconfig
>> index 5d0b4b7119af..3f83352a7313 100644
>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>> @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
>> bool "R-Car DU LVDS Encoder Support"
>> depends on DRM_RCAR_DU
>> select DRM_PANEL
>> +   select OF_FLATTREE
>> +   select OF_OVERLAY
> 
> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
> could have an overlay from a non-FDT source...
> 
>> help
>>   Enable support for the R-Car Display Unit embedded LVDS encoders.
>>
>> diff --git a/drivers/gpu/drm/rcar-du/Makefile 
>> b/drivers/gpu/drm/rcar-du/Makefile
>> index 0cf5c11030e8..8ed569a0f462 100644
>> --- a/drivers/gpu/drm/rcar-du/Makefile
>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>> @@ -5,10 +5,11 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>  rcar_du_group.o \
>>  rcar_du_kms.o \
>>  rcar_du_lvdscon.o \
>> +rcar_du_of.o \
>>  rcar_du_plane.o
>>
>>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
>> -
>> +rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of_lvds.dtb.o
>>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
>>
>>  obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
>> b/drivers/gpu/drm/rcar-du/rcar_du_of.c
>> new file mode 100644
>> index ..2bf91201fc93
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
>> @@ -0,0 +1,451 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * rcar_du_of.c - Legacy DT bindings compatibility
>> + *
>> + * Copyright (C) 2018 Laurent Pinchart 
>> + *
>> + * Based on work from Jyri Sarha 
>> + * Copyright (C) 2015 Texas Instruments
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "rcar_du_crtc.h"
>> +#include "rcar_du_drv.h"
>> +
>> +#ifdef CONFIG_DRM_RCAR_LVDS
> 
> Why not make compiling this file conditional in the Makefile?
> 
>> +
>> +struct rcar_du_of_overlay {
>> +   struct device_node *np;
>> +   void *data;
>> +   void *mem;
>> +};
>> +
>> +static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay 
>> *overlay)
>> +{
>> +   of_node_put(overlay->np);
>> +   kfree(overlay->data);
>> +   kfree(overlay->mem);
>> +}
>> +
>> +static int 

[PATCH/RFT v3] gpio: gpio-rcar: Support S2RAM

[Yoshihiro Kaneko]

From: Hien Dang 

This patch adds an implementation that saves and restores the state of
GPIO configuration on suspend and resume.

Signed-off-by: Hien Dang 
Signed-off-by: Takeshi Kihara 
[Modify structure of the bank info to simplify a saving registers]
[Remove DEV_PM_OPS macro]
Signed-off-by: Yoshihiro Kaneko 
---

This patch is based on the for-next branch of linux-gpio tree.

v2 [Yoshihiro Kaneko]
* Modify structure of the bank info as suggested by Geert Uytterhoeven

v3 [Yoshihiro Kaneko]
* Remove DEV_PM_OPS macro as suggested by Vladimir Zapolskiy

 drivers/gpio/gpio-rcar.c | 65 
 1 file changed, 65 insertions(+)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index e76de57..88fbb3e 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -31,6 +31,16 @@
 #include 
 #include 
 
+struct gpio_rcar_bank_info {
+   u32 iointsel;
+   u32 inoutsel;
+   u32 outdt;
+   u32 active_high_rising_edge;
+   u32 level_trigger;
+   u32 both;
+   u32 intmsk;
+};
+
 struct gpio_rcar_priv {
void __iomem *base;
spinlock_t lock;
@@ -41,6 +51,7 @@ struct gpio_rcar_priv {
unsigned int irq_parent;
bool has_both_edge_trigger;
bool needs_clk;
+   struct gpio_rcar_bank_info bank_info;
 };
 
 #define IOINTSEL 0x00  /* General IO/Interrupt Switching Register */
@@ -531,11 +542,65 @@ static int gpio_rcar_remove(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int gpio_rcar_suspend(struct device *dev)
+{
+   struct gpio_rcar_priv *p = dev_get_drvdata(dev);
+
+   p->bank_info.iointsel = gpio_rcar_read(p, IOINTSEL);
+   p->bank_info.inoutsel = gpio_rcar_read(p, INOUTSEL);
+   p->bank_info.outdt = gpio_rcar_read(p, OUTDT);
+   p->bank_info.intmsk = gpio_rcar_read(p, INTMSK);
+   p->bank_info.active_high_rising_edge = gpio_rcar_read(p, POSNEG);
+   p->bank_info.level_trigger = gpio_rcar_read(p, EDGLEVEL);
+   p->bank_info.both = gpio_rcar_read(p, BOTHEDGE);
+
+   return 0;
+}
+
+static int gpio_rcar_resume(struct device *dev)
+{
+   struct gpio_rcar_priv *p = dev_get_drvdata(dev);
+   int offset;
+   u32 mask;
+
+   for (offset = 0; offset < p->gpio_chip.ngpio; offset++) {
+   mask = BIT(offset);
+   /* I/O pin */
+   if (!(p->bank_info.iointsel & mask)) {
+   if (p->bank_info.inoutsel & mask)
+   gpio_rcar_direction_output(
+   >gpio_chip, offset,
+   !!(p->bank_info.outdt & mask));
+   else
+   gpio_rcar_direction_input(>gpio_chip,
+ offset);
+   /* Interrupt pin */
+   } else {
+   gpio_rcar_config_interrupt_input_mode(
+   p,
+   offset,
+   !(p->bank_info.active_high_rising_edge & mask),
+   !!(p->bank_info.level_trigger & mask),
+   !!(p->bank_info.both & mask));
+
+   if (p->bank_info.intmsk & mask)
+   gpio_rcar_write(p, MSKCLR, mask);
+   }
+   }
+
+   return 0;
+}
+#endif /* CONFIG_PM_SLEEP*/
+
+static SIMPLE_DEV_PM_OPS(gpio_rcar_pm_ops, gpio_rcar_suspend, 
gpio_rcar_resume);
+
 static struct platform_driver gpio_rcar_device_driver = {
.probe  = gpio_rcar_probe,
.remove = gpio_rcar_remove,
.driver = {
.name   = "gpio_rcar",
+   .pm = _rcar_pm_ops,
.of_match_table = of_match_ptr(gpio_rcar_of_table),
}
 };
-- 
1.9.1

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Rob,

(CC'ing Geert)

On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
> +Frank
> 
> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> > The internal LVDS encoders now have their own DT bindings. Before
> > switching the driver infrastructure to those new bindings, implement
> > backward-compatibility through live DT patching.
> 
> Uhh, we just got rid of TI's patching and now adding this one. I guess
> it's necessary, but I'd like to know how long we need to keep this?

That's a good question. How long are we supposed to keep DT backward 
compatibility for ? I don't think there's a one-size-fits-them-all answer to 
this question. Geert, any opinion ? How long do you plan to keep the CPG 
(clocks) DT backward compatibility for instance ?

> How many overlays would you need if everything is static (i.e. removing all
> your fixup code)?

Do you mean if I hardcoded support for SoCs individually instead of handling 
the differences in code ? At least 5 as that's the number of SoCs that are 
impacted, but that wouldn't be enough. Part of the DT structure that is 
patched is board-specific, not SoC-specific. That's 10 boards in mainline, 
plus all the custom boards out there, and even the development boards 
supported in mainline to which a flat panel has been externally connected.

> > Patching is disabled and will be enabled along with support for the new
> > DT bindings in the DU driver.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > Changes since v1:
> > 
> > - Select OF_FLATTREE
> > - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
> > - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> > - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
> > - Pass void begin and end pointers to rcar_du_of_get_overlay()
> > - Use of_get_parent() instead of accessing the parent pointer directly
> > - Find the LVDS endpoints nodes based on the LVDS node instead of the
> >   root of the overlay
> > - Update to the -lvds compatible string format
> > ---
> >  drivers/gpu/drm/rcar-du/Kconfig |   2 +
> >  drivers/gpu/drm/rcar-du/Makefile|   3 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 +++
> >  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
> >  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
> >  5 files changed, 553 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > b/drivers/gpu/drm/rcar-du/Kconfig index 5d0b4b7119af..3f83352a7313 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
> > bool "R-Car DU LVDS Encoder Support"
> > depends on DRM_RCAR_DU
> > select DRM_PANEL
> > +   select OF_FLATTREE
> > +   select OF_OVERLAY
> 
> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
> could have an overlay from a non-FDT source...

Up to you, I'll happily drop OF_FLATTREE if it gets selected automatically. If 
you think it's a good idea I can submit a patch.

> > help
> >   Enable support for the R-Car Display Unit embedded LVDS
> >   encoders.

[snip]

> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_of.c new file mode 100644
> > index ..2bf91201fc93
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > @@ -0,0 +1,451 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * rcar_du_of.c - Legacy DT bindings compatibility
> > + *
> > + * Copyright (C) 2018 Laurent Pinchart
> > 
> > + *
> > + * Based on work from Jyri Sarha 
> > + * Copyright (C) 2015 Texas Instruments
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "rcar_du_crtc.h"
> > +#include "rcar_du_drv.h"
> > +
> > +#ifdef CONFIG_DRM_RCAR_LVDS
> 
> Why not make compiling this file conditional in the Makefile?

In case I need to patch other DU-related constructs in the future other than 
LVDS. I could compile this conditionally in the Makefile for now and change it 
later if needed. I have no strong preference.

> > +
> > +struct rcar_du_of_overlay {
> > +   struct device_node *np;
> > +   void *data;
> > +   void *mem;
> > +};
> > +
> > +static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay
> > *overlay)
> > +{
> > +   of_node_put(overlay->np);
> > +   kfree(overlay->data);
> > +   kfree(overlay->mem);
> > +}
> > +
> > +static int __init rcar_du_of_get_overlay(struct rcar_du_of_overlay
> > *overlay,
> > +void *begin, void
> > 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

+Frank

On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart
 wrote:
> The internal LVDS encoders now have their own DT bindings. Before
> switching the driver infrastructure to those new bindings, implement
> backward-compatibility through live DT patching.

Uhh, we just got rid of TI's patching and now adding this one. I guess
it's necessary, but I'd like to know how long we need to keep this?

How many overlays would you need if everything is static (i.e.
removing all your fixup code)?

> Patching is disabled and will be enabled along with support for the new
> DT bindings in the DU driver.
>
> Signed-off-by: Laurent Pinchart 
> ---
> Changes since v1:
>
> - Select OF_FLATTREE
> - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
> - Pass void begin and end pointers to rcar_du_of_get_overlay()
> - Use of_get_parent() instead of accessing the parent pointer directly
> - Find the LVDS endpoints nodes based on the LVDS node instead of the
>   root of the overlay
> - Update to the -lvds compatible string format
> ---
>  drivers/gpu/drm/rcar-du/Kconfig |   2 +
>  drivers/gpu/drm/rcar-du/Makefile|   3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 
> 
>  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
>  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
>  5 files changed, 553 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts
>
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 5d0b4b7119af..3f83352a7313 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
> bool "R-Car DU LVDS Encoder Support"
> depends on DRM_RCAR_DU
> select DRM_PANEL
> +   select OF_FLATTREE
> +   select OF_OVERLAY

OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
could have an overlay from a non-FDT source...

> help
>   Enable support for the R-Car Display Unit embedded LVDS encoders.
>
> diff --git a/drivers/gpu/drm/rcar-du/Makefile 
> b/drivers/gpu/drm/rcar-du/Makefile
> index 0cf5c11030e8..8ed569a0f462 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -5,10 +5,11 @@ rcar-du-drm-y := rcar_du_crtc.o \
>  rcar_du_group.o \
>  rcar_du_kms.o \
>  rcar_du_lvdscon.o \
> +rcar_du_of.o \
>  rcar_du_plane.o
>
>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
> -
> +rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of_lvds.dtb.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
>
>  obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> new file mode 100644
> index ..2bf91201fc93
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -0,0 +1,451 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * rcar_du_of.c - Legacy DT bindings compatibility
> + *
> + * Copyright (C) 2018 Laurent Pinchart 
> + *
> + * Based on work from Jyri Sarha 
> + * Copyright (C) 2015 Texas Instruments
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "rcar_du_crtc.h"
> +#include "rcar_du_drv.h"
> +
> +#ifdef CONFIG_DRM_RCAR_LVDS

Why not make compiling this file conditional in the Makefile?

> +
> +struct rcar_du_of_overlay {
> +   struct device_node *np;
> +   void *data;
> +   void *mem;
> +};
> +
> +static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay 
> *overlay)
> +{
> +   of_node_put(overlay->np);
> +   kfree(overlay->data);
> +   kfree(overlay->mem);
> +}
> +
> +static int __init rcar_du_of_get_overlay(struct rcar_du_of_overlay *overlay,
> +void *begin, void *end)
> +{
> +   const size_t size = end - begin;
> +
> +   memset(overlay, 0, sizeof(*overlay));
> +
> +   if (!size)
> +   return -EINVAL;
> +
> +   overlay->data = kmemdup(begin, size, GFP_KERNEL);
> +   if (!overlay->data)
> +   return -ENOMEM;
> +
> +   overlay->mem = of_fdt_unflatten_tree(overlay->data, NULL, 
> >np);
> +   if (!overlay->mem) {
> +   rcar_du_of_free_overlay(overlay);
> +   return -ENOMEM;
> +   }
> +
> +   return 0;
> +}
> +
> +static struct device_node __init *
> +rcar_du_of_find_node_by_path(struct device_node 

Re: [RFC PATCH 0/7] i2c: sh_mobile: support per-SoC frequency calculation

[Wolfram Sang]

On Mon, Dec 18, 2017 at 10:57:55PM +0100, Wolfram Sang wrote:
> The current calculation for I2C bus speeds does not match the datasheet and 
> is,
> in deed, a little too fast for 100kHz settings (~107kHz have been measured).
> 
> This series implements a second calculation according to the datasheet. In
> order to not cause regressions on the various old SoCs, using this new formula
> is opt-in per SoC. Once the new formula has been verified to work, we can
> switch over. By default, the old behaviour is retained.
> 
> So, the first patches are refactoring the driver to support per-SoC frequency
> calculations. This is done by making the already existing per-SoC
> setup()-callback mandatory and let the frequency calculation be done in there.
> 
> This series has been tested on a R-Car H2 Lager board (r8a7790). The computed
> values match the suggested values in the datasheet. Also, measurements showed
> that the bus frequency is now 100kHz. For 400 kHz, the overall bus speed did
> not change much, but the low-to-high relationship is now matching the
> datasheet, too.
> 
> This series is RFC because I think we need to discuss when to enable a SoC to
> the new formula. For Lager, I did some basic checking but did not run a full
> V4L testsuite or other complex configurations making use of I2C clients. Gen3,
> in theory, should be easier to test because only the PMIC and an EEPROM are
> connected to IIC_DVFS. However, there are no testpoints available for 
> measuring
> on Salvator-X(S). And the suggested values in the datasheet do not match the
> module clock we have. Do ULCB boards maybe have testpoints for IIC_DVFS? For
> Lager, it could be argued that the resulting speed is slower than before which
> should not introduce regressions. But I am open to discussion here.
> 
> A branch can be found here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/iic-refactor
> 
> Thanks, and looking forward to comments
> 
>Wolfram
> 
> 
> Wolfram Sang (7):
>   i2c: sh_mobile: move type detection upwards
>   i2c: sh_mobile: allow setup callback to return errno
>   i2c: sh_mobile: require setup callback
>   i2c: sh_mobile: let RuntimePM do the clock handling
>   i2c: sh_mobile: add helper to check frequency calculations
>   i2c: sh_mobile: add new frequency calculation for later SoC
>   i2c: sh_mobile: let r8a7790 (R-Car H2) use the new formula
> 

Fixed a spelling mistake in the patch description of patch 5 and applied
to for-next.



signature.asc
Description: PGP signature

Re: [PATCH] [VSP-Tests] tests: add pseudo platform test

[Kieran Bingham]

Ahem, ...

Not that it really matters but perhaps that should read 'add platform pseudo
test' instead.?

--
Kieran


On 15/01/18 16:47, Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Provide an initial test which can run as part of the test suite.
> This test will report the platform and kernel version, along with
> the identified paths of required utilities.
> 
> This will aid in ensuring that required tools are available on a
> running platform - and report the kernel and platform details in
> any test suite output for clarification of results.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  tests/vsp-unit-test-.sh | 20 
>  1 file changed, 20 insertions(+)
>  create mode 100755 tests/vsp-unit-test-.sh
> 
> diff --git a/tests/vsp-unit-test-.sh b/tests/vsp-unit-test-.sh
> new file mode 100755
> index ..144cfc677b32
> --- /dev/null
> +++ b/tests/vsp-unit-test-.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +
> +# Report testing conditions
> +
> +model=`cat /sys/firmware/devicetree/base/model`
> +
> +echo "Test Conditions:"
> +
> +function check_all() {
> + echo "  Platform: " "$model"
> + echo "  Kernel release: " `uname -r`
> + echo "  convert: " `which convert`
> + echo "  compare: " `which compare`
> + echo "  killall: " `which killall`
> + echo "  raw2rgbpnm: " `which raw2rgbpnm`
> + echo "  stress: " `which stress`
> + echo "  yavta: " `which yavta`
> +}
> +
> +check_all | column -ts ":"
> 

[PATCH] [VSP-Tests] tests: add pseudo platform test

[Kieran Bingham]

From: Kieran Bingham 

Provide an initial test which can run as part of the test suite.
This test will report the platform and kernel version, along with
the identified paths of required utilities.

This will aid in ensuring that required tools are available on a
running platform - and report the kernel and platform details in
any test suite output for clarification of results.

Signed-off-by: Kieran Bingham 
---
 tests/vsp-unit-test-.sh | 20 
 1 file changed, 20 insertions(+)
 create mode 100755 tests/vsp-unit-test-.sh

diff --git a/tests/vsp-unit-test-.sh b/tests/vsp-unit-test-.sh
new file mode 100755
index ..144cfc677b32
--- /dev/null
+++ b/tests/vsp-unit-test-.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+# Report testing conditions
+
+model=`cat /sys/firmware/devicetree/base/model`
+
+echo "Test Conditions:"
+
+function check_all() {
+   echo "  Platform: " "$model"
+   echo "  Kernel release: " `uname -r`
+   echo "  convert: " `which convert`
+   echo "  compare: " `which compare`
+   echo "  killall: " `which killall`
+   echo "  raw2rgbpnm: " `which raw2rgbpnm`
+   echo "  stress: " `which stress`
+   echo "  yavta: " `which yavta`
+}
+
+check_all | column -ts ":"
-- 
2.7.4

[PATCH v5 7/9] v4l: vsp1: Adapt entities to configure into a body

[Kieran Bingham]

Currently the entities store their configurations into a display list.
Adapt this such that the code can be configured into a body directly,
allowing greater flexibility and control of the content.

All users of vsp1_dl_list_write() are removed in this process, thus it
too is removed.

A helper, vsp1_dl_list_get_body0() is provided to access the internal body0
from the display list.

Signed-off-by: Kieran Bingham 

---

v4:
 - Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0()
   The similarities between vsp1_dl_list_get_body and
   vsp1_dl_list_body_get() were too close

 - body0 could be removed later when the default body is no longer
   needed.

v5:
 - Support DRM/UIF changes
---
 drivers/media/platform/vsp1/vsp1_bru.c| 22 +--
 drivers/media/platform/vsp1/vsp1_clu.c| 22 +--
 drivers/media/platform/vsp1/vsp1_dl.c | 12 ++
 drivers/media/platform/vsp1/vsp1_dl.h |  2 +-
 drivers/media/platform/vsp1/vsp1_drm.c| 24 +++-
 drivers/media/platform/vsp1/vsp1_entity.c | 16 
 drivers/media/platform/vsp1/vsp1_entity.h | 12 +++---
 drivers/media/platform/vsp1/vsp1_hgo.c| 16 
 drivers/media/platform/vsp1/vsp1_hgt.c| 18 -
 drivers/media/platform/vsp1/vsp1_hsit.c   | 10 ++---
 drivers/media/platform/vsp1/vsp1_lif.c| 13 +++
 drivers/media/platform/vsp1/vsp1_lut.c| 21 +--
 drivers/media/platform/vsp1/vsp1_pipe.c   |  4 +-
 drivers/media/platform/vsp1/vsp1_pipe.h   |  3 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 47 +++-
 drivers/media/platform/vsp1/vsp1_sru.c| 14 +++
 drivers/media/platform/vsp1/vsp1_uds.c| 24 ++--
 drivers/media/platform/vsp1/vsp1_uds.h|  2 +-
 drivers/media/platform/vsp1/vsp1_uif.c| 18 -
 drivers/media/platform/vsp1/vsp1_video.c  | 11 --
 drivers/media/platform/vsp1/vsp1_wpf.c| 42 +++--
 21 files changed, 185 insertions(+), 168 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index b9ff96f76b3e..60d449d7b135 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -30,10 +30,10 @@
  * Device Access
  */
 
-static inline void vsp1_bru_write(struct vsp1_bru *bru, struct vsp1_dl_list 
*dl,
- u32 reg, u32 data)
+static inline void vsp1_bru_write(struct vsp1_bru *bru,
+ struct vsp1_dl_body *dlb, u32 reg, u32 data)
 {
-   vsp1_dl_list_write(dl, bru->base + reg, data);
+   vsp1_dl_body_write(dlb, bru->base + reg, data);
 }
 
 /* 
-
@@ -287,7 +287,7 @@ static const struct v4l2_subdev_ops bru_ops = {
 
 static void bru_prepare(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
-   struct vsp1_dl_list *dl)
+   struct vsp1_dl_body *dlb)
 {
struct vsp1_bru *bru = to_bru(>subdev);
struct v4l2_mbus_framefmt *format;
@@ -309,7 +309,7 @@ static void bru_prepare(struct vsp1_entity *entity,
 * format at the pipeline output is premultiplied.
 */
flags = pipe->output ? pipe->output->format.flags : 0;
-   vsp1_bru_write(bru, dl, VI6_BRU_INCTRL,
+   vsp1_bru_write(bru, dlb, VI6_BRU_INCTRL,
   flags & V4L2_PIX_FMT_FLAG_PREMUL_ALPHA ?
   0 : VI6_BRU_INCTRL_NRM);
 
@@ -317,12 +317,12 @@ static void bru_prepare(struct vsp1_entity *entity,
 * Set the background position to cover the whole output image and
 * configure its color.
 */
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_SIZE,
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_SIZE,
   (format->width << VI6_BRU_VIRRPF_SIZE_HSIZE_SHIFT) |
   (format->height << VI6_BRU_VIRRPF_SIZE_VSIZE_SHIFT));
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_LOC, 0);
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_LOC, 0);
 
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_COL, bru->bgcolor |
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_COL, bru->bgcolor |
   (0xff << VI6_BRU_VIRRPF_COL_A_SHIFT));
 
/*
@@ -332,7 +332,7 @@ static void bru_prepare(struct vsp1_entity *entity,
 * unit.
 */
if (entity->type == VSP1_ENTITY_BRU)
-   vsp1_bru_write(bru, dl, VI6_BRU_ROP,
+   vsp1_bru_write(bru, dlb, VI6_BRU_ROP,
   VI6_BRU_ROP_DSTSEL_BRUIN(1) |
   VI6_BRU_ROP_CROP(VI6_ROP_NOP) |
   VI6_BRU_ROP_AROP(VI6_ROP_NOP));
@@ -374,7 +374,7 @@ static void bru_prepare(struct vsp1_entity *entity,
if (!(entity->type == VSP1_ENTITY_BRU && i == 1))
ctrl |= VI6_BRU_CTRL_SRCSEL_BRUIN(i);
 
-   vsp1_bru_write(bru, dl, 

[PATCH v5 4/9] v4l: vsp1: Convert display lists to use new body pool

[Kieran Bingham]

Adapt the dl->body0 object to use an object from the body pool. This
greatly reduces the pressure on the TLB for IPMMU use cases, as all of
the lists use a single allocation for the main body.

The CLU and LUT objects pre-allocate a pool containing three bodies,
allowing a userspace update before the hardware has committed a previous
set of tables.

Bodies are no longer 'freed' in interrupt context, but instead released
back to their respective pools. This allows us to remove the garbage
collector in the DLM.

Signed-off-by: Kieran Bingham 

---
v3:
 - 's/fragment/body', 's/fragments/bodies/'
 - CLU/LUT now allocate 3 bodies
 - vsp1_dl_list_fragments_free -> vsp1_dl_list_bodies_put

v2:
 - Use dl->body0->max_entries to determine header offset, instead of the
   global constant VSP1_DL_NUM_ENTRIES which is incorrect.
 - squash updates for LUT, CLU, and fragment cleanup into single patch.
   (Not fully bisectable when separated)
---
 drivers/media/platform/vsp1/vsp1_clu.c |  27 ++-
 drivers/media/platform/vsp1/vsp1_clu.h |   1 +-
 drivers/media/platform/vsp1/vsp1_dl.c  | 223 ++
 drivers/media/platform/vsp1/vsp1_dl.h  |   3 +-
 drivers/media/platform/vsp1/vsp1_lut.c |  27 ++-
 drivers/media/platform/vsp1/vsp1_lut.h |   1 +-
 6 files changed, 101 insertions(+), 181 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index 246dd595c978..a765d56c4118 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -23,6 +23,8 @@
 #define CLU_MIN_SIZE   4U
 #define CLU_MAX_SIZE   8190U
 
+#define CLU_SIZE   (17 * 17 * 17)
+
 /* 
-
  * Device Access
  */
@@ -47,19 +49,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct 
v4l2_ctrl *ctrl)
struct vsp1_dl_body *dlb;
unsigned int i;
 
-   dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
+   dlb = vsp1_dl_body_get(clu->pool);
if (!dlb)
return -ENOMEM;
 
vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
-   for (i = 0; i < 17 * 17 * 17; ++i)
+   for (i = 0; i < CLU_SIZE; ++i)
vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
 
spin_lock_irq(>lock);
swap(clu->clu, dlb);
spin_unlock_irq(>lock);
 
-   vsp1_dl_body_free(dlb);
+   vsp1_dl_body_put(dlb);
return 0;
 }
 
@@ -220,8 +222,16 @@ static void clu_configure(struct vsp1_entity *entity,
}
 }
 
+static void clu_destroy(struct vsp1_entity *entity)
+{
+   struct vsp1_clu *clu = to_clu(>subdev);
+
+   vsp1_dl_body_pool_destroy(clu->pool);
+}
+
 static const struct vsp1_entity_operations clu_entity_ops = {
.configure = clu_configure,
+   .destroy = clu_destroy,
 };
 
 /* 
-
@@ -247,6 +257,17 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
if (ret < 0)
return ERR_PTR(ret);
 
+   /*
+* Pre-allocate a body pool, with 3 bodies allowing a userspace update
+* before the hardware has committed a previous set of tables, handling
+* both the queued and pending dl entries. One extra entry is added to
+* the CLU_SIZE to allow for the VI6_CLU_ADDR header.
+*/
+   clu->pool = vsp1_dl_body_pool_create(clu->entity.vsp1, 3, CLU_SIZE + 1,
+0);
+   if (!clu->pool)
+   return ERR_PTR(-ENOMEM);
+
/* Initialize the control handler. */
v4l2_ctrl_handler_init(>ctrls, 2);
v4l2_ctrl_new_custom(>ctrls, _table_control, NULL);
diff --git a/drivers/media/platform/vsp1/vsp1_clu.h 
b/drivers/media/platform/vsp1/vsp1_clu.h
index 036e0a2f1a42..fa3fe856725b 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.h
+++ b/drivers/media/platform/vsp1/vsp1_clu.h
@@ -36,6 +36,7 @@ struct vsp1_clu {
spinlock_t lock;
unsigned int mode;
struct vsp1_dl_body *clu;
+   struct vsp1_dl_body_pool *pool;
 };
 
 static inline struct vsp1_clu *to_clu(struct v4l2_subdev *subdev)
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 4e71792d183c..961c3fd2ff1c 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -111,7 +111,7 @@ struct vsp1_dl_list {
struct vsp1_dl_header *header;
dma_addr_t dma;
 
-   struct vsp1_dl_body body0;
+   struct vsp1_dl_body *body0;
struct list_head bodies;
 
bool has_chain;
@@ -135,8 +135,6 @@ enum vsp1_dl_mode {
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
  * @pool: body pool for the display list bodies
- * 

[PATCH v5 2/9] v4l: vsp1: Protect bodies against overflow

[Kieran Bingham]

The body write function relies on the code never asking it to write more
than the entries available in the list.

Currently with each list body containing 256 entries, this is fine, but
we can reduce this number greatly saving memory. In preparation of this
add a level of protection to catch any buffer overflows.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 

---

v3:
 - adapt for new 'body' terminology
 - simplify WARN_ON macro usage
---
 drivers/media/platform/vsp1/vsp1_dl.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 90e972a75c62..ecc3659a7884 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -50,6 +50,7 @@ struct vsp1_dl_entry {
  * @dma: DMA address of the entries
  * @size: size of the DMA memory in bytes
  * @num_entries: number of stored entries
+ * @max_entries: number of entries available
  */
 struct vsp1_dl_body {
struct list_head list;
@@ -60,6 +61,7 @@ struct vsp1_dl_body {
size_t size;
 
unsigned int num_entries;
+   unsigned int max_entries;
 };
 
 /**
@@ -139,6 +141,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1,
 
dlb->vsp1 = vsp1;
dlb->size = size;
+   dlb->max_entries = num_entries;
 
dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, >dma,
GFP_KERNEL);
@@ -220,6 +223,10 @@ void vsp1_dl_body_free(struct vsp1_dl_body *dlb)
  */
 void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
 {
+   if (WARN_ONCE(dlb->num_entries >= dlb->max_entries,
+ "DLB size exceeded (max %u)", dlb->max_entries))
+   return;
+
dlb->entries[dlb->num_entries].addr = reg;
dlb->entries[dlb->num_entries].data = data;
dlb->num_entries++;
-- 
git-series 0.9.1

[PATCH v5 5/9] v4l: vsp1: Use reference counting for bodies

[Kieran Bingham]

Extend the display list body with a reference count, allowing bodies to
be kept as long as a reference is maintained. This provides the ability
to keep a cached copy of bodies which will not change, so that they can
be re-applied to multiple display lists.

Signed-off-by: Kieran Bingham 

---
This could be squashed into the body update code, but it's not a
straightforward squash as the refcounts will affect both:
  v4l: vsp1: Provide a body pool
and
  v4l: vsp1: Convert display lists to use new body pool
therefore, I have kept this separate to prevent breaking bisectability
of the vsp-tests.

v3:
 - 's/fragment/body/'

v4:
 - Fix up reference handling comments.
---
 drivers/media/platform/vsp1/vsp1_clu.c |  7 ++-
 drivers/media/platform/vsp1/vsp1_dl.c  | 15 ++-
 drivers/media/platform/vsp1/vsp1_lut.c |  7 ++-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index a765d56c4118..1142d004e238 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -216,8 +216,13 @@ static void clu_configure(struct vsp1_entity *entity,
clu->clu = NULL;
spin_unlock_irqrestore(>lock, flags);
 
-   if (dlb)
+   if (dlb) {
vsp1_dl_list_add_body(dl, dlb);
+
+   /* release our local reference */
+   vsp1_dl_body_put(dlb);
+   }
+
break;
}
 }
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 961c3fd2ff1c..2784d3b48b02 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -58,6 +59,8 @@ struct vsp1_dl_body {
struct list_head list;
struct list_head free;
 
+   refcount_t refcnt;
+
struct vsp1_dl_body_pool *pool;
struct vsp1_device *vsp1;
 
@@ -259,6 +262,7 @@ struct vsp1_dl_body *vsp1_dl_body_get(struct 
vsp1_dl_body_pool *pool)
if (!list_empty(>free)) {
dlb = list_first_entry(>free, struct vsp1_dl_body, free);
list_del(>free);
+   refcount_set(>refcnt, 1);
}
 
spin_unlock_irqrestore(>lock, flags);
@@ -279,6 +283,9 @@ void vsp1_dl_body_put(struct vsp1_dl_body *dlb)
if (!dlb)
return;
 
+   if (!refcount_dec_and_test(>refcnt))
+   return;
+
dlb->num_entries = 0;
 
spin_lock_irqsave(>pool->lock, flags);
@@ -465,7 +472,11 @@ void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 reg, 
u32 data)
  * in the order in which bodies are added.
  *
  * Adding a body to a display list passes ownership of the body to the list. 
The
- * caller must not touch the body after this call.
+ * caller retains its reference to the fragment when adding it to the display
+ * list, but is not allowed to add new entries to the body.
+ *
+ * The reference must be explicitly released by a call to vsp1_dl_body_put()
+ * when the body isn't needed anymore.
  *
  * Additional bodies are only usable for display lists in header mode.
  * Attempting to add a body to a header-less display list will return an error.
@@ -477,6 +488,8 @@ int vsp1_dl_list_add_body(struct vsp1_dl_list *dl,
if (dl->dlm->mode != VSP1_DL_MODE_HEADER)
return -EINVAL;
 
+   refcount_inc(>refcnt);
+
list_add_tail(>list, >bodies);
return 0;
 }
diff --git a/drivers/media/platform/vsp1/vsp1_lut.c 
b/drivers/media/platform/vsp1/vsp1_lut.c
index a9cf874312c1..7643f18b1ea6 100644
--- a/drivers/media/platform/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/vsp1/vsp1_lut.c
@@ -172,8 +172,13 @@ static void lut_configure(struct vsp1_entity *entity,
lut->lut = NULL;
spin_unlock_irqrestore(>lock, flags);
 
-   if (dlb)
+   if (dlb) {
vsp1_dl_list_add_body(dl, dlb);
+
+   /* release our local reference */
+   vsp1_dl_body_put(dlb);
+   }
+
break;
}
 }
-- 
git-series 0.9.1

[PATCH v5 8/9] v4l: vsp1: Move video configuration to a cached dlb

[Kieran Bingham]

We are now able to configure a pipeline directly into a local display
list body. Take advantage of this fact, and create a cacheable body to
store the configuration of the pipeline in the video object.

vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
Convert this function to use the cached video->config body and obtain a
local display list reference.

Attach the video->config body to the display list when needed before
committing to hardware.

The pipe object is marked as un-configured when resuming from a suspend.
This ensures that when the hardware is reset - our cached configuration
will be re-attached to the next committed DL.

Signed-off-by: Kieran Bingham 
---

v3:
 - 's/fragment/body/', 's/fragments/bodies/'
 - video dlb cache allocation increased from 2 to 3 dlbs

Our video DL usage now looks like the below output:

dl->body0 contains our disposable runtime configuration. Max 41.
dl_child->body0 is our partition specific configuration. Max 12.
dl->bodies shows our constant configuration and LUTs.

  These two are LUT/CLU:
 * dl->bodies[x]->num_entries 256 / max 256
 * dl->bodies[x]->num_entries 4914 / max 4914

Which shows that our 'constant' configuration cache is currently
utilised to a maximum of 64 entries.

trace-cmd report | \
grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;

  dl->body0->num_entries 13 / max 128
  dl->body0->num_entries 14 / max 128
  dl->body0->num_entries 16 / max 128
  dl->body0->num_entries 20 / max 128
  dl->body0->num_entries 27 / max 128
  dl->body0->num_entries 34 / max 128
  dl->body0->num_entries 41 / max 128
  dl_child->body0->num_entries 10 / max 128
  dl_child->body0->num_entries 12 / max 128
  dl->bodies[x]->num_entries 15 / max 128
  dl->bodies[x]->num_entries 16 / max 128
  dl->bodies[x]->num_entries 17 / max 128
  dl->bodies[x]->num_entries 18 / max 128
  dl->bodies[x]->num_entries 20 / max 128
  dl->bodies[x]->num_entries 21 / max 128
  dl->bodies[x]->num_entries 256 / max 256
  dl->bodies[x]->num_entries 31 / max 128
  dl->bodies[x]->num_entries 32 / max 128
  dl->bodies[x]->num_entries 39 / max 128
  dl->bodies[x]->num_entries 40 / max 128
  dl->bodies[x]->num_entries 47 / max 128
  dl->bodies[x]->num_entries 48 / max 128
  dl->bodies[x]->num_entries 4914 / max 4914
  dl->bodies[x]->num_entries 55 / max 128
  dl->bodies[x]->num_entries 56 / max 128
  dl->bodies[x]->num_entries 63 / max 128
  dl->bodies[x]->num_entries 64 / max 128

v4:
 - Adjust pipe configured flag to be reset on resume rather than suspend
 - rename dl_child, dl_next
---
 drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
 drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
 drivers/media/platform/vsp1/vsp1_video.c | 67 -
 drivers/media/platform/vsp1/vsp1_video.h |  2 +-
 4 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 5012643583b6..fa445b1a2e38 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
   VI6_CMD_STRCMD);
pipe->state = VSP1_PIPELINE_RUNNING;
+   pipe->configured = true;
}
 
pipe->buffers_ready = 0;
@@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
continue;
 
spin_lock_irqsave(>irqlock, flags);
+   /*
+* The hardware may have been reset during a suspend and will
+* need a full reconfiguration
+*/
+   pipe->configured = false;
+
if (vsp1_pipeline_ready(pipe))
vsp1_pipeline_run(pipe);
spin_unlock_irqrestore(>irqlock, flags);
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h 
b/drivers/media/platform/vsp1/vsp1_pipe.h
index 90d29492b9b9..e7ad6211b4d0 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -90,6 +90,7 @@ struct vsp1_partition {
  * @irqlock: protects the pipeline state
  * @state: current state
  * @wq: wait queue to wait for state change completion
+ * @configured: flag determining if the hardware has run since reset
  * @frame_end: frame end interrupt handler
  * @lock: protects the pipeline use count and stream count
  * @kref: pipeline reference count
@@ -117,6 +118,7 @@ struct vsp1_pipeline {
spinlock_t irqlock;
enum vsp1_pipeline_state state;
wait_queue_head_t wq;
+   bool configured;
 
void (*frame_end)(struct vsp1_pipeline *pipe, bool completed);
 
@@ -143,8 +145,6 @@ struct vsp1_pipeline {
 */
struct list_head entities;
 
-   struct vsp1_dl_list *dl;
-
unsigned int partitions;
struct 

[PATCH v5 9/9] v4l: vsp1: Reduce display list body size

[Kieran Bingham]

The display list originally allocated a body of 256 entries to store all
of the register lists required for each frame.

This has now been separated into fragments for constant stream setup, and
runtime updates.

Empirical testing shows that the body0 now uses a maximum of 41
registers for each frame, for both DRM and Video API pipelines thus a
rounded 64 entries provides a suitable allocation.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_dl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 1fc52496fc13..ce315821b60c 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -21,7 +21,7 @@
 #include "vsp1.h"
 #include "vsp1_dl.h"
 
-#define VSP1_DL_NUM_ENTRIES256
+#define VSP1_DL_NUM_ENTRIES64
 
 #define VSP1_DLH_INT_ENABLE(1 << 1)
 #define VSP1_DLH_AUTO_START(1 << 0)
-- 
git-series 0.9.1

[PATCH v5 6/9] v4l: vsp1: Refactor display list configure operations

[Kieran Bingham]

The entities provide a single .configure operation which configures the
object into the target display list, based on the vsp1_entity_params
selection.

This restricts us to a single function prototype for both static
configuration (the pre-stream INIT stage) and the dynamic runtime stages
for both each frame - and each partition therein.

Split the configure function into two parts, '.prepare()' and
'.configure()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and
VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the
.configure(). The configuration for individual partitions is handled by
passing the partition number to the configure call, and processing any
runtime stage actions on the first partition only.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_bru.c|  12 +-
 drivers/media/platform/vsp1/vsp1_clu.c|  42 +--
 drivers/media/platform/vsp1/vsp1_dl.h |   1 +-
 drivers/media/platform/vsp1/vsp1_drm.c|  21 +--
 drivers/media/platform/vsp1/vsp1_entity.c |  15 +-
 drivers/media/platform/vsp1/vsp1_entity.h |  27 +--
 drivers/media/platform/vsp1/vsp1_hgo.c|  12 +-
 drivers/media/platform/vsp1/vsp1_hgt.c|  12 +-
 drivers/media/platform/vsp1/vsp1_hsit.c   |  12 +-
 drivers/media/platform/vsp1/vsp1_lif.c|  12 +-
 drivers/media/platform/vsp1/vsp1_lut.c|  24 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 162 ++---
 drivers/media/platform/vsp1/vsp1_sru.c|  12 +-
 drivers/media/platform/vsp1/vsp1_uds.c|  55 ++--
 drivers/media/platform/vsp1/vsp1_uif.c|  20 +--
 drivers/media/platform/vsp1/vsp1_video.c  |  24 +--
 drivers/media/platform/vsp1/vsp1_wpf.c| 297 ---
 17 files changed, 368 insertions(+), 392 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index e8fd2ae3b3eb..b9ff96f76b3e 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -285,19 +285,15 @@ static const struct v4l2_subdev_ops bru_ops = {
  * VSP1 Entity Operations
  */
 
-static void bru_configure(struct vsp1_entity *entity,
- struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl,
- enum vsp1_entity_params params)
+static void bru_prepare(struct vsp1_entity *entity,
+   struct vsp1_pipeline *pipe,
+   struct vsp1_dl_list *dl)
 {
struct vsp1_bru *bru = to_bru(>subdev);
struct v4l2_mbus_framefmt *format;
unsigned int flags;
unsigned int i;
 
-   if (params != VSP1_ENTITY_PARAMS_INIT)
-   return;
-
format = vsp1_entity_get_pad_format(>entity, bru->entity.config,
bru->entity.source_pad);
 
@@ -404,7 +400,7 @@ static void bru_configure(struct vsp1_entity *entity,
 }
 
 static const struct vsp1_entity_operations bru_entity_ops = {
-   .configure = bru_configure,
+   .prepare = bru_prepare,
 };
 
 /* 
-
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index 1142d004e238..006ad94bbe57 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -172,37 +172,36 @@ static const struct v4l2_subdev_ops clu_ops = {
 /* 
-
  * VSP1 Entity Operations
  */
+static void clu_prepare(struct vsp1_entity *entity,
+   struct vsp1_pipeline *pipe,
+   struct vsp1_dl_list *dl)
+{
+   struct vsp1_clu *clu = to_clu(>subdev);
+
+   /*
+* The yuv_mode can't be changed during streaming. Cache it internally
+* for future runtime configuration calls.
+*/
+   struct v4l2_mbus_framefmt *format;
+
+   format = vsp1_entity_get_pad_format(>entity,
+   clu->entity.config,
+   CLU_PAD_SINK);
+   clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
+}
 
 static void clu_configure(struct vsp1_entity *entity,
  struct vsp1_pipeline *pipe,
  struct vsp1_dl_list *dl,
- enum vsp1_entity_params params)
+ unsigned int partition)
 {
struct vsp1_clu *clu = to_clu(>subdev);
struct vsp1_dl_body *dlb;
unsigned long flags;
u32 ctrl = VI6_CLU_CTRL_AAI | VI6_CLU_CTRL_MVS | VI6_CLU_CTRL_EN;
 
-   switch (params) {
-   case VSP1_ENTITY_PARAMS_INIT: {
-   /*
-* The format can't be changed during streaming, only verify it
-* at setup time and store the information internally for future
-* runtime configuration calls.
-*/
-   

[PATCH v5 0/9] vsp1: TLB optimisation and DL caching

[Kieran Bingham]

Each display list currently allocates an area of DMA memory to store register
settings for the VSP1 to process. Each of these allocations adds pressure to
the IPMMU TLB entries.

We can reduce the pressure by pre-allocating larger areas and dividing the area
across multiple bodies represented as a pool.

With this reconfiguration of bodies, we can adapt the configuration code to
separate out constant hardware configuration and cache it for re-use.

--

The patches provided in this series can be found at:
  git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git  
tags/vsp1/tlb-optimise/v5

This series is temporarily based on the renesas-drivers-2018-01-09-v4.15-rc7 
release tag,
until changes for the DRM and UIF make it upstream.

Changelog:
--

v5:
 - Rebased on to renesas-drivers-2018-01-09-v4.15-rc7 to fix conflicts
   with DRM and UIF updates on VSP1 driver

v4:
 - Rebased to v4.14
 * v4l: vsp1: Use reference counting for bodies
   - Fix up reference handling comments

 * v4l: vsp1: Provide a body pool
   - Provide comment explaining extra allocation on body pool
 highlighting area for optimisation later.

 * v4l: vsp1: Refactor display list configure operations
   - Fix up comment to describe yuv_mode caching rather than format

 * vsp1: Adapt entities to configure into a body
   - Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0()

 * v4l: vsp1: Move video configuration to a cached dlb
   - Adjust pipe configured flag to be reset on resume rather than suspend
   - rename dl_child, dl_next

Testing:

The VSP unit tests have been run on this patch set with the following results:

- vsp-unit-test-.sh
Test Conditions:
  Platform  Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+
  Kernel release4.15.0-rc7-arm64-renesas-9-gfa58cc7e4788
  yavta /usr/bin/yavta
  convert   /usr/bin/convert
  compare   /usr/bin/compare
  killall   /usr/bin/killall
  raw2rgbpnm/usr/bin/raw2rgbpnm
- vsp-unit-test-0001.sh
Testing WPF packing in RGB332: pass
Testing WPF packing in ARGB555: pass
Testing WPF packing in XRGB555: pass
Testing WPF packing in RGB565: pass
Testing WPF packing in BGR24: pass
Testing WPF packing in RGB24: pass
Testing WPF packing in ABGR32: pass
Testing WPF packing in ARGB32: pass
Testing WPF packing in XBGR32: pass
Testing WPF packing in XRGB32: pass
- vsp-unit-test-0002.sh
Testing WPF packing in NV12M: pass
Testing WPF packing in NV16M: pass
Testing WPF packing in NV21M: pass
Testing WPF packing in NV61M: pass
Testing WPF packing in UYVY: pass
Testing WPF packing in VYUY: skip
Testing WPF packing in YUV420M: pass
Testing WPF packing in YUV422M: pass
Testing WPF packing in YUV444M: pass
Testing WPF packing in YVU420M: pass
Testing WPF packing in YVU422M: pass
Testing WPF packing in YVU444M: pass
Testing WPF packing in YUYV: pass
Testing WPF packing in YVYU: pass
- vsp-unit-test-0003.sh
Testing scaling from 640x640 to 640x480 in RGB24: pass
Testing scaling from 1024x768 to 640x480 in RGB24: pass
Testing scaling from 640x480 to 1024x768 in RGB24: pass
Testing scaling from 640x640 to 640x480 in YUV444M: pass
Testing scaling from 1024x768 to 640x480 in YUV444M: pass
Testing scaling from 640x480 to 1024x768 in YUV444M: pass
- vsp-unit-test-0004.sh
Testing histogram in RGB24: pass
Testing histogram in YUV444M: pass
- vsp-unit-test-0005.sh
Testing RPF.0: pass
Testing RPF.1: pass
Testing RPF.2: pass
Testing RPF.3: pass
Testing RPF.4: pass
- vsp-unit-test-0006.sh
Testing invalid pipeline with no RPF: pass
Testing invalid pipeline with no WPF: pass
- vsp-unit-test-0007.sh
Testing BRU in RGB24 with 1 inputs: pass
Testing BRU in RGB24 with 2 inputs: pass
Testing BRU in RGB24 with 3 inputs: pass
Testing BRU in RGB24 with 4 inputs: pass
Testing BRU in RGB24 with 5 inputs: pass
Testing BRU in YUV444M with 1 inputs: pass
Testing BRU in YUV444M with 2 inputs: pass
Testing BRU in YUV444M with 3 inputs: pass
Testing BRU in YUV444M with 4 inputs: pass
Testing BRU in YUV444M with 5 inputs: pass
- vsp-unit-test-0008.sh
Test requires unavailable feature set `bru rpf.0 uds wpf.0': skipped
- vsp-unit-test-0009.sh
Test requires unavailable feature set `rpf.0 wpf.0 wpf.1': skipped
- vsp-unit-test-0010.sh
Testing CLU in RGB24 with zero configuration: pass
Testing CLU in RGB24 with identity configuration: pass
Testing CLU in RGB24 with wave configuration: pass
Testing CLU in YUV444M with zero configuration: pass
Testing CLU in YUV444M with identity configuration: pass
Testing CLU in YUV444M with wave configuration: pass
Testing LUT in RGB24 with zero configuration: pass
Testing LUT in RGB24 with identity configuration: pass
Testing LUT in RGB24 with gamma configuration: pass
Testing LUT in YUV444M with zero configuration: pass
Testing LUT in YUV444M with identity configuration: pass
Testing LUT in YUV444M with gamma configuration: pass
- vsp-unit-test-0011.sh
Testing  hflip=0 vflip=0 rotate=0: pass
Testing  hflip=1 vflip=0 

[PATCH v5 3/9] v4l: vsp1: Provide a body pool

[Kieran Bingham]

Each display list allocates a body to store register values in a dma
accessible buffer from a dma_alloc_wc() allocation. Each of these
results in an entry in the TLB, and a large number of display list
allocations adds pressure to this resource.

Reduce TLB pressure on the IPMMUs by allocating multiple display list
bodies in a single allocation, and providing these to the display list
through a 'body pool'. A pool can be allocated by the display list
manager or entities which require their own body allocations.

Signed-off-by: Kieran Bingham 

---
v4:
 - Provide comment explaining extra allocation on body pool
   highlighting area for optimisation later.

v3:
 - s/fragment/body/, s/fragments/bodies/
 - qty -> num_bodies
 - indentation fix
 - s/vsp1_dl_body_pool_{alloc,free}/vsp1_dl_body_pool_{create,destroy}/'
 - Add kerneldoc to non-static functions

v2:
 - assign dlb->dma correctly
---
 drivers/media/platform/vsp1/vsp1_dl.c | 163 +++-
 drivers/media/platform/vsp1/vsp1_dl.h |   8 +-
 2 files changed, 171 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index ecc3659a7884..4e71792d183c 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -45,6 +45,8 @@ struct vsp1_dl_entry {
 /**
  * struct vsp1_dl_body - Display list body
  * @list: entry in the display list list of bodies
+ * @free: entry in the pool free body list
+ * @pool: pool to which this body belongs
  * @vsp1: the VSP1 device
  * @entries: array of entries
  * @dma: DMA address of the entries
@@ -54,6 +56,9 @@ struct vsp1_dl_entry {
  */
 struct vsp1_dl_body {
struct list_head list;
+   struct list_head free;
+
+   struct vsp1_dl_body_pool *pool;
struct vsp1_device *vsp1;
 
struct vsp1_dl_entry *entries;
@@ -65,6 +70,30 @@ struct vsp1_dl_body {
 };
 
 /**
+ * struct vsp1_dl_body_pool - display list body pool
+ * @dma: DMA address of the entries
+ * @size: size of the full DMA memory pool in bytes
+ * @mem: CPU memory pointer for the pool
+ * @bodies: Array of DLB structures for the pool
+ * @free: List of free DLB entries
+ * @lock: Protects the pool and free list
+ * @vsp1: the VSP1 device
+ */
+struct vsp1_dl_body_pool {
+   /* DMA allocation */
+   dma_addr_t dma;
+   size_t size;
+   void *mem;
+
+   /* Body management */
+   struct vsp1_dl_body *bodies;
+   struct list_head free;
+   spinlock_t lock;
+
+   struct vsp1_device *vsp1;
+};
+
+/**
  * struct vsp1_dl_list - Display list
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
@@ -105,6 +134,7 @@ enum vsp1_dl_mode {
  * @active: list currently being processed (loaded) by hardware
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
+ * @pool: body pool for the display list bodies
  * @gc_work: bodies garbage collector work struct
  * @gc_bodies: array of display list bodies waiting to be freed
  */
@@ -120,6 +150,8 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *queued;
struct vsp1_dl_list *pending;
 
+   struct vsp1_dl_body_pool *pool;
+
struct work_struct gc_work;
struct list_head gc_bodies;
 };
@@ -128,6 +160,137 @@ struct vsp1_dl_manager {
  * Display List Body Management
  */
 
+/**
+ * vsp1_dl_body_pool_create - Create a pool of bodies from a single allocation
+ * @vsp1: The VSP1 device
+ * @num_bodies: The quantity of bodies to allocate
+ * @num_entries: The maximum number of entries that the body can contain
+ * @extra_size: Extra allocation provided for the bodies
+ *
+ * Allocate a pool of display list bodies each with enough memory to contain 
the
+ * requested number of entries.
+ *
+ * Return a pointer to a pool on success or NULL if memory can't be allocated.
+ */
+struct vsp1_dl_body_pool *
+vsp1_dl_body_pool_create(struct vsp1_device *vsp1, unsigned int num_bodies,
+unsigned int num_entries, size_t extra_size)
+{
+   struct vsp1_dl_body_pool *pool;
+   size_t dlb_size;
+   unsigned int i;
+
+   pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+   if (!pool)
+   return NULL;
+
+   pool->vsp1 = vsp1;
+
+   /*
+* Todo: 'extra_size' is only used by vsp1_dlm_create(), to allocate
+* extra memory for the display list header. We need only one header per
+* display list, not per display list body, thus this allocation is
+* extraneous and should be reworked in the future.
+*/
+   dlb_size = num_entries * sizeof(struct vsp1_dl_entry) + extra_size;
+   pool->size = dlb_size * num_bodies;
+
+   pool->bodies = kcalloc(num_bodies, sizeof(*pool->bodies), GFP_KERNEL);
+   if (!pool->bodies) {
+   kfree(pool);
+   return NULL;
+   }
+
+   pool->mem = 

[PATCH v5 1/9] v4l: vsp1: Reword uses of 'fragment' as 'body'

[Kieran Bingham]

Throughout the codebase, the term 'fragment' is used to represent a
display list body. This term duplicates the 'body' which is already in
use.

The datasheet references these objects as a body, therefore replace all
mentions of a fragment with a body, along with the corresponding
pluralised terms.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_clu.c |  10 +-
 drivers/media/platform/vsp1/vsp1_dl.c  | 107 --
 drivers/media/platform/vsp1/vsp1_dl.h  |  14 +--
 drivers/media/platform/vsp1/vsp1_lut.c |   8 +-
 4 files changed, 69 insertions(+), 70 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index bc931a3ab498..246dd595c978 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -47,19 +47,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct 
v4l2_ctrl *ctrl)
struct vsp1_dl_body *dlb;
unsigned int i;
 
-   dlb = vsp1_dl_fragment_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
+   dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
if (!dlb)
return -ENOMEM;
 
-   vsp1_dl_fragment_write(dlb, VI6_CLU_ADDR, 0);
+   vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
for (i = 0; i < 17 * 17 * 17; ++i)
-   vsp1_dl_fragment_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
+   vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
 
spin_lock_irq(>lock);
swap(clu->clu, dlb);
spin_unlock_irq(>lock);
 
-   vsp1_dl_fragment_free(dlb);
+   vsp1_dl_body_free(dlb);
return 0;
 }
 
@@ -215,7 +215,7 @@ static void clu_configure(struct vsp1_entity *entity,
spin_unlock_irqrestore(>lock, flags);
 
if (dlb)
-   vsp1_dl_list_add_fragment(dl, dlb);
+   vsp1_dl_list_add_body(dl, dlb);
break;
}
 }
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 4257451f1bd8..90e972a75c62 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -69,7 +69,7 @@ struct vsp1_dl_body {
  * @header: display list header, NULL for headerless lists
  * @dma: DMA address for the header
  * @body0: first display list body
- * @fragments: list of extra display list bodies
+ * @bodies: list of extra display list bodies
  * @has_chain: if true, indicates that there's a partition chain
  * @chain: entry in the display list partition chain
  */
@@ -81,7 +81,7 @@ struct vsp1_dl_list {
dma_addr_t dma;
 
struct vsp1_dl_body body0;
-   struct list_head fragments;
+   struct list_head bodies;
 
bool has_chain;
struct list_head chain;
@@ -98,13 +98,13 @@ enum vsp1_dl_mode {
  * @mode: display list operation mode (header or headerless)
  * @singleshot: execute the display list in single-shot mode
  * @vsp1: the VSP1 device
- * @lock: protects the free, active, queued, pending and gc_fragments lists
+ * @lock: protects the free, active, queued, pending and gc_bodies lists
  * @free: array of all free display lists
  * @active: list currently being processed (loaded) by hardware
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
- * @gc_work: fragments garbage collector work struct
- * @gc_fragments: array of display list fragments waiting to be freed
+ * @gc_work: bodies garbage collector work struct
+ * @gc_bodies: array of display list bodies waiting to be freed
  */
 struct vsp1_dl_manager {
unsigned int index;
@@ -119,7 +119,7 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *pending;
 
struct work_struct gc_work;
-   struct list_head gc_fragments;
+   struct list_head gc_bodies;
 };
 
 /* 
-
@@ -157,17 +157,16 @@ static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 }
 
 /**
- * vsp1_dl_fragment_alloc - Allocate a display list fragment
+ * vsp1_dl_body_alloc - Allocate a display list body
  * @vsp1: The VSP1 device
- * @num_entries: The maximum number of entries that the fragment can contain
+ * @num_entries: The maximum number of entries that the body can contain
  *
- * Allocate a display list fragment with enough memory to contain the requested
+ * Allocate a display list body with enough memory to contain the requested
  * number of entries.
  *
- * Return a pointer to a fragment on success or NULL if memory can't be
- * allocated.
+ * Return a pointer to a body on success or NULL if memory can't be allocated.
  */
-struct vsp1_dl_body *vsp1_dl_fragment_alloc(struct vsp1_device *vsp1,
+struct vsp1_dl_body *vsp1_dl_body_alloc(struct vsp1_device *vsp1,
unsigned int num_entries)
 {

Re: [PATCH 0/2] PM / core: genpd fix and pm_runtime_force_suspend|resume() rework

[Rafael J. Wysocki]

On Mon, Jan 15, 2018 at 3:26 PM, Ulf Hansson  wrote:
> On 15 January 2018 at 14:22, Geert Uytterhoeven  wrote:

[cut]

>>
>> I did miss a small difference in topology: in pm/linux-next, H3 has DMA
>> enabled for SCIF2, while M3 hasn't (yet).
>> With DMA enabled on M3, it fails in the same way.
>>
>> As genpd_resume_noirq() no longer calls pm_runtime_force_resume(),
>> rcar_dmac_runtime_resume() is no longer called, and the DMAC's registers
>> are no longer reinitialized after system resume, breaking the serial port.
>
> In drivers/dma/sh/rcar-dmac.c, I would try to replace the below line:
> SET_SYSTEM_SLEEP_PM_OPS(rcar_dmac_sleep_suspend, rcar_dmac_sleep_resume)
>
> with:
> SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)

Yes, that probably is the least intrusive thing that can be done to
address the issue.

> in case that may be too early to suspend the dma device (which is
> rather common for dma devices) then try;
>
> SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, 
> pm_runtime_force_resume)

Good suggestion, and I would go straight for it anyway.

Geert, can you try if this works, please?

Thanks,
Rafael

Re: [PATCH v2] vsp1: fix video output on R8A77970

[Sergei Shtylyov]

Hello!

On 01/15/2018 03:51 PM, Laurent Pinchart wrote:


On Tuesday, 26 December 2017 23:14:12 EET Sergei Shtylyov wrote:

Laurent has added support for the VSP2-D found on R-Car V3M (R8A77970) but


I'm not sure there's a need to state my name in the commit message.


   You were the author of the patch this one has in the Fixes: tag, so I 
thought that was appropriate. I can remove that if you don't want you name 
mentioned...



the video  output that VSP2-D sends to DU has a greenish garbage-like line


Why does the text in your patches (commit message, comments, ...) sometime
have double spaces between words ?


   The text looks more pleasant (at least to me). Can remove them if you want...


repeated every 8 or so screen rows.


Is it every "8 or so" rows, or exactly every 8 rows ?


   You really want me to count the pixels?


It turns out that V3M has a teeny LIF register (at least it's documented!)
that you need to set to some kind of a  magic value for the LIF to work
correctly...

Based on the original (and large) patch by Daisuke Matsushita
.


What else is in the big patch ? Is it available somewhere ?


   Assorted changes gathered together only because they all bring the support 
for R8A77970. If you're really curious, here you are:


https://github.com/CogentEmbedded/meta-rcar/blob/v2.12.0/meta-rcar-gen3/recipes-kernel/linux/linux-renesas/0030-arm64-renesas-r8a7797-Add-Renesas-R8A7797-SoC-suppor.patch


Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
VSP2-D instances")
Signed-off-by: Sergei Shtylyov 

---
This patch is against the 'media_tree.git' repo's 'master' branch.

Changes in version 2:
- added a  comment before the V3M SoC check;
- fixed indetation in that check;
- reformatted  the patch description.

  drivers/media/platform/vsp1/vsp1_lif.c  |   12 
  drivers/media/platform/vsp1/vsp1_regs.h |5 +
  2 files changed, 17 insertions(+)

Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
===
--- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
+++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
@@ -155,6 +155,18 @@ static void lif_configure(struct vsp1_en
(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+   /*
+* R-Car V3M has the buffer attribute register you absolutely need
+* to write kinda magic value to  for the LIF to work correctly...
+*/


I'm not sure about the "kinda" magic value. 1536 is very likely a buffer size.


   Well, that's only guessing. The manual doesn't say anything about what the 
number is.



How about the following text ?

/*
 * On V3M the LBA register has to be set to a non-default value to


   I'd then spell its full name, LIF0 Buffer Attribute register.


 * guarantee proper operation (otherwise artifacts may appear on the
 * output). The value required by the datasheet is not documented but
 * is likely a buffer size or threshold.
 */

>

   OK,  can change that (modulo the name).


The commit message should also be updated to feel a bit less magic.


   I'll see about it.


+   if ((entity->vsp1->version &
+(VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
+   (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
+   vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+  VI6_LIF_LBA_LBA0 |
+  (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+   }
  }


The datasheet documents the register as being present on both V3M and M3-W
(and the test I've just run on H3 shows that the register is present there as
well). Should we program it on M3-W or leave it to the default value that
should be what is recommended by the datasheet for that SoC ?


   If the default value matches what's recommended by the manual, then I'd 
leave the register alone. But my task was R8A77970 support only anyway...



[..]

MBR, Sergei

Re: [PATCH 0/2] PM / core: genpd fix and pm_runtime_force_suspend|resume() rework

[Ulf Hansson]

On 15 January 2018 at 14:22, Geert Uytterhoeven  wrote:
> Hi Rafael,
>
> On Mon, Jan 15, 2018 at 9:16 AM, Geert Uytterhoeven
>  wrote:
>> On Mon, Jan 15, 2018 at 1:04 AM, Rafael J. Wysocki  wrote:
>>> On Sun, Jan 14, 2018 at 10:48 AM, Geert Uytterhoeven
>>>  wrote:
 On Sat, Jan 13, 2018 at 1:38 AM, Rafael J. Wysocki  
 wrote:
> On Friday, January 12, 2018 3:31:09 PM CET Geert Uytterhoeven wrote:
>> On Fri, Jan 12, 2018 at 2:00 PM, Rafael J. Wysocki  
>> wrote:
>> > This comes from the recent discussion/testing effort that ensued after 
>> > my
>> > pm_runtime_force_suspend|resume() changes proposal:
>> >
>> > https://marc.info/?t=15149777204=1=2
>> >
>> > Patch [1/2] basically is https://patchwork.kernel.org/patch/10152873/ 
>> > rebased
>> > on top of the current linux-next branch of the linux-pm.git tree (the 
>> > relevant
>> > part should be there in the linux-next tree proper ATM).  It applies 
>> > on top
>> > of https://patchwork.kernel.org/patch/10156077/ which should apply to 
>> > the Linus'
>> > tree cleanly.
>> >
>> > Patch [2/2] is a resend of 
>> > https://patchwork.kernel.org/patch/10142047/ with
>> > a very minor changelog modification and the R-b tag from Ulf.
>> >
>> > Geert, if possible, please test this on the Renesas systems that had 
>> > the
>> > problem with https://patchwork.kernel.org/patch/10142047/ previously 
>> > and let
>> > me know if you still see issues.
>>
>> I've tested this on two very similar systems: Salvator-XS with R-Car H3 
>> ES2.0,
>> and Salvator-X with R-Car M3-W ES1.0.
>>
>> On the M3-based system, everything seems to work fine.
>
> Good.
>
>> On the H3-based system, the serial console (the /dev/ttySC0 device, not 
>> kernel
>> serial output) is dead after resume from s2ram, with and without
>> no_console_suspend.
>>
>> With no_console_suspend, I see:
>>
>> ttySC ttySC0: 1 input overrun(s)
>>
>> after typing on the serial console, so it looks like an interrupt 
>> problem.
>>
>> This issue seems to be caused by patch [1/2]. But I have no idea what's
>> really happening, and why the two systems behave differently.

 Could be a firmware issue, too.
 While the kernel images are identical, the ARM trusted firmware configs 
 aren't
 (same version, though).

 I'll do some more investigation...
>>>
>>> OK, thanks!
>>>
>>> It also would be good to know the topology of the device hierarchy and
>>> how that maps to the domains on the failing system (and which UART
>>> clocks are operated by genpd).
>>
>> The topology is the same on both systems.
>
> I did miss a small difference in topology: in pm/linux-next, H3 has DMA
> enabled for SCIF2, while M3 hasn't (yet).
> With DMA enabled on M3, it fails in the same way.
>
> As genpd_resume_noirq() no longer calls pm_runtime_force_resume(),
> rcar_dmac_runtime_resume() is no longer called, and the DMAC's registers
> are no longer reinitialized after system resume, breaking the serial port.

In drivers/dma/sh/rcar-dmac.c, I would try to replace the below line:
SET_SYSTEM_SLEEP_PM_OPS(rcar_dmac_sleep_suspend, rcar_dmac_sleep_resume)

with:
SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)

in case that may be too early to suspend the dma device (which is
rather common for dma devices) then try;

SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)

Kind regards
Uffe

Re: [PATCH 0/2] PM / core: genpd fix and pm_runtime_force_suspend|resume() rework

[Geert Uytterhoeven]

Hi Rafael,

On Mon, Jan 15, 2018 at 9:16 AM, Geert Uytterhoeven
 wrote:
> On Mon, Jan 15, 2018 at 1:04 AM, Rafael J. Wysocki  wrote:
>> On Sun, Jan 14, 2018 at 10:48 AM, Geert Uytterhoeven
>>  wrote:
>>> On Sat, Jan 13, 2018 at 1:38 AM, Rafael J. Wysocki  
>>> wrote:
 On Friday, January 12, 2018 3:31:09 PM CET Geert Uytterhoeven wrote:
> On Fri, Jan 12, 2018 at 2:00 PM, Rafael J. Wysocki  
> wrote:
> > This comes from the recent discussion/testing effort that ensued after 
> > my
> > pm_runtime_force_suspend|resume() changes proposal:
> >
> > https://marc.info/?t=15149777204=1=2
> >
> > Patch [1/2] basically is https://patchwork.kernel.org/patch/10152873/ 
> > rebased
> > on top of the current linux-next branch of the linux-pm.git tree (the 
> > relevant
> > part should be there in the linux-next tree proper ATM).  It applies on 
> > top
> > of https://patchwork.kernel.org/patch/10156077/ which should apply to 
> > the Linus'
> > tree cleanly.
> >
> > Patch [2/2] is a resend of https://patchwork.kernel.org/patch/10142047/ 
> > with
> > a very minor changelog modification and the R-b tag from Ulf.
> >
> > Geert, if possible, please test this on the Renesas systems that had the
> > problem with https://patchwork.kernel.org/patch/10142047/ previously 
> > and let
> > me know if you still see issues.
>
> I've tested this on two very similar systems: Salvator-XS with R-Car H3 
> ES2.0,
> and Salvator-X with R-Car M3-W ES1.0.
>
> On the M3-based system, everything seems to work fine.

 Good.

> On the H3-based system, the serial console (the /dev/ttySC0 device, not 
> kernel
> serial output) is dead after resume from s2ram, with and without
> no_console_suspend.
>
> With no_console_suspend, I see:
>
> ttySC ttySC0: 1 input overrun(s)
>
> after typing on the serial console, so it looks like an interrupt problem.
>
> This issue seems to be caused by patch [1/2]. But I have no idea what's
> really happening, and why the two systems behave differently.
>>>
>>> Could be a firmware issue, too.
>>> While the kernel images are identical, the ARM trusted firmware configs 
>>> aren't
>>> (same version, though).
>>>
>>> I'll do some more investigation...
>>
>> OK, thanks!
>>
>> It also would be good to know the topology of the device hierarchy and
>> how that maps to the domains on the failing system (and which UART
>> clocks are operated by genpd).
>
> The topology is the same on both systems.

I did miss a small difference in topology: in pm/linux-next, H3 has DMA
enabled for SCIF2, while M3 hasn't (yet).
With DMA enabled on M3, it fails in the same way.

As genpd_resume_noirq() no longer calls pm_runtime_force_resume(),
rcar_dmac_runtime_resume() is no longer called, and the DMAC's registers
are no longer reinitialized after system resume, breaking the serial port.

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] vsp1: fix video output on R8A77970

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Tuesday, 26 December 2017 23:14:12 EET Sergei Shtylyov wrote:
> Laurent has added support for the VSP2-D found on R-Car V3M (R8A77970) but

I'm not sure there's a need to state my name in the commit message.

> the video  output that VSP2-D sends to DU has a greenish garbage-like line

Why does the text in your patches (commit message, comments, ...) sometime 
have double spaces between words ?

> repeated every 8 or so screen rows.

Is it every "8 or so" rows, or exactly every 8 rows ?

> It turns out that V3M has a teeny LIF register (at least it's documented!)
> that you need to set to some kind of a  magic value for the LIF to work
> correctly...
> 
> Based on the original (and large) patch by Daisuke Matsushita
> .

What else is in the big patch ? Is it available somewhere ?

> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
> VSP2-D instances")
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'media_tree.git' repo's 'master' branch.
> 
> Changes in version 2:
> - added a  comment before the V3M SoC check;
> - fixed indetation in that check;
> - reformatted  the patch description.
> 
>  drivers/media/platform/vsp1/vsp1_lif.c  |   12 
>  drivers/media/platform/vsp1/vsp1_regs.h |5 +
>  2 files changed, 17 insertions(+)
> 
> Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> ===
> --- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
> +++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> @@ -155,6 +155,18 @@ static void lif_configure(struct vsp1_en
>   (obth << VI6_LIF_CTRL_OBTH_SHIFT) |
>   (format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
>   VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
> +
> + /*
> +  * R-Car V3M has the buffer attribute register you absolutely need
> +  * to write kinda magic value to  for the LIF to work correctly...
> +  */

I'm not sure about the "kinda" magic value. 1536 is very likely a buffer size. 
How about the following text ?

/*
 * On V3M the LBA register has to be set to a non-default value to
 * guarantee proper operation (otherwise artifacts may appear on the
 * output). The value required by the datasheet is not documented but
 * is likely a buffer size or threshold.
 */

The commit message should also be updated to feel a bit less magic.

> + if ((entity->vsp1->version &
> +  (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
> + (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
> + vsp1_lif_write(lif, dl, VI6_LIF_LBA,
> +VI6_LIF_LBA_LBA0 |
> +(1536 << VI6_LIF_LBA_LBA1_SHIFT));
> + }
>  }

The datasheet documents the register as being present on both V3M and M3-W 
(and the test I've just run on H3 shows that the register is present there as 
well). Should we program it on M3-W or leave it to the default value that 
should be what is recommended by the datasheet for that SoC ?

>  static const struct vsp1_entity_operations lif_entity_ops = {
> Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
> ===
> --- media_tree.orig/drivers/media/platform/vsp1/vsp1_regs.h
> +++ media_tree/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -693,6 +693,11 @@
>  #define VI6_LIF_CSBTH_LBTH_MASK  (0x7ff << 0)
>  #define VI6_LIF_CSBTH_LBTH_SHIFT 0
> 
> +#define VI6_LIF_LBA  0x3b0c
> +#define VI6_LIF_LBA_LBA0 (1 << 31)
> +#define VI6_LIF_LBA_LBA1_MASK(0xfff << 16)
> +#define VI6_LIF_LBA_LBA1_SHIFT   16
> +
>  /* 
>   * Security Control Registers
>   */

-- 
Regards,

Laurent Pinchart

[PATCH] [LOCAL] arm64: renesas_defconfig: Use a default 128MB of CMA

[Kieran Bingham]

Our media tests frequently need large areas of CMA.
Define the default allocation to be 128 MBytes to support
larger use-cases.

Signed-off-by: Kieran Bingham 
Acked-by: Laurent Pinchart 
---

v1.1
 - Collected Laurent's Acked-by


 arch/arm64/configs/renesas_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/renesas_defconfig 
b/arch/arm64/configs/renesas_defconfig
index b1aeb5f366c5..d150d8c4b5df 100644
--- a/arch/arm64/configs/renesas_defconfig
+++ b/arch/arm64/configs/renesas_defconfig
@@ -87,6 +87,7 @@ CONFIG_CAN_RCAR_CANFD=y
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DMA_CMA=y
+CONFIG_CMA_SIZE_MBYTES=128
 CONFIG_CMA_ALIGNMENT=9
 CONFIG_MTD=y
 CONFIG_MTD_BLOCK=y
-- 
2.7.4

Re: [PATCH v2 06/12] ARM: dts: r8a7790: Convert to new LVDS DT bindings

[Sergei Shtylyov]

Hello!

On 01/13/2018 02:14 AM, Laurent Pinchart wrote:


The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 

[...]

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 62baabd757b6..0fcd5e34ef82 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi

[...]

@@ -1092,11 +1088,61 @@
port@1 {
reg = <1>;
du_out_lvds0: endpoint {
+   remote-endpoint = <_in>;
};
};
port@2 {
reg = <2>;
du_out_lvds1: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,lvds-r8a7790";


   Same question...


+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_in: endpoint {
+   remote-endpoint = <_out_lvds0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   lvds0_out: endpoint {
+   };
+   };
+   };
+   };
+
+   lvds1: lvds@feb94000 {
+   compatible = "renesas,lvds-r8a7790";


   Same question here as well...

MBR, Sergei

Re: [PATCH v2 07/12] ARM: dts: r8a7791: Convert to new LVDS DT bindings

[Sergei Shtylyov]

Hello!

On 01/13/2018 02:14 AM, Laurent Pinchart wrote:


The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 

[...]

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 67831d0405f3..b7fceb210de9 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi

[...]

@@ -1126,6 +1123,31 @@
port@1 {
reg = <1>;
du_out_lvds0: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,lvds-r8a7791";


  Haven't we settled on  going 1st, after the comma?

[...]

MBR, Sergei

Re: [PATCH 3/3] backlight: pwm_bl: don't use GPIOF_* with gpiod_get_direction

[Daniel Thompson]

On 14/01/18 21:07, Wolfram Sang wrote:

The documentation was wrong, gpiod_get_direction() returns 0/1 instead
of the GPIOF_* flags. The docs were fixed with commit 94fc73094abe47
("gpio: correct docs about return value of gpiod_get_direction"). Now,
fix this user (until a better, system-wide solution is in place).

Signed-off-by: Wolfram Sang 


Acked-by: Daniel Thompson 



---
Only build tested!

  drivers/video/backlight/pwm_bl.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1c2289ddd555a6..0fa7d2bd0e4811 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -301,14 +301,14 @@ static int pwm_backlight_probe(struct platform_device 
*pdev)
  
  	/*

 * If the GPIO is not known to be already configured as output, that
-* is, if gpiod_get_direction returns either GPIOF_DIR_IN or -EINVAL,
-* change the direction to output and set the GPIO as active.
+* is, if gpiod_get_direction returns either 1 or -EINVAL, change the
+* direction to output and set the GPIO as active.
 * Do not force the GPIO to active when it was already output as it
 * could cause backlight flickering or we would enable the backlight too
 * early. Leave the decision of the initial backlight state for later.
 */
if (pb->enable_gpio &&
-   gpiod_get_direction(pb->enable_gpio) != GPIOF_DIR_OUT)
+   gpiod_get_direction(pb->enable_gpio) != 0)
gpiod_direction_output(pb->enable_gpio, 1);
  
  	pb->power_supply = devm_regulator_get(>dev, "power");

Re: [PATCH 2/3] arm64: dts: renesas: eagle: Remove renesas, no-ether-link property

[Sergei Shtylyov]

Hello!

On 1/15/2018 11:41 AM, Simon Horman wrote:


Regarding Salvator-X and ULCB boards, Bogdan Mirea says:


   Intended to reply to the original patches but due to high load haven't 
ever finished that email...



 The present change is a bug fix for AVB link iteratively up/down.

 Steps to reproduce:
 - start AVB TX stream (Using aplay via MSE),
 - disconnect+reconnect the eth cable,
 - after a reconnection the eth connection goes iteratively up/down
   without user interaction,
 - this may heal after some seconds or even stay for minutes.

 As the documentation specifies, the "renesas,no-ether-link" option
 should be used when a board does not provide a proper AVB_LINK signal.
 There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
 and ULCB starter kits since the AVB_LINK is correctly handled by HW.

 Choosing to keep or remove the "renesas,no-ether-link" option will
 have impact on the code flow in the following ways:
 - keeping this option enabled may lead to unexpected behavior since
   the RX & TX are enabled/disabled directly from adjust_link function
   without any HW interrogation,


   What does he mean by that? The adjust_link() is surely called as a result 
of interrogating the PHY...



 - removing this option, the RX & TX will only be enabled/disabled after
   HW interrogation. The HW check is made through the LMON pin in PSR


   s/pin/bit/.


   register which specifies AVB_LINK signal value (0 - at low level;
   1 - at high level).


   Things are not that simple with this signal. The main cause of the issues 
is that it's connected to a LED control output of the Micrel PHY and that one 
may have different meaning -- e.g., it can well be blinking when some traffic 
happens (and IIRC that's might be a problem with KSZ9031, usually used with 
AVB). I remember I have tried without this option but finally decided to leave 
it specified as my schematics reading skills failed me in this case... I need 
to test this patch.



 In conclusion, the present change is also a safety improvement because
 it removes the "renesas,no-ether-link" option leading to a proper way
 of detecting the link state based on HW interrogation and not on
 software heuristic.

On examination of the documentation for the Eagle board this change seems
necessary as AVB_LINK is documented as being connected.

Fixes: 38525608952a ("arm64: dts: renesas: eagle: add EtherAVB support")
Cc: Bogdan Mirea 
Cc: Vladimir Zapolskiy 
Cc: Sergei Shtylyov 
Signed-off-by: Simon Horman 

[...]

MBR, Sergei

[PATCH 0/3] arm64: dts: v3msk, eagle, draak: Remove renesas,no-ether-link property

[Simon Horman]

Regarding Salvator-X and ULCB boards, Bogdan Mirea says:

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the change is also a safety improvement because it
removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

Bogdan Mirea also highlighted that this change may be relevant to the
DTS files for V3M Starter Kit, Draak and Eagle boards. Examination of the
relevant documentation has confirmed this.

Based on renesas-devel-20180115-v4.15-rc8

Simon Horman (3):
  arm64: dts: renesas: v3msk: Remove renesas, no-ether-link property
  arm64: dts: renesas: eagle: Remove renesas, no-ether-link property
  arm64: dts: renesas: draak: Remove renesas, no-ether-link property

 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 1 -
 arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts | 1 -
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 1 -
 3 files changed, 3 deletions(-)

-- 
2.11.0

[PATCH 3/3] arm64: dts: renesas: draak: Remove renesas, no-ether-link property

[Simon Horman]

Regarding Salvator-X and ULCB boards, Bogdan Mirea says:

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the present change is also a safety improvement because
it removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

On examination of the documentation for the Draak board this change seems
necessary as AVB0_LINK is documented as being connected.

Fixes: 4503b50eac08 ("arm64: dts: renesas: r8a77995: draak: enable EthernetAVB")
Cc: Bogdan Mirea 
Cc: Vladimir Zapolskiy 
Cc: Yoshihiro Shimoda 
Signed-off-by: Simon Horman 
---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 
b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 09de73b11db8..ee82ec26428a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -78,7 +78,6 @@
  {
pinctrl-0 = <_pins>;
pinctrl-names = "default";
-   renesas,no-ether-link;
phy-handle = <>;
status = "okay";
 
-- 
2.11.0

[PATCH 1/3] arm64: dts: renesas: v3msk: Remove renesas, no-ether-link property

[Simon Horman]

Regarding Salvator-X and ULCB boards, Bogdan Mirea says:

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the present change is also a safety improvement because
it removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

On examination of the documentation for the V3M Starter Kit (v3msk) this
change seems necessary as AVB0_LINK is documented as being connected.

Fixes: a6b1b7359074 ("arm64: dts: renesas: v3msk: add EtherAVB support")
Cc: Bogdan Mirea 
Cc: Vladimir Zapolskiy 
Cc: Sergei Shtylyov 
Signed-off-by: Simon Horman 
---
 arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts 
b/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
index 8624ca87d6b2..d166d61f7ab3 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77970-v3msk.dts
@@ -32,7 +32,6 @@
 };
 
  {
-   renesas,no-ether-link;
phy-handle = <>;
status = "okay";
 
-- 
2.11.0

[PATCH 2/3] arm64: dts: renesas: eagle: Remove renesas, no-ether-link property

[Simon Horman]

Regarding Salvator-X and ULCB boards, Bogdan Mirea says:

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the present change is also a safety improvement because
it removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

On examination of the documentation for the Eagle board this change seems
necessary as AVB_LINK is documented as being connected.

Fixes: 38525608952a ("arm64: dts: renesas: eagle: add EtherAVB support")
Cc: Bogdan Mirea 
Cc: Vladimir Zapolskiy 
Cc: Sergei Shtylyov 
Signed-off-by: Simon Horman 
---
 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts 
b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
index 8fe5c193e049..83210542db34 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
@@ -34,7 +34,6 @@
 };
 
  {
-   renesas,no-ether-link;
phy-handle = <>;
status = "okay";
 
-- 
2.11.0

Re: [PATCH v2 04/12] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Laurent Pinchart]

Hi Geert,

On Monday, 15 January 2018 10:30:23 EET Geert Uytterhoeven wrote:
> On Sat, Jan 13, 2018 at 12:14 AM, Laurent Pinchart wrote:
> > The LVDS encoders used to be described in DT as part of the DU. They now
> > have their own DT node, linked to the DU using the OF graph bindings.
> > This allows moving internal LVDS encoder support to a separate driver
> > modelled as a DRM bridge. Backward compatibility is retained as legacy
> > DT is patched live to move to the new bindings.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > 
> > +static void rcar_lvds_enable(struct drm_bridge *bridge)
> > +{
> > +   struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > +   u32 lvdhcr;
> > +   int ret;
> > +
> > +   WARN_ON(lvds->enabled);
> > +
> > +   ret = clk_prepare_enable(lvds->clock);
> 
> What about starting to use Runtime PM to manage the clock, and thus calling
> pm_runtime_get_sync() here instead?
> That will make the driver future-proof w.r.t. LVDS blocks in power domains.
> 
> You do need a "power-domains" property, though, which may complicate DT
> runtime patching.

And that's why I've left it out for now. It's on my to-do list, but I will do 
so in a separate patch series. Depending on the complexity I might use runtime 
PM for new DTs only and leave the runtime-patched devices trees out.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 04/12] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Jan 13, 2018 at 12:14 AM, Laurent Pinchart
 wrote:
> The LVDS encoders used to be described in DT as part of the DU. They now
> have their own DT node, linked to the DU using the OF graph bindings.
> This allows moving internal LVDS encoder support to a separate driver
> modelled as a DRM bridge. Backward compatibility is retained as legacy
> DT is patched live to move to the new bindings.
>
> Signed-off-by: Laurent Pinchart 

> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c

> +static void rcar_lvds_enable(struct drm_bridge *bridge)
> +{
> +   struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +   u32 lvdhcr;
> +   int ret;
> +
> +   WARN_ON(lvds->enabled);
> +
> +   ret = clk_prepare_enable(lvds->clock);

What about starting to use Runtime PM to manage the clock, and thus calling
pm_runtime_get_sync() here instead?
That will make the driver future-proof w.r.t. LVDS blocks in power domains.

You do need a "power-domains" property, though, which may complicate DT
runtime patching.

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 01/10] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Laurent Pinchart]

Hi Geert,

On Monday, 15 January 2018 10:05:59 EET Geert Uytterhoeven wrote:
> On Mon, Jan 15, 2018 at 7:59 AM, Laurent Pinchart wrote:
> > On Monday, 15 January 2018 08:55:29 EET Simon Horman wrote:
> >> On Fri, Jan 12, 2018 at 03:29:48PM +0200, Laurent Pinchart wrote:
> >>> On Friday, 12 January 2018 12:13:18 EET Geert Uytterhoeven wrote:
>  As this is a new binding, please use "renesas,-lvds".
> >>> 
> >>> I've recently been thinking that we made the wrong choice, -
> >>> would be better in my opinion as it aligns with -, but
> >>> it's too late to change that, so I'll change the order here.
> >> 
> >> My recollection is that in the beginning we had a bit of a mixture but
> >> leaned towards -, which made sense in my opinion. However, after
> >> some discussion it was agreed that the best-practice for upstream was to
> >> use -. Unless that situation has changed lets stock with using
> >> - for new bindings.
> > 
> > Sure, that was my plan, and it seems I failed to explain it clearly. I too
> > believe that - would be better, but as we have standardized on
> > -  and as there's no strong reason to reconsider that decision
> > at the moment, the next version of this patch will use -. It was
> > a mistake in v1, not an attempt to change what we had agreed on.
> 
> Note that I believe you have to consider the full tuple
> ",-" to see the light:  is more closely tied to
> , than  is.

I suppose there are pros and cons for both options :-) I see  more as a 
version qualifier.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/10] ARM: dts: porter: Fix HDMI output routing

[Laurent Pinchart]

Hi Simon,

On Monday, 15 January 2018 09:56:03 EET Simon Horman wrote:
> On Fri, Jan 12, 2018 at 02:58:53AM +0200, Laurent Pinchart wrote:
> > The HDMI encoder is connected to the RGB output of the DU, which is
> > port@0, not port@1. Fix the incorrect DT description.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Should this have the following tag?
> 
> Fixes: c5af8a4248d3 ("ARM: dts: porter: add DU DT support")

That would make sense, yes.

> If so, is it a fix to apply for v4.15 or a more regular patch
> to apply for v4.16?

I believe we can wait for v4.16. HDMI output works on Porter at the moment due 
to a separate bug in the DU driver that clones outputs by default when it 
shouldn't. There's however no harm in backporting the patch to stable series 
if desired, but it's not required.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] PM / core: genpd fix and pm_runtime_force_suspend|resume() rework

[Geert Uytterhoeven]

Hi Rafael,

On Mon, Jan 15, 2018 at 1:04 AM, Rafael J. Wysocki  wrote:
> On Sun, Jan 14, 2018 at 10:48 AM, Geert Uytterhoeven
>  wrote:
>> On Sat, Jan 13, 2018 at 1:38 AM, Rafael J. Wysocki  
>> wrote:
>>> On Friday, January 12, 2018 3:31:09 PM CET Geert Uytterhoeven wrote:
 On Fri, Jan 12, 2018 at 2:00 PM, Rafael J. Wysocki  
 wrote:
 > This comes from the recent discussion/testing effort that ensued after my
 > pm_runtime_force_suspend|resume() changes proposal:
 >
 > https://marc.info/?t=15149777204=1=2
 >
 > Patch [1/2] basically is https://patchwork.kernel.org/patch/10152873/ 
 > rebased
 > on top of the current linux-next branch of the linux-pm.git tree (the 
 > relevant
 > part should be there in the linux-next tree proper ATM).  It applies on 
 > top
 > of https://patchwork.kernel.org/patch/10156077/ which should apply to 
 > the Linus'
 > tree cleanly.
 >
 > Patch [2/2] is a resend of https://patchwork.kernel.org/patch/10142047/ 
 > with
 > a very minor changelog modification and the R-b tag from Ulf.
 >
 > Geert, if possible, please test this on the Renesas systems that had the
 > problem with https://patchwork.kernel.org/patch/10142047/ previously and 
 > let
 > me know if you still see issues.

 I've tested this on two very similar systems: Salvator-XS with R-Car H3 
 ES2.0,
 and Salvator-X with R-Car M3-W ES1.0.

 On the M3-based system, everything seems to work fine.
>>>
>>> Good.
>>>
 On the H3-based system, the serial console (the /dev/ttySC0 device, not 
 kernel
 serial output) is dead after resume from s2ram, with and without
 no_console_suspend.

 With no_console_suspend, I see:

 ttySC ttySC0: 1 input overrun(s)

 after typing on the serial console, so it looks like an interrupt problem.

 This issue seems to be caused by patch [1/2]. But I have no idea what's
 really happening, and why the two systems behave differently.
>>
>> Could be a firmware issue, too.
>> While the kernel images are identical, the ARM trusted firmware configs 
>> aren't
>> (same version, though).
>>
>> I'll do some more investigation...
>
> OK, thanks!
>
> It also would be good to know the topology of the device hierarchy and
> how that maps to the domains on the failing system (and which UART
> clocks are operated by genpd).

The topology is the same on both systems.
The UART's module clock is operated by genpd, on both systems.

>>> Well, that's not dramatic.
>>>
>>> Let's make a deal that we'll fix this on top of [1/2].
>>
>> ;-)
>>
>>> Which driver is this BTW?  sh-sci?  That one doesn't even support runtime
>>> PM, confusingly enough.
>>
>> Yes, sh-sci. It does make pm_runtime_*() calls.
>
> Hmm.  I overlooked that part.
>
> This is sort of unusual, because the driver doesn't provide any
> runtime PM callbacks, but still it does provided system suspend ones.
> It looks like the idea is to never put it into runtime suspend if any
> ports are enabled and always put it into runtime suspend otherwise.
>
> Which one is the case in your testing?  Is the port disabled or
> enabled during system-wide suspend?

It's enabled on both systems, as a getty is running.

>> And of course there's uart_ops.pm, which is driven from serial_core...
>
> What does this point to for that particular device?

sci_pm(), on both systems.

See, there's no difference in topology on both systems, so I'll have to look
a bit deeper first...

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] sh_eth: get Ether port # only when needed

[Geert Uytterhoeven]

On Sun, Jan 14, 2018 at 6:47 PM, Sergei Shtylyov
 wrote:
> The dual-port Ether configurations always have a shared TSU to e.g. pass
> the packets between those  ports.  With the  TSU init. code gathered under
> the single *if*, we now can only get the port # from 'platform_device::id'
> only when we actually  need it  (and not recalculate it each time)...
>
> Signed-off-by: Sergei Shtylyov 

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] sh_eth: gather all TSU init code in one place

[Geert Uytterhoeven]

On Sun, Jan 14, 2018 at 6:47 PM, Sergei Shtylyov
 wrote:
> The  sh_eth_cpu_data::chip_reset() method  always resets using ARSTR and
> this register is always located at the start of the  TSU register region.
> Therefore, we can  only call  this method if we know TSU is there and thus
> simplify  the probing code a  bit...
>
> Signed-off-by: Sergei Shtylyov 

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 01/10] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Jan 15, 2018 at 7:59 AM, Laurent Pinchart
 wrote:
> On Monday, 15 January 2018 08:55:29 EET Simon Horman wrote:
>> On Fri, Jan 12, 2018 at 03:29:48PM +0200, Laurent Pinchart wrote:
>> > On Friday, 12 January 2018 12:13:18 EET Geert Uytterhoeven wrote:
>> >> As this is a new binding, please use "renesas,-lvds".
>> >
>> > I've recently been thinking that we made the wrong choice, -
>> > would be better in my opinion as it aligns with -, but it's
>> > too late to change that, so I'll change the order here.
>>
>> My recollection is that in the beginning we had a bit of a mixture but
>> leaned towards -, which made sense in my opinion. However, after
>> some discussion it was agreed that the best-practice for upstream was to
>> use -. Unless that situation has changed lets stock with using
>> - for new bindings.
>
> Sure, that was my plan, and it seems I failed to explain it clearly. I too
> believe that - would be better, but as we have standardized on -
>  and as there's no strong reason to reconsider that decision at the
> moment, the next version of this patch will use -. It was a mistake
> in v1, not an attempt to change what we had agreed on.

Note that I believe you have to consider the full tuple ",-"
to see the light:  is more closely tied to , than  is.

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