Date:   Mon, 23 Jan 2023 11:53:39 -0600
From:   Rob Herring <robh@kernel.org>
To:     Rayyan Ansari <rayyan@ansari.sh>
Cc:     dri-devel@lists.freedesktop.org,
        ~postmarketos/upstreaming@lists.sr.ht, asahi@lists.linux.dev,
        janne@jannau.net, Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@gmail.com>, devicetree@vger.kernel.org,
        Hans de Goede <hdegoede@redhat.com>,
        Javier Martinez Canillas <javierm@redhat.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Thomas Zimmermann <tzimmermann@suse.de>
Subject: Re: [PATCH v2 2/2] dt-bindings: display: simple-framebuffer:
 Document physical width and height properties
Message-ID: <20230123175339.GA2019900-robh@kernel.org>
References: <20230121153544.467126-1-rayyan@ansari.sh>
 <20230121153544.467126-3-rayyan@ansari.sh>
 <CAL_JsqL+G=Cxkc2j_NowznpqNAnixrU+-6SdccFbpMaP6OYSqQ@mail.gmail.com>
 <cdf32cb0-4529-6bbd-fdda-ae641d141ee5@ansari.sh>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cdf32cb0-4529-6bbd-fdda-ae641d141ee5@ansari.sh>
Precedence: bulk

On Sun, Jan 22, 2023 at 05:25:38PM +0000, Rayyan Ansari wrote:
> On 22/01/2023 15:36, Rob Herring wrote:
> > On Sat, Jan 21, 2023 at 9:36 AM Rayyan Ansari <rayyan@ansari.sh> wrote:
> > > 
> > 
> > Why do you need this change?
> > 
> > The 'simple-framebuffer' contains data on how the bootloader
> > configured the display. The bootloader doesn't configure the display
> > size, so this information doesn't belong here. The information should
> > already be in the panel node, so also no point in duplicating it here.
> > 
> > > Signed-off-by: Rayyan Ansari <rayyan@ansari.sh>
> > > ---
> > >   .../devicetree/bindings/display/simple-framebuffer.yaml   | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> 
> Hi Rob,
> 
> There is the usecase that Hans has mentioned, but I have also mentioned
> another usecase previously.
> 
> Adding the width-mm and height-mm properties allows user interfaces such as
> Phosh (https://puri.sm/posts/phosh-overview/) to scale correctly to the
> screen. In my case, a panel node is not available and the aforementioned
> interface is in fact running on the SimpleDRM driver (which binds to the
> simple-framebuffer device).

Why is the panel node not available? Why not add it? Presumably it is 
not there because you aren't (yet) using the simple-panel driver (and 
others that would need). But presumably you will eventually as I'd 
imagine turning the screen off and back on might be a desired feature.

So why add a temporary DT property that's tied to your *current* kernel? 
The DT should not be tightly coupled to the kernel.

Rob