On Mon, 5 Jun 2023 at 16:07, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 05/06/2023 11:47, Amit Pundir wrote: > > This is a follow-up of the upstream discussion, > > https://lore.kernel.org/linux-kernel/20230124182857.1524912-1-amit.pundir@xxxxxxxxxx/T/#u, > > around adding a reserved memory region in sdm845-db845c > > for the framebuffer memory (the splash region set up by > > the bootloader) but the general opinion was to avoid > > adding that reserved memory for the headless DB845c > > usecase. > > > > So this patch splits the sdm845-db845c into a common dtsi, > > a new sdm845-db845-headless DT, which disables the mdss > > display subsystem, and a new sdm845-db845c DT with an > > additional reserved-memory region for the framebuffer. > > > > The default sdm845-db845c.dtb remains pretty much the same > > (with an exception of additional reserved-memory region), > > while others can use sdm845-db845c-headless.dtb for their > > headless systems. > > You should describe the hardware in commit msg. What is "headless"? If > no HDMI plugged in, then it is a NAK because plug or unplugged HDMI > cable is not a property of a DTS. Hi, my only intended goal [1] is to add a reserved-memory region (specific to the framebuffer memory reserved by the bootloader for the boot splash) in sdm45-db845c. But I have been told that reserving these 30+ MBs for every DB845c is a waste in case we are not using display at all (a headless system?). So I prepared this patch for RFC. The definition of headless is ambiguous to me as well. It could be no HDMI plugged in or no display drivers enabled at all. I believe it all comes down to specific use cases. As far as my use-case (vanilla AOSP) is concerned, display is a critical component and device doesn't boot to completion unless it is hooked to any kind of display. May I suggest we go back to my original change [1] of adding a reserved-memory region for now and let the users of a headless system, define or come up with their respective use-case? Regards, Amit Pundir [1] https://lore.kernel.org/linux-kernel/20230124182857.1524912-1-amit.pundir@xxxxxxxxxx/T/#u > > > > > Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx> > > --- > > Please pick this after the lvs regulator nodes reordering patch > > https://lore.kernel.org/lkml/20230602161246.1855448-1-amit.pundir@xxxxxxxxxx/T/#u. > > I'll rebase and resend this patch otherwise. > > > > arch/arm64/boot/dts/qcom/Makefile | 1 + > > .../boot/dts/qcom/sdm845-db845c-common.dtsi | 1178 +++++++++++++++++ > > .../boot/dts/qcom/sdm845-db845c-headless.dts | 9 + > > arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 1172 +--------------- > > A lot of duplication. Are you sure you generated the patches with > correct -M/-C/-B arguments to rename or copy? > > > 4 files changed, 1194 insertions(+), 1166 deletions(-) > > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-db845c-common.dtsi > > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-db845c-headless.dts > > > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > > index 4f9e81253e18..22876ea2e409 100644 > > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -166,6 +166,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r1.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb > > +dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c-headless.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c-navigation-mezzanine.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm845-lg-judyln.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm845-lg-judyp.dtb > > diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-db845c-common.dtsi > > new file mode 100644 > > index 000000000000..ecc4a851e29c > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-common.dtsi > > @@ -0,0 +1,1178 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2019, Linaro Ltd. > > + */ > > + > > +/dts-v1/; > > + > > +#include <dt-bindings/leds/common.h> > > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > +#include <dt-bindings/sound/qcom,q6afe.h> > > +#include <dt-bindings/sound/qcom,q6asm.h> > > +#include "sdm845.dtsi" > > +#include "sdm845-wcd9340.dtsi" > > +#include "pm8998.dtsi" > > +#include "pmi8998.dtsi" > > + > > +/ { > > + model = "Thundercomm Dragonboard 845c"; > > + compatible = "thundercomm,db845c", "qcom,sdm845"; > > So it is the same hardware? Why compatible is in common part? I don't > understand this change. > > Best regards, > Krzysztof >
