Re: [PATCH 6/9] arm64: tegra: Add VPR placeholder node on Tegra234

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3277 bytes --]

On Tue, Sep 02, 2025 at 05:46:26PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> This node contains two sets of properties, one for the case where the
> VPR is resizable (in which case the VPR region will be dynamically
> allocated at boot time) and another case where the VPR is fixed in size
> and initialized by early firmware.
> 
> The firmware running on the device is responsible for updating the node
> with the real physical address for the fixed VPR case and remove the
> properties needed only for resizable VPR. Similarly, if the VPR is
> resizable, the firmware should remove the "reg" property since it is no
> longer needed.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 34 ++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index df034dbb8285..4d572f5fa0b1 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -28,6 +28,40 @@ aliases {
>  		i2c8 = &dp_aux_ch3_i2c;
>  	};
>  
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		vpr: video-protection-region@0 {
> +			compatible = "nvidia,tegra-video-protection-region";
> +			status = "disabled";
> +			no-map;
> +
> +			/*
> +			 * Two variants exist for this. For fixed VPR, the
> +			 * firmware is supposed to update the "reg" property
> +			 * with the fixed memory region configured as VPR.
> +			 *
> +			 * For resizable VPR we don't care about the exact
> +			 * address and instead want a reserved region to be
> +			 * allocated with a certain size and alignment at
> +			 * boot time.
> +			 *
> +			 * The firmware is responsible for removing the
> +			 * unused set of properties.
> +			 */
> +
> +			/* fixed VPR */
> +			reg = <0x0 0x0 0x0 0x0>;
> +
> +			/* resizable VPR */
> +			size = <0x0 0x70000000>;
> +			alignment = <0x0 0x100000>;
> +			reusable;
> +		};
> +	};

Hi DT maintainers,

I wanted to get some feedback on this type of placeholder DT node. This
doesn't actually validate properly because it contains properties for
both the fixed and resizable VPR variants, which are mutually exclusive.
However, the way that this currently works is that UEFI will remove and
update whatever properties need to change during boot, so the booted
kernel ends up with the correct, non-conflicting information.

The reason why it was done this way is because it simplifies the code in
UEFI to update this node. Also, without this being a placeholder I don't
know what to put into this. There's no "default" for this. One option is
to not have this in the DT at all and completely create it at boot time,
but then it becomes quite difficult to create the phandle references.

While at it, I'm not sure if I properly understand how to correctly name
a reserved-memory region that is dynamically allocated like in the case
of resizable VPR? It doesn't have a base address during boot and the
kernel will allocate memory where it sees fit. Do I just leave out the
unit-address in that case?

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]