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.
>
> 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.
>
> >
> > 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, const char
> > > __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
>
--
Ville Syrjälä
Intel
