Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Geert Uytterhoeven]

Hi Simon,

On Thu, Feb 22, 2018 at 4:54 PM, Simon Horman  wrote:
> On Wed, Feb 21, 2018 at 09:53:58PM +0300, Sergei Shtylyov wrote:
>> On 02/21/2018 09:23 PM, Simon Horman wrote:
>>
>> > ...
>> >
>>  +clocks = < CPG_MOD 812>;
>>  +power-domains = < 32>;
>>  +resets = < 812>;
>>  +phy-mode = "rgmii-txid";
>> >>>
>> >>>Why not just "rgmii"? TX delay is a board specific detail, no?
>> >>>
>> >> I admit I took this one straight from r8a7796 dtsi.
>> >> Would you like to me resend and change this?
>> >
>> >Yes, unless Simon would fix it while merging...
>> 
>>  Can I confirm the desired change is s/rgmii-txid/rgmii/ ?
>> >>>
>> >>>Yes.
>> >>
>> >>Apparently that means that this prop should be overridden in the board 
>> >> file
>> >> (which may not be an easy task given the board is Salvator-XS again).
>> >>
>> >> [...]
>> >
>> > Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts?
>>
>>In salvator-common.dtsi most probably -- it has the PHY data for Ether 
>> AVB.
>>
>> > I feel that I'm missing an important point here.
>>
>>Well, r8a779{5|6}.dtsi also have phy-mode = "rgmii-txid" (which was
>>unjustified in my current understanding). Thus such board override
>>wouldn't hurt them. But we lack a patch modifying salvator-common.dtsi
>>in htis series, so I'm now thinking a respin of this series is needed
>>anyway... sorry for being unclear. :-)
>
> While I've applied other patches in this series I have
> not applied this one - mainly to allow this discussion to conclude.
>
> Is an appropriate solution to do the following?
>
> 1) Atomically update
>a) r8a779{5|6}.dtsi to use "rgmii" and
>b) salvator-common.dtsi to use "rgmii-txid"
> 2) Update this patch to use "rgmii"

No need for atomics.
It can all be done by patches touching single files only:
  1) Add "rgmii-txid" to board .dts(i) files,
  2) Change s/rgmii-txid/rgmii/ in SoC .dtsi files.

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 v4 00/16] R-Car DU: Convert LVDS code to bridge driver

[Frank Rowand]

Hi Laurent,

On 02/22/18 02:25, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Thursday, 22 February 2018 08:07:14 EET Frank Rowand wrote:
>> On 02/20/18 15:10, Laurent Pinchart wrote:
>>> Hello,
>>>
>>> This patch series addresses a design mistake that dates back from the
>>> initial DU support. Support for the LVDS encoders, which are IP cores
>>> separate from the DU, was bundled in the DU driver. Worse, both the DU
>>> and LVDS were described through a single DT node.
>>>
>>> To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS
>>> encoders, and deprecate their description inside the DU bindings. To
>>> retain backward compatibility with existing DT, patches 03/16 to 08/16
>>> then patch the device tree at runtime to convert the legacy bindings to
>>> the new ones.
>>>
>>> With the DT side addressed, patch 09/16 converts the LVDS support code to
>>> a separate bridge driver. Patches 11/16 to 16/16 then update all the
>>> device tree sources to the new DU and LVDS encoders bindings.
>>>
>>> I decided to go for live DT patching in patch 08/16 because implementing
>>> support for both the legacy and new bindings in the driver would have been
>>> very intrusive, and prevented further cleanups. This version relies more
>>> heavily on overlays to avoid touching the internals of the OF core
>>> compared to v2, even if manual fixes to the device tree are still needed.
>>>
>>> Compared to v3, this series uses the OF changeset API to update properties
>>> instead of accessing the internals of the property structure. This removes
>>> the local implementation of functions to look up nodes by path and update
>>> properties. In order to do this, I pulled in Pantelis' patch series
>>> titled "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's
>>> request, and rebased it while taking two small review comments into
>>> account.
>>
>> Wait a minute!  Why are you putting a patch set to modify core devicetree
>> in the middle of a driver series.  Please pull it out to a separate series.
> 
> Because Rob asked for the driver-local implementation of the property add 
> function to be replaced by Pantelis' series. I want to get the LVDS changes 
> in 
> v4.17 and asked Rob whether I could then take the OF changeset patches merged 
> through the DRM tree, and he didn't object. If that causes an issue I'll 
> switch back to the driver-local implementation to get the driver changes 
> merged, split the OF changeset series out, and then move to the OF changeset 
> API once merged. Would you prefer that ?

You have already created a new version of the R-Car patches without the
set of patches that I was objecting to here.  So this is somewhat just
an academic comment.

As I mentioned in the v6 thread, I am coming back here to clean up loose
ends, and explain why I had the reaction I had.  Basically, this is a
process issue to me.

(1) The patch set from Pantelis is "hidden" in the driver patch series.
When viewing collapsed threads (which is my normal mode to avoid getting
overwhelmed by the vast volume of email I scan), the Pantelis patch set
is totally invisible.  If the R-Car driver patch series had not had me
on the to: list, there is a very good chance I would not have noticed
it.  Or noticed in a more delayed time frame.  And the same applies to
anyone else who might be subscribed to the devicetree mail list.  If
the Pantelis patch series was split out into a separate patch set then
it would be more visible on the list.

(2) There is no good way to indicate in the email subject lines for
the Pantelis patches that they are version 3 of the series, since
they are also version 4 of the R-Car patch series.  If one reads the
patch 0/0 header carefully, and/or the other Pantelis patch comment
headers carefully, and then does a little detective work, it is
possible to find version 2 of the Pantelis patch series, and thus
be able to track the full history.  If just glancing at each
individual patch email subject, scanning the patch comment, and
spending more time reading the body of the patch, it would be
very easy to overlook the existence of the previous versions on
the mail list.

(2b) Small quibble:  if the Pantelis patches were in a separate series,
with v3 in the subject header, then you would have normally put
a "changes since v2" section in the patch comment header, which would
have been more visible and less cryptic than what you wrote, which was:

   ...

   Signed-off-by: Pantelis Antoniou 
   [Fixed memory leak in __of_changeset_add_update_property_copy()]
   Signed-off-by: Laurent Pinchart 
   ---
drivers/of/dynamic.c | 222 ++
include/linux/of.h   | 328 
+++
2 files changed, 550 insertions(+)

My original scan of version 2 and then the email in this series had me
questioning whether the two open issues from patch 2 had been 

Re: [PATCH v6 3/4] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 02/22/18 14:10, Frank Rowand wrote:
> Hi Laurent, Rob,
> 
> Thanks for the prompt spin to address my concerns.  There are some small
> technical issues.
> 
> I did not read the v3 patch until today.  v3 through v6 are still using the
> old overlay apply method which uses an expanded device tree as input.
> 
> Rob, I don't see my overlay patches in you for-next branch, and I have
> not seen an "Applied" message from you.  What is the status of the
> overlay patches?
> 
> Comments in the patch below.
> 
> 
> On 02/22/18 05:13, 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.
>>
>> 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 v5:
>>
>> - Use a private copy of rcar_du_of_changeset_add_property()
>>
>> Changes since v3:
>>
>> - Use the OF changeset API
>> - Use of_graph_get_endpoint_by_regs()
>> - Replace hardcoded constants by sizeof()
>>
>> Changes since v2:
>>
>> - Update the SPDX headers to use C-style comments in header files
>> - Removed the manually created __local_fixups__ node
>> - Perform manual fixups on live DT instead of overlay
>>
>> 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   |   7 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_of.c   | 342 
>> +
>>  drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 ++
>>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 +
>>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 
>>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 
>>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  53 
>>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  53 
>>  9 files changed, 661 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_r8a7790.dts
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.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

< snip >

> 
> becomes:
> 
>   ret = of_overlay_fdt_apply(dtb->begin, _id);
> 
> If my overlay patches do not exist, there are other small errors
> in the code block above.  I'll ignore them for the moment.
> 
> A quick scan of the rest of the code looks OK.  I'll read through it
> more carefully, but wanted to get this reply out without further
> delay.

< snip >

I was hoping to be able to convert the .dts files to use sugar syntax
instead of hand coding the fragment nodes, but for this specific set
of files I failed, since the labels that would have been required do
not already exist in the base .dts files that that overlays would be
applied against.

Oh well.  It was an interesting exercise anyway, trying to be crafty.

-Frank

Re: [PATCH v6 3/4] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Frank,

On Friday, 23 February 2018 00:10:17 EET Frank Rowand wrote:
> Hi Laurent, Rob,
> 
> Thanks for the prompt spin to address my concerns.  There are some small
> technical issues.
> 
> I did not read the v3 patch until today.  v3 through v6 are still using the
> old overlay apply method which uses an expanded device tree as input.
> 
> Rob, I don't see my overlay patches in you for-next branch, and I have
> not seen an "Applied" message from you.  What is the status of the
> overlay patches?
> 
> Comments in the patch below.
> 
> On 02/22/18 05:13, 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.
> > 
> > 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 v5:
> > 
> > - Use a private copy of rcar_du_of_changeset_add_property()
> > 
> > Changes since v3:
> > 
> > - Use the OF changeset API
> > - Use of_graph_get_endpoint_by_regs()
> > - Replace hardcoded constants by sizeof()
> > 
> > Changes since v2:
> > 
> > - Update the SPDX headers to use C-style comments in header files
> > - Removed the manually created __local_fixups__ node
> > - Perform manual fixups on live DT instead of overlay
> > 
> > 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   |   7 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_of.c   | 342 
> >  drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 ++
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 +
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  53 
> >  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  53 
> >  9 files changed, 661 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_r8a7790.dts
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts

[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 ..ac442ddfed16
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c

[snip]

> > +static int __init rcar_du_of_apply_overlay(const struct
> > rcar_du_of_overlay *dtbs,
> > +  const char *compatible)
> > +{
> > +   const struct rcar_du_of_overlay *dtb = NULL;
> > +   struct device_node *node = NULL;
> > +   unsigned int i;
> > +   int ovcs_id;
> > +   void *data;
> > +   void *mem;
> > +   int ret;
> > +
> > +   for (i = 0; dtbs[i].compatible; ++i) {
> > +   if (!strcmp(dtbs[i].compatible, compatible)) {
> > +   dtb = [i];
> > +   break;
> > +   }
> > +   }
> > +
> > +   if (!dtb)
> > +   return -ENODEV;
> 
> __If__ my overlay patches are accepted, this block:
> 
> > +
> > +   data = kmemdup(dtb->begin, dtb->end - dtb->begin, GFP_KERNEL);
> > +   if (!data)
> > +   return -ENOMEM;
> > +
> > +   mem = of_fdt_unflatten_tree(data, NULL, );
> > +   if (!mem) {
> > +   ret = -ENOMEM;
> > +   goto done;
> > +   }
> > +
> > +   ovcs_id = 0;
> > +   ret = of_overlay_apply(node, _id);
> > +
> > +done:
> > +   of_node_put(node);
> > +   kfree(data);
> > +   kfree(mem);
> 
> becomes:
> 
>   ret = of_overlay_fdt_apply(dtb->begin, _id);

I tried to rework this patch in a way that would make switching to FDT 
overlays easy, and I'm glad to hear I haven't done a too bad job :-)

Are your patches scheduled for merge in v4.17 ? If so, is it possible to get 
apply them in a stable branch on top of v4.16-rc1 that can be merged as a 
dependency for this series ? There are changes to the Renesas DT queued for 
merge 

Re: [PATCH v6 3/4] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

Hi Laurent, Rob,

Thanks for the prompt spin to address my concerns.  There are some small
technical issues.

I did not read the v3 patch until today.  v3 through v6 are still using the
old overlay apply method which uses an expanded device tree as input.

Rob, I don't see my overlay patches in you for-next branch, and I have
not seen an "Applied" message from you.  What is the status of the
overlay patches?

Comments in the patch below.


On 02/22/18 05:13, 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.
> 
> 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 v5:
> 
> - Use a private copy of rcar_du_of_changeset_add_property()
> 
> Changes since v3:
> 
> - Use the OF changeset API
> - Use of_graph_get_endpoint_by_regs()
> - Replace hardcoded constants by sizeof()
> 
> Changes since v2:
> 
> - Update the SPDX headers to use C-style comments in header files
> - Removed the manually created __local_fixups__ node
> - Perform manual fixups on live DT instead of overlay
> 
> 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   |   7 +-
>  drivers/gpu/drm/rcar-du/rcar_du_of.c   | 342 
> +
>  drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 ++
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 +
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  53 
>  .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  53 
>  9 files changed, 661 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_r8a7790.dts
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.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
>   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..86b337b4be5d 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -8,7 +8,12 @@ rcar-du-drm-y := rcar_du_crtc.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.o \
> +rcar_du_of_lvds_r8a7790.dtb.o \
> +rcar_du_of_lvds_r8a7791.dtb.o \
> +rcar_du_of_lvds_r8a7793.dtb.o \
> +rcar_du_of_lvds_r8a7795.dtb.o \
> +rcar_du_of_lvds_r8a7796.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 ..ac442ddfed16
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -0,0 +1,342 @@
> +// 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 
> 

[PATCH v2] v4l: vsp1: Print the correct blending unit name in debug messages

[Laurent Pinchart]

The DRM pipelines can use either the BRU or the BRS for blending. Make
sure the right name is used in debugging messages to avoid confusion.

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

- Create a macro to get the right entity name instead of duplicating the
  same code all over the driver
---
 drivers/media/platform/vsp1/vsp1_drm.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index ac85942162c1..b8fee1834253 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -27,6 +27,7 @@
 #include "vsp1_pipe.h"
 #include "vsp1_rwpf.h"
 
+#define BRU_NAME(e)(e)->type == VSP1_ENTITY_BRU ? "BRU" : "BRS"
 
 /* 
-
  * Interrupt Handling
@@ -88,7 +89,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
struct vsp1_entity *next;
struct vsp1_dl_list *dl;
struct v4l2_subdev_format format;
-   const char *bru_name;
unsigned long flags;
unsigned int i;
int ret;
@@ -99,7 +99,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
drm_pipe = >drm->pipe[pipe_index];
pipe = _pipe->pipe;
bru = to_bru(>bru->subdev);
-   bru_name = pipe->bru->type == VSP1_ENTITY_BRU ? "BRU" : "BRS";
 
if (!cfg) {
/*
@@ -165,7 +164,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
 
dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
__func__, format.format.width, format.format.height,
-   format.format.code, bru_name, i);
+   format.format.code, BRU_NAME(pipe->bru), i);
}
 
format.pad = pipe->bru->source_pad;
@@ -181,7 +180,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
 
dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
__func__, format.format.width, format.format.height,
-   format.format.code, bru_name, i);
+   format.format.code, BRU_NAME(pipe->bru), i);
 
format.pad = RWPF_PAD_SINK;
ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
@@ -473,9 +472,9 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device *vsp1,
if (ret < 0)
return ret;
 
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on BRU pad %u\n",
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
__func__, format.format.width, format.format.height,
-   format.format.code, format.pad);
+   format.format.code, BRU_NAME(pipe->bru), format.pad);
 
sel.pad = bru_input;
sel.target = V4L2_SEL_TGT_COMPOSE;
@@ -486,10 +485,9 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device *vsp1,
if (ret < 0)
return ret;
 
-   dev_dbg(vsp1->dev,
-   "%s: set selection (%u,%u)/%ux%u on BRU pad %u\n",
+   dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",
__func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
-   sel.pad);
+   BRU_NAME(pipe->bru), sel.pad);
 
return 0;
 }
@@ -514,12 +512,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
struct vsp1_entity *entity;
struct vsp1_entity *next;
struct vsp1_dl_list *dl;
-   const char *bru_name;
unsigned int i;
int ret;
 
-   bru_name = pipe->bru->type == VSP1_ENTITY_BRU ? "BRU" : "BRS";
-
/* Prepare the display list. */
dl = vsp1_dl_list_get(pipe->output->dlm);
 
@@ -570,7 +565,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
rpf->entity.sink_pad = i;
 
dev_dbg(vsp1->dev, "%s: connecting RPF.%u to %s:%u\n",
-   __func__, rpf->entity.index, bru_name, i);
+   __func__, rpf->entity.index, BRU_NAME(pipe->bru), i);
 
ret = vsp1_du_setup_rpf_pipe(vsp1, pipe, rpf, i);
if (ret < 0)
-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 0/4] R-Car DU: Convert LVDS code to bridge driver

[Laurent Pinchart]

Hi Frank,

On Thursday, 22 February 2018 22:23:20 EET Frank Rowand wrote:
> On 02/22/18 05:13, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series addresses a design mistake that dates back from the
> > initial DU support. Support for the LVDS encoders, which are IP cores
> > separate from the DU, was bundled in the DU driver. Worse, both the DU
> > and LVDS were described through a single DT node.
> > 
> > To fix the, patches 1/4 and 2/4 define new DT bindings for the LVDS
> > encoders, and deprecate their description inside the DU bindings. To
> > retain backward compatibility with existing DT, patch 3/4 then patch the
> > device tree at runtime to convert the legacy bindings to the new ones.
> > 
> > With the DT side addressed, patch 4/4 converts the LVDS support code to a
> > separate bridge driver.
> > 
> > I decided to go for live DT patching in patch 3/4 because implementing
> > support for both the legacy and new bindings in the driver would have been
> > very intrusive, and prevented further cleanups. This version relies more
> > heavily on overlays to avoid touching the internals of the OF core
> > compared to v2, even if manual fixes to the device tree are still needed.
> > 
> > As all the patches but the last one have been acked, I plan to send a pull
> > request by the end of the week if no new issue is discovered.
> > 
> > Compared to v5, I've dropped the OF changeset halpers series as Frank
> > raised concerns about hidding it in the middle of a driver patch series.
> > I've thus copied the implementation of of_changeset_add_property_copy()
> > in the driver to avoid blocking this series. The function arguments are
> > identical, so when the OF changeset helpers will land it will be very
> > easy to drop the private copy and use the
> > of_changeset_add_property_copy() helper.
> 
> Thank you Laurent.
> 
> My issues with that are procedural, and I'll reply later about this in the
> v4 patch thread, where I raised the issue.  (For the peanut gallery, I
> replied in thread v4 _after_ Laurent sent v5, so Laurent did not ignore
> me in v5.)

I would have waited for your ack anyway :-)

> My technical comments are more relevent than my process comments, in terms
> of helping Laurent get his driver submitted, so I will delay the process
> comments.

Thank you. 

> My technical comments will be in reply to patch 3/4.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 0/4] R-Car DU: Convert LVDS code to bridge driver

[Frank Rowand]

On 02/22/18 05:13, Laurent Pinchart wrote:
> Hello,
> 
> This patch series addresses a design mistake that dates back from the initial
> DU support. Support for the LVDS encoders, which are IP cores separate from
> the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
> described through a single DT node.
> 
> To fix the, patches 1/4 and 2/4 define new DT bindings for the LVDS
> encoders, and deprecate their description inside the DU bindings. To retain
> backward compatibility with existing DT, patch 3/4 then patch the device tree
> at runtime to convert the legacy bindings to the new ones.
> 
> With the DT side addressed, patch 4/4 converts the LVDS support code to a
> separate bridge driver.
> 
> I decided to go for live DT patching in patch 3/4 because implementing
> support for both the legacy and new bindings in the driver would have been
> very intrusive, and prevented further cleanups. This version relies more
> heavily on overlays to avoid touching the internals of the OF core compared to
> v2, even if manual fixes to the device tree are still needed.
> 
> As all the patches but the last one have been acked, I plan to send a pull
> request by the end of the week if no new issue is discovered.
> 
> Compared to v5, I've dropped the OF changeset halpers series as Frank raised
> concerns about hidding it in the middle of a driver patch series. I've thus
> copied the implementation of of_changeset_add_property_copy() in the driver to
> avoid blocking this series. The function arguments are identical, so when the
> OF changeset helpers will land it will be very easy to drop the private copy
> and use the of_changeset_add_property_copy() helper.

Thank you Laurent.

