On Mon, 2023-01-23 at 14:20 +0200, Jouni Högander wrote:
> On Mon, 2023-01-23 at 12:38 +0100, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 23.01.23 um 08:44 schrieb Jouni Högander:
> > > After disconnecting damage worker from update logic it's left to
> > > fbdev
> > > emulation implementation to have fb_dirty function. Currently
> > > intel
> > > fbdev doesn't have it. This is causing problems to features (PSR,
> > > FBC,
> > > DRRS) relying on dirty callback.
> > >
> > > Implement simple fb_dirty callback to deliver notifications about
> > > updates
> > > in fb console.
> > >
> > > v4: Add proper Fixes tag and modify commit message
> > > v3: Check damage clip
> > > v2: Improved commit message and added Fixes tag
> > >
> > > 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>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_fbdev.c | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > index 19f3b5d92a55..d39db8050c69 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > @@ -321,8 +321,20 @@ static int intelfb_create(struct
> > > drm_fb_helper
> > > *helper,
> > > return ret;
> > > }
> > >
> > > +static int intelfb_dirty(struct drm_fb_helper *helper, struct
> > > drm_clip_rect *clip)
> > > +{
> > > + if (!(clip->x1 < clip->x2 && clip->y1 < clip->y2))
> > > + return 0;
> > > +
> > > + if (helper->fb->funcs->dirty)
> > > + return helper->fb->funcs->dirty(helper->fb, NULL,
> > > 0, 0, clip, 1);
> >
> > Didn't I nack this approach already? You shouldn't set fb_dirty.
> > The
> > better solution was to implement i915-specific helpers for write,
> > fillarea, copyarea and blit. Those should call intelfb_dirty()
> > directly
> > after performing the output. And IIRC you already implemented
> > this.
>
> I have implemented such thing. I didn't took it as a nack back then.
Meant to say "I haven't implemented such thing". Sorry for the typo.
> Last comment from you was:
>
> "if you go with fb_dirty, please implement the clipping
> test in your callback."
>
> and v3 was prepared to address that comment. My thinking was that
> this
> is only for fb console, but Ville Syrjälä commented that there is
> more
> than console so I sticked in this approach.
>
> So you think I should just drop idea of setting dirty callback and
> implement those i915-specific helpers?
>
> >
> > Best regards
> > Thomas
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct drm_fb_helper_funcs intel_fb_helper_funcs =
> > > {
> > > .fb_probe = intelfb_create,
> > > + .fb_dirty = intelfb_dirty,
> > > };
> > >
> > > static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> >
> > --
> > 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
>
