* [PATCH] fbuffer: improve toggle cursor performance
@ 2015-05-27 0:11 Greg Kurz
2015-05-27 5:11 ` Nikunj A Dadhania
2015-05-27 5:59 ` Thomas Huth
0 siblings, 2 replies; 6+ messages in thread
From: Greg Kurz @ 2015-05-27 0:11 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Nikunj A Dadhania, David Gibson
SLOF currently calls hv-logical-load and hv-logical-store for every pixel
when enabling or disabling the cursor. This is suboptimal when writing one
char at a time to the console since terminal-write always toggles the cursor.
And this is precisely what grub is doing when the user wants to edit a menu
entry... the result is an incredibly slow and barely usable interface.
The inner loop in fb8-toggle-cursor handles a contiguous region: it can be
converted to hv-logical-memop. The result is 32 times less hcalls per char
and a serious improvement in grub usability.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
slof/fs/fbuffer.fs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
index 756f05a..46b59bf 100644
--- a/slof/fs/fbuffer.fs
+++ b/slof/fs/fbuffer.fs
@@ -99,8 +99,8 @@ CREATE bitmap-buffer 400 4 * allot
: fb8-toggle-cursor ( -- )
line# fb8-line2addr column# fb8-columns2bytes +
char-height 0 ?DO
- char-width screen-depth * 0 ?DO dup dup rb@ -1 xor swap rb! 1+ LOOP
- screen-width screen-depth * + char-width screen-depth * -
+ dup dup 0 char-width screen-depth * 1 hv-logical-memop drop
+ screen-width screen-depth * +
LOOP drop
;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fbuffer: improve toggle cursor performance
2015-05-27 0:11 [PATCH] fbuffer: improve toggle cursor performance Greg Kurz
@ 2015-05-27 5:11 ` Nikunj A Dadhania
2015-05-27 9:01 ` Greg Kurz
2015-05-27 5:59 ` Thomas Huth
1 sibling, 1 reply; 6+ messages in thread
From: Nikunj A Dadhania @ 2015-05-27 5:11 UTC (permalink / raw)
To: Greg Kurz, linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson
Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> SLOF currently calls hv-logical-load and hv-logical-store for every pixel
> when enabling or disabling the cursor. This is suboptimal when writing one
> char at a time to the console since terminal-write always toggles the cursor.
> And this is precisely what grub is doing when the user wants to edit a menu
> entry... the result is an incredibly slow and barely usable interface.
>
> The inner loop in fb8-toggle-cursor handles a contiguous region: it can be
> converted to hv-logical-memop. The result is 32 times less hcalls per char
> and a serious improvement in grub usability.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> slof/fs/fbuffer.fs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
> index 756f05a..46b59bf 100644
> --- a/slof/fs/fbuffer.fs
> +++ b/slof/fs/fbuffer.fs
> @@ -99,8 +99,8 @@ CREATE bitmap-buffer 400 4 * allot
> : fb8-toggle-cursor ( -- )
> line# fb8-line2addr column# fb8-columns2bytes +
> char-height 0 ?DO
> - char-width screen-depth * 0 ?DO dup dup rb@ -1 xor swap rb! 1+ LOOP
> - screen-width screen-depth * + char-width screen-depth * -
> + dup dup 0 char-width screen-depth * 1 hv-logical-memop drop
> + screen-width screen-depth * +
Why did you drop "char-width screen-depth * -" in the new code? This is
not me mentioned in the description.
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fbuffer: improve toggle cursor performance
2015-05-27 5:11 ` Nikunj A Dadhania
@ 2015-05-27 9:01 ` Greg Kurz
2015-05-27 9:21 ` Nikunj A Dadhania
0 siblings, 1 reply; 6+ messages in thread
From: Greg Kurz @ 2015-05-27 9:01 UTC (permalink / raw)
To: Nikunj A Dadhania; +Cc: linuxppc-dev, Alexey Kardashevskiy, David Gibson
On Wed, 27 May 2015 10:41:06 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>
> > SLOF currently calls hv-logical-load and hv-logical-store for every pixel
> > when enabling or disabling the cursor. This is suboptimal when writing one
> > char at a time to the console since terminal-write always toggles the cursor.
> > And this is precisely what grub is doing when the user wants to edit a menu
> > entry... the result is an incredibly slow and barely usable interface.
> >
> > The inner loop in fb8-toggle-cursor handles a contiguous region: it can be
> > converted to hv-logical-memop. The result is 32 times less hcalls per char
> > and a serious improvement in grub usability.
> >
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> > slof/fs/fbuffer.fs | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
> > index 756f05a..46b59bf 100644
> > --- a/slof/fs/fbuffer.fs
> > +++ b/slof/fs/fbuffer.fs
> > @@ -99,8 +99,8 @@ CREATE bitmap-buffer 400 4 * allot
> > : fb8-toggle-cursor ( -- )
> > line# fb8-line2addr column# fb8-columns2bytes +
> > char-height 0 ?DO
> > - char-width screen-depth * 0 ?DO dup dup rb@ -1 xor swap rb! 1+ LOOP
> > - screen-width screen-depth * + char-width screen-depth * -
> > + dup dup 0 char-width screen-depth * 1 hv-logical-memop drop
> > + screen-width screen-depth * +
>
> Why did you drop "char-width screen-depth * -" in the new code? This is
> not me mentioned in the description.
>
This is because the current inner loop increments the address. When the loop
ends, we're pointing at the next char, that is char-width * screen-depth bytes
too far.
In the new code, the address is duped on the stack before calling hv-logical-memop,
so we don't need to fix it when proceeding to next line.
In my first attempt, I forgot to drop the subtraction and got an interesting
visual result :)
> Regards
> Nikunj
Cheers.
--
Greg
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fbuffer: improve toggle cursor performance
2015-05-27 9:01 ` Greg Kurz
@ 2015-05-27 9:21 ` Nikunj A Dadhania
0 siblings, 0 replies; 6+ messages in thread
From: Nikunj A Dadhania @ 2015-05-27 9:21 UTC (permalink / raw)
To: Greg Kurz; +Cc: linuxppc-dev, Alexey Kardashevskiy, David Gibson
Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> On Wed, 27 May 2015 10:41:06 +0530
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
>
>> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>>
>> > SLOF currently calls hv-logical-load and hv-logical-store for every pixel
>> > when enabling or disabling the cursor. This is suboptimal when writing one
>> > char at a time to the console since terminal-write always toggles the cursor.
>> > And this is precisely what grub is doing when the user wants to edit a menu
>> > entry... the result is an incredibly slow and barely usable interface.
>> >
>> > The inner loop in fb8-toggle-cursor handles a contiguous region: it can be
>> > converted to hv-logical-memop. The result is 32 times less hcalls per char
>> > and a serious improvement in grub usability.
>> >
>> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>> > ---
>> > slof/fs/fbuffer.fs | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
>> > index 756f05a..46b59bf 100644
>> > --- a/slof/fs/fbuffer.fs
>> > +++ b/slof/fs/fbuffer.fs
>> > @@ -99,8 +99,8 @@ CREATE bitmap-buffer 400 4 * allot
>> > : fb8-toggle-cursor ( -- )
>> > line# fb8-line2addr column# fb8-columns2bytes +
>> > char-height 0 ?DO
>> > - char-width screen-depth * 0 ?DO dup dup rb@ -1 xor swap rb! 1+ LOOP
>> > - screen-width screen-depth * + char-width screen-depth * -
>> > + dup dup 0 char-width screen-depth * 1 hv-logical-memop drop
>> > + screen-width screen-depth * +
>>
>> Why did you drop "char-width screen-depth * -" in the new code? This is
>> not me mentioned in the description.
>>
>
> This is because the current inner loop increments the address. When the loop
> ends, we're pointing at the next char, that is char-width * screen-depth bytes
> too far.
>
> In the new code, the address is duped on the stack before calling hv-logical-memop,
> so we don't need to fix it when proceeding to next line.
Ah ok, i missed that 1+ in the loop.
> In my first attempt, I forgot to drop the subtraction and got an interesting
> visual result :)
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fbuffer: improve toggle cursor performance
2015-05-27 0:11 [PATCH] fbuffer: improve toggle cursor performance Greg Kurz
2015-05-27 5:11 ` Nikunj A Dadhania
@ 2015-05-27 5:59 ` Thomas Huth
2015-05-27 9:24 ` Greg Kurz
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2015-05-27 5:59 UTC (permalink / raw)
To: Greg Kurz
Cc: linuxppc-dev, Alexey Kardashevskiy, Nikunj A Dadhania,
David Gibson
On Wed, 27 May 2015 02:11:13 +0200
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> SLOF currently calls hv-logical-load and hv-logical-store for every pixel
> when enabling or disabling the cursor. This is suboptimal when writing one
> char at a time to the console since terminal-write always toggles the cursor.
> And this is precisely what grub is doing when the user wants to edit a menu
> entry... the result is an incredibly slow and barely usable interface.
>
> The inner loop in fb8-toggle-cursor handles a contiguous region: it can be
> converted to hv-logical-memop. The result is 32 times less hcalls per char
> and a serious improvement in grub usability.
Good idea for an optimization!
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> slof/fs/fbuffer.fs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
> index 756f05a..46b59bf 100644
> --- a/slof/fs/fbuffer.fs
> +++ b/slof/fs/fbuffer.fs
> @@ -99,8 +99,8 @@ CREATE bitmap-buffer 400 4 * allot
> : fb8-toggle-cursor ( -- )
> line# fb8-line2addr column# fb8-columns2bytes +
> char-height 0 ?DO
> - char-width screen-depth * 0 ?DO dup dup rb@ -1 xor swap rb! 1+ LOOP
> - screen-width screen-depth * + char-width screen-depth * -
> + dup dup 0 char-width screen-depth * 1 hv-logical-memop drop
> + screen-width screen-depth * +
> LOOP drop
> ;
If you use hv-logical-memop in this file here, you definitely break
board-js2x, since this is bare metal and hv-logical-memop is not
defined there.
I think you should either move the new function to board-qemu and handle
it there like it is done for hcall-invert-screen already, or we could
think of introducing a helper function that is defined by each board
which does the xor operation on a memory region (that way we could
maybe also unify hcall-invert-screen and fb8-invert-screen again).
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fbuffer: improve toggle cursor performance
2015-05-27 5:59 ` Thomas Huth
@ 2015-05-27 9:24 ` Greg Kurz
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2015-05-27 9:24 UTC (permalink / raw)
To: Thomas Huth
Cc: linuxppc-dev, Alexey Kardashevskiy, Nikunj A Dadhania,
David Gibson
On Wed, 27 May 2015 07:59:34 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On Wed, 27 May 2015 02:11:13 +0200
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>
> > SLOF currently calls hv-logical-load and hv-logical-store for every pixel
> > when enabling or disabling the cursor. This is suboptimal when writing one
> > char at a time to the console since terminal-write always toggles the cursor.
> > And this is precisely what grub is doing when the user wants to edit a menu
> > entry... the result is an incredibly slow and barely usable interface.
> >
> > The inner loop in fb8-toggle-cursor handles a contiguous region: it can be
> > converted to hv-logical-memop. The result is 32 times less hcalls per char
> > and a serious improvement in grub usability.
>
> Good idea for an optimization!
>
Heh no big deal... the hardest part was to find that the LOAD/STORE avalanche
was coming from these rb@ and rb! words. SLOF is still a mysterious beast to
me :)
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> > slof/fs/fbuffer.fs | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/slof/fs/fbuffer.fs b/slof/fs/fbuffer.fs
> > index 756f05a..46b59bf 100644
> > --- a/slof/fs/fbuffer.fs
> > +++ b/slof/fs/fbuffer.fs
> > @@ -99,8 +99,8 @@ CREATE bitmap-buffer 400 4 * allot
> > : fb8-toggle-cursor ( -- )
> > line# fb8-line2addr column# fb8-columns2bytes +
> > char-height 0 ?DO
> > - char-width screen-depth * 0 ?DO dup dup rb@ -1 xor swap rb! 1+ LOOP
> > - screen-width screen-depth * + char-width screen-depth * -
> > + dup dup 0 char-width screen-depth * 1 hv-logical-memop drop
> > + screen-width screen-depth * +
> > LOOP drop
> > ;
>
> If you use hv-logical-memop in this file here, you definitely break
> board-js2x, since this is bare metal and hv-logical-memop is not
> defined there.
>
Of course, this is common code... I'll remember for next time. :)
> I think you should either move the new function to board-qemu and handle
> it there like it is done for hcall-invert-screen already, or we could
> think of introducing a helper function that is defined by each board
> which does the xor operation on a memory region (that way we could
> maybe also unify hcall-invert-screen and fb8-invert-screen again).
>
I guess the first proposal is the obvious fix. From there, we can
work out a patchset for the second proposal.
> Thomas
>
Cheers.
--
Greg
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-27 9:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-27 0:11 [PATCH] fbuffer: improve toggle cursor performance Greg Kurz
2015-05-27 5:11 ` Nikunj A Dadhania
2015-05-27 9:01 ` Greg Kurz
2015-05-27 9:21 ` Nikunj A Dadhania
2015-05-27 5:59 ` Thomas Huth
2015-05-27 9:24 ` Greg Kurz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).