My issues with that are procedural, and I'll reply later about this in the
v4 patch thread, where I raised the issue.  (For the peanut gallery, I
replied in thread v4 _after_ Laurent sent v5, so Laurent did not ignore
me in v5.)

My technical comments are more relevent than my process comments, in terms
of helping Laurent get his driver submitted, so I will delay the process
comments.

My technical comments will be in reply to patch 3/4.

-Frank

[PATCH 1/6] arm64: dts: renesas: r8a77965: Add "reg" properties

[Jacopo Mondi]

Add "reg" properties to place-holder nodes with unit address defined for
R-Car M3-N SoC.

This silences the following DTC compiler warning:
Warning (unit_address_vs_reg): Node /soc/... has a unit name,
but no reg property

Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 51 +++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 55f05f7..4d9bba9 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -317,6 +317,7 @@
};
 
intc_ex: interrupt-controller@e61c {
+   reg = <0 0xe61c 0 0x200>;
/* placeholder */
};
 
@@ -520,130 +521,163 @@
};
 
avb: ethernet@e680 {
+   reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
/* placeholder */
};
 
csi20: csi2@fea8 {
+   reg = <0 0xfea8 0 0x1>;
/* placeholder */
};
 
csi40: csi2@feaa {
+   reg = <0 0xfeaa 0 0x1>;
/* placeholder */
};
 
vin0: video@e6ef {
+   reg = <0 0xe6ef 0 0x1000>;
/* placeholder */
};
 
vin1: video@e6ef1000 {
+   reg = <0 0xe6ef1000 0 0x1000>;
/* placeholder */
};
 
vin2: video@e6ef2000 {
+   reg = <0 0xe6ef2000 0 0x1000>;
/* placeholder */
};
 
vin3: video@e6ef3000 {
+   reg = <0 0xe6ef3000 0 0x1000>;
/* placeholder */
};
 
vin4: video@e6ef4000 {
+   reg = <0 0xe6ef4000 0 0x1000>;
/* placeholder */
};
 
vin5: video@e6ef5000 {
+   reg = <0 0xe6ef5000 0 0x1000>;
/* placeholder */
};
 
vin6: video@e6ef6000 {
+   reg = <0 0xe6ef6000 0 0x1000>;
/* placeholder */
};
 
vin7: video@e6ef7000 {
+   reg = <0 0xe6ef7000 0 0x1000>;
/* placeholder */
};
 
ohci0: usb@ee08 {
+   reg = <0 0xee08 0 0x100>;
/* placeholder */
};
 
ehci0: usb@ee080100 {
+   reg = <0 0xee080100 0 0x100>;
/* placeholder */
};
 
usb2_phy0: usb-phy@ee080200 {
+   reg = <0 0xee080200 0 0x700>;
/* placeholder */
};
 
ohci1: usb@ee0a {
+   reg = <0 0xee0a 0 0x100>;
/* placeholder */
};
 
ehci1: usb@ee0a0100 {
+   reg = <0 0xee0a0100 0 0x100>;
/* placeholder */
};
 
i2c0: i2c@e650 {
+   reg = <0 0xe650 0 0x40>;
/* placeholder */
};
 
i2c1: i2c@e6508000 {
+   reg = <0 0xe6508000 0 0x40>;
/* placeholder */
};
 
i2c2: i2c@e651 {
+   reg = <0 0xe651 0 0x40>;
/* placeholder */
};
 
i2c3: i2c@e66d {
+   reg = <0 0xe66d 0 0x40>;
/* placeholder */
};
 
i2c4: i2c@e66d8000 {
+   reg = <0 0xe66d8000 0 0x40>;
/* placeholder */
};
 
i2c5: i2c@e66e {
+   reg = <0 0xe66e 0 0x40>;
/* placeholder */
};
 
i2c6: i2c@e66e8000 {
+   reg = <0 0xe66e8000 0 0x40>;
/* placeholder */
};
 
i2c_dvfs: i2c@e60b {
+   reg = <0 0xe60b 0 0x425>;
/* placeholder */
};
 
pwm0: pwm@e6e3 {
+   reg = <0 0xe6e3 0 8>;
/* placeholder */
};
 
pwm1: pwm@e6e31000 {
+   reg = <0 0xe6e31000 0 8>;
/* placeholder */
};
 
pwm2: pwm@e6e32000 {
+ 

[PATCH 2/6] arm64: dts: renesas: r8a77965: Add #address-cells and #size-cells

[Jacopo Mondi]

Add "#address-cells" and "#size-cells" properties to all place-holder nodes
that have children nodes defined by salvator-x[s].dtsi device tree.

This silences the following DTC compiler warnings:
Warning (reg_format): "reg" property in /soc/.. has invalid length (4 bytes)
(#address-cells == 2, #size-cells == 1)
Warning (avoid_default_addr_size): Relying on default #address-cells
value for /soc/...
Warning (avoid_default_addr_size): Relying on default #size-cells value
for /soc/...

Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 25 +
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 4d9bba9..ab46cd2 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -521,6 +521,9 @@
};
 
avb: ethernet@e680 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>;
/* placeholder */
};
@@ -528,11 +531,21 @@
csi20: csi2@fea8 {
reg = <0 0xfea8 0 0x1>;
/* placeholder */
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
};
 
csi40: csi2@feaa {
reg = <0 0xfeaa 0 0x1>;
/* placeholder */
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
};
 
vin0: video@e6ef {
@@ -611,6 +624,9 @@
};
 
i2c2: i2c@e651 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
reg = <0 0xe651 0 0x40>;
/* placeholder */
};
@@ -621,6 +637,9 @@
};
 
i2c4: i2c@e66d8000 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
reg = <0 0xe66d8000 0 0x40>;
/* placeholder */
};
@@ -636,6 +655,9 @@
};
 
i2c_dvfs: i2c@e60b {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
reg = <0 0xe60b 0 0x425>;
/* placeholder */
};
@@ -681,6 +703,9 @@
/* placeholder */
 
ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
port@0 {
reg = <0>;
du_out_rgb: endpoint {
-- 
2.7.4

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Sergei Shtylyov]

Hello!

On 02/22/2018 06:54 PM, Simon Horman wrote:

>>> ...
>>>
>> +clocks = < CPG_MOD 812>;
>> +power-domains = < 32>;
>> +resets = < 812>;
>> +phy-mode = "rgmii-txid";
>
>Why not just "rgmii"? TX delay is a board specific detail, no?
>
 I admit I took this one straight from r8a7796 dtsi.
 Would you like to me resend and change this?
>>>
>>>Yes, unless Simon would fix it while merging...
>>
>> Can I confirm the desired change is s/rgmii-txid/rgmii/ ?
>
>Yes.

Apparently that means that this prop should be overridden in the board 
 file
 (which may not be an easy task given the board is Salvator-XS again).

 [...]
>>>
>>> Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts?
>>
>>In salvator-common.dtsi most probably -- it has the PHY data for Ether 
>> AVB.
>>
>>> I feel that I'm missing an important point here.
>>
>>Well, r8a779{5|6}.dtsi also have phy-mode = "rgmii-txid" (which was
>>unjustified in my current understanding). Thus such board override
>>wouldn't hurt them. But we lack a patch modifying salvator-common.dtsi
>>in htis series, so I'm now thinking a respin of this series is needed
>>anyway... sorry for being unclear. :-)
> 
> While I've applied other patches in this series I have
> not applied this one - mainly to allow this discussion to conclude.
> 
> Is an appropriate solution to do the following?
> 
> 1) Atomically update
>a) r8a779{5|6}.dtsi to use "rgmii" and
>b) salvator-common.dtsi to use "rgmii-txid"
> 2) Update this patch to use "rgmii"

   Yeah, I was thinking about doing exactly this...

> Sorry if I'm still missing the point.

   I was referring to what the 2nd paragraph of [1] said about the internal 
delays
(perhaps I just misunderstood it)...

[1] 
https://en.wikipedia.org/wiki/Media-independent_interface#Reduced_gigabit_media-independent_interface

MBR, Sergei

[PATCH 0/6] R-Car M3-N DTS fixes

[Jacopo Mondi]

Hello Simon, Arnd,

The recently introduced Renesas R-Car M3-N SoC device tree source file is
included from salvator-x[s]-$SOC.dts, and it thus needs place holders for
devices not yet enabled but whose nodes are referenced by those common files.

When first submitted r8a77965.dtsi I ignored DTC compiler warnings as I
(wrongly) assumed it was fine to have place-holders with missing properties or
labels.

As DTC warnings have to be silenced in order to have M3-N device tree sources
accepted, this series address all warnings and make DTC happy even with
place holders.

Simon: up to you if you like to squash this on top of my previous series when
picking that one up. To ease your job I have based this series on all patches
I sent in v2 but the still debated ether-avb one. I thus left the avb node
unpopulated and added just the required properties to silence DTC warnings
to it in this series.

Development branch based on: (renesas-drivers-2018-02-13-v4.16-rc1 +
M3-N v2 patches - ether-avb) where to apply this series on is available at:
git://jmondi.org/linux m3-n/renesas-drivers-2018-02-13-v4.16-rc1/v2-simon

Thanks
  j

Jacopo Mondi (6):
  arm64: dts: renesas: r8a77965: Add "reg" properties
  arm64: dts: renesas: r8a77965: Add #address-cells and #size-cells
  arm64: dts: renesas: r8a77965: Remove stale reg property
  arm64: dts: renesas: r8a77965: Add #phy-cells property
  arm64: dts: renesas: r8a77965: Add #pwm-cells property
  arm64: dts: renesas: r8a77965: Add #interrupt-cells property

 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 77 ++-
 1 file changed, 76 insertions(+), 1 deletion(-)

--
2.7.4

[PATCH 3/6] arm64: dts: renesas: r8a77965: Remove stale reg property

[Jacopo Mondi]

Remove "reg" property from cache-controller-0 device node as it does not
have any unit address.

This silences the following DTC compiler warning:
Warning (unit_address_vs_reg): Node /cpus/cache-controller-0 has a reg
or ranges property, but no unit name

Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index ab46cd2..be4f25c 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -47,7 +47,6 @@
 
L2_CA57: cache-controller-0 {
compatible = "cache";
-   reg = <0>;
power-domains = < 12>;
cache-unified;
cache-level = <2>;
-- 
2.7.4

[PATCH 4/6] arm64: dts: renesas: r8a77965: Add #phy-cells property

[Jacopo Mondi]

Add "#phy-cells" property to "usb-phy@e65ee000" device node.

This silences the following DTC compiler warning:
Warning (phys_property): Missing property '#phy-cells' in node
/soc/usb-phy@e65ee000 or bad phandle (referred from
/soc/usb@ee02:phys[0])

Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index be4f25c..cd12176 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -795,6 +795,7 @@
 
usb3_phy0: usb-phy@e65ee000 {
reg = <0 0xe65ee000 0 0x90>;
+   #phy-cells = <0>;
/* placeholder */
};
 
-- 
2.7.4

[PATCH 5/6] arm64: dts: renesas: r8a77965: Add #pwm-cells property

[Jacopo Mondi]

Add "#pwm-cells" property to "pwm@e6e31000" device node.

This silences the following DTC compiler warning:
Warning (pwms_property): Missing property '#pwm-cells' in node
/soc/pwm@e6e31000 or bad phandle (referred from /backlight:pwms[0])

Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index cd12176..cdd4868 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -668,6 +668,7 @@
 
pwm1: pwm@e6e31000 {
reg = <0 0xe6e31000 0 8>;
+   #pwm-cells = <2>;
/* placeholder */
};
 
-- 
2.7.4

[PATCH 6/6] arm64: dts: renesas: r8a77965: Add #interrupt-cells property

[Jacopo Mondi]

Add "#interrupt-cells" property and "interrupt-controller" label to
"interrupt-controller@e61c" device node.

This silences the following DTC compiler warnings:
Warning (interrupts_property): Missing interrupt-controller or
interrupt-map property in /soc/interrupt-controller@e61c
Warning (interrupts_property): Missing #interrupt-cells in
interrupt-parent /soc/interrupt-controller@e61c000

Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index cdd4868..665e126 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -316,6 +316,8 @@
};
 
intc_ex: interrupt-controller@e61c {
+   #interrupt-cells = <2>;
+   interrupt-controller;
reg = <0 0xe61c 0 0x200>;
/* placeholder */
};
-- 
2.7.4

Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970

[Sergei Shtylyov]

On 02/22/2018 09:46 PM, Laurent Pinchart wrote:

>>> From: Sergei Shtylyov 
>>>
>>> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
>>> and VSP2-D instances") added support for the VSP2-D found in the R-Car
>>> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
>>> garbage-like line repeated every 8 screen rows. It turns out that R-Car
>>> V3M has the LIF0 buffer attribute register that you need to set to a non-
>>> default value in order to get rid of the output artifacts.
>>>
>>> Based on the original (and large) patch by Daisuke Matsushita
>>> .
>>>
>>> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
>>> VSP2-D instances") Signed-off-by: Sergei Shtylyov
>>>  Reviewed-by: Laurent Pinchart
>>>  [Removed braces, added
>>> VI6_IP_VERSION_MASK to improve readabiliy]
>>> Signed-off-by: Laurent Pinchart
>>> 
>>
>> [...]
>>
>>> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
>>> b/drivers/media/platform/vsp1/vsp1_regs.h index
>>> b1912c83a1da..dae0c1901297 100644
>>> --- a/drivers/media/platform/vsp1/vsp1_regs.h
>>> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
>>
>> [...]
>>
>>> @@ -705,6 +710,7 @@
>>>   */
>>>  
>>>  #define VI6_IP_VERSION 0x3f00
>>>
>>> +#define VI6_IP_VERSION_MASK(0x << 0)
>>
>> Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be 
>> clearer?
> 
> I thought about it and the line length went over 80 characters so I went for 
> an easy solution. I can change it if you want.

   OK, let's be leave it as is.

MBR, Sergei

Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970

[Laurent Pinchart]

Hi Sergei,

On Thursday, 22 February 2018 20:34:37 EET Sergei Shtylyov wrote:
> On 02/22/2018 07:32 PM, Laurent Pinchart wrote:
> > From: Sergei Shtylyov 
> > 
> > Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> > and VSP2-D instances") added support for the VSP2-D found in the R-Car
> > V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> > garbage-like line repeated every 8 screen rows. It turns out that R-Car
> > V3M has the LIF0 buffer attribute register that you need to set to a non-
> > default value in order to get rid of the output artifacts.
> > 
> > Based on the original (and large) patch by Daisuke Matsushita
> > .
> > 
> > Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
> > VSP2-D instances") Signed-off-by: Sergei Shtylyov
> >  Reviewed-by: Laurent Pinchart
> >  [Removed braces, added
> > VI6_IP_VERSION_MASK to improve readabiliy]
> > Signed-off-by: Laurent Pinchart
> > 
> 
> [...]
> 
> > diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> > b/drivers/media/platform/vsp1/vsp1_regs.h index
> > b1912c83a1da..dae0c1901297 100644
> > --- a/drivers/media/platform/vsp1/vsp1_regs.h
> > +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> 
> [...]
> 
> > @@ -705,6 +710,7 @@
> >   */
> >  
> >  #define VI6_IP_VERSION 0x3f00
> > 
> > +#define VI6_IP_VERSION_MASK(0x << 0)
> 
> Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be 
> clearer?

I thought about it and the line length went over 80 characters so I went for 
an easy solution. I can change it if you want.

-- 
Regards,

Laurent Pinchart

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

[Sergei Shtylyov]

On 02/22/2018 07:26 PM, Laurent Pinchart wrote:
> Hi Sergei,
> 
> Thank you for the patch.
> 
> On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
>> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
>> and VSP2-D instances") added support for the VSP2-D found in the R-Car
>> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
>> garbage-like line repeated every 8 screen rows. It turns out that R-Car
>> V3M has the LIF0 buffer attribute register that you need to set to a non-
>> default value in order to get rid of the output artifacts...
>>
>> Based on the original (and large) patch by Daisuke Matsushita
>> .
>>
>> 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 'fixes' branch.
>>
>> Changes in version 3:
>> - reworded the comment in lif_configure();
>> - reworded the patch description.
>>
>> 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  |   15 +++
>>  drivers/media/platform/vsp1/vsp1_regs.h |5 +
>>  2 files changed, 20 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,21 @@ 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);
>> +
>> +/*
>> + * On R-Car V3M the LIF0 buffer attribute 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 manual is not explained but is likely a buffer size or
>> + * threshold...
> 
> One period is enough :-)

   Naw, I just like ellipsis! :-)

>> + */
>> +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));
>> +}
> 
> There's no need for braces

   Well, multi-line statement was asking for them...

> or inner parentheses.

   I thought so too. :-)

> To make this easier to read I 
> propose defining a new macro VI6_IP_VERSION_MASK that combines both the model 
> and SoC. Otherwise this looks good to me,
> 
> Reviewed-by: Laurent Pinchart 
> 
> I'll post a v4 with that change in reply to this e-mail, please let me know 
> if 
> you're fine with it.

   Generally fine, yes...

[...]

MBR, Sergei

[PATCH v4] v4l: vsp1: Fix video output on R8A77970

[Laurent Pinchart]

From: Sergei Shtylyov 

Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
and VSP2-D instances") added support for the VSP2-D found in the R-Car
V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
garbage-like line repeated every 8 screen rows. It turns out that R-Car
V3M has the LIF0 buffer attribute register that you need to set to a non-
default value in order to get rid of the output artifacts.

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

Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and 
VSP2-D instances")
Signed-off-by: Sergei Shtylyov 
Reviewed-by: Laurent Pinchart 
[Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_lif.c  | 12 
 drivers/media/platform/vsp1/vsp1_regs.h |  6 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_lif.c 
b/drivers/media/platform/vsp1/vsp1_lif.c
index e6fa16d7fda8..704920753998 100644
--- a/drivers/media/platform/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/vsp1/vsp1_lif.c
@@ -155,6 +155,18 @@ static void lif_configure(struct vsp1_entity *entity,
(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+   /*
+* On R-Car V3M the LIF0 buffer attribute 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 manual is not
+* explained but is likely a buffer size or threshold.
+*/
+   if ((entity->vsp1->version & VI6_IP_VERSION_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));
 }
 
 static const struct vsp1_entity_operations lif_entity_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h 
b/drivers/media/platform/vsp1/vsp1_regs.h
index b1912c83a1da..dae0c1901297 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/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_LBA0x3b0c
+#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
  */
@@ -705,6 +710,7 @@
  */
 
 #define VI6_IP_VERSION 0x3f00
+#define VI6_IP_VERSION_MASK(0x << 0)
 #define VI6_IP_VERSION_MODEL_MASK  (0xff << 8)
 #define VI6_IP_VERSION_MODEL_VSPS_H2   (0x09 << 8)
 #define VI6_IP_VERSION_MODEL_VSPR_H2   (0x0a << 8)
-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Sergei,

On Thursday, 22 February 2018 18:26:20 EET Laurent Pinchart wrote:
> On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
> > Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> > and VSP2-D instances") added support for the VSP2-D found in the R-Car
> > V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> > garbage-like line repeated every 8 screen rows. It turns out that R-Car
> > V3M has the LIF0 buffer attribute register that you need to set to a non-
> > default value in order to get rid of the output artifacts...
> > 
> > Based on the original (and large) patch by Daisuke Matsushita
> > .
> > 
> > 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 'fixes' branch.
> > 
> > Changes in version 3:
> > - reworded the comment in lif_configure();
> > - reworded the patch description.
> > 
> > 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  |   15 +++
> >  drivers/media/platform/vsp1/vsp1_regs.h |5 +
> >  2 files changed, 20 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,21 @@ 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);
> > 
> > +
> > +   /*
> > +* On R-Car V3M the LIF0 buffer attribute 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 manual is not explained but is likely a buffer size or
> > +* threshold...
> 
> One period is enough :-)
> 
> > +*/
> > +   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));
> > +   }
> 
> There's no need for braces or inner parentheses.

There's of course a need for the inner parentheses, please ignore this 
comment.

> To make this easier to read I propose defining a new macro
> VI6_IP_VERSION_MASK that combines both the model and SoC. Otherwise this
> looks good to me,
> 
> Reviewed-by: Laurent Pinchart 
> 
> I'll post a v4 with that change in reply to this e-mail, please let me know
> if you're fine with it.
> 
> >  }
> >  
> >  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_LBA0x3b0c
> > +#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

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

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> and VSP2-D instances") added support for the VSP2-D found in the R-Car
> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> garbage-like line repeated every 8 screen rows. It turns out that R-Car
> V3M has the LIF0 buffer attribute register that you need to set to a non-
> default value in order to get rid of the output artifacts...
> 
> Based on the original (and large) patch by Daisuke Matsushita
> .
> 
> 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 'fixes' branch.
> 
> Changes in version 3:
> - reworded the comment in lif_configure();
> - reworded the patch description.
> 
> 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  |   15 +++
>  drivers/media/platform/vsp1/vsp1_regs.h |5 +
>  2 files changed, 20 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,21 @@ 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);
> +
> + /*
> +  * On R-Car V3M the LIF0 buffer attribute 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 manual is not explained but is likely a buffer size or
> +  * threshold...

One period is enough :-)

> +  */
> + 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));
> + }

There's no need for braces or inner parentheses. To make this easier to read I 
propose defining a new macro VI6_IP_VERSION_MASK that combines both the model 
and SoC. Otherwise this looks good to me,

Reviewed-by: Laurent Pinchart 

I'll post a v4 with that change in reply to this e-mail, please let me know if 
you're fine with it.

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

Re: [PATCH 1/4] v4l: vsp1: fix mask creation for MULT_ALPHA_RATIO

[Laurent Pinchart]

Hi Wolfram,

Thank you for the patch.

On Monday, 5 February 2018 22:09:58 EET Wolfram Sang wrote:
> Due to a typo, the mask was destroyed by a comparison instead of a bit
> shift. No regression since the mask has not been used yet.
> 
> Signed-off-by: Wolfram Sang 

Oops.

Reviewed-by: Laurent Pinchart 

and taken in my tree.

