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.
> 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.
-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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
