Hi,

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

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.

I _think_ you could make it work with four-character codes (only
specifying 3 characters), AKA single-quoting something like 'AUO' but
I think four-character codes are not dealt with in a standard enough
way between compilers so they're not allowed in Linux.

If you like it better, I could do something like this:

#define ACR_CODE 'A', 'C', 'R'
#define AUO_CODE 'A', 'U', 'O'
...
...

...then I could refer to the #defines...

Just bikeshedding, really. ;)

I'll take this comment (without any formal tags) as:

* You've seen this patch (and the previous ones) and wouldn't object
to it merging.

* You're not planning on any deeper review / testing, so I shouldn't
wait for more stuff from you before merging. Please yell if this is
not the case. I'm happy to wait but I don't want to wait if no further
review is planned.


In general I'm still planning to give this series at least another
week for comments before merging. ${SUBJECT} patch also is the only
one lacking any type of Review / Ack tags so I'll see if I can find
someone to give it something before merging, too.


-Doug