Received-SPF: pass (google.com: domain of linux-kernel+bounces-187724-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240523-edp-panel-drop-v1-1-045d62511d09@linaro.org>
In-Reply-To: <20240523-edp-panel-drop-v1-1-045d62511d09@linaro.org>
From: Doug Anderson <dianders@chromium.org>
Date: Thu, 23 May 2024 08:36:39 -0700
Message-ID: <CAD=FV=Xz1VsF8jG0q-Ldk+p=NY-ChJkTk0iW8fq2bO=oRFvXRA@mail.gmail.com>
Subject: Re: [PATCH RFC] drm/panel-edp: add fat warning against adding new
 panel compatibles
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: 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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Wed, May 22, 2024 at 3:07=E2=80=AFPM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Add a fat warning against adding new panel compatibles to the panel-edp
> driver. All new users of the eDP panels are supposed to use the generic
> "edp-panel" compatible device on the AUX bus. The remaining compatibles
> are either used by the existing DT or were used previously and are
> retained for backwards compatibility.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> The following compatibles were never used by the devices supported by
> the upstream kernel and are a subject to possible removal:
>
> - auo,b133han05

Ack. Looks like this was added by Bjorn but by the time the dts for
the board (from context, sc8180x-primus) using it made it upstream it
was using "edp-panel".


> - auo,b140han06

Ack. Same as above, but this time the board was "sc8180x-lenovo-flex-5g".


> - ivo,m133nwf4-r0

Ack. Also added by Bjorn but not easy to tell from context which board
it was from. "m133nwf4" never shows up in the history of arm64 qcom
dts files.


> - lg,lp097qx1-spa1

Maybe. Added by Yakir at Rockchip. I don't think this was ChromeOS
related so I don't have any inside knowledge. Presumably this is for
some other hardware they were using. Probably worth CCing Rockchip
mailing list. It's entirely possible that they have downstream dts
files using this and there's no requirement to upstream dts files that
I'm aware of.


> - lg,lp129qe

NAK. See "arch/arm/boot/dts/nvidia/tegra124-venice2.dts"


> - samsung,lsn122dl01-c01

Maybe. Also by Yakir at Rockchip and also doesn't appear to be
ChromeOS, so you should ask them.


> - samsung,ltn140at29-301

NAK. arch/arm/boot/dts/nvidia/tegra124-nyan-blaze.dts


> - sharp,ld-d5116z01b

Added by Jeffrey Hugo. Maybe Cc him to make sure it's OK to delete?


> - sharp,lq140m1jw46

Ack. Feel free to delete. This was used in the sc7280 reference board
(hoglin/zoglin) and by the time the dts made it upstream it was
already using generic edp-panel.


> - starry,kr122ea0sra

Ack. From my previous notes: "starry,kr122ea0sra - rk3399-gru-gru
(reference board, not upstream)". Nobody needs this.


> I'm considering dropping them, unless there is a good reason not to do
> so.
> ---
>  drivers/gpu/drm/panel/panel-edp.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/pa=
nel-edp.c
> index 6db277efcbb7..95b25ec67168 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1776,7 +1776,23 @@ static const struct of_device_id platform_of_match=
[] =3D {
>         {
>                 /* Must be first */
>                 .compatible =3D "edp-panel",
> -       }, {
> +       },
> +       /*
> +        * Do not add panels to the list below unless they cannot be hand=
led by
> +        * the generic edp-panel compatible.
> +        *
> +        * The only two valid reasons are:
> +        * - because of the panel issues (e.g. broken EDID or broken
> +        *   identification),
> +        * - because the platform which uses the panel didn't wire up the=
 AUX
> +        *   bus properly.

You mean that they physically didn't wire up the AUX bus? I don't
think that's actually possible. I don't believe you can use an eDP
panel without this working. Conceivably a reason to add here is if
there's some old platform that hasn't coded up support for AUX bus.
..but it would be much preferred to update the platform driver to
support it.

-Doug