Hi,

On Wed, Mar 6, 2024 at 12:04 PM Hsin-Yi Wang <hsi...@chromium.org> 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 <diand...@chromium.org>



-Doug

Reply via email to