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;
Feedback-ID: i8771445c:Fastmail
Date:   Tue, 4 Apr 2023 18:04:19 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Michael Riesch <michael.riesch@wolfvision.net>
Cc:     Rob Herring <robh@kernel.org>,
        Gerald Loacker <gerald.loacker@wolfvision.net>,
        dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Thierry Reding <thierry.reding@gmail.com>,
        Sam Ravnborg <sam@ravnborg.org>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: Re: [PATCH 7/7] dt-bindings: display: add panel-timing property to
 sitronix,st7789v
Message-ID: <20230404160419.xlnlxq7sbsqopfwo@houat>
References: <20230314115644.3775169-1-gerald.loacker@wolfvision.net>
 <20230314115644.3775169-8-gerald.loacker@wolfvision.net>
 <20230316215735.GA3940832-robh@kernel.org>
 <dd26836f-d54c-65d1-0acc-8a09745bb066@wolfvision.net>
 <20230329091636.mu6ml3gvw5mvkhm4@penduick>
 <d5a31f75-eb93-0ff2-cd5b-19cdec58e103@wolfvision.net>
 <20230330145811.asot2cvux4ebbeqy@penduick>
 <5f1f90e2-eee8-d941-e4b0-7f2411a9d415@wolfvision.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="v7utqc2px55llm33"
Content-Disposition: inline
In-Reply-To: <5f1f90e2-eee8-d941-e4b0-7f2411a9d415@wolfvision.net>
Precedence: bulk


--v7utqc2px55llm33
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 31, 2023 at 11:36:43AM +0200, Michael Riesch wrote:
> On 3/30/23 16:58, Maxime Ripard wrote:
> > On Wed, Mar 29, 2023 at 12:08:50PM +0200, Michael Riesch wrote:
> >> On 3/29/23 11:16, Maxime Ripard wrote:
> >>> On Thu, Mar 16, 2023 at 11:29:53PM +0100, Michael Riesch wrote:
> >>>> Hi Rob,
> >>>>
> >>>> On 3/16/23 22:57, Rob Herring wrote:
> >>>>> On Tue, Mar 14, 2023 at 12:56:44PM +0100, Gerald Loacker wrote:
> >>>>>> The sitronix-st7789v driver now considers the panel-timing propert=
y.
> >>>>>
> >>>>> I read the patch for that and still don't know 'why'. Commit messag=
es=20
> >>>>> should answer why.
> >>>>>
> >>>>>> Add the property to the documentation.
> >>>>>
> >>>>> We generally don't put timings in DT for panels. Why is this one=20
> >>>>> special?
> >>>>
> >>>> For now, having the timings in the device tree allows for setting the
> >>>> hsync/vsync/de polarity.
> >>>>
> >>>> As a next step, we aim to implement the partial mode feature of this
> >>>> panel. It is possible to use only a certain region of the panel, whi=
ch
> >>>> is helpful e.g., when a part of the panel is occluded and should not=
 be
> >>>> considered by DRM. We thought that this could be specified as timing=
 in DT.
> >>>>
> >>>> (The hactive and vactive properties serve as dimensions of this cert=
ain
> >>>> region, of course. We still need to specify somehow the position of =
the
> >>>> region. Maybe with additional properties hactive-start and vactive-s=
tart?)
> >>>>
> >>>> What do you think about that?
> >>>
> >>> I don't see why we would need the device tree to support that. What y=
ou
> >>> described is essentially what overscan is for HDMI/analog output, and=
 we
> >>> already have everything to deal with overscan properly in KMS.
> >>
> >> Thanks for your response, but I am afraid I don't quite follow.
> >>
> >> How are we supposed to expose control over the hsync/vsync/data enable
> >> polarity? I only know that the display controller and the panel need to
> >> agree on a setting that works for both. What is the canonical way to do
> >> this?
> >=20
> > So typically, it would come from the panel datasheet and would thus be
> > exposed by the panel driver. st7789v is not a panel itself but a (pretty
> > flexible) panel controller so it's not fixed and I don't think we have a
> > good answer to that (yet).
>=20
> Then it seems to me that creating a panel driver (=3D st8879v panel
> controller driver with a new compatible) would make sense.

I don't see why? The entire controller is the same except (maybe) for
some initialization data. Doing a new driver for it seems like taking
the easy way out?

> By coincidence Sebastian Reichel has come up with this approach
> recently, see
> https://lore.kernel.org/dri-devel/20230317232355.1554980-1-sre@kernel.org/
> We just need a way to resolve the conflicts between the two series.
>=20
> Cc: Sebastian

That's not a new driver though? That approach looks sane to me.

> >> A different question is the partial mode, for which (IIUC) you suggest
> >> the overscan feature. As I have never heard of this before, it would be
> >> very nice if you could point me to some examples. Where would the
> >> effective resolution be set in this case?
> >=20
> > So, back when CRT were a thing the edges of the tube were masked by the
> > plastic case. HDMI inherited from that and that's why you still have
> > some UI on some devices (like consoles) to setup the active area of the
> > display.
> >=20
> > The underlying issue is exactly what you describe: the active area is
> > larger than what the plastic case allows to see. I don't think anyone
> > ever had the usecase you have, but it would be the right solution to me
> > to solve essentially the same issue the same way we do on other output
> > types.
>=20
> OK, we'll look into the overscan feature. But still the information
> about the active area should come from the driver, right?

No, the userspace is in charge there.

Maxime

--v7utqc2px55llm33
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCZCxKgwAKCRDj7w1vZxhR
xVipAP9Iuopj25k9qKGAx0RxPd382YqPkKlLJHe75NBMZWCIEgEA+Yk5S6h0cLuD
lqjyBNtPVjpd1r1ePFYCbs/04ufdNwQ=
=tyEC
-----END PGP SIGNATURE-----

--v7utqc2px55llm33--