Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 5 Jun 2023 12:23:53 +0200
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Amit Pundir <amit.pundir@linaro.org>
Cc:     Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
        Caleb Connolly <caleb.connolly@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arm64: dts: qcom: Split sdm845-db845c to add headless
 support
Message-ID: <ZH23uaMD2I249E8I@gerhold.net>
References: <20230605094710.2037879-1-amit.pundir@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230605094710.2037879-1-amit.pundir@linaro.org>
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, Jun 05, 2023 at 03:17:10PM +0530, Amit Pundir wrote:
> This is a follow-up of the upstream discussion,
> https://lore.kernel.org/linux-kernel/20230124182857.1524912-1-amit.pundir@linaro.org/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.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>

AFAIU the DB845c doesn't have a locked ABL. Wouldn't it be cleaner to
add a simple check in the bootloader to see if there is a framebuffer
set up or not and then disable/remove the reserved framebuffer memory
from there? This should be just a couple lines of additional C code in ABL.

It would also prevent that one accidentally uses the headless DTB with
a bootloader splash and gets the UFS crashes you observed.

Out of curiosity, can someone explain what's so special about the
framebuffer memory on newer platforms? Is it protected by the firmware
to disallow other devices like UFS to access it? On older platforms like
MSM8916 the bootloader framebuffer memory isn't really special. Without
reservation there can be temporary garbage displayed on the screen, but
it's gone as soon as the MDP driver is loaded.

Thanks,
Stephan