Hi,

On Tue, Sep 14, 2021 at 12:36 PM Andrzej Hajda [off-list ref] wrote:

W dniu 14.09.2021 o 20:59, Jani Nikula pisze:

quoted

On Tue, 14 Sep 2021, Doug Anderson [off-list ref] wrote:

quoted

Hi,

On Tue, Sep 14, 2021 at 11:16 AM Jani Nikula
[off-list ref] wrote:

quoted

On Thu, 09 Sep 2021, Douglas Anderson [off-list ref] wrote:

quoted

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