| Message ID | 20250123100747.1841357-13-damon.ding@rock-chips.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add eDP support for RK3588 | expand |
On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote: > The raw edid for LP079QX1-SP0V panel model is: > > 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00 > 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26 > 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 > 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10 > 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c > 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc > 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f > > Signed-off-by: Damon Ding <damon.ding@rock-chips.com> > --- > drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++ > 1 file changed, 8 insertions(+) Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas to CC manually. Or, better, split irrelevant patches to separate series. > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index f8511fe5fb0d..77e3fd3ed160 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = { > .enable = 50, > }; > > +static const struct panel_delay delay_50_500_e200 = { > + .hpd_absent = 50, > + .unprepare = 500, > + .enable = 200, > +}; > + > #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \ > { \ > .ident = { \ > @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = { > EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"), > EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"), > > + EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"), > + > EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d51, &delay_200_500_e200, "Unknown"), > EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5b, &delay_200_500_e200, "MB116AN01"), > EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"), > -- > 2.34.1 >
Hi, On Thu, Jan 23, 2025 at 3:31 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote: > > The raw edid for LP079QX1-SP0V panel model is: > > > > 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00 > > 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26 > > 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 > > 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10 > > 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00 > > 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c > > 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc > > 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f > > > > Signed-off-by: Damon Ding <damon.ding@rock-chips.com> > > --- > > drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas > to CC manually. > > Or, better, split irrelevant patches to separate series. > > > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > > index f8511fe5fb0d..77e3fd3ed160 100644 > > --- a/drivers/gpu/drm/panel/panel-edp.c > > +++ b/drivers/gpu/drm/panel/panel-edp.c > > @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = { > > .enable = 50, > > }; > > > > +static const struct panel_delay delay_50_500_e200 = { > > + .hpd_absent = 50, > > + .unprepare = 500, > > + .enable = 200, > > +}; Wow, hpd_absent is 50ms. That's excellent! I was curious if this was really correct since it's the lowest I've seen, but I searched the internet and found a datasheet that confirms it. Crazy. Although my datasheet has that whole section grayed out and says "Measurement not available". How worried should be about that? I guess if "hpd_absent" of 50 eventually doesn't work it can always be increased. Looking at that datasheet (assuming you can find the same one), I wonder how you handle the T4 120ms requirement. It looks like that's the time from tcon reset (which is nearly power on) until you're allowed to put valid data on the panel. I _think_ you could meet the requirement via setting `powered_on_to_enable` to 320 though, right? ...or maybe 335 to handle the maximum value of "tcon_reset" (no idea how you control this signal). If I remember correctly (it's been a while), things will already be clocking but outputting black when the panel's "enable" function is called. After the function is called then we'll turn on the backlight (and still outputting black) and eventually we'll put valid data. So as long as you consider the continued output of black as "valid data" then the timing diagram is satisfied. BTW: don't you also need a 200 ms "disable"? > > #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \ > > { \ > > .ident = { \ > > @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = { > > EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"), > > EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"), > > > > + EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"), I don't love that the ID is 0x0000. That makes me nervous that the panel vendor isn't setting the ID properly. ...but at least the string matches in the EDID so hopefully that will be enough to uniquely identify things.
Hi Dmitry, On 2025/1/23 19:31, Dmitry Baryshkov wrote: > On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote: >> The raw edid for LP079QX1-SP0V panel model is: >> >> 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00 >> 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26 >> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 >> 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10 >> 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00 >> 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c >> 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc >> 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f >> >> Signed-off-by: Damon Ding <damon.ding@rock-chips.com> >> --- >> drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) > > Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas > to CC manually. > > Or, better, split irrelevant patches to separate series. > I will split the panel related patch into a separate series in the next version. >> >> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c >> index f8511fe5fb0d..77e3fd3ed160 100644 >> --- a/drivers/gpu/drm/panel/panel-edp.c >> +++ b/drivers/gpu/drm/panel/panel-edp.c >> @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = { >> .enable = 50, >> }; >> >> +static const struct panel_delay delay_50_500_e200 = { >> + .hpd_absent = 50, >> + .unprepare = 500, >> + .enable = 200, >> +}; >> + >> #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \ >> { \ >> .ident = { \ >> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = { >> EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"), >> EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"), >> >> + EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"), >> + >> EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d51, &delay_200_500_e200, "Unknown"), >> EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5b, &delay_200_500_e200, "MB116AN01"), >> EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"), >> -- >> 2.34.1 >> > Best regards, Damon
Hi Doug, On 2025/1/24 8:30, Doug Anderson wrote: > Hi, > > On Thu, Jan 23, 2025 at 3:31 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote: >>> The raw edid for LP079QX1-SP0V panel model is: >>> >>> 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00 >>> 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26 >>> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 >>> 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10 >>> 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00 >>> 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c >>> 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc >>> 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f >>> >>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com> >>> --- >>> drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >> >> Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas >> to CC manually. >> >> Or, better, split irrelevant patches to separate series. >> >>> >>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c >>> index f8511fe5fb0d..77e3fd3ed160 100644 >>> --- a/drivers/gpu/drm/panel/panel-edp.c >>> +++ b/drivers/gpu/drm/panel/panel-edp.c >>> @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = { >>> .enable = 50, >>> }; >>> >>> +static const struct panel_delay delay_50_500_e200 = { >>> + .hpd_absent = 50, >>> + .unprepare = 500, >>> + .enable = 200, >>> +}; > > Wow, hpd_absent is 50ms. That's excellent! > > I was curious if this was really correct since it's the lowest I've > seen, but I searched the internet and found a datasheet that confirms > it. Crazy. Although my datasheet has that whole section grayed out and > says "Measurement not available". How worried should be about that? I > guess if "hpd_absent" of 50 eventually doesn't work it can always be > increased. > The datasheet I have should be the same as yours, and the T3 is "Min. 2ms | Typ. 30ms | Max. 50ms" with the gray "Measurement not available". > Looking at that datasheet (assuming you can find the same one), I > wonder how you handle the T4 120ms requirement. It looks like that's > the time from tcon reset (which is nearly power on) until you're > allowed to put valid data on the panel. I _think_ you could meet the > requirement via setting `powered_on_to_enable` to 320 though, right? > ...or maybe 335 to handle the maximum value of "tcon_reset" (no idea > how you control this signal). If I remember correctly (it's been a > while), things will already be clocking but outputting black when the > panel's "enable" function is called. After the function is called then > we'll turn on the backlight (and still outputting black) and > eventually we'll put valid data. So as long as you consider the > continued output of black as "valid data" then the timing diagram is > satisfied. > Yes, it is indeed better to set the "powered_on_to_enable" to 335ms, as this will help prevent the display from showing invalid black screens. I will add it in the next version. > BTW: don't you also need a 200 ms "disable"? > According to the datasheet, it is a good idea to set "disable" to 200ms. > >>> #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \ >>> { \ >>> .ident = { \ >>> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = { >>> EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"), >>> EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"), >>> >>> + EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"), > > I don't love that the ID is 0x0000. That makes me nervous that the > panel vendor isn't setting the ID properly. ...but at least the string > matches in the EDID so hopefully that will be enough to uniquely > identify things. > > The ID "0x0000" makes me nervous too, but the EDID obtained from this panel indeed show it, which is quite surprising. May I still set it to "0x0000" as it really is? Best regards, Damon
Hi, On Fri, Jan 24, 2025 at 8:18 PM Damon Ding <damon.ding@rock-chips.com> wrote: > > >>> #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \ > >>> { \ > >>> .ident = { \ > >>> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = { > >>> EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"), > >>> EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"), > >>> > >>> + EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"), > > > > I don't love that the ID is 0x0000. That makes me nervous that the > > panel vendor isn't setting the ID properly. ...but at least the string > > matches in the EDID so hopefully that will be enough to uniquely > > identify things. > > > > > > The ID "0x0000" makes me nervous too, but the EDID obtained from this > panel indeed show it, which is quite surprising. May I still set it to > "0x0000" as it really is? Yeah, keep it as 0x0000. Since the panel name is in the EDID then commit bf201127c1b8 ("drm/panel-edp: Match edp_panels with panel identity") should provide some safety. -Doug
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index f8511fe5fb0d..77e3fd3ed160 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = { .enable = 50, }; +static const struct panel_delay delay_50_500_e200 = { + .hpd_absent = 50, + .unprepare = 500, + .enable = 200, +}; + #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \ { \ .ident = { \ @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = { EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"), EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"), + EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"), + EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d51, &delay_200_500_e200, "Unknown"), EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5b, &delay_200_500_e200, "MB116AN01"), EDP_PANEL_ENTRY('H', 'K', 'C', 0x2d5c, &delay_200_500_e200, "MB116AN01-2"),
The raw edid for LP079QX1-SP0V panel model is: 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f Signed-off-by: Damon Ding <damon.ding@rock-chips.com> --- drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++ 1 file changed, 8 insertions(+)