diff mbox

[09/20] drm: merge drm_memory.h into drm_memory.c

Message ID 1409307166-12396-10-git-send-email-dh.herrmann@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Herrmann Aug. 29, 2014, 10:12 a.m. UTC 
The drm_memory.h header is only used to define PAGE_AGP, which is only
used in drm_memory.c. Fold the header into drm_memory.c and drop it.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_memory.c | 11 +++++++++
 include/drm/drmP.h           |  6 ++---
 include/drm/drm_memory.h     | 59 --------------------------------------------
 3 files changed, 13 insertions(+), 63 deletions(-)
 delete mode 100644 include/drm/drm_memory.h

Comments

Thierry Reding Aug. 29, 2014, 11:56 a.m. UTC | #1 
On Fri, Aug 29, 2014 at 12:12:35PM +0200, David Herrmann wrote:
> The drm_memory.h header is only used to define PAGE_AGP, which is only
> used in drm_memory.c. Fold the header into drm_memory.c and drop it.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  drivers/gpu/drm/drm_memory.c | 11 +++++++++
>  include/drm/drmP.h           |  6 ++---
>  include/drm/drm_memory.h     | 59 --------------------------------------------
>  3 files changed, 13 insertions(+), 63 deletions(-)
>  delete mode 100644 include/drm/drm_memory.h
> 
> diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> index 7888dad..62fda6a 100644
> --- a/drivers/gpu/drm/drm_memory.c
> +++ b/drivers/gpu/drm/drm_memory.c
> @@ -39,6 +39,17 @@
>  #include "drm_legacy.h"
>  
>  #if __OS_HAS_AGP
> +
> +#ifdef HAVE_PAGE_AGP
> +# include <asm/agp.h>
> +#else

This check seems to be redundant. All architectures that support AGP
provide this header.

> +# ifdef __powerpc__
> +#  define PAGE_AGP	__pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)

Is this even necessary? It seems like PowerPC always defines
HAVE_PAGE_AGP.

> +# else
> +#  define PAGE_AGP	PAGE_KERNEL
> +# endif
> +#endif

Shouldn't this simply be moved into the asm/agp.h header for each of the
architectures that has AGP?

Thierry
Daniel Vetter Aug. 29, 2014, 12:43 p.m. UTC | #2 
On Fri, Aug 29, 2014 at 01:56:23PM +0200, Thierry Reding wrote:
> On Fri, Aug 29, 2014 at 12:12:35PM +0200, David Herrmann wrote:
> > The drm_memory.h header is only used to define PAGE_AGP, which is only
> > used in drm_memory.c. Fold the header into drm_memory.c and drop it.
> > 
> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_memory.c | 11 +++++++++
> >  include/drm/drmP.h           |  6 ++---
> >  include/drm/drm_memory.h     | 59 --------------------------------------------
> >  3 files changed, 13 insertions(+), 63 deletions(-)
> >  delete mode 100644 include/drm/drm_memory.h
> > 
> > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> > index 7888dad..62fda6a 100644
> > --- a/drivers/gpu/drm/drm_memory.c
> > +++ b/drivers/gpu/drm/drm_memory.c
> > @@ -39,6 +39,17 @@
> >  #include "drm_legacy.h"
> >  
> >  #if __OS_HAS_AGP
> > +
> > +#ifdef HAVE_PAGE_AGP
> > +# include <asm/agp.h>
> > +#else
> 
> This check seems to be redundant. All architectures that support AGP
> provide this header.
> 
> > +# ifdef __powerpc__
> > +#  define PAGE_AGP	__pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
> 
> Is this even necessary? It seems like PowerPC always defines
> HAVE_PAGE_AGP.

Iirc I've tried to untangle this before and I'm not 100% this is actually
the case on all ppc platforms. It looked like there's some crazy include
header depency stuff going on.

But given how popular drm on ppc and that the few platforms where people
actually use gpus are the saner ones (hopefully) I think we can just move
ahead with this change and fixup any compile breakage once it's reported.
If there is any.

> > +# else
> > +#  define PAGE_AGP	PAGE_KERNEL
> > +# endif
> > +#endif
> 
> Shouldn't this simply be moved into the asm/agp.h header for each of the
> architectures that has AGP?

