[Mesa-dev] [PATCH 5/5] tegra: Initial support

[Dmitry Osipenko]

On 22.02.2018 18:16, Thierry Reding wrote:
> On Thu, Feb 22, 2018 at 02:31:48PM +0100, Erik Faye-Lund wrote:
>> On Thu, Feb 22, 2018 at 2:23 PM, Thierry Reding
>> <thierry.reding at gmail.com> wrote:
>>> On Wed, Feb 21, 2018 at 05:01:02PM +0000, Emil Velikov wrote:
>>>> Hi Thierry,
>>>>
>>>> On 21 February 2018 at 15:30, Thierry Reding <thierry.reding at gmail.com> wrote:
>>>>> +static const char *
>>>>> +tegra_screen_get_name(struct pipe_screen *pscreen)
>>>>> +{
>>>>> +   return "tegra";
>>>>> +}
>>>>> +
>>>>> +static const char *
>>>>> +tegra_screen_get_vendor(struct pipe_screen *pscreen)
>>>>> +{
>>>>> +   return "NVIDIA";
>>>>> +}
>>>>> +
>>>>> +static const char *
>>>>> +tegra_screen_get_device_vendor(struct pipe_screen *pscreen)
>>>>> +{
>>>>> +   return "NVIDIA";
>>>>> +}
>>>>> +
>>>> As-is these might be a bit fiddly, but will do for now.
>>>> For example - how will devs distinguish between the closed-source
>>>> driver and Mesa.
>>>
>>> Good point. Let me check what exactly we use in the closed-source driver
>>> and then come up with a proposal.
>>>
>>> I think perhaps a good choice for the vendor would be "grate". Even
>>> though this driver isn't part of the grate project, I'm hoping that we
>>> will eventually see Erik's and Dmitry's work merged into this.
>>>
>>> Adding Erik and Dmitry to get their opinion.
>>>
>>
>> It's not entirely clear to me what the most natural boundary between
>> Tegra 2/3/4 (the GPUs the grate-project targets) and Tegra K1 would
>> be. The later Tegras are so fundamentally different in how they
>> work...
>>
>> Should they share the implementation of tegra_screen_create? From a
>> quick look, it doesn't seem like it (mostly K1 specifics going on
>> there, it seems), but I could be wrong. And if it shouldn't perhaps it
>> should simply be a separate gallium-driver ("grate" vs "tegra"
>> maybe?)... We can probably share some code, but we can do that without
>> sharing the driver, just like intel, amd and broadcom does with just a
>> shared folder at src/nvidia or something for utilities...
>>
>> I don't know, just trying to avoid having to add too many conditionals here...
> 
> I agree with you mostly. I don't think there's a lot that could be
> shared here. On the other hand, I think we can easily add the
> conditionals to only a few places. We could for example have a
> grate_screen_create() function that is called by tegra_screen_create()
> after detecting whether or not we have a Nouveau GPU in the system (as
> fallback for a tegra_open_render_node() failure). Or we could do it
> earlier after checking for Tegra chip ID in sysfs, or perhaps some DT
> compatible string (via the drmDevice API).
> 
> That way there is one conditional in the initial stages, but we get to
> deal with less fragmentation. I think it gives users a better experience
> if they just install the Tegra driver and it works regardless of whether
> it runs on a Tegra20 or a Tegra124.
> 
> We have a pretty consistent experience across all Tegra generations, and
> I'd really like to keep that.
> 
> All of that said, I'm not exactly sure I understand the purpose of the
> vendor name here. We already have the screen name that identifies this
> as a Tegra screen and we have the device vendor which identifies who
> built the device. So vendor seems to me like it would be whoever
> provides the driver. In this case, couldn't that just be "Mesa"?

We would have to have a common tegra_drm_screen_create() in order to avoid the
error-message noise during of device probing. For now you may add a stub that
checks if bustype==DRM_BUS_HOST1X and make tegra_screen_create() use it to bail
out silently if stub fails, that way pre-K1 Tegra's won't get odd error
messages. Maybe it would be handy to have a libdrm-tegra helper for that, not sure..

I'm for having "grate" driver for older Tegra's and "tegra" for newer. So I'm
fine with this patch, only would nice to avoid the error noise.

It's awesome that finally something is getting to upstream ;)