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

[Javier Martinez Canillas]

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