[PATCH 17/25] drm: rip out drm_core_has_MTRR checks
David Herrmann
dh.herrmann at gmail.com
Thu Aug 15 05:44:26 PDT 2013
Hi
On Thu, Aug 8, 2013 at 3:41 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> The new arch_phys_wc_add/del functions do the right thing both with
> and without MTRR support in the kernel. So we can drop these
> additional checks.
>
> David Herrmann suggest to also kill the DRIVER_USE_MTRR flag since
> it's now unused, which spurred me to do a bit a better audit of the
> affected drivers. David helped a lot in that. Quoting our mail
> discussion:
>
> On Wed, Jul 10, 2013 at 5:41 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
>> On Wed, Jul 10, 2013 at 5:22 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>>> On Wed, Jul 10, 2013 at 3:51 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
>>>>> -#if __OS_HAS_MTRR
>>>>> -static inline int drm_core_has_MTRR(struct drm_device *dev)
>>>>> -{
>>>>> - return drm_core_check_feature(dev, DRIVER_USE_MTRR);
>>>>> -}
>>>>> -#else
>>>>> -#define drm_core_has_MTRR(dev) (0)
>>>>> -#endif
>>>>> -
>>>>
>>>> That was the last user of DRIVER_USE_MTRR (apart from drivers setting
>>>> it in .driver_features). Any reason to keep it around?
>>>
>>> Yeah, I guess we could rip things out. Which will also force me to
>>> properly audit drivers for the eventual behaviour change this could
>>> entail (in case there's an x86 driver which did not ask for an mtrr,
>>> but iirc there isn't).
>>
>> david at david-mb ~/dev/kernel/linux $ for i in drivers/gpu/drm/* ; do if
>> test -d "$i" ; then if ! grep -q USE_MTRR -r $i ; then echo $i ; fi ;
>> fi ; done
>> drivers/gpu/drm/exynos
>> drivers/gpu/drm/gma500
>> drivers/gpu/drm/i2c
>> drivers/gpu/drm/nouveau
>> drivers/gpu/drm/omapdrm
>> drivers/gpu/drm/qxl
>> drivers/gpu/drm/rcar-du
>> drivers/gpu/drm/shmobile
>> drivers/gpu/drm/tilcdc
>> drivers/gpu/drm/ttm
>> drivers/gpu/drm/udl
>> drivers/gpu/drm/vmwgfx
>> david at david-mb ~/dev/kernel/linux $
>>
>> So for x86 gma500,nouveau,qxl,udl,vmwgfx don't set DRIVER_USE_MTRR.
>> But I cannot tell whether they break if we call arch_phys_wc_add/del,
>> anyway. At least nouveau seemed to work here, but it doesn't use AGP
>> or drm_bufs, I guess.
>
> Cool, thanks a lot for stitching together the list of drivers to look
> at. So for real KMS drivers it's the drives responsibility to add an
> mtrr if it needs one. nouvea, radeon, mgag200, i915 and vmwgfx do that
> already. Somehow the savage driver also ends up doing that, I have no
> idea why.
>
> Note that gma500 as a pure KMS driver doesn't need MTRR setup since
> the platforms that it supports all support PAT. So no MTRRs needed to
> get wc iomappings.
>
> The mtrr support in the drm core is all for legacy mappings of garts,
> framebuffers and registers. All legacy drivers set the USE_MTRR flag,
> so we're good there.
>
> All in all I think we can really just ditch this
>
> /endquote
>
> v2: Also kill DRIVER_USE_MTRR as suggested by David Herrmann
>
> v3: Rebase on top of David Herrmann's agp setup/cleanup changes.
>
> Cc: David Herrmann <dh.herrmann at gmail.com>
> Cc: Andy Lutomirski <luto at amacapital.net>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Reviewed-by: David Herrmann <dh.herrmann at gmail.com>
Regards
David
> ---
> drivers/gpu/drm/ast/ast_drv.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_drv.c | 2 +-
> drivers/gpu/drm/drm_bufs.c | 13 +++++--------
> drivers/gpu/drm/drm_pci.c | 14 ++++++--------
> drivers/gpu/drm/drm_vm.c | 3 +--
> drivers/gpu/drm/i810/i810_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/mga/mga_drv.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +-
> drivers/gpu/drm/r128/r128_drv.c | 2 +-
> drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
> drivers/gpu/drm/savage/savage_drv.c | 2 +-
> drivers/gpu/drm/sis/sis_drv.c | 2 +-
> drivers/gpu/drm/tdfx/tdfx_drv.c | 1 -
> drivers/gpu/drm/via/via_drv.c | 2 +-
> include/drm/drmP.h | 11 -----------
> 16 files changed, 24 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 60f1ce3..32e270d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -197,7 +197,7 @@ static const struct file_operations ast_fops = {
> };
>
> static struct drm_driver driver = {
> - .driver_features = DRIVER_USE_MTRR | DRIVER_MODESET | DRIVER_GEM,
> + .driver_features = DRIVER_MODESET | DRIVER_GEM,
> .dev_priv_size = 0,
>
> .load = ast_driver_load,
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c b/drivers/gpu/drm/cirrus/cirrus_drv.c
> index dd9c908..138364d 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
> @@ -87,7 +87,7 @@ static const struct file_operations cirrus_driver_fops = {
> #endif
> };
> static struct drm_driver driver = {
> - .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_USE_MTRR,
> + .driver_features = DRIVER_MODESET | DRIVER_GEM,
> .load = cirrus_driver_load,
> .unload = cirrus_driver_unload,
> .fops = &cirrus_driver_fops,
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index 5f73f0a..f63133b 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -207,12 +207,10 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
> return 0;
> }
>
> - if (drm_core_has_MTRR(dev)) {
> - if (map->type == _DRM_FRAME_BUFFER ||
> - (map->flags & _DRM_WRITE_COMBINING)) {
> - map->mtrr =
> - arch_phys_wc_add(map->offset, map->size);
> - }
> + if (map->type == _DRM_FRAME_BUFFER ||
> + (map->flags & _DRM_WRITE_COMBINING)) {
> + map->mtrr =
> + arch_phys_wc_add(map->offset, map->size);
> }
> if (map->type == _DRM_REGISTERS) {
> if (map->flags & _DRM_WRITE_COMBINING)
> @@ -464,8 +462,7 @@ int drm_rmmap_locked(struct drm_device *dev, struct drm_local_map *map)
> iounmap(map->handle);
> /* FALLTHROUGH */
> case _DRM_FRAME_BUFFER:
> - if (drm_core_has_MTRR(dev))
> - arch_phys_wc_del(map->mtrr);
> + arch_phys_wc_del(map->mtrr);
> break;
> case _DRM_SHM:
> vfree(map->handle);
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 0f54ad8..3fca2db 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -272,12 +272,11 @@ static int drm_pci_agp_init(struct drm_device *dev)
> DRM_ERROR("Cannot initialize the agpgart module.\n");
> return -EINVAL;
> }
> - if (drm_core_has_MTRR(dev)) {
> - if (dev->agp)
> - dev->agp->agp_mtrr = arch_phys_wc_add(
> - dev->agp->agp_info.aper_base,
> - dev->agp->agp_info.aper_size *
> - 1024 * 1024);
> + if (dev->agp) {
> + dev->agp->agp_mtrr = arch_phys_wc_add(
> + dev->agp->agp_info.aper_base,
> + dev->agp->agp_info.aper_size *
> + 1024 * 1024);
> }
> }
> return 0;
> @@ -286,8 +285,7 @@ static int drm_pci_agp_init(struct drm_device *dev)
> static void drm_pci_agp_destroy(struct drm_device *dev)
> {
> if (drm_core_has_AGP(dev) && dev->agp) {
> - if (drm_core_has_MTRR(dev))
> - arch_phys_wc_del(dev->agp->agp_mtrr);
> + arch_phys_wc_del(dev->agp->agp_mtrr);
> drm_agp_clear(dev);
> drm_agp_destroy(dev->agp);
> dev->agp = NULL;
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index feb2003..b5c5af7 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -251,8 +251,7 @@ static void drm_vm_shm_close(struct vm_area_struct *vma)
> switch (map->type) {
> case _DRM_REGISTERS:
> case _DRM_FRAME_BUFFER:
> - if (drm_core_has_MTRR(dev))
> - arch_phys_wc_del(map->mtrr);
> + arch_phys_wc_del(map->mtrr);
> iounmap(map->handle);
> break;
> case _DRM_SHM:
> diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
> index d85c05b..d8180d2 100644
> --- a/drivers/gpu/drm/i810/i810_drv.c
> +++ b/drivers/gpu/drm/i810/i810_drv.c
> @@ -57,7 +57,7 @@ static const struct file_operations i810_driver_fops = {
>
> static struct drm_driver driver = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | DRIVER_USE_MTRR |
> + DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
> DRIVER_HAVE_DMA,
> .dev_priv_size = sizeof(drm_i810_buf_priv_t),
> .load = i810_driver_load,
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9411a74..eec47bd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1006,7 +1006,7 @@ static struct drm_driver driver = {
> * deal with them for Intel hardware.
> */
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | /* DRIVER_USE_MTRR |*/
> + DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
> DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
> .load = i915_driver_load,
> .unload = i915_driver_unload,
> diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
> index fe71e1e..6b1a87c 100644
> --- a/drivers/gpu/drm/mga/mga_drv.c
> +++ b/drivers/gpu/drm/mga/mga_drv.c
> @@ -58,7 +58,7 @@ static const struct file_operations mga_driver_fops = {
>
> static struct drm_driver driver = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA |
> + DRIVER_USE_AGP | DRIVER_PCI_DMA |
> DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
> .dev_priv_size = sizeof(drm_mga_buf_priv_t),
> .load = mga_driver_load,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index b570127..fcce7b2 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -88,7 +88,7 @@ static const struct file_operations mgag200_driver_fops = {
> };
>
> static struct drm_driver driver = {
> - .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_USE_MTRR,
> + .driver_features = DRIVER_GEM | DRIVER_MODESET,
> .load = mgag200_driver_load,
> .unload = mgag200_driver_unload,
> .fops = &mgag200_driver_fops,
> diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
> index c2338cb..5bd307c 100644
> --- a/drivers/gpu/drm/r128/r128_drv.c
> +++ b/drivers/gpu/drm/r128/r128_drv.c
> @@ -56,7 +56,7 @@ static const struct file_operations r128_driver_fops = {
>
> static struct drm_driver driver = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
> + DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG |
> DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
> .dev_priv_size = sizeof(drm_r128_buf_priv_t),
> .load = r128_driver_load,
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 3e52331..1f93dd5 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -275,7 +275,7 @@ static const struct file_operations radeon_driver_old_fops = {
>
> static struct drm_driver driver_old = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
> + DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG |
> DRIVER_HAVE_IRQ | DRIVER_HAVE_DMA | DRIVER_IRQ_SHARED,
> .dev_priv_size = sizeof(drm_radeon_buf_priv_t),
> .load = radeon_driver_load,
> @@ -382,7 +382,7 @@ static const struct file_operations radeon_driver_kms_fops = {
>
> static struct drm_driver kms_driver = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_USE_MTRR |
> + DRIVER_USE_AGP |
> DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
> DRIVER_PRIME,
> .dev_priv_size = 0,
> diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c
> index 9135c8b..3c03021 100644
> --- a/drivers/gpu/drm/savage/savage_drv.c
> +++ b/drivers/gpu/drm/savage/savage_drv.c
> @@ -50,7 +50,7 @@ static const struct file_operations savage_driver_fops = {
>
> static struct drm_driver driver = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_HAVE_DMA | DRIVER_PCI_DMA,
> + DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA,
> .dev_priv_size = sizeof(drm_savage_buf_priv_t),
> .load = savage_driver_load,
> .firstopen = savage_driver_firstopen,
> diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
> index b88b2d3..4383b74 100644
> --- a/drivers/gpu/drm/sis/sis_drv.c
> +++ b/drivers/gpu/drm/sis/sis_drv.c
> @@ -102,7 +102,7 @@ void sis_driver_postclose(struct drm_device *dev, struct drm_file *file)
> }
>
> static struct drm_driver driver = {
> - .driver_features = DRIVER_USE_AGP | DRIVER_USE_MTRR,
> + .driver_features = DRIVER_USE_AGP,
> .load = sis_driver_load,
> .unload = sis_driver_unload,
> .open = sis_driver_open,
> diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c
> index 951ec13..3492ca5 100644
> --- a/drivers/gpu/drm/tdfx/tdfx_drv.c
> +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c
> @@ -55,7 +55,6 @@ static const struct file_operations tdfx_driver_fops = {
> };
>
> static struct drm_driver driver = {
> - .driver_features = DRIVER_USE_MTRR,
> .fops = &tdfx_driver_fops,
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c
> index 4487999..92684a9 100644
> --- a/drivers/gpu/drm/via/via_drv.c
> +++ b/drivers/gpu/drm/via/via_drv.c
> @@ -72,7 +72,7 @@ static const struct file_operations via_driver_fops = {
>
> static struct drm_driver driver = {
> .driver_features =
> - DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_HAVE_IRQ |
> + DRIVER_USE_AGP | DRIVER_HAVE_IRQ |
> DRIVER_IRQ_SHARED,
> .load = via_driver_load,
> .unload = via_driver_unload,
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 914055e..5b9462d 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -76,7 +76,6 @@
> #include <linux/idr.h>
>
> #define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && defined(MODULE)))
> -#define __OS_HAS_MTRR (defined(CONFIG_MTRR))
>
> struct module;
>
> @@ -141,7 +140,6 @@ int drm_err(const char *func, const char *format, ...);
> /* driver capabilities and requirements mask */
> #define DRIVER_USE_AGP 0x1
> #define DRIVER_REQUIRE_AGP 0x2
> -#define DRIVER_USE_MTRR 0x4
> #define DRIVER_PCI_DMA 0x8
> #define DRIVER_SG 0x10
> #define DRIVER_HAVE_DMA 0x20
> @@ -1219,15 +1217,6 @@ static inline int drm_core_has_AGP(struct drm_device *dev)
> #define drm_core_has_AGP(dev) (0)
> #endif
>
> -#if __OS_HAS_MTRR
> -static inline int drm_core_has_MTRR(struct drm_device *dev)
> -{
> - return drm_core_check_feature(dev, DRIVER_USE_MTRR);
> -}
> -#else
> -#define drm_core_has_MTRR(dev) (0)
> -#endif
> -
> static inline void drm_device_set_unplugged(struct drm_device *dev)
> {
> smp_wmb();
> --
> 1.8.3.2
>
More information about the dri-devel
mailing list