Re: [PATCH 00/39] clean out drm cruft and hide it better for kms drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Wed, Jul 10, 2013 at 2:11 PM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> Hi all,
>
> I've figured that it's again time for a bit of (late) drm spring cleanup. This
> series here consists of a pile of "rip old stuff out" patches interleaved with
> "disable old cruft for kms drivers and hide it better".
>
> Comments, flames and review highly welcome. I'd be especially happy if the arm
> guys could check whether I haven't badly broken their drivers - compile-testing
> arm is a pita, so I haven't yet done that.
>
> There's a few driver-wide patches included, but the more invasive ones (i.e.
> changing more than the drm driver vtable) are split out per-driver for easier
> merging. If no one screams my plan is to rebase this pile on top of -rc1, give
> it some more testing (check arm, ugh) and then send a pull request to Dave.
> That should reduce interference with ongoing driver work as much as possible I
> hope.
>
> My drm cruft todo list still has a pile of ideas, but I've figured I need to
> stop now for 3.12. For those interested further cleanups could include:
>
> - setversion/set_busid: All drm core version 1.1 is legacy cruft, kms drivers
>   should never run in this mode. We could clean up the setversion ioclt (and
>   move the drm core version handling into a legacy function) and set up the bus
>   id unconditionally at driver load time.
>
> - There's a few more legacy ioctls/subsystems that could be blocked out for kms
>   drivers (but the required git history digging tends to be tedious). Also I
>   think we could more aggressively move legacy cruft setup/teardown code
>   out-of-line from the main code by extracting it into drm_legacy_ functions
>   (like this series here already does for the context and dma stuff).
>
> - I think creating a drm_internal.h header for functions not exported to drivers
>   would be useful. That way we could move all the legacy functions out of drmP.h
>   (which are a lot of them), which should make it much clearer what the real drm
>   driver interface actually is.

Please do that! It also reduces drmP.h and thus I don't have to
recompile all drivers once we change legacy core code.

> - drm_os_linux.h should just die in fire.
>
> - There's a pile of needless indirection around our agp handling, we duplicate
>   the agp core's CONFIG_AGP=n no-oping function handling in large parts, among
>   other stuff.

I have a cleanup patch for AGP setup+teardown which further simplifies
drm_fill_in_dev(). See:
  https://github.com/dvdhrm/linux/commits/drmdev
I will rebase the drm_dev_*() helpers once Dave merged your series.

> - The drm coherent dma alloc helpers could get ripped out, at least for kms
>   drivers. For ums drivers there's some funny cases where this mapping is
>   exchanged with userspace for e.g. register access. In a least one case
>   (i810.ko) userspace even sets up that mapping, which allows it to crash the
>   kernel at will (since those maps aren't refcounted). Maybe we need to shovel
>   those interfaces into a drm_legacy.ko module to keep them around but make sure
>   that no new driver even thinks about using them.
>
> - There's also the matter of the vblank support code, imo that should be split
>   int a ums and a kms part. That'd would allow us to use struct drm_crtc * in
>   interfaces and proper locking (by grabbing crtc->mutex to exclude races with
>   dpms/modesets).
>
> If anyone wants to dig around in those areas please poke me.
>
> Cheers, Daniel
>
> Daniel Vetter (39):
>   drm: remove drm_modctx ioctl and use drm_noop instead
>   drm: kill dev->context_wait
>   drm: remove dev->last_switch
>   drm: kill dev->interrupt_flag and dev->dma_flag
>   drm: kill dev->ctx_start and dev->lck_start
>   drm/radoen: kill radeon_dma_ioctl_kms
>   drm: kill dev->buf_readers and dev->buf_writers
>   drm: remove redundant clears from drm_setup
>   drm/omap: kill firstopen callback
>   drm/radeon: kill firstopen callback for kms driver
>   drm/imx: kill firstopen callback
>   drm/vmwgfx: remove ->firstopen callback
>   drm: don't call ->firstopen for KMS drivers
>   drm: kill dev->driver->set_version
>   drm/radeon: remove DRIVER_HAS_DMA/SG/PCI_DMA from the kms driver
>   drm: fold in drm_sg_alloc into the ioctl
>   drm: hide legacy sg cleanup better from common code
>   drm: disallow legacy sg ioctls for modesetting drivers
>   drm: mark dma setup/teardown as legacy systems
>   drm/nouveau: drop DRIVER_PCI_DMA and DRIVER_SG
>   drm: disallow legacy dma ioctls for modesetting drivers
>   drm: move drm_getsarea into drm_bufs.c
>   drm/bufs: s/drm_order/order_base_2/
>   drm/r128: s/drm_order/order_base_2/
>   drm/radeon: s/drm_order/order_base_2/
>   drm: remove drm_order
>   drm: mark context support as a legacy subsystem
>   drm/vmwgfx: remove redundant clearing of driver->dma_quiescent
>   drm: remove FASYNC support
>   drm: rip out DRIVER_FB_DMA and related code
>   drm: rip out a few unused DRIVER flags
>   drm: remove a bunch of unused #defines from drmP.h
>   drm: rip out drm_core_has_MTRR checks
>   drm: remove the dma_ioctl special-case
>   drm/memory: don't export agp helpers
>   drm: hollow-out GET_CLIENT ioctl
>   drm: no-op out GET_STATS ioctl
>   drm: fix locking in gem debugfs/procfs file
>   drm: remove procfs code, take 2

Apart from driver chances, I reviewed all of them and they look good,
except for few things I mentioned for the individual patches. But most
of them are trivial, anyway.
Also (on nouveau):
  Tested-by: David Herrmann <dh.herrmann@xxxxxxxxx>

Cheers
David

>  drivers/gpu/drm/Makefile                 |   2 +-
>  drivers/gpu/drm/ast/ast_drv.c            |   1 -
>  drivers/gpu/drm/cirrus/cirrus_drv.c      |   1 -
>  drivers/gpu/drm/drm_bufs.c               | 238 +++++++------------------------
>  drivers/gpu/drm/drm_context.c            |  81 ++++++++---
>  drivers/gpu/drm/drm_dma.c                |  17 ++-
>  drivers/gpu/drm/drm_drv.c                |  44 +-----
>  drivers/gpu/drm/drm_fops.c               |  66 +--------
>  drivers/gpu/drm/drm_info.c               |   4 +
>  drivers/gpu/drm/drm_ioctl.c              |  43 +-----
>  drivers/gpu/drm/drm_memory.c             |   2 -
>  drivers/gpu/drm/drm_pci.c                |  11 +-
>  drivers/gpu/drm/drm_proc.c               | 209 ---------------------------
>  drivers/gpu/drm/drm_scatter.c            |  29 ++--
>  drivers/gpu/drm/drm_stub.c               |  44 ++----
>  drivers/gpu/drm/drm_vm.c                 |   3 +-
>  drivers/gpu/drm/gma500/psb_drv.c         |   3 +-
>  drivers/gpu/drm/i810/i810_dma.c          |   1 -
>  drivers/gpu/drm/i810/i810_drv.c          |   1 -
>  drivers/gpu/drm/i915/i915_drv.c          |   1 -
>  drivers/gpu/drm/mga/mga_drv.c            |   1 -
>  drivers/gpu/drm/mgag200/mgag200_drv.c    |   1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c    |   3 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c       |   8 --
>  drivers/gpu/drm/qxl/qxl_drv.c            |   1 -
>  drivers/gpu/drm/r128/r128_cce.c          |   2 +-
>  drivers/gpu/drm/r128/r128_drv.c          |   1 -
>  drivers/gpu/drm/radeon/cik.c             |  14 +-
>  drivers/gpu/drm/radeon/evergreen.c       |   4 +-
>  drivers/gpu/drm/radeon/ni.c              |   6 +-
>  drivers/gpu/drm/radeon/r100.c            |   2 +-
>  drivers/gpu/drm/radeon/r600.c            |  14 +-
>  drivers/gpu/drm/radeon/r600_cp.c         |   6 +-
>  drivers/gpu/drm/radeon/radeon_cp.c       |   6 +-
>  drivers/gpu/drm/radeon/radeon_drv.c      |  11 +-
>  drivers/gpu/drm/radeon/radeon_kms.c      |  23 ---
>  drivers/gpu/drm/radeon/si.c              |  14 +-
>  drivers/gpu/drm/savage/savage_drv.c      |   1 -
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c |   1 -
>  drivers/gpu/drm/sis/sis_drv.c            |   1 -
>  drivers/gpu/drm/tdfx/tdfx_drv.c          |   1 -
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c      |   1 -
>  drivers/gpu/drm/udl/udl_drv.c            |   1 -
>  drivers/gpu/drm/via/via_drv.c            |   1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c      |  20 ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h      |   1 -
>  drivers/gpu/host1x/drm/drm.c             |   1 -
>  drivers/staging/imx-drm/imx-drm-core.c   |  20 +--
>  include/drm/drmP.h                       |  65 ++-------
>  49 files changed, 239 insertions(+), 793 deletions(-)
>  delete mode 100644 drivers/gpu/drm/drm_proc.c
>
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux