Received-SPF: pass (google.com: domain of linux-kernel+bounces-94449-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240306004347.974304-1-hsinyi@chromium.org> <20240306004347.974304-5-hsinyi@chromium.org>
 <87msrbzpba.fsf@intel.com>
In-Reply-To: <87msrbzpba.fsf@intel.com>
From: Hsin-Yi Wang <hsinyi@chromium.org>
Date: Wed, 6 Mar 2024 10:48:53 -0800
Message-ID: <CAJMQK-jU6mROM8W00_Ps31Hk3S6+gj7dOSuEj7jHJfUSKx+MVA@mail.gmail.com>
Subject: Re: [PATCH v4 4/5] drm/panel-edp: Match edp_panels with panel identity
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Douglas Anderson <dianders@chromium.org>, 
	Neil Armstrong <neil.armstrong@linaro.org>, Jessica Zhang <quic_jesszhan@quicinc.com>, 
	Sam Ravnborg <sam@ravnborg.org>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, 
	Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, 
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, dri-devel@lists.freedesktop.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 6, 2024 at 1:23=E2=80=AFAM Jani Nikula <jani.nikula@linux.intel=
com> wrote:
>
> On Tue, 05 Mar 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> > It's found that some panels have variants that they share the same pane=
l 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@chromium.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(struc=
t device *dev,
> >               dev_err(dev, "Reject override mode: No display_timing fou=
nd\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, cons=
t struct drm_edid *edid);
> >
> >  static int generic_edp_panel_probe(struct device *dev, struct panel_ed=
p *panel)
> >  {
> > @@ -799,7 +796,6 @@ static int generic_edp_panel_probe(struct device *d=
ev, struct panel_edp *panel)
> >       base_block =3D drm_edid_read_base_block(panel->ddc);
> >       if (base_block) {
> >               panel_id =3D drm_edid_get_panel_id(base_block);
> > -             drm_edid_free(base_block);
> >       } else {
> >               dev_err(dev, "Couldn't identify panel via EDID\n");
> >               ret =3D -EIO;
> > @@ -807,7 +803,9 @@ static int generic_edp_panel_probe(struct device *d=
ev, struct panel_edp *panel)
> >       }
> >       drm_edid_decode_panel_id(panel_id, vend, &product_id);
> >
> > -     panel->detected_panel =3D find_edp_panel(panel_id);
> > +     panel->detected_panel =3D find_edp_panel(panel_id, base_block);
> > +
> > +     drm_edid_free(base_block);
> >
> >       /*
> >        * We're using non-optimized timings and want it really obvious t=
hat
> > @@ -840,7 +838,7 @@ static int generic_edp_panel_probe(struct device *d=
ev, struct panel_edp *panel)
> >               panel->detected_panel =3D 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 =3D *panel->detected_panel->delay;
> > @@ -1930,17 +1928,21 @@ static const struct panel_delay delay_200_500_e=
50_po2e200 =3D {
> >
> >  #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id=
, _delay, _name) \
> >  { \
> > -     .name =3D _name, \
> > -     .panel_id =3D drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, ve=
nd_chr_2, \
> > -                                          product_id), \
> > +     .ident =3D { \
> > +             .name =3D _name, \
> > +             .panel_id =3D drm_edid_encode_panel_id(vend_chr_0, vend_c=
hr_1, vend_chr_2, \
> > +                                                  product_id), \
> > +     }, \
> >       .delay =3D _delay \
> >  }
> >
> >  #define EDP_PANEL_ENTRY2(vend_chr_0, vend_chr_1, vend_chr_2, product_i=
d, _delay, _name, _mode) \
> >  { \
> > -     .name =3D _name, \
> > -     .panel_id =3D drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, ve=
nd_chr_2, \
> > -                                          product_id), \
> > +     .ident =3D { \
> > +             .name =3D _name, \
> > +             .panel_id =3D drm_edid_encode_panel_id(vend_chr_0, vend_c=
hr_1, vend_chr_2, \
> > +                                                  product_id), \
> > +     }, \
> >       .delay =3D _delay, \
> >       .override_edid_mode =3D _mode \
> >  }
> > @@ -2087,15 +2089,16 @@ static const struct edp_panel_entry edp_panels[=
] =3D {
> >       { /* 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, cons=
t struct drm_edid *edid)
> >  {
> >       const struct edp_panel_entry *panel;
> >
> > -     if (!panel_id)
> > -             return NULL;
> > +     for (panel =3D edp_panels; panel->ident.panel_id; panel++)
> > +             if (drm_edid_match_identity(edid, &panel->ident))
> > +                     return panel;
> >
> > -     for (panel =3D edp_panels; panel->panel_id; panel++)
> > -             if (panel->panel_id =3D=3D panel_id)
> > +     for (panel =3D edp_panels; panel->ident.panel_id; panel++)
> > +             if (panel->ident.panel_id =3D=3D panel_id)
> >                       return panel;
> >
> >       return NULL;
>
> --
> Jani Nikula, Intel