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

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

Here is the original patch by me:

https://patchwork.freedesktop.org/series/111433/

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