[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHwB_NJ3yQxf9fTMT_cQv50z8X_NKyQMOJEuqDqY-BfKX8QzXQ@mail.gmail.com>
Date: Thu, 31 Jul 2025 15:14:59 +0800
From: cong yang <yangcong5@...qin.corp-partner.google.com>
To: Doug Anderson <dianders@...omium.org>
Cc: neil.armstrong@...aro.org, jessica.zhang@....qualcomm.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, simona@...ll.ch, treapking@...omium.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel-edp: Add several generic edp panels
Hi,
Doug Anderson <dianders@...omium.org> 于2025年7月31日周四 01:40写道:
>
> Hi,
>
> On Wed, Jul 30, 2025 at 12:16 AM Cong Yang
> <yangcong5@...qin.corp-partner.google.com> wrote:
> >
> > Add a few generic edp panels used by mt8189 chromebooks, most of them use
> > the same general timing. For CMN-N116BCA-EAK, the enable timing should be
> > 200ms. For TMA-TL140VDMS03-01, the enable timing should be 80ms and the
> > disable timing should be 100ms.
> >
> > 1. AUO B122UAN01.0
> > 2. AUO B116XAK02.0
> > 3. AUO B140UAN08.5
> > 4. AUO B140UAX01.2
> > 5. BOE NV116WHM-N4B
> > 6. BOE NV116WHM-T01
> > 7. CMN N122JCA-ENK
> > 8. CMN N140JCA-ELP
> > 9. CMN N116BCA-EAK
> > 10. CSW MNE007QS5-2
> > 11. SCW MNE007QB2-2
>
> nit: CSW, not SCW.
Fix next version, thanks.
>
>
> > 12. TMA TM140VDXP01-04
> > 13. TMA TL140VDMS03-01
> >
> > Signed-off-by: Cong Yang <yangcong5@...qin.corp-partner.google.com>
> > ---
> > drivers/gpu/drm/panel/panel-edp.c | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
>
> Please make the subject line a little less generic so we can tell one
> change for the next. If all changes just say "add more panels" then
> when cherry-picking and looking at patch subject lines it's hard to
> tell which patches have been picked and which haven't.
>
> In this case you could mention that the panels are used by mt8180
> Chromebooks, like:
>
> drm/panel-edp: Add edp panels used by mt8180 Chromebooks
>
> Also: even though it's a bit of a pain, can you please include the
> EDIDs in your commit message? I know there are 13 panels and that can
> make a long commit message, but I'd still rather see it here just in
> case we have to later go back and reference exactly what panel you
> were working with in case some manufacturer re-uses panel IDs (it's
> happened in the past).
Got it, fix next version, thanks.
>
>
> > @@ -1865,6 +1880,7 @@ static const struct panel_delay delay_200_500_e50_d100 = {
> > * Sort first by vendor, then by product ID.
> > */
> > static const struct edp_panel_entry edp_panels[] = {
> > + EDP_PANEL_ENTRY('A', 'U', 'O', 0x04a4, &delay_200_500_e50, "B122UAN01.0"),
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x105c, &delay_200_500_e50, "B116XTN01.0"),
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x125c, &delay_200_500_e50, "Unknown"),
> > @@ -1883,6 +1899,7 @@ static const struct edp_panel_entry edp_panels[] = {
> > EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0",
> > &auo_b116xa3_mode),
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x435c, &delay_200_500_e50, "Unknown"),
> > + EDP_PANEL_ENTRY('A', 'U', 'O', 0x52b0, &delay_200_500_e80_d50, "B116XAK02.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"),
> > @@ -1890,10 +1907,12 @@ static const struct edp_panel_entry edp_panels[] = {
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x723c, &delay_200_500_e50, "B140XTN07.2"),
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x73aa, &delay_200_500_e50, "B116XTN02.3"),
> > EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
> > + EDP_PANEL_ENTRY('A', 'U', 'O', 0x8bba, &delay_200_500_e80_d50, "B140UAN08.5"),
>
> All the old AUX panels seem to use just "e50" but all the new ones
> you're adding use "e80_d50". That looks really suspicious. Are you
> sure that these new panels need "e80_d50"? Were the old definitions
> wrong?
I checked all the panel specs again, most of them require "e50", only
a few require "e80_d50".
Sorry, I took the maximum value, will fix it in the next version. Thanks.
Powered by blists - more mailing lists