Re: [RFC] WIP: fbconsole: very dirty font selection via param_enum

[Antony Pavlov <antonynpavlov@gmail.com>]

On Tue, 14 Jul 2015 07:27:56 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Mon, Jul 13, 2015 at 11:28:09PM +0300, Antony Pavlov wrote:
> > Alas I can't find any tab-complition for param_enum,
> > so addition 'fonts' command is introduced.
> > 
> > Usage example:
> > ==============
> > 
> >     barebox@barebox sandbox:/ fonts
> >     VGA8x16
> >     MINI4x6
> >     barebox@barebox sandbox:/ fbconsole0.font=MINI4x6
> >     barebox@barebox sandbox:/ fbconsole0.active=o
> >     fb0: framebuffer console 160x80 activated
> >     barebox@barebox sandbox:/ fbconsole0.font=VGA8x16
> > 
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  drivers/video/fbconsole.c | 55 +++++++++++++++++++++++++++++++++++++++++------
> >  include/linux/font.h      |  8 +++++++
> >  lib/fonts/fonts.c         | 44 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 100 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> > index 36fd138..43a73d9 100644
> > --- a/drivers/video/fbconsole.c
> > +++ b/drivers/video/fbconsole.c
> > @@ -20,6 +20,10 @@ struct fbc_priv {
> >  	struct fb_info *fb;
> >  
> >  	struct screen *sc;
> > +
> > +	struct param_d *par_font;
> > +	int par_font_val;
> > +
> >  /* FIXME */
> >  #define VIDEO_FONT_CHARS 256
> >  	struct image *chars[VIDEO_FONT_CHARS];
> > @@ -47,6 +51,16 @@ static int fbc_tstc(struct console_device *cdev)
> >  	return 0;
> >  }
> >  
> > +static void cls(struct fbc_priv *priv)
> > +{
> > +	void *buf = gui_screen_render_buffer(priv->sc);
> > +
> > +	memset(buf, 0, priv->fb->line_length * priv->fb->yres);
> > +
> > +	priv->x = 0;
> > +	priv->y = 0;
> > +}
> > +
> >  static void drawchar(struct fbc_priv *priv, int x, int y, char c)
> >  {
> >  	void *buf;
> > @@ -173,12 +187,7 @@ static void fbc_parse_csi(struct fbc_priv *priv)
> >  	}
> >  
> >  	if (*end == 'J' && a == 2 && b == -1) {
> > -		void *buf = gui_screen_render_buffer(priv->sc);
> > -
> > -		memset(buf, 0, priv->fb->line_length * priv->fb->yres);
> > -
> > -		priv->x = 0;
> > -		priv->y = 0;
> > +		cls(priv);
> >  		video_invertchar(priv, priv->x, priv->y);
> >  	}
> >  
> > @@ -270,7 +279,7 @@ static int fbc_set_active(struct console_device *cdev, unsigned flags)
> >  	struct fb_info *fb = priv->fb;
> >  	const struct font_desc *font;
> >  
> > -	font = find_font("MINI4x6");
> > +	font = find_font(font_by_param_value(priv->par_font_val));
> 
> This find_font(font_by_param_value()) doesn't look very nice. I think
> there should be a const struct font_desc *find_font_enum(int); or
> similar which doesn't take the detour via the font name.

I have fixed this in the RFCv2 patch.
Also I have dropped unused find_font() so some patch splitting&rebasing is necessary.


> > +	if (cdev->f_active & CONSOLE_STDOUT) {
> 
> (CONSOLE_STDOUT | CONSOLE_ERR)

fixed in RFCv2.
> 
> > +		struct fb_info *fb = priv->fb;
> > +		const struct font_desc *font;
> > +
> > +		cls(priv);
> > +
> > +		font = find_font(font_by_param_value(priv->par_font_val));
> > +
> > +		/* FIXME: code duplication */
> > +		priv->font_width = font->width;
> > +		priv->font_height = font->height;
> > +		priv->fontdata = font->data;
> > +
> > +		priv->rows = fb->yres / priv->font_height - 1;
> > +		priv->cols = fb->xres / priv->font_width - 1;
> > +
> > +		priv->state = LIT;
> 
> Is this correct? If we are in the middle of an escape sequence, why
> should we leave it here?

I can't imagine this situation.
I have dropped it in RFCv2 anyway.

> > +struct param_d *add_param_font(struct device_d *dev,
> > +		int (*set)(struct param_d *p, void *priv),
> > +		int (*get)(struct param_d *p, void *priv),
> > +		int *value, void *priv)
> > +{
> > +	unsigned int i;
> > +	int t = num_fonts;
> > +
> 
> 	if (!font_names) {
> > +	font_names = xmalloc(sizeof(char *) * t);
> > +
> > +	for (i = 0; i < num_fonts; i++)
> > +		((const char **)font_names)[i] = fonts[i]->name;
> 	}

I have planned to add separate initcall for font_names initialization,
but font_names initialization in add_parm_font() looks better:
  * it is simplier;
  * it runs "on-demand" only.

I have fixed this in RFCv2 too.

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox