* [PATCH] Fixed 1600x900 Display Issue
@ 2014-08-19 9:10 Y.C. Chen
2014-08-21 9:06 ` Egbert Eich
2014-08-21 10:12 ` Egbert Eich
0 siblings, 2 replies; 4+ messages in thread
From: Y.C. Chen @ 2014-08-19 9:10 UTC (permalink / raw)
To: dri-devel; +Cc: Y.C. Chen
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[] = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Fixed 1600x900 Display Issue
2014-08-19 9:10 [PATCH] Fixed 1600x900 Display Issue Y.C. Chen
@ 2014-08-21 9:06 ` Egbert Eich
2014-08-21 9:27 ` YC Chen
2014-08-21 10:12 ` Egbert Eich
1 sibling, 1 reply; 4+ messages in thread
From: Egbert Eich @ 2014-08-21 9:06 UTC (permalink / raw)
To: Y.C. Chen; +Cc: dri-devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] Fixed 1600x900 Display Issue
2014-08-21 9:06 ` Egbert Eich
@ 2014-08-21 9:27 ` YC Chen
0 siblings, 0 replies; 4+ messages in thread
From: YC Chen @ 2014-08-21 9:27 UTC (permalink / raw)
To: Egbert Eich; +Cc: YC Chen, dri-devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fixed 1600x900 Display Issue
2014-08-19 9:10 [PATCH] Fixed 1600x900 Display Issue Y.C. Chen
2014-08-21 9:06 ` Egbert Eich
@ 2014-08-21 10:12 ` Egbert Eich
1 sibling, 0 replies; 4+ messages in thread
From: Egbert Eich @ 2014-08-21 10:12 UTC (permalink / raw)
To: Y.C. Chen; +Cc: dri-devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-08-21 10:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-19 9:10 [PATCH] Fixed 1600x900 Display Issue Y.C. Chen
2014-08-21 9:06 ` Egbert Eich
2014-08-21 9:27 ` YC Chen
2014-08-21 10:12 ` Egbert Eich
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.