Re: [PATCH] arm64: dts: mediatek: mt8173-elm: remove panel model number in DT

From: AngeloGioacchino Del Regno
Date: Mon May 29 2023 - 04:02:41 EST


Il 26/05/23 16:24, Doug Anderson ha scritto:
Hi,

On Fri, May 26, 2023 at 3:09 AM Icenowy Zheng <uwu@xxxxxxxxxx> wrote:

Currently a specific panel number is used in the Elm DTSI, which is
corresponded to a 12" panel. However, according to the official Chrome
OS devices document, Elm refers to Acer Chromebook R13, which, as the
name specifies, uses a 13.3" panel, which comes with EDID information.

As the kernel currently prioritizes the hardcoded timing parameters
matched with the panel number compatible, a wrong timing will be applied
to the 13.3" panel on Acer Chromebook R13, which leads to blank display.

Because the Elm DTSI is shared with Hana board, and Hana corresponds to
multiple devices from 11" to 14", a certain panel model number shouldn't
be present, and driving the panel according to its EDID information is
necessary.

Signed-off-by: Icenowy Zheng <uwu@xxxxxxxxxx>
---
arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

We went through a bunch of back-and-forth here but in the end in the
ChromeOS tree we have "edp-panel" as the "compatible" here in the
ChromeOS 5.15 tree and this makes sense.

Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

...in theory one would wish for a "Fixes" tag, but I think in previous
discussions it was decided that it was too complicated. Hardcoding the
other compatible string has always been technically wrong, but I guess
it worked at some point in time. The more correct way (as you're doing
here) needs the DP AUX bus support and the generic eDP panels, both of
which are significantly newer than the elm dts. So I guess leaving no
"Fixes" tag is OK, or perhaps you could do the somewhat weak:

Fixes: c2d94f72140a ("arm64: dts: mediatek: mt8173-elm: Move display
to ps8640 auxiliary bus")

I remember I didn't change the compatible to panel-edp because it didn't
work at that time, but it does now... I'm not sure what actually fixed that
and if the commit(s) was/were backported to that suggested point, so I
would leave the Fixes tag out, as that may break older kernels.

Anyway, for this commit:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>