| Message ID | 1435196060-27350-2-git-send-email-mcgrof@do-not-panic.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Jun 24, 2015 at 06:34:18PM -0700, Luis R. Rodriguez wrote: > 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: Toshi Kani <toshi.kani@hp.com> > Cc: Suresh Siddha <sbsiddha@gmail.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: Ville Syrjälä <syrjala@sci.fi> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Mathias Krause <minipli@googlemail.com> > Cc: Andrzej Hajda <a.hajda@samsung.com> > Cc: Mel Gorman <mgorman@suse.de> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Borislav Petkov <bp@suse.de> > Cc: Davidlohr Bueso <dbueso@suse.de> > Cc: 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); The framebuffer size isn't always 8MB. That's the size of the BAR. So this change isn't really correct. I suppose it doesn't hurt too much since smem_len gets overwritten later in aty_init(). > if (info->screen_base == NULL) { > ret = -ENOMEM; > goto atyfb_setup_generic_fail; > -- > 2.3.2.209.gd67f9d5.dirty >
On Thu, Jun 25, 2015 at 4:04 PM, Ville Syrjälä <syrjala@sci.fi> wrote: > it doesn't hurt too much > since smem_len gets overwritten later in aty_init(). That's the idea, we set it with a default as it will be overwritten later anyway. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 25, 2015 at 04:06:45PM -0700, Luis R. Rodriguez wrote: > On Thu, Jun 25, 2015 at 4:04 PM, Ville Syrjälä <syrjala@sci.fi> wrote: > > it doesn't hurt too much > > since smem_len gets overwritten later in aty_init(). > > That's the idea, we set it with a default as it will be overwritten > later anyway. Maybe toss in a comment? Otherwise it's a bit dishonest and might give someone the impression that all PCI cards really have 8MB of memory.
On Fri, Jun 26, 2015 at 02:11:03AM +0300, Ville Syrjälä wrote: > On Thu, Jun 25, 2015 at 04:06:45PM -0700, Luis R. Rodriguez wrote: > > On Thu, Jun 25, 2015 at 4:04 PM, Ville Syrjälä <syrjala@sci.fi> wrote: > > > it doesn't hurt too much > > > since smem_len gets overwritten later in aty_init(). > > > > That's the idea, we set it with a default as it will be overwritten > > later anyway. > > Maybe toss in a comment? Otherwise it's a bit dishonest and might give > someone the impression that all PCI cards really have 8MB of memory. Sure, mind this as a follow up patch if its too late? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 26, 2015 at 03:09:27AM +0200, Luis R. Rodriguez wrote:
> Sure, mind this as a follow up patch if its too late?
No need, you can send me an updated one - I'll replace it.
Thanks.
On Fri, Jun 26, 2015 at 12:30 AM, Borislav Petkov <bp@suse.de> wrote: > On Fri, Jun 26, 2015 at 03:09:27AM +0200, Luis R. Rodriguez wrote: >> Sure, mind this as a follow up patch if its too late? > > No need, you can send me an updated one - I'll replace it. Will do! Luis -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 2, 2015 at 4:23 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote: > On Fri, Jun 26, 2015 at 12:30 AM, Borislav Petkov <bp@suse.de> wrote: >> On Fri, Jun 26, 2015 at 03:09:27AM +0200, Luis R. Rodriguez wrote: >>> Sure, mind this as a follow up patch if its too late? >> >> No need, you can send me an updated one - I'll replace it. > > Will do! OK the commend I'm adding: @@ -3489,6 +3489,15 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info, /* Map in frame buffer */ info->fix.smem_start = addr; + + /* + * The framebuffer is not always 8 MiB that's just the size of the + * PCI BAR, this is later corrected for use with write-combining + * helpers with aty_fudge_framebuffer_len() which will adjust the + * framebuffer accordingly depending on the device. We do this + * to match semantics over ioremap calls on framebuffer devices + * with with other drivers with the info->fix.smem_len. + */ info->fix.smem_len = 0x800000; info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len); Will respin. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 07, 2015 at 05:24:57PM -0700, Luis R. Rodriguez wrote: > On Thu, Jul 2, 2015 at 4:23 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote: > > On Fri, Jun 26, 2015 at 12:30 AM, Borislav Petkov <bp@suse.de> wrote: > >> On Fri, Jun 26, 2015 at 03:09:27AM +0200, Luis R. Rodriguez wrote: > >>> Sure, mind this as a follow up patch if its too late? > >> > >> No need, you can send me an updated one - I'll replace it. > > > > Will do! > > OK the commend I'm adding: > > @@ -3489,6 +3489,15 @@ static int atyfb_setup_generic(struct pci_dev > *pdev, struct fb_info *info, > > /* Map in frame buffer */ > info->fix.smem_start = addr; > + > + /* > + * The framebuffer is not always 8 MiB that's just the size of the > + * PCI BAR, this is later corrected for use with write-combining > + * helpers with aty_fudge_framebuffer_len() which will adjust the > + * framebuffer accordingly depending on the device. That somehow gives me the impression that aty_fudge_framebuffer_len() changes smem_len to match the framebuffer size, which it does not. Dunno, maybe something like this? /* * The framebuffer is not always 8 MiB that's just the size of the * PCI BAR. We temporarily abuse smem_len here to store the size * of the BAR. aty_init() will later correct it to match the actual * framebuffer size. * * On devices that don't have the auxiliary register aperture, the * registers are housed at the top end of the framebuffer PCI BAR. * aty_fudge_framebuffer_len() is used to reduce smem_len to not * overlap with the registers. */ > We do this > + * to match semantics over ioremap calls on framebuffer devices > + * with with other drivers with the info->fix.smem_len. > + */ > info->fix.smem_len = 0x800000; > > info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len); > > Will respin. > > Luis
On Wed, Jul 08, 2015 at 11:38:49AM +0300, Ville Syrjälä wrote: > On Tue, Jul 07, 2015 at 05:24:57PM -0700, Luis R. Rodriguez wrote: > > On Thu, Jul 2, 2015 at 4:23 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote: > > > On Fri, Jun 26, 2015 at 12:30 AM, Borislav Petkov <bp@suse.de> wrote: > > >> On Fri, Jun 26, 2015 at 03:09:27AM +0200, Luis R. Rodriguez wrote: > > >>> Sure, mind this as a follow up patch if its too late? > > >> > > >> No need, you can send me an updated one - I'll replace it. > > > > > > Will do! > > > > OK the commend I'm adding: > > > > @@ -3489,6 +3489,15 @@ static int atyfb_setup_generic(struct pci_dev > > *pdev, struct fb_info *info, > > > > /* Map in frame buffer */ > > info->fix.smem_start = addr; > > + > > + /* > > + * The framebuffer is not always 8 MiB that's just the size of the > > + * PCI BAR, this is later corrected for use with write-combining > > + * helpers with aty_fudge_framebuffer_len() which will adjust the > > + * framebuffer accordingly depending on the device. > > That somehow gives me the impression that aty_fudge_framebuffer_len() > changes smem_len to match the framebuffer size, which it does > not. > > Dunno, maybe something like this? > /* > * The framebuffer is not always 8 MiB that's just the size of the > * PCI BAR. We temporarily abuse smem_len here to store the size > * of the BAR. aty_init() will later correct it to match the actual > * framebuffer size. > * > * On devices that don't have the auxiliary register aperture, the > * registers are housed at the top end of the framebuffer PCI BAR. > * aty_fudge_framebuffer_len() is used to reduce smem_len to not > * overlap with the registers. > */ Thanks Ville, I used that. Will send out a v6 series. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;