Re: Re: [PATCH RFC v2 04/11] ARM: dts: omap4: Add device tree entry for SGX GPU

[Maxime Ripard]

On Mon, Jan 15, 2024 at 09:55:00AM +0100, H. Nikolaus Schaller wrote:
> Hi,
> 
> > Am 15.01.2024 um 09:25 schrieb Maxime Ripard <mripard@xxxxxxxxxx>:
> > 
> > Hi,
> > 
> > On Fri, Jan 12, 2024 at 06:33:58PM +0100, H. Nikolaus Schaller wrote:
> >>> Am 08.01.2024 um 19:32 schrieb Andrew Davis <afd@xxxxxx>:
> >>> 
> >>> Add SGX GPU device entry to base OMAP4 dtsi file.
> >>> 
> >>> Signed-off-by: Andrew Davis <afd@xxxxxx>
> >>> ---
> >>> arch/arm/boot/dts/ti/omap/omap4.dtsi | 9 +++++----
> >>> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>> 
> >>> diff --git a/arch/arm/boot/dts/ti/omap/omap4.dtsi b/arch/arm/boot/dts/ti/omap/omap4.dtsi
> >>> index 2bbff9032be3e..559b2bfe4ca7c 100644
> >>> --- a/arch/arm/boot/dts/ti/omap/omap4.dtsi
> >>> +++ b/arch/arm/boot/dts/ti/omap/omap4.dtsi
> >>> @@ -501,10 +501,11 @@ sgx_module: target-module@56000000 {
> >>> #size-cells = <1>;
> >>> ranges = <0 0x56000000 0x2000000>;
> >>> 
> >>> - /*
> >>> - * Closed source PowerVR driver, no child device
> >>> - * binding or driver in mainline
> >>> - */
> >>> + gpu@0 {
> >> 
> >> I wonder why we don't add a "gpu:" label here.
> >> 
> >> Almost all other subsystem nodes have one (e.g. emif:, aes:, dss:, dsi:, hdmi:, etc.),
> >> obviously for convenience when using a .dtsi file.
> >> 
> >> It would allow a board-specific DTS to easily add status = "disabled" to avoid driver
> >> probing or disabling the GPU (e.g. if there is no display).
> > 
> > There's no reason to disable it in the DT: the hardware block would
> > still be there and it's rendering to memory so it still could be useful.
> 
> Well, if you know that the board does not have a dm3730 but a dm3725 without
> GPU it is better to disable the GPU completely instead of loading the driver
> and make it detect by some internal bits that it has no GPU on the SoC.

It shouldn't even be in the DTSI if it's not in the SoC.

> > If there's no display on the board and you really don't want the GPU
> > driver, then you can disable the driver or block the module loading, but
> > it should be a distro / package / user decision, not a DT / kernel one
> > still.
> 
> The same holds for aes: dss: dsi: hdmi: etc. If they are not used by some
> board file, they don't change a single bit of the DTB [1] which IMHO would
> be of reasonable concern to question additional labels.

Not really, no. If there's no HDMI connector, the HDMI controller is
useless. A GPU without a display can still be useful, depending on the
workload.

Maxime

Attachment: signature.asc
Description: PGP signature