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 at 75 was the highest
> >>> resolution / rate, so it was picked up as the preferred mode...
> >>
> >> Quite possible.  Problem is that 1280x1024 at 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 at 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 at 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