From: Marcin S. <mar...@gm...> - 2008-09-21 16:00:38
|
[PATCH 1/2] vgacon: optimize scrolling [PATCH 2/2] vgacon: vgacon_scrolldelta simplification drivers/video/console/vgacon.c | 48 ++++++++++++++++++++-------------------- 1 files changed, 24 insertions(+), 24 deletions(-) |
From: Marcin S. <mar...@gm...> - 2008-09-21 16:00:57
|
Join multiple scr_memcpyw into 1-3 calls (usually 2). (benchmarked average speedup: 1%) Signed-off-by: Marcin Slusarz <mar...@gm...> Cc: Antonino Daplas <ad...@gm...> Cc: lin...@li... --- drivers/video/console/vgacon.c | 28 ++++++++++++++++------------ 1 files changed, 16 insertions(+), 12 deletions(-) diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c index bd1f57b..de4f66c 100644 --- a/drivers/video/console/vgacon.c +++ b/drivers/video/console/vgacon.c @@ -292,23 +292,27 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) d = (void *) c->vc_origin; s = (void *) c->vc_screenbuf; - while (count--) { - scr_memcpyw(d, vgacon_scrollback + soff, c->vc_size_row); - d += c->vc_size_row; - soff += c->vc_size_row; - - if (soff >= vgacon_scrollback_size) - soff = 0; + if (count) { + int copysize; + count *= c->vc_size_row; + /* how much memory to end of buffer left? */ + copysize = min(count, vgacon_scrollback_size - soff); + scr_memcpyw(d, vgacon_scrollback + soff, copysize); + d += copysize; + count -= copysize; + + if (count) { + copysize = min(count, vgacon_scrollback_size); + scr_memcpyw(d, vgacon_scrollback, copysize); + d += copysize; + } } if (diff == c->vc_rows) { vgacon_cursor(c, CM_MOVE); } else { - while (diff--) { - scr_memcpyw(d, s, c->vc_size_row); - d += c->vc_size_row; - s += c->vc_size_row; - } + if (diff) + scr_memcpyw(d, s, diff * c->vc_size_row); } return 1; -- 1.5.6.4 |
From: Marcin S. <mar...@gm...> - 2008-09-21 16:01:20
|
There's no point in checking diff == c->vc_rows, because it can be true only when count == 0, but we already checked that. Additionally move variables used only in one block to this block. Signed-off-by: Marcin Slusarz <mar...@gm...> Cc: Antonino Daplas <ad...@gm...> Cc: lin...@li... --- drivers/video/console/vgacon.c | 22 +++++++++------------- 1 files changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c index de4f66c..a785f99 100644 --- a/drivers/video/console/vgacon.c +++ b/drivers/video/console/vgacon.c @@ -239,8 +239,7 @@ static void vgacon_restore_screen(struct vc_data *c) static int vgacon_scrolldelta(struct vc_data *c, int lines) { - int start, end, count, soff, diff; - void *d, *s; + int start, end, count, soff; if (!lines) { c->vc_visible_origin = c->vc_origin; @@ -287,13 +286,13 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) if (count > c->vc_rows) count = c->vc_rows; - diff = c->vc_rows - count; - - d = (void *) c->vc_origin; - s = (void *) c->vc_screenbuf; - if (count) { int copysize; + + int diff = c->vc_rows - count; + void *d = (void *) c->vc_origin; + void *s = (void *) c->vc_screenbuf; + count *= c->vc_size_row; /* how much memory to end of buffer left? */ copysize = min(count, vgacon_scrollback_size - soff); @@ -306,14 +305,11 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) scr_memcpyw(d, vgacon_scrollback, copysize); d += copysize; } - } - - if (diff == c->vc_rows) { - vgacon_cursor(c, CM_MOVE); - } else { + if (diff) scr_memcpyw(d, s, diff * c->vc_size_row); - } + } else + vgacon_cursor(c, CM_MOVE); return 1; } -- 1.5.6.4 |
From: Krzysztof H. <krz...@po...> - 2008-09-24 17:40:51
|
On Sun, 21 Sep 2008 18:00:09 +0200 Marcin Slusarz <mar...@gm...> wrote: > There's no point in checking diff == c->vc_rows, because it can be true > only when count == 0, but we already checked that. > Additionally move variables used only in one block to this block. > > Signed-off-by: Marcin Slusarz <mar...@gm...> > Cc: Antonino Daplas <ad...@gm...> > Cc: lin...@li... > --- Acked-by: Krzysztof Helt <krz...@wp...> > drivers/video/console/vgacon.c | 22 +++++++++------------- > 1 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > index de4f66c..a785f99 100644 > --- a/drivers/video/console/vgacon.c > +++ b/drivers/video/console/vgacon.c > @@ -239,8 +239,7 @@ static void vgacon_restore_screen(struct vc_data *c) > > static int vgacon_scrolldelta(struct vc_data *c, int lines) > { > - int start, end, count, soff, diff; > - void *d, *s; > + int start, end, count, soff; > > if (!lines) { > c->vc_visible_origin = c->vc_origin; > @@ -287,13 +286,13 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) > if (count > c->vc_rows) > count = c->vc_rows; > > - diff = c->vc_rows - count; > - > - d = (void *) c->vc_origin; > - s = (void *) c->vc_screenbuf; > - > if (count) { > int copysize; > + > + int diff = c->vc_rows - count; > + void *d = (void *) c->vc_origin; > + void *s = (void *) c->vc_screenbuf; > + > count *= c->vc_size_row; > /* how much memory to end of buffer left? */ > copysize = min(count, vgacon_scrollback_size - soff); > @@ -306,14 +305,11 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) > scr_memcpyw(d, vgacon_scrollback, copysize); > d += copysize; > } > - } > - > - if (diff == c->vc_rows) { > - vgacon_cursor(c, CM_MOVE); > - } else { > + > if (diff) > scr_memcpyw(d, s, diff * c->vc_size_row); > - } > + } else > + vgacon_cursor(c, CM_MOVE); > > return 1; > } > -- > 1.5.6.4 > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > Linux-fbdev-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel > ---------------------------------------------------------------------- Znajd¼ mieszkanie w Twoim regionie! kliknij >>> http://link.interia.pl/f1f19 |
From: Krzysztof H. <krz...@po...> - 2008-09-21 18:37:15
|
On Sun, 21 Sep 2008 18:00:08 +0200 Marcin Slusarz <mar...@gm...> wrote: > Join multiple scr_memcpyw into 1-3 calls (usually 2). > (benchmarked average speedup: 1%) > The scr_memcpyw is an inline function so there is no real call here. However, have you tested the patch with scroll range > 2 * vgacon_scrollback_size? It seems that your patch only handles scrolling if it is not bigger than 2 * vgacon_scrollback_size. Regards, Krzysztof > Signed-off-by: Marcin Slusarz <mar...@gm...> > Cc: Antonino Daplas <ad...@gm...> > Cc: lin...@li... > --- > drivers/video/console/vgacon.c | 28 ++++++++++++++++------------ > 1 files changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > index bd1f57b..de4f66c 100644 > --- a/drivers/video/console/vgacon.c > +++ b/drivers/video/console/vgacon.c > @@ -292,23 +292,27 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) > d = (void *) c->vc_origin; > s = (void *) c->vc_screenbuf; > > - while (count--) { > - scr_memcpyw(d, vgacon_scrollback + soff, c->vc_size_row); > - d += c->vc_size_row; > - soff += c->vc_size_row; > - > - if (soff >= vgacon_scrollback_size) > - soff = 0; > + if (count) { > + int copysize; > + count *= c->vc_size_row; > + /* how much memory to end of buffer left? */ > + copysize = min(count, vgacon_scrollback_size - soff); > + scr_memcpyw(d, vgacon_scrollback + soff, copysize); > + d += copysize; > + count -= copysize; > + > + if (count) { > + copysize = min(count, vgacon_scrollback_size); > + scr_memcpyw(d, vgacon_scrollback, copysize); > + d += copysize; > + } > } > > if (diff == c->vc_rows) { > vgacon_cursor(c, CM_MOVE); > } else { > - while (diff--) { > - scr_memcpyw(d, s, c->vc_size_row); > - d += c->vc_size_row; > - s += c->vc_size_row; > - } > + if (diff) > + scr_memcpyw(d, s, diff * c->vc_size_row); > } > > return 1; > -- > 1.5.6.4 > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > Linux-fbdev-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel > ---------------------------------------------------------------------- Kredyt Hipoteczny w Banku Millennium - zdobywca Zlotego Lauru Klienta! Sprawdz >> http://link.interia.pl/f1f15 |
From: Marcin S. <mar...@gm...> - 2008-09-21 21:23:09
|
On Sun, Sep 21, 2008 at 08:37:57PM +0200, Krzysztof Helt wrote: > On Sun, 21 Sep 2008 18:00:08 +0200 > Marcin Slusarz <mar...@gm...> wrote: > > > Join multiple scr_memcpyw into 1-3 calls (usually 2). > > (benchmarked average speedup: 1%) > > > > The scr_memcpyw is an inline function so there is no real call here. ... or it's defined to memcpy > However, have you tested the patch with scroll range > 2 * vgacon_scrollback_size? > > It seems that your patch only handles scrolling if it is not bigger than 2 * vgacon_scrollback_size. No. count is always within range <0, c->vc_rows> (look how it's computed just before this loop). Second copy is needed only when end of screen crosses end/beginning of scrollback (it's a circular buffer). > Regards, > Krzysztof > > > Signed-off-by: Marcin Slusarz <mar...@gm...> > > Cc: Antonino Daplas <ad...@gm...> > > Cc: lin...@li... > > --- > > drivers/video/console/vgacon.c | 28 ++++++++++++++++------------ > > 1 files changed, 16 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > > index bd1f57b..de4f66c 100644 > > --- a/drivers/video/console/vgacon.c > > +++ b/drivers/video/console/vgacon.c > > @@ -292,23 +292,27 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) > > d = (void *) c->vc_origin; > > s = (void *) c->vc_screenbuf; > > > > - while (count--) { > > - scr_memcpyw(d, vgacon_scrollback + soff, c->vc_size_row); > > - d += c->vc_size_row; > > - soff += c->vc_size_row; > > - > > - if (soff >= vgacon_scrollback_size) > > - soff = 0; > > + if (count) { > > + int copysize; > > + count *= c->vc_size_row; > > + /* how much memory to end of buffer left? */ > > + copysize = min(count, vgacon_scrollback_size - soff); > > + scr_memcpyw(d, vgacon_scrollback + soff, copysize); > > + d += copysize; > > + count -= copysize; > > + > > + if (count) { > > + copysize = min(count, vgacon_scrollback_size); > > + scr_memcpyw(d, vgacon_scrollback, copysize); > > + d += copysize; > > + } > > } > > > > if (diff == c->vc_rows) { > > vgacon_cursor(c, CM_MOVE); > > } else { > > - while (diff--) { > > - scr_memcpyw(d, s, c->vc_size_row); > > - d += c->vc_size_row; > > - s += c->vc_size_row; > > - } > > + if (diff) > > + scr_memcpyw(d, s, diff * c->vc_size_row); > > } > > > > return 1; > > -- > > 1.5.6.4 |
From: Krzysztof H. <krz...@po...> - 2008-09-23 21:58:33
|
On Sun, 21 Sep 2008 23:22:17 +0200 Marcin Slusarz <mar...@gm...> wrote: > On Sun, Sep 21, 2008 at 08:37:57PM +0200, Krzysztof Helt wrote: > > On Sun, 21 Sep 2008 18:00:08 +0200 > > Marcin Slusarz <mar...@gm...> wrote: > > > > > Join multiple scr_memcpyw into 1-3 calls (usually 2). > > > (benchmarked average speedup: 1%) > > > > > > > The scr_memcpyw is an inline function so there is no real call here. > > ... or it's defined to memcpy > > > However, have you tested the patch with scroll range > 2 * vgacon_scrollback_size? > > > > It seems that your patch only handles scrolling if it is not bigger than 2 * vgacon_scrollback_size. > > No. count is always within range <0, c->vc_rows> (look how it's computed > just before this loop). Second copy is needed only when end of screen > crosses end/beginning of scrollback (it's a circular buffer). > This is not what I asked for. If the vgacon_scrollback_cnt is smaller then c->vc_rows it can happen. The answer to my question is that count is always < vgacon_scrollback_cnt (set earlier before checking if it is in the <0,c->vc_rows> range). (...) > > > + if (count) { > > > + int copysize; > > > + count *= c->vc_size_row; > > > + /* how much memory to end of buffer left? */ > > > + copysize = min(count, vgacon_scrollback_size - soff); > > > + scr_memcpyw(d, vgacon_scrollback + soff, copysize); > > > + d += copysize; > > > + count -= copysize; > > > + > > > + if (count) { > > > + copysize = min(count, vgacon_scrollback_size); This line I got confused by. If the count is always smaller then the vgacon_scrollback_cnt before (thus the vgacon_scrollback_size inside the if clause) this line always evaluates to copysize = count here. I thought something more sophisticated was done here. Regards, Krzysztof ---------------------------------------------------------------------- Znajd¼ mieszkanie w Twoim regionie! kliknij >>> http://link.interia.pl/f1f19 |
From: Marcin S. <mar...@gm...> - 2008-09-24 19:41:00
|
On Tue, Sep 23, 2008 at 11:59:18PM +0200, Krzysztof Helt wrote: > On Sun, 21 Sep 2008 23:22:17 +0200 > Marcin Slusarz <mar...@gm...> wrote: > > > On Sun, Sep 21, 2008 at 08:37:57PM +0200, Krzysztof Helt wrote: > > > On Sun, 21 Sep 2008 18:00:08 +0200 > > > Marcin Slusarz <mar...@gm...> wrote: > > > > > > > Join multiple scr_memcpyw into 1-3 calls (usually 2). > > > > (benchmarked average speedup: 1%) > > > > > > > > > > The scr_memcpyw is an inline function so there is no real call here. > > > > ... or it's defined to memcpy > > > > > However, have you tested the patch with scroll range > 2 * vgacon_scrollback_size? > > > > > > It seems that your patch only handles scrolling if it is not bigger than 2 * vgacon_scrollback_size. > > > > No. count is always within range <0, c->vc_rows> (look how it's computed > > just before this loop). Second copy is needed only when end of screen > > crosses end/beginning of scrollback (it's a circular buffer). > > > > This is not what I asked for. If the vgacon_scrollback_cnt is smaller then c->vc_rows it can happen. > The answer to my question is that count is always < vgacon_scrollback_cnt (set earlier before > checking if it is in the <0,c->vc_rows> range). Yep > (...) > > > > > + if (count) { > > > > + int copysize; > > > > + count *= c->vc_size_row; > > > > + /* how much memory to end of buffer left? */ > > > > + copysize = min(count, vgacon_scrollback_size - soff); > > > > + scr_memcpyw(d, vgacon_scrollback + soff, copysize); > > > > + d += copysize; > > > > + count -= copysize; > > > > + > > > > + if (count) { > > > > + copysize = min(count, vgacon_scrollback_size); > > This line I got confused by. If the count is always smaller then > the vgacon_scrollback_cnt before (thus the vgacon_scrollback_size > inside the if clause) this line always evaluates to copysize = count here. You are right. I dropped this "call". Thanks for a review. --- Subject: [PATCH 1/2] vgacon: optimize scrolling Join multiple scr_memcpyw into 1-3 calls (usually 2). (benchmarked average speedup: 1%) Signed-off-by: Marcin Slusarz <mar...@gm...> Cc: Krzysztof Helt <krz...@po...> Cc: Antonino Daplas <ad...@gm...> Cc: lin...@li... --- drivers/video/console/vgacon.c | 27 +++++++++++++++------------ 1 files changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c index bd1f57b..29d1209 100644 --- a/drivers/video/console/vgacon.c +++ b/drivers/video/console/vgacon.c @@ -292,23 +292,26 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines) d = (void *) c->vc_origin; s = (void *) c->vc_screenbuf; - while (count--) { - scr_memcpyw(d, vgacon_scrollback + soff, c->vc_size_row); - d += c->vc_size_row; - soff += c->vc_size_row; - - if (soff >= vgacon_scrollback_size) - soff = 0; + if (count) { + int copysize; + count *= c->vc_size_row; + /* how much memory to end of buffer left? */ + copysize = min(count, vgacon_scrollback_size - soff); + scr_memcpyw(d, vgacon_scrollback + soff, copysize); + d += copysize; + count -= copysize; + + if (count) { + scr_memcpyw(d, vgacon_scrollback, count); + d += count; + } } if (diff == c->vc_rows) { vgacon_cursor(c, CM_MOVE); } else { - while (diff--) { - scr_memcpyw(d, s, c->vc_size_row); - d += c->vc_size_row; - s += c->vc_size_row; - } + if (diff) + scr_memcpyw(d, s, diff * c->vc_size_row); } return 1; -- 1.5.6.4 |