Same comment really, I think the include mess is to hard to untangle. At
least I've failed. So with the #ifdef HAVE_PAGE_AGP dropped around the
include as Thierry suggested this is

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel
Thierry Reding Aug. 29, 2014, 1:26 p.m. UTC | #3 
On Fri, Aug 29, 2014 at 02:43:10PM +0200, Daniel Vetter wrote:
> On Fri, Aug 29, 2014 at 01:56:23PM +0200, Thierry Reding wrote:
> > On Fri, Aug 29, 2014 at 12:12:35PM +0200, David Herrmann wrote:
> > > The drm_memory.h header is only used to define PAGE_AGP, which is only
> > > used in drm_memory.c. Fold the header into drm_memory.c and drop it.
> > > 
> > > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> > > ---
> > >  drivers/gpu/drm/drm_memory.c | 11 +++++++++
> > >  include/drm/drmP.h           |  6 ++---
> > >  include/drm/drm_memory.h     | 59 --------------------------------------------
> > >  3 files changed, 13 insertions(+), 63 deletions(-)
> > >  delete mode 100644 include/drm/drm_memory.h
> > > 
> > > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> > > index 7888dad..62fda6a 100644
> > > --- a/drivers/gpu/drm/drm_memory.c
> > > +++ b/drivers/gpu/drm/drm_memory.c
> > > @@ -39,6 +39,17 @@
> > >  #include "drm_legacy.h"
> > >  
> > >  #if __OS_HAS_AGP
> > > +
> > > +#ifdef HAVE_PAGE_AGP
> > > +# include <asm/agp.h>
> > > +#else
> > 
> > This check seems to be redundant. All architectures that support AGP
> > provide this header.
> > 
> > > +# ifdef __powerpc__
> > > +#  define PAGE_AGP	__pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
> > 
> > Is this even necessary? It seems like PowerPC always defines
> > HAVE_PAGE_AGP.
> 
> Iirc I've tried to untangle this before and I'm not 100% this is actually
> the case on all ppc platforms. It looked like there's some crazy include
> header depency stuff going on.

Interestingly, I don't even see _PAGE_KERNEL defined on PowerPC...

> But given how popular drm on ppc and that the few platforms where people
> actually use gpus are the saner ones (hopefully) I think we can just move
> ahead with this change and fixup any compile breakage once it's reported.
> If there is any.

I'm not objecting to this change since it's merely reorganizing code.
This is just more possible future cleanup.

Thierry
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 7888dad..62fda6a 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -39,6 +39,17 @@ 
 #include "drm_legacy.h"
 
 #if __OS_HAS_AGP
+
+#ifdef HAVE_PAGE_AGP
+# include <asm/agp.h>
+#else
+# ifdef __powerpc__
+#  define PAGE_AGP	__pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
+# else
+#  define PAGE_AGP	PAGE_KERNEL
+# endif
+#endif
+
 static void *agp_remap(unsigned long offset, unsigned long size,
 		       struct drm_device * dev)
 {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d3504c6..294f7da 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -58,6 +58,8 @@ 
 #include <linux/agp_backend.h>
 #include <linux/workqueue.h>
 #include <linux/poll.h>
+#include <linux/highmem.h>
+#include <linux/vmalloc.h>
 #include <asm/pgalloc.h>
 #include <drm/drm.h>
 #include <drm/drm_sarea.h>
@@ -1127,10 +1129,6 @@  extern void drm_vm_open_locked(struct drm_device *dev, struct vm_area_struct *vm
 extern void drm_vm_close_locked(struct drm_device *dev, struct vm_area_struct *vma);
 extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
 
-				/* Memory management support (drm_memory.h) */
-#include <drm/drm_memory.h>
-
-
 				/* Misc. IOCTL support (drm_ioctl.h) */
 extern int drm_irq_by_busid(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv);
diff --git a/include/drm/drm_memory.h b/include/drm/drm_memory.h
deleted file mode 100644
index 4baf57a..0000000
--- a/include/drm/drm_memory.h
+++ /dev/null
@@ -1,59 +0,0 @@ 
-/**
- * \file drm_memory.h
- * Memory management wrappers for DRM
- *
- * \author Rickard E. (Rik) Faith <faith@valinux.com>
- * \author Gareth Hughes <gareth@valinux.com>
- */
-
-/*
- * Created: Thu Feb  4 14:00:34 1999 by faith@valinux.com
- *
- * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
- * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- */
-
-#include <linux/highmem.h>
-#include <linux/vmalloc.h>
-#include <drm/drmP.h>
-
-/**
- * Cut down version of drm_memory_debug.h, which used to be called
- * drm_memory.h.
- */
-
-#if __OS_HAS_AGP
-
-#ifdef HAVE_PAGE_AGP
-#include <asm/agp.h>
-#else
-# ifdef __powerpc__
-#  define PAGE_AGP	__pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
-# else
-#  define PAGE_AGP	PAGE_KERNEL
-# endif
-#endif
-
-#else				/* __OS_HAS_AGP */
-
-#endif