Message ID | 20181002133526.13685-8-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | atomic helper cleanups | expand |
Hi, Daniel, On 10/02/2018 03:35 PM, Daniel Vetter wrote: > The idea behind allowing drivers to override legacy ioctls (instead of > using the generic implementations unconditionally) is to handle bugs > in old driver-specific userspace. Like e.g. vmw_kms_set_config does, > to work around some vmwgfx userspace not clearing its ioctl structs > properly. > > But you can't use it to augment semantics and put in additional > checks, since from a correctly working userspace's pov there should > not be any difference in behaviour between the legacy and the atomic > paths. > > vmwgfx seems to be doing some strange things in its page_flip > handlers. Since I'm not an expert of this codebase just wrap some > FIXME comments around the potentially problematic code. > We've got proper patches for these. Apparently they got lost in my -next pull request, though. /Thomas
On Tue, Oct 02, 2018 at 04:49:30PM +0000, Thomas Hellstrom wrote: > Hi, Daniel, > > On 10/02/2018 03:35 PM, Daniel Vetter wrote: > > The idea behind allowing drivers to override legacy ioctls (instead of > > using the generic implementations unconditionally) is to handle bugs > > in old driver-specific userspace. Like e.g. vmw_kms_set_config does, > > to work around some vmwgfx userspace not clearing its ioctl structs > > properly. > > > > But you can't use it to augment semantics and put in additional > > checks, since from a correctly working userspace's pov there should > > not be any difference in behaviour between the legacy and the atomic > > paths. > > > > vmwgfx seems to be doing some strange things in its page_flip > > handlers. Since I'm not an expert of this codebase just wrap some > > FIXME comments around the potentially problematic code. > > > > We've got proper patches for these. Apparently they got lost in my -next > pull request, though. Yeah I wondered why this one here didn't conflict yet, I can carry it around a bit longer as a memo. -Daniel
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c index 4c68ad6f3605..8e692334c3b9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c @@ -326,6 +326,7 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc, struct vmw_private *dev_priv = vmw_priv(crtc->dev); int ret; + /* FIMXE: This check needs to be moved into ->atomic_check code. */ if (!vmw_kms_crtc_flippable(dev_priv, crtc)) return -EINVAL; @@ -335,6 +336,7 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc, return ret; } + /* FIMXE: This code needs to be moved into ->atomic_commit callbacks. */ if (vmw_crtc_to_du(crtc)->is_implicit) vmw_kms_update_implicit_fb(dev_priv, crtc); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index e28bb08114a5..199927f20e0a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -501,6 +501,7 @@ static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc, struct vmw_screen_target_display_unit *stdu = vmw_crtc_to_stdu(crtc); int ret; + /* FIMXE: This check needs to be moved into ->atomic_check code. */ if (!stdu->defined || !vmw_kms_crtc_flippable(dev_priv, crtc)) return -EINVAL;
The idea behind allowing drivers to override legacy ioctls (instead of using the generic implementations unconditionally) is to handle bugs in old driver-specific userspace. Like e.g. vmw_kms_set_config does, to work around some vmwgfx userspace not clearing its ioctl structs properly. But you can't use it to augment semantics and put in additional checks, since from a correctly working userspace's pov there should not be any difference in behaviour between the legacy and the atomic paths. vmwgfx seems to be doing some strange things in its page_flip handlers. Since I'm not an expert of this codebase just wrap some FIXME comments around the potentially problematic code. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Sinclair Yeh <syeh@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 ++ drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 1 + 2 files changed, 3 insertions(+)