Re: [Intel-gfx] [PATCH] drm/i915/fbdev: Implement fb_dirty for intel custom fb helper

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

On Tue, 2022-12-20 at 19:58 +0200, Ville Syrjälä wrote:
> On Tue, Dec 20, 2022 at 07:53:06PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 29, 2022 at 02:43:02PM +0200, Jouni Högander wrote:
> > > After splitting generic drm_fb_helper into it's own file it's
> > > left to
> > > helper implementation to have fb_dirty function. Currently intel
> > > fb doesn't have it. This is causing problems when PSR is enabled.
> > > 
> > > Implement simple fb_dirty callback to deliver notifications to
> > > psr
> > > about updates in fb console.
> > 
> > Just found this regression myself after being baffled why the
> > vt console was inoperable right after the driver gets loaded.
> > 
> > It's also not just psr, but also fbc that is having issues.
> > 
> > Needs a fixes + cc:stable tags.
> 
> Actually looks like it didn't make it into 6.1 so I guess
> no cc:stable needed.

Added Fixes tag.

> 
> > 
> > > 
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_fbdev.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > index 5575d7abdc09..7c7fba3fe69e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > > @@ -328,8 +328,17 @@ 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)
> > > +{
> > 
> > The original thing had some kind of "is this rect actually
> > visible?"
> > check here. Does anyone know why it was there, and if so maybe it
> > should
> > go back to the higher level function so everyone doens't need to
> > add it
> > back in?

I'll guess you mean this one ?:

// snip
       /* Call damage handlers only if necessary */
       if (!(clip->x1 < clip->x2 && clip->y1 < clip->y2))
               return 0;
// snip

I will create separate patch for this.

> > 
> > > +       if (helper->fb->funcs->dirty)
> > > +               return helper->fb->funcs->dirty(helper->fb, NULL,
> > > 0, 0, clip, 1);
> > > +
> > > +       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)
> > > -- 
> > > 2.34.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 

BR,

Jouni Högander