lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJMQK-jU6mROM8W00_Ps31Hk3S6+gj7dOSuEj7jHJfUSKx+MVA@mail.gmail.com>
Date: Wed, 6 Mar 2024 10:48:53 -0800
From: Hsin-Yi Wang <hsinyi@...omium.org>
To: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Douglas Anderson <dianders@...omium.org>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Jessica Zhang <quic_jesszhan@...cinc.com>, 
	Sam Ravnborg <sam@...nborg.org>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/5] drm/panel-edp: Match edp_panels with panel identity

On Wed, Mar 6, 2024 at 1:23 AM Jani Nikula <jani.nikula@...ux.intelcom> wrote:
>
> On Tue, 05 Mar 2024, Hsin-Yi Wang <hsinyi@...omium.org> wrote:
> > It's found that some panels have variants that they share the same panel id
> > although their EDID and names are different. When matching generic edp
> > panels, we should first match with both panel identity, which contains both
> > panel id and panel name. If not found, match with panel id only.
>
> Do you want to start matching also with name, for all panels? That's
> totally up to you, but that's the big functional change here.
>

It might be difficult to find all the datasheets for all the panels to
verify the names. Also, some of the names in the panels are also
marked as "Unknown", so I think we still want to keep the matching
with id only.

Without really testing on the exact panel, I'm afraid that this might
break them.


> BR,
> Jani.
>
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@...omium.org>
> > ---
> > v3->v4: combine name and id to identity.
> > ---
> >  drivers/gpu/drm/panel/panel-edp.c | 45 ++++++++++++++++---------------
> >  1 file changed, 24 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index d094cfc43da8..fb70e97a2e71 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -210,15 +210,12 @@ struct panel_desc {
> >   * struct edp_panel_entry - Maps panel ID to delay / panel name.
> >   */
> >  struct edp_panel_entry {
> > -     /** @panel_id: 32-bit ID for panel, encoded with drm_edid_encode_panel_id(). */
> > -     u32 panel_id;
> > +     /** @ident: edid identity used for panel matching. */
> > +     const struct drm_edid_ident ident;
> >
> >       /** @delay: The power sequencing delays needed for this panel. */
> >       const struct panel_delay *delay;
> >
> > -     /** @name: Name of this panel (for printing to logs). */
> > -     const char *name;
> > -
> >       /** @override_edid_mode: Override the mode obtained by edid. */
> >       const struct drm_display_mode *override_edid_mode;
> >  };
> > @@ -691,7 +688,7 @@ static int detected_panel_show(struct seq_file *s, void *data)
> >       else if (!p->detected_panel)
> >               seq_puts(s, "HARDCODED\n");
> >       else
> > -             seq_printf(s, "%s\n", p->detected_panel->name);
> > +             seq_printf(s, "%s\n", p->detected_panel->ident.name);
> >
> >       return 0;
> >  }
> > @@ -761,7 +758,7 @@ static void panel_edp_parse_panel_timing_node(struct device *dev,
> >               dev_err(dev, "Reject override mode: No display_timing found\n");
> >  }
> >
> > -static const struct edp_panel_entry *find_edp_panel(u32 panel_id);
> > +static const struct edp_panel_entry *find_edp_panel(u32 panel_id, const struct drm_edid *edid);
> >
> >  static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> >  {
> > @@ -799,7 +796,6 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> >       base_block = drm_edid_read_base_block(panel->ddc);
> >       if (base_block) {
> >               panel_id = drm_edid_get_panel_id(base_block);
> > -             drm_edid_free(base_block);
> >       } else {
> >               dev_err(dev, "Couldn't identify panel via EDID\n");
> >               ret = -EIO;
> > @@ -807,7 +803,9 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> >       }
> >       drm_edid_decode_panel_id(panel_id, vend, &product_id);
> >
> > -     panel->detected_panel = find_edp_panel(panel_id);
> > +     panel->detected_panel = find_edp_panel(panel_id, base_block);
> > +
> > +     drm_edid_free(base_block);
> >
> >       /*
> >        * We're using non-optimized timings and want it really obvious that
> > @@ -840,7 +838,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> >               panel->detected_panel = ERR_PTR(-EINVAL);
> >       } else {
> >               dev_info(dev, "Detected %s %s (%#06x)\n",
> > -                      vend, panel->detected_panel->name, product_id);
> > +                      vend, panel->detected_panel->ident.name, product_id);
> >
> >               /* Update the delay; everything else comes from EDID */
> >               desc->delay = *panel->detected_panel->delay;
> > @@ -1930,17 +1928,21 @@ static const struct panel_delay delay_200_500_e50_po2e200 = {
> >
> >  #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> >  { \
> > -     .name = _name, \
> > -     .panel_id = drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, vend_chr_2, \
> > -                                          product_id), \
> > +     .ident = { \
> > +             .name = _name, \
> > +             .panel_id = drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, vend_chr_2, \
> > +                                                  product_id), \
> > +     }, \
> >       .delay = _delay \
> >  }
> >
> >  #define EDP_PANEL_ENTRY2(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name, _mode) \
> >  { \
> > -     .name = _name, \
> > -     .panel_id = drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, vend_chr_2, \
> > -                                          product_id), \
> > +     .ident = { \
> > +             .name = _name, \
> > +             .panel_id = drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, vend_chr_2, \
> > +                                                  product_id), \
> > +     }, \
> >       .delay = _delay, \
> >       .override_edid_mode = _mode \
> >  }
> > @@ -2087,15 +2089,16 @@ static const struct edp_panel_entry edp_panels[] = {
> >       { /* sentinal */ }
> >  };
> >
> > -static const struct edp_panel_entry *find_edp_panel(u32 panel_id)
> > +static const struct edp_panel_entry *find_edp_panel(u32 panel_id, const struct drm_edid *edid)
> >  {
> >       const struct edp_panel_entry *panel;
> >
> > -     if (!panel_id)
> > -             return NULL;
> > +     for (panel = edp_panels; panel->ident.panel_id; panel++)
> > +             if (drm_edid_match_identity(edid, &panel->ident))
> > +                     return panel;
> >
> > -     for (panel = edp_panels; panel->panel_id; panel++)
> > -             if (panel->panel_id == panel_id)
> > +     for (panel = edp_panels; panel->ident.panel_id; panel++)
> > +             if (panel->ident.panel_id == panel_id)
> >                       return panel;
> >
> >       return NULL;
>
> --
> Jani Nikula, Intel

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