linux-kernel - Re: [PATCH v5 5/6] drm/panel-edp: Match edp

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]

Date: Wed, 6 Mar 2024 15:30:00 -0800
From: Doug Anderson <dianders@...omium.org>
To: Hsin-Yi Wang <hsinyi@...omium.org>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.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 v5 5/6] drm/panel-edp: Match edp_panels with panel identity

Hi,

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



-Doug