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