From: Antony Pavlov <antonynpavlov@gmail.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [RFC] WIP: fbconsole: very dirty font selection via param_enum
Date: Tue, 14 Jul 2015 13:01:43 +0300 [thread overview]
Message-ID: <20150714130143.d42ee95d3b57eb44443fd94a@gmail.com> (raw)
In-Reply-To: <20150714052756.GP18700@pengutronix.de>
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
prev parent reply other threads:[~2015-07-14 9:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-13 20:28 Antony Pavlov
2015-07-14 4:42 ` Sascha Hauer
2015-07-14 9:56 ` Antony Pavlov
2015-07-14 5:27 ` Sascha Hauer
2015-07-14 10:01 ` Antony Pavlov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150714130143.d42ee95d3b57eb44443fd94a@gmail.com \
--to=antonynpavlov@gmail.com \
--cc=barebox@lists.infradead.org \
--cc=s.hauer@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox