| Message ID | 1435610686-2249-4-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 29, 2015 at 01:44:46PM -0700, Rodrigo Vivi wrote: > Now that we have fb user dirty invalidating frontbuffer and we have > the new fbdev dirty callback let's merge them. > > So it doesn't matter if fbcon throught fbdev or splash screen throught > drm_ioctl_dirtyfb, in any case we will have frontbuffer properly > invalidated and power savings features that rely on frontbuffer tracking > will be able to work as expected. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Hm, I think we still need these but not sure. There's also fbdev client from userspace which directly draw into the mmap area. But tbh no idea how those are supposed to work with manually updating screens (like i915 psr, udl or qxl). -Daniel > --- > drivers/gpu/drm/i915/intel_fbdev.c | 86 ++++++-------------------------------- > 1 file changed, 13 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 2a1724e..f1592c7 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -45,92 +45,32 @@ > #include <drm/i915_drm.h> > #include "i915_drv.h" > > -static int intel_fbdev_set_par(struct fb_info *info) > +static void intel_fbdev_dirty(struct fb_info *info, u32 x1, u32 y1, > + u32 x2, u32 y2) > { > struct drm_fb_helper *fb_helper = info->par; > struct intel_fbdev *ifbdev = > container_of(fb_helper, struct intel_fbdev, helper); > - int ret; > - > - ret = drm_fb_helper_set_par(info); > - > - if (ret == 0) { > - /* > - * FIXME: fbdev presumes that all callbacks also work from > - * atomic contexts and relies on that for emergency oops > - * printing. KMS totally doesn't do that and the locking here is > - * by far not the only place this goes wrong. Ignore this for > - * now until we solve this for real. > - */ > - mutex_lock(&fb_helper->dev->struct_mutex); > - ret = i915_gem_object_set_to_gtt_domain(ifbdev->fb->obj, > - true); > - mutex_unlock(&fb_helper->dev->struct_mutex); > - } > - > - return ret; > -} > - > -static int intel_fbdev_blank(int blank, struct fb_info *info) > -{ > - struct drm_fb_helper *fb_helper = info->par; > - struct intel_fbdev *ifbdev = > - container_of(fb_helper, struct intel_fbdev, helper); > - int ret; > - > - ret = drm_fb_helper_blank(blank, info); > - > - if (ret == 0) { > - /* > - * FIXME: fbdev presumes that all callbacks also work from > - * atomic contexts and relies on that for emergency oops > - * printing. KMS totally doesn't do that and the locking here is > - * by far not the only place this goes wrong. Ignore this for > - * now until we solve this for real. > - */ > - mutex_lock(&fb_helper->dev->struct_mutex); > - intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > - mutex_unlock(&fb_helper->dev->struct_mutex); > - } > - > - return ret; > -} > - > -static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, > - struct fb_info *info) > -{ > - struct drm_fb_helper *fb_helper = info->par; > - struct intel_fbdev *ifbdev = > - container_of(fb_helper, struct intel_fbdev, helper); > - > - int ret; > - ret = drm_fb_helper_pan_display(var, info); > - > - if (ret == 0) { > - /* > - * FIXME: fbdev presumes that all callbacks also work from > - * atomic contexts and relies on that for emergency oops > - * printing. KMS totally doesn't do that and the locking here is > - * by far not the only place this goes wrong. Ignore this for > - * now until we solve this for real. > - */ > - mutex_lock(&fb_helper->dev->struct_mutex); > - intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > - mutex_unlock(&fb_helper->dev->struct_mutex); > - } > + struct intel_framebuffer *intel_fb = ifbdev->fb; > + struct drm_framebuffer *fb = &intel_fb->base; > > - return ret; > + /* > + * Our fb dirty callback is just used to invalidate frontbuffer > + * entirely. So just fb reference is needed and rest is ignored. > + */ > + fb->funcs->dirty(fb, NULL, 0, 0, NULL, 1); > } > > static struct fb_ops intelfb_ops = { > .owner = THIS_MODULE, > .fb_check_var = drm_fb_helper_check_var, > - .fb_set_par = intel_fbdev_set_par, > + .fb_set_par = drm_fb_helper_set_par, > .fb_fillrect = cfb_fillrect, > .fb_copyarea = cfb_copyarea, > .fb_imageblit = cfb_imageblit, > - .fb_pan_display = intel_fbdev_pan_display, > - .fb_blank = intel_fbdev_blank, > + .fb_dirty = intel_fbdev_dirty, > + .fb_pan_display = drm_fb_helper_pan_display, > + .fb_blank = drm_fb_helper_blank, > .fb_setcmap = drm_fb_helper_setcmap, > .fb_debug_enter = drm_fb_helper_debug_enter, > .fb_debug_leave = drm_fb_helper_debug_leave, > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jun 30, 2015 at 12:08 AM Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Jun 29, 2015 at 01:44:46PM -0700, Rodrigo Vivi wrote: > > Now that we have fb user dirty invalidating frontbuffer and we have > > the new fbdev dirty callback let's merge them. > > > > So it doesn't matter if fbcon throught fbdev or splash screen throught > > drm_ioctl_dirtyfb, in any case we will have frontbuffer properly > > invalidated and power savings features that rely on frontbuffer tracking > > will be able to work as expected. > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Hm, I think we still need these but not sure. There's also fbdev client > from userspace which directly draw into the mmap area. But tbh no idea how > those are supposed to work with manually updating screens (like i915 psr, > udl or qxl). > Oh! Agree... I got the issue after blank so we still need those... Let's forget about these fbdev changes and focus on a simple fb user dirty ops that fix the current real remaining issue... > -Daniel > > > --- > > drivers/gpu/drm/i915/intel_fbdev.c | 86 > ++++++-------------------------------- > > 1 file changed, 13 insertions(+), 73 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > b/drivers/gpu/drm/i915/intel_fbdev.c > > index 2a1724e..f1592c7 100644 > > --- a/drivers/gpu/drm/i915/intel_fbdev.c > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > > @@ -45,92 +45,32 @@ > > #include <drm/i915_drm.h> > > #include "i915_drv.h" > > > > -static int intel_fbdev_set_par(struct fb_info *info) > > +static void intel_fbdev_dirty(struct fb_info *info, u32 x1, u32 y1, > > + u32 x2, u32 y2) > > { > > struct drm_fb_helper *fb_helper = info->par; > > struct intel_fbdev *ifbdev = > > container_of(fb_helper, struct intel_fbdev, helper); > > - int ret; > > - > > - ret = drm_fb_helper_set_par(info); > > - > > - if (ret == 0) { > > - /* > > - * FIXME: fbdev presumes that all callbacks also work from > > - * atomic contexts and relies on that for emergency oops > > - * printing. KMS totally doesn't do that and the locking > here is > > - * by far not the only place this goes wrong. Ignore this > for > > - * now until we solve this for real. > > - */ > > - mutex_lock(&fb_helper->dev->struct_mutex); > > - ret = i915_gem_object_set_to_gtt_domain(ifbdev->fb->obj, > > - true); > > - mutex_unlock(&fb_helper->dev->struct_mutex); > > - } > > - > > - return ret; > > -} > > - > > -static int intel_fbdev_blank(int blank, struct fb_info *info) > > -{ > > - struct drm_fb_helper *fb_helper = info->par; > > - struct intel_fbdev *ifbdev = > > - container_of(fb_helper, struct intel_fbdev, helper); > > - int ret; > > - > > - ret = drm_fb_helper_blank(blank, info); > > - > > - if (ret == 0) { > > - /* > > - * FIXME: fbdev presumes that all callbacks also work from > > - * atomic contexts and relies on that for emergency oops > > - * printing. KMS totally doesn't do that and the locking > here is > > - * by far not the only place this goes wrong. Ignore this > for > > - * now until we solve this for real. > > - */ > > - mutex_lock(&fb_helper->dev->struct_mutex); > > - intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > > - mutex_unlock(&fb_helper->dev->struct_mutex); > > - } > > - > > - return ret; > > -} > > - > > -static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, > > - struct fb_info *info) > > -{ > > - struct drm_fb_helper *fb_helper = info->par; > > - struct intel_fbdev *ifbdev = > > - container_of(fb_helper, struct intel_fbdev, helper); > > - > > - int ret; > > - ret = drm_fb_helper_pan_display(var, info); > > - > > - if (ret == 0) { > > - /* > > - * FIXME: fbdev presumes that all callbacks also work from > > - * atomic contexts and relies on that for emergency oops > > - * printing. KMS totally doesn't do that and the locking > here is > > - * by far not the only place this goes wrong. Ignore this > for > > - * now until we solve this for real. > > - */ > > - mutex_lock(&fb_helper->dev->struct_mutex); > > - intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > > - mutex_unlock(&fb_helper->dev->struct_mutex); > > - } > > + struct intel_framebuffer *intel_fb = ifbdev->fb; > > + struct drm_framebuffer *fb = &intel_fb->base; > > > > - return ret; > > + /* > > + * Our fb dirty callback is just used to invalidate frontbuffer > > + * entirely. So just fb reference is needed and rest is ignored. > > + */ > > + fb->funcs->dirty(fb, NULL, 0, 0, NULL, 1); > > } > > > > static struct fb_ops intelfb_ops = { > > .owner = THIS_MODULE, > > .fb_check_var = drm_fb_helper_check_var, > > - .fb_set_par = intel_fbdev_set_par, > > + .fb_set_par = drm_fb_helper_set_par, > > .fb_fillrect = cfb_fillrect, > > .fb_copyarea = cfb_copyarea, > > .fb_imageblit = cfb_imageblit, > > - .fb_pan_display = intel_fbdev_pan_display, > > - .fb_blank = intel_fbdev_blank, > > + .fb_dirty = intel_fbdev_dirty, > > + .fb_pan_display = drm_fb_helper_pan_display, > > + .fb_blank = drm_fb_helper_blank, > > .fb_setcmap = drm_fb_helper_setcmap, > > .fb_debug_enter = drm_fb_helper_debug_enter, > > .fb_debug_leave = drm_fb_helper_debug_leave, > > -- > > 2.1.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 2a1724e..f1592c7 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -45,92 +45,32 @@ #include <drm/i915_drm.h> #include "i915_drv.h" -static int intel_fbdev_set_par(struct fb_info *info) +static void intel_fbdev_dirty(struct fb_info *info, u32 x1, u32 y1, + u32 x2, u32 y2) { struct drm_fb_helper *fb_helper = info->par; struct intel_fbdev *ifbdev = container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_set_par(info); - - if (ret == 0) { - /* - * FIXME: fbdev presumes that all callbacks also work from - * atomic contexts and relies on that for emergency oops - * printing. KMS totally doesn't do that and the locking here is - * by far not the only place this goes wrong. Ignore this for - * now until we solve this for real. - */ - mutex_lock(&fb_helper->dev->struct_mutex); - ret = i915_gem_object_set_to_gtt_domain(ifbdev->fb->obj, - true); - mutex_unlock(&fb_helper->dev->struct_mutex); - } - - return ret; -} - -static int intel_fbdev_blank(int blank, struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - int ret; - - ret = drm_fb_helper_blank(blank, info); - - if (ret == 0) { - /* - * FIXME: fbdev presumes that all callbacks also work from - * atomic contexts and relies on that for emergency oops - * printing. KMS totally doesn't do that and the locking here is - * by far not the only place this goes wrong. Ignore this for - * now until we solve this for real. - */ - mutex_lock(&fb_helper->dev->struct_mutex); - intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); - mutex_unlock(&fb_helper->dev->struct_mutex); - } - - return ret; -} - -static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, - struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct intel_fbdev *ifbdev = - container_of(fb_helper, struct intel_fbdev, helper); - - int ret; - ret = drm_fb_helper_pan_display(var, info); - - if (ret == 0) { - /* - * FIXME: fbdev presumes that all callbacks also work from - * atomic contexts and relies on that for emergency oops - * printing. KMS totally doesn't do that and the locking here is - * by far not the only place this goes wrong. Ignore this for - * now until we solve this for real. - */ - mutex_lock(&fb_helper->dev->struct_mutex); - intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); - mutex_unlock(&fb_helper->dev->struct_mutex); - } + struct intel_framebuffer *intel_fb = ifbdev->fb; + struct drm_framebuffer *fb = &intel_fb->base; - return ret; + /* + * Our fb dirty callback is just used to invalidate frontbuffer + * entirely. So just fb reference is needed and rest is ignored. + */ + fb->funcs->dirty(fb, NULL, 0, 0, NULL, 1); } static struct fb_ops intelfb_ops = { .owner = THIS_MODULE, .fb_check_var = drm_fb_helper_check_var, - .fb_set_par = intel_fbdev_set_par, + .fb_set_par = drm_fb_helper_set_par, .fb_fillrect = cfb_fillrect, .fb_copyarea = cfb_copyarea, .fb_imageblit = cfb_imageblit, - .fb_pan_display = intel_fbdev_pan_display, - .fb_blank = intel_fbdev_blank, + .fb_dirty = intel_fbdev_dirty, + .fb_pan_display = drm_fb_helper_pan_display, + .fb_blank = drm_fb_helper_blank, .fb_setcmap = drm_fb_helper_setcmap, .fb_debug_enter = drm_fb_helper_debug_enter, .fb_debug_leave = drm_fb_helper_debug_leave,
Now that we have fb user dirty invalidating frontbuffer and we have the new fbdev dirty callback let's merge them. So it doesn't matter if fbcon throught fbdev or splash screen throught drm_ioctl_dirtyfb, in any case we will have frontbuffer properly invalidated and power savings features that rely on frontbuffer tracking will be able to work as expected. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_fbdev.c | 86 ++++++-------------------------------- 1 file changed, 13 insertions(+), 73 deletions(-)