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

Sam Ravnborg sam at ravnborg.org
Tue Jul 19 07:55:49 UTC 2022


Hi Javier, Thomas,

On Mon, Jul 18, 2022 at 02:18:13PM +0200, Javier Martinez Canillas wrote:
> On 7/18/22 12:50, Thomas Zimmermann wrote:
> 
> [...]
> 
> >>> 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
> >>
> >> That could be avoided by moving drivers/gpu/drm/via/via_3d_reg.h and other
> >> header files to include/drm/via_3d_reg.h for example. Other drivers (i.e:
> >> i915) do the same for headers that are shared across different subsystems.
> >>   
> >>> 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?
> >>
> >> I believe for these we could also move them to include/drm/ if needed.
> > 
> > That pollutes these shared directories at the expense of everyone else. 
> > If anything, we want to move files out of the shared include paths.
> >
> 
> Yes, every option has a different set of trade offs.
>  
> > It would make sense to me if we'd have two distinct drivers. But here, 
> > the new and the old driver is really just one DRM driver with badly 
> > organized source code.
> >
> 
> I see. I haven't looked at the via drivers in detail.
> 
> >>   
> >>> Patches 1 and 2 look reasonable to me. The other driver patches have
> >>> basically zero upside IMHO.
> >>>
> >>
> >> I disagree with the zero upside. It may be that the trade offs are not
> >> worth it but there are upsides of having all DRI1 drivers and core DRI1
> >> bits in the same place. It makes grep-ing and reading files easier if
> >> one doesn't care about legacy DRI1 drivers.
> > 
> > The grep-ability is a minor point. It does come up, but is usually 
> > sorted out easily.
> > 
> > If we want to improve grep output, we should consider applying Sam's 
> > via_dri1 changes to all DRI1 drivers. So we'd end up with a single 
> > mga_dri1.c, tdfx_dri1.c, savage_dri1.c and so on. If the core/helper 
> > code is also in a _dri1-named source file, grep runs can filter out 
> > those filenames.
> >
> 
> Having everything with a _dri1 suffix would be an improvement I agree
> and if that's the consensus then I'm OK with that approach as well.
> 
> [...]

I will update the series with the following:
- Drop drm/dri1/
- Keep the CONFIG_DRM_* change and keep the DRIVER_DRI1 change
- All config options for DRI1 drivers will get a CONFIG_DRM_DRI1_*
  prefix
- Convert at least some of the drivers to single file drivers named
  foo_dri1. 
- I may add SPDX for licenses when I am touching the files
- I may try to concatenate all dri1 specific drm core files to drm_dri1.
  It is easy to do but I will take a look at the result before posting
  anything. 

When I have posted the above let's see what we all agree on.
May take a couple of days before I get back to it.

	Sam


More information about the dri-devel mailing list