* [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
@ 2015-12-01 15:16 Jani Nikula
2015-12-01 15:20 ` Alex Deucher
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jani Nikula @ 2015-12-01 15:16 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula
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;
}
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-01 15:16 [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC Jani Nikula
@ 2015-12-01 15:20 ` Alex Deucher
2015-12-01 15:24 ` Ville Syrjälä
2016-01-08 11:21 ` [PATCH v2] " Jani Nikula
2 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2015-12-01 15:20 UTC (permalink / raw)
To: Jani Nikula; +Cc: Maling list - DRI developers
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 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;
> }
>
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-01 15:16 [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC Jani Nikula
2015-12-01 15:20 ` Alex Deucher
@ 2015-12-01 15:24 ` Ville Syrjälä
2015-12-02 11:46 ` Jani Nikula
2016-01-08 11:21 ` [PATCH v2] " Jani Nikula
2 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2015-12-01 15:24 UTC (permalink / raw)
To: Jani Nikula; +Cc: 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 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
<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?
Otherwise I like it.
> + 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-01 15:24 ` Ville Syrjälä
@ 2015-12-02 11:46 ` Jani Nikula
2015-12-02 12:24 ` Ville Syrjälä
0 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2015-12-02 11:46 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: dri-devel
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 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
> <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);
>> } 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
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-02 11:46 ` Jani Nikula
@ 2015-12-02 12:24 ` Ville Syrjälä
2015-12-02 12:57 ` Jani Nikula
0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2015-12-02 12:24 UTC (permalink / raw)
To: Jani Nikula; +Cc: dri-devel
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 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
> > <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);
> >> } 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
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-02 12:24 ` Ville Syrjälä
@ 2015-12-02 12:57 ` Jani Nikula
2015-12-03 9:53 ` Ville Syrjälä
0 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2015-12-02 12:57 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: dri-devel
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 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
>> > <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);
>> >> } 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
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-02 12:57 ` Jani Nikula
@ 2015-12-03 9:53 ` Ville Syrjälä
0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2015-12-03 9:53 UTC (permalink / raw)
To: Jani Nikula; +Cc: dri-devel
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 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
> >> > <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);
> >> >> } 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
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] drm/edid: index CEA/HDMI mode tables using the VIC
2015-12-01 15:16 [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC Jani Nikula
2015-12-01 15:20 ` Alex Deucher
2015-12-01 15:24 ` Ville Syrjälä
@ 2016-01-08 11:21 ` Jani Nikula
2016-01-08 11:30 ` Ville Syrjälä
2 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2016-01-08 11:21 UTC (permalink / raw)
To: dri-devel
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;
}
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/edid: index CEA/HDMI mode tables using the VIC
2016-01-08 11:21 ` [PATCH v2] " Jani Nikula
@ 2016-01-08 11:30 ` Ville Syrjälä
2016-01-08 14:31 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2016-01-08 11:30 UTC (permalink / raw)
To: Jani Nikula; +Cc: dri-devel
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;
> }
> 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;
> }
>
> --
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/edid: index CEA/HDMI mode tables using the VIC
2016-01-08 11:30 ` Ville Syrjälä
@ 2016-01-08 14:31 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-01-08 14:31 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, dri-devel
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;
> > }
> > 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;
> > }
> >
> > --
> > 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-01-08 14:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-01 15:16 [RFC PATCH] drm/edid: index CEA/HDMI mode tables using the VIC Jani Nikula
2015-12-01 15:20 ` Alex Deucher
2015-12-01 15:24 ` Ville Syrjälä
2015-12-02 11:46 ` Jani Nikula
2015-12-02 12:24 ` Ville Syrjälä
2015-12-02 12:57 ` Jani Nikula
2015-12-03 9:53 ` Ville Syrjälä
2016-01-08 11:21 ` [PATCH v2] " Jani Nikula
2016-01-08 11:30 ` Ville Syrjälä
2016-01-08 14:31 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.