Re: [PATCH v1 0/11] drm: move dri1 drivers to drm/dri1/

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

 



Hi

Am 19.07.22 um 11:52 schrieb Daniel Vetter:
On Mon, 18 Jul 2022 at 08:56, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:

Hi

Am 16.07.22 um 20:17 schrieb Sam Ravnborg:
While discussing the way forward for the via driver
Javier came up with the proposal to move all DRI1 drivers
to their own folder.

The idea is to move the old DRI1 drivers so one do not
accidentally consider them modern drivers.

This set of patches implements this idea.

To prepare the move, DRIVER_LEGACY and CONFIG_DRM_LEGACY
are both renamed to *_DRI1. This makes it more obvious
that we are dealing with DRI1 drivers, as we have
a lot of other legacy support.

The drivers continue to have their own sub-directory
so the driver files are not mixed with the core files
which are copied in the last commit.

The DRI1 specific part of drm/Kconfig is likewise pulled
out and located in the dri1/ folder.

Feedback welcome!

To be honest, I still don't like this rename. Especially in the case of
via, which has a KMS driver coming up. It will now have an include
statement that crosses several levels in the directory hierarchy. And
what about the other DRI1 drivers? If we ever get KMS drivers for those,
do we want to move some header files back into their original locations?
Patches 1 and 2 look reasonable to me. The other driver patches have
basically zero upside IMHO.

Imo transitional drivers with both legacy dri1 and kms+gem support
made some sense 10+ years ago when all this infrastructure was still
being built. Now I really don't see much point.

For via imo make it a clean new driver, and copypaste anything from
the old one (like register headers) it needs. That will also make
review a ton easier I think. There has not been any actual via work,
just general refactoring, in that driver for 10+ years, so "bugfix
sharing" is really not an argument.

At some point, I suggested to turn the VIA KMS driver into a new 'unichrome' driver, but everyone wants to keep 'via' instead.


In the case of moving the core files into dri1/, the resulting Makefile
rule looks really ugly. I'd suggest to move all code into a separate
file drm_dri1.c and be done with it.  For something more elaborate,
there could by drm_dri1.c and drm_dri1_helper.c, where the latter
contains all DRI1 code that is only used by the drivers.

Ugly Makefile for dri1 might be a feature :-) But personally no stake
on this bikeshed.

It's the core DRM makefile that would look ugly. :/

Best regards
Thomas

-Daniel


Best regards
Thomsa


       Sam

