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

[Dave Airlie]

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?

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.

Dave.