> ---
> Only build tested. To be applied individually per subsystem.
> 
>  drivers/media/platform/vsp1/vsp1_regs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> b/drivers/media/platform/vsp1/vsp1_regs.h index
> 26c4ffad2f4656..b1912c83a1dae2 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -225,7 +225,7 @@
>  #define VI6_RPF_MULT_ALPHA_P_MMD_RATIO   (1 << 8)
>  #define VI6_RPF_MULT_ALPHA_P_MMD_IMAGE   (2 << 8)
>  #define VI6_RPF_MULT_ALPHA_P_MMD_BOTH(3 << 8)
> -#define VI6_RPF_MULT_ALPHA_RATIO_MASK(0xff < 0)
> +#define VI6_RPF_MULT_ALPHA_RATIO_MASK(0xff << 0)
>  #define VI6_RPF_MULT_ALPHA_RATIO_SHIFT   0
> 
>  /* 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node

[Simon Horman]

On Wed, Feb 21, 2018 at 09:53:58PM +0300, Sergei Shtylyov wrote:
> On 02/21/2018 09:23 PM, Simon Horman wrote:
> 
> > ...
> > 
>  +clocks = < CPG_MOD 812>;
>  +power-domains = < 32>;
>  +resets = < 812>;
>  +phy-mode = "rgmii-txid";
> >>>
> >>>Why not just "rgmii"? TX delay is a board specific detail, no?
> >>>
> >> I admit I took this one straight from r8a7796 dtsi.
> >> Would you like to me resend and change this?
> >
> >Yes, unless Simon would fix it while merging...
> 
>  Can I confirm the desired change is s/rgmii-txid/rgmii/ ?
> >>>
> >>>Yes.
> >>
> >>Apparently that means that this prop should be overridden in the board 
> >> file
> >> (which may not be an easy task given the board is Salvator-XS again).
> >>
> >> [...]
> > 
> > Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts?
> 
>In salvator-common.dtsi most probably -- it has the PHY data for Ether AVB.
> 
> > I feel that I'm missing an important point here.
> 
>Well, r8a779{5|6}.dtsi also have phy-mode = "rgmii-txid" (which was
>unjustified in my current understanding). Thus such board override
>wouldn't hurt them. But we lack a patch modifying salvator-common.dtsi
>in htis series, so I'm now thinking a respin of this series is needed
>anyway... sorry for being unclear. :-)

While I've applied other patches in this series I have
not applied this one - mainly to allow this discussion to conclude.

Is an appropriate solution to do the following?

1) Atomically update
   a) r8a779{5|6}.dtsi to use "rgmii" and
   b) salvator-common.dtsi to use "rgmii-txid"
2) Update this patch to use "rgmii"

Sorry if I'm still missing the point.

Re: [PATCH v2 09/19] ARM64: dts: Add R-Car Salvator-x M3-N support

[Arnd Bergmann]

On Thu, Feb 22, 2018 at 4:38 PM, Simon Horman  wrote:
> On Thu, Feb 22, 2018 at 02:28:46PM +0100, Geert Uytterhoeven wrote:

>> Removing the .dts file removes the ability to boot the newly added board...
>>
>> The issue here is that we are sharing board .dtsi for boards that can be
>> equipped with different pin-compatible SiPs (Ri-Car H3, M3-W, M3-N), while
>> not all SoC support has been enabled and/or tested. As the board .dtsi
>> references on-SoC devices using phandles, the new r8a77965.dtsi declares
>> placeholders for the devices that are not yet enabled/tested.
>> This worked fine for r8a7796.dtsi before, but causes dtc warnings due to
>> recent improvements in DT validation.
>>
>> I think the simplest way to fix this is to add reg properties to the
>> placeholders, as Simon did for the last two placeholders in r8a7796.dtsi in
>> commit 4316989537a6ed53 ("arm64: dts: renesas: r8a7796: add reg properties to
>> pciec[01] nodes").
>
> I think the best solution is an incremental patch on top of the renesas
> devel branch to, as Geert suggests, add reg properties to the placeholders.
>
> I've asked Jacopo to prepare such a patch.

Sounds good, thanks a lot for the quick reply!

 Arnd

Re: [PATCH v2 09/19] ARM64: dts: Add R-Car Salvator-x M3-N support

[Simon Horman]

On Thu, Feb 22, 2018 at 02:28:46PM +0100, Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> On Thu, Feb 22, 2018 at 12:39 PM, Arnd Bergmann  wrote:
> > On Wed, Feb 21, 2018 at 6:15 PM, Simon Horman  wrote:
> >> On Tue, Feb 20, 2018 at 04:46:23PM +0100, Geert Uytterhoeven wrote:
> >>> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
> >>> wrote:
> >>> > Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
> >>> >
> >>> > Based on original work from:
> >>> > Takeshi Kihara 
> >>> > Magnus Damm 
> >>> >
> >>> > Signed-off-by: Jacopo Mondi 
> >>>
> >>> Reviewed-by: Geert Uytterhoeven 
> >>
> >> Thanks, applied with prefix updated to
> >>
> >> "arm64: dts: renesas:"
> >>
> >> There was some fuzz applying this patch, the result is as follows.
> >>
> >> From: Jacopo Mondi 
> >> Subject: [PATCH] arm64: dts: renesas: Add R-Car Salvator-x M3-N support
> >>
> >> Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
> >>
> >> Based on original work from:
> >> Takeshi Kihara 
> >> Magnus Damm 
> >>
> >> Signed-off-by: Jacopo Mondi 
> >> Reviewed-by: Geert Uytterhoeven 
> >
> > Something went wrong with this patch, I suspect it's missing some 
> > prerequisites
> > that fill in a few more device nodes to avoid these warnings in linux-next:
> >
> > arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
> > (reg_format): "reg" property in /soc/ethernet@e680/ethernet-phy@0
> > has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
> 
> [...]
> 
> > Maybe just take out the patch adding the .dts file for the time being until
> > the important nodes are all in place?
> 
> Removing the .dts file removes the ability to boot the newly added board...
> 
> The issue here is that we are sharing board .dtsi for boards that can be
> equipped with different pin-compatible SiPs (Ri-Car H3, M3-W, M3-N), while
> not all SoC support has been enabled and/or tested. As the board .dtsi
> references on-SoC devices using phandles, the new r8a77965.dtsi declares
> placeholders for the devices that are not yet enabled/tested.
> This worked fine for r8a7796.dtsi before, but causes dtc warnings due to
> recent improvements in DT validation.
> 
> I think the simplest way to fix this is to add reg properties to the
> placeholders, as Simon did for the last two placeholders in r8a7796.dtsi in
> commit 4316989537a6ed53 ("arm64: dts: renesas: r8a7796: add reg properties to
> pciec[01] nodes").

Hi Arnd,

Sorry for letting this slip through.

I think the best solution is an incremental patch on top of the renesas
devel branch to, as Geert suggests, add reg properties to the placeholders.

I've asked Jacopo to prepare such a patch.

Re: [PATCH/RFT v4] gpio: gpio-rcar: Support S2RAM

[Simon Horman]

On Thu, Feb 22, 2018 at 03:51:27PM +0100, Linus Walleij wrote:
> On Sun, Feb 4, 2018 at 8:15 PM, Yoshihiro Kaneko  
> wrote:
> 
> > 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 v4 version applied with Tested- and reviewed tags.

Excellent, thanks!

Re: [PATCH/RFT v4] gpio: gpio-rcar: Support S2RAM

[Linus Walleij]

On Sun, Feb 4, 2018 at 8:15 PM, Yoshihiro Kaneko  wrote:

> 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 v4 version applied with Tested- and reviewed tags.

Yours,
Linus Walleij

Re: [PATCH v6 4/4] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2018-02-22 15:13:36 +0200, 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 

I'm not so strong when it comes to DRM, but I have done my best to grasp 
this patch. Looking at datasheets and what the code looked before feel 
free to add.

Reviewed-by: Niklas Söderlund 

> ---
> Changes since v1:
> 
> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> - Update to the -lvds compatible string format
> ---
>  drivers/gpu/drm/rcar-du/Kconfig   |   4 +-
>  drivers/gpu/drm/rcar-du/Makefile  |   3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c |  21 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h |   5 -
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +-
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.h |  12 -
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  14 +-
>  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c |  93 --
>  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h |  24 --
>  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 --
>  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h |  64 
>  drivers/gpu/drm/rcar-du/rcar_lvds.c   | 524 
> ++
>  12 files changed, 561 insertions(+), 616 deletions(-)
>  delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
>  delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
>  delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>  delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 3f83352a7313..edde8d4b87a3 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -19,8 +19,8 @@ config DRM_RCAR_DW_HDMI
> Enable support for R-Car Gen3 internal HDMI encoder.
>  
>  config DRM_RCAR_LVDS
> - bool "R-Car DU LVDS Encoder Support"
> - depends on DRM_RCAR_DU
> + tristate "R-Car DU LVDS Encoder Support"
> + depends on DRM && DRM_BRIDGE && OF
>   select DRM_PANEL
>   select OF_FLATTREE
>   select OF_OVERLAY
> diff --git a/drivers/gpu/drm/rcar-du/Makefile 
> b/drivers/gpu/drm/rcar-du/Makefile
> index 86b337b4be5d..3e58ed93d5b1 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -4,10 +4,8 @@ rcar-du-drm-y := rcar_du_crtc.o \
>rcar_du_encoder.o \
>rcar_du_group.o \
>rcar_du_kms.o \
> -  rcar_du_lvdscon.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.o \
>  rcar_du_of_lvds_r8a7790.dtb.o \
>  rcar_du_of_lvds_r8a7791.dtb.o \
> @@ -18,3 +16,4 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)  += rcar_du_vsp.o
>  
>  obj-$(CONFIG_DRM_RCAR_DU)+= rcar-du-drm.o
>  obj-$(CONFIG_DRM_RCAR_DW_HDMI)   += rcar_dw_hdmi.o
> +obj-$(CONFIG_DRM_RCAR_LVDS)  += rcar_lvds.o
> 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
> @@ -29,6 +29,7 @@
>  
>  #include "rcar_du_drv.h"
>  #include "rcar_du_kms.h"
> +#include "rcar_du_of.h"
>  #include "rcar_du_regs.h"
>  
>  /* 
> -
> @@ -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 

Re: [PATCH v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

Hi Linus,

On Thu, Feb 22, 2018 at 3:23 PM, Linus Walleij  wrote:
> On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
>  wrote:
>
>> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
>> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
>> block's module clock (if exists) is manually kept running during system
>> suspend, to make sure the device stays active.
>>
>> However, this explicit clock handling is merely a workaround for a
>> failure to properly communicate wakeup information to the device core.
>>
>> Instead, set the device's power.wakeup_path field, to indicate this
>> device is part of the wakeup path.  Depending on the PM Domain's
>> active_wakeup configuration, the genpd core code will keep the device
>> enabled (and the clock running) during system suspend when needed.
>> This allows for the removal of all explicit clock handling code from the
>> driver.
>>
>> Signed-off-by: Geert Uytterhoeven 
>
> Acked-by: Linus Walleij 
>
> Can I apply this and only this patch to GPIO?

Yes please.

> Should it be applied for fixes, next?

No, as it depends on infrastructure (device_set_wakeup_path()) and fixes
(various "Keep wakeup sources active during system suspend") in v4.16-rc1.

Thanks!

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 v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[jacopo mondi]

Hi Laurent,

On Thu, Feb 22, 2018 at 02:47:06PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Thursday, 22 February 2018 14:36:00 EET jacopo mondi wrote:
> > On Thu, Feb 22, 2018 at 02:14:53PM +0200, Laurent Pinchart wrote:
> > > On Thursday, 22 February 2018 14:04:12 EET jacopo mondi wrote:
> > >> On Wed, Feb 21, 2018 at 10:28:06PM +0200, Laurent Pinchart wrote:
> > >>> On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:
>
> [snip]
>
> >  This actually makes me wonder if those informations (ycbcb_enc,
> >  quantization and xfer_func) shouldn't actually be part of the
> >  supported format list... I blindly added those default fields in the
> >  try_fmt function, but I doubt they applies to all supported formats.
> > 
> >  Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
> >  RGB 565) and 1 raw format (BGGR). I now have a question here:
> > 
> >  1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
> >  applies to RGB and raw formats? I don't think so, and what value is
> >  the correct one for the ycbcr_enc field in this case? I assume
> >  xfer_func and quantization applies to all formats instead..
> > >>>
> > >>> There's no encoding for RGB formats if I understand things correctly,
> > >>> so I'd set ycbcr_enc to V4L2_YCBCR_ENC_DEFAULT. The transfer function
> > >>> and the quantization apply to all formats, but I'd be surprised to find
> > >>> a sensor outputting limited range for RGB.
> > >>
> > >> Ack, we got the same understanding for RGB formats. I wonder if for
> > >> those formats we wouldn't need a V4L2_YCBCR_ENC_NONE or similar...
> > >
> > > That, or explicitly documenting that when the format is not YUV the field
> > > should be set by both drivers and applications to V4L2_YCBCR_ENC_DEFAULT
> > > when written and ignored when read.
> >
> > Well, if no encoding is performed because the color encoding scheme is
> > RGB, the colorspace does anyway define an encoding method, so it seems
> > to me the latter is more appropriate (use DEFAULT and ignore when read).
> >
> > That's anyway just my opinion, but I could send a patch for
> > documentation and see what feedback it gets.
> >
> > >>> Have you been able to check whether the sensor outputs limited range
> > >>> of full range YUV ? If it outputs full range you can hardcode
> > >>> quantization to V4L2_QUANTIZATION_FULL_RANGE for all formats.
> > >>
> > >> In YUYV mode, I see values > 0xf0 ( > 240, which is the max value for
> > >> CbCr samples in limited quantization range), so I assume quantization
> > >> is full range.
> > >
> > > It should be, yes. What's the minimum and maximum values you get ?
> >
> > From a white surface:
> > min = 0x39
> > max = 0xfc
> >
> > From a black surface:
> > min = 0x00 (with 62 occurrences)
> > max = 0x8b
> >
> > I guess that's indeed full range quantization
>
> Could you check Y and UV separately ?
>
> #!/usr/bin/python
>
> import sys
>
> def main(argv):
> if len(argv) != 2:
> print('Usage: %s ' % argv[0])
> return 1
>
> data = open(argv[1], 'rb').read()
>
> y_min = 255
> y_max = 0
> uv_min = 255
> uv_max = 0
>
> for i in range(len(data) // 2):
> y = data[2*i]
> uv = data[2*i]

uv = data[2*i+1]

>
> y_min = min(y_min, y)
> y_max = max(y_max, y)
> uv_min = min(uv_min, uv)
> uv_max = max(uv_max, uv)
>
> print('Y [%u, %u] UV [%u, %u]' % (y_min, y_max, uv_min, uv_max))
> return 0
>
> if __name__ == '__main__':
> sys.exit(main(sys.argv))
>


White image:
Y [57, 252] UV [105, 145]

Black image:
Y [0, 32] UV [116, 139]

Re: [PATCH v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Linus Walleij]

On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
 wrote:

> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
> block's module clock (if exists) is manually kept running during system
> suspend, to make sure the device stays active.
>
> However, this explicit clock handling is merely a workaround for a
> failure to properly communicate wakeup information to the device core.
>
> Instead, set the device's power.wakeup_path field, to indicate this
> device is part of the wakeup path.  Depending on the PM Domain's
> active_wakeup configuration, the genpd core code will keep the device
> enabled (and the clock running) during system suspend when needed.
> This allows for the removal of all explicit clock handling code from the
> driver.
>
> Signed-off-by: Geert Uytterhoeven 

Acked-by: Linus Walleij 

Can I apply this and only this patch to GPIO?

Should it be applied for fixes, next?

Else please funnel this through the irqchip tree with my
ACK.

Yours,
Linus Walleij

Re: [PATCH v6 2/4] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2018-02-22 15:13:34 +0200, Laurent Pinchart wrote:
> The internal LVDS encoders now have their own DT bindings, representing
> them as part of the DU is deprecated.
> 
> Signed-off-by: Laurent Pinchart 
> Reviewed-by: Rob Herring 

Reviewed-by: Niklas Söderlund 

> ---
> Changes since v1:
> 
> - Remove the LVDS reg range from the example
> - Remove the reg-names property
> ---
>  .../devicetree/bindings/display/renesas,du.txt | 31 
> --
>  1 file changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt 
> b/Documentation/devicetree/bindings/display/renesas,du.txt
> index cd48aba3bc8c..e79cf9b0ad38 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -14,12 +14,7 @@ Required Properties:
>  - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
>  - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
>  
> -  - reg: A list of base address and length of each memory resource, one for
> -each entry in the reg-names property.
> -  - reg-names: Name of the memory resources. The DU requires one memory
> -resource for the DU core (named "du") and one memory resource for each
> -LVDS encoder (named "lvds.x" with "x" being the LVDS controller numerical
> -index).
> +  - reg: the memory-mapped I/O registers base address and length
>  
>- interrupt-parent: phandle of the parent interrupt controller.
>- interrupts: Interrupt specifiers for the DU interrupts.
> @@ -29,14 +24,13 @@ Required Properties:
>- clock-names: Name of the clocks. This property is model-dependent.
>  - R8A7779 uses a single functional clock. The clock doesn't need to be
>named.
> -- All other DU instances use one functional clock per channel and one
> -  clock per LVDS encoder (if available). The functional clocks must be
> -  named "du.x" with "x" being the channel numerical index. The LVDS 
> clocks
> -  must be named "lvds.x" with "x" being the LVDS encoder numerical index.
> -- In addition to the functional and encoder clocks, all DU versions also
> -  support externally supplied pixel clocks. Those clocks are optional.
> -  When supplied they must be named "dclkin.x" with "x" being the input
> -  clock numerical index.
> +- All other DU instances use one functional clock per channel The
> +  functional clocks must be named "du.x" with "x" being the channel
> +  numerical index.
> +- In addition to the functional clocks, all DU versions also support
> +  externally supplied pixel clocks. Those clocks are optional. When
> +  supplied they must be named "dclkin.x" with "x" being the input clock
> +  numerical index.
>  
>- vsps: A list of phandle and channel index tuples to the VSPs that handle
>  the memory interfaces for the DU channels. The phandle identifies the VSP
> @@ -69,9 +63,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
>  
>   du: display@feb0 {
>   compatible = "renesas,du-r8a7795";
> - reg = <0 0xfeb0 0 0x8>,
> -   <0 0xfeb9 0 0x14>;
> - reg-names = "du", "lvds.0";
> + reg = <0 0xfeb0 0 0x8>;
>   interrupts = ,
>,
>,
> @@ -79,9 +71,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
>   clocks = < CPG_MOD 724>,
>< CPG_MOD 723>,
>< CPG_MOD 722>,
> -  < CPG_MOD 721>,
> -  < CPG_MOD 727>;
> - clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0";
> +  < CPG_MOD 721>;
> + clock-names = "du.0", "du.1", "du.2", "du.3";
>   vsps = < 0>, < 0>, < 0>, < 1>;
>  
>   ports {
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v6 1/4] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2018-02-22 15:13:33 +0200, Laurent Pinchart wrote:
> The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add
> corresponding device tree bindings.
> 
> Signed-off-by: Laurent Pinchart 
> Reviewed-by: Rob Herring 
> ---
> Changes since v1:
> 
> - Move the SoC name before the IP name in compatible strings
> - Rename parallel input to parallel RGB input
> - Fixed "renesas,r8a7743-lvds" description
> - Document the resets property
> - Fixed typo
> ---
>  .../bindings/display/bridge/renesas,lvds.txt   | 56 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 57 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
> b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> new file mode 100644
> index ..2b19ce51ec07
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -0,0 +1,56 @@
> +Renesas R-Car LVDS Encoder
> +==
> +
> +These DT bindings describe the LVDS encoder embedded in the Renesas R-Car
> +Gen2, R-Car Gen3 and RZ/G SoCs.
> +
> +Required properties:
> +
> +- compatible : Shall contain one of
> +  - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
> +  - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders
> +  - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders
> +  - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders

Small typo here s/R8A7791/R8A7793/, with that fixed 

Reviewed-by: Niklas Söderlund 


> +  - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders
> +  - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders
> +
> +- reg: Base address and length for the memory-mapped registers
> +- clocks: A phandle + clock-specifier pair for the functional clock
> +- resets: A phandle + reset specifier for the module reset
> +
> +Required nodes:
> +
> +The LVDS encoder has two video ports. Their connections are modelled using 
> the
> +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 corresponds to the parallel RGB input
> +- Video port 1 corresponds to the LVDS output
> +
> +Each port shall have a single endpoint.
> +
> +
> +Example:
> +
> + lvds0: lvds@feb9 {
> + compatible = "renesas,r8a7790-lvds";
> + reg = <0 0xfeb9 0 0x1c>;
> + clocks = < CPG_MOD 726>;
> + resets = < 726>;
> +
> + 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 {
> + };
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2afba909724c..13c8ec11135a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4744,6 +4744,7 @@ F:  drivers/gpu/drm/rcar-du/
>  F:   drivers/gpu/drm/shmobile/
>  F:   include/linux/platform_data/shmob_drm.h
>  F:   Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
> +F:   Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
>  F:   Documentation/devicetree/bindings/display/renesas,du.txt
>  
>  DRM DRIVERS FOR ROCKCHIP
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 09/19] ARM64: dts: Add R-Car Salvator-x M3-N support

[Geert Uytterhoeven]

Hi Arnd,

On Thu, Feb 22, 2018 at 12:39 PM, Arnd Bergmann  wrote:
> On Wed, Feb 21, 2018 at 6:15 PM, Simon Horman  wrote:
>> On Tue, Feb 20, 2018 at 04:46:23PM +0100, Geert Uytterhoeven wrote:
>>> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
>>> wrote:
>>> > Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
>>> >
>>> > Based on original work from:
>>> > Takeshi Kihara 
>>> > Magnus Damm 
>>> >
>>> > Signed-off-by: Jacopo Mondi 
>>>
>>> Reviewed-by: Geert Uytterhoeven 
>>
>> Thanks, applied with prefix updated to
>>
>> "arm64: dts: renesas:"
>>
>> There was some fuzz applying this patch, the result is as follows.
>>
>> From: Jacopo Mondi 
>> Subject: [PATCH] arm64: dts: renesas: Add R-Car Salvator-x M3-N support
>>
>> Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
>>
>> Based on original work from:
>> Takeshi Kihara 
>> Magnus Damm 
>>
>> Signed-off-by: Jacopo Mondi 
>> Reviewed-by: Geert Uytterhoeven 
>
> Something went wrong with this patch, I suspect it's missing some 
> prerequisites
> that fill in a few more device nodes to avoid these warnings in linux-next:
>
> arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
> (reg_format): "reg" property in /soc/ethernet@e680/ethernet-phy@0
> has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)

[...]

> Maybe just take out the patch adding the .dts file for the time being until
> the important nodes are all in place?

Removing the .dts file removes the ability to boot the newly added board...

The issue here is that we are sharing board .dtsi for boards that can be
equipped with different pin-compatible SiPs (Ri-Car H3, M3-W, M3-N), while
not all SoC support has been enabled and/or tested. As the board .dtsi
references on-SoC devices using phandles, the new r8a77965.dtsi declares
placeholders for the devices that are not yet enabled/tested.
This worked fine for r8a7796.dtsi before, but causes dtc warnings due to
recent improvements in DT validation.

I think the simplest way to fix this is to add reg properties to the
placeholders, as Simon did for the last two placeholders in r8a7796.dtsi in
commit 4316989537a6ed53 ("arm64: dts: renesas: r8a7796: add reg properties to
pciec[01] nodes").

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH v6 0/4] R-Car DU: Convert LVDS code to bridge driver

[Laurent Pinchart]

Hello,

This patch series addresses a design mistake that dates back from the initial
DU support. Support for the LVDS encoders, which are IP cores separate from
the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
described through a single DT node.

To fix the, patches 1/4 and 2/4 define new DT bindings for the LVDS
encoders, and deprecate their description inside the DU bindings. To retain
backward compatibility with existing DT, patch 3/4 then patch the device tree
at runtime to convert the legacy bindings to the new ones.

With the DT side addressed, patch 4/4 converts the LVDS support code to a
separate bridge driver.

I decided to go for live DT patching in patch 3/4 because implementing
support for both the legacy and new bindings in the driver would have been
very intrusive, and prevented further cleanups. This version relies more
heavily on overlays to avoid touching the internals of the OF core compared to
v2, even if manual fixes to the device tree are still needed.

As all the patches but the last one have been acked, I plan to send a pull
request by the end of the week if no new issue is discovered.

Compared to v5, I've dropped the OF changeset halpers series as Frank raised
concerns about hidding it in the middle of a driver patch series. I've thus
copied the implementation of of_changeset_add_property_copy() in the driver to
avoid blocking this series. The function arguments are identical, so when the
OF changeset helpers will land it will be very easy to drop the private copy
and use the of_changeset_add_property_copy() helper.

Compared to v4, the patch "of: changeset: Add of_changeset_node_move method"
has been dropped as it's not needed. The patches that update the DT sources to
the new DU and LVDS bindings have been dropped as well to avoid spamming the
lists as they depend on the driver changes going in first to avoid
regressions and haven't been changed.

Compared to v3, this series uses the OF changeset API to update properties
instead of accessing the internals of the property structure. This removes the
local implementation of functions to look up nodes by path and update
properties. In order to do this, I pulled in Pantelis' patch series titled
"[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and
rebased it while taking two small review comments into account.

Rob, I'd like this series to be merged in v4.17. As the changeset helpers are
now a dependency, I'd need you to merge them early (ideally on top of
v4.16-rc1) and provide a stable branch, or get your ack to merge them through
Dave's tree if they don't conflict with what you have and will queue for
v4.17.

This version also drops the small fix to the Porter board device tree that has
been queued for v4.17 already.

Compared to v2, the biggest change is in patch 3/4. Following Rob's and
Frank's reviews it was clear that modifying the unflattened DT structure of
the overlay before applying it wasn't popular. I have thus decided to use one
overlay source per SoC to move as much of the DT changes to the overlay as
possible, and only perform manual modifications (that are still needed as some
of the information is board-specific) on the system DT after applying the
overlay. As a result the overlay is parsed and applied without being modified.

Compared to v1, this series update the r8a7792 and r8a7794 device tree sources
and incorporate review feedback as described by the changelogs of individual
patches.

Laurent Pinchart (4):
  dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
  dt-bindings: display: renesas: Deprecate LVDS support in the DU
bindings
  drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
  drm: rcar-du: Convert LVDS encoder code to bridge driver

 .../bindings/display/bridge/renesas,lvds.txt   |  56 +++
 .../devicetree/bindings/display/renesas,du.txt |  31 +-
 MAINTAINERS|   1 +
 drivers/gpu/drm/rcar-du/Kconfig|   6 +-
 drivers/gpu/drm/rcar-du/Makefile   |  10 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  21 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h  |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c  | 175 +--
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h  |  12 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c  |  14 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c  |  93 
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h  |  24 -
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c  | 238 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h  |  64 ---
 drivers/gpu/drm/rcar-du/rcar_du_of.c   | 342 ++
 drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 +
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 +++
 

[PATCH v6 2/4] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings

[Laurent Pinchart]

The internal LVDS encoders now have their own DT bindings, representing
them as part of the DU is deprecated.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Remove the LVDS reg range from the example
- Remove the reg-names property
---
 .../devicetree/bindings/display/renesas,du.txt | 31 --
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt 
b/Documentation/devicetree/bindings/display/renesas,du.txt
index cd48aba3bc8c..e79cf9b0ad38 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -14,12 +14,7 @@ Required Properties:
 - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
 - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
 
-  - reg: A list of base address and length of each memory resource, one for
-each entry in the reg-names property.
-  - reg-names: Name of the memory resources. The DU requires one memory
-resource for the DU core (named "du") and one memory resource for each
-LVDS encoder (named "lvds.x" with "x" being the LVDS controller numerical
-index).
+  - reg: the memory-mapped I/O registers base address and length
 
   - interrupt-parent: phandle of the parent interrupt controller.
   - interrupts: Interrupt specifiers for the DU interrupts.
@@ -29,14 +24,13 @@ Required Properties:
   - clock-names: Name of the clocks. This property is model-dependent.
 - R8A7779 uses a single functional clock. The clock doesn't need to be
   named.
-- All other DU instances use one functional clock per channel and one
-  clock per LVDS encoder (if available). The functional clocks must be
-  named "du.x" with "x" being the channel numerical index. The LVDS clocks
-  must be named "lvds.x" with "x" being the LVDS encoder numerical index.
-- In addition to the functional and encoder clocks, all DU versions also
-  support externally supplied pixel clocks. Those clocks are optional.
-  When supplied they must be named "dclkin.x" with "x" being the input
-  clock numerical index.
+- All other DU instances use one functional clock per channel The
+  functional clocks must be named "du.x" with "x" being the channel
+  numerical index.
+- In addition to the functional clocks, all DU versions also support
+  externally supplied pixel clocks. Those clocks are optional. When
+  supplied they must be named "dclkin.x" with "x" being the input clock
+  numerical index.
 
   - vsps: A list of phandle and channel index tuples to the VSPs that handle
 the memory interfaces for the DU channels. The phandle identifies the VSP
@@ -69,9 +63,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 
du: display@feb0 {
compatible = "renesas,du-r8a7795";
-   reg = <0 0xfeb0 0 0x8>,
- <0 0xfeb9 0 0x14>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x8>;
interrupts = ,
 ,
 ,
@@ -79,9 +71,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
 < CPG_MOD 722>,
-< CPG_MOD 721>,
-< CPG_MOD 727>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0";
+< CPG_MOD 721>;
+   clock-names = "du.0", "du.1", "du.2", "du.3";
vsps = < 0>, < 0>, < 0>, < 1>;
 
ports {
-- 
Regards,

Laurent Pinchart

[PATCH v6 1/4] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Laurent Pinchart]

The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add
corresponding device tree bindings.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Move the SoC name before the IP name in compatible strings
- Rename parallel input to parallel RGB input
- Fixed "renesas,r8a7743-lvds" description
- Document the resets property
- Fixed typo
---
 .../bindings/display/bridge/renesas,lvds.txt   | 56 ++
 MAINTAINERS|  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
new file mode 100644
index ..2b19ce51ec07
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -0,0 +1,56 @@
+Renesas R-Car LVDS Encoder
+==
+
+These DT bindings describe the LVDS encoder embedded in the Renesas R-Car
+Gen2, R-Car Gen3 and RZ/G SoCs.
+
+Required properties:
+
+- compatible : Shall contain one of
+  - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
+  - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders
+  - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders
+  - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders
+  - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders
+  - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders
+
+- reg: Base address and length for the memory-mapped registers
+- clocks: A phandle + clock-specifier pair for the functional clock
+- resets: A phandle + reset specifier for the module reset
+
+Required nodes:
+
+The LVDS encoder has two video ports. Their connections are modelled using the
+OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 corresponds to the parallel RGB input
+- Video port 1 corresponds to the LVDS output
+
+Each port shall have a single endpoint.
+
+
+Example:
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7790-lvds";
+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   resets = < 726>;
+
+   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 {
+   };
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 2afba909724c..13c8ec11135a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4744,6 +4744,7 @@ F:drivers/gpu/drm/rcar-du/
 F: drivers/gpu/drm/shmobile/
 F: include/linux/platform_data/shmob_drm.h
 F: Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
+F: Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
 F: Documentation/devicetree/bindings/display/renesas,du.txt
 
 DRM DRIVERS FOR ROCKCHIP
-- 
Regards,

Laurent Pinchart

[PATCH v6 4/4] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Laurent Pinchart]

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 
---
Changes since v1:

- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Update to the -lvds compatible string format
---
 drivers/gpu/drm/rcar-du/Kconfig   |   4 +-
 drivers/gpu/drm/rcar-du/Makefile  |   3 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |  21 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |  12 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c |  14 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c |  93 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h |  24 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h |  64 
 drivers/gpu/drm/rcar-du/rcar_lvds.c   | 524 ++
 12 files changed, 561 insertions(+), 616 deletions(-)
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 3f83352a7313..edde8d4b87a3 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -19,8 +19,8 @@ config DRM_RCAR_DW_HDMI
  Enable support for R-Car Gen3 internal HDMI encoder.
 
 config DRM_RCAR_LVDS
-   bool "R-Car DU LVDS Encoder Support"
-   depends on DRM_RCAR_DU
+   tristate "R-Car DU LVDS Encoder Support"
+   depends on DRM && DRM_BRIDGE && OF
select DRM_PANEL
select OF_FLATTREE
select OF_OVERLAY
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 86b337b4be5d..3e58ed93d5b1 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -4,10 +4,8 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_encoder.o \
 rcar_du_group.o \
 rcar_du_kms.o \
-rcar_du_lvdscon.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.o \
   rcar_du_of_lvds_r8a7790.dtb.o \
   rcar_du_of_lvds_r8a7791.dtb.o \
@@ -18,3 +16,4 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)+= rcar_du_vsp.o
 
 obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
+obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o
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
@@ -29,6 +29,7 @@
 
 #include "rcar_du_drv.h"
 #include "rcar_du_kms.h"
+#include "rcar_du_of.h"
 #include "rcar_du_regs.h"
 
 /* 
-
@@ -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,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7795_info = {
@@ -434,7 +431,19 @@ static struct platform_driver rcar_du_platform_driver = {
},
 };
 
-module_platform_driver(rcar_du_platform_driver);
+static int __init rcar_du_init(void)
+{
+   

[PATCH v6 3/4] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

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.

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

- Use a private copy of rcar_du_of_changeset_add_property()

Changes since v3:

- Use the OF changeset API
- Use of_graph_get_endpoint_by_regs()
- Replace hardcoded constants by sizeof()

Changes since v2:

- Update the SPDX headers to use C-style comments in header files
- Removed the manually created __local_fixups__ node
- Perform manual fixups on live DT instead of overlay

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   |   7 +-
 drivers/gpu/drm/rcar-du/rcar_du_of.c   | 342 +
 drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 ++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  79 +
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  53 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  53 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  53 
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  53 
 9 files changed, 661 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_r8a7790.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.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
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..86b337b4be5d 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -8,7 +8,12 @@ rcar-du-drm-y := rcar_du_crtc.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.o \
+  rcar_du_of_lvds_r8a7790.dtb.o \
+  rcar_du_of_lvds_r8a7791.dtb.o \
+  rcar_du_of_lvds_r8a7793.dtb.o \
+  rcar_du_of_lvds_r8a7795.dtb.o \
+  rcar_du_of_lvds_r8a7796.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 ..ac442ddfed16
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -0,0 +1,342 @@
+// 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"
+
+/* 
-
+ * Generic Overlay Handling
+ */
+
+struct rcar_du_of_overlay {
+   const char *compatible;
+   void *begin;
+   void *end;
+};
+
+#define RCAR_DU_OF_DTB(type, soc)  \
+   extern char __dtb_rcar_du_of_##type##_##soc##_begin[];  \
+   extern char __dtb_rcar_du_of_##type##_##soc##_end[]
+
+#define RCAR_DU_OF_OVERLAY(type, soc)  \
+   {   \
+   .compatible = 

Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[Laurent Pinchart]

Hi Jacopo,

On Thursday, 22 February 2018 14:36:00 EET jacopo mondi wrote:
> On Thu, Feb 22, 2018 at 02:14:53PM +0200, Laurent Pinchart wrote:
> > On Thursday, 22 February 2018 14:04:12 EET jacopo mondi wrote:
> >> On Wed, Feb 21, 2018 at 10:28:06PM +0200, Laurent Pinchart wrote:
> >>> On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:

[snip]

>  This actually makes me wonder if those informations (ycbcb_enc,
>  quantization and xfer_func) shouldn't actually be part of the
>  supported format list... I blindly added those default fields in the
>  try_fmt function, but I doubt they applies to all supported formats.
>  
>  Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
>  RGB 565) and 1 raw format (BGGR). I now have a question here:
>  
>  1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
>  applies to RGB and raw formats? I don't think so, and what value is
>  the correct one for the ycbcr_enc field in this case? I assume
>  xfer_func and quantization applies to all formats instead..
> >>> 
> >>> There's no encoding for RGB formats if I understand things correctly,
> >>> so I'd set ycbcr_enc to V4L2_YCBCR_ENC_DEFAULT. The transfer function
> >>> and the quantization apply to all formats, but I'd be surprised to find
> >>> a sensor outputting limited range for RGB.
> >> 
> >> Ack, we got the same understanding for RGB formats. I wonder if for
> >> those formats we wouldn't need a V4L2_YCBCR_ENC_NONE or similar...
> > 
> > That, or explicitly documenting that when the format is not YUV the field
> > should be set by both drivers and applications to V4L2_YCBCR_ENC_DEFAULT
> > when written and ignored when read.
> 
> Well, if no encoding is performed because the color encoding scheme is
> RGB, the colorspace does anyway define an encoding method, so it seems
> to me the latter is more appropriate (use DEFAULT and ignore when read).
> 
> That's anyway just my opinion, but I could send a patch for
> documentation and see what feedback it gets.
> 
> >>> Have you been able to check whether the sensor outputs limited range
> >>> of full range YUV ? If it outputs full range you can hardcode
> >>> quantization to V4L2_QUANTIZATION_FULL_RANGE for all formats.
> >> 
> >> In YUYV mode, I see values > 0xf0 ( > 240, which is the max value for
> >> CbCr samples in limited quantization range), so I assume quantization
> >> is full range.
> > 
> > It should be, yes. What's the minimum and maximum values you get ?
> 
> From a white surface:
> min = 0x39
> max = 0xfc
> 
> From a black surface:
> min = 0x00 (with 62 occurrences)
> max = 0x8b
> 
> I guess that's indeed full range quantization

Could you check Y and UV separately ?

#!/usr/bin/python
  
import sys

def main(argv):
if len(argv) != 2:
print('Usage: %s ' % argv[0])
return 1