Sam Ravnborg (11):
        drm: rename DRIVER_LEGACY to DRIVER_DRI1
        drm: Rename CONFIG_DRM_LEGACY to CONFIG_DRM_DRI1
        drm/tdfx: Move the tdfx driver to drm/dri1/
        drm/r128: Move the r128 driver to drm/dri1/
        drm/i810: Move the i810 driver to drm/dri1/
        drm/mga: Move the mga driver to drm/dri1/
        drm/sis: Move the sis driver to drm/dri1/
        drm/via: Move the via driver to drm/dri1/
        drm/savage: Move the savage driver to drm/dri1/
        drm/dri1: Move Kconfig logic to drm/dri1
        drm: Move dri1 core files to drm/dri1

   arch/powerpc/configs/pmac32_defconfig              |  2 +-
   arch/powerpc/configs/ppc6xx_defconfig              |  2 +-
   drivers/char/agp/Makefile                          |  2 +-
   drivers/char/agp/agp.h                             |  2 +-
   drivers/gpu/drm/Kconfig                            | 79 +---------------------
   drivers/gpu/drm/Makefile                           | 18 +++--
   drivers/gpu/drm/dri1/Kconfig                       | 79 ++++++++++++++++++++++
   drivers/gpu/drm/dri1/Makefile                      | 11 +++
   drivers/gpu/drm/{ => dri1}/drm_agpsupport.c        |  4 +-
   drivers/gpu/drm/{ => dri1}/drm_bufs.c              | 22 +++---
   drivers/gpu/drm/{ => dri1}/drm_context.c           | 24 +++----
   drivers/gpu/drm/{ => dri1}/drm_dma.c               |  4 +-
   drivers/gpu/drm/{ => dri1}/drm_hashtab.c           |  0
   drivers/gpu/drm/{ => dri1}/drm_irq.c               |  6 +-
   drivers/gpu/drm/{ => dri1}/drm_legacy_misc.c       |  2 +-
   drivers/gpu/drm/{ => dri1}/drm_lock.c              |  6 +-
   drivers/gpu/drm/{ => dri1}/drm_memory.c            |  0
   drivers/gpu/drm/{ => dri1}/drm_scatter.c           |  6 +-
   drivers/gpu/drm/{ => dri1}/drm_vm.c                |  2 +-
   drivers/gpu/drm/{ => dri1}/i810/Makefile           |  0
   drivers/gpu/drm/{ => dri1}/i810/i810_dma.c         |  0
   drivers/gpu/drm/{ => dri1}/i810/i810_drv.c         |  2 +-
   drivers/gpu/drm/{ => dri1}/i810/i810_drv.h         |  0
   drivers/gpu/drm/{ => dri1}/mga/Makefile            |  0
   drivers/gpu/drm/{ => dri1}/mga/mga_dma.c           |  0
   drivers/gpu/drm/{ => dri1}/mga/mga_drv.c           |  2 +-
   drivers/gpu/drm/{ => dri1}/mga/mga_drv.h           |  0
   drivers/gpu/drm/{ => dri1}/mga/mga_ioc32.c         |  0
   drivers/gpu/drm/{ => dri1}/mga/mga_irq.c           |  0
   drivers/gpu/drm/{ => dri1}/mga/mga_state.c         |  0
   drivers/gpu/drm/{ => dri1}/mga/mga_warp.c          |  0
   drivers/gpu/drm/{ => dri1}/r128/Makefile           |  0
   drivers/gpu/drm/{ => dri1}/r128/ati_pcigart.c      |  0
   drivers/gpu/drm/{ => dri1}/r128/ati_pcigart.h      |  0
   drivers/gpu/drm/{ => dri1}/r128/r128_cce.c         |  0
   drivers/gpu/drm/{ => dri1}/r128/r128_drv.c         |  2 +-
   drivers/gpu/drm/{ => dri1}/r128/r128_drv.h         |  0
   drivers/gpu/drm/{ => dri1}/r128/r128_ioc32.c       |  0
   drivers/gpu/drm/{ => dri1}/r128/r128_irq.c         |  0
   drivers/gpu/drm/{ => dri1}/r128/r128_state.c       |  0
   drivers/gpu/drm/{ => dri1}/savage/Makefile         |  0
   drivers/gpu/drm/{ => dri1}/savage/savage_bci.c     |  0
   drivers/gpu/drm/{ => dri1}/savage/savage_drv.c     |  2 +-
   drivers/gpu/drm/{ => dri1}/savage/savage_drv.h     |  0
   drivers/gpu/drm/{ => dri1}/savage/savage_state.c   |  0
   drivers/gpu/drm/{ => dri1}/sis/Makefile            |  0
   drivers/gpu/drm/{ => dri1}/sis/sis_drv.c           |  2 +-
   drivers/gpu/drm/{ => dri1}/sis/sis_drv.h           |  0
   drivers/gpu/drm/{ => dri1}/sis/sis_mm.c            |  0
   drivers/gpu/drm/{ => dri1}/tdfx/Makefile           |  0
   drivers/gpu/drm/{ => dri1}/tdfx/tdfx_drv.c         |  2 +-
   drivers/gpu/drm/{ => dri1}/tdfx/tdfx_drv.h         |  0
   drivers/gpu/drm/{ => dri1}/via/Makefile            |  4 +-
   drivers/gpu/drm/{via/via_dri1.c => dri1/via/via.c} |  4 +-
   drivers/gpu/drm/drm_drv.c                          |  2 +-
   drivers/gpu/drm/drm_file.c                         | 12 ++--
   drivers/gpu/drm/drm_internal.h                     |  2 +-
   drivers/gpu/drm/drm_ioc32.c                        | 12 ++--
   drivers/gpu/drm/drm_ioctl.c                        |  4 +-
   drivers/gpu/drm/drm_legacy.h                       | 32 ++++-----
   drivers/gpu/drm/drm_pci.c                          | 12 ++--
   drivers/gpu/drm/drm_vblank.c                       | 12 ++--
   include/drm/drm_auth.h                             |  2 +-
   include/drm/drm_device.h                           |  4 +-
   include/drm/drm_drv.h                              | 10 +--
   include/drm/drm_file.h                             |  2 +-
   include/drm/drm_legacy.h                           |  2 +-
   67 files changed, 205 insertions(+), 194 deletions(-)



--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev




--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[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