Hi Sam Am 19.07.22 um 09:55 schrieb Sam Ravnborg:
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. AndThat 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.
Sounds like a plan to me. Best regards Thomas
Sam
-- 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
