Bogus video resolution in Linux 3.5-rc4

Bogus video resolution in Linux 3.5-rc4

[Sven Joachim]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

After upgrading to Linux 3.5-rc4 from 3.4.4, I noticed that my monitor
switched to a resolution of 1280x960 rather than the native 1280x1024,
and nouveau has set up a framebuffer of 1680x945.  It goes without
saying that the result looks terrible.

Bisecting shows that the problem started with commit cb21aafe1:

--8<---------------cut here---------------start------------->8---
commit cb21aafe121b1c3ad4c77cc5c22320163f16ba42
Author: Adam Jackson <ajax@redhat.com>
Date:   Fri Apr 13 16:33:36 2012 -0400

    drm/edid: Do drm_dmt_modes_for_range() for all range descriptor types
    
    EDID 1.4 retcons the meaning of the "GTF feature" bit to mean "is
    continuous frequency", and moves the set of supported timing formulas
    into the range descriptor itself.  In any event, the range descriptor
    can act as a filter on the DMT list without regard to a specific timing
    formula.
    
    Signed-off-by: Adam Jackson <ajax@redhat.com>
    Tested-by: Takashi Iwai <tiwai@suse.de>
    Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
    Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index cb40611..9363349 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1042,12 +1042,13 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
 {
 	struct detailed_mode_closure *closure = c;
 	struct detailed_non_pixel *data = &timing->data.other_data;
-	int gtf = (closure->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF);
 
-	if (gtf && data->type == EDID_DETAIL_MONITOR_RANGE)
-		closure->modes += drm_dmt_modes_for_range(closure->connector,
-							  closure->edid,
-							  timing);
+	if (data->type != EDID_DETAIL_MONITOR_RANGE)
+		return;
+
+	closure->modes += drm_dmt_modes_for_range(closure->connector,
+						  closure->edid,
+						  timing);
 }
 
 static int
--8<---------------cut here---------------end--------------->8---

With a kernel from this commit nouveau sets up a resolution of
1400x1050; kernels built from commits cffd754 and fc48f16 complain about
an invalid EDID and use the 1024x768 fallback.

Attached is the /sys/class/drm/card0-DVI-I-1/edid file from a working
kernel; in 3.5-rc4 it is identical.

Cheers,
       Sven


[-- Attachment #2: edid --]
[-- Type: application/octet-stream, Size: 128 bytes --]

Re: Bogus video resolution in Linux 3.5-rc4

[Takashi Iwai]

At Mon, 25 Jun 2012 16:03:36 +0200,
Sven Joachim wrote:
> 
> After upgrading to Linux 3.5-rc4 from 3.4.4, I noticed that my monitor
> switched to a resolution of 1280x960 rather than the native 1280x1024,
> and nouveau has set up a framebuffer of 1680x945.  It goes without
> saying that the result looks terrible.
> 
> Bisecting shows that the problem started with commit cb21aafe1:
> 
> --8<---------------cut here---------------start------------->8---
> commit cb21aafe121b1c3ad4c77cc5c22320163f16ba42
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Fri Apr 13 16:33:36 2012 -0400
> 
>     drm/edid: Do drm_dmt_modes_for_range() for all range descriptor types
>     
>     EDID 1.4 retcons the meaning of the "GTF feature" bit to mean "is
>     continuous frequency", and moves the set of supported timing formulas
>     into the range descriptor itself.  In any event, the range descriptor
>     can act as a filter on the DMT list without regard to a specific timing
>     formula.
>     
>     Signed-off-by: Adam Jackson <ajax@redhat.com>
>     Tested-by: Takashi Iwai <tiwai@suse.de>
>     Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>     Signed-off-by: Dave Airlie <airlied@redhat.com>
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index cb40611..9363349 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1042,12 +1042,13 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
>  {
>  	struct detailed_mode_closure *closure = c;
>  	struct detailed_non_pixel *data = &timing->data.other_data;
> -	int gtf = (closure->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF);
>  
> -	if (gtf && data->type == EDID_DETAIL_MONITOR_RANGE)
> -		closure->modes += drm_dmt_modes_for_range(closure->connector,
> -							  closure->edid,
> -							  timing);
> +	if (data->type != EDID_DETAIL_MONITOR_RANGE)
> +		return;
> +
> +	closure->modes += drm_dmt_modes_for_range(closure->connector,
> +						  closure->edid,
> +						  timing);
>  }
>  
>  static int
> --8<---------------cut here---------------end--------------->8---
> 
> With a kernel from this commit nouveau sets up a resolution of
> 1400x1050; kernels built from commits cffd754 and fc48f16 complain about
> an invalid EDID and use the 1024x768 fallback.
> 
> Attached is the /sys/class/drm/card0-DVI-I-1/edid file from a working
> kernel; in 3.5-rc4 it is identical.

Looking at the EDID data, the problem is likely that your monitor
doesn't give the proper preferred mode.
What does xrandr output show?

And, does the patch below help?


Takashi

---
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5873e48..dab8580 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -116,6 +116,7 @@ static struct edid_quirk {
 
 	/* Proview AY765C */
 	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ "PTS", 793, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
 
 	/* Samsung SyncMaster 205BW.  Note: irony */
 	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
@@ -1404,7 +1405,9 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
 		if (!newmode)
 			return;
 
-		if (closure->preferred)
+		if (closure->preferred ||
+		    ((closure->quirks & EDID_QUIRK_FIRST_DETAILED_PREFERRED) &&
+		     !closure->modes))
 			newmode->type |= DRM_MODE_TYPE_PREFERRED;
 
 		drm_mode_probed_add(closure->connector, newmode);

Re: Bogus video resolution in Linux 3.5-rc4

[Takashi Iwai]

At Mon, 25 Jun 2012 17:53:12 +0200,
Takashi Iwai wrote:
> 
> And, does the patch below help?

BTW, the patch below contains the possible generic fix.
It seems that EDID_QUIRK_FIRST_DETAILED_PREFERRED handling is missing
from the beginning.  So I wrote it just from what I can imagine from
the comment:
  /* Monitor forgot to set the first detailed is preferred bit. */
  #define EDID_QUIRK_FIRST_DETAILED_PREFERRED	(1 << 5)

Adam, is my interpretation correct?


Takashi

> 
> 
> Takashi
> 
> ---
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5873e48..dab8580 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -116,6 +116,7 @@ static struct edid_quirk {
>  
>  	/* Proview AY765C */
>  	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> +	{ "PTS", 793, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
>  
>  	/* Samsung SyncMaster 205BW.  Note: irony */
>  	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
> @@ -1404,7 +1405,9 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
>  		if (!newmode)
>  			return;
>  
> -		if (closure->preferred)
> +		if (closure->preferred ||
> +		    ((closure->quirks & EDID_QUIRK_FIRST_DETAILED_PREFERRED) &&
> +		     !closure->modes))
>  			newmode->type |= DRM_MODE_TYPE_PREFERRED;
>  
>  		drm_mode_probed_add(closure->connector, newmode);

Re: Bogus video resolution in Linux 3.5-rc4

[Sven Joachim]

Am 25.06.2012 um 17:53 schrieb Takashi Iwai:

> Looking at the EDID data, the problem is likely that your monitor
> doesn't give the proper preferred mode.
> What does xrandr output show?

,----
| Screen 0: minimum 320 x 200, current 1280 x 1024, maximum 8192 x 8192
| DVI-I-1 connected 1280x1024+0+0 (normal left inverted right x axis y axis) 376mm x 301mm
|    1280x1024      75.0*    60.0  
|    1024x768       75.1     75.0     70.1     60.0  
|    832x624        74.6  
|    800x600        72.2     75.0     60.3     56.2  
|    640x480        72.8     75.0     66.7     60.0     59.9  
| VGA-1 disconnected (normal left inverted right x axis y axis)
`----

I should note that the monitor is actually connected via VGA, not DVI.
This seems to be similar to https://bugs.freedesktop.org/50830, but
since it never caused any real problems for me, I didn't bother.

> And, does the patch below help?

Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.
The xrandr command shows various bogus modes.

,----
| Screen 0: minimum 320 x 200, current 1280 x 1024, maximum 8192 x 8192
| DVI-I-1 connected 1280x1024+0+0 (normal left inverted right x axis y axis) 376mm x 301mm
|    1280x1024      60.0*+   75.0  
|    1680x945       60.0  
|    1400x1050      60.0  
|    1600x900       60.0  
|    1440x900       75.0     59.9  
|    1280x960       60.0  
|    1366x768       60.0  
|    1360x768       60.0  
|    1280x800       74.9     59.8  
|    1152x864       75.0  
|    1280x768       74.9     59.9  
|    1024x768       75.1     75.0     70.1     60.0  
|    1024x576       60.0  
|    832x624        74.6  
|    800x600        72.2     75.0     60.3     56.2  
|    848x480        60.0  
|    640x480        75.0     72.8     72.8     66.7     60.0     59.9  
| VGA-1 disconnected (normal left inverted right x axis y axis)
`----

Cheers,
       Sven


> ---
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5873e48..dab8580 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -116,6 +116,7 @@ static struct edid_quirk {
>  
>  	/* Proview AY765C */
>  	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> +	{ "PTS", 793, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
>  
>  	/* Samsung SyncMaster 205BW.  Note: irony */
>  	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
> @@ -1404,7 +1405,9 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
>  		if (!newmode)
>  			return;
>  
> -		if (closure->preferred)
> +		if (closure->preferred ||
> +		    ((closure->quirks & EDID_QUIRK_FIRST_DETAILED_PREFERRED) &&
> +		     !closure->modes))
>  			newmode->type |= DRM_MODE_TYPE_PREFERRED;
>  
>  		drm_mode_probed_add(closure->connector, newmode);

Re: Bogus video resolution in Linux 3.5-rc4

[Adam Jackson]

[-- Attachment #1: Type: text/plain, Size: 341 bytes --]

On Mon, 2012-06-25 at 19:40 +0200, Sven Joachim wrote:

> > And, does the patch below help?
> 
> Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.

That is, in fact, what your monitor claims to prefer.

> The xrandr command shows various bogus modes.

Most of which my patch series would eliminate.

- ajax

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Bogus video resolution in Linux 3.5-rc4

[Takashi Iwai]

At Mon, 25 Jun 2012 19:40:48 +0200,
Sven Joachim wrote:
> 
> Am 25.06.2012 um 17:53 schrieb Takashi Iwai:
> 
> > Looking at the EDID data, the problem is likely that your monitor
> > doesn't give the proper preferred mode.
> > What does xrandr output show?
> 
> ,----
> | Screen 0: minimum 320 x 200, current 1280 x 1024, maximum 8192 x 8192
> | DVI-I-1 connected 1280x1024+0+0 (normal left inverted right x axis y axis) 376mm x 301mm
> |    1280x1024      75.0*    60.0  
> |    1024x768       75.1     75.0     70.1     60.0  
> |    832x624        74.6  
> |    800x600        72.2     75.0     60.3     56.2  
> |    640x480        72.8     75.0     66.7     60.0     59.9  
> | VGA-1 disconnected (normal left inverted right x axis y axis)
> `----
> 
> I should note that the monitor is actually connected via VGA, not DVI.
> This seems to be similar to https://bugs.freedesktop.org/50830, but
> since it never caused any real problems for me, I didn't bother.
> 
> > And, does the patch below help?
> 
> Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.

I guess it worked casually because 1280x1024@75 was the highest
resolution / rate, so it was picked up as the preferred mode...

> The xrandr command shows various bogus modes.

Can't these values be displayed on your monitor at all?
If they can be displayed, they are valid modes, not really bogus.
After all, they are values that EDID of your montor advertises as
available ranges.


Takashi

> ,----
> | Screen 0: minimum 320 x 200, current 1280 x 1024, maximum 8192 x 8192
> | DVI-I-1 connected 1280x1024+0+0 (normal left inverted right x axis y axis) 376mm x 301mm
> |    1280x1024      60.0*+   75.0  
> |    1680x945       60.0  
> |    1400x1050      60.0  
> |    1600x900       60.0  
> |    1440x900       75.0     59.9  
> |    1280x960       60.0  
> |    1366x768       60.0  
> |    1360x768       60.0  
> |    1280x800       74.9     59.8  
> |    1152x864       75.0  
> |    1280x768       74.9     59.9  
> |    1024x768       75.1     75.0     70.1     60.0  
> |    1024x576       60.0  
> |    832x624        74.6  
> |    800x600        72.2     75.0     60.3     56.2  
> |    848x480        60.0  
> |    640x480        75.0     72.8     72.8     66.7     60.0     59.9  
> | VGA-1 disconnected (normal left inverted right x axis y axis)
> `----
> 
> Cheers,
>        Sven
> 
> 
> > ---
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 5873e48..dab8580 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -116,6 +116,7 @@ static struct edid_quirk {
> >  
> >  	/* Proview AY765C */
> >  	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> > +	{ "PTS", 793, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> >  
> >  	/* Samsung SyncMaster 205BW.  Note: irony */
> >  	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
> > @@ -1404,7 +1405,9 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
> >  		if (!newmode)
> >  			return;
> >  
> > -		if (closure->preferred)
> > +		if (closure->preferred ||
> > +		    ((closure->quirks & EDID_QUIRK_FIRST_DETAILED_PREFERRED) &&
> > +		     !closure->modes))
> >  			newmode->type |= DRM_MODE_TYPE_PREFERRED;
> >  
> >  		drm_mode_probed_add(closure->connector, newmode);
> 

Re: Bogus video resolution in Linux 3.5-rc4

[Sven Joachim]

Am 25.06.2012 um 21:24 schrieb Takashi Iwai:

>> > And, does the patch below help?
>> 
>> Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.
>
> I guess it worked casually because 1280x1024@75 was the highest
> resolution / rate, so it was picked up as the preferred mode...

Quite possible.  Problem is that 1280x1024@60 looks worse, so I'd like
to get the 75 Hz back.

>> The xrandr command shows various bogus modes.
>
> Can't these values be displayed on your monitor at all?

It's a TFT LCD with 1280x1024 pixels.

Cheers,
       Sven

Re: Bogus video resolution in Linux 3.5-rc4

[Andy Furniss]

Takashi Iwai wrote:

>> The xrandr command shows various bogus modes.
>
> Can't these values be displayed on your monitor at all?
> If they can be displayed, they are valid modes, not really bogus.
> After all, they are values that EDID of your montor advertises as
> available ranges.

I have already commented on bogus modes when the patch first went into 
dcn, but to repeat -

HDMI TV - lots of new modes but it already advertised all the CVT that 
it supports and all the new are bogus.

DVI 120Hz 1920x1080 monitor many bogus modes as it won't display > it's 
res and won't scale up some of the new modes.

Even some of the new ones that are not "out of range" will actually end 
up setting something different and show some distortion.

Maybe gained a couple.

Re: Bogus video resolution in Linux 3.5-rc4

[Andy Furniss]

Andy Furniss wrote:

> HDMI TV - lots of new modes but it already advertised all the CVT that
> it supports and all the new are bogus.

Oops CEA not CVT.

Re: Bogus video resolution in Linux 3.5-rc4

[Takashi Iwai]

At Mon, 25 Jun 2012 21:38:56 +0200,
Sven Joachim wrote:
> 
> Am 25.06.2012 um 21:24 schrieb Takashi Iwai:
> 
> >> > And, does the patch below help?
> >> 
> >> Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.
> >
> > I guess it worked casually because 1280x1024@75 was the highest
> > resolution / rate, so it was picked up as the preferred mode...
> 
> Quite possible.  Problem is that 1280x1024@60 looks worse, so I'd like
> to get the 75 Hz back.
> 
> >> The xrandr command shows various bogus modes.
> >
> > Can't these values be displayed on your monitor at all?
> 
> It's a TFT LCD with 1280x1024 pixels.

Yes, but displaying higher resolutions wasn't too uncommon in the
earlier VGA days.  So, this doesn't mean the higher modes are
"bogus" as long they are in the range the monitor itself advertises.

On the second thought, if there are many such broken monitors, it
might be safer to exclude the inferred modes with higher resolutions.

The original problem was that the resolution like 1366x768 or 1600x900
doesn't appear in the mode list.  These are supposed to be lower than
the native.  Restricting the higher resolutions won't regress for
these problems.

The patch below is a quick fix.


Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution

When a monitor EDID doesn't give the preferred bit, driver assumes
that the mode with the higest resolution and rate is the preferred
mode.  Meanwhile the recent changes for allowing more modes in the
GFT/CVT ranges give actually more modes, and some modes may be over
the native size.  Thus such a mode would be picked up as the preferred
mode although it's no native resolution.

For avoiding such a problem, this patch limits the addition of
inferred modes by checking not to be greater than other modes.
Also, it checks the duplicated mode entry at the same time.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/drm_edid.c |   27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5873e48..a8743c3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1039,6 +1039,24 @@ mode_in_range(const struct drm_display_mode *mode, struct edid *edid,
 	return true;
 }
 
+static bool valid_inferred_mode(const struct drm_connector *connector,
+				const struct drm_display_mode *mode)
+{
+	struct drm_display_mode *m;
+	bool ok = false;
+
+	list_for_each_entry(m, &connector->probed_modes, head) {
+		if (mode->hdisplay == m->hdisplay &&
+		    mode->vdisplay == m->vdisplay &&
+		    drm_mode_vrefresh(mode) == drm_mode_vrefresh(m))
+			return false; /* duplicated */
+		if (mode->hdisplay <= m->hdisplay &&
+		    mode->vdisplay <= m->vdisplay)
+			ok = true;
+	}
+	return ok;
+}
+
 static int
 drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
 			struct detailed_timing *timing)
@@ -1048,7 +1066,8 @@ drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
 	struct drm_device *dev = connector->dev;
 
 	for (i = 0; i < drm_num_dmt_modes; i++) {
-		if (mode_in_range(drm_dmt_modes + i, edid, timing)) {
+		if (mode_in_range(drm_dmt_modes + i, edid, timing) &&
+		    valid_inferred_mode(connector, drm_dmt_modes + i)) {
 			newmode = drm_mode_duplicate(dev, &drm_dmt_modes[i]);
 			if (newmode) {
 				drm_mode_probed_add(connector, newmode);
@@ -1088,7 +1107,8 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid,
 			return modes;
 
 		fixup_mode_1366x768(newmode);
-		if (!mode_in_range(newmode, edid, timing)) {
+		if (!mode_in_range(newmode, edid, timing) ||
+		    !valid_inferred_mode(connector, newmode)) {
 			drm_mode_destroy(dev, newmode);
 			continue;
 		}
@@ -1116,7 +1136,8 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
 			return modes;
 
 		fixup_mode_1366x768(newmode);
-		if (!mode_in_range(newmode, edid, timing)) {
+		if (!mode_in_range(newmode, edid, timing) ||
+		    !valid_inferred_mode(connector, newmode)) {
 			drm_mode_destroy(dev, newmode);
 			continue;
 		}
-- 
1.7.10.4

Re: Bogus video resolution in Linux 3.5-rc4

[Calvin Owens]

On 06/26/2012 02:21 AM, Takashi Iwai wrote:
> At Mon, 25 Jun 2012 21:38:56 +0200,
> Sven Joachim wrote:
>>
>> Am 25.06.2012 um 21:24 schrieb Takashi Iwai:
>>
>>>>> And, does the patch below help?
>>>>
>>>> Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.
>>>
>>> I guess it worked casually because 1280x1024@75 was the highest
>>> resolution / rate, so it was picked up as the preferred mode...
>>
>> Quite possible.  Problem is that 1280x1024@60 looks worse, so I'd like
>> to get the 75 Hz back.
>>
>>>> The xrandr command shows various bogus modes.
>>>
>>> Can't these values be displayed on your monitor at all?
>>
>> It's a TFT LCD with 1280x1024 pixels.
> 
> Yes, but displaying higher resolutions wasn't too uncommon in the
> earlier VGA days.  So, this doesn't mean the higher modes are
> "bogus" as long they are in the range the monitor itself advertises.
> 
> On the second thought, if there are many such broken monitors, it
> might be safer to exclude the inferred modes with higher resolutions.
> 
> The original problem was that the resolution like 1366x768 or 1600x900
> doesn't appear in the mode list.  These are supposed to be lower than
> the native.  Restricting the higher resolutions won't regress for
> these problems.
> 
> The patch below is a quick fix.
> 
> 
> Takashi
> 
> ---
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution
> 
> When a monitor EDID doesn't give the preferred bit, driver assumes
> that the mode with the higest resolution and rate is the preferred
> mode.  Meanwhile the recent changes for allowing more modes in the
> GFT/CVT ranges give actually more modes, and some modes may be over
> the native size.  Thus such a mode would be picked up as the preferred
> mode although it's no native resolution.
> 
> For avoiding such a problem, this patch limits the addition of
> inferred modes by checking not to be greater than other modes.
> Also, it checks the duplicated mode entry at the same time.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/drm_edid.c |   27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5873e48..a8743c3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1039,6 +1039,24 @@ mode_in_range(const struct drm_display_mode *mode, struct edid *edid,
>  	return true;
>  }
>  
> +static bool valid_inferred_mode(const struct drm_connector *connector,
> +				const struct drm_display_mode *mode)
> +{
> +	struct drm_display_mode *m;
> +	bool ok = false;
> +
> +	list_for_each_entry(m, &connector->probed_modes, head) {
> +		if (mode->hdisplay == m->hdisplay &&
> +		    mode->vdisplay == m->vdisplay &&
> +		    drm_mode_vrefresh(mode) == drm_mode_vrefresh(m))
> +			return false; /* duplicated */
> +		if (mode->hdisplay <= m->hdisplay &&
> +		    mode->vdisplay <= m->vdisplay)
> +			ok = true;
> +	}
> +	return ok;
> +}
> +
>  static int
>  drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
>  			struct detailed_timing *timing)
> @@ -1048,7 +1066,8 @@ drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
>  	struct drm_device *dev = connector->dev;
>  
>  	for (i = 0; i < drm_num_dmt_modes; i++) {
> -		if (mode_in_range(drm_dmt_modes + i, edid, timing)) {
> +		if (mode_in_range(drm_dmt_modes + i, edid, timing) &&
> +		    valid_inferred_mode(connector, drm_dmt_modes + i)) {
>  			newmode = drm_mode_duplicate(dev, &drm_dmt_modes[i]);
>  			if (newmode) {
>  				drm_mode_probed_add(connector, newmode);
> @@ -1088,7 +1107,8 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid,
>  			return modes;
>  
>  		fixup_mode_1366x768(newmode);
> -		if (!mode_in_range(newmode, edid, timing)) {
> +		if (!mode_in_range(newmode, edid, timing) ||
> +		    !valid_inferred_mode(connector, newmode)) {
>  			drm_mode_destroy(dev, newmode);
>  			continue;
>  		}
> @@ -1116,7 +1136,8 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
>  			return modes;
>  
>  		fixup_mode_1366x768(newmode);
> -		if (!mode_in_range(newmode, edid, timing)) {
> +		if (!mode_in_range(newmode, edid, timing) ||
> +		    !valid_inferred_mode(connector, newmode)) {
>  			drm_mode_destroy(dev, newmode);
>  			continue;
>  		}

Hello all,

I had the exact same problem as Sven, bisected to cb21aafe121b1c3ad4c77cc5c22320163f16ba42.
Takashi's patch (supra) fixes the issue for me.

Regards,
Calvin Owens

Re: Bogus video resolution in Linux 3.5-rc4

[Takashi Iwai]

At Sat, 30 Jun 2012 13:46:54 -0500,
Calvin Owens wrote:
> 
> On 06/26/2012 02:21 AM, Takashi Iwai wrote:
> > At Mon, 25 Jun 2012 21:38:56 +0200,
> > Sven Joachim wrote:
> >>
> >> Am 25.06.2012 um 21:24 schrieb Takashi Iwai:
> >>
> >>>>> And, does the patch below help?
> >>>>
> >>>> Somewhat: at least I get 1280x1024 again, but at 60 rather than 75 Hz.
> >>>
> >>> I guess it worked casually because 1280x1024@75 was the highest
> >>> resolution / rate, so it was picked up as the preferred mode...
> >>
> >> Quite possible.  Problem is that 1280x1024@60 looks worse, so I'd like
> >> to get the 75 Hz back.
> >>
> >>>> The xrandr command shows various bogus modes.
> >>>
> >>> Can't these values be displayed on your monitor at all?
> >>
> >> It's a TFT LCD with 1280x1024 pixels.
> > 
> > Yes, but displaying higher resolutions wasn't too uncommon in the
> > earlier VGA days.  So, this doesn't mean the higher modes are
> > "bogus" as long they are in the range the monitor itself advertises.
> > 
> > On the second thought, if there are many such broken monitors, it
> > might be safer to exclude the inferred modes with higher resolutions.
> > 
> > The original problem was that the resolution like 1366x768 or 1600x900
> > doesn't appear in the mode list.  These are supposed to be lower than
> > the native.  Restricting the higher resolutions won't regress for
> > these problems.
> > 
> > The patch below is a quick fix.
> > 
> > 
> > Takashi
> > 
> > ---
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution
> > 
> > When a monitor EDID doesn't give the preferred bit, driver assumes
> > that the mode with the higest resolution and rate is the preferred
> > mode.  Meanwhile the recent changes for allowing more modes in the
> > GFT/CVT ranges give actually more modes, and some modes may be over
> > the native size.  Thus such a mode would be picked up as the preferred
> > mode although it's no native resolution.
> > 
> > For avoiding such a problem, this patch limits the addition of
> > inferred modes by checking not to be greater than other modes.
> > Also, it checks the duplicated mode entry at the same time.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/drm_edid.c |   27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 5873e48..a8743c3 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -1039,6 +1039,24 @@ mode_in_range(const struct drm_display_mode *mode, struct edid *edid,
> >  	return true;
> >  }
> >  
> > +static bool valid_inferred_mode(const struct drm_connector *connector,
> > +				const struct drm_display_mode *mode)
> > +{
> > +	struct drm_display_mode *m;
> > +	bool ok = false;
> > +
> > +	list_for_each_entry(m, &connector->probed_modes, head) {
> > +		if (mode->hdisplay == m->hdisplay &&
> > +		    mode->vdisplay == m->vdisplay &&
> > +		    drm_mode_vrefresh(mode) == drm_mode_vrefresh(m))
> > +			return false; /* duplicated */
> > +		if (mode->hdisplay <= m->hdisplay &&
> > +		    mode->vdisplay <= m->vdisplay)
> > +			ok = true;
> > +	}
> > +	return ok;
> > +}
> > +
> >  static int
> >  drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
> >  			struct detailed_timing *timing)
> > @@ -1048,7 +1066,8 @@ drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
> >  	struct drm_device *dev = connector->dev;
> >  
> >  	for (i = 0; i < drm_num_dmt_modes; i++) {
> > -		if (mode_in_range(drm_dmt_modes + i, edid, timing)) {
> > +		if (mode_in_range(drm_dmt_modes + i, edid, timing) &&
> > +		    valid_inferred_mode(connector, drm_dmt_modes + i)) {
> >  			newmode = drm_mode_duplicate(dev, &drm_dmt_modes[i]);
> >  			if (newmode) {
> >  				drm_mode_probed_add(connector, newmode);
> > @@ -1088,7 +1107,8 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid,
> >  			return modes;
> >  
> >  		fixup_mode_1366x768(newmode);
> > -		if (!mode_in_range(newmode, edid, timing)) {
> > +		if (!mode_in_range(newmode, edid, timing) ||
> > +		    !valid_inferred_mode(connector, newmode)) {
> >  			drm_mode_destroy(dev, newmode);
> >  			continue;
> >  		}
> > @@ -1116,7 +1136,8 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
> >  			return modes;
> >  
> >  		fixup_mode_1366x768(newmode);
> > -		if (!mode_in_range(newmode, edid, timing)) {
> > +		if (!mode_in_range(newmode, edid, timing) ||
> > +		    !valid_inferred_mode(connector, newmode)) {
> >  			drm_mode_destroy(dev, newmode);
> >  			continue;
> >  		}
> 
> Hello all,
> 
> I had the exact same problem as Sven, bisected to cb21aafe121b1c3ad4c77cc5c22320163f16ba42.
> Takashi's patch (supra) fixes the issue for me.

OK, so there can be more people hit by this issue.

Adam, what do you think?  My patch is acceptable?


thanks,

Takashi

Re: Bogus video resolution in Linux 3.5-rc4

[Sven Joachim]

On 2012-06-30 20:46 +0200, Calvin Owens wrote:

> I had the exact same problem as Sven, bisected to cb21aafe121b1c3ad4c77cc5c22320163f16ba42.
> Takashi's patch (supra) fixes the issue for me.

For me as well.  Sorry for the slightly belated reply, was busy with
other things in the last few days.

Thanks,
       Sven

Re: Bogus video resolution in Linux 3.5-rc4

[Adam Jackson]

On 6/26/12 3:21 AM, Takashi Iwai wrote:

> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution
>
> When a monitor EDID doesn't give the preferred bit, driver assumes
> that the mode with the higest resolution and rate is the preferred
> mode.  Meanwhile the recent changes for allowing more modes in the
> GFT/CVT ranges give actually more modes, and some modes may be over
> the native size.  Thus such a mode would be picked up as the preferred
> mode although it's no native resolution.
>
> For avoiding such a problem, this patch limits the addition of
> inferred modes by checking not to be greater than other modes.
> Also, it checks the duplicated mode entry at the same time.

This is a little aggressive on CRTs, but whatever, better than what's there.

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax

Re: Bogus video resolution in Linux 3.5-rc4

[Takashi Iwai]

At Mon, 02 Jul 2012 15:46:29 -0400,
Adam Jackson wrote:
> 
> On 6/26/12 3:21 AM, Takashi Iwai wrote:
> 
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] drm: edid: Don't add inferred modes with higher resolution
> >
> > When a monitor EDID doesn't give the preferred bit, driver assumes
> > that the mode with the higest resolution and rate is the preferred
> > mode.  Meanwhile the recent changes for allowing more modes in the
> > GFT/CVT ranges give actually more modes, and some modes may be over
> > the native size.  Thus such a mode would be picked up as the preferred
> > mode although it's no native resolution.
> >
> > For avoiding such a problem, this patch limits the addition of
> > inferred modes by checking not to be greater than other modes.
> > Also, it checks the duplicated mode entry at the same time.
> 
> This is a little aggressive on CRTs, but whatever, better than what's there.
> 
> Reviewed-by: Adam Jackson <ajax@redhat.com>

Thanks.  I'm going to resend the patch with your tag.


Takashi