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

[Thomas Zimmermann]

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. 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.

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220719/8e427958/attachment.sig>