* Re: [PATCH v4 3/3] video: fbdev: vesafb: use arch_phys_wc_add()
From: Tomi Valkeinen @ 2015-06-12 9:04 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: plagnioj, bp, hpa, linux-fbdev, luto, toshi.kani, sbsiddha, mingo,
tglx, jgross, daniel.vetter, airlied, adaplas, robdclark, jg1.han,
wsa, linux-kernel, Luis R. Rodriguez
In-Reply-To: <1433436288-16792-4-git-send-email-mcgrof@do-not-panic.com>
[-- Attachment #1: Type: text/plain, Size: 5383 bytes --]
On 04/06/15 19:44, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> This driver uses the same area for MTRR as for the ioremap_wc(), if
> anything it just uses a smaller size in case MTRR reservation fails.
> ioremap_wc() API is already used to take advantage of architecture
> write-combining when available.
>
> Convert the driver from using the x86 specific MTRR code to
> the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
> will avoid MTRR if write-combining is available.
>
> There are a few motivations for this:
>
> a) Take advantage of PAT when available
>
> b) Help bury MTRR code away, MTRR is architecture specific and on
> x86 its replaced by PAT
>
> c) Help with the goal of eventually using _PAGE_CACHE_UC over
> _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
> de33c442e titled "x86 PAT: fix performance drop for glx,
> use UC minus for ioremap(), ioremap_nocache() and
> pci_mmap_page_range()")
>
> The conversion done is expressed by the following Coccinelle
> SmPL patch, it additionally required manual intervention to
> address all the #ifdery and removal of redundant things which
> arch_phys_wc_add() already addresses such as verbose message
> about when MTRR fails and doing nothing when we didn't get
> an MTRR.
>
> @ mtrr_found @
> expression index, base, size;
> @@
>
> -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> +index = arch_phys_wc_add(base, size);
>
> @ mtrr_rm depends on mtrr_found @
> expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> @@
>
> -mtrr_del(index, base, size);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_zero_arg depends on mtrr_found @
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, 0, 0);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_fb_info depends on mtrr_found @
> struct fb_info *info;
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> +arch_phys_wc_del(index);
>
> @ ioremap_replace_nocache depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap_nocache(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> @ ioremap_replace_default depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> Generated-by: Coccinelle SmPL
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
> drivers/video/fbdev/vesafb.c | 29 ++++++++---------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
> index 3db3908..528fe91 100644
> --- a/drivers/video/fbdev/vesafb.c
> +++ b/drivers/video/fbdev/vesafb.c
> @@ -19,10 +19,9 @@
> #include <linux/init.h>
> #include <linux/platform_device.h>
> #include <linux/screen_info.h>
> +#include <linux/io.h>
>
> #include <video/vga.h>
> -#include <asm/io.h>
> -#include <asm/mtrr.h>
>
> #define dac_reg (0x3c8)
> #define dac_val (0x3c9)
> @@ -180,16 +179,10 @@ static int vesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
>
> static void vesafb_destroy(struct fb_info *info)
> {
> -#ifdef CONFIG_MTRR
> struct vesafb_par *par = info->par;
> -#endif
>
> fb_dealloc_cmap(&info->cmap);
> -
> -#ifdef CONFIG_MTRR
> - if (par->wc_cookie >= 0)
> - mtrr_del(par->wc_cookie, 0, 0);
> -#endif
> + arch_phys_wc_del(par->wc_cookie);
> if (info->screen_base)
> iounmap(info->screen_base);
> release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
> @@ -420,7 +413,6 @@ static int vesafb_probe(struct platform_device *dev)
> request_region(0x3c0, 32, "vesafb");
>
> if (mtrr == 3) {
> -#ifdef CONFIG_MTRR
> unsigned int temp_size = size_total;
>
> /* Find the largest power-of-two */
> @@ -428,18 +420,16 @@ static int vesafb_probe(struct platform_device *dev)
>
> /* Try and find a power of two to add */
> do {
> - par->wc_cookie = mtrr_add(vesafb_fix.smem_start,
> - temp_size,
> - MTRR_TYPE_WRCOMB, 1);
> + par->wc_cookie =
> + arch_phys_wc_add(vesafb_fix.smem_start,
> + temp_size);
> temp_size >>= 1;
> - } while (temp_size >= PAGE_SIZE && par->wc_cookie == -EINVAL);
> -#endif
> + } while (temp_size >= PAGE_SIZE && par->wc_cookie < 0);
> +
Looks like arch_phys_wc_add prints a warning if it fails, and the above
loop may retry it many (?) times. Probably not a big issue, but may be
somewhat confusing.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Jan Beulich @ 2015-06-12 7:59 UTC (permalink / raw)
To: Toshi Kani
Cc: Andy Lutomirski, Luis R. Rodriguez, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, ville.syrjala, Julia Lawall, xen-devel,
Dave Airlie, syrjala, Juergen Gross, Luis Rodriguez,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1434064996.11808.64.camel@misato.fc.hp.com>
>>> On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
> There are two usages on MTRRs:
> 1) MTRR entries set by firmware
> 2) MTRR entries set by OS drivers
>
> We can obsolete 2), but we have no control over 1). As UEFI firmwares
> also set this up, this usage will continue to stay. So, we should not
> get rid of the MTRR code that looks up the MTRR entries, while we have
> no need to modify them.
>
> Such MTRR entries provide safe guard to /dev/mem, which allows
> privileged user to access a range that may require UC mapping while
> the /dev/mem driver blindly maps it with WB. MTRRs converts WB to UC in
> such a case.
But it wouldn't be impossible to simply read the MTRRs upon boot,
store the information, disable MTRRs, and correctly use PAT to
achieve the same effect (i.e. the "blindly maps" part of course
would need fixing).
> UEFI memory table has memory attribute, which describes cache types
> supported in physical memory ranges. However, this information gets
> lost when it it is converted to e820 table.
I'm afraid you rather don't want to trust that information, as
firmware vendors frequently screw it up.
Jan
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-12 7:25 UTC (permalink / raw)
To: Alexander Holler
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-samsung-soc,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dmitry Torokhov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
list-7NY9aMYB+ISttCpgsWEBFg@public.gmane.org:DRM PANEL DRIVERS,
dmaengine-u79uwXL29TY76Z2rM5mHXA, Dan Williams,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <5579B9E8.9040609-SXC+2es9fhnfWeYVQQPykw@public.gmane.org>
On Thu, Jun 11, 2015 at 6:40 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 11.06.2015 um 14:30 schrieb Linus Walleij:
>> Certainly it is possible to create deadlocks in this scenario, but the
>> scope is not to create an ubreakable system.
>
> IAnd what happens if you run into a deadlock? Do you print "you've lost, try
> changing your kernel config" in some output hidden by a splash-screen? ;)
Sorry it sounds like a blanket argument, the fact that there are
mutexes in the kernel makes it possible to deadlock, it doesn't
mean we don't use mutexes. Some programming problems are
just like such.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] staging: sm750fb: fix c99 comments
From: Juston Li @ 2015-06-12 7:07 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
linux-kernel
Cc: Juston Li
fixed all checkpatch.pl ERROR: do not use C99 // comments
Any C99 comments used to comment out code are simply removed.
Also some of the errors occur inside '#if 0' blocks which I
might as well fix since checkpatch.pl caught them but the blocks
themselves should probably be cleaned up later.
Signed-off-by: Juston Li <juston.h.li@gmail.com>
---
drivers/staging/sm750fb/ddk750_chip.c | 11 ++------
drivers/staging/sm750fb/ddk750_dvi.c | 4 +--
drivers/staging/sm750fb/ddk750_help.c | 2 --
drivers/staging/sm750fb/ddk750_sii164.c | 11 --------
drivers/staging/sm750fb/sm750.h | 10 +++----
drivers/staging/sm750fb/sm750_accel.c | 49 ++++++++++++++++-----------------
drivers/staging/sm750fb/sm750_accel.h | 2 +-
drivers/staging/sm750fb/sm750_cursor.c | 9 +++---
drivers/staging/sm750fb/sm750_help.h | 28 +++----------------
drivers/staging/sm750fb/sm750_hw.c | 4 +--
drivers/staging/sm750fb/sm750_hw.h | 3 --
11 files changed, 41 insertions(+), 92 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 8b47c1b..b2137c8 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -17,7 +17,7 @@ logical_chip_type_t getChipType(void)
char physicalRev;
logical_chip_type_t chip;
- physicalID = devId750;//either 0x718 or 0x750
+ physicalID = devId750; /* either 0x718 or 0x750 /
physicalRev = revId750;
if (physicalID = 0x718)
@@ -257,7 +257,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
unsigned int ulReg;
#if 0
- //move the code to map regiter function.
+ /* move the code to map regiter function. */
if (getChipType() = SM718) {
/* turn on big endian bit*/
ulReg = PEEK32(0x74);
@@ -487,8 +487,6 @@ unsigned int calcPllValue(unsigned int request_orig, pll_value_t *pll)
}
}
}
-
- //printk("Finally: pll->n[%lu],m[%lu],od[%lu],pod[%lu]\n",pll->N,pll->M,pll->OD,pll->POD);
return ret;
}
@@ -580,14 +578,9 @@ pll_value_t *pPLL /* Structure to hold the value to be set in PLL */
}
/* Restore input frequency from Khz to hz unit */
-// pPLL->inputFreq *= 1000;
ulRequestClk *= 1000;
pPLL->inputFreq = DEFAULT_INPUT_CLOCK; /* Default reference clock */
- /* Output debug information */
- //DDKDEBUGPRINT((DISPLAY_LEVEL, "calcPllValue: Requested Frequency = %d\n", ulRequestClk));
- //DDKDEBUGPRINT((DISPLAY_LEVEL, "calcPllValue: Input CLK = %dHz, M=%d, N=%d, OD=%d, POD=%d\n", pPLL->inputFreq, pPLL->M, pPLL->N, pPLL->OD, pPLL->POD));
-
/* Return actual frequency that the PLL can set */
ret = calcPLL(pPLL);
return ret;
diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
index f5932bb..b2bf7e6 100644
--- a/drivers/staging/sm750fb/ddk750_dvi.c
+++ b/drivers/staging/sm750fb/ddk750_dvi.c
@@ -51,7 +51,7 @@ int dviInit(
vsyncEnable, deskewEnable, deskewSetting, continuousSyncEnable,
pllFilterEnable, pllFilterValue);
}
- return -1;//error
+ return -1; /* error */
}
@@ -66,7 +66,6 @@ unsigned short dviGetVendorID(void)
{
dvi_ctrl_device_t *pCurrentDviCtrl;
- //pCurrentDviCtrl = getDviCtrl();
pCurrentDviCtrl = g_dcftSupportedDviController;
if (pCurrentDviCtrl != (dvi_ctrl_device_t *)0)
return pCurrentDviCtrl->pfnGetVendorId();
@@ -86,7 +85,6 @@ unsigned short dviGetDeviceID(void)
{
dvi_ctrl_device_t *pCurrentDviCtrl;
-// pCurrentDviCtrl = getDviCtrl();
pCurrentDviCtrl = g_dcftSupportedDviController;
if (pCurrentDviCtrl != (dvi_ctrl_device_t *)0)
return pCurrentDviCtrl->pfnGetDeviceId();
diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
index 647004d..96c18eb 100644
--- a/drivers/staging/sm750fb/ddk750_help.c
+++ b/drivers/staging/sm750fb/ddk750_help.c
@@ -1,5 +1,3 @@
-//#include "ddk750_reg.h"
-//#include "ddk750_chip.h"
#include "ddk750_help.h"
void __iomem * mmio750 = NULL;
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index 84464c1..b6395b8 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -125,10 +125,7 @@ long sii164InitChip(
unsigned char pllFilterValue
)
{
- //unsigned char ucRegIndex, ucRegValue;
- //unsigned char ucDeviceAddress,
unsigned char config;
- //unsigned long delayCount;
/* Initialize the i2c bus */
#ifdef USE_HW_I2C
@@ -141,10 +138,6 @@ long sii164InitChip(
/* Check if SII164 Chip exists */
if ((sii164GetVendorID() = SII164_VENDOR_ID) && (sii164GetDeviceID() = SII164_DEVICE_ID))
{
-
-#ifdef DDKDEBUG
- //sii164PrintRegisterValues();
-#endif
/*
* Initialize SII164 controller chip.
*/
@@ -241,10 +234,6 @@ long sii164InitChip(
config |= SII164_CONFIGURATION_POWER_NORMAL;
i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
-#ifdef DDKDEBUG
- //sii164PrintRegisterValues();
-#endif
-
return 0;
}
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 30aa312..273882f 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -7,8 +7,6 @@
/* please use revision id to distinguish sm750le and sm750*/
#define SPC_SM750 0
-//#define SPC_SM750LE 8
-
#define MB(x) ((x)<<20)
#define MHZ(x) ((x) * 1000000)
/* align should be 2,4,8,16 */
@@ -95,10 +93,10 @@ struct lynx_cursor{
};
struct lynxfb_crtc{
- unsigned char __iomem * vCursor;//virtual address of cursor
- unsigned char __iomem * vScreen;//virtual address of on_screen
- int oCursor;//cursor address offset in vidmem
- int oScreen;//onscreen address offset in vidmem
+ unsigned char __iomem * vCursor; /* virtual address of cursor */
+ unsigned char __iomem * vScreen; /* virtual address of on_screen */
+ int oCursor; /* cursor address offset in vidmem */
+ int oScreen; /* onscreen address offset in vidmem */
int channel;/* which channel this crtc stands for*/
resource_size_t vidmem_size;/* this view's video memory max size */
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 001e980..a2baffd 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -57,10 +57,10 @@ void hw_de_init(struct lynx_accel * accel)
write_dpr(accel, DE_STRETCH_FORMAT, (read_dpr(accel, DE_STRETCH_FORMAT) & clr) | reg);
/* disable clipping and transparent */
- write_dpr(accel, DE_CLIP_TL, 0);//dpr2c
- write_dpr(accel, DE_CLIP_BR, 0);//dpr30
+ write_dpr(accel, DE_CLIP_TL, 0); /* dpr2c */
+ write_dpr(accel, DE_CLIP_BR, 0); /* dpr30 */
- write_dpr(accel, DE_COLOR_COMPARE_MASK, 0);//dpr24
+ write_dpr(accel, DE_COLOR_COMPARE_MASK, 0); /* dpr24 */
write_dpr(accel, DE_COLOR_COMPARE, 0);
reg = FIELD_SET(0, DE_CONTROL, TRANSPARENCY, DISABLE)|
@@ -104,25 +104,25 @@ int hw_fillrect(struct lynx_accel * accel,
return -1;
}
- write_dpr(accel, DE_WINDOW_DESTINATION_BASE, base);//dpr40
+ write_dpr(accel, DE_WINDOW_DESTINATION_BASE, base); /* dpr40 */
write_dpr(accel, DE_PITCH,
FIELD_VALUE(0, DE_PITCH, DESTINATION, pitch/Bpp)|
- FIELD_VALUE(0, DE_PITCH, SOURCE, pitch/Bpp));//dpr10
+ FIELD_VALUE(0, DE_PITCH, SOURCE, pitch/Bpp)); /* dpr10 */
write_dpr(accel, DE_WINDOW_WIDTH,
FIELD_VALUE(0, DE_WINDOW_WIDTH, DESTINATION, pitch/Bpp)|
- FIELD_VALUE(0, DE_WINDOW_WIDTH, SOURCE, pitch/Bpp));//dpr44
+ FIELD_VALUE(0, DE_WINDOW_WIDTH, SOURCE, pitch/Bpp)); /* dpr44 */
- write_dpr(accel, DE_FOREGROUND, color);//DPR14
+ write_dpr(accel, DE_FOREGROUND, color); /* DPR14 */
write_dpr(accel, DE_DESTINATION,
FIELD_SET(0, DE_DESTINATION, WRAP, DISABLE)|
FIELD_VALUE(0, DE_DESTINATION, X, x)|
- FIELD_VALUE(0, DE_DESTINATION, Y, y));//dpr4
+ FIELD_VALUE(0, DE_DESTINATION, Y, y)); /* dpr4 */
write_dpr(accel, DE_DIMENSION,
FIELD_VALUE(0, DE_DIMENSION, X, width)|
- FIELD_VALUE(0, DE_DIMENSION, Y_ET, height));//dpr8
+ FIELD_VALUE(0, DE_DIMENSION, Y_ET, height)); /* dpr8 */
deCtrl FIELD_SET(0, DE_CONTROL, STATUS, START)|
@@ -130,7 +130,7 @@ int hw_fillrect(struct lynx_accel * accel,
FIELD_SET(0, DE_CONTROL, LAST_PIXEL, ON)|
FIELD_SET(0, DE_CONTROL, COMMAND, RECTANGLE_FILL)|
FIELD_SET(0, DE_CONTROL, ROP_SELECT, ROP2)|
- FIELD_VALUE(0, DE_CONTROL, ROP, rop);//dpr0xc
+ FIELD_VALUE(0, DE_CONTROL, ROP, rop); /* dpr0xc */
write_dpr(accel, DE_CONTROL, deCtrl);
return 0;
@@ -236,12 +236,12 @@ unsigned int rop2) /* ROP value */
/* 2D Source Base.
It is an address offset (128 bit aligned) from the beginning of frame buffer.
*/
- write_dpr(accel, DE_WINDOW_SOURCE_BASE, sBase);//dpr40
+ write_dpr(accel, DE_WINDOW_SOURCE_BASE, sBase); /* dpr40 */
/* 2D Destination Base.
It is an address offset (128 bit aligned) from the beginning of frame buffer.
*/
- write_dpr(accel, DE_WINDOW_DESTINATION_BASE, dBase);//dpr44
+ write_dpr(accel, DE_WINDOW_DESTINATION_BASE, dBase); /* dpr44 */
#if 0
/* Program pitch (distance between the 1st points of two adjacent lines).
@@ -254,14 +254,14 @@ unsigned int rop2) /* ROP value */
width *= 3;
write_dpr(accel, DE_PITCH,
FIELD_VALUE(0, DE_PITCH, DESTINATION, dPitch) |
- FIELD_VALUE(0, DE_PITCH, SOURCE, sPitch));//dpr10
+ FIELD_VALUE(0, DE_PITCH, SOURCE, sPitch)); /* dpr10 */
}
else
#endif
{
write_dpr(accel, DE_PITCH,
FIELD_VALUE(0, DE_PITCH, DESTINATION, (dPitch/Bpp)) |
- FIELD_VALUE(0, DE_PITCH, SOURCE, (sPitch/Bpp)));//dpr10
+ FIELD_VALUE(0, DE_PITCH, SOURCE, (sPitch/Bpp))); /* dpr10 */
}
/* Screen Window width in Pixels.
@@ -269,7 +269,7 @@ unsigned int rop2) /* ROP value */
*/
write_dpr(accel, DE_WINDOW_WIDTH,
FIELD_VALUE(0, DE_WINDOW_WIDTH, DESTINATION, (dPitch/Bpp)) |
- FIELD_VALUE(0, DE_WINDOW_WIDTH, SOURCE, (sPitch/Bpp)));//dpr3c
+ FIELD_VALUE(0, DE_WINDOW_WIDTH, SOURCE, (sPitch/Bpp))); /* dpr3c */
if (accel->de_wait() != 0){
return -1;
@@ -280,14 +280,14 @@ unsigned int rop2) /* ROP value */
write_dpr(accel, DE_SOURCE,
FIELD_SET (0, DE_SOURCE, WRAP, DISABLE) |
FIELD_VALUE(0, DE_SOURCE, X_K1, sx) |
- FIELD_VALUE(0, DE_SOURCE, Y_K2, sy));//dpr0
+ FIELD_VALUE(0, DE_SOURCE, Y_K2, sy)); /* dpr0 */
write_dpr(accel, DE_DESTINATION,
FIELD_SET (0, DE_DESTINATION, WRAP, DISABLE) |
FIELD_VALUE(0, DE_DESTINATION, X, dx) |
- FIELD_VALUE(0, DE_DESTINATION, Y, dy));//dpr04
+ FIELD_VALUE(0, DE_DESTINATION, Y, dy)); /* dpr04 */
write_dpr(accel, DE_DIMENSION,
FIELD_VALUE(0, DE_DIMENSION, X, width) |
- FIELD_VALUE(0, DE_DIMENSION, Y_ET, height));//dpr08
+ FIELD_VALUE(0, DE_DIMENSION, Y_ET, height)); /* dpr08 */
de_ctrl FIELD_VALUE(0, DE_CONTROL, ROP, rop2) |
@@ -297,7 +297,7 @@ unsigned int rop2) /* ROP value */
FIELD_SET(0, DE_CONTROL, DIRECTION, RIGHT_TO_LEFT)
: FIELD_SET(0, DE_CONTROL, DIRECTION, LEFT_TO_RIGHT)) |
FIELD_SET(0, DE_CONTROL, STATUS, START);
- write_dpr(accel, DE_CONTROL, de_ctrl);//dpr0c
+ write_dpr(accel, DE_CONTROL, de_ctrl); /* dpr0c */
}
return 0;
@@ -346,7 +346,6 @@ int hw_imageblit(struct lynx_accel *accel,
if(accel->de_wait() != 0)
{
-// inf_msg("*** ImageBlit return -1 ***\n");
return -1;
}
@@ -370,7 +369,7 @@ int hw_imageblit(struct lynx_accel *accel,
startBit *= 3;
write_dpr(accel, DE_PITCH,
FIELD_VALUE(0, DE_PITCH, DESTINATION, dPitch) |
- FIELD_VALUE(0, DE_PITCH, SOURCE, dPitch));//dpr10
+ FIELD_VALUE(0, DE_PITCH, SOURCE, dPitch)); /* dpr10 */
}
else
@@ -378,7 +377,7 @@ int hw_imageblit(struct lynx_accel *accel,
{
write_dpr(accel, DE_PITCH,
FIELD_VALUE(0, DE_PITCH, DESTINATION, dPitch/bytePerPixel) |
- FIELD_VALUE(0, DE_PITCH, SOURCE, dPitch/bytePerPixel));//dpr10
+ FIELD_VALUE(0, DE_PITCH, SOURCE, dPitch/bytePerPixel)); /* dpr10 */
}
/* Screen Window width in Pixels.
@@ -392,16 +391,16 @@ int hw_imageblit(struct lynx_accel *accel,
For mono bitmap, use startBit for X_K1. */
write_dpr(accel, DE_SOURCE,
FIELD_SET (0, DE_SOURCE, WRAP, DISABLE) |
- FIELD_VALUE(0, DE_SOURCE, X_K1_MONO, startBit));//dpr00
+ FIELD_VALUE(0, DE_SOURCE, X_K1_MONO, startBit)); /* dpr00 */
write_dpr(accel, DE_DESTINATION,
FIELD_SET (0, DE_DESTINATION, WRAP, DISABLE) |
FIELD_VALUE(0, DE_DESTINATION, X, dx) |
- FIELD_VALUE(0, DE_DESTINATION, Y, dy));//dpr04
+ FIELD_VALUE(0, DE_DESTINATION, Y, dy)); /* dpr04 */
write_dpr(accel, DE_DIMENSION,
FIELD_VALUE(0, DE_DIMENSION, X, width) |
- FIELD_VALUE(0, DE_DIMENSION, Y_ET, height));//dpr08
+ FIELD_VALUE(0, DE_DIMENSION, Y_ET, height)); /* dpr08 */
write_dpr(accel, DE_FOREGROUND, fColor);
write_dpr(accel, DE_BACKGROUND, bColor);
diff --git a/drivers/staging/sm750fb/sm750_accel.h b/drivers/staging/sm750fb/sm750_accel.h
index e9d217b..57e20fb 100644
--- a/drivers/staging/sm750fb/sm750_accel.h
+++ b/drivers/staging/sm750fb/sm750_accel.h
@@ -113,7 +113,7 @@
#define DE_CONTROL_TRANSPARENCY_ENABLE 1
#define DE_CONTROL_ROP 7:0
-// Pseudo fields.
+/* Pseudo fields. */
#define DE_CONTROL_SHORT_STROKE_DIR 27:24
#define DE_CONTROL_SHORT_STROKE_DIR_225 0
diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index b145285..cb37ea3 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -141,10 +141,10 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
{
if(opr & (0x80 >> j))
- { //use fg color,id = 2
+ { /* use fg color,id = 2 */
data |= 2 << (j*2);
}else{
- //use bg color,id = 1
+ /* use bg color,id = 1 */
data |= 1 << (j*2);
}
}
@@ -221,10 +221,10 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
{
if(opr & (0x80 >> j))
- { //use fg color,id = 2
+ { /* use fg color,id = 2 */
data |= 2 << (j*2);
}else{
- //use bg color,id = 1
+ /* use bg color,id = 1 */
data |= 1 << (j*2);
}
}
@@ -238,7 +238,6 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
/* assume pitch is 1,2,4,8,...*/
if(!(i&(pitch-1)))
- //if((i+1) % pitch = 0)
{
/* need a return */
pstart += offset;
diff --git a/drivers/staging/sm750fb/sm750_help.h b/drivers/staging/sm750fb/sm750_help.h
index ebc946c..05777f7 100644
--- a/drivers/staging/sm750fb/sm750_help.h
+++ b/drivers/staging/sm750fb/sm750_help.h
@@ -1,9 +1,7 @@
#ifndef LYNX_HELP_H__
#define LYNX_HELP_H__
-/*****************************************************************************\
- * FIELD MACROS *
-\*****************************************************************************/
+/* FIELD MACROS */
#define _LSB(f) (0 ? f)
#define _MSB(f) (1 ? f)
#define _COUNT(f) (_MSB(f) - _LSB(f) + 1)
@@ -17,13 +15,7 @@
#define SET_FIELDV(d, f, v) (((d) & ~GET_MASK(f)) | \
(((v) & RAW_MASK(f)) << _LSB(f)))
-
-////////////////////////////////////////////////////////////////////////////////
-// //
-// Internal macros //
-// //
-////////////////////////////////////////////////////////////////////////////////
-
+/* Internal macros */
#define _F_START(f) (0 ? f)
#define _F_END(f) (1 ? f)
#define _F_SIZE(f) (1 + _F_END(f) - _F_START(f))
@@ -31,13 +23,7 @@
#define _F_NORMALIZE(v, f) (((v) & _F_MASK(f)) >> _F_START(f))
#define _F_DENORMALIZE(v, f) (((v) << _F_START(f)) & _F_MASK(f))
-
-////////////////////////////////////////////////////////////////////////////////
-// //
-// Global macros //
-// //
-////////////////////////////////////////////////////////////////////////////////
-
+/* Global macros */
#define FIELD_GET(x, reg, field) \
( \
_F_NORMALIZE((x), reg ## _ ## field) \
@@ -60,13 +46,7 @@
~ _F_MASK(reg ## _ ## field) \
)
-
-////////////////////////////////////////////////////////////////////////////////
-// //
-// Field Macros //
-// //
-////////////////////////////////////////////////////////////////////////////////
-
+/* Field Macros */
#define FIELD_START(field) (0 ? field)
#define FIELD_END(field) (1 ? field)
#define FIELD_SIZE(field) (1 + FIELD_END(field) - FIELD_START(field))
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 131125c..e70961e 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -193,7 +193,6 @@ int hw_sm750_inithw(struct lynx_share* share, struct pci_dev * pdev)
/* init 2d engine */
if(!share->accel_off){
hw_sm750_initAccel(share);
-// share->accel.de_wait = hw_sm750_deWait;
}
return 0;
@@ -328,7 +327,6 @@ int hw_sm750_crtc_setMode(struct lynxfb_crtc* crtc,
#endif
/* set timing */
-// modparm.pixel_clock = PS_TO_HZ(var->pixclock);
modparm.pixel_clock = ps_to_hz(var->pixclock);
modparm.vertical_sync_polarity = (var->sync & FB_SYNC_HOR_HIGH_ACT) ? POS:NEG;
modparm.horizontal_sync_polarity = (var->sync & FB_SYNC_VERT_HIGH_ACT) ? POS:NEG;
@@ -618,7 +616,7 @@ int hw_sm750_pan_display(struct lynxfb_crtc *crtc,
const struct fb_info *info)
{
uint32_t total;
- //check params
+ /* check params */
if ((var->xoffset + var->xres > var->xres_virtual) ||
(var->yoffset + var->yres > var->yres_virtual)) {
return -EINVAL;
diff --git a/drivers/staging/sm750fb/sm750_hw.h b/drivers/staging/sm750fb/sm750_hw.h
index 4fee28d..93288b3 100644
--- a/drivers/staging/sm750fb/sm750_hw.h
+++ b/drivers/staging/sm750fb/sm750_hw.h
@@ -8,9 +8,6 @@
#define SM750LE_REVISION_ID (unsigned char)0xfe
#endif
-//#define DEFAULT_MEM_CLOCK (DEFAULT_SM750_CHIP_CLOCK/1)
-//#define DEFAULT_MASTER_CLOCK (DEFAULT_SM750_CHIP_CLOCK/3)
-
enum sm750_pnltype{
--
2.4.3
^ permalink raw reply related
* Re: [PATCH v6 2/3] IB/ipath: add counting for MTRR
From: Doug Ledford @ 2015-06-12 3:36 UTC (permalink / raw)
To: Borislav Petkov
Cc: Luis R. Rodriguez, mchehab, tomi.valkeinen, bhelgaas, luto,
linux-media, linux-rdma, linux-kernel, Luis R. Rodriguez,
Toshi Kani, Roland Dreier, Sean Hefty, Hal Rosenstock,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, infinipath, linux-fbdev
In-Reply-To: <20150611195424.GG30391@pd.tnic>
[-- Attachment #1: Type: text/plain, Size: 1872 bytes --]
On 06/11/2015 03:54 PM, Borislav Petkov wrote:
> On Thu, Jun 11, 2015 at 10:50:01AM -0700, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> There is no good reason not to, we eventually delete it as well.
>>
>> Cc: Toshi Kani <toshi.kani@hp.com>
>> Cc: Roland Dreier <roland@kernel.org>
>> Cc: Sean Hefty <sean.hefty@intel.com>
>> Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
>> Cc: Suresh Siddha <sbsiddha@gmail.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Antonino Daplas <adaplas@gmail.com>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: infinipath@intel.com
>> Cc: linux-rdma@vger.kernel.org
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>> ---
>> drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
>> index 4ad0b93..70c1f3a 100644
>> --- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
>> +++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
>> @@ -127,7 +127,7 @@ int ipath_enable_wc(struct ipath_devdata *dd)
>> "(addr %llx, len=0x%llx)\n",
>> (unsigned long long) pioaddr,
>> (unsigned long long) piolen);
>> - cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
>> + cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
>> if (cookie < 0) {
>> {
>> dev_info(&dd->pcidev->dev,
>> --
>
> Doug, ack?
>
Ack.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-06-12 0:52 UTC (permalink / raw)
To: Toshi Kani
Cc: Luis R. Rodriguez, Tomi Valkeinen, Borislav Petkov, Bjorn Helgaas,
Jej B, Ville Syrjälä, Ville Syrjälä,
Andrew Morton, linux-kernel@vger.kernel.org, linux-media,
linux-fbdev, linux-pci@vger.kernel.org, Andy Lutomirski, X86 ML,
Juergen Gross, Dave Airlie, xen-devel, Julia Lawall
In-Reply-To: <1434064996.11808.64.camel@misato.fc.hp.com>
On Thu, Jun 11, 2015 at 05:23:16PM -0600, Toshi Kani wrote:
> On Thu, 2015-06-11 at 13:36 -0700, Luis R. Rodriguez wrote:
> :
> > Pending RIP MTRR patches
> > ==========
> >
> > There are a few pending series so I wanted to provide a status update
> > on those series.
> >
> > mtrr: bury MTRR - unexport mtrr_add() and mtrr_del()
> >
> > This is the nail on the MTRR coffin, it will prevent future direct
> > access to MTRR code. This will not be posted until all of the below
> > patches are in and merged. A possible next step here might be to
> > consider separating PAT code from MTRR code and making PAT a first
> > class citizen, enabling distributions to disable MTRR code in the
> > future. I thought this was possible but for some reason I recently
> > thought that there was one possible issue to make this happen. I
> > suppose we won't know unless we try, unless of course someone already
> > knows, Toshi?
>
> There are two usages on MTRRs:
> 1) MTRR entries set by firmware
> 2) MTRR entries set by OS drivers
>
> We can obsolete 2), but we have no control over 1). As UEFI firmwares
> also set this up, this usage will continue to stay. So, we should not
> get rid of the MTRR code that looks up the MTRR entries, while we have
> no need to modify them.
>
> Such MTRR entries provide safe guard to /dev/mem, which allows
> privileged user to access a range that may require UC mapping while
> the /dev/mem driver blindly maps it with WB. MTRRs converts WB to UC in
> such a case.
>
> UEFI memory table has memory attribute, which describes cache types
> supported in physical memory ranges. However, this information gets
> lost when it it is converted to e820 table.
Is there no way to modify CPU capability bits upon boot and kick UEFI
to re-evaluate ? In such UEFI cases what happens for instance when
Xen is used which does not support MTRR?
Luis
^ permalink raw reply
* Re: RIP MTRR - status update for upcoming v4.2
From: Toshi Kani @ 2015-06-11 23:23 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Tomi Valkeinen, Borislav Petkov, Bjorn Helgaas, Jej B,
Ville Syrjälä, Ville Syrjälä, Andrew Morton,
linux-kernel@vger.kernel.org, linux-media, linux-fbdev,
linux-pci@vger.kernel.org, Andy Lutomirski, X86 ML, Juergen Gross,
Dave Airlie, Luis R. Rodriguez, xen-devel, Julia Lawall
In-Reply-To: <CAB=NE6UgtdSoBsA=8+ueYRAZHDnWUSmQAoHhAaefqudBrSY7Zw@mail.gmail.com>
On Thu, 2015-06-11 at 13:36 -0700, Luis R. Rodriguez wrote:
:
> Pending RIP MTRR patches
> ==========
>
> There are a few pending series so I wanted to provide a status update
> on those series.
>
> mtrr: bury MTRR - unexport mtrr_add() and mtrr_del()
>
> This is the nail on the MTRR coffin, it will prevent future direct
> access to MTRR code. This will not be posted until all of the below
> patches are in and merged. A possible next step here might be to
> consider separating PAT code from MTRR code and making PAT a first
> class citizen, enabling distributions to disable MTRR code in the
> future. I thought this was possible but for some reason I recently
> thought that there was one possible issue to make this happen. I
> suppose we won't know unless we try, unless of course someone already
> knows, Toshi?
There are two usages on MTRRs:
1) MTRR entries set by firmware
2) MTRR entries set by OS drivers
We can obsolete 2), but we have no control over 1). As UEFI firmwares
also set this up, this usage will continue to stay. So, we should not
get rid of the MTRR code that looks up the MTRR entries, while we have
no need to modify them.
Such MTRR entries provide safe guard to /dev/mem, which allows
privileged user to access a range that may require UC mapping while
the /dev/mem driver blindly maps it with WB. MTRRs converts WB to UC in
such a case.
UEFI memory table has memory attribute, which describes cache types
supported in physical memory ranges. However, this information gets
lost when it it is converted to e820 table.
Thanks,
-Toshi
^ permalink raw reply
* RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-06-11 20:36 UTC (permalink / raw)
To: Tomi Valkeinen, Borislav Petkov, Bjorn Helgaas, Jej B,
Ville Syrjälä, Ville Syrjälä, Andrew Morton
Cc: linux-kernel@vger.kernel.org, linux-media, linux-fbdev,
linux-pci@vger.kernel.org, Andy Lutomirski, Toshi Kani, X86 ML,
Juergen Gross, Dave Airlie, Luis R. Rodriguez, xen-devel,
Julia Lawall
The series to bury direct MTRR use is almost all in and on its way to
v4.2. As the pending series continue slowly to be merged I wanted to
take the time to reiterate the justification for these changes in
hopes it may help those still reviewing some of these patches which
are pending and to help document all these changes. There are also
some series which depend on symbols now exported through some other
subsystem trees so coordination is needed there. In those cases we
have the option there to sit and wait for the exported symbols to
trickle in through v4.2 and later on v4.3 finalize the changes, or to
let some of the depending changes to in through other subsystem trees.
I don't consider the coordination required difficult to handle so
would prefer to see the changes in for v4.2 to be able to put a nail
on the MTRR coffin sooner rather than later and to also help get more
testing out of this sooner rather than later. PAT is known to have
errata for some CPUs so hearing reports of issues with PAT would be
very valuable. I'll let maintainers decide on how that trickles
through. To help with all this towards the end I provide a status of
all the pending patches to get this work completed.
Justification
====
We want to bury direct use of MTRR code because:
a) MTRR is x86 specific, this means all existing MTRR code is #idef'd
out. PAT support for x86 was implemented using architecture agnostic
APIs, this enables other architectures to provide support for a
similar write-combining feature, and removes the nasty #idef eyesores.
MTRR should be seen as a first step temporary architectural evolution
to what PAT eventually became on x86.
b) We have a long term goal to change the default behavior of
ioremap_nocache() and pci_mmap_page_range() to use PAT strong UC,
right now we cannot do this, but after all drivers are converted (all
these series I've been posting) we expect to be able to make the
change. Making a change to strong UC on these two calls can only
happen after a period of time of having Linux bake with all these
changes merged and in place. How many kernels we will want Linux baked
with all these transformations to arch_phys before making a change to
ioremap_nocache() and pci_mmap_page_range() is up to x86 folks. There
are other gains possible with this but I welcome others to chime in
here with what gains we can expect from this.
c) MTRR acts on physical addresses and requires power-of-two
alignment, on both the base used and size, this limits the flexibility
for MTRR use. For a good example of its limitations refer to the
patches which change the atyfb driver from using MTRR to PAT.
d) MTRR is known to be unreliable, it can at times not work even on
modern systems.
e) There is a limit to how many MTRRs you can use on a system. If
using a large number of devices with MTRR support you will quickly run
out of MTRRs. This is why originally Andy Lutomirski ended up adding
the arch_phys_wc_add() API, in order to take advantage of PAT which is
*not* bound to the same limitations as MTRRs are.
f) PAT has been available for quite a long time, since Pentium III
(circa 1999) and newer, but having PAT enabled does not restrict use
of MTRR and because of this some systems may end up then combining
MTRR and PAT. I do not believe this wasn't an original highly expected
wide use situation, it technically should work to combine both but
there might be issues with interactions between both, exactly what
issues can exist or have existed remains quite unclear as MTRR in and
of itself has been known to be unreliable anyway. If possible its best
to just be binary about this and only use MTRR if and only if
necessary because of the other issues known with MTRR.
g) Linux has support for Xen PV domains using PAT, this was introduced
by Juergen via v3.19 via commit 47591df50512 ("xen: Support Xen
pv-domains using PAT"). Since MTRR is old we don't want to add MTRR
support into Xen on Linux, instead since Linux now supports PV domains
with PAT we can take full advantage of write combining for PV domains
on Xen provided all Linux drivers are converted to use PAT properly.a
framebuffer folks's ACK
Review of the changes
========
Most of the series has consisted of driver transformations using
Coccinelle SmPL patches to transform existing code which access MTRR
APIs directly to the architecture agnostic write-combining calls.
Other patches extend bus subsystems to make available new
write-combining architecture agnostic APIs. Other patches have
consisted of extending architecture agnostic APIs to help work around
old MTRR hacks -- this was perhaps the hardest task and took quite a
bit of time and review as it required review of implications of all
combinatorial possibilities with MTRR and PAT, which also got
documented as part of the series. In the end it was also determined
some drivers required substantial work to be able to work properly
with PAT, the atyfb driver is an example driver which had the homework
required done. Due to complexities and since the driver / hardware was
ancient we decided to reach a compromise and require users of those
drivers to boot with a kernel parameter to disable PAT, fortunately
this was only required for two old device drivers:
* ipath: the ipath device driver powers the old HTX bus cards that
only work in AMD systems, while the newer IB/qib device driver powers
all PCI-e cards. The ipath device driver is obsolete, hardware hard to
find
* ivtv: the hardware is really rare these days, and perhaps only some
lost souls in some third world country are expected to be using this
feature of the device driver.
Pending RIP MTRR patches
==========
There are a few pending series so I wanted to provide a status update
on those series.
mtrr: bury MTRR - unexport mtrr_add() and mtrr_del()
This is the nail on the MTRR coffin, it will prevent future direct
access to MTRR code. This will not be posted until all of the below
patches are in and merged. A possible next step here might be to
consider separating PAT code from MTRR code and making PAT a first
class citizen, enabling distributions to disable MTRR code in the
future. I thought this was possible but for some reason I recently
thought that there was one possible issue to make this happen. I
suppose we won't know unless we try, unless of course someone already
knows, Toshi?
IB/ipath: use arch_phys_wc_add() and require PAT disabled
IB/ipath: add counting for MTRR
ivtv: use arch_phys_wc_add() and require PAT disabled
This v7 series just posted addresses all drivers which cannot work
with PAT, fortunately we end up with only 2! This series has all
subsystem and driver maintainers ACKs, it was just posted with a few
fixes with the intent to be merged through Boris' x86 tree as it
depends on the newly exported pat_enabled() symbol.
fusion: remove dead MTRR code
This should go through Bottomley's tree, driver maintainer provided an
ACKed, this is just pending integration into the SCSI subsystem tree.
As a last resort my hope is that this can go through Andrew Morton's
tree.
video: fbdev: vesafb: use arch_phys_wc_add()
video: fbdev: vesafb: add missing mtrr_del() for added MTRR
video: fbdev: vesafb: only support MTRR_TYPE_WRCOMB
A v4 series was posted, pending review / Integration through Tomis' tree
video: fbdev: atyfb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: atyfb: replace MTRR UC hole with strong UC
video: fbdev: atyfb: clarify ioremap() base and length used
video: fbdev: atyfb: move framebuffer length fudging to helper
This provides a work around replacement for the direct MTRR use hack
using the new ioremap_uc() API. This is pending review / ACK by the
driver maintainer Ville Syrjälä, and the subsystem maintainer, Tomi.
Since it relies on ioremap_uc(), which is merged through Boris' tree,
this should go through Boris' tree *iff* we want it in for v4.2,
otherwise this will have to wait for v4.3.
video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
lib: devres: add pcim_iomap_wc() variants
pci: add pci_iomap_wc() variants
This has Tomi's ACK already for the driver specific changes, Bjorn
asked for a Documentation guidance update for EXPORT_SYMBOL_GPL(),
this is now merged via linux-next. This is pending consent from Tomi
if it can go in through Bjorn's tree. It is also obviously pending a
final ACK from Bjorn. The devres change would go in without any users,
I leave this for Bjorn to decide but do note my concern of someone in
the future adding a non EXPORT_SYMBOL_GPL() for this implementation.
video: fbdev: gxt4500: use pci_ioremap_wc_bar() for framebuffer
video: fbdev: kyrofb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
video: fbdev: i740fb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
pci: add pci_ioremap_wc_bar()
This requires Tomi's Ack / review, and then obviously Bjorn's own ACK
/ integration. Consent from Tomi of whether or not this can go through
Bjorn's tree is also needed.
Luis
^ permalink raw reply
* [PATCH v7 3/3] IB/ipath: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-06-11 20:19 UTC (permalink / raw)
To: bp
Cc: mchehab, tomi.valkeinen, bhelgaas, luto, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Doug Ledford, Andy Walls,
Hal Rosenstock, Sean Hefty, Suresh Siddha, Rickard Strandqvist,
Mike Marciniszyn, Roland Dreier, Ingo Molnar, Linus Torvalds,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard,
Ville Syrjälä, Dave Hansen, Arnd Bergmann,
Michael S. Tsirkin, Stefan Bader, konrad.wilk, ville.syrjala,
jbeulich, toshi.kani, Roger Pau Monné, infinipath,
linux-fbdev, xen-devel
In-Reply-To: <1434053994-2196-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy compromise given that the ipath device
driver powers the old HTX bus cards that only work in
AMD systems, while the newer IB/qib device driver
powers all PCI-e cards. The ipath device driver is
obsolete, hardware hard to find and because of this
this its a reasonable compromise to make to require
users of ipath to boot with nopat.
Acked-by: Doug Ledford <dledford@redhat.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/Kconfig | 3 ++
drivers/infiniband/hw/ipath/ipath_driver.c | 18 +++++++----
drivers/infiniband/hw/ipath/ipath_kernel.h | 4 +--
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 43 ++++++---------------------
4 files changed, 26 insertions(+), 42 deletions(-)
diff --git a/drivers/infiniband/hw/ipath/Kconfig b/drivers/infiniband/hw/ipath/Kconfig
index 1d9bb11..8fe54ff 100644
--- a/drivers/infiniband/hw/ipath/Kconfig
+++ b/drivers/infiniband/hw/ipath/Kconfig
@@ -9,3 +9,6 @@ config INFINIBAND_IPATH
as IP-over-InfiniBand as well as with userspace applications
(in conjunction with InfiniBand userspace access).
For QLogic PCIe QLE based cards, use the QIB driver instead.
+
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86-64 systems, use the nopat kernel parameter.
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index bd0caed..2d7e503 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -42,6 +42,9 @@
#include <linux/bitmap.h>
#include <linux/slab.h>
#include <linux/module.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
+#endif
#include "ipath_kernel.h"
#include "ipath_verbs.h"
@@ -395,6 +398,14 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
unsigned long long addr;
u32 bar0 = 0, bar1 = 0;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ipath needs PAT disabled, boot with nopat kernel parameter\n")) {
+ ret = -ENODEV;
+ goto bail;
+ }
+#endif
+
dd = ipath_alloc_devdata(pdev);
if (IS_ERR(dd)) {
ret = PTR_ERR(dd);
@@ -542,6 +553,7 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dd->ipath_kregbase = __ioremap(addr, len,
(_PAGE_NO_CACHE|_PAGE_WRITETHRU));
#else
+ /* XXX: split this properly to enable on PAT */
dd->ipath_kregbase = ioremap_nocache(addr, len);
#endif
@@ -587,12 +599,8 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
ret = ipath_enable_wc(dd);
- if (ret) {
- ipath_dev_err(dd, "Write combining not enabled "
- "(err %d): performance may be poor\n",
- -ret);
+ if (ret)
ret = 0;
- }
ipath_verify_pioperf(dd);
diff --git a/drivers/infiniband/hw/ipath/ipath_kernel.h b/drivers/infiniband/hw/ipath/ipath_kernel.h
index e08db70..f0f9471 100644
--- a/drivers/infiniband/hw/ipath/ipath_kernel.h
+++ b/drivers/infiniband/hw/ipath/ipath_kernel.h
@@ -463,9 +463,7 @@ struct ipath_devdata {
/* offset in HT config space of slave/primary interface block */
u8 ipath_ht_slave_off;
/* for write combining settings */
- unsigned long ipath_wc_cookie;
- unsigned long ipath_wc_base;
- unsigned long ipath_wc_len;
+ int wc_cookie;
/* ref count for each pkey */
atomic_t ipath_pkeyrefs[4];
/* shadow copy of struct page *'s for exp tid pages */
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 70c1f3a..7b6e4c8 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -37,7 +37,6 @@
*/
#include <linux/pci.h>
-#include <asm/mtrr.h>
#include <asm/processor.h>
#include "ipath_kernel.h"
@@ -122,27 +121,14 @@ int ipath_enable_wc(struct ipath_devdata *dd)
}
if (!ret) {
- int cookie;
- ipath_cdbg(VERBOSE, "Setting mtrr for chip to WC "
- "(addr %llx, len=0x%llx)\n",
- (unsigned long long) pioaddr,
- (unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
- if (cookie < 0) {
- {
- dev_info(&dd->pcidev->dev,
- "mtrr_add() WC for PIO bufs "
- "failed (%d)\n",
- cookie);
- ret = -EINVAL;
- }
- } else {
- ipath_cdbg(VERBOSE, "Set mtrr for chip to WC, "
- "cookie is %d\n", cookie);
- dd->ipath_wc_cookie = cookie;
- dd->ipath_wc_base = (unsigned long) pioaddr;
- dd->ipath_wc_len = (unsigned long) piolen;
- }
+ dd->wc_cookie = arch_phys_wc_add(pioaddr, piolen);
+ if (dd->wc_cookie < 0) {
+ ipath_dev_err(dd, "Seting mtrr failed on PIO buffers\n");
+ ret = -ENODEV;
+ } else if (dd->wc_cookie = 0)
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC not needed\n");
+ else
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC\n");
}
return ret;
@@ -154,16 +140,5 @@ int ipath_enable_wc(struct ipath_devdata *dd)
*/
void ipath_disable_wc(struct ipath_devdata *dd)
{
- if (dd->ipath_wc_cookie) {
- int r;
- ipath_cdbg(VERBOSE, "undoing WCCOMB on pio buffers\n");
- r = mtrr_del(dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len);
- if (r < 0)
- dev_info(&dd->pcidev->dev,
- "mtrr_del(%lx, %lx, %lx) failed: %d\n",
- dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len, r);
- dd->ipath_wc_cookie = 0; /* even on failure */
- }
+ arch_phys_wc_del(dd->wc_cookie);
}
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v7 2/3] IB/ipath: add counting for MTRR
From: Luis R. Rodriguez @ 2015-06-11 20:19 UTC (permalink / raw)
To: bp
Cc: mchehab, tomi.valkeinen, bhelgaas, luto, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Toshi Kani, Roland Dreier,
Sean Hefty, Hal Rosenstock, Suresh Siddha, Ingo Molnar,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard, infinipath,
linux-fbdev
In-Reply-To: <1434053994-2196-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
There is no good reason not to, we eventually delete it as well.
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 4ad0b93..70c1f3a 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -127,7 +127,7 @@ int ipath_enable_wc(struct ipath_devdata *dd)
"(addr %llx, len=0x%llx)\n",
(unsigned long long) pioaddr,
(unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
+ cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
if (cookie < 0) {
{
dev_info(&dd->pcidev->dev,
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH v6 2/3] IB/ipath: add counting for MTRR
From: Borislav Petkov @ 2015-06-11 19:54 UTC (permalink / raw)
To: Doug Ledford
Cc: Luis R. Rodriguez, mchehab, tomi.valkeinen, bhelgaas, luto,
linux-media, linux-rdma, linux-kernel, Luis R. Rodriguez,
Toshi Kani, Roland Dreier, Sean Hefty, Hal Rosenstock,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, infinipath, linux-fbdev
In-Reply-To: <1434045002-31575-3-git-send-email-mcgrof@do-not-panic.com>
On Thu, Jun 11, 2015 at 10:50:01AM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> There is no good reason not to, we eventually delete it as well.
>
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Roland Dreier <roland@kernel.org>
> Cc: Sean Hefty <sean.hefty@intel.com>
> Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: infinipath@intel.com
> Cc: linux-rdma@vger.kernel.org
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
> drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
> index 4ad0b93..70c1f3a 100644
> --- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
> +++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
> @@ -127,7 +127,7 @@ int ipath_enable_wc(struct ipath_devdata *dd)
> "(addr %llx, len=0x%llx)\n",
> (unsigned long long) pioaddr,
> (unsigned long long) piolen);
> - cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
> + cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
> if (cookie < 0) {
> {
> dev_info(&dd->pcidev->dev,
> --
Doug, ack?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply
* [PATCH v6 3/3] IB/ipath: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-06-11 17:50 UTC (permalink / raw)
To: bp
Cc: mchehab, tomi.valkeinen, bhelgaas, luto, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Doug Ledford, Andy Walls,
Hal Rosenstock, Sean Hefty, Suresh Siddha, Rickard Strandqvist,
Mike Marciniszyn, Roland Dreier, Ingo Molnar, Linus Torvalds,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Davidlohr Bueso, Dave Hansen, Arnd Bergmann, Michael S. Tsirkin,
Stefan Bader, konrad.wilk, ville.syrjala, david.vrabel, jbeulich,
toshi.kani, Roger Pau Monné, infinipath, linux-fbdev,
xen-devel
In-Reply-To: <1434045002-31575-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy compromise given that the ipath device
driver powers the old HTX bus cards that only work in
AMD systems, while the newer IB/qib device driver
powers all PCI-e cards. The ipath device driver is
obsolete, hardware hard to find and because of this
this its a reasonable compromise to make to require
users of ipath to boot with nopat.
Acked-by: Doug Ledford <dledford@redhat.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/Kconfig | 3 ++
drivers/infiniband/hw/ipath/ipath_driver.c | 18 +++++++----
drivers/infiniband/hw/ipath/ipath_kernel.h | 4 +--
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 43 ++++++---------------------
4 files changed, 26 insertions(+), 42 deletions(-)
diff --git a/drivers/infiniband/hw/ipath/Kconfig b/drivers/infiniband/hw/ipath/Kconfig
index 1d9bb11..8fe54ff 100644
--- a/drivers/infiniband/hw/ipath/Kconfig
+++ b/drivers/infiniband/hw/ipath/Kconfig
@@ -9,3 +9,6 @@ config INFINIBAND_IPATH
as IP-over-InfiniBand as well as with userspace applications
(in conjunction with InfiniBand userspace access).
For QLogic PCIe QLE based cards, use the QIB driver instead.
+
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86-64 systems, use the nopat kernel parameter.
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index bd0caed..441cfe5 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -42,6 +42,9 @@
#include <linux/bitmap.h>
#include <linux/slab.h>
#include <linux/module.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
+#endif
#include "ipath_kernel.h"
#include "ipath_verbs.h"
@@ -395,6 +398,14 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
unsigned long long addr;
u32 bar0 = 0, bar1 = 0;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ipath needs PAT disabled, boot with nopat kernel parameter\n")) {
+ ret = EINVAL;
+ goto bail;
+ }
+#endif
+
dd = ipath_alloc_devdata(pdev);
if (IS_ERR(dd)) {
ret = PTR_ERR(dd);
@@ -542,6 +553,7 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dd->ipath_kregbase = __ioremap(addr, len,
(_PAGE_NO_CACHE|_PAGE_WRITETHRU));
#else
+ /* XXX: split this properly to enable on PAT */
dd->ipath_kregbase = ioremap_nocache(addr, len);
#endif
@@ -587,12 +599,8 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
ret = ipath_enable_wc(dd);
- if (ret) {
- ipath_dev_err(dd, "Write combining not enabled "
- "(err %d): performance may be poor\n",
- -ret);
+ if (ret)
ret = 0;
- }
ipath_verify_pioperf(dd);
diff --git a/drivers/infiniband/hw/ipath/ipath_kernel.h b/drivers/infiniband/hw/ipath/ipath_kernel.h
index e08db70..f0f9471 100644
--- a/drivers/infiniband/hw/ipath/ipath_kernel.h
+++ b/drivers/infiniband/hw/ipath/ipath_kernel.h
@@ -463,9 +463,7 @@ struct ipath_devdata {
/* offset in HT config space of slave/primary interface block */
u8 ipath_ht_slave_off;
/* for write combining settings */
- unsigned long ipath_wc_cookie;
- unsigned long ipath_wc_base;
- unsigned long ipath_wc_len;
+ int wc_cookie;
/* ref count for each pkey */
atomic_t ipath_pkeyrefs[4];
/* shadow copy of struct page *'s for exp tid pages */
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 70c1f3a..7b6e4c8 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -37,7 +37,6 @@
*/
#include <linux/pci.h>
-#include <asm/mtrr.h>
#include <asm/processor.h>
#include "ipath_kernel.h"
@@ -122,27 +121,14 @@ int ipath_enable_wc(struct ipath_devdata *dd)
}
if (!ret) {
- int cookie;
- ipath_cdbg(VERBOSE, "Setting mtrr for chip to WC "
- "(addr %llx, len=0x%llx)\n",
- (unsigned long long) pioaddr,
- (unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
- if (cookie < 0) {
- {
- dev_info(&dd->pcidev->dev,
- "mtrr_add() WC for PIO bufs "
- "failed (%d)\n",
- cookie);
- ret = -EINVAL;
- }
- } else {
- ipath_cdbg(VERBOSE, "Set mtrr for chip to WC, "
- "cookie is %d\n", cookie);
- dd->ipath_wc_cookie = cookie;
- dd->ipath_wc_base = (unsigned long) pioaddr;
- dd->ipath_wc_len = (unsigned long) piolen;
- }
+ dd->wc_cookie = arch_phys_wc_add(pioaddr, piolen);
+ if (dd->wc_cookie < 0) {
+ ipath_dev_err(dd, "Seting mtrr failed on PIO buffers\n");
+ ret = -ENODEV;
+ } else if (dd->wc_cookie = 0)
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC not needed\n");
+ else
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC\n");
}
return ret;
@@ -154,16 +140,5 @@ int ipath_enable_wc(struct ipath_devdata *dd)
*/
void ipath_disable_wc(struct ipath_devdata *dd)
{
- if (dd->ipath_wc_cookie) {
- int r;
- ipath_cdbg(VERBOSE, "undoing WCCOMB on pio buffers\n");
- r = mtrr_del(dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len);
- if (r < 0)
- dev_info(&dd->pcidev->dev,
- "mtrr_del(%lx, %lx, %lx) failed: %d\n",
- dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len, r);
- dd->ipath_wc_cookie = 0; /* even on failure */
- }
+ arch_phys_wc_del(dd->wc_cookie);
}
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v6 2/3] IB/ipath: add counting for MTRR
From: Luis R. Rodriguez @ 2015-06-11 17:50 UTC (permalink / raw)
To: bp
Cc: mchehab, tomi.valkeinen, bhelgaas, luto, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Toshi Kani, Roland Dreier,
Sean Hefty, Hal Rosenstock, Suresh Siddha, Ingo Molnar,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard, infinipath,
linux-fbdev
In-Reply-To: <1434045002-31575-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
There is no good reason not to, we eventually delete it as well.
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 4ad0b93..70c1f3a 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -127,7 +127,7 @@ int ipath_enable_wc(struct ipath_devdata *dd)
"(addr %llx, len=0x%llx)\n",
(unsigned long long) pioaddr,
(unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
+ cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
if (cookie < 0) {
{
dev_info(&dd->pcidev->dev,
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-06-11 17:50 UTC (permalink / raw)
To: bp
Cc: mchehab, tomi.valkeinen, bhelgaas, luto, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Andy Walls, Doug Ledford,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
Michael S. Tsirkin, Stefan Bader, Ville Syrjälä,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, konrad.wilk,
ville.syrjala, david.vrabel, jbeulich, toshi.kani,
Roger Pau Monné, linux-fbdev, ivtv-devel, xen-devel
In-Reply-To: <1434045002-31575-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.
Acked-by: Andy Walls <awalls@md.metrocast.net>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: ivtv-devel@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/media/pci/ivtv/Kconfig | 3 +++
drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
2 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
This is used in the Hauppauge PVR-350 card. There is a driver
homepage at <http://www.ivtvdriver.org>.
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
To compile this driver as a module, choose M here: the
module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
#include <linux/ivtvfb.h>
#include <linux/slab.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
#endif
#include "ivtv-driver.h"
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+ int wc_cookie;
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
static int ivtvfb_init_io(struct ivtv *itv)
{
struct osd_info *oi = itv->osd_info;
+ /* Find the largest power of two that maps the whole buffer */
+ int size_shift = 31;
mutex_lock(&itv->serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_pbase, oi->video_vbase,
oi->video_buffer_size / 1024);
-#ifdef CONFIG_MTRR
- {
- /* Find the largest power of two that maps the whole buffer */
- int size_shift = 31;
-
- while (!(oi->video_buffer_size & (1 << size_shift))) {
- size_shift--;
- }
- size_shift++;
- oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
- oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
- oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
- oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
- if (mtrr_add(oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
- MTRR_TYPE_WRCOMB, 1) < 0) {
- IVTVFB_INFO("disabled mttr\n");
- oi->fb_start_aligned_physaddr = 0;
- oi->fb_end_aligned_physaddr = 0;
- }
- }
-#endif
-
+ while (!(oi->video_buffer_size & (1 << size_shift)))
+ size_shift--;
+ size_shift++;
+ oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
+ oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
+ oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
+ oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
+ oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
+ oi->fb_end_aligned_physaddr -
+ oi->fb_start_aligned_physaddr);
/* Blank the entire osd. */
memset_io(oi->video_vbase, 0, oi->video_buffer_size);
@@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
/* Release pseudo palette */
kfree(oi->ivtvfb_info.pseudo_palette);
-
-#ifdef CONFIG_MTRR
- if (oi->fb_end_aligned_physaddr) {
- mtrr_del(-1, oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
- }
-#endif
-
+ arch_phys_wc_del(oi->wc_cookie);
kfree(oi);
itv->osd_info = NULL;
}
@@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
+ return EINVAL;
+ }
+#endif
+
if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
IVTV_MAX_CARDS - 1);
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-11 16:40 UTC (permalink / raw)
To: Linus Walleij
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc, linux-tegra@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
Dmitry Torokhov, linux-kernel@vger.kernel.org, Rob Herring,
linux-pwm@vger.kernel.org,
list@wandq.ahsoftware:DRM PANEL DRIVERS, dmaengine, Dan Williams,
linux-usb@vger.kernel.org
In-Reply-To: <CACRpkdaZgBR5rHwrEYqcm5K4w3y8MXG8a0k=Y+oQY+jukHfuyw@mail.gmail.com>
Am 11.06.2015 um 14:30 schrieb Linus Walleij:
> On Thu, Jun 11, 2015 at 12:17 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de>
>>> wrote:
>
>>>> You would end up with the same problem of deadlocks as currently, and you
>>>> would still need something ugly like the defered probe brutforce to avoid
>>>> them.
>>>
>>>
>>> Sorry I don't get that. Care to elaborate on why?
>>
>>
>> Because loading/initializing on demand doesn't give you any solved order of
>> drivers to initialize. And it can't because it has no idea about the
>> requirements of other drivers. The reason why it might work better in the
>> case of the tegra is that it might give you another initialization order
>> than the one which is currently choosen, which, by luck, might be a better
>> one.
>>
>> But maybe I missed something, I haven't looked at the patches at all. But
>> just loading on demand, can't magically give you a working order of drivers
>> to initialize. E.g. how do you choose the first driver to initialize?
>
> So the current patch set introduces dependencies (just for device tree)
> and Tomeu is working on a more generic dependency approach for
> any HW description.
>
> The first driver to initialize will be as usual the first one in the list for
> that initlevel, then walking up the initilevels.
>
> However if any driver runs into a resource roadblock it will postpone
> and wait for dependencies to probe first.
>
> Certainly it is possible to create deadlocks in this scenario, but the
> scope is not to create an ubreakable system.
IAnd what happens if you run into a deadlock? Do you print "you've lost,
try changing your kernel config" in some output hidden by a
splash-screen? ;)
That sounds like the fun with duck typed languages where you have to
test any and every possible screnario (something which is almost
impossible) in order to not run into something unexpected.
Anyway, have fun, good luck.
Alexander Holler
^ permalink raw reply
* [PATCH] video: sm501fb: fixing static checker warning
From: Heiko Schocher @ 2015-06-11 13:13 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA
Cc: Heiko Schocher, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rasmus Villemoes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Tomi Valkeinen, Grant Likely, Rob Herring,
Jean-Christophe Plagniol-Villard, Andrew Morton
Dan Carpenter reported the static checker warning:
drivers/video/fbdev/sm501fb.c:1958 sm501fb_probe()
warn: strcpy() 'cp' of unknown size might be too large for 'fb_mode'
Fix it, as the SM501 datasheet says the SM501 can "200 MHz
DAC support 1280x1024 resolution", which would result in a
too long mode string for current fb_mode var.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Heiko Schocher <hs@denx.de>
---
drivers/video/fbdev/sm501fb.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index 9e74e8f..ea50df3 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -43,7 +43,9 @@
#include "edid.h"
-static char *fb_mode = "640x480-16@60";
+static char *fb_default_mode = "640x480-16@60";
+static char fb_mode[20];
+static char *fb_mode_cmdline;
static unsigned long default_bpp = 16;
static struct fb_videomode sm501_default_mode = {
@@ -1963,6 +1965,12 @@ static int sm501fb_probe(struct platform_device *pdev)
if (info->edid_data)
found = 1;
}
+ } else {
+ if (fb_mode_cmdline)
+ strncpy(fb_mode, fb_mode_cmdline,
+ sizeof(fb_mode) - 1);
+ else
+ strcpy(fb_mode, fb_default_mode);
}
#endif
if (!found) {
@@ -2230,7 +2238,7 @@ static struct platform_driver sm501fb_driver = {
module_platform_driver(sm501fb_driver);
-module_param_named(mode, fb_mode, charp, 0);
+module_param_named(mode, fb_mode_cmdline, charp, 0);
MODULE_PARM_DESC(mode,
"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
module_param_named(bpp, default_bpp, ulong, 0);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 00/21] On-demand device registration
From: Tomeu Vizoso @ 2015-06-11 13:09 UTC (permalink / raw)
To: Alexander Holler, Linus Walleij
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-samsung-soc, open, dmaengine,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-tegra@vger.kernel.org, Rob Herring,
list@wandq.ahsoftware:DRM PANEL DRIVERS, Grant Likely,
Dan Williams, Dmitry Torokhov, linux-usb@vger.kernel.org
In-Reply-To: <5579602F.1070801@ahsoftware.de>
On 06/11/2015 12:17 PM, Alexander Holler wrote:
> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>>
>>>> i2c host comes out, probes the regulator driver, regulator driver
>>>> probes and then the regulator_get() call returns.
>>>>
>>>> This requires instrumentation on anything providing a resource
>>>> to another driver like those I mentioned and a lot of overhead
>>>> infrastructure, but I think it's the right approach. However I don't
>>>> know if I would ever be able to pull that off myself, I know talk
>>>> is cheap and I should show the code instead.
>>>
>>> You would end up with the same problem of deadlocks as currently, and you
>>> would still need something ugly like the defered probe brutforce to avoid
>>> them.
>>
>> Sorry I don't get that. Care to elaborate on why?
>
> Because loading/initializing on demand doesn't give you any solved order
> of drivers to initialize. And it can't because it has no idea about the
> requirements of other drivers.
So, this is only about ordering device probing. All built-in drivers
have already registered themselves by when we start probing.
> The reason why it might work better in
> the case of the tegra is that it might give you another initialization
> order than the one which is currently choosen, which, by luck, might be
> a better one.
Note that this series was also tested on iMX.6, Exynos and OMAP4.
> But maybe I missed something, I haven't looked at the patches at all.
It's a really small patchset :)
19 files changed, 130 insertions(+), 45 deletions(-)
Thanks,
Tomeu
> But just loading on demand, can't magically give you a working order of
> drivers to initialize. E.g. how do you choose the first driver to
> initialize?
>
> Regards,
>
> Alexander Holler
>
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-11 12:30 UTC (permalink / raw)
To: Alexander Holler
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-samsung-soc, open, dmaengine,
Tomeu Vizoso, linux-gpio@vger.kernel.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pwm@vger.kernel.org, linux-tegra@vger.kernel.org,
Rob Herring, list@wandq.ahsoftware:DRM PANEL DRIVERS,
Grant Likely, Dan Williams, Dmitry Torokhov,
linux-usb@vger.kernel.org
In-Reply-To: <5579602F.1070801@ahsoftware.de>
On Thu, Jun 11, 2015 at 12:17 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de>
>> wrote:
>>> You would end up with the same problem of deadlocks as currently, and you
>>> would still need something ugly like the defered probe brutforce to avoid
>>> them.
>>
>>
>> Sorry I don't get that. Care to elaborate on why?
>
>
> Because loading/initializing on demand doesn't give you any solved order of
> drivers to initialize. And it can't because it has no idea about the
> requirements of other drivers. The reason why it might work better in the
> case of the tegra is that it might give you another initialization order
> than the one which is currently choosen, which, by luck, might be a better
> one.
>
> But maybe I missed something, I haven't looked at the patches at all. But
> just loading on demand, can't magically give you a working order of drivers
> to initialize. E.g. how do you choose the first driver to initialize?
So the current patch set introduces dependencies (just for device tree)
and Tomeu is working on a more generic dependency approach for
any HW description.
The first driver to initialize will be as usual the first one in the list for
that initlevel, then walking up the initilevels.
However if any driver runs into a resource roadblock it will postpone
and wait for dependencies to probe first.
Certainly it is possible to create deadlocks in this scenario, but the
scope is not to create an ubreakable system.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-11 11:49 UTC (permalink / raw)
To: Linus Walleij
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-samsung-soc,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dmitry Torokhov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
list-7NY9aMYB+ISttCpgsWEBFg@public.gmane.org:DRM PANEL DRIVERS,
dmaengine-u79uwXL29TY76Z2rM5mHXA, Dan Williams,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <55796FDE.7080701-SXC+2es9fhnfWeYVQQPykw@public.gmane.org>
Am 11.06.2015 um 13:24 schrieb Alexander Holler:
> Am 11.06.2015 um 12:17 schrieb Alexander Holler:
>> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler
>>> <holler@ahsoftware.de> wrote:
>>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>>>
>>>>> i2c host comes out, probes the regulator driver, regulator driver
>>>>> probes and then the regulator_get() call returns.
>>>>>
>>>>> This requires instrumentation on anything providing a resource
>>>>> to another driver like those I mentioned and a lot of overhead
>>>>> infrastructure, but I think it's the right approach. However I don't
>>>>> know if I would ever be able to pull that off myself, I know talk
>>>>> is cheap and I should show the code instead.
>>>>
>>>> You would end up with the same problem of deadlocks as currently, and
>>>> you
>>>> would still need something ugly like the defered probe brutforce to
>>>> avoid
>>>> them.
>>>
>>> Sorry I don't get that. Care to elaborate on why?
>>
>> Because loading/initializing on demand doesn't give you any solved order
>> of drivers to initialize. And it can't because it has no idea about the
>> requirements of other drivers. The reason why it might work better in
>> the case of the tegra is that it might give you another initialization
>> order than the one which is currently choosen, which, by luck, might be
>> a better one.
>>
>> But maybe I missed something, I haven't looked at the patches at all.
>> But just loading on demand, can't magically give you a working order of
>> drivers to initialize. E.g. how do you choose the first driver to
>> initialize?
>
> Other problems you will run into are time constraints and multithreaded
> drivers.
>
> E.g. we all should know how tricky it sometimes is to avoid deadlocks.
> And with loading on demand, you are extending this problem over the
> initialization of maybe a whole bunch of other drivers which might be
> started by calling one function of another driver. And a function call
> might need a very long time to finish during which an unpredictable
> amount of things may happen.
>
> It would make me wonder if that will end up with a good, usable and as
> simple as possible solution.
Besides that instrumenting every call to another driver in order to fix
a onetime operation (the initialization) sounds like an enormous
overhead. Initialization is done pnly once, regardless how long a system
runs, but the instrumentation to fix this onetime operation would slow
down the operation during the whole runtime of a system.
I don't think this is what should be done.
>
> Regards,
>
> Alexander Holler
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-11 11:24 UTC (permalink / raw)
To: Linus Walleij
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-samsung-soc,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dmitry Torokhov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
open-7NY9aMYB+ISttCpgsWEBFg,
list-7NY9aMYB+ISttCpgsWEBFg@public.gmane.org:DRM PANEL DRIVERS,
dmaengine-u79uwXL29TY76Z2rM5mHXA, Dan Williams,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <5579602F.1070801-SXC+2es9fhnfWeYVQQPykw@public.gmane.org>
Am 11.06.2015 um 12:17 schrieb Alexander Holler:
> Am 11.06.2015 um 10:12 schrieb Linus Walleij:
>> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler
>> <holler@ahsoftware.de> wrote:
>>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>>
>>>> i2c host comes out, probes the regulator driver, regulator driver
>>>> probes and then the regulator_get() call returns.
>>>>
>>>> This requires instrumentation on anything providing a resource
>>>> to another driver like those I mentioned and a lot of overhead
>>>> infrastructure, but I think it's the right approach. However I don't
>>>> know if I would ever be able to pull that off myself, I know talk
>>>> is cheap and I should show the code instead.
>>>
>>> You would end up with the same problem of deadlocks as currently, and
>>> you
>>> would still need something ugly like the defered probe brutforce to
>>> avoid
>>> them.
>>
>> Sorry I don't get that. Care to elaborate on why?
>
> Because loading/initializing on demand doesn't give you any solved order
> of drivers to initialize. And it can't because it has no idea about the
> requirements of other drivers. The reason why it might work better in
> the case of the tegra is that it might give you another initialization
> order than the one which is currently choosen, which, by luck, might be
> a better one.
>
> But maybe I missed something, I haven't looked at the patches at all.
> But just loading on demand, can't magically give you a working order of
> drivers to initialize. E.g. how do you choose the first driver to
> initialize?
Other problems you will run into are time constraints and multithreaded
drivers.
E.g. we all should know how tricky it sometimes is to avoid deadlocks.
And with loading on demand, you are extending this problem over the
initialization of maybe a whole bunch of other drivers which might be
started by calling one function of another driver. And a function call
might need a very long time to finish during which an unpredictable
amount of things may happen.
It would make me wonder if that will end up with a good, usable and as
simple as possible solution.
Regards,
Alexander Holler
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-11 10:17 UTC (permalink / raw)
To: Linus Walleij
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc, linux-tegra@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
Dmitry Torokhov, linux-kernel@vger.kernel.org, Rob Herring,
linux-pwm@vger.kernel.org, open,
list@wandq.ahsoftware:DRM PANEL DRIVERS, dmaengine, Dan Williams,
linux-usb@vger.kernel.org
In-Reply-To: <CACRpkdYF5K7ckGbgvZc6qRzGFJTQ4WqXdu_7Tcqczn2LkFbn3A@mail.gmail.com>
Am 11.06.2015 um 10:12 schrieb Linus Walleij:
> On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>
>>> i2c host comes out, probes the regulator driver, regulator driver
>>> probes and then the regulator_get() call returns.
>>>
>>> This requires instrumentation on anything providing a resource
>>> to another driver like those I mentioned and a lot of overhead
>>> infrastructure, but I think it's the right approach. However I don't
>>> know if I would ever be able to pull that off myself, I know talk
>>> is cheap and I should show the code instead.
>>
>> You would end up with the same problem of deadlocks as currently, and you
>> would still need something ugly like the defered probe brutforce to avoid
>> them.
>
> Sorry I don't get that. Care to elaborate on why?
Because loading/initializing on demand doesn't give you any solved order
of drivers to initialize. And it can't because it has no idea about the
requirements of other drivers. The reason why it might work better in
the case of the tegra is that it might give you another initialization
order than the one which is currently choosen, which, by luck, might be
a better one.
But maybe I missed something, I haven't looked at the patches at all.
But just loading on demand, can't magically give you a working order of
drivers to initialize. E.g. how do you choose the first driver to
initialize?
Regards,
Alexander Holler
^ permalink raw reply
* Re: video, sm501: add OF binding to support SM501
From: Dan Carpenter @ 2015-06-11 9:32 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20150610155132.GK10549@mwanda>
On Thu, Jun 11, 2015 at 11:24:41AM +0200, Heiko Schocher wrote:
> static struct fb_videomode sm501_default_mode = {
> @@ -1963,6 +1965,11 @@ static int sm501fb_probe(struct platform_device *pdev)
> if (info->edid_data)
> found = 1;
> }
> + } else {
> + if (fb_mode_cmdline)
> + strcpy(fb_mode, fb_mode_cmdline);
Could you make this a strncpy()? These days strlcpy() is
unfashionable... It's sort of pedantic and it goes over the 80 char
limit but the my static checker will complain if we don't.
strncpy(fb_mod, fb_mode_cmdline,
sizeof(fb_mod) - 1);
> + else
> + strcpy(fb_mode, fb_default_mode);
regards,
dan carpenter
^ permalink raw reply
* Re: video, sm501: add OF binding to support SM501
From: Heiko Schocher @ 2015-06-11 9:24 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20150610155132.GK10549@mwanda>
Hello Dan,
Am 10.06.2015 17:51, schrieb Dan Carpenter:
> Hello Heiko Schocher,
>
> The patch 4295f9bf74a8: "video, sm501: add OF binding to support
> SM501" from Jan 26, 2011, leads to the following static checker
> warning:
>
> drivers/video/fbdev/sm501fb.c:1958 sm501fb_probe()
> warn: strcpy() 'cp' of unknown size might be too large for 'fb_mode'
>
> drivers/video/fbdev/sm501fb.c
> 46 static char *fb_mode = "640x480-16@60";
>
> [ snip ]
>
> 1953 info->pdata = &sm501fb_def_pdata;
> 1954 if (np) {
> 1955 /* Get EDID */
> 1956 cp = of_get_property(np, "mode", &len);
> 1957 if (cp)
> 1958 strcpy(fb_mode, cp);
>
> I don't know anything about this hardware but there might be a mode
> longer than "640x480-16@60"?
Hmm... I just used it with this resolution, but you are right, the
datasheet says "200 MHz DAC support 1280×1024 resolution" ... so, in
this case fb_mode would be to short ... I propose such a fix:
$ git diff
diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index 9e74e8f..2e57f2e 100644
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -43,7 +43,9 @@
#include "edid.h"
-static char *fb_mode = "640x480-16@60";
+static char *fb_default_mode = "640x480-16@60";
+static char fb_mode[20];
+static char *fb_mode_cmdline;
static unsigned long default_bpp = 16;
static struct fb_videomode sm501_default_mode = {
@@ -1963,6 +1965,11 @@ static int sm501fb_probe(struct platform_device *pdev)
if (info->edid_data)
found = 1;
}
+ } else {
+ if (fb_mode_cmdline)
+ strcpy(fb_mode, fb_mode_cmdline);
+ else
+ strcpy(fb_mode, fb_default_mode);
}
#endif
if (!found) {
@@ -2230,7 +2237,7 @@ static struct platform_driver sm501fb_driver = {
module_platform_driver(sm501fb_driver);
-module_param_named(mode, fb_mode, charp, 0);
+module_param_named(mode, fb_mode_cmdline, charp, 0);
MODULE_PARM_DESC(mode,
"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
module_param_named(bpp, default_bpp, ulong, 0);
What do you think? If this is OK from your side, I can post this patch
"officially" ...
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply related
* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-11 8:15 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-samsung-soc, dmaengine,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-clk, linux-tegra@vger.kernel.org, Rob Herring,
open list:DRM PANEL DRIVERS, Grant Likely, Alexander Holler,
Dan Williams, Dmitry Torokhov, linux-usb@vger.kernel.org
In-Reply-To: <CAAObsKD97nEARO5eZYA2g8+z_MrazBy-m8PV_kSyj19ya72mAw@mail.gmail.com>
On Wed, Jun 10, 2015 at 12:19 PM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> On 10 June 2015 at 09:30, Linus Walleij <linus.walleij@linaro.org> wrote:
>> regulator_get(...) -> not available, so:
>> - identify target regulator provider - this will need instrumentation
>> - probe it
>>
>> It then turns out the regulator driver is on the i2c bus, so we
>> need to probe the i2c driver:
>> - identify target i2c host for the regulator driver - this will need
>> instrumentation
>> - probe the i2c host driver
>>
>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
>
> Hmm, if I understand correctly what you say, this is exactly what this
> particular series does:
>
> regulator_get -> of_platform_device_ensure -> probe() on the platform
> device that encloses the requested device node (i2c host) -> i2c slave
> gets probed and the regulator registered -> regulator_get returns the
> requested resource
Yes. But only for device tree.
> The downside I'm currently looking at is that an explicit dependency
> graph would be useful to have for other purposes. For example to print
> a neat warning when a dependency cannot be fulfilled. Or to refuse to
> unbind a device which other devices depend on, or to automatically
> unbind the devices that depend on it, or to print a warning if a
> device is hotplugged off and other devices depend on it.
Unbind/remove() calls are the inverse usually yes.
But also the [runtime] power up/down sequences for the
devices tend to depend on a similar ordering or mostly
the same. (Mentioned this before I think.)
>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
>
> Yeah, if you can give it a second look and say if it matches what you
> wrote above, it would be very much appreciated.
Yes you are right. But what about ACPI, board files,
Simple Firmware and future hardware description languages...
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-11 8:12 UTC (permalink / raw)
To: Alexander Holler
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc, linux-tegra@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
Dmitry Torokhov, linux-kernel@vger.kernel.org, Rob Herring,
linux-pwm@vger.kernel.org, open,
list@wandq.ahsoftware:DRM PANEL DRIVERS <dri-devel@lists.freedesktop.org>, dmaengine@vger.kernel.org, Dan Williams <dan.j.williams@intel.com>, linux-usb@vger.kernel.org
In-Reply-To: <5577F533.1060007@ahsoftware.de>
On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
>>
>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
>
> You would end up with the same problem of deadlocks as currently, and you
> would still need something ugly like the defered probe brutforce to avoid
> them.
Sorry I don't get that. Care to elaborate on why?
Yours,
Linus Walleij
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox