| Message ID | 20221231164628.19688-1-samuel@sholland.org |
|---|---|
| Headers | show |
| Series | Allwinner D1 video engine support | expand |
Hi Samuel, On Sat 31 Dec 22, 10:46, Samuel Holland wrote: > The video engine in the D1 family of SoCs does not have a switchable > SRAM section. Allow the driver to probe even when the SRAM section > reference is missing. > > Signed-off-by: Samuel Holland <samuel@sholland.org> Looks good, I've also just checked that calling sunxi_sram_release with no reference to the SRAM held is fine (maybe a word about it in the commit log would be nice, but probably not worth making a v2 just for that). Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> Cheers, Paul > --- > > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > index fa86a658fdc6..11e859617932 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c > @@ -257,7 +257,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev) > } > > ret = sunxi_sram_claim(dev->dev); > - if (ret) { > + if (ret && ret != -ENOENT) { > dev_err(dev->dev, "Failed to claim SRAM\n"); > > goto err_mem; > -- > 2.37.4 >
Dne sobota, 31. december 2022 ob 17:46:24 CET je Samuel Holland napisal(a): > The Allwinner D1 SoC contains a separate power domain for its video > engine, controlled via the "PPU" power controller. Allow the > power-domains property so this can be represented in the devicetree. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine. > yaml > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine. > yaml index 541325f900a1..d5be7f604e8c 100644 > --- > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine. > yaml +++ > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine. > yaml @@ -63,6 +63,9 @@ properties: > CMA pool to use for buffers allocation instead of the default > CMA pool. > > + power-domains: > + maxItems: 1 Please add check if it's needed based on compatible. This yaml is one of the few for sunxi platform which doesn't have strict checks. Same goes for sram property. Best regards, Jernej > + > required: > - compatible > - reg
Dne četrtek, 05. januar 2023 ob 15:38:36 CET je Samuel Holland napisal(a): > Hi Paul, > > On 1/5/23 04:11, Paul Kocialkowski wrote: > > On Sat 31 Dec 22, 10:46, Samuel Holland wrote: > >> D1 contains a video engine which is supported by the Cedrus driver. > > > > Does it work "outside the box" without power domain management? > > If not, it might be a bit confusing to add the node at this point. > > Yes, it does. All of the power domains are enabled by default. However, > if the PPU series is merged first, I will respin this to include the > power-domains property from the beginning. I would rather see that merged before and having complete node right away. I've been away, but I'll merge everything that's ready for sunxi tree until end of the weekend. Best regards, Jernej > > Regards, > Samuel > > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> > >> arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi > >> b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi index > >> dff363a3c934..4bd374279155 100644 > >> --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi > >> +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi > >> @@ -34,6 +34,17 @@ soc { > >> > >> #address-cells = <1>; > >> #size-cells = <1>; > >> > >> + ve: video-codec@1c0e000 { > >> + compatible = "allwinner,sun20i-d1-video- engine"; > >> + reg = <0x1c0e000 0x2000>; > >> + interrupts = <SOC_PERIPHERAL_IRQ(66) IRQ_TYPE_LEVEL_HIGH>; > >> + clocks = <&ccu CLK_BUS_VE>, > >> + <&ccu CLK_VE>, > >> + <&ccu CLK_MBUS_VE>; > >> + clock-names = "ahb", "mod", "ram"; > >> + resets = <&ccu RST_BUS_VE>; > >> + }; > >> + > >> > >> pio: pinctrl@2000000 { > >> > >> compatible = "allwinner,sun20i-d1-pinctrl"; > >> reg = <0x2000000 0x800>;
Hi Samuel, What is the status of this series? It seems to be mostly OK, but I did see a few comments suggesting improvements. Does this series depend on your PPU work? That was not clear. I do think there were enough small comments to warrant a v2, unless you disagree? Regards, Hans On 31/12/2022 17:46, Samuel Holland wrote: > This series finishes adding Cedrus support for Allwinner D1. I had > tested the hardware and documented the compatible string a while back, > but at the time I had a dummy SRAM section in the devicetree. Further > testing shows that there is no switchable SRAM section -- there is no > need for it, I was unable to guess the address, and the usual bits in > the SRAM controller register have no effect on the video engine. So that > needs to be made optional in the binding and driver. > > With that done, the node can be added to the devicetree. > > > Samuel Holland (4): > media: dt-bindings: cedrus: Allow power domain references > media: dt-bindings: cedrus: Make allwinner,sram optional > media: cedrus: Make SRAM section claiming optional > riscv: dts: allwinner: d1: Add video engine node > > .../media/allwinner,sun4i-a10-video-engine.yaml | 4 +++- > arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 11 +++++++++++ > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 2 +- > 3 files changed, 15 insertions(+), 2 deletions(-) >
Hi Hans, On 1/23/23 06:33, Hans Verkuil wrote: > Hi Samuel, > > What is the status of this series? It seems to be mostly OK, but I did see > a few comments suggesting improvements. > > Does this series depend on your PPU work? That was not clear. The first three patches do not, but the actual DT node does, for completeness. > I do think there were enough small comments to warrant a v2, unless you disagree? I agree, and plan to send a v2 with the binding/driver changes made compatible-specific. Regards, Samuel