Hi, On Wed, Mar 6, 2024 at 12:04 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote: > > @@ -2111,15 +2113,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(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; Reading through this another time, I wouldn't object to a comment reminding the user why there are two loops here. Something like "Try to match both the panel ID and name at first. This allows handling the case where vendors incorrectly reused the same panel ID for multiple panels that need different settings. If we don't get a match with the name, that's OK. Panel ID _should_ be unique anyway". Feel free to reword. In any case: Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> -Doug