Hi Doug, Thank you for the review.
On 20-Dec-25 01:23, Doug Anderson wrote: > Hi, > > On Fri, Dec 19, 2025 at 12:58 AM Hans de Goede > <[email protected]> wrote: >> >> Add powerseq timing info for the BOE NV140WUM-T08 panel used on Lenovo >> Thinkpad T14s gen 6 (Snapdragon X1 Elite) laptops. >> >> edid-decode (hex): >> >> 00 ff ff ff ff ff ff 00 09 e5 26 0c 00 00 00 00 >> 0a 21 01 04 a5 1e 13 78 03 d6 62 99 5e 5a 8e 27 >> 25 53 58 00 00 00 01 01 01 01 01 01 01 01 01 01 >> 01 01 01 01 01 01 33 3f 80 dc 70 b0 3c 40 30 20 >> 36 00 2e bc 10 00 00 1a 00 00 00 fd 00 28 3c 4c >> 4c 10 01 0a 20 20 20 20 20 20 00 00 00 fe 00 42 >> 4f 45 20 43 51 0a 20 20 20 20 20 20 00 00 00 fe >> 00 4e 56 31 34 30 57 55 4d 2d 54 30 38 0a 00 fa >> >> Signed-off-by: Hans de Goede <[email protected]> >> --- >> drivers/gpu/drm/panel/panel-edp.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/panel/panel-edp.c >> b/drivers/gpu/drm/panel/panel-edp.c >> index 415b894890ad..7b8e5cd41594 100644 >> --- a/drivers/gpu/drm/panel/panel-edp.c >> +++ b/drivers/gpu/drm/panel/panel-edp.c >> @@ -1730,6 +1730,12 @@ static const struct panel_delay delay_200_500_p2e100 >> = { >> .prepare_to_enable = 100, >> }; >> >> +static const struct panel_delay delay_200_500_p2e200 = { >> + .hpd_absent = 200, >> + .unprepare = 500, >> + .prepare_to_enable = 200, >> +}; >> + >> static const struct panel_delay delay_200_500_e50 = { >> .hpd_absent = 200, >> .unprepare = 500, >> @@ -1975,6 +1981,7 @@ static const struct edp_panel_entry edp_panels[] = { >> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0b56, &delay_200_500_e80, >> "NT140FHM-N47"), >> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0b66, &delay_200_500_e80, >> "NE140WUM-N6G"), >> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0c20, &delay_200_500_e80, >> "NT140FHM-N47"), >> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0c26, &delay_200_500_p2e200, >> "NV140WUM-T08"), > > This looks OK, but I'd like to make sure that you really need > `prepare_to_enable` as 200 and it's not supposed to be > `powered_on_to_enable`. > > prepare_to_enable is usually usually (T4+T5+T6+T8)-min The datasheet gives (T4+T5+T6+T8) > 200ms The documentation in struct panel_delay for powered_on_to_enable says: /** * @powered_on_to_enable: Time between panel powered on and enable. * ... * This doesn't normally need to be set if timings are already met by * prepare_to_enable or enable. */ and looking at the code it also seems better to use prepare_to_enable where we can, to have the shortest possible delay for panels which have HPD properly connected. > I notice that some other BOE panels have "delay_200_500_e50_po2e200". That e50 there is .enable = 50: /** * @enable: Time for the panel to display a valid frame. * * The time (in milliseconds) that it takes for the panel to * display the first valid frame after starting to receive * video data. * * This is (T6-min + max(T7-max, T8-min)) on eDP timing diagrams or * the delay after link training finishes until we can turn the * backlight on and see valid data. */ The datasheet does not specify T4 / T5 / T6, so the enable time is unknown, so delay_200_500_e50_po2e200 seems wrong for this panel. > powered_on_to_enable is usually (T3+T4+T5+T6+T8)-min That would be .hpd_absent + .prepare_to_enable aka 400 ms which is the worst case scenario if HPD is not connected. If we get a HPD response from the panel within the 200ms hpd_absent then specifying separate .hpd_absent and .prepare_to_enable times will result in a lower total power-on-delay for the panel. > If you're sure you need `prepare_to_enable` then that's fine--just > thought I'd check. :-P Ack, see above. Regards, Hans
