Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH RESEND 2 4/5] staging: sm750fb: Fix __iomem pointer types
From: Lorenzo Stoakes @ 2015-03-20 15:22 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426864935-29350-1-git-send-email-lstoakes@gmail.com>

This patch annotates pointers as referring to I/O mapped memory where they ought
to be, removes now unnecessary ugly casts, eliminates an incorrect deref on I/O
mapped memory by using iowrite16 instead, and updates the pointer arithmetic
accordingly to take into account that the pointers are now byte-sized. This
fixes the following sparse warnings:-

drivers/staging/sm750fb/sm750_cursor.c:113:19: warning: cast removes address space of expression
drivers/staging/sm750fb/sm750_cursor.c:204:19: warning: cast removes address space of expression

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---

 drivers/staging/sm750fb/sm750_cursor.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index 6cceef1..c2ff3bd 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -98,7 +98,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 	int i,j,count,pitch,offset;
 	u8 color,mask,opr;
 	u16 data;
-	u16 * pbuffer,*pstart;
+	void __iomem *pbuffer, *pstart;

 	/*  in byte*/
 	pitch = cursor->w >> 3;
@@ -106,11 +106,11 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 	/* in byte	*/
 	count = pitch * cursor->h;

-	/* in ushort */
-	offset = cursor->maxW * 2 / 8 / 2;
+	/* in byte */
+	offset = cursor->maxW * 2 / 8;

 	data = 0;
-	pstart = (u16 *)cursor->vstart;
+	pstart = cursor->vstart;
 	pbuffer = pstart;

 /*
@@ -161,7 +161,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 			}
 		}
 #endif
-		*pbuffer = data;
+		iowrite16(data, pbuffer);

 		/* assume pitch is 1,2,4,8,...*/
 #if 0
@@ -174,7 +174,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 			pstart += offset;
 			pbuffer = pstart;
 		}else{
-			pbuffer++;
+			pbuffer += sizeof(u16);
 		}

 	}
@@ -189,7 +189,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 	int i,j,count,pitch,offset;
 	u8 color, mask;
 	u16 data;
-	u16 * pbuffer,*pstart;
+	void __iomem *pbuffer, *pstart;

 	/*  in byte*/
 	pitch = cursor->w >> 3;
@@ -197,11 +197,11 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 	/* in byte	*/
 	count = pitch * cursor->h;

-	/* in ushort */
-	offset = cursor->maxW * 2 / 8 / 2;
+	/* in byte */
+	offset = cursor->maxW * 2 / 8;

 	data = 0;
-	pstart = (u16 *)cursor->vstart;
+	pstart = cursor->vstart;
 	pbuffer = pstart;

 	for(i=0;i<count;i++)
@@ -234,7 +234,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 				data |= ((color & (1<<j))?1:2)<<(j*2);
 		}
 #endif
-		*pbuffer = data;
+		iowrite16(data, pbuffer);

 		/* assume pitch is 1,2,4,8,...*/
 		if(!(i&(pitch-1)))
@@ -244,7 +244,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 			pstart += offset;
 			pbuffer = pstart;
 		}else{
-			pbuffer++;
+			pbuffer += sizeof(u16);
 		}

 	}
--
2.3.2

^ permalink raw reply related

* [PATCH RESEND 2 5/5] staging: sm750fb: Remove spinlock helper function
From: Lorenzo Stoakes @ 2015-03-20 15:22 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes
In-Reply-To: <1426864935-29350-1-git-send-email-lstoakes@gmail.com>

This patch removes the unnecessary spinlock helper function and instead
calls spin_lock and spin_unlock directly.

This does *not* resolve sparse warnings about context imbalances but these are
spurious.

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 53 +++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index be35429..a6658e1 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -56,23 +56,6 @@ static char * g_settings = NULL;
 static int g_dualview;
 static char * g_option = NULL;

-/* if not use spin_lock,system will die if user load driver
- * and immediatly unload driver frequently (dual)*/
-static inline void myspin_lock(spinlock_t * sl){
-	struct lynx_share * share;
-	share = container_of(sl,struct lynx_share,slock);
-	if(share->dual){
-		spin_lock(sl);
-	}
-}
-
-static inline void myspin_unlock(spinlock_t * sl){
-	struct lynx_share * share;
-	share = container_of(sl,struct lynx_share,slock);
-	if(share->dual){
-		spin_unlock(sl);
-	}
-}
 static const struct fb_videomode lynx750_ext[] = {
 	/*  	1024x600-60 VESA 	[1.71:1]	*/
 	{NULL,  60, 1024, 600, 20423, 144,  40, 18, 1, 104, 3,
@@ -209,13 +192,21 @@ static void lynxfb_ops_fillrect(struct fb_info* info,const struct fb_fillrect* r
 	color = (Bpp = 1)?region->color:((u32*)info->pseudo_palette)[region->color];
 	rop = ( region->rop != ROP_COPY ) ? HW_ROP2_XOR:HW_ROP2_COPY;

-	myspin_lock(&share->slock);
+	/*
+	 * If not use spin_lock,system will die if user load driver
+	 * and immediatly unload driver frequently (dual)
+	 */
+	if (share->dual)
+		spin_lock(&share->slock);
+
 	share->accel.de_fillrect(&share->accel,
 							base,pitch,Bpp,
 							region->dx,region->dy,
 							region->width,region->height,
 							color,rop);
-	myspin_unlock(&share->slock);
+
+	if (share->dual)
+		spin_unlock(&share->slock);
 }

 static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea * region)
@@ -233,12 +224,20 @@ static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea *
 	pitch = info->fix.line_length;
 	Bpp = info->var.bits_per_pixel >> 3;

-	myspin_lock(&share->slock);
+	/*
+	 * If not use spin_lock, system will die if user load driver
+	 * and immediatly unload driver frequently (dual)
+	 */
+	if (share->dual)
+		spin_lock(&share->slock);
+
 	share->accel.de_copyarea(&share->accel,
 							base,pitch,region->sx,region->sy,
 							base,pitch,Bpp,region->dx,region->dy,
 							region->width,region->height,HW_ROP2_COPY);
-	myspin_unlock(&share->slock);
+
+	if (share->dual)
+		spin_unlock(&share->slock);
 }

 static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* image)
@@ -272,14 +271,22 @@ static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* imag
 	}
 	return;
 _do_work:
-	myspin_lock(&share->slock);
+	/*
+	 * If not use spin_lock, system will die if user load driver
+	 * and immediatly unload driver frequently (dual)
+	 */
+	if (share->dual)
+		spin_lock(&share->slock);
+
 	share->accel.de_imageblit(&share->accel,
 					image->data,image->width>>3,0,
 					base,pitch,Bpp,
 					image->dx,image->dy,
 					image->width,image->height,
 					fgcol,bgcol,HW_ROP2_COPY);
-	myspin_unlock(&share->slock);
+
+	if (share->dual)
+		spin_unlock(&share->slock);
 }

 static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
--
2.3.3

^ permalink raw reply related

* Re: [PATCH RESEND v2 4/5] staging: sm750fb: Fix __iomem pointer types
From: Lorenzo Stoakes @ 2015-03-20 15:24 UTC (permalink / raw)
  To: Greg KH; +Cc: Sudip Mukherjee, Teddy Wang, devel, linux-fbdev, linux-kernel
In-Reply-To: <20150320130329.GA23287@kroah.com>

On 20 March 2015 at 13:03, Greg KH <gregkh@linuxfoundation.org> wrote:
> I only see 2 patches in this "series", yet you refer to 5?  Please
> resend the whole series, I dropped your previous ones from my queue.
>
> thanks,
>
> greg k-h

I have resent all the patches as RESEND 2 as well as updating them to
apply against staging-testing.

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Geert Uytterhoeven @ 2015-03-20 15:24 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Tomi Valkeinen, Thomas Niederprüm,
	Linux Fbdev development list, Jean-Christophe PLAGNIOL-VILLARD,
	linux-kernel@vger.kernel.org
In-Reply-To: <20150320144740.GH4255@lukather>

On Fri, Mar 20, 2015 at 3:47 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, Mar 20, 2015 at 01:37:50PM +0200, Tomi Valkeinen wrote:
>> On 15/03/15 00:02, Geert Uytterhoeven wrote:
>> > On Fri, Mar 13, 2015 at 10:31 PM, Thomas Niederprüm
>> > <niederp@physik.uni-kl.de> wrote:
>> >> Am Tue, 10 Mar 2015 13:28:25 +0200
>> >> schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
>> >>> Also, isn't doing __pa() for the memory returned by vmalloc plain
>> >>> wrong?
>> >>
>> >>> What was the crash about when using kmalloc? It would be good to fix
>> >>> defio, as I don't see why it should not work with kmalloced memory.
>> >>
>> >> The main challenge here is that the memory handed to userspace upon
>> >> mmap call needs to be page aligned. The memory returned by kmalloc has
>> >> no such alignment, but the pointer presented to the userspace program
>> >> gets aligned to next page boundary. It's not clear to me whether there
>> >> is an easy way to obtain page aligned kmalloc memory. Memory
>> >> allocated by vmalloc on the other hand is always aligned to page
>> >> boundaries. This is why I chose to go for vmalloc.
>> >
>> > __get_free_pages()?
>>
>> I'm not that experienced with mem management, so I have to ask...
>> __get_free_pages() probably works fine, but isn't vmalloc better here?
>>
>> __get_free_pages() will give you possibly a lot more memory than you
>> need. And the memory is contiguous, so it could be difficult to allocate
>> a larger memory area. The driver doesn't need contiguous memory (except
>> in the virtual sense).
>
> vmalloc also returns pages, so the size will be page-aligned. It
> doesn't make much of a difference here, since we will only use a
> single page in both case (the max resolution of these screens is
> 128x39, with one bit per pixel).

In that case I recommend get_zeroed_page(), to avoid the vmalloc()
overhead of setting up a mapping.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Tomi Valkeinen @ 2015-03-20 15:25 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Geert Uytterhoeven, Thomas Niederprüm,
	Linux Fbdev development list, Jean-Christophe PLAGNIOL-VILLARD,
	linux-kernel@vger.kernel.org
In-Reply-To: <20150320144740.GH4255@lukather>

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

