[PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding

[PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding

[Douglas Anderson]

The goal of this patch series is to move away from hardcoding exact
eDP panels in device tree files. As discussed in the various patches
in this series (I'm not repeating everything here), most eDP panels
are 99% probable and we can get that last 1% by allowing two "power
up" delays to be specified in the device tree file and then using the
panel ID (found in the EDID) to look up additional power sequencing
delays for the panel.

This patch series is the logical contiunation of a previous patch
series where I proposed solving this problem by adding a
board-specific compatible string [1]. In the discussion that followed
it sounded like people were open to something like the solution
proposed in this new series.

In version 2 I got rid of the idea that we could have a "fallback"
compatible string that we'd use if we didn't recognize the ID in the
EDID. This simplifies the bindings a lot and the implementation
somewhat. As a result of not having a "fallback", though, I'm not
confident in transitioning any existing boards over to this since
we'll have to fallback to very conservative timings if we don't
recognize the ID from the EDID and I can't guarantee that I've seen
every panel that might have shipped on an existing product. The plan
is to use "edp-panel" only on new boards or new revisions of old
boards where we can guarantee that every EDID that ships out of the
factory has an ID in the table.

Version 3 of this series now splits out all eDP panels to their own
driver and adds the generic eDP panel support to this new driver. I
believe this is what Sam was looking for [2].

[1] https://lore.kernel.org/r/YFKQaXOmOwYyeqvM@google.com/
[2] https://lore.kernel.org/r/YRTsFNTn%2FT8fLxyB@ravnborg.org/

Changes in v3:
- Decode hex product ID w/ same endianness as everyone else.
- ("Reorder logicpd_type_28...") patch new for v3.
- Split eDP panels patch new for v3.
- Move wayward panels patch new for v3.
- ("Non-eDP panels don't need "HPD" handling") new for v3.
- Split the delay structure out patch just on eDP now.
- ("Better describe eDP panel delays") new for v3.
- Fix "prepare_to_enable" patch new for v3.
- ("Don't re-read the EDID every time") moved to eDP only patch.
- Generic "edp-panel" handled by the eDP panel driver now.
- Change init order to we power at the end.
- Adjust endianness of product ID.
- Fallback to conservative delays if panel not recognized.
- Add Sharp LQ116M1JW10 to table.
- Add AUO B116XAN06.1 to table.
- Rename delays more generically so they can be reused.

Changes in v2:
- No longer allow fallback to panel-simple.
- Add "-ms" suffix to delays.
- Don't support a "fallback" panel. Probed panels must be probed.
- Not based on patch to copy "desc"--just allocate for probed panels.
- Add "-ms" suffix to delays.

Douglas Anderson (16):
  dt-bindings: drm/panel-simple-edp: Introduce generic eDP panels
  drm/edid: Break out reading block 0 of the EDID
  drm/edid: Allow the querying/working with the panel ID from the EDID
  drm/panel-simple: Reorder logicpd_type_28 / mitsubishi_aa070mc01
  drm/panel-simple-edp: Split eDP panels out of panel-simple
  ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP
  arm64: defconfig: Everyone who had PANEL_SIMPLE now gets
    PANEL_SIMPLE_EDP
  MIPS: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP
  drm/panel-simple-edp: Move some wayward panels to the eDP driver
  drm/panel-simple: Non-eDP panels don't need "HPD" handling
  drm/panel-simple-edp: Split the delay structure out
  drm/panel-simple-edp: Better describe eDP panel delays
  drm/panel-simple-edp: hpd_reliable shouldn't be subtraced from
    hpd_absent
  drm/panel-simple-edp: Fix "prepare_to_enable" if panel doesn't handle
    HPD
  drm/panel-simple-edp: Don't re-read the EDID every time we power off
    the panel
  drm/panel-simple-edp: Implement generic "edp-panel"s probed by EDID

 .../bindings/display/panel/panel-edp.yaml     |  188 ++
 arch/arm/configs/at91_dt_defconfig            |    1 +
 arch/arm/configs/exynos_defconfig             |    1 +
 arch/arm/configs/imx_v6_v7_defconfig          |    1 +
 arch/arm/configs/lpc32xx_defconfig            |    1 +
 arch/arm/configs/multi_v5_defconfig           |    1 +
 arch/arm/configs/multi_v7_defconfig           |    1 +
 arch/arm/configs/omap2plus_defconfig          |    1 +
 arch/arm/configs/qcom_defconfig               |    1 +
 arch/arm/configs/realview_defconfig           |    1 +
 arch/arm/configs/sama5_defconfig              |    1 +
 arch/arm/configs/shmobile_defconfig           |    1 +
 arch/arm/configs/sunxi_defconfig              |    1 +
 arch/arm/configs/tegra_defconfig              |    1 +
 arch/arm/configs/versatile_defconfig          |    1 +
 arch/arm/configs/vexpress_defconfig           |    1 +
 arch/arm64/configs/defconfig                  |    1 +
 arch/mips/configs/qi_lb60_defconfig           |    1 +
 arch/mips/configs/rs90_defconfig              |    1 +
 drivers/gpu/drm/drm_edid.c                    |  121 +-
 drivers/gpu/drm/panel/Kconfig                 |   16 +-
 drivers/gpu/drm/panel/Makefile                |    1 +
 drivers/gpu/drm/panel/panel-simple-edp.c      | 1895 +++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c          | 1100 +---------
 include/drm/drm_edid.h                        |   47 +
 25 files changed, 2293 insertions(+), 1093 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-edp.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-simple-edp.c

-- 
2.33.0.259.gc128427fd7-goog

[PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Douglas Anderson]

In the patch ("drm/panel-simple-edp: Split eDP panels out of
panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
give everyone who had the old driver enabled the new driver too. If
folks want to opt-out of one or the other they always can later.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

(no changes since v1)

 arch/arm/configs/at91_dt_defconfig   | 1 +
 arch/arm/configs/exynos_defconfig    | 1 +
 arch/arm/configs/imx_v6_v7_defconfig | 1 +
 arch/arm/configs/lpc32xx_defconfig   | 1 +
 arch/arm/configs/multi_v5_defconfig  | 1 +
 arch/arm/configs/multi_v7_defconfig  | 1 +
 arch/arm/configs/omap2plus_defconfig | 1 +
 arch/arm/configs/qcom_defconfig      | 1 +
 arch/arm/configs/realview_defconfig  | 1 +
 arch/arm/configs/sama5_defconfig     | 1 +
 arch/arm/configs/shmobile_defconfig  | 1 +
 arch/arm/configs/sunxi_defconfig     | 1 +
 arch/arm/configs/tegra_defconfig     | 1 +
 arch/arm/configs/versatile_defconfig | 1 +
 arch/arm/configs/vexpress_defconfig  | 1 +
 15 files changed, 15 insertions(+)

diff --git a/arch/arm/configs/at91_dt_defconfig b/arch/arm/configs/at91_dt_defconfig
index b1564e0aa000..3c92ba8c850d 100644
--- a/arch/arm/configs/at91_dt_defconfig
+++ b/arch/arm/configs/at91_dt_defconfig
@@ -144,6 +144,7 @@ CONFIG_VIDEO_MT9V032=m
 CONFIG_DRM=y
 CONFIG_DRM_ATMEL_HLCDC=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_FB_ATMEL=y
 CONFIG_BACKLIGHT_ATMEL_LCDC=y
 CONFIG_BACKLIGHT_PWM=y
diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index f4e1873912a3..3fc348d5765d 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -227,6 +227,7 @@ CONFIG_DRM_EXYNOS_DPI=y
 CONFIG_DRM_EXYNOS_DSI=y
 CONFIG_DRM_EXYNOS_HDMI=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_PANEL_SAMSUNG_LD9040=y
 CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03=y
 CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0=y
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 079fcd8d1d11..ece13c0dc153 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -280,6 +280,7 @@ CONFIG_DRM=y
 CONFIG_DRM_MSM=y
 CONFIG_DRM_PANEL_LVDS=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_PANEL_SEIKO_43WVF1G=y
 CONFIG_DRM_TI_TFP410=y
 CONFIG_DRM_DW_HDMI_AHB_AUDIO=m
diff --git a/arch/arm/configs/lpc32xx_defconfig b/arch/arm/configs/lpc32xx_defconfig
index 989bcc84e7fb..86db9cdced97 100644
--- a/arch/arm/configs/lpc32xx_defconfig
+++ b/arch/arm/configs/lpc32xx_defconfig
@@ -108,6 +108,7 @@ CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_DRM=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_PL111=y
 CONFIG_FB_MODE_HELPERS=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
diff --git a/arch/arm/configs/multi_v5_defconfig b/arch/arm/configs/multi_v5_defconfig
index 80a3ae02d759..fab163305918 100644
--- a/arch/arm/configs/multi_v5_defconfig
+++ b/arch/arm/configs/multi_v5_defconfig
@@ -194,6 +194,7 @@ CONFIG_VIDEO_ATMEL_ISI=m
 CONFIG_DRM=y
 CONFIG_DRM_ATMEL_HLCDC=m
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_ASPEED_GFX=m
 CONFIG_FB_IMX=y
 CONFIG_FB_ATMEL=y
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index d9abaae118dd..d299d0045823 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -699,6 +699,7 @@ CONFIG_DRM_TEGRA=y
 CONFIG_DRM_STM=m
 CONFIG_DRM_STM_DSI=m
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_PANEL_SAMSUNG_LD9040=m
 CONFIG_DRM_PANEL_ORISETECH_OTM8009A=m
 CONFIG_DRM_PANEL_RAYDIUM_RM68200=m
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index 2ac2418084ab..dcc55aa62d69 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -511,6 +511,7 @@ CONFIG_OMAP2_DSS_DSI=y
 CONFIG_DRM_TILCDC=m
 CONFIG_DRM_PANEL_DSI_CM=m
 CONFIG_DRM_PANEL_SIMPLE=m
+CONFIG_DRM_PANEL_SIMPLE_EDP=m
 CONFIG_DRM_PANEL_LG_LB035Q02=m
 CONFIG_DRM_PANEL_NEC_NL8048HL11=m
 CONFIG_DRM_PANEL_SHARP_LS037V7DW01=m
diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 26353cbfa968..37116db013f8 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -158,6 +158,7 @@ CONFIG_MEDIA_SUPPORT=y
 CONFIG_DRM=y
 CONFIG_DRM_MSM=m
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_ANALOGIX_ANX78XX=m
 CONFIG_FB=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
diff --git a/arch/arm/configs/realview_defconfig b/arch/arm/configs/realview_defconfig
index 4c01e313099f..c433890fc4e9 100644
--- a/arch/arm/configs/realview_defconfig
+++ b/arch/arm/configs/realview_defconfig
@@ -61,6 +61,7 @@ CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_DRM=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_DISPLAY_CONNECTOR=y
 CONFIG_DRM_SIMPLE_BRIDGE=y
 CONFIG_DRM_PL111=y
diff --git a/arch/arm/configs/sama5_defconfig b/arch/arm/configs/sama5_defconfig
index 17db3b3e2dd3..c2ab428e6327 100644
--- a/arch/arm/configs/sama5_defconfig
+++ b/arch/arm/configs/sama5_defconfig
@@ -160,6 +160,7 @@ CONFIG_VIDEO_MT9V032=m
 CONFIG_DRM=y
 CONFIG_DRM_ATMEL_HLCDC=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
 CONFIG_BACKLIGHT_PWM=y
diff --git a/arch/arm/configs/shmobile_defconfig b/arch/arm/configs/shmobile_defconfig
index d9a27e4e0914..3b3e9a16c956 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -129,6 +129,7 @@ CONFIG_VIDEO_ML86V7667=y
 CONFIG_DRM=y
 CONFIG_DRM_RCAR_DU=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_DISPLAY_CONNECTOR=y
 CONFIG_DRM_LVDS_CODEC=y
 CONFIG_DRM_SII902X=y
diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index a60c134c5e04..f6b4f6684631 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -108,6 +108,7 @@ CONFIG_DRM_SUN4I_HDMI_CEC=y
 CONFIG_DRM_SUN8I_DW_HDMI=y
 CONFIG_DRM_PANEL_LVDS=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_SIMPLE_BRIDGE=y
 CONFIG_DRM_LIMA=y
 CONFIG_FB_SIMPLE=y
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 3d8d8af9524d..918134415254 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -204,6 +204,7 @@ CONFIG_DRM_TEGRA=y
 CONFIG_DRM_TEGRA_STAGING=y
 CONFIG_DRM_PANEL_LVDS=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_LVDS_CODEC=y
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
diff --git a/arch/arm/configs/versatile_defconfig b/arch/arm/configs/versatile_defconfig
index b703f4757021..f424671523a9 100644
--- a/arch/arm/configs/versatile_defconfig
+++ b/arch/arm/configs/versatile_defconfig
@@ -57,6 +57,7 @@ CONFIG_GPIO_PL061=y
 CONFIG_DRM=y
 CONFIG_DRM_PANEL_ARM_VERSATILE=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_DISPLAY_CONNECTOR=y
 CONFIG_DRM_SIMPLE_BRIDGE=y
 CONFIG_DRM_PL111=y
diff --git a/arch/arm/configs/vexpress_defconfig b/arch/arm/configs/vexpress_defconfig
index b5e246dd23f4..baf9c7810a14 100644
--- a/arch/arm/configs/vexpress_defconfig
+++ b/arch/arm/configs/vexpress_defconfig
@@ -77,6 +77,7 @@ CONFIG_SENSORS_VEXPRESS=y
 CONFIG_REGULATOR_VEXPRESS=y
 CONFIG_DRM=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_PANEL_SIMPLE_EDP=y
 CONFIG_DRM_SII902X=y
 CONFIG_DRM_PL111=y
 CONFIG_FB=y
-- 
2.33.0.259.gc128427fd7-goog

Re: [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Olof Johansson]

On Wed, Sep 1, 2021 at 1:20 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> In the patch ("drm/panel-simple-edp: Split eDP panels out of
> panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
> give everyone who had the old driver enabled the new driver too. If
> folks want to opt-out of one or the other they always can later.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Isn't this a case where the new option should just have had the old
option as the default value to avoid this kind of churn and possibly
broken platforms?


-Olof

Re: [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Doug Anderson]

Hi,

On Wed, Sep 1, 2021 at 2:12 PM Olof Johansson <olof@lixom.net> wrote:
>
> On Wed, Sep 1, 2021 at 1:20 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > In the patch ("drm/panel-simple-edp: Split eDP panels out of
> > panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
> > give everyone who had the old driver enabled the new driver too. If
> > folks want to opt-out of one or the other they always can later.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> Isn't this a case where the new option should just have had the old
> option as the default value to avoid this kind of churn and possibly
> broken platforms?

I'm happy to go either way. I guess I didn't do that originally
because logically there's not any reason to link the two drivers going
forward. Said another way, someone enabling the "simple panel" driver
for non-eDP panels wouldn't expect that the "simple panel" driver for
DP panels would also get enabled by default. They really have nothing
to do with one another. Enabling by default for something like this
also seems like it would lead to bloat. I could have sworn that
periodically people get yelled at for marking drivers on by default
when it doesn't make sense.

...that being said, I'm happy to change the default as you suggest.
Just let me know.

-Doug

Re: [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Andrzej Hajda]

On 02.09.2021 01:10, Doug Anderson wrote:
> Hi,
> 
> On Wed, Sep 1, 2021 at 2:12 PM Olof Johansson <olof@lixom.net> wrote:
>>
>> On Wed, Sep 1, 2021 at 1:20 PM Douglas Anderson <dianders@chromium.org> wrote:
>>>
>>> In the patch ("drm/panel-simple-edp: Split eDP panels out of
>>> panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
>>> give everyone who had the old driver enabled the new driver too. If
>>> folks want to opt-out of one or the other they always can later.
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>
>> Isn't this a case where the new option should just have had the old
>> option as the default value to avoid this kind of churn and possibly
>> broken platforms?
> 
> I'm happy to go either way. I guess I didn't do that originally
> because logically there's not any reason to link the two drivers going
> forward. Said another way, someone enabling the "simple panel" driver
> for non-eDP panels wouldn't expect that the "simple panel" driver for
> DP panels would also get enabled by default. They really have nothing
> to do with one another. Enabling by default for something like this
> also seems like it would lead to bloat. I could have sworn that
> periodically people get yelled at for marking drivers on by default
> when it doesn't make sense.
> 
> ...that being said, I'm happy to change the default as you suggest.
> Just let me know.

I guess this is just misunderstanding. Symbol names:
	CONFIG_DRM_PANEL_SIMPLE=y
	CONFIG_DRM_PANEL_SIMPLE_EDP=y
suggests that CONFIG_DRM_PANEL_SIMPLE_EDP is an 'suboption' of 
CONFIG_DRM_PANEL_SIMPLE, but these symbols are independent - old symbol 
has been split into two independent new symbols.
So Doug's approach seems correct to me. Maybe one could change names of 
symbols to avoid confusion(?).

One more thing, I suspect previous patch can break platforms with EDP 
panels. Even if this patch fixes it, maybe it would be better to squash 
these patches? Or add temporal solution to save bisecatability.

Regards
Andrzej

> 
> -Doug
> 

Re: [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Stephen Boyd]

Quoting Doug Anderson (2021-09-01 16:10:15)
> Hi,
> 
> On Wed, Sep 1, 2021 at 2:12 PM Olof Johansson <olof@lixom.net> wrote:
> >
> > On Wed, Sep 1, 2021 at 1:20 PM Douglas Anderson <dianders@chromium.org> wrote:
> > >
> > > In the patch ("drm/panel-simple-edp: Split eDP panels out of
> > > panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
> > > give everyone who had the old driver enabled the new driver too. If
> > > folks want to opt-out of one or the other they always can later.
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >
> > Isn't this a case where the new option should just have had the old
> > option as the default value to avoid this kind of churn and possibly
> > broken platforms?
> 
> I'm happy to go either way. I guess I didn't do that originally
> because logically there's not any reason to link the two drivers going
> forward. Said another way, someone enabling the "simple panel" driver
> for non-eDP panels wouldn't expect that the "simple panel" driver for
> DP panels would also get enabled by default. They really have nothing
> to do with one another. Enabling by default for something like this
> also seems like it would lead to bloat. I could have sworn that
> periodically people get yelled at for marking drivers on by default
> when it doesn't make sense.
> 
> ...that being said, I'm happy to change the default as you suggest.
> Just let me know.

Having the default will help olddefconfig users seamlessly migrate to
the new Kconfig. Sadly they don't notice that they should probably
disable the previous Kconfig symbol, but oh well. At least with the
default they don't go on a hunt/bisect to figure out that some Kconfig
needed to be enabled now that they're using a new kernel version.

Maybe the default should have a TODO comment next to it indicating we
should remove the default in a year or two.

Re: [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Doug Anderson]

Hi,

On Fri, Sep 3, 2021 at 1:38 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Doug Anderson (2021-09-01 16:10:15)
> > Hi,
> >
> > On Wed, Sep 1, 2021 at 2:12 PM Olof Johansson <olof@lixom.net> wrote:
> > >
> > > On Wed, Sep 1, 2021 at 1:20 PM Douglas Anderson <dianders@chromium.org> wrote:
> > > >
> > > > In the patch ("drm/panel-simple-edp: Split eDP panels out of
> > > > panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
> > > > give everyone who had the old driver enabled the new driver too. If
> > > > folks want to opt-out of one or the other they always can later.
> > > >
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > >
> > > Isn't this a case where the new option should just have had the old
> > > option as the default value to avoid this kind of churn and possibly
> > > broken platforms?
> >
> > I'm happy to go either way. I guess I didn't do that originally
> > because logically there's not any reason to link the two drivers going
> > forward. Said another way, someone enabling the "simple panel" driver
> > for non-eDP panels wouldn't expect that the "simple panel" driver for
> > DP panels would also get enabled by default. They really have nothing
> > to do with one another. Enabling by default for something like this
> > also seems like it would lead to bloat. I could have sworn that
> > periodically people get yelled at for marking drivers on by default
> > when it doesn't make sense.
> >
> > ...that being said, I'm happy to change the default as you suggest.
> > Just let me know.
>
> Having the default will help olddefconfig users seamlessly migrate to
> the new Kconfig. Sadly they don't notice that they should probably
> disable the previous Kconfig symbol, but oh well. At least with the
> default they don't go on a hunt/bisect to figure out that some Kconfig
> needed to be enabled now that they're using a new kernel version.
>
> Maybe the default should have a TODO comment next to it indicating we
> should remove the default in a year or two.

OK, so I'm trying to figure out how to do this without just "kicking
the can" down the road. I guess your idea is that for the next year
this will be the default and that anyone who really wants
"CONFIG_DRM_PANEL_EDP" will "opt-in" to keep it by adding
"CONFIG_DRM_PANEL_EDP=y" to their config? ...and then after a year
passes we remove the default? ...but that won't work, will it? Since
"CONFIG_DRM_PANEL_EDP" will be the default for the next year then you
really can't add it to the "defconfig", at least if you ever
"normalize" it. The "defconfig" by definition has everything stripped
from it that's already the "default", so for the next year anyone who
tries to opt-in will get their preference stripped.

Hrm, so let me explain options as I see them. Maybe someone can point
out something that I missed. I'll assume that we'll change the config
option from CONFIG_DRM_PANEL_SIMPLE_EDP to CONFIG_DRM_PANEL_EDP
(remove the "SIMPLE" part).

==

Where we were before my series:

* One config "CONFIG_DRM_PANEL_SIMPLE" and it enables simple non-eDP
and eDP drivers.

==

Option 1: update everyone's configs (this patch)

* Keep old config "CONFIG_DRM_PANEL_SIMPLE" but it now only means
enable the panel-simple (non-eDP) driver.
* Anyone who wants eDP panels must opt-in to "CONFIG_DRM_PANEL_EDP"
* Update all configs in mainline; any out-of mainline configs must
figure this out themselves.

Pros:
* no long term baggage

Cons:
* patch upstream is a bit of "churn"
* anyone with downstream config will have to figure out what happened.

==

Option 2: kick the can down the road + accept cruft

* Keep old config "CONFIG_DRM_PANEL_SIMPLE" and it means enable the
panel-simple (non-eDP) driver.
* Anyone with "CONFIG_DRM_PANEL_SIMPLE" is opted in by default to
"CONFIG_DRM_PANEL_EDP"

AKA:
config DRM_PANEL_EDP
  default DRM_PANEL_SIMPLE

Pros:
* no config patches needed upstream at all and everything just works!

Cons:
* people are opted in to extra cruft by default and need to know to turn it off.
* unclear if we can change the default without the same problems.

==

Option 3: try to be clever

* Add _two_ new configs. CONFIG_DRM_PANEL_SIMPLE_V2 and CONFIG_DRM_PANEL_EDP.
* Old config "CONFIG_DRM_PANEL_SIMPLE" gets marked as "deprecated".
* Both new configs have "default CONFIG_DRM_PANEL_SIMPLE"

Now anyone old will magically get both the new config options by
default. Anyone looking at this in the future _won't_ set the
deprecated CONFIG_DRM_PANEL_SIMPLE but will instead choose if they
want either the eDP or "simple" driver.

Pros:
* No long term baggage.
* Everyone is transitioned automatically by default with no cruft patches.

Cons:
* I can't think of a better name than "CONFIG_DRM_PANEL_SIMPLE_V2" and
that name is ugly.

==

Option 4: shave a yak

When thinking about this I came up with a clever idea of stashing the
kernel version in a defconfig when it's generated. Then you could do
something like:

config DRM_PANEL_EDP
  default DRM_PANEL_SIMPLE if DEFCONFIG_GENERATED_AT <= 0x00050f00

That feels like a good idea to me but who knows what others would
think. In general I think this series already shaves enough yaks. This
isn't a new problem we're trying to solve so it seems like we should
pick one of the options above.

==

Unless I get an explicit NAK from someone like Olof or Arnd or I hear
that everyone loves Option #3 I'll probably just stick with the
existing approach since:

* Olof's wording didn't make it sound like a strong objection.

* From git history it looks as if config patches don't necessarily
land through the SoC tree and thus I'd by default follow the
suggestions of the DRM folks. Andrzej suggested going with the
existing approach as long as I changed the symbol names and re-ordered
the patches.


Please yell if anything above sounds wrong! I'll probably try to send
out a new version tomorrow or the next day, but I won't land it right
away to give people time to yell.


-Doug

Re: [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP

[Olof Johansson]

On Wed, Sep 8, 2021 at 3:36 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Fri, Sep 3, 2021 at 1:38 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Doug Anderson (2021-09-01 16:10:15)
> > > Hi,
> > >
> > > On Wed, Sep 1, 2021 at 2:12 PM Olof Johansson <olof@lixom.net> wrote:
> > > >
> > > > On Wed, Sep 1, 2021 at 1:20 PM Douglas Anderson <dianders@chromium.org> wrote:
> > > > >
> > > > > In the patch ("drm/panel-simple-edp: Split eDP panels out of
> > > > > panel-simple") we split the PANEL_SIMPLE driver in 2. By default let's
> > > > > give everyone who had the old driver enabled the new driver too. If
> > > > > folks want to opt-out of one or the other they always can later.
> > > > >
> > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > >
> > > > Isn't this a case where the new option should just have had the old
> > > > option as the default value to avoid this kind of churn and possibly
> > > > broken platforms?
> > >
> > > I'm happy to go either way. I guess I didn't do that originally
> > > because logically there's not any reason to link the two drivers going
> > > forward. Said another way, someone enabling the "simple panel" driver
> > > for non-eDP panels wouldn't expect that the "simple panel" driver for
> > > DP panels would also get enabled by default. They really have nothing
> > > to do with one another. Enabling by default for something like this
> > > also seems like it would lead to bloat. I could have sworn that
> > > periodically people get yelled at for marking drivers on by default
> > > when it doesn't make sense.
> > >
> > > ...that being said, I'm happy to change the default as you suggest.
> > > Just let me know.
> >
> > Having the default will help olddefconfig users seamlessly migrate to
> > the new Kconfig. Sadly they don't notice that they should probably
> > disable the previous Kconfig symbol, but oh well. At least with the
> > default they don't go on a hunt/bisect to figure out that some Kconfig
> > needed to be enabled now that they're using a new kernel version.
> >
> > Maybe the default should have a TODO comment next to it indicating we
> > should remove the default in a year or two.
>
> OK, so I'm trying to figure out how to do this without just "kicking
> the can" down the road. I guess your idea is that for the next year
> this will be the default and that anyone who really wants
> "CONFIG_DRM_PANEL_EDP" will "opt-in" to keep it by adding
> "CONFIG_DRM_PANEL_EDP=y" to their config? ...and then after a year
> passes we remove the default? ...but that won't work, will it? Since
> "CONFIG_DRM_PANEL_EDP" will be the default for the next year then you
> really can't add it to the "defconfig", at least if you ever
> "normalize" it. The "defconfig" by definition has everything stripped
> from it that's already the "default", so for the next year anyone who
> tries to opt-in will get their preference stripped.
>
> Hrm, so let me explain options as I see them. Maybe someone can point
> out something that I missed. I'll assume that we'll change the config
> option from CONFIG_DRM_PANEL_SIMPLE_EDP to CONFIG_DRM_PANEL_EDP
> (remove the "SIMPLE" part).
>
> ==
>
> Where we were before my series:
>
> * One config "CONFIG_DRM_PANEL_SIMPLE" and it enables simple non-eDP
> and eDP drivers.
>
> ==
>
> Option 1: update everyone's configs (this patch)
>
> * Keep old config "CONFIG_DRM_PANEL_SIMPLE" but it now only means
> enable the panel-simple (non-eDP) driver.
> * Anyone who wants eDP panels must opt-in to "CONFIG_DRM_PANEL_EDP"
> * Update all configs in mainline; any out-of mainline configs must
> figure this out themselves.
>
> Pros:
> * no long term baggage
>
> Cons:
> * patch upstream is a bit of "churn"
> * anyone with downstream config will have to figure out what happened.
>
> ==
>
> Option 2: kick the can down the road + accept cruft
>
> * Keep old config "CONFIG_DRM_PANEL_SIMPLE" and it means enable the
> panel-simple (non-eDP) driver.
> * Anyone with "CONFIG_DRM_PANEL_SIMPLE" is opted in by default to
> "CONFIG_DRM_PANEL_EDP"
>
> AKA:
> config DRM_PANEL_EDP
>   default DRM_PANEL_SIMPLE
>
> Pros:
> * no config patches needed upstream at all and everything just works!
>
> Cons:
> * people are opted in to extra cruft by default and need to know to turn it off.
> * unclear if we can change the default without the same problems.
>
> ==
>
> Option 3: try to be clever
>
> * Add _two_ new configs. CONFIG_DRM_PANEL_SIMPLE_V2 and CONFIG_DRM_PANEL_EDP.
> * Old config "CONFIG_DRM_PANEL_SIMPLE" gets marked as "deprecated".
> * Both new configs have "default CONFIG_DRM_PANEL_SIMPLE"
>
> Now anyone old will magically get both the new config options by
> default. Anyone looking at this in the future _won't_ set the
> deprecated CONFIG_DRM_PANEL_SIMPLE but will instead choose if they
> want either the eDP or "simple" driver.
>
> Pros:
> * No long term baggage.
> * Everyone is transitioned automatically by default with no cruft patches.
>
> Cons:
> * I can't think of a better name than "CONFIG_DRM_PANEL_SIMPLE_V2" and
> that name is ugly.
>
> ==
>
> Option 4: shave a yak
>
> When thinking about this I came up with a clever idea of stashing the
> kernel version in a defconfig when it's generated. Then you could do
> something like:
>
> config DRM_PANEL_EDP
>   default DRM_PANEL_SIMPLE if DEFCONFIG_GENERATED_AT <= 0x00050f00
>
> That feels like a good idea to me but who knows what others would
> think. In general I think this series already shaves enough yaks. This
> isn't a new problem we're trying to solve so it seems like we should
> pick one of the options above.
>
> ==
>
> Unless I get an explicit NAK from someone like Olof or Arnd or I hear
> that everyone loves Option #3 I'll probably just stick with the
> existing approach since:
>
> * Olof's wording didn't make it sound like a strong objection.

Yeah, not a strong objection but an enquiry if there's a better way to
handle it. TL;DR: I don't think there really is.

My comment mostly came from the fact that when olddefconfig gets
broken like this, we tend to have a bunch of patches trickle in over
time as downstream users discover the need to turn on the new option.
You covered (most) of that  by doing the appropriate defconfigs to
this patch series, so it won't be as bad (besides any newly added
defconfigs during the same release, and we're quite careful about
doing that these days).

I think most of the other options, besides 2, are just more overhead
than needed here. So I'd be fine with just picking up option 1.

What's clear is that this is not a very convenient activity that
scales, but we don't do it all that often. This is where something
like a "HAVE_EDP" type config that the platform can provide helps, but
adding it just for this rework seems to be more work than it's worth.

> * From git history it looks as if config patches don't necessarily
> land through the SoC tree and thus I'd by default follow the
> suggestions of the DRM folks. Andrzej suggested going with the
> existing approach as long as I changed the symbol names and re-ordered
> the patches.

Right, Kconfig changes usually go with the driver. dts and defconfig
changes go to the SoC tree though since otherwise we end up with a
bunch of churn and conflicts.

> Please yell if anything above sounds wrong! I'll probably try to send
> out a new version tomorrow or the next day, but I won't land it right
> away to give people time to yell.

I'd leave it up to you if you want to do option 1 or 2, since there's
no really convenient way to do it better. 3 seems to be a bigger
hammer than what this situation calls for IMHO.


-Olof

Re: [PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding

[Andrzej Hajda]

Removed most CC: SMTP server protested.

On 01.09.2021 22:19, Douglas Anderson wrote:
> The goal of this patch series is to move away from hardcoding exact
> eDP panels in device tree files. As discussed in the various patches
> in this series (I'm not repeating everything here), most eDP panels
> are 99% probable and we can get that last 1% by allowing two "power
> up" delays to be specified in the device tree file and then using the
> panel ID (found in the EDID) to look up additional power sequencing
> delays for the panel.
> 
> This patch series is the logical contiunation of a previous patch
> series where I proposed solving this problem by adding a
> board-specific compatible string [1]. In the discussion that followed
> it sounded like people were open to something like the solution
> proposed in this new series.
> 
> In version 2 I got rid of the idea that we could have a "fallback"
> compatible string that we'd use if we didn't recognize the ID in the
> EDID. This simplifies the bindings a lot and the implementation
> somewhat. As a result of not having a "fallback", though, I'm not
> confident in transitioning any existing boards over to this since
> we'll have to fallback to very conservative timings if we don't
> recognize the ID from the EDID and I can't guarantee that I've seen
> every panel that might have shipped on an existing product. The plan
> is to use "edp-panel" only on new boards or new revisions of old
> boards where we can guarantee that every EDID that ships out of the
> factory has an ID in the table.
> 
> Version 3 of this series now splits out all eDP panels to their own
> driver and adds the generic eDP panel support to this new driver. I
> believe this is what Sam was looking for [2].
> 
> [1] https://lore.kernel.org/r/YFKQaXOmOwYyeqvM@google.com/
> [2] https://lore.kernel.org/r/YRTsFNTn%2FT8fLxyB@ravnborg.org/
> 
I like the idea - if something can be configured dynamically lets do it.
But I have few questions:
1. Have you read different real panels id's? In many cases (MIPI DSI, 
LVDS with EDID) manufacturers often forgot to set proper id fields. I do 
not know how EDID's ids are reliable in case of edp panels.
2. You are working with edp panels - beside EDID they have DPCD which 
contains things like IEEE_OUI and "Device Identification String", I 
guess they could be also used for detecting panels, have you considered 
it? I think DPCD Id should be assigned to EDP-Sink interface, and EDID 
Id to the actual panel behind it. With this assumption one could 
consider which timings should be property of which entity.


Regards
Andrzej

Re: [PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding

[Doug Anderson]

Hi,

On Thu, Sep 2, 2021 at 3:10 PM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> Removed most CC: SMTP server protested.

Yeah, it was because of the dang defconfig patches. My general policy
is to send the cover letter to everyone even if not everyone gets CCed
on all patches, but it ended up as a lot... Speaking of which, I'd
definitely be interested in your take on the defconfig topic:

https://lore.kernel.org/r/CAD=FV=WPXAUyuAHb1jKx9F_aw+JGX4MWB3or=Eq5rXoKY=OQMw@mail.gmail.com

Do you agree with Olof that I should set the "default" for the new
config to be the same as the old config? It doesn't make sense going
forward but it helps for anyone with old configs...


> On 01.09.2021 22:19, Douglas Anderson wrote:
> > The goal of this patch series is to move away from hardcoding exact
> > eDP panels in device tree files. As discussed in the various patches
> > in this series (I'm not repeating everything here), most eDP panels
> > are 99% probable and we can get that last 1% by allowing two "power
> > up" delays to be specified in the device tree file and then using the
> > panel ID (found in the EDID) to look up additional power sequencing
> > delays for the panel.
> >
> > This patch series is the logical contiunation of a previous patch
> > series where I proposed solving this problem by adding a
> > board-specific compatible string [1]. In the discussion that followed
> > it sounded like people were open to something like the solution
> > proposed in this new series.
> >
> > In version 2 I got rid of the idea that we could have a "fallback"
> > compatible string that we'd use if we didn't recognize the ID in the
> > EDID. This simplifies the bindings a lot and the implementation
> > somewhat. As a result of not having a "fallback", though, I'm not
> > confident in transitioning any existing boards over to this since
> > we'll have to fallback to very conservative timings if we don't
> > recognize the ID from the EDID and I can't guarantee that I've seen
> > every panel that might have shipped on an existing product. The plan
> > is to use "edp-panel" only on new boards or new revisions of old
> > boards where we can guarantee that every EDID that ships out of the
> > factory has an ID in the table.
> >
> > Version 3 of this series now splits out all eDP panels to their own
> > driver and adds the generic eDP panel support to this new driver. I
> > believe this is what Sam was looking for [2].
> >
> > [1] https://lore.kernel.org/r/YFKQaXOmOwYyeqvM@google.com/
> > [2] https://lore.kernel.org/r/YRTsFNTn%2FT8fLxyB@ravnborg.org/
> >
> I like the idea - if something can be configured dynamically lets do it.
> But I have few questions:
> 1. Have you read different real panels id's? In many cases (MIPI DSI,
> LVDS with EDID) manufacturers often forgot to set proper id fields. I do
> not know how EDID's ids are reliable in case of edp panels.

I have read several and (so far) they have been quite reliable but I
will admit that I haven't done an exhaustive sample. I guess my answer
to whether we can trust it is:

a) Presumably you'd only use this new "edp-panel" compatible on
systems whose panel IDs are known to be reliable. Old systems can keep
determining the panel by compatible string. The two schemes can
co-exist.

b) As we start using this new scheme then there will be more
validation of panel ID strings and, presumably, they will become more
reliable.

It is still true that ID conflicts could exist. A vendor could ship
two different panels with the same ID and maybe nobody would notice
because they weren't ever hooked up to the same board. In that case
we'd have a question of what to do upstream. If this really happens
then I suppose (worst case) we could use the device tree to help
differentiate and each board could say that "panel ID <x> hooked up to
this board is actually panel <y>". I hope we don't have to do that
because it feels dirty, but at least it gives us _something_ if we get
backed into a corner.


> 2. You are working with edp panels - beside EDID they have DPCD which
> contains things like IEEE_OUI and "Device Identification String", I
> guess they could be also used for detecting panels, have you considered
> it? I think DPCD Id should be assigned to EDP-Sink interface, and EDID
> Id to the actual panel behind it. With this assumption one could
> consider which timings should be property of which entity.

This could be another way to help us if we're backed into a corner in
the future. Right now the driver requires that you give access to a
full eDP AUX bus to use the "generic eDP" panel support even though it
currently only relies on the EDID, so it would be pretty easy to
utilize this info in the future. So far the ID has been reliable for
me though.


-Doug

Re: [PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding

[Sam Ravnborg]

Hi Douglas,

On Wed, Sep 01, 2021 at 01:19:18PM -0700, Douglas Anderson wrote:
> The goal of this patch series is to move away from hardcoding exact
> eDP panels in device tree files. As discussed in the various patches
> in this series (I'm not repeating everything here), most eDP panels
> are 99% probable and we can get that last 1% by allowing two "power
> up" delays to be specified in the device tree file and then using the
> panel ID (found in the EDID) to look up additional power sequencing
> delays for the panel.
> 
> This patch series is the logical contiunation of a previous patch
> series where I proposed solving this problem by adding a
> board-specific compatible string [1]. In the discussion that followed
> it sounded like people were open to something like the solution
> proposed in this new series.
> 
> In version 2 I got rid of the idea that we could have a "fallback"
> compatible string that we'd use if we didn't recognize the ID in the
> EDID. This simplifies the bindings a lot and the implementation
> somewhat. As a result of not having a "fallback", though, I'm not
> confident in transitioning any existing boards over to this since
> we'll have to fallback to very conservative timings if we don't
> recognize the ID from the EDID and I can't guarantee that I've seen
> every panel that might have shipped on an existing product. The plan
> is to use "edp-panel" only on new boards or new revisions of old
> boards where we can guarantee that every EDID that ships out of the
> factory has an ID in the table.
> 
> Version 3 of this series now splits out all eDP panels to their own
> driver and adds the generic eDP panel support to this new driver. I
> believe this is what Sam was looking for [2].
> 
> [1] https://lore.kernel.org/r/YFKQaXOmOwYyeqvM@google.com/
> [2] https://lore.kernel.org/r/YRTsFNTn%2FT8fLxyB@ravnborg.org/
> 
> Changes in v3:
> - Decode hex product ID w/ same endianness as everyone else.
> - ("Reorder logicpd_type_28...") patch new for v3.
> - Split eDP panels patch new for v3.
> - Move wayward panels patch new for v3.
> - ("Non-eDP panels don't need "HPD" handling") new for v3.
> - Split the delay structure out patch just on eDP now.
> - ("Better describe eDP panel delays") new for v3.
> - Fix "prepare_to_enable" patch new for v3.
> - ("Don't re-read the EDID every time") moved to eDP only patch.
> - Generic "edp-panel" handled by the eDP panel driver now.
> - Change init order to we power at the end.
> - Adjust endianness of product ID.
> - Fallback to conservative delays if panel not recognized.
> - Add Sharp LQ116M1JW10 to table.
> - Add AUO B116XAN06.1 to table.
> - Rename delays more generically so they can be reused.
> 
> Changes in v2:
> - No longer allow fallback to panel-simple.
> - Add "-ms" suffix to delays.
> - Don't support a "fallback" panel. Probed panels must be probed.
> - Not based on patch to copy "desc"--just allocate for probed panels.
> - Add "-ms" suffix to delays.
> 
> Douglas Anderson (16):
>   dt-bindings: drm/panel-simple-edp: Introduce generic eDP panels
>   drm/edid: Break out reading block 0 of the EDID
>   drm/edid: Allow the querying/working with the panel ID from the EDID
>   drm/panel-simple: Reorder logicpd_type_28 / mitsubishi_aa070mc01
>   drm/panel-simple-edp: Split eDP panels out of panel-simple
>   ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP
>   arm64: defconfig: Everyone who had PANEL_SIMPLE now gets
>     PANEL_SIMPLE_EDP
>   MIPS: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP
>   drm/panel-simple-edp: Move some wayward panels to the eDP driver
>   drm/panel-simple: Non-eDP panels don't need "HPD" handling
>   drm/panel-simple-edp: Split the delay structure out
>   drm/panel-simple-edp: Better describe eDP panel delays
>   drm/panel-simple-edp: hpd_reliable shouldn't be subtraced from
>     hpd_absent
>   drm/panel-simple-edp: Fix "prepare_to_enable" if panel doesn't handle
>     HPD
>   drm/panel-simple-edp: Don't re-read the EDID every time we power off
>     the panel
>   drm/panel-simple-edp: Implement generic "edp-panel"s probed by EDID

Thanks for looking into this. I really like the outcome.
We have panel-simple that now (mostly) handle simple panels,
and thus all the eDP functionality is in a separate driver.

I have provided a few nits.
My only take on this is the naming - as we do not want to confuse
panel-simple and panel-edp I strongly suggest renaming the driver to
panel-edp.
And then rename the corresponding Kconfig entry.

With these few changes all patches are:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

For bisectability I suggest to move the defconfig patches up before you
introduce the new Kconfig symbol. Or maybe they will be added via
another tree and then this is not possible to control

I assume you will apply the patches yourself.

Thanks again!

	Sam

Re: [PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding

[Doug Anderson]

Hi,

On Sun, Sep 5, 2021 at 11:55 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Douglas,
>
> On Wed, Sep 01, 2021 at 01:19:18PM -0700, Douglas Anderson wrote:
> > The goal of this patch series is to move away from hardcoding exact
> > eDP panels in device tree files. As discussed in the various patches
> > in this series (I'm not repeating everything here), most eDP panels
> > are 99% probable and we can get that last 1% by allowing two "power
> > up" delays to be specified in the device tree file and then using the
> > panel ID (found in the EDID) to look up additional power sequencing
> > delays for the panel.
> >
> > This patch series is the logical contiunation of a previous patch
> > series where I proposed solving this problem by adding a
> > board-specific compatible string [1]. In the discussion that followed
> > it sounded like people were open to something like the solution
> > proposed in this new series.
> >
> > In version 2 I got rid of the idea that we could have a "fallback"
> > compatible string that we'd use if we didn't recognize the ID in the
> > EDID. This simplifies the bindings a lot and the implementation
> > somewhat. As a result of not having a "fallback", though, I'm not
> > confident in transitioning any existing boards over to this since
> > we'll have to fallback to very conservative timings if we don't
> > recognize the ID from the EDID and I can't guarantee that I've seen
> > every panel that might have shipped on an existing product. The plan
> > is to use "edp-panel" only on new boards or new revisions of old
> > boards where we can guarantee that every EDID that ships out of the
> > factory has an ID in the table.
> >
> > Version 3 of this series now splits out all eDP panels to their own
> > driver and adds the generic eDP panel support to this new driver. I
> > believe this is what Sam was looking for [2].
> >
> > [1] https://lore.kernel.org/r/YFKQaXOmOwYyeqvM@google.com/
> > [2] https://lore.kernel.org/r/YRTsFNTn%2FT8fLxyB@ravnborg.org/
> >
> > Changes in v3:
> > - Decode hex product ID w/ same endianness as everyone else.
> > - ("Reorder logicpd_type_28...") patch new for v3.
> > - Split eDP panels patch new for v3.
> > - Move wayward panels patch new for v3.
> > - ("Non-eDP panels don't need "HPD" handling") new for v3.
> > - Split the delay structure out patch just on eDP now.
> > - ("Better describe eDP panel delays") new for v3.
> > - Fix "prepare_to_enable" patch new for v3.
> > - ("Don't re-read the EDID every time") moved to eDP only patch.
> > - Generic "edp-panel" handled by the eDP panel driver now.
> > - Change init order to we power at the end.
> > - Adjust endianness of product ID.
> > - Fallback to conservative delays if panel not recognized.
> > - Add Sharp LQ116M1JW10 to table.
> > - Add AUO B116XAN06.1 to table.
> > - Rename delays more generically so they can be reused.
> >
> > Changes in v2:
> > - No longer allow fallback to panel-simple.
> > - Add "-ms" suffix to delays.
> > - Don't support a "fallback" panel. Probed panels must be probed.
> > - Not based on patch to copy "desc"--just allocate for probed panels.
> > - Add "-ms" suffix to delays.
> >
> > Douglas Anderson (16):
> >   dt-bindings: drm/panel-simple-edp: Introduce generic eDP panels
> >   drm/edid: Break out reading block 0 of the EDID
> >   drm/edid: Allow the querying/working with the panel ID from the EDID
> >   drm/panel-simple: Reorder logicpd_type_28 / mitsubishi_aa070mc01
> >   drm/panel-simple-edp: Split eDP panels out of panel-simple
> >   ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP
> >   arm64: defconfig: Everyone who had PANEL_SIMPLE now gets
> >     PANEL_SIMPLE_EDP
> >   MIPS: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP
> >   drm/panel-simple-edp: Move some wayward panels to the eDP driver
> >   drm/panel-simple: Non-eDP panels don't need "HPD" handling
> >   drm/panel-simple-edp: Split the delay structure out
> >   drm/panel-simple-edp: Better describe eDP panel delays
> >   drm/panel-simple-edp: hpd_reliable shouldn't be subtraced from
> >     hpd_absent
> >   drm/panel-simple-edp: Fix "prepare_to_enable" if panel doesn't handle
> >     HPD
> >   drm/panel-simple-edp: Don't re-read the EDID every time we power off
> >     the panel
> >   drm/panel-simple-edp: Implement generic "edp-panel"s probed by EDID
>
> Thanks for looking into this. I really like the outcome.
> We have panel-simple that now (mostly) handle simple panels,
> and thus all the eDP functionality is in a separate driver.
>
> I have provided a few nits.
> My only take on this is the naming - as we do not want to confuse
> panel-simple and panel-edp I strongly suggest renaming the driver to
> panel-edp.

Sure, I'll do that. I was trying to express the fact that the new
"panel-edp" driver won't actually handle _all_ eDP panels, only the
eDP panels that are (comparatively) simpler. For instance, I'm not
planning to handle panel-samsung-atna33xc20.c in "panel-edp". I guess
people will figure it out, though.


> And then rename the corresponding Kconfig entry.
>
> With these few changes all patches are:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Thanks, I'll add it to the patches. If there's anything major I need
to change I'll give you a yell to make sure you see it.


> For bisectability I suggest to move the defconfig patches up before you
> introduce the new Kconfig symbol. Or maybe they will be added via
> another tree and then this is not possible to control

Yup, I'll do that. There was some question about the defconfig patch
but they are hopefully cleared up now.


> I assume you will apply the patches yourself.

Sure, I can do that with your Ack. I'll also make sure that patches
that Jani commented on get resolved.


-Doug