linux-kernel - Re: [PATCH v6 4/5] drm/panel-edp: Add override_edid

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] [day] [month] [year] [list]

Message-ID: <CAD=FV=Vq4eMh0sKZn4qwT4vtxvKjZ48xkEZ7QUbBybDRyVdMVw@mail.gmail.com>
Date:   Thu, 9 Nov 2023 16:19:11 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Maxime Ripard <mripard@...nel.org>
Cc:     Hsin-Yi Wang <hsinyi@...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>,
        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 v6 4/5] drm/panel-edp: Add override_edid_mode quirk for
 generic edp

Hi,

On Wed, Nov 8, 2023 at 9:04 AM Doug Anderson <dianders@...omium.org> wrote:
>
> Hi,
>
> On Wed, Nov 8, 2023 at 7:45 AM Maxime Ripard <mripard@...nel.org> wrote:
> >
> > > @@ -575,9 +599,18 @@ static int panel_edp_get_modes(struct drm_panel *panel,
> > >
> > >               if (!p->edid)
> > >                       p->edid = drm_get_edid(connector, p->ddc);
> > > -
> > > -             if (p->edid)
> > > -                     num += drm_add_edid_modes(connector, p->edid);
> > > +             if (p->edid) {
> > > +                     if (has_override_edid_mode) {
> >
> > It's not clear to me why the override mechanism is only there when
> > there's a ddc bus?
>
> I think you're confusing the two different (but related) issues
> addressed by this series. One is when you're using the generic
> "edp-panel" compatible string. In that case the mode comes from the
> EDID and only the EDID since there's no hardcoded mode. We need a mode
> override there since some EDIDs shipped with a bad mode. That's the
> subject of ${SUBJECT} patch.
>
> The second issue is what to do with a hardcoded mode. That's the
> subject of the next patch in the series (patch #5). Previously we
> merged the hardcoded and EDID modes. Now (in the next patch) we use
> only the hardcoded mode. There's no need for a fixup because the mode
> is hardcoded in the kernel.
>
>
> > You mentioned before that you were following panel-simple,
>
> As of the newest version of the patch, it's no longer following
> panel-simple in response to your feedback on previous versions.
>
> > but that's a
> > clear deviation from what I can see. If there's a reason for that
> > deviation, that's fine by me, but it should at least be documented in
> > the commit log.
>
> I think the commit log is OK. I suspect the confusion is only because
> you've reviewed previous versions of the series. Please shout if
> things still look confusing.
>
>
> > > @@ -950,6 +983,19 @@ static const struct panel_desc auo_b101ean01 = {
> > >       },
> > >  };
> > >
> > > +static const struct drm_display_mode auo_b116xa3_mode = {
> > > +     .clock = 70589,
> > > +     .hdisplay = 1366,
> > > +     .hsync_start = 1366 + 40,
> > > +     .hsync_end = 1366 + 40 + 40,
> > > +     .htotal = 1366 + 40 + 40 + 32,
> > > +     .vdisplay = 768,
> > > +     .vsync_start = 768 + 10,
> > > +     .vsync_end = 768 + 10 + 12,
> > > +     .vtotal = 768 + 10 + 12 + 6,
> > > +     .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> > > +};
> >
> > That should be a separate patch
>
> That's fair. I didn't think it was a huge deal, but I agree that it's
> slightly cleaner.

I've pushed the first 3 patches but not this patch nor the next one.
It wouldn't hurt to split patch #4 as Maxime requests and then send
the split patch #4 plus patch #5 as a v7. It's probably worth holding
off until either some time passes or Maxime responds and says that his
other concerns are addressed.


-Doug