On 20/03/15 16:47, Maxime Ripard wrote:
> On Fri, Mar 20, 2015 at 01:37:50PM +0200, Tomi Valkeinen wrote:
>> On 15/03/15 00:02, Geert Uytterhoeven wrote:
>>> On Fri, Mar 13, 2015 at 10:31 PM, Thomas Niederprüm
>>> <niederp@physik.uni-kl.de> wrote:
>>>> Am Tue, 10 Mar 2015 13:28:25 +0200
>>>> schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
>>>>> Also, isn't doing __pa() for the memory returned by vmalloc plain
>>>>> wrong?
>>>>
>>>>> What was the crash about when using kmalloc? It would be good to fix
>>>>> defio, as I don't see why it should not work with kmalloced memory.
>>>>
>>>> The main challenge here is that the memory handed to userspace upon
>>>> mmap call needs to be page aligned. The memory returned by kmalloc has
>>>> no such alignment, but the pointer presented to the userspace program
>>>> gets aligned to next page boundary. It's not clear to me whether there
>>>> is an easy way to obtain page aligned kmalloc memory. Memory
>>>> allocated by vmalloc on the other hand is always aligned to page
>>>> boundaries. This is why I chose to go for vmalloc.
>>>
>>> __get_free_pages()?
>>
>> I'm not that experienced with mem management, so I have to ask...
>> __get_free_pages() probably works fine, but isn't vmalloc better here?
>>
>> __get_free_pages() will give you possibly a lot more memory than you
>> need. And the memory is contiguous, so it could be difficult to allocate
>> a larger memory area. The driver doesn't need contiguous memory (except
>> in the virtual sense).
> 
> vmalloc also returns pages, so the size will be page-aligned. It
> doesn't make much of a difference here, since we will only use a
> single page in both case (the max resolution of these screens is
> 128x39, with one bit per pixel).

Ok, that's not much, then =).

In that case __get_free_pages sounds fine. Even if the resolution would
be slightly higher, we're only talking about a page or two extra.

Usually double-underscore in front of a func means "don't call this". I
don't know why this one has the underscores.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] omapdss: extend pm notifier to handle hibernation
From: Grygorii.Strashko@linaro.org @ 2015-03-20 17:19 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jean-Christophe Plagniol-Villard, nm, sumit.semwal, linux-omap,
	kishon, linux-fbdev
In-Reply-To: <550C3AEB.1090802@ti.com>

On 03/20/2015 05:21 PM, Tomi Valkeinen wrote:
> On 20/03/15 16:57, Grygorii.Strashko@linaro.org wrote:
>> On 03/20/2015 02:20 PM, Tomi Valkeinen wrote:
>>> On 25/02/15 19:03, grygorii.strashko@linaro.org wrote:
>>>> From: Grygorii Strashko <Grygorii.Strashko@linaro.org>
>>>>
>>>> Add handling of missed events in omap_dss_pm_notif which are
>>>> needed to support hibernation (suspend to disk).
>>>>
>>>> Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
>>>> ---
>>>>    drivers/video/fbdev/omap2/dss/core.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/video/fbdev/omap2/dss/core.c b/drivers/video/fbdev/omap2/dss/core.c
>>>> index 6b74f73..e60976a 100644
>>>> --- a/drivers/video/fbdev/omap2/dss/core.c
>>>> +++ b/drivers/video/fbdev/omap2/dss/core.c
>>>> @@ -178,11 +178,15 @@ static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
>>>>    	DSSDBG("pm notif %lu\n", v);
>>>>    
>>>>    	switch (v) {
>>>> +	case PM_HIBERNATION_PREPARE:
>>>>    	case PM_SUSPEND_PREPARE:
>>>> +	case PM_RESTORE_PREPARE:
>>>>    		DSSDBG("suspending displays\n");
>>>>    		return dss_suspend_all_devices();
>>>>    
>>>>    	case PM_POST_SUSPEND:
>>>> +	case PM_POST_HIBERNATION:
>>>> +	case PM_POST_RESTORE:
>>>>    		DSSDBG("resuming displays\n");
>>>>    		return dss_resume_all_devices();
>>>>    
>>>
>>> Why suspend displays when PM_RESTORE_PREPARE happens? Why resume when
>>> PM_POST_RESTORE happens?
>>
>> We have following sequence when system is restored from hibernation:
>> - original kernel booted;
>> - late_initcall_sync(software_resume);
>>    - pm_notifier_call_chain(PM_RESTORE_PREPARE);
>>    - freeze_processes
>>    - check & read hibernation image
>>    - suspend all devices (.freeze())
>>    - jump to stored kernel
>>    - restore devices
>>    ...
>>
>> So, all devices should be in frozen/suspended state when we will jump to stored kernel
>> (device's state should be the same as before creating hib image).
>>
>> Without this patch I can see a lot of log messages like below:
> 
> Yes, I am sure a fix is needed for hibernation. But I still don't quite
> understand PM_RESTORE_PREPARE and PM_POST_RESTORE.
> 
> When we enter hibernation, there's only PM_HIBERNATION_PREPARE?

No. There is always PM_POST_HIBERNATION:
    - original Kernel in case of failure
    - restored Kernel on success

> 
> When waking from hibernation, there's first PM_RESTORE_PREPARE, where we
> need to disable displays that were enabled during boot. Then either
> PM_POST_HIBERNATION if all went well, or PM_POST_RESTORE if there was an
> error, and in both cases we want to enable the displays?

Yes.

==
int hibernate(void)
	error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
...
	error = hibernation_snapshot(hibernation_mode = HIBERNATION_PLATFORM);
		^^^ restored kernel will start from here

	if (error || freezer_test_done)
		goto Free_bitmaps;

	if (in_suspend) {
...

		pr_debug("PM: writing image.\n");
		error = swsusp_write(flags);
		swsusp_free();
		if (!error)
			power_down();
		in_suspend = 0;
		pm_restore_gfp_mask();
		^^ this part will be executed by Kernel requested Hibernation

	} else {
		pr_debug("PM: Image restored successfully.\n");
		^^ this part will be executed by Kernel restored from Hibernation
	}

...
	pm_notifier_call_chain(PM_POST_HIBERNATION);
^^^ Both Kernels will be notified here:
    - original Kernel in case of failure
    - restored Kernel on success
    
    


==
static int software_resume(void)
	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
	error = swsusp_read(&flags);
	swsusp_close(FMODE_READ);
	if (!error)
		hibernation_restore(flags & SF_PLATFORM_MODE);
                ^^^ if ok we will never return from this function

	printk(KERN_ERR "PM: Failed to load hibernation image, recovering.\n");
	pm_notifier_call_chain(PM_POST_RESTORE);
^^^
   if fail - just continue work as usual


-- 
regards,
-grygorii

^ permalink raw reply

* [PATCH] sm750fb: Fix indentation, spacing and switch-case
From: Amitoj Kaur Chawla @ 2015-03-20 18:53 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
	linux-kernel

Fix the spacing problems with correct indentation and correct use of
braces and spacing in switch-case statements.

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c | 63 +++++++++++++++++------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 1e6b474..de56302 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -46,53 +46,52 @@ inline unsigned int twoToPowerOfx(unsigned long x)
 
 inline unsigned int calcPLL(pll_value_t *pPLL)
 {
-    return (pPLL->inputFreq * pPLL->M / pPLL->N / twoToPowerOfx(pPLL->OD) / twoToPowerOfx(pPLL->POD));
+	return (pPLL->inputFreq * pPLL->M / pPLL->N / twoToPowerOfx(pPLL->OD) / twoToPowerOfx(pPLL->POD));
 }
 
 unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL)
 {
-    unsigned int ulPllReg = 0;
-
-    pPLL->inputFreq = DEFAULT_INPUT_CLOCK;
-    pPLL->clockType = clockType;
-
-    switch (clockType)
-    {
-        case MXCLK_PLL:
-            ulPllReg = PEEK32(MXCLK_PLL_CTRL);
-            break;
-        case PRIMARY_PLL:
-            ulPllReg = PEEK32(PANEL_PLL_CTRL);
-            break;
-        case SECONDARY_PLL:
-            ulPllReg = PEEK32(CRT_PLL_CTRL);
-            break;
-        case VGA0_PLL:
-            ulPllReg = PEEK32(VGA_PLL0_CTRL);
-            break;
-        case VGA1_PLL:
-            ulPllReg = PEEK32(VGA_PLL1_CTRL);
-            break;
-    }
+	unsigned int ulPllReg = 0;
+
+	pPLL->inputFreq = DEFAULT_INPUT_CLOCK;
+	pPLL->clockType = clockType;
+
+	switch (clockType) {
+	case MXCLK_PLL:
+		ulPllReg = PEEK32(MXCLK_PLL_CTRL);
+		break;
+	case PRIMARY_PLL:
+		ulPllReg = PEEK32(PANEL_PLL_CTRL);
+		break;
+	case SECONDARY_PLL:
+		ulPllReg = PEEK32(CRT_PLL_CTRL);
+		break;
+	case VGA0_PLL:
+		ulPllReg = PEEK32(VGA_PLL0_CTRL);
+		break;
+	case VGA1_PLL:
+		ulPllReg = PEEK32(VGA_PLL1_CTRL);
+		break;
+	}
 
-    pPLL->M = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, M);
-    pPLL->N = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, N);
-    pPLL->OD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, OD);
-    pPLL->POD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, POD);
+	pPLL->M = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, M);
+	pPLL->N = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, N);
+	pPLL->OD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, OD);
+	pPLL->POD = FIELD_GET(ulPllReg, PANEL_PLL_CTRL, POD);
 
-    return calcPLL(pPLL);
+	return calcPLL(pPLL);
 }
 
 
 unsigned int getChipClock(void)
 {
-    pll_value_t pll;
+	pll_value_t pll;
 #if 1
-	if(getChipType() = SM750LE)
+	if (getChipType() = SM750LE)
 		return MHz(130);
 #endif
 
-    return getPllValue(MXCLK_PLL, &pll);
+	return getPllValue(MXCLK_PLL, &pll);
 }
 
 
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Thomas Niederprüm @ 2015-03-20 20:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Maxime Ripard, Tomi Valkeinen, Linux Fbdev development list,
	Jean-Christophe PLAGNIOL-VILLARD, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdXA87LOYxTwf5G3EZ58dPQJfdgv=3esSJQ_MQqCQciKKw@mail.gmail.com>

Am Fri, 20 Mar 2015 16:24:29 +0100
schrieb Geert Uytterhoeven <geert@linux-m68k.org>:

> On Fri, Mar 20, 2015 at 3:47 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Fri, Mar 20, 2015 at 01:37:50PM +0200, Tomi Valkeinen wrote:
> >> On 15/03/15 00:02, Geert Uytterhoeven wrote:
> >> > On Fri, Mar 13, 2015 at 10:31 PM, Thomas Niederprüm
> >> > <niederp@physik.uni-kl.de> wrote:
> >> >> Am Tue, 10 Mar 2015 13:28:25 +0200
> >> >> schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
> >> >>> Also, isn't doing __pa() for the memory returned by vmalloc
> >> >>> plain wrong?
> >> >>
> >> >>> What was the crash about when using kmalloc? It would be good
> >> >>> to fix defio, as I don't see why it should not work with
> >> >>> kmalloced memory.
> >> >>
> >> >> The main challenge here is that the memory handed to userspace
> >> >> upon mmap call needs to be page aligned. The memory returned by
> >> >> kmalloc has no such alignment, but the pointer presented to the
> >> >> userspace program gets aligned to next page boundary. It's not
> >> >> clear to me whether there is an easy way to obtain page aligned
> >> >> kmalloc memory. Memory allocated by vmalloc on the other hand
> >> >> is always aligned to page boundaries. This is why I chose to go
> >> >> for vmalloc.
> >> >
> >> > __get_free_pages()?
> >>
> >> I'm not that experienced with mem management, so I have to ask...
> >> __get_free_pages() probably works fine, but isn't vmalloc better
> >> here?
> >>
> >> __get_free_pages() will give you possibly a lot more memory than
> >> you need. And the memory is contiguous, so it could be difficult
> >> to allocate a larger memory area. The driver doesn't need
> >> contiguous memory (except in the virtual sense).
> >
> > vmalloc also returns pages, so the size will be page-aligned. It
> > doesn't make much of a difference here, since we will only use a
> > single page in both case (the max resolution of these screens is
> > 128x39, with one bit per pixel).
> 
> In that case I recommend get_zeroed_page(), to avoid the vmalloc()
> overhead of setting up a mapping.

I looked into get_zeroed_page() too but I thought __get_free_pages()
might be more future proof since get_zeroed_page() will not reserve
enough memory if more than one page is needed. This might occur if a new
controller pops up that has more pixels than bits in a page or the
driver is used on a system with a small page size. Also
get_zeroed_page() is also just calling __get_free_pages() with the
order parameter set to 0. Therefore I think a call like

__get_free_pages(GFP_KERNEL | __GFP_ZERO, get_order(size)) 

does the same as get_zeroed_page() if only one page is needed but has
the ability to reserve more pages if needed.

Thomas


^ permalink raw reply

* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Thomas Niederprüm @ 2015-03-20 20:36 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Maxime Ripard, Geert Uytterhoeven, Linux Fbdev development list,
	Jean-Christophe PLAGNIOL-VILLARD, linux-kernel@vger.kernel.org
In-Reply-To: <550C3BE9.7080905@ti.com>

Am Fri, 20 Mar 2015 17:25:29 +0200
schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:

> On 20/03/15 16:47, Maxime Ripard wrote:
> > On Fri, Mar 20, 2015 at 01:37:50PM +0200, Tomi Valkeinen wrote:
> >> On 15/03/15 00:02, Geert Uytterhoeven wrote:
> >>> On Fri, Mar 13, 2015 at 10:31 PM, Thomas Niederprüm
> >>> <niederp@physik.uni-kl.de> wrote:
> >>>> Am Tue, 10 Mar 2015 13:28:25 +0200
> >>>> schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
> >>>>> Also, isn't doing __pa() for the memory returned by vmalloc
> >>>>> plain wrong?
> >>>>
> >>>>> What was the crash about when using kmalloc? It would be good
> >>>>> to fix defio, as I don't see why it should not work with
> >>>>> kmalloced memory.
> >>>>
> >>>> The main challenge here is that the memory handed to userspace
> >>>> upon mmap call needs to be page aligned. The memory returned by
> >>>> kmalloc has no such alignment, but the pointer presented to the
> >>>> userspace program gets aligned to next page boundary. It's not
> >>>> clear to me whether there is an easy way to obtain page aligned
> >>>> kmalloc memory. Memory allocated by vmalloc on the other hand is
> >>>> always aligned to page boundaries. This is why I chose to go for
> >>>> vmalloc.
> >>>
> >>> __get_free_pages()?
> >>
> >> I'm not that experienced with mem management, so I have to ask...
> >> __get_free_pages() probably works fine, but isn't vmalloc better
> >> here?
> >>
> >> __get_free_pages() will give you possibly a lot more memory than
> >> you need. And the memory is contiguous, so it could be difficult
> >> to allocate a larger memory area. The driver doesn't need
> >> contiguous memory (except in the virtual sense).
> > 
> > vmalloc also returns pages, so the size will be page-aligned. It
> > doesn't make much of a difference here, since we will only use a
> > single page in both case (the max resolution of these screens is
> > 128x39, with one bit per pixel).
> 
> Ok, that's not much, then =).
> 
> In that case __get_free_pages sounds fine. Even if the resolution
> would be slightly higher, we're only talking about a page or two
> extra.
> 
> Usually double-underscore in front of a func means "don't call this".
> I don't know why this one has the underscores.

This irritated me as well. But since it turned out that there is even a
section on "get_free_page and Friends" in LDD3 that talks about
__get_free_pages() without any word of caution, I assumed it's ok to
call this functions.

Thomas

^ permalink raw reply

* Re: [PATCHv4 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Thomas Niederprüm @ 2015-03-20 21:12 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: plagnioj, tomi.valkeinen, kernel, shawn.guo, robh+dt, linux-fbdev,
	linux-kernel
In-Reply-To: <20150318192719.GV4638@lukather>

Am Wed, 18 Mar 2015 20:27:19 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> Hi Thomas,
> 
> On Mon, Mar 16, 2015 at 06:11:52PM +0100, Thomas Niederprüm wrote:
> > The 130X controllers are very similar from the configuration point
> > of view. The configuration registers for the SSD1305/6/7 are bit
> > identical (except the the VHCOM register and the the default values
> > for clock setup register). This patch unifies the init code of the
> > controller and adds hardware specific properties to DT that are
> > needed to correctly initialize the device.
> > 
> > The SSD130X can be wired to the OLED panel in various ways. Even
> > for the same controller this wiring can differ from one display
> > module to another and can not be probed by software. The added DT
> > properties reflect these hardware decisions of the display module
> > manufacturer. The 'com-sequential', 'com-lrremap' and 'com-invdir'
> > values define different possibilities for the COM signals pin
> > configuration and readout direction of the video memory. The
> > 'segment-no-remap' allows the inversion of the memory-to-pin
> > mapping ultimately inverting the order of the controllers output
> > pins. The 'prechargepX' values need to be adapted according the
> > capacitance of the OLEDs pixel cells.
> > 
> > So far these hardware specific bits are hard coded in the init
> > code, making the driver usable only for one certain wiring of the
> > controller. This patch makes the driver usable with all possible
> > hardware setups, given a valid hw description in DT. If these
> > values are not set in DT the default values, as they are set in the
> > ssd1307 init code right now, are used. This implies that without
> > the corresponding DT property "segment-no-remap" the segment remap
> > of the ssd130X controller gets activated. Even though this is not
> > the default behaviour according to the datasheet it maintains
> > backward compatibility with older DTBs.
> > 
> > Note that the SSD1306 does not seem to be using the configuration
> > written to the registers at all. Therefore this patch does not try
> > to maintain these values without changes in DT. For reference an
> > example is added to the DT bindings documentation that reproduces
> > the configuration that is set in the current init code.
> > 
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> 
> This looks mostly fine, thanks for your effort on this.
> 
> > ---
> >  .../devicetree/bindings/video/ssd1307fb.txt        |  21 +++
> >  drivers/video/fbdev/ssd1307fb.c                    | 195
> > ++++++++++++--------- 2 files changed, 137 insertions(+), 79
> > deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > 7a12542..be27562 100644 ---
> > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
> > +15,16 @@ Required properties: 
> >  Optional properties:
> >    - reset-active-low: Is the reset gpio is active on physical low?
> > +  - solomon,segment-no-remap: Display needs normal (non-inverted)
> > data column
> > +                              to segment mapping
> > +  - solomon,com-sequential: Display uses sequential COM pin
> > configuration
> > +  - solomon,com-lrremap: Display uses left-right COM pin remap
> > +  - solomon,com-invdir: Display uses inverted COM pin scan
> > direction
> > +  - solomon,com-offset: Offset of the first COM pin wired to the
> > panel
> 
> Documenting the offset unit would be fine.

The com-offset determines which line of the display panel should
receive the first line of video memory. When designing your display
module you could decide to do something crazy and connect the first
lines of your panel to  (for example) pin COM32 onward. By setting the
com-offset to 32 you could still recover the correct image on your
panel. So in some sense the unit of the com-offset is display lines or
number of pins, depending on whether you see it from the controller
side or from the panel side. I will update the documentation to reflect
this.

> 
> > +  - solomon,prechargep1: Length of deselect period (phase 1) in
> > clock cycles.
> > +  - solomon,prechargep2: Length of precharge period (phase 2) in
> > clock cycles.
> > +                         This needs to be the higher, the higher
> > the capacitance
> > +                         of the OLED's pixels is
> >  
> >  [0]: Documentation/devicetree/bindings/pwm/pwm.txt
> >  
> > @@ -26,3 +36,14 @@ ssd1307: oled@3c {
> >          reset-gpios = <&gpio2 7>;
> >          reset-active-low;
> >  };
> > +
> > +ssd1306: oled@3c {
> > +        compatible = "solomon,ssd1306fb-i2c";
> > +        reg = <0x3c>;
> > +        pwms = <&pwm 4 3000>;
> > +        reset-gpios = <&gpio2 7>;
> > +        reset-active-low;
> > +        solomon,com-lrremap;
> > +        solomon,com-invdir;
> > +        solomon,com-offset = <32>;
> > +};
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 8d34c56..9a66118 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -16,6 +16,9 @@
> >  #include <linux/pwm.h>
> >  #include <linux/delay.h>
> >  
> > +#define DEVID_SSD1306 6
> > +#define DEVID_SSD1307 7
> > +
> >  #define SSD1307FB_DATA			0x40
> >  #define SSD1307FB_COMMAND		0x80
> >  
> > @@ -38,22 +41,38 @@
> >  #define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
> >  #define	SSD1307FB_SET_VCOMH		0xdb
> >  
> > +static u_int contrast = 128;
> > +module_param(contrast, uint, S_IRUGO);
> > +
> 
> Why is this patch adding a contrast parameter? You don't even mention
> it in your commit log. This should be a separate patch.

I agree, this should disappear or go into another patch. Since the
contrast can also be set through the backlight class, I wonder whether
it is useful to keep this module parameter. I'm using it right now to
set the initial contrast as in my use case I almost always want the
smallest possible contrast setting. In any case I will split this into
a separate patch.
 
> >  struct ssd1307fb_par;
> >  
> > -struct ssd1307fb_ops {
> > -	int (*init)(struct ssd1307fb_par *);
> > -	int (*remove)(struct ssd1307fb_par *);
> > +struct ssd1307fb_deviceinfo {
> > +	int device_id;
> > +	u32 default_vcomh;
> > +	u32 default_dclk_div;
> > +	u32 default_dclk_frq;
> >  };
> >  
> >  struct ssd1307fb_par {
> > +	u32 com_invdir;
> > +	u32 com_lrremap;
> > +	u32 com_offset;
> > +	u32 com_seq;
> > +	u32 contrast;
> > +	u32 dclk_div;
> > +	u32 dclk_frq;
> > +	struct ssd1307fb_deviceinfo *device_info;
> >  	struct i2c_client *client;
> >  	u32 height;
> >  	struct fb_info *info;
> > -	struct ssd1307fb_ops *ops;
> >  	u32 page_offset;
> > +	u32 prechargep1;
> > +	u32 prechargep2;
> >  	struct pwm_device *pwm;
> >  	u32 pwm_period;
> >  	int reset;
> > +	u32 seg_remap;
> > +	u32 vcomh;
> >  	u32 width;
> >  };
> >  
> > @@ -254,69 +273,46 @@ static struct fb_deferred_io ssd1307fb_defio
> > = { .deferred_io	= ssd1307fb_deferred_io,
> >  };
> >  
> > -static int ssd1307fb_ssd1307_init(struct ssd1307fb_par *par)
> > +static int ssd1307fb_init(struct ssd1307fb_par *par)
> >  {
> >  	int ret;
> > +	u32 precharge, dclk, com_invdir, compins;
> >  
> > -	par->pwm = pwm_get(&par->client->dev, NULL);
> > -	if (IS_ERR(par->pwm)) {
> > -		dev_err(&par->client->dev, "Could not get PWM from
> > device tree!\n");
> > -		return PTR_ERR(par->pwm);
> > -	}
> > -
> > -	par->pwm_period = pwm_get_period(par->pwm);
> > -	/* Enable the PWM */
> > -	pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> > -	pwm_enable(par->pwm);
> > -
> > -	dev_dbg(&par->client->dev, "Using PWM%d with a %dns
> > period.\n",
> > -		par->pwm->pwm, par->pwm_period);
> > -
> > -	/* Map column 127 of the OLED to segment 0 */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SEG_REMAP_ON);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	/* Turn on the display */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_ON);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	return 0;
> > -}
> > -
> > -static int ssd1307fb_ssd1307_remove(struct ssd1307fb_par *par)
> > -{
> > -	pwm_disable(par->pwm);
> > -	pwm_put(par->pwm);
> > -	return 0;
> > -}
> > +	if (par->device_info->device_id = DEVID_SSD1307) {
> > +		par->pwm = pwm_get(&par->client->dev, NULL);
> > +		if (IS_ERR(par->pwm)) {
> > +			dev_err(&par->client->dev, "Could not get
> > PWM from device tree!\n");
> > +			return PTR_ERR(par->pwm);
> > +		}
> >  
> > -static struct ssd1307fb_ops ssd1307fb_ssd1307_ops = {
> > -	.init	= ssd1307fb_ssd1307_init,
> > -	.remove	= ssd1307fb_ssd1307_remove,
> > -};
> > +		par->pwm_period = pwm_get_period(par->pwm);
> > +		/* Enable the PWM */
> > +		pwm_config(par->pwm, par->pwm_period / 2,
> > par->pwm_period);
> > +		pwm_enable(par->pwm);
> >  
> > -static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
> > -{
> > -	int ret;
> > +		dev_dbg(&par->client->dev, "Using PWM%d with a
> > %dns period.\n",
> > +			par->pwm->pwm, par->pwm_period);
> > +	};
> >  
> >  	/* Set initial contrast */
> >  	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x7f);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	/* Set COM direction */
> > -	ret = ssd1307fb_write_cmd(par->client, 0xc8);
> > +	ret = ssd1307fb_write_cmd(par->client, par->contrast);
> >  	if (ret < 0)
> >  		return ret;
> >  
> >  	/* Set segment re-map */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SEG_REMAP_ON);
> > +	if (par->seg_remap) {
> > +		ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SEG_REMAP_ON);
> > +		if (ret < 0)
> > +			return ret;
> > +	};
> > +
> > +	/* Set COM direction */
> > +	com_invdir = 0xc0 | (par->com_invdir & 0xf) << 3;
> > +	ret = ssd1307fb_write_cmd(par->client,  com_invdir);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -334,34 +330,38 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x20);
> > +	ret = ssd1307fb_write_cmd(par->client, par->com_offset);
> >  	if (ret < 0)
> >  		return ret;
> >  
> >  	/* Set clock frequency */
> > +	dclk = (par->dclk_div & 0xf) | (par->dclk_frq & 0xf) << 4;
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_CLOCK_FREQ); if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0xf0);
> > +	ret = ssd1307fb_write_cmd(par->client, dclk);
> 
> It would make more sense to assign the variable where you use it.
I will move the assignment right in front of the write command.
 
