2003-08-25 19:43:11

by Arvind Sankar

[permalink] [raw]
Subject: vesafb mtrr setup question

The following lines are pulled out of drivers/video/vesafb.c (2.4.20):
Line 646 onwards..

> if (mtrr) {
> int temp_size = video_size;
> /* Find the largest power-of-two */
> while (temp_size & (temp_size - 1))
> temp_size &= (temp_size - 1);
>
In the first place, the power of two computation computes the largest
power of 2 that is _smaller_ than video_size, so it looks like an
off-by-1 bug.

> /* Try and find a power of two to add */
> while (temp_size && mtrr_add(video_base, temp_size, MTRR_TYPE_WRCOMB, 1)==-EINVAL) {
> temp_size >>= 1;
> }
> }

Secondly, what's the point of requesting a smaller write-combining
segment that won't cover all the video memory being used? If it fails
the first time round, shouldn't we either give up or attempt requesting
several contiguous segments?

-- arvind


2003-08-26 15:43:06

by Alan

[permalink] [raw]
Subject: Re: vesafb mtrr setup question

On Llu, 2003-08-25 at 20:43, Arvind Sankar wrote:
> In the first place, the power of two computation computes the largest
> power of 2 that is _smaller_ than video_size, so it looks like an
> off-by-1 bug.

Not a bug - we don't know what lives above it so we can't extend the
mtrr safely

> > /* Try and find a power of two to add */
> > while (temp_size && mtrr_add(video_base, temp_size, MTRR_TYPE_WRCOMB, 1)==-EINVAL) {
> > temp_size >>= 1;
> > }
> > }
>
> Secondly, what's the point of requesting a smaller write-combining
> segment that won't cover all the video memory being used?

Generally we don't use all the videoram. Its a heuristic rather than
perfection. You might want to play with improvements

2003-08-26 18:39:18

by Arvind Sankar

[permalink] [raw]
Subject: Re: vesafb mtrr setup question

On Tue, Aug 26, 2003 at 04:42:23PM +0100, Alan Cox wrote:
> On Llu, 2003-08-25 at 20:43, Arvind Sankar wrote:
> > In the first place, the power of two computation computes the largest
> > power of 2 that is _smaller_ than video_size, so it looks like an
> > off-by-1 bug.
>
> Not a bug - we don't know what lives above it so we can't extend the
> mtrr safely
>
Ah. On a side not, could you drop a quick hint as to how
screen_info.lfb_size is obtained?

In older (or just different?) versions of vesafb, the video_size was
actually computed by multiplying xres, yres, and the bpp.

> > > /* Try and find a power of two to add */
> > > while (temp_size && mtrr_add(video_base, temp_size, MTRR_TYPE_WRCOMB, 1)==-EINVAL) {
> > > temp_size >>= 1;
> > > }
> > > }
> >
> > Secondly, what's the point of requesting a smaller write-combining
> > segment that won't cover all the video memory being used?
>
> Generally we don't use all the videoram. Its a heuristic rather than
> perfection. You might want to play with improvements
>

I thought the yres_virtual was computed based on how much video_ram was
being used, so all of it _is_ being used, for scrollback?

-- arvind

2003-08-27 15:15:37

by Alan

[permalink] [raw]
Subject: Re: vesafb mtrr setup question

On Maw, 2003-08-26 at 16:50, Arvind Sankar wrote:
> Ah. On a side not, could you drop a quick hint as to how
> screen_info.lfb_size is obtained?
>
> In older (or just different?) versions of vesafb, the video_size was
> actually computed by multiplying xres, yres, and the bpp.

The BIOS will provide the size of the LFB. In 2.4 we now work out the
size we need for a sensible amount of scrollback and the like because
the LFB size may be huge (256Mb for example) and we don't want to blow
all our kernel address space on a useless mapping