From: Doug Anderson <dianders@chromium.org>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>,
Rob Herring <robh+dt@kernel.org>, Sam Ravnborg <sam@ravnborg.org>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Linus W <linus.walleij@linaro.org>,
Daniel Vetter <daniel@ffwll.ch>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>, Steev Klimaszewski <steev@kali.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
Maxime Ripard <mripard@kernel.org>,
David Airlie <airlied@linux.ie>,
dri-devel <dri-devel@lists.freedesktop.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 03/16] drm/edid: Allow the querying/working with the panel ID from the EDID
Date: Wed, 8 Sep 2021 17:24:05 -0700 [thread overview]
Message-ID: <CAD=FV=X-d8XH5bmcAhDGnbs-DHgQ7D6G9g3gRsjo7RN1xQ1kNA@mail.gmail.com> (raw)
In-Reply-To: <87h7ey81e9.fsf@intel.com>
Hi,
On Mon, Sep 6, 2021 at 3:05 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> > +{
> > + struct edid *edid;
> > + u32 val;
> > +
> > + edid = drm_do_get_edid_blk0(drm_do_probe_ddc_edid, adapter, NULL, NULL);
> > +
> > + /*
> > + * There are no manufacturer IDs of 0, so if there is a problem reading
> > + * the EDID then we'll just return 0.
> > + */
> > + if (IS_ERR_OR_NULL(edid))
> > + return 0;
> > +
> > + /*
> > + * In theory we could try to de-obfuscate this like edid_get_quirks()
> > + * does, but it's easier to just deal with a 32-bit number.
>
> Hmm, but is it, really? AFAICT this is just an internal representation
> for a table, where it could just as well be stored in a struct that
> could be just as compact now, but extensible later. You populate the
> table via an encoding macro, then decode the id using a function - while
> it could be in a format that's directly usable without the decode. If
> suitably chosen, the struct could perhaps be reused between the quirks
> code and your code.
I'm not 100% sure, but I think you're suggesting having this function
return a `struct edid_panel_id` or something like that. Is that right?
Maybe that would look something like this?
struct edid_panel_id {
char vendor[4];
u16 product_id;
}
...or perhaps this (untested, but I think it works):
struct edid_panel_id {
u16 vend_c1:5;
u16 vend_c2:5;
u16 vend_c3:5;
u16 product_id;
}
...and then change `struct edid_quirk` to something like this:
static const struct edid_quirk {
struct edid_panel_id panel_id;
u32 quirks;
} ...
Is that correct? There are a few downsides that I can see:
a) I think the biggest downside is the inability compare with "==". I
don't believe it's legal to compare structs with "==" in C. Yeah, we
can use memcmp() but that feels more awkward to me.
b) Unless you use the bitfield approach, it takes up more space. I
know it's not a huge deal, but the format in the EDID is pretty much
_forced_ to fit in 32-bits. The bitfield approach seems like it'd be
more awkward than my encoding macros.
-Doug
next prev parent reply other threads:[~2021-09-09 0:30 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-01 20:19 [PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 01/16] dt-bindings: drm/panel-simple-edp: Introduce generic eDP panels Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 02/16] drm/edid: Break out reading block 0 of the EDID Douglas Anderson
2021-09-06 9:50 ` Jani Nikula
2021-09-01 20:19 ` [PATCH v3 03/16] drm/edid: Allow the querying/working with the panel ID from " Douglas Anderson
2021-09-06 10:05 ` Jani Nikula
2021-09-09 0:24 ` Doug Anderson [this message]
2021-09-14 17:59 ` Jani Nikula
2021-09-01 20:19 ` [PATCH v3 04/16] drm/panel-simple: Reorder logicpd_type_28 / mitsubishi_aa070mc01 Douglas Anderson
2021-09-09 20:48 ` Doug Anderson
2021-09-01 20:19 ` [PATCH v3 05/16] drm/panel-simple-edp: Split eDP panels out of panel-simple Douglas Anderson
[not found] ` <YTUPiyOjsUJXN11h@ravnborg.org>
2021-09-09 19:33 ` Doug Anderson
2021-09-01 20:19 ` [PATCH v3 06/16] ARM: configs: Everyone who had PANEL_SIMPLE now gets PANEL_SIMPLE_EDP Douglas Anderson
2021-09-01 21:12 ` Olof Johansson
2021-09-01 23:10 ` Doug Anderson
[not found] ` <CGME20210903071832eucas1p10a7b8a295e68df4d2735110c9ec09cf1@eucas1p1.samsung.com>
2021-09-03 7:18 ` Andrzej Hajda
[not found] ` <163070152582.405991.9480635890491684680@swboyd.mtv.corp.google.com>
2021-09-08 22:36 ` Doug Anderson
2021-09-08 23:08 ` Olof Johansson
2021-09-01 20:19 ` [PATCH v3 07/16] arm64: defconfig: " Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 08/16] MIPS: configs: " Douglas Anderson
2021-09-01 20:39 ` Paul Cercueil
2021-09-01 20:19 ` [PATCH v3 09/16] drm/panel-simple-edp: Move some wayward panels to the eDP driver Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 10/16] drm/panel-simple: Non-eDP panels don't need "HPD" handling Douglas Anderson
[not found] ` <YTUQhnt0GxYxqg/i@ravnborg.org>
2021-09-08 21:10 ` Doug Anderson
2021-09-01 20:19 ` [PATCH v3 11/16] drm/panel-simple-edp: Split the delay structure out Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 12/16] drm/panel-simple-edp: Better describe eDP panel delays Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 13/16] drm/panel-simple-edp: hpd_reliable shouldn't be subtraced from hpd_absent Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 14/16] drm/panel-simple-edp: Fix "prepare_to_enable" if panel doesn't handle HPD Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 15/16] drm/panel-simple-edp: Don't re-read the EDID every time we power off the panel Douglas Anderson
2021-09-01 20:19 ` [PATCH v3 16/16] drm/panel-simple-edp: Implement generic "edp-panel"s probed by EDID Douglas Anderson
[not found] ` <CGME20210902221015eucas1p26fae8f6ba4c70087dc7b007a271dce4b@eucas1p2.samsung.com>
2021-09-02 22:10 ` [PATCH v3 00/16] eDP: Support probing eDP panels dynamically instead of hardcoding Andrzej Hajda
2021-09-02 22:33 ` Doug Anderson
[not found] ` <YTUSiHiCgihz1AcO@ravnborg.org>
2021-09-09 0:24 ` Doug Anderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAD=FV=X-d8XH5bmcAhDGnbs-DHgQ7D6G9g3gRsjo7RN1xQ1kNA@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=airlied@linux.ie \
--cc=bjorn.andersson@linaro.org \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=robh+dt@kernel.org \
--cc=sam@ravnborg.org \
--cc=steev@kali.org \
--cc=thierry.reding@gmail.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).