Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
---
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock difference
in drm-intel/topic/drm-misc --- drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = { /* * Probably taken from CEA-861 spec. * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c. + * + * Index using the VIC. */ static const struct drm_display_mode edid_cea_modes[] = { + /* 0 - dummy, VICs start at 1 */ + { 0 }, /* 1 - 640x480@60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0, @@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[] = { };
/* - * HDMI 1.4 4k modes. + * HDMI 1.4 4k modes. Index using the VIC. */ static const struct drm_display_mode edid_4k_modes[] = { + /* 0 - dummy, VICs start at 1 */ + { 0 }, /* 1 - 3840x2160@30Hz */ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, 4104, 4400, 0, @@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */ @@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue;
if (drm_mode_equal_no_clocks(to_match, cea_mode)) - return mode + 1; + return vic; }
return 0; @@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */ @@ -2597,7 +2603,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode)) - return mode + 1; + return vic; } return 0; } @@ -2612,10 +2618,7 @@ EXPORT_SYMBOL(drm_match_cea_mode); */ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) { - /* return picture aspect ratio for video_code - 1 to access the - * right array element - */ - return edid_cea_modes[video_code-1].picture_aspect_ratio; + return edid_cea_modes[video_code].picture_aspect_ratio; } EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
@@ -2639,13 +2642,13 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */ @@ -2657,7 +2660,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ continue;
if (drm_mode_equal_no_clocks(to_match, hdmi_mode)) - return mode + 1; + return vic; }
return 0; @@ -2673,13 +2676,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */ @@ -2689,7 +2692,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) - return mode + 1; + return vic; } return 0; } @@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode; - u8 mode_idx = drm_match_cea_mode(mode) - 1; + u8 vic = drm_match_cea_mode(mode); unsigned int clock1, clock2;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { - cea_mode = &edid_cea_modes[mode_idx]; + if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { + cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode); } else { - mode_idx = drm_match_hdmi_mode(mode) - 1; - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { - cea_mode = &edid_4k_modes[mode_idx]; + vic = drm_match_hdmi_mode(mode); + if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { + cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); } } @@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode; - u8 cea_mode; + u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */ - cea_mode = (video_db[video_index] & 127) - 1; - if (cea_mode >= ARRAY_SIZE(edid_cea_modes)) + vic = (video_db[video_index] & 127); + if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */ - if (vic >= ARRAY_SIZE(edid_4k_modes)) { + if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; } @@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock; - u8 mode_idx; + u8 vic; const char *type;
/* * allow 5kHz clock difference either way to account for * the 10kHz clock resolution limit of detailed timings. */ - mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1; - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { + vic = drm_match_cea_mode_clock_tolerance(mode, 5); + if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA"; - cea_mode = &edid_cea_modes[mode_idx]; + cea_mode = &edid_cea_modes[vic]; clock1 = cea_mode->clock; clock2 = cea_mode_alternate_clock(cea_mode); } else { - mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1; - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { + vic = drm_match_hdmi_mode_clock_tolerance(mode, 5); + if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI"; - cea_mode = &edid_4k_modes[mode_idx]; + cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode); } else { @@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n", - type, mode_idx + 1, mode->clock, clock); + type, vic, mode->clock, clock); mode->clock = clock; }
On Tue, Dec 1, 2015 at 10:16 AM, Jani Nikula jani.nikula@intel.com wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock differencein drm-intel/topic/drm-misc
drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = { /*
- Probably taken from CEA-861 spec.
- This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
*/
- Index using the VIC.
static const struct drm_display_mode edid_cea_modes[] = {
/* 0 - dummy, VICs start at 1 */{ 0 }, /* 1 - 640x480@60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0,@@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[] = { };
/*
- HDMI 1.4 4k modes.
*/
- HDMI 1.4 4k modes. Index using the VIC.
static const struct drm_display_mode edid_4k_modes[] = {
/* 0 - dummy, VICs start at 1 */{ 0 }, /* 1 - 3840x2160@30Hz */ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, 4104, 4400, 0,@@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) {
u8 mode;
u8 vic; if (!to_match->clock) return 0;
for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2; /* Check both 60Hz and 59.94Hz */@@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue;
if (drm_mode_equal_no_clocks(to_match, cea_mode))
return mode + 1;
return vic; } return 0;@@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) {
u8 mode;
u8 vic; if (!to_match->clock) return 0;
for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2; /* Check both 60Hz and 59.94Hz */@@ -2597,7 +2603,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode))
return mode + 1;
return vic; } return 0;} @@ -2612,10 +2618,7 @@ EXPORT_SYMBOL(drm_match_cea_mode); */ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) {
/* return picture aspect ratio for video_code - 1 to access the* right array element*/return edid_cea_modes[video_code-1].picture_aspect_ratio;
return edid_cea_modes[video_code].picture_aspect_ratio;} EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
@@ -2639,13 +2642,13 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) {
u8 mode;
u8 vic; if (!to_match->clock) return 0;
for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2; /* Make sure to also match alternate clocks */@@ -2657,7 +2660,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ continue;
if (drm_mode_equal_no_clocks(to_match, hdmi_mode))
return mode + 1;
return vic; } return 0;@@ -2673,13 +2676,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) {
u8 mode;
u8 vic; if (!to_match->clock) return 0;
for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2; /* Make sure to also match alternate clocks */@@ -2689,7 +2692,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
return mode + 1;
return vic; } return 0;} @@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode;
u8 mode_idx = drm_match_cea_mode(mode) - 1;
u8 vic = drm_match_cea_mode(mode); unsigned int clock1, clock2;
if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {cea_mode = &edid_cea_modes[mode_idx];
if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode); } else {
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); } }@@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
u8 cea_mode;
u8 vic; if (video_db == NULL || video_index >= video_len) return NULL; /* CEA modes are numbered 1..127 */
cea_mode = (video_db[video_index] & 127) - 1;if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
vic = (video_db[video_index] & 127);if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
vic--; /* VICs start at 1 */if (vic >= ARRAY_SIZE(edid_4k_modes)) {
if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }@@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
u8 mode_idx;
u8 vic; const char *type; /* * allow 5kHz clock difference either way to account for * the 10kHz clock resolution limit of detailed timings. */
mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
vic = drm_match_cea_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic]; clock1 = cea_mode->clock; clock2 = cea_mode_alternate_clock(cea_mode); } else {
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode); } else {@@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock); mode->clock = clock;}
-- 2.1.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Dec 01, 2015 at 05:16:54PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock differencein drm-intel/topic/drm-misc
drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c
<snip>
@@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode;
u8 mode_idx = drm_match_cea_mode(mode) - 1;
u8 vic = drm_match_cea_mode(mode);
if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {cea_mode = &edid_cea_modes[mode_idx];
if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
Maybe add some sort of vic_is_valid() helpers to avoid duplicating these in so many places?
Otherwise I like it.
cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
On Tue, 01 Dec 2015, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Dec 01, 2015 at 05:16:54PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock differencein drm-intel/topic/drm-misc
drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c
<snip> > @@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) > list_for_each_entry(mode, &connector->probed_modes, head) { > const struct drm_display_mode *cea_mode = NULL; > struct drm_display_mode *newmode; > - u8 mode_idx = drm_match_cea_mode(mode) - 1; > + u8 vic = drm_match_cea_mode(mode); > unsigned int clock1, clock2; > > - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { > - cea_mode = &edid_cea_modes[mode_idx]; > + if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
Maybe add some sort of vic_is_valid() helpers to avoid duplicating these in so many places?
Any thoughts on how that should handle CEA vs. HDMI VICs, comparing against ARRAY_SIZE(edid_cea_modes) and ARRAY_SIZE(edid_4k_modes)?
Add versions of drm_match_*_mode*() that return struct drm_display_mode * or NULL on errors?
BR, Jani.
Otherwise I like it.
cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
On Wed, Dec 02, 2015 at 01:46:40PM +0200, Jani Nikula wrote:
On Tue, 01 Dec 2015, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Dec 01, 2015 at 05:16:54PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock differencein drm-intel/topic/drm-misc
drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c
<snip> > @@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) > list_for_each_entry(mode, &connector->probed_modes, head) { > const struct drm_display_mode *cea_mode = NULL; > struct drm_display_mode *newmode; > - u8 mode_idx = drm_match_cea_mode(mode) - 1; > + u8 vic = drm_match_cea_mode(mode); > unsigned int clock1, clock2; > > - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { > - cea_mode = &edid_cea_modes[mode_idx]; > + if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
Maybe add some sort of vic_is_valid() helpers to avoid duplicating these in so many places?
Any thoughts on how that should handle CEA vs. HDMI VICs, comparing against ARRAY_SIZE(edid_cea_modes) and ARRAY_SIZE(edid_4k_modes)?
I was thinking one function for each type, and yeah just
return vic != 0 && vic < ARRAY_SIZE(whatever_modes);
or something along those lines.
Add versions of drm_match_*_mode*() that return struct drm_display_mode
- or NULL on errors?
Some time ago I was readin the DP spec, and ended up sketching out some functions that might help deal with EDIDless DP branch devices. For that I cooked up a function to return a pointer to the appropriate CEA mode based on the vic (or NULL for bad vics). So I guess if you did something like that + take the already existing match function, you could wrap them in a neat package that returns the matching cea mode for whatever mode is passed in.
BR, Jani.
Otherwise I like it.
cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
-- Jani Nikula, Intel Open Source Technology Center
On Wed, 02 Dec 2015, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Wed, Dec 02, 2015 at 01:46:40PM +0200, Jani Nikula wrote:
On Tue, 01 Dec 2015, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Dec 01, 2015 at 05:16:54PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock differencein drm-intel/topic/drm-misc
drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c
<snip> > @@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) > list_for_each_entry(mode, &connector->probed_modes, head) { > const struct drm_display_mode *cea_mode = NULL; > struct drm_display_mode *newmode; > - u8 mode_idx = drm_match_cea_mode(mode) - 1; > + u8 vic = drm_match_cea_mode(mode); > unsigned int clock1, clock2; > > - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { > - cea_mode = &edid_cea_modes[mode_idx]; > + if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
Maybe add some sort of vic_is_valid() helpers to avoid duplicating these in so many places?
Any thoughts on how that should handle CEA vs. HDMI VICs, comparing against ARRAY_SIZE(edid_cea_modes) and ARRAY_SIZE(edid_4k_modes)?
I was thinking one function for each type, and yeah just
return vic != 0 && vic < ARRAY_SIZE(whatever_modes);
or something along those lines.
Add versions of drm_match_*_mode*() that return struct drm_display_mode
- or NULL on errors?
Some time ago I was readin the DP spec, and ended up sketching out some functions that might help deal with EDIDless DP branch devices. For that I cooked up a function to return a pointer to the appropriate CEA mode based on the vic (or NULL for bad vics). So I guess if you did something like that + take the already existing match function, you could wrap them in a neat package that returns the matching cea mode for whatever mode is passed in.
Mmmh, so how about just taking this patch as-is for now...?
BR, Jani.
BR, Jani.
Otherwise I like it.
cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
-- Jani Nikula, Intel Open Source Technology Center
On Wed, Dec 02, 2015 at 02:57:39PM +0200, Jani Nikula wrote:
On Wed, 02 Dec 2015, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Wed, Dec 02, 2015 at 01:46:40PM +0200, Jani Nikula wrote:
On Tue, 01 Dec 2015, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Dec 01, 2015 at 05:16:54PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we now explicitly check for 0 instead of just subtracting one from an unsigned type.
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com
This is on top of
commit 4c6bcf44549907cb50b67f98eb13717a4adc6b33 Author: Ville Syrjälä ville.syrjala@linux.intel.com Date: Mon Nov 16 21:05:12 2015 +0200
drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock differencein drm-intel/topic/drm-misc
drivers/gpu/drm/drm_edid.c | 84 ++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..aea09eae407e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c
<snip> > @@ -2713,16 +2716,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) > list_for_each_entry(mode, &connector->probed_modes, head) { > const struct drm_display_mode *cea_mode = NULL; > struct drm_display_mode *newmode; > - u8 mode_idx = drm_match_cea_mode(mode) - 1; > + u8 vic = drm_match_cea_mode(mode); > unsigned int clock1, clock2; > > - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { > - cea_mode = &edid_cea_modes[mode_idx]; > + if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) {
Maybe add some sort of vic_is_valid() helpers to avoid duplicating these in so many places?
Any thoughts on how that should handle CEA vs. HDMI VICs, comparing against ARRAY_SIZE(edid_cea_modes) and ARRAY_SIZE(edid_4k_modes)?
I was thinking one function for each type, and yeah just
return vic != 0 && vic < ARRAY_SIZE(whatever_modes);
or something along those lines.
Add versions of drm_match_*_mode*() that return struct drm_display_mode
- or NULL on errors?
Some time ago I was readin the DP spec, and ended up sketching out some functions that might help deal with EDIDless DP branch devices. For that I cooked up a function to return a pointer to the appropriate CEA mode based on the vic (or NULL for bad vics). So I guess if you did something like that + take the already existing match function, you could wrap them in a neat package that returns the matching cea mode for whatever mode is passed in.
Mmmh, so how about just taking this patch as-is for now...?
We can do that. But I just figured it wouldn't be too much extra work to yank yank the "is this a valid vic" logic into two small functions.
BR, Jani.
BR, Jani.
Otherwise I like it.
cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2776,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (vic == 0 || vic >= ARRAY_SIZE(edid_cea_modes)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2881,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (vic == 0 || vic >= ARRAY_SIZE(edid_4k_modes)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3172,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (vic > 0 && vic < ARRAY_SIZE(edid_cea_modes)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (vic > 0 && vic < ARRAY_SIZE(edid_4k_modes)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3207,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
-- Jani Nikula, Intel Open Source Technology Center
-- Jani Nikula, Intel Open Source Technology Center
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we can now explicitly check for 0 instead of just subtracting one from an unsigned type.
Also add drm_valid_cea_vic() and drm_valid_hdmi_vic() helpers for checking valid VICs.
v2: add drm_valid_cea_vic and drm_valid_hdmi_vic helpers (Ville) use { } instead of { 0 } for initializing the dummy modes
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Alex Deucher alexander.deucher@amd.com Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 94 ++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..04cb4877fabd 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = { /* * Probably taken from CEA-861 spec. * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c. + * + * Index using the VIC. */ static const struct drm_display_mode edid_cea_modes[] = { + /* 0 - dummy, VICs start at 1 */ + { }, /* 1 - 640x480@60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0, @@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[] = { };
/* - * HDMI 1.4 4k modes. + * HDMI 1.4 4k modes. Index using the VIC. */ static const struct drm_display_mode edid_4k_modes[] = { + /* 0 - dummy, VICs start at 1 */ + { }, /* 1 - 3840x2160@30Hz */ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, 4104, 4400, 0, @@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */ @@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue;
if (drm_mode_equal_no_clocks(to_match, cea_mode)) - return mode + 1; + return vic; }
return 0; @@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */ @@ -2597,12 +2603,17 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode)) - return mode + 1; + return vic; } return 0; } EXPORT_SYMBOL(drm_match_cea_mode);
+static bool drm_valid_cea_vic(u8 vic) +{ + return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes); +} + /** * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to * the input VIC from the CEA mode list @@ -2612,10 +2623,7 @@ EXPORT_SYMBOL(drm_match_cea_mode); */ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) { - /* return picture aspect ratio for video_code - 1 to access the - * right array element - */ - return edid_cea_modes[video_code-1].picture_aspect_ratio; + return edid_cea_modes[video_code].picture_aspect_ratio; } EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
@@ -2639,13 +2647,13 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */ @@ -2657,7 +2665,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ continue;
if (drm_mode_equal_no_clocks(to_match, hdmi_mode)) - return mode + 1; + return vic; }
return 0; @@ -2673,13 +2681,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) { - u8 mode; + u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */ @@ -2689,11 +2697,16 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) - return mode + 1; + return vic; } return 0; }
+static bool drm_valid_hdmi_vic(u8 vic) +{ + return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes); +} + static int add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -2713,16 +2726,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode; - u8 mode_idx = drm_match_cea_mode(mode) - 1; + u8 vic = drm_match_cea_mode(mode); unsigned int clock1, clock2;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { - cea_mode = &edid_cea_modes[mode_idx]; + if (drm_valid_cea_vic(vic)) { + cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode); } else { - mode_idx = drm_match_hdmi_mode(mode) - 1; - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { - cea_mode = &edid_4k_modes[mode_idx]; + vic = drm_match_hdmi_mode(mode); + if (drm_valid_hdmi_vic(vic)) { + cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); } } @@ -2773,17 +2786,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode; - u8 cea_mode; + u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */ - cea_mode = (video_db[video_index] & 127) - 1; - if (cea_mode >= ARRAY_SIZE(edid_cea_modes)) + vic = (video_db[video_index] & 127); + if (!drm_valid_cea_vic(vic)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2891,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */ - if (vic >= ARRAY_SIZE(edid_4k_modes)) { + if (!drm_valid_hdmi_vic(vic)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; } @@ -3170,24 +3182,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock; - u8 mode_idx; + u8 vic; const char *type;
/* * allow 5kHz clock difference either way to account for * the 10kHz clock resolution limit of detailed timings. */ - mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1; - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { + vic = drm_match_cea_mode_clock_tolerance(mode, 5); + if (drm_valid_cea_vic(vic)) { type = "CEA"; - cea_mode = &edid_cea_modes[mode_idx]; + cea_mode = &edid_cea_modes[vic]; clock1 = cea_mode->clock; clock2 = cea_mode_alternate_clock(cea_mode); } else { - mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1; - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { + vic = drm_match_hdmi_mode_clock_tolerance(mode, 5); + if (drm_valid_hdmi_vic(vic)) { type = "HDMI"; - cea_mode = &edid_4k_modes[mode_idx]; + cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode); } else { @@ -3205,7 +3217,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n", - type, mode_idx + 1, mode->clock, clock); + type, vic, mode->clock, clock); mode->clock = clock; }
On Fri, Jan 08, 2016 at 01:21:51PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we can now explicitly check for 0 instead of just subtracting one from an unsigned type.
Also add drm_valid_cea_vic() and drm_valid_hdmi_vic() helpers for checking valid VICs.
v2: add drm_valid_cea_vic and drm_valid_hdmi_vic helpers (Ville) use { } instead of { 0 } for initializing the dummy modes
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Alex Deucher alexander.deucher@amd.com Signed-off-by: Jani Nikula jani.nikula@intel.com
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 94 ++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..04cb4877fabd 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = { /*
- Probably taken from CEA-861 spec.
- This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
*/
- Index using the VIC.
static const struct drm_display_mode edid_cea_modes[] = {
- /* 0 - dummy, VICs start at 1 */
- { }, /* 1 - 640x480@60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0,
@@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[] = { };
/*
- HDMI 1.4 4k modes.
*/
- HDMI 1.4 4k modes. Index using the VIC.
static const struct drm_display_mode edid_4k_modes[] = {
- /* 0 - dummy, VICs start at 1 */
- { }, /* 1 - 3840x2160@30Hz */ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, 4104, 4400, 0,
@@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */
@@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue;
if (drm_mode_equal_no_clocks(to_match, cea_mode))
return mode + 1;
return vic;}
return 0;
@@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */
@@ -2597,12 +2603,17 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode))
return mode + 1;
return vic;} EXPORT_SYMBOL(drm_match_cea_mode);
+static bool drm_valid_cea_vic(u8 vic) +{
- return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
+}
/**
- drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
- the input VIC from the CEA mode list
@@ -2612,10 +2623,7 @@ EXPORT_SYMBOL(drm_match_cea_mode); */ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) {
- /* return picture aspect ratio for video_code - 1 to access the
* right array element- */
- return edid_cea_modes[video_code-1].picture_aspect_ratio;
- return edid_cea_modes[video_code].picture_aspect_ratio;
} EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
@@ -2639,13 +2647,13 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */
@@ -2657,7 +2665,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ continue;
if (drm_mode_equal_no_clocks(to_match, hdmi_mode))
return mode + 1;
return vic;}
return 0;
@@ -2673,13 +2681,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */
@@ -2689,11 +2697,16 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
return mode + 1;
return vic;}
+static bool drm_valid_hdmi_vic(u8 vic) +{
- return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);
+}
static int add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -2713,16 +2726,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode;
u8 mode_idx = drm_match_cea_mode(mode) - 1;
u8 vic = drm_match_cea_mode(mode);
if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {cea_mode = &edid_cea_modes[mode_idx];
if (drm_valid_cea_vic(vic)) {cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (drm_valid_hdmi_vic(vic)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2786,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (!drm_valid_cea_vic(vic)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2891,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (!drm_valid_hdmi_vic(vic)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3182,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (drm_valid_cea_vic(vic)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (drm_valid_hdmi_vic(vic)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3217,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
On Fri, Jan 08, 2016 at 01:30:29PM +0200, Ville Syrjälä wrote:
On Fri, Jan 08, 2016 at 01:21:51PM +0200, Jani Nikula wrote:
Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we can now explicitly check for 0 instead of just subtracting one from an unsigned type.
Also add drm_valid_cea_vic() and drm_valid_hdmi_vic() helpers for checking valid VICs.
v2: add drm_valid_cea_vic and drm_valid_hdmi_vic helpers (Ville) use { } instead of { 0 } for initializing the dummy modes
Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Alex Deucher alexander.deucher@amd.com Signed-off-by: Jani Nikula jani.nikula@intel.com
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Applied to drm-misc, thanks. -Daniel
drivers/gpu/drm/drm_edid.c | 94 ++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f1246cb4..04cb4877fabd 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = { /*
- Probably taken from CEA-861 spec.
- This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
*/
- Index using the VIC.
static const struct drm_display_mode edid_cea_modes[] = {
- /* 0 - dummy, VICs start at 1 */
- { }, /* 1 - 640x480@60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0,
@@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[] = { };
/*
- HDMI 1.4 4k modes.
*/
- HDMI 1.4 4k modes. Index using the VIC.
static const struct drm_display_mode edid_4k_modes[] = {
- /* 0 - dummy, VICs start at 1 */
- { }, /* 1 - 3840x2160@30Hz */ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, 4104, 4400, 0,
@@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */
@@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue;
if (drm_mode_equal_no_clocks(to_match, cea_mode))
return mode + 1;
return vic;}
return 0;
@@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];unsigned int clock1, clock2;
/* Check both 60Hz and 59.94Hz */
@@ -2597,12 +2603,17 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode))
return mode + 1;
return vic;} EXPORT_SYMBOL(drm_match_cea_mode);
+static bool drm_valid_cea_vic(u8 vic) +{
- return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
+}
/**
- drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
- the input VIC from the CEA mode list
@@ -2612,10 +2623,7 @@ EXPORT_SYMBOL(drm_match_cea_mode); */ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) {
- /* return picture aspect ratio for video_code - 1 to access the
* right array element- */
- return edid_cea_modes[video_code-1].picture_aspect_ratio;
- return edid_cea_modes[video_code].picture_aspect_ratio;
} EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
@@ -2639,13 +2647,13 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */
@@ -2657,7 +2665,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ continue;
if (drm_mode_equal_no_clocks(to_match, hdmi_mode))
return mode + 1;
return vic;}
return 0;
@@ -2673,13 +2681,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) {
- u8 mode;
u8 vic;
if (!to_match->clock) return 0;
- for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];unsigned int clock1, clock2;
/* Make sure to also match alternate clocks */
@@ -2689,11 +2697,16 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
return mode + 1;
return vic;}
+static bool drm_valid_hdmi_vic(u8 vic) +{
- return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);
+}
static int add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -2713,16 +2726,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode;
u8 mode_idx = drm_match_cea_mode(mode) - 1;
u8 vic = drm_match_cea_mode(mode);
if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {cea_mode = &edid_cea_modes[mode_idx];
if (drm_valid_cea_vic(vic)) {cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode);
mode_idx = drm_match_hdmi_mode(mode) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {cea_mode = &edid_4k_modes[mode_idx];
vic = drm_match_hdmi_mode(mode);if (drm_valid_hdmi_vic(vic)) {cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); }@@ -2773,17 +2786,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- u8 cea_mode;
u8 vic;
if (video_db == NULL || video_index >= video_len) return NULL;
/* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
- vic = (video_db[video_index] & 127);
- if (!drm_valid_cea_vic(vic)) return NULL;
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL;
@@ -2878,8 +2891,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode;
- vic--; /* VICs start at 1 */
- if (vic >= ARRAY_SIZE(edid_4k_modes)) {
- if (!drm_valid_hdmi_vic(vic)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; }
@@ -3170,24 +3182,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock;
- u8 mode_idx;
u8 vic; const char *type;
/*
- allow 5kHz clock difference either way to account for
- the 10kHz clock resolution limit of detailed timings.
*/
- mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
- if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
- vic = drm_match_cea_mode_clock_tolerance(mode, 5);
- if (drm_valid_cea_vic(vic)) { type = "CEA";
cea_mode = &edid_cea_modes[mode_idx];
cea_mode = &edid_cea_modes[vic];
mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
vic = drm_match_hdmi_mode_clock_tolerance(mode, 5);if (drm_valid_hdmi_vic(vic)) { type = "HDMI";
cea_mode = &edid_4k_modes[mode_idx];
cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode);@@ -3205,7 +3217,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return;
DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n",
type, mode_idx + 1, mode->clock, clock);
type, vic, mode->clock, clock);}
-- 2.1.4
-- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Daniel Vetter -
Jani Nikula -
Ville Syrjälä