Received-SPF: pass (google.com: domain of linux-kernel+bounces-197250-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240531-edp-panel-drop-v3-0-4c98b2b95e3a@linaro.org>
 <7428a2f7-befc-6db8-76f4-3ca8dc12d31c@quicinc.com> <CAD=FV=Xcq-p5OxSnDJVF-Wp88ZfXOaOKJmh941ymy-f0wkhdhw@mail.gmail.com>
 <197777e0-e6e1-7004-be27-edb98f8a235e@quicinc.com>
In-Reply-To: <197777e0-e6e1-7004-be27-edb98f8a235e@quicinc.com>
From: Doug Anderson <dianders@chromium.org>
Date: Fri, 31 May 2024 10:29:33 -0700
Message-ID: <CAD=FV=UUmnKuBCwLLjrg69kCi7MTjMDYAVtdWEb3yqeZc=-5iQ@mail.gmail.com>
Subject: Re: [PATCH v3 0/3] drm/panel-edp: remove several legacy compatibles
 used by the driver
To: Jeffrey Hugo <quic_jhugo@quicinc.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Neil Armstrong <neil.armstrong@linaro.org>, 
	Jessica Zhang <quic_jesszhan@quicinc.com>, Sam Ravnborg <sam@ravnborg.org>, 
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, 
	Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, 
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, 
	linux-arm-msm@vger.kernel.org, linux-rockchip@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Fri, May 31, 2024 at 9:51=E2=80=AFAM Jeffrey Hugo <quic_jhugo@quicinc.co=
m> wrote:
>
> On 5/31/2024 10:20 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, May 31, 2024 at 9:18=E2=80=AFAM Jeffrey Hugo <quic_jhugo@quicin=
c.com> wrote:
> >>
> >> On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote:
> >>> There are two ways to describe an eDP panel in device tree. The
> >>> recommended way is to add a device on the AUX bus, ideally using the
> >>> edp-panel compatible. The legacy way is to define a top-level platfor=
m
> >>> device for the panel.
> >>>
> >>> Document that adding support for eDP panels in a legacy way is strong=
ly
> >>> discouraged (if not forbidden at all).
> >>>
> >>> While we are at it, also drop legacy compatible strings and bindings =
for
> >>> five panels. These compatible strings were never used by a DT file
> >>> present in Linux kernel and most likely were never used with the
> >>> upstream Linux kernel.
> >>>
> >>> The following compatibles were never used by the devices supported by
> >>> the upstream kernel and are a subject to possible removal:
> >>>
> >>> - lg,lp097qx1-spa1
> >>> - samsung,lsn122dl01-c01
> >>> - sharp,ld-d5116z01b
> >>
> >> Ok to drop the sharp one I added.  It should be able to be handled by
> >> the (newish) edp-panel, but I think the TI bridge driver needs some wo=
rk
> >> for the specific platform (no I2C connection) to verify.
> >
> > Is the platform supported upstream? If so, which platform is it? Is
> > the TI bridge chip the ti-sn65dsi86? If so, I'm confused how you could
> > use that bridge chip without an i2c connection, but perhaps I'm
> > misunderstanding. :-P
>
> Yes, the platform is upstream.  The 8998 laptops (clamshell).  It is the
> ti-sn65si86.  I suspect the I2C connection was not populated for cost
> reasons, then determined its much more convenient to have it as every
> generation after that I've seen has the I2C.
>
> If you check the datasheet closely, the I2C connection is optional.  You
> can also configure the bridge inband using DSI commands.  This is what
> the FW and Windows does.
>
> So, the DT binding needs to make the I2C property optional (this should
> be backwards compatible).  The driver needs to detect that the I2C
> connection is not provided, and fall back to DSI commands.  Regmap would
> be nice for this, but I got pushback on the proposal.  Then I got
> sidetracked looking at other issues.

Crazy! I'm sure I've skimmed over that part of the ti-sn65dsi86
datasheet before but I don't think I internalized it. I guess if you
did it this way then you'd instantiate it as a platform device instead
of an i2c device and that would be how you'd detect the difference. I
could imagine this being a bit of a challenge to get working in the
driver.