On 05/30/2015 10:20 PM, Emil Velikov wrote:
On 29 May 2015 at 07:50, Joonyoung Shim jy0922.shim@samsung.com wrote:
On 05/29/2015 12:51 AM, Emil Velikov wrote:
Seems like I'm either too subtle and/or too stingy earlier.
If drmAvailable() returns false, we have two options:
- opt for the old-schoold (dri1) and ask drm_server_info to load the
module for us, or
- bail out, as neither drmOpenByBusid() or drmOpenByName() will be
able to open the device considering that a DRM module is not loaded.
So what I was hinting earlier was to make the above more obvious, rather than reordering the arguments in the if clause. How does that sound ?
I'm unhappy about to open DRM device always via drmAvailable(). IMHO it's enough to check DRM device can be open by drmOpenByBusid() or drmOpenByName() if don't load module and actually i expect DRM device is open only once when call drmOpenWithType().
Seems that checking via drmAvailable() is quicker than going through the whole drmOpen*. On the other side the former does not cater for render only devices... fun.
If want to check via drmAvailable(), we can call it regardless of drmOpen*.
Seems like this is the price to pay, considering that our current libdrm caters after both UMS(DRI1) and KMS(DRI2+) drivers. In the kernel we did have some nice cleanup (props to Daniel V and others) although in the userspace side things are less polished.
Perhaps we can get libdrm 3 soon or start annotating the old functions as depreciated ? Curious if one should add libdrm_ancient.so (or similar) to ensure that things don't break when building old apps against new libdrm ?
TLDR: No objections against the patch, but we should consider cleaning/splitting the ancient APIs.
Agree but what i wanted to say is just to remove unnecessary to open DRM device in drmOpen*.