> >  	if (ret < 0)
> >  		return ret;
> >  
> >  	/* Set precharge period in number of ticks from the
> > internal clock */
> > +	precharge = (par->prechargep1 & 0xf) | (par->prechargep2 &
> > 0xf) << 4; ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_PRECHARGE_PERIOD); if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x22);
> > +	ret = ssd1307fb_write_cmd(par->client, precharge);
> 
> Here too.
> 
> >  	if (ret < 0)
> >  		return ret;
> >  
> >  	/* Set COM pins configuration */
> > +	compins = 0x02 | (!par->com_seq & 0x1) << 4
> > +				   | (par->com_lrremap & 0x1) << 5;
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_COM_PINS_CONFIG); if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x22);
> > +	ret = ssd1307fb_write_cmd(par->client, compins);
> 
> And here.
> 
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -370,18 +370,20 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x49);
> > +	ret = ssd1307fb_write_cmd(par->client, par->vcomh);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	/* Turn on the DC-DC Charge Pump */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_CHARGE_PUMP);
> > -	if (ret < 0)
> > -		return ret;
> > +	if (par->device_info->device_id = DEVID_SSD1306) {
> > +		/* Turn on the DC-DC Charge Pump */
> > +		ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_CHARGE_PUMP);
> > +		if (ret < 0)
> > +			return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x14);
> > -	if (ret < 0)
> > -		return ret;
> > +		ret = ssd1307fb_write_cmd(par->client, 0x14);
> > +		if (ret < 0)
> > +			return ret;
> > +	};
> >  
> >  	/* Switch to horizontal addressing mode */
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_ADDRESS_MODE); @@ -393,6 +395,7 @@ static int
> > ssd1307fb_ssd1306_init(struct ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > +    /* Set column range */
> 
> The indentation is wrong.
Im surprised that I didn't see that myself, because the way it
highlighted is hard to miss. However, thank you for catching this.

> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_COL_RANGE); if (ret < 0)
> >  		return ret;
> > @@ -405,6 +408,7 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > +    /* Set page range */
> 
> Here too.
> 
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_PAGE_RANGE); if (ret < 0)
> >  		return ret;
> > @@ -426,18 +430,28 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) return 0;
> >  }
> >  
> > -static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
> > -	.init	= ssd1307fb_ssd1306_init,
> > +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
> > +	.device_id  = DEVID_SSD1306,
> > +	.default_vcomh = 0x20,
> > +	.default_dclk_div = 0,
> > +	.default_dclk_frq = 8,
> > +};
> > +
> > +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
> > +	.device_id  = DEVID_SSD1307,
> > +	.default_vcomh = 0x20,
> > +	.default_dclk_div = 1,
> > +	.default_dclk_frq = 12,
> >  };
> >  
> >  static const struct of_device_id ssd1307fb_of_match[] = {
> >  	{
> >  		.compatible = "solomon,ssd1306fb-i2c",
> > -		.data = (void *)&ssd1307fb_ssd1306_ops,
> > +		.data = (void *)&ssd1307fb_ssd1306_deviceinfo,
> >  	},
> >  	{
> >  		.compatible = "solomon,ssd1307fb-i2c",
> > -		.data = (void *)&ssd1307fb_ssd1307_ops,
> > +		.data = (void *)&ssd1307fb_ssd1307_deviceinfo,
> 
> Do we need this ID? Wouldn't it make more sense to pass the pointer to
> the struct we need to use?

Are you talking about the device_id inside the struct
ssd1307_deviceinfo? I need the device_id to serve special needs of the
individual controllers during initialization. For example the ssd1307
needs to set up the pwm in the init code. 


> >  	},
> >  	{},
> >  };
> > @@ -468,8 +482,8 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, par->info = info;
> >  	par->client = client;
> >  
> > -	par->ops = (struct ssd1307fb_ops
> > *)of_match_device(ssd1307fb_of_match,
> > -
> > &client->dev)->data;
> > +	par->device_info = (struct ssd1307fb_deviceinfo
> > *)of_match_device(
> > +			ssd1307fb_of_match, &client->dev)->data;
> >  
> >  	par->reset = of_get_named_gpio(client->dev.of_node,
> >  					 "reset-gpios", 0);
> > @@ -487,6 +501,27 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, if (of_property_read_u32(node, "solomon,page-offset",
> > &par->page_offset)) par->page_offset = 1;
> >  
> > +	if (of_property_read_u32(node, "solomon,com-offset",
> > &par->com_offset))
> > +		par->com_offset = 0;
> > +
> > +	if (of_property_read_u32(node, "solomon,prechargep1",
> > &par->prechargep1))
> > +		par->prechargep1 = 2;
> > +
> > +	if (of_property_read_u32(node, "solomon,prechargep2",
> > &par->prechargep2))
> > +		par->prechargep2 = 0;
> > +
> > +	par->seg_remap = !of_property_read_bool(node,
> > "solomon,segment-no-remap");
> > +	par->com_seq = of_property_read_bool(node,
> > "solomon,com-sequential");
> > +	par->com_lrremap = of_property_read_bool(node,
> > "solomon,com-lrremap");
> > +	par->com_invdir = of_property_read_bool(node,
> > "solomon,com-invdir"); +
> > +	par->contrast = contrast;
> > +	par->vcomh = par->device_info->default_vcomh;
> > +
> > +	/* Setup display timing */
> > +	par->dclk_div = par->device_info->default_dclk_div;
> > +	par->dclk_frq = par->device_info->default_dclk_frq;
> > +
> >  	vmem_size = par->width * par->height / 8;
> >  
> >  	vmem = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
> > @@ -539,11 +574,9 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, gpio_set_value(par->reset, 1);
> >  	udelay(4);
> >  
> > -	if (par->ops->init) {
> > -		ret = par->ops->init(par);
> > -		if (ret)
> > -			goto reset_oled_error;
> > -	}
> > +	ret = ssd1307fb_init(par);
> > +	if (ret)
> > +		goto reset_oled_error;
> >  
> >  	ret = register_framebuffer(info);
> >  	if (ret) {
> > @@ -556,8 +589,10 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, return 0;
> >  
> >  panel_init_error:
> > -	if (par->ops->remove)
> > -		par->ops->remove(par);
> > +	if (par->device_info->device_id = DEVID_SSD1307) {
> > +		pwm_disable(par->pwm);
> > +		pwm_put(par->pwm);
> > +	};
> >  reset_oled_error:
> >  	fb_deferred_io_cleanup(info);
> >  fb_alloc_error:
> > @@ -571,8 +606,10 @@ static int ssd1307fb_remove(struct i2c_client
> > *client) struct ssd1307fb_par *par = info->par;
> >  
> >  	unregister_framebuffer(info);
> > -	if (par->ops->remove)
> > -		par->ops->remove(par);
> > +	if (par->device_info->device_id = DEVID_SSD1307) {
> > +		pwm_disable(par->pwm);
> > +		pwm_put(par->pwm);
> > +	};
> >  	fb_deferred_io_cleanup(info);
> >  	__free_pages(__va(info->fix.smem_start),
> > get_order(info->fix.smem_len)); framebuffer_release(info);
> > -- 
> > 2.3.0
> > 

Thomas


^ permalink raw reply

* Re: [PATCHv4 07/10] fbdev: ssd1307fb: Add a module parameter to set the refresh rate
From: Thomas Niederprüm @ 2015-03-20 21:16 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: plagnioj, tomi.valkeinen, kernel, shawn.guo, robh+dt, linux-fbdev,
	linux-kernel
In-Reply-To: <20150319131824.GB4255@lukather>

Am Thu, 19 Mar 2015 14:18:24 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> On Mon, Mar 16, 2015 at 06:11:55PM +0100, Thomas Niederprüm wrote:
> > This patch adds the module parameter "refreshrate" to set delay for
> > the deferred io. The refresh rate is given in units of Hertz. The
> > default refresh rate is 1 Hz. The refresh rate set through the
> > newly introduced parameter applies to all instances of the driver
> > and for now it is not possible to change it individually.
> > 
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> >  drivers/video/fbdev/ssd1307fb.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 6fecec8..8a8d305 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -42,6 +42,11 @@
> >  #define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
> >  #define	SSD1307FB_SET_VCOMH		0xdb
> >  
> > +#define REFRASHRATE 1
> > +
> > +static u_int refreshrate = REFRASHRATE;
> 
> s/REFRASH/REFRESH/ :)

