[PATCH] drm/edid: Adding common CVT inferred modes when monitor allows range limited ones trough EDID.
There are many bugs open on fd.o regarding missing modes that are supported on Windows and other closed source drivers.
From EDID spec we can (might?) infer modes using GTF and CVT when monitor allows it trough range limited flag... obviously limiting by the range. From our code:
* EDID spec says modes should be preferred in this order: * - preferred detailed mode * - other detailed modes from base block * - detailed modes from extension blocks * - CVT 3-byte code modes * - standard timing codes * - established timing codes * - modes inferred from GTF or CVT range information * * We get this pretty much right.
Not actually so right... We were inferring just using GTF... not CVT or even GTF2. This patch not just add some common cvt modes but also allows some modes been inferred when using gtf2 as well.
Cheers, Rodrigo.
From 4b7a88d0d812583d850ca691d1ac491355230d11 Mon Sep 17 00:00:00 2001
From: Rodrigo Vivi rodrigo.vivi@intel.com Date: Wed, 11 Apr 2012 15:36:31 -0300 Subject: [PATCH] drm/edid: Adding common CVT inferred modes when monitor allows range limited ones trough EDID.
Signed-off-by: Rodrigo Vivi rodrigo.vivi@intel.com --- drivers/gpu/drm/drm_edid.c | 37 +++++++++++++- drivers/gpu/drm/drm_edid_modes.h | 101 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7ee7be1..3179572 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1020,17 +1020,50 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid, return modes; }
+static int +drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid, + struct detailed_timing *timing) +{ + int i, modes = 0; + struct drm_display_mode *newmode; + struct drm_device *dev = connector->dev; + + for (i = 0; i < drm_num_cvt_inferred_modes; i++) { + if (mode_in_range(drm_cvt_inferred_modes + i, edid, timing)) { + newmode = drm_mode_duplicate(dev, &drm_cvt_inferred_modes[i]); + if (newmode) { + drm_mode_probed_add(connector, newmode); + modes++; + } + } + } + + return modes; +} + static void do_inferred_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; struct detailed_non_pixel *data = &timing->data.other_data; - int gtf = (closure->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF); + int timing_level = standard_timing_level(closure->edid);
- if (gtf && data->type == EDID_DETAIL_MONITOR_RANGE) + if (data->type == EDID_DETAIL_MONITOR_RANGE) + switch (timing_level) { + case LEVEL_DMT: + break; + case LEVEL_GTF: + case LEVEL_GTF2: closure->modes += drm_gtf_modes_for_range(closure->connector, closure->edid, timing); + break; + case LEVEL_CVT: + closure->modes += drm_cvt_modes_for_range(closure->connector, + closure->edid, + timing); + break; + } }
static int diff --git a/drivers/gpu/drm/drm_edid_modes.h b/drivers/gpu/drm/drm_edid_modes.h index a91ffb1..7e14a32 100644 --- a/drivers/gpu/drm/drm_edid_modes.h +++ b/drivers/gpu/drm/drm_edid_modes.h @@ -266,6 +266,107 @@ static const struct drm_display_mode drm_dmt_modes[] = { static const int drm_num_dmt_modes = sizeof(drm_dmt_modes) / sizeof(struct drm_display_mode);
+static const struct drm_display_mode drm_cvt_inferred_modes[] = { + /* 640x480@60Hz */ + { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 23750 640, 664, + 720, 800, 0, 480, 483, 487, 500, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 800x600@60Hz */ + { DRM_MODE("800x600", DRM_MODE_TYPE_DRIVER, 38250, 800, 832, + 912, 1024, 0, 600, 603, 607, 624, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 900x600@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) }, + /* 1024x576@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) }, + /* 1024x768@60Hz */ + { DRM_MODE("1024x768", DRM_MODE_TYPE_DRIVER, 63500, 1024, 1072, + 1176, 1328, 0, 768, 771, 775, 798, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1152x864@60Hz */ + { DRM_MODE("1152x864", DRM_MODE_TYPE_DRIVER, 81750, 1152, 1216, + 1336, 1520, 0, 864, 867, 871, 897, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1280x720@60Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74500, 1280, 1344, + 1472, 1664, 0, 720, 723, 728, 748, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1280x768@60Hz */ + { DRM_MODE("1280x768", DRM_MODE_TYPE_DRIVER, 79500, 1280, 1344, + 1472, 1664, 0, 768, 771, 781, 798, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1280x800@60Hz */ + { DRM_MODE("1280x800", DRM_MODE_TYPE_DRIVER, 83500, 1280, 1352, + 1480, 1680, 0, 800, 803, 809, 831, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1280x1024@60Hz */ + { DRM_MODE("1280x1024", DRM_MODE_TYPE_DRIVER, 109000, 1280, 1368, + 1496, 1712, 0, 1024, 1027, 1034, 1063, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1360x768@60Hz */ + { DRM_MODE("1360x768", DRM_MODE_TYPE_DRIVER, 84750, 1360, 1432, + 1568, 1776, 0, 768, 771, 781, 798, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1366x768@60Hz */ + { DRM_MODE("1366x768", DRM_MODE_TYPE_DRIVER, 85250, 1368, 1440, + 1576, 1784, 0, 768, 771, 781, 798, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1440x900@60Hz */ + { DRM_MODE("1440x900", DRM_MODE_TYPE_DRIVER, 106500, 1440, 1528, + 1672, 1904, 0, 900, 903, 909, 934, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1400x1050@60Hz */ + { DRM_MODE("1400x1050", DRM_MODE_TYPE_DRIVER, 121750, 1400, 1488, + 1632, 1864, 0, 1050, 1053, 1057, 1089, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1600x900@60Hz */ + { DRM_MODE("1600x900", DRM_MODE_TYPE_DRIVER, 118250, 1600, 1696, + 1856, 2112, 0, 900, 903, 908, 934, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1600x1200@60Hz */ + { DRM_MODE("1600x1200", DRM_MODE_TYPE_DRIVER, 161000, 1600, 1712, + 1880, 2160, 0, 1200, 1203, 1207, 1245, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1680x945@60Hz */ + { DRM_MODE("1680x945", DRM_MODE_TYPE_DRIVER, 130750, 1680, 1776, + 1952, 2224, 0, 945, 948, 953, 981, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1680x1050@60Hz */ + { DRM_MODE("1680x1050", DRM_MODE_TYPE_DRIVER, 146250, 1680, 1784, + 1960, 2240, 0, 1050, 1053, 1059, 1089, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1920x1080@60Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 73000, 1920, 2048, + 2248, 2576, 0, 1080, 1083, 1088, 1120, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1920x1200@60Hz */ + { DRM_MODE("1920x1200", DRM_MODE_TYPE_DRIVER, 193250, 1920, 2056, + 2256, 2592, 0, 1200, 1203, 1209, 1245, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 1920x1440@60Hz */ + { DRM_MODE("1920x1440", DRM_MODE_TYPE_DRIVER, 233500, 1920, 2064, + 2264, 2608, 0, 1440, 1443, 1447, 1493, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 2048x1152@60Hz */ + { DRM_MODE("2048x1152", DRM_MODE_TYPE_DRIVER, 197000, 2048, 2184, + 2400, 2752, 0, 1152, 1155, 1160, 1195, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 2048x1536@60Hz */ + { DRM_MODE("2048x1536", DRM_MODE_TYPE_DRIVER, 272000, 2048, 2208, + 2424, 2800, 0, 1563, 1566, 1576, 1620, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, + /* 2560x1600@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) }, +}; +static const int drm_num_cvt_inferred_modes = + sizeof(drm_cvt_inferred_modes) / sizeof(struct drm_display_mode); + static const struct drm_display_mode edid_est_modes[] = { { DRM_MODE("800x600", DRM_MODE_TYPE_DRIVER, 40000, 800, 840, 968, 1056, 0, 600, 601, 605, 628, 0,
On Wed, 2012-04-11 at 21:59 -0300, Rodrigo Vivi wrote:
There are many bugs open on fd.o regarding missing modes that are supported on Windows and other closed source drivers. From EDID spec we can (might?) infer modes using GTF and CVT when monitor allows it trough range limited flag... obviously limiting by the range. From our code:
- EDID spec says modes should be preferred in this order:
- preferred detailed mode
- other detailed modes from base block
- detailed modes from extension blocks
- CVT 3-byte code modes
- standard timing codes
- established timing codes
- modes inferred from GTF or CVT range information
- We get this pretty much right.
Not actually so right... We were inferring just using GTF... not CVT or even GTF2. This patch not just add some common cvt modes but also allows some modes been inferred when using gtf2 as well.
The intent here is great, but I don't like the way this is phrased, or the implementation.
CVT monitors _must_ accept GTF as well, EDID says so. So functionally there's nothing wrong with the existing code. 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.
So...
+static int +drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+{
- int i, modes = 0;
- struct drm_display_mode *newmode;
- struct drm_device *dev = connector->dev;
- for (i = 0; i < drm_num_cvt_inferred_modes; i++) {
if (mode_in_range(drm_cvt_inferred_modes + i, edid, timing)) {newmode = drm_mode_duplicate(dev, &drm_cvt_inferred_modes[i]);if (newmode) {drm_mode_probed_add(connector, newmode);modes++;}}- }
- return modes;
+}
The mode list you're iterating over here should just be a w/h/r/rb tuple like est3_modes. This list should _not_ duplicate any size or rate already present in the DMT list. There should be a function like this for each of CVT and GTF (and I guess dual-curve GTF too, although honestly I have no monitors left that support it with which to test), all iterating over the same list, and they should generate the timing from drm_{cvt,gtf}_mode(). The CVT version should generate RB modes if the monitor is RB-capable.
static void do_inferred_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; struct detailed_non_pixel *data = &timing->data.other_data;
- int gtf = (closure->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF);
- int timing_level = standard_timing_level(closure->edid);
- if (gtf && data->type == EDID_DETAIL_MONITOR_RANGE)
- if (data->type == EDID_DETAIL_MONITOR_RANGE)
switch (timing_level) {case LEVEL_DMT:break;case LEVEL_GTF:case LEVEL_GTF2:break;case LEVEL_CVT:closure->modes += drm_cvt_modes_for_range(closure->connector,closure->edid,timing);break;}}
drm_gtf_modes_for_range should be renamed drm_dmt_modes_for_range, and run unconditionally for the range descriptor. drm_*_modes_for_range will then handle generating the timings by formula.
- /* 900x600@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) },- /* 1024x576@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) },
Citation needed. Can you point to real hardware with these panel sizes, or are you just copying from Windows?
- /* 2560x1600@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) },
lol no. Nobody does a size this large without also doing reduced blanking.
I have a patch somewhere to fix the DMT list to re-include the reduced blanking modes (as should have been done in the first place). I'll send that along.
- ajax
At Wed, 11 Apr 2012 21:59:28 -0300, Rodrigo Vivi wrote:
There are many bugs open on fd.o regarding missing modes that are supported on Windows and other closed source drivers.
From EDID spec we can (might?) infer modes using GTF and CVT when monitor allows it trough range limited flag... obviously limiting by the range. From our code:
- EDID spec says modes should be preferred in this order:
- preferred detailed mode
- other detailed modes from base block
- detailed modes from extension blocks
- CVT 3-byte code modes
- standard timing codes
- established timing codes
- modes inferred from GTF or CVT range information
- We get this pretty much right.
Not actually so right... We were inferring just using GTF... not CVT or even GTF2. This patch not just add some common cvt modes but also allows some modes been inferred when using gtf2 as well.
I tested the patch but it doesn't detect the desired resolutions such as 1600x900 or 1366x768, unfortunately.
Also, about the patch:
+static const struct drm_display_mode drm_cvt_inferred_modes[] = {
- /* 640x480@60Hz */
- { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 23750 640, 664,
A missing comma here.
720, 800, 0, 480, 483, 487, 500, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 800x600@60Hz */
- { DRM_MODE("800x600", DRM_MODE_TYPE_DRIVER, 38250, 800, 832,
912, 1024, 0, 600, 603, 607, 624, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 900x600@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) },- /* 1024x576@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) },- /* 1024x768@60Hz */
- { DRM_MODE("1024x768", DRM_MODE_TYPE_DRIVER, 63500, 1024, 1072,
1176, 1328, 0, 768, 771, 775, 798, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1152x864@60Hz */
- { DRM_MODE("1152x864", DRM_MODE_TYPE_DRIVER, 81750, 1152, 1216,
1336, 1520, 0, 864, 867, 871, 897, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1280x720@60Hz */
- { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74500, 1280, 1344,
1472, 1664, 0, 720, 723, 728, 748, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1280x768@60Hz */
- { DRM_MODE("1280x768", DRM_MODE_TYPE_DRIVER, 79500, 1280, 1344,
1472, 1664, 0, 768, 771, 781, 798, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1280x800@60Hz */
- { DRM_MODE("1280x800", DRM_MODE_TYPE_DRIVER, 83500, 1280, 1352,
1480, 1680, 0, 800, 803, 809, 831, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1280x1024@60Hz */
- { DRM_MODE("1280x1024", DRM_MODE_TYPE_DRIVER, 109000, 1280, 1368,
1496, 1712, 0, 1024, 1027, 1034, 1063, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1360x768@60Hz */
- { DRM_MODE("1360x768", DRM_MODE_TYPE_DRIVER, 84750, 1360, 1432,
1568, 1776, 0, 768, 771, 781, 798, 0,DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) },- /* 1366x768@60Hz */
- { DRM_MODE("1366x768", DRM_MODE_TYPE_DRIVER, 85250, 1368, 1440,
Here the hdisplay is 1368, not 1366.
thanks,
Takashi
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@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@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@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@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.
On 4/12/12 7:09 PM, Rodrigo Vivi wrote:
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.
Yeah, that's a bug. That's why I said it should be renamed drm_dmt_modes_for_range and run unconditionally if we find a range descriptor.
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.
Why are they complaining, and why do you think you're obligated to care?
If it's not the native panel size and it's not a commonly found size in the wild, why are we obligated to provide them for every user? Remember that userspace has the ability to hand in modes from above.
/* 1024x576@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@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.
1024x576 almost makes some sense, it's the fat-pixel version of 2048x1152. Except that you typically do fat-pixel modes like 1280x800 on a 2560x1600 screen because you're compensating for the host not being able to do dual-link DVI (cough Intel), but 2048x1152 fits in a single link.
That 2560 mode appears to have been copied from the DMT list, but seriously nobody does that size without doing reduced blanking. So if we took my series to add the RB modes to the DMT list, and fixed that bug I mentioned above...
- ajax
At Fri, 13 Apr 2012 10:14:46 -0400, Adam Jackson wrote:
On 4/12/12 7:09 PM, Rodrigo Vivi wrote:
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.
Yeah, that's a bug. That's why I said it should be renamed drm_dmt_modes_for_range and run unconditionally if we find a range descriptor.
Yeah, I saw your patches. Should the further work base on them?
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.
Why are they complaining, and why do you think you're obligated to care?
Because it's his job. And mine, too. What a pity.
Yesterday I've read a news reporting that 1366x768 is the most commonly used panel now, more than 1024x768. And, 1600x900 is in the second place of the modern laptop panels.
Windows and others do work with these resolutions on the same monitor. Why Linux driver can't? Everbody (but developers) thinks like that way.
If it's not the native panel size and it's not a commonly found size in the wild, why are we obligated to provide them for every user? Remember that userspace has the ability to hand in modes from above.
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
thanks,
Takashi
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
You guys still haven't answered the basic question, what HW is this broken?
Can you provide some EDIDs?
Dave.
At Fri, 13 Apr 2012 15:35:04 +0100, Dave Airlie wrote:
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
You guys still haven't answered the basic question, what HW is this broken?
The reported problem is about HP laptops with i915 driver (no matter chip chip is) and several monitors with resolutions more than the laptop panel.
The LVDS provides only the native resolution (either 1366x768 or 1600x900) and a few other VESA ones (1024x768, 800x600 and 640x480). Meanwhile, the monitor EDID doesn't provide such laptop-native resolutions. Thus, in clone mode, the only possible resolution is 1024x768 or lower. That's the whole problem. It's too low and doesn't match with 16:9 although both laptop and monitor panels are 16:9.
HP wants the clone mode of the laptop-native resolution and/or a higher resolution with the right aspect ratio like 1280x720. Neither work as of now unless you add the extra mode manually.
Can you provide some EDIDs?
OK, here we go. Outputs of "xrandr --verbose" and EDID data from two monitors.
thanks,
Takashi
On Fri, Apr 13, 2012 at 11:13 AM, Takashi Iwai tiwai@suse.de wrote:
At Fri, 13 Apr 2012 15:35:04 +0100, Dave Airlie wrote:
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
You guys still haven't answered the basic question, what HW is this broken?
The reported problem is about HP laptops with i915 driver (no matter chip chip is) and several monitors with resolutions more than the laptop panel.
The LVDS provides only the native resolution (either 1366x768 or 1600x900) and a few other VESA ones (1024x768, 800x600 and 640x480). Meanwhile, the monitor EDID doesn't provide such laptop-native resolutions. Thus, in clone mode, the only possible resolution is 1024x768 or lower. That's the whole problem. It's too low and doesn't match with 16:9 although both laptop and monitor panels are 16:9.
HP wants the clone mode of the laptop-native resolution and/or a higher resolution with the right aspect ratio like 1280x720. Neither work as of now unless you add the extra mode manually.
One thing to be careful of is that some monitors (especially LCD panels) don't like modes that are not in their EDIDs. As such when you try and set them you often get a wonky display or more often a blank screen. We used to add a lot of inferred modes to the mode list in the xserver which resulted in a lot of blank screens when some odd mode was picked as the best match for a cloned display. The "fix" was to only add the inferred modes on analog monitors which were more likely to be able to support them.
Alex
Can you provide some EDIDs?
OK, here we go. Outputs of "xrandr --verbose" and EDID data from two monitors.
thanks,
Takashi
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
At Fri, 13 Apr 2012 11:30:01 -0400, Alex Deucher wrote:
On Fri, Apr 13, 2012 at 11:13 AM, Takashi Iwai tiwai@suse.de wrote:
At Fri, 13 Apr 2012 15:35:04 +0100, Dave Airlie wrote:
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
You guys still haven't answered the basic question, what HW is this broken?
The reported problem is about HP laptops with i915 driver (no matter chip chip is) and several monitors with resolutions more than the laptop panel.
The LVDS provides only the native resolution (either 1366x768 or 1600x900) and a few other VESA ones (1024x768, 800x600 and 640x480). Meanwhile, the monitor EDID doesn't provide such laptop-native resolutions. Thus, in clone mode, the only possible resolution is 1024x768 or lower. That's the whole problem. It's too low and doesn't match with 16:9 although both laptop and monitor panels are 16:9.
HP wants the clone mode of the laptop-native resolution and/or a higher resolution with the right aspect ratio like 1280x720. Neither work as of now unless you add the extra mode manually.
One thing to be careful of is that some monitors (especially LCD panels) don't like modes that are not in their EDIDs. As such when you try and set them you often get a wonky display or more often a blank screen. We used to add a lot of inferred modes to the mode list in the xserver which resulted in a lot of blank screens when some odd mode was picked as the best match for a cloned display. The "fix" was to only add the inferred modes on analog monitors which were more likely to be able to support them.
Thanks, it's good to know!
Though, I still wonder whether adding inferred modes for 1366x768 or 1600x900 would cause any big problems. On such monitors, 1360x768 or 1440x900 (or 1680x1050) are usually seen in the supported list.
Of course, it's never 100% safe. But not so bad odds?
Takashi
On Fri, Apr 13, 2012 at 11:41 AM, Takashi Iwai tiwai@suse.de wrote:
At Fri, 13 Apr 2012 11:30:01 -0400, Alex Deucher wrote:
On Fri, Apr 13, 2012 at 11:13 AM, Takashi Iwai tiwai@suse.de wrote:
At Fri, 13 Apr 2012 15:35:04 +0100, Dave Airlie wrote:
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
You guys still haven't answered the basic question, what HW is this broken?
The reported problem is about HP laptops with i915 driver (no matter chip chip is) and several monitors with resolutions more than the laptop panel.
The LVDS provides only the native resolution (either 1366x768 or 1600x900) and a few other VESA ones (1024x768, 800x600 and 640x480). Meanwhile, the monitor EDID doesn't provide such laptop-native resolutions. Thus, in clone mode, the only possible resolution is 1024x768 or lower. That's the whole problem. It's too low and doesn't match with 16:9 although both laptop and monitor panels are 16:9.
HP wants the clone mode of the laptop-native resolution and/or a higher resolution with the right aspect ratio like 1280x720. Neither work as of now unless you add the extra mode manually.
One thing to be careful of is that some monitors (especially LCD panels) don't like modes that are not in their EDIDs. As such when you try and set them you often get a wonky display or more often a blank screen. We used to add a lot of inferred modes to the mode list in the xserver which resulted in a lot of blank screens when some odd mode was picked as the best match for a cloned display. The "fix" was to only add the inferred modes on analog monitors which were more likely to be able to support them.
Thanks, it's good to know!
Though, I still wonder whether adding inferred modes for 1366x768 or 1600x900 would cause any big problems. On such monitors, 1360x768 or 1440x900 (or 1680x1050) are usually seen in the supported list.
Of course, it's never 100% safe. But not so bad odds?
Probably ok on recent LCD panels as long as rb cvt modes are mostly used. Just something to keep in mind.
Alex
On 4/13/12 11:41 AM, Takashi Iwai wrote:
At Fri, 13 Apr 2012 11:30:01 -0400, Alex Deucher wrote:
One thing to be careful of is that some monitors (especially LCD panels) don't like modes that are not in their EDIDs. As such when you try and set them you often get a wonky display or more often a blank screen. We used to add a lot of inferred modes to the mode list in the xserver which resulted in a lot of blank screens when some odd mode was picked as the best match for a cloned display. The "fix" was to only add the inferred modes on analog monitors which were more likely to be able to support them.
Thanks, it's good to know!
Though, I still wonder whether adding inferred modes for 1366x768 or 1600x900 would cause any big problems. On such monitors, 1360x768 or 1440x900 (or 1680x1050) are usually seen in the supported list.
Of course, it's never 100% safe. But not so bad odds?
"Mostly working" is a fancy way of saying "broken".
The semantics of the range descriptor are "I can support modes within these ranges generated by these timing formulas and/or listed in DMT". That's why we only walk that mode list when we find a range descriptor: if you _don't_ find a range descriptor then the monitor is explicitly telling you it doesn't support arbitrary modes over a range.
You can be more aggressive than that if you know your CRTC's scaler can compensate, in which case you'd run the display at the native mode and let the scaler translate. The EDID parser is currently not told that bit of context, and in a sense it really shouldn't; it would be a function of the CRTC and the DMT list, independent of whether you have EDID at all.
It's not especially hard to add, I suppose. You'd want to mark modes so added so the CRTC setup knows to do the appropriate panel magic.
- ajax
At Fri, 13 Apr 2012 12:31:25 -0400, Adam Jackson wrote:
On 4/13/12 11:41 AM, Takashi Iwai wrote:
At Fri, 13 Apr 2012 11:30:01 -0400, Alex Deucher wrote:
One thing to be careful of is that some monitors (especially LCD panels) don't like modes that are not in their EDIDs. As such when you try and set them you often get a wonky display or more often a blank screen. We used to add a lot of inferred modes to the mode list in the xserver which resulted in a lot of blank screens when some odd mode was picked as the best match for a cloned display. The "fix" was to only add the inferred modes on analog monitors which were more likely to be able to support them.
Thanks, it's good to know!
Though, I still wonder whether adding inferred modes for 1366x768 or 1600x900 would cause any big problems. On such monitors, 1360x768 or 1440x900 (or 1680x1050) are usually seen in the supported list.
Of course, it's never 100% safe. But not so bad odds?
"Mostly working" is a fancy way of saying "broken".
True.
The semantics of the range descriptor are "I can support modes within these ranges generated by these timing formulas and/or listed in DMT". That's why we only walk that mode list when we find a range descriptor: if you _don't_ find a range descriptor then the monitor is explicitly telling you it doesn't support arbitrary modes over a range.
You can be more aggressive than that if you know your CRTC's scaler can compensate, in which case you'd run the display at the native mode and let the scaler translate. The EDID parser is currently not told that bit of context, and in a sense it really shouldn't; it would be a function of the CRTC and the DMT list, independent of whether you have EDID at all.
It's not especially hard to add, I suppose. You'd want to mark modes so added so the CRTC setup knows to do the appropriate panel magic.
Right, we need some way to verify that the mode is acceptable. It's what Rodirgo wants to figure out now...
Takashi
On 4/13/12 10:29 AM, Takashi Iwai wrote:
At Fri, 13 Apr 2012 10:14:46 -0400, Adam Jackson wrote:
Yeah, that's a bug. That's why I said it should be renamed drm_dmt_modes_for_range and run unconditionally if we find a range descriptor.
Yeah, I saw your patches. Should the further work base on them?
Would be nice.
Yesterday I've read a news reporting that 1366x768 is the most commonly used panel now, more than 1024x768. And, 1600x900 is in the second place of the modern laptop panels.
Windows and others do work with these resolutions on the same monitor. Why Linux driver can't? Everbody (but developers) thinks like that way.
I think you're trying to make me defend a position I wasn't taking...
If it's not the native panel size and it's not a commonly found size in the wild, why are we obligated to provide them for every user? Remember that userspace has the ability to hand in modes from above.
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
I'm not disagreeing. I think common sizes should be available, and we have code already that's intended to do that.
My issue with the list in the patch is it contains some nonsense. If some of those more weird-looking sizes _do_ exist in the wild they should be already present in EDID as the native size. For panels where they're not native I have difficulty imagining anyone wanting to set that mode intentionally. And for someone who really does want it they have the ability to pass in arbitrary crap from userspace anyway.
1600x900 is reasonable to add to an "extras" list, because it is actually common despite not being in DMT. I'm even willing to take Windows as the example for what modes should be in the extras list. But I'm not willing to take "I wish this was in a preset list" as the sole justification.
- ajax
At Fri, 13 Apr 2012 10:55:16 -0400, Adam Jackson wrote:
On 4/13/12 10:29 AM, Takashi Iwai wrote:
At Fri, 13 Apr 2012 10:14:46 -0400, Adam Jackson wrote:
Yeah, that's a bug. That's why I said it should be renamed drm_dmt_modes_for_range and run unconditionally if we find a range descriptor.
Yeah, I saw your patches. Should the further work base on them?
Would be nice.
Yesterday I've read a news reporting that 1366x768 is the most commonly used panel now, more than 1024x768. And, 1600x900 is in the second place of the modern laptop panels.
Windows and others do work with these resolutions on the same monitor. Why Linux driver can't? Everbody (but developers) thinks like that way.
I think you're trying to make me defend a position I wasn't taking...
Heh, don't take it offensive.
If it's not the native panel size and it's not a commonly found size in the wild, why are we obligated to provide them for every user? Remember that userspace has the ability to hand in modes from above.
I don't think we need to support all wild modes, too. But the _very_ common modes like 1366x768 and 1600x900 should be really supported as default.
I'm not disagreeing. I think common sizes should be available, and we have code already that's intended to do that.
OK.
My issue with the list in the patch is it contains some nonsense. If some of those more weird-looking sizes _do_ exist in the wild they should be already present in EDID as the native size. For panels where they're not native I have difficulty imagining anyone wanting to set that mode intentionally. And for someone who really does want it they have the ability to pass in arbitrary crap from userspace anyway.
1600x900 is reasonable to add to an "extras" list, because it is actually common despite not being in DMT. I'm even willing to take Windows as the example for what modes should be in the extras list. But I'm not willing to take "I wish this was in a preset list" as the sole justification.
I agree with your point, too. When I worked on supporting these modes in X server side, I didn't pick up all such modes but only really de facto standard ones. It should suffice for most demands, indeed.
Also, we don't have to add always 1600x900 or 1366x768. Such a mode is necessary basically only when the laptop panel resolution isn't found in the mode list. We may add it selectively, too.
thanks,
Takashi
I agree with your point, too. When I worked on supporting these modes in X server side, I didn't pick up all such modes but only really de facto standard ones. It should suffice for most demands, indeed.
Also, we don't have to add always 1600x900 or 1366x768. Such a mode is necessary basically only when the laptop panel resolution isn't found in the mode list. We may add it selectively, too.
I'm still intrigued about what hardware exists with a panel with a native mode it doesn't describe.
How are we to know what the panel preferred mode is in this case?
Or is this for larger panels wanting to use smaller modes?
Still confused.
Dave.
On 4/13/12 11:25 AM, David Airlie wrote:
I'm still intrigued about what hardware exists with a panel with a native mode it doesn't describe.
How are we to know what the panel preferred mode is in this case?
Or is this for larger panels wanting to use smaller modes?
AFAICT, yes, this last one. For asymmetric cloning, a misfeature exceeded in stupidity only by panning. Because you have a panel with a bizarre size and then an external that's sane, and you try to clone, and even though the bizarre size would fit it's not available on both mode lists.
And at _that_ point, this isn't an EDID parser issue at all, it's driver policy.
- ajax
So, the modes that I added in that list was half got from windows on my monitor and half requested by HP.
So let forget about other modes and focus on the one required by HP. HP has requested the same list for everybody 3 years ago and it was added by Windows and AMD at that time but our open drivers never supported.
The list is:
640x480 800x600 1024x576 SD, 16:9 1024x768 1152x864 1280x720 720p 16:9 aspect ratio 1280x800 1280x1024 1366x768 HD, 16:9 1440x900 1600x900 HD+, 16:9 1600x1200 1680x945 HD+, 18.4" panel, 16:9 1680x1050 1920x1080 HDTV, 16:9 1920x1200 1920x1440 2048x1152 16:9 aspect ratio 2048x1536 2560x1600
So I'm in favor of doing something to catch up closed drivers in terms of giving our customers the same kind of resolutions. I'm not saying that we are worst because we don't add something that is not a standard as others do, but I do think we could provide the customers and final users all the options that he might be expecting, once everybody else on the market is also doing.
Ok, I didn't want to add it blindly as well, so I got the edid and verified that we can add any mode checking the range when the range flag is set using gtf or cvt depending on monitor and for cvt we weren't adding any mode at all. ok ok... this bug is more easy to fix.. renaming it for dmt and applying it for cvt solves this bug.. However it does'' t solve the HP request for all those modes in the list... So, do you have an idea about how to add those modes?
Takashi's patch on X probably fixes the critical part for HP, but they still wants to see those modes added by default. So.. any other ideas?
Thanks Rodrigo.
On Fri, Apr 13, 2012 at 2:16 PM, Adam Jackson ajax@redhat.com wrote:
On 4/13/12 11:25 AM, David Airlie wrote:
I'm still intrigued about what hardware exists with a panel with a native mode it doesn't describe.
How are we to know what the panel preferred mode is in this case?
Or is this for larger panels wanting to use smaller modes?
AFAICT, yes, this last one. For asymmetric cloning, a misfeature exceeded in stupidity only by panning. Because you have a panel with a bizarre size and then an external that's sane, and you try to clone, and even though the bizarre size would fit it's not available on both mode lists.
And at _that_ point, this isn't an EDID parser issue at all, it's driver policy.
- ajax
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Adam Jackson -
Alex Deucher -
Dave Airlie -
David Airlie -
Rodrigo Vivi -
Takashi Iwai