This series enables the hardware decoder present on mt8183. At first
glance, the only missing piece is the devicetree node for it, however,
simply adding it as is would cause an address collision between the
first register iospace and the clock-controller node, so a rework of the
dt-binding and driver, as well as addition of a clock, were needed
first.
Tested that H264 decoding works with the hardware decoder on
mt8183-kukui-jacuzzi-juniper-sku16, giving a fluster score of 98/135 on
the JVT-AVC_V1 test suite. And ensured other SoCs (MT8192 and MT8195)
still work as usual.
Nícolas F. R. A. Prado (5):
media: dt-bindings: mediatek,vcodec: Allow single clock for mt8183
media: dt-bindings: mediatek,vcodec: Don't require assigned-clocks
media: dt-bindings: mediatek,vcodec: Remove VDEC_SYS for mt8183
media: mediatek: vcodec: Read HW active status from clock
clk: mediatek: mt8183: Add CLK_VDEC_ACTIVE to vdec
Yunfei Dong (1):
arm64: dts: mediatek: mt8183: Add decoder
.../media/mediatek,vcodec-decoder.yaml | 56 ++++++++++++++----
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 39 ++++++++++++
drivers/clk/mediatek/clk-mt8183-vdec.c | 5 ++
.../mediatek/vcodec/mtk_vcodec_dec_drv.c | 59 +++++++++++++++----
.../mediatek/vcodec/mtk_vcodec_dec_hw.c | 20 +++++--
.../mediatek/vcodec/mtk_vcodec_dec_pm.c | 12 +++-
.../platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 +
include/dt-bindings/clock/mt8183-clk.h | 3 +-
8 files changed, 165 insertions(+), 30 deletions(-)
--
2.40.1
The binding expects the first register space to be VDEC_SYS. But on
mt8183, which uses the stateless decoders, this space is used only for
controlling clocks and resets, which are better described as separate
clock-controller and reset-controller nodes.
In fact, in mt8173's devicetree there are already such separate
clock-controller nodes, which cause duplicate addresses between the
vdecsys node and the vcodec node. But for this SoC, since the stateful
decoder code makes other uses of the VDEC_SYS register space, it's not
straightforward to remove it.
In order to avoid the same address conflict to happen on mt8183,
since the only current use of the VDEC_SYS register space in
the driver is to read the status of a clock that indicates the hardware
is active, remove the VDEC_SYS register space from the binding and
describe an extra clock that will be used to directly check the hardware
status.
Also add reg-names to be able to tell that this new register schema is
used, so the driver can keep backward compatibility.
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
.../media/mediatek,vcodec-decoder.yaml | 29 +++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
index 6447e6c86f29..36a53b2484d6 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
@@ -21,17 +21,21 @@ properties:
- mediatek,mt8183-vcodec-dec
reg:
+ minItems: 11
maxItems: 12
+ reg-names:
+ minItems: 11
+
interrupts:
maxItems: 1
clocks:
- minItems: 1
+ minItems: 2
maxItems: 8
clock-names:
- minItems: 1
+ minItems: 2
maxItems: 8
assigned-clocks: true
@@ -84,6 +88,24 @@ allOf:
clock-names:
items:
- const: vdec
+ - const: active
+
+ reg:
+ maxItems: 11
+
+ reg-names:
+ items:
+ - const: misc
+ - const: ld
+ - const: top
+ - const: cm
+ - const: ad
+ - const: av
+ - const: pp
+ - const: hwd
+ - const: hwq
+ - const: hwb
+ - const: hwg
- if:
properties:
@@ -108,6 +130,9 @@ allOf:
- const: venc_lt_sel
- const: vdec_bus_clk_src
+ reg:
+ minItems: 12
+
additionalProperties: false
examples:
--
2.40.1
Il 05/06/23 18:20, Nícolas F. R. A. Prado ha scritto:
> The binding expects the first register space to be VDEC_SYS. But on
> mt8183, which uses the stateless decoders, this space is used only for
> controlling clocks and resets, which are better described as separate
> clock-controller and reset-controller nodes.
>
> In fact, in mt8173's devicetree there are already such separate
> clock-controller nodes, which cause duplicate addresses between the
> vdecsys node and the vcodec node. But for this SoC, since the stateful
> decoder code makes other uses of the VDEC_SYS register space, it's not
> straightforward to remove it.
>
> In order to avoid the same address conflict to happen on mt8183,
> since the only current use of the VDEC_SYS register space in
> the driver is to read the status of a clock that indicates the hardware
> is active, remove the VDEC_SYS register space from the binding and
> describe an extra clock that will be used to directly check the hardware
> status.
>
> Also add reg-names to be able to tell that this new register schema is
> used, so the driver can keep backward compatibility.
>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
On 05/06/2023 18:20, Nícolas F. R. A. Prado wrote:
> The binding expects the first register space to be VDEC_SYS. But on
> mt8183, which uses the stateless decoders, this space is used only for
> controlling clocks and resets, which are better described as separate
> clock-controller and reset-controller nodes.
>
> In fact, in mt8173's devicetree there are already such separate
> clock-controller nodes, which cause duplicate addresses between the
> vdecsys node and the vcodec node. But for this SoC, since the stateful
> decoder code makes other uses of the VDEC_SYS register space, it's not
> straightforward to remove it.
>
> In order to avoid the same address conflict to happen on mt8183,
> since the only current use of the VDEC_SYS register space in
> the driver is to read the status of a clock that indicates the hardware
> is active, remove the VDEC_SYS register space from the binding and
> describe an extra clock that will be used to directly check the hardware
> status.
>
> Also add reg-names to be able to tell that this new register schema is
> used, so the driver can keep backward compatibility.
>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
> ---
>
> .../media/mediatek,vcodec-decoder.yaml | 29 +++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
> index 6447e6c86f29..36a53b2484d6 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
> @@ -21,17 +21,21 @@ properties:
> - mediatek,mt8183-vcodec-dec
>
> reg:
> + minItems: 11
> maxItems: 12
>
> + reg-names:
> + minItems: 11
maxItems
> +
> interrupts:
> maxItems: 1
>
> clocks:
> - minItems: 1
> + minItems: 2
It does not make any sense. Just two patches ago you made it 1! Don't
add incorrect values which are immediately changed in the same patchset.
> maxItems: 8
>
> clock-names:
> - minItems: 1
> + minItems: 2
> maxItems: 8
>
> assigned-clocks: true
> @@ -84,6 +88,24 @@ allOf:
> clock-names:
> items:
> - const: vdec
> + - const: active
> +
> + reg:
> + maxItems: 11
> +
> + reg-names:
> + items:
> + - const: misc
> + - const: ld
> + - const: top
> + - const: cm
> + - const: ad
> + - const: av
> + - const: pp
> + - const: hwd
> + - const: hwq
> + - const: hwb
> + - const: hwg
>
> - if:
> properties:
> @@ -108,6 +130,9 @@ allOf:
> - const: venc_lt_sel
> - const: vdec_bus_clk_src
>
> + reg:
> + minItems: 12
so max can be 1000?
Best regards,
Krzysztof
On Tue, Jun 06, 2023 at 11:16:12AM +0200, Krzysztof Kozlowski wrote:
> On 05/06/2023 18:20, N?colas F. R. A. Prado wrote:
> > The binding expects the first register space to be VDEC_SYS. But on
> > mt8183, which uses the stateless decoders, this space is used only for
> > controlling clocks and resets, which are better described as separate
> > clock-controller and reset-controller nodes.
> >
> > In fact, in mt8173's devicetree there are already such separate
> > clock-controller nodes, which cause duplicate addresses between the
> > vdecsys node and the vcodec node. But for this SoC, since the stateful
> > decoder code makes other uses of the VDEC_SYS register space, it's not
> > straightforward to remove it.
> >
> > In order to avoid the same address conflict to happen on mt8183,
> > since the only current use of the VDEC_SYS register space in
> > the driver is to read the status of a clock that indicates the hardware
> > is active, remove the VDEC_SYS register space from the binding and
> > describe an extra clock that will be used to directly check the hardware
> > status.
> >
> > Also add reg-names to be able to tell that this new register schema is
> > used, so the driver can keep backward compatibility.
> >
> > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> > ---
> >
> > .../media/mediatek,vcodec-decoder.yaml | 29 +++++++++++++++++--
> > 1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
> > index 6447e6c86f29..36a53b2484d6 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
> > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
> > @@ -21,17 +21,21 @@ properties:
> > - mediatek,mt8183-vcodec-dec
> >
> > reg:
> > + minItems: 11
> > maxItems: 12
> >
> > + reg-names:
> > + minItems: 11
>
> maxItems
>
> > +
> > interrupts:
> > maxItems: 1
> >
> > clocks:
> > - minItems: 1
> > + minItems: 2
>
> It does not make any sense. Just two patches ago you made it 1! Don't
> add incorrect values which are immediately changed in the same patchset.
It's not an incorrect value. At the point that commit was added, the first reg
was still VDEC_SYS, so an active clock wasn't required. This commit removes the
VDEC_SYS reg, and so the active clock becomes necessary. Splitting it like this
made the most sense to me, and I thought it would also simplify review. But
since it seems to be a problem I'll merge commit 1 with this one in v2 to avoid
changing the number of clocks twice.
>
> > maxItems: 8
> >
> > clock-names:
> > - minItems: 1
> > + minItems: 2
> > maxItems: 8
> >
> > assigned-clocks: true
> > @@ -84,6 +88,24 @@ allOf:
> > clock-names:
> > items:
> > - const: vdec
> > + - const: active
> > +
> > + reg:
> > + maxItems: 11
> > +
> > + reg-names:
> > + items:
> > + - const: misc
> > + - const: ld
> > + - const: top
> > + - const: cm
> > + - const: ad
> > + - const: av
> > + - const: pp
> > + - const: hwd
> > + - const: hwq
> > + - const: hwb
> > + - const: hwg
> >
> > - if:
> > properties:
> > @@ -108,6 +130,9 @@ allOf:
> > - const: venc_lt_sel
> > - const: vdec_bus_clk_src
> >
> > + reg:
> > + minItems: 12
>
> so max can be 1000?
No, there's 'maxItems: 12' up above in the general case.
Thanks,
N?colas