It's written the wrong way and the right way in the same line :) Of
course I will fix that.

Thanks,
Thomas

^ permalink raw reply

* [PATCH v1 00/47] mtrr/x86/drivers: bury MTRR
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@suse.com>

When a system has PAT support enabled you don't need to be
using MTRRs. Andy had added arch_phys_wc_add() long ago to
help with this but not all drivers were converted over. We
have to take care to only convert drivers where we know that
the proper ioremap_wc() API has been used. Doing this requires
a bit of work on verifying the driver split out the ioremap'd
areas -- and if not doing that ourselves. Verifying a driver
uses the same areas can be hard but with a bit of love Coccinelle
can help with that.

We're motivated to change drivers for a few reasons:

1) Take advantage of PAT when available

2) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (de33c442e)

3) Bury MTRR code away from drivers as it is architecture specific

While working on the conversion I noticed a few things.

a) Run time disabling of MTRR

Some systems can technically have both PAT and MTRR enabled
and even if they support it, a system may end up not enabling MTRR.
There are a few reasons why this can happen but the code right now
doesn't address this well. This leads to another point: PAT code
right now is not a first class citizen on x86 -- pat_init() depends
on MTRR code so we can't actually enable PAT without building MTRR.
Doing this requires quite a bit more work so let this serve as
a starting point for conversation if we want to address that.

b) Driver work and required ioremap split

In order to take advantage of PAT device drivers that were using
MTRR must make sure that the area that was using MTRR is ioremap'd
separately. Fortunately a lot of drivers already do this, but there's
quite a bit of drivers that require some love to get that happen.
This leaves us needing to expose an last resort API to annotate this
and also avoid a regression on performance for systems that may have
PAT but can't yet move away from using MTRR. To find the drivers that
need love check out __arch_phys_wc_add(). For a good example driver
where the work was done refer to the atyfb driver fixes.

c) Missing APIs for write-combining

There's a few API calls missing to take advantage of write-combining,
this series add those.

d) Further framebuffer driver MTRR usage simplication

We can simplify MTRR usage by having the framebuffer core
add the MTRR by passing a flag when register_framebuffer()
is called, this could for instance be done on very few drivers
where the smem_len and smem_start are both used for the ioremap_wc()
and also for the arch_phys_wc_add(). Coccinelle can be easily used
to do a transformation here. I didn't do that here given that it
does not work for all device drivers *and* DRM drivers already
have something similar. Lastly this technically could also be done
on some other generic helper --- but figured its best we review that
here. One reason to *not* do this is that tons of framebuffer drivers
have mtrr options exposed -- we'd need to generalize those and provide
a port ... or deal with the fact that we are going to remove all that.

Luis R. Rodriguez (47):
  x86: mtrr: annotate mtrr_type_lookup() is only implemented on
    generic_mtrr_ops
  x86: mtrr: generalize run time disabling of MTRR
  devres: add devm_ioremap_wc()
  pci: add pci_ioremap_wc_bar()
  pci: add pci_iomap_wc() variants
  mtrr: add __arch_phys_wc_add()
  video: fbdev: atyfb: move framebuffer length fudging to helper
  video: fbdev: atyfb: clarify ioremap() base and length used
  vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
  video: fbdev: atyfb: use arch_phys_wc_add() and ioremap_wc()
  IB/qib: add acounting for MTRR
  IB/qib: use arch_phys_wc_add()
  IB/ipath: add counting for MTRR
  IB/ipath: use __arch_phys_wc_add()
  [media] media: ivtv: use __arch_phys_wc_add()
  fusion: use __arch_phys_wc_add()
  video: fbdev: vesafb: only support MTRR_TYPE_WRCOMB
  vidoe: fbdev: vesafb: add missing mtrr_del() for added MTRR
  video: fbdev: vesafb: use arch_phys_wc_add()
  mtrr: avoid ifdef'ery with phys_wc_to_mtrr_index()
  ethernet: myri10ge: use arch_phys_wc_add()
  staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()
  staging: xgifb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
  video: fbdev: radeonfb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: gbefb: add missing mtrr_del() calls
  video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
  video: fbdev: intelfb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: matrox: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: neofb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
  video: fbdev: nvidia: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: savagefb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: sisfb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: aty: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: i810: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: i740fb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
  video: fbdev: kyrofb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
  video: fbdev: pm2fb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: pm3fb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: rivafb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: tdfxfb: use arch_phys_wc_add() and ioremap_wc()
  video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
  video: fbdev: atmel_lcdfb: use ioremap_wc() for framebuffer
  video: fbdev: geode gxfb: use ioremap_wc() for framebuffer
  video: fbdev: gxt4500: use pci_ioremap_wc_bar() for framebuffer
  mtrr: bury MTRR - unexport mtrr_add() and mtrr_del()

 Documentation/driver-model/devres.txt            |  1 +
 arch/x86/include/asm/io.h                        |  6 ++
 arch/x86/include/asm/mtrr.h                      |  7 +-
 arch/x86/kernel/cpu/mtrr/cleanup.c               |  2 +-
 arch/x86/kernel/cpu/mtrr/generic.c               |  7 +-
 arch/x86/kernel/cpu/mtrr/if.c                    |  3 +
 arch/x86/kernel/cpu/mtrr/main.c                  | 73 +++++++++++++------
 drivers/gpu/drm/drm_ioctl.c                      | 14 +---
 drivers/infiniband/hw/ipath/ipath_driver.c       |  7 +-
 drivers/infiniband/hw/ipath/ipath_kernel.h       |  4 +-
 drivers/infiniband/hw/ipath/ipath_wc_x86_64.c    | 47 +++++--------
 drivers/infiniband/hw/qib/qib_wc_x86_64.c        | 31 ++------
 drivers/media/pci/ivtv/ivtvfb.c                  | 51 ++++----------
 drivers/message/fusion/mptbase.c                 | 19 ++---
 drivers/message/fusion/mptbase.h                 |  2 +-
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 36 +++-------
 drivers/pci/pci.c                                | 14 ++++
 drivers/staging/sm750fb/sm750.c                  | 34 ++-------
 drivers/staging/sm750fb/sm750.h                  |  3 -
 drivers/staging/sm750fb/sm750_hw.c               |  3 +-
 drivers/staging/xgifb/XGI_main_26.c              | 27 ++-----
 drivers/video/fbdev/arkfb.c                      | 36 ++--------
 drivers/video/fbdev/atmel_lcdfb.c                |  3 +-
 drivers/video/fbdev/aty/aty128fb.c               | 36 ++--------
 drivers/video/fbdev/aty/atyfb.h                  |  5 +-
 drivers/video/fbdev/aty/atyfb_base.c             | 90 ++++++++----------------
 drivers/video/fbdev/aty/radeon_base.c            | 29 ++------
 drivers/video/fbdev/aty/radeonfb.h               |  2 +-
 drivers/video/fbdev/gbefb.c                      | 18 +++--
 drivers/video/fbdev/geode/gxfb_core.c            |  3 +-
 drivers/video/fbdev/gxt4500.c                    |  2 +-
 drivers/video/fbdev/i740fb.c                     | 35 ++-------
 drivers/video/fbdev/i810/i810.h                  |  3 +-
 drivers/video/fbdev/i810/i810_main.c             | 11 +--
 drivers/video/fbdev/i810/i810_main.h             | 26 -------
 drivers/video/fbdev/intelfb/intelfb.h            |  4 +-
 drivers/video/fbdev/intelfb/intelfbdrv.c         | 38 ++--------
 drivers/video/fbdev/kyro/fbdev.c                 | 33 +++------
 drivers/video/fbdev/matrox/matroxfb_base.c       | 36 ++++------
 drivers/video/fbdev/matrox/matroxfb_base.h       | 27 +------
 drivers/video/fbdev/neofb.c                      | 26 ++-----
 drivers/video/fbdev/nvidia/nv_type.h             |  7 +-
 drivers/video/fbdev/nvidia/nvidia.c              | 37 ++--------
 drivers/video/fbdev/pm2fb.c                      | 31 ++------
 drivers/video/fbdev/pm3fb.c                      | 30 ++------
 drivers/video/fbdev/riva/fbdev.c                 | 39 ++--------
 drivers/video/fbdev/riva/rivafb.h                |  4 +-
 drivers/video/fbdev/s3fb.c                       | 35 ++-------
 drivers/video/fbdev/savage/savagefb.h            |  4 +-
 drivers/video/fbdev/savage/savagefb_driver.c     | 17 +----
 drivers/video/fbdev/sis/sis.h                    |  2 +-
 drivers/video/fbdev/sis/sis_main.c               | 27 ++-----
 drivers/video/fbdev/tdfxfb.c                     | 41 ++---------
 drivers/video/fbdev/vesafb.c                     | 77 +++++++-------------
 drivers/video/fbdev/vt8623fb.c                   | 31 ++------
 include/asm-generic/pci_iomap.h                  | 14 ++++
 include/linux/io.h                               | 12 ++++
 include/linux/pci.h                              |  1 +
 include/video/kyro.h                             |  4 +-
 include/video/neomagic.h                         |  5 +-
 include/video/tdfx.h                             |  2 +-
 lib/devres.c                                     | 29 ++++++++
 lib/pci_iomap.c                                  | 61 ++++++++++++++++
 63 files changed, 463 insertions(+), 901 deletions(-)