data = open(argv[1], 'rb').read()

y_min = 255
y_max = 0
uv_min = 255
uv_max = 0

for i in range(len(data) // 2):
y = data[2*i]
uv = data[2*i]

y_min = min(y_min, y)
y_max = max(y_max, y)
uv_min = min(uv_min, uv)
uv_max = max(uv_max, uv)

print('Y [%u, %u] UV [%u, %u]' % (y_min, y_max, uv_min, uv_max))
return 0

if __name__ == '__main__':
sys.exit(main(sys.argv))

> > > Actually the hardest part here was having a white enough surface to
> > > point the sensor to :)
> > 
> > Pointing a flashlight to the sensor usually does the trick.
> 
> I had a yellowish light that didn't work that well, I ended up putting
> a white paper sheet in front of it and then took the picture.

Time to get a white flashlight ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Laurent Pinchart]

Hi Hans,

On Thursday, 22 February 2018 09:38:46 EET Hans Verkuil wrote:
> On 02/21/2018 09:16 PM, Laurent Pinchart wrote:
> > On Tuesday, 20 February 2018 10:37:22 EET Hans Verkuil wrote:
> >> On 02/19/2018 11:28 PM, Niklas Söderlund wrote:
> >>> Hi Hans,
> >>> 
> >>> Thanks for your feedback.
> >>> 
> >>> [snip]
> >>> 
>  Can you then fix v4l2-compliance to stop testing colorspace
>  against 0xff
>  ?
> >>> 
> >>> For now I can simply relax this test for subdevs with sources and
> >>> sinks.
> >> 
> >> You also need to relax it for video nodes with MC drivers, as the DMA
> >> engines don't care about colorspaces.
> > 
> > Yes, they do. Many DMA engines can at least do RGB <-> YUV
> > conversions, so they should get the colorspace info from their source
> > and pass it on to userspace (after correcting for any conversions done
> > by the DMA engine).
>  
>  Not in the MC case. Video nodes there only model the DMA engine, and
>  are thus not aware of colorspaces. What MC drivers do is check at
>  stream on time when validating the pipeline that the colorspace set by
>  userspace on the video node corresponds to the colorspace on the source
>  pad of the connected subdev, but that's only to ensure that userspace
>  gets a coherent view of colorspace across the pipeline, not to program
>  the hardware. There could be exceptions, but in the general case, the
>  video node implementation of an MC driver will accept any colorspace
>  and only validate it at stream on time, similarly to how it does for
>  the frame size format instance (and in the frame size case it will
>  usually enforce min/max limits when the DMA engine limits the frame
>  size).
> >>> 
> >>> I'm afraid the issue described above by Laurent is what sparked me to
> >>> write this commit to begin with. In my never ending VIN Gen3 patch-set I
> >>> currency need to carry a patch [1] to implement a hack to make sure
> >>> v4l2-compliance do not fail for the VIN Gen3 MC-centric use-case. This
> >>> patch was an attempt to be able to validate the colorspace using the
> >>> magic value 0xff.
> >> 
> >> This is NOT a magic value. The test that's done here is to memset the
> >> format structure with 0xff, then call the ioctl. Afterwards it checks
> >> if there are any remaining 0xff bytes left in the struct since it expects
> >> the driver to have overwritten it by something else. That's where the
> >> 0xff comes from.
> > 
> > It's no less or more magic than using 0xdeadbeef or any fixed value :-) I
> > think we all agree that it isn't a value that is meant to be handled
> > specifically by drivers, so it's not magic in that sense.
> > 
> >>> I don't feel strongly for this patch in particular and I'm happy to drop
> >>> it.  But I would like to receive some guidance on how to then properly
> >>> be able to handle this problem for the MC-centric VIN driver use-case.
> >>> One option is as you suggested to relax the test in v4l-compliance to
> >>> not check colorspace, but commit [2] is not enough to resolve the issue
> >>> for my MC use-case.
> >>> 
> >>> As Laurent stated above, the use-case is that the video device shall
> >>> accept any colorspace set from user-space. This colorspace is then only
> >>> used as stream on time to validate the MC pipeline. The VIN driver do
> >>> not care about colorspace, but I care about not breaking v4l2-compliance
> >>> as I find it's a very useful tool :-)
> >> 
> >> I think part of my confusion here is that there are two places where you
> >> deal with colorspaces in a DMA engine: first there is a input pad of the
> >> DMA engine entity, secondly there is the v4l2_pix_format for the memory
> >> description.
> >> 
> >> The second is set by the driver based on what userspace specified for the
> >> input pad, together with any changes due to additional conversions such
> >> as quantization range and RGB <-> YUV by the DMA engine.
> > 
> > No, I'm sorry, for MC-based drivers this isn't correct. The media entity
> > that symbolizes the DMA engine indeed has a sink pad, but it's a video
> > node, not a subdev. It thus has no media bus format configured for its
> > sink pad. The closest pad in the pipeline that has a media bus format is
> > the source pad of the subdev connected to the video node.
> > 
> > There's no communication within the kernel at G/S_FMT time between the
> > video node and its connected subdev. The only time we look at the
> > pipeline as a whole is when starting the stream to validate that the
> > pipeline is correctly configured. We thus have to implement G/S_FMT on
> > the video node without any knowledge about the connected subdev, and thus
> > accept any colorspace.
> > 
> >> So any colorspace validation is done for the input pad. The question is
> >> what that validation should be. It's never been defined.
> > 
> > No format is set on the video node's entity sink pad for 

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Laurent Pinchart]

Hi Hans,

On Thursday, 22 February 2018 10:01:13 EET Hans Verkuil wrote:
> On 02/21/2018 10:01 PM, Sakari Ailus wrote:
> > On Wed, Feb 21, 2018 at 10:16:25PM +0200, Laurent Pinchart wrote:
> >> No, I'm sorry, for MC-based drivers this isn't correct. The media entity
> >> that symbolizes the DMA engine indeed has a sink pad, but it's a video
> >> node, not a subdev. It thus has no media bus format configured for its
> >> sink pad. The closest pad in the pipeline that has a media bus format is
> >> the source pad of the subdev connected to the video node.
> >> 
> >> There's no communication within the kernel at G/S_FMT time between the
> >> video node and its connected subdev. The only time we look at the
> >> pipeline as a whole is when starting the stream to validate that the
> >> pipeline is correctly configured. We thus have to implement G/S_FMT on
> >> the video node without any knowledge about the connected subdev, and
> >> thus accept any colorspace.
> > 
> > A few more notes related to this --- there's no propagation of sub-device
> > format across the entities; there were several reasons for the design
> > choice. The V4L2 pixel format in the video node must be compatible with
> > the sub-device format when streaming is started. And... the streaming will
> > start in the pipeline defined by the enabled links to/from the video node.
> > In principle nothign prevents having multiple links there, and at the time
> > S_FMT IOCTL is called on the video node, none of those links could be
> > enabled. And that's perfectly valid use of the API.
> > 
> > A lot of the DMA engine drivers are simply devices that receive data and
> > write that to system memory, they really don't necessarily know anything
> > else. For the hardware this data is just pixels (or even bits, especially
> > in the case of CSI-2!).
> 
> Not in my experience. Most DMA engines I've ever worked with can do at
> least quantization and RGB <-> YUV conversions. Which is pretty much
> required functionality if you work with video receivers.

That's because you have a coarser view of the hardware in the PC world, where 
the RGB <-> YUV converter and DMA engine are bundled in a single block in 
documentation. In the embedded world a DMA engine is just a DMA engine. There 
is in many cases a RGB <-> YUV converter just before the DMA engine, but it's 
then usually exposed as a subdevice separate from the video node.

> And in order to program that correctly in the DMA engine you have to
> know what you receive.

Depending on the hardware I'm fine letting driver authors decide whether to 
expose the RGB <-> YUV conversion as a separate subdev, or as part of the 
video node. In the latter case the video node implementation will need to 
program the RGB <-> YUV conversion hardware based on the output pixel format 
and input media bus format. This can be done without any issue at stream on 
time: the video node implementation can access the format of the connected 
source pad in the pipeline when starting the stream, as links are configured 
and frozen at that time. What the video node implementation can't do is access 
that information at G/S_FMT time, as G/S_FMT on both subdevs and video nodes 
must be contained to a single entity when using MC-based drivers.

> Full-fledged colorspace converters that can convert between different
> colorspaces and transfer functions are likely to be separate subdevs
> due to the complexity of that.
> 
> > So I agree with Laurent here that requiring correct colour space for
> > [GS]_FMT IOCTLs on video nodes in the general case is not feasible
> > (especially on MC-centric devices), due to the way the Media controller
> > pipeline and formats along that pipeline are configured and validated
> > (i.e. at streamon time).
> > 
> > But what to do here then? The colourspace field is not verified even in
> > link validation so there's no guarantee it's correctly set more or less
> > anywhere else than in the source of the stream. And if the stream has
> > multiple sources with different colour spaces, then what do you do? That's
> > perhaps a theoretical case but the current frameworks and APIs do in
> > principle support that.
> 
> It's not theoretical at all. But anyway, in that case it is up to userspace
> to decide. A typical example is an sRGB OSD on top of a Rec.709 video.
> 
> In practice the differences in color at too small to be a problem, you'd
> just use Rec. 709 and slight color differences in the sRGB OSD is not
> something that is noticeable. But with HDR and BT.2020 this becomes much
> more complicated.
> 
> However, that is out of scope of the kernel driver.

What I believe should be in scope for the kernel driver is reporting correct 
colorspace information. Once a pipeline is correctly configured for streaming, 
the colorspace reported by G_FMT on a capture video node should correspond to 
the colorspace of the data that will be written to memory. That's why we 
validate pipelines at stream on time: 

Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[jacopo mondi]

Hi Laurent,

On Thu, Feb 22, 2018 at 02:14:53PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Thursday, 22 February 2018 14:04:12 EET jacopo mondi wrote:
> > On Wed, Feb 21, 2018 at 10:28:06PM +0200, Laurent Pinchart wrote:
> > > On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:
> > > > On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
> > > > > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote:
> > > > >> The sensor driver sets mbus format colorspace information and sizes,
> > > > >> but not ycbcr encoding, quantization and xfer function. When supplied
> > > > >> with an badly initialized mbus frame format structure, those fields
> > > > >> need to be set explicitly not to leave them uninitialized. This is
> > > > >> tested by v4l2-compliance, which supplies a mbus format description
> > > > >> structure and checks for all fields to be properly set.
> > > > >>
> > > > >> Without this commit, v4l2-compliance fails when testing formats with:
> > > > >> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> > > > >>
> > > > >> Signed-off-by: Jacopo Mondi 
> > > > >> ---
> > > > >>
> > > > >>  drivers/media/i2c/ov7670.c | 4 
> > > > >>  1 file changed, 4 insertions(+)
> > > > >>
> > > > >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > > > >> index 25b26d4..61c472e 100644
> > > > >> --- a/drivers/media/i2c/ov7670.c
> > > > >> +++ b/drivers/media/i2c/ov7670.c
> > > > >> @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct
> > > > >> v4l2_subdev
> > > > >> *sd, fmt->height = wsize->height;
> > > > >>
> > > > >>  fmt->colorspace = ov7670_formats[index].colorspace;
> > > > >
> > > > > On a side note, if I'm not mistaken the colorspace field is set to
> > > > > SRGB
> > > > > for all entries. Shouldn't you hardcode it here and remove the field ?
> > > > >
> > > > >> +fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > > > >> +fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > > > >> +fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > > > >
> > > > > How about setting the values explicitly instead of relying on defaults
> > > > > ?
> > > > > That would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and
> > > > > V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see
> > > > > if
> > > > > the sensor outputs limited or full range ?
> > > >
> > > > This actually makes me wonder if those informations (ycbcb_enc,
> > > > quantization and xfer_func) shouldn't actually be part of the
> > > > supported format list... I blindly added those default fields in the
> > > > try_fmt function, but I doubt they applies to all supported formats.
> > > >
> > > > Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
> > > > RGB 565) and 1 raw format (BGGR). I now have a question here:
> > > >
> > > > 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
> > > > applies to RGB and raw formats? I don't think so, and what value is
> > > > the correct one for the ycbcr_enc field in this case? I assume
> > > > xfer_func and quantization applies to all formats instead..
> > >
> > > There's no encoding for RGB formats if I understand things correctly, so
> > > I'd set ycbcr_enc to V4L2_YCBCR_ENC_DEFAULT. The transfer function and
> > > the quantization apply to all formats, but I'd be surprised to find a
> > > sensor outputting limited range for RGB.
> >
> > Ack, we got the same understanding for RGB formats. I wonder if for
> > those formats we wouldn't need a V4L2_YCBCR_ENC_NONE or similar...
>
> That, or explicitly documenting that when the format is not YUV the field
> should be set by both drivers and applications to V4L2_YCBCR_ENC_DEFAULT when
> written and ignored when read.
>

Well, if no encoding is performed because the color encoding scheme is
RGB, the colorspace does anyway define an encoding method, so it seems
to me the latter is more appropriate (use DEFAULT and ignore when read).

That's anyway just my opinion, but I could send a patch for
documentation and see what feedback it gets.

> > > Have you been able to check whether the sensor outputs limited range of
> > > full range YUV ? If it outputs full range you can hardcode quantization
> > > to V4L2_QUANTIZATION_FULL_RANGE for all formats.
> >
> > In YUYV mode, I see values > 0xf0 ( > 240, which is the max value for
> > CbCr samples in limited quantization range), so I assume quantization
> > is full range.
>
> It should be, yes. What's the minimum and maximum values you get ?

>From a white surface:
min = 0x39
max = 0xfc

>From a black surface:
min = 0x00 (with 62 occurrences)
max = 0x8b

I guess that's indeed full range quantization

>
> > Actually the hardest part here was having a white enough surface to
> > point the sensor to :)
>
> Pointing a flashlight to the sensor usually does the trick.

I had a yellowish light that didn't work that well, I ended up putting
a white paper sheet in front of it and then took the 

Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt

[jacopo mondi]

Hi Laurent,

On Wed, Feb 21, 2018 at 10:28:06PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:
> > On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
> > > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote:
> > >> The sensor driver sets mbus format colorspace information and sizes,
> > >> but not ycbcr encoding, quantization and xfer function. When supplied
> > >> with an badly initialized mbus frame format structure, those fields
> > >> need to be set explicitly not to leave them uninitialized. This is
> > >> tested by v4l2-compliance, which supplies a mbus format description
> > >> structure and checks for all fields to be properly set.
> > >>
> > >> Without this commit, v4l2-compliance fails when testing formats with:
> > >> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
> > >>
> > >> Signed-off-by: Jacopo Mondi 
> > >> ---
> > >>
> > >>  drivers/media/i2c/ov7670.c | 4 
> > >>  1 file changed, 4 insertions(+)
> > >>
> > >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > >> index 25b26d4..61c472e 100644
> > >> --- a/drivers/media/i2c/ov7670.c
> > >> +++ b/drivers/media/i2c/ov7670.c
> > >> @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct
> > >> v4l2_subdev
> > >> *sd, fmt->height = wsize->height;
> > >>
> > >>  fmt->colorspace = ov7670_formats[index].colorspace;
> > >
> > > On a side note, if I'm not mistaken the colorspace field is set to SRGB
> > > for all entries. Shouldn't you hardcode it here and remove the field ?
> > >
> > >> +fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > >> +fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > >> +fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > >
> > > How about setting the values explicitly instead of relying on defaults ?
> > > That would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and
> > > V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if
> > > the sensor outputs limited or full range ?
> >
> > This actually makes me wonder if those informations (ycbcb_enc,
> > quantization and xfer_func) shouldn't actually be part of the
> > supported format list... I blindly added those default fields in the
> > try_fmt function, but I doubt they applies to all supported formats.
> >
> > Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and
> > RGB 565) and 1 raw format (BGGR). I now have a question here:
> >
> > 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this
> > applies to RGB and raw formats? I don't think so, and what value is
> > the correct one for the ycbcr_enc field in this case? I assume
> > xfer_func and quantization applies to all formats instead..
>
> There's no encoding for RGB formats if I understand things correctly, so I'd
> set ycbcr_enc to V4L2_YCBCR_ENC_DEFAULT. The transfer function and the
> quantization apply to all formats, but I'd be surprised to find a sensor
> outputting limited range for RGB.

Ack, we got the same understanding for RGB formats. I wonder if for
those formats we wouldn't need a V4L2_YCBCR_ENC_NONE or similar...

>
> Have you been able to check whether the sensor outputs limited range of full
> range YUV ? If it outputs full range you can hardcode quantization to
> V4L2_QUANTIZATION_FULL_RANGE for all formats.

In YUYV mode, I see values > 0xf0 ( > 240, which is the max value for
CbCr samples in limited quantization range), so I assume quantization
is full range.

Actually the hardest part here was having a white enough surface to
point the sensor to :)

Thanks
  j

>
> > >>  info->format = *fmt;
> > >>
> > >>  return 0;
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH v2 09/19] ARM64: dts: Add R-Car Salvator-x M3-N support

[Arnd Bergmann]

On Wed, Feb 21, 2018 at 6:15 PM, Simon Horman  wrote:
> On Tue, Feb 20, 2018 at 04:46:23PM +0100, Geert Uytterhoeven wrote:
>> On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi  
>> wrote:
>> > Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
>> >
>> > Based on original work from:
>> > Takeshi Kihara 
>> > Magnus Damm 
>> >
>> > Signed-off-by: Jacopo Mondi 
>>
>> Reviewed-by: Geert Uytterhoeven 
>
> Thanks, applied with prefix updated to
>
> "arm64: dts: renesas:"
>
> There was some fuzz applying this patch, the result is as follows.
>
> From: Jacopo Mondi 
> Subject: [PATCH] arm64: dts: renesas: Add R-Car Salvator-x M3-N support
>
> Add basic support for R-Car Salvator-X M3-N (R8A77965) board.
>
> Based on original work from:
> Takeshi Kihara 
> Magnus Damm 
>
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 

Something went wrong with this patch, I suspect it's missing some prerequisites
that fill in a few more device nodes to avoid these warnings in linux-next:

arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/ethernet@e680/ethernet-phy@0
has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e651/codec@10 has invalid
length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e651/clk_multiplier@4f
has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e66d8000/gpio@20 has invalid
length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e66d8000/adc@7c has invalid
length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e66d8000/adc@7f has invalid
length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e66d8000/clock-generator@6a
has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/i2c@e60b/pmic@30 has invalid
length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/display@feb0/ports/port@0 has
invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/display@feb0/ports/port@1 has
invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(reg_format): "reg" property in /soc/display@feb0/ports/port@2 has
invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #address-cells value for
/soc/ethernet@e680/ethernet-phy@0
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #size-cells value for
/soc/ethernet@e680/ethernet-phy@0
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #address-cells value for
/soc/i2c@e651/codec@10
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #size-cells value for
/soc/i2c@e651/codec@10
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #address-cells value for
/soc/i2c@e651/clk_multiplier@4f
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #size-cells value for
/soc/i2c@e651/clk_multiplier@4f
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #address-cells value for
/soc/i2c@e66d8000/gpio@20
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #size-cells value for
/soc/i2c@e66d8000/gpio@20
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default #address-cells value for
/soc/i2c@e66d8000/adc@7c
arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dtb: Warning
(avoid_default_addr_size): Relying on default 

Re: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

[Wolfram Sang]

> > Why? ePAPR says "okay", "disabled", "fail", or "fail-sss".
> > 
> > Sorry for missing this in the previous round.
> 
> That was per Wolfram's request, and because the existing code uses "ok". I'm 
> personally fine with any.

I did? Well, today I don't have a strong preference. Any is fine with
me, too.



signature.asc
Description: PGP signature

[PATCH v11 01/10] dt-bindings: media: Add Renesas CEU bindings

[Jacopo Mondi]

Add bindings documentation for Renesas Capture Engine Unit (CEU).

Signed-off-by: Jacopo Mondi 
Reviewed-by: Rob Herring 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 .../devicetree/bindings/media/renesas,ceu.txt  | 81 ++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt

diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt 
b/Documentation/devicetree/bindings/media/renesas,ceu.txt
new file mode 100644
index 000..3fc66df
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
@@ -0,0 +1,81 @@
+Renesas Capture Engine Unit (CEU)
+--
+
+The Capture Engine Unit is the image capture interface found in the Renesas
+SH Mobile and RZ SoCs.
+
+The interface supports a single parallel input with data bus width of 8 or 16
+bits.
+
+Required properties:
+- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1H
+  and RZ/A1M SoCs.
+- reg: Registers address base and size.
+- interrupts: The interrupt specifier.
+
+The CEU supports a single parallel input and should contain a single 'port'
+subnode with a single 'endpoint'. Connection to input devices are modeled
+according to the video interfaces OF bindings specified in:
+Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Optional endpoint properties applicable to parallel input bus described in
+the above mentioned "video-interfaces.txt" file are supported.
+
+- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH 
respectively.
+  If property is not present, default is active high.
+- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH 
respectively.
+  If property is not present, default is active high.
+
+Example:
+
+The example describes the connection between the Capture Engine Unit and an
+OV7670 image sensor connected to i2c1 interface.
+
+ceu: ceu@e821 {
+   reg = <0xe821 0x209c>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   status = "okay";
+
+   port {
+   ceu_in: endpoint {
+   remote-endpoint = <_out>;
+
+   hsync-active = <1>;
+   vsync-active = <0>;
+   };
+   };
+};
+
+i2c1: i2c@fcfee400 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   status = "okay";
+
+   clock-frequency = <10>;
+
+   ov7670: camera@21 {
+   compatible = "ovti,ov7670";
+   reg = <0x21>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   reset-gpios = < 11 GPIO_ACTIVE_LOW>;
+   powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov7670_out: endpoint {
+   remote-endpoint = <_in>;
+
+   hsync-active = <1>;
+   vsync-active = <0>;
+   };
+   };
+   };
+};
-- 
2.7.4

[PATCH v11 02/10] include: media: Add Renesas CEU driver interface

[Jacopo Mondi]

Add renesas-ceu header file.

Do not remove the existing sh_mobile_ceu.h one as long as the original
driver does not go away.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 include/media/drv-intf/renesas-ceu.h | 26 ++
 1 file changed, 26 insertions(+)
 create mode 100644 include/media/drv-intf/renesas-ceu.h

diff --git a/include/media/drv-intf/renesas-ceu.h 
b/include/media/drv-intf/renesas-ceu.h
new file mode 100644
index 000..52841d1
--- /dev/null
+++ b/include/media/drv-intf/renesas-ceu.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * renesas-ceu.h - Renesas CEU driver interface
+ *
+ * Copyright 2017-2018 Jacopo Mondi 
+ */
+
+#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__
+#define __MEDIA_DRV_INTF_RENESAS_CEU_H__
+
+#define CEU_MAX_SUBDEVS2
+
+struct ceu_async_subdev {
+   unsigned long flags;
+   unsigned char bus_width;
+   unsigned char bus_shift;
+   unsigned int i2c_adapter_id;
+   unsigned int i2c_address;
+};
+
+struct ceu_platform_data {
+   unsigned int num_subdevs;
+   struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS];
+};
+
+#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */
-- 
2.7.4

[PATCH v11 03/10] media: platform: Add Renesas CEU driver

[Jacopo Mondi]

Add driver for Renesas Capture Engine Unit (CEU).

The CEU interface supports capturing 'data' (YUV422) and 'images'
(NV[12|21|16|61]).

This driver aims to replace the soc_camera-based sh_mobile_ceu one.

Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
platform GR-Peach.

Tested with ov7725 camera sensor on SH4 platform Migo-R.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/platform/Kconfig   |9 +
 drivers/media/platform/Makefile  |1 +
 drivers/media/platform/renesas-ceu.c | 1675 ++
 3 files changed, 1685 insertions(+)
 create mode 100644 drivers/media/platform/renesas-ceu.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 614fbef..f9cc058 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
  To compile this driver as a module, choose M here: the module
  will be called stm32-dcmi.
 
+config VIDEO_RENESAS_CEU
+   tristate "Renesas Capture Engine Unit (CEU) driver"
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_FWNODE
+   ---help---
+ This is a v4l2 driver for the Renesas CEU Interface
+
 source "drivers/media/platform/soc_camera/Kconfig"
 source "drivers/media/platform/exynos4-is/Kconfig"
 source "drivers/media/platform/am437x/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 7f30804..85e1121 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o
 obj-$(CONFIG_SOC_CAMERA)   += soc_camera/
 
 obj-$(CONFIG_VIDEO_RCAR_DRIF)  += rcar_drif.o
+obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o
 obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1)   += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
diff --git a/drivers/media/platform/renesas-ceu.c 
b/drivers/media/platform/renesas-ceu.c
new file mode 100644
index 000..cfabe1a
--- /dev/null
+++ b/drivers/media/platform/renesas-ceu.c
@@ -0,0 +1,1675 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface
+ * Copyright (C) 2017-2018 Jacopo Mondi 
+ *
+ * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c"
+ * Copyright (C) 2008 Magnus Damm
+ *
+ * Based on V4L2 Driver for PXA camera host - "pxa_camera.c",
+ * Copyright (C) 2006, Sascha Hauer, Pengutronix
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define DRIVER_NAME"renesas-ceu"
+
+/* CEU registers offsets and masks. */
+#define CEU_CAPSR  0x00 /* Capture start register  */
+#define CEU_CAPCR  0x04 /* Capture control register*/
+#define CEU_CAMCR  0x08 /* Capture interface control register  */
+#define CEU_CAMOR  0x10 /* Capture interface offset register   */
+#define CEU_CAPWR  0x14 /* Capture interface width register*/
+#define CEU_CAIFR  0x18 /* Capture interface input format register */
+#define CEU_CRCNTR 0x28 /* CEU register control register   */
+#define CEU_CRCMPR 0x2c /* CEU register forcible control register  */
+#define CEU_CFLCR  0x30 /* Capture filter control register */
+#define CEU_CFSZR  0x34 /* Capture filter size clip register   */
+#define CEU_CDWDR  0x38 /* Capture destination width register  */
+#define CEU_CDAYR  0x3c /* Capture data address Y register */
+#define CEU_CDACR  0x40 /* Capture data address C register */
+#define CEU_CFWCR  0x5c /* Firewall operation control register */
+#define CEU_CDOCR  0x64 /* Capture data output control register*/
+#define CEU_CEIER  0x70 /* Capture event interrupt enable register */
+#define CEU_CETCR  0x74 /* Capture event flag clear register   */
+#define CEU_CSTSR  0x7c /* Capture status register */
+#define CEU_CSRTR  0x80 /* Capture software reset register */
+
+/* Data synchronous fetch mode. */
+#define CEU_CAMCR_JPEG BIT(4)
+
+/* Input components ordering: CEU_CAMCR.DTARY field. */
+#define CEU_CAMCR_DTARY_8_UYVY (0x00 << 8)
+#define CEU_CAMCR_DTARY_8_VYUY (0x01 << 8)
+#define 

[PATCH v11 05/10] media: i2c: Copy ov772x soc_camera sensor driver

[Jacopo Mondi]

Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.

Signed-off-by: Jacopo Mondi 
Acked-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/ov772x.c | 1124 
 1 file changed, 1124 insertions(+)
 create mode 100644 drivers/media/i2c/ov772x.c

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
new file mode 100644
index 000..8063835
--- /dev/null
+++ b/drivers/media/i2c/ov772x.c
@@ -0,0 +1,1124 @@
+/*
+ * ov772x Camera Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto 
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet 
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * register offset
+ */
+#define GAIN0x00 /* AGC - Gain control gain setting */
+#define BLUE0x01 /* AWB - Blue channel gain setting */
+#define RED 0x02 /* AWB - Red   channel gain setting */
+#define GREEN   0x03 /* AWB - Green channel gain setting */
+#define COM10x04 /* Common control 1 */
+#define BAVG0x05 /* U/B Average Level */
+#define GAVG0x06 /* Y/Gb Average Level */
+#define RAVG0x07 /* V/R Average Level */
+#define AECH0x08 /* Exposure Value - AEC MSBs */
+#define COM20x09 /* Common control 2 */
+#define PID 0x0A /* Product ID Number MSB */
+#define VER 0x0B /* Product ID Number LSB */
+#define COM30x0C /* Common control 3 */
+#define COM40x0D /* Common control 4 */
+#define COM50x0E /* Common control 5 */
+#define COM60x0F /* Common control 6 */
+#define AEC 0x10 /* Exposure Value */
+#define CLKRC   0x11 /* Internal clock */
+#define COM70x12 /* Common control 7 */
+#define COM80x13 /* Common control 8 */
+#define COM90x14 /* Common control 9 */
+#define COM10   0x15 /* Common control 10 */
+#define REG16   0x16 /* Register 16 */
+#define HSTART  0x17 /* Horizontal sensor size */
+#define HSIZE   0x18 /* Horizontal frame (HREF column) end high 8-bit */
+#define VSTART  0x19 /* Vertical frame (row) start high 8-bit */
+#define VSIZE   0x1A /* Vertical sensor size */
+#define PSHFT   0x1B /* Data format - pixel delay select */
+#define MIDH0x1C /* Manufacturer ID byte - high */
+#define MIDL0x1D /* Manufacturer ID byte - low  */
+#define LAEC0x1F /* Fine AEC value */
+#define COM11   0x20 /* Common control 11 */
+#define BDBASE  0x22 /* Banding filter Minimum AEC value */
+#define DBSTEP  0x23 /* Banding filter Maximum Setp */
+#define AEW 0x24 /* AGC/AEC - Stable operating region (upper limit) */
+#define AEB 0x25 /* AGC/AEC - Stable operating region (lower limit) */
+#define VPT 0x26 /* AGC/AEC Fast mode operating region */
+#define REG28   0x28 /* Register 28 */
+#define HOUTSIZE0x29 /* Horizontal data output size MSBs */
+#define EXHCH   0x2A /* Dummy pixel insert MSB */
+#define EXHCL   0x2B /* Dummy pixel insert LSB */
+#define VOUTSIZE0x2C /* Vertical data output size MSBs */
+#define ADVFL   0x2D /* LSB of insert dummy lines in Vertical direction */
+#define ADVFH   0x2E /* MSG of insert dummy lines in Vertical direction */
+#define YAVE0x2F /* Y/G Channel Average value */
+#define LUMHTH  0x30 /* Histogram AEC/AGC Luminance high level threshold */
+#define LUMLTH  0x31 /* Histogram AEC/AGC Luminance low  level threshold */
+#define HREF0x32 /* Image start and size control */
+#define DM_LNL  0x33 /* Dummy line low  8 bits */
+#define DM_LNH  0x34 /* Dummy line high 8 bits */
+#define ADOFF_B 0x35 /* AD offset compensation value for B  channel */
+#define ADOFF_R 0x36 /* AD offset compensation value for R  channel */
+#define ADOFF_GB0x37 /* AD offset compensation value for Gb channel */
+#define ADOFF_GR0x38 /* AD offset compensation value for Gr channel */
+#define OFF_B   0x39 /* Analog process B  channel offset value */
+#define OFF_R   0x3A /* Analog process R  channel offset value */
+#define OFF_GB  0x3B /* Analog process Gb channel offset value */
+#define OFF_GR  0x3C /* Analog process Gr 

[PATCH v11 04/10] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)

[Jacopo Mondi]

Add Capture Engine Unit (CEU) node to device tree.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Geert Uytterhoeven 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 arch/arm/boot/dts/r7s72100.dtsi | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ab9645a..23e05ce 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -135,9 +135,9 @@
#clock-cells = <1>;
compatible = "renesas,r7s72100-mstp-clocks", 
"renesas,cpg-mstp-clocks";
reg = <0xfcfe042c 4>;
-   clocks = <_clk>;
-   clock-indices = ;
-   clock-output-names = "rtc";
+   clocks = <_clk>, <_clk>;
+   clock-indices = ;
+   clock-output-names = "ceu", "rtc";
};
 
mstp7_clks: mstp7_clks@fcfe0430 {
@@ -667,4 +667,13 @@
power-domains = <_clocks>;
status = "disabled";
};
+
+   ceu: camera@e821 {
+   reg = <0xe821 0x3000>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_CEU>;
+   power-domains = <_clocks>;
+   status = "disabled";
+   };
 };
-- 
2.7.4

[PATCH v11 06/10] media: i2c: ov772x: Remove soc_camera dependencies

[Jacopo Mondi]

Remove soc_camera framework dependencies from ov772x sensor driver.
- Handle clock and gpios
- Register async subdevice
- Remove soc_camera specific g/s_mbus_config operations
- Change image format colorspace from JPEG to SRGB as the two use the
  same colorspace information but JPEG makes assumptions on color
  components quantization that do not apply to the sensor
- Remove sizes crop from get_selection as driver can't scale
- Add kernel doc to driver interface header file
- Adjust build system

This commit does not remove the original soc_camera based driver as long
as other platforms depends on soc_camera-based CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/i2c/Kconfig  |  11 +++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov772x.c | 172 ++---
 include/media/i2c/ov772x.h |   6 +-
 4 files changed, 133 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd2..e71e968 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -645,6 +645,17 @@ config VIDEO_OV5670
  To compile this driver as a module, choose M here: the
  module will be called ov5670.
 
+config VIDEO_OV772X
+   tristate "OmniVision OV772x sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV772x camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov772x.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd..b0489a1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
+obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
 obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
 obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 8063835..23106d7 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1,6 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * ov772x Camera Driver
  *
+ * Copyright (C) 2017 Jacopo Mondi 
+ *
  * Copyright (C) 2008 Renesas Solutions Corp.
  * Kuninori Morimoto 
  *
@@ -9,27 +12,25 @@
  * Copyright 2006-7 Jonathan Corbet 
  * Copyright (C) 2008 Magnus Damm
  * Copyright (C) 2008, Guennadi Liakhovetski 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
 #include 
 #include 
 
 #include 
-#include 
-#include 
+
 #include 
-#include 
+#include 
 #include 
+#include 
 
 /*
  * register offset
@@ -393,8 +394,10 @@ struct ov772x_win_size {
 struct ov772x_priv {
struct v4l2_subdevsubdev;
struct v4l2_ctrl_handler  hdl;
-   struct v4l2_clk  *clk;
+   struct clk   *clk;
struct ov772x_camera_info*info;
+   struct gpio_desc *pwdn_gpio;
+   struct gpio_desc *rstb_gpio;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
unsigned shortflag_vflip:1;
@@ -409,7 +412,7 @@ struct ov772x_priv {
 static const struct ov772x_color_format ov772x_cfmts[] = {
{
.code   = MEDIA_BUS_FMT_YUYV8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = V4L2_COLORSPACE_SRGB,
.dsp3   = 0x0,
.dsp4   = DSP_OFMT_YUV,
.com3   = SWAP_YUV,
@@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
{
.code   = MEDIA_BUS_FMT_YVYU8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = V4L2_COLORSPACE_SRGB,
.dsp3   = UV_ON,
.dsp4   = DSP_OFMT_YUV,
.com3   = SWAP_YUV,
@@ -425,7 +428,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
{
.code   = MEDIA_BUS_FMT_UYVY8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = 

[PATCH v11 08/10] media: i2c: Copy tw9910 soc_camera sensor driver

[Jacopo Mondi]

Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.

Signed-off-by: Jacopo Mondi 
Acked-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/tw9910.c | 999 +
 1 file changed, 999 insertions(+)
 create mode 100644 drivers/media/i2c/tw9910.c

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
new file mode 100644
index 000..bdb5e0a
--- /dev/null
+++ b/drivers/media/i2c/tw9910.c
@@ -0,0 +1,999 @@
+/*
+ * tw9910 Video Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto 
+ *
+ * Based on ov772x driver,
+ *
+ * Copyright (C) 2008 Kuninori Morimoto 
+ * Copyright 2006-7 Jonathan Corbet 
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define GET_ID(val)  ((val & 0xF8) >> 3)
+#define GET_REV(val) (val & 0x07)
+
+/*
+ * register offset
+ */
+#define ID 0x00 /* Product ID Code Register */
+#define STATUS10x01 /* Chip Status Register I */
+#define INFORM 0x02 /* Input Format */
+#define OPFORM 0x03 /* Output Format Control Register */
+#define DLYCTR 0x04 /* Hysteresis and HSYNC Delay Control */
+#define OUTCTR10x05 /* Output Control I */
+#define ACNTL1 0x06 /* Analog Control Register 1 */
+#define CROP_HI0x07 /* Cropping Register, High */
+#define VDELAY_LO  0x08 /* Vertical Delay Register, Low */
+#define VACTIVE_LO 0x09 /* Vertical Active Register, Low */
+#define HDELAY_LO  0x0A /* Horizontal Delay Register, Low */
+#define HACTIVE_LO 0x0B /* Horizontal Active Register, Low */
+#define CNTRL1 0x0C /* Control Register I */
+#define VSCALE_LO  0x0D /* Vertical Scaling Register, Low */
+#define SCALE_HI   0x0E /* Scaling Register, High */
+#define HSCALE_LO  0x0F /* Horizontal Scaling Register, Low */
+#define BRIGHT 0x10 /* BRIGHTNESS Control Register */
+#define CONTRAST   0x11 /* CONTRAST Control Register */
+#define SHARPNESS  0x12 /* SHARPNESS Control Register I */
+#define SAT_U  0x13 /* Chroma (U) Gain Register */
+#define SAT_V  0x14 /* Chroma (V) Gain Register */
+#define HUE0x15 /* Hue Control Register */
+#define CORING10x17
+#define CORING20x18 /* Coring and IF compensation */
+#define VBICNTL0x19 /* VBI Control Register */
+#define ACNTL2 0x1A /* Analog Control 2 */
+#define OUTCTR20x1B /* Output Control 2 */
+#define SDT0x1C /* Standard Selection */
+#define SDTR   0x1D /* Standard Recognition */
+#define TEST   0x1F /* Test Control Register */
+#define CLMPG  0x20 /* Clamping Gain */
+#define IAGC   0x21 /* Individual AGC Gain */
+#define AGCGAIN0x22 /* AGC Gain */
+#define PEAKWT 0x23 /* White Peak Threshold */
+#define CLMPL  0x24 /* Clamp level */
+#define SYNCT  0x25 /* Sync Amplitude */
+#define MISSCNT0x26 /* Sync Miss Count Register */
+#define PCLAMP 0x27 /* Clamp Position Register */
+#define VCNTL1 0x28 /* Vertical Control I */
+#define VCNTL2 0x29 /* Vertical Control II */
+#define CKILL  0x2A /* Color Killer Level Control */
+#define COMB   0x2B /* Comb Filter Control */
+#define LDLY   0x2C /* Luma Delay and H Filter Control */
+#define MISC1  0x2D /* Miscellaneous Control I */
+#define LOOP   0x2E /* LOOP Control Register */
+#define MISC2  0x2F /* Miscellaneous Control II */
+#define MVSN   0x30 /* Macrovision Detection */
+#define STATUS20x31 /* Chip STATUS II */
+#define HFREF  0x32 /* H monitor */
+#define CLMD   0x33 /* CLAMP MODE */
+#define IDCNTL 0x34 /* ID Detection Control */
+#define CLCNTL10x35 /* Clamp Control I */
+#define ANAPLLCTL  0x4C
+#define VBIMIN 0x4D
+#define HSLOWCTL   0x4E
+#define WSS3   0x4F
+#define FILLDATA   0x50
+#define SDID   0x51
+#define DID0x52
+#define WSS1   0x53
+#define WSS2   0x54
+#define VVBI   0x55
+#define LCTL6  0x56
+#define LCTL7  

[PATCH v11 07/10] media: i2c: ov772x: Support frame interval handling

[Jacopo Mondi]

Add support to ov772x driver for frame intervals handling and enumeration.
Tested with 10MHz and 24MHz input clock at VGA and QVGA resolutions for
10, 15 and 30 frame per second rates.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/ov772x.c | 213 +
 1 file changed, 196 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 23106d7..3edf0cb 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -250,6 +250,7 @@
 #define AEC_1p2 0x10   /*  01: 1/2  window */
 #define AEC_1p4 0x20   /*  10: 1/4  window */
 #define AEC_2p3 0x30   /*  11: Low 2/3 window */
+#define COM4_RESERVED   0x01   /* Reserved bit */
 
 /* COM5 */
 #define AFR_ON_OFF  0x80   /* Auto frame rate control ON/OFF selection */
@@ -267,6 +268,10 @@
/* AEC max step control */
 #define AEC_NO_LIMIT0x01   /*   0 : AEC incease step has limit */
/*   1 : No limit to AEC increase step */
+/* CLKRC */
+   /* Input clock divider register */
+#define CLKRC_RESERVED  0x80   /* Reserved bit */
+#define CLKRC_DIV(n)((n) - 1)
 
 /* COM7 */
/* SCCB Register Reset */
@@ -373,6 +378,19 @@
 #define VERSION(pid, ver) ((pid<<8)|(ver&0xFF))
 
 /*
+ * PLL multipliers
+ */
+static struct {
+   unsigned int mult;
+   u8 com4;
+} ov772x_pll[] = {
+   { 1, PLL_BYPASS, },
+   { 4, PLL_4x, },
+   { 6, PLL_6x, },
+   { 8, PLL_8x, },
+};
+
+/*
  * struct
  */
 
@@ -388,6 +406,7 @@ struct ov772x_color_format {
 struct ov772x_win_size {
char *name;
unsigned char com7_bit;
+   unsigned int  sizeimage;
struct v4l2_rect  rect;
 };
 
@@ -404,6 +423,7 @@ struct ov772x_priv {
unsigned shortflag_hflip:1;
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
+   unsigned int  fps;
 };
 
 /*
@@ -487,27 +507,34 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 
 static const struct ov772x_win_size ov772x_win_sizes[] = {
{
-   .name = "VGA",
-   .com7_bit = SLCT_VGA,
+   .name   = "VGA",
+   .com7_bit   = SLCT_VGA,
+   .sizeimage  = 510 * 748,
.rect = {
-   .left = 140,
-   .top = 14,
-   .width = VGA_WIDTH,
-   .height = VGA_HEIGHT,
+   .left   = 140,
+   .top= 14,
+   .width  = VGA_WIDTH,
+   .height = VGA_HEIGHT,
},
}, {
-   .name = "QVGA",
-   .com7_bit = SLCT_QVGA,
+   .name   = "QVGA",
+   .com7_bit   = SLCT_QVGA,
+   .sizeimage  = 278 * 576,
.rect = {
-   .left = 252,
-   .top = 6,
-   .width = QVGA_WIDTH,
-   .height = QVGA_HEIGHT,
+   .left   = 252,
+   .top= 6,
+   .width  = QVGA_WIDTH,
+   .height = QVGA_HEIGHT,
},
},
 };
 
 /*
+ * frame rate settings lists
+ */
+static unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 };
+
+/*
  * general function
  */
 
@@ -574,6 +601,128 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }
 
+static int ov772x_set_frame_rate(struct ov772x_priv *priv,
+struct v4l2_fract *tpf,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+   unsigned long fin = clk_get_rate(priv->clk);
+   unsigned int fps = tpf->numerator ?
+  tpf->denominator / tpf->numerator :
+  tpf->denominator;
+   unsigned int best_diff;
+   unsigned int fsize;
+   unsigned int pclk;
+   unsigned int diff;
+   unsigned int idx;
+   unsigned int i;
+   u8 clkrc = 0;
+   u8 com4 = 0;
+   int ret;
+
+   /* Approximate to the closest supported frame interval. */
+   best_diff = ~0L;
+   for (i = 0, idx = 0; i < ARRAY_SIZE(ov772x_frame_intervals); i++) {
+   diff = abs(fps - ov772x_frame_intervals[i]);
+   if (diff < best_diff) {
+   idx = i;
+   best_diff = diff;
+   }

[PATCH v11 10/10] arch: sh: migor: Use new renesas-ceu camera driver

[Jacopo Mondi]

Migo-R platform uses sh_mobile_ceu camera driver, which is now being
replaced by a proper V4L2 camera driver named 'renesas-ceu'.

Move Migo-R platform to use the v4l2 renesas-ceu camera driver
interface and get rid of soc_camera defined components used to register
sensor drivers and of platform specific enable/disable routines.

Register clock source and GPIOs for sensor drivers, so they can use
clock and gpio APIs.

Also, memory for CEU video buffers is now reserved with membocks APIs,
and need to be declared as dma_coherent during machine initialization to
remove that architecture specific part from CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 arch/sh/boards/mach-migor/setup.c  | 225 +++--
 arch/sh/kernel/cpu/sh4a/clock-sh7722.c |   2 +-
 2 files changed, 101 insertions(+), 126 deletions(-)

diff --git a/arch/sh/boards/mach-migor/setup.c 
b/arch/sh/boards/mach-migor/setup.c
index 0bcbe58..271dfc2 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -1,17 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Renesas System Solutions Asia Pte. Ltd - Migo-R
  *
  * Copyright (C) 2008 Magnus Damm
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
  */
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -23,10 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -45,6 +45,9 @@
  * 0x1800   8GB8   NAND Flash (K9K8G08U0A)
  */
 
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu_dma_membase;
+
 static struct smc91x_platdata smc91x_info = {
.flags = SMC91X_USE_16BIT | SMC91X_NOWAIT,
 };
@@ -301,65 +304,24 @@ static struct platform_device migor_lcdc_device = {
},
 };
 
-static struct clk *camera_clk;
-static DEFINE_MUTEX(camera_lock);
-
-static void camera_power_on(int is_tw)
-{
-   mutex_lock(_lock);
-
-   /* Use 10 MHz VIO_CKO instead of 24 MHz to work
-* around signal quality issues on Panel Board V2.1.
-*/
-   camera_clk = clk_get(NULL, "video_clk");
-   clk_set_rate(camera_clk, 1000);
-   clk_enable(camera_clk); /* start VIO_CKO */
-
-   /* use VIO_RST to take camera out of reset */
-   mdelay(10);
-   if (is_tw) {
-   gpio_set_value(GPIO_PTT2, 0);
-   gpio_set_value(GPIO_PTT0, 0);
-   } else {
-   gpio_set_value(GPIO_PTT0, 1);
-   }
-   gpio_set_value(GPIO_PTT3, 0);
-   mdelay(10);
-   gpio_set_value(GPIO_PTT3, 1);
-   mdelay(10); /* wait to let chip come out of reset */
-}
-
-static void camera_power_off(void)
-{
-   clk_disable(camera_clk); /* stop VIO_CKO */
-   clk_put(camera_clk);
-
-   gpio_set_value(GPIO_PTT3, 0);
-   mutex_unlock(_lock);
-}
-
-static int ov7725_power(struct device *dev, int mode)
-{
-   if (mode)
-   camera_power_on(0);
-   else
-   camera_power_off();
-
-   return 0;
-}
-
-static int tw9910_power(struct device *dev, int mode)
-{
-   if (mode)
-   camera_power_on(1);
-   else
-   camera_power_off();
-
-   return 0;
-}
-
-static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
-   .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu_pdata = {
+   .num_subdevs= 2,
+   .subdevs = {
+   { /* [0] = ov772x */
+   .flags  = 0,
+   .bus_width  = 8,
+   .bus_shift  = 0,
+   .i2c_adapter_id = 0,
+   .i2c_address= 0x21,
+   },
+   { /* [1] = tw9910 */
+   .flags  = 0,
+   .bus_width  = 8,
+   .bus_shift  = 0,
+   .i2c_adapter_id = 0,
+   .i2c_address= 0x45,
+   },
+   },
 };
 
 static struct resource migor_ceu_resources[] = {
@@ -373,18 +335,32 @@ static struct resource migor_ceu_resources[] = {
.start  = evt2irq(0x880),
.flags  = IORESOURCE_IRQ,
},
-   [2] = {
-   /* place holder for contiguous memory */
-   },
 };
 
 static struct platform_device migor_ceu_device = {
-   .name   = "sh_mobile_ceu",
-   .id = 0, /* "ceu0" clock */
+   .name   = "renesas-ceu",
+   .id = 0, /* ceu.0 */
.num_resources  = ARRAY_SIZE(migor_ceu_resources),
.resource   = migor_ceu_resources,
.dev= {
-   

Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver

[Laurent Pinchart]

Hi Frank,

On Thursday, 22 February 2018 08:07:14 EET Frank Rowand wrote:
> On 02/20/18 15:10, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series addresses a design mistake that dates back from the
> > initial DU support. Support for the LVDS encoders, which are IP cores
> > separate from the DU, was bundled in the DU driver. Worse, both the DU
> > and LVDS were described through a single DT node.
> > 
> > To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS
> > encoders, and deprecate their description inside the DU bindings. To
> > retain backward compatibility with existing DT, patches 03/16 to 08/16
> > then patch the device tree at runtime to convert the legacy bindings to
> > the new ones.
> > 
> > With the DT side addressed, patch 09/16 converts the LVDS support code to
> > a separate bridge driver. Patches 11/16 to 16/16 then update all the
> > device tree sources to the new DU and LVDS encoders bindings.
> > 
> > I decided to go for live DT patching in patch 08/16 because implementing
> > support for both the legacy and new bindings in the driver would have been
> > very intrusive, and prevented further cleanups. This version relies more
> > heavily on overlays to avoid touching the internals of the OF core
> > compared to v2, even if manual fixes to the device tree are still needed.
> > 
> > Compared to v3, this series uses the OF changeset API to update properties
> > instead of accessing the internals of the property structure. This removes
> > the local implementation of functions to look up nodes by path and update
> > properties. In order to do this, I pulled in Pantelis' patch series
> > titled "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's
> > request, and rebased it while taking two small review comments into
> > account.
> 
> Wait a minute!  Why are you putting a patch set to modify core devicetree
> in the middle of a driver series.  Please pull it out to a separate series.

Because Rob asked for the driver-local implementation of the property add 
function to be replaced by Pantelis' series. I want to get the LVDS changes in 
v4.17 and asked Rob whether I could then take the OF changeset patches merged 
through the DRM tree, and he didn't object. If that causes an issue I'll 
switch back to the driver-local implementation to get the driver changes 
merged, split the OF changeset series out, and then move to the OF changeset 
API once merged. Would you prefer that ?

> I'll try to look at the patches, as they are in this series, sometime
> tomorrow.  I have a vague memory of unresolved issues from the last
> time they were proposed.
> 
> > Rob, I'd like this series to be merged in v4.17. As the changeset helpers
> > are now a dependency, I'd need you to merge them early (ideally on top of
> > v4.16-rc1) and provide a stable branch, or get your ack to merge them
> > through Dave's tree if they don't conflict with what you have and will
> > queue for v4.17.
> > 
> > This version also drops the small fix to the Porter board device tree that
> > has been queued for v4.17 already.
> > 
> > Compared to v2, the biggest change is in patch 03/16. Following Rob's and
> > Frank's reviews it was clear that modifying the unflattened DT structure
> > of the overlay before applying it wasn't popular. I have thus decided to
> > use one overlay source per SoC to move as much of the DT changes to the
> > overlay as possible, and only perform manual modifications (that are
> > still needed as some of the information is board-specific) on the system
> > DT after applying the overlay. As a result the overlay is parsed and
> > applied without being modified.
> > 
> > Compared to v1, this series update the r8a7792 and r8a7794 device tree
> > sources and incorporate review feedback as described by the changelogs of
> > individual patches.
> > 
> > Laurent Pinchart (11):
> >   dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
> >   dt-bindings: display: renesas: Deprecate LVDS support in the DU
> > bindings
> >   drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
> >   drm: rcar-du: Convert LVDS encoder code to bridge driver
> >   ARM: dts: r8a7790: Convert to new LVDS DT bindings
> >   ARM: dts: r8a7791: Convert to new LVDS DT bindings
> >   ARM: dts: r8a7792: Convert to new DU DT bindings
> >   ARM: dts: r8a7793: Convert to new LVDS DT bindings
> >   ARM: dts: r8a7794: Convert to new DU DT bindings
> >   arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings
> >   arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings
> > 
> > Pantelis Antoniou (5):
> >   of: dynamic: Add __of_node_dupv()
> >   of: changesets: Introduce changeset helper methods
> >   of: changeset: Add of_changeset_node_move method
> >   of: unittest: changeset helpers
> >   i2c: demux: Use changeset helpers for clarity
> >  
> >  .../bindings/display/bridge/renesas,lvds.txt   |  56 +++
> >  .../devicetree/bindings/display/renesas,du.txt |  31 +-

Re: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

[Laurent Pinchart]

Hi Geert,

On Thursday, 22 February 2018 11:26:44 EET Geert Uytterhoeven wrote:
> On Thu, Feb 22, 2018 at 1:05 AM, Laurent Pinchart wrote:
> > From: Pantelis Antoniou 
> > 
> > The changeset helpers are easier to use, use them instead of
> > using the static property.
> > 
> > Signed-off-by: Pantelis Antoniou 
> > Acked-by: Wolfram Sang 
> > ["okay" -> "ok"]
> 
> Why? ePAPR says "okay", "disabled", "fail", or "fail-sss".
> 
> Sorry for missing this in the previous round.

That was per Wolfram's request, and because the existing code uses "ok". I'm 
personally fine with any.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v10 03/10] media: platform: Add Renesas CEU driver

[Hans Verkuil]

On 02/21/18 18:47, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
> 
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
> 
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> 
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
> 
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

I get these warnings when I try to compile this driver:


  CC [M]  drivers/media/platform/renesas-ceu.o
drivers/media/platform/renesas-ceu.c: In function ‘ceu_start_streaming’:
drivers/media/platform/renesas-ceu.c:290:2: warning: ‘cdwdr’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  iowrite32(data, priv->base + reg_offs);
  ^~
drivers/media/platform/renesas-ceu.c:338:27: note: ‘cdwdr’ was declared here
  u32 camcr, cdocr, cfzsr, cdwdr, capwr;
   ^
drivers/media/platform/renesas-ceu.c:290:2: warning: ‘cfzsr’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  iowrite32(data, priv->base + reg_offs);
  ^~
drivers/media/platform/renesas-ceu.c:338:20: note: ‘cfzsr’ was declared here
  u32 camcr, cdocr, cfzsr, cdwdr, capwr;
^
drivers/media/platform/renesas-ceu.c:418:8: warning: ‘camcr’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  camcr |= mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
  ~~^~~
drivers/media/platform/renesas-ceu.c:338:6: note: ‘camcr’ was declared here
  u32 camcr, cdocr, cfzsr, cdwdr, capwr;
  ^
drivers/media/platform/renesas-ceu.c: In function ‘ceu_probe’:
drivers/media/platform/renesas-ceu.c:1632:9: warning: ‘ret’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
  return ret;
 ^~~
cc1: some warnings being treated as errors

The last warning is indeed correct.

The others are only right if pixelformat is illegal, which can't happen.
I'd add a:

default:
return -EINVAL;

to the switch, this shuts up the warnings.

So I need a v11 (just for this patch) after all.

Regards,

Hans

[PATCH 3/3] MAINTAINERS: Add entry for Techwell TW9910

[Jacopo Mondi]

Add entry for Techwell TW9910 video decoder. The driver is currently
orphaned.

Signed-off-by: Jacopo Mondi 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 64b8cd4..da1a88d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13391,6 +13391,12 @@ L: linux-me...@vger.kernel.org
 S: Maintained
 F: drivers/media/rc/ttusbir.c
 
+TECHWELL TW9910 VIDEO DECODER
+L: linux-me...@vger.kernel.org
+S: Orphan
+F: drivers/media/i2c/tw9910.c
+F: include/media/i2c/tw9910.h
+
 TEE SUBSYSTEM
 M: Jens Wiklander 
 S: Maintained
-- 
2.7.4

[PATCH 0/3] Update MAINTAINERS file preparing for CEU inclusion

[Jacopo Mondi]

Hi Hans,
   this 3 patches update MAINTAINERS in preparation for CEU inclusion.

I have listed myself as contact for CEU driver, as well as for ov772x as
I've a access to a test platform but for "Odd fixes" only.
I listed tw9910 as unmaintained instead, as I've not been able to test it.

Thanks
   j

Jacopo Mondi (3):
  MAINTAINERS: Add entry for Renesas CEU
  MAINTAINERS: Add entry for Omnivision OV772x
  MAINTAINERS: Add entry for Techwell TW9910

 MAINTAINERS | 24 
 1 file changed, 24 insertions(+)

--
2.7.4

[PATCH 1/3] MAINTAINERS: Add entry for Renesas CEU

[Jacopo Mondi]

Add entry for Renesas Capture Engine Interface listing myself as
maintainer.

Signed-off-by: Jacopo Mondi 
---
 MAINTAINERS | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index aee793b..de0d4c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8617,6 +8617,16 @@ T:   git git://linuxtv.org/media_tree.git
 S: Supported
 F: drivers/media/pci/netup_unidvb/*
 
+MEDIA DRIVERS FOR RENESAS - CEU
+M: Jacopo Mondi 
+L: linux-me...@vger.kernel.org
+L: linux-renesas-soc@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Supported
+F: Documentation/devicetree/bindings/media/renesas,ceu.txt
+F: drivers/media/platform/renesas-ceu.c
+F: include/media/drv-intf/renesas-ceu.h
+
 MEDIA DRIVERS FOR RENESAS - DRIF
 M: Ramesh Shanmugasundaram 
 L: linux-me...@vger.kernel.org
-- 
2.7.4

[PATCH 2/3] MAINTAINERS: Add entry for Omnivision OV772x

[Jacopo Mondi]

Add entry for Omnivision OV772x image sensor listing myself as maintainer
for 'Odd fixes' only, as I currently have access to a platform for
testing.

Signed-off-by: Jacopo Mondi 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index de0d4c6..64b8cd4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10071,6 +10071,14 @@ S: Maintained
 F: drivers/media/i2c/ov7670.c
 F: Documentation/devicetree/bindings/media/i2c/ov7670.txt
 
+OMNIVISION OV772x SENSOR DRIVER
+M: Jacopo Mondi 
+L: linux-me...@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Odd fixes
+F: drivers/media/i2c/ov772x.c
+F: include/media/i2c/ov772x.h
+
 OMNIVISION OV7740 SENSOR DRIVER
 M: Wenyou Yang 
 L: linux-me...@vger.kernel.org
-- 
2.7.4

Re: [PATCH 2/2] vfio: platform: Add generic DT reset support

[Philipp Zabel]

Hi Geert,

On Thu, 2018-02-22 at 09:50 +0100, Geert Uytterhoeven wrote:
[...]
> > > @@ -127,8 +134,15 @@ static int vfio_platform_get_reset(struct 
> > > vfio_platform_device *vdev)
> > >   vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
> > >   
> > > >reset_module);
> > >   }
> > > + if (vdev->of_reset)
> > > + return 0;
> > > +
> > > + vdev->reset_control = __of_reset_control_get(vdev->device->of_node,
> > > +  NULL, 0, false, false);
> > > + if (!IS_ERR(vdev->reset_control))
> > > + return 0;
> > 
> > if you assign to a local variable first here:
> > 
> > struct reset_control *rstc;
> > 
> >  ...
> > 
> > rstc = of_reset_control_get_exclusive(vdev->device->of_node, NULL);
> > if (!IS_ERR(rstc)) {
> > vdev->reset_control = rstc;
> > return 0;
> > }
> > 
> > Also, please don't use __of_reset_control_get directly.
> 
> OK, apparently I didn't read  beyond the first #else...

I don't blame you, this is missing some documentation.

> > > @@ -217,6 +233,9 @@ static int vfio_platform_call_reset(struct 
> > > vfio_platform_device *vdev,
> > >   } else if (vdev->of_reset) {
> > >   dev_info(vdev->device, "reset\n");
> > >   return vdev->of_reset(vdev);
> > > + } else if (!IS_ERR_OR_NULL(vdev->reset_control)) {
> > > + dev_info(vdev->device, "reset\n");
> > > + return reset_control_reset(vdev->reset_control);
> > 
> > } else {
> > if (vdev->reset_control)
> > dev_info(vdev->device, "reset\n");
> > return reset_control_reset(vdev->reset_control);
> > 
> > >   }
> 
> I'd like to keep the "else if", as that's the pattern used by the blocks 
> above.

my bad, there's more code after this.

} else if (vdev->reset_control) {
dev_info(vdev->device, "reset\n");
return reset_control_reset(vdev->reset_control);
}

is better as it doesn't lose the warning if vdev->reset_control == NULL.
It seems I've focused too much on getting rid of the IS_ERR_OR_NULL
macro.

regards
Philipp

Re: [PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity

[Geert Uytterhoeven]

Hi Laurent,

On Thu, Feb 22, 2018 at 1:05 AM, Laurent Pinchart
 wrote:
> From: Pantelis Antoniou 
>
> The changeset helpers are easier to use, use them instead of
> using the static property.
>
> Signed-off-by: Pantelis Antoniou 
> Acked-by: Wolfram Sang 
> ["okay" -> "ok"]

Why? ePAPR says "okay", "disabled", "fail", or "fail-sss".

Sorry for missing this in the previous round.

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] i2c: adv748x: afe: fix sparse warning

[Geert Uytterhoeven]

Hi Niklas,

On Thu, Feb 22, 2018 at 12:21 AM, Niklas Söderlund
 wrote:
> This fixes the following sparse warning:
>
> drivers/media/i2c/adv748x/adv748x-afe.c:294:34:expected unsigned int 
> [usertype] *signal
> drivers/media/i2c/adv748x/adv748x-afe.c:294:34:got int *
> drivers/media/i2c/adv748x/adv748x-afe.c:294:34: warning: incorrect type in 
> argument 2 (different signedness)
>
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/i2c/adv748x/adv748x-afe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c 
> b/drivers/media/i2c/adv748x/adv748x-afe.c
> index 5188178588c9067d..39a9996d0db08c31 100644
> --- a/drivers/media/i2c/adv748x/adv748x-afe.c
> +++ b/drivers/media/i2c/adv748x/adv748x-afe.c
> @@ -275,7 +275,8 @@ static int adv748x_afe_s_stream(struct v4l2_subdev *sd, 
> int enable)
>  {
> struct adv748x_afe *afe = adv748x_sd_to_afe(sd);
> struct adv748x_state *state = adv748x_afe_to_state(afe);
> -   int ret, signal = V4L2_IN_ST_NO_SIGNAL;
> +   unsigned int signal = V4L2_IN_ST_NO_SIGNAL;

u32, as adv748x_afe_status() takes an u32 signal pointer?

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] pinctrl: sh-pfc: r8a7795: remove duplicate of CLKOUT pin in pinmux_pins[]

[Geert Uytterhoeven]

Hi Niklas, Linus,

CC pinctrl

On Thu, Feb 22, 2018 at 12:32 AM, Niklas Söderlund
 wrote:
> When adding GP-1-28 port pin support it was forgotten to remove the
> CLKOUT pin from the list of pins that are not associated with a GPIO
> port in pinmux_pins[]. This results in a warning when reading the
> pinctrl files in sysfs as the CLKOUT pin is still added as a none GPIO
> pin. Fix this by removing the duplicated entry which is no longer
> needed.
>
> ~ # cat /sys/kernel/debug/pinctrl/e606.pin-controller/pinconf-pins
> [   89.432081] [ cut here ]
> [   89.436904] Pin 496 is not in bias info list
> [   89.441252] WARNING: CPU: 1 PID: 456 at drivers/pinctrl/sh-pfc/core.c:408 
> sh_pfc_pin_to_bias_reg+0xb0/0xb8
> [   89.451002] CPU: 1 PID: 456 Comm: cat Not tainted 
> 4.16.0-rc1-arm64-renesas-00048-gdfafc344a4f24dde #12
> [   89.460394] Hardware name: Renesas Salvator-X 2nd version board based on 
> r8a7795 ES2.0+ (DT)
> [   89.468910] pstate: 8085 (Nzcv daIf -PAN -UAO)
> [   89.473747] pc : sh_pfc_pin_to_bias_reg+0xb0/0xb8
> [   89.478495] lr : sh_pfc_pin_to_bias_reg+0xb0/0xb8
> [   89.483241] sp : 0aff3ab0
> [   89.486587] x29: 0aff3ab0 x28: 0893c698
> [   89.491955] x27: 08ad7d98 x26: 
> [   89.497323] x25: 8006fb3f5028 x24: 8006fb3f5018
> [   89.502690] x23: 0001 x22: 01f0
> [   89.508057] x21: 8006fb3f5018 x20: 08bef000
> [   89.513423] x19:  x18: 
> [   89.518790] x17: 6c4a x16: 08d67c98
> [   89.524157] x15: 0001 x14: 0896ca98
> [   89.529524] x13: cce5f611 x12: 8006f8d3b5a8
> [   89.534891] x11: 0981e000 x10: 08befa08
> [   89.540258] x9 : 8006f9b987a0 x8 : 08befa08
> [   89.545625] x7 : 08137094 x6 : 
> [   89.550991] x5 :  x4 : 0001
> [   89.556357] x3 : 0007 x2 : 0007
> [   89.561723] x1 : 1ff24f80f1818600 x0 : 
> [   89.567091] Call trace:
> [   89.569561]  sh_pfc_pin_to_bias_reg+0xb0/0xb8
> [   89.573960]  r8a7795_pinmux_get_bias+0x30/0xc0
> [   89.578445]  sh_pfc_pinconf_get+0x1e0/0x2d8
> [   89.582669]  pin_config_get_for_pin+0x20/0x30
> [   89.587067]  pinconf_generic_dump_one+0x180/0x1c8
> [   89.591815]  pinconf_generic_dump_pins+0x84/0xd8
> [   89.596476]  pinconf_pins_show+0xc8/0x130
> [   89.600528]  seq_read+0xe4/0x510
> [   89.603789]  full_proxy_read+0x60/0x90
> [   89.607576]  __vfs_read+0x30/0x140
> [   89.611010]  vfs_read+0x90/0x170
> [   89.614269]  SyS_read+0x60/0xd8
> [   89.617443]  __sys_trace_return+0x0/0x4
> [   89.621314] ---[ end trace 99c8d0d39c13e794 ]---
>
> Fixes: 82d2de5a4f646f72 ("pinctrl: sh-pfc: r8a7795: Add GP-1-28 port pin 
> support")
> Signed-off-by: Niklas Söderlund 

Reviewed-and-tested-by: Geert Uytterhoeven 

Linus: As the offending patch is only in v4.16-rc1 and later, can you please
take it directly as a fix for v4.16?

https://patchwork.kernel.org/patch/10234351/

Thanks!

> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> index 18aeee592fdcf246..35951e7b89d2fae3 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> @@ -1538,7 +1538,6 @@ static const struct sh_pfc_pin pinmux_pins[] = {
> SH_PFC_PIN_NAMED_CFG('B', 18, AVB_TD1, CFG_FLAGS),
> SH_PFC_PIN_NAMED_CFG('B', 19, AVB_RXC, CFG_FLAGS),
> SH_PFC_PIN_NAMED_CFG('C',  1, PRESETOUT#, CFG_FLAGS),
> -   SH_PFC_PIN_NAMED_CFG('F',  1, CLKOUT, CFG_FLAGS),
> SH_PFC_PIN_NAMED_CFG('H', 37, MLB_REF, CFG_FLAGS),
> SH_PFC_PIN_NAMED_CFG('V',  3, QSPI1_SPCLK, CFG_FLAGS),
> SH_PFC_PIN_NAMED_CFG('V',  5, QSPI1_SSL, CFG_FLAGS),

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] vfio: platform: Add generic DT reset support

[Geert Uytterhoeven]

Hi Philipp,

On Wed, Feb 21, 2018 at 5:51 PM, Philipp Zabel  wrote:
> I have a suggestion to avoid having to use the IS_ERR_OR_NULL macro, see
> below:
>
> On Tue, 2018-02-13 at 17:36 +0100, Geert Uytterhoeven wrote:
>> Vfio-platform requires reset support, provided either by ACPI, or, on DT
>> platforms, by a device-specific reset driver matching against the
>> device's compatible value.
>>
>> On many SoCs, devices are connected to an SoC-internal reset controller,
>> and can be reset in a generic way.  Hence add support to reset such
>> devices using the reset controller subsystem, provided the reset
>> hierarchy is described correctly in DT using the "resets" property.
>>
>> Devices that require a more complex reset procedure can still
>> provide a device-specific reset driver, as that takes precedence.
>>
>> Note that this functionality depends on CONFIG_RESET_CONTROLLER=y, and
>> becomes a no-op if reset controller support is disabled.
>>
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>>  drivers/vfio/platform/vfio_platform_common.c  | 23 +--
>>  drivers/vfio/platform/vfio_platform_private.h |  1 +
>>  2 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c 
>> b/drivers/vfio/platform/vfio_platform_common.c
>> index b60bb5326668498c..5d1e48f96e423508 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -17,6 +17,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -112,7 +113,13 @@ static bool vfio_platform_has_reset(struct 
>> vfio_platform_device *vdev)
>>   if (VFIO_PLATFORM_IS_ACPI(vdev))
>>   return vfio_platform_acpi_has_reset(vdev);
>>
>> - return vdev->of_reset ? true : false;
>> + if (vdev->of_reset)
>> + return true;
>> +
>> + if (!IS_ERR_OR_NULL(vdev->reset_control))
>> + return true;
>
> I'd avoid storing error values in vdev->reset_control at all, so this
> could be:
>
> if (vdev->reset_control)
> return true;

Thanks, much better!

>> @@ -127,8 +134,15 @@ static int vfio_platform_get_reset(struct 
>> vfio_platform_device *vdev)
>>   vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
>>   >reset_module);
>>   }
>> + if (vdev->of_reset)
>> + return 0;
>> +
>> + vdev->reset_control = __of_reset_control_get(vdev->device->of_node,
>> +  NULL, 0, false, false);
>> + if (!IS_ERR(vdev->reset_control))
>> + return 0;
>
> if you assign to a local variable first here:
>
> struct reset_control *rstc;
>
>  ...
>
> rstc = of_reset_control_get_exclusive(vdev->device->of_node, NULL);
> if (!IS_ERR(rstc)) {
> vdev->reset_control = rstc;
> return 0;
> }
>
> Also, please don't use __of_reset_control_get directly.

OK, apparently I didn't read  beyond the first #else...

>> @@ -217,6 +233,9 @@ static int vfio_platform_call_reset(struct 
>> vfio_platform_device *vdev,
>>   } else if (vdev->of_reset) {
>>   dev_info(vdev->device, "reset\n");
>>   return vdev->of_reset(vdev);
>> + } else if (!IS_ERR_OR_NULL(vdev->reset_control)) {
>> + dev_info(vdev->device, "reset\n");
>> + return reset_control_reset(vdev->reset_control);
>
> } else {
> if (vdev->reset_control)
> dev_info(vdev->device, "reset\n");
> return reset_control_reset(vdev->reset_control);
>
>>   }

I'd like to keep the "else if", as that's the pattern used by the blocks above.

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 v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1

[Geert Uytterhoeven]

Hi Simon,

On Wed, Feb 21, 2018 at 7:32 PM, Simon Horman  wrote:
> On Wed, Feb 21, 2018 at 05:30:12PM +0100, Geert Uytterhoeven wrote:
>> On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman  wrote:
>> > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote:
>> >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
>> >>  wrote:
>> >> > this series has been around for some time as RFC, and it has collected
>> >> > useful comments from the community along the way.
>> >> > The solution proposed by this patch set works for most R-Car Gen2 and
>> >> > RZ/G1 devices, but not all of them. We now know that for some R-Car
>> >> > Gen2 early revisions there is no proper software fix. Anyway, no
>> >> > product has been built around early revisions, but development boards
>> >> > mounting early revisions (basically prototypes) are still out there.
>> >> > As a result, this series isn't enabling the internal watchdog on R-Car
>> >> > Gen2 boards, developers may enable it in board specific device trees
>> >> > if needed.
>> >> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt,
>> >> > and Koelsch boards.
>> >> >
>> >> > The problem
>> >> > ===
>> >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector
>> >> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON
>> >> > the non-boot CPUs they are redirected to the reset vector installed by
>> >> > Linux in ICRAM1, and eventually they continue the execution to RAM,
>> >> > where the SMP bring-up code will take care of the rest.
>> >> > The content of the [S]BAR registers survives a watchdog triggered reset,
>> >> > and as such after the watchdog fires the boot core will try and execute
>> >> > the SMP bring-up code instead of jumping to the bootrom code.
>> >> >
>> >> > The fix
>> >> > ===
>> >> > The main strategy for the solution is to let the reset vector decide
>> >> > if it needs to jump to shmobile_boot_fn or to the bootrom code.
>> >> > In a watchdog triggered reset scenario, since the [S]BAR registers keep
>> >> > their values, the boot CPU will jump into the newly designed reset
>> >> > vector, the assembly routine will eventually test WOVF (a bit in 
>> >> > register
>> >> > RWTCSRA that indicates if the watchdog counter has overflown, the value
>> >> > of this bit gets retained in this scenario), and jump to the bootrom 
>> >> > code
>> >> > which will in turn load up the bootloader, etc.
>> >> > When bringing up SMP or using CPU hotplug, the reset vector will jump
>> >> > to shmobile_boot_fn instead.
>> >> >
>> >> > Thank you All for your help.
>> >> >
>> >> > Best regards,
>> >> >
>> >> > Fabrizio Castro (26):
>> >> >   ARM: shmobile: Add watchdog support
>> >> >   ARM: dts: r8a7743: Adjust SMP routine size
>> >> >   ARM: dts: r8a7745: Adjust SMP routine size
>> >> >   ARM: dts: r8a7790: Adjust SMP routine size
>> >> >   ARM: dts: r8a7791: Adjust SMP routine size
>> >> >   ARM: dts: r8a7792: Adjust SMP routine size
>> >> >   ARM: dts: r8a7793: Adjust SMP routine size
>> >> >   ARM: dts: r8a7794: Adjust SMP routine size
>> >> >   soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2
>> >> >   ARM: shmobile: rcar-gen2: Add watchdog support
>> >> >   dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support
>> >> >   watchdog: renesas_wdt: Add R-Car Gen2 support
>> >> >   watchdog: renesas_wdt: Add restart handler
>> >> >   ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN
>> >> >   clk: renesas: r8a7743: Add rwdt clock
>> >> >   clk: renesas: r8a7745: Add rwdt clock
>> >> >   clk: renesas: r8a7790: Add rwdt clock
>> >> >   clk: renesas: r8a7791/r8a7793: Add rwdt clock
>> >> >   clk: renesas: r8a7794: Add rwdt clock
>> >> >   ARM: dts: r8a7743: Add watchdog support to SoC dtsi
>> >> >   ARM: dts: r8a7745: Add watchdog support to SoC dtsi
>> >> >   ARM: dts: r8a7790: Add watchdog support to SoC dtsi
>> >> >   ARM: dts: r8a7791: Add watchdog support to SoC dtsi
>> >> >   ARM: dts: r8a7794: Add watchdog support to SoC dtsi
>> >> >   ARM: dts: iwg20m: Add watchdog support to SoM dtsi
>> >> >   ARM: dts: iwg22m: Add watchdog support to SoM dtsi
>> >> >
>> >> >  .../devicetree/bindings/watchdog/renesas-wdt.txt   | 19 ++--
>> >> >  arch/arm/boot/dts/r8a7743-iwg20m.dtsi  |  5 ++
>> >> >  arch/arm/boot/dts/r8a7743.dtsi | 12 -
>> >> >  arch/arm/boot/dts/r8a7745-iwg22m.dtsi  |  5 ++
>> >> >  arch/arm/boot/dts/r8a7745.dtsi | 12 -
>> >> >  arch/arm/boot/dts/r8a7790.dtsi | 12 -
>> >> >  arch/arm/boot/dts/r8a7791.dtsi | 12 -
>> >> >  arch/arm/boot/dts/r8a7792.dtsi |  2 +-
>> >> >  arch/arm/boot/dts/r8a7793.dtsi |  2 +-
>> >> >  arch/arm/boot/dts/r8a7794.dtsi | 12 -
>> >> >  arch/arm/configs/shmobile_defconfig|  1 +
>> >> >  

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Hans Verkuil]

On 02/21/2018 10:01 PM, Sakari Ailus wrote:
> Hi Laurent and Hans,
> 
> On Wed, Feb 21, 2018 at 10:16:25PM +0200, Laurent Pinchart wrote:
>> No, I'm sorry, for MC-based drivers this isn't correct. The media entity 
>> that 
>> symbolizes the DMA engine indeed has a sink pad, but it's a video node, not 
>> a 
>> subdev. It thus has no media bus format configured for its sink pad. The 
>> closest pad in the pipeline that has a media bus format is the source pad of 
>> the subdev connected to the video node.
>>
>> There's no communication within the kernel at G/S_FMT time between the video 
>> node and its connected subdev. The only time we look at the pipeline as a 
>> whole is when starting the stream to validate that the pipeline is correctly 
>> configured. We thus have to implement G/S_FMT on the video node without any 
>> knowledge about the connected subdev, and thus accept any colorspace.
> 
> A few more notes related to this --- there's no propagation of sub-device
> format across the entities; there were several reasons for the design
> choice. The V4L2 pixel format in the video node must be compatible with the
> sub-device format when streaming is started. And... the streaming will
> start in the pipeline defined by the enabled links to/from the video node.
> In principle nothign prevents having multiple links there, and at the time
> S_FMT IOCTL is called on the video node, none of those links could be
> enabled. And that's perfectly valid use of the API.
> 
> A lot of the DMA engine drivers are simply devices that receive data and
> write that to system memory, they really don't necessarily know anything
> else. For the hardware this data is just pixels (or even bits, especially
> in the case of CSI-2!).

Not in my experience. Most DMA engines I've ever worked with can do at
least quantization and RGB <-> YUV conversions. Which is pretty much
required functionality if you work with video receivers.

And in order to program that correctly in the DMA engine you have to
know what you receive.

Full-fledged colorspace converters that can convert between different
colorspaces and transfer functions are likely to be separate subdevs
due to the complexity of that.

> So I agree with Laurent here that requiring correct colour space for
> [GS]_FMT IOCTLs on video nodes in the general case is not feasible
> (especially on MC-centric devices), due to the way the Media controller
> pipeline and formats along that pipeline are configured and validated (i.e.
> at streamon time).
> 
> But what to do here then? The colourspace field is not verified even in
> link validation so there's no guarantee it's correctly set more or less
> anywhere else than in the source of the stream. And if the stream has
> multiple sources with different colour spaces, then what do you do? That's
> perhaps a theoretical case but the current frameworks and APIs do in
> principle support that.

It's not theoretical at all. But anyway, in that case it is up to userspace
to decide. A typical example is an sRGB OSD on top of a Rec.709 video.

In practice the differences in color at too small to be a problem, you'd
just use Rec. 709 and slight color differences in the sRGB OSD is not something
that is noticeable. But with HDR and BT.2020 this becomes much more complicated.

However, that is out of scope of the kernel driver.

> 
> Perhaps we should specify that the user should always set the same
> colourspace on the sink end of a link that was there in the source? The
> same should likely apply to the rest of the fields apart from width, height
> and code, too. Before the user configures formats this doesn't work though,
> and this does not address the matter with v4l2-compliance.
> 
> Granted that the drivers will themselves handle the colour space
> information correctly, it'd still provide a way for the user to gain the
> knowledge of the colour space which I believe is what matters.
> 

Urgh. It's really wrong IMHO that the DMA engine's input pad can't be
configured. It's inconsistent. I don't think we ever thought this through
properly.

Let me first fix all the other compliance issues and then I'll have to get
back to this. It's a mess.

Regards,

Hans