[Mesa-dev] [PATCH] loader: add special logic to distinguish nouveau from nouveau_vieux

[Emil Velikov]

On 17/03/14 17:33, Ilia Mirkin wrote:
> On Mon, Mar 17, 2014 at 1:12 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 17/03/14 14:22, Ilia Mirkin wrote:
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Hi Ilia
>>
>> I'm not sure if a nouveau specific quirk is a nice idea, although the
>> only other solution is to add the "quirk" in the driver_map table.
> 
> Meh, I don't see what the big deal is. That function is there to
> abstract away all the nasties of figuring out which driver to use.
> Having a little specialized logic in there doesn't seem like such a
> big deal. This has the advantage that it doesn't require us to update
> it as new chipsets come out. This is nice, since things tend to be
> supported on a per-family basis. I suppose an alternative is to list
> out all the supported chip versions one by one, but IMO that's tedious
> and unnecessary, just to keep out a couple of easy-to-read lines from
> a rarey-looked-at piece of code.
> 
And at the same time, loader_get_pci_id_for_fd returns the device id,
despite the somewhat ambugious parameter name chip_id. Which means that
things will not works as expected with your approach.

>>
>>> ---
>>>  include/pci_ids/pci_id_driver_map.h |  1 -
>>>  src/loader/loader.c                 | 25 +++++++++++++++++++++++--
>>>  2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/pci_ids/pci_id_driver_map.h b/include/pci_ids/pci_id_driver_map.h
>>> index db9e07f..064e8d4 100644
>>> --- a/include/pci_ids/pci_id_driver_map.h
>>> +++ b/include/pci_ids/pci_id_driver_map.h
>>> @@ -73,7 +73,6 @@ static const struct {
>>>     { 0x1002, "r300", r300_chip_ids, ARRAY_SIZE(r300_chip_ids), _LOADER_GALLIUM },
>>>     { 0x1002, "r600", r600_chip_ids, ARRAY_SIZE(r600_chip_ids), _LOADER_GALLIUM },
>>>     { 0x1002, "radeonsi", radeonsi_chip_ids, ARRAY_SIZE(radeonsi_chip_ids), _LOADER_GALLIUM},
>>> -   { 0x10de, "nouveau", NULL, -1,  _LOADER_GALLIUM  },
>> +   { 0x10de, "nouveau_vieux", NULL, -1,  _LOADER_DRI  },
>>
>> This produces slightly shorter patch diff, although I'm not sure if this
>> isn't a bigger hack than your approach.
> 
> It also has the disadvantage of not working. dri2_glx.c passes in 0 as
> the driver_types, which maps to GALLIUM | DRI, so it would still load
> nouveau.
> 
sigh, I feared that there will be a lovely case like that somewhere. So
we're back to square one it seems.

-Emil

P.S. Presently _LOADER_DRI, seems somewhat unused.

>   -ilia
>