Hi,
On 6/18/22 23:08, Maya Matuszczyk wrote:
> sob., 18 cze 2022 o 22:57 Hans de Goede <hdegoede@xxxxxxxxxx> napisał(a):
>>
>> Hi Maya,
>>
>> On 6/11/22 12:39, Maccraft123 wrote:
>>> From: Maya Matuszczyk <maccraft123mc@xxxxxxxxx>
>>>
>>> The device is identified by "NEXT" in board name, however there are
>>> different versions of it, "Next Advance" and "Next Pro", that have
>>> different DMI board names.
>>> Due to a production error a batch or two have their board names prefixed
>>> by "AYANEO" and that's how we ended up with 6 different entries for
>>> what's basically one device with different hardware specifications.
>>>
>>> Signed-off-by: Maya Matuszczyk <maccraft123mc@xxxxxxxxx>
>>> ---
>>> .../gpu/drm/drm_panel_orientation_quirks.c | 36 +++++++++++++++++++
>>> 1 file changed, 36 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c b/drivers/gpu/drm/drm_panel_orientation_quirks.c
>>> index 4e853acfd1e8..62fc7eb69341 100644
>>> --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
>>> +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
>>> @@ -152,6 +152,42 @@ static const struct dmi_system_id orientation_data[] = {
>>> DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "AYA NEO 2021"),
>>> },
>>> .driver_data = (void *)&lcd800x1280_rightside_up,
>>> + }, { /* AYA NEO NEXT */
>>> + .matches = {
>>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>>> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "NEXT"),
>>> + },
>>> + .driver_data = (void *)&lcd800x1280_rightside_up,
>>> + },
>>
>> If you change:
>>
>> DMI_EXACT_MATCH(DMI_BOARD_NAME, "NEXT"),
>>
>> to:
>>
>> DMI_MATCH(DMI_BOARD_NAME, "NEXT"),
>>
>> Then the matching code will do a substring search for "NEXT"
>> in DMI_BOARD_NAME. IOW it will do a strstr using DMI_BOARD_NAME
>> as the haystack and "NEXT" as the needle.
>>
>> And since all 6 board-names contain "NEXT", this single entry
>> will then match all 6 variants.
>>
>> So please respin this patch using just a single entry with:
>>
>> DMI_MATCH(DMI_BOARD_NAME, "NEXT"),
>>
> I considered this and decided against it because if "Aya Neo Next 2" was
> released it would likely have "NEXT 2" as DMI board name, and if it had
> screen rotated differently this patch would rotate it in a wrong way.
Hmm, I understand but I'm not sure what is the best thing to do.
There also is the resolution check. So if a new "NEXT 2" is released
then this really only is an issue if it too uses a 800x1280 screen,
but then with its left-side-up.
If it uses a normal landscape 1280x800 screen the resolution won't
match the 800x1280 the quirk checks for so nothing happens.
If it again uses a 800x1280 portrait screen in landscape mode
with its right-side-up then the quirk matching actualy is
a good thing.
So the only bad scenario is a 800x1280 screen which is left-side-up,
which the quirk will then turn from being on its side to being
upside-down. The question then is though if upside-down is much
worse then on its side, neither is really usable, so we would
need a quirk for this screen then anyways...
So I understand where you are coming from, but I see the chances
of this becoming a problem as pretty small (and we can fix it
when it does happen). OTOH the dmi_system_id table we are
discussing is almost always built into the vmlinuz image.
So growing it will cause extra disk/RAM uses for every Linux
user out there (not just Aya Neo Next users).
So IMHO going with only using 1 entry with the small risc of
needing to revisit this later is the better solution here.
Regards,
Hans
>
>
>> Thanks & Regards,
>>
>> Hans
>>
>>
>>
>> { /* AYA NEO NEXT Advance */
>>> + .matches = {
>>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>>> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "NEXT Advance"),
>>> + },
>>> + .driver_data = (void *)&lcd800x1280_rightside_up,
>>> + }, { /* AYA NEO NEXT Pro */
>>> + .matches = {
>>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>>> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "NEXT Pro"),
>>> + },
>>> + .driver_data = (void *)&lcd800x1280_rightside_up,
>>> + }, { /* AYA NEO NEXT (Batch with different board name) */
>>> + .matches = {
>>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>>> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "AYANEO NEXT"),
>>> + },
>>> + .driver_data = (void *)&lcd800x1280_rightside_up,
>>> + }, { /* AYA NEO NEXT Advance (Batch with different board name) */
>>> + .matches = {
>>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>>> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "AYANEO NEXT Advance"),
>>> + },
>>> + .driver_data = (void *)&lcd800x1280_rightside_up,
>>> + }, { /* AYA NEO NEXT Pro (Batch with different board name) */
>>> + .matches = {
>>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
>>> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "AYANEO NEXT Pro"),
>>> + },
>>> + .driver_data = (void *)&lcd800x1280_rightside_up,
>>> }, { /* Chuwi HiBook (CWI514) */
>>> .matches = {
>>> DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
>>
>
