-----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
