Hi Am 03.02.23 um 12:09 schrieb Hogander, Jouni:
On Fri, 2023-02-03 at 12:42 +0200, Ville Syrjälä wrote:On Fri, Feb 03, 2023 at 07:21:27AM +0000, Hogander, Jouni wrote:On Tue, 2023-01-24 at 13:27 +0100, Thomas Zimmermann wrote:Hi Am 24.01.23 um 10:10 schrieb Jouni Högander:After disconnecting damage worker from update logic our dirty callback is not called on fbcon events. This is causing problems to features (PSR, FBC, DRRS) relying on dirty callback getting called and breaking fb console when these features are in use. Implement wrappers for callbacks used by fbcon and call our dirty callback in those. Fixes: f231af498c29 ("drm/fb-helper: Disconnect damage worker from update logic") Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> Cc: Jani Nikula <jani.nikula@xxxxxxxxx> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>This is the better solution wrt what fbdev wants.There was a failure from testing robot. drivers/tty/vt/vt.c is using spinlock and in our dirty callback we are taking mutex.
I think I've seen this problem before in the context of vc4. We'd probably have to add a damage worker to i915, which seems a bit much for this fix.
Do you have any suggestions? Shall we fallback to original fix which was setting the dirty callback where call is made from workqueue?Please just fix the original regression as straightforwardly as possible.Here is the original patch by me: https://patchwork.freedesktop.org/series/111433/
Given the circumstances, Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>for this older patch. Maybe add a TODO comment that says that the fb_dirty worker needs to be moved into i915 and that the fb_ write ops need to be reimplemented for a good solution.
Best regards Thomas
which was eventually nacked by Thomas. Maybe we could continue with that one and solve these problems when/if fb_dirty is removed?Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> Best regards Thomas--- drivers/gpu/drm/i915/display/intel_fbdev.c | 53 ++++++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 19f3b5d92a55..b1653624552e 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -77,6 +77,18 @@ static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev) intel_frontbuffer_invalidate(to_frontbuffer(ifbdev), ORIGIN_CPU); }+static void intel_fbdev_dirty(struct fb_info *info)+{ + struct drm_fb_helper *helper = info->par; + + /* + * Intel_fb dirty implementation doesn't use damage clips -+ * no need to pass them here + */ + if (helper->fb->funcs->dirty) + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, NULL, 0); +} + static int intel_fbdev_set_par(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; @@ -91,6 +103,39 @@ static int intel_fbdev_set_par(struct fb_info *info) return ret; }+static ssize_t intel_fbdev_write(struct fb_info *info, constchar __user *buf, + size_t count, loff_t *ppos) +{ + int ret; + + ret = drm_fb_helper_cfb_write(info, buf, count, ppos); + if (ret > 0) + intel_fbdev_dirty(info); + + return ret; +} + +static void intel_fbdev_fillrect(struct fb_info *info, + const struct fb_fillrect *rect) +{ + drm_fb_helper_cfb_fillrect(info, rect); + intel_fbdev_dirty(info); +} + +static void intel_fbdev_copyarea(struct fb_info *info, + const struct fb_copyarea *area) +{ + drm_fb_helper_cfb_copyarea(info, area); + intel_fbdev_dirty(info); +} + +static void intel_fbdev_imageblit(struct fb_info *info, + const struct fb_image *image) +{ + drm_fb_helper_cfb_imageblit(info, image); + intel_fbdev_dirty(info); +} + static int intel_fbdev_blank(int blank, struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; @@ -125,10 +170,10 @@ static const struct fb_ops intelfb_ops = { DRM_FB_HELPER_DEFAULT_OPS, .fb_set_par = intel_fbdev_set_par, .fb_read = drm_fb_helper_cfb_read, - .fb_write = drm_fb_helper_cfb_write, - .fb_fillrect = drm_fb_helper_cfb_fillrect, - .fb_copyarea = drm_fb_helper_cfb_copyarea, - .fb_imageblit = drm_fb_helper_cfb_imageblit, + .fb_write = intel_fbdev_write, + .fb_fillrect = intel_fbdev_fillrect, + .fb_copyarea = intel_fbdev_copyarea, + .fb_imageblit = intel_fbdev_imageblit, .fb_pan_display = intel_fbdev_pan_display, .fb_blank = intel_fbdev_blank, };-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