-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply

* [PATCH v1 01/47] x86: mtrr: annotate mtrr_type_lookup() is only implemented on generic_mtrr_ops
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: 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, jbeulich,
	toshi.kani, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

There area few users of mtrr_type_lookup(), including PAT.
Note that PAT can be in theory enabled without MTRR fully
kicking in, such is the case with Xen.

Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: 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: venkatesh.pallipadi@intel.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: bhelgaas@google.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
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>
---
 arch/x86/kernel/cpu/mtrr/generic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 7d74f7b..09c82de 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -230,6 +230,8 @@ u8 mtrr_type_lookup(u64 start, u64 end)
 	int repeat;
 	u64 partial_end;
 
+	/* XXX: Currently only implemented on generic_mtrr_ops */
+
 	type = __mtrr_type_lookup(start, end, &partial_end, &repeat);
 
 	/*
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: 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, jbeulich,
	toshi.kani, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

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
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.

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.

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
also adds a few checks where they were not before
which could potentially safeguard ourselves against
incorrect usage of MTRR where this was not desirable.

Where possible match error codes as if MTRR was
disabled on arch/x86/include/asm/mtrr.h.

Lastly, since disabling MTRR can happen at run time
and we could end up with PAT enabled best record now
on our logs when MTRR is disabled.

[0] ~/devel/xen (git::stable-4.5)$ git describe --contains 586ab6a
4.4.0-rc1~18

Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: 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: venkatesh.pallipadi@intel.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: bhelgaas@google.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
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>
---
 arch/x86/include/asm/mtrr.h        |  2 ++
 arch/x86/kernel/cpu/mtrr/cleanup.c |  2 +-
 arch/x86/kernel/cpu/mtrr/generic.c |  5 +++--
 arch/x86/kernel/cpu/mtrr/if.c      |  3 +++
 arch/x86/kernel/cpu/mtrr/main.c    | 31 ++++++++++++++++++++++---------
 5 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
index f768f62..cade917 100644
--- a/arch/x86/include/asm/mtrr.h
+++ b/arch/x86/include/asm/mtrr.h
@@ -31,6 +31,7 @@
  * arch_phys_wc_add and arch_phys_wc_del.
  */
 # ifdef CONFIG_MTRR
+extern int mtrr_enabled;
 extern u8 mtrr_type_lookup(u64 addr, u64 end);
 extern void mtrr_save_fixed_ranges(void *);
 extern void mtrr_save_state(void);
@@ -50,6 +51,7 @@ extern int mtrr_trim_uncached_memory(unsigned long end_pfn);
 extern int amd_special_default_mtrr(void);
 extern int phys_wc_to_mtrr_index(int handle);
 #  else
+static const int mtrr_enabled;
 static inline u8 mtrr_type_lookup(u64 addr, u64 end)
 {
 	/*
diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
index 5f90b85..784dc55 100644
--- a/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -880,7 +880,7 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
 	 * Make sure we only trim uncachable memory on machines that
 	 * support the Intel MTRR architecture:
 	 */
-	if (!is_cpu(INTEL) || disable_mtrr_trim)
+	if (!is_cpu(INTEL) || disable_mtrr_trim || !mtrr_enabled)
 		return 0;
 
 	rdmsr(MSR_MTRRdefType, def, dummy);
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 09c82de..df321b2 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -116,7 +116,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat)
 	u8 prev_match, curr_match;
 
 	*repeat = 0;
-	if (!mtrr_state_set)
+	/* generic_mtrr_ops is only set for generic_mtrr_ops */
+	if (!mtrr_state_set || !mtrr_enabled)
 		return 0xFF;
 
 	if (!mtrr_state.enabled)
@@ -290,7 +291,7 @@ static void get_fixed_ranges(mtrr_type *frs)
 
 void mtrr_save_fixed_ranges(void *info)
 {
-	if (cpu_has_mtrr)
+	if (mtrr_enabled && cpu_has_mtrr)
 		get_fixed_ranges(mtrr_state.fixed_ranges);
 }
 
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index d76f13d..e9e001a 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -436,6 +436,9 @@ static int __init mtrr_if_init(void)
 {
 	struct cpuinfo_x86 *c = &boot_cpu_data;
 
+	if (!mtrr_enabled)
+		return 0;
+
 	if ((!cpu_has(c, X86_FEATURE_MTRR)) &&
 	    (!cpu_has(c, X86_FEATURE_K6_MTRR)) &&
 	    (!cpu_has(c, X86_FEATURE_CYRIX_ARR)) &&
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index ea5f363..7db9c47 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -59,6 +59,7 @@
 #define MTRR_TO_PHYS_WC_OFFSET 1000
 
 u32 num_var_ranges;
+int mtrr_enabled;
 
 unsigned int mtrr_usage_table[MTRR_MAX_VAR_RANGES];
 static DEFINE_MUTEX(mtrr_mutex);
@@ -84,6 +85,9 @@ static int have_wrcomb(void)
 {
 	struct pci_dev *dev;
 
+	if (!mtrr_enabled)
+		return 0;
+
 	dev = pci_get_class(PCI_CLASS_BRIDGE_HOST << 8, NULL);
 	if (dev != NULL) {
 		/*
@@ -286,7 +290,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 	int i, replace, error;
 	mtrr_type ltype;
 
-	if (!mtrr_if)
+	if (!mtrr_enabled)
 		return -ENXIO;
 
 	error = mtrr_if->validate_add_page(base, size, type);
@@ -388,6 +392,8 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 
 static int mtrr_check(unsigned long base, unsigned long size)
 {
+	if (!mtrr_enabled)
+		return -ENODEV;
 	if ((base & (PAGE_SIZE - 1)) || (size & (PAGE_SIZE - 1))) {
 		pr_warning("mtrr: size and base must be multiples of 4 kiB\n");
 		pr_debug("mtrr: size: 0x%lx  base: 0x%lx\n", size, base);
@@ -463,8 +469,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
 	unsigned long lbase, lsize;
 	int error = -EINVAL;
 
-	if (!mtrr_if)
-		return -ENXIO;
+	if (!mtrr_enabled)
+		return -ENODEV;
 
 	max = num_var_ranges;
 	/* No CPU hotplug when we change MTRR entries */
@@ -523,6 +529,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
  */
 int mtrr_del(int reg, unsigned long base, unsigned long size)
 {
+	if (!mtrr_enabled)
+		return -ENODEV;
 	if (mtrr_check(base, size))
 		return -EINVAL;
 	return mtrr_del_page(reg, base >> PAGE_SHIFT, size >> PAGE_SHIFT);
@@ -545,7 +553,7 @@ int arch_phys_wc_add(unsigned long base, unsigned long size)
 {
 	int ret;
 
-	if (pat_enabled)
+	if (pat_enabled || !mtrr_enabled)
 		return 0;  /* Success!  (We don't need to do anything.) */
 
 	ret = mtrr_add(base, size, MTRR_TYPE_WRCOMB, true);
@@ -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()) {
@@ -744,12 +753,13 @@ void __init mtrr_bp_init(void)
 				mtrr_if->set_all();
 			}
 		}
-	}
+	} else
+		pr_info("mtrr: system does not support MTRR\n");
 }
 
 void mtrr_ap_init(void)
 {
-	if (!use_intel() || mtrr_aps_delayed_init)
+	if (!use_intel() || mtrr_aps_delayed_init || !mtrr_enabled)
 		return;
 	/*
 	 * Ideally we should hold mtrr_mutex here to avoid mtrr entries
@@ -774,6 +784,9 @@ void mtrr_save_state(void)
 {
 	int first_cpu;
 
+	if (!mtrr_enabled)
+		return;
+
 	get_online_cpus();
 	first_cpu = cpumask_first(cpu_online_mask);
 	smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
@@ -782,7 +795,7 @@ void mtrr_save_state(void)
 
 void set_mtrr_aps_delayed_init(void)
 {
-	if (!use_intel())
+	if (!use_intel() || !mtrr_enabled)
 		return;
 
 	mtrr_aps_delayed_init = true;
@@ -810,7 +823,7 @@ void mtrr_aps_init(void)
 
 void mtrr_bp_restore(void)
 {
-	if (!use_intel())
+	if (!use_intel() || !mtrr_enabled)
 		return;
 
 	mtrr_if->set_all();
@@ -818,7 +831,7 @@ void mtrr_bp_restore(void)
 
 static int __init mtrr_init_finialize(void)
 {
-	if (!mtrr_if)
+	if (!mtrr_enabled)
 		return 0;
 
 	if (use_intel()) {
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 03/47] devres: add devm_ioremap_wc()
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

We have devm_ioremap_nocache() but no devm_ioremap_wc()
so add that. This will be used later.

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 Documentation/driver-model/devres.txt |  1 +
 include/linux/io.h                    |  2 ++
 lib/devres.c                          | 29 +++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index e1e2bbd..831a536 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -276,6 +276,7 @@ IOMAP
   devm_ioport_unmap()
   devm_ioremap()
   devm_ioremap_nocache()
+  devm_ioremap_wc()
   devm_ioremap_resource() : checks resource, requests memory region, ioremaps
   devm_iounmap()
   pcim_iomap()
diff --git a/include/linux/io.h b/include/linux/io.h
index 4cc299c..91101a1 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -72,6 +72,8 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
 			   resource_size_t size);
 void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
 				   resource_size_t size);
+void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
+			      resource_size_t size);
 void devm_iounmap(struct device *dev, void __iomem *addr);
 int check_signature(const volatile void __iomem *io_addr,
 			const unsigned char *signature, int length);
diff --git a/lib/devres.c b/lib/devres.c
index 0f1dd2e..2eb2bfe 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -72,6 +72,35 @@ void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
 EXPORT_SYMBOL(devm_ioremap_nocache);
 
 /**
+ * devm_ioremap_wc - Managed ioremap_wc()
+ * @dev: Generic device to remap IO address for
+ * @offset: BUS offset to map
+ * @size: Size of map
+ *
+ * Managed ioremap_wc().  Map is automatically unmapped on driver
+ * detach.
+ */
+void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
+			      resource_size_t size)
+{
+	void __iomem **ptr, *addr;
+
+	ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	addr = ioremap_wc(offset, size);
+	if (addr) {
+		*ptr = addr;
+		devres_add(dev, ptr);
+	} else
+		devres_free(ptr);
+
+	return addr;
+}
+EXPORT_SYMBOL_GPL(devm_ioremap_wc);
+
+/**
  * devm_iounmap - Managed iounmap()
  * @dev: Generic device to unmap for
  * @addr: Address to unmap
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 04/47] pci: add pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This lets drivers take advanate of PAT when available. This
should help with the transition of converting video drivers over
to ioremap_wc() to help with the goal of eventually using
_PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
ioremap_nocache() (de33c442e)

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/pci/pci.c   | 14 ++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 81f06e8..6afd507 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -137,6 +137,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
 				     pci_resource_len(pdev, bar));
 }
 EXPORT_SYMBOL_GPL(pci_ioremap_bar);
+
+void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
+{
+	/*
+	 * Make sure the BAR is actually a memory resource, not an IO resource
+	 */
+	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
+		WARN_ON(1);
+		return NULL;
+	}
+	return ioremap_wc(pci_resource_start(pdev, bar),
+			  pci_resource_len(pdev, bar));
+}
+EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
 #endif
 
 #define PCI_FIND_CAP_TTL	48
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 211e9da..c235b09 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1667,6 +1667,7 @@ static inline void pci_mmcfg_late_init(void) { }
 int pci_ext_cfg_avail(void);
 
 void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar);
+void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar);
 
 #ifdef CONFIG_PCI_IOV
 int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 05/47] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Bjorn Helgaas, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Dave Hansen,
	Arnd Bergmann, Michael S. Tsirkin, Stefan Bader, konrad.wilk,
	ville.syrjala, david.vrabel, jbeulich, toshi.kani,
	Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

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
x86 when a system does not have PAT) we void polluting
pci_iomap() space with it and force drivers and subsystems
that want to use it to be explicit.

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() (de33c442e)

Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: venkatesh.pallipadi@intel.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: 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>
---
 include/asm-generic/pci_iomap.h | 14 ++++++++++
 lib/pci_iomap.c                 | 61 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index 7389c87..b1e17fc 100644
--- a/include/asm-generic/pci_iomap.h
+++ b/include/asm-generic/pci_iomap.h
@@ -15,9 +15,13 @@ struct pci_dev;
 #ifdef CONFIG_PCI
 /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
 extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max);
 extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
 				     unsigned long offset,
 				     unsigned long maxlen);
+extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+					unsigned long offset,
+					unsigned long maxlen);
 /* Create a virtual mapping cookie for a port on a given PCI device.
  * Do not call this directly, it exists to make it easier for architectures
  * to override */
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
 	return NULL;
 }
 
+static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max)
+{
+	return NULL;
+}
 static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
 					    unsigned long offset,
 					    unsigned long maxlen)
 {
 	return NULL;
 }
+static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+					       unsigned long offset,
+					       unsigned long maxlen)
+{
+	return NULL;
+}
 #endif
 
 #endif /* __ASM_GENERIC_IO_H */
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.
+ * */
+void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
+				 int bar,
+				 unsigned long offset,
+				 unsigned long maxlen)
+{
+	resource_size_t start = pci_resource_start(dev, bar);
+	resource_size_t len = pci_resource_len(dev, bar);
+	unsigned long flags = pci_resource_flags(dev, bar);
+
+	if (len <= offset || !start)
+		return NULL;
+	len -= offset;
+	start += offset;
+	if (maxlen && len > maxlen)
+		len = maxlen;
+	if (flags & IORESOURCE_IO)
+		return __pci_ioport_map(dev, start, len);
+	if (flags & IORESOURCE_MEM)
+		return ioremap_wc(start, len);
+	/* What? */
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
+
+/**
  * pci_iomap - create a virtual mapping cookie for a PCI BAR
  * @dev: PCI device that owns the BAR
  * @bar: BAR number
@@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
 	return pci_iomap_range(dev, bar, 0, maxlen);
 }
 EXPORT_SYMBOL(pci_iomap);
+
+/**
+ * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @maxlen: 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 without checking for its length first, pass %0 here.
+ * */
+void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+	return pci_iomap_wc_range(dev, bar, 0, maxlen);
+}
+EXPORT_SYMBOL_GPL(pci_iomap_wc);
 #endif /* CONFIG_PCI */
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

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.

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 arch/x86/include/asm/io.h       |  4 ++++
 arch/x86/kernel/cpu/mtrr/main.c | 36 +++++++++++++++++++++++++++++-------
 include/linux/io.h              |  4 ++++
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 34a5b93..a144d05 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -338,6 +338,10 @@ extern bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
 #define IO_SPACE_LIMIT 0xffff
 
 #ifdef CONFIG_MTRR
+extern int __must_check __arch_phys_wc_add(unsigned long base,
+					   unsigned long size);
+#define __arch_phys_wc_add __arch_phys_wc_add
+
 extern int __must_check arch_phys_wc_add(unsigned long base,
 					 unsigned long size);
 extern void arch_phys_wc_del(int handle);
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 7db9c47..5ae830b 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -538,23 +538,24 @@ int mtrr_del(int reg, unsigned long base, unsigned long size)
 EXPORT_SYMBOL(mtrr_del);
 
 /**
- * arch_phys_wc_add - add a WC MTRR and handle errors if PAT is unavailable
+ * __arch_phys_wc_add - add a WC MTRR even if PAT is available
  * @base: Physical base address
  * @size: Size of region
  *
- * If PAT is available, this does nothing.  If PAT is unavailable, it
- * attempts to add a WC MTRR covering size bytes starting at base and
- * logs an error if this fails.
+ * We typically do not want to use MTRR if PAT is available but there
+ * are some drivers which require significant work to get this to work
+ * properly. This call should only be used by those drivers where it is
+ * clear that hard work is required to modify them to use arch_phys_wc_add()
  *
  * Drivers must store the return value to pass to mtrr_del_wc_if_needed,
  * but drivers should not try to interpret that return value.
  */
-int arch_phys_wc_add(unsigned long base, unsigned long size)
+int __arch_phys_wc_add(unsigned long base, unsigned long size)
 {
 	int ret;
 
-	if (pat_enabled || !mtrr_enabled)
-		return 0;  /* Success!  (We don't need to do anything.) */
+	if (!mtrr_enabled)
+		return 0;
 
 	ret = mtrr_add(base, size, MTRR_TYPE_WRCOMB, true);
 	if (ret < 0) {
@@ -564,6 +565,27 @@ int arch_phys_wc_add(unsigned long base, unsigned long size)
 	}
 	return ret + MTRR_TO_PHYS_WC_OFFSET;
 }
+EXPORT_SYMBOL_GPL(__arch_phys_wc_add);
+
+/**
+ * arch_phys_wc_add - add a WC MTRR and handle errors if PAT is unavailable
+ * @base: Physical base address
+ * @size: Size of region
+ *
+ * If PAT is available, this does nothing.  If PAT is unavailable, it
+ * attempts to add a WC MTRR covering size bytes starting at base and
+ * logs an error if this fails.
+ *
+ * Drivers must store the return value to pass to mtrr_del_wc_if_needed,
+ * but drivers should not try to interpret that return value.
+ */
+int arch_phys_wc_add(unsigned long base, unsigned long size)
+{
+	if (pat_enabled || !mtrr_enabled)
+		return 0;  /* Success!  (We don't need to do anything.) */
+
+	return __arch_phys_wc_add(base, size);
+}
 EXPORT_SYMBOL(arch_phys_wc_add);
 
 /*
diff --git a/include/linux/io.h b/include/linux/io.h
index 91101a1..ecc51c3 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -111,6 +111,10 @@ static inline void arch_phys_wc_del(int handle)
 }
 
 #define arch_phys_wc_add arch_phys_wc_add
+#ifndef __arch_phys_wc_add
+#define __arch_phys_wc_add arch_phys_wc_add
+#endif
+
 #endif
 
 #endif /* _LINUX_IO_H */
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 07/47] video: fbdev: atyfb: move framebuffer length fudging to helper
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: 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: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

The size of the framebuffer to be used needs to
be fudged to account for the different type of
devices that are out there. This captures what
is required to do well, we'll resuse this later.

This has no functional changes.

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
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: 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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/aty/atyfb_base.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index 8789e48..16936bb 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -427,6 +427,20 @@ static struct {
 #endif /* CONFIG_FB_ATY_CT */
 };
 
