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. [...] >> >>> 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 >> >> Maybe this could be sorted by splitting the DRI1 core bits in a separate >> drm_dri1.ko module? > > The dri1 core files cannot be in a separate module as they are linked > with other core stuff. It would result in dependency cycles IIRC. > Got it. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
