[PATCH] drm/vgem: Fix vgem_init to get drm device avaliable.

[Sharma, Deepak]

-----Original Message-----
From: Emil Velikov [mailto:emil.l.velikov at gmail.com] 
Sent: Friday, November 10, 2017 5:11 AM
To: Sharma, Deepak <Deepak.Sharma at amd.com>
Cc: ML dri-devel <dri-devel at lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher at amd.com>; Stéphane Marchesin <marcheu at chromium.org>
Subject: Re: [PATCH] drm/vgem: Fix vgem_init to get drm device avaliable.

On 9 November 2017 at 23:46, Sharma, Deepak <Deepak.Sharma at amd.com> wrote:

>>>
>>> Modify vgem_init to take platform dev as parent in drm_dev_init.
>>> This will make drm device available at "/sys/devices/platform/vgem"
>>> in x86 chromebook.
>>>
>> Shouldn't one update the drm_dev_init/drm_dev_alloc documentation while doing this?
>> But more importantly, this will change the "unique" string (see drm_dev_set_unique).
>
> Sorry I did not get your comment about updating 
> drm_dev_init/drm_dev_alloc documentation for this change. Do you see 
> any issue if this "unique string " is changed
>
VGEM is unlike other DRM drivers and I'm not sure if there is any userspace that depends on the exact value.
If there's none - great.

I'd still recommend updating the two functions' documentation, ideally coupled with enforcing for *parent to be non NULL.
Could be code as follow-up though.

If I got it correctly you are referring   "Note that for purely virtual devices @parent can be NULL" for said two functions.
I think changes might be required if it was "should/must be NULL"?

>> The topic around it rather convoluted and messy, so please check this change doesn't cause subtle regressions.
>> There's a doc hunk in drm_ioctl.c to begin with, plus userspace such as IGT [1] might rely on the current behaviour.
>>
>> HTH
>> Emil
>
> [1] https://cgit.freedesktop.org/drm/igt-gpu-tools/
>
> I did run vgem test from IGT to check for regression , do you suspect regression in other tests ?
>
There are two vgem only and a handful of others. Quick grep shows:

tests/amdgpu/amd_prime.c
tests/gem_concurrent_all.c
tests/gem_exec_await.c
tests/gem_exec_fence.c
tests/gem_exec_latency.c
tests/gem_exec_schedule.c
tests/gem_ringfill.c
tests/gem_wait.c
tests/prime_vgem.c
tests/vgem_basic.c
tests/vgem_slow.c

Most of which use i915 <> vgem. If you don't have the HW to test, one can use the Intel GFX trybot.
Just keep [1] in the To/CC list and you'll get a report with the results.

Thanks. I have added Intel GFX trybot in CC, that should be sufficient or I need to send patch again using git send-mail?

HTH
Emil

[1] intel-gfx-trybot at lists.freedesktop.org