linux-kernel - Re: [PATCH v2 09/14] drm/edid: Use the cached EDID in drm_get

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:   Tue, 30 Mar 2021 14:31:41 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Ville Syrjälä <ville.syrjala@...ux.intel.com>
Cc:     Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Sam Ravnborg <sam@...nborg.org>,
        Rob Clark <robdclark@...omium.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        David Airlie <airlied@...ux.ie>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Steev Klimaszewski <steev@...i.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Stanislav Lisovskiy <stanislav.lisovskiy@...el.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 09/14] drm/edid: Use the cached EDID in drm_get_edid()
 if eDP

Hi,

On Tue, Mar 30, 2021 at 7:01 AM Ville Syrjälä
<ville.syrjala@...ux.intel.com> wrote:
>
> > @@ -2049,15 +2049,39 @@ struct edid *drm_get_edid(struct drm_connector *connector,
> >                         struct i2c_adapter *adapter)
> >  {
> >       struct edid *edid;
> > +     size_t old_edid_size;
> > +     const struct edid *old_edid;
> >
> >       if (connector->force == DRM_FORCE_OFF)
> >               return NULL;
> >
> > -     if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))
> > -             return NULL;
> > +     if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> > +         connector->edid_blob_ptr) {
> > +             /*
> > +              * eDP devices are non-removable, or at least not something
> > +              * that's expected to be hot-pluggable. We can freely use
> > +              * the cached EDID.
> > +              *
> > +              * NOTE: technically we could probably even use the cached
> > +              * EDID even for non-eDP because the cached EDID should be
> > +              * cleared if we ever notice a display is not connected, but
> > +              * we'll use an abundance of caution and only do it for eDP.
> > +              * It's more important for eDP anyway because the EDID may not
> > +              * always be readable, like when the panel is powered down.
> > +              */
> > +             old_edid = (const struct edid *)connector->edid_blob_ptr->data;
> > +             old_edid_size = ksize(old_edid);
> > +             edid = kmalloc(old_edid_size, GFP_KERNEL);
> > +             if (edid)
> > +                     memcpy(edid, old_edid, old_edid_size);
> > +     } else {
> > +             if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))
> > +                     return NULL;
> > +
> > +             edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
> > +             drm_connector_update_edid_property(connector, edid);
> > +     }
>
> This is a pretty low level function. Too low level for this caching
> IMO. So I think better just do it a bit higher up like other drivers.

Fair enough. In the past I'd gotten feedback that I'd been jamming too
much stuff in my own driver instead of putting it in the core, but I'm
happy to leave the EDID caching in the driver if that's what people
prefer. It actually makes a bit of the code in the driver a bit less
awkward...

-Doug