Re: [PATCH 1/3] drm/panel-edp: Add several generic edp panels

From: Doug Anderson
Date: Thu Nov 02 2023 - 12:54:25 EST

Next message: kernel test robot: "drivers/dma/dw-edma/dw-edma-v0-debugfs.c:250:50: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 8"
Previous message: Alex Deucher: "Re: mainline build failure due to 7966f319c66d ("drm/amd/display: Introduce DML2")"
In reply to: Hsin-Yi Wang: "[PATCH 1/3] drm/panel-edp: Add several generic edp panels"
Next in thread: Hsin-Yi Wang: "Re: [PATCH 1/3] drm/panel-edp: Add several generic edp panels"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

On Wed, Nov 1, 2023 at 2:26 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> Add a few generic edp panels used by mt8186 chromebooks.
> Besides, modify the following panel:
> - AUO 0x235c B116XTN02 renamed to B116XTN02.3.
> - AUO 0x405c B116XAK01 adjust the timing to delay_200_500_e50. According
> to the datasheet: T3=200, T12=500, T7_max = 50.

Can you modify this in the `auo_b116xak01` structure? That should make
timing work more correctly for anyone directly specifying this panel.
The reason I had it point to the other structure was so we didn't
treat anyone specifying this panel directly differently than anyone
autodetecting it...

> Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/panel/panel-edp.c | 62 ++++++++++++++++++++++++++++++-
> 1 file changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 9dce4c702414..06ce3a73d740 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1830,6 +1830,12 @@ static const struct panel_delay delay_200_500_e50 = {
> .enable = 50,
> };
>
> +static const struct panel_delay delay_200_500_e80 = {
> + .hpd_absent = 200,
> + .unprepare = 500,
> + .enable = 80,
> +};
> +
> static const struct panel_delay delay_200_500_e80_d50 = {
> .hpd_absent = 200,
> .unprepare = 500,
> @@ -1849,6 +1855,25 @@ static const struct panel_delay delay_200_500_e200 = {
> .enable = 200,
> };
>
> +static const struct panel_delay delay_200_500_e200_d10 = {
> + .hpd_absent = 200,
> + .unprepare = 500,
> + .enable = 200,
> + .disable = 10,
> +};
> +
> +static const struct panel_delay delay_200_150_e50 = {
> + .hpd_absent = 200,
> + .unprepare = 150,

I worry a little bit about seeing "unprepare" of 150. It doesn't mean
it's wrong, but it's been so consistent for me to see .500 here that
it's good to double-confirm. It looks like you use this one on
"KD116N2930A15". From the datasheet I see that T12 has a min of 150 ms
and a max of 500 ms. Is that the same thing you see?

Specifying a "max" for T12 makes no sense. That's saying that it
violates timing specs to ever turn the panel off for more than half a
second which is nonsense.

Given that:
* The spec is obviously nonsense for this number.
* The 500 ms number is present in the spec and somewhat standard for eDP panels.
* Having a larger number is safer.
* 500 ms usually won't have a real world impact since it just prevents
turning on the panel again right after turning it off (and we use
autosuspend to avoid this in most cases).

Maybe it's better to just use 500 here?

> + .enable = 50,
> +};
> +
> +static const struct panel_delay delay_200_150_e200 = {
> + .hpd_absent = 200,
> + .unprepare = 150,
> + .enable = 200,

Let's look at this unprepare of 150 too. I guess it's used for two panels.

NT116WHM-N21: Wow, there sure are a lot of panels that call themselves
"NT116WHM-N21" but have different IDs. :-P If you're sure that this is
150 this is fine, but if there's any doubt (like above) then 500 is
safer.

R140NWFM R1: I think I found this spec and yeah, it's super clear. 150 ms.

NOTE: I didn't try to double-check any of the other timings, mostly I
just looked at the unprepare since it stood out. ;-)

> +};
> +
> #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> { \
> .name = _name, \
> @@ -1869,38 +1894,71 @@ static const struct edp_panel_entry edp_panels[] = {
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x145c, &delay_200_500_e50, "B116XAB01.4"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x1e9b, &delay_200_500_e50, "B133UAN02.1"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x1ea5, &delay_200_500_e50, "B116XAK01.6"),
> - EDP_PANEL_ENTRY('A', 'U', 'O', 0x235c, &delay_200_500_e50, "B116XTN02"),
> - EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x208d, &delay_200_500_e50, "B140HTN02.1"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x235c, &delay_200_500_e50, "B116XTN02.3"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x239b, &delay_200_500_e50, "B116XAN06.1"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x255c, &delay_200_500_e50, "B116XTN02.5"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x403d, &delay_200_500_e50, "B140HAN04.0"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &delay_200_500_e50, "B116XAK01.0"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x582d, &delay_200_500_e50, "B133UAN01.0"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x635c, &delay_200_500_e50, "B116XAN06.3"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0x639c, &delay_200_500_e50, "B140HAK02.7"),
> EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
> + EDP_PANEL_ENTRY('A', 'U', 'O', 0xf390, &delay_200_500_e50, "B140XTN07.7"),
>
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0715, &delay_200_150_e200, "NT116WHM-N21"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0731, &delay_200_500_e80, "NT116WHM-N42"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0741, &delay_200_500_e200, "NT116WHM-N44"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0786, &delay_200_500_p2e80, "NV116WHM-T01"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x07d1, &boe_nv133fhm_n61.delay, "NV133FHM-N61"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x07f6, &delay_200_500_e200, "NT140FHM-N44"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x082d, &boe_nv133fhm_n61.delay, "NV133FHM-N62"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x08b2, &delay_200_500_e200, "NT140WHM-N49"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x09c3, &delay_200_500_e50, "NT116WHM-N21,836X2"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x094b, &delay_200_500_e50, "NT116WHM-N21"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x095f, &delay_200_500_e50, "NE135FBM-N41 v8.1"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0979, &delay_200_500_e50, "NV116WHM-N49 V8.0"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x094b, &delay_200_500_e50, "NT116WHM-N21"),

0x904b is already specified above and this is in the wrong sort order.

> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0951, &delay_200_500_e80, "NV116WHM-N47"),

Please sort 0x0951 before 0x0979.

> EDP_PANEL_ENTRY('B', 'O', 'E', 0x098d, &boe_nv110wtm_n61.delay, "NV110WTM-N61"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x09ae, &delay_200_500_e200, "NT140FHM-N45"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x09dd, &delay_200_500_e50, "NT116WHM-N21"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a5d, &delay_200_500_e50, "NV116WHM-N45"),
> EDP_PANEL_ENTRY('B', 'O', 'E', 0x0ac5, &delay_200_500_e50, "NV116WHM-N4C"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0b43, &delay_200_500_e200, "NV140FHM-T09"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0b56, &delay_200_500_e80, "NT140FHM-N47"),
> + EDP_PANEL_ENTRY('B', 'O', 'E', 0x0c20, &delay_200_500_e80, "NT140FHM-N47"),
>
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x1132, &delay_200_500_e80_d50, "N116BGE-EA2"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x1138, &innolux_n116bca_ea1.delay, "N116BCA-EA1-RC4"),
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x1139, &delay_200_500_e80_d50, "N116BGE-EA2"),
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x1145, &delay_200_500_e80_d50, "N116BCN-EB1"),

Please sort 0x1145 above 0x114c

> EDP_PANEL_ENTRY('C', 'M', 'N', 0x1152, &delay_200_500_e80_d50, "N116BCN-EA1"),
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x1153, &delay_200_500_e80_d50, "N116BGE-EA2"),
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x1154, &delay_200_500_e80_d50, "N116BCA-EA2"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x1157, &delay_200_500_e80_d50, "N116BGE-EA2"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x115b, &delay_200_500_e80_d50, "N116BCN-EB1"),
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x1247, &delay_200_500_e80_d50, "N120ACA-EA1"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x142b, &delay_200_500_e80_d50, "N140HCA-EAC"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x144f, &delay_200_500_e80_d50, "N140HGA-EA1"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x1468, &delay_200_500_e80, "N140HGA-EA1"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x14e5, &delay_200_500_e80_d50, "N140HGA-EA1"),
> EDP_PANEL_ENTRY('C', 'M', 'N', 0x14d4, &delay_200_500_e80_d50, "N140HCA-EAC"),
> + EDP_PANEL_ENTRY('C', 'M', 'N', 0x14d6, &delay_200_500_e80_d50, "N140BGA-EA4"),
> +
> + EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"),
>
> + EDP_PANEL_ENTRY('I', 'V', 'O', 0x048e, &delay_200_500_e200_d10, "M116NWR6 R5"),
> EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> EDP_PANEL_ENTRY('I', 'V', 'O', 0x854a, &delay_200_500_p2e100, "M133NW4J"),
> EDP_PANEL_ENTRY('I', 'V', 'O', 0x854b, &delay_200_500_p2e100, "R133NW4K-R0"),
> + EDP_PANEL_ENTRY('I', 'V', 'O', 0x8c4d, &delay_200_150_e200, "R140NWFM R1"),
>
> EDP_PANEL_ENTRY('K', 'D', 'B', 0x0624, &kingdisplay_kd116n21_30nv_a010.delay, "116N21-30NV-A010"),
> EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50, "116N29-30NK-C007"),
> + EDP_PANEL_ENTRY('K', 'D', 'C', 0x0809, &delay_200_150_e50, "KD116N2930A15"),

Please sort 0x0809 above 0x1120.

Next message: kernel test robot: "drivers/dma/dw-edma/dw-edma-v0-debugfs.c:250:50: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 8"
Previous message: Alex Deucher: "Re: mainline build failure due to 7966f319c66d ("drm/amd/display: Introduce DML2")"
In reply to: Hsin-Yi Wang: "[PATCH 1/3] drm/panel-edp: Add several generic edp panels"
Next in thread: Hsin-Yi Wang: "Re: [PATCH 1/3] drm/panel-edp: Add several generic edp panels"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]