[PATCH 5/6] compositor-drm: Fix refresh rate selection in drm_output_switch_mode

[Mario Kleiner]

On 04/16/2015 07:14 PM, Derek Foreman wrote:
> On 02/04/15 12:10 AM, Mario Kleiner wrote:
>> The matching logic in choose_mode() compared refresh rate
>> of a drm_mode candidate mode expressed in Hz against the
>> requested refresh rate of the target weston_mode expressed
>> in milliHz, so the match always failed and the logic always
>> ended up the mode with the highest refresh rate for a given
>> resolution, instead of the one matching the requested rate.
>>
>> Match proper fields to fix this.
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
>> ---
>>   src/compositor-drm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>> index 9ddb6d6..396938f 100644
>> --- a/src/compositor-drm.c
>> +++ b/src/compositor-drm.c
>> @@ -1270,7 +1270,7 @@ choose_mode (struct drm_output *output, struct weston_mode *target_mode)
>>   	wl_list_for_each(mode, &output->base.mode_list, base.link) {
>>   		if (mode->mode_info.hdisplay == target_mode->width &&
>>   		    mode->mode_info.vdisplay == target_mode->height) {
>> -			if (mode->mode_info.vrefresh == target_mode->refresh ||
>> +			if (mode->base.refresh == target_mode->refresh ||
>
> Definitely wrong in the current form, but I'm mildly concerned that the
> math used to calculate mode->base.refresh is not just 1000 *
> mode_info.vdisplay.
>
> For my monitor, 1280x1024 with vrefresh = 60 has a refresh = 60020
> 1440x900 at 60 -> 59901
>
> Should we be trying to do a nearest match instead of an exact match?
>

This specific patch is a bug-fix for the current wrong behaviour.

If we want the nearest match behaviour i'd probably write a separate 
patch with the "find mode with identical resolution but nearest refresh 
rate" on top of this one?

The way my app uses it is it enumerates all video modes from the 
wl_output listener, selects the best matching mode for its purpose, then 
passes in the exact parameters from that mode, because what it needs is 
exact mode selection. But a nearest match wrt. refresh rate would make 
sense and not prevent that use.

thanks,
-mario