2020-04-30 16:45:01

by Paul Kocialkowski

[permalink] [raw]
Subject: [PATCH v3 0/4] media: rockchip: rga: PX30 support and YUV2YUV fix

Hi,

This series adds support for the Rockchip PX30 SoC in the V4L2 M2M RGA driver.
It also contains a fix for the YUV2YUV case that was not properly handled.

Changes since v2:
- Used RK3288 compatible in PX30 dts, removed PX30 compatible from driver;
- Added cleanup patch with format macros;
- Added comment about CSC mode fix.

Changes since v1:
- Rebased on media tree master (changed dt binding to yaml);
- Removed spurious line removal.

Cheers,

Paul

Paul Kocialkowski (4):
dt-bindings: rockchip-rga: Add PX30 compatible
arm64: dts: rockchip: Add RGA support to the PX30
media: rockchip: rga: Introduce color fmt macros and refactor CSC mode
logic
media: rockchip: rga: Only set output CSC mode for RGB input

.../bindings/media/rockchip-rga.yaml | 3 ++
arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++
drivers/media/platform/rockchip/rga/rga-hw.c | 29 ++++++++++---------
drivers/media/platform/rockchip/rga/rga-hw.h | 5 ++++
4 files changed, 35 insertions(+), 13 deletions(-)

--
2.26.0


2020-04-30 16:47:12

by Paul Kocialkowski

[permalink] [raw]
Subject: [PATCH v3 2/4] arm64: dts: rockchip: Add RGA support to the PX30

The PX30 features a RGA block: add the necessary node to support it.

Signed-off-by: Paul Kocialkowski <[email protected]>
---
arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index f809dd6d5dc3..3de70aa4f1ce 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -1102,6 +1102,17 @@ vopl_mmu: iommu@ff470f00 {
status = "disabled";
};

+ rga: rga@ff480000 {
+ compatible = "rockchip,px30-rga", "rockchip,rk3288-rga";
+ reg = <0x0 0xff480000 0x0 0x10000>;
+ interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
+ clock-names = "aclk", "hclk", "sclk";
+ resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
+ reset-names = "core", "axi", "ahb";
+ power-domains = <&power PX30_PD_VO>;
+ };
+
qos_gmac: qos@ff518000 {
compatible = "syscon";
reg = <0x0 0xff518000 0x0 0x20>;
--
2.26.0

2020-04-30 22:07:51

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] arm64: dts: rockchip: Add RGA support to the PX30

Hi Paul,

> The PX30 features a RGA block: add the necessary node to support it.
>
> Signed-off-by: Paul Kocialkowski <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> index f809dd6d5dc3..3de70aa4f1ce 100644
> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> @@ -1102,6 +1102,17 @@ vopl_mmu: iommu@ff470f00 {
> status = "disabled";
> };
>
> + rga: rga@ff480000 {
> + compatible = "rockchip,px30-rga", "rockchip,rk3288-rga";
> + reg = <0x0 0xff480000 0x0 0x10000>;
> + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> + clock-names = "aclk", "hclk", "sclk";

> + resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> + reset-names = "core", "axi", "ahb";
> + power-domains = <&power PX30_PD_VO>;

sort

power-domains = <&power PX30_PD_VO>;
resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
reset-names = "core", "axi", "ahb";



> + };
> +
> qos_gmac: qos@ff518000 {
> compatible = "syscon";
> reg = <0x0 0xff518000 0x0 0x20>;
> --
> 2.26.0

2020-05-07 20:27:51

by Paul Kocialkowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] arm64: dts: rockchip: Add RGA support to the PX30

Hi,

On Fri 01 May 20, 00:05, Johan Jonker wrote:
> Hi Paul,
>
> > The PX30 features a RGA block: add the necessary node to support it.
> >
> > Signed-off-by: Paul Kocialkowski <[email protected]>
> > ---
> > arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > index f809dd6d5dc3..3de70aa4f1ce 100644
> > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > @@ -1102,6 +1102,17 @@ vopl_mmu: iommu@ff470f00 {
> > status = "disabled";
> > };
> >
> > + rga: rga@ff480000 {
> > + compatible = "rockchip,px30-rga", "rockchip,rk3288-rga";
> > + reg = <0x0 0xff480000 0x0 0x10000>;
> > + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> > + clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> > + clock-names = "aclk", "hclk", "sclk";
>
> > + resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> > + reset-names = "core", "axi", "ahb";
> > + power-domains = <&power PX30_PD_VO>;
>
> sort
>
> power-domains = <&power PX30_PD_VO>;
> resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> reset-names = "core", "axi", "ahb";

What's the rationale behind this (besides alphabetic sorting, which I don't
believe is a rule for dt properties)? Some nodes above in the file have it in
the same order that I do, and I like to see clocks followed by resets.

Cheers,

Paul

>
>
> > + };
> > +
> > qos_gmac: qos@ff518000 {
> > compatible = "syscon";
> > reg = <0x0 0xff518000 0x0 0x20>;
> > --
> > 2.26.0
>

--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


Attachments:
(No filename) (1.78 kB)
signature.asc (499.00 B)
Download all attachments

2020-05-07 23:42:37

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] arm64: dts: rockchip: Add RGA support to the PX30

Hi Paul,

