Re: [PATCH 1/2] drm_edid: Add a function to get EDID base block

Doug Anderson
2024-02-29T00:20:52Z

Hi,

On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> > > It's found that some panels have variants that they share the same panel id
> > > although their EDID and names are different. Besides panel id, now we need
> > > the hash of entire EDID base block to distinguish these panel variants.
> > >
> > > Add drm_edid_get_base_block to returns the EDID base block, so caller can
> > > further use it to get panel id and/or the hash.
> >
> > Please reconsider the whole approach here.
> >
> > Please let's not add single-use special case functions to read an EDID
> > base block.
> >
> > Please consider reading the whole EDID, using the regular EDID reading
> > functions, and use that instead.
> >
> > Most likely you'll only have 1-2 blocks anyway. And you might consider
> > caching the EDID in struct panel_edp if reading the entire EDID is too
> > slow. (And if it is, this is probably sensible even if the EDID only
> > consists of one block.)
> >
> > Anyway, please do *not* merge this as-is.
> >
>
> hi Jani,
>
> I sent a v2 here implementing this method:
> https://lore.kernel.org/lkml/20240228011133.1238439-2-hsinyi@chromium.org/
>
> We still have to read edid twice due to:
> 1. The first caller is in panel probe, at that time, connector is
> still unknown, so we can't update connector status (eg. update
> edid_corrupt).
> 2. It's possible that the connector can have some override
> (drm_edid_override_get) to EDID, that is still unknown during the
> first read.

I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the
fact that we can't cache the EDID (because we don't yet have a
"drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that
allows reading just the first block. If we try to boot a device with a
multi-block EDID we're now wastefully reading all the blocks of the
EDID twice at bootup which will slow boot time.

If you can see a good solution to avoid reading the EDID twice then
that would be amazing, but if not it seems like we should go back to
what's here in v1. What do you think? Anyone else have any opinions?

-Doug