Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 18 Dec 2009 08:20:39 +0100 | From | Clemens Ladisch <> | Subject | Re: [PATCH] drm_fb_helper: fix regression in pixclock check |
| |
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
| ![\](/images/icornerr.gif) |