All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Fix undefined reference to drm_agp_clear() on non-AGP platforms
@ 2013-08-07 12:17 Laurent Pinchart
  2013-08-07 12:19 ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Laurent Pinchart @ 2013-08-07 12:17 UTC (permalink / raw)
  To: dri-devel; +Cc: Dave Airlie

The drm_agp_clear() function is only defined on platforms with AGP
support. Move the drm_core_has_AGP() check from drm_agp_clear() to the
caller to let the compiler optimize the drm_agp_clear() call away.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/drm_agpsupport.c | 2 +-
 drivers/gpu/drm/drm_drv.c        | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

This fixes a link-time build error in drm-next. An alternative approach would
be to guard the drm_agp_clear() call in drm_drv.c with an #if __OS_HAS_AGP. I
can resubmit the patch to do so if preferred.

diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
index e301d65..084a674 100644
--- a/drivers/gpu/drm/drm_agpsupport.c
+++ b/drivers/gpu/drm/drm_agpsupport.c
@@ -439,7 +439,7 @@ void drm_agp_clear(struct drm_device *dev)
 {
 	struct drm_agp_mem *entry, *tempe;
 
-	if (!drm_core_has_AGP(dev) || !dev->agp)
+	if (!dev->agp)
 		return;
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dddd799..1e2ad35 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -195,7 +195,8 @@ int drm_lastclose(struct drm_device * dev)
 
 	mutex_lock(&dev->struct_mutex);
 
-	drm_agp_clear(dev);
+	if (drm_core_has_AGP(dev))
+		drm_agp_clear(dev);
 
 	if (drm_core_check_feature(dev, DRIVER_SG) && dev->sg &&
 	    !drm_core_check_feature(dev, DRIVER_MODESET)) {
-- 
Regards,

Laurent Pinchart

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: Fix undefined reference to drm_agp_clear() on non-AGP platforms
  2013-08-07 12:17 [PATCH] drm: Fix undefined reference to drm_agp_clear() on non-AGP platforms Laurent Pinchart
@ 2013-08-07 12:19 ` Daniel Vetter
  2013-08-07 12:39   ` Laurent Pinchart
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-08-07 12:19 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Dave Airlie, dri-devel

On Wed, Aug 7, 2013 at 2:17 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The drm_agp_clear() function is only defined on platforms with AGP
> support. Move the drm_core_has_AGP() check from drm_agp_clear() to the
> caller to let the compiler optimize the drm_agp_clear() call away.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Can't we use the usual approach of an empty static inline helper for
the !CONFIG_AGP case here?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: Fix undefined reference to drm_agp_clear() on non-AGP platforms
  2013-08-07 12:19 ` Daniel Vetter
@ 2013-08-07 12:39   ` Laurent Pinchart
  2013-08-07 12:50     ` David Herrmann
  0 siblings, 1 reply; 4+ messages in thread
From: Laurent Pinchart @ 2013-08-07 12:39 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Dave Airlie, Laurent Pinchart, dri-devel

Hi Daniel,

On Wednesday 07 August 2013 14:19:34 Daniel Vetter wrote:
> On Wed, Aug 7, 2013 at 2:17 PM, Laurent Pinchart wrote:
> > The drm_agp_clear() function is only defined on platforms with AGP
> > support. Move the drm_core_has_AGP() check from drm_agp_clear() to the
> > caller to let the compiler optimize the drm_agp_clear() call away.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Can't we use the usual approach of an empty static inline helper for
> the !CONFIG_AGP case here?

Sure, that's possible as well. I find my solution slightly more explicit as it 
shows that drm_agp_clear() will only be called for platforms that have AGP, 
but I would be fine with a static inline as well.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm: Fix undefined reference to drm_agp_clear() on non-AGP platforms
  2013-08-07 12:39   ` Laurent Pinchart
@ 2013-08-07 12:50     ` David Herrmann
  0 siblings, 0 replies; 4+ messages in thread
From: David Herrmann @ 2013-08-07 12:50 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Dave Airlie, Laurent Pinchart, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

Hi

On Wed, Aug 7, 2013 at 2:39 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Daniel,
>
> On Wednesday 07 August 2013 14:19:34 Daniel Vetter wrote:
>> On Wed, Aug 7, 2013 at 2:17 PM, Laurent Pinchart wrote:
>> > The drm_agp_clear() function is only defined on platforms with AGP
>> > support. Move the drm_core_has_AGP() check from drm_agp_clear() to the
>> > caller to let the compiler optimize the drm_agp_clear() call away.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > <laurent.pinchart+renesas@ideasonboard.com>
>>
>> Can't we use the usual approach of an empty static inline helper for
>> the !CONFIG_AGP case here?
>
> Sure, that's possible as well. I find my solution slightly more explicit as it
> shows that drm_agp_clear() will only be called for platforms that have AGP,
> but I would be fine with a static inline as well.

But the compile fails with -O0.. we don't depend on optimizations to
avoid compiler errors. Besides, "static inline" is standard kernel
coding-style so I think we should stick to it.

I've attached a patch, feel free to pick it up. Otherwise, I will give
it some test-compiles once I get home and send it to dri-devel.

Thanks
David

[-- Attachment #2: drm_agp_fix.patch --]
[-- Type: application/octet-stream, Size: 8617 bytes --]

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fba5473..3ecdde6 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -62,10 +62,8 @@
 #endif
 #include <asm/mman.h>
 #include <asm/uaccess.h>
-#if defined(CONFIG_AGP) || defined(CONFIG_AGP_MODULE)
 #include <linux/types.h>
 #include <linux/agp_backend.h>
-#endif
 #include <linux/workqueue.h>
 #include <linux/poll.h>
 #include <asm/pgalloc.h>
@@ -1226,16 +1224,6 @@ static inline int drm_dev_to_irq(struct drm_device *dev)
 	return dev->driver->bus->get_irq(dev);
 }
 
-
-#if __OS_HAS_AGP
-static inline int drm_core_has_AGP(struct drm_device *dev)
-{
-	return drm_core_check_feature(dev, DRIVER_USE_AGP);
-}
-#else
-#define drm_core_has_AGP(dev) (0)
-#endif
-
 #if __OS_HAS_MTRR
 static inline int drm_core_has_MTRR(struct drm_device *dev)
 {
@@ -1292,14 +1280,6 @@ extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
 
 				/* Memory management support (drm_memory.h) */
 #include <drm/drm_memory.h>
-extern void drm_free_agp(DRM_AGP_MEM * handle, int pages);
-extern int drm_bind_agp(DRM_AGP_MEM * handle, unsigned int start);
-extern DRM_AGP_MEM *drm_agp_bind_pages(struct drm_device *dev,
-				       struct page **pages,
-				       unsigned long num_pages,
-				       uint32_t gtt_offset,
-				       uint32_t type);
-extern int drm_unbind_agp(DRM_AGP_MEM * handle);
 
 				/* Misc. IOCTL support (drm_ioctl.h) */
 extern int drm_irq_by_busid(struct drm_device *dev, void *data,
@@ -1453,33 +1433,8 @@ extern int drm_modeset_ctl(struct drm_device *dev, void *data,
 			   struct drm_file *file_priv);
 
 				/* AGP/GART support (drm_agpsupport.h) */
-extern struct drm_agp_head *drm_agp_init(struct drm_device *dev);
-extern void drm_agp_destroy(struct drm_agp_head *agp);
-extern void drm_agp_clear(struct drm_device *dev);
-extern int drm_agp_acquire(struct drm_device *dev);
-extern int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
-				 struct drm_file *file_priv);
-extern int drm_agp_release(struct drm_device *dev);
-extern int drm_agp_release_ioctl(struct drm_device *dev, void *data,
-				 struct drm_file *file_priv);
-extern int drm_agp_enable(struct drm_device *dev, struct drm_agp_mode mode);
-extern int drm_agp_enable_ioctl(struct drm_device *dev, void *data,
-				struct drm_file *file_priv);
-extern int drm_agp_info(struct drm_device *dev, struct drm_agp_info *info);
-extern int drm_agp_info_ioctl(struct drm_device *dev, void *data,
-			struct drm_file *file_priv);
-extern int drm_agp_alloc(struct drm_device *dev, struct drm_agp_buffer *request);
-extern int drm_agp_alloc_ioctl(struct drm_device *dev, void *data,
-			 struct drm_file *file_priv);
-extern int drm_agp_free(struct drm_device *dev, struct drm_agp_buffer *request);
-extern int drm_agp_free_ioctl(struct drm_device *dev, void *data,
-			struct drm_file *file_priv);
-extern int drm_agp_unbind(struct drm_device *dev, struct drm_agp_binding *request);
-extern int drm_agp_unbind_ioctl(struct drm_device *dev, void *data,
-			  struct drm_file *file_priv);
-extern int drm_agp_bind(struct drm_device *dev, struct drm_agp_binding *request);
-extern int drm_agp_bind_ioctl(struct drm_device *dev, void *data,
-			struct drm_file *file_priv);
+
+#include <drm/drm_agpsupport.h>
 
 				/* Stub support (drm_stub.h) */
 extern int drm_setmaster_ioctl(struct drm_device *dev, void *data,
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
new file mode 100644
index 0000000..f926542
--- /dev/null
+++ b/include/drm/drm_agpsupport.h
@@ -0,0 +1,194 @@
+#ifndef _DRM_AGPSUPPORT_H_
+#define _DRM_AGPSUPPORT_H_
+
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/agp_backend.h>
+#include <drm/drmP.h>
+
+#ifdef __OS_HAS_AGP
+
+void drm_free_agp(DRM_AGP_MEM * handle, int pages);
+int drm_bind_agp(DRM_AGP_MEM * handle, unsigned int start);
+int drm_unbind_agp(DRM_AGP_MEM * handle);
+DRM_AGP_MEM *drm_agp_bind_pages(struct drm_device *dev,
+				struct page **pages,
+				unsigned long num_pages,
+				uint32_t gtt_offset,
+				uint32_t type);
+
+struct drm_agp_head *drm_agp_init(struct drm_device *dev);
+void drm_agp_destroy(struct drm_agp_head *agp);
+void drm_agp_clear(struct drm_device *dev);
+int drm_agp_acquire(struct drm_device *dev);
+int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
+			  struct drm_file *file_priv);
+int drm_agp_release(struct drm_device *dev);
+int drm_agp_release_ioctl(struct drm_device *dev, void *data,
+			  struct drm_file *file_priv);
+int drm_agp_enable(struct drm_device *dev, struct drm_agp_mode mode);
+int drm_agp_enable_ioctl(struct drm_device *dev, void *data,
+			 struct drm_file *file_priv);
+int drm_agp_info(struct drm_device *dev, struct drm_agp_info *info);
+int drm_agp_info_ioctl(struct drm_device *dev, void *data,
+		       struct drm_file *file_priv);
+int drm_agp_alloc(struct drm_device *dev, struct drm_agp_buffer *request);
+int drm_agp_alloc_ioctl(struct drm_device *dev, void *data,
+			struct drm_file *file_priv);
+int drm_agp_free(struct drm_device *dev, struct drm_agp_buffer *request);
+int drm_agp_free_ioctl(struct drm_device *dev, void *data,
+		       struct drm_file *file_priv);
+int drm_agp_unbind(struct drm_device *dev, struct drm_agp_binding *request);
+int drm_agp_unbind_ioctl(struct drm_device *dev, void *data,
+			 struct drm_file *file_priv);
+int drm_agp_bind(struct drm_device *dev, struct drm_agp_binding *request);
+int drm_agp_bind_ioctl(struct drm_device *dev, void *data,
+		       struct drm_file *file_priv);
+
+static inline int drm_core_has_AGP(struct drm_device *dev)
+{
+	return drm_core_check_feature(dev, DRIVER_USE_AGP);
+}
+
+#else /* __OS_HAS_AGP */
+
+static inline void drm_free_agp(DRM_AGP_MEM * handle, int pages)
+{
+}
+
+static inline int drm_bind_agp(DRM_AGP_MEM * handle, unsigned int start)
+{
+	return -ENODEV;
+}
+
+static inline int drm_unbind_agp(DRM_AGP_MEM * handle)
+{
+	return -ENODEV;
+}
+
+static inline DRM_AGP_MEM *drm_agp_bind_pages(struct drm_device *dev,
+					      struct page **pages,
+					      unsigned long num_pages,
+					      uint32_t gtt_offset,
+					      uint32_t type)
+{
+	return NULL;
+}
+
+static inline struct drm_agp_head *drm_agp_init(struct drm_device *dev)
+{
+	return NULL;
+}
+
+static inline void drm_agp_destroy(struct drm_agp_head *agp)
+{
+}
+
+static inline void drm_agp_clear(struct drm_device *dev)
+{
+}
+
+static inline int drm_agp_acquire(struct drm_device *dev)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_acquire_ioctl(struct drm_device *dev, void *data,
+					struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_release(struct drm_device *dev)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_release_ioctl(struct drm_device *dev, void *data,
+					struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_enable(struct drm_device *dev,
+				 struct drm_agp_mode mode)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_enable_ioctl(struct drm_device *dev, void *data,
+				       struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_info(struct drm_device *dev,
+			       struct drm_agp_info *info)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_info_ioctl(struct drm_device *dev, void *data,
+				     struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_alloc(struct drm_device *dev,
+				struct drm_agp_buffer *request)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_alloc_ioctl(struct drm_device *dev, void *data,
+				      struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_free(struct drm_device *dev,
+			       struct drm_agp_buffer *request)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_free_ioctl(struct drm_device *dev, void *data,
+				     struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_unbind(struct drm_device *dev,
+				 struct drm_agp_binding *request)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_unbind_ioctl(struct drm_device *dev, void *data,
+				       struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_bind(struct drm_device *dev,
+			       struct drm_agp_binding *request)
+{
+	return -ENODEV;
+}
+
+static inline int drm_agp_bind_ioctl(struct drm_device *dev, void *data,
+				     struct drm_file *file_priv)
+{
+	return -ENODEV;
+}
+
+static inline int drm_core_has_AGP(struct drm_device *dev)
+{
+	return 0;
+}
+
+#endif /* __OS_HAS_AGP */
+
+#endif /* _DRM_AGPSUPPORT_H_ */

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-08-07 12:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-07 12:17 [PATCH] drm: Fix undefined reference to drm_agp_clear() on non-AGP platforms Laurent Pinchart
2013-08-07 12:19 ` Daniel Vetter
2013-08-07 12:39   ` Laurent Pinchart
2013-08-07 12:50     ` David Herrmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.