[Mesa-dev] [PATCH 0/3] misc janitorial

[Emil Velikov]

On 30 November 2015 at 22:06, Dave Airlie <airlied at gmail.com> wrote:
> On 1 December 2015 at 07:18, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 30 November 2015 at 20:00, Ian Romanick <idr at freedesktop.org> wrote:
>>> On 11/29/2015 07:21 AM, Emil Velikov wrote:
>>>> Hi Giuseppe,
>>>>
>>>> On 28 November 2015 at 15:43, Giuseppe Bilotta
>>>> <giuseppe.bilotta at gmail.com> wrote:
>>>>> The second and third patches are mostly aimed at removing non-spurious
>>>>> compiler warnings. The first one is just minor whitespace cleanup in the
>>>>> general area of code touched by the second patch.
>>>>>
>>>>> Giuseppe Bilotta (3):
>>>>>   radeon: whitespace cleanup
>>>>>   radeon: const correctness
>>>>>   xvmc: force assertion in XvMC tests
>>>>>
>>>> With the small comment in patch 3 addressed the series is
>>>> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>>
>>>> If you're looking for an easy task - there is one in radeon/r200.
>>>> Currently we have a lot of nasty in-tree symlinks, symbol duplication
>>>> and hacks to get around it. Let me know if you'd like more info/tips
>>>> on how to tackle it.
>>>
>>> This is something that has been annoying me a bit lately.  What do you
>>> think the right solution is?  The Intel driver used to have a common
>>> src/mesa/drivers/dri/intel directory that was compiled once and shared
>>> by i915 and i965.  The code paths started to diverge quite a lot, so we
>>> copied the shared files into each directory... and became non-shared.
>>> Note that we didn't rename any of the function names, so we still have
>>> the ugly hacks to get around the symbol duplication.
>>>
>> I'm not sure about "right and wrong" but the possible solution I was
>> thinking is the one you mentioned - have a common place where that
>> code is built once.
>>
>> The long version:
>>  1. Grep for RADEON_R{1,2}00 and look at the guarded code
>>  1.1 Some guards can just be dropped (CHIP_FAMILY_*, get_chip_family_name),
>>  1.2 Other hunks of code can be split/moved into the respective
>> r100/r200 codebase (radeonTexBufferExtension and
>> r200TexBufferExtension).
>>  1.3 Or folded into the vtbl dispatch - get_depth_z32/16 or the
>> respective callers, depending on the overhead (neither one is likely
>> to be noticeable).
>>  2. Copy/pasta an existing radeon Makefile to radeon_common. Tweak to
>> produce an empty library and get it into the final mega_dri_drivers.so
>>  2.1 With most/all of the RADEON_R*00 guards gone, one should be able
>> to just git mv the files into the new location. (alongside dropping
>> the symlinks)
>>  2.2 Remove the symbol redefinition hack.
>>
>> It's a lengthy task, yet relatively easy. As long one tries to divide
>> and concur, rather than understand, in depth, how the driver/autohell
>> buildsystem works :-)
>> I'm more than willing to help out on the lot (esp. the latter), just
>> shout when you get confused/stuck.
>
> But why bother?
>
Three reasons - symlinks in git is frowned upon, binary is "bloated"
and nasty symbol redefinition(hack).

> Getting someone with r100/r200 hardware to test is the fun part,
>
> We've already seen that changes in this area untested mostly break stuff,
> rather than help, and go unnoticed for whole releases at a time.
>
Recent breakage and Ian's voice for testing is the one that reminded
be about this. Without the latter I've abandoned the idea some 2 years
ago. Again it's very low priority stuff in my book - just throwing it
out there for whoever's interested.

-Emil