[Mesa-stable] [PATCH] radeonsi: add new OLAND pci id

[Emil Velikov]

On 13 August 2015 at 22:59, Alex Deucher <alexdeucher at gmail.com> wrote:
> On Thu, Aug 13, 2015 at 5:29 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 13/08/15 22:22, Emil Velikov wrote:
>>> On 13/08/15 18:11, Alex Deucher wrote:
>>>> On Thu, Aug 13, 2015 at 12:06 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>> On 13 August 2015 at 16:42, Alex Deucher <alexdeucher at gmail.com> wrote:
>>>>>> On Thu, Aug 13, 2015 at 11:11 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>> On 10 August 2015 at 20:36, Alex Deucher <alexdeucher at gmail.com> wrote:
>>>>>>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>>>>>>> Cc: mesa-stable at lists.freedesktop.org
>>>>>>>> ---
>>>>>>>>  include/pci_ids/radeonsi_pci_ids.h | 1 +
>>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git a/include/pci_ids/radeonsi_pci_ids.h b/include/pci_ids/radeonsi_pci_ids.h
>>>>>>>> index c01ee20..52eada1 100644
>>>>>>>> --- a/include/pci_ids/radeonsi_pci_ids.h
>>>>>>>> +++ b/include/pci_ids/radeonsi_pci_ids.h
>>>>>>>> @@ -63,6 +63,7 @@ CHIPSET(0x6608, OLAND_6608, OLAND)
>>>>>>>>  CHIPSET(0x6610, OLAND_6610, OLAND)
>>>>>>>>  CHIPSET(0x6611, OLAND_6611, OLAND)
>>>>>>>>  CHIPSET(0x6613, OLAND_6613, OLAND)
>>>>>>>> +CHIPSET(0x6617, OLAND_6617, OLAND)
>>>>>>> Has there been any ideas/plans on getting this information
>>>>>>> consolidated in a single place ?
>>>>>>>
>>>>>>> It feels a bit "dirty" having the same information in four places -
>>>>>>> kernel, libdrm, ddx, mesa.
>>>>>>
>>>>>> There's not really a good solution that I know of due to the way X
>>>>>> works.
>>>>> If I have to guess, obtaining OLAND via DRM_IOCTL_RADEON_INFO won't be
>>>>> impacted by (nor have any impact on) how X works. Shouldn't this "just
>>>>> work" or there is something subtly off with the idea ? Can you
>>>>> elaborate what part of X might be an obstacle ?
>>>>
>>>> IIRC, X decides what driver to load based on the pci ids they support
>>>> rather than letting drivers claim their devices.  It's remnant of the
>>>> UMS days.
>>>>
>>> I think we're talking about different things here.
>>>
>>> * The (dare I say it) detection code used to determine the ddx/dri
>>> module name happens before (and afaics is unrelated to) the driver
>>> internals, which depend on the OLAND{,_foo} values.
>>>
>>> Obviously there is a handfull of extra information about "all the
>>> supported vendor/device ids" in the ddx that you're thinking about, that
>>> I'm afraid one cannot get rid of, unless...
>>>
>>> * The DDX uses a nouveau-like approach, accepting every device id. At
>>> PreInit stage, one does a quick check with libdrm_radeon/amdgpu
>>> (amdgpu_device_initialize?). The latter of which already has a
>>> comprehensive enough list of device ids.
>>>
>> Actually scratch this part, libdrm_radeon does have a list of the
>> devices, but does not have a "device_init" type API. On the other hand
>> amdgpu has the API, but doesn't have the device ids.
>>
>> The earlier (original) idea still stands though:
>> By the time OLAND* is queried/used in the ddx/mesa, the ddx/dri module
>> has already been picked up, irrespective of that extra info
>
> I don't think that will work.
Seems like I wasn't clear enough. Let me try one final time.

> The X server attempts to load the ddxes
> based on the pci vendor id unless you manually specify the driver in
> your xorg.conf.  In the case of 0x1002 AMD/ATI, there are 5 drivers
> for device ids for that vendor id: ati_misc, mach64, r128, radeon, and
> amdgpu.
At this point the "CHIPSET(0x6617, OLAND_6617, OLAND)" information is
not used/needed, is it ?

> The ddx
Here we already have the fd or the bus information, the latter of
which can be used to retrieve the former. Now we can fetch OLAND/etc.
using the ioctl.

> then tells mesa what driver to load via dri2 based on
> the pci id in the ddx.
>
For 1.5 years, the mesa "loader" takes presedence over the driver_name
sent over the dri2 protocol. Regardless, at this stage we already have
the fd so

Note that I'm not trying to persuade anyone that it must be done, just
pointing out that it's perfectly doable.

Thanks
Emil