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.
