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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

On 9 November 2017 at 23:46, Sharma, Deepak <Deepak.Sharma@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux