* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Andy Walls @ 2015-04-15 23:59 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Luis R. Rodriguez, Toshi Kani, H. Peter Anvin, Ingo Molnar,
linux-kernel@vger.kernel.org, Hal Rosenstock, Sean Hefty,
Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Juergen Gross, Mauro Carvalho Chehab,
Borislav Petkov, Mel Gorman, Vlastimil Babka, Davidlohr Bueso,
Dave Hansen, Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list,
linux-media, X86 ML
In-Reply-To: <CALCETrUFtEMYh8i00ke0f939=17bAQxMDOBZMn_3yk3Nz1AnFA@mail.gmail.com>
On Wed, 2015-04-15 at 16:42 -0700, Andy Lutomirski wrote:
> On Wed, Apr 15, 2015 at 3:38 PM, Andy Walls <awalls@md.metrocast.net> wrote:
> >
>
> IMO the right solution would be to avoid ioremapping the whole bar at
> startup. Instead ioremap pieces once the driver learns what they are.
> This wouldn't have any of these problems -- you'd ioremap() register
> regions and you'd ioremap_wc() the framebuffer once you find it. If
> there are regions of unknown purpose, just don't map them all.
>
> Would this be feasible?
Feasible? Maybe.
Worth the time and effort for end-of-life, convential PCI hardware so I
can have an optimally performing X display on a Standard Def Analog TV
screen? Nope. I don't have that level of nostalgia.
We sort of know where some things are in the MMIO space due to
experimentation and past efforts examining the firmware binary.
Documentation/video4linux/cx2341x/fw-*.txt documents some things. The
driver code actually codifies a little bit more knowledge.
The driver code for doing transfers between host and card is complex and
fragile with some streams that use DMA, other streams that use PIO,
digging VBI data straight out of card memory, and scatter-gather being
broken on newer firmwares. Playing around with ioremapping will be hard
to get right and likely cause something in the code to break for the
primary use case of the ivtv supported cards.
Regards,
Andy
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Luis R. Rodriguez @ 2015-04-15 23:58 UTC (permalink / raw)
To: Andy Walls, Hyong-Youb Kim, netdev
Cc: Andy Lutomirski, Toshi Kani, H. Peter Anvin, Ingo Molnar,
linux-kernel, Hal Rosenstock, Sean Hefty, Suresh Siddha,
Rickard Strandqvist, Mike Marciniszyn, Roland Dreier,
Juergen Gross, Mauro Carvalho Chehab, Borislav Petkov, Mel Gorman,
Vlastimil Babka, Davidlohr Bueso, dave.hansen, plagnioj, tglx,
Ville Syrjälä, linux-fbdev, linux-media, x86
In-Reply-To: <1429137531.1899.28.camel@palomino.walls.org>
Hey Andy, thanks for your review, adding Hyong-Youb Kim for review of the
full range ioremap_wc() idea below.
On Wed, Apr 15, 2015 at 06:38:51PM -0400, Andy Walls wrote:
> Hi All,
>
> On Mon, 2015-04-13 at 19:49 +0200, Luis R. Rodriguez wrote:
> > From the beginning it seems only framebuffer devices used MTRR/WC,
> [snip]
> > The ivtv device is a good example of the worst type of
> > situations and these days. So perhap __arch_phys_wc_add() and a
> > ioremap_ucminus() might be something more than transient unless hardware folks
> > get a good memo or already know how to just Do The Right Thing (TM).
>
> Just to reiterate a subtle point, use of the ivtvfb is *optional*. A
> user may or may not load it. When the user does load the ivtvfb driver,
> the ivtv driver has already been initialized and may have functions of
> the card already in use by userspace.
I suspected this and its why I note that a rewrite to address a clean
split with separate ioremap seems rather difficult in this case.
> Hopefully no one is trying to use the OSD as framebuffer and the video
> decoder/output engine for video display at the same time.
Worst case concern I have also is the implications of having overlapping
ioremap() calls (as proposed in my last reply) for different memory types
and having the different virtual memory addresse used by different parts
of the driver. Its not clear to me what the hardware implications of this
are.
> But the video
> decoder/output device nodes may already be open for performing ioctl()
> functions so unmapping the decoder IO space out from under them, when
> loading the ivtvfb driver module, might not be a good thing.
Using overlapping ioremap() calls with different memory types would address
this concern provided hardware won't barf both on the device and CPU. Hardware
folks could provide feedback or an ivtvfb user could test the patch supplied
on both non-PAT and PAT systems. Even so, who knows, this might work on some
systems while not on others, only hardware folks would know.
An alternative... is to just ioremap_wc() the entire region, including
MMIO registers for these old devices. I see one ethernet driver that does
this, myri10ge, and am curious how and why they ended up deciding this
and if they have run into any issues. I wonder if this is a reasonable
comrpomise for these 2 remaining corner cases.
Luis
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Andy Lutomirski @ 2015-04-15 23:52 UTC (permalink / raw)
To: Andy Walls
Cc: Luis R. Rodriguez, linux-rdma, Toshi Kani, H. Peter Anvin,
Ingo Molnar, linux-kernel@vger.kernel.org, Hal Rosenstock,
Sean Hefty, Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Juergen Gross, Mauro Carvalho Chehab,
Borislav Petkov, Mel Gorman, Vlastimil Babka, Davidlohr Bueso,
Dave Hansen, Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list,
linux-media, X86 ML
In-Reply-To: <1429138212.1899.34.camel@palomino.walls.org>
On Wed, Apr 15, 2015 at 3:50 PM, Andy Walls <awalls@md.metrocast.net> wrote:
> On Wed, 2015-04-15 at 13:42 -0700, Andy Lutomirski wrote:
>> On Mon, Apr 13, 2015 at 10:49 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>
>> > c) ivtv: the driver does not have the PCI space mapped out separately, and
>> > in fact it actually does not do the math for the framebuffer, instead it lets
>> > the device's own CPU do that and assume where its at, see
>> > ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
>> > but not a setter. Its not clear if the firmware would make a split easy.
>> > We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
>> >
>>
>> IMO this should be conceptually easy to split. Once we get the
>> framebuffer address, just unmap it (or don't prematurely map it) and
>> then ioremap the thing.
>
> Not so easy. The main ivtv driver has already set up the PCI device and
> done the mapping for the MPEG-2 decoder/video output engine. The video
> decoder/output device nodes might already be open by user space calling
> into the main driver, before the ivtvfb module is even loaded.
Surely the MPEG-2 decoder/video engine won't overlap the framebuffer,
though. Am I missing something?
--Andy
>
> This could be mitigated by integrating all the ivtvfb module code into
> the main ivtv module. But even then not every PVR-350 owner wants to
> use the video output OSD as a framebuffer. Users might just want an
> actual OSD overlaying their TV video playback.
>
> Regards,
> Andy
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Andy Lutomirski @ 2015-04-15 23:42 UTC (permalink / raw)
To: Andy Walls
Cc: Luis R. Rodriguez, Toshi Kani, H. Peter Anvin, Ingo Molnar,
linux-kernel@vger.kernel.org, Hal Rosenstock, Sean Hefty,
Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Juergen Gross, Mauro Carvalho Chehab,
Borislav Petkov, Mel Gorman, Vlastimil Babka, Davidlohr Bueso,
Dave Hansen, Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list,
linux-media, X86 ML
In-Reply-To: <1429137531.1899.28.camel@palomino.walls.org>
On Wed, Apr 15, 2015 at 3:38 PM, Andy Walls <awalls@md.metrocast.net> wrote:
> Hi All,
>
> On Mon, 2015-04-13 at 19:49 +0200, Luis R. Rodriguez wrote:
> [snip]
>> I only saw a few drivers using overlapping ioremap*()
>> calls though on my MTRR review and they are all old devices so likely mostly
>> used on non-PAT systems, but there might be other corner cases elsewhere.
>>
>> Lets recap, as I see it we have a few options with all this in mind on our
>> quest to bury MTRR (and later make strong UC default):
>>
>> 1) Let drivers do their own get_vm_area() calls as you note and handle the
>> cut and splicing of ioremap areas
>>
>> 2) Provide an API to split and splice ioremap ranges
>>
>> 3) Try to avoid these types of situations and let drivers simply try to
>> work towards a proper clean non-overlapping different ioremap() ranges
>>
>> Let me know if you think of others but please keep in mind the UC- to strong
>> UC converstion we want to do later as well. We have ruled out using
>> set_memor_wc() now.
>>
>> I prefer option 3), its technically possible but only for *new* drivers
>> and we'd need either some hard work to split the ioremap() areas or provide
>> a a set of *transient* APIs as I had originally proposed to phase / hope for
>> final transition. There are 3 drivers to address:
>>
>> [snip]
>
> Just some background for folks:
> The ivtv driver supports cards that perform Standard Definition PAL,
> NTSC, and SECAM TV capture + hardware MPEG-2 encoding and MPEG-2
> decoding + TV output.
>
> Of the supported cards only *one* card type, the PVR-350 based on the
> CX23415 chip, can perform the MPEG-2 or YUV video decoding and output,
> with an OSD overlay. PVR-350's are legacy PCI cards that have been end
> of life since 2088 or earlier. Despite that, there are still used units
> available on Amazon and eBay.
>
> The ivtvfb driver module is an *optional* companion driver module that
> provides a framebuffer interface for the user to output an X display, FB
> console, or whatever to a standard definition analog PAL, NTSC, or SECAM
> TV or VCR. It does this by co-opting the OSD overlay of the video
> decoding output engine and having it take up the whole screen.
>
>
>
>> c) ivtv: the driver does not have the PCI space mapped out separately, and
>> in fact it actually does not do the math for the framebuffer, instead it lets
>> the device's own CPU do that and assume where its at, see
>> ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
>> but not a setter. Its not clear if the firmware would make a split easy.
>
> The CX2341[56]'s firmware + hardware machine are notorious for bugs and
> being hard to work with. It would be difficult to determine with any
> certainty that the firmware returned predictable OSD framebuffer
> addresses from one user's system to the next.
>
>
>> We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
>
> Yes.
>
> As a driver writer/maintainer, IMO the name ioremap_ucminus() is jargon,
> without a clear meaning from the name, and maybe too x86 PAT specific?
> The pat.txt file under Documentation didn't explain what UC- meant; I
> had to go searching old LKML emails to understand it was a unchached
> region that could be overridden with write combine regions.
>
> To me names along, these lines would be better:
> ioremap_nocache_weak(), or
> ioremap_nocache_mutable(), or
> ioremap_nocache() (with ioremap_nocache_strong() or something for
> the UC version)
>
IMO the right solution would be to avoid ioremapping the whole bar at
startup. Instead ioremap pieces once the driver learns what they are.
This wouldn't have any of these problems -- you'd ioremap() register
regions and you'd ioremap_wc() the framebuffer once you find it. If
there are regions of unknown purpose, just don't map them all.
Would this be feasible?
--Andy
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Andy Walls @ 2015-04-15 22:50 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Luis R. Rodriguez, linux-rdma, Toshi Kani, H. Peter Anvin,
Ingo Molnar, linux-kernel@vger.kernel.org, Hal Rosenstock,
Sean Hefty, Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Juergen Gross, Mauro Carvalho Chehab,
Borislav Petkov, Mel Gorman, Vlastimil Babka, Davidlohr Bueso,
Dave Hansen, Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list,
linux-media, X86 ML
In-Reply-To: <CALCETrV0B7rp08-VYjp5=1CWJp7=xTUTBYo3uGxX317RxAQT+w@mail.gmail.com>
On Wed, 2015-04-15 at 13:42 -0700, Andy Lutomirski wrote:
> On Mon, Apr 13, 2015 at 10:49 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>
> > c) ivtv: the driver does not have the PCI space mapped out separately, and
> > in fact it actually does not do the math for the framebuffer, instead it lets
> > the device's own CPU do that and assume where its at, see
> > ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
> > but not a setter. Its not clear if the firmware would make a split easy.
> > We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
> >
>
> IMO this should be conceptually easy to split. Once we get the
> framebuffer address, just unmap it (or don't prematurely map it) and
> then ioremap the thing.
Not so easy. The main ivtv driver has already set up the PCI device and
done the mapping for the MPEG-2 decoder/video output engine. The video
decoder/output device nodes might already be open by user space calling
into the main driver, before the ivtvfb module is even loaded.
This could be mitigated by integrating all the ivtvfb module code into
the main ivtv module. But even then not every PVR-350 owner wants to
use the video output OSD as a framebuffer. Users might just want an
actual OSD overlaying their TV video playback.
Regards,
Andy
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Andy Walls @ 2015-04-15 22:38 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Andy Lutomirski, Toshi Kani, H. Peter Anvin, Ingo Molnar,
linux-kernel, Hal Rosenstock, Sean Hefty, Suresh Siddha,
Rickard Strandqvist, Mike Marciniszyn, Roland Dreier,
Juergen Gross, Mauro Carvalho Chehab, Borislav Petkov, Mel Gorman,
Vlastimil Babka, Davidlohr Bueso, dave.hansen, plagnioj, tglx,
Ville Syrjälä, linux-fbdev, linux-media, x86
In-Reply-To: <20150413174938.GE5622@wotan.suse.de>
Hi All,
On Mon, 2015-04-13 at 19:49 +0200, Luis R. Rodriguez wrote:
[snip]
> I only saw a few drivers using overlapping ioremap*()
> calls though on my MTRR review and they are all old devices so likely mostly
> used on non-PAT systems, but there might be other corner cases elsewhere.
>
> Lets recap, as I see it we have a few options with all this in mind on our
> quest to bury MTRR (and later make strong UC default):
>
> 1) Let drivers do their own get_vm_area() calls as you note and handle the
> cut and splicing of ioremap areas
>
> 2) Provide an API to split and splice ioremap ranges
>
> 3) Try to avoid these types of situations and let drivers simply try to
> work towards a proper clean non-overlapping different ioremap() ranges
>
> Let me know if you think of others but please keep in mind the UC- to strong
> UC converstion we want to do later as well. We have ruled out using
> set_memor_wc() now.
>
> I prefer option 3), its technically possible but only for *new* drivers
> and we'd need either some hard work to split the ioremap() areas or provide
> a a set of *transient* APIs as I had originally proposed to phase / hope for
> final transition. There are 3 drivers to address:
>
> [snip]
Just some background for folks:
The ivtv driver supports cards that perform Standard Definition PAL,
NTSC, and SECAM TV capture + hardware MPEG-2 encoding and MPEG-2
decoding + TV output.
Of the supported cards only *one* card type, the PVR-350 based on the
CX23415 chip, can perform the MPEG-2 or YUV video decoding and output,
with an OSD overlay. PVR-350's are legacy PCI cards that have been end
of life since 2088 or earlier. Despite that, there are still used units
available on Amazon and eBay.
The ivtvfb driver module is an *optional* companion driver module that
provides a framebuffer interface for the user to output an X display, FB
console, or whatever to a standard definition analog PAL, NTSC, or SECAM
TV or VCR. It does this by co-opting the OSD overlay of the video
decoding output engine and having it take up the whole screen.
> c) ivtv: the driver does not have the PCI space mapped out separately, and
> in fact it actually does not do the math for the framebuffer, instead it lets
> the device's own CPU do that and assume where its at, see
> ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
> but not a setter. Its not clear if the firmware would make a split easy.
The CX2341[56]'s firmware + hardware machine are notorious for bugs and
being hard to work with. It would be difficult to determine with any
certainty that the firmware returned predictable OSD framebuffer
addresses from one user's system to the next.
> We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
Yes.
As a driver writer/maintainer, IMO the name ioremap_ucminus() is jargon,
without a clear meaning from the name, and maybe too x86 PAT specific?
The pat.txt file under Documentation didn't explain what UC- meant; I
had to go searching old LKML emails to understand it was a unchached
region that could be overridden with write combine regions.
To me names along, these lines would be better:
ioremap_nocache_weak(), or
ioremap_nocache_mutable(), or
ioremap_nocache() (with ioremap_nocache_strong() or something for
the UC version)
> From the beginning it seems only framebuffer devices used MTRR/WC,
[snip]
> The ivtv device is a good example of the worst type of
> situations and these days. So perhap __arch_phys_wc_add() and a
> ioremap_ucminus() might be something more than transient unless hardware folks
> get a good memo or already know how to just Do The Right Thing (TM).
Just to reiterate a subtle point, use of the ivtvfb is *optional*. A
user may or may not load it. When the user does load the ivtvfb driver,
the ivtv driver has already been initialized and may have functions of
the card already in use by userspace.
Hopefully no one is trying to use the OSD as framebuffer and the video
decoder/output engine for video display at the same time. But the video
decoder/output device nodes may already be open for performing ioctl()
functions so unmapping the decoder IO space out from under them, when
loading the ivtvfb driver module, might not be a good thing.
Regards,
Andy
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Luis R. Rodriguez @ 2015-04-15 22:15 UTC (permalink / raw)
To: Andy Lutomirski, Mauro Carvalho Chehab, linux-media
Cc: linux-rdma, Toshi Kani, H. Peter Anvin, Ingo Molnar,
linux-kernel@vger.kernel.org, Hal Rosenstock, Sean Hefty,
Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Juergen Gross, Andy Walls, Borislav Petkov,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, Dave Hansen,
Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list, X86 ML
In-Reply-To: <CALCETrV0B7rp08-VYjp5=1CWJp7=xTUTBYo3uGxX317RxAQT+w@mail.gmail.com>
On Wed, Apr 15, 2015 at 01:42:47PM -0700, Andy Lutomirski wrote:
> On Mon, Apr 13, 2015 at 10:49 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>
> > c) ivtv: the driver does not have the PCI space mapped out separately, and
> > in fact it actually does not do the math for the framebuffer, instead it lets
> > the device's own CPU do that and assume where its at, see
> > ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
> > but not a setter. Its not clear if the firmware would make a split easy.
> > We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
> >
>
> IMO this should be conceptually easy to split. Once we get the
> framebuffer address, just unmap it (or don't prematurely map it) and
> then ioremap the thing.
The driver has split code for handling framebuffer devices, the framebuffer
base address will also vary depending on the type of device it has, for
some its on the encoder, for others its on the decoder. We'd have to account
for the removal of the framebuffer on either of those regions, and would also
need some vetting that the driver doesn't use areas beyond that for MMIO.
Using the trick you suggest though we could overlap ioremap calls and if that
truly works on PAT and non-PAT adding a new ioremap_wc() could do the trick,
I'd appreciate a Tested-by or Acked-by to be done with this. Mauro, any chance
we can get a tested-by of ivtvfb for both non-PAT and PAT systems with this:
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..1838738 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,10 +44,6 @@
#include <linux/ivtvfb.h>
#include <linux/slab.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
#include "ivtv-driver.h"
#include "ivtv-cards.h"
#include "ivtv-i2c.h"
@@ -155,12 +151,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+ int wc_cookie;
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1094,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
static int ivtvfb_init_io(struct ivtv *itv)
{
struct osd_info *oi = itv->osd_info;
+ /* Find the largest power of two that maps the whole buffer */
+ int size_shift = 31;
mutex_lock(&itv->serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1120,7 +1117,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_buffer_size = 1704960;
oi->video_pbase = itv->base_addr + IVTV_DECODER_OFFSET + oi->video_rbase;
- oi->video_vbase = itv->dec_mem + oi->video_rbase;
+ oi->video_vbase = ioremap_wc(oi->video_pbase, oi->video_buffer_size);
if (!oi->video_vbase) {
IVTVFB_ERR("abort, video memory 0x%x @ 0x%lx isn't mapped!\n",
@@ -1132,29 +1129,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_pbase, oi->video_vbase,
oi->video_buffer_size / 1024);
-#ifdef CONFIG_MTRR
- {
- /* Find the largest power of two that maps the whole buffer */
- int size_shift = 31;
-
- while (!(oi->video_buffer_size & (1 << size_shift))) {
- size_shift--;
- }
- size_shift++;
- oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
- oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
- oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
- oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
- if (mtrr_add(oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
- MTRR_TYPE_WRCOMB, 1) < 0) {
- IVTVFB_INFO("disabled mttr\n");
- oi->fb_start_aligned_physaddr = 0;
- oi->fb_end_aligned_physaddr = 0;
- }
- }
-#endif
-
+ while (!(oi->video_buffer_size & (1 << size_shift)))
+ size_shift--;
+ size_shift++;
+ oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
+ oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
+ oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
+ oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
+ oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
+ oi->fb_end_aligned_physaddr -
+ oi->fb_start_aligned_physaddr);
/* Blank the entire osd. */
memset_io(oi->video_vbase, 0, oi->video_buffer_size);
@@ -1172,14 +1156,8 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
/* Release pseudo palette */
kfree(oi->ivtvfb_info.pseudo_palette);
-
-#ifdef CONFIG_MTRR
- if (oi->fb_end_aligned_physaddr) {
- mtrr_del(-1, oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
- }
-#endif
-
+ iounmap(oi->video_vbase);
+ arch_phys_wc_del(oi->wc_cookie);
kfree(oi);
itv->osd_info = NULL;
}
>
> > From the beginning it seems only framebuffer devices used MTRR/WC, lately it
> > seems infiniband drivers also find good use for for it for PIO TX buffers to
> > blast some sort of data, in the future I would not be surprised if other
> > devices found use for it.
>
> IMO the Infiniband maintainers should fix their code. Especially in
> the server space, there aren't that many MTRRs to go around. I wrote
> arch_phys_wc_add in the first place because my server *ran out of
> MTRRs*.
>
> Hey, IB people: can you fix your drivers to use arch_phys_wc_add
> (which is permitted to be a no-op) along with ioremap_wc? Your users
> will thank you.
Provided the above ivtv driver changes are OK this would be the *last* and only
driver required to be changed.
> > It may be true that the existing drivers that
> > requires the above type of work are corner cases -- but I wouldn't hold my
> > breath for that. The ivtv device is a good example of the worst type of
> > situations and these days. So perhap __arch_phys_wc_add() and a
> > ioremap_ucminus() might be something more than transient unless hardware folks
> > get a good memo or already know how to just Do The Right Thing (TM).
>
> I disagree. We should try to NACK any new code that can't function
> without MTRRs.
>
> (Plus, ARM is growing in popularity in the server space, and ARM quite
> sensibly doesn't have MTRRs.)
Great, happy with this, but we need to address the last few drivers and their
exisitng code then.
Luis
^ permalink raw reply related
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: H. Peter Anvin @ 2015-04-15 20:56 UTC (permalink / raw)
To: Andy Lutomirski, Luis R. Rodriguez, linux-rdma
Cc: Toshi Kani, Ingo Molnar, linux-kernel@vger.kernel.org,
Hal Rosenstock, Sean Hefty, Suresh Siddha, Rickard Strandqvist,
Mike Marciniszyn, Roland Dreier, Juergen Gross,
Mauro Carvalho Chehab, Andy Walls, Borislav Petkov, Mel Gorman,
Vlastimil Babka, Davidlohr Bueso, Dave Hansen,
Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list,
linux-media, X86 ML
In-Reply-To: <CALCETrV0B7rp08-VYjp5=1CWJp7=xTUTBYo3uGxX317RxAQT+w@mail.gmail.com>
On 04/15/2015 01:42 PM, Andy Lutomirski wrote:
>
> I disagree. We should try to NACK any new code that can't function
> without MTRRs.
>
> (Plus, ARM is growing in popularity in the server space, and ARM quite
> sensibly doesn't have MTRRs.)
>
<NOT SPEAKING FOR INTEL HERE>
Yes. People need to understand that MTRRs are fundamentally a
transitional solution, a replacement for the KEN# logic in the P4 and P5
generation processors. The KEN# logic in the chipset would notify the
CPU that a specific address should not be cached, without affecting the
software (which may have been written for x86s built before caching
existed, even.)
MTRRs move this to the head end, so the CPU knows ahead of time what to
do, as is required with newer architectures. It also enabled write
combining in a transparent fashion. However, it is still transitional;
it is there to describe the underlying constraints of the memory system
so that code which doesn't use paging can run at all, but the only thing
that can actually scale is PAT.
-hpa
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Andy Lutomirski @ 2015-04-15 20:42 UTC (permalink / raw)
To: Luis R. Rodriguez, linux-rdma
Cc: Toshi Kani, H. Peter Anvin, Ingo Molnar,
linux-kernel@vger.kernel.org, Hal Rosenstock, Sean Hefty,
Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Juergen Gross, Mauro Carvalho Chehab, Andy Walls,
Borislav Petkov, Mel Gorman, Vlastimil Babka, Davidlohr Bueso,
Dave Hansen, Jean-Christophe Plagniol-Villard, Thomas Gleixner,
Ville Syrjälä, Linux Fbdev development list,
linux-media, X86 ML
In-Reply-To: <20150413174938.GE5622@wotan.suse.de>
On Mon, Apr 13, 2015 at 10:49 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> c) ivtv: the driver does not have the PCI space mapped out separately, and
> in fact it actually does not do the math for the framebuffer, instead it lets
> the device's own CPU do that and assume where its at, see
> ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
> but not a setter. Its not clear if the firmware would make a split easy.
> We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
>
IMO this should be conceptually easy to split. Once we get the
framebuffer address, just unmap it (or don't prematurely map it) and
then ioremap the thing.
> From the beginning it seems only framebuffer devices used MTRR/WC, lately it
> seems infiniband drivers also find good use for for it for PIO TX buffers to
> blast some sort of data, in the future I would not be surprised if other
> devices found use for it.
IMO the Infiniband maintainers should fix their code. Especially in
the server space, there aren't that many MTRRs to go around. I wrote
arch_phys_wc_add in the first place because my server *ran out of
MTRRs*.
Hey, IB people: can you fix your drivers to use arch_phys_wc_add
(which is permitted to be a no-op) along with ioremap_wc? Your users
will thank you.
> It may be true that the existing drivers that
> requires the above type of work are corner cases -- but I wouldn't hold my
> breath for that. The ivtv device is a good example of the worst type of
> situations and these days. So perhap __arch_phys_wc_add() and a
> ioremap_ucminus() might be something more than transient unless hardware folks
> get a good memo or already know how to just Do The Right Thing (TM).
I disagree. We should try to NACK any new code that can't function
without MTRRs.
(Plus, ARM is growing in popularity in the server space, and ARM quite
sensibly doesn't have MTRRs.)
--Andy
^ permalink raw reply
* Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR
From: Luis R. Rodriguez @ 2015-04-13 17:49 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Toshi Kani, H. Peter Anvin, Ingo Molnar, linux-kernel,
Hal Rosenstock, Sean Hefty, Suresh Siddha, Rickard Strandqvist,
Mike Marciniszyn, Roland Dreier, Juergen Gross,
Mauro Carvalho Chehab, Andy Walls, Borislav Petkov, Mel Gorman,
Vlastimil Babka, Davidlohr Bueso, dave.hansen, plagnioj, tglx,
Ville Syrjälä, linux-fbdev, linux-media, x86
In-Reply-To: <CALCETrXd19C6pARde3pv-4pt-i52APtw5xs20itwROPq9VmCfg@mail.gmail.com>
Cc'ing a few others as we ended up talking about the cruxes of my
unposted v2 series as I confirmed that set_memor_wc() would not work
as an alternative to my originally proposed __arch_phys_wc_add() to
force MTRR as a last resort on a few set of last remaining drivers.
This also discusses overlapping ioremap() calls and what we'd need
for a default shift from UC- to strong UC.
On Fri, Apr 10, 2015 at 11:25:22PM -0700, Andy Lutomirski wrote:
> On Apr 10, 2015 6:29 PM, "Luis R. Rodriguez" <mcgrof@suse.com> wrote:
> >
> > On Fri, Apr 10, 2015 at 02:22:51PM -0700, Andy Lutomirski wrote:
> > > On Fri, Apr 10, 2015 at 1:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > > > On Fri, 2015-04-10 at 23:05 +0200, Luis R. Rodriguez wrote:
> > > >> On Fri, Apr 10, 2015 at 01:49:39PM -0600, Toshi Kani wrote:
> > > >> > On Fri, 2015-04-10 at 12:34 -0700, Luis R. Rodriguez wrote:
> > > >> > > On Fri, Apr 10, 2015 at 12:14 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> > > >> > > > On Fri, Apr 10, 2015 at 10:17 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > > >> > > >>
> > > >> > > >> Andy, I'm ready to post a v2 series based on review of the first iteration of
> > > >> > > >> the bury-MTRR series however I ran into a snag in vetting for the ioremap_uc()
> > > >> > > >> followed by a set_memory_wc() strategy as a measure to both avoid when possible
> > > >> > > >> overlapping ioremap*() calls and/or to avoid the power of 2 MTRR size
> > > >> > > >> implications on having to use multiple MTRRs.
> > > >> > > >>
> > > >> > > >> I tested the strategy by just making my thinkpad's i915 driver use iremap_uc()
> > > >> > > >> which I add, and then use set_memory_wc(). To start off with I should note
> > > >> > > >> set_memory_*() helpers are x86 specific right now, this is not a problem for
> > > >> > > >> the series but its worth noting as we're replacing MTRR strategies which
> > > >> > > >> are x86 specific, but I am having issues getting set_memory_wc() take effect
> > > >> > > >> on my intel graphics card.
> > > >> > > >>
> > > >> > > >> I've reviewed if this is OK in code and I see no issues other than set_memory_*()
> > > >> > > >> helpers seem to be desirable for RAM, not IO memory, so was wondering if we need
> > > >> > > >> to add other helpers which can address IO memory or if this should work? The diff
> > > >> > > >> for the drivers is below, the actual commit for adding ioremap_uc() folllows
> > > >> > > >> with its commit log. Feedback / review on both is welcome as well as if you
> > > >> > > >> could help me figure out why this test-patch for i915 fails.
> > > >> > > >
> > > >> > > > I think they should work for IO memory, but I'm not really an authority here.
> > > >> > > >
> > > >> > > > Adding some people who have looked at the code recently.
> > > >> > >
> > > >> > > I was avoiding reviewing the cpa code but since this failed I just had
> > > >> > > to review it and I see nothing prevent it from being used on IO memory
> > > >> > > but -- memtype_rb_check_conflict() does prevent an overlap to be set
> > > >> > > on an *existing range* -- and since ioremap_uc() was used earlier the
> > > >> > > first reserve_memtype() with _PAGE_CACHE_MODE_WC by set_memory_wc() I
> > > >> > > believe should fail then. Please correct me if I'm wrong, I don't see
> > > >> > > the "conflicting memory types" print though, so not sure if it was
> > > >> > > because of that.
> > > >> > >
> > > >> > > I only started looking at this though but shouldn't this also mean we
> > > >> > > can't use overlapping ioremap() calls too? I thought that worked,
> > > >> > > because at least some drivers are using that strategy.
> > > >> >
> > > >> > set_memory_*() does not work with I/O memory ranges with the following
> > > >> > reasons:
> > > >> >
> > > >> > 1. __pa(addr) returns a fake address since there is no direct map.
> > > >> > 2. reserve_memtype() tracks regular memory and I/O memory differently.
> > > >> > For regular memory, set_memory_*() can modify WB with a new type since
> > > >> > reserve_memtype() does not track WB. For I/O memory, reserve_memtype()
> > > >> > detects a conflict when a given type is different from a tracked type.
> > > >>
> > > >> Interesting, but I also just realized I had messed up my test patch too,
> > > >> I checked for (!ret) instead of (ret). This works now.
> > > >>
> > > >> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > >> > > index dccdc8a..dd9501b 100644
> > > >> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > >> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > >> > > @@ -1958,12 +1958,22 @@ static int ggtt_probe_common(struct drm_device *dev,
> > > >> > > gtt_phys_addr = pci_resource_start(dev->pdev, 0) +
> > > >> > > (pci_resource_len(dev->pdev, 0) / 2);
> > > >> > >
> > > >> > > - dev_priv->gtt.gsm = ioremap_wc(gtt_phys_addr, gtt_size);
> > > >> > > + dev_priv->gtt.gsm = ioremap_uc(gtt_phys_addr, gtt_size);
> > > >> > > if (!dev_priv->gtt.gsm) {
> > > >> > > DRM_ERROR("Failed to map the gtt page table\n");
> > > >> > > return -ENOMEM;
> > > >> > > }
> > > >> > >
> > > >> > > + printk("mcgrof:set_memory_wc() ggtt_probe_common()\n");
> > > >> > > +
> > > >> > > + ret = set_memory_wc((unsigned long) dev_priv->gtt.gsm, gtt_size >> PAGE_SHIFT);
> > > >> > > + if (!ret) {
> > > >>
> > > >> Mess up here.
> > > >>
> > > >> > > + DRM_ERROR("mcgrof: failed set_memory_wc()\n");
> > > >> > > + iounmap(dev_priv->gtt.gsm);
> > > >> > > + return ret;
> > > >> > > + }
> > > >> > > +
> > > >> > > +
> > > >> > > ret = setup_scratch_page(dev);
> > > >> > > if (ret) {
> > > >> > > DRM_ERROR("Scratch setup failed\n");
> > > >>
> > > >> as I read the code though reserve_memtype() should not allow for a change
> > > >> of an existing type as you note though (and therefore prevent also
> > > >> overlapping ioremap() calls on PAT), but since this is going through now
> > > >> I am not sure at if this is by chance somehow... ?
> > > >
> > > > Unless you fixed issue #1 above, set_memory_wc() passes a bogus address
> > > > to reserve_memtype(). Hence, it won't cause any conflict.
> > >
> > > Presumably fixing #1 would be okay (slow_virt_to_phys or whatever)
> > > since this stuff is already slow.
> >
> > That wouldn't cut it I think. That would still fail as the types
> > don't match. That means we would have to free_memtype() and
> > then alloc a new reserve_memtype(), but I don't think that's
> > enought still. For instance a new ioremap() does:
> >
> > 1) sanity checks on regions of memory
> > 2) get_vm_area_caller() for the range
> > 3) kernel_map_sync_memtype()
> > 4) ioremap_page_range()
> > 5) mmiotrace_ioremap()
> >
> > Correct me if I'm wrong but I think we'd need to do all this as well.
> > If so we'd be cutting and splicing an ioremap() range. This code would
> > seem tricky to get right. Maybe its best to not support this then and
> > simply have to live with having drivers do their own splititn gof
> > ioremap() calls.
> >
> > This *might* implicate some constraints on burrying MTRR though so...
> > please let me know what you think.
>
> To throw out more ideas, what if the drivers instead did their own
> get_vm_area vmap calls and then mapped the two consecutive regions
> separately at consecutive addresses?
Instead of having an API do it for them? They'd have to implement quite a bit.
Driver writers also tend to not have the best of judgement and *typically* just
copy and paste code, so we'd have to either annotate it well as a work around
for a few drivers (in my series there are 3 that need this in order for us to
bury MTRR) and not share the code or provide an API for these cases.
Note that I belive these findings should also mean, if I understand things
correctly, that when PAT is used overlapping ioremap*() calls may work with
the caveat that the type of cache used will depend on the offset used, one
cannot hope that using either offset will yield the same caching effects.
There might even be other caveats with this but that will depend on the
hardware and not sure what those effects are. The reason is that for the
the cache effects requiring the appropriate offset is reserve_memtype() on IO
memory will use a fake address and this can in turn mean that even though the
same "pa" address was used we could end up with it not really checking
conflicts with the original reserve_memtype(). Some parts of this complexity
were described above. I only saw a few drivers using overlapping ioremap*()
calls though on my MTRR review and they are all old devices so likely mostly
used on non-PAT systems, but there might be other corner cases elsewhere.
Lets recap, as I see it we have a few options with all this in mind on our
quest to bury MTRR (and later make strong UC default):
1) Let drivers do their own get_vm_area() calls as you note and handle the
cut and splicing of ioremap areas
2) Provide an API to split and splice ioremap ranges
3) Try to avoid these types of situations and let drivers simply try to
work towards a proper clean non-overlapping different ioremap() ranges
Let me know if you think of others but please keep in mind the UC- to strong
UC converstion we want to do later as well. We have ruled out using
set_memor_wc() now.
I prefer option 3), its technically possible but only for *new* drivers
and we'd need either some hard work to split the ioremap() areas or provide
a a set of *transient* APIs as I had originally proposed to phase / hope for
final transition. There are 3 drivers to address:
a) atyfb: fortunately I believe I have finished the split for the atyfb
driver, ioremap_uc() would be used on only the MMIO region while ioremap_wc()
on the framebuffer (with newly corrected fixed size). This would go in as
an example of what work was required to do a split.
b) ipath: while a split on ipath is possible the changes are quite
significant. Apart from changing the driver to use different offset bases
in different regions the driver also has a debug userspace filemap for
the entire register map, the code there would need to be modified to
use the right virtual address base depending on the virtual address accessed.
The qib driver already has logic which could be mimic'd for this fortunatley,
but still - this is considerable work. One side hack I was hoping for
was that overlapping ioremap*() calls with different page attribute
types would work even if the 2nd returned __iomem address was not used,
based on my review and Toshi's feedback on reserve_memtype() this would
_not work_, only using the right virtual address would ensure the right
caching technique is used. In fact since this is not really all clear
I would not be surprised if there are other caveats. Sticking to the original
__arch_phys_wc_add() to force MTRR use here might be best but note that
even if this is done an eventual change from ioremap_nocache() to default
from UC- to strong UC would cause a regression on the desired WC area, in
this case on the PIO buffers. We may need an API to keep UC- for drivers that
know that need it -- or we may need to really do tha hard work to try to
convert this driver to split the iorenmap*() areas. If we do not want to do the
work to split this driver's ioremap*() space we could live with having
__arch_phys_wc_add() and another API to force UC- even if later the default is
strong UC.
c) ivtv: the driver does not have the PCI space mapped out separately, and
in fact it actually does not do the math for the framebuffer, instead it lets
the device's own CPU do that and assume where its at, see
ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
but not a setter. Its not clear if the firmware would make a split easy.
We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
From the beginning it seems only framebuffer devices used MTRR/WC, lately it
seems infiniband drivers also find good use for for it for PIO TX buffers to
blast some sort of data, in the future I would not be surprised if other
devices found use for it. It may be true that the existing drivers that
requires the above type of work are corner cases -- but I wouldn't hold my
breath for that. The ivtv device is a good example of the worst type of
situations and these days. So perhap __arch_phys_wc_add() and a
ioremap_ucminus() might be something more than transient unless hardware folks
get a good memo or already know how to just Do The Right Thing (TM).
Luis
^ permalink raw reply
* [PATCH] staging: sm750: remove incorrect __exit annotation
From: Arnd Bergmann @ 2015-04-10 22:03 UTC (permalink / raw)
To: linux-arm-kernel
The lynxfb_pci_remove function is used as the 'remove' callback
of the driver, and must not be discarded:
lynxfb_pci_remove' referenced in section `.data' of drivers/built-in.o: defined in discarded section `.exit.text' of drivers/built-in.o
This removes the extraneous annotation.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3c7ea95dd9f9..dbbb2f879a29 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -1250,7 +1250,7 @@ err_enable:
return -ENODEV;
}
-static void __exit lynxfb_pci_remove(struct pci_dev *pdev)
+static void lynxfb_pci_remove(struct pci_dev *pdev)
{
struct fb_info *info;
struct lynx_share *share;
^ permalink raw reply related
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Archit Taneja @ 2015-04-10 7:17 UTC (permalink / raw)
To: Tomi Valkeinen, Pavel Machek
Cc: Geert Uytterhoeven, Marek Vasut, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Grant Likely, Rob Herring,
Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org, shc_work, linux, hsweeten
In-Reply-To: <552663C2.70308@ti.com>
On 04/09/2015 05:04 PM, Tomi Valkeinen wrote:
> On 09/04/15 14:21, Tomi Valkeinen wrote:
>> On 09/04/15 14:06, Pavel Machek wrote:
>>> On Tue 2015-04-07 14:19:33, Geert Uytterhoeven wrote:
>>>> Hi Pavel,
>>>>
>>>> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
>>>>> I have an socfpga board, which uses has simple framebuffer implemented
>>>>> in the FPGA. On 3.15, framebuffer is fast:
>>>>>
>>>>> root@wagabuibui:~# time cat /dev/fb0 > /dev/null
>>>>> real 0m 0.00s
>>>>> user 0m 0.00s
>>>>> sys 0m 0.00s
>>>>>
>>>>> on 3.18, this takes 220msec. Similar slowdown exists for
>>>>> writes. Simple framebuffer did not change at all between 3.15 and
>>>>> 3.18; resource flags of the framebuffer are still same (0x200).
>>>>>
>>>>> If I enable caching on 3.18, it speeds up a bit, to 70msec or
>>>>> so... Which means problem is not only in caching.
>>>>>
>>>>> Any ideas?
>>>>
>>>> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
>>>> copying optimisations"), but this was introduced only in v4.0-rc1.
>>>>
>>>> Just in case you encounter another performance regression after upgrading
>>>> to a more modern kernel ;-)
>>>
>>> :-). I did a git bisect, and it pointed to this. And reverting it
>>> indeed fixes the problem in 3.18. Problem is still there in 4.0.
>
> The difference is probably caused by memcpy() vs memcpy_fromio(). The
> comment above memcpy_fromio() says "This needs to be optimized". I think
> generally speaking memcpy_fromio() is correct for a framebuffer.
>
> That said, if the fb is in RAM, and is only written by the CPU, I think
> a normal memcpy() for fb_memcpy_fromfb() should be fine...
I didn't test for performance regressions when I posted this patch.
A look at _memcpy_fromio in arch/arm/kernel/io.c shows that readb() is
used all the time, even when the source and destination addresses are
aligned for larger reads to be possible. Other archs seem to use readl()
or readq() when they can. Maybe that makes memcpy_fromio slower than the
implementation of memcpy on arm?
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Arnd Bergmann @ 2015-04-09 19:51 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Pavel Machek, Geert Uytterhoeven, Marek Vasut, kernel list,
Dinh Nguyen, Jean-Christophe PLAGNIOL-VILLARD, Grant Likely,
Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
shc_work-JGs/UdohzUI, linux-lFZ/pmaqli7XmaaqVzeoHQ,
hsweeten-3FF4nKcrg1dE2c76skzGb0EOCMrvLtNR, Archit Taneja
In-Reply-To: <552663C2.70308-l0cyMroinI0@public.gmane.org>
On Thursday 09 April 2015 14:34:26 Tomi Valkeinen wrote:
> On 09/04/15 14:21, Tomi Valkeinen wrote:
> > On 09/04/15 14:06, Pavel Machek wrote:
> >> On Tue 2015-04-07 14:19:33, Geert Uytterhoeven wrote:
> >>> Hi Pavel,
> >>>
> >>> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >>>> I have an socfpga board, which uses has simple framebuffer implemented
> >>>> in the FPGA. On 3.15, framebuffer is fast:
> >>>>
> >>>> root@wagabuibui:~# time cat /dev/fb0 > /dev/null
> >>>> real 0m 0.00s
> >>>> user 0m 0.00s
> >>>> sys 0m 0.00s
> >>>>
> >>>> on 3.18, this takes 220msec. Similar slowdown exists for
> >>>> writes. Simple framebuffer did not change at all between 3.15 and
> >>>> 3.18; resource flags of the framebuffer are still same (0x200).
> >>>>
> >>>> If I enable caching on 3.18, it speeds up a bit, to 70msec or
> >>>> so... Which means problem is not only in caching.
> >>>>
> >>>> Any ideas?
> >>>
> >>> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
> >>> copying optimisations"), but this was introduced only in v4.0-rc1.
> >>>
> >>> Just in case you encounter another performance regression after upgrading
> >>> to a more modern kernel
> >>
> >> :-). I did a git bisect, and it pointed to this. And reverting it
> >> indeed fixes the problem in 3.18. Problem is still there in 4.0.
>
> The difference is probably caused by memcpy() vs memcpy_fromio(). The
> comment above memcpy_fromio() says "This needs to be optimized". I think
> generally speaking memcpy_fromio() is correct for a framebuffer.
>
> That said, if the fb is in RAM, and is only written by the CPU, I think
> a normal memcpy() for fb_memcpy_fromfb() should be fine...
Could memcpy() cause alignment traps here if the fb pointer is unaligned
and uncached?
Arnd
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Tomi Valkeinen @ 2015-04-09 11:34 UTC (permalink / raw)
To: Pavel Machek
Cc: Geert Uytterhoeven, Marek Vasut, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Grant Likely, Rob Herring,
Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org, shc_work, linux, hsweeten,
Archit Taneja
In-Reply-To: <552660C7.4020805@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]
On 09/04/15 14:21, Tomi Valkeinen wrote:
> On 09/04/15 14:06, Pavel Machek wrote:
>> On Tue 2015-04-07 14:19:33, Geert Uytterhoeven wrote:
>>> Hi Pavel,
>>>
>>> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
>>>> I have an socfpga board, which uses has simple framebuffer implemented
>>>> in the FPGA. On 3.15, framebuffer is fast:
>>>>
>>>> root@wagabuibui:~# time cat /dev/fb0 > /dev/null
>>>> real 0m 0.00s
>>>> user 0m 0.00s
>>>> sys 0m 0.00s
>>>>
>>>> on 3.18, this takes 220msec. Similar slowdown exists for
>>>> writes. Simple framebuffer did not change at all between 3.15 and
>>>> 3.18; resource flags of the framebuffer are still same (0x200).
>>>>
>>>> If I enable caching on 3.18, it speeds up a bit, to 70msec or
>>>> so... Which means problem is not only in caching.
>>>>
>>>> Any ideas?
>>>
>>> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
>>> copying optimisations"), but this was introduced only in v4.0-rc1.
>>>
>>> Just in case you encounter another performance regression after upgrading
>>> to a more modern kernel ;-)
>>
>> :-). I did a git bisect, and it pointed to this. And reverting it
>> indeed fixes the problem in 3.18. Problem is still there in 4.0.
The difference is probably caused by memcpy() vs memcpy_fromio(). The
comment above memcpy_fromio() says "This needs to be optimized". I think
generally speaking memcpy_fromio() is correct for a framebuffer.
That said, if the fb is in RAM, and is only written by the CPU, I think
a normal memcpy() for fb_memcpy_fromfb() should be fine...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Tomi Valkeinen @ 2015-04-09 11:21 UTC (permalink / raw)
To: Pavel Machek
Cc: Geert Uytterhoeven, Marek Vasut, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Grant Likely, Rob Herring,
Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org, shc_work, linux, hsweeten,
Archit Taneja
In-Reply-To: <20150409110634.GA27407@amd>
[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]
On 09/04/15 14:06, Pavel Machek wrote:
> On Tue 2015-04-07 14:19:33, Geert Uytterhoeven wrote:
>> Hi Pavel,
>>
>> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
>>> I have an socfpga board, which uses has simple framebuffer implemented
>>> in the FPGA. On 3.15, framebuffer is fast:
>>>
>>> root@wagabuibui:~# time cat /dev/fb0 > /dev/null
>>> real 0m 0.00s
>>> user 0m 0.00s
>>> sys 0m 0.00s
>>>
>>> on 3.18, this takes 220msec. Similar slowdown exists for
>>> writes. Simple framebuffer did not change at all between 3.15 and
>>> 3.18; resource flags of the framebuffer are still same (0x200).
>>>
>>> If I enable caching on 3.18, it speeds up a bit, to 70msec or
>>> so... Which means problem is not only in caching.
>>>
>>> Any ideas?
>>
>> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
>> copying optimisations"), but this was introduced only in v4.0-rc1.
>>
>> Just in case you encounter another performance regression after upgrading
>> to a more modern kernel ;-)
>
> :-). I did a git bisect, and it pointed to this. And reverting it
> indeed fixes the problem in 3.18. Problem is still there in 4.0.
Interesting. I can reproduce this also on TI's AM437x board, on 3.14
kernel. Without the patch:
# time cat /dev/fb0 > /dev/null
real 0m 0.01s
user 0m 0.00s
sys 0m 0.01s
With the patch:
# time cat /dev/fb0 > /dev/null
real 0m 0.19s
user 0m 0.01s
sys 0m 0.17s
> Archit do you know what is going on there? Should the revert be filled
> for 4.0?
(Cc'ing Archit's new email)
>
> Pavel
>
> commit 981409b25e2a99409b26daa67293ca1cfd5ea0a0
> Author: Archit Taneja <archit@ti.com>
> Date: Fri Nov 16 14:46:04 2012 +0530
>
> fbdev: arm has __raw I/O accessors, use them in fb.h
>
> This removes the sparse warnings on arm platforms:
>
> warning: cast removes address space of expression
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: H Hartley Sweeten <hsweeten at visionengravers.com>
> Cc: Alexander Shiyan <shc_work@mail.ru>
> Cc: Russell King <linux@arm.linux.org.uk>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Pavel Machek @ 2015-04-09 11:06 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Marek Vasut, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Grant Likely,
Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org, archit, shc_work, linux, hsweeten
In-Reply-To: <CAMuHMdXWRx7bKWhQuWH05CbPTXPy5imCUcW1xUf2E8f7BtF9NQ@mail.gmail.com>
On Tue 2015-04-07 14:19:33, Geert Uytterhoeven wrote:
> Hi Pavel,
>
> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > I have an socfpga board, which uses has simple framebuffer implemented
> > in the FPGA. On 3.15, framebuffer is fast:
> >
> > root@wagabuibui:~# time cat /dev/fb0 > /dev/null
> > real 0m 0.00s
> > user 0m 0.00s
> > sys 0m 0.00s
> >
> > on 3.18, this takes 220msec. Similar slowdown exists for
> > writes. Simple framebuffer did not change at all between 3.15 and
> > 3.18; resource flags of the framebuffer are still same (0x200).
> >
> > If I enable caching on 3.18, it speeds up a bit, to 70msec or
> > so... Which means problem is not only in caching.
> >
> > Any ideas?
>
> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
> copying optimisations"), but this was introduced only in v4.0-rc1.
>
> Just in case you encounter another performance regression after upgrading
> to a more modern kernel ;-)
:-). I did a git bisect, and it pointed to this. And reverting it
indeed fixes the problem in 3.18. Problem is still there in 4.0.
Archit do you know what is going on there? Should the revert be filled
for 4.0?
Pavel
commit 981409b25e2a99409b26daa67293ca1cfd5ea0a0
Author: Archit Taneja <archit@ti.com>
Date: Fri Nov 16 14:46:04 2012 +0530
fbdev: arm has __raw I/O accessors, use them in fb.h
This removes the sparse warnings on arm platforms:
warning: cast removes address space of expression
Signed-off-by: Archit Taneja <archit@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: H Hartley Sweeten <hsweeten at visionengravers.com>
Cc: Alexander Shiyan <shc_work@mail.ru>
Cc: Russell King <linux@arm.linux.org.uk>
> 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
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* [PATCH] Staging: sm750fb: Remove space after parenthesis
From: Amitoj Kaur Chawla @ 2015-04-08 16:55 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
linux-kernel
Fixed error by removing space after open parenthesis '('
Problem found using checkpatch.pl
ERROR: space prohibited after that open parenthesis '('
Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
drivers/staging/sm750fb/ddk750_chip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 7b28328..3cb860c 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -285,7 +285,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
ulReg = FIELD_SET(ulReg, VGA_CONFIGURATION, MODE, GRAPHIC);
POKE32(VGA_CONFIGURATION, ulReg);
} else {
-#if defined(__i386__) || defined( __x86_64__)
+#if defined(__i386__) || defined(__x86_64__)
/* set graphic mode via IO method */
outb_p(0x88, 0x3d4);
outb_p(0x06, 0x3d5);
@@ -382,7 +382,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
unsigned int absDiff(unsigned int a, unsigned int b)
{
- if ( a > b )
+ if (a > b)
return(a - b);
else
return(b - a);
--
1.9.1
^ permalink raw reply related
* Re: Questions about simplefb and hand-over
From: David Herrmann @ 2015-04-08 8:32 UTC (permalink / raw)
To: Hans de Goede
Cc: Johannes Pointner,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree
In-Reply-To: <5524D92B.4040408-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi
On Wed, Apr 8, 2015 at 9:30 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 08-04-15 09:24, Johannes Pointner wrote:
>>
>> Hello,
>>
>> thank you for your reply.
>> I will take a look at the x86 code and at the function you mentioned.
>>
>> Do you know where the simpledrm is hosted, because I would like to
>> take look at it too.
>
>
> David Herrmann (in the Cc) is the best person to answer that, I
> think he needs to rebase on it on the latest upstream kernel.
>
> David ?
http://cgit.freedesktop.org/~dvdhrm/linux/log/?h=simpledrm
I should _definitely_ revive that. My TODO list does get shorter, so
lets see whether I can find some spare time for it.
Please note that you can use remove_conflicting_framebuffers() with
simplefb just fine. No reason to wait for simpledrm (except that it's
way cooler!). Once I push the patches, I will convert
remove_conflicting_framebuffers() to work just fine, and simpledrm
will take-over all simplefb devices seamlessly.
Thanks
David
^ permalink raw reply
* Re: Questions about simplefb and hand-over
From: Hans de Goede @ 2015-04-08 7:30 UTC (permalink / raw)
To: Johannes Pointner
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, devicetree, David Herrmann
In-Reply-To: <CAJKbBDD_DQS2DkwMa04ijdOCE4+g-bvw=TWcZDwYJgXfB2xH1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 08-04-15 09:24, Johannes Pointner wrote:
> Hello,
>
> thank you for your reply.
> I will take a look at the x86 code and at the function you mentioned.
>
> Do you know where the simpledrm is hosted, because I would like to
> take look at it too.
David Herrmann (in the Cc) is the best person to answer that, I
think he needs to rebase on it on the latest upstream kernel.
David ?
Regards,
Hans
^ permalink raw reply
* Re: Questions about simplefb and hand-over
From: Johannes Pointner @ 2015-04-08 7:24 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, devicetree, David Herrmann
In-Reply-To: <551E5715.90600-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hello,
thank you for your reply.
I will take a look at the x86 code and at the function you mentioned.
Do you know where the simpledrm is hosted, because I would like to
take look at it too.
Thanks,
Hannes
2015-04-03 11:02 GMT+02:00 Hans de Goede <hdegoede@redhat.com>:
> Hi,
>
>
> On 03-04-15 10:57, Johannes Pointner wrote:
>>
>> Hello all,
>>
>> I'd like to use the simplefb in combination with tilcdc drm driver
>> with a TI AM335x processor. I got the u-boot to simplefb part working,
>> but I'm not sure how to do the hand-over part.
>>
>> Is it yet possible to do that? Does the tilcdc drm driver need
>> additional functionality to support that? Or are there other things I
>> have to take into account?
>>
>> Any suggestions would be highly appreciated!
>
>
> There is a remove_conflicting_framebuffers call which is already used
> on x86 to remove e.g. efifb when a kms driver loads, the plan for
> simplefb / arm is to do the same. That is where we stand now, we've
> a plan but you're the first person to actually try this.
>
> Note that you may want to use simpledrm by David Herrmann (added to the
> Cc) instead of simplefb as that will give you a kms driver right from
> the start, simpledrm is not yet upstream but should go upstream soon.
>
> Regards,
>
> Hans
>
>
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Marek Vasut @ 2015-04-07 14:24 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Pavel Machek, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Grant Likely,
Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org
In-Reply-To: <CAMuHMdXWRx7bKWhQuWH05CbPTXPy5imCUcW1xUf2E8f7BtF9NQ@mail.gmail.com>
On Tuesday, April 07, 2015 at 02:19:33 PM, Geert Uytterhoeven wrote:
> Hi Pavel,
>
> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > I have an socfpga board, which uses has simple framebuffer implemented
> > in the FPGA. On 3.15, framebuffer is fast:
> >
> > root@wagabuibui:~# time cat /dev/fb0 > /dev/null
> > real 0m 0.00s
> > user 0m 0.00s
> > sys 0m 0.00s
> >
> > on 3.18, this takes 220msec. Similar slowdown exists for
> > writes. Simple framebuffer did not change at all between 3.15 and
> > 3.18; resource flags of the framebuffer are still same (0x200).
> >
> > If I enable caching on 3.18, it speeds up a bit, to 70msec or
> > so... Which means problem is not only in caching.
> >
> > Any ideas?
>
> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
> copying optimisations"), but this was introduced only in v4.0-rc1.
>
> Just in case you encounter another performance regression after upgrading
> to a more modern kernel ;-)
Why don't you use the Altera VIP FB on SoCFPGA ?
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [RFT PATCHv2] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Inki Dae @ 2015-04-07 13:32 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jingoo Han, Joonyoung Shim, Seung-Woo Kim, Kyungmin Park,
David Airlie, Kukjin Kim, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-fbdev, Marek Szyprowski,
Andrzej Hajda, Javier Martinez Canillas, stable
In-Reply-To: <1427472488-21454-1-git-send-email-k.kozlowski@samsung.com>
On 2015년 03월 28일 01:08, Krzysztof Kozlowski wrote:
> After adding display power domain for Exynos5250 in commit
> 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
> display on Chromebook Snow and others stopped working after boot.
>
> The reason for this suggested Andrzej Hajda: the DP clock was disabled.
> This clock is required by Display Port and is enabled by bootloader.
> However when FIMD driver probing was deferred, the display power domain
> was turned off. This effectively reset the value of DP clock enable
> register.
Applied.
Thanks,
Inki Dae
>
> When exynos-dp is later probed, the clock is not enabled and display is
> not properly configured:
>
> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
> exynos-dp 145b0000.dp-controller: unable to config video
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
> Cc: <stable@vger.kernel.org>
>
> ---
>
> This should fix issue reported by Javier [1][2].
>
> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
> especially on other Exynos 5xxx products.
>
> [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/43889
> [2] http://thread.gmane.org/gmane.linux.ports.arm.kernel/400290
>
> Changes since v1:
> 1. Added missing exynos_drm_fimd.h.
> ---
> drivers/gpu/drm/exynos/exynos_dp_core.c | 10 ++++++++++
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 19 +++++++++++++++++++
> drivers/gpu/drm/exynos/exynos_drm_fimd.h | 15 +++++++++++++++
> include/video/samsung_fimd.h | 6 ++++++
> 4 files changed, 50 insertions(+)
> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimd.h
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index bf17a60b40ed..1dbfba58f909 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -32,10 +32,16 @@
> #include <drm/bridge/ptn3460.h>
>
> #include "exynos_dp_core.h"
> +#include "exynos_drm_fimd.h"
>
> #define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \
> connector)
>
> +static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
> +{
> + return to_exynos_crtc(dp->encoder->crtc);
> +}
> +
> static inline struct exynos_dp_device *
> display_to_dp(struct exynos_drm_display *d)
> {
> @@ -1070,6 +1076,8 @@ static void exynos_dp_poweron(struct exynos_dp_device *dp)
> }
> }
>
> + fimd_dp_clock_enable(dp_to_crtc(dp), true);
> +
> clk_prepare_enable(dp->clock);
> exynos_dp_phy_init(dp);
> exynos_dp_init_dp(dp);
> @@ -1094,6 +1102,8 @@ static void exynos_dp_poweroff(struct exynos_dp_device *dp)
> exynos_dp_phy_exit(dp);
> clk_disable_unprepare(dp->clock);
>
> + fimd_dp_clock_enable(dp_to_crtc(dp), false);
> +
> if (dp->panel) {
> if (drm_panel_unprepare(dp->panel))
> DRM_ERROR("failed to turnoff the panel\n");
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index c300e22da8ac..bdf0818dc8f5 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -32,6 +32,7 @@
> #include "exynos_drm_fbdev.h"
> #include "exynos_drm_crtc.h"
> #include "exynos_drm_iommu.h"
> +#include "exynos_drm_fimd.h"
>
> /*
> * FIMD stands for Fully Interactive Mobile Display and
> @@ -1231,6 +1232,24 @@ static int fimd_remove(struct platform_device *pdev)
> return 0;
> }
>
> +void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
> +{
> + struct fimd_context *ctx = crtc->ctx;
> + u32 val;
> +
> + /*
> + * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
> + * clock. On these SoCs the bootloader may enable it but any
> + * power domain off/on will reset it to disable state.
> + */
> + if (ctx->driver_data != &exynos5_fimd_driver_data)
> + return;
> +
> + val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
> + writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
> +}
> +EXPORT_SYMBOL_GPL(fimd_dp_clock_enable);
> +
> struct platform_driver fimd_driver = {
> .probe = fimd_probe,
> .remove = fimd_remove,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.h b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> new file mode 100644
> index 000000000000..b4fcaa568456
> --- /dev/null
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#ifndef _EXYNOS_DRM_FIMD_H_
> +#define _EXYNOS_DRM_FIMD_H_
> +
> +extern void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable);
> +
> +#endif /* _EXYNOS_DRM_FIMD_H_ */
> diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
> index a20e4a3a8b15..847a0a2b399c 100644
> --- a/include/video/samsung_fimd.h
> +++ b/include/video/samsung_fimd.h
> @@ -436,6 +436,12 @@
> #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0)
> #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
>
> +/* Display port clock control */
> +#define DP_MIE_CLKCON 0x27c
> +#define DP_MIE_CLK_DISABLE 0x0
> +#define DP_MIE_CLK_DP_ENABLE 0x2
> +#define DP_MIE_CLK_MIE_ENABLE 0x3
> +
> /* Notes on per-window bpp settings
> *
> * Value Win0 Win1 Win2 Win3 Win 4
>
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Fix destruction of uninitialized mutex
From: Tomi Valkeinen @ 2015-04-07 13:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1428065428-25473-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]
On 03/04/15 15:50, Geert Uytterhoeven wrote:
> If sh_mobile_lcdc_probe() fails after the allocation of driver-private
> data, but before the initialization of all channels, a warning will be
> printed due to the destruction of an uninitialized mutex:
>
> WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:116 mutex_destroy+0x5c/0x7c()
> DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock))
> ...
> Backtrace:
> ...
> [<c00425b4>] (mutex_destroy) from [<c01d5858>] (sh_mobile_lcdc_remove+0x1bc/0x230)
> r4:df6a4800 r3:00000000
> [<c01d569c>] (sh_mobile_lcdc_remove) from [<c01d6620>] (sh_mobile_lcdc_probe+0xd54/0xe28)
>
> Move the initialization of the mutexes from sh_mobile_lcdc_channel_init()
> to immediately after the allocation of driver-private data to fix this.
>
> Note that the interrupt number is moved to a new variable "irq", so we
> can reuse the existing variable "i" for iterating over the channels.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
Thanks, queued for 4.1.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Geert Uytterhoeven @ 2015-04-07 12:19 UTC (permalink / raw)
To: Pavel Machek
Cc: Marek Vasut, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Grant Likely,
Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org
In-Reply-To: <20150407121247.GA29497@amd>
Hi Pavel,
On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> I have an socfpga board, which uses has simple framebuffer implemented
> in the FPGA. On 3.15, framebuffer is fast:
>
> root@wagabuibui:~# time cat /dev/fb0 > /dev/null
> real 0m 0.00s
> user 0m 0.00s
> sys 0m 0.00s
>
> on 3.18, this takes 220msec. Similar slowdown exists for
> writes. Simple framebuffer did not change at all between 3.15 and
> 3.18; resource flags of the framebuffer are still same (0x200).
>
> If I enable caching on 3.18, it speeds up a bit, to 70msec or
> so... Which means problem is not only in caching.
>
> Any ideas?
My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
copying optimisations"), but this was introduced only in v4.0-rc1.
Just in case you encounter another performance regression after upgrading
to a more modern kernel ;-)
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
* simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Pavel Machek @ 2015-04-07 12:12 UTC (permalink / raw)
To: Marek Vasut, kernel list, dinh.linux
Cc: plagnioj, tomi.valkeinen, grant.likely, robh+dt, jg1.han,
robdclark, geert, linux-fbdev, devicetree
Hi!
I have an socfpga board, which uses has simple framebuffer implemented
in the FPGA. On 3.15, framebuffer is fast:
root@wagabuibui:~# time cat /dev/fb0 > /dev/null
real 0m 0.00s
user 0m 0.00s
sys 0m 0.00s
on 3.18, this takes 220msec. Similar slowdown exists for
writes. Simple framebuffer did not change at all between 3.15 and
3.18; resource flags of the framebuffer are still same (0x200).
If I enable caching on 3.18, it speeds up a bit, to 70msec or
so... Which means problem is not only in caching.
Any ideas?
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox