Hi,
On Tue, Sep 14, 2021 at 12:36 PM Andrzej Hajda a.hajda@samsung.com wrote:
W dniu 14.09.2021 o 20:59, Jani Nikula pisze:
On Tue, 14 Sep 2021, Doug Anderson dianders@chromium.org wrote:
Hi,
On Tue, Sep 14, 2021 at 11:16 AM Jani Nikula jani.nikula@linux.intel.com wrote:
On Thu, 09 Sep 2021, Douglas Anderson dianders@chromium.org wrote:
In the patch ("drm/edid: Allow the querying/working with the panel ID from the EDID") we introduced a different way of working with the panel ID stored in the EDID. Let's use this new way for the quirks code.
Advantages of the new style:
- Smaller data structure size. Saves 4 bytes per panel.
- Iterate through quirks structure with just "==" instead of strncmp()
- In-kernel storage is more similar to what's stored in the EDID itself making it easier to grok that they are referring to the same value.
The quirk table itself is arguably a bit less readable in the new style but not a ton less and it feels like the above advantages make up for it.
I suppose you could pass vendor as a string to EDID_QUIRK() to retain better readability?
I would love to, but I couldn't figure out how to do this and have it compile! Notably I need the compiler to be able to do math at compile time to compute the final u32 to store in the init data. I don't think the compiler can dereference strings (even constant strings) and do math on the result at compile time.
Ah, right.
What about:
+#define drm_edid_encode_panel_id(vend, product_id) \
((((u32)((vend)[0]) - '@') & 0x1f) << 26 | \
(((u32)((vend)[1]) - '@') & 0x1f) << 21 | \
(((u32)((vend)[2]) - '@') & 0x1f) << 16 | \
((product_id) & 0xffff))
Wow, I _swear_ I tried exactly that syntax, but clearly I didn't. It works great and it looks _sooo_ much nicer now. Thanks! I'll send out a v5 shortly with this.
-Doug