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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@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?

Please just fix the original regression as straightforwardly as
possible.

> 
> > 
> > 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
> 

-- 
Ville Syrjälä
Intel