diff mbox

[10/20] drm: move __OS_HAS_AGP into drm_agpsupport.h

Message ID 1409307166-12396-11-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
With drm_memory.h gone, there is no header left that uses __OS_HAS_AGP.
Move it into drm_agpsupport.h (which is itself included from drmP.h) to
hide it harder from public eyes.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/drm/drmP.h           | 2 --
 include/drm/drm_agpsupport.h | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Thierry Reding Aug. 29, 2014, 12:03 p.m. UTC | #1
On Fri, Aug 29, 2014 at 12:12:36PM +0200, David Herrmann wrote:
> With drm_memory.h gone, there is no header left that uses __OS_HAS_AGP.
> Move it into drm_agpsupport.h (which is itself included from drmP.h) to
> hide it harder from public eyes.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  include/drm/drmP.h           | 2 --
>  include/drm/drm_agpsupport.h | 3 +++
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 294f7da..c6f337c 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -67,8 +67,6 @@
>  
>  #include <linux/idr.h>
>  
> -#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && defined(MODULE)))
> -
>  struct module;
>  
>  struct drm_file;
> diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
> index 3bebeb4..4f1724c 100644
> --- a/include/drm/drm_agpsupport.h
> +++ b/include/drm/drm_agpsupport.h
> @@ -8,6 +8,9 @@
>  #include <linux/agp_backend.h>
>  #include <drm/drmP.h>
>  
> +#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && \
> +					      defined(MODULE)))

I'm not really sure what the intent was of the final defined(MODULE).
Surely the fact whether a driver is being built as a module or not does
not influence whether or not the OS supports AGP.

And if this is merely meant to make sure that drivers that are built-in
don't break to build if AGP is a module, then that should be a job for
Kconfig rather than some macro.

So I think the above could simply become:

	#define __OS_HAS_AGP IS_ENABLED(CONFIG_AGP)

And once we have that I think we could even easily get rid of the custom
__OS_HAS_AGP macro and use IS_ENABLED(CONFIG_AGP) directly.

Thierry
Daniel Vetter Aug. 29, 2014, 12:45 p.m. UTC | #2
On Fri, Aug 29, 2014 at 02:03:12PM +0200, Thierry Reding wrote:
> On Fri, Aug 29, 2014 at 12:12:36PM +0200, David Herrmann wrote:
> > With drm_memory.h gone, there is no header left that uses __OS_HAS_AGP.
> > Move it into drm_agpsupport.h (which is itself included from drmP.h) to
> > hide it harder from public eyes.
> > 
> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> > ---
> >  include/drm/drmP.h           | 2 --
> >  include/drm/drm_agpsupport.h | 3 +++
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 294f7da..c6f337c 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -67,8 +67,6 @@
> >  
> >  #include <linux/idr.h>
> >  
> > -#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && defined(MODULE)))
> > -
> >  struct module;
> >  
> >  struct drm_file;
> > diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
> > index 3bebeb4..4f1724c 100644
> > --- a/include/drm/drm_agpsupport.h
> > +++ b/include/drm/drm_agpsupport.h
> > @@ -8,6 +8,9 @@
> >  #include <linux/agp_backend.h>
> >  #include <drm/drmP.h>
> >  
> > +#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && \
> > +					      defined(MODULE)))
> 
> I'm not really sure what the intent was of the final defined(MODULE).
> Surely the fact whether a driver is being built as a module or not does
> not influence whether or not the OS supports AGP.

I think this was to make sure agp drivers are loaded before the drm
drivers. But since ages we can build the different agp drivers
indidivudally as modules, so this stopped making any sense at all.

> And if this is merely meant to make sure that drivers that are built-in
> don't break to build if AGP is a module, then that should be a job for
> Kconfig rather than some macro.
> 
> So I think the above could simply become:
> 
> 	#define __OS_HAS_AGP IS_ENABLED(CONFIG_AGP)
> 
> And once we have that I think we could even easily get rid of the custom
> __OS_HAS_AGP macro and use IS_ENABLED(CONFIG_AGP) directly.

Yeah, I think a simple IS_ENABLED(CONFIG_AGP) sould be good enough.
-Daniel
diff mbox

Patch

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 294f7da..c6f337c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -67,8 +67,6 @@ 
 
 #include <linux/idr.h>
 
-#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && defined(MODULE)))
-
 struct module;
 
 struct drm_file;
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index 3bebeb4..4f1724c 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -8,6 +8,9 @@ 
 #include <linux/agp_backend.h>
 #include <drm/drmP.h>
 
+#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && \
+					      defined(MODULE)))
+
 struct drm_agp_head {
 	struct agp_kern_info agp_info;
 	struct list_head memory;