Re: [PATCH RFC v3 04/11] arm64: dts: qcom: msm8939: Add venus node

[Bryan O'Donoghue <bod@kernel.org>]

On 27/04/2026 18:58, Erikas Bitovtas wrote:
> +			video-decoder {
> +				compatible = "venus-decoder";
> +				clocks = <&gcc GCC_VENUS0_CORE0_VCODEC0_CLK>,
> +					 <&gcc GCC_VENUS0_CORE1_VCODEC0_CLK>;
> +				clock-names = "core0", "core1";
> +				power-domains = <&gcc VENUS_CORE0_GDSC>,
> +						<&gcc VENUS_CORE1_GDSC>;
> +				power-domain-names = "core0", "core1";
> +			};
> +
> +			video-encoder {
> +				compatible = "venus-encoder";
> +				clocks = <&gcc GCC_VENUS0_CORE0_VCODEC0_CLK>,
> +					 <&gcc GCC_VENUS0_CORE1_VCODEC0_CLK>;
> +				clock-names = "core0", "core1";
> +				power-domains = <&gcc VENUS_CORE0_GDSC>,
> +						<&gcc VENUS_CORE1_GDSC>;
> +				power-domain-names = "core0", "core1";
> +			};

So to be fair in this case you do have a reason to have an encoder and 
decoder compatible here _but_ it should be the case that one one of the 
sub-devices contains CORE0 related stuff and the other CORE1 related stuff.

Because in that case the sub-devices actually represent individual 
hardware settings.

So listing power-domains and clocks for both cores in each node like 
this militates against that.

The other thing is to double check of the encoder and decoder are 
inter-changable here i.e. can either core be encoder or decoder or is it 
fixed ?

I believe on older generations - perhaps not on 8939 it is not 
interchangable.

If it is interchangable then declaring either node specifically encoder 
or decoder is incorrect so you should declare that inline in venus itself.

Perhaps Vikash or Dikshita could confirm whether or not the cores have 
fixed encoder/decoder functionality.

TBH I think you should stick to the inline declaration.

---
bod