From: Peilin Ye <yepeilin.cs@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Daniel Vetter <daniel@ffwll.ch>
Cc: Jiri Slaby <jirislaby@kernel.org>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org
Subject: Re: [PATCH v3 0/5] console: Miscellaneous clean-ups, do not use FNTCHARCNT() in fbcon.c
Date: Sat, 14 Nov 2020 03:10:21 -0500 [thread overview]
Message-ID: <20201114081021.GA11811@PWN> (raw)
In-Reply-To: <X68NFzaAuImemnqh@kroah.com>
> On Fri, Nov 13, 2020 at 10:16:33PM +0100, Daniel Vetter wrote:
> > On Thu, Nov 12, 2020 at 07:02:21AM -0500, Peilin Ye wrote:
> > > Peilin Ye (5):
> > > console: Delete unused con_font_copy() callback implementations
> > > console: Delete dummy con_font_set() and con_font_default() callback implementations
> > > Fonts: Add charcount field to font_desc
> > > parisc/sticore: Avoid hard-coding built-in font charcount
> > > fbcon: Avoid using FNTCHARCNT() and hard-coded built-in font charcount
> >
> > Patches all look good to me, if Greg is ok with me applying the entire
> > pile to drm-misc-next I'll do that next week.
On Fri, Nov 13, 2020 at 11:47:51PM +0100, Greg Kroah-Hartman wrote:
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Thanks for reviewing! Questions about the last patch [5/5] though, it
depends on the following assumption:
"""
For each console `idx`, `vc_cons[idx].d->vc_font.data` and
`fb_display[idx].fontdata` always point to the same buffer.
"""
Is this true? I think it is true by grepping for `fontdata`. I also
noticed that fbcon.c is using `vc->vc_font.data` and `p->fontdata`
interchangeably, see fbcon_get_requirement():
vc = vc_cons[fg_console].d;
[...]
p = &fb_display[fg_console];
caps->x = 1 << (vc->vc_font.width - 1);
^^^^^^^^^^^
caps->y = 1 << (vc->vc_font.height - 1);
^^^^^^^^^^^
caps->len = (p->userfont) ?
FNTCHARCNT(p->fontdata) : 256;
^^^^^^^^^^^
If it is true, then what is the point of using `fontdata` in `struct
fbcon_display`? Just for the `userfont` flag? Should we delete
`fontdata`, when we no longer need the `userfont` flag?
In this sense I think [5/5] needs more testing. Do we have test files
for fbcon, or should I try to write some tests from scratch?
Thank you,
Peilin Ye
WARNING: multiple messages have this Message-ID (diff)
From: Peilin Ye <yepeilin.cs@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Daniel Vetter <daniel@ffwll.ch>
Cc: linux-fbdev@vger.kernel.org, Jiri Slaby <jirislaby@kernel.org>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 0/5] console: Miscellaneous clean-ups, do not use FNTCHARCNT() in fbcon.c
Date: Sat, 14 Nov 2020 03:10:21 -0500 [thread overview]
Message-ID: <20201114081021.GA11811@PWN> (raw)
In-Reply-To: <X68NFzaAuImemnqh@kroah.com>
> On Fri, Nov 13, 2020 at 10:16:33PM +0100, Daniel Vetter wrote:
> > On Thu, Nov 12, 2020 at 07:02:21AM -0500, Peilin Ye wrote:
> > > Peilin Ye (5):
> > > console: Delete unused con_font_copy() callback implementations
> > > console: Delete dummy con_font_set() and con_font_default() callback implementations
> > > Fonts: Add charcount field to font_desc
> > > parisc/sticore: Avoid hard-coding built-in font charcount
> > > fbcon: Avoid using FNTCHARCNT() and hard-coded built-in font charcount
> >
> > Patches all look good to me, if Greg is ok with me applying the entire
> > pile to drm-misc-next I'll do that next week.
On Fri, Nov 13, 2020 at 11:47:51PM +0100, Greg Kroah-Hartman wrote:
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Thanks for reviewing! Questions about the last patch [5/5] though, it
depends on the following assumption:
"""
For each console `idx`, `vc_cons[idx].d->vc_font.data` and
`fb_display[idx].fontdata` always point to the same buffer.
"""
Is this true? I think it is true by grepping for `fontdata`. I also
noticed that fbcon.c is using `vc->vc_font.data` and `p->fontdata`
interchangeably, see fbcon_get_requirement():
vc = vc_cons[fg_console].d;
[...]
p = &fb_display[fg_console];
caps->x = 1 << (vc->vc_font.width - 1);
^^^^^^^^^^^
caps->y = 1 << (vc->vc_font.height - 1);
^^^^^^^^^^^
caps->len = (p->userfont) ?
FNTCHARCNT(p->fontdata) : 256;
^^^^^^^^^^^
If it is true, then what is the point of using `fontdata` in `struct
fbcon_display`? Just for the `userfont` flag? Should we delete
`fontdata`, when we no longer need the `userfont` flag?
In this sense I think [5/5] needs more testing. Do we have test files
for fbcon, or should I try to write some tests from scratch?
Thank you,
Peilin Ye
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-11-14 8:10 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-12 12:02 [PATCH v3 0/5] console: Miscellaneous clean-ups, do not use FNTCHARCNT() in fbcon.c Peilin Ye
2020-11-12 12:02 ` Peilin Ye
2020-11-12 12:04 ` [PATCH 1/5] console: Delete unused con_font_copy() callback implementations Peilin Ye
2020-11-12 12:04 ` Peilin Ye
2020-11-12 12:11 ` [PATCH v3 2/5] console: Delete dummy con_font_set() and con_font_default() " Peilin Ye
2020-11-12 12:11 ` Peilin Ye
2020-11-12 12:13 ` [PATCH v3 3/5] Fonts: Add charcount field to font_desc Peilin Ye
2020-11-12 12:13 ` Peilin Ye
2020-11-12 12:14 ` [PATCH v3 4/5] parisc/sticore: Avoid hard-coding built-in font charcount Peilin Ye
2020-11-12 12:14 ` Peilin Ye
2020-11-12 12:15 ` [PATCH v3 RFC 5/5] fbcon: Avoid using FNTCHARCNT() and hard-coded " Peilin Ye
2020-11-12 12:15 ` Peilin Ye
2020-11-13 21:16 ` [PATCH v3 0/5] console: Miscellaneous clean-ups, do not use FNTCHARCNT() in fbcon.c Daniel Vetter
2020-11-13 21:16 ` Daniel Vetter
2020-11-13 22:47 ` Greg Kroah-Hartman
2020-11-13 22:47 ` Greg Kroah-Hartman
2020-11-14 8:10 ` Peilin Ye [this message]
2020-11-14 8:10 ` Peilin Ye
2020-11-14 12:18 ` Greg Kroah-Hartman
2020-11-14 12:18 ` Greg Kroah-Hartman
2020-11-14 12:22 ` Greg Kroah-Hartman
2020-11-14 12:22 ` Greg Kroah-Hartman
2020-11-14 12:47 ` Peilin Ye
2020-11-14 12:47 ` Peilin Ye
2020-11-16 10:09 ` Daniel Vetter
2020-11-16 10:09 ` Daniel Vetter
2020-11-17 6:21 ` Peilin Ye
2020-11-17 6:21 ` Peilin Ye
2020-11-19 8:32 ` Peilin Ye
2020-11-19 8:32 ` Peilin Ye
2020-11-19 15:10 ` Daniel Vetter
2020-11-19 15:10 ` Daniel Vetter
2020-11-20 8:37 ` Peilin Ye
2020-11-20 8:37 ` Peilin Ye
2020-11-16 15:33 ` Daniel Vetter
2020-11-16 15:33 ` Daniel Vetter
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=20201114081021.GA11811@PWN \
--to=yepeilin.cs@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.