Re: [PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
On Tue, Feb 20, 2018 at 3:15 AM, Laurent Pinchart wrote: > Hi Rob, > > On Tuesday, 20 February 2018 02:54:00 EET Rob Herring wrote: >> On Wed, Feb 14, 2018 at 6:04 PM, Laurent Pinchart wrote: >> > The internal LVDS encoders now have their own DT bindings. Before >> > switching the driver infrastructure to those new bindings, implement >> > backward-compatibility through live DT patching. >> > >> > Patching is disabled and will be enabled along with support for the new >> > DT bindings in the DU driver. >> > >> > Signed-off-by: Laurent Pinchart >> > >> > +static int __init rcar_du_of_add_property(struct device_node *np, >> > + const char *name, const void >> > *value, >> > + size_t length) >> >> So, were you going to revive Pantelis' patch or move this to the core? > > Do you have a pointer to that patch ? https://patchwork.kernel.org/patch/9104571/ https://patchwork.kernel.org/patch/9104591/ https://patchwork.kernel.org/patch/9104611/ https://patchwork.kernel.org/patch/9104581/ https://patchwork.kernel.org/patch/9104601/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
Hi Rob,
On Tuesday, 20 February 2018 02:54:00 EET Rob Herring wrote:
> On Wed, Feb 14, 2018 at 6:04 PM, Laurent Pinchart wrote:
> > The internal LVDS encoders now have their own DT bindings. Before
> > switching the driver infrastructure to those new bindings, implement
> > backward-compatibility through live DT patching.
> >
> > 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 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
>
> Generally looks fine to me. A few things below.
>
> > 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 | 374
> > + drivers/gpu/drm/rcar-du/rcar_du_of.h
> > | 20 ++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts| 81 +
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts| 55 +++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts| 55 +++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts| 55 +++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts| 55 +++
> > 9 files changed, 703 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 ..141f6eda6e98
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
[snip]
> > +static struct device_node __init *
> > +rcar_du_of_find_node_by_path(struct device_node *parent, const char
> > *path)
> > +{
>
> I guess I never followed up on the last version. I think a wrapper
> around __of_find_node_by_path would work for you here. It can start at
> any level. Though maybe it is not needed. See below.
You're right, I won't need this function anymore, so I'll drop it. I think
looking up a node by path starting at a given parent would still be useful,
but there's no need to add a function without a user.
> > + parent = of_node_get(parent);
> > + if (!parent)
> > + return NULL;
> > +
> > + while (parent && *path == '/') {
> > +
Re: [PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
On Wed, Feb 14, 2018 at 6:04 PM, Laurent Pinchart
wrote:
> The internal LVDS encoders now have their own DT bindings. Before
> switching the driver infrastructure to those new bindings, implement
> backward-compatibility through live DT patching.
>
> 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 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
Generally looks fine to me. A few things below.
>
> 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 | 374
> +
> drivers/gpu/drm/rcar-du/rcar_du_of.h | 20 ++
> .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts| 81 +
> .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts| 55 +++
> .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts| 55 +++
> .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts| 55 +++
> .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts| 55 +++
> 9 files changed, 703 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 ..141f6eda6e98
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -0,0 +1,374 @@
> +// 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) \
> + {
Re: [PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
Hi Laurent,
On Thu, Feb 15, 2018 at 12:03 PM, Laurent Pinchart
wrote:
> On Thursday, 15 February 2018 11:18:25 EET Geert Uytterhoeven wrote:
>> On Thu, Feb 15, 2018 at 1:04 AM, 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
>> >
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
>> > @@ -0,0 +1,81 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +/*
>> > + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for
>> > R8A7790 + *
>> > + * Copyright (C) 2018 Laurent Pinchart
>> >
>> > + *
>> > + * Based on work from Jyri Sarha
>> > + * Copyright (C) 2015 Texas Instruments
>> > + */
>> > +
>> > +#include
>> > +
>> > +/dts-v1/;
>> > +/plugin/;
>> > +/ {
>> > + fragment@1 {
>> > + target-path = "/display@feb0/ports";
>>
>> Does this work now there can be an "soc" subnode, too?
>>
>> Your code obtains the right parent:
>>
>> soc_node = of_get_parent(du_node);
>>
>> but the overlay is applied without considering that, if I'm not mistaken.
>
> Correct, and as I can't modify the target-path in the overlay dynamically
> anymore, I can't take this into account. I could duplicate the r8a7790 and
> r8a7791 overlays in two versions to handle this, but as the soc subnode isn't
> in mainline yet, I'd rather get this patch series merged with the current
> implementation in v4.17 to not have to handle this issue :-)
That's of course an option.
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
Hi Geert,
On Thursday, 15 February 2018 11:18:25 EET Geert Uytterhoeven wrote:
> On Thu, Feb 15, 2018 at 1:04 AM, 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
> >
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
> > @@ -0,0 +1,81 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for
> > R8A7790 + *
> > + * Copyright (C) 2018 Laurent Pinchart
> >
> > + *
> > + * Based on work from Jyri Sarha
> > + * Copyright (C) 2015 Texas Instruments
> > + */
> > +
> > +#include
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +/ {
> > + fragment@0 {
> > + target-path = "/";
> > + __overlay__ {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + lvds@feb9 {
> > + compatible = "renesas,r8a7790-lvds";
> > + reg = <0 0xfeb9 0 0x1c>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + lvds0_input: endpoint {
> > + };
> > + };
> > + port@1 {
> > + reg = <1>;
> > + lvds0_out: endpoint {
> > + };
> > + };
> > + };
> > + };
> > +
> > + lvds@feb94000 {
> > + compatible = "renesas,r8a7790-lvds";
> > + reg = <0 0xfeb94000 0 0x1c>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + lvds1_input: endpoint {
> > + };
> > + };
> > + port@1 {
> > + reg = <1>;
> > + lvds1_out: endpoint {
> > + };
> > + };
> > + };
> > + };
> > + };
> > + };
> > +
> > + fragment@1 {
> > + target-path = "/display@feb0/ports";
>
> Does this work now there can be an "soc" subnode, too?
>
> Your code obtains the right parent:
>
> soc_node = of_get_parent(du_node);
>
> but the overlay is applied without considering that, if I'm not mistaken.
Correct, and as I can't modify the target-path in the overlay dynamically
anymore, I can't take this into account. I could duplicate the r8a7790 and
r8a7791 overlays in two versions to handle this, but as the soc subnode isn't
in mainline yet, I'd rather get this patch series merged with the current
implementation in v4.17 to not have to handle this issue :-)
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
Hi Laurent,
On Thu, Feb 15, 2018 at 1:04 AM, 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
Thanks for your patch!
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for
> R8A7790
> + *
> + * Copyright (C) 2018 Laurent Pinchart
> + *
> + * Based on work from Jyri Sarha
> + * Copyright (C) 2015 Texas Instruments
> + */
> +
> +#include
> +
> +/dts-v1/;
> +/plugin/;
> +/ {
> + fragment@0 {
> + target-path = "/";
> + __overlay__ {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + lvds@feb9 {
> + compatible = "renesas,r8a7790-lvds";
> + reg = <0 0xfeb9 0 0x1c>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds0_input: endpoint {
> + };
> + };
> + port@1 {
> + reg = <1>;
> + lvds0_out: endpoint {
> + };
> + };
> + };
> + };
> +
> + lvds@feb94000 {
> + compatible = "renesas,r8a7790-lvds";
> + reg = <0 0xfeb94000 0 0x1c>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds1_input: endpoint {
> + };
> + };
> + port@1 {
> + reg = <1>;
> + lvds1_out: endpoint {
> + };
> + };
> + };
> + };
> + };
> + };
> +
> + fragment@1 {
> + target-path = "/display@feb0/ports";
Does this work now there can be an "soc" subnode, too?
Your code obtains the right parent:
soc_node = of_get_parent(du_node);
but the overlay is applied without considering that, if I'm not mistaken.
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
