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
