[PATCH] drm: edid: enable probing and listing of non rb modes

[Adam Jackson]

On Fri, 2013-11-15 at 10:38 +0530, Shirish S wrote:
> The current solution checks for the existing RB mode,
> if available in the edid block returns by adding it,
> but does not populate the connector with the modes
> of same resolution but which are non-rb modes.
> 
> As a result the probing and listing of non-rb modes can't
> be made, in case the rb mode's pixel clock is not
> supported but non-rb mode is supported.

This is... almost okay.

So the EDID 1.4 spec says that for modes in standard descriptors:

a) if there's a match in DMT, use DMT
b) if there's a range descriptor and it says GTF, use GTF
c) if there's a range descriptor and it says CVT, use CVT
d) if there's no range descriptor, use CVT

But case d) is clearly insane if the sink is EDID 1.3, CVT wasn't a
standard when 1.3 was defined, so the sink may not in fact support CVT
timings.  Hence the logic in standard_timing_level().

The other thing the spec says is that if you're using CVT for standard
descriptors that you should use normal blanking instead of reduced.
This is... problematic.  If we see 1920x1200 at 60 in a standard descriptor
we almost certainly _don't_ mean normal blanking, because that won't fit
on single-link DVI.  And if both the source and the sink support reduced
blanking we should probably prefer it, it's marginally less power
consumption on digital links and marginally better picture quality on
analog links.

So probably what we should do instead is:

- if drm_monitor_supports_rb(), add both normal and reduced blanking
timings for either the DMT or CVT path
- extend drm_connector to also have rb-allowed to match interlace and
doublescan
- fix up all the drivers to indicate rb-allowed (except whatever broken
thing it is you're trying to work around here)
- fix drm_mode_validate_flag and callers to filter out rb modes as
appropriate

Also: CVT dates to 2003.  EDID 1.4 was 2006.  Any hardware being made in
2013 that can't generate RB timings is going out of its way to be
broken.  You should really demand better from your hardware.

> 1. adds "rb" suffix to rb modes.

No.  The userspace convention is:

dmt:~% cvt -r 1920 1080
# 1920x1080 59.93 Hz (CVT 2.07M9-R) hsync: 66.59 kHz; pclk: 138.50 MHz
Modeline "1920x1080R"  138.50  1920 1968 2000 2080  1080 1083 1088 1111 +hsync -vsync

drm_mode_parse_command_line_for_connector() expects the same thing.
Let's be consistent.

> @@ -1621,19 +1621,20 @@ drm_mode_std(struct drm_connector *connector, struct edid *edid,
>  		mode->hdisplay = 1366;
>  		mode->hsync_start = mode->hsync_start - 1;
>  		mode->hsync_end = mode->hsync_end - 1;
> -		return mode;
> +		goto done;
>  	}
>  
>  	/* check whether it can be found in default mode table */
>  	if (drm_monitor_supports_rb(edid)) {
>  		mode = drm_mode_find_dmt(dev, hsize, vsize, vrefresh_rate,
>  					 true);
> -		if (mode)
> -			return mode;
> +		if (mode) {
> +			drm_mode_probed_add(connector, mode);
> +			modes++;
> +		}
>  	}
>  	mode = drm_mode_find_dmt(dev, hsize, vsize, vrefresh_rate, false);
> -	if (mode)
> -		return mode;
> +	goto done;
>  
>  	/* okay, generate it */
>  	switch (timing_level) {

Unconditional "goto done" that skips all the logic for generating modes
not in the dmt list?  This breaks everything _but_ case a) above, so eg.
1600x900 at 60 would no longer be generated.

- ajax