Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Luis R. Rodriguez <hidden>
Date: 2015-03-28 00:21:54
Also in:
lkml
On Fri, Mar 27, 2015 at 11:56:55PM +0200, Ville Syrjälä wrote:
On Fri, Mar 27, 2015 at 08:57:59PM +0100, Luis R. Rodriguez wrote:quoted
On Fri, Mar 27, 2015 at 12:43:55PM -0700, Andy Lutomirski wrote:quoted
On Fri, Mar 27, 2015 at 12:38 PM, Luis R. Rodriguez [off-list ref] wrote:quoted
On Sat, Mar 21, 2015 at 11:15:14AM +0200, Ville Syrjälä wrote:quoted
On Fri, Mar 20, 2015 at 04:17:59PM -0700, Luis R. Rodriguez wrote:quoted
diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c index 8025624..8875e56 100644 --- a/drivers/video/fbdev/aty/atyfb_base.c +++ b/drivers/video/fbdev/aty/atyfb_base.c@@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info) #ifdef CONFIG_MTRR par->mtrr_aper = -1; - par->mtrr_reg = -1; if (!nomtrr) { - /* Cover the whole resource. */ - par->mtrr_aper = mtrr_add(par->res_start, par->res_size, + par->mtrr_aper = mtrr_add(info->fix.smem_start, + info->fix.smem_len, MTRR_TYPE_WRCOMB, 1);MTRRs need power of two size, so how is this supposed to work?As per mtrr_add_page() [0] the base and size are just supposed to be in units of 4 KiB, although the practice is to use powers of 2 in *some* drivers this is not standardized and by no means recorded as a requirement. Obviously powers of 2 will work too and you'd end up neatly aligned as well. mtrr_add() will use mtrr_check() to verify the the same requirement. Furthermore, as per my commit log message:Whatever the code may or may not do, the x86 architecture uses power-of-two MTRR sizes. So I'm confused.There should be no confusion, I simply did not know that *was* the requirement for x86, if that is the case we should add a check for that and perhaps generalize a helper that does the power of two helper changes, the cleanest I found was the vesafb driver solution. Thoughts?The vesafb solution is bad since you'll only end up covering only the first 4MB of the framebuffer instead of the almost 8MB you want.
OK so the power of 2 requirement implicates us *having* to use a large MTRR that includes the MMIo region in the shared PCI case? Andy, Ville, are we 100% certain about this power of two requirement? Is that for the base and size or just the size? Luis