Hi David,
On Wednesday 07 August 2013 18:12:23 David Herrmann wrote:
> On Wed, Aug 7, 2013 at 5:53 PM, Laurent Pinchart wrote:
> > On Wednesday 07 August 2013 15:53:15 David Herrmann wrote:
> >> We currently rely on gcc dead-code elimination so the drm_agp_* helpers
> >> are not called if drm_core_has_AGP() is false. That's ugly as hell so
> >> provide "static inline" dummies for the case that AGP is disabled.
> >
> > Please note that we rely on -O2 in many places in the kernel through the
> > IS_ENABLED() macro. If you start a crusade against this you will find way
> > more work than you expect, and most probably people who will disagree
> > with you.
>
> Where? I didn't find a line which does that. All code-paths still provide
> dummies for all accessed variables and functions.
I would have sworn I had seen examples of this just the other day, but I can't
find them :-/ I might have been wrong, sorry about that.
> Anyhow, I wrote this because I never heard that we actually do this, so I am
> fine with your patch. Sorry for the confusion.
>
> > This being said, I still prefer moving the explicit drm_core_has_AGP()
> > check to drm_drv.c, but I'm OK with this patch.
>
> Again, fine to me. I thought it's cleaner to move it to drm_agpsupport.c,
> but that's a matter of taste.
Let's go with your patch :-)
> >> Fixes a build-regression introduced by:
> >> commit 28ec711cd427f8b61f73712a43b8100ba8ca933b
> >> Author: David Herrmann <dh.herrmann@xxxxxxxxx>
> >> Date: Sat Jul 27 16:37:00 2013 +0200
> >>
> >> drm/agp: move AGP cleanup paths to drm_agpsupport.c
> >>
> >> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >> Cc: Daniel Vetter <daniel@xxxxxxxx>
> >> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
> >> ---
> >> Hi guys
> >>
> >> I test-compiled this with AGP=y and AGP=n but I don't have the resources
> >> to do more extensive tests. It would take forever on my intel Atom. Maybe
> >> someone can push this to Fenguan's build-bot?
> >>
> >> Cheers
> >> David
> >>
> >> include/drm/drmP.h | 49 +----------
> >> include/drm/drm_agpsupport.h | 194
> >> ++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 196 insertions(+), 47 deletions(-)
> >> create mode 100644 include/drm/drm_agpsupport.h
> >>
> >> 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_ */
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
