Re: [PATCH 21/21] fbdev: Make registered_fb[] private to fbmem.c

[Daniel Vetter]

On Tue, Feb 08, 2022 at 08:00:38PM +0100, Sam Ravnborg wrote:
> Hi Daniel,
> 
> On Mon, Jan 31, 2022 at 10:05:52PM +0100, Daniel Vetter wrote:
> > Well except when the olpc dcon fbdev driver is enabled, that thing
> > digs around in there in rather unfixable ways.
> > 
> > Cc oldc_dcon maintainers as fyi.
> > 
> > Cc: Jens Frederich <jfrederich@xxxxxxxxx>
> > Cc: Jon Nettleton <jon.nettleton@xxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: linux-staging@xxxxxxxxxxxxxxx
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > Cc: Daniel Vetter <daniel@xxxxxxxx>
> > Cc: Helge Deller <deller@xxxxxx>
> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> > Cc: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
> > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > Cc: Xiyu Yang <xiyuyang19@xxxxxxxxxxxx>
> > Cc: linux-fbdev@xxxxxxxxxxxxxxx
> > Cc: Zheyu Ma <zheyuma97@xxxxxxxxx>
> > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> 
> with the build thingy fixed:
> Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> 
> I do wonder if there is a more clean way to trigger a blank
> in the main fbdev driver from the olpc driver.
> 
> The current hack is not nice and it would be good to see it gone.

Yeah this is just badly engineered. In drm we'd do this with the self
refresh helpers, which pretty much give you this exact functionality, but
in the helpers, while not randomly breaking actual visible behaviour of
the display driver.

Well ok the illusion is not perfect, since if the display is suspended the
next page flip will take a tad longer. But that's it.

I'll also add this to the TODO.
-Daniel

> 
> 	Sam
> 
> > ---
> >  drivers/video/fbdev/core/fbmem.c | 8 ++++++--
> >  include/linux/fb.h               | 7 +++----
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index 904ef1250677..dad6572942fa 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -48,10 +48,14 @@
> >  static DEFINE_MUTEX(registration_lock);
> >  
> >  struct fb_info *registered_fb[FB_MAX] __read_mostly;
> > -EXPORT_SYMBOL(registered_fb);
> > -
> >  int num_registered_fb __read_mostly;
> > +#if IS_ENABLED(CONFIG_OLPC_DCON)
> > +EXPORT_SYMBOL(registered_fb);
> >  EXPORT_SYMBOL(num_registered_fb);
> > +#endif
> > +#define for_each_registered_fb(i)		\
> > +	for (i = 0; i < FB_MAX; i++)		\
> > +		if (!registered_fb[i]) {} else
> >  
> >  bool fb_center_logo __read_mostly;
> >  
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index a8a00d2ba1f3..e236817502c2 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -622,16 +622,15 @@ extern int fb_get_color_depth(struct fb_var_screeninfo *var,
> >  extern int fb_get_options(const char *name, char **option);
> >  extern int fb_new_modelist(struct fb_info *info);
> >  
> > +#if IS_ENABLED(CONFIG_OLPC_DCON)
> >  extern struct fb_info *registered_fb[FB_MAX];
> > +
> >  extern int num_registered_fb;
> > +#endif
> >  extern bool fb_center_logo;
> >  extern int fb_logo_count;
> >  extern struct class *fb_class;
> >  
> > -#define for_each_registered_fb(i)		\
> > -	for (i = 0; i < FB_MAX; i++)		\
> > -		if (!registered_fb[i]) {} else
> > -
> >  static inline void lock_fb_info(struct fb_info *info)
> >  {
> >  	mutex_lock(&info->lock);
> > -- 
> > 2.33.0

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch