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 <[email protected]>
---
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
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 <[email protected]>
[...]
MBR, Sergei
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 <[email protected]>
> [...]
>
> MBR, Sergei
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 <[email protected]>
>> [...]
>>
>> MBR, Sergei
>
Hi Kieran,
On Thu, Sep 12, 2019 at 12:26 PM Kieran Bingham
<[email protected]> 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 -- [email protected]
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
On 12/09/2019 13:03, Geert Uytterhoeven wrote:
> Hi Kieran,
>
> On Thu, Sep 12, 2019 at 12:26 PM Kieran Bingham
> <[email protected]> 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
On Fri, Sep 13, 2019 at 10:30:45AM +0200, Geert Uytterhoeven wrote:
> On Fri, Sep 13, 2019 at 10:21 AM Simon Horman <[email protected]> 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 <[email protected]>
> >
> > Reviewed-by: Simon Horman <[email protected]>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
>
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 <[email protected]>
> > > Signed-off-by: Kieran Bingham <[email protected]>
>
> Reviewed-by: Simon Horman <[email protected]>
--
Regards,
Laurent Pinchart
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 <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
> [...]
>
> MBR, Sergei
>
On Fri, Sep 13, 2019 at 10:21 AM Simon Horman <[email protected]> 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 <[email protected]>
>
> Reviewed-by: Simon Horman <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
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 <[email protected]>
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 <[email protected]>
>>
>> Reviewed-by: Simon Horman <[email protected]>
>
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 <[email protected]>
>
> 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 <[email protected]>
> >>
> >> Reviewed-by: Simon Horman <[email protected]>
--
Regards,
Laurent Pinchart