Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Fri, Apr 10, 2020 at 12:57 AM Benjamin Herrenschmidt wrote: > > On Thu, 2020-04-09 at 11:41 +0200, Daniel Vetter wrote: > > Now if these boxes didn't ever have agp then I think we can get away > > with deleting this, since we've already deleted the legacy radeon > > driver. And that one used vmalloc for everything. The new kms one does > > use the dma-api if the gpu isn't connected through agp > > Definitely no AGP there. Ah in that case I think we can be sure that this code is dead. Acked-by: Daniel Vetter Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Thu, 2020-04-09 at 11:41 +0200, Daniel Vetter wrote: > Now if these boxes didn't ever have agp then I think we can get away > with deleting this, since we've already deleted the legacy radeon > driver. And that one used vmalloc for everything. The new kms one does > use the dma-api if the gpu isn't connected through agp Definitely no AGP there. Cheers Ben.
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Thu, Apr 9, 2020 at 4:19 PM Alex Deucher wrote: > > On Thu, Apr 9, 2020 at 5:41 AM Daniel Vetter wrote: > > > > On Thu, Apr 9, 2020 at 10:54 AM Benjamin Herrenschmidt > > wrote: > > > > > > On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote: > > > > On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote: > > > > > If this code was broken for non-coherent caches a crude powerpc hack > > > > > isn't going to help anyone else. Remove the hack as it is the last > > > > > user of __vmalloc passing a page protection flag other than > > > > > PAGE_KERNEL. > > > > > > > > Well Ben added this to make stuff work on ppc, ofc the home grown dma > > > > layer in drm from back then isn't going to work in other places. I guess > > > > should have at least an ack from him, in case anyone still cares about > > > > this on ppc. Adding Ben to cc. > > > > > > This was due to some drivers (radeon ?) trying to use vmalloc pages for > > > coherent DMA, which means on those 4xx powerpc's need to be non-cached. > > > > > > There were machines using that (440 based iirc), though I honestly > > > can't tell if anybody still uses any of it. > > > > agp subsystem still seems to happily do that (vmalloc memory for > > device access), never having been ported to dma apis (or well > > converted to iommu drivers, which they kinda are really). So I think > > this all still works exactly as back then, even with the kms radeon > > drivers. Question really is whether we have users left, and I have no > > clue about that either. > > > > Now if these boxes didn't ever have agp then I think we can get away > > with deleting this, since we've already deleted the legacy radeon > > driver. And that one used vmalloc for everything. The new kms one does > > use the dma-api if the gpu isn't connected through agp. > > All radeons have a built in remapping table to handle non-AGP systems. > On the earlier radeons it wasn't quite as performant as AGP, but it > was always more reliable because AGP is AGP. Maybe it's time to let > AGP go? I'd be very much in favour of that, if we can just use the integrated gart and drop agp fast writes wobbliness on the floor. I think the only other modern driver using agp would be nouveau at that point. -Daniel > > Alex > > > -Daniel > > > > > Cheers, > > > Ben. > > > > > > > -Daniel > > > > > > > > > > > > > > Signed-off-by: Christoph Hellwig > > > > > --- > > > > > drivers/gpu/drm/drm_scatter.c | 11 +-- > > > > > 1 file changed, 1 insertion(+), 10 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_scatter.c > > > > > b/drivers/gpu/drm/drm_scatter.c > > > > > index ca520028b2cb..f4e6184d1877 100644 > > > > > --- a/drivers/gpu/drm/drm_scatter.c > > > > > +++ b/drivers/gpu/drm/drm_scatter.c > > > > > @@ -43,15 +43,6 @@ > > > > > > > > > > #define DEBUG_SCATTER 0 > > > > > > > > > > -static inline void *drm_vmalloc_dma(unsigned long size) > > > > > -{ > > > > > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) > > > > > - return __vmalloc(size, GFP_KERNEL, > > > > > pgprot_noncached_wc(PAGE_KERNEL)); > > > > > -#else > > > > > - return vmalloc_32(size); > > > > > -#endif > > > > > -} > > > > > - > > > > > static void drm_sg_cleanup(struct drm_sg_mem * entry) > > > > > { > > > > > struct page *page; > > > > > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, > > > > > void *data, > > > > > return -ENOMEM; > > > > > } > > > > > > > > > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); > > > > > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); > > > > > if (!entry->virtual) { > > > > > kfree(entry->busaddr); > > > > > kfree(entry->pagelist); > > > > > -- > > > > > 2.25.1 > > > > > > > > > > > > > > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Thu, Apr 9, 2020 at 5:41 AM Daniel Vetter wrote: > > On Thu, Apr 9, 2020 at 10:54 AM Benjamin Herrenschmidt > wrote: > > > > On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote: > > > On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote: > > > > If this code was broken for non-coherent caches a crude powerpc hack > > > > isn't going to help anyone else. Remove the hack as it is the last > > > > user of __vmalloc passing a page protection flag other than PAGE_KERNEL. > > > > > > Well Ben added this to make stuff work on ppc, ofc the home grown dma > > > layer in drm from back then isn't going to work in other places. I guess > > > should have at least an ack from him, in case anyone still cares about > > > this on ppc. Adding Ben to cc. > > > > This was due to some drivers (radeon ?) trying to use vmalloc pages for > > coherent DMA, which means on those 4xx powerpc's need to be non-cached. > > > > There were machines using that (440 based iirc), though I honestly > > can't tell if anybody still uses any of it. > > agp subsystem still seems to happily do that (vmalloc memory for > device access), never having been ported to dma apis (or well > converted to iommu drivers, which they kinda are really). So I think > this all still works exactly as back then, even with the kms radeon > drivers. Question really is whether we have users left, and I have no > clue about that either. > > Now if these boxes didn't ever have agp then I think we can get away > with deleting this, since we've already deleted the legacy radeon > driver. And that one used vmalloc for everything. The new kms one does > use the dma-api if the gpu isn't connected through agp. All radeons have a built in remapping table to handle non-AGP systems. On the earlier radeons it wasn't quite as performant as AGP, but it was always more reliable because AGP is AGP. Maybe it's time to let AGP go? Alex > -Daniel > > > Cheers, > > Ben. > > > > > -Daniel > > > > > > > > > > > Signed-off-by: Christoph Hellwig > > > > --- > > > > drivers/gpu/drm/drm_scatter.c | 11 +-- > > > > 1 file changed, 1 insertion(+), 10 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_scatter.c > > > > b/drivers/gpu/drm/drm_scatter.c > > > > index ca520028b2cb..f4e6184d1877 100644 > > > > --- a/drivers/gpu/drm/drm_scatter.c > > > > +++ b/drivers/gpu/drm/drm_scatter.c > > > > @@ -43,15 +43,6 @@ > > > > > > > > #define DEBUG_SCATTER 0 > > > > > > > > -static inline void *drm_vmalloc_dma(unsigned long size) > > > > -{ > > > > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) > > > > - return __vmalloc(size, GFP_KERNEL, > > > > pgprot_noncached_wc(PAGE_KERNEL)); > > > > -#else > > > > - return vmalloc_32(size); > > > > -#endif > > > > -} > > > > - > > > > static void drm_sg_cleanup(struct drm_sg_mem * entry) > > > > { > > > > struct page *page; > > > > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, > > > > void *data, > > > > return -ENOMEM; > > > > } > > > > > > > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); > > > > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); > > > > if (!entry->virtual) { > > > > kfree(entry->busaddr); > > > > kfree(entry->pagelist); > > > > -- > > > > 2.25.1 > > > > > > > > > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
Am 09.04.20 um 10:54 schrieb Benjamin Herrenschmidt: > On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote: >> On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote: >>> If this code was broken for non-coherent caches a crude powerpc hack >>> isn't going to help anyone else. Remove the hack as it is the last >>> user of __vmalloc passing a page protection flag other than PAGE_KERNEL. >> >> Well Ben added this to make stuff work on ppc, ofc the home grown dma >> layer in drm from back then isn't going to work in other places. I guess >> should have at least an ack from him, in case anyone still cares about >> this on ppc. Adding Ben to cc. > > This was due to some drivers (radeon ?) trying to use vmalloc pages for > coherent DMA, which means on those 4xx powerpc's need to be non-cached. > > There were machines using that (440 based iirc), though I honestly > can't tell if anybody still uses any of it. The first-gen amigaone platform (6xx/book32s) uses the radeon driver together with non-coherent DMA. However this only ever worked reliably for DRI1. br, Gerhard > Cheers, > Ben. > >> -Daniel >> >>> >>> Signed-off-by: Christoph Hellwig >>> --- >>> drivers/gpu/drm/drm_scatter.c | 11 +-- >>> 1 file changed, 1 insertion(+), 10 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c >>> index ca520028b2cb..f4e6184d1877 100644 >>> --- a/drivers/gpu/drm/drm_scatter.c >>> +++ b/drivers/gpu/drm/drm_scatter.c >>> @@ -43,15 +43,6 @@ >>> >>> #define DEBUG_SCATTER 0 >>> >>> -static inline void *drm_vmalloc_dma(unsigned long size) >>> -{ >>> -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) >>> - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL)); >>> -#else >>> - return vmalloc_32(size); >>> -#endif >>> -} >>> - >>> static void drm_sg_cleanup(struct drm_sg_mem * entry) >>> { >>> struct page *page; >>> @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void >>> *data, >>> return -ENOMEM; >>> } >>> >>> - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); >>> + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); >>> if (!entry->virtual) { >>> kfree(entry->busaddr); >>> kfree(entry->pagelist); >>> -- >>> 2.25.1 >>> >> >> >
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Thu, Apr 9, 2020 at 10:54 AM Benjamin Herrenschmidt wrote: > > On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote: > > On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote: > > > If this code was broken for non-coherent caches a crude powerpc hack > > > isn't going to help anyone else. Remove the hack as it is the last > > > user of __vmalloc passing a page protection flag other than PAGE_KERNEL. > > > > Well Ben added this to make stuff work on ppc, ofc the home grown dma > > layer in drm from back then isn't going to work in other places. I guess > > should have at least an ack from him, in case anyone still cares about > > this on ppc. Adding Ben to cc. > > This was due to some drivers (radeon ?) trying to use vmalloc pages for > coherent DMA, which means on those 4xx powerpc's need to be non-cached. > > There were machines using that (440 based iirc), though I honestly > can't tell if anybody still uses any of it. agp subsystem still seems to happily do that (vmalloc memory for device access), never having been ported to dma apis (or well converted to iommu drivers, which they kinda are really). So I think this all still works exactly as back then, even with the kms radeon drivers. Question really is whether we have users left, and I have no clue about that either. Now if these boxes didn't ever have agp then I think we can get away with deleting this, since we've already deleted the legacy radeon driver. And that one used vmalloc for everything. The new kms one does use the dma-api if the gpu isn't connected through agp. -Daniel > Cheers, > Ben. > > > -Daniel > > > > > > > > Signed-off-by: Christoph Hellwig > > > --- > > > drivers/gpu/drm/drm_scatter.c | 11 +-- > > > 1 file changed, 1 insertion(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c > > > index ca520028b2cb..f4e6184d1877 100644 > > > --- a/drivers/gpu/drm/drm_scatter.c > > > +++ b/drivers/gpu/drm/drm_scatter.c > > > @@ -43,15 +43,6 @@ > > > > > > #define DEBUG_SCATTER 0 > > > > > > -static inline void *drm_vmalloc_dma(unsigned long size) > > > -{ > > > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) > > > - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL)); > > > -#else > > > - return vmalloc_32(size); > > > -#endif > > > -} > > > - > > > static void drm_sg_cleanup(struct drm_sg_mem * entry) > > > { > > > struct page *page; > > > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void > > > *data, > > > return -ENOMEM; > > > } > > > > > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); > > > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); > > > if (!entry->virtual) { > > > kfree(entry->busaddr); > > > kfree(entry->pagelist); > > > -- > > > 2.25.1 > > > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote: > On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote: > > If this code was broken for non-coherent caches a crude powerpc hack > > isn't going to help anyone else. Remove the hack as it is the last > > user of __vmalloc passing a page protection flag other than PAGE_KERNEL. > > Well Ben added this to make stuff work on ppc, ofc the home grown dma > layer in drm from back then isn't going to work in other places. I guess > should have at least an ack from him, in case anyone still cares about > this on ppc. Adding Ben to cc. This was due to some drivers (radeon ?) trying to use vmalloc pages for coherent DMA, which means on those 4xx powerpc's need to be non-cached. There were machines using that (440 based iirc), though I honestly can't tell if anybody still uses any of it. Cheers, Ben. > -Daniel > > > > > Signed-off-by: Christoph Hellwig > > --- > > drivers/gpu/drm/drm_scatter.c | 11 +-- > > 1 file changed, 1 insertion(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c > > index ca520028b2cb..f4e6184d1877 100644 > > --- a/drivers/gpu/drm/drm_scatter.c > > +++ b/drivers/gpu/drm/drm_scatter.c > > @@ -43,15 +43,6 @@ > > > > #define DEBUG_SCATTER 0 > > > > -static inline void *drm_vmalloc_dma(unsigned long size) > > -{ > > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) > > - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL)); > > -#else > > - return vmalloc_32(size); > > -#endif > > -} > > - > > static void drm_sg_cleanup(struct drm_sg_mem * entry) > > { > > struct page *page; > > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void > > *data, > > return -ENOMEM; > > } > > > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); > > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); > > if (!entry->virtual) { > > kfree(entry->busaddr); > > kfree(entry->pagelist); > > -- > > 2.25.1 > > > >
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote: > If this code was broken for non-coherent caches a crude powerpc hack > isn't going to help anyone else. Remove the hack as it is the last > user of __vmalloc passing a page protection flag other than PAGE_KERNEL. Well Ben added this to make stuff work on ppc, ofc the home grown dma layer in drm from back then isn't going to work in other places. I guess should have at least an ack from him, in case anyone still cares about this on ppc. Adding Ben to cc. -Daniel > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/drm_scatter.c | 11 +-- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c > index ca520028b2cb..f4e6184d1877 100644 > --- a/drivers/gpu/drm/drm_scatter.c > +++ b/drivers/gpu/drm/drm_scatter.c > @@ -43,15 +43,6 @@ > > #define DEBUG_SCATTER 0 > > -static inline void *drm_vmalloc_dma(unsigned long size) > -{ > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) > - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL)); > -#else > - return vmalloc_32(size); > -#endif > -} > - > static void drm_sg_cleanup(struct drm_sg_mem * entry) > { > struct page *page; > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void > *data, > return -ENOMEM; > } > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); > if (!entry->virtual) { > kfree(entry->busaddr); > kfree(entry->pagelist); > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
If this code was broken for non-coherent caches a crude powerpc hack isn't going to help anyone else. Remove the hack as it is the last user of __vmalloc passing a page protection flag other than PAGE_KERNEL. Signed-off-by: Christoph Hellwig --- drivers/gpu/drm/drm_scatter.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c index ca520028b2cb..f4e6184d1877 100644 --- a/drivers/gpu/drm/drm_scatter.c +++ b/drivers/gpu/drm/drm_scatter.c @@ -43,15 +43,6 @@ #define DEBUG_SCATTER 0 -static inline void *drm_vmalloc_dma(unsigned long size) -{ -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE) - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL)); -#else - return vmalloc_32(size); -#endif -} - static void drm_sg_cleanup(struct drm_sg_mem * entry) { struct page *page; @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void *data, return -ENOMEM; } - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT); + entry->virtual = vmalloc_32(pages << PAGE_SHIFT); if (!entry->virtual) { kfree(entry->busaddr); kfree(entry->pagelist); -- 2.25.1