Messages in this thread | | | Date | Wed, 23 Dec 2009 13:04:27 -0600 | From | Jason Wessel <> | Subject | Re: [PATCH] drm_fb_helper: fix regression in pixclock check |
| |
Clemens Ladisch wrote: > 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. > >
Fair enough. We'll make a special case for when the kernel debugger is active.
Looks to me like it worked by luck during the 2.6.32 kernel cycle for the atomic kernel mode setting. I have changed it in the next version of this patch to always return -EINVAL while the kernel debugger is active. This restores the atomic kernel mode setting back to the working state, until we can find a better solution.
Thanks, Jason.
| |