* [PATCH] drm: rcar-du: Add r8a77980 support @ 2019-09-11 19:25 Kieran Bingham 2019-09-12 10:00 ` Sergei Shtylyov 0 siblings, 1 reply; 15+ messages in thread From: Kieran Bingham @ 2019-09-11 19:25 UTC (permalink / raw) To: linux-renesas-soc, Laurent Pinchart Cc: Kieran Bingham, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Add direct support for the r8a77980 (V3H). The V3H shares a common, compatible configuration with the r8a77970 (V3M) so that device info structure is reused. Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index d1003d31cfaf..fc5b0949daf0 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -374,7 +374,10 @@ static const struct rcar_du_device_info rcar_du_r8a77970_info = { | RCAR_DU_FEATURE_TVM_SYNC, .channels_mask = BIT(0), .routes = { - /* R8A77970 has one RGB output and one LVDS output. */ + /* + * R8A77970 and R8A77980 have one RGB output and one LVDS + * output. + */ [RCAR_DU_OUTPUT_DPAD0] = { .possible_crtcs = BIT(0), .port = 0, @@ -432,6 +435,7 @@ static const struct of_device_id rcar_du_of_table[] = { { .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info }, { .compatible = "renesas,du-r8a77965", .data = &rcar_du_r8a77965_info }, { .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info }, + { .compatible = "renesas,du-r8a77980", .data = &rcar_du_r8a77970_info }, { .compatible = "renesas,du-r8a77990", .data = &rcar_du_r8a7799x_info }, { .compatible = "renesas,du-r8a77995", .data = &rcar_du_r8a7799x_info }, { } -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-11 19:25 [PATCH] drm: rcar-du: Add r8a77980 support Kieran Bingham @ 2019-09-12 10:00 ` Sergei Shtylyov 2019-09-12 10:26 ` Kieran Bingham 2019-09-13 8:21 ` Simon Horman 0 siblings, 2 replies; 15+ messages in thread From: Sergei Shtylyov @ 2019-09-12 10:00 UTC (permalink / raw) To: Kieran Bingham, linux-renesas-soc, Laurent Pinchart Cc: David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hello! On 11.09.2019 22:25, Kieran Bingham wrote: > Add direct support for the r8a77980 (V3H). > > The V3H shares a common, compatible configuration with the r8a77970 > (V3M) so that device info structure is reused. Do we really need to add yet another compatible in this case? I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why a patch like this one didn't get posted by me. > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> [...] MBR, Sergei ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-12 10:00 ` Sergei Shtylyov @ 2019-09-12 10:26 ` Kieran Bingham 2019-09-12 10:45 ` Kieran Bingham 2019-09-12 12:03 ` Geert Uytterhoeven 2019-09-13 8:21 ` Simon Horman 1 sibling, 2 replies; 15+ messages in thread From: Kieran Bingham @ 2019-09-12 10:26 UTC (permalink / raw) To: Sergei Shtylyov, linux-renesas-soc, Laurent Pinchart, Geert Uytterhoeven Cc: David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Sergei, (pulling in +Geert for his opinion on compatible string usages) On 12/09/2019 11:00, Sergei Shtylyov wrote:> Hello! > > On 11.09.2019 22:25, Kieran Bingham wrote: > >> Add direct support for the r8a77980 (V3H). >> >> The V3H shares a common, compatible configuration with the r8a77970 >> (V3M) so that device info structure is reused. > > Do we really need to add yet another compatible in this case? > I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > a patch like this one didn't get posted by me. It's not just about the compatible string for me here, There is no indication in the driver that it supports the r8a77980, and no comment in the driver to explain that the r8a77980 is shared by the r8a77970. This patch makes that explicit at the driver. Also - I am considering sending a patch (that I've already created anyway) to remove the r8a77970 reference from the arch/arm64/boot/dts/renesas/r8a77980.dtsi file. This is the *only* non r8a77980 reference in this file, so it seems very out of place. In fact more so than that - except for a seemingly glaring typo, that I'll investigate and send a patch for next, this is the *only* cross-soc compatible reference: #!/bin/sh files=r8a77*.dtsi for f in $files; do soc=`basename $f .dtsi | sed 's/-.*//'` echo "F: $f soc: $soc"; # Find all references to all socs, then hide 'this' soc grep r8a77 $f | grep -v $soc done; Finds : > F: r8a774a1.dtsi soc: r8a774a1 > F: r8a774c0.dtsi soc: r8a774c0 > F: r8a7795-es1.dtsi soc: r8a7795 > F: r8a7795.dtsi soc: r8a7795 > F: r8a7796.dtsi soc: r8a7796 > F: r8a77965.dtsi soc: r8a77965 > * Based on r8a7796.dtsi > F: r8a77970.dtsi soc: r8a77970 > compatible = "renesas,pwm-r8a7790", "renesas,pwm-rcar"; > F: r8a77980.dtsi soc: r8a77980 > "renesas,du-r8a77970"; > F: r8a77990.dtsi soc: r8a77990 > F: r8a77995.dtsi soc: r8a77995 -- KB > >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > [...] > > MBR, Sergei ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-12 10:26 ` Kieran Bingham @ 2019-09-12 10:45 ` Kieran Bingham 2019-09-12 12:03 ` Geert Uytterhoeven 1 sibling, 0 replies; 15+ messages in thread From: Kieran Bingham @ 2019-09-12 10:45 UTC (permalink / raw) To: Sergei Shtylyov, linux-renesas-soc, Laurent Pinchart, Geert Uytterhoeven Cc: David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Sergei, On 12/09/2019 11:26, Kieran Bingham wrote: > Hi Sergei, > > (pulling in +Geert for his opinion on compatible string usages) > > On 12/09/2019 11:00, Sergei Shtylyov wrote:> Hello! >> >> On 11.09.2019 22:25, Kieran Bingham wrote: >> >>> Add direct support for the r8a77980 (V3H). >>> >>> The V3H shares a common, compatible configuration with the r8a77970 >>> (V3M) so that device info structure is reused. >> >> Do we really need to add yet another compatible in this case? >> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why >> a patch like this one didn't get posted by me. Also, the documentation at : Documentation/devicetree/bindings/display/renesas,du.txt already states the the "renesas,du-r8a77980" compatible string is supported thanks to [0], so actually - this addition is a requirement to make the driver match the documentation. [0] 4ffe5aa53791 ("dt-bindings: display: renesas: du: document R8A77980 bindings") >>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> [...] >> >> MBR, Sergei > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-12 10:26 ` Kieran Bingham 2019-09-12 10:45 ` Kieran Bingham @ 2019-09-12 12:03 ` Geert Uytterhoeven 2019-09-12 12:11 ` Kieran Bingham 1 sibling, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2019-09-12 12:03 UTC (permalink / raw) To: Kieran Bingham Cc: Sergei Shtylyov, Linux-Renesas, Laurent Pinchart, Geert Uytterhoeven, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Kieran, On Thu, Sep 12, 2019 at 12:26 PM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > (pulling in +Geert for his opinion on compatible string usages) > > On 12/09/2019 11:00, Sergei Shtylyov wrote:> Hello! > > On 11.09.2019 22:25, Kieran Bingham wrote: > >> Add direct support for the r8a77980 (V3H). > >> > >> The V3H shares a common, compatible configuration with the r8a77970 > >> (V3M) so that device info structure is reused. > > > > Do we really need to add yet another compatible in this case? > > I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > > a patch like this one didn't get posted by me. > > It's not just about the compatible string for me here, > > There is no indication in the driver that it supports the r8a77980, and > no comment in the driver to explain that the r8a77980 is shared by the > r8a77970. > > This patch makes that explicit at the driver. > > Also - I am considering sending a patch (that I've already created > anyway) to remove the r8a77970 reference from the > > arch/arm64/boot/dts/renesas/r8a77980.dtsi file. > > This is the *only* non r8a77980 reference in this file, so it seems very > out of place. Agreed. > In fact more so than that - except for a seemingly glaring typo, that > I'll investigate and send a patch for next, this is the *only* cross-soc > compatible reference: > > #!/bin/sh > > files=r8a77*.dtsi > > for f in $files; > do > soc=`basename $f .dtsi | sed 's/-.*//'` > echo "F: $f soc: $soc"; > > # Find all references to all socs, then hide 'this' soc > grep r8a77 $f | grep -v $soc This hides the complete line. So you better use e.g. sed -e "s/$soc/soc/ig" $f | grep -i r8a instead. No new offenders, though. > done; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-12 12:03 ` Geert Uytterhoeven @ 2019-09-12 12:11 ` Kieran Bingham 0 siblings, 0 replies; 15+ messages in thread From: Kieran Bingham @ 2019-09-12 12:11 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Sergei Shtylyov, Linux-Renesas, Laurent Pinchart, Geert Uytterhoeven, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list On 12/09/2019 13:03, Geert Uytterhoeven wrote: > Hi Kieran, > > On Thu, Sep 12, 2019 at 12:26 PM Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com> wrote: >> (pulling in +Geert for his opinion on compatible string usages) >> >> On 12/09/2019 11:00, Sergei Shtylyov wrote:> Hello! >>> On 11.09.2019 22:25, Kieran Bingham wrote: >>>> Add direct support for the r8a77980 (V3H). >>>> >>>> The V3H shares a common, compatible configuration with the r8a77970 >>>> (V3M) so that device info structure is reused. >>> >>> Do we really need to add yet another compatible in this case? >>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why >>> a patch like this one didn't get posted by me. >> >> It's not just about the compatible string for me here, >> >> There is no indication in the driver that it supports the r8a77980, and >> no comment in the driver to explain that the r8a77980 is shared by the >> r8a77970. >> >> This patch makes that explicit at the driver. >> >> Also - I am considering sending a patch (that I've already created >> anyway) to remove the r8a77970 reference from the >> >> arch/arm64/boot/dts/renesas/r8a77980.dtsi file. >> >> This is the *only* non r8a77980 reference in this file, so it seems very >> out of place. > > Agreed. > >> In fact more so than that - except for a seemingly glaring typo, that >> I'll investigate and send a patch for next, this is the *only* cross-soc >> compatible reference: >> >> #!/bin/sh >> >> files=r8a77*.dtsi >> >> for f in $files; >> do >> soc=`basename $f .dtsi | sed 's/-.*//'` >> echo "F: $f soc: $soc"; >> >> # Find all references to all socs, then hide 'this' soc >> grep r8a77 $f | grep -v $soc > > This hides the complete line. So you better use e.g. > > sed -e "s/$soc/soc/ig" $f | grep -i r8a Aha yes, excellent point! (I'm glad I posted my working) > > instead. No new offenders, though. Phew, I still got the right answer :-D -- Kieran ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-12 10:00 ` Sergei Shtylyov 2019-09-12 10:26 ` Kieran Bingham @ 2019-09-13 8:21 ` Simon Horman 2019-09-13 8:30 ` Geert Uytterhoeven 2019-09-13 9:03 ` Laurent Pinchart 1 sibling, 2 replies; 15+ messages in thread From: Simon Horman @ 2019-09-13 8:21 UTC (permalink / raw) To: Sergei Shtylyov Cc: Kieran Bingham, linux-renesas-soc, Laurent Pinchart, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > Hello! > > On 11.09.2019 22:25, Kieran Bingham wrote: > > > Add direct support for the r8a77980 (V3H). > > > > The V3H shares a common, compatible configuration with the r8a77970 > > (V3M) so that device info structure is reused. > > Do we really need to add yet another compatible in this case? > I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > a patch like this one didn't get posted by me. The reason for having per-SoC compat strings is that the IP blocks are not versioned and while we can observe that there are similarities between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that differences may not emerge at some point. By having per-SoC compat strings we have the flexibility for the driver to address any such differences as the need arises. My recollection is that this scheme has been adopted for non-versioned Renesas IP blocks since June 2015 and uses of this scheme well before that. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > [...] > > MBR, Sergei > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-13 8:21 ` Simon Horman @ 2019-09-13 8:30 ` Geert Uytterhoeven 2019-09-13 8:57 ` Simon Horman 2019-09-13 9:03 ` Laurent Pinchart 1 sibling, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2019-09-13 8:30 UTC (permalink / raw) To: Simon Horman Cc: Sergei Shtylyov, Kieran Bingham, Linux-Renesas, Laurent Pinchart, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list On Fri, Sep 13, 2019 at 10:21 AM Simon Horman <horms@verge.net.au> wrote: > On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > > On 11.09.2019 22:25, Kieran Bingham wrote: > > > Add direct support for the r8a77980 (V3H). > > > The V3H shares a common, compatible configuration with the r8a77970 > > > (V3M) so that device info structure is reused. > > > > Do we really need to add yet another compatible in this case? > > I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > > a patch like this one didn't get posted by me. > > The reason for having per-SoC compat strings is that the IP blocks > are not versioned and while we can observe that there are similarities > between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that > differences may not emerge at some point. By having per-SoC compat strings > we have the flexibility for the driver to address any such differences as > the need arises. Thanks, that is the generic reason, applicable to all IP blocks without version registers. For the Display Unit, there are documented differences (e.g. number and type of ports), so we definitely need it there. > My recollection is that this scheme has been adopted for non-versioned > Renesas IP blocks since June 2015 and uses of this scheme well before that. > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-13 8:30 ` Geert Uytterhoeven @ 2019-09-13 8:57 ` Simon Horman 0 siblings, 0 replies; 15+ messages in thread From: Simon Horman @ 2019-09-13 8:57 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Sergei Shtylyov, Kieran Bingham, Linux-Renesas, Laurent Pinchart, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list On Fri, Sep 13, 2019 at 10:30:45AM +0200, Geert Uytterhoeven wrote: > On Fri, Sep 13, 2019 at 10:21 AM Simon Horman <horms@verge.net.au> wrote: > > On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > > > On 11.09.2019 22:25, Kieran Bingham wrote: > > > > Add direct support for the r8a77980 (V3H). > > > > The V3H shares a common, compatible configuration with the r8a77970 > > > > (V3M) so that device info structure is reused. > > > > > > Do we really need to add yet another compatible in this case? > > > I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > > > a patch like this one didn't get posted by me. > > > > The reason for having per-SoC compat strings is that the IP blocks > > are not versioned and while we can observe that there are similarities > > between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that > > differences may not emerge at some point. By having per-SoC compat strings > > we have the flexibility for the driver to address any such differences as > > the need arises. > > Thanks, that is the generic reason, applicable to all IP blocks without > version registers. > > For the Display Unit, there are documented differences (e.g. number and type > of ports), so we definitely need it there. Ack. > > My recollection is that this scheme has been adopted for non-versioned > > Renesas IP blocks since June 2015 and uses of this scheme well before that. > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-13 8:21 ` Simon Horman 2019-09-13 8:30 ` Geert Uytterhoeven @ 2019-09-13 9:03 ` Laurent Pinchart 2019-12-09 12:41 ` Kieran Bingham 1 sibling, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2019-09-13 9:03 UTC (permalink / raw) To: Simon Horman Cc: Sergei Shtylyov, Kieran Bingham, linux-renesas-soc, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hello, On Fri, Sep 13, 2019 at 10:21:29AM +0200, Simon Horman wrote: > On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > > On 11.09.2019 22:25, Kieran Bingham wrote: > > > > > Add direct support for the r8a77980 (V3H). > > > > > > The V3H shares a common, compatible configuration with the r8a77970 > > > (V3M) so that device info structure is reused. > > > > Do we really need to add yet another compatible in this case? > > I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > > a patch like this one didn't get posted by me. > > The reason for having per-SoC compat strings is that the IP blocks > are not versioned and while we can observe that there are similarities > between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that > differences may not emerge at some point. By having per-SoC compat strings > we have the flexibility for the driver to address any such differences as > the need arises. > > My recollection is that this scheme has been adopted for non-versioned > Renesas IP blocks since June 2015 and uses of this scheme well before that. Sure, but we could use compatible = "renesas,du-r8a77980", "renesas,du-r8a77970"; in DT without updating the driver. If the r8a77980 turns out to be different, we'll then update the driver without a need to modify DT. I'm fine either way, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au> -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-09-13 9:03 ` Laurent Pinchart @ 2019-12-09 12:41 ` Kieran Bingham 2019-12-13 0:48 ` Laurent Pinchart 0 siblings, 1 reply; 15+ messages in thread From: Kieran Bingham @ 2019-12-09 12:41 UTC (permalink / raw) To: Laurent Pinchart, Simon Horman Cc: Sergei Shtylyov, linux-renesas-soc, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Laurent, On 13/09/2019 10:03, Laurent Pinchart wrote: > Hello, > > On Fri, Sep 13, 2019 at 10:21:29AM +0200, Simon Horman wrote: >> On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: >>> On 11.09.2019 22:25, Kieran Bingham wrote: >>> >>>> Add direct support for the r8a77980 (V3H). >>>> >>>> The V3H shares a common, compatible configuration with the r8a77970 >>>> (V3M) so that device info structure is reused. >>> >>> Do we really need to add yet another compatible in this case? >>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why >>> a patch like this one didn't get posted by me. >> >> The reason for having per-SoC compat strings is that the IP blocks >> are not versioned and while we can observe that there are similarities >> between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that >> differences may not emerge at some point. By having per-SoC compat strings >> we have the flexibility for the driver to address any such differences as >> the need arises. >> >> My recollection is that this scheme has been adopted for non-versioned >> Renesas IP blocks since June 2015 and uses of this scheme well before that. > > Sure, but we could use > > compatible = "renesas,du-r8a77980", "renesas,du-r8a77970"; > > in DT without updating the driver. If the r8a77980 turns out to be > different, we'll then update the driver without a need to modify DT. I'm > fine either way, so > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, This patch has an RB tag from you, and Simon, but alas I don't believe it has been picked up in your drm/du/next branch. Is this patch acceptable? Or do I need to repost? -- Kieran > >>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-12-09 12:41 ` Kieran Bingham @ 2019-12-13 0:48 ` Laurent Pinchart 2019-12-16 9:47 ` Kieran Bingham 0 siblings, 1 reply; 15+ messages in thread From: Laurent Pinchart @ 2019-12-13 0:48 UTC (permalink / raw) To: Kieran Bingham Cc: Simon Horman, Sergei Shtylyov, linux-renesas-soc, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Kieran, On Mon, Dec 09, 2019 at 12:41:07PM +0000, Kieran Bingham wrote: > On 13/09/2019 10:03, Laurent Pinchart wrote: > > On Fri, Sep 13, 2019 at 10:21:29AM +0200, Simon Horman wrote: > >> On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > >>> On 11.09.2019 22:25, Kieran Bingham wrote: > >>> > >>>> Add direct support for the r8a77980 (V3H). > >>>> > >>>> The V3H shares a common, compatible configuration with the r8a77970 > >>>> (V3M) so that device info structure is reused. > >>> > >>> Do we really need to add yet another compatible in this case? > >>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > >>> a patch like this one didn't get posted by me. > >> > >> The reason for having per-SoC compat strings is that the IP blocks > >> are not versioned and while we can observe that there are similarities > >> between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that > >> differences may not emerge at some point. By having per-SoC compat strings > >> we have the flexibility for the driver to address any such differences as > >> the need arises. > >> > >> My recollection is that this scheme has been adopted for non-versioned > >> Renesas IP blocks since June 2015 and uses of this scheme well before that. > > > > Sure, but we could use > > > > compatible = "renesas,du-r8a77980", "renesas,du-r8a77970"; > > > > in DT without updating the driver. If the r8a77980 turns out to be > > different, we'll then update the driver without a need to modify DT. I'm > > fine either way, so > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks, > > This patch has an RB tag from you, and Simon, but alas I don't believe > it has been picked up in your drm/du/next branch. > > Is this patch acceptable? Or do I need to repost? Could you just confirm I should apply this patch, and not go for the alternative proposal above ? > >>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >> > >> Reviewed-by: Simon Horman <horms+renesas@verge.net.au> -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-12-13 0:48 ` Laurent Pinchart @ 2019-12-16 9:47 ` Kieran Bingham 2019-12-16 10:37 ` Geert Uytterhoeven 0 siblings, 1 reply; 15+ messages in thread From: Kieran Bingham @ 2019-12-16 9:47 UTC (permalink / raw) To: Laurent Pinchart Cc: Simon Horman, Sergei Shtylyov, linux-renesas-soc, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Laurent, On 13/12/2019 00:48, Laurent Pinchart wrote: > Hi Kieran, > > On Mon, Dec 09, 2019 at 12:41:07PM +0000, Kieran Bingham wrote: >> On 13/09/2019 10:03, Laurent Pinchart wrote: >>> On Fri, Sep 13, 2019 at 10:21:29AM +0200, Simon Horman wrote: >>>> On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: >>>>> On 11.09.2019 22:25, Kieran Bingham wrote: >>>>> >>>>>> Add direct support for the r8a77980 (V3H). >>>>>> >>>>>> The V3H shares a common, compatible configuration with the r8a77970 >>>>>> (V3M) so that device info structure is reused. >>>>> >>>>> Do we really need to add yet another compatible in this case? >>>>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why >>>>> a patch like this one didn't get posted by me. >>>> >>>> The reason for having per-SoC compat strings is that the IP blocks >>>> are not versioned and while we can observe that there are similarities >>>> between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that >>>> differences may not emerge at some point. By having per-SoC compat strings >>>> we have the flexibility for the driver to address any such differences as >>>> the need arises. >>>> >>>> My recollection is that this scheme has been adopted for non-versioned >>>> Renesas IP blocks since June 2015 and uses of this scheme well before that. >>> >>> Sure, but we could use >>> >>> compatible = "renesas,du-r8a77980", "renesas,du-r8a77970"; We already do in arch/arm64/boot/dts/renesas/r8a77980.dtsi. However that is the *only* non r8a77980 reference in the file so it, itself looks *very* much out of place. Furthermore, the main purpose of this patch is that we clearly document the driver as supporting the r8a77980 in the bindings (No mention that you must use the ..970 binding), yet in actual fact - the driver could not currently support loading a device with the following compatible: compatible = "renesas,du-r8a77980"; >>> in DT without updating the driver. If the r8a77980 turns out to be >>> different, we'll then update the driver without a need to modify DT. I'm >>> fine either way, so >>> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> Thanks, >> >> This patch has an RB tag from you, and Simon, but alas I don't believe >> it has been picked up in your drm/du/next branch. >> >> Is this patch acceptable? Or do I need to repost? > > Could you just confirm I should apply this patch, and not go for the > alternative proposal above ? I believe the alternative proposal above is what we have today isn't it? Yes, I do believe we should apply this patch. I'm going to assume you haven't read the other arguments on this thread so I'll paste them here: >>> <Sergei> >>> Do we really need to add yet another compatible in this case? >>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why >>> a patch like this one didn't get posted by me. >> >> <Kieran> >> It's not just about the compatible string for me here, >> >> There is no indication in the driver that it supports the r8a77980, and >> no comment in the driver to explain that the r8a77980 is shared by the >> r8a77970. >> >> This patch makes that explicit at the driver. >> >> Also - I am considering sending a patch (that I've already created >> anyway) to remove the r8a77970 reference from the >> >> arch/arm64/boot/dts/renesas/r8a77980.dtsi file. >> >> This is the *only* non r8a77980 reference in this file, so it seems very >> out of place. > > <Geert> > Agreed. > >> In fact more so than that - except for a seemingly glaring typo, that >> I'll investigate and send a patch for next, this is the *only* cross-soc >> compatible reference: >> >> #!/bin/sh >> >> files=r8a77*.dtsi >> >> for f in $files; >> do >> soc=`basename $f .dtsi | sed 's/-.*//'` >> echo "F: $f soc: $soc"; >> >> # Find all references to all socs, then hide 'this' soc >> grep r8a77 $f | grep -v $soc > > This hides the complete line. So you better use e.g. > > sed -e "s/$soc/soc/ig" $f | grep -i r8a > > instead. No new offenders, though. > >> done; > > Gr{oetje,eeting}s, > > Geert This is the only occurrence within *all* of our compatibles where we do not reference the compatible string of the device, and we require specifying 'another compatible'. This is not documented anywhere, and doesn't seem to follow {best,any}-practices. That's why I'm trying to fix it up. -- Regards Kieran ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-12-16 9:47 ` Kieran Bingham @ 2019-12-16 10:37 ` Geert Uytterhoeven 2019-12-17 23:11 ` Laurent Pinchart 0 siblings, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2019-12-16 10:37 UTC (permalink / raw) To: Kieran Bingham, Laurent Pinchart Cc: Simon Horman, Sergei Shtylyov, Linux-Renesas, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list Hi Kieran, Laurent, On Mon, Dec 16, 2019 at 10:47 AM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > On 13/12/2019 00:48, Laurent Pinchart wrote: > > On Mon, Dec 09, 2019 at 12:41:07PM +0000, Kieran Bingham wrote: > >> On 13/09/2019 10:03, Laurent Pinchart wrote: > >>> On Fri, Sep 13, 2019 at 10:21:29AM +0200, Simon Horman wrote: > >>>> On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > >>>>> On 11.09.2019 22:25, Kieran Bingham wrote: > >>>>> > >>>>>> Add direct support for the r8a77980 (V3H). > >>>>>> > >>>>>> The V3H shares a common, compatible configuration with the r8a77970 > >>>>>> (V3M) so that device info structure is reused. > >>>>> > >>>>> Do we really need to add yet another compatible in this case? > >>>>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > >>>>> a patch like this one didn't get posted by me. > >>>> > >>>> The reason for having per-SoC compat strings is that the IP blocks > >>>> are not versioned and while we can observe that there are similarities > >>>> between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that > >>>> differences may not emerge at some point. By having per-SoC compat strings > >>>> we have the flexibility for the driver to address any such differences as > >>>> the need arises. > >>>> > >>>> My recollection is that this scheme has been adopted for non-versioned > >>>> Renesas IP blocks since June 2015 and uses of this scheme well before that. > >>> > >>> Sure, but we could use > >>> > >>> compatible = "renesas,du-r8a77980", "renesas,du-r8a77970"; > > We already do in arch/arm64/boot/dts/renesas/r8a77980.dtsi. > > However that is the *only* non r8a77980 reference in the file so it, > itself looks *very* much out of place. > > > Furthermore, the main purpose of this patch is that we clearly document > the driver as supporting the r8a77980 in the bindings (No mention that > you must use the ..970 binding), yet in actual fact - the driver could > not currently support loading a device with the following compatible: > > compatible = "renesas,du-r8a77980"; > > > >>> in DT without updating the driver. If the r8a77980 turns out to be > >>> different, we'll then update the driver without a need to modify DT. I'm > >>> fine either way, so > >>> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > >> Thanks, > >> > >> This patch has an RB tag from you, and Simon, but alas I don't believe > >> it has been picked up in your drm/du/next branch. > >> > >> Is this patch acceptable? Or do I need to repost? > > > > Could you just confirm I should apply this patch, and not go for the > > alternative proposal above ? > > I believe the alternative proposal above is what we have today isn't it? > > > Yes, I do believe we should apply this patch. +1. I'm waiting for the driver part to go upstream, so I can apply the DTS patch. Note that this will lead to a messy situation in LTS, as the DTS patch will likely be backported, so the driver part must be backported, too. > I'm going to assume you haven't read the other arguments on this thread > so I'll paste them here: Thanks for recollecting! ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm: rcar-du: Add r8a77980 support 2019-12-16 10:37 ` Geert Uytterhoeven @ 2019-12-17 23:11 ` Laurent Pinchart 0 siblings, 0 replies; 15+ messages in thread From: Laurent Pinchart @ 2019-12-17 23:11 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Kieran Bingham, Simon Horman, Sergei Shtylyov, Linux-Renesas, David Airlie, Daniel Vetter, open list:DRM DRIVERS FOR RENESAS, open list On Mon, Dec 16, 2019 at 11:37:00AM +0100, Geert Uytterhoeven wrote: > On Mon, Dec 16, 2019 at 10:47 AM Kieran Bingham wrote: > > On 13/12/2019 00:48, Laurent Pinchart wrote: > >> On Mon, Dec 09, 2019 at 12:41:07PM +0000, Kieran Bingham wrote: > >>> On 13/09/2019 10:03, Laurent Pinchart wrote: > >>>> On Fri, Sep 13, 2019 at 10:21:29AM +0200, Simon Horman wrote: > >>>>> On Thu, Sep 12, 2019 at 01:00:41PM +0300, Sergei Shtylyov wrote: > >>>>>> On 11.09.2019 22:25, Kieran Bingham wrote: > >>>>>> > >>>>>>> Add direct support for the r8a77980 (V3H). > >>>>>>> > >>>>>>> The V3H shares a common, compatible configuration with the r8a77970 > >>>>>>> (V3M) so that device info structure is reused. > >>>>>> > >>>>>> Do we really need to add yet another compatible in this case? > >>>>>> I just added r8a77970 to the compatible prop in the r8a77980 DT. That's why > >>>>>> a patch like this one didn't get posted by me. > >>>>> > >>>>> The reason for having per-SoC compat strings is that the IP blocks > >>>>> are not versioned and while we can observe that there are similarities > >>>>> between, f.e. the DU on the r8a77970 and r8a77980, we can't be certain that > >>>>> differences may not emerge at some point. By having per-SoC compat strings > >>>>> we have the flexibility for the driver to address any such differences as > >>>>> the need arises. > >>>>> > >>>>> My recollection is that this scheme has been adopted for non-versioned > >>>>> Renesas IP blocks since June 2015 and uses of this scheme well before that. > >>>> > >>>> Sure, but we could use > >>>> > >>>> compatible = "renesas,du-r8a77980", "renesas,du-r8a77970"; > > > > We already do in arch/arm64/boot/dts/renesas/r8a77980.dtsi. > > > > However that is the *only* non r8a77980 reference in the file so it, > > itself looks *very* much out of place. > > > > > > Furthermore, the main purpose of this patch is that we clearly document > > the driver as supporting the r8a77980 in the bindings (No mention that > > you must use the ..970 binding), yet in actual fact - the driver could > > not currently support loading a device with the following compatible: > > > > compatible = "renesas,du-r8a77980"; > > > > > >>>> in DT without updating the driver. If the r8a77980 turns out to be > >>>> different, we'll then update the driver without a need to modify DT. I'm > >>>> fine either way, so > >>>> > >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> > >>> Thanks, > >>> > >>> This patch has an RB tag from you, and Simon, but alas I don't believe > >>> it has been picked up in your drm/du/next branch. > >>> > >>> Is this patch acceptable? Or do I need to repost? > >> > >> Could you just confirm I should apply this patch, and not go for the > >> alternative proposal above ? > > > > I believe the alternative proposal above is what we have today isn't it? > > > > > > Yes, I do believe we should apply this patch. > > +1. > > I'm waiting for the driver part to go upstream, so I can apply the DTS patch. > Note that this will lead to a messy situation in LTS, as the DTS patch will > likely be backported, so the driver part must be backported, too. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> and taken in my tree. > > I'm going to assume you haven't read the other arguments on this thread > > so I'll paste them here: > > Thanks for recollecting! ;-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-12-17 23:11 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-11 19:25 [PATCH] drm: rcar-du: Add r8a77980 support Kieran Bingham 2019-09-12 10:00 ` Sergei Shtylyov 2019-09-12 10:26 ` Kieran Bingham 2019-09-12 10:45 ` Kieran Bingham 2019-09-12 12:03 ` Geert Uytterhoeven 2019-09-12 12:11 ` Kieran Bingham 2019-09-13 8:21 ` Simon Horman 2019-09-13 8:30 ` Geert Uytterhoeven 2019-09-13 8:57 ` Simon Horman 2019-09-13 9:03 ` Laurent Pinchart 2019-12-09 12:41 ` Kieran Bingham 2019-12-13 0:48 ` Laurent Pinchart 2019-12-16 9:47 ` Kieran Bingham 2019-12-16 10:37 ` Geert Uytterhoeven 2019-12-17 23:11 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).