Re: [Intel-gfx] [PATCH] drm/i915/fbdev: Implement wrappers for callbacks used by fbcon

["Hogander, Jouni" <jouni.hogander@intel.com>]

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@linux.intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> 
> 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?

> 
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> 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