Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
To:     Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
        Rob Herring <robh@kernel.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Heiko Stuebner <heiko@sntech.de>,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] arm64: dts: rockchip: Add Hantro encoder node to rk356x
Date:   Fri, 13 May 2022 18:36:30 +0200
Message-ID: <42373378.kc9IRrpb2S@archbook>
In-Reply-To: <20220513152358.GA334264-robh@kernel.org>
References: <20220508202544.501981-1-frattaroli.nicolas@gmail.com> <CAAEAJfDMG9ykJQHUf6oB5gjV_J0M+eycCqiUvDv2WjpC3g8dKQ@mail.gmail.com> <20220513152358.GA334264-robh@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

Hi Rob,

On Freitag, 13. Mai 2022 17:23:58 CEST Rob Herring wrote:
> On Fri, May 13, 2022 at 10:07:51AM -0300, Ezequiel Garcia wrote:
> > [...]
> > 
> > I didn't ignore it, I just didn't reply to it. You think this is about
> > changing a dt-binding, but you are actually introducing a new dt-binding
> > since you are adding a new compatible string.
> > 
> > You are doing so by extending an existing dt-binding.
> > 
> > I am explaining you the _existing_ dt-binding models the (incorrect) idea
> > of a combined decoder and encoder. Since your device is encoder-only
> > and has a single interrupt line, you should omit the interrupt-names,
> > because it doesn't not add anything.
> 
> While in general I agree on single entries don't need -names, given just 
> 'vdpu' is already allowed I would go with symmetry here and allow it for 
> the encoder.
> 
> If you wanted to mark 'vdpu' alone as deprecated, then that would be 
> fine too. No need for an if/then schema to disallow interrupt-names 
> either. Eventually, I plan to (optionally) remove deprecated schemas 
> from validation and that would have the effect of requiring 
> interrupt-names to have 2 entries here.
> 
> Rob

After some discussion in the #linux-media IRC channel on OFTC, we've
come to the conclusion that moving forward with a separate binding for
encoder-only instances would probably be a wise move, as the VPU binding
assumes we'll always have a decoder, and breaking this assumption will
just make it more complex the longer we go on not separating these.

While I was writing such a separate binding for v4 of my series, I
discovered that this makes things quite a bit simpler. Especially
looking into the near future, there will be between one to two
additional compatibles that will be added to this binding for the
RK3588, as it has no less than 5 instances capable of encoding
JPEG, and I believe at least one of them is capable of other
formats as well.

Regards,
Nicolas Frattaroli