[Intel-gfx] [PATCH] drm/edid: Adding common CVT inferred modes when monitor allows range limited ones trough EDID.

[Rodrigo Vivi]

Hi Ajax and Takashi,

Thanks for your comments.

> The intent here is great, but I don't like the way this is phrased, or
> the implementation.

To be honest I don't like this implementation as well. I just tried to
follow the way it wasa already there.

> CVT monitors _must_ accept GTF as well, EDID says so.  So functionally
> there's nothing wrong with the existing code.

Actually the current code just check for gtf flag... so if a monitor
is gtf2 or cvt this dmt modes are not being added.

>The thing you're trying
> to sneak in here is a 1600x900 timing that doesn't correspond to
> anything in DMT (at least, not in the copy of DMT that I have handy).
> It's fine to want to add more modes - although I'm still unclear exactly
> which machine you're trying to compensate for here - but not if it comes
> by sacrificing the DMT list, which is there for a reason.

There are customers complaining about lots of missing modes that are
supported by windows and/or other drivers and we are missing. If this
modes are ok on edid spec I se no problem in add them... ok.. I don't
like hardcoded as well... I think we could find another way to invent
this modes and use the cvt function to calculate timings... or
something like that.

>> +     /* 900x600 at 60Hz */
>> +     { DRM_MODE("900x600", DRM_MODE_TYPE_DRIVER, 45250, 960, 992,
>> +                1088, 1216, 0, 600, 603, 609, 624, 0,
>> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },

This one I copied from windows...

>> +     /* 1024x576 at 60Hz */
>> +     { DRM_MODE("1024x576", DRM_MODE_TYPE_DRIVER, 46500, 1024, 1064,
>> +                1160, 1296, 0, 576, 579, 584, 599, 0,
>> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },
>> +     /* 2560x1600 at 60Hz */
>> +     { DRM_MODE("2560x1600", DRM_MODE_TYPE_DRIVER, 348500, 2560, 2760,
>> +                3032, 3504, 0, 1600, 1603, 1609, 1658, 0,
>> +                DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },

These ones came as request from HP.
I'll check how they made that list.

> I tested the patch but it doesn't detect the desired resolutions such
> as 1600x900 or 1366x768, unfortunately.

:( That is said...
So I see no other way of doing this hardcoded...
add them for any monitor isn't good...

>
> Also, about the patch:
>
>
>> +static const struct drm_display_mode drm_cvt_inferred_modes[] = {
>> +     /* 640x480 at 60Hz */
>> +     { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 23750  640, 664,
>
> A missing comma here.

Sorry, I fixed here and ammend to my commit but forgot to format-patch
again before send-email

Thanks
Rodrigo.