Hi
On Wed, Aug 7, 2013 at 5:53 PM, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> Hi David,
>
> 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. 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.
Cheers
David
>> 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