On 5/7/20 10:25 PM, Paul Kocialkowski wrote:
> Hi,
>
> On Fri 01 May 20, 00:05, Johan Jonker wrote:
>> Hi Paul,
>>
>>> The PX30 features a RGA block: add the necessary node to support it.
>>>
>>> Signed-off-by: Paul Kocialkowski <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> index f809dd6d5dc3..3de70aa4f1ce 100644
>>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
>>> @@ -1102,6 +1102,17 @@ vopl_mmu: iommu@ff470f00 {
>>> status = "disabled";
>>> };
>>>
>>> + rga: rga@ff480000 {
>>> + compatible = "rockchip,px30-rga", "rockchip,rk3288-rga";
>>> + reg = <0x0 0xff480000 0x0 0x10000>;
>>> + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
>>> + clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
>>> + clock-names = "aclk", "hclk", "sclk";
>>
>>> + resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>>> + reset-names = "core", "axi", "ahb";
>>> + power-domains = <&power PX30_PD_VO>;
>>
>> sort
>>
>> power-domains = <&power PX30_PD_VO>;
>> resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
>> reset-names = "core", "axi", "ahb";
>
> What's the rationale behind this (besides alphabetic sorting, which I don't
> believe is a rule for dt properties)? Some nodes above in the file have it in
> the same order that I do, and I like to see clocks followed by resets.

My short list.
There is no hard rule... It mostly depend on Heiko...

For nodes:
If exists on top: model, compatible and chosen.
Sort things without reg alphabetical first,
then sort the rest by reg address.

Inside nodes:
If exists on top: compatible, reg and interrupts.
In alphabetical order the required properties.
Then in alphabetical order the other properties.
And as last things that start with '#' in alphabetical order.
Add status below all other properties for soc internal components with
any board-specifics.
Keep an empty line between properties and nodes.

Exceptions:
Sort pinctrl-0 above pinctrl-names, so it stays in line with clock-names
and dma-names.
Sort simple-audio-card,name above other simple-audio-card properties.
Sort regulator-name above other regulator properties.
Sort regulator-min-microvolt above regulator-max-microvolt.

>
> Cheers,
>
> Paul
>
>>
>>
>>> + };
>>> +
>>> qos_gmac: qos@ff518000 {
>>> compatible = "syscon";
>>> reg = <0x0 0xff518000 0x0 0x20>;
>>> --
>>> 2.26.0
>>
>

2020-05-08 10:58:53

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] arm64: dts: rockchip: Add RGA support to the PX30

Am Freitag, 8. Mai 2020, 01:40:08 CEST schrieb Johan Jonker:
> Hi Paul,
>
> On 5/7/20 10:25 PM, Paul Kocialkowski wrote:
> > Hi,
> >
> > On Fri 01 May 20, 00:05, Johan Jonker wrote:
> >> Hi Paul,
> >>
> >>> The PX30 features a RGA block: add the necessary node to support it.
> >>>
> >>> Signed-off-by: Paul Kocialkowski <[email protected]>
> >>> ---
> >>> arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
> >>> 1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> index f809dd6d5dc3..3de70aa4f1ce 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> >>> @@ -1102,6 +1102,17 @@ vopl_mmu: iommu@ff470f00 {
> >>> status = "disabled";
> >>> };
> >>>
> >>> + rga: rga@ff480000 {
> >>> + compatible = "rockchip,px30-rga", "rockchip,rk3288-rga";
> >>> + reg = <0x0 0xff480000 0x0 0x10000>;
> >>> + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> + clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru SCLK_RGA_CORE>;
> >>> + clock-names = "aclk", "hclk", "sclk";
> >>
> >>> + resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >>> + reset-names = "core", "axi", "ahb";
> >>> + power-domains = <&power PX30_PD_VO>;
> >>
> >> sort
> >>
> >> power-domains = <&power PX30_PD_VO>;
> >> resets = <&cru SRST_RGA>, <&cru SRST_RGA_A>, <&cru SRST_RGA_H>;
> >> reset-names = "core", "axi", "ahb";
> >
> > What's the rationale behind this (besides alphabetic sorting, which I don't
> > believe is a rule for dt properties)? Some nodes above in the file have it in
> > the same order that I do, and I like to see clocks followed by resets.
>
> My short list.
> There is no hard rule... It mostly depend on Heiko...

For the record, if needed I do any re-sorting myself normally, so there is
no need to respin patches just because nodes are sorted differently.

But yes, since the early Chromebook project in 2014 we agreed on
doing in Rockchip dts files:

----
compatible
reg
interrupts
[alphabetical]
status [if needed]
----

This works most of the time, but sometimes gets missed but is not _that_
big a deal if that happens ;-) .


Heiko


> For nodes:
> If exists on top: model, compatible and chosen.
> Sort things without reg alphabetical first,
> then sort the rest by reg address.
>
> Inside nodes:
> If exists on top: compatible, reg and interrupts.
> In alphabetical order the required properties.
> Then in alphabetical order the other properties.
> And as last things that start with '#' in alphabetical order.
> Add status below all other properties for soc internal components with
> any board-specifics.
> Keep an empty line between properties and nodes.
>
> Exceptions:
> Sort pinctrl-0 above pinctrl-names, so it stays in line with clock-names
> and dma-names.
> Sort simple-audio-card,name above other simple-audio-card properties.
> Sort regulator-name above other regulator properties.
> Sort regulator-min-microvolt above regulator-max-microvolt.
>
> >
> > Cheers,
> >
> > Paul
> >
> >>
> >>
> >>> + };
> >>> +
> >>> qos_gmac: qos@ff518000 {
> >>> compatible = "syscon";
> >>> reg = <0x0 0xff518000 0x0 0x20>;
> >>
> >
>
>