Re: [PATCH] drm_fb_helper: fix regression in pixclock check

[Clemens Ladisch <clemens@ladisch.de>]

Jason Wessel wrote:
> Commit 5349ef3127c77075ff70b2014f17ae0fbcaaf199 changed pixclock to be
> initialized to zero instead of -1.

Zero meaning "there is no pixel clock value".  When I wrote the patch,
the FB (helper) code did not use this value at all, so zero was the only
value that could possibly be used.

> The validation routine always returns -EINVAL for a valid pixclock

It declares that the only valid pixclock value is "none".  This was
required to make the userspace API work, since the GET ioctls return
this value, and all programs expect to be able to PUT this value back.

> which prevents atomic kernel mode setting from working correctly.
> ...
> @@ -602,7 +602,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
>  
> -	if (var->pixclock != 0)
> +	if (!var->pixclock)
>  		return -EINVAL;

This change breaks the userspace API again, unless the other parts of
the code have been changed to always return a valid pixclock value when
reading the current mode settings.

If we want to accept both zero and non-zero values for pixclock, this
check should be dropped altogether.

In drm_fb_helper_set_par(), there is a corresponding check that must be
kept in sync with this one.


Best regards,
Clemens