+/*
+ * Last page of 8 MB (4 MB on ISA) aperture is MMIO,
+ * unless the auxiliary register aperture is used.
+ */
+static void aty_fudge_framebuffer_len(struct fb_info *info)
+{
+	struct atyfb_par *par = (struct atyfb_par *) info->par;
+
+	if (!par->aux_start &&
+	    (info->fix.smem_len = 0x800000 ||
+	     (par->bus_type = ISA && info->fix.smem_len = 0x400000)))
+		info->fix.smem_len -= GUI_RESERVE;
+}
+
 static int correct_chipset(struct atyfb_par *par)
 {
 	u8 rev;
@@ -2603,14 +2617,7 @@ static int aty_init(struct fb_info *info)
 	if (par->pll_ops->resume_pll)
 		par->pll_ops->resume_pll(info, &par->pll);
 
-	/*
-	 * Last page of 8 MB (4 MB on ISA) aperture is MMIO,
-	 * unless the auxiliary register aperture is used.
-	 */
-	if (!par->aux_start &&
-	    (info->fix.smem_len = 0x800000 ||
-	     (par->bus_type = ISA && info->fix.smem_len = 0x400000)))
-		info->fix.smem_len -= GUI_RESERVE;
+	aty_fudge_framebuffer_len(info);
 
 	/*
 	 * Disable register access through the linear aperture
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 08/47] video: fbdev: atyfb: clarify ioremap() base and length used
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: 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: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This has no functional changes, it just adjusts
the ioremap() call for the framebuffer to use
the same values we later use for the framebuffer,
this will make it easier to review the next change.

The size of the framebuffer varies but since this is
for PCI we *know* this defaults to 0x800000.
atyfb_setup_generic() is *only* used on PCI probe.

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
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: 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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/aty/atyfb_base.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index 16936bb..8025624 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -3489,7 +3489,9 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
 
 	/* Map in frame buffer */
 	info->fix.smem_start = addr;
-	info->screen_base = ioremap(addr, 0x800000);
+	info->fix.smem_len = 0x800000;
+
+	info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
 	if (info->screen_base = NULL) {
 		ret = -ENOMEM;
 		goto atyfb_setup_generic_fail;
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO "hole" work around
From: Luis R. Rodriguez @ 2015-03-20 23:17 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: 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: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

The atyfb driver uses an MTRR work around since some
cards use the same PCI BAR for the framebuffer and MMIO.
In such cards the last page is used for MMIO, the rest for
the framebuffer, so on those cards we ioremap() the MMIO
page alone, then again ioremap() the full framebuffer
including the MMIO space *and* ___then___ use an MTRR with
MTRR_TYPE_WRCOMB on the full PCI BAR... and finally "hole"
in an MTRR_TYPE_UNCACHABLE MTRR only for MMIO.

This is a terrible fucking work around, and should by no means
be necessary however evidence through a large series of conversion
of drivers to ioremap_wc() for the framebuffer shows that around
the time MTRR started becoming popular devices did not have things
lined up for easily separating the framebuffer and MMIO register
access. In some cases a driver requires significant intrusive
changes in order to make the split for an ioremap() for MMIO registers
and another ioremap_wc() for the framebuffer, at other times a
bit of careful study of the driver suffices. This example driver
falls into the later category.

We can replace the MTRR MTRR_TYPE_UNCACHABLE
work around by using ioremap_nocache(), the length of the
MMIO space should already be correct. The other part we
need to correct is ensuring we ioremap() for the framebuffer
only the required size. Since the ioremap() happens early
on probe for PCI devices before aty_init() where we typically
adjust the length and know how to do it, we can fix this by
pegging the bus type as PCI on PCI probe, and finally fudging
and framebuffer length just as we do on aty_init().

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.

This fix lets us nuke the MTRR_TYPE_UNCACHABLE MTRR "hole".

Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
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: 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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/aty/atyfb.h      |  1 -
 drivers/video/fbdev/aty/atyfb_base.c | 28 ++++++----------------------
 2 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/video/fbdev/aty/atyfb.h b/drivers/video/fbdev/aty/atyfb.h
index 1f39a62..89ec439 100644
--- a/drivers/video/fbdev/aty/atyfb.h
+++ b/drivers/video/fbdev/aty/atyfb.h
@@ -184,7 +184,6 @@ struct atyfb_par {
 	spinlock_t int_lock;
 #ifdef CONFIG_MTRR
 	int mtrr_aper;
-	int mtrr_reg;
 #endif
 	u32 mem_cntl;
 	struct crtc saved_crtc;
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);
 	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


^ permalink raw reply related

* [PATCH v1 10/47] video: fbdev: atyfb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-03-20 23:18 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This driver uses the same area for MTRR as for the ioremap().
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, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

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() (de33c442e)

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: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/aty/atyfb.h      |  4 +---
 drivers/video/fbdev/aty/atyfb_base.c | 41 +++++++++---------------------------
 2 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/video/fbdev/aty/atyfb.h b/drivers/video/fbdev/aty/atyfb.h
index 89ec439..63c4842 100644
--- a/drivers/video/fbdev/aty/atyfb.h
+++ b/drivers/video/fbdev/aty/atyfb.h
@@ -182,9 +182,7 @@ struct atyfb_par {
 	unsigned long irq_flags;
 	unsigned int irq;
 	spinlock_t int_lock;
-#ifdef CONFIG_MTRR
-	int mtrr_aper;
-#endif
+	int wc_cookie;
 	u32 mem_cntl;
 	struct crtc saved_crtc;
 	union aty_pll saved_pll;
diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index 8875e56..af278bb 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -98,9 +98,6 @@
 #ifdef CONFIG_PMAC_BACKLIGHT
 #include <asm/backlight.h>
 #endif
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
 
 /*
  * Debug flags.
@@ -303,9 +300,7 @@ static struct fb_ops atyfb_ops = {
 };
 
 static bool noaccel;
-#ifdef CONFIG_MTRR
 static bool nomtrr;
-#endif
 static int vram;
 static int pll;
 static int mclk;
@@ -2628,14 +2623,9 @@ static int aty_init(struct fb_info *info)
 		aty_st_le32(BUS_CNTL, aty_ld_le32(BUS_CNTL, par) |
 			    BUS_APER_REG_DIS, par);
 
-#ifdef CONFIG_MTRR
-	par->mtrr_aper = -1;
-	if (!nomtrr) {
-		par->mtrr_aper = mtrr_add(info->fix.smem_start,
-					  info->fix.smem_len,
-					  MTRR_TYPE_WRCOMB, 1);
-	}
-#endif
+	if (!nomtrr)
+		par->wc_cookie = arch_phys_wc_add(info->fix.smem_start,
+						  info->fix.smem_len);
 
 	info->fbops = &atyfb_ops;
 	info->pseudo_palette = par->pseudo_palette;
@@ -2763,13 +2753,8 @@ aty_init_exit:
 	/* restore video mode */
 	aty_set_crtc(par, &par->saved_crtc);
 	par->pll_ops->set_pll(info, &par->saved_pll);
+	arch_phys_wc_del(par->wc_cookie);
 
-#ifdef CONFIG_MTRR
-	if (par->mtrr_aper >= 0) {
-		mtrr_del(par->mtrr_aper, 0, 0);
-		par->mtrr_aper = -1;
-	}
-#endif
 	return ret;
 }
 
@@ -3478,7 +3463,8 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
 
 	aty_fudge_framebuffer_len(info);
 
-	info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
+	info->screen_base = ioremap_wc(info->fix.smem_start,
+				       info->fix.smem_len);
 	if (info->screen_base = NULL) {
 		ret = -ENOMEM;
 		goto atyfb_setup_generic_fail;
@@ -3652,7 +3638,8 @@ static int __init atyfb_atari_probe(void)
 		 * Map the video memory (physical address given)
 		 * to somewhere in the kernel address space.
 		 */
-		info->screen_base = ioremap(phys_vmembase[m64_num], phys_size[m64_num]);
+		info->screen_base = ioremap_wc(phys_vmembase[m64_num],
+					       phys_size[m64_num]);
 		info->fix.smem_start = (unsigned long)info->screen_base; /* Fake! */
 		par->ati_regbase = ioremap(phys_guiregbase[m64_num], 0x10000) +
 						0xFC00ul;
@@ -3719,12 +3706,8 @@ static void atyfb_remove(struct fb_info *info)
 		aty_bl_exit(info->bl_dev);
 #endif
 
-#ifdef CONFIG_MTRR
-	if (par->mtrr_aper >= 0) {
-		mtrr_del(par->mtrr_aper, 0, 0);
-		par->mtrr_aper = -1;
-	}
-#endif
+	arch_phys_wc_del(par->wc_cookie);
+
 #ifndef __sparc__
 	if (par->ati_regbase)
 		iounmap(par->ati_regbase);
@@ -3840,10 +3823,8 @@ static int __init atyfb_setup(char *options)
 	while ((this_opt = strsep(&options, ",")) != NULL) {
 		if (!strncmp(this_opt, "noaccel", 7)) {
 			noaccel = 1;
-#ifdef CONFIG_MTRR
 		} else if (!strncmp(this_opt, "nomtrr", 6)) {
 			nomtrr = 1;
-#endif
 		} else if (!strncmp(this_opt, "vram:", 5))
 			vram = simple_strtoul(this_opt + 5, NULL, 0);
 		else if (!strncmp(this_opt, "pll:", 4))
@@ -4013,7 +3994,5 @@ module_param(comp_sync, int, 0);
 MODULE_PARM_DESC(comp_sync, "Set composite sync signal to low (0) or high (1)");
 module_param(mode, charp, 0);
 MODULE_PARM_DESC(mode, "Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
-#ifdef CONFIG_MTRR
 module_param(nomtrr, bool, 0);
 MODULE_PARM_DESC(nomtrr, "bool: disable use of MTRR registers");
-#endif
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 11/47] IB/qib: add acounting for MTRR
From: Luis R. Rodriguez @ 2015-03-20 23:18 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-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: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/infiniband/hw/qib/qib_wc_x86_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/qib/qib_wc_x86_64.c b/drivers/infiniband/hw/qib/qib_wc_x86_64.c
index 81b225f..fe0850a 100644
--- a/drivers/infiniband/hw/qib/qib_wc_x86_64.c
+++ b/drivers/infiniband/hw/qib/qib_wc_x86_64.c
@@ -118,7 +118,7 @@ int qib_enable_wc(struct qib_devdata *dd)
 	if (!ret) {
 		int cookie;
 
-		cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
+		cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
 		if (cookie < 0) {
 			{
 				qib_devinfo(dd->pcidev,
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 12/47] IB/qib: use arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-03-20 23:18 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Rickard Strandqvist, Dennis Dalessandro, Mike Marciniszyn,
	Roland Dreier, Ingo Molnar, Daniel Vetter, Bjorn Helgaas,
	Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Dave Hansen, Arnd Bergmann, Michael S. Tsirkin, Stefan Bader,
	konrad.wilk, ville.syrjala, david.vrabel, jbeulich, toshi.kani,
	Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This driver already makes use of ioremap_wc() on PIO buffers,
so convert it to use arch_phys_wc_add().

Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: venkatesh.pallipadi@intel.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: 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/qib/qib_wc_x86_64.c | 31 ++++---------------------------
 1 file changed, 4 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_wc_x86_64.c b/drivers/infiniband/hw/qib/qib_wc_x86_64.c
index fe0850a..6d61ef9 100644
--- a/drivers/infiniband/hw/qib/qib_wc_x86_64.c
+++ b/drivers/infiniband/hw/qib/qib_wc_x86_64.c
@@ -116,21 +116,9 @@ int qib_enable_wc(struct qib_devdata *dd)
 	}
 
 	if (!ret) {
-		int cookie;
-
-		cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
-		if (cookie < 0) {
-			{
-				qib_devinfo(dd->pcidev,
-					 "mtrr_add()  WC for PIO bufs failed (%d)\n",
-					 cookie);
-				ret = -EINVAL;
-			}
-		} else {
-			dd->wc_cookie = cookie;
-			dd->wc_base = (unsigned long) pioaddr;
-			dd->wc_len = (unsigned long) piolen;
-		}
+		dd->wc_cookie = arch_phys_wc_add(pioaddr, piolen);
+		if (dd->wc_cookie < 0)
+			ret = -EINVAL;
 	}
 
 	return ret;
@@ -142,18 +130,7 @@ int qib_enable_wc(struct qib_devdata *dd)
  */
 void qib_disable_wc(struct qib_devdata *dd)
 {
-	if (dd->wc_cookie) {
-		int r;
-
-		r = mtrr_del(dd->wc_cookie, dd->wc_base,
-			     dd->wc_len);
-		if (r < 0)
-			qib_devinfo(dd->pcidev,
-				 "mtrr_del(%lx, %lx, %lx) failed: %d\n",
-				 dd->wc_cookie, dd->wc_base,
-				 dd->wc_len, r);
-		dd->wc_cookie = 0; /* even on failure */
-	}
+	arch_phys_wc_del(dd->wc_cookie);
 }
 
 /**
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v1 13/47] IB/ipath: add counting for MTRR
From: Luis R. Rodriguez @ 2015-03-20 23:18 UTC (permalink / raw)
  To: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied
  Cc: linux-kernel, linux-fbdev, x86, xen-devel, Luis R. Rodriguez,
	Ingo Molnar, Daniel Vetter, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-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: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.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: 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox