* [PATCH v2 09/12] staging: sm750: blank line after declaration
From: Michel von Czettritz @ 2015-03-26 22:26 UTC (permalink / raw)
To: linux-fbdev
This patch fixes "WARNING: Missing a blank line after declaration"
checkpatch.pl warnings.
Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
v2: fix ident
---
drivers/staging/sm750fb/sm750.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 6887926..c72d9d6 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -156,6 +156,7 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
if (fbcursor->set & FB_CUR_SETCMAP) {
/* get the 16bit color of kernel means */
u16 fg, bg;
+
fg = ((info->cmap.red[fbcursor->image.fg_color] & 0xf800))|
((info->cmap.green[fbcursor->image.fg_color] & 0xfc00) >> 5)|
((info->cmap.blue[fbcursor->image.fg_color] & 0xf800) >> 11);
@@ -269,7 +270,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info,
if (image->depth = 1) {
if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
- info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
+ info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
fgcol = ((u32*)info->pseudo_palette)[image->fg_color];
bgcol = ((u32*)info->pseudo_palette)[image->bg_color];
} else {
@@ -620,9 +621,12 @@ exit:
}
-static int lynxfb_ops_setcolreg(unsigned regno, unsigned red,
- unsigned green, unsigned blue,
- unsigned transp, struct fb_info *info)
+static int lynxfb_ops_setcolreg(unsigned regno,
+ unsigned red,
+ unsigned green,
+ unsigned blue,
+ unsigned transp,
+ struct fb_info *info)
{
struct lynxfb_par *par;
struct lynxfb_crtc *crtc;
@@ -654,9 +658,10 @@ static int lynxfb_ops_setcolreg(unsigned regno, unsigned red,
if (info->fix.visual = FB_VISUAL_TRUECOLOR && regno < 256) {
u32 val;
+
if (var->bits_per_pixel = 16 ||
- var->bits_per_pixel = 32 ||
- var->bits_per_pixel = 24) {
+ var->bits_per_pixel = 32 ||
+ var->bits_per_pixel = 24) {
val = chan_to_field(red, &var->red);
val |= chan_to_field(green, &var->green);
val |= chan_to_field(blue, &var->blue);
@@ -1189,6 +1194,7 @@ ALLOC_FB:
} else {
struct lynxfb_par *par;
int errno;
+
pr_info("framebuffer #%d alloc okay\n", fbidx);
share->fbinfo[fbidx] = info[fbidx];
par = info[fbidx]->par;
@@ -1208,8 +1214,8 @@ ALLOC_FB:
errno = register_framebuffer(info[fbidx]);
if (errno < 0) {
pr_err("Failed to register fb_info #%d. err %d\n",
- fbidx,
- errno);
+ fbidx,
+ errno);
if (fbidx = 0)
goto err_register0;
else
--
2.3.3
^ permalink raw reply related
* [PATCH v2 10/12] staging: sm750: fix whitespace
From: Michel von Czettritz @ 2015-03-26 22:26 UTC (permalink / raw)
To: linux-fbdev
This patch fixes whitespace checkpatch.pl warnings and errors.
Mainly whitespaces around operators.
- ERROR: spaces required around that ':' (ctx:VxW)
- ERROR: space required after that ';' (ctx:VxV)
- ERROR: spaces required around that '<' (ctx:VxV)
- ERROR: spaces required around that '!=' (ctx:VxV)
And some small warnings around whitespace:
- ERROR: trailing whitespace
- WARNING: space prohibited before semicolon
- WARNING: unnecessary whitespace before a quoted newline
Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c72d9d6..ff2b26b 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -378,7 +378,7 @@ static int lynxfb_ops_set_par(struct fb_info *info)
var->red.length = 8;
var->green.offset = 8;
var->green.length = 8;
- var->blue.offset = 0 ;
+ var->blue.offset = 0;
var->blue.length = 8;
fix->visual = FB_VISUAL_TRUECOLOR;
break;
@@ -591,7 +591,7 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
var->red.length = 8;
var->green.offset = 8;
var->green.length = 8;
- var->blue.offset = 0 ;
+ var->blue.offset = 0;
var->blue.length = 8;
info->fix.visual = FB_VISUAL_TRUECOLOR;
break;
@@ -722,7 +722,7 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
output->proc_setBLANK = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_setBLANK:hw_sm750_setBLANK;
output->clear = hw_sm750_output_clear;
/* chip specific phase */
- share->accel.de_wait = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_deWait: hw_sm750_deWait;
+ share->accel.de_wait = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_deWait : hw_sm750_deWait;
switch (spec_share->state.dataflow) {
case sm750_simul_pri:
output->paths = sm750_pnc;
@@ -772,7 +772,7 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
return ret;
}
-static struct fb_ops lynxfb_ops={
+static struct fb_ops lynxfb_ops = {
.owner = THIS_MODULE,
.fb_check_var = lynxfb_ops_check_var,
.fb_set_par = lynxfb_ops_set_par,
@@ -800,7 +800,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
lynx750_ext, NULL, vesa_modes,
};
int cdb[] = {ARRAY_SIZE(lynx750_ext), 0, VESA_MODEDB_SIZE};
- static const char *mdb_desc[] ={
+ static const char *mdb_desc[] = {
"driver prepared modes",
"kernel prepared default modedb",
"kernel HELPERS prepared vesa_modes",
@@ -869,7 +869,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
}
- for (i=0;i<3;i++) {
+ for (i = 0; i < 3; i++) {
ret = fb_find_mode(var, info, g_fbmode[index],
pdb[i], cdb[i], NULL, 8);
@@ -980,7 +980,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
goto exit;
}
- pr_debug("#2 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
+ pr_debug("#2 show info->cmap :\nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
info->cmap.start, info->cmap.len,
info->cmap.red, info->cmap.green, info->cmap.blue,
info->cmap.transp);
@@ -1314,7 +1314,7 @@ static int __init lynxfb_setup(char *options)
strsep() updates @options to pointer after the first found token
it also returns the pointer ahead the token.
*/
- while ((opt = strsep(&options, ":"))!=NULL) {
+ while ((opt = strsep(&options, ":")) != NULL) {
/* options that mean for any lynx chips are configured here */
if (!strncmp(opt, "noaccel", strlen("noaccel")))
g_noaccel = 1;
@@ -1360,7 +1360,7 @@ static struct pci_driver lynxfb_driver = {
static int __init lynxfb_init(void)
{
- char *option ;
+ char *option;
int ret;
#ifdef MODULE
--
2.3.3
^ permalink raw reply related
* [PATCH v2 11/12] staging: sm750: fix whitspace for cast to pointer
From: Michel von Czettritz @ 2015-03-26 22:27 UTC (permalink / raw)
To: linux-fbdev
This patch fixes the checkpatch.pl warning:
'ERROR: "(foo*)" should be "(foo *)"'
Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index ff2b26b..d363f55 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -202,7 +202,7 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
- color = (Bpp = 1)?region->color:((u32*)info->pseudo_palette)[region->color];
+ color = (Bpp = 1)?region->color:((u32 *)info->pseudo_palette)[region->color];
rop = (region->rop != ROP_COPY) ? HW_ROP2_XOR:HW_ROP2_COPY;
/*
@@ -271,8 +271,8 @@ static void lynxfb_ops_imageblit(struct fb_info *info,
if (image->depth = 1) {
if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
- fgcol = ((u32*)info->pseudo_palette)[image->fg_color];
- bgcol = ((u32*)info->pseudo_palette)[image->bg_color];
+ fgcol = ((u32 *)info->pseudo_palette)[image->fg_color];
+ bgcol = ((u32 *)info->pseudo_palette)[image->bg_color];
} else {
fgcol = image->fg_color;
bgcol = image->bg_color;
--
2.3.3
^ permalink raw reply related
* [PATCH v2 12/12] staging: sm750: move assignment out of if cond
From: Michel von Czettritz @ 2015-03-26 22:27 UTC (permalink / raw)
To: linux-fbdev
This patch moves the assignments from the if conditions to the line before the condition.
The 3 occurrence are return values and the checks for errors.
Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
v2: remove double negation in if statments
---
drivers/staging/sm750fb/sm750.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index d363f55..25df813 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -477,7 +477,8 @@ static int lynxfb_resume(struct pci_dev *pdev)
console_lock();
- if ((ret = pci_set_power_state(pdev, PCI_D0)) != 0) {
+ ret = pci_set_power_state(pdev, PCI_D0);
+ if (ret) {
pr_err("error:%d occured in pci_set_power_state\n", ret);
return ret;
}
@@ -485,7 +486,8 @@ static int lynxfb_resume(struct pci_dev *pdev)
if (pdev->dev.power.power_state.event != PM_EVENT_FREEZE) {
pci_restore_state(pdev);
- if ((ret = pci_enable_device(pdev)) != 0) {
+ ret = pci_enable_device(pdev);
+ if (ret) {
pr_err("error:%d occured in pci_enable_device\n", ret);
return ret;
}
@@ -975,7 +977,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
info->cmap.red, info->cmap.green, info->cmap.blue,
info->cmap.transp);
- if ((ret = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
+ ret = fb_alloc_cmap(&info->cmap, 256, 0);
+ if (ret < 0) {
pr_err("Could not allcate memory for cmap.\n");
goto exit;
}
--
2.3.3
^ permalink raw reply related
* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Luis R. Rodriguez @ 2015-03-26 23:35 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Dave Hansen, Stefan Bader, ville.syrjala, david.vrabel,
toshi.kani, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <20150325195941.GM25884@l.oracle.com>
On Wed, Mar 25, 2015 at 03:59:41PM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 20, 2015 at 04:17:52PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > It is possible to enable CONFIG_MTRR and up with it
> > disabled at run time and yet CONFIG_X86_PAT continues
> > to kick through fully functionally. This can happen
>
> s/fully/full/ ?
I'll rephrase this to:
---
It is possible to enable CONFIG_MTRR and up with it
disabled at run time and yet CONFIG_X86_PAT continues
to kick through with all functionally enabled. This
can happen for instance on Xen where MTRR is not
supported but PAT is, this can happen now on Linux as
of commit 47591df50 by Juergen introduced as of v3.19.
---
Which BTW I had also mentioned on the cover letter that
this is a good time to address if we want to make PAT
then a first class citizen, to detangle it from depending
on MTRR. If so I can do that later.
> > Technically we should assume the proper CPU
> > bits would be set to disable MTRR but we can't
> > always rely on this. At least on the Xen Hypervisor
> > for instance only X86_FEATURE_MTRR was disabled
> > as of Xen 4.4 through Xen commit 586ab6a [0],
> > but not X86_FEATURE_K6_MTRR, X86_FEATURE_CENTAUR_MCR,
> > or X86_FEATURE_CYRIX_ARR for instance.
>
> Oh, could you send an patch for that to Xen please?
Done.
> > x86 mtrr code relies on quite a bit of checks for
> > mtrr_if being set to check to see if MTRR did get
> > set up, instead of using that lets provide a generic
> > setter which when set we know MTRR is enabled. This
>
> s/we know MTRR is enabled/will let us know that MTRR is enabled/
Amended.
Luis
^ permalink raw reply
* Re: [PATCH v2 06/12] staging: sm750: no braces for single stmt if blocks
From: Dan Carpenter @ 2015-03-27 6:00 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <9b7d1ab21e735024fec2d2586fb1e0550eaa6369.1427406762.git.michel.von.czettritz@gmail.com>
On Thu, Mar 26, 2015 at 11:25:50PM +0100, Michel von Czettritz wrote:
> @@ -487,8 +484,8 @@ static int lynxfb_resume(struct pci_dev *pdev)
> par = info->par;
> crtc = &par->crtc;
> cursor = &crtc->cursor;
> - memset_io(cursor->vstart, 0x0, cursor->size);
> - memset_io(crtc->vScreen, 0x0, crtc->vidmem_size);
> + memset(cursor->vstart, 0x0, cursor->size);
> + memset(crtc->vScreen, 0x0, crtc->vidmem_size);
> lynxfb_ops_set_par(info);
> fb_set_suspend(info, 0);
> }
This is a bug.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Ville Syrjälä @ 2015-03-27 8:37 UTC (permalink / raw)
To: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Luis R. Rodriguez, Ingo Molnar,
Linus Torvalds, Daniel Vetter, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <20150321091514.GA22926@sci.fi>
On Sat, Mar 21, 2015 at 11:15:14AM +0200, Ville Syrjälä wrote:
> On Fri, Mar 20, 2015 at 04:17:59PM -0700, Luis R. Rodriguez wrote:
> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> > index 8025624..8875e56 100644
> > --- a/drivers/video/fbdev/aty/atyfb_base.c
> > +++ b/drivers/video/fbdev/aty/atyfb_base.c
> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
> >
> > #ifdef CONFIG_MTRR
> > par->mtrr_aper = -1;
> > - par->mtrr_reg = -1;
> > if (!nomtrr) {
> > - /* Cover the whole resource. */
> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
> > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
> > + info->fix.smem_len,
> > MTRR_TYPE_WRCOMB, 1);
>
> MTRRs need power of two size, so how is this supposed to work?
Still waiting for an answer...
>
> > - if (par->mtrr_aper >= 0 && !par->aux_start) {
> > - /* Make a hole for mmio. */
> > - par->mtrr_reg = mtrr_add(par->res_start + 0x800000 -
> > - GUI_RESERVE, GUI_RESERVE,
> > - MTRR_TYPE_UNCACHABLE, 1);
> > - if (par->mtrr_reg < 0) {
> > - mtrr_del(par->mtrr_aper, 0, 0);
> > - par->mtrr_aper = -1;
> > - }
> > - }
> > }
> > #endif
> >
> > @@ -2776,10 +2765,6 @@ aty_init_exit:
> > par->pll_ops->set_pll(info, &par->saved_pll);
> >
> > #ifdef CONFIG_MTRR
> > - if (par->mtrr_reg >= 0) {
> > - mtrr_del(par->mtrr_reg, 0, 0);
> > - par->mtrr_reg = -1;
> > - }
> > if (par->mtrr_aper >= 0) {
> > mtrr_del(par->mtrr_aper, 0, 0);
> > par->mtrr_aper = -1;
> > @@ -3466,7 +3451,7 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
> > }
> >
> > info->fix.mmio_start = raddr;
> > - par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000);
> > + par->ati_regbase = ioremap_nocache(info->fix.mmio_start, 0x1000);
> > if (par->ati_regbase = NULL)
> > return -ENOMEM;
> >
> > @@ -3491,6 +3476,8 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
> > info->fix.smem_start = addr;
> > info->fix.smem_len = 0x800000;
> >
> > + aty_fudge_framebuffer_len(info);
> > +
> > info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
> > if (info->screen_base = NULL) {
> > ret = -ENOMEM;
> > @@ -3563,6 +3550,7 @@ static int atyfb_pci_probe(struct pci_dev *pdev,
> > return -ENOMEM;
> > }
> > par = info->par;
> > + par->bus_type = PCI;
> > info->fix = atyfb_fix;
> > info->device = &pdev->dev;
> > par->pci_id = pdev->device;
> > @@ -3732,10 +3720,6 @@ static void atyfb_remove(struct fb_info *info)
> > #endif
> >
> > #ifdef CONFIG_MTRR
> > - if (par->mtrr_reg >= 0) {
> > - mtrr_del(par->mtrr_reg, 0, 0);
> > - par->mtrr_reg = -1;
> > - }
> > if (par->mtrr_aper >= 0) {
> > mtrr_del(par->mtrr_aper, 0, 0);
> > par->mtrr_aper = -1;
> > --
> > 2.3.2.209.gd67f9d5.dirty
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
--
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
^ permalink raw reply
* [RFT PATCH] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Krzysztof Kozlowski @ 2015-03-27 15:57 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, David Airlie, Kukjin Kim, dri-devel,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-fbdev
Cc: Marek Szyprowski, Andrzej Hajda, Javier Martinez Canillas,
Krzysztof Kozlowski, stable
After adding display power domain for Exynos5250 in commit
2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
display on Chromebook Snow and others stopped working after boot.
The reason for this suggested Andrzej Hajda: the DP clock was disabled.
This clock is required by Display Port and is enabled by bootloader.
However when FIMD driver probing was deferred, the display power domain
was turned off. This effectively reset the value of DP clock enable
register.
When exynos-dp is later probed, the clock is not enabled and display is
not properly configured:
exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
exynos-dp 145b0000.dp-controller: unable to config video
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
Cc: <stable@vger.kernel.org>
---
This should fix issue reported by Javier [1][2].
Tested on Chromebook Snow (Exynos 5250). More testing would be great,
especially on other Exynos 5xxx products.
[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/43889
[2] http://thread.gmane.org/gmane.linux.ports.arm.kernel/400290
---
drivers/gpu/drm/exynos/exynos_dp_core.c | 10 ++++++++++
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 19 +++++++++++++++++++
include/video/samsung_fimd.h | 6 ++++++
3 files changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index bf17a60b40ed..1dbfba58f909 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -32,10 +32,16 @@
#include <drm/bridge/ptn3460.h>
#include "exynos_dp_core.h"
+#include "exynos_drm_fimd.h"
#define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \
connector)
+static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
+{
+ return to_exynos_crtc(dp->encoder->crtc);
+}
+
static inline struct exynos_dp_device *
display_to_dp(struct exynos_drm_display *d)
{
@@ -1070,6 +1076,8 @@ static void exynos_dp_poweron(struct exynos_dp_device *dp)
}
}
+ fimd_dp_clock_enable(dp_to_crtc(dp), true);
+
clk_prepare_enable(dp->clock);
exynos_dp_phy_init(dp);
exynos_dp_init_dp(dp);
@@ -1094,6 +1102,8 @@ static void exynos_dp_poweroff(struct exynos_dp_device *dp)
exynos_dp_phy_exit(dp);
clk_disable_unprepare(dp->clock);
+ fimd_dp_clock_enable(dp_to_crtc(dp), false);
+
if (dp->panel) {
if (drm_panel_unprepare(dp->panel))
DRM_ERROR("failed to turnoff the panel\n");
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index c300e22da8ac..bdf0818dc8f5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -32,6 +32,7 @@
#include "exynos_drm_fbdev.h"
#include "exynos_drm_crtc.h"
#include "exynos_drm_iommu.h"
+#include "exynos_drm_fimd.h"
/*
* FIMD stands for Fully Interactive Mobile Display and
@@ -1231,6 +1232,24 @@ static int fimd_remove(struct platform_device *pdev)
return 0;
}
+void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
+{
+ struct fimd_context *ctx = crtc->ctx;
+ u32 val;
+
+ /*
+ * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
+ * clock. On these SoCs the bootloader may enable it but any
+ * power domain off/on will reset it to disable state.
+ */
+ if (ctx->driver_data != &exynos5_fimd_driver_data)
+ return;
+
+ val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
+ writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
+}
+EXPORT_SYMBOL_GPL(fimd_dp_clock_enable);
+
struct platform_driver fimd_driver = {
.probe = fimd_probe,
.remove = fimd_remove,
diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
index a20e4a3a8b15..847a0a2b399c 100644
--- a/include/video/samsung_fimd.h
+++ b/include/video/samsung_fimd.h
@@ -436,6 +436,12 @@
#define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0)
#define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Display port clock control */
+#define DP_MIE_CLKCON 0x27c
+#define DP_MIE_CLK_DISABLE 0x0
+#define DP_MIE_CLK_DP_ENABLE 0x2
+#define DP_MIE_CLK_MIE_ENABLE 0x3
+
/* Notes on per-window bpp settings
*
* Value Win0 Win1 Win2 Win3 Win 4
--
1.9.1
^ permalink raw reply related
* [RFT PATCHv2] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Krzysztof Kozlowski @ 2015-03-27 16:08 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, David Airlie, Kukjin Kim, dri-devel,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-fbdev
Cc: Marek Szyprowski, Andrzej Hajda, Javier Martinez Canillas,
Krzysztof Kozlowski, stable
After adding display power domain for Exynos5250 in commit
2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
display on Chromebook Snow and others stopped working after boot.
The reason for this suggested Andrzej Hajda: the DP clock was disabled.
This clock is required by Display Port and is enabled by bootloader.
However when FIMD driver probing was deferred, the display power domain
was turned off. This effectively reset the value of DP clock enable
register.
When exynos-dp is later probed, the clock is not enabled and display is
not properly configured:
exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
exynos-dp 145b0000.dp-controller: unable to config video
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
Cc: <stable@vger.kernel.org>
---
This should fix issue reported by Javier [1][2].
Tested on Chromebook Snow (Exynos 5250). More testing would be great,
especially on other Exynos 5xxx products.
[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/43889
[2] http://thread.gmane.org/gmane.linux.ports.arm.kernel/400290
Changes since v1:
1. Added missing exynos_drm_fimd.h.
---
drivers/gpu/drm/exynos/exynos_dp_core.c | 10 ++++++++++
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 19 +++++++++++++++++++
drivers/gpu/drm/exynos/exynos_drm_fimd.h | 15 +++++++++++++++
include/video/samsung_fimd.h | 6 ++++++
4 files changed, 50 insertions(+)
create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimd.h
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index bf17a60b40ed..1dbfba58f909 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -32,10 +32,16 @@
#include <drm/bridge/ptn3460.h>
#include "exynos_dp_core.h"
+#include "exynos_drm_fimd.h"
#define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \
connector)
+static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
+{
+ return to_exynos_crtc(dp->encoder->crtc);
+}
+
static inline struct exynos_dp_device *
display_to_dp(struct exynos_drm_display *d)
{
@@ -1070,6 +1076,8 @@ static void exynos_dp_poweron(struct exynos_dp_device *dp)
}
}
+ fimd_dp_clock_enable(dp_to_crtc(dp), true);
+
clk_prepare_enable(dp->clock);
exynos_dp_phy_init(dp);
exynos_dp_init_dp(dp);
@@ -1094,6 +1102,8 @@ static void exynos_dp_poweroff(struct exynos_dp_device *dp)
exynos_dp_phy_exit(dp);
clk_disable_unprepare(dp->clock);
+ fimd_dp_clock_enable(dp_to_crtc(dp), false);
+
if (dp->panel) {
if (drm_panel_unprepare(dp->panel))
DRM_ERROR("failed to turnoff the panel\n");
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index c300e22da8ac..bdf0818dc8f5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -32,6 +32,7 @@
#include "exynos_drm_fbdev.h"
#include "exynos_drm_crtc.h"
#include "exynos_drm_iommu.h"
+#include "exynos_drm_fimd.h"
/*
* FIMD stands for Fully Interactive Mobile Display and
@@ -1231,6 +1232,24 @@ static int fimd_remove(struct platform_device *pdev)
return 0;
}
+void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
+{
+ struct fimd_context *ctx = crtc->ctx;
+ u32 val;
+
+ /*
+ * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
+ * clock. On these SoCs the bootloader may enable it but any
+ * power domain off/on will reset it to disable state.
+ */
+ if (ctx->driver_data != &exynos5_fimd_driver_data)
+ return;
+
+ val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
+ writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
+}
+EXPORT_SYMBOL_GPL(fimd_dp_clock_enable);
+
struct platform_driver fimd_driver = {
.probe = fimd_probe,
.remove = fimd_remove,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.h b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
new file mode 100644
index 000000000000..b4fcaa568456
--- /dev/null
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#ifndef _EXYNOS_DRM_FIMD_H_
+#define _EXYNOS_DRM_FIMD_H_
+
+extern void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable);
+
+#endif /* _EXYNOS_DRM_FIMD_H_ */
diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
index a20e4a3a8b15..847a0a2b399c 100644
--- a/include/video/samsung_fimd.h
+++ b/include/video/samsung_fimd.h
@@ -436,6 +436,12 @@
#define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0)
#define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
+/* Display port clock control */
+#define DP_MIE_CLKCON 0x27c
+#define DP_MIE_CLK_DISABLE 0x0
+#define DP_MIE_CLK_DP_ENABLE 0x2
+#define DP_MIE_CLK_MIE_ENABLE 0x3
+
/* Notes on per-window bpp settings
*
* Value Win0 Win1 Win2 Win3 Win 4
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v1 05/47] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-03-27 18:40 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Ingo Molnar, Daniel Vetter,
Bjorn Helgaas, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Dave Hansen, Arnd Bergmann, Michael S. Tsirkin,
Stefan Bader, ville.syrjala, david.vrabel, toshi.kani,
Roger Pau Monné, xen-devel
In-Reply-To: <20150325200743.GP25884@l.oracle.com>
On Wed, Mar 25, 2015 at 04:07:43PM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 20, 2015 at 04:17:55PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > This allows drivers to take advantage of write-combining
> > when possible. Ideally we'd have pci_read_bases() just
> > peg an IORESOURCE_WC flag for us but where exactly
> > video devices memory lie varies *largely* and at times things
> > are mixed with MMIO registers, sometimes we can address
> > the changes in drivers, other times the change requires
> > intrusive changes.
> >
> > Although there is also arch_phys_wc_add() that makes use of
> > architecture specific write-combinging alternatives (MTRR on
>
> combinging?
Amended.
> > diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
> > index bcce5f1..30b65ae 100644
> > --- a/lib/pci_iomap.c
> > +++ b/lib/pci_iomap.c
> > @@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev,
> > EXPORT_SYMBOL(pci_iomap_range);
> >
> > /**
> > + * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR
> > + * @dev: PCI device that owns the BAR
> > + * @bar: BAR number
> > + * @offset: map memory at the given offset in BAR
> > + * @maxlen: max length of the memory to map
> > + *
> > + * Using this function you will get a __iomem address to your device BAR.
> > + * You can access it using ioread*() and iowrite*(). These functions hide
> > + * the details if this is a MMIO or PIO address space and will just do what
> > + * you expect from them in the correct way. When possible write combining
> > + * is used.
> > + *
> > + * @maxlen specifies the maximum length to map. If you want to get access to
> > + * the complete BAR from offset to the end, pass %0 here.
>
> s/%0/0 ? Or is that some special syntax?
This copies the syntax of pci_iomap_range() which also uses %0, and as per
Documentation/kernel-doc-nano-HOWTO.txt % is used for constants. See:
scripts/kernel-doc -man -function pci_iomap_range lib/pci_iomap.c | nroff -man | less
Luis
^ permalink raw reply
* [PATCH v3 06/11] staging: sm750: no braces for single stmt if blocks
From: Michel von Czettritz @ 2015-03-27 18:42 UTC (permalink / raw)
To: linux-fbdev
This patches fixes checkpatch.pl warning:
"WARNING: braces {} are not necessary for single statement blocks"
Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
v3: fix wrong merging
---
drivers/staging/sm750fb/sm750.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index d9a94d7..786679d 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -131,9 +131,8 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
}
cursor->disable(cursor);
- if (fbcursor->set & FB_CUR_SETSIZE) {
+ if (fbcursor->set & FB_CUR_SETSIZE)
cursor->setSize(cursor, fbcursor->image.width, fbcursor->image.height);
- }
if (fbcursor->set & FB_CUR_SETPOS) {
cursor->setPos(cursor, fbcursor->image.dx - info->var.xoffset,
@@ -162,9 +161,8 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
fbcursor->mask);
}
- if (fbcursor->enable) {
+ if (fbcursor->enable)
cursor->enable(cursor);
- }
return 0;
}
@@ -176,9 +174,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info, const struct fb_fillrect *
unsigned int base, pitch, Bpp, rop;
u32 color;
- if (info->state != FBINFO_STATE_RUNNING) {
+ if (info->state != FBINFO_STATE_RUNNING)
return;
- }
par = info->par;
share = par->share;
--
2.3.3
^ permalink raw reply related
* Re: [PATCH v1 05/47] pci: add pci_iomap_wc() variants
From: Toshi Kani @ 2015-03-27 19:18 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Luis R. Rodriguez, Andy Lutomirski, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, jgross, Jan Beulich, Borislav Petkov,
Suresh Siddha, venkatesh.pallipadi, Dave Airlie,
linux-kernel@vger.kernel.org, linux-fbdev, x86@kernel.org,
xen-devel@lists.xenproject.org, Luis R. Rodriguez, Ingo Molnar,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Dave Hansen, Arnd Bergmann, Michael S. Tsirkin,
Stefan Bader, Konrad Rzeszutek Wilk, Ville Syrjälä,
David Vrabel, Roger Pau Monné, xen-devel
In-Reply-To: <CAErSpo5LfgTM879jmt58yLrXD6e7spsaX1wNDcc2ppU9JrMXOg@mail.gmail.com>
On Mon, 2015-03-23 at 12:20 -0500, Bjorn Helgaas wrote:
:
> pci_iomap_range() already makes a cacheable mapping if
> IORESOURCE_CACHEABLE; I'm guessing that you would like it to
> automatically use WC if the BAR if IORESOURCE_PREFETCH, e.g.,
>
> if (flags & IORESOURCE_CACHEABLE)
> return ioremap(start, len);
Is this supposed to be ioremap_cache()? ioremap() is the same as
ioremap_nocache() at least on x86 per arch/x86/include/asm/io.h.
> if (flags & IORESOURCE_PREFETCH)
> return ioremap_wc(start, len);
> return ioremap_nocache(start, len);
>
-Toshi
^ permalink raw reply
* Re: [RFT PATCHv2] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Javier Martinez Canillas @ 2015-03-27 19:21 UTC (permalink / raw)
To: Krzysztof Kozlowski, Jingoo Han, Inki Dae, Joonyoung Shim,
Seung-Woo Kim, Kyungmin Park, David Airlie, Kukjin Kim, dri-devel,
linux-arm-kernel, linux-samsung-soc, linux-kernel, linux-fbdev
Cc: Marek Szyprowski, Andrzej Hajda, stable
In-Reply-To: <1427472488-21454-1-git-send-email-k.kozlowski@samsung.com>
Hello Krzysztof,
On 03/27/2015 05:08 PM, Krzysztof Kozlowski wrote:
> After adding display power domain for Exynos5250 in commit
> 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
> display on Chromebook Snow and others stopped working after boot.
>
> The reason for this suggested Andrzej Hajda: the DP clock was disabled.
> This clock is required by Display Port and is enabled by bootloader.
> However when FIMD driver probing was deferred, the display power domain
> was turned off. This effectively reset the value of DP clock enable
> register.
>
> When exynos-dp is later probed, the clock is not enabled and display is
> not properly configured:
>
> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
> exynos-dp 145b0000.dp-controller: unable to config video
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
> Cc: <stable@vger.kernel.org>
>
> ---
>
> This should fix issue reported by Javier [1][2].
>
I tested this patch and does indeed solves both issues I reported
The exynos-dp probe deferral does not make the display to not be
working and also disabling and enabling the display with:
with /sys/devices/platform/exynos-drm/graphics/fb0/blank works.
Thanks a lot for fixing this issue.
On an Exynos5250 Snow Chromebook:
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Best regards,
Javier
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Luis R. Rodriguez @ 2015-03-27 19:38 UTC (permalink / raw)
To: Ville Syrjälä, Bjorn Helgaas, Luis R. Rodriguez,
Andy Lutomirski
Cc: mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
venkatesh.pallipadi, airlied, linux-kernel, linux-fbdev, x86,
xen-devel, Ingo Molnar, Linus Torvalds, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <20150321091514.GA22926@sci.fi>
On Sat, Mar 21, 2015 at 11:15:14AM +0200, Ville Syrjälä wrote:
> On Fri, Mar 20, 2015 at 04:17:59PM -0700, Luis R. Rodriguez wrote:
> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> > index 8025624..8875e56 100644
> > --- a/drivers/video/fbdev/aty/atyfb_base.c
> > +++ b/drivers/video/fbdev/aty/atyfb_base.c
> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
> >
> > #ifdef CONFIG_MTRR
> > par->mtrr_aper = -1;
> > - par->mtrr_reg = -1;
> > if (!nomtrr) {
> > - /* Cover the whole resource. */
> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
> > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
> > + info->fix.smem_len,
> > MTRR_TYPE_WRCOMB, 1);
>
> MTRRs need power of two size, so how is this supposed to work?
As per mtrr_add_page() [0] the base and size are just supposed to be in units
of 4 KiB, although the practice is to use powers of 2 in *some* drivers this
is not standardized and by no means recorded as a requirement. Obviously
powers of 2 will work too and you'd end up neatly aligned as well. mtrr_add()
will use mtrr_check() to verify the the same requirement. Furthermore,
as per my commit log message:
---
The last thing we do must do to remain sane is ensure we
use the info->fix.smem_start and info->fix.smem_len for
the framebuffer MTRR as we know that is always well adjusted.
The *one* concern here would be if the MTRR is not in units
of 4K __but__ we already know that in the PCI case this cannot
happen, in the shared space setting the MTRR would be up to
0x7ff000 and assuming a 4K page:
; 0x7ff000 / 0x1000
2047
Also, internally when MTRR is used mtrr_add() will use mtrr_check()
and that should splat a warning when the MTRR base and size are
not compatible with what is expected for MTRR usage.
---
If any of this is too risky we can use the __arch_phys_wc_add() (or as
Andy suggested perhaps use set_page_* stuff, although I am still evaluating
this) but I did this change to show the effort required for a change when
the registers / framebuffer is on the same PCI BAR but at different offsets.
[0] scripts/kernel-doc -man -function mtrr_add_page arch/x86/kernel/cpu/mtrr/main.c | nroff -man | less
Luis
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Luis R. Rodriguez @ 2015-03-27 19:38 UTC (permalink / raw)
To: Ville Syrjälä, Luis R. Rodriguez, luto, mingo, tglx,
hpa, jgross, JBeulich, bp, suresh.b.siddha, venkatesh.pallipadi,
airlied, linux-kernel, linux-fbdev, x86, xen-devel, Ingo Molnar,
Linus Torvalds, Daniel Vetter, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <20150327083703.GB22926@sci.fi>
On Fri, Mar 27, 2015 at 10:37:04AM +0200, Ville Syrjälä wrote:
> On Sat, Mar 21, 2015 at 11:15:14AM +0200, Ville Syrjälä wrote:
> > On Fri, Mar 20, 2015 at 04:17:59PM -0700, Luis R. Rodriguez wrote:
> > > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> > > index 8025624..8875e56 100644
> > > --- a/drivers/video/fbdev/aty/atyfb_base.c
> > > +++ b/drivers/video/fbdev/aty/atyfb_base.c
> > > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
> > >
> > > #ifdef CONFIG_MTRR
> > > par->mtrr_aper = -1;
> > > - par->mtrr_reg = -1;
> > > if (!nomtrr) {
> > > - /* Cover the whole resource. */
> > > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
> > > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
> > > + info->fix.smem_len,
> > > MTRR_TYPE_WRCOMB, 1);
> >
> > MTRRs need power of two size, so how is this supposed to work?
>
> Still waiting for an answer...
Sorry was in the desert for a bit, I'm back now.
Luis
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Andy Lutomirski @ 2015-03-27 19:43 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Ville Syrjälä, Bjorn Helgaas, Luis R. Rodriguez,
Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Juergen Gross,
Jan Beulich, Borislav Petkov, Suresh Siddha, venkatesh.pallipadi,
Dave Airlie, linux-kernel@vger.kernel.org,
Linux Fbdev development list, X86 ML,
xen-devel@lists.xenproject.org, Ingo Molnar, Linus Torvalds,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <20150327193813.GH5622@wotan.suse.de>
On Fri, Mar 27, 2015 at 12:38 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Sat, Mar 21, 2015 at 11:15:14AM +0200, Ville Syrjälä wrote:
>> On Fri, Mar 20, 2015 at 04:17:59PM -0700, Luis R. Rodriguez wrote:
>> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
>> > index 8025624..8875e56 100644
>> > --- a/drivers/video/fbdev/aty/atyfb_base.c
>> > +++ b/drivers/video/fbdev/aty/atyfb_base.c
>> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
>> >
>> > #ifdef CONFIG_MTRR
>> > par->mtrr_aper = -1;
>> > - par->mtrr_reg = -1;
>> > if (!nomtrr) {
>> > - /* Cover the whole resource. */
>> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
>> > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
>> > + info->fix.smem_len,
>> > MTRR_TYPE_WRCOMB, 1);
>>
>> MTRRs need power of two size, so how is this supposed to work?
>
> As per mtrr_add_page() [0] the base and size are just supposed to be in units
> of 4 KiB, although the practice is to use powers of 2 in *some* drivers this
> is not standardized and by no means recorded as a requirement. Obviously
> powers of 2 will work too and you'd end up neatly aligned as well. mtrr_add()
> will use mtrr_check() to verify the the same requirement. Furthermore,
> as per my commit log message:
Whatever the code may or may not do, the x86 architecture uses
power-of-two MTRR sizes. So I'm confused.
--Andy
>
> ---
> The last thing we do must do to remain sane is ensure we
> use the info->fix.smem_start and info->fix.smem_len for
> the framebuffer MTRR as we know that is always well adjusted.
> The *one* concern here would be if the MTRR is not in units
> of 4K __but__ we already know that in the PCI case this cannot
> happen, in the shared space setting the MTRR would be up to
> 0x7ff000 and assuming a 4K page:
>
> ; 0x7ff000 / 0x1000
> 2047
>
> Also, internally when MTRR is used mtrr_add() will use mtrr_check()
> and that should splat a warning when the MTRR base and size are
> not compatible with what is expected for MTRR usage.
> ---
>
> If any of this is too risky we can use the __arch_phys_wc_add() (or as
> Andy suggested perhaps use set_page_* stuff, although I am still evaluating
> this) but I did this change to show the effort required for a change when
> the registers / framebuffer is on the same PCI BAR but at different offsets.
>
> [0] scripts/kernel-doc -man -function mtrr_add_page arch/x86/kernel/cpu/mtrr/main.c | nroff -man | less
>
> Luis
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-03-27 19:53 UTC (permalink / raw)
To: Andy Lutomirski, Bjorn Helgaas, Ville Syrjälä,
Mauro Carvalho Chehab, Mike Marciniszyn
Cc: Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
Juergen Gross, Jan Beulich, Borislav Petkov, Suresh Siddha,
venkatesh.pallipadi, Dave Airlie, linux-kernel@vger.kernel.org,
Linux Fbdev development list, X86 ML,
xen-devel@lists.xenproject.org, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <CALCETrWJzOCqUOg2oAzTeAaURwkjCfZdPyT--mTDsf4aatJoDw@mail.gmail.com>
On Fri, Mar 20, 2015 at 04:48:46PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > Ideally on systems using PAT we can expect a swift
> > transition away from MTRR. There can be a few exceptions
> > to this, one is where device drivers are known to exist
> > on PATs with errata, another situation is observed on
> > old device drivers where devices had combined MMIO
> > register access with whatever area they typically
> > later wanted to end up using MTRR for on the same
> > PCI BAR. This situation can still be addressed by
> > splitting up ioremap'd PCI BAR into two ioremap'd
> > calls, one for MMIO registers, and another for whatever
> > is desirable for write-combining -- in order to
> > accomplish this though quite a bit of driver
> > restructuring is required.
> >
> > Device drivers which are known to require large
> > amount of re-work in order to split ioremap'd areas
> > can use __arch_phys_wc_add() to avoid regressions
> > when PAT is enabled.
> >
> > For a good example driver where things are neatly
> > split up on a PCI BAR refer the infiniband qib
> > driver. For a good example of a driver where good
> > amount of work is required refer to the infiniband
> > ipath driver.
> >
> > This is *only* a transitive API -- and as such no new
> > drivers are ever expected to use this.
>
> What's the exact layout that this helps? I'm sceptical that this can
> ever be correct.
>
> Is there some awful driver that has a large ioremap that's supposed to
> contain multiple different memtypes?
Yes, I cc'd you just now on one where I made changes on a driver which uses one
PCI with mixed memtypes and uses MTRR to hole in WC. A transition to
arch_phys_wc_add() is therefore not possible if PAT is enabled as it would
regress those drivers by making the MTRR WC hole trick non functional.
The changes are non trivial and so in this series I supplied changes on
one driver only to show the effort required. The other drivers which
required this were:
Driver File
------------------------------------------------------------
fusion drivers/message/fusion/mptbase.c
ivtv drivers/media/pci/ivtv/ivtvfb.c
ipath drivers/infiniband/hw/ipath/ipath_driver.c
This series makes those drivers use __arch_phys_wc_add() more as a
transitory phase in hopes we can address the proper split as with the
atyfb illustrates. For ipath the changes required have a nice template
with the qib driver as they share very similar driver structure, the
qib driver *did* do the nice split.
> If so, can we ioremap + set_page_xyz instead?
I'm not sure I see which call we'd use. Care to provide an example patch
alternative for the atyfb as a case in point alternative to the work required
to do the split?
Luis
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Luis R. Rodriguez @ 2015-03-27 19:57 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Ville Syrjälä, Bjorn Helgaas, Luis R. Rodriguez,
Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Juergen Gross,
Jan Beulich, Borislav Petkov, Suresh Siddha, venkatesh.pallipadi,
Dave Airlie, linux-kernel@vger.kernel.org,
Linux Fbdev development list, X86 ML,
xen-devel@lists.xenproject.org, Ingo Molnar, Linus Torvalds,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <CALCETrVfC8aGKuPvaqv3edBADnKKvgDRiKbDvaaOr-HLB7jijg@mail.gmail.com>
On Fri, Mar 27, 2015 at 12:43:55PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 27, 2015 at 12:38 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > On Sat, Mar 21, 2015 at 11:15:14AM +0200, Ville Syrjälä wrote:
> >> On Fri, Mar 20, 2015 at 04:17:59PM -0700, Luis R. Rodriguez wrote:
> >> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> >> > index 8025624..8875e56 100644
> >> > --- a/drivers/video/fbdev/aty/atyfb_base.c
> >> > +++ b/drivers/video/fbdev/aty/atyfb_base.c
> >> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
> >> >
> >> > #ifdef CONFIG_MTRR
> >> > par->mtrr_aper = -1;
> >> > - par->mtrr_reg = -1;
> >> > if (!nomtrr) {
> >> > - /* Cover the whole resource. */
> >> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
> >> > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
> >> > + info->fix.smem_len,
> >> > MTRR_TYPE_WRCOMB, 1);
> >>
> >> MTRRs need power of two size, so how is this supposed to work?
> >
> > As per mtrr_add_page() [0] the base and size are just supposed to be in units
> > of 4 KiB, although the practice is to use powers of 2 in *some* drivers this
> > is not standardized and by no means recorded as a requirement. Obviously
> > powers of 2 will work too and you'd end up neatly aligned as well. mtrr_add()
> > will use mtrr_check() to verify the the same requirement. Furthermore,
> > as per my commit log message:
>
> Whatever the code may or may not do, the x86 architecture uses
> power-of-two MTRR sizes. So I'm confused.
There should be no confusion, I simply did not know that *was* the
requirement for x86, if that is the case we should add a check for that
and perhaps generalize a helper that does the power of two helper changes,
the cleanest I found was the vesafb driver solution.
Thoughts?
Luis
^ permalink raw reply
* Re: [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Andy Lutomirski @ 2015-03-27 19:58 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Bjorn Helgaas, Ville Syrjälä, Mauro Carvalho Chehab,
Mike Marciniszyn, Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, Juergen Gross, Jan Beulich, Borislav Petkov,
Suresh Siddha, venkatesh.pallipadi, Dave Airlie,
linux-kernel@vger.kernel.org, Linux Fbdev development list,
X86 ML, xen-devel@lists.xenproject.org, Ingo Molnar,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <20150327195350.GJ5622@wotan.suse.de>
On Fri, Mar 27, 2015 at 12:53 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Fri, Mar 20, 2015 at 04:48:46PM -0700, Andy Lutomirski wrote:
>> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>> >
>> > Ideally on systems using PAT we can expect a swift
>> > transition away from MTRR. There can be a few exceptions
>> > to this, one is where device drivers are known to exist
>> > on PATs with errata, another situation is observed on
>> > old device drivers where devices had combined MMIO
>> > register access with whatever area they typically
>> > later wanted to end up using MTRR for on the same
>> > PCI BAR. This situation can still be addressed by
>> > splitting up ioremap'd PCI BAR into two ioremap'd
>> > calls, one for MMIO registers, and another for whatever
>> > is desirable for write-combining -- in order to
>> > accomplish this though quite a bit of driver
>> > restructuring is required.
>> >
>> > Device drivers which are known to require large
>> > amount of re-work in order to split ioremap'd areas
>> > can use __arch_phys_wc_add() to avoid regressions
>> > when PAT is enabled.
>> >
>> > For a good example driver where things are neatly
>> > split up on a PCI BAR refer the infiniband qib
>> > driver. For a good example of a driver where good
>> > amount of work is required refer to the infiniband
>> > ipath driver.
>> >
>> > This is *only* a transitive API -- and as such no new
>> > drivers are ever expected to use this.
>>
>> What's the exact layout that this helps? I'm sceptical that this can
>> ever be correct.
>>
>> Is there some awful driver that has a large ioremap that's supposed to
>> contain multiple different memtypes?
>
> Yes, I cc'd you just now on one where I made changes on a driver which uses one
> PCI with mixed memtypes and uses MTRR to hole in WC. A transition to
> arch_phys_wc_add() is therefore not possible if PAT is enabled as it would
> regress those drivers by making the MTRR WC hole trick non functional.
> The changes are non trivial and so in this series I supplied changes on
> one driver only to show the effort required. The other drivers which
> required this were:
>
> Driver File
> ------------------------------------------------------------
> fusion drivers/message/fusion/mptbase.c
> ivtv drivers/media/pci/ivtv/ivtvfb.c
> ipath drivers/infiniband/hw/ipath/ipath_driver.c
>
> This series makes those drivers use __arch_phys_wc_add() more as a
> transitory phase in hopes we can address the proper split as with the
> atyfb illustrates. For ipath the changes required have a nice template
> with the qib driver as they share very similar driver structure, the
> qib driver *did* do the nice split.
>
>> If so, can we ioremap + set_page_xyz instead?
>
> I'm not sure I see which call we'd use. Care to provide an example patch
> alternative for the atyfb as a case in point alternative to the work required
> to do the split?
>
I'm still confused. Would it be insufficient to ioremap_nocache the
whole thing and then call set_memory_wc on parts of it? (Sorry,
set_page_xyz was a typo.)
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Luis R. Rodriguez @ 2015-03-27 20:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
Juergen Gross, Jan Beulich, Borislav Petkov, Suresh Siddha,
venkatesh.pallipadi, Dave Airlie, linux-kernel@vger.kernel.org,
Linux Fbdev development list, X86 ML,
xen-devel@lists.xenproject.org, Ingo Molnar, Linus Torvalds,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <CALCETrWXk0KVU__DXpGj4RSyY1K7dyjv2h5qJBrZT8psEyM-cQ@mail.gmail.com>
On Fri, Mar 20, 2015 at 04:52:18PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> > index 8025624..8875e56 100644
> > --- a/drivers/video/fbdev/aty/atyfb_base.c
> > +++ b/drivers/video/fbdev/aty/atyfb_base.c
> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
> >
> > #ifdef CONFIG_MTRR
> > par->mtrr_aper = -1;
> > - par->mtrr_reg = -1;
> > if (!nomtrr) {
> > - /* Cover the whole resource. */
> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
> > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
> > + info->fix.smem_len,
> > MTRR_TYPE_WRCOMB, 1);
> > - if (par->mtrr_aper >= 0 && !par->aux_start) {
> > - /* Make a hole for mmio. */
> > - par->mtrr_reg = mtrr_add(par->res_start + 0x800000 -
> > - GUI_RESERVE, GUI_RESERVE,
> > - MTRR_TYPE_UNCACHABLE, 1);
> > - if (par->mtrr_reg < 0) {
> > - mtrr_del(par->mtrr_aper, 0, 0);
> > - par->mtrr_aper = -1;
> > - }
> > - }
> > }
> > #endif
> >
> > @@ -2776,10 +2765,6 @@ aty_init_exit:
> > par->pll_ops->set_pll(info, &par->saved_pll);
> >
> > #ifdef CONFIG_MTRR
> > - if (par->mtrr_reg >= 0) {
> > - mtrr_del(par->mtrr_reg, 0, 0);
> > - par->mtrr_reg = -1;
> > - }
> > if (par->mtrr_aper >= 0) {
> > mtrr_del(par->mtrr_aper, 0, 0);
> > par->mtrr_aper = -1;
> > @@ -3466,7 +3451,7 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
> > }
> >
> > info->fix.mmio_start = raddr;
> > - par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000);
> > + par->ati_regbase = ioremap_nocache(info->fix.mmio_start, 0x1000);
>
> Double-check me, but I think that ioremap_nocache + WC MTRR = WC.
Precicely, in this case the WC hole was obtained by using MTRR WC. This
patch removes that WC hole trick and now we can be explciit about
only wanting ioremap_nocache() on the registers, that is WC is not
desired here and is not used. The patch does not highlight the fact
that there was left in place another ioremap() call for the framebuffer:
info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
That is the one that later after this patch we use ioremap_wc() for.
This patch just removes the hole solution. That's all.
Luis
^ permalink raw reply
* Re: [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-03-27 20:30 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Bjorn Helgaas, Ville Syrjälä, Mauro Carvalho Chehab,
Mike Marciniszyn, Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, Juergen Gross, Jan Beulich, Borislav Petkov,
Suresh Siddha, venkatesh.pallipadi, Dave Airlie,
linux-kernel@vger.kernel.org, Linux Fbdev development list,
X86 ML, xen-devel@lists.xenproject.org, Ingo Molnar,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <CALCETrU-EATXPNt8p21=1rsbAsWX-kJys1rwzYyZ2S1h2enVYw@mail.gmail.com>
On Fri, Mar 27, 2015 at 12:58:02PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 27, 2015 at 12:53 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > On Fri, Mar 20, 2015 at 04:48:46PM -0700, Andy Lutomirski wrote:
> >> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
> >> <mcgrof@do-not-panic.com> wrote:
> >> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >> >
> >> > Ideally on systems using PAT we can expect a swift
> >> > transition away from MTRR. There can be a few exceptions
> >> > to this, one is where device drivers are known to exist
> >> > on PATs with errata, another situation is observed on
> >> > old device drivers where devices had combined MMIO
> >> > register access with whatever area they typically
> >> > later wanted to end up using MTRR for on the same
> >> > PCI BAR. This situation can still be addressed by
> >> > splitting up ioremap'd PCI BAR into two ioremap'd
> >> > calls, one for MMIO registers, and another for whatever
> >> > is desirable for write-combining -- in order to
> >> > accomplish this though quite a bit of driver
> >> > restructuring is required.
> >> >
> >> > Device drivers which are known to require large
> >> > amount of re-work in order to split ioremap'd areas
> >> > can use __arch_phys_wc_add() to avoid regressions
> >> > when PAT is enabled.
> >> >
> >> > For a good example driver where things are neatly
> >> > split up on a PCI BAR refer the infiniband qib
> >> > driver. For a good example of a driver where good
> >> > amount of work is required refer to the infiniband
> >> > ipath driver.
> >> >
> >> > This is *only* a transitive API -- and as such no new
> >> > drivers are ever expected to use this.
> >>
> >> What's the exact layout that this helps? I'm sceptical that this can
> >> ever be correct.
> >>
> >> Is there some awful driver that has a large ioremap that's supposed to
> >> contain multiple different memtypes?
> >
> > Yes, I cc'd you just now on one where I made changes on a driver which uses one
> > PCI with mixed memtypes and uses MTRR to hole in WC. A transition to
> > arch_phys_wc_add() is therefore not possible if PAT is enabled as it would
> > regress those drivers by making the MTRR WC hole trick non functional.
> > The changes are non trivial and so in this series I supplied changes on
> > one driver only to show the effort required. The other drivers which
> > required this were:
> >
> > Driver File
> > ------------------------------------------------------------
> > fusion drivers/message/fusion/mptbase.c
> > ivtv drivers/media/pci/ivtv/ivtvfb.c
> > ipath drivers/infiniband/hw/ipath/ipath_driver.c
> >
> > This series makes those drivers use __arch_phys_wc_add() more as a
> > transitory phase in hopes we can address the proper split as with the
> > atyfb illustrates. For ipath the changes required have a nice template
> > with the qib driver as they share very similar driver structure, the
> > qib driver *did* do the nice split.
> >
> >> If so, can we ioremap + set_page_xyz instead?
> >
> > I'm not sure I see which call we'd use. Care to provide an example patch
> > alternative for the atyfb as a case in point alternative to the work required
> > to do the split?
> >
>
> I'm still confused. Would it be insufficient to ioremap_nocache the
> whole thing and then call set_memory_wc on parts of it? (Sorry,
> set_page_xyz was a typo.)
I think that would be a sexy alternative.
In this driver's case the thing is a bit messy as it not only used
the WC MTRR for a hole but it also then used a UC MTRR on top of
it all, so since I already tried to address the split, and if we address
the power of 2 woes, I think it'd be best to try to remove the UC MTRR
and just avoid set_page_wc() in this driver's case, but for the other cases
(fusion, ivtv, ipath) I think this makes sense.
Thoughts?
Luis
^ permalink raw reply
* Re: [PATCH v1 21/47] ethernet: myri10ge: use arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-03-27 20:36 UTC (permalink / raw)
To: Hyong-Youb Kim
Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Ingo Molnar, Daniel Vetter,
Hyong-Youb Kim, netdev, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <20150321070759.GB8766@gtest01.arcus>
On Sat, Mar 21, 2015 at 04:08:00PM +0900, Hyong-Youb Kim wrote:
> On Fri, Mar 20, 2015 at 04:18:11PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > This driver already uses ioremap_wc() on the same range
> > so when write-combining is available that will be used
> > instead.
> >
> [...]
> > --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> > +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> [...]
> > @@ -1984,7 +1979,6 @@ myri10ge_get_ethtool_stats(struct net_device *netdev,
> > data[i] = ((u64 *)&link_stats)[i];
> >
> > data[i++] = (unsigned int)mgp->tx_boundary;
> > - data[i++] = (unsigned int)mgp->wc_enabled;
> > data[i++] = (unsigned int)mgp->pdev->irq;
> > data[i++] = (unsigned int)mgp->msi_enabled;
> > data[i++] = (unsigned int)mgp->msix_enabled;
>
> You would have to delete "WC from myri10ge_gstrings_main_stats too.
> Something like below. Thanks.
>
> @@ -1905,7 +1905,7 @@ static const char myri10ge_gstrings_main_stats[][ETH_GSTRING_LEN] = {
> "tx_aborted_errors", "tx_carrier_errors", "tx_fifo_errors",
> "tx_heartbeat_errors", "tx_window_errors",
> /* device-specific stats */
> - "tx_boundary", "WC", "irq", "MSI", "MSIX",
> + "tx_boundary", "irq", "MSI", "MSIX",
> "read_dma_bw_MBs", "write_dma_bw_MBs", "read_write_dma_bw_MBs",
> "serial_number", "watchdog_resets",
> #ifdef CONFIG_MYRI10GE_DCA
OK great thanks. Amended.
Luis
^ permalink raw reply
* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Toshi Kani @ 2015-03-27 20:40 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
venkatesh.pallipadi, airlied, linux-kernel, linux-fbdev, x86,
xen-devel, Luis R. Rodriguez, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Dave Hansen, Stefan Bader, konrad.wilk, ville.syrjala,
david.vrabel, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-3-git-send-email-mcgrof@do-not-panic.com>
On Fri, 2015-03-20 at 16:17 -0700, Luis R. Rodriguez wrote:
:
> @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
> }
>
> if (mtrr_if) {
> + mtrr_enabled = true;
> set_num_var_ranges();
> init_table();
> if (use_intel()) {
get_mtrr_state();
After setting mtrr_enabled to true, get_mtrr_state() reads
MSR_MTRRdefType and sets 'mtrr_state.enabled', which also indicates if
MTRRs are enabled or not on the system. So, potentially, we could have
a case that mtrr_enabled is set to true, but mtrr_state.enabled is set
to disabled when MTRRs are disabled by BIOS.
Thanks,
-Toshi
ps.
I recently cleaned up this part of the MTRR code in the patch below,
which is currently available in the -mm & -next trees.
https://lkml.org/lkml/2015/3/24/1063
^ permalink raw reply
* Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Andy Lutomirski @ 2015-03-27 21:21 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
Juergen Gross, Jan Beulich, Borislav Petkov, Suresh Siddha,
venkatesh.pallipadi, Dave Airlie, linux-kernel@vger.kernel.org,
Linux Fbdev development list, X86 ML,
xen-devel@lists.xenproject.org, Ingo Molnar, Linus Torvalds,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <20150327201201.GL5622@wotan.suse.de>
On Fri, Mar 27, 2015 at 1:12 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Fri, Mar 20, 2015 at 04:52:18PM -0700, Andy Lutomirski wrote:
>> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
>> > index 8025624..8875e56 100644
>> > --- a/drivers/video/fbdev/aty/atyfb_base.c
>> > +++ b/drivers/video/fbdev/aty/atyfb_base.c
>> > @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
>> >
>> > #ifdef CONFIG_MTRR
>> > par->mtrr_aper = -1;
>> > - par->mtrr_reg = -1;
>> > if (!nomtrr) {
>> > - /* Cover the whole resource. */
>> > - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
>> > + par->mtrr_aper = mtrr_add(info->fix.smem_start,
>> > + info->fix.smem_len,
>> > MTRR_TYPE_WRCOMB, 1);
>> > - if (par->mtrr_aper >= 0 && !par->aux_start) {
>> > - /* Make a hole for mmio. */
>> > - par->mtrr_reg = mtrr_add(par->res_start + 0x800000 -
>> > - GUI_RESERVE, GUI_RESERVE,
>> > - MTRR_TYPE_UNCACHABLE, 1);
>> > - if (par->mtrr_reg < 0) {
>> > - mtrr_del(par->mtrr_aper, 0, 0);
>> > - par->mtrr_aper = -1;
>> > - }
>> > - }
>> > }
>> > #endif
>> >
>> > @@ -2776,10 +2765,6 @@ aty_init_exit:
>> > par->pll_ops->set_pll(info, &par->saved_pll);
>> >
>> > #ifdef CONFIG_MTRR
>> > - if (par->mtrr_reg >= 0) {
>> > - mtrr_del(par->mtrr_reg, 0, 0);
>> > - par->mtrr_reg = -1;
>> > - }
>> > if (par->mtrr_aper >= 0) {
>> > mtrr_del(par->mtrr_aper, 0, 0);
>> > par->mtrr_aper = -1;
>> > @@ -3466,7 +3451,7 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
>> > }
>> >
>> > info->fix.mmio_start = raddr;
>> > - par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000);
>> > + par->ati_regbase = ioremap_nocache(info->fix.mmio_start, 0x1000);
>>
>> Double-check me, but I think that ioremap_nocache + WC MTRR = WC.
>
> Precicely, in this case the WC hole was obtained by using MTRR WC. This
> patch removes that WC hole trick and now we can be explciit about
> only wanting ioremap_nocache() on the registers, that is WC is not
> desired here and is not used. The patch does not highlight the fact
> that there was left in place another ioremap() call for the framebuffer:
>
> info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
>
> That is the one that later after this patch we use ioremap_wc() for.
> This patch just removes the hole solution. That's all.
>
I don't understand.
If I read it right, there's a 2^n byte BAR. You're requesting WC for
the whole think using arch_phys_wc_add. On a PAT system that has no
effect and all is well. On a non-PAT system, it adds an MTRR. That
means that you need to override the MTRR somehow for the mmio regs,
and UC- won't do the trick.
Or am I missing something here?
--Andy
> Luis
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Andy Lutomirski @ 2015-03-27 21:23 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Bjorn Helgaas, Ville Syrjälä, Mauro Carvalho Chehab,
Mike Marciniszyn, Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, Juergen Gross, Jan Beulich, Borislav Petkov,
Suresh Siddha, venkatesh.pallipadi, Dave Airlie,
linux-kernel@vger.kernel.org, Linux Fbdev development list,
X86 ML, xen-devel@lists.xenproject.org, Ingo Molnar,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <20150327203029.GM5622@wotan.suse.de>
On Fri, Mar 27, 2015 at 1:30 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Fri, Mar 27, 2015 at 12:58:02PM -0700, Andy Lutomirski wrote:
>> On Fri, Mar 27, 2015 at 12:53 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> > On Fri, Mar 20, 2015 at 04:48:46PM -0700, Andy Lutomirski wrote:
>> >> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
>> >> <mcgrof@do-not-panic.com> wrote:
>> >> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>> >> >
>> >> > Ideally on systems using PAT we can expect a swift
>> >> > transition away from MTRR. There can be a few exceptions
>> >> > to this, one is where device drivers are known to exist
>> >> > on PATs with errata, another situation is observed on
>> >> > old device drivers where devices had combined MMIO
>> >> > register access with whatever area they typically
>> >> > later wanted to end up using MTRR for on the same
>> >> > PCI BAR. This situation can still be addressed by
>> >> > splitting up ioremap'd PCI BAR into two ioremap'd
>> >> > calls, one for MMIO registers, and another for whatever
>> >> > is desirable for write-combining -- in order to
>> >> > accomplish this though quite a bit of driver
>> >> > restructuring is required.
>> >> >
>> >> > Device drivers which are known to require large
>> >> > amount of re-work in order to split ioremap'd areas
>> >> > can use __arch_phys_wc_add() to avoid regressions
>> >> > when PAT is enabled.
>> >> >
>> >> > For a good example driver where things are neatly
>> >> > split up on a PCI BAR refer the infiniband qib
>> >> > driver. For a good example of a driver where good
>> >> > amount of work is required refer to the infiniband
>> >> > ipath driver.
>> >> >
>> >> > This is *only* a transitive API -- and as such no new
>> >> > drivers are ever expected to use this.
>> >>
>> >> What's the exact layout that this helps? I'm sceptical that this can
>> >> ever be correct.
>> >>
>> >> Is there some awful driver that has a large ioremap that's supposed to
>> >> contain multiple different memtypes?
>> >
>> > Yes, I cc'd you just now on one where I made changes on a driver which uses one
>> > PCI with mixed memtypes and uses MTRR to hole in WC. A transition to
>> > arch_phys_wc_add() is therefore not possible if PAT is enabled as it would
>> > regress those drivers by making the MTRR WC hole trick non functional.
>> > The changes are non trivial and so in this series I supplied changes on
>> > one driver only to show the effort required. The other drivers which
>> > required this were:
>> >
>> > Driver File
>> > ------------------------------------------------------------
>> > fusion drivers/message/fusion/mptbase.c
>> > ivtv drivers/media/pci/ivtv/ivtvfb.c
>> > ipath drivers/infiniband/hw/ipath/ipath_driver.c
>> >
>> > This series makes those drivers use __arch_phys_wc_add() more as a
>> > transitory phase in hopes we can address the proper split as with the
>> > atyfb illustrates. For ipath the changes required have a nice template
>> > with the qib driver as they share very similar driver structure, the
>> > qib driver *did* do the nice split.
>> >
>> >> If so, can we ioremap + set_page_xyz instead?
>> >
>> > I'm not sure I see which call we'd use. Care to provide an example patch
>> > alternative for the atyfb as a case in point alternative to the work required
>> > to do the split?
>> >
>>
>> I'm still confused. Would it be insufficient to ioremap_nocache the
>> whole thing and then call set_memory_wc on parts of it? (Sorry,
>> set_page_xyz was a typo.)
>
> I think that would be a sexy alternative.
>
> In this driver's case the thing is a bit messy as it not only used
> the WC MTRR for a hole but it also then used a UC MTRR on top of
> it all, so since I already tried to address the split, and if we address
> the power of 2 woes, I think it'd be best to try to remove the UC MTRR
> and just avoid set_page_wc() in this driver's case, but for the other cases
> (fusion, ivtv, ipath) I think this makes sense.
>
> Thoughts?
Once that WC MTRR is in place, I think you really need UC and not UC-
if you want to override it. Otherwise I agree with all of this.
--Andy
^ 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