From: "Y.C. Chen" yc_chen@aspeedtech.com
--- drivers/gpu/drm/ast/ast_mode.c | 27 +++++++++++++++++--------- drivers/gpu/drm/ast/ast_tables.h | 42 ++++++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 5389350..e7fed29 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -140,15 +140,24 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo return false; }
- refresh_rate = drm_mode_vrefresh(mode); - while (vbios_mode->enh_table->refresh_rate < refresh_rate) { - vbios_mode->enh_table++; - if ((vbios_mode->enh_table->refresh_rate > refresh_rate) || - (vbios_mode->enh_table->refresh_rate == 0xff)) { - vbios_mode->enh_table--; - break; - } - } + if (vbios_mode->enh_table->flags & WideScreenMode) + { + /* parsing for wide screen reduced blank mode */ + if ((mode->flags & DRM_MODE_FLAG_NVSYNC) && (mode->flags & DRM_MODE_FLAG_PHSYNC)) /* CVT RB */ + vbios_mode->enh_table++; + } + else + { + refresh_rate = drm_mode_vrefresh(mode); + while (vbios_mode->enh_table->refresh_rate < refresh_rate) { + vbios_mode->enh_table++; + if ((vbios_mode->enh_table->refresh_rate > refresh_rate) || + (vbios_mode->enh_table->refresh_rate == 0xff)) { + vbios_mode->enh_table--; + break; + } + } + }
hborder = (vbios_mode->enh_table->flags & HBorder) ? 8 : 0; vborder = (vbios_mode->enh_table->flags & VBorder) ? 8 : 0; diff --git a/drivers/gpu/drm/ast/ast_tables.h b/drivers/gpu/drm/ast/ast_tables.h index 4c761dc..56b6abe 100644 --- a/drivers/gpu/drm/ast/ast_tables.h +++ b/drivers/gpu/drm/ast/ast_tables.h @@ -72,6 +72,7 @@ #define VCLK119 0x17 #define VCLK85_5 0x18 #define VCLK97_75 0x19 +#define VCLK118_25 0x1A
static struct ast_vbios_dclk_info dclk_table[] = { {0x2C, 0xE7, 0x03}, /* 00: VCLK25_175 */ @@ -99,6 +100,8 @@ static struct ast_vbios_dclk_info dclk_table[] = { {0x25, 0x65, 0x80}, /* 16: VCLK88.75 */ {0x77, 0x58, 0x80}, /* 17: VCLK119 */ {0x32, 0x67, 0x80}, /* 18: VCLK85_5 */ + {0x6a, 0x6d, 0x80}, /* 19: VCLK97_75 */ + {0x3b, 0x2c, 0x81}, /* 1A: VCLK118_25 */ };
static struct ast_vbios_stdtable vbios_stdtable[] = { @@ -243,6 +246,8 @@ static struct ast_vbios_enhtable res_1360x768[] = { };
static struct ast_vbios_enhtable res_1600x900[] = { + {2112, 1600, 88,168, 934, 900, 3, 5, VCLK118_25, /* 60Hz CVT */ + (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x3A }, {1760, 1600, 48, 32, 926, 900, 3, 5, VCLK97_75, /* 60Hz CVT RB */ (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x3A }, {1760, 1600, 48, 32, 926, 900, 3, 5, VCLK97_75, /* end */ @@ -259,31 +264,30 @@ static struct ast_vbios_enhtable res_1920x1080[] = {
/* 16:10 */ static struct ast_vbios_enhtable res_1280x800[] = { - {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz RB */ - (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 35 }, - {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */ - (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 }, - {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */ - (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x35 }, - + {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz CVT */ + (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 }, + {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */ + (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 35 }, + {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */ + (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 35 }, };
static struct ast_vbios_enhtable res_1440x900[] = { - {1600, 1440, 48, 32, 926, 900, 3, 6, VCLK88_75, /* 60Hz RB */ - (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x36 }, - {1904, 1440, 80,152, 934, 900, 3, 6, VCLK106_5, /* 60Hz */ - (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x36 }, - {1904, 1440, 80,152, 934, 900, 3, 6, VCLK106_5, /* 60Hz */ - (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x36 }, + {1904, 1440, 80,152, 934, 900, 3, 6, VCLK106_5, /* 60Hz CVT */ + (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x36 }, + {1600, 1440, 48, 32, 926, 900, 3, 6, VCLK88_75, /* 60Hz CVT RB */ + (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x36 }, + {1600, 1440, 48, 32, 926, 900, 3, 6, VCLK88_75, /* 60Hz CVT RB */ + (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 0x36 }, };
static struct ast_vbios_enhtable res_1680x1050[] = { - {1840, 1680, 48, 32, 1080, 1050, 3, 6, VCLK119, /* 60Hz RB */ - (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x37 }, - {2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25, /* 60Hz */ - (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x37 }, - {2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25, /* 60Hz */ - (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x37 }, + {2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25, /* 60Hz CVT */ + (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x37 }, + {1840, 1680, 48, 32, 1080, 1050, 3, 6, VCLK119, /* 60Hz CVT RB */ + (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x37 }, + {1840, 1680, 48, 32, 1080, 1050, 3, 6, VCLK119, /* 60Hz CVT RB */ + (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 0x37 }, };
static struct ast_vbios_enhtable res_1920x1200[] = {
On Tue, Aug 19, 2014 at 05:10:51PM +0800, Y.C. Chen wrote:
From: "Y.C. Chen" yc_chen@aspeedtech.com
drivers/gpu/drm/ast/ast_mode.c | 27 +++++++++++++++++--------- drivers/gpu/drm/ast/ast_tables.h | 42 ++++++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 5389350..e7fed29 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -140,15 +140,24 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo return false; }
- refresh_rate = drm_mode_vrefresh(mode);
- while (vbios_mode->enh_table->refresh_rate < refresh_rate) {
vbios_mode->enh_table++;if ((vbios_mode->enh_table->refresh_rate > refresh_rate) ||(vbios_mode->enh_table->refresh_rate == 0xff)) {vbios_mode->enh_table--;break;}- }
- if (vbios_mode->enh_table->flags & WideScreenMode)
- {
/* parsing for wide screen reduced blank mode */if ((mode->flags & DRM_MODE_FLAG_NVSYNC) && (mode->flags & DRM_MODE_FLAG_PHSYNC)) /* CVT RB */vbios_mode->enh_table++;
I'm not really happy with assuming fixed list positions. Wouldn't it be better to walk the list until you reach an entry with: (vbios_mode->enh_table->flags & SyncNP) && (vbios_mode->enh_table->refresh_rate != 0xff)? This way it would be independent of the position of the modes in the list. Assuming fixed list postition seems to somewhat defy the sophisticated flag concept used here.
- }
- else
- {
refresh_rate = drm_mode_vrefresh(mode);while (vbios_mode->enh_table->refresh_rate < refresh_rate) {vbios_mode->enh_table++;if ((vbios_mode->enh_table->refresh_rate > refresh_rate) ||(vbios_mode->enh_table->refresh_rate == 0xff)) {vbios_mode->enh_table--;break;}}- }
[..]
static struct ast_vbios_enhtable res_1600x900[] = {
- {2112, 1600, 88,168, 934, 900, 3, 5, VCLK118_25, /* 60Hz CVT */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x3A },@@ -259,31 +264,30 @@ static struct ast_vbios_enhtable res_1920x1080[] = {
/* 16:10 */ static struct ast_vbios_enhtable res_1280x800[] = {
- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 35 },- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x35 },
- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz CVT */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 35 },- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 35 },};
The last entry marks the end of the list by setting refresh rate to 0xff. Would be nice if all other entries in this list element were obviously invalid as well to enhance readability and clearly mark that there is no magic involved here. But you didn't introduce this, it was done this way before already.
Cheers, Egbert.
Hi Egbert, Thanks for your comment!! I will try to modify it and test in-house first.
Regards,
Y.C. Chen
-----Original Message----- From: Egbert Eich [mailto:eich@freedesktop.org] Sent: Thursday, August 21, 2014 5:06 PM To: YC Chen Cc: dri-devel@lists.freedesktop.org Subject: Re: [PATCH] Fixed 1600x900 Display Issue
On Tue, Aug 19, 2014 at 05:10:51PM +0800, Y.C. Chen wrote:
From: "Y.C. Chen" yc_chen@aspeedtech.com
drivers/gpu/drm/ast/ast_mode.c | 27 +++++++++++++++++--------- drivers/gpu/drm/ast/ast_tables.h | 42 ++++++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 5389350..e7fed29 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -140,15 +140,24 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct drm_display_mo return false; }
- refresh_rate = drm_mode_vrefresh(mode);
- while (vbios_mode->enh_table->refresh_rate < refresh_rate) {
vbios_mode->enh_table++;if ((vbios_mode->enh_table->refresh_rate > refresh_rate) ||(vbios_mode->enh_table->refresh_rate == 0xff)) {vbios_mode->enh_table--;break;}- }
- if (vbios_mode->enh_table->flags & WideScreenMode)
- {
/* parsing for wide screen reduced blank mode */if ((mode->flags & DRM_MODE_FLAG_NVSYNC) && (mode->flags & DRM_MODE_FLAG_PHSYNC)) /* CVT RB */vbios_mode->enh_table++;
I'm not really happy with assuming fixed list positions. Wouldn't it be better to walk the list until you reach an entry with: (vbios_mode->enh_table->flags & SyncNP) && (vbios_mode->enh_table->refresh_rate != 0xff)? This way it would be independent of the position of the modes in the list. Assuming fixed list postition seems to somewhat defy the sophisticated flag concept used here.
- }
- else
- {
refresh_rate = drm_mode_vrefresh(mode);while (vbios_mode->enh_table->refresh_rate < refresh_rate) {vbios_mode->enh_table++;if ((vbios_mode->enh_table->refresh_rate > refresh_rate) ||(vbios_mode->enh_table->refresh_rate == 0xff)) {vbios_mode->enh_table--;break;}}- }
[..]
static struct ast_vbios_enhtable res_1600x900[] = {
- {2112, 1600, 88,168, 934, 900, 3, 5, VCLK118_25, /* 60Hz CVT */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |- NewModeInfo), 60, 1, 0x3A }, {1760, 1600, 48, 32, 926, 900, 3, 5, VCLK97_75, /* 60Hz CVT RB */ (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x3A }, {1760, 1600, 48, 32, 926, 900, 3, 5, VCLK97_75, /* end */
@@ -259,31 +264,30 @@ static struct ast_vbios_enhtable res_1920x1080[] = {
/* 16:10 */ static struct ast_vbios_enhtable res_1280x800[] = {
- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 35 },- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x35 },
- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz CVT */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 35 },- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |- NewModeInfo), 0xFF, 2, 35 },
};
The last entry marks the end of the list by setting refresh rate to 0xff. Would be nice if all other entries in this list element were obviously invalid as well to enhance readability and clearly mark that there is no magic involved here. But you didn't introduce this, it was done this way before already.
Cheers, Egbert.
Just noticed two other things:
On Tue, Aug 19, 2014 at 05:10:51PM +0800, Y.C. Chen wrote:
--- a/drivers/gpu/drm/ast/ast_tables.h +++ b/drivers/gpu/drm/ast/ast_tables.h @@ -72,6 +72,7 @@
static struct ast_vbios_dclk_info dclk_table[] = { {0x2C, 0xE7, 0x03}, /* 00: VCLK25_175 */ @@ -99,6 +100,8 @@ static struct ast_vbios_dclk_info dclk_table[] = { {0x25, 0x65, 0x80}, /* 16: VCLK88.75 */ {0x77, 0x58, 0x80}, /* 17: VCLK119 */ {0x32, 0x67, 0x80}, /* 18: VCLK85_5 */
- {0x6a, 0x6d, 0x80}, /* 19: VCLK97_75 */
This addition fixes another bug: without this entry the unpatched code may run off the end of the list. Maybe you can provide this addition (just this one line) in a separate patch, explaining in the changelog what it fixes.
- {0x3b, 0x2c, 0x81}, /* 1A: VCLK118_25 */
};
/* 16:10 */ static struct ast_vbios_enhtable res_1280x800[] = {
- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 35 },- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x35 },
- {1680, 1280, 72,128, 831, 800, 3, 6, VCLK83_5, /* 60Hz CVT */
(SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 35 },
_______^^ This was like this in the code before - but shouldn't this read: 0x35?
- {1440, 1280, 48, 32, 823, 800, 3, 6, VCLK71, /* 60Hz CVT RB */
(SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 35 },
_______^^ Same here.
dri-devel@lists.freedesktop.org
-
Egbert Eich -
Y.C. Chen -
YC Chen